[PATCH 0/3] accel/kvm: Cleanups around kvm_arch_put_registers()

[PATCH 0/3] accel/kvm: Cleanups around kvm_arch_put_registers()

[Philippe Mathieu-Daudé]

Extracted from a bigger series aiming to make accelerator
synchronization of vcpu state slightly clearer. Here KVM
patches around kvm_arch_put_registers():
- Move KVM_PUT_[RESET|RUNTIME|FULL]_STATE to an enum
- Factor common code out of kvm_cpu_synchronize_post_*()

Philippe Mathieu-Daudé (3):
  accel/kvm: Do not expect more then KVM_PUT_FULL_STATE
  accel/kvm: Introduce KvmPutState enum
  accel/kvm: Factor kvm_cpu_synchronize_put() out

 include/system/kvm.h       | 16 +++++++------
 accel/kvm/kvm-all.c        | 47 +++++++++++++++-----------------------
 target/i386/kvm/kvm.c      |  6 ++---
 target/loongarch/kvm/kvm.c |  8 +++----
 target/mips/kvm.c          |  6 ++---
 target/ppc/kvm.c           |  2 +-
 target/riscv/kvm/kvm-cpu.c |  2 +-
 target/s390x/kvm/kvm.c     |  2 +-
 8 files changed, 41 insertions(+), 48 deletions(-)

-- 
2.51.0

[PATCH 1/3] accel/kvm: Do not expect more then KVM_PUT_FULL_STATE

[Philippe Mathieu-Daudé]

KVM_PUT_FULL_STATE is the higher level defined so far in
"system/kvm.h"; do not check for more.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/loongarch/kvm/kvm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/loongarch/kvm/kvm.c b/target/loongarch/kvm/kvm.c
index e5ea2dba9da..45292edcb1c 100644
--- a/target/loongarch/kvm/kvm.c
+++ b/target/loongarch/kvm/kvm.c
@@ -397,7 +397,7 @@ static int kvm_loongarch_put_csr(CPUState *cs, int level)
                            &env->CSR_RVACFG);
 
     /* CPUID is constant after poweron, it should be set only once */
-    if (level >= KVM_PUT_FULL_STATE) {
+    if (level == KVM_PUT_FULL_STATE) {
         ret |= kvm_set_one_reg(cs, KVM_IOC_CSRID(LOONGARCH_CSR_CPUID),
                            &env->CSR_CPUID);
     }
@@ -801,7 +801,7 @@ int kvm_arch_put_registers(CPUState *cs, int level, Error **errp)
         once = 1;
     }
 
