On Thu, 5 Mar 2026 15:32:18 +0100 Igor Mammedov <[email protected]> wrote:
> 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 what breaks is '-M q35 -smp >1 + seabios", where the later asks for SMI => unexpected broadcast with this patch and as result unhappy SeaBIOS, I'll send v3 as reply here. > > > > > Thomas > > >
