Re: [PATCH v1] xen/arm: align *(.proc.info) in the linker script

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

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@xxxxxxxxx/
> > > > > 
> > > > > 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@xxxxxxxxx/
> > > "
> > > 
> > > 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@xxxxxxxxxx>
> 
> 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