LinuxLists.cc - [PATCH v17 0/3] vfio/nvgrace-gpu: Add vfio pci variant module for grace hopper

[permalink] [raw]

Subject: [PATCH v17 2/3] vfio/pci: rename and export range_intesect_range

From: Ankit Agrawal <[email protected]>

range_intesect_range determines an overlap between two ranges. If an
overlap, the helper function returns the overlapping offset and size.

The VFIO PCI variant driver emulates the PCI config space BAR offset
registers. These offset may be accessed for read/write with a variety
of lengths including sub-word sizes from sub-word offsets. The driver
makes use of this helper function to read/write the targeted part of
the emulated register.

Make this a vfio_pci_core function, rename and export as GPL. Also
update references in virtio driver.

Signed-off-by: Ankit Agrawal <[email protected]>
---
drivers/vfio/pci/vfio_pci_config.c | 45 +++++++++++++++++++
drivers/vfio/pci/virtio/main.c | 72 +++++++++++-------------------
include/linux/vfio_pci_core.h | 5 +++
3 files changed, 76 insertions(+), 46 deletions(-)

diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c
index 672a1804af6a..4fc3c605af13 100644
--- a/drivers/vfio/pci/vfio_pci_config.c
+++ b/drivers/vfio/pci/vfio_pci_config.c
@@ -1966,3 +1966,48 @@ ssize_t vfio_pci_config_rw(struct vfio_pci_core_device *vdev, char __user *buf,

return done;
}
+
+/**
+ * vfio_pci_core_range_intersect_range() - Determine overlap between a buffer
+ * and register offset ranges.
+ * @range1_start: start offset of the buffer
+ * @count1: number of buffer bytes.
+ * @range2_start: start register offset
+ * @count2: number of bytes of register
+ * @start_offset: start offset of overlap start in the buffer
+ * @intersect_count: number of overlapping bytes
+ * @register_offset: start offset of overlap start in register
+ *
+ * The function determines an overlap between a register and a buffer.
+ * range1 represents the buffer and range2 represents register.
+ *
+ * Returns: true if there is overlap, false if not.
+ * The overlap start and range is returned through function args.
+ */
+bool vfio_pci_core_range_intersect_range(loff_t range1_start, size_t count1,
+ loff_t range2_start, size_t count2,
+ loff_t *start_offset,
+ size_t *intersect_count,
+ size_t *register_offset)
+{
+ if (range1_start <= range2_start &&
+ range1_start + count1 > range2_start) {
+ *start_offset = range2_start - range1_start;
+ *intersect_count = min_t(size_t, count2,
+ range1_start + count1 - range2_start);
+ *register_offset = 0;
+ return true;
+ }
+
+ if (range1_start > range2_start &&
+ range1_start < range2_start + count2) {
+ *start_offset = 0;
+ *intersect_count = min_t(size_t, count1,
+ range2_start + count2 - range1_start);
+ *register_offset = range1_start - range2_start;
+ return true;
+ }
+
+ return false;
+}
+EXPORT_SYMBOL_GPL(vfio_pci_core_range_intersect_range);
diff --git a/drivers/vfio/pci/virtio/main.c b/drivers/vfio/pci/virtio/main.c
index d5af683837d3..b5d3a8c5bbc9 100644
--- a/drivers/vfio/pci/virtio/main.c
+++ b/drivers/vfio/pci/virtio/main.c
@@ -132,33 +132,6 @@ virtiovf_pci_bar0_rw(struct virtiovf_pci_core_device *virtvdev,
return ret ? ret : count;
}

