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

[Julien Grall <julien@xen.org>]

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,

-- 
Julien Grall