[sparc64] 2.6.18 unaligned acccess in ehci_hub_control

[sparc64] 2.6.18 unaligned acccess in ehci_hub_control

[Will Simoneau]

[-- Attachment #1: Type: text/plain, Size: 1073 bytes --]

I upgraded from 2.6.17.7 to 2.6.18 today, and in dmesg I have 5 of these
messages in a row:

Kernel unaligned access at TPC[100be8c8] ehci_hub_control+0x350/0x680 [ehci_hcd]

This message wasn't there before... I suppose it is pretty harmless as
the kernel is supposed to handle unaligned accesses (right?) but this is
the first time it's happened.

According to gdb (my first time using it...),
drivers/usb/host/ehci-hub.c:280 is to blame:

(gdb) list *ehci_hub_control+0x350
0x8d0 is in ehci_hub_control (drivers/usb/host/ehci-hub.c:280).
275      struct usb_hub_descriptor  *desc
276   ) {
277      int      ports = HCS_N_PORTS (ehci->hcs_params);
278      u16      temp;
279   
280      desc->bDescriptorType = 0x29;
281      desc->bPwrOn2PwrGood = 10; /* ehci 1.0, 2.3.9 says 20ms max */
282      desc->bHubContrCurrent = 0;
283   
284      desc->bNbrPorts = ports;

System is a Sun U80 4x450 with 2.5G and a USB2 PCI card. GCC for the
kernel is 4.1.1 (with gentoo patchset). The only USB2 device is an
external Sony dual layer DVD writer.

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [sparc64] 2.6.18 unaligned acccess in ehci_hub_control

[David Miller]

From: Will Simoneau <simoneau@ele.uri.edu>
Date: Wed, 20 Sep 2006 13:21:23 -0400

> I upgraded from 2.6.17.7 to 2.6.18 today, and in dmesg I have 5 of these
> messages in a row:
> 
> Kernel unaligned access at TPC[100be8c8] ehci_hub_control+0x350/0x680 [ehci_hcd]
> 
> This message wasn't there before... I suppose it is pretty harmless as
> the kernel is supposed to handle unaligned accesses (right?) but this is
> the first time it's happened.

Yes, I've been meaning to send Greg KH patches to fix these
cases, thanks for reminding me about it.

Re: [sparc64] 2.6.18 unaligned acccess in ehci_hub_control

[Mikael Pettersson]

David Miller writes:
 > From: Will Simoneau <simoneau@ele.uri.edu>
 > Date: Wed, 20 Sep 2006 13:21:23 -0400
 > 
 > > I upgraded from 2.6.17.7 to 2.6.18 today, and in dmesg I have 5 of these
 > > messages in a row:
 > > 
 > > Kernel unaligned access at TPC[100be8c8] ehci_hub_control+0x350/0x680 [ehci_hcd]
 > > 
 > > This message wasn't there before... I suppose it is pretty harmless as
 > > the kernel is supposed to handle unaligned accesses (right?) but this is
 > > the first time it's happened.
 > 
 > Yes, I've been meaning to send Greg KH patches to fix these
 > cases, thanks for reminding me about it.

I don't think it's harmless. My Ultra5 has an add-on PCI USB controller
card (Belkin). A 2.6.18-rc kernel compiled with gcc-4.1.1 will throw a few
unaligned accesses when I initialise USB by inserting a USB memory stick.
Removing the memory stick then results in PCI errors and other breakage.

The same kernel compiled with gcc-3.4.6 has no problems at all, so I've
been assuming it's a gcc-4 issue and not a kernel issue.

/Mikael

Re: [sparc64] 2.6.18 unaligned acccess in ehci_hub_control

[David Miller]

From: Mikael Pettersson <mikpe@it.uu.se>
Date: Thu, 21 Sep 2006 12:37:09 +0200

> I don't think it's harmless. My Ultra5 has an add-on PCI USB controller
> card (Belkin). A 2.6.18-rc kernel compiled with gcc-4.1.1 will throw a few
> unaligned accesses when I initialise USB by inserting a USB memory stick.
> Removing the memory stick then results in PCI errors and other breakage.
> 
> The same kernel compiled with gcc-3.4.6 has no problems at all, so I've
> been assuming it's a gcc-4 issue and not a kernel issue.

Compiled with gcc-4.0.x I get the same ehci_hub_control unaligned
accesses, and putting the correct {get,put}_unaligned() in that
function makes them go away.

It's a pure mystery if gcc-3.4.x somehow avoids those, as by the
way the code is written those unaligned accesses are to be expected.

Re: [sparc64] 2.6.18 unaligned acccess in ehci_hub_control

[Mikael Pettersson]

On Thu, 21 Sep 2006 13:51:21 -0700 (PDT), David Miller wrote:
>> I don't think it's harmless. My Ultra5 has an add-on PCI USB controller
>> card (Belkin). A 2.6.18-rc kernel compiled with gcc-4.1.1 will throw a few
>> unaligned accesses when I initialise USB by inserting a USB memory stick.
>> Removing the memory stick then results in PCI errors and other breakage.
>> 
>> The same kernel compiled with gcc-3.4.6 has no problems at all, so I've
>> been assuming it's a gcc-4 issue and not a kernel issue.
>
>Compiled with gcc-4.0.x I get the same ehci_hub_control unaligned
>accesses, and putting the correct {get,put}_unaligned() in that
>function makes them go away.
>
>It's a pure mystery if gcc-3.4.x somehow avoids those, as by the
>way the code is written those unaligned accesses are to be expected.

I rechecked with 2.6.18 final, and the behaviour is as I described:
gcc-4.1.1 causes the alignment exceptions, while gcc-3.4.6 does not.
I didn't get any PCI errors now, but I'm sure I did get them in the
2.6.17 or early 2.6.18-rc kernels.

Here's a dmesg diff to show what happens, between the gcc-4.1.1
and gcc-3.4.6 compiled kernels. The first part is from kernel
bootup + user-space modprobe of the USB EHCI etc modules:

--- dmesg-2.6.18.usb-remove	2006-09-22 22:43:36.000000000 +0200
+++ dmesg-2.6.18-gcc346.usb-remove	2006-09-22 23:30:36.000000000 +0200
@@ -1,20 +1,20 @@
 PROMLIB: Sun IEEE Boot Prom 'OBP 3.25.3 2000/06/29 14:12'
 PROMLIB: Root node compatible: 
-Linux version 2.6.18 (mikpe@sparge) (gcc version 4.1.1) #1 Fri Sep 22 22:32:42 CEST 2006
+Linux version 2.6.18-gcc346 (mikpe@sparge) (gcc version 3.4.6) #1 Fri Sep 22 23:09:51 CEST 2006
 ARCH: SUN4U
 Ethernet address: 08:00:20:fd:ec:1f
 PROM: Built device tree with 46570 bytes of memory.
-On node 0 totalpages: 32312
-  DMA zone: 32312 pages, LIFO batch:7
+On node 0 totalpages: 32309
+  DMA zone: 32309 pages, LIFO batch:7
 CPU[0]: Caches D[sz(16384):line_sz(32)] I[sz(16384):line_sz(32)] E[sz(2097152):line_sz(64)]
-Built 1 zonelists.  Total pages: 32312
+Built 1 zonelists.  Total pages: 32309
 Kernel command line: ro root=/dev/hda5
 PID hash table entries: 1024 (order: 10, 8192 bytes)
 Console: colour dummy device 80x25
 Dentry cache hash table entries: 32768 (order: 5, 262144 bytes)
 Inode-cache hash table entries: 16384 (order: 4, 131072 bytes)
-Memory: 255928k available (1736k kernel code, 624k data, 112k init) [fffff80000000000,0000000017f46000]
-Calibrating delay using timer specific routine.. 800.26 BogoMIPS (lpj=4001318)
+Memory: 255872k available (1752k kernel code, 624k data, 112k init) [fffff80000000000,0000000017f46000]
+Calibrating delay using timer specific routine.. 800.30 BogoMIPS (lpj=4001524)
 Mount-cache hash table entries: 512
 NET: Registered protocol family 16
 PCI: Probing for controllers.
@@ -88,9 +88,6 @@
 usb usb1: configuration #1 chosen from 1 choice
 hub 1-0:1.0: USB hub found
 hub 1-0:1.0: 3 ports detected
-Kernel unaligned access at TPC[1006a9e4] ehci_hub_control+0x54c/0x68c [ehci_hcd]
-Kernel unaligned access at TPC[1006a9e4] ehci_hub_control+0x54c/0x68c [ehci_hcd]
-Kernel unaligned access at TPC[1006a9e4] ehci_hub_control+0x54c/0x68c [ehci_hcd]
 ohci_hcd: 2005 April 22 USB 1.1 'Open' Host Controller (OHCI) Driver (PCI)
 ohci_hcd 0000:02:01.0: OHCI Host Controller
 ohci_hcd 0000:02:01.0: new USB bus registered, assigned bus number 2

The following messages are from when I insert the USB memory stick:

@@ -112,11 +109,6 @@
 EXT3 FS on hda2, internal journal
 EXT3-fs: mounted filesystem with ordered data mode.
 Adding 1048808k swap on /dev/hda4.  Priority:-1 extents:1 across:1048808k
-Kernel unaligned access at TPC[1006a9e4] ehci_hub_control+0x54c/0x68c [ehci_hcd]
-Kernel unaligned access at TPC[1006a9e4] ehci_hub_control+0x54c/0x68c [ehci_hcd]
-Kernel unaligned access at TPC[1006a9e4] ehci_hub_control+0x54c/0x68c [ehci_hcd]
-Kernel unaligned access at TPC[1006a9e4] ehci_hub_control+0x54c/0x68c [ehci_hcd]
-Kernel unaligned access at TPC[1006a9e4] ehci_hub_control+0x54c/0x68c [ehci_hcd]
 usb 1-2: new high speed USB device using ehci_hcd and address 2
 usb 1-2: configuration #1 chosen from 1 choice
 SCSI subsystem initialized

The following messages are from when I remove the USB memory stick:

@@ -139,9 +131,4 @@
 sda: assuming drive cache: write through
  sda: sda1
 sd 0:0:0:0: Attached scsi removable disk sda
-Kernel unaligned access at TPC[1006a9e4] ehci_hub_control+0x54c/0x68c [ehci_hcd]
 usb 1-2: USB disconnect, address 2
-Kernel unaligned access at TPC[1006a9e4] ehci_hub_control+0x54c/0x68c [ehci_hcd]
-Kernel unaligned access at TPC[1006a9e4] ehci_hub_control+0x54c/0x68c [ehci_hcd]
-Kernel unaligned access at TPC[1006a9e4] ehci_hub_control+0x54c/0x68c [ehci_hcd]
-Kernel unaligned access at TPC[1006a9e4] ehci_hub_control+0x54c/0x68c [ehci_hcd]

/Mikael

Re: [sparc64] 2.6.18 unaligned acccess in ehci_hub_control

[David Miller]

From: Mikael Pettersson <mikpe@it.uu.se>
Date: Sat, 23 Sep 2006 00:15:41 +0200 (MEST)

> I rechecked with 2.6.18 final, and the behaviour is as I described:
> gcc-4.1.1 causes the alignment exceptions, while gcc-3.4.6 does not.
> I didn't get any PCI errors now, but I'm sure I did get them in the
> 2.6.17 or early 2.6.18-rc kernels.
> 
> Here's a dmesg diff to show what happens, between the gcc-4.1.1
> and gcc-3.4.6 compiled kernels. The first part is from kernel
> bootup + user-space modprobe of the USB EHCI etc modules:

Thanks for taking a look into this, I'll dig deeper when
I get a chance.

Re: [sparc64] 2.6.18 unaligned acccess in ehci_hub_control

[David Miller]

From: Mikael Pettersson <mikpe@it.uu.se>
Date: Sat, 23 Sep 2006 00:15:41 +0200 (MEST)

[ Greg, USB bug, synopsis: rh_call_control() needs to declare the
  'tbuf' local variable with correct alignment or else implementations
  of ->hub_control() get unaligned traps because they assume the
  passed-in buffer is at least 4-byte aligned which is not necessarily
  true if tbuf is simply declared as 'u8' as it is now. ]

> On Thu, 21 Sep 2006 13:51:21 -0700 (PDT), David Miller wrote:
> >> I don't think it's harmless. My Ultra5 has an add-on PCI USB controller
> >> card (Belkin). A 2.6.18-rc kernel compiled with gcc-4.1.1 will throw a few
> >> unaligned accesses when I initialise USB by inserting a USB memory stick.
> >> Removing the memory stick then results in PCI errors and other breakage.
> >> 
> >> The same kernel compiled with gcc-3.4.6 has no problems at all, so I've
> >> been assuming it's a gcc-4 issue and not a kernel issue.
> >
> >Compiled with gcc-4.0.x I get the same ehci_hub_control unaligned
> >accesses, and putting the correct {get,put}_unaligned() in that
> >function makes them go away.
> >
> >It's a pure mystery if gcc-3.4.x somehow avoids those, as by the
> >way the code is written those unaligned accesses are to be expected.
> 
> I rechecked with 2.6.18 final, and the behaviour is as I described:
> gcc-4.1.1 causes the alignment exceptions, while gcc-3.4.6 does not.
> I didn't get any PCI errors now, but I'm sure I did get them in the
> 2.6.17 or early 2.6.18-rc kernels.
> 
> Here's a dmesg diff to show what happens, between the gcc-4.1.1
> and gcc-3.4.6 compiled kernels. The first part is from kernel
> bootup + user-space modprobe of the USB EHCI etc modules:

I've determined that it's actually not a compiler bug or problem,
but rather the code in the call chain is buggy.

Gcc-4.x is just optimizing the packing of the on-stack variables more
aggressively, and it is doing so in a legitimate way.

The next to last argument to the ->hub_control() method is a buffer,
and this comes on-stack from rh_call_control().  This 'tbuf'
variable is declared like this:

	u8		tbuf [sizeof (struct usb_hub_descriptor)];

which only guarentees byte alignment.  I've verified in the assembly
on sparc64 that with gcc-4.x 'tbuf' is placed at an odd-byte offet on
the local stack.

Yet the implementations of ->hub_control() derefernce this area as if
it were at least 4-byte aligned, for example in this code snippet in
ehci_hub_control(), which is what is being triggered on sparc64, we
have:

		// we "know" this alignment is good, caller used kmalloc()...
		*((__le32 *) buf) = cpu_to_le32 (status);

That comment is also extremely bogus :-)

