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

[Juergen Gross <jgross@xxxxxxxx>]

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: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature