qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] target/riscv: fix the issue of guest reboot then no response or crash in kvm-mode
@ 2023-06-25  2:50 liguang.zhang
  2023-07-10  1:16 ` Alistair Francis
                   ` (10 more replies)
  0 siblings, 11 replies; 29+ messages in thread
From: liguang.zhang @ 2023-06-25  2:50 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, alistair23, liguang.zhang

From: "liguang.zhang" <liguang.zhang@hexintek.com>

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.

kernel log
```shell
$reboot

The system is going down NOW!
Sent SIGTERM to all processes
logout
Sent SIGKILL to all processes
Requesting system reboot

```
then no response

for qemu command:
$system_reset:

kernel log:
```shell
[   53.739556] kvm [150]: VCPU exit error -95
[   53.739563] kvm [148]: VCPU exit error -95
[   53.739557] kvm [149]: VCPU exit error -95
[   53.740957] kvm [149]: SEPC=0x0 SSTATUS=0x200004120 HSTATUS=0x2002001c0
[   53.740957] kvm [148]: SEPC=0x0 SSTATUS=0x200004120 HSTATUS=0x2002001c0
[   53.741054] kvm [148]: SCAUSE=0x14 STVAL=0x0 HTVAL=0x0 HTINST=0x0
[   53.741058] kvm [149]: SCAUSE=0x14 STVAL=0x0 HTVAL=0x0 HTINST=0x0
[   53.756187] kvm [150]: SEPC=0x0 SSTATUS=0x200004120 HSTATUS=0x2002001c0
[   53.757797] kvm [150]: SCAUSE=0x14 STVAL=0x0 HTVAL=0x0 HTINST=0x0
```

solution:

add reset csr and context for riscv vcpu
qemu ioctl reset vcpu->arch.power_off state of kvm

tests:

qemu-system-riscv64 -M virt -bios none -kernel Image \
   -smp 4 -enable-kvm \
   -append "rootwait root=/dev/vda ro" \
   -drive file=rootfs.ext2,format=raw,id=hd0 \
   -device virtio-blk-device,drive=hd0

in guest shell:
$reboot

qemu command:
$system_reset

---
v3:
- change kvm_riscv_set_mpstate_to_kvm to kvm_riscv_sync_mpstate_to_kvm
- remove newline after if(cap_has_mp_state)
- update submit description

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

diff --git a/target/riscv/kvm.c b/target/riscv/kvm.c
index 0f932a5b96..c478c71905 100644
--- a/target/riscv/kvm.c
+++ b/target/riscv/kvm.c
@@ -42,6 +42,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)
 {
@@ -99,7 +101,7 @@ static uint64_t kvm_riscv_reg_id(CPURISCVState *env, uint64_t type,
 
 #define KVM_RISCV_SET_TIMER(cs, env, name, reg) \
     do { \
-        int ret = kvm_set_one_reg(cs, RISCV_TIMER_REG(env, time), &reg); \
+        int ret = kvm_set_one_reg(cs, RISCV_TIMER_REG(env, name), &reg); \
         if (ret) { \
             abort(); \
         } \
@@ -335,6 +337,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;
@@ -354,6 +374,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;
 }
 
@@ -428,6 +460,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;
 }
 
@@ -506,10 +539,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 ed281bdce0..88aee902dd 100644
--- a/target/riscv/kvm_riscv.h
+++ b/target/riscv/kvm_riscv.h
@@ -21,5 +21,6 @@
 
 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



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH] target/riscv: fix the issue of guest reboot then no response or crash in kvm-mode
  2023-06-25  2:50 [PATCH] target/riscv: fix the issue of guest reboot then no response or crash in kvm-mode liguang.zhang
@ 2023-07-10  1:16 ` Alistair Francis
  2023-07-18 11:06   ` 18622748025
                     ` (2 more replies)
  2023-07-18 11:49 ` liguang.zhang
                   ` (9 subsequent siblings)
  10 siblings, 3 replies; 29+ messages in thread
From: Alistair Francis @ 2023-07-10  1:16 UTC (permalink / raw)
  To: liguang.zhang; +Cc: qemu-devel, pbonzini, liguang.zhang

On Sun, Jun 25, 2023 at 12:50 PM liguang.zhang <18622748025@163.com> wrote:
>
> From: "liguang.zhang" <liguang.zhang@hexintek.com>
>
> 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.

Thanks for the patch.

I think we should move everything

--- from here ---

>
> kernel log
> ```shell
> $reboot
>
> The system is going down NOW!
> Sent SIGTERM to all processes
> logout
> Sent SIGKILL to all processes
> Requesting system reboot
>
> ```
> then no response
>
> for qemu command:
> $system_reset:
>
> kernel log:
> ```shell
> [   53.739556] kvm [150]: VCPU exit error -95
> [   53.739563] kvm [148]: VCPU exit error -95
> [   53.739557] kvm [149]: VCPU exit error -95
> [   53.740957] kvm [149]: SEPC=0x0 SSTATUS=0x200004120 HSTATUS=0x2002001c0
> [   53.740957] kvm [148]: SEPC=0x0 SSTATUS=0x200004120 HSTATUS=0x2002001c0
> [   53.741054] kvm [148]: SCAUSE=0x14 STVAL=0x0 HTVAL=0x0 HTINST=0x0
> [   53.741058] kvm [149]: SCAUSE=0x14 STVAL=0x0 HTVAL=0x0 HTINST=0x0
> [   53.756187] kvm [150]: SEPC=0x0 SSTATUS=0x200004120 HSTATUS=0x2002001c0
> [   53.757797] kvm [150]: SCAUSE=0x14 STVAL=0x0 HTVAL=0x0 HTINST=0x0
> ```
>
> solution:
>
> add reset csr and context for riscv vcpu
> qemu ioctl reset vcpu->arch.power_off state of kvm
>
> tests:
>
> qemu-system-riscv64 -M virt -bios none -kernel Image \
>    -smp 4 -enable-kvm \
>    -append "rootwait root=/dev/vda ro" \
>    -drive file=rootfs.ext2,format=raw,id=hd0 \
>    -device virtio-blk-device,drive=hd0
>
> in guest shell:
> $reboot
>
> qemu command:
> $system_reset
>
> ---
> v3:
> - change kvm_riscv_set_mpstate_to_kvm to kvm_riscv_sync_mpstate_to_kvm
> - remove newline after if(cap_has_mp_state)
> - update submit description

--- to here ---

>
> Signed-off-by: liguang.zhang <liguang.zhang@hexintek.com>
> ---

below this line. That way it will be included in the patch submission,
but won't be included in the git tree.

We never include the patch changelog in the git tree and I don't think
we need to include the steps as well (they will be preserved on the
mailing list).

For the patch title (the git commit title) can you explain what the
patch is doing?

Also, do you mind rebasing on
https://github.com/alistair23/qemu/tree/riscv-to-apply.next

Then the patch should be good to go!

Alistair

>  target/riscv/kvm.c       | 44 +++++++++++++++++++++++++++++++++++++++-
>  target/riscv/kvm_riscv.h |  1 +
>  2 files changed, 44 insertions(+), 1 deletion(-)
>
> diff --git a/target/riscv/kvm.c b/target/riscv/kvm.c
> index 0f932a5b96..c478c71905 100644
> --- a/target/riscv/kvm.c
> +++ b/target/riscv/kvm.c
> @@ -42,6 +42,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)
>  {
> @@ -99,7 +101,7 @@ static uint64_t kvm_riscv_reg_id(CPURISCVState *env, uint64_t type,
>
>  #define KVM_RISCV_SET_TIMER(cs, env, name, reg) \
>      do { \
> -        int ret = kvm_set_one_reg(cs, RISCV_TIMER_REG(env, time), &reg); \
> +        int ret = kvm_set_one_reg(cs, RISCV_TIMER_REG(env, name), &reg); \
>          if (ret) { \
>              abort(); \
>          } \
> @@ -335,6 +337,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;
> @@ -354,6 +374,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;
>  }
>
> @@ -428,6 +460,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;
>  }
>
> @@ -506,10 +539,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 ed281bdce0..88aee902dd 100644
> --- a/target/riscv/kvm_riscv.h
> +++ b/target/riscv/kvm_riscv.h
> @@ -21,5 +21,6 @@
>
>  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
>


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: Re:  [PATCH] target/riscv: fix the issue of guest reboot then no response or crash in kvm-mode
  2023-07-10  1:16 ` Alistair Francis
