[PATCH v4] drm/i915/huc: differentiate the 2 steps of the MTL HuC auth flow

John Harrison john.c.harrison at intel.com
Wed May 31 00:01:15 UTC 2023


On 5/30/2023 08:29, Daniele Ceraolo Spurio wrote:
> Before we add the second step of the MTL HuC auth (via GSC), we need to
> have the ability to differentiate between them. To do so, the huc
> authentication check is duplicated for GuC and GSC auth, with meu
meu?

> binaries being considered fully authenticated only after the GSC auth
> step.
>
> To report the difference between the 2 auth steps, a new case is added
> to the HuC getparam. This way, the clear media driver can start
> submitting before full auth, as partial auth is enough for those
> workloads.
>
> v2: fix authentication status check for DG2
>
> v3: add a better comment at the top of the HuC file to explain the
>      different approaches to load and auth (John)
>
> v4: update call to intel_huc_is_authenticated in the pxp code to check
> for GSC authentication
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Cc: Alan Previn <alan.previn.teres.alexis at intel.com>
> Cc: John Harrison <John.C.Harrison at Intel.com>
> Reviewed-by: Alan Previn <alan.previn.teres.alexis at intel.com> #v2
> ---
>   drivers/gpu/drm/i915/gt/uc/intel_huc.c     | 111 ++++++++++++++++-----
>   drivers/gpu/drm/i915/gt/uc/intel_huc.h     |  16 ++-
>   drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c  |   4 +-
>   drivers/gpu/drm/i915/i915_reg.h            |   3 +
>   drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c |   2 +-
>   include/uapi/drm/i915_drm.h                |   3 +-
>   6 files changed, 104 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> index 37c6a8ca5c71..ab5246ae3979 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> @@ -10,6 +10,7 @@
>   #include "intel_huc.h"
>   #include "intel_huc_print.h"
>   #include "i915_drv.h"
> +#include "i915_reg.h"
>   
>   #include <linux/device/bus.h>
>   #include <linux/mei_aux.h>
> @@ -22,15 +23,23 @@
>    * capabilities by adding HuC specific commands to batch buffers.
>    *
>    * The kernel driver is only responsible for loading the HuC firmware and
> - * triggering its security authentication, which is performed by the GuC on
> - * older platforms and by the GSC on newer ones. For the GuC to correctly
> - * perform the authentication, the HuC binary must be loaded before the GuC one.
> + * triggering its security authentication. This is done differently depending
> + * on the platform:
> + * - older platforms (from Gen9 to most Gen12s): the load is performed via DMA
> + *   and the authentication via GuC
> + * - DG2: load and authentication are both performed via GSC.
> + * - MTL and newer platforms: the load is performed via DMA (same as with
> + *   not-DG2 older platforms), while the authentication is done in 2-steps,
> + *   a first auth for clear-media workloads via GuC and a second one for all
> + *   workloads via GSC.
> + * On platforms where the GuC does the authentication, to correctly do so the
> + * HuC binary must be loaded before the GuC one.
>    * Loading the HuC is optional; however, not using the HuC might negatively
>    * impact power usage and/or performance of media workloads, depending on the
>    * use-cases.
>    * HuC must be reloaded on events that cause the WOPCM to lose its contents
> - * (S3/S4, FLR); GuC-authenticated HuC must also be reloaded on GuC/GT reset,
> - * while GSC-managed HuC will survive that.
> + * (S3/S4, FLR); on older platforms the HuC must also be reloaded on GuC/GT
> + * reset, while on newer ones it will survive that.
>    *
>    * See https://github.com/intel/media-driver for the latest details on HuC
>    * functionality.
> @@ -106,7 +115,7 @@ static enum hrtimer_restart huc_delayed_load_timer_callback(struct hrtimer *hrti
>   {
>   	struct intel_huc *huc = container_of(hrtimer, struct intel_huc, delayed_load.timer);
>   
> -	if (!intel_huc_is_authenticated(huc)) {
> +	if (!intel_huc_is_authenticated(huc, INTEL_HUC_AUTH_BY_GSC)) {
>   		if (huc->delayed_load.status == INTEL_HUC_WAITING_ON_GSC)
>   			huc_notice(huc, "timed out waiting for MEI GSC\n");
>   		else if (huc->delayed_load.status == INTEL_HUC_WAITING_ON_PXP)
> @@ -124,7 +133,7 @@ static void huc_delayed_load_start(struct intel_huc *huc)
>   {
>   	ktime_t delay;
>   
> -	GEM_BUG_ON(intel_huc_is_authenticated(huc));
> +	GEM_BUG_ON(intel_huc_is_authenticated(huc, INTEL_HUC_AUTH_BY_GSC));
>   
>   	/*
>   	 * On resume we don't have to wait for MEI-GSC to be re-probed, but we
> @@ -284,13 +293,23 @@ void intel_huc_init_early(struct intel_huc *huc)
>   	}
>   
>   	if (GRAPHICS_VER(i915) >= 11) {
> -		huc->status.reg = GEN11_HUC_KERNEL_LOAD_INFO;
> -		huc->status.mask = HUC_LOAD_SUCCESSFUL;
> -		huc->status.value = HUC_LOAD_SUCCESSFUL;
> +		huc->status[INTEL_HUC_AUTH_BY_GUC].reg = GEN11_HUC_KERNEL_LOAD_INFO;
> +		huc->status[INTEL_HUC_AUTH_BY_GUC].mask = HUC_LOAD_SUCCESSFUL;
> +		huc->status[INTEL_HUC_AUTH_BY_GUC].value = HUC_LOAD_SUCCESSFUL;
>   	} else {
> -		huc->status.reg = HUC_STATUS2;
> -		huc->status.mask = HUC_FW_VERIFIED;
> -		huc->status.value = HUC_FW_VERIFIED;
> +		huc->status[INTEL_HUC_AUTH_BY_GUC].reg = HUC_STATUS2;
> +		huc->status[INTEL_HUC_AUTH_BY_GUC].mask = HUC_FW_VERIFIED;
> +		huc->status[INTEL_HUC_AUTH_BY_GUC].value = HUC_FW_VERIFIED;
> +	}
> +
> +	if (IS_DG2(i915)) {
> +		huc->status[INTEL_HUC_AUTH_BY_GSC].reg = GEN11_HUC_KERNEL_LOAD_INFO;
> +		huc->status[INTEL_HUC_AUTH_BY_GSC].mask = HUC_LOAD_SUCCESSFUL;
> +		huc->status[INTEL_HUC_AUTH_BY_GSC].value = HUC_LOAD_SUCCESSFUL;
> +	} else {
> +		huc->status[INTEL_HUC_AUTH_BY_GSC].reg = HECI_FWSTS5(MTL_GSC_HECI1_BASE);
> +		huc->status[INTEL_HUC_AUTH_BY_GSC].mask = HECI_FWSTS5_HUC_AUTH_DONE;
> +		huc->status[INTEL_HUC_AUTH_BY_GSC].value = HECI_FWSTS5_HUC_AUTH_DONE;
>   	}
>   }
>   
> @@ -381,28 +400,38 @@ void intel_huc_suspend(struct intel_huc *huc)
>   	delayed_huc_load_complete(huc);
>   }
>   
> -int intel_huc_wait_for_auth_complete(struct intel_huc *huc)
> +static const char *auth_mode_string(struct intel_huc *huc,
> +				    enum intel_huc_authentication_type type)
> +{
> +	bool partial = huc->fw.has_gsc_headers && type == INTEL_HUC_AUTH_BY_GUC;
> +
> +	return partial ? "clear media" : "all workloads";
> +}
> +
> +int intel_huc_wait_for_auth_complete(struct intel_huc *huc,
> +				     enum intel_huc_authentication_type type)
>   {
>   	struct intel_gt *gt = huc_to_gt(huc);
>   	int ret;
>   
>   	ret = __intel_wait_for_register(gt->uncore,
> -					huc->status.reg,
> -					huc->status.mask,
> -					huc->status.value,
> +					huc->status[type].reg,
> +					huc->status[type].mask,
> +					huc->status[type].value,
>   					2, 50, NULL);
>   
>   	/* mark the load process as complete even if the wait failed */
>   	delayed_huc_load_complete(huc);
>   
>   	if (ret) {
> -		huc_err(huc, "firmware not verified %pe\n", ERR_PTR(ret));
> +		huc_err(huc, "firmware not verified for %s: %pe\n",
> +			auth_mode_string(huc, type), ERR_PTR(ret));
>   		intel_uc_fw_change_status(&huc->fw, INTEL_UC_FIRMWARE_LOAD_FAIL);
>   		return ret;
>   	}
>   
>   	intel_uc_fw_change_status(&huc->fw, INTEL_UC_FIRMWARE_RUNNING);
> -	huc_info(huc, "authenticated!\n");
> +	huc_info(huc, "authenticated for %s!\n", auth_mode_string(huc, type));
Not a blocker but should this have an exclamation? It sort of implies 
the authentication is unexpected.

>   	return 0;
>   }
>   
> @@ -442,7 +471,7 @@ int intel_huc_auth(struct intel_huc *huc)
>   	}
>   
>   	/* Check authentication status, it should be done by now */
> -	ret = intel_huc_wait_for_auth_complete(huc);
> +	ret = intel_huc_wait_for_auth_complete(huc, INTEL_HUC_AUTH_BY_GUC);
>   	if (ret)
>   		goto fail;
>   
> @@ -453,16 +482,29 @@ int intel_huc_auth(struct intel_huc *huc)
>   	return ret;
>   }
>   
> -bool intel_huc_is_authenticated(struct intel_huc *huc)
> +bool intel_huc_is_authenticated(struct intel_huc *huc,
> +				enum intel_huc_authentication_type type)
>   {
>   	struct intel_gt *gt = huc_to_gt(huc);
>   	intel_wakeref_t wakeref;
>   	u32 status = 0;
>   
>   	with_intel_runtime_pm(gt->uncore->rpm, wakeref)
> -		status = intel_uncore_read(gt->uncore, huc->status.reg);
> +		status = intel_uncore_read(gt->uncore, huc->status[type].reg);
>   
> -	return (status & huc->status.mask) == huc->status.value;
> +	return (status & huc->status[type].mask) == huc->status[type].value;
> +}
> +
> +static bool huc_is_fully_authenticated(struct intel_huc *huc)
> +{
> +	struct intel_uc_fw *huc_fw = &huc->fw;
> +
> +	if (!huc_fw->has_gsc_headers)
> +		return intel_huc_is_authenticated(huc, INTEL_HUC_AUTH_BY_GUC);
> +	else if (intel_huc_is_loaded_by_gsc(huc) || HAS_ENGINE(huc_to_gt(huc), GSC0))
> +		return intel_huc_is_authenticated(huc, INTEL_HUC_AUTH_BY_GSC);
> +	else
> +		return false;
>   }
>   
>   /**
> @@ -477,7 +519,9 @@ bool intel_huc_is_authenticated(struct intel_huc *huc)
>    */
>   int intel_huc_check_status(struct intel_huc *huc)
>   {
> -	switch (__intel_uc_fw_status(&huc->fw)) {
> +	struct intel_uc_fw *huc_fw = &huc->fw;
> +
> +	switch (__intel_uc_fw_status(huc_fw)) {
>   	case INTEL_UC_FIRMWARE_NOT_SUPPORTED:
>   		return -ENODEV;
>   	case INTEL_UC_FIRMWARE_DISABLED:
> @@ -494,7 +538,17 @@ int intel_huc_check_status(struct intel_huc *huc)
>   		break;
>   	}
>   
> -	return intel_huc_is_authenticated(huc);
> +	/*
> +	 * meu binaries loaded by GuC are first partially authenticated by GuC
meu?

John.


> +	 * and then fully authenticated by GSC
> +	 */
> +	if (huc_is_fully_authenticated(huc))
> +		return 1; /* full auth */
> +	else if (huc_fw->has_gsc_headers && !intel_huc_is_loaded_by_gsc(huc) &&
> +		 intel_huc_is_authenticated(huc, INTEL_HUC_AUTH_BY_GUC))
> +		return 2; /* clear media only */
> +	else
> +		return 0;
>   }
>   
>   static bool huc_has_delayed_load(struct intel_huc *huc)
> @@ -508,7 +562,10 @@ void intel_huc_update_auth_status(struct intel_huc *huc)
>   	if (!intel_uc_fw_is_loadable(&huc->fw))
>   		return;
>   
> -	if (intel_huc_is_authenticated(huc))
> +	if (!huc->fw.has_gsc_headers)
> +		return;
> +
> +	if (huc_is_fully_authenticated(huc))
>   		intel_uc_fw_change_status(&huc->fw,
>   					  INTEL_UC_FIRMWARE_RUNNING);
>   	else if (huc_has_delayed_load(huc))
> @@ -541,5 +598,5 @@ void intel_huc_load_status(struct intel_huc *huc, struct drm_printer *p)
>   
>   	with_intel_runtime_pm(gt->uncore->rpm, wakeref)
>   		drm_printf(p, "HuC status: 0x%08x\n",
> -			   intel_uncore_read(gt->uncore, huc->status.reg));
> +			   intel_uncore_read(gt->uncore, huc->status[INTEL_HUC_AUTH_BY_GUC].reg));
>   }
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.h b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
> index 112f0dce4702..3f6aa7c37abc 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
> @@ -22,6 +22,12 @@ enum intel_huc_delayed_load_status {
>   	INTEL_HUC_DELAYED_LOAD_ERROR,
>   };
>   
> +enum intel_huc_authentication_type {
> +	INTEL_HUC_AUTH_BY_GUC = 0,
> +	INTEL_HUC_AUTH_BY_GSC,
> +	INTEL_HUC_AUTH_MAX_MODES
> +};
> +
>   struct intel_huc {
>   	/* Generic uC firmware management */
>   	struct intel_uc_fw fw;
> @@ -31,7 +37,7 @@ struct intel_huc {
>   		i915_reg_t reg;
>   		u32 mask;
>   		u32 value;
> -	} status;
> +	} status[INTEL_HUC_AUTH_MAX_MODES];
>   
>   	struct {
>   		struct i915_sw_fence fence;
> @@ -49,10 +55,12 @@ int intel_huc_init(struct intel_huc *huc);
>   void intel_huc_fini(struct intel_huc *huc);
>   void intel_huc_suspend(struct intel_huc *huc);
>   int intel_huc_auth(struct intel_huc *huc);
> -int intel_huc_wait_for_auth_complete(struct intel_huc *huc);
> +int intel_huc_wait_for_auth_complete(struct intel_huc *huc,
> +				     enum intel_huc_authentication_type type);
> +bool intel_huc_is_authenticated(struct intel_huc *huc,
> +				enum intel_huc_authentication_type type);
>   int intel_huc_check_status(struct intel_huc *huc);
>   void intel_huc_update_auth_status(struct intel_huc *huc);
> -bool intel_huc_is_authenticated(struct intel_huc *huc);
>   
>   void intel_huc_register_gsc_notifier(struct intel_huc *huc, const struct bus_type *bus);
>   void intel_huc_unregister_gsc_notifier(struct intel_huc *huc, const struct bus_type *bus);
> @@ -81,7 +89,7 @@ static inline bool intel_huc_is_loaded_by_gsc(const struct intel_huc *huc)
>   static inline bool intel_huc_wait_required(struct intel_huc *huc)
>   {
>   	return intel_huc_is_used(huc) && intel_huc_is_loaded_by_gsc(huc) &&
> -	       !intel_huc_is_authenticated(huc);
> +	       !intel_huc_is_authenticated(huc, INTEL_HUC_AUTH_BY_GSC);
>   }
>   
>   void intel_huc_load_status(struct intel_huc *huc, struct drm_printer *p);
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
> index 3355dc1e2bc6..d2b4176c81d6 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
> @@ -160,7 +160,7 @@ int intel_huc_fw_load_and_auth_via_gsc(struct intel_huc *huc)
>   	 * component gets re-bound and this function called again. If so, just
>   	 * mark the HuC as loaded.
>   	 */
> -	if (intel_huc_is_authenticated(huc)) {
> +	if (intel_huc_is_authenticated(huc, INTEL_HUC_AUTH_BY_GSC)) {
>   		intel_uc_fw_change_status(&huc->fw, INTEL_UC_FIRMWARE_RUNNING);
>   		return 0;
>   	}
> @@ -173,7 +173,7 @@ int intel_huc_fw_load_and_auth_via_gsc(struct intel_huc *huc)
>   
>   	intel_uc_fw_change_status(&huc->fw, INTEL_UC_FIRMWARE_TRANSFERRED);
>   
> -	return intel_huc_wait_for_auth_complete(huc);
> +	return intel_huc_wait_for_auth_complete(huc, INTEL_HUC_AUTH_BY_GSC);
>   }
>   
>   /**
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 0523418129c5..c14433795c91 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -941,6 +941,9 @@
>   #define HECI_H_GS1(base)	_MMIO((base) + 0xc4c)
>   #define   HECI_H_GS1_ER_PREP	REG_BIT(0)
>   
> +#define HECI_FWSTS5(base)		_MMIO(base + 0xc68)
> +#define   HECI_FWSTS5_HUC_AUTH_DONE	(1 << 19)
> +
>   #define HSW_GTT_CACHE_EN	_MMIO(0x4024)
>   #define   GTT_CACHE_EN_ALL	0xF0007FFF
>   #define GEN7_WR_WATERMARK	_MMIO(0x4028)
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c b/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c
> index 8dc41de3f6f7..016bd8fad89d 100644
> --- a/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c
> @@ -196,7 +196,7 @@ bool intel_pxp_gsccs_is_ready_for_sessions(struct intel_pxp *pxp)
>   	 * gsc-proxy init flow (the last set of dependencies that
>   	 * are out of order) will suffice.
>   	 */
> -	if (intel_huc_is_authenticated(&pxp->ctrl_gt->uc.huc) &&
> +	if (intel_huc_is_authenticated(&pxp->ctrl_gt->uc.huc, INTEL_HUC_AUTH_BY_GSC) &&
>   	    intel_gsc_uc_fw_proxy_init_done(&pxp->ctrl_gt->uc.gsc))
>   		return true;
>   
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index f31dfacde601..a1848e806059 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -674,7 +674,8 @@ typedef struct drm_i915_irq_wait {
>    * If the IOCTL is successful, the returned parameter will be set to one of the
>    * following values:
>    *  * 0 if HuC firmware load is not complete,
> - *  * 1 if HuC firmware is authenticated and running.
> + *  * 1 if HuC firmware is loaded and fully authenticated,
> + *  * 2 if HuC firmware is loaded and authenticated for clear media only
>    */
>   #define I915_PARAM_HUC_STATUS		 42
>   



More information about the dri-devel mailing list