[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: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Wed, 13 Oct 2021 16:28:49 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.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=u3lueVMk3OR7O4L8bxtNPd6WkVa3YP6XSnKGi/uvWbo=; b=JVaqFuoZkgak+kQvj5bNA9n7C7cO5qGTq+efy9LG/tVbdKPDdkNndijrlrG1JWv070S7QeKGfqUoDeWjZGgyVmk2CT3wZooLOOtgJxMR9WfyFi7ioDpXQrXO+0DcpzbHpDyG+4p0cgGYwWsXuqAbgG6jLdjidJzRSRVHZmNOMW6DzUHHrlrLb8FIOyXD3Q9A7nhue8FX8qV/IBsssdK9GO498Mn6H0Gfe+k5rCTMCY+fJXvfIsD4tDqlt38gDz03e/NzvZeBhXVQcEaBya6wxj1QzWNDGmfxGbewUawDXIFYkger5StFtMXWv5SL7Ip2EiyAVmM51Gseuk0hc6VKsw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Bv3Opd7tNsgM1g3kNhLqHxsUKTcycMer0Lb2ZmZWD9t/fC6nmQ8Kb9mxDxVhayoaoI6cft71Jobj4fAi4Jp90cxKhtqBolCDFd93W3nu0+tavMOY1JeKSitHdjy/TrYaLfK1g0SBhpVjHVRuC8R+i5/6q/D5bvnToQ4+uNF+U0xohVpWMpBmRDOmHYHJLPTJ5Ng8FKo3jJ54itdWbU1md95UJEzJEWrZs7Pll4VULloVzj5icbiQUUqANaRJH34DzjHtBtJ1oGhbHDM0q6NyN0h9S4sSaeHa09FYAIfYTmTRGmTEXjyvuV697RbOCW0bzPCPNSZQdXEcXRA78FlSxA==
  • Authentication-results: esa4.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.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 14:29:24 +0000
  • Ironport-data: A9a23:LCGMLqin/kHsWBewSXj0WE5kX161QBYKZh0ujC45NGQN5FlHY01je htvC2/XbKuKZTHzf4hxbNvloRtQvZPQyIIyG1ZrrH8xH38b9cadCdqndUqhZCn6wu8v7a5EA 2fyTvGacajYm1eF/k/F3oAMKRCQ7InQLlbGILes1htZGEk0FU/NtTo5w7Rg29Yx2oDia++wk YiaT/P3aQfNNwFcagr424rbwP+4lK2v0N+wlgVWicFj5DcypVFMZH4sDfjZw0/DaptVBoaHq 9Prl9lVyI97EyAFUbtJmp6jGqEDryW70QKm0hK6UID66vROS7BbPg/W+5PwZG8O4whlkeydx /1x743oFltuOJaP28QXCUB1GCB3L59/reqvzXiX6aR/zmXDenrohf5vEFs3LcsT/eMf7WNmr KJCbmpXN1ba2rzwkOnTpupE36zPKOHxO4wSoDd4xCzxBvc6W5HTBa7N4Le02R9t2JERQ6uEN 6L1bxJANivtezhBAW5ON4syg+WZg2n/biRh/Qf9Sa0fvDGIkV0ZPKLWGNjfd8GORM5Vtl2Fv W+A9GP8ajkFMPSPxDzD9Wij7sfOgiHTSI8UDKe/9PNhnBuU3GN7ICMRUVy3sPyokHmUUthUK 1EX0ic2pK10/0uuJvHmRAGxqnOAuh8aWvJTHvc85QXLzbDbiy6bG2wFQzhpeNEg8sgsSlQC7 FaJgtevPj1pv729QGiYsLyTqFuaHCkeLWYGIwgeXwYBy9D5pcc4iRenZvxuCrKvh9v5XxT52 SmXrTMWjq8Wy8UM0s2T8VnZjhq2q5POTwpz4R/YNkqM6A9jacidfZ659lHB5N5JNoPfRV6E1 FAUls7b4O0QAJWlkC2WXP5LDLyv/+yCMjDXnRhoBZZJyti20yf9J8YKumg4fRo3dJZfEdP0X KPNkT8L9ccMYDysVuw0Q4jrUukhzKHyJ+2wA5g4ceFySpR2cQaG+gRnakiRw33hnSAQrE0vB XuIWZ3zVShCWMyL2BLzHr1HieZ6mUjS0EuKHcijpylLx4Zyc5J8pV0tC1CJcvwipJ2NpAHY4 r6z3OPblk0BDoUSjsTRmLP/zGzmz1BnW/gaSOQNL4ZvxzaK/kl6VJc9Jpt7KuRYc1x9zLugw 51EchYwJKDDrXPGMx6WTXtodaniW51yxVpiY3dwZQj0gSB5Pd3zhEv6S3fRVeJ4nACE5aQlJ 8Tphu3aWqgfItg502V1gWbBQHxKK03w2FPm09uNazkjZZ9wLzElCfe/FjYDABImV3Lt3eNn+ uXI/lqCHfIrGlQzZO6LOanH5w7g4hAgdBdaAhKgzi97Ix63ruCH6kXZ05cKHi37AUybnGXFj FzKXU5wSCuki9ZdzeQlTJus9u+BO+B/AlBbDy/c67O3PjPd5W2t3clLV+PgQNwXfDikkEl7T ekKnfz6LtMdm1NG79h1H7pxlPps7Nrzvb5KiA9jGSyTPVisD7phJFiA3NVO6fIRluMI51PuV xLd4MReNJWIJNjhTAwbKj06Y7nRzvoTgDTTs6g4eR2o+C9t8bObekxOJB3Q2jdFJb54Pdp9k +csscIb8SKljR8uPorUhyxY7T3UfHcBT78mptcRB4qy0lgnzVRLYJr9DC7q4c7QN4UQYxdye jLN3fjMnbVRwEbGYkEfL3mV0LoPn4kKtTBL0EQGewaDlO3ai6JlxxZW6zk2EFhYl00Vz+JpN 2F3HERpPqHSrSxwjc1OUm3wSQFMABqVph74x1cTzTCLSkCpUirGLXEnOPbL90ccqjoOcj9e9 bCe6WDkTTe1I52hgnpsARZo+675UNh81gzeg8T2Tc2KEq4zbSfhnqLzN3EDrAHqAJ9piUDKz QWwED2ctUEv2fYsnpAG
  • Ironport-hdrordr: A9a23:MA/UdqiqutDLi9w5/eOypzO80XBQX0t13DAbv31ZSRFFG/FwyP rAoB1L73PJYWgqNU3I+ergBEGBKUmskqKdxbNhR4tKOzOWxVdATbsSlrcKpgePJ8SQzJ8+6U 4NSdkaNDS0NykHsS+Y2njILz9D+qj/zEnAv463pB0MPGIaG52IrT0JcjpzencGOjWubqBJcq Z0iPA3wwZJLh8sH7uG7zQ+LqX+juyOsKijTQ8NBhYh5gXLpTS06ITiGxzd+hsFSTtAzZor7G CAymXCl+qemsD+7iWZ+37Y7pxQltek4txfBPaUgsxQDjn3kA6naKloRrXHljEop+OE7kosjb D30lsdFvU2z0mUUnC+oBPr1QWl+DEy60X6wVvdunfnqdyRfkNzN+NxwaZiNjfJ4Uspu99xlI hR2XiCipZRBRTc2Azg+tnhTXhR5wqJiEtntdRWo21UUIMYZrMUh5cY5llpHJAJGz+/wJw7Ed NpENrX6J9tABKnhkjizytSKeGXLzEO9k/seDlHhiXV6UkZoJlB9Tpa+CRF9U1ws67USPF/lq 352+pT5fdzpmJ/V9MIOA47e7rENoX6e2O7DIujGyWVKEg5AQO5l3fW2sR/2Aj4Qu1D8HMN8K 6xJ2+w81RCIn7TNQ==
  • Ironport-sdr: hjuuT9gSa05fYbgWkD3sZ4Use2JKatNkzPCs5Cs6TZZtqyT3hSi+ETg/5S+Xjkx1oEGDIey/WU Un/SzQV88MrWFYUEC35HnEMGnEql4h4W+ej3/ytbQgXU7NA36EFtYV4Masa2iAToJ1T4Fs+s7N ayXEZlcMhoq29MHYbLQlFJtx2pbzHijghPQu9O9i5JnR/w8gClsZZ+PPDlDUrR900zaZyiEDEW P8ERgmVlVWSz9uqWb6XoiJ/5+lcqeV18sotGjdlvgZVD06/n9DF16OBMPHAi0OV7Fuum06TBuU rH1LeGVBieKIlEh47EkfVylA
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Wed, Oct 13, 2021 at 12:11:30PM +0000, Bertrand Marquis wrote:
> 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.

Well, I don't want to sound rude, but you already changed the
interface first and very recently by introducing CDF_vpci. Complaining
that you don't want to change it anymore just after that seems
slightly weird.

> > 
> >> 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.

Well, even if vPCI was fully supported on x86 for both domU and dom0
it would only apply to PVH and maybe HVM guests, but classic PV will
never get vPCI. It's kind of trying to create a PV guest with HAP
support.

One option would be to defer the introduction of the CDF_vpci flag
until the vpci work has been finished - then we could decide if the
current code can also be used by x86 so maybe it could be enabled for
both arches, and there would be no problem in setting the PHYSCAP.

> >> 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.

First version of this patch I have on my inbox is from 23rd of
September, that's less than 3 weeks ago.

> > 
> > 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.

I think it's fair to expect changes to the interface when things are
reviewed, that's part of the review process, changes to subsequent
patches can be painful, but we have all been there and dealt with
them.

I'm not saying we must remove the CDF_vpci flag. I have to admit I was
fine using emulation_flags, but I could also be fine using CDF_vpci if
the x86 side is fixed and XEN_X86_EMU_VPCI is removed.

Thanks, Roger.



 


Rackspace

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