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

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

[yu.dai]

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

By using information from GuC css header, we can eliminate some
hard code w.r.t size of some components of firmware.

Signed-off-by: Alex Dai <yu.dai@intel.com>
---
 drivers/gpu/drm/i915/intel_guc.h        |  2 +-
 drivers/gpu/drm/i915/intel_guc_fwif.h   | 36 +++++++++++++
 drivers/gpu/drm/i915/intel_guc_loader.c | 91 ++++++++++++++++++++++-----------
 3 files changed, 98 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 4ec2d27..e1389fc 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -71,6 +71,7 @@ struct intel_guc_fw {
 	struct drm_i915_gem_object *	guc_fw_obj;
 	enum intel_guc_fw_status	guc_fw_fetch_status;
 	enum intel_guc_fw_status	guc_fw_load_status;
+	struct guc_css_header		guc_fw_header;
 
 	uint16_t			guc_fw_major_wanted;
 	uint16_t			guc_fw_minor_wanted;
@@ -80,7 +81,6 @@ struct intel_guc_fw {
 
 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..d6cb4e8 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -122,6 +122,42 @@
 
 #define GUC_CTL_MAX_DWORDS		(GUC_CTL_RSRVD + 1)
 
+struct guc_css_header {
+	uint32_t module_type;
+	uint32_t header_len; /* header length plus size of all other keys */
+	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; /* uCode size plus header_len */
+	uint32_t key_size;
+	uint32_t modulus_size;
+	uint32_t exponent_size;
+	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;
+};
+
 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 5823615..96826ae 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -215,18 +215,24 @@ static inline bool guc_ucode_response(struct drm_i915_private *dev_priv,
 		((val & GS_MIA_CORE_STATE) && uk_val == GS_UKERNEL_LAPIC_DONE));
 }
 
-/*
+/**
  * Transfer the firmware image to RAM for execution by the microcontroller.
  *
  * GuC Firmware layout:
- * +-------------------------------+  ----
- * |          CSS header           |  128B
+ * +-------------------------------+
+ * |        guc_css_header         |
  * | contains major/minor version  |
- * +-------------------------------+  ----
+ * +-------------------------------+
  * |             uCode             |
- * +-------------------------------+  ----
- * |         RSA signature         |  256B
- * +-------------------------------+  ----
+ * +-------------------------------+
+ * |         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.
  *
  * Architecturally, the DMA engine is bidirectional, and can potentially even
  * transfer between GTT locations. This functionality is left out of the API
@@ -236,30 +242,39 @@ static inline bool guc_ucode_response(struct drm_i915_private *dev_priv,
  * 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 guc_css_header *header = &guc_fw->guc_fw_header;
 	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, header_size, rsa_size, ucode_size, *rsa;
 	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);
+	/* The header plus uCode will be copied to WOPCM via DMA, excluding any
+	 * other components */
+	header_size = (header->header_len - header->key_size -
+		header->modulus_size - header->exponent_size) * sizeof(u32);
+	ucode_size = (header->size - header->header_len) * sizeof(u32);
+
+	I915_WRITE(DMA_COPY_SIZE, header_size + ucode_size);
+
+	/* where RSA signature starts */
+	offset = header_size + ucode_size;
+
+	rsa_size = header->key_size * sizeof(u32);
+	rsa = kmalloc(rsa_size, GFP_KERNEL);
+	if (!rsa)
+		return -ENOMEM;
 
 	/* 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, rsa_size, offset);
+	for (i = 0; i < rsa_size / sizeof(u32); i++)
 		I915_WRITE(UOS_RSA_SCRATCH_0 + i * sizeof(u32), rsa[i]);
 
+	kfree(rsa);
+
 	/* Set the source address for the new blob */
 	offset = i915_gem_obj_ggtt_offset(fw_obj);
 	I915_WRITE(DMA_ADDR_0_LOW, lower_32_bits(offset));
@@ -458,10 +473,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 = &guc_fw->guc_fw_header;
+	size_t size;
 	int err;
 
 	DRM_DEBUG_DRIVER("before requesting firmware: GuC fw fetch status %s\n",
@@ -475,12 +488,31 @@ 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;
+	}
+
+	memcpy(css_header, fw->data, sizeof(struct guc_css_header));
+
+	/* At least, it should have header, uCode and RSA. Size of all three. */
+	size = (css_header->size - css_header->modulus_size -
+			css_header->exponent_size) * sizeof(u32);
+	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 -= css_header->key_size * sizeof(u32);
+
+	/* 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
@@ -488,9 +520,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_header->guc_sw_version >> 16;
+	guc_fw->guc_fw_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) {
-- 
1.9.1

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

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

[Jani Nikula]

On Thu, 03 Sep 2015, yu.dai@intel.com wrote:
> From: Alex Dai <yu.dai@intel.com>
>
> By using information from GuC css header, we can eliminate some
> hard code w.r.t size of some components of firmware.
>
> Signed-off-by: Alex Dai <yu.dai@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_guc.h        |  2 +-
>  drivers/gpu/drm/i915/intel_guc_fwif.h   | 36 +++++++++++++
>  drivers/gpu/drm/i915/intel_guc_loader.c | 91 ++++++++++++++++++++++-----------
>  3 files changed, 98 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
> index 4ec2d27..e1389fc 100644
> --- a/drivers/gpu/drm/i915/intel_guc.h
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -71,6 +71,7 @@ struct intel_guc_fw {
>  	struct drm_i915_gem_object *	guc_fw_obj;
>  	enum intel_guc_fw_status	guc_fw_fetch_status;
>  	enum intel_guc_fw_status	guc_fw_load_status;
> +	struct guc_css_header		guc_fw_header;
>  
>  	uint16_t			guc_fw_major_wanted;
>  	uint16_t			guc_fw_minor_wanted;
> @@ -80,7 +81,6 @@ struct intel_guc_fw {
>  
>  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..d6cb4e8 100644
> --- a/drivers/gpu/drm/i915/intel_guc_fwif.h
> +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
> @@ -122,6 +122,42 @@
>  
>  #define GUC_CTL_MAX_DWORDS		(GUC_CTL_RSRVD + 1)
>  
> +struct guc_css_header {
> +	uint32_t module_type;
> +	uint32_t header_len; /* header length plus size of all other keys */
> +	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; /* uCode size plus header_len */
> +	uint32_t key_size;
> +	uint32_t modulus_size;
> +	uint32_t exponent_size;
> +	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;
> +};

