[Intel-gfx] [PATCH v2 5/8] drm/i915/huc: differentiate the 2 steps of the MTL HuC auth flow
Ceraolo Spurio, Daniele
daniele.ceraolospurio at intel.com
Sat May 20 00:17:48 UTC 2023
On 5/19/2023 11:45 AM, John Harrison wrote:
> 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?
Single step authentication always counts as "all workloads". The auth is
partial only if this is a DMA (i.e. non-gsc) load with a gsc-enabled
binary and we're doing an auth via GuC, which is what the condition
above is checking.
>
>> +
>> + 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?
No, this is purely just for our internal debug tracking. The HuC will
still work if just the second stage fails (the status returned to
userspace is based on the HW regs).
>
>> 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'?
I guess it is technically possible, but who's going to provide such
binary? We're only shipping gsc-enabled binaries for GSC platforms.
>
>> + 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)?
intel_huc_is_loaded_by_gsc is only true for DG2, while HAS_ENGINE(gsc)
covers MTL+.
>
> Is it even worth making the engine check a part of the loaded_by_gsc()
> helper?
no, because on MTL we don't load via GSC, we authenticate via it, so we
want loaded_by_gsc to be false there
>
>> + 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?
the aim was to use huc->loaded_via_gsc for internal huc functions and
intel_huc_is_loaded_by_gsc for external ones, but it looks like I didn't
manage it well.
Regarding the loading, the partial auth can only be done if the HuC is
NOT loaded via GSC. The load is done via DMA (same as legacy) and then
we authenticate first via GuC and then via GSC.
Daniele
>
> 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