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

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


  • To: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Alejandro Vallejo <alejandro.vallejo@xxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • From: "Orzel, Michal" <michal.orzel@xxxxxxx>
  • Date: Wed, 19 Mar 2025 15:37:37 +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=V561ibfyU5XpB6hqyTUnsZDQ7bN8IiD5uvJxFDkjtjs=; b=Od+hWrFy7dS4LjwwynTg3mUpI3+LwwOx9rMECGPfVYBVxPJKP8WROdvGjrQuQws/hLkLiBwa181vuyS5hxaiMgKgc5wjfXLHInxsmjkZz7cBBrT6n4gV7zLBNomrit04XbGVYtERR0vZmUMZDnipF0lOuSOYtOucv+bhXZbGHbU/JgKQtFEwjRDPrKflEWnvy7ASBFnUCtDclkzTp4Gw9GIA51LsTIVsj2zCGVHioF2GU0NsxqjLwHB8JK4dgYv10H2AVkLhJkCV2nUWRr/bMj7AL+FkwO0VrZ66b62izuzFuRemeF0vQ6Hdtzi+kSA2CL33TNo3vDKeoNHsja4gTQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=dCBqJ5x4eAH6H/gYThCYexoo5VfLDoStFdHF2ph+hcCOMLRxoQP7Lmt5rg2MIN3qhOjHUXgDPm5nKv7S6nXUqwuYPPxQKmrFXm8fqOVv+NxBHheX76YTBEdPSCZ55xan6yI2S2AReCoArpvXEj3HI3Rr1MvkHzsXRyr3wkJ5vN7sYwBOeUILkh6o6P5CT+OnhdiOIET6GnKoTS2k8LlQc3ltNPef2MtHX8luTRFuRAhoVnTtr08VC8X1cCYpwalvg5M4aQ02FP76MkYHibaDnmT7IdoXIP4VwZ4NAHrrlMer/LMjnv2HZ0VlkMCgezEN3canODS0Q3EUmeIi/GJ2MA==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=amd.com;
  • Cc: Anthony PERARD <anthony.perard@xxxxxxxxxx>, Juergen Gross <jgross@xxxxxxxx>, Luca Fancellu <luca.fancellu@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>
  • Delivery-date: Wed, 19 Mar 2025 14:37:56 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>


On 19/03/2025 15:05, Andrew Cooper wrote:
> 
> 
> On 19/03/2025 2:01 pm, 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.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;
>>>      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?
> 
> You need a build environment with golang for that to happen.  It's easy
> to miss.
> 
> In this case, best to ask the committer to regen.
This patch is already waiting for tools Ab, so maybe @Anthony could do that on
commit?

~Michal




 


Rackspace

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