From: "Gerd Hoffmann" <kraxel@redhat.com>
To: Laszlo Ersek <lersek@redhat.com>
Cc: devel@edk2.groups.io, Michael Brown <mcb30@ipxe.org>,
Ard Biesheuvel <ardb+tianocore@kernel.org>,
Brijesh Singh <brijesh.singh@amd.com>,
Erdem Aktas <erdemaktas@google.com>,
James Bottomley <jejb@linux.ibm.com>,
Jiewen Yao <jiewen.yao@intel.com>,
Jordan Justen <jordan.l.justen@intel.com>,
Min Xu <min.m.xu@intel.com>, Oliver Steffen <osteffen@redhat.com>,
Sebastien Boeuf <sebastien.boeuf@intel.com>,
Tom Lendacky <thomas.lendacky@amd.com>
Subject: Re: [edk2-devel] [PATCH v2] OvmfPkg/PlatformInitLib: catch QEMU's CPU hotplug reg block regression
Date: Fri, 13 Jan 2023 13:22:46 +0100 [thread overview]
Message-ID: <20230113122246.uabdhut4ziwerivm@sirius.home.kraxel.org> (raw)
In-Reply-To: <a8d268c9-f54a-4cad-8e11-ec8ef2838f54@redhat.com>
On Fri, Jan 13, 2023 at 11:10:54AM +0100, Laszlo Ersek wrote:
> On 1/13/23 10:32, Gerd Hoffmann wrote:
> > On Fri, Jan 13, 2023 at 07:03:54AM +0100, Gerd Hoffmann wrote:
> >> Hi,
> >>
> >>> - QEMU can be configured with other compat properties on the command
> >>> line so that "CPU hotplug with SMI" and "CPU hot-unplug with SMI" *not*
> >>> be offered to the firmware. Then QEMU will reject hotplug attempts, and
> >>> the SMM hotplug code in edk2 will not be triggered by the (virtual)
> >>> hardware.
> >>
> >> Can we have edk2 print instructions for that in the error message?
> >
> > This seems to be:
> >
> > qemu -M q35 \
> > -global ICH9-LPC.x-smi-cpu-hotplug=off \
> > -global ICH9-LPC.x-smi-cpu-hotunplug=off
>
> Yes, those are the flags.
>
> > But it appears to not work.
>
> They should work, but they take effect in QEMU, and not in the firmware.
> These knobs control what CPU hot(un)plug+SMI features QEMU exposes to
> the guest fw, via fw_cfg,
Ok, I see, only the SMM code actually checks that.
> In particular the firmware makes no further decisions based on whether
> QEMU advertized some of these features.
I was thinking the other way around: When cpu hotplug is disabled in
qemu it should be safe to skip the whole cpu hotplug checking dance.
See test patch below.
That would give us a config switch (turn off cpu hotplug support) which
would allow edk2 run on qemu versions with broken cpu hotplug.
Does the idea look sane or do I miss something?
take care,
Gerd
commit bd2e36eba35268ab46c0125d2b9125391ea6f9fc
Author: Gerd Hoffmann <kraxel@redhat.com>
Date: Fri Jan 13 13:07:36 2023 +0100
skip cpu present checking when hotplug is off
diff --git a/OvmfPkg/Library/PlatformInitLib/Platform.c b/OvmfPkg/Library/PlatformInitLib/Platform.c
index 13348afb4890..2b0f0c836f85 100644
--- a/OvmfPkg/Library/PlatformInitLib/Platform.c
+++ b/OvmfPkg/Library/PlatformInitLib/Platform.c
@@ -415,8 +415,9 @@ PlatformMaxCpuCountInitialization (
IN OUT EFI_HOB_PLATFORM_INFO *PlatformInfoHob
)
{
- UINT16 BootCpuCount = 0;
- UINT32 MaxCpuCount;
+ UINT16 BootCpuCount = 0;
+ UINT32 MaxCpuCount;
+ BOOLEAN CpuHotplugSupported = FALSE;
//
// Try to fetch the boot CPU count.
@@ -424,6 +425,31 @@ PlatformMaxCpuCountInitialization (
if (QemuFwCfgIsAvailable ()) {
QemuFwCfgSelectItem (QemuFwCfgItemSmpCpuCount);
BootCpuCount = QemuFwCfgRead16 ();
+ DEBUG ((DEBUG_INFO, "%a: BootCpuCount: %d\n", __FUNCTION__, BootCpuCount));
+ }
+
+ {
+ FIRMWARE_CONFIG_ITEM SupportedFeaturesItem;
+ UINTN SupportedFeaturesSize;
+ UINT64 mSmiFeatures;
+ EFI_STATUS Status;
+
+ Status = QemuFwCfgFindFile (
+ "etc/smi/supported-features",
+ &SupportedFeaturesItem,
+ &SupportedFeaturesSize
+ );
+
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_INFO, "%a: etc/smi/supported-features: %r\n", __FUNCTION__, Status));
+ } else {
+ QemuFwCfgSelectItem (SupportedFeaturesItem);
+ QemuFwCfgReadBytes (sizeof mSmiFeatures, &mSmiFeatures);
+ DEBUG ((DEBUG_INFO, "%a: etc/smi/supported-features: 0x%x\n", __FUNCTION__, mSmiFeatures));
+ if (mSmiFeatures & (BIT1 /* hotplug */ | BIT2 /* hotunplug */)) {
+ CpuHotplugSupported = TRUE;
+ }
+ }
}
if (BootCpuCount == 0) {
@@ -435,6 +461,9 @@ PlatformMaxCpuCountInitialization (
//
DEBUG ((DEBUG_WARN, "%a: boot CPU count unavailable\n", __FUNCTION__));
MaxCpuCount = PlatformInfoHob->DefaultMaxCpuNumber;
+ } else if (!CpuHotplugSupported) {
+ DEBUG ((DEBUG_INFO, "%a: CPU hotplug support not available\n", __FUNCTION__));
+ MaxCpuCount = BootCpuCount;
} else {
//
// We will expose BootCpuCount to MpInitLib. MpInitLib will count APs up to
next prev parent reply other threads:[~2023-01-13 12:22 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-12 8:28 [PATCH v2] OvmfPkg/PlatformInitLib: catch QEMU's CPU hotplug reg block regression Laszlo Ersek
2023-01-12 9:55 ` [edk2-devel] " Michael Brown
2023-01-12 10:09 ` Ard Biesheuvel
2023-01-12 13:31 ` Laszlo Ersek
2023-01-12 13:22 ` Laszlo Ersek
2023-01-12 16:08 ` Michael Brown
2023-01-12 17:58 ` Laszlo Ersek
2023-01-12 18:22 ` Laszlo Ersek
2023-01-12 22:49 ` Michael Brown
2023-01-13 6:03 ` Gerd Hoffmann
2023-01-13 9:32 ` Gerd Hoffmann
2023-01-13 10:10 ` Laszlo Ersek
2023-01-13 12:22 ` Gerd Hoffmann [this message]
2023-01-16 14:42 ` Ard Biesheuvel
2023-01-16 14:48 ` Laszlo Ersek
2023-01-17 12:37 ` Gerd Hoffmann
2023-01-17 16:43 ` Ard Biesheuvel
2023-01-18 7:25 ` Gerd Hoffmann
2023-01-18 11:50 ` Laszlo Ersek
2023-01-18 13:10 ` Gerd Hoffmann
2023-01-18 13:25 ` Laszlo Ersek
2023-01-18 13:10 ` Ard Biesheuvel
2023-01-18 13:21 ` Laszlo Ersek
2023-01-12 18:34 ` Laszlo Ersek
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-list 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=20230113122246.uabdhut4ziwerivm@sirius.home.kraxel.org \
--to=devel@edk2.groups.io \
/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