[PATCH 0/9] hw/sysbus/platform-bus: Introduce TYPE_DYNAMIC_SYS_BUS_DEVICE

[PATCH 0/9] hw/sysbus/platform-bus: Introduce TYPE_DYNAMIC_SYS_BUS_DEVICE

[Philippe Mathieu-Daudé]

Some SysBus devices can optionally be dynamically plugged onto
the sysbus-platform-bus (then virtual guests are aware of
mmio mapping and IRQs via device tree / ACPI rules).

This series makes these devices explicit by having them implement
the DYNAMIC_SYS_BUS_DEVICE class, which only sets 'user_creatable'
flag to %true but makes the codebase a bit clearer (IMHO, at least
now we can grep for DYNAMIC_SYS_BUS_DEVICE).

Philippe Mathieu-Daudé (9):
  hw/sysbus: Use sizeof(BusState) in main_system_bus_create()
  hw/sysbus: Declare QOM types using DEFINE_TYPES() macro
  hw/sysbus: Introduce TYPE_DYNAMIC_SYS_BUS_DEVICE
  hw/vfio: Have VFIO_PLATFORM devices inherit from
    DYNAMIC_SYS_BUS_DEVICE
  hw/display: Have RAMFB device inherit from DYNAMIC_SYS_BUS_DEVICE
  hw/i386: Have X86_IOMMU devices inherit from DYNAMIC_SYS_BUS_DEVICE
  hw/net: Have eTSEC device inherit from DYNAMIC_SYS_BUS_DEVICE
  hw/tpm: Have TPM TIS sysbus device inherit from DYNAMIC_SYS_BUS_DEVICE
  hw/xen: Have legacy Xen backend inherit from DYNAMIC_SYS_BUS_DEVICE

 include/hw/sysbus.h           |  2 ++
 include/hw/xen/xen_pvdev.h    |  3 +-
 hw/core/sysbus.c              | 53 ++++++++++++++++++++---------------
 hw/display/ramfb-standalone.c |  3 +-
 hw/i386/amd_iommu.c           |  2 --
 hw/i386/intel_iommu.c         |  2 --
 hw/i386/x86-iommu.c           |  2 +-
 hw/net/fsl_etsec/etsec.c      |  4 +--
 hw/tpm/tpm_tis_sysbus.c       |  3 +-
 hw/vfio/amd-xgbe.c            |  2 --
 hw/vfio/calxeda-xgmac.c       |  2 --
 hw/vfio/platform.c            |  4 +--
 hw/xen/xen-legacy-backend.c   |  7 ++---
 13 files changed, 42 insertions(+), 47 deletions(-)

-- 
2.47.1

[PATCH 1/9] hw/sysbus: Use sizeof(BusState) in main_system_bus_create()

[Philippe Mathieu-Daudé]

Rather than using the obscure system_bus_info.instance_size,
directly use sizeof(BusState).

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/core/sysbus.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
index 9355849ff0a..f713bbfe04f 100644
--- a/hw/core/sysbus.c
+++ b/hw/core/sysbus.c
@@ -323,8 +323,8 @@ static void main_system_bus_create(void)
      * assign main_system_bus before qbus_init()
      * in order to make "if (bus != sysbus_get_default())" work
      */
-    main_system_bus = g_malloc0(system_bus_info.instance_size);
-    qbus_init(main_system_bus, system_bus_info.instance_size,
+    main_system_bus = g_new0(BusState, 1);
+    qbus_init(main_system_bus, sizeof(BusState),
               TYPE_SYSTEM_BUS, NULL, "main-system-bus");
     OBJECT(main_system_bus)->free = g_free;
 }
-- 
2.47.1

Re: [PATCH 1/9] hw/sysbus: Use sizeof(BusState) in main_system_bus_create()

[Richard Henderson]

On 1/25/25 10:13, Philippe Mathieu-Daudé wrote:
> Rather than using the obscure system_bus_info.instance_size,
> directly use sizeof(BusState).
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/core/sysbus.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

Nice.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH 1/9] hw/sysbus: Use sizeof(BusState) in main_system_bus_create()

[CLEMENT MATHIEU--DRIF]

Reviewed-by: Clément Mathieu--Drif<clement.mathieu--drif@eviden.com>



On 25/01/2025 19:13, Philippe Mathieu-Daudé wrote:
> Caution: External email. Do not open attachments or click links, unless this email comes from a known sender and you know the content is safe.
>
>
> Rather than using the obscure system_bus_info.instance_size,
> directly use sizeof(BusState).
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/core/sysbus.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
> index 9355849ff0a..f713bbfe04f 100644
> --- a/hw/core/sysbus.c
> +++ b/hw/core/sysbus.c
> @@ -323,8 +323,8 @@ static void main_system_bus_create(void)
>        * assign main_system_bus before qbus_init()
>        * in order to make "if (bus != sysbus_get_default())" work
>        */
> -    main_system_bus = g_malloc0(system_bus_info.instance_size);
> -    qbus_init(main_system_bus, system_bus_info.instance_size,
> +    main_system_bus = g_new0(BusState, 1);
> +    qbus_init(main_system_bus, sizeof(BusState),
>                 TYPE_SYSTEM_BUS, NULL, "main-system-bus");
>       OBJECT(main_system_bus)->free = g_free;
>   }
> --
> 2.47.1
>

[PATCH 2/9] hw/sysbus: Declare QOM types using DEFINE_TYPES() macro

[Philippe Mathieu-Daudé]

When multiple QOM types are registered in the same file,
it is simpler to use the the DEFINE_TYPES() macro. In
particular because type array declared with such macro
are easier to review.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/core/sysbus.c | 39 +++++++++++++++++----------------------
 1 file changed, 17 insertions(+), 22 deletions(-)

diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
index f713bbfe04f..306f98406c0 100644
--- a/hw/core/sysbus.c
+++ b/hw/core/sysbus.c
@@ -80,13 +80,6 @@ static void system_bus_class_init(ObjectClass *klass, void *data)
     k->get_fw_dev_path = sysbus_get_fw_dev_path;
 }
 
-static const TypeInfo system_bus_info = {
-    .name = TYPE_SYSTEM_BUS,
-    .parent = TYPE_BUS,
-    .instance_size = sizeof(BusState),
-    .class_init = system_bus_class_init,
-};
-
 /* Check whether an IRQ source exists */
 bool sysbus_has_irq(SysBusDevice *dev, int n)
 {
@@ -306,15 +299,6 @@ static void sysbus_device_class_init(ObjectClass *klass, void *data)
     k->user_creatable = false;
 }
 
-static const TypeInfo sysbus_device_type_info = {
-    .name = TYPE_SYS_BUS_DEVICE,
-    .parent = TYPE_DEVICE,
-    .instance_size = sizeof(SysBusDevice),
-    .abstract = true,
-    .class_size = sizeof(SysBusDeviceClass),
-    .class_init = sysbus_device_class_init,
-};
-
 static BusState *main_system_bus;
 
 static void main_system_bus_create(void)
@@ -337,10 +321,21 @@ BusState *sysbus_get_default(void)
     return main_system_bus;
 }
 
-static void sysbus_register_types(void)
-{
-    type_register_static(&system_bus_info);
-    type_register_static(&sysbus_device_type_info);
-}
+static const TypeInfo sysbus_types[] = {
+    {
+        .name           = TYPE_SYSTEM_BUS,
+        .parent         = TYPE_BUS,
+        .instance_size  = sizeof(BusState),
+        .class_init     = system_bus_class_init,
+    },
+    {
+        .name           = TYPE_SYS_BUS_DEVICE,
+        .parent         = TYPE_DEVICE,
+        .instance_size  = sizeof(SysBusDevice),
+        .abstract       = true,
+        .class_size     = sizeof(SysBusDeviceClass),
+        .class_init     = sysbus_device_class_init,
+    },
+};
 
-type_init(sysbus_register_types)
+DEFINE_TYPES(sysbus_types)
-- 
2.47.1

Re: [PATCH 2/9] hw/sysbus: Declare QOM types using DEFINE_TYPES() macro

[Richard Henderson]

On 1/25/25 10:13, Philippe Mathieu-Daudé wrote:
> When multiple QOM types are registered in the same file,
> it is simpler to use the the DEFINE_TYPES() macro. In
> particular because type array declared with such macro
> are easier to review.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/core/sysbus.c | 39 +++++++++++++++++----------------------
>   1 file changed, 17 insertions(+), 22 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH 2/9] hw/sysbus: Declare QOM types using DEFINE_TYPES() macro

[CLEMENT MATHIEU--DRIF]

Reviewed-by: Clément Mathieu--Drif<clement.mathieu--drif@eviden.com>



On 25/01/2025 19:13, Philippe Mathieu-Daudé wrote:
> Caution: External email. Do not open attachments or click links, unless this email comes from a known sender and you know the content is safe.
>
>
> When multiple QOM types are registered in the same file,
> it is simpler to use the the DEFINE_TYPES() macro. In
> particular because type array declared with such macro
> are easier to review.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/core/sysbus.c | 39 +++++++++++++++++----------------------
>   1 file changed, 17 insertions(+), 22 deletions(-)
>
> diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
> index f713bbfe04f..306f98406c0 100644
> --- a/hw/core/sysbus.c
> +++ b/hw/core/sysbus.c
> @@ -80,13 +80,6 @@ static void system_bus_class_init(ObjectClass *klass, void *data)
>       k->get_fw_dev_path = sysbus_get_fw_dev_path;
>   }
>
> -static const TypeInfo system_bus_info = {
> -    .name = TYPE_SYSTEM_BUS,
> -    .parent = TYPE_BUS,
> -    .instance_size = sizeof(BusState),
> -    .class_init = system_bus_class_init,
> -};
> -
>   /* Check whether an IRQ source exists */
>   bool sysbus_has_irq(SysBusDevice *dev, int n)
>   {
> @@ -306,15 +299,6 @@ static void sysbus_device_class_init(ObjectClass *klass, void *data)
>       k->user_creatable = false;
>   }
>
> -static const TypeInfo sysbus_device_type_info = {
> -    .name = TYPE_SYS_BUS_DEVICE,
> -    .parent = TYPE_DEVICE,
> -    .instance_size = sizeof(SysBusDevice),
> -    .abstract = true,
> -    .class_size = sizeof(SysBusDeviceClass),
> -    .class_init = sysbus_device_class_init,
> -};
> -
>   static BusState *main_system_bus;
>
>   static void main_system_bus_create(void)
> @@ -337,10 +321,21 @@ BusState *sysbus_get_default(void)
>       return main_system_bus;
>   }
>
> -static void sysbus_register_types(void)
> -{
> -    type_register_static(&system_bus_info);
> -    type_register_static(&sysbus_device_type_info);
> -}
> +static const TypeInfo sysbus_types[] = {
> +    {
> +        .name           = TYPE_SYSTEM_BUS,
> +        .parent         = TYPE_BUS,
> +        .instance_size  = sizeof(BusState),
> +        .class_init     = system_bus_class_init,
> +    },
> +    {
> +        .name           = TYPE_SYS_BUS_DEVICE,
> +        .parent         = TYPE_DEVICE,
> +        .instance_size  = sizeof(SysBusDevice),
> +        .abstract       = true,
> +        .class_size     = sizeof(SysBusDeviceClass),
> +        .class_init     = sysbus_device_class_init,
> +    },
> +};
>
> -type_init(sysbus_register_types)
> +DEFINE_TYPES(sysbus_types)
> --
> 2.47.1
>

Re: [PATCH 2/9] hw/sysbus: Declare QOM types using DEFINE_TYPES() macro

[Bernhard Beschow]

Am 25. Januar 2025 18:13:36 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>When multiple QOM types are registered in the same file,
>it is simpler to use the the DEFINE_TYPES() macro. In
>particular because type array declared with such macro
>are easier to review.
>
>Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>---
> hw/core/sysbus.c | 39 +++++++++++++++++----------------------
> 1 file changed, 17 insertions(+), 22 deletions(-)
>
>diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
>index f713bbfe04f..306f98406c0 100644
>--- a/hw/core/sysbus.c
>+++ b/hw/core/sysbus.c
>@@ -80,13 +80,6 @@ static void system_bus_class_init(ObjectClass *klass, void *data)
>     k->get_fw_dev_path = sysbus_get_fw_dev_path;
> }
> 
>-static const TypeInfo system_bus_info = {
>-    .name = TYPE_SYSTEM_BUS,
>-    .parent = TYPE_BUS,
>-    .instance_size = sizeof(BusState),
>-    .class_init = system_bus_class_init,
>-};
>-
> /* Check whether an IRQ source exists */
> bool sysbus_has_irq(SysBusDevice *dev, int n)
> {
>@@ -306,15 +299,6 @@ static void sysbus_device_class_init(ObjectClass *klass, void *data)
>     k->user_creatable = false;
> }
> 
>-static const TypeInfo sysbus_device_type_info = {
>-    .name = TYPE_SYS_BUS_DEVICE,
>-    .parent = TYPE_DEVICE,
>-    .instance_size = sizeof(SysBusDevice),
>-    .abstract = true,
>-    .class_size = sizeof(SysBusDeviceClass),
>-    .class_init = sysbus_device_class_init,
>-};
>-
> static BusState *main_system_bus;
> 
> static void main_system_bus_create(void)
>@@ -337,10 +321,21 @@ BusState *sysbus_get_default(void)
>     return main_system_bus;
> }
> 
>-static void sysbus_register_types(void)
>-{
>-    type_register_static(&system_bus_info);
>-    type_register_static(&sysbus_device_type_info);
>-}
>+static const TypeInfo sysbus_types[] = {
>+    {
>+        .name           = TYPE_SYSTEM_BUS,
>+        .parent         = TYPE_BUS,
>+        .instance_size  = sizeof(BusState),
>+        .class_init     = system_bus_class_init,
>+    },
>+    {
>+        .name           = TYPE_SYS_BUS_DEVICE,
>+        .parent         = TYPE_DEVICE,
>+        .instance_size  = sizeof(SysBusDevice),
>+        .abstract       = true,
>+        .class_size     = sizeof(SysBusDeviceClass),
>+        .class_init     = sysbus_device_class_init,
>+    },
>+};
> 
>-type_init(sysbus_register_types)
>+DEFINE_TYPES(sysbus_types)

Can now omit the "qom/module.h" include. With that changed:

Reviewed-by: Bernhard Beschow

[PATCH 3/9] hw/sysbus: Introduce TYPE_DYNAMIC_SYS_BUS_DEVICE

[Philippe Mathieu-Daudé]

Some TYPE_SYS_BUS_DEVICEs can be optionally dynamically
plugged on the TYPE_PLATFORM_BUS_DEVICE.
Rather than sometimes noting that with comment around
the 'user_creatable = true' line in each DeviceRealize
handler, introduce an abstract TYPE_DYNAMIC_SYS_BUS_DEVICE
class.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/sysbus.h |  2 ++
 hw/core/sysbus.c    | 14 ++++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/include/hw/sysbus.h b/include/hw/sysbus.h
index c9b1e0e90e3..81bbda10d37 100644
--- a/include/hw/sysbus.h
+++ b/include/hw/sysbus.h
@@ -19,6 +19,8 @@ DECLARE_INSTANCE_CHECKER(BusState, SYSTEM_BUS,
 OBJECT_DECLARE_TYPE(SysBusDevice, SysBusDeviceClass,
                     SYS_BUS_DEVICE)
 
+#define TYPE_DYNAMIC_SYS_BUS_DEVICE "dynamic-sysbus-device"
+
 /**
  * SysBusDeviceClass:
  *
diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
index 306f98406c0..e8d03fd28d9 100644
--- a/hw/core/sysbus.c
+++ b/hw/core/sysbus.c
@@ -321,6 +321,14 @@ BusState *sysbus_get_default(void)
     return main_system_bus;
 }
 
+static void dynamic_sysbus_device_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *k = DEVICE_CLASS(klass);
+
+    k->user_creatable = true;
+    k->hotpluggable = false;
+}
+
 static const TypeInfo sysbus_types[] = {
     {
         .name           = TYPE_SYSTEM_BUS,
@@ -336,6 +344,12 @@ static const TypeInfo sysbus_types[] = {
         .class_size     = sizeof(SysBusDeviceClass),
         .class_init     = sysbus_device_class_init,
     },
+    {
+        .name           = TYPE_DYNAMIC_SYS_BUS_DEVICE,
+        .parent         = TYPE_SYS_BUS_DEVICE,
+        .class_init     = dynamic_sysbus_device_class_init,
+        .abstract       = true,
+    }
 };
 
 DEFINE_TYPES(sysbus_types)
-- 
2.47.1

Re: [PATCH 3/9] hw/sysbus: Introduce TYPE_DYNAMIC_SYS_BUS_DEVICE

[Richard Henderson]

On 1/25/25 10:13, Philippe Mathieu-Daudé wrote:
> Some TYPE_SYS_BUS_DEVICEs can be optionally dynamically
> plugged on the TYPE_PLATFORM_BUS_DEVICE.
> Rather than sometimes noting that with comment around
> the 'user_creatable = true' line in each DeviceRealize
> handler, introduce an abstract TYPE_DYNAMIC_SYS_BUS_DEVICE
> class.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   include/hw/sysbus.h |  2 ++
>   hw/core/sysbus.c    | 14 ++++++++++++++
>   2 files changed, 16 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 3/9] hw/sysbus: Introduce TYPE_DYNAMIC_SYS_BUS_DEVICE

[CLEMENT MATHIEU--DRIF]

Reviewed-by: Clément Mathieu--Drif<clement.mathieu--drif@eviden.com>




On 25/01/2025 19:13, Philippe Mathieu-Daudé wrote:
> Caution: External email. Do not open attachments or click links, unless this email comes from a known sender and you know the content is safe.
>
>
> Some TYPE_SYS_BUS_DEVICEs can be optionally dynamically
> plugged on the TYPE_PLATFORM_BUS_DEVICE.
> Rather than sometimes noting that with comment around
> the 'user_creatable = true' line in each DeviceRealize
> handler, introduce an abstract TYPE_DYNAMIC_SYS_BUS_DEVICE
> class.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/hw/sysbus.h |  2 ++
>   hw/core/sysbus.c    | 14 ++++++++++++++
>   2 files changed, 16 insertions(+)
>
> diff --git a/include/hw/sysbus.h b/include/hw/sysbus.h
> index c9b1e0e90e3..81bbda10d37 100644
> --- a/include/hw/sysbus.h
> +++ b/include/hw/sysbus.h
> @@ -19,6 +19,8 @@ DECLARE_INSTANCE_CHECKER(BusState, SYSTEM_BUS,
>   OBJECT_DECLARE_TYPE(SysBusDevice, SysBusDeviceClass,
>                       SYS_BUS_DEVICE)
>
> +#define TYPE_DYNAMIC_SYS_BUS_DEVICE "dynamic-sysbus-device"
> +
>   /**
>    * SysBusDeviceClass:
>    *
> diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
> index 306f98406c0..e8d03fd28d9 100644
> --- a/hw/core/sysbus.c
> +++ b/hw/core/sysbus.c
> @@ -321,6 +321,14 @@ BusState *sysbus_get_default(void)
>       return main_system_bus;
>   }
>
> +static void dynamic_sysbus_device_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *k = DEVICE_CLASS(klass);
> +
> +    k->user_creatable = true;
> +    k->hotpluggable = false;
> +}
> +
>   static const TypeInfo sysbus_types[] = {
>       {
>           .name           = TYPE_SYSTEM_BUS,
> @@ -336,6 +344,12 @@ static const TypeInfo sysbus_types[] = {
>           .class_size     = sizeof(SysBusDeviceClass),
>           .class_init     = sysbus_device_class_init,
>       },
> +    {
> +        .name           = TYPE_DYNAMIC_SYS_BUS_DEVICE,
> +        .parent         = TYPE_SYS_BUS_DEVICE,
> +        .class_init     = dynamic_sysbus_device_class_init,
> +        .abstract       = true,
> +    }
>   };
>
>   DEFINE_TYPES(sysbus_types)
> --
> 2.47.1
>

[PATCH 4/9] hw/vfio: Have VFIO_PLATFORM devices inherit from DYNAMIC_SYS_BUS_DEVICE

[Philippe Mathieu-Daudé]

Do not explain why VFIO_PLATFORM devices are user_creatable,
have them inherit TYPE_DYNAMIC_SYS_BUS_DEVICE, to explicit
they can optionally be plugged on TYPE_PLATFORM_BUS_DEVICE.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/vfio/amd-xgbe.c      | 2 --
 hw/vfio/calxeda-xgmac.c | 2 --
 hw/vfio/platform.c      | 4 +---
 3 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/hw/vfio/amd-xgbe.c b/hw/vfio/amd-xgbe.c
index 96bd608b8dd..aaa96903db0 100644
--- a/hw/vfio/amd-xgbe.c
+++ b/hw/vfio/amd-xgbe.c
@@ -41,8 +41,6 @@ static void vfio_amd_xgbe_class_init(ObjectClass *klass, void *data)
                                     &vcxc->parent_realize);
     dc->desc = "VFIO AMD XGBE";
     dc->vmsd = &vfio_platform_amd_xgbe_vmstate;
-    /* Supported by TYPE_VIRT_MACHINE */
-    dc->user_creatable = true;
 }
 
 static const TypeInfo vfio_amd_xgbe_dev_info = {
diff --git a/hw/vfio/calxeda-xgmac.c b/hw/vfio/calxeda-xgmac.c
index 87c382e7361..b016d42b496 100644
--- a/hw/vfio/calxeda-xgmac.c
+++ b/hw/vfio/calxeda-xgmac.c
@@ -41,8 +41,6 @@ static void vfio_calxeda_xgmac_class_init(ObjectClass *klass, void *data)
                                     &vcxc->parent_realize);
     dc->desc = "VFIO Calxeda XGMAC";
     dc->vmsd = &vfio_platform_calxeda_xgmac_vmstate;
