[Intel-xe] [PATCH v2 3/4] drm/xe/guc: Fix handling of GUC_HXG_TYPE_NO_RESPONSE_BUSY

[Matthew Brost]

On Thu, Nov 16, 2023 at 04:12:41PM +0100, Michal Wajdeczko wrote:
> If GuC responds with the NO_RESPONSE_BUSY message, we extend
> our timeout while waiting for the actual response, but we wrongly
> assumed that the next message will be RESPONSE_SUCCESS, missing
> that we still can get RESPONSE_FAILURE.
> 
> Change the condition for the expected message type, using only
> common bits from RESPONSE_SUCCESS and RESPONSE_FAILURE (as they
> differ, by ABI design, only by the last bit).
> 
> v2: add comment/checks to the code (Matt)
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>

Reviewed-by: Matthew Brost <matthew.brost at intel.com>

> ---
>  drivers/gpu/drm/xe/xe_guc.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_guc.c b/drivers/gpu/drm/xe/xe_guc.c
> index aa4af536c1e9..56edcb2b0e45 100644
> --- a/drivers/gpu/drm/xe/xe_guc.c
> +++ b/drivers/gpu/drm/xe/xe_guc.c
> @@ -659,9 +659,20 @@ int xe_guc_mmio_send_recv(struct xe_guc *guc, const u32 *request,
>  	header = xe_mmio_read32(gt, reply_reg);
>  	if (FIELD_GET(GUC_HXG_MSG_0_TYPE, header) ==
>  	    GUC_HXG_TYPE_NO_RESPONSE_BUSY) {
> +		/*
> +		 * Once we got a BUSY reply we must wait again for the final
> +		 * response but this time we can't use ORIGIN mask anymore.
> +		 * To spot a right change in the reply, we take advantage that
> +		 * response SUCCESS and FAILURE differ only by the single bit
> +		 * and all other bits are set and can be used as a new mask.
> +		 */
> +		u32 resp_bits = GUC_HXG_TYPE_RESPONSE_SUCCESS & GUC_HXG_TYPE_RESPONSE_FAILURE;
> +		u32 resp_mask = FIELD_PREP(GUC_HXG_MSG_0_TYPE, resp_bits);
>  
> -		ret = xe_mmio_wait32(gt, reply_reg, GUC_HXG_MSG_0_TYPE,
> -				     FIELD_PREP(GUC_HXG_MSG_0_TYPE, GUC_HXG_TYPE_RESPONSE_SUCCESS),
> +		BUILD_BUG_ON(FIELD_MAX(GUC_HXG_MSG_0_TYPE) != GUC_HXG_TYPE_RESPONSE_SUCCESS);
> +		BUILD_BUG_ON((GUC_HXG_TYPE_RESPONSE_SUCCESS ^ GUC_HXG_TYPE_RESPONSE_FAILURE) != 1);
> +
> +		ret = xe_mmio_wait32(gt, reply_reg,  resp_mask, resp_mask,
>  				     1000000, &header, false);
>  
>  		if (unlikely(FIELD_GET(GUC_HXG_MSG_0_ORIGIN, header) !=
> -- 
> 2.25.1
>