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

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

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>?
> to be defined in order to then derive {u,}int<N>_t from them.
> 
> > --- /dev/null
> > +++ b/xen/arch/riscv/include/asm/types.h
> > @@ -0,0 +1,43 @@
> > +#ifndef __RISCV_TYPES_H__
> > +#define __RISCV_TYPES_H__
> > +
> > +#ifndef __ASSEMBLY__
> > +
> > +#if defined(CONFIG_RISCV_32)
> > +typedef unsigned long long u64;
> > +typedef unsigned int u32;
> > +typedef u32 vaddr_t;
> > +#define PRIvaddr PRIx32
> > +typedef u64 paddr_t;
> > +#define INVALID_PADDR (~0ULL)
> > +#define PRIpaddr "016llx"
> > +typedef u32 register_t;
> > +#define PRIregister "x"
> > +#elif defined (CONFIG_RISCV_64)
> > +typedef unsigned long u64;
> > +typedef u64 vaddr_t;
> > +#define PRIvaddr PRIx64
> > +typedef u64 paddr_t;
> > +#define INVALID_PADDR (~0UL)
> > +#define PRIpaddr "016lx"
> > +typedef u64 register_t;
> > +#define PRIregister "lx"
> > +#endif
> 
> Any chance you could insert blank lines after #if, around #elif, and
> before #endif?
> 
Sure, I will fix that.
> > +#if defined(__SIZE_TYPE__)
> > +typedef __SIZE_TYPE__ size_t;
> > +#else
> > +typedef unsigned long size_t;
> > +#endif
> 
> I'd appreciate if this part was dropped by re-basing on top of my
> "include/types: move stddef.h-kind types to common header" [1], to
> avoid that (re-based) patch then needing to drop this from here
> again. I would have committed this already, if osstest wasn't
> completely broken right now.
> 
I'll take it into account for the next version of the patch series.
> Jan
> 
> [1]
> https://lists.xen.org/archives/html/xen-devel/2023-01/msg00720.html
> (since you would not be able to find a patch of the quoted title,
> as in the submission I mistakenly referenced stdlib.h)
Thanks for the link.

~Oleksii