All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/guc: Add GuC css header parser
@ 2015-09-25 18:45 yu.dai
  2015-09-28 21:30 ` yu.dai
  0 siblings, 1 reply; 10+ messages in thread
From: yu.dai @ 2015-09-25 18:45 UTC (permalink / raw)
  To: intel-gfx

From: Alex Dai <yu.dai@intel.com>

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.

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@intel.com>
---
 Documentation/DocBook/drm.tmpl          |   9 ++-
 drivers/gpu/drm/i915/i915_debugfs.c     |  10 ++-
 drivers/gpu/drm/i915/i915_guc_reg.h     |   1 +
 drivers/gpu/drm/i915/intel_guc.h        |  16 +++--
 drivers/gpu/drm/i915/intel_guc_fwif.h   |  72 +++++++++++++++++++++
 drivers/gpu/drm/i915/intel_guc_loader.c | 110 +++++++++++++++++++-------------
 6 files changed, 166 insertions(+), 52 deletions(-)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index 66bc646..4d8dffb 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -4155,14 +4155,19 @@ int num_ioctls;</synopsis>
       <title>GuC-based Command Submission</title>
       <sect2>
         <title>GuC</title>
-!Pdrivers/gpu/drm/i915/intel_guc_loader.c GuC-specific firmware loader
+!Pdrivers/gpu/drm/i915/intel_guc_loader.c GuC
 !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
+!Pdrivers/gpu/drm/i915/i915_guc_submission.c GuC Client
 !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
+!Idrivers/gpu/drm/i915/intel_guc_fwif.h
+      </sect2>
     </sect1>
 
     <sect1>
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 5615d3d..0753b2f 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2386,9 +2386,15 @@ static int i915_guc_load_status_info(struct seq_file *m, void *data)
 	seq_printf(m, "\tload: %s\n",
 		intel_guc_fw_status_repr(guc_fw->guc_fw_load_status));
 	seq_printf(m, "\tversion wanted: %d.%d\n",
-		guc_fw->guc_fw_major_wanted, guc_fw->guc_fw_minor_wanted);
+		guc_fw->ver_major_wanted, guc_fw->ver_minor_wanted);
 	seq_printf(m, "\tversion found: %d.%d\n",
-		guc_fw->guc_fw_major_found, guc_fw->guc_fw_minor_found);
+		guc_fw->ver_major_found, guc_fw->ver_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 b355661..578739d 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_0		0xc200
+#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/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 4ec2d27..55c9bf8 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -72,15 +72,21 @@ struct intel_guc_fw {
 	enum intel_guc_fw_status	guc_fw_fetch_status;
 	enum intel_guc_fw_status	guc_fw_load_status;
 
-	uint16_t			guc_fw_major_wanted;
-	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;
+
+	uint16_t ver_major_wanted;
+	uint16_t ver_minor_wanted;
+	uint16_t ver_major_found;
+	uint16_t ver_minor_found;
 };
 
 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 e1f47ba..cd94075 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_len includes all non-uCode bits, which is:
+	  * < css_header + key_len + modulus_len + exponent_len > */
+	uint32_t header_len;
+	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 length; /* uCode length plus header_len */
+	uint32_t key_len;
+	uint32_t modulus_len;
+	uint32_t exponent_len;
+	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 40241f3..e2813d4 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -218,16 +218,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.
@@ -235,33 +225,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_0 + i * sizeof(u32), 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);
 
@@ -465,10 +451,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_header;
+	size_t size;
 	int err;
 
 	DRM_DEBUG_DRIVER("before requesting firmware: GuC fw fetch status %s\n",
@@ -482,12 +466,53 @@ 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_header = (struct guc_css_header *)fw->data;
+
+	/* Firmware bits always start from header */
+	guc_fw->header_offset = 0;
+	guc_fw->header_size = (css_header->header_len - css_header->modulus_len
+		- css_header->key_len - css_header->exponent_len) * 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_header->length - css_header->header_len) * sizeof(u32);
+
+	/* now RSA */
+	if (css_header->key_len != 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_header->key_len * 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
@@ -495,22 +520,21 @@ 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->ver_major_found = css_header->guc_sw_version >> 16;
+	guc_fw->ver_minor_found = css_header->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) {
+	if (guc_fw->ver_major_found != guc_fw->ver_major_wanted ||
+	    guc_fw->ver_minor_found < guc_fw->ver_minor_wanted) {
 		DRM_ERROR("GuC firmware version %d.%d, required %d.%d\n",
-			guc_fw->guc_fw_major_found, guc_fw->guc_fw_minor_found,
-			guc_fw->guc_fw_major_wanted, guc_fw->guc_fw_minor_wanted);
+			guc_fw->ver_major_found, guc_fw->ver_minor_found,
+			guc_fw->ver_major_wanted, guc_fw->ver_minor_wanted);
 		err = -ENOEXEC;
 		goto fail;
 	}
 
 	DRM_DEBUG_DRIVER("firmware version %d.%d OK (minimum %d.%d)\n",
-			guc_fw->guc_fw_major_found, guc_fw->guc_fw_minor_found,
-			guc_fw->guc_fw_major_wanted, guc_fw->guc_fw_minor_wanted);
+			guc_fw->ver_major_found, guc_fw->ver_minor_found,
+			guc_fw->ver_major_wanted, guc_fw->ver_minor_wanted);
 
 	obj = i915_gem_object_create_from_data(dev, fw->data, fw->size);
 	if (IS_ERR_OR_NULL(obj)) {
@@ -567,8 +591,8 @@ void intel_guc_ucode_init(struct drm_device *dev)
 		fw_path = NULL;
 	} else if (IS_SKYLAKE(dev)) {
 		fw_path = I915_SKL_GUC_UCODE;
-		guc_fw->guc_fw_major_wanted = 4;
-		guc_fw->guc_fw_minor_wanted = 3;
+		guc_fw->ver_major_wanted = 4;
+		guc_fw->ver_minor_wanted = 3;
 	} else {
 		i915.enable_guc_submission = false;
 		fw_path = "";	/* unknown device */
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH] drm/i915/guc: Add GuC css header parser
  2015-09-25 18:45 [PATCH] drm/i915/guc: Add GuC css header parser yu.dai
@ 2015-09-28 21:30 ` yu.dai
  2015-10-13 11:13   ` Dave Gordon
  0 siblings, 1 reply; 10+ messages in thread
From: yu.dai @ 2015-09-28 21:30 UTC (permalink / raw)
  To: intel-gfx

From: Alex Dai <yu.dai@intel.com>

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.

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@intel.com>
---
 Documentation/DocBook/drm.tmpl          |  9 +++-
 drivers/gpu/drm/i915/i915_debugfs.c     |  6 +++
 drivers/gpu/drm/i915/i915_guc_reg.h     |  1 +
 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 | 93 ++++++++++++++++++++-------------
 6 files changed, 151 insertions(+), 38 deletions(-)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index 308b141..b7d7e13 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -4155,14 +4155,19 @@ int num_ioctls;</synopsis>
       <title>GuC-based Command Submission</title>
       <sect2>
         <title>GuC</title>
-!Pdrivers/gpu/drm/i915/intel_guc_loader.c GuC-specific firmware loader
+!Pdrivers/gpu/drm/i915/intel_guc_loader.c GuC
 !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
+!Pdrivers/gpu/drm/i915/i915_guc_submission.c GuC Client
 !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
+!Idrivers/gpu/drm/i915/intel_guc_fwif.h
+      </sect2>
     </sect1>
 
     <sect1>
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index afa7982..02693d6 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2396,6 +2396,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/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 4ec2d27..63e73f3 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 e1f47ba..5b4633b 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 5d17b63..3a7f08e 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -218,16 +218,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.
@@ -235,33 +225,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);
 
@@ -465,10 +451,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",
@@ -482,12 +466,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
@@ -495,9 +519,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) {
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915/guc: Add GuC css header parser
  2015-09-28 21:30 ` yu.dai
