[PATCH 0/6] hw: Log unassigned MMIO accesses with unassigned_mem_ops

[PATCH 0/6] hw: Log unassigned MMIO accesses with unassigned_mem_ops

[Philippe Mathieu-Daudé]

Do not log unassigned MMIO accesses as I/O ones:
expose unassigned_mem_ops then use it instead of
unassigned_io_ops.

Philippe Mathieu-Daudé (6):
  system/memory: Expose unassigned_mem_ops symbol
  hw/display/vga: Log unassigned MMIO accesses with unassigned_mem_ops
  hw/pci-host/gpex: Log unassigned MMIO accesses with unassigned_mem_ops
  hw/pci-host/aspeed: Log unassigned MMIO accesses with
    unassigned_mem_ops
  hw/pci-host/astro: Log unassigned MMIO accesses with
    unassigned_mem_ops
  hw/sparc64/ebus: Log unassigned MMIO accesses with unassigned_mem_ops

 include/system/memory.h    | 2 ++
 system/memory-internal.h   | 2 --
 hw/display/bochs-display.c | 2 +-
 hw/display/vga-pci.c       | 4 ++--
 hw/pci-host/aspeed_pcie.c  | 2 +-
 hw/pci-host/astro.c        | 2 +-
 hw/pci-host/gpex.c         | 2 +-
 hw/sparc64/sun4u.c         | 2 +-
 8 files changed, 9 insertions(+), 9 deletions(-)

-- 
2.51.0

[PATCH 1/6] system/memory: Expose unassigned_mem_ops symbol

[Philippe Mathieu-Daudé]

To log unassigned I/O region accesses, we provide unassigned_io_ops,
exposed in "system/ioport.h". Similarly, expose unassigned_mem_ops
in "system/memory.h" to be able to log accesses to unassigned MMIO.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/system/memory.h  | 2 ++
 system/memory-internal.h | 2 --
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/system/memory.h b/include/system/memory.h
index 3bd5ffa5e0d..f42c6ba31c8 100644
--- a/include/system/memory.h
+++ b/include/system/memory.h
@@ -2714,6 +2714,8 @@ MemTxResult memory_region_dispatch_write(MemoryRegion *mr,
                                          MemOp op,
                                          MemTxAttrs attrs);
 
