[PATCH v2 0/8] riscv: AIA: kernel-irqchip=off support

[PATCH v2 0/8] riscv: AIA: kernel-irqchip=off support

[Yong-Xuan Wang]

This series introduces the user-space AIA MSI emulation when using KVM
acceleration.

After this series, RISC-V QEMU virt machine with KVM acceleration has
3 parameters to control the type of irqchip and its emulation method:
- Machine prop "aia" controls the type of irqchip
  - none: use PLIC and emulated in user-space
  - aplic: use AIA wired and emulated in user-space
  - aplic-imsic: use AIA MSI, emulation mode is determined by
    "kernel-irqchip" and "riscv-aia". This can only be used
    when the host machine support the AIA extension.
- Accel prop "kernel-irqchip", effective with AIA MSI
  - on: in-kernel APLIC and in-kernel IMSIC
  - off: user-space APLIC and user-space IMSIC
  - split: user-space APLIC and in-kernel IMSIC
- Accel prop "riscv-aia", effective with in-kernel IMSIC
  - emul: use MRIF as in-kernel IMSIC
  - hw: use hardware guest IMSIC file as in-kernel IMSIC
  - auto: use the hardware guest IMSICs whenever available otherwise
    fallback to MRIF

Glossary
- PLIC: Platform-Level Interrupt Controller
- AIA: Advanced Interrupt Architecture
- APLIC: Advanced Platform Level Interrupt Controller
- IMSIC: Incoming Message Signaled Interrupt Controller
- MSI: Message Signaled Interrupts
- MRIF: Message Routed Interrupt Facility

---
v2:
- Add glossary of RISC-V irqchips in cover letter (Kashyap)
- Add assertions in riscv_get_csr_name() of PATCH4 (Andrew)
- remove the changing of property name (Andrew)
- Add PATCH8 to prevent the usage of IMSIC devices when the host
  machine doesn't support the AIA extension.
- fix typo in cover letter

Yong-Xuan Wang (8):
  target/riscv/kvm: rewrite get/set for KVM_REG_RISCV_CSR
  target/riscv/kvm: add KVM_REG_RISCV_CSR_AIA
  target/riscv/kvm: add KVM_REG_RISCV_CSR_SMSTATEEN
  target/riscv: add helper to get CSR name
  target/riscv/kvm: rewrite kvm_riscv_handle_csr
  target/riscv/kvm: add CSR_SIREG and CSR_STOPEI emulation
  docs: update the description about RISC-V AIA
  hw/intc/imsic: prevent to use IMSIC when host doesn't support AIA
    extension

 docs/specs/riscv-aia.rst   |  24 +++-
 docs/system/riscv/virt.rst |  10 +-
 hw/intc/riscv_imsic.c      |  17 ++-
 target/riscv/cpu.h         |  10 +-
 target/riscv/csr.c         |  30 ++---
 target/riscv/kvm/kvm-cpu.c | 235 +++++++++++++++++++++++++++++++------
 6 files changed, 254 insertions(+), 72 deletions(-)

-- 
2.17.1

[PATCH v2 1/8] target/riscv/kvm: rewrite get/set for KVM_REG_RISCV_CSR

[Yong-Xuan Wang]

As KVM_REG_RISCV_CSR includes several subtypes of CSR, rewrite the
related macros and functions to prepare for other subtypes.

Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
---
 target/riscv/kvm/kvm-cpu.c | 70 +++++++++++++++++++++++---------------
 1 file changed, 43 insertions(+), 27 deletions(-)

diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index 471fd554b369..ff1211d2fe39 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -111,9 +111,8 @@ static uint64_t kvm_riscv_vector_reg_id(RISCVCPU *cpu,
     kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CORE, \
                            KVM_REG_RISCV_CORE_REG(name))
 
-#define RISCV_CSR_REG(env, name) \
-    kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CSR, \
-                           KVM_REG_RISCV_CSR_REG(name))
+#define RISCV_CSR_REG(env, idx) \
+    kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CSR, idx)
 
 #define RISCV_CONFIG_REG(env, name) \
     kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CONFIG, \
@@ -130,17 +129,20 @@ static uint64_t kvm_riscv_vector_reg_id(RISCVCPU *cpu,
     kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_VECTOR, \
                            KVM_REG_RISCV_VECTOR_CSR_REG(name))
 
-#define KVM_RISCV_GET_CSR(cs, env, csr, reg) \
+#define RISCV_GENERAL_CSR_REG(name) \
+    (KVM_REG_RISCV_CSR_GENERAL | KVM_REG_RISCV_CSR_REG(name))
+
+#define KVM_RISCV_GET_CSR(cs, env, idx, reg) \
     do { \
-        int _ret = kvm_get_one_reg(cs, RISCV_CSR_REG(env, csr), &reg); \
+        int _ret = kvm_get_one_reg(cs, RISCV_CSR_REG(env, idx), &reg); \
         if (_ret) { \
             return _ret; \
         } \
     } while (0)
 
-#define KVM_RISCV_SET_CSR(cs, env, csr, reg) \
+#define KVM_RISCV_SET_CSR(cs, env, idx, reg) \
     do { \
-        int _ret = kvm_set_one_reg(cs, RISCV_CSR_REG(env, csr), &reg); \
+        int _ret = kvm_set_one_reg(cs, RISCV_CSR_REG(env, idx), &reg); \
         if (_ret) { \
             return _ret; \
         } \
@@ -608,36 +610,50 @@ static int kvm_riscv_put_regs_core(CPUState *cs)
     return ret;
 }
 
-static int kvm_riscv_get_regs_csr(CPUState *cs)
+static int kvm_riscv_get_regs_general_csr(CPUState *cs)
 {
     CPURISCVState *env = &RISCV_CPU(cs)->env;
 
-    KVM_RISCV_GET_CSR(cs, env, sstatus, env->mstatus);
-    KVM_RISCV_GET_CSR(cs, env, sie, env->mie);
-    KVM_RISCV_GET_CSR(cs, env, stvec, env->stvec);
-    KVM_RISCV_GET_CSR(cs, env, sscratch, env->sscratch);
-    KVM_RISCV_GET_CSR(cs, env, sepc, env->sepc);
-    KVM_RISCV_GET_CSR(cs, env, scause, env->scause);
-    KVM_RISCV_GET_CSR(cs, env, stval, env->stval);
-    KVM_RISCV_GET_CSR(cs, env, sip, env->mip);
-    KVM_RISCV_GET_CSR(cs, env, satp, env->satp);
+    KVM_RISCV_GET_CSR(cs, env, RISCV_GENERAL_CSR_REG(sstatus), env->mstatus);
+    KVM_RISCV_GET_CSR(cs, env, RISCV_GENERAL_CSR_REG(sie), env->mie);
+    KVM_RISCV_GET_CSR(cs, env, RISCV_GENERAL_CSR_REG(stvec), env->stvec);
+    KVM_RISCV_GET_CSR(cs, env, RISCV_GENERAL_CSR_REG(sscratch), env->sscratch);
+    KVM_RISCV_GET_CSR(cs, env, RISCV_GENERAL_CSR_REG(sepc), env->sepc);
+    KVM_RISCV_GET_CSR(cs, env, RISCV_GENERAL_CSR_REG(scause), env->scause);
+    KVM_RISCV_GET_CSR(cs, env, RISCV_GENERAL_CSR_REG(stval), env->stval);
+    KVM_RISCV_GET_CSR(cs, env, RISCV_GENERAL_CSR_REG(sip), env->mip);
+    KVM_RISCV_GET_CSR(cs, env, RISCV_GENERAL_CSR_REG(satp), env->satp);
 
     return 0;
 }
 
-static int kvm_riscv_put_regs_csr(CPUState *cs)
+static int kvm_riscv_put_regs_general_csr(CPUState *cs)
 {
     CPURISCVState *env = &RISCV_CPU(cs)->env;
 
-    KVM_RISCV_SET_CSR(cs, env, sstatus, env->mstatus);
-    KVM_RISCV_SET_CSR(cs, env, sie, env->mie);
-    KVM_RISCV_SET_CSR(cs, env, stvec, env->stvec);
-    KVM_RISCV_SET_CSR(cs, env, sscratch, env->sscratch);
-    KVM_RISCV_SET_CSR(cs, env, sepc, env->sepc);
-    KVM_RISCV_SET_CSR(cs, env, scause, env->scause);
-    KVM_RISCV_SET_CSR(cs, env, stval, env->stval);
-    KVM_RISCV_SET_CSR(cs, env, sip, env->mip);
-    KVM_RISCV_SET_CSR(cs, env, satp, env->satp);
+    KVM_RISCV_SET_CSR(cs, env, RISCV_GENERAL_CSR_REG(sstatus), env->mstatus);
+    KVM_RISCV_SET_CSR(cs, env, RISCV_GENERAL_CSR_REG(sie), env->mie);
+    KVM_RISCV_SET_CSR(cs, env, RISCV_GENERAL_CSR_REG(stvec), env->stvec);
+    KVM_RISCV_SET_CSR(cs, env, RISCV_GENERAL_CSR_REG(sscratch), env->sscratch);
+    KVM_RISCV_SET_CSR(cs, env, RISCV_GENERAL_CSR_REG(sepc), env->sepc);
+    KVM_RISCV_SET_CSR(cs, env, RISCV_GENERAL_CSR_REG(scause), env->scause);
+    KVM_RISCV_SET_CSR(cs, env, RISCV_GENERAL_CSR_REG(stval), env->stval);
+    KVM_RISCV_SET_CSR(cs, env, RISCV_GENERAL_CSR_REG(sip), env->mip);
+    KVM_RISCV_SET_CSR(cs, env, RISCV_GENERAL_CSR_REG(satp), env->satp);
+
+    return 0;
+}
+
+static int kvm_riscv_get_regs_csr(CPUState *cs)
+{
+    kvm_riscv_get_regs_general_csr(cs);
+
+    return 0;
+}
+
+static int kvm_riscv_put_regs_csr(CPUState *cs)
+{
+    kvm_riscv_put_regs_general_csr(cs);
 
     return 0;
 }
