On Sat, Jul 29, 2017 at 12:53:36PM +0300, Denis Pynkin wrote:
28.07.2017 00:26, Joe Hershberger wrote:
PXE boot is broken with GCC 7.1 due option '-fstore-merging' enabled by default for '-O2':
So, what I've been wondering, and others have poked me about (who can chime in if they like), how is the kernel not also tripping over this?
Great question.
Believe kernel do not work in single static buffer for the whole packet as u-boot do.
Problem is in ethernet header structure with size ETHER_HDR_SIZE=14, so IP and BOOTP headers are not aligned to 4 bytes.
Let's take bootp.c code as example:
755 bp = (struct bootp_hdr *)pkt; 756 bp->bp_op = OP_BOOTREQUEST; 757 bp->bp_htype = HWT_ETHER; 758 bp->bp_hlen = HWL_ETHER; 759 bp->bp_hops = 0;
Without '-fstore-merging' or with enabled 'packed' structures assembler code looks like: 6a: 77a3 strb r3, [r4, #30] 6c: f884 b01c strb.w fp, [r4, #28] 70: f884 b01d strb.w fp, [r4, #29] 74: 77e5 strb r5, [r4, #31]
but with option '-fstore-merging' all these instructions merged into single instruction: 62: 61e3 str r3, [r4, #28]
and store address is not aligned to 32b boundary here, so it results to 'data abort'.
I see some possible solutions here:
- add option '-fno-store-merging' -- works, I test it already
- use packed structures -- since there are some unsafe code in sources
- rewrite networking part to work with any protocol separately and
merge headers and data only on send.
Even simple reordering of header flags initialization helps here, but it looks like a bit of black magic in code.
PS forgot to mention in patch description -- '-Os' includes '-fstore-merging' option as well for gcc 7.1.
Thanks for explaining. My inclination is that we should have a packed change in for -rc1. Philipp, have you had a chance to edit doc/README.unaligned-memory-access.txt to your liking? Thanks!