Re: [PATCH 1/3] introduce unaligned.h

[Jan Beulich <jbeulich@xxxxxxxx>]

On 18.01.2021 10:33, Jan Beulich wrote:
> On 15.01.2021 12:27, Jan Beulich wrote:
>> On 15.01.2021 12:13, Andrew Cooper wrote:
>>> On 15/01/2021 10:05, Jan Beulich wrote:
>>>> Rather than open-coding commonly used constructs in yet more places when
>>>> pulling in zstd decompression support (and its xxhash prereq), pull out
>>>> the custom bits into a commonly used header (for the hypervisor build;
>>>> the tool stack and stubdom builds of libxenguest will still remain in
>>>> need of similarly taking care of). For now this is limited to x86, where
>>>> custom logic isn't needed (considering this is going to be used in init
>>>> code only, even using alternatives patching to use MOVBE doesn't seem
>>>> worthwhile).
>>>>
>>>> No change in generated code.
>>>>
>>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>>> ---
>>>> Iirc use of include/asm-generic/ was disliked, hence the generic header
>>>> goes into include/xen/.
>>>
>>> Really?  I think its going to be the only sane way of fixing up some of
>>> our header tangle.
>>>
>>> This series probably isn't the right place to fix this argument, so
>>> Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>>
>> Thanks.
>>
>>> However, presumably we're going to want an ARM side of this imminently?
>>
>> Why? It's only used (and going to be further used) by code not
>> built for Arm. So while it certainly would be nice for such a
>> header to also appear there (and the x86-special casing going
>> away in patch 2), it's not a strict requirement at this point.
>> Therefore I'd prefer to leave this to the Arm maintainers (and
>> probably for 4.16).
> 
> I was wrong here, when it comes to an Arm64 build with ACPI
> support enabled. xen/arch/arm/efi/efi-dom0.c has
> 
> #define XZ_EXTERN STATIC
> #include "../../../common/xz/crc32.c"
> 
> in order to later do
> 
>     xz_crc32_init();
>     efi_sys_tbl->Hdr.CRC32 = xz_crc32((uint8_t *)efi_sys_tbl,
>                                       efi_sys_tbl->Hdr.HeaderSize, 0);
> 
> Urgh. Why in the world does xz code get re-used like this?
> If we need generic crc32 support, such should imo live in
> xen/lib/.
> 
> So we have two possible courses of action: Eliminate this
> unsuitable re-use of code, or introduce asm/unaligned.h
> for Arm (or at least Arm64, in case it makes a difference)
> right away.

There's an even easier 3rd option: Drop #include "private.h"
from xz/crc32.c. I'll check all the various builds, but I
don't foresee any problems. In case there indeed are none,
this would be my preferred short-term workaround (which I'd
fold right into this patch).

Jan