-- 
2.17.1

Re: [PATCH v2 1/8] target/riscv/kvm: rewrite get/set for KVM_REG_RISCV_CSR

[Andrew Jones]

On Mon, Feb 24, 2025 at 04:24:08PM +0800, Yong-Xuan Wang wrote:
> As KVM_REG_RISCV_CSR includes several subtypes of CSR, rewrite the
> related macros and functions to prepare for other subtypes.
> 
> Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
> ---
>  target/riscv/kvm/kvm-cpu.c | 70 +++++++++++++++++++++++---------------
>  1 file changed, 43 insertions(+), 27 deletions(-)
> 
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index 471fd554b369..ff1211d2fe39 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -111,9 +111,8 @@ static uint64_t kvm_riscv_vector_reg_id(RISCVCPU *cpu,
>      kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CORE, \
>                             KVM_REG_RISCV_CORE_REG(name))
>  
> -#define RISCV_CSR_REG(env, name) \
> -    kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CSR, \
> -                           KVM_REG_RISCV_CSR_REG(name))
> +#define RISCV_CSR_REG(env, idx) \
> +    kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CSR, idx)
>  
>  #define RISCV_CONFIG_REG(env, name) \
>      kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CONFIG, \
> @@ -130,17 +129,20 @@ static uint64_t kvm_riscv_vector_reg_id(RISCVCPU *cpu,
>      kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_VECTOR, \
>                             KVM_REG_RISCV_VECTOR_CSR_REG(name))
>  
> -#define KVM_RISCV_GET_CSR(cs, env, csr, reg) \
> +#define RISCV_GENERAL_CSR_REG(name) \
> +    (KVM_REG_RISCV_CSR_GENERAL | KVM_REG_RISCV_CSR_REG(name))
> +
> +#define KVM_RISCV_GET_CSR(cs, env, idx, reg) \
>      do { \
> -        int _ret = kvm_get_one_reg(cs, RISCV_CSR_REG(env, csr), &reg); \
> +        int _ret = kvm_get_one_reg(cs, RISCV_CSR_REG(env, idx), &reg); \
>          if (_ret) { \
>              return _ret; \
>          } \
>      } while (0)
>  
> -#define KVM_RISCV_SET_CSR(cs, env, csr, reg) \
> +#define KVM_RISCV_SET_CSR(cs, env, idx, reg) \
>      do { \
> -        int _ret = kvm_set_one_reg(cs, RISCV_CSR_REG(env, csr), &reg); \
> +        int _ret = kvm_set_one_reg(cs, RISCV_CSR_REG(env, idx), &reg); \
>          if (_ret) { \
>              return _ret; \
>          } \
> @@ -608,36 +610,50 @@ static int kvm_riscv_put_regs_core(CPUState *cs)
>      return ret;
>  }
>  
> -static int kvm_riscv_get_regs_csr(CPUState *cs)
> +static int kvm_riscv_get_regs_general_csr(CPUState *cs)
>  {
>      CPURISCVState *env = &RISCV_CPU(cs)->env;
>  
> -    KVM_RISCV_GET_CSR(cs, env, sstatus, env->mstatus);
> -    KVM_RISCV_GET_CSR(cs, env, sie, env->mie);
> -    KVM_RISCV_GET_CSR(cs, env, stvec, env->stvec);
> -    KVM_RISCV_GET_CSR(cs, env, sscratch, env->sscratch);
> -    KVM_RISCV_GET_CSR(cs, env, sepc, env->sepc);
> -    KVM_RISCV_GET_CSR(cs, env, scause, env->scause);
> -    KVM_RISCV_GET_CSR(cs, env, stval, env->stval);
> -    KVM_RISCV_GET_CSR(cs, env, sip, env->mip);
> -    KVM_RISCV_GET_CSR(cs, env, satp, env->satp);
> +    KVM_RISCV_GET_CSR(cs, env, RISCV_GENERAL_CSR_REG(sstatus), env->mstatus);
> +    KVM_RISCV_GET_CSR(cs, env, RISCV_GENERAL_CSR_REG(sie), env->mie);
> +    KVM_RISCV_GET_CSR(cs, env, RISCV_GENERAL_CSR_REG(stvec), env->stvec);
> +    KVM_RISCV_GET_CSR(cs, env, RISCV_GENERAL_CSR_REG(sscratch), env->sscratch);
> +    KVM_RISCV_GET_CSR(cs, env, RISCV_GENERAL_CSR_REG(sepc), env->sepc);
> +    KVM_RISCV_GET_CSR(cs, env, RISCV_GENERAL_CSR_REG(scause), env->scause);
> +    KVM_RISCV_GET_CSR(cs, env, RISCV_GENERAL_CSR_REG(stval), env->stval);
> +    KVM_RISCV_GET_CSR(cs, env, RISCV_GENERAL_CSR_REG(sip), env->mip);
> +    KVM_RISCV_GET_CSR(cs, env, RISCV_GENERAL_CSR_REG(satp), env->satp);
>  
>      return 0;
>  }
>  
> -static int kvm_riscv_put_regs_csr(CPUState *cs)
> +static int kvm_riscv_put_regs_general_csr(CPUState *cs)
>  {
>      CPURISCVState *env = &RISCV_CPU(cs)->env;
>  
> -    KVM_RISCV_SET_CSR(cs, env, sstatus, env->mstatus);
> -    KVM_RISCV_SET_CSR(cs, env, sie, env->mie);
> -    KVM_RISCV_SET_CSR(cs, env, stvec, env->stvec);
> -    KVM_RISCV_SET_CSR(cs, env, sscratch, env->sscratch);
> -    KVM_RISCV_SET_CSR(cs, env, sepc, env->sepc);
> -    KVM_RISCV_SET_CSR(cs, env, scause, env->scause);
> -    KVM_RISCV_SET_CSR(cs, env, stval, env->stval);
> -    KVM_RISCV_SET_CSR(cs, env, sip, env->mip);
> -    KVM_RISCV_SET_CSR(cs, env, satp, env->satp);
> +    KVM_RISCV_SET_CSR(cs, env, RISCV_GENERAL_CSR_REG(sstatus), env->mstatus);
> +    KVM_RISCV_SET_CSR(cs, env, RISCV_GENERAL_CSR_REG(sie), env->mie);
> +    KVM_RISCV_SET_CSR(cs, env, RISCV_GENERAL_CSR_REG(stvec), env->stvec);
> +    KVM_RISCV_SET_CSR(cs, env, RISCV_GENERAL_CSR_REG(sscratch), env->sscratch);
> +    KVM_RISCV_SET_CSR(cs, env, RISCV_GENERAL_CSR_REG(sepc), env->sepc);
> +    KVM_RISCV_SET_CSR(cs, env, RISCV_GENERAL_CSR_REG(scause), env->scause);
> +    KVM_RISCV_SET_CSR(cs, env, RISCV_GENERAL_CSR_REG(stval), env->stval);
> +    KVM_RISCV_SET_CSR(cs, env, RISCV_GENERAL_CSR_REG(sip), env->mip);
> +    KVM_RISCV_SET_CSR(cs, env, RISCV_GENERAL_CSR_REG(satp), env->satp);
> +
> +    return 0;
> +}
> +
> +static int kvm_riscv_get_regs_csr(CPUState *cs)
> +{
> +    kvm_riscv_get_regs_general_csr(cs);

KVM_RISCV_GET/SET_CSR() can return errors so the return value should be
checked here and below (just kvm_arch_get/put_registers() checks the
returns values of kvm_riscv_get/put_regs_csr()).

Otherwise,

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

> +
> +    return 0;
> +}
> +
> +static int kvm_riscv_put_regs_csr(CPUState *cs)
> +{
> +    kvm_riscv_put_regs_general_csr(cs);
>  
>      return 0;
>  }
> -- 
> 2.17.1
> 

[PATCH v2 2/8] target/riscv/kvm: add KVM_REG_RISCV_CSR_AIA

[Yong-Xuan Wang]

