[RFC 0/7] qom: deprecate embedded objects and instance properties

[RFC 0/7] qom: deprecate embedded objects and instance properties

[Daniel P. Berrangé]

QOM has two rather unusual / surprising features historicall

 * The ability to embed a QOM instance's memory inside another
   struct
 * The ability to register properties against the instnce
   instead of struct

While they both look convenient on the surface, they also
have significant undesirable side effects (see the commit
message for each patch for details).

The premise of this series is that their convenience does
not outweigh their downsides, and we would be better off
long term by eliminating their usage, rather than trying
to add more hacks on top to mitigate their downsides.

This comes out of two separate conversations this week

 * Migration series where Peter proposed changes that
   make use of instance  properties. When I commented,
   Peter rightly pointed out that our docs do not
   discourage use of instance properties:
   
    https://lists.gnu.org/archive/html/qemu-devel/2026-06/msg02368.html

 * QOM series where Akihiko proposed some funky hacks
   to reference counting to better track object lifecycle
   when embedding structs. We had a short discussion about
   discouraging QOM embedding, so this is a real world
   proposal:

     https://lists.gnu.org/archive/html/qemu-devel/2026-06/msg03459.html

This is not likely to be a quick task, so this series
starts small

 * Adds a "QEMU_DEPRECATIONS" annotation for internal APIs
 * Deprecate the QOM instance embedding
 * Deprecate the QOM instance properties
 * Deprecate the memory region embedding APIs
 * Deprecate the IRQ embedding APIs
 * Convert PIIX to eliminate embedding as a
   demonstration.

The QEMU_DEPRECATIONS idea is an effect to improve our our
historic practice where we introduce a new preferred API
and never really tell anyone the old APIs is bad to use.

If --enable-deprecations is given to configure, every use
of a deprecated API emits a compiler warning. This is not
enabled by default in this series, since we have -Werror
by default.

Note that the proposal to stop embedding memory regions has
been made by Zoltan earlier this year and was rejected by Paolo:

  https://lists.nongnu.org/archive/html/qemu-devel/2026-01/msg05435.html
  https://lists.nongnu.org/archive/html/qemu-devel/2026-05/msg06665.html

I don't really agree with the analysis there. IMHO,  the
concept of embedding objects is to horrendous to allow to
live any longer. Yes, that's a big job, but long term it
is worth it.

Daniel P. Berrangé (7):
  meson: add --enable-deprecations configure flag
  qom: deprecated embedding object structs within other objects
  qom: deprecate use of instance properties
  system: add memory_region_new / memory_region_new_io
  system: add qemu_irq_new / qemu_irq_new_child / qemu_irq_new_array
  hw/isa: convert PIIX embedded QOM objects to heap allocated
  qom: improve error message for invalid ID values

 hw/core/irq.c                 | 35 +++++++++++++
 hw/isa/piix.c                 | 65 ++++++++++++++---------
 include/hw/core/irq.h         | 75 +++++++++++++++++++++++++--
 include/hw/southbridge/piix.h | 12 ++---
 include/qemu/osdep.h          | 19 +++++++
 include/qom/object.h          | 98 ++++++++++++++++++++++++++++++-----
 include/system/memory.h       | 76 +++++++++++++++++++++++----
 meson.build                   |  1 +
 meson_options.txt             |  2 +
 qom/object.c                  | 36 ++++++++++++-
 scripts/meson-buildoptions.sh |  3 ++
 system/memory.c               | 85 +++++++++++++++++++++++++-----
 12 files changed, 433 insertions(+), 74 deletions(-)

-- 
2.54.0

[RFC 1/7] meson: add --enable-deprecations configure flag

[Daniel P. Berrangé]

A challenge with QEMU is that internal APIs get obsoleted but chasing
down use of old APIs is not as simple as it should be. Introduce a
--enable-deprecations configure flag which expands the macro
"QEMU_DEPRECATIONS" into "G_GNUC_DEPRECATED". This will trigger a
compiler warning for any use of the deprecated APIs.

It would generally be a good idea to disable -Werror when turning on
deprecations otherwise the build will quickly abort.


XXX: possibly add  -Wno-error=deprecated-declarations, so we have
show deprecations by default despite -Werror being on by default.
This would trigger very many warnings but might be worth it if we
want to strongly nudge maintainers to convert existing code. Our
historical approach of "hope" has never worked to eliminate old
code patterns.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 include/qemu/osdep.h          | 19 +++++++++++++++++++
 meson.build                   |  1 +
 meson_options.txt             |  2 ++
 scripts/meson-buildoptions.sh |  3 +++
 4 files changed, 25 insertions(+)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 2f0e61ad6b..99b8f9cbb7 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -447,6 +447,25 @@ void QEMU_ERROR("code path is reachable")
         ((void)0))
 #endif
 
+/*
+ * Tag an internal APIs which should no longer be used
+ * to emit a warning during build if --enable-deprecations
+ * is used with configure. Use of -Werror will trigger
+ * immediate build failure if this is used.
+ */
+#ifdef CONFIG_DEPRECATIONS
+# define QEMU_DEPRECATED G_GNUC_DEPRECATED
+# define QEMU_DEPRECATIONS_OFF \
+    _Pragma("GCC diagnostic push") \
+    _Pragma("GCC diagnostic ignored \"-Wdeprecated-declarations\"")
+# define QEMU_DEPRECATIONS_ON \
+    _Pragma("GCC diagnostic pop")
+#else
+# define QEMU_DEPRECATED
+# define QEMU_DEPRECATIONS_OFF
+# define QEMU_DEPRECATIONS_ON
+#endif
+
 /*
  * Minimum function that returns zero only if both values are zero.
  * Intended for use with unsigned values only.
diff --git a/meson.build b/meson.build
index 19e123423b..9e863aa897 100644
--- a/meson.build
+++ b/meson.build
@@ -2567,6 +2567,7 @@ config_host_data.set('CONFIG_DEBUG_STACK_USAGE', get_option('debug_stack_usage')
 config_host_data.set('CONFIG_DEBUG_TCG', get_option('debug_tcg'))
 config_host_data.set('CONFIG_DEBUG_REMAP', get_option('debug_remap'))
 config_host_data.set('CONFIG_QOM_CAST_DEBUG', get_option('qom_cast_debug'))
+config_host_data.set('CONFIG_DEPRECATIONS', get_option('deprecations'))
 config_host_data.set('CONFIG_REPLICATION', get_option('replication').allowed())
 config_host_data.set('CONFIG_FSFREEZE', qga_fsfreeze)
 config_host_data.set('CONFIG_FSTRIM', qga_fstrim)
diff --git a/meson_options.txt b/meson_options.txt
index a07cb47d35..8f5924422d 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -367,6 +367,8 @@ option('qom_cast_debug', type: 'boolean', value: true,
        description: 'cast debugging support')
 option('slirp_smbd', type : 'feature', value : 'auto',
        description: 'use smbd (at path --smbd=*) in slirp networking')
+option('deprecations', type: 'boolean', value: true,
+       description: 'enable internal API deprecation warnings')
 
 option('qemu_ga_manufacturer', type: 'string', value: 'QEMU',
        description: '"manufacturer" name for qemu-ga registry entries')
diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
index c003985047..fb57f0610d 100644
--- a/scripts/meson-buildoptions.sh
+++ b/scripts/meson-buildoptions.sh
@@ -13,6 +13,7 @@ meson_options_help() {
   printf "%s\n" '  --datadir=VALUE          Data file directory [share]'
   printf "%s\n" '  --disable-coroutine-pool coroutine freelist (better performance)'
   printf "%s\n" '  --disable-debug-info     Enable debug symbols and other information'
+  printf "%s\n" '  --disable-deprecations   enable internal API deprecation warnings'
   printf "%s\n" '  --disable-hexagon-idef-parser'
   printf "%s\n" '                           use idef-parser to automatically generate TCG'
   printf "%s\n" '                           code for the Hexagon frontend'
@@ -306,6 +307,8 @@ _meson_option_parse() {
     --disable-debug-stack-usage) printf "%s" -Ddebug_stack_usage=false ;;
     --enable-debug-tcg) printf "%s" -Ddebug_tcg=true ;;
     --disable-debug-tcg) printf "%s" -Ddebug_tcg=false ;;
+    --enable-deprecations) printf "%s" -Ddeprecations=true ;;
+    --disable-deprecations) printf "%s" -Ddeprecations=false ;;
     --enable-dmg) printf "%s" -Ddmg=enabled ;;
     --disable-dmg) printf "%s" -Ddmg=disabled ;;
     --docdir=*) quote_sh "-Ddocdir=$2" ;;
-- 
2.54.0

Re: [RFC 1/7] meson: add --enable-deprecations configure flag

[Akihiko Odaki]

On 2026/06/17 0:55, Daniel P. Berrangé wrote:
> A challenge with QEMU is that internal APIs get obsoleted but chasing
> down use of old APIs is not as simple as it should be. Introduce a
> --enable-deprecations configure flag which expands the macro
> "QEMU_DEPRECATIONS" into "G_GNUC_DEPRECATED". This will trigger a
> compiler warning for any use of the deprecated APIs.
> 
> It would generally be a good idea to disable -Werror when turning on
> deprecations otherwise the build will quickly abort.
> 
> 
> XXX: possibly add  -Wno-error=deprecated-declarations, so we have
> show deprecations by default despite -Werror being on by default.
> This would trigger very many warnings but might be worth it if we
> want to strongly nudge maintainers to convert existing code. Our
> historical approach of "hope" has never worked to eliminate old
> code patterns.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   include/qemu/osdep.h          | 19 +++++++++++++++++++
>   meson.build                   |  1 +
>   meson_options.txt             |  2 ++
>   scripts/meson-buildoptions.sh |  3 +++
>   4 files changed, 25 insertions(+)
> 
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index 2f0e61ad6b..99b8f9cbb7 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -447,6 +447,25 @@ void QEMU_ERROR("code path is reachable")
>           ((void)0))
>   #endif
>   
> +/*
> + * Tag an internal APIs which should no longer be used
> + * to emit a warning during build if --enable-deprecations
> + * is used with configure. Use of -Werror will trigger
> + * immediate build failure if this is used.
> + */
> +#ifdef CONFIG_DEPRECATIONS
> +# define QEMU_DEPRECATED G_GNUC_DEPRECATED
> +# define QEMU_DEPRECATIONS_OFF \
> +    _Pragma("GCC diagnostic push") \
> +    _Pragma("GCC diagnostic ignored \"-Wdeprecated-declarations\"")
> +# define QEMU_DEPRECATIONS_ON \
> +    _Pragma("GCC diagnostic pop")
> +#else
> +# define QEMU_DEPRECATED
> +# define QEMU_DEPRECATIONS_OFF
> +# define QEMU_DEPRECATIONS_ON
> +#endif
> +
>   /*
>    * Minimum function that returns zero only if both values are zero.
>    * Intended for use with unsigned values only.
> diff --git a/meson.build b/meson.build
> index 19e123423b..9e863aa897 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2567,6 +2567,7 @@ config_host_data.set('CONFIG_DEBUG_STACK_USAGE', get_option('debug_stack_usage')
>   config_host_data.set('CONFIG_DEBUG_TCG', get_option('debug_tcg'))
>   config_host_data.set('CONFIG_DEBUG_REMAP', get_option('debug_remap'))
>   config_host_data.set('CONFIG_QOM_CAST_DEBUG', get_option('qom_cast_debug'))
> +config_host_data.set('CONFIG_DEPRECATIONS', get_option('deprecations'))
>   config_host_data.set('CONFIG_REPLICATION', get_option('replication').allowed())
>   config_host_data.set('CONFIG_FSFREEZE', qga_fsfreeze)
>   config_host_data.set('CONFIG_FSTRIM', qga_fstrim)
> diff --git a/meson_options.txt b/meson_options.txt
> index a07cb47d35..8f5924422d 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -367,6 +367,8 @@ option('qom_cast_debug', type: 'boolean', value: true,
>          description: 'cast debugging support')
>   option('slirp_smbd', type : 'feature', value : 'auto',
>          description: 'use smbd (at path --smbd=*) in slirp networking')
> +option('deprecations', type: 'boolean', value: true,
> +       description: 'enable internal API deprecation warnings')

A build with the default settings will fail because both werror and 
deprecations are enabled by default.

>   
>   option('qemu_ga_manufacturer', type: 'string', value: 'QEMU',
>          description: '"manufacturer" name for qemu-ga registry entries')
> diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
> index c003985047..fb57f0610d 100644
> --- a/scripts/meson-buildoptions.sh
> +++ b/scripts/meson-buildoptions.sh
> @@ -13,6 +13,7 @@ meson_options_help() {
>     printf "%s\n" '  --datadir=VALUE          Data file directory [share]'
>     printf "%s\n" '  --disable-coroutine-pool coroutine freelist (better performance)'
>     printf "%s\n" '  --disable-debug-info     Enable debug symbols and other information'
> +  printf "%s\n" '  --disable-deprecations   enable internal API deprecation warnings'

The help text is inverted: it says --disable-deprecations will enable 
warnings.

Those said, I think the approach to modify checkpatch discussed in 
another thread is better.

Regards,
Akihiko Odaki

>     printf "%s\n" '  --disable-hexagon-idef-parser'
>     printf "%s\n" '                           use idef-parser to automatically generate TCG'
>     printf "%s\n" '                           code for the Hexagon frontend'
> @@ -306,6 +307,8 @@ _meson_option_parse() {
>       --disable-debug-stack-usage) printf "%s" -Ddebug_stack_usage=false ;;
>       --enable-debug-tcg) printf "%s" -Ddebug_tcg=true ;;
>       --disable-debug-tcg) printf "%s" -Ddebug_tcg=false ;;
> +    --enable-deprecations) printf "%s" -Ddeprecations=true ;;
> +    --disable-deprecations) printf "%s" -Ddeprecations=false ;;
>       --enable-dmg) printf "%s" -Ddmg=enabled ;;
>       --disable-dmg) printf "%s" -Ddmg=disabled ;;
>       --docdir=*) quote_sh "-Ddocdir=$2" ;;

[RFC 2/7] qom: deprecated embedding object structs within other objects

[Daniel P. Berrangé]

The QOM APIs currently allow objects to be either allocated directly
on the heap, or statically embedded inside the struct of another object.

For the latter QOM has logic to avoid calling 'free' on the object when
finalizers complete, however, this is not sufficient to make the
practice safe.

Users of QOM expect that if they call "object_ref" to acquire their own
reference, then object will never be freed as long as they hold it.

This expectation is broken when an instance is embedded, as the "owner"
object's may be finalized, which frees the memory that is storing the
embedded QOM instance, even if its ref-count is still live.

Worse still is that a user of a QOM object cannot easily tell if the
instance they're using is embedded or directly heap allocated.

Mark the APIs for embedding objects as deprecated as the first step
towards removal of this flawed design concept. All objects must now
be directly heap allocated going forward, and existing usage must be
incrementally converted.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 include/qom/object.h | 28 ++++++++++++++++++++++++----
 qom/object.c         |  6 ++++++
 2 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/include/qom/object.h b/include/qom/object.h
index 11f55613fc..dd708b1136 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -854,8 +854,13 @@ bool object_set_props_from_keyval(Object *obj, const QDict *qdict,
  * This function will initialize an object.  The memory for the object should
  * have already been allocated.  The returned object has a reference count of 1,
  * and will be finalized when the last reference is dropped.
+ *
+ * Use of this function is now deprecated. All objects must be
+ * allocated using the object_new() family of functions and not
+ * statically embedded in a larger struct.
  */
