USB: xhci: Remove packed attribute from structures.
authorSarah Sharp <sarah.a.sharp@linux.intel.com>
Thu, 14 May 2009 18:44:18 +0000 (11:44 -0700)
committerGreg Kroah-Hartman <gregkh@suse.de>
Tue, 16 Jun 2009 04:44:51 +0000 (21:44 -0700)
The packed attribute allows gcc to muck with the alignment of data
structures, which may lead to byte-wise writes that break atomicity of
writes.  Packed should only be used when the compile may add undesired
padding to the structure.  Each element of the structure will be aligned
by C based on its size and the size of the elements around it.  E.g. a u64
would be aligned on an 8 byte boundary, the next u32 would be aligned on a
four byte boundary, etc.

Since most of the xHCI structures contain only u32 bit values, removing
the packed attribute for them should be harmless.  (A future patch will
change some of the twin 32-bit address fields to one 64-bit field, but all
those places have an even number of 32-bit fields before them, so the
alignment should be correct.)  Add BUILD_BUG_ON statements to check that
the compiler doesn't add padding to the data structures that have a
hardware-defined layout.

While we're modifying the registers, change the name of intr_reg to
xhci_intr_reg to avoid global conflicts.

Signed-off-by: Sarah Sharp <sarah.a.sharp@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
drivers/usb/host/xhci-dbg.c
drivers/usb/host/xhci-hcd.c
drivers/usb/host/xhci.h

index 6473cbf..2501c57 100644 (file)
@@ -169,7 +169,7 @@ static void xhci_print_ports(struct xhci_hcd *xhci)
        }
 }
 
-void xhci_print_ir_set(struct xhci_hcd *xhci, struct intr_reg *ir_set, int set_num)
+void xhci_print_ir_set(struct xhci_hcd *xhci, struct xhci_intr_reg *ir_set, int set_num)
 {
        void *addr;
        u32 temp;
index c6b9219..59ee61d 100644 (file)
@@ -1243,6 +1243,25 @@ static int __init xhci_hcd_init(void)
                return retval;
        }
 #endif
+       /*
+        * Check the compiler generated sizes of structures that must be laid
+        * out in specific ways for hardware access.
+        */
+       BUILD_BUG_ON(sizeof(struct xhci_doorbell_array) != 256*32/8);
+       BUILD_BUG_ON(sizeof(struct xhci_slot_ctx) != 8*32/8);
+       BUILD_BUG_ON(sizeof(struct xhci_ep_ctx) != 8*32/8);
+       /* xhci_device_control has eight fields, and also
+        * embeds one xhci_slot_ctx and 31 xhci_ep_ctx
+        */
+       BUILD_BUG_ON(sizeof(struct xhci_device_control) != (8+8+8*31)*32/8);
+       BUILD_BUG_ON(sizeof(struct xhci_stream_ctx) != 4*32/8);
+       BUILD_BUG_ON(sizeof(union xhci_trb) != 4*32/8);
+       BUILD_BUG_ON(sizeof(struct xhci_erst_entry) != 4*32/8);
+       BUILD_BUG_ON(sizeof(struct xhci_cap_regs) != 7*32/8);
+       BUILD_BUG_ON(sizeof(struct xhci_intr_reg) != 8*32/8);
+       /* xhci_run_regs has eight fields and embeds 128 xhci_intr_regs */
+       BUILD_BUG_ON(sizeof(struct xhci_run_regs) != (8+8*128)*32/8);
+       BUILD_BUG_ON(sizeof(struct xhci_doorbell_array) != 256*32/8);
        return 0;
 }
 module_init(xhci_hcd_init);
index df8778e..3e8e09c 100644 (file)
@@ -71,7 +71,7 @@ struct xhci_cap_regs {
        u32     db_off;
        u32     run_regs_off;
        /* Reserved up to (CAPLENGTH - 0x1C) */
-} __attribute__ ((packed));
+};
 
 /* hc_capbase bitmasks */
 /* bits 7:0 - how long is the Capabilities register */
@@ -180,7 +180,7 @@ struct xhci_op_regs {
        u32     reserved5;
        /* registers for ports 2-255 */
        u32     reserved6[NUM_PORT_REGS*254];
-} __attribute__ ((packed));
+};
 
 /* USBCMD - USB command - command bitmasks */
 /* start/stop HC execution - do not write unless HC is halted*/
