On 19/03/2025 15:01, Alejandro Vallejo wrote:
>
>
> On Tue Mar 18, 2025 at 9:00 AM GMT, Michal Orzel 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 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>
>> ---
>> tools/libs/light/libxl_arm.c | 7 ++++---
>> tools/libs/light/libxl_types.idl | 2 +-
>> 2 files changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
>> index 2d895408cac3..5bb5bac61def 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,14 @@ 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) {
>> + /* We use UINT32_MAX to denote if a user provided a value or not */
>> + if ((cfg_nr_spis != UINT32_MAX) && (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 = max(nr_spis, d_config->b_info.arch_arm.nr_spis);
>> + config->arch.nr_spis = (cfg_nr_spis != UINT32_MAX) ? cfg_nr_spis :
>> 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..129c00ce929c 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':
>> 'UINT32_MAX'}),
>> ])),
>> ("arch_x86", Struct(None, [("msr_relaxed", libxl_defbool),
>> ])),
>
> Doesn't this regenerate the golang bindings?
FYI, it does not. The bindings are already there for NrSpis and default value is
does not result in a change (for verification I checked max_grant_frames that
uses LIBXL_MAX_GRANT_DEFAULT).
~Michal
