Hi Jean-Philippe,
One comment below.
Jean-Philippe Brucker <jean-philippe.brucker@xxxxxxx> writes:
> Assigning devices using VFIO allows the guest to have direct access to the
> device, whilst filtering accesses to sensitive areas by trapping config
> space accesses and mapping DMA with an IOMMU.
>
> This patch adds a new option to lkvm run: --vfio-pci=<BDF>. Before
> assigning a device to a VM, some preparation is required. As described in
> Linux Documentation/vfio.txt, the device driver needs to be changed to
> vfio-pci:
>
> $ dev=0000:00:00.0
>
> $ echo $dev > /sys/bus/pci/devices/$dev/driver/unbind
> $ echo vfio-pci > /sys/bus/pci/devices/$dev/driver_override
> $ echo $dev > /sys/bus/pci/drivers_probe
>
> Adding --vfio-pci=$dev to lkvm-run will pass the device to the guest.
> Multiple devices can be passed to the guest by adding more --vfio-pci
> parameters.
>
> This patch only implements PCI with INTx. MSI-X routing will be added in a
> subsequent patch, and at some point we might add support for passing
> platform devices to guests.
>
> Signed-off-by: Will Deacon <will.deacon@xxxxxxx>
> Signed-off-by: Robin Murphy <robin.murphy@xxxxxxx>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@xxxxxxx>
>
> ---
> Changes v3->v4
> * Pass individual devices on the command-line instead of the whole group
> * Handle 64-bit BARs
> * Remove VFIO_TYPE1_NESTED which isn't supported by x86 IOMMUs and
> cannot be probed with VFIO_CHECK_EXTENSION (we'll have to try
> VFIO_SET_IOMMU instead).
> ---
> Makefile | 2 +
> arm/pci.c | 1 +
> builtin-run.c | 5 +
> include/kvm/kvm-config.h | 3 +
> include/kvm/pci.h | 3 +-
> include/kvm/vfio.h | 71 +++++++
> vfio/core.c | 488 +++++++++++++++++++++++++++++++++++++++++++++++
> vfio/pci.c | 395 ++++++++++++++++++++++++++++++++++++++
> 8 files changed, 967 insertions(+), 1 deletion(-)
> create mode 100644 include/kvm/vfio.h
> create mode 100644 vfio/core.c
> create mode 100644 vfio/pci.c
>
[...]
> diff --git a/vfio/core.c b/vfio/core.c
> new file mode 100644
> index 000000000000..e1b7366b9eda
> --- /dev/null
> +++ b/vfio/core.c
> @@ -0,0 +1,488 @@
> +#include "kvm/kvm.h"
> +#include "kvm/vfio.h"
> +
> +#include <linux/list.h>
> +
> +#define VFIO_DEV_DIR "/dev/vfio"
> +#define VFIO_DEV_NODE VFIO_DEV_DIR "/vfio"
> +#define IOMMU_GROUP_DIR "/sys/kernel/iommu_groups"
> +
> +static int vfio_container;
> +static LIST_HEAD(vfio_groups);
> +static struct vfio_device *vfio_devices;
> +
> +static int vfio_device_pci_parser(const struct option *opt, char *arg,
> + struct vfio_device_params *dev)
> +{
> + unsigned int domain, bus, devnr, fn;
> +
> + int nr = sscanf(arg, "%4x:%2x:%2x.%1x", &domain, &bus, &devnr, &fn);
> + if (nr < 4) {
> + domain = 0;
> + nr = sscanf(arg, "%2x:%2x.%1x", &bus, &devnr, &fn);
> + if (nr < 3) {
> + pr_err("Invalid device identifier %s", arg);
> + return -EINVAL;
> + }
> + }
> +
> + dev->type = VFIO_DEVICE_PCI;
> + dev->bus = "pci";
> + dev->name = malloc(13);
> + if (!dev->name)
> + return -ENOMEM;
> +
> + snprintf(dev->name, 13, "%04x:%02x:%02x.%x", domain, bus, devnr, fn);
> +
> + return 0;
> +}
> +
> +int vfio_device_parser(const struct option *opt, const char *arg, int unset)
> +{
> + int ret = -EINVAL;
> + static int idx = 0;
> + struct kvm *kvm = opt->ptr;
> + struct vfio_device_params *dev, *devs;
> + char *cur, *buf = strdup(arg);
> +
> + if (!buf)
> + return -ENOMEM;
> +
> + if (idx >= MAX_VFIO_DEVICES) {
> + pr_warning("Too many VFIO devices");
> + goto out_free_buf;
> + }
> +
> + devs = realloc(kvm->cfg.vfio_devices, sizeof(*dev) * (idx + 1));
> + if (!devs) {
> + ret = -ENOMEM;
> + goto out_free_buf;
> + }
> +
> + kvm->cfg.vfio_devices = devs;
I noticed that vfio_devices never gets freed. I'm not sure if this needs
fixing as I noticed a similar pattern in virtio as well.
The previous patches in the series look fine. I'll have a look at the
rest in the next couple of days.
Thanks,
Punit
> + dev = &devs[idx];
> +
> + cur = strtok(buf, ",");
> +
> + if (!strcmp(opt->long_name, "vfio-pci"))
> + ret = vfio_device_pci_parser(opt, cur, dev);
> + else
> + ret = -EINVAL;
> +
> + if (!ret)
> + kvm->cfg.num_vfio_devices = ++idx;
> +
> +out_free_buf:
> + free(buf);
> +
> + return ret;
> +}
> +
[...]
