Re: [PATCH v4 1/1] drm/xe/guc: Add Wa_14025883347 for GuC DMA failure on reset

[Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>]

On 2/3/2026 3:36 PM, Matt Roper wrote:
> On Mon, Feb 02, 2026 at 11:47:18AM -0800, Daniele Ceraolo Spurio wrote:
>>
>> On 2/2/2026 2:53 AM, Sk Anirban wrote:
>>> Prevent GuC firmware DMA failures during GuC-only reset by disabling
>>> idle flow and verifying SRAM handling completion. Without this, reset
>>> can be issued while SRAM handler is copying WOPCM to SRAM,
>>> causing GuC HW to get stuck.
>>>
>>> v2: Modify error message (Badal)
>>>       Rename reg bit name (Daniele)
>>>       Update WA skip condition (Daniele)
>>>       Update SRAM handling logic (Daniele)
>>> v3: Reorder WA call (Badal)
>>>       Wait for GuC ready status (Daniele)
>>> v4: Update reg name (Badal)
>>>       Add comment (Daniele)
>>>       Add extended graphics version (Daniele)
>>>       Modify rules
>>>
>>> Signed-off-by: Sk Anirban <sk.anirban@intel.com>
>>> ---
>>>    drivers/gpu/drm/xe/regs/xe_guc_regs.h |  8 ++++++
>>>    drivers/gpu/drm/xe/xe_guc.c           | 38 +++++++++++++++++++++++++++
>>>    drivers/gpu/drm/xe/xe_wa_oob.rules    |  3 +++
>>>    3 files changed, 49 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/xe/regs/xe_guc_regs.h b/drivers/gpu/drm/xe/regs/xe_guc_regs.h
>>> index 87984713dd12..5faac8316b66 100644
>>> --- a/drivers/gpu/drm/xe/regs/xe_guc_regs.h
>>> +++ b/drivers/gpu/drm/xe/regs/xe_guc_regs.h
>>> @@ -40,6 +40,9 @@
>>>    #define   GS_BOOTROM_JUMP_PASSED		REG_FIELD_PREP(GS_BOOTROM_MASK, 0x76)
>>>    #define   GS_MIA_IN_RESET			REG_BIT(0)
>>> +#define BOOT_HASH_CHK				XE_REG(0xc010)
>>> +#define   GUC_BOOT_UKERNEL_VALID		REG_BIT(31)
>>> +
>>>    #define GUC_HEADER_INFO				XE_REG(0xc014)
>>>    #define GUC_WOPCM_SIZE				XE_REG(0xc050)
>>> @@ -83,7 +86,12 @@
>>>    #define   GUC_WOPCM_OFFSET_MASK			REG_GENMASK(31, GUC_WOPCM_OFFSET_SHIFT)
>>>    #define   HUC_LOADING_AGENT_GUC			REG_BIT(1)
>>>    #define   GUC_WOPCM_OFFSET_VALID		REG_BIT(0)
>>> +
>>> +#define GUC_SRAM_STATUS				XE_REG(0xc398)
>>> +#define   GUC_SRAM_HANDLING_MASK		REG_GENMASK(8, 7)
>>> +
>>>    #define GUC_MAX_IDLE_COUNT			XE_REG(0xc3e4)
>>> +#define   GUC_IDLE_FLOW_DISABLE			REG_BIT(31)
>>>    #define GUC_PMTIMESTAMP_LO			XE_REG(0xc3e8)
>>>    #define GUC_PMTIMESTAMP_HI			XE_REG(0xc3ec)
>>> diff --git a/drivers/gpu/drm/xe/xe_guc.c b/drivers/gpu/drm/xe/xe_guc.c
>>> index 2efc4678fa73..369b5a8f129a 100644
>>> --- a/drivers/gpu/drm/xe/xe_guc.c
>>> +++ b/drivers/gpu/drm/xe/xe_guc.c
>>> @@ -911,6 +911,41 @@ int xe_guc_post_load_init(struct xe_guc *guc)
>>>    	return xe_guc_submit_enable(guc);
>>>    }
>>> +/*
>>> + * Wa_14025883347: Prevent GuC firmware DMA failures during GuC-only reset by ensuring
>>> + * SRAM save/restore operations are complete before reset.
>>> + */
>>> +static void guc_prevent_fw_dma_failure_on_reset(struct xe_guc *guc)
>>> +{
>>> +	struct xe_gt *gt = guc_to_gt(guc);
>>> +	u32 boot_hash_chk, guc_status, sram_status;
>>> +	int ret;
>>> +
>>> +	guc_status = xe_mmio_read32(&gt->mmio, GUC_STATUS);
>>> +	if (guc_status & GS_MIA_IN_RESET)
>>> +		return;
>>> +
>>> +	boot_hash_chk = xe_mmio_read32(&gt->mmio, BOOT_HASH_CHK);
>>> +	if (!(boot_hash_chk & GUC_BOOT_UKERNEL_VALID))
>>> +		return;
>>> +
>>> +	/* Disable idle flow during reset (GuC reset re-enables it automatically) */
>>> +	xe_mmio_rmw32(&gt->mmio, GUC_MAX_IDLE_COUNT, 0, GUC_IDLE_FLOW_DISABLE);
>>> +
>>> +	ret = xe_mmio_wait32(&gt->mmio, GUC_STATUS, GS_UKERNEL_MASK,
>>> +			     FIELD_PREP(GS_UKERNEL_MASK, XE_GUC_LOAD_STATUS_READY),
>>> +			     100000, &guc_status, false);
>>> +	if (ret)
>>> +		xe_gt_warn(gt, "GuC not ready after disabling idle flow (GUC_STATUS: 0x%x)\n",
>>> +			   guc_status);
>>> +
>>> +	ret = xe_mmio_wait32(&gt->mmio, GUC_SRAM_STATUS, GUC_SRAM_HANDLING_MASK,
>>> +			     0, 5000, &sram_status, false);
>>> +	if (ret)
>>> +		xe_gt_warn(gt, "SRAM handling not complete (GUC_SRAM_STATUS: 0x%x)\n",
>>> +			   sram_status);
>>> +}
>>> +
>>>    int xe_guc_reset(struct xe_guc *guc)
>>>    {
>>>    	struct xe_gt *gt = guc_to_gt(guc);
>>> @@ -923,6 +958,9 @@ int xe_guc_reset(struct xe_guc *guc)
>>>    	if (IS_SRIOV_VF(gt_to_xe(gt)))
>>>    		return xe_gt_sriov_vf_bootstrap(gt);
>>> +	if (XE_GT_WA(gt, 14025883347))
>>> +		guc_prevent_fw_dma_failure_on_reset(guc);
>>> +
>>>    	xe_mmio_write32(mmio, GDRST, GRDOM_GUC);
>>>    	ret = xe_mmio_wait32(mmio, GDRST, GRDOM_GUC, 0, 5000, &gdrst, false);
>>> diff --git a/drivers/gpu/drm/xe/xe_wa_oob.rules b/drivers/gpu/drm/xe/xe_wa_oob.rules
>>> index 5cd7fa6d2a5c..ac08f94f90a1 100644
>>> --- a/drivers/gpu/drm/xe/xe_wa_oob.rules
>>> +++ b/drivers/gpu/drm/xe/xe_wa_oob.rules
>>> @@ -73,3 +73,6 @@
>>>    15015404425_disable	PLATFORM(PANTHERLAKE), MEDIA_STEP(B0, FOREVER)
>>>    16026007364    MEDIA_VERSION(3000)
>>>    14020316580    MEDIA_VERSION(1301)
>>> +
>>> +14025883347	MEDIA_VERSION_RANGE(1301, 3503)
>>> +		GRAPHICS_VERSION_RANGE(2004, 3005)
>> We usually avoid having these big ranges in the WA table, in case a new
>> derivative gets introduced that doesn't require the WA. If you think it is
>> justified in this case, please get an ack from the platform enabling side.
>> Apart from this the patch LGTM, so I'll give my r-b if you get the ack.
> Yeah, in the past we've tried to avoid big ranges for this reason, but
> I've heard suggestions from a few people lately that they'd like us to
> revisit that policy.
>
> Early in Xe development the driver didn't really support that many
> different platforms / IP versions, so even when we had a rare workaround
> that needed to hit them all, listing each separately wasn't much of a
> hassle.  But now that we're a couple years down the road, the number of
> platforms and IP versions has grown a lot, so it's a lot more painful on
> workarounds like this to list them all out.  If we switch to ranges,
> then we'll need to do more work when first enabling a new IP with an
> "intermediate" number (like the Xe3p_LPG that just landed on the mailing
> list) to make sure none of the existing workarounds need to be dropped
> out for it.  But I think I'm okay with that now.
>
> tl;dr: let's go ahead with the ranges
>
> Acked-by: Matt Roper <matthew.d.roper@intel.com>

Sounds good.

Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

Daniele

>
>
> Matt
>
>> Daniele
>>
>>