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

Re: [PATCH 05/23] xen/arm: Add capabilities to dom0less


  • To: Julien Grall <julien@xxxxxxx>
  • From: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>
  • Date: Mon, 10 Mar 2025 08:01:45 +0000
  • Accept-language: en-GB, en-US
  • Arc-authentication-results: i=2; mx.microsoft.com 1; spf=pass (sender ip is 63.35.35.123) smtp.rcpttodomain=lists.xenproject.org smtp.mailfrom=arm.com; dmarc=pass (p=none sp=none pct=100) action=none header.from=arm.com; dkim=pass (signature was verified) header.d=arm.com; arc=pass (0 oda=1 ltdi=1 spf=[1,1,smtp.mailfrom=arm.com] dkim=[1,1,header.d=arm.com] dmarc=[1,1,header.from=arm.com])
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none
  • Arc-message-signature: i=2; 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=ZOkUF/QJDo4Pm67xoymNLYakN7APMs/mYU9MjFEQ/Q4=; b=siB3BYLwBi/xvTuwuGnWO+zPebBn4fLuK0f2OlJ24GwVUvDKeYDwL+0OOYC1GHiF95c8WGeYNUbDR3MXnx37n+ixH63WczjeVrSnjvy+ahG9D9BdwucLsxykPFDJm1IuhNsqvCKfTOnpAH8ibGsQx/Fj8ca9kr02LPUUYBlQVShQIie1ioO0qyDopH3fyhEHX90RZTn+n1cRtjh/UcD7mN9lnKUgzvtoHWdrpSO+6U2tGOSRGhvISzW+sGZaQV8v9YwK1JTKYjejnex+FwUOBD2RY4oxyloBtnfhbhNREbzKDKYWaP4nDZdsFO5PSDszfzOaZ2b3lz0HswDG3B+odA==
  • 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=ZOkUF/QJDo4Pm67xoymNLYakN7APMs/mYU9MjFEQ/Q4=; b=e+0+x/qYFYPVCafLBdLYk7qUcZpJou8ix3A5zRvUVwKptj2uKBRwVLyjdmVyZnkP9j9TMJR3j1Llsyqq5yv0MTd2MYJHbwWBEmMfGUK6H9NqNrmRKDB8HS03bkK/sB8jJSYLBsEqk9OmMSMpyY15GbHDgD1qNgMiLYHz9Q2sH8Eb0JyPhcU0C81GiWhRf4k1lgGHsbBMf7RAKO78b/SD0WVnfXtAoCQIOiXrKOB41HHFDjYUJE28OLc+yPfo7j46TgFIqQ5OemQeKOgPZKpzNiMLdlcV6wyubAyM8Dk5hWjHMeocEdEc8VKTFmNLt4Qs3i5eNDWPpFsLEp/+Lqrvkg==
  • Arc-seal: i=2; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=pass; b=dYrD8IEPPNKjZhfV7RzuF01JhirGefiXC7VsD2JdRVDY3omXdSXMtHNv8Y4XnhonUYc5ygzU0XWYbfWHoPOyb7cZXqV8gTLQMBRIVgK2Dx3YSRN9yEYS512IHzzz4blmnGG+xu/hsZ/HLCPBCOe/qZzn+FYtUOpu+II6DGQp7+lMs2X1eQl0uiKytKqKqaCgUYBZ0WJ2YTpIvafCEPUXgbFmGfilalUNbMzX5QKcwg1mK+b95iJ1RsbnO8Uv8oNRl9LH8gdDFUnug27g+wNILtlgSSiSPML+AsZB1PVr9ApJpD4xkPJcNP5n4RVq9aAitsyMZnVdkOCnxNXiu4JkvQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=hMPbfE/5gyafAwqDq9Lyov40f2ATnQMCvGRTiqYDPQUI7wlYGjkvzfmy7T74Zl+eziioAr/JHU8LOX+1qDIFW7k7nTcCYRkZFN9/KvKOm6p1iIPGtYve6h7kMSrcfX2VtJkeezJmvNLn6l+0KWS7QgR++Mp/myKJiyknZIsFFzTDf3lDM/vkzGkL3ugjRKKYm4OhdCiOn5UJqPsaQ4KrgZB4L2nEu1K+uriPXr1Wp8QNr4PghIzxMbnVtaS9O2Zqs7XySeqAiu8kLKxefC38x3awPKqK6jXfDC9YAHSp0Be19Uh89Xb6J1evhPtvPh0pTsrLfpwcsHRurX5CmoEaYw==
  • Authentication-results-original: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com;
  • Cc: Jason Andryuk <jason.andryuk@xxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Michal Orzel <michal.orzel@xxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Anthony PERARD <anthony.perard@xxxxxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Delivery-date: Mon, 10 Mar 2025 08:03:26 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Nodisclaimer: true
  • Original-authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com;
  • Thread-index: AQHbjuOsEpKBv2XdREGmb6t5T5YNe7NnYSeAgACWFoCAADjQAIAALPwAgADS4ICAAteYgA==
  • Thread-topic: [PATCH 05/23] xen/arm: Add capabilities to dom0less

