Intel-XE Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Jia Yao <jia.yao@intel.com>
To: intel-xe@lists.freedesktop.org
Cc: Jia Yao <jia.yao@intel.com>,
	Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>,
	Matthew Auld <matthew.auld@intel.com>
Subject: [PATCH v2] drm/xe/guc_ads: allocate UM queues in a separate UC BO
Date: Mon, 13 Jul 2026 00:40:48 +0000	[thread overview]
Message-ID: <20260713004048.1245794-1-jia.yao@intel.com> (raw)

On integrated graphics, the GPU updates the HWQ tail pointer
(non-posted, strongly ordered) before the fault descriptor data reaches
DRAM (posted WB through L3, weakly ordered).  GuC reads the ring after
seeing the tail update but before the L3 write is flushed, resulting in
an all-zero descriptor being forwarded to the driver as a spurious page
fault.  In the worst case this triggers a P2I timeout on an unrelated
context.

On discrete graphics, xe_bo_main_addr() returns a CPU DMA IOVA for
SYSTEM BOs.  On platforms with limited DRAM (e.g. BMG with 4 GB) the
IOVA can alias a PCI MMIO reserved window, so GPU writes to the fault
ring go nowhere and GuC reads all-zeros.

Additionally, xe_guc_ads_populate() zeroes the entire ADS blob on every
GT reset, destroying any fault descriptor written between GDRST and GuC
restart.

Fix all three by allocating the UM queues in a separate BO
(ads->um_queue_bo):

- UC mapping (XE_BO_FLAG_NEEDS_UC) makes GPU writes bypass L3 and land
  in DRAM before the tail-pointer update reaches GuC.  On dGFX, PCIe
  writes are already strongly ordered, so UC is a harmless no-op.

- VRAM placement on dGFX (XE_BO_FLAG_VRAM_IF_DGFX) gives a well-defined
  DPA, sidestepping the IOVA aliasing issue.

- A separate BO is never touched by xe_guc_ads_populate()'s memset.

v2: also fix dGFX (VRAM placement + UC flag); remove now-dead
    guc_ads_um_queues_size/offset helpers

Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Signed-off-by: Jia Yao <jia.yao@intel.com>
---
 drivers/gpu/drm/xe/xe_guc_ads.c       | 65 ++++++++++++++++-----------
 drivers/gpu/drm/xe/xe_guc_ads_types.h | 10 ++++-
 2 files changed, 49 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_guc_ads.c b/drivers/gpu/drm/xe/xe_guc_ads.c
index c98454545a85..e321fab2ee1b 100644
--- a/drivers/gpu/drm/xe/xe_guc_ads.c
+++ b/drivers/gpu/drm/xe/xe_guc_ads.c
@@ -155,16 +155,6 @@ static size_t guc_ads_capture_size(struct xe_guc_ads *ads)
 	return PAGE_ALIGN(ads->capture_size);
 }
 
-static size_t guc_ads_um_queues_size(struct xe_guc_ads *ads)
-{
-	struct xe_device *xe = ads_to_xe(ads);
-
-	if (!xe->info.has_usm)
-		return 0;
-
-	return GUC_UM_QUEUE_SIZE * GUC_UM_HW_QUEUE_MAX;
-}
-
 static size_t guc_ads_private_data_size(struct xe_guc_ads *ads)
 {
 	return PAGE_ALIGN(ads_to_guc(ads)->fw.private_data_size);
@@ -205,22 +195,12 @@ static size_t guc_ads_capture_offset(struct xe_guc_ads *ads)
 	return PAGE_ALIGN(offset);
 }
 
-static size_t guc_ads_um_queues_offset(struct xe_guc_ads *ads)
-{
-	u32 offset;
-
-	offset = guc_ads_capture_offset(ads) +
-		 guc_ads_capture_size(ads);
-
-	return PAGE_ALIGN(offset);
-}
-
 static size_t guc_ads_private_data_offset(struct xe_guc_ads *ads)
 {
 	size_t offset;
 
-	offset = guc_ads_um_queues_offset(ads) +
-		guc_ads_um_queues_size(ads);
+	offset = guc_ads_capture_offset(ads) +
+		 guc_ads_capture_size(ads);
 
 	return PAGE_ALIGN(offset);
 }
@@ -409,6 +389,30 @@ int xe_guc_ads_init(struct xe_guc_ads *ads)
 
 	ads->bo = bo;
 