@ 2015-10-13 11:13   ` Dave Gordon
  2015-10-13 22:30     ` [PATCH v5] " yu.dai
  0 siblings, 1 reply; 10+ messages in thread
From: Dave Gordon @ 2015-10-13 11:13 UTC (permalink / raw)
  To: intel-gfx, Alex Dai

On 28/09/15 22:30, yu.dai@intel.com wrote:
> From: Alex Dai <yu.dai@intel.com>
>
> 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.
>
> 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@intel.com>
> ---
>   Documentation/DocBook/drm.tmpl          |  9 +++-
>   drivers/gpu/drm/i915/i915_debugfs.c     |  6 +++
>   drivers/gpu/drm/i915/i915_guc_reg.h     |  1 +
>   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 | 93 ++++++++++++++++++++-------------
>   6 files changed, 151 insertions(+), 38 deletions(-)

The code looks OK; the CSS header fields are adequately checked now, so:

Reviewed-by: Dave Gordon <david.s.gordon@intel.com>

with one note about the DocBook change below:

> diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
> index 308b141..b7d7e13 100644
> --- a/Documentation/DocBook/drm.tmpl
> +++ b/Documentation/DocBook/drm.tmpl
> @@ -4155,14 +4155,19 @@ int num_ioctls;</synopsis>
>         <title>GuC-based Command Submission</title>
>         <sect2>
>           <title>GuC</title>
> -!Pdrivers/gpu/drm/i915/intel_guc_loader.c GuC-specific firmware loader
> +!Pdrivers/gpu/drm/i915/intel_guc_loader.c GuC

