Re: [edk2-devel] [PATCH v2] OvmfPkg/PlatformInitLib: catch QEMU's CPU hotplug reg block regression

["Laszlo Ersek" <lersek@redhat.com>]

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 <kraxel@redhat.com>
> 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 <kraxel@redhat.com>
> ---
>  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 "
>            "<https://bugzilla.tianocore.org/show_bug.cgi?id=4250>.\n",
>            __FUNCTION__,
>            Present,
>            Possible,
>            __FUNCTION__,
> +          __FUNCTION__,
> +          __FUNCTION__,
> +          __FUNCTION__,
> +          __FUNCTION__,
>            __FUNCTION__
>            ));
>          ASSERT (FALSE);

... let me post a v3 in this direction, later.

Thanks
Laszlo