@ 2023-07-18 11:06   ` 18622748025
  2023-07-18 11:08   ` 18622748025
  2023-07-18 11:44   ` Re:Re:[PATCH v3] " liguang.zhang
  2 siblings, 0 replies; 29+ messages in thread
From: 18622748025 @ 2023-07-18 11:06 UTC (permalink / raw)
  To: alistair23; +Cc: qemu-devel, pbonzini, liguang.zhang

[-- Attachment #1: Type: text/plain, Size: 6017 bytes --]

On Sun, July 18, 2023 at 6:55 PM liguang.zhang <18622748025@163.com> wrote:

On Sun, Jun 25, 2023 at 12:50 PM liguang.zhang <18622748025@163.com> wrote:
From: "liguang.zhang" <liguang.zhang@hexintek.com>

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.

Thanks for the patch.

I think we should move everything
--- from here ---


kernel log
```shell
$reboot

The system is going down NOW!
Sent SIGTERM to all processes
logout
Sent SIGKILL to all processes
Requesting system reboot

```
then no response

for qemu command:
$system_reset:

kernel log:
```shell
[   53.739556] kvm [150]: VCPU exit error -95
[   53.739563] kvm [148]: VCPU exit error -95
[   53.739557] kvm [149]: VCPU exit error -95
[   53.740957] kvm [149]: SEPC=0x0 SSTATUS=0x200004120 HSTATUS=0x2002001c0
[   53.740957] kvm [148]: SEPC=0x0 SSTATUS=0x200004120 HSTATUS=0x2002001c0
[   53.741054] kvm [148]: SCAUSE=0x14 STVAL=0x0 HTVAL=0x0 HTINST=0x0
[   53.741058] kvm [149]: SCAUSE=0x14 STVAL=0x0 HTVAL=0x0 HTINST=0x0
[   53.756187] kvm [150]: SEPC=0x0 SSTATUS=0x200004120 HSTATUS=0x2002001c0
[   53.757797] kvm [150]: SCAUSE=0x14 STVAL=0x0 HTVAL=0x0 HTINST=0x0
```

solution:

add reset csr and context for riscv vcpu
qemu ioctl reset vcpu->arch.power_off state of kvm

tests:

qemu-system-riscv64 -M virt -bios none -kernel Image \
-smp 4 -enable-kvm \
-append "rootwait root=/dev/vda ro" \
-drive file=rootfs.ext2,format=raw,id=hd0 \
-device virtio-blk-device,drive=hd0

in guest shell:
$reboot

qemu command:
$system_reset

---
v3:
- change kvm_riscv_set_mpstate_to_kvm to kvm_riscv_sync_mpstate_to_kvm
- remove newline after if(cap_has_mp_state)
- update submit description

--- to here ---


Signed-off-by: liguang.zhang <liguang.zhang@hexintek.com>
---

below this line. That way it will be included in the patch submission,
but won't be included in the git tree.

We never include the patch changelog in the git tree and I don't think
we need to include the steps as well (they will be preserved on the
mailing list).

Ok, It's no problem.

For the patch title (the git commit title) can you explain what the
patch is doing?

How about the title use:
"target/riscv: Clearing the CSR values at reset and syncing the MPSTATE with the host"


And the reason description is


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.


Also, do you mind rebasing on
https://github.com/alistair23/qemu/tree/riscv-to-apply.next

I don't remind rebase on it.
Thanks~
-- liguang.zhang

Then the patch should be good to go!

Alistair

target/riscv/kvm.c       | 44 +++++++++++++++++++++++++++++++++++++++-
target/riscv/kvm_riscv.h |  1 +
2 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/target/riscv/kvm.c b/target/riscv/kvm.c
index 0f932a5b96..c478c71905 100644
--- a/target/riscv/kvm.c
+++ b/target/riscv/kvm.c
@@ -42,6 +42,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)
{
@@ -99,7 +101,7 @@ static uint64_t kvm_riscv_reg_id(CPURISCVState *env, uint64_t type,

#define KVM_RISCV_SET_TIMER(cs, env, name, reg) \
do { \
-        int ret = kvm_set_one_reg(cs, RISCV_TIMER_REG(env, time), &reg); \
+        int ret = kvm_set_one_reg(cs, RISCV_TIMER_REG(env, name), &reg); \
if (ret) { \
abort(); \
} \
@@ -335,6 +337,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;
@@ -354,6 +374,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;
}

@@ -428,6 +460,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;
}

@@ -506,10 +539,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 ed281bdce0..88aee902dd 100644
--- a/target/riscv/kvm_riscv.h
+++ b/target/riscv/kvm_riscv.h
@@ -21,5 +21,6 @@

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

[-- Attachment #2: Type: text/html, Size: 36194 bytes --]

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: Re: [PATCH] target/riscv: fix the issue of guest reboot then no response or crash in kvm-mode
  2023-07-10  1:16 ` Alistair Francis
  2023-07-18 11:06   ` 18622748025
@ 2023-07-18 11:08   ` 18622748025
  2023-07-18 11:44   ` Re:Re:[PATCH v3] " liguang.zhang
  2 siblings, 0 replies; 29+ messages in thread
From: 18622748025 @ 2023-07-18 11:08 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, liguang.zhang, alistair23

[-- Attachment #1: Type: text/plain, Size: 6017 bytes --]

On Sun, July 18, 2023 at 6:55 PM liguang.zhang <18622748025@163.com> wrote:

On Sun, Jun 25, 2023 at 12:50 PM liguang.zhang <18622748025@163.com> wrote:
From: "liguang.zhang" <liguang.zhang@hexintek.com>

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.

Thanks for the patch.

I think we should move everything
--- from here ---


kernel log
```shell
$reboot

The system is going down NOW!
Sent SIGTERM to all processes
logout
Sent SIGKILL to all processes
Requesting system reboot

