2023-10-15 16:32:05

by Ankit Agrawal

[permalink] [raw]
Subject: [PATCH v12 1/1] vfio/nvgpu: Add vfio pci variant module for grace hopper

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. Note that the GPU device
driver [1] is capable of knowing the exact device memory size through
separate means. The device memory size is primarily kept in the system
ACPI tables for use by the VFIO PCI variant module.

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 over next-20231005.

[1] https://github.com/NVIDIA/open-gpu-kernel-modules

Signed-off-by: Ankit Agrawal <[email protected]>
Signed-off-by: Aniket Agashe <[email protected]>
Reviewed-by: Kevin Tian <[email protected]>
---

Link for v11:
https://lore.kernel.org/all/[email protected]/

v11 -> v12
- More details in commit message on device memory size

v10 -> v11
- Removed sysfs attribute to expose the CPU coherent memory feature
- Addressed review comments

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 | 481 ++++++++++++++++++++++++++
6 files changed, 504 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 e60e800ed91c..c47230def6f1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -22770,6 +22770,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..936e88d8d41d
--- /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 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..b8634974e5cc
--- /dev/null
+++ b/drivers/vfio/pci/nvgrace-gpu/main.c
@@ -0,0 +1,481 @@
+// 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. Map up
+ * to the size of the device 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);
+}
+
+static int nvgrace_gpu_memmap(struct nvgrace_gpu_vfio_pci_core_device *nvdev)
+{
+ 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 0;
+}
+
+/*
+ * 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 an offset greater than reported size, 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.
+ * Read request beyond the actual device memory size is filled with ~0,
+ * while those beyond the actual reported size is skipped.
+ */
+ if (offset >= nvdev->memlength)
+ mem_count = 0;
+ else
+ 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 an offset beyond the device memory
+ * size is filled with ~0.
+ */
+ for (i = mem_count; i < count; i++)
+ put_user(val, (unsigned char __user *)(buf + i));
+
+ *ppos += count;
+ 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);
+ int ret;
+
+ if (index == VFIO_PCI_BAR2_REGION_INDEX) {
+ ret = nvgrace_gpu_memmap(nvdev);
+ if (ret)
+ return ret;
+
+ 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 an offset greater than the reported size, 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 write 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)
+ goto exitfn;
+
+ 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;
+
+exitfn:
+ *ppos += count;
+ 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);
+ int ret;
+
+ if (index == VFIO_PCI_BAR2_REGION_INDEX) {
+ ret = nvgrace_gpu_memmap(nvdev);
+ if (ret)
+ return ret;
+
+ 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 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,
+};
+
+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


2023-10-17 22:55:48

by Alex Williamson

[permalink] [raw]
Subject: Re: [PATCH v12 1/1] vfio/nvgpu: Add vfio pci variant module for grace hopper

On Sun, 15 Oct 2023 22:00:47 +0530
<[email protected]> wrote:
> +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);
> + int ret;
> +
> + if (index == VFIO_PCI_BAR2_REGION_INDEX) {
> + ret = nvgrace_gpu_memmap(nvdev);
> + if (ret)
> + return ret;
> +
> + return nvgrace_gpu_read_mem(buf, count, ppos, nvdev);
> + }

After looking at Yishai's virtio-vfio-pci driver where BAR0 is emulated
as an IO Port BAR, it occurs to me that there's no config space
emulation of BAR2 (or BAR3) here. Doesn't this mean that QEMU registers
the BAR as 32-bit, non-prefetchable? ie. VFIOBAR.type & .mem64 are
wrong?

I'd certainly expect this to be emulated as a 64-bit, prefetchable BAR
and the commit log indicates the intention is that this is exposed as a
64-bit BAR.

We also need to decide how strictly variant drivers need to emulate
vfio_pci_config_rw with respect to BAR sizing, where the core code
provides emulation of sizing and Yishai's virtio driver only emulates
the IO port indicator bit. QEMU doesn't really need this, but the
vfio-pci implementation sets the precedent that this behavior is
provided and could be used by other userspace drivers. It's essentially
just providing a masked buffer to service reads and writes to the BAR2
and BAR3 config address here. Thanks,

Alex

> +
> + return vfio_pci_core_read(core_vdev, buf, count, ppos);
> +}

2023-10-23 12:48:45

by Ankit Agrawal

