From: "Laszlo Ersek" <lersek@redhat.com>
To: Michael Brown <mcb30@ipxe.org>, devel@edk2.groups.io
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>,
Brijesh Singh <brijesh.singh@amd.com>,
Erdem Aktas <erdemaktas@google.com>,
Gerd Hoffmann <kraxel@redhat.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: Thu, 12 Jan 2023 19:22:04 +0100 [thread overview]
Message-ID: <ee050f45-420f-8dd9-1033-628011a61ba3@redhat.com> (raw)
In-Reply-To: <49e4e8bb-3bbd-0ca8-ee59-e75560deffa7@redhat.com>
On 1/12/23 18:58, Laszlo Ersek wrote:
> Your proposal is entirely justified, from a practical / user
> perspective, but I'm not the right person for it.
This should do what you propose (untested), but I hate the code myself.
diff --git a/OvmfPkg/Library/PlatformInitLib/Platform.c b/OvmfPkg/Library/PlatformInitLib/Platform.c
index 3e13c5d4b34f..b4c0b47c8bb5 100644
--- a/OvmfPkg/Library/PlatformInitLib/Platform.c
+++ b/OvmfPkg/Library/PlatformInitLib/Platform.c
@@ -541,6 +541,22 @@ PlatformMaxCpuCountInitialization (
ASSERT (Selected == Possible || Selected == 0);
} while (Selected > 0);
+ if (Present == 0) {
+ if (FeaturePcdGet (PcdSmmSmramRequire)) {
+ ASSERT (FALSE);
+ CpuDeadLoop();
+ }
+ //
+ // The bug is in QEMU v5.1.0+, where we're not affected by the QEMU v2.7
+ // reset bug, so BootCpuCount from fw_cfg is reliable. Assume a fully
+ // populated topology, precluding hotplug, like when the modern CPU
+ // hotplug interface is unavailable. This will satisfy the QEMU v2.7
+ // reset bug sanity check below as well.
+ //
+ Present = BootCpuCount;
+ Possible = BootCpuCount;
+ }
+
//
// Sanity check: fw_cfg and the modern CPU hotplug interface should
// return the same boot CPU count.
I *really* don't like this. With this, you might get through the firmware and reach the guest OS, and pre-boot, the MP PPI and protocol might even work. But, because the QEMU register block is broken, I don't have the slightest idea what's going to happen if you attempt a CPU hot-*unplug* at OS runtime (unplug because in effect you start with a fully populated topology). OVMF does not participate in that (because of no SMM), but the ACPI content generated by QEMU does, and that content (AML methods) do massage the register block.
In a sense, with the register block negotiation broken, even without SMM in the picture, the guest OS could consider OVMF doing it a *service* by hanging, so that guest OS-level data is not lost when the admin attempts an unplug, and the guest OS crashes. (I've not reproduced such a crash to be clear, I just think it could be possible.)
There's got to be a limit to how far we try to compensate for broken (virtual) hardware. :( The right thing to do is to wait for the QEMU patch to reach as many as possible stable branches, let the distros pick up the new stable releases, and then merge the hardliner hang.
Laszlo
next prev parent reply other threads:[~2023-01-12 18: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 [this message]
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
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=ee050f45-420f-8dd9-1033-628011a61ba3@redhat.com \
--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