From: Laszlo Ersek <lersek@redhat.com>
To: Igor Mammedov <imammedo@redhat.com>, qemu-devel@nongnu.org
Cc: boris.ostrovsky@oracle.com
Subject: Re: [PATCH 1/6] x86: lpc9: let firmware negotiate 'CPU hotplug with SMI' features
Date: Wed, 22 Jul 2020 15:26:47 +0200 [thread overview]
Message-ID: <2d97286a-2e8e-dd6b-822c-58959d0a74e2@redhat.com> (raw)
In-Reply-To: <00b7e32c-36ad-a71a-00da-d1f0d9977e79@redhat.com>
On 07/22/20 14:03, Laszlo Ersek wrote:
> On 07/20/20 16:16, Igor Mammedov wrote:
>> It will allow firmware to notify QEMU that firmware requires SMI
>> being triggered on CPU hot[un]plug, so that it would be able to account
>> for hotplugged CPU and relocate it to new SMM base and/or safely remove
>> CPU on unplug.
>>
>> Using negotiated features, follow up patches will insert SMI upcall
>> into AML code, to make sure that firmware processes hotplug before
>> guest OS would attempt to use new CPU.
>>
>> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
>> ---
>> include/hw/i386/ich9.h | 2 ++
>> hw/i386/pc.c | 5 ++++-
>> hw/isa/lpc_ich9.c | 12 ++++++++++++
>> 3 files changed, 18 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h
>> index a98d10b252..d1bb3f7bf0 100644
>> --- a/include/hw/i386/ich9.h
>> +++ b/include/hw/i386/ich9.h
>> @@ -247,5 +247,7 @@ typedef struct ICH9LPCState {
>>
>> /* bit positions used in fw_cfg SMI feature negotiation */
>> #define ICH9_LPC_SMI_F_BROADCAST_BIT 0
>> +#define ICH9_LPC_SMI_F_CPU_HOTPLUG_BIT 1
>> +#define ICH9_LPC_SMI_F_CPU_HOT_UNPLUG_BIT 2
>>
>> #endif /* HW_ICH9_H */
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index 3d419d5991..57d50fad6b 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -97,7 +97,10 @@
>> #include "fw_cfg.h"
>> #include "trace.h"
>>
>> -GlobalProperty pc_compat_5_0[] = {};
>> +GlobalProperty pc_compat_5_0[] = {
>> + { "ICH9-LPC", "x-smi-cpu-hotplug", "off" },
>> + { "ICH9-LPC", "x-smi-cpu-hotunplug", "off" },
>> +};
>> const size_t pc_compat_5_0_len = G_N_ELEMENTS(pc_compat_5_0);
>>
>> GlobalProperty pc_compat_4_2[] = {
>> diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
>> index cd6e169d47..c9305080b5 100644
>> --- a/hw/isa/lpc_ich9.c
>> +++ b/hw/isa/lpc_ich9.c
>> @@ -373,6 +373,14 @@ static void smi_features_ok_callback(void *opaque)
>> /* guest requests invalid features, leave @features_ok at zero */
>> return;
>> }
>> + if (!(guest_features & BIT_ULL(ICH9_LPC_SMI_F_BROADCAST_BIT)) &&
>> + guest_features & (BIT_ULL(ICH9_LPC_SMI_F_CPU_HOTPLUG_BIT) |
>> + BIT_ULL(ICH9_LPC_SMI_F_CPU_HOT_UNPLUG_BIT))) {
>> + /* cpu hot-[un]plug with SMI requires SMI broadcast,
>> + * leave @features_ok at zero
>> + */
>> + return;
>> + }
>>
>> /* valid feature subset requested, lock it down, report success */
>> lpc->smi_negotiated_features = guest_features;
>> @@ -747,6 +755,10 @@ static Property ich9_lpc_properties[] = {
>> DEFINE_PROP_BOOL("noreboot", ICH9LPCState, pin_strap.spkr_hi, true),
>> DEFINE_PROP_BIT64("x-smi-broadcast", ICH9LPCState, smi_host_features,
>> ICH9_LPC_SMI_F_BROADCAST_BIT, true),
>> + DEFINE_PROP_BIT64("x-smi-cpu-hotplug", ICH9LPCState, smi_host_features,
>> + ICH9_LPC_SMI_F_CPU_HOTPLUG_BIT, true),
>> + DEFINE_PROP_BIT64("x-smi-cpu-hotunplug", ICH9LPCState, smi_host_features,
>> + ICH9_LPC_SMI_F_CPU_HOT_UNPLUG_BIT, true),
>> DEFINE_PROP_END_OF_LIST(),
>> };
>>
>>
>
> (1) I think that, at this time, the default value for
> "x-smi-cpu-hotunplug" should be false. Accordingly, the
> "x-smi-cpu-hotunplug" compat property should be dropped.
>
> The reason is that in this series, QEMU won't actually learn to handle
> CPU hot-unplug with SMI. We shouldn't advertize support for it.
>
> We should only recognize the feature, so that the QMP command can rely
> on it for rejecting hot-unplug attempts. If (later) we have a more
> advanced OVMF binary with unplug support (so that OVMF would try to
> negotiate unplug), but the user runs it on QEMU-5.1 (or more precisely
> with an 5.1 machine type), which will support plug, but not unplug, then
> QEMU should reject the device_del QMP command.
>
> In brief, we need both properties because we want both device_add and
> device_del to fail gracefully, but the default values (and accordingly
> the compat properties) should reflect the actual feature support. So
> unplug should default to false at this point.
With (1) updated:
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
next prev parent reply other threads:[~2020-07-22 13:27 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-20 14:16 [PATCH 0/6] x86: fix cpu hotplug with secure boot Igor Mammedov
2020-07-20 14:16 ` [PATCH 1/6] x86: lpc9: let firmware negotiate 'CPU hotplug with SMI' features Igor Mammedov
2020-07-22 12:03 ` Laszlo Ersek
2020-07-22 13:26 ` Laszlo Ersek [this message]
2020-07-20 14:16 ` [PATCH 2/6] x86: cphp: prevent guest crash on CPU hotplug when broadcast SMI is in use Igor Mammedov
2020-07-22 13:16 ` Laszlo Ersek
2020-07-20 14:16 ` [PATCH 3/6] x86: cpuhp: refuse cpu hot-unplug request earlier if not supported Igor Mammedov
2020-07-22 13:24 ` Laszlo Ersek
2020-07-20 14:16 ` [PATCH 4/6] tests: acpi: mark to be changed tables in bios-tables-test-allowed-diff Igor Mammedov
2020-07-20 14:16 ` [PATCH 5/6] x68: acpi: trigger SMI before scanning for hotplugged CPUs Igor Mammedov
2020-07-22 15:30 ` Laszlo Ersek
2020-07-20 14:16 ` [PATCH 6/6] tests: acpi: update acpi blobs with new AML Igor Mammedov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2d97286a-2e8e-dd6b-822c-58959d0a74e2@redhat.com \
--to=lersek@redhat.com \
--cc=boris.ostrovsky@oracle.com \
--cc=imammedo@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).