From 98441973105b80e133fcaa47ebf17be1e024ea30 Mon Sep 17 00:00:00 2001 From: Sarah Sharp Date: Thu, 14 May 2009 11:44:18 -0700 Subject: [PATCH] USB: xhci: Remove packed attribute from structures. 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 Signed-off-by: Greg Kroah-Hartman --- drivers/usb/host/xhci-dbg.c | 2 +- drivers/usb/host/xhci-hcd.c | 19 +++++++++++++++ drivers/usb/host/xhci.h | 46 ++++++++++++++++++------------------- 3 files changed, 43 insertions(+), 24 deletions(-) diff --git a/drivers/usb/host/xhci-dbg.c b/drivers/usb/host/xhci-dbg.c index 6473cbf329f9..2501c571f855 100644 --- a/drivers/usb/host/xhci-dbg.c +++ b/drivers/usb/host/xhci-dbg.c @@ -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; diff --git a/drivers/usb/host/xhci-hcd.c b/drivers/usb/host/xhci-hcd.c index c6b921994b28..59ee61d2a198 100644 --- a/drivers/usb/host/xhci-hcd.c +++ b/drivers/usb/host/xhci-hcd.c @@ -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); diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index df8778e1cfc6..3e8e09c4c2bf 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -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); -- 2.20.1