* [PATCH v5 01/15] mei: add support to GSC extended header
2022-09-13 0:57 [PATCH v5 00/15] drm/i915: HuC loading for DG2 Daniele Ceraolo Spurio
@ 2022-09-13 0:57 ` Daniele Ceraolo Spurio
2022-09-24 12:15 ` Greg Kroah-Hartman
2022-09-13 0:57 ` [PATCH v5 02/15] mei: bus: enable sending gsc commands Daniele Ceraolo Spurio
` (14 subsequent siblings)
15 siblings, 1 reply; 27+ messages in thread
From: Daniele Ceraolo Spurio @ 2022-09-13 0:57 UTC (permalink / raw)
To: intel-gfx
Cc: Greg Kroah-Hartman, Tomas Winkler, Daniele Ceraolo Spurio,
Vitaly Lubart, dri-devel
From: Tomas Winkler <tomas.winkler@intel.com>
GSC extend header is of variable size and data
is provided in a sgl list inside the header
and not in the data buffers, need to enable the path.
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Vitaly Lubart <vitaly.lubart@intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
V2-3: Rebase
V4:
1. Add missing kdoc for mei_cl_cb
2. In mei_me_hbuf_write()
use dev_err() when validationg parameters instead of WARN_ON()
V5:
1. mei_ext_hdr_len: use if else instead of ?: (Greg)
2. add kdoc for gsc host to firmware interface structures
drivers/misc/mei/client.c | 55 +++++++++++++++++-------
drivers/misc/mei/hbm.c | 13 ++++++
drivers/misc/mei/hw-me.c | 7 +++-
drivers/misc/mei/hw.h | 81 ++++++++++++++++++++++++++++++++++++
drivers/misc/mei/interrupt.c | 47 +++++++++++++++++----
drivers/misc/mei/mei_dev.h | 4 ++
6 files changed, 184 insertions(+), 23 deletions(-)
diff --git a/drivers/misc/mei/client.c b/drivers/misc/mei/client.c
index 0b2fbe1335a7..6c8b71ae32c8 100644
--- a/drivers/misc/mei/client.c
+++ b/drivers/misc/mei/client.c
@@ -322,6 +322,7 @@ void mei_io_cb_free(struct mei_cl_cb *cb)
list_del(&cb->list);
kfree(cb->buf.data);
+ kfree(cb->ext_hdr);
kfree(cb);
}
@@ -401,6 +402,7 @@ static struct mei_cl_cb *mei_io_cb_init(struct mei_cl *cl,
cb->buf_idx = 0;
cb->fop_type = type;
cb->vtag = 0;
+ cb->ext_hdr = NULL;
return cb;
}
@@ -1740,6 +1742,17 @@ static inline u8 mei_ext_hdr_set_vtag(void *ext, u8 vtag)
return vtag_hdr->hdr.length;
}
+static inline bool mei_ext_hdr_is_gsc(struct mei_ext_hdr *ext)
+{
+ return ext && ext->type == MEI_EXT_HDR_GSC;
+}
+
+static inline u8 mei_ext_hdr_set_gsc(struct mei_ext_hdr *ext, struct mei_ext_hdr *gsc_hdr)
+{
+ memcpy(ext, gsc_hdr, mei_ext_hdr_len(gsc_hdr));
+ return ext->length;
+}
+
/**
* mei_msg_hdr_init - allocate and initialize mei message header
*
@@ -1752,14 +1765,17 @@ static struct mei_msg_hdr *mei_msg_hdr_init(const struct mei_cl_cb *cb)
size_t hdr_len;
struct mei_ext_meta_hdr *meta;
struct mei_msg_hdr *mei_hdr;
- bool is_ext, is_vtag;
+ bool is_ext, is_hbm, is_gsc, is_vtag;
+ struct mei_ext_hdr *next_ext;
if (!cb)
return ERR_PTR(-EINVAL);
/* Extended header for vtag is attached only on the first fragment */
is_vtag = (cb->vtag && cb->buf_idx == 0);
- is_ext = is_vtag;
+ is_hbm = cb->cl->me_cl->client_id == 0;
+ is_gsc = ((!is_hbm) && cb->cl->dev->hbm_f_gsc_supported && mei_ext_hdr_is_gsc(cb->ext_hdr));
+ is_ext = is_vtag || is_gsc;
/* Compute extended header size */
hdr_len = sizeof(*mei_hdr);
@@ -1771,6 +1787,9 @@ static struct mei_msg_hdr *mei_msg_hdr_init(const struct mei_cl_cb *cb)
if (is_vtag)
hdr_len += sizeof(struct mei_ext_hdr_vtag);
+ if (is_gsc)
+ hdr_len += mei_ext_hdr_len(cb->ext_hdr);
+
setup_hdr:
mei_hdr = kzalloc(hdr_len, GFP_KERNEL);
if (!mei_hdr)
@@ -1785,10 +1804,20 @@ static struct mei_msg_hdr *mei_msg_hdr_init(const struct mei_cl_cb *cb)
goto out;
meta = (struct mei_ext_meta_hdr *)mei_hdr->extension;
+ meta->size = 0;
+ next_ext = (struct mei_ext_hdr *)meta->hdrs;
if (is_vtag) {
meta->count++;
- meta->size += mei_ext_hdr_set_vtag(meta->hdrs, cb->vtag);
+ meta->size += mei_ext_hdr_set_vtag(next_ext, cb->vtag);
+ next_ext = mei_ext_next(next_ext);
+ }
+
+ if (is_gsc) {
+ meta->count++;
+ meta->size += mei_ext_hdr_set_gsc(next_ext, cb->ext_hdr);
+ next_ext = mei_ext_next(next_ext);
}
+
out:
mei_hdr->length = hdr_len - sizeof(*mei_hdr);
return mei_hdr;
@@ -1812,14 +1841,14 @@ int mei_cl_irq_write(struct mei_cl *cl, struct mei_cl_cb *cb,
struct mei_msg_hdr *mei_hdr = NULL;
size_t hdr_len;
size_t hbuf_len, dr_len;
- size_t buf_len;
+ size_t buf_len = 0;
size_t data_len;
int hbuf_slots;
u32 dr_slots;
u32 dma_len;
int rets;
bool first_chunk;
- const void *data;
+ const void *data = NULL;
if (WARN_ON(!cl || !cl->dev))
return -ENODEV;
@@ -1839,8 +1868,10 @@ int mei_cl_irq_write(struct mei_cl *cl, struct mei_cl_cb *cb,
return 0;
}
- buf_len = buf->size - cb->buf_idx;
- data = buf->data + cb->buf_idx;
+ if (buf->data) {
+ buf_len = buf->size - cb->buf_idx;
+ data = buf->data + cb->buf_idx;
+ }
hbuf_slots = mei_hbuf_empty_slots(dev);
if (hbuf_slots < 0) {
rets = -EOVERFLOW;
@@ -1858,9 +1889,6 @@ int mei_cl_irq_write(struct mei_cl *cl, struct mei_cl_cb *cb,
goto err;
}
- cl_dbg(dev, cl, "Extended Header %d vtag = %d\n",
- mei_hdr->extended, cb->vtag);
-
hdr_len = sizeof(*mei_hdr) + mei_hdr->length;
/**
@@ -1889,7 +1917,7 @@ int mei_cl_irq_write(struct mei_cl *cl, struct mei_cl_cb *cb,
}
mei_hdr->length += data_len;
- if (mei_hdr->dma_ring)
+ if (mei_hdr->dma_ring && buf->data)
mei_dma_ring_write(dev, buf->data + cb->buf_idx, buf_len);
rets = mei_write_message(dev, mei_hdr, hdr_len, data, data_len);
@@ -1983,9 +2011,6 @@ ssize_t mei_cl_write(struct mei_cl *cl, struct mei_cl_cb *cb)
goto err;
}
- cl_dbg(dev, cl, "Extended Header %d vtag = %d\n",
- mei_hdr->extended, cb->vtag);
-
hdr_len = sizeof(*mei_hdr) + mei_hdr->length;
if (rets == 0) {
@@ -2030,7 +2055,7 @@ ssize_t mei_cl_write(struct mei_cl *cl, struct mei_cl_cb *cb)
mei_hdr->length += data_len;
- if (mei_hdr->dma_ring)
+ if (mei_hdr->dma_ring && buf->data)
mei_dma_ring_write(dev, buf->data, buf_len);
rets = mei_write_message(dev, mei_hdr, hdr_len, data, data_len);
diff --git a/drivers/misc/mei/hbm.c b/drivers/misc/mei/hbm.c
index de712cbf5d07..12a62a911e42 100644
--- a/drivers/misc/mei/hbm.c
+++ b/drivers/misc/mei/hbm.c
@@ -340,9 +340,13 @@ static int mei_hbm_capabilities_req(struct mei_device *dev)
req.hbm_cmd = MEI_HBM_CAPABILITIES_REQ_CMD;
if (dev->hbm_f_vt_supported)
req.capability_requested[0] |= HBM_CAP_VT;
+
if (dev->hbm_f_cd_supported)
req.capability_requested[0] |= HBM_CAP_CD;
+ if (dev->hbm_f_gsc_supported)
+ req.capability_requested[0] |= HBM_CAP_GSC;
+
ret = mei_hbm_write_message(dev, &mei_hdr, &req);
if (ret) {
dev_err(dev->dev,
@@ -1200,6 +1204,12 @@ static void mei_hbm_config_features(struct mei_device *dev)
dev->version.minor_version >= HBM_MINOR_VERSION_VT))
dev->hbm_f_vt_supported = 1;
+ /* GSC support */
+ if (dev->version.major_version > HBM_MAJOR_VERSION_GSC ||
+ (dev->version.major_version == HBM_MAJOR_VERSION_GSC &&
+ dev->version.minor_version >= HBM_MINOR_VERSION_GSC))
+ dev->hbm_f_gsc_supported = 1;
+
/* Capability message Support */
dev->hbm_f_cap_supported = 0;
if (dev->version.major_version > HBM_MAJOR_VERSION_CAP ||
@@ -1367,6 +1377,9 @@ int mei_hbm_dispatch(struct mei_device *dev, struct mei_msg_hdr *hdr)
if (!(capability_res->capability_granted[0] & HBM_CAP_CD))
dev->hbm_f_cd_supported = 0;
+ if (!(capability_res->capability_granted[0] & HBM_CAP_GSC))
+ dev->hbm_f_gsc_supported = 0;
+
if (dev->hbm_f_dr_supported) {
if (mei_dmam_ring_alloc(dev))
dev_info(dev->dev, "running w/o dma ring\n");
diff --git a/drivers/misc/mei/hw-me.c b/drivers/misc/mei/hw-me.c
index 9e2f781c6ed5..da4ef0b51954 100644
--- a/drivers/misc/mei/hw-me.c
+++ b/drivers/misc/mei/hw-me.c
@@ -590,9 +590,14 @@ static int mei_me_hbuf_write(struct mei_device *dev,
u32 dw_cnt;
int empty_slots;
- if (WARN_ON(!hdr || !data || hdr_len & 0x3))
+ if (WARN_ON(!hdr || hdr_len & 0x3))
return -EINVAL;
+ if (!data && data_len) {
+ dev_err(dev->dev, "wrong parameters null data with data_len = %zu\n", data_len);
+ return -EINVAL;
+ }
+
dev_dbg(dev->dev, MEI_HDR_FMT, MEI_HDR_PRM((struct mei_msg_hdr *)hdr));
empty_slots = mei_hbuf_empty_slots(dev);
diff --git a/drivers/misc/mei/hw.h b/drivers/misc/mei/hw.h
index e7e020dba6b1..70d405c67f0e 100644
--- a/drivers/misc/mei/hw.h
+++ b/drivers/misc/mei/hw.h
@@ -92,6 +92,12 @@
#define HBM_MINOR_VERSION_VT 2
#define HBM_MAJOR_VERSION_VT 2
+/*
+ * MEI version with GSC support
+ */
+#define HBM_MINOR_VERSION_GSC 2
+#define HBM_MAJOR_VERSION_GSC 2
+
/*
* MEI version with capabilities message support
*/
@@ -229,10 +235,12 @@ enum mei_cl_disconnect_status {
*
* @MEI_EXT_HDR_NONE: sentinel
* @MEI_EXT_HDR_VTAG: vtag header
+ * @MEI_EXT_HDR_GSC: gsc header
*/
enum mei_ext_hdr_type {
MEI_EXT_HDR_NONE = 0,
MEI_EXT_HDR_VTAG = 1,
+ MEI_EXT_HDR_GSC = 2,
};
/**
@@ -305,6 +313,60 @@ static inline bool mei_ext_last(struct mei_ext_meta_hdr *meta,
return (u8 *)ext >= (u8 *)meta + sizeof(*meta) + (meta->size * 4);
}
+struct mei_gsc_sgl {
+ u32 low;
+ u32 high;
+ u32 length;
+} __packed;
+
+#define GSC_HECI_MSG_KERNEL 0
+#define GSC_HECI_MSG_USER 1
+
+#define GSC_ADDRESS_TYPE_GTT 0
+#define GSC_ADDRESS_TYPE_PPGTT 1
+#define GSC_ADDRESS_TYPE_PHYSICAL_CONTINUOUS 2 /* max of 64K */
+#define GSC_ADDRESS_TYPE_PHYSICAL_SGL 3
+
+/**
+ * struct mei_ext_hdr_gsc_h2f - extended header: gsc host to firmware interface
+ *
+ * @hdr: extended header
+ * @client_id: GSC_HECI_MSG_KERNEL or GSC_HECI_MSG_USER
+ * @addr_type: GSC_ADDRESS_TYPE_{GTT, PPGTT, PHYSICAL_CONTINUOUS, PHYSICAL_SGL}
+ * @fence_id: synchronization marker
+ * @input_address_count: number of input sgl buffers
+ * @output_address_count: number of output sgl buffers
+ * @reserved: reserved
+ * @sgl: sg list
+ */
+struct mei_ext_hdr_gsc_h2f {
+ struct mei_ext_hdr hdr;
+ u8 client_id;
+ u8 addr_type;
+ u32 fence_id;
+ u8 input_address_count;
+ u8 output_address_count;
+ u8 reserved[2];
+ struct mei_gsc_sgl sgl[];
+} __packed;
+
+/**
+ * struct mei_ext_hdr_gsc_f2h - gsc firmware to host interface
+ *
+ * @hdr: extended header
+ * @client_id: GSC_HECI_MSG_KERNEL or GSC_HECI_MSG_USER
+ * @reserved: reserved
+ * @fence_id: synchronization marker
+ * @written: number of bytes written to firmware
+ */
+struct mei_ext_hdr_gsc_f2h {
+ struct mei_ext_hdr hdr;
+ u8 client_id;
+ u8 reserved;
+ u32 fence_id;
+ u32 written;
+} __packed;
+
/**
* mei_ext_next - following extended header on the TLV list
*
@@ -320,6 +382,21 @@ static inline struct mei_ext_hdr *mei_ext_next(struct mei_ext_hdr *ext)
return (struct mei_ext_hdr *)((u8 *)ext + (ext->length * 4));
}
+/**
+ * mei_ext_hdr_len - get ext header length in bytes
+ *
+ * @ext: extend header
+ *
+ * Return: extend header length in bytes
+ */
+static inline u32 mei_ext_hdr_len(const struct mei_ext_hdr *ext)
+{
+ if (!ext)
+ return 0;
+
+ return ext->length * sizeof(u32);
+}
+
/**
* struct mei_msg_hdr - MEI BUS Interface Section
*
@@ -682,6 +759,10 @@ struct hbm_dma_ring_ctrl {
/* virtual tag supported */
#define HBM_CAP_VT BIT(0)
+
+/* gsc extended header support */
+#define HBM_CAP_GSC BIT(1)
+
/* client dma supported */
#define HBM_CAP_CD BIT(2)
diff --git a/drivers/misc/mei/interrupt.c b/drivers/misc/mei/interrupt.c
index 0706322154cb..0a0e984e5673 100644
--- a/drivers/misc/mei/interrupt.c
+++ b/drivers/misc/mei/interrupt.c
@@ -98,9 +98,12 @@ static int mei_cl_irq_read_msg(struct mei_cl *cl,
struct mei_device *dev = cl->dev;
struct mei_cl_cb *cb;
+ struct mei_ext_hdr_vtag *vtag_hdr = NULL;
+ struct mei_ext_hdr_gsc_f2h *gsc_f2h = NULL;
+
size_t buf_sz;
u32 length;
- int ext_len;
+ u32 ext_len;
length = mei_hdr->length;
ext_len = 0;
@@ -122,18 +125,24 @@ static int mei_cl_irq_read_msg(struct mei_cl *cl,
}
if (mei_hdr->extended) {
- struct mei_ext_hdr *ext;
- struct mei_ext_hdr_vtag *vtag_hdr = NULL;
-
- ext = mei_ext_begin(meta);
+ struct mei_ext_hdr *ext = mei_ext_begin(meta);
do {
switch (ext->type) {
case MEI_EXT_HDR_VTAG:
vtag_hdr = (struct mei_ext_hdr_vtag *)ext;
break;
+ case MEI_EXT_HDR_GSC:
+ gsc_f2h = (struct mei_ext_hdr_gsc_f2h *)ext;
+ cb->ext_hdr = kzalloc(sizeof(*gsc_f2h), GFP_KERNEL);
+ if (!cb->ext_hdr) {
+ cb->status = -ENOMEM;
+ goto discard;
+ }
+ break;
case MEI_EXT_HDR_NONE:
fallthrough;
default:
+ cl_err(dev, cl, "unknown extended header\n");
cb->status = -EPROTO;
break;
}
@@ -141,12 +150,14 @@ static int mei_cl_irq_read_msg(struct mei_cl *cl,
ext = mei_ext_next(ext);
} while (!mei_ext_last(meta, ext));
- if (!vtag_hdr) {
- cl_dbg(dev, cl, "vtag not found in extended header.\n");
+ if (!vtag_hdr && !gsc_f2h) {
+ cl_dbg(dev, cl, "no vtag or gsc found in extended header.\n");
cb->status = -EPROTO;
goto discard;
}
+ }
+ if (vtag_hdr) {
cl_dbg(dev, cl, "vtag: %d\n", vtag_hdr->vtag);
if (cb->vtag && cb->vtag != vtag_hdr->vtag) {
cl_err(dev, cl, "mismatched tag: %d != %d\n",
@@ -157,6 +168,28 @@ static int mei_cl_irq_read_msg(struct mei_cl *cl,
cb->vtag = vtag_hdr->vtag;
}
+ if (gsc_f2h) {
+ u32 ext_hdr_len = mei_ext_hdr_len(&gsc_f2h->hdr);
+
+ if (!dev->hbm_f_gsc_supported) {
+ cl_err(dev, cl, "gsc extended header is not supported\n");
+ cb->status = -EPROTO;
+ goto discard;
+ }
+
+ if (length) {
+ cl_err(dev, cl, "no data allowed in cb with gsc\n");
+ cb->status = -EPROTO;
+ goto discard;
+ }
+ if (ext_hdr_len > sizeof(*gsc_f2h)) {
+ cl_err(dev, cl, "gsc extended header is too big %u\n", ext_hdr_len);
+ cb->status = -EPROTO;
+ goto discard;
+ }
+ memcpy(cb->ext_hdr, gsc_f2h, ext_hdr_len);
+ }
+
if (!mei_cl_is_connected(cl)) {
cl_dbg(dev, cl, "not connected\n");
cb->status = -ENODEV;
diff --git a/drivers/misc/mei/mei_dev.h b/drivers/misc/mei/mei_dev.h
index 6bb3e1ba9ded..31784bbc2d2a 100644
--- a/drivers/misc/mei/mei_dev.h
+++ b/drivers/misc/mei/mei_dev.h
@@ -206,6 +206,7 @@ struct mei_cl;
* @status: io status of the cb
* @internal: communication between driver and FW flag
* @blocking: transmission blocking mode
+ * @ext_hdr: extended header
*/
struct mei_cl_cb {
struct list_head list;
@@ -218,6 +219,7 @@ struct mei_cl_cb {
int status;
u32 internal:1;
u32 blocking:1;
+ struct mei_ext_hdr *ext_hdr;
};
/**
@@ -494,6 +496,7 @@ struct mei_dev_timeouts {
* @hbm_f_vt_supported : hbm feature vtag supported
* @hbm_f_cap_supported : hbm feature capabilities message supported
* @hbm_f_cd_supported : hbm feature client dma supported
+ * @hbm_f_gsc_supported : hbm feature gsc supported
*
* @fw_ver : FW versions
*
@@ -585,6 +588,7 @@ struct mei_device {
unsigned int hbm_f_vt_supported:1;
unsigned int hbm_f_cap_supported:1;
unsigned int hbm_f_cd_supported:1;
+ unsigned int hbm_f_gsc_supported:1;
struct mei_fw_version fw_ver[MEI_MAX_FW_VER_BLOCKS];
--
2.37.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v5 01/15] mei: add support to GSC extended header
2022-09-13 0:57 ` [PATCH v5 01/15] mei: add support to GSC extended header Daniele Ceraolo Spurio
@ 2022-09-24 12:15 ` Greg Kroah-Hartman
0 siblings, 0 replies; 27+ messages in thread
From: Greg Kroah-Hartman @ 2022-09-24 12:15 UTC (permalink / raw)
To: Daniele Ceraolo Spurio; +Cc: intel-gfx, Tomas Winkler, dri-devel, Vitaly Lubart
On Mon, Sep 12, 2022 at 05:57:25PM -0700, Daniele Ceraolo Spurio wrote:
> From: Tomas Winkler <tomas.winkler@intel.com>
>
> GSC extend header is of variable size and data
> is provided in a sgl list inside the header
> and not in the data buffers, need to enable the path.
>
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Vitaly Lubart <vitaly.lubart@intel.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v5 02/15] mei: bus: enable sending gsc commands
2022-09-13 0:57 [PATCH v5 00/15] drm/i915: HuC loading for DG2 Daniele Ceraolo Spurio
2022-09-13 0:57 ` [PATCH v5 01/15] mei: add support to GSC extended header Daniele Ceraolo Spurio
@ 2022-09-13 0:57 ` Daniele Ceraolo Spurio
2022-09-24 12:15 ` Greg Kroah-Hartman
2022-09-13 0:57 ` [PATCH v5 03/15] mei: adjust extended header kdocs Daniele Ceraolo Spurio
` (13 subsequent siblings)
15 siblings, 1 reply; 27+ messages in thread
From: Daniele Ceraolo Spurio @ 2022-09-13 0:57 UTC (permalink / raw)
To: intel-gfx
Cc: Greg Kroah-Hartman, Tomas Winkler, Daniele Ceraolo Spurio,
Vitaly Lubart, dri-devel
From: Tomas Winkler <tomas.winkler@intel.com>
GSC command is and extended header containing a scatter gather
list and without a data buffer. Using MEI_CL_IO_SGL flag,
the caller send the GSC command as a data and the function internally
moves it to the extended header.
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Vitaly Lubart <vitaly.lubart@intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
V2-5: Rebase
drivers/misc/mei/bus.c | 20 ++++++++++++++++++--
drivers/misc/mei/mei_dev.h | 4 ++++
2 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
index 46aa3554e97b..225f0b04c021 100644
--- a/drivers/misc/mei/bus.c
+++ b/drivers/misc/mei/bus.c
@@ -100,9 +100,18 @@ ssize_t __mei_cl_send(struct mei_cl *cl, const u8 *buf, size_t length, u8 vtag,
cb->internal = !!(mode & MEI_CL_IO_TX_INTERNAL);
cb->blocking = !!(mode & MEI_CL_IO_TX_BLOCKING);
memcpy(cb->buf.data, buf, length);
+ /* hack we point data to header */
+ if (mode & MEI_CL_IO_SGL) {
+ cb->ext_hdr = (struct mei_ext_hdr *)cb->buf.data;
+ cb->buf.data = NULL;
+ cb->buf.size = 0;
+ }
rets = mei_cl_write(cl, cb);
+ if (mode & MEI_CL_IO_SGL && rets == 0)
+ rets = length;
+
out:
mutex_unlock(&bus->device_lock);
@@ -205,9 +214,16 @@ ssize_t __mei_cl_recv(struct mei_cl *cl, u8 *buf, size_t length, u8 *vtag,
goto free;
}
- r_length = min_t(size_t, length, cb->buf_idx);
- memcpy(buf, cb->buf.data, r_length);
+ /* for the GSC type - copy the extended header to the buffer */
+ if (cb->ext_hdr && cb->ext_hdr->type == MEI_EXT_HDR_GSC) {
+ r_length = min_t(size_t, length, cb->ext_hdr->length * sizeof(u32));
+ memcpy(buf, cb->ext_hdr, r_length);
+ } else {
+ r_length = min_t(size_t, length, cb->buf_idx);
+ memcpy(buf, cb->buf.data, r_length);
+ }
rets = r_length;
+
if (vtag)
*vtag = cb->vtag;
diff --git a/drivers/misc/mei/mei_dev.h b/drivers/misc/mei/mei_dev.h
index 31784bbc2d2a..8d8018428d9d 100644
--- a/drivers/misc/mei/mei_dev.h
+++ b/drivers/misc/mei/mei_dev.h
@@ -116,12 +116,16 @@ enum mei_cb_file_ops {
* @MEI_CL_IO_TX_INTERNAL: internal communication between driver and FW
*
* @MEI_CL_IO_RX_NONBLOCK: recv is non-blocking
+ *
+ * @MEI_CL_IO_SGL: send command with sgl list.
*/
enum mei_cl_io_mode {
MEI_CL_IO_TX_BLOCKING = BIT(0),
MEI_CL_IO_TX_INTERNAL = BIT(1),
MEI_CL_IO_RX_NONBLOCK = BIT(2),
+
+ MEI_CL_IO_SGL = BIT(3),
};
/*
--
2.37.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v5 02/15] mei: bus: enable sending gsc commands
2022-09-13 0:57 ` [PATCH v5 02/15] mei: bus: enable sending gsc commands Daniele Ceraolo Spurio
@ 2022-09-24 12:15 ` Greg Kroah-Hartman
0 siblings, 0 replies; 27+ messages in thread
From: Greg Kroah-Hartman @ 2022-09-24 12:15 UTC (permalink / raw)
To: Daniele Ceraolo Spurio; +Cc: intel-gfx, Tomas Winkler, dri-devel, Vitaly Lubart
On Mon, Sep 12, 2022 at 05:57:26PM -0700, Daniele Ceraolo Spurio wrote:
> From: Tomas Winkler <tomas.winkler@intel.com>
>
> GSC command is and extended header containing a scatter gather
> list and without a data buffer. Using MEI_CL_IO_SGL flag,
> the caller send the GSC command as a data and the function internally
> moves it to the extended header.
>
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Vitaly Lubart <vitaly.lubart@intel.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v5 03/15] mei: adjust extended header kdocs
2022-09-13 0:57 [PATCH v5 00/15] drm/i915: HuC loading for DG2 Daniele Ceraolo Spurio
2022-09-13 0:57 ` [PATCH v5 01/15] mei: add support to GSC extended header Daniele Ceraolo Spurio
2022-09-13 0:57 ` [PATCH v5 02/15] mei: bus: enable sending gsc commands Daniele Ceraolo Spurio
@ 2022-09-13 0:57 ` Daniele Ceraolo Spurio
2022-09-24 12:15 ` Greg Kroah-Hartman
2022-09-13 0:57 ` [PATCH v5 04/15] mei: bus: extend bus API to support command streamer API Daniele Ceraolo Spurio
` (12 subsequent siblings)
15 siblings, 1 reply; 27+ messages in thread
From: Daniele Ceraolo Spurio @ 2022-09-13 0:57 UTC (permalink / raw)
To: intel-gfx
Cc: Greg Kroah-Hartman, Tomas Winkler, Daniele Ceraolo Spurio, dri-devel
From: Tomas Winkler <tomas.winkler@intel.com>
Fix kdoc for struct mei_ext_hdr and mei_ext_begin().
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
V4: New in the series
V5: Rebase
drivers/misc/mei/hw.h | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/misc/mei/hw.h b/drivers/misc/mei/hw.h
index 70d405c67f0e..319418ddf4fb 100644
--- a/drivers/misc/mei/hw.h
+++ b/drivers/misc/mei/hw.h
@@ -247,8 +247,7 @@ enum mei_ext_hdr_type {
* struct mei_ext_hdr - extend header descriptor (TLV)
* @type: enum mei_ext_hdr_type
* @length: length excluding descriptor
- * @ext_payload: payload of the specific extended header
- * @hdr: place holder for actual header
+ * @data: the extended header payload
*/
struct mei_ext_hdr {
u8 type;
@@ -287,12 +286,11 @@ struct mei_ext_hdr_vtag {
* Extended header iterator functions
*/
/**
- * mei_ext_hdr - extended header iterator begin
+ * mei_ext_begin - extended header iterator begin
*
* @meta: meta header of the extended header list
*
- * Return:
- * The first extended header
+ * Return: The first extended header
*/
static inline struct mei_ext_hdr *mei_ext_begin(struct mei_ext_meta_hdr *meta)
{
--
2.37.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v5 03/15] mei: adjust extended header kdocs
2022-09-13 0:57 ` [PATCH v5 03/15] mei: adjust extended header kdocs Daniele Ceraolo Spurio
@ 2022-09-24 12:15 ` Greg Kroah-Hartman
0 siblings, 0 replies; 27+ messages in thread
From: Greg Kroah-Hartman @ 2022-09-24 12:15 UTC (permalink / raw)
To: Daniele Ceraolo Spurio; +Cc: intel-gfx, Tomas Winkler, dri-devel
On Mon, Sep 12, 2022 at 05:57:27PM -0700, Daniele Ceraolo Spurio wrote:
> From: Tomas Winkler <tomas.winkler@intel.com>
>
> Fix kdoc for struct mei_ext_hdr and mei_ext_begin().
>
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v5 04/15] mei: bus: extend bus API to support command streamer API
2022-09-13 0:57 [PATCH v5 00/15] drm/i915: HuC loading for DG2 Daniele Ceraolo Spurio
` (2 preceding siblings ...)
2022-09-13 0:57 ` [PATCH v5 03/15] mei: adjust extended header kdocs Daniele Ceraolo Spurio
@ 2022-09-13 0:57 ` Daniele Ceraolo Spurio
2022-09-24 12:16 ` Greg Kroah-Hartman
2022-09-13 0:57 ` [PATCH v5 05/15] mei: pxp: add command streamer API to the PXP driver Daniele Ceraolo Spurio
` (11 subsequent siblings)
15 siblings, 1 reply; 27+ messages in thread
From: Daniele Ceraolo Spurio @ 2022-09-13 0:57 UTC (permalink / raw)
To: intel-gfx
Cc: Greg Kroah-Hartman, Tomas Winkler, Daniele Ceraolo Spurio,
Vitaly Lubart, dri-devel
From: Vitaly Lubart <vitaly.lubart@intel.com>
Add mei bus API for sending gsc commands: mei_cldev_send_gsc_command()
The GSC commands are originated in the graphics stack
and are in form of SGL DMA buffers.
The GSC commands are synchronous, the response is received
in the same call on the out sg list buffers.
The function setups pointers for in and out sg lists in the
mei sgl extended header and sends it to the firmware.
Signed-off-by: Vitaly Lubart <vitaly.lubart@intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
V2:
1. More detailed commit message
2. Fix typo in the comments
V3-4:Rebase
V5:
1. Use forward declaration instead of
#include <linux/scatterlist.h> in mei_cl_bus.h
drivers/misc/mei/bus.c | 126 +++++++++++++++++++++++++++++++++++++
include/linux/mei_cl_bus.h | 6 ++
2 files changed, 132 insertions(+)
diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
index 225f0b04c021..1fbe127ff633 100644
--- a/drivers/misc/mei/bus.c
+++ b/drivers/misc/mei/bus.c
@@ -13,6 +13,7 @@
#include <linux/slab.h>
#include <linux/mutex.h>
#include <linux/interrupt.h>
+#include <linux/scatterlist.h>
#include <linux/mei_cl_bus.h>
#include "mei_dev.h"
@@ -838,6 +839,131 @@ int mei_cldev_disable(struct mei_cl_device *cldev)
}
EXPORT_SYMBOL_GPL(mei_cldev_disable);
+/**
+ * mei_cldev_send_gsc_command - sends a gsc command, by sending
+ * a gsl mei message to gsc and receiving reply from gsc
+ *
+ * @cldev: me client device
+ * @client_id: client id to send the command to
+ * @fence_id: fence id to send the command to
+ * @sg_in: scatter gather list containing addresses for rx message buffer
+ * @total_in_len: total length of data in 'in' sg, can be less than the sum of buffers sizes
+ * @sg_out: scatter gather list containing addresses for tx message buffer
+ *
+ * Return:
+ * * written size in bytes
+ * * < 0 on error
+ */
+ssize_t mei_cldev_send_gsc_command(struct mei_cl_device *cldev,
+ u8 client_id, u32 fence_id,
+ struct scatterlist *sg_in,
+ size_t total_in_len,
+ struct scatterlist *sg_out)
+{
+ struct mei_cl *cl;
+ struct mei_device *bus;
+ ssize_t ret = 0;
+
+ struct mei_ext_hdr_gsc_h2f *ext_hdr;
+ size_t buf_sz = sizeof(struct mei_ext_hdr_gsc_h2f);
+ int sg_out_nents, sg_in_nents;
+ int i;
+ struct scatterlist *sg;
+ struct mei_ext_hdr_gsc_f2h rx_msg;
+ unsigned int sg_len;
+
+ if (!cldev || !sg_in || !sg_out)
+ return -EINVAL;
+
+ cl = cldev->cl;
+ bus = cldev->bus;
+
+ dev_dbg(bus->dev, "client_id %u, fence_id %u\n", client_id, fence_id);
+
+ if (!bus->hbm_f_gsc_supported)
+ return -EOPNOTSUPP;
+
+ sg_out_nents = sg_nents(sg_out);
+ sg_in_nents = sg_nents(sg_in);
+ /* at least one entry in tx and rx sgls must be present */
+ if (sg_out_nents <= 0 || sg_in_nents <= 0)
+ return -EINVAL;
+
+ buf_sz += (sg_out_nents + sg_in_nents) * sizeof(struct mei_gsc_sgl);
+ ext_hdr = kzalloc(buf_sz, GFP_KERNEL);
+ if (!ext_hdr)
+ return -ENOMEM;
+
+ /* construct the GSC message */
+ ext_hdr->hdr.type = MEI_EXT_HDR_GSC;
+ ext_hdr->hdr.length = buf_sz / sizeof(u32); /* length is in dw */
+
+ ext_hdr->client_id = client_id;
+ ext_hdr->addr_type = GSC_ADDRESS_TYPE_PHYSICAL_SGL;
+ ext_hdr->fence_id = fence_id;
+ ext_hdr->input_address_count = sg_in_nents;
+ ext_hdr->output_address_count = sg_out_nents;
+ ext_hdr->reserved[0] = 0;
+ ext_hdr->reserved[1] = 0;
+
+ /* copy in-sgl to the message */
+ for (i = 0, sg = sg_in; i < sg_in_nents; i++, sg++) {
+ ext_hdr->sgl[i].low = lower_32_bits(sg_dma_address(sg));
+ ext_hdr->sgl[i].high = upper_32_bits(sg_dma_address(sg));
+ sg_len = min_t(unsigned int, sg_dma_len(sg), PAGE_SIZE);
+ ext_hdr->sgl[i].length = (sg_len <= total_in_len) ? sg_len : total_in_len;
+ total_in_len -= ext_hdr->sgl[i].length;
+ }
+
+ /* copy out-sgl to the message */
+ for (i = sg_in_nents, sg = sg_out; i < sg_in_nents + sg_out_nents; i++, sg++) {
+ ext_hdr->sgl[i].low = lower_32_bits(sg_dma_address(sg));
+ ext_hdr->sgl[i].high = upper_32_bits(sg_dma_address(sg));
+ sg_len = min_t(unsigned int, sg_dma_len(sg), PAGE_SIZE);
+ ext_hdr->sgl[i].length = sg_len;
+ }
+
+ /* send the message to GSC */
+ ret = __mei_cl_send(cl, (u8 *)ext_hdr, buf_sz, 0, MEI_CL_IO_SGL);
+ if (ret < 0) {
+ dev_err(bus->dev, "__mei_cl_send failed, returned %zd\n", ret);
+ goto end;
+ }
+ if (ret != buf_sz) {
+ dev_err(bus->dev, "__mei_cl_send returned %zd instead of expected %zd\n",
+ ret, buf_sz);
+ ret = -EIO;
+ goto end;
+ }
+
+ /* receive the reply from GSC, note that at this point sg_in should contain the reply */
+ ret = __mei_cl_recv(cl, (u8 *)&rx_msg, sizeof(rx_msg), NULL, MEI_CL_IO_SGL, 0);
+
+ if (ret != sizeof(rx_msg)) {
+ dev_err(bus->dev, "__mei_cl_recv returned %zd instead of expected %zd\n",
+ ret, sizeof(rx_msg));
+ if (ret >= 0)
+ ret = -EIO;
+ goto end;
+ }
+
+ /* check rx_msg.client_id and rx_msg.fence_id match the ones we send */
+ if (rx_msg.client_id != client_id || rx_msg.fence_id != fence_id) {
+ dev_err(bus->dev, "received client_id/fence_id %u/%u instead of %u/%u sent\n",
+ rx_msg.client_id, rx_msg.fence_id, client_id, fence_id);
+ ret = -EFAULT;
+ goto end;
+ }
+
+ dev_dbg(bus->dev, "gsc command: successfully written %u bytes\n", rx_msg.written);
+ ret = rx_msg.written;
+
+end:
+ kfree(ext_hdr);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(mei_cldev_send_gsc_command);
+
/**
* mei_cl_device_find - find matching entry in the driver id table
*
diff --git a/include/linux/mei_cl_bus.h b/include/linux/mei_cl_bus.h
index df1fab44ea5c..fd6e0620658d 100644
--- a/include/linux/mei_cl_bus.h
+++ b/include/linux/mei_cl_bus.h
@@ -11,6 +11,7 @@
struct mei_cl_device;
struct mei_device;
+struct scatterlist;
typedef void (*mei_cldev_cb_t)(struct mei_cl_device *cldev);
@@ -116,6 +117,11 @@ void mei_cldev_set_drvdata(struct mei_cl_device *cldev, void *data);
int mei_cldev_enable(struct mei_cl_device *cldev);
int mei_cldev_disable(struct mei_cl_device *cldev);
bool mei_cldev_enabled(const struct mei_cl_device *cldev);
+ssize_t mei_cldev_send_gsc_command(struct mei_cl_device *cldev,
+ u8 client_id, u32 fence_id,
+ struct scatterlist *sg_in,
+ size_t total_in_len,
+ struct scatterlist *sg_out);
void *mei_cldev_dma_map(struct mei_cl_device *cldev, u8 buffer_id, size_t size);
int mei_cldev_dma_unmap(struct mei_cl_device *cldev);
--
2.37.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v5 04/15] mei: bus: extend bus API to support command streamer API
2022-09-13 0:57 ` [PATCH v5 04/15] mei: bus: extend bus API to support command streamer API Daniele Ceraolo Spurio
@ 2022-09-24 12:16 ` Greg Kroah-Hartman
0 siblings, 0 replies; 27+ messages in thread
From: Greg Kroah-Hartman @ 2022-09-24 12:16 UTC (permalink / raw)
To: Daniele Ceraolo Spurio; +Cc: intel-gfx, Tomas Winkler, Vitaly Lubart, dri-devel
On Mon, Sep 12, 2022 at 05:57:28PM -0700, Daniele Ceraolo Spurio wrote:
> From: Vitaly Lubart <vitaly.lubart@intel.com>
>
> Add mei bus API for sending gsc commands: mei_cldev_send_gsc_command()
>
> The GSC commands are originated in the graphics stack
> and are in form of SGL DMA buffers.
> The GSC commands are synchronous, the response is received
> in the same call on the out sg list buffers.
> The function setups pointers for in and out sg lists in the
> mei sgl extended header and sends it to the firmware.
>
> Signed-off-by: Vitaly Lubart <vitaly.lubart@intel.com>
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v5 05/15] mei: pxp: add command streamer API to the PXP driver
2022-09-13 0:57 [PATCH v5 00/15] drm/i915: HuC loading for DG2 Daniele Ceraolo Spurio
` (3 preceding siblings ...)
2022-09-13 0:57 ` [PATCH v5 04/15] mei: bus: extend bus API to support command streamer API Daniele Ceraolo Spurio
@ 2022-09-13 0:57 ` Daniele Ceraolo Spurio
2022-09-24 12:16 ` Greg Kroah-Hartman
2022-09-13 0:57 ` [PATCH v5 06/15] mei: pxp: support matching with a gfx discrete card Daniele Ceraolo Spurio
` (10 subsequent siblings)
15 siblings, 1 reply; 27+ messages in thread
From: Daniele Ceraolo Spurio @ 2022-09-13 0:57 UTC (permalink / raw)
To: intel-gfx
Cc: Alan Previn, Greg Kroah-Hartman, dri-devel,
Daniele Ceraolo Spurio, Tomas Winkler, Vitaly Lubart
From: Vitaly Lubart <vitaly.lubart@intel.com>
The discrete graphics card with GSC firmware
using command streamer API hence it requires to enhance
pxp module with the new gsc_command() handler.
The handler is implemented via mei_pxp_gsc_command() which is
just a thin wrapper around mei_cldev_send_gsc_command()
Signed-off-by: Vitaly Lubart <vitaly.lubart@intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>
---
V2:
1. More detailed commit message
2. Fix typo in the comments
V3: Rebase
V4:
1. Use forward declaration for struct scatterlist (Jani)
2. Drop double 'just' in the commit message
V5:
1. Drop usless input params checks in mei_pxp_gsc_command (Greg)
drivers/misc/mei/pxp/mei_pxp.c | 25 +++++++++++++++++++++++++
include/drm/i915_pxp_tee_interface.h | 5 +++++
2 files changed, 30 insertions(+)
diff --git a/drivers/misc/mei/pxp/mei_pxp.c b/drivers/misc/mei/pxp/mei_pxp.c
index 5c39457e3f53..f221464c4009 100644
--- a/drivers/misc/mei/pxp/mei_pxp.c
+++ b/drivers/misc/mei/pxp/mei_pxp.c
@@ -77,10 +77,35 @@ mei_pxp_receive_message(struct device *dev, void *buffer, size_t size)
return byte;
}
+/**
+ * mei_pxp_gsc_command() - sends a gsc command, by sending
+ * a sgl mei message to gsc and receiving reply from gsc
+ *
+ * @dev: device corresponding to the mei_cl_device
+ * @client_id: client id to send the command to
+ * @fence_id: fence id to send the command to
+ * @sg_in: scatter gather list containing addresses for rx message buffer
+ * @total_in_len: total length of data in 'in' sg, can be less than the sum of buffers sizes
+ * @sg_out: scatter gather list containing addresses for tx message buffer
+ *
+ * Return: bytes sent on Success, <0 on Failure
+ */
+static ssize_t mei_pxp_gsc_command(struct device *dev, u8 client_id, u32 fence_id,
+ struct scatterlist *sg_in, size_t total_in_len,
+ struct scatterlist *sg_out)
+{
+ struct mei_cl_device *cldev;
+
+ cldev = to_mei_cl_device(dev);
+
+ return mei_cldev_send_gsc_command(cldev, client_id, fence_id, sg_in, total_in_len, sg_out);
+}
+
static const struct i915_pxp_component_ops mei_pxp_ops = {
.owner = THIS_MODULE,
.send = mei_pxp_send_message,
.recv = mei_pxp_receive_message,
+ .gsc_command = mei_pxp_gsc_command,
};
static int mei_component_master_bind(struct device *dev)
diff --git a/include/drm/i915_pxp_tee_interface.h b/include/drm/i915_pxp_tee_interface.h
index af593ec64469..a702b6ec17f7 100644
--- a/include/drm/i915_pxp_tee_interface.h
+++ b/include/drm/i915_pxp_tee_interface.h
@@ -8,6 +8,7 @@
#include <linux/mutex.h>
#include <linux/device.h>
+struct scatterlist;
/**
* struct i915_pxp_component_ops - ops for PXP services.
@@ -23,6 +24,10 @@ struct i915_pxp_component_ops {
int (*send)(struct device *dev, const void *message, size_t size);
int (*recv)(struct device *dev, void *buffer, size_t size);
+ ssize_t (*gsc_command)(struct device *dev, u8 client_id, u32 fence_id,
+ struct scatterlist *sg_in, size_t total_in_len,
+ struct scatterlist *sg_out);
+
};
/**
--
2.37.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v5 05/15] mei: pxp: add command streamer API to the PXP driver
2022-09-13 0:57 ` [PATCH v5 05/15] mei: pxp: add command streamer API to the PXP driver Daniele Ceraolo Spurio
@ 2022-09-24 12:16 ` Greg Kroah-Hartman
0 siblings, 0 replies; 27+ messages in thread
From: Greg Kroah-Hartman @ 2022-09-24 12:16 UTC (permalink / raw)
To: Daniele Ceraolo Spurio
Cc: intel-gfx, Tomas Winkler, Alan Previn, Vitaly Lubart, dri-devel
On Mon, Sep 12, 2022 at 05:57:29PM -0700, Daniele Ceraolo Spurio wrote:
> From: Vitaly Lubart <vitaly.lubart@intel.com>
>
> The discrete graphics card with GSC firmware
> using command streamer API hence it requires to enhance
> pxp module with the new gsc_command() handler.
>
> The handler is implemented via mei_pxp_gsc_command() which is
> just a thin wrapper around mei_cldev_send_gsc_command()
>
> Signed-off-by: Vitaly Lubart <vitaly.lubart@intel.com>
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>
> ---
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v5 06/15] mei: pxp: support matching with a gfx discrete card
2022-09-13 0:57 [PATCH v5 00/15] drm/i915: HuC loading for DG2 Daniele Ceraolo Spurio
` (4 preceding siblings ...)
2022-09-13 0:57 ` [PATCH v5 05/15] mei: pxp: add command streamer API to the PXP driver Daniele Ceraolo Spurio
@ 2022-09-13 0:57 ` Daniele Ceraolo Spurio
2022-09-24 12:16 ` Greg Kroah-Hartman
2022-09-13 0:57 ` [PATCH v5 07/15] drm/i915/pxp: load the pxp module when we have a gsc-loaded huc Daniele Ceraolo Spurio
` (9 subsequent siblings)
15 siblings, 1 reply; 27+ messages in thread
From: Daniele Ceraolo Spurio @ 2022-09-13 0:57 UTC (permalink / raw)
To: intel-gfx
Cc: Alan Previn, Greg Kroah-Hartman, dri-devel,
Daniele Ceraolo Spurio, Tomas Winkler, Vitaly Lubart
From: Tomas Winkler <tomas.winkler@intel.com>
With on-boards graphics card, both i915 and MEI
are in the same device hierarchy with the same parent,
while for discrete gfx card the MEI is its child device.
Adjust the match function for that scenario
by matching MEI parent device with i915.
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Vitaly Lubart <vitaly.lubart@intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>
---
V2:
1. More detailed commit message
2. Check for dev is not null before it is accessed.
V3-5: Rebase
drivers/misc/mei/pxp/mei_pxp.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/misc/mei/pxp/mei_pxp.c b/drivers/misc/mei/pxp/mei_pxp.c
index f221464c4009..8dd09b1722eb 100644
--- a/drivers/misc/mei/pxp/mei_pxp.c
+++ b/drivers/misc/mei/pxp/mei_pxp.c
@@ -156,17 +156,24 @@ static int mei_pxp_component_match(struct device *dev, int subcomponent,
{
struct device *base = data;
+ if (!dev)
+ return 0;
+
if (!dev->driver || strcmp(dev->driver->name, "i915") ||
subcomponent != I915_COMPONENT_PXP)
return 0;
base = base->parent;
- if (!base)
+ if (!base) /* mei device */
return 0;
- base = base->parent;
- dev = dev->parent;
+ base = base->parent; /* pci device */
+ /* for dgfx */
+ if (base && dev == base)
+ return 1;
+ /* for pch */
+ dev = dev->parent;
return (base && dev && dev == base);
}
--
2.37.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v5 06/15] mei: pxp: support matching with a gfx discrete card
2022-09-13 0:57 ` [PATCH v5 06/15] mei: pxp: support matching with a gfx discrete card Daniele Ceraolo Spurio
@ 2022-09-24 12:16 ` Greg Kroah-Hartman
0 siblings, 0 replies; 27+ messages in thread
From: Greg Kroah-Hartman @ 2022-09-24 12:16 UTC (permalink / raw)
To: Daniele Ceraolo Spurio
Cc: intel-gfx, Alan Previn, Tomas Winkler, dri-devel, Vitaly Lubart
On Mon, Sep 12, 2022 at 05:57:30PM -0700, Daniele Ceraolo Spurio wrote:
> From: Tomas Winkler <tomas.winkler@intel.com>
>
> With on-boards graphics card, both i915 and MEI
> are in the same device hierarchy with the same parent,
> while for discrete gfx card the MEI is its child device.
> Adjust the match function for that scenario
> by matching MEI parent device with i915.
>
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Vitaly Lubart <vitaly.lubart@intel.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>
> ---
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v5 07/15] drm/i915/pxp: load the pxp module when we have a gsc-loaded huc
2022-09-13 0:57 [PATCH v5 00/15] drm/i915: HuC loading for DG2 Daniele Ceraolo Spurio
` (5 preceding siblings ...)
2022-09-13 0:57 ` [PATCH v5 06/15] mei: pxp: support matching with a gfx discrete card Daniele Ceraolo Spurio
@ 2022-09-13 0:57 ` Daniele Ceraolo Spurio
2022-09-13 0:57 ` [PATCH v5 08/15] drm/i915/pxp: implement function for sending tee stream command Daniele Ceraolo Spurio
` (8 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Daniele Ceraolo Spurio @ 2022-09-13 0:57 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniele Ceraolo Spurio, Alan Previn, dri-devel
The mei_pxp module is required to send the command to load authenticate
the HuC to the GSC even if pxp is not in use for protected content
management.
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Alan Previn <alan.previn.teres.alexis@intel.com>
Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>
---
drivers/gpu/drm/i915/Makefile | 10 +++---
drivers/gpu/drm/i915/pxp/intel_pxp.c | 32 +++++++++++++-------
drivers/gpu/drm/i915/pxp/intel_pxp.h | 32 --------------------
drivers/gpu/drm/i915/pxp/intel_pxp_irq.h | 8 +++++
drivers/gpu/drm/i915/pxp/intel_pxp_session.c | 8 ++++-
drivers/gpu/drm/i915/pxp/intel_pxp_session.h | 11 +++++--
drivers/gpu/drm/i915/pxp/intel_pxp_tee.c | 10 ++++--
7 files changed, 57 insertions(+), 54 deletions(-)
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index a26edcdadc21..26fc2f23c4e0 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -309,15 +309,17 @@ i915-y += \
i915-y += i915_perf.o
-# Protected execution platform (PXP) support
-i915-$(CONFIG_DRM_I915_PXP) += \
+# Protected execution platform (PXP) support. Base support is required for HuC
+i915-y += \
pxp/intel_pxp.o \
+ pxp/intel_pxp_tee.o
+
+i915-$(CONFIG_DRM_I915_PXP) += \
pxp/intel_pxp_cmd.o \
pxp/intel_pxp_debugfs.o \
pxp/intel_pxp_irq.o \
pxp/intel_pxp_pm.o \
- pxp/intel_pxp_session.o \
- pxp/intel_pxp_tee.o
+ pxp/intel_pxp_session.o
# Post-mortem debug and GPU hang state capture
i915-$(CONFIG_DRM_I915_CAPTURE_ERROR) += i915_gpu_error.o
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.c b/drivers/gpu/drm/i915/pxp/intel_pxp.c
index 69cdaaddc4a9..5efe61f67546 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp.c
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp.c
@@ -103,19 +103,15 @@ static int create_vcs_context(struct intel_pxp *pxp)
static void destroy_vcs_context(struct intel_pxp *pxp)
{
- intel_engine_destroy_pinned_context(fetch_and_zero(&pxp->ce));
+ if (pxp->ce)
+ intel_engine_destroy_pinned_context(fetch_and_zero(&pxp->ce));
}
-void intel_pxp_init(struct intel_pxp *pxp)
+static void pxp_init_full(struct intel_pxp *pxp)
{
struct intel_gt *gt = pxp_to_gt(pxp);
int ret;
- if (!HAS_PXP(gt->i915))
- return;
-
- mutex_init(&pxp->tee_mutex);
-
/*
* we'll use the completion to check if there is a termination pending,
* so we start it as completed and we reinit it when a termination
@@ -124,8 +120,7 @@ void intel_pxp_init(struct intel_pxp *pxp)
init_completion(&pxp->termination);
complete_all(&pxp->termination);
- mutex_init(&pxp->arb_mutex);
- INIT_WORK(&pxp->session_work, intel_pxp_session_work);
+ intel_pxp_session_management_init(pxp);
ret = create_vcs_context(pxp);
if (ret)
@@ -143,11 +138,26 @@ void intel_pxp_init(struct intel_pxp *pxp)
destroy_vcs_context(pxp);
}
-void intel_pxp_fini(struct intel_pxp *pxp)
+void intel_pxp_init(struct intel_pxp *pxp)
{
- if (!intel_pxp_is_enabled(pxp))
+ struct intel_gt *gt = pxp_to_gt(pxp);
+
+ /* we rely on the mei PXP module */
+ if (!IS_ENABLED(CONFIG_INTEL_MEI_PXP))
return;
+ /*
+ * If HuC is loaded by GSC but PXP is disabled, we can skip the init of
+ * the full PXP session/object management and just init the tee channel.
+ */
+ if (HAS_PXP(gt->i915))
+ pxp_init_full(pxp);
+ else if (intel_huc_is_loaded_by_gsc(>->uc.huc) && intel_uc_uses_huc(>->uc))
+ intel_pxp_tee_component_init(pxp);
+}
+
+void intel_pxp_fini(struct intel_pxp *pxp)
+{
pxp->arb_is_valid = false;
intel_pxp_tee_component_fini(pxp);
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.h b/drivers/gpu/drm/i915/pxp/intel_pxp.h
index 73847e535cab..2da309088c6d 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp.h
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp.h
@@ -12,7 +12,6 @@
struct intel_pxp;
struct drm_i915_gem_object;
-#ifdef CONFIG_DRM_I915_PXP
struct intel_gt *pxp_to_gt(const struct intel_pxp *pxp);
bool intel_pxp_is_enabled(const struct intel_pxp *pxp);
bool intel_pxp_is_active(const struct intel_pxp *pxp);
@@ -32,36 +31,5 @@ int intel_pxp_key_check(struct intel_pxp *pxp,
bool assign);
void intel_pxp_invalidate(struct intel_pxp *pxp);
-#else
-static inline void intel_pxp_init(struct intel_pxp *pxp)
-{
-}
-
-static inline void intel_pxp_fini(struct intel_pxp *pxp)
-{
-}
-
-static inline int intel_pxp_start(struct intel_pxp *pxp)
-{
- return -ENODEV;
-}
-
-static inline bool intel_pxp_is_enabled(const struct intel_pxp *pxp)
-{
- return false;
-}
-
-static inline bool intel_pxp_is_active(const struct intel_pxp *pxp)
-{
- return false;
-}
-
-static inline int intel_pxp_key_check(struct intel_pxp *pxp,
- struct drm_i915_gem_object *obj,
- bool assign)
-{
- return -ENODEV;
-}
-#endif
#endif /* __INTEL_PXP_H__ */
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_irq.h b/drivers/gpu/drm/i915/pxp/intel_pxp_irq.h
index 8b5793654844..8c292dc86f68 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_irq.h
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_irq.h
@@ -27,6 +27,14 @@ void intel_pxp_irq_handler(struct intel_pxp *pxp, u16 iir);
static inline void intel_pxp_irq_handler(struct intel_pxp *pxp, u16 iir)
{
}
+
+static inline void intel_pxp_irq_enable(struct intel_pxp *pxp)
+{
+}
+
+static inline void intel_pxp_irq_disable(struct intel_pxp *pxp)
+{
+}
#endif
#endif /* __INTEL_PXP_IRQ_H__ */
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_session.c b/drivers/gpu/drm/i915/pxp/intel_pxp_session.c
index 1bb5b5249157..ecff0935adbf 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_session.c
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_session.c
@@ -137,7 +137,7 @@ static void pxp_terminate_complete(struct intel_pxp *pxp)
complete_all(&pxp->termination);
}
-void intel_pxp_session_work(struct work_struct *work)
+static void pxp_session_work(struct work_struct *work)
{
struct intel_pxp *pxp = container_of(work, typeof(*pxp), session_work);
struct intel_gt *gt = pxp_to_gt(pxp);
@@ -172,3 +172,9 @@ void intel_pxp_session_work(struct work_struct *work)
intel_runtime_pm_put(gt->uncore->rpm, wakeref);
}
+
+void intel_pxp_session_management_init(struct intel_pxp *pxp)
+{
+ mutex_init(&pxp->arb_mutex);
+ INIT_WORK(&pxp->session_work, pxp_session_work);
+}
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_session.h b/drivers/gpu/drm/i915/pxp/intel_pxp_session.h
index ba4c9d2b94b7..903ac52cffa1 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_session.h
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_session.h
@@ -8,8 +8,13 @@
#include <linux/types.h>
-struct work_struct;
-
-void intel_pxp_session_work(struct work_struct *work);
+struct intel_pxp;
+#ifdef CONFIG_DRM_I915_PXP
+void intel_pxp_session_management_init(struct intel_pxp *pxp);
+#else
+static inline void intel_pxp_session_management_init(struct intel_pxp *pxp)
+{
+}
+#endif
#endif /* __INTEL_PXP_SESSION_H__ */
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
index 4b6f5655fab5..2c1fc49ecec1 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
@@ -97,7 +97,8 @@ static int i915_pxp_tee_component_bind(struct device *i915_kdev,
return 0;
/* the component is required to fully start the PXP HW */
- intel_pxp_init_hw(pxp);
+ if (intel_pxp_is_enabled(pxp))
+ intel_pxp_init_hw(pxp);
intel_runtime_pm_put(&i915->runtime_pm, wakeref);
@@ -111,8 +112,9 @@ static void i915_pxp_tee_component_unbind(struct device *i915_kdev,
struct intel_pxp *pxp = i915_dev_to_pxp(i915_kdev);
intel_wakeref_t wakeref;
- with_intel_runtime_pm_if_in_use(&i915->runtime_pm, wakeref)
- intel_pxp_fini_hw(pxp);
+ if (intel_pxp_is_enabled(pxp))
+ with_intel_runtime_pm_if_in_use(&i915->runtime_pm, wakeref)
+ intel_pxp_fini_hw(pxp);
mutex_lock(&pxp->tee_mutex);
pxp->pxp_component = NULL;
@@ -130,6 +132,8 @@ int intel_pxp_tee_component_init(struct intel_pxp *pxp)
struct intel_gt *gt = pxp_to_gt(pxp);
struct drm_i915_private *i915 = gt->i915;
+ mutex_init(&pxp->tee_mutex);
+
ret = component_add_typed(i915->drm.dev, &i915_pxp_tee_component_ops,
I915_COMPONENT_PXP);
if (ret < 0) {
--
2.37.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v5 08/15] drm/i915/pxp: implement function for sending tee stream command
2022-09-13 0:57 [PATCH v5 00/15] drm/i915: HuC loading for DG2 Daniele Ceraolo Spurio
` (6 preceding siblings ...)
2022-09-13 0:57 ` [PATCH v5 07/15] drm/i915/pxp: load the pxp module when we have a gsc-loaded huc Daniele Ceraolo Spurio
@ 2022-09-13 0:57 ` Daniele Ceraolo Spurio
2022-09-13 0:57 ` [PATCH v5 09/15] drm/i915/pxp: add huc authentication and loading command Daniele Ceraolo Spurio
` (7 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Daniele Ceraolo Spurio @ 2022-09-13 0:57 UTC (permalink / raw)
To: intel-gfx
Cc: Alan Previn, dri-devel, Daniele Ceraolo Spurio, Rodrigo Vivi,
Tomas Winkler, Vitaly Lubart
From: Vitaly Lubart <vitaly.lubart@intel.com>
Command to be sent via the stream interface are written to a local
memory page, whose address is then provided to the GSC.
The interface supports providing a full sg with multiple pages for both
input and output messages, but since for now we only aim to support short
and synchronous messages we can use a single page for both input and
output.
Note that the mei interface expects an sg of 4k pages, while our lmem pages
are 64k. If we ever need to support more than 4k we'll need to convert.
Added a TODO comment to the code to record this.
Signed-off-by: Vitaly Lubart <vitaly.lubart@intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Alan Previn <alan.previn.teres.alexis@intel.com>
Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>
---
drivers/gpu/drm/i915/pxp/intel_pxp_tee.c | 114 ++++++++++++++++++++-
drivers/gpu/drm/i915/pxp/intel_pxp_tee.h | 5 +
drivers/gpu/drm/i915/pxp/intel_pxp_types.h | 6 ++
3 files changed, 124 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
index 2c1fc49ecec1..e0d09455a92e 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
@@ -7,6 +7,7 @@
#include <drm/i915_pxp_tee_interface.h>
#include <drm/i915_component.h>
+#include "gem/i915_gem_region.h"
#include "i915_drv.h"
#include "intel_pxp.h"
@@ -69,6 +70,47 @@ static int intel_pxp_tee_io_message(struct intel_pxp *pxp,
return ret;
}
+int intel_pxp_tee_stream_message(struct intel_pxp *pxp,
+ u8 client_id, u32 fence_id,
+ void *msg_in, size_t msg_in_len,
+ void *msg_out, size_t msg_out_len)
+{
+ /* TODO: for bigger objects we need to use a sg of 4k pages */
+ const size_t max_msg_size = PAGE_SIZE;
+ struct drm_i915_private *i915 = pxp_to_gt(pxp)->i915;
+ struct i915_pxp_component *pxp_component = pxp->pxp_component;
+ unsigned int offset = 0;
+ struct scatterlist *sg;
+ int ret;
+
+ if (msg_in_len > max_msg_size || msg_out_len > max_msg_size)
+ return -ENOSPC;
+
+ mutex_lock(&pxp->tee_mutex);
+
+ if (unlikely(!pxp_component || !pxp_component->ops->gsc_command)) {
+ ret = -ENODEV;
+ goto unlock;
+ }
+
+ GEM_BUG_ON(!pxp->stream_cmd.obj);
+
+ sg = i915_gem_object_get_sg_dma(pxp->stream_cmd.obj, 0, &offset);
+
+ memcpy(pxp->stream_cmd.vaddr, msg_in, msg_in_len);
+
+ ret = pxp_component->ops->gsc_command(pxp_component->tee_dev, client_id,
+ fence_id, sg, msg_in_len, sg);
+ if (ret < 0)
+ drm_err(&i915->drm, "Failed to send PXP TEE gsc command\n");
+ else
+ memcpy(msg_out, pxp->stream_cmd.vaddr, msg_out_len);
+
+unlock:
+ mutex_unlock(&pxp->tee_mutex);
+ return ret;
+}
+
/**
* i915_pxp_tee_component_bind - bind function to pass the function pointers to pxp_tee
* @i915_kdev: pointer to i915 kernel device
@@ -126,6 +168,66 @@ static const struct component_ops i915_pxp_tee_component_ops = {
.unbind = i915_pxp_tee_component_unbind,
};
+static int alloc_streaming_command(struct intel_pxp *pxp)
+{
+ struct drm_i915_private *i915 = pxp_to_gt(pxp)->i915;
+ struct drm_i915_gem_object *obj = NULL;
+ void *cmd;
+ int err;
+
+ pxp->stream_cmd.obj = NULL;
+ pxp->stream_cmd.vaddr = NULL;
+
+ if (!IS_DGFX(i915))
+ return 0;
+
+ /* allocate lmem object of one page for PXP command memory and store it */
+ obj = i915_gem_object_create_lmem(i915, PAGE_SIZE, I915_BO_ALLOC_CONTIGUOUS);
+ if (IS_ERR(obj)) {
+ drm_err(&i915->drm, "Failed to allocate pxp streaming command!\n");
+ return PTR_ERR(obj);
+ }
+
+ err = i915_gem_object_pin_pages_unlocked(obj);
+ if (err) {
+ drm_err(&i915->drm, "Failed to pin gsc message page!\n");
+ goto out_put;
+ }
+
+ /* map the lmem into the virtual memory pointer */
+ cmd = i915_gem_object_pin_map_unlocked(obj, i915_coherent_map_type(i915, obj, true));
+ if (IS_ERR(cmd)) {
+ drm_err(&i915->drm, "Failed to map gsc message page!\n");
+ err = PTR_ERR(cmd);
+ goto out_unpin;
+ }
+
+ memset(cmd, 0, obj->base.size);
+
+ pxp->stream_cmd.obj = obj;
+ pxp->stream_cmd.vaddr = cmd;
+
+ return 0;
+
+out_unpin:
+ i915_gem_object_unpin_pages(obj);
+out_put:
+ i915_gem_object_put(obj);
+ return err;
+}
+
+static void free_streaming_command(struct intel_pxp *pxp)
+{
+ struct drm_i915_gem_object *obj = fetch_and_zero(&pxp->stream_cmd.obj);
+
+ if (!obj)
+ return;
+
+ i915_gem_object_unpin_map(obj);
+ i915_gem_object_unpin_pages(obj);
+ i915_gem_object_put(obj);
+}
+
int intel_pxp_tee_component_init(struct intel_pxp *pxp)
{
int ret;
@@ -134,16 +236,24 @@ int intel_pxp_tee_component_init(struct intel_pxp *pxp)
mutex_init(&pxp->tee_mutex);
+ ret = alloc_streaming_command(pxp);
+ if (ret)
+ return ret;
+
ret = component_add_typed(i915->drm.dev, &i915_pxp_tee_component_ops,
I915_COMPONENT_PXP);
if (ret < 0) {
drm_err(&i915->drm, "Failed to add PXP component (%d)\n", ret);
- return ret;
+ goto out_free;
}
pxp->pxp_component_added = true;
return 0;
+
+out_free:
+ free_streaming_command(pxp);
+ return ret;
}
void intel_pxp_tee_component_fini(struct intel_pxp *pxp)
@@ -155,6 +265,8 @@ void intel_pxp_tee_component_fini(struct intel_pxp *pxp)
component_del(i915->drm.dev, &i915_pxp_tee_component_ops);
pxp->pxp_component_added = false;
+
+ free_streaming_command(pxp);
}
int intel_pxp_tee_cmd_create_arb_session(struct intel_pxp *pxp,
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.h b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.h
index c136053ce340..aeb3dfe7ce96 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.h
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.h
@@ -14,4 +14,9 @@ void intel_pxp_tee_component_fini(struct intel_pxp *pxp);
int intel_pxp_tee_cmd_create_arb_session(struct intel_pxp *pxp,
int arb_session_id);
+int intel_pxp_tee_stream_message(struct intel_pxp *pxp,
+ u8 client_id, u32 fence_id,
+ void *msg_in, size_t msg_in_len,
+ void *msg_out, size_t msg_out_len);
+
#endif /* __INTEL_PXP_TEE_H__ */
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_types.h b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h
index 7ce5f37ee12e..f74b1e11a505 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_types.h
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h
@@ -53,6 +53,12 @@ struct intel_pxp {
/** @tee_mutex: protects the tee channel binding and messaging. */
struct mutex tee_mutex;
+ /** @stream_cmd: LMEM obj used to send stream PXP commands to the GSC */
+ struct {
+ struct drm_i915_gem_object *obj; /* contains PXP command memory */
+ void *vaddr; /* virtual memory for PXP command */
+ } stream_cmd;
+
/**
* @hw_state_invalidated: if the HW perceives an attack on the integrity
* of the encryption it will invalidate the keys and expect SW to
--
2.37.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v5 09/15] drm/i915/pxp: add huc authentication and loading command
2022-09-13 0:57 [PATCH v5 00/15] drm/i915: HuC loading for DG2 Daniele Ceraolo Spurio
` (7 preceding siblings ...)
2022-09-13 0:57 ` [PATCH v5 08/15] drm/i915/pxp: implement function for sending tee stream command Daniele Ceraolo Spurio
@ 2022-09-13 0:57 ` Daniele Ceraolo Spurio
2022-09-13 0:57 ` [PATCH v5 10/15] drm/i915/dg2: setup HuC loading via GSC Daniele Ceraolo Spurio
` (6 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Daniele Ceraolo Spurio @ 2022-09-13 0:57 UTC (permalink / raw)
To: intel-gfx
Cc: Tomas Winkler, Daniele Ceraolo Spurio, Alan Previn,
Vitaly Lubart, dri-devel
From: Tomas Winkler <tomas.winkler@intel.com>
Add support for loading HuC via a pxp stream command.
V4:
1. Remove unnecessary include in intel_pxp_huc.h (Jani)
2. Adjust copyright year to 2022
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Vitaly Lubart <vitaly.lubart@intel.com>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Alan Previn <alan.previn.teres.alexis@intel.com>
Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>
---
drivers/gpu/drm/i915/Makefile | 3 +-
drivers/gpu/drm/i915/pxp/intel_pxp_huc.c | 69 +++++++++++++++++++
drivers/gpu/drm/i915/pxp/intel_pxp_huc.h | 13 ++++
.../drm/i915/pxp/intel_pxp_tee_interface.h | 23 ++++++-
4 files changed, 106 insertions(+), 2 deletions(-)
create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp_huc.c
create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp_huc.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 26fc2f23c4e0..f8cc1eb52626 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -312,7 +312,8 @@ i915-y += i915_perf.o
# Protected execution platform (PXP) support. Base support is required for HuC
i915-y += \
pxp/intel_pxp.o \
- pxp/intel_pxp_tee.o
+ pxp/intel_pxp_tee.o \
+ pxp/intel_pxp_huc.o
i915-$(CONFIG_DRM_I915_PXP) += \
pxp/intel_pxp_cmd.o \
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_huc.c b/drivers/gpu/drm/i915/pxp/intel_pxp_huc.c
new file mode 100644
index 000000000000..7ec36d94e758
--- /dev/null
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_huc.c
@@ -0,0 +1,69 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright(c) 2021-2022, Intel Corporation. All rights reserved.
+ */
+
+#include "drm/i915_drm.h"
+#include "i915_drv.h"
+
+#include "gem/i915_gem_region.h"
+#include "gt/intel_gt.h"
+
+#include "intel_pxp.h"
+#include "intel_pxp_huc.h"
+#include "intel_pxp_tee.h"
+#include "intel_pxp_types.h"
+#include "intel_pxp_tee_interface.h"
+
+int intel_pxp_huc_load_and_auth(struct intel_pxp *pxp)
+{
+ struct intel_gt *gt = pxp_to_gt(pxp);
+ struct intel_huc *huc = >->uc.huc;
+ struct pxp_tee_start_huc_auth_in huc_in = {0};
+ struct pxp_tee_start_huc_auth_out huc_out = {0};
+ dma_addr_t huc_phys_addr;
+ u8 client_id = 0;
+ u8 fence_id = 0;
+ int err;
+
+ if (!pxp->pxp_component)
+ return -ENODEV;
+
+ huc_phys_addr = i915_gem_object_get_dma_address(huc->fw.obj, 0);
+
+ /* write the PXP message into the lmem (the sg list) */
+ huc_in.header.api_version = PXP_TEE_43_APIVER;
+ huc_in.header.command_id = PXP_TEE_43_START_HUC_AUTH;
+ huc_in.header.status = 0;
+ huc_in.header.buffer_len = sizeof(huc_in.huc_base_address);
+ huc_in.huc_base_address = huc_phys_addr;
+
+ err = intel_pxp_tee_stream_message(pxp, client_id, fence_id,
+ &huc_in, sizeof(huc_in),
+ &huc_out, sizeof(huc_out));
+ if (err < 0) {
+ drm_err(>->i915->drm,
+ "Failed to send HuC load and auth command to GSC [%d]!\n",
+ err);
+ return err;
+ }
+
+ /*
+ * HuC does sometimes survive suspend/resume (it depends on how "deep"
+ * a sleep state the device reaches) so we can end up here on resume
+ * with HuC already loaded, in which case the GSC will return
+ * PXP_STATUS_OP_NOT_PERMITTED. We can therefore consider the GuC
+ * correctly transferred in this scenario; if the same error is ever
+ * returned with HuC not loaded we'll still catch it when we check the
+ * authentication bit later.
+ */
+ if (huc_out.header.status != PXP_STATUS_SUCCESS &&
+ huc_out.header.status != PXP_STATUS_OP_NOT_PERMITTED) {
+ drm_err(>->i915->drm,
+ "HuC load failed with GSC error = 0x%x\n",
+ huc_out.header.status);
+ return -EPROTO;
+ }
+
+ return 0;
+}
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_huc.h b/drivers/gpu/drm/i915/pxp/intel_pxp_huc.h
new file mode 100644
index 000000000000..e40847a91c39
--- /dev/null
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_huc.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright(c) 2021-2022, Intel Corporation. All rights reserved.
+ */
+
+#ifndef __INTEL_PXP_HUC_H__
+#define __INTEL_PXP_HUC_H__
+
+struct intel_pxp;
+
+int intel_pxp_huc_load_and_auth(struct intel_pxp *pxp);
+
+#endif /* __INTEL_PXP_HUC_H__ */
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_tee_interface.h b/drivers/gpu/drm/i915/pxp/intel_pxp_tee_interface.h
index 36e9b0868f5c..7edc1760f142 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_tee_interface.h
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_tee_interface.h
@@ -1,6 +1,6 @@
/* SPDX-License-Identifier: MIT */
/*
- * Copyright(c) 2020, Intel Corporation. All rights reserved.
+ * Copyright(c) 2020-2022, Intel Corporation. All rights reserved.
*/
#ifndef __INTEL_PXP_TEE_INTERFACE_H__
@@ -9,8 +9,20 @@
#include <linux/types.h>
#define PXP_TEE_APIVER 0x40002
+#define PXP_TEE_43_APIVER 0x00040003
#define PXP_TEE_ARB_CMDID 0x1e
#define PXP_TEE_ARB_PROTECTION_MODE 0x2
+#define PXP_TEE_43_START_HUC_AUTH 0x0000003A
+
+/*
+ * there are a lot of status codes for PXP, but we only define the ones we
+ * actually can handle in the driver. other failure codes will be printed to
+ * error msg for debug.
+ */
+enum pxp_status {
+ PXP_STATUS_SUCCESS = 0x0,
+ PXP_STATUS_OP_NOT_PERMITTED = 0x4013
+};
/* PXP TEE message header */
struct pxp_tee_cmd_header {
@@ -33,4 +45,13 @@ struct pxp_tee_create_arb_out {
struct pxp_tee_cmd_header header;
} __packed;
+struct pxp_tee_start_huc_auth_in {
+ struct pxp_tee_cmd_header header;
+ __le64 huc_base_address;
+};
+
+struct pxp_tee_start_huc_auth_out {
+ struct pxp_tee_cmd_header header;
+};
+
#endif /* __INTEL_PXP_TEE_INTERFACE_H__ */
--
2.37.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v5 10/15] drm/i915/dg2: setup HuC loading via GSC
2022-09-13 0:57 [PATCH v5 00/15] drm/i915: HuC loading for DG2 Daniele Ceraolo Spurio
` (8 preceding siblings ...)
2022-09-13 0:57 ` [PATCH v5 09/15] drm/i915/pxp: add huc authentication and loading command Daniele Ceraolo Spurio
@ 2022-09-13 0:57 ` Daniele Ceraolo Spurio
2022-09-13 0:57 ` [PATCH v5 11/15] drm/i915/huc: track delayed HuC load with a fence Daniele Ceraolo Spurio
` (5 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Daniele Ceraolo Spurio @ 2022-09-13 0:57 UTC (permalink / raw)
To: intel-gfx
Cc: Tomas Winkler, Daniele Ceraolo Spurio, Alan Previn,
Vitaly Lubart, dri-devel
The GSC will perform both the load and the authentication, so we just
need to check the auth bit after the GSC has replied.
Since we require the PXP module to load the HuC, the earliest we can
trigger the load is during the pxp_bind operation.
Note that GSC-loaded HuC survives GT reset, so we need to just mark it
as ready when we re-init the GT HW.
V2: move setting of HuC fw error state to the failure path of the HuC
auth function, so it covers both the legacy and new auth flows
V4:
1. Fix typo in the commit message
2. style fix in intel_huc_wait_for_auth_complete()
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Signed-off-by: Vitaly Lubart <vitaly.lubart@intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>
---
drivers/gpu/drm/i915/gt/uc/intel_huc.c | 41 +++++++++++++++--------
drivers/gpu/drm/i915/gt/uc/intel_huc.h | 2 ++
drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c | 34 +++++++++++++++++++
drivers/gpu/drm/i915/gt/uc/intel_huc_fw.h | 1 +
drivers/gpu/drm/i915/pxp/intel_pxp_tee.c | 14 +++++++-
5 files changed, 77 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
index 3bb8838e325a..f0188931d8e4 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
@@ -125,6 +125,28 @@ void intel_huc_fini(struct intel_huc *huc)
intel_uc_fw_fini(&huc->fw);
}
+int intel_huc_wait_for_auth_complete(struct intel_huc *huc)
+{
+ 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,
+ 2, 50, NULL);
+
+ if (ret) {
+ drm_err(>->i915->drm, "HuC: Firmware not verified %d\n", ret);
+ intel_uc_fw_change_status(&huc->fw, INTEL_UC_FIRMWARE_LOAD_FAIL);
+ return ret;
+ }
+
+ intel_uc_fw_change_status(&huc->fw, INTEL_UC_FIRMWARE_RUNNING);
+ drm_info(>->i915->drm, "HuC authenticated\n");
+ return 0;
+}
+
/**
* intel_huc_auth() - Authenticate HuC uCode
* @huc: intel_huc structure
@@ -161,27 +183,18 @@ int intel_huc_auth(struct intel_huc *huc)
}
/* Check authentication status, it should be done by now */
- ret = __intel_wait_for_register(gt->uncore,
- huc->status.reg,
- huc->status.mask,
- huc->status.value,
- 2, 50, NULL);
- if (ret) {
- DRM_ERROR("HuC: Firmware not verified %d\n", ret);
+ ret = intel_huc_wait_for_auth_complete(huc);
+ if (ret)
goto fail;
- }
- intel_uc_fw_change_status(&huc->fw, INTEL_UC_FIRMWARE_RUNNING);
- drm_info(>->i915->drm, "HuC authenticated\n");
return 0;
fail:
i915_probe_error(gt->i915, "HuC: Authentication failed %d\n", ret);
- intel_uc_fw_change_status(&huc->fw, INTEL_UC_FIRMWARE_LOAD_FAIL);
return ret;
}
-static bool huc_is_authenticated(struct intel_huc *huc)
+bool intel_huc_is_authenticated(struct intel_huc *huc)
{
struct intel_gt *gt = huc_to_gt(huc);
intel_wakeref_t wakeref;
@@ -223,7 +236,7 @@ int intel_huc_check_status(struct intel_huc *huc)
break;
}
- return huc_is_authenticated(huc);
+ return intel_huc_is_authenticated(huc);
}
void intel_huc_update_auth_status(struct intel_huc *huc)
@@ -231,7 +244,7 @@ void intel_huc_update_auth_status(struct intel_huc *huc)
if (!intel_uc_fw_is_loadable(&huc->fw))
return;
- if (huc_is_authenticated(huc))
+ if (intel_huc_is_authenticated(huc))
intel_uc_fw_change_status(&huc->fw,
INTEL_UC_FIRMWARE_RUNNING);
}
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.h b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
index d7e25b6e879e..51f9d96a3ca3 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
@@ -26,8 +26,10 @@ void intel_huc_init_early(struct intel_huc *huc);
int intel_huc_init(struct intel_huc *huc);
void intel_huc_fini(struct intel_huc *huc);
int intel_huc_auth(struct intel_huc *huc);
+int intel_huc_wait_for_auth_complete(struct intel_huc *huc);
int intel_huc_check_status(struct intel_huc *huc);
void intel_huc_update_auth_status(struct intel_huc *huc);
+bool intel_huc_is_authenticated(struct intel_huc *huc);
static inline int intel_huc_sanitize(struct intel_huc *huc)
{
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
index 9d6ab1e01639..4f246416db17 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
@@ -3,9 +3,43 @@
* Copyright © 2014-2019 Intel Corporation
*/
+#include "gt/intel_gsc.h"
#include "gt/intel_gt.h"
+#include "intel_huc.h"
#include "intel_huc_fw.h"
#include "i915_drv.h"
+#include "pxp/intel_pxp_huc.h"
+
+int intel_huc_fw_load_and_auth_via_gsc(struct intel_huc *huc)
+{
+ int ret;
+
+ if (!intel_huc_is_loaded_by_gsc(huc))
+ return -ENODEV;
+
+ if (!intel_uc_fw_is_loadable(&huc->fw))
+ return -ENOEXEC;
+
+ /*
+ * If we abort a suspend, HuC might still be loaded when the mei
+ * component gets re-bound and this function called again. If so, just
+ * mark the HuC as loaded.
+ */
+ if (intel_huc_is_authenticated(huc)) {
+ intel_uc_fw_change_status(&huc->fw, INTEL_UC_FIRMWARE_RUNNING);
+ return 0;
+ }
+
+ GEM_WARN_ON(intel_uc_fw_is_loaded(&huc->fw));
+
+ ret = intel_pxp_huc_load_and_auth(&huc_to_gt(huc)->pxp);
+ if (ret)
+ return ret;
+
+ intel_uc_fw_change_status(&huc->fw, INTEL_UC_FIRMWARE_TRANSFERRED);
+
+ return intel_huc_wait_for_auth_complete(huc);
+}
/**
* intel_huc_fw_upload() - load HuC uCode to device via DMA transfer
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.h b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.h
index 12f264ee3e0b..db42e238b45f 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.h
@@ -8,6 +8,7 @@
struct intel_huc;
+int intel_huc_fw_load_and_auth_via_gsc(struct intel_huc *huc);
int intel_huc_fw_upload(struct intel_huc *huc);
#endif
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
index e0d09455a92e..00433f59e2c8 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
@@ -14,6 +14,7 @@
#include "intel_pxp_session.h"
#include "intel_pxp_tee.h"
#include "intel_pxp_tee_interface.h"
+#include "intel_pxp_huc.h"
static inline struct intel_pxp *i915_dev_to_pxp(struct device *i915_kdev)
{
@@ -126,13 +127,24 @@ static int i915_pxp_tee_component_bind(struct device *i915_kdev,
{
struct drm_i915_private *i915 = kdev_to_i915(i915_kdev);
struct intel_pxp *pxp = i915_dev_to_pxp(i915_kdev);
+ struct intel_uc *uc = &pxp_to_gt(pxp)->uc;
intel_wakeref_t wakeref;
+ int ret = 0;
mutex_lock(&pxp->tee_mutex);
pxp->pxp_component = data;
pxp->pxp_component->tee_dev = tee_kdev;
mutex_unlock(&pxp->tee_mutex);
+ if (intel_uc_uses_huc(uc) && intel_huc_is_loaded_by_gsc(&uc->huc)) {
+ with_intel_runtime_pm(&i915->runtime_pm, wakeref) {
+ /* load huc via pxp */
+ ret = intel_huc_fw_load_and_auth_via_gsc(&uc->huc);
+ if (ret < 0)
+ drm_err(&i915->drm, "failed to load huc via gsc %d\n", ret);
+ }
+ }
+
/* if we are suspended, the HW will be re-initialized on resume */
wakeref = intel_runtime_pm_get_if_in_use(&i915->runtime_pm);
if (!wakeref)
@@ -144,7 +156,7 @@ static int i915_pxp_tee_component_bind(struct device *i915_kdev,
intel_runtime_pm_put(&i915->runtime_pm, wakeref);
- return 0;
+ return ret;
}
static void i915_pxp_tee_component_unbind(struct device *i915_kdev,
--
2.37.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v5 11/15] drm/i915/huc: track delayed HuC load with a fence
2022-09-13 0:57 [PATCH v5 00/15] drm/i915: HuC loading for DG2 Daniele Ceraolo Spurio
` (9 preceding siblings ...)
2022-09-13 0:57 ` [PATCH v5 10/15] drm/i915/dg2: setup HuC loading via GSC Daniele Ceraolo Spurio
@ 2022-09-13 0:57 ` Daniele Ceraolo Spurio
2022-09-13 0:57 ` [PATCH v5 12/15] drm/i915/huc: stall media submission until HuC is loaded Daniele Ceraolo Spurio
` (4 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Daniele Ceraolo Spurio @ 2022-09-13 0:57 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniele Ceraolo Spurio, Alan Previn, dri-devel
Given that HuC load is delayed on DG2, this patch adds support for a fence
that can be used to wait for load completion. No waiters are added in this
patch (they're coming up in the next one), to keep the focus of the
patch on the tracking logic.
The full HuC loading flow on boot DG2 is as follows:
1) i915 exports the GSC as an aux device;
2) the mei-gsc driver is loaded on the aux device;
3) the mei-pxp component is loaded;
4) mei-pxp calls back into i915 and we load the HuC.
Between steps 1 and 2 there can be several seconds of gap, mainly due to
the kernel doing other work during the boot.
The resume flow is slightly different, because we don't need to
re-expose or re-probe the aux device, so we go directly to step 3 once
i915 and mei-gsc have completed their resume flow.
Here's an example of the boot timing, captured with some logs added to
i915:
[ 17.908307] [drm] adding GSC device
[ 17.915717] [drm] i915 probe done
[ 22.282917] [drm] mei-gsc bound
[ 22.938153] [drm] HuC authenticated
Also to note is that if something goes wrong during GSC HW init the
mei-gsc driver will still bind, but steps 3 and 4 will not happen.
The status tracking is done by registering a bus_notifier to receive a
callback when the mei-gsc driver binds, with a large enough timeout to
account for delays. Once mei-gsc is bound, we switch to a smaller
timeout to wait for the mei-pxp component to load.
The fence is signalled on HuC load complete or if anything goes wrong in
any of the tracking steps. Timeout are enforced via hrtimer callbacks.
v2: fix includes (Jani)
v5: gsc_notifier() remove unneeded ()
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>
---
drivers/gpu/drm/i915/gt/intel_gsc.c | 22 ++-
drivers/gpu/drm/i915/gt/uc/intel_huc.c | 199 +++++++++++++++++++++++++
drivers/gpu/drm/i915/gt/uc/intel_huc.h | 23 +++
3 files changed, 241 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gsc.c b/drivers/gpu/drm/i915/gt/intel_gsc.c
index 7af6db3194dd..f544f70401f8 100644
--- a/drivers/gpu/drm/i915/gt/intel_gsc.c
+++ b/drivers/gpu/drm/i915/gt/intel_gsc.c
@@ -142,8 +142,14 @@ static void gsc_destroy_one(struct drm_i915_private *i915,
struct intel_gsc_intf *intf = &gsc->intf[intf_id];
if (intf->adev) {
- auxiliary_device_delete(&intf->adev->aux_dev);
- auxiliary_device_uninit(&intf->adev->aux_dev);
+ struct auxiliary_device *aux_dev = &intf->adev->aux_dev;
+
+ if (intf_id == 0)
+ intel_huc_unregister_gsc_notifier(&gsc_to_gt(gsc)->uc.huc,
+ aux_dev->dev.bus);
+
+ auxiliary_device_delete(aux_dev);
+ auxiliary_device_uninit(aux_dev);
intf->adev = NULL;
}
@@ -242,14 +248,24 @@ static void gsc_init_one(struct drm_i915_private *i915, struct intel_gsc *gsc,
goto fail;
}
+ intf->adev = adev; /* needed by the notifier */
+
+ if (intf_id == 0)
+ intel_huc_register_gsc_notifier(&gsc_to_gt(gsc)->uc.huc,
+ aux_dev->dev.bus);
+
ret = auxiliary_device_add(aux_dev);
if (ret < 0) {
drm_err(&i915->drm, "gsc aux add failed %d\n", ret);
+ if (intf_id == 0)
+ intel_huc_unregister_gsc_notifier(&gsc_to_gt(gsc)->uc.huc,
+ aux_dev->dev.bus);
+ intf->adev = NULL;
+
/* adev will be freed with the put_device() and .release sequence */
auxiliary_device_uninit(aux_dev);
goto fail;
}
- intf->adev = adev;
return;
fail:
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
index f0188931d8e4..5f2144c78f8a 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
@@ -10,6 +10,9 @@
#include "intel_huc.h"
#include "i915_drv.h"
+#include <linux/device/bus.h>
+#include <linux/mei_aux.h>
+
/**
* DOC: HuC
*
@@ -42,6 +45,164 @@
* HuC-specific commands.
*/
+/*
+ * MEI-GSC load is an async process. The probing of the exposed aux device
+ * (see intel_gsc.c) usually happens a few seconds after i915 probe, depending
+ * on when the kernel schedules it. Unless something goes terribly wrong, we're
+ * guaranteed for this to happen during boot, so the big timeout is a safety net
+ * that we never expect to need.
+ * MEI-PXP + HuC load usually takes ~300ms, but if the GSC needs to be resumed
+ * and/or reset, this can take longer.
+ */
+#define GSC_INIT_TIMEOUT_MS 10000
+#define PXP_INIT_TIMEOUT_MS 2000
+
+static int sw_fence_dummy_notify(struct i915_sw_fence *sf,
+ enum i915_sw_fence_notify state)
+{
+ return NOTIFY_DONE;
+}
+
+static void __delayed_huc_load_complete(struct intel_huc *huc)
+{
+ if (!i915_sw_fence_done(&huc->delayed_load.fence))
+ i915_sw_fence_complete(&huc->delayed_load.fence);
+}
+
+static void delayed_huc_load_complete(struct intel_huc *huc)
+{
+ hrtimer_cancel(&huc->delayed_load.timer);
+ __delayed_huc_load_complete(huc);
+}
+
+static void __gsc_init_error(struct intel_huc *huc)
+{
+ huc->delayed_load.status = INTEL_HUC_DELAYED_LOAD_ERROR;
+ __delayed_huc_load_complete(huc);
+}
+
+static void gsc_init_error(struct intel_huc *huc)
+{
+ hrtimer_cancel(&huc->delayed_load.timer);
+ __gsc_init_error(huc);
+}
+
+static void gsc_init_done(struct intel_huc *huc)
+{
+ hrtimer_cancel(&huc->delayed_load.timer);
+
+ /* MEI-GSC init is done, now we wait for MEI-PXP to bind */
+ huc->delayed_load.status = INTEL_HUC_WAITING_ON_PXP;
+ if (!i915_sw_fence_done(&huc->delayed_load.fence))
+ hrtimer_start(&huc->delayed_load.timer,
+ ms_to_ktime(PXP_INIT_TIMEOUT_MS),
+ HRTIMER_MODE_REL);
+}
+
+static enum hrtimer_restart huc_delayed_load_timer_callback(struct hrtimer *hrtimer)
+{
+ struct intel_huc *huc = container_of(hrtimer, struct intel_huc, delayed_load.timer);
+
+ if (!intel_huc_is_authenticated(huc)) {
+ drm_err(&huc_to_gt(huc)->i915->drm,
+ "timed out waiting for GSC init to load HuC\n");
+
+ __gsc_init_error(huc);
+ }
+
+ return HRTIMER_NORESTART;
+}
+
+static void huc_delayed_load_start(struct intel_huc *huc)
+{
+ ktime_t delay;
+
+ GEM_BUG_ON(intel_huc_is_authenticated(huc));
+
+ /*
+ * On resume we don't have to wait for MEI-GSC to be re-probed, but we
+ * do need to wait for MEI-PXP to reset & re-bind
+ */
+ switch (huc->delayed_load.status) {
+ case INTEL_HUC_WAITING_ON_GSC:
+ delay = ms_to_ktime(GSC_INIT_TIMEOUT_MS);
+ break;
+ case INTEL_HUC_WAITING_ON_PXP:
+ delay = ms_to_ktime(PXP_INIT_TIMEOUT_MS);
+ break;
+ default:
+ gsc_init_error(huc);
+ return;
+ }
+
+ /*
+ * This fence is always complete unless we're waiting for the
+ * GSC device to come up to load the HuC. We arm the fence here
+ * and complete it when we confirm that the HuC is loaded from
+ * the PXP bind callback.
+ */
+ GEM_BUG_ON(!i915_sw_fence_done(&huc->delayed_load.fence));
+ i915_sw_fence_fini(&huc->delayed_load.fence);
+ i915_sw_fence_reinit(&huc->delayed_load.fence);
+ i915_sw_fence_await(&huc->delayed_load.fence);
+ i915_sw_fence_commit(&huc->delayed_load.fence);
+
+ hrtimer_start(&huc->delayed_load.timer, delay, HRTIMER_MODE_REL);
+}
+
+static int gsc_notifier(struct notifier_block *nb, unsigned long action, void *data)
+{
+ struct device *dev = data;
+ struct intel_huc *huc = container_of(nb, struct intel_huc, delayed_load.nb);
+ struct intel_gsc_intf *intf = &huc_to_gt(huc)->gsc.intf[0];
+
+ if (!intf->adev || &intf->adev->aux_dev.dev != dev)
+ return 0;
+
+ switch (action) {
+ case BUS_NOTIFY_BOUND_DRIVER: /* mei driver bound to aux device */
+ gsc_init_done(huc);
+ break;
+
+ case BUS_NOTIFY_DRIVER_NOT_BOUND: /* mei driver fails to be bound */
+ case BUS_NOTIFY_UNBIND_DRIVER: /* mei driver about to be unbound */
+ drm_info(&huc_to_gt(huc)->i915->drm,
+ "mei driver not bound, disabling HuC load\n");
+ gsc_init_error(huc);
+ break;
+ }
+
+ return 0;
+}
+
+void intel_huc_register_gsc_notifier(struct intel_huc *huc, struct bus_type *bus)
+{
+ int ret;
+
+ if (!intel_huc_is_loaded_by_gsc(huc))
+ return;
+
+ huc->delayed_load.nb.notifier_call = gsc_notifier;
+ ret = bus_register_notifier(bus, &huc->delayed_load.nb);
+ if (ret) {
+ drm_err(&huc_to_gt(huc)->i915->drm,
+ "failed to register GSC notifier\n");
+ huc->delayed_load.nb.notifier_call = NULL;
+ gsc_init_error(huc);
+ }
+}
+
+void intel_huc_unregister_gsc_notifier(struct intel_huc *huc, struct bus_type *bus)
+{
+ if (!huc->delayed_load.nb.notifier_call)
+ return;
+
+ delayed_huc_load_complete(huc);
+
+ bus_unregister_notifier(bus, &huc->delayed_load.nb);
+ huc->delayed_load.nb.notifier_call = NULL;
+}
+
void intel_huc_init_early(struct intel_huc *huc)
{
struct drm_i915_private *i915 = huc_to_gt(huc)->i915;
@@ -57,6 +218,17 @@ void intel_huc_init_early(struct intel_huc *huc)
huc->status.mask = HUC_FW_VERIFIED;
huc->status.value = HUC_FW_VERIFIED;
}
+
+ /*
+ * Initialize fence to be complete as this is expected to be complete
+ * unless there is a delayed HuC reload in progress.
+ */
+ i915_sw_fence_init(&huc->delayed_load.fence,
+ sw_fence_dummy_notify);
+ i915_sw_fence_commit(&huc->delayed_load.fence);
+
+ hrtimer_init(&huc->delayed_load.timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+ huc->delayed_load.timer.function = huc_delayed_load_timer_callback;
}
#define HUC_LOAD_MODE_STRING(x) (x ? "GSC" : "legacy")
@@ -122,9 +294,25 @@ void intel_huc_fini(struct intel_huc *huc)
if (!intel_uc_fw_is_loadable(&huc->fw))
return;
+ delayed_huc_load_complete(huc);
+
+ i915_sw_fence_fini(&huc->delayed_load.fence);
intel_uc_fw_fini(&huc->fw);
}
+void intel_huc_suspend(struct intel_huc *huc)
+{
+ if (!intel_uc_fw_is_loadable(&huc->fw))
+ return;
+
+ /*
+ * in the unlikely case that we're suspending before the GSC has
+ * completed its loading sequence, just stop waiting. We'll restart
+ * on resume.
+ */
+ delayed_huc_load_complete(huc);
+}
+
int intel_huc_wait_for_auth_complete(struct intel_huc *huc)
{
struct intel_gt *gt = huc_to_gt(huc);
@@ -136,6 +324,9 @@ int intel_huc_wait_for_auth_complete(struct intel_huc *huc)
huc->status.value,
2, 50, NULL);
+ /* mark the load process as complete even if the wait failed */
+ delayed_huc_load_complete(huc);
+
if (ret) {
drm_err(>->i915->drm, "HuC: Firmware not verified %d\n", ret);
intel_uc_fw_change_status(&huc->fw, INTEL_UC_FIRMWARE_LOAD_FAIL);
@@ -239,6 +430,12 @@ int intel_huc_check_status(struct intel_huc *huc)
return intel_huc_is_authenticated(huc);
}
+static bool huc_has_delayed_load(struct intel_huc *huc)
+{
+ return intel_huc_is_loaded_by_gsc(huc) &&
+ (huc->delayed_load.status != INTEL_HUC_DELAYED_LOAD_ERROR);
+}
+
void intel_huc_update_auth_status(struct intel_huc *huc)
{
if (!intel_uc_fw_is_loadable(&huc->fw))
@@ -247,6 +444,8 @@ void intel_huc_update_auth_status(struct intel_huc *huc)
if (intel_huc_is_authenticated(huc))
intel_uc_fw_change_status(&huc->fw,
INTEL_UC_FIRMWARE_RUNNING);
+ else if (huc_has_delayed_load(huc))
+ huc_delayed_load_start(huc);
}
/**
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.h b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
index 51f9d96a3ca3..915d281c1c72 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
@@ -7,9 +7,21 @@
#define _INTEL_HUC_H_
#include "i915_reg_defs.h"
+#include "i915_sw_fence.h"
#include "intel_uc_fw.h"
#include "intel_huc_fw.h"
+#include <linux/notifier.h>
+#include <linux/hrtimer.h>
+
+struct bus_type;
+
+enum intel_huc_delayed_load_status {
+ INTEL_HUC_WAITING_ON_GSC = 0,
+ INTEL_HUC_WAITING_ON_PXP,
+ INTEL_HUC_DELAYED_LOAD_ERROR,
+};
+
struct intel_huc {
/* Generic uC firmware management */
struct intel_uc_fw fw;
@@ -20,17 +32,28 @@ struct intel_huc {
u32 mask;
u32 value;
} status;
+
+ struct {
+ struct i915_sw_fence fence;
+ struct hrtimer timer;
+ struct notifier_block nb;
+ enum intel_huc_delayed_load_status status;
+ } delayed_load;
};
void intel_huc_init_early(struct intel_huc *huc);
int intel_huc_init(struct intel_huc *huc);
void intel_huc_fini(struct intel_huc *huc);
+void intel_huc_suspend(struct intel_huc *huc);
int intel_huc_auth(struct intel_huc *huc);
int intel_huc_wait_for_auth_complete(struct intel_huc *huc);
int intel_huc_check_status(struct intel_huc *huc);
void intel_huc_update_auth_status(struct intel_huc *huc);
bool intel_huc_is_authenticated(struct intel_huc *huc);
+void intel_huc_register_gsc_notifier(struct intel_huc *huc, struct bus_type *bus);
+void intel_huc_unregister_gsc_notifier(struct intel_huc *huc, struct bus_type *bus);
+
static inline int intel_huc_sanitize(struct intel_huc *huc)
{
intel_uc_fw_sanitize(&huc->fw);
--
2.37.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v5 12/15] drm/i915/huc: stall media submission until HuC is loaded
2022-09-13 0:57 [PATCH v5 00/15] drm/i915: HuC loading for DG2 Daniele Ceraolo Spurio
` (10 preceding siblings ...)
2022-09-13 0:57 ` [PATCH v5 11/15] drm/i915/huc: track delayed HuC load with a fence Daniele Ceraolo Spurio
@ 2022-09-13 0:57 ` Daniele Ceraolo Spurio
2022-09-13 23:22 ` [PATCH] " Daniele Ceraolo Spurio
2022-09-13 0:57 ` [PATCH v5 13/15] drm/i915/huc: better define HuC status getparam possible return values Daniele Ceraolo Spurio
` (3 subsequent siblings)
15 siblings, 1 reply; 27+ messages in thread
From: Daniele Ceraolo Spurio @ 2022-09-13 0:57 UTC (permalink / raw)
To: intel-gfx; +Cc: Tony Ye, Daniele Ceraolo Spurio, Alan Previn, dri-devel
Wait on the fence to be signalled to avoid the submissions finding HuC
not yet loaded.
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Tony Ye <tony.ye@intel.com>
Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>
Acked-by: Tony Ye <tony.ye@intel.com>
---
drivers/gpu/drm/i915/gt/uc/intel_huc.h | 6 ++++++
drivers/gpu/drm/i915/i915_request.c | 24 ++++++++++++++++++++++++
2 files changed, 30 insertions(+)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.h b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
index 915d281c1c72..52db03620c60 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
@@ -81,6 +81,12 @@ static inline bool intel_huc_is_loaded_by_gsc(const struct intel_huc *huc)
return huc->fw.loaded_via_gsc;
}
+static inline bool intel_huc_wait_required(struct intel_huc *huc)
+{
+ return intel_huc_is_used(huc) && intel_huc_is_loaded_by_gsc(huc) &&
+ !intel_huc_is_authenticated(huc);
+}
+
void intel_huc_load_status(struct intel_huc *huc, struct drm_printer *p);
#endif
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index 62fad16a55e8..77f45a3cb01f 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -1621,6 +1621,20 @@ i915_request_await_object(struct i915_request *to,
return ret;
}
+static void i915_request_await_huc(struct i915_request *rq)
+{
+ struct intel_huc *huc = &rq->context->engine->gt->uc.huc;
+
+ /* don't stall kernel submissions! */
+ if (!rcu_access_pointer(rq->context->gem_context))
+ return;
+
+ if (intel_huc_wait_required(huc))
+ i915_sw_fence_await_sw_fence(&rq->submit,
+ &huc->delayed_load.fence,
+ &rq->submitq);
+}
+
static struct i915_request *
__i915_request_ensure_parallel_ordering(struct i915_request *rq,
struct intel_timeline *timeline)
@@ -1702,6 +1716,16 @@ __i915_request_add_to_timeline(struct i915_request *rq)
struct intel_timeline *timeline = i915_request_timeline(rq);
struct i915_request *prev;
+ /*
+ * Media workloads may require HuC, so stall them until HuC loading is
+ * complete. Note that HuC not being loaded when a user submission
+ * arrives can only happen when HuC is loaded via GSC and in that case
+ * we still expect the window between us starting to accept submissions
+ * and HuC loading completion to be small (a few hundred ms).
+ */
+ if (rq->engine->class == VIDEO_DECODE_CLASS)
+ i915_request_await_huc(rq);
+
/*
* Dependency tracking and request ordering along the timeline
* is special cased so that we can eliminate redundant ordering
--
2.37.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH] drm/i915/huc: stall media submission until HuC is loaded
2022-09-13 0:57 ` [PATCH v5 12/15] drm/i915/huc: stall media submission until HuC is loaded Daniele Ceraolo Spurio
@ 2022-09-13 23:22 ` Daniele Ceraolo Spurio
2022-09-27 18:54 ` Teres Alexis, Alan Previn
0 siblings, 1 reply; 27+ messages in thread
From: Daniele Ceraolo Spurio @ 2022-09-13 23:22 UTC (permalink / raw)
To: intel-gfx; +Cc: Tony Ye, Daniele Ceraolo Spurio, Alan Previn, dri-devel
Wait on the fence to be signalled to avoid the submissions finding HuC
not yet loaded.
v2: use dedicaded wait_queue_entry for waiting in HuC load, as submitq
can't be re-used for it.
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Tony Ye <tony.ye@intel.com>
Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com> #v1
Acked-by: Tony Ye <tony.ye@intel.com>
---
drivers/gpu/drm/i915/gt/uc/intel_huc.h | 6 ++++++
drivers/gpu/drm/i915/i915_request.c | 24 ++++++++++++++++++++++++
drivers/gpu/drm/i915/i915_request.h | 5 +++++
3 files changed, 35 insertions(+)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.h b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
index 915d281c1c72..52db03620c60 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
@@ -81,6 +81,12 @@ static inline bool intel_huc_is_loaded_by_gsc(const struct intel_huc *huc)
return huc->fw.loaded_via_gsc;
}
+static inline bool intel_huc_wait_required(struct intel_huc *huc)
+{
+ return intel_huc_is_used(huc) && intel_huc_is_loaded_by_gsc(huc) &&
+ !intel_huc_is_authenticated(huc);
+}
+
void intel_huc_load_status(struct intel_huc *huc, struct drm_printer *p);
#endif
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index 62fad16a55e8..f949a9495758 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -1621,6 +1621,20 @@ i915_request_await_object(struct i915_request *to,
return ret;
}
+static void i915_request_await_huc(struct i915_request *rq)
+{
+ struct intel_huc *huc = &rq->context->engine->gt->uc.huc;
+
+ /* don't stall kernel submissions! */
+ if (!rcu_access_pointer(rq->context->gem_context))
+ return;
+
+ if (intel_huc_wait_required(huc))
+ i915_sw_fence_await_sw_fence(&rq->submit,
+ &huc->delayed_load.fence,
+ &rq->hucq);
+}
+
static struct i915_request *
__i915_request_ensure_parallel_ordering(struct i915_request *rq,
struct intel_timeline *timeline)
@@ -1702,6 +1716,16 @@ __i915_request_add_to_timeline(struct i915_request *rq)
struct intel_timeline *timeline = i915_request_timeline(rq);
struct i915_request *prev;
+ /*
+ * Media workloads may require HuC, so stall them until HuC loading is
+ * complete. Note that HuC not being loaded when a user submission
+ * arrives can only happen when HuC is loaded via GSC and in that case
+ * we still expect the window between us starting to accept submissions
+ * and HuC loading completion to be small (a few hundred ms).
+ */
+ if (rq->engine->class == VIDEO_DECODE_CLASS)
+ i915_request_await_huc(rq);
+
/*
* Dependency tracking and request ordering along the timeline
* is special cased so that we can eliminate redundant ordering
diff --git a/drivers/gpu/drm/i915/i915_request.h b/drivers/gpu/drm/i915/i915_request.h
index 47041ec68df8..f5e1bb5e857a 100644
--- a/drivers/gpu/drm/i915/i915_request.h
+++ b/drivers/gpu/drm/i915/i915_request.h
@@ -348,6 +348,11 @@ struct i915_request {
#define GUC_PRIO_FINI 0xfe
u8 guc_prio;
+ /**
+ * @hucq: wait queue entry used to wait on the HuC load to complete
+ */
+ wait_queue_entry_t hucq;
+
I915_SELFTEST_DECLARE(struct {
struct list_head link;
unsigned long delay;
--
2.37.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH] drm/i915/huc: stall media submission until HuC is loaded
2022-09-13 23:22 ` [PATCH] " Daniele Ceraolo Spurio
@ 2022-09-27 18:54 ` Teres Alexis, Alan Previn
0 siblings, 0 replies; 27+ messages in thread
From: Teres Alexis, Alan Previn @ 2022-09-27 18:54 UTC (permalink / raw)
To: Ceraolo Spurio, Daniele, intel-gfx; +Cc: Ye, Tony, dri-devel
On Tue, 2022-09-13 at 16:22 -0700, Ceraolo Spurio, Daniele wrote:
> Wait on the fence to be signalled to avoid the submissions finding HuC
> not yet loaded.
>
> v2: use dedicaded wait_queue_entry for waiting in HuC load, as submitq
> can't be re-used for it.
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Tony Ye <tony.ye@intel.com>
> Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com> #v1
> Acked-by: Tony Ye <tony.ye@intel.com>
> ---
> drivers/gpu/drm/i915/gt/uc/intel_huc.h | 6 ++++++
> drivers/gpu/drm/i915/i915_request.c | 24 ++++++++++++++++++++++++
> drivers/gpu/drm/i915/i915_request.h | 5 +++++
> 3 files changed, 35 insertions(+)
>
[snip]
> diff --git a/drivers/gpu/drm/i915/i915_request.h b/drivers/gpu/drm/i915/i915_request.h
> index 47041ec68df8..f5e1bb5e857a 100644
> --- a/drivers/gpu/drm/i915/i915_request.h
> +++ b/drivers/gpu/drm/i915/i915_request.h
> @@ -348,6 +348,11 @@ struct i915_request {
> #define GUC_PRIO_FINI 0xfe
> u8 guc_prio;
>
> + /**
> + * @hucq: wait queue entry used to wait on the HuC load to complete
> + */
> + wait_queue_entry_t hucq;
> +
>
>
I believe that in future if we have multiple engines that
requires a similiar stalled initialization wait, we should
have an array of ptrs here and a not-huc-specific-helper that
can sort out adding fence-signalled-waiters. But for now this is
a very rare race condition that only happens with HuC so
this hucq specific wait-entry will do fine. Thus:
Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v5 13/15] drm/i915/huc: better define HuC status getparam possible return values.
2022-09-13 0:57 [PATCH v5 00/15] drm/i915: HuC loading for DG2 Daniele Ceraolo Spurio
` (11 preceding siblings ...)
2022-09-13 0:57 ` [PATCH v5 12/15] drm/i915/huc: stall media submission until HuC is loaded Daniele Ceraolo Spurio
@ 2022-09-13 0:57 ` Daniele Ceraolo Spurio
2022-09-13 0:57 ` [PATCH v5 14/15] drm/i915/huc: define gsc-compatible HuC fw for DG2 Daniele Ceraolo Spurio
` (2 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Daniele Ceraolo Spurio @ 2022-09-13 0:57 UTC (permalink / raw)
To: intel-gfx
Cc: Tony Ye, Tvrtko Ursulin, Alan Previn, Tvrtko Ursulin, dri-devel,
Daniele Ceraolo Spurio
The current HuC status getparam return values are a bit confusing in
regards to what happens in some scenarios. In particular, most of the
error cases cause the ioctl to return an error, but a couple of them,
INIT_FAIL and LOAD_FAIL, are not explicitly handled and neither is
their expected return value documented; these 2 error cases therefore
end up into the catch-all umbrella of the "HuC not loaded" case, with
this case therefore including both some error scenarios and the load
in progress one.
The updates included in this patch change the handling so that all
error cases behave the same way, i.e. return an errno code, and so
that the HuC load in progress case is unambiguous.
The patch also includes a small change to the FW init path to make sure
we always transition to an error state if something goes wrong.
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Tony Ye <tony.ye@intel.com>
Acked-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Acked-by: Tony Ye <tony.ye@intel.com>
Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>
---
drivers/gpu/drm/i915/gt/uc/intel_guc.c | 1 +
drivers/gpu/drm/i915/gt/uc/intel_huc.c | 14 +++++++-------
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 1 -
include/uapi/drm/i915_drm.h | 16 ++++++++++++++++
4 files changed, 24 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
index bac06e3d6f2c..27b09ba1d295 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
@@ -441,6 +441,7 @@ int intel_guc_init(struct intel_guc *guc)
err_fw:
intel_uc_fw_fini(&guc->fw);
out:
+ intel_uc_fw_change_status(&guc->fw, INTEL_UC_FIRMWARE_INIT_FAIL);
i915_probe_error(gt->i915, "failed with %d\n", ret);
return ret;
}
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
index 5f2144c78f8a..4d1cc383b681 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
@@ -285,6 +285,7 @@ int intel_huc_init(struct intel_huc *huc)
return 0;
out:
+ intel_uc_fw_change_status(&huc->fw, INTEL_UC_FIRMWARE_INIT_FAIL);
drm_info(&i915->drm, "HuC init failed with %d\n", err);
return err;
}
@@ -404,13 +405,8 @@ bool intel_huc_is_authenticated(struct intel_huc *huc)
* This function reads status register to verify if HuC
* firmware was successfully loaded.
*
- * Returns:
- * * -ENODEV if HuC is not present on this platform,
- * * -EOPNOTSUPP if HuC firmware is disabled,
- * * -ENOPKG if HuC firmware was not installed,
- * * -ENOEXEC if HuC firmware is invalid or mismatched,
- * * 0 if HuC firmware is not running,
- * * 1 if HuC firmware is authenticated and running.
+ * The return values match what is expected for the I915_PARAM_HUC_STATUS
+ * getparam.
*/
int intel_huc_check_status(struct intel_huc *huc)
{
@@ -423,6 +419,10 @@ int intel_huc_check_status(struct intel_huc *huc)
return -ENOPKG;
case INTEL_UC_FIRMWARE_ERROR:
return -ENOEXEC;
+ case INTEL_UC_FIRMWARE_INIT_FAIL:
+ return -ENOMEM;
+ case INTEL_UC_FIRMWARE_LOAD_FAIL:
+ return -EIO;
default:
break;
}
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
index af425916cdf6..4792960d9c04 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -890,7 +890,6 @@ int intel_uc_fw_init(struct intel_uc_fw *uc_fw)
out_unpin:
i915_gem_object_unpin_pages(uc_fw->obj);
out:
- intel_uc_fw_change_status(uc_fw, INTEL_UC_FIRMWARE_INIT_FAIL);
return err;
}
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 520ad2691a99..629198f1d8d8 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -645,6 +645,22 @@ typedef struct drm_i915_irq_wait {
*/
#define I915_SCHEDULER_CAP_STATIC_PRIORITY_MAP (1ul << 5)
+/*
+ * Query the status of HuC load.
+ *
+ * The query can fail in the following scenarios with the listed error codes:
+ * -ENODEV if HuC is not present on this platform,
+ * -EOPNOTSUPP if HuC firmware usage is disabled,
+ * -ENOPKG if HuC firmware fetch failed,
+ * -ENOEXEC if HuC firmware is invalid or mismatched,
+ * -ENOMEM if i915 failed to prepare the FW objects for transfer to the uC,
+ * -EIO if the FW transfer or the FW authentication failed.
+ *
+ * If the IOCTL is successful, the returned parameter will be set to one of the
+ * following values:
+ * * 0 if HuC firmware load is not complete,
+ * * 1 if HuC firmware is authenticated and running.
+ */
#define I915_PARAM_HUC_STATUS 42
/* Query whether DRM_I915_GEM_EXECBUFFER2 supports the ability to opt-out of
--
2.37.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v5 14/15] drm/i915/huc: define gsc-compatible HuC fw for DG2
2022-09-13 0:57 [PATCH v5 00/15] drm/i915: HuC loading for DG2 Daniele Ceraolo Spurio
` (12 preceding siblings ...)
2022-09-13 0:57 ` [PATCH v5 13/15] drm/i915/huc: better define HuC status getparam possible return values Daniele Ceraolo Spurio
@ 2022-09-13 0:57 ` Daniele Ceraolo Spurio
2022-09-13 0:57 ` [PATCH v5 15/15] HAX: drm/i915: force INTEL_MEI_GSC and INTEL_MEI_PXP on for CI Daniele Ceraolo Spurio
2022-09-14 16:51 ` [PATCH v5 00/15] drm/i915: HuC loading for DG2 Winkler, Tomas
15 siblings, 0 replies; 27+ messages in thread
From: Daniele Ceraolo Spurio @ 2022-09-13 0:57 UTC (permalink / raw)
To: intel-gfx; +Cc: Tony Ye, Daniele Ceraolo Spurio, Alan Previn, dri-devel
The fw name is different and we need to record the fact that the blob is
gsc-loaded, so add a new macro to help.
Note: A-step DG2 G10 does not support HuC loading via GSC and would
require a separate firmware to be loaded the legacy way, but that's
not a production stepping so we're not going to bother.
v2: rebase on new fw fetch logic
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Tony Ye <tony.ye@intel.com>
Acked-by: Tony Ye <tony.ye@intel.com>
Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>
---
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 23 ++++++++++++++++-------
1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
index 4792960d9c04..09e06ac8bcf1 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -91,7 +91,8 @@ void intel_uc_fw_change_status(struct intel_uc_fw *uc_fw,
fw_def(BROXTON, 0, guc_mmp(bxt, 70, 1, 1)) \
fw_def(SKYLAKE, 0, guc_mmp(skl, 70, 1, 1))
-#define INTEL_HUC_FIRMWARE_DEFS(fw_def, huc_raw, huc_mmp) \
+#define INTEL_HUC_FIRMWARE_DEFS(fw_def, huc_raw, huc_mmp, huc_gsc) \
+ fw_def(DG2, 0, huc_gsc(dg2)) \
fw_def(ALDERLAKE_P, 0, huc_mmp(tgl, 7, 9, 3)) \
fw_def(ALDERLAKE_S, 0, huc_mmp(tgl, 7, 9, 3)) \
fw_def(DG1, 0, huc_mmp(dg1, 7, 9, 3)) \
@@ -137,6 +138,9 @@ void intel_uc_fw_change_status(struct intel_uc_fw *uc_fw,
#define MAKE_HUC_FW_PATH_BLANK(prefix_) \
__MAKE_UC_FW_PATH_BLANK(prefix_, "_huc")
+#define MAKE_HUC_FW_PATH_GSC(prefix_) \
+ __MAKE_UC_FW_PATH_BLANK(prefix_, "_huc_gsc")
+
#define MAKE_HUC_FW_PATH_MMP(prefix_, major_, minor_, patch_) \
__MAKE_UC_FW_PATH_MMP(prefix_, "_huc_", major_, minor_, patch_)
@@ -149,7 +153,7 @@ void intel_uc_fw_change_status(struct intel_uc_fw *uc_fw,
MODULE_FIRMWARE(uc_);
INTEL_GUC_FIRMWARE_DEFS(INTEL_UC_MODULE_FW, MAKE_GUC_FW_PATH_MAJOR, MAKE_GUC_FW_PATH_MMP)
-INTEL_HUC_FIRMWARE_DEFS(INTEL_UC_MODULE_FW, MAKE_HUC_FW_PATH_BLANK, MAKE_HUC_FW_PATH_MMP)
+INTEL_HUC_FIRMWARE_DEFS(INTEL_UC_MODULE_FW, MAKE_HUC_FW_PATH_BLANK, MAKE_HUC_FW_PATH_MMP, MAKE_HUC_FW_PATH_GSC)
/*
* The next expansion of the table macros (in __uc_fw_auto_select below) provides
@@ -164,6 +168,7 @@ struct __packed uc_fw_blob {
u8 major;
u8 minor;
u8 patch;
+ bool loaded_via_gsc;
};
#define UC_FW_BLOB_BASE(major_, minor_, patch_, path_) \
@@ -172,16 +177,16 @@ struct __packed uc_fw_blob {
.patch = patch_, \
.path = path_,
-#define UC_FW_BLOB_NEW(major_, minor_, patch_, path_) \
+#define UC_FW_BLOB_NEW(major_, minor_, patch_, gsc_, path_) \
{ UC_FW_BLOB_BASE(major_, minor_, patch_, path_) \
- .legacy = false }
+ .legacy = false, .loaded_via_gsc = gsc_ }
#define UC_FW_BLOB_OLD(major_, minor_, patch_, path_) \
{ UC_FW_BLOB_BASE(major_, minor_, patch_, path_) \
.legacy = true }
#define GUC_FW_BLOB(prefix_, major_, minor_) \
- UC_FW_BLOB_NEW(major_, minor_, 0, \
+ UC_FW_BLOB_NEW(major_, minor_, 0, false, \
MAKE_GUC_FW_PATH_MAJOR(prefix_, major_, minor_))
#define GUC_FW_BLOB_MMP(prefix_, major_, minor_, patch_) \
@@ -189,12 +194,15 @@ struct __packed uc_fw_blob {
MAKE_GUC_FW_PATH_MMP(prefix_, major_, minor_, patch_))
#define HUC_FW_BLOB(prefix_) \
- UC_FW_BLOB_NEW(0, 0, 0, MAKE_HUC_FW_PATH_BLANK(prefix_))
+ UC_FW_BLOB_NEW(0, 0, 0, false, MAKE_HUC_FW_PATH_BLANK(prefix_))
#define HUC_FW_BLOB_MMP(prefix_, major_, minor_, patch_) \
UC_FW_BLOB_OLD(major_, minor_, patch_, \
MAKE_HUC_FW_PATH_MMP(prefix_, major_, minor_, patch_))
+#define HUC_FW_BLOB_GSC(prefix_) \
+ UC_FW_BLOB_NEW(0, 0, 0, true, MAKE_HUC_FW_PATH_GSC(prefix_))
+
struct __packed uc_fw_platform_requirement {
enum intel_platform p;
u8 rev; /* first platform rev using this FW */
@@ -220,7 +228,7 @@ __uc_fw_auto_select(struct drm_i915_private *i915, struct intel_uc_fw *uc_fw)
INTEL_GUC_FIRMWARE_DEFS(MAKE_FW_LIST, GUC_FW_BLOB, GUC_FW_BLOB_MMP)
};
static const struct uc_fw_platform_requirement blobs_huc[] = {
- INTEL_HUC_FIRMWARE_DEFS(MAKE_FW_LIST, HUC_FW_BLOB, HUC_FW_BLOB_MMP)
+ INTEL_HUC_FIRMWARE_DEFS(MAKE_FW_LIST, HUC_FW_BLOB, HUC_FW_BLOB_MMP, HUC_FW_BLOB_GSC)
};
static const struct fw_blobs_by_type blobs_all[INTEL_UC_FW_NUM_TYPES] = {
[INTEL_UC_FW_TYPE_GUC] = { blobs_guc, ARRAY_SIZE(blobs_guc) },
@@ -266,6 +274,7 @@ __uc_fw_auto_select(struct drm_i915_private *i915, struct intel_uc_fw *uc_fw)
uc_fw->file_wanted.path = blob->path;
uc_fw->file_wanted.major_ver = blob->major;
uc_fw->file_wanted.minor_ver = blob->minor;
+ uc_fw->loaded_via_gsc = blob->loaded_via_gsc;
break;
}
--
2.37.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v5 15/15] HAX: drm/i915: force INTEL_MEI_GSC and INTEL_MEI_PXP on for CI
2022-09-13 0:57 [PATCH v5 00/15] drm/i915: HuC loading for DG2 Daniele Ceraolo Spurio
` (13 preceding siblings ...)
2022-09-13 0:57 ` [PATCH v5 14/15] drm/i915/huc: define gsc-compatible HuC fw for DG2 Daniele Ceraolo Spurio
@ 2022-09-13 0:57 ` Daniele Ceraolo Spurio
2022-09-14 16:51 ` [PATCH v5 00/15] drm/i915: HuC loading for DG2 Winkler, Tomas
15 siblings, 0 replies; 27+ messages in thread
From: Daniele Ceraolo Spurio @ 2022-09-13 0:57 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniele Ceraolo Spurio, dri-devel
Both are required for HuC loading.
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
drivers/gpu/drm/i915/Kconfig.debug | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/i915/Kconfig.debug b/drivers/gpu/drm/i915/Kconfig.debug
index e7fd3e76f8a2..a6576ffbc4dc 100644
--- a/drivers/gpu/drm/i915/Kconfig.debug
+++ b/drivers/gpu/drm/i915/Kconfig.debug
@@ -48,6 +48,8 @@ config DRM_I915_DEBUG
select DRM_I915_DEBUG_RUNTIME_PM
select DRM_I915_SW_FENCE_DEBUG_OBJECTS
select DRM_I915_SELFTEST
+ select INTEL_MEI_GSC
+ select INTEL_MEI_PXP
select BROKEN # for prototype uAPI
default n
help
--
2.37.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* RE: [PATCH v5 00/15] drm/i915: HuC loading for DG2
2022-09-13 0:57 [PATCH v5 00/15] drm/i915: HuC loading for DG2 Daniele Ceraolo Spurio
` (14 preceding siblings ...)
2022-09-13 0:57 ` [PATCH v5 15/15] HAX: drm/i915: force INTEL_MEI_GSC and INTEL_MEI_PXP on for CI Daniele Ceraolo Spurio
@ 2022-09-14 16:51 ` Winkler, Tomas
2022-09-14 18:11 ` Greg Kroah-Hartman
15 siblings, 1 reply; 27+ messages in thread
From: Winkler, Tomas @ 2022-09-14 16:51 UTC (permalink / raw)
To: Greg Kroah-Hartman, intel-gfx, Ceraolo Spurio, Daniele
Cc: Ye, Tony, Greg Kroah-Hartman, Usyskin, Alexander, Teres Alexis,
Alan Previn, dri-devel
>
> On DG2, HuC loading is performed by the GSC, via a PXP command. The load
> operation itself is relatively simple (just send a message to the GSC with the
> physical address of the HuC in LMEM), but there are timing changes that
> requires special attention. In particular, to send a PXP command we need to
> first export the GSC as an aux device and then wait for the mei-gsc and mei-
> pxp modules to start, which means that HuC load will complete after i915
> load is complete. This means that there is a small window of time after i915 is
> registered and before HuC is loaded during which userspace could submit
> and/or check the HuC load status, although this is quite unlikely to happen
> (HuC is usually loaded before kernel init/resume completes).
> We've consulted with the media team in regards to how to handle this and
> they've asked us to stall all userspace VCS submission until HuC is loaded.
> Stalls are expected to be very rare (if any), due to the fact that HuC is usually
> loaded before kernel init/resume is completed.
>
> Timeouts are in place to ensure all submissions are unlocked in case
> something goes wrong. Since we need to monitor the status of the mei
> driver to know what's happening and when to time out, a notifier has been
> added so we get a callback when the status of the mei driver changes.
>
> Note that this series includes several mei patches that add support for
> sending the HuC loading command via mei-gsc. We plan to merge those
> patches through the drm tree because i915 is the sole user.
>
> v2: address review comments, Reporting HuC loading still in progress while
> we wait for mei-gsc init to complete, rebase on latest mei-gsc series.
>
> v3: fix cc list in mei patches.
>
> v4: update mei patches, fix includes, rebase on new FW fetch logic and
> merged mei-gsc support.
>
> v5: update mei patches
Greg, I hope I've addressed most of your comments.
Can you please check if the mei patches are in acceptable state or anything else can be improved with this series. Appreciated.
Thanks
Tomas
>
> Cc: Alan Previn <alan.previn.teres.alexis@intel.com>
> Cc: Tony Ye <tony.ye@intel.com>
> Cc: Alexander Usyskin <alexander.usyskin@intel.com>
> Cc: Tomas Winkler <tomas.winkler@intel.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>
> Daniele Ceraolo Spurio (7):
> drm/i915/pxp: load the pxp module when we have a gsc-loaded huc
> drm/i915/dg2: setup HuC loading via GSC
> drm/i915/huc: track delayed HuC load with a fence
> drm/i915/huc: stall media submission until HuC is loaded
> drm/i915/huc: better define HuC status getparam possible return
> values.
> drm/i915/huc: define gsc-compatible HuC fw for DG2
> HAX: drm/i915: force INTEL_MEI_GSC and INTEL_MEI_PXP on for CI
>
> Tomas Winkler (5):
> mei: add support to GSC extended header
> mei: bus: enable sending gsc commands
> mei: adjust extended header kdocs
> mei: pxp: support matching with a gfx discrete card
> drm/i915/pxp: add huc authentication and loading command
>
> Vitaly Lubart (3):
> mei: bus: extend bus API to support command streamer API
> mei: pxp: add command streamer API to the PXP driver
> drm/i915/pxp: implement function for sending tee stream command
>
> drivers/gpu/drm/i915/Kconfig.debug | 2 +
> drivers/gpu/drm/i915/Makefile | 11 +-
> drivers/gpu/drm/i915/gt/intel_gsc.c | 22 +-
> drivers/gpu/drm/i915/gt/uc/intel_guc.c | 1 +
> drivers/gpu/drm/i915/gt/uc/intel_huc.c | 254 ++++++++++++++++--
> drivers/gpu/drm/i915/gt/uc/intel_huc.h | 31 +++
> drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c | 34 +++
> drivers/gpu/drm/i915/gt/uc/intel_huc_fw.h | 1 +
> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 24 +-
> drivers/gpu/drm/i915/i915_request.c | 24 ++
> drivers/gpu/drm/i915/pxp/intel_pxp.c | 32 ++-
> drivers/gpu/drm/i915/pxp/intel_pxp.h | 32 ---
> drivers/gpu/drm/i915/pxp/intel_pxp_huc.c | 69 +++++
> drivers/gpu/drm/i915/pxp/intel_pxp_huc.h | 13 +
> drivers/gpu/drm/i915/pxp/intel_pxp_irq.h | 8 +
> drivers/gpu/drm/i915/pxp/intel_pxp_session.c | 8 +-
> drivers/gpu/drm/i915/pxp/intel_pxp_session.h | 11 +-
> drivers/gpu/drm/i915/pxp/intel_pxp_tee.c | 138 +++++++++-
> drivers/gpu/drm/i915/pxp/intel_pxp_tee.h | 5 +
> .../drm/i915/pxp/intel_pxp_tee_interface.h | 23 +-
> drivers/gpu/drm/i915/pxp/intel_pxp_types.h | 6 +
> drivers/misc/mei/bus.c | 146 +++++++++-
> drivers/misc/mei/client.c | 55 ++--
> drivers/misc/mei/hbm.c | 13 +
> drivers/misc/mei/hw-me.c | 7 +-
> drivers/misc/mei/hw.h | 89 +++++-
> drivers/misc/mei/interrupt.c | 47 +++-
> drivers/misc/mei/mei_dev.h | 8 +
> drivers/misc/mei/pxp/mei_pxp.c | 38 ++-
> include/drm/i915_pxp_tee_interface.h | 5 +
> include/linux/mei_cl_bus.h | 6 +
> include/uapi/drm/i915_drm.h | 16 ++
> 32 files changed, 1057 insertions(+), 122 deletions(-) create mode 100644
> drivers/gpu/drm/i915/pxp/intel_pxp_huc.c
> create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp_huc.h
>
> --
> 2.37.2
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v5 00/15] drm/i915: HuC loading for DG2
2022-09-14 16:51 ` [PATCH v5 00/15] drm/i915: HuC loading for DG2 Winkler, Tomas
@ 2022-09-14 18:11 ` Greg Kroah-Hartman
2022-09-15 20:54 ` Winkler, Tomas
0 siblings, 1 reply; 27+ messages in thread
From: Greg Kroah-Hartman @ 2022-09-14 18:11 UTC (permalink / raw)
To: Winkler, Tomas
Cc: Ye, Tony, Teres Alexis, Alan Previn, intel-gfx, Usyskin,
Alexander, dri-devel, Ceraolo Spurio, Daniele
On Wed, Sep 14, 2022 at 04:51:03PM +0000, Winkler, Tomas wrote:
> >
> > On DG2, HuC loading is performed by the GSC, via a PXP command. The load
> > operation itself is relatively simple (just send a message to the GSC with the
> > physical address of the HuC in LMEM), but there are timing changes that
> > requires special attention. In particular, to send a PXP command we need to
> > first export the GSC as an aux device and then wait for the mei-gsc and mei-
> > pxp modules to start, which means that HuC load will complete after i915
> > load is complete. This means that there is a small window of time after i915 is
> > registered and before HuC is loaded during which userspace could submit
> > and/or check the HuC load status, although this is quite unlikely to happen
> > (HuC is usually loaded before kernel init/resume completes).
> > We've consulted with the media team in regards to how to handle this and
> > they've asked us to stall all userspace VCS submission until HuC is loaded.
> > Stalls are expected to be very rare (if any), due to the fact that HuC is usually
> > loaded before kernel init/resume is completed.
> >
> > Timeouts are in place to ensure all submissions are unlocked in case
> > something goes wrong. Since we need to monitor the status of the mei
> > driver to know what's happening and when to time out, a notifier has been
> > added so we get a callback when the status of the mei driver changes.
> >
> > Note that this series includes several mei patches that add support for
> > sending the HuC loading command via mei-gsc. We plan to merge those
> > patches through the drm tree because i915 is the sole user.
> >
> > v2: address review comments, Reporting HuC loading still in progress while
> > we wait for mei-gsc init to complete, rebase on latest mei-gsc series.
> >
> > v3: fix cc list in mei patches.
> >
> > v4: update mei patches, fix includes, rebase on new FW fetch logic and
> > merged mei-gsc support.
> >
> > v5: update mei patches
>
> Greg, I hope I've addressed most of your comments.
> Can you please check if the mei patches are in acceptable state or anything else can be improved with this series. Appreciated.
These were sent 2 days ago, in the middle of a conference travel.
Please relax, there's no special rush needed here, you know better.
In the mean time, if you are just waiting for my review, please take the
time to review other pending patches from other developers to help
lighten the load on me, and other maintainers.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 27+ messages in thread
* RE: [PATCH v5 00/15] drm/i915: HuC loading for DG2
2022-09-14 18:11 ` Greg Kroah-Hartman
@ 2022-09-15 20:54 ` Winkler, Tomas
0 siblings, 0 replies; 27+ messages in thread
From: Winkler, Tomas @ 2022-09-15 20:54 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Ye, Tony, Teres Alexis, Alan Previn, intel-gfx, Usyskin,
Alexander, dri-devel, Ceraolo Spurio, Daniele
> On Wed, Sep 14, 2022 at 04:51:03PM +0000, Winkler, Tomas wrote:
> > >
> > > On DG2, HuC loading is performed by the GSC, via a PXP command. The
> > > load operation itself is relatively simple (just send a message to
> > > the GSC with the physical address of the HuC in LMEM), but there are
> > > timing changes that requires special attention. In particular, to
> > > send a PXP command we need to first export the GSC as an aux device
> > > and then wait for the mei-gsc and mei- pxp modules to start, which
> > > means that HuC load will complete after i915 load is complete. This
> > > means that there is a small window of time after i915 is registered
> > > and before HuC is loaded during which userspace could submit and/or
> > > check the HuC load status, although this is quite unlikely to happen (HuC
> is usually loaded before kernel init/resume completes).
> > > We've consulted with the media team in regards to how to handle this
> > > and they've asked us to stall all userspace VCS submission until HuC is
> loaded.
> > > Stalls are expected to be very rare (if any), due to the fact that
> > > HuC is usually loaded before kernel init/resume is completed.
> > >
> > > Timeouts are in place to ensure all submissions are unlocked in case
> > > something goes wrong. Since we need to monitor the status of the mei
> > > driver to know what's happening and when to time out, a notifier has
> > > been added so we get a callback when the status of the mei driver
> changes.
> > >
> > > Note that this series includes several mei patches that add support
> > > for sending the HuC loading command via mei-gsc. We plan to merge
> > > those patches through the drm tree because i915 is the sole user.
> > >
> > > v2: address review comments, Reporting HuC loading still in progress
> > > while we wait for mei-gsc init to complete, rebase on latest mei-gsc
> series.
> > >
> > > v3: fix cc list in mei patches.
> > >
> > > v4: update mei patches, fix includes, rebase on new FW fetch logic
> > > and merged mei-gsc support.
> > >
> > > v5: update mei patches
> >
> > Greg, I hope I've addressed most of your comments.
> > Can you please check if the mei patches are in acceptable state or anything
> else can be improved with this series. Appreciated.
>
> These were sent 2 days ago, in the middle of a conference travel.
> Please relax, there's no special rush needed here, you know better.
Sure
> In the mean time, if you are just waiting for my review, please take the time
> to review other pending patches from other developers to help lighten the
> load on me, and other maintainers.
Fair enough, that's all I do every day anyway.
Thanks
Tomas
^ permalink raw reply [flat|nested] 27+ messages in thread