[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] tools/arm: Fix nr_spis handling v2


  • To: Anthony PERARD <anthony.perard@xxxxxxxxxx>
  • From: "Orzel, Michal" <michal.orzel@xxxxxxx>
  • Date: Mon, 24 Mar 2025 09:54:48 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=amd.com; dmarc=pass action=none header.from=amd.com; dkim=pass header.d=amd.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=LeYnyl5HoKJCGH0AX66P9UbGs4UBU43/q7iCBbJmfMI=; b=vHRVUEKdB7WgKp1O5yoxPZZu5exM3Wdc/8adAexe3zOs6hmS91xcDpwbjWK6cAO668UZkL9gbXBvjV4Dszgc1rY4KwiNooB5d8+iviQ59Z2VX2ICoNW0L+oJqo5de9AceSTEQgEUxcw+qNdz1SOr4igvjhJBGBbIY8SIIYPKogWv0C5oHiLvJlDuAm1/ll3N7niQuYc1f3CopKrv+c1fNO0CcureVQ/eqL1CUxvzqfFwX7mtAkOHxDoo/fCwUfeqY6P8gzHmGUiQycpazwtf6hsh9DR+wDbDJg6GEV2TyM4MFJheaN3bmA47EdDA8dxalBTH1F866hHlGZnw2Np/AA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=IZQMVvDMaDT2qP8yuP/YTde01p00tlLAtLsXs8FKpFwlbDPUoA5nrwBdplC3XC0s89p3FXw4NhoP4iQ1PR1AseD3MulAwUi4zHep42AjTMkdW+tXa2R5aW91dJQhfpH1rSyY9B8KvvMw2bCDtx7eiG69Zrypye5Qps98gP5nrhkKXCdYQfDu9mg4dXsUjE41Om/gSLYT5cjY30gE4wa8Ljz0z+0bBoz0sMjR1rOR48MjT+p3VLPqpwsXQsyRZs676vEoNlQ2ZwOD1Aa3hRTQc/A7GepLvL9B8TYI1nzQkFHFlRZ2WAcPFWW9CL6/uMGfZaLULHPtiidjD/VZOEL7OA==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=amd.com;
  • Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx, Juergen Gross <jgross@xxxxxxxx>, Luca Fancellu <luca.fancellu@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>
  • Delivery-date: Mon, 24 Mar 2025 08:55:08 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>


On 20/03/2025 17:46, Anthony PERARD wrote:
> 
> 
> On Tue, Mar 18, 2025 at 10:00:13AM +0100, 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.fancellu@xxxxxxx>
>> Signed-off-by: Michal Orzel <michal.orzel@xxxxxxx>
>> ---
>>  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;
> 
> Could you try to check only once if there is a user provided value for
> nr_spis?
Sure, but that's one extra variable or more if/else blocks.

> 
>>      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'}),
> 
> Could you introduce something like LIBXL_NR_SPIS_DEFAULT in libxl.h
> instead? (Like we have LIBXL_MAX_GRANT_DEFAULT or LIBXL_MEMKB_DEFAULT)
Sure, no problem.

~Michal




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.