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

["Arnd Bergmann" <arnd@xxxxxxxx>]

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:
>>> On 05/12/2023 13:41, Juergen Gross wrote:
>>>> On 05.12.23 14:31, Julien Grall wrote:
>>>>> Anyway, given you don't seem to have a use-case yet, I would simply to
>>>>> consider to surround the declaration with an a config which can be
>>>>> selected if unaligned access is supported.
>>>>
>>>> Like in xen/common/lzo.c et al?
>>>
>>> Just to clarify, I am suggesting to add in unaligned.h:
>>>
>>> #ifdef CONFIG_HAS_UNALIGNED_ACCESS
>>>
>>> your definitions
>>>
>>> #endif
>> 
>> But that would be wrong: HAS_UNALIGNED_ACCESS would be there to indicate
>> one does _not_ need any special accessors.
>
> I am guessing you are disagreeing on the name rather than the concept? 
> If so, what about CONFIG_UNALIGNED_ACCESS_ALLOWED?

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].

The helpers that Jürgen copied from Linux is what we
use to abstract accesses to objects that we know may
be misaligned. If the underlying ISA allows a direct
access (e.g. on arm64 and x86), this is as efficient
as a normal pointer access but prevents the dangerous
optimizations in gcc. On architectures without these
instructions, the access will be turned into safe
bytewise access.

This is similar to a __packed annotation on a
data structure, but also works in cases where such
an annotation wouldn't work for other reasons.

     Arnd

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100363