Re: [RFC 0/3] x86: fix cpu hotplug with secure boot

[Laszlo Ersek <lersek@redhat.com>]

On 07/10/20 18:17, Igor Mammedov wrote:
> CPU hotplug with Secure Boot was not really supported and firmware wasn't aware
> of hotplugged CPUs (which might lead to guest crashes). During 4.2 we introduced
> locked SMI handler RAM arrea to make sure that guest OS wasn't able to inject
> its own SMI handler and OVMF added initial CPU hotplug support.
>
> This series is QEMU part of that support [1] which lets QMVF tell QEMU that
> CPU hotplug with SMI broadcast enabled is supported so that QEMU would be able
> to prevent hotplug in case it's not supported and trigger SMI on hotplug when
> it's necessary.
>
> 1) CPU hotplug negotiation part was introduced later so it might not be
> in upstream OVMF yet or I might have missed the patch on edk2-devel
> (Laszlo will point out to it/post formal patch)
>
> Igor Mammedov (3):
>   x86: lpc9: let firmware negotiate CPU hotplug SMI feature
>   x86: cphp: prevent guest crash on CPU hotplug when broadcast SMI is in
>     use
>   x68: acpi: trigger SMI before scanning for hotplugged CPUs
>
>  include/hw/acpi/cpu.h  |  1 +
>  include/hw/i386/ich9.h |  1 +
>  hw/acpi/cpu.c          |  6 ++++++
>  hw/acpi/ich9.c         | 12 +++++++++++-
>  hw/i386/acpi-build.c   | 33 ++++++++++++++++++++++++++++++++-
>  hw/i386/pc.c           | 15 ++++++++++++++-
>  hw/isa/lpc_ich9.c      | 10 ++++++++++
>  7 files changed, 75 insertions(+), 3 deletions(-)
>

I applied the series on top of QEMU commit 20c1df5476e1,
plus the unrelated build fix that I proposed in:

  Re: [PULL 14/31] target/i386: reimplement f2xm1 using floatx80
  operation

