* [PATCH v3 2/4] target/arm: Correct store of FPSCR value via FPCXT_S
2020-12-10 20:14 [PATCH v3 0/4] target/arm: Implement v8.1M and Cortex-M55 Peter Maydell
2020-12-10 20:14 ` [PATCH v3 1/4] hw/intc/armv7m_nvic: Correct handling of CCR.BFHFNMIGN Peter Maydell
@ 2020-12-10 20:14 ` Peter Maydell
2020-12-16 21:19 ` Richard Henderson
2020-12-10 20:14 ` [PATCH v3 3/4] target/arm: Implement FPCXT_NS fp system register Peter Maydell
2020-12-10 20:14 ` [PATCH v3 4/4] target/arm: Implement Cortex-M55 model Peter Maydell
3 siblings, 1 reply; 9+ messages in thread
From: Peter Maydell @ 2020-12-10 20:14 UTC (permalink / raw)
To: qemu-devel
In commit 64f863baeedc8659 we implemented the v8.1M FPCXT_S register,
but we got the write behaviour wrong. On read, this register reads
bits [27:0] of FPSCR plus the CONTROL.SFPA bit. On write, it doesn't
just write back those bits -- it writes a value to the whole FPSCR,
whose upper 4 bits are zeroes.
We also incorrectly implemented the write-to-FPSCR as a simple store
to vfp.xregs; this skips the "update the softfloat flags" part of
the vfp_set_fpscr helper so the value would read back correctly but
not actually take effect.
Fix both of these things by doing a complete write to the FPSCR
using the helper function.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target/arm/translate-vfp.c.inc | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/target/arm/translate-vfp.c.inc b/target/arm/translate-vfp.c.inc
index 0db936084bd..8b4cfd68cad 100644
--- a/target/arm/translate-vfp.c.inc
+++ b/target/arm/translate-vfp.c.inc
@@ -723,8 +723,11 @@ static bool gen_M_fp_sysreg_write(DisasContext *s, int regno,
}
case ARM_VFP_FPCXT_S:
{
- TCGv_i32 sfpa, control, fpscr;
- /* Set FPSCR[27:0] and CONTROL.SFPA from value */
+ TCGv_i32 sfpa, control;
+ /*
+ * Set FPSCR and CONTROL.SFPA from value; the new FPSCR takes
+ * bits [27:0] from value and zeroes bits [31:28].
+ */
tmp = loadfn(s, opaque);
sfpa = tcg_temp_new_i32();
tcg_gen_shri_i32(sfpa, tmp, 31);
@@ -732,11 +735,8 @@ static bool gen_M_fp_sysreg_write(DisasContext *s, int regno,
tcg_gen_deposit_i32(control, control, sfpa,
R_V7M_CONTROL_SFPA_SHIFT, 1);
store_cpu_field(control, v7m.control[M_REG_S]);
- fpscr = load_cpu_field(vfp.xregs[ARM_VFP_FPSCR]);
- tcg_gen_andi_i32(fpscr, fpscr, FPCR_NZCV_MASK);
tcg_gen_andi_i32(tmp, tmp, ~FPCR_NZCV_MASK);
- tcg_gen_or_i32(fpscr, fpscr, tmp);
- store_cpu_field(fpscr, vfp.xregs[ARM_VFP_FPSCR]);
+ gen_helper_vfp_set_fpscr(cpu_env, tmp);
tcg_temp_free_i32(tmp);
tcg_temp_free_i32(sfpa);
break;
--
2.20.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v3 3/4] target/arm: Implement FPCXT_NS fp system register
2020-12-10 20:14 [PATCH v3 0/4] target/arm: Implement v8.1M and Cortex-M55 Peter Maydell
2020-12-10 20:14 ` [PATCH v3 1/4] hw/intc/armv7m_nvic: Correct handling of CCR.BFHFNMIGN Peter Maydell
2020-12-10 20:14 ` [PATCH v3 2/4] target/arm: Correct store of FPSCR value via FPCXT_S Peter Maydell
@ 2020-12-10 20:14 ` Peter Maydell
2020-12-16 21:29 ` Richard Henderson
2020-12-10 20:14 ` [PATCH v3 4/4] target/arm: Implement Cortex-M55 model Peter Maydell
3 siblings, 1 reply; 9+ messages in thread
From: Peter Maydell @ 2020-12-10 20:14 UTC (permalink / raw)
To: qemu-devel
Implement the v8.1M FPCXT_NS floating-point system register. This is
a little more complicated than FPCXT_S, because it has specific
handling for "current FP state is inactive", and it only wants to do
PreserveFPState(), not the full set of actions done by
ExecuteFPCheck() which vfp_access_check() implements.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
Changes since v2: refactored along lines suggested by RTH
---
target/arm/translate-vfp.c.inc | 102 ++++++++++++++++++++++++++++++++-
1 file changed, 99 insertions(+), 3 deletions(-)
diff --git a/target/arm/translate-vfp.c.inc b/target/arm/translate-vfp.c.inc
index 8b4cfd68cad..10766f210c1 100644
--- a/target/arm/translate-vfp.c.inc
+++ b/target/arm/translate-vfp.c.inc
@@ -663,6 +663,7 @@ static FPSysRegCheckResult fp_sysreg_checks(DisasContext *s, int regno)
}
break;
case ARM_VFP_FPCXT_S:
+ case ARM_VFP_FPCXT_NS:
if (!arm_dc_feature(s, ARM_FEATURE_V8_1M)) {
return false;
}
@@ -674,13 +675,48 @@ static FPSysRegCheckResult fp_sysreg_checks(DisasContext *s, int regno)
return FPSysRegCheckFailed;
}
- if (!vfp_access_check(s)) {
+ /*
+ * FPCXT_NS is a special case: it has specific handling for
+ * "current FP state is inactive", and must do the PreserveFPState()
+ * but not the usual full set of actions done by ExecuteFPCheck().
+ * So we don't call vfp_access_check() and the callers must handle this.
+ */
+ if (regno != ARM_VFP_FPCXT_NS && !vfp_access_check(s)) {
return FPSysRegCheckDone;
}
-
return FPSysRegCheckContinue;
}
+static void gen_branch_fpInactive(DisasContext *s, TCGCond cond,
+ TCGLabel *label)
+{
+ /*
+ * FPCXT_NS is a special case: it has specific handling for
+ * "current FP state is inactive", and must do the PreserveFPState()
+ * but not the usual full set of actions done by ExecuteFPCheck().
+ * We don't have a TB flag that matches the fpInactive check, so we
+ * do it at runtime as we don't expect FPCXT_NS accesses to be frequent.
+ *
+ * Emit code that checks fpInactive and does a conditional
+ * branch to label based on it:
+ * if cond is TCG_COND_NE then branch if fpInactive != 0 (ie if inactive)
+ * if cond is TCG_COND_EQ then branch if fpInactive == 0 (ie if active)
+ */
+ assert(cond == TCG_COND_EQ || cond == TCG_COND_NE);
+
+ /* fpInactive = FPCCR_NS.ASPEN == 1 && CONTROL.FPCA == 0 */
+ TCGv_i32 aspen, fpca;
+ aspen = load_cpu_field(v7m.fpccr[M_REG_NS]);
+ fpca = load_cpu_field(v7m.control[M_REG_S]);
+ tcg_gen_andi_i32(aspen, aspen, R_V7M_FPCCR_ASPEN_MASK);
+ tcg_gen_xori_i32(aspen, aspen, R_V7M_FPCCR_ASPEN_MASK);
+ tcg_gen_andi_i32(fpca, fpca, R_V7M_CONTROL_FPCA_MASK);
+ tcg_gen_or_i32(fpca, fpca, aspen);
+ tcg_gen_brcondi_i32(tcg_invert_cond(cond), fpca, 0, label);
+ tcg_temp_free_i32(aspen);
+ tcg_temp_free_i32(fpca);
+}
+
static bool gen_M_fp_sysreg_write(DisasContext *s, int regno,
fp_sysreg_loadfn *loadfn,
@@ -688,6 +724,7 @@ static bool gen_M_fp_sysreg_write(DisasContext *s, int regno,
{
/* Do a write to an M-profile floating point system register */
TCGv_i32 tmp;
+ TCGLabel *lab_end = NULL;
switch (fp_sysreg_checks(s, regno)) {
case FPSysRegCheckFailed:
@@ -721,6 +758,13 @@ static bool gen_M_fp_sysreg_write(DisasContext *s, int regno,
tcg_temp_free_i32(tmp);
break;
}
+ case ARM_VFP_FPCXT_NS:
+ lab_end = gen_new_label();
+ /* fpInactive case: write is a NOP, so branch to end */
+ gen_branch_fpInactive(s, TCG_COND_NE, lab_end);
+ /* !fpInactive: PreserveFPState(), and reads same as FPCXT_S */
+ gen_preserve_fp_state(s);
+ /* fall through */
case ARM_VFP_FPCXT_S:
{
TCGv_i32 sfpa, control;
@@ -744,6 +788,9 @@ static bool gen_M_fp_sysreg_write(DisasContext *s, int regno,
default:
g_assert_not_reached();
}
+ if (lab_end) {
+ gen_set_label(lab_end);
+ }
return true;
}
@@ -753,6 +800,8 @@ static bool gen_M_fp_sysreg_read(DisasContext *s, int regno,
{
/* Do a read from an M-profile floating point system register */
TCGv_i32 tmp;
+ TCGLabel *lab_end = NULL;
+ bool lookup_tb = false;
switch (fp_sysreg_checks(s, regno)) {
case FPSysRegCheckFailed:
@@ -811,12 +860,59 @@ static bool gen_M_fp_sysreg_read(DisasContext *s, int regno,
fpscr = load_cpu_field(v7m.fpdscr[M_REG_NS]);
gen_helper_vfp_set_fpscr(cpu_env, fpscr);
tcg_temp_free_i32(fpscr);
- gen_lookup_tb(s);
+ lookup_tb = true;
+ break;
+ }
+ case ARM_VFP_FPCXT_NS:
+ {
+ TCGv_i32 control, sfpa, fpscr, fpdscr, zero;
+ TCGLabel *lab_active = gen_new_label();
+
+ lookup_tb = true;
+
+ gen_branch_fpInactive(s, TCG_COND_EQ, lab_active);
+ /* fpInactive case: reads as FPDSCR_NS */
+ TCGv_i32 tmp = load_cpu_field(v7m.fpdscr[M_REG_NS]);
+ storefn(s, opaque, tmp);
+ lab_end = gen_new_label();
+ tcg_gen_br(lab_end);
+
+ gen_set_label(lab_active);
+ /* !fpInactive: Reads the same as FPCXT_S, but side effects differ */
+ gen_preserve_fp_state(s);
+ tmp = tcg_temp_new_i32();
+ sfpa = tcg_temp_new_i32();
+ fpscr = tcg_temp_new_i32();
+ gen_helper_vfp_get_fpscr(fpscr, cpu_env);
+ tcg_gen_andi_i32(tmp, fpscr, ~FPCR_NZCV_MASK);
+ control = load_cpu_field(v7m.control[M_REG_S]);
+ tcg_gen_andi_i32(sfpa, control, R_V7M_CONTROL_SFPA_MASK);
+ tcg_gen_shli_i32(sfpa, sfpa, 31 - R_V7M_CONTROL_SFPA_SHIFT);
+ tcg_gen_or_i32(tmp, tmp, sfpa);
+ tcg_temp_free_i32(control);
+ /* Store result before updating FPSCR, in case it faults */
+ storefn(s, opaque, tmp);
+ /* If SFPA is zero then set FPSCR from FPDSCR_NS */
+ fpdscr = load_cpu_field(v7m.fpdscr[M_REG_NS]);
+ zero = tcg_const_i32(0);
+ tcg_gen_movcond_i32(TCG_COND_EQ, fpscr, sfpa, zero, fpdscr, fpscr);
+ gen_helper_vfp_set_fpscr(cpu_env, fpscr);
+ tcg_temp_free_i32(zero);
+ tcg_temp_free_i32(sfpa);
+ tcg_temp_free_i32(fpdscr);
+ tcg_temp_free_i32(fpscr);
break;
}
default:
g_assert_not_reached();
}
+
+ if (lab_end) {
+ gen_set_label(lab_end);
+ }
+ if (lookup_tb) {
+ gen_lookup_tb(s);
+ }
return true;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v3 4/4] target/arm: Implement Cortex-M55 model
2020-12-10 20:14 [PATCH v3 0/4] target/arm: Implement v8.1M and Cortex-M55 Peter Maydell
` (2 preceding siblings ...)
2020-12-10 20:14 ` [PATCH v3 3/4] target/arm: Implement FPCXT_NS fp system register Peter Maydell
@ 2020-12-10 20:14 ` Peter Maydell
2020-12-16 21:30 ` Richard Henderson
3 siblings, 1 reply; 9+ messages in thread
From: Peter Maydell @ 2020-12-10 20:14 UTC (permalink / raw)
To: qemu-devel
Now that we have implemented all the features needed by the v8.1M
architecture, we can add the model of the Cortex-M55. This is the
configuration without MVE support; we'll add MVE later.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target/arm/cpu_tcg.c | 42 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/target/arm/cpu_tcg.c b/target/arm/cpu_tcg.c
index 0013e25412f..98544db2df3 100644
--- a/target/arm/cpu_tcg.c
+++ b/target/arm/cpu_tcg.c
@@ -401,6 +401,46 @@ static void cortex_m33_initfn(Object *obj)
cpu->ctr = 0x8000c000;
}
+static void cortex_m55_initfn(Object *obj)
+{
+ ARMCPU *cpu = ARM_CPU(obj);
+
+ set_feature(&cpu->env, ARM_FEATURE_V8);
+ set_feature(&cpu->env, ARM_FEATURE_V8_1M);
+ set_feature(&cpu->env, ARM_FEATURE_M);
+ set_feature(&cpu->env, ARM_FEATURE_M_MAIN);
+ set_feature(&cpu->env, ARM_FEATURE_M_SECURITY);
+ set_feature(&cpu->env, ARM_FEATURE_THUMB_DSP);
+ cpu->midr = 0x410fd221; /* r0p1 */
+ cpu->revidr = 0;
+ cpu->pmsav7_dregion = 16;
+ cpu->sau_sregion = 8;
+ /*
+ * These are the MVFR* values for the FPU, no MVE configuration;
+ * we will update them later when we implement MVE
+ */
+ cpu->isar.mvfr0 = 0x10110221;
+ cpu->isar.mvfr1 = 0x12100011;
+ cpu->isar.mvfr2 = 0x00000040;
+ cpu->isar.id_pfr0 = 0x20000030;
+ cpu->isar.id_pfr1 = 0x00000230;
+ cpu->isar.id_dfr0 = 0x10200000;
+ cpu->id_afr0 = 0x00000000;
+ cpu->isar.id_mmfr0 = 0x00111040;
+ cpu->isar.id_mmfr1 = 0x00000000;
+ cpu->isar.id_mmfr2 = 0x01000000;
+ cpu->isar.id_mmfr3 = 0x00000011;
+ cpu->isar.id_isar0 = 0x01103110;
+ cpu->isar.id_isar1 = 0x02212000;
+ cpu->isar.id_isar2 = 0x20232232;
+ cpu->isar.id_isar3 = 0x01111131;
+ cpu->isar.id_isar4 = 0x01310132;
+ cpu->isar.id_isar5 = 0x00000000;
+ cpu->isar.id_isar6 = 0x00000000;
+ cpu->clidr = 0x00000000; /* caches not implemented */
+ cpu->ctr = 0x8303c003;
+}
+
static const ARMCPRegInfo cortexr5_cp_reginfo[] = {
/* Dummy the TCM region regs for the moment */
{ .name = "ATCM", .cp = 15, .opc1 = 0, .crn = 9, .crm = 1, .opc2 = 0,
@@ -655,6 +695,8 @@ static const ARMCPUInfo arm_tcg_cpus[] = {
.class_init = arm_v7m_class_init },
{ .name = "cortex-m33", .initfn = cortex_m33_initfn,
.class_init = arm_v7m_class_init },
+ { .name = "cortex-m55", .initfn = cortex_m55_initfn,
+ .class_init = arm_v7m_class_init },
{ .name = "cortex-r5", .initfn = cortex_r5_initfn },
{ .name = "cortex-r5f", .initfn = cortex_r5f_initfn },
{ .name = "ti925t", .initfn = ti925t_initfn },
--
2.20.1
^ permalink raw reply related [flat|nested] 9+ messages in thread