Re: [PATCH v6] drm/i915/guc: Add GuC css header parser

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Oct 21, 2015 at 12:13:13PM +0100, Dave Gordon wrote:
> On 20/10/15 00:10, yu.dai@xxxxxxxxx wrote:
> >From: Alex Dai <yu.dai@xxxxxxxxx>
> >
> >The size / offset information of all firmware ingredients are
> >now caculated from header. Driver will validate the header and
> >rsa key size. If any component is out of boundary, driver will
> >reject the loading too.
> >
> >v6: Clean up warnings from make docs
> >
> >v5: Tidy up GuC titles in kernel/Doc
> >
> >v4: Now using 'size_dw' for those defined in css_header
> >
> >v3: 1) Move DOC to intel_guc_fwif.h right before css_header
> >definition. Add more comments.
> >     2) Change 'size' to 'len' or 'length' to avoid confusion.
> >     3) Add UOS_RSA_SCRATCH_MAX_COUNT according to BSpec. And
> >driver validate size of RSA key now.
> >     4) Add fw component size/offset info to intel_guc_fw.
> >
> >v2: Add indent into DOC to make fixed-width format rather than
> >change the tmpl.
> >
> >v1: 1) guc_css_header is defined as __packed now
> >     2) Add and correct GuC related topics in kernel/Doc
> >
> >Signed-off-by: Alex Dai <yu.dai@xxxxxxxxx>
> >---
> 
> Docbook looks OK now, and the code is the same as last version I reviewed,
> so ...
> 
> Reviewed-by: Dave Gordon <david.s.gordon@xxxxxxxxx>

Queued for -next, thanks for the patch.
-Daniel

