2006-09-20 17:21:28

by Will Simoneau

[permalink] [raw]
Subject: [sparc64] 2.6.18 unaligned acccess in ehci_hub_control

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.


Attachments:
(No filename) (1.02 kB)
(No filename) (189.00 B)
Download all attachments

2006-09-20 17:57:19

by David Miller

[permalink] [raw]
Subject: Re: [sparc64] 2.6.18 unaligned acccess in ehci_hub_control

From: Will Simoneau <[email protected]>
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.

2006-09-21 10:37:28

by Mikael Pettersson

[permalink] [raw]
Subject: Re: [sparc64] 2.6.18 unaligned acccess in ehci_hub_control

David Miller writes:
> From: Will Simoneau <[email protected]>
> 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

2006-09-21 20:51:10

by David Miller

[permalink] [raw]
Subject: Re: [sparc64] 2.6.18 unaligned acccess in ehci_hub_control

From: Mikael Pettersson <[email protected]>
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.

2006-09-22 22:15:55

by Mikael Pettersson

[permalink] [raw]
Subject: Re: [sparc64] 2.6.18 unaligned acccess in ehci_hub_control

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

2006-09-22 22:22:43

by David Miller

[permalink] [raw]
Subject: Re: [sparc64] 2.6.18 unaligned acccess in ehci_hub_control

From: Mikael Pettersson <[email protected]>
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.

2006-09-24 00:05:37

by David Miller

[permalink] [raw]
Subject: Re: [sparc64] 2.6.18 unaligned acccess in ehci_hub_control

From: Mikael Pettersson <[email protected]>
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 <[email protected]>

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;

2006-09-24 13:59:01

by Mikael Pettersson

[permalink] [raw]
Subject: Re: [sparc64] 2.6.18 unaligned acccess in ehci_hub_control

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 <[email protected]>
>
>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