Re: [PATCH v4 1/4] xen/riscv: introduce asm/types.h header file

[Jan Beulich <jbeulich@xxxxxxxx>]

On 17.01.2023 10:29, Oleksii wrote:
> On Mon, 2023-01-16 at 15:59 +0100, Jan Beulich wrote:
>> On 16.01.2023 15:39, Oleksii Kurochko wrote:
>>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
>>> ---
>>> Changes in V4:
>>>     - Clean up types in <asm/types.h> and remain only necessary.
>>>       The following types was removed as they are defined in
>>> <xen/types.h>:
>>>       {__|}{u|s}{8|16|32|64}
>>
>> For one you still typedef u32 and u64. And imo correctly so, until we
>> get around to move the definition basic types into xen/types.h. Plus
>> I can't see how things build for you: xen/types.h expects __{u,s}<N>
> It builds because nothing is used <xen/types.h> now so that's why I
> missed them but you are right __{u,s}<N> should be backed.
> It looks like {__,}{u,s}{8,16,32} are the same for all available in Xen
> architectures so probably can I move them to <xen/types.h> instead of
> remain them in <asm/types.h>?

This next step isn't quite as obvious, i.e. has room for being
contentious. In particular deriving fixed width types from C basic
types is setting us up for future problems (especially in the
context of RISC-V think of RV128). Therefore, if we touch and
generalize this, I'd like to sanitize things at the same time.

I'd then prefer to typedef {u,}int<N>_t by using either the "mode"
attribute (requiring us to settle on a prereq of there always being
8 bits per char) or the compiler supplied __{U,}INT<N>_TYPE__
(taking gcc 4.7 as a prereq; didn't check clang yet). Both would
allow {u,}int64_t to also be put in the common header. Yet if e.g.
a prereq assumption faced opposition, some other approach would
need to be found. Plus using either of the named approaches has
issues with the printf() format specifiers, for which I'm yet to
figure out a solution (or maybe someone else knows a good way to
deal with that; using compiler provided headers isn't an option
afaict, as gcc provides stdint.h but not inttypes.h, but maybe
glibc's simplistic approach is good enough - they're targeting
far more architectures than we do and get away with that).

Jan