-    if (level >= KVM_PUT_FULL_STATE) {
+    if (level == KVM_PUT_FULL_STATE) {
         /*
          * only KVM_PUT_FULL_STATE is required, kvm kernel will clear
          * guest_addr for KVM_PUT_RESET_STATE
-- 
2.51.0

[PATCH 2/3] accel/kvm: Introduce KvmPutState enum

[Philippe Mathieu-Daudé]

Join the 3 KVM_PUT_*_STATE definitions in a single enum.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/system/kvm.h       | 16 +++++++++-------
 target/i386/kvm/kvm.c      |  6 +++---
 target/loongarch/kvm/kvm.c |  4 ++--
 target/mips/kvm.c          |  6 +++---
 target/ppc/kvm.c           |  2 +-
 target/riscv/kvm/kvm-cpu.c |  2 +-
 target/s390x/kvm/kvm.c     |  2 +-
 7 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/include/system/kvm.h b/include/system/kvm.h
index 4fc09e38910..8f9eecf044c 100644
--- a/include/system/kvm.h
+++ b/include/system/kvm.h
@@ -340,14 +340,16 @@ int kvm_arch_process_async_events(CPUState *cpu);
 
 int kvm_arch_get_registers(CPUState *cpu, Error **errp);
 
-/* state subset only touched by the VCPU itself during runtime */
-#define KVM_PUT_RUNTIME_STATE   1
-/* state subset modified during VCPU reset */
-#define KVM_PUT_RESET_STATE     2
-/* full state set, modified during initialization or on vmload */
-#define KVM_PUT_FULL_STATE      3
+typedef enum kvm_put_state {
+    /* state subset only touched by the VCPU itself during runtime */
+    KVM_PUT_RUNTIME_STATE = 1,
+    /* state subset modified during VCPU reset */
+    KVM_PUT_RESET_STATE = 2,
+    /* full state set, modified during initialization or on vmload */
+    KVM_PUT_FULL_STATE = 3,
+} KvmPutState;
 
-int kvm_arch_put_registers(CPUState *cpu, int level, Error **errp);
+int kvm_arch_put_registers(CPUState *cpu, KvmPutState level, Error **errp);
 
 int kvm_arch_get_default_type(MachineState *ms);
 
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 6a3a1c1ed8e..d06f55938cd 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -3911,7 +3911,7 @@ static void kvm_init_msrs(X86CPU *cpu)
     assert(kvm_buf_set_msrs(cpu) == 0);
 }
 
-static int kvm_put_msrs(X86CPU *cpu, int level)
+static int kvm_put_msrs(X86CPU *cpu, KvmPutState level)
 {
     CPUX86State *env = &cpu->env;
     int i;
@@ -5031,7 +5031,7 @@ static int kvm_get_apic(X86CPU *cpu)
     return 0;
 }
 
-static int kvm_put_vcpu_events(X86CPU *cpu, int level)
+static int kvm_put_vcpu_events(X86CPU *cpu, KvmPutState level)
 {
     CPUState *cs = CPU(cpu);
     CPUX86State *env = &cpu->env;
@@ -5274,7 +5274,7 @@ static int kvm_get_nested_state(X86CPU *cpu)
     return ret;
 }
 
-int kvm_arch_put_registers(CPUState *cpu, int level, Error **errp)
+int kvm_arch_put_registers(CPUState *cpu, KvmPutState level, Error **errp)
 {
     X86CPU *x86_cpu = X86_CPU(cpu);
     int ret;
diff --git a/target/loongarch/kvm/kvm.c b/target/loongarch/kvm/kvm.c
index 45292edcb1c..32cd7c5d003 100644
--- a/target/loongarch/kvm/kvm.c
+++ b/target/loongarch/kvm/kvm.c
@@ -325,7 +325,7 @@ static int kvm_loongarch_get_csr(CPUState *cs)
     return ret;
 }
 
-static int kvm_loongarch_put_csr(CPUState *cs, int level)
+static int kvm_loongarch_put_csr(CPUState *cs, KvmPutState level)
 {
     int ret = 0;
     CPULoongArchState *env = cpu_env(cs);
@@ -763,7 +763,7 @@ int kvm_arch_get_registers(CPUState *cs, Error **errp)
     return ret;
 }
 
-int kvm_arch_put_registers(CPUState *cs, int level, Error **errp)
+int kvm_arch_put_registers(CPUState *cs, KvmPutState level, Error **errp)
 {
     int ret;
     static int once;
diff --git a/target/mips/kvm.c b/target/mips/kvm.c
index 450947c3fa5..912cd5dfa0e 100644
--- a/target/mips/kvm.c
+++ b/target/mips/kvm.c
@@ -590,7 +590,7 @@ static void kvm_mips_update_state(void *opaque, bool running, RunState state)
     }
 }
 
-static int kvm_mips_put_fpu_registers(CPUState *cs, int level)
+static int kvm_mips_put_fpu_registers(CPUState *cs, KvmPutState level)
 {
     CPUMIPSState *env = cpu_env(cs);
     int err, ret = 0;
@@ -749,7 +749,7 @@ static int kvm_mips_get_fpu_registers(CPUState *cs)
 }
 
 
