On Thu, 5 Mar 2026 13:33:21 +0100 Thomas Huth <[email protected]> wrote:
> On 05/03/2026 13.10, Igor Mammedov wrote: > > On Wed, 4 Mar 2026 18:20:27 +0100 > > Thomas Huth <[email protected]> wrote: > > > >> On 03/03/2026 15.00, Igor Mammedov wrote: > >>> From: Philippe Mathieu-Daudé <[email protected]> > >>> > >>> v2: > >>> - do not check for SMI features if hotplug happens when > >>> SMI is not enabled. (matters for qtest and possibly seabios) > >>> - removing property also removes default ICH9_LPC_SMI_F_BROADCAST_BIT > >>> put default back in place only set it initfn() instead > >>> > >>> The ICH9_LPC_SMI_F_BROADCAST_BIT feature bit was only set > >>> in the pc_compat_2_8[] array, via the 'x-smi-broadcast=off' > >>> property. We removed all machines using that array, lets remove > >>> that property and all the code around it. > >>> > >>> Signed-off-by: Philippe Mathieu-Daudé <[email protected]> > >>> Signed-off-by: Igor Mammedov <[email protected]> > >>> --- > >>> hw/acpi/ich9.c | 4 ++-- > >>> hw/isa/lpc_ich9.c | 23 ++++------------------- > >>> 2 files changed, 6 insertions(+), 21 deletions(-) > >>> > >>> diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c > >>> index bbb1bd60a2..87afe86bcc 100644 > >>> --- a/hw/acpi/ich9.c > >>> +++ b/hw/acpi/ich9.c > >>> @@ -432,7 +432,7 @@ void ich9_pm_device_pre_plug_cb(HotplugHandler > >>> *hotplug_dev, DeviceState *dev, > >>> if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) { > >>> uint64_t negotiated = lpc->smi_negotiated_features; > >>> > >>> - if (negotiated & BIT_ULL(ICH9_LPC_SMI_F_BROADCAST_BIT) && > >>> + if (lpc->pm.smi_en & ICH9_PMIO_SMI_EN_APMC_EN && > >>> !(negotiated & BIT_ULL(ICH9_LPC_SMI_F_CPU_HOTPLUG_BIT))) { > >>> error_setg(errp, "cpu hotplug with SMI wasn't enabled by > >>> firmware"); > >>> error_append_hint(errp, "update machine type to newer than > >>> 5.1 " > >>> @@ -476,7 +476,7 @@ void ich9_pm_device_unplug_request_cb(HotplugHandler > >>> *hotplug_dev, > >>> } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) { > >>> uint64_t negotiated = lpc->smi_negotiated_features; > >>> > >>> - if (negotiated & BIT_ULL(ICH9_LPC_SMI_F_BROADCAST_BIT) && > >>> + if (lpc->pm.smi_en & ICH9_PMIO_SMI_EN_APMC_EN && > >>> !(negotiated & > >>> BIT_ULL(ICH9_LPC_SMI_F_CPU_HOT_UNPLUG_BIT))) { > >>> error_setg(errp, "cpu hot-unplug with SMI wasn't enabled " > >>> "by firmware"); > >>> diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c > >>> index 51dc680029..04169ffa24 100644 > >>> --- a/hw/isa/lpc_ich9.c > >>> +++ b/hw/isa/lpc_ich9.c > >>> @@ -404,15 +404,6 @@ static void smi_features_ok_callback(void *opaque) > >>> guest_cpu_hotplug_features = guest_features & > >>> > >>> (BIT_ULL(ICH9_LPC_SMI_F_CPU_HOTPLUG_BIT) | > >>> > >>> BIT_ULL(ICH9_LPC_SMI_F_CPU_HOT_UNPLUG_BIT)); > >>> - if (!(guest_features & BIT_ULL(ICH9_LPC_SMI_F_BROADCAST_BIT)) && > >>> - guest_cpu_hotplug_features) { > >>> - /* > >>> - * cpu hot-[un]plug with SMI requires SMI broadcast, > >>> - * leave @features_ok at zero > >>> - */ > >>> - return; > >>> - } > >>> - > >>> if (guest_cpu_hotplug_features == > >>> BIT_ULL(ICH9_LPC_SMI_F_CPU_HOT_UNPLUG_BIT)) { > >>> /* cpu hot-unplug is unsupported without cpu-hotplug */ > >>> @@ -474,14 +465,9 @@ static void ich9_apm_ctrl_changed(uint32_t val, void > >>> *arg) > >>> > >>> /* SMI_EN = PMBASE + 30. SMI control and enable register */ > >>> if (lpc->pm.smi_en & ICH9_PMIO_SMI_EN_APMC_EN) { > >>> - if (lpc->smi_negotiated_features & > >>> - (UINT64_C(1) << ICH9_LPC_SMI_F_BROADCAST_BIT)) { > >>> - CPUState *cs; > >>> - CPU_FOREACH(cs) { > >>> - cpu_interrupt(cs, CPU_INTERRUPT_SMI); > >>> - } > >>> - } else { > >>> - cpu_interrupt(current_cpu, CPU_INTERRUPT_SMI); > >>> + CPUState *cs; > >>> + CPU_FOREACH(cs) { > >>> + cpu_interrupt(cs, CPU_INTERRUPT_SMI); > >>> } > >>> } > >>> } > >>> @@ -685,6 +671,7 @@ static void ich9_lpc_initfn(Object *obj) > >>> > >>> static const uint8_t acpi_enable_cmd = ICH9_APM_ACPI_ENABLE; > >>> static const uint8_t acpi_disable_cmd = ICH9_APM_ACPI_DISABLE; > >>> + lpc->smi_host_features = BIT_ULL(ICH9_LPC_SMI_F_BROADCAST_BIT); > >> > >> Shouldn't that rather be a "|=" instead of the "=" so that we don't lose > >> the > >> other bits? > > > > Indeed it should be '|=', > > can you fix it up on merge or should I better send fixed up version? > > I tried to fix it, but I still see an issue with the bios-tables-test qtest > during "make check" - it does not finish anymore and is finally killed by > the timeout. Does "make check-qtest" work for you? cpu hotplug test did work, let me fix and check tests again > > Thomas >
