Message ID | 20200818122208.1243901-4-imammedo@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | x86: fix cpu hotplug with secure boot | expand |
On 08/18/20 14:22, Igor Mammedov wrote: > CPU hot-unplug with SMM requires firmware participation to prevent > guest crash (i.e. CPU can be removed only after OS _and_ firmware > were prepared for the action). > Previous patches introduced ICH9_LPC_SMI_F_CPU_HOT_UNPLUG_BIT > feature bit, which is advertised by firmware when it has support > for CPU hot-unplug. Use it to check if guest is able to handle > unplug and make device_del fail gracefully if hot-unplug feature > hasn't been negotiated. > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > Tested-by: Laszlo Ersek <lersek@redhat.com> > Reviewed-by: Laszlo Ersek <lersek@redhat.com> > --- > v2: > - fix typo in commit message > - drop 5.1 version from hint message (Laszlo) > --- > hw/acpi/ich9.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c > index 0acc9a3107..95cb0f935b 100644 > --- a/hw/acpi/ich9.c > +++ b/hw/acpi/ich9.c > @@ -460,6 +460,18 @@ void ich9_pm_device_unplug_request_cb(HotplugHandler *hotplug_dev, > errp); > } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU) && > !lpc->pm.cpu_hotplug_legacy) { > + 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_HOT_UNPLUG_BIT))) { > + error_setg(errp, "cpu hot-unplug with SMI wasn't enabled " > + "by firmware"); > + error_append_hint(errp, "update machine type to a version having " > + "x-smi-cpu-hotunplug=on and firmware that " > + "supports CPU hot-unplug with SMM"); > + return; > + } > + > acpi_cpu_unplug_request_cb(hotplug_dev, &lpc->pm.cpuhp_state, > dev, errp); > } else { > A trivial comment: Patch#2 says "x86: cphp: " in the subject line, but patch#3 says "x86: cpuhp: " (note the extra "u"). I'm fine with either "cphp" or "cpuhp", but the subjects should be consistent -- both patches should use the same word. Preserve my T-b and R-b on both patches #2 and #3, after fixing up one of the subjects. (Up to you which one.) Thanks! Laszlo
diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c index 0acc9a3107..95cb0f935b 100644 --- a/hw/acpi/ich9.c +++ b/hw/acpi/ich9.c @@ -460,6 +460,18 @@ void ich9_pm_device_unplug_request_cb(HotplugHandler *hotplug_dev, errp); } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU) && !lpc->pm.cpu_hotplug_legacy) { + 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_HOT_UNPLUG_BIT))) { + error_setg(errp, "cpu hot-unplug with SMI wasn't enabled " + "by firmware"); + error_append_hint(errp, "update machine type to a version having " + "x-smi-cpu-hotunplug=on and firmware that " + "supports CPU hot-unplug with SMM"); + return; + } + acpi_cpu_unplug_request_cb(hotplug_dev, &lpc->pm.cpuhp_state, dev, errp); } else {