Hi,

> On 8 Mar 2025, at 13:37, Julien Grall <julien@xxxxxxx> wrote:
> 
> Hi Jason,
> 
> On 08/03/2025 00:02, Jason Andryuk wrote:
>> On 2025-03-07 16:21, Julien Grall wrote:
>>> Hi Jason,
>>> 
>>> On 07/03/2025 17:58, Jason Andryuk wrote:
>>>> On 2025-03-07 04:01, Julien Grall wrote:
>>>>> Hi,
>>>>> 
>>>>> On 06/03/2025 22:03, Jason Andryuk wrote:
>>>>>> Add capabilities property to dom0less to allow building a
>>>>>> disaggregated system.
>>>>>> 
>>>>>> Introduce bootfdt.h to contain these constants.
>>>>>> 
>>>>>> When using the hardware or xenstore capabilities, adjust the grant and
>>>>>> event channel limits similar to dom0.
>>>>>  > > Also for the hardware domain, set directmap and iommu.  This brings 
>>>>> its
>>>>>> configuration in line with a dom0.
>>>>> 
>>>>> Looking the device tree bindings, a user would be allowed to disable 
>>>>> "passthrough" or even "directmap". This means, we would never be able to 
>>>>> disable "directmap" for the hardware domain in the future with the 
>>>>> existing property (this is to avoid break backwards compatibility).
>>>>> 
>>>>> Instead, I think we should check what the user provided and confirm this 
>>>>> is matching our expectation for an hardware domain.
>>>>  >
>>>>> That said, I am not entirely sure why we should force directmap for the 
>>>>> HW domain. We are starting from a clean slate, so I think it would be 
>>>>> better to have by default no directmap and imposing the presence of an 
>>>>> IOMMU in the system.
>>>> 
>>>> Ok, it seems like directmap is not necessary.  It was helpful early on to 
>>>> get things booting, but I think it's no longer necessary after factoring 
>>>> out construct_hwdom().
>>>> 
>>>> What exactly do you mean by imposing with respect to the iommu? Require 
>>>> one, or mirror the dom0 code and set it for the hwdom?
>>>> 
>>>>      if ( iommu_enabled )
>>>>          dom0_cfg.flags |= XEN_DOMCTL_CDF_iommu;
>>> 
>>> I mean requires one. Without it, you would need to set directmap and I 
>>> don't think this should be allowed (at least for now) for the HW domain.
>>> 
>>>> 
>>>>> Lastly, can you provide an example of what the hardware domain DT node 
>>>>> would looke like?
>>>> 
>>>> I've attached a dump of /sys/firmware/fdt from hwdom.  (This is direct 
>>>> mapped).
>>> 
>>> Sorry if this was not clear, I am asking for the configuration you wrote in 
>>> the host DT for create the hardware domain. I am interested to know which 
>>> properties you set...
>> I've attached the u-boot fdt commands which generate the DT.  Hopefully that 
>> works for you.
>>>> 
>>>>>> --- a/xen/arch/arm/dom0less-build.c
>>>>>> +++ b/xen/arch/arm/dom0less-build.c
>>>>>> @@ -12,6 +12,7 @@
>>>>>>   #include <xen/sizes.h>
>>>>>>   #include <xen/vmap.h>
>>>>>> +#include <public/bootfdt.h>
>>>>>>   #include <public/io/xs_wire.h>
>>>>>>   #include <asm/arm64/sve.h>
>>>>>> @@ -994,6 +995,34 @@ void __init create_domUs(void)
>>>>>>           if ( (max_init_domid + 1) >= DOMID_FIRST_RESERVED )
>>>>>>               panic("No more domain IDs available\n");
>>>>>> +        if ( dt_property_read_u32(node, "capabilities", &val) )
>>>>>> +        {
>>>>>> +            if ( val & ~DOMAIN_CAPS_MASK )
>>>>>> +                panic("Invalid capabilities (%"PRIx32")\n", val);
>>>>>> +
>>>>>> +            if ( val & DOMAIN_CAPS_CONTROL )
>>>>>> +                flags |= CDF_privileged;
>>>>>> +
>>>>>> +            if ( val & DOMAIN_CAPS_HARDWARE )
>>>>>> +            {
>>>>>> +                if ( hardware_domain )
>>>>>> +                    panic("Only 1 hardware domain can be specified! 
>>>>>> (%pd)\n",
>>>>>> +                           hardware_domain);
>>>>>> +
>>>>>> +                d_cfg.max_grant_frames = gnttab_dom0_frames();
>>>>>> +                d_cfg.max_evtchn_port = -1;
>>>>> 
>>>>> What about d_cfg.arch.nr_spis? Are we expecting the user to pass 
>>>>> "nr_spis"?
>>>> 
>>>> Further down, when nr_spis isn't specified in the DT, it defaults to:
>>>>      d_cfg.arch.nr_spis = gic_number_lines() - 32;
>>> 
>>> Thanks. One further question, what if the user pass "nr_spis" for the HW 
>>> domain. Wouldn't this result to more issue further down the line?
>> I'm not familiar with ARM, so I'll to whatever is best.  I did put the 
>> capabilities first, thinking it would set defaults, and then later options 
>> could override them.
> 
> I am not sure it is related to Arm. It is more that the HW domain is going to 
> re-use the memory layout of the host (this is including the mapping for the 
> GIC) and also have all the irqs with pirq == virq.
> 
> I am a bit concerned that letting the users mistakenly tweaking the defaults 
> could prevent attaching devices (for instance if the IRQ ID is above > 
> nr_spis).
> 
>>>> 
>>>> Dom0 does:
>>>>      /*
>>>>       * Xen vGIC supports a maximum of 992 interrupt lines.
>>>>       * 32 are substracted to cover local IRQs.
>>>>       */
>>>>      dom0_cfg.arch.nr_spis = min(gic_number_lines(), (unsigned int) 992) - 
>>>> 32;
>>>>      if ( gic_number_lines() > 992 )
>>>>          printk(XENLOG_WARNING "Maximum number of vGIC IRQs exceeded. \n");
>>>> 
>>>> So I think it's fine as-is?  I could add the min() and warning if you 
>>>> think it's necessary.
>>> 
>>> Regardless this discussion, I wonder why we didn't add the min(...) here 
>>> like for dom0 because we are using the same vGIC emulation. So the max SPIs 
>>> supported is the same...
>>> 
>>> What I am trying to understand is whether it is ok to allow the user to 
>>> select "nr_spis", "vpl011" & co if they are either not honored (like for 
>>> vpl011) or could introduce further issue (like for nr_spis as the HW domain 
>>> is supposed to have the same configuration as dom0).
>>> 
>>> I also don't think it is a good idea to silently ignore what the user 
>>> requested. So far, on Arm, we mainly decided to reject/panic early if the 
>>> setup is not correct.
>> Again, I'll do whatever is best.
> 
> Bertrand, Michal, Stefano, any opinions?

I definitely think that any user configuration mistake should end up in a 
panic, a warning message is definitely not enough.
Here the user might discover or not at runtime that what he thought was 
configured is not.
So a panic here would be better from my point of view.

Cheers
Bertrand 

> 
> Cheers,
> 
> -- 
> Julien Grall





 


Rackspace

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