[Intel-gfx] [PATCH 5/6] drm/i915/huc: Support HuC authentication

[Antoine, Peter]

-----Original Message-----
From: Gordon, David S 
Sent: Tuesday, June 28, 2016 4:08 PM
To: Antoine, Peter <peter.antoine at intel.com>; intel-gfx at lists.freedesktop.org
Cc: Prigent, Christophe <christophe.prigent at intel.com>; Kelley, Sean V <sean.v.kelley at intel.com>; Li, Lawrence T <lawrence.t.li at intel.com>; Vivi, Rodrigo <rodrigo.vivi at intel.com>; Dai, Yu <yu.dai at intel.com>
Subject: Re: [PATCH 5/6] drm/i915/huc: Support HuC authentication

On 21/06/16 19:11, Peter Antoine wrote:
> The HuC authentication is done by host2guc call. The HuC RSA keys are 
> sent to GuC for authentication.
>
> Signed-off-by: Alex Dai <yu.dai at intel.com>
> Signed-off-by: Peter Antoine <peter.antoine at intel.com>
> ---
>   drivers/gpu/drm/i915/i915_guc_submission.c | 65 ++++++++++++++++++++++++++++++
>   drivers/gpu/drm/i915/intel_guc_fwif.h      |  1 +
>   drivers/gpu/drm/i915/intel_guc_loader.c    |  2 +
>   drivers/gpu/drm/i915/intel_huc_loader.c    |  5 +++
>   4 files changed, 73 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c 
> b/drivers/gpu/drm/i915/i915_guc_submission.c
> index bfb8400..41f3a42 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -25,6 +25,7 @@
>   #include <linux/circ_buf.h>
>   #include "i915_drv.h"
>   #include "intel_guc.h"
> +#include "intel_huc.h"
>
>   /**
>    * DOC: GuC-based command submission @@ -1077,3 +1078,67 @@ int 
> intel_guc_resume(struct drm_device *dev)
>
>   	return host2guc_action(guc, data, ARRAY_SIZE(data));
>   }
> +
> +/**
> + * intel_huc_auth() - authenticate ucode
> + * @dev: the drm device
> + *
> + * Triggers a HuC fw authentication request to the GuC via host-2-guc
> + * interface.
> + */
> +void intel_huc_auth(struct drm_device *dev) {
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_guc *guc = &dev_priv->guc;
> +	struct intel_huc *huc = &dev_priv->huc;
> +	int ret;
> +	u32 data[2];
> +
> +	/* Bypass the case where there is no HuC firmware */
> +	if (huc->huc_fw.fetch_status == UC_FIRMWARE_NONE ||
> +	    huc->huc_fw.load_status == UC_FIRMWARE_NONE)
> +		return;
> +
> +	if (guc->guc_fw.load_status != UC_FIRMWARE_SUCCESS) {
> +		DRM_ERROR("HuC: GuC fw wasn't loaded. Can't authenticate");
> +		return;
> +	}
> +
> +	if (huc->huc_fw.load_status != UC_FIRMWARE_SUCCESS) {
> +		DRM_ERROR("HuC: fw wasn't loaded. Nothing to authenticate");
> +		return;
> +	}
> +
> +	ret = i915_gem_obj_ggtt_pin(huc->huc_fw.uc_fw_obj, 0, 0);
> +	if (ret) {
> +		DRM_ERROR("HuC: Pin failed");
> +		return;
> +	}
> +
> +	/* Invalidate GuC TLB to let GuC take the latest updates to GTT. */
> +	I915_WRITE(GEN8_GTCR, GEN8_GTCR_INVALIDATE);
> +
> +	/* Specify auth action and where public signature is. It's stored
> +	 * at the beginning of the gem object, before the fw bits
> +	 */
> +	data[0] = HOST2GUC_ACTION_AUTHENTICATE_HUC;
> +	data[1] = i915_gem_obj_ggtt_offset(huc->huc_fw.uc_fw_obj) +
> +			huc->huc_fw.rsa_offset;
> +
> +	ret = host2guc_action(guc, data, ARRAY_SIZE(data));
> +	if (ret) {
> +		DRM_ERROR("HuC: GuC did not ack Auth request\n");
> +		goto out;
> +	}
> +
> +	/* Check authentication status, it should be done by now */
> +	ret = wait_for_atomic(
> +		(I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED) > 0, 50);
> +	if (ret) {
> +		DRM_ERROR("HuC: Authentication failed\n");
> +		goto out;
> +	}
> +
> +out:
> +	i915_gem_object_ggtt_unpin(huc->huc_fw.uc_fw_obj);
> +}
> diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h 
> b/drivers/gpu/drm/i915/intel_guc_fwif.h
> index a69ee36..c5a6227 100644
> --- a/drivers/gpu/drm/i915/intel_guc_fwif.h
> +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
> @@ -437,6 +437,7 @@ enum host2guc_action {
>   	HOST2GUC_ACTION_ENTER_S_STATE = 0x501,
>   	HOST2GUC_ACTION_EXIT_S_STATE = 0x502,
>   	HOST2GUC_ACTION_SLPC_REQUEST = 0x3003,
> +	HOST2GUC_ACTION_AUTHENTICATE_HUC = 0x4000,
>   	HOST2GUC_ACTION_LIMIT
>   };
>
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c 
> b/drivers/gpu/drm/i915/intel_guc_loader.c
> index c4a210d..e876a23f 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -500,6 +500,8 @@ int intel_guc_setup(struct drm_device *dev)
>   		intel_uc_fw_status_repr(guc_fw->fetch_status),
>   		intel_uc_fw_status_repr(guc_fw->load_status));
>
> +	intel_huc_auth(dev);
> +
>   	if (i915.enable_guc_submission) {
>   		err = i915_guc_submission_enable(dev_priv);
>   		if (err)
> diff --git a/drivers/gpu/drm/i915/intel_huc_loader.c 
> b/drivers/gpu/drm/i915/intel_huc_loader.c
> index 472fabe..7205e9e 100644
> --- a/drivers/gpu/drm/i915/intel_huc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_huc_loader.c
> @@ -86,6 +86,11 @@ static int huc_ucode_xfer(struct drm_i915_private *dev_priv)
>   	WARN_ON(!mutex_is_locked(&dev_priv->dev->struct_mutex));
>   	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
>
> +	/* init WOPCM */
> +	I915_WRITE(GUC_WOPCM_SIZE, guc_wopcm_size(dev_priv->dev));

Didn't we already do this somewhere else, in one of the earlier patches?

Otherwise looks OK.

.Dave.

In the GuC loader it also sets this up. But, as we are using a shared function to get the size and the registers is write once, if the HuC is not supported (or there is no firmware) that wont get called but this will. It (I think) makes the code more robust and with the retry and reset code, it is called in the required functions and does not need conditionals around it.

> +	I915_WRITE(DMA_GUC_WOPCM_OFFSET, GUC_WOPCM_OFFSET_VALUE |
> +			HUC_LOADING_AGENT_GUC);
> +
>   	/* Set the source address for the uCode */
>   	offset = i915_gem_obj_ggtt_offset(huc_fw->uc_fw_obj) +
>   			huc_fw->header_offset;