[PATCH v2 06/12] drm/xe/pxp: Add GSC session initialization support
Daniele Ceraolo Spurio
daniele.ceraolospurio at intel.com
Thu Nov 7 22:37:55 UTC 2024
On 10/8/2024 11:43 AM, John Harrison wrote:
> On 8/16/2024 12:00, Daniele Ceraolo Spurio wrote:
>> A session is initialized (i.e. started) by sending a message to the GSC.
>>
>> Note that this patch is meant to be squashed with the follow-up patches
>> that implement the other pieces of the session initialization and queue
>> setup flow. It is separate for now for ease of review.
>>
>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
>> ---
>> drivers/gpu/drm/xe/abi/gsc_pxp_commands_abi.h | 21 ++++++++
>> drivers/gpu/drm/xe/xe_pxp_submit.c | 50 +++++++++++++++++++
>> drivers/gpu/drm/xe/xe_pxp_submit.h | 1 +
>> 3 files changed, 72 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/xe/abi/gsc_pxp_commands_abi.h
>> b/drivers/gpu/drm/xe/abi/gsc_pxp_commands_abi.h
>> index 4a59c564a0d0..734feb38f570 100644
>> --- a/drivers/gpu/drm/xe/abi/gsc_pxp_commands_abi.h
>> +++ b/drivers/gpu/drm/xe/abi/gsc_pxp_commands_abi.h
>> @@ -50,6 +50,7 @@ struct pxp_cmd_header {
>> } __packed;
>> #define PXP43_CMDID_INVALIDATE_STREAM_KEY 0x00000007
>> +#define PXP43_CMDID_INIT_SESSION 0x00000036
>> #define PXP43_CMDID_NEW_HUC_AUTH 0x0000003F /* MTL+ */
>> /* PXP-Input-Packet: HUC Auth-only */
>> @@ -64,6 +65,26 @@ struct pxp43_huc_auth_out {
>> struct pxp_cmd_header header;
>> } __packed;
>> +/* PXP-Input-Packet: Init PXP session */
>> +struct pxp43_create_arb_in {
>> + struct pxp_cmd_header header;
>> + /* header.stream_id fields for vesion 4.3 of Init PXP
>> session: */
>> + #define PXP43_INIT_SESSION_VALID BIT(0)
>> + #define PXP43_INIT_SESSION_APPTYPE BIT(1)
>> + #define PXP43_INIT_SESSION_APPID GENMASK(17, 2)
>> + u32 protection_mode;
>> + #define PXP43_INIT_SESSION_PROTECTION_ARB 0x2
>> + u32 sub_session_id;
>> + u32 init_flags;
>> + u32 rsvd[12];
>> +} __packed;
>> +
>> +/* PXP-Input-Packet: Init PXP session */
>> +struct pxp43_create_arb_out {
>> + struct pxp_cmd_header header;
>> + u32 rsvd[8];
>> +} __packed;
>> +
>> /* PXP-Input-Packet: Invalidate Stream Key */
>> struct pxp43_inv_stream_key_in {
>> struct pxp_cmd_header header;
>> diff --git a/drivers/gpu/drm/xe/xe_pxp_submit.c
>> b/drivers/gpu/drm/xe/xe_pxp_submit.c
>> index 41684d666376..c9258c861556 100644
>> --- a/drivers/gpu/drm/xe/xe_pxp_submit.c
>> +++ b/drivers/gpu/drm/xe/xe_pxp_submit.c
>> @@ -26,6 +26,8 @@
>> #include "instructions/xe_mi_commands.h"
>> #include "regs/xe_gt_regs.h"
>> +#define ARB_SESSION 0xF /* TODO: move to UAPI */
> This same define is now in two separate source files? Even if it can't
> be moved to the UAPI header yet it should at least be in an internal
> header rather than being replicated.
I thought about it, but couldn't find a clean solution. This define
would belong in pxp.h, but that's not included from this file and I
didn't want to add the include just for a define that is going away a
few patches later. I could put it in pxp_types.h or pxp_submit.h, but if
it needs to be in the wrong place anyway I thought I might as well just
duplicate it.
Any preference?
>
>> +
>> /*
>> * The VCS is used for kernel-owned GGTT submissions to issue key
>> termination.
>> * Terminations are serialized, so we only need a single queue and
>> a single
>> @@ -470,6 +472,54 @@ static int gsccs_send_message(struct
>> xe_pxp_gsc_client_resources *gsc_res,
>> return ret;
>> }
>> +/**
>> + * xe_pxp_submit_session_init - submits a PXP GSC session
>> initialization
>> + * @gsc_res: the pxp client resources
>> + * @id: the session to initialize
>> + *
>> + * Submit a message to the GSC FW to initialize (i.e. start) a PXP
>> session.
>> + *
>> + * Returns 0 if the submission is successful, an errno value otherwise.
>> + */
>> +int xe_pxp_submit_session_init(struct xe_pxp_gsc_client_resources
>> *gsc_res, u32 id)
>> +{
>> + struct xe_device *xe = gsc_res->vm->xe;
>> + struct pxp43_create_arb_in msg_in = {0};
>> + struct pxp43_create_arb_out msg_out = {0};
>> + int ret;
>> +
>> + msg_in.header.api_version = PXP_APIVER(4, 3);
>> + msg_in.header.command_id = PXP43_CMDID_INIT_SESSION;
>> + msg_in.header.stream_id = (FIELD_PREP(PXP43_INIT_SESSION_APPID,
>> id) |
>> + FIELD_PREP(PXP43_INIT_SESSION_VALID, 1) |
>> + FIELD_PREP(PXP43_INIT_SESSION_APPTYPE, 0));
>> + msg_in.header.buffer_len = sizeof(msg_in) - sizeof(msg_in.header);
>> +
>> + if (id == ARB_SESSION)
>> + msg_in.protection_mode = PXP43_INIT_SESSION_PROTECTION_ARB;
>> +
>> + ret = gsccs_send_message(gsc_res, &msg_in, sizeof(msg_in),
>> + &msg_out, sizeof(msg_out));
>> + if (ret) {
>> + drm_err(&xe->drm, "Failed to init session %d, ret=[%d]\n",
>> id, ret);
> %pe for error code
>
>> + } else if (msg_out.header.status != 0) {
>> + if (is_fw_err_platform_config(msg_out.header.status)) {
>> + drm_info_once(&xe->drm,
>> + "PXP init-session-%d failed due to
>> BIOS/SOC:0x%08x:%s\n",
> Style mis-match - "init session %d" in the first error but then
> "init-session-%d" in this one and the one below (I prefer the first
> one that actually looks like an operation rather than variable).
>
>> + id, msg_out.header.status,
>> + fw_err_to_string(msg_out.header.status));
>> + } else {
>> + drm_dbg(&xe->drm, "PXP init-session-%d failed
>> 0x%08x:%st:\n",
>> + id, msg_out.header.status,
>> + fw_err_to_string(msg_out.header.status));
>> + drm_dbg(&xe->drm, " cmd-detail:
>> ID=[0x%08x],API-Ver-[0x%08x]\n",
> More mis-matching message styles - 'SOC:%s:%s' vs 'ID=[%x]'. Neither
> of which is the normal format of kernel messages.
Those I've copied straight from i915. Will reword.
Daniele
>
> John.
>
>> + msg_in.header.command_id, msg_in.header.api_version);
>> + }
>> + }
>> +
>> + return ret;
>> +}
>> +
>> /**
>> * xe_pxp_submit_session_invalidation - submits a PXP GSC invalidation
>> * @gsc_res: the pxp client resources
>> diff --git a/drivers/gpu/drm/xe/xe_pxp_submit.h
>> b/drivers/gpu/drm/xe/xe_pxp_submit.h
>> index 48fdc9b09116..c9efda02f4b0 100644
>> --- a/drivers/gpu/drm/xe/xe_pxp_submit.h
>> +++ b/drivers/gpu/drm/xe/xe_pxp_submit.h
>> @@ -14,6 +14,7 @@ struct xe_pxp_gsc_client_resources;
>> int xe_pxp_allocate_execution_resources(struct xe_pxp *pxp);
>> void xe_pxp_destroy_execution_resources(struct xe_pxp *pxp);
>> +int xe_pxp_submit_session_init(struct xe_pxp_gsc_client_resources
>> *gsc_res, u32 id);
>> int xe_pxp_submit_session_termination(struct xe_pxp *pxp, u32 id);
>> int xe_pxp_submit_session_invalidation(struct
>> xe_pxp_gsc_client_resources *gsc_res,
>> u32 id);
More information about the Intel-xe
mailing list