On 8/26/2026 7:05 PM, Joel Stanley wrote:
riscv_aplic_create() maps the device it creates into system_memory,
which prevents its use by SoCs that map devices into their own memory
container.
Add a MemoryRegion parameter and map the device into it instead of
calling sysbus_mmio_map(). The mapping still only happens when the
APLIC is emulated. All callers pass system_memory, so there is no
change in behaviour.
Signed-off-by: Joel Stanley <[email protected]>
---
include/hw/intc/riscv_aplic.h | 2 +-
hw/intc/riscv_aplic.c | 5 +++--
hw/riscv/aia.c | 6 ++++--
hw/riscv/cps.c | 6 ++++--
hw/riscv/xiangshan_kmh.c | 6 ++++--
5 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/include/hw/intc/riscv_aplic.h b/include/hw/intc/riscv_aplic.h
index c7a4d4ad0172..baeec76e4c3e 100644
--- a/include/hw/intc/riscv_aplic.h
+++ b/include/hw/intc/riscv_aplic.h
@@ -80,7 +80,7 @@ bool riscv_is_kvm_aia_aplic_imsic(bool msimode);
bool riscv_use_emulated_aplic(bool msimode);
void riscv_aplic_set_kvm_msicfgaddr(RISCVAPLICState *aplic, hwaddr addr);
-DeviceState *riscv_aplic_create(hwaddr addr, hwaddr size,
+DeviceState *riscv_aplic_create(MemoryRegion *mr, hwaddr addr, hwaddr size,
uint32_t hartid_base, uint32_t num_harts, uint32_t num_sources,
uint32_t iprio_bits, bool msimode, bool mmode, DeviceState *parent);
diff --git a/hw/intc/riscv_aplic.c b/hw/intc/riscv_aplic.c
index 84606e9f3d88..87d7d9e49cdd 100644
--- a/hw/intc/riscv_aplic.c
+++ b/hw/intc/riscv_aplic.c
@@ -1109,7 +1109,7 @@ void riscv_aplic_add_child(DeviceState *parent,
DeviceState *child)
/*
* Create APLIC device.
*/
-DeviceState *riscv_aplic_create(hwaddr addr, hwaddr size,
+DeviceState *riscv_aplic_create(MemoryRegion *mr, hwaddr addr, hwaddr size,
uint32_t hartid_base, uint32_t num_harts, uint32_t num_sources,
uint32_t iprio_bits, bool msimode, bool mmode, DeviceState *parent)
{
@@ -1137,7 +1137,8 @@ DeviceState *riscv_aplic_create(hwaddr addr, hwaddr size,
sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
if (riscv_use_emulated_aplic(msimode)) {
- sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, addr);
+ memory_region_add_subregion(mr, addr,
+ sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0));
if (!msimode) {
for (i = 0; i < num_harts; i++) {
diff --git a/hw/riscv/aia.c b/hw/riscv/aia.c
index 49cb14349f00..974806c425e2 100644
--- a/hw/riscv/aia.c
+++ b/hw/riscv/aia.c
@@ -71,7 +71,8 @@ DeviceState *riscv_create_aia(MemoryRegion *mr, bool msimode,
int aia_guests,
if (!kvm_enabled()) {
/* Per-socket M-level APLIC */
- aplic_m_dev = riscv_aplic_create(aplic_m->base +
+ aplic_m_dev = riscv_aplic_create(mr,
+ aplic_m->base +
socket * aplic_m->size,
aplic_m->size,
(msimode) ? 0 : base_hartid,
Not a problem with your patch but seems like we have a goofed indentation here
and down below in the next riscv_aplic_create() instance. Seems like we tried
very hard to avoid breaking the 80 char line and we ended up doing this:
aplic_m_dev = riscv_aplic_create(aplic_m->base +
socket * aplic_m->size,
aplic_m->size,
(msimode) ? 0 : base_hartid,
(msimode) ? 0 : hart_count,
num_sources,
num_prio_bits,
msimode, true, NULL);
i.e we moved the MULT to the next line, indented it, and all other parameters
follow suit. The result is not pleasant.
If you could take the oppourtinity to also fix the indentation of these 2
instances
that would be terrific. It will break 80+ char line soft rule and that's fine
- the
hard limit for checkpatch is 90 chars per line for 10+ years now. It should be
120 char/line at this point but anyway ...
As for the code:
Reviewed-by: Daniel Henrique Barboza <[email protected]>
@@ -82,7 +83,8 @@ DeviceState *riscv_create_aia(MemoryRegion *mr, bool msimode,
int aia_guests,
}
/* Per-socket S-level APLIC */
- aplic_s_dev = riscv_aplic_create(aplic_s->base +
+ aplic_s_dev = riscv_aplic_create(mr,
+ aplic_s->base +
socket * aplic_s->size,
aplic_s->size,
(msimode) ? 0 : base_hartid,
diff --git a/hw/riscv/cps.c b/hw/riscv/cps.c
index 5cfb54aa27d2..d2fd9638a08d 100644
--- a/hw/riscv/cps.c
+++ b/hw/riscv/cps.c
@@ -135,14 +135,16 @@ static void riscv_cps_realize(DeviceState *dev, Error
**errp)
for (i = 0; i < num_of_clusters; i++) {
uint64_t cm_base = GLOBAL_CM_BASE + (CM_SIZE * i);
uint32_t hartid_base = i << MHARTID_CLUSTER_SHIFT;
- s->aplic = riscv_aplic_create(cm_base + AIA_PLIC_M_OFFSET,
+ s->aplic = riscv_aplic_create(get_system_memory(),
+ cm_base + AIA_PLIC_M_OFFSET,
AIA_PLIC_M_SIZE,
hartid_base, /* hartid_base */
MAX_HARTS, /* num_harts */
APLIC_NUM_SOURCES,
APLIC_NUM_PRIO_BITS,
false, true, NULL);
- riscv_aplic_create(cm_base + AIA_PLIC_S_OFFSET,
+ riscv_aplic_create(get_system_memory(),
+ cm_base + AIA_PLIC_S_OFFSET,
AIA_PLIC_S_SIZE,
hartid_base, /* hartid_base */
MAX_HARTS, /* num_harts */
diff --git a/hw/riscv/xiangshan_kmh.c b/hw/riscv/xiangshan_kmh.c
index 9eb608f8c578..c5a654ab417d 100644
--- a/hw/riscv/xiangshan_kmh.c
+++ b/hw/riscv/xiangshan_kmh.c
@@ -80,13 +80,15 @@ static DeviceState *xiangshan_kmh_create_aia(uint32_t
num_harts)
}
/* M-level APLIC */
- aplic_m = riscv_aplic_create(memmap[XIANGSHAN_KMH_APLIC_M].base,
+ aplic_m = riscv_aplic_create(get_system_memory(),
+ memmap[XIANGSHAN_KMH_APLIC_M].base,
memmap[XIANGSHAN_KMH_APLIC_M].size,
0, 0, XIANGSHAN_KMH_APLIC_NUM_SOURCES,
1, true, true, NULL);
/* S-level APLIC */
- riscv_aplic_create(memmap[XIANGSHAN_KMH_APLIC_S].base,
+ riscv_aplic_create(get_system_memory(),
+ memmap[XIANGSHAN_KMH_APLIC_S].base,
memmap[XIANGSHAN_KMH_APLIC_S].size,
0, 0, XIANGSHAN_KMH_APLIC_NUM_SOURCES,
1, true, false, aplic_m);
