[06/17] target/arm: Implement the IRG instruction

Message ID 20190114011122.5995-7-richard.henderson@linaro.org
State New
Headers show
Series
  • target/arm: Implement ARMv8.5-MemTag
Related show

Commit Message

Richard Henderson Jan. 14, 2019, 1:11 a.m.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

---
 target/arm/helper-a64.h    |  1 +
 target/arm/mte_helper.c    | 55 ++++++++++++++++++++++++++++++++++++++
 target/arm/translate-a64.c |  7 +++++
 3 files changed, 63 insertions(+)

-- 
2.17.2

Comments

Peter Maydell Feb. 7, 2019, 4:47 p.m. | #1
On Mon, 14 Jan 2019 at 01:11, Richard Henderson
<richard.henderson@linaro.org> wrote:
>

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

> ---

>  target/arm/helper-a64.h    |  1 +

>  target/arm/mte_helper.c    | 55 ++++++++++++++++++++++++++++++++++++++

>  target/arm/translate-a64.c |  7 +++++

>  3 files changed, 63 insertions(+)

>

> diff --git a/target/arm/helper-a64.h b/target/arm/helper-a64.h

> index fa4c371a47..7a6051fdab 100644

> --- a/target/arm/helper-a64.h

> +++ b/target/arm/helper-a64.h

> @@ -104,3 +104,4 @@ DEF_HELPER_FLAGS_2(xpaci, TCG_CALL_NO_RWG_SE, i64, env, i64)

>  DEF_HELPER_FLAGS_2(xpacd, TCG_CALL_NO_RWG_SE, i64, env, i64)

>

>  DEF_HELPER_FLAGS_2(mte_check, TCG_CALL_NO_WG, i64, env, i64)

> +DEF_HELPER_FLAGS_3(irg, TCG_CALL_NO_RWG, i64, env, i64, i64)

> diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c

> index 6f4bc0aa04..1878393fc4 100644

> --- a/target/arm/mte_helper.c

> +++ b/target/arm/mte_helper.c

> @@ -36,6 +36,48 @@ static int allocation_tag_from_addr(uint64_t ptr)

>      return (extract64(ptr, 56, 4) + extract64(ptr, 55, 1)) & 15;

>  }

>

> +/* Like ChooseNonExcludedTag, except that GCR_EL1 is already in.  */


I don't understand this comment -- neither the pseudocode
function nor this code refer to GCR_EL1.

> +static int choose_nonexcluded_tag(int tag, int offset, uint16_t exclude)

> +{

> +    if (exclude != 0xffff) {

> +        int i;

> +        for (i = 0; i < offset; ++i) {

> +            do {

> +                tag = (tag + 1) & 15;

> +            } while (exclude & (1 << tag));

> +        }

> +    }

> +    return tag;


This doesn't seem to do the same thing as ChooseNonExcludedTag()
for the offset==0 case, or for the exclude == 0xffff case.

> +}

> +

> +static int choose_random_nonexcluded_tag(CPUARMState *env, uint16_t exclude)

> +{

> +    /* Ignore GCR_EL1.RRND.  Always produce deterministic results.  */


This comment is trying to say that our IMPDEF choice for GCR_EL1.RRND==1
is "behave the same as if RRND==0", right? I think that would be
clearer as a comment at the callsite, because if you're following
along with the pseudocode you expect the IRG code to do an
"if RRND == 1 then { choose_random_nonexcluded_tag(); } else { ... }".

thanks
-- PMM
Richard Henderson Feb. 10, 2019, 3:43 a.m. | #2
On 2/7/19 8:47 AM, Peter Maydell wrote:
> On Mon, 14 Jan 2019 at 01:11, Richard Henderson

> <richard.henderson@linaro.org> wrote:

>>

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

>> ---

>>  target/arm/helper-a64.h    |  1 +

>>  target/arm/mte_helper.c    | 55 ++++++++++++++++++++++++++++++++++++++

>>  target/arm/translate-a64.c |  7 +++++

>>  3 files changed, 63 insertions(+)

>>

>> diff --git a/target/arm/helper-a64.h b/target/arm/helper-a64.h

>> index fa4c371a47..7a6051fdab 100644

>> --- a/target/arm/helper-a64.h

>> +++ b/target/arm/helper-a64.h

>> @@ -104,3 +104,4 @@ DEF_HELPER_FLAGS_2(xpaci, TCG_CALL_NO_RWG_SE, i64, env, i64)

>>  DEF_HELPER_FLAGS_2(xpacd, TCG_CALL_NO_RWG_SE, i64, env, i64)

>>

>>  DEF_HELPER_FLAGS_2(mte_check, TCG_CALL_NO_WG, i64, env, i64)

>> +DEF_HELPER_FLAGS_3(irg, TCG_CALL_NO_RWG, i64, env, i64, i64)

>> diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c

>> index 6f4bc0aa04..1878393fc4 100644

>> --- a/target/arm/mte_helper.c

>> +++ b/target/arm/mte_helper.c

>> @@ -36,6 +36,48 @@ static int allocation_tag_from_addr(uint64_t ptr)

>>      return (extract64(ptr, 56, 4) + extract64(ptr, 55, 1)) & 15;

>>  }

>>

>> +/* Like ChooseNonExcludedTag, except that GCR_EL1 is already in.  */

> 

> I don't understand this comment -- neither the pseudocode

> function nor this code refer to GCR_EL1.


