On Tue, Jan 16, 2024 at 09:56:25AM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> Now that the GGTT PTE updates go straight to GSMBASE (bypassing
> GTTMMADR) there should be no more risk of system hangs? So the
> "binder" (ie. update the PTEs via MI_UPDATE_GTT) is no longer
> necessary, disable it.
>
> My main worry with the MI_UPDATE_GTT are:
> - only used on this one platform so very limited testing coverage
> - async so more opprtunities to screw things up
> - what happens if the engine hangs while we're waiting for MI_UPDATE_GTT
> to finish?
> - requires working command submission, so even getting a working
> display now depends on a lot more extra components working correctly
>
> TODO: MI_UPDATE_GTT might be interesting as an optimization
> though, so perhaps someone should look into always using it
> (assuming the GPU is alive and well)?
>
> v2: Keep using MI_UPDATE_GTT on VM guests
>
> Cc: Paz Zcharya <pazz@xxxxxxxxxxxx>
> Cc: Nirmoy Das <nirmoy.das@xxxxxxxxx>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/gt/intel_gtt.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c b/drivers/gpu/drm/i915/gt/intel_gtt.c
> index 86f73fe558ca..e83dabc56a14 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gtt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gtt.c
> @@ -24,7 +24,8 @@
> bool i915_ggtt_require_binder(struct drm_i915_private *i915)
> {
> /* Wa_13010847436 & Wa_14019519902 */
> - return MEDIA_VER_FULL(i915) == IP_VER(13, 0);
> + return i915_run_as_guest() &&
> + MEDIA_VER_FULL(i915) == IP_VER(13, 0);
Note that i915_run_as_guest() is not the most reliable way to decide
whether to use MI_UPDATE_GTT or straight to GSMBASE, as it requires the
hypervisor to "opt-in" and set the X86_FEATURE_HYPERVISOR.
If it's not set - the driver will go into GSMBASE, which is not mapped
inside the guest.
Does the system firmware advertise whether GSMBASE is "open" or "closed"
to CPU access in any way?
-Michał
> }
>
> static bool intel_ggtt_update_needs_vtd_wa(struct drm_i915_private *i915)
> --
> 2.41.0
>