Drive-by review, this will need __packed.

BR,
Jani.


> +
>  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 5823615..96826ae 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -215,18 +215,24 @@ static inline bool guc_ucode_response(struct drm_i915_private *dev_priv,
>  		((val & GS_MIA_CORE_STATE) && uk_val == GS_UKERNEL_LAPIC_DONE));
>  }
>  
> -/*
> +/**
>   * Transfer the firmware image to RAM for execution by the microcontroller.
>   *
>   * GuC Firmware layout:
> - * +-------------------------------+  ----
> - * |          CSS header           |  128B
> + * +-------------------------------+
> + * |        guc_css_header         |
>   * | contains major/minor version  |
> - * +-------------------------------+  ----
> + * +-------------------------------+
>   * |             uCode             |
> - * +-------------------------------+  ----
> - * |         RSA signature         |  256B
> - * +-------------------------------+  ----
> + * +-------------------------------+
> + * |         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.
>   *
>   * Architecturally, the DMA engine is bidirectional, and can potentially even
>   * transfer between GTT locations. This functionality is left out of the API
> @@ -236,30 +242,39 @@ static inline bool guc_ucode_response(struct drm_i915_private *dev_priv,
>   * 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 guc_css_header *header = &guc_fw->guc_fw_header;
>  	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, header_size, rsa_size, ucode_size, *rsa;
>  	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);
> +	/* The header plus uCode will be copied to WOPCM via DMA, excluding any
> +	 * other components */
> +	header_size = (header->header_len - header->key_size -
> +		header->modulus_size - header->exponent_size) * sizeof(u32);
> +	ucode_size = (header->size - header->header_len) * sizeof(u32);
> +
> +	I915_WRITE(DMA_COPY_SIZE, header_size + ucode_size);
> +
> +	/* where RSA signature starts */
> +	offset = header_size + ucode_size;
> +
> +	rsa_size = header->key_size * sizeof(u32);
> +	rsa = kmalloc(rsa_size, GFP_KERNEL);
> +	if (!rsa)
> +		return -ENOMEM;
>  
>  	/* 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, rsa_size, offset);
> +	for (i = 0; i < rsa_size / sizeof(u32); i++)
>  		I915_WRITE(UOS_RSA_SCRATCH_0 + i * sizeof(u32), rsa[i]);
>  
> +	kfree(rsa);
> +
>  	/* Set the source address for the new blob */
>  	offset = i915_gem_obj_ggtt_offset(fw_obj);
>  	I915_WRITE(DMA_ADDR_0_LOW, lower_32_bits(offset));
> @@ -458,10 +473,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 = &guc_fw->guc_fw_header;
> +	size_t size;
>  	int err;
>  
>  	DRM_DEBUG_DRIVER("before requesting firmware: GuC fw fetch status %s\n",
> @@ -475,12 +488,31 @@ 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;
> +	}
> +
> +	memcpy(css_header, fw->data, sizeof(struct guc_css_header));
> +
> +	/* At least, it should have header, uCode and RSA. Size of all three. */
> +	size = (css_header->size - css_header->modulus_size -
> +			css_header->exponent_size) * sizeof(u32);
> +	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 -= css_header->key_size * sizeof(u32);
> +
> +	/* 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
> @@ -488,9 +520,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_header->guc_sw_version >> 16;
> +	guc_fw->guc_fw_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) {
> -- 
> 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

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

[Yu Dai]

On 09/03/2015 12:36 AM, Jani Nikula wrote:
> On Thu, 03 Sep 2015, yu.dai@intel.com wrote:
> > From: Alex Dai <yu.dai@intel.com>
> >
> > By using information from GuC css header, we can eliminate some
> > hard code w.r.t size of some components of firmware.
> >
> > Signed-off-by: Alex Dai <yu.dai@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_guc.h        |  2 +-
> >  drivers/gpu/drm/i915/intel_guc_fwif.h   | 36 +++++++++++++
> >  drivers/gpu/drm/i915/intel_guc_loader.c | 91 ++++++++++++++++++++++-----------
> >  3 files changed, 98 insertions(+), 31 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
> > index 4ec2d27..e1389fc 100644
> > --- a/drivers/gpu/drm/i915/intel_guc.h
> > +++ b/drivers/gpu/drm/i915/intel_guc.h
> > @@ -71,6 +71,7 @@ struct intel_guc_fw {
> >  	struct drm_i915_gem_object *	guc_fw_obj;
> >  	enum intel_guc_fw_status	guc_fw_fetch_status;
> >  	enum intel_guc_fw_status	guc_fw_load_status;
> > +	struct guc_css_header		guc_fw_header;
> >
> >  	uint16_t			guc_fw_major_wanted;
> >  	uint16_t			guc_fw_minor_wanted;
> > @@ -80,7 +81,6 @@ struct intel_guc_fw {
> >
> >  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..d6cb4e8 100644
> > --- a/drivers/gpu/drm/i915/intel_guc_fwif.h
> > +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
> > @@ -122,6 +122,42 @@
> >
> >  #define GUC_CTL_MAX_DWORDS		(GUC_CTL_RSRVD + 1)
> >
> > +struct guc_css_header {
> > +	uint32_t module_type;
> > +	uint32_t header_len; /* header length plus size of all other keys */
> > +	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; /* uCode size plus header_len */
> > +	uint32_t key_size;
> > +	uint32_t modulus_size;
> > +	uint32_t exponent_size;
> > +	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;
> > +};
>
> Drive-by review, this will need __packed.
>
>
Yes, will correct this in next version. Thanks, -Alex
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Daniel Vetter]

On Wed, Sep 02, 2015 at 03:52:35PM -0700, yu.dai@intel.com wrote:
> From: Alex Dai <yu.dai@intel.com>
> 
> By using information from GuC css header, we can eliminate some
> hard code w.r.t size of some components of firmware.
> 
> Signed-off-by: Alex Dai <yu.dai@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_guc.h        |  2 +-
>  drivers/gpu/drm/i915/intel_guc_fwif.h   | 36 +++++++++++++
>  drivers/gpu/drm/i915/intel_guc_loader.c | 91 ++++++++++++++++++++++-----------
>  3 files changed, 98 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
> index 4ec2d27..e1389fc 100644
> --- a/drivers/gpu/drm/i915/intel_guc.h
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -71,6 +71,7 @@ struct intel_guc_fw {
>  	struct drm_i915_gem_object *	guc_fw_obj;
>  	enum intel_guc_fw_status	guc_fw_fetch_status;
>  	enum intel_guc_fw_status	guc_fw_load_status;
> +	struct guc_css_header		guc_fw_header;
>  
>  	uint16_t			guc_fw_major_wanted;
>  	uint16_t			guc_fw_minor_wanted;
> @@ -80,7 +81,6 @@ struct intel_guc_fw {
>  
>  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..d6cb4e8 100644
> --- a/drivers/gpu/drm/i915/intel_guc_fwif.h
> +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
> @@ -122,6 +122,42 @@
>  
>  #define GUC_CTL_MAX_DWORDS		(GUC_CTL_RSRVD + 1)
>  
> +struct guc_css_header {
> +	uint32_t module_type;
> +	uint32_t header_len; /* header length plus size of all other keys */
> +	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; /* uCode size plus header_len */
> +	uint32_t key_size;
> +	uint32_t modulus_size;
> +	uint32_t exponent_size;
> +	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;
> +};
> +
>  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 5823615..96826ae 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -215,18 +215,24 @@ static inline bool guc_ucode_response(struct drm_i915_private *dev_priv,
>  		((val & GS_MIA_CORE_STATE) && uk_val == GS_UKERNEL_LAPIC_DONE));
>  }
>  
> -/*
> +/**

You need at least a DOC: here plus add a line to pull this into the
drm.tmpl for kerneldoc to pick this up. Otherwise it'll complain. But I'd
really like this to be in the docbook, that's for sure.
-Daniel


>   * Transfer the firmware image to RAM for execution by the microcontroller.
>   *
>   * GuC Firmware layout:
> - * +-------------------------------+  ----
> - * |          CSS header           |  128B
> + * +-------------------------------+
> + * |        guc_css_header         |
>   * | contains major/minor version  |
> - * +-------------------------------+  ----
> + * +-------------------------------+
>   * |             uCode             |
> - * +-------------------------------+  ----
> - * |         RSA signature         |  256B
> - * +-------------------------------+  ----
> + * +-------------------------------+
> + * |         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.
>   *
>   * Architecturally, the DMA engine is bidirectional, and can potentially even
>   * transfer between GTT locations. This functionality is left out of the API
> @@ -236,30 +242,39 @@ static inline bool guc_ucode_response(struct drm_i915_private *dev_priv,
>   * 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 guc_css_header *header = &guc_fw->guc_fw_header;
>  	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, header_size, rsa_size, ucode_size, *rsa;
>  	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);
> +	/* The header plus uCode will be copied to WOPCM via DMA, excluding any
> +	 * other components */
> +	header_size = (header->header_len - header->key_size -
> +		header->modulus_size - header->exponent_size) * sizeof(u32);
> +	ucode_size = (header->size - header->header_len) * sizeof(u32);
> +
> +	I915_WRITE(DMA_COPY_SIZE, header_size + ucode_size);
> +
> +	/* where RSA signature starts */
> +	offset = header_size + ucode_size;
> +
> +	rsa_size = header->key_size * sizeof(u32);
> +	rsa = kmalloc(rsa_size, GFP_KERNEL);
> +	if (!rsa)
> +		return -ENOMEM;
>  
>  	/* 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, rsa_size, offset);
> +	for (i = 0; i < rsa_size / sizeof(u32); i++)
>  		I915_WRITE(UOS_RSA_SCRATCH_0 + i * sizeof(u32), rsa[i]);
>  
> +	kfree(rsa);
> +
>  	/* Set the source address for the new blob */
>  	offset = i915_gem_obj_ggtt_offset(fw_obj);
>  	I915_WRITE(DMA_ADDR_0_LOW, lower_32_bits(offset));
> @@ -458,10 +473,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 = &guc_fw->guc_fw_header;
> +	size_t size;
>  	int err;
>  
>  	DRM_DEBUG_DRIVER("before requesting firmware: GuC fw fetch status %s\n",
> @@ -475,12 +488,31 @@ 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;
> +	}
> +
> +	memcpy(css_header, fw->data, sizeof(struct guc_css_header));
> +
> +	/* At least, it should have header, uCode and RSA. Size of all three. */
> +	size = (css_header->size - css_header->modulus_size -
> +			css_header->exponent_size) * sizeof(u32);
> +	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 -= css_header->key_size * sizeof(u32);
> +
> +	/* 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
> @@ -488,9 +520,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_header->guc_sw_version >> 16;
> +	guc_fw->guc_fw_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) {
> -- 
> 1.9.1
> 
> _______________________________________________
> 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

[PATCH v1] drm/i915: Add GuC css header parser

[yu.dai]

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

By using information from GuC css header, we can eliminate some
hard code w.r.t size of some components of firmware.

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          |  24 +++++++-
 drivers/gpu/drm/i915/intel_guc.h        |   2 +-
 drivers/gpu/drm/i915/intel_guc_fwif.h   |  36 +++++++++++
 drivers/gpu/drm/i915/intel_guc_loader.c | 105 +++++++++++++++++++++-----------
 4 files changed, 130 insertions(+), 37 deletions(-)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index 66bc646..f430da6 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -4155,14 +4155,34 @@ 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>
+        <para>The GuC firmware layout looks like this:</para>
+<programlisting><![CDATA[
+ +-------------------------------+
+ |        guc_css_header         |
+ | contains major/minor version  |
+ +-------------------------------+
+ |             uCode             |
+ +-------------------------------+
+ |         RSA signature         |
+ +-------------------------------+
+ |          modulus key          |
+ +-------------------------------+
+ |          exponent val         |
+ +-------------------------------+
+]]></programlisting>
+!Pdrivers/gpu/drm/i915/intel_guc_loader.c GuC Firmware Layout
+!Idrivers/gpu/drm/i915/intel_guc_loader.c
+      </sect2>
     </sect1>
 
     <sect1>
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 4ec2d27..e1389fc 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -71,6 +71,7 @@ struct intel_guc_fw {
 	struct drm_i915_gem_object *	guc_fw_obj;
 	enum intel_guc_fw_status	guc_fw_fetch_status;
 	enum intel_guc_fw_status	guc_fw_load_status;
+	struct guc_css_header		guc_fw_header;
 
 	uint16_t			guc_fw_major_wanted;
 	uint16_t			guc_fw_minor_wanted;
@@ -80,7 +81,6 @@ struct intel_guc_fw {
 
 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..006dc0d 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -122,6 +122,42 @@
 
 #define GUC_CTL_MAX_DWORDS		(GUC_CTL_RSRVD + 1)
 
+struct guc_css_header {
+	uint32_t module_type;
+	uint32_t header_len; /* header length plus size of all other keys */
+	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; /* uCode size plus header_len */
+	uint32_t key_size;
+	uint32_t modulus_size;
+	uint32_t exponent_size;
+	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 5823615..0c3c4c9 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -215,18 +215,27 @@ static inline bool guc_ucode_response(struct drm_i915_private *dev_priv,
 		((val & GS_MIA_CORE_STATE) && uk_val == GS_UKERNEL_LAPIC_DONE));
 }
 
