On 19/12/24 17:16, Ani Sinha wrote:
>
>
> On Thu, 19 Dec, 2024, 9:22 pm Philippe Mathieu-Daudé, <philmd@linaro.org
> <mailto:philmd@linaro.org>> wrote:
>
> On 16/12/24 12:48, Ani Sinha wrote:
>
> > diff --git a/hw/misc/meson.build b/hw/misc/meson.build
> > index d02d96e403..4c5bdb0de2 100644
> > --- a/hw/misc/meson.build
> > +++ b/hw/misc/meson.build
> > @@ -148,6 +148,8 @@ specific_ss.add(when: 'CONFIG_MAC_VIA',
> if_true: files('mac_via.c'))
> > specific_ss.add(when: 'CONFIG_MIPS_CPS', if_true:
> files('mips_cmgcr.c', 'mips_cpc.c'))
> > specific_ss.add(when: 'CONFIG_MIPS_ITU', if_true:
> files('mips_itu.c'))
> >
> > +specific_ss.add(when: 'CONFIG_FW_CFG_DMA', if_true:
> files('vmfwupdate.c'))
>
> FW_CFG_DMA is offered by multiple targets ...:
>
> $ git grep -w FW_CFG_DMA
> hw/arm/Kconfig:19: select FW_CFG_DMA
> hw/i386/Kconfig:82: select FW_CFG_DMA
> hw/i386/Kconfig:113: select FW_CFG_DMA
> hw/loongarch/Kconfig:22: select FW_CFG_DMA
> hw/riscv/Kconfig:59: select FW_CFG_DMA
>
> > diff --git a/hw/misc/vmfwupdate.c b/hw/misc/vmfwupdate.c
> > new file mode 100644
> > index 0000000000..1e29a610c0
> > --- /dev/null
> > +++ b/hw/misc/vmfwupdate.c
> > @@ -0,0 +1,157 @@
> > +/*
> > + * Guest driven VM boot component update device
> > + * For details and specification, please look at docs/specs/
> vmfwupdate.rst.
> > + *
> > + * Copyright (C) 2024 Red Hat, Inc.
> > + *
> > + * Authors: Ani Sinha <anisinha@redhat.com
> <mailto:anisinha@redhat.com>>
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version
> 2 or later.
> > + * See the COPYING file in the top-level directory.
> > + *
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "qapi/error.h"
> > +#include "qemu/module.h"
> > +#include "sysemu/reset.h"
> > +#include "hw/nvram/fw_cfg.h"
> > +#include "hw/i386/pc.h"
>
> ... however ...
>
> > +#include "hw/qdev-properties.h"
> > +#include "hw/misc/vmfwupdate.h"
> > +#include "qemu/error-report.h"
> > +
> > +static void fw_update_reset(void *dev)
> > +{
> > + /* a NOOP at present */
> > + return;
> > +}
> > +
> > +
> > +static uint64_t get_max_fw_size(void)
> > +{
> > + Object *m_obj = qdev_get_machine();
> > + PCMachineState *pcms = PC_MACHINE(m_obj);
> > +
> > + if (pcms) {
> > + return pcms->max_fw_size;
>
> ... this code depends on x86/PC.
>
> Could it be wiser to add a new VM_FWUPDATE Kconfig
> symbol, having it depending on FW_CFG_DMA && I386?
>
>
> There is no reason why vmfwupdate would be limited to x86 only. There is
> minimal support needed from hypervisor side for this mechanism. That
> mechanism has little dependency on specific platform.
OK, then please remove that PCMachineState use
That is used because the max_fw_size property only exists for pc machines. Do you have a better idea to get this value in a platform agnostic way? Like I said in the previous reply I do not know how to get this value reliably for all machines. If anyone has better ideas, I'm all ears.
.
What about the FW_CFG_DMA dependency?