Our documentation says that memory regions are automatically freed
when the owner dies and the reference counting to do this is also
implemented. However this relies on the QOM free funtion that can only
be set by creating objects with object_new but memory API only
provides constructors that call object_initialize which clears the
free function that prevents QOM to manage the memory region lifetime.
Implement corresponding memory_region_new_* functions that do the same
as the memory_region_init_* functions but create the memory region
with object_new so the lifetime can be automatically managed by QOM as
documented. The memory_region_init functions are kept because they are
useful for memory regions embedded in other object or managed
externally and not by QOM for some reason.

v2:
- rebase on master
- update documentation
- use these function to fix some leaks (there may be more, e.g. in
hw/pci-host/bonito but I leave that for later and/or others)

BALATON Zoltan (10):
  memory: Add internal memory_region_set_ops helper function
  memory: Factor out common ram region initialization
  memory: Factor out more common ram region initialization
  memory: Shorten memory_region_init_rom_nomigrate
  memory: Add internal memory_region_register_ram function
  memory: Add memory_region_new* functions
  memory: Update documentation for memory_region_new*()
  hw/ide/sii3112: Use memory_region_new to avoid leaking regions
  hw/pci-host/articia: Map PCI memory windows in realize
  hw/pci-host/articia: Add variable for common type cast

 docs/devel/memory.rst   |  21 +-
 hw/ide/sii3112.c        |  30 +--
 hw/pci-host/articia.c   |  22 +-
 hw/ppc/amigaone.c       |  28 +--
 hw/ppc/pegasos.c        |  13 --
 include/system/memory.h | 360 ++++++++++++++++++++++++++++++
 system/memory.c         | 484 +++++++++++++++++++++++++---------------
 7 files changed, 713 insertions(+), 245 deletions(-)

-- 
2.41.3

On Sun, 25 Jan 2026 at 18:36, BALATON Zoltan <balaton@eik.bme.hu> wrote:
>
> Our documentation says that memory regions are automatically freed
> when the owner dies and the reference counting to do this is also
> implemented. However this relies on the QOM free funtion that can only
> be set by creating objects with object_new but memory API only
> provides constructors that call object_initialize which clears the
> free function that prevents QOM to manage the memory region lifetime.
> Implement corresponding memory_region_new_* functions that do the same
> as the memory_region_init_* functions but create the memory region
> with object_new so the lifetime can be automatically managed by QOM as
> documented. The memory_region_init functions are kept because they are
> useful for memory regions embedded in other object or managed
> externally and not by QOM for some reason.

If we have a problem with embedded MemoryRegions being leaked,
we need to fix that. I'm very reluctant to create a whole new
set of APIs which handles MRs in a non-standard way (i.e.
"QOM object has a pointer to MR rather than embedding it")
and leaves the existing ones alone. QEMU already has way too
many situations where we have multiple different APIs that can
be used to do something and incomplete transitions from the
old one to the new one.

thanks
-- PMM

On Tue, 27 Jan 2026, Peter Maydell wrote:
> On Sun, 25 Jan 2026 at 18:36, BALATON Zoltan <balaton@eik.bme.hu> wrote:
>>
>> Our documentation says that memory regions are automatically freed
>> when the owner dies and the reference counting to do this is also
>> implemented. However this relies on the QOM free funtion that can only
>> be set by creating objects with object_new but memory API only
>> provides constructors that call object_initialize which clears the
>> free function that prevents QOM to manage the memory region lifetime.
>> Implement corresponding memory_region_new_* functions that do the same
>> as the memory_region_init_* functions but create the memory region
>> with object_new so the lifetime can be automatically managed by QOM as
>> documented. The memory_region_init functions are kept because they are
>> useful for memory regions embedded in other object or managed
>> externally and not by QOM for some reason.
>
> If we have a problem with embedded MemoryRegions being leaked,
> we need to fix that.

I have a problem with embedded memory regions (and embedded objects in 
general) because:

- they are making state structs unnecessarily crowded obscuring what 
should really be there
- leaving the fields to store these memory regions in the parent classes 
and the QOM way of lifetime management unused
- exposes internal object implementation to the outside
which is against encapsulation and reuse of object oriented design.

Therefore, I consider embedded memory regions as a hack to solve leaks 
because the intended and documented way of using reference counting to 
manage the lifetime of these regions cannot be used due to missing API to 
instantiate them in a way so it works. This series tries to resolve that 
not add new principle or standard.

> I'm very reluctant to create a whole new
> set of APIs which handles MRs in a non-standard way (i.e.
> "QOM object has a pointer to MR rather than embedding it")

But this is the standard and documented way if you read the life cycle 
section of the memory docs and the embedded regions became more used 
instead of resolving this problem in the way it was originaly intended. 
The QOM subclass does not store a pointer to these as it's not needed, 
they are managed by QOM so they are created and can be forgotten about 
without littering the state struct with fields never used in the object 
methods just to manage the memory that QOM could already manage. (The 
parent sysbus or PCI class keeps a pointer when registering them as 
regions and Object stores them as a child but that's already there and 
needed for those classes not the subclass so we can avoid duplicating it 
in the subclass.)

> and leaves the existing ones alone. QEMU already has way too
> many situations where we have multiple different APIs that can
> be used to do something and incomplete transitions from the
> old one to the new one.

This series does not convert everything but opens the way to do that and 
simplify objects by only storing things in the state that are really 
needed by the object and let QOM manage what it can. I'd really like to 
avoid having a list of unused stuff in the state struct just because I 
can't use something that works and documented just no API to actually use 
it. Cf. sii3112 state after this series:

struct SiI3112PCIState {
     PCIIDEState i;

     SiI3112Regs regs[2];
};

vs. your standard:

struct SiI3112PCIState {
     PCIIDEState i;

     MemoryRegion mmio;
     MemoryRegion bar0;
     MemoryRegion bar1;
     MemoryRegion bar2;
     MemoryRegion bar3;
     MemoryRegion bar4;
     SiI3112Regs regs[2];
};

First I thought we could use object_new then memory_region_init but it 
turns out that does not work because memory_region_init calls 
object_initialize which removes the free function installed by object_new 
and since there's no other way to set it other than by calling object_new 
a split init like in macOS (i.e. alloc+init) is also not possible so a new 
set of memory_region_new functions seem to be the easiest way to use it as 
intended and avoid changing every user of memory_region_init functions.

As mentioned in the docs and cover letter both set of functions are useful 
in different situations and they are otherwise the same so it would not 
make the API much more complex as these are basically variants of one set 
of functions only differing in dynamically allocating object or 
initialising pre-allocated object that are not managed by QOM (which is 
what the embedded objects do: take object out from QOM and let the 
subclass manage it instead of using the infrastructure that's already 
there for it). If you go the embedded way then the registering memory 
regions as children should be removed and everything converted to use 
embedded regions but I don't think that would be an improvement.

