[PATCH 2/3] drm/xe/pxp: Clarify PXP queue creation behavior if PXP is not ready

[Souza, Jose]

On Mon, 2025-05-19 at 16:32 -0700, Daniele Ceraolo Spurio wrote:
> The expected flow of operations when using PXP is to query the PXP
> status and wait for it to transition to "ready" before attempting to
> create an exec_queue. This flow is followed by the Mesa driver, but
> there is no guarantee that an incorrectly coded (or malicious) app
> will not attempt to create the queue first without querying the status.
> Therefore, we need to clarify what the expected behavior of the queue
> creation ioctl is in this scenario.
> 
> Currently, the ioctl always fails with an -EBUSY code no matter the
> error, but for consistency it is better to distinguish between "failed
> to init" (-EIO) and "not ready" (-EBUSY), the same way the query ioctl
> does. Note that, while this is a change in the return code of an ioctl,
> the behavior of the ioctl in this particular corner case was not clearly
> spec'd, so no one should have been relying on it (and we know that Mesa,
> which is the only known userspace for this, didn't).
> 

Reviewed-by: José Roberto de Souza <jose.souza at intel.com>

> Fixes: 72d479601d67 ("drm/xe/pxp/uapi: Add userspace and LRC support for PXP-using queues")
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Cc: John Harrison <John.C.Harrison at Intel.com>
> Cc: José Roberto de Souza <jose.souza at intel.com>
> ---
>  drivers/gpu/drm/xe/xe_pxp.c | 8 ++++++--
>  include/uapi/drm/xe_drm.h   | 5 +++++
>  2 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_pxp.c b/drivers/gpu/drm/xe/xe_pxp.c
> index 454ea7dc08ac..b5bc15f436fa 100644
> --- a/drivers/gpu/drm/xe/xe_pxp.c
> +++ b/drivers/gpu/drm/xe/xe_pxp.c
> @@ -541,10 +541,14 @@ int xe_pxp_exec_queue_add(struct xe_pxp *pxp, struct xe_exec_queue *q)
>  	 */
>  	xe_pm_runtime_get(pxp->xe);
>  
> -	if (!pxp_prerequisites_done(pxp)) {
> -		ret = -EBUSY;
> +	/* get_readiness_status() returns 0 for in-progress and 1 for done */
> +	ret = xe_pxp_get_readiness_status(pxp);
> +	if (ret <= 0) {
> +		if (!ret)
> +			ret = -EBUSY;
>  		goto out;
>  	}
> +	ret = 0;
>  
>  wait_for_idle:
>  	/*
> diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
> index 9c08738c3b91..2708eb73c590 100644
> --- a/include/uapi/drm/xe_drm.h
> +++ b/include/uapi/drm/xe_drm.h
> @@ -1210,6 +1210,11 @@ struct drm_xe_vm_bind {
>   *    there is no need to explicitly set that. When a queue of type
>   *    %DRM_XE_PXP_TYPE_HWDRM is created, the PXP default HWDRM session
>   *    (%XE_PXP_HWDRM_DEFAULT_SESSION) will be started, if isn't already running.
> + *    The user is expected to query the PXP status via the query ioctl (see
> + *    %DRM_XE_DEVICE_QUERY_PXP_STATUS) and to wait for PXP to be ready before
> + *    attempting to create a queue with this property. If a queue is created
> + *    before PXP is ready, the ioctl will return -EBUSY if init is still in
> + *    progress or -EIO if init failed.
>   *    Given that going into a power-saving state kills PXP HWDRM sessions,
>   *    runtime PM will be blocked while queues of this type are alive.
>   *    All PXP queues will be killed if a PXP invalidation event occurs.