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

Re: [PATCH v5 07/11] xen/domctl: Introduce XEN_DOMCTL_CDF_vpci flag

  • To: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • From: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>
  • Date: Wed, 13 Oct 2021 12:11:30 +0000
  • Accept-language: en-GB, en-US
  • 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=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; 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=vwMYQ1g5ZE8bVRXL4ibtdUpb6Os9usTBYPvK82x2kCc=; b=KwYdSbBk7q38YzZrwSnscbhuBaj4g80bzwb5b5S9rZWSWxJG+K9OjlgYFCO8OMEb3chgUSFxdq7ZF2iEKi3XPcAE/sHQtLG+/BjltLH0JKMEKCJv2mGobGW3oqVshRdMmfiazf9q0gyaCKs5FMM1QdvLNuohGHunUk/ZlouYmpc27bEYuIBvzNuYJItll3n0OSRrhbQh0j13csQDz78A6cBr0R0dpJ2zY9aBxKXgc3oMFwF2TNYY7YZRyFVuIe+bYZG3ZZomP8mt/47jcps0ji5CQS1GRxFL4JdxefjdsygSApmZ/eV5mDkQYXy2wsG4HHIiAv3IZDHNCAM9DvxpTg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=JeIYqpBQk3WM0z9GML25L6xUei/8zxsX8NNx0oNKrrtW2zaTi6mb9yE3Do2yjq9jxYj1rmnbu7Uy9X+stuRMfAKfo8GSAlDTBegWwqU60EQAjhGNgItiU4YWx7AVCyoSN1oAG7mtobaFKXAPGpn8+HsYptw+oSyAOpcDJNm7IzYqR/6LtDKlJ8J4lg3Yg6GBxqfgC0E/7GVdFLr9Gq6YWUqTK4FZoy1AFZX3FtQiW4s9IWrI0TNcSGDReJlBBWTEuP/dtRssHMM2xFinGVJHY87FhfzaQythPvSAtl11HaTgf7j1D+/8sk72x8juXeGOBAltjvTmc5JFpzBQyC2Biw==
  • Authentication-results-original: citrix.com; dkim=none (message not signed) header.d=none;citrix.com; dmarc=none action=none header.from=arm.com;
  • Cc: Michal Orzel <Michal.Orzel@xxxxxxx>, Rahul Singh <Rahul.Singh@xxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Andre Przywara <Andre.Przywara@xxxxxxx>, Christian Lindig <christian.lindig@xxxxxxxxxx>, David Scott <dave@xxxxxxxxxx>, Ian Jackson <iwj@xxxxxxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
  • Delivery-date: Wed, 13 Oct 2021 12:12:17 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Nodisclaimer: true
  • Original-authentication-results: citrix.com; dkim=none (message not signed) header.d=none;citrix.com; dmarc=none action=none header.from=arm.com;
  • Thread-index: AQHXutmFlM61oLukkkqK7dSQSsVJ1qvNjrkAgAGmMoCAAW6PgIAAEkkAgAAWnQCAABTUgA==
  • Thread-topic: [PATCH v5 07/11] xen/domctl: Introduce XEN_DOMCTL_CDF_vpci flag

Hi Roger,

