Re: [Qemu-devel] [PATCH qemu] RFC: vfio-pci: Allow mmap of MSIX BAR

[Alexey Kardashevskiy]

On 12/12/17 16:21, Alexey Kardashevskiy wrote:
> This makes use of a new VFIO_REGION_INFO_CAP_MSIX_MAPPABLE capability
> which tells that a region with MSIX data can be mapped entirely, i.e.
> the VFIO PCI driver won't prevent MSIX vectors area from being mapped.
> 
> This adds a "msix-no-mmap" property to the vfio-pci device, it is "true"
> by default and "false" for pseries-2.12+ machines.
> 
> This requites kernel's "vfio-pci: Allow mapping MSIX BAR"
> https://www.spinics.net/lists/kvm/msg160282.html
> 
> Signed-off-by: Alexey Kardashevskiy <address@hidden>
> ---
> 
> This is an RFC as it requires kernel headers update which is not there yet.
> 
> I'd like to make it "msix-mmap" (without "no") but could not find a way
> of enabling a device property for machine versions newer than some value.

Ah, this remark is wrong, making it "no" property does not help.

How do we enforce some property on some device depending on a machine type?



> 
> I changed 2.11 machine just for the demonstration purpose.
> 
> 
> ---
>  hw/vfio/pci.h                 |  1 +
>  include/hw/vfio/vfio-common.h |  1 +
>  linux-headers/linux/vfio.h    |  5 +++++
>  hw/ppc/spapr.c                | 10 +++++++++-
>  hw/vfio/common.c              | 15 +++++++++++++++
>  hw/vfio/pci.c                 | 11 +++++++++++
>  6 files changed, 42 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
> index a8fb3b3..53912ef 100644
> --- a/hw/vfio/pci.h
> +++ b/hw/vfio/pci.h
> @@ -142,6 +142,7 @@ typedef struct VFIOPCIDevice {
>      bool no_kvm_intx;
>      bool no_kvm_msi;
>      bool no_kvm_msix;
> +    bool msix_no_mmap;
>  } VFIOPCIDevice;
>  
>  uint32_t vfio_pci_read_config(PCIDevice *pdev, uint32_t addr, int len);
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index f3a2ac9..927d600 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -171,6 +171,7 @@ int vfio_get_region_info(VFIODevice *vbasedev, int index,
>                           struct vfio_region_info **info);
>  int vfio_get_dev_region_info(VFIODevice *vbasedev, uint32_t type,
>                               uint32_t subtype, struct vfio_region_info 
> **info);
> +bool vfio_is_cap_present(VFIODevice *vbasedev, uint16_t cap_type, int 
> region);
>  #endif
>  extern const MemoryListener vfio_prereg_listener;
>  
> diff --git a/linux-headers/linux/vfio.h b/linux-headers/linux/vfio.h
> index 4e7ab4c..bce9baf 100644
> --- a/linux-headers/linux/vfio.h
> +++ b/linux-headers/linux/vfio.h
> @@ -300,6 +300,11 @@ struct vfio_region_info_cap_type {
>  #define VFIO_REGION_SUBTYPE_INTEL_IGD_HOST_CFG       (2)
>  #define VFIO_REGION_SUBTYPE_INTEL_IGD_LPC_CFG        (3)
>  
> +/*
> + * The MSIX mappable capability informs that MSIX data of a BAR can be 
> mmapped.
> + */
> +#define VFIO_REGION_INFO_CAP_MSIX_MAPPABLE   3
> +
>  /**
>   * VFIO_DEVICE_GET_IRQ_INFO - _IOWR(VFIO_TYPE, VFIO_BASE + 9,
>   *                               struct vfio_irq_info)
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 9de63f0..1dfc386 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3742,13 +3742,21 @@ static const TypeInfo spapr_machine_info = {
>  /*
>   * pseries-2.11
>   */
> +#define SPAPR_COMPAT_2_11                                             \
> +    HW_COMPAT_2_10                                                    \
> +    {                                                                 \
> +        .driver = "vfio-pci",                                         \
> +        .property = "msix-no-mmap",                                   \
> +        .value    = "on",                                             \
> +    },                                                                \
> +
>  static void spapr_machine_2_11_instance_options(MachineState *machine)
>  {
>  }
>  
>  static void spapr_machine_2_11_class_options(MachineClass *mc)
>  {
> -    /* Defaults for the latest behaviour inherited from the base class */
> +    SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_11);
>  }
>  
>  DEFINE_SPAPR_MACHINE(2_11, "2.11", true);
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index ed7717d..593514c 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -1408,6 +1408,21 @@ int vfio_get_dev_region_info(VFIODevice *vbasedev, 
> uint32_t type,
>      return -ENODEV;
>  }
>  
> +bool vfio_is_cap_present(VFIODevice *vbasedev, uint16_t cap_type, int region)
> +{
> +        struct vfio_region_info *info = NULL;
> +        bool ret = false;
> +
> +        if (!vfio_get_region_info(vbasedev, region, &info)) {
> +            if (vfio_get_region_info_cap(info, cap_type)) {
> +                ret = true;
> +            }
> +            g_free(info);
> +        }
> +
> +        return ret;
> +}
> +
>  /*
>   * Interfaces for IBM EEH (Enhanced Error Handling)
>   */
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index c977ee3..d9aeae8 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -1289,6 +1289,12 @@ static void vfio_pci_fixup_msix_region(VFIOPCIDevice 
> *vdev)
>      off_t start, end;
>      VFIORegion *region = &vdev->bars[vdev->msix->table_bar].region;
>  
> +    if (!vdev->msix_no_mmap &&
> +        vfio_is_cap_present(&vdev->vbasedev, 
> VFIO_REGION_INFO_CAP_MSIX_MAPPABLE,
> +                            vdev->msix->table_bar)) {
> +        return;
> +    }
> +
>      /*
>       * We expect to find a single mmap covering the whole BAR, anything else
>       * means it's either unsupported or already setup.
> @@ -1473,6 +1479,10 @@ static int vfio_msix_setup(VFIOPCIDevice *vdev, int 
> pos, Error **errp)
>       */
>      memory_region_set_enabled(&vdev->pdev.msix_pba_mmio, false);
>  
> +    if (!vdev->msix_no_mmap) {
> +        memory_region_set_enabled(&vdev->pdev.msix_table_mmio, false);
> +    }
> +
>      return 0;
>  }
>  
> @@ -2986,6 +2996,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("msix-no-mmap", VFIOPCIDevice, msix_no_mmap, true),
>      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),
> 


-- 
Alexey