Re: [PATCH v2] hw/misc/vmfwupdate: Introduce hypervisor fw-cfg interface support

[Ani Sinha]

On Thu, Dec 19, 2024 at 6:25 PM Marc-André Lureau
<marcandre.lureau@redhat.com> wrote:
>
> Hi
>
> On Thu, Dec 19, 2024 at 2:03 PM Philippe Mathieu-Daudé
> <philmd@linaro.org> wrote:
> > >>> +static const TypeInfo vmfwupdate_device_info = {
> > >>> +    .name          = TYPE_VMFWUPDATE,
> > >>> +    .parent        = TYPE_DEVICE,
> > >>
> > >> What is the qdev API used here? Why not use a plain object?
> > >
> > > I wrote this taking vmcoreinfo device as starting point. I will leave 
> > > this as is for now unless anyone has strong opinions.
> >
> > We shouldn't blindly copy/paste & spread possible design mistakes.
> >
> > Marc-André, any particular reason to implement vmcoreinfo using qdev
> > and not plain object?
> >
>
> I don't remember (damn 8y ago..). It seems the design changed over
> time during review, qdev might have been necessary and stayed this
> way.

I changed it to TYPE_OBJECT and we get a crash here:

#3  0x0000aaaaab207a48 [PAC] in object_class_dynamic_cast_assert
    (class=0xaaaaac608880, typename=typename@entry=0xaaaaab4b9630
"device", file=file@entry=0xaaaaab4300d0
"/workspace/qemu-ani/include/hw/qdev-core.h", line=line@entry=77,
func=func@entry=0xaaaaab595a90 <__func__.0> "DEVICE_CLASS") at
../qom/object.c:1021
#4  0x0000aaaaaaec2d74 in DEVICE_CLASS (klass=<optimized out>) at
/workspace/qemu-ani/include/hw/qdev-core.h:77
#5  vmcoreinfo_device_class_init (klass=<optimized out>,
data=<optimized out>) at ../hw/misc/vmcoreinfo.c:88

Basically doing this would be illegal for vmcoreinfo and we need to
adjust the code :

   DeviceClass *dc = DEVICE_CLASS(klass);

    dc->vmsd = &vmstate_vmcoreinfo;
    dc->realize = vmcoreinfo_realize;
    dc->hotpluggable = false;
    set_bit(DEVICE_CATEGORY_MISC, dc->categories);

Anyway, for vmfwupdate, it is actually like a device with device properties:

+    device_class_set_props(dc, vmfwupdate_properties);

So I prefer to make it qdev type for now.