[permalink] [raw]
Subject: Re: [PATCH v12 1/1] vfio/nvgpu: Add vfio pci variant module for grace hopper

> After looking at Yishai's virtio-vfio-pci driver where BAR0 is emulated
> as an IO Port BAR, it occurs to me that there's no config space
> emulation of BAR2 (or BAR3) here.? Doesn't this mean that QEMU registers
> the BAR as 32-bit, non-prefetchable?? ie. VFIOBAR.type & .mem64 are
> wrong?

Maybe I didn't understand the question, but the PCI config space read/write
would still be handled by vfio_pci_core_read/write() which returns the
appropriate flags. I have checked that the device BARs are 64b and
prefetchable in the VM.

> We also need to decide how strictly variant drivers need to emulate
> vfio_pci_config_rw with respect to BAR sizing, where the core code
> provides emulation of sizing and Yishai's virtio driver only emulates
> the IO port indicator bit.

Sorry, it isn't clear to me how would resizable BAR is applicable in this
variant driver as the BAR represents the device memory. Should we be
exposing such feature as unsupported for this variant driver?

2023-10-23 14:46:08

by Alex Williamson

[permalink] [raw]
Subject: Re: [PATCH v12 1/1] vfio/nvgpu: Add vfio pci variant module for grace hopper

On Mon, 23 Oct 2023 12:48:22 +0000
Ankit Agrawal <[email protected]> wrote:

> > After looking at Yishai's virtio-vfio-pci driver where BAR0 is emulated
> > as an IO Port BAR, it occurs to me that there's no config space
> > emulation of BAR2 (or BAR3) here.  Doesn't this mean that QEMU registers
> > the BAR as 32-bit, non-prefetchable?  ie. VFIOBAR.type & .mem64 are
> > wrong?
>
> Maybe I didn't understand the question, but the PCI config space read/write
> would still be handled by vfio_pci_core_read/write() which returns the
> appropriate flags. I have checked that the device BARs are 64b and
> prefetchable in the VM.

vfio_pci_core_read/write() accesses the physical device, which doesn't
implement BAR2. Why would an unimplemented BAR2 on the physical device
report 64-bit, prefetchable?

QEMU records VFIOBAR.type and .mem64 from reading the BAR register in
vfio_bar_prepare() and passes this type to pci_register_bar() in
vfio_bar_register(). Without an implementation of a config space read
op in the variant driver and with no physical implementation of BAR2 on
the device, I don't see how we get correct values in these fields.

> > We also need to decide how strictly variant drivers need to emulate
> > vfio_pci_config_rw with respect to BAR sizing, where the core code
> > provides emulation of sizing and Yishai's virtio driver only emulates
> > the IO port indicator bit.
>
> Sorry, it isn't clear to me how would resizable BAR is applicable in this
> variant driver as the BAR represents the device memory. Should we be
> exposing such feature as unsupported for this variant driver?

Bar SIZING, not resizing. This is the standard in-band mechanism for
determining the BAR size as described in PCIe 6.0.1, 7.5.1.2.1. QEMU
makes use of the region size but does rely on the BAR flags when
registering the BAR into QEMU as described above. Additionally,
vfio-pci-core supports this in-band sizing mechanism for physical BARs.
A variant driver which does not implement config space BAR sizing for a
virtual BAR is arguably not presenting a PCI compatible config space
where a non-QEMU userspace may depend on standard PCI behavior here.
Thanks,

Alex

2023-10-24 14:03:54

by Ankit Agrawal

[permalink] [raw]
Subject: Re: [PATCH v12 1/1] vfio/nvgpu: Add vfio pci variant module for grace hopper

>> > After looking at Yishai's virtio-vfio-pci driver where BAR0 is emulated
>> > as an IO Port BAR, it occurs to me that there's no config space
>> > emulation of BAR2 (or BAR3) here.? Doesn't this mean that QEMU registers
>> > the BAR as 32-bit, non-prefetchable?? ie. VFIOBAR.type & .mem64 are
>> > wrong?
>>
>> Maybe I didn't understand the question, but the PCI config space read/write
>> would still be handled by vfio_pci_core_read/write() which returns the
>> appropriate flags. I have checked that the device BARs are 64b and
>> prefetchable in the VM.
>
> vfio_pci_core_read/write() accesses the physical device, which doesn't
> implement BAR2.? Why would an unimplemented BAR2 on the physical device
> report 64-bit, prefetchable?
>
> QEMU records VFIOBAR.type and .mem64 from reading the BAR register in
> vfio_bar_prepare() and passes this type to pci_register_bar() in
> vfio_bar_register().? Without an implementation of a config space read
> op in the variant driver and with no physical implementation of BAR2 on
> the device, I don't see how we get correct values in these fields.

