On Wed, 17 Jun 2026, Paolo Bonzini wrote:
On 6/16/26 17:55, Daniel P. Berrangé wrote:
Prepare for the move to dynamically allocated memory regions by
introducing memory_region_new and memory_region_new_io functions
which call through to object_new instead of object_initialize.
TBD: add "new" variants for all the other memory_region_init
variants.
Signed-off-by: Daniel P. Berrangé <[email protected]>
Zoltan already proposed this, but I really think it's a bad idea.
MemoryRegionOps callback will certainly access fields in the parent. Having a
separate object gives you no benefit because you still have the same
use-after-free if the MemoryRegion outlives the device. Likewise for buses.
For buses, the fact that the parent and child live at the same time is
explicitly tracked with mutual refcount and unparent. If it's not, we have
the hack that Akihiko mentioned
(https://lists.gnu.org/archive/html/qemu-devel/2026-06/msg03459.html):
However, this is insufficient to avoid calling object_ref() for all
embedded objects. For example, consider an embedded Device that has a
MemoryRegion. When referencing a MemoryRegion for guest memory access,
QEMU automatically references the owning Device to keep the MemoryRegion
alive. However, that reference is ineffective if the Device itself is
embedded, because object_ref() does not keep the containing storage
alive.
If the device itself is embedded, then all the invariants must hold
recursively:
- the device must be kept alive by mutual references between the device
itself and its parent;
- unparent for the device should wait for all guest memory accesses to be
either completed or cancelled.
So, the problem is that "QEMU automatically references the owning Device to
keep the MemoryRegion alive" is a hack that should die. I added it, mea
culpa.
The model is that if it makes sense to have B outlive A, you use new. If it
doesn't, you use init. For MemoryRegions or anything that has callbacks, it
makes no sense to outlive the implementor of the callbacks. There are
exceptions if the number of objects is dynamic but they're very rare.
I agree that there's extra complexity added by embedding; but I disagree that
removing embedding will fix any bugs, and because it will hide a relationship
between objects, I believe the complexity is not accidental.
I'm trying to understand this better. The way it originally meant to work
according to the memory region docs and implementation is that memory
region is added to an owner as a child which takes a reference and when
the owner is freed it will unparent its children which will then get
unrefed and freed as well if nothing else increased their ref count. The
problem is that when something accesses the memory region it has to keep
the owner alive as well so instead of referencing the memory region it
references the owner (not directly but using memory_region_ref which does
this). This is OK but I don't see how this mandates that the memory region
cannot be freed by the above ref counting together with the owner and why
it must be embedded? All this ref count hack does is that it passes
references of the memory region to the owner so the only reference to the
memory region itself would be from the owner and when everything unrefs
the owner (either directly or through any memory region) they would be
freed together as above. But this is now prevented by memory_region_init
having hard coded object_initialize which can only be used with embedded
objects. The problem is not referencing the owner but that memory regions
are not created with object_new so cannot be freed by QOM. So embedding
seems unrelated to this passing refs on memory regions to owner and could
be fixed separately that my patches attempted. Where the referencing the
owner hack may fail is when the owner is also owned by something else and
this does not pass refs further up the chain but that's a separate issue
which embedding does not fix either so that could be handled and fixed
separately.
Regards,
BALATON Zoltan
