| Message ID | 20230414123414.21645-1-nipun.gupta@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] vfio/cdx: add support for CDX bus | expand |
On Fri, 14 Apr 2023 18:04:14 +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. > > 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 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/vfio_cdx.c | 290 ++++++++++++++++++++++++++++ > drivers/vfio/cdx/vfio_cdx_private.h | 31 +++ > include/linux/cdx/cdx_bus.h | 1 - > include/linux/mod_devicetable.h | 6 + > scripts/mod/devicetable-offsets.c | 1 + > scripts/mod/file2alias.c | 17 +- > 11 files changed, 378 insertions(+), 2 deletions(-) > create mode 100644 drivers/vfio/cdx/Kconfig > create mode 100644 drivers/vfio/cdx/Makefile > create mode 100644 drivers/vfio/cdx/vfio_cdx.c > create mode 100644 drivers/vfio/cdx/vfio_cdx_private.h > > diff --git a/MAINTAINERS b/MAINTAINERS > index 7f74d8571ac9..c4fd42ba8f46 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -22064,6 +22064,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..82e4ef412c0f > --- /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 := vfio_cdx.o > diff --git a/drivers/vfio/cdx/vfio_cdx.c b/drivers/vfio/cdx/vfio_cdx.c > new file mode 100644 > index 000000000000..c19062ce7680 > --- /dev/null > +++ b/drivers/vfio/cdx/vfio_cdx.c > @@ -0,0 +1,290 @@ > +// 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 "vfio_cdx_private.h" > + > +static struct cdx_driver vfio_cdx_driver; > + > +static int vfio_cdx_init_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); > + > + vdev->cdx_dev = cdx_dev; > + vdev->dev = &cdx_dev->dev; Both of these seem trivial to factor out of this patch, cdx_device is always available via to_cdx_device(core_vdev->dev) and the struct device is always available via core_vdev->dev. vdev->dev isn't even used anywhere yet. Both the init and release functions here could be dropped afaict. > + > + return 0; > +} > + > +static void vfio_cdx_release_device(struct vfio_device *core_vdev) > +{ > + struct vfio_cdx_device *vdev = > + container_of(core_vdev, struct vfio_cdx_device, vdev); > + > + vdev->cdx_dev = NULL; > + vdev->dev = NULL; > +} > + > +/** > + * CDX_DRIVER_OVERRIDE_DEVICE_VFIO - macro used to describe a VFIO > + * "driver_override" CDX device. > + * @vend: the 16 bit CDX Vendor ID > + * @dev: the 16 bit CDX Device ID > + * > + * This macro is used to create a struct cdx_device_id that matches a > + * specific device. driver_override will be set to > + * CDX_ID_F_VFIO_DRIVER_OVERRIDE. > + */ > +#define CDX_DRIVER_OVERRIDE_DEVICE_VFIO(vend, dev) \ > + CDX_DEVICE_DRIVER_OVERRIDE(vend, dev, CDX_ID_F_VFIO_DRIVER_OVERRIDE) > + > +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 = vdev->cdx_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_regions_cleanup(struct vfio_cdx_device *vdev) > +{ > + kfree(vdev->regions); > +} > + > +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); > + int ret; > + > + vfio_cdx_regions_cleanup(vdev); > + > + /* reset the device before cleaning up the interrupts */ > + ret = cdx_dev_reset(&vdev->cdx_dev->dev); > + if (WARN_ON(ret)) > + dev_warn(core_vdev->dev, > + "VFIO_CDX: reset device has failed (%d)\n", ret); WARN_ON() + dev_warn(), do we need both? Curious we're not even using vdev->dev here ;) > +} > + > +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); > + struct cdx_device *cdx_dev = vdev->cdx_dev; > + unsigned long minsz; > + > + switch (cmd) { > + case VFIO_DEVICE_GET_INFO: > + { > + struct vfio_device_info info; > + > + minsz = offsetofend(struct vfio_device_info, num_irqs); > + > + if (copy_from_user(&info, (void __user *)arg, minsz)) > + return -EFAULT; > + > + if (info.argsz < minsz) > + return -EINVAL; > + > + info.flags = VFIO_DEVICE_FLAGS_RESET; There ought to be a flag here indicating the vfio device exposed is vfio-cdx. > + > + info.num_regions = cdx_dev->res_count; > + info.num_irqs = 0; > + > + return copy_to_user((void __user *)arg, &info, minsz) ? > + -EFAULT : 0; > + } > + case VFIO_DEVICE_GET_REGION_INFO: > + { > + struct vfio_region_info info; > + > + minsz = offsetofend(struct vfio_region_info, offset); > + > + if (copy_from_user(&info, (void __user *)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; > + > + if (copy_to_user((void __user *)arg, &info, minsz)) > + return -EFAULT; > + return 0; > + } > + case VFIO_DEVICE_RESET: > + { > + return cdx_dev_reset(&vdev->cdx_dev->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 = vdev->cdx_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", > + .init = vfio_cdx_init_device, > + .release = vfio_cdx_release_device, > + .open_device = vfio_cdx_open_device, > + .close_device = vfio_cdx_close_device, > + .ioctl = vfio_cdx_ioctl, > + .mmap = vfio_cdx_mmap, Missing bind_iommufd, unbind_iommufd, and attach_ioas, which will be necessary for iommufd support. > +}; > + > +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) { > + dev_err(dev, "VFIO_CDX: Failed to add to vfio group\n"); > + 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; > + > + 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_DRIVER_OVERRIDE_DEVICE_VFIO(CDX_ANY_ID, CDX_ANY_ID) }, /* 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, > +}; > + > +static int __init vfio_cdx_driver_init(void) > +{ > + return cdx_driver_register(&vfio_cdx_driver); > +} > + > +static void __exit vfio_cdx_driver_exit(void) > +{ > + cdx_driver_unregister(&vfio_cdx_driver); > +} > + > +module_init(vfio_cdx_driver_init); > +module_exit(vfio_cdx_driver_exit); > + > +MODULE_LICENSE("GPL"); > +MODULE_DESCRIPTION("VFIO for CDX devices - User Level meta-driver"); > diff --git a/drivers/vfio/cdx/vfio_cdx_private.h b/drivers/vfio/cdx/vfio_cdx_private.h > new file mode 100644 > index 000000000000..5f143736eb7a > --- /dev/null > +++ b/drivers/vfio/cdx/vfio_cdx_private.h > @@ -0,0 +1,31 @@ > +/* 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; > + void __iomem *ioaddr; I don't see that ioaddr is used in this patch. Future r/w support? Thanks, Alex > +}; > + > +struct vfio_cdx_device { > + struct vfio_device vdev; > + struct cdx_device *cdx_dev; > + struct device *dev; > + 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/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; > } >
> -----Original Message----- > From: Alex Williamson <alex.williamson@redhat.com> > Sent: Saturday, April 15, 2023 3:06 AM > To: Gupta, Nipun <Nipun.Gupta@amd.com> > Cc: jgg@ziepe.ca; linux-kernel@vger.kernel.org; kvm@vger.kernel.org; > masahiroy@kernel.org; nathan@kernel.org; ndesaulniers@google.com; > nicolas@fjasle.eu; git (AMD-Xilinx) <git@amd.com>; Anand, Harpreet > <harpreet.anand@amd.com>; Jansen Van Vuuren, Pieter <pieter.jansen-van- > vuuren@amd.com>; Agarwal, Nikhil <nikhil.agarwal@amd.com>; Simek, Michal > <michal.simek@amd.com> > Subject: Re: [PATCH v2] vfio/cdx: add support for CDX bus > > Caution: This message originated from an External Source. Use proper caution > when opening attachments, clicking links, or responding. > > > On Fri, 14 Apr 2023 18:04:14 +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. > > > > 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 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/vfio_cdx.c | 290 ++++++++++++++++++++++++++++ > > drivers/vfio/cdx/vfio_cdx_private.h | 31 +++ > > include/linux/cdx/cdx_bus.h | 1 - > > include/linux/mod_devicetable.h | 6 + > > scripts/mod/devicetable-offsets.c | 1 + > > scripts/mod/file2alias.c | 17 +- > > 11 files changed, 378 insertions(+), 2 deletions(-) > > create mode 100644 drivers/vfio/cdx/Kconfig > > create mode 100644 drivers/vfio/cdx/Makefile > > create mode 100644 drivers/vfio/cdx/vfio_cdx.c > > create mode 100644 drivers/vfio/cdx/vfio_cdx_private.h > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 7f74d8571ac9..c4fd42ba8f46 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -22064,6 +22064,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..82e4ef412c0f > > --- /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 := vfio_cdx.o > > diff --git a/drivers/vfio/cdx/vfio_cdx.c b/drivers/vfio/cdx/vfio_cdx.c > > new file mode 100644 > > index 000000000000..c19062ce7680 > > --- /dev/null > > +++ b/drivers/vfio/cdx/vfio_cdx.c > > @@ -0,0 +1,290 @@ > > +// 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 "vfio_cdx_private.h" > > + > > +static struct cdx_driver vfio_cdx_driver; > > + > > +static int vfio_cdx_init_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); > > + > > + vdev->cdx_dev = cdx_dev; > > + vdev->dev = &cdx_dev->dev; > > Both of these seem trivial to factor out of this patch, cdx_device is > always available via to_cdx_device(core_vdev->dev) and the struct > device is always available via core_vdev->dev. vdev->dev isn't even > used anywhere yet. Both the init and release functions here could be > dropped afaict. Agree, these functions can be removed. Will update, test, and send in next spin. > > > + > > + return 0; > > +} > > + > > +static void vfio_cdx_release_device(struct vfio_device *core_vdev) > > +{ > > + struct vfio_cdx_device *vdev = > > + container_of(core_vdev, struct vfio_cdx_device, vdev); > > + > > + vdev->cdx_dev = NULL; > > + vdev->dev = NULL; > > +} > > + > > +/** > > + * CDX_DRIVER_OVERRIDE_DEVICE_VFIO - macro used to describe a VFIO > > + * "driver_override" CDX device. > > + * @vend: the 16 bit CDX Vendor ID > > + * @dev: the 16 bit CDX Device ID > > + * > > + * This macro is used to create a struct cdx_device_id that matches a > > + * specific device. driver_override will be set to > > + * CDX_ID_F_VFIO_DRIVER_OVERRIDE. > > + */ > > +#define CDX_DRIVER_OVERRIDE_DEVICE_VFIO(vend, dev) \ > > + CDX_DEVICE_DRIVER_OVERRIDE(vend, dev, > CDX_ID_F_VFIO_DRIVER_OVERRIDE) > > + > > +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 = vdev->cdx_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_regions_cleanup(struct vfio_cdx_device *vdev) > > +{ > > + kfree(vdev->regions); > > +} > > + > > +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); > > + int ret; > > + > > + vfio_cdx_regions_cleanup(vdev); > > + > > + /* reset the device before cleaning up the interrupts */ > > + ret = cdx_dev_reset(&vdev->cdx_dev->dev); > > + if (WARN_ON(ret)) > > + dev_warn(core_vdev->dev, > > + "VFIO_CDX: reset device has failed (%d)\n", ret); > > WARN_ON() + dev_warn(), do we need both? Curious we're not even using > vdev->dev here ;) WARN_ON will be removed and we will keep using core_vdev->dev as vdev->dev will be removed :) > > > +} > > + > > +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); > > + struct cdx_device *cdx_dev = vdev->cdx_dev; > > + unsigned long minsz; > > + > > + switch (cmd) { > > + case VFIO_DEVICE_GET_INFO: > > + { > > + struct vfio_device_info info; > > + > > + minsz = offsetofend(struct vfio_device_info, num_irqs); > > + > > + if (copy_from_user(&info, (void __user *)arg, minsz)) > > + return -EFAULT; > > + > > + if (info.argsz < minsz) > > + return -EINVAL; > > + > > + info.flags = VFIO_DEVICE_FLAGS_RESET; > > There ought to be a flag here indicating the vfio device exposed is > vfio-cdx. Sure, will add. > > > + > > + info.num_regions = cdx_dev->res_count; > > + info.num_irqs = 0; > > + > > + return copy_to_user((void __user *)arg, &info, minsz) ? > > + -EFAULT : 0; > > + } > > + case VFIO_DEVICE_GET_REGION_INFO: > > + { > > + struct vfio_region_info info; > > + > > + minsz = offsetofend(struct vfio_region_info, offset); > > + > > + if (copy_from_user(&info, (void __user *)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; > > + > > + if (copy_to_user((void __user *)arg, &info, minsz)) > > + return -EFAULT; > > + return 0; > > + } > > + case VFIO_DEVICE_RESET: > > + { > > + return cdx_dev_reset(&vdev->cdx_dev->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 = vdev->cdx_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", > > + .init = vfio_cdx_init_device, > > + .release = vfio_cdx_release_device, > > + .open_device = vfio_cdx_open_device, > > + .close_device = vfio_cdx_close_device, > > + .ioctl = vfio_cdx_ioctl, > > + .mmap = vfio_cdx_mmap, > > Missing bind_iommufd, unbind_iommufd, and attach_ioas, which will be > necessary for iommufd support. Sure, will add these too. > > > +}; > > + > > +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) { > > + dev_err(dev, "VFIO_CDX: Failed to add to vfio group\n"); > > + 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; > > + > > + 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_DRIVER_OVERRIDE_DEVICE_VFIO(CDX_ANY_ID, CDX_ANY_ID) }, /* > 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, > > +}; > > + > > +static int __init vfio_cdx_driver_init(void) > > +{ > > + return cdx_driver_register(&vfio_cdx_driver); > > +} > > + > > +static void __exit vfio_cdx_driver_exit(void) > > +{ > > + cdx_driver_unregister(&vfio_cdx_driver); > > +} > > + > > +module_init(vfio_cdx_driver_init); > > +module_exit(vfio_cdx_driver_exit); > > + > > +MODULE_LICENSE("GPL"); > > +MODULE_DESCRIPTION("VFIO for CDX devices - User Level meta-driver"); > > diff --git a/drivers/vfio/cdx/vfio_cdx_private.h > b/drivers/vfio/cdx/vfio_cdx_private.h > > new file mode 100644 > > index 000000000000..5f143736eb7a > > --- /dev/null > > +++ b/drivers/vfio/cdx/vfio_cdx_private.h > > @@ -0,0 +1,31 @@ > > +/* 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; > > + void __iomem *ioaddr; > > I don't see that ioaddr is used in this patch. Future r/w support? Agree, will remove this from this patch. Thanks, Nipun > Thanks, > > Alex > > > +}; > > + > > +struct vfio_cdx_device { > > + struct vfio_device vdev; > > + struct cdx_device *cdx_dev; > > + struct device *dev; > > + 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/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 Fri, Apr 14, 2023 at 03:36:14PM -0600, Alex Williamson wrote: > > +static int vfio_cdx_init_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); > > + > > + vdev->cdx_dev = cdx_dev; > > + vdev->dev = &cdx_dev->dev; > > Both of these seem trivial to factor out of this patch, cdx_device is > always available via to_cdx_device(core_vdev->dev) and the struct > device is always available via core_vdev->dev. vdev->dev isn't even > used anywhere yet. Both the init and release functions here could be > dropped afaict. Yes please, I have a series someplace that gets rid of all these redundent ->devs we keep around everwhere. to_cdx_device(core_vdev->dev) is a good solution, maybe with a static inline. Jason
[AMD Official Use Only - General] > -----Original Message----- > From: Jason Gunthorpe <jgg@ziepe.ca> > Sent: Monday, April 17, 2023 6:49 PM > To: Alex Williamson <alex.williamson@redhat.com> > Cc: Gupta, Nipun <Nipun.Gupta@amd.com>; linux-kernel@vger.kernel.org; > kvm@vger.kernel.org; masahiroy@kernel.org; nathan@kernel.org; > ndesaulniers@google.com; nicolas@fjasle.eu; git (AMD-Xilinx) <git@amd.com>; > Anand, Harpreet <harpreet.anand@amd.com>; Jansen Van Vuuren, Pieter > <pieter.jansen-van-vuuren@amd.com>; Agarwal, Nikhil > <nikhil.agarwal@amd.com>; Simek, Michal <michal.simek@amd.com> > Subject: Re: [PATCH v2] vfio/cdx: add support for CDX bus > > Caution: This message originated from an External Source. Use proper caution > when opening attachments, clicking links, or responding. > > > On Fri, Apr 14, 2023 at 03:36:14PM -0600, Alex Williamson wrote: > > > > +static int vfio_cdx_init_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); > > > + > > > + vdev->cdx_dev = cdx_dev; > > > + vdev->dev = &cdx_dev->dev; > > > > Both of these seem trivial to factor out of this patch, cdx_device is > > always available via to_cdx_device(core_vdev->dev) and the struct > > device is always available via core_vdev->dev. vdev->dev isn't even > > used anywhere yet. Both the init and release functions here could be > > dropped afaict. > > Yes please, I have a series someplace that gets rid of all these > redundent ->devs we keep around everwhere. > > to_cdx_device(core_vdev->dev) is a good solution, maybe with a static > inline. Agree, we have already rolled out v3 which has this change i.e using to_cdx_dev(core_vdev->dev) instead of storing these again. Thanks, Nipun > > Jason
diff --git a/MAINTAINERS b/MAINTAINERS index 7f74d8571ac9..c4fd42ba8f46 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -22064,6 +22064,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..82e4ef412c0f --- /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 := vfio_cdx.o diff --git a/drivers/vfio/cdx/vfio_cdx.c b/drivers/vfio/cdx/vfio_cdx.c new file mode 100644 index 000000000000..c19062ce7680 --- /dev/null +++ b/drivers/vfio/cdx/vfio_cdx.c @@ -0,0 +1,290 @@ +// 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 "vfio_cdx_private.h" + +static struct cdx_driver vfio_cdx_driver; + +static int vfio_cdx_init_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); + + vdev->cdx_dev = cdx_dev; + vdev->dev = &cdx_dev->dev; + + return 0; +} + +static void vfio_cdx_release_device(struct vfio_device *core_vdev) +{ + struct vfio_cdx_device *vdev = + container_of(core_vdev, struct vfio_cdx_device, vdev); + + vdev->cdx_dev = NULL; + vdev->dev = NULL; +} + +/** + * CDX_DRIVER_OVERRIDE_DEVICE_VFIO - macro used to describe a VFIO + * "driver_override" CDX device. + * @vend: the 16 bit CDX Vendor ID + * @dev: the 16 bit CDX Device ID + * + * This macro is used to create a struct cdx_device_id that matches a + * specific device. driver_override will be set to + * CDX_ID_F_VFIO_DRIVER_OVERRIDE. + */ +#define CDX_DRIVER_OVERRIDE_DEVICE_VFIO(vend, dev) \ + CDX_DEVICE_DRIVER_OVERRIDE(vend, dev, CDX_ID_F_VFIO_DRIVER_OVERRIDE) + +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 = vdev->cdx_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_regions_cleanup(struct vfio_cdx_device *vdev) +{ + kfree(vdev->regions); +} + +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); + int ret; + + vfio_cdx_regions_cleanup(vdev); + + /* reset the device before cleaning up the interrupts */ + ret = cdx_dev_reset(&vdev->cdx_dev->dev); + if (WARN_ON(ret)) + dev_warn(core_vdev->dev, + "VFIO_CDX: reset device has failed (%d)\n", ret); +} + +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); + struct cdx_device *cdx_dev = vdev->cdx_dev; + unsigned long minsz; + + switch (cmd) { + case VFIO_DEVICE_GET_INFO: + { + struct vfio_device_info info; + + minsz = offsetofend(struct vfio_device_info, num_irqs); + + if (copy_from_user(&info, (void __user *)arg, minsz)) + return -EFAULT; + + if (info.argsz < minsz) + return -EINVAL; + + info.flags = VFIO_DEVICE_FLAGS_RESET; + + info.num_regions = cdx_dev->res_count; + info.num_irqs = 0; + + return copy_to_user((void __user *)arg, &info, minsz) ? + -EFAULT : 0; + } + case VFIO_DEVICE_GET_REGION_INFO: + { + struct vfio_region_info info; + + minsz = offsetofend(struct vfio_region_info, offset); + + if (copy_from_user(&info, (void __user *)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; + + if (copy_to_user((void __user *)arg, &info, minsz)) + return -EFAULT; + return 0; + } + case VFIO_DEVICE_RESET: + { + return cdx_dev_reset(&vdev->cdx_dev->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 = vdev->cdx_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", + .init = vfio_cdx_init_device, + .release = vfio_cdx_release_device, + .open_device = vfio_cdx_open_device, + .close_device = vfio_cdx_close_device, + .ioctl = vfio_cdx_ioctl, + .mmap = vfio_cdx_mmap, +}; + +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) { + dev_err(dev, "VFIO_CDX: Failed to add to vfio group\n"); + 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; + + 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_DRIVER_OVERRIDE_DEVICE_VFIO(CDX_ANY_ID, CDX_ANY_ID) }, /* 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, +}; + +static int __init vfio_cdx_driver_init(void) +{ + return cdx_driver_register(&vfio_cdx_driver); +} + +static void __exit vfio_cdx_driver_exit(void) +{ + cdx_driver_unregister(&vfio_cdx_driver); +} + +module_init(vfio_cdx_driver_init); +module_exit(vfio_cdx_driver_exit); + +MODULE_LICENSE("GPL"); +MODULE_DESCRIPTION("VFIO for CDX devices - User Level meta-driver"); diff --git a/drivers/vfio/cdx/vfio_cdx_private.h b/drivers/vfio/cdx/vfio_cdx_private.h new file mode 100644 index 000000000000..5f143736eb7a --- /dev/null +++ b/drivers/vfio/cdx/vfio_cdx_private.h @@ -0,0 +1,31 @@ +/* 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; + void __iomem *ioaddr; +}; + +struct vfio_cdx_device { + struct vfio_device vdev; + struct cdx_device *cdx_dev; + struct device *dev; + 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/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; }