That's changed since the first rev.  Now ChooseNonExcludedTag just has an
exclude argument, and the caller includes GCR_EL1.Exclude.

>> +static int choose_nonexcluded_tag(int tag, int offset, uint16_t exclude)

>> +{

>> +    if (exclude != 0xffff) {

>> +        int i;

>> +        for (i = 0; i < offset; ++i) {

>> +            do {

>> +                tag = (tag + 1) & 15;

>> +            } while (exclude & (1 << tag));

>> +        }

>> +    }

>> +    return tag;

> 

> This doesn't seem to do the same thing as ChooseNonExcludedTag()

> for the offset==0 case, or for the exclude == 0xffff case.


All of this has changed since the first rev too.

> This comment is trying to say that our IMPDEF choice for GCR_EL1.RRND==1

> is "behave the same as if RRND==0", right? I think that would be

> clearer as a comment at the callsite, because if you're following

> along with the pseudocode you expect the IRG code to do an

> "if RRND == 1 then { choose_random_nonexcluded_tag(); } else { ... }".


Thanks for the verbage.


r~

Patch

diff --git a/target/arm/helper-a64.h b/target/arm/helper-a64.h
index fa4c371a47..7a6051fdab 100644
--- a/target/arm/helper-a64.h
+++ b/target/arm/helper-a64.h
@@ -104,3 +104,4 @@  DEF_HELPER_FLAGS_2(xpaci, TCG_CALL_NO_RWG_SE, i64, env, i64)
 DEF_HELPER_FLAGS_2(xpacd, TCG_CALL_NO_RWG_SE, i64, env, i64)
 
 DEF_HELPER_FLAGS_2(mte_check, TCG_CALL_NO_WG, i64, env, i64)
+DEF_HELPER_FLAGS_3(irg, TCG_CALL_NO_RWG, i64, env, i64, i64)
diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c
index 6f4bc0aa04..1878393fc4 100644
--- a/target/arm/mte_helper.c
+++ b/target/arm/mte_helper.c
@@ -36,6 +36,48 @@  static int allocation_tag_from_addr(uint64_t ptr)
     return (extract64(ptr, 56, 4) + extract64(ptr, 55, 1)) & 15;
 }
 
+/* Like ChooseNonExcludedTag, except that GCR_EL1 is already in.  */
+static int choose_nonexcluded_tag(int tag, int offset, uint16_t exclude)
+{
+    if (exclude != 0xffff) {
+        int i;
+        for (i = 0; i < offset; ++i) {
+            do {
+                tag = (tag + 1) & 15;
+            } while (exclude & (1 << tag));
+        }
+    }
+    return tag;
+}
+
+static int choose_random_nonexcluded_tag(CPUARMState *env, uint16_t exclude)
+{
+    /* Ignore GCR_EL1.RRND.  Always produce deterministic results.  */
+    int start = extract32(env->cp15.rgsr_el1, 0, 4);
+    int seed = extract32(env->cp15.rgsr_el1, 8, 16);
+    int offset, rtag, i;
+
+    /* RandomTag */
+    for (i = offset = 0; i < 4; ++i) {
+        /* NextRandomTagBit */
+        int top = (extract32(seed, 5, 1) ^ extract32(seed, 3, 1) ^
+                   extract32(seed, 2, 1) ^ extract32(seed, 0, 1));
+        seed = (top << 15) | (seed >> 1);
+        offset |= top << i;
+    }
+    rtag = choose_nonexcluded_tag(start, offset, exclude);
+
+    env->cp15.rgsr_el1 = rtag | (seed << 8);
+
+    return rtag;
+}
+
+static uint64_t address_with_allocation_tag(uint64_t ptr, int rtag)
+{
+    rtag -= extract64(ptr, 55, 1);
+    return deposit64(ptr, 56, 4, rtag);
+}
+
 uint64_t HELPER(mte_check)(CPUARMState *env, uint64_t ptr)
 {
     ARMMMUIdx mmu_idx = arm_stage1_mmu_idx(env);
@@ -108,3 +150,16 @@  uint64_t HELPER(mte_check)(CPUARMState *env, uint64_t ptr)
     /* Unchecked, or tag check pass.  Ignore the top byte.  */
     return sextract64(ptr, 0, 55);
 }
+
+uint64_t HELPER(irg)(CPUARMState *env, uint64_t rn, uint64_t rm)
+{
+    int el = arm_current_el(env);
+    uint64_t sctlr = arm_sctlr(env, el);
+    int rtag = 0;
+
+    if (allocation_tag_access_enabled(env, el, sctlr)) {
+        uint16_t exclude = rm | env->cp15.gcr_el1;
+        rtag = choose_random_nonexcluded_tag(env, exclude);
+    }
+    return address_with_allocation_tag(rn, rtag);
+}
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index ee95ba7165..b0349bffc4 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -5105,6 +5105,13 @@  static void disas_data_proc_2src(DisasContext *s, uint32_t insn)
     case 3: /* SDIV */
         handle_div(s, true, sf, rm, rn, rd);
         break;
+    case 4: /* IRG */
+        if (sf == 0 || !dc_isar_feature(aa64_mte_insn_reg, s)) {
+            goto do_unallocated;
+        }
+        gen_helper_irg(cpu_reg_sp(s, rd), cpu_env,
+                       cpu_reg_sp(s, rn), cpu_reg(s, rm));
+        break;
     case 8: /* LSLV */
         handle_shift_reg(s, A64_SHIFT_TYPE_LSL, sf, rm, rn, rd);
         break;