* [Intel-gfx] [PATCH 0/4] drm/i915: Prepare for GSC-loaded HuC
@ 2022-04-27 0:26 ` Daniele Ceraolo Spurio
0 siblings, 0 replies; 22+ messages in thread
From: Daniele Ceraolo Spurio @ 2022-04-27 0:26 UTC (permalink / raw)
To: intel-gfx; +Cc: Alan Previn, dri-devel
On newer platforms (starting DG2 G10 B-step and G11 A-step), ownership of
HuC loading and authentication has been moved from the GuC to the GSC, with
both actions being performed via a single PXP command.
Given that the mei code has not fully landed yet (see [1]), we can't
implement the new load mechanism, but we can start getting ready for it
by taking care of the changes required for the existing code:
- The HuC header is now different from the GuC one. This also means that
if the FW is for GSC-loading and the HW fuse is set to legacy load (or
vice-versa) we can't load the HuC.
- To send a PXP message to the GSC we need both MEI_GSC and MEI_PXP.
- All legacy HuC loading paths can be skipped.
Note that the HuC fw version for DG2 is still not defined, so the HuC
code will be skipped until the define is added.
[1] https://patchwork.freedesktop.org/series/102339/
Cc: Alan Previn <alan.previn.teres.alexis@intel.com>
Cc: John Harrison <john.c.harrison@intel.com>
Daniele Ceraolo Spurio (4):
drm/i915/huc: check HW directly for HuC auth status
drm/i915/huc: Add fetch support for gsc-loaded HuC binary
drm/i915/huc: Prepare for GSC-loaded HuC
drm/i915/huc: Don't fail the probe if HuC init fails
drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h | 1 +
drivers/gpu/drm/i915/gt/uc/intel_huc.c | 97 +++++++++++++++----
drivers/gpu/drm/i915/gt/uc/intel_huc.h | 5 +-
drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c | 5 +-
drivers/gpu/drm/i915/gt/uc/intel_uc.c | 22 +++--
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 99 ++++++++++++--------
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h | 2 +
drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h | 9 ++
8 files changed, 172 insertions(+), 68 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 1/4] drm/i915/huc: check HW directly for HuC auth status
2022-04-27 0:26 ` [Intel-gfx] " Daniele Ceraolo Spurio
@ 2022-04-27 0:26 ` Daniele Ceraolo Spurio
-1 siblings, 0 replies; 22+ messages in thread
From: Daniele Ceraolo Spurio @ 2022-04-27 0:26 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniele Ceraolo Spurio, dri-devel
The huc_is_authenticated function return is based on our SW tracking of
the HuC auth status. However, around suspend/resume and reset this can
go out of sync with the actual HW state, which is why we use
huc_check_state() to look at the actual HW state. Instead of having this
duality, just make huc_is_authenticated() return the HW state and use it
everywhere we need to know if HuC is running.
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
drivers/gpu/drm/i915/gt/uc/intel_huc.c | 23 ++++++++++++++---------
drivers/gpu/drm/i915/gt/uc/intel_huc.h | 5 -----
2 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
index 556829de9c172..773020e69589a 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
@@ -80,6 +80,18 @@ void intel_huc_fini(struct intel_huc *huc)
intel_uc_fw_fini(&huc->fw);
}
+static bool huc_is_authenticated(struct intel_huc *huc)
+{
+ 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);
+
+ return (status & huc->status.mask) == huc->status.value;
+}
+
/**
* intel_huc_auth() - Authenticate HuC uCode
* @huc: intel_huc structure
@@ -96,7 +108,7 @@ int intel_huc_auth(struct intel_huc *huc)
struct intel_guc *guc = >->uc.guc;
int ret;
- GEM_BUG_ON(intel_huc_is_authenticated(huc));
+ GEM_BUG_ON(huc_is_authenticated(huc));
if (!intel_uc_fw_is_loaded(&huc->fw))
return -ENOEXEC;
@@ -150,10 +162,6 @@ int intel_huc_auth(struct intel_huc *huc)
*/
int intel_huc_check_status(struct intel_huc *huc)
{
- struct intel_gt *gt = huc_to_gt(huc);
- intel_wakeref_t wakeref;
- u32 status = 0;
-
switch (__intel_uc_fw_status(&huc->fw)) {
case INTEL_UC_FIRMWARE_NOT_SUPPORTED:
return -ENODEV;
@@ -167,10 +175,7 @@ int intel_huc_check_status(struct intel_huc *huc)
break;
}
- with_intel_runtime_pm(gt->uncore->rpm, wakeref)
- status = intel_uncore_read(gt->uncore, huc->status.reg);
-
- return (status & huc->status.mask) == huc->status.value;
+ return huc_is_authenticated(huc);
}
/**
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.h b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
index 73ec670800f2b..77d813840d76c 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
@@ -50,11 +50,6 @@ static inline bool intel_huc_is_used(struct intel_huc *huc)
return intel_uc_fw_is_available(&huc->fw);
}
-static inline bool intel_huc_is_authenticated(struct intel_huc *huc)
-{
- return intel_uc_fw_is_running(&huc->fw);
-}
-
void intel_huc_load_status(struct intel_huc *huc, struct drm_printer *p);
#endif
--
2.25.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [Intel-gfx] [PATCH 1/4] drm/i915/huc: check HW directly for HuC auth status
@ 2022-04-27 0:26 ` Daniele Ceraolo Spurio
0 siblings, 0 replies; 22+ messages in thread
From: Daniele Ceraolo Spurio @ 2022-04-27 0:26 UTC (permalink / raw)
To: intel-gfx; +Cc: dri-devel
The huc_is_authenticated function return is based on our SW tracking of
the HuC auth status. However, around suspend/resume and reset this can
go out of sync with the actual HW state, which is why we use
huc_check_state() to look at the actual HW state. Instead of having this
duality, just make huc_is_authenticated() return the HW state and use it
everywhere we need to know if HuC is running.
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
drivers/gpu/drm/i915/gt/uc/intel_huc.c | 23 ++++++++++++++---------
drivers/gpu/drm/i915/gt/uc/intel_huc.h | 5 -----
2 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
index 556829de9c172..773020e69589a 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
@@ -80,6 +80,18 @@ void intel_huc_fini(struct intel_huc *huc)
intel_uc_fw_fini(&huc->fw);
}
+static bool huc_is_authenticated(struct intel_huc *huc)
+{
+ 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);
+
+ return (status & huc->status.mask) == huc->status.value;
+}
+
/**
* intel_huc_auth() - Authenticate HuC uCode
* @huc: intel_huc structure
@@ -96,7 +108,7 @@ int intel_huc_auth(struct intel_huc *huc)
struct intel_guc *guc = >->uc.guc;
int ret;
- GEM_BUG_ON(intel_huc_is_authenticated(huc));
+ GEM_BUG_ON(huc_is_authenticated(huc));
if (!intel_uc_fw_is_loaded(&huc->fw))
return -ENOEXEC;
@@ -150,10 +162,6 @@ int intel_huc_auth(struct intel_huc *huc)
*/
int intel_huc_check_status(struct intel_huc *huc)
{
- struct intel_gt *gt = huc_to_gt(huc);
- intel_wakeref_t wakeref;
- u32 status = 0;
-
switch (__intel_uc_fw_status(&huc->fw)) {
case INTEL_UC_FIRMWARE_NOT_SUPPORTED:
return -ENODEV;
@@ -167,10 +175,7 @@ int intel_huc_check_status(struct intel_huc *huc)
break;
}
- with_intel_runtime_pm(gt->uncore->rpm, wakeref)
- status = intel_uncore_read(gt->uncore, huc->status.reg);
-
- return (status & huc->status.mask) == huc->status.value;
+ return huc_is_authenticated(huc);
}
/**
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.h b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
index 73ec670800f2b..77d813840d76c 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
@@ -50,11 +50,6 @@ static inline bool intel_huc_is_used(struct intel_huc *huc)
return intel_uc_fw_is_available(&huc->fw);
}
-static inline bool intel_huc_is_authenticated(struct intel_huc *huc)
-{
- return intel_uc_fw_is_running(&huc->fw);
-}
-
void intel_huc_load_status(struct intel_huc *huc, struct drm_printer *p);
#endif
--
2.25.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 1/4] drm/i915/huc: check HW directly for HuC auth status
2022-04-27 0:26 ` [Intel-gfx] " Daniele Ceraolo Spurio
@ 2022-04-27 12:39 ` Rodrigo Vivi
-1 siblings, 0 replies; 22+ messages in thread
From: Rodrigo Vivi @ 2022-04-27 12:39 UTC (permalink / raw)
To: Daniele Ceraolo Spurio; +Cc: intel-gfx, dri-devel
On Tue, Apr 26, 2022 at 05:26:14PM -0700, Daniele Ceraolo Spurio wrote:
> The huc_is_authenticated function return is based on our SW tracking of
> the HuC auth status. However, around suspend/resume and reset this can
> go out of sync with the actual HW state, which is why we use
> huc_check_state() to look at the actual HW state. Instead of having this
> duality, just make huc_is_authenticated() return the HW state and use it
> everywhere we need to know if HuC is running.
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
> drivers/gpu/drm/i915/gt/uc/intel_huc.c | 23 ++++++++++++++---------
> drivers/gpu/drm/i915/gt/uc/intel_huc.h | 5 -----
> 2 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> index 556829de9c172..773020e69589a 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> @@ -80,6 +80,18 @@ void intel_huc_fini(struct intel_huc *huc)
> intel_uc_fw_fini(&huc->fw);
> }
>
> +static bool huc_is_authenticated(struct intel_huc *huc)
> +{
> + 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);
> +
> + return (status & huc->status.mask) == huc->status.value;
> +}
> +
> /**
> * intel_huc_auth() - Authenticate HuC uCode
> * @huc: intel_huc structure
> @@ -96,7 +108,7 @@ int intel_huc_auth(struct intel_huc *huc)
> struct intel_guc *guc = >->uc.guc;
> int ret;
>
> - GEM_BUG_ON(intel_huc_is_authenticated(huc));
> + GEM_BUG_ON(huc_is_authenticated(huc));
>
> if (!intel_uc_fw_is_loaded(&huc->fw))
> return -ENOEXEC;
> @@ -150,10 +162,6 @@ int intel_huc_auth(struct intel_huc *huc)
> */
> int intel_huc_check_status(struct intel_huc *huc)
> {
> - struct intel_gt *gt = huc_to_gt(huc);
> - intel_wakeref_t wakeref;
> - u32 status = 0;
> -
> switch (__intel_uc_fw_status(&huc->fw)) {
> case INTEL_UC_FIRMWARE_NOT_SUPPORTED:
> return -ENODEV;
> @@ -167,10 +175,7 @@ int intel_huc_check_status(struct intel_huc *huc)
> break;
> }
>
> - with_intel_runtime_pm(gt->uncore->rpm, wakeref)
> - status = intel_uncore_read(gt->uncore, huc->status.reg);
> -
> - return (status & huc->status.mask) == huc->status.value;
oh, these variable names look so generic, while it looks like the only usage
for them is for mask = loaded and value = loaded...
But anyway it is better this indirection with some generic name than duplicating
the definition depending on platform in here...
so:
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> + return huc_is_authenticated(huc);
> }
>
> /**
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.h b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
> index 73ec670800f2b..77d813840d76c 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
> @@ -50,11 +50,6 @@ static inline bool intel_huc_is_used(struct intel_huc *huc)
> return intel_uc_fw_is_available(&huc->fw);
> }
>
> -static inline bool intel_huc_is_authenticated(struct intel_huc *huc)
> -{
> - return intel_uc_fw_is_running(&huc->fw);
> -}
> -
> void intel_huc_load_status(struct intel_huc *huc, struct drm_printer *p);
>
> #endif
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [Intel-gfx] [PATCH 1/4] drm/i915/huc: check HW directly for HuC auth status
@ 2022-04-27 12:39 ` Rodrigo Vivi
0 siblings, 0 replies; 22+ messages in thread
From: Rodrigo Vivi @ 2022-04-27 12:39 UTC (permalink / raw)
To: Daniele Ceraolo Spurio; +Cc: intel-gfx, dri-devel
On Tue, Apr 26, 2022 at 05:26:14PM -0700, Daniele Ceraolo Spurio wrote:
> The huc_is_authenticated function return is based on our SW tracking of
> the HuC auth status. However, around suspend/resume and reset this can
> go out of sync with the actual HW state, which is why we use
> huc_check_state() to look at the actual HW state. Instead of having this
> duality, just make huc_is_authenticated() return the HW state and use it
> everywhere we need to know if HuC is running.
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
> drivers/gpu/drm/i915/gt/uc/intel_huc.c | 23 ++++++++++++++---------
> drivers/gpu/drm/i915/gt/uc/intel_huc.h | 5 -----
> 2 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> index 556829de9c172..773020e69589a 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> @@ -80,6 +80,18 @@ void intel_huc_fini(struct intel_huc *huc)
> intel_uc_fw_fini(&huc->fw);
> }
>
> +static bool huc_is_authenticated(struct intel_huc *huc)
> +{
> + 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);
> +
> + return (status & huc->status.mask) == huc->status.value;
> +}
> +
> /**
> * intel_huc_auth() - Authenticate HuC uCode
> * @huc: intel_huc structure
> @@ -96,7 +108,7 @@ int intel_huc_auth(struct intel_huc *huc)
> struct intel_guc *guc = >->uc.guc;
> int ret;
>
> - GEM_BUG_ON(intel_huc_is_authenticated(huc));
> + GEM_BUG_ON(huc_is_authenticated(huc));
>
> if (!intel_uc_fw_is_loaded(&huc->fw))
> return -ENOEXEC;
> @@ -150,10 +162,6 @@ int intel_huc_auth(struct intel_huc *huc)
> */
> int intel_huc_check_status(struct intel_huc *huc)
> {
> - struct intel_gt *gt = huc_to_gt(huc);
> - intel_wakeref_t wakeref;
> - u32 status = 0;
> -
> switch (__intel_uc_fw_status(&huc->fw)) {
> case INTEL_UC_FIRMWARE_NOT_SUPPORTED:
> return -ENODEV;
> @@ -167,10 +175,7 @@ int intel_huc_check_status(struct intel_huc *huc)
> break;
> }
>
> - with_intel_runtime_pm(gt->uncore->rpm, wakeref)
> - status = intel_uncore_read(gt->uncore, huc->status.reg);
> -
> - return (status & huc->status.mask) == huc->status.value;
oh, these variable names look so generic, while it looks like the only usage
for them is for mask = loaded and value = loaded...
But anyway it is better this indirection with some generic name than duplicating
the definition depending on platform in here...
so:
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> + return huc_is_authenticated(huc);
> }
>
> /**
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.h b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
> index 73ec670800f2b..77d813840d76c 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
> @@ -50,11 +50,6 @@ static inline bool intel_huc_is_used(struct intel_huc *huc)
> return intel_uc_fw_is_available(&huc->fw);
> }
>
> -static inline bool intel_huc_is_authenticated(struct intel_huc *huc)
> -{
> - return intel_uc_fw_is_running(&huc->fw);
> -}
> -
> void intel_huc_load_status(struct intel_huc *huc, struct drm_printer *p);
>
> #endif
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] drm/i915/huc: check HW directly for HuC auth status
2022-04-27 0:26 ` [Intel-gfx] " Daniele Ceraolo Spurio
@ 2022-04-27 21:01 ` Ceraolo Spurio, Daniele
-1 siblings, 0 replies; 22+ messages in thread
From: Ceraolo Spurio, Daniele @ 2022-04-27 21:01 UTC (permalink / raw)
To: intel-gfx, Vivi, Rodrigo; +Cc: dri-devel
On 4/26/2022 5:26 PM, Daniele Ceraolo Spurio wrote:
> The huc_is_authenticated function return is based on our SW tracking of
> the HuC auth status. However, around suspend/resume and reset this can
> go out of sync with the actual HW state, which is why we use
> huc_check_state() to look at the actual HW state. Instead of having this
> duality, just make huc_is_authenticated() return the HW state and use it
> everywhere we need to know if HuC is running.
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
> drivers/gpu/drm/i915/gt/uc/intel_huc.c | 23 ++++++++++++++---------
> drivers/gpu/drm/i915/gt/uc/intel_huc.h | 5 -----
> 2 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> index 556829de9c172..773020e69589a 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> @@ -80,6 +80,18 @@ void intel_huc_fini(struct intel_huc *huc)
> intel_uc_fw_fini(&huc->fw);
> }
>
> +static bool huc_is_authenticated(struct intel_huc *huc)
> +{
> + 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);
> +
> + return (status & huc->status.mask) == huc->status.value;
> +}
> +
> /**
> * intel_huc_auth() - Authenticate HuC uCode
> * @huc: intel_huc structure
> @@ -96,7 +108,7 @@ int intel_huc_auth(struct intel_huc *huc)
> struct intel_guc *guc = >->uc.guc;
> int ret;
>
> - GEM_BUG_ON(intel_huc_is_authenticated(huc));
> + GEM_BUG_ON(huc_is_authenticated(huc));
It looks like even on gen9 HuC is surviving the reset, so this BUG_ON is
now being triggered. I'm going to just change this to a BUG_ON on
intel_uc_fw_is_running() for now, which would be equivalent to what we
have right now, and in the meantime I'll follow up with the HuC team to
see if we can just skip this (and the huc_fw_upload) if HuC shows up as
already authenticated.
Daniele
>
> if (!intel_uc_fw_is_loaded(&huc->fw))
> return -ENOEXEC;
> @@ -150,10 +162,6 @@ int intel_huc_auth(struct intel_huc *huc)
> */
> int intel_huc_check_status(struct intel_huc *huc)
> {
> - struct intel_gt *gt = huc_to_gt(huc);
> - intel_wakeref_t wakeref;
> - u32 status = 0;
> -
> switch (__intel_uc_fw_status(&huc->fw)) {
> case INTEL_UC_FIRMWARE_NOT_SUPPORTED:
> return -ENODEV;
> @@ -167,10 +175,7 @@ int intel_huc_check_status(struct intel_huc *huc)
> break;
> }
>
> - with_intel_runtime_pm(gt->uncore->rpm, wakeref)
> - status = intel_uncore_read(gt->uncore, huc->status.reg);
> -
> - return (status & huc->status.mask) == huc->status.value;
> + return huc_is_authenticated(huc);
> }
>
> /**
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.h b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
> index 73ec670800f2b..77d813840d76c 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
> @@ -50,11 +50,6 @@ static inline bool intel_huc_is_used(struct intel_huc *huc)
> return intel_uc_fw_is_available(&huc->fw);
> }
>
> -static inline bool intel_huc_is_authenticated(struct intel_huc *huc)
> -{
> - return intel_uc_fw_is_running(&huc->fw);
> -}
> -
> void intel_huc_load_status(struct intel_huc *huc, struct drm_printer *p);
>
> #endif
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [Intel-gfx] [PATCH 1/4] drm/i915/huc: check HW directly for HuC auth status
@ 2022-04-27 21:01 ` Ceraolo Spurio, Daniele
0 siblings, 0 replies; 22+ messages in thread
From: Ceraolo Spurio, Daniele @ 2022-04-27 21:01 UTC (permalink / raw)
To: intel-gfx, Vivi, Rodrigo; +Cc: dri-devel
On 4/26/2022 5:26 PM, Daniele Ceraolo Spurio wrote:
> The huc_is_authenticated function return is based on our SW tracking of
> the HuC auth status. However, around suspend/resume and reset this can
> go out of sync with the actual HW state, which is why we use
> huc_check_state() to look at the actual HW state. Instead of having this
> duality, just make huc_is_authenticated() return the HW state and use it
> everywhere we need to know if HuC is running.
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
> drivers/gpu/drm/i915/gt/uc/intel_huc.c | 23 ++++++++++++++---------
> drivers/gpu/drm/i915/gt/uc/intel_huc.h | 5 -----
> 2 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> index 556829de9c172..773020e69589a 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> @@ -80,6 +80,18 @@ void intel_huc_fini(struct intel_huc *huc)
> intel_uc_fw_fini(&huc->fw);
> }
>
> +static bool huc_is_authenticated(struct intel_huc *huc)
> +{
> + 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);
> +
> + return (status & huc->status.mask) == huc->status.value;
> +}
> +
> /**
> * intel_huc_auth() - Authenticate HuC uCode
> * @huc: intel_huc structure
> @@ -96,7 +108,7 @@ int intel_huc_auth(struct intel_huc *huc)
> struct intel_guc *guc = >->uc.guc;
> int ret;
>
> - GEM_BUG_ON(intel_huc_is_authenticated(huc));
> + GEM_BUG_ON(huc_is_authenticated(huc));
It looks like even on gen9 HuC is surviving the reset, so this BUG_ON is
now being triggered. I'm going to just change this to a BUG_ON on
intel_uc_fw_is_running() for now, which would be equivalent to what we
have right now, and in the meantime I'll follow up with the HuC team to
see if we can just skip this (and the huc_fw_upload) if HuC shows up as
already authenticated.
Daniele
>
> if (!intel_uc_fw_is_loaded(&huc->fw))
> return -ENOEXEC;
> @@ -150,10 +162,6 @@ int intel_huc_auth(struct intel_huc *huc)
> */
> int intel_huc_check_status(struct intel_huc *huc)
> {
> - struct intel_gt *gt = huc_to_gt(huc);
> - intel_wakeref_t wakeref;
> - u32 status = 0;
> -
> switch (__intel_uc_fw_status(&huc->fw)) {
> case INTEL_UC_FIRMWARE_NOT_SUPPORTED:
> return -ENODEV;
> @@ -167,10 +175,7 @@ int intel_huc_check_status(struct intel_huc *huc)
> break;
> }
>
> - with_intel_runtime_pm(gt->uncore->rpm, wakeref)
> - status = intel_uncore_read(gt->uncore, huc->status.reg);
> -
> - return (status & huc->status.mask) == huc->status.value;
> + return huc_is_authenticated(huc);
> }
>
> /**
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.h b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
> index 73ec670800f2b..77d813840d76c 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
> @@ -50,11 +50,6 @@ static inline bool intel_huc_is_used(struct intel_huc *huc)
> return intel_uc_fw_is_available(&huc->fw);
> }
>
> -static inline bool intel_huc_is_authenticated(struct intel_huc *huc)
> -{
> - return intel_uc_fw_is_running(&huc->fw);
> -}
> -
> void intel_huc_load_status(struct intel_huc *huc, struct drm_printer *p);
>
> #endif
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 2/4] drm/i915/huc: Add fetch support for gsc-loaded HuC binary
2022-04-27 0:26 ` [Intel-gfx] " Daniele Ceraolo Spurio
@ 2022-04-27 0:26 ` Daniele Ceraolo Spurio
-1 siblings, 0 replies; 22+ messages in thread
From: Daniele Ceraolo Spurio @ 2022-04-27 0:26 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniele Ceraolo Spurio, dri-devel
On newer platforms (starting DG2 G10 B-step and G11 A-step), ownership of
HuC loading has been moved from the GuC to the GSC. As part of the
change, the header format of the HuC binary has been updated and does not
match the GuC anymore. The GSC will perform all the required checks on
the binary size, so we only need to check that the version matches.
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 99 ++++++++++++--------
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h | 2 +
drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h | 9 ++
3 files changed, 72 insertions(+), 38 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
index cb5dd16421d0d..8d602d87a7295 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -301,45 +301,31 @@ static void __force_fw_fetch_failures(struct intel_uc_fw *uc_fw, int e)
}
}
-/**
- * intel_uc_fw_fetch - fetch uC firmware
- * @uc_fw: uC firmware
- *
- * Fetch uC firmware into GEM obj.
- *
- * Return: 0 on success, a negative errno code on failure.
- */
-int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
+static int check_gsc_manifest(const struct firmware *fw,
+ struct intel_uc_fw *uc_fw)
{
- struct drm_i915_private *i915 = __uc_fw_to_gt(uc_fw)->i915;
- struct device *dev = i915->drm.dev;
- struct drm_i915_gem_object *obj;
- const struct firmware *fw = NULL;
- struct uc_css_header *css;
- size_t size;
- int err;
+ u32 *dw = (u32 *)fw->data;
+ u32 version = dw[HUC_GSC_VERSION_DW];
- GEM_BUG_ON(!i915->wopcm.size);
- GEM_BUG_ON(!intel_uc_fw_is_enabled(uc_fw));
-
- err = i915_inject_probe_error(i915, -ENXIO);
- if (err)
- goto fail;
+ uc_fw->major_ver_found = FIELD_GET(HUC_GSC_MAJOR_VER_MASK, version);
+ uc_fw->minor_ver_found = FIELD_GET(HUC_GSC_MINOR_VER_MASK, version);
- __force_fw_fetch_failures(uc_fw, -EINVAL);
- __force_fw_fetch_failures(uc_fw, -ESTALE);
+ return 0;
+}
- err = request_firmware(&fw, uc_fw->path, dev);
- if (err)
- goto fail;
+static int check_ccs_header(struct drm_i915_private *i915,
+ const struct firmware *fw,
+ struct intel_uc_fw *uc_fw)
+{
+ struct uc_css_header *css;
+ size_t size;
/* Check the size of the blob before examining buffer contents */
if (unlikely(fw->size < sizeof(struct uc_css_header))) {
drm_warn(&i915->drm, "%s firmware %s: invalid size: %zu < %zu\n",
intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
fw->size, sizeof(struct uc_css_header));
- err = -ENODATA;
- goto fail;
+ return -ENODATA;
}
css = (struct uc_css_header *)fw->data;
@@ -352,8 +338,7 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
"%s firmware %s: unexpected header size: %zu != %zu\n",
intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
fw->size, sizeof(struct uc_css_header));
- err = -EPROTO;
- goto fail;
+ return -EPROTO;
}
/* uCode size must calculated from other sizes */
@@ -368,8 +353,7 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
drm_warn(&i915->drm, "%s firmware %s: invalid size: %zu < %zu\n",
intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
fw->size, size);
- err = -ENOEXEC;
- goto fail;
+ return -ENOEXEC;
}
/* Sanity check whether this fw is not larger than whole WOPCM memory */
@@ -378,8 +362,7 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
drm_warn(&i915->drm, "%s firmware %s: invalid size: %zu > %zu\n",
intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
size, (size_t)i915->wopcm.size);
- err = -E2BIG;
- goto fail;
+ return -E2BIG;
}
/* Get version numbers from the CSS header */
@@ -388,6 +371,49 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
uc_fw->minor_ver_found = FIELD_GET(CSS_SW_VERSION_UC_MINOR,
css->sw_version);
+ if (uc_fw->type == INTEL_UC_FW_TYPE_GUC)
+ uc_fw->private_data_size = css->private_data_size;
+
+ return 0;
+}
+
+/**
+ * intel_uc_fw_fetch - fetch uC firmware
+ * @uc_fw: uC firmware
+ *
+ * Fetch uC firmware into GEM obj.
+ *
+ * Return: 0 on success, a negative errno code on failure.
+ */
+int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
+{
+ struct drm_i915_private *i915 = __uc_fw_to_gt(uc_fw)->i915;
+ struct device *dev = i915->drm.dev;
+ struct drm_i915_gem_object *obj;
+ const struct firmware *fw = NULL;
+ int err;
+
+ GEM_BUG_ON(!i915->wopcm.size);
+ GEM_BUG_ON(!intel_uc_fw_is_enabled(uc_fw));
+
+ err = i915_inject_probe_error(i915, -ENXIO);
+ if (err)
+ goto fail;
+
+ __force_fw_fetch_failures(uc_fw, -EINVAL);
+ __force_fw_fetch_failures(uc_fw, -ESTALE);
+
+ err = request_firmware(&fw, uc_fw->path, dev);
+ if (err)
+ goto fail;
+
+ if (uc_fw->loaded_via_gsc)
+ err = check_gsc_manifest(fw, uc_fw);
+ else
+ err = check_ccs_header(i915, fw, uc_fw);
+ if (err)
+ goto fail;
+
if (uc_fw->major_ver_found != uc_fw->major_ver_wanted ||
uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) {
drm_notice(&i915->drm, "%s firmware %s: unexpected version: %u.%u != %u.%u\n",
@@ -400,9 +426,6 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
}
}
- if (uc_fw->type == INTEL_UC_FW_TYPE_GUC)
- uc_fw->private_data_size = css->private_data_size;
-
if (HAS_LMEM(i915)) {
obj = i915_gem_object_create_lmem_from_data(i915, fw->data, fw->size);
if (!IS_ERR(obj))
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
index 3229018877d3d..4f169035f5041 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
@@ -102,6 +102,8 @@ struct intel_uc_fw {
u32 ucode_size;
u32 private_data_size;
+
+ bool loaded_via_gsc;
};
#ifdef CONFIG_DRM_I915_DEBUG_GUC
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
index e41ffc7a7fbcb..b05e0e35b734c 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
@@ -39,6 +39,11 @@
* 3. Length info of each component can be found in header, in dwords.
* 4. Modulus and exponent key are not required by driver. They may not appear
* in fw. So driver will load a truncated firmware in this case.
+ *
+ * Starting from DG2, the HuC is loaded by the GSC instead of i915. The GSC
+ * firmware performs all the required integrity checks, we just need to check
+ * the version. Note that the header for GSC-managed blobs is different from the
+ * CSS used for dma-loaded firmwares.
*/
struct uc_css_header {
@@ -78,4 +83,8 @@ struct uc_css_header {
} __packed;
static_assert(sizeof(struct uc_css_header) == 128);
+#define HUC_GSC_VERSION_DW 44
+#define HUC_GSC_MAJOR_VER_MASK (0xFF << 0)
+#define HUC_GSC_MINOR_VER_MASK (0xFF << 16)
+
#endif /* _INTEL_UC_FW_ABI_H */
--
2.25.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [Intel-gfx] [PATCH 2/4] drm/i915/huc: Add fetch support for gsc-loaded HuC binary
@ 2022-04-27 0:26 ` Daniele Ceraolo Spurio
0 siblings, 0 replies; 22+ messages in thread
From: Daniele Ceraolo Spurio @ 2022-04-27 0:26 UTC (permalink / raw)
To: intel-gfx; +Cc: dri-devel
On newer platforms (starting DG2 G10 B-step and G11 A-step), ownership of
HuC loading has been moved from the GuC to the GSC. As part of the
change, the header format of the HuC binary has been updated and does not
match the GuC anymore. The GSC will perform all the required checks on
the binary size, so we only need to check that the version matches.
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 99 ++++++++++++--------
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h | 2 +
drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h | 9 ++
3 files changed, 72 insertions(+), 38 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
index cb5dd16421d0d..8d602d87a7295 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -301,45 +301,31 @@ static void __force_fw_fetch_failures(struct intel_uc_fw *uc_fw, int e)
}
}
-/**
- * intel_uc_fw_fetch - fetch uC firmware
- * @uc_fw: uC firmware
- *
- * Fetch uC firmware into GEM obj.
- *
- * Return: 0 on success, a negative errno code on failure.
- */
-int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
+static int check_gsc_manifest(const struct firmware *fw,
+ struct intel_uc_fw *uc_fw)
{
- struct drm_i915_private *i915 = __uc_fw_to_gt(uc_fw)->i915;
- struct device *dev = i915->drm.dev;
- struct drm_i915_gem_object *obj;
- const struct firmware *fw = NULL;
- struct uc_css_header *css;
- size_t size;
- int err;
+ u32 *dw = (u32 *)fw->data;
+ u32 version = dw[HUC_GSC_VERSION_DW];
- GEM_BUG_ON(!i915->wopcm.size);
- GEM_BUG_ON(!intel_uc_fw_is_enabled(uc_fw));
-
- err = i915_inject_probe_error(i915, -ENXIO);
- if (err)
- goto fail;
+ uc_fw->major_ver_found = FIELD_GET(HUC_GSC_MAJOR_VER_MASK, version);
+ uc_fw->minor_ver_found = FIELD_GET(HUC_GSC_MINOR_VER_MASK, version);
- __force_fw_fetch_failures(uc_fw, -EINVAL);
- __force_fw_fetch_failures(uc_fw, -ESTALE);
+ return 0;
+}
- err = request_firmware(&fw, uc_fw->path, dev);
- if (err)
- goto fail;
+static int check_ccs_header(struct drm_i915_private *i915,
+ const struct firmware *fw,
+ struct intel_uc_fw *uc_fw)
+{
+ struct uc_css_header *css;
+ size_t size;
/* Check the size of the blob before examining buffer contents */
if (unlikely(fw->size < sizeof(struct uc_css_header))) {
drm_warn(&i915->drm, "%s firmware %s: invalid size: %zu < %zu\n",
intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
fw->size, sizeof(struct uc_css_header));
- err = -ENODATA;
- goto fail;
+ return -ENODATA;
}
css = (struct uc_css_header *)fw->data;
@@ -352,8 +338,7 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
"%s firmware %s: unexpected header size: %zu != %zu\n",
intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
fw->size, sizeof(struct uc_css_header));
- err = -EPROTO;
- goto fail;
+ return -EPROTO;
}
/* uCode size must calculated from other sizes */
@@ -368,8 +353,7 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
drm_warn(&i915->drm, "%s firmware %s: invalid size: %zu < %zu\n",
intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
fw->size, size);
- err = -ENOEXEC;
- goto fail;
+ return -ENOEXEC;
}
/* Sanity check whether this fw is not larger than whole WOPCM memory */
@@ -378,8 +362,7 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
drm_warn(&i915->drm, "%s firmware %s: invalid size: %zu > %zu\n",
intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
size, (size_t)i915->wopcm.size);
- err = -E2BIG;
- goto fail;
+ return -E2BIG;
}
/* Get version numbers from the CSS header */
@@ -388,6 +371,49 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
uc_fw->minor_ver_found = FIELD_GET(CSS_SW_VERSION_UC_MINOR,
css->sw_version);
+ if (uc_fw->type == INTEL_UC_FW_TYPE_GUC)
+ uc_fw->private_data_size = css->private_data_size;
+
+ return 0;
+}
+
+/**
+ * intel_uc_fw_fetch - fetch uC firmware
+ * @uc_fw: uC firmware
+ *
+ * Fetch uC firmware into GEM obj.
+ *
+ * Return: 0 on success, a negative errno code on failure.
+ */
+int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
+{
+ struct drm_i915_private *i915 = __uc_fw_to_gt(uc_fw)->i915;
+ struct device *dev = i915->drm.dev;
+ struct drm_i915_gem_object *obj;
+ const struct firmware *fw = NULL;
+ int err;
+
+ GEM_BUG_ON(!i915->wopcm.size);
+ GEM_BUG_ON(!intel_uc_fw_is_enabled(uc_fw));
+
+ err = i915_inject_probe_error(i915, -ENXIO);
+ if (err)
+ goto fail;
+
+ __force_fw_fetch_failures(uc_fw, -EINVAL);
+ __force_fw_fetch_failures(uc_fw, -ESTALE);
+
+ err = request_firmware(&fw, uc_fw->path, dev);
+ if (err)
+ goto fail;
+
+ if (uc_fw->loaded_via_gsc)
+ err = check_gsc_manifest(fw, uc_fw);
+ else
+ err = check_ccs_header(i915, fw, uc_fw);
+ if (err)
+ goto fail;
+
if (uc_fw->major_ver_found != uc_fw->major_ver_wanted ||
uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) {
drm_notice(&i915->drm, "%s firmware %s: unexpected version: %u.%u != %u.%u\n",
@@ -400,9 +426,6 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
}
}
- if (uc_fw->type == INTEL_UC_FW_TYPE_GUC)
- uc_fw->private_data_size = css->private_data_size;
-
if (HAS_LMEM(i915)) {
obj = i915_gem_object_create_lmem_from_data(i915, fw->data, fw->size);
if (!IS_ERR(obj))
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
index 3229018877d3d..4f169035f5041 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
@@ -102,6 +102,8 @@ struct intel_uc_fw {
u32 ucode_size;
u32 private_data_size;
+
+ bool loaded_via_gsc;
};
#ifdef CONFIG_DRM_I915_DEBUG_GUC
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
index e41ffc7a7fbcb..b05e0e35b734c 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
@@ -39,6 +39,11 @@
* 3. Length info of each component can be found in header, in dwords.
* 4. Modulus and exponent key are not required by driver. They may not appear
* in fw. So driver will load a truncated firmware in this case.
+ *
+ * Starting from DG2, the HuC is loaded by the GSC instead of i915. The GSC
+ * firmware performs all the required integrity checks, we just need to check
+ * the version. Note that the header for GSC-managed blobs is different from the
+ * CSS used for dma-loaded firmwares.
*/
struct uc_css_header {
@@ -78,4 +83,8 @@ struct uc_css_header {
} __packed;
static_assert(sizeof(struct uc_css_header) == 128);
+#define HUC_GSC_VERSION_DW 44
+#define HUC_GSC_MAJOR_VER_MASK (0xFF << 0)
+#define HUC_GSC_MINOR_VER_MASK (0xFF << 16)
+
#endif /* _INTEL_UC_FW_ABI_H */
--
2.25.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [2/4] drm/i915/huc: Add fetch support for gsc-loaded HuC binary
2022-04-27 0:26 ` [Intel-gfx] " Daniele Ceraolo Spurio
@ 2022-04-29 18:20 ` Teres Alexis, Alan Previn
-1 siblings, 0 replies; 22+ messages in thread
From: Teres Alexis, Alan Previn @ 2022-04-29 18:20 UTC (permalink / raw)
To: Ceraolo Spurio, Daniele, intel-gfx; +Cc: dri-devel
minor nit: not sure if its worth mentioning in commit msg that "loaded_via_gsc"
is zero-init-ed as fw_def macros we havent added fw_defs for gsc-loaded-huc-bins
which explains why "loaded_via_gsc" not getting set anywhere in this series.
Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>
On Tue, 2022-04-26 at 17:26 -0700, Daniele Ceraolo Spurio wrote:
> On newer platforms (starting DG2 G10 B-step and G11 A-step), ownership of
> HuC loading has been moved from the GuC to the GSC. As part of the
> change, the header format of the HuC binary has been updated and does not
> match the GuC anymore. The GSC will perform all the required checks on
> the binary size, so we only need to check that the version matches.
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 99 ++++++++++++--------
> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h | 2 +
> drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h | 9 ++
> 3 files changed, 72 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> index cb5dd16421d0d..8d602d87a7295 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> @@ -301,45 +301,31 @@ static void __force_fw_fetch_failures(struct intel_uc_fw *uc_fw, int e)
> }
> }
>
> -/**
> - * intel_uc_fw_fetch - fetch uC firmware
> - * @uc_fw: uC firmware
> - *
> - * Fetch uC firmware into GEM obj.
> - *
> - * Return: 0 on success, a negative errno code on failure.
> - */
> -int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
> +static int check_gsc_manifest(const struct firmware *fw,
> + struct intel_uc_fw *uc_fw)
> {
> - struct drm_i915_private *i915 = __uc_fw_to_gt(uc_fw)->i915;
> - struct device *dev = i915->drm.dev;
> - struct drm_i915_gem_object *obj;
> - const struct firmware *fw = NULL;
> - struct uc_css_header *css;
> - size_t size;
> - int err;
> + u32 *dw = (u32 *)fw->data;
> + u32 version = dw[HUC_GSC_VERSION_DW];
>
> - GEM_BUG_ON(!i915->wopcm.size);
> - GEM_BUG_ON(!intel_uc_fw_is_enabled(uc_fw));
> -
> - err = i915_inject_probe_error(i915, -ENXIO);
> - if (err)
> - goto fail;
> + uc_fw->major_ver_found = FIELD_GET(HUC_GSC_MAJOR_VER_MASK, version);
> + uc_fw->minor_ver_found = FIELD_GET(HUC_GSC_MINOR_VER_MASK, version);
>
> - __force_fw_fetch_failures(uc_fw, -EINVAL);
> - __force_fw_fetch_failures(uc_fw, -ESTALE);
> + return 0;
> +}
>
> - err = request_firmware(&fw, uc_fw->path, dev);
> - if (err)
> - goto fail;
> +static int check_ccs_header(struct drm_i915_private *i915,
> + const struct firmware *fw,
> + struct intel_uc_fw *uc_fw)
> +{
> + struct uc_css_header *css;
> + size_t size;
>
> /* Check the size of the blob before examining buffer contents */
> if (unlikely(fw->size < sizeof(struct uc_css_header))) {
> drm_warn(&i915->drm, "%s firmware %s: invalid size: %zu < %zu\n",
> intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
> fw->size, sizeof(struct uc_css_header));
> - err = -ENODATA;
> - goto fail;
> + return -ENODATA;
> }
>
> css = (struct uc_css_header *)fw->data;
> @@ -352,8 +338,7 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
> "%s firmware %s: unexpected header size: %zu != %zu\n",
> intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
> fw->size, sizeof(struct uc_css_header));
> - err = -EPROTO;
> - goto fail;
> + return -EPROTO;
> }
>
> /* uCode size must calculated from other sizes */
> @@ -368,8 +353,7 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
> drm_warn(&i915->drm, "%s firmware %s: invalid size: %zu < %zu\n",
> intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
> fw->size, size);
> - err = -ENOEXEC;
> - goto fail;
> + return -ENOEXEC;
> }
>
> /* Sanity check whether this fw is not larger than whole WOPCM memory */
> @@ -378,8 +362,7 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
> drm_warn(&i915->drm, "%s firmware %s: invalid size: %zu > %zu\n",
> intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
> size, (size_t)i915->wopcm.size);
> - err = -E2BIG;
> - goto fail;
> + return -E2BIG;
> }
>
> /* Get version numbers from the CSS header */
> @@ -388,6 +371,49 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
> uc_fw->minor_ver_found = FIELD_GET(CSS_SW_VERSION_UC_MINOR,
> css->sw_version);
>
> + if (uc_fw->type == INTEL_UC_FW_TYPE_GUC)
> + uc_fw->private_data_size = css->private_data_size;
> +
> + return 0;
> +}
> +
> +/**
> + * intel_uc_fw_fetch - fetch uC firmware
> + * @uc_fw: uC firmware
> + *
> + * Fetch uC firmware into GEM obj.
> + *
> + * Return: 0 on success, a negative errno code on failure.
> + */
> +int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
> +{
> + struct drm_i915_private *i915 = __uc_fw_to_gt(uc_fw)->i915;
> + struct device *dev = i915->drm.dev;
> + struct drm_i915_gem_object *obj;
> + const struct firmware *fw = NULL;
> + int err;
> +
> + GEM_BUG_ON(!i915->wopcm.size);
> + GEM_BUG_ON(!intel_uc_fw_is_enabled(uc_fw));
> +
> + err = i915_inject_probe_error(i915, -ENXIO);
> + if (err)
> + goto fail;
> +
> + __force_fw_fetch_failures(uc_fw, -EINVAL);
> + __force_fw_fetch_failures(uc_fw, -ESTALE);
> +
> + err = request_firmware(&fw, uc_fw->path, dev);
> + if (err)
> + goto fail;
> +
> + if (uc_fw->loaded_via_gsc)
> + err = check_gsc_manifest(fw, uc_fw);
> + else
> + err = check_ccs_header(i915, fw, uc_fw);
> + if (err)
> + goto fail;
> +
> if (uc_fw->major_ver_found != uc_fw->major_ver_wanted ||
> uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) {
> drm_notice(&i915->drm, "%s firmware %s: unexpected version: %u.%u != %u.%u\n",
> @@ -400,9 +426,6 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
> }
> }
>
> - if (uc_fw->type == INTEL_UC_FW_TYPE_GUC)
> - uc_fw->private_data_size = css->private_data_size;
> -
> if (HAS_LMEM(i915)) {
> obj = i915_gem_object_create_lmem_from_data(i915, fw->data, fw->size);
> if (!IS_ERR(obj))
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
> index 3229018877d3d..4f169035f5041 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
> @@ -102,6 +102,8 @@ struct intel_uc_fw {
> u32 ucode_size;
>
> u32 private_data_size;
> +
> + bool loaded_via_gsc;
> };
>
> #ifdef CONFIG_DRM_I915_DEBUG_GUC
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
> index e41ffc7a7fbcb..b05e0e35b734c 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
> @@ -39,6 +39,11 @@
> * 3. Length info of each component can be found in header, in dwords.
> * 4. Modulus and exponent key are not required by driver. They may not appear
> * in fw. So driver will load a truncated firmware in this case.
> + *
> + * Starting from DG2, the HuC is loaded by the GSC instead of i915. The GSC
> + * firmware performs all the required integrity checks, we just need to check
> + * the version. Note that the header for GSC-managed blobs is different from the
> + * CSS used for dma-loaded firmwares.
> */
>
> struct uc_css_header {
> @@ -78,4 +83,8 @@ struct uc_css_header {
> } __packed;
> static_assert(sizeof(struct uc_css_header) == 128);
>
> +#define HUC_GSC_VERSION_DW 44
> +#define HUC_GSC_MAJOR_VER_MASK (0xFF << 0)
> +#define HUC_GSC_MINOR_VER_MASK (0xFF << 16)
> +
> #endif /* _INTEL_UC_FW_ABI_H */
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [Intel-gfx] [2/4] drm/i915/huc: Add fetch support for gsc-loaded HuC binary
@ 2022-04-29 18:20 ` Teres Alexis, Alan Previn
0 siblings, 0 replies; 22+ messages in thread
From: Teres Alexis, Alan Previn @ 2022-04-29 18:20 UTC (permalink / raw)
To: Ceraolo Spurio, Daniele, intel-gfx; +Cc: dri-devel
minor nit: not sure if its worth mentioning in commit msg that "loaded_via_gsc"
is zero-init-ed as fw_def macros we havent added fw_defs for gsc-loaded-huc-bins
which explains why "loaded_via_gsc" not getting set anywhere in this series.
Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>
On Tue, 2022-04-26 at 17:26 -0700, Daniele Ceraolo Spurio wrote:
> On newer platforms (starting DG2 G10 B-step and G11 A-step), ownership of
> HuC loading has been moved from the GuC to the GSC. As part of the
> change, the header format of the HuC binary has been updated and does not
> match the GuC anymore. The GSC will perform all the required checks on
> the binary size, so we only need to check that the version matches.
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 99 ++++++++++++--------
> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h | 2 +
> drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h | 9 ++
> 3 files changed, 72 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> index cb5dd16421d0d..8d602d87a7295 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> @@ -301,45 +301,31 @@ static void __force_fw_fetch_failures(struct intel_uc_fw *uc_fw, int e)
> }
> }
>
> -/**
> - * intel_uc_fw_fetch - fetch uC firmware
> - * @uc_fw: uC firmware
> - *
> - * Fetch uC firmware into GEM obj.
> - *
> - * Return: 0 on success, a negative errno code on failure.
> - */
> -int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
> +static int check_gsc_manifest(const struct firmware *fw,
> + struct intel_uc_fw *uc_fw)
> {
> - struct drm_i915_private *i915 = __uc_fw_to_gt(uc_fw)->i915;
> - struct device *dev = i915->drm.dev;
> - struct drm_i915_gem_object *obj;
> - const struct firmware *fw = NULL;
> - struct uc_css_header *css;
> - size_t size;
> - int err;
> + u32 *dw = (u32 *)fw->data;
> + u32 version = dw[HUC_GSC_VERSION_DW];
>
> - GEM_BUG_ON(!i915->wopcm.size);
> - GEM_BUG_ON(!intel_uc_fw_is_enabled(uc_fw));
> -
> - err = i915_inject_probe_error(i915, -ENXIO);
> - if (err)
> - goto fail;
> + uc_fw->major_ver_found = FIELD_GET(HUC_GSC_MAJOR_VER_MASK, version);
> + uc_fw->minor_ver_found = FIELD_GET(HUC_GSC_MINOR_VER_MASK, version);
>
> - __force_fw_fetch_failures(uc_fw, -EINVAL);
> - __force_fw_fetch_failures(uc_fw, -ESTALE);
> + return 0;
> +}
>
> - err = request_firmware(&fw, uc_fw->path, dev);
> - if (err)
> - goto fail;
> +static int check_ccs_header(struct drm_i915_private *i915,
> + const struct firmware *fw,
> + struct intel_uc_fw *uc_fw)
> +{
> + struct uc_css_header *css;
> + size_t size;
>
> /* Check the size of the blob before examining buffer contents */
> if (unlikely(fw->size < sizeof(struct uc_css_header))) {
> drm_warn(&i915->drm, "%s firmware %s: invalid size: %zu < %zu\n",
> intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
> fw->size, sizeof(struct uc_css_header));
> - err = -ENODATA;
> - goto fail;
> + return -ENODATA;
> }
>
> css = (struct uc_css_header *)fw->data;
> @@ -352,8 +338,7 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
> "%s firmware %s: unexpected header size: %zu != %zu\n",
> intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
> fw->size, sizeof(struct uc_css_header));
> - err = -EPROTO;
> - goto fail;
> + return -EPROTO;
> }
>
> /* uCode size must calculated from other sizes */
> @@ -368,8 +353,7 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
> drm_warn(&i915->drm, "%s firmware %s: invalid size: %zu < %zu\n",
> intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
> fw->size, size);
> - err = -ENOEXEC;
> - goto fail;
> + return -ENOEXEC;
> }
>
> /* Sanity check whether this fw is not larger than whole WOPCM memory */
> @@ -378,8 +362,7 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
> drm_warn(&i915->drm, "%s firmware %s: invalid size: %zu > %zu\n",
> intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
> size, (size_t)i915->wopcm.size);
> - err = -E2BIG;
> - goto fail;
> + return -E2BIG;
> }
>
> /* Get version numbers from the CSS header */
> @@ -388,6 +371,49 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
> uc_fw->minor_ver_found = FIELD_GET(CSS_SW_VERSION_UC_MINOR,
> css->sw_version);
>
> + if (uc_fw->type == INTEL_UC_FW_TYPE_GUC)
> + uc_fw->private_data_size = css->private_data_size;
> +
> + return 0;
> +}
> +
> +/**
> + * intel_uc_fw_fetch - fetch uC firmware
> + * @uc_fw: uC firmware
> + *
> + * Fetch uC firmware into GEM obj.
> + *
> + * Return: 0 on success, a negative errno code on failure.
> + */
> +int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
> +{
> + struct drm_i915_private *i915 = __uc_fw_to_gt(uc_fw)->i915;
> + struct device *dev = i915->drm.dev;
> + struct drm_i915_gem_object *obj;
> + const struct firmware *fw = NULL;
> + int err;
> +
> + GEM_BUG_ON(!i915->wopcm.size);
> + GEM_BUG_ON(!intel_uc_fw_is_enabled(uc_fw));
> +
> + err = i915_inject_probe_error(i915, -ENXIO);
> + if (err)
> + goto fail;
> +
> + __force_fw_fetch_failures(uc_fw, -EINVAL);
> + __force_fw_fetch_failures(uc_fw, -ESTALE);
> +
> + err = request_firmware(&fw, uc_fw->path, dev);
> + if (err)
> + goto fail;
> +
> + if (uc_fw->loaded_via_gsc)
> + err = check_gsc_manifest(fw, uc_fw);
> + else
> + err = check_ccs_header(i915, fw, uc_fw);
> + if (err)
> + goto fail;
> +
> if (uc_fw->major_ver_found != uc_fw->major_ver_wanted ||
> uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) {
> drm_notice(&i915->drm, "%s firmware %s: unexpected version: %u.%u != %u.%u\n",
> @@ -400,9 +426,6 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
> }
> }
>
> - if (uc_fw->type == INTEL_UC_FW_TYPE_GUC)
> - uc_fw->private_data_size = css->private_data_size;
> -
> if (HAS_LMEM(i915)) {
> obj = i915_gem_object_create_lmem_from_data(i915, fw->data, fw->size);
> if (!IS_ERR(obj))
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
> index 3229018877d3d..4f169035f5041 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
> @@ -102,6 +102,8 @@ struct intel_uc_fw {
> u32 ucode_size;
>
> u32 private_data_size;
> +
> + bool loaded_via_gsc;
> };
>
> #ifdef CONFIG_DRM_I915_DEBUG_GUC
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
> index e41ffc7a7fbcb..b05e0e35b734c 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
> @@ -39,6 +39,11 @@
> * 3. Length info of each component can be found in header, in dwords.
> * 4. Modulus and exponent key are not required by driver. They may not appear
> * in fw. So driver will load a truncated firmware in this case.
> + *
> + * Starting from DG2, the HuC is loaded by the GSC instead of i915. The GSC
> + * firmware performs all the required integrity checks, we just need to check
> + * the version. Note that the header for GSC-managed blobs is different from the
> + * CSS used for dma-loaded firmwares.
> */
>
> struct uc_css_header {
> @@ -78,4 +83,8 @@ struct uc_css_header {
> } __packed;
> static_assert(sizeof(struct uc_css_header) == 128);
>
> +#define HUC_GSC_VERSION_DW 44
> +#define HUC_GSC_MAJOR_VER_MASK (0xFF << 0)
> +#define HUC_GSC_MINOR_VER_MASK (0xFF << 16)
> +
> #endif /* _INTEL_UC_FW_ABI_H */
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 3/4] drm/i915/huc: Prepare for GSC-loaded HuC
2022-04-27 0:26 ` [Intel-gfx] " Daniele Ceraolo Spurio
@ 2022-04-27 0:26 ` Daniele Ceraolo Spurio
-1 siblings, 0 replies; 22+ messages in thread
From: Daniele Ceraolo Spurio @ 2022-04-27 0:26 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniele Ceraolo Spurio, dri-devel
HuC loading via GSC is performed via a PXP command sent through the mei
modules, so we need both MEI_GSC and MEI_PXP to be available. Given that
the GSC will do both the transfer and the authentication, the legacy HuC
loading paths can be safely skipped.
Also note that the GSC-loaded HuC survives GT reset.
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h | 1 +
drivers/gpu/drm/i915/gt/uc/intel_huc.c | 76 +++++++++++++++++++---
drivers/gpu/drm/i915/gt/uc/intel_huc.h | 6 ++
drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c | 5 +-
drivers/gpu/drm/i915/gt/uc/intel_uc.c | 11 +++-
5 files changed, 88 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h
index 66027a42cda9e..2516705b9f365 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h
@@ -96,6 +96,7 @@
#define GUC_SHIM_CONTROL2 _MMIO(0xc068)
#define GUC_IS_PRIVILEGED (1<<29)
+#define GSC_LOADS_HUC (1<<30)
#define GUC_SEND_INTERRUPT _MMIO(0xc4c8)
#define GUC_SEND_TRIGGER (1<<0)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
index 773020e69589a..76a7df7f136fc 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
@@ -6,6 +6,7 @@
#include <linux/types.h>
#include "gt/intel_gt.h"
+#include "intel_guc_reg.h"
#include "intel_huc.h"
#include "i915_drv.h"
@@ -17,11 +18,15 @@
* 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. For
- * The GuC to correctly perform the authentication, 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.
+ * 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.
+ * 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.
*
* See https://github.com/intel/media-driver for the latest details on HuC
* functionality.
@@ -54,11 +59,51 @@ void intel_huc_init_early(struct intel_huc *huc)
}
}
+#define HUC_LOAD_MODE_STRING(x) (x ? "GSC" : "legacy")
+static int check_huc_loading_mode(struct intel_huc *huc)
+{
+ struct intel_gt *gt = huc_to_gt(huc);
+ bool fw_needs_gsc = intel_huc_is_loaded_by_gsc(huc);
+ bool hw_uses_gsc = false;
+
+ /*
+ * The fuse for HuC load via GSC is only valid on platforms that have
+ * GuC deprivilege.
+ */
+ if (HAS_GUC_DEPRIVILEGE(gt->i915))
+ hw_uses_gsc = intel_uncore_read(gt->uncore, GUC_SHIM_CONTROL2) &
+ GSC_LOADS_HUC;
+
+ if (fw_needs_gsc != hw_uses_gsc) {
+ drm_err(>->i915->drm,
+ "mismatch between HuC FW (%s) and HW (%s) load modes\n",
+ HUC_LOAD_MODE_STRING(fw_needs_gsc),
+ HUC_LOAD_MODE_STRING(hw_uses_gsc));
+ return -ENOEXEC;
+ }
+
+ /* make sure we can access the GSC via the mei driver if we need it */
+ if (!(IS_ENABLED(CONFIG_INTEL_MEI_PXP) && IS_ENABLED(CONFIG_INTEL_MEI_GSC)) &&
+ fw_needs_gsc) {
+ drm_info(>->i915->drm,
+ "Can't load HuC due to missing MEI modules\n");
+ return -EIO;
+ }
+
+ drm_dbg(>->i915->drm, "GSC loads huc=%s\n", str_yes_no(fw_needs_gsc));
+
+ return 0;
+}
+
int intel_huc_init(struct intel_huc *huc)
{
struct drm_i915_private *i915 = huc_to_gt(huc)->i915;
int err;
+ err = check_huc_loading_mode(huc);
+ if (err)
+ goto out;
+
err = intel_uc_fw_init(&huc->fw);
if (err)
goto out;
@@ -108,17 +153,20 @@ int intel_huc_auth(struct intel_huc *huc)
struct intel_guc *guc = >->uc.guc;
int ret;
- GEM_BUG_ON(huc_is_authenticated(huc));
-
if (!intel_uc_fw_is_loaded(&huc->fw))
return -ENOEXEC;
+ /* GSC will do the auth */
+ if (intel_huc_is_loaded_by_gsc(huc))
+ return -ENODEV;
+
ret = i915_inject_probe_error(gt->i915, -ENXIO);
if (ret)
goto fail;
- ret = intel_guc_auth_huc(guc,
- intel_guc_ggtt_offset(guc, huc->fw.rsa_data));
+ GEM_BUG_ON(huc_is_authenticated(huc));
+
+ ret = intel_guc_auth_huc(guc, intel_guc_ggtt_offset(guc, huc->fw.rsa_data));
if (ret) {
DRM_ERROR("HuC: GuC did not ack Auth request %d\n", ret);
goto fail;
@@ -178,6 +226,16 @@ int intel_huc_check_status(struct intel_huc *huc)
return huc_is_authenticated(huc);
}
+void intel_huc_update_auth_status(struct intel_huc *huc)
+{
+ if (!intel_uc_fw_is_loadable(&huc->fw))
+ return;
+
+ if (huc_is_authenticated(huc))
+ intel_uc_fw_change_status(&huc->fw,
+ INTEL_UC_FIRMWARE_RUNNING);
+}
+
/**
* intel_huc_load_status - dump information about HuC load status
* @huc: the HuC
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.h b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
index 77d813840d76c..d7e25b6e879eb 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
@@ -27,6 +27,7 @@ int intel_huc_init(struct intel_huc *huc);
void intel_huc_fini(struct intel_huc *huc);
int intel_huc_auth(struct intel_huc *huc);
int intel_huc_check_status(struct intel_huc *huc);
+void intel_huc_update_auth_status(struct intel_huc *huc);
static inline int intel_huc_sanitize(struct intel_huc *huc)
{
@@ -50,6 +51,11 @@ static inline bool intel_huc_is_used(struct intel_huc *huc)
return intel_uc_fw_is_available(&huc->fw);
}
+static inline bool intel_huc_is_loaded_by_gsc(const struct intel_huc *huc)
+{
+ return huc->fw.loaded_via_gsc;
+}
+
void intel_huc_load_status(struct intel_huc *huc, struct drm_printer *p);
#endif
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 e5ef509c70e89..9d6ab1e016395 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
@@ -8,7 +8,7 @@
#include "i915_drv.h"
/**
- * intel_huc_fw_upload() - load HuC uCode to device
+ * intel_huc_fw_upload() - load HuC uCode to device via DMA transfer
* @huc: intel_huc structure
*
* Called from intel_uc_init_hw() during driver load, resume from sleep and
@@ -21,6 +21,9 @@
*/
int intel_huc_fw_upload(struct intel_huc *huc)
{
+ if (intel_huc_is_loaded_by_gsc(huc))
+ return -ENODEV;
+
/* HW doesn't look at destination address for HuC, so set it to 0 */
return intel_uc_fw_upload(&huc->fw, 0, HUC_UKERNEL);
}
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index 8c9ef690ac9d8..0dce94f896a8c 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -509,7 +509,16 @@ static int __uc_init_hw(struct intel_uc *uc)
if (ret)
goto err_log_capture;
- intel_huc_auth(huc);
+ /*
+ * GSC-loaded HuC is authenticated by the GSC, so we don't need to
+ * trigger the auth here. However, given that the HuC loaded this way
+ * survive GT reset, we still need to update our SW bookkeeping to make
+ * sure it reflects the correct HW status.
+ */
+ if (intel_huc_is_loaded_by_gsc(huc))
+ intel_huc_update_auth_status(huc);
+ else
+ intel_huc_auth(huc);
if (intel_uc_uses_guc_submission(uc))
intel_guc_submission_enable(guc);
--
2.25.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [Intel-gfx] [PATCH 3/4] drm/i915/huc: Prepare for GSC-loaded HuC
@ 2022-04-27 0:26 ` Daniele Ceraolo Spurio
0 siblings, 0 replies; 22+ messages in thread
From: Daniele Ceraolo Spurio @ 2022-04-27 0:26 UTC (permalink / raw)
To: intel-gfx; +Cc: dri-devel
HuC loading via GSC is performed via a PXP command sent through the mei
modules, so we need both MEI_GSC and MEI_PXP to be available. Given that
the GSC will do both the transfer and the authentication, the legacy HuC
loading paths can be safely skipped.
Also note that the GSC-loaded HuC survives GT reset.
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h | 1 +
drivers/gpu/drm/i915/gt/uc/intel_huc.c | 76 +++++++++++++++++++---
drivers/gpu/drm/i915/gt/uc/intel_huc.h | 6 ++
drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c | 5 +-
drivers/gpu/drm/i915/gt/uc/intel_uc.c | 11 +++-
5 files changed, 88 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h
index 66027a42cda9e..2516705b9f365 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h
@@ -96,6 +96,7 @@
#define GUC_SHIM_CONTROL2 _MMIO(0xc068)
#define GUC_IS_PRIVILEGED (1<<29)
+#define GSC_LOADS_HUC (1<<30)
#define GUC_SEND_INTERRUPT _MMIO(0xc4c8)
#define GUC_SEND_TRIGGER (1<<0)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
index 773020e69589a..76a7df7f136fc 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
@@ -6,6 +6,7 @@
#include <linux/types.h>
#include "gt/intel_gt.h"
+#include "intel_guc_reg.h"
#include "intel_huc.h"
#include "i915_drv.h"
@@ -17,11 +18,15 @@
* 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. For
- * The GuC to correctly perform the authentication, 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.
+ * 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.
+ * 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.
*
* See https://github.com/intel/media-driver for the latest details on HuC
* functionality.
@@ -54,11 +59,51 @@ void intel_huc_init_early(struct intel_huc *huc)
}
}
+#define HUC_LOAD_MODE_STRING(x) (x ? "GSC" : "legacy")
+static int check_huc_loading_mode(struct intel_huc *huc)
+{
+ struct intel_gt *gt = huc_to_gt(huc);
+ bool fw_needs_gsc = intel_huc_is_loaded_by_gsc(huc);
+ bool hw_uses_gsc = false;
+
+ /*
+ * The fuse for HuC load via GSC is only valid on platforms that have
+ * GuC deprivilege.
+ */
+ if (HAS_GUC_DEPRIVILEGE(gt->i915))
+ hw_uses_gsc = intel_uncore_read(gt->uncore, GUC_SHIM_CONTROL2) &
+ GSC_LOADS_HUC;
+
+ if (fw_needs_gsc != hw_uses_gsc) {
+ drm_err(>->i915->drm,
+ "mismatch between HuC FW (%s) and HW (%s) load modes\n",
+ HUC_LOAD_MODE_STRING(fw_needs_gsc),
+ HUC_LOAD_MODE_STRING(hw_uses_gsc));
+ return -ENOEXEC;
+ }
+
+ /* make sure we can access the GSC via the mei driver if we need it */
+ if (!(IS_ENABLED(CONFIG_INTEL_MEI_PXP) && IS_ENABLED(CONFIG_INTEL_MEI_GSC)) &&
+ fw_needs_gsc) {
+ drm_info(>->i915->drm,
+ "Can't load HuC due to missing MEI modules\n");
+ return -EIO;
+ }
+
+ drm_dbg(>->i915->drm, "GSC loads huc=%s\n", str_yes_no(fw_needs_gsc));
+
+ return 0;
+}
+
int intel_huc_init(struct intel_huc *huc)
{
struct drm_i915_private *i915 = huc_to_gt(huc)->i915;
int err;
+ err = check_huc_loading_mode(huc);
+ if (err)
+ goto out;
+
err = intel_uc_fw_init(&huc->fw);
if (err)
goto out;
@@ -108,17 +153,20 @@ int intel_huc_auth(struct intel_huc *huc)
struct intel_guc *guc = >->uc.guc;
int ret;
- GEM_BUG_ON(huc_is_authenticated(huc));
-
if (!intel_uc_fw_is_loaded(&huc->fw))
return -ENOEXEC;
+ /* GSC will do the auth */
+ if (intel_huc_is_loaded_by_gsc(huc))
+ return -ENODEV;
+
ret = i915_inject_probe_error(gt->i915, -ENXIO);
if (ret)
goto fail;
- ret = intel_guc_auth_huc(guc,
- intel_guc_ggtt_offset(guc, huc->fw.rsa_data));
+ GEM_BUG_ON(huc_is_authenticated(huc));
+
+ ret = intel_guc_auth_huc(guc, intel_guc_ggtt_offset(guc, huc->fw.rsa_data));
if (ret) {
DRM_ERROR("HuC: GuC did not ack Auth request %d\n", ret);
goto fail;
@@ -178,6 +226,16 @@ int intel_huc_check_status(struct intel_huc *huc)
return huc_is_authenticated(huc);
}
+void intel_huc_update_auth_status(struct intel_huc *huc)
+{
+ if (!intel_uc_fw_is_loadable(&huc->fw))
+ return;
+
+ if (huc_is_authenticated(huc))
+ intel_uc_fw_change_status(&huc->fw,
+ INTEL_UC_FIRMWARE_RUNNING);
+}
+
/**
* intel_huc_load_status - dump information about HuC load status
* @huc: the HuC
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.h b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
index 77d813840d76c..d7e25b6e879eb 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
@@ -27,6 +27,7 @@ int intel_huc_init(struct intel_huc *huc);
void intel_huc_fini(struct intel_huc *huc);
int intel_huc_auth(struct intel_huc *huc);
int intel_huc_check_status(struct intel_huc *huc);
+void intel_huc_update_auth_status(struct intel_huc *huc);
static inline int intel_huc_sanitize(struct intel_huc *huc)
{
@@ -50,6 +51,11 @@ static inline bool intel_huc_is_used(struct intel_huc *huc)
return intel_uc_fw_is_available(&huc->fw);
}
+static inline bool intel_huc_is_loaded_by_gsc(const struct intel_huc *huc)
+{
+ return huc->fw.loaded_via_gsc;
+}
+
void intel_huc_load_status(struct intel_huc *huc, struct drm_printer *p);
#endif
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 e5ef509c70e89..9d6ab1e016395 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
@@ -8,7 +8,7 @@
#include "i915_drv.h"
/**
- * intel_huc_fw_upload() - load HuC uCode to device
+ * intel_huc_fw_upload() - load HuC uCode to device via DMA transfer
* @huc: intel_huc structure
*
* Called from intel_uc_init_hw() during driver load, resume from sleep and
@@ -21,6 +21,9 @@
*/
int intel_huc_fw_upload(struct intel_huc *huc)
{
+ if (intel_huc_is_loaded_by_gsc(huc))
+ return -ENODEV;
+
/* HW doesn't look at destination address for HuC, so set it to 0 */
return intel_uc_fw_upload(&huc->fw, 0, HUC_UKERNEL);
}
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index 8c9ef690ac9d8..0dce94f896a8c 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -509,7 +509,16 @@ static int __uc_init_hw(struct intel_uc *uc)
if (ret)
goto err_log_capture;
- intel_huc_auth(huc);
+ /*
+ * GSC-loaded HuC is authenticated by the GSC, so we don't need to
+ * trigger the auth here. However, given that the HuC loaded this way
+ * survive GT reset, we still need to update our SW bookkeeping to make
+ * sure it reflects the correct HW status.
+ */
+ if (intel_huc_is_loaded_by_gsc(huc))
+ intel_huc_update_auth_status(huc);
+ else
+ intel_huc_auth(huc);
if (intel_uc_uses_guc_submission(uc))
intel_guc_submission_enable(guc);
--
2.25.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [3/4] drm/i915/huc: Prepare for GSC-loaded HuC
2022-04-27 0:26 ` [Intel-gfx] " Daniele Ceraolo Spurio
@ 2022-04-29 18:39 ` Teres Alexis, Alan Previn
-1 siblings, 0 replies; 22+ messages in thread
From: Teres Alexis, Alan Previn @ 2022-04-29 18:39 UTC (permalink / raw)
To: Ceraolo Spurio, Daniele, intel-gfx; +Cc: dri-devel
Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>
On Tue, 2022-04-26 at 17:26 -0700, Daniele Ceraolo Spurio wrote:
> HuC loading via GSC is performed via a PXP command sent through the mei
> modules, so we need both MEI_GSC and MEI_PXP to be available. Given that
> the GSC will do both the transfer and the authentication, the legacy HuC
> loading paths can be safely skipped.
> Also note that the GSC-loaded HuC survives GT reset.
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
> drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h | 1 +
> drivers/gpu/drm/i915/gt/uc/intel_huc.c | 76 +++++++++++++++++++---
> drivers/gpu/drm/i915/gt/uc/intel_huc.h | 6 ++
> drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c | 5 +-
> drivers/gpu/drm/i915/gt/uc/intel_uc.c | 11 +++-
> 5 files changed, 88 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h
> index 66027a42cda9e..2516705b9f365 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h
> @@ -96,6 +96,7 @@
>
> #define GUC_SHIM_CONTROL2 _MMIO(0xc068)
> #define GUC_IS_PRIVILEGED (1<<29)
> +#define GSC_LOADS_HUC (1<<30)
>
> #define GUC_SEND_INTERRUPT _MMIO(0xc4c8)
> #define GUC_SEND_TRIGGER (1<<0)
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> index 773020e69589a..76a7df7f136fc 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> @@ -6,6 +6,7 @@
> #include <linux/types.h>
>
> #include "gt/intel_gt.h"
> +#include "intel_guc_reg.h"
> #include "intel_huc.h"
> #include "i915_drv.h"
>
> @@ -17,11 +18,15 @@
> * 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. For
> - * The GuC to correctly perform the authentication, 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.
> + * 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.
> + * 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.
> *
> * See https://github.com/intel/media-driver for the latest details on HuC
> * functionality.
> @@ -54,11 +59,51 @@ void intel_huc_init_early(struct intel_huc *huc)
> }
> }
>
> +#define HUC_LOAD_MODE_STRING(x) (x ? "GSC" : "legacy")
> +static int check_huc_loading_mode(struct intel_huc *huc)
> +{
> + struct intel_gt *gt = huc_to_gt(huc);
> + bool fw_needs_gsc = intel_huc_is_loaded_by_gsc(huc);
> + bool hw_uses_gsc = false;
> +
> + /*
> + * The fuse for HuC load via GSC is only valid on platforms that have
> + * GuC deprivilege.
> + */
> + if (HAS_GUC_DEPRIVILEGE(gt->i915))
> + hw_uses_gsc = intel_uncore_read(gt->uncore, GUC_SHIM_CONTROL2) &
> + GSC_LOADS_HUC;
> +
> + if (fw_needs_gsc != hw_uses_gsc) {
> + drm_err(>->i915->drm,
> + "mismatch between HuC FW (%s) and HW (%s) load modes\n",
> + HUC_LOAD_MODE_STRING(fw_needs_gsc),
> + HUC_LOAD_MODE_STRING(hw_uses_gsc));
> + return -ENOEXEC;
> + }
> +
> + /* make sure we can access the GSC via the mei driver if we need it */
> + if (!(IS_ENABLED(CONFIG_INTEL_MEI_PXP) && IS_ENABLED(CONFIG_INTEL_MEI_GSC)) &&
> + fw_needs_gsc) {
> + drm_info(>->i915->drm,
> + "Can't load HuC due to missing MEI modules\n");
> + return -EIO;
> + }
> +
> + drm_dbg(>->i915->drm, "GSC loads huc=%s\n", str_yes_no(fw_needs_gsc));
> +
> + return 0;
> +}
> +
> int intel_huc_init(struct intel_huc *huc)
> {
> struct drm_i915_private *i915 = huc_to_gt(huc)->i915;
> int err;
>
> + err = check_huc_loading_mode(huc);
> + if (err)
> + goto out;
> +
> err = intel_uc_fw_init(&huc->fw);
> if (err)
> goto out;
> @@ -108,17 +153,20 @@ int intel_huc_auth(struct intel_huc *huc)
> struct intel_guc *guc = >->uc.guc;
> int ret;
>
> - GEM_BUG_ON(huc_is_authenticated(huc));
> -
> if (!intel_uc_fw_is_loaded(&huc->fw))
> return -ENOEXEC;
>
> + /* GSC will do the auth */
> + if (intel_huc_is_loaded_by_gsc(huc))
> + return -ENODEV;
> +
> ret = i915_inject_probe_error(gt->i915, -ENXIO);
> if (ret)
> goto fail;
>
> - ret = intel_guc_auth_huc(guc,
> - intel_guc_ggtt_offset(guc, huc->fw.rsa_data));
> + GEM_BUG_ON(huc_is_authenticated(huc));
> +
> + ret = intel_guc_auth_huc(guc, intel_guc_ggtt_offset(guc, huc->fw.rsa_data));
> if (ret) {
> DRM_ERROR("HuC: GuC did not ack Auth request %d\n", ret);
> goto fail;
> @@ -178,6 +226,16 @@ int intel_huc_check_status(struct intel_huc *huc)
> return huc_is_authenticated(huc);
> }
>
> +void intel_huc_update_auth_status(struct intel_huc *huc)
> +{
> + if (!intel_uc_fw_is_loadable(&huc->fw))
> + return;
> +
> + if (huc_is_authenticated(huc))
> + intel_uc_fw_change_status(&huc->fw,
> + INTEL_UC_FIRMWARE_RUNNING);
> +}
> +
> /**
> * intel_huc_load_status - dump information about HuC load status
> * @huc: the HuC
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.h b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
> index 77d813840d76c..d7e25b6e879eb 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
> @@ -27,6 +27,7 @@ int intel_huc_init(struct intel_huc *huc);
> void intel_huc_fini(struct intel_huc *huc);
> int intel_huc_auth(struct intel_huc *huc);
> int intel_huc_check_status(struct intel_huc *huc);
> +void intel_huc_update_auth_status(struct intel_huc *huc);
>
> static inline int intel_huc_sanitize(struct intel_huc *huc)
> {
> @@ -50,6 +51,11 @@ static inline bool intel_huc_is_used(struct intel_huc *huc)
> return intel_uc_fw_is_available(&huc->fw);
> }
>
> +static inline bool intel_huc_is_loaded_by_gsc(const struct intel_huc *huc)
> +{
> + return huc->fw.loaded_via_gsc;
> +}
> +
> void intel_huc_load_status(struct intel_huc *huc, struct drm_printer *p);
>
> #endif
> 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 e5ef509c70e89..9d6ab1e016395 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
> @@ -8,7 +8,7 @@
> #include "i915_drv.h"
>
> /**
> - * intel_huc_fw_upload() - load HuC uCode to device
> + * intel_huc_fw_upload() - load HuC uCode to device via DMA transfer
> * @huc: intel_huc structure
> *
> * Called from intel_uc_init_hw() during driver load, resume from sleep and
> @@ -21,6 +21,9 @@
> */
> int intel_huc_fw_upload(struct intel_huc *huc)
> {
> + if (intel_huc_is_loaded_by_gsc(huc))
> + return -ENODEV;
> +
> /* HW doesn't look at destination address for HuC, so set it to 0 */
> return intel_uc_fw_upload(&huc->fw, 0, HUC_UKERNEL);
> }
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> index 8c9ef690ac9d8..0dce94f896a8c 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> @@ -509,7 +509,16 @@ static int __uc_init_hw(struct intel_uc *uc)
> if (ret)
> goto err_log_capture;
>
> - intel_huc_auth(huc);
> + /*
> + * GSC-loaded HuC is authenticated by the GSC, so we don't need to
> + * trigger the auth here. However, given that the HuC loaded this way
> + * survive GT reset, we still need to update our SW bookkeeping to make
> + * sure it reflects the correct HW status.
> + */
> + if (intel_huc_is_loaded_by_gsc(huc))
> + intel_huc_update_auth_status(huc);
> + else
> + intel_huc_auth(huc);
>
> if (intel_uc_uses_guc_submission(uc))
> intel_guc_submission_enable(guc);
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [Intel-gfx] [3/4] drm/i915/huc: Prepare for GSC-loaded HuC
@ 2022-04-29 18:39 ` Teres Alexis, Alan Previn
0 siblings, 0 replies; 22+ messages in thread
From: Teres Alexis, Alan Previn @ 2022-04-29 18:39 UTC (permalink / raw)
To: Ceraolo Spurio, Daniele, intel-gfx; +Cc: dri-devel
Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>
On Tue, 2022-04-26 at 17:26 -0700, Daniele Ceraolo Spurio wrote:
> HuC loading via GSC is performed via a PXP command sent through the mei
> modules, so we need both MEI_GSC and MEI_PXP to be available. Given that
> the GSC will do both the transfer and the authentication, the legacy HuC
> loading paths can be safely skipped.
> Also note that the GSC-loaded HuC survives GT reset.
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
> drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h | 1 +
> drivers/gpu/drm/i915/gt/uc/intel_huc.c | 76 +++++++++++++++++++---
> drivers/gpu/drm/i915/gt/uc/intel_huc.h | 6 ++
> drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c | 5 +-
> drivers/gpu/drm/i915/gt/uc/intel_uc.c | 11 +++-
> 5 files changed, 88 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h
> index 66027a42cda9e..2516705b9f365 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h
> @@ -96,6 +96,7 @@
>
> #define GUC_SHIM_CONTROL2 _MMIO(0xc068)
> #define GUC_IS_PRIVILEGED (1<<29)
> +#define GSC_LOADS_HUC (1<<30)
>
> #define GUC_SEND_INTERRUPT _MMIO(0xc4c8)
> #define GUC_SEND_TRIGGER (1<<0)
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> index 773020e69589a..76a7df7f136fc 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> @@ -6,6 +6,7 @@
> #include <linux/types.h>
>
> #include "gt/intel_gt.h"
> +#include "intel_guc_reg.h"
> #include "intel_huc.h"
> #include "i915_drv.h"
>
> @@ -17,11 +18,15 @@
> * 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. For
> - * The GuC to correctly perform the authentication, 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.
> + * 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.
> + * 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.
> *
> * See https://github.com/intel/media-driver for the latest details on HuC
> * functionality.
> @@ -54,11 +59,51 @@ void intel_huc_init_early(struct intel_huc *huc)
> }
> }
>
> +#define HUC_LOAD_MODE_STRING(x) (x ? "GSC" : "legacy")
> +static int check_huc_loading_mode(struct intel_huc *huc)
> +{
> + struct intel_gt *gt = huc_to_gt(huc);
> + bool fw_needs_gsc = intel_huc_is_loaded_by_gsc(huc);
> + bool hw_uses_gsc = false;
> +
> + /*
> + * The fuse for HuC load via GSC is only valid on platforms that have
> + * GuC deprivilege.
> + */
> + if (HAS_GUC_DEPRIVILEGE(gt->i915))
> + hw_uses_gsc = intel_uncore_read(gt->uncore, GUC_SHIM_CONTROL2) &
> + GSC_LOADS_HUC;
> +
> + if (fw_needs_gsc != hw_uses_gsc) {
> + drm_err(>->i915->drm,
> + "mismatch between HuC FW (%s) and HW (%s) load modes\n",
> + HUC_LOAD_MODE_STRING(fw_needs_gsc),
> + HUC_LOAD_MODE_STRING(hw_uses_gsc));
> + return -ENOEXEC;
> + }
> +
> + /* make sure we can access the GSC via the mei driver if we need it */
> + if (!(IS_ENABLED(CONFIG_INTEL_MEI_PXP) && IS_ENABLED(CONFIG_INTEL_MEI_GSC)) &&
> + fw_needs_gsc) {
> + drm_info(>->i915->drm,
> + "Can't load HuC due to missing MEI modules\n");
> + return -EIO;
> + }
> +
> + drm_dbg(>->i915->drm, "GSC loads huc=%s\n", str_yes_no(fw_needs_gsc));
> +
> + return 0;
> +}
> +
> int intel_huc_init(struct intel_huc *huc)
> {
> struct drm_i915_private *i915 = huc_to_gt(huc)->i915;
> int err;
>
> + err = check_huc_loading_mode(huc);
> + if (err)
> + goto out;
> +
> err = intel_uc_fw_init(&huc->fw);
> if (err)
> goto out;
> @@ -108,17 +153,20 @@ int intel_huc_auth(struct intel_huc *huc)
> struct intel_guc *guc = >->uc.guc;
> int ret;
>
> - GEM_BUG_ON(huc_is_authenticated(huc));
> -
> if (!intel_uc_fw_is_loaded(&huc->fw))
> return -ENOEXEC;
>
> + /* GSC will do the auth */
> + if (intel_huc_is_loaded_by_gsc(huc))
> + return -ENODEV;
> +
> ret = i915_inject_probe_error(gt->i915, -ENXIO);
> if (ret)
> goto fail;
>
> - ret = intel_guc_auth_huc(guc,
> - intel_guc_ggtt_offset(guc, huc->fw.rsa_data));
> + GEM_BUG_ON(huc_is_authenticated(huc));
> +
> + ret = intel_guc_auth_huc(guc, intel_guc_ggtt_offset(guc, huc->fw.rsa_data));
> if (ret) {
> DRM_ERROR("HuC: GuC did not ack Auth request %d\n", ret);
> goto fail;
> @@ -178,6 +226,16 @@ int intel_huc_check_status(struct intel_huc *huc)
> return huc_is_authenticated(huc);
> }
>
> +void intel_huc_update_auth_status(struct intel_huc *huc)
> +{
> + if (!intel_uc_fw_is_loadable(&huc->fw))
> + return;
> +
> + if (huc_is_authenticated(huc))
> + intel_uc_fw_change_status(&huc->fw,
> + INTEL_UC_FIRMWARE_RUNNING);
> +}
> +
> /**
> * intel_huc_load_status - dump information about HuC load status
> * @huc: the HuC
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.h b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
> index 77d813840d76c..d7e25b6e879eb 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
> @@ -27,6 +27,7 @@ int intel_huc_init(struct intel_huc *huc);
> void intel_huc_fini(struct intel_huc *huc);
> int intel_huc_auth(struct intel_huc *huc);
> int intel_huc_check_status(struct intel_huc *huc);
> +void intel_huc_update_auth_status(struct intel_huc *huc);
>
> static inline int intel_huc_sanitize(struct intel_huc *huc)
> {
> @@ -50,6 +51,11 @@ static inline bool intel_huc_is_used(struct intel_huc *huc)
> return intel_uc_fw_is_available(&huc->fw);
> }
>
> +static inline bool intel_huc_is_loaded_by_gsc(const struct intel_huc *huc)
> +{
> + return huc->fw.loaded_via_gsc;
> +}
> +
> void intel_huc_load_status(struct intel_huc *huc, struct drm_printer *p);
>
> #endif
> 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 e5ef509c70e89..9d6ab1e016395 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
> @@ -8,7 +8,7 @@
> #include "i915_drv.h"
>
> /**
> - * intel_huc_fw_upload() - load HuC uCode to device
> + * intel_huc_fw_upload() - load HuC uCode to device via DMA transfer
> * @huc: intel_huc structure
> *
> * Called from intel_uc_init_hw() during driver load, resume from sleep and
> @@ -21,6 +21,9 @@
> */
> int intel_huc_fw_upload(struct intel_huc *huc)
> {
> + if (intel_huc_is_loaded_by_gsc(huc))
> + return -ENODEV;
> +
> /* HW doesn't look at destination address for HuC, so set it to 0 */
> return intel_uc_fw_upload(&huc->fw, 0, HUC_UKERNEL);
> }
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> index 8c9ef690ac9d8..0dce94f896a8c 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> @@ -509,7 +509,16 @@ static int __uc_init_hw(struct intel_uc *uc)
> if (ret)
> goto err_log_capture;
>
> - intel_huc_auth(huc);
> + /*
> + * GSC-loaded HuC is authenticated by the GSC, so we don't need to
> + * trigger the auth here. However, given that the HuC loaded this way
> + * survive GT reset, we still need to update our SW bookkeeping to make
> + * sure it reflects the correct HW status.
> + */
> + if (intel_huc_is_loaded_by_gsc(huc))
> + intel_huc_update_auth_status(huc);
> + else
> + intel_huc_auth(huc);
>
> if (intel_uc_uses_guc_submission(uc))
> intel_guc_submission_enable(guc);
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 4/4] drm/i915/huc: Don't fail the probe if HuC init fails
2022-04-27 0:26 ` [Intel-gfx] " Daniele Ceraolo Spurio
@ 2022-04-27 0:26 ` Daniele Ceraolo Spurio
-1 siblings, 0 replies; 22+ messages in thread
From: Daniele Ceraolo Spurio @ 2022-04-27 0:26 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniele Ceraolo Spurio, dri-devel
The previous patch introduced new failure cases in the HuC init flow
that can be hit by simply changing the config, so we want to avoid
failing the probe in those scenarios. HuC load failure is already
considered a non-fatal error and we have a way to report to userspace
if the HuC is not available via a dedicated getparam, so no changes
in expectation there.
The error message in the HuC init code has also been lowered to info to
avoid throwing error message for an expected behavior.
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
drivers/gpu/drm/i915/gt/uc/intel_huc.c | 2 +-
drivers/gpu/drm/i915/gt/uc/intel_uc.c | 11 ++---------
2 files changed, 3 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
index 76a7df7f136fc..3d2e7a6d7c1b7 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
@@ -113,7 +113,7 @@ int intel_huc_init(struct intel_huc *huc)
return 0;
out:
- i915_probe_error(i915, "failed with %d\n", err);
+ drm_info(&i915->drm, "HuC init failed with %d\n", err);
return err;
}
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index 0dce94f896a8c..ecf149c5fdb02 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -323,17 +323,10 @@ static int __uc_init(struct intel_uc *uc)
if (ret)
return ret;
- if (intel_uc_uses_huc(uc)) {
- ret = intel_huc_init(huc);
- if (ret)
- goto out_guc;
- }
+ if (intel_uc_uses_huc(uc))
+ intel_huc_init(huc);
return 0;
-
-out_guc:
- intel_guc_fini(guc);
- return ret;
}
static void __uc_fini(struct intel_uc *uc)
--
2.25.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [Intel-gfx] [PATCH 4/4] drm/i915/huc: Don't fail the probe if HuC init fails
@ 2022-04-27 0:26 ` Daniele Ceraolo Spurio
0 siblings, 0 replies; 22+ messages in thread
From: Daniele Ceraolo Spurio @ 2022-04-27 0:26 UTC (permalink / raw)
To: intel-gfx; +Cc: dri-devel
The previous patch introduced new failure cases in the HuC init flow
that can be hit by simply changing the config, so we want to avoid
failing the probe in those scenarios. HuC load failure is already
considered a non-fatal error and we have a way to report to userspace
if the HuC is not available via a dedicated getparam, so no changes
in expectation there.
The error message in the HuC init code has also been lowered to info to
avoid throwing error message for an expected behavior.
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
drivers/gpu/drm/i915/gt/uc/intel_huc.c | 2 +-
drivers/gpu/drm/i915/gt/uc/intel_uc.c | 11 ++---------
2 files changed, 3 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
index 76a7df7f136fc..3d2e7a6d7c1b7 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
@@ -113,7 +113,7 @@ int intel_huc_init(struct intel_huc *huc)
return 0;
out:
- i915_probe_error(i915, "failed with %d\n", err);
+ drm_info(&i915->drm, "HuC init failed with %d\n", err);
return err;
}
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index 0dce94f896a8c..ecf149c5fdb02 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -323,17 +323,10 @@ static int __uc_init(struct intel_uc *uc)
if (ret)
return ret;
- if (intel_uc_uses_huc(uc)) {
- ret = intel_huc_init(huc);
- if (ret)
- goto out_guc;
- }
+ if (intel_uc_uses_huc(uc))
+ intel_huc_init(huc);
return 0;
-
-out_guc:
- intel_guc_fini(guc);
- return ret;
}
static void __uc_fini(struct intel_uc *uc)
--
2.25.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [Intel-gfx] [PATCH 4/4] drm/i915/huc: Don't fail the probe if HuC init fails
2022-04-27 0:26 ` [Intel-gfx] " Daniele Ceraolo Spurio
(?)
@ 2022-04-27 12:42 ` Rodrigo Vivi
-1 siblings, 0 replies; 22+ messages in thread
From: Rodrigo Vivi @ 2022-04-27 12:42 UTC (permalink / raw)
To: Daniele Ceraolo Spurio; +Cc: intel-gfx, dri-devel
On Tue, Apr 26, 2022 at 05:26:17PM -0700, Daniele Ceraolo Spurio wrote:
> The previous patch introduced new failure cases in the HuC init flow
> that can be hit by simply changing the config, so we want to avoid
> failing the probe in those scenarios. HuC load failure is already
> considered a non-fatal error and we have a way to report to userspace
> if the HuC is not available via a dedicated getparam, so no changes
> in expectation there.
> The error message in the HuC init code has also been lowered to info to
> avoid throwing error message for an expected behavior.
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
> drivers/gpu/drm/i915/gt/uc/intel_huc.c | 2 +-
> drivers/gpu/drm/i915/gt/uc/intel_uc.c | 11 ++---------
> 2 files changed, 3 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> index 76a7df7f136fc..3d2e7a6d7c1b7 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> @@ -113,7 +113,7 @@ int intel_huc_init(struct intel_huc *huc)
> return 0;
>
> out:
> - i915_probe_error(i915, "failed with %d\n", err);
> + drm_info(&i915->drm, "HuC init failed with %d\n", err);
> return err;
> }
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> index 0dce94f896a8c..ecf149c5fdb02 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> @@ -323,17 +323,10 @@ static int __uc_init(struct intel_uc *uc)
> if (ret)
> return ret;
>
> - if (intel_uc_uses_huc(uc)) {
> - ret = intel_huc_init(huc);
> - if (ret)
> - goto out_guc;
> - }
> + if (intel_uc_uses_huc(uc))
> + intel_huc_init(huc);
>
> return 0;
> -
> -out_guc:
> - intel_guc_fini(guc);
> - return ret;
> }
>
> static void __uc_fini(struct intel_uc *uc)
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Prepare for GSC-loaded HuC
2022-04-27 0:26 ` [Intel-gfx] " Daniele Ceraolo Spurio
` (4 preceding siblings ...)
(?)
@ 2022-04-27 0:39 ` Patchwork
-1 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2022-04-27 0:39 UTC (permalink / raw)
To: Daniele Ceraolo Spurio; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Prepare for GSC-loaded HuC
URL : https://patchwork.freedesktop.org/series/103186/
State : warning
== Summary ==
Error: dim checkpatch failed
8916eb4e3081 drm/i915/huc: check HW directly for HuC auth status
0cbbd92e569f drm/i915/huc: Add fetch support for gsc-loaded HuC binary
f17e836d85dc drm/i915/huc: Prepare for GSC-loaded HuC
-:22: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#22: FILE: drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h:99:
+#define GSC_LOADS_HUC (1<<30)
^
total: 0 errors, 0 warnings, 1 checks, 177 lines checked
120b114272a8 drm/i915/huc: Don't fail the probe if HuC init fails
^ permalink raw reply [flat|nested] 22+ messages in thread* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915: Prepare for GSC-loaded HuC
2022-04-27 0:26 ` [Intel-gfx] " Daniele Ceraolo Spurio
` (5 preceding siblings ...)
(?)
@ 2022-04-27 0:39 ` Patchwork
-1 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2022-04-27 0:39 UTC (permalink / raw)
To: Daniele Ceraolo Spurio; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Prepare for GSC-loaded HuC
URL : https://patchwork.freedesktop.org/series/103186/
State : warning
== Summary ==
Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.
^ permalink raw reply [flat|nested] 22+ messages in thread* [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Prepare for GSC-loaded HuC
2022-04-27 0:26 ` [Intel-gfx] " Daniele Ceraolo Spurio
` (6 preceding siblings ...)
(?)
@ 2022-04-27 1:05 ` Patchwork
-1 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2022-04-27 1:05 UTC (permalink / raw)
To: Daniele Ceraolo Spurio; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 11554 bytes --]
== Series Details ==
Series: drm/i915: Prepare for GSC-loaded HuC
URL : https://patchwork.freedesktop.org/series/103186/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_11550 -> Patchwork_103186v1
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_103186v1 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_103186v1, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103186v1/index.html
Participating hosts (43 -> 44)
------------------------------
Additional (2): fi-kbl-x1275 bat-dg1-6
Missing (1): fi-bsw-cyan
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_103186v1:
### IGT changes ###
#### Possible regressions ####
* igt@gem_exec_suspend@basic-s0@smem:
- fi-skl-guc: [PASS][1] -> [INCOMPLETE][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-skl-guc/igt@gem_exec_suspend@basic-s0@smem.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103186v1/fi-skl-guc/igt@gem_exec_suspend@basic-s0@smem.html
#### Warnings ####
* igt@debugfs_test@read_all_entries:
- fi-apl-guc: [DMESG-WARN][3] ([i915#5595]) -> [DMESG-WARN][4]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-apl-guc/igt@debugfs_test@read_all_entries.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103186v1/fi-apl-guc/igt@debugfs_test@read_all_entries.html
Known issues
------------
Here are the changes found in Patchwork_103186v1 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_exec_suspend@basic-s0@smem:
- fi-kbl-guc: [PASS][5] -> [INCOMPLETE][6] ([i915#4831])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-kbl-guc/igt@gem_exec_suspend@basic-s0@smem.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103186v1/fi-kbl-guc/igt@gem_exec_suspend@basic-s0@smem.html
- bat-dg1-6: NOTRUN -> [INCOMPLETE][7] ([i915#5827])
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103186v1/bat-dg1-6/igt@gem_exec_suspend@basic-s0@smem.html
- fi-cfl-guc: [PASS][8] -> [INCOMPLETE][9] ([i915#4831])
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-cfl-guc/igt@gem_exec_suspend@basic-s0@smem.html
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103186v1/fi-cfl-guc/igt@gem_exec_suspend@basic-s0@smem.html
* igt@gem_huc_copy@huc-copy:
- fi-kbl-x1275: NOTRUN -> [SKIP][10] ([fdo#109271] / [i915#2190])
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103186v1/fi-kbl-x1275/igt@gem_huc_copy@huc-copy.html
* igt@gem_lmem_swapping@verify-random:
- fi-kbl-x1275: NOTRUN -> [SKIP][11] ([fdo#109271] / [i915#4613]) +3 similar issues
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103186v1/fi-kbl-x1275/igt@gem_lmem_swapping@verify-random.html
* igt@i915_selftest@live@mman:
- fi-bdw-5557u: NOTRUN -> [INCOMPLETE][12] ([i915#5704])
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103186v1/fi-bdw-5557u/igt@i915_selftest@live@mman.html
* igt@kms_chamelium@dp-crc-fast:
- fi-bdw-5557u: NOTRUN -> [SKIP][13] ([fdo#109271] / [fdo#111827]) +8 similar issues
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103186v1/fi-bdw-5557u/igt@kms_chamelium@dp-crc-fast.html
* igt@kms_chamelium@hdmi-crc-fast:
- fi-kbl-x1275: NOTRUN -> [SKIP][14] ([fdo#109271] / [fdo#111827]) +8 similar issues
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103186v1/fi-kbl-x1275/igt@kms_chamelium@hdmi-crc-fast.html
* igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d:
- fi-kbl-x1275: NOTRUN -> [SKIP][15] ([fdo#109271] / [i915#533])
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103186v1/fi-kbl-x1275/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d.html
* igt@kms_psr@primary_page_flip:
- fi-kbl-x1275: NOTRUN -> [SKIP][16] ([fdo#109271]) +12 similar issues
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103186v1/fi-kbl-x1275/igt@kms_psr@primary_page_flip.html
* igt@kms_setmode@basic-clone-single-crtc:
- fi-bdw-5557u: NOTRUN -> [SKIP][17] ([fdo#109271]) +14 similar issues
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103186v1/fi-bdw-5557u/igt@kms_setmode@basic-clone-single-crtc.html
#### Possible fixes ####
* igt@i915_selftest@live@hangcheck:
- fi-snb-2600: [INCOMPLETE][18] ([i915#3921]) -> [PASS][19]
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-snb-2600/igt@i915_selftest@live@hangcheck.html
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103186v1/fi-snb-2600/igt@i915_selftest@live@hangcheck.html
* igt@kms_busy@basic@flip:
- {bat-adlp-6}: [DMESG-WARN][20] ([i915#3576]) -> [PASS][21]
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/bat-adlp-6/igt@kms_busy@basic@flip.html
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103186v1/bat-adlp-6/igt@kms_busy@basic@flip.html
#### Warnings ####
* igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c:
- fi-kbl-7567u: [SKIP][22] ([fdo#109271] / [i915#5341]) -> [SKIP][23] ([fdo#109271])
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-kbl-7567u/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103186v1/fi-kbl-7567u/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
- fi-pnv-d510: [SKIP][24] ([fdo#109271] / [i915#5341]) -> [SKIP][25] ([fdo#109271])
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-pnv-d510/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103186v1/fi-pnv-d510/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
- fi-snb-2520m: [SKIP][26] ([fdo#109271] / [i915#5341]) -> [SKIP][27] ([fdo#109271])
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-snb-2520m/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103186v1/fi-snb-2520m/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
- fi-bsw-kefka: [SKIP][28] ([fdo#109271] / [i915#5341]) -> [SKIP][29] ([fdo#109271])
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-bsw-kefka/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103186v1/fi-bsw-kefka/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
- fi-kbl-8809g: [SKIP][30] ([fdo#109271] / [i915#5341]) -> [SKIP][31] ([fdo#109271])
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-kbl-8809g/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103186v1/fi-kbl-8809g/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
- fi-bsw-nick: [SKIP][32] ([fdo#109271] / [i915#5341]) -> [SKIP][33] ([fdo#109271])
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-bsw-nick/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103186v1/fi-bsw-nick/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
- fi-bwr-2160: [SKIP][34] ([fdo#109271] / [i915#5341]) -> [SKIP][35] ([fdo#109271])
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-bwr-2160/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103186v1/fi-bwr-2160/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
- fi-snb-2600: [SKIP][36] ([fdo#109271] / [i915#5341]) -> [SKIP][37] ([fdo#109271])
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-snb-2600/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103186v1/fi-snb-2600/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
- fi-elk-e7500: [SKIP][38] ([fdo#109271] / [i915#5341]) -> [SKIP][39] ([fdo#109271])
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-elk-e7500/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103186v1/fi-elk-e7500/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
- fi-blb-e6850: [SKIP][40] ([fdo#109271] / [i915#5341]) -> [SKIP][41] ([fdo#109271])
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-blb-e6850/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103186v1/fi-blb-e6850/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
- fi-ilk-650: [SKIP][42] ([fdo#109271] / [i915#5341]) -> [SKIP][43] ([fdo#109271])
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-ilk-650/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103186v1/fi-ilk-650/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
[i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
[i915#3576]: https://gitlab.freedesktop.org/drm/intel/issues/3576
[i915#3921]: https://gitlab.freedesktop.org/drm/intel/issues/3921
[i915#4391]: https://gitlab.freedesktop.org/drm/intel/issues/4391
[i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
[i915#4831]: https://gitlab.freedesktop.org/drm/intel/issues/4831
[i915#5329]: https://gitlab.freedesktop.org/drm/intel/issues/5329
[i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533
[i915#5341]: https://gitlab.freedesktop.org/drm/intel/issues/5341
[i915#5401]: https://gitlab.freedesktop.org/drm/intel/issues/5401
[i915#5537]: https://gitlab.freedesktop.org/drm/intel/issues/5537
[i915#5595]: https://gitlab.freedesktop.org/drm/intel/issues/5595
[i915#5704]: https://gitlab.freedesktop.org/drm/intel/issues/5704
[i915#5827]: https://gitlab.freedesktop.org/drm/intel/issues/5827
Build changes
-------------
* Linux: CI_DRM_11550 -> Patchwork_103186v1
CI-20190529: 20190529
CI_DRM_11550: 56b089ae03ef8ea8ab7f474eaa70367898891ef0 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_6451: f055bd83bd831a938d639718c2359516224f15f9 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_103186v1: 56b089ae03ef8ea8ab7f474eaa70367898891ef0 @ git://anongit.freedesktop.org/gfx-ci/linux
### Linux commits
b392f1399947 drm/i915/huc: Don't fail the probe if HuC init fails
67bdab8536ae drm/i915/huc: Prepare for GSC-loaded HuC
f373894825d9 drm/i915/huc: Add fetch support for gsc-loaded HuC binary
473e7c0f9a5e drm/i915/huc: check HW directly for HuC auth status
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103186v1/index.html
[-- Attachment #2: Type: text/html, Size: 15036 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread