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

[Julien Grall <julien@xxxxxxx>]

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?

Cheers,

[1] c71163f6-2646-6fae-cb22-600eb0486539@xxxxxxx

--
Julien Grall