On Wed, Dec 08, 2021 at 04:56:10PM -0800, Daniele Ceraolo Spurio wrote:
> Some of the newer HW will use bigger RSA keys to authenticate the GuC
> binary. On those platforms the HW will read the key from memory instead
> of the RSA registers, so we need to copy it in a dedicated vma, like we
> do for the HuC. The address of the key is provided to the HW via the
> first RSA register.
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
> Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
> Cc: John Harrison <John.C.Harrison@xxxxxxxxx>
> Cc: Matthew Brost <matthew.brost@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c | 30 ++++++--
> drivers/gpu/drm/i915/gt/uc/intel_huc.c | 73 +------------------
> drivers/gpu/drm/i915/gt/uc/intel_huc.h | 2 -
> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 86 ++++++++++++++++++++++-
> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h | 1 +
> 5 files changed, 113 insertions(+), 79 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
> index 796483a41353..811f032199eb 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
> @@ -40,9 +40,8 @@ static void guc_prepare_xfer(struct intel_uncore *uncore)
> }
> }
>
> -/* Copy RSA signature from the fw image to HW for verification */
> -static int guc_xfer_rsa(struct intel_uc_fw *guc_fw,
> - struct intel_uncore *uncore)
> +static int guc_xfer_rsa_mmio(struct intel_uc_fw *guc_fw,
> + struct intel_uncore *uncore)
> {
> u32 rsa[UOS_RSA_SCRATCH_COUNT];
> size_t copied;
> @@ -58,6 +57,27 @@ static int guc_xfer_rsa(struct intel_uc_fw *guc_fw,
> return 0;
> }
>
> +static int guc_xfer_rsa_vma(struct intel_uc_fw *guc_fw,
> + struct intel_uncore *uncore)
> +{
> + struct intel_guc *guc = container_of(guc_fw, struct intel_guc, fw);
> +
> + intel_uncore_write(uncore, UOS_RSA_SCRATCH(0),
> + intel_guc_ggtt_offset(guc, guc_fw->rsa_data));
> +
> + return 0;
> +}
> +
> +/* Copy RSA signature from the fw image to HW for verification */
> +static int guc_xfer_rsa(struct intel_uc_fw *guc_fw,
> + struct intel_uncore *uncore)
> +{
> + if (guc_fw->rsa_data)
> + return guc_xfer_rsa_vma(guc_fw, uncore);
> + else
> + return guc_xfer_rsa_mmio(guc_fw, uncore);
> +}
> +
> /*
> * Read the GuC status register (GUC_STATUS) and store it in the
> * specified location; then return a boolean indicating whether
> @@ -142,7 +162,9 @@ int intel_guc_fw_upload(struct intel_guc *guc)
> /*
> * 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.
> + * loaded separately, either by copying it to the UOS_RSA_SCRATCH
> + * register (if key size <= 256) or through a ggtt-pinned vma (if key
> + * size > 256).
Maybe add comment saying whether the key is in MMIO or memory is hard
coded in the bootrom.
With an extra comment:
Reviewed-by: Matthew Brost <matthew.brost@xxxxxxxxx>
> */
> ret = guc_xfer_rsa(&guc->fw, uncore);
> if (ret)
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> index c10736dddfb4..d10b227ac4aa 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> @@ -54,65 +54,6 @@ void intel_huc_init_early(struct intel_huc *huc)
> }
> }
>
> -static int intel_huc_rsa_data_create(struct intel_huc *huc)
> -{
> - struct intel_gt *gt = huc_to_gt(huc);
> - struct intel_guc *guc = >->uc.guc;
> - struct i915_vma *vma;
> - size_t copied;
> - void *vaddr;
> - int err;
> -
> - err = i915_inject_probe_error(gt->i915, -ENXIO);
> - if (err)
> - return err;
> -
> - /*
> - * HuC firmware will sit above GUC_GGTT_TOP and will not map
> - * through GTT. Unfortunately, this means GuC cannot perform
> - * the HuC auth. as the rsa offset now falls within the GuC
> - * inaccessible range. We resort to perma-pinning an additional
> - * vma within the accessible range that only contains the rsa
> - * signature. The GuC can use this extra pinning to perform
> - * the authentication since its GGTT offset will be GuC
> - * accessible.
> - */
> - GEM_BUG_ON(huc->fw.rsa_size > PAGE_SIZE);
> - vma = intel_guc_allocate_vma(guc, PAGE_SIZE);
> - if (IS_ERR(vma))
> - return PTR_ERR(vma);
> -
> - vaddr = i915_gem_object_pin_map_unlocked(vma->obj,
> - i915_coherent_map_type(gt->i915,
> - vma->obj, true));
> - if (IS_ERR(vaddr)) {
> - i915_vma_unpin_and_release(&vma, 0);
> - err = PTR_ERR(vaddr);
> - goto unpin_out;
> - }
> -
> - copied = intel_uc_fw_copy_rsa(&huc->fw, vaddr, vma->size);
> - i915_gem_object_unpin_map(vma->obj);
> -
> - if (copied < huc->fw.rsa_size) {
> - err = -ENOMEM;
> - goto unpin_out;
> - }
> -
> - huc->rsa_data = vma;
> -
> - return 0;
> -
> -unpin_out:
> - i915_vma_unpin_and_release(&vma, 0);
> - return err;
> -}
> -
> -static void intel_huc_rsa_data_destroy(struct intel_huc *huc)
> -{
> - i915_vma_unpin_and_release(&huc->rsa_data, 0);
> -}
> -
> int intel_huc_init(struct intel_huc *huc)
> {
> struct drm_i915_private *i915 = huc_to_gt(huc)->i915;
> @@ -122,21 +63,10 @@ int intel_huc_init(struct intel_huc *huc)
> if (err)
> goto out;
>
> - /*
> - * HuC firmware image is outside GuC accessible range.
> - * Copy the RSA signature out of the image into
> - * a perma-pinned region set aside for it
> - */
> - err = intel_huc_rsa_data_create(huc);
> - if (err)
> - goto out_fini;
> -
> intel_uc_fw_change_status(&huc->fw, INTEL_UC_FIRMWARE_LOADABLE);
>
> return 0;
>
> -out_fini:
> - intel_uc_fw_fini(&huc->fw);
> out:
> i915_probe_error(i915, "failed with %d\n", err);
> return err;
> @@ -147,7 +77,6 @@ void intel_huc_fini(struct intel_huc *huc)
> if (!intel_uc_fw_is_loadable(&huc->fw))
> return;
>
> - intel_huc_rsa_data_destroy(huc);
> intel_uc_fw_fini(&huc->fw);
> }
>
> @@ -177,7 +106,7 @@ int intel_huc_auth(struct intel_huc *huc)
> goto fail;
>
> ret = intel_guc_auth_huc(guc,
> - intel_guc_ggtt_offset(guc, huc->rsa_data));
> + intel_guc_ggtt_offset(guc, huc->fw.rsa_data));
> if (ret) {
> DRM_ERROR("HuC: GuC did not ack Auth request %d\n", ret);
> goto fail;
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.h b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
> index daee43b661d4..ae8c8a6c8cc8 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
> @@ -15,8 +15,6 @@ struct intel_huc {
> struct intel_uc_fw fw;
>
> /* HuC-specific additions */
> - struct i915_vma *rsa_data;
> -
> struct {
> i915_reg_t reg;
> u32 mask;
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> index 01683f5f95bd..ee1254f3f30b 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> @@ -537,6 +537,75 @@ int intel_uc_fw_upload(struct intel_uc_fw *uc_fw, u32 dst_offset, u32 dma_flags)
> return err;
> }
>
> +static inline bool uc_fw_need_rsa_in_memory(struct intel_uc_fw *uc_fw)
> +{
> + /*
> + * The HW reads the GuC RSA from memory if the key size is > 256 bytes,
> + * while it reads it from the 64 RSA registers if it is smaller.
> + * The HuC RSA is always read from memory.
> + */
> + return uc_fw->type == INTEL_UC_FW_TYPE_HUC || uc_fw->rsa_size > 256;
> +}
> +
> +static int uc_fw_rsa_data_create(struct intel_uc_fw *uc_fw)
> +{
> + struct intel_gt *gt = __uc_fw_to_gt(uc_fw);
> + struct i915_vma *vma;
> + size_t copied;
> + void *vaddr;
> + int err;
> +
> + err = i915_inject_probe_error(gt->i915, -ENXIO);
> + if (err)
> + return err;
> +
> + if (!uc_fw_need_rsa_in_memory(uc_fw))
> + return 0;
> +
> + /*
> + * uC firmwares will sit above GUC_GGTT_TOP and will not map through
> + * GGTT. Unfortunately, this means that the GuC HW cannot perform the uC
> + * authentication from memory, as the RSA offset now falls within the
> + * GuC inaccessible range. We resort to perma-pinning an additional vma
> + * within the accessible range that only contains the RSA signature.
> + * The GuC HW can use this extra pinning to perform the authentication
> + * since its GGTT offset will be GuC accessible.
> + */
> + GEM_BUG_ON(uc_fw->rsa_size > PAGE_SIZE);
> + vma = intel_guc_allocate_vma(>->uc.guc, PAGE_SIZE);
> + if (IS_ERR(vma))
> + return PTR_ERR(vma);
> +
> + vaddr = i915_gem_object_pin_map_unlocked(vma->obj,
> + i915_coherent_map_type(gt->i915, vma->obj, true));
> + if (IS_ERR(vaddr)) {
> + i915_vma_unpin_and_release(&vma, 0);
> + err = PTR_ERR(vaddr);
> + goto unpin_out;
> + }
> +
> + copied = intel_uc_fw_copy_rsa(uc_fw, vaddr, vma->size);
> + i915_gem_object_unpin_map(vma->obj);
> +
> + if (copied < uc_fw->rsa_size) {
> + err = -ENOMEM;
> + goto unpin_out;
> + }
> +
> + uc_fw->rsa_data = vma;
> +
> + return 0;
> +
> +unpin_out:
> + i915_vma_unpin_and_release(&vma, 0);
> + return err;
> +}
> +
> +static void uc_fw_rsa_data_destroy(struct intel_uc_fw *uc_fw)
> +{
> + i915_vma_unpin_and_release(&uc_fw->rsa_data, 0);
> +}
> +
> int intel_uc_fw_init(struct intel_uc_fw *uc_fw)
> {
> int err;
> @@ -551,14 +620,29 @@ int intel_uc_fw_init(struct intel_uc_fw *uc_fw)
> if (err) {
> DRM_DEBUG_DRIVER("%s fw pin-pages err=%d\n",
> intel_uc_fw_type_repr(uc_fw->type), err);
> - intel_uc_fw_change_status(uc_fw, INTEL_UC_FIRMWARE_INIT_FAIL);
> + goto out;
> }
>
> + err = uc_fw_rsa_data_create(uc_fw);
> + if (err) {
> + DRM_DEBUG_DRIVER("%s fw rsa data creation failed, err=%d\n",
> + intel_uc_fw_type_repr(uc_fw->type), err);
> + goto out_unpin;
> + }
> +
> + return 0;
> +
> +out_unpin:
> + i915_gem_object_unpin_pages(uc_fw->obj);
> +out:
> + intel_uc_fw_change_status(uc_fw, INTEL_UC_FIRMWARE_INIT_FAIL);
> return err;
> }
>
> void intel_uc_fw_fini(struct intel_uc_fw *uc_fw)
> {
> + uc_fw_rsa_data_destroy(uc_fw);
> +
> if (i915_gem_object_has_pinned_pages(uc_fw->obj))
> i915_gem_object_unpin_pages(uc_fw->obj);
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
> index fd17abf2ab02..d9d1dc0b4cbb 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
> @@ -86,6 +86,7 @@ struct intel_uc_fw {
> * or during a GT reset (mutex guarantees single threaded).
> */
> struct i915_vma dummy;
> + struct i915_vma *rsa_data;
>
> /*
> * The firmware build process will generate a version header file with major and
> --
> 2.25.1
>