Was this intended? "GuC" doesn't convey much, so I'd suggest keep the 
original or shorten it to "GuC Firmware Loader", or even "GuC Loader", 
but not to just "GuC".

.Dave.

>   !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
> +!Pdrivers/gpu/drm/i915/i915_guc_submission.c GuC Client
>   !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
> +!Idrivers/gpu/drm/i915/intel_guc_fwif.h
> +      </sect2>
>       </sect1>
>
>       <sect1>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index afa7982..02693d6 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2396,6 +2396,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/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
> index 4ec2d27..63e73f3 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 e1f47ba..5b4633b 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 5d17b63..3a7f08e 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -218,16 +218,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.
> @@ -235,33 +225,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);
>
> @@ -465,10 +451,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",
> @@ -482,12 +466,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
> @@ -495,9 +519,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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v5] drm/i915/guc: Add GuC css header parser
  2015-10-13 11:13   ` Dave Gordon
@ 2015-10-13 22:30     ` yu.dai
  2015-10-13 23:20       ` kbuild test robot
                         ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: yu.dai @ 2015-10-13 22:30 UTC (permalink / raw)
  To: intel-gfx

From: Alex Dai <yu.dai@intel.com>

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.

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@intel.com>
---
 Documentation/DocBook/drm.tmpl             | 13 ++--
 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 |  2 +-
 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, 155 insertions(+), 42 deletions(-)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index 9e644b3..dbd5d7d 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -4093,17 +4093,22 @@ 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
+!Idrivers/gpu/drm/i915/intel_guc_fwif.h
+      </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..0611483 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
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..d25c5b7 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) {
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v5] drm/i915/guc: Add GuC css header parser
  2015-10-13 22:30     ` [PATCH v5] " yu.dai
