Re: [PATCH v1] xen/config.h: Move BITS_PER_* definitions from asm/config.h to xen/config.h

[Jan Beulich <jbeulich@xxxxxxxx>]

On 27.03.2025 13:53, Andrew Cooper wrote:
> On 27/03/2025 8:21 am, Jan Beulich wrote:
>> On 27.03.2025 01:44, Andrew Cooper wrote:
>>> On 26/03/2025 5:47 pm, Oleksii Kurochko wrote:
>>>> diff --git a/xen/include/xen/config.h b/xen/include/xen/config.h
>>>> index d888b2314d..dbbf2fce62 100644
>>>> --- a/xen/include/xen/config.h
>>>> +++ b/xen/include/xen/config.h
>>>> @@ -98,4 +98,13 @@
>>>>  #define ZERO_BLOCK_PTR ((void *)-1L)
>>>>  #endif
>>>>  
>>>> +#define BYTES_PER_LONG  __SIZEOF_LONG__
>>>> +
>>>> +#define BITS_PER_BYTE   __CHAR_BIT__
>>>> +#define BITS_PER_INT    (__SIZEOF_INT__ << 3)
>>>> +#define BITS_PER_LONG   (BYTES_PER_LONG << 3)
>>>> +#define BITS_PER_LLONG  (__SIZEOF_LONG_LONG__ << 3)
>>>> +
>>>> +#define POINTER_ALIGN   __SIZEOF_POINTER__
>>> See how much nicer this is.  This patch possibly wants to wait until
>>> I've fixed the compiler checks to update to the new baseline, or we can
>>> just say that staging is staging and corner case error messages are fine.
>>>
>>> One thing, you have to replace the "<< 3" as you're hard-coding 8 here
>>> and ignoring __CHAR_BIT__.
>>>
>>> I'd suggest keeping the BITS_PER_BYTE on the LHS, e.g:
>>>
>>> #define BITS_PER_INT    (BITS_PER_BYTE * __SIZEOF_INT__)
>>>
>>> which tabulates better.
>>>
>>> I suggest keeping BITS_PER_XEN_ULONG to be arch-local.
>> I agree here despite ...
>>
>>>   ARM is the
>>> odd-one-out having a non-64bit arch use a 64bit XEN_ULONG.
>> ... not agreeing here: x86 is the odd-one-out; I sincerely hope any new ports
>> to 32-bit architectures / flavors will avoid compat layer translation by 
>> making
>> this type a proper 64-bit one. Architectures truly being 32-bit only, with no
>> expectation of a 64-bit flavor ever appearing, would be different.
> 
> I have some very firm opinions about this.
> 
> It is an error that the type xen_ulong exists, anywhere.  The fact it
> wasn't named guest_ulong shows a profound misunderstanding of its
> purpose and usage in the API/ABI.
> 
> Similarly, BITS_PER_XEN_ULONG is buggily named, and should be
> BITS_PER_GUEST_ULONG, as demonstrated by it's singular use in Xen
> (calculating BITS_PER_EVTCHN_WORD(d)).
> 
> ARM declaring that arm32 uses 64-bit xen_ulongs was cutting a corner
> that's going to bite twice as hard when 128bit comes along, and
> RISCV-128 is in progress already.

Yes indeed.

> All of this needs purging from the API/ABIs before RISC-V/PPC inherit
> the mistakes that are holding x86 and ARM back.

I'm curious to learn how you envision to support a 32-bit guest on RV128,
for example. You dislike the compat layer, and you also dislike xen_ulong_t
(I agree its name is potentially misleading). So you must be thinking of
something entirely new to express in particular interfacing structures.

Jan