All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Richard Henderson <richard.henderson@linaro.org>
To: Peter Collingbourne <pcc@google.com>
Cc: Peter Maydell <peter.maydell@linaro.org>,
	Vincenzo Frascino <vincenzo.frascino@arm.com>,
	qemu-devel@nongnu.org, Evgenii Stepanov <eugenis@google.com>
Subject: Re: [PATCH] target/arm: Implement MTE3
Date: Sat, 12 Jun 2021 14:19:20 -0700	[thread overview]
Message-ID: <08ec844b-5d44-e2dc-407b-beec56b2f4c7@linaro.org> (raw)
In-Reply-To: <20210611190653.754648-1-pcc@google.com>

[-- Attachment #1: Type: text/plain, Size: 4573 bytes --]

On 6/11/21 12:06 PM, Peter Collingbourne wrote:
> MTE3 introduces an asymmetric tag checking mode, in which loads are
> checked synchronously and stores are checked asynchronously. Add
> support for it.
> 
> Signed-off-by: Peter Collingbourne <pcc@google.com>
> ---
>   target/arm/cpu64.c      |  2 +-
>   target/arm/mte_helper.c | 83 ++++++++++++++++++++++++++---------------
>   2 files changed, 53 insertions(+), 32 deletions(-)
> 
> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> index 1c23187d1a..c7a1626bec 100644
> --- a/target/arm/cpu64.c
> +++ b/target/arm/cpu64.c
> @@ -683,7 +683,7 @@ static void aarch64_max_initfn(Object *obj)
>            * during realize if the board provides no tag memory, much like
>            * we do for EL2 with the virtualization=on property.
>            */
> -        t = FIELD_DP64(t, ID_AA64PFR1, MTE, 2);
> +        t = FIELD_DP64(t, ID_AA64PFR1, MTE, 3);
>           cpu->isar.id_aa64pfr1 = t;
>   
>           t = cpu->isar.id_aa64mmfr0;
> diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c
> index 166b9d260f..7b76d871ff 100644
> --- a/target/arm/mte_helper.c
> +++ b/target/arm/mte_helper.c
> @@ -538,13 +538,51 @@ void HELPER(stzgm_tags)(CPUARMState *env, uint64_t ptr, uint64_t val)
>       }
>   }
>   
> +static void mte_sync_check_fail(CPUARMState *env, uint32_t desc,
> +                                uint64_t dirty_ptr, uintptr_t ra)
> +{
> +    int is_write, syn;
> +
> +    env->exception.vaddress = dirty_ptr;
> +
> +    is_write = FIELD_EX32(desc, MTEDESC, WRITE);
> +    syn = syn_data_abort_no_iss(arm_current_el(env) != 0, 0, 0, 0, 0, is_write,
> +                                0x11);
> +    raise_exception_ra(env, EXCP_DATA_ABORT, syn, exception_target_el(env), ra);
> +    g_assert_not_reached();
> +}
> +
> +static void mte_async_check_fail(CPUARMState *env, uint32_t desc,
> +                                 uint64_t dirty_ptr, uintptr_t ra,
> +                                 ARMMMUIdx arm_mmu_idx, int el)
> +{
> +    int select;
> +
> +    if (regime_has_2_ranges(arm_mmu_idx)) {
> +        select = extract64(dirty_ptr, 55, 1);
> +    } else {
> +        select = 0;
> +    }
> +    env->cp15.tfsr_el[el] |= 1 << select;
> +#ifdef CONFIG_USER_ONLY
> +    /*
> +     * Stand in for a timer irq, setting _TIF_MTE_ASYNC_FAULT,
> +     * which then sends a SIGSEGV when the thread is next scheduled.
> +     * This cpu will return to the main loop at the end of the TB,
> +     * which is rather sooner than "normal".  But the alternative
> +     * is waiting until the next syscall.
> +     */
> +    qemu_cpu_kick(env_cpu(env));
> +#endif
> +}

This is ok, though the desc parameter is unused for async.
I'm not adverse to using a goto, like so.

But either way,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

---%<
diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c
index 9e615cc513..e93603bc02 100644
--- a/target/arm/mte_helper.c
+++ b/target/arm/mte_helper.c
@@ -561,12 +561,23 @@ static void mte_check_fail(CPUARMState *env, uint32_t desc,
          tcf = extract64(sctlr, 40, 2);
      }

