[Qemu-devel] [PATCH for-2.6 0/4] various regdef fixes for EL2/EL3 regs

[Qemu-devel] [PATCH for-2.6 0/4] various regdef fixes for EL2/EL3 regs

[Peter Maydell]

This patchset fixes a number of minor bugs in regdefs for some EL2
and EL3 registers. The most interesting one here is the first --
we weren't resetting SCTLR_EL3 correctly for 64-bit CPUs. The
rest are things I discovered by code inspection looking at other
registers:
 * we weren't migrating ESR_EL2 and ESR_EL3
 * we weren't migrating the (RES0) high 32 bits of VTCR_EL2
 * unneeded TLB flush on TCR_EL2 writes

I think these should go into 2.6 since they're bug fixes.

thanks
-- PMM


Peter Maydell (4):
  target-arm: Correctly reset SCTLR_EL3 for 64-bit CPUs
  target-arm: Remove incorrect ALIAS tags from ESR_EL2 and ESR_EL3
  target-arm: Make the 64-bit version of VTCR do the migration
  target-arm: Avoid unnecessary TLB flush on TCR_EL2 writes

 target-arm/helper.c | 37 ++++++++++++++++++++++---------------
 1 file changed, 22 insertions(+), 15 deletions(-)

-- 
1.9.1

[Qemu-devel] [PATCH 1/4] target-arm: Correctly reset SCTLR_EL3 for 64-bit CPUs

[Peter Maydell]

The regdef for SCTRL_EL3 was incorrectly marked as being an
ARM_CP_ALIAS, with the remark that this was because the 32-bit
definition would take care of reset and migration. However the
intention for banked registers as documented in the comment in
add_cpreg_to_hashtable() is:

 * 2) If ARMv8 is enabled then we can count on a 64-bit version
 *    taking care of the secure bank.  This requires that separate
 *    32 and 64-bit definitions are provided.

and so it marks the 32-bit secure banked version as an alias.
This results in the sctlr_s/sctlr_el[3] field never being reset
or migrated for a 64-bit CPU with EL3 enabled.

Fix this by removing the ARM_CP_ALIAS annotation from SCTLR_EL3.
Since this means it now needs a real reset value, move the regdef
into the same place that we define the 32-bit SCTLR.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/helper.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 19d5d52..e583e6a 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -3744,11 +3744,6 @@ static const ARMCPRegInfo el3_cp_reginfo[] = {
       .access = PL1_RW, .accessfn = access_trap_aa32s_el1,
       .writefn = vbar_write, .resetvalue = 0,
       .fieldoffset = offsetof(CPUARMState, cp15.mvbar) },
