Re: [PATCH v8 1/1] vfio/nvgrace-gpu: Add Blackwell-Next GPU readiness check via CXL DVSEC

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Read and write operations spuriously fail with -EAGAIN if they race with a device reset due to missing retry logic.
--

commit 2aea957cd945c5158a2106e110886e0b017eeb9a
Author: Ankit Agrawal <ankita@nvidia.com>

    vfio/nvgrace-gpu: Add Blackwell-Next GPU readiness check via CXL DVSEC

This commit introduces a CXL DVSEC-based readiness check for Blackwell-Next
GPUs. It adds lockless polling followed by an in-lock non-blocking check,
and updates the probe, fault, read, and write paths to use the new flow.

> diff --git a/drivers/vfio/pci/nvgrace-gpu/main.c b/drivers/vfio/pci/nvgrace-gpu/main.c
> index fa056b69f899a..4829ae29cff04 100644
> --- a/drivers/vfio/pci/nvgrace-gpu/main.c
> +++ b/drivers/vfio/pci/nvgrace-gpu/main.c
[ ... ]
> @@ -712,6 +825,12 @@ nvgrace_gpu_read_mem(struct nvgrace_gpu_pci_core_device *nvdev,
>  	else
>  		mem_count = min(count, memregion->memlength - (size_t)offset);
>  
> +	if (nvdev->cxl_dvsec && READ_ONCE(nvdev->reset_done)) {
> +		ret = nvgrace_gpu_wait_device_ready_cxl(nvdev);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	scoped_guard(rwsem_read, &vdev->memory_lock) {
>  		ret = nvgrace_gpu_check_device_ready(nvdev);
>  		if (ret)

[Severity: High]
If a reset races between the lockless wait and the in-lock check, could this
code return -EAGAIN to userspace?

The page fault handler was updated with a retry loop to handle the -EAGAIN
returned by nvgrace_gpu_check_device_ready() when a concurrent reset happens.
However, in nvgrace_gpu_read_mem(), if nvgrace_gpu_check_device_ready()
returns -EAGAIN, that error will be directly propagated to the caller. 

Returning a spurious -EAGAIN on a standard blocking file descriptor violates
POSIX semantics and may cause unexpected application failures.

Should nvgrace_gpu_read_mem() include the same retry loop around the wait
and lock acquisition as the fault handler?

>  			return ret;
[ ... ]
> @@ -846,6 +965,12 @@ nvgrace_gpu_write_mem(struct nvgrace_gpu_pci_core_device *nvdev,
>  	 */
>  	mem_count = min(count, memregion->memlength - (size_t)offset);
>  
> +	if (nvdev->cxl_dvsec && READ_ONCE(nvdev->reset_done)) {
> +		ret = nvgrace_gpu_wait_device_ready_cxl(nvdev);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	scoped_guard(rwsem_read, &vdev->memory_lock) {
>  		ret = nvgrace_gpu_check_device_ready(nvdev);
>  		if (ret)

[Severity: High]
Does nvgrace_gpu_write_mem() have the same missing retry logic?

Similar to the read path, a racing reset could cause this to return a
spurious -EAGAIN to userspace.

>  			return ret;

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