Skip to contentSkip to conversation

riscv: AIA: Add in-kernel irqchips save and restore function support

PATCH Superseded Wang Yechao 3 participants 13 messages Oct 23 – Nov 11 RISC-V

v4 of 5 Superseded Latest is v5 →

lore
Share & tools
lore
#0 lore raw

v4 cover riscv: AIA: Add in-kernel irqchips save and restore function support

This series adds support for in-kernel AIA irqchip save and restore on RISC-V.

The original author is on an extended leave, so I am picking up her v3 series and carrying it forward with a few fixes. Compared to v3[1], this version mainly includes:

- Fix build issues in the TCG-only configuration.
- Rename several functions to make them more readable.
- Fix the missing section footer problem.
- Add patch 04, which fixes an assertion failure during save on --disable-tcg builds.
- rebased on riscv-to-apply.next

[1]. https://lore.kernel.org/qemu-devel/[email protected]/

Changes in v3:
- Removed unnecessary changes in bitops.h
- Link to v2: https://lore.kernel.org/qemu-devel/[email protected]/

Changes in v2:
- Added the BITS_TO_U64S macro
- Added the nr_eix field and dynamically allocated the eie/eip arrays
- Replaced the CONFIG_32BIT macro with is_32bit boolean flag
- Link to v1: https://lore.kernel.org/qemu-devel/[email protected]/

Wang Yechao (1):
target/riscv: Only save vmstate_debug when TCG is enabled

Xuemei Liu (3):
target/riscv/kvm: create kvm_riscv_aia_access_reg hw/intc/riscv_aplic: Add in-kernel aplic save and restore function hw/intc/imsic: Add in-kernel imsic save and restore function

 hw/intc/riscv_aplic.c         | 201 +++++++++++++++++++++++++++++-----
 hw/intc/riscv_imsic.c         | 180 +++++++++++++++++++++++++++---
 include/hw/intc/riscv_aplic.h |   4 +
 include/hw/intc/riscv_imsic.h |   3 +
 include/qemu/bitops.h         |   1 +
 migration/vmstate-types.c     |   1 -
 target/riscv/kvm/kvm-cpu.c    |   7 +-
 target/riscv/kvm/kvm_riscv.h  |   1 +
 target/riscv/machine.c        |   2 +-
 9 files changed, 357 insertions(+), 43 deletions(-)
Signature · 1 line
2.47.3
#1 lore raw

PATCH v4 1/4 target/riscv/kvm: create kvm_riscv_aia_access_reg

From: Xuemei Liu <[email protected]>

Create common function kvm_riscv_aia_access_reg to access APLIC and IMSIC regs

---
 target/riscv/kvm/kvm-cpu.c   | 7 ++++++-
 target/riscv/kvm/kvm_riscv.h | 1 +
 2 files changed, 7 insertions(+), 1 deletion(-)
target/riscv/kvm/kvm-cpu.c +6 -1
index c085b22b23..3ed94f7586 100644
@@ -58,6 +58,7 @@ void riscv_kvm_aplic_request(void *opaque, int irq, int level)
 
 static bool cap_has_mp_state;
 static bool mp_state_reset;
+static int aia_fd = -1;
 
 #define KVM_RISCV_REG_ID_U32(type, idx) (KVM_REG_RISCV | KVM_REG_SIZE_U32 | \
                                          type | idx)
@@ -1924,13 +1925,17 @@ void kvm_arch_accel_class_init(ObjectClass *oc)
                                     "auto");
 }
 
+void kvm_riscv_aia_access_reg(int group, uint64_t addr, void *val, bool write)
+{
+    kvm_device_access(aia_fd, group, addr, val, write, &error_abort);
+}
+
 void kvm_riscv_aia_create(MachineState *machine, uint64_t group_shift,
                           uint64_t aia_irq_num, uint64_t aia_msi_num,
                           uint64_t aplic_base, uint64_t imsic_base,
                           uint64_t guest_num)
 {
     int ret, i;
-    int aia_fd = -1;
     uint64_t default_aia_mode;
     uint64_t socket_count = riscv_socket_count(machine);
     uint64_t max_hart_per_socket = 0;
target/riscv/kvm/kvm_riscv.h +1 -0
index aef681f0a2..8649219e0e 100644
@@ -29,6 +29,7 @@ void kvm_riscv_reset_vcpu(RISCVCPU *cpu);
  * @level: %0 to clear the interrupt, non-zero to set it.
  */
 void kvm_riscv_set_irq(RISCVCPU *cpu, int irq, int level);
+void kvm_riscv_aia_access_reg(int group, uint64_t addr, void *val, bool write);
 void kvm_riscv_aia_create(MachineState *machine, uint64_t group_shift,
                           uint64_t aia_irq_num, uint64_t aia_msi_num,
                           uint64_t aplic_base, uint64_t imsic_base,
Signature · 1 line
2.47.3
Reviewed-by on v4 1/4
#2 lore raw

Re: [PATCH v4 1/4] target/riscv/kvm: create kvm_riscv_aia_access_reg

Quoted 8 lines · Wang Yechao

From: Xuemei Liu <[email protected]>

Create common function kvm_riscv_aia_access_reg to access APLIC and IMSIC regs

---

Quoted 47 lines
  target/riscv/kvm/kvm-cpu.c   | 7 ++++++-
  target/riscv/kvm/kvm_riscv.h | 1 +
  2 files changed, 7 insertions(+), 1 deletion(-)
target/riscv/kvm/kvm-cpu.c +6 -1
index c085b22b23..3ed94f7586 100644
@@ -58,6 +58,7 @@ void riscv_kvm_aplic_request(void *opaque, int irq, int level)
 
  static bool cap_has_mp_state;
  static bool mp_state_reset;
+static int aia_fd = -1;
 
  #define KVM_RISCV_REG_ID_U32(type, idx) (KVM_REG_RISCV | KVM_REG_SIZE_U32 | \
                                           type | idx)
@@ -1924,13 +1925,17 @@ void kvm_arch_accel_class_init(ObjectClass *oc)
                                      "auto");
  }
 
+void kvm_riscv_aia_access_reg(int group, uint64_t addr, void *val, bool write)
+{
+    kvm_device_access(aia_fd, group, addr, val, write, &error_abort);
+}
+
  void kvm_riscv_aia_create(MachineState *machine, uint64_t group_shift,
                            uint64_t aia_irq_num, uint64_t aia_msi_num,
                            uint64_t aplic_base, uint64_t imsic_base,
                            uint64_t guest_num)
  {
      int ret, i;
-    int aia_fd = -1;
      uint64_t default_aia_mode;
      uint64_t socket_count = riscv_socket_count(machine);
      uint64_t max_hart_per_socket = 0;
target/riscv/kvm/kvm_riscv.h +1 -0
index aef681f0a2..8649219e0e 100644
@@ -29,6 +29,7 @@ void kvm_riscv_reset_vcpu(RISCVCPU *cpu);
   * @level: %0 to clear the interrupt, non-zero to set it.
   */
  void kvm_riscv_set_irq(RISCVCPU *cpu, int irq, int level);
+void kvm_riscv_aia_access_reg(int group, uint64_t addr, void *val, bool write);
  void kvm_riscv_aia_create(MachineState *machine, uint64_t group_shift,
                            uint64_t aia_irq_num, uint64_t aia_msi_num,
                            uint64_t aplic_base, uint64_t imsic_base,
Reviewed-by on v4 1/4
#3 lore raw

Re: [PATCH v4 1/4] target/riscv/kvm: create kvm_riscv_aia_access_reg

In reply to Wang Yechao
Quoted 7 lines · Wang Yechao

From: Xuemei Liu <[email protected]>

Create common function kvm_riscv_aia_access_reg to access APLIC and IMSIC regs

Thanks,
Nutty

Quoted 48 lines
---
  target/riscv/kvm/kvm-cpu.c   | 7 ++++++-
  target/riscv/kvm/kvm_riscv.h | 1 +
  2 files changed, 7 insertions(+), 1 deletion(-)
target/riscv/kvm/kvm-cpu.c +6 -1
index c085b22b23..3ed94f7586 100644
@@ -58,6 +58,7 @@ void riscv_kvm_aplic_request(void *opaque, int irq, int level)
 
  static bool cap_has_mp_state;
  static bool mp_state_reset;
+static int aia_fd = -1;
 
  #define KVM_RISCV_REG_ID_U32(type, idx) (KVM_REG_RISCV | KVM_REG_SIZE_U32 | \
                                           type | idx)
@@ -1924,13 +1925,17 @@ void kvm_arch_accel_class_init(ObjectClass *oc)
                                      "auto");
  }
 