-static int kvm_mips_put_cp0_registers(CPUState *cs, int level)
+static int kvm_mips_put_cp0_registers(CPUState *cs, KvmPutState level)
 {
     CPUMIPSState *env = cpu_env(cs);
     int err, ret = 0;
@@ -1177,7 +1177,7 @@ static int kvm_mips_get_cp0_registers(CPUState *cs)
     return ret;
 }
 
-int kvm_arch_put_registers(CPUState *cs, int level, Error **errp)
+int kvm_arch_put_registers(CPUState *cs, KvmPutState level, Error **errp)
 {
     CPUMIPSState *env = cpu_env(cs);
     struct kvm_regs regs;
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 2521ff65c6c..cd60893a17d 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -907,7 +907,7 @@ int kvmppc_put_books_sregs(PowerPCCPU *cpu)
     return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_SREGS, &sregs);
 }
 
-int kvm_arch_put_registers(CPUState *cs, int level, Error **errp)
+int kvm_arch_put_registers(CPUState *cs, KvmPutState level, Error **errp)
 {
     PowerPCCPU *cpu = POWERPC_CPU(cs);
     CPUPPCState *env = &cpu->env;
diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index 187c2c9501e..75ca3fb9fd9 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -1369,7 +1369,7 @@ int kvm_riscv_sync_mpstate_to_kvm(RISCVCPU *cpu, int state)
     return 0;
 }
 
-int kvm_arch_put_registers(CPUState *cs, int level, Error **errp)
+int kvm_arch_put_registers(CPUState *cs, KvmPutState level, Error **errp)
 {
     int ret = 0;
 
diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c
index 491cc5f9756..916dac1f14e 100644
--- a/target/s390x/kvm/kvm.c
+++ b/target/s390x/kvm/kvm.c
@@ -468,7 +468,7 @@ static int can_sync_regs(CPUState *cs, int regs)
 #define KVM_SYNC_REQUIRED_REGS (KVM_SYNC_GPRS | KVM_SYNC_ACRS | \
                                 KVM_SYNC_CRS | KVM_SYNC_PREFIX)
 
-int kvm_arch_put_registers(CPUState *cs, int level, Error **errp)
+int kvm_arch_put_registers(CPUState *cs, KvmPutState level, Error **errp)
 {
     CPUS390XState *env = cpu_env(cs);
     struct kvm_fpu fpu = {};
-- 
2.51.0

[PATCH 3/3] accel/kvm: Factor kvm_cpu_synchronize_put() out

[Philippe Mathieu-Daudé]

The same code is duplicated 3 times: factor a common method.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 accel/kvm/kvm-all.c | 47 ++++++++++++++++++---------------------------
 1 file changed, 19 insertions(+), 28 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 9060599cd73..de79f4ca099 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -2935,22 +2935,32 @@ void kvm_cpu_synchronize_state(CPUState *cpu)
     }
 }
 
