| Message ID | 74973920d8722df3ce533979314564f331acf16e.1677687247.git.oleksii.kurochko@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v1] xen/arm: align *(.proc.info) in the linker script | expand |
Hi Oleksii, On 01/03/2023 16:14, Oleksii Kurochko wrote: > During testing of bug.h's macros generic implementation yocto-qemuarm > job crashed with data abort: The commit message is lacking some information. You are telling us that there is an error when building with your series, but this doesn't tell me why this is the correct fix. This is also why I asked to have the xen binary because I want to check whether this was a latent bug in Xen or your series effectively introduce a bug. Note that regardless what I just wrote this is a good idea to align __proc_info_start. I will try to have a closer look later and propose a commit message and/or any action for your other series. Cheers,
Hi Julien, On Wed, 2023-03-01 at 16:21 +0000, Julien Grall wrote: > Hi Oleksii, > > On 01/03/2023 16:14, Oleksii Kurochko wrote: > > During testing of bug.h's macros generic implementation yocto- > > qemuarm > > job crashed with data abort: > > The commit message is lacking some information. You are telling us > that > there is an error when building with your series, but this doesn't > tell > me why this is the correct fix. > > This is also why I asked to have the xen binary because I want to > check > whether this was a latent bug in Xen or your series effectively > introduce a bug. > > Note that regardless what I just wrote this is a good idea to align > __proc_info_start. I will try to have a closer look later and propose > a > commit message and/or any action for your other series. Regarding binaries please take a look here: https://lore.kernel.org/xen-devel/aa2862eacccfb0574859bf4cda8f4992baa5d2e1.camel@gmail.com/ I am not sure if you get my answer as I had the message from delivery server that it was blocked for some reason. ~ Oleksii
On 01/03/2023 16:38, Oleksii wrote: > Hi Julien, Hi, > On Wed, 2023-03-01 at 16:21 +0000, Julien Grall wrote: >> Hi Oleksii, >> >> On 01/03/2023 16:14, Oleksii Kurochko wrote: >>> During testing of bug.h's macros generic implementation yocto- >>> qemuarm >>> job crashed with data abort: >> >> The commit message is lacking some information. You are telling us >> that >> there is an error when building with your series, but this doesn't >> tell >> me why this is the correct fix. >> >> This is also why I asked to have the xen binary because I want to >> check >> whether this was a latent bug in Xen or your series effectively >> introduce a bug. >> >> Note that regardless what I just wrote this is a good idea to align >> __proc_info_start. I will try to have a closer look later and propose >> a >> commit message and/or any action for your other series. > Regarding binaries please take a look here: > https://lore.kernel.org/xen-devel/aa2862eacccfb0574859bf4cda8f4992baa5d2e1.camel@gmail.com/ > > I am not sure if you get my answer as I had the message from delivery > server that it was blocked for some reason. I got the answer. The problem now is gitlab only keep the artifact for the latest build and it only provide a zImage (having the ELF would be easier). I will try to reproduce the error on my end. Cheers,
Hi, On 01/03/2023 17:50, Julien Grall wrote: > > > On 01/03/2023 16:38, Oleksii wrote: >> Hi Julien, > > Hi, > >> On Wed, 2023-03-01 at 16:21 +0000, Julien Grall wrote: >>> Hi Oleksii, >>> >>> On 01/03/2023 16:14, Oleksii Kurochko wrote: >>>> During testing of bug.h's macros generic implementation yocto- >>>> qemuarm >>>> job crashed with data abort: >>> >>> The commit message is lacking some information. You are telling us >>> that >>> there is an error when building with your series, but this doesn't >>> tell >>> me why this is the correct fix. >>> >>> This is also why I asked to have the xen binary because I want to >>> check >>> whether this was a latent bug in Xen or your series effectively >>> introduce a bug. >>> >>> Note that regardless what I just wrote this is a good idea to align >>> __proc_info_start. I will try to have a closer look later and propose >>> a >>> commit message and/or any action for your other series. >> Regarding binaries please take a look here: >> https://lore.kernel.org/xen-devel/aa2862eacccfb0574859bf4cda8f4992baa5d2e1.camel@gmail.com/ >> >> I am not sure if you get my answer as I had the message from delivery >> server that it was blocked for some reason. > > I got the answer. The problem now is gitlab only keep the artifact for > the latest build and it only provide a zImage (having the ELF would be > easier). > > I will try to reproduce the error on my end. I managed to reproduce it. It looks like that after your bug patch, *(.rodata.*) will not be end on a 4-byte boundary. Before your patch, all the messages will be in .rodata.str. Now they are in .bug_frames.*, so there some difference in .rodata.*. That said, it is not entirely clear why we never seen the issue before because my guessing there are no guarantee that .rodata.* will be suitably aligned. Anyway, here a proposal for the commit message: " xen/arm: Ensure the start *(.proc.info) of is 4-byte aligned The entries in *(.proc.info) are expected to be 4-byte aligned and the compiler will access them using 4-byte load instructions. On Arm32, the alignment is strictly enforced by the processor and will result to a data abort if it is not correct. However, the linker script doesn't encode this requirement. So we are at the mercy of the compiler/linker to have aligned the previous sections suitably. This was spotted when trying to use the upcoming generic bug infrastructure with the compiler provided by Yocto. Link: https://lore.kernel.org/xen-devel/6735859208c6dcb7320f89664ae298005f70827b.camel@gmail.com/ " If you are happy with the proposed commit message, then I can update it while committing. Cheers,
Hi Julien, > > > On Wed, 2023-03-01 at 16:21 +0000, Julien Grall wrote: > > > > Hi Oleksii, > > > > > > > > On 01/03/2023 16:14, Oleksii Kurochko wrote: > > > > > During testing of bug.h's macros generic implementation > > > > > yocto- > > > > > qemuarm > > > > > job crashed with data abort: > > > > > > > > The commit message is lacking some information. You are telling > > > > us > > > > that > > > > there is an error when building with your series, but this > > > > doesn't > > > > tell > > > > me why this is the correct fix. > > > > > > > > This is also why I asked to have the xen binary because I want > > > > to > > > > check > > > > whether this was a latent bug in Xen or your series effectively > > > > introduce a bug. > > > > > > > > Note that regardless what I just wrote this is a good idea to > > > > align > > > > __proc_info_start. I will try to have a closer look later and > > > > propose > > > > a > > > > commit message and/or any action for your other series. > > > Regarding binaries please take a look here: > > > https://lore.kernel.org/xen-devel/aa2862eacccfb0574859bf4cda8f4992baa5d2e1.camel@gmail.com/ > > > > > > I am not sure if you get my answer as I had the message from > > > delivery > > > server that it was blocked for some reason. > > > > I got the answer. The problem now is gitlab only keep the artifact > > for > > the latest build and it only provide a zImage (having the ELF would > > be > > easier). > > > > I will try to reproduce the error on my end. > > I managed to reproduce it. It looks like that after your bug patch, > *(.rodata.*) will not be end on a 4-byte boundary. Before your patch, > all the messages will be in .rodata.str. Now they are in > .bug_frames.*, > so there some difference in .rodata.*. > > That said, it is not entirely clear why we never seen the issue > before > because my guessing there are no guarantee that .rodata.* will be > suitably aligned. > > Anyway, here a proposal for the commit message: > > " > xen/arm: Ensure the start *(.proc.info) of is 4-byte aligned > > The entries in *(.proc.info) are expected to be 4-byte aligned and > the > compiler will access them using 4-byte load instructions. On Arm32, > the > alignment is strictly enforced by the processor and will result to a > data abort if it is not correct. > > However, the linker script doesn't encode this requirement. So we are > at > the mercy of the compiler/linker to have aligned the previous > sections > suitably. > > This was spotted when trying to use the upcoming generic bug > infrastructure with the compiler provided by Yocto. > > Link: > https://lore.kernel.org/xen-devel/6735859208c6dcb7320f89664ae298005f70827b.camel@gmail.com/ > " > > If you are happy with the proposed commit message, then I can update > it > while committing. I am happy with the proposed commit message. Thanks a lot. ~ Oleksii
On 01.03.2023 21:38, Julien Grall wrote: > On 01/03/2023 17:50, Julien Grall wrote: >> I got the answer. The problem now is gitlab only keep the artifact for >> the latest build and it only provide a zImage (having the ELF would be >> easier). >> >> I will try to reproduce the error on my end. > > I managed to reproduce it. It looks like that after your bug patch, > *(.rodata.*) will not be end on a 4-byte boundary. Before your patch, > all the messages will be in .rodata.str. Now they are in .bug_frames.*, > so there some difference in .rodata.*. Strings in .bug_frames.*? This sounds like a mistake, which - if indeed the case - will want investigating before the conversion series is actually considered for committing. > That said, it is not entirely clear why we never seen the issue before > because my guessing there are no guarantee that .rodata.* will be > suitably aligned. > > Anyway, here a proposal for the commit message: > > " > xen/arm: Ensure the start *(.proc.info) of is 4-byte aligned > > The entries in *(.proc.info) are expected to be 4-byte aligned and the > compiler will access them using 4-byte load instructions. On Arm32, the > alignment is strictly enforced by the processor and will result to a > data abort if it is not correct. > > However, the linker script doesn't encode this requirement. So we are at > the mercy of the compiler/linker to have aligned the previous sections > suitably. May I suggest "aligned/padded", because it's really the tail of the previous section which matters? Jan > This was spotted when trying to use the upcoming generic bug > infrastructure with the compiler provided by Yocto. > > Link: > https://lore.kernel.org/xen-devel/6735859208c6dcb7320f89664ae298005f70827b.camel@gmail.com/ > " > > If you are happy with the proposed commit message, then I can update it > while committing. > > Cheers, >
Hi, On 02/03/2023 09:45, Jan Beulich wrote: > On 01.03.2023 21:38, Julien Grall wrote: >> On 01/03/2023 17:50, Julien Grall wrote: >>> I got the answer. The problem now is gitlab only keep the artifact for >>> the latest build and it only provide a zImage (having the ELF would be >>> easier). >>> >>> I will try to reproduce the error on my end. >> >> I managed to reproduce it. It looks like that after your bug patch, >> *(.rodata.*) will not be end on a 4-byte boundary. Before your patch, >> all the messages will be in .rodata.str. Now they are in .bug_frames.*, >> so there some difference in .rodata.*. > > Strings in .bug_frames.*? This sounds like a mistake, which - if indeed > the case - will want investigating before the conversion series is > actually considered for committing. No. I misread the code. But there are definitely a difference in size: Before: Section Headers: [Nr] Name Type Addr Off Size ES Flg Lk Inf Al [ 0] NULL 00000000 000000 000000 00 0 0 0 [ 1] .text PROGBITS 00200000 008000 07e7a8 00 WAX 0 0 32 [ 2] .rodata PROGBITS 0027f000 087000 02acc8 00 A 0 0 16 [ 3] .note.gnu.build-i NOTE 002a9cc8 0b1cc8 000024 00 A 0 0 4 [ 4] .data.ro_after_in PROGBITS 002aa000 0b2000 001000 00 WA 0 0 8 [ 5] .data.read_mostly PROGBITS 002ab000 0b3000 001118 00 WA 0 0 8 [ 6] .data PROGBITS 002ad000 0b5000 006ca8 00 WA 0 0 4096 [ 7] .arch.info PROGBITS 002b3ca8 0bbca8 000208 00 A 0 0 4 [ 8] .dev.info PROGBITS 002b3eb0 0bbeb0 000070 00 A 0 0 4 [ 9] .init.text PROGBITS 002b4000 0bc000 016054 00 AX 0 0 8 [10] .init.data PROGBITS 002d0000 0d8000 030008 00 WA 0 0 32768 [11] .bss NOBITS 00308000 108008 0394c0 00 WA 0 0 4096 [12] .debug_abbrev PROGBITS 00000000 108008 03006e 00 0 0 1 [13] .debug_info PROGBITS 00000000 138076 27b6b5 00 0 0 1 [14] .debug_str PROGBITS 00000000 3b372b 01a123 01 MS 0 0 1 [15] .debug_line PROGBITS 00000000 3cd84e 0a59e6 00 0 0 1 [16] .debug_frame PROGBITS 00000000 473234 018cc8 00 0 0 4 [17] .debug_loc PROGBITS 00000000 48befc 108473 00 0 0 1 [18] .debug_ranges PROGBITS 00000000 594370 031450 00 0 0 8 [19] .debug_aranges PROGBITS 00000000 5c57c0 004dd0 00 0 0 8 [20] .comment PROGBITS 00000000 5ca590 00005d 01 MS 0 0 1 [21] .ARM.attributes ARM_ATTRIBUTES 00000000 5ca5ed 000037 00 0 0 1 [22] .symtab SYMTAB 00000000 5ca624 022d60 10 23 7573 4 [23] .strtab STRTAB 00000000 5ed384 00c631 00 0 0 1 [24] .shstrtab STRTAB 00000000 5f99b5 00010b 00 0 0 1 After: [Nr] Name Type Addr Off Size ES Flg Lk Inf Al [ 0] NULL 00000000 000000 000000 00 0 0 0 [ 1] .text PROGBITS 00200000 008000 07e670 00 WAX 0 0 32 [ 2] .rodata PROGBITS 0027f000 087000 02b3e8 00 A 0 0 16 [ 3] .note.gnu.build-i NOTE 002aa3e8 0b23e8 000024 00 A 0 0 4 [ 4] .data.ro_after_in PROGBITS 002ab000 0b3000 001000 00 WA 0 0 8 [ 5] .data.read_mostly PROGBITS 002ac000 0b4000 001118 00 WA 0 0 8 [ 6] .data PROGBITS 002ae000 0b6000 006ca8 00 WA 0 0 4096 [ 7] .arch.info PROGBITS 002b4ca8 0bcca8 000208 00 A 0 0 4 [ 8] .dev.info PROGBITS 002b4eb0 0bceb0 000070 00 A 0 0 4 [ 9] .init.text PROGBITS 002b5000 0bd000 016054 00 AX 0 0 8 [10] .init.data PROGBITS 002d0000 0d8000 030008 00 WA 0 0 32768 [11] .bss NOBITS 00308000 108008 0394c0 00 WA 0 0 4096 [12] .debug_abbrev PROGBITS 00000000 108008 02fe48 00 0 0 1 [13] .debug_info PROGBITS 00000000 137e50 27ac8f 00 0 0 1 [14] .debug_str PROGBITS 00000000 3b2adf 01a107 01 MS 0 0 1 [15] .debug_line PROGBITS 00000000 3ccbe6 0a590e 00 0 0 1 [16] .debug_frame PROGBITS 00000000 4724f4 018da0 00 0 0 4 [17] .debug_loc PROGBITS 00000000 48b294 10822c 00 0 0 1 [18] .debug_ranges PROGBITS 00000000 5934c0 031028 00 0 0 8 [19] .debug_aranges PROGBITS 00000000 5c44e8 004dd8 00 0 0 8 [20] .comment PROGBITS 00000000 5c92c0 00005d 01 MS 0 0 1 [21] .ARM.attributes ARM_ATTRIBUTES 00000000 5c931d 000037 00 0 0 1 [22] .symtab SYMTAB 00000000 5c9354 0220f0 10 23 7374 4 [23] .strtab STRTAB 00000000 5eb444 00c677 00 0 0 1 [24] .shstrtab STRTAB 00000000 5f7abb 00010b 00 0 0 1 It is not entirely clear why. Anyway, it doesn't matter too much. Note that the size of Xen grew a little bit on Arm (+0.03%). I am not too concerned though as we now consolidated the BUG infrastructure. So that's a plus. > >> That said, it is not entirely clear why we never seen the issue before >> because my guessing there are no guarantee that .rodata.* will be >> suitably aligned. >> >> Anyway, here a proposal for the commit message: >> >> " >> xen/arm: Ensure the start *(.proc.info) of is 4-byte aligned >> >> The entries in *(.proc.info) are expected to be 4-byte aligned and the >> compiler will access them using 4-byte load instructions. On Arm32, the >> alignment is strictly enforced by the processor and will result to a >> data abort if it is not correct. >> >> However, the linker script doesn't encode this requirement. So we are at >> the mercy of the compiler/linker to have aligned the previous sections >> suitably. > > May I suggest "aligned/padded", because it's really the tail of the > previous section which matters? Sure. Cheers,
On 02.03.2023 12:01, Julien Grall wrote: > On 02/03/2023 09:45, Jan Beulich wrote: >> On 01.03.2023 21:38, Julien Grall wrote: >>> I managed to reproduce it. It looks like that after your bug patch, >>> *(.rodata.*) will not be end on a 4-byte boundary. Before your patch, >>> all the messages will be in .rodata.str. Now they are in .bug_frames.*, >>> so there some difference in .rodata.*. >> >> Strings in .bug_frames.*? This sounds like a mistake, which - if indeed >> the case - will want investigating before the conversion series is >> actually considered for committing. > > No. I misread the code. But there are definitely a difference in size: > > Before: > > Section Headers: > [Nr] Name Type Addr Off Size ES Flg > Lk Inf Al > [ 0] NULL 00000000 000000 000000 00 > 0 0 0 > [ 1] .text PROGBITS 00200000 008000 07e7a8 00 WAX > 0 0 32 > [ 2] .rodata PROGBITS 0027f000 087000 02acc8 00 A > 0 0 16 >[...] > After: > > [Nr] Name Type Addr Off Size ES Flg > Lk Inf Al > [ 0] NULL 00000000 000000 000000 00 > 0 0 0 > [ 1] .text PROGBITS 00200000 008000 07e670 00 WAX > 0 0 32 > [ 2] .rodata PROGBITS 0027f000 087000 02b3e8 00 A > 0 0 16 I still find this concerning (as in: at least needing explanation), as the bug frame representation shrinks in size: Entries for assertions remain to be 4 ".long"-s, while all other entries use only two now. So in a release build the size of all .bug_frame.* together should halve, while in debug builds it should shrink at least some. And Oleksii's series doesn't add meaningful other contributions to .rodata, iirc. Jan
Hi Jan, On 02/03/2023 11:21, Jan Beulich wrote: > On 02.03.2023 12:01, Julien Grall wrote: >> On 02/03/2023 09:45, Jan Beulich wrote: >>> On 01.03.2023 21:38, Julien Grall wrote: >>>> I managed to reproduce it. It looks like that after your bug patch, >>>> *(.rodata.*) will not be end on a 4-byte boundary. Before your patch, >>>> all the messages will be in .rodata.str. Now they are in .bug_frames.*, >>>> so there some difference in .rodata.*. >>> >>> Strings in .bug_frames.*? This sounds like a mistake, which - if indeed >>> the case - will want investigating before the conversion series is >>> actually considered for committing. >> >> No. I misread the code. But there are definitely a difference in size: >> >> Before: >> >> Section Headers: >> [Nr] Name Type Addr Off Size ES Flg >> Lk Inf Al >> [ 0] NULL 00000000 000000 000000 00 >> 0 0 0 >> [ 1] .text PROGBITS 00200000 008000 07e7a8 00 WAX >> 0 0 32 >> [ 2] .rodata PROGBITS 0027f000 087000 02acc8 00 A >> 0 0 16 >> [...] >> After: >> >> [Nr] Name Type Addr Off Size ES Flg >> Lk Inf Al >> [ 0] NULL 00000000 000000 000000 00 >> 0 0 0 >> [ 1] .text PROGBITS 00200000 008000 07e670 00 WAX >> 0 0 32 >> [ 2] .rodata PROGBITS 0027f000 087000 02b3e8 00 A >> 0 0 16 > > I still find this concerning (as in: at least needing explanation), as > the bug frame representation shrinks in size: Entries for assertions > remain to be 4 ".long"-s, while all other entries use only two now. So > in a release build the size of all .bug_frame.* together should halve, > while in debug builds it should shrink at least some. And Oleksii's > series doesn't add meaningful other contributions to .rodata, iirc. Doh, I inverted the two tables. So .rodata is shrinking but .txt is slightly increasing. Cheers,
Hi Oleksii, On 02/03/2023 07:34, Oleksii wrote: > Hi Julien, >>>> On Wed, 2023-03-01 at 16:21 +0000, Julien Grall wrote: >>>>> Hi Oleksii, >>>>> >>>>> On 01/03/2023 16:14, Oleksii Kurochko wrote: >>>>>> During testing of bug.h's macros generic implementation >>>>>> yocto- >>>>>> qemuarm >>>>>> job crashed with data abort: >>>>> >>>>> The commit message is lacking some information. You are telling >>>>> us >>>>> that >>>>> there is an error when building with your series, but this >>>>> doesn't >>>>> tell >>>>> me why this is the correct fix. >>>>> >>>>> This is also why I asked to have the xen binary because I want >>>>> to >>>>> check >>>>> whether this was a latent bug in Xen or your series effectively >>>>> introduce a bug. >>>>> >>>>> Note that regardless what I just wrote this is a good idea to >>>>> align >>>>> __proc_info_start. I will try to have a closer look later and >>>>> propose >>>>> a >>>>> commit message and/or any action for your other series. >>>> Regarding binaries please take a look here: >>>> https://lore.kernel.org/xen-devel/aa2862eacccfb0574859bf4cda8f4992baa5d2e1.camel@gmail.com/ >>>> >>>> I am not sure if you get my answer as I had the message from >>>> delivery >>>> server that it was blocked for some reason. >>> >>> I got the answer. The problem now is gitlab only keep the artifact >>> for >>> the latest build and it only provide a zImage (having the ELF would >>> be >>> easier). >>> >>> I will try to reproduce the error on my end. >> >> I managed to reproduce it. It looks like that after your bug patch, >> *(.rodata.*) will not be end on a 4-byte boundary. Before your patch, >> all the messages will be in .rodata.str. Now they are in >> .bug_frames.*, >> so there some difference in .rodata.*. >> >> That said, it is not entirely clear why we never seen the issue >> before >> because my guessing there are no guarantee that .rodata.* will be >> suitably aligned. >> >> Anyway, here a proposal for the commit message: >> >> " >> xen/arm: Ensure the start *(.proc.info) of is 4-byte aligned >> >> The entries in *(.proc.info) are expected to be 4-byte aligned and >> the >> compiler will access them using 4-byte load instructions. On Arm32, >> the >> alignment is strictly enforced by the processor and will result to a >> data abort if it is not correct. >> >> However, the linker script doesn't encode this requirement. So we are >> at >> the mercy of the compiler/linker to have aligned the previous >> sections >> suitably. >> >> This was spotted when trying to use the upcoming generic bug >> infrastructure with the compiler provided by Yocto. >> >> Link: >> https://lore.kernel.org/xen-devel/6735859208c6dcb7320f89664ae298005f70827b.camel@gmail.com/ >> " >> >> If you are happy with the proposed commit message, then I can update >> it >> while committing. > I am happy with the proposed commit message. Thanks. With that: Reviewed-by: Julien Grall <jgrall@amazon.com> I have addressed Jan's comment and committed the patch. Cheers,
On Thu, 2023-03-02 at 14:50 +0000, Julien Grall wrote: > Hi Oleksii, > > On 02/03/2023 07:34, Oleksii wrote: > > Hi Julien, > > > > > On Wed, 2023-03-01 at 16:21 +0000, Julien Grall wrote: > > > > > > Hi Oleksii, > > > > > > > > > > > > On 01/03/2023 16:14, Oleksii Kurochko wrote: > > > > > > > During testing of bug.h's macros generic implementation > > > > > > > yocto- > > > > > > > qemuarm > > > > > > > job crashed with data abort: > > > > > > > > > > > > The commit message is lacking some information. You are > > > > > > telling > > > > > > us > > > > > > that > > > > > > there is an error when building with your series, but this > > > > > > doesn't > > > > > > tell > > > > > > me why this is the correct fix. > > > > > > > > > > > > This is also why I asked to have the xen binary because I > > > > > > want > > > > > > to > > > > > > check > > > > > > whether this was a latent bug in Xen or your series > > > > > > effectively > > > > > > introduce a bug. > > > > > > > > > > > > Note that regardless what I just wrote this is a good idea > > > > > > to > > > > > > align > > > > > > __proc_info_start. I will try to have a closer look later > > > > > > and > > > > > > propose > > > > > > a > > > > > > commit message and/or any action for your other series. > > > > > Regarding binaries please take a look here: > > > > > https://lore.kernel.org/xen-devel/aa2862eacccfb0574859bf4cda8f4992baa5d2e1.camel@gmail.com/ > > > > > > > > > > I am not sure if you get my answer as I had the message from > > > > > delivery > > > > > server that it was blocked for some reason. > > > > > > > > I got the answer. The problem now is gitlab only keep the > > > > artifact > > > > for > > > > the latest build and it only provide a zImage (having the ELF > > > > would > > > > be > > > > easier). > > > > > > > > I will try to reproduce the error on my end. > > > > > > I managed to reproduce it. It looks like that after your bug > > > patch, > > > *(.rodata.*) will not be end on a 4-byte boundary. Before your > > > patch, > > > all the messages will be in .rodata.str. Now they are in > > > .bug_frames.*, > > > so there some difference in .rodata.*. > > > > > > That said, it is not entirely clear why we never seen the issue > > > before > > > because my guessing there are no guarantee that .rodata.* will be > > > suitably aligned. > > > > > > Anyway, here a proposal for the commit message: > > > > > > " > > > xen/arm: Ensure the start *(.proc.info) of is 4-byte aligned > > > > > > The entries in *(.proc.info) are expected to be 4-byte aligned > > > and > > > the > > > compiler will access them using 4-byte load instructions. On > > > Arm32, > > > the > > > alignment is strictly enforced by the processor and will result > > > to a > > > data abort if it is not correct. > > > > > > However, the linker script doesn't encode this requirement. So we > > > are > > > at > > > the mercy of the compiler/linker to have aligned the previous > > > sections > > > suitably. > > > > > > This was spotted when trying to use the upcoming generic bug > > > infrastructure with the compiler provided by Yocto. > > > > > > Link: > > > https://lore.kernel.org/xen-devel/6735859208c6dcb7320f89664ae298005f70827b.camel@gmail.com/ > > > " > > > > > > If you are happy with the proposed commit message, then I can > > > update > > > it > > > while committing. > > I am happy with the proposed commit message. > > Thanks. With that: > > Reviewed-by: Julien Grall <jgrall@amazon.com> > > I have addressed Jan's comment and committed the patch. > Thanks a lot. Not generic bug feature is unblock. I'll wait for comments till tomorrow. If it won't be any that will sent new patch series. ~ Oleksii
diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S index 3f7ebd19f3..1b392345bc 100644 --- a/xen/arch/arm/xen.lds.S +++ b/xen/arch/arm/xen.lds.S @@ -67,6 +67,7 @@ SECTIONS *(.data.rel.ro) *(.data.rel.ro.*) + . = ALIGN(4); __proc_info_start = .; *(.proc.info) __proc_info_end = .;
During testing of bug.h's macros generic implementation yocto-qemuarm job crashed with data abort: (XEN) Data Abort Trap. Syndrome=0x1830021 (XEN) Walking Hypervisor VA 0x2a5ca6 on CPU0 via TTBR 0x000000004014a000 (XEN) 1ST[0x000] = 0x0000000040149f7f (XEN) 2ND[0x001] = 0x0040000040148f7f (XEN) 3RD[0x0a5] = 0x00400000400b5fff (XEN) CPU0: Unexpected Trap: Data Abort (XEN) ----[ Xen-4.18-unstable arm32 debug=y Not tainted ]---- (XEN) CPU: 0 (XEN) PC: 0020063c head.o#__lookup_processor_type+0x1c/0x44 (XEN) CPSR: 600001da MODE:Hypervisor (XEN) R0: 412fc0f1 R1: 002a5ca2 R2: 002a5cd2 R3: 600001da (XEN) R4: 002a7e9c R5: 00000011 R6: 00000000 R7: ffffffff (XEN) R8: 48008f20 R9: 00000000 R10:00000000 R11:002ffecc R12:00000000 (XEN) HYP: SP: 002ffeb8 LR: 00200618 (XEN) (XEN) VTCR_EL2: 00000000 (XEN) VTTBR_EL2: 0000000000000000 (XEN) (XEN) SCTLR_EL2: 30cd187f (XEN) HCR_EL2: 00000038 (XEN) TTBR0_EL2: 000000004014a000 (XEN) (XEN) ESR_EL2: 97830021 (XEN) HPFAR_EL2: 00000000 (XEN) HDFAR: 002a5ca6 (XEN) HIFAR: 00000000 (XEN) (XEN) Xen stack trace from sp=002ffeb8: (XEN) 97830021 002a7e9c 00000000 00276a88 002fff54 002c8fc4 11112131 10011142 (XEN) 00000000 002a5500 00000000 00000000 00008f20 00002000 48000000 002e01f0 (XEN) 00000000 60000000 00000000 40000000 00000001 002e0208 002a7e8c 002a7e88 (XEN) 002b0ab4 002e31f0 00000000 60000000 00000003 ffffffff 00000000 002aa000 (XEN) 00200060 00000000 00000000 48000000 40010000 3fe10000 00000000 00200068 (XEN) 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 (XEN) 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 (XEN) 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 (XEN) 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 (XEN) 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 (XEN) 00000000 00000000 (XEN) Xen call trace: (XEN) [<0020063c>] head.o#__lookup_processor_type+0x1c/0x44 (PC) (XEN) [<00200618>] lookup_processor_type+0xc/0x14 (LR) (XEN) [<002c8fc4>] start_xen+0xb8c/0x1138 (XEN) [<00200068>] head.o#primary_switched+0x8/0x1c (XEN) (XEN) (XEN) **************************************** (XEN) Panic on CPU 0: (XEN) CPU0: Unexpected Trap: Data Abort (XEN) **************************************** (XEN) (XEN) Reboot in five seconds... (XEN) Xen: Platform reset did not work properly! (XEN) Xen: Platform reset did not work properly! Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> --- xen/arch/arm/xen.lds.S | 1 + 1 file changed, 1 insertion(+)