```
then no response

for qemu command:
$system_reset:

kernel log:
```shell
[   53.739556] kvm [150]: VCPU exit error -95
[   53.739563] kvm [148]: VCPU exit error -95
[   53.739557] kvm [149]: VCPU exit error -95
[   53.740957] kvm [149]: SEPC=0x0 SSTATUS=0x200004120 HSTATUS=0x2002001c0
[   53.740957] kvm [148]: SEPC=0x0 SSTATUS=0x200004120 HSTATUS=0x2002001c0
[   53.741054] kvm [148]: SCAUSE=0x14 STVAL=0x0 HTVAL=0x0 HTINST=0x0
[   53.741058] kvm [149]: SCAUSE=0x14 STVAL=0x0 HTVAL=0x0 HTINST=0x0
[   53.756187] kvm [150]: SEPC=0x0 SSTATUS=0x200004120 HSTATUS=0x2002001c0
[   53.757797] kvm [150]: SCAUSE=0x14 STVAL=0x0 HTVAL=0x0 HTINST=0x0
```

solution:

add reset csr and context for riscv vcpu
qemu ioctl reset vcpu->arch.power_off state of kvm

tests:

qemu-system-riscv64 -M virt -bios none -kernel Image \
-smp 4 -enable-kvm \
-append "rootwait root=/dev/vda ro" \
-drive file=rootfs.ext2,format=raw,id=hd0 \
-device virtio-blk-device,drive=hd0

in guest shell:
$reboot

qemu command:
$system_reset

---
v3:
- change kvm_riscv_set_mpstate_to_kvm to kvm_riscv_sync_mpstate_to_kvm
- remove newline after if(cap_has_mp_state)
- update submit description

--- to here ---


Signed-off-by: liguang.zhang <liguang.zhang@hexintek.com>
---

below this line. That way it will be included in the patch submission,
but won't be included in the git tree.

We never include the patch changelog in the git tree and I don't think
we need to include the steps as well (they will be preserved on the
mailing list).

Ok, It's no problem.

For the patch title (the git commit title) can you explain what the
patch is doing?

How about the title use:
"target/riscv: Clearing the CSR values at reset and syncing the MPSTATE with the host"


And the reason description is


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.


Also, do you mind rebasing on
https://github.com/alistair23/qemu/tree/riscv-to-apply.next

I don't remind rebase on it.
Thanks~
-- liguang.zhang

Then the patch should be good to go!

Alistair

target/riscv/kvm.c       | 44 +++++++++++++++++++++++++++++++++++++++-
target/riscv/kvm_riscv.h |  1 +
2 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/target/riscv/kvm.c b/target/riscv/kvm.c
index 0f932a5b96..c478c71905 100644
--- a/target/riscv/kvm.c
+++ b/target/riscv/kvm.c
@@ -42,6 +42,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)
{
@@ -99,7 +101,7 @@ static uint64_t kvm_riscv_reg_id(CPURISCVState *env, uint64_t type,

#define KVM_RISCV_SET_TIMER(cs, env, name, reg) \
do { \
-        int ret = kvm_set_one_reg(cs, RISCV_TIMER_REG(env, time), &reg); \
+        int ret = kvm_set_one_reg(cs, RISCV_TIMER_REG(env, name), &reg); \
if (ret) { \
abort(); \
} \
@@ -335,6 +337,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;
@@ -354,6 +374,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;
}

@@ -428,6 +460,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;
}

@@ -506,10 +539,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 ed281bdce0..88aee902dd 100644
--- a/target/riscv/kvm_riscv.h
+++ b/target/riscv/kvm_riscv.h
@@ -21,5 +21,6 @@

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

[-- Attachment #2: Type: text/html, Size: 37139 bytes --]

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re:Re:[PATCH v3] target/riscv: fix the issue of guest reboot then no response or crash in kvm-mode
  2023-07-10  1:16 ` Alistair Francis
  2023-07-18 11:06   ` 18622748025
  2023-07-18 11:08   ` 18622748025
@ 2023-07-18 11:44   ` liguang.zhang
  2 siblings, 0 replies; 29+ messages in thread
From: liguang.zhang @ 2023-07-18 11:44 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, alistair23, liguang.zhang

From: "liguang.zhang" <liguang.zhang@hexintek.com>

On Sun, July 18, 2023 at 6:55 PM liguang.zhang <18622748025@163.com> wrote:

> On Sun, July 10, 2023 at 9:17 alistair <alistair23@gmail.com> wrote:
> 
> > From: "liguang.zhang" <liguang.zhang@hexintek.com>
> > 
> > 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.
> 
> Thanks for the patch.
> 
> I think we should move everything
> 
> --- from here ---
> 
> 
> > kernel log
> > ```shell
> > $reboot
> > 
> > The system is going down NOW!
> > Sent SIGTERM to all processes
> > logout
> > Sent SIGKILL to all processes
> > Requesting system reboot
> > 
> > ```
> > then no response
> > 
> > for qemu command:
> > $system_reset:
> > 
> > kernel log:
> > ```shell
> > [   53.739556] kvm [150]: VCPU exit error -95
> > [   53.739563] kvm [148]: VCPU exit error -95
> > [   53.739557] kvm [149]: VCPU exit error -95
> > [   53.740957] kvm [149]: SEPC=0x0 SSTATUS=0x200004120 HSTATUS=0x2002001c0
> > [   53.740957] kvm [148]: SEPC=0x0 SSTATUS=0x200004120 HSTATUS=0x2002001c0
> > [   53.741054] kvm [148]: SCAUSE=0x14 STVAL=0x0 HTVAL=0x0 HTINST=0x0
> > [   53.741058] kvm [149]: SCAUSE=0x14 STVAL=0x0 HTVAL=0x0 HTINST=0x0
> > [   53.756187] kvm [150]: SEPC=0x0 SSTATUS=0x200004120 HSTATUS=0x2002001c0
> > [   53.757797] kvm [150]: SCAUSE=0x14 STVAL=0x0 HTVAL=0x0 HTINST=0x0
> > ```
> > 
> > solution:
> > 
> > add reset csr and context for riscv vcpu
> > qemu ioctl reset vcpu->arch.power_off state of kvm
> > 
> > tests:
> > 
> > qemu-system-riscv64 -M virt -bios none -kernel Image \
> > -smp 4 -enable-kvm \
> > -append "rootwait root=/dev/vda ro" \
> > -drive file=rootfs.ext2,format=raw,id=hd0 \
> > -device virtio-blk-device,drive=hd0
> > 
> > in guest shell:
> > $reboot
> > 
> > qemu command:
> > $system_reset
> > 
> > ---
> > v3:
> > - change kvm_riscv_set_mpstate_to_kvm to kvm_riscv_sync_mpstate_to_kvm
> > - remove newline after if(cap_has_mp_state)
> > - update submit description
> > 
> --- to here ---
> 
> 
> > Signed-off-by: liguang.zhang <liguang.zhang@hexintek.com>
> > ---
> 
> below this line. That way it will be included in the patch submission,
> but won't be included in the git tree.
> 
> We never include the patch changelog in the git tree and I don't think
> we need to include the steps as well (they will be preserved on the
> mailing list).
> 

Ok, It's no problem.

> 
> For the patch title (the git commit title) can you explain what the
> patch is doing?
> 
How about the title use:
"target/riscv: Clearing the CSR values at reset and syncing the MPSTATE with the host"

And the reason description is

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.

> 
> Also, do you mind rebasing on
> https://github.com/alistair23/qemu/tree/riscv-to-apply.next
> 
I don't remind rebase on it.
Thanks~
-- liguang.zhang
> 
> Then the patch should be good to go!
> 
> Alistair
> 
> target/riscv/kvm.c       | 44 +++++++++++++++++++++++++++++++++++++++-
> target/riscv/kvm_riscv.h |  1 +
> 2 files changed, 44 insertions(+), 1 deletion(-)
> 
> diff --git a/target/riscv/kvm.c b/target/riscv/kvm.c
> index 0f932a5b96..c478c71905 100644
> --- a/target/riscv/kvm.c
> +++ b/target/riscv/kvm.c
> @@ -42,6 +42,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)
> {
> @@ -99,7 +101,7 @@ static uint64_t kvm_riscv_reg_id(CPURISCVState *env, uint64_t type,
> 
> #define KVM_RISCV_SET_TIMER(cs, env, name, reg) \
> do { \
> -        int ret = kvm_set_one_reg(cs, RISCV_TIMER_REG(env, time), &reg); \
> +        int ret = kvm_set_one_reg(cs, RISCV_TIMER_REG(env, name), &reg); \
> if (ret) { \
> abort(); \
> } \
> @@ -335,6 +337,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;
> @@ -354,6 +374,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;
> }
> 
> @@ -428,6 +460,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;
> }
> 
> @@ -506,10 +539,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 ed281bdce0..88aee902dd 100644
> --- a/target/riscv/kvm_riscv.h
> +++ b/target/riscv/kvm_riscv.h
> @@ -21,5 +21,6 @@
> 
> 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



^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re:Re:[PATCH v3] target/riscv: fix the issue of guest reboot then no response or crash in kvm-mode
  2023-06-25  2:50 [PATCH] target/riscv: fix the issue of guest reboot then no response or crash in kvm-mode liguang.zhang
  2023-07-10  1:16 ` Alistair Francis
