| Message ID | 20250125181343.59151-10-philmd@linaro.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | hw/sysbus/platform-bus: Introduce TYPE_DYNAMIC_SYS_BUS_DEVICE | expand |
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)
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" /* ------------------------------------------------------------- */ --- ?
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>
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)
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(-)