Regards,
BALATON Zoltan

On Tue, Jan 27, 2026 at 03:10:28PM +0100, BALATON Zoltan wrote:
> This series does not convert everything but opens the way to do that and
> simplify objects by only storing things in the state that are really needed
> by the object and let QOM manage what it can. I'd really like to avoid
> having a list of unused stuff in the state struct just because I can't use
> something that works and documented just no API to actually use it. Cf.
> sii3112 state after this series:
> 
> struct SiI3112PCIState {
>     PCIIDEState i;
> 
>     SiI3112Regs regs[2];
> };
> 
> vs. your standard:
> 
> struct SiI3112PCIState {
>     PCIIDEState i;
> 
>     MemoryRegion mmio;
>     MemoryRegion bar0;
>     MemoryRegion bar1;
>     MemoryRegion bar2;
>     MemoryRegion bar3;
>     MemoryRegion bar4;
>     SiI3112Regs regs[2];
> };

The latter solution is fine at least to me.

I respect your preference on how it can be done, but IMHO it is subjective,
and I tend to agree with Peter that we should try to stick with one way of
solving problems, unless very necessary.

In the last three patches the MRs do not look need dynamic deallocation to
me at all..  Introducing fully dynamic MR allocations only for those is an
overkill to me.  I'm not fully convinced we need to merge 600+ new LOCs for
new APIs just for them.

We have real but rare users of dynamically allocated MRs, currently AFAIU
we're leaning towards having an object wrapping the MRs needed, when one
wants to provide a complete lifecycle for the MR so the memory backing the
MRs can be freed completely separately from the parent object.

Thanks,

-- 
Peter Xu

On Tue, 27 Jan 2026, Peter Xu wrote:
> On Tue, Jan 27, 2026 at 03:10:28PM +0100, BALATON Zoltan wrote:
>> This series does not convert everything but opens the way to do that and
>> simplify objects by only storing things in the state that are really needed
>> by the object and let QOM manage what it can. I'd really like to avoid
>> having a list of unused stuff in the state struct just because I can't use
>> something that works and documented just no API to actually use it. Cf.
>> sii3112 state after this series:
>>
>> struct SiI3112PCIState {
>>     PCIIDEState i;
>>
>>     SiI3112Regs regs[2];
>> };
>>
>> vs. your standard:
>>
>> struct SiI3112PCIState {
>>     PCIIDEState i;
>>
>>     MemoryRegion mmio;
>>     MemoryRegion bar0;
>>     MemoryRegion bar1;
>>     MemoryRegion bar2;
>>     MemoryRegion bar3;
>>     MemoryRegion bar4;
>>     SiI3112Regs regs[2];
>> };
>
> The latter solution is fine at least to me.
>
> I respect your preference on how it can be done, but IMHO it is subjective,
> and I tend to agree with Peter that we should try to stick with one way of
> solving problems, unless very necessary.
>
> In the last three patches the MRs do not look need dynamic deallocation to
> me at all..  Introducing fully dynamic MR allocations only for those is an
> overkill to me.  I'm not fully convinced we need to merge 600+ new LOCs for
> new APIs just for them.

It's not a new API in the sense that it's the same as existing 
memory_region_init functions just a variant of it to allow using QOM to 
manage the lifetime of the memory region which the current API does not 
allow. Also this series does not introduce this dynamic lifetime 
management, it's already implemented and documented just there's currently 
no way to use it. The actual change is much less, most of the lines are 
just the documentation comments. If it's a problem to increase 
documentation size I could make it much shorter not duplicating a separate 
doc comment for these just mention in the corresponding memory_region_init 
doc comment that there's also an analogous memory_region_new variant of 
it. That also shows there's no new API just two ways of using the same 
API.

> We have real but rare users of dynamically allocated MRs, currently AFAIU

Since it's already implemented why not use it if it makes the object state 
simpler? Adding additional fields that would already be managed by QOM 
seems to be unnecessary duplication and complication to me so I'd like to 
avoid that and keep the subclass simple.

Regards,
BALATON Zoltan

> we're leaning towards having an object wrapping the MRs needed, when one
> wants to provide a complete lifecycle for the MR so the memory backing the
> MRs can be freed completely separately from the parent object.
>
> Thanks,
>
>

On 2026/01/28 5:22, BALATON Zoltan wrote:
> On Tue, 27 Jan 2026, Peter Xu wrote:
>> On Tue, Jan 27, 2026 at 03:10:28PM +0100, BALATON Zoltan wrote:
>>> This series does not convert everything but opens the way to do that and
>>> simplify objects by only storing things in the state that are really 
>>> needed
>>> by the object and let QOM manage what it can. I'd really like to avoid
>>> having a list of unused stuff in the state struct just because I 
>>> can't use
>>> something that works and documented just no API to actually use it. Cf.
>>> sii3112 state after this series:
>>>
>>> struct SiI3112PCIState {
>>>     PCIIDEState i;
>>>
>>>     SiI3112Regs regs[2];
>>> };
>>>
>>> vs. your standard:
>>>
>>> struct SiI3112PCIState {
>>>     PCIIDEState i;
>>>
>>>     MemoryRegion mmio;
>>>     MemoryRegion bar0;
>>>     MemoryRegion bar1;
>>>     MemoryRegion bar2;
>>>     MemoryRegion bar3;
>>>     MemoryRegion bar4;
>>>     SiI3112Regs regs[2];
>>> };
>>
>> The latter solution is fine at least to me.
>>
>> I respect your preference on how it can be done, but IMHO it is 
>> subjective,
>> and I tend to agree with Peter that we should try to stick with one 
>> way of
>> solving problems, unless very necessary.
>>
>> In the last three patches the MRs do not look need dynamic 
>> deallocation to
>> me at all..  Introducing fully dynamic MR allocations only for those 
>> is an
>> overkill to me.  I'm not fully convinced we need to merge 600+ new 
>> LOCs for
>> new APIs just for them.
> 
> It's not a new API in the sense that it's the same as existing 
> memory_region_init functions just a variant of it to allow using QOM to 
> manage the lifetime of the memory region which the current API does not 
> allow. Also this series does not introduce this dynamic lifetime 
> management, it's already implemented and documented just there's 
> currently no way to use it. The actual change is much less, most of the 
> lines are just the documentation comments. If it's a problem to increase 
> documentation size I could make it much shorter not duplicating a 
> separate doc comment for these just mention in the corresponding 
> memory_region_init doc comment that there's also an analogous 
> memory_region_new variant of it. That also shows there's no new API just 
> two ways of using the same API.
> 
>> We have real but rare users of dynamically allocated MRs, currently AFAIU
> 
> Since it's already implemented why not use it if it makes the object 
> state simpler? Adding additional fields that would already be managed by 
> QOM seems to be unnecessary duplication and complication to me so I'd 
> like to avoid that and keep the subclass simple.

