Re: [PATCH RESEND] i386: Only configure HPET firmware info when HPET is enabled

["Michael S. Tsirkin" <mst@redhat.com>]

On Tue, Jan 21, 2025 at 10:01:21PM +0800, Zhao Liu wrote:
> At present, the hpet_cfg is written unconditionally since 40ac17cd56eb
> ("pass info about hpets to seabios.]"), because it concerns ACPI HPET is
> created unconditionally.
> 
> But that fact has changed since 51124bbfd2ea ("i386: acpi: Don't build
> HPET ACPI entry if HPET is disabled") and ACPI checks if HPET device
> exists in (hw/i386/acpi-build.c).
> 
> Therefore, configure HPET firmware information if and only if HPET is
> enabled.
>

and what is the gain from this change? just a cleanup?

> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
> Resend:
>  * Resend the patch since it was missed on https://lore.kernel.org/qemu-devel/.




> ---
>  hw/i386/fw_cfg.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/fw_cfg.c b/hw/i386/fw_cfg.c
> index 91bf1df0f2e4..d2cb08715a21 100644
> --- a/hw/i386/fw_cfg.c
> +++ b/hw/i386/fw_cfg.c
> @@ -149,7 +149,14 @@ FWCfgState *fw_cfg_arch_create(MachineState *ms,
>  #endif
>      fw_cfg_add_i32(fw_cfg, FW_CFG_IRQ0_OVERRIDE, 1);
>  
> -    fw_cfg_add_bytes(fw_cfg, FW_CFG_HPET, &hpet_cfg, sizeof(hpet_cfg));
> +#ifdef CONFIG_HPET
> +    PCMachineState *pcms =
> +        (PCMachineState *)object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE);
> +    if (pcms && pcms->hpet_enabled) {
> +        fw_cfg_add_bytes(fw_cfg, FW_CFG_HPET, &hpet_cfg, sizeof(hpet_cfg));
> +    }
> +#endif
> +


Hmm. Wouldn't this break cross version migration? I suspect we need
a compat tweak if we do this. Might not be worth it ...


>      /* allocate memory for the NUMA channel: one (64bit) word for the number
>       * of nodes, one word for each VCPU->node and one word for each node to
>       * hold the amount of memory.
> -- 
> 2.34.1