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

[Ani Sinha]

On Thu, 19 Dec, 2024, 9:22 pm Philippe Mathieu-Daudé, <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>
> + *
> + * 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. 

> +    } else {
> +        return 0;
> +    }
> +}