From: Jordan Crouse <jcrouse@codeaurora.org>
To: Akhil P Oommen <akhilpo@codeaurora.org>
Cc: linux-arm-msm@vger.kernel.org, Daniel Vetter <daniel@ffwll.ch>,
David Airlie <airlied@linux.ie>, Eric Anholt <eric@anholt.net>,
Jonathan Marek <jonathan@marek.ca>,
Rob Clark <robdclark@gmail.com>,
Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>,
Sean Paul <sean@poorly.run>,
Sharat Masetty <smasetty@codeaurora.org>,
dri-devel@lists.freedesktop.org, freedreno@lists.freedesktop.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] drm/msm: a6xx: Make sure the SQE microcode is safe
Date: Thu, 11 Feb 2021 09:02:37 -0700 [thread overview]
Message-ID: <20210211160237.GB26503@jcrouse1-lnx.qualcomm.com> (raw)
In-Reply-To: <8aa916f9-238a-779c-bcaf-51bfb2b761d2@codeaurora.org>
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 <jcrouse@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
next prev parent reply other threads:[~2021-02-11 17:13 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-10 0:52 [PATCH v2] drm/msm: a6xx: Make sure the SQE microcode is safe Jordan Crouse
2021-02-11 13:20 ` Akhil P Oommen
2021-02-11 16:02 ` Jordan Crouse [this message]
2021-02-12 7:37 ` Akhil P Oommen
2021-03-31 13:40 ` Dmitry Baryshkov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210211160237.GB26503@jcrouse1-lnx.qualcomm.com \
--to=jcrouse@codeaurora.org \
--cc=airlied@linux.ie \
--cc=akhilpo@codeaurora.org \
--cc=daniel@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=eric@anholt.net \
--cc=freedreno@lists.freedesktop.org \
--cc=jonathan@marek.ca \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=robdclark@gmail.com \
--cc=saiprakash.ranjan@codeaurora.org \
--cc=sean@poorly.run \
--cc=smasetty@codeaurora.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).