From: Ankit Agrawal <[email protected]>
NVIDIA's upcoming Grace Hopper Superchip provides a PCI-like device
for the on-chip GPU that is the logical OS representation of the
internal proprietary cache coherent interconnect.
This representation has a number of limitations compared to a real PCI
device, in particular, it does not model the coherent GPU memory
aperture as a PCI config space BAR, and PCI doesn't know anything
about cacheable memory types.
Provide a VFIO PCI variant driver that adapts the unique PCI
representation into a more standard PCI representation facing
userspace. The GPU memory aperture is obtained from ACPI using
device_property_read_u64(), according to the FW specification,
and exported to userspace as a separate VFIO_REGION. Since the device
implements only one 64-bit BAR (BAR0), the GPU memory aperture is mapped
to the next available PCI BAR (BAR2). Qemu will then naturally generate a
PCI device in the VM with two 64-bit BARs (where the cacheable aperture
reported in BAR2).
Since this memory region is actually cache coherent with the CPU, the
VFIO variant driver will mmap it into VMA using a cacheable mapping. The
mapping is done using remap_pfn_range().
PCI BAR are aligned to the power-of-2, but the actual memory on the
device may not. A read or write access to the physical address from the
last device PFN up to the next power-of-2 aligned physical address
results in reading ~0 and dropped writes.
Lastly the presence of CPU cache coherent device memory is exposed
through sysfs for use by user space.
This goes along with a qemu series to provides the necessary
implementation of the Grace Hopper Superchip firmware specification so
that the guest operating system can see the correct ACPI modeling for
the coherent GPU device. Verified with the CUDA workload in the VM.
https://lore.kernel.org/all/[email protected]/
This patch is split from a patch series being pursued separately:
https://lore.kernel.org/lkml/[email protected]/
Applied and tested over next-20230911.
Signed-off-by: Ankit Agrawal <[email protected]>
Signed-off-by: Aniket Agashe <[email protected]>
---
Link for v9:
https://lore.kernel.org/all/[email protected]/
v9 -> v10
- Add new sysfs attribute to expose the CPU coherent memory feature.
v8 -> v9
- Minor code adjustment suggested in v8.
v7 -> v8
- Various field names updated.
- Added a new function to handle VFIO_DEVICE_GET_REGION_INFO ioctl.
- Locking protection for memremap to bar region and other changes
recommended in v7.
- Added code to fail if the devmem size advertized is 0 in system DSDT.
v6 -> v7
- Handled out-of-bound and overflow conditions at various places to validate
input offset and length.
- Added code to return EINVAL for offset beyond region size.
v5 -> v6
- Added the code to handle BAR2 read/write using memremap to the device
memory.
v4 -> v5
- Changed the module name from nvgpu-vfio-pci to nvgrace-gpu-vfio-pci.
- Fixed memory leak and added suggested boundary checks on device memory
mapping.
- Added code to read all Fs and ignored write on region outside of the
physical memory.
- Other miscellaneous cleanup suggestions.
v3 -> v4
- Mapping the available device memory using sparse mmap. The region outside
the device memory is handled by read/write ops.
- Removed the fault handler added in v3.
v2 -> v3
- Added fault handler to map the region outside the physical GPU memory
up to the next power-of-2 to a dummy PFN.
- Changed to select instead of "depends on" VFIO_PCI_CORE for all the
vfio-pci variant driver.
- Code cleanup based on feedback comments.
- Code implemented and tested against v6.4-rc4.
v1 -> v2
- Updated the wording of reference to BAR offset and replaced with
index.
- The GPU memory is exposed at the fixed BAR2_REGION_INDEX.
- Code cleanup based on feedback comments.
MAINTAINERS | 6 +
drivers/vfio/pci/Kconfig | 2 +
drivers/vfio/pci/Makefile | 2 +
drivers/vfio/pci/nvgrace-gpu/Kconfig | 10 +
drivers/vfio/pci/nvgrace-gpu/Makefile | 3 +
drivers/vfio/pci/nvgrace-gpu/main.c | 501 ++++++++++++++++++++++++++
6 files changed, 524 insertions(+)
create mode 100644 drivers/vfio/pci/nvgrace-gpu/Kconfig
create mode 100644 drivers/vfio/pci/nvgrace-gpu/Makefile
create mode 100644 drivers/vfio/pci/nvgrace-gpu/main.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 2833e2da63e0..0578b8774d2a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -22638,6 +22638,12 @@ L: [email protected]
S: Maintained
F: drivers/vfio/platform/
+VFIO NVIDIA GRACE GPU DRIVER
+M: Ankit Agrawal <[email protected]>
+L: [email protected]
+S: Maintained
+F: drivers/vfio/pci/nvgrace-gpu/
+
VGA_SWITCHEROO
R: Lukas Wunner <[email protected]>
S: Maintained
diff --git a/drivers/vfio/pci/Kconfig b/drivers/vfio/pci/Kconfig
index 8125e5f37832..2456210e85f1 100644
--- a/drivers/vfio/pci/Kconfig
+++ b/drivers/vfio/pci/Kconfig
@@ -65,4 +65,6 @@ source "drivers/vfio/pci/hisilicon/Kconfig"
source "drivers/vfio/pci/pds/Kconfig"
+source "drivers/vfio/pci/nvgrace-gpu/Kconfig"
+
endmenu
diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile
index 45167be462d8..1352c65e568a 100644
--- a/drivers/vfio/pci/Makefile
+++ b/drivers/vfio/pci/Makefile
@@ -13,3 +13,5 @@ obj-$(CONFIG_MLX5_VFIO_PCI) += mlx5/
obj-$(CONFIG_HISI_ACC_VFIO_PCI) += hisilicon/
obj-$(CONFIG_PDS_VFIO_PCI) += pds/
+
+obj-$(CONFIG_NVGRACE_GPU_VFIO_PCI) += nvgrace-gpu/
diff --git a/drivers/vfio/pci/nvgrace-gpu/Kconfig b/drivers/vfio/pci/nvgrace-gpu/Kconfig
new file mode 100644
index 000000000000..b46f2d97a1d6
--- /dev/null
+++ b/drivers/vfio/pci/nvgrace-gpu/Kconfig
@@ -0,0 +1,10 @@
+# SPDX-License-Identifier: GPL-2.0-only
+config NVGRACE_GPU_VFIO_PCI
+ tristate "VFIO support for the GPU in the NVIDIA Grace Hopper Superchip"
+ depends on ARM64 || (COMPILE_TEST && 64BIT)
+ select VFIO_PCI_CORE
+ help
+ VFIO support for the GPU in the NVIDIA Grace Hopper Superchip is
+ required to assign the GPU device to a VM using KVM/qemu/etc.
+
+ If you don't know what to do here, say N.
diff --git a/drivers/vfio/pci/nvgrace-gpu/Makefile b/drivers/vfio/pci/nvgrace-gpu/Makefile
new file mode 100644
index 000000000000..3ca8c187897a
--- /dev/null
+++ b/drivers/vfio/pci/nvgrace-gpu/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0-only
+obj-$(CONFIG_NVGRACE_GPU_VFIO_PCI) += nvgrace-gpu-vfio-pci.o
+nvgrace-gpu-vfio-pci-y := main.o
diff --git a/drivers/vfio/pci/nvgrace-gpu/main.c b/drivers/vfio/pci/nvgrace-gpu/main.c
new file mode 100644
index 000000000000..2795ac6e77e1
--- /dev/null
+++ b/drivers/vfio/pci/nvgrace-gpu/main.c
@@ -0,0 +1,501 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved
+ */
+
+#include <linux/pci.h>
+#include <linux/vfio_pci_core.h>
+#include <linux/vfio.h>
+
+struct nvgrace_gpu_vfio_pci_core_device {
+ struct vfio_pci_core_device core_device;
+ phys_addr_t memphys;
+ size_t memlength;
+ void *memmap;
+ struct mutex memmap_lock;
+};
+
+static int nvgrace_gpu_vfio_pci_open_device(struct vfio_device *core_vdev)
+{
+ struct vfio_pci_core_device *vdev =
+ container_of(core_vdev, struct vfio_pci_core_device, vdev);
+ struct nvgrace_gpu_vfio_pci_core_device *nvdev = container_of(
+ core_vdev, struct nvgrace_gpu_vfio_pci_core_device, core_device.vdev);
+ int ret;
+
+ ret = vfio_pci_core_enable(vdev);
+ if (ret)
+ return ret;
+
+ vfio_pci_core_finish_enable(vdev);
+
+ mutex_init(&nvdev->memmap_lock);
+
+ return 0;
+}
+
+static void nvgrace_gpu_vfio_pci_close_device(struct vfio_device *core_vdev)
+{
+ struct nvgrace_gpu_vfio_pci_core_device *nvdev = container_of(
+ core_vdev, struct nvgrace_gpu_vfio_pci_core_device, core_device.vdev);
+
+ if (nvdev->memmap) {
+ memunmap(nvdev->memmap);
+ nvdev->memmap = NULL;
+ }
+
+ mutex_destroy(&nvdev->memmap_lock);
+
+ vfio_pci_core_close_device(core_vdev);
+}
+
+static int nvgrace_gpu_vfio_pci_mmap(struct vfio_device *core_vdev,
+ struct vm_area_struct *vma)
+{
+ struct nvgrace_gpu_vfio_pci_core_device *nvdev = container_of(
+ core_vdev, struct nvgrace_gpu_vfio_pci_core_device, core_device.vdev);
+
+ unsigned long start_pfn;
+ unsigned int index;
+ u64 req_len, pgoff, end;
+ int ret = 0;
+
+ index = vma->vm_pgoff >> (VFIO_PCI_OFFSET_SHIFT - PAGE_SHIFT);
+ if (index != VFIO_PCI_BAR2_REGION_INDEX)
+ return vfio_pci_core_mmap(core_vdev, vma);
+
+ /*
+ * Request to mmap the BAR. Map to the CPU accessible memory on the
+ * GPU using the memory information gathered from the system ACPI
+ * tables.
+ */
+ pgoff = vma->vm_pgoff &
+ ((1U << (VFIO_PCI_OFFSET_SHIFT - PAGE_SHIFT)) - 1);
+
+ if (check_sub_overflow(vma->vm_end, vma->vm_start, &req_len) ||
+ check_add_overflow(PHYS_PFN(nvdev->memphys), pgoff, &start_pfn) ||
+ check_add_overflow(PFN_PHYS(pgoff), req_len, &end))
+ return -EOVERFLOW;
+
+ /*
+ * Check that the mapping request does not go beyond available device
+ * memory size
+ */
+ if (end > nvdev->memlength)
+ return -EINVAL;
+
+ /*
+ * Perform a PFN map to the memory and back the device BAR by the
+ * GPU memory.
+ *
+ * The available GPU memory size may not be power-of-2 aligned. Given
+ * that the memory is exposed as a BAR, the mapping request is of the
+ * power-of-2 aligned size. Map only up to the size of the GPU memory.
+ * If the memory access is beyond the actual GPU memory size, it will
+ * be handled by the vfio_device_ops read/write.
+ *
+ * During device reset, the GPU is safely disconnected to the CPU
+ * and access to the BAR will be immediately returned preventing
+ * machine check.
+ */
+ ret = remap_pfn_range(vma, vma->vm_start, start_pfn,
+ req_len, vma->vm_page_prot);
+ if (ret)
+ return ret;
+
+ vma->vm_pgoff = start_pfn;
+
+ return 0;
+}
+
+static long
+nvgrace_gpu_vfio_pci_ioctl_get_region_info(struct vfio_device *core_vdev,
+ unsigned long arg)
+{
+ unsigned long minsz = offsetofend(struct vfio_region_info, offset);
+ struct nvgrace_gpu_vfio_pci_core_device *nvdev = container_of(
+ core_vdev, struct nvgrace_gpu_vfio_pci_core_device, core_device.vdev);
+ struct vfio_region_info info;
+
+ if (copy_from_user(&info, (void __user *)arg, minsz))
+ return -EFAULT;
+
+ if (info.argsz < minsz)
+ return -EINVAL;
+
+ if (info.index == VFIO_PCI_BAR2_REGION_INDEX) {
+ /*
+ * Request to determine the BAR region information. Send the
+ * GPU memory information.
+ */
+ uint32_t size;
+ int ret;
+ struct vfio_region_info_cap_sparse_mmap *sparse;
+ struct vfio_info_cap caps = { .buf = NULL, .size = 0 };
+
+ size = struct_size(sparse, areas, 1);
+
+ /*
+ * Setup for sparse mapping for the device memory. Only the
+ * available device memory on the hardware is shown as a
+ * mappable region.
+ */
+ sparse = kzalloc(size, GFP_KERNEL);
+ if (!sparse)
+ return -ENOMEM;
+
+ sparse->nr_areas = 1;
+ sparse->areas[0].offset = 0;
+ sparse->areas[0].size = nvdev->memlength;
+ sparse->header.id = VFIO_REGION_INFO_CAP_SPARSE_MMAP;
+ sparse->header.version = 1;
+
+ ret = vfio_info_add_capability(&caps, &sparse->header, size);
+ kfree(sparse);
+ if (ret)
+ return ret;
+
+ info.offset = VFIO_PCI_INDEX_TO_OFFSET(info.index);
+ /*
+ * The available GPU memory size may not be power-of-2 aligned.
+ * Given that the memory is exposed as a BAR and may not be
+ * aligned, roundup to the next power-of-2.
+ */
+ info.size = roundup_pow_of_two(nvdev->memlength);
+ info.flags = VFIO_REGION_INFO_FLAG_READ |
+ VFIO_REGION_INFO_FLAG_WRITE |
+ VFIO_REGION_INFO_FLAG_MMAP;
+
+ if (caps.size) {
+ info.flags |= VFIO_REGION_INFO_FLAG_CAPS;
+ if (info.argsz < sizeof(info) + caps.size) {
+ info.argsz = sizeof(info) + caps.size;
+ info.cap_offset = 0;
+ } else {
+ vfio_info_cap_shift(&caps, sizeof(info));
+ if (copy_to_user((void __user *)arg +
+ sizeof(info), caps.buf,
+ caps.size)) {
+ kfree(caps.buf);
+ return -EFAULT;
+ }
+ info.cap_offset = sizeof(info);
+ }
+ kfree(caps.buf);
+ }
+ return copy_to_user((void __user *)arg, &info, minsz) ?
+ -EFAULT : 0;
+ }
+ return vfio_pci_core_ioctl(core_vdev, VFIO_DEVICE_GET_REGION_INFO, arg);
+}
+
+static long nvgrace_gpu_vfio_pci_ioctl(struct vfio_device *core_vdev,
+ unsigned int cmd, unsigned long arg)
+{
+ if (cmd == VFIO_DEVICE_GET_REGION_INFO)
+ return nvgrace_gpu_vfio_pci_ioctl_get_region_info(core_vdev, arg);
+
+ return vfio_pci_core_ioctl(core_vdev, cmd, arg);
+}
+
+/*
+ * Read count bytes from the device memory at an offset. The actual device
+ * memory size (available) may not be a power-of-2. So the driver fakes
+ * the size to a power-of-2 (reported) when exposing to a user space driver.
+ *
+ * Read request beyond the actual device size is filled with ~0, while
+ * those beyond the actual reported size is skipped.
+ *
+ * A read from a negative or a reported+ offset, a negative count are
+ * considered error conditions and returned with an -EINVAL.
+ */
+ssize_t nvgrace_gpu_read_mem(void __user *buf, size_t count, loff_t *ppos,
+ struct nvgrace_gpu_vfio_pci_core_device *nvdev)
+{
+ u64 offset = *ppos & VFIO_PCI_OFFSET_MASK;
+ size_t mem_count, i, bar_size = roundup_pow_of_two(nvdev->memlength);
+ u8 val = 0xFF;
+
+ if (offset >= bar_size)
+ return -EINVAL;
+
+ /* Clip short the read request beyond reported BAR size */
+ count = min(count, bar_size - (size_t)offset);
+
+ /*
+ * Determine how many bytes to be actually read from the device memory.
+ * Do not read from the offset beyond available size.
+ */
+ if (offset >= nvdev->memlength)
+ return 0;
+
+ mem_count = min(count, nvdev->memlength - (size_t)offset);
+
+ /*
+ * Handle read on the BAR2 region. Map to the target device memory
+ * physical address and copy to the request read buffer.
+ */
+ if (copy_to_user(buf, (u8 *)nvdev->memmap + offset, mem_count))
+ return -EFAULT;
+
+ /*
+ * Only the device memory present on the hardware is mapped, which may
+ * not be power-of-2 aligned. A read to the BAR2 region implies an
+ * access outside the available device memory on the hardware. Fill
+ * such read request with ~0.
+ */
+ for (i = mem_count; i < count; i++)
+ put_user(val, (unsigned char __user *)(buf + i));
+
+ return count;
+}
+
+static ssize_t nvgrace_gpu_vfio_pci_read(struct vfio_device *core_vdev,
+ char __user *buf, size_t count, loff_t *ppos)
+{
+ unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos);
+ struct nvgrace_gpu_vfio_pci_core_device *nvdev = container_of(
+ core_vdev, struct nvgrace_gpu_vfio_pci_core_device, core_device.vdev);
+
+ if (index == VFIO_PCI_BAR2_REGION_INDEX) {
+ mutex_lock(&nvdev->memmap_lock);
+ if (!nvdev->memmap) {
+ nvdev->memmap = memremap(nvdev->memphys, nvdev->memlength, MEMREMAP_WB);
+ if (!nvdev->memmap) {
+ mutex_unlock(&nvdev->memmap_lock);
+ return -ENOMEM;
+ }
+ }
+ mutex_unlock(&nvdev->memmap_lock);
+
+ return nvgrace_gpu_read_mem(buf, count, ppos, nvdev);
+ }
+
+ return vfio_pci_core_read(core_vdev, buf, count, ppos);
+}
+
+/*
+ * Write count bytes to the device memory at a given offset. The actual device
+ * memory size (available) may not be a power-of-2. So the driver fakes the
+ * size to a power-of-2 (reported) when exposing to a user space driver.
+ *
+ * Write request beyond the actual device size are dropped, while those
+ * beyond the actual reported size are skipped entirely.
+ *
+ * A write to a negative or a reported+ offset, a negative count are
+ * considered error conditions and returned with an -EINVAL.
+ */
+ssize_t nvgrace_gpu_write_mem(size_t count, loff_t *ppos, const void __user *buf,
+ struct nvgrace_gpu_vfio_pci_core_device *nvdev)
+{
+ u64 offset = *ppos & VFIO_PCI_OFFSET_MASK;
+ size_t mem_count, bar_size = roundup_pow_of_two(nvdev->memlength);
+
+ if (offset >= bar_size)
+ return -EINVAL;
+
+ /* Clip short the read request beyond reported BAR size */
+ count = min(count, bar_size - (size_t)offset);
+
+ /*
+ * Determine how many bytes to be actually written to the device memory.
+ * Do not write to the offset beyond available size.
+ */
+ if (offset >= nvdev->memlength)
+ return 0;
+
+ mem_count = min(count, nvdev->memlength - (size_t)offset);
+
+ /*
+ * Only the device memory present on the hardware is mapped, which may
+ * not be power-of-2 aligned. A write to the BAR2 region implies an
+ * access outside the available device memory on the hardware. Drop
+ * those write requests.
+ */
+ if (copy_from_user((u8 *)nvdev->memmap + offset, buf, mem_count))
+ return -EFAULT;
+
+ return count;
+}
+
+static ssize_t nvgrace_gpu_vfio_pci_write(struct vfio_device *core_vdev,
+ const char __user *buf, size_t count, loff_t *ppos)
+{
+ unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos);
+ struct nvgrace_gpu_vfio_pci_core_device *nvdev = container_of(
+ core_vdev, struct nvgrace_gpu_vfio_pci_core_device, core_device.vdev);
+
+ if (index == VFIO_PCI_BAR2_REGION_INDEX) {
+ mutex_lock(&nvdev->memmap_lock);
+ if (!nvdev->memmap) {
+ nvdev->memmap = memremap(nvdev->memphys, nvdev->memlength, MEMREMAP_WB);
+ if (!nvdev->memmap) {
+ mutex_unlock(&nvdev->memmap_lock);
+ return -ENOMEM;
+ }
+ }
+ mutex_unlock(&nvdev->memmap_lock);
+
+ return nvgrace_gpu_write_mem(count, ppos, buf, nvdev);
+ }
+
+ return vfio_pci_core_write(core_vdev, buf, count, ppos);
+}
+
+static const struct vfio_device_ops nvgrace_gpu_vfio_pci_ops = {
+ .name = "nvgrace-gpu-vfio-pci",
+ .init = vfio_pci_core_init_dev,
+ .release = vfio_pci_core_release_dev,
+ .open_device = nvgrace_gpu_vfio_pci_open_device,
+ .close_device = nvgrace_gpu_vfio_pci_close_device,
+ .ioctl = nvgrace_gpu_vfio_pci_ioctl,
+ .read = nvgrace_gpu_vfio_pci_read,
+ .write = nvgrace_gpu_vfio_pci_write,
+ .mmap = nvgrace_gpu_vfio_pci_mmap,
+ .request = vfio_pci_core_request,
+ .match = vfio_pci_core_match,
+ .bind_iommufd = vfio_iommufd_physical_bind,
+ .unbind_iommufd = vfio_iommufd_physical_unbind,
+ .attach_ioas = vfio_iommufd_physical_attach_ioas,
+};
+
+static struct
+nvgrace_gpu_vfio_pci_core_device *nvgrace_gpu_drvdata(struct pci_dev *pdev)
+{
+ struct vfio_pci_core_device *core_device = dev_get_drvdata(&pdev->dev);
+
+ return container_of(core_device, struct nvgrace_gpu_vfio_pci_core_device,
+ core_device);
+}
+
+static int
+nvgrace_gpu_vfio_pci_fetch_memory_property(struct pci_dev *pdev,
+ struct nvgrace_gpu_vfio_pci_core_device *nvdev)
+{
+ int ret;
+ u64 memphys, memlength;
+
+ /*
+ * The memory information is present in the system ACPI tables as DSD
+ * properties nvidia,gpu-mem-base-pa and nvidia,gpu-mem-size.
+ */
+ ret = device_property_read_u64(&pdev->dev, "nvidia,gpu-mem-base-pa",
+ &(memphys));
+ if (ret)
+ return ret;
+
+ if (memphys > type_max(phys_addr_t))
+ return -EOVERFLOW;
+
+ nvdev->memphys = memphys;
+
+ ret = device_property_read_u64(&pdev->dev, "nvidia,gpu-mem-size",
+ &(memlength));
+ if (ret)
+ return ret;
+
+ if (memlength > type_max(size_t))
+ return -EOVERFLOW;
+
+ /*
+ * If the C2C link is not up due to an error, the coherent device
+ * memory size is returned as 0. Fail in such case.
+ */
+ if (memlength == 0)
+ return -ENOMEM;
+
+ nvdev->memlength = memlength;
+
+ return ret;
+}
+
+static int nvgrace_gpu_vfio_pci_probe(struct pci_dev *pdev,
+ const struct pci_device_id *id)
+{
+ struct nvgrace_gpu_vfio_pci_core_device *nvdev;
+ int ret;
+
+ nvdev = vfio_alloc_device(nvgrace_gpu_vfio_pci_core_device, core_device.vdev,
+ &pdev->dev, &nvgrace_gpu_vfio_pci_ops);
+ if (IS_ERR(nvdev))
+ return PTR_ERR(nvdev);
+
+ dev_set_drvdata(&pdev->dev, nvdev);
+
+ ret = nvgrace_gpu_vfio_pci_fetch_memory_property(pdev, nvdev);
+ if (ret)
+ goto out_put_vdev;
+
+ ret = vfio_pci_core_register_device(&nvdev->core_device);
+ if (ret)
+ goto out_put_vdev;
+
+ return ret;
+
+out_put_vdev:
+ vfio_put_device(&nvdev->core_device.vdev);
+ return ret;
+}
+
+static void nvgrace_gpu_vfio_pci_remove(struct pci_dev *pdev)
+{
+ struct nvgrace_gpu_vfio_pci_core_device *nvdev = nvgrace_gpu_drvdata(pdev);
+ struct vfio_pci_core_device *vdev = &nvdev->core_device;
+
+ vfio_pci_core_unregister_device(vdev);
+ vfio_put_device(&vdev->vdev);
+}
+
+static const struct pci_device_id nvgrace_gpu_vfio_pci_table[] = {
+ /* GH200 120GB */
+ { PCI_DRIVER_OVERRIDE_DEVICE_VFIO(PCI_VENDOR_ID_NVIDIA, 0x2342) },
+ /* GH200 480GB */
+ { PCI_DRIVER_OVERRIDE_DEVICE_VFIO(PCI_VENDOR_ID_NVIDIA, 0x2345) },
+ {}
+};
+
+MODULE_DEVICE_TABLE(pci, nvgrace_gpu_vfio_pci_table);
+
+static ssize_t coherent_mem_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct vfio_pci_core_device *core_device = dev_get_drvdata(dev);
+ struct nvgrace_gpu_vfio_pci_core_device *nvdev
+ = container_of(core_device, struct nvgrace_gpu_vfio_pci_core_device,
+ core_device);
+
+ return sprintf(buf, "%u\n", nvdev->memlength ? 1 : 0);
+}
+static DEVICE_ATTR_RO(coherent_mem);
+
+static struct attribute *nvgrace_gpu_vfio_dev_attributes[] = {
+ &dev_attr_coherent_mem.attr,
+ NULL,
+};
+
+static const struct attribute_group nvgrace_gpu_vfio_dev_attribute_group = {
+ .attrs = nvgrace_gpu_vfio_dev_attributes,
+};
+
+static const struct attribute_group *nvgrace_gpu_vfio_dev_attribute_groups[] = {
+ &nvgrace_gpu_vfio_dev_attribute_group,
+ NULL,
+};
+
+static struct pci_driver nvgrace_gpu_vfio_pci_driver = {
+ .name = KBUILD_MODNAME,
+ .id_table = nvgrace_gpu_vfio_pci_table,
+ .probe = nvgrace_gpu_vfio_pci_probe,
+ .remove = nvgrace_gpu_vfio_pci_remove,
+ .err_handler = &vfio_pci_core_err_handlers,
+ .driver_managed_dma = true,
+ .dev_groups = nvgrace_gpu_vfio_dev_attribute_groups,
+};
+
+module_pci_driver(nvgrace_gpu_vfio_pci_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Ankit Agrawal <[email protected]>");
+MODULE_AUTHOR("Aniket Agashe <[email protected]>");
+MODULE_DESCRIPTION(
+ "VFIO NVGRACE GPU PF - User Level driver for NVIDIA devices with CPU coherently accessible device memory");
--
2.17.1
On Thu, 14 Sep 2023 19:54:15 -0700
<[email protected]> wrote:
> From: Ankit Agrawal <[email protected]>
>
> NVIDIA's upcoming Grace Hopper Superchip provides a PCI-like device
> for the on-chip GPU that is the logical OS representation of the
> internal proprietary cache coherent interconnect.
>
> This representation has a number of limitations compared to a real PCI
> device, in particular, it does not model the coherent GPU memory
> aperture as a PCI config space BAR, and PCI doesn't know anything
> about cacheable memory types.
>
> Provide a VFIO PCI variant driver that adapts the unique PCI
> representation into a more standard PCI representation facing
> userspace. The GPU memory aperture is obtained from ACPI using
> device_property_read_u64(), according to the FW specification,
> and exported to userspace as a separate VFIO_REGION. Since the device
> implements only one 64-bit BAR (BAR0), the GPU memory aperture is mapped
> to the next available PCI BAR (BAR2). Qemu will then naturally generate a
> PCI device in the VM with two 64-bit BARs (where the cacheable aperture
> reported in BAR2).
>
> Since this memory region is actually cache coherent with the CPU, the
> VFIO variant driver will mmap it into VMA using a cacheable mapping. The
> mapping is done using remap_pfn_range().
>
> PCI BAR are aligned to the power-of-2, but the actual memory on the
> device may not. A read or write access to the physical address from the
> last device PFN up to the next power-of-2 aligned physical address
> results in reading ~0 and dropped writes.
>
> Lastly the presence of CPU cache coherent device memory is exposed
> through sysfs for use by user space.
This looks like a giant red flag that this approach of masquerading the
coherent memory as a PCI BAR is the wrong way to go. If the VMM needs
to know about this coherent memory, it needs to get that information
in-band. VMMs like QEMU operate in a controlled sandbox and should not
be reaching out to arbitrary sysfs attributes. Minimally this
information should be provided via a capability on the region info
chain, but at that point I again need to ask, why isn't this a device
specific region?
It's still possible to expose the coherent memory to the guest as a PCI
BAR if it's exposed to through the vfio API as a device specific
region, but it avoids the kernel driver making the policy decision
that it must be exposed as a BAR and also avoids needing to provide
separate meta data via other channels regarding this region.
A "coherent_mem" attribute on the device provides a very weak
association to the memory region it's trying to describe. Whereas a
device specific region directly describes both the nature and actual
size of the coherent region. Thanks,
Alex
> This goes along with a qemu series to provides the necessary
> implementation of the Grace Hopper Superchip firmware specification so
> that the guest operating system can see the correct ACPI modeling for
> the coherent GPU device. Verified with the CUDA workload in the VM.
> https://lore.kernel.org/all/[email protected]/
>
> This patch is split from a patch series being pursued separately:
> https://lore.kernel.org/lkml/[email protected]/
>
> Applied and tested over next-20230911.
>
> Signed-off-by: Ankit Agrawal <[email protected]>
> Signed-off-by: Aniket Agashe <[email protected]>
> ---
>
> Link for v9:
> https://lore.kernel.org/all/[email protected]/
>
> v9 -> v10
> - Add new sysfs attribute to expose the CPU coherent memory feature.
>
> v8 -> v9
> - Minor code adjustment suggested in v8.
>
> v7 -> v8
> - Various field names updated.
> - Added a new function to handle VFIO_DEVICE_GET_REGION_INFO ioctl.
> - Locking protection for memremap to bar region and other changes
> recommended in v7.
> - Added code to fail if the devmem size advertized is 0 in system DSDT.
>
> v6 -> v7
> - Handled out-of-bound and overflow conditions at various places to validate
> input offset and length.
> - Added code to return EINVAL for offset beyond region size.
>
> v5 -> v6
> - Added the code to handle BAR2 read/write using memremap to the device
> memory.
>
> v4 -> v5
> - Changed the module name from nvgpu-vfio-pci to nvgrace-gpu-vfio-pci.
> - Fixed memory leak and added suggested boundary checks on device memory
> mapping.
> - Added code to read all Fs and ignored write on region outside of the
> physical memory.
> - Other miscellaneous cleanup suggestions.
>
> v3 -> v4
> - Mapping the available device memory using sparse mmap. The region outside
> the device memory is handled by read/write ops.
> - Removed the fault handler added in v3.
>
> v2 -> v3
> - Added fault handler to map the region outside the physical GPU memory
> up to the next power-of-2 to a dummy PFN.
> - Changed to select instead of "depends on" VFIO_PCI_CORE for all the
> vfio-pci variant driver.
> - Code cleanup based on feedback comments.
> - Code implemented and tested against v6.4-rc4.
>
> v1 -> v2
> - Updated the wording of reference to BAR offset and replaced with
> index.
> - The GPU memory is exposed at the fixed BAR2_REGION_INDEX.
> - Code cleanup based on feedback comments.
>
> MAINTAINERS | 6 +
> drivers/vfio/pci/Kconfig | 2 +
> drivers/vfio/pci/Makefile | 2 +
> drivers/vfio/pci/nvgrace-gpu/Kconfig | 10 +
> drivers/vfio/pci/nvgrace-gpu/Makefile | 3 +
> drivers/vfio/pci/nvgrace-gpu/main.c | 501 ++++++++++++++++++++++++++
> 6 files changed, 524 insertions(+)
> create mode 100644 drivers/vfio/pci/nvgrace-gpu/Kconfig
> create mode 100644 drivers/vfio/pci/nvgrace-gpu/Makefile
> create mode 100644 drivers/vfio/pci/nvgrace-gpu/main.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2833e2da63e0..0578b8774d2a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -22638,6 +22638,12 @@ L: [email protected]
> S: Maintained
> F: drivers/vfio/platform/
>
> +VFIO NVIDIA GRACE GPU DRIVER
> +M: Ankit Agrawal <[email protected]>
> +L: [email protected]
> +S: Maintained
> +F: drivers/vfio/pci/nvgrace-gpu/
> +
> VGA_SWITCHEROO
> R: Lukas Wunner <[email protected]>
> S: Maintained
> diff --git a/drivers/vfio/pci/Kconfig b/drivers/vfio/pci/Kconfig
> index 8125e5f37832..2456210e85f1 100644
> --- a/drivers/vfio/pci/Kconfig
> +++ b/drivers/vfio/pci/Kconfig
> @@ -65,4 +65,6 @@ source "drivers/vfio/pci/hisilicon/Kconfig"
>
> source "drivers/vfio/pci/pds/Kconfig"
>
> +source "drivers/vfio/pci/nvgrace-gpu/Kconfig"
> +
> endmenu
> diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile
> index 45167be462d8..1352c65e568a 100644
> --- a/drivers/vfio/pci/Makefile
> +++ b/drivers/vfio/pci/Makefile
> @@ -13,3 +13,5 @@ obj-$(CONFIG_MLX5_VFIO_PCI) += mlx5/
> obj-$(CONFIG_HISI_ACC_VFIO_PCI) += hisilicon/
>
> obj-$(CONFIG_PDS_VFIO_PCI) += pds/
> +
> +obj-$(CONFIG_NVGRACE_GPU_VFIO_PCI) += nvgrace-gpu/
> diff --git a/drivers/vfio/pci/nvgrace-gpu/Kconfig b/drivers/vfio/pci/nvgrace-gpu/Kconfig
> new file mode 100644
> index 000000000000..b46f2d97a1d6
> --- /dev/null
> +++ b/drivers/vfio/pci/nvgrace-gpu/Kconfig
> @@ -0,0 +1,10 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +config NVGRACE_GPU_VFIO_PCI
> + tristate "VFIO support for the GPU in the NVIDIA Grace Hopper Superchip"
> + depends on ARM64 || (COMPILE_TEST && 64BIT)
> + select VFIO_PCI_CORE
> + help
> + VFIO support for the GPU in the NVIDIA Grace Hopper Superchip is
> + required to assign the GPU device to a VM using KVM/qemu/etc.
> +
> + If you don't know what to do here, say N.
> diff --git a/drivers/vfio/pci/nvgrace-gpu/Makefile b/drivers/vfio/pci/nvgrace-gpu/Makefile
> new file mode 100644
> index 000000000000..3ca8c187897a
> --- /dev/null
> +++ b/drivers/vfio/pci/nvgrace-gpu/Makefile
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +obj-$(CONFIG_NVGRACE_GPU_VFIO_PCI) += nvgrace-gpu-vfio-pci.o
> +nvgrace-gpu-vfio-pci-y := main.o
> diff --git a/drivers/vfio/pci/nvgrace-gpu/main.c b/drivers/vfio/pci/nvgrace-gpu/main.c
> new file mode 100644
> index 000000000000..2795ac6e77e1
> --- /dev/null
> +++ b/drivers/vfio/pci/nvgrace-gpu/main.c
> @@ -0,0 +1,501 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved
> + */
> +
> +#include <linux/pci.h>
> +#include <linux/vfio_pci_core.h>
> +#include <linux/vfio.h>
> +
> +struct nvgrace_gpu_vfio_pci_core_device {
> + struct vfio_pci_core_device core_device;
> + phys_addr_t memphys;
> + size_t memlength;
> + void *memmap;
> + struct mutex memmap_lock;
> +};
> +
> +static int nvgrace_gpu_vfio_pci_open_device(struct vfio_device *core_vdev)
> +{
> + struct vfio_pci_core_device *vdev =
> + container_of(core_vdev, struct vfio_pci_core_device, vdev);
> + struct nvgrace_gpu_vfio_pci_core_device *nvdev = container_of(
> + core_vdev, struct nvgrace_gpu_vfio_pci_core_device, core_device.vdev);
> + int ret;
> +
> + ret = vfio_pci_core_enable(vdev);
> + if (ret)
> + return ret;
> +
> + vfio_pci_core_finish_enable(vdev);
> +
> + mutex_init(&nvdev->memmap_lock);
> +
> + return 0;
> +}
> +
> +static void nvgrace_gpu_vfio_pci_close_device(struct vfio_device *core_vdev)
> +{
> + struct nvgrace_gpu_vfio_pci_core_device *nvdev = container_of(
> + core_vdev, struct nvgrace_gpu_vfio_pci_core_device, core_device.vdev);
> +
> + if (nvdev->memmap) {
> + memunmap(nvdev->memmap);
> + nvdev->memmap = NULL;
> + }
> +
> + mutex_destroy(&nvdev->memmap_lock);
> +
> + vfio_pci_core_close_device(core_vdev);
> +}
> +
> +static int nvgrace_gpu_vfio_pci_mmap(struct vfio_device *core_vdev,
> + struct vm_area_struct *vma)
> +{
> + struct nvgrace_gpu_vfio_pci_core_device *nvdev = container_of(
> + core_vdev, struct nvgrace_gpu_vfio_pci_core_device, core_device.vdev);
> +
> + unsigned long start_pfn;
> + unsigned int index;
> + u64 req_len, pgoff, end;
> + int ret = 0;
> +
> + index = vma->vm_pgoff >> (VFIO_PCI_OFFSET_SHIFT - PAGE_SHIFT);
> + if (index != VFIO_PCI_BAR2_REGION_INDEX)
> + return vfio_pci_core_mmap(core_vdev, vma);
> +
> + /*
> + * Request to mmap the BAR. Map to the CPU accessible memory on the
> + * GPU using the memory information gathered from the system ACPI
> + * tables.
> + */
> + pgoff = vma->vm_pgoff &
> + ((1U << (VFIO_PCI_OFFSET_SHIFT - PAGE_SHIFT)) - 1);
> +
> + if (check_sub_overflow(vma->vm_end, vma->vm_start, &req_len) ||
> + check_add_overflow(PHYS_PFN(nvdev->memphys), pgoff, &start_pfn) ||
> + check_add_overflow(PFN_PHYS(pgoff), req_len, &end))
> + return -EOVERFLOW;
> +
> + /*
> + * Check that the mapping request does not go beyond available device
> + * memory size
> + */
> + if (end > nvdev->memlength)
> + return -EINVAL;
> +
> + /*
> + * Perform a PFN map to the memory and back the device BAR by the
> + * GPU memory.
> + *
> + * The available GPU memory size may not be power-of-2 aligned. Given
> + * that the memory is exposed as a BAR, the mapping request is of the
> + * power-of-2 aligned size. Map only up to the size of the GPU memory.
> + * If the memory access is beyond the actual GPU memory size, it will
> + * be handled by the vfio_device_ops read/write.
> + *
> + * During device reset, the GPU is safely disconnected to the CPU
> + * and access to the BAR will be immediately returned preventing
> + * machine check.
> + */
> + ret = remap_pfn_range(vma, vma->vm_start, start_pfn,
> + req_len, vma->vm_page_prot);
> + if (ret)
> + return ret;
> +
> + vma->vm_pgoff = start_pfn;
> +
> + return 0;
> +}
> +
> +static long
> +nvgrace_gpu_vfio_pci_ioctl_get_region_info(struct vfio_device *core_vdev,
> + unsigned long arg)
> +{
> + unsigned long minsz = offsetofend(struct vfio_region_info, offset);
> + struct nvgrace_gpu_vfio_pci_core_device *nvdev = container_of(
> + core_vdev, struct nvgrace_gpu_vfio_pci_core_device, core_device.vdev);
> + struct vfio_region_info info;
> +
> + if (copy_from_user(&info, (void __user *)arg, minsz))
> + return -EFAULT;
> +
> + if (info.argsz < minsz)
> + return -EINVAL;
> +
> + if (info.index == VFIO_PCI_BAR2_REGION_INDEX) {
> + /*
> + * Request to determine the BAR region information. Send the
> + * GPU memory information.
> + */
> + uint32_t size;
> + int ret;
> + struct vfio_region_info_cap_sparse_mmap *sparse;
> + struct vfio_info_cap caps = { .buf = NULL, .size = 0 };
> +
> + size = struct_size(sparse, areas, 1);
> +
> + /*
> + * Setup for sparse mapping for the device memory. Only the
> + * available device memory on the hardware is shown as a
> + * mappable region.
> + */
> + sparse = kzalloc(size, GFP_KERNEL);
> + if (!sparse)
> + return -ENOMEM;
> +
> + sparse->nr_areas = 1;
> + sparse->areas[0].offset = 0;
> + sparse->areas[0].size = nvdev->memlength;
> + sparse->header.id = VFIO_REGION_INFO_CAP_SPARSE_MMAP;
> + sparse->header.version = 1;
> +
> + ret = vfio_info_add_capability(&caps, &sparse->header, size);
> + kfree(sparse);
> + if (ret)
> + return ret;
> +
> + info.offset = VFIO_PCI_INDEX_TO_OFFSET(info.index);
> + /*
> + * The available GPU memory size may not be power-of-2 aligned.
> + * Given that the memory is exposed as a BAR and may not be
> + * aligned, roundup to the next power-of-2.
> + */
> + info.size = roundup_pow_of_two(nvdev->memlength);
> + info.flags = VFIO_REGION_INFO_FLAG_READ |
> + VFIO_REGION_INFO_FLAG_WRITE |
> + VFIO_REGION_INFO_FLAG_MMAP;
> +
> + if (caps.size) {
> + info.flags |= VFIO_REGION_INFO_FLAG_CAPS;
> + if (info.argsz < sizeof(info) + caps.size) {
> + info.argsz = sizeof(info) + caps.size;
> + info.cap_offset = 0;
> + } else {
> + vfio_info_cap_shift(&caps, sizeof(info));
> + if (copy_to_user((void __user *)arg +
> + sizeof(info), caps.buf,
> + caps.size)) {
> + kfree(caps.buf);
> + return -EFAULT;
> + }
> + info.cap_offset = sizeof(info);
> + }
> + kfree(caps.buf);
> + }
> + return copy_to_user((void __user *)arg, &info, minsz) ?
> + -EFAULT : 0;
> + }
> + return vfio_pci_core_ioctl(core_vdev, VFIO_DEVICE_GET_REGION_INFO, arg);
> +}
> +
> +static long nvgrace_gpu_vfio_pci_ioctl(struct vfio_device *core_vdev,
> + unsigned int cmd, unsigned long arg)
> +{
> + if (cmd == VFIO_DEVICE_GET_REGION_INFO)
> + return nvgrace_gpu_vfio_pci_ioctl_get_region_info(core_vdev, arg);
> +
> + return vfio_pci_core_ioctl(core_vdev, cmd, arg);
> +}
> +
> +/*
> + * Read count bytes from the device memory at an offset. The actual device
> + * memory size (available) may not be a power-of-2. So the driver fakes
> + * the size to a power-of-2 (reported) when exposing to a user space driver.
> + *
> + * Read request beyond the actual device size is filled with ~0, while
> + * those beyond the actual reported size is skipped.
> + *
> + * A read from a negative or a reported+ offset, a negative count are
> + * considered error conditions and returned with an -EINVAL.
> + */
> +ssize_t nvgrace_gpu_read_mem(void __user *buf, size_t count, loff_t *ppos,
> + struct nvgrace_gpu_vfio_pci_core_device *nvdev)
> +{
> + u64 offset = *ppos & VFIO_PCI_OFFSET_MASK;
> + size_t mem_count, i, bar_size = roundup_pow_of_two(nvdev->memlength);
> + u8 val = 0xFF;
> +
> + if (offset >= bar_size)
> + return -EINVAL;
> +
> + /* Clip short the read request beyond reported BAR size */
> + count = min(count, bar_size - (size_t)offset);
> +
> + /*
> + * Determine how many bytes to be actually read from the device memory.
> + * Do not read from the offset beyond available size.
> + */
> + if (offset >= nvdev->memlength)
> + return 0;
> +
> + mem_count = min(count, nvdev->memlength - (size_t)offset);
> +
> + /*
> + * Handle read on the BAR2 region. Map to the target device memory
> + * physical address and copy to the request read buffer.
> + */
> + if (copy_to_user(buf, (u8 *)nvdev->memmap + offset, mem_count))
> + return -EFAULT;
> +
> + /*
> + * Only the device memory present on the hardware is mapped, which may
> + * not be power-of-2 aligned. A read to the BAR2 region implies an
> + * access outside the available device memory on the hardware. Fill
> + * such read request with ~0.
> + */
> + for (i = mem_count; i < count; i++)
> + put_user(val, (unsigned char __user *)(buf + i));
> +
> + return count;
> +}
> +
> +static ssize_t nvgrace_gpu_vfio_pci_read(struct vfio_device *core_vdev,
> + char __user *buf, size_t count, loff_t *ppos)
> +{
> + unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos);
> + struct nvgrace_gpu_vfio_pci_core_device *nvdev = container_of(
> + core_vdev, struct nvgrace_gpu_vfio_pci_core_device, core_device.vdev);
> +
> + if (index == VFIO_PCI_BAR2_REGION_INDEX) {
> + mutex_lock(&nvdev->memmap_lock);
> + if (!nvdev->memmap) {
> + nvdev->memmap = memremap(nvdev->memphys, nvdev->memlength, MEMREMAP_WB);
> + if (!nvdev->memmap) {
> + mutex_unlock(&nvdev->memmap_lock);
> + return -ENOMEM;
> + }
> + }
> + mutex_unlock(&nvdev->memmap_lock);
> +
> + return nvgrace_gpu_read_mem(buf, count, ppos, nvdev);
> + }
> +
> + return vfio_pci_core_read(core_vdev, buf, count, ppos);
> +}
> +
> +/*
> + * Write count bytes to the device memory at a given offset. The actual device
> + * memory size (available) may not be a power-of-2. So the driver fakes the
> + * size to a power-of-2 (reported) when exposing to a user space driver.
> + *
> + * Write request beyond the actual device size are dropped, while those
> + * beyond the actual reported size are skipped entirely.
> + *
> + * A write to a negative or a reported+ offset, a negative count are
> + * considered error conditions and returned with an -EINVAL.
> + */
> +ssize_t nvgrace_gpu_write_mem(size_t count, loff_t *ppos, const void __user *buf,
> + struct nvgrace_gpu_vfio_pci_core_device *nvdev)
> +{
> + u64 offset = *ppos & VFIO_PCI_OFFSET_MASK;
> + size_t mem_count, bar_size = roundup_pow_of_two(nvdev->memlength);
> +
> + if (offset >= bar_size)
> + return -EINVAL;
> +
> + /* Clip short the read request beyond reported BAR size */
> + count = min(count, bar_size - (size_t)offset);
> +
> + /*
> + * Determine how many bytes to be actually written to the device memory.
> + * Do not write to the offset beyond available size.
> + */
> + if (offset >= nvdev->memlength)
> + return 0;
> +
> + mem_count = min(count, nvdev->memlength - (size_t)offset);
> +
> + /*
> + * Only the device memory present on the hardware is mapped, which may
> + * not be power-of-2 aligned. A write to the BAR2 region implies an
> + * access outside the available device memory on the hardware. Drop
> + * those write requests.
> + */
> + if (copy_from_user((u8 *)nvdev->memmap + offset, buf, mem_count))
> + return -EFAULT;
> +
> + return count;
> +}
> +
> +static ssize_t nvgrace_gpu_vfio_pci_write(struct vfio_device *core_vdev,
> + const char __user *buf, size_t count, loff_t *ppos)
> +{
> + unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos);
> + struct nvgrace_gpu_vfio_pci_core_device *nvdev = container_of(
> + core_vdev, struct nvgrace_gpu_vfio_pci_core_device, core_device.vdev);
> +
> + if (index == VFIO_PCI_BAR2_REGION_INDEX) {
> + mutex_lock(&nvdev->memmap_lock);
> + if (!nvdev->memmap) {
> + nvdev->memmap = memremap(nvdev->memphys, nvdev->memlength, MEMREMAP_WB);
> + if (!nvdev->memmap) {
> + mutex_unlock(&nvdev->memmap_lock);
> + return -ENOMEM;
> + }
> + }
> + mutex_unlock(&nvdev->memmap_lock);
> +
> + return nvgrace_gpu_write_mem(count, ppos, buf, nvdev);
> + }
> +
> + return vfio_pci_core_write(core_vdev, buf, count, ppos);
> +}
> +
> +static const struct vfio_device_ops nvgrace_gpu_vfio_pci_ops = {
> + .name = "nvgrace-gpu-vfio-pci",
> + .init = vfio_pci_core_init_dev,
> + .release = vfio_pci_core_release_dev,
> + .open_device = nvgrace_gpu_vfio_pci_open_device,
> + .close_device = nvgrace_gpu_vfio_pci_close_device,
> + .ioctl = nvgrace_gpu_vfio_pci_ioctl,
> + .read = nvgrace_gpu_vfio_pci_read,
> + .write = nvgrace_gpu_vfio_pci_write,
> + .mmap = nvgrace_gpu_vfio_pci_mmap,
> + .request = vfio_pci_core_request,
> + .match = vfio_pci_core_match,
> + .bind_iommufd = vfio_iommufd_physical_bind,
> + .unbind_iommufd = vfio_iommufd_physical_unbind,
> + .attach_ioas = vfio_iommufd_physical_attach_ioas,
> +};
> +
> +static struct
> +nvgrace_gpu_vfio_pci_core_device *nvgrace_gpu_drvdata(struct pci_dev *pdev)
> +{
> + struct vfio_pci_core_device *core_device = dev_get_drvdata(&pdev->dev);
> +
> + return container_of(core_device, struct nvgrace_gpu_vfio_pci_core_device,
> + core_device);
> +}
> +
> +static int
> +nvgrace_gpu_vfio_pci_fetch_memory_property(struct pci_dev *pdev,
> + struct nvgrace_gpu_vfio_pci_core_device *nvdev)
> +{
> + int ret;
> + u64 memphys, memlength;
> +
> + /*
> + * The memory information is present in the system ACPI tables as DSD
> + * properties nvidia,gpu-mem-base-pa and nvidia,gpu-mem-size.
> + */
> + ret = device_property_read_u64(&pdev->dev, "nvidia,gpu-mem-base-pa",
> + &(memphys));
> + if (ret)
> + return ret;
> +
> + if (memphys > type_max(phys_addr_t))
> + return -EOVERFLOW;
> +
> + nvdev->memphys = memphys;
> +
> + ret = device_property_read_u64(&pdev->dev, "nvidia,gpu-mem-size",
> + &(memlength));
> + if (ret)
> + return ret;
> +
> + if (memlength > type_max(size_t))
> + return -EOVERFLOW;
> +
> + /*
> + * If the C2C link is not up due to an error, the coherent device
> + * memory size is returned as 0. Fail in such case.
> + */
> + if (memlength == 0)
> + return -ENOMEM;
> +
> + nvdev->memlength = memlength;
> +
> + return ret;
> +}
> +
> +static int nvgrace_gpu_vfio_pci_probe(struct pci_dev *pdev,
> + const struct pci_device_id *id)
> +{
> + struct nvgrace_gpu_vfio_pci_core_device *nvdev;
> + int ret;
> +
> + nvdev = vfio_alloc_device(nvgrace_gpu_vfio_pci_core_device, core_device.vdev,
> + &pdev->dev, &nvgrace_gpu_vfio_pci_ops);
> + if (IS_ERR(nvdev))
> + return PTR_ERR(nvdev);
> +
> + dev_set_drvdata(&pdev->dev, nvdev);
> +
> + ret = nvgrace_gpu_vfio_pci_fetch_memory_property(pdev, nvdev);
> + if (ret)
> + goto out_put_vdev;
> +
> + ret = vfio_pci_core_register_device(&nvdev->core_device);
> + if (ret)
> + goto out_put_vdev;
> +
> + return ret;
> +
> +out_put_vdev:
> + vfio_put_device(&nvdev->core_device.vdev);
> + return ret;
> +}
> +
> +static void nvgrace_gpu_vfio_pci_remove(struct pci_dev *pdev)
> +{
> + struct nvgrace_gpu_vfio_pci_core_device *nvdev = nvgrace_gpu_drvdata(pdev);
> + struct vfio_pci_core_device *vdev = &nvdev->core_device;
> +
> + vfio_pci_core_unregister_device(vdev);
> + vfio_put_device(&vdev->vdev);
> +}
> +
> +static const struct pci_device_id nvgrace_gpu_vfio_pci_table[] = {
> + /* GH200 120GB */
> + { PCI_DRIVER_OVERRIDE_DEVICE_VFIO(PCI_VENDOR_ID_NVIDIA, 0x2342) },
> + /* GH200 480GB */
> + { PCI_DRIVER_OVERRIDE_DEVICE_VFIO(PCI_VENDOR_ID_NVIDIA, 0x2345) },
> + {}
> +};
> +
> +MODULE_DEVICE_TABLE(pci, nvgrace_gpu_vfio_pci_table);
> +
> +static ssize_t coherent_mem_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct vfio_pci_core_device *core_device = dev_get_drvdata(dev);
> + struct nvgrace_gpu_vfio_pci_core_device *nvdev
> + = container_of(core_device, struct nvgrace_gpu_vfio_pci_core_device,
> + core_device);
> +
> + return sprintf(buf, "%u\n", nvdev->memlength ? 1 : 0);
> +}
> +static DEVICE_ATTR_RO(coherent_mem);
> +
> +static struct attribute *nvgrace_gpu_vfio_dev_attributes[] = {
> + &dev_attr_coherent_mem.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group nvgrace_gpu_vfio_dev_attribute_group = {
> + .attrs = nvgrace_gpu_vfio_dev_attributes,
> +};
> +
> +static const struct attribute_group *nvgrace_gpu_vfio_dev_attribute_groups[] = {
> + &nvgrace_gpu_vfio_dev_attribute_group,
> + NULL,
> +};
> +
> +static struct pci_driver nvgrace_gpu_vfio_pci_driver = {
> + .name = KBUILD_MODNAME,
> + .id_table = nvgrace_gpu_vfio_pci_table,
> + .probe = nvgrace_gpu_vfio_pci_probe,
> + .remove = nvgrace_gpu_vfio_pci_remove,
> + .err_handler = &vfio_pci_core_err_handlers,
> + .driver_managed_dma = true,
> + .dev_groups = nvgrace_gpu_vfio_dev_attribute_groups,
> +};
> +
> +module_pci_driver(nvgrace_gpu_vfio_pci_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Ankit Agrawal <[email protected]>");
> +MODULE_AUTHOR("Aniket Agashe <[email protected]>");
> +MODULE_DESCRIPTION(
> + "VFIO NVGRACE GPU PF - User Level driver for NVIDIA devices with CPU coherently accessible device memory");
On Fri, Sep 15, 2023 at 08:24:30AM -0600, Alex Williamson wrote:
> On Thu, 14 Sep 2023 19:54:15 -0700
> <[email protected]> wrote:
>
> > From: Ankit Agrawal <[email protected]>
> >
> > NVIDIA's upcoming Grace Hopper Superchip provides a PCI-like device
> > for the on-chip GPU that is the logical OS representation of the
> > internal proprietary cache coherent interconnect.
> >
> > This representation has a number of limitations compared to a real PCI
> > device, in particular, it does not model the coherent GPU memory
> > aperture as a PCI config space BAR, and PCI doesn't know anything
> > about cacheable memory types.
> >
> > Provide a VFIO PCI variant driver that adapts the unique PCI
> > representation into a more standard PCI representation facing
> > userspace. The GPU memory aperture is obtained from ACPI using
> > device_property_read_u64(), according to the FW specification,
> > and exported to userspace as a separate VFIO_REGION. Since the device
> > implements only one 64-bit BAR (BAR0), the GPU memory aperture is mapped
> > to the next available PCI BAR (BAR2). Qemu will then naturally generate a
> > PCI device in the VM with two 64-bit BARs (where the cacheable aperture
> > reported in BAR2).
> >
> > Since this memory region is actually cache coherent with the CPU, the
> > VFIO variant driver will mmap it into VMA using a cacheable mapping. The
> > mapping is done using remap_pfn_range().
> >
> > PCI BAR are aligned to the power-of-2, but the actual memory on the
> > device may not. A read or write access to the physical address from the
> > last device PFN up to the next power-of-2 aligned physical address
> > results in reading ~0 and dropped writes.
> >
> > Lastly the presence of CPU cache coherent device memory is exposed
> > through sysfs for use by user space.
>
> This looks like a giant red flag that this approach of masquerading the
> coherent memory as a PCI BAR is the wrong way to go. If the VMM needs
> to know about this coherent memory, it needs to get that information
> in-band.
The VMM part doesn't need this flag, nor does the VM. The
orchestration needs to know when to setup the pxm stuff.
I think we should drop the sysfs for now until the qemu thread about
the pxm stuff settles into an idea.
When the qemu API is clear we can have a discussion on what component
should detect this driver and setup the pxm things, then answer the
how should the detection work from the kernel side.
> be reaching out to arbitrary sysfs attributes. Minimally this
> information should be provided via a capability on the region info
> chain,
That definitely isn't suitable, eg libvirt won't have access to inband
information if it turns out libvirt is supposed to setup the pxm qemu
arguments?
> A "coherent_mem" attribute on the device provides a very weak
> association to the memory region it's trying to describe.
That's because it's use has nothing to do with the memory region :)
Jason
On Mon, Sep 18, 2023 at 11:19:49AM -0600, Alex Williamson wrote:
> > > And the VMM usage is self inflicted because we insist on
> > > masquerading the coherent memory as a nondescript PCI BAR rather
> > > than providing a device specific region to enlighten the VMM to this
> > > unique feature.
> >
> > I see it as two completely seperate things.
> >
> > 1) VFIO and qemu creating a vPCI device. Here we don't need this
> > information.
> >
> > 2) This ACPI pxm stuff to emulate the bare metal FW.
> > Including a proposal for auto-detection what kind of bare metal FW
> > is being used.
> >
> > This being a poor idea for #2 doesn't jump to problems with #1, it
> > just says more work is needed on the ACPI PXM stuff.
>
> But I don't think we've justified why it's a good idea for #1. Does
> the composed vPCI device with coherent memory masqueraded as BAR2 have
> a stand alone use case without #2?
Today there is no SW that can operate that configuration. But that is
a purely SW in the VM problem.
Jonathan got it right here:
https://lore.kernel.org/all/[email protected]/
If Linux in the VM wants to use certain Linux kernel APIs then the FW
must provision these empty nodes. Universally. It is a CXL problem as
well.
For instance I could hack up Linux and force it to create extra nodes
regardless of ACPI and then everything would be fine with #1 alone.
When/if Linux learns to dynmically create these things without relying
on FW then we don't need #2.
It is ugly, it is hack, but it is copying what real FW decided to do.
> My understanding based on these series is that the guest driver somehow
> carves up the coherent memory among a set of memory-less NUMA nodes
> (how to know how many?) created by the VMM and reported via the _DSD for
> the device. If this sort of configuration is a requirement for making
> use of the coherent memory, then what exactly becomes easier by the fact
> that it's exposed as a PCI BAR?
It is keeping two concerns seperate. The vPCI layer doesn't care about
any of this because it is a Linux problem. A coherent BAR is fine and
results in the least amount of special code everywhere.
The ACPI layer has to learn how to make this hack to support Linux.
I don't think we should dramatically warp the modeling of the VFIO
regions just to support auto detecting an ACPI hack.
> In fact, if it weren't a BAR I'd probably suggest that the whole
> configuration of this device should be centered around a new
> nvidia-gpu-mem object. That object could reference the ID of a
> vfio-pci device providing the coherent memory via a device specific
> region and be provided with a range of memory-less nodes created for
> its use. The object would insert the coherent memory range into the VM
> address space and provide the device properties to make use of it in
> the same way as done on bare metal.
How does that give auto configuration? The other thread mentions that
many other things need this too, like CXL and imagined coherent
virtio stuff?
Can we do the API you imagine more generically with any VFIO region
(even a normal BAR) providing the memory object?
> It seems to me that the PCI BAR representation of coherent memory is
> largely just a shortcut to getting it into the VM address space, but
> it's also leading us down these paths where the "pxm stuff" is invoked
> based on the device attached to the VM, which is getting a lot of
> resistance.
I don't like the idea of a dedicated memory region type, I think we
will have more of these than just one.
Some kind of flag on the vfio device indicating that PXM nodes (and
how many) should be auto created would be fine.
But if there is resistance to auto configuration I don't see how that
goes away just because we shift around the indicator to trigger
auto configuration??
Jason
On Mon, Sep 18, 2023 at 08:27:48AM -0600, Alex Williamson wrote:
> > > This looks like a giant red flag that this approach of masquerading the
> > > coherent memory as a PCI BAR is the wrong way to go. If the VMM needs
> > > to know about this coherent memory, it needs to get that information
> > > in-band.
> >
> > The VMM part doesn't need this flag, nor does the VM. The
> > orchestration needs to know when to setup the pxm stuff.
>
> Subject: [PATCH v1 1/4] vfio: new command line params for device memory NUMA nodes
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> ...
> +static bool vfio_pci_read_cohmem_support_sysfs(VFIODevice *vdev)
> +{
> + gchar *contents = NULL;
> + gsize length;
> + char *path;
> + bool ret = false;
> + uint32_t supported;
> +
> + path = g_strdup_printf("%s/coherent_mem", vdev->sysfsdev);
> + if (g_file_get_contents(path, &contents, &length, NULL) && length > 0) {
> + if ((sscanf(contents, "%u", &supported) == 1) && supported) {
> + ret = true;
> + }
> + }
Yes, but it drives the ACPI pxm auto configuration stuff, not really
vfio stuff.
> > I think we should drop the sysfs for now until the qemu thread about
> > the pxm stuff settles into an idea.
> >
> > When the qemu API is clear we can have a discussion on what component
> > should detect this driver and setup the pxm things, then answer the
> > how should the detection work from the kernel side.
> >
> > > be reaching out to arbitrary sysfs attributes. Minimally this
> > > information should be provided via a capability on the region info
> > > chain,
> >
> > That definitely isn't suitable, eg libvirt won't have access to inband
> > information if it turns out libvirt is supposed to setup the pxm qemu
> > arguments?
>
> Why would libvirt look for a "coherent_mem" attribute in sysfs when it
> can just look at the driver used by the device.
Sure, if that is consensus. Also I think coherent_mem is a terrible
sysfs name for this, it should be more like 'num_pxm_nodes' or
something.
> Part of the QEMU series is also trying to invoke the VM
> configuration based only on this
> device being attached to avoid libvirt orchestration changes:
Right, that is where it gets confusing - it mixes the vfio world in
qemu with the pxm world. That should be cleaned up somehow.
> > > A "coherent_mem" attribute on the device provides a very weak
> > > association to the memory region it's trying to describe.
> >
> > That's because it's use has nothing to do with the memory region :)
>
> So we're creating a very generic sysfs attribute, which is meant to be
> used by orchestration to invoke device specific configuration, but is
> currently only proposed for use by the VMM. The orchestration problem
> doesn't really exist, libvirt could know simply by the driver name that
> the device requires this configuration.
Yep
> And the VMM usage is self inflicted because we insist on
> masquerading the coherent memory as a nondescript PCI BAR rather
> than providing a device specific region to enlighten the VMM to this
> unique feature.
I see it as two completely seperate things.
1) VFIO and qemu creating a vPCI device. Here we don't need this
information.
2) This ACPI pxm stuff to emulate the bare metal FW.
Including a proposal for auto-detection what kind of bare metal FW
is being used.
This being a poor idea for #2 doesn't jump to problems with #1, it
just says more work is needed on the ACPI PXM stuff.
So lets remove the sysfs, reach a conclusion with the qemu folks on
how the pxm is supposed to be modeled and then Ankit can come back
with some suggestion how to do auto configure. If it needs a sysfs or
not we can see.
Jason
On Mon, 18 Sep 2023 10:02:56 -0300
Jason Gunthorpe <[email protected]> wrote:
> On Fri, Sep 15, 2023 at 08:24:30AM -0600, Alex Williamson wrote:
> > On Thu, 14 Sep 2023 19:54:15 -0700
> > <[email protected]> wrote:
> >
> > > From: Ankit Agrawal <[email protected]>
> > >
> > > NVIDIA's upcoming Grace Hopper Superchip provides a PCI-like device
> > > for the on-chip GPU that is the logical OS representation of the
> > > internal proprietary cache coherent interconnect.
> > >
> > > This representation has a number of limitations compared to a real PCI
> > > device, in particular, it does not model the coherent GPU memory
> > > aperture as a PCI config space BAR, and PCI doesn't know anything
> > > about cacheable memory types.
> > >
> > > Provide a VFIO PCI variant driver that adapts the unique PCI
> > > representation into a more standard PCI representation facing
> > > userspace. The GPU memory aperture is obtained from ACPI using
> > > device_property_read_u64(), according to the FW specification,
> > > and exported to userspace as a separate VFIO_REGION. Since the device
> > > implements only one 64-bit BAR (BAR0), the GPU memory aperture is mapped
> > > to the next available PCI BAR (BAR2). Qemu will then naturally generate a
> > > PCI device in the VM with two 64-bit BARs (where the cacheable aperture
> > > reported in BAR2).
> > >
> > > Since this memory region is actually cache coherent with the CPU, the
> > > VFIO variant driver will mmap it into VMA using a cacheable mapping. The
> > > mapping is done using remap_pfn_range().
> > >
> > > PCI BAR are aligned to the power-of-2, but the actual memory on the
> > > device may not. A read or write access to the physical address from the
> > > last device PFN up to the next power-of-2 aligned physical address
> > > results in reading ~0 and dropped writes.
> > >
> > > Lastly the presence of CPU cache coherent device memory is exposed
> > > through sysfs for use by user space.
> >
> > This looks like a giant red flag that this approach of masquerading the
> > coherent memory as a PCI BAR is the wrong way to go. If the VMM needs
> > to know about this coherent memory, it needs to get that information
> > in-band.
>
> The VMM part doesn't need this flag, nor does the VM. The
> orchestration needs to know when to setup the pxm stuff.
Subject: [PATCH v1 1/4] vfio: new command line params for device memory NUMA nodes
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
...
+static bool vfio_pci_read_cohmem_support_sysfs(VFIODevice *vdev)
+{
+ gchar *contents = NULL;
+ gsize length;
+ char *path;
+ bool ret = false;
+ uint32_t supported;
+
+ path = g_strdup_printf("%s/coherent_mem", vdev->sysfsdev);
+ if (g_file_get_contents(path, &contents, &length, NULL) && length > 0) {
+ if ((sscanf(contents, "%u", &supported) == 1) && supported) {
+ ret = true;
+ }
+ }
> I think we should drop the sysfs for now until the qemu thread about
> the pxm stuff settles into an idea.
>
> When the qemu API is clear we can have a discussion on what component
> should detect this driver and setup the pxm things, then answer the
> how should the detection work from the kernel side.
>
> > be reaching out to arbitrary sysfs attributes. Minimally this
> > information should be provided via a capability on the region info
> > chain,
>
> That definitely isn't suitable, eg libvirt won't have access to inband
> information if it turns out libvirt is supposed to setup the pxm qemu
> arguments?
Why would libvirt look for a "coherent_mem" attribute in sysfs when it
can just look at the driver used by the device. Part of the QEMU
series is also trying to invoke the VM configuration based only on this
device being attached to avoid libvirt orchestration changes:
Subject: [PATCH v1 2/4] vfio: assign default values to node params
It may be desirable for some deployments to have QEMU automatically
pick a range and create the NUMA nodes. So the admin need not care
about passing any additional params. Another advantage is that the
feature is not dependent on newer libvirt that support the new
parameters pxm-ns and pxm-nc.
> > A "coherent_mem" attribute on the device provides a very weak
> > association to the memory region it's trying to describe.
>
> That's because it's use has nothing to do with the memory region :)
So we're creating a very generic sysfs attribute, which is meant to be
used by orchestration to invoke device specific configuration, but is
currently only proposed for use by the VMM. The orchestration problem
doesn't really exist, libvirt could know simply by the driver name that
the device requires this configuration. And the VMM usage is self
inflicted because we insist on masquerading the coherent memory as a
nondescript PCI BAR rather than providing a device specific region to
enlighten the VMM to this unique feature. Thanks,
Alex
On Mon, 18 Sep 2023 11:49:23 -0300
Jason Gunthorpe <[email protected]> wrote:
> On Mon, Sep 18, 2023 at 08:27:48AM -0600, Alex Williamson wrote:
>
> > > > This looks like a giant red flag that this approach of masquerading the
> > > > coherent memory as a PCI BAR is the wrong way to go. If the VMM needs
> > > > to know about this coherent memory, it needs to get that information
> > > > in-band.
> > >
> > > The VMM part doesn't need this flag, nor does the VM. The
> > > orchestration needs to know when to setup the pxm stuff.
> >
> > Subject: [PATCH v1 1/4] vfio: new command line params for device memory NUMA nodes
> > --- a/hw/vfio/pci.c
> > +++ b/hw/vfio/pci.c
> > ...
> > +static bool vfio_pci_read_cohmem_support_sysfs(VFIODevice *vdev)
> > +{
> > + gchar *contents = NULL;
> > + gsize length;
> > + char *path;
> > + bool ret = false;
> > + uint32_t supported;
> > +
> > + path = g_strdup_printf("%s/coherent_mem", vdev->sysfsdev);
> > + if (g_file_get_contents(path, &contents, &length, NULL) && length > 0) {
> > + if ((sscanf(contents, "%u", &supported) == 1) && supported) {
> > + ret = true;
> > + }
> > + }
>
> Yes, but it drives the ACPI pxm auto configuration stuff, not really
> vfio stuff.
>
> > > I think we should drop the sysfs for now until the qemu thread about
> > > the pxm stuff settles into an idea.
> > >
> > > When the qemu API is clear we can have a discussion on what component
> > > should detect this driver and setup the pxm things, then answer the
> > > how should the detection work from the kernel side.
> > >
> > > > be reaching out to arbitrary sysfs attributes. Minimally this
> > > > information should be provided via a capability on the region info
> > > > chain,
> > >
> > > That definitely isn't suitable, eg libvirt won't have access to inband
> > > information if it turns out libvirt is supposed to setup the pxm qemu
> > > arguments?
> >
> > Why would libvirt look for a "coherent_mem" attribute in sysfs when it
> > can just look at the driver used by the device.
>
> Sure, if that is consensus. Also I think coherent_mem is a terrible
> sysfs name for this, it should be more like 'num_pxm_nodes' or
> something.
>
> > Part of the QEMU series is also trying to invoke the VM
> > configuration based only on this
> > device being attached to avoid libvirt orchestration changes:
>
> Right, that is where it gets confusing - it mixes the vfio world in
> qemu with the pxm world. That should be cleaned up somehow.
>
> > > > A "coherent_mem" attribute on the device provides a very weak
> > > > association to the memory region it's trying to describe.
> > >
> > > That's because it's use has nothing to do with the memory region :)
> >
> > So we're creating a very generic sysfs attribute, which is meant to be
> > used by orchestration to invoke device specific configuration, but is
> > currently only proposed for use by the VMM. The orchestration problem
> > doesn't really exist, libvirt could know simply by the driver name that
> > the device requires this configuration.
>
> Yep
>
> > And the VMM usage is self inflicted because we insist on
> > masquerading the coherent memory as a nondescript PCI BAR rather
> > than providing a device specific region to enlighten the VMM to this
> > unique feature.
>
> I see it as two completely seperate things.
>
> 1) VFIO and qemu creating a vPCI device. Here we don't need this
> information.
>
> 2) This ACPI pxm stuff to emulate the bare metal FW.
> Including a proposal for auto-detection what kind of bare metal FW
> is being used.
>
> This being a poor idea for #2 doesn't jump to problems with #1, it
> just says more work is needed on the ACPI PXM stuff.
But I don't think we've justified why it's a good idea for #1. Does
the composed vPCI device with coherent memory masqueraded as BAR2 have
a stand alone use case without #2?
My understanding based on these series is that the guest driver somehow
carves up the coherent memory among a set of memory-less NUMA nodes
(how to know how many?) created by the VMM and reported via the _DSD for
the device. If this sort of configuration is a requirement for making
use of the coherent memory, then what exactly becomes easier by the fact
that it's exposed as a PCI BAR?
In fact, if it weren't a BAR I'd probably suggest that the whole
configuration of this device should be centered around a new
nvidia-gpu-mem object. That object could reference the ID of a
vfio-pci device providing the coherent memory via a device specific
region and be provided with a range of memory-less nodes created for
its use. The object would insert the coherent memory range into the VM
address space and provide the device properties to make use of it in
the same way as done on bare metal.
It seems to me that the PCI BAR representation of coherent memory is
largely just a shortcut to getting it into the VM address space, but
it's also leading us down these paths where the "pxm stuff" is invoked
based on the device attached to the VM, which is getting a lot of
resistance. Thanks,
Alex
> From: [email protected] <[email protected]>
> Sent: Friday, September 15, 2023 10:54 AM
>
> PCI BAR are aligned to the power-of-2, but the actual memory on the
> device may not. A read or write access to the physical address from the
> last device PFN up to the next power-of-2 aligned physical address
> results in reading ~0 and dropped writes.
Though the variant driver emulates the access to the offset beyond
the available memory size, how does the userspace driver or the guest
learn to know the actual size and avoid using the invalid hole to hold
valid data?
> +config NVGRACE_GPU_VFIO_PCI
> + tristate "VFIO support for the GPU in the NVIDIA Grace Hopper
> Superchip"
> + depends on ARM64 || (COMPILE_TEST && 64BIT)
> + select VFIO_PCI_CORE
> + help
> + VFIO support for the GPU in the NVIDIA Grace Hopper Superchip is
> + required to assign the GPU device to a VM using KVM/qemu/etc.
VFIO driver is not only for VM.
> +
> +static int nvgrace_gpu_vfio_pci_mmap(struct vfio_device *core_vdev,
> + struct vm_area_struct *vma)
> +{
> + struct nvgrace_gpu_vfio_pci_core_device *nvdev = container_of(
> + core_vdev, struct nvgrace_gpu_vfio_pci_core_device,
> core_device.vdev);
> +
> + unsigned long start_pfn;
> + unsigned int index;
> + u64 req_len, pgoff, end;
> + int ret = 0;
> +
> + index = vma->vm_pgoff >> (VFIO_PCI_OFFSET_SHIFT - PAGE_SHIFT);
> + if (index != VFIO_PCI_BAR2_REGION_INDEX)
> + return vfio_pci_core_mmap(core_vdev, vma);
> +
> + /*
> + * Request to mmap the BAR. Map to the CPU accessible memory on
> the
> + * GPU using the memory information gathered from the system ACPI
> + * tables.
> + */
> + pgoff = vma->vm_pgoff &
> + ((1U << (VFIO_PCI_OFFSET_SHIFT - PAGE_SHIFT)) - 1);
> +
> + if (check_sub_overflow(vma->vm_end, vma->vm_start, &req_len) ||
> + check_add_overflow(PHYS_PFN(nvdev->memphys), pgoff,
> &start_pfn) ||
> + check_add_overflow(PFN_PHYS(pgoff), req_len, &end))
> + return -EOVERFLOW;
> +
> + /*
> + * Check that the mapping request does not go beyond available
> device
> + * memory size
> + */
> + if (end > nvdev->memlength)
> + return -EINVAL;
> +
> + /*
> + * Perform a PFN map to the memory and back the device BAR by the
> + * GPU memory.
> + *
> + * The available GPU memory size may not be power-of-2 aligned.
> Given
> + * that the memory is exposed as a BAR, the mapping request is of
> the
> + * power-of-2 aligned size. Map only up to the size of the GPU
why is the mapping request in power-of-2? The caller should follow the
sparse mapping to raise the request. Otherwise the check on 'end' right
above will always fail.
> +
> +/*
> + * Read count bytes from the device memory at an offset. The actual device
> + * memory size (available) may not be a power-of-2. So the driver fakes
> + * the size to a power-of-2 (reported) when exposing to a user space driver.
> + *
> + * Read request beyond the actual device size is filled with ~0, while
> + * those beyond the actual reported size is skipped.
> + *
> + * A read from a negative or a reported+ offset, a negative count are
it's hard to understand 'reported+'. Please update it with a descriptive
word.
> + * considered error conditions and returned with an -EINVAL.
> + */
> +ssize_t nvgrace_gpu_read_mem(void __user *buf, size_t count, loff_t *ppos,
> + struct nvgrace_gpu_vfio_pci_core_device *nvdev)
> +{
> + u64 offset = *ppos & VFIO_PCI_OFFSET_MASK;
> + size_t mem_count, i, bar_size = roundup_pow_of_two(nvdev-
> >memlength);
> + u8 val = 0xFF;
> +
> + if (offset >= bar_size)
> + return -EINVAL;
> +
> + /* Clip short the read request beyond reported BAR size */
> + count = min(count, bar_size - (size_t)offset);
> +
> + /*
> + * Determine how many bytes to be actually read from the device
> memory.
> + * Do not read from the offset beyond available size.
> + */
> + if (offset >= nvdev->memlength)
> + return 0;
This violates the comment:
* Read request beyond the actual device size is filled with ~0, while
* those beyond the actual reported size is skipped.
> +
> + mem_count = min(count, nvdev->memlength - (size_t)offset);
> +
> + /*
> + * Handle read on the BAR2 region. Map to the target device memory
> + * physical address and copy to the request read buffer.
> + */
> + if (copy_to_user(buf, (u8 *)nvdev->memmap + offset, mem_count))
> + return -EFAULT;
> +
> + /*
> + * Only the device memory present on the hardware is mapped,
> which may
> + * not be power-of-2 aligned. A read to the BAR2 region implies an
> + * access outside the available device memory on the hardware. Fill
Why is a read to BAR2 region implies an out-of-scope access?
> +static ssize_t nvgrace_gpu_vfio_pci_read(struct vfio_device *core_vdev,
> + char __user *buf, size_t count, loff_t
> *ppos)
> +{
> + unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos);
> + struct nvgrace_gpu_vfio_pci_core_device *nvdev = container_of(
> + core_vdev, struct nvgrace_gpu_vfio_pci_core_device,
> core_device.vdev);
> +
> + if (index == VFIO_PCI_BAR2_REGION_INDEX) {
> + mutex_lock(&nvdev->memmap_lock);
> + if (!nvdev->memmap) {
> + nvdev->memmap = memremap(nvdev->memphys,
> nvdev->memlength, MEMREMAP_WB);
> + if (!nvdev->memmap) {
> + mutex_unlock(&nvdev->memmap_lock);
> + return -ENOMEM;
> + }
> + }
> + mutex_unlock(&nvdev->memmap_lock);
move above to a helper and share with the write fn.
> +ssize_t nvgrace_gpu_write_mem(size_t count, loff_t *ppos, const void
> __user *buf,
> + struct nvgrace_gpu_vfio_pci_core_device *nvdev)
> +{
> + u64 offset = *ppos & VFIO_PCI_OFFSET_MASK;
> + size_t mem_count, bar_size = roundup_pow_of_two(nvdev-
> >memlength);
> +
> + if (offset >= bar_size)
> + return -EINVAL;
> +
> + /* Clip short the read request beyond reported BAR size */
> + count = min(count, bar_size - (size_t)offset);
s/read/write/
> +
> + /*
> + * Determine how many bytes to be actually written to the device
> memory.
> + * Do not write to the offset beyond available size.
> + */
> + if (offset >= nvdev->memlength)
> + return 0;
still need return count as no-write is still a success emulation of write.
ditto for the read fn.
> +
> + mem_count = min(count, nvdev->memlength - (size_t)offset);
> +
> + /*
> + * Only the device memory present on the hardware is mapped,
> which may
> + * not be power-of-2 aligned. A write to the BAR2 region implies an
> + * access outside the available device memory on the hardware. Drop
> + * those write requests.
> + */
> + if (copy_from_user((u8 *)nvdev->memmap + offset, buf,
> mem_count))
> + return -EFAULT;
> +
> + return count;
*ppos is not adjusted. ditto for the read fn.
> +
> +static ssize_t coherent_mem_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct vfio_pci_core_device *core_device = dev_get_drvdata(dev);
> + struct nvgrace_gpu_vfio_pci_core_device *nvdev
> + = container_of(core_device, struct
> nvgrace_gpu_vfio_pci_core_device,
> + core_device);
> +
> + return sprintf(buf, "%u\n", nvdev->memlength ? 1 : 0);
if nvdev->memlength is 0 then probe will fail.
So here can always return 1.