-static void do_kvm_cpu_synchronize_post_reset(CPUState *cpu, run_on_cpu_data arg)
+static bool kvm_cpu_synchronize_put(CPUState *cpu, KvmPutState state,
+                                    const char *desc)
 {
     Error *err = NULL;
-    int ret = kvm_arch_put_registers(cpu, KVM_PUT_RESET_STATE, &err);
+    int ret = kvm_arch_put_registers(cpu, state, &err);
     if (ret) {
         if (err) {
-            error_reportf_err(err, "Restoring resisters after reset: ");
+            error_reportf_err(err, "Restoring resisters %s: ", desc);
         } else {
-            error_report("Failed to put registers after reset: %s",
+            error_report("Failed to put registers %s: %s", desc,
                          strerror(-ret));
         }
-        cpu_dump_state(cpu, stderr, CPU_DUMP_CODE);
-        vm_stop(RUN_STATE_INTERNAL_ERROR);
+        return false;
     }
 
     cpu->vcpu_dirty = false;
+
+    return true;
+}
+
+static void do_kvm_cpu_synchronize_post_reset(CPUState *cpu, run_on_cpu_data arg)
+{
+    if (kvm_cpu_synchronize_put(cpu, KVM_PUT_RESET_STATE, "after reset")) {
+        cpu_dump_state(cpu, stderr, CPU_DUMP_CODE);
+        vm_stop(RUN_STATE_INTERNAL_ERROR);
+    }
 }
 
 void kvm_cpu_synchronize_post_reset(CPUState *cpu)
@@ -2964,19 +2974,9 @@ void kvm_cpu_synchronize_post_reset(CPUState *cpu)
 
 static void do_kvm_cpu_synchronize_post_init(CPUState *cpu, run_on_cpu_data arg)
 {
-    Error *err = NULL;
-    int ret = kvm_arch_put_registers(cpu, KVM_PUT_FULL_STATE, &err);
-    if (ret) {
-        if (err) {
-            error_reportf_err(err, "Putting registers after init: ");
-        } else {
-            error_report("Failed to put registers after init: %s",
-                         strerror(-ret));
-        }
+    if (kvm_cpu_synchronize_put(cpu, KVM_PUT_FULL_STATE, "after init")) {
         exit(1);
     }
-
-    cpu->vcpu_dirty = false;
 }
 
 void kvm_cpu_synchronize_post_init(CPUState *cpu)
@@ -3166,20 +3166,11 @@ int kvm_cpu_exec(CPUState *cpu)
         MemTxAttrs attrs;
 
         if (cpu->vcpu_dirty) {
-            Error *err = NULL;
-            ret = kvm_arch_put_registers(cpu, KVM_PUT_RUNTIME_STATE, &err);
-            if (ret) {
-                if (err) {
-                    error_reportf_err(err, "Putting registers after init: ");
-                } else {
-                    error_report("Failed to put registers after init: %s",
-                                 strerror(-ret));
-                }
+            if (kvm_cpu_synchronize_put(cpu, KVM_PUT_RUNTIME_STATE,
+                                        "at runtime")) {
                 ret = -1;
                 break;
             }
-
-            cpu->vcpu_dirty = false;
         }
 
         kvm_arch_pre_run(cpu, run);
-- 
2.51.0

Re: [PATCH 1/3] accel/kvm: Do not expect more then KVM_PUT_FULL_STATE

[Harsh Prateek Bora]

On 10/7/25 13:46, Philippe Mathieu-Daudé wrote:
> KVM_PUT_FULL_STATE is the higher level defined so far in

s/higher/highest ?

With that,
Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>

> "system/kvm.h"; do not check for more.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/loongarch/kvm/kvm.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/loongarch/kvm/kvm.c b/target/loongarch/kvm/kvm.c
> index e5ea2dba9da..45292edcb1c 100644
> --- a/target/loongarch/kvm/kvm.c
> +++ b/target/loongarch/kvm/kvm.c
> @@ -397,7 +397,7 @@ static int kvm_loongarch_put_csr(CPUState *cs, int level)
>                              &env->CSR_RVACFG);
>   
>       /* CPUID is constant after poweron, it should be set only once */
> -    if (level >= KVM_PUT_FULL_STATE) {
> +    if (level == KVM_PUT_FULL_STATE) {
>           ret |= kvm_set_one_reg(cs, KVM_IOC_CSRID(LOONGARCH_CSR_CPUID),
>                              &env->CSR_CPUID);
>       }
> @@ -801,7 +801,7 @@ int kvm_arch_put_registers(CPUState *cs, int level, Error **errp)
>           once = 1;
>       }
>   
> -    if (level >= KVM_PUT_FULL_STATE) {
> +    if (level == KVM_PUT_FULL_STATE) {
>           /*
>            * only KVM_PUT_FULL_STATE is required, kvm kernel will clear
>            * guest_addr for KVM_PUT_RESET_STATE

Re: [PATCH 2/3] accel/kvm: Introduce KvmPutState enum

[Harsh Prateek Bora]

On 10/7/25 13:46, Philippe Mathieu-Daudé wrote:
> Join the 3 KVM_PUT_*_STATE definitions in a single enum.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>

> ---
>   include/system/kvm.h       | 16 +++++++++-------
>   target/i386/kvm/kvm.c      |  6 +++---
>   target/loongarch/kvm/kvm.c |  4 ++--
>   target/mips/kvm.c          |  6 +++---
>   target/ppc/kvm.c           |  2 +-
>   target/riscv/kvm/kvm-cpu.c |  2 +-
>   target/s390x/kvm/kvm.c     |  2 +-
>   7 files changed, 20 insertions(+), 18 deletions(-)
> 
> diff --git a/include/system/kvm.h b/include/system/kvm.h
> index 4fc09e38910..8f9eecf044c 100644
> --- a/include/system/kvm.h
> +++ b/include/system/kvm.h
> @@ -340,14 +340,16 @@ int kvm_arch_process_async_events(CPUState *cpu);
>   
>   int kvm_arch_get_registers(CPUState *cpu, Error **errp);
>   
> -/* state subset only touched by the VCPU itself during runtime */
> -#define KVM_PUT_RUNTIME_STATE   1
> -/* state subset modified during VCPU reset */
> -#define KVM_PUT_RESET_STATE     2
> -/* full state set, modified during initialization or on vmload */
> -#define KVM_PUT_FULL_STATE      3
> +typedef enum kvm_put_state {
> +    /* state subset only touched by the VCPU itself during runtime */
> +    KVM_PUT_RUNTIME_STATE = 1,
> +    /* state subset modified during VCPU reset */
> +    KVM_PUT_RESET_STATE = 2,
> +    /* full state set, modified during initialization or on vmload */
> +    KVM_PUT_FULL_STATE = 3,
> +} KvmPutState;
>   
> -int kvm_arch_put_registers(CPUState *cpu, int level, Error **errp);
> +int kvm_arch_put_registers(CPUState *cpu, KvmPutState level, Error **errp);
>   
>   int kvm_arch_get_default_type(MachineState *ms);
>   
> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> index 6a3a1c1ed8e..d06f55938cd 100644
> --- a/target/i386/kvm/kvm.c
> +++ b/target/i386/kvm/kvm.c
> @@ -3911,7 +3911,7 @@ static void kvm_init_msrs(X86CPU *cpu)
>       assert(kvm_buf_set_msrs(cpu) == 0);
>   }
>   
> -static int kvm_put_msrs(X86CPU *cpu, int level)
> +static int kvm_put_msrs(X86CPU *cpu, KvmPutState level)
>   {
>       CPUX86State *env = &cpu->env;
>       int i;
> @@ -5031,7 +5031,7 @@ static int kvm_get_apic(X86CPU *cpu)
>       return 0;
>   }
>   
> -static int kvm_put_vcpu_events(X86CPU *cpu, int level)
> +static int kvm_put_vcpu_events(X86CPU *cpu, KvmPutState level)
>   {
>       CPUState *cs = CPU(cpu);
>       CPUX86State *env = &cpu->env;
> @@ -5274,7 +5274,7 @@ static int kvm_get_nested_state(X86CPU *cpu)
>       return ret;
>   }
>   
> -int kvm_arch_put_registers(CPUState *cpu, int level, Error **errp)
> +int kvm_arch_put_registers(CPUState *cpu, KvmPutState level, Error **errp)
>   {
>       X86CPU *x86_cpu = X86_CPU(cpu);
>       int ret;
> diff --git a/target/loongarch/kvm/kvm.c b/target/loongarch/kvm/kvm.c
> index 45292edcb1c..32cd7c5d003 100644
> --- a/target/loongarch/kvm/kvm.c
> +++ b/target/loongarch/kvm/kvm.c
> @@ -325,7 +325,7 @@ static int kvm_loongarch_get_csr(CPUState *cs)
>       return ret;
>   }
>   
> -static int kvm_loongarch_put_csr(CPUState *cs, int level)
> +static int kvm_loongarch_put_csr(CPUState *cs, KvmPutState level)
>   {
>       int ret = 0;
>       CPULoongArchState *env = cpu_env(cs);
> @@ -763,7 +763,7 @@ int kvm_arch_get_registers(CPUState *cs, Error **errp)
>       return ret;
>   }
>   
> -int kvm_arch_put_registers(CPUState *cs, int level, Error **errp)
> +int kvm_arch_put_registers(CPUState *cs, KvmPutState level, Error **errp)
>   {
>       int ret;
>       static int once;
> diff --git a/target/mips/kvm.c b/target/mips/kvm.c
> index 450947c3fa5..912cd5dfa0e 100644
> --- a/target/mips/kvm.c
> +++ b/target/mips/kvm.c
> @@ -590,7 +590,7 @@ static void kvm_mips_update_state(void *opaque, bool running, RunState state)
>       }
>   }
>   
> -static int kvm_mips_put_fpu_registers(CPUState *cs, int level)
> +static int kvm_mips_put_fpu_registers(CPUState *cs, KvmPutState level)
>   {
>       CPUMIPSState *env = cpu_env(cs);
>       int err, ret = 0;
> @@ -749,7 +749,7 @@ static int kvm_mips_get_fpu_registers(CPUState *cs)
>   }
>   
>   
> -static int kvm_mips_put_cp0_registers(CPUState *cs, int level)
> +static int kvm_mips_put_cp0_registers(CPUState *cs, KvmPutState level)
>   {
>       CPUMIPSState *env = cpu_env(cs);
>       int err, ret = 0;
> @@ -1177,7 +1177,7 @@ static int kvm_mips_get_cp0_registers(CPUState *cs)
>       return ret;
>   }
>   
> -int kvm_arch_put_registers(CPUState *cs, int level, Error **errp)
> +int kvm_arch_put_registers(CPUState *cs, KvmPutState level, Error **errp)
>   {
>       CPUMIPSState *env = cpu_env(cs);
>       struct kvm_regs regs;
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index 2521ff65c6c..cd60893a17d 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -907,7 +907,7 @@ int kvmppc_put_books_sregs(PowerPCCPU *cpu)
>       return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_SREGS, &sregs);
>   }
>   
> -int kvm_arch_put_registers(CPUState *cs, int level, Error **errp)
> +int kvm_arch_put_registers(CPUState *cs, KvmPutState level, Error **errp)
>   {
>       PowerPCCPU *cpu = POWERPC_CPU(cs);
>       CPUPPCState *env = &cpu->env;
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index 187c2c9501e..75ca3fb9fd9 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -1369,7 +1369,7 @@ int kvm_riscv_sync_mpstate_to_kvm(RISCVCPU *cpu, int state)
>       return 0;
>   }
>   
> -int kvm_arch_put_registers(CPUState *cs, int level, Error **errp)
> +int kvm_arch_put_registers(CPUState *cs, KvmPutState level, Error **errp)
>   {
>       int ret = 0;
>   
> diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c
> index 491cc5f9756..916dac1f14e 100644
> --- a/target/s390x/kvm/kvm.c
> +++ b/target/s390x/kvm/kvm.c
> @@ -468,7 +468,7 @@ static int can_sync_regs(CPUState *cs, int regs)
>   #define KVM_SYNC_REQUIRED_REGS (KVM_SYNC_GPRS | KVM_SYNC_ACRS | \
>                                   KVM_SYNC_CRS | KVM_SYNC_PREFIX)
>   
> -int kvm_arch_put_registers(CPUState *cs, int level, Error **errp)
> +int kvm_arch_put_registers(CPUState *cs, KvmPutState level, Error **errp)
>   {
>       CPUS390XState *env = cpu_env(cs);
>       struct kvm_fpu fpu = {};

Re: [PATCH 3/3] accel/kvm: Factor kvm_cpu_synchronize_put() out

[Andrew Jones]

On Tue, Oct 07, 2025 at 10:16:16AM +0200, Philippe Mathieu-Daudé wrote:
> The same code is duplicated 3 times: factor a common method.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  accel/kvm/kvm-all.c | 47 ++++++++++++++++++---------------------------
>  1 file changed, 19 insertions(+), 28 deletions(-)
> 
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index 9060599cd73..de79f4ca099 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -2935,22 +2935,32 @@ void kvm_cpu_synchronize_state(CPUState *cpu)
>      }
>  }
>  
> -static void do_kvm_cpu_synchronize_post_reset(CPUState *cpu, run_on_cpu_data arg)
> +static bool kvm_cpu_synchronize_put(CPUState *cpu, KvmPutState state,
> +                                    const char *desc)
>  {
>      Error *err = NULL;
> -    int ret = kvm_arch_put_registers(cpu, KVM_PUT_RESET_STATE, &err);
> +    int ret = kvm_arch_put_registers(cpu, state, &err);
>      if (ret) {
>          if (err) {
> -            error_reportf_err(err, "Restoring resisters after reset: ");
> +            error_reportf_err(err, "Restoring resisters %s: ", desc);
>          } else {
> -            error_report("Failed to put registers after reset: %s",
> +            error_report("Failed to put registers %s: %s", desc,
>                           strerror(-ret));
>          }
> -        cpu_dump_state(cpu, stderr, CPU_DUMP_CODE);
> -        vm_stop(RUN_STATE_INTERNAL_ERROR);
> +        return false;
>      }
>  
>      cpu->vcpu_dirty = false;
> +
> +    return true;
> +}
> +
> +static void do_kvm_cpu_synchronize_post_reset(CPUState *cpu, run_on_cpu_data arg)
> +{
> +    if (kvm_cpu_synchronize_put(cpu, KVM_PUT_RESET_STATE, "after reset")) {

This should be !kvm_cpu_synchronize_put() and same comment for the other
calls below.

Thanks,
drew

> +        cpu_dump_state(cpu, stderr, CPU_DUMP_CODE);
> +        vm_stop(RUN_STATE_INTERNAL_ERROR);
> +    }
>  }
>  
>  void kvm_cpu_synchronize_post_reset(CPUState *cpu)
> @@ -2964,19 +2974,9 @@ void kvm_cpu_synchronize_post_reset(CPUState *cpu)
>  
>  static void do_kvm_cpu_synchronize_post_init(CPUState *cpu, run_on_cpu_data arg)
>  {
> -    Error *err = NULL;
> -    int ret = kvm_arch_put_registers(cpu, KVM_PUT_FULL_STATE, &err);
> -    if (ret) {
> -        if (err) {
> -            error_reportf_err(err, "Putting registers after init: ");
> -        } else {
> -            error_report("Failed to put registers after init: %s",
> -                         strerror(-ret));
> -        }
> +    if (kvm_cpu_synchronize_put(cpu, KVM_PUT_FULL_STATE, "after init")) {
>          exit(1);
>      }
> -
> -    cpu->vcpu_dirty = false;
>  }
>  
>  void kvm_cpu_synchronize_post_init(CPUState *cpu)
> @@ -3166,20 +3166,11 @@ int kvm_cpu_exec(CPUState *cpu)
>          MemTxAttrs attrs;
>  
>          if (cpu->vcpu_dirty) {
> -            Error *err = NULL;
> -            ret = kvm_arch_put_registers(cpu, KVM_PUT_RUNTIME_STATE, &err);
> -            if (ret) {
> -                if (err) {
> -                    error_reportf_err(err, "Putting registers after init: ");
> -                } else {
> -                    error_report("Failed to put registers after init: %s",
> -                                 strerror(-ret));
> -                }
> +            if (kvm_cpu_synchronize_put(cpu, KVM_PUT_RUNTIME_STATE,
> +                                        "at runtime")) {
>                  ret = -1;
>                  break;
>              }
> -
> -            cpu->vcpu_dirty = false;
>          }
>  
>          kvm_arch_pre_run(cpu, run);
> -- 
> 2.51.0
> 
> 

Re: [PATCH 3/3] accel/kvm: Factor kvm_cpu_synchronize_put() out

[Philippe Mathieu-Daudé]

On 7/10/25 15:30, Andrew Jones wrote:
> On Tue, Oct 07, 2025 at 10:16:16AM +0200, Philippe Mathieu-Daudé wrote:
>> The same code is duplicated 3 times: factor a common method.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   accel/kvm/kvm-all.c | 47 ++++++++++++++++++---------------------------
>>   1 file changed, 19 insertions(+), 28 deletions(-)
>>
>> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
>> index 9060599cd73..de79f4ca099 100644
>> --- a/accel/kvm/kvm-all.c
>> +++ b/accel/kvm/kvm-all.c
>> @@ -2935,22 +2935,32 @@ void kvm_cpu_synchronize_state(CPUState *cpu)
>>       }
>>   }
>>   
>> -static void do_kvm_cpu_synchronize_post_reset(CPUState *cpu, run_on_cpu_data arg)
>> +static bool kvm_cpu_synchronize_put(CPUState *cpu, KvmPutState state,
>> +                                    const char *desc)
>>   {
>>       Error *err = NULL;
>> -    int ret = kvm_arch_put_registers(cpu, KVM_PUT_RESET_STATE, &err);
>> +    int ret = kvm_arch_put_registers(cpu, state, &err);
>>       if (ret) {
>>           if (err) {
>> -            error_reportf_err(err, "Restoring resisters after reset: ");
>> +            error_reportf_err(err, "Restoring resisters %s: ", desc);
>>           } else {
>> -            error_report("Failed to put registers after reset: %s",
>> +            error_report("Failed to put registers %s: %s", desc,
>>                            strerror(-ret));
>>           }
>> -        cpu_dump_state(cpu, stderr, CPU_DUMP_CODE);
>> -        vm_stop(RUN_STATE_INTERNAL_ERROR);
>> +        return false;
>>       }
>>   
>>       cpu->vcpu_dirty = false;
>> +
>> +    return true;
>> +}
>> +
>> +static void do_kvm_cpu_synchronize_post_reset(CPUState *cpu, run_on_cpu_data arg)
>> +{
>> +    if (kvm_cpu_synchronize_put(cpu, KVM_PUT_RESET_STATE, "after reset")) {
> 
> This should be !kvm_cpu_synchronize_put() and same comment for the other
> calls below.

Oops! Thanks :)

> 
> Thanks,
> drew

Re: [PATCH 3/3] accel/kvm: Factor kvm_cpu_synchronize_put() out

[Richard Henderson]

On 10/7/25 01:16, Philippe Mathieu-Daudé wrote:
> The same code is duplicated 3 times: factor a common method.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   accel/kvm/kvm-all.c | 47 ++++++++++++++++++---------------------------
>   1 file changed, 19 insertions(+), 28 deletions(-)
> 
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index 9060599cd73..de79f4ca099 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -2935,22 +2935,32 @@ void kvm_cpu_synchronize_state(CPUState *cpu)
>       }
>   }
>   
> -static void do_kvm_cpu_synchronize_post_reset(CPUState *cpu, run_on_cpu_data arg)
> +static bool kvm_cpu_synchronize_put(CPUState *cpu, KvmPutState state,
> +                                    const char *desc)
>   {
>       Error *err = NULL;
> -    int ret = kvm_arch_put_registers(cpu, KVM_PUT_RESET_STATE, &err);
> +    int ret = kvm_arch_put_registers(cpu, state, &err);
>       if (ret) {
>           if (err) {
> -            error_reportf_err(err, "Restoring resisters after reset: ");
> +            error_reportf_err(err, "Restoring resisters %s: ", desc);
>           } else {
> -            error_report("Failed to put registers after reset: %s",
> +            error_report("Failed to put registers %s: %s", desc,
>                            strerror(-ret));
>           }

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

For the to-do list: the arch routine really should be using error_setg_errno, not 
deferring the test of errno to here.  But it seems i386 is the only target that actually 
sets errp.


r~