[Qemu-devel] [PATCH v1 00/10] arm: Steps towards EL2 support round 4

[Qemu-devel] [PATCH v1 00/10] arm: Steps towards EL2 support round 4

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Hi,

This is another series with small steps towards EL2 emulation.

Patch 1 is just for debugging convinience.
Patch 2 is a bug-fix.
Patches 3 and on add regs and a few small steps towards 2-stage MMU.

Comments welcome!

Best regards,
Edgar

Edgar E. Iglesias (10):
  target-arm: Log the target EL when taking exceptions
  target-arm: Correct opc1 for AT_S12Exx
  target-arm: Add AArch64 access to PAR_EL1
  target-arm: Add VTCR_EL2
  target-arm: Add VTTBR_EL2
  target-arm: Supress TBI for S2 translations
  target-arm: Supress the use of TTBR1 for S2 translations
  target-arm: Supress EPD for S2, EL2 and EL3 translations
  target-arm: Add VPIDR_EL2
  target-arm: Add VMPIDR_EL2

 target-arm/cpu.h        |   4 ++
 target-arm/helper-a64.c |   3 +-
 target-arm/helper.c     | 144 ++++++++++++++++++++++++++++++++++++++++++++----
 3 files changed, 138 insertions(+), 13 deletions(-)

-- 
1.9.1

[Qemu-devel] [PATCH v1 01/10] target-arm: Log the target EL when taking exceptions

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Log the target EL when taking exceptions. This is useful when
debugging guest SW or QEMU itself while transitioning through
the various ELs.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/helper-a64.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
index 08c95a3..2e5e356 100644
--- a/target-arm/helper-a64.c
+++ b/target-arm/helper-a64.c
@@ -478,7 +478,8 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
     }
 
     arm_log_exception(cs->exception_index);