> 
> >  Documentation/DocBook/gpu.tmpl             | 12 ++--
> >  drivers/gpu/drm/i915/i915_debugfs.c        |  6 ++
> >  drivers/gpu/drm/i915/i915_guc_reg.h        |  1 +
> >  drivers/gpu/drm/i915/i915_guc_submission.c |  8 +--
> >  drivers/gpu/drm/i915/intel_guc.h           |  8 ++-
> >  drivers/gpu/drm/i915/intel_guc_fwif.h      | 72 ++++++++++++++++++++++
> >  drivers/gpu/drm/i915/intel_guc_loader.c    | 95 +++++++++++++++++++-----------
> >  7 files changed, 157 insertions(+), 45 deletions(-)
> >
> >diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
> >index c05d7df..3dce6f6 100644
> >--- a/Documentation/DocBook/gpu.tmpl
> >+++ b/Documentation/DocBook/gpu.tmpl
> >@@ -4115,17 +4115,21 @@ int num_ioctls;</synopsis>
> >        </sect2>
> >      </sect1>
> >      <sect1>
> >-      <title>GuC-based Command Submission</title>
> >+      <title>GuC</title>
> >        <sect2>
> >-        <title>GuC</title>
> >+        <title>GuC-specific firmware loader</title>
> >  !Pdrivers/gpu/drm/i915/intel_guc_loader.c GuC-specific firmware loader
> >  !Idrivers/gpu/drm/i915/intel_guc_loader.c
> >        </sect2>
> >        <sect2>
> >-        <title>GuC Client</title>
> >-!Pdrivers/gpu/drm/i915/i915_guc_submission.c GuC-based command submissison
> >+        <title>GuC-based command submission</title>
> >+!Pdrivers/gpu/drm/i915/i915_guc_submission.c GuC-based command submission
> >  !Idrivers/gpu/drm/i915/i915_guc_submission.c
> >        </sect2>
> >+      <sect2>
> >+        <title>GuC Firmware Layout</title>
> >+!Pdrivers/gpu/drm/i915/intel_guc_fwif.h GuC Firmware Layout
> >+      </sect2>
> >      </sect1>
> >
> >      <sect1>
> >diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> >index a3b22bd..eca94d0 100644
> >--- a/drivers/gpu/drm/i915/i915_debugfs.c
> >+++ b/drivers/gpu/drm/i915/i915_debugfs.c
> >@@ -2402,6 +2402,12 @@ static int i915_guc_load_status_info(struct seq_file *m, void *data)
> >  		guc_fw->guc_fw_major_wanted, guc_fw->guc_fw_minor_wanted);
> >  	seq_printf(m, "\tversion found: %d.%d\n",
> >  		guc_fw->guc_fw_major_found, guc_fw->guc_fw_minor_found);
> >+	seq_printf(m, "\theader: offset is %d; size = %d\n",
> >+		guc_fw->header_offset, guc_fw->header_size);
> >+	seq_printf(m, "\tuCode: offset is %d; size = %d\n",
> >+		guc_fw->ucode_offset, guc_fw->ucode_size);
> >+	seq_printf(m, "\tRSA: offset is %d; size = %d\n",
> >+		guc_fw->rsa_offset, guc_fw->rsa_size);
> >
> >  	tmp = I915_READ(GUC_STATUS);
> >
> >diff --git a/drivers/gpu/drm/i915/i915_guc_reg.h b/drivers/gpu/drm/i915/i915_guc_reg.h
> >index c4cb1c0..b51b828 100644
> >--- a/drivers/gpu/drm/i915/i915_guc_reg.h
> >+++ b/drivers/gpu/drm/i915/i915_guc_reg.h
> >@@ -42,6 +42,7 @@
> >  #define SOFT_SCRATCH(n)			(0xc180 + ((n) * 4))
> >
> >  #define UOS_RSA_SCRATCH(i)		(0xc200 + (i) * 4)
> >+#define   UOS_RSA_SCRATCH_MAX_COUNT	  64
> >  #define DMA_ADDR_0_LOW			0xc300
> >  #define DMA_ADDR_0_HIGH			0xc304
> >  #define DMA_ADDR_1_LOW			0xc308
> >diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> >index 036b42b..58a61c4 100644
> >--- a/drivers/gpu/drm/i915/i915_guc_submission.c
> >+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> >@@ -27,7 +27,7 @@
> >  #include "intel_guc.h"
> >
> >  /**
> >- * DOC: GuC Client
> >+ * DOC: GuC-based command submission
> >   *
> >   * i915_guc_client:
> >   * We use the term client to avoid confusion with contexts. A i915_guc_client is
> >@@ -588,8 +588,7 @@ static void lr_context_update(struct drm_i915_gem_request *rq)
> >  /**
> >   * i915_guc_submit() - Submit commands through GuC
> >   * @client:	the guc client where commands will go through
> >- * @ctx:	LRC where commands come from
> >- * @ring:	HW engine that will excute the commands
> >+ * @rq:		request associated with the commands
> >   *
> >   * Return:	0 if succeed
> >   */
> >@@ -731,7 +730,8 @@ static void guc_client_free(struct drm_device *dev,
> >   * 		The kernel client to replace ExecList submission is created with
> >   * 		NORMAL priority. Priority of a client for scheduler can be HIGH,
> >   * 		while a preemption context can use CRITICAL.
> >- * @ctx		the context to own the client (we use the default render context)
> >+ * @ctx:	the context that owns the client (we use the default render
> >+ * 		context)
> >   *
> >   * Return:	An i915_guc_client object if success.
> >   */
> >diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
> >index 081d5f6..5ba5866 100644
> >--- a/drivers/gpu/drm/i915/intel_guc.h
> >+++ b/drivers/gpu/drm/i915/intel_guc.h
> >@@ -76,11 +76,17 @@ struct intel_guc_fw {
> >  	uint16_t			guc_fw_minor_wanted;
> >  	uint16_t			guc_fw_major_found;
> >  	uint16_t			guc_fw_minor_found;
> >+
> >+	uint32_t header_size;
> >+	uint32_t header_offset;
> >+	uint32_t rsa_size;
> >+	uint32_t rsa_offset;
> >+	uint32_t ucode_size;
> >+	uint32_t ucode_offset;
> >  };
> >
> >  struct intel_guc {
> >  	struct intel_guc_fw guc_fw;
> >-
> >  	uint32_t log_flags;
> >  	struct drm_i915_gem_object *log_obj;
> >
> >diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h
> >index 593d2f5..40b2ea5 100644
> >--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
> >+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
> >@@ -122,6 +122,78 @@
> >
> >  #define GUC_CTL_MAX_DWORDS		(GUC_CTL_RSRVD + 1)
> >
> >+/**
> >+ * DOC: GuC Firmware Layout
> >+ *
> >+ * The GuC firmware layout looks like this:
> >+ *
> >+ *     +-------------------------------+
> >+ *     |        guc_css_header         |
> >+ *     | contains major/minor version  |
> >+ *     +-------------------------------+
> >+ *     |             uCode             |
> >+ *     +-------------------------------+
> >+ *     |         RSA signature         |
> >+ *     +-------------------------------+
> >+ *     |          modulus key          |
> >+ *     +-------------------------------+
> >+ *     |          exponent val         |
> >+ *     +-------------------------------+
> >+ *
> >+ * The firmware may or may not have modulus key and exponent data. The header,
> >+ * uCode and RSA signature are must-have components that will be used by driver.
> >+ * Length of each components, which is all in dwords, can be found in header.
> >+ * In the case that modulus and exponent are not present in fw, a.k.a truncated
> >+ * image, the length value still appears in header.
> >+ *
> >+ * Driver will do some basic fw size validation based on the following rules:
> >+ *
> >+ * 1. Header, uCode and RSA are must-have components.
> >+ * 2. All firmware components, if they present, are in the sequence illustrated
> >+ * in the layout table above.
> >+ * 3. Length info of each component can be found in header, in dwords.
> >+ * 4. Modulus and exponent key are not required by driver. They may not appear
> >+ * in fw. So driver will load a truncated firmware in this case.
> >+ */
> >+
> >+struct guc_css_header {
> >+	uint32_t module_type;
> >+	/* header_size includes all non-uCode bits, including css_header, rsa
> >+	 * key, modulus key and exponent data. */
> >+	uint32_t header_size_dw;
> >+	uint32_t header_version;
> >+	uint32_t module_id;
> >+	uint32_t module_vendor;
> >+	union {
> >+		struct {
> >+			uint8_t day;
> >+			uint8_t month;
> >+			uint16_t year;
> >+		};
> >+		uint32_t date;
> >+	};
> >+	uint32_t size_dw; /* uCode plus header_size_dw */
> >+	uint32_t key_size_dw;
> >+	uint32_t modulus_size_dw;
> >+	uint32_t exponent_size_dw;
> >+	union {
> >+		struct {
> >+			uint8_t hour;
> >+			uint8_t min;
> >+			uint16_t sec;
> >+		};
> >+		uint32_t time;
> >+	};
> >+
> >+	char username[8];
> >+	char buildnumber[12];
> >+	uint32_t device_id;
> >+	uint32_t guc_sw_version;
> >+	uint32_t prod_preprod_fw;
> >+	uint32_t reserved[12];
> >+	uint32_t header_info;
> >+} __packed;
> >+
> >  struct guc_doorbell_info {
> >  	u32 db_status;
> >  	u32 cookie;
> >diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
> >index a17b6a5..612ead7 100644
> >--- a/drivers/gpu/drm/i915/intel_guc_loader.c
> >+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> >@@ -31,7 +31,7 @@
> >  #include "intel_guc.h"
> >
> >  /**
> >- * DOC: GuC
> >+ * DOC: GuC-specific firmware loader
> >   *
> >   * intel_guc:
> >   * Top level structure of guc. It handles firmware loading and manages client
> >@@ -208,16 +208,6 @@ static inline bool guc_ucode_response(struct drm_i915_private *dev_priv,
> >  /*
> >   * Transfer the firmware image to RAM for execution by the microcontroller.
> >   *
> >- * GuC Firmware layout:
> >- * +-------------------------------+  ----
> >- * |          CSS header           |  128B
> >- * | contains major/minor version  |
> >- * +-------------------------------+  ----
> >- * |             uCode             |
> >- * +-------------------------------+  ----
> >- * |         RSA signature         |  256B
> >- * +-------------------------------+  ----
> >- *
> >   * Architecturally, the DMA engine is bidirectional, and can potentially even
> >   * transfer between GTT locations. This functionality is left out of the API
> >   * for now as there is no need for it.
> >@@ -225,33 +215,29 @@ static inline bool guc_ucode_response(struct drm_i915_private *dev_priv,
> >   * Note that GuC needs the CSS header plus uKernel code to be copied by the
> >   * DMA engine in one operation, whereas the RSA signature is loaded via MMIO.
> >   */
> >-
> >-#define UOS_CSS_HEADER_OFFSET		0
> >-#define UOS_VER_MINOR_OFFSET		0x44
> >-#define UOS_VER_MAJOR_OFFSET		0x46
> >-#define UOS_CSS_HEADER_SIZE		0x80
> >-#define UOS_RSA_SIG_SIZE		0x100
> >-
> >  static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv)
> >  {
> >  	struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw;
> >  	struct drm_i915_gem_object *fw_obj = guc_fw->guc_fw_obj;
> >  	unsigned long offset;
> >  	struct sg_table *sg = fw_obj->pages;
> >-	u32 status, ucode_size, rsa[UOS_RSA_SIG_SIZE / sizeof(u32)];
> >+	u32 status, rsa[UOS_RSA_SCRATCH_MAX_COUNT];
> >  	int i, ret = 0;
> >
> >-	/* uCode size, also is where RSA signature starts */
> >-	offset = ucode_size = guc_fw->guc_fw_size - UOS_RSA_SIG_SIZE;
> >-	I915_WRITE(DMA_COPY_SIZE, ucode_size);
> >+	/* where RSA signature starts */
> >+	offset = guc_fw->rsa_offset;
> >
> >  	/* Copy RSA signature from the fw image to HW for verification */
> >-	sg_pcopy_to_buffer(sg->sgl, sg->nents, rsa, UOS_RSA_SIG_SIZE, offset);
> >-	for (i = 0; i < UOS_RSA_SIG_SIZE / sizeof(u32); i++)
> >+	sg_pcopy_to_buffer(sg->sgl, sg->nents, rsa, sizeof(rsa), offset);
> >+	for (i = 0; i < UOS_RSA_SCRATCH_MAX_COUNT; i++)
> >  		I915_WRITE(UOS_RSA_SCRATCH(i), rsa[i]);
> >
> >+	/* The header plus uCode will be copied to WOPCM via DMA, excluding any
> >+	 * other components */
> >+	I915_WRITE(DMA_COPY_SIZE, guc_fw->header_size + guc_fw->ucode_size);
> >+
> >  	/* Set the source address for the new blob */
> >-	offset = i915_gem_obj_ggtt_offset(fw_obj);
> >+	offset = i915_gem_obj_ggtt_offset(fw_obj) + guc_fw->header_offset;
> >  	I915_WRITE(DMA_ADDR_0_LOW, lower_32_bits(offset));
> >  	I915_WRITE(DMA_ADDR_0_HIGH, upper_32_bits(offset) & 0xFFFF);
> >
> >@@ -457,10 +443,8 @@ static void guc_fw_fetch(struct drm_device *dev, struct intel_guc_fw *guc_fw)
> >  {
> >  	struct drm_i915_gem_object *obj;
> >  	const struct firmware *fw;
> >-	const u8 *css_header;
> >-	const size_t minsize = UOS_CSS_HEADER_SIZE + UOS_RSA_SIG_SIZE;
> >-	const size_t maxsize = GUC_WOPCM_SIZE_VALUE + UOS_RSA_SIG_SIZE
> >-			- 0x8000; /* 32k reserved (8K stack + 24k context) */
> >+	struct guc_css_header *css;
> >+	size_t size;
> >  	int err;
> >
> >  	DRM_DEBUG_DRIVER("before requesting firmware: GuC fw fetch status %s\n",
> >@@ -474,12 +458,52 @@ static void guc_fw_fetch(struct drm_device *dev, struct intel_guc_fw *guc_fw)
> >
> >  	DRM_DEBUG_DRIVER("fetch GuC fw from %s succeeded, fw %p\n",
> >  		guc_fw->guc_fw_path, fw);
> >-	DRM_DEBUG_DRIVER("firmware file size %zu (minimum %zu, maximum %zu)\n",
> >-		fw->size, minsize, maxsize);
> >
> >-	/* Check the size of the blob befoe examining buffer contents */
> >-	if (fw->size < minsize || fw->size > maxsize)
> >+	/* Check the size of the blob before examining buffer contents */
> >+	if (fw->size < sizeof(struct guc_css_header)) {
> >+		DRM_ERROR("Firmware header is missing\n");
> >  		goto fail;
> >+	}
> >+
> >+	css = (struct guc_css_header *)fw->data;
> >+
> >+	/* Firmware bits always start from header */
> >+	guc_fw->header_offset = 0;
> >+	guc_fw->header_size = (css->header_size_dw - css->modulus_size_dw -
> >+		css->key_size_dw - css->exponent_size_dw) * sizeof(u32);
> >+
> >+	if (guc_fw->header_size != sizeof(struct guc_css_header)) {
> >+		DRM_ERROR("CSS header definition mismatch\n");
> >+		goto fail;
> >+	}
> >+
> >+	/* then, uCode */
> >+	guc_fw->ucode_offset = guc_fw->header_offset + guc_fw->header_size;
> >+	guc_fw->ucode_size = (css->size_dw - css->header_size_dw) * sizeof(u32);
> >+
> >+	/* now RSA */
> >+	if (css->key_size_dw != UOS_RSA_SCRATCH_MAX_COUNT) {
> >+		DRM_ERROR("RSA key size is bad\n");
> >+		goto fail;
> >+	}
> >+	guc_fw->rsa_offset = guc_fw->ucode_offset + guc_fw->ucode_size;
> >+	guc_fw->rsa_size = css->key_size_dw * sizeof(u32);
> >+
> >+	/* At least, it should have header, uCode and RSA. Size of all three. */
> >+	size = guc_fw->header_size + guc_fw->ucode_size + guc_fw->rsa_size;
> >+	if (fw->size < size) {
> >+		DRM_ERROR("Missing firmware components\n");
> >+		goto fail;
> >+	}
> >+
> >+	/* Header and uCode will be loaded to WOPCM. Size of the two. */
> >+	size = guc_fw->header_size + guc_fw->ucode_size;
> >+
> >+	/* Top 32k of WOPCM is reserved (8K stack + 24k RC6 context). */
> >+	if (size > GUC_WOPCM_SIZE_VALUE - 0x8000) {
> >+		DRM_ERROR("Firmware is too large to fit in WOPCM\n");
> >+		goto fail;
> >+	}
> >
> >  	/*
> >  	 * The GuC firmware image has the version number embedded at a well-known
> >@@ -487,9 +511,8 @@ static void guc_fw_fetch(struct drm_device *dev, struct intel_guc_fw *guc_fw)
> >  	 * TWO bytes each (i.e. u16), although all pointers and offsets are defined
> >  	 * in terms of bytes (u8).
> >  	 */
> >-	css_header = fw->data + UOS_CSS_HEADER_OFFSET;
> >-	guc_fw->guc_fw_major_found = *(u16 *)(css_header + UOS_VER_MAJOR_OFFSET);
> >-	guc_fw->guc_fw_minor_found = *(u16 *)(css_header + UOS_VER_MINOR_OFFSET);
> >+	guc_fw->guc_fw_major_found = css->guc_sw_version >> 16;
> >+	guc_fw->guc_fw_minor_found = css->guc_sw_version & 0xFFFF;
> >
> >  	if (guc_fw->guc_fw_major_found != guc_fw->guc_fw_major_wanted ||
> >  	    guc_fw->guc_fw_minor_found < guc_fw->guc_fw_minor_wanted) {
> >
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux