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