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

[Alex Williamson <alex.williamson@nvidia.com>]

On Fri, 10 Apr 2026 22:19:59 +0000
David Matlack <dmatlack@google.com> wrote:
> On 2026-04-08 04:54 PM, Alex Williamson wrote:
> > From: Rubin Du <rubind@nvidia.com>
> > +static int fsp_init(struct vfio_pci_device *device)
> > +{
> > +	int ret;
> > +
> > +	ret = gpu_poll_register(device, "fsp_boot_complete",
> > +				NV_FSP_BOOT_COMPLETE_OFFSET,
> > +				NV_FSP_BOOT_COMPLETE_MASK, 0xffffffff, 5000);  
> 
> Sashiko is wondering if the mask should be something other than
> 0xffffffff:
> 
>   https://sashiko.dev/#/patchset/20260408225459.3088623-1-alex.williamson%40nvidia.com?part=4

I consulted the gpu-admin-tools and it performs the same test.  I think
the hangup here is that we're labeling the expected value field with a
macro named _MASK, when the mask is actually provided in the next
field.  I'll rename this to _SUCCESS for consistency with the reference
implementation.

> [...]
> > +	if (!falcon->no_outside_reset) {
> > +		ret = falcon_reset(device);
> > +		if (ret)
> > +			return ret;
> > +	}  
> 
> falcon_enable() is called right before nv_falcon_dma_init(). Calling
> falcon_reset() here will do falcon_disable() and then falcon_enable()
> again. So it seems unnecessary?

Yes, I think that initial falcon_enable() is unnecessary, and it's a
bit subtle, but a no_outside_reset device doesn't do anything for
enable or disable.  I think the solution is to pull that test to the
top of the enable function, at which point we can more clearly call
falcon_reset() unconditionally here and remove the earlier
falcon_enable() call.

I've incorporated all your other comments, aside from the optional
refactoring, thanks!  Testing to make sure the above change doesn't
regress on any devices will take some time, so I'll expect this to miss
the merge window and hopefully get queued for 7.2.  Thanks,

Alex