+	/*
+	 * UM queues are in a separate UC BO (not the main ADS blob) so that
+	 * xe_guc_ads_populate() cannot zero fault descriptors written between
+	 * GDRST and GuC restart.  UC ensures GPU descriptor writes bypass L3
+	 * and reach DRAM before GuC reads the ring.  On dGFX, VRAM is used so
+	 * the DPA is well-defined; SYSTEM BOs use a CPU DMA IOVA that may
+	 * alias a PCI MMIO region on platforms with limited DRAM.
+	 */
+	if (xe->info.has_usm) {
+		u32 um_flags = XE_BO_FLAG_GGTT |
+			       XE_BO_FLAG_GGTT_INVALIDATE |
+			       XE_BO_FLAG_PINNED_NORESTORE |
+			       XE_BO_FLAG_NEEDS_UC |
+			       XE_BO_FLAG_VRAM_IF_DGFX(tile);
+
+		bo = xe_managed_bo_create_pin_map(xe, tile,
+					  GUC_UM_QUEUE_SIZE * GUC_UM_HW_QUEUE_MAX,
+					  um_flags);
+		if (IS_ERR(bo))
+			return PTR_ERR(bo);
+
+		ads->um_queue_bo = bo;
+	}
+
 	return 0;
 }
 ALLOW_ERROR_INJECTION(xe_guc_ads_init, ERRNO); /* See xe_pci_probe() */
@@ -820,9 +824,9 @@ static void guc_mmio_reg_state_init(struct xe_guc_ads *ads)
 
 static void guc_um_init_params(struct xe_guc_ads *ads)
 {
-	u32 um_queue_offset = guc_ads_um_queues_offset(ads);
 	struct xe_guc *guc = ads_to_guc(ads);
 	struct xe_device *xe = ads_to_xe(ads);
+	struct xe_bo *um_bo = ads->um_queue_bo;
 	u64 base_dpa;
 	u32 base_ggtt;
 	bool with_dpa;
@@ -830,8 +834,14 @@ static void guc_um_init_params(struct xe_guc_ads *ads)
 
 	with_dpa = !xe_guc_using_main_gamctrl_queues(guc);
 
-	base_ggtt = xe_bo_ggtt_addr(ads->bo) + um_queue_offset;
-	base_dpa = xe_bo_main_addr(ads->bo, PAGE_SIZE) + um_queue_offset;
+	if (um_bo) {
+		/* All USM platforms: UM queues in dedicated um_queue_bo */
+		base_ggtt = xe_bo_ggtt_addr(um_bo);
+		base_dpa = xe_bo_main_addr(um_bo, PAGE_SIZE);
+	} else {
+		/* Platform does not support USM: no UM queues, nothing to do */
+		return;
+	}
 
 	for (i = 0; i < GUC_UM_HW_QUEUE_MAX; ++i) {
 		/*
@@ -912,6 +922,11 @@ void xe_guc_ads_populate(struct xe_guc_ads *ads)
 	xe_gt_assert(gt, ads->bo);
 
 	xe_map_memset(ads_to_xe(ads), ads_to_map(ads), 0, 0, xe_bo_size(ads->bo));
+	/*
+	 * um_queue_bo is not zeroed here. GuC resets the queue head/tail to
+	 * zero via MMIO on every restart, so the ring contents are never read
+	 * until GPU hardware produces a new entry. No CPU zeroing needed.
+	 */
 	guc_policies_init(ads);
 	fill_engine_enable_masks(gt, &info_map);
 	guc_mmio_reg_state_init(ads);
diff --git a/drivers/gpu/drm/xe/xe_guc_ads_types.h b/drivers/gpu/drm/xe/xe_guc_ads_types.h
index 48a8e092023f..cced8a7b4885 100644
--- a/drivers/gpu/drm/xe/xe_guc_ads_types.h
+++ b/drivers/gpu/drm/xe/xe_guc_ads_types.h
@@ -14,8 +14,16 @@ struct xe_bo;
  * struct xe_guc_ads - GuC additional data structures (ADS)
  */
 struct xe_guc_ads {
-	/** @bo: Xe BO for GuC ads blob */
+	/** @bo: Xe BO for GuC ads blob (WB cached) */
 	struct xe_bo *bo;
+	/**
+	 * @um_queue_bo: Dedicated UC BO for the HW fault ring (UM queues).
+	 * Separate from the main ADS blob to avoid being zeroed by
+	 * xe_guc_ads_populate() during GT reset. UC-mapped so GPU writes
+	 * bypass L3 and are visible in DRAM before GuC reads the ring.
+	 * VRAM on dGFX for a well-defined DPA; NULL if no USM support.
+	 */
+	struct xe_bo *um_queue_bo;
 	/** @golden_lrc_size: golden LRC size */
 	size_t golden_lrc_size;
 	/** @regset_size: size of register set passed to GuC for save/restore */
-- 
2.43.0


             reply	other threads:[~2026-07-13  0:40 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-13  0:40 Jia Yao [this message]
2026-07-13  0:46 ` ✗ CI.checkpatch: warning for drm/xe/guc_ads: allocate UM queues in a separate UC BO (rev2) Patchwork
2026-07-13  0:47 ` ✓ CI.KUnit: success " Patchwork
2026-07-13  1:29 ` ✓ Xe.CI.BAT: " Patchwork
2026-07-13  2:36 ` ✗ Xe.CI.FULL: failure " Patchwork
2026-07-21 10:09 ` [PATCH v2] drm/xe/guc_ads: allocate UM queues in a separate UC BO Matthew Auld

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260713004048.1245794-1-jia.yao@intel.com \
    --to=jia.yao@intel.com \
    --cc=gwan-gyeong.mun@intel.com \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=matthew.auld@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox