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

[PATCH 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-imsic"
- 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 "kernel-irqchip", 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
  target/riscv/kvm: rename riscv-aia to riscv-imsic
  docs: update the description about RISC-V AIA

 docs/specs/riscv-aia.rst   |  24 +++-
 docs/system/riscv/virt.rst |  10 +-
 target/riscv/cpu.h         |   8 +-
 target/riscv/csr.c         |  30 ++--
 target/riscv/kvm/kvm-cpu.c | 287 +++++++++++++++++++++++++++++--------
 5 files changed, 266 insertions(+), 93 deletions(-)

-- 
2.17.1

[PATCH 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 23ce77935940..e315fea46973 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; \
         } \
@@ -605,36 +607,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

[PATCH 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 e315fea46973..e74c1d7cdcee 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); \
@@ -641,9 +644,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;
 }
@@ -651,6 +695,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

[PATCH 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 e74c1d7cdcee..79e80cdf7406 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); \
@@ -684,10 +687,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;
 }
@@ -696,6 +720,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

[PATCH 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 | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 616c3bdc1c24..133d1852ee1e 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -920,8 +920,12 @@ 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)
+{
+    return csr_ops[csr_no & (CSR_TABLE_SIZE - 1)].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

[PATCH 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 133d1852ee1e..e30c4aa0e778 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -926,8 +926,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 a62c50f057f4..df724575a5a0 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -5241,8 +5241,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;
@@ -5266,18 +5268,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 79e80cdf7406..bcd28a355a66 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -1623,26 +1623,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

[PATCH 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 df724575a5a0..95841ce65730 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 bcd28a355a66..c047d5f36951 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -1624,6 +1624,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);
@@ -1632,6 +1657,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 7/8] target/riscv/kvm: rename riscv-aia to riscv-imsic

[Yong-Xuan Wang]

The riscv-aia property only controls the in-kernel IMSIC mode, the
emulation of AIA MSI mode is controlled by the kernel-irqchip property.
Rename the riscv-aia property to riscv-imsic to prevent the confusion.

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

diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index c047d5f36951..ab53b76ab81f 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -1798,9 +1798,9 @@ void kvm_riscv_set_irq(RISCVCPU *cpu, int irq, int level)
     }
 }
 
-static int aia_mode;
+static int imsic_mode;
 
-static const char *kvm_aia_mode_str(uint64_t mode)
+static const char *kvm_imsic_mode_str(uint64_t mode)
 {
     switch (mode) {
     case KVM_DEV_RISCV_AIA_MODE_EMUL:
@@ -1813,19 +1813,19 @@ static const char *kvm_aia_mode_str(uint64_t mode)
     };
 }
 
-static char *riscv_get_kvm_aia(Object *obj, Error **errp)
+static char *riscv_get_kvm_imsic(Object *obj, Error **errp)
 {
-    return g_strdup(kvm_aia_mode_str(aia_mode));
+    return g_strdup(kvm_imsic_mode_str(imsic_mode));
 }
 
