On 2026/06/18 7:44, BALATON Zoltan wrote:
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.
I agree with your analysis. That's also what I pointed out in another
thread:
http://lore.kernel.org/qemu-devel/[email protected]/
Regards,
Akihiko Odaki
