[libvirt] Questions about function virPCIDeviceIsBehindSwitchLackingACS in virpci.c
Wuzongyong (Euler Dept)
cordius.wu at huawei.com
Thu Sep 21 06:34:05 UTC 2017
> -----Original Message-----
> From: Alex Williamson [mailto:alex.williamson at redhat.com]
> Sent: Thursday, September 21, 2017 10:57 AM
> To: Laine Stump <laine at laine.org>
> Cc: libvirt-list at redhat.com; Wuzongyong (Euler Dept)
> <cordius.wu at huawei.com>; Wanzongshun (Vincent)
> <wanzongshun at huawei.com>
> Subject: Re: [libvirt] Questions about function
> virPCIDeviceIsBehindSwitchLackingACS in virpci.c
>
> On Wed, 20 Sep 2017 21:39:55 -0400
> Laine Stump <laine at laine.org> wrote:
>
> > On 09/20/2017 09:25 PM, Wuzongyong (Euler Dept) wrote:
> > >> -----Original Message-----
> > >> From: sendmail [mailto:justsendmailnothingelse at gmail.com] On Behalf
> > >> Of Laine Stump
> > >> Sent: Thursday, September 21, 2017 8:57 AM
> > >> To: libvirt-list at redhat.com
> > >> Cc: Wuzongyong (Euler Dept) <cordius.wu at huawei.com>;
> Wanzongshun
> > >> (Vincent) <wanzongshun at huawei.com>; Alex Williamson
> > >> <Alex.Williamson at redhat.com>
> > >> Subject: Re: [libvirt] 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.
More information about the libvir-list
mailing list