-    { .name = "SCTLR_EL3", .state = ARM_CP_STATE_AA64,
-      .type = ARM_CP_ALIAS, /* reset handled by AArch32 view */
-      .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 0, .opc2 = 0,
-      .access = PL3_RW, .raw_writefn = raw_write, .writefn = sctlr_write,
-      .fieldoffset = offsetof(CPUARMState, cp15.sctlr_el[3]) },
     { .name = "TTBR0_EL3", .state = ARM_CP_STATE_AA64,
       .opc0 = 3, .opc1 = 6, .crn = 2, .crm = 0, .opc2 = 0,
       .access = PL3_RW, .writefn = vmsa_ttbr_write, .resetvalue = 0,
@@ -4641,12 +4636,20 @@ void register_cp_regs_for_features(ARMCPU *cpu)
     }
     if (arm_feature(env, ARM_FEATURE_EL3)) {
         define_arm_cp_regs(cpu, el3_cp_reginfo);
-        ARMCPRegInfo rvbar = {
-            .name = "RVBAR_EL3", .state = ARM_CP_STATE_AA64,
-            .opc0 = 3, .opc1 = 6, .crn = 12, .crm = 0, .opc2 = 1,
-            .type = ARM_CP_CONST, .access = PL3_R, .resetvalue = cpu->rvbar
+        ARMCPRegInfo el3_regs[] = {
+            { .name = "RVBAR_EL3", .state = ARM_CP_STATE_AA64,
+              .opc0 = 3, .opc1 = 6, .crn = 12, .crm = 0, .opc2 = 1,
+              .type = ARM_CP_CONST, .access = PL3_R, .resetvalue = cpu->rvbar },
+            { .name = "SCTLR_EL3", .state = ARM_CP_STATE_AA64,
+              .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 0, .opc2 = 0,
+              .access = PL3_RW,
+              .raw_writefn = raw_write, .writefn = sctlr_write,
+              .fieldoffset = offsetof(CPUARMState, cp15.sctlr_el[3]),
+              .resetvalue = cpu->reset_sctlr },
+            REGINFO_SENTINEL
         };
-        define_one_arm_cp_reg(cpu, &rvbar);
+
+        define_arm_cp_regs(cpu, el3_regs);
     }
     /* The behaviour of NSACR is sufficiently various that we don't
      * try to describe it in a single reginfo:
-- 
1.9.1

Re: [Qemu-devel] [PATCH 1/4] target-arm: Correctly reset SCTLR_EL3 for 64-bit CPUs

[Laurent Desnogues]

On Thu, Mar 31, 2016 at 4:49 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> The regdef for SCTRL_EL3 was incorrectly marked as being an
> ARM_CP_ALIAS, with the remark that this was because the 32-bit
> definition would take care of reset and migration. However the
> intention for banked registers as documented in the comment in
> add_cpreg_to_hashtable() is:
>
>  * 2) If ARMv8 is enabled then we can count on a 64-bit version
>  *    taking care of the secure bank.  This requires that separate
>  *    32 and 64-bit definitions are provided.
>
> and so it marks the 32-bit secure banked version as an alias.
> This results in the sctlr_s/sctlr_el[3] field never being reset
> or migrated for a 64-bit CPU with EL3 enabled.
>
> Fix this by removing the ARM_CP_ALIAS annotation from SCTLR_EL3.
> Since this means it now needs a real reset value, move the regdef
> into the same place that we define the 32-bit SCTLR.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Laurent Desnogues <laurent.desnogues@gmail.com>

Thanks,

Laurent

> ---
>  target-arm/helper.c | 23 +++++++++++++----------
>  1 file changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 19d5d52..e583e6a 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -3744,11 +3744,6 @@ static const ARMCPRegInfo el3_cp_reginfo[] = {
>        .access = PL1_RW, .accessfn = access_trap_aa32s_el1,
>        .writefn = vbar_write, .resetvalue = 0,
>        .fieldoffset = offsetof(CPUARMState, cp15.mvbar) },
> -    { .name = "SCTLR_EL3", .state = ARM_CP_STATE_AA64,
> -      .type = ARM_CP_ALIAS, /* reset handled by AArch32 view */
> -      .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 0, .opc2 = 0,
> -      .access = PL3_RW, .raw_writefn = raw_write, .writefn = sctlr_write,
> -      .fieldoffset = offsetof(CPUARMState, cp15.sctlr_el[3]) },
>      { .name = "TTBR0_EL3", .state = ARM_CP_STATE_AA64,
>        .opc0 = 3, .opc1 = 6, .crn = 2, .crm = 0, .opc2 = 0,
>        .access = PL3_RW, .writefn = vmsa_ttbr_write, .resetvalue = 0,
> @@ -4641,12 +4636,20 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>      }
>      if (arm_feature(env, ARM_FEATURE_EL3)) {
>          define_arm_cp_regs(cpu, el3_cp_reginfo);
> -        ARMCPRegInfo rvbar = {
> -            .name = "RVBAR_EL3", .state = ARM_CP_STATE_AA64,
> -            .opc0 = 3, .opc1 = 6, .crn = 12, .crm = 0, .opc2 = 1,
> -            .type = ARM_CP_CONST, .access = PL3_R, .resetvalue = cpu->rvbar
> +        ARMCPRegInfo el3_regs[] = {
> +            { .name = "RVBAR_EL3", .state = ARM_CP_STATE_AA64,
> +              .opc0 = 3, .opc1 = 6, .crn = 12, .crm = 0, .opc2 = 1,
> +              .type = ARM_CP_CONST, .access = PL3_R, .resetvalue = cpu->rvbar },
> +            { .name = "SCTLR_EL3", .state = ARM_CP_STATE_AA64,
> +              .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 0, .opc2 = 0,
> +              .access = PL3_RW,
> +              .raw_writefn = raw_write, .writefn = sctlr_write,
> +              .fieldoffset = offsetof(CPUARMState, cp15.sctlr_el[3]),
> +              .resetvalue = cpu->reset_sctlr },
> +            REGINFO_SENTINEL
>          };
> -        define_one_arm_cp_reg(cpu, &rvbar);
> +
> +        define_arm_cp_regs(cpu, el3_regs);
>      }
>      /* The behaviour of NSACR is sufficiently various that we don't
>       * try to describe it in a single reginfo:
> --
> 1.9.1
>
>

Re: [Qemu-devel] [PATCH 1/4] target-arm: Correctly reset SCTLR_EL3 for 64-bit CPUs

[Sergey Fedorov]

On 31/03/16 17:49, Peter Maydell wrote:
> The regdef for SCTRL_EL3 was incorrectly marked as being an
> ARM_CP_ALIAS, with the remark that this was because the 32-bit
> definition would take care of reset and migration. However the
> intention for banked registers as documented in the comment in
> add_cpreg_to_hashtable() is:
>
>  * 2) If ARMv8 is enabled then we can count on a 64-bit version
>  *    taking care of the secure bank.  This requires that separate
>  *    32 and 64-bit definitions are provided.
>
> and so it marks the 32-bit secure banked version as an alias.
> This results in the sctlr_s/sctlr_el[3] field never being reset
> or migrated for a 64-bit CPU with EL3 enabled.
>
> Fix this by removing the ARM_CP_ALIAS annotation from SCTLR_EL3.
> Since this means it now needs a real reset value, move the regdef
> into the same place that we define the 32-bit SCTLR.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Sergey Fedorov <sergey.fedorov@linaro.org>

> ---
>  target-arm/helper.c | 23 +++++++++++++----------
>  1 file changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 19d5d52..e583e6a 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -3744,11 +3744,6 @@ static const ARMCPRegInfo el3_cp_reginfo[] = {
>        .access = PL1_RW, .accessfn = access_trap_aa32s_el1,
>        .writefn = vbar_write, .resetvalue = 0,
>        .fieldoffset = offsetof(CPUARMState, cp15.mvbar) },
> -    { .name = "SCTLR_EL3", .state = ARM_CP_STATE_AA64,
> -      .type = ARM_CP_ALIAS, /* reset handled by AArch32 view */
> -      .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 0, .opc2 = 0,
> -      .access = PL3_RW, .raw_writefn = raw_write, .writefn = sctlr_write,
> -      .fieldoffset = offsetof(CPUARMState, cp15.sctlr_el[3]) },
>      { .name = "TTBR0_EL3", .state = ARM_CP_STATE_AA64,
>        .opc0 = 3, .opc1 = 6, .crn = 2, .crm = 0, .opc2 = 0,
>        .access = PL3_RW, .writefn = vmsa_ttbr_write, .resetvalue = 0,
> @@ -4641,12 +4636,20 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>      }
>      if (arm_feature(env, ARM_FEATURE_EL3)) {
>          define_arm_cp_regs(cpu, el3_cp_reginfo);
> -        ARMCPRegInfo rvbar = {
> -            .name = "RVBAR_EL3", .state = ARM_CP_STATE_AA64,
> -            .opc0 = 3, .opc1 = 6, .crn = 12, .crm = 0, .opc2 = 1,
> -            .type = ARM_CP_CONST, .access = PL3_R, .resetvalue = cpu->rvbar
> +        ARMCPRegInfo el3_regs[] = {
> +            { .name = "RVBAR_EL3", .state = ARM_CP_STATE_AA64,
> +              .opc0 = 3, .opc1 = 6, .crn = 12, .crm = 0, .opc2 = 1,
> +              .type = ARM_CP_CONST, .access = PL3_R, .resetvalue = cpu->rvbar },
> +            { .name = "SCTLR_EL3", .state = ARM_CP_STATE_AA64,
> +              .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 0, .opc2 = 0,
> +              .access = PL3_RW,
> +              .raw_writefn = raw_write, .writefn = sctlr_write,
> +              .fieldoffset = offsetof(CPUARMState, cp15.sctlr_el[3]),
> +              .resetvalue = cpu->reset_sctlr },
> +            REGINFO_SENTINEL
>          };
> -        define_one_arm_cp_reg(cpu, &rvbar);
> +
> +        define_arm_cp_regs(cpu, el3_regs);
>      }
>      /* The behaviour of NSACR is sufficiently various that we don't
>       * try to describe it in a single reginfo:

[Qemu-devel] [PATCH 2/4] target-arm: Remove incorrect ALIAS tags from ESR_EL2 and ESR_EL3

[Peter Maydell]

The regdefs for the ESR_EL2 and ESR_EL3 system registers should not
be marked as ARM_CP_ALIAS, because these are the master copies; the
DFSR regdef in vmsa_pmsa_cp_reginfo[] is marked as an alias.
Remove the ALIAS tags so that these registers are correctly migrated.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/helper.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index e583e6a..0e54d90 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -3509,7 +3509,6 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
       .access = PL2_RW,
       .fieldoffset = offsetof(CPUARMState, elr_el[2]) },
     { .name = "ESR_EL2", .state = ARM_CP_STATE_AA64,
-      .type = ARM_CP_ALIAS,
       .opc0 = 3, .opc1 = 4, .crn = 5, .crm = 2, .opc2 = 0,
       .access = PL2_RW, .fieldoffset = offsetof(CPUARMState, cp15.esr_el[2]) },
     { .name = "FAR_EL2", .state = ARM_CP_STATE_AA64,
@@ -3759,7 +3758,6 @@ static const ARMCPRegInfo el3_cp_reginfo[] = {
       .access = PL3_RW,
       .fieldoffset = offsetof(CPUARMState, elr_el[3]) },
     { .name = "ESR_EL3", .state = ARM_CP_STATE_AA64,
-      .type = ARM_CP_ALIAS,
       .opc0 = 3, .opc1 = 6, .crn = 5, .crm = 2, .opc2 = 0,
       .access = PL3_RW, .fieldoffset = offsetof(CPUARMState, cp15.esr_el[3]) },
     { .name = "FAR_EL3", .state = ARM_CP_STATE_AA64,
-- 
1.9.1

Re: [Qemu-devel] [PATCH 2/4] target-arm: Remove incorrect ALIAS tags from ESR_EL2 and ESR_EL3

[Sergey Fedorov]

On 31/03/16 17:49, Peter Maydell wrote:
> The regdefs for the ESR_EL2 and ESR_EL3 system registers should not
> be marked as ARM_CP_ALIAS, because these are the master copies; the
> DFSR regdef in vmsa_pmsa_cp_reginfo[] is marked as an alias.
> Remove the ALIAS tags so that these registers are correctly migrated.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Sergey Fedorov <sergey.fedorov@linaro.rog>

> ---
>  target-arm/helper.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index e583e6a..0e54d90 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -3509,7 +3509,6 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
>        .access = PL2_RW,
>        .fieldoffset = offsetof(CPUARMState, elr_el[2]) },
>      { .name = "ESR_EL2", .state = ARM_CP_STATE_AA64,
> -      .type = ARM_CP_ALIAS,
>        .opc0 = 3, .opc1 = 4, .crn = 5, .crm = 2, .opc2 = 0,
>        .access = PL2_RW, .fieldoffset = offsetof(CPUARMState, cp15.esr_el[2]) },
>      { .name = "FAR_EL2", .state = ARM_CP_STATE_AA64,
> @@ -3759,7 +3758,6 @@ static const ARMCPRegInfo el3_cp_reginfo[] = {
>        .access = PL3_RW,
>        .fieldoffset = offsetof(CPUARMState, elr_el[3]) },
>      { .name = "ESR_EL3", .state = ARM_CP_STATE_AA64,
> -      .type = ARM_CP_ALIAS,
>        .opc0 = 3, .opc1 = 6, .crn = 5, .crm = 2, .opc2 = 0,
>        .access = PL3_RW, .fieldoffset = offsetof(CPUARMState, cp15.esr_el[3]) },
>      { .name = "FAR_EL3", .state = ARM_CP_STATE_AA64,

[Qemu-devel] [PATCH 3/4] target-arm: Make the 64-bit version of VTCR do the migration

[Peter Maydell]

Move the ALIAS tag from VTCR_EL2 to VTCR so that we migrate the
64-bit version, as is usual. (This has no particular effect now
unless the guest wrote to the high RES0 bits of VTCR_EL2.)
Add a comment about why it's OK that we don't have the various
accessor functions that the EL1 TCR regdefs do.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/helper.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 0e54d90..09638b2 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -3564,11 +3564,15 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
       .fieldoffset = offsetof(CPUARMState, cp15.tcr_el[2]) },
     { .name = "VTCR", .state = ARM_CP_STATE_AA32,
       .cp = 15, .opc1 = 4, .crn = 2, .crm = 1, .opc2 = 2,
+      .type = ARM_CP_ALIAS,
       .access = PL2_RW, .accessfn = access_el3_aa32ns,
       .fieldoffset = offsetof(CPUARMState, cp15.vtcr_el2) },
     { .name = "VTCR_EL2", .state = ARM_CP_STATE_AA64,
       .opc0 = 3, .opc1 = 4, .crn = 2, .crm = 1, .opc2 = 2,
-      .access = PL2_RW, .type = ARM_CP_ALIAS,
+      .access = PL2_RW,
+      /* no .writefn needed as this can't cause an ASID change;
+       * no .raw_writefn or .resetfn needed as we never use mask/base_mask
+       */
       .fieldoffset = offsetof(CPUARMState, cp15.vtcr_el2) },
     { .name = "VTTBR", .state = ARM_CP_STATE_AA32,
       .cp = 15, .opc1 = 6, .crm = 2,
-- 
1.9.1

Re: [Qemu-devel] [PATCH 3/4] target-arm: Make the 64-bit version of VTCR do the migration

[Sergey Fedorov]

On 31/03/16 17:49, Peter Maydell wrote:
> Move the ALIAS tag from VTCR_EL2 to VTCR so that we migrate the
> 64-bit version, as is usual. (This has no particular effect now
> unless the guest wrote to the high RES0 bits of VTCR_EL2.)
> Add a comment about why it's OK that we don't have the various
> accessor functions that the EL1 TCR regdefs do.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Sergey Fedorov <sergey.fedorov@linaro.org>

> ---
>  target-arm/helper.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 0e54d90..09638b2 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -3564,11 +3564,15 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
>        .fieldoffset = offsetof(CPUARMState, cp15.tcr_el[2]) },
>      { .name = "VTCR", .state = ARM_CP_STATE_AA32,
>        .cp = 15, .opc1 = 4, .crn = 2, .crm = 1, .opc2 = 2,
> +      .type = ARM_CP_ALIAS,
>        .access = PL2_RW, .accessfn = access_el3_aa32ns,
>        .fieldoffset = offsetof(CPUARMState, cp15.vtcr_el2) },
>      { .name = "VTCR_EL2", .state = ARM_CP_STATE_AA64,
>        .opc0 = 3, .opc1 = 4, .crn = 2, .crm = 1, .opc2 = 2,
> -      .access = PL2_RW, .type = ARM_CP_ALIAS,
> +      .access = PL2_RW,
> +      /* no .writefn needed as this can't cause an ASID change;
> +       * no .raw_writefn or .resetfn needed as we never use mask/base_mask
> +       */
>        .fieldoffset = offsetof(CPUARMState, cp15.vtcr_el2) },
>      { .name = "VTTBR", .state = ARM_CP_STATE_AA32,
>        .cp = 15, .opc1 = 6, .crm = 2,

[Qemu-devel] [PATCH 4/4] target-arm: Avoid unnecessary TLB flush on TCR_EL2 writes

[Peter Maydell]

The TCR_EL2 regdef was incorrectly using the vmsa_tcr_el1_write
function for writes. Since TCR_EL2 doesn't have the A1 bit that
TCR_EL1 does, we don't need to do a tlb_flush() when it is written.
Remove the unnecessary .writefn and also the harmless but unneeded
.raw_writefn and .resetfn definitions.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 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 09638b2..4dbd844 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -3559,8 +3559,10 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
       .resetvalue = 0 },
     { .name = "TCR_EL2", .state = ARM_CP_STATE_BOTH,
       .opc0 = 3, .opc1 = 4, .crn = 2, .crm = 0, .opc2 = 2,
-      .access = PL2_RW, .writefn = vmsa_tcr_el1_write,
-      .resetfn = vmsa_ttbcr_reset, .raw_writefn = raw_write,
+      .access = PL2_RW,
+      /* no .writefn needed as this can't cause an ASID change;
+       * no .raw_writefn or .resetfn needed as we never use mask/base_mask
+       */
       .fieldoffset = offsetof(CPUARMState, cp15.tcr_el[2]) },
     { .name = "VTCR", .state = ARM_CP_STATE_AA32,
       .cp = 15, .opc1 = 4, .crn = 2, .crm = 1, .opc2 = 2,
-- 
1.9.1

Re: [Qemu-devel] [PATCH 4/4] target-arm: Avoid unnecessary TLB flush on TCR_EL2 writes

[Sergey Fedorov]

On 31/03/16 17:49, Peter Maydell wrote:
> The TCR_EL2 regdef was incorrectly using the vmsa_tcr_el1_write
> function for writes. Since TCR_EL2 doesn't have the A1 bit that
> TCR_EL1 does, we don't need to do a tlb_flush() when it is written.
> Remove the unnecessary .writefn and also the harmless but unneeded
> .raw_writefn and .resetfn definitions.

How about TCR_EL3 which doesn't have A1 bit as well?

Kind regards,
Sergey

>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  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 09638b2..4dbd844 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -3559,8 +3559,10 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
>        .resetvalue = 0 },
>      { .name = "TCR_EL2", .state = ARM_CP_STATE_BOTH,
>        .opc0 = 3, .opc1 = 4, .crn = 2, .crm = 0, .opc2 = 2,
> -      .access = PL2_RW, .writefn = vmsa_tcr_el1_write,
> -      .resetfn = vmsa_ttbcr_reset, .raw_writefn = raw_write,
> +      .access = PL2_RW,
> +      /* no .writefn needed as this can't cause an ASID change;
> +       * no .raw_writefn or .resetfn needed as we never use mask/base_mask
> +       */
>        .fieldoffset = offsetof(CPUARMState, cp15.tcr_el[2]) },
>      { .name = "VTCR", .state = ARM_CP_STATE_AA32,
>        .cp = 15, .opc1 = 4, .crn = 2, .crm = 1, .opc2 = 2,

Re: [Qemu-devel] [PATCH 4/4] target-arm: Avoid unnecessary TLB flush on TCR_EL2 writes

[Peter Maydell]

On 4 April 2016 at 15:58, Sergey Fedorov <sergey.fedorov@linaro.org> wrote:
> On 31/03/16 17:49, Peter Maydell wrote:
>> The TCR_EL2 regdef was incorrectly using the vmsa_tcr_el1_write
>> function for writes. Since TCR_EL2 doesn't have the A1 bit that
>> TCR_EL1 does, we don't need to do a tlb_flush() when it is written.
>> Remove the unnecessary .writefn and also the harmless but unneeded
>> .raw_writefn and .resetfn definitions.
>
> How about TCR_EL3 which doesn't have A1 bit as well?

Yes, that should have this change too I think.

I'll put patches 1-3 into target-arm.next, and respin this one
(which then probably doesn't need to go into 2.6.)

thanks
-- PMM