On Thu, 27 Aug 2026 at 20:38, Daniel Henrique Barboza
<[email protected]> wrote:
>
>
>
> 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.

Agreed, x1000.

> 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 ...

But I'll move this to the next line as to not pass comment on this can of worms.

Reply via email to