[Intel-gfx] [PATCH v3] drm/i915/pxp: limit drm-errors or warning on firmware API failures
Tvrtko Ursulin
tvrtko.ursulin at linux.intel.com
Thu Mar 16 08:50:38 UTC 2023
On 15/03/2023 09:16, Eero Tamminen wrote:
> Hi,
>
> Tested the patch with Ubuntu 22.04 desktop + Linux 6.2-rc3 (drm-tip)
> kernel, on TGL-H HW.
>
> With it, this log spam has disappeared:
> ----------------------------------------------------
> [ 8691.608933] i915 0000:00:02.0: [drm] PXP firmware failed arb session
> init request ret=[0x0000101f]
> [ 8691.709442] i915 0000:00:02.0: [drm] *ERROR* arb session failed to go
> in play
> ----------------------------------------------------
>
> There's now just a single message on boot:
> ----------------------------------------------------
> [ 11.674177] ------------[ cut here ]------------
> [ 11.674183] i915 0000:00:02.0: PXP init-arb-session-15 failed due to
> BIOS/SOC:0x0000101a:ERR_PLATFORM_CONFIG
> [ 11.674222] WARNING: CPU: 12 PID: 123 at
> drivers/gpu/drm/i915/pxp/intel_pxp_tee.c:345
> intel_pxp_tee_cmd_create_arb_session+0x19a/0x1e0 [i915]
> ...
> [ 11.674961] Call Trace:
> [ 11.674966] <TASK>
> [ 11.674970] ? pxp_session_work+0x123/0x1d0 [i915]
> [ 11.675130] pxp_session_work+0x123/0x1d0 [i915]
> [ 11.675286] process_one_work+0x1d9/0x3e0
> [ 11.675295] worker_thread+0x21b/0x3d0
> [ 11.675301] ? __pfx_worker_thread+0x10/0x10
> [ 11.675307] kthread+0xe9/0x110
> [ 11.675311] ? __pfx_kthread+0x10/0x10
> [ 11.675315] ret_from_fork+0x29/0x50
> [ 11.675323] </TASK>
> [ 11.675325] ---[ end trace 0000000000000000 ]---
> ----------------------------------------------------
Alan - is this expected during normal operation on some parts, or it's
something truly unexpected/unexplained? If the former then I think it
would be good to downgrade away from drm_WARN so it is less scary.
Commit message talks about "HW/platform gaps" - if it is like a missing
BIOS support or so then I think WARN_ON is too much.
Regards,
Tvrtko
>
>
> - Eero
>
> On 14.3.2023 19.58, Alan Previn wrote:
>> MESA driver is creating protected context on every driver handle
>> creation to query caps bits for app. So when running CI tests,
>> they are observing hundreds of drm_errors when enabling PXP
>> in .config but using SOC fusing or BIOS configuration that cannot
>> support PXP sessions.
>>
>> The fixes tag referenced below was to resolve a related issue
>> where we wanted to silence error messages, but that case was due
>> to outdated IFWI (firmware) that definitely needed an upgrade and
>> was, at that point, considered a one-off case as opposed to today's
>> realization that default CI was enabling PXP in kernel config for
>> all testing.
>>
>> So with this patch, let's strike a balance between issues that is
>> critical but are root-caused from HW/platform gaps (louder drm-warn
>> but just ONCE) vs other cases where it could also come from session
>> state machine (which cannot be a WARN_ONCE since it can be triggered
>> due to runtime operation events).
>>
>> Let's use helpers for these so as more functions are added in future
>> features / HW (or as FW designers continue to bless upstreaming of
>> the error codes and meanings), we only need to update the helpers.
>>
>> NOTE: Don't completely remove FW errors (via drm_debug) or else cusomer
>> apps that really needs to know that content protection failed won't
>> be aware of it.
>>
>> v2: - Add fixes tag (Trvtko)
>> v3: - Break multi-line drm_dbg strings into separate drm_dbg (Daniele)
>> - Fix couple of typecasting nits (Daniele)
>>
>> Fixes: b762787bf767 ("drm/i915/pxp: Use drm_dbg if arb session failed
>> due to fw version")
>> Signed-off-by: Alan Previn <alan.previn.teres.alexis at intel.com>
>> ---
>> .../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 | 77 +++++++++++++++----
>> 3 files changed, 67 insertions(+), 15 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(>->i915->drm, "arb session failed to go in play\n");
>> + drm_dbg(>->i915->drm, "arb session failed to go in play\n");
>> return ret;
>> }
>> drm_dbg(>->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..6f89cd850251 100644
>> --- a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
>> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
>> @@ -19,6 +19,37 @@
>> #include "intel_pxp_tee.h"
>> #include "intel_pxp_types.h"
>> +static bool
>> +is_fw_err_platform_config(u32 type)
>> +{
>> + switch (type) {
>> + case PXP_STATUS_ERROR_API_VERSION:
>> + case PXP_STATUS_PLATFCONFIG_KF1_NOVERIF:
>> + case PXP_STATUS_PLATFCONFIG_KF1_BAD:
>> + return true;
>> + default:
>> + break;
>> + }
>> + return false;
>> +}
>> +
>> +static const char *
>> +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 +338,22 @@ 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",
>> - 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);
>> + 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 (is_fw_err_platform_config(msg_out.header.status)) {
>> + drm_WARN_ONCE(&i915->drm, true,
>> + "PXP init-arb-session-%d failed due to
>> BIOS/SOC:0x%08x:%s\n",
>> + arb_session_id, msg_out.header.status,
>> + fw_err_to_string(msg_out.header.status));
>> + } else {
>> + drm_dbg(&i915->drm, "PXP init-arb-session--%d failed
>> 0x%08x:%st:\n",
>> + arb_session_id, msg_out.header.status,
>> + fw_err_to_string(msg_out.header.status));
>> + drm_dbg(&i915->drm, " cmd-detail:
>> ID=[0x%08x],API-Ver-[0x%08x]\n",
>> + msg_in.header.command_id, msg_in.header.api_version);
>> + }
>> + }
>> return ret;
>> }
>> @@ -347,10 +385,21 @@ 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)
>> - drm_err(&i915->drm, "Failed to send tee msg for
>> inv-stream-key-%d, ret=[%d]\n",
>> + if (ret) {
>> + drm_err(&i915->drm, "Failed to send tee msg for
>> inv-stream-key-%u, 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 (is_fw_err_platform_config(msg_out.header.status)) {
>> + drm_WARN_ONCE(&i915->drm, true,
>> + "PXP inv-stream-key-%u failed due to BIOS/SOC
>> :0x%08x:%s\n",
>> + session_id, msg_out.header.status,
>> + fw_err_to_string(msg_out.header.status));
>> + } else {
>> + drm_dbg(&i915->drm, "PXP inv-stream-key-%u failed
>> 0x%08x:%s:\n",
>> + session_id, msg_out.header.status,
>> + fw_err_to_string(msg_out.header.status));
>> + drm_dbg(&i915->drm, " cmd-detail:
>> ID=[0x%08x],API-Ver-[0x%08x]\n",
>> + msg_in.header.command_id, msg_in.header.api_version);
>> + }
>> + }
>> }
>>
>> base-commit: 07f982ec08c9bfc4aa82d98022111126cd638a0b
More information about the dri-devel
mailing list