> -----Original Message----- > From: Alex Williamson [mailto:alex.williamson@xxxxxxxxxx] > Sent: Thursday, September 21, 2017 10:57 AM > To: Laine Stump <laine@xxxxxxxxx> > Cc: libvirt-list@xxxxxxxxxx; Wuzongyong (Euler Dept) > <cordius.wu@xxxxxxxxxx>; Wanzongshun (Vincent) > <wanzongshun@xxxxxxxxxx> > Subject: Re: Questions about function > virPCIDeviceIsBehindSwitchLackingACS in virpci.c > > On Wed, 20 Sep 2017 21:39:55 -0400 > Laine Stump <laine@xxxxxxxxx> wrote: > > > On 09/20/2017 09:25 PM, Wuzongyong (Euler Dept) wrote: > > >> -----Original Message----- > > >> From: sendmail [mailto:justsendmailnothingelse@xxxxxxxxx] On Behalf > > >> Of Laine Stump > > >> Sent: Thursday, September 21, 2017 8:57 AM > > >> To: libvirt-list@xxxxxxxxxx > > >> Cc: Wuzongyong (Euler Dept) <cordius.wu@xxxxxxxxxx>; > Wanzongshun > > >> (Vincent) <wanzongshun@xxxxxxxxxx>; Alex Williamson > > >> <Alex.Williamson@xxxxxxxxxx> > > >> Subject: Re: Questions about function > > >> virPCIDeviceIsBehindSwitchLackingACS in virpci.c > > >> > > >> On 09/18/2017 09:24 PM, Wuzongyong (Euler Dept) wrote: > > >> Hi, > > >> > > >> In function virPCIDeviceIsBehindSwitchLackingACS, I noticed that(line 8): > > >> > > >> 1 if (virPCIDeviceGetParent(dev, &parent) < 0) > > >> 2 return -1; > > >> 3 if (!parent) { > > >> 4 /* if we have no parent, and this is the root bus, ACS > > >> doesn't come > > >> 5 * into play since devices on the root bus can't P2P > > >> without going > > >> 6 * through the root IOMMU. > > >> 7 */ > > >> 8 if (dev->address.bus == 0) { > > >> 9 return 0; > > >> 10 } else { > > >> 11 virReportError(VIR_ERR_INTERNAL_ERROR, > > >> 12 _("Failed to find parent device for > > >> %s"), > > >> 13 dev->name); > > >> 14 return -1; > > >> 15 } > > >> 16 } > > >> > > >> Why we just return 0 only if device’s bus is 0? > > >> In my server, I can see a root bus which bus number is greater than > > >> 0, see the results(just a part) after I run lspci -t: > > >> > > >> +-[0000:80]-+-02.0-[81-83]--+-00.0 > > >> | | \-00.1 > > >> | +-05.0 > > >> | +-05.1 > > >> | +-05.2 > > >> | \-05.4 > > >> +-[0000:7f]-+-08.0 > > >> | +-08.2 > > >> | +-08.3 > > >> | + . . . > > >> | \-1f.2 > > >> \-[0000:00]-+-00.0 > > >> +-01.0-[01]----00.0 > > >> +-02.0-[02]--+-00.0 > > >> | +-00.1 > > >> | +-00.2 > > >> | \-00.3 > > >> +-02.2-[03]-- > > >> +-03.0-[04-0b]----00.0-[05-0b]--+-08.0-[06-08]----00.0 > > >> | \-10.0-[09-0b]----00.0 > > >> +-05.0 > > >> +-05.1 > > >> +-05.2 > > >> +-05.4 > > >> +-11.0 > > >> +-11.4 > > >> +-16.0 > > >> +-16.1 > > >> +-1a.0 > > >> > > >> If I assign the device 0000:81:00.0 to a VM, I get “Failed to find > > >> parent device”. > > >> I think I should get no error with return value 0 just like bus > > >> number is 0, because bus 80 is the root bus as well in my case. > > >> > > >> In the <<Intel C610 Series Chipset and Intel X99 Chipset Platform > > >> Controller Hub(PCH)>> Datasheet, I found that(Chapter 9.1): > > >> For some server platforms, it may be desirable to have > > >> multiple PCHs in the system > > >> Which means some PCH’s may reside on a bus greater than 0. > > >> > > >> So, is this a bug? > > >> > > >> My memory is that if you're using VFIO for device assignment, all > > >> that checking should be performed by VFIO, and libvirt shouldn't be > > >> checking for ACS at all. (Alex, can you confirm or refute this?) > > Yes, the libvirt ACS checking was never complete and is easily bypassed, vfio > handles this now. > > > >> virPCIDeviceIsBehindSwitchLackingACS() is only called from > > >> virPCIDeviceIsAssignable(), and that function is only called if the > > >> device's stubDriver is set to something other than "vfio-pci" (see > > >> step 1 in virHostdevPreparePCIDevices()). Digging deeper, it looks > > >> like the device's stubDriver is set by > > >> virHostdevGetPCIHostDeviceList(), which appears to set it to > > >> vfio-pci if the backend is specified as vfio (i.e. <driver > > >> name='vfio'/> in the libvirt XML. This *should* be the default > > >> setting!) > > >> > > >> Since I assume you're not using RHEL6, meaning that you will be > > >> using VFIO by default, not legacy KVM assignment. > > >> > > >> TL;DR I think the bug here is that the ...CheckACS function is > > >> being called *at all*. That code path should be completely obsolete. > > > Yeah, I'm using the legacy KVM assignment just for check if > > > virPCIDeviceIsBehindSwitchLackingACS > > > do right thing. > > > So, do you mean legacy KVM assignment code path in libvirt is not > maintained any more? > > > > > > Yes, that is correct. I'm pretty sure it's also not maintained in the > > kernel anymore either. As a matter of fact I had thought that legacy > > KVM device assignment had been disabled in the kernel for a very long > > time (it's disabled downstream in RHEL7). As far as I know, there is > > no reason at all that anyone should be using legacy KVM device > > assignment unless they're on an old 2.6.x kernel (like > > RHEL6/CentOS6).I'm fairly certain nobody has *intentionally* > > used/tested legacy KVM device assignment with anything other than the > > extremely old downstream builds of libvirt used by RHEL6/CentOS6 in a > > very long time. Some day when we decide that we no longer want to > > support building new upstream libvirt on those old systems, we should > remove the code for it. > > > > Legacy KVM device assignment doesn't exist in the kernel anymore, it was > deprecated in 4.2 and was finally removed in 4.12. It's long past time to move > to vfio. I also wouldn't classify legacy KVM device assignment as "supported" > in QEMU either, bugs are not likely to get fixed and before long we'll > probably be looking for an excuse to remove it from that code base as well. > Thanks, > > Alex I see, thank you all. -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list