Re: [PATCH v5 3/7] tools/arm: Introduce the "nr_spis" xl config entry
On Fri, 23 May 2024, Julien Grall wrote:
> Hi,
>
> On 24/05/2024 03:18, Stefano Stabellini wrote:
> > From: Henry Wang
> >
> > 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
> > Signed-off-by: Stefano Stabellini
> > Reviewed-by: Jason Andryuk
> > ---
> > 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
> > +
> > +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
Re: [PATCH v5 3/7] tools/arm: Introduce the "nr_spis" xl config entry
Hi,
On 24/05/2024 03:18, Stefano Stabellini wrote:
From: Henry Wang
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
Signed-off-by: Stefano Stabellini
Reviewed-by: Jason Andryuk
---
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
+
+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
[PATCH v5 3/7] tools/arm: Introduce the "nr_spis" xl config entry
From: Henry Wang
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
Signed-off-by: Stefano Stabellini
Reviewed-by: Jason Andryuk
---
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
+
+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),
