Re: [PATCH v10 2/2] selftests/vfio: Add NVIDIA Falcon driver for DMA testing

[David Matlack]

On 2026-03-25 02:43 PM, Rubin Du wrote:
> Add a new VFIO PCI driver for NVIDIA GPUs that enables DMA testing
> via the Falcon (Fast Logic Controller) microcontrollers. This driver
> extracts and adapts the DMA test functionality from NVIDIA's
> gpu-admin-tools project and integrates it into the existing VFIO
> selftest framework.
> 
> Falcons are general-purpose microcontrollers present on NVIDIA GPUs
> that can perform DMA operations between system memory and device
> memory. By leveraging Falcon DMA, this driver allows NVIDIA GPUs to
> be tested alongside Intel IOAT and DSA devices using the same
> selftest infrastructure.

> The driver is named 'nv_falcon' to reflect that it specifically
> controls the Falcon microcontrollers for DMA operations, rather
> than exposing general GPU functionality.

Do you forsee this driver ever expanding to utilize other functionality
on the GPU  other than the Falcon microcontroller, e.g. to add
send_msi() support in the future and/or support asynchronous memcpy?

If so, the name "nv_falcon" could get out of date and maybe "nv_gpu"
would be a better name for this driver?

> Reference implementation:
> https://github.com/NVIDIA/gpu-admin-tools
> 
> Signed-off-by: Rubin Du <rubind@xxxxxxxxxx>
> ---
>  .../vfio/lib/drivers/nv_falcons/hw.h          | 365 +++++++++
>  .../vfio/lib/drivers/nv_falcons/nv_falcons.c  | 739 ++++++++++++++++++

Please make the directory and file names match the driver. i.e.
s/nv_falcons/nv_falcon/.

> diff --git a/tools/testing/selftests/vfio/lib/drivers/nv_falcons/hw.h b/tools/testing/selftests/vfio/lib/drivers/nv_falcons/hw.h
> new file mode 100644
> index 000000000000..a92cdcfec63f
> --- /dev/null
> +++ b/tools/testing/selftests/vfio/lib/drivers/nv_falcons/hw.h
> @@ -0,0 +1,365 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES.  All rights reserved.
> + */
> +#ifndef _NV_FALCONS_HW_H_
> +#define _NV_FALCONS_HW_H_
> +
> +/* PMC (Power Management Controller) Registers */
> +#define NV_PMC_BOOT_0							0x00000000
> +#define NV_PMC_ENABLE							0x00000200
> +#define NV_PMC_ENABLE_PWR						0x00002000
> +#define NV_PMC_ENABLE_HUB						0x20000000

Please make the alignment of macro values consistent here and below.

> +
> +/* Falcon Base Pages for Different Engines */
> +#define NV_PPWR_FALCON_BASE						0x10a000
> +#define NV_PGSP_FALCON_BASE						0x110000
> +
> +/* Falcon Common Register Offsets (relative to base_page) */
> +#define NV_FALCON_DMACTL_OFFSET					0x010c
> +#define NV_FALCON_CPUCTL_OFFSET					0x0100
> +#define NV_FALCON_ENGINE_RESET_OFFSET			0x03c0
> +
> +/* DMEM Control Register Flags */
> +#define NV_PPWR_FALCON_DMEMC_AINCR_TRUE			0x01000000
> +#define NV_PPWR_FALCON_DMEMC_AINCW_TRUE			0x02000000
> +
> +/* Falcon DMEM port offsets (for port 0) */
> +#define NV_FALCON_DMEMC_OFFSET					0x1c0
> +#define NV_FALCON_DMEMD_OFFSET					0x1c4
> +
> +/* DMA Register Offsets (relative to base_page) */
> +#define NV_FALCON_DMA_ADDR_LOW_OFFSET			0x110
> +#define NV_FALCON_DMA_MEM_OFFSET				0x114
> +#define NV_FALCON_DMA_CMD_OFFSET				0x118
> +#define NV_FALCON_DMA_BLOCK_OFFSET				0x11c
> +#define NV_FALCON_DMA_ADDR_HIGH_OFFSET			0x128

> +struct gpu_device {
> +	enum gpu_arch arch;
> +	void *bar0;
> +	bool is_memory_clear_supported;
> +	const struct falcon *falcon;
> +	u32 pmc_enable_mask;
> +	bool fsp_dma_enabled;
> +
> +	/* Pending memcpy parameters, set by memcpy_start() */
> +	u64 memcpy_src;
> +	u64 memcpy_dst;
> +	u64 memcpy_size;
> +	u64 memcpy_count;
> +};

nit: I would move this to the driver .c file since that's where its used
and this isn't a hardware definition. It will make the driver easier to
read to have this struct definition in the same file.

