| Message ID | 20240524021814.2666257-3-stefano.stabellini@amd.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Remaining patches for dynamic node programming using overlay dtbo | expand |
Hi, On 24/05/2024 03:18, Stefano Stabellini wrote: > From: Henry Wang <xin.wang2@amd.com> > > 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@amd.com> > Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com> > Reviewed-by: Jason Andryuk <jason.andryuk@amd.com> > --- > 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,
On Fri, 23 May 2024, Julien Grall wrote: > Hi, > > On 24/05/2024 03:18, Stefano Stabellini wrote: > > From: Henry Wang <xin.wang2@amd.com> > > > > 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@amd.com> > > Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com> > > Reviewed-by: Jason Andryuk <jason.andryuk@amd.com> > > --- > > 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
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 + /* * libxl memory management * diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c index 1cb89fa584..a4029e3ac8 100644 --- a/tools/libs/light/libxl_arm.c +++ b/tools/libs/light/libxl_arm.c @@ -181,8 +181,8 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc, LOG(DEBUG, "Configure the domain"); - config->arch.nr_spis = nr_spis; - LOG(DEBUG, " - Allocate %u SPIs", nr_spis); + config->arch.nr_spis = max(nr_spis, d_config->b_info.arch_arm.nr_spis); + LOG(DEBUG, " - Allocate %u SPIs", config->arch.nr_spis); switch (d_config->b_info.arch_arm.gic_version) { case LIBXL_GIC_VERSION_DEFAULT: diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl index 79e9c656cc..4e65e6fda5 100644 --- a/tools/libs/light/libxl_types.idl +++ b/tools/libs/light/libxl_types.idl @@ -722,6 +722,7 @@ libxl_domain_build_info = Struct("domain_build_info",[ ("arch_arm", Struct(None, [("gic_version", libxl_gic_version), ("vuart", libxl_vuart_type), ("sve_vl", libxl_sve_type), + ("nr_spis", uint32), ])), ("arch_x86", Struct(None, [("msr_relaxed", libxl_defbool), ])), diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c index c504ab3711..e3a4800f6e 100644 --- a/tools/xl/xl_parse.c +++ b/tools/xl/xl_parse.c @@ -2935,6 +2935,9 @@ skip_usbdev: } } + if (!xlu_cfg_get_long (config, "nr_spis", &l, 0)) + b_info->arch_arm.nr_spis = l; + parse_vkb_list(config, d_config); d_config->virtios = NULL;