[v2,01/67] target/arm: Enable SVE for aarch64-linux-user

Message ID 20180217182323.25885-2-richard.henderson@linaro.org
State New
Headers show
Series
  • target/arm: Scalable Vector Extension
Related show

Commit Message

Richard Henderson Feb. 17, 2018, 6:22 p.m.
Enable ARM_FEATURE_SVE for the generic "any" cpu.

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

---
 target/arm/cpu.c   | 7 +++++++
 target/arm/cpu64.c | 1 +
 2 files changed, 8 insertions(+)

-- 
2.14.3

Comments

Peter Maydell Feb. 22, 2018, 5:28 p.m. | #1
On 17 February 2018 at 18:22, Richard Henderson
<richard.henderson@linaro.org> wrote:
> Enable ARM_FEATURE_SVE for the generic "any" cpu.

>

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

> ---

>  target/arm/cpu.c   | 7 +++++++

>  target/arm/cpu64.c | 1 +

>  2 files changed, 8 insertions(+)


Reviewed-by: Peter Maydell <peter.maydell@linaro.org>


though this should probably go at the end of the patchseries rather
than the beginning?

thanks
-- PMM
Richard Henderson Feb. 22, 2018, 7:27 p.m. | #2
On 02/22/2018 09:28 AM, Peter Maydell wrote:
> On 17 February 2018 at 18:22, Richard Henderson

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

>> Enable ARM_FEATURE_SVE for the generic "any" cpu.

>>

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

>> ---

>>  target/arm/cpu.c   | 7 +++++++

>>  target/arm/cpu64.c | 1 +

>>  2 files changed, 8 insertions(+)

> 

> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

> 

> though this should probably go at the end of the patchseries rather

> than the beginning?


Yes, but of course I need it at the beginning for testing.
I'll sort it to the end for the final version.


r~
Alex Bennée Feb. 23, 2018, 5 p.m. | #3
Richard Henderson <richard.henderson@linaro.org> writes:

> Enable ARM_FEATURE_SVE for the generic "any" cpu.

>

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

> ---

>  target/arm/cpu.c   | 7 +++++++

>  target/arm/cpu64.c | 1 +

>  2 files changed, 8 insertions(+)

>

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

> index 1b3ae62db6..10843994c3 100644

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

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

> @@ -150,6 +150,13 @@ static void arm_cpu_reset(CPUState *s)

>          env->cp15.sctlr_el[1] |= SCTLR_UCT | SCTLR_UCI | SCTLR_DZE;

>          /* and to the FP/Neon instructions */

>          env->cp15.cpacr_el1 = deposit64(env->cp15.cpacr_el1, 20, 2, 3);

> +        /* and to the SVE instructions */

> +        env->cp15.cpacr_el1 = deposit64(env->cp15.cpacr_el1, 16, 2, 3);

> +        env->cp15.cptr_el[3] |= CPTR_EZ;

> +        /* with maximum vector length */

> +        env->vfp.zcr_el[1] = ARM_MAX_VQ - 1;

> +        env->vfp.zcr_el[2] = ARM_MAX_VQ - 1;

> +        env->vfp.zcr_el[3] = ARM_MAX_VQ - 1;

>  #else


I notice this is linux-user only but what happens if you specify a
specific CPU in linux-user mode, do we still end up running SVE specific
initialisation?

It seems to me that we should be seeing feature guarded reset stuff in here.

>          /* Reset into the highest available EL */

>          if (arm_feature(env, ARM_FEATURE_EL3)) {

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

> index efc519b49b..36ef9e9d9d 100644

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

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

> @@ -231,6 +231,7 @@ static void aarch64_any_initfn(Object *obj)

>      set_feature(&cpu->env, ARM_FEATURE_V8_PMULL);

>      set_feature(&cpu->env, ARM_FEATURE_CRC);

>      set_feature(&cpu->env, ARM_FEATURE_V8_FP16);

> +    set_feature(&cpu->env, ARM_FEATURE_SVE);

>      cpu->ctr = 0x80038003; /* 32 byte I and D cacheline size, VIPT icache */

>      cpu->dcz_blocksize = 7; /*  512 bytes */

>  }



