Re: [PATCH v14 4/4] vfio: selftests: Add NVIDIA Falcon driver for DMA testing

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Double fetch of the NV_PMC_BOOT_0 register leads to unvalidated array index and out-of-bounds access.
- [Medium] Missing 4-byte alignment validation for source and destination addresses causes an assertion crash.
--

commit 50161cb43066f380e2c0477f9ea68885628a0a3b
Author: Rubin Du <rubind@nvidia.com>

vfio: selftests: Add NVIDIA Falcon driver for DMA testing

This patch adds a new VFIO PCI driver for NVIDIA GPUs to enable DMA
testing via Falcon microcontrollers within the existing VFIO selftest
framework.

> diff --git a/tools/testing/selftests/vfio/lib/drivers/nv_falcon/nv_falcon.c b/tools/testing/selftests/vfio/lib/drivers/nv_falcon/nv_falcon.c
> new file mode 100644
> index 0000000000000..c08aa81c44f41
> --- /dev/null
> +++ b/tools/testing/selftests/vfio/lib/drivers/nv_falcon/nv_falcon.c

[ ... ]

> +static void nv_falcon_init(struct vfio_pci_device *device)
> +{
> +	struct gpu_device *gpu = to_gpu_device(device);
> +	const struct gpu_properties *props;
> +	u32 pmc_boot_0;
> +	int ret;
> +
> +	VFIO_ASSERT_GE(device->driver.region.size, sizeof(*gpu));
> +
> +	/* Read PMC_BOOT_0 register from BAR0 to identify GPU */
> +	pmc_boot_0 = readl(device->bars[0].vaddr + NV_PMC_BOOT_0);
> +
> +	/* Look up GPU architecture */
> +	gpu->arch = nv_gpu_arch_lookup(pmc_boot_0);
> +
> +	props = &gpu_properties_map[gpu->arch];

[Severity: Medium]
Since the NV_PMC_BOOT_0 register is re-read from MMIO here rather than
reusing the validated value from nv_falcon_probe(), could a hardware state
change cause nv_gpu_arch_lookup() to return GPU_ARCH_UNKNOWN (-1)?

If so, this would use -1 as an out-of-bounds index into gpu_properties_map.
Would it be safer to store the validated architecture in the device struct
during probe, or validate the return value of nv_gpu_arch_lookup() here?

[ ... ]

> +static void nv_falcon_memcpy_start(struct vfio_pci_device *device,
> +				   iova_t src, iova_t dst, u64 size, u64 count)
> +{
> +	struct gpu_device *gpu = to_gpu_device(device);
> +
> +	VFIO_ASSERT_EQ(count, 1);
> +	VFIO_ASSERT_EQ(size & (NV_FALCON_DMA_MIN_TRANSFER_SIZE - 1), 0,
> +		       "size 0x%lx must be %u-byte aligned\n",
> +		       (unsigned long)size, NV_FALCON_DMA_MIN_TRANSFER_SIZE);
> +
> +	gpu->memcpy_src = src;
> +	gpu->memcpy_dst = dst;

[Severity: Medium]
Does this function also need to validate that the src and dst addresses
are 4-byte aligned?

If an unaligned address is passed (e.g., offset by 1 byte),
dma_block_remain() in nv_falcon_memcpy_wait() will yield a chunk size of 1.
This invalid size is then passed to nv_falcon_dma(), which triggers the
VFIO_ASSERT_GE(size, NV_FALCON_DMA_MIN_TRANSFER_SIZE) assertion in
size_to_dma_encoding() and crashes the test framework.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609232855.3808971-1-rubind@nvidia.com?part=4