+    is_write = FIELD_EX32(desc, MTEDESC, WRITE);
+
      switch (tcf) {
+    default: /* case 3 */
+        /*
+         * Tag check fail causes asynchronous flag set for stores,
+         * or a synchronous exception for loads.
+         */
+        if (is_write) {
+            goto fail_async;
+        }
+        /* fall through */
+
      case 1:
          /* Tag check fail causes a synchronous exception. */
          env->exception.vaddress = dirty_ptr;

-        is_write = FIELD_EX32(desc, MTEDESC, WRITE);
          syn = syn_data_abort_no_iss(arm_current_el(env) != 0, 0, 0, 0, 0,
                                      is_write, 0x11);
          raise_exception_ra(env, EXCP_DATA_ABORT, syn,
@@ -582,6 +593,7 @@ static void mte_check_fail(CPUARMState *env, uint32_t desc,
          g_assert_not_reached();

      case 2:
+    fail_async:
          /* Tag check fail causes asynchronous flag set.  */
          if (regime_has_2_ranges(arm_mmu_idx)) {
              select = extract64(dirty_ptr, 55, 1);
@@ -600,14 +612,6 @@ static void mte_check_fail(CPUARMState *env, uint32_t desc,
          qemu_cpu_kick(env_cpu(env));
  #endif
          break;
-
-    default:
-        /* Case 3: Reserved. */
-        qemu_log_mask(LOG_GUEST_ERROR,
-                      "Tag check failure with SCTLR_EL%d.TCF%s "
-                      "set to reserved value %d\n",
-                      reg_el, el ? "" : "0", tcf);
-        break;
      }
  }

[-- Attachment #2: z --]
[-- Type: text/plain, Size: 2601 bytes --]

diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 1c23187d1a..c4afee77d7 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -679,9 +679,10 @@ static void aarch64_max_initfn(Object *obj)
         t = FIELD_DP64(t, ID_AA64PFR1, BT, 1);
         t = FIELD_DP64(t, ID_AA64PFR1, SSBS, 2);
         /*
-         * Begin with full support for MTE. This will be downgraded to MTE=0
-         * during realize if the board provides no tag memory, much like
-         * we do for EL2 with the virtualization=on property.
+         * Begin with full support for MTE (FEAT_MTE3). This will be
+         * downgraded to MTE=0 (no MTE) during realize if the board
+         * provides no tag memory, much like we do for EL2 with the
+         * virtualization=on property.
          */
         t = FIELD_DP64(t, ID_AA64PFR1, MTE, 2);
         cpu->isar.id_aa64pfr1 = t;
diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c
index 9e615cc513..e93603bc02 100644
--- a/target/arm/mte_helper.c
+++ b/target/arm/mte_helper.c
@@ -561,12 +561,23 @@ static void mte_check_fail(CPUARMState *env, uint32_t desc,
         tcf = extract64(sctlr, 40, 2);
     }
 
+    is_write = FIELD_EX32(desc, MTEDESC, WRITE);
+
     switch (tcf) {
+    default: /* case 3 */
+        /*
+         * Tag check fail causes asynchronous flag set for stores,
+         * or a synchronous exception for loads.
+         */
+        if (is_write) {
+            goto fail_async;
+        }
+        /* fall through */
+
     case 1:
         /* Tag check fail causes a synchronous exception. */
         env->exception.vaddress = dirty_ptr;
 
-        is_write = FIELD_EX32(desc, MTEDESC, WRITE);
         syn = syn_data_abort_no_iss(arm_current_el(env) != 0, 0, 0, 0, 0,
                                     is_write, 0x11);
         raise_exception_ra(env, EXCP_DATA_ABORT, syn,
@@ -582,6 +593,7 @@ static void mte_check_fail(CPUARMState *env, uint32_t desc,
         g_assert_not_reached();
 
     case 2:
+    fail_async:
         /* Tag check fail causes asynchronous flag set.  */
         if (regime_has_2_ranges(arm_mmu_idx)) {
             select = extract64(dirty_ptr, 55, 1);
@@ -600,14 +612,6 @@ static void mte_check_fail(CPUARMState *env, uint32_t desc,
         qemu_cpu_kick(env_cpu(env));
 #endif
         break;
-
-    default:
-        /* Case 3: Reserved. */
-        qemu_log_mask(LOG_GUEST_ERROR,
-                      "Tag check failure with SCTLR_EL%d.TCF%s "
-                      "set to reserved value %d\n",
-                      reg_el, el ? "" : "0", tcf);
-        break;
     }
 }
 

  reply	other threads:[~2021-06-12 21:20 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-11 19:06 [PATCH] target/arm: Implement MTE3 Peter Collingbourne
2021-06-12 21:19 ` Richard Henderson [this message]
2021-07-08 15:08 ` Richard Henderson
2021-07-08 15:23   ` Richard Henderson

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=08ec844b-5d44-e2dc-407b-beec56b2f4c7@linaro.org \
    --to=richard.henderson@linaro.org \
    --cc=eugenis@google.com \
    --cc=pcc@google.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=vincenzo.frascino@arm.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.