I think I see the cause of confusion. There are real PCIe compliant BARs
present on the device, just that it isn't being used once the C2C
interconnect is active. The BARs are 64b prefetchable. Here it the lspci
snippet of the device on the host.
# lspci -v -s 9:1:0.0
0009:01:00.0 3D controller: NVIDIA Corporation Device 2342 (rev a1)
Subsystem: NVIDIA Corporation Device 16eb
Physical Slot: 0-5
Flags: bus master, fast devsel, latency 0, IRQ 263, NUMA node 0, IOMMU group 19
Memory at 661002000000 (64-bit, prefetchable) [size=16M]
Memory at 662000000000 (64-bit, prefetchable) [size=128G]
Memory at 661000000000 (64-bit, prefetchable) [size=32M]

I suppose this answers the BAR sizing question as well?

2023-10-24 14:30:00

by Alex Williamson

[permalink] [raw]
Subject: Re: [PATCH v12 1/1] vfio/nvgpu: Add vfio pci variant module for grace hopper

On Tue, 24 Oct 2023 14:03:25 +0000
Ankit Agrawal <[email protected]> wrote:

> >> > After looking at Yishai's virtio-vfio-pci driver where BAR0 is emulated
> >> > as an IO Port BAR, it occurs to me that there's no config space
> >> > emulation of BAR2 (or BAR3) here.  Doesn't this mean that QEMU registers
> >> > the BAR as 32-bit, non-prefetchable?  ie. VFIOBAR.type & .mem64 are
> >> > wrong?
> >>
> >> Maybe I didn't understand the question, but the PCI config space read/write
> >> would still be handled by vfio_pci_core_read/write() which returns the
> >> appropriate flags. I have checked that the device BARs are 64b and
> >> prefetchable in the VM.
> >
> > vfio_pci_core_read/write() accesses the physical device, which doesn't
> > implement BAR2.  Why would an unimplemented BAR2 on the physical device
> > report 64-bit, prefetchable?
> >
> > QEMU records VFIOBAR.type and .mem64 from reading the BAR register in
> > vfio_bar_prepare() and passes this type to pci_register_bar() in
> > vfio_bar_register().  Without an implementation of a config space read
> > op in the variant driver and with no physical implementation of BAR2 on
> > the device, I don't see how we get correct values in these fields.
>
> I think I see the cause of confusion. There are real PCIe compliant BARs
> present on the device, just that it isn't being used once the C2C
> interconnect is active. The BARs are 64b prefetchable. Here it the lspci
> snippet of the device on the host.
> # lspci -v -s 9:1:0.0
> 0009:01:00.0 3D controller: NVIDIA Corporation Device 2342 (rev a1)
> Subsystem: NVIDIA Corporation Device 16eb
> Physical Slot: 0-5
> Flags: bus master, fast devsel, latency 0, IRQ 263, NUMA node 0, IOMMU group 19
> Memory at 661002000000 (64-bit, prefetchable) [size=16M]
> Memory at 662000000000 (64-bit, prefetchable) [size=128G]
> Memory at 661000000000 (64-bit, prefetchable) [size=32M]
>
> I suppose this answers the BAR sizing question as well?

Does this BAR2 size match the size we're reporting for the region? Now
I'm confused why we need to intercept the BAR2 region info if there's
physically a real BAR behind it. Thanks,

Alex

2023-10-25 08:29:14

by Tian, Kevin

[permalink] [raw]
Subject: RE: [PATCH v12 1/1] vfio/nvgpu: Add vfio pci variant module for grace hopper

