Re: [PATCH v5 3/7] tools/arm: Introduce the "nr_spis" xl config entry

[Julien Grall <julien@xxxxxxx>]

Hi,

On 24/05/2024 03:18, Stefano Stabellini wrote:
> From: Henry Wang <xin.wang2@xxxxxxx>
>
> Currently, the number of SPIs allocated to the domain is only
> configurable for Dom0less DomUs. Xen domains are supposed to be
> platform agnostics and therefore the numbers of SPIs for libxl
> guests should not be based on the hardware.
>
> Introduce a new xl config entry for Arm to provide a method for
> user to decide the number of SPIs. This would help to avoid
> bumping the `config->arch.nr_spis` in libxl everytime there is a
> new platform with increased SPI numbers.
>
> Update the doc and the golang bindings accordingly.
>
> Signed-off-by: Henry Wang <xin.wang2@xxxxxxx>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxx>
> Reviewed-by: Jason Andryuk <jason.andryuk@xxxxxxx>
> ---
>   docs/man/xl.cfg.5.pod.in             | 16 ++++++++++++++++
>   tools/golang/xenlight/helpers.gen.go |  2 ++
>   tools/golang/xenlight/types.gen.go   |  1 +
>   tools/include/libxl.h                |  7 +++++++
>   tools/libs/light/libxl_arm.c         |  4 ++--
>   tools/libs/light/libxl_types.idl     |  1 +
>   tools/xl/xl_parse.c                  |  3 +++
>   7 files changed, 32 insertions(+), 2 deletions(-)
>
> diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
> index 8f2b375ce9..ac3f88fd57 100644
> --- a/docs/man/xl.cfg.5.pod.in
> +++ b/docs/man/xl.cfg.5.pod.in
> @@ -3072,6 +3072,22 @@ raised.
>   
>   =back
>   
> +=over 4
> +
> +=item B<nr_spis="NR_SPIS">
> +
> +An optional integer parameter specifying the number of SPIs (Shared
> +Peripheral Interrupts) to allocate for the domain. Max is 991 SPIs. If
> +the value specified by the `nr_spis` parameter is smaller than the
> +number of SPIs calculated by the toolstack based on the devices
> +allocated for the domain, or the `nr_spis` parameter is not specified,
> +the value calculated by the toolstack will be used for the domain.
> +Otherwise, the value specified by the `nr_spis` parameter will be used.
> +The number of SPIs should match the highest interrupt ID that will be
> +assigned to the domain.
> +
> +=back
> +
>   =head3 x86
>   
>   =over 4
> diff --git a/tools/golang/xenlight/helpers.gen.go 
> b/tools/golang/xenlight/helpers.gen.go
> index b9cb5b33c7..fe5110474d 100644
> --- a/tools/golang/xenlight/helpers.gen.go
> +++ b/tools/golang/xenlight/helpers.gen.go
> @@ -1154,6 +1154,7 @@ return fmt.Errorf("invalid union key '%v'", x.Type)}
>   x.ArchArm.GicVersion = GicVersion(xc.arch_arm.gic_version)
>   x.ArchArm.Vuart = VuartType(xc.arch_arm.vuart)
>   x.ArchArm.SveVl = SveType(xc.arch_arm.sve_vl)
> +x.ArchArm.NrSpis = uint32(xc.arch_arm.nr_spis)
>   if err := x.ArchX86.MsrRelaxed.fromC(&xc.arch_x86.msr_relaxed);err != nil {
>   return fmt.Errorf("converting field ArchX86.MsrRelaxed: %v", err)
>   }
> @@ -1670,6 +1671,7 @@ return fmt.Errorf("invalid union key '%v'", x.Type)}
>   xc.arch_arm.gic_version = C.libxl_gic_version(x.ArchArm.GicVersion)
>   xc.arch_arm.vuart = C.libxl_vuart_type(x.ArchArm.Vuart)
>   xc.arch_arm.sve_vl = C.libxl_sve_type(x.ArchArm.SveVl)
> +xc.arch_arm.nr_spis = C.uint32_t(x.ArchArm.NrSpis)
>   if err := x.ArchX86.MsrRelaxed.toC(&xc.arch_x86.msr_relaxed); err != nil {
>   return fmt.Errorf("converting field ArchX86.MsrRelaxed: %v", err)
>   }
> diff --git a/tools/golang/xenlight/types.gen.go 
> b/tools/golang/xenlight/types.gen.go
> index 5b293755d7..c9e45b306f 100644
> --- a/tools/golang/xenlight/types.gen.go
> +++ b/tools/golang/xenlight/types.gen.go
> @@ -597,6 +597,7 @@ ArchArm struct {
>   GicVersion GicVersion
>   Vuart VuartType
>   SveVl SveType
> +NrSpis uint32
>   }
>   ArchX86 struct {
>   MsrRelaxed Defbool
> diff --git a/tools/include/libxl.h b/tools/include/libxl.h
> index 62cb07dea6..3b5c18b48b 100644
> --- a/tools/include/libxl.h
> +++ b/tools/include/libxl.h
> @@ -636,6 +636,13 @@
>    */
>   #define LIBXL_HAVE_XEN_9PFS 1
>   
> +/*
> + * LIBXL_HAVE_NR_SPIS indicates the presence of the nr_spis field in
> + * libxl_domain_build_info that specifies the number of SPIs interrupts
> + * for the guest.
> + */
> +#define LIBXL_HAVE_NR_SPIS 1
> +
Looking at the other arch.arm field, I think this wants to be:

/*
 * libxl_domain_build_info has the arch_arm.nr_spis field
 */
#define LIBXL_HAVE_BUILDINFO_ARCH_NR_SPIS 1

This would also clarify that the field is Arm specific.

Cheers,

--
Julien Grall