@ 2023-07-18 11:49 ` liguang.zhang
  2023-07-18 12:21 ` [PATCH] target/riscv: Clearing the CSR values at reset and syncing the MPSTATE with the host liguang.zhang
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 29+ messages in thread
From: liguang.zhang @ 2023-07-18 11:49 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, alistair23, liguang.zhang

From: "liguang.zhang" <liguang.zhang@hexintek.com>

On Sun, July 18, 2023 at 6:55 PM liguang.zhang <18622748025@163.com> wrote:

> On Sun, July 10, 2023 at 9:17 alistair <alistair23@gmail.com> wrote:
> 
> > From: "liguang.zhang" <liguang.zhang@hexintek.com>
> > 
> > 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.
> 
> Thanks for the patch.
> 
> I think we should move everything
> 
> --- from here ---
> 
> 
> > kernel log
> > ```shell
> > $reboot
> > 
> > The system is going down NOW!
> > Sent SIGTERM to all processes
> > logout
> > Sent SIGKILL to all processes
> > Requesting system reboot
> > 
> > ```
> > then no response
> > 
> > for qemu command:
> > $system_reset:
> > 
> > kernel log:
> > ```shell
> > [   53.739556] kvm [150]: VCPU exit error -95
> > [   53.739563] kvm [148]: VCPU exit error -95
> > [   53.739557] kvm [149]: VCPU exit error -95
> > [   53.740957] kvm [149]: SEPC=0x0 SSTATUS=0x200004120 HSTATUS=0x2002001c0
> > [   53.740957] kvm [148]: SEPC=0x0 SSTATUS=0x200004120 HSTATUS=0x2002001c0
> > [   53.741054] kvm [148]: SCAUSE=0x14 STVAL=0x0 HTVAL=0x0 HTINST=0x0
> > [   53.741058] kvm [149]: SCAUSE=0x14 STVAL=0x0 HTVAL=0x0 HTINST=0x0
> > [   53.756187] kvm [150]: SEPC=0x0 SSTATUS=0x200004120 HSTATUS=0x2002001c0
> > [   53.757797] kvm [150]: SCAUSE=0x14 STVAL=0x0 HTVAL=0x0 HTINST=0x0
> > ```
> > 
> > solution:
> > 
> > add reset csr and context for riscv vcpu
> > qemu ioctl reset vcpu->arch.power_off state of kvm
> > 
> > tests:
> > 
> > qemu-system-riscv64 -M virt -bios none -kernel Image \
> > -smp 4 -enable-kvm \
> > -append "rootwait root=/dev/vda ro" \
> > -drive file=rootfs.ext2,format=raw,id=hd0 \
> > -device virtio-blk-device,drive=hd0
> > 
> > in guest shell:
> > $reboot
> > 
> > qemu command:
> > $system_reset
> > 
> > ---
> > v3:
> > - change kvm_riscv_set_mpstate_to_kvm to kvm_riscv_sync_mpstate_to_kvm
> > - remove newline after if(cap_has_mp_state)
> > - update submit description
> > 
> --- to here ---
> 
> 
> > Signed-off-by: liguang.zhang <liguang.zhang@hexintek.com>
> > ---
> 
> below this line. That way it will be included in the patch submission,
> but won't be included in the git tree.
> 
> We never include the patch changelog in the git tree and I don't think
> we need to include the steps as well (they will be preserved on the
> mailing list).
> 

Ok, It's no problem.

> 
> For the patch title (the git commit title) can you explain what the
> patch is doing?
> 
How about the title use:
"target/riscv: Clearing the CSR values at reset and syncing the MPSTATE with the host"

And the reason description is

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.

> 
> Also, do you mind rebasing on
> https://github.com/alistair23/qemu/tree/riscv-to-apply.next
> 
I don't remind rebase on it.
Thanks~
-- liguang.zhang
> 
> Then the patch should be good to go!
> 
> Alistair
> 
> target/riscv/kvm.c       | 44 +++++++++++++++++++++++++++++++++++++++-
> target/riscv/kvm_riscv.h |  1 +
> 2 files changed, 44 insertions(+), 1 deletion(-)
> 
> diff --git a/target/riscv/kvm.c b/target/riscv/kvm.c
> index 0f932a5b96..c478c71905 100644
> --- a/target/riscv/kvm.c
> +++ b/target/riscv/kvm.c
> @@ -42,6 +42,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)
> {
> @@ -99,7 +101,7 @@ static uint64_t kvm_riscv_reg_id(CPURISCVState *env, uint64_t type,
> 
> #define KVM_RISCV_SET_TIMER(cs, env, name, reg) \
> do { \
> -        int ret = kvm_set_one_reg(cs, RISCV_TIMER_REG(env, time), &reg); \
> +        int ret = kvm_set_one_reg(cs, RISCV_TIMER_REG(env, name), &reg); \
> if (ret) { \
> abort(); \
> } \
> @@ -335,6 +337,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;
> @@ -354,6 +374,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;
> }
> 
> @@ -428,6 +460,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;
> }
> 
> @@ -506,10 +539,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 ed281bdce0..88aee902dd 100644
> --- a/target/riscv/kvm_riscv.h
> +++ b/target/riscv/kvm_riscv.h
> @@ -21,5 +21,6 @@
> 
> 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



^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH] target/riscv: Clearing the CSR values at reset and syncing the MPSTATE with the host
  2023-06-25  2:50 [PATCH] target/riscv: fix the issue of guest reboot then no response or crash in kvm-mode liguang.zhang
  2023-07-10  1:16 ` Alistair Francis
  2023-07-18 11:49 ` liguang.zhang
