| Message ID | 20250311090409.122577-3-michal.orzel@amd.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | arm: better handling of nr_spis | expand |
Hi Michal, > On 11 Mar 2025, at 10:04, Michal Orzel <michal.orzel@amd.com> wrote: > > If the calculated value for nr_spis by the toolstack is bigger than the > value provided by the user, we silently ignore the latter. This is not > consistent with the approach we have in Xen on Arm when we try to reject > incorrect configuration. Also, the documentation for nr_spis is > incorrect as it mentions 991 as the number of max SPIs, where it should > be 960 i.e. (1020 - 32) rounded down to the nearest multiple of 32. > > Signed-off-by: Michal Orzel <michal.orzel@amd.com> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com> With a small NIT... > --- > docs/man/xl.cfg.5.pod.in | 13 +++++-------- > tools/libs/light/libxl_arm.c | 6 ++++++ > 2 files changed, 11 insertions(+), 8 deletions(-) > > diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in > index 8e1422104e50..7339c44efd54 100644 > --- a/docs/man/xl.cfg.5.pod.in > +++ b/docs/man/xl.cfg.5.pod.in > @@ -3083,14 +3083,11 @@ interval of colors (such as "0-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. > +Peripheral Interrupts) to allocate for the domain. Max is 960 SPIs. If > +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 > > diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c > index 5a9db5e85f6f..ee9154298f2a 100644 > --- a/tools/libs/light/libxl_arm.c > +++ b/tools/libs/light/libxl_arm.c > @@ -181,6 +181,12 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc, > > LOG(DEBUG, "Configure the domain"); > > + if (nr_spis > d_config->b_info.arch_arm.nr_spis) { > + LOG(ERROR, "Provided nr_spis value is too small (required %u)\n", I would say "minimum required" here. Cheers Bertrand > + nr_spis); > + return ERROR_FAIL; > + } > + > config->arch.nr_spis = max(nr_spis, d_config->b_info.arch_arm.nr_spis); > LOG(DEBUG, " - Allocate %u SPIs", config->arch.nr_spis); > > -- > 2.25.1 >
diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in index 8e1422104e50..7339c44efd54 100644 --- a/docs/man/xl.cfg.5.pod.in +++ b/docs/man/xl.cfg.5.pod.in @@ -3083,14 +3083,11 @@ interval of colors (such as "0-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. +Peripheral Interrupts) to allocate for the domain. Max is 960 SPIs. If +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 diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c index 5a9db5e85f6f..ee9154298f2a 100644 --- a/tools/libs/light/libxl_arm.c +++ b/tools/libs/light/libxl_arm.c @@ -181,6 +181,12 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc, LOG(DEBUG, "Configure the domain"); + if (nr_spis > d_config->b_info.arch_arm.nr_spis) { + LOG(ERROR, "Provided nr_spis value is too small (required %u)\n", + nr_spis); + return ERROR_FAIL; + } + config->arch.nr_spis = max(nr_spis, d_config->b_info.arch_arm.nr_spis); LOG(DEBUG, " - Allocate %u SPIs", config->arch.nr_spis);
If the calculated value for nr_spis by the toolstack is bigger than the value provided by the user, we silently ignore the latter. This is not consistent with the approach we have in Xen on Arm when we try to reject incorrect configuration. Also, the documentation for nr_spis is incorrect as it mentions 991 as the number of max SPIs, where it should be 960 i.e. (1020 - 32) rounded down to the nearest multiple of 32. Signed-off-by: Michal Orzel <michal.orzel@amd.com> --- docs/man/xl.cfg.5.pod.in | 13 +++++-------- tools/libs/light/libxl_arm.c | 6 ++++++ 2 files changed, 11 insertions(+), 8 deletions(-)