Maxime,

> On 11 Jul 2017, at 18:59, Tom Rini <tr...@konsulko.com> wrote:
> 
> On Tue, Jul 11, 2017 at 06:54:55PM +0200, Maxime Ripard wrote:
>> Hi,
>> 
>> I recently got a gcc 7.1 based toolchain, and it seems like it
>> generates unaligned code, specifically in the net_set_ip_header
>> function in my case.
>> 
>> Whenever some packet is sent, this data abort is triggered:
>> 
>> => setenv ipaddr 10.42.0.1; ping 10.42.0.254
>> using musb-hdrc, OUT ep1out IN ep1in STATUS ep2in
>> MAC de:ad:be:ef:00:01
>> HOST MAC de:ad:be:af:00:00
>> RNDIS ready
>> musb-hdrc: peripheral reset irq lost!
>> high speed config #2: 2 mA, Ethernet Gadget, using RNDIS
>> USB RNDIS network up!
>> Using usb_ether device
>> data abort
>> pc : [<7ff9db10>]       lr : [<7ff9f00c>]
>> reloc pc : [<4a043b10>]         lr : [<4a04500c>]
>> sp : 7bf37cc8  ip : 00000000  fp : 7ff6236c
>> r10: 7ffed2b8  r9 : 7bf39ee8  r8 : 7ffed2b8
>> r7 : 00000001  r6 : 00000000  r5 : 0000002a  r4 : 7ffed30e
>> r3 : 14000045  r2 : 01002a0a  r1 : fe002a0a  r0 : 7ffed30e
>> Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
>> Resetting CPU ...
>> 
>> 
>> Running objdump on it gives us this:
>> 
>> 4a043b04 <net_set_ip_header>:
>> 
>>      /*
>>       *      Construct an IP header.
>>       */
>>      /* IP_HDR_SIZE / 4 (not including UDP) */
>>      ip->ip_hl_v  = 0x45;
>> 4a043b04:    e59f3074        ldr     r3, [pc, #116]  ; 4a043b80 
>> <net_set_ip_header+0x7c>
>> {
>> 4a043b08:    e92d4013        push    {r0, r1, r4, lr}
>> 4a043b0c:    e1a04000        mov     r4, r0
>>      ip->ip_hl_v  = 0x45;
>> 4a043b10:    e5803000        str     r3, [r0] <---- Abort
>>      ip->ip_tos   = 0;
>>      ip->ip_len   = htons(IP_HDR_SIZE);
>>      ip->ip_id    = htons(net_ip_id++);
>> 4a043b14:    e59f3068        ldr     r3, [pc, #104]  ; 4a043b84 
>> <net_set_ip_header+0x80>
>> 
>> It seems like r0 is indeed set to an unaligned address (0x7ffed30e)
>> for some reason.
>> 
>> Using a Linaro 6.3 toolchain works on the same commit with the same
>> config, so it really seems to be a compiler-related issue.
>> 
>> It generates this code:
>> 
>> 4a043ec4 <net_set_ip_header>:
>> 
>>      /*
>>       *      Construct an IP header.
>>       */
>>      /* IP_HDR_SIZE / 4 (not including UDP) */
>>      ip->ip_hl_v  = 0x45;
>> 4a043ec4:    e3a03045        mov     r3, #69 ; 0x45
>> {
>> 4a043ec8:    e92d4013        push    {r0, r1, r4, lr}
>> 4a043ecc:    e1a04000        mov     r4, r0
>>      ip->ip_hl_v  = 0x45;
>> 4a043ed0:    e5c03000        strb    r3, [r0]
>>      ip->ip_tos   = 0;
>>      ip->ip_len   = htons(IP_HDR_SIZE);
>> 4a043ed4:    e3a03b05        mov     r3, #5120       ; 0x1400
>>      ip->ip_tos   = 0;
>> 4a043ed8:    e3a00000        mov     r0, #0
>>      ip->ip_len   = htons(IP_HDR_SIZE);
>> 4a043edc:    e1c430b2        strh    r3, [r4, #2]
>>      ip->ip_id    = htons(net_ip_id++);
>> 4a043ee0:    e59f3064        ldr     r3, [pc, #100]  ; 4a043f4c 
>> <net_set_ip_header+0x88>
>> 
>> And it seems like it's using an strb instruction to avoid the
>> unaligned access.
>> 
>> As far as I know, we are passing --wno-unaligned-access, so the broken
>> situation should not arise, and yet it does, so I'm a bit confused,
>> and not really sure what to do from there.
> 
> Can you reduce the code into a testcase?  I think the first step is
> filing a bug with gcc and seeing where it goes from there as yes, we
> should be passing -mno-unaligned-access.

I don’t think that this is a GCC bug, as “-mno-unaligned-access” will
change the behaviour for packed data-structures only. Here’s from
the GCC docs:
> -munaligned-access
> -mno-unaligned-access
> Enables (or disables) reading and writing of 16- and 32- bit values from 
> addresses that are not 16- or 32- bit aligned. By default unaligned access is 
> disabled for all pre-ARMv6, all ARMv6-M and for ARMv8-M Baseline 
> architectures, and enabled for all other architectures. If unaligned access 
> is not enabled then words in packed data structures are accessed a byte at a 
> time.

The key word seems to be “in packed data structures”.
However, I don’t see an attribute “packed” for the 'struct ip_udp_hdr’
(in include/net.h).

Could you try to verify that the error reproduces with a packed variant
of the ‘struct ip_udp_hdr’?

Regards,
Phil.

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to