Messages in this thread | | | Date | Mon, 18 Sep 2023 20:32:19 -0300 | From | Jason Gunthorpe <> | Subject | Re: [PATCH vfio] vfio/pci: remove msi domain on msi disable |
| |
On Mon, Sep 18, 2023 at 10:48:54AM -0700, Nelson, Shannon wrote:
> In our case, the VF device's msix count value found in PCI config space is > changed by device configuration management outside of the baremetal host and > read by the QEMU instance when it starts up, and then read by the vfio PCI > core when QEMU requests the first IRQ.
Oh, you definitely can't do that!
PCI config space is not allowed to change outside the OS's view and we added sriov_set_msix_vec_count() specifically as a way to provide the necessary synchronization between all the parts.
Randomly changing, what should be immutable, parts of the config space from under a running OS is just non-compliant PCI behavior.
> The msix vectors are freed, but the msi_domain is not, and the msi_domain > holds the MSIx count that it read when it was created. If the device's MSIx > count is increased, the next QEMU session will see the new number in PCI > config space and try to use that new larger number, but the msi_domain is > still using the smaller hwsize and the QEMU IRQ setup fails in > msi_insert_desc().
Correct, devices are not allowed to change these parameters autonomously, so there is no reason to accommodate this.
> This patch adds a msi_remove_device_irq_domain() call when the irqs are > disabled in order to force a new read on the next IRQ allocation cycle. This > is limited to only the vfio use of the msi_domain.
Definately no.
> I suppose we could add this to the trailing end of callbacks in our own > driver, but this looks more like a generic vfio/msi issue than a driver > specific thing.
Certainly not.
> The other possibility is to force the user to always do a bind cycle between > QEMU sessions using the VF. This seems to be unnecessary overhead and was > not necessary when using the v6.1 kernel. To the user, this looks like a > regression - this is how it was reported to me.
You need to use sriov_set_msix_vec_count() and only sriov_set_msix_vec_count() to change this parameter or I expect you will constantly experience problems.
Jason
| |