| Message ID | 20230517095718.16117-1-nipun.gupta@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v6] vfio/cdx: add support for CDX bus | expand |
On Wed, 17 May 2023 15:27:18 +0530 Nipun Gupta <nipun.gupta@amd.com> wrote: > vfio-cdx driver enables IOCTLs for user space to query > MMIO regions for CDX devices and mmap them. This change > also adds support for reset of CDX devices. With VFIO > enabled on CDX devices, user-space applications can also > exercise DMA securely via IOMMU on these devices. > > This change adds the VFIO CDX driver and enables the following > ioctls for CDX devices: > - VFIO_DEVICE_GET_INFO: > - VFIO_DEVICE_GET_REGION_INFO > - VFIO_DEVICE_RESET > > Signed-off-by: Nipun Gupta <nipun.gupta@amd.com> > Reviewed-by: Pieter Jansen van Vuuren <pieter.jansen-van-vuuren@amd.com> > Tested-by: Nikhil Agarwal <nikhil.agarwal@amd.com> > --- > > Changes v5->v6: > - removed forward declaration of vfio_cdx_driver > - removed un-necessary CDX_DRIVER_OVERRIDE_DEVICE_VFIO and > vfio_cdx_regions_cleanup. > - removed unrequired dev_warn/dev_err > - used module_driver instead of module_init/exit > > Changes v4->v5: > - renamed vfio_cdx.c to main.c and vfio_cdx_private.h > to private.h > - have separate functions for get_info and get_region_info > > Changes v3->v4: > - fix vfio info flags > > Changes v2->v3: > - removed redundant init and release functions > - removed redundant dev and cdx_dev from vfio_cdx_device > - added support for iommufd > - added VFIO_DEVICE_FLAGS_CDX > - removed unrequried WARN_ON > - removed unused ioaddr > > Changes v1->v2: > - Updated file2alias to support vfio_cdx > - removed some un-necessary checks in mmap > - removed vfio reset wrapper API > - converted complex macros to static APIs > - used pgprot_device and io_remap_pfn_range > > MAINTAINERS | 7 + > drivers/vfio/Kconfig | 1 + > drivers/vfio/Makefile | 1 + > drivers/vfio/cdx/Kconfig | 17 +++ > drivers/vfio/cdx/Makefile | 8 + > drivers/vfio/cdx/main.c | 234 ++++++++++++++++++++++++++++++ > drivers/vfio/cdx/private.h | 28 ++++ > include/linux/cdx/cdx_bus.h | 1 - > include/linux/mod_devicetable.h | 6 + > include/uapi/linux/vfio.h | 1 + > scripts/mod/devicetable-offsets.c | 1 + > scripts/mod/file2alias.c | 17 ++- > 12 files changed, 320 insertions(+), 2 deletions(-) > create mode 100644 drivers/vfio/cdx/Kconfig > create mode 100644 drivers/vfio/cdx/Makefile > create mode 100644 drivers/vfio/cdx/main.c > create mode 100644 drivers/vfio/cdx/private.h > > diff --git a/MAINTAINERS b/MAINTAINERS > index a72b8fcea261..d6d1ddb854d7 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -22096,6 +22096,13 @@ F: Documentation/filesystems/vfat.rst > F: fs/fat/ > F: tools/testing/selftests/filesystems/fat/ > > +VFIO CDX DRIVER > +M: Nipun Gupta <nipun.gupta@amd.com> > +M: Nikhil Agarwal <nikhil.agarwal@amd.com> > +L: kvm@vger.kernel.org > +S: Maintained > +F: drivers/vfio/cdx/* > + > VFIO DRIVER > M: Alex Williamson <alex.williamson@redhat.com> > L: kvm@vger.kernel.org > diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig > index 89e06c981e43..aba36f5be4ec 100644 > --- a/drivers/vfio/Kconfig > +++ b/drivers/vfio/Kconfig > @@ -57,6 +57,7 @@ source "drivers/vfio/pci/Kconfig" > source "drivers/vfio/platform/Kconfig" > source "drivers/vfio/mdev/Kconfig" > source "drivers/vfio/fsl-mc/Kconfig" > +source "drivers/vfio/cdx/Kconfig" > endif > > source "virt/lib/Kconfig" > diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile > index 70e7dcb302ef..1a27b2516612 100644 > --- a/drivers/vfio/Makefile > +++ b/drivers/vfio/Makefile > @@ -14,3 +14,4 @@ obj-$(CONFIG_VFIO_PCI) += pci/ > obj-$(CONFIG_VFIO_PLATFORM) += platform/ > obj-$(CONFIG_VFIO_MDEV) += mdev/ > obj-$(CONFIG_VFIO_FSL_MC) += fsl-mc/ > +obj-$(CONFIG_VFIO_CDX) += cdx/ > diff --git a/drivers/vfio/cdx/Kconfig b/drivers/vfio/cdx/Kconfig > new file mode 100644 > index 000000000000..e6de0a0caa32 > --- /dev/null > +++ b/drivers/vfio/cdx/Kconfig > @@ -0,0 +1,17 @@ > +# SPDX-License-Identifier: GPL-2.0 > +# > +# VFIO CDX configuration > +# > +# Copyright (C) 2022-2023, Advanced Micro Devices, Inc. > +# > + > +config VFIO_CDX > + tristate "VFIO support for CDX bus devices" > + depends on CDX_BUS > + select EVENTFD > + help > + Driver to enable VFIO support for the devices on CDX bus. > + This is required to make use of CDX devices present in > + the system using the VFIO framework. > + > + If you don't know what to do here, say N. > diff --git a/drivers/vfio/cdx/Makefile b/drivers/vfio/cdx/Makefile > new file mode 100644 > index 000000000000..cd4a2e6fe609 > --- /dev/null > +++ b/drivers/vfio/cdx/Makefile > @@ -0,0 +1,8 @@ > +# SPDX-License-Identifier: GPL-2.0 > +# > +# Copyright (C) 2022-2023, Advanced Micro Devices, Inc. > +# > + > +obj-$(CONFIG_VFIO_CDX) += vfio-cdx.o > + > +vfio-cdx-objs := main.o > diff --git a/drivers/vfio/cdx/main.c b/drivers/vfio/cdx/main.c > new file mode 100644 > index 000000000000..f03f491e0435 > --- /dev/null > +++ b/drivers/vfio/cdx/main.c > @@ -0,0 +1,234 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2022-2023, Advanced Micro Devices, Inc. > + */ > + > +#include <linux/vfio.h> > +#include <linux/cdx/cdx_bus.h> > + > +#include "private.h" > + > +static int vfio_cdx_open_device(struct vfio_device *core_vdev) > +{ > + struct vfio_cdx_device *vdev = > + container_of(core_vdev, struct vfio_cdx_device, vdev); > + struct cdx_device *cdx_dev = to_cdx_device(core_vdev->dev); > + int count = cdx_dev->res_count; > + int i; > + > + vdev->regions = kcalloc(count, sizeof(struct vfio_cdx_region), > + GFP_KERNEL); Nit, GFP_KERNEL_ACCOUNT since we're allocating long term storage on behalf of a user operation. > + if (!vdev->regions) > + return -ENOMEM; > + > + for (i = 0; i < count; i++) { > + struct resource *res = &cdx_dev->res[i]; > + > + vdev->regions[i].addr = res->start; > + vdev->regions[i].size = resource_size(res); > + vdev->regions[i].type = res->flags; > + /* > + * Only regions addressed with PAGE granularity may be > + * MMAP'ed securely. > + */ > + if (!(vdev->regions[i].addr & ~PAGE_MASK) && > + !(vdev->regions[i].size & ~PAGE_MASK)) > + vdev->regions[i].flags |= > + VFIO_REGION_INFO_FLAG_MMAP; > + vdev->regions[i].flags |= VFIO_REGION_INFO_FLAG_READ; > + if (!(cdx_dev->res[i].flags & IORESOURCE_READONLY)) > + vdev->regions[i].flags |= VFIO_REGION_INFO_FLAG_WRITE; > + } > + > + return 0; > +} > + > +static void vfio_cdx_close_device(struct vfio_device *core_vdev) > +{ > + struct vfio_cdx_device *vdev = > + container_of(core_vdev, struct vfio_cdx_device, vdev); > + > + kfree(vdev->regions); > + cdx_dev_reset(core_vdev->dev); > +} > + > +static int vfio_cdx_ioctl_get_info(struct vfio_cdx_device *vdev, > + struct vfio_device_info __user *arg) > +{ > + unsigned long minsz = offsetofend(struct vfio_device_info, num_irqs); > + struct cdx_device *cdx_dev = to_cdx_device(vdev->vdev.dev); > + struct vfio_device_info info; > + > + if (copy_from_user(&info, arg, minsz)) > + return -EFAULT; > + > + if (info.argsz < minsz) > + return -EINVAL; > + > + info.flags = VFIO_DEVICE_FLAGS_CDX; > + info.flags |= VFIO_DEVICE_FLAGS_RESET; > + > + info.num_regions = cdx_dev->res_count; > + info.num_irqs = 0; > + > + return copy_to_user(arg, &info, minsz) ? -EFAULT : 0; > +} > + > +static int vfio_cdx_ioctl_get_region_info(struct vfio_cdx_device *vdev, > + struct vfio_region_info __user *arg) > +{ > + unsigned long minsz = offsetofend(struct vfio_region_info, offset); > + struct cdx_device *cdx_dev = to_cdx_device(vdev->vdev.dev); > + struct vfio_region_info info; > + > + if (copy_from_user(&info, arg, minsz)) > + return -EFAULT; > + > + if (info.argsz < minsz) > + return -EINVAL; > + > + if (info.index >= cdx_dev->res_count) > + return -EINVAL; > + > + /* map offset to the physical address */ > + info.offset = vfio_cdx_index_to_offset(info.index); > + info.size = vdev->regions[info.index].size; > + info.flags = vdev->regions[info.index].flags; > + > + return copy_to_user(arg, &info, minsz) ? -EFAULT : 0; > +} > + > +static long vfio_cdx_ioctl(struct vfio_device *core_vdev, > + unsigned int cmd, unsigned long arg) > +{ > + struct vfio_cdx_device *vdev = > + container_of(core_vdev, struct vfio_cdx_device, vdev); > + void __user *uarg = (void __user *)arg; > + > + switch (cmd) { > + case VFIO_DEVICE_GET_INFO: > + return vfio_cdx_ioctl_get_info(vdev, uarg); > + case VFIO_DEVICE_GET_REGION_INFO: > + return vfio_cdx_ioctl_get_region_info(vdev, uarg); > + case VFIO_DEVICE_RESET: > + return cdx_dev_reset(core_vdev->dev); > + default: > + return -ENOTTY; > + } > +} > + > +static int vfio_cdx_mmap_mmio(struct vfio_cdx_region region, > + struct vm_area_struct *vma) > +{ > + u64 size = vma->vm_end - vma->vm_start; > + u64 pgoff, base; > + > + pgoff = vma->vm_pgoff & > + ((1U << (VFIO_CDX_OFFSET_SHIFT - PAGE_SHIFT)) - 1); > + base = pgoff << PAGE_SHIFT; > + > + if (region.size < PAGE_SIZE || base + size > region.size) Nit, we've already enforced the first condition, a sub-page region won't have the mmap flag set and we already verified this region does have that flag set. > + return -EINVAL; > + > + vma->vm_pgoff = (region.addr >> PAGE_SHIFT) + pgoff; > + vma->vm_page_prot = pgprot_device(vma->vm_page_prot); > + > + return io_remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff, > + size, vma->vm_page_prot); > +} > + > +static int vfio_cdx_mmap(struct vfio_device *core_vdev, > + struct vm_area_struct *vma) > +{ > + struct vfio_cdx_device *vdev = > + container_of(core_vdev, struct vfio_cdx_device, vdev); > + struct cdx_device *cdx_dev = to_cdx_device(core_vdev->dev); > + unsigned int index; > + > + index = vma->vm_pgoff >> (VFIO_CDX_OFFSET_SHIFT - PAGE_SHIFT); > + > + if (index >= cdx_dev->res_count) > + return -EINVAL; > + > + if (!(vdev->regions[index].flags & VFIO_REGION_INFO_FLAG_MMAP)) > + return -EINVAL; > + > + if (!(vdev->regions[index].flags & VFIO_REGION_INFO_FLAG_READ) && > + (vma->vm_flags & VM_READ)) > + return -EINVAL; > + > + if (!(vdev->regions[index].flags & VFIO_REGION_INFO_FLAG_WRITE) && > + (vma->vm_flags & VM_WRITE)) > + return -EINVAL; It might be useful to distinguish these two cases with -EPERM. Otherwise this looks ok to me. Thanks, Alex > + > + return vfio_cdx_mmap_mmio(vdev->regions[index], vma); > +} > + > +static const struct vfio_device_ops vfio_cdx_ops = { > + .name = "vfio-cdx", > + .open_device = vfio_cdx_open_device, > + .close_device = vfio_cdx_close_device, > + .ioctl = vfio_cdx_ioctl, > + .mmap = vfio_cdx_mmap, > + .bind_iommufd = vfio_iommufd_physical_bind, > + .unbind_iommufd = vfio_iommufd_physical_unbind, > + .attach_ioas = vfio_iommufd_physical_attach_ioas, > +}; > + > +static int vfio_cdx_probe(struct cdx_device *cdx_dev) > +{ > + struct vfio_cdx_device *vdev = NULL; > + struct device *dev = &cdx_dev->dev; > + int ret; > + > + vdev = vfio_alloc_device(vfio_cdx_device, vdev, dev, > + &vfio_cdx_ops); > + if (IS_ERR(vdev)) > + return PTR_ERR(vdev); > + > + ret = vfio_register_group_dev(&vdev->vdev); > + if (ret) > + goto out_uninit; > + > + dev_set_drvdata(dev, vdev); > + return 0; > + > +out_uninit: > + vfio_put_device(&vdev->vdev); > + return ret; > +} > + > +static int vfio_cdx_remove(struct cdx_device *cdx_dev) > +{ > + struct device *dev = &cdx_dev->dev; > + struct vfio_cdx_device *vdev = dev_get_drvdata(dev); > + > + vfio_unregister_group_dev(&vdev->vdev); > + vfio_put_device(&vdev->vdev); > + > + return 0; > +} > + > +static const struct cdx_device_id vfio_cdx_table[] = { > + { CDX_DEVICE_DRIVER_OVERRIDE(CDX_ANY_ID, CDX_ANY_ID, > + CDX_ID_F_VFIO_DRIVER_OVERRIDE) }, /* match all by default */ > + {} > +}; > + > +MODULE_DEVICE_TABLE(cdx, vfio_cdx_table); > + > +static struct cdx_driver vfio_cdx_driver = { > + .probe = vfio_cdx_probe, > + .remove = vfio_cdx_remove, > + .match_id_table = vfio_cdx_table, > + .driver = { > + .name = "vfio-cdx", > + .owner = THIS_MODULE, > + }, > + .driver_managed_dma = true, > +}; > + > +module_driver(vfio_cdx_driver, cdx_driver_register, cdx_driver_unregister); > + > +MODULE_LICENSE("GPL"); > +MODULE_DESCRIPTION("VFIO for CDX devices - User Level meta-driver"); > diff --git a/drivers/vfio/cdx/private.h b/drivers/vfio/cdx/private.h > new file mode 100644 > index 000000000000..8bdc117ea88e > --- /dev/null > +++ b/drivers/vfio/cdx/private.h > @@ -0,0 +1,28 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Copyright (C) 2022-2023, Advanced Micro Devices, Inc. > + */ > + > +#ifndef VFIO_CDX_PRIVATE_H > +#define VFIO_CDX_PRIVATE_H > + > +#define VFIO_CDX_OFFSET_SHIFT 40 > + > +static inline u64 vfio_cdx_index_to_offset(u32 index) > +{ > + return ((u64)(index) << VFIO_CDX_OFFSET_SHIFT); > +} > + > +struct vfio_cdx_region { > + u32 flags; > + u32 type; > + u64 addr; > + resource_size_t size; > +}; > + > +struct vfio_cdx_device { > + struct vfio_device vdev; > + struct vfio_cdx_region *regions; > +}; > + > +#endif /* VFIO_CDX_PRIVATE_H */ > diff --git a/include/linux/cdx/cdx_bus.h b/include/linux/cdx/cdx_bus.h > index 35ef41d8a61a..bead71b7bc73 100644 > --- a/include/linux/cdx/cdx_bus.h > +++ b/include/linux/cdx/cdx_bus.h > @@ -14,7 +14,6 @@ > #include <linux/mod_devicetable.h> > > #define MAX_CDX_DEV_RESOURCES 4 > -#define CDX_ANY_ID (0xFFFF) > #define CDX_CONTROLLER_ID_SHIFT 4 > #define CDX_BUS_NUM_MASK 0xF > > diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h > index ccaaeda792c0..ccf017353bb6 100644 > --- a/include/linux/mod_devicetable.h > +++ b/include/linux/mod_devicetable.h > @@ -912,6 +912,12 @@ struct ishtp_device_id { > kernel_ulong_t driver_data; > }; > > +#define CDX_ANY_ID (0xFFFF) > + > +enum { > + CDX_ID_F_VFIO_DRIVER_OVERRIDE = 1, > +}; > + > /** > * struct cdx_device_id - CDX device identifier > * @vendor: Vendor ID > diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h > index 0552e8dcf0cb..8e91aaf973e7 100644 > --- a/include/uapi/linux/vfio.h > +++ b/include/uapi/linux/vfio.h > @@ -213,6 +213,7 @@ struct vfio_device_info { > #define VFIO_DEVICE_FLAGS_AP (1 << 5) /* vfio-ap device */ > #define VFIO_DEVICE_FLAGS_FSL_MC (1 << 6) /* vfio-fsl-mc device */ > #define VFIO_DEVICE_FLAGS_CAPS (1 << 7) /* Info supports caps */ > +#define VFIO_DEVICE_FLAGS_CDX (1 << 8) /* vfio-cdx device */ > __u32 num_regions; /* Max region index + 1 */ > __u32 num_irqs; /* Max IRQ index + 1 */ > __u32 cap_offset; /* Offset within info struct of first cap */ > diff --git a/scripts/mod/devicetable-offsets.c b/scripts/mod/devicetable-offsets.c > index 62dc988df84d..abe65f8968dd 100644 > --- a/scripts/mod/devicetable-offsets.c > +++ b/scripts/mod/devicetable-offsets.c > @@ -265,6 +265,7 @@ int main(void) > DEVID(cdx_device_id); > DEVID_FIELD(cdx_device_id, vendor); > DEVID_FIELD(cdx_device_id, device); > + DEVID_FIELD(cdx_device_id, override_only); > > return 0; > } > diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c > index 28da34ba4359..38120f932b0d 100644 > --- a/scripts/mod/file2alias.c > +++ b/scripts/mod/file2alias.c > @@ -1458,8 +1458,23 @@ static int do_cdx_entry(const char *filename, void *symval, > { > DEF_FIELD(symval, cdx_device_id, vendor); > DEF_FIELD(symval, cdx_device_id, device); > + DEF_FIELD(symval, cdx_device_id, override_only); > > - sprintf(alias, "cdx:v%08Xd%08Xd", vendor, device); > + switch (override_only) { > + case 0: > + strcpy(alias, "cdx:"); > + break; > + case CDX_ID_F_VFIO_DRIVER_OVERRIDE: > + strcpy(alias, "vfio_cdx:"); > + break; > + default: > + warn("Unknown CDX driver_override alias %08X\n", > + override_only); > + return 0; > + } > + > + ADD(alias, "v", vendor != CDX_ANY_ID, vendor); > + ADD(alias, "d", device != CDX_ANY_ID, device); > return 1; > } >
On Wed, 24 May 2023 10:45:29 -0600 Alex Williamson <alex.williamson@redhat.com> wrote: > On Wed, 17 May 2023 15:27:18 +0530 > Nipun Gupta <nipun.gupta@amd.com> wrote: > > > vfio-cdx driver enables IOCTLs for user space to query > > MMIO regions for CDX devices and mmap them. This change > > also adds support for reset of CDX devices. With VFIO > > enabled on CDX devices, user-space applications can also > > exercise DMA securely via IOMMU on these devices. > > > > This change adds the VFIO CDX driver and enables the following > > ioctls for CDX devices: > > - VFIO_DEVICE_GET_INFO: > > - VFIO_DEVICE_GET_REGION_INFO > > - VFIO_DEVICE_RESET > > > > Signed-off-by: Nipun Gupta <nipun.gupta@amd.com> > > Reviewed-by: Pieter Jansen van Vuuren <pieter.jansen-van-vuuren@amd.com> > > Tested-by: Nikhil Agarwal <nikhil.agarwal@amd.com> > > --- > > > > Changes v5->v6: > > - removed forward declaration of vfio_cdx_driver > > - removed un-necessary CDX_DRIVER_OVERRIDE_DEVICE_VFIO and > > vfio_cdx_regions_cleanup. > > - removed unrequired dev_warn/dev_err > > - used module_driver instead of module_init/exit > > > > Changes v4->v5: > > - renamed vfio_cdx.c to main.c and vfio_cdx_private.h > > to private.h > > - have separate functions for get_info and get_region_info > > > > Changes v3->v4: > > - fix vfio info flags > > > > Changes v2->v3: > > - removed redundant init and release functions > > - removed redundant dev and cdx_dev from vfio_cdx_device > > - added support for iommufd > > - added VFIO_DEVICE_FLAGS_CDX > > - removed unrequried WARN_ON > > - removed unused ioaddr > > > > Changes v1->v2: > > - Updated file2alias to support vfio_cdx > > - removed some un-necessary checks in mmap > > - removed vfio reset wrapper API > > - converted complex macros to static APIs > > - used pgprot_device and io_remap_pfn_range > > > > MAINTAINERS | 7 + > > drivers/vfio/Kconfig | 1 + > > drivers/vfio/Makefile | 1 + > > drivers/vfio/cdx/Kconfig | 17 +++ > > drivers/vfio/cdx/Makefile | 8 + > > drivers/vfio/cdx/main.c | 234 ++++++++++++++++++++++++++++++ > > drivers/vfio/cdx/private.h | 28 ++++ > > include/linux/cdx/cdx_bus.h | 1 - > > include/linux/mod_devicetable.h | 6 + > > include/uapi/linux/vfio.h | 1 + > > scripts/mod/devicetable-offsets.c | 1 + > > scripts/mod/file2alias.c | 17 ++- > > 12 files changed, 320 insertions(+), 2 deletions(-) > > create mode 100644 drivers/vfio/cdx/Kconfig > > create mode 100644 drivers/vfio/cdx/Makefile > > create mode 100644 drivers/vfio/cdx/main.c > > create mode 100644 drivers/vfio/cdx/private.h > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index a72b8fcea261..d6d1ddb854d7 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -22096,6 +22096,13 @@ F: Documentation/filesystems/vfat.rst > > F: fs/fat/ > > F: tools/testing/selftests/filesystems/fat/ > > > > +VFIO CDX DRIVER > > +M: Nipun Gupta <nipun.gupta@amd.com> > > +M: Nikhil Agarwal <nikhil.agarwal@amd.com> > > +L: kvm@vger.kernel.org > > +S: Maintained > > +F: drivers/vfio/cdx/* > > + > > VFIO DRIVER > > M: Alex Williamson <alex.williamson@redhat.com> > > L: kvm@vger.kernel.org > > diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig > > index 89e06c981e43..aba36f5be4ec 100644 > > --- a/drivers/vfio/Kconfig > > +++ b/drivers/vfio/Kconfig > > @@ -57,6 +57,7 @@ source "drivers/vfio/pci/Kconfig" > > source "drivers/vfio/platform/Kconfig" > > source "drivers/vfio/mdev/Kconfig" > > source "drivers/vfio/fsl-mc/Kconfig" > > +source "drivers/vfio/cdx/Kconfig" > > endif > > > > source "virt/lib/Kconfig" > > diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile > > index 70e7dcb302ef..1a27b2516612 100644 > > --- a/drivers/vfio/Makefile > > +++ b/drivers/vfio/Makefile > > @@ -14,3 +14,4 @@ obj-$(CONFIG_VFIO_PCI) += pci/ > > obj-$(CONFIG_VFIO_PLATFORM) += platform/ > > obj-$(CONFIG_VFIO_MDEV) += mdev/ > > obj-$(CONFIG_VFIO_FSL_MC) += fsl-mc/ > > +obj-$(CONFIG_VFIO_CDX) += cdx/ > > diff --git a/drivers/vfio/cdx/Kconfig b/drivers/vfio/cdx/Kconfig > > new file mode 100644 > > index 000000000000..e6de0a0caa32 > > --- /dev/null > > +++ b/drivers/vfio/cdx/Kconfig > > @@ -0,0 +1,17 @@ > > +# SPDX-License-Identifier: GPL-2.0 > > +# > > +# VFIO CDX configuration > > +# > > +# Copyright (C) 2022-2023, Advanced Micro Devices, Inc. > > +# > > + > > +config VFIO_CDX > > + tristate "VFIO support for CDX bus devices" > > + depends on CDX_BUS > > + select EVENTFD > > + help > > + Driver to enable VFIO support for the devices on CDX bus. > > + This is required to make use of CDX devices present in > > + the system using the VFIO framework. > > + > > + If you don't know what to do here, say N. > > diff --git a/drivers/vfio/cdx/Makefile b/drivers/vfio/cdx/Makefile > > new file mode 100644 > > index 000000000000..cd4a2e6fe609 > > --- /dev/null > > +++ b/drivers/vfio/cdx/Makefile > > @@ -0,0 +1,8 @@ > > +# SPDX-License-Identifier: GPL-2.0 > > +# > > +# Copyright (C) 2022-2023, Advanced Micro Devices, Inc. > > +# > > + > > +obj-$(CONFIG_VFIO_CDX) += vfio-cdx.o > > + > > +vfio-cdx-objs := main.o > > diff --git a/drivers/vfio/cdx/main.c b/drivers/vfio/cdx/main.c > > new file mode 100644 > > index 000000000000..f03f491e0435 > > --- /dev/null > > +++ b/drivers/vfio/cdx/main.c > > @@ -0,0 +1,234 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Copyright (C) 2022-2023, Advanced Micro Devices, Inc. > > + */ > > + > > +#include <linux/vfio.h> > > +#include <linux/cdx/cdx_bus.h> > > + > > +#include "private.h" > > + > > +static int vfio_cdx_open_device(struct vfio_device *core_vdev) > > +{ > > + struct vfio_cdx_device *vdev = > > + container_of(core_vdev, struct vfio_cdx_device, vdev); > > + struct cdx_device *cdx_dev = to_cdx_device(core_vdev->dev); > > + int count = cdx_dev->res_count; > > + int i; > > + > > + vdev->regions = kcalloc(count, sizeof(struct vfio_cdx_region), > > + GFP_KERNEL); > > Nit, GFP_KERNEL_ACCOUNT since we're allocating long term storage on > behalf of a user operation. > > > + if (!vdev->regions) > > + return -ENOMEM; > > + > > + for (i = 0; i < count; i++) { > > + struct resource *res = &cdx_dev->res[i]; > > + > > + vdev->regions[i].addr = res->start; > > + vdev->regions[i].size = resource_size(res); > > + vdev->regions[i].type = res->flags; > > + /* > > + * Only regions addressed with PAGE granularity may be > > + * MMAP'ed securely. > > + */ > > + if (!(vdev->regions[i].addr & ~PAGE_MASK) && > > + !(vdev->regions[i].size & ~PAGE_MASK)) > > + vdev->regions[i].flags |= > > + VFIO_REGION_INFO_FLAG_MMAP; > > + vdev->regions[i].flags |= VFIO_REGION_INFO_FLAG_READ; > > + if (!(cdx_dev->res[i].flags & IORESOURCE_READONLY)) > > + vdev->regions[i].flags |= VFIO_REGION_INFO_FLAG_WRITE; > > + } > > + > > + return 0; > > +} > > + > > +static void vfio_cdx_close_device(struct vfio_device *core_vdev) > > +{ > > + struct vfio_cdx_device *vdev = > > + container_of(core_vdev, struct vfio_cdx_device, vdev); > > + > > + kfree(vdev->regions); > > + cdx_dev_reset(core_vdev->dev); > > +} > > + > > +static int vfio_cdx_ioctl_get_info(struct vfio_cdx_device *vdev, > > + struct vfio_device_info __user *arg) > > +{ > > + unsigned long minsz = offsetofend(struct vfio_device_info, num_irqs); > > + struct cdx_device *cdx_dev = to_cdx_device(vdev->vdev.dev); > > + struct vfio_device_info info; > > + > > + if (copy_from_user(&info, arg, minsz)) > > + return -EFAULT; > > + > > + if (info.argsz < minsz) > > + return -EINVAL; > > + > > + info.flags = VFIO_DEVICE_FLAGS_CDX; > > + info.flags |= VFIO_DEVICE_FLAGS_RESET; > > + > > + info.num_regions = cdx_dev->res_count; > > + info.num_irqs = 0; > > + > > + return copy_to_user(arg, &info, minsz) ? -EFAULT : 0; > > +} > > + > > +static int vfio_cdx_ioctl_get_region_info(struct vfio_cdx_device *vdev, > > + struct vfio_region_info __user *arg) > > +{ > > + unsigned long minsz = offsetofend(struct vfio_region_info, offset); > > + struct cdx_device *cdx_dev = to_cdx_device(vdev->vdev.dev); > > + struct vfio_region_info info; > > + > > + if (copy_from_user(&info, arg, minsz)) > > + return -EFAULT; > > + > > + if (info.argsz < minsz) > > + return -EINVAL; > > + > > + if (info.index >= cdx_dev->res_count) > > + return -EINVAL; > > + > > + /* map offset to the physical address */ > > + info.offset = vfio_cdx_index_to_offset(info.index); > > + info.size = vdev->regions[info.index].size; > > + info.flags = vdev->regions[info.index].flags; > > + > > + return copy_to_user(arg, &info, minsz) ? -EFAULT : 0; > > +} > > + > > +static long vfio_cdx_ioctl(struct vfio_device *core_vdev, > > + unsigned int cmd, unsigned long arg) > > +{ > > + struct vfio_cdx_device *vdev = > > + container_of(core_vdev, struct vfio_cdx_device, vdev); > > + void __user *uarg = (void __user *)arg; > > + > > + switch (cmd) { > > + case VFIO_DEVICE_GET_INFO: > > + return vfio_cdx_ioctl_get_info(vdev, uarg); > > + case VFIO_DEVICE_GET_REGION_INFO: > > + return vfio_cdx_ioctl_get_region_info(vdev, uarg); > > + case VFIO_DEVICE_RESET: > > + return cdx_dev_reset(core_vdev->dev); > > + default: > > + return -ENOTTY; > > + } > > +} > > + > > +static int vfio_cdx_mmap_mmio(struct vfio_cdx_region region, > > + struct vm_area_struct *vma) > > +{ > > + u64 size = vma->vm_end - vma->vm_start; > > + u64 pgoff, base; > > + > > + pgoff = vma->vm_pgoff & > > + ((1U << (VFIO_CDX_OFFSET_SHIFT - PAGE_SHIFT)) - 1); > > + base = pgoff << PAGE_SHIFT; > > + > > + if (region.size < PAGE_SIZE || base + size > region.size) > > Nit, we've already enforced the first condition, a sub-page region > won't have the mmap flag set and we already verified this region does > have that flag set. > > > + return -EINVAL; > > + > > + vma->vm_pgoff = (region.addr >> PAGE_SHIFT) + pgoff; > > + vma->vm_page_prot = pgprot_device(vma->vm_page_prot); > > + > > + return io_remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff, > > + size, vma->vm_page_prot); > > +} > > + > > +static int vfio_cdx_mmap(struct vfio_device *core_vdev, > > + struct vm_area_struct *vma) > > +{ > > + struct vfio_cdx_device *vdev = > > + container_of(core_vdev, struct vfio_cdx_device, vdev); > > + struct cdx_device *cdx_dev = to_cdx_device(core_vdev->dev); > > + unsigned int index; > > + > > + index = vma->vm_pgoff >> (VFIO_CDX_OFFSET_SHIFT - PAGE_SHIFT); > > + > > + if (index >= cdx_dev->res_count) > > + return -EINVAL; > > + > > + if (!(vdev->regions[index].flags & VFIO_REGION_INFO_FLAG_MMAP)) > > + return -EINVAL; > > + > > + if (!(vdev->regions[index].flags & VFIO_REGION_INFO_FLAG_READ) && > > + (vma->vm_flags & VM_READ)) > > + return -EINVAL; > > + > > + if (!(vdev->regions[index].flags & VFIO_REGION_INFO_FLAG_WRITE) && > > + (vma->vm_flags & VM_WRITE)) > > + return -EINVAL; > > It might be useful to distinguish these two cases with -EPERM. > Otherwise this looks ok to me. Thanks, > > Alex > > > + > > + return vfio_cdx_mmap_mmio(vdev->regions[index], vma); > > +} > > + > > +static const struct vfio_device_ops vfio_cdx_ops = { > > + .name = "vfio-cdx", > > + .open_device = vfio_cdx_open_device, > > + .close_device = vfio_cdx_close_device, > > + .ioctl = vfio_cdx_ioctl, > > + .mmap = vfio_cdx_mmap, > > + .bind_iommufd = vfio_iommufd_physical_bind, > > + .unbind_iommufd = vfio_iommufd_physical_unbind, > > + .attach_ioas = vfio_iommufd_physical_attach_ioas, > > +}; > > + > > +static int vfio_cdx_probe(struct cdx_device *cdx_dev) > > +{ > > + struct vfio_cdx_device *vdev = NULL; > > + struct device *dev = &cdx_dev->dev; > > + int ret; > > + > > + vdev = vfio_alloc_device(vfio_cdx_device, vdev, dev, > > + &vfio_cdx_ops); > > + if (IS_ERR(vdev)) > > + return PTR_ERR(vdev); > > + > > + ret = vfio_register_group_dev(&vdev->vdev); > > + if (ret) > > + goto out_uninit; > > + > > + dev_set_drvdata(dev, vdev); > > + return 0; > > + > > +out_uninit: > > + vfio_put_device(&vdev->vdev); > > + return ret; > > +} > > + > > +static int vfio_cdx_remove(struct cdx_device *cdx_dev) > > +{ > > + struct device *dev = &cdx_dev->dev; > > + struct vfio_cdx_device *vdev = dev_get_drvdata(dev); > > + > > + vfio_unregister_group_dev(&vdev->vdev); > > + vfio_put_device(&vdev->vdev); > > + > > + return 0; > > +} > > + > > +static const struct cdx_device_id vfio_cdx_table[] = { > > + { CDX_DEVICE_DRIVER_OVERRIDE(CDX_ANY_ID, CDX_ANY_ID, > > + CDX_ID_F_VFIO_DRIVER_OVERRIDE) }, /* match all by default */ > > + {} > > +}; > > + > > +MODULE_DEVICE_TABLE(cdx, vfio_cdx_table); > > + > > +static struct cdx_driver vfio_cdx_driver = { > > + .probe = vfio_cdx_probe, > > + .remove = vfio_cdx_remove, > > + .match_id_table = vfio_cdx_table, > > + .driver = { > > + .name = "vfio-cdx", > > + .owner = THIS_MODULE, > > + }, > > + .driver_managed_dma = true, Hmm, looks like cdx bus is broken here, there's no actual implementation of a dma_configure callback and no setup of the IOMMU default domain for theoretical cdx drivers that might want to use the DMA API. Without that, this driver_manged_dma flag doesn't provide any guarantees to a vfio driver that we have exclusive ownership of the group. Please fix, this flag needs to actually have some meaning on cdx. Thanks, Alex > > +}; > > + > > +module_driver(vfio_cdx_driver, cdx_driver_register, cdx_driver_unregister); > > + > > +MODULE_LICENSE("GPL"); > > +MODULE_DESCRIPTION("VFIO for CDX devices - User Level meta-driver"); > > diff --git a/drivers/vfio/cdx/private.h b/drivers/vfio/cdx/private.h > > new file mode 100644 > > index 000000000000..8bdc117ea88e > > --- /dev/null > > +++ b/drivers/vfio/cdx/private.h > > @@ -0,0 +1,28 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +/* > > + * Copyright (C) 2022-2023, Advanced Micro Devices, Inc. > > + */ > > + > > +#ifndef VFIO_CDX_PRIVATE_H > > +#define VFIO_CDX_PRIVATE_H > > + > > +#define VFIO_CDX_OFFSET_SHIFT 40 > > + > > +static inline u64 vfio_cdx_index_to_offset(u32 index) > > +{ > > + return ((u64)(index) << VFIO_CDX_OFFSET_SHIFT); > > +} > > + > > +struct vfio_cdx_region { > > + u32 flags; > > + u32 type; > > + u64 addr; > > + resource_size_t size; > > +}; > > + > > +struct vfio_cdx_device { > > + struct vfio_device vdev; > > + struct vfio_cdx_region *regions; > > +}; > > + > > +#endif /* VFIO_CDX_PRIVATE_H */ > > diff --git a/include/linux/cdx/cdx_bus.h b/include/linux/cdx/cdx_bus.h > > index 35ef41d8a61a..bead71b7bc73 100644 > > --- a/include/linux/cdx/cdx_bus.h > > +++ b/include/linux/cdx/cdx_bus.h > > @@ -14,7 +14,6 @@ > > #include <linux/mod_devicetable.h> > > > > #define MAX_CDX_DEV_RESOURCES 4 > > -#define CDX_ANY_ID (0xFFFF) > > #define CDX_CONTROLLER_ID_SHIFT 4 > > #define CDX_BUS_NUM_MASK 0xF > > > > diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h > > index ccaaeda792c0..ccf017353bb6 100644 > > --- a/include/linux/mod_devicetable.h > > +++ b/include/linux/mod_devicetable.h > > @@ -912,6 +912,12 @@ struct ishtp_device_id { > > kernel_ulong_t driver_data; > > }; > > > > +#define CDX_ANY_ID (0xFFFF) > > + > > +enum { > > + CDX_ID_F_VFIO_DRIVER_OVERRIDE = 1, > > +}; > > + > > /** > > * struct cdx_device_id - CDX device identifier > > * @vendor: Vendor ID > > diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h > > index 0552e8dcf0cb..8e91aaf973e7 100644 > > --- a/include/uapi/linux/vfio.h > > +++ b/include/uapi/linux/vfio.h > > @@ -213,6 +213,7 @@ struct vfio_device_info { > > #define VFIO_DEVICE_FLAGS_AP (1 << 5) /* vfio-ap device */ > > #define VFIO_DEVICE_FLAGS_FSL_MC (1 << 6) /* vfio-fsl-mc device */ > > #define VFIO_DEVICE_FLAGS_CAPS (1 << 7) /* Info supports caps */ > > +#define VFIO_DEVICE_FLAGS_CDX (1 << 8) /* vfio-cdx device */ > > __u32 num_regions; /* Max region index + 1 */ > > __u32 num_irqs; /* Max IRQ index + 1 */ > > __u32 cap_offset; /* Offset within info struct of first cap */ > > diff --git a/scripts/mod/devicetable-offsets.c b/scripts/mod/devicetable-offsets.c > > index 62dc988df84d..abe65f8968dd 100644 > > --- a/scripts/mod/devicetable-offsets.c > > +++ b/scripts/mod/devicetable-offsets.c > > @@ -265,6 +265,7 @@ int main(void) > > DEVID(cdx_device_id); > > DEVID_FIELD(cdx_device_id, vendor); > > DEVID_FIELD(cdx_device_id, device); > > + DEVID_FIELD(cdx_device_id, override_only); > > > > return 0; > > } > > diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c > > index 28da34ba4359..38120f932b0d 100644 > > --- a/scripts/mod/file2alias.c > > +++ b/scripts/mod/file2alias.c > > @@ -1458,8 +1458,23 @@ static int do_cdx_entry(const char *filename, void *symval, > > { > > DEF_FIELD(symval, cdx_device_id, vendor); > > DEF_FIELD(symval, cdx_device_id, device); > > + DEF_FIELD(symval, cdx_device_id, override_only); > > > > - sprintf(alias, "cdx:v%08Xd%08Xd", vendor, device); > > + switch (override_only) { > > + case 0: > > + strcpy(alias, "cdx:"); > > + break; > > + case CDX_ID_F_VFIO_DRIVER_OVERRIDE: > > + strcpy(alias, "vfio_cdx:"); > > + break; > > + default: > > + warn("Unknown CDX driver_override alias %08X\n", > > + override_only); > > + return 0; > > + } > > + > > + ADD(alias, "v", vendor != CDX_ANY_ID, vendor); > > + ADD(alias, "d", device != CDX_ANY_ID, device); > > return 1; > > } > > >
On 5/25/2023 1:18 AM, Alex Williamson wrote: > > On Wed, 24 May 2023 10:45:29 -0600 > Alex Williamson <alex.williamson@redhat.com> wrote: > >> On Wed, 17 May 2023 15:27:18 +0530 >> Nipun Gupta <nipun.gupta@amd.com> wrote: >> <snip> >>> + >>> +MODULE_DEVICE_TABLE(cdx, vfio_cdx_table); >>> + >>> +static struct cdx_driver vfio_cdx_driver = { >>> + .probe = vfio_cdx_probe, >>> + .remove = vfio_cdx_remove, >>> + .match_id_table = vfio_cdx_table, >>> + .driver = { >>> + .name = "vfio-cdx", >>> + .owner = THIS_MODULE, >>> + }, >>> + .driver_managed_dma = true, > > Hmm, looks like cdx bus is broken here, there's no actual > implementation of a dma_configure callback and no setup of the IOMMU > default domain for theoretical cdx drivers that might want to use the > DMA API. Without that, this driver_manged_dma flag doesn't provide any > guarantees to a vfio driver that we have exclusive ownership of the > group. Please fix, this flag needs to actually have some meaning on > cdx. Thanks, > > Alex Agree, this change was missed on CDX bus and we are working on fixing this. Shall I send this fix as a commit with this patch? Thanks, Nipun
On Mon, May 29, 2023 at 01:36:30PM +0530, Nipun Gupta wrote: > > > On 5/25/2023 1:18 AM, Alex Williamson wrote: > > > > On Wed, 24 May 2023 10:45:29 -0600 > > Alex Williamson <alex.williamson@redhat.com> wrote: > > > > > On Wed, 17 May 2023 15:27:18 +0530 > > > Nipun Gupta <nipun.gupta@amd.com> wrote: > > > > > <snip> > > > > > + > > > > +MODULE_DEVICE_TABLE(cdx, vfio_cdx_table); > > > > + > > > > +static struct cdx_driver vfio_cdx_driver = { > > > > + .probe = vfio_cdx_probe, > > > > + .remove = vfio_cdx_remove, > > > > + .match_id_table = vfio_cdx_table, > > > > + .driver = { > > > > + .name = "vfio-cdx", > > > > + .owner = THIS_MODULE, > > > > + }, > > > > + .driver_managed_dma = true, > > > > Hmm, looks like cdx bus is broken here, there's no actual > > implementation of a dma_configure callback and no setup of the IOMMU > > default domain for theoretical cdx drivers that might want to use the > > DMA API. Without that, this driver_manged_dma flag doesn't provide any > > guarantees to a vfio driver that we have exclusive ownership of the > > group. Please fix, this flag needs to actually have some meaning on > > cdx. Thanks, > > > > Alex > > Agree, this change was missed on CDX bus and we are working on fixing this. > Shall I send this fix as a commit with this patch? You should send it as a rc fixup for your already merged CDX bus stuff Jason
diff --git a/MAINTAINERS b/MAINTAINERS index a72b8fcea261..d6d1ddb854d7 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -22096,6 +22096,13 @@ F: Documentation/filesystems/vfat.rst F: fs/fat/ F: tools/testing/selftests/filesystems/fat/ +VFIO CDX DRIVER +M: Nipun Gupta <nipun.gupta@amd.com> +M: Nikhil Agarwal <nikhil.agarwal@amd.com> +L: kvm@vger.kernel.org +S: Maintained +F: drivers/vfio/cdx/* + VFIO DRIVER M: Alex Williamson <alex.williamson@redhat.com> L: kvm@vger.kernel.org diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig index 89e06c981e43..aba36f5be4ec 100644 --- a/drivers/vfio/Kconfig +++ b/drivers/vfio/Kconfig @@ -57,6 +57,7 @@ source "drivers/vfio/pci/Kconfig" source "drivers/vfio/platform/Kconfig" source "drivers/vfio/mdev/Kconfig" source "drivers/vfio/fsl-mc/Kconfig" +source "drivers/vfio/cdx/Kconfig" endif source "virt/lib/Kconfig" diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile index 70e7dcb302ef..1a27b2516612 100644 --- a/drivers/vfio/Makefile +++ b/drivers/vfio/Makefile @@ -14,3 +14,4 @@ obj-$(CONFIG_VFIO_PCI) += pci/ obj-$(CONFIG_VFIO_PLATFORM) += platform/ obj-$(CONFIG_VFIO_MDEV) += mdev/ obj-$(CONFIG_VFIO_FSL_MC) += fsl-mc/ +obj-$(CONFIG_VFIO_CDX) += cdx/ diff --git a/drivers/vfio/cdx/Kconfig b/drivers/vfio/cdx/Kconfig new file mode 100644 index 000000000000..e6de0a0caa32 --- /dev/null +++ b/drivers/vfio/cdx/Kconfig @@ -0,0 +1,17 @@ +# SPDX-License-Identifier: GPL-2.0 +# +# VFIO CDX configuration +# +# Copyright (C) 2022-2023, Advanced Micro Devices, Inc. +# + +config VFIO_CDX + tristate "VFIO support for CDX bus devices" + depends on CDX_BUS + select EVENTFD + help + Driver to enable VFIO support for the devices on CDX bus. + This is required to make use of CDX devices present in + the system using the VFIO framework. + + If you don't know what to do here, say N. diff --git a/drivers/vfio/cdx/Makefile b/drivers/vfio/cdx/Makefile new file mode 100644 index 000000000000..cd4a2e6fe609 --- /dev/null +++ b/drivers/vfio/cdx/Makefile @@ -0,0 +1,8 @@ +# SPDX-License-Identifier: GPL-2.0 +# +# Copyright (C) 2022-2023, Advanced Micro Devices, Inc. +# + +obj-$(CONFIG_VFIO_CDX) += vfio-cdx.o + +vfio-cdx-objs := main.o diff --git a/drivers/vfio/cdx/main.c b/drivers/vfio/cdx/main.c new file mode 100644 index 000000000000..f03f491e0435 --- /dev/null +++ b/drivers/vfio/cdx/main.c @@ -0,0 +1,234 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2022-2023, Advanced Micro Devices, Inc. + */ + +#include <linux/vfio.h> +#include <linux/cdx/cdx_bus.h> + +#include "private.h" + +static int vfio_cdx_open_device(struct vfio_device *core_vdev) +{ + struct vfio_cdx_device *vdev = + container_of(core_vdev, struct vfio_cdx_device, vdev); + struct cdx_device *cdx_dev = to_cdx_device(core_vdev->dev); + int count = cdx_dev->res_count; + int i; + + vdev->regions = kcalloc(count, sizeof(struct vfio_cdx_region), + GFP_KERNEL); + if (!vdev->regions) + return -ENOMEM; + + for (i = 0; i < count; i++) { + struct resource *res = &cdx_dev->res[i]; + + vdev->regions[i].addr = res->start; + vdev->regions[i].size = resource_size(res); + vdev->regions[i].type = res->flags; + /* + * Only regions addressed with PAGE granularity may be + * MMAP'ed securely. + */ + if (!(vdev->regions[i].addr & ~PAGE_MASK) && + !(vdev->regions[i].size & ~PAGE_MASK)) + vdev->regions[i].flags |= + VFIO_REGION_INFO_FLAG_MMAP; + vdev->regions[i].flags |= VFIO_REGION_INFO_FLAG_READ; + if (!(cdx_dev->res[i].flags & IORESOURCE_READONLY)) + vdev->regions[i].flags |= VFIO_REGION_INFO_FLAG_WRITE; + } + + return 0; +} + +static void vfio_cdx_close_device(struct vfio_device *core_vdev) +{ + struct vfio_cdx_device *vdev = + container_of(core_vdev, struct vfio_cdx_device, vdev); + + kfree(vdev->regions); + cdx_dev_reset(core_vdev->dev); +} + +static int vfio_cdx_ioctl_get_info(struct vfio_cdx_device *vdev, + struct vfio_device_info __user *arg) +{ + unsigned long minsz = offsetofend(struct vfio_device_info, num_irqs); + struct cdx_device *cdx_dev = to_cdx_device(vdev->vdev.dev); + struct vfio_device_info info; + + if (copy_from_user(&info, arg, minsz)) + return -EFAULT; + + if (info.argsz < minsz) + return -EINVAL; + + info.flags = VFIO_DEVICE_FLAGS_CDX; + info.flags |= VFIO_DEVICE_FLAGS_RESET; + + info.num_regions = cdx_dev->res_count; + info.num_irqs = 0; + + return copy_to_user(arg, &info, minsz) ? -EFAULT : 0; +} + +static int vfio_cdx_ioctl_get_region_info(struct vfio_cdx_device *vdev, + struct vfio_region_info __user *arg) +{ + unsigned long minsz = offsetofend(struct vfio_region_info, offset); + struct cdx_device *cdx_dev = to_cdx_device(vdev->vdev.dev); + struct vfio_region_info info; + + if (copy_from_user(&info, arg, minsz)) + return -EFAULT; + + if (info.argsz < minsz) + return -EINVAL; + + if (info.index >= cdx_dev->res_count) + return -EINVAL; + + /* map offset to the physical address */ + info.offset = vfio_cdx_index_to_offset(info.index); + info.size = vdev->regions[info.index].size; + info.flags = vdev->regions[info.index].flags; + + return copy_to_user(arg, &info, minsz) ? -EFAULT : 0; +} + +static long vfio_cdx_ioctl(struct vfio_device *core_vdev, + unsigned int cmd, unsigned long arg) +{ + struct vfio_cdx_device *vdev = + container_of(core_vdev, struct vfio_cdx_device, vdev); + void __user *uarg = (void __user *)arg; + + switch (cmd) { + case VFIO_DEVICE_GET_INFO: + return vfio_cdx_ioctl_get_info(vdev, uarg); + case VFIO_DEVICE_GET_REGION_INFO: + return vfio_cdx_ioctl_get_region_info(vdev, uarg); + case VFIO_DEVICE_RESET: + return cdx_dev_reset(core_vdev->dev); + default: + return -ENOTTY; + } +} + +static int vfio_cdx_mmap_mmio(struct vfio_cdx_region region, + struct vm_area_struct *vma) +{ + u64 size = vma->vm_end - vma->vm_start; + u64 pgoff, base; + + pgoff = vma->vm_pgoff & + ((1U << (VFIO_CDX_OFFSET_SHIFT - PAGE_SHIFT)) - 1); + base = pgoff << PAGE_SHIFT; + + if (region.size < PAGE_SIZE || base + size > region.size) + return -EINVAL; + + vma->vm_pgoff = (region.addr >> PAGE_SHIFT) + pgoff; + vma->vm_page_prot = pgprot_device(vma->vm_page_prot); + + return io_remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff, + size, vma->vm_page_prot); +} + +static int vfio_cdx_mmap(struct vfio_device *core_vdev, + struct vm_area_struct *vma) +{ + struct vfio_cdx_device *vdev = + container_of(core_vdev, struct vfio_cdx_device, vdev); + struct cdx_device *cdx_dev = to_cdx_device(core_vdev->dev); + unsigned int index; + + index = vma->vm_pgoff >> (VFIO_CDX_OFFSET_SHIFT - PAGE_SHIFT); + + if (index >= cdx_dev->res_count) + return -EINVAL; + + if (!(vdev->regions[index].flags & VFIO_REGION_INFO_FLAG_MMAP)) + return -EINVAL; + + if (!(vdev->regions[index].flags & VFIO_REGION_INFO_FLAG_READ) && + (vma->vm_flags & VM_READ)) + return -EINVAL; + + if (!(vdev->regions[index].flags & VFIO_REGION_INFO_FLAG_WRITE) && + (vma->vm_flags & VM_WRITE)) + return -EINVAL; + + return vfio_cdx_mmap_mmio(vdev->regions[index], vma); +} + +static const struct vfio_device_ops vfio_cdx_ops = { + .name = "vfio-cdx", + .open_device = vfio_cdx_open_device, + .close_device = vfio_cdx_close_device, + .ioctl = vfio_cdx_ioctl, + .mmap = vfio_cdx_mmap, + .bind_iommufd = vfio_iommufd_physical_bind, + .unbind_iommufd = vfio_iommufd_physical_unbind, + .attach_ioas = vfio_iommufd_physical_attach_ioas, +}; + +static int vfio_cdx_probe(struct cdx_device *cdx_dev) +{ + struct vfio_cdx_device *vdev = NULL; + struct device *dev = &cdx_dev->dev; + int ret; + + vdev = vfio_alloc_device(vfio_cdx_device, vdev, dev, + &vfio_cdx_ops); + if (IS_ERR(vdev)) + return PTR_ERR(vdev); + + ret = vfio_register_group_dev(&vdev->vdev); + if (ret) + goto out_uninit; + + dev_set_drvdata(dev, vdev); + return 0; + +out_uninit: + vfio_put_device(&vdev->vdev); + return ret; +} + +static int vfio_cdx_remove(struct cdx_device *cdx_dev) +{ + struct device *dev = &cdx_dev->dev; + struct vfio_cdx_device *vdev = dev_get_drvdata(dev); + + vfio_unregister_group_dev(&vdev->vdev); + vfio_put_device(&vdev->vdev); + + return 0; +} + +static const struct cdx_device_id vfio_cdx_table[] = { + { CDX_DEVICE_DRIVER_OVERRIDE(CDX_ANY_ID, CDX_ANY_ID, + CDX_ID_F_VFIO_DRIVER_OVERRIDE) }, /* match all by default */ + {} +}; + +MODULE_DEVICE_TABLE(cdx, vfio_cdx_table); + +static struct cdx_driver vfio_cdx_driver = { + .probe = vfio_cdx_probe, + .remove = vfio_cdx_remove, + .match_id_table = vfio_cdx_table, + .driver = { + .name = "vfio-cdx", + .owner = THIS_MODULE, + }, + .driver_managed_dma = true, +}; + +module_driver(vfio_cdx_driver, cdx_driver_register, cdx_driver_unregister); + +MODULE_LICENSE("GPL"); +MODULE_DESCRIPTION("VFIO for CDX devices - User Level meta-driver"); diff --git a/drivers/vfio/cdx/private.h b/drivers/vfio/cdx/private.h new file mode 100644 index 000000000000..8bdc117ea88e --- /dev/null +++ b/drivers/vfio/cdx/private.h @@ -0,0 +1,28 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (C) 2022-2023, Advanced Micro Devices, Inc. + */ + +#ifndef VFIO_CDX_PRIVATE_H +#define VFIO_CDX_PRIVATE_H + +#define VFIO_CDX_OFFSET_SHIFT 40 + +static inline u64 vfio_cdx_index_to_offset(u32 index) +{ + return ((u64)(index) << VFIO_CDX_OFFSET_SHIFT); +} + +struct vfio_cdx_region { + u32 flags; + u32 type; + u64 addr; + resource_size_t size; +}; + +struct vfio_cdx_device { + struct vfio_device vdev; + struct vfio_cdx_region *regions; +}; + +#endif /* VFIO_CDX_PRIVATE_H */ diff --git a/include/linux/cdx/cdx_bus.h b/include/linux/cdx/cdx_bus.h index 35ef41d8a61a..bead71b7bc73 100644 --- a/include/linux/cdx/cdx_bus.h +++ b/include/linux/cdx/cdx_bus.h @@ -14,7 +14,6 @@ #include <linux/mod_devicetable.h> #define MAX_CDX_DEV_RESOURCES 4 -#define CDX_ANY_ID (0xFFFF) #define CDX_CONTROLLER_ID_SHIFT 4 #define CDX_BUS_NUM_MASK 0xF diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h index ccaaeda792c0..ccf017353bb6 100644 --- a/include/linux/mod_devicetable.h +++ b/include/linux/mod_devicetable.h @@ -912,6 +912,12 @@ struct ishtp_device_id { kernel_ulong_t driver_data; }; +#define CDX_ANY_ID (0xFFFF) + +enum { + CDX_ID_F_VFIO_DRIVER_OVERRIDE = 1, +}; + /** * struct cdx_device_id - CDX device identifier * @vendor: Vendor ID diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h index 0552e8dcf0cb..8e91aaf973e7 100644 --- a/include/uapi/linux/vfio.h +++ b/include/uapi/linux/vfio.h @@ -213,6 +213,7 @@ struct vfio_device_info { #define VFIO_DEVICE_FLAGS_AP (1 << 5) /* vfio-ap device */ #define VFIO_DEVICE_FLAGS_FSL_MC (1 << 6) /* vfio-fsl-mc device */ #define VFIO_DEVICE_FLAGS_CAPS (1 << 7) /* Info supports caps */ +#define VFIO_DEVICE_FLAGS_CDX (1 << 8) /* vfio-cdx device */ __u32 num_regions; /* Max region index + 1 */ __u32 num_irqs; /* Max IRQ index + 1 */ __u32 cap_offset; /* Offset within info struct of first cap */ diff --git a/scripts/mod/devicetable-offsets.c b/scripts/mod/devicetable-offsets.c index 62dc988df84d..abe65f8968dd 100644 --- a/scripts/mod/devicetable-offsets.c +++ b/scripts/mod/devicetable-offsets.c @@ -265,6 +265,7 @@ int main(void) DEVID(cdx_device_id); DEVID_FIELD(cdx_device_id, vendor); DEVID_FIELD(cdx_device_id, device); + DEVID_FIELD(cdx_device_id, override_only); return 0; } diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c index 28da34ba4359..38120f932b0d 100644 --- a/scripts/mod/file2alias.c +++ b/scripts/mod/file2alias.c @@ -1458,8 +1458,23 @@ static int do_cdx_entry(const char *filename, void *symval, { DEF_FIELD(symval, cdx_device_id, vendor); DEF_FIELD(symval, cdx_device_id, device); + DEF_FIELD(symval, cdx_device_id, override_only); - sprintf(alias, "cdx:v%08Xd%08Xd", vendor, device); + switch (override_only) { + case 0: + strcpy(alias, "cdx:"); + break; + case CDX_ID_F_VFIO_DRIVER_OVERRIDE: + strcpy(alias, "vfio_cdx:"); + break; + default: + warn("Unknown CDX driver_override alias %08X\n", + override_only); + return 0; + } + + ADD(alias, "v", vendor != CDX_ANY_ID, vendor); + ADD(alias, "d", device != CDX_ANY_ID, device); return 1; }