From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mx.groups.io with SMTP id smtpd.web10.14721.1674048321048896591 for ; Wed, 18 Jan 2023 05:25:21 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=h4Jle68a; spf=pass (domain: redhat.com, ip: 170.10.133.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1674048320; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=zqAY0dv8Ozm/1Mu04JePajksOPRjm7UtY5chOnQqrtM=; b=h4Jle68aHPS6T84xHSk77FlHQtSr2KQL9VRvlIROM7RQBtd0Vf5r6hZmaaMuxcvTzg1do+ o+k2veFjTFH/UKRqJ5w2UZeMs6wbLAj3yR15vEOJ/TWqzNZyydIIUthzcEXHKBEiVgCUxb YHrHP1288sTKEg+9sui9Jt3xLhYE96U= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-540-xcjF6O9yPyyAKAUL57h7hQ-1; Wed, 18 Jan 2023 08:25:15 -0500 X-MC-Unique: xcjF6O9yPyyAKAUL57h7hQ-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 60B9D196EF89; Wed, 18 Jan 2023 13:25:14 +0000 (UTC) Received: from [10.39.192.94] (unknown [10.39.192.94]) by smtp.corp.redhat.com (Postfix) with ESMTPS id E852D1121315; Wed, 18 Jan 2023 13:25:11 +0000 (UTC) Message-ID: <37568c75-2a5d-4cba-a798-c7f6b6c92b3d@redhat.com> Date: Wed, 18 Jan 2023 14:25:10 +0100 MIME-Version: 1.0 Subject: Re: [edk2-devel] [PATCH v2] OvmfPkg/PlatformInitLib: catch QEMU's CPU hotplug reg block regression To: Gerd Hoffmann Cc: Ard Biesheuvel , devel@edk2.groups.io, Michael Brown , Ard Biesheuvel , Brijesh Singh , Erdem Aktas , James Bottomley , Jiewen Yao , Jordan Justen , Min Xu , Oliver Steffen , Sebastien Boeuf , Tom Lendacky References: <49e4e8bb-3bbd-0ca8-ee59-e75560deffa7@redhat.com> <20230113060354.siony3rjwpgzd5tk@sirius.home.kraxel.org> <20230113093205.oh7euprqlmp26wpu@sirius.home.kraxel.org> <20230113122246.uabdhut4ziwerivm@sirius.home.kraxel.org> <9141ad66-f868-762c-7ea5-d88753466fa6@redhat.com> <20230117123700.ntg5fk7a3ggr2xyo@sirius.home.kraxel.org> <20230118072525.cnk5ysqcnvdyeqow@sirius.home.kraxel.org> <7c15856d-d09c-092d-e8c8-6f5eab27182e@redhat.com> <20230118131044.2rtb2wmb77qcbb4q@sirius.home.kraxel.org> From: "Laszlo Ersek" In-Reply-To: <20230118131044.2rtb2wmb77qcbb4q@sirius.home.kraxel.org> X-Scanned-By: MIMEDefang 3.1 on 10.11.54.3 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On 1/18/23 14:10, Gerd Hoffmann wrote: > Hi, > >> ... you could introduce a new fw_cfg boolean switch (and explain it in >> the hang message) that meant: "I know what this QEMU bug is, I >> understand its consequences are obscure, risky, and far-reaching in >> OVMF, I've been warned, I know what I'm doing". That's a relatively >> small addition to this patch, and then the risk is assumed by the user. >> It resolves "being out of luck *entirely*". > > Using -fw_cfg would work on old qemu versions indeed. > > take care, > Gerd > > From 65a4f683eaf94f82693811ce9b2393586a15afd7 Mon Sep 17 00:00:00 2001 > From: Gerd Hoffmann > Date: Fri, 13 Jan 2023 13:07:36 +0100 > Subject: [PATCH 1/1] OvmfPkg/PlatformInitLib: add switch to disable cpu > hotplug support. > > Add a fw_cfg-based switch to disable cpu hotplug support in OVMF. > This allows to boot OVMF on known-broken qemu versions. > > This is only safe to do in case hotplug is not used. Taking care > of that is left to the user. > > Signed-off-by: Gerd Hoffmann > --- > OvmfPkg/Library/PlatformInitLib/Platform.c | 30 ++++++++++++++++++++-- > 1 file changed, 28 insertions(+), 2 deletions(-) > > diff --git a/OvmfPkg/Library/PlatformInitLib/Platform.c b/OvmfPkg/Library/PlatformInitLib/Platform.c > index c3d5f5eeb375..15811a4ff726 100644 > --- a/OvmfPkg/Library/PlatformInitLib/Platform.c > +++ b/OvmfPkg/Library/PlatformInitLib/Platform.c > @@ -414,8 +414,12 @@ PlatformMaxCpuCountInitialization ( > IN OUT EFI_HOB_PLATFORM_INFO *PlatformInfoHob > ) > { > - UINT16 BootCpuCount = 0; > - UINT32 MaxCpuCount; > + FIRMWARE_CONFIG_ITEM SupportedFeaturesItem; > + UINTN SupportedFeaturesSize; > + EFI_STATUS Status; > + UINT16 BootCpuCount = 0; > + UINT32 MaxCpuCount; > + BOOLEAN CpuHotplugDisabled = FALSE; > > // > // Try to fetch the boot CPU count. > @@ -425,6 +429,15 @@ PlatformMaxCpuCountInitialization ( > BootCpuCount = QemuFwCfgRead16 (); > } > > + Status = QemuFwCfgFindFile ( > + "opt/org.tianocore/nocpuhotplug", > + &SupportedFeaturesItem, > + &SupportedFeaturesSize > + ); > + if (!EFI_ERROR (Status)) { > + CpuHotplugDisabled = TRUE; > + } > + > if (BootCpuCount == 0) { > // > // QEMU doesn't report the boot CPU count. (BootCpuCount == 0) will let > @@ -434,6 +447,9 @@ PlatformMaxCpuCountInitialization ( > // > DEBUG ((DEBUG_WARN, "%a: boot CPU count unavailable\n", __FUNCTION__)); > MaxCpuCount = PlatformInfoHob->DefaultMaxCpuNumber; > + } else if (CpuHotplugDisabled) { > + DEBUG ((DEBUG_INFO, "%a: CPU hotplug support disabled via opt/org.tianocore/nocpuhotplug\n", __FUNCTION__)); > + MaxCpuCount = BootCpuCount; > } else { > // > // We will expose BootCpuCount to MpInitLib. MpInitLib will count APs up to > @@ -563,12 +579,22 @@ PlatformMaxCpuCountInitialization ( > DEBUG_ERROR, > "%a: Broken CPU hotplug register block: Present=%u Possible=%u.\n" > "%a: Update QEMU to v8, or to stable with dab30fbef389 backported.\n" > + "\n" > + "%a: Alternatively start qemu with:\n" > + "%a: -fw_cfg name=opt/org.tianocore/nocpuhotplug,string=1\n" > + "%a: to disable OVMF cpu hotplug support. Note that you must\n" > + "%a: not ask qemu to hotplug CPUs then\n" > + "\n" > "%a: Refer to " > ".\n", > __FUNCTION__, > Present, > Possible, > __FUNCTION__, > + __FUNCTION__, > + __FUNCTION__, > + __FUNCTION__, > + __FUNCTION__, > __FUNCTION__ > )); > ASSERT (FALSE); ... let me post a v3 in this direction, later. Thanks Laszlo