-    qemu_log_mask(CPU_LOG_INT, "...from EL%d\n", arm_current_el(env));
+    qemu_log_mask(CPU_LOG_INT, "...from EL%d to EL%d\n", arm_current_el(env),
+                  new_el);
     if (qemu_loglevel_mask(CPU_LOG_INT)
         && !excp_is_internal(cs->exception_index)) {
         qemu_log_mask(CPU_LOG_INT, "...with ESR 0x%" PRIx32 "\n",
-- 
1.9.1

Re: [Qemu-devel] [PATCH v1 01/10] target-arm: Log the target EL when taking exceptions

[Alistair Francis]

On Thu, Sep 3, 2015 at 1:14 PM, Edgar E. Iglesias
<edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Log the target EL when taking exceptions. This is useful when
> debugging guest SW or QEMU itself while transitioning through
> the various ELs.
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Thanks,

Alistair

> ---
>  target-arm/helper-a64.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
> index 08c95a3..2e5e356 100644
> --- a/target-arm/helper-a64.c
> +++ b/target-arm/helper-a64.c
> @@ -478,7 +478,8 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
>      }
>
>      arm_log_exception(cs->exception_index);
> -    qemu_log_mask(CPU_LOG_INT, "...from EL%d\n", arm_current_el(env));
> +    qemu_log_mask(CPU_LOG_INT, "...from EL%d to EL%d\n", arm_current_el(env),
> +                  new_el);
>      if (qemu_loglevel_mask(CPU_LOG_INT)
>          && !excp_is_internal(cs->exception_index)) {
>          qemu_log_mask(CPU_LOG_INT, "...with ESR 0x%" PRIx32 "\n",
> --
> 1.9.1
>
>

[Qemu-devel] [PATCH v1 02/10] target-arm: Correct opc1 for AT_S12Exx

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/helper.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 7df1f06..4234e7c 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2975,16 +2975,16 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
       .opc0 = 1, .opc1 = 0, .crn = 7, .crm = 8, .opc2 = 3,
       .access = PL1_W, .type = ARM_CP_NO_RAW, .writefn = ats_write64 },
     { .name = "AT_S12E1R", .state = ARM_CP_STATE_AA64,
-      .opc0 = 1, .opc1 = 0, .crn = 7, .crm = 8, .opc2 = 4,
+      .opc0 = 1, .opc1 = 4, .crn = 7, .crm = 8, .opc2 = 4,
       .access = PL2_W, .type = ARM_CP_NO_RAW, .writefn = ats_write64 },
     { .name = "AT_S12E1W", .state = ARM_CP_STATE_AA64,
-      .opc0 = 1, .opc1 = 0, .crn = 7, .crm = 8, .opc2 = 5,
+      .opc0 = 1, .opc1 = 4, .crn = 7, .crm = 8, .opc2 = 5,
       .access = PL2_W, .type = ARM_CP_NO_RAW, .writefn = ats_write64 },
     { .name = "AT_S12E0R", .state = ARM_CP_STATE_AA64,
-      .opc0 = 1, .opc1 = 0, .crn = 7, .crm = 8, .opc2 = 6,
+      .opc0 = 1, .opc1 = 4, .crn = 7, .crm = 8, .opc2 = 6,
       .access = PL2_W, .type = ARM_CP_NO_RAW, .writefn = ats_write64 },
     { .name = "AT_S12E0W", .state = ARM_CP_STATE_AA64,
-      .opc0 = 1, .opc1 = 0, .crn = 7, .crm = 8, .opc2 = 7,
+      .opc0 = 1, .opc1 = 4, .crn = 7, .crm = 8, .opc2 = 7,
       .access = PL2_W, .type = ARM_CP_NO_RAW, .writefn = ats_write64 },
     /* AT S1E2* are elsewhere as they UNDEF from EL3 if EL2 is not present */
     { .name = "AT_S1E3R", .state = ARM_CP_STATE_AA64,
-- 
1.9.1

Re: [Qemu-devel] [PATCH v1 02/10] target-arm: Correct opc1 for AT_S12Exx

[Alistair Francis]

On Thu, Sep 3, 2015 at 1:14 PM, Edgar E. Iglesias
<edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Thanks,

Alistair

> ---
>  target-arm/helper.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 7df1f06..4234e7c 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -2975,16 +2975,16 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
>        .opc0 = 1, .opc1 = 0, .crn = 7, .crm = 8, .opc2 = 3,
>        .access = PL1_W, .type = ARM_CP_NO_RAW, .writefn = ats_write64 },
>      { .name = "AT_S12E1R", .state = ARM_CP_STATE_AA64,
> -      .opc0 = 1, .opc1 = 0, .crn = 7, .crm = 8, .opc2 = 4,
> +      .opc0 = 1, .opc1 = 4, .crn = 7, .crm = 8, .opc2 = 4,
>        .access = PL2_W, .type = ARM_CP_NO_RAW, .writefn = ats_write64 },
>      { .name = "AT_S12E1W", .state = ARM_CP_STATE_AA64,
> -      .opc0 = 1, .opc1 = 0, .crn = 7, .crm = 8, .opc2 = 5,
> +      .opc0 = 1, .opc1 = 4, .crn = 7, .crm = 8, .opc2 = 5,
>        .access = PL2_W, .type = ARM_CP_NO_RAW, .writefn = ats_write64 },
>      { .name = "AT_S12E0R", .state = ARM_CP_STATE_AA64,
> -      .opc0 = 1, .opc1 = 0, .crn = 7, .crm = 8, .opc2 = 6,
> +      .opc0 = 1, .opc1 = 4, .crn = 7, .crm = 8, .opc2 = 6,
>        .access = PL2_W, .type = ARM_CP_NO_RAW, .writefn = ats_write64 },
>      { .name = "AT_S12E0W", .state = ARM_CP_STATE_AA64,
> -      .opc0 = 1, .opc1 = 0, .crn = 7, .crm = 8, .opc2 = 7,
> +      .opc0 = 1, .opc1 = 4, .crn = 7, .crm = 8, .opc2 = 7,
>        .access = PL2_W, .type = ARM_CP_NO_RAW, .writefn = ats_write64 },
>      /* AT S1E2* are elsewhere as they UNDEF from EL3 if EL2 is not present */
>      { .name = "AT_S1E3R", .state = ARM_CP_STATE_AA64,
> --
> 1.9.1
>
>

[Qemu-devel] [PATCH v1 03/10] target-arm: Add AArch64 access to PAR_EL1

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/helper.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 4234e7c..a057a70 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2993,6 +2993,12 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
     { .name = "AT_S1E3W", .state = ARM_CP_STATE_AA64,
       .opc0 = 1, .opc1 = 6, .crn = 7, .crm = 8, .opc2 = 1,
       .access = PL3_W, .type = ARM_CP_NO_RAW, .writefn = ats_write64 },
+    { .name = "PAR_EL1", .state = ARM_CP_STATE_AA64,
+      .type = ARM_CP_ALIAS,
+      .opc0 = 3, .opc1 = 0, .crn = 7, .crm = 4, .opc2 = 0,
+      .access = PL1_RW, .resetvalue = 0,
+      .fieldoffset = offsetof(CPUARMState, cp15.par_el[1]),
+      .writefn = par_write },
 #endif
     /* TLB invalidate last level of translation table walk */
     { .name = "TLBIMVALIS", .cp = 15, .opc1 = 0, .crn = 8, .crm = 3, .opc2 = 5,
-- 
1.9.1

Re: [Qemu-devel] [PATCH v1 03/10] target-arm: Add AArch64 access to PAR_EL1

[Alistair Francis]

On Thu, Sep 3, 2015 at 1:14 PM, Edgar E. Iglesias
<edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Thanks,

Alistair

> ---
>  target-arm/helper.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 4234e7c..a057a70 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -2993,6 +2993,12 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
>      { .name = "AT_S1E3W", .state = ARM_CP_STATE_AA64,
>        .opc0 = 1, .opc1 = 6, .crn = 7, .crm = 8, .opc2 = 1,
>        .access = PL3_W, .type = ARM_CP_NO_RAW, .writefn = ats_write64 },
> +    { .name = "PAR_EL1", .state = ARM_CP_STATE_AA64,
> +      .type = ARM_CP_ALIAS,
> +      .opc0 = 3, .opc1 = 0, .crn = 7, .crm = 4, .opc2 = 0,
> +      .access = PL1_RW, .resetvalue = 0,
> +      .fieldoffset = offsetof(CPUARMState, cp15.par_el[1]),
> +      .writefn = par_write },
>  #endif
>      /* TLB invalidate last level of translation table walk */
>      { .name = "TLBIMVALIS", .cp = 15, .opc1 = 0, .crn = 8, .crm = 3, .opc2 = 5,
> --
> 1.9.1
>
>

[Qemu-devel] [PATCH v1 04/10] target-arm: Add VTCR_EL2

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/cpu.h    |  1 +
 target-arm/helper.c | 28 ++++++++++++++++++++++++++--
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 31825d3..ba22e12 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -223,6 +223,7 @@ typedef struct CPUARMState {
         };
         /* MMU translation table base control. */
         TCR tcr_el[4];
+        TCR vtcr_el2; /* Virtualization Translation Control.  */
         uint32_t c2_data; /* MPU data cachable bits.  */
         uint32_t c2_insn; /* MPU instruction cachable bits.  */
         union { /* MMU domain access control register
diff --git a/target-arm/helper.c b/target-arm/helper.c
index a057a70..c82aa1d 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -325,6 +325,21 @@ void init_cpreg_list(ARMCPU *cpu)
     g_list_free(keys);
 }
 
+/*
+ * Some registers are not accessible if EL3.NS=0 and EL3 is using AArch32 but
+ * they are accesible when EL3 is using AArch64 regardless of EL3.NS.
+ */
+static CPAccessResult access_el3_aa32ns_aa64any(CPUARMState *env,
+                                                const ARMCPRegInfo *ri)
+{
+    bool secure = arm_is_secure_below_el3(env);
+
+    if (secure && !arm_el_is_aa64(env, 3)) {
+        return CP_ACCESS_TRAP_UNCATEGORIZED;
+    }
+    return CP_ACCESS_OK;
+}
+
 static void dacr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
 {
     ARMCPU *cpu = arm_env_get_cpu(env);
@@ -3112,6 +3127,10 @@ static const ARMCPRegInfo el3_no_el2_cp_reginfo[] = {
     { .name = "TCR_EL2", .state = ARM_CP_STATE_BOTH,
       .opc0 = 3, .opc1 = 4, .crn = 2, .crm = 0, .opc2 = 2,
       .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "VTCR_EL2", .state = ARM_CP_STATE_BOTH,
+      .opc0 = 3, .opc1 = 4, .crn = 2, .crm = 1, .opc2 = 2,
+      .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any,
+      .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore },
     { .name = "SCTLR_EL2", .state = ARM_CP_STATE_BOTH,
       .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 0, .opc2 = 0,
       .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
@@ -3246,6 +3265,12 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
       .access = PL2_RW, .writefn = vmsa_tcr_el1_write,
       .resetfn = vmsa_ttbcr_reset, .raw_writefn = raw_write,
       .fieldoffset = offsetof(CPUARMState, cp15.tcr_el[2]) },
+    { .name = "VTCR_EL2", .state = ARM_CP_STATE_BOTH,
+      .opc0 = 3, .opc1 = 4, .crn = 2, .crm = 1, .opc2 = 2,
+      .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any,
+      .writefn = vmsa_tcr_el1_write,
+      .resetfn = vmsa_ttbcr_reset, .raw_writefn = raw_write,
+      .fieldoffset = offsetof(CPUARMState, cp15.vtcr_el2) },
     { .name = "SCTLR_EL2", .state = ARM_CP_STATE_BOTH,
       .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 0, .opc2 = 0,
       .access = PL2_RW, .raw_writefn = raw_write, .writefn = sctlr_write,
@@ -5735,8 +5760,7 @@ static inline bool regime_translation_disabled(CPUARMState *env,
 static inline TCR *regime_tcr(CPUARMState *env, ARMMMUIdx mmu_idx)
 {
     if (mmu_idx == ARMMMUIdx_S2NS) {
-        /* TODO: return VTCR_EL2 */
-        g_assert_not_reached();
+        return &env->cp15.vtcr_el2;
     }
     return &env->cp15.tcr_el[regime_el(env, mmu_idx)];
 }
-- 
1.9.1

Re: [Qemu-devel] [PATCH v1 04/10] target-arm: Add VTCR_EL2

[Peter Maydell]

On 3 September 2015 at 21:14, Edgar E. Iglesias
<edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
>  target-arm/cpu.h    |  1 +
>  target-arm/helper.c | 28 ++++++++++++++++++++++++++--
>  2 files changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 31825d3..ba22e12 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -223,6 +223,7 @@ typedef struct CPUARMState {
>          };
>          /* MMU translation table base control. */
>          TCR tcr_el[4];
> +        TCR vtcr_el2; /* Virtualization Translation Control.  */
>          uint32_t c2_data; /* MPU data cachable bits.  */
>          uint32_t c2_insn; /* MPU instruction cachable bits.  */
>          union { /* MMU domain access control register
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index a057a70..c82aa1d 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -325,6 +325,21 @@ void init_cpreg_list(ARMCPU *cpu)
>      g_list_free(keys);
>  }
>
> +/*
> + * Some registers are not accessible if EL3.NS=0 and EL3 is using AArch32 but
> + * they are accesible when EL3 is using AArch64 regardless of EL3.NS.
> + */
> +static CPAccessResult access_el3_aa32ns_aa64any(CPUARMState *env,
> +                                                const ARMCPRegInfo *ri)
> +{
> +    bool secure = arm_is_secure_below_el3(env);
> +
> +    if (secure && !arm_el_is_aa64(env, 3)) {
> +        return CP_ACCESS_TRAP_UNCATEGORIZED;
> +    }
> +    return CP_ACCESS_OK;
> +}

This access function will always return OK for the AArch64 register,
so probably better to split the regdef rather than using STATE_BOTH,
and then avoid the accessfn on the 64-bit register.

> +
>  static void dacr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
>  {
>      ARMCPU *cpu = arm_env_get_cpu(env);
> @@ -3112,6 +3127,10 @@ static const ARMCPRegInfo el3_no_el2_cp_reginfo[] = {
>      { .name = "TCR_EL2", .state = ARM_CP_STATE_BOTH,
>        .opc0 = 3, .opc1 = 4, .crn = 2, .crm = 0, .opc2 = 2,
>        .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
> +    { .name = "VTCR_EL2", .state = ARM_CP_STATE_BOTH,
> +      .opc0 = 3, .opc1 = 4, .crn = 2, .crm = 1, .opc2 = 2,
> +      .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any,
> +      .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore },

RAZ/WI register should use CP_CONST/resetvalue=0. (Access functions
apply even for const registers.)

>      { .name = "SCTLR_EL2", .state = ARM_CP_STATE_BOTH,
>        .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 0, .opc2 = 0,
>        .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
> @@ -3246,6 +3265,12 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
>        .access = PL2_RW, .writefn = vmsa_tcr_el1_write,
>        .resetfn = vmsa_ttbcr_reset, .raw_writefn = raw_write,
>        .fieldoffset = offsetof(CPUARMState, cp15.tcr_el[2]) },
> +    { .name = "VTCR_EL2", .state = ARM_CP_STATE_BOTH,
> +      .opc0 = 3, .opc1 = 4, .crn = 2, .crm = 1, .opc2 = 2,
> +      .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any,
> +      .writefn = vmsa_tcr_el1_write,

There's no AS bit in the VTCR_EL2, so you could avoid an unnecessary
TLB flush by not using the writefn we use for TCR_EL1. (I think
that if you don't provide a writefn or raw_writefn it should just
work, but check that...)

> +      .resetfn = vmsa_ttbcr_reset, .raw_writefn = raw_write,
> +      .fieldoffset = offsetof(CPUARMState, cp15.vtcr_el2) },
>      { .name = "SCTLR_EL2", .state = ARM_CP_STATE_BOTH,
>        .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 0, .opc2 = 0,
>        .access = PL2_RW, .raw_writefn = raw_write, .writefn = sctlr_write,
> @@ -5735,8 +5760,7 @@ static inline bool regime_translation_disabled(CPUARMState *env,
>  static inline TCR *regime_tcr(CPUARMState *env, ARMMMUIdx mmu_idx)
>  {
>      if (mmu_idx == ARMMMUIdx_S2NS) {
> -        /* TODO: return VTCR_EL2 */
> -        g_assert_not_reached();
> +        return &env->cp15.vtcr_el2;
>      }
>      return &env->cp15.tcr_el[regime_el(env, mmu_idx)];
>  }

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 04/10] target-arm: Add VTCR_EL2

[Edgar E. Iglesias]

On Tue, Sep 08, 2015 at 03:19:37PM +0100, Peter Maydell wrote:
> On 3 September 2015 at 21:14, Edgar E. Iglesias
> <edgar.iglesias@gmail.com> wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> >
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> > ---
> >  target-arm/cpu.h    |  1 +
> >  target-arm/helper.c | 28 ++++++++++++++++++++++++++--
> >  2 files changed, 27 insertions(+), 2 deletions(-)
> >
> > diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> > index 31825d3..ba22e12 100644
> > --- a/target-arm/cpu.h
> > +++ b/target-arm/cpu.h
> > @@ -223,6 +223,7 @@ typedef struct CPUARMState {
> >          };
> >          /* MMU translation table base control. */
> >          TCR tcr_el[4];
> > +        TCR vtcr_el2; /* Virtualization Translation Control.  */
> >          uint32_t c2_data; /* MPU data cachable bits.  */
> >          uint32_t c2_insn; /* MPU instruction cachable bits.  */
> >          union { /* MMU domain access control register
> > diff --git a/target-arm/helper.c b/target-arm/helper.c
> > index a057a70..c82aa1d 100644
> > --- a/target-arm/helper.c
> > +++ b/target-arm/helper.c
> > @@ -325,6 +325,21 @@ void init_cpreg_list(ARMCPU *cpu)
> >      g_list_free(keys);
> >  }
> >
> > +/*
> > + * Some registers are not accessible if EL3.NS=0 and EL3 is using AArch32 but
> > + * they are accesible when EL3 is using AArch64 regardless of EL3.NS.
> > + */
> > +static CPAccessResult access_el3_aa32ns_aa64any(CPUARMState *env,
> > +                                                const ARMCPRegInfo *ri)
> > +{
> > +    bool secure = arm_is_secure_below_el3(env);
> > +
> > +    if (secure && !arm_el_is_aa64(env, 3)) {
> > +        return CP_ACCESS_TRAP_UNCATEGORIZED;
> > +    }
> > +    return CP_ACCESS_OK;
> > +}
> 
> This access function will always return OK for the AArch64 register,
> so probably better to split the regdef rather than using STATE_BOTH,
> and then avoid the accessfn on the 64-bit register.
> 
> > +
> >  static void dacr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
> >  {
> >      ARMCPU *cpu = arm_env_get_cpu(env);
> > @@ -3112,6 +3127,10 @@ static const ARMCPRegInfo el3_no_el2_cp_reginfo[] = {
> >      { .name = "TCR_EL2", .state = ARM_CP_STATE_BOTH,
> >        .opc0 = 3, .opc1 = 4, .crn = 2, .crm = 0, .opc2 = 2,
> >        .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
> > +    { .name = "VTCR_EL2", .state = ARM_CP_STATE_BOTH,
> > +      .opc0 = 3, .opc1 = 4, .crn = 2, .crm = 1, .opc2 = 2,
> > +      .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any,
> > +      .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore },
> 
> RAZ/WI register should use CP_CONST/resetvalue=0. (Access functions
> apply even for const registers.)
> 
> >      { .name = "SCTLR_EL2", .state = ARM_CP_STATE_BOTH,
> >        .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 0, .opc2 = 0,
> >        .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
> > @@ -3246,6 +3265,12 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
> >        .access = PL2_RW, .writefn = vmsa_tcr_el1_write,
> >        .resetfn = vmsa_ttbcr_reset, .raw_writefn = raw_write,
> >        .fieldoffset = offsetof(CPUARMState, cp15.tcr_el[2]) },
> > +    { .name = "VTCR_EL2", .state = ARM_CP_STATE_BOTH,
> > +      .opc0 = 3, .opc1 = 4, .crn = 2, .crm = 1, .opc2 = 2,
> > +      .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any,
> > +      .writefn = vmsa_tcr_el1_write,
> 
> There's no AS bit in the VTCR_EL2, so you could avoid an unnecessary
> TLB flush by not using the writefn we use for TCR_EL1. (I think
> that if you don't provide a writefn or raw_writefn it should just
> work, but check that...)

I think you are right, nice catch. I'll fix all of these up.

Cheers,
Edgar


> 
> > +      .resetfn = vmsa_ttbcr_reset, .raw_writefn = raw_write,
> > +      .fieldoffset = offsetof(CPUARMState, cp15.vtcr_el2) },
> >      { .name = "SCTLR_EL2", .state = ARM_CP_STATE_BOTH,
> >        .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 0, .opc2 = 0,
> >        .access = PL2_RW, .raw_writefn = raw_write, .writefn = sctlr_write,
> > @@ -5735,8 +5760,7 @@ static inline bool regime_translation_disabled(CPUARMState *env,
> >  static inline TCR *regime_tcr(CPUARMState *env, ARMMMUIdx mmu_idx)
> >  {
> >      if (mmu_idx == ARMMMUIdx_S2NS) {
> > -        /* TODO: return VTCR_EL2 */
> > -        g_assert_not_reached();
> > +        return &env->cp15.vtcr_el2;
> >      }
> >      return &env->cp15.tcr_el[regime_el(env, mmu_idx)];
> >  }
> 
> thanks
> -- PMM

Re: [Qemu-devel] [PATCH v1 04/10] target-arm: Add VTCR_EL2

[Edgar E. Iglesias]

On Tue, Sep 08, 2015 at 03:19:37PM +0100, Peter Maydell wrote:
> On 3 September 2015 at 21:14, Edgar E. Iglesias
> <edgar.iglesias@gmail.com> wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> >
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> > ---
> >  target-arm/cpu.h    |  1 +
> >  target-arm/helper.c | 28 ++++++++++++++++++++++++++--
> >  2 files changed, 27 insertions(+), 2 deletions(-)
> >
> > diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> > index 31825d3..ba22e12 100644
> > --- a/target-arm/cpu.h
> > +++ b/target-arm/cpu.h
> > @@ -223,6 +223,7 @@ typedef struct CPUARMState {
> >          };
> >          /* MMU translation table base control. */
> >          TCR tcr_el[4];
> > +        TCR vtcr_el2; /* Virtualization Translation Control.  */
> >          uint32_t c2_data; /* MPU data cachable bits.  */
> >          uint32_t c2_insn; /* MPU instruction cachable bits.  */
> >          union { /* MMU domain access control register
> > diff --git a/target-arm/helper.c b/target-arm/helper.c
> > index a057a70..c82aa1d 100644
> > --- a/target-arm/helper.c
> > +++ b/target-arm/helper.c
> > @@ -325,6 +325,21 @@ void init_cpreg_list(ARMCPU *cpu)
> >      g_list_free(keys);
> >  }
> >
> > +/*
> > + * Some registers are not accessible if EL3.NS=0 and EL3 is using AArch32 but
> > + * they are accesible when EL3 is using AArch64 regardless of EL3.NS.
> > + */
> > +static CPAccessResult access_el3_aa32ns_aa64any(CPUARMState *env,
> > +                                                const ARMCPRegInfo *ri)
> > +{
> > +    bool secure = arm_is_secure_below_el3(env);
> > +
> > +    if (secure && !arm_el_is_aa64(env, 3)) {
> > +        return CP_ACCESS_TRAP_UNCATEGORIZED;
> > +    }
> > +    return CP_ACCESS_OK;
> > +}
> 
> This access function will always return OK for the AArch64 register,
> so probably better to split the regdef rather than using STATE_BOTH,
> and then avoid the accessfn on the 64-bit register.


Hi Peter,

In the interest avoiding duplication, do you think the following makes
sense for regs with the el3_aa32ns_aa64any access checks?

1. Use STATE_BOTH for "low-activity" registers (e.g the EL3 view when EL2 does not exist).
2. Use STATE_BOTH for regs that anyway have a read/write function
3. Split AA64 and AA32 reg entries for regs without read/write helper call for spead (e.g VTCR_EL2).

Cheers,
Edgar

Re: [Qemu-devel] [PATCH v1 04/10] target-arm: Add VTCR_EL2

[Peter Maydell]

On 11 September 2015 at 15:40, Edgar E. Iglesias
<edgar.iglesias@xilinx.com> wrote:
> In the interest avoiding duplication, do you think the following makes
> sense for regs with the el3_aa32ns_aa64any access checks?
>
> 1. Use STATE_BOTH for "low-activity" registers (e.g the EL3 view when EL2 does not exist).
> 2. Use STATE_BOTH for regs that anyway have a read/write function
> 3. Split AA64 and AA32 reg entries for regs without read/write helper call for spead (e.g VTCR_EL2).

Sounds plausible.

-- PMM

Re: [Qemu-devel] [PATCH v1 04/10] target-arm: Add VTCR_EL2

[Edgar E. Iglesias]

On Fri, Sep 11, 2015 at 03:43:48PM +0100, Peter Maydell wrote:
> On 11 September 2015 at 15:40, Edgar E. Iglesias
> <edgar.iglesias@xilinx.com> wrote:
> > In the interest avoiding duplication, do you think the following makes
> > sense for regs with the el3_aa32ns_aa64any access checks?
> >
> > 1. Use STATE_BOTH for "low-activity" registers (e.g the EL3 view when EL2 does not exist).
> > 2. Use STATE_BOTH for regs that anyway have a read/write function
> > 3. Split AA64 and AA32 reg entries for regs without read/write helper call for spead (e.g VTCR_EL2).
> 
> Sounds plausible.
>

OK, thanks! 

[Qemu-devel] [PATCH v1 05/10] target-arm: Add VTTBR_EL2

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/cpu.h    |  1 +
 target-arm/helper.c | 34 ++++++++++++++++++++++++++++++++--
 2 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index ba22e12..0ebdaf7 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -221,6 +221,7 @@ typedef struct CPUARMState {
             };
             uint64_t ttbr1_el[4];
         };
+        uint64_t vttbr_el2; /* Virtualization Translation Table Base.  */
         /* MMU translation table base control. */
         TCR tcr_el[4];
         TCR vtcr_el2; /* Virtualization Translation Control.  */
diff --git a/target-arm/helper.c b/target-arm/helper.c
index c82aa1d..ec19e68 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2200,6 +2200,19 @@ static void vmsa_ttbr_write(CPUARMState *env, const ARMCPRegInfo *ri,
     raw_write(env, ri, value);
 }
 
+static void vttbr_write(CPUARMState *env, const ARMCPRegInfo *ri,
+                        uint64_t value)
+{
+    ARMCPU *cpu = arm_env_get_cpu(env);
+    CPUState *cs = CPU(cpu);
+
+    if (raw_read(env, ri) != value) {
+        tlb_flush_by_mmuidx(cs, ARMMMUIdx_S12NSE1, ARMMMUIdx_S12NSE0,
+                            ARMMMUIdx_S2NS, -1);
+        raw_write(env, ri, value);
+    }
+}
+
 static const ARMCPRegInfo vmsa_pmsa_cp_reginfo[] = {
     { .name = "DFSR", .cp = 15, .crn = 5, .crm = 0, .opc1 = 0, .opc2 = 0,
       .access = PL1_RW, .type = ARM_CP_ALIAS,
@@ -3131,6 +3144,14 @@ static const ARMCPRegInfo el3_no_el2_cp_reginfo[] = {
       .opc0 = 3, .opc1 = 4, .crn = 2, .crm = 1, .opc2 = 2,
       .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any,
       .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore },
+    { .name = "VTTBR", .state = ARM_CP_STATE_AA32, .type = ARM_CP_64BIT,
+      .cp = 15, .opc1 = 6, .crm = 2,
+      .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any,
+      .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore },
+    { .name = "VTTBR_EL2", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 4, .crn = 2, .crm = 1, .opc2 = 0,
+      .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any,
+      .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore },
     { .name = "SCTLR_EL2", .state = ARM_CP_STATE_BOTH,
       .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 0, .opc2 = 0,
       .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
@@ -3271,6 +3292,16 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
       .writefn = vmsa_tcr_el1_write,
       .resetfn = vmsa_ttbcr_reset, .raw_writefn = raw_write,
       .fieldoffset = offsetof(CPUARMState, cp15.vtcr_el2) },
+    { .name = "VTTBR", .state = ARM_CP_STATE_AA32, .type = ARM_CP_64BIT,
+      .cp = 15, .opc1 = 6, .crm = 2,
+      .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any,
+      .fieldoffset = offsetof(CPUARMState, cp15.vttbr_el2),
+      .writefn = vttbr_write },
+    { .name = "VTTBR_EL2", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 4, .crn = 2, .crm = 1, .opc2 = 0,
+      .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any,
+      .fieldoffset = offsetof(CPUARMState, cp15.vttbr_el2),
+      .writefn = vttbr_write },
     { .name = "SCTLR_EL2", .state = ARM_CP_STATE_BOTH,
       .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 0, .opc2 = 0,
       .access = PL2_RW, .raw_writefn = raw_write, .writefn = sctlr_write,
@@ -5770,8 +5801,7 @@ static inline uint64_t regime_ttbr(CPUARMState *env, ARMMMUIdx mmu_idx,
                                    int ttbrn)
 {
     if (mmu_idx == ARMMMUIdx_S2NS) {
-        /* TODO: return VTTBR_EL2 */
-        g_assert_not_reached();
+        return env->cp15.vttbr_el2;
     }
     if (ttbrn == 0) {
         return env->cp15.ttbr0_el[regime_el(env, mmu_idx)];
-- 
1.9.1

Re: [Qemu-devel] [PATCH v1 05/10] target-arm: Add VTTBR_EL2

[Peter Maydell]

On 3 September 2015 at 21:14, Edgar E. Iglesias
<edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
>  target-arm/cpu.h    |  1 +
>  target-arm/helper.c | 34 ++++++++++++++++++++++++++++++++--
>  2 files changed, 33 insertions(+), 2 deletions(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index ba22e12..0ebdaf7 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -221,6 +221,7 @@ typedef struct CPUARMState {
>              };
>              uint64_t ttbr1_el[4];
>          };
> +        uint64_t vttbr_el2; /* Virtualization Translation Table Base.  */
>          /* MMU translation table base control. */
>          TCR tcr_el[4];
>          TCR vtcr_el2; /* Virtualization Translation Control.  */
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index c82aa1d..ec19e68 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -2200,6 +2200,19 @@ static void vmsa_ttbr_write(CPUARMState *env, const ARMCPRegInfo *ri,
>      raw_write(env, ri, value);
>  }
>
> +static void vttbr_write(CPUARMState *env, const ARMCPRegInfo *ri,
> +                        uint64_t value)
> +{
> +    ARMCPU *cpu = arm_env_get_cpu(env);
> +    CPUState *cs = CPU(cpu);
> +
> +    if (raw_read(env, ri) != value) {
> +        tlb_flush_by_mmuidx(cs, ARMMMUIdx_S12NSE1, ARMMMUIdx_S12NSE0,
> +                            ARMMMUIdx_S2NS, -1);

We only need the TLB flush because this could change the VMID and
our TLB doesn't handle VMIDs, right? That could use a comment
(compare the remark about ASIDs in vmsa_ttbr_write()).

> +        raw_write(env, ri, value);
> +    }
> +}
> +
>  static const ARMCPRegInfo vmsa_pmsa_cp_reginfo[] = {
>      { .name = "DFSR", .cp = 15, .crn = 5, .crm = 0, .opc1 = 0, .opc2 = 0,
>        .access = PL1_RW, .type = ARM_CP_ALIAS,
> @@ -3131,6 +3144,14 @@ static const ARMCPRegInfo el3_no_el2_cp_reginfo[] = {
>        .opc0 = 3, .opc1 = 4, .crn = 2, .crm = 1, .opc2 = 2,
>        .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any,
>        .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore },
> +    { .name = "VTTBR", .state = ARM_CP_STATE_AA32, .type = ARM_CP_64BIT,
> +      .cp = 15, .opc1 = 6, .crm = 2,
> +      .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any,
> +      .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore },
> +    { .name = "VTTBR_EL2", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 4, .crn = 2, .crm = 1, .opc2 = 0,
> +      .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any,
> +      .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore },

RAZ/WI registers not using ARM_CP_CONST again...

>      { .name = "SCTLR_EL2", .state = ARM_CP_STATE_BOTH,
>        .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 0, .opc2 = 0,
>        .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
> @@ -3271,6 +3292,16 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
>        .writefn = vmsa_tcr_el1_write,
>        .resetfn = vmsa_ttbcr_reset, .raw_writefn = raw_write,
>        .fieldoffset = offsetof(CPUARMState, cp15.vtcr_el2) },
> +    { .name = "VTTBR", .state = ARM_CP_STATE_AA32, .type = ARM_CP_64BIT,
> +      .cp = 15, .opc1 = 6, .crm = 2,
> +      .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any,
> +      .fieldoffset = offsetof(CPUARMState, cp15.vttbr_el2),
> +      .writefn = vttbr_write },
> +    { .name = "VTTBR_EL2", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 4, .crn = 2, .crm = 1, .opc2 = 0,
> +      .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any,
> +      .fieldoffset = offsetof(CPUARMState, cp15.vttbr_el2),
> +      .writefn = vttbr_write },
>      { .name = "SCTLR_EL2", .state = ARM_CP_STATE_BOTH,
>        .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 0, .opc2 = 0,
>        .access = PL2_RW, .raw_writefn = raw_write, .writefn = sctlr_write,
> @@ -5770,8 +5801,7 @@ static inline uint64_t regime_ttbr(CPUARMState *env, ARMMMUIdx mmu_idx,
>                                     int ttbrn)
>  {
>      if (mmu_idx == ARMMMUIdx_S2NS) {
> -        /* TODO: return VTTBR_EL2 */
> -        g_assert_not_reached();
> +        return env->cp15.vttbr_el2;
>      }
>      if (ttbrn == 0) {
>          return env->cp15.ttbr0_el[regime_el(env, mmu_idx)];
> --
> 1.9.1
>

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 05/10] target-arm: Add VTTBR_EL2

[Edgar E. Iglesias]

On Tue, Sep 08, 2015 at 03:27:05PM +0100, Peter Maydell wrote:
> On 3 September 2015 at 21:14, Edgar E. Iglesias
> <edgar.iglesias@gmail.com> wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> >
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> > ---
> >  target-arm/cpu.h    |  1 +
> >  target-arm/helper.c | 34 ++++++++++++++++++++++++++++++++--
> >  2 files changed, 33 insertions(+), 2 deletions(-)
> >
> > diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> > index ba22e12..0ebdaf7 100644
> > --- a/target-arm/cpu.h
> > +++ b/target-arm/cpu.h
> > @@ -221,6 +221,7 @@ typedef struct CPUARMState {
> >              };
> >              uint64_t ttbr1_el[4];
> >          };
> > +        uint64_t vttbr_el2; /* Virtualization Translation Table Base.  */
> >          /* MMU translation table base control. */
> >          TCR tcr_el[4];
> >          TCR vtcr_el2; /* Virtualization Translation Control.  */
> > diff --git a/target-arm/helper.c b/target-arm/helper.c
> > index c82aa1d..ec19e68 100644
> > --- a/target-arm/helper.c
> > +++ b/target-arm/helper.c
> > @@ -2200,6 +2200,19 @@ static void vmsa_ttbr_write(CPUARMState *env, const ARMCPRegInfo *ri,
> >      raw_write(env, ri, value);
> >  }
> >
> > +static void vttbr_write(CPUARMState *env, const ARMCPRegInfo *ri,
> > +                        uint64_t value)
> > +{
> > +    ARMCPU *cpu = arm_env_get_cpu(env);
> > +    CPUState *cs = CPU(cpu);
> > +
> > +    if (raw_read(env, ri) != value) {
> > +        tlb_flush_by_mmuidx(cs, ARMMMUIdx_S12NSE1, ARMMMUIdx_S12NSE0,
> > +                            ARMMMUIdx_S2NS, -1);
> 
> We only need the TLB flush because this could change the VMID and
> our TLB doesn't handle VMIDs, right? That could use a comment
> (compare the remark about ASIDs in vmsa_ttbr_write()).

Yes, will add a comment.


> 
> > +        raw_write(env, ri, value);
> > +    }
> > +}
> > +
> >  static const ARMCPRegInfo vmsa_pmsa_cp_reginfo[] = {
> >      { .name = "DFSR", .cp = 15, .crn = 5, .crm = 0, .opc1 = 0, .opc2 = 0,
> >        .access = PL1_RW, .type = ARM_CP_ALIAS,
> > @@ -3131,6 +3144,14 @@ static const ARMCPRegInfo el3_no_el2_cp_reginfo[] = {
> >        .opc0 = 3, .opc1 = 4, .crn = 2, .crm = 1, .opc2 = 2,
> >        .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any,
> >        .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore },
> > +    { .name = "VTTBR", .state = ARM_CP_STATE_AA32, .type = ARM_CP_64BIT,
> > +      .cp = 15, .opc1 = 6, .crm = 2,
> > +      .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any,
> > +      .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore },
> > +    { .name = "VTTBR_EL2", .state = ARM_CP_STATE_AA64,
> > +      .opc0 = 3, .opc1 = 4, .crn = 2, .crm = 1, .opc2 = 0,
> > +      .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any,
> > +      .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore },
> 
> RAZ/WI registers not using ARM_CP_CONST again...

Thanks, will fix!

Cheers,
Edgar




> 
> >      { .name = "SCTLR_EL2", .state = ARM_CP_STATE_BOTH,
> >        .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 0, .opc2 = 0,
> >        .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
> > @@ -3271,6 +3292,16 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
> >        .writefn = vmsa_tcr_el1_write,
> >        .resetfn = vmsa_ttbcr_reset, .raw_writefn = raw_write,
> >        .fieldoffset = offsetof(CPUARMState, cp15.vtcr_el2) },
> > +    { .name = "VTTBR", .state = ARM_CP_STATE_AA32, .type = ARM_CP_64BIT,
> > +      .cp = 15, .opc1 = 6, .crm = 2,
> > +      .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any,
> > +      .fieldoffset = offsetof(CPUARMState, cp15.vttbr_el2),
> > +      .writefn = vttbr_write },
> > +    { .name = "VTTBR_EL2", .state = ARM_CP_STATE_AA64,
> > +      .opc0 = 3, .opc1 = 4, .crn = 2, .crm = 1, .opc2 = 0,
> > +      .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any,
> > +      .fieldoffset = offsetof(CPUARMState, cp15.vttbr_el2),
> > +      .writefn = vttbr_write },
> >      { .name = "SCTLR_EL2", .state = ARM_CP_STATE_BOTH,
> >        .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 0, .opc2 = 0,
> >        .access = PL2_RW, .raw_writefn = raw_write, .writefn = sctlr_write,
> > @@ -5770,8 +5801,7 @@ static inline uint64_t regime_ttbr(CPUARMState *env, ARMMMUIdx mmu_idx,
> >                                     int ttbrn)
> >  {
> >      if (mmu_idx == ARMMMUIdx_S2NS) {
> > -        /* TODO: return VTTBR_EL2 */
> > -        g_assert_not_reached();
> > +        return env->cp15.vttbr_el2;
> >      }
> >      if (ttbrn == 0) {
> >          return env->cp15.ttbr0_el[regime_el(env, mmu_idx)];
> > --
> > 1.9.1
> >
> 
> thanks
> -- PMM

[Qemu-devel] [PATCH v1 06/10] target-arm: Supress TBI for S2 translations

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Stage-2 MMU translations do not have configurable TBI as
the top byte is always 0 (48-bit IPAs).

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index ec19e68..9ea9719 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -6350,7 +6350,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,
         va_size = 64;
         if (el > 1) {
             tbi = extract64(tcr->raw_tcr, 20, 1);
-        } else {
+        } else if (mmu_idx != ARMMMUIdx_S2NS) {
             if (extract64(address, 55, 1)) {
                 tbi = extract64(tcr->raw_tcr, 38, 1);
             } else {
-- 
1.9.1

Re: [Qemu-devel] [PATCH v1 06/10] target-arm: Supress TBI for S2 translations

[Peter Maydell]

On 3 September 2015 at 21:14, Edgar E. Iglesias
<edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Stage-2 MMU translations do not have configurable TBI as
> the top byte is always 0 (48-bit IPAs).
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
>  target-arm/helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index ec19e68..9ea9719 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -6350,7 +6350,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,
>          va_size = 64;
>          if (el > 1) {
>              tbi = extract64(tcr->raw_tcr, 20, 1);
> -        } else {
> +        } else if (mmu_idx != ARMMMUIdx_S2NS) {
>              if (extract64(address, 55, 1)) {
>                  tbi = extract64(tcr->raw_tcr, 38, 1);
>              } else {

This doesn't look right. regime_el() for S2NS is 2, so
in this else clause mmu_idx can never be S2NS.

Also "suppress" has two 'p's in it :-)

thanks
-- PMM

[Qemu-devel] [PATCH v1 07/10] target-arm: Supress the use of TTBR1 for S2 translations

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Stage-2 MMU translations do not use TTBR1.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/helper.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 9ea9719..66b3fed 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -6372,6 +6372,11 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,
         }
     }
 
+    /* Stage2 translations do not use TTBR1.  */
+    if (mmu_idx == ARMMMUIdx_S2NS) {
+        ttbr1_valid = false;
+    }
+
     /* Determine whether this address is in the region controlled by
      * TTBR0 or TTBR1 (or if it is in neither region and should fault).
      * This is a Non-secure PL0/1 stage 1 translation, so controlled by
-- 
1.9.1

Re: [Qemu-devel] [PATCH v1 07/10] target-arm: Supress the use of TTBR1 for S2 translations

[Peter Maydell]

On 3 September 2015 at 21:14, Edgar E. Iglesias
<edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Stage-2 MMU translations do not use TTBR1.
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
>  target-arm/helper.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 9ea9719..66b3fed 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -6372,6 +6372,11 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,
>          }
>      }
>
> +    /* Stage2 translations do not use TTBR1.  */
> +    if (mmu_idx == ARMMMUIdx_S2NS) {
> +        ttbr1_valid = false;
> +    }
> +

I think this is unnecessary, because we've already set ttbr1_valid
to false in the previous chunk of code for the case where el == 2
(as it is for stage 2 translations).

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 07/10] target-arm: Supress the use of TTBR1 for S2 translations

[Edgar E. Iglesias]

On Tue, Sep 08, 2015 at 03:32:36PM +0100, Peter Maydell wrote:
> On 3 September 2015 at 21:14, Edgar E. Iglesias
> <edgar.iglesias@gmail.com> wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> >
> > Stage-2 MMU translations do not use TTBR1.
> >
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> > ---
> >  target-arm/helper.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/target-arm/helper.c b/target-arm/helper.c
> > index 9ea9719..66b3fed 100644
> > --- a/target-arm/helper.c
> > +++ b/target-arm/helper.c
> > @@ -6372,6 +6372,11 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,
> >          }
> >      }
> >
> > +    /* Stage2 translations do not use TTBR1.  */
> > +    if (mmu_idx == ARMMMUIdx_S2NS) {
> > +        ttbr1_valid = false;
> > +    }
> > +
> 
> I think this is unnecessary, because we've already set ttbr1_valid
> to false in the previous chunk of code for the case where el == 2
> (as it is for stage 2 translations).

I think we may be confused here.

Note S2NS translations are controlled by EL2 but apply to NS EL0 and EL1.

Maybe I should have waited with this stuff until I've posted a more
complete S2 implementation but basically what will happen is that
when HCR.VM is set, we'll do a S2 translation after S1 for NS EL0 and 1.
I don't have it all complete yet though, so I started with these smaller
chunks...

Cheers,
Edgar

Re: [Qemu-devel] [PATCH v1 07/10] target-arm: Supress the use of TTBR1 for S2 translations

[Peter Maydell]

On 8 September 2015 at 15:42, Edgar E. Iglesias
<edgar.iglesias@xilinx.com> wrote:
> On Tue, Sep 08, 2015 at 03:32:36PM +0100, Peter Maydell wrote:
>> On 3 September 2015 at 21:14, Edgar E. Iglesias
>> <edgar.iglesias@gmail.com> wrote:
>> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>> >
>> > Stage-2 MMU translations do not use TTBR1.
>> >
>> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
>> > ---
>> >  target-arm/helper.c | 5 +++++
>> >  1 file changed, 5 insertions(+)
>> >
>> > diff --git a/target-arm/helper.c b/target-arm/helper.c
>> > index 9ea9719..66b3fed 100644
>> > --- a/target-arm/helper.c
>> > +++ b/target-arm/helper.c
>> > @@ -6372,6 +6372,11 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,
>> >          }
>> >      }
>> >
>> > +    /* Stage2 translations do not use TTBR1.  */
>> > +    if (mmu_idx == ARMMMUIdx_S2NS) {
>> > +        ttbr1_valid = false;
>> > +    }
>> > +
>>
>> I think this is unnecessary, because we've already set ttbr1_valid
>> to false in the previous chunk of code for the case where el == 2
>> (as it is for stage 2 translations).
>
> I think we may be confused here.
>
> Note S2NS translations are controlled by EL2 but apply to NS EL0 and EL1.

Yep. el is the result of regime_el(), which returns what the ARM ARM
calls "the EL that the translation regime is controlled from".
In particular, we do things this way because it's the register width
of the controlling EL that determines whether the translation
regime is 64 bit, whether the TCR/TTBR/etc registers are the 64-bit
forms or not, etc.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 07/10] target-arm: Supress the use of TTBR1 for S2 translations

[Edgar E. Iglesias]

On Tue, Sep 08, 2015 at 03:50:34PM +0100, Peter Maydell wrote:
> On 8 September 2015 at 15:42, Edgar E. Iglesias
> <edgar.iglesias@xilinx.com> wrote:
> > On Tue, Sep 08, 2015 at 03:32:36PM +0100, Peter Maydell wrote:
> >> On 3 September 2015 at 21:14, Edgar E. Iglesias
> >> <edgar.iglesias@gmail.com> wrote:
> >> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> >> >
> >> > Stage-2 MMU translations do not use TTBR1.
> >> >
> >> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> >> > ---
> >> >  target-arm/helper.c | 5 +++++
> >> >  1 file changed, 5 insertions(+)
> >> >
> >> > diff --git a/target-arm/helper.c b/target-arm/helper.c
> >> > index 9ea9719..66b3fed 100644
> >> > --- a/target-arm/helper.c
> >> > +++ b/target-arm/helper.c
> >> > @@ -6372,6 +6372,11 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,
> >> >          }
> >> >      }
> >> >
> >> > +    /* Stage2 translations do not use TTBR1.  */
> >> > +    if (mmu_idx == ARMMMUIdx_S2NS) {
> >> > +        ttbr1_valid = false;
> >> > +    }
> >> > +
> >>
> >> I think this is unnecessary, because we've already set ttbr1_valid
> >> to false in the previous chunk of code for the case where el == 2
> >> (as it is for stage 2 translations).
> >
> > I think we may be confused here.
> >
> > Note S2NS translations are controlled by EL2 but apply to NS EL0 and EL1.
> 
> Yep. el is the result of regime_el(), which returns what the ARM ARM
> calls "the EL that the translation regime is controlled from".
> In particular, we do things this way because it's the register width
> of the controlling EL that determines whether the translation
> regime is 64 bit, whether the TCR/TTBR/etc registers are the 64-bit
> forms or not, etc.

OK, I see. I'll have another look at this...

Thanks!
Edgar

[Qemu-devel] [PATCH v1 08/10] target-arm: Supress EPD for S2, EL2 and EL3 translations

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Stage-2 translations, EL2 and EL3 regimes don't have the
EPD control.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/helper.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 66b3fed..a53d713 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -6323,7 +6323,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,
     /* Read an LPAE long-descriptor translation table. */
     MMUFaultType fault_type = translation_fault;
     uint32_t level = 1;
-    uint32_t epd;
+    uint32_t epd = 0;
     int32_t tsz;
     uint32_t tg;
     uint64_t ttbr;
@@ -6420,7 +6420,9 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,
      */
     if (ttbr_select == 0) {
         ttbr = regime_ttbr(env, mmu_idx, 0);
-        epd = extract32(tcr->raw_tcr, 7, 1);
+        if (el < 2 && mmu_idx != ARMMMUIdx_S2NS) {
+            epd = extract32(tcr->raw_tcr, 7, 1);
+        }
         tsz = t0sz;
 
         tg = extract32(tcr->raw_tcr, 14, 2);
-- 
1.9.1

Re: [Qemu-devel] [PATCH v1 08/10] target-arm: Supress EPD for S2, EL2 and EL3 translations

[Peter Maydell]

On 3 September 2015 at 21:14, Edgar E. Iglesias
<edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Stage-2 translations, EL2 and EL3 regimes don't have the
> EPD control.
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
>  target-arm/helper.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 66b3fed..a53d713 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -6323,7 +6323,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,
>      /* Read an LPAE long-descriptor translation table. */
>      MMUFaultType fault_type = translation_fault;
>      uint32_t level = 1;
> -    uint32_t epd;
> +    uint32_t epd = 0;
>      int32_t tsz;
>      uint32_t tg;
>      uint64_t ttbr;
> @@ -6420,7 +6420,9 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,
>       */
>      if (ttbr_select == 0) {
>          ttbr = regime_ttbr(env, mmu_idx, 0);
> -        epd = extract32(tcr->raw_tcr, 7, 1);
> +        if (el < 2 && mmu_idx != ARMMMUIdx_S2NS) {
> +            epd = extract32(tcr->raw_tcr, 7, 1);
> +        }

Just "if (el < 2)" is sufficient.

Also typo in subject again.

thanks
-- PMM

[Qemu-devel] [PATCH v1 09/10] target-arm: Add VPIDR_EL2

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/cpu.h    |  1 +
 target-arm/helper.c | 39 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 0ebdaf7..cdecfdf 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -384,6 +384,7 @@ typedef struct CPUARMState {
          */
         uint64_t c15_ccnt;
         uint64_t pmccfiltr_el0; /* Performance Monitor Filter Register */
+        uint64_t vpidr_el2; /* Virtualization Processor ID Register */
     } cp15;
 
     struct {
diff --git a/target-arm/helper.c b/target-arm/helper.c
index a53d713..3701207 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2431,6 +2431,18 @@ static const ARMCPRegInfo strongarm_cp_reginfo[] = {
     REGINFO_SENTINEL
 };
 
+static uint64_t midr_read(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+    ARMCPU *cpu = arm_env_get_cpu(env);
+    unsigned int cur_el = arm_current_el(env);
+    bool secure = arm_is_secure(env);
+
+    if (arm_feature(&cpu->env, ARM_FEATURE_EL2) && !secure && cur_el == 1) {
+        return env->cp15.vpidr_el2;
+    }
+    return raw_read(env, ri);
+}
+
 static uint64_t mpidr_read(CPUARMState *env, const ARMCPRegInfo *ri)
 {
     ARMCPU *cpu = ARM_CPU(arm_env_get_cpu(env));
@@ -4106,6 +4118,15 @@ void register_cp_regs_for_features(ARMCPU *cpu)
         define_arm_cp_regs(cpu, v8_cp_reginfo);
     }
     if (arm_feature(env, ARM_FEATURE_EL2)) {
+        ARMCPRegInfo vpidr_regs[] = {
+            { .name = "VPIDR_EL2", .state = ARM_CP_STATE_BOTH,
+              .opc0 = 3, .opc1 = 4, .crn = 0, .crm = 0, .opc2 = 0,
+              .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any,
+              .resetvalue = cpu->midr,
+              .fieldoffset = offsetof(CPUARMState, cp15.vpidr_el2) },
+            REGINFO_SENTINEL
+        };
+        define_arm_cp_regs(cpu, vpidr_regs);
         define_arm_cp_regs(cpu, el2_cp_reginfo);
         /* RVBAR_EL2 is only implemented if EL2 is the highest EL */
         if (!arm_feature(env, ARM_FEATURE_EL3)) {
@@ -4121,6 +4142,19 @@ void register_cp_regs_for_features(ARMCPU *cpu)
          * register the no_el2 reginfos.
          */
         if (arm_feature(env, ARM_FEATURE_EL3)) {
+            /* When EL3 exists but not EL2, VPIDR takes the value
+             * of MIDR_EL1.
+             */
+            ARMCPRegInfo vpidr_regs[] = {
+                { .name = "VPIDR_EL2", .state = ARM_CP_STATE_BOTH,
+                  .opc0 = 3, .opc1 = 4, .crn = 0, .crm = 0, .opc2 = 0,
+                  .resetvalue = cpu->midr,
+                  .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any,
+                  .type = ARM_CP_CONST,
+                  .fieldoffset = offsetof(CPUARMState, cp15.vpidr_el2) },
+                REGINFO_SENTINEL
+            };
+            define_arm_cp_regs(cpu, vpidr_regs);
             define_arm_cp_regs(cpu, el3_no_el2_cp_reginfo);
         }
     }
@@ -4198,6 +4232,7 @@ void register_cp_regs_for_features(ARMCPU *cpu)
               .cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = CP_ANY,
               .access = PL1_R, .resetvalue = cpu->midr,
               .writefn = arm_cp_write_ignore, .raw_writefn = raw_write,
+              .readfn = midr_read,
               .fieldoffset = offsetof(CPUARMState, cp15.c0_cpuid),
               .type = ARM_CP_OVERRIDE },
             /* crn = 0 op1 = 0 crm = 3..7 : currently unassigned; we RAZ. */
@@ -4221,7 +4256,9 @@ void register_cp_regs_for_features(ARMCPU *cpu)
         ARMCPRegInfo id_v8_midr_cp_reginfo[] = {
             { .name = "MIDR_EL1", .state = ARM_CP_STATE_BOTH,
               .opc0 = 3, .opc1 = 0, .crn = 0, .crm = 0, .opc2 = 0,
-              .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = cpu->midr },
+              .access = PL1_R, .type = ARM_CP_NO_RAW, .resetvalue = cpu->midr,
+              .fieldoffset = offsetof(CPUARMState, cp15.c0_cpuid),
+              .readfn = midr_read },
             /* crn = 0 op1 = 0 crm = 0 op2 = 4,7 : AArch32 aliases of MIDR */
             { .name = "MIDR", .type = ARM_CP_ALIAS | ARM_CP_CONST,
               .cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 4,
-- 
1.9.1

Re: [Qemu-devel] [PATCH v1 09/10] target-arm: Add VPIDR_EL2

[Peter Maydell]

On 3 September 2015 at 21:14, Edgar E. Iglesias
<edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

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

thanks
-- PMM

[Qemu-devel] [PATCH v1 10/10] target-arm: Add VMPIDR_EL2

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/cpu.h    |  1 +
 target-arm/helper.c | 20 ++++++++++++++++++--
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index cdecfdf..1929a2f 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -385,6 +385,7 @@ typedef struct CPUARMState {
         uint64_t c15_ccnt;
         uint64_t pmccfiltr_el0; /* Performance Monitor Filter Register */
         uint64_t vpidr_el2; /* Virtualization Processor ID Register */
+        uint64_t vmpidr_el2; /* Virtualization Multiprocessor ID Register */
     } cp15;
 
     struct {
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 3701207..e335f8f 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2447,6 +2447,12 @@ static uint64_t mpidr_read(CPUARMState *env, const ARMCPRegInfo *ri)
 {
     ARMCPU *cpu = ARM_CPU(arm_env_get_cpu(env));
     uint64_t mpidr = cpu->mp_affinity;
+    unsigned int cur_el = arm_current_el(env);
+    bool secure = arm_is_secure(env);
+
+    if (arm_feature(env, ARM_FEATURE_EL2) && !secure && cur_el == 2) {
+        mpidr = env->cp15.vmpidr_el2;
+    }
 
     if (arm_feature(env, ARM_FEATURE_V7MP)) {
         mpidr |= (1U << 31);
@@ -4124,6 +4130,11 @@ void register_cp_regs_for_features(ARMCPU *cpu)
               .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any,
               .resetvalue = cpu->midr,
               .fieldoffset = offsetof(CPUARMState, cp15.vpidr_el2) },
+            { .name = "VMPIDR_EL2", .state = ARM_CP_STATE_BOTH,
+              .opc0 = 3, .opc1 = 4, .crn = 0, .crm = 0, .opc2 = 5,
+              .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any,
+              .resetvalue = cpu->mp_affinity,
+              .fieldoffset = offsetof(CPUARMState, cp15.vmpidr_el2) },
             REGINFO_SENTINEL
         };
         define_arm_cp_regs(cpu, vpidr_regs);
@@ -4142,8 +4153,8 @@ void register_cp_regs_for_features(ARMCPU *cpu)
          * register the no_el2 reginfos.
          */
         if (arm_feature(env, ARM_FEATURE_EL3)) {
-            /* When EL3 exists but not EL2, VPIDR takes the value
-             * of MIDR_EL1.
+            /* When EL3 exists but not EL2, VPIDR and VMPIDR take the value
+             * of MIDR_EL1 and MPIDR_EL1.
              */
             ARMCPRegInfo vpidr_regs[] = {
                 { .name = "VPIDR_EL2", .state = ARM_CP_STATE_BOTH,
@@ -4152,6 +4163,11 @@ void register_cp_regs_for_features(ARMCPU *cpu)
                   .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any,
                   .type = ARM_CP_CONST,
                   .fieldoffset = offsetof(CPUARMState, cp15.vpidr_el2) },
+                { .name = "VMPIDR_EL2", .state = ARM_CP_STATE_AA64,
+                  .opc0 = 3, .opc1 = 4, .crn = 0, .crm = 0, .opc2 = 5,
+                  .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any,
+                  .type = ARM_CP_CONST | ARM_CP_NO_RAW,
+                  .readfn = mpidr_read },
                 REGINFO_SENTINEL
             };
             define_arm_cp_regs(cpu, vpidr_regs);
-- 
1.9.1

Re: [Qemu-devel] [PATCH v1 10/10] target-arm: Add VMPIDR_EL2

[Peter Maydell]

On 3 September 2015 at 21:14, Edgar E. Iglesias
<edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
>  target-arm/cpu.h    |  1 +
>  target-arm/helper.c | 20 ++++++++++++++++++--
>  2 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index cdecfdf..1929a2f 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -385,6 +385,7 @@ typedef struct CPUARMState {
>          uint64_t c15_ccnt;
>          uint64_t pmccfiltr_el0; /* Performance Monitor Filter Register */
>          uint64_t vpidr_el2; /* Virtualization Processor ID Register */
> +        uint64_t vmpidr_el2; /* Virtualization Multiprocessor ID Register */
>      } cp15;
>
>      struct {
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 3701207..e335f8f 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -2447,6 +2447,12 @@ static uint64_t mpidr_read(CPUARMState *env, const ARMCPRegInfo *ri)
>  {
>      ARMCPU *cpu = ARM_CPU(arm_env_get_cpu(env));
>      uint64_t mpidr = cpu->mp_affinity;
> +    unsigned int cur_el = arm_current_el(env);
> +    bool secure = arm_is_secure(env);
> +
> +    if (arm_feature(env, ARM_FEATURE_EL2) && !secure && cur_el == 2) {
> +        mpidr = env->cp15.vmpidr_el2;
> +    }

Shouldn't we be returning the VMPIDR if we're in NS-EL1, not NS-EL2?

>      if (arm_feature(env, ARM_FEATURE_V7MP)) {
>          mpidr |= (1U << 31);
> @@ -4124,6 +4130,11 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>                .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any,
>                .resetvalue = cpu->midr,
>                .fieldoffset = offsetof(CPUARMState, cp15.vpidr_el2) },
> +            { .name = "VMPIDR_EL2", .state = ARM_CP_STATE_BOTH,
> +              .opc0 = 3, .opc1 = 4, .crn = 0, .crm = 0, .opc2 = 5,
> +              .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any,
> +              .resetvalue = cpu->mp_affinity,

This resetvalue is missing the M and U bits which are ORed in when we
read the MPIDR.

> +              .fieldoffset = offsetof(CPUARMState, cp15.vmpidr_el2) },
>              REGINFO_SENTINEL
>          };
>          define_arm_cp_regs(cpu, vpidr_regs);
> @@ -4142,8 +4153,8 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>           * register the no_el2 reginfos.
>           */
>          if (arm_feature(env, ARM_FEATURE_EL3)) {
> -            /* When EL3 exists but not EL2, VPIDR takes the value
> -             * of MIDR_EL1.
> +            /* When EL3 exists but not EL2, VPIDR and VMPIDR take the value
> +             * of MIDR_EL1 and MPIDR_EL1.
>               */
>              ARMCPRegInfo vpidr_regs[] = {
>                  { .name = "VPIDR_EL2", .state = ARM_CP_STATE_BOTH,
> @@ -4152,6 +4163,11 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>                    .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any,
>                    .type = ARM_CP_CONST,
>                    .fieldoffset = offsetof(CPUARMState, cp15.vpidr_el2) },
> +                { .name = "VMPIDR_EL2", .state = ARM_CP_STATE_AA64,
> +                  .opc0 = 3, .opc1 = 4, .crn = 0, .crm = 0, .opc2 = 5,
> +                  .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any,
> +                  .type = ARM_CP_CONST | ARM_CP_NO_RAW,
> +                  .readfn = mpidr_read },

CP_CONST and a readfn doesn't make much sense.

>                  REGINFO_SENTINEL
>              };
>              define_arm_cp_regs(cpu, vpidr_regs);
> --
> 1.9.1
>

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 00/10] arm: Steps towards EL2 support round 4

[Peter Maydell]

On 3 September 2015 at 21:14, Edgar E. Iglesias
<edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Hi,
>
> This is another series with small steps towards EL2 emulation.
>
> Patch 1 is just for debugging convinience.
> Patch 2 is a bug-fix.
> Patches 3 and on add regs and a few small steps towards 2-stage MMU.

Patches 1..3 look good and I've added them to target-arm.next.
4..10 I've sent review comments for.

thanks
-- PMM