On 31/03/2025 9:32 am, Jan Beulich wrote:
> On 28.03.2025 14:44, Andrew Cooper wrote:
>> From: Lin Liu <lin....@citrix.com>
>>
>> The current swab??() infrastructure is unecesserily complicated, and can be
>> repated entirely with compiler builtins.
>>
>> All supported compilers provide __BYTE_ORDER__ and __builtin_bswap??().
>>
>> Nothing in Xen cares about the values of __{BIG,LITTLE}_ENDIAN; just that one
>> of them is defined.  Therefore, centralise their definitions in xen/config.h
> And even if we cared somewhere, __ORDER_{BIG,LITTLE}_ENDIAN__ supply them
> just fine.
>
>> Signed-off-by: Lin Liu <lin....@citrix.com>
>> Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>
> Reviewed-by: Jan Beulich <jbeul...@suse.com>
> with two nits taken care of:
>
>> --- /dev/null
>> +++ b/xen/include/xen/byteorder.h
>> @@ -0,0 +1,44 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +#ifndef XEN_BYTEORDER_H
>> +#define XEN_BYTEORDER_H
>> +
>> +#include <xen/byteswap.h>
>> +#include <xen/types.h>
> It's stdint.h that's needed here, not types.h?

Perhaps.

>
>> +#if defined(__LITTLE_ENDIAN)
>> +
>> +# define cpu_to_le64(x) (uint64_t)(x)
>> +# define le64_to_cpu(x) (uint64_t)(x)
>> +# define cpu_to_le32(x) (uint32_t)(x)
>> +# define le32_to_cpu(x) (uint32_t)(x)
>> +# define cpu_to_le16(x) (uint16_t)(x)
>> +# define le16_to_cpu(x) (uint16_t)(x)
> (Not just) for Misra these all need another pair of parentheses around the
> entire expressions.

Why?  For both points?  Eclair is happy, with this at least.

What does fail is a bit a of a curveball.

https://saas.eclairit.com:3787/fs/var/local/eclair/xen-project.ecdf/xen-project/people/andyhhp/xen/ECLAIR_normal/xen-bswap/ARM64/9556392204/PROJECT.ecd;/by_service/MC3A2.R20.6.html

cpu_to_le64() turns from a real function to a macro under ARM, meaning
that the #ifdef __BIG_ENDIAN in the middle of a bunch of constants
becomes UB.

I'll need to fix that separately.

~Andrew

Reply via email to