diff mbox series

target/riscv: Clearing the CSR values at reset and syncing the MPSTATE with the host

Message ID 20230718122157.7425-1-18622748025@163.com
State New
Headers show
Series target/riscv: Clearing the CSR values at reset and syncing the MPSTATE with the host | expand

Commit Message

liguang.zhang July 18, 2023, 12:21 p.m. UTC 
From: "liguang.zhang" <liguang.zhang@hexintek.com>

Fix the guest reboot error when using KVM
There are two issues when rebooting a guest using KVM
1. When the guest initiates a reboot the host is unable to stop the vcpu
2. When running a SMP guest the qemu monitor system_reset causes a vcpu crash

This can be fixed by clearing the CSR values at reset and syncing the
MPSTATE with the host.

Signed-off-by: liguang.zhang <liguang.zhang@hexintek.com>
---
 target/riscv/kvm.c       | 42 ++++++++++++++++++++++++++++++++++++++++
 target/riscv/kvm_riscv.h |  1 +
 2 files changed, 43 insertions(+)

Comments

Alistair Francis July 24, 2023, 3:30 a.m. UTC | #1 
On Tue, Jul 18, 2023 at 10:22 PM liguang.zhang <18622748025@163.com> wrote:
>
> From: "liguang.zhang" <liguang.zhang@hexintek.com>
>
> Fix the guest reboot error when using KVM
> There are two issues when rebooting a guest using KVM
> 1. When the guest initiates a reboot the host is unable to stop the vcpu
> 2. When running a SMP guest the qemu monitor system_reset causes a vcpu crash
>
> This can be fixed by clearing the CSR values at reset and syncing the
> MPSTATE with the host.
>
> Signed-off-by: liguang.zhang <liguang.zhang@hexintek.com>

Thanks!

When sending new versions of patches please increment the patch
version: https://www.qemu.org/docs/master/devel/submitting-a-patch.html#when-resending-patches-add-a-version-tag

The patch looks good, but don't we need an equivalent for the get register call?

Alistair

> ---
>  target/riscv/kvm.c       | 42 ++++++++++++++++++++++++++++++++++++++++
>  target/riscv/kvm_riscv.h |  1 +
>  2 files changed, 43 insertions(+)
>
> diff --git a/target/riscv/kvm.c b/target/riscv/kvm.c
> index 9d8a8982f9..ecc8ab8238 100644
> --- a/target/riscv/kvm.c
> +++ b/target/riscv/kvm.c
> @@ -44,6 +44,8 @@
>  #include "migration/migration.h"
>  #include "sysemu/runstate.h"
>
> +static bool cap_has_mp_state;
> +
>  static uint64_t kvm_riscv_reg_id(CPURISCVState *env, uint64_t type,
>                                   uint64_t idx)
>  {
> @@ -790,6 +792,24 @@ int kvm_arch_get_registers(CPUState *cs)
>      return ret;
>  }
>
> +int kvm_riscv_sync_mpstate_to_kvm(RISCVCPU *cpu, int state)
> +{
> +    if (cap_has_mp_state) {
> +        struct kvm_mp_state mp_state = {
> +            .mp_state = state
> +        };
> +
> +        int ret = kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MP_STATE, &mp_state);
> +        if (ret) {
> +            fprintf(stderr, "%s: failed to sync MP_STATE %d/%s\n",
> +                    __func__, ret, strerror(-ret));
> +            return -1;
> +        }
> +    }
> +
> +    return 0;
> +}
> +
>  int kvm_arch_put_registers(CPUState *cs, int level)
>  {
>      int ret = 0;
> @@ -809,6 +829,18 @@ int kvm_arch_put_registers(CPUState *cs, int level)
>          return ret;
>      }
>
> +    if (KVM_PUT_RESET_STATE == level) {
> +        RISCVCPU *cpu = RISCV_CPU(cs);
> +        if (cs->cpu_index == 0) {
> +            ret = kvm_riscv_sync_mpstate_to_kvm(cpu, KVM_MP_STATE_RUNNABLE);
> +        } else {
> +            ret = kvm_riscv_sync_mpstate_to_kvm(cpu, KVM_MP_STATE_STOPPED);
> +        }
> +        if (ret) {
> +            return ret;
> +        }
> +    }
> +
>      return ret;
>  }
>
> @@ -909,6 +941,7 @@ int kvm_arch_add_msi_route_post(struct kvm_irq_routing_entry *route,
>
>  int kvm_arch_init(MachineState *ms, KVMState *s)
>  {
> +    cap_has_mp_state = kvm_check_extension(s, KVM_CAP_MP_STATE);
>      return 0;
>  }
>
> @@ -987,10 +1020,19 @@ void kvm_riscv_reset_vcpu(RISCVCPU *cpu)
>      if (!kvm_enabled()) {
>          return;
>      }
> +    for (int i=0; i<32; i++)
> +        env->gpr[i] = 0;
>      env->pc = cpu->env.kernel_addr;
>      env->gpr[10] = kvm_arch_vcpu_id(CPU(cpu)); /* a0 */
>      env->gpr[11] = cpu->env.fdt_addr;          /* a1 */
>      env->satp = 0;
> +    env->mie = 0;
> +    env->stvec = 0;
> +    env->sscratch = 0;
> +    env->sepc = 0;
> +    env->scause = 0;
> +    env->stval = 0;
> +    env->mip = 0;
>  }
>
>  void kvm_riscv_set_irq(RISCVCPU *cpu, int irq, int level)
> diff --git a/target/riscv/kvm_riscv.h b/target/riscv/kvm_riscv.h
> index e3ba935808..3ea68c38e3 100644
> --- a/target/riscv/kvm_riscv.h
> +++ b/target/riscv/kvm_riscv.h
> @@ -22,5 +22,6 @@
>  void kvm_riscv_init_user_properties(Object *cpu_obj);
>  void kvm_riscv_reset_vcpu(RISCVCPU *cpu);
>  void kvm_riscv_set_irq(RISCVCPU *cpu, int irq, int level);
> +int kvm_riscv_sync_mpstate_to_kvm(RISCVCPU *cpu, int state);
>
>  #endif
> --
> 2.17.1
>
liguang.zhang July 24, 2023, 6:05 a.m. UTC | #2 
> On Tue, Jul 18, 2023 at 10:22 PM liguang.zhang <18622748...@163.com> wrote:
> >
> > From: "liguang.zhang" <liguang.zh...@hexintek.com>
> >
> > Fix the guest reboot error when using KVM
> > There are two issues when rebooting a guest using KVM
> > 1. When the guest initiates a reboot the host is unable to stop the vcpu
> > 2. When running a SMP guest the qemu monitor system_reset causes a vcpu crash
> >
> > This can be fixed by clearing the CSR values at reset and syncing the
> > MPSTATE with the host.
> >
> > Signed-off-by: liguang.zhang <liguang.zh...@hexintek.com>
>
> Thanks!
>
> When sending new versions of patches please increment the patch
> version:
> https://www.qemu.org/docs/master/devel/submitting-a-patch.html#when-resending-patches-add-a-version-tag
>