-/*
- * Transfer the firmware image to RAM for execution by the microcontroller.
+/**
+ * DOC: GuC Firmware Layout
  *
- * GuC Firmware layout:
- * +-------------------------------+  ----
- * |          CSS header           |  128B
- * | contains major/minor version  |
- * +-------------------------------+  ----
- * |             uCode             |
- * +-------------------------------+  ----
- * |         RSA signature         |  256B
- * +-------------------------------+  ----
+ +-------------------------------+
+ |        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.
+ * Size of each components (in dwords) can be found in header. In the case that
+ * modulus and exponent are not present in fw, the size value still appears in
+ * header.
  *
  * Architecturally, the DMA engine is bidirectional, and can potentially even
  * transfer between GTT locations. This functionality is left out of the API
@@ -236,30 +245,42 @@ static inline bool guc_ucode_response(struct drm_i915_private *dev_priv,
  * 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
-
+/*
+ * Transfer the firmware image to RAM for execution by the microcontroller.
+ */
 static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv)
 {
 	struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw;
+	struct guc_css_header *header = &guc_fw->guc_fw_header;
 	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, header_size, rsa_size, ucode_size, *rsa;
 	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);
+	/* The header plus uCode will be copied to WOPCM via DMA, excluding any
+	 * other components */
+	header_size = (header->header_len - header->key_size -
+		header->modulus_size - header->exponent_size) * sizeof(u32);
+	ucode_size = (header->size - header->header_len) * sizeof(u32);
+
+	I915_WRITE(DMA_COPY_SIZE, header_size + ucode_size);
+
+	/* where RSA signature starts */
+	offset = header_size + ucode_size;
+
+	rsa_size = header->key_size * sizeof(u32);
+	rsa = kmalloc(rsa_size, GFP_KERNEL);
+	if (!rsa)
+		return -ENOMEM;
 
 	/* 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, rsa_size, offset);
+	for (i = 0; i < rsa_size / sizeof(u32); i++)
 		I915_WRITE(UOS_RSA_SCRATCH_0 + i * sizeof(u32), rsa[i]);
 
+	kfree(rsa);
+
 	/* Set the source address for the new blob */
 	offset = i915_gem_obj_ggtt_offset(fw_obj);
 	I915_WRITE(DMA_ADDR_0_LOW, lower_32_bits(offset));
