Hi C.,
Thanks very much for reviewing the patches.
> On July 28, 2023 12:58 AM, Cédric Le Goater <c...@redhat.com> wrote:
>
> Hello Jing,
>
> On 7/27/23 09:24, Jing Liu wrote:
> > From: Reinette Chatre <reinette.cha...@intel.com>
> >
> > Kernel provides the guidance of dynamic MSI-X allocation support of
> > passthrough device, by clearing the VFIO_IRQ_INFO_NORESIZE flag to
> > guide user space.
> >
> > Fetch and store the flags from host for later use to determine if
> > specific flags are set.
> >
> > Signed-off-by: Reinette Chatre <reinette.cha...@intel.com>
> > Signed-off-by: Jing Liu <jing2....@intel.com>
> > ---
> > hw/vfio/pci.c | 12 ++++++++++++
> > hw/vfio/pci.h | 1 +
> > hw/vfio/trace-events | 2 ++
> > 3 files changed, 15 insertions(+)
> >
> > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index
> > a205c6b1130f..0c4ac0873d40 100644
> > --- a/hw/vfio/pci.c
> > +++ b/hw/vfio/pci.c
> > @@ -1572,6 +1572,7 @@ static void vfio_msix_early_setup(VFIOPCIDevice
> > *vdev, Error **errp)
> >
> > static int vfio_msix_setup(VFIOPCIDevice *vdev, int pos, Error **errp)
> > {
> > + struct vfio_irq_info irq_info = { .argsz = sizeof(irq_info) };
> > int ret;
> > Error *err = NULL;
> >
> > @@ -1624,6 +1625,17 @@ static int vfio_msix_setup(VFIOPCIDevice *vdev, int
> pos, Error **errp)
> > memory_region_set_enabled(&vdev->pdev.msix_table_mmio, false);
> > }
> >
> > + irq_info.index = VFIO_PCI_MSIX_IRQ_INDEX;
> > + ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_GET_IRQ_INFO, &irq_info);
> > + if (ret) {
> > + /* This can fail for an old kernel or legacy PCI dev */
> > + trace_vfio_msix_setup_get_irq_info_failure(strerror(errno));
>
> Is it possible to detect the error reported by a kernel (< 6.4) when dynamic
> MSI-X
> are not supported.
I just realized that ioctl(VFIO_DEVICE_GET_IRQ_INFO) with
VFIO_PCI_MSIX_IRQ_INDEX
should always exists and would not cause failure for older kernel reason, after
reading
Alex's comment on this patch. (If my understanding is correct)
So the possible failure might be other reason except old kernel or legacy PCI
dev.
Looking at vfio_pci_ioctl_get_irq_info() in the kernel, could
> info.flags be tested against VFIO_IRQ_INFO_NORESIZE ?
>
Sorry I didn't quite understand "info.flags be tested against
VFIO_IRQ_INFO_NORESIZE".
I saw kernel < 6.4 simply added NORESIZE to info.flags and latest kernel adds
if has_dyn_msix.
Would you please kindly describe more on your point?
> In that case, QEMU should report an error and the trace event is not needed.
I replied an email with new error handling draft code based on my
understanding, which
reports the error and need no trace. Could you please help review if that is
what we want?
Thanks,
Jing
>
> Thanks,
>
> C.
>
> > + } else {
> > + vdev->msix->irq_info_flags = irq_info.flags;
> > + }
> > + trace_vfio_msix_setup_irq_info_flags(vdev->vbasedev.name,
> > + vdev->msix->irq_info_flags);
> > +
> > return 0;
> > }
> >
> > diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h index
> > a2771b9ff3cc..ad34ec56d0ae 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;
> > + uint32_t irq_info_flags;
> > } VFIOMSIXInfo;
> >
> > #define TYPE_VFIO_PCI "vfio-pci"
> > diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events index
> > ee7509e68e4f..7d4a398f044d 100644
> > --- a/hw/vfio/trace-events
> > +++ b/hw/vfio/trace-events
> > @@ -28,6 +28,8 @@ vfio_pci_read_config(const char *name, int addr, int len,
> int val) " (%s, @0x%x,
> > vfio_pci_write_config(const char *name, int addr, int val, int len) " (%s,
> @0x%x, 0x%x, len=0x%x)"
> > vfio_msi_setup(const char *name, int pos) "%s PCI MSI CAP @0x%x"
> > vfio_msix_early_setup(const char *name, int pos, int table_bar, int
> > offset, int
> entries) "%s PCI MSI-X CAP @0x%x, BAR %d, offset 0x%x, entries %d"
> > +vfio_msix_setup_get_irq_info_failure(const char *errstr)
> "VFIO_DEVICE_GET_IRQ_INFO failure: %s"
> > +vfio_msix_setup_irq_info_flags(const char *name, uint32_t flags) " (%s)
> > MSI-X
> irq info flags 0x%x"
> > vfio_check_pcie_flr(const char *name) "%s Supports FLR via PCIe cap"
> > vfio_check_pm_reset(const char *name) "%s Supports PM reset"
> > vfio_check_af_flr(const char *name) "%s Supports FLR via AF cap"
