Hi Michal,
> On 25 Mar 2025, at 12:00, Michal Orzel <michal.or...@amd.com> wrote:
>
> We are missing a way to detect whether a user provided a value for
> nr_spis equal to 0 or did not provide any value (default is also 0) which
> can cause issues when calculated nr_spis is > 0 and the value from domain
> config is 0. Fix it by setting default value for nr_spis to newly added
> LIBXL_NR_SPIS_DEFAULT i.e. UINT32_MAX (max supported nr of SPIs is 960
> anyway).
>
> Fixes: 55d62b8d4636 ("tools/arm: Reject configuration with incorrect nr_spis
> value")
> Reported-by: Luca Fancellu <luca.fance...@arm.com>
> Signed-off-by: Michal Orzel <michal.or...@amd.com>
Sounds good to me, so for Arm side of things:
Reviewed-by: Bertrand Marquis <bertrand.marq...@arm.com>
Cheers
Bertrand
> ---
> Changes in v2:
> - add LIBXL_NR_SPIS_DEFAULT, rearrange checks
> ---
> tools/include/libxl.h | 1 +
> tools/libs/light/libxl_arm.c | 17 +++++++++++------
> tools/libs/light/libxl_types.idl | 2 +-
> 3 files changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/tools/include/libxl.h b/tools/include/libxl.h
> index f8fe4afd7dca..b7ad7735ca4c 100644
> --- a/tools/include/libxl.h
> +++ b/tools/include/libxl.h
> @@ -1579,6 +1579,7 @@ bool libxl_defbool_val(libxl_defbool db);
>
> const char *libxl_defbool_to_string(libxl_defbool b);
>
> +#define LIBXL_NR_SPIS_DEFAULT (~(uint32_t)0)
> #define LIBXL_TIMER_MODE_DEFAULT -1
> #define LIBXL_MEMKB_DEFAULT ~0ULL
>
> diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
> index 2d895408cac3..0adcaa373b54 100644
> --- a/tools/libs/light/libxl_arm.c
> +++ b/tools/libs/light/libxl_arm.c
> @@ -84,7 +84,7 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
> libxl_domain_config *d_config,
> struct xen_domctl_createdomain *config)
> {
> - uint32_t nr_spis = 0;
> + uint32_t nr_spis = 0, cfg_nr_spis = d_config->b_info.arch_arm.nr_spis;
> unsigned int i;
> uint32_t vuart_irq, virtio_irq = 0;
> bool vuart_enabled = false, virtio_enabled = false;
> @@ -181,13 +181,18 @@ 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 (minimum required
> %u)\n",
> - nr_spis);
> - return ERROR_FAIL;
> + /* Check if a user provided a value or not */
> + if (cfg_nr_spis != LIBXL_NR_SPIS_DEFAULT) {
> + if (nr_spis > cfg_nr_spis) {
> + LOG(ERROR, "Provided nr_spis value is too small (minimum
> required %u)\n",
> + nr_spis);
> + return ERROR_FAIL;
> + }
> + config->arch.nr_spis = cfg_nr_spis;
> }
> + else
> + config->arch.nr_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) {
> diff --git a/tools/libs/light/libxl_types.idl
> b/tools/libs/light/libxl_types.idl
> index bd4b8721ff19..9bb296993199 100644
> --- a/tools/libs/light/libxl_types.idl
> +++ b/tools/libs/light/libxl_types.idl
> @@ -723,7 +723,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),
> + ("nr_spis", uint32, {'init_val':
> 'LIBXL_NR_SPIS_DEFAULT'}),
> ])),
> ("arch_x86", Struct(None, [("msr_relaxed", libxl_defbool),
> ])),
> --
> 2.25.1
>