@ 2015-10-13 23:20       ` kbuild test robot
  2015-10-19 23:06       ` [PATCH v6] drm-intel-nightly: 2015y-10m-19d-20h-41m-28s UTC integration manifest yu.dai
  2015-10-19 23:10       ` [PATCH v6] drm/i915/guc: Add GuC css header parser yu.dai
  2 siblings, 0 replies; 10+ messages in thread
From: kbuild test robot @ 2015-10-13 23:20 UTC (permalink / raw)
  To: yu.dai; +Cc: intel-gfx, kbuild-all

[-- Attachment #1: Type: text/plain, Size: 12429 bytes --]

Hi Alex,

[auto build test WARNING on drm-intel/for-linux-next -- if it's inappropriate base, please suggest rules for selecting the more suitable base]

url:    https://github.com/0day-ci/linux/commits/yu-dai-intel-com/drm-i915-guc-Add-GuC-css-header-parser/20151014-063407
reproduce: make htmldocs

All warnings (new ones prefixed by >>):

   include/drm/drm_dp_mst_helper.h:470: warning: No description found for parameter 'destroy_connector_list'
   include/drm/drm_dp_mst_helper.h:470: warning: No description found for parameter 'destroy_connector_lock'
   include/drm/drm_dp_mst_helper.h:470: warning: No description found for parameter 'destroy_connector_work'
   drivers/gpu/drm/drm_dp_mst_topology.c:2211: warning: No description found for parameter 'connector'
   drivers/gpu/drm/drm_irq.c:173: warning: No description found for parameter 'flags'
   include/drm/drmP.h:164: warning: No description found for parameter 'fmt'
   include/drm/drmP.h:180: warning: No description found for parameter 'fmt'
   include/drm/drmP.h:198: warning: No description found for parameter 'fmt'
   include/drm/drmP.h:238: warning: No description found for parameter 'dev'
   include/drm/drmP.h:238: warning: No description found for parameter 'data'
   include/drm/drmP.h:238: warning: No description found for parameter 'file_priv'
   include/drm/drmP.h:271: warning: No description found for parameter 'ioctl'
   include/drm/drmP.h:271: warning: No description found for parameter '_func'
   include/drm/drmP.h:271: warning: No description found for parameter '_flags'
   include/drm/drmP.h:344: warning: cannot understand function prototype: 'struct drm_lock_data '
   include/drm/drmP.h:397: warning: cannot understand function prototype: 'struct drm_driver '
   include/drm/drmP.h:648: warning: cannot understand function prototype: 'struct drm_info_list '
   include/drm/drmP.h:658: warning: cannot understand function prototype: 'struct drm_info_node '
   include/drm/drmP.h:668: warning: cannot understand function prototype: 'struct drm_minor '
   include/drm/drmP.h:716: warning: cannot understand function prototype: 'struct drm_device '
   drivers/gpu/drm/i915/i915_irq.c:2584: warning: No description found for parameter 'wedged'
   drivers/gpu/drm/i915/i915_irq.c:2584: warning: No description found for parameter 'fmt'
   drivers/gpu/drm/i915/i915_irq.c:2584: warning: No description found for parameter 'wedged'
   drivers/gpu/drm/i915/i915_irq.c:2584: warning: No description found for parameter 'fmt'
   drivers/gpu/drm/i915/i915_irq.c:2584: warning: No description found for parameter 'wedged'
   drivers/gpu/drm/i915/i915_irq.c:2584: warning: No description found for parameter 'fmt'
   drivers/gpu/drm/i915/i915_irq.c:2584: warning: No description found for parameter 'wedged'
   drivers/gpu/drm/i915/i915_irq.c:2584: warning: No description found for parameter 'fmt'
   drivers/gpu/drm/i915/i915_gem.c:421: warning: No description found for parameter 'dev'
   drivers/gpu/drm/i915/i915_gem.c:421: warning: No description found for parameter 'data'
   drivers/gpu/drm/i915/i915_gem.c:421: warning: No description found for parameter 'file'
   drivers/gpu/drm/i915/i915_gem.c:686: warning: No description found for parameter 'dev'
   drivers/gpu/drm/i915/i915_gem.c:686: warning: No description found for parameter 'data'
   drivers/gpu/drm/i915/i915_gem.c:686: warning: No description found for parameter 'file'
   drivers/gpu/drm/i915/i915_gem.c:767: warning: No description found for parameter 'dev'
   drivers/gpu/drm/i915/i915_gem.c:767: warning: No description found for parameter 'obj'
   drivers/gpu/drm/i915/i915_gem.c:767: warning: No description found for parameter 'args'
   drivers/gpu/drm/i915/i915_gem.c:767: warning: No description found for parameter 'file'
   drivers/gpu/drm/i915/i915_gem.c:1029: warning: No description found for parameter 'dev'
   drivers/gpu/drm/i915/i915_gem.c:1029: warning: No description found for parameter 'data'
   drivers/gpu/drm/i915/i915_gem.c:1029: warning: No description found for parameter 'file'
   drivers/gpu/drm/i915/i915_gem.c:1194: warning: No description found for parameter 'rps'
   drivers/gpu/drm/i915/i915_gem.c:1400: warning: No description found for parameter 'req'
   drivers/gpu/drm/i915/i915_gem.c:1435: warning: No description found for parameter 'obj'
   drivers/gpu/drm/i915/i915_gem.c:1435: warning: No description found for parameter 'readonly'
   drivers/gpu/drm/i915/i915_gem.c:1558: warning: No description found for parameter 'dev'
   drivers/gpu/drm/i915/i915_gem.c:1558: warning: No description found for parameter 'data'
   drivers/gpu/drm/i915/i915_gem.c:1558: warning: No description found for parameter 'file'
   drivers/gpu/drm/i915/i915_gem.c:1621: warning: No description found for parameter 'dev'
   drivers/gpu/drm/i915/i915_gem.c:1621: warning: No description found for parameter 'data'
   drivers/gpu/drm/i915/i915_gem.c:1621: warning: No description found for parameter 'file'
   drivers/gpu/drm/i915/i915_gem.c:1666: warning: No description found for parameter 'dev'
   drivers/gpu/drm/i915/i915_gem.c:1666: warning: No description found for parameter 'data'
   drivers/gpu/drm/i915/i915_gem.c:1666: warning: No description found for parameter 'file'
   drivers/gpu/drm/i915/i915_gem.c:1954: warning: No description found for parameter 'dev'
   drivers/gpu/drm/i915/i915_gem.c:1954: warning: No description found for parameter 'size'
   drivers/gpu/drm/i915/i915_gem.c:1954: warning: No description found for parameter 'tiling_mode'
   drivers/gpu/drm/i915/i915_gem.c:1954: warning: No description found for parameter 'fenced'
   drivers/gpu/drm/i915/i915_gem.c:1954: warning: Excess function parameter 'obj' description in 'i915_gem_get_gtt_alignment'
   drivers/gpu/drm/i915/i915_gem.c:2817: warning: No description found for parameter 'ring'
   drivers/gpu/drm/i915/i915_gem.c:2946: warning: No description found for parameter 'obj'
   drivers/gpu/drm/i915/i915_gem.c:2996: warning: No description found for parameter 'dev'
   drivers/gpu/drm/i915/i915_gem.c:2996: warning: No description found for parameter 'data'
   drivers/gpu/drm/i915/i915_gem.c:2996: warning: No description found for parameter 'file'
   drivers/gpu/drm/i915/i915_gem.c:2996: warning: Excess function parameter 'DRM_IOCTL_ARGS' description in 'i915_gem_wait_ioctl'
   drivers/gpu/drm/i915/i915_gem.c:3365: warning: No description found for parameter 'obj'
   drivers/gpu/drm/i915/i915_gem.c:3365: warning: No description found for parameter 'vm'
   drivers/gpu/drm/i915/i915_gem.c:3365: warning: No description found for parameter 'ggtt_view'
   drivers/gpu/drm/i915/i915_gem.c:3365: warning: No description found for parameter 'alignment'
   drivers/gpu/drm/i915/i915_gem.c:3365: warning: No description found for parameter 'flags'
   drivers/gpu/drm/i915/i915_gem.c:3600: warning: No description found for parameter 'obj'
   drivers/gpu/drm/i915/i915_gem.c:3600: warning: No description found for parameter 'write'
   drivers/gpu/drm/i915/i915_gem.c:3675: warning: No description found for parameter 'obj'
   drivers/gpu/drm/i915/i915_gem.c:3675: warning: No description found for parameter 'cache_level'
   drivers/gpu/drm/i915/i915_gem.c:3949: warning: No description found for parameter 'obj'
   drivers/gpu/drm/i915/i915_gem.c:3949: warning: No description found for parameter 'write'
   drivers/gpu/drm/i915/intel_lrc.c:869: warning: No description found for parameter 'params'
   drivers/gpu/drm/i915/intel_lrc.c:869: warning: Excess function parameter 'dev' description in 'intel_execlists_submission'
   drivers/gpu/drm/i915/intel_lrc.c:869: warning: Excess function parameter 'file' description in 'intel_execlists_submission'
   drivers/gpu/drm/i915/intel_lrc.c:869: warning: Excess function parameter 'ring' description in 'intel_execlists_submission'
   drivers/gpu/drm/i915/intel_lrc.c:869: warning: Excess function parameter 'ctx' description in 'intel_execlists_submission'
   drivers/gpu/drm/i915/intel_lrc.c:869: warning: Excess function parameter 'batch_obj' description in 'intel_execlists_submission'
   drivers/gpu/drm/i915/intel_lrc.c:869: warning: Excess function parameter 'exec_start' description in 'intel_execlists_submission'
   drivers/gpu/drm/i915/intel_lrc.c:869: warning: Excess function parameter 'dispatch_flags' description in 'intel_execlists_submission'
   drivers/gpu/drm/i915/intel_lrc.c:869: warning: No description found for parameter 'params'
   drivers/gpu/drm/i915/intel_lrc.c:869: warning: Excess function parameter 'dev' description in 'intel_execlists_submission'
   drivers/gpu/drm/i915/intel_lrc.c:869: warning: Excess function parameter 'file' description in 'intel_execlists_submission'
   drivers/gpu/drm/i915/intel_lrc.c:869: warning: Excess function parameter 'ring' description in 'intel_execlists_submission'
   drivers/gpu/drm/i915/intel_lrc.c:869: warning: Excess function parameter 'ctx' description in 'intel_execlists_submission'
   drivers/gpu/drm/i915/intel_lrc.c:869: warning: Excess function parameter 'batch_obj' description in 'intel_execlists_submission'
   drivers/gpu/drm/i915/intel_lrc.c:869: warning: Excess function parameter 'exec_start' description in 'intel_execlists_submission'
   drivers/gpu/drm/i915/intel_lrc.c:869: warning: Excess function parameter 'dispatch_flags' description in 'intel_execlists_submission'
   drivers/gpu/drm/i915/i915_guc_submission.c:598: warning: No description found for parameter 'rq'
   drivers/gpu/drm/i915/i915_guc_submission.c:598: warning: Excess function parameter 'ctx' description in 'i915_guc_submit'
   drivers/gpu/drm/i915/i915_guc_submission.c:598: warning: Excess function parameter 'ring' description in 'i915_guc_submit'
   drivers/gpu/drm/i915/i915_guc_submission.c:741: warning: No description found for parameter 'ctx'
   drivers/gpu/drm/i915/i915_guc_submission.c:598: warning: No description found for parameter 'rq'
   drivers/gpu/drm/i915/i915_guc_submission.c:598: warning: Excess function parameter 'ctx' description in 'i915_guc_submit'
   drivers/gpu/drm/i915/i915_guc_submission.c:598: warning: Excess function parameter 'ring' description in 'i915_guc_submit'
   drivers/gpu/drm/i915/i915_guc_submission.c:741: warning: No description found for parameter 'ctx'
>> drivers/gpu/drm/i915/intel_guc_fwif.h:1: warning: no structured comments found
   Warning: didn't use docs for i915_hotplug_interrupt_update
   Warning: didn't use docs for ilk_update_display_irq
   Warning: didn't use docs for ilk_update_gt_irq
   Warning: didn't use docs for snb_update_pm_irq
   Warning: didn't use docs for bdw_update_port_irq
   Warning: didn't use docs for ibx_display_interrupt_update
   Warning: didn't use docs for i915_enable_asle_pipestat
   Warning: didn't use docs for ivybridge_parity_work
   Warning: didn't use docs for i915_reset_and_wakeup
   Warning: didn't use docs for i915_handle_error
   Warning: didn't use docs for intel_irq_install
   Warning: didn't use docs for intel_irq_uninstall

vim +1 drivers/gpu/drm/i915/intel_guc_fwif.h

2617268f Dave Gordon 2015-07-09 @1  /*
2617268f Dave Gordon 2015-07-09  2   * Copyright © 2014 Intel Corporation
2617268f Dave Gordon 2015-07-09  3   *
2617268f Dave Gordon 2015-07-09  4   * Permission is hereby granted, free of charge, to any person obtaining a
2617268f Dave Gordon 2015-07-09  5   * copy of this software and associated documentation files (the "Software"),
2617268f Dave Gordon 2015-07-09  6   * to deal in the Software without restriction, including without limitation
2617268f Dave Gordon 2015-07-09  7   * the rights to use, copy, modify, merge, publish, distribute, sublicense,
2617268f Dave Gordon 2015-07-09  8   * and/or sell copies of the Software, and to permit persons to whom the
2617268f Dave Gordon 2015-07-09  9   * Software is furnished to do so, subject to the following conditions:

:::::: The code at line 1 was first introduced by commit
:::::: 2617268ff9d8f8b6901a07962c985d774b999e58 drm/i915: Add GuC-related header files

:::::: TO: Dave Gordon <david.s.gordon@intel.com>
:::::: CC: Daniel Vetter <daniel.vetter@ffwll.ch>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 6062 bytes --]

[-- Attachment #3: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v6] drm-intel-nightly: 2015y-10m-19d-20h-41m-28s UTC integration manifest
  2015-10-13 22:30     ` [PATCH v5] " yu.dai
  2015-10-13 23:20       ` kbuild test robot
@ 2015-10-19 23:06       ` yu.dai
  2015-10-20  6:48         ` Jani Nikula
  2015-10-19 23:10       ` [PATCH v6] drm/i915/guc: Add GuC css header parser yu.dai
  2 siblings, 1 reply; 10+ messages in thread