@@ -458,10 +479,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 = &guc_fw->guc_fw_header;
+	size_t size;
 	int err;
 
 	DRM_DEBUG_DRIVER("before requesting firmware: GuC fw fetch status %s\n",
@@ -475,12 +494,31 @@ 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;
+	}
+
+	memcpy(css_header, fw->data, sizeof(struct guc_css_header));
+
+	/* At least, it should have header, uCode and RSA. Size of all three. */
+	size = (css_header->size - css_header->modulus_size -
+			css_header->exponent_size) * sizeof(u32);
+	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 -= css_header->key_size * sizeof(u32);
+
+	/* 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
@@ -488,9 +526,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_header->guc_sw_version >> 16;
+	guc_fw->guc_fw_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) {
-- 
1.9.1

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

[PATCH 02/15] drm/i915/guc: Add GuC css header parser

[yu.dai]

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

By using information from GuC css header, we can eliminate some
hard code w.r.t size of some components of firmware.

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/intel_guc.h        |   2 +-
 drivers/gpu/drm/i915/intel_guc_fwif.h   |  36 +++++++++++
 drivers/gpu/drm/i915/intel_guc_loader.c | 107 ++++++++++++++++++++++----------
 4 files changed, 117 insertions(+), 37 deletions(-)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index 66bc646..116332f 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_loader.c GuC Firmware Layout
+!Idrivers/gpu/drm/i915/intel_guc_loader.c
+      </sect2>
     </sect1>
 
     <sect1>
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 4ec2d27..e1389fc 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -71,6 +71,7 @@ struct intel_guc_fw {
 	struct drm_i915_gem_object *	guc_fw_obj;
 	enum intel_guc_fw_status	guc_fw_fetch_status;
 	enum intel_guc_fw_status	guc_fw_load_status;
+	struct guc_css_header		guc_fw_header;
 
 	uint16_t			guc_fw_major_wanted;
 	uint16_t			guc_fw_minor_wanted;
@@ -80,7 +81,6 @@ struct intel_guc_fw {
 
 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..006dc0d 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -122,6 +122,42 @@
 
 #define GUC_CTL_MAX_DWORDS		(GUC_CTL_RSRVD + 1)
 
+struct guc_css_header {
+	uint32_t module_type;
+	uint32_t header_len; /* header length plus size of all other keys */
+	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; /* uCode size plus header_len */
+	uint32_t key_size;
+	uint32_t modulus_size;
+	uint32_t exponent_size;
+	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..a6703b4 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -215,18 +215,29 @@ static inline bool guc_ucode_response(struct drm_i915_private *dev_priv,
 		((val & GS_MIA_CORE_STATE) && uk_val == GS_UKERNEL_LAPIC_DONE));
 }
 