I feel the discussion is going a wrong direction. This thread and the 
cover letter talks different things as the motivation of this series; 
The cover letter says the documentation mismatches with the 
implementation and that is the motivation for this series. This thread 
talks about duplicate memory management and extra fields, which is not 
mentioned in the cover letter.

Besides, the "extra fields" problem should have been already discussed 
in [1] and I don't see a new argument here. So, in summary, for a 
constructive discussion I think:
- There should be a consistent goal of discussion or patch series
   (documentation mismatch or extra fields) and
- A new point needs to be made; reminding the points of past discussion
   may be worthwhile but is insufficient.

[1] 
https://lore.kernel.org/qemu-devel/ceda4c28887c40e1c8eae3f561ee381ca98b0484.1761346145.git.balaton@eik.bme.hu/

Regards,
Akihiko Odaki

On Wed, 28 Jan 2026, Akihiko Odaki wrote:
> On 2026/01/28 5:22, BALATON Zoltan wrote:
>> On Tue, 27 Jan 2026, Peter Xu wrote:
>>> On Tue, Jan 27, 2026 at 03:10:28PM +0100, BALATON Zoltan wrote:
>>>> This series does not convert everything but opens the way to do that and
>>>> simplify objects by only storing things in the state that are really 
>>>> needed
>>>> by the object and let QOM manage what it can. I'd really like to avoid
>>>> having a list of unused stuff in the state struct just because I can't 
>>>> use
>>>> something that works and documented just no API to actually use it. Cf.
>>>> sii3112 state after this series:
>>>> 
>>>> struct SiI3112PCIState {
>>>>     PCIIDEState i;
>>>> 
>>>>     SiI3112Regs regs[2];
>>>> };
>>>> 
>>>> vs. your standard:
>>>> 
>>>> struct SiI3112PCIState {
>>>>     PCIIDEState i;
>>>> 
>>>>     MemoryRegion mmio;
>>>>     MemoryRegion bar0;
>>>>     MemoryRegion bar1;
>>>>     MemoryRegion bar2;
>>>>     MemoryRegion bar3;
>>>>     MemoryRegion bar4;
>>>>     SiI3112Regs regs[2];
>>>> };
>>> 
>>> The latter solution is fine at least to me.
>>> 
>>> I respect your preference on how it can be done, but IMHO it is 
>>> subjective,
>>> and I tend to agree with Peter that we should try to stick with one way of
>>> solving problems, unless very necessary.
>>> 
>>> In the last three patches the MRs do not look need dynamic deallocation to
>>> me at all..  Introducing fully dynamic MR allocations only for those is an
>>> overkill to me.  I'm not fully convinced we need to merge 600+ new LOCs 
>>> for
>>> new APIs just for them.
>> 
>> It's not a new API in the sense that it's the same as existing 
>> memory_region_init functions just a variant of it to allow using QOM to 
>> manage the lifetime of the memory region which the current API does not 
>> allow. Also this series does not introduce this dynamic lifetime 
>> management, it's already implemented and documented just there's currently 
>> no way to use it. The actual change is much less, most of the lines are 
>> just the documentation comments. If it's a problem to increase 
>> documentation size I could make it much shorter not duplicating a separate 
>> doc comment for these just mention in the corresponding memory_region_init 
>> doc comment that there's also an analogous memory_region_new variant of it. 
>> That also shows there's no new API just two ways of using the same API.
>> 
>>> We have real but rare users of dynamically allocated MRs, currently AFAIU
>> 
>> Since it's already implemented why not use it if it makes the object state 
>> simpler? Adding additional fields that would already be managed by QOM 
>> seems to be unnecessary duplication and complication to me so I'd like to 
>> avoid that and keep the subclass simple.
>
> I feel the discussion is going a wrong direction. This thread and the cover 
> letter talks different things as the motivation of this series; The cover 
> letter says the documentation mismatches with the implementation and that is 
> the motivation for this series. This thread talks about duplicate memory 
> management and extra fields, which is not mentioned in the cover letter.
>
> Besides, the "extra fields" problem should have been already discussed in [1] 
> and I don't see a new argument here. So, in summary, for a constructive 
> discussion I think:
> - There should be a consistent goal of discussion or patch series
>  (documentation mismatch or extra fields) and
> - A new point needs to be made; reminding the points of past discussion
>  may be worthwhile but is insufficient.
>
> [1] 
> https://lore.kernel.org/qemu-devel/ceda4c28887c40e1c8eae3f561ee381ca98b0484.1761346145.git.balaton@eik.bme.hu/

Adding some QOM people in case they have something to add.

OK I try to summarise the motivation again:

1. Documentation in docs/devel/memory.rst says that memory regions' 
lifecycle is managed by QOM and they are freed with their owner or when 
nothing else uses them. This is also already implemented for a long time 
as described but cannot be used because the only constructors available 
kill this feature when calling object_initialize that clears the free 
function added by object_new. (The life time management is implemented 
through adding memory regions as children to the owner and unparenting 
them on freeing the owner which decreases ref count of the memory region 
and will free it when nothing else references it as far as I can tell.)

2. This also allows to keep subclasses' state simpler as demonstrated by 
the last two patches by allowing QOM to manage memory regions as 
originally intended (based on the documentation and implementation) and 
not needing to embed memory regions not otherwise needed in the subclass 
as they are already handled by super classes (e.g. registering as PCI 
regions or sysbus mmio regions and their lifetime managed by QOM; this way 
the subclass only contains what it has to add to the superclass and kept 
simpler as expected in an object oriented design).

So the motivation is to match documentation and allow using already 
implemented memory region lifecycle management to make device models 
simpler.

These are my motivation for this change. What is the motivation for using 
embedded memory regions instead and against this change? If that is found 
to be a better way then the lifecycle management as implementated and 
documentated is likely no longer needed as it cannot and will not be used 
with embedded memory regions that are initialsed with object_initialize 
and thus could be removed. But I think that would lead to more complex 
device models not using what QOM and QDev was meant to help with. (More 
complex device models is a problem because they are harder to understand 
and find bugs in them having additional things that disctract from actual 
functionality of the subclass so I'd like to make them as simple as 
possible only containing things that are related to the subclass. So 
less QOM boilet plate, less additional static fields in the state is the 
better.)

Regards,
BALATON Zoltan