My original idea to fix this was to turn "tbuf" into a union comprised
of the byte array and a "struct usb_hub_descriptor" in order to get
the necessary alignment.  That doesn't work because usb_hub_descriptor
is declared as "packed".  So I suggest the following patch to fix this
bug.

Greg, please apply, thanks.

[USB]: Fix alignment of buffer passed down to ->hub_control()

Implementations assume the buffer is at least 4 byte aligned.

Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index fb4d058..7766d7b 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -344,7 +344,8 @@ static int rh_call_control (struct usb_h
 	struct usb_ctrlrequest *cmd;
  	u16		typeReq, wValue, wIndex, wLength;
 	u8		*ubuf = urb->transfer_buffer;
-	u8		tbuf [sizeof (struct usb_hub_descriptor)];
+	u8		tbuf [sizeof (struct usb_hub_descriptor)]
+		__attribute__((aligned(4)));
 	const u8	*bufp = tbuf;
 	int		len = 0;
 	int		patch_wakeup = 0;

Re: [sparc64] 2.6.18 unaligned acccess in ehci_hub_control

[Mikael Pettersson]

On Sat, 23 Sep 2006 17:05:31 -0700 (PDT), David Miller wrote:
>[USB]: Fix alignment of buffer passed down to ->hub_control()
>
>Implementations assume the buffer is at least 4 byte aligned.
>
>Signed-off-by: David S. Miller <davem@davemloft.net>
>
>diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
>index fb4d058..7766d7b 100644
>--- a/drivers/usb/core/hcd.c
>+++ b/drivers/usb/core/hcd.c
>@@ -344,7 +344,8 @@ static int rh_call_control (struct usb_h
> 	struct usb_ctrlrequest *cmd;
>  	u16		typeReq, wValue, wIndex, wLength;
> 	u8		*ubuf = urb->transfer_buffer;
>-	u8		tbuf [sizeof (struct usb_hub_descriptor)];
>+	u8		tbuf [sizeof (struct usb_hub_descriptor)]
>+		__attribute__((aligned(4)));
> 	const u8	*bufp = tbuf;
> 	int		len = 0;
> 	int		patch_wakeup = 0;
>

Thanks, this eliminated the USB alignment traps in my
gcc-4.1.1 compiled sparc64 kernel.

/Mikael