Section D6.7 of the ARM ARM states:

For the purpose of determining Tag Check Fault handling, unprivileged
load and store instructions are treated as if executed at EL0 when
executed at either:
- EL1, when the Effective value of PSTATE.UAO is 0.
- EL2, when both the Effective value of HCR_EL2.{E2H, TGE} is {1, 1}
  and the Effective value of PSTATE.UAO is 0.

ARM has confirmed a defect in the pseudocode function
AArch64.TagCheckFault that makes it inconsistent with the above
wording. The remedy is to adjust references to PSTATE.EL in that
function to instead refer to AArch64.AccessUsesEL(acctype), so
that unprivileged instructions use SCTLR_EL1.TCF0 and TFSRE0_EL1.
The exception type for synchronous tag check faults remains unchanged.

This patch implements the described change by partially reverting
commits 50244cc76abc and cc97b0019bb5.

Signed-off-by: Peter Collingbourne <pcc@google.com>
---
 target/arm/helper.c     |  2 +-
 target/arm/mte_helper.c | 13 +++++++++----
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 0e1a3b9421..b0223bda4c 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -13133,7 +13133,7 @@ static uint32_t rebuild_hflags_a64(CPUARMState *env, int el, int fp_el,
         if (FIELD_EX32(flags, TBFLAG_A64, UNPRIV)
             && tbid
             && !(env->pstate & PSTATE_TCO)
-            && (sctlr & SCTLR_TCF)
+            && (sctlr & SCTLR_TCF0)
             && allocation_tag_access_enabled(env, 0, sctlr)) {
             flags = FIELD_DP32(flags, TBFLAG_A64, MTE0_ACTIVE, 1);
         }
diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c
index 1c569336ea..0bbb9ec346 100644
--- a/target/arm/mte_helper.c
+++ b/target/arm/mte_helper.c
@@ -550,10 +550,14 @@ static void mte_check_fail(CPUARMState *env, uint32_t desc,
     reg_el = regime_el(env, arm_mmu_idx);
     sctlr = env->cp15.sctlr_el[reg_el];
 
-    el = arm_current_el(env);
-    if (el == 0) {
+    switch (arm_mmu_idx) {
+    case ARMMMUIdx_E10_0:
+    case ARMMMUIdx_E20_0:
+        el = 0;
         tcf = extract64(sctlr, 38, 2);
-    } else {
+        break;
+    default:
+        el = reg_el;
         tcf = extract64(sctlr, 40, 2);
     }
 
@@ -570,7 +574,8 @@ static void mte_check_fail(CPUARMState *env, uint32_t desc,
         env->exception.vaddress = dirty_ptr;
 
         is_write = FIELD_EX32(desc, MTEDESC, WRITE);
-        syn = syn_data_abort_no_iss(el != 0, 0, 0, 0, 0, is_write, 0x11);
+        syn = syn_data_abort_no_iss(arm_current_el(env) != 0, 0, 0, 0, 0,
+                                    is_write, 0x11);
         raise_exception(env, EXCP_DATA_ABORT, syn, exception_target_el(env));
         /* noreturn, but fall through to the assert anyway */
 
-- 
2.30.0.617.g56c4b15f3c-goog

Patchew URL: https://patchew.org/QEMU/20210219201820.2672077-1-pcc@google.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20210219201820.2672077-1-pcc@google.com
Subject: [PATCH] target/arm: Use TCF0 and TFSRE0 for unprivileged tag checks

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/20210219201820.2672077-1-pcc@google.com -> patchew/20210219201820.2672077-1-pcc@google.com
Switched to a new branch 'test'
8b335c2 target/arm: Use TCF0 and TFSRE0 for unprivileged tag checks

=== OUTPUT BEGIN ===
ERROR: Author email address is mangled by the mailing list
#2: 
Author: Peter Collingbourne via <qemu-devel@nongnu.org>

total: 1 errors, 0 warnings, 34 lines checked

Commit 8b335c251c00 (target/arm: Use TCF0 and TFSRE0 for unprivileged tag checks) has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20210219201820.2672077-1-pcc@google.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

On 2/19/21 12:18 PM, Peter Collingbourne wrote:
> Section D6.7 of the ARM ARM states:
> 
> For the purpose of determining Tag Check Fault handling, unprivileged
> load and store instructions are treated as if executed at EL0 when
> executed at either:
> - EL1, when the Effective value of PSTATE.UAO is 0.
> - EL2, when both the Effective value of HCR_EL2.{E2H, TGE} is {1, 1}
>   and the Effective value of PSTATE.UAO is 0.
> 
> ARM has confirmed a defect in the pseudocode function
> AArch64.TagCheckFault that makes it inconsistent with the above
> wording. The remedy is to adjust references to PSTATE.EL in that
> function to instead refer to AArch64.AccessUsesEL(acctype), so
> that unprivileged instructions use SCTLR_EL1.TCF0 and TFSRE0_EL1.
> The exception type for synchronous tag check faults remains unchanged.
> 
> This patch implements the described change by partially reverting
> commits 50244cc76abc and cc97b0019bb5.
> 
> Signed-off-by: Peter Collingbourne <pcc@google.com>
> ---
>  target/arm/helper.c     |  2 +-
>  target/arm/mte_helper.c | 13 +++++++++----
>  2 files changed, 10 insertions(+), 5 deletions(-)

Interesting.  When the the 50244cc bug was reported, I had wondered if this
were intentional.  The reversions, with the additional change to the el for the
syndrome, looks correct based on the described change to TagCheckFault.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

On Sat, 20 Feb 2021 at 16:19, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 2/19/21 12:18 PM, Peter Collingbourne wrote:
> > Section D6.7 of the ARM ARM states:
> >
> > For the purpose of determining Tag Check Fault handling, unprivileged
> > load and store instructions are treated as if executed at EL0 when
> > executed at either:
> > - EL1, when the Effective value of PSTATE.UAO is 0.
> > - EL2, when both the Effective value of HCR_EL2.{E2H, TGE} is {1, 1}
> >   and the Effective value of PSTATE.UAO is 0.
> >
> > ARM has confirmed a defect in the pseudocode function
> > AArch64.TagCheckFault that makes it inconsistent with the above
> > wording. The remedy is to adjust references to PSTATE.EL in that
> > function to instead refer to AArch64.AccessUsesEL(acctype), so
> > that unprivileged instructions use SCTLR_EL1.TCF0 and TFSRE0_EL1.
> > The exception type for synchronous tag check faults remains unchanged.
> >
> > This patch implements the described change by partially reverting
> > commits 50244cc76abc and cc97b0019bb5.
> >
> > Signed-off-by: Peter Collingbourne <pcc@google.com>
> > ---
> >  target/arm/helper.c     |  2 +-
> >  target/arm/mte_helper.c | 13 +++++++++----
> >  2 files changed, 10 insertions(+), 5 deletions(-)
>
> Interesting.  When the the 50244cc bug was reported, I had wondered if this
> were intentional.  The reversions, with the additional change to the el for the
> syndrome, looks correct based on the described change to TagCheckFault.
>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>



Applied to target-arm.next, thanks.

-- PMM