| Message ID | 20250121140121.84550-1-zhao1.liu@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RESEND] i386: Only configure HPET firmware info when HPET is enabled | expand |
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
Hi Michael, Thanks for looking at here! On Thu, Feb 20, 2025 at 10:16:56AM -0500, Michael S. Tsirkin wrote: > Date: Thu, 20 Feb 2025 10:16:56 -0500 > From: "Michael S. Tsirkin" <mst@redhat.com> > Subject: Re: [PATCH RESEND] i386: Only configure HPET firmware info when > HPET is enabled > > 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? This patch was trying to decouple hpet_cfw with i386 codes, which can resolve the compilation issue when both the Rust and C versions of HPET are both disabled. But this patch is not needed since Paolo has figured out a simpler way [*]. And thank you anyway! [*]: https://lore.kernel.org/qemu-devel/CABgObfb6PhiKO9=iWne9AoXQ+Ek7FddoW8D0VcWvw3Qa3TW-9w@mail.gmail.com/ Regards, Zhao
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 + /* 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.
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. 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(-)