On Wed, 28 Jan 2026 at 11:40, BALATON Zoltan <balaton@eik.bme.hu> wrote:
> OK I try to summarise the motivation again:
>
> 1. Documentation in docs/devel/memory.rst says that memory regions'
> lifecycle is managed by QOM and they are freed with their owner or when
> nothing else uses them. This is also already implemented for a long time
> as described but cannot be used because the only constructors available
> kill this feature when calling object_initialize that clears the free
> function added by object_new. (The life time management is implemented
> through adding memory regions as children to the owner and unparenting
> them on freeing the owner which decreases ref count of the memory region
> and will free it when nothing else references it as far as I can tell.)

If we have leaks because of our very common pattern of "embed
a MemoryRegion struct in the device state struct" then we must
fix those, because there's no way we're going to convert all
that existing code to a new set of APIs. But I was under the
impression we had already dealt with those, because MRs track
their owner's refcount, and don't have their own independent one ?

> These are my motivation for this change. What is the motivation for using
> embedded memory regions instead and against this change?

Simply that it's a consistent pattern we use in a lot of the codebase:
the device embeds a lot of the structs it uses, rather than allocating
memory for them and keeping pointers to that allocated memory. We
still have also various older device models that use the previous
pattern of "allocate memory and have pointers" too, but most new
code doesn't do that. I think we should for preference write code
in one pattern, not two, and "embed structs" seems to be what
we have mostly settled on for new code.

There is an argument to be made that the pointer model would
fit better with a possible future world of "the user can wire
configurably wire up their own board model from devices", and
that it works better in a part-Rust-part-C world where the two
different languages don't have convenient access to the exact
size of structs defined in the other language. But that future
model is not something anybody has yet really fleshed out in any
detail, so it's still a bit speculative.

I'm not actually opposed to the idea of making a design decision
that this struct-embedding is no longer what we want to do, and defining
that something else is our new best practice for how to write devices.
But I think we would need to start by reaching a consensus that that
*is* what we want to do, and documenting that "best practice" somewhere
in docs/devel/. Then we can examine proposed new APIs and all be
on the same page about the design patterns we want and it will
be clearer to reviewers whether the new APIs fit into those
patterns or not.

thanks
-- PMM

On Wed, 28 Jan 2026, Peter Maydell wrote:
> On Wed, 28 Jan 2026 at 11:40, BALATON Zoltan <balaton@eik.bme.hu> wrote:
>> OK I try to summarise the motivation again:
>>
>> 1. Documentation in docs/devel/memory.rst says that memory regions'
>> lifecycle is managed by QOM and they are freed with their owner or when
>> nothing else uses them. This is also already implemented for a long time
>> as described but cannot be used because the only constructors available
>> kill this feature when calling object_initialize that clears the free
>> function added by object_new. (The life time management is implemented
>> through adding memory regions as children to the owner and unparenting
>> them on freeing the owner which decreases ref count of the memory region
>> and will free it when nothing else references it as far as I can tell.)
>
> If we have leaks because of our very common pattern of "embed
> a MemoryRegion struct in the device state struct" then we must
> fix those, because there's no way we're going to convert all
> that existing code to a new set of APIs. But I was under the
> impression we had already dealt with those, because MRs track
> their owner's refcount, and don't have their own independent one ?

I'm not sure if all those leaks are resolved as there were some patches 
and discussion about this recently but I think that problem or the need to 
use the owner's ref count to circumwent it instead of using the memory 
region's own ref count may also come from that there's currently no way to 
allocate memory regions that are ref counted and automatically freed as it 
should work with QOM and the documentation implies. (Only the constuctor 
is missing that is all this series adds, the mechanism is already there 
and implemented.) There may still be a problem with circular references if 
the memory region needs the owner so the owner can't be freed until the 
memory region is also freed but the memory region is not freed until the 
owner is freed but if both the owner and memory region used their own ref 
count things may become a bit less confusing and could be easier to find a 
way to break circular reference (e.g. by owner unparent child regions on 
unrealize but isn't freed until memory regions unref owner in their free 
method).

>> These are my motivation for this change. What is the motivation for using
>> embedded memory regions instead and against this change?
>
> Simply that it's a consistent pattern we use in a lot of the codebase:
> the device embeds a lot of the structs it uses, rather than allocating
> memory for them and keeping pointers to that allocated memory. We

You mix in the issue of SoCs and complex devices using other devices in 
which case the recommendation was to embed those in the parent device so 
they don't have to be freed or kept track of by a pointer but won't be 
leaked. This series does not mean to change that, it's only limited to 
memory regions. (Although that problem may also stem from similar issue 
with object_initialize_child not allowing creating reference counted 
objects only initializing preallocated instances but that's not 
something this series touches.)

We can say that memory regions are like other embedded objects but they 
are often used for sysbus and PCI devices only to be registered in the 
parent device that already has pointers in their state to track these so 
there's no need to keep track of them in the subclass if we can rely on 
QOM freeing them when not needed any more and this is already implemented 
and documented that way. So even if we keep embedding other child devices 
into complex parent devices that I think does not directly apply to memory 
regions and we could use what the documentation and implementation 
already allows and says for memory regions at least.

> still have also various older device models that use the previous
> pattern of "allocate memory and have pointers" too, but most new
> code doesn't do that. I think we should for preference write code
> in one pattern, not two, and "embed structs" seems to be what
> we have mostly settled on for new code.
>
> There is an argument to be made that the pointer model would
> fit better with a possible future world of "the user can wire
> configurably wire up their own board model from devices", and
> that it works better in a part-Rust-part-C world where the two
> different languages don't have convenient access to the exact
> size of structs defined in the other language. But that future
> model is not something anybody has yet really fleshed out in any
> detail, so it's still a bit speculative.

You keep mentioning pointers but the point of ref counts and regisrering 
memory region as child of an owner is to avoid needing a pointer or 
embedding it in the subclass state as the relationship and lifecycle 
management are then handled by QOM. If we don't use that we could remove 
this from QOM and memory regions to simplify it but if it's already there 
and makes the device state simpler I think we better use it.

> I'm not actually opposed to the idea of making a design decision
> that this struct-embedding is no longer what we want to do, and defining
> that something else is our new best practice for how to write devices.
> But I think we would need to start by reaching a consensus that that
> *is* what we want to do, and documenting that "best practice" somewhere
> in docs/devel/. Then we can examine proposed new APIs and all be
> on the same page about the design patterns we want and it will
> be clearer to reviewers whether the new APIs fit into those
> patterns or not.

I think we're in that discussion now in this thread. I don't propose to 
change the struct-embedding for sub devices used in SoC or south bridge or 
other complex devices but only propose to not embed memory regions that 
are already documented as and handled by QOM and simply allocate them and 
let QOM handle them so we only need to reference them in the devices state 
unless they are needed for some reason by the device methods which is 
rarely the case. So this is limited to memory regions and the series only 
seems to add a lot of lines because of the extensive documentation 
comments. The actual change is just factoring out actual memory region 
init from memory_region_init functions then add a memory_region_new 
variant that does object_new; do_init and keep the memory_region_init do 
object_initializel do_init. Nothing else is changed, the way to manage and 
free regions based on ref counting is already there this series just 
enables them to be actually used becase currently despite what the docs 
say memory regions are either leaked or must be embedded.

