Re: [PATCH] drm/i915: Double check we didn't miss an unclaimed register access

[Mika Kuoppala <mika.kuoppala@linux.intel.com>]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Currently, if the user has enabled mmio-debug around each register
> access, we presume that we have then checked them all. However, it is
> still possible through omission (raw register access) or external
> interaction that the unclaimed access was not highlighted.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_uncore.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 20f2f5ad9c3f..05f0cda18501 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -2283,15 +2283,16 @@ bool intel_uncore_unclaimed_mmio(struct drm_i915_private *dev_priv)
>  bool
>  intel_uncore_arm_unclaimed_mmio_detection(struct drm_i915_private *dev_priv)
>  {
> -	if (unlikely(i915_modparams.mmio_debug ||
> -		     dev_priv->uncore.unclaimed_mmio_check <= 0))
> +	if (unlikely(dev_priv->uncore.unclaimed_mmio_check <= 0))
>  		return false;
>

We could catch the readers attention by marking this as READ_ONCE.


And then take spinlock here before checking for unclaimed.

We poke here from hangcheck at unknown intervals and I am
concerned both the trampling on the check values and also
the register access of the unclaimed debug regs.

Which also raises the question that should we just move
the arming check to park/unpark?

-Mika

>  	if (unlikely(intel_uncore_unclaimed_mmio(dev_priv))) {
> -		DRM_DEBUG("Unclaimed register detected, "
> -			  "enabling oneshot unclaimed register reporting. "
> -			  "Please use i915.mmio_debug=N for more information.\n");
> -		i915_modparams.mmio_debug++;
> +		if (!i915_modparams.mmio_debug) {
> +			DRM_DEBUG("Unclaimed register detected, "
> +				  "enabling oneshot unclaimed register reporting. "
> +				  "Please use i915.mmio_debug=N for more information.\n");
> +			i915_modparams.mmio_debug++;
> +		}
>  		dev_priv->uncore.unclaimed_mmio_check--;
>  		return true;
>  	}
> -- 
> 2.19.0.rc1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx