| Message ID | 20250125181343.59151-1-philmd@linaro.org |
|---|---|
| Headers | show |
| Series | hw/sysbus/platform-bus: Introduce TYPE_DYNAMIC_SYS_BUS_DEVICE | expand |
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~
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~
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~
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~
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~
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~
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
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 >
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 >
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 >
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 >
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 >
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,
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(-)