+void kvm_riscv_aia_access_reg(int group, uint64_t addr, void *val, bool write)
+{
+    kvm_device_access(aia_fd, group, addr, val, write, &error_abort);
+}
+
  void kvm_riscv_aia_create(MachineState *machine, uint64_t group_shift,
                            uint64_t aia_irq_num, uint64_t aia_msi_num,
                            uint64_t aplic_base, uint64_t imsic_base,
                            uint64_t guest_num)
  {
      int ret, i;
-    int aia_fd = -1;
      uint64_t default_aia_mode;
      uint64_t socket_count = riscv_socket_count(machine);
      uint64_t max_hart_per_socket = 0;
target/riscv/kvm/kvm_riscv.h +1 -0
index aef681f0a2..8649219e0e 100644
@@ -29,6 +29,7 @@ void kvm_riscv_reset_vcpu(RISCVCPU *cpu);
   * @level: %0 to clear the interrupt, non-zero to set it.
   */
  void kvm_riscv_set_irq(RISCVCPU *cpu, int irq, int level);
+void kvm_riscv_aia_access_reg(int group, uint64_t addr, void *val, bool write);
  void kvm_riscv_aia_create(MachineState *machine, uint64_t group_shift,
                            uint64_t aia_irq_num, uint64_t aia_msi_num,
                            uint64_t aplic_base, uint64_t imsic_base,
#4 lore raw

PATCH v4 2/4 hw/intc/riscv_aplic: Add in-kernel aplic save and restore function

In reply to Wang Yechao

From: Xuemei Liu <[email protected]>

Add save and restore function if riscv_use_emulated_aplic return false, it is to get and set APLIC irqchip state from KVM kernel.

---
 hw/intc/riscv_aplic.c         | 201 +++++++++++++++++++++++++++++-----
 include/hw/intc/riscv_aplic.h |   4 +
 2 files changed, 179 insertions(+), 26 deletions(-)
hw/intc/riscv_aplic.c +175 -26
index 03a0852b8b..a3d209ca23 100644
@@ -979,9 +979,7 @@ static void riscv_aplic_realize(DeviceState *dev, Error **errp)
         }
 
         aplic->bitfield_words = (aplic->num_irqs + 31) >> 5;
-        aplic->sourcecfg = g_new0(uint32_t, aplic->num_irqs);
         aplic->state = g_new0(uint32_t, aplic->num_irqs);
-        aplic->target = g_new0(uint32_t, aplic->num_irqs);
         aplic->idelivery = g_new0(uint32_t, aplic->num_harts);
         aplic->iforce = g_new0(uint32_t, aplic->num_harts);
         aplic->ithreshold = g_new0(uint32_t, aplic->num_harts);
@@ -993,8 +991,16 @@ static void riscv_aplic_realize(DeviceState *dev, Error **errp)
         if (kvm_enabled()) {
             aplic->kvm_splitmode = true;
         }
+    } else {
+        aplic->nr_words = BITS_TO_U32S(aplic->num_irqs);
+        aplic->setip = g_new0(uint32_t, aplic->nr_words);
+        aplic->clrip = g_new0(uint32_t, aplic->nr_words);
+        aplic->setie = g_new0(uint32_t, aplic->nr_words);
     }
 
+    aplic->sourcecfg = g_new0(uint32_t, aplic->num_irqs);
+    aplic->target = g_new0(uint32_t, aplic->num_irqs);
+
     /*
      * Only root APLICs have hardware IRQ lines. All non-root APLICs
      * have IRQ lines delegated by their parent APLIC.
@@ -1020,47 +1026,190 @@ static const Property riscv_aplic_properties[] = {
     DEFINE_PROP_BOOL("mmode", RISCVAPLICState, mmode, 0),
 };
 
-static bool riscv_aplic_state_needed(void *opaque)
+static bool riscv_emulated_aplic_state_needed(void *opaque)
 {
     RISCVAPLICState *aplic = opaque;
 
     return riscv_use_emulated_aplic(aplic->msimode);
 }
 
+static const VMStateDescription vmstate_emulated_aplic_state = {
+    .name = "riscv_aplic/emulated_aplic_state",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = riscv_emulated_aplic_state_needed,
+    .fields = (const VMStateField[]) {
+        VMSTATE_VARRAY_UINT32(state, RISCVAPLICState,
+                              num_irqs, 0,
+                              vmstate_info_uint32, uint32_t),
+        VMSTATE_UINT32(mmsicfgaddr, RISCVAPLICState),
+        VMSTATE_UINT32(mmsicfgaddrH, RISCVAPLICState),
+        VMSTATE_UINT32(smsicfgaddr, RISCVAPLICState),
+        VMSTATE_UINT32(smsicfgaddrH, RISCVAPLICState),
+        VMSTATE_UINT32(kvm_msicfgaddr, RISCVAPLICState),
+        VMSTATE_UINT32(kvm_msicfgaddrH, RISCVAPLICState),
+        VMSTATE_VARRAY_UINT32(idelivery, RISCVAPLICState,
+                              num_harts, 0,
+                              vmstate_info_uint32, uint32_t),
+        VMSTATE_VARRAY_UINT32(iforce, RISCVAPLICState,
+                              num_harts, 0,
+                              vmstate_info_uint32, uint32_t),
+        VMSTATE_VARRAY_UINT32(ithreshold, RISCVAPLICState,
+                              num_harts, 0,
+                              vmstate_info_uint32, uint32_t),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+#ifdef CONFIG_KVM
+static bool kvm_riscv_aplic_state_needed(void *opaque)
+{
+    RISCVAPLICState *aplic = opaque;
+
+    return !riscv_use_emulated_aplic(aplic->msimode);
+}
+
+static int kvm_riscv_aplic_state_pre_save(void *opaque)
+{
+    RISCVAPLICState *aplic = opaque;
+
+    for (uint32_t i = 0; i < aplic->nr_words; i++) {
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC,
+                                 APLIC_SETIP_BASE + i * 4,
+                                 aplic->setip + i, false);
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC,
+                                 APLIC_CLRIP_BASE + i * 4,
+                                 aplic->clrip + i, false);
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC,
+                                 APLIC_SETIE_BASE + i * 4,
+                                 aplic->setie + i, false);
+    }
+
+    return 0;
+}
+
+static int kvm_riscv_aplic_state_post_load(void *opaque, int version_id)
+{
+    RISCVAPLICState *aplic = opaque;
+
+    for (uint32_t i = 0; i < aplic->nr_words; i++) {
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC,
+                                 APLIC_SETIP_BASE + i * 4,
+                                 aplic->setip + i, true);
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC,
+                                 APLIC_CLRIP_BASE + i * 4,
+                                 aplic->clrip + i, true);
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC,
+                                 APLIC_SETIE_BASE + i * 4,
+                                 aplic->setie + i, true);
+    }
+
+    return 0;
+}
+
+static const VMStateDescription vmstate_kvm_aplic_state = {
+    .name = "riscv_aplic/kvm_aplic_state",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = kvm_riscv_aplic_state_needed,
+    .pre_save = kvm_riscv_aplic_state_pre_save,
+    .post_load = kvm_riscv_aplic_state_post_load,
+    .fields = (const VMStateField[]) {
+            VMSTATE_VARRAY_UINT32(setip, RISCVAPLICState,
+                                  nr_words, 0,
+                                  vmstate_info_uint32, uint32_t),
+            VMSTATE_VARRAY_UINT32(clrip, RISCVAPLICState,
+                                  nr_words, 0,
+                                  vmstate_info_uint32, uint32_t),
+            VMSTATE_VARRAY_UINT32(setie, RISCVAPLICState,
+                                  nr_words, 0,
+                                  vmstate_info_uint32, uint32_t),
+            VMSTATE_END_OF_LIST()
+    }
+};
+
+static int riscv_aplic_pre_save(void *opaque)
+{
+    RISCVAPLICState *aplic = opaque;
+
+    if (!riscv_use_emulated_aplic(aplic->msimode)) {
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC, APLIC_DOMAINCFG,
+                                 &aplic->domaincfg, false);
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC, APLIC_GENMSI,
+                                 &aplic->genmsi, false);
+
+        for (uint32_t i = 1; i < aplic->num_irqs; i++) {
+            kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC,
+                                     APLIC_SOURCECFG_BASE + (i - 1) * 4,
+                                     aplic->sourcecfg + i, false);
+            kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC,
+                                     APLIC_TARGET_BASE + (i - 1) * 4,
+                                     aplic->target + i, false);
+        }
+    }
+
+    return 0;
+}
+
+static int riscv_aplic_post_load(void *opaque, int version_id)
+{
+    RISCVAPLICState *aplic = opaque;
+
+    if (!riscv_use_emulated_aplic(aplic->msimode)) {
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC, APLIC_DOMAINCFG,
+                                 &aplic->domaincfg, true);
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC, APLIC_GENMSI,
+                                 &aplic->genmsi, true);
+
+        for (uint32_t i = 1; i < aplic->num_irqs; i++) {
+            kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC,
+                                     APLIC_SOURCECFG_BASE + (i - 1) * 4,
+                                     aplic->sourcecfg + i, true);
+            kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC,
+                                     APLIC_TARGET_BASE + (i - 1) * 4,
+                                     aplic->target + i, true);
+        }
+    }
+
+    return 0;
+}
+
+#else
+static int riscv_aplic_pre_save(void *opaque)
+{
+    return 0;
+}
+
+static int riscv_aplic_post_load(void *opaque, int version_id)
+{
+    return 0;
+}
+#endif
+
 static const VMStateDescription vmstate_riscv_aplic = {
     .name = "riscv_aplic",
-    .version_id = 3,
-    .minimum_version_id = 3,
-    .needed = riscv_aplic_state_needed,
+    .version_id = 4,
+    .minimum_version_id = 4,
+    .pre_save = riscv_aplic_pre_save,
+    .post_load = riscv_aplic_post_load,
     .fields = (const VMStateField[]) {
             VMSTATE_UINT32(domaincfg, RISCVAPLICState),
-            VMSTATE_UINT32(mmsicfgaddr, RISCVAPLICState),
-            VMSTATE_UINT32(mmsicfgaddrH, RISCVAPLICState),
-            VMSTATE_UINT32(smsicfgaddr, RISCVAPLICState),
-            VMSTATE_UINT32(smsicfgaddrH, RISCVAPLICState),
             VMSTATE_UINT32(genmsi, RISCVAPLICState),
-            VMSTATE_UINT32(kvm_msicfgaddr, RISCVAPLICState),
-            VMSTATE_UINT32(kvm_msicfgaddrH, RISCVAPLICState),
-            VMSTATE_VARRAY_UINT32(sourcecfg, RISCVAPLICState,
-                                  num_irqs, 0,
-                                  vmstate_info_uint32, uint32_t),
-            VMSTATE_VARRAY_UINT32(state, RISCVAPLICState,
+            VMSTATE_VARRAY_UINT32(sourcecfg , RISCVAPLICState,
                                   num_irqs, 0,
                                   vmstate_info_uint32, uint32_t),
             VMSTATE_VARRAY_UINT32(target, RISCVAPLICState,
                                   num_irqs, 0,
                                   vmstate_info_uint32, uint32_t),
-            VMSTATE_VARRAY_UINT32(idelivery, RISCVAPLICState,
-                                  num_harts, 0,
-                                  vmstate_info_uint32, uint32_t),
-            VMSTATE_VARRAY_UINT32(iforce, RISCVAPLICState,
-                                  num_harts, 0,
-                                  vmstate_info_uint32, uint32_t),
-            VMSTATE_VARRAY_UINT32(ithreshold, RISCVAPLICState,
-                                  num_harts, 0,
-                                  vmstate_info_uint32, uint32_t),
             VMSTATE_END_OF_LIST()
-        }
+    },
+    .subsections = (const VMStateDescription * const []) {
+        &vmstate_emulated_aplic_state,
+#ifdef CONFIG_KVM
+        &vmstate_kvm_aplic_state,
+#endif
+        NULL
+    }
 };
 
 static void riscv_aplic_class_init(ObjectClass *klass, const void *data)
include/hw/intc/riscv_aplic.h +4 -0
index 62d2b33ade..de0fed4748 100644
@@ -53,6 +53,9 @@ struct RISCVAPLICState {
     uint32_t *idelivery;
     uint32_t *iforce;
     uint32_t *ithreshold;
+    uint32_t *setip;
+    uint32_t *clrip;
+    uint32_t *setie;
 
     /* topology */
 #define QEMU_APLIC_MAX_CHILDREN        16