Regards,
BALATON Zoltan

On 2026/01/29 0:46, BALATON Zoltan wrote:
> On Wed, 28 Jan 2026, Peter Maydell wrote:
>> On Wed, 28 Jan 2026 at 11:40, BALATON Zoltan <balaton@eik.bme.hu> wrote:
>>> OK I try to summarise the motivation again:
>>>
>>> 1. Documentation in docs/devel/memory.rst says that memory regions'
>>> lifecycle is managed by QOM and they are freed with their owner or when
>>> nothing else uses them. This is also already implemented for a long time
>>> as described but cannot be used because the only constructors available
>>> kill this feature when calling object_initialize that clears the free
>>> function added by object_new. (The life time management is implemented
>>> through adding memory regions as children to the owner and unparenting
>>> them on freeing the owner which decreases ref count of the memory region
>>> and will free it when nothing else references it as far as I can tell.)
>>
>> If we have leaks because of our very common pattern of "embed
>> a MemoryRegion struct in the device state struct" then we must
>> fix those, because there's no way we're going to convert all
>> that existing code to a new set of APIs. But I was under the
>> impression we had already dealt with those, because MRs track
>> their owner's refcount, and don't have their own independent one ?
> 
> I'm not sure if all those leaks are resolved as there were some patches 
> and discussion about this recently but I think that problem or the need 
> to use the owner's ref count to circumwent it instead of using the 
> memory region's own ref count may also come from that there's currently 
> no way to allocate memory regions that are ref counted and automatically 
> freed as it should work with QOM and the documentation implies. (Only 
> the constuctor is missing that is all this series adds, the mechanism is 
> already there and implemented.) There may still be a problem with 
> circular references if the memory region needs the owner so the owner 
> can't be freed until the memory region is also freed but the memory 
> region is not freed until the owner is freed but if both the owner and 
> memory region used their own ref count things may become a bit less 
> confusing and could be easier to find a way to break circular reference 
> (e.g. by owner unparent child regions on unrealize but isn't freed until 
> memory regions unref owner in their free method).
> 
>>> These are my motivation for this change. What is the motivation for 
>>> using
>>> embedded memory regions instead and against this change?
>>
>> Simply that it's a consistent pattern we use in a lot of the codebase:
>> the device embeds a lot of the structs it uses, rather than allocating
>> memory for them and keeping pointers to that allocated memory. We
> 
> You mix in the issue of SoCs and complex devices using other devices in 
> which case the recommendation was to embed those in the parent device so 
> they don't have to be freed or kept track of by a pointer but won't be 
> leaked. This series does not mean to change that, it's only limited to 
> memory regions. (Although that problem may also stem from similar issue 
> with object_initialize_child not allowing creating reference counted 
> objects only initializing preallocated instances but that's not 
> something this series touches.)
> 
> We can say that memory regions are like other embedded objects but they 
> are often used for sysbus and PCI devices only to be registered in the 
> parent device that already has pointers in their state to track these so 
> there's no need to keep track of them in the subclass if we can rely on 
> QOM freeing them when not needed any more and this is already 
> implemented and documented that way. So even if we keep embedding other 
> child devices into complex parent devices that I think does not directly 
> apply to memory regions and we could use what the documentation and 
> implementation already allows and says for memory regions at least.
> 
>> still have also various older device models that use the previous
>> pattern of "allocate memory and have pointers" too, but most new
>> code doesn't do that. I think we should for preference write code
>> in one pattern, not two, and "embed structs" seems to be what
>> we have mostly settled on for new code.
>>
>> There is an argument to be made that the pointer model would
>> fit better with a possible future world of "the user can wire
>> configurably wire up their own board model from devices", and
>> that it works better in a part-Rust-part-C world where the two
>> different languages don't have convenient access to the exact
>> size of structs defined in the other language. But that future
>> model is not something anybody has yet really fleshed out in any
>> detail, so it's still a bit speculative.
> 
> You keep mentioning pointers but the point of ref counts and regisrering 
> memory region as child of an owner is to avoid needing a pointer or 
> embedding it in the subclass state as the relationship and lifecycle 
> management are then handled by QOM. If we don't use that we could remove 
> this from QOM and memory regions to simplify it but if it's already 
> there and makes the device state simpler I think we better use it.
> 
>> I'm not actually opposed to the idea of making a design decision
>> that this struct-embedding is no longer what we want to do, and defining
>> that something else is our new best practice for how to write devices.
>> But I think we would need to start by reaching a consensus that that
>> *is* what we want to do, and documenting that "best practice" somewhere
>> in docs/devel/. Then we can examine proposed new APIs and all be
>> on the same page about the design patterns we want and it will
>> be clearer to reviewers whether the new APIs fit into those
>> patterns or not.
> 
> I think we're in that discussion now in this thread. I don't propose to 
> change the struct-embedding for sub devices used in SoC or south bridge 
> or other complex devices but only propose to not embed memory regions 
> that are already documented as and handled by QOM and simply allocate 
> them and let QOM handle them so we only need to reference them in the 
> devices state unless they are needed for some reason by the device 
> methods which is rarely the case. So this is limited to memory regions 
> and the series only seems to add a lot of lines because of the extensive 
> documentation comments. The actual change is just factoring out actual 
> memory region init from memory_region_init functions then add a 
> memory_region_new variant that does object_new; do_init and keep the 
> memory_region_init do object_initializel do_init. Nothing else is 
> changed, the way to manage and free regions based on ref counting is 
> already there this series just enables them to be actually used becase 
> currently despite what the docs say memory regions are either leaked or 
> must be embedded.

I actually think deprecating struct-embedding for all QOM objects is a 
good idea.

The problem initially stated in this thread is that embedding requires 
having extra field, but people see the benefit is too small. There is no 
real logic involved in having such fields so it does not reduce code 
complexity much; it saves some lines and that's it.

However, I see another problem in struct embedding; it breaks 
object_ref(). When embedding, the child object effectively takes the 
reference to the storage of the parent object, but this reference is not 
counted, so use-after-free can happen if someone takes a reference to 
the child object with object_ref(). That is why the wrapper of 
object_ref() in rust/qom/src/qom.rs needs to be marked unsafe. Memory 
regions workaround this with memory_region_ref(), but it's not perfect 
since it relies on object_ref() in the end.

For this reason I think object_initialize(), object_initialize_child(), 
and the like are better to be noted as deprecated in
include/qom/object.h. Then memory_region_init() can be deprecated 
referring to them.