> From: Alex Williamson <[email protected]>
> Sent: Tuesday, October 24, 2023 10:29 PM
>
> On Tue, 24 Oct 2023 14:03:25 +0000
> Ankit Agrawal <[email protected]> wrote:
>
> > >> > After looking at Yishai's virtio-vfio-pci driver where BAR0 is emulated
> > >> > as an IO Port BAR, it occurs to me that there's no config space
> > >> > emulation of BAR2 (or BAR3) here.  Doesn't this mean that QEMU
> registers
> > >> > the BAR as 32-bit, non-prefetchable?  ie. VFIOBAR.type & .mem64 are
> > >> > wrong?
> > >>
> > >> Maybe I didn't understand the question, but the PCI config space
> read/write
> > >> would still be handled by vfio_pci_core_read/write() which returns the
> > >> appropriate flags. I have checked that the device BARs are 64b and
> > >> prefetchable in the VM.
> > >
> > > vfio_pci_core_read/write() accesses the physical device, which doesn't
> > > implement BAR2.  Why would an unimplemented BAR2 on the physical
> device
> > > report 64-bit, prefetchable?
> > >
> > > QEMU records VFIOBAR.type and .mem64 from reading the BAR register
> in
> > > vfio_bar_prepare() and passes this type to pci_register_bar() in
> > > vfio_bar_register().  Without an implementation of a config space read
> > > op in the variant driver and with no physical implementation of BAR2 on
> > > the device, I don't see how we get correct values in these fields.
> >
> > I think I see the cause of confusion. There are real PCIe compliant BARs
> > present on the device, just that it isn't being used once the C2C
> > interconnect is active. The BARs are 64b prefetchable. Here it the lspci
> > snippet of the device on the host.
> > # lspci -v -s 9:1:0.0
> > 0009:01:00.0 3D controller: NVIDIA Corporation Device 2342 (rev a1)
> > Subsystem: NVIDIA Corporation Device 16eb
> > Physical Slot: 0-5
> > Flags: bus master, fast devsel, latency 0, IRQ 263, NUMA node 0,
> IOMMU group 19
> > Memory at 661002000000 (64-bit, prefetchable) [size=16M]
> > Memory at 662000000000 (64-bit, prefetchable) [size=128G]
> > Memory at 661000000000 (64-bit, prefetchable) [size=32M]
> >
> > I suppose this answers the BAR sizing question as well?
>
> Does this BAR2 size match the size we're reporting for the region? Now
> I'm confused why we need to intercept the BAR2 region info if there's
> physically a real BAR behind it. Thanks,
>

same confusion.

probably vfio-pci-core can include a helper for cfg space emulation
on emulated BARs to be used by all variant drivers in that category?

btw intel vgpu also includes an emulation of BAR sizing. same for
future SIOV devices. so there sounds like a general requirement but
of course sharing it between vfio-pci and mdev/siov would be more
difficult.

2023-10-25 12:43:37

by Ankit Agrawal

[permalink] [raw]
Subject: Re: [PATCH v12 1/1] vfio/nvgpu: Add vfio pci variant module for grace hopper

While the physical BAR is present on the device, it is not being used on the host
system. The access to the device memory region on the host occur through the
C2C interconnect link (not the PCIe) and is present for access as a separate memory
region in the physical address space on the host. The variant driver queries this range
from the host ACPI DSD tables.

Now, this device memory region on the host is exposed as a device BAR in the VM.
So the device BAR in the VM is actually mapped to the device memory region in the
physical address space (and not to the physical BAR) on the host. The config space
accesses to the device however, are still going to the physical BAR on the host.

> Does this BAR2 size match the size we're reporting for the region?? Now
> I'm confused why we need to intercept the BAR2 region info if there's
> physically a real BAR behind it.? Thanks,

Yes, it does match the size being reported through region info. But the region
info ioctl is still intercepted to provide additional cap to establish the sparse
mapping. Why we do sparse mapping? The actual device memory size is not
power-of-2 aligned (a requirement for a BAR). So we roundup to the next
power-of-2 value and report the size as such. Then we utilize sparse mapping
to show only the actual size of the device memory as mappable.

2023-10-25 14:21:27

by Alex Williamson

[permalink] [raw]
Subject: Re: [PATCH v12 1/1] vfio/nvgpu: Add vfio pci variant module for grace hopper

On Wed, 25 Oct 2023 12:43:24 +0000
Ankit Agrawal <[email protected]> wrote:

> While the physical BAR is present on the device, it is not being used on the host
> system. The access to the device memory region on the host occur through the
> C2C interconnect link (not the PCIe) and is present for access as a separate memory
> region in the physical address space on the host. The variant driver queries this range
> from the host ACPI DSD tables.

BTW, it's still never been answered why the latest QEMU series dropped
the _DSD support.

> Now, this device memory region on the host is exposed as a device BAR in the VM.
> So the device BAR in the VM is actually mapped to the device memory region in the
> physical address space (and not to the physical BAR) on the host. The config space
> accesses to the device however, are still going to the physical BAR on the host.
>
> > Does this BAR2 size match the size we're reporting for the region?  Now
> > I'm confused why we need to intercept the BAR2 region info if there's
> > physically a real BAR behind it.  Thanks,
>
> Yes, it does match the size being reported through region info. But the region
> info ioctl is still intercepted to provide additional cap to establish the sparse
> mapping. Why we do sparse mapping? The actual device memory size is not
> power-of-2 aligned (a requirement for a BAR). So we roundup to the next
> power-of-2 value and report the size as such. Then we utilize sparse mapping
> to show only the actual size of the device memory as mappable.

Yes, it's clear to me why we need the sparse mapping and why we
intercept the accesses, but the fact that there's an underlying
physical BAR of the same size in config space has been completely
absent in any previous discussions.

In light of that, I don't think we should be independently calculating
the BAR2 region size using roundup_pow_of_two(nvdev->memlength).
Instead we should be using pci_resource_len() of the physical BAR2 to
make it evident that this relationship exists.

The comments throughout should also be updated to reflect this as
currently they're written as if there is no physical BAR2 and we're
making a completely independent decision relative to BAR2 sizing. A
comment should also be added to nvgrace_gpu_vfio_pci_read/write()
explaining that the physical BAR2 provides the correct behavior
relative to config space accesses.

The probe function should also fail if pci_resource_len() for BAR2 is
not sufficient for the coherent memory region. Thanks,

Alex

2023-10-25 14:31:13

by Alex Williamson

[permalink] [raw]
Subject: Re: [PATCH v12 1/1] vfio/nvgpu: Add vfio pci variant module for grace hopper

On Wed, 25 Oct 2023 08:28:44 +0000
"Tian, Kevin" <[email protected]> wrote:

> > From: Alex Williamson <[email protected]>
> > Sent: Tuesday, October 24, 2023 10:29 PM
> >
> > On Tue, 24 Oct 2023 14:03:25 +0000
> > Ankit Agrawal <[email protected]> wrote:
> >
> > > >> > After looking at Yishai's virtio-vfio-pci driver where BAR0 is emulated
> > > >> > as an IO Port BAR, it occurs to me that there's no config space
> > > >> > emulation of BAR2 (or BAR3) here.  Doesn't this mean that QEMU
> > registers
> > > >> > the BAR as 32-bit, non-prefetchable?  ie. VFIOBAR.type & .mem64 are
> > > >> > wrong?
> > > >>
> > > >> Maybe I didn't understand the question, but the PCI config space
> > read/write
> > > >> would still be handled by vfio_pci_core_read/write() which returns the
> > > >> appropriate flags. I have checked that the device BARs are 64b and
> > > >> prefetchable in the VM.
> > > >
> > > > vfio_pci_core_read/write() accesses the physical device, which doesn't
> > > > implement BAR2.  Why would an unimplemented BAR2 on the physical
> > device
> > > > report 64-bit, prefetchable?
> > > >
> > > > QEMU records VFIOBAR.type and .mem64 from reading the BAR register
> > in
> > > > vfio_bar_prepare() and passes this type to pci_register_bar() in
> > > > vfio_bar_register().  Without an implementation of a config space read
> > > > op in the variant driver and with no physical implementation of BAR2 on
> > > > the device, I don't see how we get correct values in these fields.
> > >
> > > I think I see the cause of confusion. There are real PCIe compliant BARs
> > > present on the device, just that it isn't being used once the C2C
> > > interconnect is active. The BARs are 64b prefetchable. Here it the lspci
> > > snippet of the device on the host.
> > > # lspci -v -s 9:1:0.0
> > > 0009:01:00.0 3D controller: NVIDIA Corporation Device 2342 (rev a1)
> > > Subsystem: NVIDIA Corporation Device 16eb
> > > Physical Slot: 0-5
> > > Flags: bus master, fast devsel, latency 0, IRQ 263, NUMA node 0,
> > IOMMU group 19
> > > Memory at 661002000000 (64-bit, prefetchable) [size=16M]
> > > Memory at 662000000000 (64-bit, prefetchable) [size=128G]
> > > Memory at 661000000000 (64-bit, prefetchable) [size=32M]
> > >
> > > I suppose this answers the BAR sizing question as well?
> >
> > Does this BAR2 size match the size we're reporting for the region? Now
> > I'm confused why we need to intercept the BAR2 region info if there's
> > physically a real BAR behind it. Thanks,
> >
>
> same confusion.
>
> probably vfio-pci-core can include a helper for cfg space emulation
> on emulated BARs to be used by all variant drivers in that category?
>
> btw intel vgpu also includes an emulation of BAR sizing. same for
> future SIOV devices. so there sounds like a general requirement but
> of course sharing it between vfio-pci and mdev/siov would be more
> difficult.