@ 2023-07-18 12:21 ` liguang.zhang
  2023-07-24  3:30   ` Alistair Francis
  2023-07-18 12:39 ` [PATCH] " liguang.zhang
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 29+ messages in thread
From: liguang.zhang @ 2023-07-18 12:21 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, alistair23, liguang.zhang

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(+)

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



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH] target/riscv: Clearing the CSR values at reset and syncing the MPSTATE with the host
  2023-06-25  2:50 [PATCH] target/riscv: fix the issue of guest reboot then no response or crash in kvm-mode liguang.zhang
                   ` (2 preceding siblings ...)
  2023-07-18 12:21 ` [PATCH] target/riscv: Clearing the CSR values at reset and syncing the MPSTATE with the host liguang.zhang
@ 2023-07-18 12:39 ` liguang.zhang
  2023-07-18 12:43 ` liguang.zhang
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 29+ messages in thread
From: liguang.zhang @ 2023-07-18 12:39 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, alistair23, liguang.zhang

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(+)

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



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH] target/riscv: Clearing the CSR values at reset and syncing the MPSTATE with the host
  2023-06-25  2:50 [PATCH] target/riscv: fix the issue of guest reboot then no response or crash in kvm-mode liguang.zhang
                   ` (3 preceding siblings ...)
  2023-07-18 12:39 ` [PATCH] " liguang.zhang
@ 2023-07-18 12:43 ` liguang.zhang
  2023-07-18 12:47 ` [PATCH] target/riscv: fix the issue of guest reboot then no response or crash in kvm-mode liguang.zhang
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 29+ messages in thread
From: liguang.zhang @ 2023-07-18 12:43 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, alistair23, liguang.zhang

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(+)

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



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH] target/riscv: fix the issue of guest reboot then no response or crash in kvm-mode
  2023-06-25  2:50 [PATCH] target/riscv: fix the issue of guest reboot then no response or crash in kvm-mode liguang.zhang
                   ` (4 preceding siblings ...)
  2023-07-18 12:43 ` liguang.zhang
@ 2023-07-18 12:47 ` liguang.zhang
  2023-07-18 12:52 ` liguang.zhang
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 29+ messages in thread
From: liguang.zhang @ 2023-07-18 12:47 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, alistair23, liguang.zhang

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(+)

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



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH] target/riscv: fix the issue of guest reboot then no response or crash in kvm-mode
  2023-06-25  2:50 [PATCH] target/riscv: fix the issue of guest reboot then no response or crash in kvm-mode liguang.zhang
                   ` (5 preceding siblings ...)
  2023-07-18 12:47 ` [PATCH] target/riscv: fix the issue of guest reboot then no response or crash in kvm-mode liguang.zhang
@ 2023-07-18 12:52 ` liguang.zhang
  2023-07-18 13:21   ` [PATCH v4] " liguang.zhang
  2023-07-18 12:56 ` [PATCH] target/riscv: Clearing the CSR values at reset and syncing the MPSTATE with the host liguang.zhang
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 29+ messages in thread
From: liguang.zhang @ 2023-07-18 12:52 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, alistair23, liguang.zhang

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(+)

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



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH] target/riscv: Clearing the CSR values at reset and syncing the MPSTATE with the host
  2023-06-25  2:50 [PATCH] target/riscv: fix the issue of guest reboot then no response or crash in kvm-mode liguang.zhang
                   ` (6 preceding siblings ...)
  2023-07-18 12:52 ` liguang.zhang
@ 2023-07-18 12:56 ` liguang.zhang
  2023-07-18 12:58 ` liguang.zhang
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 29+ messages in thread
From: liguang.zhang @ 2023-07-18 12:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, alistair23, liguang.zhang

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(+)

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



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH] target/riscv: Clearing the CSR values at reset and syncing the MPSTATE with the host
  2023-06-25  2:50 [PATCH] target/riscv: fix the issue of guest reboot then no response or crash in kvm-mode liguang.zhang
                   ` (7 preceding siblings ...)
  2023-07-18 12:56 ` [PATCH] target/riscv: Clearing the CSR values at reset and syncing the MPSTATE with the host liguang.zhang
@ 2023-07-18 12:58 ` liguang.zhang
  2023-07-18 13:02 ` liguang.zhang
  2023-07-18 13:07 ` [PATCH v4] target/riscv: fix the issue of guest reboot then no response or crash in kvm-mode liguang.zhang
  10 siblings, 0 replies; 29+ messages in thread
From: liguang.zhang @ 2023-07-18 12:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, alistair23, liguang.zhang

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(+)

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



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH] target/riscv: Clearing the CSR values at reset and syncing the MPSTATE with the host
  2023-06-25  2:50 [PATCH] target/riscv: fix the issue of guest reboot then no response or crash in kvm-mode liguang.zhang
                   ` (8 preceding siblings ...)
  2023-07-18 12:58 ` liguang.zhang
@ 2023-07-18 13:02 ` liguang.zhang
  2023-07-18 13:07 ` [PATCH v4] target/riscv: fix the issue of guest reboot then no response or crash in kvm-mode liguang.zhang
  10 siblings, 0 replies; 29+ messages in thread
From: liguang.zhang @ 2023-07-18 13:02 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, alistair23, liguang.zhang

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(+)

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



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v4] target/riscv: fix the issue of guest reboot then no response or crash in kvm-mode
  2023-06-25  2:50 [PATCH] target/riscv: fix the issue of guest reboot then no response or crash in kvm-mode liguang.zhang
                   ` (9 preceding siblings ...)
  2023-07-18 13:02 ` liguang.zhang
@ 2023-07-18 13:07 ` liguang.zhang
  10 siblings, 0 replies; 29+ messages in thread
From: liguang.zhang @ 2023-07-18 13:07 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, alistair23, liguang.zhang

From: "liguang.zhang" <liguang.zhang@hexintek.com>

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

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(+)

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



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v4] target/riscv: fix the issue of guest reboot then no response or crash in kvm-mode
  2023-07-18 12:52 ` liguang.zhang
@ 2023-07-18 13:21   ` liguang.zhang
  0 siblings, 0 replies; 29+ messages in thread
From: liguang.zhang @ 2023-07-18 13:21 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, alistair23, liguang.zhang

From: "liguang.zhang" <liguang.zhang@hexintek.com>

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

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(+)

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



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH] target/riscv: Clearing the CSR values at reset and syncing the MPSTATE with the host
  2023-07-18 12:21 ` [PATCH] target/riscv: Clearing the CSR values at reset and syncing the MPSTATE with the host liguang.zhang
@ 2023-07-24  3:30   ` Alistair Francis
  2023-07-24  6:05     ` liguang.zhang
                       ` (2 more replies)
  0 siblings, 3 replies; 29+ messages in thread
