[Intel-gfx] [PATCH v6] drm/i915/guc: Add GuC css header parser

Dave Gordon david.s.gordon at intel.com
Wed Oct 21 04:13:13 PDT 2015


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



More information about the Intel-gfx mailing list