Hi Roger, > On 13 Oct 2021, at 11:56, Roger Pau Monné <roger....@citrix.com> 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....@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.
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. Regards Bertrand > > Thanks, Roger.