Sorry about it, since i'm confused about the git send-email, original mail thread lost. -> https://www.mail-archive.com/qemu-devel@nongnu.org/msg977038.html
I would like to resubmit and track the email history.

> The patch looks good, but don't we need an equivalent for the get register call?
>
> Alistair

Sorry, "get register call" refers to which section? It was not mentioned in the previous suggestions for modifications.
Follow the original modification suggestions, I hope to upstream as soon as possible, as it has been delayed for quite some time.

Thanks ~
Alistair Francis Aug. 10, 2023, 6:43 p.m. UTC | #3 
On Mon, Jul 24, 2023 at 2:06 AM liguang.zhang <18622748025@163.com> wrote:
>
> > On Tue, Jul 18, 2023 at 10:22 PM liguang.zhang <18622748...@163.com> wrote:
> > >
> > > From: "liguang.zhang" <liguang.zh...@hexintek.com>
> > >
> > > Fix the guest reboot error when using KVM
> > > There are two issues when rebooting a guest using KVM
> > > 1. When the guest initiates a reboot the host is unable to stop the vcpu
> > > 2. When running a SMP guest the qemu monitor system_reset causes a vcpu crash
> > >
> > > This can be fixed by clearing the CSR values at reset and syncing the
> > > MPSTATE with the host.
> > >
> > > Signed-off-by: liguang.zhang <liguang.zh...@hexintek.com>
> >
> > Thanks!
> >
> > When sending new versions of patches please increment the patch
> > version:
> > https://www.qemu.org/docs/master/devel/submitting-a-patch.html#when-resending-patches-add-a-version-tag
> >
>
> Sorry about it, since i'm confused about the git send-email, original mail thread lost. -> https://www.mail-archive.com/qemu-devel@nongnu.org/msg977038.html
> I would like to resubmit and track the email history.

No worries :)

I have noticed that when you send the patch emails I get multiple
emails sent a few minutes apart. I count at least 8 copies of the same
email. Do you mind trying to fix whatever is causing that?

