Re: [PATCH 1/2] xen: make include/xen/unaligned.h usable on all architectures

[Juergen Gross <jgross@suse.com>]

[-- Attachment #1.1.1: Type: text/plain, Size: 2345 bytes --]

On 05.12.23 17:29, Julien Grall wrote:
> Hi Arnd,
> 
> On 05/12/2023 14:37, Arnd Bergmann wrote:
>> On Tue, Dec 5, 2023, at 15:19, Julien Grall wrote:
>>> On 05/12/2023 14:10, Arnd Bergmann wrote:
>>>> On Tue, Dec 5, 2023, at 15:01, Julien Grall wrote:
>>>>> On 05/12/2023 13:59, Jan Beulich wrote:
>>>>>> On 05.12.2023 14:46, Julien Grall wrote:
>>>> This would repeat the mistake that we had in Linux in the
>>>> past (and still do in some drivers): Simply dereferencing
>>>> a misaligned pointer is always a bug, even on architectures
>>>> that have efficient unaligned load/store instructions,
>>>> because C makes this undefined behavior and gcc has
>>>> optimizations that assume e.g. 'int *' to never have
>>>> the lower two bits set [1].
>>>
>>> Just to clarify, I haven't suggested to use 'int *'. My point was more
>>> that I don't think that the helpers would work as-is on arm32 because
>>> even if the ISA allows a direct access, we are setting the bit in SCTLR
>>> to disable unaligned access.
>>>
>>> As Juergen is proposing a common header, then I could ask him to do the
>>> work to confirm that the helpers properly work on arm32. But I think
>>> this is unfair.
>>
>> When I introduced the helpers in Linux, I showed that these
>> produce the best output on all modern compilers (at least gcc-5,
>> probably earlier) for both architectures that allow unaligned
>> access and for those that don't. We used to have architecture
>> specific helpers depending on what each architecture could
>> do, but all the other variants we had were either wrong or
>> less efficient.
>>
>> If for some reason an Arm system is configured to trap
>> all unaligned access, then you must already pass
>> -mno-unaligned-access to the compiler to prevent certain
>> optimizations, and then the helpers will still behave
>> correctly (the same way they do on armv5, which never has
>> unaligned access). On armv7 with -munaligned-access, the
>> same functions only prevent the use of stm/ldm and strd/ldrd
>> but still use ldr/str.
> 
> Unfortunately we don't explicitely do. This would explain why I saw some issues 
> with certain compiler [1].
> 
> So I agree that adding -mno-unaligned-access for arm32 makes sense.
> 
> @Juergen, do you want me to send a patch?

Yes, will do.


Juergen


[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]