Re: [RFC 4/7] system: add memory_region_new / memory_region_new_io

["Daniel P. Berrangé" <berrange@redhat.com>]

On Fri, Jun 19, 2026 at 06:32:27PM +0200, Paolo Bonzini wrote:
> On 6/18/26 00: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.
> > > 
> > > 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.
> > > 
> > > 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 disagree that removing embedding will fix any bugs
> > 
> > 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.
> 
> Yes, and that's broken.
> 
> > 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).
> 
> A mechanism to keep the owner alive exists, and it is mutual references (so
> that neither owner nor MR die) + unparent of owner (which should wait for
> pending users to complete before returning).
> 
> The fact that this mechanism is not used, i.e. unparent of a PCIDevice does
> not wait for pending BAR accesses to complete, is a bug.
> 
> > So embedding seems unrelated to this passing refs on
> > memory regions to owner and could be fixed separately that my patches
> > attempted.
> 
> Your patches add the possibility of not embedding; I don't like having two
> separate API, but I think we only have a disagreement on aesthetics.
> 
> This is different for Daniel's patches, which try to fix something by not
> embedding, and I don't think they do.

NB, I'm not claiming my patches fix all problems - foremost this was
just an initial RFC, not a fully fleshed out final solution.

>                                       Currently, if the owner dies, the
> MemoryRegion dies with it while it shouldn't (which is a problem,
> absolutely).  But these patches, while preventing the MR from dying, still
> leave around a dangling pointer to the owner from the MR, so there's not
> much gained if anything.

The mutual reference logic needs fixing regardless. These patches
simplify the modelling such that we only have a single design approach
to think about going forward.

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|