-void object_initialize(void *obj, size_t size, const char *typename);
+void object_initialize(void *obj, size_t size, const char *typename)
+    QEMU_DEPRECATED;
 
 /**
  * object_initialize_child_with_props:
@@ -873,6 +878,10 @@ void object_initialize(void *obj, size_t size, const char *typename);
  * has a reference count of 1 (for the "child<...>" property from the parent),
  * so the object will be finalized automatically when the parent gets removed.
  *
+ * Use of this function is now deprecated. All objects must be
+ * allocated using the object_new() family of functions and not
+ * statically embedded in a larger struct.
+ *
  * The variadic parameters are a list of pairs of (propname, propvalue)
  * strings. The propname of %NULL indicates the end of the property list.
  * If the object implements the user creatable interface, the object will
@@ -883,7 +892,8 @@ void object_initialize(void *obj, size_t size, const char *typename);
 bool object_initialize_child_with_props(Object *parentobj,
                              const char *propname,
                              void *childobj, size_t size, const char *type,
-                             Error **errp, ...) G_GNUC_NULL_TERMINATED;
+                             Error **errp, ...) G_GNUC_NULL_TERMINATED
+    QEMU_DEPRECATED;
 
 /**
  * object_initialize_child_with_propsv:
@@ -897,12 +907,17 @@ bool object_initialize_child_with_props(Object *parentobj,
  *
  * See object_initialize_child() for documentation.
  *
+ * Use of this function is now deprecated. All objects must be
+ * allocated using the object_new() family of functions and not
+ * statically embedded in a larger struct.
+ *
  * Returns: %true on success, %false on failure.
  */
 bool object_initialize_child_with_propsv(Object *parentobj,
                               const char *propname,
                               void *childobj, size_t size, const char *type,
-                              Error **errp, va_list vargs);
+                              Error **errp, va_list vargs)
+    QEMU_DEPRECATED;
 
 /**
  * object_initialize_child:
@@ -917,13 +932,18 @@ bool object_initialize_child_with_propsv(Object *parentobj,
  *   object_initialize_child_with_props(parent, propname,
  *                                      child, sizeof(*child), type,
  *                                      &error_abort, NULL)
+ *
+ * Use of this function is now deprecated. All objects must be
+ * allocated using the object_new() family of functions and not
+ * statically embedded in a larger struct.
  */
 #define object_initialize_child(parent, propname, child, type)          \
     object_initialize_child_internal((parent), (propname),              \
                                      (child), sizeof(*(child)), (type))
 void object_initialize_child_internal(Object *parent, const char *propname,
                                       void *child, size_t size,
-                                      const char *type);
+                                      const char *type)
+    QEMU_DEPRECATED;
 
 /**
  * object_dynamic_cast:
diff --git a/qom/object.c b/qom/object.c
index 0ac201de4c..33b2801ee4 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -522,9 +522,11 @@ bool object_initialize_child_with_props(Object *parentobj,
     bool ok;
 
     va_start(vargs, errp);
+QEMU_DEPRECATIONS_OFF;
     ok = object_initialize_child_with_propsv(parentobj, propname,
                                              childobj, size, type, errp,
                                              vargs);
+QEMU_DEPRECATIONS_ON;
     va_end(vargs);
     return ok;
 }
@@ -539,7 +541,9 @@ bool object_initialize_child_with_propsv(Object *parentobj,
     Object *obj;
     UserCreatable *uc;
 
+QEMU_DEPRECATIONS_OFF;
     object_initialize(childobj, size, type);
+QEMU_DEPRECATIONS_ON;
     obj = OBJECT(childobj);
 
     if (!object_set_propv(obj, vargs, errp)) {
@@ -576,8 +580,10 @@ void object_initialize_child_internal(Object *parent,
                                       void *child, size_t size,
                                       const char *type)
 {
+QEMU_DEPRECATIONS_OFF;
     object_initialize_child_with_props(parent, propname, child, size, type,
                                        &error_abort, NULL);
+QEMU_DEPRECATIONS_ON;
 }
 
 static inline bool object_property_is_child(ObjectProperty *prop)
-- 
2.54.0

Re: [RFC 2/7] qom: deprecated embedding object structs within other objects

[Peter Maydell]

On Tue, 16 Jun 2026 at 16:56, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> The QOM APIs currently allow objects to be either allocated directly
> on the heap, or statically embedded inside the struct of another object.
>
> For the latter QOM has logic to avoid calling 'free' on the object when
> finalizers complete, however, this is not sufficient to make the
> practice safe.
>
> Users of QOM expect that if they call "object_ref" to acquire their own
> reference, then object will never be freed as long as they hold it.
>
> This expectation is broken when an instance is embedded, as the "owner"
> object's may be finalized, which frees the memory that is storing the
> embedded QOM instance, even if its ref-count is still live.
>
> Worse still is that a user of a QOM object cannot easily tell if the
> instance they're using is embedded or directly heap allocated.
>
> Mark the APIs for embedding objects as deprecated as the first step
> towards removal of this flawed design concept. All objects must now
> be directly heap allocated going forward, and existing usage must be
> incrementally converted.

I think if you mark an API that we call in 800+ places and 150+ files
as deprecated, mostly what will happen is nobody ever turns on the
--enable-deprecated option because the output will be full of noise...

-- PMM

Re: [RFC 2/7] qom: deprecated embedding object structs within other objects

[Daniel P. Berrangé]

On Tue, Jun 16, 2026 at 05:15:22PM +0100, Peter Maydell wrote:
> On Tue, 16 Jun 2026 at 16:56, Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > The QOM APIs currently allow objects to be either allocated directly
> > on the heap, or statically embedded inside the struct of another object.
> >
> > For the latter QOM has logic to avoid calling 'free' on the object when
> > finalizers complete, however, this is not sufficient to make the
> > practice safe.
> >
> > Users of QOM expect that if they call "object_ref" to acquire their own
> > reference, then object will never be freed as long as they hold it.
> >
> > This expectation is broken when an instance is embedded, as the "owner"
> > object's may be finalized, which frees the memory that is storing the
> > embedded QOM instance, even if its ref-count is still live.
> >
> > Worse still is that a user of a QOM object cannot easily tell if the
> > instance they're using is embedded or directly heap allocated.
> >
> > Mark the APIs for embedding objects as deprecated as the first step
> > towards removal of this flawed design concept. All objects must now
> > be directly heap allocated going forward, and existing usage must be
> > incrementally converted.
> 
> I think if you mark an API that we call in 800+ places and 150+ files
> as deprecated, mostly what will happen is nobody ever turns on the
> --enable-deprecated option because the output will be full of noise...

That is a challenge. The baseline is that developers have no visibility
of use of deprecated APIs at all, and we barely even document them.

My thought is that even if --enable-deprecated is rarely enabled, at
least having QEMU_DEPRECATED visible in the header files is a red flag
for developers to look elsewhere for their solution.

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 :|

[RFC 3/7] qom: deprecate use of instance properties

[Daniel P. Berrangé]

The concept of class properties was introduced over 10 years ago.
While there is significant use of class properties, there is a
long way to go before any conversion is complete and new uses are
often introduced as it is not obvious that class properties are
preferred.

The downsides of instances properties are:

 * Increased memory usage as the property info is duplicated
   across every instance.

 * Introspection side effects, since instances need to be created
   to be introspected and this can have unexpected side effects
   if code is not expecting these throwaway instances to be
   around.

 * Non-introspectable designs, if properties are conditionally
   registered against instances, those props may not be visible
   when a dummy instances is created for introspection.

This deprecates the use of instance properties for all regular
scalar properties. ie strings, integers, enums, etc.

It does not deprecate instance properties used for setting up
the QOM composition tree child relationships, nor the link or
alias properties.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 include/qom/object.h | 70 +++++++++++++++++++++++++++++++++++++-------
 qom/object.c         | 28 ++++++++++++++++++
 2 files changed, 88 insertions(+), 10 deletions(-)

diff --git a/include/qom/object.h b/include/qom/object.h
index dd708b1136..e9ce15d595 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -1175,6 +1175,10 @@ void object_unref(void *obj);
  * @opaque: an opaque pointer to pass to the callbacks for the property
  * @errp: pointer to error object
  *
+ * Use of this function is now deprecated. All properties must be
+ * registered against the class, using the object_class_property_add()
+ * function and not registered against instances.
+ *
  * Returns: The #ObjectProperty; this can be used to set the @resolve
  * callback for child and link properties.
  */
@@ -1183,7 +1187,8 @@ ObjectProperty *object_property_try_add(Object *obj, const char *name,
                                         ObjectPropertyAccessor *get,
                                         ObjectPropertyAccessor *set,
                                         ObjectPropertyRelease *release,
-                                        void *opaque, Error **errp);
+                                        void *opaque, Error **errp)
+    QEMU_DEPRECATED;
 
 /**
  * object_property_add:
@@ -1206,13 +1211,18 @@ ObjectProperty *object_property_try_add(Object *obj, const char *name,
  *   meant to allow a property to free its opaque upon object
  *   destruction.  This may be NULL.
  * @opaque: an opaque pointer to pass to the callbacks for the property
+ *
+ * Use of this function is now deprecated. All properties must be
+ * registered against the class, using the object_class_property_add()
+ * function and not registered against instances.
  */
 ObjectProperty *object_property_add(Object *obj, const char *name,
                                     const char *type,
                                     ObjectPropertyAccessor *get,
                                     ObjectPropertyAccessor *set,
                                     ObjectPropertyRelease *release,
-                                    void *opaque);
+                                    void *opaque)
+    QEMU_DEPRECATED;
 
 void object_property_del(Object *obj, const char *name);
 
