On Thu, Feb 11, 2021 at 06:50:28PM +0530, Akhil P Oommen wrote:
> On 2/10/2021 6:22 AM, Jordan Crouse wrote:
> >Most a6xx targets have security issues that were fixed with new versions
> >of the microcode(s). Make sure that we are booting with a safe version of
> >the microcode for the target and print a message and error if not.
> >
> >v2: Add more informative error messages and fix typos
> >
> >Signed-off-by: Jordan Crouse <jcro...@codeaurora.org>
> >---
> >
> > drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 77 ++++++++++++++++++++++-----
> > 1 file changed, 64 insertions(+), 13 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> >b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> >index ba8e9d3cf0fe..064b7face504 100644
> >--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> >+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> >@@ -522,28 +522,73 @@ static int a6xx_cp_init(struct msm_gpu *gpu)
> > return a6xx_idle(gpu, ring) ? 0 : -EINVAL;
> > }
> >-static void a6xx_ucode_check_version(struct a6xx_gpu *a6xx_gpu,
> >+/*
> >+ * Check that the microcode version is new enough to include several key
> >+ * security fixes. Return true if the ucode is safe.
> >+ */
> >+static bool a6xx_ucode_check_version(struct a6xx_gpu *a6xx_gpu,
> > struct drm_gem_object *obj)
> > {
> >+ struct adreno_gpu *adreno_gpu = &a6xx_gpu->base;
> >+ struct msm_gpu *gpu = &adreno_gpu->base;
> > u32 *buf = msm_gem_get_vaddr(obj);
> >+ bool ret = false;
> > if (IS_ERR(buf))
> >- return;
> >+ return false;
> > /*
> >- * If the lowest nibble is 0xa that is an indication that this microcode
> >- * has been patched. The actual version is in dword [3] but we only care
> >- * about the patchlevel which is the lowest nibble of dword [3]
> >- *
> >- * Otherwise check that the firmware is greater than or equal to 1.90
> >- * which was the first version that had this fix built in
> >+ * Targets up to a640 (a618, a630 and a640) need to check for a
> >+ * microcode version that is patched to support the whereami opcode or
> >+ * one that is new enough to include it by default.
> > */
> >- if (((buf[0] & 0xf) == 0xa) && (buf[2] & 0xf) >= 1)
> >- a6xx_gpu->has_whereami = true;
> >- else if ((buf[0] & 0xfff) > 0x190)
> >- a6xx_gpu->has_whereami = true;
> >+ if (adreno_is_a618(adreno_gpu) || adreno_is_a630(adreno_gpu) ||
> >+ adreno_is_a640(adreno_gpu)) {
> >+ /*
> >+ * If the lowest nibble is 0xa that is an indication that this
> >+ * microcode has been patched. The actual version is in dword
> >+ * [3] but we only care about the patchlevel which is the lowest
> >+ * nibble of dword [3]
> >+ *
> >+ * Otherwise check that the firmware is greater than or equal
> >+ * to 1.90 which was the first version that had this fix built
> >+ * in
> >+ */
> >+ if ((((buf[0] & 0xf) == 0xa) && (buf[2] & 0xf) >= 1) ||
> >+ (buf[0] & 0xfff) >= 0x190) {
> >+ a6xx_gpu->has_whereami = true;
> >+ ret = true;
> >+ goto out;
> >+ }
> >+ DRM_DEV_ERROR(&gpu->pdev->dev,
> >+ "a630 SQE ucode is too old. Have version %x need at
> >least %x\n",
> >+ buf[0] & 0xfff, 0x190);
> >+ } else {
> >+ /*
> >+ * a650 tier targets don't need whereami but still need to be
> >+ * equal to or newer than 1.95 for other security fixes
> >+ */
> >+ if (adreno_is_a650(adreno_gpu)) {
> >+ if ((buf[0] & 0xfff) >= 0x195) {
> >+ ret = true;
> >+ goto out;
> >+ }
> >+
> >+ DRM_DEV_ERROR(&gpu->pdev->dev,
> >+ "a650 SQE ucode is too old. Have version %x
> >need at least %x\n",
> >+ buf[0] & 0xfff, 0x195);
> >+ }
> >+
> >+ /*
> >+ * When a660 is added those targets should return true here
> >+ * since those have all the critical security fixes built in
> >+ * from the start
> >+ */
> Or we can just initialize 'ret' as true.
I thought about it and I think I want to force an accept list here instead of
letting new targets get by with an implicit pass.
Jordan
> -Akhil
> >+ }
> >+out:
> > msm_gem_put_vaddr(obj);
> >+ return ret;
> > }
> > static int a6xx_ucode_init(struct msm_gpu *gpu)
> >@@ -566,7 +611,13 @@ static int a6xx_ucode_init(struct msm_gpu *gpu)
> > }
> > msm_gem_object_set_name(a6xx_gpu->sqe_bo, "sqefw");
> >- a6xx_ucode_check_version(a6xx_gpu, a6xx_gpu->sqe_bo);
> >+ if (!a6xx_ucode_check_version(a6xx_gpu, a6xx_gpu->sqe_bo)) {
> >+ msm_gem_unpin_iova(a6xx_gpu->sqe_bo, gpu->aspace);
> >+ drm_gem_object_put(a6xx_gpu->sqe_bo);
> >+
> >+ a6xx_gpu->sqe_bo = NULL;
> >+ return -EPERM;
> >+ }
> > }
> > gpu_write64(gpu, REG_A6XX_CP_SQE_INSTR_BASE_LO,
> >
>
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