Regards,
Akihiko Odaki

On 29/01/2026 04:41, Akihiko Odaki wrote:

> On 2026/01/29 0:46, BALATON Zoltan wrote:
>> On Wed, 28 Jan 2026, Peter Maydell wrote:
>>> On Wed, 28 Jan 2026 at 11:40, BALATON Zoltan <balaton@eik.bme.hu> wrote:
>>>> OK I try to summarise the motivation again:
>>>>
>>>> 1. Documentation in docs/devel/memory.rst says that memory regions'
>>>> lifecycle is managed by QOM and they are freed with their owner or when
>>>> nothing else uses them. This is also already implemented for a long time
>>>> as described but cannot be used because the only constructors available
>>>> kill this feature when calling object_initialize that clears the free
>>>> function added by object_new. (The life time management is implemented
>>>> through adding memory regions as children to the owner and unparenting
>>>> them on freeing the owner which decreases ref count of the memory region
>>>> and will free it when nothing else references it as far as I can tell.)
>>>
>>> If we have leaks because of our very common pattern of "embed
>>> a MemoryRegion struct in the device state struct" then we must
>>> fix those, because there's no way we're going to convert all
>>> that existing code to a new set of APIs. But I was under the
>>> impression we had already dealt with those, because MRs track
>>> their owner's refcount, and don't have their own independent one ?
>>
>> I'm not sure if all those leaks are resolved as there were some patches and 
>> discussion about this recently but I think that problem or the need to use the 
>> owner's ref count to circumwent it instead of using the memory region's own ref 
>> count may also come from that there's currently no way to allocate memory regions 
>> that are ref counted and automatically freed as it should work with QOM and the 
>> documentation implies. (Only the constuctor is missing that is all this series 
>> adds, the mechanism is already there and implemented.) There may still be a problem 
>> with circular references if the memory region needs the owner so the owner can't be 
>> freed until the memory region is also freed but the memory region is not freed 
>> until the owner is freed but if both the owner and memory region used their own ref 
>> count things may become a bit less confusing and could be easier to find a way to 
>> break circular reference (e.g. by owner unparent child regions on unrealize but 
>> isn't freed until memory regions unref owner in their free method).
>>
>>>> These are my motivation for this change. What is the motivation for using
>>>> embedded memory regions instead and against this change?
>>>
>>> Simply that it's a consistent pattern we use in a lot of the codebase:
>>> the device embeds a lot of the structs it uses, rather than allocating
>>> memory for them and keeping pointers to that allocated memory. We
>>
>> You mix in the issue of SoCs and complex devices using other devices in which case 
>> the recommendation was to embed those in the parent device so they don't have to be 
>> freed or kept track of by a pointer but won't be leaked. This series does not mean 
>> to change that, it's only limited to memory regions. (Although that problem may 
>> also stem from similar issue with object_initialize_child not allowing creating 
>> reference counted objects only initializing preallocated instances but that's not 
>> something this series touches.)
>>
>> We can say that memory regions are like other embedded objects but they are often 
>> used for sysbus and PCI devices only to be registered in the parent device that 
>> already has pointers in their state to track these so there's no need to keep track 
>> of them in the subclass if we can rely on QOM freeing them when not needed any more 
>> and this is already implemented and documented that way. So even if we keep 
>> embedding other child devices into complex parent devices that I think does not 
>> directly apply to memory regions and we could use what the documentation and 
>> implementation already allows and says for memory regions at least.
>>
>>> still have also various older device models that use the previous
>>> pattern of "allocate memory and have pointers" too, but most new
>>> code doesn't do that. I think we should for preference write code
>>> in one pattern, not two, and "embed structs" seems to be what
>>> we have mostly settled on for new code.
>>>
>>> There is an argument to be made that the pointer model would
>>> fit better with a possible future world of "the user can wire
>>> configurably wire up their own board model from devices", and
>>> that it works better in a part-Rust-part-C world where the two
>>> different languages don't have convenient access to the exact
>>> size of structs defined in the other language. But that future
>>> model is not something anybody has yet really fleshed out in any
>>> detail, so it's still a bit speculative.
>>
>> You keep mentioning pointers but the point of ref counts and regisrering memory 
>> region as child of an owner is to avoid needing a pointer or embedding it in the 
>> subclass state as the relationship and lifecycle management are then handled by 
>> QOM. If we don't use that we could remove this from QOM and memory regions to 
>> simplify it but if it's already there and makes the device state simpler I think we 
>> better use it.
>>
>>> I'm not actually opposed to the idea of making a design decision
>>> that this struct-embedding is no longer what we want to do, and defining
>>> that something else is our new best practice for how to write devices.
>>> But I think we would need to start by reaching a consensus that that
>>> *is* what we want to do, and documenting that "best practice" somewhere
>>> in docs/devel/. Then we can examine proposed new APIs and all be
>>> on the same page about the design patterns we want and it will
>>> be clearer to reviewers whether the new APIs fit into those
>>> patterns or not.
>>
>> I think we're in that discussion now in this thread. I don't propose to change the 
>> struct-embedding for sub devices used in SoC or south bridge or other complex 
>> devices but only propose to not embed memory regions that are already documented as 
>> and handled by QOM and simply allocate them and let QOM handle them so we only need 
>> to reference them in the devices state unless they are needed for some reason by 
>> the device methods which is rarely the case. So this is limited to memory regions 
>> and the series only seems to add a lot of lines because of the extensive 
>> documentation comments. The actual change is just factoring out actual memory 
>> region init from memory_region_init functions then add a memory_region_new variant 
>> that does object_new; do_init and keep the memory_region_init do object_initializel 
>> do_init. Nothing else is changed, the way to manage and free regions based on ref 
>> counting is already there this series just enables them to be actually used becase 
>> currently despite what the docs say memory regions are either leaked or must be 
>> embedded.
> 
> I actually think deprecating struct-embedding for all QOM objects is a good idea.
> 
> The problem initially stated in this thread is that embedding requires having extra 
> field, but people see the benefit is too small. There is no real logic involved in 
> having such fields so it does not reduce code complexity much; it saves some lines 
> and that's it.
> 
> However, I see another problem in struct embedding; it breaks object_ref(). When 
> embedding, the child object effectively takes the reference to the storage of the 
> parent object, but this reference is not counted, so use-after-free can happen if 
> someone takes a reference to the child object with object_ref(). That is why the 
> wrapper of object_ref() in rust/qom/src/qom.rs needs to be marked unsafe. Memory 
> regions workaround this with memory_region_ref(), but it's not perfect since it 
> relies on object_ref() in the end.
> 
> For this reason I think object_initialize(), object_initialize_child(), and the like 
> are better to be noted as deprecated in
> include/qom/object.h. Then memory_region_init() can be deprecated referring to them.