-    /* Supported by TYPE_VIRT_MACHINE */
-    dc->user_creatable = true;
 }
 
 static const TypeInfo vfio_calxeda_xgmac_dev_info = {
diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
index 1070a2113a1..f491f4dc954 100644
--- a/hw/vfio/platform.c
+++ b/hw/vfio/platform.c
@@ -672,13 +672,11 @@ static void vfio_platform_class_init(ObjectClass *klass, void *data)
     dc->desc = "VFIO-based platform device assignment";
     sbc->connect_irq_notifier = vfio_start_irqfd_injection;
     set_bit(DEVICE_CATEGORY_MISC, dc->categories);
-    /* Supported by TYPE_VIRT_MACHINE */
-    dc->user_creatable = true;
 }
 
 static const TypeInfo vfio_platform_dev_info = {
     .name = TYPE_VFIO_PLATFORM,
-    .parent = TYPE_SYS_BUS_DEVICE,
+    .parent = TYPE_DYNAMIC_SYS_BUS_DEVICE,
     .instance_size = sizeof(VFIOPlatformDevice),
     .instance_init = vfio_platform_instance_init,
     .class_init = vfio_platform_class_init,
-- 
2.47.1

Re: [PATCH 4/9] hw/vfio: Have VFIO_PLATFORM devices inherit from DYNAMIC_SYS_BUS_DEVICE

[Richard Henderson]

On 1/25/25 10:13, Philippe Mathieu-Daudé wrote:
> Do not explain why VFIO_PLATFORM devices are user_creatable,
> have them inherit TYPE_DYNAMIC_SYS_BUS_DEVICE, to explicit
> they can optionally be plugged on TYPE_PLATFORM_BUS_DEVICE.

to make it explicit that they can be

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 5/9] hw/display: Have RAMFB device inherit from DYNAMIC_SYS_BUS_DEVICE

[Philippe Mathieu-Daudé]

Because the RAM FB device can be optionally plugged on the
TYPE_PLATFORM_BUS_DEVICE, have it inherit TYPE_DYNAMIC_SYS_BUS_DEVICE.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/display/ramfb-standalone.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/display/ramfb-standalone.c b/hw/display/ramfb-standalone.c
index 6c35028965d..1be106b57f2 100644
--- a/hw/display/ramfb-standalone.c
+++ b/hw/display/ramfb-standalone.c
@@ -72,13 +72,12 @@ static void ramfb_class_initfn(ObjectClass *klass, void *data)
     dc->vmsd = &ramfb_dev_vmstate;
     dc->realize = ramfb_realizefn;
     dc->desc = "ram framebuffer standalone device";
-    dc->user_creatable = true;
     device_class_set_props(dc, ramfb_properties);
 }
 
 static const TypeInfo ramfb_info = {
     .name          = TYPE_RAMFB_DEVICE,
-    .parent        = TYPE_SYS_BUS_DEVICE,
+    .parent        = TYPE_DYNAMIC_SYS_BUS_DEVICE,
     .instance_size = sizeof(RAMFBStandaloneState),
     .class_init    = ramfb_class_initfn,
 };
-- 
2.47.1

Re: [PATCH 5/9] hw/display: Have RAMFB device inherit from DYNAMIC_SYS_BUS_DEVICE

[Richard Henderson]

On 1/25/25 10:13, Philippe Mathieu-Daudé wrote:
> Because the RAM FB device can be optionally plugged on the
> TYPE_PLATFORM_BUS_DEVICE, have it inherit TYPE_DYNAMIC_SYS_BUS_DEVICE.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/display/ramfb-standalone.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH 5/9] hw/display: Have RAMFB device inherit from DYNAMIC_SYS_BUS_DEVICE

[CLEMENT MATHIEU--DRIF]

Reviewed-by: Clément Mathieu--Drif<clement.mathieu--drif@eviden.com>




On 25/01/2025 19:13, Philippe Mathieu-Daudé wrote:
> Caution: External email. Do not open attachments or click links, unless this email comes from a known sender and you know the content is safe.
>
>
> Because the RAM FB device can be optionally plugged on the
> TYPE_PLATFORM_BUS_DEVICE, have it inherit TYPE_DYNAMIC_SYS_BUS_DEVICE.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/display/ramfb-standalone.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/hw/display/ramfb-standalone.c b/hw/display/ramfb-standalone.c
> index 6c35028965d..1be106b57f2 100644
> --- a/hw/display/ramfb-standalone.c
> +++ b/hw/display/ramfb-standalone.c
> @@ -72,13 +72,12 @@ static void ramfb_class_initfn(ObjectClass *klass, void *data)
>       dc->vmsd = &ramfb_dev_vmstate;
>       dc->realize = ramfb_realizefn;
>       dc->desc = "ram framebuffer standalone device";
> -    dc->user_creatable = true;
>       device_class_set_props(dc, ramfb_properties);
>   }
>
>   static const TypeInfo ramfb_info = {
>       .name          = TYPE_RAMFB_DEVICE,
> -    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .parent        = TYPE_DYNAMIC_SYS_BUS_DEVICE,
>       .instance_size = sizeof(RAMFBStandaloneState),
>       .class_init    = ramfb_class_initfn,
>   };
> --
> 2.47.1
>

[PATCH 6/9] hw/i386: Have X86_IOMMU devices inherit from DYNAMIC_SYS_BUS_DEVICE

[Philippe Mathieu-Daudé]

Do not explain why _X86_IOMMU devices are user_creatable,
have them inherit TYPE_DYNAMIC_SYS_BUS_DEVICE, to explicit
they can optionally be plugged on TYPE_PLATFORM_BUS_DEVICE.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/amd_iommu.c   | 2 --
 hw/i386/intel_iommu.c | 2 --
 hw/i386/x86-iommu.c   | 2 +-
 3 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 6b13ce894b1..e8e084c7cf8 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -1687,8 +1687,6 @@ static void amdvi_sysbus_class_init(ObjectClass *klass, void *data)
     dc->hotpluggable = false;
     dc_class->realize = amdvi_sysbus_realize;
     dc_class->int_remap = amdvi_int_remap;
-    /* Supported by the pc-q35-* machine types */
-    dc->user_creatable = true;
     set_bit(DEVICE_CATEGORY_MISC, dc->categories);
     dc->desc = "AMD IOMMU (AMD-Vi) DMA Remapping device";
     device_class_set_props(dc, amdvi_properties);
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index f366c223d0e..7fde0603bfe 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -4871,8 +4871,6 @@ static void vtd_class_init(ObjectClass *klass, void *data)
     dc->hotpluggable = false;
     x86_class->realize = vtd_realize;
     x86_class->int_remap = vtd_int_remap;
-    /* Supported by the pc-q35-* machine types */
-    dc->user_creatable = true;
     set_bit(DEVICE_CATEGORY_MISC, dc->categories);
     dc->desc = "Intel IOMMU (VT-d) DMA Remapping device";
 }
diff --git a/hw/i386/x86-iommu.c b/hw/i386/x86-iommu.c
index fed34b2fcfa..5cdd165af0d 100644
--- a/hw/i386/x86-iommu.c
+++ b/hw/i386/x86-iommu.c
@@ -146,7 +146,7 @@ bool x86_iommu_ir_supported(X86IOMMUState *s)
 
 static const TypeInfo x86_iommu_info = {
     .name          = TYPE_X86_IOMMU_DEVICE,
-    .parent        = TYPE_SYS_BUS_DEVICE,
+    .parent        = TYPE_DYNAMIC_SYS_BUS_DEVICE,
     .instance_size = sizeof(X86IOMMUState),
     .class_init    = x86_iommu_class_init,
     .class_size    = sizeof(X86IOMMUClass),
-- 
2.47.1

Re: [PATCH 6/9] hw/i386: Have X86_IOMMU devices inherit from DYNAMIC_SYS_BUS_DEVICE

[Richard Henderson]

On 1/25/25 10:13, Philippe Mathieu-Daudé wrote:
> Do not explain why _X86_IOMMU devices are user_creatable,
> have them inherit TYPE_DYNAMIC_SYS_BUS_DEVICE, to explicit
> they can optionally be plugged on TYPE_PLATFORM_BUS_DEVICE.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/i386/amd_iommu.c   | 2 --
>   hw/i386/intel_iommu.c | 2 --
>   hw/i386/x86-iommu.c   | 2 +-
>   3 files changed, 1 insertion(+), 5 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 6/9] hw/i386: Have X86_IOMMU devices inherit from DYNAMIC_SYS_BUS_DEVICE

[CLEMENT MATHIEU--DRIF]

Reviewed-by: Clément Mathieu--Drif<clement.mathieu--drif@eviden.com>

Thanks phil