> On 13 Oct 2021, at 11:56, Roger Pau Monné <roger.pau@xxxxxxxxxx> wrote:
> On Wed, Oct 13, 2021 at 09:36:01AM +0000, Bertrand Marquis wrote:
>> Hi Roger,
>>> On 13 Oct 2021, at 09:30, Roger Pau Monné <roger.pau@xxxxxxxxxx> wrote:
>>> On Tue, Oct 12, 2021 at 12:38:35PM +0200, Michal Orzel wrote:
>>>> Hi Roger,
>>>> On 11.10.2021 11:27, Roger Pau Monné wrote:
>>>>> On Wed, Oct 06, 2021 at 06:40:33PM +0100, Rahul Singh wrote:
>>>>>> Introduce XEN_DOMCTL_CDF_vpci flag to enable VPCI support in XEN.
>>>>>> Reject the use of this new flag for x86 as VPCI is not supported for
>>>>>> DOMU guests for x86.
>>>>> I don't like this approach, XEN_DOMCTL_CDF_vpci should be set for x86
>>>>> PVH dom0, like we do for any other CDF flags when Xen builds dom0.
>>>>> Things like PVH vs PV get translated into CDF flags by create_dom0,
>>>>> and processed normally by the sanitise_domain_config logic, vPCI
>>>>> should be handled that way.
>>>>> Do you think you could see about fixing this?
>>>>> Thanks, Roger.
>>>> I have one question about this fix.
>>>> If I set XEN_DOMCTL_CDF_vpci for dom0 pvh in create_dom0, then in
>>>> sanitise_domain_config or arch_sanitise_domain_config I have no
>>>> knowledge on whether I am dom0 or not. I can check if I'm PVH but not if 
>>>> dom0.
>>>> This would be needed to add a warning if this flag is set but we are not 
>>>> dom0 pvh.
>>>> Any ideas?
>>> I've just realized this is more wrong that I thought. vPCI is
>>> signaled on x86 in xen_arch_domainconfig.emulation_flags, so
>>> introducing a top level option for it without removing the arch
>>> specific one is wrong, as then on x86 we have a duplicated option.
>>> Then I'm also not sure whether we want to move it from
>>> emulation_flags, it seems like the more natural place for it to live
>>> on x86.
>>> If we really want to make vPCI a CDF option we must deal with the
>>> removal of XEN_X86_EMU_VPCI, or else you could introduce an arch
>>> specific flag for vPCI on Arm.
>> First issue that we have here is that there is no emulation_flags right now 
>> on arm.
> You don't explicitly need an emulation_flags field, you could add a
> uint8_t vpci or some such to xen_arch_domainconfig for Arm if you
> don't think there's a need to select more emulation. That's up to Arm
> folks.

One way or an other it is still changing the interface.

>> So I think there are 2 solutions:
>> - introduce PHYSCAP. The problem here is that it is not a physical capacity 
>> and
>> I think that will hit us in the future for example if we want to use vpci 
>> for VIRTIO
>> even if there is not physical PCI on the platform.
> Hm, PHYSCAP is a bit weird, for example Xen signals shadow paging
> support using PHYSCAP which doesn't depend on any hardware feature.
> I think you could signal vPCI regardless of whether the underlying
> platform has PCI devices or not, as vPCI is purely a software
> component.

But in the x86 case this is something not supported in all cases and actually 
only by dom0 pvh.
So having something like that defined as a PHYSCAP is a bit weird.

> Regarding VirtIO, won't it be implemented using ioreqs in user-space,
> and thus won't depend on vPCI?

Yes that’s a solution but you can list them through a PCI interface to let a 
guest discover them.
I am not saying that we will do that but that was an example of case where we 
could use vPCI without hardware PCI present.

>> - introduce an emulation flag on arm. The problem here is that there is no 
>> emulation
>> flag right now on arm so adding this feature will require a change of 
>> interface in
>> arch-arm.h and in xen domctl interface. But if we introduce one on Arm, it 
>> would allow
>> the tools to check if CDF_vpci can be set or not which would solve current 
>> issues.
> I'm not sure I follow. If we go the emulation flags route this will all
> be set in xen_arch_domainconfig, and CDF_vpci will completely go away.

This is a mistake on my side. You are right using emulation flags will require 
to remove CDF_vpci.

>> I think the second solution is the right one but it cannot be done so near 
>> from the
>> feature freeze.
>> The CDF flag as introduced right now is not creating any issue and will be 
>> used once
>> the emulation flag will be introduce. We will be able at this stage to set 
>> this properly
>> also on x86 (for dom0 pvh).
>> Moreover keeping it will allow to continue to merge the remaining part of 
>> the PCI
>> passthrough which are otherwise not possible to be done as they are 
>> dependent on this flag.
>> Can we agree on keep the DOMCTL_CDF_vpci flag and introduce the emulation
>> flag on Arm after 4.16 release ?
> If vPCI for Arm on 4.16 is not going to be functional, why so much
> pressure in pushing those patches so fast? I understand the need to
> remove stuff from the queue, but I don't think it's worth the cost of
> introducing a broken interface deliberately on a release.

We did not push that fast, those have been on the mailing list for a while 
(this is the 5th version of the serie).
PCI passthrough is a big change requiring lots of patches and we decided to 
push it piece by piece to make
the review easier.

> I think we need to at least settle on whether we want to keep
> CDF_vpci or use an arch specific signal mechanism in order to decide
> what to do regarding the release.

Agree and I have no definitive idea on this so but switching will require to 
revert the patch adding CDF_vpci
and change the subsequent patches.


> Thanks, Roger.



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