From fbc9d37161b7d7e0e5f838445bbeff721dc4fc8d Mon Sep 17 00:00:00 2001 From: Bhaskar Chowdhury Date: Tue, 30 Mar 2021 09:54:21 -0600 Subject: vfio: pci: Spello fix in the file vfio_pci.c s/permision/permission/ Signed-off-by: Bhaskar Chowdhury Acked-by: Randy Dunlap Message-Id: <20210314052925.3560-1-unixbhaskar@gmail.com> Signed-off-by: Alex Williamson --- drivers/vfio/pci/vfio_pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c index 65e7e6b44578..d2ab8b5bc8a8 100644 --- a/drivers/vfio/pci/vfio_pci.c +++ b/drivers/vfio/pci/vfio_pci.c @@ -2409,7 +2409,7 @@ static int __init vfio_pci_init(void) { int ret; - /* Allocate shared config space permision data used by all devices */ + /* Allocate shared config space permission data used by all devices */ ret = vfio_pci_init_perm_bits(); if (ret) return ret; -- cgit v1.2.3 From 36f0be5a30bbe6f1783db459449f2d88c5cd1e34 Mon Sep 17 00:00:00 2001 From: Zhou Wang Date: Tue, 30 Mar 2021 09:54:21 -0600 Subject: vfio/pci: Remove an unnecessary blank line in vfio_pci_enable This blank line is unnecessary, so remove it. Signed-off-by: Zhou Wang Message-Id: <1615808073-178604-1-git-send-email-wangzhou1@hisilicon.com> Signed-off-by: Alex Williamson --- drivers/vfio/pci/vfio_pci.c | 1 - 1 file changed, 1 deletion(-) (limited to 'drivers') diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c index d2ab8b5bc8a8..37a4a4824e30 100644 --- a/drivers/vfio/pci/vfio_pci.c +++ b/drivers/vfio/pci/vfio_pci.c @@ -378,7 +378,6 @@ static int vfio_pci_enable(struct vfio_pci_device *vdev) if (!vfio_vga_disabled() && vfio_pci_is_vga(pdev)) vdev->has_vga = true; - if (vfio_pci_is_vga(pdev) && pdev->vendor == PCI_VENDOR_ID_INTEL && IS_ENABLED(CONFIG_VFIO_PCI_IGD)) { -- cgit v1.2.3 From bab2c1990b78b90d7e1cffbb05ccf1009a55f0d3 Mon Sep 17 00:00:00 2001 From: Fred Gao Date: Tue, 30 Mar 2021 09:54:21 -0600 Subject: vfio/pci: Add support for opregion v2.1+ Before opregion version 2.0 VBT data is stored in opregion mailbox #4, but when VBT data exceeds 6KB size and cannot be within mailbox #4 then from opregion v2.0+, Extended VBT region, next to opregion is used to hold the VBT data, so the total size will be opregion size plus extended VBT region size. Since opregion v2.0 with physical host VBT address would not be practically available for end user and guest can not directly access host physical address, so it is not supported. Cc: Zhenyu Wang Signed-off-by: Swee Yee Fonn Signed-off-by: Fred Gao Message-Id: <20210325170953.24549-1-fred.gao@intel.com> Reviewed-by: Zhenyu Wang Signed-off-by: Alex Williamson --- drivers/vfio/pci/vfio_pci_igd.c | 53 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) (limited to 'drivers') diff --git a/drivers/vfio/pci/vfio_pci_igd.c b/drivers/vfio/pci/vfio_pci_igd.c index e66dfb0178ed..228df565e9bc 100644 --- a/drivers/vfio/pci/vfio_pci_igd.c +++ b/drivers/vfio/pci/vfio_pci_igd.c @@ -21,6 +21,10 @@ #define OPREGION_SIZE (8 * 1024) #define OPREGION_PCI_ADDR 0xfc +#define OPREGION_RVDA 0x3ba +#define OPREGION_RVDS 0x3c2 +#define OPREGION_VERSION 0x16 + static size_t vfio_pci_igd_rw(struct vfio_pci_device *vdev, char __user *buf, size_t count, loff_t *ppos, bool iswrite) { @@ -58,6 +62,7 @@ static int vfio_pci_igd_opregion_init(struct vfio_pci_device *vdev) u32 addr, size; void *base; int ret; + u16 version; ret = pci_read_config_dword(vdev->pdev, OPREGION_PCI_ADDR, &addr); if (ret) @@ -83,6 +88,54 @@ static int vfio_pci_igd_opregion_init(struct vfio_pci_device *vdev) size *= 1024; /* In KB */ + /* + * Support opregion v2.1+ + * When VBT data exceeds 6KB size and cannot be within mailbox #4, then + * the Extended VBT region next to opregion is used to hold the VBT data. + * RVDA (Relative Address of VBT Data from Opregion Base) and RVDS + * (Raw VBT Data Size) from opregion structure member are used to hold the + * address from region base and size of VBT data. RVDA/RVDS are not + * defined before opregion 2.0. + * + * opregion 2.1+: RVDA is unsigned, relative offset from + * opregion base, and should point to the end of opregion. + * otherwise, exposing to userspace to allow read access to everything between + * the OpRegion and VBT is not safe. + * RVDS is defined as size in bytes. + * + * opregion 2.0: rvda is the physical VBT address. + * Since rvda is HPA it cannot be directly used in guest. + * And it should not be practically available for end user,so it is not supported. + */ + version = le16_to_cpu(*(__le16 *)(base + OPREGION_VERSION)); + if (version >= 0x0200) { + u64 rvda; + u32 rvds; + + rvda = le64_to_cpu(*(__le64 *)(base + OPREGION_RVDA)); + rvds = le32_to_cpu(*(__le32 *)(base + OPREGION_RVDS)); + if (rvda && rvds) { + /* no support for opregion v2.0 with physical VBT address */ + if (version == 0x0200) { + memunmap(base); + pci_err(vdev->pdev, + "IGD assignment does not support opregion v2.0 with an extended VBT region\n"); + return -EINVAL; + } + + if (rvda != size) { + memunmap(base); + pci_err(vdev->pdev, + "Extended VBT does not follow opregion on version 0x%04x\n", + version); + return -EINVAL; + } + + /* region size for opregion v2.0+: opregion and VBT size. */ + size += rvds; + } + } + if (size != OPREGION_SIZE) { memunmap(base); base = memremap(addr, size, MEMREMAP_WB); -- cgit v1.2.3 From 06d738c8ab56f3de68f785154b87dae1ec94c823 Mon Sep 17 00:00:00 2001 From: Zhen Lei Date: Tue, 30 Mar 2021 09:54:21 -0600 Subject: vfio/type1: fix a couple of spelling mistakes There are several spelling mistakes, as follows: userpsace ==> userspace Accouting ==> Accounting exlude ==> exclude Signed-off-by: Zhen Lei Reviewed-by: Eric Auger Message-Id: <20210326083528.1329-2-thunder.leizhen@huawei.com> Signed-off-by: Alex Williamson --- drivers/vfio/vfio_iommu_type1.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers') diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c index 45cbfd4879a5..0485ad8e1b55 100644 --- a/drivers/vfio/vfio_iommu_type1.c +++ b/drivers/vfio/vfio_iommu_type1.c @@ -16,7 +16,7 @@ * IOMMU to support the IOMMU API and have few to no restrictions around * the IOVA range that can be mapped. The Type1 IOMMU is currently * optimized for relatively static mappings of a userspace process with - * userpsace pages pinned into memory. We also assume devices and IOMMU + * userspace pages pinned into memory. We also assume devices and IOMMU * domains are PCI based as the IOMMU API is still centered around a * device/bus interface rather than a group interface. */ @@ -877,7 +877,7 @@ again: /* * If iommu capable domain exist in the container then all pages are - * already pinned and accounted. Accouting should be done if there is no + * already pinned and accounted. Accounting should be done if there is no * iommu capable domain in the container. */ do_accounting = !IS_IOMMU_CAP_DOMAIN_IN_CONTAINER(iommu); @@ -2177,7 +2177,7 @@ static int vfio_iommu_resv_exclude(struct list_head *iova, continue; /* * Insert a new node if current node overlaps with the - * reserve region to exlude that from valid iova range. + * reserve region to exclude that from valid iova range. * Note that, new node is inserted before the current * node and finally the current node is deleted keeping * the list updated and sorted. -- cgit v1.2.3 From d0a7541dd9998c3b911943fee3bb726d9c2d89c3 Mon Sep 17 00:00:00 2001 From: Zhen Lei Date: Tue, 30 Mar 2021 09:54:21 -0600 Subject: vfio/mdev: Fix spelling mistake "interal" -> "internal" There is a spelling mistake in a comment, fix it. Signed-off-by: Zhen Lei Reviewed-by: Eric Auger Message-Id: <20210326083528.1329-3-thunder.leizhen@huawei.com> Signed-off-by: Alex Williamson --- drivers/vfio/mdev/mdev_private.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/vfio/mdev/mdev_private.h b/drivers/vfio/mdev/mdev_private.h index 7d922950caaf..4d62b76c4734 100644 --- a/drivers/vfio/mdev/mdev_private.h +++ b/drivers/vfio/mdev/mdev_private.h @@ -1,6 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-only */ /* - * Mediated device interal definitions + * Mediated device internal definitions * * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved. * Author: Neo Jia -- cgit v1.2.3 From d0915b32917c3afb390c0eca1a77fd656527d5dc Mon Sep 17 00:00:00 2001 From: Zhen Lei Date: Tue, 30 Mar 2021 09:54:21 -0600 Subject: vfio/pci: fix a couple of spelling mistakes There are several spelling mistakes, as follows: thru ==> through presense ==> presence Signed-off-by: Zhen Lei Reviewed-by: Eric Auger Message-Id: <20210326083528.1329-4-thunder.leizhen@huawei.com> Signed-off-by: Alex Williamson --- drivers/vfio/pci/vfio_pci_config.c | 2 +- drivers/vfio/pci/vfio_pci_nvlink2.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers') diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c index a402adee8a21..d57f037f65b8 100644 --- a/drivers/vfio/pci/vfio_pci_config.c +++ b/drivers/vfio/pci/vfio_pci_config.c @@ -101,7 +101,7 @@ static const u16 pci_ext_cap_length[PCI_EXT_CAP_ID_MAX + 1] = { /* * Read/Write Permission Bits - one bit for each bit in capability * Any field can be read if it exists, but what is read depends on - * whether the field is 'virtualized', or just pass thru to the + * whether the field is 'virtualized', or just pass through to the * hardware. Any virtualized field is also virtualized for writes. * Writes are only permitted if they have a 1 bit here. */ diff --git a/drivers/vfio/pci/vfio_pci_nvlink2.c b/drivers/vfio/pci/vfio_pci_nvlink2.c index 9adcf6a8f888..f276624fec79 100644 --- a/drivers/vfio/pci/vfio_pci_nvlink2.c +++ b/drivers/vfio/pci/vfio_pci_nvlink2.c @@ -219,7 +219,7 @@ int vfio_pci_nvdia_v100_nvlink2_init(struct vfio_pci_device *vdev) unsigned long events = VFIO_GROUP_NOTIFY_SET_KVM; /* - * PCI config space does not tell us about NVLink presense but + * PCI config space does not tell us about NVLink presence but * platform does, use this. */ npu_dev = pnv_pci_get_npu_dev(vdev->pdev, 0); @@ -402,7 +402,7 @@ int vfio_pci_ibm_npu2_init(struct vfio_pci_device *vdev) u32 link_speed = 0xff; /* - * PCI config space does not tell us about NVLink presense but + * PCI config space does not tell us about NVLink presence but * platform does, use this. */ if (!pnv_pci_get_gpu_dev(vdev->pdev)) -- cgit v1.2.3 From f5c858ec2b1d2a2656d78a5efe37cfcf568fce31 Mon Sep 17 00:00:00 2001 From: Zhen Lei Date: Tue, 30 Mar 2021 09:54:22 -0600 Subject: vfio/platform: Fix spelling mistake "registe" -> "register" There is a spelling mistake in a comment, fix it. Signed-off-by: Zhen Lei Acked-by: Eric Auger Message-Id: <20210326083528.1329-5-thunder.leizhen@huawei.com> Signed-off-by: Alex Williamson --- drivers/vfio/platform/reset/vfio_platform_calxedaxgmac.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/vfio/platform/reset/vfio_platform_calxedaxgmac.c b/drivers/vfio/platform/reset/vfio_platform_calxedaxgmac.c index 09a9453b75c5..63cc7f0b2e4a 100644 --- a/drivers/vfio/platform/reset/vfio_platform_calxedaxgmac.c +++ b/drivers/vfio/platform/reset/vfio_platform_calxedaxgmac.c @@ -26,7 +26,7 @@ #define XGMAC_DMA_CONTROL 0x00000f18 /* Ctrl (Operational Mode) */ #define XGMAC_DMA_INTR_ENA 0x00000f1c /* Interrupt Enable */ -/* DMA Control registe defines */ +/* DMA Control register defines */ #define DMA_CONTROL_ST 0x00002000 /* Start/Stop Transmission */ #define DMA_CONTROL_SR 0x00000002 /* Start/Stop Receive */ -- cgit v1.2.3 From a536019d3e7d85a901c5e6a2f2894c0aa0acaefa Mon Sep 17 00:00:00 2001 From: Shenming Lu Date: Tue, 6 Apr 2021 21:50:09 +0800 Subject: vfio/type1: Remove the almost unused check in vfio_iommu_type1_unpin_pages The check i > npage at the end of vfio_iommu_type1_unpin_pages is unused unless npage < 0, but if npage < 0, this function will return npage, which should return -EINVAL instead. So let's just check the parameter npage at the start of the function. By the way, replace unpin_exit with break. Signed-off-by: Shenming Lu Message-Id: <20210406135009.1707-1-lushenming@huawei.com> Signed-off-by: Alex Williamson --- drivers/vfio/vfio_iommu_type1.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'drivers') diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c index 0485ad8e1b55..5fb1bb52b057 100644 --- a/drivers/vfio/vfio_iommu_type1.c +++ b/drivers/vfio/vfio_iommu_type1.c @@ -960,7 +960,7 @@ static int vfio_iommu_type1_unpin_pages(void *iommu_data, bool do_accounting; int i; - if (!iommu || !user_pfn) + if (!iommu || !user_pfn || npage <= 0) return -EINVAL; /* Supported for v2 version only */ @@ -977,13 +977,13 @@ static int vfio_iommu_type1_unpin_pages(void *iommu_data, iova = user_pfn[i] << PAGE_SHIFT; dma = vfio_find_dma(iommu, iova, PAGE_SIZE); if (!dma) - goto unpin_exit; + break; + vfio_unpin_page_external(dma, iova, do_accounting); } -unpin_exit: mutex_unlock(&iommu->lock); - return i > npage ? npage : (i > 0 ? i : -EINVAL); + return i > 0 ? i : -EINVAL; } static long vfio_sync_unpin(struct vfio_dma *dma, struct vfio_domain *domain, -- cgit v1.2.3 From b392a198917020cac996fd207355211ecfcfad84 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Tue, 30 Mar 2021 10:01:18 -0600 Subject: vfio/pci: remove vfio_pci_nvlink2 This driver never had any open userspace (which for VFIO would include VM kernel drivers) that use it, and thus should never have been added by our normal userspace ABI rules. Signed-off-by: Christoph Hellwig Acked-by: Greg Kroah-Hartman Message-Id: <20210326061311.1497642-2-hch@lst.de> Signed-off-by: Alex Williamson --- drivers/vfio/pci/Kconfig | 6 - drivers/vfio/pci/Makefile | 1 - drivers/vfio/pci/vfio_pci.c | 18 -- drivers/vfio/pci/vfio_pci_nvlink2.c | 490 ------------------------------------ drivers/vfio/pci/vfio_pci_private.h | 14 -- include/uapi/linux/vfio.h | 38 +-- 6 files changed, 4 insertions(+), 563 deletions(-) delete mode 100644 drivers/vfio/pci/vfio_pci_nvlink2.c (limited to 'drivers') diff --git a/drivers/vfio/pci/Kconfig b/drivers/vfio/pci/Kconfig index 4abddbebd4b2..53ce78d7d07b 100644 --- a/drivers/vfio/pci/Kconfig +++ b/drivers/vfio/pci/Kconfig @@ -39,9 +39,3 @@ config VFIO_PCI_IGD and LPC bridge config space. To enable Intel IGD assignment through vfio-pci, say Y. - -config VFIO_PCI_NVLINK2 - def_bool y - depends on VFIO_PCI && PPC_POWERNV && SPAPR_TCE_IOMMU - help - VFIO PCI support for P9 Witherspoon machine with NVIDIA V100 GPUs diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile index eff97a7cd9f1..3ff42093962f 100644 --- a/drivers/vfio/pci/Makefile +++ b/drivers/vfio/pci/Makefile @@ -2,7 +2,6 @@ vfio-pci-y := vfio_pci.o vfio_pci_intrs.o vfio_pci_rdwr.o vfio_pci_config.o vfio-pci-$(CONFIG_VFIO_PCI_IGD) += vfio_pci_igd.o -vfio-pci-$(CONFIG_VFIO_PCI_NVLINK2) += vfio_pci_nvlink2.o vfio-pci-$(CONFIG_S390) += vfio_pci_zdev.o obj-$(CONFIG_VFIO_PCI) += vfio-pci.o diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c index 65e7e6b44578..d691006b6428 100644 --- a/drivers/vfio/pci/vfio_pci.c +++ b/drivers/vfio/pci/vfio_pci.c @@ -389,24 +389,6 @@ static int vfio_pci_enable(struct vfio_pci_device *vdev) } } - if (pdev->vendor == PCI_VENDOR_ID_NVIDIA && - IS_ENABLED(CONFIG_VFIO_PCI_NVLINK2)) { - ret = vfio_pci_nvdia_v100_nvlink2_init(vdev); - if (ret && ret != -ENODEV) { - pci_warn(pdev, "Failed to setup NVIDIA NV2 RAM region\n"); - goto disable_exit; - } - } - - if (pdev->vendor == PCI_VENDOR_ID_IBM && - IS_ENABLED(CONFIG_VFIO_PCI_NVLINK2)) { - ret = vfio_pci_ibm_npu2_init(vdev); - if (ret && ret != -ENODEV) { - pci_warn(pdev, "Failed to setup NVIDIA NV2 ATSD region\n"); - goto disable_exit; - } - } - vfio_pci_probe_mmaps(vdev); return 0; diff --git a/drivers/vfio/pci/vfio_pci_nvlink2.c b/drivers/vfio/pci/vfio_pci_nvlink2.c deleted file mode 100644 index 9adcf6a8f888..000000000000 --- a/drivers/vfio/pci/vfio_pci_nvlink2.c +++ /dev/null @@ -1,490 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0-only -/* - * VFIO PCI NVIDIA Whitherspoon GPU support a.k.a. NVLink2. - * - * Copyright (C) 2018 IBM Corp. All rights reserved. - * Author: Alexey Kardashevskiy - * - * Register an on-GPU RAM region for cacheable access. - * - * Derived from original vfio_pci_igd.c: - * Copyright (C) 2016 Red Hat, Inc. All rights reserved. - * Author: Alex Williamson - */ - -#include -#include -#include -#include -#include -#include -#include -#include "vfio_pci_private.h" - -#define CREATE_TRACE_POINTS -#include "trace.h" - -EXPORT_TRACEPOINT_SYMBOL_GPL(vfio_pci_nvgpu_mmap_fault); -EXPORT_TRACEPOINT_SYMBOL_GPL(vfio_pci_nvgpu_mmap); -EXPORT_TRACEPOINT_SYMBOL_GPL(vfio_pci_npu2_mmap); - -struct vfio_pci_nvgpu_data { - unsigned long gpu_hpa; /* GPU RAM physical address */ - unsigned long gpu_tgt; /* TGT address of corresponding GPU RAM */ - unsigned long useraddr; /* GPU RAM userspace address */ - unsigned long size; /* Size of the GPU RAM window (usually 128GB) */ - struct mm_struct *mm; - struct mm_iommu_table_group_mem_t *mem; /* Pre-registered RAM descr. */ - struct pci_dev *gpdev; - struct notifier_block group_notifier; -}; - -static size_t vfio_pci_nvgpu_rw(struct vfio_pci_device *vdev, - char __user *buf, size_t count, loff_t *ppos, bool iswrite) -{ - unsigned int i = VFIO_PCI_OFFSET_TO_INDEX(*ppos) - VFIO_PCI_NUM_REGIONS; - struct vfio_pci_nvgpu_data *data = vdev->region[i].data; - loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK; - loff_t posaligned = pos & PAGE_MASK, posoff = pos & ~PAGE_MASK; - size_t sizealigned; - void __iomem *ptr; - - if (pos >= vdev->region[i].size) - return -EINVAL; - - count = min(count, (size_t)(vdev->region[i].size - pos)); - - /* - * We map only a bit of GPU RAM for a short time instead of mapping it - * for the guest lifetime as: - * - * 1) we do not know GPU RAM size, only aperture which is 4-8 times - * bigger than actual RAM size (16/32GB RAM vs. 128GB aperture); - * 2) mapping GPU RAM allows CPU to prefetch and if this happens - * before NVLink bridge is reset (which fences GPU RAM), - * hardware management interrupts (HMI) might happen, this - * will freeze NVLink bridge. - * - * This is not fast path anyway. - */ - sizealigned = ALIGN(posoff + count, PAGE_SIZE); - ptr = ioremap_cache(data->gpu_hpa + posaligned, sizealigned); - if (!ptr) - return -EFAULT; - - if (iswrite) { - if (copy_from_user(ptr + posoff, buf, count)) - count = -EFAULT; - else - *ppos += count; - } else { - if (copy_to_user(buf, ptr + posoff, count)) - count = -EFAULT; - else - *ppos += count; - } - - iounmap(ptr); - - return count; -} - -static void vfio_pci_nvgpu_release(struct vfio_pci_device *vdev, - struct vfio_pci_region *region) -{ - struct vfio_pci_nvgpu_data *data = region->data; - long ret; - - /* If there were any mappings at all... */ - if (data->mm) { - if (data->mem) { - ret = mm_iommu_put(data->mm, data->mem); - WARN_ON(ret); - } - - mmdrop(data->mm); - } - - vfio_unregister_notifier(&data->gpdev->dev, VFIO_GROUP_NOTIFY, - &data->group_notifier); - - pnv_npu2_unmap_lpar_dev(data->gpdev); - - kfree(data); -} - -static vm_fault_t vfio_pci_nvgpu_mmap_fault(struct vm_fault *vmf) -{ - vm_fault_t ret; - struct vm_area_struct *vma = vmf->vma; - struct vfio_pci_region *region = vma->vm_private_data; - struct vfio_pci_nvgpu_data *data = region->data; - unsigned long vmf_off = (vmf->address - vma->vm_start) >> PAGE_SHIFT; - unsigned long nv2pg = data->gpu_hpa >> PAGE_SHIFT; - unsigned long vm_pgoff = vma->vm_pgoff & - ((1U << (VFIO_PCI_OFFSET_SHIFT - PAGE_SHIFT)) - 1); - unsigned long pfn = nv2pg + vm_pgoff + vmf_off; - - ret = vmf_insert_pfn(vma, vmf->address, pfn); - trace_vfio_pci_nvgpu_mmap_fault(data->gpdev, pfn << PAGE_SHIFT, - vmf->address, ret); - - return ret; -} - -static const struct vm_operations_struct vfio_pci_nvgpu_mmap_vmops = { - .fault = vfio_pci_nvgpu_mmap_fault, -}; - -static int vfio_pci_nvgpu_mmap(struct vfio_pci_device *vdev, - struct vfio_pci_region *region, struct vm_area_struct *vma) -{ - int ret; - struct vfio_pci_nvgpu_data *data = region->data; - - if (data->useraddr) - return -EPERM; - - if (vma->vm_end - vma->vm_start > data->size) - return -EINVAL; - - vma->vm_private_data = region; - vma->vm_flags |= VM_PFNMAP; - vma->vm_ops = &vfio_pci_nvgpu_mmap_vmops; - - /* - * Calling mm_iommu_newdev() here once as the region is not - * registered yet and therefore right initialization will happen now. - * Other places will use mm_iommu_find() which returns - * registered @mem and does not go gup(). - */ - data->useraddr = vma->vm_start; - data->mm = current->mm; - - mmgrab(data->mm); - ret = (int) mm_iommu_newdev(data->mm, data->useraddr, - vma_pages(vma), data->gpu_hpa, &data->mem); - - trace_vfio_pci_nvgpu_mmap(vdev->pdev, data->gpu_hpa, data->useraddr, - vma->vm_end - vma->vm_start, ret); - - return ret; -} - -static int vfio_pci_nvgpu_add_capability(struct vfio_pci_device *vdev, - struct vfio_pci_region *region, struct vfio_info_cap *caps) -{ - struct vfio_pci_nvgpu_data *data = region->data; - struct vfio_region_info_cap_nvlink2_ssatgt cap = { - .header.id = VFIO_REGION_INFO_CAP_NVLINK2_SSATGT, - .header.version = 1, - .tgt = data->gpu_tgt - }; - - return vfio_info_add_capability(caps, &cap.header, sizeof(cap)); -} - -static const struct vfio_pci_regops vfio_pci_nvgpu_regops = { - .rw = vfio_pci_nvgpu_rw, - .release = vfio_pci_nvgpu_release, - .mmap = vfio_pci_nvgpu_mmap, - .add_capability = vfio_pci_nvgpu_add_capability, -}; - -static int vfio_pci_nvgpu_group_notifier(struct notifier_block *nb, - unsigned long action, void *opaque) -{ - struct kvm *kvm = opaque; - struct vfio_pci_nvgpu_data *data = container_of(nb, - struct vfio_pci_nvgpu_data, - group_notifier); - - if (action == VFIO_GROUP_NOTIFY_SET_KVM && kvm && - pnv_npu2_map_lpar_dev(data->gpdev, - kvm->arch.lpid, MSR_DR | MSR_PR)) - return NOTIFY_BAD; - - return NOTIFY_OK; -} - -int vfio_pci_nvdia_v100_nvlink2_init(struct vfio_pci_device *vdev) -{ - int ret; - u64 reg[2]; - u64 tgt = 0; - struct device_node *npu_node, *mem_node; - struct pci_dev *npu_dev; - struct vfio_pci_nvgpu_data *data; - uint32_t mem_phandle = 0; - unsigned long events = VFIO_GROUP_NOTIFY_SET_KVM; - - /* - * PCI config space does not tell us about NVLink presense but - * platform does, use this. - */ - npu_dev = pnv_pci_get_npu_dev(vdev->pdev, 0); - if (!npu_dev) - return -ENODEV; - - npu_node = pci_device_to_OF_node(npu_dev); - if (!npu_node) - return -EINVAL; - - if (of_property_read_u32(npu_node, "memory-region", &mem_phandle)) - return -ENODEV; - - mem_node = of_find_node_by_phandle(mem_phandle); - if (!mem_node) - return -EINVAL; - - if (of_property_read_variable_u64_array(mem_node, "reg", reg, - ARRAY_SIZE(reg), ARRAY_SIZE(reg)) != - ARRAY_SIZE(reg)) - return -EINVAL; - - if (of_property_read_u64(npu_node, "ibm,device-tgt-addr", &tgt)) { - dev_warn(&vdev->pdev->dev, "No ibm,device-tgt-addr found\n"); - return -EFAULT; - } - - data = kzalloc(sizeof(*data), GFP_KERNEL); - if (!data) - return -ENOMEM; - - data->gpu_hpa = reg[0]; - data->gpu_tgt = tgt; - data->size = reg[1]; - - dev_dbg(&vdev->pdev->dev, "%lx..%lx\n", data->gpu_hpa, - data->gpu_hpa + data->size - 1); - - data->gpdev = vdev->pdev; - data->group_notifier.notifier_call = vfio_pci_nvgpu_group_notifier; - - ret = vfio_register_notifier(&data->gpdev->dev, VFIO_GROUP_NOTIFY, - &events, &data->group_notifier); - if (ret) - goto free_exit; - - /* - * We have just set KVM, we do not need the listener anymore. - * Also, keeping it registered means that if more than one GPU is - * assigned, we will get several similar notifiers notifying about - * the same device again which does not help with anything. - */ - vfio_unregister_notifier(&data->gpdev->dev, VFIO_GROUP_NOTIFY, - &data->group_notifier); - - ret = vfio_pci_register_dev_region(vdev, - PCI_VENDOR_ID_NVIDIA | VFIO_REGION_TYPE_PCI_VENDOR_TYPE, - VFIO_REGION_SUBTYPE_NVIDIA_NVLINK2_RAM, - &vfio_pci_nvgpu_regops, - data->size, - VFIO_REGION_INFO_FLAG_READ | - VFIO_REGION_INFO_FLAG_WRITE | - VFIO_REGION_INFO_FLAG_MMAP, - data); - if (ret) - goto free_exit; - - return 0; -free_exit: - kfree(data); - - return ret; -} - -/* - * IBM NPU2 bridge - */ -struct vfio_pci_npu2_data { - void *base; /* ATSD register virtual address, for emulated access */ - unsigned long mmio_atsd; /* ATSD physical address */ - unsigned long gpu_tgt; /* TGT address of corresponding GPU RAM */ - unsigned int link_speed; /* The link speed from DT's ibm,nvlink-speed */ -}; - -static size_t vfio_pci_npu2_rw(struct vfio_pci_device *vdev, - char __user *buf, size_t count, loff_t *ppos, bool iswrite) -{ - unsigned int i = VFIO_PCI_OFFSET_TO_INDEX(*ppos) - VFIO_PCI_NUM_REGIONS; - struct vfio_pci_npu2_data *data = vdev->region[i].data; - loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK; - - if (pos >= vdev->region[i].size) - return -EINVAL; - - count = min(count, (size_t)(vdev->region[i].size - pos)); - - if (iswrite) { - if (copy_from_user(data->base + pos, buf, count)) - return -EFAULT; - } else { - if (copy_to_user(buf, data->base + pos, count)) - return -EFAULT; - } - *ppos += count; - - return count; -} - -static int vfio_pci_npu2_mmap(struct vfio_pci_device *vdev, - struct vfio_pci_region *region, struct vm_area_struct *vma) -{ - int ret; - struct vfio_pci_npu2_data *data = region->data; - unsigned long req_len = vma->vm_end - vma->vm_start; - - if (req_len != PAGE_SIZE) - return -EINVAL; - - vma->vm_flags |= VM_PFNMAP; - vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); - - ret = remap_pfn_range(vma, vma->vm_start, data->mmio_atsd >> PAGE_SHIFT, - req_len, vma->vm_page_prot); - trace_vfio_pci_npu2_mmap(vdev->pdev, data->mmio_atsd, vma->vm_start, - vma->vm_end - vma->vm_start, ret); - - return ret; -} - -static void vfio_pci_npu2_release(struct vfio_pci_device *vdev, - struct vfio_pci_region *region) -{ - struct vfio_pci_npu2_data *data = region->data; - - memunmap(data->base); - kfree(data); -} - -static int vfio_pci_npu2_add_capability(struct vfio_pci_device *vdev, - struct vfio_pci_region *region, struct vfio_info_cap *caps) -{ - struct vfio_pci_npu2_data *data = region->data; - struct vfio_region_info_cap_nvlink2_ssatgt captgt = { - .header.id = VFIO_REGION_INFO_CAP_NVLINK2_SSATGT, - .header.version = 1, - .tgt = data->gpu_tgt - }; - struct vfio_region_info_cap_nvlink2_lnkspd capspd = { - .header.id = VFIO_REGION_INFO_CAP_NVLINK2_LNKSPD, - .header.version = 1, - .link_speed = data->link_speed - }; - int ret; - - ret = vfio_info_add_capability(caps, &captgt.header, sizeof(captgt)); - if (ret) - return ret; - - return vfio_info_add_capability(caps, &capspd.header, sizeof(capspd)); -} - -static const struct vfio_pci_regops vfio_pci_npu2_regops = { - .rw = vfio_pci_npu2_rw, - .mmap = vfio_pci_npu2_mmap, - .release = vfio_pci_npu2_release, - .add_capability = vfio_pci_npu2_add_capability, -}; - -int vfio_pci_ibm_npu2_init(struct vfio_pci_device *vdev) -{ - int ret; - struct vfio_pci_npu2_data *data; - struct device_node *nvlink_dn; - u32 nvlink_index = 0, mem_phandle = 0; - struct pci_dev *npdev = vdev->pdev; - struct device_node *npu_node = pci_device_to_OF_node(npdev); - struct pci_controller *hose = pci_bus_to_host(npdev->bus); - u64 mmio_atsd = 0; - u64 tgt = 0; - u32 link_speed = 0xff; - - /* - * PCI config space does not tell us about NVLink presense but - * platform does, use this. - */ - if (!pnv_pci_get_gpu_dev(vdev->pdev)) - return -ENODEV; - - if (of_property_read_u32(npu_node, "memory-region", &mem_phandle)) - return -ENODEV; - - /* - * NPU2 normally has 8 ATSD registers (for concurrency) and 6 links - * so we can allocate one register per link, using nvlink index as - * a key. - * There is always at least one ATSD register so as long as at least - * NVLink bridge #0 is passed to the guest, ATSD will be available. - */ - nvlink_dn = of_parse_phandle(npdev->dev.of_node, "ibm,nvlink", 0); - if (WARN_ON(of_property_read_u32(nvlink_dn, "ibm,npu-link-index", - &nvlink_index))) - return -ENODEV; - - if (of_property_read_u64_index(hose->dn, "ibm,mmio-atsd", nvlink_index, - &mmio_atsd)) { - if (of_property_read_u64_index(hose->dn, "ibm,mmio-atsd", 0, - &mmio_atsd)) { - dev_warn(&vdev->pdev->dev, "No available ATSD found\n"); - mmio_atsd = 0; - } else { - dev_warn(&vdev->pdev->dev, - "Using fallback ibm,mmio-atsd[0] for ATSD.\n"); - } - } - - if (of_property_read_u64(npu_node, "ibm,device-tgt-addr", &tgt)) { - dev_warn(&vdev->pdev->dev, "No ibm,device-tgt-addr found\n"); - return -EFAULT; - } - - if (of_property_read_u32(npu_node, "ibm,nvlink-speed", &link_speed)) { - dev_warn(&vdev->pdev->dev, "No ibm,nvlink-speed found\n"); - return -EFAULT; - } - - data = kzalloc(sizeof(*data), GFP_KERNEL); - if (!data) - return -ENOMEM; - - data->mmio_atsd = mmio_atsd; - data->gpu_tgt = tgt; - data->link_speed = link_speed; - if (data->mmio_atsd) { - data->base = memremap(data->mmio_atsd, SZ_64K, MEMREMAP_WT); - if (!data->base) { - ret = -ENOMEM; - goto free_exit; - } - } - - /* - * We want to expose the capability even if this specific NVLink - * did not get its own ATSD register because capabilities - * belong to VFIO regions and normally there will be ATSD register - * assigned to the NVLink bridge. - */ - ret = vfio_pci_register_dev_region(vdev, - PCI_VENDOR_ID_IBM | - VFIO_REGION_TYPE_PCI_VENDOR_TYPE, - VFIO_REGION_SUBTYPE_IBM_NVLINK2_ATSD, - &vfio_pci_npu2_regops, - data->mmio_atsd ? PAGE_SIZE : 0, - VFIO_REGION_INFO_FLAG_READ | - VFIO_REGION_INFO_FLAG_WRITE | - VFIO_REGION_INFO_FLAG_MMAP, - data); - if (ret) - goto free_exit; - - return 0; - -free_exit: - if (data->base) - memunmap(data->base); - kfree(data); - - return ret; -} diff --git a/drivers/vfio/pci/vfio_pci_private.h b/drivers/vfio/pci/vfio_pci_private.h index 9cd1882a05af..cdae2e4cf11c 100644 --- a/drivers/vfio/pci/vfio_pci_private.h +++ b/drivers/vfio/pci/vfio_pci_private.h @@ -199,20 +199,6 @@ static inline int vfio_pci_igd_init(struct vfio_pci_device *vdev) return -ENODEV; } #endif -#ifdef CONFIG_VFIO_PCI_NVLINK2 -extern int vfio_pci_nvdia_v100_nvlink2_init(struct vfio_pci_device *vdev); -extern int vfio_pci_ibm_npu2_init(struct vfio_pci_device *vdev); -#else -static inline int vfio_pci_nvdia_v100_nvlink2_init(struct vfio_pci_device *vdev) -{ - return -ENODEV; -} - -static inline int vfio_pci_ibm_npu2_init(struct vfio_pci_device *vdev) -{ - return -ENODEV; -} -#endif #ifdef CONFIG_S390 extern int vfio_pci_info_zdev_add_caps(struct vfio_pci_device *vdev, diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h index 8ce36c1d53ca..34b1f53a3901 100644 --- a/include/uapi/linux/vfio.h +++ b/include/uapi/linux/vfio.h @@ -333,17 +333,10 @@ struct vfio_region_info_cap_type { #define VFIO_REGION_SUBTYPE_INTEL_IGD_LPC_CFG (3) /* 10de vendor PCI sub-types */ -/* - * NVIDIA GPU NVlink2 RAM is coherent RAM mapped onto the host address space. - */ -#define VFIO_REGION_SUBTYPE_NVIDIA_NVLINK2_RAM (1) +/* subtype 1 was VFIO_REGION_SUBTYPE_NVIDIA_NVLINK2_RAM, don't use */ /* 1014 vendor PCI sub-types */ -/* - * IBM NPU NVlink2 ATSD (Address Translation Shootdown) register of NPU - * to do TLB invalidation on a GPU. - */ -#define VFIO_REGION_SUBTYPE_IBM_NVLINK2_ATSD (1) +/* subtype 1 was VFIO_REGION_SUBTYPE_IBM_NVLINK2_ATSD, don't use */ /* sub-types for VFIO_REGION_TYPE_GFX */ #define VFIO_REGION_SUBTYPE_GFX_EDID (1) @@ -637,32 +630,9 @@ struct vfio_device_migration_info { */ #define VFIO_REGION_INFO_CAP_MSIX_MAPPABLE 3 -/* - * Capability with compressed real address (aka SSA - small system address) - * where GPU RAM is mapped on a system bus. Used by a GPU for DMA routing - * and by the userspace to associate a NVLink bridge with a GPU. - */ -#define VFIO_REGION_INFO_CAP_NVLINK2_SSATGT 4 - -struct vfio_region_info_cap_nvlink2_ssatgt { - struct vfio_info_cap_header header; - __u64 tgt; -}; +/* subtype 4 was VFIO_REGION_INFO_CAP_NVLINK2_SSATGT, don't use */ -/* - * Capability with an NVLink link speed. The value is read by - * the NVlink2 bridge driver from the bridge's "ibm,nvlink-speed" - * property in the device tree. The value is fixed in the hardware - * and failing to provide the correct value results in the link - * not working with no indication from the driver why. - */ -#define VFIO_REGION_INFO_CAP_NVLINK2_LNKSPD 5 - -struct vfio_region_info_cap_nvlink2_lnkspd { - struct vfio_info_cap_header header; - __u32 link_speed; - __u32 __pad; -}; +/* subtype 5 was VFIO_REGION_INFO_CAP_NVLINK2_LNKSPD, don't use */ /** * VFIO_DEVICE_GET_IRQ_INFO - _IOWR(VFIO_TYPE, VFIO_BASE + 9, -- cgit v1.2.3 From e572bfb2b6a83b05acd30c03010e661b1967960f Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Tue, 30 Mar 2021 09:53:05 -0600 Subject: vfio: Remove extra put/gets around vfio_device->group The vfio_device->group value has a get obtained during vfio_add_group_dev() which gets moved from the stack to vfio_device->group in vfio_group_create_device(). The reference remains until we reach the end of vfio_del_group_dev() when it is put back. Thus anything that already has a kref on the vfio_device is guaranteed a valid group pointer. Remove all the extra reference traffic. It is tricky to see, but the get at the start of vfio_del_group_dev() is actually pairing with the put hidden inside vfio_device_put() a few lines below. A later patch merges vfio_group_create_device() into vfio_add_group_dev() which makes the ownership and error flow on the create side easier to follow. Reviewed-by: Christoph Hellwig Reviewed-by: Kevin Tian Reviewed-by: Max Gurtovoy Reviewed-by: Cornelia Huck Reviewed-by: Eric Auger Signed-off-by: Jason Gunthorpe Message-Id: <1-v3-225de1400dfc+4e074-vfio1_jgg@nvidia.com> Signed-off-by: Alex Williamson --- drivers/vfio/vfio.c | 21 ++------------------- 1 file changed, 2 insertions(+), 19 deletions(-) (limited to 'drivers') diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c index 38779e6fd80c..15d8e678e556 100644 --- a/drivers/vfio/vfio.c +++ b/drivers/vfio/vfio.c @@ -546,14 +546,12 @@ struct vfio_device *vfio_group_create_device(struct vfio_group *group, kref_init(&device->kref); device->dev = dev; + /* Our reference on group is moved to the device */ device->group = group; device->ops = ops; device->device_data = device_data; dev_set_drvdata(dev, device); - /* No need to get group_lock, caller has group reference */ - vfio_group_get(group); - mutex_lock(&group->device_lock); list_add(&device->group_next, &group->device_list); group->dev_counter++; @@ -585,13 +583,11 @@ void vfio_device_put(struct vfio_device *device) { struct vfio_group *group = device->group; kref_put_mutex(&device->kref, vfio_device_release, &group->device_lock); - vfio_group_put(group); } EXPORT_SYMBOL_GPL(vfio_device_put); static void vfio_device_get(struct vfio_device *device) { - vfio_group_get(device->group); kref_get(&device->kref); } @@ -841,14 +837,6 @@ int vfio_add_group_dev(struct device *dev, vfio_group_put(group); return PTR_ERR(device); } - - /* - * Drop all but the vfio_device reference. The vfio_device holds - * a reference to the vfio_group, which holds a reference to the - * iommu_group. - */ - vfio_group_put(group); - return 0; } EXPORT_SYMBOL_GPL(vfio_add_group_dev); @@ -928,12 +916,6 @@ void *vfio_del_group_dev(struct device *dev) unsigned int i = 0; bool interrupted = false; - /* - * The group exists so long as we have a device reference. Get - * a group reference and use it to scan for the device going away. - */ - vfio_group_get(group); - /* * When the device is removed from the group, the group suddenly * becomes non-viable; the device has a driver (until the unbind @@ -1008,6 +990,7 @@ void *vfio_del_group_dev(struct device *dev) if (list_empty(&group->device_list)) wait_event(group->container_q, !group->container); + /* Matches the get in vfio_group_create_device() */ vfio_group_put(group); return device_data; -- cgit v1.2.3 From 5e42c999445bd0ae86e35affeb3e7c473d74a893 Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Tue, 30 Mar 2021 09:53:05 -0600 Subject: vfio: Simplify the lifetime logic for vfio_device The vfio_device is using a 'sleep until all refs go to zero' pattern for its lifetime, but it is indirectly coded by repeatedly scanning the group list waiting for the device to be removed on its own. Switch this around to be a direct representation, use a refcount to count the number of places that are blocking destruction and sleep directly on a completion until that counter goes to zero. kfree the device after other accesses have been excluded in vfio_del_group_dev(). This is a fairly common Linux idiom. Due to this we can now remove kref_put_mutex(), which is very rarely used in the kernel. Here it is being used to prevent a zero ref device from being seen in the group list. Instead allow the zero ref device to continue to exist in the device_list and use refcount_inc_not_zero() to exclude it once refs go to zero. This patch is organized so the next patch will be able to alter the API to allow drivers to provide the kfree. Reviewed-by: Christoph Hellwig Reviewed-by: Kevin Tian Reviewed-by: Cornelia Huck Reviewed-by: Eric Auger Signed-off-by: Jason Gunthorpe Message-Id: <2-v3-225de1400dfc+4e074-vfio1_jgg@nvidia.com> Signed-off-by: Alex Williamson --- drivers/vfio/vfio.c | 79 +++++++++++++++++------------------------------------ 1 file changed, 25 insertions(+), 54 deletions(-) (limited to 'drivers') diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c index 15d8e678e556..32660e8a69ae 100644 --- a/drivers/vfio/vfio.c +++ b/drivers/vfio/vfio.c @@ -46,7 +46,6 @@ static struct vfio { struct mutex group_lock; struct cdev group_cdev; dev_t group_devt; - wait_queue_head_t release_q; } vfio; struct vfio_iommu_driver { @@ -91,7 +90,8 @@ struct vfio_group { }; struct vfio_device { - struct kref kref; + refcount_t refcount; + struct completion comp; struct device *dev; const struct vfio_device_ops *ops; struct vfio_group *group; @@ -544,7 +544,8 @@ struct vfio_device *vfio_group_create_device(struct vfio_group *group, if (!device) return ERR_PTR(-ENOMEM); - kref_init(&device->kref); + refcount_set(&device->refcount, 1); + init_completion(&device->comp); device->dev = dev; /* Our reference on group is moved to the device */ device->group = group; @@ -560,35 +561,17 @@ struct vfio_device *vfio_group_create_device(struct vfio_group *group, return device; } -static void vfio_device_release(struct kref *kref) -{ - struct vfio_device *device = container_of(kref, - struct vfio_device, kref); - struct vfio_group *group = device->group; - - list_del(&device->group_next); - group->dev_counter--; - mutex_unlock(&group->device_lock); - - dev_set_drvdata(device->dev, NULL); - - kfree(device); - - /* vfio_del_group_dev may be waiting for this device */ - wake_up(&vfio.release_q); -} - /* Device reference always implies a group reference */ void vfio_device_put(struct vfio_device *device) { - struct vfio_group *group = device->group; - kref_put_mutex(&device->kref, vfio_device_release, &group->device_lock); + if (refcount_dec_and_test(&device->refcount)) + complete(&device->comp); } EXPORT_SYMBOL_GPL(vfio_device_put); -static void vfio_device_get(struct vfio_device *device) +static bool vfio_device_try_get(struct vfio_device *device) { - kref_get(&device->kref); + return refcount_inc_not_zero(&device->refcount); } static struct vfio_device *vfio_group_get_device(struct vfio_group *group, @@ -598,8 +581,7 @@ static struct vfio_device *vfio_group_get_device(struct vfio_group *group, mutex_lock(&group->device_lock); list_for_each_entry(device, &group->device_list, group_next) { - if (device->dev == dev) { - vfio_device_get(device); + if (device->dev == dev && vfio_device_try_get(device)) { mutex_unlock(&group->device_lock); return device; } @@ -883,9 +865,8 @@ static struct vfio_device *vfio_device_get_from_name(struct vfio_group *group, ret = !strcmp(dev_name(it->dev), buf); } - if (ret) { + if (ret && vfio_device_try_get(it)) { device = it; - vfio_device_get(device); break; } } @@ -908,13 +889,13 @@ EXPORT_SYMBOL_GPL(vfio_device_data); * removed. Open file descriptors for the device... */ void *vfio_del_group_dev(struct device *dev) { - DEFINE_WAIT_FUNC(wait, woken_wake_function); struct vfio_device *device = dev_get_drvdata(dev); struct vfio_group *group = device->group; void *device_data = device->device_data; struct vfio_unbound_dev *unbound; unsigned int i = 0; bool interrupted = false; + long rc; /* * When the device is removed from the group, the group suddenly @@ -935,32 +916,18 @@ void *vfio_del_group_dev(struct device *dev) WARN_ON(!unbound); vfio_device_put(device); - - /* - * If the device is still present in the group after the above - * 'put', then it is in use and we need to request it from the - * bus driver. The driver may in turn need to request the - * device from the user. We send the request on an arbitrary - * interval with counter to allow the driver to take escalating - * measures to release the device if it has the ability to do so. - */ - add_wait_queue(&vfio.release_q, &wait); - - do { - device = vfio_group_get_device(group, dev); - if (!device) - break; - + rc = try_wait_for_completion(&device->comp); + while (rc <= 0) { if (device->ops->request) device->ops->request(device_data, i++); - vfio_device_put(device); - if (interrupted) { - wait_woken(&wait, TASK_UNINTERRUPTIBLE, HZ * 10); + rc = wait_for_completion_timeout(&device->comp, + HZ * 10); } else { - wait_woken(&wait, TASK_INTERRUPTIBLE, HZ * 10); - if (signal_pending(current)) { + rc = wait_for_completion_interruptible_timeout( + &device->comp, HZ * 10); + if (rc < 0) { interrupted = true; dev_warn(dev, "Device is currently in use, task" @@ -969,10 +936,13 @@ void *vfio_del_group_dev(struct device *dev) current->comm, task_pid_nr(current)); } } + } - } while (1); + mutex_lock(&group->device_lock); + list_del(&device->group_next); + group->dev_counter--; + mutex_unlock(&group->device_lock); - remove_wait_queue(&vfio.release_q, &wait); /* * In order to support multiple devices per group, devices can be * plucked from the group while other devices in the group are still @@ -992,6 +962,8 @@ void *vfio_del_group_dev(struct device *dev) /* Matches the get in vfio_group_create_device() */ vfio_group_put(group); + dev_set_drvdata(dev, NULL); + kfree(device); return device_data; } @@ -2362,7 +2334,6 @@ static int __init vfio_init(void) mutex_init(&vfio.iommu_drivers_lock); INIT_LIST_HEAD(&vfio.group_list); INIT_LIST_HEAD(&vfio.iommu_drivers_list); - init_waitqueue_head(&vfio.release_q); ret = misc_register(&vfio_dev); if (ret) { -- cgit v1.2.3 From 0bfc6a4ea63c2adac71a824397ef48f28dbc5e47 Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Tue, 30 Mar 2021 09:53:05 -0600 Subject: vfio: Split creation of a vfio_device into init and register ops This makes the struct vfio_device part of the public interface so it can be used with container_of and so forth, as is typical for a Linux subystem. This is the first step to bring some type-safety to the vfio interface by allowing the replacement of 'void *' and 'struct device *' inputs with a simple and clear 'struct vfio_device *' For now the self-allocating vfio_add_group_dev() interface is kept so each user can be updated as a separate patch. The expected usage pattern is driver core probe() function: my_device = kzalloc(sizeof(*mydevice)); vfio_init_group_dev(&my_device->vdev, dev, ops, mydevice); /* other driver specific prep */ vfio_register_group_dev(&my_device->vdev); dev_set_drvdata(dev, my_device); driver core remove() function: my_device = dev_get_drvdata(dev); vfio_unregister_group_dev(&my_device->vdev); /* other driver specific tear down */ kfree(my_device); Allowing the driver to be able to use the drvdata and vfio_device to go to/from its own data. The pattern also makes it clear that vfio_register_group_dev() must be last in the sequence, as once it is called the core code can immediately start calling ops. The init/register gap is provided to allow for the driver to do setup before ops can be called and thus avoid races. Reviewed-by: Christoph Hellwig Reviewed-by: Liu Yi L Reviewed-by: Cornelia Huck Reviewed-by: Max Gurtovoy Reviewed-by: Kevin Tian Reviewed-by: Eric Auger Signed-off-by: Jason Gunthorpe Message-Id: <3-v3-225de1400dfc+4e074-vfio1_jgg@nvidia.com> Signed-off-by: Alex Williamson --- Documentation/driver-api/vfio.rst | 31 ++++++---- drivers/vfio/vfio.c | 125 ++++++++++++++++++++------------------ include/linux/vfio.h | 16 +++++ 3 files changed, 99 insertions(+), 73 deletions(-) (limited to 'drivers') diff --git a/Documentation/driver-api/vfio.rst b/Documentation/driver-api/vfio.rst index f1a4d3c3ba0b..d3a02300913a 100644 --- a/Documentation/driver-api/vfio.rst +++ b/Documentation/driver-api/vfio.rst @@ -249,18 +249,23 @@ VFIO bus driver API VFIO bus drivers, such as vfio-pci make use of only a few interfaces into VFIO core. When devices are bound and unbound to the driver, -the driver should call vfio_add_group_dev() and vfio_del_group_dev() -respectively:: - - extern int vfio_add_group_dev(struct device *dev, - const struct vfio_device_ops *ops, - void *device_data); - - extern void *vfio_del_group_dev(struct device *dev); - -vfio_add_group_dev() indicates to the core to begin tracking the -iommu_group of the specified dev and register the dev as owned by -a VFIO bus driver. The driver provides an ops structure for callbacks +the driver should call vfio_register_group_dev() and +vfio_unregister_group_dev() respectively:: + + void vfio_init_group_dev(struct vfio_device *device, + struct device *dev, + const struct vfio_device_ops *ops, + void *device_data); + int vfio_register_group_dev(struct vfio_device *device); + void vfio_unregister_group_dev(struct vfio_device *device); + +The driver should embed the vfio_device in its own structure and call +vfio_init_group_dev() to pre-configure it before going to registration. +vfio_register_group_dev() indicates to the core to begin tracking the +iommu_group of the specified dev and register the dev as owned by a VFIO bus +driver. Once vfio_register_group_dev() returns it is possible for userspace to +start accessing the driver, thus the driver should ensure it is completely +ready before calling it. The driver provides an ops structure for callbacks similar to a file operations structure:: struct vfio_device_ops { @@ -276,7 +281,7 @@ similar to a file operations structure:: }; Each function is passed the device_data that was originally registered -in the vfio_add_group_dev() call above. This allows the bus driver +in the vfio_register_group_dev() call above. This allows the bus driver an easy place to store its opaque, private data. The open/release callbacks are issued when a new file descriptor is created for a device (via VFIO_GROUP_GET_DEVICE_FD). The ioctl interface provides diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c index 32660e8a69ae..2ea430de505b 100644 --- a/drivers/vfio/vfio.c +++ b/drivers/vfio/vfio.c @@ -89,16 +89,6 @@ struct vfio_group { struct blocking_notifier_head notifier; }; -struct vfio_device { - refcount_t refcount; - struct completion comp; - struct device *dev; - const struct vfio_device_ops *ops; - struct vfio_group *group; - struct list_head group_next; - void *device_data; -}; - #ifdef CONFIG_VFIO_NOIOMMU static bool noiommu __read_mostly; module_param_named(enable_unsafe_noiommu_mode, @@ -532,35 +522,6 @@ static struct vfio_group *vfio_group_get_from_dev(struct device *dev) /** * Device objects - create, release, get, put, search */ -static -struct vfio_device *vfio_group_create_device(struct vfio_group *group, - struct device *dev, - const struct vfio_device_ops *ops, - void *device_data) -{ - struct vfio_device *device; - - device = kzalloc(sizeof(*device), GFP_KERNEL); - if (!device) - return ERR_PTR(-ENOMEM); - - refcount_set(&device->refcount, 1); - init_completion(&device->comp); - device->dev = dev; - /* Our reference on group is moved to the device */ - device->group = group; - device->ops = ops; - device->device_data = device_data; - dev_set_drvdata(dev, device); - - mutex_lock(&group->device_lock); - list_add(&device->group_next, &group->device_list); - group->dev_counter++; - mutex_unlock(&group->device_lock); - - return device; -} - /* Device reference always implies a group reference */ void vfio_device_put(struct vfio_device *device) { @@ -779,14 +740,23 @@ static int vfio_iommu_group_notifier(struct notifier_block *nb, /** * VFIO driver API */ -int vfio_add_group_dev(struct device *dev, - const struct vfio_device_ops *ops, void *device_data) +void vfio_init_group_dev(struct vfio_device *device, struct device *dev, + const struct vfio_device_ops *ops, void *device_data) +{ + init_completion(&device->comp); + device->dev = dev; + device->ops = ops; + device->device_data = device_data; +} +EXPORT_SYMBOL_GPL(vfio_init_group_dev); + +int vfio_register_group_dev(struct vfio_device *device) { + struct vfio_device *existing_device; struct iommu_group *iommu_group; struct vfio_group *group; - struct vfio_device *device; - iommu_group = iommu_group_get(dev); + iommu_group = iommu_group_get(device->dev); if (!iommu_group) return -EINVAL; @@ -805,21 +775,50 @@ int vfio_add_group_dev(struct device *dev, iommu_group_put(iommu_group); } - device = vfio_group_get_device(group, dev); - if (device) { - dev_WARN(dev, "Device already exists on group %d\n", + existing_device = vfio_group_get_device(group, device->dev); + if (existing_device) { + dev_WARN(device->dev, "Device already exists on group %d\n", iommu_group_id(iommu_group)); - vfio_device_put(device); + vfio_device_put(existing_device); vfio_group_put(group); return -EBUSY; } - device = vfio_group_create_device(group, dev, ops, device_data); - if (IS_ERR(device)) { - vfio_group_put(group); - return PTR_ERR(device); - } + /* Our reference on group is moved to the device */ + device->group = group; + + /* Refcounting can't start until the driver calls register */ + refcount_set(&device->refcount, 1); + + mutex_lock(&group->device_lock); + list_add(&device->group_next, &group->device_list); + group->dev_counter++; + mutex_unlock(&group->device_lock); + + return 0; +} +EXPORT_SYMBOL_GPL(vfio_register_group_dev); + +int vfio_add_group_dev(struct device *dev, const struct vfio_device_ops *ops, + void *device_data) +{ + struct vfio_device *device; + int ret; + + device = kzalloc(sizeof(*device), GFP_KERNEL); + if (!device) + return -ENOMEM; + + vfio_init_group_dev(device, dev, ops, device_data); + ret = vfio_register_group_dev(device); + if (ret) + goto err_kfree; + dev_set_drvdata(dev, device); return 0; + +err_kfree: + kfree(device); + return ret; } EXPORT_SYMBOL_GPL(vfio_add_group_dev); @@ -887,11 +886,9 @@ EXPORT_SYMBOL_GPL(vfio_device_data); /* * Decrement the device reference count and wait for the device to be * removed. Open file descriptors for the device... */ -void *vfio_del_group_dev(struct device *dev) +void vfio_unregister_group_dev(struct vfio_device *device) { - struct vfio_device *device = dev_get_drvdata(dev); struct vfio_group *group = device->group; - void *device_data = device->device_data; struct vfio_unbound_dev *unbound; unsigned int i = 0; bool interrupted = false; @@ -908,7 +905,7 @@ void *vfio_del_group_dev(struct device *dev) */ unbound = kzalloc(sizeof(*unbound), GFP_KERNEL); if (unbound) { - unbound->dev = dev; + unbound->dev = device->dev; mutex_lock(&group->unbound_lock); list_add(&unbound->unbound_next, &group->unbound_list); mutex_unlock(&group->unbound_lock); @@ -919,7 +916,7 @@ void *vfio_del_group_dev(struct device *dev) rc = try_wait_for_completion(&device->comp); while (rc <= 0) { if (device->ops->request) - device->ops->request(device_data, i++); + device->ops->request(device->device_data, i++); if (interrupted) { rc = wait_for_completion_timeout(&device->comp, @@ -929,7 +926,7 @@ void *vfio_del_group_dev(struct device *dev) &device->comp, HZ * 10); if (rc < 0) { interrupted = true; - dev_warn(dev, + dev_warn(device->dev, "Device is currently in use, task" " \"%s\" (%d) " "blocked until device is released", @@ -960,11 +957,19 @@ void *vfio_del_group_dev(struct device *dev) if (list_empty(&group->device_list)) wait_event(group->container_q, !group->container); - /* Matches the get in vfio_group_create_device() */ + /* Matches the get in vfio_register_group_dev() */ vfio_group_put(group); +} +EXPORT_SYMBOL_GPL(vfio_unregister_group_dev); + +void *vfio_del_group_dev(struct device *dev) +{ + struct vfio_device *device = dev_get_drvdata(dev); + void *device_data = device->device_data; + + vfio_unregister_group_dev(device); dev_set_drvdata(dev, NULL); kfree(device); - return device_data; } EXPORT_SYMBOL_GPL(vfio_del_group_dev); diff --git a/include/linux/vfio.h b/include/linux/vfio.h index b7e18bde5aa8..ad8b579d67d3 100644 --- a/include/linux/vfio.h +++ b/include/linux/vfio.h @@ -15,6 +15,18 @@ #include #include +struct vfio_device { + struct device *dev; + const struct vfio_device_ops *ops; + struct vfio_group *group; + + /* Members below here are private, not for driver use */ + refcount_t refcount; + struct completion comp; + struct list_head group_next; + void *device_data; +}; + /** * struct vfio_device_ops - VFIO bus driver device callbacks * @@ -48,11 +60,15 @@ struct vfio_device_ops { extern struct iommu_group *vfio_iommu_group_get(struct device *dev); extern void vfio_iommu_group_put(struct iommu_group *group, struct device *dev); +void vfio_init_group_dev(struct vfio_device *device, struct device *dev, + const struct vfio_device_ops *ops, void *device_data); +int vfio_register_group_dev(struct vfio_device *device); extern int vfio_add_group_dev(struct device *dev, const struct vfio_device_ops *ops, void *device_data); extern void *vfio_del_group_dev(struct device *dev); +void vfio_unregister_group_dev(struct vfio_device *device); extern struct vfio_device *vfio_device_get_from_dev(struct device *dev); extern void vfio_device_put(struct vfio_device *device); extern void *vfio_device_data(struct vfio_device *device); -- cgit v1.2.3 From cb6164586814bbe41f55559ae5505d8373d6f781 Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Tue, 30 Mar 2021 09:53:06 -0600 Subject: vfio/platform: Use vfio_init/register/unregister_group_dev platform already allocates a struct vfio_platform_device with exactly the same lifetime as vfio_device, switch to the new API and embed vfio_device in vfio_platform_device. Reviewed-by: Christoph Hellwig Reviewed-by: Cornelia Huck Acked-by: Eric Auger Tested-by: Eric Auger Signed-off-by: Jason Gunthorpe Message-Id: <4-v3-225de1400dfc+4e074-vfio1_jgg@nvidia.com> Signed-off-by: Alex Williamson --- drivers/vfio/platform/vfio_amba.c | 8 +++++--- drivers/vfio/platform/vfio_platform.c | 20 +++++++++----------- drivers/vfio/platform/vfio_platform_common.c | 23 ++++++++--------------- drivers/vfio/platform/vfio_platform_private.h | 5 +++-- 4 files changed, 25 insertions(+), 31 deletions(-) (limited to 'drivers') diff --git a/drivers/vfio/platform/vfio_amba.c b/drivers/vfio/platform/vfio_amba.c index 3626c2150101..f970eb2a999f 100644 --- a/drivers/vfio/platform/vfio_amba.c +++ b/drivers/vfio/platform/vfio_amba.c @@ -66,16 +66,18 @@ static int vfio_amba_probe(struct amba_device *adev, const struct amba_id *id) if (ret) { kfree(vdev->name); kfree(vdev); + return ret; } - return ret; + dev_set_drvdata(&adev->dev, vdev); + return 0; } static void vfio_amba_remove(struct amba_device *adev) { - struct vfio_platform_device *vdev = - vfio_platform_remove_common(&adev->dev); + struct vfio_platform_device *vdev = dev_get_drvdata(&adev->dev); + vfio_platform_remove_common(vdev); kfree(vdev->name); kfree(vdev); } diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c index 9fb6818cea12..e4027799a154 100644 --- a/drivers/vfio/platform/vfio_platform.c +++ b/drivers/vfio/platform/vfio_platform.c @@ -54,23 +54,21 @@ static int vfio_platform_probe(struct platform_device *pdev) vdev->reset_required = reset_required; ret = vfio_platform_probe_common(vdev, &pdev->dev); - if (ret) + if (ret) { kfree(vdev); - - return ret; + return ret; + } + dev_set_drvdata(&pdev->dev, vdev); + return 0; } static int vfio_platform_remove(struct platform_device *pdev) { - struct vfio_platform_device *vdev; - - vdev = vfio_platform_remove_common(&pdev->dev); - if (vdev) { - kfree(vdev); - return 0; - } + struct vfio_platform_device *vdev = dev_get_drvdata(&pdev->dev); - return -EINVAL; + vfio_platform_remove_common(vdev); + kfree(vdev); + return 0; } static struct platform_driver vfio_platform_driver = { diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c index fb4b385191f2..6eb749250ee4 100644 --- a/drivers/vfio/platform/vfio_platform_common.c +++ b/drivers/vfio/platform/vfio_platform_common.c @@ -659,8 +659,7 @@ int vfio_platform_probe_common(struct vfio_platform_device *vdev, struct iommu_group *group; int ret; - if (!vdev) - return -EINVAL; + vfio_init_group_dev(&vdev->vdev, dev, &vfio_platform_ops, vdev); ret = vfio_platform_acpi_probe(vdev, dev); if (ret) @@ -685,13 +684,13 @@ int vfio_platform_probe_common(struct vfio_platform_device *vdev, goto put_reset; } - ret = vfio_add_group_dev(dev, &vfio_platform_ops, vdev); + ret = vfio_register_group_dev(&vdev->vdev); if (ret) goto put_iommu; mutex_init(&vdev->igate); - pm_runtime_enable(vdev->device); + pm_runtime_enable(dev); return 0; put_iommu: @@ -702,19 +701,13 @@ put_reset: } EXPORT_SYMBOL_GPL(vfio_platform_probe_common); -struct vfio_platform_device *vfio_platform_remove_common(struct device *dev) +void vfio_platform_remove_common(struct vfio_platform_device *vdev) { - struct vfio_platform_device *vdev; - - vdev = vfio_del_group_dev(dev); + vfio_unregister_group_dev(&vdev->vdev); - if (vdev) { - pm_runtime_disable(vdev->device); - vfio_platform_put_reset(vdev); - vfio_iommu_group_put(dev->iommu_group, dev); - } - - return vdev; + pm_runtime_disable(vdev->device); + vfio_platform_put_reset(vdev); + vfio_iommu_group_put(vdev->vdev.dev->iommu_group, vdev->vdev.dev); } EXPORT_SYMBOL_GPL(vfio_platform_remove_common); diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h index 289089910643..a5ba82c8cbc3 100644 --- a/drivers/vfio/platform/vfio_platform_private.h +++ b/drivers/vfio/platform/vfio_platform_private.h @@ -9,6 +9,7 @@ #include #include +#include #define VFIO_PLATFORM_OFFSET_SHIFT 40 #define VFIO_PLATFORM_OFFSET_MASK (((u64)(1) << VFIO_PLATFORM_OFFSET_SHIFT) - 1) @@ -42,6 +43,7 @@ struct vfio_platform_region { }; struct vfio_platform_device { + struct vfio_device vdev; struct vfio_platform_region *regions; u32 num_regions; struct vfio_platform_irq *irqs; @@ -80,8 +82,7 @@ struct vfio_platform_reset_node { extern int vfio_platform_probe_common(struct vfio_platform_device *vdev, struct device *dev); -extern struct vfio_platform_device *vfio_platform_remove_common - (struct device *dev); +void vfio_platform_remove_common(struct vfio_platform_device *vdev); extern int vfio_platform_irq_init(struct vfio_platform_device *vdev); extern void vfio_platform_irq_cleanup(struct vfio_platform_device *vdev); -- cgit v1.2.3 From 2b1fe162e584a88ec7f12a651a2a50f94dd8cfac Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Tue, 30 Mar 2021 09:53:06 -0600 Subject: vfio/fsl-mc: Re-order vfio_fsl_mc_probe() vfio_add_group_dev() must be called only after all of the private data in vdev is fully setup and ready, otherwise there could be races with user space instantiating a device file descriptor and starting to call ops. For instance vfio_fsl_mc_reflck_attach() sets vdev->reflck and vfio_fsl_mc_open(), called by fops open, unconditionally derefs it, which will crash if things get out of order. This driver started life with the right sequence, but two commits added stuff after vfio_add_group_dev(). Fixes: 2e0d29561f59 ("vfio/fsl-mc: Add irq infrastructure for fsl-mc devices") Fixes: f2ba7e8c947b ("vfio/fsl-mc: Added lock support in preparation for interrupt handling") Co-developed-by: Diana Craciun OSS Signed-off-by: Jason Gunthorpe Message-Id: <5-v3-225de1400dfc+4e074-vfio1_jgg@nvidia.com> Signed-off-by: Alex Williamson --- drivers/vfio/fsl-mc/vfio_fsl_mc.c | 74 +++++++++++++++++++++++++-------------- 1 file changed, 47 insertions(+), 27 deletions(-) (limited to 'drivers') diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c index f27e25112c40..8722f5effacd 100644 --- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c @@ -568,23 +568,39 @@ static int vfio_fsl_mc_init_device(struct vfio_fsl_mc_device *vdev) dev_err(&mc_dev->dev, "VFIO_FSL_MC: Failed to setup DPRC (%d)\n", ret); goto out_nc_unreg; } + return 0; + +out_nc_unreg: + bus_unregister_notifier(&fsl_mc_bus_type, &vdev->nb); + return ret; +} +static int vfio_fsl_mc_scan_container(struct fsl_mc_device *mc_dev) +{ + int ret; + + /* non dprc devices do not scan for other devices */ + if (!is_fsl_mc_bus_dprc(mc_dev)) + return 0; ret = dprc_scan_container(mc_dev, false); if (ret) { - dev_err(&mc_dev->dev, "VFIO_FSL_MC: Container scanning failed (%d)\n", ret); - goto out_dprc_cleanup; + dev_err(&mc_dev->dev, + "VFIO_FSL_MC: Container scanning failed (%d)\n", ret); + dprc_remove_devices(mc_dev, NULL, 0); + return ret; } - return 0; +} + +static void vfio_fsl_uninit_device(struct vfio_fsl_mc_device *vdev) +{ + struct fsl_mc_device *mc_dev = vdev->mc_dev; + + if (!is_fsl_mc_bus_dprc(mc_dev)) + return; -out_dprc_cleanup: - dprc_remove_devices(mc_dev, NULL, 0); dprc_cleanup(mc_dev); -out_nc_unreg: bus_unregister_notifier(&fsl_mc_bus_type, &vdev->nb); - vdev->nb.notifier_call = NULL; - - return ret; } static int vfio_fsl_mc_probe(struct fsl_mc_device *mc_dev) @@ -607,29 +623,39 @@ static int vfio_fsl_mc_probe(struct fsl_mc_device *mc_dev) } vdev->mc_dev = mc_dev; - - ret = vfio_add_group_dev(dev, &vfio_fsl_mc_ops, vdev); - if (ret) { - dev_err(dev, "VFIO_FSL_MC: Failed to add to vfio group\n"); - goto out_group_put; - } + mutex_init(&vdev->igate); ret = vfio_fsl_mc_reflck_attach(vdev); if (ret) - goto out_group_dev; + goto out_group_put; ret = vfio_fsl_mc_init_device(vdev); if (ret) goto out_reflck; - mutex_init(&vdev->igate); + ret = vfio_add_group_dev(dev, &vfio_fsl_mc_ops, vdev); + if (ret) { + dev_err(dev, "VFIO_FSL_MC: Failed to add to vfio group\n"); + goto out_device; + } + /* + * This triggers recursion into vfio_fsl_mc_probe() on another device + * and the vfio_fsl_mc_reflck_attach() must succeed, which relies on the + * vfio_add_group_dev() above. It has no impact on this vdev, so it is + * safe to be after the vfio device is made live. + */ + ret = vfio_fsl_mc_scan_container(mc_dev); + if (ret) + goto out_group_dev; return 0; -out_reflck: - vfio_fsl_mc_reflck_put(vdev->reflck); out_group_dev: vfio_del_group_dev(dev); +out_device: + vfio_fsl_uninit_device(vdev); +out_reflck: + vfio_fsl_mc_reflck_put(vdev->reflck); out_group_put: vfio_iommu_group_put(group, dev); return ret; @@ -646,16 +672,10 @@ static int vfio_fsl_mc_remove(struct fsl_mc_device *mc_dev) mutex_destroy(&vdev->igate); + dprc_remove_devices(mc_dev, NULL, 0); + vfio_fsl_uninit_device(vdev); vfio_fsl_mc_reflck_put(vdev->reflck); - if (is_fsl_mc_bus_dprc(mc_dev)) { - dprc_remove_devices(mc_dev, NULL, 0); - dprc_cleanup(mc_dev); - } - - if (vdev->nb.notifier_call) - bus_unregister_notifier(&fsl_mc_bus_type, &vdev->nb); - vfio_iommu_group_put(mc_dev->dev.iommu_group, dev); return 0; -- cgit v1.2.3 From 0ca78666fa06cf2a7d068a593428dc4039706e00 Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Tue, 30 Mar 2021 09:53:06 -0600 Subject: vfio/fsl-mc: Use vfio_init/register/unregister_group_dev fsl-mc already allocates a struct vfio_fsl_mc_device with exactly the same lifetime as vfio_device, switch to the new API and embed vfio_device in vfio_fsl_mc_device. While here remove the devm usage for the vdev, this code is clean and doesn't need devm. Reviewed-by: Christoph Hellwig Reviewed-by: Cornelia Huck Signed-off-by: Jason Gunthorpe Message-Id: <6-v3-225de1400dfc+4e074-vfio1_jgg@nvidia.com> Signed-off-by: Alex Williamson --- drivers/vfio/fsl-mc/vfio_fsl_mc.c | 20 +++++++++++--------- drivers/vfio/fsl-mc/vfio_fsl_mc_private.h | 1 + 2 files changed, 12 insertions(+), 9 deletions(-) (limited to 'drivers') diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c index 8722f5effacd..ad6c6feeeb4b 100644 --- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c @@ -616,24 +616,25 @@ static int vfio_fsl_mc_probe(struct fsl_mc_device *mc_dev) return -EINVAL; } - vdev = devm_kzalloc(dev, sizeof(*vdev), GFP_KERNEL); + vdev = kzalloc(sizeof(*vdev), GFP_KERNEL); if (!vdev) { ret = -ENOMEM; goto out_group_put; } + vfio_init_group_dev(&vdev->vdev, dev, &vfio_fsl_mc_ops, vdev); vdev->mc_dev = mc_dev; mutex_init(&vdev->igate); ret = vfio_fsl_mc_reflck_attach(vdev); if (ret) - goto out_group_put; + goto out_kfree; ret = vfio_fsl_mc_init_device(vdev); if (ret) goto out_reflck; - ret = vfio_add_group_dev(dev, &vfio_fsl_mc_ops, vdev); + ret = vfio_register_group_dev(&vdev->vdev); if (ret) { dev_err(dev, "VFIO_FSL_MC: Failed to add to vfio group\n"); goto out_device; @@ -648,14 +649,17 @@ static int vfio_fsl_mc_probe(struct fsl_mc_device *mc_dev) ret = vfio_fsl_mc_scan_container(mc_dev); if (ret) goto out_group_dev; + dev_set_drvdata(dev, vdev); return 0; out_group_dev: - vfio_del_group_dev(dev); + vfio_unregister_group_dev(&vdev->vdev); out_device: vfio_fsl_uninit_device(vdev); out_reflck: vfio_fsl_mc_reflck_put(vdev->reflck); +out_kfree: + kfree(vdev); out_group_put: vfio_iommu_group_put(group, dev); return ret; @@ -663,19 +667,17 @@ out_group_put: static int vfio_fsl_mc_remove(struct fsl_mc_device *mc_dev) { - struct vfio_fsl_mc_device *vdev; struct device *dev = &mc_dev->dev; + struct vfio_fsl_mc_device *vdev = dev_get_drvdata(dev); - vdev = vfio_del_group_dev(dev); - if (!vdev) - return -EINVAL; - + vfio_unregister_group_dev(&vdev->vdev); mutex_destroy(&vdev->igate); dprc_remove_devices(mc_dev, NULL, 0); vfio_fsl_uninit_device(vdev); vfio_fsl_mc_reflck_put(vdev->reflck); + kfree(vdev); vfio_iommu_group_put(mc_dev->dev.iommu_group, dev); return 0; diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h b/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h index a97ee691ed47..89700e00e77d 100644 --- a/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h @@ -36,6 +36,7 @@ struct vfio_fsl_mc_region { }; struct vfio_fsl_mc_device { + struct vfio_device vdev; struct fsl_mc_device *mc_dev; struct notifier_block nb; int refcnt; -- cgit v1.2.3 From 61e90817482871b614133c0f20feb1aba2faec86 Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Tue, 30 Mar 2021 09:53:06 -0600 Subject: vfio/pci: Move VGA and VF initialization to functions vfio_pci_probe() is quite complicated, with optional VF and VGA sub components. Move these into clear init/uninit functions and have a linear flow in probe/remove. This fixes a few little buglets: - vfio_pci_remove() is in the wrong order, vga_client_register() removes a notifier and is after kfree(vdev), but the notifier refers to vdev, so it can use after free in a race. - vga_client_register() can fail but was ignored Organize things so destruction order is the reverse of creation order. Fixes: ecaa1f6a0154 ("vfio-pci: Add VGA arbiter client") Reviewed-by: Christoph Hellwig Reviewed-by: Kevin Tian Reviewed-by: Max Gurtovoy Reviewed-by: Cornelia Huck Reviewed-by: Eric Auger Signed-off-by: Jason Gunthorpe Message-Id: <7-v3-225de1400dfc+4e074-vfio1_jgg@nvidia.com> Signed-off-by: Alex Williamson --- drivers/vfio/pci/vfio_pci.c | 116 ++++++++++++++++++++++++++++---------------- 1 file changed, 74 insertions(+), 42 deletions(-) (limited to 'drivers') diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c index 65e7e6b44578..f95b58376156 100644 --- a/drivers/vfio/pci/vfio_pci.c +++ b/drivers/vfio/pci/vfio_pci.c @@ -1922,6 +1922,68 @@ static int vfio_pci_bus_notifier(struct notifier_block *nb, return 0; } +static int vfio_pci_vf_init(struct vfio_pci_device *vdev) +{ + struct pci_dev *pdev = vdev->pdev; + int ret; + + if (!pdev->is_physfn) + return 0; + + vdev->vf_token = kzalloc(sizeof(*vdev->vf_token), GFP_KERNEL); + if (!vdev->vf_token) + return -ENOMEM; + + mutex_init(&vdev->vf_token->lock); + uuid_gen(&vdev->vf_token->uuid); + + vdev->nb.notifier_call = vfio_pci_bus_notifier; + ret = bus_register_notifier(&pci_bus_type, &vdev->nb); + if (ret) { + kfree(vdev->vf_token); + return ret; + } + return 0; +} + +static void vfio_pci_vf_uninit(struct vfio_pci_device *vdev) +{ + if (!vdev->vf_token) + return; + + bus_unregister_notifier(&pci_bus_type, &vdev->nb); + WARN_ON(vdev->vf_token->users); + mutex_destroy(&vdev->vf_token->lock); + kfree(vdev->vf_token); +} + +static int vfio_pci_vga_init(struct vfio_pci_device *vdev) +{ + struct pci_dev *pdev = vdev->pdev; + int ret; + + if (!vfio_pci_is_vga(pdev)) + return 0; + + ret = vga_client_register(pdev, vdev, NULL, vfio_pci_set_vga_decode); + if (ret) + return ret; + vga_set_legacy_decoding(pdev, vfio_pci_set_vga_decode(vdev, false)); + return 0; +} + +static void vfio_pci_vga_uninit(struct vfio_pci_device *vdev) +{ + struct pci_dev *pdev = vdev->pdev; + + if (!vfio_pci_is_vga(pdev)) + return; + vga_client_register(pdev, NULL, NULL, NULL); + vga_set_legacy_decoding(pdev, VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM | + VGA_RSRC_LEGACY_IO | + VGA_RSRC_LEGACY_MEM); +} + static int vfio_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) { struct vfio_pci_device *vdev; @@ -1975,28 +2037,12 @@ static int vfio_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) ret = vfio_pci_reflck_attach(vdev); if (ret) goto out_del_group_dev; - - if (pdev->is_physfn) { - vdev->vf_token = kzalloc(sizeof(*vdev->vf_token), GFP_KERNEL); - if (!vdev->vf_token) { - ret = -ENOMEM; - goto out_reflck; - } - - mutex_init(&vdev->vf_token->lock); - uuid_gen(&vdev->vf_token->uuid); - - vdev->nb.notifier_call = vfio_pci_bus_notifier; - ret = bus_register_notifier(&pci_bus_type, &vdev->nb); - if (ret) - goto out_vf_token; - } - - if (vfio_pci_is_vga(pdev)) { - vga_client_register(pdev, vdev, NULL, vfio_pci_set_vga_decode); - vga_set_legacy_decoding(pdev, - vfio_pci_set_vga_decode(vdev, false)); - } + ret = vfio_pci_vf_init(vdev); + if (ret) + goto out_reflck; + ret = vfio_pci_vga_init(vdev); + if (ret) + goto out_vf; vfio_pci_probe_power_state(vdev); @@ -2016,8 +2062,8 @@ static int vfio_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) return ret; -out_vf_token: - kfree(vdev->vf_token); +out_vf: + vfio_pci_vf_uninit(vdev); out_reflck: vfio_pci_reflck_put(vdev->reflck); out_del_group_dev: @@ -2039,33 +2085,19 @@ static void vfio_pci_remove(struct pci_dev *pdev) if (!vdev) return; - if (vdev->vf_token) { - WARN_ON(vdev->vf_token->users); - mutex_destroy(&vdev->vf_token->lock); - kfree(vdev->vf_token); - } - - if (vdev->nb.notifier_call) - bus_unregister_notifier(&pci_bus_type, &vdev->nb); - + vfio_pci_vf_uninit(vdev); vfio_pci_reflck_put(vdev->reflck); + vfio_pci_vga_uninit(vdev); vfio_iommu_group_put(pdev->dev.iommu_group, &pdev->dev); - kfree(vdev->region); - mutex_destroy(&vdev->ioeventfds_lock); if (!disable_idle_d3) vfio_pci_set_power_state(vdev, PCI_D0); + mutex_destroy(&vdev->ioeventfds_lock); + kfree(vdev->region); kfree(vdev->pm_save); kfree(vdev); - - if (vfio_pci_is_vga(pdev)) { - vga_client_register(pdev, NULL, NULL, NULL); - vga_set_legacy_decoding(pdev, - VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM | - VGA_RSRC_LEGACY_IO | VGA_RSRC_LEGACY_MEM); - } } static pci_ers_result_t vfio_pci_aer_err_detected(struct pci_dev *pdev, -- cgit v1.2.3 From 4aeec3984ddc853f7c65903bde472ffdef738bae Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Tue, 30 Mar 2021 09:53:06 -0600 Subject: vfio/pci: Re-order vfio_pci_probe() vfio_add_group_dev() must be called only after all of the private data in vdev is fully setup and ready, otherwise there could be races with user space instantiating a device file descriptor and starting to call ops. For instance vfio_pci_reflck_attach() sets vdev->reflck and vfio_pci_open(), called by fops open, unconditionally derefs it, which will crash if things get out of order. Fixes: cc20d7999000 ("vfio/pci: Introduce VF token") Fixes: e309df5b0c9e ("vfio/pci: Parallelize device open and release") Fixes: 6eb7018705de ("vfio-pci: Move idle devices to D3hot power state") Fixes: ecaa1f6a0154 ("vfio-pci: Add VGA arbiter client") Reviewed-by: Christoph Hellwig Reviewed-by: Max Gurtovoy Reviewed-by: Kevin Tian Reviewed-by: Cornelia Huck Reviewed-by: Eric Auger Signed-off-by: Jason Gunthorpe Message-Id: <8-v3-225de1400dfc+4e074-vfio1_jgg@nvidia.com> Signed-off-by: Alex Williamson --- drivers/vfio/pci/vfio_pci.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) (limited to 'drivers') diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c index f95b58376156..0e7682e7a0b4 100644 --- a/drivers/vfio/pci/vfio_pci.c +++ b/drivers/vfio/pci/vfio_pci.c @@ -2030,13 +2030,9 @@ static int vfio_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) INIT_LIST_HEAD(&vdev->vma_list); init_rwsem(&vdev->memory_lock); - ret = vfio_add_group_dev(&pdev->dev, &vfio_pci_ops, vdev); - if (ret) - goto out_free; - ret = vfio_pci_reflck_attach(vdev); if (ret) - goto out_del_group_dev; + goto out_free; ret = vfio_pci_vf_init(vdev); if (ret) goto out_reflck; @@ -2060,15 +2056,20 @@ static int vfio_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) vfio_pci_set_power_state(vdev, PCI_D3hot); } - return ret; + ret = vfio_add_group_dev(&pdev->dev, &vfio_pci_ops, vdev); + if (ret) + goto out_power; + return 0; +out_power: + if (!disable_idle_d3) + vfio_pci_set_power_state(vdev, PCI_D0); out_vf: vfio_pci_vf_uninit(vdev); out_reflck: vfio_pci_reflck_put(vdev->reflck); -out_del_group_dev: - vfio_del_group_dev(&pdev->dev); out_free: + kfree(vdev->pm_save); kfree(vdev); out_group_put: vfio_iommu_group_put(group, &pdev->dev); -- cgit v1.2.3 From 6b018e203d5effc97961cd9477687fe09e2fe79f Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Tue, 30 Mar 2021 09:53:07 -0600 Subject: vfio/pci: Use vfio_init/register/unregister_group_dev pci already allocates a struct vfio_pci_device with exactly the same lifetime as vfio_device, switch to the new API and embed vfio_device in vfio_pci_device. Reviewed-by: Christoph Hellwig Reviewed-by: Liu Yi L Reviewed-by: Kevin Tian Reviewed-by: Cornelia Huck Reviewed-by: Eric Auger Signed-off-by: Jason Gunthorpe Message-Id: <9-v3-225de1400dfc+4e074-vfio1_jgg@nvidia.com> Signed-off-by: Alex Williamson --- drivers/vfio/pci/vfio_pci.c | 10 +++++----- drivers/vfio/pci/vfio_pci_private.h | 1 + 2 files changed, 6 insertions(+), 5 deletions(-) (limited to 'drivers') diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c index 0e7682e7a0b4..a0ac20a499cf 100644 --- a/drivers/vfio/pci/vfio_pci.c +++ b/drivers/vfio/pci/vfio_pci.c @@ -2019,6 +2019,7 @@ static int vfio_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) goto out_group_put; } + vfio_init_group_dev(&vdev->vdev, &pdev->dev, &vfio_pci_ops, vdev); vdev->pdev = pdev; vdev->irq_type = VFIO_PCI_NUM_IRQS; mutex_init(&vdev->igate); @@ -2056,9 +2057,10 @@ static int vfio_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) vfio_pci_set_power_state(vdev, PCI_D3hot); } - ret = vfio_add_group_dev(&pdev->dev, &vfio_pci_ops, vdev); + ret = vfio_register_group_dev(&vdev->vdev); if (ret) goto out_power; + dev_set_drvdata(&pdev->dev, vdev); return 0; out_power: @@ -2078,13 +2080,11 @@ out_group_put: static void vfio_pci_remove(struct pci_dev *pdev) { - struct vfio_pci_device *vdev; + struct vfio_pci_device *vdev = dev_get_drvdata(&pdev->dev); pci_disable_sriov(pdev); - vdev = vfio_del_group_dev(&pdev->dev); - if (!vdev) - return; + vfio_unregister_group_dev(&vdev->vdev); vfio_pci_vf_uninit(vdev); vfio_pci_reflck_put(vdev->reflck); diff --git a/drivers/vfio/pci/vfio_pci_private.h b/drivers/vfio/pci/vfio_pci_private.h index 9cd1882a05af..8755a0febd05 100644 --- a/drivers/vfio/pci/vfio_pci_private.h +++ b/drivers/vfio/pci/vfio_pci_private.h @@ -100,6 +100,7 @@ struct vfio_pci_mmap_vma { }; struct vfio_pci_device { + struct vfio_device vdev; struct pci_dev *pdev; void __iomem *barmap[PCI_STD_NUM_BARS]; bool bar_mmap_supported[PCI_STD_NUM_BARS]; -- cgit v1.2.3 From 1ae1b20f6f2c67659c963e5fe58f9b4a47df9f12 Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Tue, 30 Mar 2021 09:53:07 -0600 Subject: vfio/mdev: Use vfio_init/register/unregister_group_dev mdev gets little benefit because it doesn't actually do anything, however it is the last user, so move the vfio_init/register/unregister_group_dev() code here for now. Reviewed-by: Christoph Hellwig Reviewed-by: Liu Yi L Reviewed-by: Kevin Tian Reviewed-by: Cornelia Huck Signed-off-by: Jason Gunthorpe Message-Id: <10-v3-225de1400dfc+4e074-vfio1_jgg@nvidia.com> Signed-off-by: Alex Williamson --- drivers/vfio/mdev/vfio_mdev.c | 20 ++++++++++++++++++-- drivers/vfio/vfio.c | 39 ++------------------------------------- include/linux/vfio.h | 5 ----- 3 files changed, 20 insertions(+), 44 deletions(-) (limited to 'drivers') diff --git a/drivers/vfio/mdev/vfio_mdev.c b/drivers/vfio/mdev/vfio_mdev.c index b52eea128549..4043cc91f952 100644 --- a/drivers/vfio/mdev/vfio_mdev.c +++ b/drivers/vfio/mdev/vfio_mdev.c @@ -124,13 +124,29 @@ static const struct vfio_device_ops vfio_mdev_dev_ops = { static int vfio_mdev_probe(struct device *dev) { struct mdev_device *mdev = to_mdev_device(dev); + struct vfio_device *vdev; + int ret; - return vfio_add_group_dev(dev, &vfio_mdev_dev_ops, mdev); + vdev = kzalloc(sizeof(*vdev), GFP_KERNEL); + if (!vdev) + return -ENOMEM; + + vfio_init_group_dev(vdev, &mdev->dev, &vfio_mdev_dev_ops, mdev); + ret = vfio_register_group_dev(vdev); + if (ret) { + kfree(vdev); + return ret; + } + dev_set_drvdata(&mdev->dev, vdev); + return 0; } static void vfio_mdev_remove(struct device *dev) { - vfio_del_group_dev(dev); + struct vfio_device *vdev = dev_get_drvdata(dev); + + vfio_unregister_group_dev(vdev); + kfree(vdev); } static struct mdev_driver vfio_mdev_driver = { diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c index 2ea430de505b..180b4ab02d11 100644 --- a/drivers/vfio/vfio.c +++ b/drivers/vfio/vfio.c @@ -99,8 +99,8 @@ MODULE_PARM_DESC(enable_unsafe_noiommu_mode, "Enable UNSAFE, no-IOMMU mode. Thi /* * vfio_iommu_group_{get,put} are only intended for VFIO bus driver probe * and remove functions, any use cases other than acquiring the first - * reference for the purpose of calling vfio_add_group_dev() or removing - * that symmetric reference after vfio_del_group_dev() should use the raw + * reference for the purpose of calling vfio_register_group_dev() or removing + * that symmetric reference after vfio_unregister_group_dev() should use the raw * iommu_group_{get,put} functions. In particular, vfio_iommu_group_put() * removes the device from the dummy group and cannot be nested. */ @@ -799,29 +799,6 @@ int vfio_register_group_dev(struct vfio_device *device) } EXPORT_SYMBOL_GPL(vfio_register_group_dev); -int vfio_add_group_dev(struct device *dev, const struct vfio_device_ops *ops, - void *device_data) -{ - struct vfio_device *device; - int ret; - - device = kzalloc(sizeof(*device), GFP_KERNEL); - if (!device) - return -ENOMEM; - - vfio_init_group_dev(device, dev, ops, device_data); - ret = vfio_register_group_dev(device); - if (ret) - goto err_kfree; - dev_set_drvdata(dev, device); - return 0; - -err_kfree: - kfree(device); - return ret; -} -EXPORT_SYMBOL_GPL(vfio_add_group_dev); - /** * Get a reference to the vfio_device for a device. Even if the * caller thinks they own the device, they could be racing with a @@ -962,18 +939,6 @@ void vfio_unregister_group_dev(struct vfio_device *device) } EXPORT_SYMBOL_GPL(vfio_unregister_group_dev); -void *vfio_del_group_dev(struct device *dev) -{ - struct vfio_device *device = dev_get_drvdata(dev); - void *device_data = device->device_data; - - vfio_unregister_group_dev(device); - dev_set_drvdata(dev, NULL); - kfree(device); - return device_data; -} -EXPORT_SYMBOL_GPL(vfio_del_group_dev); - /** * VFIO base fd, /dev/vfio/vfio */ diff --git a/include/linux/vfio.h b/include/linux/vfio.h index ad8b579d67d3..4995faf51efe 100644 --- a/include/linux/vfio.h +++ b/include/linux/vfio.h @@ -63,11 +63,6 @@ extern void vfio_iommu_group_put(struct iommu_group *group, struct device *dev); void vfio_init_group_dev(struct vfio_device *device, struct device *dev, const struct vfio_device_ops *ops, void *device_data); int vfio_register_group_dev(struct vfio_device *device); -extern int vfio_add_group_dev(struct device *dev, - const struct vfio_device_ops *ops, - void *device_data); - -extern void *vfio_del_group_dev(struct device *dev); void vfio_unregister_group_dev(struct vfio_device *device); extern struct vfio_device *vfio_device_get_from_dev(struct device *dev); extern void vfio_device_put(struct vfio_device *device); -- cgit v1.2.3 From 66873b5fa738ca02b5c075ca4a410b13d88e6e9a Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Tue, 30 Mar 2021 09:53:07 -0600 Subject: vfio/mdev: Make to_mdev_device() into a static inline The macro wrongly uses 'dev' as both the macro argument and the member name, which means it fails compilation if any caller uses a word other than 'dev' as the single argument. Fix this defect by making it into proper static inline, which is more clear and typesafe anyhow. Fixes: 99e3123e3d72 ("vfio-mdev: Make mdev_device private and abstract interfaces") Reviewed-by: Christoph Hellwig Reviewed-by: Kevin Tian Reviewed-by: Cornelia Huck Signed-off-by: Jason Gunthorpe Message-Id: <11-v3-225de1400dfc+4e074-vfio1_jgg@nvidia.com> Signed-off-by: Alex Williamson --- drivers/vfio/mdev/mdev_private.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/vfio/mdev/mdev_private.h b/drivers/vfio/mdev/mdev_private.h index 7d922950caaf..74c2e5411469 100644 --- a/drivers/vfio/mdev/mdev_private.h +++ b/drivers/vfio/mdev/mdev_private.h @@ -35,7 +35,10 @@ struct mdev_device { bool active; }; -#define to_mdev_device(dev) container_of(dev, struct mdev_device, dev) +static inline struct mdev_device *to_mdev_device(struct device *dev) +{ + return container_of(dev, struct mdev_device, dev); +} #define dev_is_mdev(d) ((d)->bus == &mdev_bus_type) struct mdev_type { -- cgit v1.2.3 From 6df62c5b05f4ad6876815ea8b8775905a090224a Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Tue, 30 Mar 2021 09:53:08 -0600 Subject: vfio: Make vfio_device_ops pass a 'struct vfio_device *' instead of 'void *' This is the standard kernel pattern, the ops associated with a struct get the struct pointer in for typesafety. The expected design is to use container_of to cleanly go from the subsystem level type to the driver level type without having any type erasure in a void *. Reviewed-by: Dan Williams Reviewed-by: Christoph Hellwig Reviewed-by: Cornelia Huck Signed-off-by: Jason Gunthorpe Message-Id: <12-v3-225de1400dfc+4e074-vfio1_jgg@nvidia.com> Signed-off-by: Alex Williamson --- Documentation/driver-api/vfio.rst | 18 ++++++----- drivers/vfio/fsl-mc/vfio_fsl_mc.c | 36 ++++++++++++--------- drivers/vfio/mdev/vfio_mdev.c | 33 ++++++++++--------- drivers/vfio/pci/vfio_pci.c | 47 +++++++++++++++++----------- drivers/vfio/platform/vfio_platform_common.c | 33 +++++++++++-------- drivers/vfio/vfio.c | 20 ++++++------ include/linux/vfio.h | 16 +++++----- 7 files changed, 117 insertions(+), 86 deletions(-) (limited to 'drivers') diff --git a/Documentation/driver-api/vfio.rst b/Documentation/driver-api/vfio.rst index d3a02300913a..3337f337293a 100644 --- a/Documentation/driver-api/vfio.rst +++ b/Documentation/driver-api/vfio.rst @@ -269,20 +269,22 @@ ready before calling it. The driver provides an ops structure for callbacks similar to a file operations structure:: struct vfio_device_ops { - int (*open)(void *device_data); - void (*release)(void *device_data); - ssize_t (*read)(void *device_data, char __user *buf, + int (*open)(struct vfio_device *vdev); + void (*release)(struct vfio_device *vdev); + ssize_t (*read)(struct vfio_device *vdev, char __user *buf, size_t count, loff_t *ppos); - ssize_t (*write)(void *device_data, const char __user *buf, + ssize_t (*write)(struct vfio_device *vdev, + const char __user *buf, size_t size, loff_t *ppos); - long (*ioctl)(void *device_data, unsigned int cmd, + long (*ioctl)(struct vfio_device *vdev, unsigned int cmd, unsigned long arg); - int (*mmap)(void *device_data, struct vm_area_struct *vma); + int (*mmap)(struct vfio_device *vdev, + struct vm_area_struct *vma); }; -Each function is passed the device_data that was originally registered +Each function is passed the vdev that was originally registered in the vfio_register_group_dev() call above. This allows the bus driver -an easy place to store its opaque, private data. The open/release +to obtain its private data using container_of(). The open/release callbacks are issued when a new file descriptor is created for a device (via VFIO_GROUP_GET_DEVICE_FD). The ioctl interface provides a direct pass through for VFIO_DEVICE_* ioctls. The read/write/mmap diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c index ad6c6feeeb4b..45f397c04a89 100644 --- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c @@ -135,9 +135,10 @@ static void vfio_fsl_mc_regions_cleanup(struct vfio_fsl_mc_device *vdev) kfree(vdev->regions); } -static int vfio_fsl_mc_open(void *device_data) +static int vfio_fsl_mc_open(struct vfio_device *core_vdev) { - struct vfio_fsl_mc_device *vdev = device_data; + struct vfio_fsl_mc_device *vdev = + container_of(core_vdev, struct vfio_fsl_mc_device, vdev); int ret; if (!try_module_get(THIS_MODULE)) @@ -161,9 +162,10 @@ err_reg_init: return ret; } -static void vfio_fsl_mc_release(void *device_data) +static void vfio_fsl_mc_release(struct vfio_device *core_vdev) { - struct vfio_fsl_mc_device *vdev = device_data; + struct vfio_fsl_mc_device *vdev = + container_of(core_vdev, struct vfio_fsl_mc_device, vdev); int ret; mutex_lock(&vdev->reflck->lock); @@ -197,11 +199,12 @@ static void vfio_fsl_mc_release(void *device_data) module_put(THIS_MODULE); } -static long vfio_fsl_mc_ioctl(void *device_data, unsigned int cmd, - unsigned long arg) +static long vfio_fsl_mc_ioctl(struct vfio_device *core_vdev, + unsigned int cmd, unsigned long arg) { unsigned long minsz; - struct vfio_fsl_mc_device *vdev = device_data; + struct vfio_fsl_mc_device *vdev = + container_of(core_vdev, struct vfio_fsl_mc_device, vdev); struct fsl_mc_device *mc_dev = vdev->mc_dev; switch (cmd) { @@ -327,10 +330,11 @@ static long vfio_fsl_mc_ioctl(void *device_data, unsigned int cmd, } } -static ssize_t vfio_fsl_mc_read(void *device_data, char __user *buf, +static ssize_t vfio_fsl_mc_read(struct vfio_device *core_vdev, char __user *buf, size_t count, loff_t *ppos) { - struct vfio_fsl_mc_device *vdev = device_data; + struct vfio_fsl_mc_device *vdev = + container_of(core_vdev, struct vfio_fsl_mc_device, vdev); unsigned int index = VFIO_FSL_MC_OFFSET_TO_INDEX(*ppos); loff_t off = *ppos & VFIO_FSL_MC_OFFSET_MASK; struct fsl_mc_device *mc_dev = vdev->mc_dev; @@ -404,10 +408,12 @@ static int vfio_fsl_mc_send_command(void __iomem *ioaddr, uint64_t *cmd_data) return 0; } -static ssize_t vfio_fsl_mc_write(void *device_data, const char __user *buf, - size_t count, loff_t *ppos) +static ssize_t vfio_fsl_mc_write(struct vfio_device *core_vdev, + const char __user *buf, size_t count, + loff_t *ppos) { - struct vfio_fsl_mc_device *vdev = device_data; + struct vfio_fsl_mc_device *vdev = + container_of(core_vdev, struct vfio_fsl_mc_device, vdev); unsigned int index = VFIO_FSL_MC_OFFSET_TO_INDEX(*ppos); loff_t off = *ppos & VFIO_FSL_MC_OFFSET_MASK; struct fsl_mc_device *mc_dev = vdev->mc_dev; @@ -468,9 +474,11 @@ static int vfio_fsl_mc_mmap_mmio(struct vfio_fsl_mc_region region, size, vma->vm_page_prot); } -static int vfio_fsl_mc_mmap(void *device_data, struct vm_area_struct *vma) +static int vfio_fsl_mc_mmap(struct vfio_device *core_vdev, + struct vm_area_struct *vma) { - struct vfio_fsl_mc_device *vdev = device_data; + struct vfio_fsl_mc_device *vdev = + container_of(core_vdev, struct vfio_fsl_mc_device, vdev); struct fsl_mc_device *mc_dev = vdev->mc_dev; unsigned int index; diff --git a/drivers/vfio/mdev/vfio_mdev.c b/drivers/vfio/mdev/vfio_mdev.c index 4043cc91f952..11b3e15403ba 100644 --- a/drivers/vfio/mdev/vfio_mdev.c +++ b/drivers/vfio/mdev/vfio_mdev.c @@ -21,10 +21,11 @@ #define DRIVER_AUTHOR "NVIDIA Corporation" #define DRIVER_DESC "VFIO based driver for Mediated device" -static int vfio_mdev_open(void *device_data) +static int vfio_mdev_open(struct vfio_device *core_vdev) { - struct mdev_device *mdev = device_data; + struct mdev_device *mdev = to_mdev_device(core_vdev->dev); struct mdev_parent *parent = mdev->parent; + int ret; if (unlikely(!parent->ops->open)) @@ -40,9 +41,9 @@ static int vfio_mdev_open(void *device_data) return ret; } -static void vfio_mdev_release(void *device_data) +static void vfio_mdev_release(struct vfio_device *core_vdev) { - struct mdev_device *mdev = device_data; + struct mdev_device *mdev = to_mdev_device(core_vdev->dev); struct mdev_parent *parent = mdev->parent; if (likely(parent->ops->release)) @@ -51,10 +52,10 @@ static void vfio_mdev_release(void *device_data) module_put(THIS_MODULE); } -static long vfio_mdev_unlocked_ioctl(void *device_data, +static long vfio_mdev_unlocked_ioctl(struct vfio_device *core_vdev, unsigned int cmd, unsigned long arg) { - struct mdev_device *mdev = device_data; + struct mdev_device *mdev = to_mdev_device(core_vdev->dev); struct mdev_parent *parent = mdev->parent; if (unlikely(!parent->ops->ioctl)) @@ -63,10 +64,10 @@ static long vfio_mdev_unlocked_ioctl(void *device_data, return parent->ops->ioctl(mdev, cmd, arg); } -static ssize_t vfio_mdev_read(void *device_data, char __user *buf, +static ssize_t vfio_mdev_read(struct vfio_device *core_vdev, char __user *buf, size_t count, loff_t *ppos) { - struct mdev_device *mdev = device_data; + struct mdev_device *mdev = to_mdev_device(core_vdev->dev); struct mdev_parent *parent = mdev->parent; if (unlikely(!parent->ops->read)) @@ -75,10 +76,11 @@ static ssize_t vfio_mdev_read(void *device_data, char __user *buf, return parent->ops->read(mdev, buf, count, ppos); } -static ssize_t vfio_mdev_write(void *device_data, const char __user *buf, - size_t count, loff_t *ppos) +static ssize_t vfio_mdev_write(struct vfio_device *core_vdev, + const char __user *buf, size_t count, + loff_t *ppos) { - struct mdev_device *mdev = device_data; + struct mdev_device *mdev = to_mdev_device(core_vdev->dev); struct mdev_parent *parent = mdev->parent; if (unlikely(!parent->ops->write)) @@ -87,9 +89,10 @@ static ssize_t vfio_mdev_write(void *device_data, const char __user *buf, return parent->ops->write(mdev, buf, count, ppos); } -static int vfio_mdev_mmap(void *device_data, struct vm_area_struct *vma) +static int vfio_mdev_mmap(struct vfio_device *core_vdev, + struct vm_area_struct *vma) { - struct mdev_device *mdev = device_data; + struct mdev_device *mdev = to_mdev_device(core_vdev->dev); struct mdev_parent *parent = mdev->parent; if (unlikely(!parent->ops->mmap)) @@ -98,9 +101,9 @@ static int vfio_mdev_mmap(void *device_data, struct vm_area_struct *vma) return parent->ops->mmap(mdev, vma); } -static void vfio_mdev_request(void *device_data, unsigned int count) +static void vfio_mdev_request(struct vfio_device *core_vdev, unsigned int count) { - struct mdev_device *mdev = device_data; + struct mdev_device *mdev = to_mdev_device(core_vdev->dev); struct mdev_parent *parent = mdev->parent; if (parent->ops->request) diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c index a0ac20a499cf..5f1a782d1c65 100644 --- a/drivers/vfio/pci/vfio_pci.c +++ b/drivers/vfio/pci/vfio_pci.c @@ -553,9 +553,10 @@ static void vfio_pci_vf_token_user_add(struct vfio_pci_device *vdev, int val) vfio_device_put(pf_dev); } -static void vfio_pci_release(void *device_data) +static void vfio_pci_release(struct vfio_device *core_vdev) { - struct vfio_pci_device *vdev = device_data; + struct vfio_pci_device *vdev = + container_of(core_vdev, struct vfio_pci_device, vdev); mutex_lock(&vdev->reflck->lock); @@ -581,9 +582,10 @@ static void vfio_pci_release(void *device_data) module_put(THIS_MODULE); } -static int vfio_pci_open(void *device_data) +static int vfio_pci_open(struct vfio_device *core_vdev) { - struct vfio_pci_device *vdev = device_data; + struct vfio_pci_device *vdev = + container_of(core_vdev, struct vfio_pci_device, vdev); int ret = 0; if (!try_module_get(THIS_MODULE)) @@ -797,10 +799,11 @@ struct vfio_devices { int max_index; }; -static long vfio_pci_ioctl(void *device_data, +static long vfio_pci_ioctl(struct vfio_device *core_vdev, unsigned int cmd, unsigned long arg) { - struct vfio_pci_device *vdev = device_data; + struct vfio_pci_device *vdev = + container_of(core_vdev, struct vfio_pci_device, vdev); unsigned long minsz; if (cmd == VFIO_DEVICE_GET_INFO) { @@ -1402,11 +1405,10 @@ hot_reset_release: return -ENOTTY; } -static ssize_t vfio_pci_rw(void *device_data, char __user *buf, +static ssize_t vfio_pci_rw(struct vfio_pci_device *vdev, char __user *buf, size_t count, loff_t *ppos, bool iswrite) { unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos); - struct vfio_pci_device *vdev = device_data; if (index >= VFIO_PCI_NUM_REGIONS + vdev->num_regions) return -EINVAL; @@ -1434,22 +1436,28 @@ static ssize_t vfio_pci_rw(void *device_data, char __user *buf, return -EINVAL; } -static ssize_t vfio_pci_read(void *device_data, char __user *buf, +static ssize_t vfio_pci_read(struct vfio_device *core_vdev, char __user *buf, size_t count, loff_t *ppos) { + struct vfio_pci_device *vdev = + container_of(core_vdev, struct vfio_pci_device, vdev); + if (!count) return 0; - return vfio_pci_rw(device_data, buf, count, ppos, false); + return vfio_pci_rw(vdev, buf, count, ppos, false); } -static ssize_t vfio_pci_write(void *device_data, const char __user *buf, +static ssize_t vfio_pci_write(struct vfio_device *core_vdev, const char __user *buf, size_t count, loff_t *ppos) { + struct vfio_pci_device *vdev = + container_of(core_vdev, struct vfio_pci_device, vdev); + if (!count) return 0; - return vfio_pci_rw(device_data, (char __user *)buf, count, ppos, true); + return vfio_pci_rw(vdev, (char __user *)buf, count, ppos, true); } /* Return 1 on zap and vma_lock acquired, 0 on contention (only with @try) */ @@ -1646,9 +1654,10 @@ static const struct vm_operations_struct vfio_pci_mmap_ops = { .fault = vfio_pci_mmap_fault, }; -static int vfio_pci_mmap(void *device_data, struct vm_area_struct *vma) +static int vfio_pci_mmap(struct vfio_device *core_vdev, struct vm_area_struct *vma) { - struct vfio_pci_device *vdev = device_data; + struct vfio_pci_device *vdev = + container_of(core_vdev, struct vfio_pci_device, vdev); struct pci_dev *pdev = vdev->pdev; unsigned int index; u64 phys_len, req_len, pgoff, req_start; @@ -1714,9 +1723,10 @@ static int vfio_pci_mmap(void *device_data, struct vm_area_struct *vma) return 0; } -static void vfio_pci_request(void *device_data, unsigned int count) +static void vfio_pci_request(struct vfio_device *core_vdev, unsigned int count) { - struct vfio_pci_device *vdev = device_data; + struct vfio_pci_device *vdev = + container_of(core_vdev, struct vfio_pci_device, vdev); struct pci_dev *pdev = vdev->pdev; mutex_lock(&vdev->igate); @@ -1830,9 +1840,10 @@ static int vfio_pci_validate_vf_token(struct vfio_pci_device *vdev, #define VF_TOKEN_ARG "vf_token=" -static int vfio_pci_match(void *device_data, char *buf) +static int vfio_pci_match(struct vfio_device *core_vdev, char *buf) { - struct vfio_pci_device *vdev = device_data; + struct vfio_pci_device *vdev = + container_of(core_vdev, struct vfio_pci_device, vdev); bool vf_token = false; uuid_t uuid; int ret; diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c index 6eb749250ee4..f5f6b537084a 100644 --- a/drivers/vfio/platform/vfio_platform_common.c +++ b/drivers/vfio/platform/vfio_platform_common.c @@ -218,9 +218,10 @@ static int vfio_platform_call_reset(struct vfio_platform_device *vdev, return -EINVAL; } -static void vfio_platform_release(void *device_data) +static void vfio_platform_release(struct vfio_device *core_vdev) { - struct vfio_platform_device *vdev = device_data; + struct vfio_platform_device *vdev = + container_of(core_vdev, struct vfio_platform_device, vdev); mutex_lock(&driver_lock); @@ -244,9 +245,10 @@ static void vfio_platform_release(void *device_data) module_put(vdev->parent_module); } -static int vfio_platform_open(void *device_data) +static int vfio_platform_open(struct vfio_device *core_vdev) { - struct vfio_platform_device *vdev = device_data; + struct vfio_platform_device *vdev = + container_of(core_vdev, struct vfio_platform_device, vdev); int ret; if (!try_module_get(vdev->parent_module)) @@ -293,10 +295,12 @@ err_reg: return ret; } -static long vfio_platform_ioctl(void *device_data, +static long vfio_platform_ioctl(struct vfio_device *core_vdev, unsigned int cmd, unsigned long arg) { - struct vfio_platform_device *vdev = device_data; + struct vfio_platform_device *vdev = + container_of(core_vdev, struct vfio_platform_device, vdev); + unsigned long minsz; if (cmd == VFIO_DEVICE_GET_INFO) { @@ -455,10 +459,11 @@ err: return -EFAULT; } -static ssize_t vfio_platform_read(void *device_data, char __user *buf, - size_t count, loff_t *ppos) +static ssize_t vfio_platform_read(struct vfio_device *core_vdev, + char __user *buf, size_t count, loff_t *ppos) { - struct vfio_platform_device *vdev = device_data; + struct vfio_platform_device *vdev = + container_of(core_vdev, struct vfio_platform_device, vdev); unsigned int index = VFIO_PLATFORM_OFFSET_TO_INDEX(*ppos); loff_t off = *ppos & VFIO_PLATFORM_OFFSET_MASK; @@ -531,10 +536,11 @@ err: return -EFAULT; } -static ssize_t vfio_platform_write(void *device_data, const char __user *buf, +static ssize_t vfio_platform_write(struct vfio_device *core_vdev, const char __user *buf, size_t count, loff_t *ppos) { - struct vfio_platform_device *vdev = device_data; + struct vfio_platform_device *vdev = + container_of(core_vdev, struct vfio_platform_device, vdev); unsigned int index = VFIO_PLATFORM_OFFSET_TO_INDEX(*ppos); loff_t off = *ppos & VFIO_PLATFORM_OFFSET_MASK; @@ -573,9 +579,10 @@ static int vfio_platform_mmap_mmio(struct vfio_platform_region region, req_len, vma->vm_page_prot); } -static int vfio_platform_mmap(void *device_data, struct vm_area_struct *vma) +static int vfio_platform_mmap(struct vfio_device *core_vdev, struct vm_area_struct *vma) { - struct vfio_platform_device *vdev = device_data; + struct vfio_platform_device *vdev = + container_of(core_vdev, struct vfio_platform_device, vdev); unsigned int index; index = vma->vm_pgoff >> (VFIO_PLATFORM_OFFSET_SHIFT - PAGE_SHIFT); diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c index 180b4ab02d11..e6f5109fba48 100644 --- a/drivers/vfio/vfio.c +++ b/drivers/vfio/vfio.c @@ -832,7 +832,7 @@ static struct vfio_device *vfio_device_get_from_name(struct vfio_group *group, int ret; if (it->ops->match) { - ret = it->ops->match(it->device_data, buf); + ret = it->ops->match(it, buf); if (ret < 0) { device = ERR_PTR(ret); break; @@ -893,7 +893,7 @@ void vfio_unregister_group_dev(struct vfio_device *device) rc = try_wait_for_completion(&device->comp); while (rc <= 0) { if (device->ops->request) - device->ops->request(device->device_data, i++); + device->ops->request(device, i++); if (interrupted) { rc = wait_for_completion_timeout(&device->comp, @@ -1379,7 +1379,7 @@ static int vfio_group_get_device_fd(struct vfio_group *group, char *buf) if (IS_ERR(device)) return PTR_ERR(device); - ret = device->ops->open(device->device_data); + ret = device->ops->open(device); if (ret) { vfio_device_put(device); return ret; @@ -1391,7 +1391,7 @@ static int vfio_group_get_device_fd(struct vfio_group *group, char *buf) */ ret = get_unused_fd_flags(O_CLOEXEC); if (ret < 0) { - device->ops->release(device->device_data); + device->ops->release(device); vfio_device_put(device); return ret; } @@ -1401,7 +1401,7 @@ static int vfio_group_get_device_fd(struct vfio_group *group, char *buf) if (IS_ERR(filep)) { put_unused_fd(ret); ret = PTR_ERR(filep); - device->ops->release(device->device_data); + device->ops->release(device); vfio_device_put(device); return ret; } @@ -1558,7 +1558,7 @@ static int vfio_device_fops_release(struct inode *inode, struct file *filep) { struct vfio_device *device = filep->private_data; - device->ops->release(device->device_data); + device->ops->release(device); vfio_group_try_dissolve_container(device->group); @@ -1575,7 +1575,7 @@ static long vfio_device_fops_unl_ioctl(struct file *filep, if (unlikely(!device->ops->ioctl)) return -EINVAL; - return device->ops->ioctl(device->device_data, cmd, arg); + return device->ops->ioctl(device, cmd, arg); } static ssize_t vfio_device_fops_read(struct file *filep, char __user *buf, @@ -1586,7 +1586,7 @@ static ssize_t vfio_device_fops_read(struct file *filep, char __user *buf, if (unlikely(!device->ops->read)) return -EINVAL; - return device->ops->read(device->device_data, buf, count, ppos); + return device->ops->read(device, buf, count, ppos); } static ssize_t vfio_device_fops_write(struct file *filep, @@ -1598,7 +1598,7 @@ static ssize_t vfio_device_fops_write(struct file *filep, if (unlikely(!device->ops->write)) return -EINVAL; - return device->ops->write(device->device_data, buf, count, ppos); + return device->ops->write(device, buf, count, ppos); } static int vfio_device_fops_mmap(struct file *filep, struct vm_area_struct *vma) @@ -1608,7 +1608,7 @@ static int vfio_device_fops_mmap(struct file *filep, struct vm_area_struct *vma) if (unlikely(!device->ops->mmap)) return -EINVAL; - return device->ops->mmap(device->device_data, vma); + return device->ops->mmap(device, vma); } static const struct file_operations vfio_device_fops = { diff --git a/include/linux/vfio.h b/include/linux/vfio.h index 4995faf51efe..784c34c0a287 100644 --- a/include/linux/vfio.h +++ b/include/linux/vfio.h @@ -44,17 +44,17 @@ struct vfio_device { */ struct vfio_device_ops { char *name; - int (*open)(void *device_data); - void (*release)(void *device_data); - ssize_t (*read)(void *device_data, char __user *buf, + int (*open)(struct vfio_device *vdev); + void (*release)(struct vfio_device *vdev); + ssize_t (*read)(struct vfio_device *vdev, char __user *buf, size_t count, loff_t *ppos); - ssize_t (*write)(void *device_data, const char __user *buf, + ssize_t (*write)(struct vfio_device *vdev, const char __user *buf, size_t count, loff_t *size); - long (*ioctl)(void *device_data, unsigned int cmd, + long (*ioctl)(struct vfio_device *vdev, unsigned int cmd, unsigned long arg); - int (*mmap)(void *device_data, struct vm_area_struct *vma); - void (*request)(void *device_data, unsigned int count); - int (*match)(void *device_data, char *buf); + int (*mmap)(struct vfio_device *vdev, struct vm_area_struct *vma); + void (*request)(struct vfio_device *vdev, unsigned int count); + int (*match)(struct vfio_device *vdev, char *buf); }; extern struct iommu_group *vfio_iommu_group_get(struct device *dev); -- cgit v1.2.3 From 07d47b4222d5d1cd933f01587dda00398d8daf40 Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Tue, 30 Mar 2021 09:53:08 -0600 Subject: vfio/pci: Replace uses of vfio_device_data() with container_of This tidies a few confused places that think they can have a refcount on the vfio_device but the device_data could be NULL, that isn't possible by design. Most of the change falls out when struct vfio_devices is updated to just store the struct vfio_pci_device itself. This wasn't possible before because there was no easy way to get from the 'struct vfio_pci_device' to the 'struct vfio_device' to put back the refcount. Reviewed-by: Christoph Hellwig Reviewed-by: Kevin Tian Reviewed-by: Cornelia Huck Signed-off-by: Jason Gunthorpe Message-Id: <13-v3-225de1400dfc+4e074-vfio1_jgg@nvidia.com> Signed-off-by: Alex Williamson --- drivers/vfio/pci/vfio_pci.c | 67 ++++++++++++++++----------------------------- 1 file changed, 24 insertions(+), 43 deletions(-) (limited to 'drivers') diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c index 5f1a782d1c65..1f70387c8afe 100644 --- a/drivers/vfio/pci/vfio_pci.c +++ b/drivers/vfio/pci/vfio_pci.c @@ -517,30 +517,29 @@ out: static struct pci_driver vfio_pci_driver; -static struct vfio_pci_device *get_pf_vdev(struct vfio_pci_device *vdev, - struct vfio_device **pf_dev) +static struct vfio_pci_device *get_pf_vdev(struct vfio_pci_device *vdev) { struct pci_dev *physfn = pci_physfn(vdev->pdev); + struct vfio_device *pf_dev; if (!vdev->pdev->is_virtfn) return NULL; - *pf_dev = vfio_device_get_from_dev(&physfn->dev); - if (!*pf_dev) + pf_dev = vfio_device_get_from_dev(&physfn->dev); + if (!pf_dev) return NULL; if (pci_dev_driver(physfn) != &vfio_pci_driver) { - vfio_device_put(*pf_dev); + vfio_device_put(pf_dev); return NULL; } - return vfio_device_data(*pf_dev); + return container_of(pf_dev, struct vfio_pci_device, vdev); } static void vfio_pci_vf_token_user_add(struct vfio_pci_device *vdev, int val) { - struct vfio_device *pf_dev; - struct vfio_pci_device *pf_vdev = get_pf_vdev(vdev, &pf_dev); + struct vfio_pci_device *pf_vdev = get_pf_vdev(vdev); if (!pf_vdev) return; @@ -550,7 +549,7 @@ static void vfio_pci_vf_token_user_add(struct vfio_pci_device *vdev, int val) WARN_ON(pf_vdev->vf_token->users < 0); mutex_unlock(&pf_vdev->vf_token->lock); - vfio_device_put(pf_dev); + vfio_device_put(&pf_vdev->vdev); } static void vfio_pci_release(struct vfio_device *core_vdev) @@ -794,7 +793,7 @@ int vfio_pci_register_dev_region(struct vfio_pci_device *vdev, } struct vfio_devices { - struct vfio_device **devices; + struct vfio_pci_device **devices; int cur_index; int max_index; }; @@ -1283,9 +1282,7 @@ reset_info_exit: goto hot_reset_release; for (; mem_idx < devs.cur_index; mem_idx++) { - struct vfio_pci_device *tmp; - - tmp = vfio_device_data(devs.devices[mem_idx]); + struct vfio_pci_device *tmp = devs.devices[mem_idx]; ret = down_write_trylock(&tmp->memory_lock); if (!ret) { @@ -1300,17 +1297,13 @@ reset_info_exit: hot_reset_release: for (i = 0; i < devs.cur_index; i++) { - struct vfio_device *device; - struct vfio_pci_device *tmp; - - device = devs.devices[i]; - tmp = vfio_device_data(device); + struct vfio_pci_device *tmp = devs.devices[i]; if (i < mem_idx) up_write(&tmp->memory_lock); else mutex_unlock(&tmp->vma_lock); - vfio_device_put(device); + vfio_device_put(&tmp->vdev); } kfree(devs.devices); @@ -1777,8 +1770,7 @@ static int vfio_pci_validate_vf_token(struct vfio_pci_device *vdev, return 0; /* No VF token provided or required */ if (vdev->pdev->is_virtfn) { - struct vfio_device *pf_dev; - struct vfio_pci_device *pf_vdev = get_pf_vdev(vdev, &pf_dev); + struct vfio_pci_device *pf_vdev = get_pf_vdev(vdev); bool match; if (!pf_vdev) { @@ -1791,7 +1783,7 @@ static int vfio_pci_validate_vf_token(struct vfio_pci_device *vdev, } if (!vf_token) { - vfio_device_put(pf_dev); + vfio_device_put(&pf_vdev->vdev); pci_info_ratelimited(vdev->pdev, "VF token required to access device\n"); return -EACCES; @@ -1801,7 +1793,7 @@ static int vfio_pci_validate_vf_token(struct vfio_pci_device *vdev, match = uuid_equal(uuid, &pf_vdev->vf_token->uuid); mutex_unlock(&pf_vdev->vf_token->lock); - vfio_device_put(pf_dev); + vfio_device_put(&pf_vdev->vdev); if (!match) { pci_info_ratelimited(vdev->pdev, @@ -2122,11 +2114,7 @@ static pci_ers_result_t vfio_pci_aer_err_detected(struct pci_dev *pdev, if (device == NULL) return PCI_ERS_RESULT_DISCONNECT; - vdev = vfio_device_data(device); - if (vdev == NULL) { - vfio_device_put(device); - return PCI_ERS_RESULT_DISCONNECT; - } + vdev = container_of(device, struct vfio_pci_device, vdev); mutex_lock(&vdev->igate); @@ -2142,7 +2130,6 @@ static pci_ers_result_t vfio_pci_aer_err_detected(struct pci_dev *pdev, static int vfio_pci_sriov_configure(struct pci_dev *pdev, int nr_virtfn) { - struct vfio_pci_device *vdev; struct vfio_device *device; int ret = 0; @@ -2155,12 +2142,6 @@ static int vfio_pci_sriov_configure(struct pci_dev *pdev, int nr_virtfn) if (!device) return -ENODEV; - vdev = vfio_device_data(device); - if (!vdev) { - vfio_device_put(device); - return -ENODEV; - } - if (nr_virtfn == 0) pci_disable_sriov(pdev); else @@ -2220,7 +2201,7 @@ static int vfio_pci_reflck_find(struct pci_dev *pdev, void *data) return 0; } - vdev = vfio_device_data(device); + vdev = container_of(device, struct vfio_pci_device, vdev); if (vdev->reflck) { vfio_pci_reflck_get(vdev->reflck); @@ -2282,7 +2263,7 @@ static int vfio_pci_get_unused_devs(struct pci_dev *pdev, void *data) return -EBUSY; } - vdev = vfio_device_data(device); + vdev = container_of(device, struct vfio_pci_device, vdev); /* Fault if the device is not unused */ if (vdev->refcnt) { @@ -2290,7 +2271,7 @@ static int vfio_pci_get_unused_devs(struct pci_dev *pdev, void *data) return -EBUSY; } - devs->devices[devs->cur_index++] = device; + devs->devices[devs->cur_index++] = vdev; return 0; } @@ -2312,7 +2293,7 @@ static int vfio_pci_try_zap_and_vma_lock_cb(struct pci_dev *pdev, void *data) return -EBUSY; } - vdev = vfio_device_data(device); + vdev = container_of(device, struct vfio_pci_device, vdev); /* * Locking multiple devices is prone to deadlock, runaway and @@ -2323,7 +2304,7 @@ static int vfio_pci_try_zap_and_vma_lock_cb(struct pci_dev *pdev, void *data) return -EBUSY; } - devs->devices[devs->cur_index++] = device; + devs->devices[devs->cur_index++] = vdev; return 0; } @@ -2371,7 +2352,7 @@ static void vfio_pci_try_bus_reset(struct vfio_pci_device *vdev) /* Does at least one need a reset? */ for (i = 0; i < devs.cur_index; i++) { - tmp = vfio_device_data(devs.devices[i]); + tmp = devs.devices[i]; if (tmp->needs_reset) { ret = pci_reset_bus(vdev->pdev); break; @@ -2380,7 +2361,7 @@ static void vfio_pci_try_bus_reset(struct vfio_pci_device *vdev) put_devs: for (i = 0; i < devs.cur_index; i++) { - tmp = vfio_device_data(devs.devices[i]); + tmp = devs.devices[i]; /* * If reset was successful, affected devices no longer need @@ -2396,7 +2377,7 @@ put_devs: vfio_pci_set_power_state(tmp, PCI_D3hot); } - vfio_device_put(devs.devices[i]); + vfio_device_put(&tmp->vdev); } kfree(devs.devices); -- cgit v1.2.3 From 1e04ec14204dec28131855d8dd160c3d55d12797 Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Tue, 30 Mar 2021 09:53:08 -0600 Subject: vfio: Remove device_data from the vfio bus driver API There are no longer any users, so it can go away. Everything is using container_of now. Reviewed-by: Christoph Hellwig Reviewed-by: Kevin Tian Reviewed-by: Cornelia Huck Reviewed-by: Max Gurtovoy Signed-off-by: Jason Gunthorpe Message-Id: <14-v3-225de1400dfc+4e074-vfio1_jgg@nvidia.com> Signed-off-by: Alex Williamson --- Documentation/driver-api/vfio.rst | 3 +-- drivers/vfio/fsl-mc/vfio_fsl_mc.c | 5 +++-- drivers/vfio/mdev/vfio_mdev.c | 2 +- drivers/vfio/pci/vfio_pci.c | 2 +- drivers/vfio/platform/vfio_platform_common.c | 2 +- drivers/vfio/vfio.c | 12 +----------- include/linux/vfio.h | 4 +--- 7 files changed, 9 insertions(+), 21 deletions(-) (limited to 'drivers') diff --git a/Documentation/driver-api/vfio.rst b/Documentation/driver-api/vfio.rst index 3337f337293a..decc68cb8114 100644 --- a/Documentation/driver-api/vfio.rst +++ b/Documentation/driver-api/vfio.rst @@ -254,8 +254,7 @@ vfio_unregister_group_dev() respectively:: void vfio_init_group_dev(struct vfio_device *device, struct device *dev, - const struct vfio_device_ops *ops, - void *device_data); + const struct vfio_device_ops *ops); int vfio_register_group_dev(struct vfio_device *device); void vfio_unregister_group_dev(struct vfio_device *device); diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c index 45f397c04a89..980e59551301 100644 --- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c @@ -75,7 +75,8 @@ static int vfio_fsl_mc_reflck_attach(struct vfio_fsl_mc_device *vdev) goto unlock; } - cont_vdev = vfio_device_data(device); + cont_vdev = + container_of(device, struct vfio_fsl_mc_device, vdev); if (!cont_vdev || !cont_vdev->reflck) { vfio_device_put(device); ret = -ENODEV; @@ -630,7 +631,7 @@ static int vfio_fsl_mc_probe(struct fsl_mc_device *mc_dev) goto out_group_put; } - vfio_init_group_dev(&vdev->vdev, dev, &vfio_fsl_mc_ops, vdev); + vfio_init_group_dev(&vdev->vdev, dev, &vfio_fsl_mc_ops); vdev->mc_dev = mc_dev; mutex_init(&vdev->igate); diff --git a/drivers/vfio/mdev/vfio_mdev.c b/drivers/vfio/mdev/vfio_mdev.c index 11b3e15403ba..ae7e322fbe3c 100644 --- a/drivers/vfio/mdev/vfio_mdev.c +++ b/drivers/vfio/mdev/vfio_mdev.c @@ -134,7 +134,7 @@ static int vfio_mdev_probe(struct device *dev) if (!vdev) return -ENOMEM; - vfio_init_group_dev(vdev, &mdev->dev, &vfio_mdev_dev_ops, mdev); + vfio_init_group_dev(vdev, &mdev->dev, &vfio_mdev_dev_ops); ret = vfio_register_group_dev(vdev); if (ret) { kfree(vdev); diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c index 1f70387c8afe..55ef27a15d4d 100644 --- a/drivers/vfio/pci/vfio_pci.c +++ b/drivers/vfio/pci/vfio_pci.c @@ -2022,7 +2022,7 @@ static int vfio_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) goto out_group_put; } - vfio_init_group_dev(&vdev->vdev, &pdev->dev, &vfio_pci_ops, vdev); + vfio_init_group_dev(&vdev->vdev, &pdev->dev, &vfio_pci_ops); vdev->pdev = pdev; vdev->irq_type = VFIO_PCI_NUM_IRQS; mutex_init(&vdev->igate); diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c index f5f6b537084a..361e5b57e369 100644 --- a/drivers/vfio/platform/vfio_platform_common.c +++ b/drivers/vfio/platform/vfio_platform_common.c @@ -666,7 +666,7 @@ int vfio_platform_probe_common(struct vfio_platform_device *vdev, struct iommu_group *group; int ret; - vfio_init_group_dev(&vdev->vdev, dev, &vfio_platform_ops, vdev); + vfio_init_group_dev(&vdev->vdev, dev, &vfio_platform_ops); ret = vfio_platform_acpi_probe(vdev, dev); if (ret) diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c index e6f5109fba48..5e631c359ef2 100644 --- a/drivers/vfio/vfio.c +++ b/drivers/vfio/vfio.c @@ -741,12 +741,11 @@ static int vfio_iommu_group_notifier(struct notifier_block *nb, * VFIO driver API */ void vfio_init_group_dev(struct vfio_device *device, struct device *dev, - const struct vfio_device_ops *ops, void *device_data) + const struct vfio_device_ops *ops) { init_completion(&device->comp); device->dev = dev; device->ops = ops; - device->device_data = device_data; } EXPORT_SYMBOL_GPL(vfio_init_group_dev); @@ -851,15 +850,6 @@ static struct vfio_device *vfio_device_get_from_name(struct vfio_group *group, return device; } -/* - * Caller must hold a reference to the vfio_device - */ -void *vfio_device_data(struct vfio_device *device) -{ - return device->device_data; -} -EXPORT_SYMBOL_GPL(vfio_device_data); - /* * Decrement the device reference count and wait for the device to be * removed. Open file descriptors for the device... */ diff --git a/include/linux/vfio.h b/include/linux/vfio.h index 784c34c0a287..a2c5b30e1763 100644 --- a/include/linux/vfio.h +++ b/include/linux/vfio.h @@ -24,7 +24,6 @@ struct vfio_device { refcount_t refcount; struct completion comp; struct list_head group_next; - void *device_data; }; /** @@ -61,12 +60,11 @@ extern struct iommu_group *vfio_iommu_group_get(struct device *dev); extern void vfio_iommu_group_put(struct iommu_group *group, struct device *dev); void vfio_init_group_dev(struct vfio_device *device, struct device *dev, - const struct vfio_device_ops *ops, void *device_data); + const struct vfio_device_ops *ops); int vfio_register_group_dev(struct vfio_device *device); void vfio_unregister_group_dev(struct vfio_device *device); extern struct vfio_device *vfio_device_get_from_dev(struct device *dev); extern void vfio_device_put(struct vfio_device *device); -extern void *vfio_device_data(struct vfio_device *device); /* events for the backend driver notify callback */ enum vfio_iommu_notify_type { -- cgit v1.2.3 From b5a1f8921d5040bb788492bf33a66758021e4be5 Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Tue, 6 Apr 2021 16:40:25 -0300 Subject: vfio/mdev: Do not allow a mdev_type to have a NULL parent pointer There is a small race where the parent is NULL even though the kobj has already been made visible in sysfs. For instance the attribute_group is made visible in sysfs_create_files() and the mdev_type_attr_show() does: ret = attr->show(kobj, type->parent->dev, buf); Which will crash on NULL parent. Move the parent setup to before the type pointer leaves the stack frame. Fixes: 7b96953bc640 ("vfio: Mediated device Core driver") Reviewed-by: Christoph Hellwig Reviewed-by: Kevin Tian Reviewed-by: Max Gurtovoy Reviewed-by: Cornelia Huck Signed-off-by: Jason Gunthorpe Message-Id: <2-v2-d36939638fc6+d54-vfio2_jgg@nvidia.com> Signed-off-by: Alex Williamson --- drivers/vfio/mdev/mdev_sysfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/vfio/mdev/mdev_sysfs.c b/drivers/vfio/mdev/mdev_sysfs.c index 917fd84c1c6f..367ff5412a38 100644 --- a/drivers/vfio/mdev/mdev_sysfs.c +++ b/drivers/vfio/mdev/mdev_sysfs.c @@ -105,6 +105,7 @@ static struct mdev_type *add_mdev_supported_type(struct mdev_parent *parent, return ERR_PTR(-ENOMEM); type->kobj.kset = parent->mdev_types_kset; + type->parent = parent; ret = kobject_init_and_add(&type->kobj, &mdev_type_ktype, NULL, "%s-%s", dev_driver_string(parent->dev), @@ -132,7 +133,6 @@ static struct mdev_type *add_mdev_supported_type(struct mdev_parent *parent, } type->group = group; - type->parent = parent; return type; attrs_failed: -- cgit v1.2.3 From 2a3d15f270efa50d78d8a32d895e9d5396668f3a Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Tue, 6 Apr 2021 16:40:26 -0300 Subject: vfio/mdev: Add missing typesafety around mdev_device The mdev API should accept and pass a 'struct mdev_device *' in all places, not pass a 'struct device *' and cast it internally with to_mdev_device(). Particularly in its struct mdev_driver functions, the whole point of a bus's struct device_driver wrapper is to provide type safety compared to the default struct device_driver. Further, the driver core standard is for bus drivers to expose their device structure in their public headers that can be used with container_of() inlines and '&foo->dev' to go between the class levels, and '&foo->dev' to be used with dev_err/etc driver core helper functions. Move 'struct mdev_device' to mdev.h Once done this allows moving some one instruction exported functions to static inlines, which in turns allows removing one of the two grotesque symbol_get()'s related to mdev in the core code. Reviewed-by: Kevin Tian Reviewed-by: Cornelia Huck Signed-off-by: Jason Gunthorpe Reviewed-by: Christoph Hellwig Message-Id: <3-v2-d36939638fc6+d54-vfio2_jgg@nvidia.com> Signed-off-by: Alex Williamson --- Documentation/driver-api/vfio-mediated-device.rst | 4 +- drivers/vfio/mdev/mdev_core.c | 64 +++-------------------- drivers/vfio/mdev/mdev_driver.c | 4 +- drivers/vfio/mdev/mdev_private.h | 23 ++------ drivers/vfio/mdev/mdev_sysfs.c | 26 +++++---- drivers/vfio/mdev/vfio_mdev.c | 7 ++- drivers/vfio/vfio_iommu_type1.c | 25 +++------ include/linux/mdev.h | 58 +++++++++++++++----- 8 files changed, 83 insertions(+), 128 deletions(-) (limited to 'drivers') diff --git a/Documentation/driver-api/vfio-mediated-device.rst b/Documentation/driver-api/vfio-mediated-device.rst index 25eb7d5b834b..c43c1dc33333 100644 --- a/Documentation/driver-api/vfio-mediated-device.rst +++ b/Documentation/driver-api/vfio-mediated-device.rst @@ -105,8 +105,8 @@ structure to represent a mediated device's driver:: */ struct mdev_driver { const char *name; - int (*probe) (struct device *dev); - void (*remove) (struct device *dev); + int (*probe) (struct mdev_device *dev); + void (*remove) (struct mdev_device *dev); struct device_driver driver; }; diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c index 6de97d25a3f8..057922a1707e 100644 --- a/drivers/vfio/mdev/mdev_core.c +++ b/drivers/vfio/mdev/mdev_core.c @@ -33,36 +33,6 @@ struct device *mdev_parent_dev(struct mdev_device *mdev) } EXPORT_SYMBOL(mdev_parent_dev); -void *mdev_get_drvdata(struct mdev_device *mdev) -{ - return mdev->driver_data; -} -EXPORT_SYMBOL(mdev_get_drvdata); - -void mdev_set_drvdata(struct mdev_device *mdev, void *data) -{ - mdev->driver_data = data; -} -EXPORT_SYMBOL(mdev_set_drvdata); - -struct device *mdev_dev(struct mdev_device *mdev) -{ - return &mdev->dev; -} -EXPORT_SYMBOL(mdev_dev); - -struct mdev_device *mdev_from_dev(struct device *dev) -{ - return dev_is_mdev(dev) ? to_mdev_device(dev) : NULL; -} -EXPORT_SYMBOL(mdev_from_dev); - -const guid_t *mdev_uuid(struct mdev_device *mdev) -{ - return &mdev->uuid; -} -EXPORT_SYMBOL(mdev_uuid); - /* Should be called holding parent_list_lock */ static struct mdev_parent *__find_parent_device(struct device *dev) { @@ -107,7 +77,7 @@ static void mdev_device_remove_common(struct mdev_device *mdev) int ret; type = to_mdev_type(mdev->type_kobj); - mdev_remove_sysfs_files(&mdev->dev, type); + mdev_remove_sysfs_files(mdev, type); device_del(&mdev->dev); parent = mdev->parent; lockdep_assert_held(&parent->unreg_sem); @@ -122,12 +92,10 @@ static void mdev_device_remove_common(struct mdev_device *mdev) static int mdev_device_remove_cb(struct device *dev, void *data) { - if (dev_is_mdev(dev)) { - struct mdev_device *mdev; + struct mdev_device *mdev = mdev_from_dev(dev); - mdev = to_mdev_device(dev); + if (mdev) mdev_device_remove_common(mdev); - } return 0; } @@ -332,7 +300,7 @@ int mdev_device_create(struct kobject *kobj, if (ret) goto add_fail; - ret = mdev_create_sysfs_files(&mdev->dev, type); + ret = mdev_create_sysfs_files(mdev, type); if (ret) goto sysfs_fail; @@ -354,13 +322,11 @@ mdev_fail: return ret; } -int mdev_device_remove(struct device *dev) +int mdev_device_remove(struct mdev_device *mdev) { - struct mdev_device *mdev, *tmp; + struct mdev_device *tmp; struct mdev_parent *parent; - mdev = to_mdev_device(dev); - mutex_lock(&mdev_list_lock); list_for_each_entry(tmp, &mdev_list, next) { if (tmp == mdev) @@ -390,24 +356,6 @@ int mdev_device_remove(struct device *dev) return 0; } -int mdev_set_iommu_device(struct device *dev, struct device *iommu_device) -{ - struct mdev_device *mdev = to_mdev_device(dev); - - mdev->iommu_device = iommu_device; - - return 0; -} -EXPORT_SYMBOL(mdev_set_iommu_device); - -struct device *mdev_get_iommu_device(struct device *dev) -{ - struct mdev_device *mdev = to_mdev_device(dev); - - return mdev->iommu_device; -} -EXPORT_SYMBOL(mdev_get_iommu_device); - static int __init mdev_init(void) { return mdev_bus_register(); diff --git a/drivers/vfio/mdev/mdev_driver.c b/drivers/vfio/mdev/mdev_driver.c index 0d3223aee20b..44c3ba7e56d9 100644 --- a/drivers/vfio/mdev/mdev_driver.c +++ b/drivers/vfio/mdev/mdev_driver.c @@ -48,7 +48,7 @@ static int mdev_probe(struct device *dev) return ret; if (drv && drv->probe) { - ret = drv->probe(dev); + ret = drv->probe(mdev); if (ret) mdev_detach_iommu(mdev); } @@ -62,7 +62,7 @@ static int mdev_remove(struct device *dev) struct mdev_device *mdev = to_mdev_device(dev); if (drv && drv->remove) - drv->remove(dev); + drv->remove(mdev); mdev_detach_iommu(mdev); diff --git a/drivers/vfio/mdev/mdev_private.h b/drivers/vfio/mdev/mdev_private.h index fdbb3bee99a9..97e2225f7f49 100644 --- a/drivers/vfio/mdev/mdev_private.h +++ b/drivers/vfio/mdev/mdev_private.h @@ -24,23 +24,6 @@ struct mdev_parent { struct rw_semaphore unreg_sem; }; -struct mdev_device { - struct device dev; - struct mdev_parent *parent; - guid_t uuid; - void *driver_data; - struct list_head next; - struct kobject *type_kobj; - struct device *iommu_device; - bool active; -}; - -static inline struct mdev_device *to_mdev_device(struct device *dev) -{ - return container_of(dev, struct mdev_device, dev); -} -#define dev_is_mdev(d) ((d)->bus == &mdev_bus_type) - struct mdev_type { struct kobject kobj; struct kobject *devices_kobj; @@ -57,11 +40,11 @@ struct mdev_type { int parent_create_sysfs_files(struct mdev_parent *parent); void parent_remove_sysfs_files(struct mdev_parent *parent); -int mdev_create_sysfs_files(struct device *dev, struct mdev_type *type); -void mdev_remove_sysfs_files(struct device *dev, struct mdev_type *type); +int mdev_create_sysfs_files(struct mdev_device *mdev, struct mdev_type *type); +void mdev_remove_sysfs_files(struct mdev_device *mdev, struct mdev_type *type); int mdev_device_create(struct kobject *kobj, struct device *dev, const guid_t *uuid); -int mdev_device_remove(struct device *dev); +int mdev_device_remove(struct mdev_device *dev); #endif /* MDEV_PRIVATE_H */ diff --git a/drivers/vfio/mdev/mdev_sysfs.c b/drivers/vfio/mdev/mdev_sysfs.c index 367ff5412a38..18114f3e090a 100644 --- a/drivers/vfio/mdev/mdev_sysfs.c +++ b/drivers/vfio/mdev/mdev_sysfs.c @@ -225,6 +225,7 @@ create_err: static ssize_t remove_store(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) { + struct mdev_device *mdev = to_mdev_device(dev); unsigned long val; if (kstrtoul(buf, 0, &val) < 0) @@ -233,7 +234,7 @@ static ssize_t remove_store(struct device *dev, struct device_attribute *attr, if (val && device_remove_file_self(dev, attr)) { int ret; - ret = mdev_device_remove(dev); + ret = mdev_device_remove(mdev); if (ret) return ret; } @@ -248,34 +249,37 @@ static const struct attribute *mdev_device_attrs[] = { NULL, }; -int mdev_create_sysfs_files(struct device *dev, struct mdev_type *type) +int mdev_create_sysfs_files(struct mdev_device *mdev, struct mdev_type *type) { + struct kobject *kobj = &mdev->dev.kobj; int ret; - ret = sysfs_create_link(type->devices_kobj, &dev->kobj, dev_name(dev)); + ret = sysfs_create_link(type->devices_kobj, kobj, dev_name(&mdev->dev)); if (ret) return ret; - ret = sysfs_create_link(&dev->kobj, &type->kobj, "mdev_type"); + ret = sysfs_create_link(kobj, &type->kobj, "mdev_type"); if (ret) goto type_link_failed; - ret = sysfs_create_files(&dev->kobj, mdev_device_attrs); + ret = sysfs_create_files(kobj, mdev_device_attrs); if (ret) goto create_files_failed; return ret; create_files_failed: - sysfs_remove_link(&dev->kobj, "mdev_type"); + sysfs_remove_link(kobj, "mdev_type"); type_link_failed: - sysfs_remove_link(type->devices_kobj, dev_name(dev)); + sysfs_remove_link(type->devices_kobj, dev_name(&mdev->dev)); return ret; } -void mdev_remove_sysfs_files(struct device *dev, struct mdev_type *type) +void mdev_remove_sysfs_files(struct mdev_device *mdev, struct mdev_type *type) { - sysfs_remove_files(&dev->kobj, mdev_device_attrs); - sysfs_remove_link(&dev->kobj, "mdev_type"); - sysfs_remove_link(type->devices_kobj, dev_name(dev)); + struct kobject *kobj = &mdev->dev.kobj; + + sysfs_remove_files(kobj, mdev_device_attrs); + sysfs_remove_link(kobj, "mdev_type"); + sysfs_remove_link(type->devices_kobj, dev_name(&mdev->dev)); } diff --git a/drivers/vfio/mdev/vfio_mdev.c b/drivers/vfio/mdev/vfio_mdev.c index ae7e322fbe3c..91b7b8b9eb9c 100644 --- a/drivers/vfio/mdev/vfio_mdev.c +++ b/drivers/vfio/mdev/vfio_mdev.c @@ -124,9 +124,8 @@ static const struct vfio_device_ops vfio_mdev_dev_ops = { .request = vfio_mdev_request, }; -static int vfio_mdev_probe(struct device *dev) +static int vfio_mdev_probe(struct mdev_device *mdev) { - struct mdev_device *mdev = to_mdev_device(dev); struct vfio_device *vdev; int ret; @@ -144,9 +143,9 @@ static int vfio_mdev_probe(struct device *dev) return 0; } -static void vfio_mdev_remove(struct device *dev) +static void vfio_mdev_remove(struct mdev_device *mdev) { - struct vfio_device *vdev = dev_get_drvdata(dev); + struct vfio_device *vdev = dev_get_drvdata(&mdev->dev); vfio_unregister_group_dev(vdev); kfree(vdev); diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c index 5fb1bb52b057..53b4a6345a23 100644 --- a/drivers/vfio/vfio_iommu_type1.c +++ b/drivers/vfio/vfio_iommu_type1.c @@ -1933,28 +1933,13 @@ static bool vfio_iommu_has_sw_msi(struct list_head *group_resv_regions, return ret; } -static struct device *vfio_mdev_get_iommu_device(struct device *dev) -{ - struct device *(*fn)(struct device *dev); - struct device *iommu_device; - - fn = symbol_get(mdev_get_iommu_device); - if (fn) { - iommu_device = fn(dev); - symbol_put(mdev_get_iommu_device); - - return iommu_device; - } - - return NULL; -} - static int vfio_mdev_attach_domain(struct device *dev, void *data) { + struct mdev_device *mdev = to_mdev_device(dev); struct iommu_domain *domain = data; struct device *iommu_device; - iommu_device = vfio_mdev_get_iommu_device(dev); + iommu_device = mdev_get_iommu_device(mdev); if (iommu_device) { if (iommu_dev_feature_enabled(iommu_device, IOMMU_DEV_FEAT_AUX)) return iommu_aux_attach_device(domain, iommu_device); @@ -1967,10 +1952,11 @@ static int vfio_mdev_attach_domain(struct device *dev, void *data) static int vfio_mdev_detach_domain(struct device *dev, void *data) { + struct mdev_device *mdev = to_mdev_device(dev); struct iommu_domain *domain = data; struct device *iommu_device; - iommu_device = vfio_mdev_get_iommu_device(dev); + iommu_device = mdev_get_iommu_device(mdev); if (iommu_device) { if (iommu_dev_feature_enabled(iommu_device, IOMMU_DEV_FEAT_AUX)) iommu_aux_detach_device(domain, iommu_device); @@ -2018,9 +2004,10 @@ static bool vfio_bus_is_mdev(struct bus_type *bus) static int vfio_mdev_iommu_device(struct device *dev, void *data) { + struct mdev_device *mdev = to_mdev_device(dev); struct device **old = data, *new; - new = vfio_mdev_get_iommu_device(dev); + new = mdev_get_iommu_device(mdev); if (!new || (*old && *old != new)) return -EINVAL; diff --git a/include/linux/mdev.h b/include/linux/mdev.h index 27eb383cb95d..52f7ea19dd0f 100644 --- a/include/linux/mdev.h +++ b/include/linux/mdev.h @@ -10,7 +10,21 @@ #ifndef MDEV_H #define MDEV_H -struct mdev_device; +struct mdev_device { + struct device dev; + struct mdev_parent *parent; + guid_t uuid; + void *driver_data; + struct list_head next; + struct kobject *type_kobj; + struct device *iommu_device; + bool active; +}; + +static inline struct mdev_device *to_mdev_device(struct device *dev) +{ + return container_of(dev, struct mdev_device, dev); +} /* * Called by the parent device driver to set the device which represents @@ -19,12 +33,17 @@ struct mdev_device; * * @dev: the mediated device that iommu will isolate. * @iommu_device: a pci device which represents the iommu for @dev. - * - * Return 0 for success, otherwise negative error value. */ -int mdev_set_iommu_device(struct device *dev, struct device *iommu_device); +static inline void mdev_set_iommu_device(struct mdev_device *mdev, + struct device *iommu_device) +{ + mdev->iommu_device = iommu_device; +} -struct device *mdev_get_iommu_device(struct device *dev); +static inline struct device *mdev_get_iommu_device(struct mdev_device *mdev) +{ + return mdev->iommu_device; +} /** * struct mdev_parent_ops - Structure to be registered for each parent device to @@ -126,16 +145,25 @@ struct mdev_type_attribute mdev_type_attr_##_name = \ **/ struct mdev_driver { const char *name; - int (*probe)(struct device *dev); - void (*remove)(struct device *dev); + int (*probe)(struct mdev_device *dev); + void (*remove)(struct mdev_device *dev); struct device_driver driver; }; #define to_mdev_driver(drv) container_of(drv, struct mdev_driver, driver) -void *mdev_get_drvdata(struct mdev_device *mdev); -void mdev_set_drvdata(struct mdev_device *mdev, void *data); -const guid_t *mdev_uuid(struct mdev_device *mdev); +static inline void *mdev_get_drvdata(struct mdev_device *mdev) +{ + return mdev->driver_data; +} +static inline void mdev_set_drvdata(struct mdev_device *mdev, void *data) +{ + mdev->driver_data = data; +} +static inline const guid_t *mdev_uuid(struct mdev_device *mdev) +{ + return &mdev->uuid; +} extern struct bus_type mdev_bus_type; @@ -146,7 +174,13 @@ int mdev_register_driver(struct mdev_driver *drv, struct module *owner); void mdev_unregister_driver(struct mdev_driver *drv); struct device *mdev_parent_dev(struct mdev_device *mdev); -struct device *mdev_dev(struct mdev_device *mdev); -struct mdev_device *mdev_from_dev(struct device *dev); +static inline struct device *mdev_dev(struct mdev_device *mdev) +{ + return &mdev->dev; +} +static inline struct mdev_device *mdev_from_dev(struct device *dev) +{ + return dev->bus == &mdev_bus_type ? to_mdev_device(dev) : NULL; +} #endif /* MDEV_H */ -- cgit v1.2.3 From 91b9969d9c6bb7c02253bbfc536bfd892f636fdc Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Tue, 6 Apr 2021 16:40:27 -0300 Subject: vfio/mdev: Simplify driver registration This is only done once, we don't need to generate code to initialize a structure stored in the ELF .data segment. Fill in the three required .driver members directly instead of copying data into them during mdev_register_driver(). Further the to_mdev_driver() function doesn't belong in a public header, just inline it into the two places that need it. Finally, we can now clearly see that 'drv' derived from dev->driver cannot be NULL, firstly because the driver core forbids it, and secondly because NULL won't pass through the container_of(). Remove the dead code. Reviewed-by: Christoph Hellwig Reviewed-by: Kevin Tian Reviewed-by: Cornelia Huck Signed-off-by: Jason Gunthorpe Message-Id: <4-v2-d36939638fc6+d54-vfio2_jgg@nvidia.com> Signed-off-by: Alex Williamson --- Documentation/driver-api/vfio-mediated-device.rst | 5 +---- drivers/vfio/mdev/mdev_driver.c | 15 +++++++-------- drivers/vfio/mdev/vfio_mdev.c | 8 ++++++-- include/linux/mdev.h | 6 +----- 4 files changed, 15 insertions(+), 19 deletions(-) (limited to 'drivers') diff --git a/Documentation/driver-api/vfio-mediated-device.rst b/Documentation/driver-api/vfio-mediated-device.rst index c43c1dc33333..1779b85f014e 100644 --- a/Documentation/driver-api/vfio-mediated-device.rst +++ b/Documentation/driver-api/vfio-mediated-device.rst @@ -98,13 +98,11 @@ structure to represent a mediated device's driver:: /* * struct mdev_driver [2] - Mediated device's driver - * @name: driver name * @probe: called when new device created * @remove: called when device removed * @driver: device driver structure */ struct mdev_driver { - const char *name; int (*probe) (struct mdev_device *dev); void (*remove) (struct mdev_device *dev); struct device_driver driver; @@ -115,8 +113,7 @@ to register and unregister itself with the core driver: * Register:: - extern int mdev_register_driver(struct mdev_driver *drv, - struct module *owner); + extern int mdev_register_driver(struct mdev_driver *drv); * Unregister:: diff --git a/drivers/vfio/mdev/mdev_driver.c b/drivers/vfio/mdev/mdev_driver.c index 44c3ba7e56d9..041699571b7e 100644 --- a/drivers/vfio/mdev/mdev_driver.c +++ b/drivers/vfio/mdev/mdev_driver.c @@ -39,7 +39,8 @@ static void mdev_detach_iommu(struct mdev_device *mdev) static int mdev_probe(struct device *dev) { - struct mdev_driver *drv = to_mdev_driver(dev->driver); + struct mdev_driver *drv = + container_of(dev->driver, struct mdev_driver, driver); struct mdev_device *mdev = to_mdev_device(dev); int ret; @@ -47,7 +48,7 @@ static int mdev_probe(struct device *dev) if (ret) return ret; - if (drv && drv->probe) { + if (drv->probe) { ret = drv->probe(mdev); if (ret) mdev_detach_iommu(mdev); @@ -58,10 +59,11 @@ static int mdev_probe(struct device *dev) static int mdev_remove(struct device *dev) { - struct mdev_driver *drv = to_mdev_driver(dev->driver); + struct mdev_driver *drv = + container_of(dev->driver, struct mdev_driver, driver); struct mdev_device *mdev = to_mdev_device(dev); - if (drv && drv->remove) + if (drv->remove) drv->remove(mdev); mdev_detach_iommu(mdev); @@ -79,16 +81,13 @@ EXPORT_SYMBOL_GPL(mdev_bus_type); /** * mdev_register_driver - register a new MDEV driver * @drv: the driver to register - * @owner: module owner of driver to be registered * * Returns a negative value on error, otherwise 0. **/ -int mdev_register_driver(struct mdev_driver *drv, struct module *owner) +int mdev_register_driver(struct mdev_driver *drv) { /* initialize common driver fields */ - drv->driver.name = drv->name; drv->driver.bus = &mdev_bus_type; - drv->driver.owner = owner; /* register with core */ return driver_register(&drv->driver); diff --git a/drivers/vfio/mdev/vfio_mdev.c b/drivers/vfio/mdev/vfio_mdev.c index 91b7b8b9eb9c..cc9507ed85a1 100644 --- a/drivers/vfio/mdev/vfio_mdev.c +++ b/drivers/vfio/mdev/vfio_mdev.c @@ -152,14 +152,18 @@ static void vfio_mdev_remove(struct mdev_device *mdev) } static struct mdev_driver vfio_mdev_driver = { - .name = "vfio_mdev", + .driver = { + .name = "vfio_mdev", + .owner = THIS_MODULE, + .mod_name = KBUILD_MODNAME, + }, .probe = vfio_mdev_probe, .remove = vfio_mdev_remove, }; static int __init vfio_mdev_init(void) { - return mdev_register_driver(&vfio_mdev_driver, THIS_MODULE); + return mdev_register_driver(&vfio_mdev_driver); } static void __exit vfio_mdev_exit(void) diff --git a/include/linux/mdev.h b/include/linux/mdev.h index 52f7ea19dd0f..cb771c712da0 100644 --- a/include/linux/mdev.h +++ b/include/linux/mdev.h @@ -137,21 +137,17 @@ struct mdev_type_attribute mdev_type_attr_##_name = \ /** * struct mdev_driver - Mediated device driver - * @name: driver name * @probe: called when new device created * @remove: called when device removed * @driver: device driver structure * **/ struct mdev_driver { - const char *name; int (*probe)(struct mdev_device *dev); void (*remove)(struct mdev_device *dev); struct device_driver driver; }; -#define to_mdev_driver(drv) container_of(drv, struct mdev_driver, driver) - static inline void *mdev_get_drvdata(struct mdev_device *mdev) { return mdev->driver_data; @@ -170,7 +166,7 @@ extern struct bus_type mdev_bus_type; int mdev_register_device(struct device *dev, const struct mdev_parent_ops *ops); void mdev_unregister_device(struct device *dev); -int mdev_register_driver(struct mdev_driver *drv, struct module *owner); +int mdev_register_driver(struct mdev_driver *drv); void mdev_unregister_driver(struct mdev_driver *drv); struct device *mdev_parent_dev(struct mdev_device *mdev); -- cgit v1.2.3 From 417fd5bf242d7691c15fe0bd705ab76c69276572 Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Tue, 6 Apr 2021 16:40:28 -0300 Subject: vfio/mdev: Use struct mdev_type in struct mdev_device The kobj pointer in mdev_device is actually pointing at a struct mdev_type. Use the proper type so things are understandable. There are a number of places that are confused and passing both the mdev and the mtype as function arguments, fix these to derive the mtype directly from the mdev to remove the redundancy. Reviewed-by: Christoph Hellwig Reviewed-by: Kevin Tian Reviewed-by: Cornelia Huck Signed-off-by: Jason Gunthorpe Message-Id: <5-v2-d36939638fc6+d54-vfio2_jgg@nvidia.com> Signed-off-by: Alex Williamson --- drivers/vfio/mdev/mdev_core.c | 16 ++++++---------- drivers/vfio/mdev/mdev_private.h | 7 +++---- drivers/vfio/mdev/mdev_sysfs.c | 11 ++++++----- include/linux/mdev.h | 4 +++- 4 files changed, 18 insertions(+), 20 deletions(-) (limited to 'drivers') diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c index 057922a1707e..5ca0efa5266b 100644 --- a/drivers/vfio/mdev/mdev_core.c +++ b/drivers/vfio/mdev/mdev_core.c @@ -73,11 +73,9 @@ static void mdev_put_parent(struct mdev_parent *parent) static void mdev_device_remove_common(struct mdev_device *mdev) { struct mdev_parent *parent; - struct mdev_type *type; int ret; - type = to_mdev_type(mdev->type_kobj); - mdev_remove_sysfs_files(mdev, type); + mdev_remove_sysfs_files(mdev); device_del(&mdev->dev); parent = mdev->parent; lockdep_assert_held(&parent->unreg_sem); @@ -241,13 +239,11 @@ static void mdev_device_release(struct device *dev) mdev_device_free(mdev); } -int mdev_device_create(struct kobject *kobj, - struct device *dev, const guid_t *uuid) +int mdev_device_create(struct mdev_type *type, const guid_t *uuid) { int ret; struct mdev_device *mdev, *tmp; struct mdev_parent *parent; - struct mdev_type *type = to_mdev_type(kobj); parent = mdev_get_parent(type->parent); if (!parent) @@ -285,14 +281,14 @@ int mdev_device_create(struct kobject *kobj, } device_initialize(&mdev->dev); - mdev->dev.parent = dev; + mdev->dev.parent = parent->dev; mdev->dev.bus = &mdev_bus_type; mdev->dev.release = mdev_device_release; dev_set_name(&mdev->dev, "%pUl", uuid); mdev->dev.groups = parent->ops->mdev_attr_groups; - mdev->type_kobj = kobj; + mdev->type = type; - ret = parent->ops->create(kobj, mdev); + ret = parent->ops->create(&type->kobj, mdev); if (ret) goto ops_create_fail; @@ -300,7 +296,7 @@ int mdev_device_create(struct kobject *kobj, if (ret) goto add_fail; - ret = mdev_create_sysfs_files(mdev, type); + ret = mdev_create_sysfs_files(mdev); if (ret) goto sysfs_fail; diff --git a/drivers/vfio/mdev/mdev_private.h b/drivers/vfio/mdev/mdev_private.h index 97e2225f7f49..f12e34e16ab9 100644 --- a/drivers/vfio/mdev/mdev_private.h +++ b/drivers/vfio/mdev/mdev_private.h @@ -40,11 +40,10 @@ struct mdev_type { int parent_create_sysfs_files(struct mdev_parent *parent); void parent_remove_sysfs_files(struct mdev_parent *parent); -int mdev_create_sysfs_files(struct mdev_device *mdev, struct mdev_type *type); -void mdev_remove_sysfs_files(struct mdev_device *mdev, struct mdev_type *type); +int mdev_create_sysfs_files(struct mdev_device *mdev); +void mdev_remove_sysfs_files(struct mdev_device *mdev); -int mdev_device_create(struct kobject *kobj, - struct device *dev, const guid_t *uuid); +int mdev_device_create(struct mdev_type *kobj, const guid_t *uuid); int mdev_device_remove(struct mdev_device *dev); #endif /* MDEV_PRIVATE_H */ diff --git a/drivers/vfio/mdev/mdev_sysfs.c b/drivers/vfio/mdev/mdev_sysfs.c index 18114f3e090a..bcfe48d56e8a 100644 --- a/drivers/vfio/mdev/mdev_sysfs.c +++ b/drivers/vfio/mdev/mdev_sysfs.c @@ -67,7 +67,7 @@ static ssize_t create_store(struct kobject *kobj, struct device *dev, if (ret) return ret; - ret = mdev_device_create(kobj, dev, &uuid); + ret = mdev_device_create(to_mdev_type(kobj), &uuid); if (ret) return ret; @@ -249,8 +249,9 @@ static const struct attribute *mdev_device_attrs[] = { NULL, }; -int mdev_create_sysfs_files(struct mdev_device *mdev, struct mdev_type *type) +int mdev_create_sysfs_files(struct mdev_device *mdev) { + struct mdev_type *type = mdev->type; struct kobject *kobj = &mdev->dev.kobj; int ret; @@ -271,15 +272,15 @@ int mdev_create_sysfs_files(struct mdev_device *mdev, struct mdev_type *type) create_files_failed: sysfs_remove_link(kobj, "mdev_type"); type_link_failed: - sysfs_remove_link(type->devices_kobj, dev_name(&mdev->dev)); + sysfs_remove_link(mdev->type->devices_kobj, dev_name(&mdev->dev)); return ret; } -void mdev_remove_sysfs_files(struct mdev_device *mdev, struct mdev_type *type) +void mdev_remove_sysfs_files(struct mdev_device *mdev) { struct kobject *kobj = &mdev->dev.kobj; sysfs_remove_files(kobj, mdev_device_attrs); sysfs_remove_link(kobj, "mdev_type"); - sysfs_remove_link(type->devices_kobj, dev_name(&mdev->dev)); + sysfs_remove_link(mdev->type->devices_kobj, dev_name(&mdev->dev)); } diff --git a/include/linux/mdev.h b/include/linux/mdev.h index cb771c712da0..349e8ac1fe33 100644 --- a/include/linux/mdev.h +++ b/include/linux/mdev.h @@ -10,13 +10,15 @@ #ifndef MDEV_H #define MDEV_H +struct mdev_type; + struct mdev_device { struct device dev; struct mdev_parent *parent; guid_t uuid; void *driver_data; struct list_head next; - struct kobject *type_kobj; + struct mdev_type *type; struct device *iommu_device; bool active; }; -- cgit v1.2.3 From a9f8111d0b5f445d853345e6917c1781573e4ba9 Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Tue, 6 Apr 2021 16:40:29 -0300 Subject: vfio/mdev: Expose mdev_get/put_parent to mdev_private.h The next patch will use these in mdev_sysfs.c While here remove the now dead code checks for NULL, a mdev_type can never have a NULL parent. Reviewed-by: Christoph Hellwig Reviewed-by: Kevin Tian Reviewed-by: Cornelia Huck Signed-off-by: Jason Gunthorpe Message-Id: <6-v2-d36939638fc6+d54-vfio2_jgg@nvidia.com> Signed-off-by: Alex Williamson --- drivers/vfio/mdev/mdev_core.c | 23 +++-------------------- drivers/vfio/mdev/mdev_private.h | 12 ++++++++++++ 2 files changed, 15 insertions(+), 20 deletions(-) (limited to 'drivers') diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c index 5ca0efa5266b..7ec21c907397 100644 --- a/drivers/vfio/mdev/mdev_core.c +++ b/drivers/vfio/mdev/mdev_core.c @@ -45,7 +45,7 @@ static struct mdev_parent *__find_parent_device(struct device *dev) return NULL; } -static void mdev_release_parent(struct kref *kref) +void mdev_release_parent(struct kref *kref) { struct mdev_parent *parent = container_of(kref, struct mdev_parent, ref); @@ -55,20 +55,6 @@ static void mdev_release_parent(struct kref *kref) put_device(dev); } -static struct mdev_parent *mdev_get_parent(struct mdev_parent *parent) -{ - if (parent) - kref_get(&parent->ref); - - return parent; -} - -static void mdev_put_parent(struct mdev_parent *parent) -{ - if (parent) - kref_put(&parent->ref, mdev_release_parent); -} - /* Caller must hold parent unreg_sem read or write lock */ static void mdev_device_remove_common(struct mdev_device *mdev) { @@ -243,12 +229,9 @@ int mdev_device_create(struct mdev_type *type, const guid_t *uuid) { int ret; struct mdev_device *mdev, *tmp; - struct mdev_parent *parent; - - parent = mdev_get_parent(type->parent); - if (!parent) - return -EINVAL; + struct mdev_parent *parent = type->parent; + mdev_get_parent(parent); mutex_lock(&mdev_list_lock); /* Check for duplicate */ diff --git a/drivers/vfio/mdev/mdev_private.h b/drivers/vfio/mdev/mdev_private.h index f12e34e16ab9..fddab240ccc3 100644 --- a/drivers/vfio/mdev/mdev_private.h +++ b/drivers/vfio/mdev/mdev_private.h @@ -46,4 +46,16 @@ void mdev_remove_sysfs_files(struct mdev_device *mdev); int mdev_device_create(struct mdev_type *kobj, const guid_t *uuid); int mdev_device_remove(struct mdev_device *dev); +void mdev_release_parent(struct kref *kref); + +static inline void mdev_get_parent(struct mdev_parent *parent) +{ + kref_get(&parent->ref); +} + +static inline void mdev_put_parent(struct mdev_parent *parent) +{ + kref_put(&parent->ref, mdev_release_parent); +} + #endif /* MDEV_PRIVATE_H */ -- cgit v1.2.3 From 9a302449a58d45d0ef2aab686f64b35919bc604c Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Tue, 6 Apr 2021 16:40:30 -0300 Subject: vfio/mdev: Add missing reference counting to mdev_type struct mdev_type holds a pointer to the kref'd object struct mdev_parent, but doesn't hold the kref. The lifetime of the parent becomes implicit because parent_remove_sysfs_files() is supposed to remove all the access before the parent can be freed, but this is very hard to reason about. Make it obviously correct by adding the missing get. Reviewed-by: Christoph Hellwig Reviewed-by: Kevin Tian Reviewed-by: Cornelia Huck Signed-off-by: Jason Gunthorpe Message-Id: <7-v2-d36939638fc6+d54-vfio2_jgg@nvidia.com> Signed-off-by: Alex Williamson --- drivers/vfio/mdev/mdev_sysfs.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'drivers') diff --git a/drivers/vfio/mdev/mdev_sysfs.c b/drivers/vfio/mdev/mdev_sysfs.c index bcfe48d56e8a..8c169d12ba7d 100644 --- a/drivers/vfio/mdev/mdev_sysfs.c +++ b/drivers/vfio/mdev/mdev_sysfs.c @@ -81,6 +81,8 @@ static void mdev_type_release(struct kobject *kobj) struct mdev_type *type = to_mdev_type(kobj); pr_debug("Releasing group %s\n", kobj->name); + /* Pairs with the get in add_mdev_supported_type() */ + mdev_put_parent(type->parent); kfree(type); } @@ -106,6 +108,8 @@ static struct mdev_type *add_mdev_supported_type(struct mdev_parent *parent, type->kobj.kset = parent->mdev_types_kset; type->parent = parent; + /* Pairs with the put in mdev_type_release() */ + mdev_get_parent(parent); ret = kobject_init_and_add(&type->kobj, &mdev_type_ktype, NULL, "%s-%s", dev_driver_string(parent->dev), -- cgit v1.2.3 From fbd0e2b0c3d0b2eeaef471c9fe19ae5a7b2ee970 Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Tue, 6 Apr 2021 16:40:31 -0300 Subject: vfio/mdev: Reorganize mdev_device_create() Once the memory for the struct mdev_device is allocated it should immediately be device_initialize()'d and filled in so that put_device() can always be used to undo the allocation. Place the mdev_get/put_parent() so that they are clearly protecting the mdev->parent pointer. Move the final put to the release function so that the lifetime rules are trivial to understand. Update the goto labels to follow the normal convention. Remove mdev_device_free() as the release function via device_put() is now usable in all cases. Reviewed-by: Christoph Hellwig Reviewed-by: Kevin Tian Reviewed-by: Cornelia Huck Signed-off-by: Jason Gunthorpe Message-Id: <8-v2-d36939638fc6+d54-vfio2_jgg@nvidia.com> Signed-off-by: Alex Williamson --- drivers/vfio/mdev/mdev_core.c | 60 +++++++++++++++++++------------------------ 1 file changed, 27 insertions(+), 33 deletions(-) (limited to 'drivers') diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c index 7ec21c907397..f7559835b061 100644 --- a/drivers/vfio/mdev/mdev_core.c +++ b/drivers/vfio/mdev/mdev_core.c @@ -71,7 +71,6 @@ static void mdev_device_remove_common(struct mdev_device *mdev) /* Balances with device_initialize() */ put_device(&mdev->dev); - mdev_put_parent(parent); } static int mdev_device_remove_cb(struct device *dev, void *data) @@ -208,8 +207,13 @@ void mdev_unregister_device(struct device *dev) } EXPORT_SYMBOL(mdev_unregister_device); -static void mdev_device_free(struct mdev_device *mdev) +static void mdev_device_release(struct device *dev) { + struct mdev_device *mdev = to_mdev_device(dev); + + /* Pairs with the get in mdev_device_create() */ + mdev_put_parent(mdev->parent); + mutex_lock(&mdev_list_lock); list_del(&mdev->next); mutex_unlock(&mdev_list_lock); @@ -218,70 +222,61 @@ static void mdev_device_free(struct mdev_device *mdev) kfree(mdev); } -static void mdev_device_release(struct device *dev) -{ - struct mdev_device *mdev = to_mdev_device(dev); - - mdev_device_free(mdev); -} - int mdev_device_create(struct mdev_type *type, const guid_t *uuid) { int ret; struct mdev_device *mdev, *tmp; struct mdev_parent *parent = type->parent; - mdev_get_parent(parent); mutex_lock(&mdev_list_lock); /* Check for duplicate */ list_for_each_entry(tmp, &mdev_list, next) { if (guid_equal(&tmp->uuid, uuid)) { mutex_unlock(&mdev_list_lock); - ret = -EEXIST; - goto mdev_fail; + return -EEXIST; } } mdev = kzalloc(sizeof(*mdev), GFP_KERNEL); if (!mdev) { mutex_unlock(&mdev_list_lock); - ret = -ENOMEM; - goto mdev_fail; + return -ENOMEM; } + device_initialize(&mdev->dev); + mdev->dev.parent = parent->dev; + mdev->dev.bus = &mdev_bus_type; + mdev->dev.release = mdev_device_release; + mdev->dev.groups = parent->ops->mdev_attr_groups; + mdev->type = type; + mdev->parent = parent; + /* Pairs with the put in mdev_device_release() */ + mdev_get_parent(parent); + guid_copy(&mdev->uuid, uuid); list_add(&mdev->next, &mdev_list); mutex_unlock(&mdev_list_lock); - mdev->parent = parent; + dev_set_name(&mdev->dev, "%pUl", uuid); /* Check if parent unregistration has started */ if (!down_read_trylock(&parent->unreg_sem)) { - mdev_device_free(mdev); ret = -ENODEV; - goto mdev_fail; + goto out_put_device; } - device_initialize(&mdev->dev); - mdev->dev.parent = parent->dev; - mdev->dev.bus = &mdev_bus_type; - mdev->dev.release = mdev_device_release; - dev_set_name(&mdev->dev, "%pUl", uuid); - mdev->dev.groups = parent->ops->mdev_attr_groups; - mdev->type = type; - ret = parent->ops->create(&type->kobj, mdev); if (ret) - goto ops_create_fail; + goto out_unlock; ret = device_add(&mdev->dev); if (ret) - goto add_fail; + goto out_remove; ret = mdev_create_sysfs_files(mdev); if (ret) - goto sysfs_fail; + goto out_del; mdev->active = true; dev_dbg(&mdev->dev, "MDEV: created\n"); @@ -289,15 +284,14 @@ int mdev_device_create(struct mdev_type *type, const guid_t *uuid) return 0; -sysfs_fail: +out_del: device_del(&mdev->dev); -add_fail: +out_remove: parent->ops->remove(mdev); -ops_create_fail: +out_unlock: up_read(&parent->unreg_sem); +out_put_device: put_device(&mdev->dev); -mdev_fail: - mdev_put_parent(parent); return ret; } -- cgit v1.2.3 From 18d731242d5c67c0783126c42d3f85870cec2df5 Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Tue, 6 Apr 2021 16:40:32 -0300 Subject: vfio/mdev: Add missing error handling to dev_set_name() This can fail, and seems to be a popular target for syzkaller error injection. Check the error return and unwind with put_device(). Fixes: 7b96953bc640 ("vfio: Mediated device Core driver") Reviewed-by: Christoph Hellwig Reviewed-by: Kevin Tian Reviewed-by: Max Gurtovoy Reviewed-by: Cornelia Huck Signed-off-by: Jason Gunthorpe Message-Id: <9-v2-d36939638fc6+d54-vfio2_jgg@nvidia.com> Signed-off-by: Alex Williamson --- drivers/vfio/mdev/mdev_core.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c index f7559835b061..4caedb3d4fbf 100644 --- a/drivers/vfio/mdev/mdev_core.c +++ b/drivers/vfio/mdev/mdev_core.c @@ -258,7 +258,9 @@ int mdev_device_create(struct mdev_type *type, const guid_t *uuid) list_add(&mdev->next, &mdev_list); mutex_unlock(&mdev_list_lock); - dev_set_name(&mdev->dev, "%pUl", uuid); + ret = dev_set_name(&mdev->dev, "%pUl", uuid); + if (ret) + goto out_put_device; /* Check if parent unregistration has started */ if (!down_read_trylock(&parent->unreg_sem)) { -- cgit v1.2.3 From fbea43239074e16c91048f5ce70378664efbdb99 Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Tue, 6 Apr 2021 16:40:33 -0300 Subject: vfio/mdev: Remove duplicate storage of parent in mdev_device mdev_device->type->parent is the same thing. The struct mdev_device was relying on the kref on the mdev_parent to also indirectly hold a kref on the mdev_type pointer. Now that the type holds a kref on the parent we can directly kref the mdev_type and remove this implicit relationship. Reviewed-by: Christoph Hellwig Reviewed-by: Cornelia Huck Signed-off-by: Jason Gunthorpe Message-Id: <10-v2-d36939638fc6+d54-vfio2_jgg@nvidia.com> Signed-off-by: Alex Williamson --- drivers/vfio/mdev/mdev_core.c | 13 +++++-------- drivers/vfio/mdev/vfio_mdev.c | 14 +++++++------- include/linux/mdev.h | 1 - 3 files changed, 12 insertions(+), 16 deletions(-) (limited to 'drivers') diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c index 4caedb3d4fbf..2a20bdaf6142 100644 --- a/drivers/vfio/mdev/mdev_core.c +++ b/drivers/vfio/mdev/mdev_core.c @@ -29,7 +29,7 @@ static DEFINE_MUTEX(mdev_list_lock); struct device *mdev_parent_dev(struct mdev_device *mdev) { - return mdev->parent->dev; + return mdev->type->parent->dev; } EXPORT_SYMBOL(mdev_parent_dev); @@ -58,12 +58,11 @@ void mdev_release_parent(struct kref *kref) /* Caller must hold parent unreg_sem read or write lock */ static void mdev_device_remove_common(struct mdev_device *mdev) { - struct mdev_parent *parent; + struct mdev_parent *parent = mdev->type->parent; int ret; mdev_remove_sysfs_files(mdev); device_del(&mdev->dev); - parent = mdev->parent; lockdep_assert_held(&parent->unreg_sem); ret = parent->ops->remove(mdev); if (ret) @@ -212,7 +211,7 @@ static void mdev_device_release(struct device *dev) struct mdev_device *mdev = to_mdev_device(dev); /* Pairs with the get in mdev_device_create() */ - mdev_put_parent(mdev->parent); + kobject_put(&mdev->type->kobj); mutex_lock(&mdev_list_lock); list_del(&mdev->next); @@ -250,9 +249,8 @@ int mdev_device_create(struct mdev_type *type, const guid_t *uuid) mdev->dev.release = mdev_device_release; mdev->dev.groups = parent->ops->mdev_attr_groups; mdev->type = type; - mdev->parent = parent; /* Pairs with the put in mdev_device_release() */ - mdev_get_parent(parent); + kobject_get(&type->kobj); guid_copy(&mdev->uuid, uuid); list_add(&mdev->next, &mdev_list); @@ -300,7 +298,7 @@ out_put_device: int mdev_device_remove(struct mdev_device *mdev) { struct mdev_device *tmp; - struct mdev_parent *parent; + struct mdev_parent *parent = mdev->type->parent; mutex_lock(&mdev_list_lock); list_for_each_entry(tmp, &mdev_list, next) { @@ -321,7 +319,6 @@ int mdev_device_remove(struct mdev_device *mdev) mdev->active = false; mutex_unlock(&mdev_list_lock); - parent = mdev->parent; /* Check if parent unregistration has started */ if (!down_read_trylock(&parent->unreg_sem)) return -ENODEV; diff --git a/drivers/vfio/mdev/vfio_mdev.c b/drivers/vfio/mdev/vfio_mdev.c index cc9507ed85a1..922729071c5a 100644 --- a/drivers/vfio/mdev/vfio_mdev.c +++ b/drivers/vfio/mdev/vfio_mdev.c @@ -24,7 +24,7 @@ static int vfio_mdev_open(struct vfio_device *core_vdev) { struct mdev_device *mdev = to_mdev_device(core_vdev->dev); - struct mdev_parent *parent = mdev->parent; + struct mdev_parent *parent = mdev->type->parent; int ret; @@ -44,7 +44,7 @@ static int vfio_mdev_open(struct vfio_device *core_vdev) static void vfio_mdev_release(struct vfio_device *core_vdev) { struct mdev_device *mdev = to_mdev_device(core_vdev->dev); - struct mdev_parent *parent = mdev->parent; + struct mdev_parent *parent = mdev->type->parent; if (likely(parent->ops->release)) parent->ops->release(mdev); @@ -56,7 +56,7 @@ static long vfio_mdev_unlocked_ioctl(struct vfio_device *core_vdev, unsigned int cmd, unsigned long arg) { struct mdev_device *mdev = to_mdev_device(core_vdev->dev); - struct mdev_parent *parent = mdev->parent; + struct mdev_parent *parent = mdev->type->parent; if (unlikely(!parent->ops->ioctl)) return -EINVAL; @@ -68,7 +68,7 @@ static ssize_t vfio_mdev_read(struct vfio_device *core_vdev, char __user *buf, size_t count, loff_t *ppos) { struct mdev_device *mdev = to_mdev_device(core_vdev->dev); - struct mdev_parent *parent = mdev->parent; + struct mdev_parent *parent = mdev->type->parent; if (unlikely(!parent->ops->read)) return -EINVAL; @@ -81,7 +81,7 @@ static ssize_t vfio_mdev_write(struct vfio_device *core_vdev, loff_t *ppos) { struct mdev_device *mdev = to_mdev_device(core_vdev->dev); - struct mdev_parent *parent = mdev->parent; + struct mdev_parent *parent = mdev->type->parent; if (unlikely(!parent->ops->write)) return -EINVAL; @@ -93,7 +93,7 @@ static int vfio_mdev_mmap(struct vfio_device *core_vdev, struct vm_area_struct *vma) { struct mdev_device *mdev = to_mdev_device(core_vdev->dev); - struct mdev_parent *parent = mdev->parent; + struct mdev_parent *parent = mdev->type->parent; if (unlikely(!parent->ops->mmap)) return -EINVAL; @@ -104,7 +104,7 @@ static int vfio_mdev_mmap(struct vfio_device *core_vdev, static void vfio_mdev_request(struct vfio_device *core_vdev, unsigned int count) { struct mdev_device *mdev = to_mdev_device(core_vdev->dev); - struct mdev_parent *parent = mdev->parent; + struct mdev_parent *parent = mdev->type->parent; if (parent->ops->request) parent->ops->request(mdev, count); diff --git a/include/linux/mdev.h b/include/linux/mdev.h index 349e8ac1fe33..fb582adda28a 100644 --- a/include/linux/mdev.h +++ b/include/linux/mdev.h @@ -14,7 +14,6 @@ struct mdev_type; struct mdev_device { struct device dev; - struct mdev_parent *parent; guid_t uuid; void *driver_data; struct list_head next; -- cgit v1.2.3 From 15fcc44be0c7afa2945b1896a96ac2ddf09f1fa7 Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Tue, 6 Apr 2021 16:40:34 -0300 Subject: vfio/mdev: Add mdev/mtype_get_type_group_id() This returns the index in the supported_type_groups array that is associated with the mdev_type attached to the struct mdev_device or its containing struct kobject. Each mdev_device can be spawned from exactly one mdev_type, which in turn originates from exactly one supported_type_group. Drivers are using weird string calculations to try and get back to this index, providing a direct access to the index removes a bunch of wonky driver code. mdev_type->group can be deleted as the group is obtained using the type_group_id. Reviewed-by: Christoph Hellwig Reviewed-by: Kevin Tian Reviewed-by: Cornelia Huck Signed-off-by: Jason Gunthorpe Message-Id: <11-v2-d36939638fc6+d54-vfio2_jgg@nvidia.com> Signed-off-by: Alex Williamson --- drivers/vfio/mdev/mdev_core.c | 20 ++++++++++++++++++++ drivers/vfio/mdev/mdev_private.h | 2 +- drivers/vfio/mdev/mdev_sysfs.c | 15 +++++++++------ include/linux/mdev.h | 3 +++ 4 files changed, 33 insertions(+), 7 deletions(-) (limited to 'drivers') diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c index 2a20bdaf6142..5ae06f951a09 100644 --- a/drivers/vfio/mdev/mdev_core.c +++ b/drivers/vfio/mdev/mdev_core.c @@ -33,6 +33,26 @@ struct device *mdev_parent_dev(struct mdev_device *mdev) } EXPORT_SYMBOL(mdev_parent_dev); +/* + * Return the index in supported_type_groups that this mdev_device was created + * from. + */ +unsigned int mdev_get_type_group_id(struct mdev_device *mdev) +{ + return mdev->type->type_group_id; +} +EXPORT_SYMBOL(mdev_get_type_group_id); + +/* + * Used in mdev_type_attribute sysfs functions to return the index in the + * supported_type_groups that the sysfs is called from. + */ +unsigned int mtype_get_type_group_id(struct kobject *mtype_kobj) +{ + return container_of(mtype_kobj, struct mdev_type, kobj)->type_group_id; +} +EXPORT_SYMBOL(mtype_get_type_group_id); + /* Should be called holding parent_list_lock */ static struct mdev_parent *__find_parent_device(struct device *dev) { diff --git a/drivers/vfio/mdev/mdev_private.h b/drivers/vfio/mdev/mdev_private.h index fddab240ccc3..6999c89db7b1 100644 --- a/drivers/vfio/mdev/mdev_private.h +++ b/drivers/vfio/mdev/mdev_private.h @@ -29,7 +29,7 @@ struct mdev_type { struct kobject *devices_kobj; struct mdev_parent *parent; struct list_head next; - struct attribute_group *group; + unsigned int type_group_id; }; #define to_mdev_type_attr(_attr) \ diff --git a/drivers/vfio/mdev/mdev_sysfs.c b/drivers/vfio/mdev/mdev_sysfs.c index 8c169d12ba7d..712fbc78b12e 100644 --- a/drivers/vfio/mdev/mdev_sysfs.c +++ b/drivers/vfio/mdev/mdev_sysfs.c @@ -92,9 +92,11 @@ static struct kobj_type mdev_type_ktype = { }; static struct mdev_type *add_mdev_supported_type(struct mdev_parent *parent, - struct attribute_group *group) + unsigned int type_group_id) { struct mdev_type *type; + struct attribute_group *group = + parent->ops->supported_type_groups[type_group_id]; int ret; if (!group->name) { @@ -110,6 +112,7 @@ static struct mdev_type *add_mdev_supported_type(struct mdev_parent *parent, type->parent = parent; /* Pairs with the put in mdev_type_release() */ mdev_get_parent(parent); + type->type_group_id = type_group_id; ret = kobject_init_and_add(&type->kobj, &mdev_type_ktype, NULL, "%s-%s", dev_driver_string(parent->dev), @@ -135,8 +138,6 @@ static struct mdev_type *add_mdev_supported_type(struct mdev_parent *parent, ret = -ENOMEM; goto attrs_failed; } - - type->group = group; return type; attrs_failed: @@ -151,8 +152,11 @@ attr_create_failed: static void remove_mdev_supported_type(struct mdev_type *type) { + struct attribute_group *group = + type->parent->ops->supported_type_groups[type->type_group_id]; + sysfs_remove_files(&type->kobj, - (const struct attribute **)type->group->attrs); + (const struct attribute **)group->attrs); kobject_put(type->devices_kobj); sysfs_remove_file(&type->kobj, &mdev_type_attr_create.attr); kobject_del(&type->kobj); @@ -166,8 +170,7 @@ static int add_mdev_supported_type_groups(struct mdev_parent *parent) for (i = 0; parent->ops->supported_type_groups[i]; i++) { struct mdev_type *type; - type = add_mdev_supported_type(parent, - parent->ops->supported_type_groups[i]); + type = add_mdev_supported_type(parent, i); if (IS_ERR(type)) { struct mdev_type *ltype, *tmp; diff --git a/include/linux/mdev.h b/include/linux/mdev.h index fb582adda28a..41e919365223 100644 --- a/include/linux/mdev.h +++ b/include/linux/mdev.h @@ -46,6 +46,9 @@ static inline struct device *mdev_get_iommu_device(struct mdev_device *mdev) return mdev->iommu_device; } +unsigned int mdev_get_type_group_id(struct mdev_device *mdev); +unsigned int mtype_get_type_group_id(struct kobject *mtype_kobj); + /** * struct mdev_parent_ops - Structure to be registered for each parent device to * register the device to mdev module. -- cgit v1.2.3 From 07e543f4f9d116d6b4240644191dee6388ef4a85 Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Tue, 6 Apr 2021 16:40:38 -0300 Subject: vfio/gvt: Make DRM_I915_GVT depend on VFIO_MDEV At some point there may have been some reason for this weird split in this driver, but today only the VFIO side is actually implemented. However, it got messed up at some point and mdev code was put in gvt.c and is pretending to be "generic" by masquerading as some generic attribute list: static MDEV_TYPE_ATTR_RO(description); But MDEV_TYPE attributes are only usable with mdev_device, nothing else. Ideally all of this would be moved to kvmgt.c, but it is entangled with the rest of the "generic" code in an odd way. Thus put in a kconfig dependency so we don't get randconfig failures when the next patch creates a link time dependency related to the use of MDEV_TYPE. Reviewed-by: Kevin Tian Reviewed-by: Christoph Hellwig Signed-off-by: Jason Gunthorpe Message-Id: <15-v2-d36939638fc6+d54-vfio2_jgg@nvidia.com> Acked-by: Zhenyu Wang Signed-off-by: Alex Williamson --- drivers/gpu/drm/i915/Kconfig | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers') diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig index 1e1cb245fca7..483e9ff8ca1d 100644 --- a/drivers/gpu/drm/i915/Kconfig +++ b/drivers/gpu/drm/i915/Kconfig @@ -101,6 +101,7 @@ config DRM_I915_GVT bool "Enable Intel GVT-g graphics virtualization host support" depends on DRM_I915 depends on 64BIT + depends on VFIO_MDEV default n help Choose this option if you want to enable Intel GVT-g graphics -- cgit v1.2.3 From 383987fd15ba8f37bdc20994dadeb13df76342d6 Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Tue, 6 Apr 2021 16:40:39 -0300 Subject: vfio/gvt: Use mdev_get_type_group_id() intel_gvt_init_vgpu_type_groups() makes gvt->types 1:1 with the supported_type_groups array, so the type_group_id is also the index into gvt->types. Use it directly and remove the string matching. Reviewed-by: Kevin Tian Reviewed-by: Christoph Hellwig Signed-off-by: Jason Gunthorpe Message-Id: <16-v2-d36939638fc6+d54-vfio2_jgg@nvidia.com> Reviewed-by: Zhenyu Wang Signed-off-by: Alex Williamson --- drivers/gpu/drm/i915/gvt/gvt.c | 24 +++++++----------------- drivers/gpu/drm/i915/gvt/gvt.h | 4 ++-- drivers/gpu/drm/i915/gvt/kvmgt.c | 5 ++--- 3 files changed, 11 insertions(+), 22 deletions(-) (limited to 'drivers') diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c index d1d8ee4a5f16..4b47a18e9dfa 100644 --- a/drivers/gpu/drm/i915/gvt/gvt.c +++ b/drivers/gpu/drm/i915/gvt/gvt.c @@ -46,22 +46,12 @@ static const char * const supported_hypervisors[] = { [INTEL_GVT_HYPERVISOR_KVM] = "KVM", }; -static struct intel_vgpu_type *intel_gvt_find_vgpu_type(struct intel_gvt *gvt, - const char *name) +static struct intel_vgpu_type * +intel_gvt_find_vgpu_type(struct intel_gvt *gvt, unsigned int type_group_id) { - const char *driver_name = - dev_driver_string(&gvt->gt->i915->drm.pdev->dev); - int i; - - name += strlen(driver_name) + 1; - for (i = 0; i < gvt->num_types; i++) { - struct intel_vgpu_type *t = &gvt->types[i]; - - if (!strncmp(t->name, name, sizeof(t->name))) - return t; - } - - return NULL; + if (WARN_ON(type_group_id >= gvt->num_types)) + return NULL; + return &gvt->types[type_group_id]; } static ssize_t available_instances_show(struct kobject *kobj, @@ -71,7 +61,7 @@ static ssize_t available_instances_show(struct kobject *kobj, unsigned int num = 0; void *gvt = kdev_to_i915(dev)->gvt; - type = intel_gvt_find_vgpu_type(gvt, kobject_name(kobj)); + type = intel_gvt_find_vgpu_type(gvt, mtype_get_type_group_id(kobj)); if (!type) num = 0; else @@ -92,7 +82,7 @@ static ssize_t description_show(struct kobject *kobj, struct device *dev, struct intel_vgpu_type *type; void *gvt = kdev_to_i915(dev)->gvt; - type = intel_gvt_find_vgpu_type(gvt, kobject_name(kobj)); + type = intel_gvt_find_vgpu_type(gvt, mtype_get_type_group_id(kobj)); if (!type) return 0; diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h index 03c993d68f10..0cf480f42850 100644 --- a/drivers/gpu/drm/i915/gvt/gvt.h +++ b/drivers/gpu/drm/i915/gvt/gvt.h @@ -569,8 +569,8 @@ struct intel_gvt_ops { void (*vgpu_reset)(struct intel_vgpu *); void (*vgpu_activate)(struct intel_vgpu *); void (*vgpu_deactivate)(struct intel_vgpu *); - struct intel_vgpu_type *(*gvt_find_vgpu_type)(struct intel_gvt *gvt, - const char *name); + struct intel_vgpu_type *(*gvt_find_vgpu_type)( + struct intel_gvt *gvt, unsigned int type_group_id); bool (*get_gvt_attrs)(struct attribute_group ***intel_vgpu_type_groups); int (*vgpu_query_plane)(struct intel_vgpu *vgpu, void *); int (*vgpu_get_dmabuf)(struct intel_vgpu *vgpu, unsigned int); diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c index b4348256ae95..16e1e4a38aa1 100644 --- a/drivers/gpu/drm/i915/gvt/kvmgt.c +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c @@ -700,10 +700,9 @@ static int intel_vgpu_create(struct kobject *kobj, struct mdev_device *mdev) pdev = mdev_parent_dev(mdev); gvt = kdev_to_i915(pdev)->gvt; - type = intel_gvt_ops->gvt_find_vgpu_type(gvt, kobject_name(kobj)); + type = intel_gvt_ops->gvt_find_vgpu_type(gvt, + mdev_get_type_group_id(mdev)); if (!type) { - gvt_vgpu_err("failed to find type %s to create\n", - kobject_name(kobj)); ret = -EINVAL; goto out; } -- cgit v1.2.3 From c2ef2f50ad0ccf5460bf4824bc6669240b6c7936 Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Tue, 6 Apr 2021 16:40:40 -0300 Subject: vfio/mdev: Remove kobj from mdev_parent_ops->create() The kobj here is a type-erased version of mdev_type, which is already stored in the struct mdev_device being passed in. It was only ever used to compute the type_group_id, which is now extracted directly from the mdev. Reviewed-by: Kevin Tian Reviewed-by: Cornelia Huck Reviewed-by: Christoph Hellwig Signed-off-by: Jason Gunthorpe Message-Id: <17-v2-d36939638fc6+d54-vfio2_jgg@nvidia.com> Signed-off-by: Alex Williamson --- drivers/gpu/drm/i915/gvt/kvmgt.c | 2 +- drivers/s390/cio/vfio_ccw_ops.c | 2 +- drivers/s390/crypto/vfio_ap_ops.c | 2 +- drivers/vfio/mdev/mdev_core.c | 2 +- include/linux/mdev.h | 3 +-- samples/vfio-mdev/mbochs.c | 2 +- samples/vfio-mdev/mdpy.c | 2 +- samples/vfio-mdev/mtty.c | 2 +- 8 files changed, 8 insertions(+), 9 deletions(-) (limited to 'drivers') diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c index 16e1e4a38aa1..6bf176e8426e 100644 --- a/drivers/gpu/drm/i915/gvt/kvmgt.c +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c @@ -689,7 +689,7 @@ static void kvmgt_put_vfio_device(void *vgpu) vfio_device_put(vdev->vfio_device); } -static int intel_vgpu_create(struct kobject *kobj, struct mdev_device *mdev) +static int intel_vgpu_create(struct mdev_device *mdev) { struct intel_vgpu *vgpu = NULL; struct intel_vgpu_type *type; diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c index 767ac41686fe..10407cf67583 100644 --- a/drivers/s390/cio/vfio_ccw_ops.c +++ b/drivers/s390/cio/vfio_ccw_ops.c @@ -110,7 +110,7 @@ static struct attribute_group *mdev_type_groups[] = { NULL, }; -static int vfio_ccw_mdev_create(struct kobject *kobj, struct mdev_device *mdev) +static int vfio_ccw_mdev_create(struct mdev_device *mdev) { struct vfio_ccw_private *private = dev_get_drvdata(mdev_parent_dev(mdev)); diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c index 1ffdd411201c..d319152dd484 100644 --- a/drivers/s390/crypto/vfio_ap_ops.c +++ b/drivers/s390/crypto/vfio_ap_ops.c @@ -322,7 +322,7 @@ static void vfio_ap_matrix_init(struct ap_config_info *info, matrix->adm_max = info->apxa ? info->Nd : 15; } -static int vfio_ap_mdev_create(struct kobject *kobj, struct mdev_device *mdev) +static int vfio_ap_mdev_create(struct mdev_device *mdev) { struct ap_matrix_mdev *matrix_mdev; diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c index 5ae06f951a09..10eff33ce1f2 100644 --- a/drivers/vfio/mdev/mdev_core.c +++ b/drivers/vfio/mdev/mdev_core.c @@ -286,7 +286,7 @@ int mdev_device_create(struct mdev_type *type, const guid_t *uuid) goto out_put_device; } - ret = parent->ops->create(&type->kobj, mdev); + ret = parent->ops->create(mdev); if (ret) goto out_unlock; diff --git a/include/linux/mdev.h b/include/linux/mdev.h index 41e919365223..c3a800051d61 100644 --- a/include/linux/mdev.h +++ b/include/linux/mdev.h @@ -61,7 +61,6 @@ unsigned int mtype_get_type_group_id(struct kobject *mtype_kobj); * @create: Called to allocate basic resources in parent device's * driver for a particular mediated device. It is * mandatory to provide create ops. - * @kobj: kobject of type for which 'create' is called. * @mdev: mdev_device structure on of mediated device * that is being created * Returns integer: success (0) or error (< 0) @@ -107,7 +106,7 @@ struct mdev_parent_ops { const struct attribute_group **mdev_attr_groups; struct attribute_group **supported_type_groups; - int (*create)(struct kobject *kobj, struct mdev_device *mdev); + int (*create)(struct mdev_device *mdev); int (*remove)(struct mdev_device *mdev); int (*open)(struct mdev_device *mdev); void (*release)(struct mdev_device *mdev); diff --git a/samples/vfio-mdev/mbochs.c b/samples/vfio-mdev/mbochs.c index a1af30df10a2..ac4d0dc24907 100644 --- a/samples/vfio-mdev/mbochs.c +++ b/samples/vfio-mdev/mbochs.c @@ -506,7 +506,7 @@ static int mbochs_reset(struct mdev_device *mdev) return 0; } -static int mbochs_create(struct kobject *kobj, struct mdev_device *mdev) +static int mbochs_create(struct mdev_device *mdev) { const struct mbochs_type *type = &mbochs_types[mdev_get_type_group_id(mdev)]; diff --git a/samples/vfio-mdev/mdpy.c b/samples/vfio-mdev/mdpy.c index 08c15f9f06a8..da88fd7dd423 100644 --- a/samples/vfio-mdev/mdpy.c +++ b/samples/vfio-mdev/mdpy.c @@ -216,7 +216,7 @@ static int mdpy_reset(struct mdev_device *mdev) return 0; } -static int mdpy_create(struct kobject *kobj, struct mdev_device *mdev) +static int mdpy_create(struct mdev_device *mdev) { const struct mdpy_type *type = &mdpy_types[mdev_get_type_group_id(mdev)]; diff --git a/samples/vfio-mdev/mtty.c b/samples/vfio-mdev/mtty.c index 191a587a8d5a..f2e36c06ac6a 100644 --- a/samples/vfio-mdev/mtty.c +++ b/samples/vfio-mdev/mtty.c @@ -708,7 +708,7 @@ accessfailed: return ret; } -static int mtty_create(struct kobject *kobj, struct mdev_device *mdev) +static int mtty_create(struct mdev_device *mdev) { struct mdev_state *mdev_state; int nr_ports = mdev_get_type_group_id(mdev) + 1; -- cgit v1.2.3 From 9169cff168ff262b4b78597f542e23843d0c494a Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Tue, 6 Apr 2021 16:40:41 -0300 Subject: vfio/mdev: Correct the function signatures for the mdev_type_attributes The driver core standard is to pass in the properly typed object, the properly typed attribute and the buffer data. It stems from the root kobject method: ssize_t (*show)(struct kobject *kobj, struct kobj_attribute *attr,..) Each subclass of kobject should provide their own function with the same signature but more specific types, eg struct device uses: ssize_t (*show)(struct device *dev, struct device_attribute *attr,..) In this case the existing signature is: ssize_t (*show)(struct kobject *kobj, struct device *dev,..) Where kobj is a 'struct mdev_type *' and dev is 'mdev_type->parent->dev'. Change the mdev_type related sysfs attribute functions to: ssize_t (*show)(struct mdev_type *mtype, struct mdev_type_attribute *attr,..) In order to restore type safety and match the driver core standard There are no current users of 'attr', but if it is ever needed it would be hard to add in retroactively, so do it now. Reviewed-by: Kevin Tian Reviewed-by: Cornelia Huck Reviewed-by: Christoph Hellwig Signed-off-by: Jason Gunthorpe Message-Id: <18-v2-d36939638fc6+d54-vfio2_jgg@nvidia.com> Signed-off-by: Alex Williamson --- drivers/gpu/drm/i915/gvt/gvt.c | 21 +++++++++++---------- drivers/s390/cio/vfio_ccw_ops.c | 15 +++++++++------ drivers/s390/crypto/vfio_ap_ops.c | 12 +++++++----- drivers/vfio/mdev/mdev_core.c | 14 ++++++++++++-- drivers/vfio/mdev/mdev_sysfs.c | 11 ++++++----- include/linux/mdev.h | 11 +++++++---- samples/vfio-mdev/mbochs.c | 26 +++++++++++++++----------- samples/vfio-mdev/mdpy.c | 24 ++++++++++++++---------- samples/vfio-mdev/mtty.c | 18 +++++++++--------- 9 files changed, 90 insertions(+), 62 deletions(-) (limited to 'drivers') diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c index 4b47a18e9dfa..3703814a669b 100644 --- a/drivers/gpu/drm/i915/gvt/gvt.c +++ b/drivers/gpu/drm/i915/gvt/gvt.c @@ -54,14 +54,15 @@ intel_gvt_find_vgpu_type(struct intel_gvt *gvt, unsigned int type_group_id) return &gvt->types[type_group_id]; } -static ssize_t available_instances_show(struct kobject *kobj, - struct device *dev, char *buf) +static ssize_t available_instances_show(struct mdev_type *mtype, + struct mdev_type_attribute *attr, + char *buf) { struct intel_vgpu_type *type; unsigned int num = 0; - void *gvt = kdev_to_i915(dev)->gvt; + void *gvt = kdev_to_i915(mtype_get_parent_dev(mtype))->gvt; - type = intel_gvt_find_vgpu_type(gvt, mtype_get_type_group_id(kobj)); + type = intel_gvt_find_vgpu_type(gvt, mtype_get_type_group_id(mtype)); if (!type) num = 0; else @@ -70,19 +71,19 @@ static ssize_t available_instances_show(struct kobject *kobj, return sprintf(buf, "%u\n", num); } -static ssize_t device_api_show(struct kobject *kobj, struct device *dev, - char *buf) +static ssize_t device_api_show(struct mdev_type *mtype, + struct mdev_type_attribute *attr, char *buf) { return sprintf(buf, "%s\n", VFIO_DEVICE_API_PCI_STRING); } -static ssize_t description_show(struct kobject *kobj, struct device *dev, - char *buf) +static ssize_t description_show(struct mdev_type *mtype, + struct mdev_type_attribute *attr, char *buf) { struct intel_vgpu_type *type; - void *gvt = kdev_to_i915(dev)->gvt; + void *gvt = kdev_to_i915(mtype_get_parent_dev(mtype))->gvt; - type = intel_gvt_find_vgpu_type(gvt, mtype_get_type_group_id(kobj)); + type = intel_gvt_find_vgpu_type(gvt, mtype_get_type_group_id(mtype)); if (!type) return 0; diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c index 10407cf67583..491a64c61fff 100644 --- a/drivers/s390/cio/vfio_ccw_ops.c +++ b/drivers/s390/cio/vfio_ccw_ops.c @@ -71,23 +71,26 @@ static int vfio_ccw_mdev_notifier(struct notifier_block *nb, return NOTIFY_DONE; } -static ssize_t name_show(struct kobject *kobj, struct device *dev, char *buf) +static ssize_t name_show(struct mdev_type *mtype, + struct mdev_type_attribute *attr, char *buf) { return sprintf(buf, "I/O subchannel (Non-QDIO)\n"); } static MDEV_TYPE_ATTR_RO(name); -static ssize_t device_api_show(struct kobject *kobj, struct device *dev, - char *buf) +static ssize_t device_api_show(struct mdev_type *mtype, + struct mdev_type_attribute *attr, char *buf) { return sprintf(buf, "%s\n", VFIO_DEVICE_API_CCW_STRING); } static MDEV_TYPE_ATTR_RO(device_api); -static ssize_t available_instances_show(struct kobject *kobj, - struct device *dev, char *buf) +static ssize_t available_instances_show(struct mdev_type *mtype, + struct mdev_type_attribute *attr, + char *buf) { - struct vfio_ccw_private *private = dev_get_drvdata(dev); + struct vfio_ccw_private *private = + dev_get_drvdata(mtype_get_parent_dev(mtype)); return sprintf(buf, "%d\n", atomic_read(&private->avail)); } diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c index d319152dd484..0ce00c9311d3 100644 --- a/drivers/s390/crypto/vfio_ap_ops.c +++ b/drivers/s390/crypto/vfio_ap_ops.c @@ -366,15 +366,17 @@ static int vfio_ap_mdev_remove(struct mdev_device *mdev) return 0; } -static ssize_t name_show(struct kobject *kobj, struct device *dev, char *buf) +static ssize_t name_show(struct mdev_type *mtype, + struct mdev_type_attribute *attr, char *buf) { return sprintf(buf, "%s\n", VFIO_AP_MDEV_NAME_HWVIRT); } static MDEV_TYPE_ATTR_RO(name); -static ssize_t available_instances_show(struct kobject *kobj, - struct device *dev, char *buf) +static ssize_t available_instances_show(struct mdev_type *mtype, + struct mdev_type_attribute *attr, + char *buf) { return sprintf(buf, "%d\n", atomic_read(&matrix_dev->available_instances)); @@ -382,8 +384,8 @@ static ssize_t available_instances_show(struct kobject *kobj, static MDEV_TYPE_ATTR_RO(available_instances); -static ssize_t device_api_show(struct kobject *kobj, struct device *dev, - char *buf) +static ssize_t device_api_show(struct mdev_type *mtype, + struct mdev_type_attribute *attr, char *buf) { return sprintf(buf, "%s\n", VFIO_DEVICE_API_AP_STRING); } diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c index 10eff33ce1f2..2a85d6fcb7dd 100644 --- a/drivers/vfio/mdev/mdev_core.c +++ b/drivers/vfio/mdev/mdev_core.c @@ -47,12 +47,22 @@ EXPORT_SYMBOL(mdev_get_type_group_id); * Used in mdev_type_attribute sysfs functions to return the index in the * supported_type_groups that the sysfs is called from. */ -unsigned int mtype_get_type_group_id(struct kobject *mtype_kobj) +unsigned int mtype_get_type_group_id(struct mdev_type *mtype) { - return container_of(mtype_kobj, struct mdev_type, kobj)->type_group_id; + return mtype->type_group_id; } EXPORT_SYMBOL(mtype_get_type_group_id); +/* + * Used in mdev_type_attribute sysfs functions to return the parent struct + * device + */ +struct device *mtype_get_parent_dev(struct mdev_type *mtype) +{ + return mtype->parent->dev; +} +EXPORT_SYMBOL(mtype_get_parent_dev); + /* Should be called holding parent_list_lock */ static struct mdev_parent *__find_parent_device(struct device *dev) { diff --git a/drivers/vfio/mdev/mdev_sysfs.c b/drivers/vfio/mdev/mdev_sysfs.c index 712fbc78b12e..f5cf1931c54e 100644 --- a/drivers/vfio/mdev/mdev_sysfs.c +++ b/drivers/vfio/mdev/mdev_sysfs.c @@ -26,7 +26,7 @@ static ssize_t mdev_type_attr_show(struct kobject *kobj, ssize_t ret = -EIO; if (attr->show) - ret = attr->show(kobj, type->parent->dev, buf); + ret = attr->show(type, attr, buf); return ret; } @@ -39,7 +39,7 @@ static ssize_t mdev_type_attr_store(struct kobject *kobj, ssize_t ret = -EIO; if (attr->store) - ret = attr->store(&type->kobj, type->parent->dev, buf, count); + ret = attr->store(type, attr, buf, count); return ret; } @@ -48,8 +48,9 @@ static const struct sysfs_ops mdev_type_sysfs_ops = { .store = mdev_type_attr_store, }; -static ssize_t create_store(struct kobject *kobj, struct device *dev, - const char *buf, size_t count) +static ssize_t create_store(struct mdev_type *mtype, + struct mdev_type_attribute *attr, const char *buf, + size_t count) { char *str; guid_t uuid; @@ -67,7 +68,7 @@ static ssize_t create_store(struct kobject *kobj, struct device *dev, if (ret) return ret; - ret = mdev_device_create(to_mdev_type(kobj), &uuid); + ret = mdev_device_create(mtype, &uuid); if (ret) return ret; diff --git a/include/linux/mdev.h b/include/linux/mdev.h index c3a800051d61..1fb34ea394ad 100644 --- a/include/linux/mdev.h +++ b/include/linux/mdev.h @@ -47,7 +47,8 @@ static inline struct device *mdev_get_iommu_device(struct mdev_device *mdev) } unsigned int mdev_get_type_group_id(struct mdev_device *mdev); -unsigned int mtype_get_type_group_id(struct kobject *mtype_kobj); +unsigned int mtype_get_type_group_id(struct mdev_type *mtype); +struct device *mtype_get_parent_dev(struct mdev_type *mtype); /** * struct mdev_parent_ops - Structure to be registered for each parent device to @@ -123,9 +124,11 @@ struct mdev_parent_ops { /* interface for exporting mdev supported type attributes */ struct mdev_type_attribute { struct attribute attr; - ssize_t (*show)(struct kobject *kobj, struct device *dev, char *buf); - ssize_t (*store)(struct kobject *kobj, struct device *dev, - const char *buf, size_t count); + ssize_t (*show)(struct mdev_type *mtype, + struct mdev_type_attribute *attr, char *buf); + ssize_t (*store)(struct mdev_type *mtype, + struct mdev_type_attribute *attr, const char *buf, + size_t count); }; #define MDEV_TYPE_ATTR(_name, _mode, _show, _store) \ diff --git a/samples/vfio-mdev/mbochs.c b/samples/vfio-mdev/mbochs.c index ac4d0dc24907..861c76914e76 100644 --- a/samples/vfio-mdev/mbochs.c +++ b/samples/vfio-mdev/mbochs.c @@ -1330,37 +1330,41 @@ static const struct attribute_group *mdev_dev_groups[] = { NULL, }; -static ssize_t -name_show(struct kobject *kobj, struct device *dev, char *buf) +static ssize_t name_show(struct mdev_type *mtype, + struct mdev_type_attribute *attr, char *buf) { - return sprintf(buf, "%s\n", kobj->name); + const struct mbochs_type *type = + &mbochs_types[mtype_get_type_group_id(mtype)]; + + return sprintf(buf, "%s\n", type->name); } static MDEV_TYPE_ATTR_RO(name); -static ssize_t -description_show(struct kobject *kobj, struct device *dev, char *buf) +static ssize_t description_show(struct mdev_type *mtype, + struct mdev_type_attribute *attr, char *buf) { const struct mbochs_type *type = - &mbochs_types[mtype_get_type_group_id(kobj)]; + &mbochs_types[mtype_get_type_group_id(mtype)]; return sprintf(buf, "virtual display, %d MB video memory\n", type ? type->mbytes : 0); } static MDEV_TYPE_ATTR_RO(description); -static ssize_t -available_instances_show(struct kobject *kobj, struct device *dev, char *buf) +static ssize_t available_instances_show(struct mdev_type *mtype, + struct mdev_type_attribute *attr, + char *buf) { const struct mbochs_type *type = - &mbochs_types[mtype_get_type_group_id(kobj)]; + &mbochs_types[mtype_get_type_group_id(mtype)]; int count = (max_mbytes - mbochs_used_mbytes) / type->mbytes; return sprintf(buf, "%d\n", count); } static MDEV_TYPE_ATTR_RO(available_instances); -static ssize_t device_api_show(struct kobject *kobj, struct device *dev, - char *buf) +static ssize_t device_api_show(struct mdev_type *mtype, + struct mdev_type_attribute *attr, char *buf) { return sprintf(buf, "%s\n", VFIO_DEVICE_API_PCI_STRING); } diff --git a/samples/vfio-mdev/mdpy.c b/samples/vfio-mdev/mdpy.c index da88fd7dd423..885b88ea20e2 100644 --- a/samples/vfio-mdev/mdpy.c +++ b/samples/vfio-mdev/mdpy.c @@ -652,18 +652,21 @@ static const struct attribute_group *mdev_dev_groups[] = { NULL, }; -static ssize_t -name_show(struct kobject *kobj, struct device *dev, char *buf) +static ssize_t name_show(struct mdev_type *mtype, + struct mdev_type_attribute *attr, char *buf) { - return sprintf(buf, "%s\n", kobj->name); + const struct mdpy_type *type = + &mdpy_types[mtype_get_type_group_id(mtype)]; + + return sprintf(buf, "%s\n", type->name); } static MDEV_TYPE_ATTR_RO(name); -static ssize_t -description_show(struct kobject *kobj, struct device *dev, char *buf) +static ssize_t description_show(struct mdev_type *mtype, + struct mdev_type_attribute *attr, char *buf) { const struct mdpy_type *type = - &mdpy_types[mtype_get_type_group_id(kobj)]; + &mdpy_types[mtype_get_type_group_id(mtype)]; return sprintf(buf, "virtual display, %dx%d framebuffer\n", type ? type->width : 0, @@ -671,15 +674,16 @@ description_show(struct kobject *kobj, struct device *dev, char *buf) } static MDEV_TYPE_ATTR_RO(description); -static ssize_t -available_instances_show(struct kobject *kobj, struct device *dev, char *buf) +static ssize_t available_instances_show(struct mdev_type *mtype, + struct mdev_type_attribute *attr, + char *buf) { return sprintf(buf, "%d\n", max_devices - mdpy_count); } static MDEV_TYPE_ATTR_RO(available_instances); -static ssize_t device_api_show(struct kobject *kobj, struct device *dev, - char *buf) +static ssize_t device_api_show(struct mdev_type *mtype, + struct mdev_type_attribute *attr, char *buf) { return sprintf(buf, "%s\n", VFIO_DEVICE_API_PCI_STRING); } diff --git a/samples/vfio-mdev/mtty.c b/samples/vfio-mdev/mtty.c index f2e36c06ac6a..b9b24be4abda 100644 --- a/samples/vfio-mdev/mtty.c +++ b/samples/vfio-mdev/mtty.c @@ -1292,23 +1292,24 @@ static const struct attribute_group *mdev_dev_groups[] = { NULL, }; -static ssize_t -name_show(struct kobject *kobj, struct device *dev, char *buf) +static ssize_t name_show(struct mdev_type *mtype, + struct mdev_type_attribute *attr, char *buf) { static const char *name_str[2] = { "Single port serial", "Dual port serial" }; return sysfs_emit(buf, "%s\n", - name_str[mtype_get_type_group_id(kobj)]); + name_str[mtype_get_type_group_id(mtype)]); } static MDEV_TYPE_ATTR_RO(name); -static ssize_t -available_instances_show(struct kobject *kobj, struct device *dev, char *buf) +static ssize_t available_instances_show(struct mdev_type *mtype, + struct mdev_type_attribute *attr, + char *buf) { struct mdev_state *mds; - unsigned int ports = mtype_get_type_group_id(kobj) + 1; + unsigned int ports = mtype_get_type_group_id(mtype) + 1; int used = 0; list_for_each_entry(mds, &mdev_devices_list, next) @@ -1319,9 +1320,8 @@ available_instances_show(struct kobject *kobj, struct device *dev, char *buf) static MDEV_TYPE_ATTR_RO(available_instances); - -static ssize_t device_api_show(struct kobject *kobj, struct device *dev, - char *buf) +static ssize_t device_api_show(struct mdev_type *mtype, + struct mdev_type_attribute *attr, char *buf) { return sprintf(buf, "%s\n", VFIO_DEVICE_API_PCI_STRING); } -- cgit v1.2.3 From 43dcf6ccf8953a868dfd194ff2fad279eb55883e Mon Sep 17 00:00:00 2001 From: Keqian Zhu Date: Mon, 12 Apr 2021 10:44:15 +0800 Subject: vfio/iommu_type1: Remove unused pinned_page_dirty_scope in vfio_iommu pinned_page_dirty_scope is optimized out by commit 010321565a7d ("vfio/iommu_type1: Mantain a counter for non_pinned_groups"), but appears again due to some issues during merging branches. We can safely remove it here. Signed-off-by: Keqian Zhu Message-Id: <20210412024415.30676-1-zhukeqian1@huawei.com> Signed-off-by: Alex Williamson --- drivers/vfio/vfio_iommu_type1.c | 1 - 1 file changed, 1 deletion(-) (limited to 'drivers') diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c index 53b4a6345a23..07296326d24d 100644 --- a/drivers/vfio/vfio_iommu_type1.c +++ b/drivers/vfio/vfio_iommu_type1.c @@ -77,7 +77,6 @@ struct vfio_iommu { bool v2; bool nesting; bool dirty_page_tracking; - bool pinned_page_dirty_scope; bool container_open; }; -- cgit v1.2.3 From adaeb718d46f6b42a3fc1dffd4f946f26b33779a Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Thu, 22 Apr 2021 15:35:33 +0200 Subject: vfio/gvt: fix DRM_I915_GVT dependency on VFIO_MDEV The Kconfig dependency is incomplete since DRM_I915_GVT is a 'bool' symbol that depends on the 'tristate' VFIO_MDEV. This allows a configuration with VFIO_MDEV=m, DRM_I915_GVT=y and DRM_I915=y that causes a link failure: x86_64-linux-ld: drivers/gpu/drm/i915/gvt/gvt.o: in function `available_instances_show': gvt.c:(.text+0x67a): undefined reference to `mtype_get_parent_dev' x86_64-linux-ld: gvt.c:(.text+0x6a5): undefined reference to `mtype_get_type_group_id' x86_64-linux-ld: drivers/gpu/drm/i915/gvt/gvt.o: in function `description_show': gvt.c:(.text+0x76e): undefined reference to `mtype_get_parent_dev' x86_64-linux-ld: gvt.c:(.text+0x799): undefined reference to `mtype_get_type_group_id' Clarify the dependency by specifically disallowing the broken configuration. If VFIO_MDEV is built-in, it will work, but if VFIO_MDEV=m, the i915 driver cannot be built-in here. Fixes: 07e543f4f9d1 ("vfio/gvt: Make DRM_I915_GVT depend on VFIO_MDEV") Fixes: 9169cff168ff ("vfio/mdev: Correct the function signatures for the mdev_type_attributes") Signed-off-by: Arnd Bergmann Acked-by: Zhenyu Wang Message-Id: <20210422133547.1861063-1-arnd@kernel.org> Reviewed-by: Jason Gunthorpe Signed-off-by: Alex Williamson --- drivers/gpu/drm/i915/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig index 483e9ff8ca1d..369695df8b15 100644 --- a/drivers/gpu/drm/i915/Kconfig +++ b/drivers/gpu/drm/i915/Kconfig @@ -101,7 +101,7 @@ config DRM_I915_GVT bool "Enable Intel GVT-g graphics virtualization host support" depends on DRM_I915 depends on 64BIT - depends on VFIO_MDEV + depends on VFIO_MDEV=y || VFIO_MDEV=DRM_I915 default n help Choose this option if you want to enable Intel GVT-g graphics -- cgit v1.2.3