Re: [Intel-gfx] [PATCH v2 5/8] drm/i915/huc: differentiate the 2 steps of the MTL HuC auth flow

2023-05-19 Thread Ceraolo Spurio, Daniele 




On 5/19/2023 11:45 AM, John Harrison wrote:

On 4/28/2023 11:58, Daniele Ceraolo Spurio wrote:

Before we add the second step of the MTL HuC auth (via GSC), we need to
have the ability to differentiate between them. To do so, the huc
authentication check is duplicated for GuC and GSC auth, with meu
binaries being considered fully authenticated only after the GSC auth
step.

To report the difference between the 2 auth steps, a new case is added
to the HuC getparam. This way, the clear media driver can start
submitting before full auth, as partial auth is enough for those
workloads.

v2: fix authentication status check for DG2

Signed-off-by: Daniele Ceraolo Spurio 
Cc: Alan Previn 
---
  drivers/gpu/drm/i915/gt/uc/intel_huc.c    | 94 +--
  drivers/gpu/drm/i915/gt/uc/intel_huc.h    | 16 +++-
  drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c |  4 +-
  drivers/gpu/drm/i915/i915_reg.h   |  3 +
  include/uapi/drm/i915_drm.h   |  3 +-
  5 files changed, 91 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c 
b/drivers/gpu/drm/i915/gt/uc/intel_huc.c

index c189ede4ef55..60f95d98e5fd 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
@@ -10,6 +10,7 @@
  #include "intel_huc.h"
  #include "intel_huc_print.h"
  #include "i915_drv.h"
+#include "i915_reg.h"
    #include 
  #include 
