Hi,
Sorry for taking this long to give a review. Now:
On 6/2/2026 3:31 AM, [email protected] wrote:
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.
Signed-off-by: Xuemei Liu <[email protected]>
---
hw/intc/riscv_aplic.c | 190 +++++++++++++++++++++++++++++-----
include/hw/intc/riscv_aplic.h | 4 +
2 files changed, 168 insertions(+), 26 deletions(-)
diff --git a/hw/intc/riscv_aplic.c b/hw/intc/riscv_aplic.c
index 0f61b67fc5..f544733954 100644
--- a/hw/intc/riscv_aplic.c
+++ b/hw/intc/riscv_aplic.c
@@ -975,9 +975,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);
@@ -989,8 +987,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.
@@ -1016,47 +1022,179 @@ static const Property riscv_aplic_properties[] = {
DEFINE_PROP_BOOL("mmode", RISCVAPLICState, mmode, 0),
};
-static bool riscv_aplic_state_needed(void *opaque)
+static bool riscv_aplic_emul_state_needed(void *opaque)
{
RISCVAPLICState *aplic = opaque;
return riscv_use_emulated_aplic(aplic->msimode);
}
+static const VMStateDescription vmstate_riscv_aplic_emul = {
+ .name = "riscv_aplic_emul",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .needed = riscv_aplic_emul_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()
+ }
+};
+
+static bool riscv_aplic_in_kernel_state_needed(void *opaque)
+{
+ RISCVAPLICState *aplic = opaque;
+
+ return !riscv_use_emulated_aplic(aplic->msimode);
+}
+
+static int riscv_aplic_in_kernel_pre_save(void *opaque)
+{
+ RISCVAPLICState *aplic = opaque;
+
+ if (!riscv_use_emulated_aplic(aplic->msimode)) {
+ 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 riscv_aplic_in_kernel_post_load(void *opaque, int version_id)
+{
+ RISCVAPLICState *aplic = opaque;
+
+ if (!riscv_use_emulated_aplic(aplic->msimode)) {
+ 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_riscv_aplic_in_kernel = {
+ .name = "riscv_aplic_in_kernel",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .needed = riscv_aplic_in_kernel_state_needed,
+ .pre_save = riscv_aplic_in_kernel_pre_save,
+ .post_load = riscv_aplic_in_kernel_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;
+}
+
These pre_load and post_load functions will fail to build in a non RISC-V host:
ntc_riscv_aplic.c.o -MF libqemu-riscv64-softmmu.a.p/hw_intc_riscv_aplic.c.o.d
-o libqemu-riscv64-softmmu.a.p/hw_intc_riscv_aplic.c.o -c
../hw/intc/riscv_aplic.c
../hw/intc/riscv_aplic.c: In function ‘riscv_aplic_in_kernel_pre_save’:
../hw/intc/riscv_aplic.c:1077:38: error: ‘KVM_DEV_RISCV_AIA_GRP_APLIC’
undeclared (first use in this function)
1077 | kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
../hw/intc/riscv_aplic.c:1077:38: note: each undeclared identifier is reported
only once for each function it appears in
../hw/intc/riscv_aplic.c: In function ‘riscv_aplic_in_kernel_post_load’:
../hw/intc/riscv_aplic.c:1098:38: error: ‘KVM_DEV_RISCV_AIA_GRP_APLIC’
undeclared (first use in this function)
1098 | kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
../hw/intc/riscv_aplic.c: In function ‘riscv_aplic_pre_save’:
../hw/intc/riscv_aplic.c:1139:34: error: ‘KVM_DEV_RISCV_AIA_GRP_APLIC’
undeclared (first use in this function)
1139 | kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC,
APLIC_DOMAINCFG,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
../hw/intc/riscv_aplic.c: In function ‘riscv_aplic_post_load’:
../hw/intc/riscv_aplic.c:1162:34: error: ‘KVM_DEV_RISCV_AIA_GRP_APLIC’
undeclared (first use in this function)
1162 | kvm_riscv_aia_access_reg(KVM_DEV_RISCV_AIA_GRP_APLIC,
APLIC_DOMAINCFG,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
[1984/2912] Generating docs/QEMU manual with a custom command
This happens because the riscv_aplic controller is also used by TCG and in a
TCG build we don't have any visibility of these KVM regs.
Note that we had changes in how TCG files are organized to be able to support
builds with --disable-tcg. I believe your patches happened to be sent in the
middle of this transition and this is why it's not building in current master.
We need to gate this code using IFDEFs and providing stubs when we do not
have KVM enabled in the build. What we're looking for is something like this
diff:
diff --git a/hw/intc/riscv_aplic.c b/hw/intc/riscv_aplic.c
index 891b09640a..0ec8ab590a 100644
--- a/hw/intc/riscv_aplic.c
+++ b/hw/intc/riscv_aplic.c
@@ -1068,6 +1068,7 @@ static bool riscv_aplic_in_kernel_state_needed(void
*opaque)
return !riscv_use_emulated_aplic(aplic->msimode);
}
+#ifdef CONFIG_KVM
static int riscv_aplic_in_kernel_pre_save(void *opaque)
{
RISCVAPLICState *aplic = opaque;
@@ -1109,6 +1110,16 @@ static int riscv_aplic_in_kernel_post_load(void *opaque,
int version_id)
return 0;
}
+#else
+static int riscv_aplic_in_kernel_pre_save(void *opaque)
+{
+ return 0;
+}
+static int riscv_aplic_in_kernel_post_load(void *opaque, int version_id)
+{
+ return 0;
+}
+#endif
But even then we'll need to make sure that this will keep working with
--disable-tcg
builds. I suggest testing these changes with --disable-tcg to see if
everything is
working as it should.
Phil, let me know if there's a better way of handling KVM specific code in this
controller. I tried to use (kvm_enabled()) to filter out the KVM specific parts
but it refused to build. I think it has to do with the fact that these are
callback
functions and the compiler isn't able to ellide the KVM bits away.
Thanks,
Daniel
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_riscv_aplic_emul,
+ &vmstate_riscv_aplic_in_kernel,
+ NULL
+ }
};
static void riscv_aplic_class_init(ObjectClass *klass, const void *data)
diff --git a/include/hw/intc/riscv_aplic.h b/include/hw/intc/riscv_aplic.h
index c7a4d4ad01..1976bea68c 100644
--- a/include/hw/intc/riscv_aplic.h
+++ b/include/hw/intc/riscv_aplic.h
@@ -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;