From: yu.dai @ 2015-10-19 23:06 UTC (permalink / raw)
  To: intel-gfx

From: Matt Roper <matthew.d.roper@intel.com>

---
 integration-manifest | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)
 create mode 100644 integration-manifest

diff --git a/integration-manifest b/integration-manifest
new file mode 100644
index 0000000..a726d7c
--- /dev/null
+++ b/integration-manifest
@@ -0,0 +1,24 @@
+origin/drm-intel-fixes 18e9345b0db9fe7bd18c3c43967789fe0a2fdb52
+	drm/i915: Add primary plane to mask if it's visible
+drm-upstream/drm-fixes d549f545e690c3cbdeb33df3579eae3230eb8904
+	drm/virtio: use %llu format string form atomic64_t
+origin/drm-intel-next-fixes 606bb5e0b28b540685fb94c22902cd9a948a3779
+	drm/i915: Use round to closest when computing the CEA 1.001 pixel clocks
+origin/drm-intel-next-queued 024c9045221fe45482863c47c4b4c47d37f97cbf
+	drm/i915/skl: Eliminate usage of pipe_wm_parameters from SKL-style WM (v4)
+drm-upstream/drm-next c76af02d90ee9e9d2ef478fc6f874ad2abcf3ec9
+	via_drm.h: move struct via_file_private definition to drivers/gpu/drm/via/via_drv.h
+sound-upstream/for-next 9a30ae2df29c27eca58581862928ee2c7bbdfa76
+	ALSA: firewire-tascam: off by one in identify_model()
+origin/topic/drm-misc 5ddf10bc9f6f615d27877d7a125f277042e0a046
+	drm/edid: Round to closest when computing the CEA/HDMI alternate clock
+origin/topic/drm-fixes f36203be608a38a5b5523a7aa52cc72f757b9679
+	drm/dp: Add dp_aux_i2c_speed_khz module param to set the assume i2c bus speed
+origin/topic/kerneldoc a6333fcc6ddce4533c280b288ed7c3f0d61b76b5
+	scripts/kernel-doc: Processing -nofunc for functions only
+origin/topic/crc-pmic 04cbfe68c3190f23bcfec230bfd832b533f35554
+	mfd: Add GPIOLIB dependency if INTEL_SOC_PMIC is to be enabled
+origin/topic/connector-locking 3fdefa399e4644399ce3e74e65a75122d52dba6a
+	drm: gc now dead mode_group code
+origin/topic/mst-fixes 6ea76f3cade4734e73e3da842005820558b8b828
+	drm/atomic-helpers: Make encoder picking more robust
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v6] drm/i915/guc: Add GuC css header parser
  2015-10-13 22:30     ` [PATCH v5] " yu.dai
  2015-10-13 23:20       ` kbuild test robot
  2015-10-19 23:06       ` [PATCH v6] drm-intel-nightly: 2015y-10m-19d-20h-41m-28s UTC integration manifest yu.dai
@ 2015-10-19 23:10       ` yu.dai
  2015-10-21 11:13         ` Dave Gordon
  2 siblings, 1 reply; 10+ messages in thread
From: yu.dai @ 2015-10-19 23:10 UTC (permalink / raw)
  To: intel-gfx

From: Alex Dai <yu.dai@intel.com>

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@intel.com>
---
 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) {
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v6] drm-intel-nightly: 2015y-10m-19d-20h-41m-28s UTC integration manifest
  2015-10-19 23:06       ` [PATCH v6] drm-intel-nightly: 2015y-10m-19d-20h-41m-28s UTC integration manifest yu.dai
