On Tue, Aug 22, 2023 at 01:23:03PM -0700, ankita@xxxxxxxxxx wrote:
> @@ -0,0 +1,444 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved
> + */
> +
> +#include <linux/pci.h>
> +#include <linux/vfio_pci_core.h>
> +#include <linux/vfio.h>
> +
> +struct nvgrace_gpu_vfio_pci_core_device {
> + struct vfio_pci_core_device core_device;
> + u64 hpa;
> + u64 mem_length;
> + void *opregion;
> +};
opregion is some word that has meaning for the intel drivers, use
something else please.
'hpa' has no business being in a VFIO driver.
phys_addr_t memphys
size_t memlength
void __iomem *memmap;
> +static long nvgrace_gpu_vfio_pci_ioctl(struct vfio_device *core_vdev,
> + unsigned int cmd, unsigned long arg)
> +{
> + struct nvgrace_gpu_vfio_pci_core_device *nvdev = container_of(
> + core_vdev, struct nvgrace_gpu_vfio_pci_core_device, core_device.vdev);
> +
> + unsigned long minsz = offsetofend(struct vfio_region_info, offset);
> + struct vfio_region_info info;
> +
> + if (cmd == VFIO_DEVICE_GET_REGION_INFO) {
This block is long enough to put in its own function and remove a
level of indenting
> +/*
> + * Read count bytes from the device memory at an offset. The actual device
> + * memory size (available) may not be a power-of-2. So the driver fakes
> + * the size to a power-of-2 (reported) when exposing to a user space driver.
> + *
> + * Read request beyond the actual device size is filled with ~1, while
> + * those beyond the actual reported size is skipped.
> + *
> + * A read from a reported+ offset is considered error conditions and
> + * returned with an -EINVAL. Overflow conditions are also handled.
> + */
> +ssize_t nvgrace_gpu_read_mem(void __user *buf, size_t count, loff_t *ppos,
> + const void *addr, size_t available, size_t reported)
> +{
> + u64 offset = *ppos & VFIO_PCI_OFFSET_MASK;
> + u64 end;
> + size_t read_count, i;
> + u8 val = 0xFF;
> +
> + if (offset >= reported)
> + return -EINVAL;
> +
> + if (check_add_overflow(offset, count, &end))
> + return -EOVERFLOW;
> +
> + /* Clip short the read request beyond reported BAR size */
> + if (end >= reported)
> + count = reported - offset;
> +
> + /*
> + * Determine how many bytes to be actually read from the device memory.
> + * Do not read from the offset beyond available size.
> + */
> + if (offset >= available)
> + read_count = 0;
> + else
> + read_count = min(count, available - (size_t)offset);
> +
> + /*
> + * Handle read on the BAR2 region. Map to the target device memory
> + * physical address and copy to the request read buffer.
> + */
> + if (copy_to_user(buf, (u8 *)addr + offset, read_count))
> + return -EFAULT;
> +
> + /*
> + * Only the device memory present on the hardware is mapped, which may
> + * not be power-of-2 aligned. A read to the BAR2 region implies an
> + * access outside the available device memory on the hardware. Fill
> + * such read request with ~1.
> + */
> + for (i = 0; i < count - read_count; i++)
> + if (copy_to_user(buf + read_count + i, &val, 1))
> + return -EFAULT;
Use put_user()
> +static ssize_t nvgrace_gpu_vfio_pci_write(struct vfio_device *core_vdev,
> + const char __user *buf, size_t count, loff_t *ppos)
> +{
> + unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos);
> + struct nvgrace_gpu_vfio_pci_core_device *nvdev = container_of(
> + core_vdev, struct nvgrace_gpu_vfio_pci_core_device, core_device.vdev);
> +
> + if (index == VFIO_PCI_BAR2_REGION_INDEX) {
> + if (!nvdev->opregion) {
> + nvdev->opregion = memremap(nvdev->hpa, nvdev->mem_length, MEMREMAP_WB);
> + if (!nvdev->opregion)
> + return -ENOMEM;
> + }
Needs some kind of locking on opregion
> +static int
> +nvgrace_gpu_vfio_pci_fetch_memory_property(struct pci_dev *pdev,
> + struct nvgrace_gpu_vfio_pci_core_device *nvdev)
> +{
> + 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",
> + &(nvdev->hpa));
> + if (ret)
> + return ret;
Technically you need to check that the read_u64 doesn't overflow
phys_addr_t
> + ret = device_property_read_u64(&pdev->dev, "nvidia,gpu-mem-size",
> + &(nvdev->mem_length));
> + return ret;
And that mem_length doesn't overflow size_t
Jason