@@ -66,6 +69,7 @@ struct RISCVAPLICState {
     uint32_t num_harts;
     uint32_t iprio_mask;
     uint32_t num_irqs;
+    uint32_t nr_words;
     bool msimode;
     bool mmode;
 
Signature · 1 line
2.47.3
Reviewed-by on v4 2/4
#5 lore raw

Re: [PATCH v4 2/4] hw/intc/riscv_aplic: Add in-kernel aplic save and restore function

Quoted 8 lines · Wang Yechao

From: Xuemei Liu <[email protected]>

Add save and restore function if riscv_use_emulated_aplic return false, it is to get and set APLIC irqchip state from KVM kernel.

---

Quoted 272 lines
  hw/intc/riscv_aplic.c         | 201 +++++++++++++++++++++++++++++-----
  include/hw/intc/riscv_aplic.h |   4 +
  2 files changed, 179 insertions(+), 26 deletions(-)
hw/intc/riscv_aplic.c +175 -26
index 03a0852b8b..a3d209ca23 100644
@@ -979,9 +979,7 @@ static void riscv_aplic_realize(DeviceState *dev, Error **errp)
          }
 
          aplic->bitfield_words = (aplic->num_irqs + 31) >> 5;
-        aplic->sourcecfg = g_new0(uint32_t, aplic->num_irqs);
          aplic->state = g_new0(uint32_t, aplic->num_irqs);
-        aplic->target = g_new0(uint32_t, aplic->num_irqs);
          aplic->idelivery = g_new0(uint32_t, aplic->num_harts);
          aplic->iforce = g_new0(uint32_t, aplic->num_harts);
          aplic->ithreshold = g_new0(uint32_t, aplic->num_harts);
@@ -993,8 +991,16 @@ static void riscv_aplic_realize(DeviceState *dev, Error **errp)
          if (kvm_enabled()) {
              aplic->kvm_splitmode = true;
          }
+    } else {
+        aplic->nr_words = BITS_TO_U32S(aplic->num_irqs);
+        aplic->setip = g_new0(uint32_t, aplic->nr_words);
+        aplic->clrip = g_new0(uint32_t, aplic->nr_words);
+        aplic->setie = g_new0(uint32_t, aplic->nr_words);
      }
 