-static bool range_intersect_range(loff_t range1_start, size_t count1,
- loff_t range2_start, size_t count2,
- loff_t *start_offset,
- size_t *intersect_count,
- size_t *register_offset)
-{
- if (range1_start <= range2_start &&
- range1_start + count1 > range2_start) {
- *start_offset = range2_start - range1_start;
- *intersect_count = min_t(size_t, count2,
- range1_start + count1 - range2_start);
- *register_offset = 0;
- return true;
- }
-
- if (range1_start > range2_start &&
- range1_start < range2_start + count2) {
- *start_offset = 0;
- *intersect_count = min_t(size_t, count1,
- range2_start + count2 - range1_start);
- *register_offset = range1_start - range2_start;
- return true;
- }
-
- return false;
-}
-
static ssize_t virtiovf_pci_read_config(struct vfio_device *core_vdev,
char __user *buf, size_t count,
loff_t *ppos)
@@ -178,16 +151,18 @@ static ssize_t virtiovf_pci_read_config(struct vfio_device *core_vdev,
if (ret < 0)
return ret;

- if (range_intersect_range(pos, count, PCI_DEVICE_ID, sizeof(val16),
- &copy_offset, &copy_count, &register_offset)) {
+ if (vfio_pci_core_range_intersect_range(pos, count, PCI_DEVICE_ID,
+ sizeof(val16), &copy_offset,
+ &copy_count, &register_offset)) {
val16 = cpu_to_le16(VIRTIO_TRANS_ID_NET);
if (copy_to_user(buf + copy_offset, (void *)&val16 + register_offset, copy_count))
return -EFAULT;
}

if ((le16_to_cpu(virtvdev->pci_cmd) & PCI_COMMAND_IO) &&
- range_intersect_range(pos, count, PCI_COMMAND, sizeof(val16),
- &copy_offset, &copy_count, &register_offset)) {
+ vfio_pci_core_range_intersect_range(pos, count, PCI_COMMAND,
+ sizeof(val16), &copy_offset,
+ &copy_count, &register_offset)) {
if (copy_from_user((void *)&val16 + register_offset, buf + copy_offset,
copy_count))
return -EFAULT;
@@ -197,16 +172,18 @@ static ssize_t virtiovf_pci_read_config(struct vfio_device *core_vdev,
return -EFAULT;
}

- if (range_intersect_range(pos, count, PCI_REVISION_ID, sizeof(val8),
- &copy_offset, &copy_count, &register_offset)) {
+ if (vfio_pci_core_range_intersect_range(pos, count, PCI_REVISION_ID,
+ sizeof(val8), &copy_offset,
+ &copy_count, &register_offset)) {
/* Transional needs to have revision 0 */
val8 = 0;
if (copy_to_user(buf + copy_offset, &val8, copy_count))
return -EFAULT;
}

- if (range_intersect_range(pos, count, PCI_BASE_ADDRESS_0, sizeof(val32),
- &copy_offset, &copy_count, &register_offset)) {
+ if (vfio_pci_core_range_intersect_range(pos, count, PCI_BASE_ADDRESS_0,
+ sizeof(val32), &copy_offset,
+ &copy_count, &register_offset)) {
u32 bar_mask = ~(virtvdev->bar0_virtual_buf_size - 1);
u32 pci_base_addr_0 = le32_to_cpu(virtvdev->pci_base_addr_0);

@@ -215,8 +192,9 @@ static ssize_t virtiovf_pci_read_config(struct vfio_device *core_vdev,
return -EFAULT;
}

- if (range_intersect_range(pos, count, PCI_SUBSYSTEM_ID, sizeof(val16),
- &copy_offset, &copy_count, &register_offset)) {
+ if (vfio_pci_core_range_intersect_range(pos, count, PCI_SUBSYSTEM_ID,
+ sizeof(val16), &copy_offset,
+ &copy_count, &register_offset)) {
/*
* Transitional devices use the PCI subsystem device id as
* virtio device id, same as legacy driver always did.
@@ -227,8 +205,9 @@ static ssize_t virtiovf_pci_read_config(struct vfio_device *core_vdev,
return -EFAULT;
}

- if (range_intersect_range(pos, count, PCI_SUBSYSTEM_VENDOR_ID, sizeof(val16),
- &copy_offset, &copy_count, &register_offset)) {
+ if (vfio_pci_core_range_intersect_range(pos, count, PCI_SUBSYSTEM_VENDOR_ID,
+ sizeof(val16), &copy_offset,
+ &copy_count, &register_offset)) {
val16 = cpu_to_le16(PCI_VENDOR_ID_REDHAT_QUMRANET);
if (copy_to_user(buf + copy_offset, (void *)&val16 + register_offset,
copy_count))
@@ -270,19 +249,20 @@ static ssize_t virtiovf_pci_write_config(struct vfio_device *core_vdev,
loff_t copy_offset;
size_t copy_count;

- if (range_intersect_range(pos, count, PCI_COMMAND, sizeof(virtvdev->pci_cmd),
- &copy_offset, &copy_count,
- &register_offset)) {
+ if (vfio_pci_core_range_intersect_range(pos, count, PCI_COMMAND,
+ sizeof(virtvdev->pci_cmd),
+ &copy_offset, &copy_count,
+ &register_offset)) {
if (copy_from_user((void *)&virtvdev->pci_cmd + register_offset,
buf + copy_offset,
copy_count))
return -EFAULT;
}

- if (range_intersect_range(pos, count, PCI_BASE_ADDRESS_0,
- sizeof(virtvdev->pci_base_addr_0),
- &copy_offset, &copy_count,
- &register_offset)) {
+ if (vfio_pci_core_range_intersect_range(pos, count, PCI_BASE_ADDRESS_0,
+ sizeof(virtvdev->pci_base_addr_0),
+ &copy_offset, &copy_count,
+ &register_offset)) {
if (copy_from_user((void *)&virtvdev->pci_base_addr_0 + register_offset,
buf + copy_offset,
copy_count))
diff --git a/include/linux/vfio_pci_core.h b/include/linux/vfio_pci_core.h
index cf9480a31f3e..1a056b5ef417 100644
--- a/include/linux/vfio_pci_core.h
+++ b/include/linux/vfio_pci_core.h
@@ -134,6 +134,11 @@ ssize_t vfio_pci_core_do_io_rw(struct vfio_pci_core_device *vdev, bool test_mem,
void __iomem *io, char __user *buf,
loff_t off, size_t count, size_t x_start,
size_t x_end, bool iswrite);
+bool vfio_pci_core_range_intersect_range(loff_t range1_start, size_t count1,
+ loff_t range2_start, size_t count2,
+ loff_t *start_offset,
+ size_t *intersect_count,
+ size_t *register_offset);
#define VFIO_IOWRITE_DECLATION(size) \
int vfio_pci_core_iowrite##size(struct vfio_pci_core_device *vdev, \
bool test_mem, u##size val, void __iomem *io);
--
2.34.1

2024-02-05 23:04:27

[permalink] [raw]

Subject: [PATCH v17 3/3] vfio/nvgrace-gpu: 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 chip-to-chip cache coherent interconnect.

The device is peculiar compared to a real PCI device in that whilst
there is a real 64b PCI BAR1 (comprising region 2 & region 3) on the
device, it is not used to access device memory once the faster
chip-to-chip interconnect is initialized (occurs at the time of host
system boot). The device memory is accessed instead using the chip-to-chip
interconnect that is exposed as a contiguous physically addressable
region on the host. This device memory aperture can be obtained from host
ACPI table using device_property_read_u64(), according to the FW
specification. Since the device memory is cache coherent with the CPU,
it can be mmap into the user VMA with a cacheable mapping using
remap_pfn_range() and used like a regular RAM. The device memory
is not added to the host kernel, but mapped directly as this reduces
memory wastage due to struct pages.

There is also a requirement of a reserved 1G uncached region (termed as
resmem) to support the Multi-Instance GPU (MIG) feature [1]. This is
to work around a HW defect. Based on [2], the requisite properties
(uncached, unaligned access) can be achieved through a VM mapping (S1)
of NORMAL_NC and host (S2) mapping with MemAttr[2:0]=0b101. To provide
a different non-cached property to the reserved 1G region, it needs to
be carved out from the device memory and mapped as a separate region
in Qemu VMA with pgprot_writecombine(). pgprot_writecombine() sets the
Qemu VMA page properties (pgprot) as NORMAL_NC.

Provide a VFIO PCI variant driver that adapts the unique device memory
representation into a more standard PCI representation facing userspace.

The variant driver exposes these two regions - the non-cached reserved
(resmem) and the cached rest of the device memory (termed as usemem) as
separate VFIO 64b BAR regions. This is divergent from the baremetal
approach, where the device memory is exposed as a device memory region.
The decision for a different approach was taken in view of the fact that
it would necessiate additional code in Qemu to discover and insert those
regions in the VM IPA, along with the additional VM ACPI DSDT changes to
communicate the device memory region IPA to the VM workloads. Moreover,
this behavior would have to be added to a variety of emulators (beyond
top of tree Qemu) out there desiring grace hopper support.

Since the device implements 64-bit BAR0, the VFIO PCI variant driver
maps the uncached carved out region to the next available PCI BAR (i.e.
comprising of region 2 and 3). The cached device memory aperture is
assigned BAR region 4 and 5. Qemu will then naturally generate a PCI
device in the VM with the uncached aperture reported as BAR2 region,
the cacheable as BAR4. The variant driver provides emulation for these
fake BARs' PCI config space offset registers.

The hardware ensures that the system does not crash when the memory
is accessed with the memory enable turned off. It synthesis ~0 reads
and dropped writes on such access. So there is no need to support the
disablement/enablement of BAR through PCI_COMMAND config space register.

The memory layout on the host looks like the following:
devmem (memlength)
|--------------------------------------------------|
|-------------cached------------------------|--NC--|
| |
usemem.phys/memphys resmem.phys

PCI BARs need to be 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 [6] 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.

Note that the usemem memory is added by the VM Nvidia device driver [5]
to the VM kernel as memblocks. Hence make the usable memory size memblock
aligned.

Currently there is no provision in KVM for a S2 mapping with
MemAttr[2:0]=0b101, but there is an ongoing effort to provide the same [3].
As previously mentioned, resmem is mapped pgprot_writecombine(), that
sets the Qemu VMA page properties (pgprot) as NORMAL_NC. Using the
proposed changes in [4] and [3], KVM marks the region with
MemAttr[2:0]=0b101 in S2.

If the bare metal properties are not present, the driver registers the
vfio-pci-core function pointers.

This goes along with a qemu series [6] 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.

[1] https://www.nvidia.com/en-in/technologies/multi-instance-gpu/
[2] section D8.5.5 of https://developer.arm.com/documentation/ddi0487/latest/
[3] https://lore.kernel.org/all/[email protected]/
[4] https://lore.kernel.org/all/[email protected]/
[5] https://github.com/NVIDIA/open-gpu-kernel-modules
[6] https://lore.kernel.org/all/[email protected]/

Signed-off-by: Aniket Agashe <[email protected]>
Signed-off-by: Ankit Agrawal <[email protected]>
---
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 | 856 ++++++++++++++++++++++++++
6 files changed, 879 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 8999497011a2..529ec8966f58 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -23103,6 +23103,12 @@ L: [email protected]
S: Maintained
F: drivers/vfio/platform/

+VFIO NVIDIA GRACE GPU DRIVER
+M: Ankit Agrawal <[email protected]>
+L: [email protected]
+S: Supported
+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 18c397df566d..15821a2d77d2 100644
--- a/drivers/vfio/pci/Kconfig
+++ b/drivers/vfio/pci/Kconfig
@@ -67,4 +67,6 @@ source "drivers/vfio/pci/pds/Kconfig"

source "drivers/vfio/pci/virtio/Kconfig"

+source "drivers/vfio/pci/nvgrace-gpu/Kconfig"
+
endmenu
diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile
index 046139a4eca5..ce7a61f1d912 100644
--- a/drivers/vfio/pci/Makefile
+++ b/drivers/vfio/pci/Makefile
@@ -15,3 +15,5 @@ obj-$(CONFIG_HISI_ACC_VFIO_PCI) += hisilicon/
obj-$(CONFIG_PDS_VFIO_PCI) += pds/

obj-$(CONFIG_VIRTIO_VFIO_PCI) += virtio/
+
+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..6279af2bc6b8
--- /dev/null
+++ b/drivers/vfio/pci/nvgrace-gpu/main.c
@@ -0,0 +1,856 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2024, NVIDIA CORPORATION & AFFILIATES. All rights reserved
+ */
+
+#include <linux/vfio_pci_core.h>
+
+/*
+ * The device memory usable to the workloads running in the VM is cached
+ * and showcased as a 64b device BAR (comprising of BAR4 and BAR5 region)
+ * to the VM and is represented as usemem.
+ * Moreover, the VM GPU device driver needs a non-cacheable region to
+ * support the MIG feature. This region is also exposed as a 64b BAR
+ * (comprising of BAR2 and BAR3 region) and represented as resmem.
+ */
+#define RESMEM_REGION_INDEX VFIO_PCI_BAR2_REGION_INDEX
+#define USEMEM_REGION_INDEX VFIO_PCI_BAR4_REGION_INDEX
+
+/* Memory size expected as non cached and reserved by the VM driver */
+#define RESMEM_SIZE 0x40000000
+#define MEMBLK_SIZE 0x20000000
+
+/*
+ * The state of the two device memory region - resmem and usemem - is
+ * saved as struct mem_region.
+ */
+struct mem_region {
+ phys_addr_t memphys; /* Base physical address of the region */
+ size_t memlength; /* Region size */
+ size_t bar_size; /* Reported region BAR size */
+ __le64 bar_val; /* Emulated BAR offset registers */
+ union {
+ void *memaddr;
+ void __iomem *ioaddr;
+ }; /* Base virtual address of the region */
+};
+
+struct nvgrace_gpu_vfio_pci_core_device {
+ struct vfio_pci_core_device core_device;
+ /* Cached and usable memory for the VM. */
+ struct mem_region usemem;
+ /* Non cached memory carved out from the end of device memory */
+ struct mem_region resmem;
+ /* Lock to control device memory kernel mapping */
+ struct mutex remap_lock;
+};
+
+static void nvgrace_gpu_init_fake_bar_emu_regs(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);
+
+ nvdev->resmem.bar_val = 0;
+ nvdev->usemem.bar_val = 0;
+}
+
+/* Choose the structure corresponding to the fake BAR with a given index. */
+static struct mem_region *
+nvgrace_gpu_memregion(int index,
+ struct nvgrace_gpu_vfio_pci_core_device *nvdev)
+{
+ if (index == USEMEM_REGION_INDEX)
+ return &nvdev->usemem;
+
+ if (index == RESMEM_REGION_INDEX)
+ return &nvdev->resmem;
+
+ return NULL;
+}
+
+static int nvgrace_gpu_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);
+
+ if (nvdev->usemem.memlength) {
+ nvgrace_gpu_init_fake_bar_emu_regs(core_vdev);
+ mutex_init(&nvdev->remap_lock);
+ }
+
+ return 0;
+}
+
+static void nvgrace_gpu_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);
+
+ /* Unmap the mapping to the device memory cached region */
+ if (nvdev->usemem.memaddr) {
+ memunmap(nvdev->usemem.memaddr);
+ nvdev->usemem.memaddr = NULL;
+ }
+
+ /* Unmap the mapping to the device memory non-cached region */
+ if (nvdev->resmem.ioaddr) {
+ iounmap(nvdev->resmem.ioaddr);
+ nvdev->resmem.ioaddr = NULL;
+ }
+
+ mutex_destroy(&nvdev->remap_lock);
+
+ vfio_pci_core_close_device(core_vdev);
+}
+
+static int nvgrace_gpu_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;
+ struct mem_region *memregion;
+
+ index = vma->vm_pgoff >> (VFIO_PCI_OFFSET_SHIFT - PAGE_SHIFT);
+
+ memregion = nvgrace_gpu_memregion(index, nvdev);
+ if (!memregion)
+ 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(memregion->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 > memregion->memlength)
+ return -EINVAL;
+
+ /*
+ * The carved out region of the device memory needs the NORMAL_NC
+ * property. Communicate as such to the hypervisor.
+ */
+ if (index == RESMEM_REGION_INDEX)
+ vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
+
+ /*
+ * 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. The
+ * remainder is only backed by vfio_device_ops read/write handlers.
+ *
+ * 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_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_cap_sparse_mmap *sparse;
+ struct vfio_info_cap caps = { .buf = NULL, .size = 0 };
+ struct vfio_region_info info;
+ struct mem_region *memregion;
+ u32 size;
+ int ret;
+
+ if (copy_from_user(&info, (void __user *)arg, minsz))
+ return -EFAULT;
+
+ if (info.argsz < minsz)
+ return -EINVAL;
+
+ memregion = nvgrace_gpu_memregion(info.index, nvdev);
+ if (!memregion)
+ return vfio_pci_core_ioctl(core_vdev,
+ VFIO_DEVICE_GET_REGION_INFO, arg);
+
+ /*
+ * Request to determine the BAR region information. Send the
+ * GPU memory information.
+ */
+ 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 = memregion->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 region memory size may not be power-of-2 aligned.
+ * Given that the memory as a BAR and may not be
+ * aligned, roundup to the next power-of-2.
+ */
+ info.size = memregion->bar_size;
+ 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;
+}
+
+static long nvgrace_gpu_ioctl(struct vfio_device *core_vdev,
+ unsigned int cmd, unsigned long arg)
+{
+ switch (cmd) {
+ case VFIO_DEVICE_GET_REGION_INFO:
+ return nvgrace_gpu_ioctl_get_region_info(core_vdev, arg);
+ case VFIO_DEVICE_IOEVENTFD:
+ return -ENOTTY;
+ case VFIO_DEVICE_RESET:
+ nvgrace_gpu_init_fake_bar_emu_regs(core_vdev);
+ fallthrough;
+ default:
+ return vfio_pci_core_ioctl(core_vdev, cmd, arg);
+ }
+}
+
+static __le64
+nvgrace_gpu_get_read_value(size_t bar_size, u64 flags, __le64 val64)
+{
+ u64 tmp_val;
+
+ tmp_val = le64_to_cpu(val64);
+ tmp_val &= ~(bar_size - 1);
+ tmp_val |= flags;
+
+ return cpu_to_le64(tmp_val);
+}
+
+/*
+ * Both the usable (usemem) and the reserved (resmem) device memory region
+ * are exposed as a 64b fake BARs in the VM. These fake BARs must respond
+ * to the accesses on their respective PCI config space offsets.
+ *
+ * resmem BAR owns PCI_BASE_ADDRESS_2 & PCI_BASE_ADDRESS_3.
+ * usemem BAR owns PCI_BASE_ADDRESS_4 & PCI_BASE_ADDRESS_5.
+ */
+static ssize_t
+nvgrace_gpu_read_config_emu(struct vfio_device *core_vdev,
+ char __user *buf, size_t count, loff_t *ppos)
+{
+ struct nvgrace_gpu_vfio_pci_core_device *nvdev =
+ container_of(core_vdev, struct nvgrace_gpu_vfio_pci_core_device,
+ core_device.vdev);
+ struct mem_region *memregion = NULL;
+ u64 pos = *ppos & VFIO_PCI_OFFSET_MASK;
+ __le64 val64;
+ size_t register_offset;
+ loff_t copy_offset;
+ size_t copy_count;
+ int ret;
+
+ ret = vfio_pci_core_read(core_vdev, buf, count, ppos);
+ if (ret < 0)
+ return ret;
+
+ if (vfio_pci_core_range_intersect_range(pos, count, PCI_BASE_ADDRESS_2,
+ sizeof(val64),
+ &copy_offset, &copy_count,
+ &register_offset))
+ memregion = nvgrace_gpu_memregion(RESMEM_REGION_INDEX, nvdev);
+ else if (vfio_pci_core_range_intersect_range(pos, count,
+ PCI_BASE_ADDRESS_4,
+ sizeof(val64),
+ &copy_offset, &copy_count,
+ &register_offset))
+ memregion = nvgrace_gpu_memregion(USEMEM_REGION_INDEX, nvdev);
+
+ if (memregion) {
+ val64 = nvgrace_gpu_get_read_value(memregion->bar_size,
+ PCI_BASE_ADDRESS_MEM_TYPE_64 |
+ PCI_BASE_ADDRESS_MEM_PREFETCH,
+ memregion->bar_val);
+ if (copy_to_user(buf + copy_offset,
+ (void *)&val64 + register_offset, copy_count))
+ return -EFAULT;
+ }
+
+ return count;
+}
+
+static ssize_t
+nvgrace_gpu_write_config_emu(struct vfio_device *core_vdev,
+ const char __user *buf, size_t count, loff_t *ppos)
+{
+ struct nvgrace_gpu_vfio_pci_core_device *nvdev =
+ container_of(core_vdev, struct nvgrace_gpu_vfio_pci_core_device,
+ core_device.vdev);
+ u64 pos = *ppos & VFIO_PCI_OFFSET_MASK;
+ size_t register_offset;
+ loff_t copy_offset;
+ size_t copy_count;
+ struct mem_region *memregion = NULL;
+
+ if (vfio_pci_core_range_intersect_range(pos, count, PCI_BASE_ADDRESS_2,
+ sizeof(u64), &copy_offset,
+ &copy_count, &register_offset))
+ memregion = nvgrace_gpu_memregion(RESMEM_REGION_INDEX, nvdev);
+ else if (vfio_pci_core_range_intersect_range(pos, count, PCI_BASE_ADDRESS_4,
+ sizeof(u64), &copy_offset,
+ &copy_count, &register_offset))
+ memregion = nvgrace_gpu_memregion(USEMEM_REGION_INDEX, nvdev);
+
+ if (memregion) {
+ if (copy_from_user((void *)&memregion->bar_val + register_offset,
+ buf + copy_offset, copy_count))
+ return -EFAULT;
+ *ppos += copy_count;
+ return copy_count;
+ }
+
+ return vfio_pci_core_write(core_vdev, buf, count, ppos);
+}
+
+/*
+ * Ad hoc map the device memory in the module kernel VA space. Primarily needed
+ * as vfio does not require the userspace driver to only perform accesses through
+ * mmaps of the vfio-pci BAR regions and such accesses should be supported using
+ * vfio_device_ops read/write implementations.
+ *
+ * The usemem region is cacheable memory and hence is memremaped.
+ * The resmem region is non-cached and is mapped using ioremap_wc (NORMAL_NC).
+ */
+static int
+nvgrace_gpu_map_device_mem(int index,
+ struct nvgrace_gpu_vfio_pci_core_device *nvdev)
+{
+ struct mem_region *memregion;
+ int ret = 0;
+
+ memregion = nvgrace_gpu_memregion(index, nvdev);
+ if (!memregion)
+ return -EINVAL;
+
+ mutex_lock(&nvdev->remap_lock);
+ if (index == USEMEM_REGION_INDEX && !memregion->memaddr) {
+ memregion->memaddr = memremap(memregion->memphys,
+ memregion->memlength,
+ MEMREMAP_WB);
+ if (!memregion->memaddr)
+ ret = -ENOMEM;
+ } else if (index == RESMEM_REGION_INDEX && !memregion->ioaddr) {
+ memregion->ioaddr = ioremap_wc(memregion->memphys,
+ memregion->memlength);
+ if (!memregion->ioaddr)
+ ret = -ENOMEM;
+ }
+ mutex_unlock(&nvdev->remap_lock);
+
+ return ret;
+}
+
+/*
+ * Read the data from the device memory (mapped either through ioremap
+ * or memremap) into the user buffer.
+ */
+static int
+nvgrace_gpu_map_and_read(struct nvgrace_gpu_vfio_pci_core_device *nvdev,
+ char __user *buf, size_t mem_count, loff_t *ppos)
+{
+ unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos);
+ u64 offset = *ppos & VFIO_PCI_OFFSET_MASK;
+ int ret;
+
+ /*
+ * Handle read on the BAR regions. Map to the target device memory
+ * physical address and copy to the request read buffer.
+ */
+ ret = nvgrace_gpu_map_device_mem(index, nvdev);
+ if (ret)
+ return ret;
+
+ if (index == USEMEM_REGION_INDEX) {
+ if (copy_to_user(buf,
+ (u8 *)nvdev->usemem.memaddr + offset,
+ mem_count))
+ ret = -EFAULT;
+ } else {
+ /*
+ * The hardware ensures that the system does not crash when
+ * the device memory is accessed with the memory enable
+ * turned off. It synthesizes ~0 on such read. So there is
+ * no need to check or support the disablement/enablement of
+ * BAR through PCI_COMMAND config space register. Pass
+ * test_mem flag as false.
+ */
+ ret = vfio_pci_core_do_io_rw(&nvdev->core_device, false,
+ nvdev->resmem.ioaddr,
+ buf, offset, mem_count,
+ 0, 0, false);
+ }
+
+ return ret;
+}
+
+/*
+ * 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.
+ *
+ * Reads extending beyond the reported size are truncated; reads starting
+ * beyond the reported size generate -EINVAL; reads extending beyond the
+ * actual device size is filled with ~0.
+ */
+static ssize_t
+nvgrace_gpu_read_mem(struct nvgrace_gpu_vfio_pci_core_device *nvdev,
+ char __user *buf, size_t count, loff_t *ppos)
+{
+ u64 offset = *ppos & VFIO_PCI_OFFSET_MASK;
+ unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos);
+ struct mem_region *memregion;
+ size_t mem_count, i;
+ u8 val = 0xFF;
+ int ret;
+
+ memregion = nvgrace_gpu_memregion(index, nvdev);
+ if (!memregion)
+ return -EINVAL;
+
+ if (offset >= memregion->bar_size)
+ return -EINVAL;
+
+ /* Clip short the read request beyond reported BAR size */
+ count = min(count, memregion->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 >= memregion->memlength)
+ mem_count = 0;
+ else
+ mem_count = min(count, memregion->memlength - (size_t)offset);
+
+ ret = nvgrace_gpu_map_and_read(nvdev, buf, mem_count, ppos);
+ if (ret)
+ return ret;
+
+ /*
+ * 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_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 (nvgrace_gpu_memregion(index, nvdev))
+ return nvgrace_gpu_read_mem(nvdev, buf, count, ppos);
+
+ if (index == VFIO_PCI_CONFIG_REGION_INDEX)
+ return nvgrace_gpu_read_config_emu(core_vdev, buf, count, ppos);
+
+ return vfio_pci_core_read(core_vdev, buf, count, ppos);
+}
+
+/*
+ * Write the data to the device memory (mapped either through ioremap
+ * or memremap) from the user buffer.
+ */
+static int
+nvgrace_gpu_map_and_write(struct nvgrace_gpu_vfio_pci_core_device *nvdev,
+ const char __user *buf, size_t mem_count,
+ loff_t *ppos)
+{
+ unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos);
+ loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK;
+ int ret;
+
+ ret = nvgrace_gpu_map_device_mem(index, nvdev);
+ if (ret)
+ return ret;
+
+ if (index == USEMEM_REGION_INDEX) {
+ if (copy_from_user((u8 *)nvdev->usemem.memaddr + pos,
+ buf, mem_count))
+ return -EFAULT;
+ } else {
+ /*
+ * The hardware ensures that the system does not crash when
+ * the device memory is accessed with the memory enable
+ * turned off. It drops such writes. So there is no need to
+ * check or support the disablement/enablement of BAR
+ * through PCI_COMMAND config space register. Pass test_mem
+ * flag as false.
+ */
+ ret = vfio_pci_core_do_io_rw(&nvdev->core_device, false,
+ nvdev->resmem.ioaddr,
+ (char __user *)buf, pos, mem_count,
+ 0, 0, true);
+ }
+
+ return ret;
+}
+
+/*
+ * 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.
+ *
+ * Writes extending beyond the reported size are truncated; writes starting
+ * beyond the reported size generate -EINVAL.
+ */
+static ssize_t
+nvgrace_gpu_write_mem(struct nvgrace_gpu_vfio_pci_core_device *nvdev,
+ size_t count, loff_t *ppos, const char __user *buf)
+{
+ u64 offset = *ppos & VFIO_PCI_OFFSET_MASK;
+ unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos);
+ struct mem_region *memregion;
+ size_t mem_count;
+ int ret = 0;
+
+ memregion = nvgrace_gpu_memregion(index, nvdev);
+ if (!memregion)
+ return -EINVAL;
+
+ if (offset >= memregion->bar_size)
+ return -EINVAL;
+
+ /* Clip short the write request beyond reported BAR size */
+ count = min(count, memregion->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 >= memregion->memlength)
+ goto exitfn;
+
+ /*
+ * Only the device memory present on the hardware is mapped, which may
+ * not be power-of-2 aligned. Drop access outside the available device
+ * memory on the hardware.
+ */
+ mem_count = min(count, memregion->memlength - (size_t)offset);
+
+ ret = nvgrace_gpu_map_and_write(nvdev, buf, mem_count, ppos);
+ if (ret)
+ return ret;
+
+exitfn:
+ *ppos += count;
+ return count;
+}
+
+static ssize_t
+nvgrace_gpu_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 (nvgrace_gpu_memregion(index, nvdev))
+ return nvgrace_gpu_write_mem(nvdev, count, ppos, buf);
+
+ if (index == VFIO_PCI_CONFIG_REGION_INDEX)
+ return nvgrace_gpu_write_config_emu(core_vdev, buf, count, ppos);
+
+ 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_open_device,
+ .close_device = nvgrace_gpu_close_device,
+ .ioctl = nvgrace_gpu_ioctl,
+ .read = nvgrace_gpu_read,
+ .write = nvgrace_gpu_write,
+ .mmap = nvgrace_gpu_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,
+ .detach_ioas = vfio_iommufd_physical_detach_ioas,
+};
+
+static const struct vfio_device_ops nvgrace_gpu_vfio_pci_core_ops = {
+ .name = "nvgrace-gpu-vfio-pci-core",
+ .init = vfio_pci_core_init_dev,
+ .release = vfio_pci_core_release_dev,
+ .open_device = nvgrace_gpu_open_device,
+ .close_device = vfio_pci_core_close_device,
+ .ioctl = vfio_pci_core_ioctl,
+ .device_feature = vfio_pci_core_ioctl_feature,
+ .read = vfio_pci_core_read,
+ .write = vfio_pci_core_write,
+ .mmap = vfio_pci_core_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,
+ .detach_ioas = vfio_iommufd_physical_detach_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_fetch_memory_property(struct pci_dev *pdev,
+ u64 *pmemphys, u64 *pmemlength)
+{
+ int ret;
+
+ /*
+ * 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",
+ pmemphys);
+ if (ret)
+ return ret;
+
+ if (*pmemphys > type_max(phys_addr_t))
+ return -EOVERFLOW;
+
+ ret = device_property_read_u64(&pdev->dev, "nvidia,gpu-mem-size",
+ pmemlength);
+ if (ret)
+ return ret;
+
+ if (*pmemlength > 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 (*pmemlength == 0)
+ return -ENOMEM;
+
+ return ret;
+}
+
+static int
+nvgrace_gpu_init_nvdev_struct(struct pci_dev *pdev,
+ struct nvgrace_gpu_vfio_pci_core_device *nvdev,
+ u64 memphys, u64 memlength)
+{
+ int ret = 0;
+
+ /*
+ * The VM GPU device driver needs a non-cacheable region to support
+ * the MIG feature. Since the device memory is mapped as NORMAL cached,
+ * carve out a region from the end with a different NORMAL_NC
+ * property (called as reserved memory and represented as resmem). This
+ * region then is exposed as a 64b BAR (region 2 and 3) to the VM, while
+ * exposing the rest (termed as usable memory and represented using usemem)
+ * as cacheable 64b BAR (region 4 and 5).
+ *
+ * devmem (memlength)
+ * |-------------------------------------------------|
+ * | |
+ * usemem.phys/memphys resmem.phys
+ */
+ nvdev->usemem.memphys = memphys;
+
+ /*
+ * The device memory exposed to the VM is added to the kernel by the
+ * VM driver module in chunks of memory block size. Only the usable
+ * memory (usemem) is added to the kernel for usage by the VM
+ * workloads. Make the usable memory size memblock aligned.
+ */
+ if (check_sub_overflow(memlength, RESMEM_SIZE,
+ &nvdev->usemem.memlength)) {
+ ret = -EOVERFLOW;
+ goto done;
+ }
+ nvdev->usemem.memlength = round_down(nvdev->usemem.memlength,
+ MEMBLK_SIZE);
+ if ((check_add_overflow(nvdev->usemem.memphys,
+ nvdev->usemem.memlength,
+ &nvdev->resmem.memphys)) ||
+ (check_sub_overflow(memlength, nvdev->usemem.memlength,
+ &nvdev->resmem.memlength))) {
+ ret = -EOVERFLOW;
+ goto done;
+ }
+
+ /*
+ * The memory regions are exposed as BARs. Calculate and save
+ * the BAR size for them.
+ */
+ nvdev->usemem.bar_size = roundup_pow_of_two(nvdev->usemem.memlength);
+ nvdev->resmem.bar_size = roundup_pow_of_two(nvdev->resmem.memlength);
+done:
+ return ret;
+}
+
+static int nvgrace_gpu_probe(struct pci_dev *pdev,
+ const struct pci_device_id *id)
+{
+ const struct vfio_device_ops *ops = &nvgrace_gpu_vfio_pci_core_ops;
+ struct nvgrace_gpu_vfio_pci_core_device *nvdev;
+ u64 memphys, memlength;
+ int ret;
+
+ ret = nvgrace_gpu_fetch_memory_property(pdev, &memphys, &memlength);
+ if (!ret)
+ ops = &nvgrace_gpu_vfio_pci_ops;
+
+ nvdev = vfio_alloc_device(nvgrace_gpu_vfio_pci_core_device, core_device.vdev,
+ &pdev->dev, ops);
+ if (IS_ERR(nvdev))
+ return PTR_ERR(nvdev);
+
+ dev_set_drvdata(&pdev->dev, &nvdev->core_device);
+
+ if (ops == &nvgrace_gpu_vfio_pci_ops) {
+ /*
+ * Device memory properties are identified in the host ACPI
+ * table. Set the nvgrace_gpu_vfio_pci_core_device structure.
+ */
+ ret = nvgrace_gpu_init_nvdev_struct(pdev, nvdev,
+ memphys, memlength);
+ 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_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_probe,
+ .remove = nvgrace_gpu_remove,
+ .err_handler = &vfio_pci_core_err_handlers,
+ .driver_managed_dma = true,
+};
+
+module_pci_driver(nvgrace_gpu_vfio_pci_driver);
+
+MODULE_LICENSE("GPL");
+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.34.1

2024-02-07 22:32:34

by Zhi Wang

[permalink] [raw]

Subject: Re: [PATCH v17 3/3] vfio/nvgrace-gpu: Add vfio pci variant module for grace hopper

On Tue, 6 Feb 2024 04:31:23 +0530
<[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 chip-to-chip cache coherent interconnect.
>
> The device is peculiar compared to a real PCI device in that whilst
> there is a real 64b PCI BAR1 (comprising region 2 & region 3) on the
> device, it is not used to access device memory once the faster
> chip-to-chip interconnect is initialized (occurs at the time of host
> system boot). The device memory is accessed instead using the
> chip-to-chip interconnect that is exposed as a contiguous physically
> addressable region on the host. This device memory aperture can be
> obtained from host ACPI table using device_property_read_u64(),
> according to the FW specification. Since the device memory is cache
> coherent with the CPU, it can be mmap into the user VMA with a
> cacheable mapping using remap_pfn_range() and used like a regular
> RAM. The device memory is not added to the host kernel, but mapped
> directly as this reduces memory wastage due to struct pages.
>
> There is also a requirement of a reserved 1G uncached region (termed
> as resmem) to support the Multi-Instance GPU (MIG) feature [1]. This
> is to work around a HW defect. Based on [2], the requisite properties
> (uncached, unaligned access) can be achieved through a VM mapping (S1)
> of NORMAL_NC and host (S2) mapping with MemAttr[2:0]=0b101. To provide
> a different non-cached property to the reserved 1G region, it needs to
> be carved out from the device memory and mapped as a separate region
> in Qemu VMA with pgprot_writecombine(). pgprot_writecombine() sets the
> Qemu VMA page properties (pgprot) as NORMAL_NC.
>
> Provide a VFIO PCI variant driver that adapts the unique device memory
> representation into a more standard PCI representation facing
> userspace.
>
> The variant driver exposes these two regions - the non-cached reserved
> (resmem) and the cached rest of the device memory (termed as usemem)
> as separate VFIO 64b BAR regions. This is divergent from the baremetal
> approach, where the device memory is exposed as a device memory
> region. The decision for a different approach was taken in view of
> the fact that it would necessiate additional code in Qemu to discover
> and insert those regions in the VM IPA, along with the additional VM
> ACPI DSDT changes to communicate the device memory region IPA to the
> VM workloads. Moreover, this behavior would have to be added to a
> variety of emulators (beyond top of tree Qemu) out there desiring
> grace hopper support.
>
> Since the device implements 64-bit BAR0, the VFIO PCI variant driver
> maps the uncached carved out region to the next available PCI BAR
> (i.e. comprising of region 2 and 3). The cached device memory
> aperture is assigned BAR region 4 and 5. Qemu will then naturally
> generate a PCI device in the VM with the uncached aperture reported
> as BAR2 region, the cacheable as BAR4. The variant driver provides
> emulation for these fake BARs' PCI config space offset registers.
>
> The hardware ensures that the system does not crash when the memory
> is accessed with the memory enable turned off. It synthesis ~0 reads
> and dropped writes on such access. So there is no need to support the
> disablement/enablement of BAR through PCI_COMMAND config space
> register.
>
> The memory layout on the host looks like the following:
> devmem (memlength)
> |--------------------------------------------------|
> |-------------cached------------------------|--NC--|
> | |
> usemem.phys/memphys resmem.phys
>
> PCI BARs need to be 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 [6] 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.
>
> Note that the usemem memory is added by the VM Nvidia device driver
> [5] to the VM kernel as memblocks. Hence make the usable memory size
> memblock aligned.
>
> Currently there is no provision in KVM for a S2 mapping with
> MemAttr[2:0]=0b101, but there is an ongoing effort to provide the
> same [3]. As previously mentioned, resmem is mapped
> pgprot_writecombine(), that sets the Qemu VMA page properties
> (pgprot) as NORMAL_NC. Using the proposed changes in [4] and [3], KVM
> marks the region with MemAttr[2:0]=0b101 in S2.
>
> If the bare metal properties are not present, the driver registers the
> vfio-pci-core function pointers.
>
> This goes along with a qemu series [6] 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.
>
> [1] https://www.nvidia.com/en-in/technologies/multi-instance-gpu/
> [2] section D8.5.5 of
> https://developer.arm.com/documentation/ddi0487/latest/ [3]
> https://lore.kernel.org/all/[email protected]/
> [4]
> https://lore.kernel.org/all/[email protected]/
> [5] https://github.com/NVIDIA/open-gpu-kernel-modules [6]
> https://lore.kernel.org/all/[email protected]/
>
> Signed-off-by: Aniket Agashe <[email protected]>
> Signed-off-by: Ankit Agrawal <[email protected]>
> ---
> 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 | 856
> ++++++++++++++++++++++++++ 6 files changed, 879 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 8999497011a2..529ec8966f58 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -23103,6 +23103,12 @@ L: [email protected]
> S: Maintained
> F: drivers/vfio/platform/
>
> +VFIO NVIDIA GRACE GPU DRIVER
> +M: Ankit Agrawal <[email protected]>
> +L: [email protected]
> +S: Supported
> +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 18c397df566d..15821a2d77d2 100644
> --- a/drivers/vfio/pci/Kconfig
> +++ b/drivers/vfio/pci/Kconfig
> @@ -67,4 +67,6 @@ source "drivers/vfio/pci/pds/Kconfig"
>
> source "drivers/vfio/pci/virtio/Kconfig"
>
> +source "drivers/vfio/pci/nvgrace-gpu/Kconfig"
> +
> endmenu
> diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile
> index 046139a4eca5..ce7a61f1d912 100644
> --- a/drivers/vfio/pci/Makefile
> +++ b/drivers/vfio/pci/Makefile
> @@ -15,3 +15,5 @@ obj-$(CONFIG_HISI_ACC_VFIO_PCI) += hisilicon/
> obj-$(CONFIG_PDS_VFIO_PCI) += pds/
>
> obj-$(CONFIG_VIRTIO_VFIO_PCI) += virtio/
> +
> +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..6279af2bc6b8
> --- /dev/null
> +++ b/drivers/vfio/pci/nvgrace-gpu/main.c
> @@ -0,0 +1,856 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2024, NVIDIA CORPORATION & AFFILIATES. All rights
> reserved
> + */
> +
> +#include <linux/vfio_pci_core.h>
> +
> +/*
> + * The device memory usable to the workloads running in the VM is
> cached
> + * and showcased as a 64b device BAR (comprising of BAR4 and BAR5
> region)
> + * to the VM and is represented as usemem.
> + * Moreover, the VM GPU device driver needs a non-cacheable region to
> + * support the MIG feature. This region is also exposed as a 64b BAR
> + * (comprising of BAR2 and BAR3 region) and represented as resmem.
> + */
> +#define RESMEM_REGION_INDEX VFIO_PCI_BAR2_REGION_INDEX
> +#define USEMEM_REGION_INDEX VFIO_PCI_BAR4_REGION_INDEX
> +
> +/* Memory size expected as non cached and reserved by the VM driver
> */ +#define RESMEM_SIZE 0x40000000
> +#define MEMBLK_SIZE 0x20000000
> +

Maybe use SZ_* definitions in linux/size.h

> +/*
> + * The state of the two device memory region - resmem and usemem - is
> + * saved as struct mem_region.
> + */
> +struct mem_region {
> + phys_addr_t memphys; /* Base physical address of the
> region */
> + size_t memlength; /* Region size */
> + size_t bar_size; /* Reported region BAR size */
> + __le64 bar_val; /* Emulated BAR offset registers */
> + union {
> + void *memaddr;
> + void __iomem *ioaddr;
> + }; /* Base virtual address of the
> region */ +};
> +
> +struct nvgrace_gpu_vfio_pci_core_device {
> + struct vfio_pci_core_device core_device;
> + /* Cached and usable memory for the VM. */
> + struct mem_region usemem;
> + /* Non cached memory carved out from the end of device
> memory */
> + struct mem_region resmem;
> + /* Lock to control device memory kernel mapping */
> + struct mutex remap_lock;
> +};
> +
> +static void nvgrace_gpu_init_fake_bar_emu_regs(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);
> +
> + nvdev->resmem.bar_val = 0;
> + nvdev->usemem.bar_val = 0;
> +}
> +
> +/* Choose the structure corresponding to the fake BAR with a given
> index. */ +static struct mem_region *
> +nvgrace_gpu_memregion(int index,
> + struct nvgrace_gpu_vfio_pci_core_device *nvdev)
> +{
> + if (index == USEMEM_REGION_INDEX)
> + return &nvdev->usemem;
> +
> + if (index == RESMEM_REGION_INDEX)
> + return &nvdev->resmem;
> +
> + return NULL;
> +}
> +
> +static int nvgrace_gpu_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);
> +
---
> + if (nvdev->usemem.memlength) {
> + nvgrace_gpu_init_fake_bar_emu_regs(core_vdev);
> + mutex_init(&nvdev->remap_lock);
> + }
> +
---

Better move this part to the place between vfio_pci_core_enable() and
vfio_pci_core_finish_enable() like others for respecting the expected
device initialization sequence of life cycle.

It doesn't bite something right now, but think about when someone
changes the behavior of vfio_pci_core_finish_enable() in the future,
they have to propose a patch for this.

> + return 0;
> +}
> +
> +static void nvgrace_gpu_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);
> +

