| Message ID | 20200923094650.1301166-3-imammedo@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | x86: fix cpu hotplug with secure boot | expand |
On 09/23/20 11:46, Igor Mammedov wrote: > There were reports of guest crash on CPU hotplug, when using q35 machine > type and OVMF with SMM, due to hotplugged CPU trying to process SMI at > default SMI handler location without it being relocated by firmware first. > > Fix it by refusing hotplug if firmware hasn't negotiated CPU hotplug with > SMI support while SMI broadcast is in use. > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > Reviewed-by: Laszlo Ersek <lersek@redhat.com> > Tested-by: Laszlo Ersek <lersek@redhat.com> > --- > v6: > - rebase on top of current master, due to non trivial conflicts > caused by microvm series, which moved/renamed pc_cpu_pre_plug() > (Ankur Arora <ankur.a.arora@oracle.com>) Tested-by: Laszlo Ersek <lersek@redhat.com> Thanks Laszlo > v1: > fix typos an use suggested wording in commit and error msg > s/secure boot/smm/; s/hotplug SMI/hotplug with SMI/ > (Laszlo Ersek <lersek@redhat.com>) > --- > hw/acpi/ich9.c | 12 +++++++++++- > hw/i386/x86.c | 11 +++++++++++ > 2 files changed, 22 insertions(+), 1 deletion(-) > > diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c > index 6a19070cec..0acc9a3107 100644 > --- a/hw/acpi/ich9.c > +++ b/hw/acpi/ich9.c > @@ -408,10 +408,20 @@ void ich9_pm_device_pre_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, > ICH9LPCState *lpc = ICH9_LPC_DEVICE(hotplug_dev); > > if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) && > - !lpc->pm.acpi_memory_hotplug.is_enabled) > + !lpc->pm.acpi_memory_hotplug.is_enabled) { > error_setg(errp, > "memory hotplug is not enabled: %s.memory-hotplug-support " > "is not set", object_get_typename(OBJECT(lpc))); > + } 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) && > + !(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 " > + "and firmware that suppors CPU hotplug with SMM"); > + } > + } > } > > void ich9_pm_device_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, > diff --git a/hw/i386/x86.c b/hw/i386/x86.c > index c2ea989579..403c2b1dad 100644 > --- a/hw/i386/x86.c > +++ b/hw/i386/x86.c > @@ -279,6 +279,17 @@ void x86_cpu_pre_plug(HotplugHandler *hotplug_dev, > return; > } > > + if (x86ms->acpi_dev) { > + Error *local_err = NULL; > + > + hotplug_handler_pre_plug(HOTPLUG_HANDLER(x86ms->acpi_dev), dev, > + &local_err); > + if (local_err) { > + error_propagate(errp, local_err); > + return; > + } > + } > + > init_topo_info(&topo_info, x86ms); > > env->nr_dies = x86ms->smp_dies; >
diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c index 6a19070cec..0acc9a3107 100644 --- a/hw/acpi/ich9.c +++ b/hw/acpi/ich9.c @@ -408,10 +408,20 @@ void ich9_pm_device_pre_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, ICH9LPCState *lpc = ICH9_LPC_DEVICE(hotplug_dev); if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) && - !lpc->pm.acpi_memory_hotplug.is_enabled) + !lpc->pm.acpi_memory_hotplug.is_enabled) { error_setg(errp, "memory hotplug is not enabled: %s.memory-hotplug-support " "is not set", object_get_typename(OBJECT(lpc))); + } 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) && + !(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 " + "and firmware that suppors CPU hotplug with SMM"); + } + } } void ich9_pm_device_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, diff --git a/hw/i386/x86.c b/hw/i386/x86.c index c2ea989579..403c2b1dad 100644 --- a/hw/i386/x86.c +++ b/hw/i386/x86.c @@ -279,6 +279,17 @@ void x86_cpu_pre_plug(HotplugHandler *hotplug_dev, return; } + if (x86ms->acpi_dev) { + Error *local_err = NULL; + + hotplug_handler_pre_plug(HOTPLUG_HANDLER(x86ms->acpi_dev), dev, + &local_err); + if (local_err) { + error_propagate(errp, local_err); + return; + } + } + init_topo_info(&topo_info, x86ms); env->nr_dies = x86ms->smp_dies;