From: Alistair Francis @ 2023-07-24  3:30 UTC (permalink / raw)
  To: liguang.zhang; +Cc: qemu-devel, pbonzini, liguang.zhang

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
>


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: Re: [PATCH] target/riscv: Clearing the CSR values at reset and syncing the MPSTATE with the host
  2023-07-24  3:30   ` Alistair Francis
@ 2023-07-24  6:05     ` liguang.zhang
  2023-08-10 18:43       ` Alistair Francis
  2023-07-24  6:12     ` [PATCH v2] " liguang.zhang
  2023-07-24  6:25     ` [PATCH v3] " liguang.zhang
  2 siblings, 1 reply; 29+ messages in thread
From: liguang.zhang @ 2023-07-24  6:05 UTC (permalink / raw)
  To: qemu-devel; +Cc: alistair23

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 1348 bytes --]

> 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 ~



^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v2] target/riscv: Clearing the CSR values at reset and syncing the MPSTATE with the host
  2023-07-24  3:30   ` Alistair Francis
  2023-07-24  6:05     ` liguang.zhang
@ 2023-07-24  6:12     ` liguang.zhang
  2023-07-24  6:25     ` [PATCH v3] " liguang.zhang
  2 siblings, 0 replies; 29+ messages in thread
From: liguang.zhang @ 2023-07-24  6:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: alistair23, liguang.zhang

From: "liguang.zhang" <liguang.zhang@hexintek.com>

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.

kernel log
```shell
$reboot

The system is going down NOW!
Sent SIGTERM to all processes
logout
Sent SIGKILL to all processes
Requesting system reboot

```
then no response

for qemu command:
$system_reset:

kernel log:
```shell
[   53.739556] kvm [150]: VCPU exit error -95
[   53.739563] kvm [148]: VCPU exit error -95
[   53.739557] kvm [149]: VCPU exit error -95
[   53.740957] kvm [149]: SEPC=0x0 SSTATUS=0x200004120 HSTATUS=0x2002001c0
[   53.740957] kvm [148]: SEPC=0x0 SSTATUS=0x200004120 HSTATUS=0x2002001c0
[   53.741054] kvm [148]: SCAUSE=0x14 STVAL=0x0 HTVAL=0x0 HTINST=0x0
[   53.741058] kvm [149]: SCAUSE=0x14 STVAL=0x0 HTVAL=0x0 HTINST=0x0
[   53.756187] kvm [150]: SEPC=0x0 SSTATUS=0x200004120 HSTATUS=0x2002001c0
[   53.757797] kvm [150]: SCAUSE=0x14 STVAL=0x0 HTVAL=0x0 HTINST=0x0
```

solution:

add reset csr and context for riscv vcpu
qemu ioctl reset vcpu->arch.power_off state of kvm

tests:

qemu-system-riscv64 -M virt -bios none -kernel Image \
   -smp 4 -enable-kvm \
   -append "rootwait root=/dev/vda ro" \
   -drive file=rootfs.ext2,format=raw,id=hd0 \
   -device virtio-blk-device,drive=hd0

in guest shell:
$reboot

qemu command:
$system_reset

---
v2:
- add reproduction steps

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

diff --git a/target/riscv/kvm.c b/target/riscv/kvm.c
index 0f932a5b96..c478c71905 100644
--- a/target/riscv/kvm.c
+++ b/target/riscv/kvm.c
@@ -42,6 +42,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)
 {
@@ -99,7 +101,7 @@ static uint64_t kvm_riscv_reg_id(CPURISCVState *env, uint64_t type,
 
 #define KVM_RISCV_SET_TIMER(cs, env, name, reg) \
     do { \
-        int ret = kvm_set_one_reg(cs, RISCV_TIMER_REG(env, time), &reg); \
+        int ret = kvm_set_one_reg(cs, RISCV_TIMER_REG(env, name), &reg); \
         if (ret) { \
             abort(); \
         } \
@@ -335,6 +337,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;
@@ -354,6 +374,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;
 }
 
@@ -428,6 +460,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;
 }
 
@@ -506,10 +539,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 ed281bdce0..88aee902dd 100644
--- a/target/riscv/kvm_riscv.h
+++ b/target/riscv/kvm_riscv.h
@@ -21,5 +21,6 @@
 
 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



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v3] target/riscv: Clearing the CSR values at reset and syncing the MPSTATE with the host
  2023-07-24  3:30   ` Alistair Francis
  2023-07-24  6:05     ` liguang.zhang
  2023-07-24  6:12     ` [PATCH v2] " liguang.zhang
@ 2023-07-24  6:25     ` liguang.zhang
  2023-08-10 18:34       ` Alistair Francis
  2 siblings, 1 reply; 29+ messages in thread
From: liguang.zhang @ 2023-07-24  6:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: alistair23, liguang.zhang

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(+)

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.41.0



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH v3] target/riscv: Clearing the CSR values at reset and syncing the MPSTATE with the host
  2023-07-24  6:25     ` [PATCH v3] " liguang.zhang
@ 2023-08-10 18:34       ` Alistair Francis
  2023-09-13  8:37         ` liguang.zhang
  0 siblings, 1 reply; 29+ messages in thread
From: Alistair Francis @ 2023-08-10 18:34 UTC (permalink / raw)
  To: liguang.zhang; +Cc: qemu-devel, liguang.zhang

On Mon, Jul 24, 2023 at 2:26 AM 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>
> ---
>  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;
> +        }
> +    }

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?

Alistair

> +
>      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.41.0
>


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: Re: [PATCH] target/riscv: Clearing the CSR values at reset and syncing the MPSTATE with the host
  2023-07-24  6:05     ` liguang.zhang
@ 2023-08-10 18:43       ` Alistair Francis
  0 siblings, 0 replies; 29+ messages in thread
From: Alistair Francis @ 2023-08-10 18:43 UTC (permalink / raw)
  To: liguang.zhang; +Cc: qemu-devel

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 ~
>


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: Re: [PATCH v3] target/riscv: Clearing the CSR values at reset and syncing the MPSTATE with the host
  2023-08-10 18:34       ` Alistair Francis
@ 2023-09-13  8:37         ` liguang.zhang
  2023-09-13  9:13           ` [PATCH v4] " liguang.zhang
  0 siblings, 1 reply; 29+ messages in thread
From: liguang.zhang @ 2023-09-13  8:37 UTC (permalink / raw)
  To: qemu-devel; +Cc: alistair23, liguang.zhang

From: "liguang.zhang" <liguang.zhang@hexintek.com>

>  On Mon, Jul 24, 2023 at 2:26 AM 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>
>  > ---
>  >  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;
>  > +        }
>  > +    }
>  
>  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?
>  
>  Alistair

I think don't need to add code to kvm_arch_get_registers, because i just add reset behavior 
to kvm_arch_put_registers, system reset will come to this point; this is just an suitable entry.
There is no code using when vcpu reset, qemu don't need to get register from vcpu.
According the reset logic of riscv, vcpu is just in pause state in kvm but not destroyed.
Just when qemu reset all system point, call in this entry, and sync vcpu power state and register state to kvm is ok.
And it don't need to modify kvm code to adapt with this change.