Add KVM_REG_RISCV_CSR_AIA support to get/set the context of AIA
extension in VS mode.

Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
---
 target/riscv/kvm/kvm-cpu.c | 45 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index ff1211d2fe39..c7318f64cf12 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -132,6 +132,9 @@ static uint64_t kvm_riscv_vector_reg_id(RISCVCPU *cpu,
 #define RISCV_GENERAL_CSR_REG(name) \
     (KVM_REG_RISCV_CSR_GENERAL | KVM_REG_RISCV_CSR_REG(name))
 
+#define RISCV_AIA_CSR_REG(name) \
+    (KVM_REG_RISCV_CSR_AIA | KVM_REG_RISCV_CSR_AIA_REG(name))
+
 #define KVM_RISCV_GET_CSR(cs, env, idx, reg) \
     do { \
         int _ret = kvm_get_one_reg(cs, RISCV_CSR_REG(env, idx), &reg); \
@@ -644,9 +647,50 @@ static int kvm_riscv_put_regs_general_csr(CPUState *cs)
     return 0;
 }
 
+static int kvm_riscv_get_regs_aia_csr(CPUState *cs)
+{
+    CPURISCVState *env = &RISCV_CPU(cs)->env;
+    uint64_t mask = MAKE_64BIT_MASK(32, 32);
+    uint64_t val;
+
+    KVM_RISCV_GET_CSR(cs, env, RISCV_AIA_CSR_REG(siselect), env->siselect);
+    KVM_RISCV_GET_CSR(cs, env, RISCV_AIA_CSR_REG(iprio1), env->siprio[0]);
+    KVM_RISCV_GET_CSR(cs, env, RISCV_AIA_CSR_REG(iprio1h), env->siprio[8]);
+    KVM_RISCV_GET_CSR(cs, env, RISCV_AIA_CSR_REG(iprio2), env->siprio[16]);
+    KVM_RISCV_GET_CSR(cs, env, RISCV_AIA_CSR_REG(iprio2h), env->siprio[24]);
+
+    KVM_RISCV_GET_CSR(cs, env, RISCV_AIA_CSR_REG(sieh), val);
+    env->sie = set_field(env->sie, mask, val);
+    KVM_RISCV_GET_CSR(cs, env, RISCV_AIA_CSR_REG(siph), val);
+    riscv_cpu_update_mip(env, mask, val);
+
+    return 0;
+}
+
+static int kvm_riscv_put_regs_aia_csr(CPUState *cs)
+{
+    CPURISCVState *env = &RISCV_CPU(cs)->env;
+    uint64_t mask = MAKE_64BIT_MASK(32, 32);
+    uint64_t val;
+
+    KVM_RISCV_SET_CSR(cs, env, RISCV_AIA_CSR_REG(siselect), env->siselect);
+    KVM_RISCV_SET_CSR(cs, env, RISCV_AIA_CSR_REG(iprio1), env->siprio[0]);
+    KVM_RISCV_SET_CSR(cs, env, RISCV_AIA_CSR_REG(iprio1h), env->siprio[8]);
+    KVM_RISCV_SET_CSR(cs, env, RISCV_AIA_CSR_REG(iprio2), env->siprio[16]);
+    KVM_RISCV_SET_CSR(cs, env, RISCV_AIA_CSR_REG(iprio2h), env->siprio[24]);
+
+    val = get_field(env->sie, mask);
+    KVM_RISCV_SET_CSR(cs, env, RISCV_AIA_CSR_REG(sieh), val);
+    val = get_field(env->mip, mask);
+    KVM_RISCV_SET_CSR(cs, env, RISCV_AIA_CSR_REG(siph), val);
+
+    return 0;
+}
+
 static int kvm_riscv_get_regs_csr(CPUState *cs)
 {
     kvm_riscv_get_regs_general_csr(cs);
+    kvm_riscv_get_regs_aia_csr(cs);
 
     return 0;
 }
@@ -654,6 +698,7 @@ static int kvm_riscv_get_regs_csr(CPUState *cs)
 static int kvm_riscv_put_regs_csr(CPUState *cs)
 {
     kvm_riscv_put_regs_general_csr(cs);
+    kvm_riscv_put_regs_aia_csr(cs);
 
     return 0;
 }
-- 
2.17.1

Re: [PATCH v2 2/8] target/riscv/kvm: add KVM_REG_RISCV_CSR_AIA

[Andrew Jones]

On Mon, Feb 24, 2025 at 04:24:09PM +0800, Yong-Xuan Wang wrote:
> Add KVM_REG_RISCV_CSR_AIA support to get/set the context of AIA
> extension in VS mode.
> 
> Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
> ---
>  target/riscv/kvm/kvm-cpu.c | 45 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
> 
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index ff1211d2fe39..c7318f64cf12 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -132,6 +132,9 @@ static uint64_t kvm_riscv_vector_reg_id(RISCVCPU *cpu,
>  #define RISCV_GENERAL_CSR_REG(name) \
>      (KVM_REG_RISCV_CSR_GENERAL | KVM_REG_RISCV_CSR_REG(name))
>  
> +#define RISCV_AIA_CSR_REG(name) \
> +    (KVM_REG_RISCV_CSR_AIA | KVM_REG_RISCV_CSR_AIA_REG(name))
> +
>  #define KVM_RISCV_GET_CSR(cs, env, idx, reg) \
>      do { \
>          int _ret = kvm_get_one_reg(cs, RISCV_CSR_REG(env, idx), &reg); \
> @@ -644,9 +647,50 @@ static int kvm_riscv_put_regs_general_csr(CPUState *cs)
>      return 0;
>  }
>  
> +static int kvm_riscv_get_regs_aia_csr(CPUState *cs)
> +{
> +    CPURISCVState *env = &RISCV_CPU(cs)->env;
> +    uint64_t mask = MAKE_64BIT_MASK(32, 32);
> +    uint64_t val;
> +
> +    KVM_RISCV_GET_CSR(cs, env, RISCV_AIA_CSR_REG(siselect), env->siselect);
> +    KVM_RISCV_GET_CSR(cs, env, RISCV_AIA_CSR_REG(iprio1), env->siprio[0]);
> +    KVM_RISCV_GET_CSR(cs, env, RISCV_AIA_CSR_REG(iprio1h), env->siprio[8]);
> +    KVM_RISCV_GET_CSR(cs, env, RISCV_AIA_CSR_REG(iprio2), env->siprio[16]);
> +    KVM_RISCV_GET_CSR(cs, env, RISCV_AIA_CSR_REG(iprio2h), env->siprio[24]);
> +
> +    KVM_RISCV_GET_CSR(cs, env, RISCV_AIA_CSR_REG(sieh), val);
> +    env->sie = set_field(env->sie, mask, val);
> +    KVM_RISCV_GET_CSR(cs, env, RISCV_AIA_CSR_REG(siph), val);
> +    riscv_cpu_update_mip(env, mask, val);

The *h registers should only s/r on 32-bit targets.

> +
> +    return 0;
> +}
> +
> +static int kvm_riscv_put_regs_aia_csr(CPUState *cs)
> +{
> +    CPURISCVState *env = &RISCV_CPU(cs)->env;
> +    uint64_t mask = MAKE_64BIT_MASK(32, 32);
> +    uint64_t val;
> +
> +    KVM_RISCV_SET_CSR(cs, env, RISCV_AIA_CSR_REG(siselect), env->siselect);
> +    KVM_RISCV_SET_CSR(cs, env, RISCV_AIA_CSR_REG(iprio1), env->siprio[0]);
> +    KVM_RISCV_SET_CSR(cs, env, RISCV_AIA_CSR_REG(iprio1h), env->siprio[8]);
> +    KVM_RISCV_SET_CSR(cs, env, RISCV_AIA_CSR_REG(iprio2), env->siprio[16]);
> +    KVM_RISCV_SET_CSR(cs, env, RISCV_AIA_CSR_REG(iprio2h), env->siprio[24]);
> +
> +    val = get_field(env->sie, mask);
> +    KVM_RISCV_SET_CSR(cs, env, RISCV_AIA_CSR_REG(sieh), val);
> +    val = get_field(env->mip, mask);
> +    KVM_RISCV_SET_CSR(cs, env, RISCV_AIA_CSR_REG(siph), val);
> +
> +    return 0;
> +}
> +
>  static int kvm_riscv_get_regs_csr(CPUState *cs)
>  {
>      kvm_riscv_get_regs_general_csr(cs);
> +    kvm_riscv_get_regs_aia_csr(cs);
>  
>      return 0;
>  }
> @@ -654,6 +698,7 @@ static int kvm_riscv_get_regs_csr(CPUState *cs)
>  static int kvm_riscv_put_regs_csr(CPUState *cs)
>  {
>      kvm_riscv_put_regs_general_csr(cs);
> +    kvm_riscv_put_regs_aia_csr(cs);
>  
>      return 0;
>  }
> -- 
> 2.17.1
>

Thanks,
drew

[PATCH v2 3/8] target/riscv/kvm: add KVM_REG_RISCV_CSR_SMSTATEEN

[Yong-Xuan Wang]

Add KVM_REG_RISCV_CSR_SMSTATEEN support to get/set the context of
Smstateen extension in VS mode.

Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
---
 target/riscv/kvm/kvm-cpu.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index c7318f64cf12..d421c7a1b65d 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -135,6 +135,9 @@ static uint64_t kvm_riscv_vector_reg_id(RISCVCPU *cpu,
 #define RISCV_AIA_CSR_REG(name) \
     (KVM_REG_RISCV_CSR_AIA | KVM_REG_RISCV_CSR_AIA_REG(name))
 
+#define RISCV_SMSTATEEN_CSR_REG(name) \
+    (KVM_REG_RISCV_CSR_SMSTATEEN | KVM_REG_RISCV_CSR_SMSTATEEN_REG(name))
+
 #define KVM_RISCV_GET_CSR(cs, env, idx, reg) \
     do { \
         int _ret = kvm_get_one_reg(cs, RISCV_CSR_REG(env, idx), &reg); \
@@ -687,10 +690,31 @@ static int kvm_riscv_put_regs_aia_csr(CPUState *cs)
     return 0;
 }
 
+static int kvm_riscv_get_regs_smstateen_csr(CPUState *cs)
+{
+    CPURISCVState *env = &RISCV_CPU(cs)->env;
+
+    KVM_RISCV_GET_CSR(cs, env,
+                      RISCV_SMSTATEEN_CSR_REG(sstateen0), env->sstateen[0]);
+
+    return 0;
+}
+
+static int kvm_riscv_put_regs_smstateen_csr(CPUState *cs)
+{
+    CPURISCVState *env = &RISCV_CPU(cs)->env;
+
+    KVM_RISCV_SET_CSR(cs, env,
+                      RISCV_SMSTATEEN_CSR_REG(sstateen0), env->sstateen[0]);
+
+    return 0;
+}
+
 static int kvm_riscv_get_regs_csr(CPUState *cs)
 {
     kvm_riscv_get_regs_general_csr(cs);
     kvm_riscv_get_regs_aia_csr(cs);
+    kvm_riscv_get_regs_smstateen_csr(cs);
 
     return 0;
 }
@@ -699,6 +723,7 @@ static int kvm_riscv_put_regs_csr(CPUState *cs)
 {
     kvm_riscv_put_regs_general_csr(cs);
     kvm_riscv_put_regs_aia_csr(cs);
+    kvm_riscv_put_regs_smstateen_csr(cs);
 
     return 0;
 }
-- 
2.17.1

Re: [PATCH v2 3/8] target/riscv/kvm: add KVM_REG_RISCV_CSR_SMSTATEEN

[Andrew Jones]

On Mon, Feb 24, 2025 at 04:24:10PM +0800, Yong-Xuan Wang wrote:
> Add KVM_REG_RISCV_CSR_SMSTATEEN support to get/set the context of
> Smstateen extension in VS mode.
> 
> Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
> ---
>  target/riscv/kvm/kvm-cpu.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index c7318f64cf12..d421c7a1b65d 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -135,6 +135,9 @@ static uint64_t kvm_riscv_vector_reg_id(RISCVCPU *cpu,
>  #define RISCV_AIA_CSR_REG(name) \
>      (KVM_REG_RISCV_CSR_AIA | KVM_REG_RISCV_CSR_AIA_REG(name))
>  
> +#define RISCV_SMSTATEEN_CSR_REG(name) \
> +    (KVM_REG_RISCV_CSR_SMSTATEEN | KVM_REG_RISCV_CSR_SMSTATEEN_REG(name))
> +
>  #define KVM_RISCV_GET_CSR(cs, env, idx, reg) \
>      do { \
>          int _ret = kvm_get_one_reg(cs, RISCV_CSR_REG(env, idx), &reg); \
> @@ -687,10 +690,31 @@ static int kvm_riscv_put_regs_aia_csr(CPUState *cs)
>      return 0;
>  }
>  
> +static int kvm_riscv_get_regs_smstateen_csr(CPUState *cs)
> +{
> +    CPURISCVState *env = &RISCV_CPU(cs)->env;
> +
> +    KVM_RISCV_GET_CSR(cs, env,
> +                      RISCV_SMSTATEEN_CSR_REG(sstateen0), env->sstateen[0]);
> +
> +    return 0;
> +}
> +
> +static int kvm_riscv_put_regs_smstateen_csr(CPUState *cs)
> +{
> +    CPURISCVState *env = &RISCV_CPU(cs)->env;
> +
> +    KVM_RISCV_SET_CSR(cs, env,
> +                      RISCV_SMSTATEEN_CSR_REG(sstateen0), env->sstateen[0]);
> +
> +    return 0;
> +}
> +
>  static int kvm_riscv_get_regs_csr(CPUState *cs)
>  {
>      kvm_riscv_get_regs_general_csr(cs);
>      kvm_riscv_get_regs_aia_csr(cs);
> +    kvm_riscv_get_regs_smstateen_csr(cs);
>  
>      return 0;
>  }
> @@ -699,6 +723,7 @@ static int kvm_riscv_put_regs_csr(CPUState *cs)
>  {
>      kvm_riscv_put_regs_general_csr(cs);
>      kvm_riscv_put_regs_aia_csr(cs);
> +    kvm_riscv_put_regs_smstateen_csr(cs);
>  
>      return 0;
>  }
> -- 
> 2.17.1
>

Looks good other than the missing error checking/propagating in
kvm_riscv_get/put_regs_csr() which the general and aia registers are also
missing.

Thanks,
drew

[PATCH v2 4/8] target/riscv: add helper to get CSR name

[Yong-Xuan Wang]

Add a helper function to get CSR name from CSR number.

Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
---
 target/riscv/cpu.h | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 616c3bdc1c24..df10ff63474b 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -920,8 +920,14 @@ extern const bool valid_vm_1_10_32[], valid_vm_1_10_64[];
 void riscv_get_csr_ops(int csrno, riscv_csr_operations *ops);
 void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops);
 
-void riscv_cpu_register_gdb_regs_for_features(CPUState *cs);
+static inline const char *riscv_get_csr_name(int csr_no)
+{
+    g_assert(csr_no > 0 && csr_no < CSR_TABLE_SIZE);
 
+    return csr_ops[csr_no].name;
+}
+
+void riscv_cpu_register_gdb_regs_for_features(CPUState *cs);
 target_ulong riscv_new_csr_seed(target_ulong new_value,
                                 target_ulong write_mask);
 
-- 
2.17.1

Re: [PATCH v2 4/8] target/riscv: add helper to get CSR name

[Andrew Jones]

On Mon, Feb 24, 2025 at 04:24:11PM +0800, Yong-Xuan Wang wrote:
> Add a helper function to get CSR name from CSR number.
> 
> Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
> ---
>  target/riscv/cpu.h | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 616c3bdc1c24..df10ff63474b 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -920,8 +920,14 @@ extern const bool valid_vm_1_10_32[], valid_vm_1_10_64[];
>  void riscv_get_csr_ops(int csrno, riscv_csr_operations *ops);
>  void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops);
>  
> -void riscv_cpu_register_gdb_regs_for_features(CPUState *cs);
> +static inline const char *riscv_get_csr_name(int csr_no)
> +{
> +    g_assert(csr_no > 0 && csr_no < CSR_TABLE_SIZE);
>  
> +    return csr_ops[csr_no].name;
> +}
> +
> +void riscv_cpu_register_gdb_regs_for_features(CPUState *cs);
>  target_ulong riscv_new_csr_seed(target_ulong new_value,
>                                  target_ulong write_mask);
>  
> -- 
> 2.17.1
> 
>

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

[PATCH v2 5/8] target/riscv/kvm: rewrite kvm_riscv_handle_csr

[Yong-Xuan Wang]

Rewrite the kvm_riscv_handle_csr() to support additional CSR emulation
in user space with KVM acceleration. This update reuses the TCG CSR
emulation function to simplify the implementation and reduce the
redundant work. Also it introduces two hook functions for certain CSRs.
Before emulation, the related VS mode context of the CSR can be loaded
from host in context_load() hook. After the CSR handling, the modified
VS context is written back in context_put() hook.

Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
---
 target/riscv/cpu.h         |  2 --
 target/riscv/csr.c         | 18 +++-------
 target/riscv/kvm/kvm-cpu.c | 68 ++++++++++++++++++++++++++++++++------
 3 files changed, 61 insertions(+), 27 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index df10ff63474b..81b8081d81e0 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -928,8 +928,6 @@ static inline const char *riscv_get_csr_name(int csr_no)
 }
 
 void riscv_cpu_register_gdb_regs_for_features(CPUState *cs);
-target_ulong riscv_new_csr_seed(target_ulong new_value,
-                                target_ulong write_mask);
 
 uint8_t satp_mode_max_from_map(uint32_t map);
 const char *satp_mode_str(uint8_t satp_mode, bool is_32_bit);
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index d0068ce98c15..a2830888d010 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -5389,8 +5389,10 @@ static int write_mnstatus(CPURISCVState *env, int csrno, target_ulong val)
 #endif
 
 /* Crypto Extension */
-target_ulong riscv_new_csr_seed(target_ulong new_value,
-                                target_ulong write_mask)
+static RISCVException rmw_seed(CPURISCVState *env, int csrno,
+                               target_ulong *ret_value,
+                               target_ulong new_value,
+                               target_ulong write_mask)
 {
     uint16_t random_v;
     Error *random_e = NULL;
@@ -5414,18 +5416,6 @@ target_ulong riscv_new_csr_seed(target_ulong new_value,
         rval = random_v | SEED_OPST_ES16;
     }
 
-    return rval;
-}
-
-static RISCVException rmw_seed(CPURISCVState *env, int csrno,
-                               target_ulong *ret_value,
-                               target_ulong new_value,
-                               target_ulong write_mask)
-{
-    target_ulong rval;
-
-    rval = riscv_new_csr_seed(new_value, write_mask);
-
     if (ret_value) {
         *ret_value = rval;
     }
diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index d421c7a1b65d..b088b947adae 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -1626,26 +1626,72 @@ static int kvm_riscv_handle_sbi(CPUState *cs, struct kvm_run *run)
     return ret;
 }
 
+/* User-space CSR emulation */
+struct kvm_riscv_emu_csr_data {
+    target_ulong csr_num;
+    int (*context_load)(CPUState *cs);
+    int (*context_put)(CPUState *cs);
+};
+
+struct kvm_riscv_emu_csr_data kvm_riscv_emu_csr_data[] = {
+    { CSR_SEED, NULL, NULL },
+};
+
 static int kvm_riscv_handle_csr(CPUState *cs, struct kvm_run *run)
 {
+    CPURISCVState *env = cpu_env(cs);
     target_ulong csr_num = run->riscv_csr.csr_num;
     target_ulong new_value = run->riscv_csr.new_value;
     target_ulong write_mask = run->riscv_csr.write_mask;
-    int ret = 0;
+    struct kvm_riscv_emu_csr_data *emu_csr_data = NULL;
+    target_ulong ret_value;
+    RISCVException ret_excp;
+    int i, ret;
 
-    switch (csr_num) {
-    case CSR_SEED:
-        run->riscv_csr.ret_value = riscv_new_csr_seed(new_value, write_mask);
-        break;
-    default:
+    for (i = 0; i < ARRAY_SIZE(kvm_riscv_emu_csr_data); i++) {
+        if (csr_num == kvm_riscv_emu_csr_data[i].csr_num) {
+            emu_csr_data = &kvm_riscv_emu_csr_data[i];
+
+            break;
+        }
+    }
+
+    if (!emu_csr_data) {
         qemu_log_mask(LOG_UNIMP,
-                      "%s: un-handled CSR EXIT for CSR %lx\n",
-                      __func__, csr_num);
-        ret = -1;
-        break;
+                      "%s: un-handled CSR EXIT for CSR %s\n",
+                      __func__, riscv_get_csr_name(csr_num));
+
+        return -1;
     }
 
-    return ret;
+    if (emu_csr_data->context_load) {
+        ret = emu_csr_data->context_load(cs);
+        if (ret) {
+            goto handle_failed;
+        }
+    }
+
+    ret_excp = riscv_csrrw(env, csr_num, &ret_value, new_value, write_mask);
+    if (ret_excp != RISCV_EXCP_NONE) {
+        goto handle_failed;
+    }
+    run->riscv_csr.ret_value = ret_value;
+
+    if (emu_csr_data->context_put) {
+        ret = emu_csr_data->context_put(cs);
+        if (ret) {
+            goto handle_failed;
+        }
+    }
+
+    return 0;
+
+handle_failed:
+    qemu_log_mask(LOG_UNIMP,
+                  "%s: failed to handle CSR EXIT for CSR %s\n",
+                  __func__, riscv_get_csr_name(csr_num));
+
+    return -1;
 }
 
 static bool kvm_riscv_handle_debug(CPUState *cs)
-- 
2.17.1

Re: [PATCH v2 5/8] target/riscv/kvm: rewrite kvm_riscv_handle_csr

[Andrew Jones]

On Mon, Feb 24, 2025 at 04:24:12PM +0800, Yong-Xuan Wang wrote:
> Rewrite the kvm_riscv_handle_csr() to support additional CSR emulation
> in user space with KVM acceleration. This update reuses the TCG CSR
> emulation function to simplify the implementation and reduce the
> redundant work. Also it introduces two hook functions for certain CSRs.
> Before emulation, the related VS mode context of the CSR can be loaded
> from host in context_load() hook. After the CSR handling, the modified
> VS context is written back in context_put() hook.
> 
> Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
> ---
>  target/riscv/cpu.h         |  2 --
>  target/riscv/csr.c         | 18 +++-------
>  target/riscv/kvm/kvm-cpu.c | 68 ++++++++++++++++++++++++++++++++------
>  3 files changed, 61 insertions(+), 27 deletions(-)
> 
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index df10ff63474b..81b8081d81e0 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -928,8 +928,6 @@ static inline const char *riscv_get_csr_name(int csr_no)
>  }
>  
>  void riscv_cpu_register_gdb_regs_for_features(CPUState *cs);
> -target_ulong riscv_new_csr_seed(target_ulong new_value,
> -                                target_ulong write_mask);
>  
>  uint8_t satp_mode_max_from_map(uint32_t map);
>  const char *satp_mode_str(uint8_t satp_mode, bool is_32_bit);
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index d0068ce98c15..a2830888d010 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -5389,8 +5389,10 @@ static int write_mnstatus(CPURISCVState *env, int csrno, target_ulong val)
>  #endif
>  
>  /* Crypto Extension */
> -target_ulong riscv_new_csr_seed(target_ulong new_value,
> -                                target_ulong write_mask)
> +static RISCVException rmw_seed(CPURISCVState *env, int csrno,
> +                               target_ulong *ret_value,
> +                               target_ulong new_value,
> +                               target_ulong write_mask)
>  {
>      uint16_t random_v;
>      Error *random_e = NULL;
> @@ -5414,18 +5416,6 @@ target_ulong riscv_new_csr_seed(target_ulong new_value,
>          rval = random_v | SEED_OPST_ES16;
>      }
>  
> -    return rval;
> -}
> -
> -static RISCVException rmw_seed(CPURISCVState *env, int csrno,
> -                               target_ulong *ret_value,
> -                               target_ulong new_value,
> -                               target_ulong write_mask)
> -{
> -    target_ulong rval;
> -
> -    rval = riscv_new_csr_seed(new_value, write_mask);
> -
>      if (ret_value) {
>          *ret_value = rval;
>      }
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index d421c7a1b65d..b088b947adae 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -1626,26 +1626,72 @@ static int kvm_riscv_handle_sbi(CPUState *cs, struct kvm_run *run)
>      return ret;
>  }
>  
> +/* User-space CSR emulation */
> +struct kvm_riscv_emu_csr_data {
> +    target_ulong csr_num;
> +    int (*context_load)(CPUState *cs);
> +    int (*context_put)(CPUState *cs);
> +};
> +
> +struct kvm_riscv_emu_csr_data kvm_riscv_emu_csr_data[] = {
> +    { CSR_SEED, NULL, NULL },

Needing to ensure TCG state is correct, and therefore introducing the
load/put callbacks, makes me wonder if this is a good idea at all. Sharing
code is good, but, other than the TCG state concerns, we also will have to
deal with potentially wanting to compile with CONFIG_TCG=n when we only
want QEMU to be a KVM VMM.

> +};
> +
>  static int kvm_riscv_handle_csr(CPUState *cs, struct kvm_run *run)
>  {
> +    CPURISCVState *env = cpu_env(cs);
>      target_ulong csr_num = run->riscv_csr.csr_num;
>      target_ulong new_value = run->riscv_csr.new_value;
>      target_ulong write_mask = run->riscv_csr.write_mask;
> -    int ret = 0;
> +    struct kvm_riscv_emu_csr_data *emu_csr_data = NULL;
> +    target_ulong ret_value;
> +    RISCVException ret_excp;
> +    int i, ret;
>  
> -    switch (csr_num) {
> -    case CSR_SEED:
> -        run->riscv_csr.ret_value = riscv_new_csr_seed(new_value, write_mask);
> -        break;
> -    default:
> +    for (i = 0; i < ARRAY_SIZE(kvm_riscv_emu_csr_data); i++) {
> +        if (csr_num == kvm_riscv_emu_csr_data[i].csr_num) {
> +            emu_csr_data = &kvm_riscv_emu_csr_data[i];
> +

remove this extra blank line

> +            break;
> +        }
> +    }
> +
> +    if (!emu_csr_data) {
>          qemu_log_mask(LOG_UNIMP,
> -                      "%s: un-handled CSR EXIT for CSR %lx\n",
> -                      __func__, csr_num);
> -        ret = -1;
> -        break;
> +                      "%s: un-handled CSR EXIT for CSR %s\n",
> +                      __func__, riscv_get_csr_name(csr_num));
> +
> +        return -1;
>      }
>  
> -    return ret;
> +    if (emu_csr_data->context_load) {
> +        ret = emu_csr_data->context_load(cs);
> +        if (ret) {
> +            goto handle_failed;
> +        }
> +    }
> +
> +    ret_excp = riscv_csrrw(env, csr_num, &ret_value, new_value, write_mask);
> +    if (ret_excp != RISCV_EXCP_NONE) {
> +        goto handle_failed;
> +    }
> +    run->riscv_csr.ret_value = ret_value;
> +
> +    if (emu_csr_data->context_put) {
> +        ret = emu_csr_data->context_put(cs);
> +        if (ret) {
> +            goto handle_failed;
> +        }
> +    }
> +
> +    return 0;
> +
> +handle_failed:
> +    qemu_log_mask(LOG_UNIMP,
> +                  "%s: failed to handle CSR EXIT for CSR %s\n",
> +                  __func__, riscv_get_csr_name(csr_num));
> +
> +    return -1;
>  }
>  
>  static bool kvm_riscv_handle_debug(CPUState *cs)
> -- 
> 2.17.1
>