@@ -106,7 +107,7 @@ static enum hrtimer_restart 
huc_delayed_load_timer_callback(struct hrtimer *hrti

  {
  struct intel_huc *huc = container_of(hrtimer, struct intel_huc, 
delayed_load.timer);

  -    if (!intel_huc_is_authenticated(huc)) {
+    if (!intel_huc_is_authenticated(huc, INTEL_HUC_AUTH_BY_GSC)) {
  if (huc->delayed_load.status == INTEL_HUC_WAITING_ON_GSC)
  huc_notice(huc, "timed out waiting for MEI GSC\n");
  else if (huc->delayed_load.status == INTEL_HUC_WAITING_ON_PXP)
@@ -124,7 +125,7 @@ static void huc_delayed_load_start(struct 
intel_huc *huc)

  {
  ktime_t delay;
  -    GEM_BUG_ON(intel_huc_is_authenticated(huc));
+    GEM_BUG_ON(intel_huc_is_authenticated(huc, INTEL_HUC_AUTH_BY_GSC));
    /*
   * On resume we don't have to wait for MEI-GSC to be re-probed, 
but we

@@ -284,13 +285,23 @@ void intel_huc_init_early(struct intel_huc *huc)
  }
    if (GRAPHICS_VER(i915) >= 11) {
-    huc->status.reg = GEN11_HUC_KERNEL_LOAD_INFO;
-    huc->status.mask = HUC_LOAD_SUCCESSFUL;
-    huc->status.value = HUC_LOAD_SUCCESSFUL;
+    huc->status[INTEL_HUC_AUTH_BY_GUC].reg = 
GEN11_HUC_KERNEL_LOAD_INFO;

+    huc->status[INTEL_HUC_AUTH_BY_GUC].mask = HUC_LOAD_SUCCESSFUL;
+    huc->status[INTEL_HUC_AUTH_BY_GUC].value = HUC_LOAD_SUCCESSFUL;
  } else {
-    huc->status.reg = HUC_STATUS2;
-    huc->status.mask = HUC_FW_VERIFIED;
-    huc->status.value = HUC_FW_VERIFIED;
+    huc->status[INTEL_HUC_AUTH_BY_GUC].reg = HUC_STATUS2;
+    huc->status[INTEL_HUC_AUTH_BY_GUC].mask = HUC_FW_VERIFIED;
+    huc->status[INTEL_HUC_AUTH_BY_GUC].value = HUC_FW_VERIFIED;
+    }
+
+    if (IS_DG2(i915)) {
+    huc->status[INTEL_HUC_AUTH_BY_GSC].reg = 
GEN11_HUC_KERNEL_LOAD_INFO;

+    huc->status[INTEL_HUC_AUTH_BY_GSC].mask = HUC_LOAD_SUCCESSFUL;
+    huc->status[INTEL_HUC_AUTH_BY_GSC].value = HUC_LOAD_SUCCESSFUL;
+    } else {
+    huc->status[INTEL_HUC_AUTH_BY_GSC].reg = 
HECI_FWSTS5(MTL_GSC_HECI1_BASE);
+    huc->status[INTEL_HUC_AUTH_BY_GSC].mask = 
HECI_FWSTS5_HUC_AUTH_DONE;
+    huc->status[INTEL_HUC_AUTH_BY_GSC].value = 
HECI_FWSTS5_HUC_AUTH_DONE;

  }
  }
  @@ -381,28 +392,39 @@ void intel_huc_suspend(struct intel_huc *huc)
  delayed_huc_load_complete(huc);
  }
  -int intel_huc_wait_for_auth_complete(struct intel_huc *huc)
+static const char *auth_mode_string(struct intel_huc *huc,
+    enum intel_huc_authentication_type type)
+{
+    bool partial = !huc->loaded_via_gsc && huc->fw.is_meu_binary &&
+   type == INTEL_HUC_AUTH_BY_GUC;

partial = !loaded_via_gsc?

If it is not a GSC load then there is no two stage authentication, is 
there? Does that mean the single stage auth does not count as 'all 
workloads' even on platforms where two stage is not supported?


Single step authentication always counts as "all workloads". The auth is 
partial only if this is a DMA (i.e. non-gsc) load with a gsc-enabled 
binary and we're doing an auth via GuC, which is what the condition 
above is checking.





+
+    return partial ? "clear media" : "all workloads";
+}
+
+int intel_huc_wait_for_auth_complete(struct intel_huc *huc,
+ enum intel_huc_authentication_type type)
  {
  struct intel_gt *gt = huc_to_gt(huc);
  int ret;
    ret = __intel_wait_for_register(gt->uncore,
-    huc->status.reg,
-    huc->status.mask,
-    huc->status.value,
+    huc->status[type].reg,
+   

Re: [Intel-gfx] [PATCH v2 5/8] drm/i915/huc: differentiate the 2 steps of the MTL HuC auth flow

2023-05-19 Thread John Harrison 

On 4/28/2023 11:58, Daniele Ceraolo Spurio wrote:

Before we add the second step of the MTL HuC auth (via GSC), we need to
have the ability to differentiate between them. To do so, the huc
authentication check is duplicated for GuC and GSC auth, with meu
binaries being considered fully authenticated only after the GSC auth
step.

To report the difference between the 2 auth steps, a new case is added
to the HuC getparam. This way, the clear media driver can start
submitting before full auth, as partial auth is enough for those
workloads.

v2: fix authentication status check for DG2

Signed-off-by: Daniele Ceraolo Spurio 
Cc: Alan Previn 
---
  drivers/gpu/drm/i915/gt/uc/intel_huc.c| 94 +--
  drivers/gpu/drm/i915/gt/uc/intel_huc.h| 16 +++-
  drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c |  4 +-
  drivers/gpu/drm/i915/i915_reg.h   |  3 +
  include/uapi/drm/i915_drm.h   |  3 +-
  5 files changed, 91 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c 
b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
index c189ede4ef55..60f95d98e5fd 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
@@ -10,6 +10,7 @@
  #include "intel_huc.h"
  #include "intel_huc_print.h"
  #include "i915_drv.h"
+#include "i915_reg.h"
  
  #include 

  #include 
@@ -106,7 +107,7 @@ static enum hrtimer_restart 
huc_delayed_load_timer_callback(struct hrtimer *hrti
  {
struct intel_huc *huc = container_of(hrtimer, struct intel_huc, 
delayed_load.timer);
  
-	if (!intel_huc_is_authenticated(huc)) {

+   if (!intel_huc_is_authenticated(huc, INTEL_HUC_AUTH_BY_GSC)) {
if (huc->delayed_load.status == INTEL_HUC_WAITING_ON_GSC)
huc_notice(huc, "timed out waiting for MEI GSC\n");
else if (huc->delayed_load.status == INTEL_HUC_WAITING_ON_PXP)
@@ -124,7 +125,7 @@ static void huc_delayed_load_start(struct intel_huc *huc)
  {
ktime_t delay;
  
-	GEM_BUG_ON(intel_huc_is_authenticated(huc));

+   GEM_BUG_ON(intel_huc_is_authenticated(huc, INTEL_HUC_AUTH_BY_GSC));
  
  	/*

 * On resume we don't have to wait for MEI-GSC to be re-probed, but we
@@ -284,13 +285,23 @@ void intel_huc_init_early(struct intel_huc *huc)
}
  
  	if (GRAPHICS_VER(i915) >= 11) {

-   huc->status.reg = GEN11_HUC_KERNEL_LOAD_INFO;
-   huc->status.mask = HUC_LOAD_SUCCESSFUL;
-   huc->status.value = HUC_LOAD_SUCCESSFUL;
+   huc->status[INTEL_HUC_AUTH_BY_GUC].reg = 
GEN11_HUC_KERNEL_LOAD_INFO;
+   huc->status[INTEL_HUC_AUTH_BY_GUC].mask = HUC_LOAD_SUCCESSFUL;
+   huc->status[INTEL_HUC_AUTH_BY_GUC].value = HUC_LOAD_SUCCESSFUL;
} else {
-   huc->status.reg = HUC_STATUS2;
-   huc->status.mask = HUC_FW_VERIFIED;
-   huc->status.value = HUC_FW_VERIFIED;
+   huc->status[INTEL_HUC_AUTH_BY_GUC].reg = HUC_STATUS2;
+   huc->status[INTEL_HUC_AUTH_BY_GUC].mask = HUC_FW_VERIFIED;
+   huc->status[INTEL_HUC_AUTH_BY_GUC].value = HUC_FW_VERIFIED;
+   }
+
+   if (IS_DG2(i915)) {
+   huc->status[INTEL_HUC_AUTH_BY_GSC].reg = 
GEN11_HUC_KERNEL_LOAD_INFO;
+   huc->status[INTEL_HUC_AUTH_BY_GSC].mask = HUC_LOAD_SUCCESSFUL;
+   huc->status[INTEL_HUC_AUTH_BY_GSC].value = HUC_LOAD_SUCCESSFUL;
+   } else {
+   huc->status[INTEL_HUC_AUTH_BY_GSC].reg = 
HECI_FWSTS5(MTL_GSC_HECI1_BASE);
+   huc->status[INTEL_HUC_AUTH_BY_GSC].mask = 
HECI_FWSTS5_HUC_AUTH_DONE;
+   huc->status[INTEL_HUC_AUTH_BY_GSC].value = 
HECI_FWSTS5_HUC_AUTH_DONE;
}
  }
  
@@ -381,28 +392,39 @@ void intel_huc_suspend(struct intel_huc *huc)

delayed_huc_load_complete(huc);
  }
  
-int intel_huc_wait_for_auth_complete(struct intel_huc *huc)

+static const char *auth_mode_string(struct intel_huc *huc,
+   enum intel_huc_authentication_type type)
+{
+   bool partial = !huc->loaded_via_gsc && huc->fw.is_meu_binary &&
+  type == INTEL_HUC_AUTH_BY_GUC;

partial = !loaded_via_gsc?

If it is not a GSC load then there is no two stage authentication, is 
there? Does that mean the single stage auth does not count as 'all 
workloads' even on platforms where two stage is not supported?



+
+   return partial ? "clear media" : "all workloads";
+}
+
+int intel_huc_wait_for_auth_complete(struct intel_huc *huc,
+enum intel_huc_authentication_type type)
  {
struct intel_gt *gt = huc_to_gt(huc);
int ret;
  
  	ret = __intel_wait_for_register(gt->uncore,

-   huc->status.reg,
-   huc->status.mask,
-   huc->status.value,
+   huc->status[type].reg,
+

Re: [Intel-gfx] [PATCH v2 5/8] drm/i915/huc: differentiate the 2 steps of the MTL HuC auth flow

2023-05-12 Thread Teres Alexis, Alan Previn 
On Fri, 2023-04-28 at 11:58 -0700, Ceraolo Spurio, Daniele wrote:
> Before we add the second step of the MTL HuC auth (via GSC), we need to
> have the ability to differentiate between them. To do so, the huc
> authentication check is duplicated for GuC and GSC auth, with meu
> binaries being considered fully authenticated only after the GSC auth
> step.
> 
> To report the difference between the 2 auth steps, a new case is added
> to the HuC getparam. This way, the clear media driver can start
> submitting before full auth, as partial auth is enough for those
> workloads.
> 
> v2: fix authentication status check for DG2
> 
> Signed-off-by: Daniele Ceraolo Spurio 
> Cc: Alan Previn 
> ---
>  drivers/gpu/drm/i915/gt/uc/intel_huc.c| 94 +--
>  drivers/gpu/drm/i915/gt/uc/intel_huc.h| 16 +++-
>  drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c |  4 +-
>  drivers/gpu/drm/i915/i915_reg.h   |  3 +
>  include/uapi/drm/i915_drm.h   |  3 +-
>  5 files changed, 91 insertions(+), 29 deletions(-)
> 
I believe you need a rebase with the PXP single session merged (the
readiness code in gsccs backend). Other than that, all looks good:

Reviewed-by: Alan Previn

[Intel-gfx] [PATCH v2 5/8] drm/i915/huc: differentiate the 2 steps of the MTL HuC auth flow

2023-04-28 Thread Daniele Ceraolo Spurio 
Before we add the second step of the MTL HuC auth (via GSC), we need to
have the ability to differentiate between them. To do so, the huc
authentication check is duplicated for GuC and GSC auth, with meu
binaries being considered fully authenticated only after the GSC auth
step.

To report the difference between the 2 auth steps, a new case is added
to the HuC getparam. This way, the clear media driver can start
submitting before full auth, as partial auth is enough for those
workloads.

v2: fix authentication status check for DG2

Signed-off-by: Daniele Ceraolo Spurio 
Cc: Alan Previn 
---
 drivers/gpu/drm/i915/gt/uc/intel_huc.c| 94 +--
 drivers/gpu/drm/i915/gt/uc/intel_huc.h| 16 +++-
 drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c |  4 +-
 drivers/gpu/drm/i915/i915_reg.h   |  3 +
 include/uapi/drm/i915_drm.h   |  3 +-
 5 files changed, 91 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c 
b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
index c189ede4ef55..60f95d98e5fd 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
@@ -10,6 +10,7 @@
 #include "intel_huc.h"
 #include "intel_huc_print.h"
 #include "i915_drv.h"
+#include "i915_reg.h"
 
 #include 
 #include 
@@ -106,7 +107,7 @@ static enum hrtimer_restart 
huc_delayed_load_timer_callback(struct hrtimer *hrti
 {
struct intel_huc *huc = container_of(hrtimer, struct intel_huc, 
delayed_load.timer);
 
-   if (!intel_huc_is_authenticated(huc)) {
+   if (!intel_huc_is_authenticated(huc, INTEL_HUC_AUTH_BY_GSC)) {
if (huc->delayed_load.status == INTEL_HUC_WAITING_ON_GSC)
huc_notice(huc, "timed out waiting for MEI GSC\n");
else if (huc->delayed_load.status == INTEL_HUC_WAITING_ON_PXP)
@@ -124,7 +125,7 @@ static void huc_delayed_load_start(struct intel_huc *huc)
 {
ktime_t delay;
 
-   GEM_BUG_ON(intel_huc_is_authenticated(huc));
+   GEM_BUG_ON(intel_huc_is_authenticated(huc, INTEL_HUC_AUTH_BY_GSC));
 
/*
 * On resume we don't have to wait for MEI-GSC to be re-probed, but we
@@ -284,13 +285,23 @@ void intel_huc_init_early(struct intel_huc *huc)
}
 
if (GRAPHICS_VER(i915) >= 11) {
-   huc->status.reg = GEN11_HUC_KERNEL_LOAD_INFO;
-   huc->status.mask = HUC_LOAD_SUCCESSFUL;
-   huc->status.value = HUC_LOAD_SUCCESSFUL;
+   huc->status[INTEL_HUC_AUTH_BY_GUC].reg = 
GEN11_HUC_KERNEL_LOAD_INFO;
+   huc->status[INTEL_HUC_AUTH_BY_GUC].mask = HUC_LOAD_SUCCESSFUL;
+   huc->status[INTEL_HUC_AUTH_BY_GUC].value = HUC_LOAD_SUCCESSFUL;
} else {
-   huc->status.reg = HUC_STATUS2;
-   huc->status.mask = HUC_FW_VERIFIED;
-   huc->status.value = HUC_FW_VERIFIED;
+   huc->status[INTEL_HUC_AUTH_BY_GUC].reg = HUC_STATUS2;
+   huc->status[INTEL_HUC_AUTH_BY_GUC].mask = HUC_FW_VERIFIED;
+   huc->status[INTEL_HUC_AUTH_BY_GUC].value = HUC_FW_VERIFIED;
+   }
+
+   if (IS_DG2(i915)) {
+   huc->status[INTEL_HUC_AUTH_BY_GSC].reg = 
GEN11_HUC_KERNEL_LOAD_INFO;
+   huc->status[INTEL_HUC_AUTH_BY_GSC].mask = HUC_LOAD_SUCCESSFUL;
+   huc->status[INTEL_HUC_AUTH_BY_GSC].value = HUC_LOAD_SUCCESSFUL;
+   } else {
+   huc->status[INTEL_HUC_AUTH_BY_GSC].reg = 
HECI_FWSTS5(MTL_GSC_HECI1_BASE);
+   huc->status[INTEL_HUC_AUTH_BY_GSC].mask = 
HECI_FWSTS5_HUC_AUTH_DONE;
+   huc->status[INTEL_HUC_AUTH_BY_GSC].value = 
HECI_FWSTS5_HUC_AUTH_DONE;
}
 }
 
@@ -381,28 +392,39 @@ void intel_huc_suspend(struct intel_huc *huc)
delayed_huc_load_complete(huc);
 }
 
-int intel_huc_wait_for_auth_complete(struct intel_huc *huc)
+static const char *auth_mode_string(struct intel_huc *huc,
+   enum intel_huc_authentication_type type)
+{
+   bool partial = !huc->loaded_via_gsc && huc->fw.is_meu_binary &&
+  type == INTEL_HUC_AUTH_BY_GUC;
+
+   return partial ? "clear media" : "all workloads";
+}
+
+int intel_huc_wait_for_auth_complete(struct intel_huc *huc,
+enum intel_huc_authentication_type type)
 {
struct intel_gt *gt = huc_to_gt(huc);
int ret;
 
ret = __intel_wait_for_register(gt->uncore,
-   huc->status.reg,
-   huc->status.mask,
-   huc->status.value,
+   huc->status[type].reg,
+   huc->status[type].mask,
+   huc->status[type].value,
2, 50, NULL);
 
/* mark the load process as complete even if the wait failed */
delayed_huc_load_complete(huc);
 
if