--
Alex Bennée
Richard Henderson Feb. 23, 2018, 6:47 p.m. | #4
On 02/23/2018 09:00 AM, Alex Bennée wrote:
> 

> Richard Henderson <richard.henderson@linaro.org> writes:

> 

>> Enable ARM_FEATURE_SVE for the generic "any" cpu.

>>

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

>> ---

>>  target/arm/cpu.c   | 7 +++++++

>>  target/arm/cpu64.c | 1 +

>>  2 files changed, 8 insertions(+)

>>

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

>> index 1b3ae62db6..10843994c3 100644

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

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

>> @@ -150,6 +150,13 @@ static void arm_cpu_reset(CPUState *s)

>>          env->cp15.sctlr_el[1] |= SCTLR_UCT | SCTLR_UCI | SCTLR_DZE;

>>          /* and to the FP/Neon instructions */

>>          env->cp15.cpacr_el1 = deposit64(env->cp15.cpacr_el1, 20, 2, 3);

>> +        /* and to the SVE instructions */

>> +        env->cp15.cpacr_el1 = deposit64(env->cp15.cpacr_el1, 16, 2, 3);

>> +        env->cp15.cptr_el[3] |= CPTR_EZ;

>> +        /* with maximum vector length */

>> +        env->vfp.zcr_el[1] = ARM_MAX_VQ - 1;

>> +        env->vfp.zcr_el[2] = ARM_MAX_VQ - 1;

>> +        env->vfp.zcr_el[3] = ARM_MAX_VQ - 1;

>>  #else

> 

> I notice this is linux-user only but what happens if you specify a

> specific CPU in linux-user mode, do we still end up running SVE specific

> initialisation?

> 

> It seems to me that we should be seeing feature guarded reset stuff in here.


You're right.  On the whole (probably) wouldn't matter in the end because the
actual insn decode would still be protected by ARM_FEATURE_SVE.  But even so
we'd see VQ=16 in the TB flags and do too much work in clear_high_part.


r~

Patch

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 1b3ae62db6..10843994c3 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -150,6 +150,13 @@  static void arm_cpu_reset(CPUState *s)
         env->cp15.sctlr_el[1] |= SCTLR_UCT | SCTLR_UCI | SCTLR_DZE;
         /* and to the FP/Neon instructions */
         env->cp15.cpacr_el1 = deposit64(env->cp15.cpacr_el1, 20, 2, 3);
+        /* and to the SVE instructions */
+        env->cp15.cpacr_el1 = deposit64(env->cp15.cpacr_el1, 16, 2, 3);
+        env->cp15.cptr_el[3] |= CPTR_EZ;
+        /* with maximum vector length */
+        env->vfp.zcr_el[1] = ARM_MAX_VQ - 1;
+        env->vfp.zcr_el[2] = ARM_MAX_VQ - 1;
+        env->vfp.zcr_el[3] = ARM_MAX_VQ - 1;
 #else
         /* Reset into the highest available EL */
         if (arm_feature(env, ARM_FEATURE_EL3)) {
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index efc519b49b..36ef9e9d9d 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -231,6 +231,7 @@  static void aarch64_any_initfn(Object *obj)
     set_feature(&cpu->env, ARM_FEATURE_V8_PMULL);
     set_feature(&cpu->env, ARM_FEATURE_CRC);
     set_feature(&cpu->env, ARM_FEATURE_V8_FP16);
+    set_feature(&cpu->env, ARM_FEATURE_SVE);
     cpu->ctr = 0x80038003; /* 32 byte I and D cacheline size, VIPT icache */
     cpu->dcz_blocksize = 7; /*  512 bytes */
 }