[libvirt] Questions about function virPCIDeviceIsBehindSwitchLackingACS in virpci.c

[Laine Stump]

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

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.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170920/d25c3753/attachment-0001.htm>