at:

  https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg04714.html
  (alternative link:
  <http://mid.mail-archive.com/a3302e58-c470-9305-b106-a2b6b2c52d39@redhat.com>)

I used the following command for hotplug:

$ virsh setvcpu ovmf.fedora.q35 3 --enable --live

Results:

  OVMF SMM_REQUIRE  edk2                machine type   result
  ----------------  ------------------  -------------  ----------------------------
  FALSE             9c6f3545aee0        pc-i440fx-5.0  hotplug accepted [1]
  FALSE             9c6f3545aee0        pc-i440fx-5.1  hotplug accepted [1]
  FALSE             9c6f3545aee0        pc-q35-5.0     hotplug accepted [1]
  FALSE             9c6f3545aee0        pc-q35-5.1     hotplug accepted [1]
  FALSE             9c6f3545aee0+patch  pc-i440fx-5.0  hotplug accepted [1]
  FALSE             9c6f3545aee0+patch  pc-i440fx-5.1  hotplug accepted [1]
  FALSE             9c6f3545aee0+patch  pc-q35-5.0     hotplug accepted [1]
  FALSE             9c6f3545aee0+patch  pc-q35-5.1     hotplug accepted [1]
  TRUE              9c6f3545aee0        pc-q35-5.0     hotplug rejected [2]
  TRUE              9c6f3545aee0        pc-q35-5.1     hotplug rejected [2]
  TRUE              9c6f3545aee0+BUG    pc-q35-5.1     negotiation rejected [3]
  TRUE              9c6f3545aee0+patch  pc-q35-5.0     hotplug rejected [2]
  TRUE              9c6f3545aee0+patch  pc-q35-5.1     hotplug accepted [4] [5] [6]

[1] In this case, i.e., when OVMF is built without -D SMM_REQUIRE, the
    firmware is not supposed to learn about CPU hotplug at OS runtime;
    only the OS should care. No SMI is raised in ACPI. So this is a
    regression-test for when SMM is not used at all.

[2] "error: internal error: unable to execute QEMU command 'device_add':
     cpu hotplug SMI was not enabled by firmware"

[3] In this test, I intentionally broke the firmware so that it would
    negotiate "CPU hotplug with SMI", but not "SMI broadcast". In this
    case, QEMU rejects the feature request, the firmware detects that,
    and hangs (intentionally, as this is a programming error in the
    firmware -- a failed assertion).

[4] Tested hotplug in Linux guest with "rdmsr -a 0x3a", "taskset -c X
    efibootmgr", and S3 suspend/resume, as described at
    https://github.com/tianocore/tianocore.github.io/wiki/Testing-SMM-with-QEMU,-KVM-and-libvirt#tests-to-perform-in-the-installed-guest-fedora-26-guest

[5] Also tested hotplug with Windows Server 2012 R2; checking the CPU
    count in the Task Manager | Logical Processors view, and in Device
    Manager | Processors. Tested S3 suspend/resume too.

[6] The S3 suspend/resume test is relevant in two forms -- first, after
    hot-adding CPUs, S3 suspend/resume continues to work.

    Second, during S3 resume, the "S3 boot script" re-selects the same
    SMI features originally negotiated during normal boot, so plugging a
    new CPU works after S3 resume too.

    Consider the following S3 boot script difference (executed during S3
    resume), taken between pc-q35-5.0 and pc-q35-5.1:

     ExecuteBootScript - 7BB67037
     EFI_BOOT_SCRIPT_MEM_WRITE_OPCODE
     BootScriptExecuteMemoryWrite - 0x7BB66018, 0x00000018, 0x00000000
     S3BootScriptWidthUint8 - 0x7BB66018 (0x00)
     S3BootScriptWidthUint8 - 0x7BB66019 (0x2B)
     S3BootScriptWidthUint8 - 0x7BB6601A (0x00)
     S3BootScriptWidthUint8 - 0x7BB6601B (0x18)
     S3BootScriptWidthUint8 - 0x7BB6601C (0x00)
     S3BootScriptWidthUint8 - 0x7BB6601D (0x00)
     S3BootScriptWidthUint8 - 0x7BB6601E (0x00)
     S3BootScriptWidthUint8 - 0x7BB6601F (0x08)
     S3BootScriptWidthUint8 - 0x7BB66020 (0x00)
     S3BootScriptWidthUint8 - 0x7BB66021 (0x00)
     S3BootScriptWidthUint8 - 0x7BB66022 (0x00)
     S3BootScriptWidthUint8 - 0x7BB66023 (0x00)
     S3BootScriptWidthUint8 - 0x7BB66024 (0x7B)
     S3BootScriptWidthUint8 - 0x7BB66025 (0xB6)
     S3BootScriptWidthUint8 - 0x7BB66026 (0x60)
     S3BootScriptWidthUint8 - 0x7BB66027 (0x28)
    -S3BootScriptWidthUint8 - 0x7BB66028 (0x01)
    +S3BootScriptWidthUint8 - 0x7BB66028 (0x03)
     S3BootScriptWidthUint8 - 0x7BB66029 (0x00)
     S3BootScriptWidthUint8 - 0x7BB6602A (0x00)
     S3BootScriptWidthUint8 - 0x7BB6602B (0x00)
     S3BootScriptWidthUint8 - 0x7BB6602C (0x00)
     S3BootScriptWidthUint8 - 0x7BB6602D (0x00)
     S3BootScriptWidthUint8 - 0x7BB6602E (0x00)
     S3BootScriptWidthUint8 - 0x7BB6602F (0x00)

    This is an fw_cfg DMA write preparation.

    - The first four bytes are "FWCfgDmaAccess.control" (which is big
      endian), performing a "select" (via bit#3 in value 0x18) for item
      0x2b, and initiating a "write" (bit#4 in value 0x18).

    - The next four bytes (BE) specify "FWCfgDmaAccess.length" = 8 --
      which is the size of the SMI guest features bitmask.

    - The next eight bytes (BE) are "FWCfgDmaAccess.address" =
      0x7BB66028.

    - The blob to transfer follows the FWCfgDmaAccess structure
      immediately, at 0x7BB66018 + 4 + 4 + 8 = 0x7BB66028. It consists
      of a little-endian uint64_t: the SMI guest  features bitmask. The
      diff shows that ICH9_LPC_SMI_F_CPU_HOTPLUG_BIT (value 2) is
      re-selected during S3 resume, on "pc-q35-5.1".

    The firmware checks the result of the feature (re)selection during
    S3 resume too.

All of the test cases behaved as expected.

I understand this series is an RFC, and my own self requested updates,
but to capture the results thus far (for plug only):

Tested-by: Laszlo Ersek <lersek@redhat.com>

Thanks
Laszlo