@ 2015-10-20  6:48         ` Jani Nikula
  0 siblings, 0 replies; 10+ messages in thread
From: Jani Nikula @ 2015-10-20  6:48 UTC (permalink / raw)
  To: yu.dai, intel-gfx


Please take care to drop these when rebasing your own patches.

BR,
Jani.


On Tue, 20 Oct 2015, yu.dai@intel.com wrote:
> From: Matt Roper <matthew.d.roper@intel.com>
>
> ---
>  integration-manifest | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
>  create mode 100644 integration-manifest
>
> diff --git a/integration-manifest b/integration-manifest
> new file mode 100644
> index 0000000..a726d7c
> --- /dev/null
> +++ b/integration-manifest
> @@ -0,0 +1,24 @@
> +origin/drm-intel-fixes 18e9345b0db9fe7bd18c3c43967789fe0a2fdb52
> +	drm/i915: Add primary plane to mask if it's visible
> +drm-upstream/drm-fixes d549f545e690c3cbdeb33df3579eae3230eb8904
> +	drm/virtio: use %llu format string form atomic64_t
> +origin/drm-intel-next-fixes 606bb5e0b28b540685fb94c22902cd9a948a3779
> +	drm/i915: Use round to closest when computing the CEA 1.001 pixel clocks
> +origin/drm-intel-next-queued 024c9045221fe45482863c47c4b4c47d37f97cbf
> +	drm/i915/skl: Eliminate usage of pipe_wm_parameters from SKL-style WM (v4)
> +drm-upstream/drm-next c76af02d90ee9e9d2ef478fc6f874ad2abcf3ec9
> +	via_drm.h: move struct via_file_private definition to drivers/gpu/drm/via/via_drv.h
> +sound-upstream/for-next 9a30ae2df29c27eca58581862928ee2c7bbdfa76
> +	ALSA: firewire-tascam: off by one in identify_model()
> +origin/topic/drm-misc 5ddf10bc9f6f615d27877d7a125f277042e0a046
> +	drm/edid: Round to closest when computing the CEA/HDMI alternate clock
> +origin/topic/drm-fixes f36203be608a38a5b5523a7aa52cc72f757b9679
> +	drm/dp: Add dp_aux_i2c_speed_khz module param to set the assume i2c bus speed
> +origin/topic/kerneldoc a6333fcc6ddce4533c280b288ed7c3f0d61b76b5
> +	scripts/kernel-doc: Processing -nofunc for functions only
> +origin/topic/crc-pmic 04cbfe68c3190f23bcfec230bfd832b533f35554
> +	mfd: Add GPIOLIB dependency if INTEL_SOC_PMIC is to be enabled
> +origin/topic/connector-locking 3fdefa399e4644399ce3e74e65a75122d52dba6a
> +	drm: gc now dead mode_group code
> +origin/topic/mst-fixes 6ea76f3cade4734e73e3da842005820558b8b828
> +	drm/atomic-helpers: Make encoder picking more robust
> -- 
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v6] drm/i915/guc: Add GuC css header parser
  2015-10-19 23:10       ` [PATCH v6] drm/i915/guc: Add GuC css header parser yu.dai
