Hi all,
I am running the ARM Trusted Firmware on my HiKey board. I found that BL1 hangs if I build it with the version of GCC that comes with my Ubuntu 14.10 distribution [1] or with another 4.9 build from Linaro [2]. However, it works as expected if I use the 4.8 Linaro build [3] as recommended on the HiKey UEFI wiki [4].
Basically I found two separate issues, and I'm not sure if they are bugs in GCC or HiKey ATF. Here is the story...
With GCC 4.9 [1], the boot hangs, LED#2 blinks and "00000000f20003e8" is printed on UART0 about every second. Let's call this bug #1. The hang occurs in hi6220_pll_init() [5], execution never gets passed this line: mmio_write_32(0x0, 0xa5a55a5a);
So I checked the objdump outputs (bl1.dump).
- Working compiler [3] gives: f98041f0: d2800000 mov x0, #0x0 // #0 f98041f4: 528b4b41 mov w1, #0x5a5a // #23130 f98041f8: 72b4b4a1 movk w1, #0xa5a5, lsl #16 f98041fc: b9000001 str w1, [x0]
- Bad compiler [1] produces: f9804184: d2800000 mov x0, #0x0 // #0 f9804188: b900001f str wzr, [x0] f980418c: d4207d00 brk #0x3e8
What?! Is there some kind of smart detection in the compiler assuming that one shouldn't write to address zero?
If I change address to 0x4, the code goes past this location but later hangs with the same LED status as above (b0100) and code "0000000096000021" on the console. This is bug #2.
I tracked it down to the initialization of some structures on the stack when entering usb_handle_control_request() [6]. Looks like an alignment issue, since removing the packed attribute on struct usb_endpoint_descriptor [7] fixes the bug.
So... What kind of bugs do you guys think we have here, and who should I report them to?
[1] aarch64-linux-gnu-gcc (Ubuntu/Linaro 4.9.1-16ubuntu6) 4.9.1 [2] aarch64-linux-gnu-gcc (Linaro GCC 2014.11) 4.9.3 20141031 (prerelease) [3] aarch64-linux-gnu-gcc (crosstool-NG linaro-1.13.1-4.8-2014.04 - Linaro GCC 4.8-2014.04) 4.8.3 20140401 (prerelease) [4] https://github.com/96boards/documentation/wiki/UEFI [5] https://github.com/96boards/arm-trusted-firmware/blob/bbd623798cb775c4c0445c... [6] https://github.com/96boards/arm-trusted-firmware/blob/bbd623798cb775c4c0445c... [7] https://github.com/96boards/arm-trusted-firmware/blob/bbd623798cb775c4c0445c...
Thanks,
On 19 March 2015 at 19:18, Jerome Forissier jerome.forissier@linaro.org wrote:
Hi all,
I am running the ARM Trusted Firmware on my HiKey board. I found that BL1 hangs if I build it with the version of GCC that comes with my Ubuntu 14.10 distribution [1] or with another 4.9 build from Linaro [2]. However, it works as expected if I use the 4.8 Linaro build [3] as recommended on the HiKey UEFI wiki [4].
Basically I found two separate issues, and I'm not sure if they are bugs in GCC or HiKey ATF. Here is the story...
With GCC 4.9 [1], the boot hangs, LED#2 blinks and "00000000f20003e8" is printed on UART0 about every second.
Ah! So that's what I hit when trying to test https://github.com/96boards/arm-trusted-firmware/pull/2
On Thu, Mar 19, 2015 at 06:57:46PM +0000, Koen Kooi wrote:
On 19 March 2015 at 19:18, Jerome Forissier jerome.forissier@linaro.org wrote:
Hi all,
I am running the ARM Trusted Firmware on my HiKey board. I found that BL1 hangs if I build it with the version of GCC that comes with my Ubuntu 14.10 distribution [1] or with another 4.9 build from Linaro [2]. However, it works as expected if I use the 4.8 Linaro build [3] as recommended on the HiKey UEFI wiki [4].
Basically I found two separate issues, and I'm not sure if they are bugs in GCC or HiKey ATF. Here is the story...
With GCC 4.9 [1], the boot hangs, LED#2 blinks and "00000000f20003e8" is printed on UART0 about every second.
Ah! So that's what I hit when trying to test https://github.com/96boards/arm-trusted-firmware/pull/2
Before confirmed w/t Haojian and Fathi, ATF now only can be built w/t GCC4.8. Do u think gcc options -O0 ang -fpic will be helpful?
--
Koen Kooi
Builds and Baselines | Release Manager Linaro.org | Open source software for ARM SoCs
Let's call this bug #1. The hang occurs in hi6220_pll_init() [5], execution never gets passed this line: mmio_write_32(0x0, 0xa5a55a5a);
So I checked the objdump outputs (bl1.dump).
- Working compiler [3] gives: f98041f0: d2800000 mov x0, #0x0 //
#0 f98041f4: 528b4b41 mov w1, #0x5a5a // #23130 f98041f8: 72b4b4a1 movk w1, #0xa5a5, lsl #16 f98041fc: b9000001 str w1, [x0]
- Bad compiler [1] produces: f9804184: d2800000 mov x0, #0x0 //
#0 f9804188: b900001f str wzr, [x0] f980418c: d4207d00 brk #0x3e8
What?! Is there some kind of smart detection in the compiler assuming that one shouldn't write to address zero?
If I change address to 0x4, the code goes past this location but later hangs with the same LED status as above (b0100) and code "0000000096000021" on the console. This is bug #2.
I tracked it down to the initialization of some structures on the stack when entering usb_handle_control_request() [6]. Looks like an alignment issue, since removing the packed attribute on struct usb_endpoint_descriptor [7] fixes the bug.
So... What kind of bugs do you guys think we have here, and who should I report them to?
[1] aarch64-linux-gnu-gcc (Ubuntu/Linaro 4.9.1-16ubuntu6) 4.9.1 [2] aarch64-linux-gnu-gcc (Linaro GCC 2014.11) 4.9.3 20141031 (prerelease) [3] aarch64-linux-gnu-gcc (crosstool-NG linaro-1.13.1-4.8-2014.04 - Linaro GCC 4.8-2014.04) 4.8.3 20140401 (prerelease) [4] https://github.com/96boards/documentation/wiki/UEFI [5] https://github.com/96boards/arm-trusted-firmware/blob/bbd623798cb775c4c0445c... [6] https://github.com/96boards/arm-trusted-firmware/blob/bbd623798cb775c4c0445c... [7] https://github.com/96boards/arm-trusted-firmware/blob/bbd623798cb775c4c0445c...
Thanks,
Jerome
Hi,
On 19 March 2015 at 20:18, Jerome Forissier jerome.forissier@linaro.org wrote:
Hi all,
I am running the ARM Trusted Firmware on my HiKey board. I found that BL1 hangs if I build it with the version of GCC that comes with my Ubuntu 14.10 distribution [1] or with another 4.9 build from Linaro [2]. However, it works as expected if I use the 4.8 Linaro build [3] as recommended on the HiKey UEFI wiki [4].
it isn't a recommendation. Using GCC 4.8 is done on purpose. See the line "export AARCH64_TOOLCHAIN=GCC48".
Basically I found two separate issues, and I'm not sure if they are bugs in GCC or HiKey ATF. Here is the story...
it's a known issue. EDKII should be built with GCC 4.8. It's using some compilers profile and GCC 4.9 isn't officially supported at the moment afaik.
With GCC 4.9 [1], the boot hangs, LED#2 blinks and "00000000f20003e8" is printed on UART0 about every second. Let's call this bug #1. The hang occurs in hi6220_pll_init() [5], execution never gets passed this line: mmio_write_32(0x0, 0xa5a55a5a);
So I checked the objdump outputs (bl1.dump).
- Working compiler [3] gives: f98041f0: d2800000 mov x0, #0x0 //
#0 f98041f4: 528b4b41 mov w1, #0x5a5a // #23130 f98041f8: 72b4b4a1 movk w1, #0xa5a5, lsl #16 f98041fc: b9000001 str w1, [x0]
- Bad compiler [1] produces: f9804184: d2800000 mov x0, #0x0 //
#0 f9804188: b900001f str wzr, [x0] f980418c: d4207d00 brk #0x3e8
What?! Is there some kind of smart detection in the compiler assuming that one shouldn't write to address zero?
If I change address to 0x4, the code goes past this location but later hangs with the same LED status as above (b0100) and code "0000000096000021" on the console. This is bug #2.
I tracked it down to the initialization of some structures on the stack when entering usb_handle_control_request() [6]. Looks like an alignment issue, since removing the packed attribute on struct usb_endpoint_descriptor [7] fixes the bug.
So... What kind of bugs do you guys think we have here, and who should I report them to?
Better ask Saul^Leif for the full story and implication.
[1] aarch64-linux-gnu-gcc (Ubuntu/Linaro 4.9.1-16ubuntu6) 4.9.1 [2] aarch64-linux-gnu-gcc (Linaro GCC 2014.11) 4.9.3 20141031 (prerelease) [3] aarch64-linux-gnu-gcc (crosstool-NG linaro-1.13.1-4.8-2014.04 - Linaro GCC 4.8-2014.04) 4.8.3 20140401 (prerelease) [4] https://github.com/96boards/documentation/wiki/UEFI [5] https://github.com/96boards/arm-trusted-firmware/blob/bbd623798cb775c4c0445c... [6] https://github.com/96boards/arm-trusted-firmware/blob/bbd623798cb775c4c0445c... [7] https://github.com/96boards/arm-trusted-firmware/blob/bbd623798cb775c4c0445c...
Thanks,
Jerome
On Fri, Mar 20, 2015 at 09:22:19AM +0200, Fathi Boudra wrote:
I am running the ARM Trusted Firmware on my HiKey board. I found that BL1 hangs if I build it with the version of GCC that comes with my Ubuntu 14.10 distribution [1] or with another 4.9 build from Linaro [2]. However, it works as expected if I use the 4.8 Linaro build [3] as recommended on the HiKey UEFI wiki [4].
*uses foul language*
it isn't a recommendation. Using GCC 4.8 is done on purpose. See the line "export AARCH64_TOOLCHAIN=GCC48".
What on earth for?
Basically I found two separate issues, and I'm not sure if they are bugs in GCC or HiKey ATF. Here is the story...
it's a known issue. EDKII should be built with GCC 4.8. It's using some compilers profile and GCC 4.9 isn't officially supported at the moment afaik.
Who (incorrectly) told you this? Can I just make a desperate call-out that any claims like this are checked with the linaro-uefi@lists.linaro.org mailing list before taken as gospel and put into documentation? Issues found when switching to later toolchains need to be fixed, not hidden.
Yes, we did end up with some fun from gcc4.9 generating some relocations gcc4.8 didn't. That was a 5 minute quick-fix (and a slightly longer proper one) and has been upstream since November.
EDK2 _does_ provide per-gcc-version configuration flags (making it a whole lot easier to deal with added/deleted ones), but that's just gravy. Official support for a GCC49 profile went upstream in July last year, and I was happily building with gcc4.9 on the GCC48 profile before then.
Better ask Saul^Leif for the full story and implication.
Well, this question was actually about ARM Trusted Firmware, but as per the other replies on this thread, I should clearly add -fno-delete-null-pointer-checks to at least GCC49 profile flags for EDK2.
/ Leif
On Fri, Mar 20, 2015 at 07:22:27AM +0000, Fathi Boudra wrote:
On 19 March 2015 at 20:18, Jerome Forissier jerome.forissier@linaro.org wrote:
[...]
If I change address to 0x4, the code goes past this location but later hangs with the same LED status as above (b0100) and code "0000000096000021" on the console. This is bug #2.
I tracked it down to the initialization of some structures on the stack when entering usb_handle_control_request() [6]. Looks like an alignment issue, since removing the packed attribute on struct usb_endpoint_descriptor [7] fixes the bug.
W/t Peter and Jorge's reminding, below are my analysis for th error for "0000000096000021":
- From ARMv8-ARM D7.2.27, we can know the exception class (EC) is b'100101, that means this error is caused by below errors:
"Data Abort that caused entry from a current exception level (AArch64). Used for data access generated MMU faults, alignment faults other than those caused by the Stack Pointer misalignment, and synchronous external aborts, including synchronous parity errors. Not used for debug related exceptions."
So this error is highly related w/t alignment issue.
- After print out the ELR_EL3, then can get the PC value when the exception happened, ELR_EL3 = 0xf980_5be4, so from objdump file get below code section; in this case X2=0x00000000f980b018, but in the instruction the offset = 0x3; so finally it will trigger unalignent issue...
--->8--- 5434 f9805bdc: b9004ba4 str w4, [x29,#72] 5435 f9805be0: 52800400 mov w0, #0x20 // #32 5436 f9805be4: b8403044 ldr w4, [x2,#3] --------
And use "addr2line" can get to know the error is happened to access the usb structure at line 729:
694 void usb_handle_control_request(setup_packet* req) 695 { 696 const void* addr = NULL; 697 int size = -1; 698 int maxpacket; 699 unsigned int data; 700 struct usb_endpoint_descriptor epx; 701 struct usb_config_bundle const_bundle = { 702 .config = { 703 .bLength = sizeof(struct usb_config_descriptor), 704 .bDescriptorType = USB_DT_CONFIG, 705 .wTotalLength = sizeof(struct usb_config_descriptor) + 706 sizeof(struct usb_interface_descriptor) + 707 sizeof(struct usb_endpoint_descriptor) * 708 USB_NUM_ENDPOINTS, 709 .bNumInterfaces = 1, 710 .bConfigurationValue = 1, 711 .iConfiguration = 0, 712 .bmAttributes = USB_CONFIG_ATT_ONE, 713 .bMaxPower = 0x80 714 }, 715 .interface = { 716 .bLength = sizeof(struct usb_interface_descriptor), 717 .bDescriptorType = USB_DT_INTERFACE, 718 .bInterfaceNumber = 0, 719 .bAlternateSetting = 0, 720 .bNumEndpoints = USB_NUM_ENDPOINTS, 721 .bInterfaceClass = USB_CLASS_VENDOR_SPEC, 722 .bInterfaceSubClass = 0x42, 723 .bInterfaceProtocol = 0x03, 724 .iInterface = 0 725 } 726 }; 727 728 /* avoid to hang on accessing unaligned memory */ 729 struct usb_endpoint_descriptor const_ep1 = { 730 .bLength = sizeof(struct usb_endpoint_descriptor), 731 .bDescriptorType = USB_DT_ENDPOINT, 732 .bEndpointAddress = 0x81, 733 .bmAttributes = USB_ENDPOINT_XFER_BULK, 734 .wMaxPacketSize = 0, 735 .bInterval = 0 736 };
- Quick try: if move upper local variables (in stack?) to global varaibles (in data section?), then the issue also will dismiss.
Will here have some compiler options will impact this issue?
Personally i think Koen's patch just remove "packed" so here will not have unalignment issue, but it will potentially introduce usb protocol issue.
Thanks, Leo Yan
[1] aarch64-linux-gnu-gcc (Ubuntu/Linaro 4.9.1-16ubuntu6) 4.9.1 [2] aarch64-linux-gnu-gcc (Linaro GCC 2014.11) 4.9.3 20141031 (prerelease) [3] aarch64-linux-gnu-gcc (crosstool-NG linaro-1.13.1-4.8-2014.04 - Linaro GCC 4.8-2014.04) 4.8.3 20140401 (prerelease) [4] https://github.com/96boards/documentation/wiki/UEFI [5] https://github.com/96boards/arm-trusted-firmware/blob/bbd623798cb775c4c0445c... [6] https://github.com/96boards/arm-trusted-firmware/blob/bbd623798cb775c4c0445c... [7] https://github.com/96boards/arm-trusted-firmware/blob/bbd623798cb775c4c0445c...
On Mon, Mar 23, 2015 at 02:25:15PM +0000, Leo Yan wrote:
On Fri, Mar 20, 2015 at 07:22:27AM +0000, Fathi Boudra wrote:
On 19 March 2015 at 20:18, Jerome Forissier jerome.forissier@linaro.org wrote:
[...]
If I change address to 0x4, the code goes past this location but later hangs with the same LED status as above (b0100) and code "0000000096000021" on the console. This is bug #2.
I tracked it down to the initialization of some structures on the stack when entering usb_handle_control_request() [6]. Looks like an alignment issue, since removing the packed attribute on struct usb_endpoint_descriptor [7] fixes the bug.
W/t Peter and Jorge's reminding, below are my analysis for th error for "0000000096000021":
From ARMv8-ARM D7.2.27, we can know the exception class (EC) is b'100101, that means this error is caused by below errors:
"Data Abort that caused entry from a current exception level (AArch64). Used for data access generated MMU faults, alignment faults other than those caused by the Stack Pointer misalignment, and synchronous external aborts, including synchronous parity errors. Not used for debug related exceptions."
So this error is highly related w/t alignment issue.
After print out the ELR_EL3, then can get the PC value when the exception happened, ELR_EL3 = 0xf980_5be4, so from objdump file get below code section; in this case X2=0x00000000f980b018, but in the instruction the offset = 0x3; so finally it will trigger unalignent issue...
--->8--- 5434 f9805bdc: b9004ba4 str w4, [x29,#72] 5435 f9805be0: 52800400 mov w0, #0x20 // #32 5436 f9805be4: b8403044 ldr w4, [x2,#3]
And use "addr2line" can get to know the error is happened to access the usb structure at line 729:
694 void usb_handle_control_request(setup_packet* req) 695 { 696 const void* addr = NULL; 697 int size = -1; 698 int maxpacket; 699 unsigned int data; 700 struct usb_endpoint_descriptor epx; 701 struct usb_config_bundle const_bundle = { 702 .config = { 703 .bLength = sizeof(struct usb_config_descriptor), 704 .bDescriptorType = USB_DT_CONFIG, 705 .wTotalLength = sizeof(struct usb_config_descriptor) + 706 sizeof(struct usb_interface_descriptor) + 707 sizeof(struct usb_endpoint_descriptor) * 708 USB_NUM_ENDPOINTS, 709 .bNumInterfaces = 1, 710 .bConfigurationValue = 1, 711 .iConfiguration = 0, 712 .bmAttributes = USB_CONFIG_ATT_ONE, 713 .bMaxPower = 0x80 714 }, 715 .interface = { 716 .bLength = sizeof(struct usb_interface_descriptor), 717 .bDescriptorType = USB_DT_INTERFACE, 718 .bInterfaceNumber = 0, 719 .bAlternateSetting = 0, 720 .bNumEndpoints = USB_NUM_ENDPOINTS, 721 .bInterfaceClass = USB_CLASS_VENDOR_SPEC, 722 .bInterfaceSubClass = 0x42, 723 .bInterfaceProtocol = 0x03, 724 .iInterface = 0 725 } 726 }; 727 728 /* avoid to hang on accessing unaligned memory */ 729 struct usb_endpoint_descriptor const_ep1 = { 730 .bLength = sizeof(struct usb_endpoint_descriptor), 731 .bDescriptorType = USB_DT_ENDPOINT, 732 .bEndpointAddress = 0x81, 733 .bmAttributes = USB_ENDPOINT_XFER_BULK, 734 .wMaxPacketSize = 0, 735 .bInterval = 0 736 };
- Quick try: if move upper local variables (in stack?) to global varaibles (in data section?), then the issue also will dismiss.
Will here have some compiler options will impact this issue?
Read info at: https://github.com/96boards/arm-trusted-firmware/pull/4 With Jerome's suggestion, use -mstrict-align can fix this issue.
i modify the Makefile like below, do u think i need commit one patch for this?
-------------
diff --git a/Makefile b/Makefile index aa5880e..085fb6e 100644 --- a/Makefile +++ b/Makefile @@ -258,9 +258,10 @@ ASFLAGS += -nostdinc -ffreestanding -Wa,--fatal-warnings \ ${DEFINES} ${INCLUDES} CFLAGS += -nostdinc -ffreestanding -Wall \ -Werror -Wmissing-include-dirs \ - -mgeneral-regs-only -std=c99 -c -Os \ - ${DEFINES} ${INCLUDES} -CFLAGS += -ffunction-sections -fdata-sections + -mgeneral-regs-only -mstrict-align \ + -std=c99 -c -Os ${DEFINES} ${INCLUDES} +CFLAGS += -ffunction-sections -fdata-sections \ + -fno-delete-null-pointer-checks
LDFLAGS += --fatal-warnings -O1 LDFLAGS += --gc-sections
[1] aarch64-linux-gnu-gcc (Ubuntu/Linaro 4.9.1-16ubuntu6) 4.9.1 [2] aarch64-linux-gnu-gcc (Linaro GCC 2014.11) 4.9.3 20141031 (prerelease) [3] aarch64-linux-gnu-gcc (crosstool-NG linaro-1.13.1-4.8-2014.04 - Linaro GCC 4.8-2014.04) 4.8.3 20140401 (prerelease) [4] https://github.com/96boards/documentation/wiki/UEFI [5] https://github.com/96boards/arm-trusted-firmware/blob/bbd623798cb775c4c0445c... [6] https://github.com/96boards/arm-trusted-firmware/blob/bbd623798cb775c4c0445c... [7] https://github.com/96boards/arm-trusted-firmware/blob/bbd623798cb775c4c0445c...
Dev mailing list Dev@lists.96boards.org https://lists.96boards.org/mailman/listinfo/dev
On 03/23/2015 10:23 PM, Leo Yan wrote:
On Mon, Mar 23, 2015 at 02:25:15PM +0000, Leo Yan wrote:
On Fri, Mar 20, 2015 at 07:22:27AM +0000, Fathi Boudra wrote:
On 19 March 2015 at 20:18, Jerome Forissier jerome.forissier@linaro.org wrote:
[...]
If I change address to 0x4, the code goes past this location but later hangs with the same LED status as above (b0100) and code "0000000096000021" on the console. This is bug #2.
I tracked it down to the initialization of some structures on the stack when entering usb_handle_control_request() [6]. Looks like an alignment issue, since removing the packed attribute on struct usb_endpoint_descriptor [7] fixes the bug.
W/t Peter and Jorge's reminding, below are my analysis for th error for "0000000096000021":
From ARMv8-ARM D7.2.27, we can know the exception class (EC) is b'100101, that means this error is caused by below errors:
"Data Abort that caused entry from a current exception level (AArch64). Used for data access generated MMU faults, alignment faults other than those caused by the Stack Pointer misalignment, and synchronous external aborts, including synchronous parity errors. Not used for debug related exceptions."
So this error is highly related w/t alignment issue.
After print out the ELR_EL3, then can get the PC value when the exception happened, ELR_EL3 = 0xf980_5be4, so from objdump file get below code section; in this case X2=0x00000000f980b018, but in the instruction the offset = 0x3; so finally it will trigger unalignent issue...
--->8--- 5434 f9805bdc: b9004ba4 str w4, [x29,#72] 5435 f9805be0: 52800400 mov w0, #0x20 // #32 5436 f9805be4: b8403044 ldr w4, [x2,#3]
And use "addr2line" can get to know the error is happened to access the usb structure at line 729:
694 void usb_handle_control_request(setup_packet* req) 695 { 696 const void* addr = NULL; 697 int size = -1; 698 int maxpacket; 699 unsigned int data; 700 struct usb_endpoint_descriptor epx; 701 struct usb_config_bundle const_bundle = { 702 .config = { 703 .bLength = sizeof(struct usb_config_descriptor), 704 .bDescriptorType = USB_DT_CONFIG, 705 .wTotalLength = sizeof(struct usb_config_descriptor) + 706 sizeof(struct usb_interface_descriptor) + 707 sizeof(struct usb_endpoint_descriptor) * 708 USB_NUM_ENDPOINTS, 709 .bNumInterfaces = 1, 710 .bConfigurationValue = 1, 711 .iConfiguration = 0, 712 .bmAttributes = USB_CONFIG_ATT_ONE, 713 .bMaxPower = 0x80 714 }, 715 .interface = { 716 .bLength = sizeof(struct usb_interface_descriptor), 717 .bDescriptorType = USB_DT_INTERFACE, 718 .bInterfaceNumber = 0, 719 .bAlternateSetting = 0, 720 .bNumEndpoints = USB_NUM_ENDPOINTS, 721 .bInterfaceClass = USB_CLASS_VENDOR_SPEC, 722 .bInterfaceSubClass = 0x42, 723 .bInterfaceProtocol = 0x03, 724 .iInterface = 0 725 } 726 }; 727 728 /* avoid to hang on accessing unaligned memory */ 729 struct usb_endpoint_descriptor const_ep1 = { 730 .bLength = sizeof(struct usb_endpoint_descriptor), 731 .bDescriptorType = USB_DT_ENDPOINT, 732 .bEndpointAddress = 0x81, 733 .bmAttributes = USB_ENDPOINT_XFER_BULK, 734 .wMaxPacketSize = 0, 735 .bInterval = 0 736 };
the linux kernel recommends the following when initializing endpoint descriptors so I would be reluctant to remove the packed attribute.
* Note all descriptors are declared '__attribute__((packed))' so that: * * [a] they never get padded, either internally (USB spec writers * probably handled that) or externally; * * [b] so that accessing bigger-than-a-bytes fields will never * generate bus errors on any platform, even when the location of * its descriptor inside a bundle isn't "naturally aligned", and * * [c] for consistency, removing all doubt even when it appears to * someone that the two other points are non-issues for that * particular descriptor type.
at the same time
/* USB_DT_ENDPOINT: Endpoint descriptor */ struct usb_endpoint_descriptor { __u8 bLength; __u8 bDescriptorType;
__u8 bEndpointAddress; __u8 bmAttributes; __le16 wMaxPacketSize; __u8 bInterval;
/* NOTE: these two are _only_ in audio endpoints. */ * /* use USB_DT_ENDPOINT*_SIZE in bLength, not sizeof. */* __u8 bRefresh; __u8 bSynchAddress; } __attribute__ ((packed));
#define USB_DT_ENDPOINT_SIZE 7 #define USB_DT_ENDPOINT_AUDIO_SIZE 9 /* Audio extension */
On Tue, Mar 24, 2015 at 9:48 AM, Jorge Ramirez-Ortiz jorge.ramirez-ortiz@linaro.org wrote:
On 03/23/2015 10:23 PM, Leo Yan wrote:
On Mon, Mar 23, 2015 at 02:25:15PM +0000, Leo Yan wrote:
On Fri, Mar 20, 2015 at 07:22:27AM +0000, Fathi Boudra wrote:
On 19 March 2015 at 20:18, Jerome Forissier jerome.forissier@linaro.org wrote:
[...]
If I change address to 0x4, the code goes past this location but later hangs with the same LED status as above (b0100) and code "0000000096000021" on the console. This is bug #2.
I tracked it down to the initialization of some structures on the stack when entering usb_handle_control_request() [6]. Looks like an alignment issue, since removing the packed attribute on struct usb_endpoint_descriptor [7] fixes the bug.
W/t Peter and Jorge's reminding, below are my analysis for th error for "0000000096000021":
From ARMv8-ARM D7.2.27, we can know the exception class (EC) is b'100101, that means this error is caused by below errors:
"Data Abort that caused entry from a current exception level (AArch64). Used for data access generated MMU faults, alignment faults other than those caused by the Stack Pointer misalignment, and synchronous external aborts, including synchronous parity errors. Not used for debug related exceptions."
So this error is highly related w/t alignment issue.
After print out the ELR_EL3, then can get the PC value when the exception happened, ELR_EL3 = 0xf980_5be4, so from objdump file get below code section; in this case X2=0x00000000f980b018, but in the instruction the offset = 0x3; so finally it will trigger unalignent issue...
--->8--- 5434 f9805bdc: b9004ba4 str w4, [x29,#72] 5435 f9805be0: 52800400 mov w0, #0x20 // #32 5436 f9805be4: b8403044 ldr w4, [x2,#3]
And use "addr2line" can get to know the error is happened to access the usb structure at line 729:
694 void usb_handle_control_request(setup_packet* req) 695 { 696 const void* addr = NULL; 697 int size = -1; 698 int maxpacket; 699 unsigned int data; 700 struct usb_endpoint_descriptor epx; 701 struct usb_config_bundle const_bundle = { 702 .config = { 703 .bLength = sizeof(struct usb_config_descriptor), 704 .bDescriptorType = USB_DT_CONFIG, 705 .wTotalLength = sizeof(struct usb_config_descriptor) + 706 sizeof(struct usb_interface_descriptor) + 707 sizeof(struct usb_endpoint_descriptor) * 708 USB_NUM_ENDPOINTS, 709 .bNumInterfaces = 1, 710 .bConfigurationValue = 1, 711 .iConfiguration = 0, 712 .bmAttributes = USB_CONFIG_ATT_ONE, 713 .bMaxPower = 0x80 714 }, 715 .interface = { 716 .bLength = sizeof(struct usb_interface_descriptor), 717 .bDescriptorType = USB_DT_INTERFACE, 718 .bInterfaceNumber = 0, 719 .bAlternateSetting = 0, 720 .bNumEndpoints = USB_NUM_ENDPOINTS, 721 .bInterfaceClass = USB_CLASS_VENDOR_SPEC, 722 .bInterfaceSubClass = 0x42, 723 .bInterfaceProtocol = 0x03, 724 .iInterface = 0 725 } 726 }; 727 728 /* avoid to hang on accessing unaligned memory */ 729 struct usb_endpoint_descriptor const_ep1 = { 730 .bLength = sizeof(struct usb_endpoint_descriptor), 731 .bDescriptorType = USB_DT_ENDPOINT, 732 .bEndpointAddress = 0x81, 733 .bmAttributes = USB_ENDPOINT_XFER_BULK, 734 .wMaxPacketSize = 0, 735 .bInterval = 0 736 };
the linux kernel recommends the following when initializing endpoint descriptors so I would be reluctant to remove the packed attribute.
- Note all descriptors are declared '__attribute__((packed))' so that:
- [a] they never get padded, either internally (USB spec writers
probably handled that) or externally;- [b] so that accessing bigger-than-a-bytes fields will never
generate bus errors on any platform, even when the location ofits descriptor inside a bundle isn't "naturally aligned", and- [c] for consistency, removing all doubt even when it appears to
someone that the two other points are non-issues for thatparticular descriptor type.at the same time
/* USB_DT_ENDPOINT: Endpoint descriptor */ struct usb_endpoint_descriptor { __u8 bLength; __u8 bDescriptorType;
__u8 bEndpointAddress; __u8 bmAttributes; __le16 wMaxPacketSize; __u8 bInterval; /* NOTE: these two are _only_ in audio endpoints. */
- /* use USB_DT_ENDPOINT*_SIZE in bLength, not sizeof. */* __u8 bRefresh; __u8 bSynchAddress;
} __attribute__ ((packed));
#define USB_DT_ENDPOINT_SIZE 7 #define USB_DT_ENDPOINT_AUDIO_SIZE 9 /* Audio extension */
Dev mailing list Dev@lists.96boards.org https://lists.96boards.org/mailman/listinfo/dev
How come the audio bytes are not included in the ARM TF endpoint descriptor? See: https://github.com/96boards/arm-trusted-firmware/blob/bbd623798cb775c4c0445c...
Scott
On 23/03/15 14:25, Leo Yan wrote:
On Fri, Mar 20, 2015 at 07:22:27AM +0000, Fathi Boudra wrote:
On 19 March 2015 at 20:18, Jerome Forissier jerome.forissier@linaro.org wrote:
[...]
If I change address to 0x4, the code goes past this location but later hangs with the same LED status as above (b0100) and code "0000000096000021" on the console. This is bug #2.
I tracked it down to the initialization of some structures on the stack when entering usb_handle_control_request() [6]. Looks like an alignment issue, since removing the packed attribute on struct usb_endpoint_descriptor [7] fixes the bug.
W/t Peter and Jorge's reminding, below are my analysis for th error for "0000000096000021":
From ARMv8-ARM D7.2.27, we can know the exception class (EC) is b'100101, that means this error is caused by below errors:
"Data Abort that caused entry from a current exception level (AArch64). Used for data access generated MMU faults, alignment faults other than those caused by the Stack Pointer misalignment, and synchronous external aborts, including synchronous parity errors. Not used for debug related exceptions."
So this error is highly related w/t alignment issue.
After print out the ELR_EL3, then can get the PC value when the exception happened, ELR_EL3 = 0xf980_5be4, so from objdump file get below code section; in this case X2=0x00000000f980b018, but in the instruction the offset = 0x3; so finally it will trigger unalignent issue...
--->8--- 5434 f9805bdc: b9004ba4 str w4, [x29,#72] 5435 f9805be0: 52800400 mov w0, #0x20 // #32 5436 f9805be4: b8403044 ldr w4, [x2,#3]
And use "addr2line" can get to know the error is happened to access the usb structure at line 729:
694 void usb_handle_control_request(setup_packet* req) 695 { 696 const void* addr = NULL; 697 int size = -1; 698 int maxpacket; 699 unsigned int data; 700 struct usb_endpoint_descriptor epx; 701 struct usb_config_bundle const_bundle = { 702 .config = { 703 .bLength = sizeof(struct usb_config_descriptor), 704 .bDescriptorType = USB_DT_CONFIG, 705 .wTotalLength = sizeof(struct usb_config_descriptor) + 706 sizeof(struct usb_interface_descriptor) + 707 sizeof(struct usb_endpoint_descriptor) * 708 USB_NUM_ENDPOINTS, 709 .bNumInterfaces = 1, 710 .bConfigurationValue = 1, 711 .iConfiguration = 0, 712 .bmAttributes = USB_CONFIG_ATT_ONE, 713 .bMaxPower = 0x80 714 }, 715 .interface = { 716 .bLength = sizeof(struct usb_interface_descriptor), 717 .bDescriptorType = USB_DT_INTERFACE, 718 .bInterfaceNumber = 0, 719 .bAlternateSetting = 0, 720 .bNumEndpoints = USB_NUM_ENDPOINTS, 721 .bInterfaceClass = USB_CLASS_VENDOR_SPEC, 722 .bInterfaceSubClass = 0x42, 723 .bInterfaceProtocol = 0x03, 724 .iInterface = 0 725 } 726 }; 727 728 /* avoid to hang on accessing unaligned memory */ 729 struct usb_endpoint_descriptor const_ep1 = { 730 .bLength = sizeof(struct usb_endpoint_descriptor), 731 .bDescriptorType = USB_DT_ENDPOINT, 732 .bEndpointAddress = 0x81, 733 .bmAttributes = USB_ENDPOINT_XFER_BULK, 734 .wMaxPacketSize = 0, 735 .bInterval = 0 736 };
- Quick try: if move upper local variables (in stack?) to global varaibles (in data section?), then the issue also will dismiss.
Oops. I just ended up making pretty the same suggestion in the github issue tracker. Didn't mean to duplicate the conversation here.
Anyhow I would have expected adding a const qualifier to these structures to fix the issue (check that the compiled code memcpy's them directly from .rodata).
Daniel.
-
Daniel Thompson -
Fathi Boudra -
Jerome Forissier -
Jorge Ramirez-Ortiz -
Koen Kooi -
Leif Lindholm -
Leo Yan -
Scott Bambrough