Re: [PATCH 1/2] vfio/pci: remove vfio_pci_nvlink2
On Thu, Apr 22, 2021 at 11:49:31PM +1000, Michael Ellerman wrote: > Alex Williamson writes: > > On Mon, 12 Apr 2021 19:41:41 +1000 > > Michael Ellerman wrote: > > > >> Alex Williamson writes: > >> > On Fri, 26 Mar 2021 07:13:10 +0100 > >> > Christoph Hellwig wrote: > >> > > >> >> 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 > >> >> 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 > >> > > >> > Hearing no objections, applied to vfio next branch for v5.13. Thanks, > >> > >> Looks like you only took patch 1? > >> > >> I can't take patch 2 on its own, that would break the build. > >> > >> Do you want to take both patches? There's currently no conflicts against > >> my tree. It's possible one could appear before the v5.13 merge window, > >> though it would probably just be something minor. > >> > >> Or I could apply both patches to my tree, which means patch 1 would > >> appear as two commits in the git history, but that's not a big deal. > > > > I've already got a conflict in my next branch with patch 1, so it's > > best to go through my tree. Seems like a shared branch would be > > easiest to allow you to merge and manage potential conflicts against > > patch 2, I've pushed a branch here: > > > > https://github.com/awilliam/linux-vfio.git v5.13/vfio/nvlink > > Thanks. > > My next is based on rc2, so I won't pull that in directly, because I > don't want to pull all of rc6 in with it. Linus is fine if you merge in rc's for development reasons. He doesn't like it when people just merge rc's without a purpose. Merge rc7 to your tree then pull the nvlink topic is acceptable. Or just do nothing because Alex will send it through his tree - this extra co-ordination is really only necessary if there are conflicts. Jason
Re: [PATCH 1/2] vfio/pci: remove vfio_pci_nvlink2
Alex Williamson writes: > On Mon, 12 Apr 2021 19:41:41 +1000 > Michael Ellerman wrote: > >> Alex Williamson writes: >> > On Fri, 26 Mar 2021 07:13:10 +0100 >> > Christoph Hellwig wrote: >> > >> >> 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 >> >> --- >> >> 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 >> > >> > Hearing no objections, applied to vfio next branch for v5.13. Thanks, >> >> Looks like you only took patch 1? >> >> I can't take patch 2 on its own, that would break the build. >> >> Do you want to take both patches? There's currently no conflicts against >> my tree. It's possible one could appear before the v5.13 merge window, >> though it would probably just be something minor. >> >> Or I could apply both patches to my tree, which means patch 1 would >> appear as two commits in the git history, but that's not a big deal. > > I've already got a conflict in my next branch with patch 1, so it's > best to go through my tree. Seems like a shared branch would be > easiest to allow you to merge and manage potential conflicts against > patch 2, I've pushed a branch here: > > https://github.com/awilliam/linux-vfio.git v5.13/vfio/nvlink Thanks. My next is based on rc2, so I won't pull that in directly, because I don't want to pull all of rc6 in with it. I'll put it in a topic branch and merge it into my next after my first pull has gone to Linus. cheers
Re: [PATCH 1/2] vfio/pci: remove vfio_pci_nvlink2
On Mon, 12 Apr 2021 19:41:41 +1000 Michael Ellerman wrote: > Alex Williamson writes: > > On Fri, 26 Mar 2021 07:13:10 +0100 > > Christoph Hellwig wrote: > > > >> 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 > >> --- > >> 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 > > > > Hearing no objections, applied to vfio next branch for v5.13. Thanks, > > Looks like you only took patch 1? > > I can't take patch 2 on its own, that would break the build. > > Do you want to take both patches? There's currently no conflicts against > my tree. It's possible one could appear before the v5.13 merge window, > though it would probably just be something minor. > > Or I could apply both patches to my tree, which means patch 1 would > appear as two commits in the git history, but that's not a big deal. I've already got a conflict in my next branch with patch 1, so it's best to go through my tree. Seems like a shared branch would be easiest to allow you to merge and manage potential conflicts against patch 2, I've pushed a branch here: https://github.com/awilliam/linux-vfio.git v5.13/vfio/nvlink Thanks, Alex
Re: [PATCH 1/2] vfio/pci: remove vfio_pci_nvlink2
Alex Williamson writes: > On Fri, 26 Mar 2021 07:13:10 +0100 > Christoph Hellwig wrote: > >> 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 >> --- >> 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 > > Hearing no objections, applied to vfio next branch for v5.13. Thanks, Looks like you only took patch 1? I can't take patch 2 on its own, that would break the build. Do you want to take both patches? There's currently no conflicts against my tree. It's possible one could appear before the v5.13 merge window, though it would probably just be something minor. Or I could apply both patches to my tree, which means patch 1 would appear as two commits in the git history, but that's not a big deal. cheers
Re: [PATCH 1/2] vfio/pci: remove vfio_pci_nvlink2
On Fri, 26 Mar 2021 07:13:10 +0100 Christoph Hellwig wrote: > 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 > --- > 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 Hearing no objections, applied to vfio next branch for v5.13. Thanks, Alex
[PATCH 1/2] 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
---
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
diff --git a/drivers/vfio/pci/Kconfig b/drivers/vfio/pci/Kconfig
index ac3c1dd3edeff1..53ce78d7d07be0 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
- 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 eff97a7cd9f139..3ff42093962f6f 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 65e7e6b44578c2..d691006b642839 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 9adcf6a8f88857..00
--- 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
Re: [PATCH 1/2] vfio/pci: remove vfio_pci_nvlink2
On Mon, 22 Mar 2021 16:01:54 +0100
Christoph Hellwig wrote:
> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> index 8ce36c1d53ca11..db7e782419d5d9 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -332,19 +332,6 @@ struct vfio_region_info_cap_type {
> #define VFIO_REGION_SUBTYPE_INTEL_IGD_HOST_CFG (2)
> #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)
> -
> -/* 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)
> -
> /* sub-types for VFIO_REGION_TYPE_GFX */
> #define VFIO_REGION_SUBTYPE_GFX_EDID(1)
>
> @@ -637,33 +624,6 @@ 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;
> -};
> -
> -/*
> - * 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;
> -};
> -
> /**
> * VFIO_DEVICE_GET_IRQ_INFO - _IOWR(VFIO_TYPE, VFIO_BASE + 9,
> * struct vfio_irq_info)
I'll leave any attempt to defend keeping this code to Alexey, but
minimally these region sub-types and capability IDs should probably be
reserved to avoid breaking whatever userspace might exist to consume
these. Our ID space is sufficiently large that we don't need to
recycle them any time soon. Thanks,
Alex
[PATCH 1/2] 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
---
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 | 40 ---
6 files changed, 569 deletions(-)
delete mode 100644 drivers/vfio/pci/vfio_pci_nvlink2.c
diff --git a/drivers/vfio/pci/Kconfig b/drivers/vfio/pci/Kconfig
index ac3c1dd3edeff1..53ce78d7d07be0 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
- 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 eff97a7cd9f139..3ff42093962f6f 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 65e7e6b44578c2..d691006b642839 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 9adcf6a8f88857..00
--- 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);
-