So, as I said above, I'm skeptical of this approach, since I'm not sure we
maintain TCG state well enough when using KVM to trust riscv_csrrw() nor
am I sure we can always have such simple load/put functions like the next
patch proposes for AIA registers.

I vote we manage CSRs case-by-case. SEED is already managed and shares
code at a point that is easy to audit, proving there are no TCG state
dependencies in the emulation. AIA should do the same, at least until
we've decided to build a KVM->TCG model that ensures TCG state is always
correct, in which case I don't think the load/put functions would be
necessary here.

(On a related note, Radim Krcmar suggested we consider forwarding more
SBI calls from KVM to QEMU where QEMU could then use OpenSBI's
implementation. That makes sense to me, but we'd need to create that
KVM->TCG model first for it too.)

Thanks,
drew

[PATCH v2 6/8] target/riscv/kvm: add CSR_SIREG and CSR_STOPEI emulation

[Yong-Xuan Wang]

Support user-space emulation of SIREG and STOPEI CSR with KVM
acceleration. For SIREG emulation, the SISELECT CSR value and iprio
array must be loaded before handling, and since the iprio array might
be modified, it must be written back after the emulation.

When running with KVM acceleration, the machine lacks M-mode CSRs and
does not report S-mode support in its environment configuration, even
though some S-mode CSRs are accessible. This patch adds kvm_enabled()
checks in relevant predicates to ensure proper handling and validation.

Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
---
 target/riscv/csr.c         | 12 +++++++++---
 target/riscv/kvm/kvm-cpu.c | 27 +++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index a2830888d010..594df30c456a 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -27,6 +27,7 @@
 #include "exec/exec-all.h"
 #include "exec/tb-flush.h"
 #include "system/cpu-timers.h"
