[PATCH v4] drm/i915/huc: differentiate the 2 steps of the MTL HuC auth flow
Ceraolo Spurio, Daniele
daniele.ceraolospurio at intel.com
Wed May 31 00:14:16 UTC 2023
On 5/30/2023 5:01 PM, John Harrison wrote:
> 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?
leftover from the meu drop. Will reword.
>
>> 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.
I can drop it. I just left it because it was already there.
>
>> 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?
>
Same as the other one, will fix.
Daniele
> 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