Re: [Intel-gfx] [PATCH] drm/i915/pxp: limit drm-errors or warnings on firmware API failures

Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx> · Thu, 2 Feb 2023 08:43:51 +0000

On 02/02/2023 08:13, Alan Previn wrote:
MESA driver is creating protected context on every driver handle
initialization to query caps bit for app. So when running CI tests,
they are observing hundreds of drm_errors when enabling PXP
in .config but using SOC or BIOS configuration that cannot support
PXP sessions.

Update error handling codes to be more selective on which errors
are reported as drm_error vs drm_WARN_ONCE vs drm_debug.
Don't completely remove all FW error replies (at least keep them
but use drm_debug) or else cusomers that really needs to know that
content protection failed won't be aware of it when debugging.

Signed-off-by: Alan Previn <alan.previn.teres.alexis@xxxxxxxxx>

How does this relate to b762787bf767 ("drm/i915/pxp: Use drm_dbg if arb 
session failed due to fw version") which I thought was already fixing 
the drm_error spam caused by userspace probing?

Regards,

Tvrtko

---
  .../i915/pxp/intel_pxp_cmd_interface_cmn.h    |  3 ++
  drivers/gpu/drm/i915/pxp/intel_pxp_session.c  |  2 +-
  drivers/gpu/drm/i915/pxp/intel_pxp_tee.c      | 52 ++++++++++++++-----
  3 files changed, 44 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_cmn.h b/drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_cmn.h
index ae9b151b7cb7..6f6541d5e49a 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_cmn.h
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_cmn.h
@@ -18,6 +18,9 @@
  enum pxp_status {
  	PXP_STATUS_SUCCESS = 0x0,
  	PXP_STATUS_ERROR_API_VERSION = 0x1002,
+	PXP_STATUS_NOT_READY = 0x100e,
+	PXP_STATUS_PLATFCONFIG_KF1_NOVERIF = 0x101a,
+	PXP_STATUS_PLATFCONFIG_KF1_BAD = 0x101f,
  	PXP_STATUS_OP_NOT_PERMITTED = 0x4013
  };
  
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_session.c b/drivers/gpu/drm/i915/pxp/intel_pxp_session.c
index 448cacb0465d..7de849cb6c47 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_session.c
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_session.c
@@ -74,7 +74,7 @@ static int pxp_create_arb_session(struct intel_pxp *pxp)
  
  	ret = pxp_wait_for_session_state(pxp, ARB_SESSION, true);
  	if (ret) {
-		drm_err(&gt->i915->drm, "arb session failed to go in play\n");
+		drm_dbg(&gt->i915->drm, "arb session failed to go in play\n");
  		return ret;
  	}
  	drm_dbg(&gt->i915->drm, "PXP ARB session is alive\n");
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
index d9d248b48093..1c2e4a75a968 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
@@ -19,6 +19,23 @@
  #include "intel_pxp_tee.h"
  #include "intel_pxp_types.h"
  
+static const char *
+pxp_fw_err_to_string(u32 type)
+{
+	switch (type) {
+	case PXP_STATUS_ERROR_API_VERSION:
+		return "ERR_API_VERSION";
+	case PXP_STATUS_NOT_READY:
+		return "ERR_NOT_READY";
+	case PXP_STATUS_PLATFCONFIG_KF1_NOVERIF:
+	case PXP_STATUS_PLATFCONFIG_KF1_BAD:
+		return "ERR_PLATFORM_CONFIG";
+	default:
+		break;
+	}
+	return NULL;
+}
+
  static int intel_pxp_tee_io_message(struct intel_pxp *pxp,
  				    void *msg_in, u32 msg_in_size,
  				    void *msg_out, u32 msg_out_max_size,
@@ -307,15 +324,19 @@ int intel_pxp_tee_cmd_create_arb_session(struct intel_pxp *pxp,
  				       &msg_out, sizeof(msg_out),
  				       NULL);
  
-	if (ret)
-		drm_err(&i915->drm, "Failed to send tee msg ret=[%d]\n", ret);
-	else if (msg_out.header.status == PXP_STATUS_ERROR_API_VERSION)
-		drm_dbg(&i915->drm, "PXP firmware version unsupported, requested: "
-			"CMD-ID-[0x%08x] on API-Ver-[0x%08x]\n",
+	if (ret) {
+		drm_err(&i915->drm, "Failed to send tee msg init arb session, ret=[%d]\n", ret);
+	} else if (msg_out.header.status != 0) {
+		if (msg_out.header.status == PXP_STATUS_PLATFCONFIG_KF1_NOVERIF ||
+		    msg_out.header.status == PXP_STATUS_PLATFCONFIG_KF1_BAD)
+			drm_WARN_ONCE(&i915->drm, true,
+				      "Platform BIOS or Fusing won't allow PXP arb creation\n");
+
+		drm_dbg(&i915->drm, "PXP init arb session failed 0x%08x:%s:"
+			"CMD-ID-[0x%08x]:API-Ver-[0x%08x]\n",
+			msg_out.header.status, pxp_fw_err_to_string(msg_out.header.status),
  			msg_in.header.command_id, msg_in.header.api_version);
-	else if (msg_out.header.status != 0x0)
-		drm_warn(&i915->drm, "PXP firmware failed arb session init request ret=[0x%08x]\n",
-			 msg_out.header.status);
+	}
  
  	return ret;
  }
@@ -347,10 +368,17 @@ void intel_pxp_tee_end_arb_fw_session(struct intel_pxp *pxp, u32 session_id)
  	if ((ret || msg_out.header.status != 0x0) && ++trials < 3)
  		goto try_again;
  
-	if (ret)
+	if (ret) {
  		drm_err(&i915->drm, "Failed to send tee msg for inv-stream-key-%d, ret=[%d]\n",
  			session_id, ret);
-	else if (msg_out.header.status != 0x0)
-		drm_warn(&i915->drm, "PXP firmware failed inv-stream-key-%d with status 0x%08x\n",
-			 session_id, msg_out.header.status);
+	} else if (msg_out.header.status != 0) {
+		if (msg_out.header.status == PXP_STATUS_PLATFCONFIG_KF1_NOVERIF ||
+		    msg_out.header.status == PXP_STATUS_PLATFCONFIG_KF1_BAD)
+			drm_WARN_ONCE(&i915->drm, true,
+				      "Platform BIOS or Fusing won't allow PXP arb creation\n");
+		drm_dbg(&i915->drm, "PXP inv-stream-key-%d failed 0x%08x:%st:\n"
+			"CMD-ID-[0x%08x]:API-Ver-[0x%08x]\n", (int)session_id,
+			msg_out.header.status, pxp_fw_err_to_string(msg_out.header.status),
+			msg_in.header.command_id, msg_in.header.api_version);
+	}
  }






