> On Feb 8, 2023, at 1:30 PM, Alex Williamson <alex.william...@redhat.com> > wrote: > > On Wed, 8 Feb 2023 06:38:30 +0000 > John Johnson <john.g.john...@oracle.com> wrote: > >>> On Feb 6, 2023, at 12:33 PM, Alex Williamson <alex.william...@redhat.com> >>> wrote: >>> >>> On Wed, 1 Feb 2023 21:55:51 -0800 >>> John Johnson <john.g.john...@oracle.com> wrote: >>> >>>> Server holds device current device pending state >>>> Use irq masking commands in socket case >>>> >>>> Signed-off-by: John G Johnson <john.g.john...@oracle.com> >>>> Signed-off-by: Elena Ufimtseva <elena.ufimts...@oracle.com> >>>> Signed-off-by: Jagannathan Raman <jag.ra...@oracle.com> >>>> --- >>>> hw/vfio/pci.h | 1 + >>>> include/hw/vfio/vfio-common.h | 3 ++ >>>> hw/vfio/ccw.c | 1 + >>>> hw/vfio/common.c | 26 ++++++++++++++++++ >>>> hw/vfio/pci.c | 23 +++++++++++++++- >>>> hw/vfio/platform.c | 1 + >>>> hw/vfio/user-pci.c | 64 >>>> +++++++++++++++++++++++++++++++++++++++++++ >>>> 7 files changed, 118 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h >>>> index 4f70664..d3e5d5f 100644 >>>> --- a/hw/vfio/pci.h >>>> +++ b/hw/vfio/pci.h >>>> @@ -113,6 +113,7 @@ typedef struct VFIOMSIXInfo { >>>> uint32_t table_offset; >>>> uint32_t pba_offset; >>>> unsigned long *pending; >>>> + MemoryRegion *pba_region; >>>> } VFIOMSIXInfo; >>>> >>>> /* >>>> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h >>>> index bbc4b15..2c58d7d 100644 >>>> --- a/include/hw/vfio/vfio-common.h >>>> +++ b/include/hw/vfio/vfio-common.h >>>> @@ -143,6 +143,7 @@ typedef struct VFIODevice { >>>> bool ram_block_discard_allowed; >>>> bool enable_migration; >>>> bool use_regfds; >>>> + bool can_mask_irq; >>> >>> How can this be a device level property? vfio-pci (kernel) supports >>> masking of INTx. It seems like there needs to be a per-index info or >>> probe here to to support this. >>> >> >> It is only used for MSIX masking. MSI masking is done with >> config space stores, and vfio-kernel and vfio-user handle them the >> same by forwarding the config space writes to the server so it can >> mask interrupts. > > I suppose this does slip through on vfio-kernel, though support via > SET_IRQS would really be the uAPI mechanism we'd expect to use for > masking, just as it is for enabling/disabling MSI. MSI is not well > used enough to have flushed that out and it seems harmless, but is not > the intended API. > >> MSIX is trickier because the mask bits are in a memory region. >> vfio-kernel doesn’t support SET_IRQS on MSIX vectors, so if the host >> doesn’t allow client mapping of the MSIX table to do the masking, vfio >> switches a masked vector’s event FD destination from KVM to QEMU, then >> uses the QEMU PCI emulation to mask the vector. > > Lack of support for MSIX ACTION_[UN]MASK is not an API limitation, it's > an implementation issue in the kernel. Same for MSI. I believe this > is resolved now, that there are mask/unmask APIs available within the > kernel, as well as mechanisms to avoid the NORESIZE behavior now, so > the intention would be to implement that support, along with a > mechanism for the user to know the support is present. We already have > that for NORESIZE via IRQ_INFO, so I suspect the right answer might be > to add a new VFIO_IRQ_INFO_MSI_MASK to advertise masking support. >
I looked at the linux next vfio kernel driver, and it doesn't seem to support MSI/X masking. >> vfio-user has to use messages to mask MSIX vectors, since there >> is no host config space to map. I originally forwarded the MSIX table >> writes to the server to do the masking, but the feedback on the vfio-user >> server changes was to add SET_IRQS support. > > Which is what I'm describing above, QEMU should know via the VFIO uAPI > whether MSI/X masking is supported and use that to configure the code > to make use of it rather than some inferred value based on the > interface type. Thanks, > The kernel doesn’t advertising it working either, so I think I can key it off the presence of VFIO_IRQ_INFO_MASKABLE in irq_info JJ