Re: [Qemu-devel] [PATCH qemu] vfio: Allow configuration without INTx

[Alex Williamson]

On Wed, 22 Nov 2017 16:16:49 +1100
Alexey Kardashevskiy <address@hidden> wrote:

> On some platforms INTx may not be enabled on a KVM host (one such
> example is IBM pHyp hypervisor and this is intentional). However
> the PCI_INTERRUPT_PIN is not 0 so QEMU tries initializing INTx, fails as
> (!vdev->pdev->irq) in the VFIO's vfio_intx_enable() and this is
> a fatal error.
> 
> This adds a debug switch - "x-no-intx" - in order to allow broken INTx
> configuration.
> 
> Signed-off-by: Alexey Kardashevskiy <address@hidden>
> ---
> 
> In practice, test teams run PR KVM under HV KVM and there INTx is enabled
> on all levels so having this as a debug switch is enough.
> ---
>  hw/vfio/pci.h | 1 +
>  hw/vfio/pci.c | 8 +++++++-
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
> index 502a575..c5e168e 100644
> --- a/hw/vfio/pci.h
> +++ b/hw/vfio/pci.h
> @@ -141,6 +141,7 @@ typedef struct VFIOPCIDevice {
>      bool has_flr;
>      bool has_pm_reset;
>      bool rom_read_failed;
> +    bool no_intx;
>      bool no_kvm_intx;
>      bool no_kvm_msi;
>      bool no_kvm_msix;
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index c977ee3..c9caf6a 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -2869,7 +2869,12 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
>          pci_device_set_intx_routing_notifier(&vdev->pdev, vfio_intx_update);
>          ret = vfio_intx_enable(vdev, errp);
>          if (ret) {
> -            goto out_teardown;
> +            if (vdev->no_intx) {
> +                error_report_err(*errp);
> +                *errp = NULL;
> +            } else {
> +                goto out_teardown;
> +            }

Why do we try to setup INTx and let it fail, potentially with errors to
the log, if the user has specified x-no-intx?  Why does the kernel
advertise INTx is available via the pin register if it's not
supported?  I'm not opposed to a debug flag to disable INTx, but by
definition, "x-" prefix flags are not supported and it seems like you
have a configuration that needs a supported mechanism of turning this
off.  Thanks,

Alex

>          }
>      }
>  
> @@ -2986,6 +2991,7 @@ static Property vfio_pci_dev_properties[] = {
>      DEFINE_PROP_BIT("x-igd-opregion", VFIOPCIDevice, features,
>                      VFIO_FEATURE_ENABLE_IGD_OPREGION_BIT, false),
>      DEFINE_PROP_BOOL("x-no-mmap", VFIOPCIDevice, vbasedev.no_mmap, false),
> +    DEFINE_PROP_BOOL("x-no-intx", VFIOPCIDevice, no_intx, false),
>      DEFINE_PROP_BOOL("x-no-kvm-intx", VFIOPCIDevice, no_kvm_intx, false),
>      DEFINE_PROP_BOOL("x-no-kvm-msi", VFIOPCIDevice, no_kvm_msi, false),
>      DEFINE_PROP_BOOL("x-no-kvm-msix", VFIOPCIDevice, no_kvm_msix, false),