[Intel-gfx] [PATCH v2 5/8] drm/i915/huc: differentiate the 2 steps of the MTL HuC auth flow
John Harrison
john.c.harrison at intel.com
Fri May 19 18:45:37 UTC 2023
On 4/28/2023 11:58, 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
> 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
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Cc: Alan Previn <alan.previn.teres.alexis at intel.com>
> ---
> drivers/gpu/drm/i915/gt/uc/intel_huc.c | 94 +++++++++++++++++------
> 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 +
> include/uapi/drm/i915_drm.h | 3 +-
> 5 files changed, 91 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> index c189ede4ef55..60f95d98e5fd 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>
> @@ -106,7 +107,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 +125,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 +285,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 +392,39 @@ 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->loaded_via_gsc && huc->fw.is_meu_binary &&
> + type == INTEL_HUC_AUTH_BY_GUC;
partial = !loaded_via_gsc?
If it is not a GSC load then there is no two stage authentication, is
there? Does that mean the single stage auth does not count as 'all
workloads' even on platforms where two stage is not supported?
> +
> + 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);
This means that if stage one works but stage two fails, we mark the HuC
as dead? So the previously working clear media support now stops working?
> 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));
> return 0;
> }
>
> @@ -442,7 +464,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 +475,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->is_meu_binary)
> + return intel_huc_is_authenticated(huc, INTEL_HUC_AUTH_BY_GUC);
Is it not possible to load a non-MEU firmware on a GSC system? Don't we
just treat that as the legacy part within the MEU blob and load it for
clear media only via GuC auth? In which case, it is as 'fully
authenticated' as it is possible to be but it still can't process
protected media. So it should not count as 'fully authenticated'?
> + else if (intel_huc_is_loaded_by_gsc(huc) || HAS_ENGINE(huc_to_gt(huc), GSC0))
Why not &&? Is it possible to have loaded_by_gsc != HAS_ENGINE(gsc)?
Is it even worth making the engine check a part of the loaded_by_gsc()
helper?
> + return intel_huc_is_authenticated(huc, INTEL_HUC_AUTH_BY_GSC);
> + else
> + return false;
> }
>
> /**
> @@ -477,7 +512,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 +531,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
> + * and then fully authenticated by GSC
> + */
> + if (huc_is_fully_authenticated(huc))
> + return 1; /* full auth */
> + else if (huc_fw->is_meu_binary && !huc->loaded_via_gsc &&
Why sometimes use the huc->loaded_via_gsc directly and sometimes use the
intel_huc_is_loaded_by_gsc() helper? Either way, I'm confused - isn't
that backwards? If the HuC is not loaded via GSC then it can't do two
stage auth and therefore cannot be partial? It is either fully
authenticated or not at all?
John.
> + 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 +555,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.is_meu_binary)
> + 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 +591,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 345e1b9aa062..adb063cd27a0 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, struct bus_type *bus);
> void intel_huc_unregister_gsc_notifier(struct intel_huc *huc, 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 88ad2c322c4a..3b8edaba97d2 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
> @@ -141,7 +141,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;
> }
> @@ -154,7 +154,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 dde6e91055bd..1a62e73c3d9c 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/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index dba7c5a5b25e..43b8de42a94e 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -659,7 +659,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