+    aplic->sourcecfg = g_new0(uint32_t, aplic->num_irqs);
+    aplic->target = g_new0(uint32_t, aplic->num_irqs);
+
      /*
       * Only root APLICs have hardware IRQ lines. All non-root APLICs
       * have IRQ lines delegated by their parent APLIC.
@@ -1020,47 +1026,190 @@ static const Property riscv_aplic_properties[] = {
      DEFINE_PROP_BOOL("mmode", RISCVAPLICState, mmode, 0),
  };
 
-static bool riscv_aplic_state_needed(void *opaque)
+static bool riscv_emulated_aplic_state_needed(void *opaque)
  {
      RISCVAPLICState *aplic = opaque;
 
      return riscv_use_emulated_aplic(aplic->msimode);
  }
 
+static const VMStateDescription vmstate_emulated_aplic_state = {
+    .name = "riscv_aplic/emulated_aplic_state",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = riscv_emulated_aplic_state_needed,
+    .fields = (const VMStateField[]) {
+        VMSTATE_VARRAY_UINT32(state, RISCVAPLICState,
+                              num_irqs, 0,
+                              vmstate_info_uint32, uint32_t),
+        VMSTATE_UINT32(mmsicfgaddr, RISCVAPLICState),
+        VMSTATE_UINT32(mmsicfgaddrH, RISCVAPLICState),
+        VMSTATE_UINT32(smsicfgaddr, RISCVAPLICState),
+        VMSTATE_UINT32(smsicfgaddrH, RISCVAPLICState),
+        VMSTATE_UINT32(kvm_msicfgaddr, RISCVAPLICState),
+        VMSTATE_UINT32(kvm_msicfgaddrH, RISCVAPLICState),
+        VMSTATE_VARRAY_UINT32(idelivery, RISCVAPLICState,
+                              num_harts, 0,
+                              vmstate_info_uint32, uint32_t),
+        VMSTATE_VARRAY_UINT32(iforce, RISCVAPLICState,
+                              num_harts, 0,
+                              vmstate_info_uint32, uint32_t),
+        VMSTATE_VARRAY_UINT32(ithreshold, RISCVAPLICState,
+                              num_harts, 0,
+                              vmstate_info_uint32, uint32_t),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+#ifdef CONFIG_KVM
+static bool kvm_riscv_aplic_state_needed(void *opaque)
+{
+    RISCVAPLICState *aplic = opaque;
+
+    return !riscv_use_emulated_aplic(aplic->msimode);
+}
+
+static int kvm_riscv_aplic_state_pre_save(void *opaque)
+{
+    RISCVAPLICState *aplic = opaque;
+
+    for (uint32_t i = 0; i < aplic->nr_words; i++) {
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC,
+                                 APLIC_SETIP_BASE + i * 4,
+                                 aplic->setip + i, false);
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC,
+                                 APLIC_CLRIP_BASE + i * 4,
+                                 aplic->clrip + i, false);
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC,
+                                 APLIC_SETIE_BASE + i * 4,
+                                 aplic->setie + i, false);
+    }
+
+    return 0;
+}
+
+static int kvm_riscv_aplic_state_post_load(void *opaque, int version_id)
+{
+    RISCVAPLICState *aplic = opaque;
+
+    for (uint32_t i = 0; i < aplic->nr_words; i++) {
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC,
+                                 APLIC_SETIP_BASE + i * 4,
+                                 aplic->setip + i, true);
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC,
+                                 APLIC_CLRIP_BASE + i * 4,
+                                 aplic->clrip + i, true);
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC,
+                                 APLIC_SETIE_BASE + i * 4,
+                                 aplic->setie + i, true);
+    }
+
+    return 0;
+}
+
+static const VMStateDescription vmstate_kvm_aplic_state = {
+    .name = "riscv_aplic/kvm_aplic_state",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = kvm_riscv_aplic_state_needed,
+    .pre_save = kvm_riscv_aplic_state_pre_save,
+    .post_load = kvm_riscv_aplic_state_post_load,
+    .fields = (const VMStateField[]) {
+            VMSTATE_VARRAY_UINT32(setip, RISCVAPLICState,
+                                  nr_words, 0,
+                                  vmstate_info_uint32, uint32_t),
+            VMSTATE_VARRAY_UINT32(clrip, RISCVAPLICState,
+                                  nr_words, 0,
+                                  vmstate_info_uint32, uint32_t),
+            VMSTATE_VARRAY_UINT32(setie, RISCVAPLICState,
+                                  nr_words, 0,
+                                  vmstate_info_uint32, uint32_t),
+            VMSTATE_END_OF_LIST()
+    }
+};
+
+static int riscv_aplic_pre_save(void *opaque)
+{
+    RISCVAPLICState *aplic = opaque;
+
+    if (!riscv_use_emulated_aplic(aplic->msimode)) {
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC, APLIC_DOMAINCFG,
+                                 &aplic->domaincfg, false);
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC, APLIC_GENMSI,
+                                 &aplic->genmsi, false);
+
+        for (uint32_t i = 1; i < aplic->num_irqs; i++) {
+            kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC,
+                                     APLIC_SOURCECFG_BASE + (i - 1) * 4,
+                                     aplic->sourcecfg + i, false);
+            kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC,
+                                     APLIC_TARGET_BASE + (i - 1) * 4,
+                                     aplic->target + i, false);
+        }
+    }
+
+    return 0;
+}
+
+static int riscv_aplic_post_load(void *opaque, int version_id)
+{
+    RISCVAPLICState *aplic = opaque;
+
+    if (!riscv_use_emulated_aplic(aplic->msimode)) {
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC, APLIC_DOMAINCFG,
+                                 &aplic->domaincfg, true);
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC, APLIC_GENMSI,
+                                 &aplic->genmsi, true);
+
+        for (uint32_t i = 1; i < aplic->num_irqs; i++) {
+            kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC,
+                                     APLIC_SOURCECFG_BASE + (i - 1) * 4,
+                                     aplic->sourcecfg + i, true);
+            kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC,
+                                     APLIC_TARGET_BASE + (i - 1) * 4,
+                                     aplic->target + i, true);
+        }
+    }
+
+    return 0;
+}
+
+#else
+static int riscv_aplic_pre_save(void *opaque)
+{
+    return 0;
+}
+
+static int riscv_aplic_post_load(void *opaque, int version_id)
+{
+    return 0;
+}
+#endif
+
  static const VMStateDescription vmstate_riscv_aplic = {
      .name = "riscv_aplic",
-    .version_id = 3,
-    .minimum_version_id = 3,
-    .needed = riscv_aplic_state_needed,
+    .version_id = 4,
+    .minimum_version_id = 4,
+    .pre_save = riscv_aplic_pre_save,
+    .post_load = riscv_aplic_post_load,
      .fields = (const VMStateField[]) {
              VMSTATE_UINT32(domaincfg, RISCVAPLICState),
-            VMSTATE_UINT32(mmsicfgaddr, RISCVAPLICState),
-            VMSTATE_UINT32(mmsicfgaddrH, RISCVAPLICState),
-            VMSTATE_UINT32(smsicfgaddr, RISCVAPLICState),
-            VMSTATE_UINT32(smsicfgaddrH, RISCVAPLICState),
              VMSTATE_UINT32(genmsi, RISCVAPLICState),
-            VMSTATE_UINT32(kvm_msicfgaddr, RISCVAPLICState),
-            VMSTATE_UINT32(kvm_msicfgaddrH, RISCVAPLICState),
-            VMSTATE_VARRAY_UINT32(sourcecfg, RISCVAPLICState,
-                                  num_irqs, 0,
-                                  vmstate_info_uint32, uint32_t),
-            VMSTATE_VARRAY_UINT32(state, RISCVAPLICState,
+            VMSTATE_VARRAY_UINT32(sourcecfg , RISCVAPLICState,
                                    num_irqs, 0,
                                    vmstate_info_uint32, uint32_t),
              VMSTATE_VARRAY_UINT32(target, RISCVAPLICState,
                                    num_irqs, 0,
                                    vmstate_info_uint32, uint32_t),
-            VMSTATE_VARRAY_UINT32(idelivery, RISCVAPLICState,
-                                  num_harts, 0,
-                                  vmstate_info_uint32, uint32_t),
-            VMSTATE_VARRAY_UINT32(iforce, RISCVAPLICState,
-                                  num_harts, 0,
-                                  vmstate_info_uint32, uint32_t),
-            VMSTATE_VARRAY_UINT32(ithreshold, RISCVAPLICState,
-                                  num_harts, 0,
-                                  vmstate_info_uint32, uint32_t),
              VMSTATE_END_OF_LIST()
-        }
+    },
+    .subsections = (const VMStateDescription * const []) {
+        &vmstate_emulated_aplic_state,
+#ifdef CONFIG_KVM
+        &vmstate_kvm_aplic_state,
+#endif
+        NULL
+    }
  };
 
  static void riscv_aplic_class_init(ObjectClass *klass, const void *data)
include/hw/intc/riscv_aplic.h +4 -0
index 62d2b33ade..de0fed4748 100644
@@ -53,6 +53,9 @@ struct RISCVAPLICState {
      uint32_t *idelivery;
      uint32_t *iforce;
      uint32_t *ithreshold;
+    uint32_t *setip;
+    uint32_t *clrip;
+    uint32_t *setie;
 
      /* topology */
  #define QEMU_APLIC_MAX_CHILDREN        16
@@ -66,6 +69,7 @@ struct RISCVAPLICState {
      uint32_t num_harts;
      uint32_t iprio_mask;
      uint32_t num_irqs;
+    uint32_t nr_words;
      bool msimode;
      bool mmode;
 
Reviewed-by on v4 2/4
#6 lore raw

Re: [PATCH v4 2/4] hw/intc/riscv_aplic: Add in-kernel aplic save and restore function

In reply to Wang Yechao
Quoted 7 lines · Wang Yechao

From: Xuemei Liu <[email protected]>

Add save and restore function if riscv_use_emulated_aplic return false, it is to get and set APLIC irqchip state from KVM kernel.

Thanks,
Nutty

Quoted 273 lines
---
  hw/intc/riscv_aplic.c         | 201 +++++++++++++++++++++++++++++-----
  include/hw/intc/riscv_aplic.h |   4 +
  2 files changed, 179 insertions(+), 26 deletions(-)
hw/intc/riscv_aplic.c +175 -26
index 03a0852b8b..a3d209ca23 100644
@@ -979,9 +979,7 @@ static void riscv_aplic_realize(DeviceState *dev, Error **errp)
          }
 
          aplic->bitfield_words = (aplic->num_irqs + 31) >> 5;
-        aplic->sourcecfg = g_new0(uint32_t, aplic->num_irqs);
          aplic->state = g_new0(uint32_t, aplic->num_irqs);
-        aplic->target = g_new0(uint32_t, aplic->num_irqs);
          aplic->idelivery = g_new0(uint32_t, aplic->num_harts);
          aplic->iforce = g_new0(uint32_t, aplic->num_harts);
          aplic->ithreshold = g_new0(uint32_t, aplic->num_harts);
@@ -993,8 +991,16 @@ static void riscv_aplic_realize(DeviceState *dev, Error **errp)
          if (kvm_enabled()) {
              aplic->kvm_splitmode = true;
          }
+    } else {
+        aplic->nr_words = BITS_TO_U32S(aplic->num_irqs);
+        aplic->setip = g_new0(uint32_t, aplic->nr_words);
+        aplic->clrip = g_new0(uint32_t, aplic->nr_words);
+        aplic->setie = g_new0(uint32_t, aplic->nr_words);
      }
 
