Hi, This is labeled as RFC but it's more like a FYI to let people know and comment beforehand. Shiva sent a 28 patch series last year that implements hotplug/unplug support for PCI multifunction devices [1]. The design motivation of his work was based in a RFC sent to this mailing list back in 2016 [2]. I'll briefly summarize the goals and motivations here. What we have today in Libvirt: - no hotplug/unplug support for multifunction PCI devices This is explained in details in [2]. When hotplugging a multifunction device, QEMU will queue the hotplug operation of all non-zero functions and, when function 0 is hotplugged, all functions are hotplugged together. This is true for all archs that supports PCI multifunction devices in QEMU. For unplug it varies: x86 will unplug all functions if any function is unplugged, ppc64 needs to unplug each one.

On Tue, Jun 18, 2019 at 03:04:40PM -0300, Daniel Henrique Barboza wrote: > Hi, > > This is labeled as RFC but it's more like a FYI to let people know and > comment beforehand. Shiva sent a 28 patch series last year that implements > hotplug/unplug support for PCI multifunction devices [1]. The design > motivation of his work was based in a RFC sent to this mailing list back > in 2016 [2]. > > I'll briefly summarize the goals and motivations here. What we have today > in Libvirt: > > - no hotplug/unplug support for multifunction PCI devices > > This is explained in details in [2]. When hotplugging a multifunction > device, QEMU will queue the hotplug operation of all non-zero functions and, > when function 0 is hotplugged, all functions are hotplugged together. This > is true for all archs that supports PCI multifunction devices in QEMU. For > unplug it varies: x86 will unplug all functions if any function is > unplugged, ppc64 needs to unplug each one. Do you know anything about why ppc64 & x86 are different in this respect in QEMU. I think it would be desirable to fix QEMU so that unplug works consistently across architectures. These kind of behavioural differences are a cause of pain as x86 gets all the day to day testing & leaving ppc64 to bitrot if it behaves differently.

Regards, Daniel

On Thu, Aug 22, 2019 at 05:09:10PM -0300, Daniel Henrique Barboza wrote: > Hi Daniel, > > On 6/19/19 4:31 AM, Daniel P. Berrangé wrote: >> On Tue, Jun 18, 2019 at 03:04:40PM -0300, Daniel Henrique Barboza wrote: >> [...] > Finally had the time to look into it in QEMU. The reason for the difference > is indeed architectural - x86 handles it in slot-granularity level and PPC64 > handles it in function level. This is why in x86 unplugging any function > will > remove the whole slot, while in PPC64 each non-zero function needs > to be detached first. > > I've sent a patch to QEMU to change the way PPC64 behaves in function > zero unplug [1]. Instead of bail out claiming that there are non-zero > functions left, QEMU will simply remove the remaining functions by itself. > This behavior happens only with function zero unplug (at least in this first > implementation). Not sure if QEMU will accept it, but if it does, it'll > spare us some patches in Libvirt unplug code for PCI Multifunction. > Let's see how it goes. Looks like the ppc maintainer has queued the patch, which is good.

Regards, Daniel

On 6/18/19 8:04 PM, Daniel Henrique Barboza wrote: > Hi, > > This is labeled as RFC but it's more like a FYI to let people know and > comment beforehand. Shiva sent a 28 patch series last year that > implements > hotplug/unplug support for PCI multifunction devices [1]. The design > motivation of his work was based in a RFC sent to this mailing list back > in 2016 [2]. > > I'll briefly summarize the goals and motivations here. What we have > today > in Libvirt: > > - no hotplug/unplug support for multifunction PCI devices > > This is explained in details in [2]. When hotplugging a multifunction > device, QEMU will queue the hotplug operation of all non-zero > functions and, > when function 0 is hotplugged, all functions are hotplugged together. > This > is true for all archs that supports PCI multifunction devices in > QEMU. For > unplug it varies: x86 will unplug all functions if any function is > unplugged, > ppc64 needs to unplug each one. > > Due to the nature of how Libvirt hotplug works now, hotplug of these > devices > is not possible. All hotplugs are considered in an isolated manner. > Even if > we hotplug each function in the proper order (i.e. leaving function 0 > last), > Libvirt can assign different slots in the guest for each. Similar > problems > happens with hot-unplug. > > This feature aims to address these by creating a new <devices> element, > exclusive for multifunction devices, that aggregates all functions of > a device > in a single operation. To handle this new element, the existing > attach/detach > functions in the QEMU driver now handles multiple devices. > Attaching/detaching > a single device is routed away from the specialized multifunction > code to be > handled to the existing attach/detach code base. > > - no support for partial assignment of functions > > We can't make the assumption that the guest will always assign all > devices > of a multifunction device. Some functions might be a security risk to > expose > to a guest, or the device can behave differently depending on the amount > of functions assigned. > > Even if the 'leftover' functions can't be used to anything else in > the host, > the decision of full/partial assignment of functions should come from > the > user, not us. We can't predict how any other hardware vendor will setup > its devices. > > This patch series also handles this case. > > ------ > > The latest version of this feature, rebase to cdd362e0e7a (the current > master as I'm writing this), can be found at: > > https://github.com/danielhb/libvirt/tree/multifunction-rc2 > > This is a work still ongoing that it's not ready for contribution yet > (first patches that changes the unit test code are breaking existing > tests). I haven't looked at your patches that closely, but I think I might have a fix for you. I mean, I'm working around this area too (see my latest upstream patches) and I've found that our vfio-pci driver has 'bind' action suspended - it fails. This is because RHEL kernel behaves like that (but not the vanilla one). I've spoke to a kernel developer and he experienced the same behaviour, but did not know from the top of his head what is the root cause. Anyway, it's not that big of a deal because libvirt prefers 'driver_override' and thuse it'll never try 'bind' on non-ancient kernels. And my patch does exactly that - it introduces 'driver_override' to virpcimock.c: https://github.com/zippy2/libvirt/commit/2b8ca7d4598ef479b19b61f942a17d35... Actually, there are two more patches needed: https://github.com/zippy2/libvirt/commits/nvme 2b8ca7d459 virpcimock: Create driver_override file in device dirs 9631289730 Revert "virpcitest: Test virPCIDeviceDetach failure" 4310fbbc40 virpcimock: Move actions checking one level up

Michal