-/*
- * Transfer the firmware image to RAM for execution by the microcontroller.
+/**
+ * DOC: GuC Firmware Layout
  *
- * GuC Firmware layout:
- * +-------------------------------+  ----
- * |          CSS header           |  128B
- * | contains major/minor version  |
- * +-------------------------------+  ----
- * |             uCode             |
- * +-------------------------------+  ----
- * |         RSA signature         |  256B
- * +-------------------------------+  ----
+ * 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.
+ * Size of each components (in dwords) can be found in header. In the case that
+ * modulus and exponent are not present in fw, the size value still appears in
+ * header.
  *
  * Architecturally, the DMA engine is bidirectional, and can potentially even
  * transfer between GTT locations. This functionality is left out of the API
@@ -236,30 +247,42 @@ static inline bool guc_ucode_response(struct drm_i915_private *dev_priv,
  * 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
-
+/*
+ * Transfer the firmware image to RAM for execution by the microcontroller.
+ */
 static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv)
 {
 	struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw;
+	struct guc_css_header *header = &guc_fw->guc_fw_header;
 	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, header_size, rsa_size, ucode_size, *rsa;
 	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);
+	/* The header plus uCode will be copied to WOPCM via DMA, excluding any
+	 * other components */
+	header_size = (header->header_len - header->key_size -
+		header->modulus_size - header->exponent_size) * sizeof(u32);
+	ucode_size = (header->size - header->header_len) * sizeof(u32);
+
+	I915_WRITE(DMA_COPY_SIZE, header_size + ucode_size);
+
+	/* where RSA signature starts */
+	offset = header_size + ucode_size;
+
+	rsa_size = header->key_size * sizeof(u32);
+	rsa = kmalloc(rsa_size, GFP_KERNEL);
+	if (!rsa)
+		return -ENOMEM;
 
 	/* 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, rsa_size, offset);
+	for (i = 0; i < rsa_size / sizeof(u32); i++)
 		I915_WRITE(UOS_RSA_SCRATCH_0 + i * sizeof(u32), rsa[i]);
 
+	kfree(rsa);
+
 	/* Set the source address for the new blob */
 	offset = i915_gem_obj_ggtt_offset(fw_obj);
 	I915_WRITE(DMA_ADDR_0_LOW, lower_32_bits(offset));