-static void riscv_set_kvm_aia(Object *obj, const char *val, Error **errp)
+static void riscv_set_kvm_imsic(Object *obj, const char *val, Error **errp)
 {
     if (!strcmp(val, "emul")) {
-        aia_mode = KVM_DEV_RISCV_AIA_MODE_EMUL;
+        imsic_mode = KVM_DEV_RISCV_AIA_MODE_EMUL;
     } else if (!strcmp(val, "hwaccel")) {
-        aia_mode = KVM_DEV_RISCV_AIA_MODE_HWACCEL;
+        imsic_mode = KVM_DEV_RISCV_AIA_MODE_HWACCEL;
     } else if (!strcmp(val, "auto")) {
-        aia_mode = KVM_DEV_RISCV_AIA_MODE_AUTO;
+        imsic_mode = KVM_DEV_RISCV_AIA_MODE_AUTO;
     } else {
         error_setg(errp, "Invalid KVM AIA mode");
         error_append_hint(errp, "Valid values are emul, hwaccel, and auto.\n");
@@ -1834,13 +1834,15 @@ static void riscv_set_kvm_aia(Object *obj, const char *val, Error **errp)
 
 void kvm_arch_accel_class_init(ObjectClass *oc)
 {
-    object_class_property_add_str(oc, "riscv-aia", riscv_get_kvm_aia,
-                                  riscv_set_kvm_aia);
-    object_class_property_set_description(oc, "riscv-aia",
-        "Set KVM AIA mode. Valid values are 'emul', 'hwaccel' and 'auto'. "
-        "Changing KVM AIA modes relies on host support. Defaults to 'auto' "
-        "if the host supports it");
-    object_property_set_default_str(object_class_property_find(oc, "riscv-aia"),
+    object_class_property_add_str(oc, "riscv-imsic", riscv_get_kvm_imsic,
+                                  riscv_set_kvm_imsic);
+    object_class_property_set_description(oc, "riscv-imsic",
+        "Set KVM IMSIC mode. Valid values are 'emul', 'hwaccel' and 'auto'. "
+        "Changing KVM IMSIC modes relies on host support. Defaults to 'auto' "
+        "if the host supports it. This property only takes effect when the "
+        "kernel-irqchip=on|split when using AIA MSI.");
+    object_property_set_default_str(object_class_property_find(oc,
+                                                               "riscv-imsic"),
                                     "auto");
 }
 
@@ -1851,7 +1853,7 @@ void kvm_riscv_aia_create(MachineState *machine, uint64_t group_shift,
 {
     int ret, i;
     int aia_fd = -1;
-    uint64_t default_aia_mode;
+    uint64_t default_imsic_mode;
     uint64_t socket_count = riscv_socket_count(machine);
     uint64_t max_hart_per_socket = 0;
     uint64_t socket, base_hart, hart_count, socket_imsic_base, imsic_addr;
@@ -1867,24 +1869,24 @@ void kvm_riscv_aia_create(MachineState *machine, uint64_t group_shift,
 
     ret = kvm_device_access(aia_fd, KVM_DEV_RISCV_AIA_GRP_CONFIG,
                             KVM_DEV_RISCV_AIA_CONFIG_MODE,
-                            &default_aia_mode, false, NULL);
+                            &default_imsic_mode, false, NULL);
     if (ret < 0) {
-        error_report("KVM AIA: failed to get current KVM AIA mode");
+        error_report("KVM AIA: failed to get current KVM IMSIC mode");
         exit(1);
     }
 
-    if (default_aia_mode != aia_mode) {
+    if (default_imsic_mode != imsic_mode) {
         ret = kvm_device_access(aia_fd, KVM_DEV_RISCV_AIA_GRP_CONFIG,
                                 KVM_DEV_RISCV_AIA_CONFIG_MODE,
-                                &aia_mode, true, NULL);
+                                &imsic_mode, true, NULL);
         if (ret < 0) {
-            warn_report("KVM AIA: failed to set KVM AIA mode '%s', using "
+            warn_report("KVM AIA: failed to set KVM IMSIC mode '%s', using "
                         "default host mode '%s'",
-                        kvm_aia_mode_str(aia_mode),
-                        kvm_aia_mode_str(default_aia_mode));
+                        kvm_imsic_mode_str(imsic_mode),
+                        kvm_imsic_mode_str(default_imsic_mode));
 
-            /* failed to change AIA mode, use default */
-            aia_mode = default_aia_mode;
+            /* failed to change IMSIC mode, use default */
+            imsic_mode = default_imsic_mode;
         }
     }
 
-- 
2.17.1

[PATCH 8/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 4/8] target/riscv: add helper to get CSR name

[Andrew Jones]

On Mon, Feb 17, 2025 at 04:17:24PM +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 | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 616c3bdc1c24..133d1852ee1e 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -920,8 +920,12 @@ 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)
> +{
> +    return csr_ops[csr_no & (CSR_TABLE_SIZE - 1)].name;

Shouldn't we assert csr_no is correct rather than silently
truncate it?

Thanks,
drew

> +}
>  
> +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 7/8] target/riscv/kvm: rename riscv-aia to riscv-imsic

[Andrew Jones]

On Mon, Feb 17, 2025 at 04:17:27PM +0800, Yong-Xuan Wang wrote:
> The riscv-aia property only controls the in-kernel IMSIC mode, the
> emulation of AIA MSI mode is controlled by the kernel-irqchip property.
> Rename the riscv-aia property to riscv-imsic to prevent the confusion.
> 
> Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
> ---
>  target/riscv/kvm/kvm-cpu.c | 52 ++++++++++++++++++++------------------
>  1 file changed, 27 insertions(+), 25 deletions(-)
> 
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index c047d5f36951..ab53b76ab81f 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -1798,9 +1798,9 @@ void kvm_riscv_set_irq(RISCVCPU *cpu, int irq, int level)
>      }
>  }
>  
> -static int aia_mode;
> +static int imsic_mode;
>  
> -static const char *kvm_aia_mode_str(uint64_t mode)
> +static const char *kvm_imsic_mode_str(uint64_t mode)
>  {
>      switch (mode) {
>      case KVM_DEV_RISCV_AIA_MODE_EMUL:
> @@ -1813,19 +1813,19 @@ static const char *kvm_aia_mode_str(uint64_t mode)
>      };
>  }
>  
> -static char *riscv_get_kvm_aia(Object *obj, Error **errp)
> +static char *riscv_get_kvm_imsic(Object *obj, Error **errp)
>  {
> -    return g_strdup(kvm_aia_mode_str(aia_mode));
> +    return g_strdup(kvm_imsic_mode_str(imsic_mode));
>  }
>  
> -static void riscv_set_kvm_aia(Object *obj, const char *val, Error **errp)
> +static void riscv_set_kvm_imsic(Object *obj, const char *val, Error **errp)
>  {
>      if (!strcmp(val, "emul")) {
> -        aia_mode = KVM_DEV_RISCV_AIA_MODE_EMUL;
> +        imsic_mode = KVM_DEV_RISCV_AIA_MODE_EMUL;
>      } else if (!strcmp(val, "hwaccel")) {
> -        aia_mode = KVM_DEV_RISCV_AIA_MODE_HWACCEL;
> +        imsic_mode = KVM_DEV_RISCV_AIA_MODE_HWACCEL;
>      } else if (!strcmp(val, "auto")) {
> -        aia_mode = KVM_DEV_RISCV_AIA_MODE_AUTO;
> +        imsic_mode = KVM_DEV_RISCV_AIA_MODE_AUTO;
>      } else {
>          error_setg(errp, "Invalid KVM AIA mode");
>          error_append_hint(errp, "Valid values are emul, hwaccel, and auto.\n");
> @@ -1834,13 +1834,15 @@ static void riscv_set_kvm_aia(Object *obj, const char *val, Error **errp)
>  
>  void kvm_arch_accel_class_init(ObjectClass *oc)
>  {
> -    object_class_property_add_str(oc, "riscv-aia", riscv_get_kvm_aia,
> -                                  riscv_set_kvm_aia);
> -    object_class_property_set_description(oc, "riscv-aia",
> -        "Set KVM AIA mode. Valid values are 'emul', 'hwaccel' and 'auto'. "
> -        "Changing KVM AIA modes relies on host support. Defaults to 'auto' "
> -        "if the host supports it");
> -    object_property_set_default_str(object_class_property_find(oc, "riscv-aia"),
> +    object_class_property_add_str(oc, "riscv-imsic", riscv_get_kvm_imsic,
> +                                  riscv_set_kvm_imsic);
> +    object_class_property_set_description(oc, "riscv-imsic",
> +        "Set KVM IMSIC mode. Valid values are 'emul', 'hwaccel' and 'auto'. "
> +        "Changing KVM IMSIC modes relies on host support. Defaults to 'auto' "
> +        "if the host supports it. This property only takes effect when the "
> +        "kernel-irqchip=on|split when using AIA MSI.");
> +    object_property_set_default_str(object_class_property_find(oc,
> +                                                               "riscv-imsic"),
>                                      "auto");

We can't change property names without deprecating the old name (which
isn't likely worth it).

Thanks,
drew


>  }
>  
> @@ -1851,7 +1853,7 @@ void kvm_riscv_aia_create(MachineState *machine, uint64_t group_shift,
>  {
>      int ret, i;
>      int aia_fd = -1;
> -    uint64_t default_aia_mode;
> +    uint64_t default_imsic_mode;
>      uint64_t socket_count = riscv_socket_count(machine);
>      uint64_t max_hart_per_socket = 0;
>      uint64_t socket, base_hart, hart_count, socket_imsic_base, imsic_addr;
> @@ -1867,24 +1869,24 @@ void kvm_riscv_aia_create(MachineState *machine, uint64_t group_shift,
>  
>      ret = kvm_device_access(aia_fd, KVM_DEV_RISCV_AIA_GRP_CONFIG,
>                              KVM_DEV_RISCV_AIA_CONFIG_MODE,
> -                            &default_aia_mode, false, NULL);
> +                            &default_imsic_mode, false, NULL);
>      if (ret < 0) {
> -        error_report("KVM AIA: failed to get current KVM AIA mode");
> +        error_report("KVM AIA: failed to get current KVM IMSIC mode");
>          exit(1);
>      }
>  
> -    if (default_aia_mode != aia_mode) {
> +    if (default_imsic_mode != imsic_mode) {
>          ret = kvm_device_access(aia_fd, KVM_DEV_RISCV_AIA_GRP_CONFIG,
>                                  KVM_DEV_RISCV_AIA_CONFIG_MODE,
> -                                &aia_mode, true, NULL);
> +                                &imsic_mode, true, NULL);
>          if (ret < 0) {
> -            warn_report("KVM AIA: failed to set KVM AIA mode '%s', using "
> +            warn_report("KVM AIA: failed to set KVM IMSIC mode '%s', using "
>                          "default host mode '%s'",
> -                        kvm_aia_mode_str(aia_mode),
> -                        kvm_aia_mode_str(default_aia_mode));
> +                        kvm_imsic_mode_str(imsic_mode),
> +                        kvm_imsic_mode_str(default_imsic_mode));
>  
> -            /* failed to change AIA mode, use default */
> -            aia_mode = default_aia_mode;
> +            /* failed to change IMSIC mode, use default */
> +            imsic_mode = default_imsic_mode;
>          }
>      }
>  
> -- 
> 2.17.1
> 

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

[Kashyap Chamarthy]

On Mon, Feb 17, 2025 at 04:17:20PM +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-imsic"
> - 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 "kernel-irqchip", 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

Hi; as someone new to the RISC-V ecosystem, most of these acronyms feel
impenetrable :-)  I see they're all RISC-V interrupt-handling related
terms.  I hope you don't mind my spelling them out here:

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

[...]

-- 
/kashyap

Re: [PATCH 7/8] target/riscv/kvm: rename riscv-aia to riscv-imsic

[Yong-Xuan Wang]

Hi Andrew,

On Mon, Feb 17, 2025 at 10:07 PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> On Mon, Feb 17, 2025 at 04:17:27PM +0800, Yong-Xuan Wang wrote:
> > The riscv-aia property only controls the in-kernel IMSIC mode, the
> > emulation of AIA MSI mode is controlled by the kernel-irqchip property.
> > Rename the riscv-aia property to riscv-imsic to prevent the confusion.
> >
> > Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
> > ---
> >  target/riscv/kvm/kvm-cpu.c | 52 ++++++++++++++++++++------------------
> >  1 file changed, 27 insertions(+), 25 deletions(-)
> >
> > diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> > index c047d5f36951..ab53b76ab81f 100644
> > --- a/target/riscv/kvm/kvm-cpu.c
> > +++ b/target/riscv/kvm/kvm-cpu.c
> > @@ -1798,9 +1798,9 @@ void kvm_riscv_set_irq(RISCVCPU *cpu, int irq, int level)
> >      }
> >  }
> >
> > -static int aia_mode;
> > +static int imsic_mode;
> >
> > -static const char *kvm_aia_mode_str(uint64_t mode)
> > +static const char *kvm_imsic_mode_str(uint64_t mode)
> >  {
> >      switch (mode) {
> >      case KVM_DEV_RISCV_AIA_MODE_EMUL:
> > @@ -1813,19 +1813,19 @@ static const char *kvm_aia_mode_str(uint64_t mode)
> >      };
> >  }
> >
> > -static char *riscv_get_kvm_aia(Object *obj, Error **errp)
> > +static char *riscv_get_kvm_imsic(Object *obj, Error **errp)
> >  {
> > -    return g_strdup(kvm_aia_mode_str(aia_mode));
> > +    return g_strdup(kvm_imsic_mode_str(imsic_mode));
> >  }
> >
> > -static void riscv_set_kvm_aia(Object *obj, const char *val, Error **errp)
> > +static void riscv_set_kvm_imsic(Object *obj, const char *val, Error **errp)
> >  {
> >      if (!strcmp(val, "emul")) {
> > -        aia_mode = KVM_DEV_RISCV_AIA_MODE_EMUL;
> > +        imsic_mode = KVM_DEV_RISCV_AIA_MODE_EMUL;
> >      } else if (!strcmp(val, "hwaccel")) {
> > -        aia_mode = KVM_DEV_RISCV_AIA_MODE_HWACCEL;
> > +        imsic_mode = KVM_DEV_RISCV_AIA_MODE_HWACCEL;
> >      } else if (!strcmp(val, "auto")) {
> > -        aia_mode = KVM_DEV_RISCV_AIA_MODE_AUTO;
> > +        imsic_mode = KVM_DEV_RISCV_AIA_MODE_AUTO;
> >      } else {
> >          error_setg(errp, "Invalid KVM AIA mode");
> >          error_append_hint(errp, "Valid values are emul, hwaccel, and auto.\n");
> > @@ -1834,13 +1834,15 @@ static void riscv_set_kvm_aia(Object *obj, const char *val, Error **errp)
> >
> >  void kvm_arch_accel_class_init(ObjectClass *oc)
> >  {
> > -    object_class_property_add_str(oc, "riscv-aia", riscv_get_kvm_aia,
> > -                                  riscv_set_kvm_aia);
> > -    object_class_property_set_description(oc, "riscv-aia",
> > -        "Set KVM AIA mode. Valid values are 'emul', 'hwaccel' and 'auto'. "
> > -        "Changing KVM AIA modes relies on host support. Defaults to 'auto' "
> > -        "if the host supports it");
> > -    object_property_set_default_str(object_class_property_find(oc, "riscv-aia"),
> > +    object_class_property_add_str(oc, "riscv-imsic", riscv_get_kvm_imsic,
> > +                                  riscv_set_kvm_imsic);
> > +    object_class_property_set_description(oc, "riscv-imsic",
> > +        "Set KVM IMSIC mode. Valid values are 'emul', 'hwaccel' and 'auto'. "
> > +        "Changing KVM IMSIC modes relies on host support. Defaults to 'auto' "
> > +        "if the host supports it. This property only takes effect when the "
> > +        "kernel-irqchip=on|split when using AIA MSI.");
> > +    object_property_set_default_str(object_class_property_find(oc,
> > +                                                               "riscv-imsic"),
> >                                      "auto");
>
> We can't change property names without deprecating the old name (which
> isn't likely worth it).
>

ok. I will remove this patch in the next version. Thank you!

Regards,
Yong-Xuan

> Thanks,
> drew
>
>
> >  }
> >
> > @@ -1851,7 +1853,7 @@ void kvm_riscv_aia_create(MachineState *machine, uint64_t group_shift,
> >  {
> >      int ret, i;
> >      int aia_fd = -1;
> > -    uint64_t default_aia_mode;
> > +    uint64_t default_imsic_mode;
> >      uint64_t socket_count = riscv_socket_count(machine);
> >      uint64_t max_hart_per_socket = 0;
> >      uint64_t socket, base_hart, hart_count, socket_imsic_base, imsic_addr;
> > @@ -1867,24 +1869,24 @@ void kvm_riscv_aia_create(MachineState *machine, uint64_t group_shift,
> >
> >      ret = kvm_device_access(aia_fd, KVM_DEV_RISCV_AIA_GRP_CONFIG,
> >                              KVM_DEV_RISCV_AIA_CONFIG_MODE,
> > -                            &default_aia_mode, false, NULL);
> > +                            &default_imsic_mode, false, NULL);
> >      if (ret < 0) {
> > -        error_report("KVM AIA: failed to get current KVM AIA mode");
> > +        error_report("KVM AIA: failed to get current KVM IMSIC mode");
> >          exit(1);
> >      }
> >
> > -    if (default_aia_mode != aia_mode) {
> > +    if (default_imsic_mode != imsic_mode) {
> >          ret = kvm_device_access(aia_fd, KVM_DEV_RISCV_AIA_GRP_CONFIG,
> >                                  KVM_DEV_RISCV_AIA_CONFIG_MODE,
> > -                                &aia_mode, true, NULL);
> > +                                &imsic_mode, true, NULL);
> >          if (ret < 0) {
> > -            warn_report("KVM AIA: failed to set KVM AIA mode '%s', using "
> > +            warn_report("KVM AIA: failed to set KVM IMSIC mode '%s', using "
> >                          "default host mode '%s'",
> > -                        kvm_aia_mode_str(aia_mode),
> > -                        kvm_aia_mode_str(default_aia_mode));
> > +                        kvm_imsic_mode_str(imsic_mode),
> > +                        kvm_imsic_mode_str(default_imsic_mode));
> >
> > -            /* failed to change AIA mode, use default */
> > -            aia_mode = default_aia_mode;
> > +            /* failed to change IMSIC mode, use default */
> > +            imsic_mode = default_imsic_mode;
> >          }
> >      }
> >
> > --
> > 2.17.1
> >

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

[Yong-Xuan Wang]

Hi Kashyap,

On Tue, Feb 18, 2025 at 11:53 PM Kashyap Chamarthy <kchamart@redhat.com> wrote:
>
> On Mon, Feb 17, 2025 at 04:17:20PM +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-imsic"
> > - 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 "kernel-irqchip", 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
>
> Hi; as someone new to the RISC-V ecosystem, most of these acronyms feel
> impenetrable :-)  I see they're all RISC-V interrupt-handling related
> terms.  I hope you don't mind my spelling them out here:
>
>   - AIA   : Advanced Interrupt Architecture
>   - MSI   : Message Signaled Interrupts
>   - PLIC  : Platform-Level Interrupt Controller
>   - APLIC : Advanced Platform Level Interrupt Controller
>   - IMSIC : Incoming Message Signaled Interrupt Controller
>   - MRIF  : Message Routed Interrupt Facility
>
> [...]
>
> --
> /kashyap
>

Sure. I would add them into the next version. Thank you!

Regards,
Yong-Xuan

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

[Yong-Xuan Wang]

Hi Andrew,

On Mon, Feb 17, 2025 at 10:01 PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> On Mon, Feb 17, 2025 at 04:17:24PM +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 | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index 616c3bdc1c24..133d1852ee1e 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -920,8 +920,12 @@ 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)
> > +{
> > +    return csr_ops[csr_no & (CSR_TABLE_SIZE - 1)].name;
>
> Shouldn't we assert csr_no is correct rather than silently
> truncate it?

ok. I would add some assertions in this function.

Regards,
Yong-Xuan

>
> Thanks,
> drew
>
> > +}
> >
> > +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
> >
> >