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

[Julien Grall <julien@xen.org>]

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,

-- 
Julien Grall