Re: [Intel-gfx] [PATCH v3 3/5] drm/i915/uc: Place uC firmware in upper range of GGTT
Quoting Fernando Pacheco (2019-04-20 00:00:13)
> Currently we pin the GuC or HuC firmware image just
> before uploading. Perma-pin during uC initialization
> instead and use the range reserved at the top of the
> address space.
>
> Moving the firmware resulted in needing to:
> - use an additional pinning for the rsa signature which will
> be used during HuC auth as addresses above GUC_GGTT_TOP
> do not map through GTT.
>
> v2: Remove call to set to gtt domain
> Do not restore fw gtt mapping unconditionally
> Separate out pin/unpin functions and drop usage of pin/unpin
> Use uc_fw init/fini functions to bind/unbind fw object
>
> v3: Bind is only needed during xfer (Chris)
> Remove attempts to bind outside of xfer (Chris)
> Mark fw bind/unbind static
>
> Signed-off-by: Fernando Pacheco
> ---
> drivers/gpu/drm/i915/intel_guc.c| 9 ++-
> drivers/gpu/drm/i915/intel_guc_fw.c | 18 ++---
> drivers/gpu/drm/i915/intel_huc.c| 74 +++-
> drivers/gpu/drm/i915/intel_huc.h| 4 ++
> drivers/gpu/drm/i915/intel_huc_fw.c | 47 +
> drivers/gpu/drm/i915/intel_uc.c | 23 +--
> drivers/gpu/drm/i915/intel_uc_fw.c | 103
> drivers/gpu/drm/i915/intel_uc_fw.h | 7 +-
> 8 files changed, 212 insertions(+), 73 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_guc.c
> b/drivers/gpu/drm/i915/intel_guc.c
> index 299b6aa4fe28..3bbf45a3bf78 100644
> --- a/drivers/gpu/drm/i915/intel_guc.c
> +++ b/drivers/gpu/drm/i915/intel_guc.c
> @@ -189,9 +189,13 @@ int intel_guc_init(struct intel_guc *guc)
> struct drm_i915_private *dev_priv = guc_to_i915(guc);
> int ret;
>
> - ret = guc_shared_data_create(guc);
> + ret = intel_uc_fw_init(>fw);
> if (ret)
> goto err_fetch;
> +
> + ret = guc_shared_data_create(guc);
> + if (ret)
> + goto err_fw;
> GEM_BUG_ON(!guc->shared_data);
>
> ret = intel_guc_log_create(>log);
> @@ -220,6 +224,8 @@ int intel_guc_init(struct intel_guc *guc)
> intel_guc_log_destroy(>log);
> err_shared:
> guc_shared_data_destroy(guc);
> +err_fw:
> + intel_uc_fw_fini(>fw);
> err_fetch:
> intel_uc_fw_cleanup_fetch(>fw);
> return ret;
> @@ -237,6 +243,7 @@ void intel_guc_fini(struct intel_guc *guc)
> intel_guc_ads_destroy(guc);
> intel_guc_log_destroy(>log);
> guc_shared_data_destroy(guc);
> + intel_uc_fw_fini(>fw);
> intel_uc_fw_cleanup_fetch(>fw);
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c
> b/drivers/gpu/drm/i915/intel_guc_fw.c
> index 4385d9ef02bb..8b2dcc70b956 100644
> --- a/drivers/gpu/drm/i915/intel_guc_fw.c
> +++ b/drivers/gpu/drm/i915/intel_guc_fw.c
> @@ -122,14 +122,16 @@ static void guc_prepare_xfer(struct intel_guc *guc)
> }
>
> /* Copy RSA signature from the fw image to HW for verification */
> -static void guc_xfer_rsa(struct intel_guc *guc, struct i915_vma *vma)
> +static void guc_xfer_rsa(struct intel_guc *guc)
> {
> struct drm_i915_private *dev_priv = guc_to_i915(guc);
> + struct intel_uc_fw *fw = >fw;
> + struct sg_table *pages = fw->obj->mm.pages;
> u32 rsa[UOS_RSA_SCRATCH_COUNT];
> int i;
>
> - sg_pcopy_to_buffer(vma->pages->sgl, vma->pages->nents,
> - rsa, sizeof(rsa), guc->fw.rsa_offset);
> + sg_pcopy_to_buffer(pages->sgl, pages->nents,
> + rsa, sizeof(rsa), fw->rsa_offset);
>
> for (i = 0; i < UOS_RSA_SCRATCH_COUNT; i++)
> I915_WRITE(UOS_RSA_SCRATCH(i), rsa[i]);
> @@ -201,7 +203,7 @@ static int guc_wait_ucode(struct intel_guc *guc)
> * transfer between GTT locations. This functionality is left out of the API
> * for now as there is no need for it.
> */
> -static int guc_xfer_ucode(struct intel_guc *guc, struct i915_vma *vma)
> +static int guc_xfer_ucode(struct intel_guc *guc)
> {
> struct drm_i915_private *dev_priv = guc_to_i915(guc);
> struct intel_uc_fw *guc_fw = >fw;
> @@ -214,7 +216,7 @@ static int guc_xfer_ucode(struct intel_guc *guc, struct
> i915_vma *vma)
> I915_WRITE(DMA_COPY_SIZE, guc_fw->header_size + guc_fw->ucode_size);
>
> /* Set the source address for the new blob */
> - offset = intel_guc_ggtt_offset(guc, vma) + guc_fw->header_offset;
> + offset = intel_uc_fw_ggtt_offset(guc_fw) + guc_fw->header_offset;
> I915_WRITE(DMA_ADDR_0_LOW, lower_32_bits(offset));
> I915_WRITE(DMA_ADDR_0_HIGH, upper_32_bits(offset) & 0x);
>
> @@ -233,7 +235,7 @@ static int guc_xfer_ucode(struct intel_guc *guc, struct
> i915_vma *vma)
> /*
> * Load the GuC firmware blob into the MinuteIA.
> */
> -static int guc_fw_xfer(struct intel_uc_fw *guc_fw, struct i915_vma *vma)
> +static int guc_fw_xfer(struct intel_uc_fw *guc_fw)
> {
> struct intel_guc *guc = container_of(guc_fw, struct intel_guc, fw);
[Intel-gfx] [PATCH v3 3/5] drm/i915/uc: Place uC firmware in upper range of GGTT
Currently we pin the GuC or HuC firmware image just
before uploading. Perma-pin during uC initialization
instead and use the range reserved at the top of the
address space.
Moving the firmware resulted in needing to:
- use an additional pinning for the rsa signature which will
be used during HuC auth as addresses above GUC_GGTT_TOP
do not map through GTT.
v2: Remove call to set to gtt domain
Do not restore fw gtt mapping unconditionally
Separate out pin/unpin functions and drop usage of pin/unpin
Use uc_fw init/fini functions to bind/unbind fw object
v3: Bind is only needed during xfer (Chris)
Remove attempts to bind outside of xfer (Chris)
Mark fw bind/unbind static
Signed-off-by: Fernando Pacheco
---
drivers/gpu/drm/i915/intel_guc.c| 9 ++-
drivers/gpu/drm/i915/intel_guc_fw.c | 18 ++---
drivers/gpu/drm/i915/intel_huc.c| 74 +++-
drivers/gpu/drm/i915/intel_huc.h| 4 ++
drivers/gpu/drm/i915/intel_huc_fw.c | 47 +
drivers/gpu/drm/i915/intel_uc.c | 23 +--
drivers/gpu/drm/i915/intel_uc_fw.c | 103
drivers/gpu/drm/i915/intel_uc_fw.h | 7 +-
8 files changed, 212 insertions(+), 73 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index 299b6aa4fe28..3bbf45a3bf78 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -189,9 +189,13 @@ int intel_guc_init(struct intel_guc *guc)
struct drm_i915_private *dev_priv = guc_to_i915(guc);
int ret;
- ret = guc_shared_data_create(guc);
+ ret = intel_uc_fw_init(>fw);
if (ret)
goto err_fetch;
+
+ ret = guc_shared_data_create(guc);
+ if (ret)
+ goto err_fw;
GEM_BUG_ON(!guc->shared_data);
ret = intel_guc_log_create(>log);
@@ -220,6 +224,8 @@ int intel_guc_init(struct intel_guc *guc)
intel_guc_log_destroy(>log);
err_shared:
guc_shared_data_destroy(guc);
+err_fw:
+ intel_uc_fw_fini(>fw);
err_fetch:
intel_uc_fw_cleanup_fetch(>fw);
return ret;
@@ -237,6 +243,7 @@ void intel_guc_fini(struct intel_guc *guc)
intel_guc_ads_destroy(guc);
intel_guc_log_destroy(>log);
guc_shared_data_destroy(guc);
+ intel_uc_fw_fini(>fw);
intel_uc_fw_cleanup_fetch(>fw);
}
diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c
b/drivers/gpu/drm/i915/intel_guc_fw.c
index 4385d9ef02bb..8b2dcc70b956 100644
--- a/drivers/gpu/drm/i915/intel_guc_fw.c
+++ b/drivers/gpu/drm/i915/intel_guc_fw.c
@@ -122,14 +122,16 @@ static void guc_prepare_xfer(struct intel_guc *guc)
}
/* Copy RSA signature from the fw image to HW for verification */
-static void guc_xfer_rsa(struct intel_guc *guc, struct i915_vma *vma)
+static void guc_xfer_rsa(struct intel_guc *guc)
{
struct drm_i915_private *dev_priv = guc_to_i915(guc);
+ struct intel_uc_fw *fw = >fw;
+ struct sg_table *pages = fw->obj->mm.pages;
u32 rsa[UOS_RSA_SCRATCH_COUNT];
int i;
- sg_pcopy_to_buffer(vma->pages->sgl, vma->pages->nents,
- rsa, sizeof(rsa), guc->fw.rsa_offset);
+ sg_pcopy_to_buffer(pages->sgl, pages->nents,
+ rsa, sizeof(rsa), fw->rsa_offset);
for (i = 0; i < UOS_RSA_SCRATCH_COUNT; i++)
I915_WRITE(UOS_RSA_SCRATCH(i), rsa[i]);
@@ -201,7 +203,7 @@ static int guc_wait_ucode(struct intel_guc *guc)
* transfer between GTT locations. This functionality is left out of the API
* for now as there is no need for it.
*/
-static int guc_xfer_ucode(struct intel_guc *guc, struct i915_vma *vma)
+static int guc_xfer_ucode(struct intel_guc *guc)
{
struct drm_i915_private *dev_priv = guc_to_i915(guc);
struct intel_uc_fw *guc_fw = >fw;
@@ -214,7 +216,7 @@ static int guc_xfer_ucode(struct intel_guc *guc, struct
i915_vma *vma)
I915_WRITE(DMA_COPY_SIZE, guc_fw->header_size + guc_fw->ucode_size);
/* Set the source address for the new blob */
- offset = intel_guc_ggtt_offset(guc, vma) + guc_fw->header_offset;
+ offset = intel_uc_fw_ggtt_offset(guc_fw) + guc_fw->header_offset;
I915_WRITE(DMA_ADDR_0_LOW, lower_32_bits(offset));
I915_WRITE(DMA_ADDR_0_HIGH, upper_32_bits(offset) & 0x);
@@ -233,7 +235,7 @@ static int guc_xfer_ucode(struct intel_guc *guc, struct
i915_vma *vma)
/*
* Load the GuC firmware blob into the MinuteIA.
*/
-static int guc_fw_xfer(struct intel_uc_fw *guc_fw, struct i915_vma *vma)
+static int guc_fw_xfer(struct intel_uc_fw *guc_fw)
{
struct intel_guc *guc = container_of(guc_fw, struct intel_guc, fw);
struct drm_i915_private *dev_priv = guc_to_i915(guc);
@@ -250,9 +252,9 @@ static int guc_fw_xfer(struct intel_uc_fw *guc_fw, struct
i915_vma *vma)
* by the DMA engine in one operation, whereas the RSA signature is
* loaded via MMIO.
*/
-