+    aplic->sourcecfg = g_new0(uint32_t, aplic->num_irqs);
+    aplic->target = g_new0(uint32_t, aplic->num_irqs);
+
      /*
       * Only root APLICs have hardware IRQ lines. All non-root APLICs
       * have IRQ lines delegated by their parent APLIC.
@@ -1020,47 +1026,190 @@ static const Property riscv_aplic_properties[] = {
      DEFINE_PROP_BOOL("mmode", RISCVAPLICState, mmode, 0),
  };
 
-static bool riscv_aplic_state_needed(void *opaque)
+static bool riscv_emulated_aplic_state_needed(void *opaque)
  {
      RISCVAPLICState *aplic = opaque;
 
      return riscv_use_emulated_aplic(aplic->msimode);
  }
 
+static const VMStateDescription vmstate_emulated_aplic_state = {
+    .name = "riscv_aplic/emulated_aplic_state",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = riscv_emulated_aplic_state_needed,
+    .fields = (const VMStateField[]) {
+        VMSTATE_VARRAY_UINT32(state, RISCVAPLICState,
+                              num_irqs, 0,
+                              vmstate_info_uint32, uint32_t),
+        VMSTATE_UINT32(mmsicfgaddr, RISCVAPLICState),
+        VMSTATE_UINT32(mmsicfgaddrH, RISCVAPLICState),
+        VMSTATE_UINT32(smsicfgaddr, RISCVAPLICState),
+        VMSTATE_UINT32(smsicfgaddrH, RISCVAPLICState),
+        VMSTATE_UINT32(kvm_msicfgaddr, RISCVAPLICState),
+        VMSTATE_UINT32(kvm_msicfgaddrH, RISCVAPLICState),
+        VMSTATE_VARRAY_UINT32(idelivery, RISCVAPLICState,
+                              num_harts, 0,
+                              vmstate_info_uint32, uint32_t),
+        VMSTATE_VARRAY_UINT32(iforce, RISCVAPLICState,
+                              num_harts, 0,
+                              vmstate_info_uint32, uint32_t),
+        VMSTATE_VARRAY_UINT32(ithreshold, RISCVAPLICState,
+                              num_harts, 0,
+                              vmstate_info_uint32, uint32_t),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+#ifdef CONFIG_KVM
+static bool kvm_riscv_aplic_state_needed(void *opaque)
+{
+    RISCVAPLICState *aplic = opaque;
+
+    return !riscv_use_emulated_aplic(aplic->msimode);
+}
+
+static int kvm_riscv_aplic_state_pre_save(void *opaque)
+{
+    RISCVAPLICState *aplic = opaque;
+
+    for (uint32_t i = 0; i < aplic->nr_words; i++) {
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC,
+                                 APLIC_SETIP_BASE + i * 4,
+                                 aplic->setip + i, false);
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC,
+                                 APLIC_CLRIP_BASE + i * 4,
+                                 aplic->clrip + i, false);
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC,
+                                 APLIC_SETIE_BASE + i * 4,
+                                 aplic->setie + i, false);
+    }
+
+    return 0;
+}
+
+static int kvm_riscv_aplic_state_post_load(void *opaque, int version_id)
+{
+    RISCVAPLICState *aplic = opaque;
+
+    for (uint32_t i = 0; i < aplic->nr_words; i++) {
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC,
+                                 APLIC_SETIP_BASE + i * 4,
+                                 aplic->setip + i, true);
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC,
+                                 APLIC_CLRIP_BASE + i * 4,
+                                 aplic->clrip + i, true);
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC,
+                                 APLIC_SETIE_BASE + i * 4,
+                                 aplic->setie + i, true);
+    }
+
+    return 0;
+}
+
+static const VMStateDescription vmstate_kvm_aplic_state = {
+    .name = "riscv_aplic/kvm_aplic_state",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = kvm_riscv_aplic_state_needed,
+    .pre_save = kvm_riscv_aplic_state_pre_save,
+    .post_load = kvm_riscv_aplic_state_post_load,
+    .fields = (const VMStateField[]) {
+            VMSTATE_VARRAY_UINT32(setip, RISCVAPLICState,
+                                  nr_words, 0,
+                                  vmstate_info_uint32, uint32_t),
+            VMSTATE_VARRAY_UINT32(clrip, RISCVAPLICState,
+                                  nr_words, 0,
+                                  vmstate_info_uint32, uint32_t),
+            VMSTATE_VARRAY_UINT32(setie, RISCVAPLICState,
+                                  nr_words, 0,
+                                  vmstate_info_uint32, uint32_t),
+            VMSTATE_END_OF_LIST()
+    }
+};
+
+static int riscv_aplic_pre_save(void *opaque)
+{
+    RISCVAPLICState *aplic = opaque;
+
+    if (!riscv_use_emulated_aplic(aplic->msimode)) {
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC, APLIC_DOMAINCFG,
+                                 &aplic->domaincfg, false);
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC, APLIC_GENMSI,
+                                 &aplic->genmsi, false);
+
+        for (uint32_t i = 1; i < aplic->num_irqs; i++) {
+            kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC,
+                                     APLIC_SOURCECFG_BASE + (i - 1) * 4,
+                                     aplic->sourcecfg + i, false);
+            kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC,
+                                     APLIC_TARGET_BASE + (i - 1) * 4,
+                                     aplic->target + i, false);
+        }
+    }
+
+    return 0;
+}
+
+static int riscv_aplic_post_load(void *opaque, int version_id)
+{
+    RISCVAPLICState *aplic = opaque;
+
+    if (!riscv_use_emulated_aplic(aplic->msimode)) {
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC, APLIC_DOMAINCFG,
+                                 &aplic->domaincfg, true);
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC, APLIC_GENMSI,
+                                 &aplic->genmsi, true);
+
+        for (uint32_t i = 1; i < aplic->num_irqs; i++) {
+            kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC,
+                                     APLIC_SOURCECFG_BASE + (i - 1) * 4,
+                                     aplic->sourcecfg + i, true);
+            kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC,
+                                     APLIC_TARGET_BASE + (i - 1) * 4,
+                                     aplic->target + i, true);
+        }
+    }
+
+    return 0;
+}
+
+#else
+static int riscv_aplic_pre_save(void *opaque)
+{
+    return 0;
+}
+
+static int riscv_aplic_post_load(void *opaque, int version_id)
+{
+    return 0;
+}
+#endif
+
  static const VMStateDescription vmstate_riscv_aplic = {
      .name = "riscv_aplic",
-    .version_id = 3,
-    .minimum_version_id = 3,
-    .needed = riscv_aplic_state_needed,
+    .version_id = 4,
+    .minimum_version_id = 4,
+    .pre_save = riscv_aplic_pre_save,
+    .post_load = riscv_aplic_post_load,
      .fields = (const VMStateField[]) {
              VMSTATE_UINT32(domaincfg, RISCVAPLICState),
-            VMSTATE_UINT32(mmsicfgaddr, RISCVAPLICState),
-            VMSTATE_UINT32(mmsicfgaddrH, RISCVAPLICState),
-            VMSTATE_UINT32(smsicfgaddr, RISCVAPLICState),
-            VMSTATE_UINT32(smsicfgaddrH, RISCVAPLICState),
              VMSTATE_UINT32(genmsi, RISCVAPLICState),
-            VMSTATE_UINT32(kvm_msicfgaddr, RISCVAPLICState),
-            VMSTATE_UINT32(kvm_msicfgaddrH, RISCVAPLICState),
-            VMSTATE_VARRAY_UINT32(sourcecfg, RISCVAPLICState,
-                                  num_irqs, 0,
-                                  vmstate_info_uint32, uint32_t),
-            VMSTATE_VARRAY_UINT32(state, RISCVAPLICState,
+            VMSTATE_VARRAY_UINT32(sourcecfg , RISCVAPLICState,
                                    num_irqs, 0,
                                    vmstate_info_uint32, uint32_t),
              VMSTATE_VARRAY_UINT32(target, RISCVAPLICState,
                                    num_irqs, 0,
                                    vmstate_info_uint32, uint32_t),
-            VMSTATE_VARRAY_UINT32(idelivery, RISCVAPLICState,
-                                  num_harts, 0,
-                                  vmstate_info_uint32, uint32_t),
-            VMSTATE_VARRAY_UINT32(iforce, RISCVAPLICState,
-                                  num_harts, 0,
-                                  vmstate_info_uint32, uint32_t),
-            VMSTATE_VARRAY_UINT32(ithreshold, RISCVAPLICState,
-                                  num_harts, 0,
-                                  vmstate_info_uint32, uint32_t),
              VMSTATE_END_OF_LIST()
-        }
+    },
+    .subsections = (const VMStateDescription * const []) {
+        &vmstate_emulated_aplic_state,
+#ifdef CONFIG_KVM
+        &vmstate_kvm_aplic_state,
+#endif
+        NULL
+    }
  };
 
  static void riscv_aplic_class_init(ObjectClass *klass, const void *data)
include/hw/intc/riscv_aplic.h +4 -0
index 62d2b33ade..de0fed4748 100644
@@ -53,6 +53,9 @@ struct RISCVAPLICState {
      uint32_t *idelivery;
      uint32_t *iforce;
      uint32_t *ithreshold;
+    uint32_t *setip;
+    uint32_t *clrip;
+    uint32_t *setie;
 
      /* topology */
  #define QEMU_APLIC_MAX_CHILDREN        16
@@ -66,6 +69,7 @@ struct RISCVAPLICState {
      uint32_t num_harts;
      uint32_t iprio_mask;
      uint32_t num_irqs;
+    uint32_t nr_words;
      bool msimode;
      bool mmode;
 
#7 lore raw

PATCH v4 3/4 hw/intc/imsic: Add in-kernel imsic save and restore function

In reply to Wang Yechao

From: Xuemei Liu <[email protected]>

Add save and restore funtction if kvm_irqchip_in_kernel() return true, it is to get and set IMSIC irqchip state from KVM kernel.

---
 hw/intc/riscv_imsic.c         | 180 +++++++++++++++++++++++++++++++---
 include/hw/intc/riscv_imsic.h |   3 +
 include/qemu/bitops.h         |   1 +
 migration/vmstate-types.c     |   1 -
 4 files changed, 170 insertions(+), 15 deletions(-)
hw/intc/riscv_imsic.c +166 -14
index cb9ce9612c..43e4d11426 100644
@@ -34,6 +34,7 @@
 #include "system/system.h"
 #include "system/kvm.h"
 #include "migration/vmstate.h"
+#include "kvm/kvm_riscv.h"
 
 #define IMSIC_MMIO_PAGE_LE             0x00
 #define IMSIC_MMIO_PAGE_BE             0x04
@@ -408,11 +409,16 @@ static void riscv_imsic_realize(DeviceState *dev, Error **errp)
         qdev_init_gpio_out(dev, imsic->external_irqs, imsic->num_pages);
 
         imsic->num_eistate = imsic->num_pages * imsic->num_irqs;
-        imsic->eidelivery = g_new0(uint32_t, imsic->num_pages);
-        imsic->eithreshold = g_new0(uint32_t, imsic->num_pages);
         imsic->eistate = g_new0(uint32_t, imsic->num_eistate);
+    } else {
+        imsic->nr_eix = 2 * BITS_TO_U64S(imsic->num_irqs);
+        imsic->eie = g_new0(uint32_t, imsic->nr_eix);
+        imsic->eip = g_new0(uint32_t, imsic->nr_eix);
     }
 