>
> > The patch looks good, but don't we need an equivalent for the get register call?
> >
> > Alistair
>
> Sorry, "get register call" refers to which section? It was not mentioned in the previous suggestions for modifications.

You are adding code to kvm_arch_put_registers() don't you also need to
add code to kvm_arch_get_registers()? and a *_mpstate_to_qemu()
function to match?

> Follow the original modification suggestions, I hope to upstream as soon as possible, as it has been delayed for quite some time.

Upstreamig code is an iterative process. Just because something wasn't
brought up in the first version doesn't mean it won't be raised later.

I'm sorry it has taken so long. If you don't get a reply within a week
please ping your patches or responses. Ensuring you follow patch
submission best practices can help improve the upstreaming speed, such
as incrementing patch versions and responding with plain text inline.

Hopefully just one more revision is required :)

Alistair

>
> Thanks ~
>
diff mbox series

Patch

diff --git a/target/riscv/kvm.c b/target/riscv/kvm.c
index 9d8a8982f9..ecc8ab8238 100644
--- a/target/riscv/kvm.c
+++ b/target/riscv/kvm.c
@@ -44,6 +44,8 @@ 
 #include "migration/migration.h"
 #include "sysemu/runstate.h"
 
+static bool cap_has_mp_state;
+
 static uint64_t kvm_riscv_reg_id(CPURISCVState *env, uint64_t type,
                                  uint64_t idx)
 {
@@ -790,6 +792,24 @@  int kvm_arch_get_registers(CPUState *cs)
     return ret;
 }
 
+int kvm_riscv_sync_mpstate_to_kvm(RISCVCPU *cpu, int state)
+{
+    if (cap_has_mp_state) {
+        struct kvm_mp_state mp_state = {
+            .mp_state = state
+        };
+
+        int ret = kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MP_STATE, &mp_state);
+        if (ret) {
+            fprintf(stderr, "%s: failed to sync MP_STATE %d/%s\n",
+                    __func__, ret, strerror(-ret));
+            return -1;
+        }
+    }
+
+    return 0;
+}
+
 int kvm_arch_put_registers(CPUState *cs, int level)
 {
     int ret = 0;
@@ -809,6 +829,18 @@  int kvm_arch_put_registers(CPUState *cs, int level)
         return ret;
     }
 
+    if (KVM_PUT_RESET_STATE == level) {
+        RISCVCPU *cpu = RISCV_CPU(cs);
+        if (cs->cpu_index == 0) {
+            ret = kvm_riscv_sync_mpstate_to_kvm(cpu, KVM_MP_STATE_RUNNABLE);
+        } else {
+            ret = kvm_riscv_sync_mpstate_to_kvm(cpu, KVM_MP_STATE_STOPPED);
+        }
+        if (ret) {
+            return ret;
+        }
+    }
+
     return ret;
 }
 
@@ -909,6 +941,7 @@  int kvm_arch_add_msi_route_post(struct kvm_irq_routing_entry *route,
 
 int kvm_arch_init(MachineState *ms, KVMState *s)
 {
+    cap_has_mp_state = kvm_check_extension(s, KVM_CAP_MP_STATE);
     return 0;
 }
 
@@ -987,10 +1020,19 @@  void kvm_riscv_reset_vcpu(RISCVCPU *cpu)
     if (!kvm_enabled()) {
         return;
     }
+    for (int i=0; i<32; i++)
+        env->gpr[i] = 0;
     env->pc = cpu->env.kernel_addr;
     env->gpr[10] = kvm_arch_vcpu_id(CPU(cpu)); /* a0 */
     env->gpr[11] = cpu->env.fdt_addr;          /* a1 */
     env->satp = 0;
+    env->mie = 0;
+    env->stvec = 0;
+    env->sscratch = 0;
+    env->sepc = 0;
+    env->scause = 0;
+    env->stval = 0;
+    env->mip = 0;
 }
 
 void kvm_riscv_set_irq(RISCVCPU *cpu, int irq, int level)
diff --git a/target/riscv/kvm_riscv.h b/target/riscv/kvm_riscv.h
index e3ba935808..3ea68c38e3 100644
--- a/target/riscv/kvm_riscv.h
+++ b/target/riscv/kvm_riscv.h
@@ -22,5 +22,6 @@ 
 void kvm_riscv_init_user_properties(Object *cpu_obj);
 void kvm_riscv_reset_vcpu(RISCVCPU *cpu);
 void kvm_riscv_set_irq(RISCVCPU *cpu, int irq, int level);
+int kvm_riscv_sync_mpstate_to_kvm(RISCVCPU *cpu, int state);
 
 #endif