Shouldn't there be a lock/unlock sequence when using/freeing the map? As
the vfio device open/close paths are protected by group/dev_set lock,
probably they are fine without driver-level lock protection. But
for vfio_device_read/write/ioctl, if they are racing with open/close
path, I think there should be a lock/unlock to protect the map.

---
> + /* Unmap the mapping to the device memory cached region */
> + if (nvdev->usemem.memaddr) {
> + memunmap(nvdev->usemem.memaddr);
> + nvdev->usemem.memaddr = NULL;
> + }
> +
> + /* Unmap the mapping to the device memory non-cached region
> */
> + if (nvdev->resmem.ioaddr) {
> + iounmap(nvdev->resmem.ioaddr);
> + nvdev->resmem.ioaddr = NULL;
> + }
> +
---

> + mutex_destroy(&nvdev->remap_lock);
> +
> + vfio_pci_core_close_device(core_vdev);
> +}
> +
> +static int nvgrace_gpu_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;
> + struct mem_region *memregion;
> +
> + index = vma->vm_pgoff >> (VFIO_PCI_OFFSET_SHIFT -
> PAGE_SHIFT); +
> + memregion = nvgrace_gpu_memregion(index, nvdev);
> + if (!memregion)
> + 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(memregion->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 > memregion->memlength)
> + return -EINVAL;
> +
> + /*
> + * The carved out region of the device memory needs the
> NORMAL_NC
> + * property. Communicate as such to the hypervisor.
> + */
> + if (index == RESMEM_REGION_INDEX)
> + vma->vm_page_prot =
> pgprot_writecombine(vma->vm_page_prot); +
> + /*
> + * 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. The
> + * remainder is only backed by vfio_device_ops read/write
> handlers.
> + *
> + * 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_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_cap_sparse_mmap *sparse;
> + struct vfio_info_cap caps = { .buf = NULL, .size = 0 };
> + struct vfio_region_info info;
> + struct mem_region *memregion;
> + u32 size;
> + int ret;
> +
> + if (copy_from_user(&info, (void __user *)arg, minsz))
> + return -EFAULT;
> +
> + if (info.argsz < minsz)
> + return -EINVAL;
> +
> + memregion = nvgrace_gpu_memregion(info.index, nvdev);
> + if (!memregion)
> + return vfio_pci_core_ioctl(core_vdev,
> +
> VFIO_DEVICE_GET_REGION_INFO, arg); +
> + /*
> + * Request to determine the BAR region information. Send the
> + * GPU memory information.
> + */
> + 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 = memregion->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 region memory size may not be power-of-2 aligned.
> + * Given that the memory as a BAR and may not be
> + * aligned, roundup to the next power-of-2.
> + */
> + info.size = memregion->bar_size;
> + 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;
> +}
> +
> +static long nvgrace_gpu_ioctl(struct vfio_device *core_vdev,
> + unsigned int cmd, unsigned long arg)
> +{
> + switch (cmd) {
> + case VFIO_DEVICE_GET_REGION_INFO:
> + return nvgrace_gpu_ioctl_get_region_info(core_vdev,
> arg);
> + case VFIO_DEVICE_IOEVENTFD:
> + return -ENOTTY;
> + case VFIO_DEVICE_RESET:
> + nvgrace_gpu_init_fake_bar_emu_regs(core_vdev);
> + fallthrough;
> + default:
> + return vfio_pci_core_ioctl(core_vdev, cmd, arg);
> + }
> +}
> +
> +static __le64
> +nvgrace_gpu_get_read_value(size_t bar_size, u64 flags, __le64 val64)
> +{
> + u64 tmp_val;
> +
> + tmp_val = le64_to_cpu(val64);
> + tmp_val &= ~(bar_size - 1);
> + tmp_val |= flags;
> +
> + return cpu_to_le64(tmp_val);
> +}
> +
> +/*
> + * Both the usable (usemem) and the reserved (resmem) device memory
> region
> + * are exposed as a 64b fake BARs in the VM. These fake BARs must
> respond
> + * to the accesses on their respective PCI config space offsets.
> + *
> + * resmem BAR owns PCI_BASE_ADDRESS_2 & PCI_BASE_ADDRESS_3.
> + * usemem BAR owns PCI_BASE_ADDRESS_4 & PCI_BASE_ADDRESS_5.
> + */
> +static ssize_t
> +nvgrace_gpu_read_config_emu(struct vfio_device *core_vdev,
> + char __user *buf, size_t count, loff_t
> *ppos) +{
> + struct nvgrace_gpu_vfio_pci_core_device *nvdev =
> + container_of(core_vdev, struct
> nvgrace_gpu_vfio_pci_core_device,
> + core_device.vdev);
> + struct mem_region *memregion = NULL;
> + u64 pos = *ppos & VFIO_PCI_OFFSET_MASK;
> + __le64 val64;
> + size_t register_offset;
> + loff_t copy_offset;
> + size_t copy_count;
> + int ret;
> +
> + ret = vfio_pci_core_read(core_vdev, buf, count, ppos);
> + if (ret < 0)
> + return ret;
> +
> + if (vfio_pci_core_range_intersect_range(pos, count,
> PCI_BASE_ADDRESS_2,
> + sizeof(val64),
> + &copy_offset,
> &copy_count,
> + &register_offset))
> + memregion =
> nvgrace_gpu_memregion(RESMEM_REGION_INDEX, nvdev);
> + else if (vfio_pci_core_range_intersect_range(pos, count,
> +
> PCI_BASE_ADDRESS_4,
> + sizeof(val64),
> + &copy_offset,
> &copy_count,
> +
> &register_offset))
> + memregion =
> nvgrace_gpu_memregion(USEMEM_REGION_INDEX, nvdev); +
> + if (memregion) {
> + val64 =
> nvgrace_gpu_get_read_value(memregion->bar_size,
> +
> PCI_BASE_ADDRESS_MEM_TYPE_64 |
> +
> PCI_BASE_ADDRESS_MEM_PREFETCH,
> +
> memregion->bar_val);
> + if (copy_to_user(buf + copy_offset,
> + (void *)&val64 + register_offset,
> copy_count))
> + return -EFAULT;
> + }
> +
> + return count;
> +}
> +
> +static ssize_t
> +nvgrace_gpu_write_config_emu(struct vfio_device *core_vdev,
> + const char __user *buf, size_t count,
> loff_t *ppos) +{
> + struct nvgrace_gpu_vfio_pci_core_device *nvdev =
> + container_of(core_vdev, struct
> nvgrace_gpu_vfio_pci_core_device,
> + core_device.vdev);
> + u64 pos = *ppos & VFIO_PCI_OFFSET_MASK;
> + size_t register_offset;
> + loff_t copy_offset;
> + size_t copy_count;
> + struct mem_region *memregion = NULL;
> +
> + if (vfio_pci_core_range_intersect_range(pos, count,
> PCI_BASE_ADDRESS_2,
> + sizeof(u64),
> &copy_offset,
> + &copy_count,
> &register_offset))
> + memregion =
> nvgrace_gpu_memregion(RESMEM_REGION_INDEX, nvdev);
> + else if (vfio_pci_core_range_intersect_range(pos, count,
> PCI_BASE_ADDRESS_4,
> + sizeof(u64),
> &copy_offset,
> + &copy_count,
> &register_offset))
> + memregion =
> nvgrace_gpu_memregion(USEMEM_REGION_INDEX, nvdev); +
> + if (memregion) {
> + if (copy_from_user((void *)&memregion->bar_val +
> register_offset,
> + buf + copy_offset, copy_count))
> + return -EFAULT;
> + *ppos += copy_count;
> + return copy_count;
> + }
> +
> + return vfio_pci_core_write(core_vdev, buf, count, ppos);
> +}
> +
> +/*
> + * Ad hoc map the device memory in the module kernel VA space.
> Primarily needed
> + * as vfio does not require the userspace driver to only perform
> accesses through
> + * mmaps of the vfio-pci BAR regions and such accesses should be
> supported using
> + * vfio_device_ops read/write implementations.
> + *
> + * The usemem region is cacheable memory and hence is memremaped.
> + * The resmem region is non-cached and is mapped using ioremap_wc
> (NORMAL_NC).
> + */
> +static int
> +nvgrace_gpu_map_device_mem(int index,
> + struct nvgrace_gpu_vfio_pci_core_device
> *nvdev) +{
> + struct mem_region *memregion;
> + int ret = 0;
> +
> + memregion = nvgrace_gpu_memregion(index, nvdev);
> + if (!memregion)
> + return -EINVAL;
> +
> + mutex_lock(&nvdev->remap_lock);
> + if (index == USEMEM_REGION_INDEX && !memregion->memaddr) {
> + memregion->memaddr = memremap(memregion->memphys,
> + memregion->memlength,
> + MEMREMAP_WB);
> + if (!memregion->memaddr)
> + ret = -ENOMEM;
> + } else if (index == RESMEM_REGION_INDEX &&
> !memregion->ioaddr) {
> + memregion->ioaddr = ioremap_wc(memregion->memphys,
> + memregion->memlength);
> + if (!memregion->ioaddr)
> + ret = -ENOMEM;
> + }
> + mutex_unlock(&nvdev->remap_lock);
> +
> + return ret;
> +}
> +
> +/*
> + * Read the data from the device memory (mapped either through
> ioremap
> + * or memremap) into the user buffer.
> + */
> +static int
> +nvgrace_gpu_map_and_read(struct nvgrace_gpu_vfio_pci_core_device
> *nvdev,
> + char __user *buf, size_t mem_count, loff_t
> *ppos) +{
> + unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos);
> + u64 offset = *ppos & VFIO_PCI_OFFSET_MASK;
> + int ret;
> +
> + /*
> + * Handle read on the BAR regions. Map to the target device
> memory
> + * physical address and copy to the request read buffer.
> + */
> + ret = nvgrace_gpu_map_device_mem(index, nvdev);
> + if (ret)
> + return ret;
> +

Wouldn't it be better to do the map in the open path?

> + if (index == USEMEM_REGION_INDEX) {
> + if (copy_to_user(buf,
> + (u8 *)nvdev->usemem.memaddr +
> offset,
> + mem_count))
> + ret = -EFAULT;
> + } else {
> + /*
> + * The hardware ensures that the system does not
> crash when
> + * the device memory is accessed with the memory
> enable
> + * turned off. It synthesizes ~0 on such read. So
> there is
> + * no need to check or support the
> disablement/enablement of
> + * BAR through PCI_COMMAND config space register.
> Pass
> + * test_mem flag as false.
> + */
> + ret = vfio_pci_core_do_io_rw(&nvdev->core_device,
> false,
> + nvdev->resmem.ioaddr,
> + buf, offset, mem_count,
> + 0, 0, false);
> + }
> +
> + return ret;
> +}
> +
> +/*
> + * 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.
> + *
> + * Reads extending beyond the reported size are truncated; reads
> starting
> + * beyond the reported size generate -EINVAL; reads extending beyond
> the
> + * actual device size is filled with ~0.
> + */
> +static ssize_t
> +nvgrace_gpu_read_mem(struct nvgrace_gpu_vfio_pci_core_device *nvdev,
> + char __user *buf, size_t count, loff_t *ppos)
> +{
> + u64 offset = *ppos & VFIO_PCI_OFFSET_MASK;
> + unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos);
> + struct mem_region *memregion;
> + size_t mem_count, i;
> + u8 val = 0xFF;
> + int ret;
> +
> + memregion = nvgrace_gpu_memregion(index, nvdev);
> + if (!memregion)
> + return -EINVAL;
> +
> + if (offset >= memregion->bar_size)
> + return -EINVAL;
> +
> + /* Clip short the read request beyond reported BAR size */
> + count = min(count, memregion->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 >= memregion->memlength)
> + mem_count = 0;

