[Intel-gfx] [PATCH v2 15/22] drm/i915/huc: New HuC status register for Gen11

[Daniele Ceraolo Spurio]

On 4/11/19 1:44 AM, Michal Wajdeczko wrote:
> Gen11 defines new register for checking HuC authentication status.
> Look into the right register and bit.
> 
> BSpec: 19686
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> Cc: Tony Ye <tony.ye at intel.com>
> Cc: Vinay Belgaumkar <vinay.belgaumkar at intel.com>
> Cc: John Spotswood <john.a.spotswood at intel.com>
> Cc: Anusha Srivatsa <anusha.srivatsa at intel.com>
> ---
>   drivers/gpu/drm/i915/intel_guc_reg.h |  3 ++
>   drivers/gpu/drm/i915/intel_huc.c     | 56 ++++++++++++++++++++++++----
>   2 files changed, 51 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_reg.h b/drivers/gpu/drm/i915/intel_guc_reg.h
> index d26de5193568..7eba65795b58 100644
> --- a/drivers/gpu/drm/i915/intel_guc_reg.h
> +++ b/drivers/gpu/drm/i915/intel_guc_reg.h
> @@ -79,6 +79,9 @@
>   #define HUC_STATUS2             _MMIO(0xD3B0)
>   #define   HUC_FW_VERIFIED       (1<<7)
>   
> +#define GEN11_HUC_KERNEL_LOAD_INFO	_MMIO(0xC1DC)
> +#define   HUC_LOAD_SUCCESSFUL		  (1 << 0)
> +
>   #define GUC_WOPCM_SIZE			_MMIO(0xc050)
>   #define   GUC_WOPCM_SIZE_LOCKED		  (1<<0)
>   #define   GUC_WOPCM_SIZE_SHIFT		12
> diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
> index 94c04f16a2ad..708a4b387259 100644
> --- a/drivers/gpu/drm/i915/intel_huc.c
> +++ b/drivers/gpu/drm/i915/intel_huc.c
> @@ -40,6 +40,47 @@ int intel_huc_init_misc(struct intel_huc *huc)
>   	return 0;
>   }
>   
> +static int gen8_huc_wait_verified(struct intel_huc *huc)

why gen8?

> +{
> +	struct drm_i915_private *i915 = huc_to_i915(huc);
> +	u32 status;
> +	int ret;
> +
> +	ret = __intel_wait_for_register(&i915->uncore,
> +					HUC_STATUS2,
> +					HUC_FW_VERIFIED,
> +					HUC_FW_VERIFIED,
> +					2, 50, &status);
> +	if (ret)
> +		DRM_ERROR("HuC: status %#x\n", status);
> +	return ret;
> +}
> +
> +static int gen11_huc_wait_verified(struct intel_huc *huc)
> +{
> +	struct drm_i915_private *i915 = huc_to_i915(huc);
> +	int ret;
> +
> +	ret = __intel_wait_for_register(&i915->uncore,
> +					GEN11_HUC_KERNEL_LOAD_INFO,
> +					HUC_LOAD_SUCCESSFUL,
> +					HUC_LOAD_SUCCESSFUL,
> +					2, 50, NULL);
> +	return ret;
> +}
> +
> +static int huc_wait_verified(struct intel_huc *huc)

We do call this only once, so maybe we can just avoid having a separate 
function and just have it directly in intel_huc_auth? the code is simple 
enough. Otherwise, to avoid 2 identical functions which diff only in the 
register details, we could save the register and the expected value in 
the huc struct during init_early and just wait on (huc->auth.reg & 
huc->auth.mask), which we could also use in intel_huc_check_status().

Apart from this, register values do match the FW and the specs.

Daniele

> +{
> +	struct drm_i915_private *i915 = huc_to_i915(huc);
> +	int ret;
> +
> +	if (INTEL_GEN(i915) >= 11)
> +		ret = gen11_huc_wait_verified(huc);
> +	else
> +		ret = gen8_huc_wait_verified(huc);
> +	return ret;
> +}
> +
>   /**
>    * intel_huc_auth() - Authenticate HuC uCode
>    * @huc: intel_huc structure
> @@ -56,7 +97,6 @@ int intel_huc_auth(struct intel_huc *huc)
>   	struct drm_i915_private *i915 = huc_to_i915(huc);
>   	struct intel_guc *guc = &i915->guc;
>   	struct i915_vma *vma;
> -	u32 status;
>   	int ret;
>   
>   	if (huc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
> @@ -79,13 +119,9 @@ int intel_huc_auth(struct intel_huc *huc)
>   	}
>   
>   	/* Check authentication status, it should be done by now */
> -	ret = __intel_wait_for_register(&i915->uncore,
> -					HUC_STATUS2,
> -					HUC_FW_VERIFIED,
> -					HUC_FW_VERIFIED,
> -					2, 50, &status);
> +	ret = huc_wait_verified(huc);
>   	if (ret) {
> -		DRM_ERROR("HuC: Firmware not verified %#x\n", status);
> +		DRM_ERROR("HuC: Firmware not verified %d\n", ret);
>   		goto fail_unpin;
>   	}
>   
> @@ -122,7 +158,11 @@ int intel_huc_check_status(struct intel_huc *huc)
>   		return -ENODEV;
>   
>   	with_intel_runtime_pm(dev_priv, wakeref)
> -		status = I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED;
> +		if (INTEL_GEN(dev_priv) >= 11)
> +			status = I915_READ(GEN11_HUC_KERNEL_LOAD_INFO) &
> +				HUC_LOAD_SUCCESSFUL;
> +		else
> +			status = I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED;
>   
>   	return status;
>   }
>