+    imsic->eidelivery = g_new0(uint32_t, imsic->num_pages);
+    imsic->eithreshold = g_new0(uint32_t, imsic->num_pages);
+
     memory_region_init_io(&imsic->mmio, OBJECT(dev), &riscv_imsic_ops,
                           imsic, TYPE_RISCV_IMSIC,
                           IMSIC_MMIO_SIZE(imsic->num_pages));
@@ -443,23 +449,17 @@ static const Property riscv_imsic_properties[] = {
     DEFINE_PROP_UINT32("num-irqs", RISCVIMSICState, num_irqs, 0),
 };
 
-static bool riscv_imsic_state_needed(void *opaque)
+static bool riscv_emulated_imsic_state_needed(void *opaque)
 {
     return !kvm_irqchip_in_kernel();
 }
 
-static const VMStateDescription vmstate_riscv_imsic = {
-    .name = "riscv_imsic",
-    .version_id = 2,
-    .minimum_version_id = 2,
-    .needed = riscv_imsic_state_needed,
+static const VMStateDescription vmstate_emulated_imsic_state = {
+    .name = "riscv_imsic/emulated_imsic_state",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = riscv_emulated_imsic_state_needed,
     .fields = (const VMStateField[]) {
-            VMSTATE_VARRAY_UINT32(eidelivery, RISCVIMSICState,
-                                  num_pages, 0,
-                                  vmstate_info_uint32, uint32_t),
-            VMSTATE_VARRAY_UINT32(eithreshold, RISCVIMSICState,
-                                  num_pages, 0,
-                                  vmstate_info_uint32, uint32_t),
             VMSTATE_VARRAY_UINT32(eistate, RISCVIMSICState,
                                   num_eistate, 0,
                                   vmstate_info_uint32, uint32_t),
@@ -467,6 +467,158 @@ static const VMStateDescription vmstate_riscv_imsic = {
         }
 };
 
+#ifdef CONFIG_KVM
+static bool kvm_riscv_imsic_state_needed(void *opaque)
+{
+    return kvm_irqchip_in_kernel();
+}
+
+static int kvm_riscv_imsic_state_pre_save(void *opaque)
+{
+    RISCVIMSICState *imsic = opaque;
+    RISCVCPU *rcpu = RISCV_CPU(cpu_by_arch_id(imsic->hartid));
+    bool is_32bit = riscv_cpu_is_32bit(rcpu);
+    uint32_t inc = 2;
+    uint64_t attr;
+
+    if (is_32bit) {
+        inc = 1;
+    }
+
+    for (uint32_t i = 0; i < imsic->nr_eix; i += inc) {
+        attr = KVM_DEV_RISCV_AIA_IMSIC_MKATTR(imsic->hartid,
+                                              ISELECT_IMSIC_EIE0 + i);
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_IMSIC, attr,
+                                 imsic->eie + i, false);
+
+        attr = KVM_DEV_RISCV_AIA_IMSIC_MKATTR(imsic->hartid,
+                                              ISELECT_IMSIC_EIP0 + i);
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_IMSIC, attr,
+                                     imsic->eip + i, false);
+    }
+
+    return 0;
+}
+
+static int kvm_riscv_imsic_state_post_load(void *opaque, int version_id)
+{
+    RISCVIMSICState *imsic = opaque;
+    RISCVCPU *rcpu = RISCV_CPU(cpu_by_arch_id(imsic->hartid));
+    bool is_32bit = riscv_cpu_is_32bit(rcpu);
+    uint32_t inc = 2;
+    uint64_t attr;
+
+    if (is_32bit) {
+        inc = 1;
+    }
+
+    for (uint32_t i = 0; i < imsic->nr_eix; i += inc) {
+        attr = KVM_DEV_RISCV_AIA_IMSIC_MKATTR(imsic->hartid,
+                                              ISELECT_IMSIC_EIE0 + i);
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_IMSIC, attr,
+                                 imsic->eie + i, true);
+
+        attr = KVM_DEV_RISCV_AIA_IMSIC_MKATTR(imsic->hartid,
+                                              ISELECT_IMSIC_EIP0 + i);
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_IMSIC, attr,
+                                 imsic->eip + i, true);
+    }
+
+    return 0;
+}
+
+static const VMStateDescription vmstate_kvm_imsic_state = {
+    .name = "riscv_imsic/kvm_imsic_state",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = kvm_riscv_imsic_state_needed,
+    .pre_save = kvm_riscv_imsic_state_pre_save,
+    .post_load = kvm_riscv_imsic_state_post_load,
+    .fields = (const VMStateField[]) {
+        VMSTATE_VARRAY_UINT32(eie, RISCVIMSICState,
+                              nr_eix, 0,
+                              vmstate_info_uint32, uint32_t),
+        VMSTATE_VARRAY_UINT32(eip, RISCVIMSICState,
+                              nr_eix, 0,
+                              vmstate_info_uint32, uint32_t),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static int riscv_imsic_pre_save(void *opaque)
+{
+    RISCVIMSICState *imsic = opaque;
+    uint64_t attr;
+
+    if (kvm_irqchip_in_kernel()) {
+        attr = KVM_DEV_RISCV_AIA_IMSIC_MKATTR(imsic->hartid,
+                                              ISELECT_IMSIC_EIDELIVERY);
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_IMSIC, attr,
+                                 imsic->eidelivery, false);
+
+        attr = KVM_DEV_RISCV_AIA_IMSIC_MKATTR(imsic->hartid,
+                                              ISELECT_IMSIC_EITHRESHOLD);
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_IMSIC, attr,
+                                 imsic->eithreshold, false);
+   }
+
+    return 0;
+}
+
+static int riscv_imsic_post_load(void *opaque, int version_id)
+{
+    RISCVIMSICState *imsic = opaque;
+    uint64_t attr;
+
+    if (kvm_irqchip_in_kernel()) {
+        attr = KVM_DEV_RISCV_AIA_IMSIC_MKATTR(imsic->hartid,
+                                              ISELECT_IMSIC_EIDELIVERY);
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_IMSIC, attr,
+                                 imsic->eidelivery, true);
+
+        attr = KVM_DEV_RISCV_AIA_IMSIC_MKATTR(imsic->hartid,
+                                              ISELECT_IMSIC_EITHRESHOLD);
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_IMSIC, attr,
+                                 imsic->eithreshold, true);
+   }
+
+    return 0;
+}
+#else
+static int riscv_imsic_pre_save(void *opaque)
+{
+    return 0;
+}
+static int riscv_imsic_post_load(void *opaque, int version_id)
+{
+    return 0;
+}
+#endif
+
+static const VMStateDescription vmstate_riscv_imsic = {
+    .name = "riscv_imsic",
+    .version_id = 3,
+    .minimum_version_id = 3,
+    .pre_save = riscv_imsic_pre_save,
+    .post_load = riscv_imsic_post_load,
+    .fields = (const VMStateField[]) {
+        VMSTATE_VARRAY_UINT32(eidelivery, RISCVIMSICState,
+                              num_pages, 0,
+                              vmstate_info_uint32, uint32_t),
+        VMSTATE_VARRAY_UINT32(eithreshold, RISCVIMSICState,
+                              num_pages, 0,
+                              vmstate_info_uint32, uint32_t),
+        VMSTATE_END_OF_LIST()
+    },
+    .subsections = (const VMStateDescription * const []) {
+        &vmstate_emulated_imsic_state,
+#ifdef CONFIG_KVM
+        &vmstate_kvm_imsic_state,
+#endif
+        NULL
+    }
+};
+
 static void riscv_imsic_class_init(ObjectClass *klass, const void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
include/hw/intc/riscv_imsic.h +3 -0
index 0a1df1c2f8..b61820fd29 100644
@@ -54,12 +54,15 @@ struct RISCVIMSICState {
     uint32_t *eidelivery;
     uint32_t *eithreshold;
     uint32_t *eistate;
+    uint32_t *eip;
+    uint32_t *eie;
 
     /* config */
     bool mmode;
     uint32_t hartid;
     uint32_t num_pages;
     uint32_t num_irqs;
+    uint32_t nr_eix;
 };
 
 DeviceState *riscv_imsic_create(MemoryRegion *container, hwaddr addr,
include/qemu/bitops.h +1 -0
index f7363a5479..383648fe59 100644
@@ -20,6 +20,7 @@
 #define BITS_PER_LONG           (sizeof (unsigned long) * BITS_PER_BYTE)
 #define BITS_TO_LONGS(nr)       DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(long))
 #define BITS_TO_U32S(nr)        DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(uint32_t))
+#define BITS_TO_U64S(nr)        DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(uint64_t))
 
 #define BIT(nr)                 (1UL << (nr))
 #define BIT_ULL(nr)             (1ULL << (nr))
migration/vmstate-types.c +0 -1
index c2d4645f31..d0d3a0e384 100644
@@ -597,7 +597,6 @@ const VMStateInfo vmstate_info_tmp = {
  * is an array of 'unsigned long', which may be either 32 or 64 bits.
  */
 /* This is the number of 64 bit words sent over the wire */
-#define BITS_TO_U64S(nr) DIV_ROUND_UP(nr, 64)
 static bool load_bitmap(QEMUFile *f, void *pv, size_t size,
                         const VMStateField *field, Error **errp)
 {
Signature · 1 line
2.47.3
Reviewed-by on v4 3/4
#8 lore raw

Re: [PATCH v4 3/4] hw/intc/imsic: Add in-kernel imsic save and restore function

Quoted 3 lines · Wang Yechao

From: Xuemei Liu <[email protected]>

Add save and restore funtction if kvm_irqchip_in_kernel() return

typo: s/funtction/function

Quoted 5 lines

true, it is to get and set IMSIC irqchip state from KVM kernel.

---

I think we'll need a separated patch for the bitops.h/vmstate-types.c change. That code isn't RISC-V specific and people might have different opinions about it.

Feel free to add my ack in both patches:

Quoted 270 lines
  hw/intc/riscv_imsic.c         | 180 +++++++++++++++++++++++++++++++---
  include/hw/intc/riscv_imsic.h |   3 +
  include/qemu/bitops.h         |   1 +
  migration/vmstate-types.c     |   1 -
  4 files changed, 170 insertions(+), 15 deletions(-)
hw/intc/riscv_imsic.c +166 -14
index cb9ce9612c..43e4d11426 100644
@@ -34,6 +34,7 @@
  #include "system/system.h"
  #include "system/kvm.h"
  #include "migration/vmstate.h"
+#include "kvm/kvm_riscv.h"
 
  #define IMSIC_MMIO_PAGE_LE             0x00
  #define IMSIC_MMIO_PAGE_BE             0x04
@@ -408,11 +409,16 @@ static void riscv_imsic_realize(DeviceState *dev, Error **errp)
          qdev_init_gpio_out(dev, imsic->external_irqs, imsic->num_pages);
 
          imsic->num_eistate = imsic->num_pages * imsic->num_irqs;
-        imsic->eidelivery = g_new0(uint32_t, imsic->num_pages);
-        imsic->eithreshold = g_new0(uint32_t, imsic->num_pages);
          imsic->eistate = g_new0(uint32_t, imsic->num_eistate);
+    } else {
+        imsic->nr_eix = 2 * BITS_TO_U64S(imsic->num_irqs);
+        imsic->eie = g_new0(uint32_t, imsic->nr_eix);
+        imsic->eip = g_new0(uint32_t, imsic->nr_eix);
      }
 
+    imsic->eidelivery = g_new0(uint32_t, imsic->num_pages);
+    imsic->eithreshold = g_new0(uint32_t, imsic->num_pages);
+
      memory_region_init_io(&imsic->mmio, OBJECT(dev), &riscv_imsic_ops,
                            imsic, TYPE_RISCV_IMSIC,
                            IMSIC_MMIO_SIZE(imsic->num_pages));
@@ -443,23 +449,17 @@ static const Property riscv_imsic_properties[] = {
      DEFINE_PROP_UINT32("num-irqs", RISCVIMSICState, num_irqs, 0),
  };
 
-static bool riscv_imsic_state_needed(void *opaque)
+static bool riscv_emulated_imsic_state_needed(void *opaque)
  {
      return !kvm_irqchip_in_kernel();
  }
 
-static const VMStateDescription vmstate_riscv_imsic = {
-    .name = "riscv_imsic",
-    .version_id = 2,
-    .minimum_version_id = 2,
-    .needed = riscv_imsic_state_needed,
+static const VMStateDescription vmstate_emulated_imsic_state = {
+    .name = "riscv_imsic/emulated_imsic_state",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = riscv_emulated_imsic_state_needed,
      .fields = (const VMStateField[]) {
-            VMSTATE_VARRAY_UINT32(eidelivery, RISCVIMSICState,
-                                  num_pages, 0,
-                                  vmstate_info_uint32, uint32_t),
-            VMSTATE_VARRAY_UINT32(eithreshold, RISCVIMSICState,
-                                  num_pages, 0,
-                                  vmstate_info_uint32, uint32_t),
              VMSTATE_VARRAY_UINT32(eistate, RISCVIMSICState,
                                    num_eistate, 0,
                                    vmstate_info_uint32, uint32_t),
@@ -467,6 +467,158 @@ static const VMStateDescription vmstate_riscv_imsic = {
          }
  };
 
+#ifdef CONFIG_KVM
+static bool kvm_riscv_imsic_state_needed(void *opaque)
+{
+    return kvm_irqchip_in_kernel();
+}
+
+static int kvm_riscv_imsic_state_pre_save(void *opaque)
+{
+    RISCVIMSICState *imsic = opaque;
+    RISCVCPU *rcpu = RISCV_CPU(cpu_by_arch_id(imsic->hartid));
+    bool is_32bit = riscv_cpu_is_32bit(rcpu);
+    uint32_t inc = 2;
+    uint64_t attr;
+
+    if (is_32bit) {
+        inc = 1;
+    }
+
+    for (uint32_t i = 0; i < imsic->nr_eix; i += inc) {
+        attr = KVM_DEV_RISCV_AIA_IMSIC_MKATTR(imsic->hartid,
+                                              ISELECT_IMSIC_EIE0 + i);
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_IMSIC, attr,
+                                 imsic->eie + i, false);
+
+        attr = KVM_DEV_RISCV_AIA_IMSIC_MKATTR(imsic->hartid,
+                                              ISELECT_IMSIC_EIP0 + i);
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_IMSIC, attr,
+                                     imsic->eip + i, false);
+    }
+
+    return 0;
+}
+
+static int kvm_riscv_imsic_state_post_load(void *opaque, int version_id)
+{
+    RISCVIMSICState *imsic = opaque;
+    RISCVCPU *rcpu = RISCV_CPU(cpu_by_arch_id(imsic->hartid));
+    bool is_32bit = riscv_cpu_is_32bit(rcpu);
+    uint32_t inc = 2;
+    uint64_t attr;
+
+    if (is_32bit) {
+        inc = 1;
+    }
+
+    for (uint32_t i = 0; i < imsic->nr_eix; i += inc) {
+        attr = KVM_DEV_RISCV_AIA_IMSIC_MKATTR(imsic->hartid,
+                                              ISELECT_IMSIC_EIE0 + i);
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_IMSIC, attr,
+                                 imsic->eie + i, true);
+
+        attr = KVM_DEV_RISCV_AIA_IMSIC_MKATTR(imsic->hartid,
+                                              ISELECT_IMSIC_EIP0 + i);
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_IMSIC, attr,
+                                 imsic->eip + i, true);
+    }
+
+    return 0;
+}
+
+static const VMStateDescription vmstate_kvm_imsic_state = {
+    .name = "riscv_imsic/kvm_imsic_state",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = kvm_riscv_imsic_state_needed,
+    .pre_save = kvm_riscv_imsic_state_pre_save,
+    .post_load = kvm_riscv_imsic_state_post_load,
+    .fields = (const VMStateField[]) {
+        VMSTATE_VARRAY_UINT32(eie, RISCVIMSICState,
+                              nr_eix, 0,
+                              vmstate_info_uint32, uint32_t),
+        VMSTATE_VARRAY_UINT32(eip, RISCVIMSICState,
+                              nr_eix, 0,
+                              vmstate_info_uint32, uint32_t),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static int riscv_imsic_pre_save(void *opaque)
+{
+    RISCVIMSICState *imsic = opaque;
+    uint64_t attr;
+
+    if (kvm_irqchip_in_kernel()) {
+        attr = KVM_DEV_RISCV_AIA_IMSIC_MKATTR(imsic->hartid,
+                                              ISELECT_IMSIC_EIDELIVERY);
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_IMSIC, attr,
+                                 imsic->eidelivery, false);
+
+        attr = KVM_DEV_RISCV_AIA_IMSIC_MKATTR(imsic->hartid,
+                                              ISELECT_IMSIC_EITHRESHOLD);
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_IMSIC, attr,
+                                 imsic->eithreshold, false);
+   }
+
+    return 0;
+}
+
+static int riscv_imsic_post_load(void *opaque, int version_id)
+{
+    RISCVIMSICState *imsic = opaque;
+    uint64_t attr;
+
+    if (kvm_irqchip_in_kernel()) {
+        attr = KVM_DEV_RISCV_AIA_IMSIC_MKATTR(imsic->hartid,
+                                              ISELECT_IMSIC_EIDELIVERY);
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_IMSIC, attr,
+                                 imsic->eidelivery, true);
+
+        attr = KVM_DEV_RISCV_AIA_IMSIC_MKATTR(imsic->hartid,
+                                              ISELECT_IMSIC_EITHRESHOLD);
+        kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_IMSIC, attr,
+                                 imsic->eithreshold, true);
+   }
+
+    return 0;
+}
+#else
+static int riscv_imsic_pre_save(void *opaque)
+{
+    return 0;
+}
+static int riscv_imsic_post_load(void *opaque, int version_id)
+{
+    return 0;
+}
+#endif
+
+static const VMStateDescription vmstate_riscv_imsic = {
+    .name = "riscv_imsic",
+    .version_id = 3,
+    .minimum_version_id = 3,
+    .pre_save = riscv_imsic_pre_save,
+    .post_load = riscv_imsic_post_load,
+    .fields = (const VMStateField[]) {
+        VMSTATE_VARRAY_UINT32(eidelivery, RISCVIMSICState,
+                              num_pages, 0,
+                              vmstate_info_uint32, uint32_t),
+        VMSTATE_VARRAY_UINT32(eithreshold, RISCVIMSICState,
+                              num_pages, 0,
+                              vmstate_info_uint32, uint32_t),
+        VMSTATE_END_OF_LIST()
+    },
+    .subsections = (const VMStateDescription * const []) {
+        &vmstate_emulated_imsic_state,
+#ifdef CONFIG_KVM
+        &vmstate_kvm_imsic_state,
+#endif
+        NULL
+    }
+};
+
  static void riscv_imsic_class_init(ObjectClass *klass, const void *data)
  {
      DeviceClass *dc = DEVICE_CLASS(klass);
include/hw/intc/riscv_imsic.h +3 -0
index 0a1df1c2f8..b61820fd29 100644
@@ -54,12 +54,15 @@ struct RISCVIMSICState {
      uint32_t *eidelivery;
      uint32_t *eithreshold;
      uint32_t *eistate;
+    uint32_t *eip;
+    uint32_t *eie;
 
      /* config */
      bool mmode;
      uint32_t hartid;
      uint32_t num_pages;
      uint32_t num_irqs;
+    uint32_t nr_eix;
  };
 
  DeviceState *riscv_imsic_create(MemoryRegion *container, hwaddr addr,
include/qemu/bitops.h +1 -0
index f7363a5479..383648fe59 100644
@@ -20,6 +20,7 @@
  #define BITS_PER_LONG           (sizeof (unsigned long) * BITS_PER_BYTE)
  #define BITS_TO_LONGS(nr)       DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(long))
  #define BITS_TO_U32S(nr)        DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(uint32_t))
+#define BITS_TO_U64S(nr)        DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(uint64_t))
 
  #define BIT(nr)                 (1UL << (nr))
  #define BIT_ULL(nr)             (1ULL << (nr))
migration/vmstate-types.c +0 -1
index c2d4645f31..d0d3a0e384 100644
@@ -597,7 +597,6 @@ const VMStateInfo vmstate_info_tmp = {
   * is an array of 'unsigned long', which may be either 32 or 64 bits.
   */
  /* This is the number of 64 bit words sent over the wire */
-#define BITS_TO_U64S(nr) DIV_ROUND_UP(nr, 64)
  static bool load_bitmap(QEMUFile *f, void *pv, size_t size,
                          const VMStateField *field, Error **errp)
  {
#9 lore raw

PATCH v4 4/4 target/riscv: Only save vmstate_debug when TCG is enabled

In reply to Wang Yechao

Commit d45b9bc65515 ("target/riscv: move TCG only files to tcg subdir") moved the TCG-only files into the tcg/ subdirectory, so env->tdataX is now only allocated and valid when TCG is enabled.

However, cfg.debug is always true in riscv_cpu_properties, so debug_needed() unconditionally returned true and the debug vmstate subsection was always considered needed. On --disable-tcg builds the corresponding fields are uninitialized, and attempting to save them triggers an assertion failure in vmstate_save_state_v:

qemu-system-riscv64: ../migration/vmstate.c:737:
vmstate_save_state_v: Assertion `first_elem || !n_elems || !size' failed.

Fix this by making debug_needed() also require tcg_enabled(), so the debug subsection is only saved when the TCG-only state actually exists.

Fixes d45b9bc65515 ("target/riscv: move TCG only files to tcg subdir")
---
 target/riscv/machine.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
target/riscv/machine.c +1 -1
index 0de06e9588..c5f0261ba7 100644
@@ -324,7 +324,7 @@ static bool debug_needed(void *opaque)
 {
     RISCVCPU *cpu = opaque;
 
-    return cpu->cfg.debug || cpu->cfg.ext_sdtrig;
+    return tcg_enabled() && (cpu->cfg.debug || cpu->cfg.ext_sdtrig);
 }
 
 static bool sdext_needed(void *opaque)
Signature · 1 line
2.47.3
Reviewed-by on v4 4/4
#10 lore raw

Re: [PATCH v4 4/4] target/riscv: Only save vmstate_debug when TCG is enabled

Quoted 13 lines · Wang Yechao

Commit d45b9bc65515 ("target/riscv: move TCG only files to tcg subdir") moved the TCG-only files into the tcg/ subdirectory, so env->tdataX is now only allocated and valid when TCG is enabled.

However, cfg.debug is always true in riscv_cpu_properties, so debug_needed() unconditionally returned true and the debug vmstate subsection was always considered needed. On --disable-tcg builds the corresponding fields are uninitialized, and attempting to save them triggers an assertion failure in vmstate_save_state_v:

   qemu-system-riscv64: ../migration/vmstate.c:737:
   vmstate_save_state_v: Assertion `first_elem || !n_elems || !size' failed.
Quoted 6 lines

Fix this by making debug_needed() also require tcg_enabled(), so the debug subsection is only saved when the TCG-only state actually exists.

Fixes d45b9bc65515 ("target/riscv: move TCG only files to tcg subdir")

---

Alistair, I believe this patch can be queued separately from the rest of the series since it's fixing TCG vmstate that isn't related to the rest of the AIA changes being made.

Thanks,
Daniel

Quoted 16 lines
  target/riscv/machine.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
target/riscv/machine.c +1 -1
index 0de06e9588..c5f0261ba7 100644
@@ -324,7 +324,7 @@ static bool debug_needed(void *opaque)
  {
      RISCVCPU *cpu = opaque;
 
-    return cpu->cfg.debug || cpu->cfg.ext_sdtrig;
+    return tcg_enabled() && (cpu->cfg.debug || cpu->cfg.ext_sdtrig);
  }
 
  static bool sdext_needed(void *opaque)
#11 lore raw

Re: [PATCH v4 4/4] target/riscv: Only save vmstate_debug when TCG is enabled

Hi Alistair,

I'm fine with it being queued separately. Would you like me to resend it as a standalone patch, or will you pick it directly from this thread?

Thanks,
Yechao

Quoted 17 lines
Quoted 17 lines
  target/riscv/machine.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
target/riscv/machine.c +1 -1
index 0de06e9588..c5f0261ba7 100644
@@ -324,7 +324,7 @@ static bool debug_needed(void *opaque)
  {
      RISCVCPU *cpu = opaque;
 
-    return cpu->cfg.debug || cpu->cfg.ext_sdtrig;
+    return tcg_enabled() && (cpu->cfg.debug || cpu->cfg.ext_sdtrig);
  }
 
  static bool sdext_needed(void *opaque)
Reviewed-by on v4 4/4
#12 lore raw

Re: [PATCH v4 4/4] target/riscv: Only save vmstate_debug when TCG is enabled

In reply to Wang Yechao
Quoted 10 lines · Wang Yechao

Commit d45b9bc65515 ("target/riscv: move TCG only files to tcg subdir") moved the TCG-only files into the tcg/ subdirectory, so env->tdataX is now only allocated and valid when TCG is enabled.

However, cfg.debug is always true in riscv_cpu_properties, so debug_needed() unconditionally returned true and the debug vmstate subsection was always considered needed. On --disable-tcg builds the corresponding fields are uninitialized, and attempting to save them triggers an assertion failure in vmstate_save_state_v:

Quoted 8 lines
   qemu-system-riscv64: ../migration/vmstate.c:737:
   vmstate_save_state_v: Assertion `first_elem || !n_elems || !size' failed.

Fix this by making debug_needed() also require tcg_enabled(), so the debug subsection is only saved when the TCG-only state actually exists.

Fixes d45b9bc65515 ("target/riscv: move TCG only files to tcg subdir")

Thanks,
Nutty

Quoted 17 lines
---
  target/riscv/machine.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
target/riscv/machine.c +1 -1
index 0de06e9588..c5f0261ba7 100644
@@ -324,7 +324,7 @@ static bool debug_needed(void *opaque)
  {
      RISCVCPU *cpu = opaque;
 
-    return cpu->cfg.debug || cpu->cfg.ext_sdtrig;
+    return tcg_enabled() && (cpu->cfg.debug || cpu->cfg.ext_sdtrig);
  }
 
  static bool sdext_needed(void *opaque)