If mem_count == 0, going through nvgrace_gpu_map_and_read() is not
necessary.

> + else
> + mem_count = min(count, memregion->memlength -
> (size_t)offset); +
> + ret = nvgrace_gpu_map_and_read(nvdev, buf, mem_count, ppos);
> + if (ret)
> + return ret;
> +
> + /*
> + * 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_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 (nvgrace_gpu_memregion(index, nvdev))
> + return nvgrace_gpu_read_mem(nvdev, buf, count, ppos);
> +
> + if (index == VFIO_PCI_CONFIG_REGION_INDEX)
> + return nvgrace_gpu_read_config_emu(core_vdev, buf,
> count, ppos); +
> + return vfio_pci_core_read(core_vdev, buf, count, ppos);
> +}
> +
> +/*
> + * Write the data to the device memory (mapped either through ioremap
> + * or memremap) from the user buffer.
> + */
> +static int
> +nvgrace_gpu_map_and_write(struct nvgrace_gpu_vfio_pci_core_device
> *nvdev,
> + const char __user *buf, size_t mem_count,
> + loff_t *ppos)
> +{
> + unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos);
> + loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK;
> + int ret;
> +
> + ret = nvgrace_gpu_map_device_mem(index, nvdev);
> + if (ret)
> + return ret;
> +
> + if (index == USEMEM_REGION_INDEX) {
> + if (copy_from_user((u8 *)nvdev->usemem.memaddr + pos,
> + buf, mem_count))
> + return -EFAULT;
> + } else {
> + /*
> + * The hardware ensures that the system does not
> crash when
> + * the device memory is accessed with the memory
> enable
> + * turned off. It drops such writes. So there is no
> need to
> + * check or support the disablement/enablement of BAR
> + * through PCI_COMMAND config space register. Pass
> test_mem
> + * flag as false.
> + */
> + ret = vfio_pci_core_do_io_rw(&nvdev->core_device,
> false,
> + nvdev->resmem.ioaddr,
> + (char __user *)buf,
> pos, mem_count,
> + 0, 0, true);
> + }
> +
> + return ret;
> +}
> +
> +/*
> + * 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.
> + *
> + * Writes extending beyond the reported size are truncated; writes
> starting
> + * beyond the reported size generate -EINVAL.
> + */
> +static ssize_t
> +nvgrace_gpu_write_mem(struct nvgrace_gpu_vfio_pci_core_device *nvdev,
> + size_t count, loff_t *ppos, const char __user
> *buf) +{
> + u64 offset = *ppos & VFIO_PCI_OFFSET_MASK;
> + unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos);
> + struct mem_region *memregion;
> + size_t mem_count;
> + int ret = 0;
> +
> + memregion = nvgrace_gpu_memregion(index, nvdev);
> + if (!memregion)
> + return -EINVAL;
> +
> + if (offset >= memregion->bar_size)
> + return -EINVAL;
> +
> + /* Clip short the write request beyond reported BAR size */
> + count = min(count, memregion->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 >= memregion->memlength)
> + goto exitfn;
> +
> + /*
> + * Only the device memory present on the hardware is mapped,
> which may
> + * not be power-of-2 aligned. Drop access outside the
> available device
> + * memory on the hardware.
> + */
> + mem_count = min(count, memregion->memlength -
> (size_t)offset); +
> + ret = nvgrace_gpu_map_and_write(nvdev, buf, mem_count, ppos);
> + if (ret)
> + return ret;
> +
> +exitfn:
> + *ppos += count;
> + return count;
> +}
> +
> +static ssize_t
> +nvgrace_gpu_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 (nvgrace_gpu_memregion(index, nvdev))
> + return nvgrace_gpu_write_mem(nvdev, count, ppos,
> buf); +
> + if (index == VFIO_PCI_CONFIG_REGION_INDEX)
> + return nvgrace_gpu_write_config_emu(core_vdev, buf,
> count, ppos); +
> + 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_open_device,
> + .close_device = nvgrace_gpu_close_device,
> + .ioctl = nvgrace_gpu_ioctl,
> + .read = nvgrace_gpu_read,
> + .write = nvgrace_gpu_write,
> + .mmap = nvgrace_gpu_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,
> + .detach_ioas = vfio_iommufd_physical_detach_ioas,
> +};
> +
> +static const struct vfio_device_ops nvgrace_gpu_vfio_pci_core_ops = {
> + .name = "nvgrace-gpu-vfio-pci-core",
> + .init = vfio_pci_core_init_dev,
> + .release = vfio_pci_core_release_dev,
> + .open_device = nvgrace_gpu_open_device,
> + .close_device = vfio_pci_core_close_device,
> + .ioctl = vfio_pci_core_ioctl,
> + .device_feature = vfio_pci_core_ioctl_feature,
> + .read = vfio_pci_core_read,
> + .write = vfio_pci_core_write,
> + .mmap = vfio_pci_core_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,
> + .detach_ioas = vfio_iommufd_physical_detach_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_fetch_memory_property(struct pci_dev *pdev,
> + u64 *pmemphys, u64 *pmemlength)
> +{
> + int ret;
> +
> + /*
> + * 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",
> + pmemphys);
> + if (ret)
> + return ret;
> +
> + if (*pmemphys > type_max(phys_addr_t))
> + return -EOVERFLOW;
> +
> + ret = device_property_read_u64(&pdev->dev,
> "nvidia,gpu-mem-size",
> + pmemlength);
> + if (ret)
> + return ret;
> +
> + if (*pmemlength > 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 (*pmemlength == 0)
> + return -ENOMEM;
> +
> + return ret;
> +}
> +
> +static int
> +nvgrace_gpu_init_nvdev_struct(struct pci_dev *pdev,
> + struct
> nvgrace_gpu_vfio_pci_core_device *nvdev,
> + u64 memphys, u64 memlength)
> +{
> + int ret = 0;
> +
> + /*
> + * The VM GPU device driver needs a non-cacheable region to
> support
> + * the MIG feature. Since the device memory is mapped as
> NORMAL cached,
> + * carve out a region from the end with a different NORMAL_NC
> + * property (called as reserved memory and represented as
> resmem). This
> + * region then is exposed as a 64b BAR (region 2 and 3) to
> the VM, while
> + * exposing the rest (termed as usable memory and
> represented using usemem)
> + * as cacheable 64b BAR (region 4 and 5).
> + *
> + * devmem (memlength)
> + * |-------------------------------------------------|
> + * | |
> + * usemem.phys/memphys resmem.phys
> + */
> + nvdev->usemem.memphys = memphys;
> +
> + /*
> + * The device memory exposed to the VM is added to the
> kernel by the
> + * VM driver module in chunks of memory block size. Only the
> usable
> + * memory (usemem) is added to the kernel for usage by the VM
> + * workloads. Make the usable memory size memblock aligned.
> + */
> + if (check_sub_overflow(memlength, RESMEM_SIZE,
> + &nvdev->usemem.memlength)) {
> + ret = -EOVERFLOW;
> + goto done;
> + }
> + nvdev->usemem.memlength = round_down(nvdev->usemem.memlength,
> + MEMBLK_SIZE);
> + if ((check_add_overflow(nvdev->usemem.memphys,
> + nvdev->usemem.memlength,
> + &nvdev->resmem.memphys)) ||
> + (check_sub_overflow(memlength, nvdev->usemem.memlength,
> + &nvdev->resmem.memlength))) {
> + ret = -EOVERFLOW;
> + goto done;
> + }
> +
> + /*
> + * The memory regions are exposed as BARs. Calculate and save
> + * the BAR size for them.
> + */
> + nvdev->usemem.bar_size =
> roundup_pow_of_two(nvdev->usemem.memlength);
> + nvdev->resmem.bar_size =
> roundup_pow_of_two(nvdev->resmem.memlength); +done:
> + return ret;
> +}
> +
> +static int nvgrace_gpu_probe(struct pci_dev *pdev,
> + const struct pci_device_id *id)
> +{
> + const struct vfio_device_ops *ops =
> &nvgrace_gpu_vfio_pci_core_ops;
> + struct nvgrace_gpu_vfio_pci_core_device *nvdev;
> + u64 memphys, memlength;
> + int ret;
> +
> + ret = nvgrace_gpu_fetch_memory_property(pdev, &memphys,
> &memlength);
> + if (!ret)
> + ops = &nvgrace_gpu_vfio_pci_ops;
> +
> + nvdev = vfio_alloc_device(nvgrace_gpu_vfio_pci_core_device,
> core_device.vdev,
> + &pdev->dev, ops);
> + if (IS_ERR(nvdev))
> + return PTR_ERR(nvdev);
> +
> + dev_set_drvdata(&pdev->dev, &nvdev->core_device);
> +
> + if (ops == &nvgrace_gpu_vfio_pci_ops) {
> + /*
> + * Device memory properties are identified in the
> host ACPI
> + * table. Set the nvgrace_gpu_vfio_pci_core_device
> structure.
> + */
> + ret = nvgrace_gpu_init_nvdev_struct(pdev, nvdev,
> + memphys,
> memlength);
> + 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_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_probe,
> + .remove = nvgrace_gpu_remove,
> + .err_handler = &vfio_pci_core_err_handlers,
> + .driver_managed_dma = true,
> +};
> +
> +module_pci_driver(nvgrace_gpu_vfio_pci_driver);
> +
> +MODULE_LICENSE("GPL");
> +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");

2024-02-07 23:28:00

[permalink] [raw]

Subject: Re: [PATCH v17 3/3] vfio/nvgrace-gpu: Add vfio pci variant module for grace hopper

On Thu, 8 Feb 2024 00:32:10 +0200
Zhi Wang <[email protected]> wrote:

> On Tue, 6 Feb 2024 04:31:23 +0530
> <[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 chip-to-chip cache coherent interconnect.
> >
> > The device is peculiar compared to a real PCI device in that whilst
> > there is a real 64b PCI BAR1 (comprising region 2 & region 3) on the
> > device, it is not used to access device memory once the faster
> > chip-to-chip interconnect is initialized (occurs at the time of host
> > system boot). The device memory is accessed instead using the
> > chip-to-chip interconnect that is exposed as a contiguous physically
> > addressable region on the host. This device memory aperture can be
> > obtained from host ACPI table using device_property_read_u64(),
> > according to the FW specification. Since the device memory is cache
> > coherent with the CPU, it can be mmap into the user VMA with a
> > cacheable mapping using remap_pfn_range() and used like a regular
> > RAM. The device memory is not added to the host kernel, but mapped
> > directly as this reduces memory wastage due to struct pages.
> >
> > There is also a requirement of a reserved 1G uncached region (termed
> > as resmem) to support the Multi-Instance GPU (MIG) feature [1]. This
> > is to work around a HW defect. Based on [2], the requisite properties
> > (uncached, unaligned access) can be achieved through a VM mapping (S1)
> > of NORMAL_NC and host (S2) mapping with MemAttr[2:0]=0b101. To provide
> > a different non-cached property to the reserved 1G region, it needs to
> > be carved out from the device memory and mapped as a separate region
> > in Qemu VMA with pgprot_writecombine(). pgprot_writecombine() sets the
> > Qemu VMA page properties (pgprot) as NORMAL_NC.
> >
> > Provide a VFIO PCI variant driver that adapts the unique device memory
> > representation into a more standard PCI representation facing
> > userspace.
> >
> > The variant driver exposes these two regions - the non-cached reserved
> > (resmem) and the cached rest of the device memory (termed as usemem)
> > as separate VFIO 64b BAR regions. This is divergent from the baremetal
> > approach, where the device memory is exposed as a device memory
> > region. The decision for a different approach was taken in view of
> > the fact that it would necessiate additional code in Qemu to discover
> > and insert those regions in the VM IPA, along with the additional VM
> > ACPI DSDT changes to communicate the device memory region IPA to the
> > VM workloads. Moreover, this behavior would have to be added to a
> > variety of emulators (beyond top of tree Qemu) out there desiring
> > grace hopper support.
> >
> > Since the device implements 64-bit BAR0, the VFIO PCI variant driver
> > maps the uncached carved out region to the next available PCI BAR
> > (i.e. comprising of region 2 and 3). The cached device memory
> > aperture is assigned BAR region 4 and 5. Qemu will then naturally
> > generate a PCI device in the VM with the uncached aperture reported
> > as BAR2 region, the cacheable as BAR4. The variant driver provides
> > emulation for these fake BARs' PCI config space offset registers.
> >
> > The hardware ensures that the system does not crash when the memory
> > is accessed with the memory enable turned off. It synthesis ~0 reads
> > and dropped writes on such access. So there is no need to support the
> > disablement/enablement of BAR through PCI_COMMAND config space
> > register.
> >
> > The memory layout on the host looks like the following:
> > devmem (memlength)
> > |--------------------------------------------------|
> > |-------------cached------------------------|--NC--|
> > | |
> > usemem.phys/memphys resmem.phys
> >
> > PCI BARs need to be 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 [6] 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.
> >
> > Note that the usemem memory is added by the VM Nvidia device driver
> > [5] to the VM kernel as memblocks. Hence make the usable memory size
> > memblock aligned.
> >
> > Currently there is no provision in KVM for a S2 mapping with
> > MemAttr[2:0]=0b101, but there is an ongoing effort to provide the
> > same [3]. As previously mentioned, resmem is mapped
> > pgprot_writecombine(), that sets the Qemu VMA page properties
> > (pgprot) as NORMAL_NC. Using the proposed changes in [4] and [3], KVM
> > marks the region with MemAttr[2:0]=0b101 in S2.
> >
> > If the bare metal properties are not present, the driver registers the
> > vfio-pci-core function pointers.
> >
> > This goes along with a qemu series [6] 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.
> >
> > [1] https://www.nvidia.com/en-in/technologies/multi-instance-gpu/
> > [2] section D8.5.5 of
> > https://developer.arm.com/documentation/ddi0487/latest/ [3]
> > https://lore.kernel.org/all/[email protected]/
> > [4]
> > https://lore.kernel.org/all/[email protected]/
> > [5] https://github.com/NVIDIA/open-gpu-kernel-modules [6]
> > https://lore.kernel.org/all/[email protected]/
> >
> > Signed-off-by: Aniket Agashe <[email protected]>
> > Signed-off-by: Ankit Agrawal <[email protected]>
> > ---
> > 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 | 856
> > ++++++++++++++++++++++++++ 6 files changed, 879 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 8999497011a2..529ec8966f58 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -23103,6 +23103,12 @@ L: [email protected]
> > S: Maintained
> > F: drivers/vfio/platform/
> >
> > +VFIO NVIDIA GRACE GPU DRIVER
> > +M: Ankit Agrawal <[email protected]>
> > +L: [email protected]
> > +S: Supported
> > +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 18c397df566d..15821a2d77d2 100644
> > --- a/drivers/vfio/pci/Kconfig
> > +++ b/drivers/vfio/pci/Kconfig
> > @@ -67,4 +67,6 @@ source "drivers/vfio/pci/pds/Kconfig"
> >
> > source "drivers/vfio/pci/virtio/Kconfig"
> >
> > +source "drivers/vfio/pci/nvgrace-gpu/Kconfig"
> > +
> > endmenu
> > diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile
> > index 046139a4eca5..ce7a61f1d912 100644
> > --- a/drivers/vfio/pci/Makefile
> > +++ b/drivers/vfio/pci/Makefile
> > @@ -15,3 +15,5 @@ obj-$(CONFIG_HISI_ACC_VFIO_PCI) += hisilicon/
> > obj-$(CONFIG_PDS_VFIO_PCI) += pds/
> >
> > obj-$(CONFIG_VIRTIO_VFIO_PCI) += virtio/
> > +
> > +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..6279af2bc6b8
> > --- /dev/null
> > +++ b/drivers/vfio/pci/nvgrace-gpu/main.c
> > @@ -0,0 +1,856 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (c) 2024, NVIDIA CORPORATION & AFFILIATES. All rights
> > reserved
> > + */
> > +
> > +#include <linux/vfio_pci_core.h>
> > +
> > +/*
> > + * The device memory usable to the workloads running in the VM is
> > cached
> > + * and showcased as a 64b device BAR (comprising of BAR4 and BAR5
> > region)
> > + * to the VM and is represented as usemem.
> > + * Moreover, the VM GPU device driver needs a non-cacheable region to
> > + * support the MIG feature. This region is also exposed as a 64b BAR
> > + * (comprising of BAR2 and BAR3 region) and represented as resmem.
> > + */
> > +#define RESMEM_REGION_INDEX VFIO_PCI_BAR2_REGION_INDEX
> > +#define USEMEM_REGION_INDEX VFIO_PCI_BAR4_REGION_INDEX
> > +
> > +/* Memory size expected as non cached and reserved by the VM driver
> > */ +#define RESMEM_SIZE 0x40000000
> > +#define MEMBLK_SIZE 0x20000000
> > +
>
> Maybe use SZ_* definitions in linux/size.h

Good suggestion.

>
> > +/*
> > + * The state of the two device memory region - resmem and usemem - is
> > + * saved as struct mem_region.
> > + */
> > +struct mem_region {
> > + phys_addr_t memphys; /* Base physical address of the
> > region */
> > + size_t memlength; /* Region size */
> > + size_t bar_size; /* Reported region BAR size */
> > + __le64 bar_val; /* Emulated BAR offset registers */
> > + union {
> > + void *memaddr;
> > + void __iomem *ioaddr;
> > + }; /* Base virtual address of the
> > region */ +};
> > +
> > +struct nvgrace_gpu_vfio_pci_core_device {
> > + struct vfio_pci_core_device core_device;
> > + /* Cached and usable memory for the VM. */
> > + struct mem_region usemem;
> > + /* Non cached memory carved out from the end of device
> > memory */
> > + struct mem_region resmem;
> > + /* Lock to control device memory kernel mapping */
> > + struct mutex remap_lock;
> > +};
> > +
> > +static void nvgrace_gpu_init_fake_bar_emu_regs(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);
> > +
> > + nvdev->resmem.bar_val = 0;
> > + nvdev->usemem.bar_val = 0;
> > +}
> > +
> > +/* Choose the structure corresponding to the fake BAR with a given
> > index. */ +static struct mem_region *
> > +nvgrace_gpu_memregion(int index,
> > + struct nvgrace_gpu_vfio_pci_core_device *nvdev)
> > +{
> > + if (index == USEMEM_REGION_INDEX)
> > + return &nvdev->usemem;
> > +
> > + if (index == RESMEM_REGION_INDEX)
> > + return &nvdev->resmem;
> > +
> > + return NULL;
> > +}
> > +
> > +static int nvgrace_gpu_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);
> > +
> ---
> > + if (nvdev->usemem.memlength) {
> > + nvgrace_gpu_init_fake_bar_emu_regs(core_vdev);
> > + mutex_init(&nvdev->remap_lock);
> > + }
> > +
> ---
>
> Better move this part to the place between vfio_pci_core_enable() and
> vfio_pci_core_finish_enable() like others for respecting the expected
> device initialization sequence of life cycle.
>
> It doesn't bite something right now, but think about when someone
> changes the behavior of vfio_pci_core_finish_enable() in the future,
> they have to propose a patch for this.

Agree.

> > + return 0;
> > +}
> > +
> > +static void nvgrace_gpu_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);
> > +
>
> Shouldn't there be a lock/unlock sequence when using/freeing the map? As
> the vfio device open/close paths are protected by group/dev_set lock,
> probably they are fine without driver-level lock protection. But
> for vfio_device_read/write/ioctl, if they are racing with open/close
> path, I think there should be a lock/unlock to protect the map.