+#include "system/kvm.h"
 #include "qemu/guest-random.h"
 #include "qapi/error.h"
 #include <stdbool.h>
@@ -42,6 +43,11 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops)
     csr_ops[csrno & (CSR_TABLE_SIZE - 1)] = *ops;
 }
 
+static bool riscv_has_ext_s(CPURISCVState *env)
+{
+    return riscv_has_ext(env, RVS) || kvm_enabled();
+}
+
 /* Predicates */
 #if !defined(CONFIG_USER_ONLY)
 RISCVException smstateen_acc_ok(CPURISCVState *env, int index, uint64_t bit)
@@ -52,7 +58,7 @@ RISCVException smstateen_acc_ok(CPURISCVState *env, int index, uint64_t bit)
         return RISCV_EXCP_NONE;
     }
 
-    if (!(env->mstateen[index] & bit)) {
+    if (!kvm_enabled() && !(env->mstateen[index] & bit)) {
         return RISCV_EXCP_ILLEGAL_INST;
     }
 
@@ -66,7 +72,7 @@ RISCVException smstateen_acc_ok(CPURISCVState *env, int index, uint64_t bit)
         }
     }
 
-    if (env->priv == PRV_U && riscv_has_ext(env, RVS)) {
+    if (env->priv == PRV_U && riscv_has_ext_s(env)) {
         if (!(env->sstateen[index] & bit)) {
             return RISCV_EXCP_ILLEGAL_INST;
         }
@@ -326,7 +332,7 @@ static RISCVException csrind_or_aia_any(CPURISCVState *env, int csrno)
 
 static RISCVException smode(CPURISCVState *env, int csrno)
 {
-    if (riscv_has_ext(env, RVS)) {
+    if (riscv_has_ext_s(env)) {
         return RISCV_EXCP_NONE;
     }
 
diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index b088b947adae..50b0e7c9ff7d 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -1627,6 +1627,31 @@ static int kvm_riscv_handle_sbi(CPUState *cs, struct kvm_run *run)
 }
 
 /* User-space CSR emulation */
+static int kvm_riscv_emu_sireg_ctx_load(CPUState *cs)
+{
+    CPURISCVState *env = &RISCV_CPU(cs)->env;
+
+    KVM_RISCV_GET_CSR(cs, env, RISCV_AIA_CSR_REG(siselect), env->siselect);
+    KVM_RISCV_GET_CSR(cs, env, RISCV_AIA_CSR_REG(iprio1), env->siprio[0]);
+    KVM_RISCV_GET_CSR(cs, env, RISCV_AIA_CSR_REG(iprio1h), env->siprio[8]);
+    KVM_RISCV_GET_CSR(cs, env, RISCV_AIA_CSR_REG(iprio2), env->siprio[16]);
+    KVM_RISCV_GET_CSR(cs, env, RISCV_AIA_CSR_REG(iprio2h), env->siprio[24]);
+
+    return 0;
+}
+
+static int kvm_riscv_emu_sireg_ctx_put(CPUState *cs)
+{
+    CPURISCVState *env = &RISCV_CPU(cs)->env;
+
+    KVM_RISCV_SET_CSR(cs, env, RISCV_AIA_CSR_REG(iprio1), env->siprio[0]);
+    KVM_RISCV_SET_CSR(cs, env, RISCV_AIA_CSR_REG(iprio1h), env->siprio[8]);
+    KVM_RISCV_SET_CSR(cs, env, RISCV_AIA_CSR_REG(iprio2), env->siprio[16]);
+    KVM_RISCV_SET_CSR(cs, env, RISCV_AIA_CSR_REG(iprio2h), env->siprio[24]);
+
+    return 0;
+}
+
 struct kvm_riscv_emu_csr_data {
     target_ulong csr_num;
     int (*context_load)(CPUState *cs);
@@ -1635,6 +1660,8 @@ struct kvm_riscv_emu_csr_data {
 
 struct kvm_riscv_emu_csr_data kvm_riscv_emu_csr_data[] = {
     { CSR_SEED, NULL, NULL },
+    { CSR_SIREG, kvm_riscv_emu_sireg_ctx_load, kvm_riscv_emu_sireg_ctx_put },
+    { CSR_STOPEI, NULL, NULL },
 };
 
 static int kvm_riscv_handle_csr(CPUState *cs, struct kvm_run *run)
-- 
2.17.1

Re: [PATCH v2 6/8] target/riscv/kvm: add CSR_SIREG and CSR_STOPEI emulation

[Andrew Jones]

On Mon, Feb 24, 2025 at 04:24:13PM +0800, Yong-Xuan Wang wrote:
> Support user-space emulation of SIREG and STOPEI CSR with KVM
> acceleration. For SIREG emulation, the SISELECT CSR value and iprio
> array must be loaded before handling, and since the iprio array might
> be modified, it must be written back after the emulation.
> 
> When running with KVM acceleration, the machine lacks M-mode CSRs and
> does not report S-mode support in its environment configuration, even
> though some S-mode CSRs are accessible. This patch adds kvm_enabled()
> checks in relevant predicates to ensure proper handling and validation.
> 
> Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
> ---
>  target/riscv/csr.c         | 12 +++++++++---
>  target/riscv/kvm/kvm-cpu.c | 27 +++++++++++++++++++++++++++
>  2 files changed, 36 insertions(+), 3 deletions(-)
> 
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index a2830888d010..594df30c456a 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -27,6 +27,7 @@
>  #include "exec/exec-all.h"
>  #include "exec/tb-flush.h"
>  #include "system/cpu-timers.h"
> +#include "system/kvm.h"
>  #include "qemu/guest-random.h"
>  #include "qapi/error.h"
>  #include <stdbool.h>
> @@ -42,6 +43,11 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops)
>      csr_ops[csrno & (CSR_TABLE_SIZE - 1)] = *ops;
>  }
>  
> +static bool riscv_has_ext_s(CPURISCVState *env)
> +{
> +    return riscv_has_ext(env, RVS) || kvm_enabled();
> +}
> +
>  /* Predicates */
>  #if !defined(CONFIG_USER_ONLY)
>  RISCVException smstateen_acc_ok(CPURISCVState *env, int index, uint64_t bit)
> @@ -52,7 +58,7 @@ RISCVException smstateen_acc_ok(CPURISCVState *env, int index, uint64_t bit)
>          return RISCV_EXCP_NONE;
>      }
>  
> -    if (!(env->mstateen[index] & bit)) {
> +    if (!kvm_enabled() && !(env->mstateen[index] & bit)) {
>          return RISCV_EXCP_ILLEGAL_INST;
>      }
>  
> @@ -66,7 +72,7 @@ RISCVException smstateen_acc_ok(CPURISCVState *env, int index, uint64_t bit)
>          }
>      }
>  
> -    if (env->priv == PRV_U && riscv_has_ext(env, RVS)) {
> +    if (env->priv == PRV_U && riscv_has_ext_s(env)) {
>          if (!(env->sstateen[index] & bit)) {
>              return RISCV_EXCP_ILLEGAL_INST;
>          }
> @@ -326,7 +332,7 @@ static RISCVException csrind_or_aia_any(CPURISCVState *env, int csrno)
>  
>  static RISCVException smode(CPURISCVState *env, int csrno)
>  {
> -    if (riscv_has_ext(env, RVS)) {
> +    if (riscv_has_ext_s(env)) {
>          return RISCV_EXCP_NONE;
>      }

Scattering kvm_enabled() checks around, as is done in the above hunks, is
what I meant in my reply to the previous patch about TCG likely not having
its state maintained well enough to always provide functional CSR
emulation for KVM. Rather than fixing things up as we bump into them with
kvm_enabled() checks we should come up with a way to truly support QEMU
KVM emulation forwarding to TCG.

Thanks,
drew

>  
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index b088b947adae..50b0e7c9ff7d 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -1627,6 +1627,31 @@ static int kvm_riscv_handle_sbi(CPUState *cs, struct kvm_run *run)
>  }
>  
>  /* User-space CSR emulation */
> +static int kvm_riscv_emu_sireg_ctx_load(CPUState *cs)
> +{
> +    CPURISCVState *env = &RISCV_CPU(cs)->env;
> +
> +    KVM_RISCV_GET_CSR(cs, env, RISCV_AIA_CSR_REG(siselect), env->siselect);
> +    KVM_RISCV_GET_CSR(cs, env, RISCV_AIA_CSR_REG(iprio1), env->siprio[0]);
> +    KVM_RISCV_GET_CSR(cs, env, RISCV_AIA_CSR_REG(iprio1h), env->siprio[8]);
> +    KVM_RISCV_GET_CSR(cs, env, RISCV_AIA_CSR_REG(iprio2), env->siprio[16]);
> +    KVM_RISCV_GET_CSR(cs, env, RISCV_AIA_CSR_REG(iprio2h), env->siprio[24]);
> +
> +    return 0;
> +}
> +
> +static int kvm_riscv_emu_sireg_ctx_put(CPUState *cs)
> +{
> +    CPURISCVState *env = &RISCV_CPU(cs)->env;
> +
> +    KVM_RISCV_SET_CSR(cs, env, RISCV_AIA_CSR_REG(iprio1), env->siprio[0]);
> +    KVM_RISCV_SET_CSR(cs, env, RISCV_AIA_CSR_REG(iprio1h), env->siprio[8]);
> +    KVM_RISCV_SET_CSR(cs, env, RISCV_AIA_CSR_REG(iprio2), env->siprio[16]);
> +    KVM_RISCV_SET_CSR(cs, env, RISCV_AIA_CSR_REG(iprio2h), env->siprio[24]);
> +
> +    return 0;
> +}
> +
>  struct kvm_riscv_emu_csr_data {
>      target_ulong csr_num;
>      int (*context_load)(CPUState *cs);
> @@ -1635,6 +1660,8 @@ struct kvm_riscv_emu_csr_data {
>  
>  struct kvm_riscv_emu_csr_data kvm_riscv_emu_csr_data[] = {
>      { CSR_SEED, NULL, NULL },
> +    { CSR_SIREG, kvm_riscv_emu_sireg_ctx_load, kvm_riscv_emu_sireg_ctx_put },
> +    { CSR_STOPEI, NULL, NULL },
>  };
>  
>  static int kvm_riscv_handle_csr(CPUState *cs, struct kvm_run *run)
> -- 
> 2.17.1
> 

[PATCH v2 7/8] docs: update the description about RISC-V AIA

[Yong-Xuan Wang]

Add the description about "-accel kvm,kernel-irqchip=off" into
docs/specs/riscv-aia.rst and docs/system/riscv/virt.rst.

Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
---
 docs/specs/riscv-aia.rst   | 24 ++++++++++++++++++------
 docs/system/riscv/virt.rst | 10 ++++++----
 2 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/docs/specs/riscv-aia.rst b/docs/specs/riscv-aia.rst
index 8097e2f89744..38797cca4998 100644
--- a/docs/specs/riscv-aia.rst
+++ b/docs/specs/riscv-aia.rst
@@ -25,11 +25,16 @@ When running KVM:
 - no m-mode is provided, so there is no m-mode APLIC or IMSIC emulation regardless of
   the AIA mode chosen
 - with "aia=aplic", s-mode APLIC will be emulated by userspace
-- with "aia=aplic-imsic" there are two possibilities.  If no additional KVM option
-  is provided there will be no APLIC or IMSIC emulation in userspace, and the virtual
-  machine will use the provided in-kernel APLIC and IMSIC controllers.  If the user
-  chooses to use the irqchip in split mode via "-accel kvm,kernel-irqchip=split",
-  s-mode APLIC will be emulated while using the s-mode IMSIC from the irqchip
+- with "aia=aplic-imsic" there are three possibilities.
+    - If no additional KVM option is provided there will be no APLIC or IMSIC emulation
+      in userspace, and the virtual machine will use the provided in-kernel APLIC and
+      IMSIC controllers.
+    - If the user chooses to use the irqchip in split mode via
+      "-accel kvm,kernel-irqchip=split", s-mode APLIC will be emulated while using
+      the s-mode IMSIC from the irqchip.
+    - If the user disables the in-kernel irqchip via "-accel kvm,kernel-irqchip=off",
+      both s-mode APLIC and IMSIC controller will be emulated.
+
 
 The following table summarizes how the AIA and accelerator options defines what
 we will emulate in userspace:
@@ -75,9 +80,16 @@ we will emulate in userspace:
      - in-kernel
      - in-kernel
    * - kvm
-     - irqchip=split
+     - kernel-irqchip=split
      - aplic-imsic
      - n/a
      - n/a
      - emul
      - in-kernel
+   * - kvm
+     - kernel-irqchip=off
+     - aplic-imsic
+     - n/a
+     - n/a
+     - emul
+     - emul
diff --git a/docs/system/riscv/virt.rst b/docs/system/riscv/virt.rst
index 60850970ce83..96d7ee1ebc64 100644
--- a/docs/system/riscv/virt.rst
+++ b/docs/system/riscv/virt.rst
@@ -129,12 +129,14 @@ The following machine-specific options are supported:
   MSIs. When not specified, this option is assumed to be "none" which selects
   SiFive PLIC to handle wired interrupts.
 
-  This option also interacts with '-accel kvm'.  When using "aia=aplic-imsic"
-  with KVM, it is possible to set the use of the kernel irqchip in split mode
+  This option also interacts with '-accel kvm', when using "aia=aplic-imsic"
+  with KVM.  It is possible to set the use of the kernel irqchip in split mode
   by using "-accel kvm,kernel-irqchip=split".  In this case the ``virt`` machine
   will emulate the APLIC controller instead of using the APLIC controller from
-  the irqchip.  See :ref:`riscv-aia` for more details on all available AIA
-  modes.
+  the in-kernel irqchip. Or the kernel irqchip can be disabled by using
+  "-accel kvm,kernel-irqchip=off". In this case the ``virt`` machine will
+  emulate the APLIC and IMSIC controller in user-space instead of using in-kernel
+  irqchip.  See :ref:`riscv-aia` for more details on all available AIA modes.
 
 - aia-guests=nnn
 
-- 
2.17.1

Re: [PATCH v2 7/8] docs: update the description about RISC-V AIA

[Andrew Jones]

On Mon, Feb 24, 2025 at 04:24:14PM +0800, Yong-Xuan Wang wrote:
> Add the description about "-accel kvm,kernel-irqchip=off" into
> docs/specs/riscv-aia.rst and docs/system/riscv/virt.rst.
> 
> Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
> ---
>  docs/specs/riscv-aia.rst   | 24 ++++++++++++++++++------
>  docs/system/riscv/virt.rst | 10 ++++++----
>  2 files changed, 24 insertions(+), 10 deletions(-)
> 
> diff --git a/docs/specs/riscv-aia.rst b/docs/specs/riscv-aia.rst
> index 8097e2f89744..38797cca4998 100644
> --- a/docs/specs/riscv-aia.rst
> +++ b/docs/specs/riscv-aia.rst
> @@ -25,11 +25,16 @@ When running KVM:
>  - no m-mode is provided, so there is no m-mode APLIC or IMSIC emulation regardless of
>    the AIA mode chosen
>  - with "aia=aplic", s-mode APLIC will be emulated by userspace
> -- with "aia=aplic-imsic" there are two possibilities.  If no additional KVM option
> -  is provided there will be no APLIC or IMSIC emulation in userspace, and the virtual
> -  machine will use the provided in-kernel APLIC and IMSIC controllers.  If the user
> -  chooses to use the irqchip in split mode via "-accel kvm,kernel-irqchip=split",
> -  s-mode APLIC will be emulated while using the s-mode IMSIC from the irqchip
> +- with "aia=aplic-imsic" there are three possibilities.
> +    - If no additional KVM option is provided there will be no APLIC or IMSIC emulation
> +      in userspace, and the virtual machine will use the provided in-kernel APLIC and
> +      IMSIC controllers.
> +    - If the user chooses to use the irqchip in split mode via
> +      "-accel kvm,kernel-irqchip=split", s-mode APLIC will be emulated while using
> +      the s-mode IMSIC from the irqchip.
> +    - If the user disables the in-kernel irqchip via "-accel kvm,kernel-irqchip=off",
> +      both s-mode APLIC and IMSIC controller will be emulated.
> +
>  
>  The following table summarizes how the AIA and accelerator options defines what
>  we will emulate in userspace:
> @@ -75,9 +80,16 @@ we will emulate in userspace:
>       - in-kernel
>       - in-kernel
>     * - kvm
> -     - irqchip=split
> +     - kernel-irqchip=split
>       - aplic-imsic
>       - n/a
>       - n/a
>       - emul
>       - in-kernel
> +   * - kvm
> +     - kernel-irqchip=off
> +     - aplic-imsic
> +     - n/a
> +     - n/a
> +     - emul
> +     - emul
> diff --git a/docs/system/riscv/virt.rst b/docs/system/riscv/virt.rst
> index 60850970ce83..96d7ee1ebc64 100644
> --- a/docs/system/riscv/virt.rst
> +++ b/docs/system/riscv/virt.rst
> @@ -129,12 +129,14 @@ The following machine-specific options are supported:
>    MSIs. When not specified, this option is assumed to be "none" which selects
>    SiFive PLIC to handle wired interrupts.
>  
> -  This option also interacts with '-accel kvm'.  When using "aia=aplic-imsic"
> -  with KVM, it is possible to set the use of the kernel irqchip in split mode
> +  This option also interacts with '-accel kvm', when using "aia=aplic-imsic"
> +  with KVM.  It is possible to set the use of the kernel irqchip in split mode
>    by using "-accel kvm,kernel-irqchip=split".  In this case the ``virt`` machine
>    will emulate the APLIC controller instead of using the APLIC controller from
> -  the irqchip.  See :ref:`riscv-aia` for more details on all available AIA
> -  modes.
> +  the in-kernel irqchip. Or the kernel irqchip can be disabled by using
> +  "-accel kvm,kernel-irqchip=off". In this case the ``virt`` machine will
> +  emulate the APLIC and IMSIC controller in user-space instead of using in-kernel
> +  irqchip.  See :ref:`riscv-aia` for more details on all available AIA modes.
>  
>  - aia-guests=nnn
>  
> -- 
> 2.17.1
> 
>

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

[PATCH v2 8/8] hw/intc/imsic: prevent to use IMSIC when host doesn't support AIA extension

[Yong-Xuan Wang]

Currently QEMU will continue to create the IMSIC devices and enable the
AIA extension for guest OS when the host kernel doesn't support the AIA
extension. This will cause an illegal instruction exception when the
guest OS access the AIA CSRs. Add additional checks to ensure the
guest OS only uses the IMSIC devices when the host kernel supports
the AIA extension.

Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
---
 hw/intc/riscv_imsic.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/hw/intc/riscv_imsic.c b/hw/intc/riscv_imsic.c
index dc8162c0a7c9..8c64f2c21274 100644
--- a/hw/intc/riscv_imsic.c
+++ b/hw/intc/riscv_imsic.c
@@ -375,12 +375,21 @@ static void riscv_imsic_realize(DeviceState *dev, Error **errp)
 
     /* Force select AIA feature and setup CSR read-modify-write callback */
     if (env) {
-        if (!imsic->mmode) {
-            rcpu->cfg.ext_ssaia = true;
-            riscv_cpu_set_geilen(env, imsic->num_pages - 1);
+        if (kvm_enabled()) {
+            if (!rcpu->cfg.ext_ssaia) {
+                error_report("Host machine doesn't support AIA extension. "
+                             "Do not use IMSIC as interrupt controller.");
+                exit(1);
+            }
         } else {
-            rcpu->cfg.ext_smaia = true;
+            if (!imsic->mmode) {
+                rcpu->cfg.ext_ssaia = true;
+                riscv_cpu_set_geilen(env, imsic->num_pages - 1);
+            } else {
+                rcpu->cfg.ext_smaia = true;
+            }
         }
+
         riscv_cpu_set_aia_ireg_rmw_fn(env, (imsic->mmode) ? PRV_M : PRV_S,
                                       riscv_imsic_rmw, imsic);
     }
-- 
2.17.1

Re: [PATCH v2 8/8] hw/intc/imsic: prevent to use IMSIC when host doesn't support AIA extension

[Andrew Jones]

On Mon, Feb 24, 2025 at 04:24:15PM +0800, Yong-Xuan Wang wrote:
> Currently QEMU will continue to create the IMSIC devices and enable the
> AIA extension for guest OS when the host kernel doesn't support the AIA
> extension. This will cause an illegal instruction exception when the
> guest OS access the AIA CSRs. Add additional checks to ensure the
> guest OS only uses the IMSIC devices when the host kernel supports
> the AIA extension.
> 
> Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
> ---
>  hw/intc/riscv_imsic.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/intc/riscv_imsic.c b/hw/intc/riscv_imsic.c
> index dc8162c0a7c9..8c64f2c21274 100644
> --- a/hw/intc/riscv_imsic.c
> +++ b/hw/intc/riscv_imsic.c
> @@ -375,12 +375,21 @@ static void riscv_imsic_realize(DeviceState *dev, Error **errp)
>  
>      /* Force select AIA feature and setup CSR read-modify-write callback */
>      if (env) {
> -        if (!imsic->mmode) {
> -            rcpu->cfg.ext_ssaia = true;
> -            riscv_cpu_set_geilen(env, imsic->num_pages - 1);
> +        if (kvm_enabled()) {
> +            if (!rcpu->cfg.ext_ssaia) {
> +                error_report("Host machine doesn't support AIA extension. "
> +                             "Do not use IMSIC as interrupt controller.");
> +                exit(1);
> +            }
>          } else {
> -            rcpu->cfg.ext_smaia = true;
> +            if (!imsic->mmode) {
> +                rcpu->cfg.ext_ssaia = true;
> +                riscv_cpu_set_geilen(env, imsic->num_pages - 1);
> +            } else {
> +                rcpu->cfg.ext_smaia = true;
> +            }
>          }
> +
>          riscv_cpu_set_aia_ireg_rmw_fn(env, (imsic->mmode) ? PRV_M : PRV_S,
>                                        riscv_imsic_rmw, imsic);
>      }
> -- 
> 2.17.1
> 
>

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

Re: [PATCH v2 0/8] riscv: AIA: kernel-irqchip=off support

[Kashyap Chamarthy]

On Mon, Feb 24, 2025 at 04:24:07PM +0800, Yong-Xuan Wang wrote:
> This series introduces the user-space AIA MSI emulation when using KVM
> acceleration.
> 
> After this series, RISC-V QEMU virt machine with KVM acceleration has
> 3 parameters to control the type of irqchip and its emulation method:
> - Machine prop "aia" controls the type of irqchip
>   - none: use PLIC and emulated in user-space
>   - aplic: use AIA wired and emulated in user-space
>   - aplic-imsic: use AIA MSI, emulation mode is determined by
>     "kernel-irqchip" and "riscv-aia". This can only be used
>     when the host machine support the AIA extension.
> - Accel prop "kernel-irqchip", effective with AIA MSI
>   - on: in-kernel APLIC and in-kernel IMSIC
>   - off: user-space APLIC and user-space IMSIC
>   - split: user-space APLIC and in-kernel IMSIC
> - Accel prop "riscv-aia", effective with in-kernel IMSIC
>   - emul: use MRIF as in-kernel IMSIC
>   - hw: use hardware guest IMSIC file as in-kernel IMSIC
>   - auto: use the hardware guest IMSICs whenever available otherwise
>     fallback to MRIF

[...]

> Yong-Xuan Wang (8):
>   target/riscv/kvm: rewrite get/set for KVM_REG_RISCV_CSR
>   target/riscv/kvm: add KVM_REG_RISCV_CSR_AIA
>   target/riscv/kvm: add KVM_REG_RISCV_CSR_SMSTATEEN
>   target/riscv: add helper to get CSR name
>   target/riscv/kvm: rewrite kvm_riscv_handle_csr
>   target/riscv/kvm: add CSR_SIREG and CSR_STOPEI emulation
>   docs: update the description about RISC-V AIA
>   hw/intc/imsic: prevent to use IMSIC when host doesn't support AIA
>     extension
> 
>  docs/specs/riscv-aia.rst   |  24 +++-
>  docs/system/riscv/virt.rst |  10 +-
>  hw/intc/riscv_imsic.c      |  17 ++-
>  target/riscv/cpu.h         |  10 +-
>  target/riscv/csr.c         |  30 ++---
>  target/riscv/kvm/kvm-cpu.c | 235 +++++++++++++++++++++++++++++++------
>  6 files changed, 254 insertions(+), 72 deletions(-)


I don't pretend to grok all the details here, and I couldn't test the
KVM-acceleration path ("-accel kvm,kernel-irqchip=split"), as I don't
yet have hardware to test.  I was able to test the TCG part:

I'm at:

    $> git describe
    v9.2.0-1976-g68d58113df

My QEMU command-line is here[1].  I tried out these two variataions:

    ... -machine virt,accel=tcg,aia=aplic,kernel-irqchip=split ...

    ... -machine virt,accel=tcg,aia=aplic-imsic,kernel-irqchip=split ...

And booted a Fedora-41 RISC-V host-generic guest image from here:

    https://dl.fedoraproject.org/pub/alt/risc-v/release/41/Server/riscv64/images/

As near as I see, nothing exploded :)

I see that:

  - From the docs: “aia=aplic” selects APLIC (advanced platform level
    interrupt controller) to handle wired interrupts whereas the
    “aia=aplic-imsic” selects APLIC and IMSIC (incoming message signaled
    interrupt controller) to handle both wired interrupts and MSIs

  - There's no user-visible change here, but only a developer-visible
    change: in terms of what's being emulated in userspace vs. in-kernel
    irqchip.)

So, if it's still not too late:

    Tested-by: Kashyap Chamarthy <kchamart@redhat.com>

        * * *

[1] QEMU command-line:
------------------------------------------------------------------------
$ ./qemu-system-riscv64
-display none
-cpu max
-no-user-config
-nodefaults
-serial stdio
-smp 4,sockets=4,cores=1,threads=1
-machine virt,accel=tcg,aia=aplic,kernel-irqchip=split
-blockdev node-name=node-Base1,driver=raw,file.driver=file,file.filename=./base1.raw,file.node-name=node-Base1-file
-device virtio-blk,drive=node-Base1,id=virtio0
-device virtio-net-device,netdev=net \
-netdev user,id=net,hostfwd=tcp::2222-:22 \
-blockdev '{"driver":"file","filename":"/usr/share/edk2/riscv/RISCV_VIRT_CODE.qcow2","node-name":"pflash0-storage","auto-read-only":true,"discard":"unmap"}' \
-blockdev '{"node-name":"pflash0-format","read-only":true,"driver":"qcow2","file":"pflash0-storage"}' \
-blockdev '{"driver":"file","filename":"/var/lib/libvirt/qemu/nvram/base1_VARS.qcow2","node-name":"pflash1-storage","auto-read-only":true,"discard":"unmap"}' \
-blockdev '{"node-name":"pflash1-format","read-only":false,"driver":"qcow2","file":"pflash1-storage"}' \
-machine virt,usb=off,dump-guest-core=off,memory-backend=riscv_virt_board.ram,pflash0=pflash0-format,pflash1=pflash1-format,acpi=off \
-m size=4194304k \
-object '{"qom-type":"memory-backend-ram","id":"riscv_virt_board.ram","size":4294967296}' \
-object rng-random,filename=/dev/urandom,id=rng \
-device virtio-rng-device,rng=rng 
------------------------------------------------------------------------

-- 
/kashyap