> diff --git a/tools/testing/selftests/vfio/lib/drivers/nv_falcons/nv_falcons.c b/tools/testing/selftests/vfio/lib/drivers/nv_falcons/nv_falcons.c
> new file mode 100644
> index 000000000000..4b62793570a2
> --- /dev/null
> +++ b/tools/testing/selftests/vfio/lib/drivers/nv_falcons/nv_falcons.c
> @@ -0,0 +1,739 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES.  All rights reserved.
> + */
> +#include <stdint.h>
> +#include <strings.h>
> +#include <unistd.h>
> +#include <stdbool.h>
> +#include <string.h>
> +#include <time.h>
> +
> +#include <linux/errno.h>
> +#include <linux/io.h>
> +#include <linux/pci_ids.h>
> +
> +#include <libvfio.h>
> +
> +#include "hw.h"
> +
> +static inline struct gpu_device *to_nv_gpu(struct vfio_pci_device *device)

nit: Please align the naming here. i.e. rename this function to
to_gpu_device() or rename struct gpu_device to struct nv_gpu.

> +{
> +	return device->driver.region.vaddr;
> +}

> +static int gpu_poll_register(struct vfio_pci_device *device,
> +			     const char *name, u32 offset,
> +			     u32 expected, u32 mask, u32 timeout_ms)
> +{
> +	struct gpu_device *gpu = to_nv_gpu(device);
> +	u32 value;
> +	struct timespec start, now;
> +	u64 elapsed_ms;
> +
> +	clock_gettime(CLOCK_MONOTONIC, &start);
> +
> +	for (;;) {
> +		value = gpu_read32(gpu, offset);
> +		if ((value & mask) == expected)
> +			return 0;
> +
> +		clock_gettime(CLOCK_MONOTONIC, &now);
> +		elapsed_ms = (now.tv_sec - start.tv_sec) * 1000
> +			     + (now.tv_nsec - start.tv_nsec) / 1000000;
> +
> +		if (elapsed_ms >= timeout_ms)
> +			break;
> +
> +		usleep(1000);
> +	}
> +
> +	dev_err(device,
> +		"Timeout polling %s (0x%x): value=0x%x expected=0x%x mask=0x%x after %llu ms\n",
> +		name, offset, value, expected, mask,
> +		(unsigned long long)elapsed_ms);
> +	return -ETIMEDOUT;

Please fix all the callers of this function that ignore the return
value. Presumably you should be using VFIO_ASSERT_EQ(ret, 0) to bail the
test if something goes wrong (other than during memcpy_wait(), which has
a way to return an error to the caller).

> +static void gpu_enable_bus_master(struct vfio_pci_device *device)
> +{
> +	u16 cmd;
> +
> +	cmd = vfio_pci_config_readw(device, PCI_COMMAND);
> +	vfio_pci_config_writew(device, PCI_COMMAND, cmd | PCI_COMMAND_MASTER);
> +}
> +
> +static void gpu_disable_bus_master(struct vfio_pci_device *device)
> +{
> +	u16 cmd;
> +
> +	cmd = vfio_pci_config_readw(device, PCI_COMMAND);
> +	vfio_pci_config_writew(device, PCI_COMMAND, cmd & ~PCI_COMMAND_MASTER);
> +}

These can be generic to any PCI device so please add them as static
inline helpers in vfio_pci_device.h for other tests to use in the future
(and rename them to something more generic like
vfio_pci_disable_bus_master()).

> +static int nv_gpu_falcon_dma(struct vfio_pci_device *device,
> +			     u64 address,
> +			     u32 size_encoding,
> +			     bool write)
> +{
> +	struct gpu_device *gpu = to_nv_gpu(device);
> +	const struct falcon *falcon = gpu->falcon;
> +	u32 dma_cmd;
> +	int ret;
> +
> +	gpu_write32(gpu, NV_GPU_DMA_ADDR_TOP_BITS_REG,
> +		    (address >> 47) & 0x1ffff);
> +	gpu_write32(gpu, falcon->base_page + NV_FALCON_DMA_ADDR_HIGH_OFFSET,
> +		    (address >> 40) & 0x7f);
> +	gpu_write32(gpu, falcon->base_page + NV_FALCON_DMA_ADDR_LOW_OFFSET,
> +		    (address >> 8) & 0xffffffff);
> +	gpu_write32(gpu, falcon->base_page + NV_FALCON_DMA_BLOCK_OFFSET,
> +		    address & 0xff);
> +	gpu_write32(gpu, falcon->base_page + NV_FALCON_DMA_MEM_OFFSET, 0);
> +
> +	dma_cmd = (size_encoding << NV_FALCON_DMA_CMD_SIZE_SHIFT);
> +
> +	/* Set direction: write (DMEM->mem) or read (mem->DMEM) */
> +	if (write)
> +		dma_cmd |= NV_FALCON_DMA_CMD_WRITE_BIT;
> +
> +	gpu_write32(gpu, falcon->base_page + NV_FALCON_DMA_CMD_OFFSET, dma_cmd);
> +
> +	ret = gpu_poll_register(device, "dma_done",
> +		falcon->base_page + NV_FALCON_DMA_CMD_OFFSET,
> +		NV_FALCON_DMA_CMD_DONE_BIT, NV_FALCON_DMA_CMD_DONE_BIT,
> +		1000);
> +	if (ret)
> +		return ret;
> +
> +	return 0;


nit: This can just be "return ret;" or "return gpu_poll_register(...);".

> +static void nv_gpu_init(struct vfio_pci_device *device)
> +{
> +	struct gpu_device *gpu = to_nv_gpu(device);
> +	const struct gpu_properties *props;
> +	enum gpu_arch gpu_arch;
> +	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);
> +	if (gpu_arch == GPU_ARCH_UNKNOWN) {
> +		dev_err(device, "Unsupported GPU architecture\n");
> +		return;
> +	}
> +
> +	props = &gpu_properties_map[gpu_arch];
> +
> +	/* Populate GPU structure */
> +	gpu->arch = gpu_arch;
> +	gpu->bar0 = device->bars[0].vaddr;
> +	gpu->is_memory_clear_supported = props->memory_clear_supported;
> +	gpu->falcon = &falcon_map[props->falcon_type];
> +	gpu->pmc_enable_mask = props->pmc_enable_mask;
> +
> +	falcon_enable(device);
> +
> +	/* Initialize falcon for DMA */
> +	ret = nv_gpu_falcon_dma_init(device);
> +	VFIO_ASSERT_EQ(ret, 0, "Failed to initialize falcon DMA: %d\n", ret);
> +
> +	device->driver.features |= VFIO_PCI_DRIVER_F_NO_SEND_MSI;
> +	device->driver.max_memcpy_size = NV_FALCON_DMA_MAX_TRANSFER_SIZE;
> +	device->driver.max_memcpy_count = NV_FALCON_DMA_MAX_TRANSFER_COUNT;

The small memcpy size is going to be a problem tests that want to DMA to
larger regions. They will have to do chunk up the memcpy in a loop.

One option would be to have this driver expose a larger max_memcpy_size
and implement the loop in the memcpy_start()/wait() callback, which is
actually what you have below.

But I think a better approach would be to just make the loop common
across all drivers so that tests never have to care about
max_memcpy_size when using vfio_pci_driver_memcpy().

Can you add a precursor commit to this series that adds a loop to
vfio_pci_driver_memcpy() so that it performs whatever sized memcpy the
user asks for by chunking it up in to max_memcpy_size or smaller
chunks? In that same commit, please update the vfio_pci_driver_test
so we get coverage of the new chunking logic (get rid of the code that
rounds down self->size fo max_memcpy_size, except for the memcpy_storm
test).

In other words:

 - vfio_pci_driver_memcpy() should allow any size memcpy and will chunk
   it up as necessary depending on what the driver supports.

 - vfio_pci_driver_memcpy_start() semantics don't change. This still
   gives tests direct access to the driver memcpy_start() op and thus
   must obey max_memcpy_size.

This should also allow you to greatly simplify the implementation of
memcpy_wait() down below.

> +static int nv_gpu_memcpy_wait(struct vfio_pci_device *device)
> +{
> +	struct gpu_device *gpu = to_nv_gpu(device);
> +	u64 iteration;
> +	u64 offset;
> +	int ret;
> +
> +	VFIO_ASSERT_NE(gpu->memcpy_count, 0);
> +
> +	for (iteration = 0; iteration < gpu->memcpy_count; iteration++) {

This loop is unnecessary. You set max_memcpy_count to
NV_FALCON_DMA_MAX_TRANSFER_COUNT which is 1. So
vfio_pci_driver_memcpy_start() will guarantee that memcpy_count is at
most 1.

> +		offset = 0;
> +
> +		while (offset < gpu->memcpy_size) {
> +			int chunk_encoding;
> +
> +			chunk_encoding = size_to_dma_encoding(
> +						gpu->memcpy_size - offset);

This loop is also unnecessary. You set max_memcpy_size to
NV_FALCON_DMA_MAX_TRANSFER_SIZE, so vfio_pci_driver_memcpy_start() will
guarantee memcpy_size is at most that.

> +			if (chunk_encoding < 0) {
> +				ret = -EINVAL;
> +				goto out;
> +			}
> +
> +			ret = nv_gpu_memcpy_chunk(device,
> +						  gpu->memcpy_src + offset,
> +						  gpu->memcpy_dst + offset,
> +						  chunk_encoding);
> +			if (ret)
> +				goto out;
> +
> +			offset += 0x4 << chunk_encoding;
> +		}
> +	}
> +
> +	ret = 0;
> +out:
> +	gpu->memcpy_count = 0;
> +
> +	return ret;
> +}