Re: [PATCH v3] drm/amdgpu: Fix NPA-REVOKE racing an in-flight UALink import

[Mukul Joshi <mukul.joshi@amd.com>]

On 9/29/2026 3:34 PM, David Carlier wrote:
> The exporter records an importer when it answers NPA-REQ, so it can send
> NPA-REVOKE as soon as the BO is freed, before the importer has finished
> building the dma-buf for that handle. The revoke handler assumes a fully
> imported node: it dereferences imp_xa_node->dmabuf, which is still NULL
> until the import completes, and drops the xarray reference the importing
> thread still relies on. The importer then links the node and marks it
> READY regardless, so the node can be freed while still on the per-remote
> list.
>
> Only tear down a node that is READY. A PENDING node is still being
> imported and nothing has been handed to user-space yet, so only mark it
> for teardown and send NPA-RELEASE. The importer checks for teardown under
> the xarray lock before linking the node and marking it READY, and unwinds
> otherwise. As NPA-REVOKE always follows NPA-RSP, a revoke that finds the
> node NOT_READY is stale, and one that finds it in teardown hits a node
> that is already being released, so both are ignored.
>
> Fixes: 7cc82cd90d35 ("drm/amdgpu: Implement mechanism to revoke exported memory")
> Assisted-by: LLM
> Signed-off-by: David Carlier <devnexen@gmail.com>

Reviewed-by: Mukul Joshi <mukul.joshi@amd.com>

I will get this tested first internally before pushing this.

Thanks,

Mukul

> ---
> Changes in v3:
> - Handle the node state with a switch: tear down READY nodes, mark
>   PENDING ones for teardown, ignore the rest (Mukul).
> - Drop the npa_done completion on a NOT_READY node: the exporter only
>   records the importer after sending NPA-RSP, so such a revoke is stale
>   (Mukul).
> - No longer send NPA-RELEASE for a node already in teardown (Mukul).
> - Use dev_dbg() for the revoked-during-import message (Mukul).
>
> Changes in v2:
> - Tear down only READY nodes, so a duplicate NPA-REVOKE for a node already
>   in teardown neither dereferences a NULL dmabuf nor drops the node
>   reference twice (Sashiko).
> - Complete npa_done when a revoke arrives before NPA-RSP, so the importer
>   fails right away instead of timing out into a connection reset (Sashiko).
> - Use the current Assisted-by format.
>
> Found by code analysis and compile-tested with W=1. Not tested on hardware,
> as it needs two UALink-connected accelerators in a vPod.
>
> v2: https://lore.kernel.org/all/20260926174406.346253-1-devnexen@gmail.com/
> v1: https://lore.kernel.org/all/20260926171625.288519-1-devnexen@gmail.com/
>
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c | 51 +++++++++++++++++-----
>  1 file changed, 39 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> index 8411ea17172f..ea18e7f2e0a3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> @@ -3288,16 +3288,35 @@ static void amdgpu_ualink_process_npa_revoke_msg(struct amdgpu_device *adev,
>  		return;
>  	}
>  
> -	WRITE_ONCE(imp_xa_node->node_state, AMDGPU_UALINK_NODE_TEARDOWN);
> -	list_del_init(&imp_xa_node->list);
> -	xa_unlock(&adev->ualink.imp_xa);
> +	switch (READ_ONCE(imp_xa_node->node_state)) {
> +	case AMDGPU_UALINK_NODE_READY:
> +		WRITE_ONCE(imp_xa_node->node_state, AMDGPU_UALINK_NODE_TEARDOWN);
> +		list_del_init(&imp_xa_node->list);
> +		xa_unlock(&adev->ualink.imp_xa);
>  
> -	/* Invalidate the GPUVM mappings */
> -	bo = gem_to_amdgpu_bo(imp_xa_node->dmabuf->priv);
> -	amdgpu_ualink_invalidate_import_mappings(bo);
> +		/* Invalidate the GPUVM mappings */
> +		bo = gem_to_amdgpu_bo(imp_xa_node->dmabuf->priv);
> +		amdgpu_ualink_invalidate_import_mappings(bo);
>  
> -	/* Drop the refcount for the node */
> -	amdgpu_ualink_imp_xa_entry_put(imp_xa_node);
> +		/* Drop the refcount for the node */
> +		amdgpu_ualink_imp_xa_entry_put(imp_xa_node);
> +		break;
> +	case AMDGPU_UALINK_NODE_PENDING:
> +		/* The import is still building the dma-buf and nothing has
> +		 * been handed to user-space yet. The importing thread sees
> +		 * the teardown state and unwinds.
> +		 */
> +		WRITE_ONCE(imp_xa_node->node_state, AMDGPU_UALINK_NODE_TEARDOWN);
> +		xa_unlock(&adev->ualink.imp_xa);
> +		break;
> +	default:
> +		/* NPA-REVOKE always follows NPA-RSP, so a NOT_READY node means
> +		 * a stale revoke, and a node in teardown is already being
> +		 * released by whoever moved it there.
> +		 */
> +		xa_unlock(&adev->ualink.imp_xa);
> +		return;
> +	}
>  
>  	r = amdgpu_ualink_send_npa_release_msg(adev, remote_acc_id, handle);
>  	if (r)
> @@ -3760,9 +3779,20 @@ static int amdgpu_ualink_do_import_handle(struct amdgpu_device *adev,
>  		return r;
>  	}
>  
> -	/* Add this node to the imported handles list for the remote GPU */
> +	/* Add this node to the imported handles list for the remote GPU,
> +	 * unless the exporter revoked the handle while the import was in
> +	 * flight. The dmabuf is released with the last node reference.
> +	 */
>  	xa_lock(&adev->ualink.imp_xa);
> +	if (READ_ONCE(imp_xa_node->node_state) == AMDGPU_UALINK_NODE_TEARDOWN) {
> +		xa_unlock(&adev->ualink.imp_xa);
> +		dev_dbg(adev->dev,
> +			"IMPORT: handle:%llx:%llx revoked during import\n",
> +			handle.handle_hi, handle.handle_lo);
> +		return -EINVAL;
> +	}
>  	list_add(&imp_xa_node->list, &adev->ualink.imp_handles_list[remote_acc_id]);
> +	WRITE_ONCE(imp_xa_node->node_state, AMDGPU_UALINK_NODE_READY);
>  	xa_unlock(&adev->ualink.imp_xa);
>  
>  	return 0;
> @@ -3938,9 +3968,6 @@ int amdgpu_ualink_import_handle(struct drm_device *dev,
>  					"IMPORT: XA import failed for handle:%llx:%llx\n",
>  					handle.handle_hi, handle.handle_lo);
>  			goto cleanup;
> -		} else {
> -			WRITE_ONCE(imp_xa_node->node_state,
> -				   AMDGPU_UALINK_NODE_READY);
>  		}
>  	}
>