2 scenarios of reboot:
1. When a reboot executed from the guest's internal shell enters KVM through SBI, it sets all vcpu->arch.power_off to 'on,' effectively powering off all vCPUs
2. When QEMU issues 'system_reset,' it does not handle kvm's vcpu->arch.power_off; instead, it simply causes all vCPU threads to exit the KVM_RUN loop, effectively ending the ioctl(KVM_RUN) call. This behavior is similar to the guest shell reboot. This also explains why in non-SMP scenarios, 'system_reset' can perform a normal restart. Because the vCPU responsible for booting (boot_hart) remains powered on, it can be scheduled properly.so just to actively send a vCPU power-off command to KVM for vCPUs other than boot_hart, causing the vcpu->arch.power_off state of non-boot_hart vCPUs to return to 'on'
and boot_hart vcpu in kernel call up other vcpu by opensbi hsm_hart_start ecall to kvm hypervisor change the power_off state to off.

```log
$ reboot
The system is going down NOW!
Sent SIGTERM to all processes
Sent SIGKILL to all processes
Requesting system reboot
[   90.660001] reboot: Restarting system
--------- all smp vcpu means paused not destroyed ------------------
vcpu csrs registers all in a intermediate state, and vcpu in power_off state
[just a kvm vcpu flag vcpu->arch.power_off=true]
---------------- qemu need at system reset point end stage call vcpu up and reset its registers csrs-------------

as this flow https://gitee.com/zhangliguang1/tuchuang/raw/master/img/20230913161603.png

qemu_system_reset
qemu_devices_reset
cpu_synchronize_all_post_reset
do_kvm_cpu_synchronize_post_reset
kvm_arch_put_registers(cpu, KVM_PUT_RESET_STATE)

```

Thanks~

from liguang.zhang@hexintek.com

>  
>  > +
>  >      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.41.0
>  >



^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v4] target/riscv: Clearing the CSR values at reset and syncing the MPSTATE with the host
  2023-09-13  8:37         ` liguang.zhang
@ 2023-09-13  9:13           ` liguang.zhang
  2023-09-19  4:05             ` Alistair Francis
                               ` (2 more replies)
  0 siblings, 3 replies; 29+ messages in thread
From: liguang.zhang @ 2023-09-13  9:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: alistair23, liguang.zhang

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.

v4 update:
rebase on riscv-to-apply

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(+)

diff --git a/target/riscv/kvm.c b/target/riscv/kvm.c
index c01cfb03f4..8ee410b9b1 100644
--- a/target/riscv/kvm.c
+++ b/target/riscv/kvm.c
@@ -51,6 +51,8 @@ void riscv_kvm_aplic_request(void *opaque, int irq, int level)
     kvm_set_irq(kvm_state, irq, !!level);
 }
 
+static bool cap_has_mp_state;
+
 static uint64_t kvm_riscv_reg_id(CPURISCVState *env, uint64_t type,
                                  uint64_t idx)
 {
@@ -797,6 +799,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;
@@ -816,6 +836,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;
 }
 
@@ -928,6 +960,7 @@ int kvm_arch_get_default_type(MachineState *ms)
 
 int kvm_arch_init(MachineState *ms, KVMState *s)
 {
+    cap_has_mp_state = kvm_check_extension(s, KVM_CAP_MP_STATE);
     return 0;
 }
 
@@ -1014,10 +1047,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 de8c209ebc..8f8c1f969a 100644
--- a/target/riscv/kvm_riscv.h
+++ b/target/riscv/kvm_riscv.h
@@ -27,5 +27,6 @@ void kvm_riscv_aia_create(MachineState *machine, uint64_t group_shift,
                           uint64_t aplic_base, uint64_t imsic_base,
                           uint64_t guest_num);
 void riscv_kvm_aplic_request(void *opaque, int irq, int level);
+int kvm_riscv_sync_mpstate_to_kvm(RISCVCPU *cpu, int state);
 
 #endif
-- 
2.41.0



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH v4] target/riscv: Clearing the CSR values at reset and syncing the MPSTATE with the host
  2023-09-13  9:13           ` [PATCH v4] " liguang.zhang
@ 2023-09-19  4:05             ` Alistair Francis
  2023-09-19  4:12             ` Alistair Francis
  2023-09-19  4:16             ` Alistair Francis
  2 siblings, 0 replies; 29+ messages in thread
From: Alistair Francis @ 2023-09-19  4:05 UTC (permalink / raw)
  To: liguang.zhang; +Cc: qemu-devel, liguang.zhang

On Wed, Sep 13, 2023 at 7:14 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.
>
> v4 update:
> rebase on riscv-to-apply

This should be below the line

>
> Signed-off-by: liguang.zhang <liguang.zhang@hexintek.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

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 c01cfb03f4..8ee410b9b1 100644
> --- a/target/riscv/kvm.c
> +++ b/target/riscv/kvm.c
> @@ -51,6 +51,8 @@ void riscv_kvm_aplic_request(void *opaque, int irq, int level)
>      kvm_set_irq(kvm_state, irq, !!level);
>  }
>
> +static bool cap_has_mp_state;
> +
>  static uint64_t kvm_riscv_reg_id(CPURISCVState *env, uint64_t type,
>                                   uint64_t idx)
>  {
> @@ -797,6 +799,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;
> @@ -816,6 +836,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;
>  }
>
> @@ -928,6 +960,7 @@ int kvm_arch_get_default_type(MachineState *ms)
>
>  int kvm_arch_init(MachineState *ms, KVMState *s)
>  {
> +    cap_has_mp_state = kvm_check_extension(s, KVM_CAP_MP_STATE);
>      return 0;
>  }
>
> @@ -1014,10 +1047,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 de8c209ebc..8f8c1f969a 100644
> --- a/target/riscv/kvm_riscv.h
> +++ b/target/riscv/kvm_riscv.h
> @@ -27,5 +27,6 @@ void kvm_riscv_aia_create(MachineState *machine, uint64_t group_shift,
>                            uint64_t aplic_base, uint64_t imsic_base,
>                            uint64_t guest_num);
>  void riscv_kvm_aplic_request(void *opaque, int irq, int level);
> +int kvm_riscv_sync_mpstate_to_kvm(RISCVCPU *cpu, int state);
>
>  #endif
> --
> 2.41.0
>


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v4] target/riscv: Clearing the CSR values at reset and syncing the MPSTATE with the host
  2023-09-13  9:13           ` [PATCH v4] " liguang.zhang
  2023-09-19  4:05             ` Alistair Francis
@ 2023-09-19  4:12             ` Alistair Francis
  2023-09-19  4:16             ` Alistair Francis
  2 siblings, 0 replies; 29+ messages in thread
From: Alistair Francis @ 2023-09-19  4:12 UTC (permalink / raw)
  To: liguang.zhang; +Cc: qemu-devel, liguang.zhang

On Wed, Sep 13, 2023 at 7:14 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.
>
> v4 update:
> rebase on riscv-to-apply
>
> Signed-off-by: liguang.zhang <liguang.zhang@hexintek.com>

