diff mbox series

[RFC,v2,07/19] target/arm: Move helper_set_pstate_* into cpregs.c

Message ID 20230109224232.11661-8-farosas@suse.de
State New
Headers show
Series target/arm: Allow CONFIG_TCG=n builds | expand

Commit Message

Fabiano Rosas Jan. 9, 2023, 10:42 p.m. UTC
We want to move sme_helper into the tcg directory, but the cpregs
accessor functions cannot go along, otherwise they would be separate
from the respective ARMCPRegInfo definition which needs to be compiled
with CONFIG_TCG=n as well.

Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 target/arm/cpregs.c     | 29 +++++++++++++++++++++++++++++
 target/arm/sme_helper.c | 29 -----------------------------
 2 files changed, 29 insertions(+), 29 deletions(-)

Comments

Richard Henderson Jan. 10, 2023, 5:52 a.m. UTC | #1
On 1/9/23 14:42, Fabiano Rosas wrote:
> We want to move sme_helper into the tcg directory, but the cpregs
> accessor functions cannot go along, otherwise they would be separate
> from the respective ARMCPRegInfo definition which needs to be compiled
> with CONFIG_TCG=n as well.

Hmm.  I would have hoped these could stay tcg-only, somehow.
I wonder if it warrants being an ARM_CP_SPECIAL_MASK value instead of svcr_write.


r~
Fabiano Rosas Jan. 10, 2023, 1:19 p.m. UTC | #2
Richard Henderson <richard.henderson@linaro.org> writes:

> On 1/9/23 14:42, Fabiano Rosas wrote:
>> We want to move sme_helper into the tcg directory, but the cpregs
>> accessor functions cannot go along, otherwise they would be separate
>> from the respective ARMCPRegInfo definition which needs to be compiled
>> with CONFIG_TCG=n as well.
>
> Hmm.  I would have hoped these could stay tcg-only, somehow.
> I wonder if it warrants being an ARM_CP_SPECIAL_MASK value instead of svcr_write.

In general, what characterizes as "special" for a register to use
ARM_CP_SPECIAL_MASK?

And specifically for svcr_write, it seems to me that
helper_set_pstate_{sm,za} and arm_reset_sve_state are straight-forward
enough that they could (if it made sense) be used by non-tcg code
(except they eventually call vfp_set_fpscr that still needs to be
split).

>
>
> r~
Peter Maydell Jan. 10, 2023, 2 p.m. UTC | #3
On Tue, 10 Jan 2023 at 13:19, Fabiano Rosas <farosas@suse.de> wrote:
>
> Richard Henderson <richard.henderson@linaro.org> writes:
>
> > On 1/9/23 14:42, Fabiano Rosas wrote:
> >> We want to move sme_helper into the tcg directory, but the cpregs
> >> accessor functions cannot go along, otherwise they would be separate
> >> from the respective ARMCPRegInfo definition which needs to be compiled
> >> with CONFIG_TCG=n as well.
> >
> > Hmm.  I would have hoped these could stay tcg-only, somehow.
> > I wonder if it warrants being an ARM_CP_SPECIAL_MASK value instead of svcr_write.
>
> In general, what characterizes as "special" for a register to use
> ARM_CP_SPECIAL_MASK?

It means that the register has special-case handling code
in the translate.c/translate-a64.c TCG codegen functions.
Non-special registers can be handled all in the same way
(load a constant, read a field from the CPU state struct,
or call a read/write function); the special cases directly
emit code to handle whatever they are doing. See the switch
commented "Handle special cases first" in target/arm/translate-a64.c
function handle_sys().

-- PMM
Richard Henderson Jan. 11, 2023, 3:48 a.m. UTC | #4
On 1/9/23 21:52, Richard Henderson wrote:
> On 1/9/23 14:42, Fabiano Rosas wrote:
>> We want to move sme_helper into the tcg directory, but the cpregs
>> accessor functions cannot go along, otherwise they would be separate
>> from the respective ARMCPRegInfo definition which needs to be compiled
>> with CONFIG_TCG=n as well.
> 
> Hmm.  I would have hoped these could stay tcg-only, somehow.
> I wonder if it warrants being an ARM_CP_SPECIAL_MASK value instead of svcr_write.

To answer my own question, ARM_CP_SPECIAL_MASK forces NO_RAW, which is not what we want 
for migration.

I'll think of something better here though.


r~
diff mbox series

Patch

diff --git a/target/arm/cpregs.c b/target/arm/cpregs.c
index 88d71dbe70..a6deae9926 100644
--- a/target/arm/cpregs.c
+++ b/target/arm/cpregs.c
@@ -6633,6 +6633,35 @@  static CPAccessResult access_esm(CPUARMState *env, const ARMCPRegInfo *ri,
     return CP_ACCESS_OK;
 }
 
+void helper_set_pstate_sm(CPUARMState *env, uint32_t i)
+{
+    if (i == FIELD_EX64(env->svcr, SVCR, SM)) {
+        return;
+    }
+    env->svcr ^= R_SVCR_SM_MASK;
+    arm_reset_sve_state(env);
+}
+
+void helper_set_pstate_za(CPUARMState *env, uint32_t i)
+{
+    if (i == FIELD_EX64(env->svcr, SVCR, ZA)) {
+        return;
+    }
+    env->svcr ^= R_SVCR_ZA_MASK;
+
+    /*
+     * ResetSMEState.
+     *
+     * SetPSTATE_ZA zeros on enable and disable.  We can zero this only
+     * on enable: while disabled, the storage is inaccessible and the
+     * value does not matter.  We're not saving the storage in vmstate
+     * when disabled either.
+     */
+    if (i) {
+        memset(env->zarray, 0, sizeof(env->zarray));
+    }
+}
+
 static void svcr_write(CPUARMState *env, const ARMCPRegInfo *ri,
                        uint64_t value)
 {
diff --git a/target/arm/sme_helper.c b/target/arm/sme_helper.c
index f891306bb9..ed04daf5d4 100644
--- a/target/arm/sme_helper.c
+++ b/target/arm/sme_helper.c
@@ -38,35 +38,6 @@  void arm_reset_sve_state(CPUARMState *env)
     vfp_set_fpcr(env, 0x0800009f);
 }
 
-void helper_set_pstate_sm(CPUARMState *env, uint32_t i)
-{
-    if (i == FIELD_EX64(env->svcr, SVCR, SM)) {
-        return;
-    }
-    env->svcr ^= R_SVCR_SM_MASK;
-    arm_reset_sve_state(env);
-}
-
-void helper_set_pstate_za(CPUARMState *env, uint32_t i)
-{
-    if (i == FIELD_EX64(env->svcr, SVCR, ZA)) {
-        return;
-    }
-    env->svcr ^= R_SVCR_ZA_MASK;
-
-    /*
-     * ResetSMEState.
-     *
-     * SetPSTATE_ZA zeros on enable and disable.  We can zero this only
-     * on enable: while disabled, the storage is inaccessible and the
-     * value does not matter.  We're not saving the storage in vmstate
-     * when disabled either.
-     */
-    if (i) {
-        memset(env->zarray, 0, sizeof(env->zarray));
-    }
-}
-
 void helper_sme_zero(CPUARMState *env, uint32_t imm, uint32_t svl)
 {
     uint32_t i;