On 25/01/2025 19:13, Philippe Mathieu-Daudé wrote:
> Caution: External email. Do not open attachments or click links, unless this email comes from a known sender and you know the content is safe.
> 
> 
> Do not explain why _X86_IOMMU devices are user_creatable,
> have them inherit TYPE_DYNAMIC_SYS_BUS_DEVICE, to explicit
> they can optionally be plugged on TYPE_PLATFORM_BUS_DEVICE.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/i386/amd_iommu.c   | 2 --
>   hw/i386/intel_iommu.c | 2 --
>   hw/i386/x86-iommu.c   | 2 +-
>   3 files changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
> index 6b13ce894b1..e8e084c7cf8 100644
> --- a/hw/i386/amd_iommu.c
> +++ b/hw/i386/amd_iommu.c
> @@ -1687,8 +1687,6 @@ static void amdvi_sysbus_class_init(ObjectClass *klass, void *data)
>       dc->hotpluggable = false;
>       dc_class->realize = amdvi_sysbus_realize;
>       dc_class->int_remap = amdvi_int_remap;
> -    /* Supported by the pc-q35-* machine types */
> -    dc->user_creatable = true;
>       set_bit(DEVICE_CATEGORY_MISC, dc->categories);
>       dc->desc = "AMD IOMMU (AMD-Vi) DMA Remapping device";
>       device_class_set_props(dc, amdvi_properties);
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index f366c223d0e..7fde0603bfe 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -4871,8 +4871,6 @@ static void vtd_class_init(ObjectClass *klass, void *data)
>       dc->hotpluggable = false;
>       x86_class->realize = vtd_realize;
>       x86_class->int_remap = vtd_int_remap;
> -    /* Supported by the pc-q35-* machine types */
> -    dc->user_creatable = true;
>       set_bit(DEVICE_CATEGORY_MISC, dc->categories);
>       dc->desc = "Intel IOMMU (VT-d) DMA Remapping device";
>   }
> diff --git a/hw/i386/x86-iommu.c b/hw/i386/x86-iommu.c
> index fed34b2fcfa..5cdd165af0d 100644
> --- a/hw/i386/x86-iommu.c
> +++ b/hw/i386/x86-iommu.c
> @@ -146,7 +146,7 @@ bool x86_iommu_ir_supported(X86IOMMUState *s)
> 
>   static const TypeInfo x86_iommu_info = {
>       .name          = TYPE_X86_IOMMU_DEVICE,
> -    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .parent        = TYPE_DYNAMIC_SYS_BUS_DEVICE,
>       .instance_size = sizeof(X86IOMMUState),
>       .class_init    = x86_iommu_class_init,
>       .class_size    = sizeof(X86IOMMUClass),
> --
> 2.47.1
> 

[PATCH 7/9] hw/net: Have eTSEC device inherit from DYNAMIC_SYS_BUS_DEVICE

[Philippe Mathieu-Daudé]

Because the network eTSEC device can be optionally plugged on the
TYPE_PLATFORM_BUS_DEVICE, have it inherit TYPE_DYNAMIC_SYS_BUS_DEVICE.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/net/fsl_etsec/etsec.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/net/fsl_etsec/etsec.c b/hw/net/fsl_etsec/etsec.c
index 781b9003954..3ce4fa2662d 100644
--- a/hw/net/fsl_etsec/etsec.c
+++ b/hw/net/fsl_etsec/etsec.c
@@ -425,14 +425,12 @@ static void etsec_class_init(ObjectClass *klass, void *data)
     dc->realize = etsec_realize;
     device_class_set_legacy_reset(dc, etsec_reset);
     device_class_set_props(dc, etsec_properties);
-    /* Supported by ppce500 machine */
-    dc->user_creatable = true;
 }
 
 static const TypeInfo etsec_types[] = {
     {
         .name          = TYPE_ETSEC_COMMON,
-        .parent        = TYPE_SYS_BUS_DEVICE,
+        .parent        = TYPE_DYNAMIC_SYS_BUS_DEVICE,
         .instance_size = sizeof(eTSEC),
         .class_init    = etsec_class_init,
         .instance_init = etsec_instance_init,
-- 
2.47.1

Re: [PATCH 7/9] hw/net: Have eTSEC device inherit from DYNAMIC_SYS_BUS_DEVICE

[Richard Henderson]

On 1/25/25 10:13, Philippe Mathieu-Daudé wrote:
> Because the network eTSEC device can be optionally plugged on the
> TYPE_PLATFORM_BUS_DEVICE, have it inherit TYPE_DYNAMIC_SYS_BUS_DEVICE.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/net/fsl_etsec/etsec.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 7/9] hw/net: Have eTSEC device inherit from DYNAMIC_SYS_BUS_DEVICE

[CLEMENT MATHIEU--DRIF]

Reviewed-by: Clément Mathieu--Drif<clement.mathieu--drif@eviden.com>



On 25/01/2025 19:13, Philippe Mathieu-Daudé wrote:
> Caution: External email. Do not open attachments or click links, unless this email comes from a known sender and you know the content is safe.
>
>
> Because the network eTSEC device can be optionally plugged on the
> TYPE_PLATFORM_BUS_DEVICE, have it inherit TYPE_DYNAMIC_SYS_BUS_DEVICE.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/net/fsl_etsec/etsec.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/hw/net/fsl_etsec/etsec.c b/hw/net/fsl_etsec/etsec.c
> index 781b9003954..3ce4fa2662d 100644
> --- a/hw/net/fsl_etsec/etsec.c
> +++ b/hw/net/fsl_etsec/etsec.c
> @@ -425,14 +425,12 @@ static void etsec_class_init(ObjectClass *klass, void *data)
>       dc->realize = etsec_realize;
>       device_class_set_legacy_reset(dc, etsec_reset);
>       device_class_set_props(dc, etsec_properties);
> -    /* Supported by ppce500 machine */
> -    dc->user_creatable = true;
>   }
>
>   static const TypeInfo etsec_types[] = {
>       {
>           .name          = TYPE_ETSEC_COMMON,
> -        .parent        = TYPE_SYS_BUS_DEVICE,
> +        .parent        = TYPE_DYNAMIC_SYS_BUS_DEVICE,
>           .instance_size = sizeof(eTSEC),
>           .class_init    = etsec_class_init,
>           .instance_init = etsec_instance_init,
> --
> 2.47.1
>

Re: [PATCH 7/9] hw/net: Have eTSEC device inherit from DYNAMIC_SYS_BUS_DEVICE

[Bernhard Beschow]

Am 25. Januar 2025 18:13:41 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>Because the network eTSEC device can be optionally plugged on the
>TYPE_PLATFORM_BUS_DEVICE, have it inherit TYPE_DYNAMIC_SYS_BUS_DEVICE.
>
>Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Tested-by: Bernhard Beschow <shentey@gmail.com>
Acked-by: Bernhard Beschow <shentey@gmail.com>

>---
> hw/net/fsl_etsec/etsec.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
>diff --git a/hw/net/fsl_etsec/etsec.c b/hw/net/fsl_etsec/etsec.c
>index 781b9003954..3ce4fa2662d 100644
>--- a/hw/net/fsl_etsec/etsec.c
>+++ b/hw/net/fsl_etsec/etsec.c
>@@ -425,14 +425,12 @@ static void etsec_class_init(ObjectClass *klass, void *data)
>     dc->realize = etsec_realize;
>     device_class_set_legacy_reset(dc, etsec_reset);
>     device_class_set_props(dc, etsec_properties);
>-    /* Supported by ppce500 machine */
>-    dc->user_creatable = true;
> }
> 
> static const TypeInfo etsec_types[] = {
>     {
>         .name          = TYPE_ETSEC_COMMON,
>-        .parent        = TYPE_SYS_BUS_DEVICE,
>+        .parent        = TYPE_DYNAMIC_SYS_BUS_DEVICE,
>         .instance_size = sizeof(eTSEC),
>         .class_init    = etsec_class_init,
>         .instance_init = etsec_instance_init,

[PATCH 8/9] hw/tpm: Have TPM TIS sysbus device inherit from DYNAMIC_SYS_BUS_DEVICE

[Philippe Mathieu-Daudé]

Because the TPM TIS sysbus device can be optionally plugged on the
TYPE_PLATFORM_BUS_DEVICE, have it inherit TYPE_DYNAMIC_SYS_BUS_DEVICE.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/tpm/tpm_tis_sysbus.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/tpm/tpm_tis_sysbus.c b/hw/tpm/tpm_tis_sysbus.c
index ee0bfe9538e..4f187690a28 100644
--- a/hw/tpm/tpm_tis_sysbus.c
+++ b/hw/tpm/tpm_tis_sysbus.c
@@ -133,7 +133,6 @@ static void tpm_tis_sysbus_class_init(ObjectClass *klass, void *data)
     dc->vmsd  = &vmstate_tpm_tis_sysbus;
     tc->model = TPM_MODEL_TPM_TIS;
     dc->realize = tpm_tis_sysbus_realizefn;
-    dc->user_creatable = true;
     device_class_set_legacy_reset(dc, tpm_tis_sysbus_reset);
     tc->request_completed = tpm_tis_sysbus_request_completed;
     tc->get_version = tpm_tis_sysbus_get_tpm_version;
@@ -142,7 +141,7 @@ static void tpm_tis_sysbus_class_init(ObjectClass *klass, void *data)
 
 static const TypeInfo tpm_tis_sysbus_info = {
     .name = TYPE_TPM_TIS_SYSBUS,
-    .parent = TYPE_SYS_BUS_DEVICE,
+    .parent = TYPE_DYNAMIC_SYS_BUS_DEVICE,
     .instance_size = sizeof(TPMStateSysBus),
     .instance_init = tpm_tis_sysbus_initfn,
     .class_init  = tpm_tis_sysbus_class_init,
-- 
2.47.1

Re: [PATCH 8/9] hw/tpm: Have TPM TIS sysbus device inherit from DYNAMIC_SYS_BUS_DEVICE

[Richard Henderson]

On 1/25/25 10:13, Philippe Mathieu-Daudé wrote:
> Because the TPM TIS sysbus device can be optionally plugged on the
> TYPE_PLATFORM_BUS_DEVICE, have it inherit TYPE_DYNAMIC_SYS_BUS_DEVICE.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/tpm/tpm_tis_sysbus.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH 8/9] hw/tpm: Have TPM TIS sysbus device inherit from DYNAMIC_SYS_BUS_DEVICE

[CLEMENT MATHIEU--DRIF]

Reviewed-by: Clément Mathieu--Drif<clement.mathieu--drif@eviden.com>



On 25/01/2025 19:13, Philippe Mathieu-Daudé wrote:
> Caution: External email. Do not open attachments or click links, unless this email comes from a known sender and you know the content is safe.
>
>
> Because the TPM TIS sysbus device can be optionally plugged on the
> TYPE_PLATFORM_BUS_DEVICE, have it inherit TYPE_DYNAMIC_SYS_BUS_DEVICE.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/tpm/tpm_tis_sysbus.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/hw/tpm/tpm_tis_sysbus.c b/hw/tpm/tpm_tis_sysbus.c
> index ee0bfe9538e..4f187690a28 100644
> --- a/hw/tpm/tpm_tis_sysbus.c
> +++ b/hw/tpm/tpm_tis_sysbus.c
> @@ -133,7 +133,6 @@ static void tpm_tis_sysbus_class_init(ObjectClass *klass, void *data)
>       dc->vmsd  = &vmstate_tpm_tis_sysbus;
>       tc->model = TPM_MODEL_TPM_TIS;
>       dc->realize = tpm_tis_sysbus_realizefn;
> -    dc->user_creatable = true;
>       device_class_set_legacy_reset(dc, tpm_tis_sysbus_reset);
>       tc->request_completed = tpm_tis_sysbus_request_completed;
>       tc->get_version = tpm_tis_sysbus_get_tpm_version;
> @@ -142,7 +141,7 @@ static void tpm_tis_sysbus_class_init(ObjectClass *klass, void *data)
>
>   static const TypeInfo tpm_tis_sysbus_info = {
>       .name = TYPE_TPM_TIS_SYSBUS,
> -    .parent = TYPE_SYS_BUS_DEVICE,
> +    .parent = TYPE_DYNAMIC_SYS_BUS_DEVICE,
>       .instance_size = sizeof(TPMStateSysBus),
>       .instance_init = tpm_tis_sysbus_initfn,
>       .class_init  = tpm_tis_sysbus_class_init,
> --
> 2.47.1
>

Re: [PATCH 8/9] hw/tpm: Have TPM TIS sysbus device inherit from DYNAMIC_SYS_BUS_DEVICE

[Stefan Berger]

On 1/25/25 1:13 PM, Philippe Mathieu-Daudé wrote:
> Because the TPM TIS sysbus device can be optionally plugged on the
> TYPE_PLATFORM_BUS_DEVICE, have it inherit TYPE_DYNAMIC_SYS_BUS_DEVICE.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>

> ---
>   hw/tpm/tpm_tis_sysbus.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/hw/tpm/tpm_tis_sysbus.c b/hw/tpm/tpm_tis_sysbus.c
> index ee0bfe9538e..4f187690a28 100644
> --- a/hw/tpm/tpm_tis_sysbus.c
> +++ b/hw/tpm/tpm_tis_sysbus.c
> @@ -133,7 +133,6 @@ static void tpm_tis_sysbus_class_init(ObjectClass *klass, void *data)
>       dc->vmsd  = &vmstate_tpm_tis_sysbus;
>       tc->model = TPM_MODEL_TPM_TIS;
>       dc->realize = tpm_tis_sysbus_realizefn;
> -    dc->user_creatable = true;
>       device_class_set_legacy_reset(dc, tpm_tis_sysbus_reset);
>       tc->request_completed = tpm_tis_sysbus_request_completed;
>       tc->get_version = tpm_tis_sysbus_get_tpm_version;
> @@ -142,7 +141,7 @@ static void tpm_tis_sysbus_class_init(ObjectClass *klass, void *data)
>   
>   static const TypeInfo tpm_tis_sysbus_info = {
>       .name = TYPE_TPM_TIS_SYSBUS,
> -    .parent = TYPE_SYS_BUS_DEVICE,
> +    .parent = TYPE_DYNAMIC_SYS_BUS_DEVICE,
>       .instance_size = sizeof(TPMStateSysBus),
>       .instance_init = tpm_tis_sysbus_initfn,
>       .class_init  = tpm_tis_sysbus_class_init,

[RFC PATCH 9/9] hw/xen: Have legacy Xen backend inherit from DYNAMIC_SYS_BUS_DEVICE

[Philippe Mathieu-Daudé]

Because the legacy Xen backend devices can optionally be plugged on the
TYPE_PLATFORM_BUS_DEVICE, have it inherit TYPE_DYNAMIC_SYS_BUS_DEVICE.
Remove the implicit TYPE_XENSYSDEV instance_size.

Untested, but I'm surprised the legacy devices work because they
had a broken instance size (QDev instead of Sysbus...), so accesses
of XenLegacyDevice fields were overwritting sysbus ones.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/xen/xen_pvdev.h  | 3 ++-
 hw/xen/xen-legacy-backend.c | 7 ++-----
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/include/hw/xen/xen_pvdev.h b/include/hw/xen/xen_pvdev.h
index 0c984440476..48950dc2b57 100644
--- a/include/hw/xen/xen_pvdev.h
+++ b/include/hw/xen/xen_pvdev.h
@@ -32,7 +32,8 @@ struct XenDevOps {
 };
 
 struct XenLegacyDevice {
-    DeviceState        qdev;
+    SysBusDevice parent_obj;
+
     const char         *type;
     int                dom;
     int                dev;
diff --git a/hw/xen/xen-legacy-backend.c b/hw/xen/xen-legacy-backend.c
index 118c571b3a7..4d079e35d83 100644
--- a/hw/xen/xen-legacy-backend.c
+++ b/hw/xen/xen-legacy-backend.c
@@ -640,16 +640,14 @@ static void xendev_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     set_bit(DEVICE_CATEGORY_MISC, dc->categories);
-    /* xen-backend devices can be plugged/unplugged dynamically */
-    dc->user_creatable = true;
     dc->bus_type = TYPE_XENSYSBUS;
 }
 
 static const TypeInfo xendev_type_info = {
     .name          = TYPE_XENBACKEND,
-    .parent        = TYPE_DEVICE,
+    .parent        = TYPE_DYNAMIC_SYS_BUS_DEVICE,
     .class_init    = xendev_class_init,
-    .instance_size = sizeof(struct XenLegacyDevice),
+    .instance_size = sizeof(XenLegacyDevice),
 };
 
 static void xen_sysbus_class_init(ObjectClass *klass, void *data)
@@ -672,7 +670,6 @@ static const TypeInfo xensysbus_info = {
 static const TypeInfo xensysdev_info = {
     .name          = TYPE_XENSYSDEV,
     .parent        = TYPE_SYS_BUS_DEVICE,
-    .instance_size = sizeof(SysBusDevice),
 };
 
 static void xenbe_register_types(void)
-- 
2.47.1

[PATCH] hw/*/xen*: Prefer QOM cast for XenLegacyDevice

[Bernhard Beschow]

Makes the code less sensitive regarding changes in the class hierarchy which
will be performed in the next patch.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/usb/xen-usb.c            | 6 +++---
 hw/xen/xen-legacy-backend.c | 2 +-
 hw/xen/xen_pvdev.c          | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/usb/xen-usb.c b/hw/usb/xen-usb.c
index 13901625c0..3da30efc44 100644
--- a/hw/usb/xen-usb.c
+++ b/hw/usb/xen-usb.c
@@ -755,10 +755,10 @@ static void usbback_portid_add(struct usbback_info *usbif, unsigned port,
 
     qdict = qdict_new();
     qdict_put_str(qdict, "driver", "usb-host");
-    tmp = g_strdup_printf("%s.0", usbif->xendev.qdev.id);
+    tmp = g_strdup_printf("%s.0", DEVICE(&usbif->xendev)->id);
     qdict_put_str(qdict, "bus", tmp);
     g_free(tmp);
-    tmp = g_strdup_printf("%s-%u", usbif->xendev.qdev.id, port);
+    tmp = g_strdup_printf("%s-%u", DEVICE(&usbif->xendev)->id, port);
     qdict_put_str(qdict, "id", tmp);
     g_free(tmp);
     qdict_put_int(qdict, "port", port);
@@ -1022,7 +1022,7 @@ static void usbback_alloc(struct XenLegacyDevice *xendev)
     usbif = container_of(xendev, struct usbback_info, xendev);
 
     usb_bus_new(&usbif->bus, sizeof(usbif->bus), &xen_usb_bus_ops,
-                DEVICE(&xendev->qdev));
+                DEVICE(xendev));
     for (i = 0; i < USBBACK_MAXPORTS; i++) {
         p = &(usbif->ports[i].port);
         usb_register_port(&usbif->bus, p, usbif, i, &xen_usb_port_ops,
diff --git a/hw/xen/xen-legacy-backend.c b/hw/xen/xen-legacy-backend.c
index 118c571b3a..ca2fe0e6b3 100644
--- a/hw/xen/xen-legacy-backend.c
+++ b/hw/xen/xen-legacy-backend.c
@@ -163,7 +163,7 @@ static struct XenLegacyDevice *xen_be_get_xendev(const char *type, int dom,
 
     /* init new xendev */
     xendev = g_malloc0(ops->size);
-    object_initialize(&xendev->qdev, ops->size, TYPE_XENBACKEND);
+    object_initialize(xendev, ops->size, TYPE_XENBACKEND);
     OBJECT(xendev)->free = g_free;
     qdev_set_id(DEVICE(xendev), g_strdup_printf("xen-%s-%d", type, dev),
                 &error_fatal);
diff --git a/hw/xen/xen_pvdev.c b/hw/xen/xen_pvdev.c
index c9143ba259..fe95b62d13 100644
--- a/hw/xen/xen_pvdev.c
+++ b/hw/xen/xen_pvdev.c
@@ -273,7 +273,7 @@ void xen_pv_del_xendev(struct XenLegacyDevice *xendev)
 
     QTAILQ_REMOVE(&xendevs, xendev, next);
 
-    qdev_unplug(&xendev->qdev, NULL);
+    qdev_unplug(DEVICE(xendev), NULL);
 }
 
 void xen_pv_insert_xendev(struct XenLegacyDevice *xendev)
-- 
2.48.1

Re: [PATCH] hw/*/xen*: Prefer QOM cast for XenLegacyDevice

[Philippe Mathieu-Daudé]

On 27/1/25 10:41, Bernhard Beschow wrote:
> Makes the code less sensitive regarding changes in the class hierarchy which
> will be performed in the next patch.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   hw/usb/xen-usb.c            | 6 +++---
>   hw/xen/xen-legacy-backend.c | 2 +-
>   hw/xen/xen_pvdev.c          | 2 +-
>   3 files changed, 5 insertions(+), 5 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [RFC PATCH 9/9] hw/xen: Have legacy Xen backend inherit from DYNAMIC_SYS_BUS_DEVICE

[Bernhard Beschow]

Am 25. Januar 2025 18:13:43 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>Because the legacy Xen backend devices can optionally be plugged on the
>TYPE_PLATFORM_BUS_DEVICE, have it inherit TYPE_DYNAMIC_SYS_BUS_DEVICE.
>Remove the implicit TYPE_XENSYSDEV instance_size.
>
>Untested, but I'm surprised the legacy devices work because they
>had a broken instance size (QDev instead of Sysbus...), so accesses
>of XenLegacyDevice fields were overwritting sysbus ones.
>
>Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>---
> include/hw/xen/xen_pvdev.h  | 3 ++-
> hw/xen/xen-legacy-backend.c | 7 ++-----
> 2 files changed, 4 insertions(+), 6 deletions(-)
>
>diff --git a/include/hw/xen/xen_pvdev.h b/include/hw/xen/xen_pvdev.h
>index 0c984440476..48950dc2b57 100644
>--- a/include/hw/xen/xen_pvdev.h
>+++ b/include/hw/xen/xen_pvdev.h
>@@ -32,7 +32,8 @@ struct XenDevOps {
> };
> 
> struct XenLegacyDevice {
>-    DeviceState        qdev;
>+    SysBusDevice parent_obj;

This then needs sysbus.h rather than qdev-core.h include.

Moreover, the patch in the reply needs to be inserted into the series before this patch.

Both are needed for the patch to compile.

Best regards,
Bernhard

>+
>     const char         *type;
>     int                dom;
>     int                dev;
>diff --git a/hw/xen/xen-legacy-backend.c b/hw/xen/xen-legacy-backend.c
>index 118c571b3a7..4d079e35d83 100644
>--- a/hw/xen/xen-legacy-backend.c
>+++ b/hw/xen/xen-legacy-backend.c
>@@ -640,16 +640,14 @@ static void xendev_class_init(ObjectClass *klass, void *data)
>     DeviceClass *dc = DEVICE_CLASS(klass);
> 
>     set_bit(DEVICE_CATEGORY_MISC, dc->categories);
>-    /* xen-backend devices can be plugged/unplugged dynamically */
>-    dc->user_creatable = true;
>     dc->bus_type = TYPE_XENSYSBUS;
> }
> 
> static const TypeInfo xendev_type_info = {
>     .name          = TYPE_XENBACKEND,
>-    .parent        = TYPE_DEVICE,
>+    .parent        = TYPE_DYNAMIC_SYS_BUS_DEVICE,
>     .class_init    = xendev_class_init,
>-    .instance_size = sizeof(struct XenLegacyDevice),
>+    .instance_size = sizeof(XenLegacyDevice),
> };
> 
> static void xen_sysbus_class_init(ObjectClass *klass, void *data)
>@@ -672,7 +670,6 @@ static const TypeInfo xensysbus_info = {
> static const TypeInfo xensysdev_info = {
>     .name          = TYPE_XENSYSDEV,
>     .parent        = TYPE_SYS_BUS_DEVICE,
>-    .instance_size = sizeof(SysBusDevice),
> };
> 
> static void xenbe_register_types(void)

Re: [RFC PATCH 9/9] hw/xen: Have legacy Xen backend inherit from DYNAMIC_SYS_BUS_DEVICE

[Philippe Mathieu-Daudé]

Hi Bernhard,

On 27/1/25 10:46, Bernhard Beschow wrote:
> Am 25. Januar 2025 18:13:43 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>> Because the legacy Xen backend devices can optionally be plugged on the
>> TYPE_PLATFORM_BUS_DEVICE, have it inherit TYPE_DYNAMIC_SYS_BUS_DEVICE.
>> Remove the implicit TYPE_XENSYSDEV instance_size.
>>
>> Untested, but I'm surprised the legacy devices work because they
>> had a broken instance size (QDev instead of Sysbus...), so accesses
>> of XenLegacyDevice fields were overwritting sysbus ones.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> include/hw/xen/xen_pvdev.h  | 3 ++-
>> hw/xen/xen-legacy-backend.c | 7 ++-----
>> 2 files changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/include/hw/xen/xen_pvdev.h b/include/hw/xen/xen_pvdev.h
>> index 0c984440476..48950dc2b57 100644
>> --- a/include/hw/xen/xen_pvdev.h
>> +++ b/include/hw/xen/xen_pvdev.h
>> @@ -32,7 +32,8 @@ struct XenDevOps {
>> };
>>
>> struct XenLegacyDevice {
>> -    DeviceState        qdev;
>> +    SysBusDevice parent_obj;
> 
> This then needs sysbus.h rather than qdev-core.h include.
> 
> Moreover, the patch in the reply needs to be inserted into the series before this patch.
> 
> Both are needed for the patch to compile.

Per your reply on patch #7, might I include your

Tested-by: Bernhard Beschow <shentey@gmail.com>
Acked-by: Bernhard Beschow <shentey@gmail.com>
(or R-b)

squashing:

-- >8 --
diff --git a/include/hw/xen/xen_pvdev.h b/include/hw/xen/xen_pvdev.h
index 48950dc2b57..629bec90d09 100644
--- a/include/hw/xen/xen_pvdev.h
+++ b/include/hw/xen/xen_pvdev.h
@@ -1,7 +1,7 @@
  #ifndef QEMU_HW_XEN_PVDEV_H
  #define QEMU_HW_XEN_PVDEV_H

-#include "hw/qdev-core.h"
+#include "hw/sysbus.h"
  #include "hw/xen/xen_backend_ops.h"

  /* ------------------------------------------------------------- */
---

?

Re: [RFC PATCH 9/9] hw/xen: Have legacy Xen backend inherit from DYNAMIC_SYS_BUS_DEVICE

[Bernhard Beschow]

Am 4. Februar 2025 21:25:46 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>Hi Bernhard,
>
>On 27/1/25 10:46, Bernhard Beschow wrote:
>> Am 25. Januar 2025 18:13:43 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>>> Because the legacy Xen backend devices can optionally be plugged on the
>>> TYPE_PLATFORM_BUS_DEVICE, have it inherit TYPE_DYNAMIC_SYS_BUS_DEVICE.
>>> Remove the implicit TYPE_XENSYSDEV instance_size.
>>> 
>>> Untested, but I'm surprised the legacy devices work because they
>>> had a broken instance size (QDev instead of Sysbus...), so accesses
>>> of XenLegacyDevice fields were overwritting sysbus ones.
>>> 
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>> include/hw/xen/xen_pvdev.h  | 3 ++-
>>> hw/xen/xen-legacy-backend.c | 7 ++-----
>>> 2 files changed, 4 insertions(+), 6 deletions(-)
>>> 
>>> diff --git a/include/hw/xen/xen_pvdev.h b/include/hw/xen/xen_pvdev.h
>>> index 0c984440476..48950dc2b57 100644
>>> --- a/include/hw/xen/xen_pvdev.h
>>> +++ b/include/hw/xen/xen_pvdev.h
>>> @@ -32,7 +32,8 @@ struct XenDevOps {
>>> };
>>> 
>>> struct XenLegacyDevice {
>>> -    DeviceState        qdev;
>>> +    SysBusDevice parent_obj;
>> 
>> This then needs sysbus.h rather than qdev-core.h include.
>> 
>> Moreover, the patch in the reply needs to be inserted into the series before this patch.
>> 
>> Both are needed for the patch to compile.
>
>Per your reply on patch #7, might I include your
>
>Tested-by: Bernhard Beschow <shentey@gmail.com>
>Acked-by: Bernhard Beschow <shentey@gmail.com>
>(or R-b)

I only did a compile test and I'm not a Xen maintainer, so I guess above tags don't apply. Right?


>
>squashing:
>
>-- >8 --
>diff --git a/include/hw/xen/xen_pvdev.h b/include/hw/xen/xen_pvdev.h
>index 48950dc2b57..629bec90d09 100644
>--- a/include/hw/xen/xen_pvdev.h
>+++ b/include/hw/xen/xen_pvdev.h
>@@ -1,7 +1,7 @@
> #ifndef QEMU_HW_XEN_PVDEV_H
> #define QEMU_HW_XEN_PVDEV_H
>
>-#include "hw/qdev-core.h"
>+#include "hw/sysbus.h"
> #include "hw/xen/xen_backend_ops.h"
>
> /* ------------------------------------------------------------- */
>---
>
>?

With the squash applied:
Reviewed-by: Bernhard Beschow <shentey@gmail.com>

Re: [RFC PATCH 9/9] hw/xen: Have legacy Xen backend inherit from DYNAMIC_SYS_BUS_DEVICE

[Philippe Mathieu-Daudé]

On 5/2/25 00:12, Bernhard Beschow wrote:
> 
> 
> Am 4. Februar 2025 21:25:46 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>> Hi Bernhard,
>>
>> On 27/1/25 10:46, Bernhard Beschow wrote:
>>> Am 25. Januar 2025 18:13:43 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>>>> Because the legacy Xen backend devices can optionally be plugged on the
>>>> TYPE_PLATFORM_BUS_DEVICE, have it inherit TYPE_DYNAMIC_SYS_BUS_DEVICE.
>>>> Remove the implicit TYPE_XENSYSDEV instance_size.
>>>>
>>>> Untested, but I'm surprised the legacy devices work because they
>>>> had a broken instance size (QDev instead of Sysbus...), so accesses
>>>> of XenLegacyDevice fields were overwritting sysbus ones.
>>>>
>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>> ---
>>>> include/hw/xen/xen_pvdev.h  | 3 ++-
>>>> hw/xen/xen-legacy-backend.c | 7 ++-----
>>>> 2 files changed, 4 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/include/hw/xen/xen_pvdev.h b/include/hw/xen/xen_pvdev.h
>>>> index 0c984440476..48950dc2b57 100644
>>>> --- a/include/hw/xen/xen_pvdev.h
>>>> +++ b/include/hw/xen/xen_pvdev.h
>>>> @@ -32,7 +32,8 @@ struct XenDevOps {
>>>> };
>>>>
>>>> struct XenLegacyDevice {
>>>> -    DeviceState        qdev;
>>>> +    SysBusDevice parent_obj;
>>>
>>> This then needs sysbus.h rather than qdev-core.h include.
>>>
>>> Moreover, the patch in the reply needs to be inserted into the series before this patch.
>>>
>>> Both are needed for the patch to compile.
>>
>> Per your reply on patch #7, might I include your
>>
>> Tested-by: Bernhard Beschow <shentey@gmail.com>
>> Acked-by: Bernhard Beschow <shentey@gmail.com>
>> (or R-b)
> 
> I only did a compile test and I'm not a Xen maintainer, so I guess above tags don't apply. Right?

Indeed, A-b is preferable for maintainers, but its meaning depends.

Xen maintainers have been Cc'ed for 2 weeks. If they report a problem,
we can still revert.


> 
> 
>>
>> squashing:
>>
>> -- >8 --
>> diff --git a/include/hw/xen/xen_pvdev.h b/include/hw/xen/xen_pvdev.h
>> index 48950dc2b57..629bec90d09 100644
>> --- a/include/hw/xen/xen_pvdev.h
>> +++ b/include/hw/xen/xen_pvdev.h
>> @@ -1,7 +1,7 @@
>> #ifndef QEMU_HW_XEN_PVDEV_H
>> #define QEMU_HW_XEN_PVDEV_H
>>
>> -#include "hw/qdev-core.h"
>> +#include "hw/sysbus.h"
>> #include "hw/xen/xen_backend_ops.h"
>>
>> /* ------------------------------------------------------------- */
>> ---
>>
>> ?
> 
> With the squash applied:
> Reviewed-by: Bernhard Beschow <shentey@gmail.com>

Thanks!

Re: [PATCH 0/9] hw/sysbus/platform-bus: Introduce TYPE_DYNAMIC_SYS_BUS_DEVICE

[Alexander Graf]

On 25.01.25 10:13, Philippe Mathieu-Daudé wrote:
> Some SysBus devices can optionally be dynamically plugged onto
> the sysbus-platform-bus (then virtual guests are aware of
> mmio mapping and IRQs via device tree / ACPI rules).
>
> This series makes these devices explicit by having them implement
> the DYNAMIC_SYS_BUS_DEVICE class, which only sets 'user_creatable'
> flag to %true but makes the codebase a bit clearer (IMHO, at least
> now we can grep for DYNAMIC_SYS_BUS_DEVICE).


I love it. Thank you! This is clearly much more readable than the 
magical hint swizzling I did :).

Reviewed-by: Alexander Graf <graf@amazon.com>


Alex

Re: [PATCH 0/9] hw/sysbus/platform-bus: Introduce TYPE_DYNAMIC_SYS_BUS_DEVICE

[Gerd Hoffmann]

On Sat, Jan 25, 2025 at 07:13:34PM +0100, Philippe Mathieu-Daudé wrote:
> Some SysBus devices can optionally be dynamically plugged onto
> the sysbus-platform-bus (then virtual guests are aware of
> mmio mapping and IRQs via device tree / ACPI rules).

Do we have some sane way to have user-pluggable sysbus devices on arm?

I've played around with that a bit, with the uefi variable service I'm
working on.  Specifically I'd prefer to *not* have a patch wiring things
up in machine type code like this ...

https://lore.kernel.org/qemu-devel/20250107153353.1144978-20-kraxel@redhat.com/

... and just use 'qemu -device uefi-vars-sysbus' instead.

Something like AcpiDevAmlIfClass but for device tree seems to not exist
though.  Also apparently AcpiDevAmlIfClass is not used.

take care,
  Gerd

Re: [PATCH 0/9] hw/sysbus/platform-bus: Introduce TYPE_DYNAMIC_SYS_BUS_DEVICE

[Peter Maydell]

On Tue, 28 Jan 2025 at 10:42, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> On Sat, Jan 25, 2025 at 07:13:34PM +0100, Philippe Mathieu-Daudé wrote:
> > Some SysBus devices can optionally be dynamically plugged onto
> > the sysbus-platform-bus (then virtual guests are aware of
> > mmio mapping and IRQs via device tree / ACPI rules).
>
> Do we have some sane way to have user-pluggable sysbus devices on arm?

The answer in a general sense is "no, because user pluggable
sysbus is a weird idea". "sysbus" means "it's wired into a
specific bit of the memory map and to specific IRQs, and whoever
does that needs to know what IRQs and bits of memory are usable,
and the guest OS needs to know it's there". "user-pluggable" means
"it's all automatic and the guest can just do some kind of
probing for what is or isn't present". All the platform bus stuff
is a nasty mess that's working around the things people want
to plug in not being clean devices on probeable buses :-(
And the platform bus is only supported on the "virt" board,
because that's the only one where QEMU is generating its
own dtb or ACPI tables where it can tell the guest "hey,
there's some device here".

-- PMM

Re: [PATCH 0/9] hw/sysbus/platform-bus: Introduce TYPE_DYNAMIC_SYS_BUS_DEVICE

[BALATON Zoltan]

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

On Tue, 28 Jan 2025, Peter Maydell wrote:
> On Tue, 28 Jan 2025 at 10:42, Gerd Hoffmann <kraxel@redhat.com> wrote:
>>
>> On Sat, Jan 25, 2025 at 07:13:34PM +0100, Philippe Mathieu-Daudé wrote:
>>> Some SysBus devices can optionally be dynamically plugged onto
>>> the sysbus-platform-bus (then virtual guests are aware of
>>> mmio mapping and IRQs via device tree / ACPI rules).
>>
>> Do we have some sane way to have user-pluggable sysbus devices on arm?
>
> The answer in a general sense is "no, because user pluggable
> sysbus is a weird idea". "sysbus" means "it's wired into a
> specific bit of the memory map and to specific IRQs, and whoever
> does that needs to know what IRQs and bits of memory are usable,
> and the guest OS needs to know it's there". "user-pluggable" means
> "it's all automatic and the guest can just do some kind of
> probing for what is or isn't present". All the platform bus stuff
> is a nasty mess that's working around the things people want
> to plug in not being clean devices on probeable buses :-(
> And the platform bus is only supported on the "virt" board,
> because that's the only one where QEMU is generating its
> own dtb or ACPI tables where it can tell the guest "hey,
> there's some device here".

There are some SoCs that have memory mapped devices but different versions 
in the same family have different devices. Either older ones missing some 
devices or have less USB or network ports while newer SoCs have more of 
those or they have PCIe instead of PCI. Modelling these could use 
pluggable sysbus devices so one could add the devices needed for a SoC 
version without having to write or modify a board code. I think Bernhard's 
attempt to try creating e500 SoCs from a device tree goes in that 
direction too. We could also model this by having a SoC that can 
instantiate devices based on some properties but maybe pluggable devices 
could be more generic for this. The issue seems to be how to tell the 
board or SoC where to map it and what IRQ to connect it as this is done by 
the board and not the device so properties on the device to set these does 
not really help unless the board can somehow query it and instantiate the 
devices based on that. Otherwise whatever handles the -device option to 
create the device would need knowledge about the board. (E.g. the e500 
devices are mapped in the CCSR memory region so one can't just use system 
address space for them.)

Regards,
BALATON Zoltan

Re: [PATCH 0/9] hw/sysbus/platform-bus: Introduce TYPE_DYNAMIC_SYS_BUS_DEVICE

[Philippe Mathieu-Daudé]

On 28/1/25 13:57, BALATON Zoltan wrote:
> On Tue, 28 Jan 2025, Peter Maydell wrote:
>> On Tue, 28 Jan 2025 at 10:42, Gerd Hoffmann <kraxel@redhat.com> wrote:
>>>
>>> On Sat, Jan 25, 2025 at 07:13:34PM +0100, Philippe Mathieu-Daudé wrote:
>>>> Some SysBus devices can optionally be dynamically plugged onto
>>>> the sysbus-platform-bus (then virtual guests are aware of
>>>> mmio mapping and IRQs via device tree / ACPI rules).
>>>
>>> Do we have some sane way to have user-pluggable sysbus devices on arm?
>>
>> The answer in a general sense is "no, because user pluggable
>> sysbus is a weird idea". "sysbus" means "it's wired into a
>> specific bit of the memory map and to specific IRQs, and whoever
>> does that needs to know what IRQs and bits of memory are usable,
>> and the guest OS needs to know it's there". "user-pluggable" means
>> "it's all automatic and the guest can just do some kind of
>> probing for what is or isn't present". All the platform bus stuff
>> is a nasty mess that's working around the things people want
>> to plug in not being clean devices on probeable buses :-(
>> And the platform bus is only supported on the "virt" board,
>> because that's the only one where QEMU is generating its
>> own dtb or ACPI tables where it can tell the guest "hey,
>> there's some device here".
> 
> There are some SoCs that have memory mapped devices but different 
> versions in the same family have different devices. Either older ones 
> missing some devices or have less USB or network ports while newer SoCs 
> have more of those or they have PCIe instead of PCI. Modelling these 
> could use pluggable sysbus devices so one could add the devices needed 
> for a SoC version without having to write or modify a board code. I 
> think Bernhard's attempt to try creating e500 SoCs from a device tree 
> goes in that direction too. We could also model this by having a SoC 
> that can instantiate devices based on some properties but maybe 
> pluggable devices could be more generic for this. The issue seems to be 
> how to tell the board or SoC where to map it and what IRQ to connect it 
> as this is done by the board and not the device so properties on the 
> device to set these does not really help unless the board can somehow 
> query it and instantiate the devices based on that. Otherwise whatever 
> handles the -device option to create the device would need knowledge 
> about the board. (E.g. the e500 devices are mapped in the CCSR memory 
> region so one can't just use system address space for them.)

IIRC Bernard's series takes a DTB as input and create the machine
matching this DTB.

As Peter explained, sysbus-platform-bus fits TYPE_DYNAMIC_SYS_BUS_DEVICE
in free slots, then generates the corresponding ACPI/DTB.

What you describe seems closer to the QEMU Dynamic Machine project,
following Damien's idea:
https://lore.kernel.org/qemu-devel/20220223090706.4888-1-damien.hedde@greensocs.com/
We are not quite there yet...

Re: [PATCH 0/9] hw/sysbus/platform-bus: Introduce TYPE_DYNAMIC_SYS_BUS_DEVICE

[Bernhard Beschow]

Am 28. Januar 2025 15:10:18 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>On 28/1/25 13:57, BALATON Zoltan wrote:
>> On Tue, 28 Jan 2025, Peter Maydell wrote:
>>> On Tue, 28 Jan 2025 at 10:42, Gerd Hoffmann <kraxel@redhat.com> wrote:
>>>> 
>>>> On Sat, Jan 25, 2025 at 07:13:34PM +0100, Philippe Mathieu-Daudé wrote:
>>>>> Some SysBus devices can optionally be dynamically plugged onto
>>>>> the sysbus-platform-bus (then virtual guests are aware of
>>>>> mmio mapping and IRQs via device tree / ACPI rules).
>>>> 
>>>> Do we have some sane way to have user-pluggable sysbus devices on arm?
>>> 
>>> The answer in a general sense is "no, because user pluggable
>>> sysbus is a weird idea". "sysbus" means "it's wired into a
>>> specific bit of the memory map and to specific IRQs, and whoever
>>> does that needs to know what IRQs and bits of memory are usable,
>>> and the guest OS needs to know it's there". "user-pluggable" means
>>> "it's all automatic and the guest can just do some kind of
>>> probing for what is or isn't present". All the platform bus stuff
>>> is a nasty mess that's working around the things people want
>>> to plug in not being clean devices on probeable buses :-(
>>> And the platform bus is only supported on the "virt" board,
>>> because that's the only one where QEMU is generating its
>>> own dtb or ACPI tables where it can tell the guest "hey,
>>> there's some device here".
>> 
>> There are some SoCs that have memory mapped devices but different versions in the same family have different devices. Either older ones missing some devices or have less USB or network ports while newer SoCs have more of those or they have PCIe instead of PCI. Modelling these could use pluggable sysbus devices so one could add the devices needed for a SoC version without having to write or modify a board code. I think Bernhard's attempt to try creating e500 SoCs from a device tree goes in that direction too. We could also model this by having a SoC that can instantiate devices based on some properties but maybe pluggable devices could be more generic for this. The issue seems to be how to tell the board or SoC where to map it and what IRQ to connect it as this is done by the board and not the device so properties on the device to set these does not really help unless the board can somehow query it and instantiate the devices based on that. Otherwise whatever handles the -device option to create the device would need knowledge about the board. (E.g. the e500 devices are mapped in the CCSR memory region so one can't just use system address space for them.)
>
>IIRC Bernard's series takes a DTB as input and create the machine
>matching this DTB.

That's correct. It's still on my todo list to send an RFC. I first wanted to gain some experience implementing a machine in the classic way which I've now done by means of the imx8mp-evk series. Once I clean up the e500-fdt branch I'd send an RFC.

Best regards,
Bernhard

>
>As Peter explained, sysbus-platform-bus fits TYPE_DYNAMIC_SYS_BUS_DEVICE
>in free slots, then generates the corresponding ACPI/DTB.
>
>What you describe seems closer to the QEMU Dynamic Machine project,
>following Damien's idea:
>https://lore.kernel.org/qemu-devel/20220223090706.4888-1-damien.hedde@greensocs.com/
>We are not quite there yet...

Re: [PATCH 0/9] hw/sysbus/platform-bus: Introduce TYPE_DYNAMIC_SYS_BUS_DEVICE

[Philippe Mathieu-Daudé]

On 25/1/25 19:13, Philippe Mathieu-Daudé wrote:

> Philippe Mathieu-Daudé (9):
>    hw/sysbus: Use sizeof(BusState) in main_system_bus_create()
>    hw/sysbus: Declare QOM types using DEFINE_TYPES() macro
>    hw/sysbus: Introduce TYPE_DYNAMIC_SYS_BUS_DEVICE
>    hw/vfio: Have VFIO_PLATFORM devices inherit from
>      DYNAMIC_SYS_BUS_DEVICE
>    hw/display: Have RAMFB device inherit from DYNAMIC_SYS_BUS_DEVICE
>    hw/i386: Have X86_IOMMU devices inherit from DYNAMIC_SYS_BUS_DEVICE
>    hw/net: Have eTSEC device inherit from DYNAMIC_SYS_BUS_DEVICE
>    hw/tpm: Have TPM TIS sysbus device inherit from DYNAMIC_SYS_BUS_DEVICE
>    hw/xen: Have legacy Xen backend inherit from DYNAMIC_SYS_BUS_DEVICE

Series queued (including Bernhard's patch), thanks.