Re: [Qemu-devel] [RFC PATCH v4 2/3] VFIO driver for mediated PCI device

[Tian, Kevin]

> From: Kirti Wankhede [mailto:address@hidden
> Sent: Wednesday, May 25, 2016 3:58 AM
> 
> VFIO driver registers with MDEV core driver. MDEV core driver creates
> mediated device and calls probe routine of MPCI VFIO driver. This MPCI
> VFIO driver adds mediated device to VFIO core module.
> Main aim of this module is to manage all VFIO APIs for each mediated PCI
> device.
> Those are:
> - get region information from vendor driver.
> - trap and emulate PCI config space and BAR region.
> - Send interrupt configuration information to vendor driver.
> - mmap mappable region with invalidate mapping and fault on access to
>   remap pfn.
> 
> Signed-off-by: Kirti Wankhede <address@hidden>
> Signed-off-by: Neo Jia <address@hidden>
> Change-Id: I48a34af88a9a905ec1f0f7528383c5db76c2e14d
> ---
>  drivers/vfio/mdev/Kconfig           |   7 +
>  drivers/vfio/mdev/Makefile          |   1 +
>  drivers/vfio/mdev/vfio_mpci.c       | 648
> ++++++++++++++++++++++++++++++++++++
>  drivers/vfio/pci/vfio_pci_private.h |   6 -
>  drivers/vfio/pci/vfio_pci_rdwr.c    |   1 +
>  include/linux/vfio.h                |   7 +
>  6 files changed, 664 insertions(+), 6 deletions(-)
>  create mode 100644 drivers/vfio/mdev/vfio_mpci.c
> 
> diff --git a/drivers/vfio/mdev/Kconfig b/drivers/vfio/mdev/Kconfig
> index 951e2bb06a3f..8d9e78aaa80f 100644
> --- a/drivers/vfio/mdev/Kconfig
> +++ b/drivers/vfio/mdev/Kconfig
> @@ -9,3 +9,10 @@ config MDEV
> 
>          If you don't know what do here, say N.
> 
> +config VFIO_MPCI
> +    tristate "VFIO support for Mediated PCI devices"
> +    depends on VFIO && PCI && MDEV
> +    default n
> +    help
> +        VFIO based driver for mediated PCI devices.
> +
> diff --git a/drivers/vfio/mdev/Makefile b/drivers/vfio/mdev/Makefile
> index 4adb069febce..8ab38c57df21 100644
> --- a/drivers/vfio/mdev/Makefile
> +++ b/drivers/vfio/mdev/Makefile
> @@ -2,4 +2,5 @@
>  mdev-y := mdev-core.o mdev-sysfs.o mdev-driver.o
> 
>  obj-$(CONFIG_MDEV) += mdev.o
> +obj-$(CONFIG_VFIO_MPCI) += vfio_mpci.o
> 
> diff --git a/drivers/vfio/mdev/vfio_mpci.c b/drivers/vfio/mdev/vfio_mpci.c
> new file mode 100644
> index 000000000000..ef9d757ec511
> --- /dev/null
> +++ b/drivers/vfio/mdev/vfio_mpci.c
> @@ -0,0 +1,648 @@
> +/*
> + * VFIO based Mediated PCI device driver
> + *
> + * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved.
> + *     Author: Neo Jia <address@hidden>
> + *          Kirti Wankhede <address@hidden>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/device.h>
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/uuid.h>
> +#include <linux/vfio.h>
> +#include <linux/iommu.h>
> +#include <linux/mdev.h>
> +
> +#include "mdev_private.h"
> +
> +#define DRIVER_VERSION  "0.1"
> +#define DRIVER_AUTHOR   "NVIDIA Corporation"
> +#define DRIVER_DESC     "VFIO based Mediated PCI device driver"
> +
> +struct vfio_mdevice {
> +     struct iommu_group *group;
> +     struct mdev_device *mdevice;
> +     int                 refcnt;
> +     struct pci_region_info vfio_region_info[VFIO_PCI_NUM_REGIONS];
> +     u8                  *vconfig;
> +     struct mutex        vfio_mdev_lock;
> +};
> +
> +static int get_virtual_bar_info(struct mdev_device *mdevice,
> +                             struct pci_region_info *vfio_region_info,
> +                             int index)

'virtual' or 'physical'? My feeling is to get physical region resource allocated
for a mdev.

> +{
> +     int ret = -EINVAL;
> +     struct phy_device *phy_dev = mdevice->phy_dev;
> +
> +     if (dev_is_pci(phy_dev->dev) && phy_dev->ops->get_region_info) {
> +             mutex_lock(&mdevice->ops_lock);
> +             ret = phy_dev->ops->get_region_info(mdevice, index,
> +                                                 vfio_region_info);
> +             mutex_unlock(&mdevice->ops_lock);
> +     }
> +     return ret;
> +}
> +
> +static int mdev_read_base(struct vfio_mdevice *vdev)

similar as earlier comment - vdev or mdev?

> +{
> +     int index, pos;
> +     u32 start_lo, start_hi;
> +     u32 mem_type;
> +
> +     pos = PCI_BASE_ADDRESS_0;
> +
> +     for (index = 0; index <= VFIO_PCI_BAR5_REGION_INDEX; index++) {
> +
> +             if (!vdev->vfio_region_info[index].size)
> +                     continue;
> +
> +             start_lo = (*(u32 *)(vdev->vconfig + pos)) &
> +                                     PCI_BASE_ADDRESS_MEM_MASK;
> +             mem_type = (*(u32 *)(vdev->vconfig + pos)) &
> +                                     PCI_BASE_ADDRESS_MEM_TYPE_MASK;
> +
> +             switch (mem_type) {
> +             case PCI_BASE_ADDRESS_MEM_TYPE_64:
> +                     start_hi = (*(u32 *)(vdev->vconfig + pos + 4));
> +                     pos += 4;
> +                     break;
> +             case PCI_BASE_ADDRESS_MEM_TYPE_32:
> +             case PCI_BASE_ADDRESS_MEM_TYPE_1M:
> +                     /* 1M mem BAR treated as 32-bit BAR */
> +             default:
> +                     /* mem unknown type treated as 32-bit BAR */
> +                     start_hi = 0;
> +                     break;
> +             }
> +             pos += 4;
> +             vdev->vfio_region_info[index].start = ((u64)start_hi << 32) |
> +                                                     start_lo;
> +     }
> +     return 0;
> +}

Thanks
Kevin