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

[Julien Grall <julien@xxxxxxx>]

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.

Cheers,

--
Julien Grall