This should only be called when the vfio device files is no longer
opened and therefore cannot have a race with other userspace access.

> ---
> > + /* Unmap the mapping to the device memory cached region */
> > + if (nvdev->usemem.memaddr) {
> > + memunmap(nvdev->usemem.memaddr);
> > + nvdev->usemem.memaddr = NULL;
> > + }
> > +
> > + /* Unmap the mapping to the device memory non-cached region
> > */
> > + if (nvdev->resmem.ioaddr) {
> > + iounmap(nvdev->resmem.ioaddr);
> > + nvdev->resmem.ioaddr = NULL;
> > + }
> > +
> ---
>
> > + mutex_destroy(&nvdev->remap_lock);
> > +
> > + vfio_pci_core_close_device(core_vdev);
> > +}
> > +
> > +static int nvgrace_gpu_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;
> > + struct mem_region *memregion;
> > +
> > + index = vma->vm_pgoff >> (VFIO_PCI_OFFSET_SHIFT -
> > PAGE_SHIFT); +
> > + memregion = nvgrace_gpu_memregion(index, nvdev);
> > + if (!memregion)
> > + 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(memregion->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 > memregion->memlength)
> > + return -EINVAL;
> > +
> > + /*
> > + * The carved out region of the device memory needs the
> > NORMAL_NC
> > + * property. Communicate as such to the hypervisor.
> > + */
> > + if (index == RESMEM_REGION_INDEX)
> > + vma->vm_page_prot =
> > pgprot_writecombine(vma->vm_page_prot); +
> > + /*
> > + * 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. The
> > + * remainder is only backed by vfio_device_ops read/write
> > handlers.
> > + *
> > + * 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_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_cap_sparse_mmap *sparse;
> > + struct vfio_info_cap caps = { .buf = NULL, .size = 0 };
> > + struct vfio_region_info info;
> > + struct mem_region *memregion;
> > + u32 size;
> > + int ret;
> > +
> > + if (copy_from_user(&info, (void __user *)arg, minsz))
> > + return -EFAULT;
> > +
> > + if (info.argsz < minsz)
> > + return -EINVAL;
> > +
> > + memregion = nvgrace_gpu_memregion(info.index, nvdev);
> > + if (!memregion)
> > + return vfio_pci_core_ioctl(core_vdev,
> > +
> > VFIO_DEVICE_GET_REGION_INFO, arg); +
> > + /*
> > + * Request to determine the BAR region information. Send the
> > + * GPU memory information.
> > + */
> > + 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 = memregion->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 region memory size may not be power-of-2 aligned.
> > + * Given that the memory as a BAR and may not be
> > + * aligned, roundup to the next power-of-2.
> > + */
> > + info.size = memregion->bar_size;
> > + 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;
> > +}
> > +
> > +static long nvgrace_gpu_ioctl(struct vfio_device *core_vdev,
> > + unsigned int cmd, unsigned long arg)
> > +{
> > + switch (cmd) {
> > + case VFIO_DEVICE_GET_REGION_INFO:
> > + return nvgrace_gpu_ioctl_get_region_info(core_vdev,
> > arg);
> > + case VFIO_DEVICE_IOEVENTFD:
> > + return -ENOTTY;
> > + case VFIO_DEVICE_RESET:
> > + nvgrace_gpu_init_fake_bar_emu_regs(core_vdev);
> > + fallthrough;
> > + default:
> > + return vfio_pci_core_ioctl(core_vdev, cmd, arg);
> > + }
> > +}
> > +
> > +static __le64
> > +nvgrace_gpu_get_read_value(size_t bar_size, u64 flags, __le64 val64)
> > +{
> > + u64 tmp_val;
> > +
> > + tmp_val = le64_to_cpu(val64);
> > + tmp_val &= ~(bar_size - 1);
> > + tmp_val |= flags;
> > +
> > + return cpu_to_le64(tmp_val);
> > +}
> > +
> > +/*
> > + * Both the usable (usemem) and the reserved (resmem) device memory
> > region
> > + * are exposed as a 64b fake BARs in the VM. These fake BARs must
> > respond
> > + * to the accesses on their respective PCI config space offsets.
> > + *
> > + * resmem BAR owns PCI_BASE_ADDRESS_2 & PCI_BASE_ADDRESS_3.
> > + * usemem BAR owns PCI_BASE_ADDRESS_4 & PCI_BASE_ADDRESS_5.
> > + */
> > +static ssize_t
> > +nvgrace_gpu_read_config_emu(struct vfio_device *core_vdev,
> > + char __user *buf, size_t count, loff_t
> > *ppos) +{
> > + struct nvgrace_gpu_vfio_pci_core_device *nvdev =
> > + container_of(core_vdev, struct
> > nvgrace_gpu_vfio_pci_core_device,
> > + core_device.vdev);
> > + struct mem_region *memregion = NULL;
> > + u64 pos = *ppos & VFIO_PCI_OFFSET_MASK;
> > + __le64 val64;
> > + size_t register_offset;
> > + loff_t copy_offset;
> > + size_t copy_count;
> > + int ret;
> > +
> > + ret = vfio_pci_core_read(core_vdev, buf, count, ppos);
> > + if (ret < 0)
> > + return ret;
> > +
> > + if (vfio_pci_core_range_intersect_range(pos, count,
> > PCI_BASE_ADDRESS_2,
> > + sizeof(val64),
> > + &copy_offset,
> > &copy_count,
> > + &register_offset))
> > + memregion =
> > nvgrace_gpu_memregion(RESMEM_REGION_INDEX, nvdev);
> > + else if (vfio_pci_core_range_intersect_range(pos, count,
> > +
> > PCI_BASE_ADDRESS_4,
> > + sizeof(val64),
> > + &copy_offset,
> > &copy_count,
> > +
> > &register_offset))
> > + memregion =
> > nvgrace_gpu_memregion(USEMEM_REGION_INDEX, nvdev); +
> > + if (memregion) {
> > + val64 =
> > nvgrace_gpu_get_read_value(memregion->bar_size,
> > +
> > PCI_BASE_ADDRESS_MEM_TYPE_64 |
> > +
> > PCI_BASE_ADDRESS_MEM_PREFETCH,
> > +
> > memregion->bar_val);
> > + if (copy_to_user(buf + copy_offset,
> > + (void *)&val64 + register_offset,
> > copy_count))
> > + return -EFAULT;
> > + }
> > +
> > + return count;
> > +}
> > +
> > +static ssize_t
> > +nvgrace_gpu_write_config_emu(struct vfio_device *core_vdev,
> > + const char __user *buf, size_t count,
> > loff_t *ppos) +{
> > + struct nvgrace_gpu_vfio_pci_core_device *nvdev =
> > + container_of(core_vdev, struct
> > nvgrace_gpu_vfio_pci_core_device,
> > + core_device.vdev);
> > + u64 pos = *ppos & VFIO_PCI_OFFSET_MASK;
> > + size_t register_offset;
> > + loff_t copy_offset;
> > + size_t copy_count;
> > + struct mem_region *memregion = NULL;
> > +
> > + if (vfio_pci_core_range_intersect_range(pos, count,
> > PCI_BASE_ADDRESS_2,
> > + sizeof(u64),
> > &copy_offset,
> > + &copy_count,
> > &register_offset))
> > + memregion =
> > nvgrace_gpu_memregion(RESMEM_REGION_INDEX, nvdev);
> > + else if (vfio_pci_core_range_intersect_range(pos, count,
> > PCI_BASE_ADDRESS_4,
> > + sizeof(u64),
> > &copy_offset,
> > + &copy_count,
> > &register_offset))
> > + memregion =
> > nvgrace_gpu_memregion(USEMEM_REGION_INDEX, nvdev); +
> > + if (memregion) {
> > + if (copy_from_user((void *)&memregion->bar_val +
> > register_offset,
> > + buf + copy_offset, copy_count))
> > + return -EFAULT;
> > + *ppos += copy_count;
> > + return copy_count;
> > + }
> > +
> > + return vfio_pci_core_write(core_vdev, buf, count, ppos);
> > +}
> > +
> > +/*
> > + * Ad hoc map the device memory in the module kernel VA space.
> > Primarily needed
> > + * as vfio does not require the userspace driver to only perform
> > accesses through
> > + * mmaps of the vfio-pci BAR regions and such accesses should be
> > supported using
> > + * vfio_device_ops read/write implementations.
> > + *
> > + * The usemem region is cacheable memory and hence is memremaped.
> > + * The resmem region is non-cached and is mapped using ioremap_wc
> > (NORMAL_NC).
> > + */
> > +static int
> > +nvgrace_gpu_map_device_mem(int index,
> > + struct nvgrace_gpu_vfio_pci_core_device
> > *nvdev) +{
> > + struct mem_region *memregion;
> > + int ret = 0;
> > +
> > + memregion = nvgrace_gpu_memregion(index, nvdev);
> > + if (!memregion)
> > + return -EINVAL;
> > +
> > + mutex_lock(&nvdev->remap_lock);
> > + if (index == USEMEM_REGION_INDEX && !memregion->memaddr) {
> > + memregion->memaddr = memremap(memregion->memphys,
> > + memregion->memlength,
> > + MEMREMAP_WB);
> > + if (!memregion->memaddr)
> > + ret = -ENOMEM;
> > + } else if (index == RESMEM_REGION_INDEX &&
> > !memregion->ioaddr) {
> > + memregion->ioaddr = ioremap_wc(memregion->memphys,
> > + memregion->memlength);
> > + if (!memregion->ioaddr)
> > + ret = -ENOMEM;
> > + }
> > + mutex_unlock(&nvdev->remap_lock);
> > +
> > + return ret;
> > +}
> > +
> > +/*
> > + * Read the data from the device memory (mapped either through
> > ioremap
> > + * or memremap) into the user buffer.
> > + */
> > +static int
> > +nvgrace_gpu_map_and_read(struct nvgrace_gpu_vfio_pci_core_device
> > *nvdev,
> > + char __user *buf, size_t mem_count, loff_t
> > *ppos) +{
> > + unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos);
> > + u64 offset = *ppos & VFIO_PCI_OFFSET_MASK;
> > + int ret;
> > +
> > + /*
> > + * Handle read on the BAR regions. Map to the target device
> > memory
> > + * physical address and copy to the request read buffer.
> > + */
> > + ret = nvgrace_gpu_map_device_mem(index, nvdev);
> > + if (ret)
> > + return ret;
> > +
>
> Wouldn't it be better to do the map in the open path?

AIUI the device would typically be used exclusively through the mmap of
these ranges, these mappings are only for pread/pwrite type accesses,
so I think it makes sense to map them on demand.

