* [PATCH v7 1/3] drm/xe/guc_ads: allocate UM queues in a separate BO [not found] <20260803200838.98826-1-jia.yao@intel.com> @ 2026-08-03 20:08 ` Jia Yao 2026-08-04 7:43 ` Matthew Auld 2026-08-04 8:00 ` Matthew Auld 2026-08-03 20:08 ` [PATCH v7 2/3] drm/xe/guc_ads: allocate UM queues in VRAM on dGFX Jia Yao 2026-08-03 20:08 ` [PATCH v7 3/3] drm/xe/guc_ads: use uncached mapping for UM queue BO Jia Yao 2 siblings, 2 replies; 10+ messages in thread From: Jia Yao @ 2026-08-03 20:08 UTC (permalink / raw) To: intel-xe; +Cc: Jia Yao, Gwan-gyeong Mun, Matthew Auld, stable Move the UM queues into a dedicated UC BO (ads->um_queue_bo) and avoid CPU memset operations on it, which eliminates the CPU as a potential cacheline-polluting agent and helps maintain consistency between GAM writes and GuC reads. Fixes: 9c57bc08652a ("drm/xe/lnl: Drop force_probe requirement") Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> Cc: Matthew Auld <matthew.auld@intel.com> Cc: <stable@vger.kernel.org> # v6.12+ Signed-off-by: Jia Yao <jia.yao@intel.com> --- drivers/gpu/drm/xe/xe_guc.c | 6 ++ drivers/gpu/drm/xe/xe_guc_ads.c | 89 +++++++++++++++++---------- drivers/gpu/drm/xe/xe_guc_ads_types.h | 5 ++ 3 files changed, 68 insertions(+), 32 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_guc.c b/drivers/gpu/drm/xe/xe_guc.c index 4286bd05c686..4f132bf8539d 100644 --- a/drivers/gpu/drm/xe/xe_guc.c +++ b/drivers/gpu/drm/xe/xe_guc.c @@ -732,6 +732,12 @@ static int xe_guc_realloc_post_hwconfig(struct xe_guc *guc) if (ret) return ret; + if (guc->ads.um_queue_bo) { + ret = xe_managed_bo_reinit_in_vram(xe, tile, &guc->ads.um_queue_bo); + if (ret) + return ret; + } + return 0; } diff --git a/drivers/gpu/drm/xe/xe_guc_ads.c b/drivers/gpu/drm/xe/xe_guc_ads.c index f0ac00586d3a..ef8e9cc35c8c 100644 --- a/drivers/gpu/drm/xe/xe_guc_ads.c +++ b/drivers/gpu/drm/xe/xe_guc_ads.c @@ -8,6 +8,7 @@ #include <linux/fault-inject.h> #include <drm/drm_managed.h> +#include <drm/drm_prime.h> #include <generated/xe_wa_oob.h> @@ -64,10 +65,14 @@ ads_to_map(struct xe_guc_ads *ads) /* * The Additional Data Struct (ADS) has pointers for different buffers used by - * the GuC. One single gem object contains the ADS struct itself (guc_ads) and - * all the extra buffers indirectly linked via the ADS struct's entries. + * the GuC. One gem object (ads->bo) contains the ADS struct itself (guc_ads) + * and most of the extra buffers linked via the ADS struct's entries. The UM + * fault queues (PAGE_FAULT, PAGE_FAULT_RESPONSE, ACCESS_COUNTER rings) are + * kept in a separate BO (ads->um_queue_bo) so that the full memset of ads->bo + * performed on every GT reset does not discard fault descriptors already + * written into the rings by the GPU. * - * Layout of the ADS blob allocated for the GuC: + * Layout of the ADS blob (ads->bo): * * +---------------------------------------+ <== base * | guc_ads | @@ -99,10 +104,6 @@ ads_to_map(struct xe_guc_ads *ads) * +---------------------------------------+ * | padding | * +---------------------------------------+ <== 4K aligned - * | UM queues | - * +---------------------------------------+ - * | padding | - * +---------------------------------------+ <== 4K aligned * | private data | * +---------------------------------------+ * | padding | @@ -156,16 +157,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); @@ -206,22 +197,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); } @@ -461,6 +442,44 @@ int xe_guc_ads_init(struct xe_guc_ads *ads) ads->bo = bo; + if (xe->info.has_usm) { + /* + * Allocate a separate BO for the HW fault ring (UM queues). + * + * Round the size up to the next power of two so that + * (system memory, no IOMMU) the TTM pool issues a single + * alloc_pages(order=N) call, maximising the chance of getting + * a physically contiguous block. GuC requires contiguous DPA. + */ + size_t um_size = roundup_pow_of_two(GUC_UM_QUEUE_SIZE * + GUC_UM_HW_QUEUE_MAX); + + bo = xe_managed_bo_create_pin_map(xe, tile, um_size, + XE_BO_FLAG_SYSTEM | + XE_BO_FLAG_GGTT | + XE_BO_FLAG_GGTT_INVALIDATE | + XE_BO_FLAG_PINNED_NORESTORE); + if (IS_ERR(bo)) + return PTR_ERR(bo); + + /* + * On pre-Xe3p platforms, GAM (not GuC) accesses the UM queue + * ring via base_dpa, which must be a contiguous DMA address + * range. Verify that the allocated pages are contiguous in + * DMA address space. + */ + if (xe_bo_sg(bo) && + !xe_guc_using_main_gamctrl_queues(ads_to_guc(ads)) && + unlikely(drm_prime_get_contiguous_size(xe_bo_sg(bo)) < + GUC_UM_QUEUE_SIZE * GUC_UM_HW_QUEUE_MAX)) { + drm_err(&xe->drm, + "UM fault queue memory is not contiguous in DMA address space; GAM requires contiguous DPA\n"); + return -ENOMEM; + } + + ads->um_queue_bo = bo; + } + return 0; } ALLOW_ERROR_INJECTION(xe_guc_ads_init, ERRNO); /* See xe_pci_probe() */ @@ -893,7 +912,7 @@ 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_bo *um_bo = ads->um_queue_bo; struct xe_guc *guc = ads_to_guc(ads); struct xe_device *xe = ads_to_xe(ads); u64 base_dpa; @@ -903,8 +922,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) { /* diff --git a/drivers/gpu/drm/xe/xe_guc_ads_types.h b/drivers/gpu/drm/xe/xe_guc_ads_types.h index 48a8e092023f..845c1fbd93a4 100644 --- a/drivers/gpu/drm/xe/xe_guc_ads_types.h +++ b/drivers/gpu/drm/xe/xe_guc_ads_types.h @@ -16,6 +16,11 @@ struct xe_bo; struct xe_guc_ads { /** @bo: Xe BO for GuC ads blob */ struct xe_bo *bo; + /** + * @um_queue_bo: Dedicated BO for the HW fault ring (UM queues). + * NULL if the platform does not support USM. + */ + 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v7 1/3] drm/xe/guc_ads: allocate UM queues in a separate BO 2026-08-03 20:08 ` [PATCH v7 1/3] drm/xe/guc_ads: allocate UM queues in a separate BO Jia Yao @ 2026-08-04 7:43 ` Matthew Auld 2026-08-04 8:00 ` Matthew Auld 1 sibling, 0 replies; 10+ messages in thread From: Matthew Auld @ 2026-08-04 7:43 UTC (permalink / raw) To: Jia Yao, intel-xe; +Cc: Gwan-gyeong Mun, stable On 03/08/2026 21:08, Jia Yao wrote: > Move the UM queues into a dedicated UC BO (ads->um_queue_bo) and avoid > CPU memset operations on it, which eliminates the CPU as a potential > cacheline-polluting agent and helps maintain consistency between GAM > writes and GuC reads. > > Fixes: 9c57bc08652a ("drm/xe/lnl: Drop force_probe requirement") > Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> > Cc: Matthew Auld <matthew.auld@intel.com> > Cc: <stable@vger.kernel.org> # v6.12+ > Signed-off-by: Jia Yao <jia.yao@intel.com> Reviewed-by: Matthew Auld <matthew.auld@intel.com> > --- > drivers/gpu/drm/xe/xe_guc.c | 6 ++ > drivers/gpu/drm/xe/xe_guc_ads.c | 89 +++++++++++++++++---------- > drivers/gpu/drm/xe/xe_guc_ads_types.h | 5 ++ > 3 files changed, 68 insertions(+), 32 deletions(-) > > diff --git a/drivers/gpu/drm/xe/xe_guc.c b/drivers/gpu/drm/xe/xe_guc.c > index 4286bd05c686..4f132bf8539d 100644 > --- a/drivers/gpu/drm/xe/xe_guc.c > +++ b/drivers/gpu/drm/xe/xe_guc.c > @@ -732,6 +732,12 @@ static int xe_guc_realloc_post_hwconfig(struct xe_guc *guc) > if (ret) > return ret; > > + if (guc->ads.um_queue_bo) { > + ret = xe_managed_bo_reinit_in_vram(xe, tile, &guc->ads.um_queue_bo); > + if (ret) > + return ret; > + } > + > return 0; > } > > diff --git a/drivers/gpu/drm/xe/xe_guc_ads.c b/drivers/gpu/drm/xe/xe_guc_ads.c > index f0ac00586d3a..ef8e9cc35c8c 100644 > --- a/drivers/gpu/drm/xe/xe_guc_ads.c > +++ b/drivers/gpu/drm/xe/xe_guc_ads.c > @@ -8,6 +8,7 @@ > #include <linux/fault-inject.h> > > #include <drm/drm_managed.h> > +#include <drm/drm_prime.h> > > #include <generated/xe_wa_oob.h> > > @@ -64,10 +65,14 @@ ads_to_map(struct xe_guc_ads *ads) > > /* > * The Additional Data Struct (ADS) has pointers for different buffers used by > - * the GuC. One single gem object contains the ADS struct itself (guc_ads) and > - * all the extra buffers indirectly linked via the ADS struct's entries. > + * the GuC. One gem object (ads->bo) contains the ADS struct itself (guc_ads) > + * and most of the extra buffers linked via the ADS struct's entries. The UM > + * fault queues (PAGE_FAULT, PAGE_FAULT_RESPONSE, ACCESS_COUNTER rings) are > + * kept in a separate BO (ads->um_queue_bo) so that the full memset of ads->bo > + * performed on every GT reset does not discard fault descriptors already > + * written into the rings by the GPU. > * > - * Layout of the ADS blob allocated for the GuC: > + * Layout of the ADS blob (ads->bo): > * > * +---------------------------------------+ <== base > * | guc_ads | > @@ -99,10 +104,6 @@ ads_to_map(struct xe_guc_ads *ads) > * +---------------------------------------+ > * | padding | > * +---------------------------------------+ <== 4K aligned > - * | UM queues | > - * +---------------------------------------+ > - * | padding | > - * +---------------------------------------+ <== 4K aligned > * | private data | > * +---------------------------------------+ > * | padding | > @@ -156,16 +157,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); > @@ -206,22 +197,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); > } > @@ -461,6 +442,44 @@ int xe_guc_ads_init(struct xe_guc_ads *ads) > > ads->bo = bo; > > + if (xe->info.has_usm) { > + /* > + * Allocate a separate BO for the HW fault ring (UM queues). > + * > + * Round the size up to the next power of two so that > + * (system memory, no IOMMU) the TTM pool issues a single > + * alloc_pages(order=N) call, maximising the chance of getting > + * a physically contiguous block. GuC requires contiguous DPA. > + */ > + size_t um_size = roundup_pow_of_two(GUC_UM_QUEUE_SIZE * > + GUC_UM_HW_QUEUE_MAX); > + > + bo = xe_managed_bo_create_pin_map(xe, tile, um_size, > + XE_BO_FLAG_SYSTEM | > + XE_BO_FLAG_GGTT | > + XE_BO_FLAG_GGTT_INVALIDATE | > + XE_BO_FLAG_PINNED_NORESTORE); > + if (IS_ERR(bo)) > + return PTR_ERR(bo); > + > + /* > + * On pre-Xe3p platforms, GAM (not GuC) accesses the UM queue > + * ring via base_dpa, which must be a contiguous DMA address > + * range. Verify that the allocated pages are contiguous in > + * DMA address space. > + */ > + if (xe_bo_sg(bo) && > + !xe_guc_using_main_gamctrl_queues(ads_to_guc(ads)) && > + unlikely(drm_prime_get_contiguous_size(xe_bo_sg(bo)) < > + GUC_UM_QUEUE_SIZE * GUC_UM_HW_QUEUE_MAX)) { > + drm_err(&xe->drm, > + "UM fault queue memory is not contiguous in DMA address space; GAM requires contiguous DPA\n"); > + return -ENOMEM; > + } > + > + ads->um_queue_bo = bo; > + } > + > return 0; > } > ALLOW_ERROR_INJECTION(xe_guc_ads_init, ERRNO); /* See xe_pci_probe() */ > @@ -893,7 +912,7 @@ 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_bo *um_bo = ads->um_queue_bo; > struct xe_guc *guc = ads_to_guc(ads); > struct xe_device *xe = ads_to_xe(ads); > u64 base_dpa; > @@ -903,8 +922,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) { > /* > diff --git a/drivers/gpu/drm/xe/xe_guc_ads_types.h b/drivers/gpu/drm/xe/xe_guc_ads_types.h > index 48a8e092023f..845c1fbd93a4 100644 > --- a/drivers/gpu/drm/xe/xe_guc_ads_types.h > +++ b/drivers/gpu/drm/xe/xe_guc_ads_types.h > @@ -16,6 +16,11 @@ struct xe_bo; > struct xe_guc_ads { > /** @bo: Xe BO for GuC ads blob */ > struct xe_bo *bo; > + /** > + * @um_queue_bo: Dedicated BO for the HW fault ring (UM queues). > + * NULL if the platform does not support USM. > + */ > + 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 */ ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v7 1/3] drm/xe/guc_ads: allocate UM queues in a separate BO 2026-08-03 20:08 ` [PATCH v7 1/3] drm/xe/guc_ads: allocate UM queues in a separate BO Jia Yao 2026-08-04 7:43 ` Matthew Auld @ 2026-08-04 8:00 ` Matthew Auld 2026-08-04 8:54 ` Matthew Brost 1 sibling, 1 reply; 10+ messages in thread From: Matthew Auld @ 2026-08-04 8:00 UTC (permalink / raw) To: Jia Yao, intel-xe; +Cc: Gwan-gyeong Mun, stable On 03/08/2026 21:08, Jia Yao wrote: > Move the UM queues into a dedicated UC BO (ads->um_queue_bo) and avoid > CPU memset operations on it, which eliminates the CPU as a potential > cacheline-polluting agent and helps maintain consistency between GAM > writes and GuC reads. > > Fixes: 9c57bc08652a ("drm/xe/lnl: Drop force_probe requirement") > Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> > Cc: Matthew Auld <matthew.auld@intel.com> > Cc: <stable@vger.kernel.org> # v6.12+ > Signed-off-by: Jia Yao <jia.yao@intel.com> > --- > drivers/gpu/drm/xe/xe_guc.c | 6 ++ > drivers/gpu/drm/xe/xe_guc_ads.c | 89 +++++++++++++++++---------- > drivers/gpu/drm/xe/xe_guc_ads_types.h | 5 ++ > 3 files changed, 68 insertions(+), 32 deletions(-) > > diff --git a/drivers/gpu/drm/xe/xe_guc.c b/drivers/gpu/drm/xe/xe_guc.c > index 4286bd05c686..4f132bf8539d 100644 > --- a/drivers/gpu/drm/xe/xe_guc.c > +++ b/drivers/gpu/drm/xe/xe_guc.c > @@ -732,6 +732,12 @@ static int xe_guc_realloc_post_hwconfig(struct xe_guc *guc) > if (ret) > return ret; > > + if (guc->ads.um_queue_bo) { > + ret = xe_managed_bo_reinit_in_vram(xe, tile, &guc->ads.um_queue_bo); > + if (ret) > + return ret; > + } > + > return 0; > } > > diff --git a/drivers/gpu/drm/xe/xe_guc_ads.c b/drivers/gpu/drm/xe/xe_guc_ads.c > index f0ac00586d3a..ef8e9cc35c8c 100644 > --- a/drivers/gpu/drm/xe/xe_guc_ads.c > +++ b/drivers/gpu/drm/xe/xe_guc_ads.c > @@ -8,6 +8,7 @@ > #include <linux/fault-inject.h> > > #include <drm/drm_managed.h> > +#include <drm/drm_prime.h> > > #include <generated/xe_wa_oob.h> > > @@ -64,10 +65,14 @@ ads_to_map(struct xe_guc_ads *ads) > > /* > * The Additional Data Struct (ADS) has pointers for different buffers used by > - * the GuC. One single gem object contains the ADS struct itself (guc_ads) and > - * all the extra buffers indirectly linked via the ADS struct's entries. > + * the GuC. One gem object (ads->bo) contains the ADS struct itself (guc_ads) > + * and most of the extra buffers linked via the ADS struct's entries. The UM > + * fault queues (PAGE_FAULT, PAGE_FAULT_RESPONSE, ACCESS_COUNTER rings) are > + * kept in a separate BO (ads->um_queue_bo) so that the full memset of ads->bo > + * performed on every GT reset does not discard fault descriptors already > + * written into the rings by the GPU. > * > - * Layout of the ADS blob allocated for the GuC: > + * Layout of the ADS blob (ads->bo): > * > * +---------------------------------------+ <== base > * | guc_ads | > @@ -99,10 +104,6 @@ ads_to_map(struct xe_guc_ads *ads) > * +---------------------------------------+ > * | padding | > * +---------------------------------------+ <== 4K aligned > - * | UM queues | > - * +---------------------------------------+ > - * | padding | > - * +---------------------------------------+ <== 4K aligned > * | private data | > * +---------------------------------------+ > * | padding | > @@ -156,16 +157,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); > @@ -206,22 +197,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); > } > @@ -461,6 +442,44 @@ int xe_guc_ads_init(struct xe_guc_ads *ads) > > ads->bo = bo; > > + if (xe->info.has_usm) { > + /* > + * Allocate a separate BO for the HW fault ring (UM queues). > + * > + * Round the size up to the next power of two so that > + * (system memory, no IOMMU) the TTM pool issues a single > + * alloc_pages(order=N) call, maximising the chance of getting > + * a physically contiguous block. GuC requires contiguous DPA. > + */ > + size_t um_size = roundup_pow_of_two(GUC_UM_QUEUE_SIZE * > + GUC_UM_HW_QUEUE_MAX); > + > + bo = xe_managed_bo_create_pin_map(xe, tile, um_size, > + XE_BO_FLAG_SYSTEM | > + XE_BO_FLAG_GGTT | > + XE_BO_FLAG_GGTT_INVALIDATE | > + XE_BO_FLAG_PINNED_NORESTORE); > + if (IS_ERR(bo)) > + return PTR_ERR(bo); > + > + /* > + * On pre-Xe3p platforms, GAM (not GuC) accesses the UM queue > + * ring via base_dpa, which must be a contiguous DMA address > + * range. Verify that the allocated pages are contiguous in > + * DMA address space. > + */ > + if (xe_bo_sg(bo) && There is a comment from Sashiko that there might not be a valid tt for vram buffers, so perhaps just make this check !xe_bo_is_vram() instead? > + !xe_guc_using_main_gamctrl_queues(ads_to_guc(ads)) && > + unlikely(drm_prime_get_contiguous_size(xe_bo_sg(bo)) < > + GUC_UM_QUEUE_SIZE * GUC_UM_HW_QUEUE_MAX)) { > + drm_err(&xe->drm, > + "UM fault queue memory is not contiguous in DMA address space; GAM requires contiguous DPA\n"); > + return -ENOMEM; > + } > + > + ads->um_queue_bo = bo; > + } > + > return 0; > } > ALLOW_ERROR_INJECTION(xe_guc_ads_init, ERRNO); /* See xe_pci_probe() */ > @@ -893,7 +912,7 @@ 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_bo *um_bo = ads->um_queue_bo; > struct xe_guc *guc = ads_to_guc(ads); > struct xe_device *xe = ads_to_xe(ads); > u64 base_dpa; > @@ -903,8 +922,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) { > /* > diff --git a/drivers/gpu/drm/xe/xe_guc_ads_types.h b/drivers/gpu/drm/xe/xe_guc_ads_types.h > index 48a8e092023f..845c1fbd93a4 100644 > --- a/drivers/gpu/drm/xe/xe_guc_ads_types.h > +++ b/drivers/gpu/drm/xe/xe_guc_ads_types.h > @@ -16,6 +16,11 @@ struct xe_bo; > struct xe_guc_ads { > /** @bo: Xe BO for GuC ads blob */ > struct xe_bo *bo; > + /** > + * @um_queue_bo: Dedicated BO for the HW fault ring (UM queues). > + * NULL if the platform does not support USM. > + */ > + 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 */ ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v7 1/3] drm/xe/guc_ads: allocate UM queues in a separate BO 2026-08-04 8:00 ` Matthew Auld @ 2026-08-04 8:54 ` Matthew Brost 2026-08-04 9:16 ` Matthew Auld 0 siblings, 1 reply; 10+ messages in thread From: Matthew Brost @ 2026-08-04 8:54 UTC (permalink / raw) To: Matthew Auld; +Cc: Jia Yao, intel-xe, Gwan-gyeong Mun, stable On Tue, Aug 04, 2026 at 09:00:19AM +0100, Matthew Auld wrote: > On 03/08/2026 21:08, Jia Yao wrote: > > Move the UM queues into a dedicated UC BO (ads->um_queue_bo) and avoid > > CPU memset operations on it, which eliminates the CPU as a potential > > cacheline-polluting agent and helps maintain consistency between GAM > > writes and GuC reads. > > > > Fixes: 9c57bc08652a ("drm/xe/lnl: Drop force_probe requirement") > > Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> > > Cc: Matthew Auld <matthew.auld@intel.com> > > Cc: <stable@vger.kernel.org> # v6.12+ > > Signed-off-by: Jia Yao <jia.yao@intel.com> > > --- > > drivers/gpu/drm/xe/xe_guc.c | 6 ++ > > drivers/gpu/drm/xe/xe_guc_ads.c | 89 +++++++++++++++++---------- > > drivers/gpu/drm/xe/xe_guc_ads_types.h | 5 ++ > > 3 files changed, 68 insertions(+), 32 deletions(-) > > > > diff --git a/drivers/gpu/drm/xe/xe_guc.c b/drivers/gpu/drm/xe/xe_guc.c > > index 4286bd05c686..4f132bf8539d 100644 > > --- a/drivers/gpu/drm/xe/xe_guc.c > > +++ b/drivers/gpu/drm/xe/xe_guc.c > > @@ -732,6 +732,12 @@ static int xe_guc_realloc_post_hwconfig(struct xe_guc *guc) > > if (ret) > > return ret; Add a newline here. > > + if (guc->ads.um_queue_bo) { > > + ret = xe_managed_bo_reinit_in_vram(xe, tile, &guc->ads.um_queue_bo); > > + if (ret) > > + return ret; > > + } > > + > > return 0; > > } > > diff --git a/drivers/gpu/drm/xe/xe_guc_ads.c b/drivers/gpu/drm/xe/xe_guc_ads.c > > index f0ac00586d3a..ef8e9cc35c8c 100644 > > --- a/drivers/gpu/drm/xe/xe_guc_ads.c > > +++ b/drivers/gpu/drm/xe/xe_guc_ads.c > > @@ -8,6 +8,7 @@ > > #include <linux/fault-inject.h> > > #include <drm/drm_managed.h> > > +#include <drm/drm_prime.h> > > #include <generated/xe_wa_oob.h> > > @@ -64,10 +65,14 @@ ads_to_map(struct xe_guc_ads *ads) > > /* > > * The Additional Data Struct (ADS) has pointers for different buffers used by > > - * the GuC. One single gem object contains the ADS struct itself (guc_ads) and > > - * all the extra buffers indirectly linked via the ADS struct's entries. > > + * the GuC. One gem object (ads->bo) contains the ADS struct itself (guc_ads) > > + * and most of the extra buffers linked via the ADS struct's entries. The UM > > + * fault queues (PAGE_FAULT, PAGE_FAULT_RESPONSE, ACCESS_COUNTER rings) are > > + * kept in a separate BO (ads->um_queue_bo) so that the full memset of ads->bo > > + * performed on every GT reset does not discard fault descriptors already > > + * written into the rings by the GPU. > > * > > - * Layout of the ADS blob allocated for the GuC: > > + * Layout of the ADS blob (ads->bo): > > * > > * +---------------------------------------+ <== base > > * | guc_ads | > > @@ -99,10 +104,6 @@ ads_to_map(struct xe_guc_ads *ads) > > * +---------------------------------------+ > > * | padding | > > * +---------------------------------------+ <== 4K aligned > > - * | UM queues | > > - * +---------------------------------------+ > > - * | padding | > > - * +---------------------------------------+ <== 4K aligned > > * | private data | > > * +---------------------------------------+ > > * | padding | > > @@ -156,16 +157,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); > > @@ -206,22 +197,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); > > } > > @@ -461,6 +442,44 @@ int xe_guc_ads_init(struct xe_guc_ads *ads) > > ads->bo = bo; > > + if (xe->info.has_usm) { > > + /* > > + * Allocate a separate BO for the HW fault ring (UM queues). > > + * > > + * Round the size up to the next power of two so that > > + * (system memory, no IOMMU) the TTM pool issues a single > > + * alloc_pages(order=N) call, maximising the chance of getting > > + * a physically contiguous block. GuC requires contiguous DPA. > > + */ > > + size_t um_size = roundup_pow_of_two(GUC_UM_QUEUE_SIZE * > > + GUC_UM_HW_QUEUE_MAX); > > + > > + bo = xe_managed_bo_create_pin_map(xe, tile, um_size, > > + XE_BO_FLAG_SYSTEM | > > + XE_BO_FLAG_GGTT | > > + XE_BO_FLAG_GGTT_INVALIDATE | > > + XE_BO_FLAG_PINNED_NORESTORE); > > + if (IS_ERR(bo)) > > + return PTR_ERR(bo); > > + > > + /* > > + * On pre-Xe3p platforms, GAM (not GuC) accesses the UM queue > > + * ring via base_dpa, which must be a contiguous DMA address > > + * range. Verify that the allocated pages are contiguous in > > + * DMA address space. > > + */ > > + if (xe_bo_sg(bo) && > > > There is a comment from Sashiko that there might not be a valid tt for vram > buffers, so perhaps just make this check !xe_bo_is_vram() instead? > > > + !xe_guc_using_main_gamctrl_queues(ads_to_guc(ads)) && > > + unlikely(drm_prime_get_contiguous_size(xe_bo_sg(bo)) < Using drm_prime_* really is the wrong layering. Either this helper should be some drm or sg generic or IMO we implement this helper ourselves until we have a proper one in drm or sg generic. e.g., for SG specific BOs, something like this should work. xe_bo_sg_is_contigous(struct xe_bo *bo) { struct xe_res_cursor cur; xe_assert(xe_bo_device(bo), xe_bo_sg(bo)); xe_res_first_sg(xe_bo_sg(bo), 0, xe_bo_size(bo), &cur); return cur.size == xe_bo_size(bo); } Matt > > + GUC_UM_QUEUE_SIZE * GUC_UM_HW_QUEUE_MAX)) { > > + drm_err(&xe->drm, > > + "UM fault queue memory is not contiguous in DMA address space; GAM requires contiguous DPA\n"); > > + return -ENOMEM; > > + } > > + > > + ads->um_queue_bo = bo; > > + } > > + > > return 0; > > } > > ALLOW_ERROR_INJECTION(xe_guc_ads_init, ERRNO); /* See xe_pci_probe() */ > > @@ -893,7 +912,7 @@ 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_bo *um_bo = ads->um_queue_bo; > > struct xe_guc *guc = ads_to_guc(ads); > > struct xe_device *xe = ads_to_xe(ads); > > u64 base_dpa; > > @@ -903,8 +922,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) { > > /* > > diff --git a/drivers/gpu/drm/xe/xe_guc_ads_types.h b/drivers/gpu/drm/xe/xe_guc_ads_types.h > > index 48a8e092023f..845c1fbd93a4 100644 > > --- a/drivers/gpu/drm/xe/xe_guc_ads_types.h > > +++ b/drivers/gpu/drm/xe/xe_guc_ads_types.h > > @@ -16,6 +16,11 @@ struct xe_bo; > > struct xe_guc_ads { > > /** @bo: Xe BO for GuC ads blob */ > > struct xe_bo *bo; > > + /** > > + * @um_queue_bo: Dedicated BO for the HW fault ring (UM queues). > > + * NULL if the platform does not support USM. > > + */ > > + 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 */ > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v7 1/3] drm/xe/guc_ads: allocate UM queues in a separate BO 2026-08-04 8:54 ` Matthew Brost @ 2026-08-04 9:16 ` Matthew Auld 2026-08-04 9:25 ` Matthew Brost 0 siblings, 1 reply; 10+ messages in thread From: Matthew Auld @ 2026-08-04 9:16 UTC (permalink / raw) To: Matthew Brost; +Cc: Jia Yao, intel-xe, Gwan-gyeong Mun, stable On 04/08/2026 09:54, Matthew Brost wrote: > On Tue, Aug 04, 2026 at 09:00:19AM +0100, Matthew Auld wrote: >> On 03/08/2026 21:08, Jia Yao wrote: >>> Move the UM queues into a dedicated UC BO (ads->um_queue_bo) and avoid >>> CPU memset operations on it, which eliminates the CPU as a potential >>> cacheline-polluting agent and helps maintain consistency between GAM >>> writes and GuC reads. >>> >>> Fixes: 9c57bc08652a ("drm/xe/lnl: Drop force_probe requirement") >>> Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> >>> Cc: Matthew Auld <matthew.auld@intel.com> >>> Cc: <stable@vger.kernel.org> # v6.12+ >>> Signed-off-by: Jia Yao <jia.yao@intel.com> >>> --- >>> drivers/gpu/drm/xe/xe_guc.c | 6 ++ >>> drivers/gpu/drm/xe/xe_guc_ads.c | 89 +++++++++++++++++---------- >>> drivers/gpu/drm/xe/xe_guc_ads_types.h | 5 ++ >>> 3 files changed, 68 insertions(+), 32 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/xe/xe_guc.c b/drivers/gpu/drm/xe/xe_guc.c >>> index 4286bd05c686..4f132bf8539d 100644 >>> --- a/drivers/gpu/drm/xe/xe_guc.c >>> +++ b/drivers/gpu/drm/xe/xe_guc.c >>> @@ -732,6 +732,12 @@ static int xe_guc_realloc_post_hwconfig(struct xe_guc *guc) >>> if (ret) >>> return ret; > > Add a newline here. > >>> + if (guc->ads.um_queue_bo) { >>> + ret = xe_managed_bo_reinit_in_vram(xe, tile, &guc->ads.um_queue_bo); >>> + if (ret) >>> + return ret; >>> + } >>> + >>> return 0; >>> } >>> diff --git a/drivers/gpu/drm/xe/xe_guc_ads.c b/drivers/gpu/drm/xe/xe_guc_ads.c >>> index f0ac00586d3a..ef8e9cc35c8c 100644 >>> --- a/drivers/gpu/drm/xe/xe_guc_ads.c >>> +++ b/drivers/gpu/drm/xe/xe_guc_ads.c >>> @@ -8,6 +8,7 @@ >>> #include <linux/fault-inject.h> >>> #include <drm/drm_managed.h> >>> +#include <drm/drm_prime.h> >>> #include <generated/xe_wa_oob.h> >>> @@ -64,10 +65,14 @@ ads_to_map(struct xe_guc_ads *ads) >>> /* >>> * The Additional Data Struct (ADS) has pointers for different buffers used by >>> - * the GuC. One single gem object contains the ADS struct itself (guc_ads) and >>> - * all the extra buffers indirectly linked via the ADS struct's entries. >>> + * the GuC. One gem object (ads->bo) contains the ADS struct itself (guc_ads) >>> + * and most of the extra buffers linked via the ADS struct's entries. The UM >>> + * fault queues (PAGE_FAULT, PAGE_FAULT_RESPONSE, ACCESS_COUNTER rings) are >>> + * kept in a separate BO (ads->um_queue_bo) so that the full memset of ads->bo >>> + * performed on every GT reset does not discard fault descriptors already >>> + * written into the rings by the GPU. >>> * >>> - * Layout of the ADS blob allocated for the GuC: >>> + * Layout of the ADS blob (ads->bo): >>> * >>> * +---------------------------------------+ <== base >>> * | guc_ads | >>> @@ -99,10 +104,6 @@ ads_to_map(struct xe_guc_ads *ads) >>> * +---------------------------------------+ >>> * | padding | >>> * +---------------------------------------+ <== 4K aligned >>> - * | UM queues | >>> - * +---------------------------------------+ >>> - * | padding | >>> - * +---------------------------------------+ <== 4K aligned >>> * | private data | >>> * +---------------------------------------+ >>> * | padding | >>> @@ -156,16 +157,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); >>> @@ -206,22 +197,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); >>> } >>> @@ -461,6 +442,44 @@ int xe_guc_ads_init(struct xe_guc_ads *ads) >>> ads->bo = bo; >>> + if (xe->info.has_usm) { >>> + /* >>> + * Allocate a separate BO for the HW fault ring (UM queues). >>> + * >>> + * Round the size up to the next power of two so that >>> + * (system memory, no IOMMU) the TTM pool issues a single >>> + * alloc_pages(order=N) call, maximising the chance of getting >>> + * a physically contiguous block. GuC requires contiguous DPA. >>> + */ >>> + size_t um_size = roundup_pow_of_two(GUC_UM_QUEUE_SIZE * >>> + GUC_UM_HW_QUEUE_MAX); >>> + >>> + bo = xe_managed_bo_create_pin_map(xe, tile, um_size, >>> + XE_BO_FLAG_SYSTEM | >>> + XE_BO_FLAG_GGTT | >>> + XE_BO_FLAG_GGTT_INVALIDATE | >>> + XE_BO_FLAG_PINNED_NORESTORE); >>> + if (IS_ERR(bo)) >>> + return PTR_ERR(bo); >>> + >>> + /* >>> + * On pre-Xe3p platforms, GAM (not GuC) accesses the UM queue >>> + * ring via base_dpa, which must be a contiguous DMA address >>> + * range. Verify that the allocated pages are contiguous in >>> + * DMA address space. >>> + */ >>> + if (xe_bo_sg(bo) && >> >> >> There is a comment from Sashiko that there might not be a valid tt for vram >> buffers, so perhaps just make this check !xe_bo_is_vram() instead? >> >>> + !xe_guc_using_main_gamctrl_queues(ads_to_guc(ads)) && >>> + unlikely(drm_prime_get_contiguous_size(xe_bo_sg(bo)) < > > Using drm_prime_* really is the wrong layering. Either this helper > should be some drm or sg generic or IMO we implement this helper > ourselves until we have a proper one in drm or sg generic. > > e.g., for SG specific BOs, something like this should work. > > xe_bo_sg_is_contigous(struct xe_bo *bo) > { > struct xe_res_cursor cur; > > xe_assert(xe_bo_device(bo), xe_bo_sg(bo)); > xe_res_first_sg(xe_bo_sg(bo), 0, xe_bo_size(bo), &cur); > > return cur.size == xe_bo_size(bo); > } AFAICT I don't think it's the same since you could potentially get multiple sg entries underneath, where they still occupy something contig as a whole, and xe_res_first_sg() doesn't try to coalesce. Maybe it should? The !xe_bo_sg_is_contigous() will be fatal, so I think we should try as hard as possible for the check. Agreed drm_prime_get_contiguous_size() is a bit odd, but essentially we were previously implementing the same thing. > > Matt > >>> + GUC_UM_QUEUE_SIZE * GUC_UM_HW_QUEUE_MAX)) { >>> + drm_err(&xe->drm, >>> + "UM fault queue memory is not contiguous in DMA address space; GAM requires contiguous DPA\n"); >>> + return -ENOMEM; >>> + } >>> + >>> + ads->um_queue_bo = bo; >>> + } >>> + >>> return 0; >>> } >>> ALLOW_ERROR_INJECTION(xe_guc_ads_init, ERRNO); /* See xe_pci_probe() */ >>> @@ -893,7 +912,7 @@ 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_bo *um_bo = ads->um_queue_bo; >>> struct xe_guc *guc = ads_to_guc(ads); >>> struct xe_device *xe = ads_to_xe(ads); >>> u64 base_dpa; >>> @@ -903,8 +922,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) { >>> /* >>> diff --git a/drivers/gpu/drm/xe/xe_guc_ads_types.h b/drivers/gpu/drm/xe/xe_guc_ads_types.h >>> index 48a8e092023f..845c1fbd93a4 100644 >>> --- a/drivers/gpu/drm/xe/xe_guc_ads_types.h >>> +++ b/drivers/gpu/drm/xe/xe_guc_ads_types.h >>> @@ -16,6 +16,11 @@ struct xe_bo; >>> struct xe_guc_ads { >>> /** @bo: Xe BO for GuC ads blob */ >>> struct xe_bo *bo; >>> + /** >>> + * @um_queue_bo: Dedicated BO for the HW fault ring (UM queues). >>> + * NULL if the platform does not support USM. >>> + */ >>> + 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 */ >> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v7 1/3] drm/xe/guc_ads: allocate UM queues in a separate BO 2026-08-04 9:16 ` Matthew Auld @ 2026-08-04 9:25 ` Matthew Brost 2026-08-04 9:36 ` Matthew Auld 0 siblings, 1 reply; 10+ messages in thread From: Matthew Brost @ 2026-08-04 9:25 UTC (permalink / raw) To: Matthew Auld; +Cc: Jia Yao, intel-xe, Gwan-gyeong Mun, stable On Tue, Aug 04, 2026 at 10:16:07AM +0100, Matthew Auld wrote: > On 04/08/2026 09:54, Matthew Brost wrote: > > On Tue, Aug 04, 2026 at 09:00:19AM +0100, Matthew Auld wrote: > > > On 03/08/2026 21:08, Jia Yao wrote: > > > > Move the UM queues into a dedicated UC BO (ads->um_queue_bo) and avoid > > > > CPU memset operations on it, which eliminates the CPU as a potential > > > > cacheline-polluting agent and helps maintain consistency between GAM > > > > writes and GuC reads. > > > > > > > > Fixes: 9c57bc08652a ("drm/xe/lnl: Drop force_probe requirement") > > > > Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> > > > > Cc: Matthew Auld <matthew.auld@intel.com> > > > > Cc: <stable@vger.kernel.org> # v6.12+ > > > > Signed-off-by: Jia Yao <jia.yao@intel.com> > > > > --- > > > > drivers/gpu/drm/xe/xe_guc.c | 6 ++ > > > > drivers/gpu/drm/xe/xe_guc_ads.c | 89 +++++++++++++++++---------- > > > > drivers/gpu/drm/xe/xe_guc_ads_types.h | 5 ++ > > > > 3 files changed, 68 insertions(+), 32 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/xe/xe_guc.c b/drivers/gpu/drm/xe/xe_guc.c > > > > index 4286bd05c686..4f132bf8539d 100644 > > > > --- a/drivers/gpu/drm/xe/xe_guc.c > > > > +++ b/drivers/gpu/drm/xe/xe_guc.c > > > > @@ -732,6 +732,12 @@ static int xe_guc_realloc_post_hwconfig(struct xe_guc *guc) > > > > if (ret) > > > > return ret; > > > > Add a newline here. > > > > > > + if (guc->ads.um_queue_bo) { > > > > + ret = xe_managed_bo_reinit_in_vram(xe, tile, &guc->ads.um_queue_bo); > > > > + if (ret) > > > > + return ret; > > > > + } > > > > + > > > > return 0; > > > > } > > > > diff --git a/drivers/gpu/drm/xe/xe_guc_ads.c b/drivers/gpu/drm/xe/xe_guc_ads.c > > > > index f0ac00586d3a..ef8e9cc35c8c 100644 > > > > --- a/drivers/gpu/drm/xe/xe_guc_ads.c > > > > +++ b/drivers/gpu/drm/xe/xe_guc_ads.c > > > > @@ -8,6 +8,7 @@ > > > > #include <linux/fault-inject.h> > > > > #include <drm/drm_managed.h> > > > > +#include <drm/drm_prime.h> > > > > #include <generated/xe_wa_oob.h> > > > > @@ -64,10 +65,14 @@ ads_to_map(struct xe_guc_ads *ads) > > > > /* > > > > * The Additional Data Struct (ADS) has pointers for different buffers used by > > > > - * the GuC. One single gem object contains the ADS struct itself (guc_ads) and > > > > - * all the extra buffers indirectly linked via the ADS struct's entries. > > > > + * the GuC. One gem object (ads->bo) contains the ADS struct itself (guc_ads) > > > > + * and most of the extra buffers linked via the ADS struct's entries. The UM > > > > + * fault queues (PAGE_FAULT, PAGE_FAULT_RESPONSE, ACCESS_COUNTER rings) are > > > > + * kept in a separate BO (ads->um_queue_bo) so that the full memset of ads->bo > > > > + * performed on every GT reset does not discard fault descriptors already > > > > + * written into the rings by the GPU. > > > > * > > > > - * Layout of the ADS blob allocated for the GuC: > > > > + * Layout of the ADS blob (ads->bo): > > > > * > > > > * +---------------------------------------+ <== base > > > > * | guc_ads | > > > > @@ -99,10 +104,6 @@ ads_to_map(struct xe_guc_ads *ads) > > > > * +---------------------------------------+ > > > > * | padding | > > > > * +---------------------------------------+ <== 4K aligned > > > > - * | UM queues | > > > > - * +---------------------------------------+ > > > > - * | padding | > > > > - * +---------------------------------------+ <== 4K aligned > > > > * | private data | > > > > * +---------------------------------------+ > > > > * | padding | > > > > @@ -156,16 +157,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); > > > > @@ -206,22 +197,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); > > > > } > > > > @@ -461,6 +442,44 @@ int xe_guc_ads_init(struct xe_guc_ads *ads) > > > > ads->bo = bo; > > > > + if (xe->info.has_usm) { > > > > + /* > > > > + * Allocate a separate BO for the HW fault ring (UM queues). > > > > + * > > > > + * Round the size up to the next power of two so that > > > > + * (system memory, no IOMMU) the TTM pool issues a single > > > > + * alloc_pages(order=N) call, maximising the chance of getting > > > > + * a physically contiguous block. GuC requires contiguous DPA. > > > > + */ > > > > + size_t um_size = roundup_pow_of_two(GUC_UM_QUEUE_SIZE * > > > > + GUC_UM_HW_QUEUE_MAX); > > > > + > > > > + bo = xe_managed_bo_create_pin_map(xe, tile, um_size, > > > > + XE_BO_FLAG_SYSTEM | > > > > + XE_BO_FLAG_GGTT | > > > > + XE_BO_FLAG_GGTT_INVALIDATE | > > > > + XE_BO_FLAG_PINNED_NORESTORE); > > > > + if (IS_ERR(bo)) > > > > + return PTR_ERR(bo); > > > > + > > > > + /* > > > > + * On pre-Xe3p platforms, GAM (not GuC) accesses the UM queue > > > > + * ring via base_dpa, which must be a contiguous DMA address > > > > + * range. Verify that the allocated pages are contiguous in > > > > + * DMA address space. > > > > + */ > > > > + if (xe_bo_sg(bo) && > > > > > > > > > There is a comment from Sashiko that there might not be a valid tt for vram > > > buffers, so perhaps just make this check !xe_bo_is_vram() instead? > > > > > > > + !xe_guc_using_main_gamctrl_queues(ads_to_guc(ads)) && > > > > + unlikely(drm_prime_get_contiguous_size(xe_bo_sg(bo)) < > > > > Using drm_prime_* really is the wrong layering. Either this helper > > should be some drm or sg generic or IMO we implement this helper > > ourselves until we have a proper one in drm or sg generic. > > > > e.g., for SG specific BOs, something like this should work. > > > > xe_bo_sg_is_contigous(struct xe_bo *bo) > > { > > struct xe_res_cursor cur; > > > > xe_assert(xe_bo_device(bo), xe_bo_sg(bo)); > > xe_res_first_sg(xe_bo_sg(bo), 0, xe_bo_size(bo), &cur); > > > > return cur.size == xe_bo_size(bo); > > } > > AFAICT I don't think it's the same since you could potentially get multiple > sg entries underneath, where they still occupy something contig as a whole, Yes, that is possible but SG segment in Xe is 1 or 2G iirc. > and xe_res_first_sg() doesn't try to coalesce. Maybe it should? The As long as segment is at least 1G then it doesn't really matter for existing use cases. > !xe_bo_sg_is_contigous() will be fatal, so I think we should try as hard as > possible for the check. > > Agreed drm_prime_get_contiguous_size() is a bit odd, but essentially we were > previously implementing the same thing. Maybe move this into xe_bo helper to make this slighly more palatable (i.e., at least only BO layer is looking at prime). e.g., xe_bo_sg_is_contigous(bo, len) return drm_prime_get_contiguous_size(xe_bo_sg(bo)) >= len; or xe_bo_sg_is_contigous(bo) return drm_prime_get_contiguous_size(xe_bo_sg(bo)) == xe_bo_size(bo); Matt > > > > > Matt > > > > > > + GUC_UM_QUEUE_SIZE * GUC_UM_HW_QUEUE_MAX)) { > > > > + drm_err(&xe->drm, > > > > + "UM fault queue memory is not contiguous in DMA address space; GAM requires contiguous DPA\n"); > > > > + return -ENOMEM; > > > > + } > > > > + > > > > + ads->um_queue_bo = bo; > > > > + } > > > > + > > > > return 0; > > > > } > > > > ALLOW_ERROR_INJECTION(xe_guc_ads_init, ERRNO); /* See xe_pci_probe() */ > > > > @@ -893,7 +912,7 @@ 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_bo *um_bo = ads->um_queue_bo; > > > > struct xe_guc *guc = ads_to_guc(ads); > > > > struct xe_device *xe = ads_to_xe(ads); > > > > u64 base_dpa; > > > > @@ -903,8 +922,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) { > > > > /* > > > > diff --git a/drivers/gpu/drm/xe/xe_guc_ads_types.h b/drivers/gpu/drm/xe/xe_guc_ads_types.h > > > > index 48a8e092023f..845c1fbd93a4 100644 > > > > --- a/drivers/gpu/drm/xe/xe_guc_ads_types.h > > > > +++ b/drivers/gpu/drm/xe/xe_guc_ads_types.h > > > > @@ -16,6 +16,11 @@ struct xe_bo; > > > > struct xe_guc_ads { > > > > /** @bo: Xe BO for GuC ads blob */ > > > > struct xe_bo *bo; > > > > + /** > > > > + * @um_queue_bo: Dedicated BO for the HW fault ring (UM queues). > > > > + * NULL if the platform does not support USM. > > > > + */ > > > > + 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 */ > > > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v7 1/3] drm/xe/guc_ads: allocate UM queues in a separate BO 2026-08-04 9:25 ` Matthew Brost @ 2026-08-04 9:36 ` Matthew Auld 0 siblings, 0 replies; 10+ messages in thread From: Matthew Auld @ 2026-08-04 9:36 UTC (permalink / raw) To: Matthew Brost; +Cc: Jia Yao, intel-xe, Gwan-gyeong Mun, stable On 04/08/2026 10:25, Matthew Brost wrote: > On Tue, Aug 04, 2026 at 10:16:07AM +0100, Matthew Auld wrote: >> On 04/08/2026 09:54, Matthew Brost wrote: >>> On Tue, Aug 04, 2026 at 09:00:19AM +0100, Matthew Auld wrote: >>>> On 03/08/2026 21:08, Jia Yao wrote: >>>>> Move the UM queues into a dedicated UC BO (ads->um_queue_bo) and avoid >>>>> CPU memset operations on it, which eliminates the CPU as a potential >>>>> cacheline-polluting agent and helps maintain consistency between GAM >>>>> writes and GuC reads. >>>>> >>>>> Fixes: 9c57bc08652a ("drm/xe/lnl: Drop force_probe requirement") >>>>> Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> >>>>> Cc: Matthew Auld <matthew.auld@intel.com> >>>>> Cc: <stable@vger.kernel.org> # v6.12+ >>>>> Signed-off-by: Jia Yao <jia.yao@intel.com> >>>>> --- >>>>> drivers/gpu/drm/xe/xe_guc.c | 6 ++ >>>>> drivers/gpu/drm/xe/xe_guc_ads.c | 89 +++++++++++++++++---------- >>>>> drivers/gpu/drm/xe/xe_guc_ads_types.h | 5 ++ >>>>> 3 files changed, 68 insertions(+), 32 deletions(-) >>>>> >>>>> diff --git a/drivers/gpu/drm/xe/xe_guc.c b/drivers/gpu/drm/xe/xe_guc.c >>>>> index 4286bd05c686..4f132bf8539d 100644 >>>>> --- a/drivers/gpu/drm/xe/xe_guc.c >>>>> +++ b/drivers/gpu/drm/xe/xe_guc.c >>>>> @@ -732,6 +732,12 @@ static int xe_guc_realloc_post_hwconfig(struct xe_guc *guc) >>>>> if (ret) >>>>> return ret; >>> >>> Add a newline here. >>> >>>>> + if (guc->ads.um_queue_bo) { >>>>> + ret = xe_managed_bo_reinit_in_vram(xe, tile, &guc->ads.um_queue_bo); >>>>> + if (ret) >>>>> + return ret; >>>>> + } >>>>> + >>>>> return 0; >>>>> } >>>>> diff --git a/drivers/gpu/drm/xe/xe_guc_ads.c b/drivers/gpu/drm/xe/xe_guc_ads.c >>>>> index f0ac00586d3a..ef8e9cc35c8c 100644 >>>>> --- a/drivers/gpu/drm/xe/xe_guc_ads.c >>>>> +++ b/drivers/gpu/drm/xe/xe_guc_ads.c >>>>> @@ -8,6 +8,7 @@ >>>>> #include <linux/fault-inject.h> >>>>> #include <drm/drm_managed.h> >>>>> +#include <drm/drm_prime.h> >>>>> #include <generated/xe_wa_oob.h> >>>>> @@ -64,10 +65,14 @@ ads_to_map(struct xe_guc_ads *ads) >>>>> /* >>>>> * The Additional Data Struct (ADS) has pointers for different buffers used by >>>>> - * the GuC. One single gem object contains the ADS struct itself (guc_ads) and >>>>> - * all the extra buffers indirectly linked via the ADS struct's entries. >>>>> + * the GuC. One gem object (ads->bo) contains the ADS struct itself (guc_ads) >>>>> + * and most of the extra buffers linked via the ADS struct's entries. The UM >>>>> + * fault queues (PAGE_FAULT, PAGE_FAULT_RESPONSE, ACCESS_COUNTER rings) are >>>>> + * kept in a separate BO (ads->um_queue_bo) so that the full memset of ads->bo >>>>> + * performed on every GT reset does not discard fault descriptors already >>>>> + * written into the rings by the GPU. >>>>> * >>>>> - * Layout of the ADS blob allocated for the GuC: >>>>> + * Layout of the ADS blob (ads->bo): >>>>> * >>>>> * +---------------------------------------+ <== base >>>>> * | guc_ads | >>>>> @@ -99,10 +104,6 @@ ads_to_map(struct xe_guc_ads *ads) >>>>> * +---------------------------------------+ >>>>> * | padding | >>>>> * +---------------------------------------+ <== 4K aligned >>>>> - * | UM queues | >>>>> - * +---------------------------------------+ >>>>> - * | padding | >>>>> - * +---------------------------------------+ <== 4K aligned >>>>> * | private data | >>>>> * +---------------------------------------+ >>>>> * | padding | >>>>> @@ -156,16 +157,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); >>>>> @@ -206,22 +197,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); >>>>> } >>>>> @@ -461,6 +442,44 @@ int xe_guc_ads_init(struct xe_guc_ads *ads) >>>>> ads->bo = bo; >>>>> + if (xe->info.has_usm) { >>>>> + /* >>>>> + * Allocate a separate BO for the HW fault ring (UM queues). >>>>> + * >>>>> + * Round the size up to the next power of two so that >>>>> + * (system memory, no IOMMU) the TTM pool issues a single >>>>> + * alloc_pages(order=N) call, maximising the chance of getting >>>>> + * a physically contiguous block. GuC requires contiguous DPA. >>>>> + */ >>>>> + size_t um_size = roundup_pow_of_two(GUC_UM_QUEUE_SIZE * >>>>> + GUC_UM_HW_QUEUE_MAX); >>>>> + >>>>> + bo = xe_managed_bo_create_pin_map(xe, tile, um_size, >>>>> + XE_BO_FLAG_SYSTEM | >>>>> + XE_BO_FLAG_GGTT | >>>>> + XE_BO_FLAG_GGTT_INVALIDATE | >>>>> + XE_BO_FLAG_PINNED_NORESTORE); >>>>> + if (IS_ERR(bo)) >>>>> + return PTR_ERR(bo); >>>>> + >>>>> + /* >>>>> + * On pre-Xe3p platforms, GAM (not GuC) accesses the UM queue >>>>> + * ring via base_dpa, which must be a contiguous DMA address >>>>> + * range. Verify that the allocated pages are contiguous in >>>>> + * DMA address space. >>>>> + */ >>>>> + if (xe_bo_sg(bo) && >>>> >>>> >>>> There is a comment from Sashiko that there might not be a valid tt for vram >>>> buffers, so perhaps just make this check !xe_bo_is_vram() instead? >>>> >>>>> + !xe_guc_using_main_gamctrl_queues(ads_to_guc(ads)) && >>>>> + unlikely(drm_prime_get_contiguous_size(xe_bo_sg(bo)) < >>> >>> Using drm_prime_* really is the wrong layering. Either this helper >>> should be some drm or sg generic or IMO we implement this helper >>> ourselves until we have a proper one in drm or sg generic. >>> >>> e.g., for SG specific BOs, something like this should work. >>> >>> xe_bo_sg_is_contigous(struct xe_bo *bo) >>> { >>> struct xe_res_cursor cur; >>> >>> xe_assert(xe_bo_device(bo), xe_bo_sg(bo)); >>> xe_res_first_sg(xe_bo_sg(bo), 0, xe_bo_size(bo), &cur); >>> >>> return cur.size == xe_bo_size(bo); >>> } >> >> AFAICT I don't think it's the same since you could potentially get multiple >> sg entries underneath, where they still occupy something contig as a whole, > > Yes, that is possible but SG segment in Xe is 1 or 2G iirc. > >> and xe_res_first_sg() doesn't try to coalesce. Maybe it should? The > > As long as segment is at least 1G then it doesn't really matter for > existing use cases. > >> !xe_bo_sg_is_contigous() will be fatal, so I think we should try as hard as >> possible for the check. >> >> Agreed drm_prime_get_contiguous_size() is a bit odd, but essentially we were >> previously implementing the same thing. > > Maybe move this into xe_bo helper to make this slighly more palatable > (i.e., at least only BO layer is looking at prime). > > e.g., > > xe_bo_sg_is_contigous(bo, len) > return drm_prime_get_contiguous_size(xe_bo_sg(bo)) >= len; +1 Sounds good to me. > > or > > xe_bo_sg_is_contigous(bo) > return drm_prime_get_contiguous_size(xe_bo_sg(bo)) == xe_bo_size(bo); > > Matt > >> >>> >>> Matt >>> >>>>> + GUC_UM_QUEUE_SIZE * GUC_UM_HW_QUEUE_MAX)) { >>>>> + drm_err(&xe->drm, >>>>> + "UM fault queue memory is not contiguous in DMA address space; GAM requires contiguous DPA\n"); >>>>> + return -ENOMEM; >>>>> + } >>>>> + >>>>> + ads->um_queue_bo = bo; >>>>> + } >>>>> + >>>>> return 0; >>>>> } >>>>> ALLOW_ERROR_INJECTION(xe_guc_ads_init, ERRNO); /* See xe_pci_probe() */ >>>>> @@ -893,7 +912,7 @@ 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_bo *um_bo = ads->um_queue_bo; >>>>> struct xe_guc *guc = ads_to_guc(ads); >>>>> struct xe_device *xe = ads_to_xe(ads); >>>>> u64 base_dpa; >>>>> @@ -903,8 +922,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) { >>>>> /* >>>>> diff --git a/drivers/gpu/drm/xe/xe_guc_ads_types.h b/drivers/gpu/drm/xe/xe_guc_ads_types.h >>>>> index 48a8e092023f..845c1fbd93a4 100644 >>>>> --- a/drivers/gpu/drm/xe/xe_guc_ads_types.h >>>>> +++ b/drivers/gpu/drm/xe/xe_guc_ads_types.h >>>>> @@ -16,6 +16,11 @@ struct xe_bo; >>>>> struct xe_guc_ads { >>>>> /** @bo: Xe BO for GuC ads blob */ >>>>> struct xe_bo *bo; >>>>> + /** >>>>> + * @um_queue_bo: Dedicated BO for the HW fault ring (UM queues). >>>>> + * NULL if the platform does not support USM. >>>>> + */ >>>>> + 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 */ >>>> >> ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v7 2/3] drm/xe/guc_ads: allocate UM queues in VRAM on dGFX [not found] <20260803200838.98826-1-jia.yao@intel.com> 2026-08-03 20:08 ` [PATCH v7 1/3] drm/xe/guc_ads: allocate UM queues in a separate BO Jia Yao @ 2026-08-03 20:08 ` Jia Yao 2026-08-04 7:45 ` Matthew Auld 2026-08-03 20:08 ` [PATCH v7 3/3] drm/xe/guc_ads: use uncached mapping for UM queue BO Jia Yao 2 siblings, 1 reply; 10+ messages in thread From: Jia Yao @ 2026-08-03 20:08 UTC (permalink / raw) To: intel-xe; +Cc: Jia Yao, Gwan-gyeong Mun, Matthew Auld, stable On iGPU, the UM queue BO is allocated in system memory. On dGFX, the BO was previously created in system memory and later reallocated in xe_guc_realloc_post_hwconfig(). Allocate the UM queue BO directly in VRAM on dGFX, where it is ultimately required. Fixes: 9c57bc08652a ("drm/xe/lnl: Drop force_probe requirement") Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> Cc: Matthew Auld <matthew.auld@intel.com> Cc: <stable@vger.kernel.org> # v6.12+ Signed-off-by: Jia Yao <jia.yao@intel.com> --- drivers/gpu/drm/xe/xe_guc.c | 6 ------ drivers/gpu/drm/xe/xe_guc_ads.c | 17 ++++++++++------- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_guc.c b/drivers/gpu/drm/xe/xe_guc.c index 4f132bf8539d..4286bd05c686 100644 --- a/drivers/gpu/drm/xe/xe_guc.c +++ b/drivers/gpu/drm/xe/xe_guc.c @@ -732,12 +732,6 @@ static int xe_guc_realloc_post_hwconfig(struct xe_guc *guc) if (ret) return ret; - if (guc->ads.um_queue_bo) { - ret = xe_managed_bo_reinit_in_vram(xe, tile, &guc->ads.um_queue_bo); - if (ret) - return ret; - } - return 0; } diff --git a/drivers/gpu/drm/xe/xe_guc_ads.c b/drivers/gpu/drm/xe/xe_guc_ads.c index ef8e9cc35c8c..a69ec584cc0f 100644 --- a/drivers/gpu/drm/xe/xe_guc_ads.c +++ b/drivers/gpu/drm/xe/xe_guc_ads.c @@ -446,19 +446,22 @@ int xe_guc_ads_init(struct xe_guc_ads *ads) /* * Allocate a separate BO for the HW fault ring (UM queues). * - * Round the size up to the next power of two so that + * Round the size up to the next power of two so that on iGPU * (system memory, no IOMMU) the TTM pool issues a single * alloc_pages(order=N) call, maximising the chance of getting * a physically contiguous block. GuC requires contiguous DPA. */ - size_t um_size = roundup_pow_of_two(GUC_UM_QUEUE_SIZE * + size_t um_size = IS_DGFX(xe) ? + GUC_UM_QUEUE_SIZE * GUC_UM_HW_QUEUE_MAX : + roundup_pow_of_two(GUC_UM_QUEUE_SIZE * GUC_UM_HW_QUEUE_MAX); - bo = xe_managed_bo_create_pin_map(xe, tile, um_size, - XE_BO_FLAG_SYSTEM | - XE_BO_FLAG_GGTT | - XE_BO_FLAG_GGTT_INVALIDATE | - XE_BO_FLAG_PINNED_NORESTORE); + u32 um_flags = XE_BO_FLAG_VRAM_IF_DGFX(tile) | + XE_BO_FLAG_GGTT | + XE_BO_FLAG_GGTT_INVALIDATE | + XE_BO_FLAG_PINNED_NORESTORE; + + bo = xe_managed_bo_create_pin_map(xe, tile, um_size, um_flags); if (IS_ERR(bo)) return PTR_ERR(bo); -- 2.43.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v7 2/3] drm/xe/guc_ads: allocate UM queues in VRAM on dGFX 2026-08-03 20:08 ` [PATCH v7 2/3] drm/xe/guc_ads: allocate UM queues in VRAM on dGFX Jia Yao @ 2026-08-04 7:45 ` Matthew Auld 0 siblings, 0 replies; 10+ messages in thread From: Matthew Auld @ 2026-08-04 7:45 UTC (permalink / raw) To: Jia Yao, intel-xe; +Cc: Gwan-gyeong Mun, stable On 03/08/2026 21:08, Jia Yao wrote: > On iGPU, the UM queue BO is allocated in system memory. On dGFX, the BO > was previously created in system memory and later reallocated in > xe_guc_realloc_post_hwconfig(). Allocate the UM queue BO directly in > VRAM on dGFX, where it is ultimately required. > > Fixes: 9c57bc08652a ("drm/xe/lnl: Drop force_probe requirement") > Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> > Cc: Matthew Auld <matthew.auld@intel.com> > Cc: <stable@vger.kernel.org> # v6.12+ > Signed-off-by: Jia Yao <jia.yao@intel.com> Reviewed-by: Matthew Auld <matthew.auld@intel.com> > --- > drivers/gpu/drm/xe/xe_guc.c | 6 ------ > drivers/gpu/drm/xe/xe_guc_ads.c | 17 ++++++++++------- > 2 files changed, 10 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/xe/xe_guc.c b/drivers/gpu/drm/xe/xe_guc.c > index 4f132bf8539d..4286bd05c686 100644 > --- a/drivers/gpu/drm/xe/xe_guc.c > +++ b/drivers/gpu/drm/xe/xe_guc.c > @@ -732,12 +732,6 @@ static int xe_guc_realloc_post_hwconfig(struct xe_guc *guc) > if (ret) > return ret; > > - if (guc->ads.um_queue_bo) { > - ret = xe_managed_bo_reinit_in_vram(xe, tile, &guc->ads.um_queue_bo); > - if (ret) > - return ret; > - } > - > return 0; > } > > diff --git a/drivers/gpu/drm/xe/xe_guc_ads.c b/drivers/gpu/drm/xe/xe_guc_ads.c > index ef8e9cc35c8c..a69ec584cc0f 100644 > --- a/drivers/gpu/drm/xe/xe_guc_ads.c > +++ b/drivers/gpu/drm/xe/xe_guc_ads.c > @@ -446,19 +446,22 @@ int xe_guc_ads_init(struct xe_guc_ads *ads) > /* > * Allocate a separate BO for the HW fault ring (UM queues). > * > - * Round the size up to the next power of two so that > + * Round the size up to the next power of two so that on iGPU > * (system memory, no IOMMU) the TTM pool issues a single > * alloc_pages(order=N) call, maximising the chance of getting > * a physically contiguous block. GuC requires contiguous DPA. > */ > - size_t um_size = roundup_pow_of_two(GUC_UM_QUEUE_SIZE * > + size_t um_size = IS_DGFX(xe) ? > + GUC_UM_QUEUE_SIZE * GUC_UM_HW_QUEUE_MAX : > + roundup_pow_of_two(GUC_UM_QUEUE_SIZE * > GUC_UM_HW_QUEUE_MAX); > > - bo = xe_managed_bo_create_pin_map(xe, tile, um_size, > - XE_BO_FLAG_SYSTEM | > - XE_BO_FLAG_GGTT | > - XE_BO_FLAG_GGTT_INVALIDATE | > - XE_BO_FLAG_PINNED_NORESTORE); > + u32 um_flags = XE_BO_FLAG_VRAM_IF_DGFX(tile) | > + XE_BO_FLAG_GGTT | > + XE_BO_FLAG_GGTT_INVALIDATE | > + XE_BO_FLAG_PINNED_NORESTORE; > + > + bo = xe_managed_bo_create_pin_map(xe, tile, um_size, um_flags); > if (IS_ERR(bo)) > return PTR_ERR(bo); > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v7 3/3] drm/xe/guc_ads: use uncached mapping for UM queue BO [not found] <20260803200838.98826-1-jia.yao@intel.com> 2026-08-03 20:08 ` [PATCH v7 1/3] drm/xe/guc_ads: allocate UM queues in a separate BO Jia Yao 2026-08-03 20:08 ` [PATCH v7 2/3] drm/xe/guc_ads: allocate UM queues in VRAM on dGFX Jia Yao @ 2026-08-03 20:08 ` Jia Yao 2 siblings, 0 replies; 10+ messages in thread From: Jia Yao @ 2026-08-03 20:08 UTC (permalink / raw) To: intel-xe; +Cc: Jia Yao, Gwan-gyeong Mun, Matthew Auld, stable On Pre-Xe3p platform, the GAM write the UM queue through DPA using UC. if GuC GuC reads the queue via GGTT (WB), stale data may be observed when the cacheline has been polluted by another agent. To match the GAM's UC writes, configure the GuC mapping as UC as well. Fixes: 9c57bc08652a ("drm/xe/lnl: Drop force_probe requirement") Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> Cc: Matthew Auld <matthew.auld@intel.com> Cc: <stable@vger.kernel.org> # v6.12+ Signed-off-by: Jia Yao <jia.yao@intel.com> Reviewed by: Matthew Auld <matthew.auld@intel.com> --- drivers/gpu/drm/xe/xe_guc_ads.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/xe/xe_guc_ads.c b/drivers/gpu/drm/xe/xe_guc_ads.c index a69ec584cc0f..97a2dbb03558 100644 --- a/drivers/gpu/drm/xe/xe_guc_ads.c +++ b/drivers/gpu/drm/xe/xe_guc_ads.c @@ -459,7 +459,8 @@ int xe_guc_ads_init(struct xe_guc_ads *ads) u32 um_flags = XE_BO_FLAG_VRAM_IF_DGFX(tile) | XE_BO_FLAG_GGTT | XE_BO_FLAG_GGTT_INVALIDATE | - XE_BO_FLAG_PINNED_NORESTORE; + XE_BO_FLAG_PINNED_NORESTORE | + XE_BO_FLAG_NEEDS_UC; bo = xe_managed_bo_create_pin_map(xe, tile, um_size, um_flags); if (IS_ERR(bo)) -- 2.43.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-08-04 9:36 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20260803200838.98826-1-jia.yao@intel.com>
2026-08-03 20:08 ` [PATCH v7 1/3] drm/xe/guc_ads: allocate UM queues in a separate BO Jia Yao
2026-08-04 7:43 ` Matthew Auld
2026-08-04 8:00 ` Matthew Auld
2026-08-04 8:54 ` Matthew Brost
2026-08-04 9:16 ` Matthew Auld
2026-08-04 9:25 ` Matthew Brost
2026-08-04 9:36 ` Matthew Auld
2026-08-03 20:08 ` [PATCH v7 2/3] drm/xe/guc_ads: allocate UM queues in VRAM on dGFX Jia Yao
2026-08-04 7:45 ` Matthew Auld
2026-08-03 20:08 ` [PATCH v7 3/3] drm/xe/guc_ads: use uncached mapping for UM queue BO Jia Yao
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).