Hello, On 5/27/26 16:45, Rodrigo Vivi wrote: > The PF GGTT allocator was initialised over a relative [0, usable_size) > range, with ggtt->start added on every address conversion to get the > actual hardware address. Two consequences of that model were considered > "horrible hacks": > > - ggtt->start (the WOPCM offset) had to be carried around and added > to every drm_mm result. This is still the case, only we add a separate variable now that is usually 0. We always have to ensure that this is correctly accounted for, otherwise we get subtle bugs, so there's no reason not to use it, if it's already used everywhere.
Whether it's 0 or wopcm_start doesn't change the result. > - The GUC_GGTT_TOP ceiling silently truncated the GGTT range instead > of being made explicit, leaving PTEs in [GUC_GGTT_TOP, total_size) > untouched during the initial clear. This is a valid concern, but it's very easily be fixed inside the initial clear by calling xe_ggtt_clear once more with a comment for !VF case. Something like below is enough to fix it. Adding a reserved region at the end/start is not the way to do so and just adds more complications. ---8<--- When initialising GGTT, we never clear the part above GUC_GGTT_TOP. Add a member to &xe_ggtt that holds the full GGTT size, and clear it during the pass at xe_ggtt_initial_clear. Reported-by: Ville Syrjälä <[email protected]> Signed-off-by: Maarten Lankhorst <[email protected]> --- diff --git a/drivers/gpu/drm/xe/xe_ggtt.c b/drivers/gpu/drm/xe/xe_ggtt.c index 8ec23862477fc..497b99a932f0d 100644 --- a/drivers/gpu/drm/xe/xe_ggtt.c +++ b/drivers/gpu/drm/xe/xe_ggtt.c @@ -115,6 +115,8 @@ struct xe_ggtt { u64 start; /** @size: Total usable size of this GGTT */ u64 size; + /** @entire_size: complete size of the accessible GGTT, reserved regions inclusive */ + u64 entire_size; /** * @flags: Flags for this GGTT. * Acceptable flags: @@ -406,7 +408,8 @@ int xe_ggtt_init_early(struct xe_ggtt *ggtt) return -ENOMEM; } ggtt_start = wopcm; - ggtt_size = (gsm_size / 8) * (u64)XE_PAGE_SIZE - ggtt_start; + ggtt->entire_size = (gsm_size / 8) * (u64)XE_PAGE_SIZE; + ggtt_size = ggtt->entire_size - ggtt_start; } else { ggtt_start = xe_tile_sriov_vf_ggtt_base(ggtt->tile); ggtt_size = xe_tile_sriov_vf_ggtt(ggtt->tile); @@ -417,6 +420,7 @@ int xe_ggtt_init_early(struct xe_ggtt *ggtt) ggtt_start, ggtt_start + ggtt_size - 1); return -ERANGE; } + ggtt->entire_size = ggtt_size; } ggtt->gsm = ggtt->tile->mmio.regs + SZ_8M; @@ -461,6 +465,10 @@ static void xe_ggtt_initial_clear(struct xe_ggtt *ggtt) drm_mm_for_each_hole(hole, &ggtt->mm, start, end) xe_ggtt_clear(ggtt, ggtt->start + start, end - start); + end = ggtt->start + ggtt->size; + if (ggtt->entire_size > end) + xe_ggtt_clear(ggtt, end, ggtt->entire_size - end); + xe_ggtt_invalidate(ggtt); mutex_unlock(&ggtt->lock); }
