On Mon, 27 Oct 2025 at 13:33, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> On 27/10/25 14:26, Peter Maydell wrote:
> > (2) PCI "I/O" BARs. PCI devices can have both MMIO
> > and IO BARs. A PCI controller on x86 maps IO BARs
> > into the IO space, I think. On non-x86 the IO BARs
> > typically appear in a different window for MMIO
> > accesses. Behaviour of PCI I/O accesses to unimplemented
> > regions is probably defined by the PCI spec somewhere.
> > Behaviour of PCI accesses to unimplemented MMIO
> > window areas is I think technically left unspecified
> > by the PCI standard, but "write ignore/read -1" is
> > what x86 does and what most software expects, so
> > hardware that implements something else is making
> > its life unnecessarily difficult.
>
> Right, this is what I'd like to unify, ...
>
> > I suspect we entangle the PCI IO BAR concept and
> > implementation a bit more with the x86 I/O ops
> > implementation than we ideally ought to.
>
> ... to disentangle that.
I think my suggestion (based on about five minutes'
thought, so possibly wrong-headed ;-)) would be:
* the individual PCI devices like bochs-display and
vga-pci that currently use unassigned_io_ops should
instead define their own local MemoryRegionOps that
does what that specific device requires. (I actually
suspect that having no I/O ops at all so that the
accesses to holes in that MR fall through to the
PCI controller's default behaviour for unassigned
accesses would also produce correct behaviour, but that's
trickier to verify and would require looking at a lot
of individual pci-controller models, so the easy thing is
a local MRO.)
* patches 3, 4 and 5 are all defining the default behaviour
for "access to some MMIO window provided by a PCI
controller". If we want to disentangle this from the
x86 IO port handling, then I guess that we could either
have the generic PCI code provide a "reads as all-ones,
writes ignored" MemoryRegionOps as a convenience for
controller implementations; or we could have each
controller do it separately. I guess I prefer the former.
* I'm not sure about patch 6, but it looks like some kind
of PCI-to-other-bus bridge. It should probably define its
own MemoryRegionOps with the behaviour it wants. (Though
again it's possible that letting unassigned addresses
fall through the the PCI controller's implementation
would be neater.)
Roughly, what's happened here is that various nominally
independent bits of code have borrowed unassigned_io_ops
as a convenient pre-existing "reads as all ones, writes
ignored" MemoryRegionOps; we can disentangle by having
them implement what they're after locally rather than
borrowing something that doesn't logically belong to them.
thanks
-- PMM