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

Fabiano Rosas posted 19 patches 3 years, 1 month ago
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, "Alex Bennée" <alex.bennee@linaro.org>, Thomas Huth <thuth@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, Richard Henderson <richard.henderson@linaro.org>, Laurent Vivier <lvivier@redhat.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>
There is a newer version of this series
[RFC PATCH v2 07/19] target/arm: Move helper_set_pstate_* into cpregs.c
Posted by Fabiano Rosas 3 years, 1 month ago
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(-)

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;
-- 
2.35.3
Re: [RFC PATCH v2 07/19] target/arm: Move helper_set_pstate_* into cpregs.c
Posted by Richard Henderson 3 years, 1 month ago
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~
Re: [RFC PATCH v2 07/19] target/arm: Move helper_set_pstate_* into cpregs.c
Posted by Richard Henderson 3 years, 1 month ago
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~


Re: [RFC PATCH v2 07/19] target/arm: Move helper_set_pstate_* into cpregs.c
Posted by Fabiano Rosas 3 years, 1 month ago
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~
Re: [RFC PATCH v2 07/19] target/arm: Move helper_set_pstate_* into cpregs.c
Posted by Peter Maydell 3 years, 1 month ago
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