Yes, I see a need for this in the virtio-vfio-pci driver as well. It
would simplify config emulation a lot if the variant driver could
manipulate the perm_bits.virt and .write bit arrays and simply update
vconfig for things like device-id and revision. Thanks,

Alex

2023-10-25 17:16:05

by Ankit Agrawal

[permalink] [raw]
Subject: Re: [PATCH v12 1/1] vfio/nvgpu: Add vfio pci variant module for grace hopper

> BTW, it's still never been answered why the latest QEMU series dropped
> the _DSD support.

The _DSD keys were there in v1 to communicate the PXM start id and the
count associated with the device to the VM kernel. In v2, we proposed an
alternative approach to leverage the Generic Initiator (GI) Affinity structure
in SRAT (ACPI Spec 6.5, Section 5.2.16.6) to create NUMA nodes. GI structure
allows an association between a GI (GPU in this case) and proximity domains.
So we create 8 GI structures with a unique PXM Id and the device BDF. This
removes the need for DSD keys as the VM kernel could parse the GI structures
and identify the PXM IDs associated with the device using the BDF.
(E.g. https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/amd/amdkfd/kfd_crat.c#L1938)

> In light of that, I don't think we should be independently calculating
> the BAR2 region size using roundup_pow_of_two(nvdev->memlength).
> Instead we should be using pci_resource_len() of the physical BAR2 to
> make it evident that this relationship exists.

Sure, I will make the change in the next posting.

> The comments throughout should also be updated to reflect this as
> currently they're written as if there is no physical BAR2 and we're
> making a completely independent decision relative to BAR2 sizing.? A
> comment should also be added to nvgrace_gpu_vfio_pci_read/write()
> explaining that the physical BAR2 provides the correct behavior
> relative to config space accesses.

Yeah, will update the comments.

> The probe function should also fail if pci_resource_len() for BAR2 is
> not sufficient for the coherent memory region.? Thanks,

Ack.

2023-11-04 09:36:24

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v12 1/1] vfio/nvgpu: Add vfio pci variant module for grace hopper

