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

[Alexander Graf]

On 20.12.24 11:00, Ani Sinha wrote:
> > > Either add the I386 dependency or don't use PC_MACHINE, because on
> > > non-x86 targets PC_MACHINE(qdev_get_machine()) will crash.
> > Ah this is where we have a disconnect. I assumed that
> > > pcms = PC_MACHINE(m_obj)
> > would return NULL on non-x86.
> >
> > Seems a better way to do this (as is done in vga.c) is to use
> > object_dynamic_cast()
> > How about
> >
> > diff --git a/hw/misc/vmfwupdate.c b/hw/misc/vmfwupdate.c
> > index 0e90bd10e1..19d042b929 100644
> > --- a/hw/misc/vmfwupdate.c
> > +++ b/hw/misc/vmfwupdate.c
> > @@ -32,9 +32,11 @@ static inline VMFwUpdateState *vmfwupdate_find(void)
> >   static uint64_t get_max_fw_size(void)
> >   {
> >       Object *m_obj = qdev_get_machine();
> > -    PCMachineState *pcms = PC_MACHINE(m_obj);
> > +    MachineState *ms = MACHINE(m_obj);
> > +    PCMachineState *pcms;
> > -    if (pcms) {
> > +    if (object_dynamic_cast(OBJECT(ms), TYPE_X86_MACHINE)) {
> > +        pcms = PC_MACHINE(m_obj);
> >           return pcms->max_fw_size;
> >       } else {
> >           return 0;
> For the records, I tested this with arm and the following command line
> does not crash QEMU;
>
> ./qemu-system-arm -machine virt-9.2 -device vmfwupdate
>
> I have also added a separate functional test to exercise basic device
> creation which will be part of v5 when I send it out.


You are currently not implementing the reset logic required to actually 
make vmfwupdate work. That means technically, the device should not be 
instantiable on *any* platform at the moment. For example with the 
command line above you would advertise the capability to update firmware 
in fw-cfg, but then not back it by functionality. If QEMU were to merge 
this patch, it would just create a broken device.

Please make sure that the vmfwupdate device can only be instantiated on 
machine types that it has full support for.


Alex




Amazon Web Services Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 257764 B
Sitz: Berlin
Ust-ID: DE 365 538 597