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....@citrix.com> 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. > 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. Regarding VirtIO, won't it be implemented using ioreqs in user-space, and thus won't depend on vPCI? > - 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. > 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. 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. Thanks, Roger.