FWIW this is something I've been thinking about for some time: possibly I had a chat 
with Phil about it at some point? Once example could be if you want to have a 
reference to a parent type like PCIDevice that can change at runtime e.g. it could be 
PCINE2000State or PCIPCNetState then you can never embed it, because you don't know 
the size at compile time. So then why not use object_property_add_child() everywhere 
so there is just a single way of doing things?

For memory regions it's a bit trickier because as per the virtio-gpu issues you've 
been looking at, it is possible for the memory region to exist outside of its parent 
device until it is destroyed later by the RCU thread. Is this something that can be 
solved by manipulating the refcount?

I do agree with Peter too that we don't want to add yet another way of doing things: 
if we decide to change the way memory regions work, we should go all-in and update 
all callers to reflect the new API and remove the old one. Having one easily 
understood way of modelling things makes life much easier for contributors and 
reviewers alike.


ATB,

Mark.

On Fri, 30 Jan 2026, Mark Cave-Ayland wrote:
> On 29/01/2026 04:41, Akihiko Odaki wrote:
>> On 2026/01/29 0:46, BALATON Zoltan wrote:
>>> On Wed, 28 Jan 2026, Peter Maydell wrote:
>>>> On Wed, 28 Jan 2026 at 11:40, BALATON Zoltan <balaton@eik.bme.hu> wrote:
>>>>> OK I try to summarise the motivation again:
>>>>> 
>>>>> 1. Documentation in docs/devel/memory.rst says that memory regions'
>>>>> lifecycle is managed by QOM and they are freed with their owner or when
>>>>> nothing else uses them. This is also already implemented for a long time
>>>>> as described but cannot be used because the only constructors available
>>>>> kill this feature when calling object_initialize that clears the free
>>>>> function added by object_new. (The life time management is implemented
>>>>> through adding memory regions as children to the owner and unparenting
>>>>> them on freeing the owner which decreases ref count of the memory region
>>>>> and will free it when nothing else references it as far as I can tell.)
>>>> 
>>>> If we have leaks because of our very common pattern of "embed
>>>> a MemoryRegion struct in the device state struct" then we must
>>>> fix those, because there's no way we're going to convert all
>>>> that existing code to a new set of APIs. But I was under the
>>>> impression we had already dealt with those, because MRs track
>>>> their owner's refcount, and don't have their own independent one ?
>>> 
>>> I'm not sure if all those leaks are resolved as there were some patches 
>>> and discussion about this recently but I think that problem or the need to 
>>> use the owner's ref count to circumwent it instead of using the memory 
>>> region's own ref count may also come from that there's currently no way to 
>>> allocate memory regions that are ref counted and automatically freed as it 
>>> should work with QOM and the documentation implies. (Only the constuctor 
>>> is missing that is all this series adds, the mechanism is already there 
>>> and implemented.) There may still be a problem with circular references if 
>>> the memory region needs the owner so the owner can't be freed until the 
>>> memory region is also freed but the memory region is not freed until the 
>>> owner is freed but if both the owner and memory region used their own ref 
>>> count things may become a bit less confusing and could be easier to find a 
>>> way to break circular reference (e.g. by owner unparent child regions on 
>>> unrealize but isn't freed until memory regions unref owner in their free 
>>> method).
>>> 
>>>>> These are my motivation for this change. What is the motivation for 
>>>>> using
>>>>> embedded memory regions instead and against this change?
>>>> 
>>>> Simply that it's a consistent pattern we use in a lot of the codebase:
>>>> the device embeds a lot of the structs it uses, rather than allocating
>>>> memory for them and keeping pointers to that allocated memory. We
>>> 
>>> You mix in the issue of SoCs and complex devices using other devices in 
>>> which case the recommendation was to embed those in the parent device so 
>>> they don't have to be freed or kept track of by a pointer but won't be 
>>> leaked. This series does not mean to change that, it's only limited to 
>>> memory regions. (Although that problem may also stem from similar issue 
>>> with object_initialize_child not allowing creating reference counted 
>>> objects only initializing preallocated instances but that's not something 
>>> this series touches.)
>>> 
>>> We can say that memory regions are like other embedded objects but they 
>>> are often used for sysbus and PCI devices only to be registered in the 
>>> parent device that already has pointers in their state to track these so 
>>> there's no need to keep track of them in the subclass if we can rely on 
>>> QOM freeing them when not needed any more and this is already implemented 
>>> and documented that way. So even if we keep embedding other child devices 
>>> into complex parent devices that I think does not directly apply to memory 
>>> regions and we could use what the documentation and implementation already 
>>> allows and says for memory regions at least.
>>> 
>>>> still have also various older device models that use the previous
>>>> pattern of "allocate memory and have pointers" too, but most new
>>>> code doesn't do that. I think we should for preference write code
>>>> in one pattern, not two, and "embed structs" seems to be what
>>>> we have mostly settled on for new code.
>>>> 
>>>> There is an argument to be made that the pointer model would
>>>> fit better with a possible future world of "the user can wire
>>>> configurably wire up their own board model from devices", and
>>>> that it works better in a part-Rust-part-C world where the two
>>>> different languages don't have convenient access to the exact
>>>> size of structs defined in the other language. But that future
>>>> model is not something anybody has yet really fleshed out in any
>>>> detail, so it's still a bit speculative.
>>> 
>>> You keep mentioning pointers but the point of ref counts and regisrering 
>>> memory region as child of an owner is to avoid needing a pointer or 
>>> embedding it in the subclass state as the relationship and lifecycle 
>>> management are then handled by QOM. If we don't use that we could remove 
>>> this from QOM and memory regions to simplify it but if it's already there 
>>> and makes the device state simpler I think we better use it.
>>> 
>>>> I'm not actually opposed to the idea of making a design decision
>>>> that this struct-embedding is no longer what we want to do, and defining
>>>> that something else is our new best practice for how to write devices.
>>>> But I think we would need to start by reaching a consensus that that
>>>> *is* what we want to do, and documenting that "best practice" somewhere
>>>> in docs/devel/. Then we can examine proposed new APIs and all be
>>>> on the same page about the design patterns we want and it will
>>>> be clearer to reviewers whether the new APIs fit into those
>>>> patterns or not.
>>> 
>>> I think we're in that discussion now in this thread. I don't propose to 
>>> change the struct-embedding for sub devices used in SoC or south bridge or 
>>> other complex devices but only propose to not embed memory regions that 
>>> are already documented as and handled by QOM and simply allocate them and 
>>> let QOM handle them so we only need to reference them in the devices state 
>>> unless they are needed for some reason by the device methods which is 
>>> rarely the case. So this is limited to memory regions and the series only 
>>> seems to add a lot of lines because of the extensive documentation 
>>> comments. The actual change is just factoring out actual memory region 
>>> init from memory_region_init functions then add a memory_region_new 
>>> variant that does object_new; do_init and keep the memory_region_init do 
>>> object_initializel do_init. Nothing else is changed, the way to manage and 
>>> free regions based on ref counting is already there this series just 
>>> enables them to be actually used becase currently despite what the docs 
>>> say memory regions are either leaked or must be embedded.
>> 
>> I actually think deprecating struct-embedding for all QOM objects is a good 
>> idea.
>> 
>> The problem initially stated in this thread is that embedding requires 
>> having extra field, but people see the benefit is too small. There is no 
>> real logic involved in having such fields so it does not reduce code 
>> complexity much; it saves some lines and that's it.
>> 
>> However, I see another problem in struct embedding; it breaks object_ref(). 
>> When embedding, the child object effectively takes the reference to the 
>> storage of the parent object, but this reference is not counted, so 
>> use-after-free can happen if someone takes a reference to the child object 
>> with object_ref(). That is why the wrapper of object_ref() in 
>> rust/qom/src/qom.rs needs to be marked unsafe. Memory regions workaround 
>> this with memory_region_ref(), but it's not perfect since it relies on 
>> object_ref() in the end.
>> 
>> For this reason I think object_initialize(), object_initialize_child(), and 
>> the like are better to be noted as deprecated in
>> include/qom/object.h. Then memory_region_init() can be deprecated referring 
>> to them.
>
> FWIW this is something I've been thinking about for some time: possibly I had 
> a chat with Phil about it at some point? Once example could be if you want to 
> have a reference to a parent type like PCIDevice that can change at runtime 
> e.g. it could be PCINE2000State or PCIPCNetState then you can never embed it, 
> because you don't know the size at compile time. So then why not use 
> object_property_add_child() everywhere so there is just a single way of doing 
> things?
>
> For memory regions it's a bit trickier because as per the virtio-gpu issues 
> you've been looking at, it is possible for the memory region to exist outside 
> of its parent device until it is destroyed later by the RCU thread. Is this 
> something that can be solved by manipulating the refcount?