> > + if (index == USEMEM_REGION_INDEX) {
> > + if (copy_to_user(buf,
> > + (u8 *)nvdev->usemem.memaddr +
> > offset,
> > + mem_count))
> > + ret = -EFAULT;
> > + } else {
> > + /*
> > + * The hardware ensures that the system does not
> > crash when
> > + * the device memory is accessed with the memory
> > enable
> > + * turned off. It synthesizes ~0 on such read. So
> > there is
> > + * no need to check or support the
> > disablement/enablement of
> > + * BAR through PCI_COMMAND config space register.
> > Pass
> > + * test_mem flag as false.
> > + */
> > + ret = vfio_pci_core_do_io_rw(&nvdev->core_device,
> > false,
> > + nvdev->resmem.ioaddr,
> > + buf, offset, mem_count,
> > + 0, 0, false);
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +/*
> > + * 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.
> > + *
> > + * Reads extending beyond the reported size are truncated; reads
> > starting
> > + * beyond the reported size generate -EINVAL; reads extending beyond
> > the
> > + * actual device size is filled with ~0.
> > + */
> > +static ssize_t
> > +nvgrace_gpu_read_mem(struct nvgrace_gpu_vfio_pci_core_device *nvdev,
> > + char __user *buf, size_t count, loff_t *ppos)
> > +{
> > + u64 offset = *ppos & VFIO_PCI_OFFSET_MASK;
> > + unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos);
> > + struct mem_region *memregion;
> > + size_t mem_count, i;
> > + u8 val = 0xFF;
> > + int ret;
> > +
> > + memregion = nvgrace_gpu_memregion(index, nvdev);
> > + if (!memregion)
> > + return -EINVAL;
> > +
> > + if (offset >= memregion->bar_size)
> > + return -EINVAL;
> > +
> > + /* Clip short the read request beyond reported BAR size */
> > + count = min(count, memregion->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 >= memregion->memlength)
> > + mem_count = 0;
>
> If mem_count == 0, going through nvgrace_gpu_map_and_read() is not
> necessary.

Harmless, other than the possibly unnecessary call through to
nvgrace_gpu_map_device_mem(). Maybe both nvgrace_gpu_map_and_read()
and nvgrace_gpu_map_and_write() could conditionally return 0 as their
first operation when !mem_count. Thanks,

Alex

> > + else
> > + mem_count = min(count, memregion->memlength -
> > (size_t)offset); +
> > + ret = nvgrace_gpu_map_and_read(nvdev, buf, mem_count, ppos);
> > + if (ret)
> > + return ret;
> > +
> > + /*
> > + * 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_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 (nvgrace_gpu_memregion(index, nvdev))
> > + return nvgrace_gpu_read_mem(nvdev, buf, count, ppos);
> > +
> > + if (index == VFIO_PCI_CONFIG_REGION_INDEX)
> > + return nvgrace_gpu_read_config_emu(core_vdev, buf,
> > count, ppos); +
> > + return vfio_pci_core_read(core_vdev, buf, count, ppos);
> > +}
> > +
> > +/*
> > + * Write the data to the device memory (mapped either through ioremap
> > + * or memremap) from the user buffer.
> > + */
> > +static int
> > +nvgrace_gpu_map_and_write(struct nvgrace_gpu_vfio_pci_core_device
> > *nvdev,
> > + const char __user *buf, size_t mem_count,
> > + loff_t *ppos)
> > +{
> > + unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos);
> > + loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK;
> > + int ret;
> > +
> > + ret = nvgrace_gpu_map_device_mem(index, nvdev);
> > + if (ret)
> > + return ret;
> > +
> > + if (index == USEMEM_REGION_INDEX) {
> > + if (copy_from_user((u8 *)nvdev->usemem.memaddr + pos,
> > + buf, mem_count))
> > + return -EFAULT;
> > + } else {
> > + /*
> > + * The hardware ensures that the system does not
> > crash when
> > + * the device memory is accessed with the memory
> > enable
> > + * turned off. It drops such writes. So there is no
> > need to
> > + * check or support the disablement/enablement of BAR
> > + * through PCI_COMMAND config space register. Pass
> > test_mem
> > + * flag as false.
> > + */
> > + ret = vfio_pci_core_do_io_rw(&nvdev->core_device,
> > false,
> > + nvdev->resmem.ioaddr,
> > + (char __user *)buf,
> > pos, mem_count,
> > + 0, 0, true);
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +/*
> > + * 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.
> > + *
> > + * Writes extending beyond the reported size are truncated; writes
> > starting
> > + * beyond the reported size generate -EINVAL.
> > + */
> > +static ssize_t
> > +nvgrace_gpu_write_mem(struct nvgrace_gpu_vfio_pci_core_device *nvdev,
> > + size_t count, loff_t *ppos, const char __user
> > *buf) +{
> > + u64 offset = *ppos & VFIO_PCI_OFFSET_MASK;
> > + unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos);
> > + struct mem_region *memregion;
> > + size_t mem_count;
> > + int ret = 0;
> > +
> > + memregion = nvgrace_gpu_memregion(index, nvdev);
> > + if (!memregion)
> > + return -EINVAL;
> > +
> > + if (offset >= memregion->bar_size)
> > + return -EINVAL;
> > +
> > + /* Clip short the write request beyond reported BAR size */
> > + count = min(count, memregion->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 >= memregion->memlength)
> > + goto exitfn;
> > +
> > + /*
> > + * Only the device memory present on the hardware is mapped,
> > which may
> > + * not be power-of-2 aligned. Drop access outside the
> > available device
> > + * memory on the hardware.
> > + */
> > + mem_count = min(count, memregion->memlength -
> > (size_t)offset); +
> > + ret = nvgrace_gpu_map_and_write(nvdev, buf, mem_count, ppos);
> > + if (ret)
> > + return ret;
> > +
> > +exitfn:
> > + *ppos += count;
> > + return count;
> > +}
> > +
> > +static ssize_t
> > +nvgrace_gpu_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 (nvgrace_gpu_memregion(index, nvdev))
> > + return nvgrace_gpu_write_mem(nvdev, count, ppos,
> > buf); +
> > + if (index == VFIO_PCI_CONFIG_REGION_INDEX)
> > + return nvgrace_gpu_write_config_emu(core_vdev, buf,
> > count, ppos); +
> > + 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_open_device,
> > + .close_device = nvgrace_gpu_close_device,
> > + .ioctl = nvgrace_gpu_ioctl,
> > + .read = nvgrace_gpu_read,
> > + .write = nvgrace_gpu_write,
> > + .mmap = nvgrace_gpu_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,
> > + .detach_ioas = vfio_iommufd_physical_detach_ioas,
> > +};
> > +
> > +static const struct vfio_device_ops nvgrace_gpu_vfio_pci_core_ops = {
> > + .name = "nvgrace-gpu-vfio-pci-core",
> > + .init = vfio_pci_core_init_dev,
> > + .release = vfio_pci_core_release_dev,
> > + .open_device = nvgrace_gpu_open_device,
> > + .close_device = vfio_pci_core_close_device,
> > + .ioctl = vfio_pci_core_ioctl,
> > + .device_feature = vfio_pci_core_ioctl_feature,
> > + .read = vfio_pci_core_read,
> > + .write = vfio_pci_core_write,
> > + .mmap = vfio_pci_core_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,
> > + .detach_ioas = vfio_iommufd_physical_detach_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_fetch_memory_property(struct pci_dev *pdev,
> > + u64 *pmemphys, u64 *pmemlength)
> > +{
> > + int ret;
> > +
> > + /*
> > + * 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",
> > + pmemphys);
> > + if (ret)
> > + return ret;
> > +
> > + if (*pmemphys > type_max(phys_addr_t))
> > + return -EOVERFLOW;
> > +
> > + ret = device_property_read_u64(&pdev->dev,
> > "nvidia,gpu-mem-size",
> > + pmemlength);
> > + if (ret)
> > + return ret;
> > +
> > + if (*pmemlength > 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 (*pmemlength == 0)
> > + return -ENOMEM;
> > +
> > + return ret;
> > +}
> > +
> > +static int
> > +nvgrace_gpu_init_nvdev_struct(struct pci_dev *pdev,
> > + struct
> > nvgrace_gpu_vfio_pci_core_device *nvdev,
> > + u64 memphys, u64 memlength)
> > +{
> > + int ret = 0;
> > +
> > + /*
> > + * The VM GPU device driver needs a non-cacheable region to
> > support
> > + * the MIG feature. Since the device memory is mapped as
> > NORMAL cached,
> > + * carve out a region from the end with a different NORMAL_NC
> > + * property (called as reserved memory and represented as
> > resmem). This
> > + * region then is exposed as a 64b BAR (region 2 and 3) to
> > the VM, while
> > + * exposing the rest (termed as usable memory and
> > represented using usemem)
> > + * as cacheable 64b BAR (region 4 and 5).
> > + *
> > + * devmem (memlength)
> > + * |-------------------------------------------------|
> > + * | |
> > + * usemem.phys/memphys resmem.phys
> > + */
> > + nvdev->usemem.memphys = memphys;
> > +
> > + /*
> > + * The device memory exposed to the VM is added to the
> > kernel by the
> > + * VM driver module in chunks of memory block size. Only the
> > usable
> > + * memory (usemem) is added to the kernel for usage by the VM
> > + * workloads. Make the usable memory size memblock aligned.
> > + */
> > + if (check_sub_overflow(memlength, RESMEM_SIZE,
> > + &nvdev->usemem.memlength)) {
> > + ret = -EOVERFLOW;
> > + goto done;
> > + }
> > + nvdev->usemem.memlength = round_down(nvdev->usemem.memlength,
> > + MEMBLK_SIZE);
> > + if ((check_add_overflow(nvdev->usemem.memphys,
> > + nvdev->usemem.memlength,
> > + &nvdev->resmem.memphys)) ||
> > + (check_sub_overflow(memlength, nvdev->usemem.memlength,
> > + &nvdev->resmem.memlength))) {
> > + ret = -EOVERFLOW;
> > + goto done;
> > + }
> > +
> > + /*
> > + * The memory regions are exposed as BARs. Calculate and save
> > + * the BAR size for them.
> > + */
> > + nvdev->usemem.bar_size =
> > roundup_pow_of_two(nvdev->usemem.memlength);
> > + nvdev->resmem.bar_size =
> > roundup_pow_of_two(nvdev->resmem.memlength); +done:
> > + return ret;
> > +}
> > +
> > +static int nvgrace_gpu_probe(struct pci_dev *pdev,
> > + const struct pci_device_id *id)
> > +{
> > + const struct vfio_device_ops *ops =
> > &nvgrace_gpu_vfio_pci_core_ops;
> > + struct nvgrace_gpu_vfio_pci_core_device *nvdev;
> > + u64 memphys, memlength;
> > + int ret;
> > +
> > + ret = nvgrace_gpu_fetch_memory_property(pdev, &memphys,
> > &memlength);
> > + if (!ret)
> > + ops = &nvgrace_gpu_vfio_pci_ops;
> > +
> > + nvdev = vfio_alloc_device(nvgrace_gpu_vfio_pci_core_device,
> > core_device.vdev,
> > + &pdev->dev, ops);
> > + if (IS_ERR(nvdev))
> > + return PTR_ERR(nvdev);
> > +
> > + dev_set_drvdata(&pdev->dev, &nvdev->core_device);
> > +
> > + if (ops == &nvgrace_gpu_vfio_pci_ops) {
> > + /*
> > + * Device memory properties are identified in the
> > host ACPI
> > + * table. Set the nvgrace_gpu_vfio_pci_core_device
> > structure.
> > + */
> > + ret = nvgrace_gpu_init_nvdev_struct(pdev, nvdev,
> > + memphys,
> > memlength);
> > + 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_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_probe,
> > + .remove = nvgrace_gpu_remove,
> > + .err_handler = &vfio_pci_core_err_handlers,
> > + .driver_managed_dma = true,
> > +};
> > +
> > +module_pci_driver(nvgrace_gpu_vfio_pci_driver);
> > +
> > +MODULE_LICENSE("GPL");
> > +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");
>

2024-02-07 23:34:49

[permalink] [raw]

Subject: Re: [PATCH v17 3/3] vfio/nvgrace-gpu: Add vfio pci variant module for grace hopper

On Tue, 6 Feb 2024 04:31:23 +0530
<[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 chip-to-chip cache coherent interconnect.
>
> The device is peculiar compared to a real PCI device in that whilst
> there is a real 64b PCI BAR1 (comprising region 2 & region 3) on the
> device, it is not used to access device memory once the faster
> chip-to-chip interconnect is initialized (occurs at the time of host
> system boot). The device memory is accessed instead using the chip-to-chip
> interconnect that is exposed as a contiguous physically addressable
> region on the host. This device memory aperture can be obtained from host
> ACPI table using device_property_read_u64(), according to the FW
> specification. Since the device memory is cache coherent with the CPU,
> it can be mmap into the user VMA with a cacheable mapping using
> remap_pfn_range() and used like a regular RAM. The device memory
> is not added to the host kernel, but mapped directly as this reduces
> memory wastage due to struct pages.
>
> There is also a requirement of a reserved 1G uncached region (termed as
> resmem) to support the Multi-Instance GPU (MIG) feature [1]. This is
> to work around a HW defect. Based on [2], the requisite properties
> (uncached, unaligned access) can be achieved through a VM mapping (S1)
> of NORMAL_NC and host (S2) mapping with MemAttr[2:0]=0b101. To provide
> a different non-cached property to the reserved 1G region, it needs to
> be carved out from the device memory and mapped as a separate region
> in Qemu VMA with pgprot_writecombine(). pgprot_writecombine() sets the
> Qemu VMA page properties (pgprot) as NORMAL_NC.
>
> Provide a VFIO PCI variant driver that adapts the unique device memory
> representation into a more standard PCI representation facing userspace.
>
> The variant driver exposes these two regions - the non-cached reserved
> (resmem) and the cached rest of the device memory (termed as usemem) as
> separate VFIO 64b BAR regions. This is divergent from the baremetal
> approach, where the device memory is exposed as a device memory region.
> The decision for a different approach was taken in view of the fact that
> it would necessiate additional code in Qemu to discover and insert those
> regions in the VM IPA, along with the additional VM ACPI DSDT changes to
> communicate the device memory region IPA to the VM workloads. Moreover,
> this behavior would have to be added to a variety of emulators (beyond
> top of tree Qemu) out there desiring grace hopper support.
>
> Since the device implements 64-bit BAR0, the VFIO PCI variant driver
> maps the uncached carved out region to the next available PCI BAR (i.e.
> comprising of region 2 and 3). The cached device memory aperture is
> assigned BAR region 4 and 5. Qemu will then naturally generate a PCI
> device in the VM with the uncached aperture reported as BAR2 region,
> the cacheable as BAR4. The variant driver provides emulation for these
> fake BARs' PCI config space offset registers.
>
> The hardware ensures that the system does not crash when the memory
> is accessed with the memory enable turned off. It synthesis ~0 reads
> and dropped writes on such access. So there is no need to support the
> disablement/enablement of BAR through PCI_COMMAND config space register.
>
> The memory layout on the host looks like the following:
> devmem (memlength)
> |--------------------------------------------------|
> |-------------cached------------------------|--NC--|
> | |
> usemem.phys/memphys resmem.phys
>
> PCI BARs need to be 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 [6] 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.
>
> Note that the usemem memory is added by the VM Nvidia device driver [5]
> to the VM kernel as memblocks. Hence make the usable memory size memblock
> aligned.
>
> Currently there is no provision in KVM for a S2 mapping with
> MemAttr[2:0]=0b101, but there is an ongoing effort to provide the same [3].
> As previously mentioned, resmem is mapped pgprot_writecombine(), that
> sets the Qemu VMA page properties (pgprot) as NORMAL_NC. Using the
> proposed changes in [4] and [3], KVM marks the region with
> MemAttr[2:0]=0b101 in S2.
>
> If the bare metal properties are not present, the driver registers the
> vfio-pci-core function pointers.
>
> This goes along with a qemu series [6] 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.
>
> [1] https://www.nvidia.com/en-in/technologies/multi-instance-gpu/
> [2] section D8.5.5 of https://developer.arm.com/documentation/ddi0487/latest/
> [3] https://lore.kernel.org/all/[email protected]/
> [4] https://lore.kernel.org/all/[email protected]/
> [5] https://github.com/NVIDIA/open-gpu-kernel-modules
> [6] https://lore.kernel.org/all/[email protected]/
>
> Signed-off-by: Aniket Agashe <[email protected]>
> Signed-off-by: Ankit Agrawal <[email protected]>
> ---
> 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 | 856 ++++++++++++++++++++++++++
> 6 files changed, 879 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 8999497011a2..529ec8966f58 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -23103,6 +23103,12 @@ L: [email protected]
> S: Maintained
> F: drivers/vfio/platform/
>
> +VFIO NVIDIA GRACE GPU DRIVER
> +M: Ankit Agrawal <[email protected]>
> +L: [email protected]
> +S: Supported
> +F: drivers/vfio/pci/nvgrace-gpu/
> +

Entries should be alphabetical. This will end up colliding with [1] so
I'll plan to fix it either way.

Otherwise just a couple optional comments from me below. I see Zhi also
has a few good comments. I'd suggest soliciting a review from the other
variant driver reviewers for this version and maybe we can make v18 the
final version. Thanks,

Alex

[1]https://lore.kernel.org/all/[email protected]/

> VGA_SWITCHEROO
> R: Lukas Wunner <[email protected]>
> S: Maintained
> diff --git a/drivers/vfio/pci/Kconfig b/drivers/vfio/pci/Kconfig
> index 18c397df566d..15821a2d77d2 100644
> --- a/drivers/vfio/pci/Kconfig
> +++ b/drivers/vfio/pci/Kconfig
> @@ -67,4 +67,6 @@ source "drivers/vfio/pci/pds/Kconfig"
>
> source "drivers/vfio/pci/virtio/Kconfig"
>
> +source "drivers/vfio/pci/nvgrace-gpu/Kconfig"
> +
> endmenu
> diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile
> index 046139a4eca5..ce7a61f1d912 100644
> --- a/drivers/vfio/pci/Makefile
> +++ b/drivers/vfio/pci/Makefile
> @@ -15,3 +15,5 @@ obj-$(CONFIG_HISI_ACC_VFIO_PCI) += hisilicon/
> obj-$(CONFIG_PDS_VFIO_PCI) += pds/
>
> obj-$(CONFIG_VIRTIO_VFIO_PCI) += virtio/
> +
> +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..6279af2bc6b8
> --- /dev/null
> +++ b/drivers/vfio/pci/nvgrace-gpu/main.c
> @@ -0,0 +1,856 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2024, NVIDIA CORPORATION & AFFILIATES. All rights reserved
> + */
> +
> +#include <linux/vfio_pci_core.h>
> +
> +/*
> + * The device memory usable to the workloads running in the VM is cached
> + * and showcased as a 64b device BAR (comprising of BAR4 and BAR5 region)
> + * to the VM and is represented as usemem.
> + * Moreover, the VM GPU device driver needs a non-cacheable region to
> + * support the MIG feature. This region is also exposed as a 64b BAR
> + * (comprising of BAR2 and BAR3 region) and represented as resmem.
> + */
> +#define RESMEM_REGION_INDEX VFIO_PCI_BAR2_REGION_INDEX
> +#define USEMEM_REGION_INDEX VFIO_PCI_BAR4_REGION_INDEX
> +
> +/* Memory size expected as non cached and reserved by the VM driver */
> +#define RESMEM_SIZE 0x40000000
> +#define MEMBLK_SIZE 0x20000000
> +
> +/*
> + * The state of the two device memory region - resmem and usemem - is
> + * saved as struct mem_region.
> + */
> +struct mem_region {
> + phys_addr_t memphys; /* Base physical address of the region */
> + size_t memlength; /* Region size */
> + size_t bar_size; /* Reported region BAR size */
> + __le64 bar_val; /* Emulated BAR offset registers */
> + union {
> + void *memaddr;
> + void __iomem *ioaddr;
> + }; /* Base virtual address of the region */
> +};
> +
> +struct nvgrace_gpu_vfio_pci_core_device {
> + struct vfio_pci_core_device core_device;
> + /* Cached and usable memory for the VM. */
> + struct mem_region usemem;
> + /* Non cached memory carved out from the end of device memory */
> + struct mem_region resmem;
> + /* Lock to control device memory kernel mapping */
> + struct mutex remap_lock;
> +};
> +
> +static void nvgrace_gpu_init_fake_bar_emu_regs(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);
> +
> + nvdev->resmem.bar_val = 0;
> + nvdev->usemem.bar_val = 0;
> +}
> +
> +/* Choose the structure corresponding to the fake BAR with a given index. */
> +static struct mem_region *
> +nvgrace_gpu_memregion(int index,
> + struct nvgrace_gpu_vfio_pci_core_device *nvdev)
> +{
> + if (index == USEMEM_REGION_INDEX)
> + return &nvdev->usemem;
> +
> + if (index == RESMEM_REGION_INDEX)
> + return &nvdev->resmem;
> +
> + return NULL;
> +}
> +
> +static int nvgrace_gpu_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);
> +
> + if (nvdev->usemem.memlength) {
> + nvgrace_gpu_init_fake_bar_emu_regs(core_vdev);
> + mutex_init(&nvdev->remap_lock);
> + }
> +
> + return 0;
> +}
> +
> +static void nvgrace_gpu_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);
> +
> + /* Unmap the mapping to the device memory cached region */
> + if (nvdev->usemem.memaddr) {
> + memunmap(nvdev->usemem.memaddr);
> + nvdev->usemem.memaddr = NULL;
> + }
> +
> + /* Unmap the mapping to the device memory non-cached region */
> + if (nvdev->resmem.ioaddr) {
> + iounmap(nvdev->resmem.ioaddr);
> + nvdev->resmem.ioaddr = NULL;
> + }
> +
> + mutex_destroy(&nvdev->remap_lock);
> +
> + vfio_pci_core_close_device(core_vdev);
> +}
> +
> +static int nvgrace_gpu_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;
> + struct mem_region *memregion;
> +
> + index = vma->vm_pgoff >> (VFIO_PCI_OFFSET_SHIFT - PAGE_SHIFT);
> +
> + memregion = nvgrace_gpu_memregion(index, nvdev);
> + if (!memregion)
> + 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(memregion->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 > memregion->memlength)
> + return -EINVAL;
> +
> + /*
> + * The carved out region of the device memory needs the NORMAL_NC
> + * property. Communicate as such to the hypervisor.
> + */
> + if (index == RESMEM_REGION_INDEX)
> + vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
> +
> + /*
> + * 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. The
> + * remainder is only backed by vfio_device_ops read/write handlers.
> + *
> + * 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_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_cap_sparse_mmap *sparse;
> + struct vfio_info_cap caps = { .buf = NULL, .size = 0 };
> + struct vfio_region_info info;
> + struct mem_region *memregion;
> + u32 size;
> + int ret;
> +
> + if (copy_from_user(&info, (void __user *)arg, minsz))
> + return -EFAULT;
> +
> + if (info.argsz < minsz)
> + return -EINVAL;
> +
> + memregion = nvgrace_gpu_memregion(info.index, nvdev);
> + if (!memregion)
> + return vfio_pci_core_ioctl(core_vdev,
> + VFIO_DEVICE_GET_REGION_INFO, arg);
> +
> + /*
> + * Request to determine the BAR region information. Send the
> + * GPU memory information.
> + */
> + 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 = memregion->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 region memory size may not be power-of-2 aligned.
> + * Given that the memory as a BAR and may not be
> + * aligned, roundup to the next power-of-2.
> + */
> + info.size = memregion->bar_size;
> + 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;
> +}
> +
> +static long nvgrace_gpu_ioctl(struct vfio_device *core_vdev,
> + unsigned int cmd, unsigned long arg)
> +{
> + switch (cmd) {
> + case VFIO_DEVICE_GET_REGION_INFO:
> + return nvgrace_gpu_ioctl_get_region_info(core_vdev, arg);
> + case VFIO_DEVICE_IOEVENTFD:
> + return -ENOTTY;
> + case VFIO_DEVICE_RESET:
> + nvgrace_gpu_init_fake_bar_emu_regs(core_vdev);
> + fallthrough;
> + default:
> + return vfio_pci_core_ioctl(core_vdev, cmd, arg);
> + }
> +}
> +
> +static __le64
> +nvgrace_gpu_get_read_value(size_t bar_size, u64 flags, __le64 val64)
> +{
> + u64 tmp_val;
> +
> + tmp_val = le64_to_cpu(val64);
> + tmp_val &= ~(bar_size - 1);
> + tmp_val |= flags;
> +
> + return cpu_to_le64(tmp_val);
> +}
> +
> +/*
> + * Both the usable (usemem) and the reserved (resmem) device memory region
> + * are exposed as a 64b fake BARs in the VM. These fake BARs must respond
> + * to the accesses on their respective PCI config space offsets.
> + *
> + * resmem BAR owns PCI_BASE_ADDRESS_2 & PCI_BASE_ADDRESS_3.
> + * usemem BAR owns PCI_BASE_ADDRESS_4 & PCI_BASE_ADDRESS_5.
> + */
> +static ssize_t
> +nvgrace_gpu_read_config_emu(struct vfio_device *core_vdev,
> + char __user *buf, size_t count, loff_t *ppos)
> +{
> + struct nvgrace_gpu_vfio_pci_core_device *nvdev =
> + container_of(core_vdev, struct nvgrace_gpu_vfio_pci_core_device,
> + core_device.vdev);
> + struct mem_region *memregion = NULL;
> + u64 pos = *ppos & VFIO_PCI_OFFSET_MASK;
> + __le64 val64;
> + size_t register_offset;
> + loff_t copy_offset;
> + size_t copy_count;
> + int ret;
> +
> + ret = vfio_pci_core_read(core_vdev, buf, count, ppos);
> + if (ret < 0)
> + return ret;
> +
> + if (vfio_pci_core_range_intersect_range(pos, count, PCI_BASE_ADDRESS_2,
> + sizeof(val64),
> + &copy_offset, &copy_count,
> + &register_offset))
> + memregion = nvgrace_gpu_memregion(RESMEM_REGION_INDEX, nvdev);
> + else if (vfio_pci_core_range_intersect_range(pos, count,
> + PCI_BASE_ADDRESS_4,
> + sizeof(val64),
> + &copy_offset, &copy_count,
> + &register_offset))
> + memregion = nvgrace_gpu_memregion(USEMEM_REGION_INDEX, nvdev);
> +
> + if (memregion) {
> + val64 = nvgrace_gpu_get_read_value(memregion->bar_size,
> + PCI_BASE_ADDRESS_MEM_TYPE_64 |
> + PCI_BASE_ADDRESS_MEM_PREFETCH,
> + memregion->bar_val);
> + if (copy_to_user(buf + copy_offset,
> + (void *)&val64 + register_offset, copy_count))
> + return -EFAULT;
> + }
> +
> + return count;
> +}
> +
> +static ssize_t
> +nvgrace_gpu_write_config_emu(struct vfio_device *core_vdev,
> + const char __user *buf, size_t count, loff_t *ppos)
> +{
> + struct nvgrace_gpu_vfio_pci_core_device *nvdev =
> + container_of(core_vdev, struct nvgrace_gpu_vfio_pci_core_device,
> + core_device.vdev);
> + u64 pos = *ppos & VFIO_PCI_OFFSET_MASK;
> + size_t register_offset;
> + loff_t copy_offset;
> + size_t copy_count;
> + struct mem_region *memregion = NULL;

Nit, consistency and reverse Christmas tree variable declaration would
suggest pushing this up in the list, but it's not strictly required.

> +
> + if (vfio_pci_core_range_intersect_range(pos, count, PCI_BASE_ADDRESS_2,
> + sizeof(u64), &copy_offset,
> + &copy_count, &register_offset))
> + memregion = nvgrace_gpu_memregion(RESMEM_REGION_INDEX, nvdev);
> + else if (vfio_pci_core_range_intersect_range(pos, count, PCI_BASE_ADDRESS_4,
> + sizeof(u64), &copy_offset,
> + &copy_count, &register_offset))
> + memregion = nvgrace_gpu_memregion(USEMEM_REGION_INDEX, nvdev);
> +
> + if (memregion) {
> + if (copy_from_user((void *)&memregion->bar_val + register_offset,
> + buf + copy_offset, copy_count))
> + return -EFAULT;
> + *ppos += copy_count;
> + return copy_count;
> + }
> +
> + return vfio_pci_core_write(core_vdev, buf, count, ppos);
> +}
> +
> +/*
> + * Ad hoc map the device memory in the module kernel VA space. Primarily needed
> + * as vfio does not require the userspace driver to only perform accesses through
> + * mmaps of the vfio-pci BAR regions and such accesses should be supported using
> + * vfio_device_ops read/write implementations.
> + *
> + * The usemem region is cacheable memory and hence is memremaped.
> + * The resmem region is non-cached and is mapped using ioremap_wc (NORMAL_NC).
> + */
> +static int
> +nvgrace_gpu_map_device_mem(int index,
> + struct nvgrace_gpu_vfio_pci_core_device *nvdev)
> +{
> + struct mem_region *memregion;
> + int ret = 0;
> +
> + memregion = nvgrace_gpu_memregion(index, nvdev);
> + if (!memregion)
> + return -EINVAL;
> +
> + mutex_lock(&nvdev->remap_lock);
> + if (index == USEMEM_REGION_INDEX && !memregion->memaddr) {
> + memregion->memaddr = memremap(memregion->memphys,
> + memregion->memlength,
> + MEMREMAP_WB);
> + if (!memregion->memaddr)
> + ret = -ENOMEM;
> + } else if (index == RESMEM_REGION_INDEX && !memregion->ioaddr) {
> + memregion->ioaddr = ioremap_wc(memregion->memphys,
> + memregion->memlength);
> + if (!memregion->ioaddr)
> + ret = -ENOMEM;
> + }

As .memaddr and .ioaddr are a union we can consolidate the NULL test.

> + mutex_unlock(&nvdev->remap_lock);
> +
> + return ret;
> +}
> +
> +/*
> + * Read the data from the device memory (mapped either through ioremap
> + * or memremap) into the user buffer.
> + */
> +static int
> +nvgrace_gpu_map_and_read(struct nvgrace_gpu_vfio_pci_core_device *nvdev,
> + char __user *buf, size_t mem_count, loff_t *ppos)
> +{
> + unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos);
> + u64 offset = *ppos & VFIO_PCI_OFFSET_MASK;
> + int ret;
> +
> + /*
> + * Handle read on the BAR regions. Map to the target device memory
> + * physical address and copy to the request read buffer.
> + */
> + ret = nvgrace_gpu_map_device_mem(index, nvdev);
> + if (ret)
> + return ret;
> +
> + if (index == USEMEM_REGION_INDEX) {
> + if (copy_to_user(buf,
> + (u8 *)nvdev->usemem.memaddr + offset,
> + mem_count))
> + ret = -EFAULT;
> + } else {
> + /*
> + * The hardware ensures that the system does not crash when
> + * the device memory is accessed with the memory enable
> + * turned off. It synthesizes ~0 on such read. So there is
> + * no need to check or support the disablement/enablement of
> + * BAR through PCI_COMMAND config space register. Pass
> + * test_mem flag as false.
> + */
> + ret = vfio_pci_core_do_io_rw(&nvdev->core_device, false,
> + nvdev->resmem.ioaddr,
> + buf, offset, mem_count,
> + 0, 0, false);
> + }
> +
> + return ret;
> +}
> +
> +/*
> + * 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.
> + *
> + * Reads extending beyond the reported size are truncated; reads starting
> + * beyond the reported size generate -EINVAL; reads extending beyond the
> + * actual device size is filled with ~0.
> + */
> +static ssize_t
> +nvgrace_gpu_read_mem(struct nvgrace_gpu_vfio_pci_core_device *nvdev,
> + char __user *buf, size_t count, loff_t *ppos)
> +{
> + u64 offset = *ppos & VFIO_PCI_OFFSET_MASK;
> + unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos);
> + struct mem_region *memregion;
> + size_t mem_count, i;
> + u8 val = 0xFF;
> + int ret;
> +
> + memregion = nvgrace_gpu_memregion(index, nvdev);
> + if (!memregion)
> + return -EINVAL;
> +
> + if (offset >= memregion->bar_size)
> + return -EINVAL;
> +
> + /* Clip short the read request beyond reported BAR size */
> + count = min(count, memregion->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 >= memregion->memlength)
> + mem_count = 0;
> + else
> + mem_count = min(count, memregion->memlength - (size_t)offset);
> +
> + ret = nvgrace_gpu_map_and_read(nvdev, buf, mem_count, ppos);
> + if (ret)
> + return ret;
> +
> + /*
> + * 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_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 (nvgrace_gpu_memregion(index, nvdev))
> + return nvgrace_gpu_read_mem(nvdev, buf, count, ppos);
> +
> + if (index == VFIO_PCI_CONFIG_REGION_INDEX)
> + return nvgrace_gpu_read_config_emu(core_vdev, buf, count, ppos);
> +
> + return vfio_pci_core_read(core_vdev, buf, count, ppos);
> +}
> +
> +/*
> + * Write the data to the device memory (mapped either through ioremap
> + * or memremap) from the user buffer.
> + */
> +static int
> +nvgrace_gpu_map_and_write(struct nvgrace_gpu_vfio_pci_core_device *nvdev,
> + const char __user *buf, size_t mem_count,
> + loff_t *ppos)
> +{
> + unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos);
> + loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK;
> + int ret;
> +
> + ret = nvgrace_gpu_map_device_mem(index, nvdev);
> + if (ret)
> + return ret;
> +
> + if (index == USEMEM_REGION_INDEX) {
> + if (copy_from_user((u8 *)nvdev->usemem.memaddr + pos,
> + buf, mem_count))
> + return -EFAULT;
> + } else {
> + /*
> + * The hardware ensures that the system does not crash when
> + * the device memory is accessed with the memory enable
> + * turned off. It drops such writes. So there is no need to
> + * check or support the disablement/enablement of BAR
> + * through PCI_COMMAND config space register. Pass test_mem
> + * flag as false.
> + */
> + ret = vfio_pci_core_do_io_rw(&nvdev->core_device, false,
> + nvdev->resmem.ioaddr,
> + (char __user *)buf, pos, mem_count,
> + 0, 0, true);
> + }
> +
> + return ret;
> +}
> +
> +/*
> + * 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.
> + *
> + * Writes extending beyond the reported size are truncated; writes starting
> + * beyond the reported size generate -EINVAL.
> + */
> +static ssize_t
> +nvgrace_gpu_write_mem(struct nvgrace_gpu_vfio_pci_core_device *nvdev,
> + size_t count, loff_t *ppos, const char __user *buf)
> +{
> + u64 offset = *ppos & VFIO_PCI_OFFSET_MASK;
> + unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos);
> + struct mem_region *memregion;
> + size_t mem_count;
> + int ret = 0;
> +
> + memregion = nvgrace_gpu_memregion(index, nvdev);
> + if (!memregion)
> + return -EINVAL;
> +
> + if (offset >= memregion->bar_size)
> + return -EINVAL;
> +
> + /* Clip short the write request beyond reported BAR size */
> + count = min(count, memregion->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 >= memregion->memlength)
> + goto exitfn;
> +
> + /*
> + * Only the device memory present on the hardware is mapped, which may
> + * not be power-of-2 aligned. Drop access outside the available device
> + * memory on the hardware.
> + */
> + mem_count = min(count, memregion->memlength - (size_t)offset);
> +
> + ret = nvgrace_gpu_map_and_write(nvdev, buf, mem_count, ppos);
> + if (ret)
> + return ret;
> +
> +exitfn:
> + *ppos += count;
> + return count;
> +}
> +
> +static ssize_t
> +nvgrace_gpu_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 (nvgrace_gpu_memregion(index, nvdev))
> + return nvgrace_gpu_write_mem(nvdev, count, ppos, buf);
> +
> + if (index == VFIO_PCI_CONFIG_REGION_INDEX)
> + return nvgrace_gpu_write_config_emu(core_vdev, buf, count, ppos);
> +
> + 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_open_device,
> + .close_device = nvgrace_gpu_close_device,
> + .ioctl = nvgrace_gpu_ioctl,
> + .read = nvgrace_gpu_read,
> + .write = nvgrace_gpu_write,
> + .mmap = nvgrace_gpu_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,
> + .detach_ioas = vfio_iommufd_physical_detach_ioas,
> +};
> +
> +static const struct vfio_device_ops nvgrace_gpu_vfio_pci_core_ops = {
> + .name = "nvgrace-gpu-vfio-pci-core",
> + .init = vfio_pci_core_init_dev,
> + .release = vfio_pci_core_release_dev,
> + .open_device = nvgrace_gpu_open_device,
> + .close_device = vfio_pci_core_close_device,
> + .ioctl = vfio_pci_core_ioctl,
> + .device_feature = vfio_pci_core_ioctl_feature,
> + .read = vfio_pci_core_read,
> + .write = vfio_pci_core_write,
> + .mmap = vfio_pci_core_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,
> + .detach_ioas = vfio_iommufd_physical_detach_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_fetch_memory_property(struct pci_dev *pdev,
> + u64 *pmemphys, u64 *pmemlength)
> +{
> + int ret;
> +
> + /*
> + * 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",
> + pmemphys);
> + if (ret)
> + return ret;
> +
> + if (*pmemphys > type_max(phys_addr_t))
> + return -EOVERFLOW;
> +
> + ret = device_property_read_u64(&pdev->dev, "nvidia,gpu-mem-size",
> + pmemlength);
> + if (ret)
> + return ret;
> +
> + if (*pmemlength > 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 (*pmemlength == 0)
> + return -ENOMEM;
> +
> + return ret;
> +}
> +
> +static int
> +nvgrace_gpu_init_nvdev_struct(struct pci_dev *pdev,
> + struct nvgrace_gpu_vfio_pci_core_device *nvdev,
> + u64 memphys, u64 memlength)
> +{
> + int ret = 0;
> +
> + /*
> + * The VM GPU device driver needs a non-cacheable region to support
> + * the MIG feature. Since the device memory is mapped as NORMAL cached,
> + * carve out a region from the end with a different NORMAL_NC
> + * property (called as reserved memory and represented as resmem). This
> + * region then is exposed as a 64b BAR (region 2 and 3) to the VM, while
> + * exposing the rest (termed as usable memory and represented using usemem)
> + * as cacheable 64b BAR (region 4 and 5).
> + *
> + * devmem (memlength)
> + * |-------------------------------------------------|
> + * | |
> + * usemem.phys/memphys resmem.phys
> + */
> + nvdev->usemem.memphys = memphys;
> +
> + /*
> + * The device memory exposed to the VM is added to the kernel by the
> + * VM driver module in chunks of memory block size. Only the usable
> + * memory (usemem) is added to the kernel for usage by the VM
> + * workloads. Make the usable memory size memblock aligned.
> + */
> + if (check_sub_overflow(memlength, RESMEM_SIZE,
> + &nvdev->usemem.memlength)) {
> + ret = -EOVERFLOW;
> + goto done;
> + }
> + nvdev->usemem.memlength = round_down(nvdev->usemem.memlength,
> + MEMBLK_SIZE);
> + if ((check_add_overflow(nvdev->usemem.memphys,
> + nvdev->usemem.memlength,
> + &nvdev->resmem.memphys)) ||
> + (check_sub_overflow(memlength, nvdev->usemem.memlength,
> + &nvdev->resmem.memlength))) {
> + ret = -EOVERFLOW;
> + goto done;
> + }
> +
> + /*
> + * The memory regions are exposed as BARs. Calculate and save
> + * the BAR size for them.
> + */
> + nvdev->usemem.bar_size = roundup_pow_of_two(nvdev->usemem.memlength);
> + nvdev->resmem.bar_size = roundup_pow_of_two(nvdev->resmem.memlength);
> +done:
> + return ret;
> +}
> +
> +static int nvgrace_gpu_probe(struct pci_dev *pdev,
> + const struct pci_device_id *id)
> +{
> + const struct vfio_device_ops *ops = &nvgrace_gpu_vfio_pci_core_ops;
> + struct nvgrace_gpu_vfio_pci_core_device *nvdev;
> + u64 memphys, memlength;
> + int ret;
> +
> + ret = nvgrace_gpu_fetch_memory_property(pdev, &memphys, &memlength);
> + if (!ret)
> + ops = &nvgrace_gpu_vfio_pci_ops;
> +
> + nvdev = vfio_alloc_device(nvgrace_gpu_vfio_pci_core_device, core_device.vdev,
> + &pdev->dev, ops);
> + if (IS_ERR(nvdev))
> + return PTR_ERR(nvdev);
> +
> + dev_set_drvdata(&pdev->dev, &nvdev->core_device);
> +
> + if (ops == &nvgrace_gpu_vfio_pci_ops) {
> + /*
> + * Device memory properties are identified in the host ACPI
> + * table. Set the nvgrace_gpu_vfio_pci_core_device structure.
> + */
> + ret = nvgrace_gpu_init_nvdev_struct(pdev, nvdev,
> + memphys, memlength);
> + 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_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_probe,
> + .remove = nvgrace_gpu_remove,
> + .err_handler = &vfio_pci_core_err_handlers,
> + .driver_managed_dma = true,
> +};
> +
> +module_pci_driver(nvgrace_gpu_vfio_pci_driver);
> +
> +MODULE_LICENSE("GPL");
> +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");

2024-02-08 06:14:52

[permalink] [raw]

Subject: RE: [PATCH v17 1/3] vfio/pci: rename and export do_io_rw()

> From: [email protected] <[email protected]>
> Sent: Tuesday, February 6, 2024 7:01 AM
>
> From: Ankit Agrawal <[email protected]>
>
> do_io_rw() is used to read/write to the device MMIO. The grace hopper
> VFIO PCI variant driver require this functionality to read/write to
> its memory.
>
> Rename this as vfio_pci_core functions and export as GPL.
>
> Signed-off-by: Ankit Agrawal <[email protected]>

Reviewed-by: Kevin Tian <[email protected]>

2024-02-08 06:19:36

[permalink] [raw]

Subject: RE: [PATCH v17 2/3] vfio/pci: rename and export range_intesect_range

> From: [email protected] <[email protected]>
> Sent: Tuesday, February 6, 2024 7:01 AM
>
> From: Ankit Agrawal <[email protected]>
>
> range_intesect_range determines an overlap between two ranges. If an

s/intesect/intersect/

> overlap, the helper function returns the overlapping offset and size.
>
> The VFIO PCI variant driver emulates the PCI config space BAR offset
> registers. These offset may be accessed for read/write with a variety
> of lengths including sub-word sizes from sub-word offsets. The driver
> makes use of this helper function to read/write the targeted part of
> the emulated register.
>
> Make this a vfio_pci_core function, rename and export as GPL. Also
> update references in virtio driver.
>
> Signed-off-by: Ankit Agrawal <[email protected]>
> ---
> drivers/vfio/pci/vfio_pci_config.c | 45 +++++++++++++++++++
> drivers/vfio/pci/virtio/main.c | 72 +++++++++++-------------------
> include/linux/vfio_pci_core.h | 5 +++
> 3 files changed, 76 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/vfio/pci/vfio_pci_config.c
> b/drivers/vfio/pci/vfio_pci_config.c
> index 672a1804af6a..4fc3c605af13 100644
> --- a/drivers/vfio/pci/vfio_pci_config.c
> +++ b/drivers/vfio/pci/vfio_pci_config.c
> @@ -1966,3 +1966,48 @@ ssize_t vfio_pci_config_rw(struct
> vfio_pci_core_device *vdev, char __user *buf,
>
> return done;
> }
> +
> +/**
> + * vfio_pci_core_range_intersect_range() - Determine overlap between a
> buffer
> + * and register offset ranges.
> + * @range1_start: start offset of the buffer
> + * @count1: number of buffer bytes.
> + * @range2_start: start register offset
> + * @count2: number of bytes of register
> + * @start_offset: start offset of overlap start in the buffer
> + * @intersect_count: number of overlapping bytes
> + * @register_offset: start offset of overlap start in register
> + *
> + * The function determines an overlap between a register and a buffer.
> + * range1 represents the buffer and range2 represents register.
> + *
> + * Returns: true if there is overlap, false if not.
> + * The overlap start and range is returned through function args.
> + */
> +bool vfio_pci_core_range_intersect_range(loff_t range1_start, size_t count1,
> + loff_t range2_start, size_t count2,
> + loff_t *start_offset,
> + size_t *intersect_count,
> + size_t *register_offset)

based on description it's probably clearer to rename:

range1_start -> buf_start
count1 -> buf_cnt
range2_start -> reg_start
count2 -> reg_cnt
start_offset -> buf_offset

but not big deal, so:

Reviewed-by: Kevin Tian <[email protected]>

2024-02-08 06:43:18

[permalink] [raw]

Subject: Re: [PATCH v17 2/3] vfio/pci: rename and export range_intesect_range

Thanks Kevin.

>> range_intesect_range determines an overlap between two ranges. If an
>
> s/intesect/intersect/

Will fix the typo.

>> + * vfio_pci_core_range_intersect_range() - Determine overlap between a
>> buffer
>> + *????????????????????????????????????? and register offset ranges.
>> + * @range1_start:??? start offset of the buffer
>> + * @count1:?????? number of buffer bytes.
>> + * @range2_start:??? start register offset
>> + * @count2:?????? number of bytes of register
>> + * @start_offset:??? start offset of overlap start in the buffer
>> + * @intersect_count: number of overlapping bytes
>> + * @register_offset: start offset of overlap start in register
>> + *
>> + * The function determines an overlap between a register and a buffer.
>> + * range1 represents the buffer and range2 represents register.
>> + *
>> + * Returns: true if there is overlap, false if not.
>> + * The overlap start and range is returned through function args.
>> + */
>> +bool vfio_pci_core_range_intersect_range(loff_t range1_start, size_t count1,
>> +????????????????????????????????????? loff_t range2_start, size_t count2,
>> +????????????????????????????????????? loff_t *start_offset,
>> +????????????????????????????????????? size_t *intersect_count,
>> +????????????????????????????????????? size_t *register_offset)
>
> based on description it's probably clearer to rename:
>
> range1_start -> buf_start
> count1 -> buf_cnt
> range2_start -> reg_start
> count2 -> reg_cnt
> start_offset -> buf_offset
>
> but not big deal, so:

Fine by me. Will rename them.

> Reviewed-by: Kevin Tian <[email protected]>

Thanks!

2024-02-08 06:48:14

[permalink] [raw]

Subject: Re: [PATCH v17 3/3] vfio/nvgrace-gpu: Add vfio pci variant module for grace hopper

>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 8999497011a2..529ec8966f58 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -23103,6 +23103,12 @@ L:?? [email protected]
>>? S:?? Maintained
>>? F:?? drivers/vfio/platform/
>>
>> +VFIO NVIDIA GRACE GPU DRIVER
>> +M:?? Ankit Agrawal <[email protected]>
>> +L:?? [email protected]
>> +S:?? Supported
>> +F:?? drivers/vfio/pci/nvgrace-gpu/
>> +
>
> Entries should be alphabetical.? This will end up colliding with [1] so
> I'll plan to fix it either way.

I will make the change to put it at the right place.

> Otherwise just a couple optional comments from me below.? I see Zhi also
> has a few good comments.? I'd suggest soliciting a review from the other
> variant driver reviewers for this version and maybe we can make v18 the
> final version.? Thanks,
>
> Alex
>
> [1]https://lore.kernel.org/all/[email protected]/

Great!

>> +static ssize_t
>> +nvgrace_gpu_write_config_emu(struct vfio_device *core_vdev,
>> +????????????????????????? const char __user *buf, size_t count, loff_t *ppos)
>> +{
>> +???? struct nvgrace_gpu_vfio_pci_core_device *nvdev =
>> +???????????? container_of(core_vdev, struct nvgrace_gpu_vfio_pci_core_device,
>> +????????????????????????? core_device.vdev);
>> +???? u64 pos = *ppos & VFIO_PCI_OFFSET_MASK;
>> +???? size_t register_offset;
>> +???? loff_t copy_offset;
>> +???? size_t copy_count;
>> +???? struct mem_region *memregion = NULL;
>
> Nit, consistency and reverse Christmas tree variable declaration would
> suggest pushing this up in the list, but it's not strictly required.

Ack, I'll make the change.

>> +???? if (index == USEMEM_REGION_INDEX && !memregion->memaddr) {
>> +???????????? memregion->memaddr = memremap(memregion->memphys,
>> +?????????????????????????????????????????? memregion->memlength,
>> +?????????????????????????????????????????? MEMREMAP_WB);
>> +???????????? if (!memregion->memaddr)
>> +???????????????????? ret = -ENOMEM;
>> +???? } else if (index == RESMEM_REGION_INDEX && !memregion->ioaddr) {
>> +???????????? memregion->ioaddr = ioremap_wc(memregion->memphys,
>> +??????????????????????????????????????????? memregion->memlength);
>> +???????????? if (!memregion->ioaddr)
>> +???????????????????? ret = -ENOMEM;
>> +???? }
>
> As .memaddr and .ioaddr are a union we can consolidate the NULL test.

Ok, will do that.

2024-02-08 07:13:22

[permalink] [raw]

Subject: Re: [PATCH v17 3/3] vfio/nvgrace-gpu: Add vfio pci variant module for grace hopper

>> > +/* Memory size expected as non cached and reserved by the VM driver
>> > */ +#define RESMEM_SIZE 0x40000000
>> > +#define MEMBLK_SIZE 0x20000000
>> > +
>>
>> Maybe use SZ_* definitions in linux/size.h
>
> Good suggestion.

Ack.

>>
>> Better move this part to the place between vfio_pci_core_enable() and
>> vfio_pci_core_finish_enable() like others for respecting the expected
>> device initialization sequence of life cycle.
>>
>> It doesn't bite something right now, but think about when someone
>> changes the behavior of vfio_pci_core_finish_enable() in the future,
>> they have to propose a patch for this.
>
> Agree.

Good point, will move it.

>>
>> Wouldn't it be better to do the map in the open path?
>
> AIUI the device would typically be used exclusively through the mmap of
> these ranges, these mappings are only for pread/pwrite type accesses,
> so I think it makes sense to map them on demand.

That's right, agree with Alex to keep it on-demand.

>> > +??? * 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 >= memregion->memlength)
>> > +?????????? mem_count = 0;
>>
>> If mem_count == 0, going through nvgrace_gpu_map_and_read() is not
>> necessary.
>
> Harmless, other than the possibly unnecessary call through to
> nvgrace_gpu_map_device_mem().? Maybe both nvgrace_gpu_map_and_read()
> and nvgrace_gpu_map_and_write() could conditionally return 0 as their
> first operation when !mem_count.? Thanks,
>
>Alex

IMO, this seems like adding too much code to reduce the call length for a
very specific case. If there aren't any strong opinion on this, I'm planning to
leave this code as it is.

2024-02-08 07:18:25

[permalink] [raw]

Subject: RE: [PATCH v17 3/3] vfio/nvgrace-gpu: Add vfio pci variant module for grace hopper

> From: [email protected] <[email protected]>
> Sent: Tuesday, February 6, 2024 7:01 AM
>
> Note that the usemem memory is added by the VM Nvidia device driver [5]
> to the VM kernel as memblocks. Hence make the usable memory size
> memblock
> aligned.

Is memblock size defined in spec or purely a guest implementation choice?

>
> If the bare metal properties are not present, the driver registers the
> vfio-pci-core function pointers.

so if qemu doesn't generate such property the variant driver running
inside guest will always go to use core functions and guest vfio userspace
will observe both resmem and usemem bars. But then there is nothing
in field to prohibit mapping resmem bar as cacheable.

should this driver check the presence of either ACPI property or
resmem/usemem bars to enable variant function pointers?

> +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.

"assign the GPU device to userspace"

> +
> +/* Memory size expected as non cached and reserved by the VM driver */
> +#define RESMEM_SIZE 0x40000000
> +#define MEMBLK_SIZE 0x20000000

also add a comment for MEMBLK_SIZE

> +
> +struct nvgrace_gpu_vfio_pci_core_device {

will nvgrace refer to a non-gpu device? if not probably all prefixes with
'nvgrace_gpu' can be simplified to 'nvgrace'.

btw following other variant drivers 'vfio' can be removed too.

> +
> +/*
> + * Both the usable (usemem) and the reserved (resmem) device memory
> region
> + * are exposed as a 64b fake BARs in the VM. These fake BARs must respond

s/VM/device/

> + * to the accesses on their respective PCI config space offsets.
> + *
> + * resmem BAR owns PCI_BASE_ADDRESS_2 & PCI_BASE_ADDRESS_3.
> + * usemem BAR owns PCI_BASE_ADDRESS_4 & PCI_BASE_ADDRESS_5.
> + */
> +static ssize_t
> +nvgrace_gpu_read_config_emu(struct vfio_device *core_vdev,
> + char __user *buf, size_t count, loff_t *ppos)
> +{
> + struct nvgrace_gpu_vfio_pci_core_device *nvdev =
> + container_of(core_vdev, struct
> nvgrace_gpu_vfio_pci_core_device,
> + core_device.vdev);
> + struct mem_region *memregion = NULL;
> + u64 pos = *ppos & VFIO_PCI_OFFSET_MASK;
> + __le64 val64;
> + size_t register_offset;
> + loff_t copy_offset;
> + size_t copy_count;
> + int ret;
> +
> + ret = vfio_pci_core_read(core_vdev, buf, count, ppos);
> + if (ret < 0)
> + return ret;

here if core_read succeeds *ppos has been updated...

> +
> + if (vfio_pci_core_range_intersect_range(pos, count,
> PCI_BASE_ADDRESS_2,
> + sizeof(val64),
> + &copy_offset, &copy_count,
> + &register_offset))
> + memregion =
> nvgrace_gpu_memregion(RESMEM_REGION_INDEX, nvdev);
> + else if (vfio_pci_core_range_intersect_range(pos, count,
> + PCI_BASE_ADDRESS_4,
> + sizeof(val64),
> + &copy_offset,
> &copy_count,
> + &register_offset))
> + memregion =
> nvgrace_gpu_memregion(USEMEM_REGION_INDEX, nvdev);
> +
> + if (memregion) {
> + val64 = nvgrace_gpu_get_read_value(memregion->bar_size,
> +
> PCI_BASE_ADDRESS_MEM_TYPE_64 |
> +
> PCI_BASE_ADDRESS_MEM_PREFETCH,
> + memregion->bar_val);
> + if (copy_to_user(buf + copy_offset,
> + (void *)&val64 + register_offset, copy_count))
> + return -EFAULT;

..but here it's not adjusted back upon error.

> +
> +/*
> + * Read the data from the device memory (mapped either through ioremap
> + * or memremap) into the user buffer.
> + */
> +static int
> +nvgrace_gpu_map_and_read(struct nvgrace_gpu_vfio_pci_core_device
> *nvdev,
> + char __user *buf, size_t mem_count, loff_t *ppos)
> +{
> + unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos);
> + u64 offset = *ppos & VFIO_PCI_OFFSET_MASK;
> + int ret;
> +
> + /*
> + * Handle read on the BAR regions. Map to the target device memory
> + * physical address and copy to the request read buffer.
> + */

duplicate with the earlier comment for the function.

> +/*
> + * 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.
> + *
> + * Reads extending beyond the reported size are truncated; reads starting
> + * beyond the reported size generate -EINVAL; reads extending beyond the
> + * actual device size is filled with ~0.

slightly clearer to order the description: read starting beyond reported
size, then read extending beyond device size, and read extending beyond
reported size.

> +static int
> +nvgrace_gpu_init_nvdev_struct(struct pci_dev *pdev,
> + struct nvgrace_gpu_vfio_pci_core_device *nvdev,
> + u64 memphys, u64 memlength)
> +{
> + int ret = 0;
> +
> + /*
> + * The VM GPU device driver needs a non-cacheable region to
> support
> + * the MIG feature. Since the device memory is mapped as NORMAL
> cached,
> + * carve out a region from the end with a different NORMAL_NC
> + * property (called as reserved memory and represented as resmem).
> This
> + * region then is exposed as a 64b BAR (region 2 and 3) to the VM,
> while
> + * exposing the rest (termed as usable memory and represented
> using usemem)
> + * as cacheable 64b BAR (region 4 and 5).
> + *
> + * devmem (memlength)
> + * |-------------------------------------------------|
> + * | |
> + * usemem.phys/memphys resmem.phys

there is no usemem.phys and resmem.phys

> + */
> + nvdev->usemem.memphys = memphys;
> +
> + /*
> + * The device memory exposed to the VM is added to the kernel by
> the
> + * VM driver module in chunks of memory block size. Only the usable
> + * memory (usemem) is added to the kernel for usage by the VM
> + * workloads. Make the usable memory size memblock aligned.
> + */

If memblock size is defined by hw spec then say so.

otherwise this sounds a broken contract if it's a guest-decided value.

> + if (check_sub_overflow(memlength, RESMEM_SIZE,
> + &nvdev->usemem.memlength)) {
> + ret = -EOVERFLOW;
> + goto done;
> + }

does resmem require 1G-aligned?

if usemem.memlength becomes 0 then should return error too.

2024-02-08 07:21:58

[permalink] [raw]

Subject: RE: [PATCH v17 3/3] vfio/nvgrace-gpu: Add vfio pci variant module for grace hopper

> From: Ankit Agrawal <[email protected]>
> Sent: Thursday, February 8, 2024 3:13 PM
> >> > +??? * 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 >= memregion->memlength)
> >> > +?????????? mem_count = 0;
> >>
> >> If mem_count == 0, going through nvgrace_gpu_map_and_read() is not
> >> necessary.
> >
> > Harmless, other than the possibly unnecessary call through to
> > nvgrace_gpu_map_device_mem().? Maybe both
> nvgrace_gpu_map_and_read()
> > and nvgrace_gpu_map_and_write() could conditionally return 0 as their
> > first operation when !mem_count.? Thanks,
> >
> >Alex
>
> IMO, this seems like adding too much code to reduce the call length for a
> very specific case. If there aren't any strong opinion on this, I'm planning to
> leave this code as it is.

a slight difference. if mem_count==0 the result should always succeed
no matter nvgrace_gpu_map_device_mem() succeeds or not. Of course
if it fails it's already a big problem probably nobody cares about the subtle
difference when reading non-exist range.

but regarding to readability it's still clearer:

if (mem_count)
nvgrace_gpu_map_and_read();

2024-02-08 11:19:36

[permalink] [raw]

Subject: Re: [PATCH v17 3/3] vfio/nvgrace-gpu: Add vfio pci variant module for grace hopper

>> >>
>> >> If mem_count == 0, going through nvgrace_gpu_map_and_read() is not
>> >> necessary.
>> >
>> > Harmless, other than the possibly unnecessary call through to
>> > nvgrace_gpu_map_device_mem().? Maybe both
>> nvgrace_gpu_map_and_read()
>> > and nvgrace_gpu_map_and_write() could conditionally return 0 as their
>> > first operation when !mem_count.? Thanks,
>> >
>> >Alex
>>
>> IMO, this seems like adding too much code to reduce the call length for a
>> very specific case. If there aren't any strong opinion on this, I'm planning to
>> leave this code as it is.
>
> a slight difference. if mem_count==0 the result should always succeed
> no matter nvgrace_gpu_map_device_mem() succeeds or not. Of course
> if it fails it's already a big problem probably nobody cares about the subtle
> difference when reading non-exist range.
>
> but regarding to readability it's still clearer:
>
> if (mem_count)
>??????? nvgrace_gpu_map_and_read();

Makes sense. I'll change it.

2024-02-08 16:11:45

[permalink] [raw]

Subject: Re: [PATCH v17 3/3] vfio/nvgrace-gpu: Add vfio pci variant module for grace hopper

On Thu, 8 Feb 2024 07:21:40 +0000
"Tian, Kevin" <[email protected]> wrote:

> > From: Ankit Agrawal <[email protected]>
> > Sent: Thursday, February 8, 2024 3:13 PM
> > >> > +    * 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 >= memregion->memlength)
> > >> > +           mem_count = 0;
> > >>
> > >> If mem_count == 0, going through nvgrace_gpu_map_and_read() is not
> > >> necessary.
> > >
> > > Harmless, other than the possibly unnecessary call through to
> > > nvgrace_gpu_map_device_mem(). Maybe both
> > nvgrace_gpu_map_and_read()
> > > and nvgrace_gpu_map_and_write() could conditionally return 0 as their
> > > first operation when !mem_count. Thanks,
> > >
> > >Alex
> >
> > IMO, this seems like adding too much code to reduce the call length for a
> > very specific case. If there aren't any strong opinion on this, I'm planning to
> > leave this code as it is.
>
> a slight difference. if mem_count==0 the result should always succeed
> no matter nvgrace_gpu_map_device_mem() succeeds or not. Of course
> if it fails it's already a big problem probably nobody cares about the subtle
> difference when reading non-exist range.
>
> but regarding to readability it's still clearer:
>
> if (mem_count)
> nvgrace_gpu_map_and_read();
>

The below has better flow imo vs conditionalizing the call to
map_and_read/write and subsequent error handling, but I don't think
either adds too much code. Thanks,

Alex

--- a/drivers/vfio/pci/nvgrace-gpu/main.c
+++ b/drivers/vfio/pci/nvgrace-gpu/main.c
@@ -429,6 +429,9 @@ nvgrace_gpu_map_and_read(struct nvgrace_gpu_vfio_pci_core_device *nvdev,
u64 offset = *ppos & VFIO_PCI_OFFSET_MASK;
int ret;

+ if (!mem_count)
+ return 0;
+
/*
* Handle read on the BAR regions. Map to the target device memory
* physical address and copy to the request read buffer.
@@ -547,6 +550,9 @@ nvgrace_gpu_map_and_write(struct nvgrace_gpu_vfio_pci_core_device *nvdev,
loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK;
int ret;

+ if (!mem_count)
+ return 0;
+
ret = nvgrace_gpu_map_device_mem(index, nvdev);
if (ret)
return ret;

2024-02-09 09:24:59

[permalink] [raw]

Subject: Re: [PATCH v17 3/3] vfio/nvgrace-gpu: Add vfio pci variant module for grace hopper

>> >
>> > IMO, this seems like adding too much code to reduce the call length for a
>> > very specific case. If there aren't any strong opinion on this, I'm planning to
>> > leave this code as it is.
>>
>> a slight difference. if mem_count==0 the result should always succeed
>> no matter nvgrace_gpu_map_device_mem() succeeds or not. Of course
>> if it fails it's already a big problem probably nobody cares about the subtle
>> difference when reading non-exist range.
>>
>> but regarding to readability it's still clearer:
>>
>> if (mem_count)
>>?????? nvgrace_gpu_map_and_read();
>>
>
> The below has better flow imo vs conditionalizing the call to
> map_and_read/write and subsequent error handling, but I don't think
> either adds too much code.? Thanks,
>
> Alex
>
> --- a/drivers/vfio/pci/nvgrace-gpu/main.c
> +++ b/drivers/vfio/pci/nvgrace-gpu/main.c
> @@ -429,6 +429,9 @@ nvgrace_gpu_map_and_read(struct nvgrace_gpu_vfio_pci_core_device *nvdev,
>? ?????? u64 offset = *ppos & VFIO_PCI_OFFSET_MASK;
>?? ????? int ret;
>
> +?????? if (!mem_count)
> +?????????????? return 0;
> +
> ??????? /*
> ??????? * Handle read on the BAR regions. Map to the target device memory
> ???????? * physical address and copy to the request read buffer.
> @@ -547,6 +550,9 @@ nvgrace_gpu_map_and_write(struct nvgrace_gpu_vfio_pci_core_device *nvdev,
> ??????? loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK;
> ??????? int ret;
>
> +?????? if (!mem_count)
> +?????????????? return 0;
> +
> ??????? ret = nvgrace_gpu_map_device_mem(index, nvdev);
> ??????? if (ret)
>??????????????? return ret;

Sure, will update it as mentioned above.

2024-02-09 16:17:33

[permalink] [raw]

Subject: Re: [PATCH v17 3/3] vfio/nvgrace-gpu: Add vfio pci variant module for grace hopper

On Fri, 9 Feb 2024 09:20:22 +0000
Ankit Agrawal <[email protected]> wrote:

> Thanks Kevin for the review. Comments inline.
>
> >>
> >> Note that the usemem memory is added by the VM Nvidia device driver [5]
> >> to the VM kernel as memblocks. Hence make the usable memory size
> >> memblock
> >> aligned.
> >
> > Is memblock size defined in spec or purely a guest implementation choice?
>
> The MEMBLOCK value is a hardwired and a constant ABI value between the GPU
> FW and VFIO driver.
>
> >>
> >> If the bare metal properties are not present, the driver registers the
> >> vfio-pci-core function pointers.
> >
> > so if qemu doesn't generate such property the variant driver running
> > inside guest will always go to use core functions and guest vfio userspace
> > will observe both resmem and usemem bars. But then there is nothing
> > in field to prohibit mapping resmem bar as cacheable.
> >
> > should this driver check the presence of either ACPI property or
> > resmem/usemem bars to enable variant function pointers?
>
> Maybe I am missing something here; but if the ACPI property is absent,
> the real physical BARs present on the device will be exposed by the
> vfio-pci-core functions to the VM. So I think if the variant driver is ran
> within the VM, it should not see the fake usemem and resmem BARs.

There are two possibilities here, either we're assigning the pure
physical device from a host that does not have the ACPI properties or
we're performing a nested assignment. In the former case we're simply
passing along the unmodified physical BARs. In the latter case we're
actually passing through the fake BARs, the virtualization of the
device has already happened in the level 1 assignment.

I think Kevin's point is also relative to this latter scenario, in the
L1 instance of the nvgrace-gpu driver the mmap of the usemem BAR is
cachable, but in the L2 instance of the driver where we only use the
vfio-pci-core ops nothing maintains that cachable mapping. Is that a
problem? An uncached mapping on top of a cachable mapping is often
prone to problems. Thanks,

Alex

2024-02-09 17:19:23

by Jason Gunthorpe

[permalink] [raw]

Subject: Re: [PATCH v17 3/3] vfio/nvgrace-gpu: Add vfio pci variant module for grace hopper

On Fri, Feb 09, 2024 at 08:55:31AM -0700, Alex Williamson wrote:
> I think Kevin's point is also relative to this latter scenario, in the
> L1 instance of the nvgrace-gpu driver the mmap of the usemem BAR is
> cachable, but in the L2 instance of the driver where we only use the
> vfio-pci-core ops nothing maintains that cachable mapping. Is that a
> problem? An uncached mapping on top of a cachable mapping is often
> prone to problems.

On these CPUs the ARM architecture won't permit it, the L0 level
blocks uncachable using FWB and page table attributes. The VM, no
matter what it does, cannot make the cachable memory uncachable.

Jason

2024-02-09 18:01:04