This patch fails checkpatch. You should run checkpatch on all patches
before submitting them

    git show | ./scripts/checkpatch.pl -

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 c01cfb03f4..8ee410b9b1 100644
> --- a/target/riscv/kvm.c
> +++ b/target/riscv/kvm.c
> @@ -51,6 +51,8 @@ void riscv_kvm_aplic_request(void *opaque, int irq, int level)
>      kvm_set_irq(kvm_state, irq, !!level);
>  }
>
> +static bool cap_has_mp_state;
> +
>  static uint64_t kvm_riscv_reg_id(CPURISCVState *env, uint64_t type,
>                                   uint64_t idx)
>  {
> @@ -797,6 +799,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;
> @@ -816,6 +836,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;
>  }
>
> @@ -928,6 +960,7 @@ int kvm_arch_get_default_type(MachineState *ms)
>
>  int kvm_arch_init(MachineState *ms, KVMState *s)
>  {
> +    cap_has_mp_state = kvm_check_extension(s, KVM_CAP_MP_STATE);
>      return 0;
>  }
>
> @@ -1014,10 +1047,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 de8c209ebc..8f8c1f969a 100644
> --- a/target/riscv/kvm_riscv.h
> +++ b/target/riscv/kvm_riscv.h
> @@ -27,5 +27,6 @@ void kvm_riscv_aia_create(MachineState *machine, uint64_t group_shift,
>                            uint64_t aplic_base, uint64_t imsic_base,
>                            uint64_t guest_num);
>  void riscv_kvm_aplic_request(void *opaque, int irq, int level);
> +int kvm_riscv_sync_mpstate_to_kvm(RISCVCPU *cpu, int state);
>
>  #endif
> --
> 2.41.0
>


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v4] target/riscv: Clearing the CSR values at reset and syncing the MPSTATE with the host
  2023-09-13  9:13           ` [PATCH v4] " liguang.zhang
  2023-09-19  4:05             ` Alistair Francis
  2023-09-19  4:12             ` Alistair Francis
@ 2023-09-19  4:16             ` Alistair Francis
  2 siblings, 0 replies; 29+ messages in thread
From: Alistair Francis @ 2023-09-19  4:16 UTC (permalink / raw)
  To: liguang.zhang; +Cc: qemu-devel, liguang.zhang

On Wed, Sep 13, 2023 at 7:14 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.
>
> v4 update:
> rebase on riscv-to-apply
>
> Signed-off-by: liguang.zhang <liguang.zhang@hexintek.com>

I have fixed up some of the issues and applied this

Thanks!

Applied to riscv-to-apply.next

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 c01cfb03f4..8ee410b9b1 100644
> --- a/target/riscv/kvm.c
> +++ b/target/riscv/kvm.c
> @@ -51,6 +51,8 @@ void riscv_kvm_aplic_request(void *opaque, int irq, int level)
>      kvm_set_irq(kvm_state, irq, !!level);
>  }
>
> +static bool cap_has_mp_state;
> +
>  static uint64_t kvm_riscv_reg_id(CPURISCVState *env, uint64_t type,
>                                   uint64_t idx)
>  {
> @@ -797,6 +799,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;
> @@ -816,6 +836,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;
>  }
>
> @@ -928,6 +960,7 @@ int kvm_arch_get_default_type(MachineState *ms)
>
>  int kvm_arch_init(MachineState *ms, KVMState *s)
>  {
> +    cap_has_mp_state = kvm_check_extension(s, KVM_CAP_MP_STATE);
>      return 0;
>  }
>
> @@ -1014,10 +1047,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 de8c209ebc..8f8c1f969a 100644
> --- a/target/riscv/kvm_riscv.h
> +++ b/target/riscv/kvm_riscv.h
> @@ -27,5 +27,6 @@ void kvm_riscv_aia_create(MachineState *machine, uint64_t group_shift,
>                            uint64_t aplic_base, uint64_t imsic_base,
>                            uint64_t guest_num);
>  void riscv_kvm_aplic_request(void *opaque, int irq, int level);
> +int kvm_riscv_sync_mpstate_to_kvm(RISCVCPU *cpu, int state);
>
>  #endif
> --
> 2.41.0
>


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH] target/riscv: Clearing the CSR values at reset and syncing the MPSTATE with the host
  2023-06-23  2:15 [PATCH] " Alistair Francis
  2023-07-18 12:24 ` [PATCH] target/riscv: Clearing the CSR values at reset and syncing the MPSTATE with the host liguang.zhang
@ 2023-07-18 12:29 ` liguang.zhang
  1 sibling, 0 replies; 29+ messages in thread
From: liguang.zhang @ 2023-07-18 12:29 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, alistair23, liguang.zhang

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(+)

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



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH] target/riscv: Clearing the CSR values at reset and syncing the MPSTATE with the host
  2023-06-23  2:15 [PATCH] " Alistair Francis
@ 2023-07-18 12:24 ` liguang.zhang
  2023-07-18 12:29 ` liguang.zhang
  1 sibling, 0 replies; 29+ messages in thread
From: liguang.zhang @ 2023-07-18 12:24 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, alistair23, liguang.zhang

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(+)

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



^ permalink raw reply related	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2023-09-19  4:17 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-25  2:50 [PATCH] target/riscv: fix the issue of guest reboot then no response or crash in kvm-mode liguang.zhang
2023-07-10  1:16 ` Alistair Francis
2023-07-18 11:06   ` 18622748025
2023-07-18 11:08   ` 18622748025
2023-07-18 11:44   ` Re:Re:[PATCH v3] " liguang.zhang
2023-07-18 11:49 ` liguang.zhang
2023-07-18 12:21 ` [PATCH] target/riscv: Clearing the CSR values at reset and syncing the MPSTATE with the host liguang.zhang
2023-07-24  3:30   ` Alistair Francis
2023-07-24  6:05     ` liguang.zhang
2023-08-10 18:43       ` Alistair Francis
2023-07-24  6:12     ` [PATCH v2] " liguang.zhang
2023-07-24  6:25     ` [PATCH v3] " liguang.zhang
2023-08-10 18:34       ` Alistair Francis
2023-09-13  8:37         ` liguang.zhang
2023-09-13  9:13           ` [PATCH v4] " liguang.zhang
2023-09-19  4:05             ` Alistair Francis
2023-09-19  4:12             ` Alistair Francis
2023-09-19  4:16             ` Alistair Francis
2023-07-18 12:39 ` [PATCH] " liguang.zhang
2023-07-18 12:43 ` liguang.zhang
2023-07-18 12:47 ` [PATCH] target/riscv: fix the issue of guest reboot then no response or crash in kvm-mode liguang.zhang
2023-07-18 12:52 ` liguang.zhang
2023-07-18 13:21   ` [PATCH v4] " liguang.zhang
2023-07-18 12:56 ` [PATCH] target/riscv: Clearing the CSR values at reset and syncing the MPSTATE with the host liguang.zhang
2023-07-18 12:58 ` liguang.zhang
2023-07-18 13:02 ` liguang.zhang
2023-07-18 13:07 ` [PATCH v4] target/riscv: fix the issue of guest reboot then no response or crash in kvm-mode liguang.zhang
  -- strict thread matches above, loose matches on Subject: below --
2023-06-23  2:15 [PATCH] " Alistair Francis
2023-07-18 12:24 ` [PATCH] target/riscv: Clearing the CSR values at reset and syncing the MPSTATE with the host liguang.zhang
2023-07-18 12:29 ` liguang.zhang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).