I think if memory regions could use their own ref count instead of their 
parents' this could become simpler and these memory_region_new functions 
allow us to create memory regions that are ref counted and free'd by QOM 
so this could help but I don't know the details of that problem.

> I do agree with Peter too that we don't want to add yet another way of doing 
> things: if we decide to change the way memory regions work, we should go 
> all-in and update all callers to reflect the new API and remove the old one. 
> Having one easily understood way of modelling things makes life much easier 
> for contributors and reviewers alike.

That can be a goal but this would happen gradually and not all in this 
series. I want to use memory_region_new as much as I can in devices I 
maintain and a few more I come across but I certainly won't change every 
device and that should not be required as it's not feasible for me to 
change stuff I don't even know anything about and do all that in my free 
time.

I'm not proposing a completely new API and don't change how memory regions 
work. All I propose is to allow QOM to manage memory regions as is already 
documented by adding memory_region_new functions corresponding to 
memory_region_init functions that work exactly the same with the only 
difference that memory regions created with memory_region_init have to be 
managed by the caller while memory regions allocated with 
memory_region_new is managed by QOM and will be automatically freed when 
the last reference goes away. That's all, this should not be that hard to 
understand for contributors and reviewers.

Regards,
BALATON Zoltan

On 1/29/26 05:41, Akihiko Odaki wrote:
> However, I see another problem in struct embedding; it breaks 
> object_ref(). When embedding, the child object effectively takes the 
> reference to the storage of the parent object, but this reference is not 
> counted, so use-after-free can happen if someone takes a reference to 
> the child object with object_ref(). That is why the wrapper of 
> object_ref() in rust/qom/src/qom.rs needs to be marked unsafe. Memory 
> regions workaround this with memory_region_ref(), but it's not perfect 
> since it relies on object_ref() in the end.

Yes, and in Rust the idea was to have (in addition to Owned<T> which is 
for an allocated object) another smart pointer Child<'a, T>: an embedded 
object that is owned (the parent has a reference and a field of type 
Child releases that reference when the parent is finalized) but cannot 
be cloned.

> For this reason I think object_initialize(), object_initialize_child(), 
> and the like are better to be noted as deprecated in
> include/qom/object.h. Then memory_region_init() can be deprecated 
> referring to them.

This would be huge and I don't think it's feasible.

It also only provides the appearance of safety.  If you have a backwards 
pointer (such as the memory region's owner), you still have either a 
leak or the risk of a use-after-free.

Paolo

On 2026/01/30 3:06, Paolo Bonzini wrote:
> On 1/29/26 05:41, Akihiko Odaki wrote:
>> However, I see another problem in struct embedding; it breaks 
>> object_ref(). When embedding, the child object effectively takes the 
>> reference to the storage of the parent object, but this reference is 
>> not counted, so use-after-free can happen if someone takes a reference 
>> to the child object with object_ref(). That is why the wrapper of 
>> object_ref() in rust/qom/src/qom.rs needs to be marked unsafe. Memory 
>> regions workaround this with memory_region_ref(), but it's not perfect 
>> since it relies on object_ref() in the end.
> 
> Yes, and in Rust the idea was to have (in addition to Owned<T> which is 
> for an allocated object) another smart pointer Child<'a, T>: an embedded 
> object that is owned (the parent has a reference and a field of type 
> Child releases that reference when the parent is finalized) but cannot 
> be cloned.
> 
>> For this reason I think object_initialize(), 
>> object_initialize_child(), and the like are better to be noted as 
>> deprecated in
>> include/qom/object.h. Then memory_region_init() can be deprecated 
>> referring to them.
> 
> This would be huge and I don't think it's feasible.
> 
> It also only provides the appearance of safety.  If you have a backwards 
> pointer (such as the memory region's owner), you still have either a 
> leak or the risk of a use-after-free.

Now I'm starting to recover my memory on the situation of this matter. I 
have actually written patches to make struct embedding safe and also 
deal with memory region's owner. My tree that include them can be found at:
https://gitlab.com/akihiko.odaki/qemu/-/tree/51066314fcff96303c1ad5814c3d74027855a7cb

Making struct embedding safe is done by introducing another reference 
counter for storage. So the reference graph will be looked like the 
following:

Owner -+-> Owner's storage
MR ----+

Memory region's backward references to their owning devices are 
invalidated when unrealizing the devices. The reasoning here is that the 
corner cases where devices are accessed after unrealization are unlikely 
to be handled properly, so they are better to be forbidden.

Users other than memory regions may still have backward pointers that 
are not properly tracked, so the Rust wrapper of object_ref() is still 
marked unsafe.

Regards,
Akihiko Odaki