Re: [PATCH v7 09/12] xen: add runtime parameter access support to hypfs

[Jan Beulich <jbeulich@xxxxxxxx>]

On 14.04.2020 11:45, Julien Grall wrote:
> 
> 
> On 14/04/2020 10:31, Jan Beulich wrote:
>> On 14.04.2020 11:29, Julien Grall wrote:
>>> On 03/04/2020 16:31, Jürgen Groß wrote:
>>>> On 03.04.20 16:51, Jan Beulich wrote:
>>>>> On 02.04.2020 17:46, Juergen Gross wrote:
>>>>>> V7:
>>>>>> - fine tune some parameter initializations (Jan Beulich)
>>>>>> - call custom_runtime_set_var() after updating the value
>>>>>> - modify alignment in Arm linker script to 4 (Jan Beulich)
>>>>>
>>>>> I didn't ask for this to be unilaterally 4 - I don't think this
>>>>> would work on Arm64, seeing that there are pointers inside the
>>>>> struct. This wants to be pointer size, i.e. 4 for Arm32 but 8
>>>>> for Arm64.
>>>
>>> We don't allow unaligned access on Arm32, so if your structure happen to 
>>> have a 64-bit value in it then you will get a crash at runtime.
>>>
>>> For safety, it should neither be POINTER_ALIGN or 4, but 8.
>>> This is going to make your linker more robust.
>>
>> Would you mind explaining to me why POINTER_ALIGN would be wrong
>> when the most strictly aligned field in a structure is a pointer?
> Both are valid with one difference though. If tomorrow someone send
> a patch to add a 64-bit in the structure, what are the chance one
> won't notice the alignment change? It is quite high.

Hmm, adjustments altering structure alignment that affect linker
script correctness should imo always be accompanied by checking
what the linker scripts has for the specific structure.

> If you align the section to 8, then you make your code more robust
> at the expense of possibly adding an extra 4-bytes in your binary.

Well, you're the maintainer for Arm, so you've got to judge. I'd
view things the other way around. Yes, it's less likely for even
larger alignment requirements to get introduced, but why not be
careful about these too and, say, align everything to PAGE_SIZE?
IOW - where do you draw the line in a non-arbitrary way?

Jan