On 07/20/20 16:16, 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>
> ---
> 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/pc.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/pc.c b/hw/i386/pc.c
> index 57d50fad6b..3f4b7e3d9a 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1497,6 +1497,17 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
> return;
> }
>
> + if (pcms->acpi_dev) {
> + Error *local_err = NULL;
> +
> + hotplug_handler_pre_plug(HOTPLUG_HANDLER(pcms->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;
>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>