* [Qemu-arm] [PATCH for-2.6 0/4] various regdef fixes for EL2/EL3 regs @ 2016-03-31 14:49 ` Peter Maydell 0 siblings, 0 replies; 22+ messages in thread From: Peter Maydell @ 2016-03-31 14:49 UTC (permalink / raw) To: qemu-arm, qemu-devel; +Cc: Sergey Fedorov 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH for-2.6 0/4] various regdef fixes for EL2/EL3 regs @ 2016-03-31 14:49 ` Peter Maydell 0 siblings, 0 replies; 22+ messages in thread From: Peter Maydell @ 2016-03-31 14:49 UTC (permalink / raw) To: qemu-arm, qemu-devel; +Cc: Edgar E. Iglesias, Sergey Fedorov 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-arm] [PATCH 1/4] target-arm: Correctly reset SCTLR_EL3 for 64-bit CPUs 2016-03-31 14:49 ` [Qemu-devel] " Peter Maydell @ 2016-03-31 14:49 ` Peter Maydell -1 siblings, 0 replies; 22+ messages in thread From: Peter Maydell @ 2016-03-31 14:49 UTC (permalink / raw) To: qemu-arm, qemu-devel; +Cc: Sergey Fedorov 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 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH 1/4] target-arm: Correctly reset SCTLR_EL3 for 64-bit CPUs @ 2016-03-31 14:49 ` Peter Maydell 0 siblings, 0 replies; 22+ messages in thread From: Peter Maydell @ 2016-03-31 14:49 UTC (permalink / raw) To: qemu-arm, qemu-devel; +Cc: Edgar E. Iglesias, Sergey Fedorov 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 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [Qemu-arm] [Qemu-devel] [PATCH 1/4] target-arm: Correctly reset SCTLR_EL3 for 64-bit CPUs 2016-03-31 14:49 ` [Qemu-devel] " Peter Maydell @ 2016-03-31 14:59 ` Laurent Desnogues -1 siblings, 0 replies; 22+ messages in thread From: Laurent Desnogues @ 2016-03-31 14:59 UTC (permalink / raw) To: Peter Maydell; +Cc: qemu-arm, qemu-devel@nongnu.org, Sergey Fedorov 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 > > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] target-arm: Correctly reset SCTLR_EL3 for 64-bit CPUs @ 2016-03-31 14:59 ` Laurent Desnogues 0 siblings, 0 replies; 22+ messages in thread From: Laurent Desnogues @ 2016-03-31 14:59 UTC (permalink / raw) To: Peter Maydell Cc: Edgar E. Iglesias, qemu-arm, qemu-devel@nongnu.org, Sergey Fedorov 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 > > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-arm] [PATCH 1/4] target-arm: Correctly reset SCTLR_EL3 for 64-bit CPUs 2016-03-31 14:49 ` [Qemu-devel] " Peter Maydell @ 2016-04-04 14:13 ` Sergey Fedorov -1 siblings, 0 replies; 22+ messages in thread From: Sergey Fedorov @ 2016-04-04 14:13 UTC (permalink / raw) To: Peter Maydell, qemu-arm, qemu-devel 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: ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] target-arm: Correctly reset SCTLR_EL3 for 64-bit CPUs @ 2016-04-04 14:13 ` Sergey Fedorov 0 siblings, 0 replies; 22+ messages in thread From: Sergey Fedorov @ 2016-04-04 14:13 UTC (permalink / raw) To: Peter Maydell, qemu-arm, qemu-devel; +Cc: Edgar E. Iglesias 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: ^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-arm] [PATCH 2/4] target-arm: Remove incorrect ALIAS tags from ESR_EL2 and ESR_EL3 2016-03-31 14:49 ` [Qemu-devel] " Peter Maydell @ 2016-03-31 14:49 ` Peter Maydell -1 siblings, 0 replies; 22+ messages in thread From: Peter Maydell @ 2016-03-31 14:49 UTC (permalink / raw) To: qemu-arm, qemu-devel; +Cc: Sergey Fedorov 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 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH 2/4] target-arm: Remove incorrect ALIAS tags from ESR_EL2 and ESR_EL3 @ 2016-03-31 14:49 ` Peter Maydell 0 siblings, 0 replies; 22+ messages in thread From: Peter Maydell @ 2016-03-31 14:49 UTC (permalink / raw) To: qemu-arm, qemu-devel; +Cc: Edgar E. Iglesias, Sergey Fedorov 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 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [Qemu-arm] [PATCH 2/4] target-arm: Remove incorrect ALIAS tags from ESR_EL2 and ESR_EL3 2016-03-31 14:49 ` [Qemu-devel] " Peter Maydell @ 2016-04-04 14:16 ` Sergey Fedorov -1 siblings, 0 replies; 22+ messages in thread From: Sergey Fedorov @ 2016-04-04 14:16 UTC (permalink / raw) To: Peter Maydell, qemu-arm, qemu-devel 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, ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 2/4] target-arm: Remove incorrect ALIAS tags from ESR_EL2 and ESR_EL3 @ 2016-04-04 14:16 ` Sergey Fedorov 0 siblings, 0 replies; 22+ messages in thread From: Sergey Fedorov @ 2016-04-04 14:16 UTC (permalink / raw) To: Peter Maydell, qemu-arm, qemu-devel; +Cc: Edgar E. Iglesias 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, ^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-arm] [PATCH 3/4] target-arm: Make the 64-bit version of VTCR do the migration 2016-03-31 14:49 ` [Qemu-devel] " Peter Maydell @ 2016-03-31 14:49 ` Peter Maydell -1 siblings, 0 replies; 22+ messages in thread From: Peter Maydell @ 2016-03-31 14:49 UTC (permalink / raw) To: qemu-arm, qemu-devel; +Cc: Sergey Fedorov 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 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH 3/4] target-arm: Make the 64-bit version of VTCR do the migration @ 2016-03-31 14:49 ` Peter Maydell 0 siblings, 0 replies; 22+ messages in thread From: Peter Maydell @ 2016-03-31 14:49 UTC (permalink / raw) To: qemu-arm, qemu-devel; +Cc: Edgar E. Iglesias, Sergey Fedorov 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 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [Qemu-arm] [PATCH 3/4] target-arm: Make the 64-bit version of VTCR do the migration 2016-03-31 14:49 ` [Qemu-devel] " Peter Maydell @ 2016-04-04 14:22 ` Sergey Fedorov -1 siblings, 0 replies; 22+ messages in thread From: Sergey Fedorov @ 2016-04-04 14:22 UTC (permalink / raw) To: Peter Maydell, qemu-arm, qemu-devel 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, ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 3/4] target-arm: Make the 64-bit version of VTCR do the migration @ 2016-04-04 14:22 ` Sergey Fedorov 0 siblings, 0 replies; 22+ messages in thread From: Sergey Fedorov @ 2016-04-04 14:22 UTC (permalink / raw) To: Peter Maydell, qemu-arm, qemu-devel; +Cc: Edgar E. Iglesias 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, ^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-arm] [PATCH 4/4] target-arm: Avoid unnecessary TLB flush on TCR_EL2 writes 2016-03-31 14:49 ` [Qemu-devel] " Peter Maydell @ 2016-03-31 14:49 ` Peter Maydell -1 siblings, 0 replies; 22+ messages in thread From: Peter Maydell @ 2016-03-31 14:49 UTC (permalink / raw) To: qemu-arm, qemu-devel; +Cc: Sergey Fedorov 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 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH 4/4] target-arm: Avoid unnecessary TLB flush on TCR_EL2 writes @ 2016-03-31 14:49 ` Peter Maydell 0 siblings, 0 replies; 22+ messages in thread From: Peter Maydell @ 2016-03-31 14:49 UTC (permalink / raw) To: qemu-arm, qemu-devel; +Cc: Edgar E. Iglesias, Sergey Fedorov 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 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [Qemu-arm] [PATCH 4/4] target-arm: Avoid unnecessary TLB flush on TCR_EL2 writes 2016-03-31 14:49 ` [Qemu-devel] " Peter Maydell @ 2016-04-04 14:58 ` Sergey Fedorov -1 siblings, 0 replies; 22+ messages in thread From: Sergey Fedorov @ 2016-04-04 14:58 UTC (permalink / raw) To: Peter Maydell, qemu-arm, qemu-devel 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, ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 4/4] target-arm: Avoid unnecessary TLB flush on TCR_EL2 writes @ 2016-04-04 14:58 ` Sergey Fedorov 0 siblings, 0 replies; 22+ messages in thread From: Sergey Fedorov @ 2016-04-04 14:58 UTC (permalink / raw) To: Peter Maydell, qemu-arm, qemu-devel; +Cc: Edgar E. Iglesias 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, ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-arm] [PATCH 4/4] target-arm: Avoid unnecessary TLB flush on TCR_EL2 writes 2016-04-04 14:58 ` [Qemu-devel] " Sergey Fedorov @ 2016-04-04 15:01 ` Peter Maydell -1 siblings, 0 replies; 22+ messages in thread From: Peter Maydell @ 2016-04-04 15:01 UTC (permalink / raw) To: Sergey Fedorov; +Cc: qemu-arm, QEMU Developers 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 4/4] target-arm: Avoid unnecessary TLB flush on TCR_EL2 writes @ 2016-04-04 15:01 ` Peter Maydell 0 siblings, 0 replies; 22+ messages in thread From: Peter Maydell @ 2016-04-04 15:01 UTC (permalink / raw) To: Sergey Fedorov; +Cc: Edgar E. Iglesias, qemu-arm, QEMU Developers 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2016-04-04 15:01 UTC | newest] Thread overview: 22+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-03-31 14:49 [Qemu-arm] [PATCH for-2.6 0/4] various regdef fixes for EL2/EL3 regs Peter Maydell 2016-03-31 14:49 ` [Qemu-devel] " Peter Maydell 2016-03-31 14:49 ` [Qemu-arm] [PATCH 1/4] target-arm: Correctly reset SCTLR_EL3 for 64-bit CPUs Peter Maydell 2016-03-31 14:49 ` [Qemu-devel] " Peter Maydell 2016-03-31 14:59 ` [Qemu-arm] " Laurent Desnogues 2016-03-31 14:59 ` Laurent Desnogues 2016-04-04 14:13 ` [Qemu-arm] " Sergey Fedorov 2016-04-04 14:13 ` [Qemu-devel] " Sergey Fedorov 2016-03-31 14:49 ` [Qemu-arm] [PATCH 2/4] target-arm: Remove incorrect ALIAS tags from ESR_EL2 and ESR_EL3 Peter Maydell 2016-03-31 14:49 ` [Qemu-devel] " Peter Maydell 2016-04-04 14:16 ` [Qemu-arm] " Sergey Fedorov 2016-04-04 14:16 ` [Qemu-devel] " Sergey Fedorov 2016-03-31 14:49 ` [Qemu-arm] [PATCH 3/4] target-arm: Make the 64-bit version of VTCR do the migration Peter Maydell 2016-03-31 14:49 ` [Qemu-devel] " Peter Maydell 2016-04-04 14:22 ` [Qemu-arm] " Sergey Fedorov 2016-04-04 14:22 ` [Qemu-devel] " Sergey Fedorov 2016-03-31 14:49 ` [Qemu-arm] [PATCH 4/4] target-arm: Avoid unnecessary TLB flush on TCR_EL2 writes Peter Maydell 2016-03-31 14:49 ` [Qemu-devel] " Peter Maydell 2016-04-04 14:58 ` [Qemu-arm] " Sergey Fedorov 2016-04-04 14:58 ` [Qemu-devel] " Sergey Fedorov 2016-04-04 15:01 ` [Qemu-arm] " Peter Maydell 2016-04-04 15:01 ` [Qemu-devel] " Peter Maydell
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.