[v4,1/6] x86: lpc9: let firmware negotiate 'CPU hotplug with SMI' features

It will allow firmware to notify QEMU that firmware requires SMI
being triggered on CPU hot[un]plug, so that it would be able to account
for hotplugged CPU and relocate it to new SMM base and/or safely remove
CPU on unplug.

Using negotiated features, follow up patches will insert SMI upcall
into AML code, to make sure that firmware processes hotplug before
guest OS would attempt to use new CPU.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
v4:
  - fix 5.2 machine types so they won't apply pc_compat_5_1
     (Laszlo Ersek <lersek@redhat.com>)
v3:
  - rebase on top of "[PATCH v2] hw: add compat machines for 5.2"
    so apply that before this patch
v2:
  - rebase on top of 5.1 (move compat values to 5.1 machine)
  - make "x-smi-cpu-hotunplug" false by default (Laszlo Ersek <lersek@redhat.com>)

fixup
---
 include/hw/i386/ich9.h |  2 ++
 hw/i386/pc.c           |  4 +++-
 hw/isa/lpc_ich9.c      | 13 +++++++++++++
 3 files changed, 18 insertions(+), 1 deletion(-)

On 08/24/20 13:00, Igor Mammedov wrote:
> It will allow firmware to notify QEMU that firmware requires SMI
> being triggered on CPU hot[un]plug, so that it would be able to account
> for hotplugged CPU and relocate it to new SMM base and/or safely remove
> CPU on unplug.
>
> Using negotiated features, follow up patches will insert SMI upcall
> into AML code, to make sure that firmware processes hotplug before
> guest OS would attempt to use new CPU.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> ---
> v4:
>   - fix 5.2 machine types so they won't apply pc_compat_5_1
>      (Laszlo Ersek <lersek@redhat.com>)
> v3:
>   - rebase on top of "[PATCH v2] hw: add compat machines for 5.2"
>     so apply that before this patch
> v2:
>   - rebase on top of 5.1 (move compat values to 5.1 machine)
>   - make "x-smi-cpu-hotunplug" false by default (Laszlo Ersek <lersek@redhat.com>)
>
> fixup
> ---
>  include/hw/i386/ich9.h |  2 ++
>  hw/i386/pc.c           |  4 +++-
>  hw/isa/lpc_ich9.c      | 13 +++++++++++++
>  3 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h
> index a98d10b252..d1bb3f7bf0 100644
> --- a/include/hw/i386/ich9.h
> +++ b/include/hw/i386/ich9.h
> @@ -247,5 +247,7 @@ typedef struct ICH9LPCState {
>
>  /* bit positions used in fw_cfg SMI feature negotiation */
>  #define ICH9_LPC_SMI_F_BROADCAST_BIT            0
> +#define ICH9_LPC_SMI_F_CPU_HOTPLUG_BIT          1
> +#define ICH9_LPC_SMI_F_CPU_HOT_UNPLUG_BIT       2
>
>  #endif /* HW_ICH9_H */
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 9aa813949c..583db11d28 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -97,7 +97,9 @@
>  #include "fw_cfg.h"
>  #include "trace.h"
>
> -GlobalProperty pc_compat_5_1[] = {};
> +GlobalProperty pc_compat_5_1[] = {
> +    { "ICH9-LPC", "x-smi-cpu-hotplug", "off" },
> +};
>  const size_t pc_compat_5_1_len = G_N_ELEMENTS(pc_compat_5_1);
>
>  GlobalProperty pc_compat_5_0[] = {
> diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
> index cd6e169d47..19f32bed3e 100644
> --- a/hw/isa/lpc_ich9.c
> +++ b/hw/isa/lpc_ich9.c
> @@ -373,6 +373,15 @@ static void smi_features_ok_callback(void *opaque)
>          /* guest requests invalid features, leave @features_ok at zero */
>          return;
>      }
> +    if (!(guest_features & BIT_ULL(ICH9_LPC_SMI_F_BROADCAST_BIT)) &&
> +        guest_features & (BIT_ULL(ICH9_LPC_SMI_F_CPU_HOTPLUG_BIT) |
> +                          BIT_ULL(ICH9_LPC_SMI_F_CPU_HOT_UNPLUG_BIT))) {
> +        /*
> +         * cpu hot-[un]plug with SMI requires SMI broadcast,
> +         * leave @features_ok at zero
> +         */
> +        return;
> +    }
>
>      /* valid feature subset requested, lock it down, report success */
>      lpc->smi_negotiated_features = guest_features;
> @@ -747,6 +756,10 @@ static Property ich9_lpc_properties[] = {
>      DEFINE_PROP_BOOL("noreboot", ICH9LPCState, pin_strap.spkr_hi, true),
>      DEFINE_PROP_BIT64("x-smi-broadcast", ICH9LPCState, smi_host_features,
>                        ICH9_LPC_SMI_F_BROADCAST_BIT, true),
> +    DEFINE_PROP_BIT64("x-smi-cpu-hotplug", ICH9LPCState, smi_host_features,
> +                      ICH9_LPC_SMI_F_CPU_HOTPLUG_BIT, true),
> +    DEFINE_PROP_BIT64("x-smi-cpu-hotunplug", ICH9LPCState, smi_host_features,
> +                      ICH9_LPC_SMI_F_CPU_HOT_UNPLUG_BIT, false),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>
>

This patch doesn't apply with git-am, as of commit 44423107e7b5 ("Merge
remote-tracking branch 'remotes/xtensa/tags/20200821-xtensa' into
staging", 2020-08-24).

The reason is that commit 2becc36a3e53 ("meson: infrastructure for
building emulators", 2020-08-21) introduced

#include CONFIG_DEVICES

to "hw/i386/pc.c", just above the (then) "pc_compat_5_0" array.

Then Cornelia's commit 3ff3c5d31740 ("hw: add compat machines for 5.2",
2020-08-19), which introduced "pc_compat_5_1" independently of the Meson
conversion, needed explicit resolution against CONFIG_DEVICES for
merging into master. That was covered in merge commit ca489cd037e4
("Merge remote-tracking branch
'remotes/ehabkost/tags/machine-next-pull-request' into staging",
2020-08-22).

So the patch applies on top of 3ff3c5d31740, but not on the merge
(ca489cd037e4) that brought 3ff3c5d31740 into master.

Not a problem though: I can apply the patch on 3ff3c5d31740, and then
cleanly (automatically) rebase to current HEAD (44423107e7b5) from
there. This is the range-diff across the rebase:

> 1:  9066fa4ccb49 ! 1:  05188fffe6aa x86: lpc9: let firmware negotiate 'CPU hotplug with SMI' features
>     @@ -31,8 +31,8 @@
>      --- a/hw/i386/pc.c
>      +++ b/hw/i386/pc.c
>      @@
>     - #include "fw_cfg.h"
>       #include "trace.h"
>     + #include CONFIG_DEVICES
>
>      -GlobalProperty pc_compat_5_1[] = {};
>      +GlobalProperty pc_compat_5_1[] = {


So it's indeed just a context update.

Having applied this series to QEMU, my test results are:

  OVMF has 5ba203b54e59  machine type  plug gate  unplug gate  gating result
  ---------------------  ------------  ---------  -----------  -------------
  no                     pc-q35-5.1    reject     reject       pass
  no                     pc-q35-5.2    reject     reject       pass
  yes                    pc-q35-5.1    reject     reject       pass
  yes                    pc-q35-5.2    accept     reject       pass

The results are the same after S3 suspend/resume. (This is relevant
because the features are re-negotiated during S3 resume.)

Thus, for this one patch, so far:

- (just to confirm:)
Reviewed-by: Laszlo Ersek <lersek@redhat.com>

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

Thanks!
Laszlo