@@ -361,7 +361,7 @@ struct xhci_op_regs {
 
 
 /**
- * struct intr_reg - Interrupt Register Set
+ * struct xhci_intr_reg - Interrupt Register Set
  * @irq_pending:       IMAN - Interrupt Management Register.  Used to enable
  *                     interrupts and check for pending interrupts.
  * @irq_control:       IMOD - Interrupt Moderation Register.
@@ -377,14 +377,14 @@ struct xhci_op_regs {
  * position of the Enqueue Pointer." The HCD (Linux) processes those events and
  * updates the dequeue pointer.
  */
-struct intr_reg {
+struct xhci_intr_reg {
        u32     irq_pending;
        u32     irq_control;
        u32     erst_size;
        u32     rsvd;
        u32     erst_base[2];
        u32     erst_dequeue[2];
-} __attribute__ ((packed));
+};
 
 /* irq_pending bitmasks */
 #define        ER_IRQ_PENDING(p)       ((p) & 0x1)
@@ -428,10 +428,10 @@ struct intr_reg {
  * or larger accesses"
  */
 struct xhci_run_regs {
-       u32     microframe_index;
-       u32     rsvd[7];
-       struct intr_reg ir_set[128];
-} __attribute__ ((packed));
+       u32                     microframe_index;
+       u32                     rsvd[7];
+       struct xhci_intr_reg    ir_set[128];
+};
 
 /**
  * struct doorbell_array
@@ -440,7 +440,7 @@ struct xhci_run_regs {
  */
 struct xhci_doorbell_array {
        u32     doorbell[256];
-} __attribute__ ((packed));
+};
 
 #define        DB_TARGET_MASK          0xFFFFFF00
 #define        DB_STREAM_ID_MASK       0x0000FFFF
@@ -470,7 +470,7 @@ struct xhci_slot_ctx {
        u32     dev_state;
        /* offset 0x10 to 0x1f reserved for HC internal use */
        u32     reserved[4];
-} __attribute__ ((packed));
+};
 
 /* dev_info bitmasks */
 /* Route String - 0:19 */
@@ -542,7 +542,7 @@ struct xhci_ep_ctx {
        u32     tx_info;
        /* offset 0x14 - 0x1f reserved for HC internal use */
        u32     reserved[3];
-} __attribute__ ((packed));
+};
 
 /* ep_info bitmasks */
 /*
@@ -601,7 +601,7 @@ struct xhci_device_control {
        u32     rsvd[6];
        struct xhci_slot_ctx    slot;
        struct xhci_ep_ctx      ep[31];
-} __attribute__ ((packed));
+};
 
 /* drop context bitmasks */
 #define        DROP_EP(x)      (0x1 << x)
@@ -644,7 +644,7 @@ struct xhci_device_context_array {
        u32                     dev_context_ptrs[2*MAX_HC_SLOTS];
        /* private xHCD pointers */
        dma_addr_t      dma;
-} __attribute__ ((packed));
+};
 /* TODO: write function to set the 64-bit device DMA address */
 /*
  * TODO: change this to be dynamically sized at HC mem init time since the HC
@@ -657,7 +657,7 @@ struct xhci_stream_ctx {
        u32     stream_ring[2];
        /* offset 0x14 - 0x1f reserved for HC internal use */
        u32     reserved[2];
-} __attribute__ ((packed));
+};
 
 
 struct xhci_transfer_event {
@@ -666,7 +666,7 @@ struct xhci_transfer_event {
        u32     transfer_len;
        /* This field is interpreted differently based on the type of TRB */
        u32     flags;
-} __attribute__ ((packed));
+};
 
 /** Transfer Event bit fields **/
 #define        TRB_TO_EP_ID(p) (((p) >> 16) & 0x1f)
@@ -747,7 +747,7 @@ struct xhci_link_trb {
        u32 segment_ptr[2];
        u32 intr_target;
        u32 control;
-} __attribute__ ((packed));
+};
 
 /* control bitfields */
 #define LINK_TOGGLE    (0x1<<1)
@@ -758,7 +758,7 @@ struct xhci_event_cmd {
        u32 cmd_trb[2];
        u32 status;
        u32 flags;
-} __attribute__ ((packed));
+};
 
 /* flags bitmasks */
 /* bits 16:23 are the virtual function ID */
@@ -809,7 +809,7 @@ struct xhci_event_cmd {
 
 struct xhci_generic_trb {
        u32 field[4];
-} __attribute__ ((packed));
+};
 
 union xhci_trb {
        struct xhci_link_trb            link;
@@ -904,7 +904,7 @@ struct xhci_segment {
        /* private to HCD */
        struct xhci_segment     *next;
        dma_addr_t              dma;
-} __attribute__ ((packed));
+};
 
 struct xhci_td {
        struct list_head        td_list;
@@ -946,7 +946,7 @@ struct xhci_erst_entry {
        u32     seg_size;
        /* Set to zero */
        u32     rsvd;
-} __attribute__ ((packed));
+};
 
 struct xhci_erst {
        struct xhci_erst_entry  *entries;
@@ -980,7 +980,7 @@ struct xhci_hcd {
        struct xhci_run_regs __iomem *run_regs;
        struct xhci_doorbell_array __iomem *dba;
        /* Our HCD's current interrupter register set */
-       struct  intr_reg __iomem *ir_set;
+       struct  xhci_intr_reg __iomem *ir_set;
 
        /* Cached register copies of read-only HC data */
        __u32           hcs_params1;
@@ -1079,7 +1079,7 @@ static inline void xhci_writel(struct xhci_hcd *xhci,
 }
 
 /* xHCI debugging */
-void xhci_print_ir_set(struct xhci_hcd *xhci, struct intr_reg *ir_set, int set_num);
+void xhci_print_ir_set(struct xhci_hcd *xhci, struct xhci_intr_reg *ir_set, int set_num);
 void xhci_print_registers(struct xhci_hcd *xhci);
 void xhci_dbg_regs(struct xhci_hcd *xhci);
 void xhci_print_run_regs(struct xhci_hcd *xhci);