Re: [PATCH v3 4/8] drm/i915/pxp: Add GSC-CS backend to send GSC fw messages
missed somethings on host-session-handle - for next rev.
On Wed, 2023-01-25 at 00:06 -0800, Teres Alexis, Alan Previn wrote:
> Add GSC engine based method for sending PXP firmware packets
> to the GSC firmware for MTL (and future) products.
>
> Use the newly added helpers to populate the GSC-CS memory
> header and send the message packet to the FW by dispatching
> the GSC_HECI_CMD_PKT instruction on the GSC engine.
>
> We use non-priveleged batches for submission to GSC engine
> and require two buffers for the request:
> - a buffer for the HECI packet that contains PXP FW commands
> - a batch-buffer that contains the engine instruction for
>sending the HECI packet to the GSC firmware.
>
> Thus, add the allocation and freeing of these buffers in gsccs
> init and fini.
>
> Signed-off-by: Alan Previn
> ---
> .../drm/i915/pxp/intel_pxp_cmd_interface_43.h | 4 +
> drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c| 174 +-
> drivers/gpu/drm/i915/pxp/intel_pxp_types.h| 6 +
> 3 files changed, 183 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_43.h
> b/drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_43.h
> index ad67e3f49c20..b2523d6918c7 100644
> --- a/drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_43.h
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_43.h
> @@ -12,6 +12,10 @@
> /* PXP-Cmd-Op definitions */
> #define PXP43_CMDID_START_HUC_AUTH 0x003A
>
> +/* PXP-Packet sizes for MTL's GSCCS-HECI instruction */
> +#define PXP43_MAX_HECI_IN_SIZE (SZ_32K)
> +#define PXP43_MAX_HECI_OUT_SIZE (SZ_32K)
> +
> /* PXP-Input-Packet: HUC-Authentication */
> struct pxp43_start_huc_auth_in {
> struct pxp_cmd_header header;
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c
> b/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c
> index b304864902c8..35b6bfa55dfc 100644
> --- a/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c
> @@ -6,12 +6,154 @@
> #include "gem/i915_gem_internal.h"
>
> #include "gt/intel_context.h"
> +#include "gt/uc/intel_gsc_uc_heci_cmd_submit.h"
>
> #include "i915_drv.h"
> #include "intel_pxp_cmd_interface_43.h"
> #include "intel_pxp_gsccs.h"
> #include "intel_pxp_types.h"
>
>
alan:snip
> static void
> gsccs_destroy_execution_resource(struct intel_pxp *pxp)
> {
> @@ -19,6 +161,10 @@ gsccs_destroy_execution_resource(struct intel_pxp *pxp)
>
we should send a message to firmware to cleanup all resources associated with
i915's host-session-handle here
>
> if (strm_res->ce)
> intel_context_put(strm_res->ce);
> + if (strm_res->bb_vma)
> + i915_vma_unpin_and_release(_res->bb_vma,
> I915_VMA_RELEASE_MAP);
> + if (strm_res->pkt_vma)
> + i915_vma_unpin_and_release(_res->pkt_vma,
> I915_VMA_RELEASE_MAP);
>
> memset(strm_res, 0, sizeof(*strm_res));
> }
>
alan:snip
>* First, ensure the GSC engine is present.
> @@ -40,11 +187,28 @@ gsccs_allocate_execution_resource(struct intel_pxp *pxp)
>
> mutex_init(>tee_mutex);
>
> + /*
> + * Now, allocate, pin and map two objects, one for the heci message
> packet
> + * and another for the batch buffer we submit into GSC engine (that
> includes the packet).
> + * NOTE: GSC-CS backend is currently only supported on MTL, so we
> allocate shmem.
> + */
> + err = gsccs_create_buffer(pxp->ctrl_gt, "Heci Packet",
> + PXP43_MAX_HECI_IN_SIZE +
> PXP43_MAX_HECI_OUT_SIZE,
> + _res->pkt_vma, _res->pkt_vaddr);
> + if (err)
> + return err;
> +
> + err = gsccs_create_buffer(pxp->ctrl_gt, "Batch Buffer", PAGE_SIZE,
> + _res->bb_vma, _res->bb_vaddr);
> + if (err)
> + goto free_pkt;
> +
> /* Finally, create an intel_context to be used during the submission */
> ce = intel_context_create(engine);
> if (IS_ERR(ce)) {
> drm_err(>i915->drm, "Failed creating gsccs backend ctx\n");
> - return PTR_ERR(ce);
> + err = PTR_ERR(ce);
> + goto free_batch;
> }
> i915_vm_put(ce->vm);
> ce->vm = i915_vm_get(pxp->ctrl_gt->vm);
> @@ -52,6 +216,14 @@ gsccs_allocate_execution_resource(struct intel_pxp *pxp)
> strm_res->ce = ce;
we should be allocatting the host-session-handle here
(the same one should be used throughout i915 life-cycle until unload)
so we can remove that from the arb-session-creation on next patch
alan:snip
[PATCH v3 4/8] drm/i915/pxp: Add GSC-CS backend to send GSC fw messages
Add GSC engine based method for sending PXP firmware packets
to the GSC firmware for MTL (and future) products.
Use the newly added helpers to populate the GSC-CS memory
header and send the message packet to the FW by dispatching
the GSC_HECI_CMD_PKT instruction on the GSC engine.
We use non-priveleged batches for submission to GSC engine
and require two buffers for the request:
- a buffer for the HECI packet that contains PXP FW commands
- a batch-buffer that contains the engine instruction for
sending the HECI packet to the GSC firmware.
Thus, add the allocation and freeing of these buffers in gsccs
init and fini.
Signed-off-by: Alan Previn
---
.../drm/i915/pxp/intel_pxp_cmd_interface_43.h | 4 +
drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c| 174 +-
drivers/gpu/drm/i915/pxp/intel_pxp_types.h| 6 +
3 files changed, 183 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_43.h
b/drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_43.h
index ad67e3f49c20..b2523d6918c7 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_43.h
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_43.h
@@ -12,6 +12,10 @@
/* PXP-Cmd-Op definitions */
#define PXP43_CMDID_START_HUC_AUTH 0x003A
+/* PXP-Packet sizes for MTL's GSCCS-HECI instruction */
+#define PXP43_MAX_HECI_IN_SIZE (SZ_32K)
+#define PXP43_MAX_HECI_OUT_SIZE (SZ_32K)
+
/* PXP-Input-Packet: HUC-Authentication */
struct pxp43_start_huc_auth_in {
struct pxp_cmd_header header;
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c
b/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c
index b304864902c8..35b6bfa55dfc 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c
@@ -6,12 +6,154 @@
#include "gem/i915_gem_internal.h"
#include "gt/intel_context.h"
+#include "gt/uc/intel_gsc_uc_heci_cmd_submit.h"
#include "i915_drv.h"
#include "intel_pxp_cmd_interface_43.h"
#include "intel_pxp_gsccs.h"
#include "intel_pxp_types.h"
+__maybe_unused
+static int gsccs_send_message(struct intel_pxp *pxp,
+ void *msg_in, size_t msg_in_size,
+ void *msg_out, size_t msg_out_size_max,
+ size_t *msg_out_len)
+{
+ struct intel_gt *gt = pxp->ctrl_gt;
+ struct drm_i915_private *i915 = gt->i915;
+ struct gsccs_session_resources *exec = >gsccs_res;
+ struct intel_gsc_mtl_header *header = exec->pkt_vaddr;
+ struct intel_gsc_heci_non_priv_pkt pkt;
+ size_t max_msg_size;
+ u32 reply_size;
+ int ret;
+
+ if (!exec->ce)
+ return -ENODEV;
+
+ max_msg_size = PXP43_MAX_HECI_IN_SIZE - sizeof(*header);
+
+ if (msg_in_size > max_msg_size || msg_out_size_max > max_msg_size)
+ return -ENOSPC;
+
+ mutex_lock(>tee_mutex);
+
+ if (!exec->pkt_vma || !exec->bb_vma)
+ return -ENOENT;
+
+ memset(header, 0, sizeof(*header));
+ intel_gsc_uc_heci_cmd_emit_mtl_header(header, GSC_HECI_MEADDRESS_PXP,
+ msg_in_size + sizeof(*header),
+ exec->host_session_handle);
+
+ memcpy(exec->pkt_vaddr + sizeof(*header), msg_in, msg_in_size);
+
+ pkt.addr_in = i915_vma_offset(exec->pkt_vma);
+ pkt.size_in = header->message_size;
+ pkt.addr_out = pkt.addr_in + PXP43_MAX_HECI_IN_SIZE;
+ pkt.size_out = msg_out_size_max + sizeof(*header);
+ pkt.heci_pkt_vma = exec->pkt_vma;
+ pkt.bb_vma = exec->bb_vma;
+
+ ret = intel_gsc_uc_heci_cmd_submit_nonpriv(>uc.gsc,
+ exec->ce, ,
exec->bb_vaddr, 500);
+ if (ret) {
+ drm_err(>drm, "failed to send gsc PXP msg (%d)\n", ret);
+ goto unlock;
+ }
+
+ /* we keep separate location for reply, so get the response header loc
first */
+ header = exec->pkt_vaddr + PXP43_MAX_HECI_IN_SIZE;
+
+ /* Response validity marker, status and busyness */
+ if (header->validity_marker != GSC_HECI_VALIDITY_MARKER) {
+ drm_err(>drm, "gsc PXP reply with invalid validity
marker\n");
+ ret = -EINVAL;
+ goto unlock;
+ }
+ if (header->status != 0) {
+ drm_dbg(>drm, "gsc PXP reply status has error = 0x%08x\n",
+ header->status);
+ ret = -EINVAL;
+ goto unlock;
+ }
+ if (header->flags & GSC_HECI_FLAG_MSG_PENDING) {
+ drm_dbg(>drm, "gsc PXP reply is busy\n");
+ ret = -EAGAIN;
+ goto unlock;
+ }
+
+ reply_size = header->message_size - sizeof(*header);
+ if (reply_size > msg_out_size_max) {
+ drm_warn(>drm, "caller with insufficient PXP reply size
%u (%ld)\n",
+reply_size,