+extern const MemoryRegionOps unassigned_mem_ops;
+
 /**
  * address_space_init: initializes an address space
  *
diff --git a/system/memory-internal.h b/system/memory-internal.h
index 46f758fa7e4..5588ae35081 100644
--- a/system/memory-internal.h
+++ b/system/memory-internal.h
@@ -28,8 +28,6 @@ static inline AddressSpaceDispatch *address_space_to_dispatch(AddressSpace *as)
 FlatView *address_space_get_flatview(AddressSpace *as);
 void flatview_unref(FlatView *view);
 
-extern const MemoryRegionOps unassigned_mem_ops;
-
 void flatview_add_to_dispatch(FlatView *fv, MemoryRegionSection *section);
 AddressSpaceDispatch *address_space_dispatch_new(FlatView *fv);
 void address_space_dispatch_compact(AddressSpaceDispatch *d);
-- 
2.51.0

[PATCH 2/6] hw/display/vga: Log unassigned MMIO accesses with unassigned_mem_ops

[Philippe Mathieu-Daudé]

Replace unassigned_io_ops -> unassigned_mem_ops to log
accesses as MMIO ones.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/display/bochs-display.c | 2 +-
 hw/display/vga-pci.c       | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/display/bochs-display.c b/hw/display/bochs-display.c
index ad2821c9745..9fa52234219 100644
--- a/hw/display/bochs-display.c
+++ b/hw/display/bochs-display.c
@@ -286,7 +286,7 @@ static void bochs_display_realize(PCIDevice *dev, Error **errp)
     memory_region_init_io(&s->qext, obj, &bochs_display_qext_ops, s,
                           "qemu extended regs", PCI_VGA_QEXT_SIZE);
 
-    memory_region_init_io(&s->mmio, obj, &unassigned_io_ops, NULL,
+    memory_region_init_io(&s->mmio, obj, &unassigned_mem_ops, NULL,
                           "bochs-display-mmio", PCI_VGA_MMIO_SIZE);
     memory_region_add_subregion(&s->mmio, PCI_VGA_BOCHS_OFFSET, &s->vbe);
     memory_region_add_subregion(&s->mmio, PCI_VGA_QEXT_OFFSET, &s->qext);
diff --git a/hw/display/vga-pci.c b/hw/display/vga-pci.c
index b81f7fd2d0f..583f5d5dee6 100644
--- a/hw/display/vga-pci.c
+++ b/hw/display/vga-pci.c
@@ -254,7 +254,7 @@ static void pci_std_vga_realize(PCIDevice *dev, Error **errp)
 
     /* mmio bar for vga register access */
     if (d->flags & (1 << PCI_VGA_FLAG_ENABLE_MMIO)) {
-        memory_region_init_io(&d->mmio, OBJECT(dev), &unassigned_io_ops, NULL,
+        memory_region_init_io(&d->mmio, OBJECT(dev), &unassigned_mem_ops, NULL,
                               "vga.mmio", PCI_VGA_MMIO_SIZE);
 
         if (d->flags & (1 << PCI_VGA_FLAG_ENABLE_QEXT)) {
@@ -285,7 +285,7 @@ static void pci_secondary_vga_realize(PCIDevice *dev, Error **errp)
     s->con = graphic_console_init(DEVICE(dev), 0, s->hw_ops, s);
 
     /* mmio bar */
-    memory_region_init_io(&d->mmio, OBJECT(dev), &unassigned_io_ops, NULL,
+    memory_region_init_io(&d->mmio, OBJECT(dev), &unassigned_mem_ops, NULL,
                           "vga.mmio", PCI_VGA_MMIO_SIZE);
 
     if (d->flags & (1 << PCI_VGA_FLAG_ENABLE_QEXT)) {
-- 
2.51.0

[PATCH 3/6] hw/pci-host/gpex: Log unassigned MMIO accesses with unassigned_mem_ops

[Philippe Mathieu-Daudé]

Replace unassigned_io_ops -> unassigned_mem_ops to log
accesses as MMIO ones.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/pci-host/gpex.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/pci-host/gpex.c b/hw/pci-host/gpex.c
index 5f809028be2..23c8d22489d 100644
--- a/hw/pci-host/gpex.c
+++ b/hw/pci-host/gpex.c
@@ -128,7 +128,7 @@ static void gpex_host_realize(DeviceState *dev, Error **errp)
 
     if (s->allow_unmapped_accesses) {
         memory_region_init_io(&s->io_mmio_window, OBJECT(s),
-                              &unassigned_io_ops, OBJECT(s),
+                              &unassigned_mem_ops, OBJECT(s),
                               "gpex_mmio_window", UINT64_MAX);
         memory_region_init_io(&s->io_ioport_window, OBJECT(s),
                               &unassigned_io_ops, OBJECT(s),
-- 
2.51.0

[PATCH 4/6] hw/pci-host/aspeed: Log unassigned MMIO accesses with unassigned_mem_ops

[Philippe Mathieu-Daudé]

Replace unassigned_io_ops -> unassigned_mem_ops to log
accesses as MMIO ones.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/pci-host/aspeed_pcie.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/pci-host/aspeed_pcie.c b/hw/pci-host/aspeed_pcie.c
index 2499d3fe680..0b32f217e91 100644
--- a/hw/pci-host/aspeed_pcie.c
+++ b/hw/pci-host/aspeed_pcie.c
@@ -197,7 +197,7 @@ static void aspeed_pcie_rc_realize(DeviceState *dev, Error **errp)
     memory_region_init(&rc->io, OBJECT(rc), "io", 0x10000);
 
     mmio_window_name = g_strdup_printf("pcie.%d.mmio_window", cfg->id);
-    memory_region_init_io(&rc->mmio_window, OBJECT(rc), &unassigned_io_ops,
+    memory_region_init_io(&rc->mmio_window, OBJECT(rc), &unassigned_mem_ops,
                           OBJECT(rc), mmio_window_name, UINT64_MAX);
     ioport_window_name = g_strdup_printf("pcie.%d.ioport_window", cfg->id);
     memory_region_init_io(&rc->io_window, OBJECT(rc), &unassigned_io_ops,
-- 
2.51.0

[PATCH 5/6] hw/pci-host/astro: Log unassigned MMIO accesses with unassigned_mem_ops

[Philippe Mathieu-Daudé]

Replace unassigned_io_ops -> unassigned_mem_ops to log
accesses as MMIO ones.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/pci-host/astro.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/pci-host/astro.c b/hw/pci-host/astro.c
index 1024ede7b68..0bd66ab3de3 100644
--- a/hw/pci-host/astro.c
+++ b/hw/pci-host/astro.c
@@ -449,7 +449,7 @@ static void elroy_pcihost_realize(DeviceState *dev, Error **errp)
 
     /* Elroy PCI bus memory.  */
     memory_region_init(&s->pci_mmio, obj, "pci-mmio", UINT64_MAX);
-    memory_region_init_io(&s->pci_io, obj, &unassigned_io_ops, obj,
+    memory_region_init_io(&s->pci_io, obj, &unassigned_mem_ops, obj,
                             "pci-isa-mmio",
                             ((uint32_t) IOS_DIST_BASE_SIZE) / ROPES_PER_IOC);
 
-- 
2.51.0

[PATCH 6/6] hw/sparc64/ebus: Log unassigned MMIO accesses with unassigned_mem_ops

[Philippe Mathieu-Daudé]

Replace unassigned_io_ops -> unassigned_mem_ops to log
accesses as MMIO ones.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/sparc64/sun4u.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index e9f9b0a4cb9..87cf1c6c0bd 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -360,7 +360,7 @@ static void ebus_realize(PCIDevice *pci_dev, Error **errp)
      * memory access to this region to succeed which allows the OpenBSD kernel
      * to boot.
      */
-    memory_region_init_io(&s->bar0, OBJECT(s), &unassigned_io_ops, s,
+    memory_region_init_io(&s->bar0, OBJECT(s), &unassigned_mem_ops, s,
                           "bar0", 0x1000000);
     pci_register_bar(pci_dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->bar0);
     memory_region_init_alias(&s->bar1, OBJECT(s), "bar1",
-- 
2.51.0

Re: [PATCH 0/6] hw: Log unassigned MMIO accesses with unassigned_mem_ops

[Alex Bennée]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> Do not log unassigned MMIO accesses as I/O ones:
> expose unassigned_mem_ops then use it instead of
> unassigned_io_ops.

But why? Is it because ioport.c is a x86 io thing?

>
> Philippe Mathieu-Daudé (6):
>   system/memory: Expose unassigned_mem_ops symbol
>   hw/display/vga: Log unassigned MMIO accesses with unassigned_mem_ops
>   hw/pci-host/gpex: Log unassigned MMIO accesses with unassigned_mem_ops
>   hw/pci-host/aspeed: Log unassigned MMIO accesses with
>     unassigned_mem_ops
>   hw/pci-host/astro: Log unassigned MMIO accesses with
>     unassigned_mem_ops
>   hw/sparc64/ebus: Log unassigned MMIO accesses with unassigned_mem_ops
>
>  include/system/memory.h    | 2 ++
>  system/memory-internal.h   | 2 --
>  hw/display/bochs-display.c | 2 +-
>  hw/display/vga-pci.c       | 4 ++--
>  hw/pci-host/aspeed_pcie.c  | 2 +-
>  hw/pci-host/astro.c        | 2 +-
>  hw/pci-host/gpex.c         | 2 +-
>  hw/sparc64/sun4u.c         | 2 +-
>  8 files changed, 9 insertions(+), 9 deletions(-)

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH 0/6] hw: Log unassigned MMIO accesses with unassigned_mem_ops

[Philippe Mathieu-Daudé]

On 27/10/25 14:12, Alex Bennée wrote:
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> 
>> Do not log unassigned MMIO accesses as I/O ones:
>> expose unassigned_mem_ops then use it instead of
>> unassigned_io_ops.
> 
> But why? Is it because ioport.c is a x86 io thing?

Mainly. But also because log is confusing.
This is part of a bigger PCI host-bridge address-spaces
rework.

> 
>>
>> Philippe Mathieu-Daudé (6):
>>    system/memory: Expose unassigned_mem_ops symbol
>>    hw/display/vga: Log unassigned MMIO accesses with unassigned_mem_ops
>>    hw/pci-host/gpex: Log unassigned MMIO accesses with unassigned_mem_ops
>>    hw/pci-host/aspeed: Log unassigned MMIO accesses with
>>      unassigned_mem_ops
>>    hw/pci-host/astro: Log unassigned MMIO accesses with
>>      unassigned_mem_ops
>>    hw/sparc64/ebus: Log unassigned MMIO accesses with unassigned_mem_ops
>>
>>   include/system/memory.h    | 2 ++
>>   system/memory-internal.h   | 2 --
>>   hw/display/bochs-display.c | 2 +-
>>   hw/display/vga-pci.c       | 4 ++--
>>   hw/pci-host/aspeed_pcie.c  | 2 +-
>>   hw/pci-host/astro.c        | 2 +-
>>   hw/pci-host/gpex.c         | 2 +-
>>   hw/sparc64/sun4u.c         | 2 +-
>>   8 files changed, 9 insertions(+), 9 deletions(-)
> 

Re: [PATCH 0/6] hw: Log unassigned MMIO accesses with unassigned_mem_ops

[Peter Maydell]

On Mon, 27 Oct 2025 at 13:12, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>
> > Do not log unassigned MMIO accesses as I/O ones:
> > expose unassigned_mem_ops then use it instead of
> > unassigned_io_ops.
>
> But why? Is it because ioport.c is a x86 io thing?


There is a behaviour difference: unassigned_mem_ops
will fault (because of unassigned_mem_accepts()),
but unassigned_io_ops will be "reads as -1, writes
are ignored". This patch series doesn't mention any
intention of introducing a behaviour difference, so
I suspect this is not intended...

There are a couple of different but related concepts
here that we need to keep straight:

(1) x86 I/O ops, which are different CPU instructions
that talk to a different memory-space than MMIO
accesses. In QEMU we model these as accesses to the
address_space_io AddressSpace. I believe no other
target CPU has an equivalent to this.

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

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.

-- PMM

Re: [PATCH 0/6] hw: Log unassigned MMIO accesses with unassigned_mem_ops

[Philippe Mathieu-Daudé]

On 27/10/25 14:26, Peter Maydell wrote:
> On Mon, 27 Oct 2025 at 13:12, Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>>
>>> Do not log unassigned MMIO accesses as I/O ones:
>>> expose unassigned_mem_ops then use it instead of
>>> unassigned_io_ops.
>>
>> But why? Is it because ioport.c is a x86 io thing?
> 
> 
> There is a behaviour difference: unassigned_mem_ops
> will fault (because of unassigned_mem_accepts()),
> but unassigned_io_ops will be "reads as -1, writes
> are ignored". This patch series doesn't mention any
> intention of introducing a behaviour difference, so
> I suspect this is not intended...

Oops... Good catch.

> There are a couple of different but related concepts
> here that we need to keep straight:
> 
> (1) x86 I/O ops, which are different CPU instructions
> that talk to a different memory-space than MMIO
> accesses. In QEMU we model these as accesses to the
> address_space_io AddressSpace. I believe no other
> target CPU has an equivalent to this.

This is also my understanding.

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

Re: [PATCH 0/6] hw: Log unassigned MMIO accesses with unassigned_mem_ops

[Peter Maydell]

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