@ 2015-10-21 11:13         ` Dave Gordon
  2015-10-21 12:12           ` Daniel Vetter
  0 siblings, 1 reply; 10+ messages in thread
From: Dave Gordon @ 2015-10-21 11:13 UTC (permalink / raw)
  To: yu.dai, intel-gfx

On 20/10/15 00:10, yu.dai@intel.com wrote:
> From: Alex Dai <yu.dai@intel.com>
>
> 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@intel.com>
> ---

Docbook looks OK now, and the code is the same as last version I 
reviewed, so ...

Reviewed-by: Dave Gordon <david.s.gordon@intel.com>

>   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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v6] drm/i915/guc: Add GuC css header parser
  2015-10-21 11:13         ` Dave Gordon
@ 2015-10-21 12:12           ` Daniel Vetter
  0 siblings, 0 replies; 10+ messages in thread
From: Daniel Vetter @ 2015-10-21 12:12 UTC (permalink / raw)
  To: Dave Gordon; +Cc: intel-gfx

On Wed, Oct 21, 2015 at 12:13:13PM +0100, Dave Gordon wrote:
> On 20/10/15 00:10, yu.dai@intel.com wrote:
> >From: Alex Dai <yu.dai@intel.com>
> >
> >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@intel.com>
> >---
> 
> Docbook looks OK now, and the code is the same as last version I reviewed,
> so ...
> 
> Reviewed-by: Dave Gordon <david.s.gordon@intel.com>

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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2015-10-21 12:12 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-25 18:45 [PATCH] drm/i915/guc: Add GuC css header parser yu.dai
2015-09-28 21:30 ` yu.dai
2015-10-13 11:13   ` Dave Gordon
2015-10-13 22:30     ` [PATCH v5] " yu.dai
2015-10-13 23:20       ` kbuild test robot
2015-10-19 23:06       ` [PATCH v6] drm-intel-nightly: 2015y-10m-19d-20h-41m-28s UTC integration manifest yu.dai
2015-10-20  6:48         ` Jani Nikula
2015-10-19 23:10       ` [PATCH v6] drm/i915/guc: Add GuC css header parser yu.dai
2015-10-21 11:13         ` Dave Gordon
2015-10-21 12:12           ` Daniel Vetter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.