[PATCH v2 07/37] target/arm: Add read_raw_cp_reg128, write_raw_cp_reg128

Richard Henderson posted 37 patches 1 month ago
Maintainers: Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>, Peter Maydell <peter.maydell@linaro.org>, Alexander Graf <agraf@csgraf.de>, Mads Ynddal <mads@ynddal.dk>, Paolo Bonzini <pbonzini@redhat.com>
[PATCH v2 07/37] target/arm: Add read_raw_cp_reg128, write_raw_cp_reg128
Posted by Richard Henderson 1 month ago
Add the functions and update raw_accessors_invalid to match.
Add assertions for !ARM_CP_128BIT in read_raw_cp_reg and
write_raw_cp_reg.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/cpregs.h |  1 +
 target/arm/helper.c | 49 +++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 46 insertions(+), 4 deletions(-)

diff --git a/target/arm/cpregs.h b/target/arm/cpregs.h
index 0b0004eff9..f6658abc57 100644
--- a/target/arm/cpregs.h
+++ b/target/arm/cpregs.h
@@ -1157,6 +1157,7 @@ static inline bool cp_access_ok(int current_el,
 
 /* Raw read of a coprocessor register (as needed for migration, etc) */
 uint64_t read_raw_cp_reg(CPUARMState *env, const ARMCPRegInfo *ri);
+Int128 read_raw_cp_reg128(CPUARMState *env, const ARMCPRegInfo *ri);
 
 /*
  * Return true if the cp register encoding is in the "feature ID space" as
diff --git a/target/arm/helper.c b/target/arm/helper.c
index e321f404e6..d9d8ae56e8 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -119,6 +119,7 @@ static void *raw_ptr(CPUARMState *env, const ARMCPRegInfo *ri)
 
 uint64_t read_raw_cp_reg(CPUARMState *env, const ARMCPRegInfo *ri)
 {
+    assert(!(ri->type & ARM_CP_128BIT));
     /* Raw read of a coprocessor register (as needed for migration, etc). */
     if (ri->type & ARM_CP_CONST) {
         return ri->resetvalue;
@@ -134,6 +135,7 @@ uint64_t read_raw_cp_reg(CPUARMState *env, const ARMCPRegInfo *ri)
 static void write_raw_cp_reg(CPUARMState *env, const ARMCPRegInfo *ri,
                              uint64_t v)
 {
+    assert(!(ri->type & ARM_CP_128BIT));
     /*
      * Raw write of a coprocessor register (as needed for migration, etc).
      * Note that constant registers are treated as write-ignored; the
@@ -151,6 +153,35 @@ static void write_raw_cp_reg(CPUARMState *env, const ARMCPRegInfo *ri,
     }
 }
 
+Int128 read_raw_cp_reg128(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+    assert(ri->type & ARM_CP_128BIT);
+    if (ri->raw_read128fn) {
+        return ri->raw_read128fn(env, ri);
+    } else if (ri->read128fn) {
+        return ri->read128fn(env, ri);
+    } else {
+        return raw_read128(env, ri);
+    }
+}
+
+__attribute__((unused))
+static void write_raw_cp_reg128(CPUARMState *env, const ARMCPRegInfo *ri,
+                                Int128 v)
+{
+    uint64_t lo = int128_getlo(v);
+    uint64_t hi = int128_gethi(v);
+
+    assert(ri->type & ARM_CP_128BIT);
+    if (ri->raw_write128fn) {
+        ri->raw_write128fn(env, ri, lo, hi);
+    } else if (ri->write128fn) {
+        ri->write128fn(env, ri, lo, hi);
+    } else {
+        raw_write128(env, ri, lo, hi);
+    }
+}
+
 static bool raw_accessors_invalid(const ARMCPRegInfo *ri)
 {
    /*
@@ -165,12 +196,22 @@ static bool raw_accessors_invalid(const ARMCPRegInfo *ri)
     * The tests here line up with the conditions in read/write_raw_cp_reg()
     * and assertions in raw_read()/raw_write().
     */
-    if ((ri->type & ARM_CP_CONST) ||
-        ri->fieldoffset ||
-        ((ri->raw_writefn || ri->writefn) && (ri->raw_readfn || ri->readfn))) {
+    if (ri->type & ARM_CP_CONST) {
         return false;
     }
-    return true;
+    if (ri->fieldoffset) {
+        return false;
+    }
+    if (ri->type & ARM_CP_128BIT) {
+        if (ri->fieldoffsethi) {
+            return false;
+        }
+        return !((ri->raw_write128fn || ri->write128fn) &&
+                 (ri->raw_read128fn || ri->read128fn));
+    } else {
+        return !((ri->raw_writefn || ri->writefn) &&
+                 (ri->raw_readfn || ri->readfn));
+    }
 }
 
 bool write_cpustate_to_list(ARMCPU *cpu, bool kvm_sync)
-- 
2.43.0
Re: [PATCH v2 07/37] target/arm: Add read_raw_cp_reg128, write_raw_cp_reg128
Posted by Peter Maydell 3 weeks, 4 days ago
On Tue, 14 Oct 2025 at 21:12, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Add the functions and update raw_accessors_invalid to match.
> Add assertions for !ARM_CP_128BIT in read_raw_cp_reg and
> write_raw_cp_reg.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/cpregs.h |  1 +
>  target/arm/helper.c | 49 +++++++++++++++++++++++++++++++++++++++++----
>  2 files changed, 46 insertions(+), 4 deletions(-)
>


Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM
Re: [PATCH v2 07/37] target/arm: Add read_raw_cp_reg128, write_raw_cp_reg128
Posted by Peter Maydell 4 weeks ago
On Tue, 14 Oct 2025 at 21:12, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Add the functions and update raw_accessors_invalid to match.
> Add assertions for !ARM_CP_128BIT in read_raw_cp_reg and
> write_raw_cp_reg.
>

> +__attribute__((unused))
> +static void write_raw_cp_reg128(CPUARMState *env, const ARMCPRegInfo *ri,
> +                                Int128 v)

Why does this one take an Int128 rather than a lo/hi pair?

> +{
> +    uint64_t lo = int128_getlo(v);
> +    uint64_t hi = int128_gethi(v);
> +
> +    assert(ri->type & ARM_CP_128BIT);
> +    if (ri->raw_write128fn) {
> +        ri->raw_write128fn(env, ri, lo, hi);
> +    } else if (ri->write128fn) {
> +        ri->write128fn(env, ri, lo, hi);
> +    } else {
> +        raw_write128(env, ri, lo, hi);
> +    }
> +}

thanks
-- PMM
Re: [PATCH v2 07/37] target/arm: Add read_raw_cp_reg128, write_raw_cp_reg128
Posted by Richard Henderson 4 weeks ago
On 10/17/25 06:11, Peter Maydell wrote:
> On Tue, 14 Oct 2025 at 21:12, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> Add the functions and update raw_accessors_invalid to match.
>> Add assertions for !ARM_CP_128BIT in read_raw_cp_reg and
>> write_raw_cp_reg.
>>
> 
>> +__attribute__((unused))
>> +static void write_raw_cp_reg128(CPUARMState *env, const ARMCPRegInfo *ri,
>> +                                Int128 v)
> 
> Why does this one take an Int128 rather than a lo/hi pair?

I still represent the migration stream as Int128.
See patches 12, 15, 16.

r~