@@ -1873,11 +1883,16 @@ Object *object_resolve_and_typecheck(const char *path, const char *name,
  * Add a string property using getters/setters.  This function will add a
  * property of type 'string'.
  *
+ * Use of this function is now deprecated. All properties must be
+ * registered against the class, using the object_class_property_add_str()
+ * function and not registered against instances.
+ *
  * Returns: The newly added property on success, or %NULL on failure.
  */
 ObjectProperty *object_property_add_str(Object *obj, const char *name,
                              char *(*get)(Object *, Error **),
-                             void (*set)(Object *, const char *, Error **));
+                             void (*set)(Object *, const char *, Error **))
+    QEMU_DEPRECATED;
 
 ObjectProperty *object_class_property_add_str(ObjectClass *klass,
                                    const char *name,
@@ -1895,11 +1910,16 @@ ObjectProperty *object_class_property_add_str(ObjectClass *klass,
  * Add a bool property using getters/setters.  This function will add a
  * property of type 'bool'.
  *
+ * Use of this function is now deprecated. All properties must be
+ * registered against the class, using the object_class_property_add_bool()
+ * function and not registered against instances.
+ *
  * Returns: The newly added property on success, or %NULL on failure.
  */
 ObjectProperty *object_property_add_bool(Object *obj, const char *name,
                               bool (*get)(Object *, Error **),
-                              void (*set)(Object *, bool, Error **));
+                              void (*set)(Object *, bool, Error **))
+    QEMU_DEPRECATED;
 
 ObjectProperty *object_class_property_add_bool(ObjectClass *klass,
                                     const char *name,
@@ -1918,13 +1938,18 @@ ObjectProperty *object_class_property_add_bool(ObjectClass *klass,
  * Add an enum property using getters/setters.  This function will add a
  * property of type '@typename'.
  *
+ * Use of this function is now deprecated. All properties must be
+ * registered against the class, using the object_class_property_add_enum()
+ * function and not registered against instances.
+ *
  * Returns: The newly added property on success, or %NULL on failure.
  */
 ObjectProperty *object_property_add_enum(Object *obj, const char *name,
                               const char *typename,
                               const QEnumLookup *lookup,
                               int (*get)(Object *, Error **),
-                              void (*set)(Object *, int, Error **));
+                              void (*set)(Object *, int, Error **))
+    QEMU_DEPRECATED;
 
 ObjectProperty *object_class_property_add_enum(ObjectClass *klass,
                                     const char *name,
@@ -1942,10 +1967,15 @@ ObjectProperty *object_class_property_add_enum(ObjectClass *klass,
  * Add a read-only struct tm valued property using a getter function.
  * This function will add a property of type 'struct tm'.
  *
+ * Use of this function is now deprecated. All properties must be
+ * registered against the class, using the object_class_property_add_tm()
+ * function and not registered against instances.
+ *
  * Returns: The newly added property on success, or %NULL on failure.
  */
 ObjectProperty *object_property_add_tm(Object *obj, const char *name,
-                            void (*get)(Object *, struct tm *, Error **));
+                            void (*get)(Object *, struct tm *, Error **))
+    QEMU_DEPRECATED;
 
 ObjectProperty *object_class_property_add_tm(ObjectClass *klass,
                             const char *name,
@@ -1970,11 +2000,16 @@ typedef enum {
  * Add an integer property in memory.  This function will add a
  * property of type 'uint8'.
  *
+ * Use of this function is now deprecated. All properties must be
+ * registered against the class, using the object_class_property_add_uint8_ptr()
+ * function and not registered against instances.
+ *
  * Returns: The newly added property on success, or %NULL on failure.
  */
 ObjectProperty *object_property_add_uint8_ptr(Object *obj, const char *name,
                                               const uint8_t *v,
-                                              ObjectPropertyFlags flags);
+                                              ObjectPropertyFlags flags)
+    QEMU_DEPRECATED;
 
 ObjectProperty *object_class_property_add_uint8_ptr(ObjectClass *klass,
                                          const char *name,
@@ -1991,11 +2026,16 @@ ObjectProperty *object_class_property_add_uint8_ptr(ObjectClass *klass,
  * Add an integer property in memory.  This function will add a
  * property of type 'uint16'.
  *
+ * Use of this function is now deprecated. All properties must be
+ * registered against the class, using the object_class_property_add_uint16_ptr()
+ * function and not registered against instances.
+ *
  * Returns: The newly added property on success, or %NULL on failure.
  */
 ObjectProperty *object_property_add_uint16_ptr(Object *obj, const char *name,
                                     const uint16_t *v,
-                                    ObjectPropertyFlags flags);
+                                    ObjectPropertyFlags flags)
+    QEMU_DEPRECATED;
 
 ObjectProperty *object_class_property_add_uint16_ptr(ObjectClass *klass,
                                           const char *name,
@@ -2012,11 +2052,16 @@ ObjectProperty *object_class_property_add_uint16_ptr(ObjectClass *klass,
  * Add an integer property in memory.  This function will add a
  * property of type 'uint32'.
  *
+ * Use of this function is now deprecated. All properties must be
+ * registered against the class, using the object_class_property_add_uint32_ptr()
+ * function and not registered against instances.
+ *
  * Returns: The newly added property on success, or %NULL on failure.
  */
 ObjectProperty *object_property_add_uint32_ptr(Object *obj, const char *name,
                                     const uint32_t *v,
-                                    ObjectPropertyFlags flags);
+                                    ObjectPropertyFlags flags)
+    QEMU_DEPRECATED;
 
 ObjectProperty *object_class_property_add_uint32_ptr(ObjectClass *klass,
                                           const char *name,
@@ -2033,11 +2078,16 @@ ObjectProperty *object_class_property_add_uint32_ptr(ObjectClass *klass,
  * Add an integer property in memory.  This function will add a
  * property of type 'uint64'.
  *
+ * Use of this function is now deprecated. All properties must be
+ * registered against the class, using the object_class_property_add_uint64_ptr()
+ * function and not registered against instances.
+ *
  * Returns: The newly added property on success, or %NULL on failure.
  */
 ObjectProperty *object_property_add_uint64_ptr(Object *obj, const char *name,
                                     const uint64_t *v,
-                                    ObjectPropertyFlags flags);
+                                    ObjectPropertyFlags flags)
+    QEMU_DEPRECATED;
 
 ObjectProperty *object_class_property_add_uint64_ptr(ObjectClass *klass,
                                           const char *name,
diff --git a/qom/object.c b/qom/object.c
index 33b2801ee4..415d5c5291 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1362,8 +1362,10 @@ object_property_try_add(Object *obj, const char *name, const char *type,
         for (i = 0; i < INT16_MAX; ++i) {
             char *full_name = g_strdup_printf("%s[%d]", name_no_array, i);
 
+            QEMU_DEPRECATIONS_OFF;
             ret = object_property_try_add(obj, full_name, type, get, set,
                                           release, opaque, NULL);
+            QEMU_DEPRECATIONS_ON;
             g_free(full_name);
             if (ret) {
                 break;
@@ -1403,8 +1405,10 @@ object_property_add(Object *obj, const char *name, const char *type,
                     ObjectPropertyRelease *release,
                     void *opaque)
 {
+    QEMU_DEPRECATIONS_OFF;
     return object_property_try_add(obj, name, type, get, set, release,
                                    opaque, &error_abort);
+    QEMU_DEPRECATIONS_ON;
 }
 
 ObjectProperty *
@@ -1951,9 +1955,11 @@ object_property_try_add_child(Object *obj, const char *name,
 
     type = g_strdup_printf("child<%s>", object_get_typename(child));
 
+    QEMU_DEPRECATIONS_OFF;
     op = object_property_try_add(obj, name, type, object_get_child_property,
                                  NULL, object_finalize_child_property,
                                  child, errp);
+    QEMU_DEPRECATIONS_ON;
     if (!op) {
         return NULL;
     }
@@ -1967,7 +1973,9 @@ ObjectProperty *
 object_property_add_child(Object *obj, const char *name,
                           Object *child)
 {
+    QEMU_DEPRECATIONS_OFF;
     return object_property_try_add_child(obj, name, child, &error_abort);
+    QEMU_DEPRECATIONS_ON;
 }
 
 void object_property_allow_set_link(const Object *obj, const char *name,
@@ -2145,11 +2153,13 @@ object_add_link_prop(Object *obj, const char *name,
 
     full_type = g_strdup_printf("link<%s>", type);
 
+    QEMU_DEPRECATIONS_OFF;
     op = object_property_add(obj, name, full_type,
                              object_get_link_property,
                              check ? object_set_link_property : NULL,
                              object_release_link_property,
                              prop);
+    QEMU_DEPRECATIONS_ON;
     op->resolve = object_resolve_link_property;
     return op;
 }
@@ -2442,11 +2452,13 @@ object_property_add_str(Object *obj, const char *name,
     prop->get = get;
     prop->set = set;
 
+    QEMU_DEPRECATIONS_OFF;
     return object_property_add(obj, name, "string",
                                get ? property_get_str : NULL,
                                set ? property_set_str : NULL,
                                property_release_data,
                                prop);
+    QEMU_DEPRECATIONS_ON;
 }
 
 ObjectProperty *
@@ -2512,11 +2524,13 @@ object_property_add_bool(Object *obj, const char *name,
     prop->get = get;
     prop->set = set;
 
+    QEMU_DEPRECATIONS_OFF;
     return object_property_add(obj, name, "bool",
                                get ? property_get_bool : NULL,
                                set ? property_set_bool : NULL,
                                property_release_data,
                                prop);
+    QEMU_DEPRECATIONS_ON;
 }
 
 ObjectProperty *
@@ -2577,11 +2591,13 @@ object_property_add_enum(Object *obj, const char *name,
     prop->get = get;
     prop->set = set;
 
+    QEMU_DEPRECATIONS_OFF;
     return object_property_add(obj, name, typename,
                                get ? property_get_enum : NULL,
                                set ? property_set_enum : NULL,
                                property_release_data,
                                prop);
+    QEMU_DEPRECATIONS_ON;
 }
 
 ObjectProperty *
@@ -2655,10 +2671,12 @@ object_property_add_tm(Object *obj, const char *name,
 
     prop->get = get;
 
+    QEMU_DEPRECATIONS_OFF;
     return object_property_add(obj, name, "struct tm",
                                get ? property_get_tm : NULL, NULL,
                                property_release_data,
                                prop);
+    QEMU_DEPRECATIONS_ON;
 }
 
 ObjectProperty *
@@ -2775,8 +2793,10 @@ object_property_add_uint8_ptr(Object *obj, const char *name,
         setter = property_set_uint8_ptr;
     }
 
+    QEMU_DEPRECATIONS_OFF;
     return object_property_add(obj, name, "uint8",
                                getter, setter, NULL, (void *)v);
+    QEMU_DEPRECATIONS_ON;
 }
 
 ObjectProperty *
@@ -2815,8 +2835,10 @@ object_property_add_uint16_ptr(Object *obj, const char *name,
         setter = property_set_uint16_ptr;
     }
 
+    QEMU_DEPRECATIONS_OFF;
     return object_property_add(obj, name, "uint16",
                                getter, setter, NULL, (void *)v);
+    QEMU_DEPRECATIONS_ON;
 }
 
 ObjectProperty *
@@ -2855,8 +2877,10 @@ object_property_add_uint32_ptr(Object *obj, const char *name,
         setter = property_set_uint32_ptr;
     }
 
+    QEMU_DEPRECATIONS_OFF;
     return object_property_add(obj, name, "uint32",
                                getter, setter, NULL, (void *)v);
+    QEMU_DEPRECATIONS_ON;
 }
 
 ObjectProperty *
@@ -2895,8 +2919,10 @@ object_property_add_uint64_ptr(Object *obj, const char *name,
         setter = property_set_uint64_ptr;
     }
 
+    QEMU_DEPRECATIONS_OFF;
     return object_property_add(obj, name, "uint64",
                                getter, setter, NULL, (void *)v);
+    QEMU_DEPRECATIONS_ON;
 }
 
 ObjectProperty *
@@ -2983,11 +3009,13 @@ object_property_add_alias(Object *obj, const char *name,
     prop->target_obj = target_obj;
     prop->target_name = g_strdup(target_name);
 
+    QEMU_DEPRECATIONS_OFF;
     op = object_property_add(obj, name, prop_type,
                              property_get_alias,
                              property_set_alias,
                              property_release_alias,
                              prop);
+    QEMU_DEPRECATIONS_ON;
     op->resolve = property_resolve_alias;
     if (target_prop->defval) {
         op->defval = qobject_ref(target_prop->defval);
-- 
2.54.0

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

[Daniel P. Berrangé]

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é <berrange@redhat.com>
---
 include/system/memory.h | 76 ++++++++++++++++++++++++++++++------
 system/memory.c         | 85 ++++++++++++++++++++++++++++++++++-------
 2 files changed, 136 insertions(+), 25 deletions(-)

diff --git a/include/system/memory.h b/include/system/memory.h
index 1417132f6d..b4c5a185c0 100644
--- a/include/system/memory.h
+++ b/include/system/memory.h
@@ -1300,19 +1300,44 @@ static inline bool memory_region_section_intersect_range(MemoryRegionSection *s,
 
 /**
  * memory_region_init: Initialize a memory region
- *
- * The region typically acts as a container for other memory regions.  Use
- * memory_region_add_subregion() to add subregions.
- *
  * @mr: the #MemoryRegion to be initialized
  * @owner: the object that tracks the region's reference count
  * @name: used for debugging; not visible to the user or ABI
  * @size: size of the region; any subregions beyond this size will be clipped
+ *
+ * The region typically acts as a container for other memory regions.  Use
+ * memory_region_add_subregion() to add subregions.
+ *
+ * Use of this function is now deprecated. All memory regions must be
+ * allocated using the memory_region_new() family of functions and not
+ * statically embedded in a larger struct.
  */
 void memory_region_init(MemoryRegion *mr,
                         Object *owner,
                         const char *name,
-                        uint64_t size);
+                        uint64_t size)
+    QEMU_DEPRECATED;
+
+
+/**
+ * memory_region_new: Allocate a memory region.
+ * @owner: the object that owns the memory region in the composition tree
+ * @name: used for debugging; not visible to the user or ABI
+ * @size: size of the region; any subregions beyond this size will be clipped
+ *
+ * The region typically acts as a container for other memory regions.  Use
+ * memory_region_add_subregion() to add subregions.
+ *
+ * The returned memory region will have a single reference, which is
+ * held by the @owner object in the QOM composition tree. Thus in the
+ * absence of any further references being acquired, the memory region
+ * will be freed when @owner is freed
+ *
+ * Returns: the newly allocated memory region
+ */
+MemoryRegion *memory_region_new(Object *owner,
+                                const char *name,
+                                uint64_t size);
 
 /**
  * memory_region_ref: Add 1 to a memory region's reference count
@@ -1344,11 +1369,7 @@ void memory_region_ref(MemoryRegion *mr);
 void memory_region_unref(MemoryRegion *mr);
 
 /**
- * memory_region_init_io: Initialize an I/O memory region.
- *
- * Accesses into the region will cause the callbacks in @ops to be called.
- * if @size is nonzero, subregions will be clipped to @size.
- *
+ * memory_region_init_io: Initialize an I/O memory region
  * @mr: the #MemoryRegion to be initialized.
  * @owner: the object that tracks the region's reference count
  * @ops: a structure containing read and write callbacks to be used when
@@ -1356,13 +1377,46 @@ void memory_region_unref(MemoryRegion *mr);
  * @opaque: passed to the read and write callbacks of the @ops structure.
  * @name: used for debugging; not visible to the user or ABI
  * @size: size of the region.
+ *
+ * Accesses into the region will cause the callbacks in @ops to be called.
+ * if @size is nonzero, subregions will be clipped to @size.
+ *
+ * Use of this function is now deprecated. All memory regions must be
+ * allocated using the memory_region_new() family of functions and not
+ * statically embedded in a larger struct.
  */
 void memory_region_init_io(MemoryRegion *mr,
                            Object *owner,
                            const MemoryRegionOps *ops,
                            void *opaque,
                            const char *name,
-                           uint64_t size);
+                           uint64_t size)
+    QEMU_DEPRECATED;
+
+/**
+ * memory_region_new_io: Allocates an I/O memory region
+ * @owner: the object that tracks the region's reference count
+ * @ops: a structure containing read and write callbacks to be used when
+ *       I/O is performed on the region.
+ * @opaque: passed to the read and write callbacks of the @ops structure.
+ * @name: used for debugging; not visible to the user or ABI
+ * @size: size of the region.
+ *
+ * Accesses into the region will cause the callbacks in @ops to be called.
+ * if @size is nonzero, subregions will be clipped to @size.
+ *
+ * The returned memory region will have a single reference, which is
+ * held by the @owner object in the QOM composition tree. Thus in the
+ * absence of any further references being acquired, the memory region
+ * will be freed when @owner is freed
+ *
+ * Returns: the newly allocated memory region
+ */
+MemoryRegion *memory_region_new_io(Object *owner,
+                                   const MemoryRegionOps *ops,
+                                   void *opaque,
+                                   const char *name,
+                                   uint64_t size);
 
 /**
  * memory_region_init_ram_flags_nomigrate:  Initialize RAM memory region.
diff --git a/system/memory.c b/system/memory.c
index 739ba11da6..e8a539e37e 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -1214,6 +1214,18 @@ static char *memory_region_escape_name(const char *name)
     return escaped;
 }
 
+static char *memory_region_make_child(Object **owner,
+                                      const char *name)
+{
+    g_autofree char *escaped_name = memory_region_escape_name(name);
+
+    if (!*owner) {
+        *owner = machine_get_container("unattached");
+    }
+
+    return g_strdup_printf("%s[*]", escaped_name);
+}
+
 static void memory_region_do_init(MemoryRegion *mr,
                                   Object *owner,
                                   const char *name,
@@ -1227,20 +1239,6 @@ static void memory_region_do_init(MemoryRegion *mr,
     mr->owner = owner;
     mr->dev = (DeviceState *) object_dynamic_cast(mr->owner, TYPE_DEVICE);
     mr->ram_block = NULL;
-
-    if (name) {
-        char *escaped_name = memory_region_escape_name(name);
-        char *name_array = g_strdup_printf("%s[*]", escaped_name);
-
-        if (!owner) {
-            owner = machine_get_container("unattached");
-        }
-
-        object_property_add_child(owner, name_array, OBJECT(mr));
-        object_unref(OBJECT(mr));
-        g_free(name_array);
-        g_free(escaped_name);
-    }
 }
 
 void memory_region_init(MemoryRegion *mr,
@@ -1250,6 +1248,31 @@ void memory_region_init(MemoryRegion *mr,
 {
     object_initialize(mr, sizeof(*mr), TYPE_MEMORY_REGION);
     memory_region_do_init(mr, owner, name, size);
+    if (name) {
+        g_autofree char *childname = memory_region_make_child(&owner, name);
+        object_property_add_child(owner, childname, OBJECT(mr));
+        object_unref(OBJECT(mr));
+    }
+}
+
+MemoryRegion *memory_region_new(Object *owner,
+                                const char *name,
+                                uint64_t size)
+{
+    /*
+     * error_abort is safe, because 'childname' includes a wildcard
+     * for dynamically assigning a unique name. Thus adding the child
+     * property cannot fail
+     */
+    MemoryRegion *mr = MEMORY_REGION(object_new(TYPE_MEMORY_REGION));
+    memory_region_do_init(mr, owner, name, size);
+    if (name) {
+        g_autofree char *childname = memory_region_make_child(&owner, name);
+        object_property_add_child(owner, childname, OBJECT(mr));
+        object_unref(OBJECT(mr));
+
+    }
+    return mr;
 }
 
 static void memory_region_get_container(Object *obj, Visitor *v,
@@ -1574,10 +1597,22 @@ void memory_region_init_io(MemoryRegion *mr, Object *owner,
                            const char *name, uint64_t size)
 {
     g_assert(!ops || !(ops->impl.unaligned && !ops->valid.unaligned));
+    QEMU_DEPRECATIONS_OFF;
     memory_region_init(mr, owner, name, size);
+    QEMU_DEPRECATIONS_ON;
     memory_region_set_ops(mr, ops, opaque);
 }
 
+MemoryRegion *memory_region_new_io(Object *owner,
+                                   const MemoryRegionOps *ops, void *opaque,
+                                   const char *name, uint64_t size)
+{
+    g_assert(!ops || !(ops->impl.unaligned && !ops->valid.unaligned));
+    MemoryRegion *mr = memory_region_new(owner, name, size);
+    memory_region_set_ops(mr, ops, opaque);
+    return mr;
+}
+
 static bool memory_region_set_ram_block(MemoryRegion *mr, RAMBlock *rb)
 {
     mr->terminates = true;
@@ -1597,7 +1632,9 @@ bool memory_region_init_ram_flags_nomigrate(MemoryRegion *mr, Object *owner,
 {
     RAMBlock *rb;
 
+    QEMU_DEPRECATIONS_OFF;
     memory_region_init(mr, owner, name, size);
+    QEMU_DEPRECATIONS_ON;
     mr->ram = true;
     rb = qemu_ram_alloc(size, ram_flags, mr, errp);
     return memory_region_set_ram_block(mr, rb);
@@ -1615,7 +1652,9 @@ bool memory_region_init_resizeable_ram(MemoryRegion *mr,
 {
     RAMBlock *rb;
 
+    QEMU_DEPRECATIONS_OFF;
     memory_region_init(mr, owner, name, size);
+    QEMU_DEPRECATIONS_ON;
     mr->ram = true;
     rb = qemu_ram_alloc_resizeable(size, max_size, resized, mr, errp);
     return memory_region_set_ram_block(mr, rb);
@@ -1630,7 +1669,9 @@ bool memory_region_init_ram_from_file(MemoryRegion *mr, Object *owner,
 {
     RAMBlock *rb;
 
+    QEMU_DEPRECATIONS_OFF;
     memory_region_init(mr, owner, name, size);
+    QEMU_DEPRECATIONS_ON;
     mr->ram = true;
     mr->readonly = !!(ram_flags & RAM_READONLY);
     mr->align = align;
@@ -1645,7 +1686,9 @@ bool memory_region_init_ram_from_fd(MemoryRegion *mr, Object *owner,
 {
     RAMBlock *rb;
 
+    QEMU_DEPRECATIONS_OFF;
     memory_region_init(mr, owner, name, size);
+    QEMU_DEPRECATIONS_ON;
     mr->ram = true;
     mr->readonly = !!(ram_flags & RAM_READONLY);
     rb = qemu_ram_alloc_from_fd(size, size, NULL, mr, ram_flags, fd, offset,
@@ -1667,7 +1710,9 @@ void memory_region_init_ram_ptr(MemoryRegion *mr, Object *owner,
                                 const char *name, uint64_t size,
                                 void *ptr)
 {
+    QEMU_DEPRECATIONS_OFF;
     memory_region_init(mr, owner, name, size);
+    QEMU_DEPRECATIONS_ON;
     mr->ram = true;
     memory_region_set_ram_ptr(mr, size, ptr);
 }
@@ -1676,7 +1721,9 @@ void memory_region_init_ram_device_ptr(MemoryRegion *mr, Object *owner,
                                        const char *name, uint64_t size,
                                        void *ptr)
 {
+    QEMU_DEPRECATIONS_OFF;
     memory_region_init_io(mr, owner, &ram_device_mem_ops, mr, name, size);
+    QEMU_DEPRECATIONS_ON;
     mr->ram = true;
     mr->ram_device = true;
     memory_region_set_ram_ptr(mr, size, ptr);
@@ -1686,7 +1733,9 @@ void memory_region_init_alias(MemoryRegion *mr, Object *owner,
                               const char *name, MemoryRegion *orig,
                               hwaddr offset, uint64_t size)
 {
+    QEMU_DEPRECATIONS_OFF;
     memory_region_init(mr, owner, name, size);
+    QEMU_DEPRECATIONS_ON;
     mr->alias = orig;
     mr->alias_offset = offset;
 }
@@ -1704,6 +1753,12 @@ void memory_region_init_iommu(void *_iommu_mr,
     object_initialize(_iommu_mr, instance_size, mrtypename);
     mr = MEMORY_REGION(_iommu_mr);
     memory_region_do_init(mr, owner, name, size);
+    if (name) {
+        g_autofree char *childname = memory_region_make_child(&owner, name);
+        object_property_add_child(owner, childname, OBJECT(mr));
+        object_unref(OBJECT(mr));
+    }
+
     iommu_mr = IOMMU_MEMORY_REGION(mr);
     mr->terminates = true;  /* then re-forwards */
     QLIST_INIT(&iommu_mr->iommu_notify);
@@ -3703,7 +3758,9 @@ bool memory_region_init_rom_device(MemoryRegion *mr, Object *owner,
     RAMBlock *rb;
 
     assert(ops);
+    QEMU_DEPRECATIONS_OFF;
     memory_region_init_io(mr, owner, ops, opaque, name, size);
+    QEMU_DEPRECATIONS_ON;
     rb = qemu_ram_alloc(size, 0, mr, errp);
     if (memory_region_set_ram_block(mr, rb)) {
         mr->rom_device = true;
-- 
2.54.0

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

[Paolo Bonzini]

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é <berrange@redhat.com>

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.

Paolo

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

[BALATON Zoltan]

[-- Attachment #1: Type: text/plain, Size: 3671 bytes --]

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é <berrange@redhat.com>
>
> 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.

The problem is that memory regions cannot be ref counted because they are 
piggybacking on their owner's ref count instead of using their own. So 
while memory regions are QOM objects and the code is there to ref count 
and free them with their owners this can't be used and we hack around that 
by referencing the owner instead. If there's a dependency on the parent 
then maybe the memory region should take a ref of the parent so it does 
not go away until the memory region is freed or the owner relation needs 
to be taken into account instead of using the parent ref count and 
forbidding to make memory regions proper QOM objects which is very 
confusing and leads to all sorts of other hacks.

And it also forces me to embed a bunch of memory regions in device states 
that are otherwise not needed and already stored as PCI BARs or sysbus 
regions so I should not need them in my device state. This added 
complexity was my motivation but it turned out to be a larger issue and I 
think restoring the ability to ref count memory regions and clean up the 
hacks of keeping the parent alive with every object using its own ref 
counts would be cleaner than keeping to the current way.

Regards,
BALATON Zoltan

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

[BALATON Zoltan]

[-- Attachment #1: Type: text/plain, Size: 4181 bytes --]

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é <berrange@redhat.com>
>
> 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

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

[Akihiko Odaki]

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é <berrange@redhat.com>
>>
>> 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/48a2ec70-8995-40c5-8224-aa82f267e9dc@rsg.ci.i.u-tokyo.ac.jp/

Regards,
Akihiko Odaki

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

[Paolo Bonzini]

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

Paolo

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

[BALATON Zoltan]

[-- Attachment #1: Type: text/plain, Size: 6937 bytes --]

On Fri, 19 Jun 2026, 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.

Your replies are too brief to understand what you mean so I have to keep 
guessing. What breakage do you mean? The idea itself is not broken and 
would work and can be fixed to work as intended. It's just not working 
currently because memory regions are not allocated in a way that ref 
counting could free them. That's broken in QEMU currently but the 
above described way is not fundamentally broken. Or do you mean something 
else?

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

I don't know this part but I think ref counting is meant to be the way to 
track if there are pending accesses. How else would PCI code know if there 
are pending accesses from other unrelated parts of QEMU? I don't think PCI 
code could track all accesses itself which may come from a CPU or another 
device doing DMA. Even after registering as a BAR a memory region is just 
used as any other memory region so the PCI code does not know what 
accesses it without some tracking such as ref counts.

I think the current way of passing reference to the owner could work for a 
chain of owners too. All that would be needed is to add an owner field to 
QOM object and if that is non-NULL ref/unref is passed to the owner. I.e. 
just generalising what memory_region_ref/unref does. (Or do this for all 
prarent/child where children increase their parents' ref when they get a 
ref, but I'm not sure what exactly parent/child stands for in QOM so a 
separate owner relation may be needed as parent/child may be for 
qom-tree or not even well defined.)

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

OK so at least we've arrived at that there's no other problem than 
aesthetics. It's hard to argue about that but QOM already has two ways. 
From https://www.qemu.org/docs/master/devel/qom-api.html#qom-api

object_new:
This function will initialize a new object using heap allocated memory. 
The returned object has a reference count of 1, and will be freed when the 
last reference is dropped.

object_initialize:
This function will initialize an object. The memory for the object should 
have already been allocated. The returned object has a reference count of 
1, and will be finalized when the last reference is dropped.

So we should have corresponding API for memory_region too to support both 
allocated memory regions and ones managed by some other way. Currently the 
memory region API does not allow QOM managed memory regions so we're 
causing ourselves problems having to somehow manage the lifetime of these 
memory regions instead of just relying on the solution QOM already 
provides for this. Having both APIs would be consistent with QOM and allow 
to chose the way that fits each case better. I don't want to convert 
everything to object_new but I want to be able to allocate memory regions 
that are then managed by QOM so I can avoid the complexity of keeping 
track and managing them separately when this is already there and would 
work well for a lot of cases.

> This is different for Daniel's patches, which try to fix something by not 
> embedding, and I don't think they do.  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.

I'm not sure I understand all the problems but I don't think embedding 
solves anything other than not leaking memory regions that QOM does not 
free due to using object_initialize which means you're on your own 
managing your storage. I think a solution could be passing refs up in a 
chain of owners as I described above which is unrelated to embedding. This 
way works for a single owner with memory_region_ref/unref and could work 
for multiple owners while keeping the memory regions and owners separate 
objects. I'm not sure what you have in mind but if you say that owners 
would have to embed their children that alone does not solve that they are 
still separate objects and have to be finalized separately so we still 
need ref counting to know how long to keep alive owners when one of the 
children is accessed. Embedding does not solve that and just unnecessarily 
ties storage of separte objects together but we still need to fix ref 
counting and then we can also use that to free objects when no more 
references are there so we also don't need to embed them but keeping both 
_new and _init APIs would allow to chose either way whichever fits a 
given problem.

Regards.
BALATON Zoltan

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

[Akihiko Odaki]

On 2026/06/20 1:32, 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.  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.

I think we can stack follow-up patches that reject accesses to 
MemoryRegions owned by unrealized devices. That would make this a more 
complete solution. Accessing an unrealized device is an unsafe corner 
case anyway, so it is better to close it explicitly.

memory_region_init_ram_ptr() and memory_region_init_ram_device_ptr() are 
a little more complicated, because direct accesses to those regions can 
happen outside the BQL. Still, we can unmap the memory when the 
MemoryRegion is finalized, as memory_region_init_ram() already does.

However, after looking into this further, I found one complication:
x-vfio-user-server. In that case, the unmap timing is controlled by
libvfio-user, so QEMU cannot keep the memory mapped until the 
MemoryRegion reference is dropped. This is not a new issue; it is a 
pre-existing use-after-free hazard. I think a practical fix would be to 
extend the libvfio-user API to support asynchronous unmapping.

Cc'ing the vfio-user maintainers.

Regards,
Akihiko Odaki

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

[Thanos Makatos]

> -----Original Message-----
> From: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> Sent: 22 June 2026 15:37
> To: Paolo Bonzini <pbonzini@redhat.com>; BALATON Zoltan
> <balaton@eik.bme.hu>
> Cc: Daniel P. Berrangé <berrange@redhat.com>; qemu-devel@nongnu.org;
> Philippe Mathieu-Daudé <philmd@mailo.com>; Pierrick Bouvier
> <pierrick.bouvier@oss.qualcomm.com>; Peter Xu <peterx@redhat.com>;
> Hervé Poussineau <hpoussin@reactos.org>; Alex Bennée
> <alex.bennee@linaro.org>; Michael S. Tsirkin <mst@redhat.com>; Aurelien
> Jarno <aurelien@aurel32.net>; Fabiano Rosas <farosas@suse.de>; Mark Cave-
> Ayland <mark.cave-ayland@ilande.co.uk>; Marc-André Lureau
> <marcandre.lureau@redhat.com>; John Levon <john.levon@nutanix.com>;
> Thanos Makatos <thanos.makatos@nutanix.com>; Cédric Le Goater
> <clg@redhat.com>
> Subject: Re: [RFC 4/7] system: add memory_region_new /
> memory_region_new_io
> 
> !-------------------------------------------------------------------|
>   CAUTION: External Email
> 
> |-------------------------------------------------------------------!
> 
> On 2026/06/20 1:32, 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.  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.
> 
> I think we can stack follow-up patches that reject accesses to
> MemoryRegions owned by unrealized devices. That would make this a more
> complete solution. Accessing an unrealized device is an unsafe corner
> case anyway, so it is better to close it explicitly.
> 
> memory_region_init_ram_ptr() and memory_region_init_ram_device_ptr()
> are
> a little more complicated, because direct accesses to those regions can
> happen outside the BQL. Still, we can unmap the memory when the
> MemoryRegion is finalized, as memory_region_init_ram() already does.
> 
> However, after looking into this further, I found one complication:
> x-vfio-user-server. In that case, the unmap timing is controlled by
> libvfio-user, so QEMU cannot keep the memory mapped until the
> MemoryRegion reference is dropped. This is not a new issue; it is a
> pre-existing use-after-free hazard. I think a practical fix would be to
> extend the libvfio-user API to support asynchronous unmapping.
> 
> Cc'ing the vfio-user maintainers.

VFIO_USER_DMA_UNMAP is already asynchronous, it sends the response only after it's done unmapping. What would that asynchronous unmapping API look like? VFIO_USER_DMA_UNMAP would send a respond that it received the operation and started unmapping, and then another one to say it's done unmapping?
Also, since (AFAIK) x-vfio-user-server is orphaned is it worth putting the effort in this?

> 
> Regards,
> Akihiko Odaki

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

[Akihiko Odaki]

On 2026/06/23 6:24, Thanos Makatos wrote:
>> -----Original Message-----
>> From: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
>> Sent: 22 June 2026 15:37
>> To: Paolo Bonzini <pbonzini@redhat.com>; BALATON Zoltan
>> <balaton@eik.bme.hu>
>> Cc: Daniel P. Berrangé <berrange@redhat.com>; qemu-devel@nongnu.org;
>> Philippe Mathieu-Daudé <philmd@mailo.com>; Pierrick Bouvier
>> <pierrick.bouvier@oss.qualcomm.com>; Peter Xu <peterx@redhat.com>;
>> Hervé Poussineau <hpoussin@reactos.org>; Alex Bennée
>> <alex.bennee@linaro.org>; Michael S. Tsirkin <mst@redhat.com>; Aurelien
>> Jarno <aurelien@aurel32.net>; Fabiano Rosas <farosas@suse.de>; Mark Cave-
>> Ayland <mark.cave-ayland@ilande.co.uk>; Marc-André Lureau
>> <marcandre.lureau@redhat.com>; John Levon <john.levon@nutanix.com>;
>> Thanos Makatos <thanos.makatos@nutanix.com>; Cédric Le Goater
>> <clg@redhat.com>
>> Subject: Re: [RFC 4/7] system: add memory_region_new /
>> memory_region_new_io
>>
>> !-------------------------------------------------------------------|
>>    CAUTION: External Email
>>
>> |-------------------------------------------------------------------!
>>
>> On 2026/06/20 1:32, 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.  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.
>>
>> I think we can stack follow-up patches that reject accesses to
>> MemoryRegions owned by unrealized devices. That would make this a more
>> complete solution. Accessing an unrealized device is an unsafe corner
>> case anyway, so it is better to close it explicitly.
>>
>> memory_region_init_ram_ptr() and memory_region_init_ram_device_ptr()
>> are
>> a little more complicated, because direct accesses to those regions can
>> happen outside the BQL. Still, we can unmap the memory when the
>> MemoryRegion is finalized, as memory_region_init_ram() already does.
>>
>> However, after looking into this further, I found one complication:
>> x-vfio-user-server. In that case, the unmap timing is controlled by
>> libvfio-user, so QEMU cannot keep the memory mapped until the
>> MemoryRegion reference is dropped. This is not a new issue; it is a
>> pre-existing use-after-free hazard. I think a practical fix would be to
>> extend the libvfio-user API to support asynchronous unmapping.
>>
>> Cc'ing the vfio-user maintainers.
> 
> VFIO_USER_DMA_UNMAP is already asynchronous, it sends the response only after it's done unmapping. What would that asynchronous unmapping API look like? VFIO_USER_DMA_UNMAP would send a respond that it received the operation and started unmapping, and then another one to say it's done unmapping?

The problem is of libvfio-user's API, not the vfio-user protocol. 
vfu_setup_device_dma() takes a DMA region unregistration callback, which 
is expected to synchronously unmap the region from the guest, and 
x-vfio-user-server attempts to do so by calling object_unparent().

However, object_unparent() does not actually synchronously unmap the 
region from the guest so it results in a use-after-free hazard. A 
possible solution is to have the following three changes in combination:
- This series, ensuring the MemoryRegion stays alive
- A change of memory_region_init_ram_ptr() to add a callback for
   asynchronous unmapping
- A libvfio-user API change to asynchronously complete unmapping

> Also, since (AFAIK) x-vfio-user-server is orphaned is it worth putting the effort in this?

I rechecked MAINTAINERS and found its status is Odd Fixes, not Orphan. 
That said, it is possible to leave a FIXME comment instead of fixing it 
right now, as it's already done by virtio-gpu-rutabaga:

 > There is small risk of the MemoryRegion dereferencing the pointer
 > after rutabaga unmaps it. Please see discussion here:
 >
 > https://lists.gnu.org/archive/html/qemu-devel/2023-09/msg05141.html
 >
 > It is highly unlikely to happen in practice and doesn't affect known
 > use cases. However, it should be fixed and is noted here for
 > posterity.

A difference from virtio-gpu-rutabaga is that the Rutabaga library API 
does not interfere with asynchronous unmapping so it can be fixed 
entirely on the QEMU side. On contrary, anyone who would want to fix 
x-vfio-user-server would need to change the libvfio-user API because its 
current form is too restrictive.

Regards,
Akihiko Odaki

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

[Thanos Makatos]

> -----Original Message-----
> From: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> Sent: 23 June 2026 06:42
> To: Thanos Makatos <thanos.makatos@nutanix.com>; Paolo Bonzini
> <pbonzini@redhat.com>; BALATON Zoltan <balaton@eik.bme.hu>
> Cc: Daniel P. Berrangé <berrange@redhat.com>; qemu-devel@nongnu.org;
> Philippe Mathieu-Daudé <philmd@mailo.com>; Pierrick Bouvier
> <pierrick.bouvier@oss.qualcomm.com>; Peter Xu <peterx@redhat.com>;
> Hervé Poussineau <hpoussin@reactos.org>; Alex Bennée
> <alex.bennee@linaro.org>; Michael S. Tsirkin <mst@redhat.com>; Aurelien
> Jarno <aurelien@aurel32.net>; Fabiano Rosas <farosas@suse.de>; Mark Cave-
> Ayland <mark.cave-ayland@ilande.co.uk>; Marc-André Lureau
> <marcandre.lureau@redhat.com>; John Levon <john.levon@nutanix.com>;
> Cédric Le Goater <clg@redhat.com>
> Subject: Re: [RFC 4/7] system: add memory_region_new /
> memory_region_new_io
> 
> !-------------------------------------------------------------------|
>   CAUTION: External Email
> 
> |-------------------------------------------------------------------!
> 
> On 2026/06/23 6:24, Thanos Makatos wrote:
> >> -----Original Message-----
> >> From: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> >> Sent: 22 June 2026 15:37
> >> To: Paolo Bonzini <pbonzini@redhat.com>; BALATON Zoltan
> >> <balaton@eik.bme.hu>
> >> Cc: Daniel P. Berrangé <berrange@redhat.com>; qemu-devel@nongnu.org;
> >> Philippe Mathieu-Daudé <philmd@mailo.com>; Pierrick Bouvier
> >> <pierrick.bouvier@oss.qualcomm.com>; Peter Xu <peterx@redhat.com>;
> >> Hervé Poussineau <hpoussin@reactos.org>; Alex Bennée
> >> <alex.bennee@linaro.org>; Michael S. Tsirkin <mst@redhat.com>; Aurelien
> >> Jarno <aurelien@aurel32.net>; Fabiano Rosas <farosas@suse.de>; Mark
> Cave-
> >> Ayland <mark.cave-ayland@ilande.co.uk>; Marc-André Lureau
> >> <marcandre.lureau@redhat.com>; John Levon <john.levon@nutanix.com>;
> >> Thanos Makatos <thanos.makatos@nutanix.com>; Cédric Le Goater
> >> <clg@redhat.com>
> >> Subject: Re: [RFC 4/7] system: add memory_region_new /
> >> memory_region_new_io
> >>
> >> !-------------------------------------------------------------------|
> >>    CAUTION: External Email
> >>
> >> |-------------------------------------------------------------------!
> >>
> >> On 2026/06/20 1:32, 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.  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.
> >>
> >> I think we can stack follow-up patches that reject accesses to
> >> MemoryRegions owned by unrealized devices. That would make this a
> more
> >> complete solution. Accessing an unrealized device is an unsafe corner
> >> case anyway, so it is better to close it explicitly.
> >>
> >> memory_region_init_ram_ptr() and memory_region_init_ram_device_ptr()
> >> are
> >> a little more complicated, because direct accesses to those regions can
> >> happen outside the BQL. Still, we can unmap the memory when the
> >> MemoryRegion is finalized, as memory_region_init_ram() already does.
> >>
> >> However, after looking into this further, I found one complication:
> >> x-vfio-user-server. In that case, the unmap timing is controlled by
> >> libvfio-user, so QEMU cannot keep the memory mapped until the
> >> MemoryRegion reference is dropped. This is not a new issue; it is a
> >> pre-existing use-after-free hazard. I think a practical fix would be to
> >> extend the libvfio-user API to support asynchronous unmapping.
> >>
> >> Cc'ing the vfio-user maintainers.
> >
> > VFIO_USER_DMA_UNMAP is already asynchronous, it sends the response
> only after it's done unmapping. What would that asynchronous unmapping
> API look like? VFIO_USER_DMA_UNMAP would send a respond that it
> received the operation and started unmapping, and then another one to say
> it's done unmapping?
> 
> The problem is of libvfio-user's API, not the vfio-user protocol.
> vfu_setup_device_dma() takes a DMA region unregistration callback, which
> is expected to synchronously unmap the region from the guest, and
> x-vfio-user-server attempts to do so by calling object_unparent().

I misunderstood.

libvfio-user provides an optional mechanism where the device is asked to quiesce for certain operations, including VFIO_USER_DMA_UNMAP. Once quiesced, the device must not modify state. If the device cannot quiesce immediately it can continue operating and once ready to quiesce it can call a libvfio-user function to tell the library it's quiesced. Would that solve the problem?

> 
> However, object_unparent() does not actually synchronously unmap the
> region from the guest so it results in a use-after-free hazard. A
> possible solution is to have the following three changes in combination:
> - This series, ensuring the MemoryRegion stays alive
> - A change of memory_region_init_ram_ptr() to add a callback for
>    asynchronous unmapping
> - A libvfio-user API change to asynchronously complete unmapping
> 
> > Also, since (AFAIK) x-vfio-user-server is orphaned is it worth putting the
> effort in this?
> 
> I rechecked MAINTAINERS and found its status is Odd Fixes, not Orphan.
> That said, it is possible to leave a FIXME comment instead of fixing it
> right now, as it's already done by virtio-gpu-rutabaga:
> 
>  > There is small risk of the MemoryRegion dereferencing the pointer
>  > after rutabaga unmaps it. Please see discussion here:
>  >
>  > https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-
> 2D09_msg05141.html&d=DwIDaQ&c=s883GpUCOChKOHiocYtGcg&r=XTpYsh
> 5Ps2zJvtw6ogtti46atk736SI4vgsJiUKIyDE&m=t9M_TxgBz_6K8y6rUD0ypaL_El
> PVMTV5GPtiCVHrHb9BMKzTkgWiOKmcvA812-
> 5H&s=4N2t0WV5D358WZZsjHWVvc6CwQrYydmofzhTAlZ4eVM&e=
>  >
>  > It is highly unlikely to happen in practice and doesn't affect known
>  > use cases. However, it should be fixed and is noted here for
>  > posterity.
> 
> A difference from virtio-gpu-rutabaga is that the Rutabaga library API
> does not interfere with asynchronous unmapping so it can be fixed
> entirely on the QEMU side. On contrary, anyone who would want to fix
> x-vfio-user-server would need to change the libvfio-user API because its
> current form is too restrictive.
> 
> Regards,
> Akihiko Odaki

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

[Akihiko Odaki]

On 2026/06/23 21:58, Thanos Makatos wrote:
>> -----Original Message-----
>> From: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
>> Sent: 23 June 2026 06:42
>> To: Thanos Makatos <thanos.makatos@nutanix.com>; Paolo Bonzini
>> <pbonzini@redhat.com>; BALATON Zoltan <balaton@eik.bme.hu>
>> Cc: Daniel P. Berrangé <berrange@redhat.com>; qemu-devel@nongnu.org;
>> Philippe Mathieu-Daudé <philmd@mailo.com>; Pierrick Bouvier
>> <pierrick.bouvier@oss.qualcomm.com>; Peter Xu <peterx@redhat.com>;
>> Hervé Poussineau <hpoussin@reactos.org>; Alex Bennée
>> <alex.bennee@linaro.org>; Michael S. Tsirkin <mst@redhat.com>; Aurelien
>> Jarno <aurelien@aurel32.net>; Fabiano Rosas <farosas@suse.de>; Mark Cave-
>> Ayland <mark.cave-ayland@ilande.co.uk>; Marc-André Lureau
>> <marcandre.lureau@redhat.com>; John Levon <john.levon@nutanix.com>;
>> Cédric Le Goater <clg@redhat.com>
>> Subject: Re: [RFC 4/7] system: add memory_region_new /
>> memory_region_new_io
>>
>> !-------------------------------------------------------------------|
>>    CAUTION: External Email
>>
>> |-------------------------------------------------------------------!
>>
>> On 2026/06/23 6:24, Thanos Makatos wrote:
>>>> -----Original Message-----
>>>> From: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
>>>> Sent: 22 June 2026 15:37
>>>> To: Paolo Bonzini <pbonzini@redhat.com>; BALATON Zoltan
>>>> <balaton@eik.bme.hu>
>>>> Cc: Daniel P. Berrangé <berrange@redhat.com>; qemu-devel@nongnu.org;
>>>> Philippe Mathieu-Daudé <philmd@mailo.com>; Pierrick Bouvier
>>>> <pierrick.bouvier@oss.qualcomm.com>; Peter Xu <peterx@redhat.com>;
>>>> Hervé Poussineau <hpoussin@reactos.org>; Alex Bennée
>>>> <alex.bennee@linaro.org>; Michael S. Tsirkin <mst@redhat.com>; Aurelien
>>>> Jarno <aurelien@aurel32.net>; Fabiano Rosas <farosas@suse.de>; Mark
>> Cave-
>>>> Ayland <mark.cave-ayland@ilande.co.uk>; Marc-André Lureau
>>>> <marcandre.lureau@redhat.com>; John Levon <john.levon@nutanix.com>;
>>>> Thanos Makatos <thanos.makatos@nutanix.com>; Cédric Le Goater
>>>> <clg@redhat.com>
>>>> Subject: Re: [RFC 4/7] system: add memory_region_new /
>>>> memory_region_new_io
>>>>
>>>> !-------------------------------------------------------------------|
>>>>     CAUTION: External Email
>>>>
>>>> |-------------------------------------------------------------------!
>>>>
>>>> On 2026/06/20 1:32, 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.  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.
>>>>
>>>> I think we can stack follow-up patches that reject accesses to
>>>> MemoryRegions owned by unrealized devices. That would make this a
>> more
>>>> complete solution. Accessing an unrealized device is an unsafe corner
>>>> case anyway, so it is better to close it explicitly.
>>>>
>>>> memory_region_init_ram_ptr() and memory_region_init_ram_device_ptr()
>>>> are
>>>> a little more complicated, because direct accesses to those regions can
>>>> happen outside the BQL. Still, we can unmap the memory when the
>>>> MemoryRegion is finalized, as memory_region_init_ram() already does.
>>>>
>>>> However, after looking into this further, I found one complication:
>>>> x-vfio-user-server. In that case, the unmap timing is controlled by
>>>> libvfio-user, so QEMU cannot keep the memory mapped until the
>>>> MemoryRegion reference is dropped. This is not a new issue; it is a
>>>> pre-existing use-after-free hazard. I think a practical fix would be to
>>>> extend the libvfio-user API to support asynchronous unmapping.
>>>>
>>>> Cc'ing the vfio-user maintainers.
>>>
>>> VFIO_USER_DMA_UNMAP is already asynchronous, it sends the response
>> only after it's done unmapping. What would that asynchronous unmapping
>> API look like? VFIO_USER_DMA_UNMAP would send a respond that it
>> received the operation and started unmapping, and then another one to say
>> it's done unmapping?
>>
>> The problem is of libvfio-user's API, not the vfio-user protocol.
>> vfu_setup_device_dma() takes a DMA region unregistration callback, which
>> is expected to synchronously unmap the region from the guest, and
>> x-vfio-user-server attempts to do so by calling object_unparent().
> 
> I misunderstood.
> 
> libvfio-user provides an optional mechanism where the device is asked to quiesce for certain operations, including VFIO_USER_DMA_UNMAP. Once quiesced, the device must not modify state. If the device cannot quiesce immediately it can continue operating and once ready to quiesce it can call a libvfio-user function to tell the library it's quiesced. Would that solve the problem?

I don't think that solves this specific problem.

The quiesce callback would let QEMU explicitly tell libvfio-user when it 
is safe to proceed with the unmap. For example, QEMU could register a 
quiesce callback with:

     vfu_setup_device_quiesce_cb(o->vfu_ctx, quiesce_cb);

Then quiesce_cb() could return -EBUSY while QEMU prepares for the unmap, 
and later call:

     vfu_device_quiesced(vfu_ctx, 0);

At that point, libvfio-user would call dma_unregister(), and 
dma_unregister() would be expected to synchronously unmap the memory.

The difficulty is that this assumes QEMU has a primitive that can 
prepare the guest memory mapping so that `dma_unregister()` can later 
complete the unmap synchronously. I don't think we have such a 
primitive, and I don't think it can be implemented cleanly with the 
current memory API.

If this were protected by a conventional lock, QEMU could acquire the 
lock in quiesce_cb() and use the resulting critical section in 
dma_unregister() to update the guest memory mapping synchronously. But 
the memory API uses RCU instead. RCU does not give the updater an 
exclusive critical section where old readers are blocked and the old 
mapping is guaranteed to be gone before the function returns. Instead, 
the update completes asynchronously after a grace period. That is the 
point of RCU: readers avoid blocking, while updates complete later.

So I think the quiesce mechanism is not enough here, but its API shape 
is still useful. Concretely, libvfio-user could provide a variant of the 
DMA unregister callback that may complete asynchronously. QEMU would 
start the guest memory mapping update, return -1 with errno set to 
EBUSY, and then call something like vfu_device_unmapped() once the RCU 
grace period has elapsed. libvfio-user would free or reuse the memory 
only after that completion notification.

Regards,
Akihiko Odaki
> 
>>
>> However, object_unparent() does not actually synchronously unmap the
>> region from the guest so it results in a use-after-free hazard. A
>> possible solution is to have the following three changes in combination:
>> - This series, ensuring the MemoryRegion stays alive
>> - A change of memory_region_init_ram_ptr() to add a callback for
>>     asynchronous unmapping
>> - A libvfio-user API change to asynchronously complete unmapping
>>
>>> Also, since (AFAIK) x-vfio-user-server is orphaned is it worth putting the
>> effort in this?
>>
>> I rechecked MAINTAINERS and found its status is Odd Fixes, not Orphan.
>> That said, it is possible to leave a FIXME comment instead of fixing it
>> right now, as it's already done by virtio-gpu-rutabaga:
>>
>>   > There is small risk of the MemoryRegion dereferencing the pointer
>>   > after rutabaga unmaps it. Please see discussion here:
>>   >
>>   > https://urldefense.proofpoint.com/v2/url?u=https-
>> 3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-
>> 2D09_msg05141.html&d=DwIDaQ&c=s883GpUCOChKOHiocYtGcg&r=XTpYsh
>> 5Ps2zJvtw6ogtti46atk736SI4vgsJiUKIyDE&m=t9M_TxgBz_6K8y6rUD0ypaL_El
>> PVMTV5GPtiCVHrHb9BMKzTkgWiOKmcvA812-
>> 5H&s=4N2t0WV5D358WZZsjHWVvc6CwQrYydmofzhTAlZ4eVM&e=
>>   >
>>   > It is highly unlikely to happen in practice and doesn't affect known
>>   > use cases. However, it should be fixed and is noted here for
>>   > posterity.
>>
>> A difference from virtio-gpu-rutabaga is that the Rutabaga library API
>> does not interfere with asynchronous unmapping so it can be fixed
>> entirely on the QEMU side. On contrary, anyone who would want to fix
>> x-vfio-user-server would need to change the libvfio-user API because its
>> current form is too restrictive.
>>
>> Regards,
>> Akihiko Odaki

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

[Daniel P. Berrangé]

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 :|

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

[Daniel P. Berrangé]

On Wed, Jun 17, 2026 at 02:49:57PM +0200, 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é <berrange@redhat.com>
> 
> 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.

I see the opposite rationale.  We needlessly have two ways of
allocating QOM objects. The embedding does not magically keep
the child object alive, and it introduces confusion and unexpected
behaviour as ref counts don't work.

> For buses, the fact that the parent and child live at the same time is
> explicitly tracked with mutual refcount and unparent.

If we get mutual ref counting correct, then there is not benefit
to having 2 different ways of allocating QOM objects. We should
eliminate the embedding as pointless extra complexity and focus
on getting ref counting done correctly.

>                                                      If it's not, we have
> the hack that Akihiko mentioned
> (https://lists.gnu.org/archive/html/qemu-devel/2026-06/msg03459.html):

That hack is too gross. 

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

The relationship between objects does not need to be expressed with
embedding one inside another struct. This should be represented at
the class level with properties for the child relationships, which
would give us introspection via QMP too.  The embedding isn't adding
any real value IMHO.

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 :|

[RFC 5/7] system: add qemu_irq_new / qemu_irq_new_child / qemu_irq_new_array

[Daniel P. Berrangé]

Prepare for the move to dynamically allocated IRQ objects by
introducing qemu_irq_new / qemu_irq_new_child / qemu_irq_new_array
functions which call through to object_new instead of object_initialize.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 hw/core/irq.c         | 35 ++++++++++++++++++++
 include/hw/core/irq.h | 75 ++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 106 insertions(+), 4 deletions(-)

diff --git a/hw/core/irq.c b/hw/core/irq.c
index 106805e241..e943c87b81 100644
--- a/hw/core/irq.c
+++ b/hw/core/irq.c
@@ -49,6 +49,13 @@ void qemu_init_irq(IRQState *irq, qemu_irq_handler handler, void *opaque,
     init_irq_fields(irq, handler, opaque, n);
 }
 
+IRQState *qemu_irq_new(qemu_irq_handler handler, void *opaque, int n)
+{
+    IRQState *irq = IRQ(object_new(TYPE_IRQ));
+    init_irq_fields(irq, handler, opaque, n);
+    return irq;
+}
+
 void qemu_init_irq_child(Object *parent, const char *propname,
                          IRQState *irq, qemu_irq_handler handler,
                          void *opaque, int n)
@@ -57,14 +64,42 @@ void qemu_init_irq_child(Object *parent, const char *propname,
     init_irq_fields(irq, handler, opaque, n);
 }
 
+IRQState *qemu_irq_new_child(Object *parent, const char *propname,
+                             qemu_irq_handler handler,
+                             void *opaque, int n,
+                             Error **errp)
+{
+    IRQState *irq = IRQ(object_new_with_props(TYPE_IRQ, parent, propname,
+                                              errp, NULL));
+    if (!irq) {
+        return NULL;
+    }
+    init_irq_fields(irq, handler, opaque, n);
+    return irq;
+}
+
+
 void qemu_init_irqs(IRQState irq[], size_t count,
                     qemu_irq_handler handler, void *opaque)
 {
     for (size_t i = 0; i < count; i++) {
+        QEMU_DEPRECATIONS_OFF;
         qemu_init_irq(&irq[i], handler, opaque, i);
+        QEMU_DEPRECATIONS_ON;
     }
 }
 
+IRQState **qemu_irq_new_array(size_t count,
+                              qemu_irq_handler handler, void *opaque)
+{
+    IRQState **irqs = g_new0(IRQState *, count);
+    for (size_t i = 0; i < count; i++) {
+        irqs[i] = qemu_irq_new(handler, opaque, i);
+    }
+    return irqs;
+}
+
+
 qemu_irq *qemu_extend_irqs(qemu_irq *old, int n_old, qemu_irq_handler handler,
                            void *opaque, int n)
 {
diff --git a/include/hw/core/irq.h b/include/hw/core/irq.h
index 291fdd67df..af9bf6fb12 100644
--- a/include/hw/core/irq.h
+++ b/include/hw/core/irq.h
@@ -34,7 +34,12 @@ static inline void qemu_irq_pulse(qemu_irq irq)
     qemu_set_irq(irq, 0);
 }
 
-/*
+/**
+ * qemu_init_irq: Initialize IRQ
+ * @handler: handler function for incoming interrupts
+ * @opaque: opaque data to pass to @handler
+ * @n: interrupt number to pass to @handler
+ *
  * Init a single IRQ. The irq is assigned with a handler, an opaque data
  * and the interrupt number. The caller must free this with qemu_free_irq().
  * If you are using this inside a device's init or realize method, then
@@ -42,7 +47,21 @@ static inline void qemu_irq_pulse(qemu_irq irq)
  * to manually clean up the IRQ.
  */
 void qemu_init_irq(IRQState *irq, qemu_irq_handler handler, void *opaque,
-                   int n);
+                   int n)
+    QEMU_DEPRECATED;
+/**
+ * qemu_new_irq: Allocate IRQ
+ * @handler: handler function for incoming interrupts
+ * @opaque: opaque data to pass to @handler
+ * @n: interrupt number to pass to @handler
+ *
+ * The returned IRQ will have a single reference, which is held by the
+ * caller and must be released to free the returned IRQ object when
+ * no longer required.
+ *
+ * Returns: the newly allocated IRQ
+ */
+IRQState *qemu_irq_new(qemu_irq_handler handler, void *opaque, int n);
 
 /**
  * qemu_init_irq_child: Initialize IRQ and make it a QOM child
@@ -56,10 +75,38 @@ void qemu_init_irq(IRQState *irq, qemu_irq_handler handler, void *opaque,
  * Init a single IRQ and make the IRQ object a child of @parent with
  * the child-property name @propname. The IRQ object will thus be
  * automatically freed when @parent is destroyed.
+ *
+ * Use of this function is now deprecated. All IRQs must be
+ * allocated using the qemu_irq_new() family of functions and not
+ * statically embedded in a larger struct.
  */
 void qemu_init_irq_child(Object *parent, const char *propname,
                          IRQState *irq, qemu_irq_handler handler,
-                         void *opaque, int n);
+                         void *opaque, int n)
+    QEMU_DEPRECATED;
+
+/**
+ * qemu_init_irq_child: Allocate IRQ and make it a QOM child
+ * @parent: QOM object which owns this IRQ
+ * @propname: child property name
+ * @handler: handler function for incoming interrupts
+ * @opaque: opaque data to pass to @handler
+ * @n: interrupt number to pass to @handler
+ *
+ * Allocate a single IRQ and make the IRQ object a child of @parent with
+ * the child-property name @propname.
+ *
+ * The returned IRQ will have a single reference, which is held by the
+ * @owner object in the QOM composition tree. Thus in the absence of
+ * any further references being acquired, the IRQ will be freed when
+ * @owner is freed
+ *
+ * Returns: the newly allocated IRQ
+ */
+IRQState *qemu_irq_new_child(Object *parent, const char *propname,
+                             qemu_irq_handler handler,
+                             void *opaque, int n,
+                             Error **errp);
 
 
 /**
@@ -69,9 +116,29 @@ void qemu_init_irq_child(Object *parent, const char *propname,
  * @count: number of IRQs to initialize
  * @handler: handler to assign to each IRQ
  * @opaque: opaque data to pass to @handler
+ *
+ * Use of this function is now deprecated. All IRQs must be
+ * allocated using the qemu_irq_new() family of functions and not
+ * statically embedded in a larger struct.
  */
 void qemu_init_irqs(IRQState irq[], size_t count,
-                    qemu_irq_handler handler, void *opaque);
+                    qemu_irq_handler handler, void *opaque)
+    QEMU_DEPRECATED;
+/**
+ * qemu_irq_new_array: Allocate an array of IRQs.
+ * @count: number of IRQs to initialize
+ * @handler: handler to assign to each IRQ
+ * @opaque: opaque data to pass to @handler
+ *
+ * The returned IRQs will have a single reference, which is held by the
+ * caller and must be released to free the returned IRQs object when
+ * no longer required. The return array memory must also be freed by
+ * the caller.
+ *
+ * Returns: an array of IRQ objects
+ */
+IRQState **qemu_irq_new_array(size_t count,
+                              qemu_irq_handler handler, void *opaque);
 
 /* Returns an array of N IRQs. Each IRQ is assigned the argument handler and
  * opaque data.
-- 
2.54.0

Re: [RFC 5/7] system: add qemu_irq_new / qemu_irq_new_child / qemu_irq_new_array

[Peter Maydell]

On Tue, 16 Jun 2026 at 16:57, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> Prepare for the move to dynamically allocated IRQ objects by
> introducing qemu_irq_new / qemu_irq_new_child / qemu_irq_new_array
> functions which call through to object_new instead of object_initialize.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  hw/core/irq.c         | 35 ++++++++++++++++++++
>  include/hw/core/irq.h | 75 ++++++++++++++++++++++++++++++++++++++++---
>  2 files changed, 106 insertions(+), 4 deletions(-)
>
> diff --git a/hw/core/irq.c b/hw/core/irq.c
> index 106805e241..e943c87b81 100644
> --- a/hw/core/irq.c
> +++ b/hw/core/irq.c
> @@ -49,6 +49,13 @@ void qemu_init_irq(IRQState *irq, qemu_irq_handler handler, void *opaque,
>      init_irq_fields(irq, handler, opaque, n);
>  }
>
> +IRQState *qemu_irq_new(qemu_irq_handler handler, void *opaque, int n)
> +{
> +    IRQState *irq = IRQ(object_new(TYPE_IRQ));
> +    init_irq_fields(irq, handler, opaque, n);
> +    return irq;
> +}

Isn't this the same as the existing qemu_allocate_irq() ?

(I have over the past few years occasionally been trying to get rid
of existing uses of qemu_allocate_irq() and its cousin
qemu_allocate_irqs(), because they are persistent sources of memory
leaks. The function returns a pointer that the caller has to deal
with and remember to free, whereas using e.g. qdev_init_gpio_*()
makes the new irq objects children of the device they belong to,
so they're automatically freed when the device is destroyed.
qemu_init_irq_child() similarly.)

-- PMM

Re: [RFC 5/7] system: add qemu_irq_new / qemu_irq_new_child / qemu_irq_new_array

[Daniel P. Berrangé]

On Tue, Jun 16, 2026 at 05:22:05PM +0100, Peter Maydell wrote:
> On Tue, 16 Jun 2026 at 16:57, Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > Prepare for the move to dynamically allocated IRQ objects by
> > introducing qemu_irq_new / qemu_irq_new_child / qemu_irq_new_array
> > functions which call through to object_new instead of object_initialize.
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  hw/core/irq.c         | 35 ++++++++++++++++++++
> >  include/hw/core/irq.h | 75 ++++++++++++++++++++++++++++++++++++++++---
> >  2 files changed, 106 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/core/irq.c b/hw/core/irq.c
> > index 106805e241..e943c87b81 100644
> > --- a/hw/core/irq.c
> > +++ b/hw/core/irq.c
> > @@ -49,6 +49,13 @@ void qemu_init_irq(IRQState *irq, qemu_irq_handler handler, void *opaque,
> >      init_irq_fields(irq, handler, opaque, n);
> >  }
> >
> > +IRQState *qemu_irq_new(qemu_irq_handler handler, void *opaque, int n)
> > +{
> > +    IRQState *irq = IRQ(object_new(TYPE_IRQ));
> > +    init_irq_fields(irq, handler, opaque, n);
> > +    return irq;
> > +}
> 
> Isn't this the same as the existing qemu_allocate_irq() ?

Doh, I missed the very surprising naming pattern where "qemu_irq" is a
typedef of 'struct IRQState *'.

> (I have over the past few years occasionally been trying to get rid
> of existing uses of qemu_allocate_irq() and its cousin
> qemu_allocate_irqs(), because they are persistent sources of memory
> leaks. The function returns a pointer that the caller has to deal
> with and remember to free, whereas using e.g. qdev_init_gpio_*()
> makes the new irq objects children of the device they belong to,
> so they're automatically freed when the device is destroyed.
> qemu_init_irq_child() similarly.)

If we think it is best practice to mandate passing in a parent
object so that every IRQ  is gauranteed to be owned by the QOM
tree, that's fine with me.

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 :|

Re: [RFC 5/7] system: add qemu_irq_new / qemu_irq_new_child / qemu_irq_new_array

[Mark Cave-Ayland]

On 16/06/2026 17:36, Daniel P. Berrangé wrote:

>> Isn't this the same as the existing qemu_allocate_irq() ?
> 
> Doh, I missed the very surprising naming pattern where "qemu_irq" is a
> typedef of 'struct IRQState *'.
> 
>> (I have over the past few years occasionally been trying to get rid
>> of existing uses of qemu_allocate_irq() and its cousin
>> qemu_allocate_irqs(), because they are persistent sources of memory
>> leaks. The function returns a pointer that the caller has to deal
>> with and remember to free, whereas using e.g. qdev_init_gpio_*()
>> makes the new irq objects children of the device they belong to,
>> so they're automatically freed when the device is destroyed.
>> qemu_init_irq_child() similarly.)
> 
> If we think it is best practice to mandate passing in a parent
> object so that every IRQ  is gauranteed to be owned by the QOM
> tree, that's fine with me.

Yes, this feels like the right way forward to me.


ATB,

Mark.

Re: [RFC 5/7] system: add qemu_irq_new / qemu_irq_new_child / qemu_irq_new_array

[Mark Cave-Ayland]

On 16/06/2026 17:22, Peter Maydell wrote:

> On Tue, 16 Jun 2026 at 16:57, Daniel P. Berrangé <berrange@redhat.com> wrote:
>>
>> Prepare for the move to dynamically allocated IRQ objects by
>> introducing qemu_irq_new / qemu_irq_new_child / qemu_irq_new_array
>> functions which call through to object_new instead of object_initialize.
>>
>> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>> ---
>>   hw/core/irq.c         | 35 ++++++++++++++++++++
>>   include/hw/core/irq.h | 75 ++++++++++++++++++++++++++++++++++++++++---
>>   2 files changed, 106 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/core/irq.c b/hw/core/irq.c
>> index 106805e241..e943c87b81 100644
>> --- a/hw/core/irq.c
>> +++ b/hw/core/irq.c
>> @@ -49,6 +49,13 @@ void qemu_init_irq(IRQState *irq, qemu_irq_handler handler, void *opaque,
>>       init_irq_fields(irq, handler, opaque, n);
>>   }
>>
>> +IRQState *qemu_irq_new(qemu_irq_handler handler, void *opaque, int n)
>> +{
>> +    IRQState *irq = IRQ(object_new(TYPE_IRQ));
>> +    init_irq_fields(irq, handler, opaque, n);
>> +    return irq;
>> +}
> 
> Isn't this the same as the existing qemu_allocate_irq() ?
> 
> (I have over the past few years occasionally been trying to get rid
> of existing uses of qemu_allocate_irq() and its cousin
> qemu_allocate_irqs(), because they are persistent sources of memory
> leaks. The function returns a pointer that the caller has to deal
> with and remember to free, whereas using e.g. qdev_init_gpio_*()
> makes the new irq objects children of the device they belong to,
> so they're automatically freed when the device is destroyed.
> qemu_init_irq_child() similarly.)

Indeed, this is another case where we'd want to use 
qemu_init_irq_child() since then it automatically gets cleaned up when 
the refcount hits zero.

And your point about the similarity of qdev GPIOs is also valid: in 
particular with Marc-André's recent work on property arrays, it feels 
like we're getting closer to being able to converge everything IRQ and 
GPIO-related into a single refcounted implementation.


ATB,

Mark.

Re: [RFC 5/7] system: add qemu_irq_new / qemu_irq_new_child / qemu_irq_new_array

[Akihiko Odaki]

On 2026/06/17 0:55, Daniel P. Berrangé wrote:
> Prepare for the move to dynamically allocated IRQ objects by
> introducing qemu_irq_new / qemu_irq_new_child / qemu_irq_new_array
> functions which call through to object_new instead of object_initialize.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   hw/core/irq.c         | 35 ++++++++++++++++++++
>   include/hw/core/irq.h | 75 ++++++++++++++++++++++++++++++++++++++++---
>   2 files changed, 106 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/core/irq.c b/hw/core/irq.c
> index 106805e241..e943c87b81 100644
> --- a/hw/core/irq.c
> +++ b/hw/core/irq.c
> @@ -49,6 +49,13 @@ void qemu_init_irq(IRQState *irq, qemu_irq_handler handler, void *opaque,
>       init_irq_fields(irq, handler, opaque, n);
>   }
>   
> +IRQState *qemu_irq_new(qemu_irq_handler handler, void *opaque, int n)
> +{
> +    IRQState *irq = IRQ(object_new(TYPE_IRQ));
> +    init_irq_fields(irq, handler, opaque, n);
> +    return irq;
> +}
> +
>   void qemu_init_irq_child(Object *parent, const char *propname,
>                            IRQState *irq, qemu_irq_handler handler,
>                            void *opaque, int n)
> @@ -57,14 +64,42 @@ void qemu_init_irq_child(Object *parent, const char *propname,
>       init_irq_fields(irq, handler, opaque, n);
>   }
>   
> +IRQState *qemu_irq_new_child(Object *parent, const char *propname,
> +                             qemu_irq_handler handler,
> +                             void *opaque, int n,
> +                             Error **errp)
> +{
> +    IRQState *irq = IRQ(object_new_with_props(TYPE_IRQ, parent, propname,
> +                                              errp, NULL));

object_new_with_props() rejects a property name including [*]. 
hw/char/serial-pci-multi.c passes "irq[*]" to qemu_init_irq_child() so 
this prevents simply replacing it.

Regards,
Akihiko Odaki

> +    if (!irq) {
> +        return NULL;
> +    }
> +    init_irq_fields(irq, handler, opaque, n);
> +    return irq;
> +}
> +
> +
>   void qemu_init_irqs(IRQState irq[], size_t count,
>                       qemu_irq_handler handler, void *opaque)
>   {
>       for (size_t i = 0; i < count; i++) {
> +        QEMU_DEPRECATIONS_OFF;
>           qemu_init_irq(&irq[i], handler, opaque, i);
> +        QEMU_DEPRECATIONS_ON;
>       }
>   }
>   
> +IRQState **qemu_irq_new_array(size_t count,
> +                              qemu_irq_handler handler, void *opaque)
> +{
> +    IRQState **irqs = g_new0(IRQState *, count);
> +    for (size_t i = 0; i < count; i++) {
> +        irqs[i] = qemu_irq_new(handler, opaque, i);
> +    }
> +    return irqs;
> +}
> +
> +
>   qemu_irq *qemu_extend_irqs(qemu_irq *old, int n_old, qemu_irq_handler handler,
>                              void *opaque, int n)
>   {
> diff --git a/include/hw/core/irq.h b/include/hw/core/irq.h
> index 291fdd67df..af9bf6fb12 100644
> --- a/include/hw/core/irq.h
> +++ b/include/hw/core/irq.h
> @@ -34,7 +34,12 @@ static inline void qemu_irq_pulse(qemu_irq irq)
>       qemu_set_irq(irq, 0);
>   }
>   
> -/*
> +/**
> + * qemu_init_irq: Initialize IRQ
> + * @handler: handler function for incoming interrupts
> + * @opaque: opaque data to pass to @handler
> + * @n: interrupt number to pass to @handler
> + *
>    * Init a single IRQ. The irq is assigned with a handler, an opaque data
>    * and the interrupt number. The caller must free this with qemu_free_irq().
>    * If you are using this inside a device's init or realize method, then
> @@ -42,7 +47,21 @@ static inline void qemu_irq_pulse(qemu_irq irq)
>    * to manually clean up the IRQ.
>    */
>   void qemu_init_irq(IRQState *irq, qemu_irq_handler handler, void *opaque,
> -                   int n);
> +                   int n)
> +    QEMU_DEPRECATED;
> +/**
> + * qemu_new_irq: Allocate IRQ
> + * @handler: handler function for incoming interrupts
> + * @opaque: opaque data to pass to @handler
> + * @n: interrupt number to pass to @handler
> + *
> + * The returned IRQ will have a single reference, which is held by the
> + * caller and must be released to free the returned IRQ object when
> + * no longer required.
> + *
> + * Returns: the newly allocated IRQ
> + */
> +IRQState *qemu_irq_new(qemu_irq_handler handler, void *opaque, int n);
>   
>   /**
>    * qemu_init_irq_child: Initialize IRQ and make it a QOM child
> @@ -56,10 +75,38 @@ void qemu_init_irq(IRQState *irq, qemu_irq_handler handler, void *opaque,
>    * Init a single IRQ and make the IRQ object a child of @parent with
>    * the child-property name @propname. The IRQ object will thus be
>    * automatically freed when @parent is destroyed.
> + *
> + * Use of this function is now deprecated. All IRQs must be
> + * allocated using the qemu_irq_new() family of functions and not
> + * statically embedded in a larger struct.
>    */
>   void qemu_init_irq_child(Object *parent, const char *propname,
>                            IRQState *irq, qemu_irq_handler handler,
> -                         void *opaque, int n);
> +                         void *opaque, int n)
> +    QEMU_DEPRECATED;
> +
> +/**
> + * qemu_init_irq_child: Allocate IRQ and make it a QOM child
> + * @parent: QOM object which owns this IRQ
> + * @propname: child property name
> + * @handler: handler function for incoming interrupts
> + * @opaque: opaque data to pass to @handler
> + * @n: interrupt number to pass to @handler
> + *
> + * Allocate a single IRQ and make the IRQ object a child of @parent with
> + * the child-property name @propname.
> + *
> + * The returned IRQ will have a single reference, which is held by the
> + * @owner object in the QOM composition tree. Thus in the absence of
> + * any further references being acquired, the IRQ will be freed when
> + * @owner is freed
> + *
> + * Returns: the newly allocated IRQ
> + */
> +IRQState *qemu_irq_new_child(Object *parent, const char *propname,
> +                             qemu_irq_handler handler,
> +                             void *opaque, int n,
> +                             Error **errp);
>   
>   
>   /**
> @@ -69,9 +116,29 @@ void qemu_init_irq_child(Object *parent, const char *propname,
>    * @count: number of IRQs to initialize
>    * @handler: handler to assign to each IRQ
>    * @opaque: opaque data to pass to @handler
> + *
> + * Use of this function is now deprecated. All IRQs must be
> + * allocated using the qemu_irq_new() family of functions and not
> + * statically embedded in a larger struct.
>    */
>   void qemu_init_irqs(IRQState irq[], size_t count,
> -                    qemu_irq_handler handler, void *opaque);
> +                    qemu_irq_handler handler, void *opaque)
> +    QEMU_DEPRECATED;
> +/**
> + * qemu_irq_new_array: Allocate an array of IRQs.
> + * @count: number of IRQs to initialize
> + * @handler: handler to assign to each IRQ
> + * @opaque: opaque data to pass to @handler
> + *
> + * The returned IRQs will have a single reference, which is held by the
> + * caller and must be released to free the returned IRQs object when
> + * no longer required. The return array memory must also be freed by
> + * the caller.
> + *
> + * Returns: an array of IRQ objects
> + */
> +IRQState **qemu_irq_new_array(size_t count,
> +                              qemu_irq_handler handler, void *opaque);
>   
>   /* Returns an array of N IRQs. Each IRQ is assigned the argument handler and
>    * opaque data.

[RFC 6/7] hw/isa: convert PIIX embedded QOM objects to heap allocated

[Daniel P. Berrangé]

Update for the new preferred QOM design by removing embedded QOM
objects from the PIIXState struct for the RTC, IDE, UHCI, PM,
IRQ and memory region classes.

XXXX:  there seems to be little benefit in having any of the
instance fields for RTC, IDE, UHCI, PM & IRQ objects.  These
objects are all kept alive via the 'child' property which owns
the primary reference. The instance fields are accessed durnig
setup and then never again, so all these instances fields could
arguably go away entirely.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 hw/isa/piix.c                 | 65 ++++++++++++++++++++++-------------
 include/hw/southbridge/piix.h | 12 +++----
 2 files changed, 47 insertions(+), 30 deletions(-)

diff --git a/hw/isa/piix.c b/hw/isa/piix.c
index 31fa53e6a4..cd23486ef9 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -308,18 +308,22 @@ static void pci_piix_realize(PCIDevice *dev, const char *uhci_type,
         return;
     }
 
-    memory_region_init_io(&d->rcr_mem, OBJECT(dev), &rcr_ops, d,
-                          "piix-reset-control", 1);
+    d->rcr_mem = memory_region_new_io(OBJECT(dev), &rcr_ops, d,
+                                      "piix-reset-control", 1);
     memory_region_add_subregion_overlap(pci_address_space_io(dev),
-                                        PIIX_RCR_IOPORT, &d->rcr_mem, 1);
+                                        PIIX_RCR_IOPORT, d->rcr_mem, 1);
 
     /* PIC */
     if (d->has_pic) {
         qemu_irq *i8259;
 
-        qemu_init_irq_child(OBJECT(dev), "i8259-irq", &d->i8259_irq,
-                            piix_request_i8259_irq, d, 0);
-        i8259 = i8259_init(isa_bus, &d->i8259_irq);
+        d->i8259_irq = qemu_irq_new_child(OBJECT(dev), "i8259-irq",
+                                          piix_request_i8259_irq, d, 0,
+                                          errp);
+        if (!d->i8259_irq) {
+            return;
+        }
+        i8259 = i8259_init(isa_bus, d->i8259_irq);
 
         for (size_t i = 0; i < ISA_NUM_IRQS; i++) {
             d->isa_irqs_in[i] = i8259[i];
@@ -340,38 +344,45 @@ static void pci_piix_realize(PCIDevice *dev, const char *uhci_type,
     i8257_dma_init(OBJECT(dev), isa_bus, 0);
 
     /* RTC */
-    qdev_prop_set_int32(DEVICE(&d->rtc), "base_year", 2000);
-    if (!qdev_realize(DEVICE(&d->rtc), BUS(isa_bus), errp)) {
+    qdev_prop_set_int32(DEVICE(d->rtc), "base_year", 2000);
+    if (!qdev_realize(DEVICE(d->rtc), BUS(isa_bus), errp)) {
         return;
     }
-    irq = object_property_get_uint(OBJECT(&d->rtc), "irq", &error_fatal);
-    isa_connect_gpio_out(ISA_DEVICE(&d->rtc), 0, irq);
+    irq = object_property_get_uint(OBJECT(d->rtc), "irq", &error_fatal);
+    isa_connect_gpio_out(ISA_DEVICE(d->rtc), 0, irq);
 
     /* IDE */
-    qdev_prop_set_int32(DEVICE(&d->ide), "addr", dev->devfn + 1);
-    if (!qdev_realize(DEVICE(&d->ide), BUS(pci_bus), errp)) {
+    qdev_prop_set_int32(DEVICE(d->ide), "addr", dev->devfn + 1);
+    if (!qdev_realize(DEVICE(d->ide), BUS(pci_bus), errp)) {
         return;
     }
 
     /* USB */
     if (d->has_usb) {
-        object_initialize_child(OBJECT(dev), "uhci", &d->uhci, uhci_type);
-        qdev_prop_set_int32(DEVICE(&d->uhci), "addr", dev->devfn + 2);
-        if (!qdev_realize(DEVICE(&d->uhci), BUS(pci_bus), errp)) {
+        d->uhci = UHCI(object_new_with_props(
+                           uhci_type, OBJECT(d), "uhci", errp, NULL));
+        if (!d->uhci) {
+            return;
+        }
+        qdev_prop_set_int32(DEVICE(d->uhci), "addr", dev->devfn + 2);
+        if (!qdev_realize(DEVICE(d->uhci), BUS(pci_bus), errp)) {
             return;
         }
     }
 
     /* Power Management */
     if (d->has_acpi) {
-        object_initialize_child(OBJECT(d), "pm", &d->pm, TYPE_PIIX4_PM);
-        qdev_prop_set_int32(DEVICE(&d->pm), "addr", dev->devfn + 3);
-        qdev_prop_set_uint32(DEVICE(&d->pm), "smb_io_base", d->smb_io_base);
-        qdev_prop_set_bit(DEVICE(&d->pm), "smm-enabled", d->smm_enabled);
-        if (!qdev_realize(DEVICE(&d->pm), BUS(pci_bus), errp)) {
+        d->pm = PIIX4_PM(object_new_with_props(
+                             TYPE_PIIX4_PM, OBJECT(d), "pm", errp, NULL));
+        if (!d->pm) {
+        }
+        qdev_prop_set_int32(DEVICE(d->pm), "addr", dev->devfn + 3);
+        qdev_prop_set_uint32(DEVICE(d->pm), "smb_io_base", d->smb_io_base);
+        qdev_prop_set_bit(DEVICE(d->pm), "smm-enabled", d->smm_enabled);
+        if (!qdev_realize(DEVICE(d->pm), BUS(pci_bus), errp)) {
             return;
         }
-        qdev_connect_gpio_out(DEVICE(&d->pm), 0, d->isa_irqs_in[9]);
+        qdev_connect_gpio_out(DEVICE(d->pm), 0, d->isa_irqs_in[9]);
     }
 
     pci_bus_irqs(pci_bus, piix_set_pci_irq, d, PIIX_NUM_PIRQS);
@@ -406,7 +417,9 @@ static void pci_piix_init(Object *obj)
     qdev_init_gpio_out_named(DEVICE(obj), d->isa_irqs_in, "isa-irqs",
                              ISA_NUM_IRQS);
 
-    object_initialize_child(obj, "rtc", &d->rtc, TYPE_MC146818_RTC);
+    d->rtc = MC146818_RTC(
+        object_new_with_props(TYPE_MC146818_RTC, obj, "rtc",
+                              &error_abort, NULL));
 }
 
 static const Property pci_piix_props[] = {
@@ -462,7 +475,9 @@ static void piix3_init(Object *obj)
 {
     PIIXState *d = PIIX_PCI_DEVICE(obj);
 
-    object_initialize_child(obj, "ide", &d->ide, TYPE_PIIX3_IDE);
+    d->ide = PCI_IDE(
+        object_new_with_props(TYPE_PIIX3_IDE, obj, "ide",
+                              &error_abort, NULL));
 }
 
 static void piix3_class_init(ObjectClass *klass, const void *data)
@@ -492,7 +507,9 @@ static void piix4_init(Object *obj)
 {
     PIIXState *s = PIIX_PCI_DEVICE(obj);
 
-    object_initialize_child(obj, "ide", &s->ide, TYPE_PIIX4_IDE);
+    s->ide = PCI_IDE(
+        object_new_with_props(TYPE_PIIX4_IDE, obj, "ide",
+                              &error_abort, NULL));
 }
 
 static void piix4_class_init(ObjectClass *klass, const void *data)
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index a296b1205a..8bd24946de 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -53,15 +53,15 @@ struct PIIXState {
     qemu_irq cpu_intr;
     qemu_irq isa_irqs_in[ISA_NUM_IRQS];
 
-    IRQState i8259_irq;
+    IRQState *i8259_irq;
 
     /* This member isn't used. Just for save/load compatibility */
     int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS];
 
-    MC146818RtcState rtc;
-    PCIIDEState ide;
-    UHCIState uhci;
-    PIIX4PMState pm;
+    MC146818RtcState *rtc;
+    PCIIDEState *ide;
+    UHCIState *uhci;
+    PIIX4PMState *pm;
 
     uint32_t smb_io_base;
 
@@ -69,7 +69,7 @@ struct PIIXState {
     uint8_t rcr;
 
     /* IO memory region for Reset Control Register (PIIX_RCR_IOPORT) */
-    MemoryRegion rcr_mem;
+    MemoryRegion *rcr_mem;
 
     bool has_acpi;
     bool has_pic;
-- 
2.54.0

Re: [RFC 6/7] hw/isa: convert PIIX embedded QOM objects to heap allocated

[Akihiko Odaki]

On 2026/06/17 0:55, Daniel P. Berrangé wrote:
> Update for the new preferred QOM design by removing embedded QOM
> objects from the PIIXState struct for the RTC, IDE, UHCI, PM,
> IRQ and memory region classes.
> 
> XXXX:  there seems to be little benefit in having any of the
> instance fields for RTC, IDE, UHCI, PM & IRQ objects.  These
> objects are all kept alive via the 'child' property which owns
> the primary reference. The instance fields are accessed durnig
> setup and then never again, so all these instances fields could
> arguably go away entirely.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   hw/isa/piix.c                 | 65 ++++++++++++++++++++++-------------
>   include/hw/southbridge/piix.h | 12 +++----
>   2 files changed, 47 insertions(+), 30 deletions(-)
> 
> diff --git a/hw/isa/piix.c b/hw/isa/piix.c
> index 31fa53e6a4..cd23486ef9 100644
> --- a/hw/isa/piix.c
> +++ b/hw/isa/piix.c
> @@ -308,18 +308,22 @@ static void pci_piix_realize(PCIDevice *dev, const char *uhci_type,
>           return;
>       }
>   
> -    memory_region_init_io(&d->rcr_mem, OBJECT(dev), &rcr_ops, d,
> -                          "piix-reset-control", 1);
> +    d->rcr_mem = memory_region_new_io(OBJECT(dev), &rcr_ops, d,
> +                                      "piix-reset-control", 1);
>       memory_region_add_subregion_overlap(pci_address_space_io(dev),
> -                                        PIIX_RCR_IOPORT, &d->rcr_mem, 1);
> +                                        PIIX_RCR_IOPORT, d->rcr_mem, 1);
>   
>       /* PIC */
>       if (d->has_pic) {
>           qemu_irq *i8259;
>   
> -        qemu_init_irq_child(OBJECT(dev), "i8259-irq", &d->i8259_irq,
> -                            piix_request_i8259_irq, d, 0);
> -        i8259 = i8259_init(isa_bus, &d->i8259_irq);
> +        d->i8259_irq = qemu_irq_new_child(OBJECT(dev), "i8259-irq",
> +                                          piix_request_i8259_irq, d, 0,
> +                                          errp);
> +        if (!d->i8259_irq) {
> +            return;
> +        }
> +        i8259 = i8259_init(isa_bus, d->i8259_irq);
>   
>           for (size_t i = 0; i < ISA_NUM_IRQS; i++) {
>               d->isa_irqs_in[i] = i8259[i];
> @@ -340,38 +344,45 @@ static void pci_piix_realize(PCIDevice *dev, const char *uhci_type,
>       i8257_dma_init(OBJECT(dev), isa_bus, 0);
>   
>       /* RTC */
> -    qdev_prop_set_int32(DEVICE(&d->rtc), "base_year", 2000);
> -    if (!qdev_realize(DEVICE(&d->rtc), BUS(isa_bus), errp)) {
> +    qdev_prop_set_int32(DEVICE(d->rtc), "base_year", 2000);
> +    if (!qdev_realize(DEVICE(d->rtc), BUS(isa_bus), errp)) {
>           return;
>       }
> -    irq = object_property_get_uint(OBJECT(&d->rtc), "irq", &error_fatal);
> -    isa_connect_gpio_out(ISA_DEVICE(&d->rtc), 0, irq);
> +    irq = object_property_get_uint(OBJECT(d->rtc), "irq", &error_fatal);
> +    isa_connect_gpio_out(ISA_DEVICE(d->rtc), 0, irq);
>   
>       /* IDE */
> -    qdev_prop_set_int32(DEVICE(&d->ide), "addr", dev->devfn + 1);
> -    if (!qdev_realize(DEVICE(&d->ide), BUS(pci_bus), errp)) {
> +    qdev_prop_set_int32(DEVICE(d->ide), "addr", dev->devfn + 1);
> +    if (!qdev_realize(DEVICE(d->ide), BUS(pci_bus), errp)) {
>           return;
>       }
>   
>       /* USB */
>       if (d->has_usb) {
> -        object_initialize_child(OBJECT(dev), "uhci", &d->uhci, uhci_type);
> -        qdev_prop_set_int32(DEVICE(&d->uhci), "addr", dev->devfn + 2);
> -        if (!qdev_realize(DEVICE(&d->uhci), BUS(pci_bus), errp)) {
> +        d->uhci = UHCI(object_new_with_props(
> +                           uhci_type, OBJECT(d), "uhci", errp, NULL));
> +        if (!d->uhci) {
> +            return;
> +        }
> +        qdev_prop_set_int32(DEVICE(d->uhci), "addr", dev->devfn + 2);
> +        if (!qdev_realize(DEVICE(d->uhci), BUS(pci_bus), errp)) {
>               return;
>           }
>       }
>   
>       /* Power Management */
>       if (d->has_acpi) {
> -        object_initialize_child(OBJECT(d), "pm", &d->pm, TYPE_PIIX4_PM);
> -        qdev_prop_set_int32(DEVICE(&d->pm), "addr", dev->devfn + 3);
> -        qdev_prop_set_uint32(DEVICE(&d->pm), "smb_io_base", d->smb_io_base);
> -        qdev_prop_set_bit(DEVICE(&d->pm), "smm-enabled", d->smm_enabled);
> -        if (!qdev_realize(DEVICE(&d->pm), BUS(pci_bus), errp)) {
> +        d->pm = PIIX4_PM(object_new_with_props(
> +                             TYPE_PIIX4_PM, OBJECT(d), "pm", errp, NULL));
> +        if (!d->pm) {
> +        }

This if block doesn't have a body.

Regards,
Akihiko Odaki

> +        qdev_prop_set_int32(DEVICE(d->pm), "addr", dev->devfn + 3);
> +        qdev_prop_set_uint32(DEVICE(d->pm), "smb_io_base", d->smb_io_base);
> +        qdev_prop_set_bit(DEVICE(d->pm), "smm-enabled", d->smm_enabled);
> +        if (!qdev_realize(DEVICE(d->pm), BUS(pci_bus), errp)) {
>               return;
>           }
> -        qdev_connect_gpio_out(DEVICE(&d->pm), 0, d->isa_irqs_in[9]);
> +        qdev_connect_gpio_out(DEVICE(d->pm), 0, d->isa_irqs_in[9]);
>       }
>   
>       pci_bus_irqs(pci_bus, piix_set_pci_irq, d, PIIX_NUM_PIRQS);
> @@ -406,7 +417,9 @@ static void pci_piix_init(Object *obj)
>       qdev_init_gpio_out_named(DEVICE(obj), d->isa_irqs_in, "isa-irqs",
>                                ISA_NUM_IRQS);
>   
> -    object_initialize_child(obj, "rtc", &d->rtc, TYPE_MC146818_RTC);
> +    d->rtc = MC146818_RTC(
> +        object_new_with_props(TYPE_MC146818_RTC, obj, "rtc",
> +                              &error_abort, NULL));
>   }
>   
>   static const Property pci_piix_props[] = {
> @@ -462,7 +475,9 @@ static void piix3_init(Object *obj)
>   {
>       PIIXState *d = PIIX_PCI_DEVICE(obj);
>   
> -    object_initialize_child(obj, "ide", &d->ide, TYPE_PIIX3_IDE);
> +    d->ide = PCI_IDE(
> +        object_new_with_props(TYPE_PIIX3_IDE, obj, "ide",
> +                              &error_abort, NULL));
>   }
>   
>   static void piix3_class_init(ObjectClass *klass, const void *data)
> @@ -492,7 +507,9 @@ static void piix4_init(Object *obj)
>   {
>       PIIXState *s = PIIX_PCI_DEVICE(obj);
>   
> -    object_initialize_child(obj, "ide", &s->ide, TYPE_PIIX4_IDE);
> +    s->ide = PCI_IDE(
> +        object_new_with_props(TYPE_PIIX4_IDE, obj, "ide",
> +                              &error_abort, NULL));
>   }
>   
>   static void piix4_class_init(ObjectClass *klass, const void *data)
> diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
> index a296b1205a..8bd24946de 100644
> --- a/include/hw/southbridge/piix.h
> +++ b/include/hw/southbridge/piix.h
> @@ -53,15 +53,15 @@ struct PIIXState {
>       qemu_irq cpu_intr;
>       qemu_irq isa_irqs_in[ISA_NUM_IRQS];
>   
> -    IRQState i8259_irq;
> +    IRQState *i8259_irq;
>   
>       /* This member isn't used. Just for save/load compatibility */
>       int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS];
>   
> -    MC146818RtcState rtc;
> -    PCIIDEState ide;
> -    UHCIState uhci;
> -    PIIX4PMState pm;
> +    MC146818RtcState *rtc;
> +    PCIIDEState *ide;
> +    UHCIState *uhci;
> +    PIIX4PMState *pm;
>   
>       uint32_t smb_io_base;
>   
> @@ -69,7 +69,7 @@ struct PIIXState {
>       uint8_t rcr;
>   
>       /* IO memory region for Reset Control Register (PIIX_RCR_IOPORT) */
> -    MemoryRegion rcr_mem;
> +    MemoryRegion *rcr_mem;
>   
>       bool has_acpi;
>       bool has_pic;

[RFC 7/7] qom: improve error message for invalid ID values

[Daniel P. Berrangé]

Include the actual ID that was validated, since this may not be
obvious in all contexts.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 qom/object.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qom/object.c b/qom/object.c
index 415d5c5291..9cd17e3bcc 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -774,7 +774,7 @@ object_new_with_props_helper(const char *typename,
            (id == NULL && parent == NULL));
 
     if (id != NULL && !id_wellformed(id)) {
-        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "id", "an identifier");
+        error_setg(errp, "QOM ID value '%s' is not valid", id);
         error_append_hint(errp, "Identifiers consist of letters, digits, "
                           "'-', '.', '_', starting with a letter.\n");
         return NULL;
-- 
2.54.0

Re: [RFC 7/7] qom: improve error message for invalid ID values

[Markus Armbruster]

Daniel P. Berrangé <berrange@redhat.com> writes:

> Include the actual ID that was validated, since this may not be
> obvious in all contexts.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

Reproducer?

I'm asking because:

    $ qemu-system-x86_64 -object id=@
    qemu-system-x86_64: -object id=@: Parameter 'id' expects an identifier
    Identifiers consist of letters, digits, '-', '.', '_', starting with a letter.

This is qemu_opts_create().

And:

    {"execute":"device_add","arguments":{"driver":"e1000", "id":"@"}}
    {"error": {"class": "GenericError", "desc": "Invalid qdev ID '@'"}}

This is qdev_set_id().

Ah, found it:

    $ qemu-system-x86_64 -object '{"id": "@", "qom-type": "iothread"}'
    qemu-system-x86_64: QOM ID value '@' is not valid
    Identifiers consist of letters, digits, '-', '.', '_', starting with a letter.

> ---
>  qom/object.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/qom/object.c b/qom/object.c
> index 415d5c5291..9cd17e3bcc 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -774,7 +774,7 @@ object_new_with_props_helper(const char *typename,
>             (id == NULL && parent == NULL));
>  
>      if (id != NULL && !id_wellformed(id)) {
> -        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "id", "an identifier");
> +        error_setg(errp, "QOM ID value '%s' is not valid", id);
>          error_append_hint(errp, "Identifiers consist of letters, digits, "
>                            "'-', '.', '_', starting with a letter.\n");
>          return NULL;

Improvement, but what about other places reporting an error when
id_wellformed() fails?

Re: [RFC 7/7] qom: improve error message for invalid ID values

[Daniel P. Berrangé]

On Wed, Jun 17, 2026 at 12:46:51PM +0200, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > Include the actual ID that was validated, since this may not be
> > obvious in all contexts.
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> 
> Reproducer?
> 
> I'm asking because:
> 
>     $ qemu-system-x86_64 -object id=@
>     qemu-system-x86_64: -object id=@: Parameter 'id' expects an identifier
>     Identifiers consist of letters, digits, '-', '.', '_', starting with a letter.
> 
> This is qemu_opts_create().
> 
> And:
> 
>     {"execute":"device_add","arguments":{"driver":"e1000", "id":"@"}}
>     {"error": {"class": "GenericError", "desc": "Invalid qdev ID '@'"}}
> 
> This is qdev_set_id().
> 
> Ah, found it:
> 
>     $ qemu-system-x86_64 -object '{"id": "@", "qom-type": "iothread"}'
>     qemu-system-x86_64: QOM ID value '@' is not valid
>     Identifiers consist of letters, digits, '-', '.', '_', starting with a letter.

Actually in this case it wasn't anything user controlled,
I was calling  object_new_with_props internally to create
child objects and it tripped over this due to a bug of
my own making. It was just rather confusing as to what I
had done wrong

> 
> > ---
> >  qom/object.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/qom/object.c b/qom/object.c
> > index 415d5c5291..9cd17e3bcc 100644
> > --- a/qom/object.c
> > +++ b/qom/object.c
> > @@ -774,7 +774,7 @@ object_new_with_props_helper(const char *typename,
> >             (id == NULL && parent == NULL));
> >  
> >      if (id != NULL && !id_wellformed(id)) {
> > -        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "id", "an identifier");
> > +        error_setg(errp, "QOM ID value '%s' is not valid", id);
> >          error_append_hint(errp, "Identifiers consist of letters, digits, "
> >                            "'-', '.', '_', starting with a letter.\n");
> >          return NULL;
> 
> Improvement, but what about other places reporting an error when
> id_wellformed() fails?
> 

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 :|

Re: [RFC 0/7] qom: deprecate embedded objects and instance properties

[Peter Maydell]

On Tue, 16 Jun 2026 at 16:56, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> QOM has two rather unusual / surprising features historicall
>
>  * The ability to embed a QOM instance's memory inside another
>    struct
>  * The ability to register properties against the instnce
>    instead of struct
>
> While they both look convenient on the surface, they also
> have significant undesirable side effects (see the commit
> message for each patch for details).
>
> The premise of this series is that their convenience does
> not outweigh their downsides, and we would be better off
> long term by eliminating their usage, rather than trying
> to add more hacks on top to mitigate their downsides.

The thing I would like to see before we mark object_initialize_child
and friends as deprecated is clear documentation of "this is how
we would like you to write 'container/SoC' style devices, here is
an device written to the approved style you can look at".

Currently we have in the codebase a pretty wide range of
different ways to write devices:
 - really ancient, not QOM/qdev at all
 - qdev style (lots of Device* pointers)
 - embedded-struct style
and I'm not sure if this would be adding a fourth style, or
rolling back to qdev style.

(Borrowing a paragraph I wrote last time this came up:)

I'm not 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 all be on the same page about the design
patterns we want and it will be clearer to reviewers whether new
code and new APIs and conversions of old code fit into those
patterns or not.

I think we're getting closer on the "consensus" part but
the "document the new best practice" part is important I think.

thanks
-- PMM

Re: [RFC 0/7] qom: deprecate embedded objects and instance properties

[Daniel P. Berrangé]

On Tue, Jun 16, 2026 at 05:12:27PM +0100, Peter Maydell wrote:
> On Tue, 16 Jun 2026 at 16:56, Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > QOM has two rather unusual / surprising features historicall
> >
> >  * The ability to embed a QOM instance's memory inside another
> >    struct
> >  * The ability to register properties against the instnce
> >    instead of struct
> >
> > While they both look convenient on the surface, they also
> > have significant undesirable side effects (see the commit
> > message for each patch for details).
> >
> > The premise of this series is that their convenience does
> > not outweigh their downsides, and we would be better off
> > long term by eliminating their usage, rather than trying
> > to add more hacks on top to mitigate their downsides.
> 
> The thing I would like to see before we mark object_initialize_child
> and friends as deprecated is clear documentation of "this is how
> we would like you to write 'container/SoC' style devices, here is
> an device written to the approved style you can look at".

Do we have any documentation currently that touches on
this area that we can start from / adapt to best
practices ?  I can do adaptation, but I'm not an expert
on Device code, so probably not best placed to write a
new doc fro mscratch.

> Currently we have in the codebase a pretty wide range of
> different ways to write devices:
>  - really ancient, not QOM/qdev at all
>  - qdev style (lots of Device* pointers)
>  - embedded-struct style
> and I'm not sure if this would be adding a fourth style, or
> rolling back to qdev style.

Per my commit message in the PIIX patch, I'm not convinced
we need to store any device pointers in the instance
structs at all in many (possibly most) cases. The instance
struct fields are mostly only referenced during creation
time and then never again, with the QOM 'child' property
holding the permanent reference.

> I'm not 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 all be on the same page about the design
> patterns we want and it will be clearer to reviewers whether new
> code and new APIs and conversions of old code fit into those
> patterns or not.
> 
> I think we're getting closer on the "consensus" part but
> the "document the new best practice" part is important I think.

Yep, best practice docs must be a key part of the story.

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 :|

Re: [RFC 0/7] qom: deprecate embedded objects and instance properties

[Mark Cave-Ayland]

On 16/06/2026 17:40, Daniel P. Berrangé wrote:

> On Tue, Jun 16, 2026 at 05:12:27PM +0100, Peter Maydell wrote:
>> On Tue, 16 Jun 2026 at 16:56, Daniel P. Berrangé <berrange@redhat.com> wrote:
>>>
>>> QOM has two rather unusual / surprising features historicall
>>>
>>>   * The ability to embed a QOM instance's memory inside another
>>>     struct
>>>   * The ability to register properties against the instnce
>>>     instead of struct
>>>
>>> While they both look convenient on the surface, they also
>>> have significant undesirable side effects (see the commit
>>> message for each patch for details).
>>>
>>> The premise of this series is that their convenience does
>>> not outweigh their downsides, and we would be better off
>>> long term by eliminating their usage, rather than trying
>>> to add more hacks on top to mitigate their downsides.
>>
>> The thing I would like to see before we mark object_initialize_child
>> and friends as deprecated is clear documentation of "this is how
>> we would like you to write 'container/SoC' style devices, here is
>> an device written to the approved style you can look at".
> 
> Do we have any documentation currently that touches on
> this area that we can start from / adapt to best
> practices ?  I can do adaptation, but I'm not an expert
> on Device code, so probably not best placed to write a
> new doc fro mscratch.
> 
>> Currently we have in the codebase a pretty wide range of
>> different ways to write devices:
>>   - really ancient, not QOM/qdev at all
>>   - qdev style (lots of Device* pointers)
>>   - embedded-struct style
>> and I'm not sure if this would be adding a fourth style, or
>> rolling back to qdev style.
> 
> Per my commit message in the PIIX patch, I'm not convinced
> we need to store any device pointers in the instance
> structs at all in many (possibly most) cases. The instance
> struct fields are mostly only referenced during creation
> time and then never again, with the QOM 'child' property
> holding the permanent reference.

I think it depends: we do already have hot paths where people use a 
direct pointer reference instead of an object property lookup, but these 
cases are generally in the minority. And there's no reason why for those 
cases the pointer couldn't be cached within the device state during 
device realize or similar, along with a suitable comment of course.

The other case in which I've found device pointers in the device state 
useful is during low-level debugging sessions i.e. where state is 
corrupted enough that calling object functions in gdb may fail.

>> I'm not 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 all be on the same page about the design
>> patterns we want and it will be clearer to reviewers whether new
>> code and new APIs and conversions of old code fit into those
>> patterns or not.
>>
>> I think we're getting closer on the "consensus" part but
>> the "document the new best practice" part is important I think.
> 
> Yep, best practice docs must be a key part of the story.


ATB,

Mark.

Re: [RFC 0/7] qom: deprecate embedded objects and instance properties

[Mark Cave-Ayland]

On 16/06/2026 17:12, Peter Maydell wrote:

> On Tue, 16 Jun 2026 at 16:56, Daniel P. Berrangé <berrange@redhat.com> wrote:
>>
>> QOM has two rather unusual / surprising features historicall
>>
>>   * The ability to embed a QOM instance's memory inside another
>>     struct
>>   * The ability to register properties against the instnce
>>     instead of struct
>>
>> While they both look convenient on the surface, they also
>> have significant undesirable side effects (see the commit
>> message for each patch for details).
>>
>> The premise of this series is that their convenience does
>> not outweigh their downsides, and we would be better off
>> long term by eliminating their usage, rather than trying
>> to add more hacks on top to mitigate their downsides.
> 
> The thing I would like to see before we mark object_initialize_child
> and friends as deprecated is clear documentation of "this is how
> we would like you to write 'container/SoC' style devices, here is
> an device written to the approved style you can look at".
> 
> Currently we have in the codebase a pretty wide range of
> different ways to write devices:
>   - really ancient, not QOM/qdev at all
>   - qdev style (lots of Device* pointers)
>   - embedded-struct style
> and I'm not sure if this would be adding a fourth style, or
> rolling back to qdev style.
> 
> (Borrowing a paragraph I wrote last time this came up:)
> 
> I'm not 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 all be on the same page about the design
> patterns we want and it will be clearer to reviewers whether new
> code and new APIs and conversions of old code fit into those
> patterns or not.
> 
> I think we're getting closer on the "consensus" part but
> the "document the new best practice" part is important I think.

Agreed. The only issue I can see here is that often documentation isn't 
good enough: as an example, we've standardised on using "parent_obj" as 
the field name for several years now, but we still get patches using 
different names because developers struggle to find the documentation, 
and reviewers aren't often aware of changes in how we model devices etc.

Based on this experience, I think the only way we can realistically do 
this is to teach checkpatch.pl about it so that using functions such as 
object_initialize_child(), object_property_add() etc. will cause CI failure.

I'd love to be able to teach it about "parent_obj" and not to embed 
objects that aren't pointers, but I don't know if that's possible?


ATB,

Mark.

Re: [RFC 0/7] qom: deprecate embedded objects and instance properties

[Daniel P. Berrangé]

On Wed, Jun 17, 2026 at 11:39:57AM +0100, Mark Cave-Ayland wrote:
> On 16/06/2026 17:12, Peter Maydell wrote:
> 
> > On Tue, 16 Jun 2026 at 16:56, Daniel P. Berrangé <berrange@redhat.com> wrote:
> > > 
> > > QOM has two rather unusual / surprising features historicall
> > > 
> > >   * The ability to embed a QOM instance's memory inside another
> > >     struct
> > >   * The ability to register properties against the instnce
> > >     instead of struct
> > > 
> > > While they both look convenient on the surface, they also
> > > have significant undesirable side effects (see the commit
> > > message for each patch for details).
> > > 
> > > The premise of this series is that their convenience does
> > > not outweigh their downsides, and we would be better off
> > > long term by eliminating their usage, rather than trying
> > > to add more hacks on top to mitigate their downsides.
> > 
> > The thing I would like to see before we mark object_initialize_child
> > and friends as deprecated is clear documentation of "this is how
> > we would like you to write 'container/SoC' style devices, here is
> > an device written to the approved style you can look at".
> > 
> > Currently we have in the codebase a pretty wide range of
> > different ways to write devices:
> >   - really ancient, not QOM/qdev at all
> >   - qdev style (lots of Device* pointers)
> >   - embedded-struct style
> > and I'm not sure if this would be adding a fourth style, or
> > rolling back to qdev style.
> > 
> > (Borrowing a paragraph I wrote last time this came up:)
> > 
> > I'm not 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 all be on the same page about the design
> > patterns we want and it will be clearer to reviewers whether new
> > code and new APIs and conversions of old code fit into those
> > patterns or not.
> > 
> > I think we're getting closer on the "consensus" part but
> > the "document the new best practice" part is important I think.
> 
> Agreed. The only issue I can see here is that often documentation isn't good
> enough: as an example, we've standardised on using "parent_obj" as the field
> name for several years now, but we still get patches using different names
> because developers struggle to find the documentation, and reviewers aren't
> often aware of changes in how we model devices etc.
> 
> Based on this experience, I think the only way we can realistically do this
> is to teach checkpatch.pl about it so that using functions such as
> object_initialize_child(), object_property_add() etc. will cause CI failure.
> 
> I'd love to be able to teach it about "parent_obj" and not to embed objects
> that aren't pointers, but I don't know if that's possible?

For embedding we can teach checkpatch to whine about any use of
object_initialize() in new code easily enough which catches most
cases.

For 'parent_obj' I thought about a macro assert:

$ git diff
diff --git a/include/qom/object.h b/include/qom/object.h
index e9ce15d595..00d9424e3d 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -283,6 +283,7 @@ struct Object
     module_obj_name##_class_init(ObjectClass *oc, const void *data); \
     static void \
     module_obj_name##_init(Object *obj); \
+    G_STATIC_ASSERT(offsetof(ModuleObjName, parent_obj) == 0);   \
     \
     static const TypeInfo module_obj_name##_info = { \
         .parent = TYPE_##PARENT_MODULE_OBJ_NAME, \


Unfortunately only a small number use OBJECT_DEFINE_TYPE macros,
most use DEFINE_TYPES which does not have access to the struct
names :-(  Still that does show a few violators of the current
rule.

I also tried the same assert in OBJECT_DECLARE_TYPE which would
catch all usage, but that falls over when the header pre-declares
the struct name, and the source file has its actual definition.
We don't want to force the struct definition into the header
so I'm out of options for build-time checks there :-(




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 :|

Re: [RFC 0/7] qom: deprecate embedded objects and instance properties

[Mark Cave-Ayland]

On 16/06/2026 16:55, Daniel P. Berrangé wrote:

> QOM has two rather unusual / surprising features historicall
> 
>   * The ability to embed a QOM instance's memory inside another
>     struct
>   * The ability to register properties against the instnce
>     instead of struct
> 
> While they both look convenient on the surface, they also
> have significant undesirable side effects (see the commit
> message for each patch for details).
> 
> The premise of this series is that their convenience does
> not outweigh their downsides, and we would be better off
> long term by eliminating their usage, rather than trying
> to add more hacks on top to mitigate their downsides.
> 
> This comes out of two separate conversations this week
> 
>   * Migration series where Peter proposed changes that
>     make use of instance  properties. When I commented,
>     Peter rightly pointed out that our docs do not
>     discourage use of instance properties:
>     
>      https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2026-2D06_msg02368.html&d=DwIDaQ&c=s883GpUCOChKOHiocYtGcg&r=c23RpsaH4D2MKyD3EPJTDa0BAxz6tV8aUJqVSoytEiY&m=VzbegkrZ5TGWl9jpxLUfD3gMp4ZZjyUrMiBfOzSjpcaOU-hz3SuUIaGUiIM4BQdV&s=v-N9FJZ1ugUjERquAp1KfuZ3B20G9mdbHpEEFP5zkfs&e=
> 
>   * QOM series where Akihiko proposed some funky hacks
>     to reference counting to better track object lifecycle
>     when embedding structs. We had a short discussion about
>     discouraging QOM embedding, so this is a real world
>     proposal:
> 
>       https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2026-2D06_msg03459.html&d=DwIDaQ&c=s883GpUCOChKOHiocYtGcg&r=c23RpsaH4D2MKyD3EPJTDa0BAxz6tV8aUJqVSoytEiY&m=VzbegkrZ5TGWl9jpxLUfD3gMp4ZZjyUrMiBfOzSjpcaOU-hz3SuUIaGUiIM4BQdV&s=mHTDgFeywt1U_2Q9uTnAyG6zp-VjguJ4DZIJAX59vaY&e=
> 
> This is not likely to be a quick task, so this series
> starts small
> 
>   * Adds a "QEMU_DEPRECATIONS" annotation for internal APIs
>   * Deprecate the QOM instance embedding
>   * Deprecate the QOM instance properties
>   * Deprecate the memory region embedding APIs
>   * Deprecate the IRQ embedding APIs
>   * Convert PIIX to eliminate embedding as a
>     demonstration.
> 
> The QEMU_DEPRECATIONS idea is an effect to improve our our
> historic practice where we introduce a new preferred API
> and never really tell anyone the old APIs is bad to use.
> 
> If --enable-deprecations is given to configure, every use
> of a deprecated API emits a compiler warning. This is not
> enabled by default in this series, since we have -Werror
> by default.

I'm not sure if we're ready to start flagging things via the compiler, 
however I'm in broad agreement that this is direction we need to travel. 
Thanks for beating me to it, and starting the conversation :)

> Note that the proposal to stop embedding memory regions has
> been made by Zoltan earlier this year and was rejected by Paolo:
> 
>    https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.nongnu.org_archive_html_qemu-2Ddevel_2026-2D01_msg05435.html&d=DwIDaQ&c=s883GpUCOChKOHiocYtGcg&r=c23RpsaH4D2MKyD3EPJTDa0BAxz6tV8aUJqVSoytEiY&m=VzbegkrZ5TGWl9jpxLUfD3gMp4ZZjyUrMiBfOzSjpcaOU-hz3SuUIaGUiIM4BQdV&s=oAY3a1R4rnhYdl7-qnoTVjVdGOX1Qj-GvGlOX77oZrk&e=
>    https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.nongnu.org_archive_html_qemu-2Ddevel_2026-2D05_msg06665.html&d=DwIDaQ&c=s883GpUCOChKOHiocYtGcg&r=c23RpsaH4D2MKyD3EPJTDa0BAxz6tV8aUJqVSoytEiY&m=VzbegkrZ5TGWl9jpxLUfD3gMp4ZZjyUrMiBfOzSjpcaOU-hz3SuUIaGUiIM4BQdV&s=WmpukrnrmuhhTz7hLgAA-9FStZ3fbzH3P1aT_U9f6GY&e=
> 
> I don't really agree with the analysis there. IMHO,  the
> concept of embedding objects is to horrendous to allow to
> live any longer. Yes, that's a big job, but long term it
> is worth it.

FWIW my main objection to the series was that it added yet another way 
of doing something to fix a particular issue in legacy code. I have a 
suspicion that if we can make everything in QOM properly refcounted then 
this approach could work globally: in particular I can see it would fix 
issues where the flatviews hold references to memory regions outside of 
the device lifecycle until the RCU thread tidies up.

However as you can see, there would be quite a bit of work to get there. 
But at least if we can all agree on the way forward, we can start to get 
the relevant changes in place.

> Daniel P. Berrangé (7):
>    meson: add --enable-deprecations configure flag
>    qom: deprecated embedding object structs within other objects
>    qom: deprecate use of instance properties
>    system: add memory_region_new / memory_region_new_io
>    system: add qemu_irq_new / qemu_irq_new_child / qemu_irq_new_array
>    hw/isa: convert PIIX embedded QOM objects to heap allocated
>    qom: improve error message for invalid ID values
> 
>   hw/core/irq.c                 | 35 +++++++++++++
>   hw/isa/piix.c                 | 65 ++++++++++++++---------
>   include/hw/core/irq.h         | 75 +++++++++++++++++++++++++--
>   include/hw/southbridge/piix.h | 12 ++---
>   include/qemu/osdep.h          | 19 +++++++
>   include/qom/object.h          | 98 ++++++++++++++++++++++++++++++-----
>   include/system/memory.h       | 76 +++++++++++++++++++++++----
>   meson.build                   |  1 +
>   meson_options.txt             |  2 +
>   qom/object.c                  | 36 ++++++++++++-
>   scripts/meson-buildoptions.sh |  3 ++
>   system/memory.c               | 85 +++++++++++++++++++++++++-----
>   12 files changed, 433 insertions(+), 74 deletions(-)


ATB,

Mark.

Re: [RFC 0/7] qom: deprecate embedded objects and instance properties

[BALATON Zoltan]

[-- Attachment #1: Type: text/plain, Size: 5818 bytes --]

On Wed, 17 Jun 2026, Mark Cave-Ayland wrote:
> On 16/06/2026 16:55, Daniel P. Berrangé wrote:
>
>> QOM has two rather unusual / surprising features historicall
>>
>>   * The ability to embed a QOM instance's memory inside another
>>     struct
>>   * The ability to register properties against the instnce
>>     instead of struct
>> 
>> While they both look convenient on the surface, they also
>> have significant undesirable side effects (see the commit
>> message for each patch for details).
>> 
>> The premise of this series is that their convenience does
>> not outweigh their downsides, and we would be better off
>> long term by eliminating their usage, rather than trying
>> to add more hacks on top to mitigate their downsides.
>> 
>> This comes out of two separate conversations this week
>>
>>   * Migration series where Peter proposed changes that
>>     make use of instance  properties. When I commented,
>>     Peter rightly pointed out that our docs do not
>>     discourage use of instance properties:
>>          https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2026-2D06_msg02368.html&d=DwIDaQ&c=s883GpUCOChKOHiocYtGcg&r=c23RpsaH4D2MKyD3EPJTDa0BAxz6tV8aUJqVSoytEiY&m=VzbegkrZ5TGWl9jpxLUfD3gMp4ZZjyUrMiBfOzSjpcaOU-hz3SuUIaGUiIM4BQdV&s=v-N9FJZ1ugUjERquAp1KfuZ3B20G9mdbHpEEFP5zkfs&e=
>>
>>   * QOM series where Akihiko proposed some funky hacks
>>     to reference counting to better track object lifecycle
>>     when embedding structs. We had a short discussion about
>>     discouraging QOM embedding, so this is a real world
>>     proposal:
>>
>>       https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2026-2D06_msg03459.html&d=DwIDaQ&c=s883GpUCOChKOHiocYtGcg&r=c23RpsaH4D2MKyD3EPJTDa0BAxz6tV8aUJqVSoytEiY&m=VzbegkrZ5TGWl9jpxLUfD3gMp4ZZjyUrMiBfOzSjpcaOU-hz3SuUIaGUiIM4BQdV&s=mHTDgFeywt1U_2Q9uTnAyG6zp-VjguJ4DZIJAX59vaY&e=
>> 
>> This is not likely to be a quick task, so this series
>> starts small
>>
>>   * Adds a "QEMU_DEPRECATIONS" annotation for internal APIs
>>   * Deprecate the QOM instance embedding
>>   * Deprecate the QOM instance properties
>>   * Deprecate the memory region embedding APIs
>>   * Deprecate the IRQ embedding APIs
>>   * Convert PIIX to eliminate embedding as a
>>     demonstration.
>> 
>> The QEMU_DEPRECATIONS idea is an effect to improve our our
>> historic practice where we introduce a new preferred API
>> and never really tell anyone the old APIs is bad to use.
>> 
>> If --enable-deprecations is given to configure, every use
>> of a deprecated API emits a compiler warning. This is not
>> enabled by default in this series, since we have -Werror
>> by default.
>
> I'm not sure if we're ready to start flagging things via the compiler, 
> however I'm in broad agreement that this is direction we need to travel. 
> Thanks for beating me to it, and starting the conversation :)
>
>> Note that the proposal to stop embedding memory regions has
>> been made by Zoltan earlier this year and was rejected by Paolo:
>>
>>    https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.nongnu.org_archive_html_qemu-2Ddevel_2026-2D01_msg05435.html&d=DwIDaQ&c=s883GpUCOChKOHiocYtGcg&r=c23RpsaH4D2MKyD3EPJTDa0BAxz6tV8aUJqVSoytEiY&m=VzbegkrZ5TGWl9jpxLUfD3gMp4ZZjyUrMiBfOzSjpcaOU-hz3SuUIaGUiIM4BQdV&s=oAY3a1R4rnhYdl7-qnoTVjVdGOX1Qj-GvGlOX77oZrk&e=
>>    https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.nongnu.org_archive_html_qemu-2Ddevel_2026-2D05_msg06665.html&d=DwIDaQ&c=s883GpUCOChKOHiocYtGcg&r=c23RpsaH4D2MKyD3EPJTDa0BAxz6tV8aUJqVSoytEiY&m=VzbegkrZ5TGWl9jpxLUfD3gMp4ZZjyUrMiBfOzSjpcaOU-hz3SuUIaGUiIM4BQdV&s=WmpukrnrmuhhTz7hLgAA-9FStZ3fbzH3P1aT_U9f6GY&e=
>> 
>> I don't really agree with the analysis there. IMHO,  the
>> concept of embedding objects is to horrendous to allow to
>> live any longer. Yes, that's a big job, but long term it
>> is worth it.
>
> FWIW my main objection to the series was that it added yet another way of 
> doing something to fix a particular issue in legacy code. I have a suspicion 
> that if we can make everything in QOM properly refcounted then this approach 
> could work globally: in particular I can see it would fix issues where the 
> flatviews hold references to memory regions outside of the device lifecycle 
> until the RCU thread tidies up.
>
> However as you can see, there would be quite a bit of work to get there. But 
> at least if we can all agree on the way forward, we can start to get the 
> relevant changes in place.

We can't convert the whole code base at once and it's unrealistic to 
expect a single contributor like me to do that to accept my patch. What I 
proposed is a start that can be used now for the cases I have and can 
serve as a basis for future conversions. I did not propose to convert 
everything either because I think having both ref counted and externally 
managed memory regions can be useful in different cases and I don't see 
having two ways for this confusing considering we have object_new and 
object_initialize for same reasons. But if we deprecate object_initialize 
then the memory_region_init functions are not needed either.

Originally I also thought about splitting alloc and init similar to how 
macOS Foundation does it (i.e. [[obj alloc] init]) so the allocation of 
the memory region could be done by either object_new or embedded but that 
does not work due to object_new setting free function and 
object_initialize clearing it so you can only use either one or the other. 
Therefore since memory_region_init calls object_initialize we need a 
memory_region_new variant that uses object_new instead or we end up 
having to convert the whole code base again to decouple object 
initialization from memory_region_init so I did not go that way.

Regards,
BALATON Zoltan

Re: [RFC 0/7] qom: deprecate embedded objects and instance properties

[Markus Armbruster]

Daniel P. Berrangé <berrange@redhat.com> writes:

> QOM has two rather unusual / surprising features historicall
>
>  * The ability to embed a QOM instance's memory inside another
>    struct
>  * The ability to register properties against the instnce
>    instead of struct

I figure you mean "against the instance instead of the class".

[...]

Re: [RFC 0/7] qom: deprecate embedded objects and instance properties

[Daniel P. Berrangé]

On Wed, Jun 17, 2026 at 12:59:21PM +0200, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > QOM has two rather unusual / surprising features historicall
> >
> >  * The ability to embed a QOM instance's memory inside another
> >    struct
> >  * The ability to register properties against the instnce
> >    instead of struct
> 
> I figure you mean "against the instance instead of the class".

Yes indeed

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 :|