@@ -465,10 +488,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 = &guc_fw->guc_fw_header;
+	size_t size;
 	int err;
 
 	DRM_DEBUG_DRIVER("before requesting firmware: GuC fw fetch status %s\n",
@@ -482,12 +503,31 @@ 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;
+	}
+
+	memcpy(css_header, fw->data, sizeof(struct guc_css_header));
+
+	/* At least, it should have header, uCode and RSA. Size of all three. */
+	size = (css_header->size - css_header->modulus_size -
+			css_header->exponent_size) * sizeof(u32);
+	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 -= css_header->key_size * sizeof(u32);
+
+	/* 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 +535,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_header->guc_sw_version >> 16;
+	guc_fw->guc_fw_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) {
-- 
1.9.1

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

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

[Jani Nikula]

On Wed, 16 Sep 2015, yu.dai@intel.com wrote:
> From: Alex Dai <yu.dai@intel.com>
>
> By using information from GuC css header, we can eliminate some
> hard code w.r.t size of some components of firmware.

There's a catch here. You can't trust any of the firmware blob
contents. None at all. It's all malicious user input as far as the
kernel is concerned, and we don't have the means to check the signature.

I like the use of the __packed struct here, and I like making this more
dynamic. But you have to check the bounds for everything you use from
the css header.

BR,
Jani.


>
> 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/intel_guc.h        |   2 +-
>  drivers/gpu/drm/i915/intel_guc_fwif.h   |  36 +++++++++++
>  drivers/gpu/drm/i915/intel_guc_loader.c | 107 ++++++++++++++++++++++----------
>  4 files changed, 117 insertions(+), 37 deletions(-)
>
> diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
> index 66bc646..116332f 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_loader.c GuC Firmware Layout
> +!Idrivers/gpu/drm/i915/intel_guc_loader.c
> +      </sect2>
>      </sect1>
>  
>      <sect1>
> diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
> index 4ec2d27..e1389fc 100644
> --- a/drivers/gpu/drm/i915/intel_guc.h
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -71,6 +71,7 @@ struct intel_guc_fw {
>  	struct drm_i915_gem_object *	guc_fw_obj;
>  	enum intel_guc_fw_status	guc_fw_fetch_status;
>  	enum intel_guc_fw_status	guc_fw_load_status;
> +	struct guc_css_header		guc_fw_header;
>  
>  	uint16_t			guc_fw_major_wanted;
>  	uint16_t			guc_fw_minor_wanted;
> @@ -80,7 +81,6 @@ struct intel_guc_fw {
>  
>  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..006dc0d 100644
> --- a/drivers/gpu/drm/i915/intel_guc_fwif.h
> +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
> @@ -122,6 +122,42 @@
>  
>  #define GUC_CTL_MAX_DWORDS		(GUC_CTL_RSRVD + 1)
>  
> +struct guc_css_header {
> +	uint32_t module_type;
> +	uint32_t header_len; /* header length plus size of all other keys */
> +	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; /* uCode size plus header_len */
> +	uint32_t key_size;
> +	uint32_t modulus_size;
> +	uint32_t exponent_size;
> +	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..a6703b4 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -215,18 +215,29 @@ static inline bool guc_ucode_response(struct drm_i915_private *dev_priv,
>  		((val & GS_MIA_CORE_STATE) && uk_val == GS_UKERNEL_LAPIC_DONE));
>  }
>  
> -/*
> - * Transfer the firmware image to RAM for execution by the microcontroller.
> +/**
> + * DOC: GuC Firmware Layout
>   *
> - * GuC Firmware layout:
> - * +-------------------------------+  ----
> - * |          CSS header           |  128B
> - * | contains major/minor version  |
> - * +-------------------------------+  ----
> - * |             uCode             |
> - * +-------------------------------+  ----
> - * |         RSA signature         |  256B
> - * +-------------------------------+  ----
> + * 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.
> + * Size of each components (in dwords) can be found in header. In the case that
> + * modulus and exponent are not present in fw, the size value still appears in
> + * header.
>   *
>   * Architecturally, the DMA engine is bidirectional, and can potentially even
>   * transfer between GTT locations. This functionality is left out of the API
> @@ -236,30 +247,42 @@ static inline bool guc_ucode_response(struct drm_i915_private *dev_priv,
>   * 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
> -
> +/*
> + * Transfer the firmware image to RAM for execution by the microcontroller.
> + */
>  static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv)
>  {
>  	struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw;
> +	struct guc_css_header *header = &guc_fw->guc_fw_header;
>  	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, header_size, rsa_size, ucode_size, *rsa;
>  	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);
> +	/* The header plus uCode will be copied to WOPCM via DMA, excluding any
> +	 * other components */
> +	header_size = (header->header_len - header->key_size -
> +		header->modulus_size - header->exponent_size) * sizeof(u32);
> +	ucode_size = (header->size - header->header_len) * sizeof(u32);
> +
> +	I915_WRITE(DMA_COPY_SIZE, header_size + ucode_size);
> +
> +	/* where RSA signature starts */
> +	offset = header_size + ucode_size;
> +
> +	rsa_size = header->key_size * sizeof(u32);
> +	rsa = kmalloc(rsa_size, GFP_KERNEL);
> +	if (!rsa)
> +		return -ENOMEM;
>  
>  	/* 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, rsa_size, offset);
> +	for (i = 0; i < rsa_size / sizeof(u32); i++)
>  		I915_WRITE(UOS_RSA_SCRATCH_0 + i * sizeof(u32), rsa[i]);
>  
> +	kfree(rsa);
> +
>  	/* Set the source address for the new blob */
>  	offset = i915_gem_obj_ggtt_offset(fw_obj);
>  	I915_WRITE(DMA_ADDR_0_LOW, lower_32_bits(offset));
> @@ -465,10 +488,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 = &guc_fw->guc_fw_header;
> +	size_t size;
>  	int err;
>  
>  	DRM_DEBUG_DRIVER("before requesting firmware: GuC fw fetch status %s\n",
> @@ -482,12 +503,31 @@ 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;
> +	}
> +
> +	memcpy(css_header, fw->data, sizeof(struct guc_css_header));
> +
> +	/* At least, it should have header, uCode and RSA. Size of all three. */
> +	size = (css_header->size - css_header->modulus_size -
> +			css_header->exponent_size) * sizeof(u32);
> +	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 -= css_header->key_size * sizeof(u32);
> +
> +	/* 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 +535,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_header->guc_sw_version >> 16;
> +	guc_fw->guc_fw_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) {
> -- 
> 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

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

[Daniel Vetter]

On Fri, Sep 25, 2015 at 05:36:37PM +0300, Jani Nikula wrote:
> On Wed, 16 Sep 2015, yu.dai@intel.com wrote:
> > From: Alex Dai <yu.dai@intel.com>
> >
> > By using information from GuC css header, we can eliminate some
> > hard code w.r.t size of some components of firmware.
> 
> There's a catch here. You can't trust any of the firmware blob
> contents. None at all. It's all malicious user input as far as the
> kernel is concerned, and we don't have the means to check the signature.
> 
> I like the use of the __packed struct here, and I like making this more
> dynamic. But you have to check the bounds for everything you use from
> the css header.

I'm on the fence about that one really - on real systems firmware is
tightly locked down and there's patches to auth firmware signatures too
afaik. It's blobs we upload to the hw, it could do anything after all ...
-Daniel
-- 
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