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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 23 May 2024, Julien Grall wrote:
> 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.

I made the change