Re: [PATCH v2 08/39] xen/riscv: introduce device-agnostic MMIO emulation dispatch

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 9/1/26 5:36 PM, Baptiste Le Duc wrote:
> > RISC-V guests can expose several virtual interrupt controllers at
> > distinct GPA ranges: vPLIC (hasn't been introduced yet) for legacy machines,
> > vAPLIC and vIMSIC for AIA-compliant ones (are being introduced in the follow
> > up patches).
> As Jan said here [1], we shouldn't use "as later in this series" in
> commit message...
I will reword this paragraph to:
```
A RISC-V guest can be given several emulated devices at distinct GPA
ranges; the virtual interrupt controllers alone account for vPLIC on
legacy machines and vAPLIC together with vIMSIC on AIA-compliant ones.
Routing MMIO faults via a per-device is_access() check in the trap
handler would couple that handler to every device it must serve,
requiring a new conditional branch in the fault path for each emulated
device added.
```

> [1]: 
> https://lore.kernel.org/xen-devel/cover.1787838835.git.oleksii.kurochko@xxxxxxxxx/T/#m56fbac1ceb0642d5d868e9dcb2b5ed93ecbe5058
> > Routing MMIO faults via a per-device is_access() check in the
> > trap handler would couple it to every device it must serve, requiring a
> > new conditional branch in the fault path each time a new emulated device is
> > added.
> >
> > Introduce a per-domain MMIO handler registration table, modeled
> > after the equivalent ARM framework, so that virtual devices
> > self-register their GPA ranges and read/write callbacks at domain
> > creation time. The MMIO fault path delegates to a single
> > try_handle_mmio() entry point and remains agnostic of which device
> > owns a particular address.
> >
> > A subsequent patch wires this into the MMIO fault path in traps.c.
> ...same here
I'll reword this to:
```
Nothing registers a handler and try_handle_mmio() has no callers yet,
so this patch is a no-op; the trap handler is left untouched.
```

+

I will add after Signed-off-by:
```
---
Wiring this into the MMIO fault path in traps.c is done in this patch
series later.
```


[...]

> > +
> > +int register_mmio_handler(struct domain *d,
> > +                          const struct mmio_handler_ops *ops,
> > +                          paddr_t addr, paddr_t size)
> > +{
> > +    struct vmmio *vmmio = &d->arch.vmmio;
> > +    struct mmio_handler *handlers = vmmio->handlers;
> > +    paddr_t end = addr + size;
> > +    unsigned int i;
> > +    int rc = 0;
> > +    bool overlap;
> > +
> > +    if ( !ops || !ops->read || !ops->write || !size || end < addr )
> > +        return -EINVAL;
> Just a question: is the aim of end < addr check to handle possible overflow of 
> end?
Yes, your understanding is correct.

Thanks.

~ Oleksii