Hi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on awilliam-vfio/for-linus]
[also build test WARNING on linus/master v6.6 next-20231103]
[cannot apply to awilliam-vfio/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/ankita-nvidia-com/vfio-nvgpu-Add-vfio-pci-variant-module-for-grace-hopper/20231017-131546
base: https://github.com/awilliam/linux-vfio.git for-linus
patch link: https://lore.kernel.org/r/20231015163047.20391-1-ankita%40nvidia.com
patch subject: [PATCH v12 1/1] vfio/nvgpu: Add vfio pci variant module for grace hopper
config: powerpc-allmodconfig (https://download.01.org/0day-ci/archive/20231104/[email protected]/config)
compiler: powerpc64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231104/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All warnings (new ones prefixed by >>):

>> drivers/vfio/pci/nvgrace-gpu/main.c:226:9: warning: no previous prototype for 'nvgrace_gpu_read_mem' [-Wmissing-prototypes]
226 | ssize_t nvgrace_gpu_read_mem(void __user *buf, size_t count, loff_t *ppos,
| ^~~~~~~~~~~~~~~~~~~~
>> drivers/vfio/pci/nvgrace-gpu/main.c:298:9: warning: no previous prototype for 'nvgrace_gpu_write_mem' [-Wmissing-prototypes]
298 | ssize_t nvgrace_gpu_write_mem(size_t count, loff_t *ppos, const void __user *buf,
| ^~~~~~~~~~~~~~~~~~~~~


vim +/nvgrace_gpu_read_mem +226 drivers/vfio/pci/nvgrace-gpu/main.c

214
215 /*
216 * Read count bytes from the device memory at an offset. The actual device
217 * memory size (available) may not be a power-of-2. So the driver fakes
218 * the size to a power-of-2 (reported) when exposing to a user space driver.
219 *
220 * Read request beyond the actual device size is filled with ~0, while
221 * those beyond the actual reported size is skipped.
222 *
223 * A read from a negative or an offset greater than reported size, a negative
224 * count are considered error conditions and returned with an -EINVAL.
225 */
> 226 ssize_t nvgrace_gpu_read_mem(void __user *buf, size_t count, loff_t *ppos,
227 struct nvgrace_gpu_vfio_pci_core_device *nvdev)
228 {
229 u64 offset = *ppos & VFIO_PCI_OFFSET_MASK;
230 size_t mem_count, i, bar_size = roundup_pow_of_two(nvdev->memlength);
231 u8 val = 0xFF;
232
233 if (offset >= bar_size)
234 return -EINVAL;
235
236 /* Clip short the read request beyond reported BAR size */
237 count = min(count, bar_size - (size_t)offset);
238
239 /*
240 * Determine how many bytes to be actually read from the device memory.
241 * Read request beyond the actual device memory size is filled with ~0,
242 * while those beyond the actual reported size is skipped.
243 */
244 if (offset >= nvdev->memlength)
245 mem_count = 0;
246 else
247 mem_count = min(count, nvdev->memlength - (size_t)offset);
248
249 /*
250 * Handle read on the BAR2 region. Map to the target device memory
251 * physical address and copy to the request read buffer.
252 */
253 if (copy_to_user(buf, (u8 *)nvdev->memmap + offset, mem_count))
254 return -EFAULT;
255
256 /*
257 * Only the device memory present on the hardware is mapped, which may
258 * not be power-of-2 aligned. A read to an offset beyond the device memory
259 * size is filled with ~0.
260 */
261 for (i = mem_count; i < count; i++)
262 put_user(val, (unsigned char __user *)(buf + i));
263
264 *ppos += count;
265 return count;
266 }
267
268 static ssize_t nvgrace_gpu_vfio_pci_read(struct vfio_device *core_vdev,
269 char __user *buf, size_t count, loff_t *ppos)
270 {
271 unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos);
272 struct nvgrace_gpu_vfio_pci_core_device *nvdev = container_of(
273 core_vdev, struct nvgrace_gpu_vfio_pci_core_device, core_device.vdev);
274 int ret;
275
276 if (index == VFIO_PCI_BAR2_REGION_INDEX) {
277 ret = nvgrace_gpu_memmap(nvdev);
278 if (ret)
279 return ret;
280
281 return nvgrace_gpu_read_mem(buf, count, ppos, nvdev);
282 }
283
284 return vfio_pci_core_read(core_vdev, buf, count, ppos);
285 }
286
287 /*
288 * Write count bytes to the device memory at a given offset. The actual device
289 * memory size (available) may not be a power-of-2. So the driver fakes the
290 * size to a power-of-2 (reported) when exposing to a user space driver.
291 *
292 * Write request beyond the actual device size are dropped, while those
293 * beyond the actual reported size are skipped entirely.
294 *
295 * A write to a negative or an offset greater than the reported size, a
296 * negative count are considered error conditions and returned with an -EINVAL.
297 */
> 298 ssize_t nvgrace_gpu_write_mem(size_t count, loff_t *ppos, const void __user *buf,
299 struct nvgrace_gpu_vfio_pci_core_device *nvdev)
300 {
301 u64 offset = *ppos & VFIO_PCI_OFFSET_MASK;
302 size_t mem_count, bar_size = roundup_pow_of_two(nvdev->memlength);
303
304 if (offset >= bar_size)
305 return -EINVAL;
306
307 /* Clip short the write request beyond reported BAR size */
308 count = min(count, bar_size - (size_t)offset);
309
310 /*
311 * Determine how many bytes to be actually written to the device memory.
312 * Do not write to the offset beyond available size.
313 */
314 if (offset >= nvdev->memlength)
315 goto exitfn;
316
317 mem_count = min(count, nvdev->memlength - (size_t)offset);
318
319 /*
320 * Only the device memory present on the hardware is mapped, which may
321 * not be power-of-2 aligned. A write to the BAR2 region implies an
322 * access outside the available device memory on the hardware. Drop
323 * those write requests.
324 */
325 if (copy_from_user((u8 *)nvdev->memmap + offset, buf, mem_count))
326 return -EFAULT;
327
328 exitfn:
329 *ppos += count;
330 return count;
331 }
332

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki