Re: [PATCH 2/7] hw/pci-host: add basic Nuvoton PCIe window support

[Peter Maydell <peter.maydell@linaro.org>]

On Thu, 25 Sept 2025 at 20:40, Titus Rwantare <titusr@google.com> wrote:
>
> On Thu, 25 Sept 2025 at 09:38, Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > On Tue, 9 Sept 2025 at 23:11, Yubin Zou <yubinz@google.com> wrote:
> > >
> > > From: Titus Rwantare <titusr@google.com>
> > >
> > > Adds the windowing registers without address translation
> > >
> > > Signed-off-by: Titus Rwantare <titusr@google.com>
> > > ---
> > >  hw/pci-host/npcm_pcierc.c         | 223 +++++++++++++++++++++++++++++++++++++-
> > >  include/hw/pci-host/npcm_pcierc.h |  77 ++++++++++++-
> > >  2 files changed, 297 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/hw/pci-host/npcm_pcierc.c b/hw/pci-host/npcm_pcierc.c
> > > index 3afe92e264f6ce4312e94f05b5e908840008df64..bffdec71acaba6562856b3bdd8aec07c3c153323 100644
> > > --- a/hw/pci-host/npcm_pcierc.c
> > > +++ b/hw/pci-host/npcm_pcierc.c
> > > @@ -16,6 +16,193 @@
> > >  #include "qom/object.h"
> > >  #include "trace.h"
> > >
> > > +/* Map enabled windows to a memory subregion */
> > > +static void npcm_pcierc_map_enabled(NPCMPCIERCState *s, NPCMPCIEWindow *w)
> > > +{
> > > +    MemoryRegion *system = get_system_memory();
> >
> > > +    /* TODO: set subregion to target translation address */
> > > +    /* add subregion starting at the window source address */
> > > +    if (!memory_region_is_mapped(&w->mem)) {
> > > +        memory_region_init(&w->mem, OBJECT(s), name, size);
> > > +        memory_region_add_subregion(system, bar, &w->mem);
> > > +    }
> >
> > This looks weird. Generally devices should not map themselves
> > into the system address space, although some of our older
> > pci-host devices do for historical reasons. Should we
> > be modelling this some other way?

> I can update this. What devices are doing it the new way?

I'm not sure exactly what your device is doing here in
general, but typically the device exposes sysbus MRs
to the SoC and the SoC maps those where they ought to go.

Is there documentation on what the hardware behaviour is?

thanks
-- PMM