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

From: "Arnd Bergmann" <arnd@arndb.de>
To: "Julien Grall" <julien@xen.org>, "Jan Beulich" <jbeulich@suse.com>
Cc: "Andrew Cooper" <andrew.cooper3@citrix.com>,
	"George Dunlap" <george.dunlap@citrix.com>,
	"Stefano Stabellini" <sstabellini@kernel.org>,
	"Wei Liu" <wl@xen.org>, "Juergen Gross" <jgross@suse.com>,
	xen-devel@lists.xenproject.org
Subject: Re: [PATCH 1/2] xen: make include/xen/unaligned.h usable on all architectures
Date: Tue, 05 Dec 2023 15:10:40 +0100	[thread overview]
Message-ID: <5a05e194-28d1-409d-9f45-ca4de0fc78c8@app.fastmail.com> (raw)
In-Reply-To: <c65b22b2-ca1c-4c4c-9aa5-6cf74c0fdc6b@xen.org>

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