Re: Questions about function virPCIDeviceIsBehindSwitchLackingACS in virpci.c

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux