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

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Tue, 2023-01-17 at 11:04 +0100, Jan Beulich wrote:
> 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).
> 
Thanks for the explanation.

If back to RISCV's <asm/types.h> it looks that the v2 of the patch
(https://lore.kernel.org/xen-devel/ca2674739cfa71cae0bf084a7b471ad4518026d3.1673278109.git.oleksii.kurochko@xxxxxxxxx/)
is the best one option now because as I can see some work is going on
around <xen/types.h> and keeping the minimal set of types now will
allow us to not remove unneeded types for RISCV's port from asm/types.h
in the future.

Even more based on your patch [
https://lists.xen.org/archives/html/xen-devel/2023-01/msg00720.html ]
RISCV's <asm/types.h> can be empty for this patch series.

> Jan
~Oleksii