qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 00/11] x86: fix cpu hotplug with secure boot
@ 2020-09-23  9:46 Igor Mammedov
  2020-09-23  9:46 ` [PATCH v6 01/11] x86: lpc9: let firmware negotiate 'CPU hotplug with SMI' features Igor Mammedov
                   ` (11 more replies)
  0 siblings, 12 replies; 16+ messages in thread
From: Igor Mammedov @ 2020-09-23  9:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: lersek, ankur.a.arora, mst

v6:
  - [9/10] Add comment explaining why while_ctx2 restarts from the last processed CPU.
  - rebase on top of current master, due to non trivial conflict
    caused by microvm series, which moved/renamed pc_cpu_pre_plug()
v5:
  - fix hotplug on Windows when there is more than 256 possible CPUs
    (Windows isn't able to handle VarPackage over 255 elements
     so process CPUs in batches)
  - fix off-by-one in package length (Laszlo)
  - fix not selecting CPU before clearing insert event (Laszlo)
  - use aml_lgreater() instead of aml_lnot(aml_equal(num_added_cpus, zero) (Laszlo)
  - split 'x68: acpi: trigger SMI before sending hotplug Notify event to OSPM'
    in samller chunks (Laszlo)
  - fix comment to match spec (Laszlo)
  - reorder aml_lor() and aml_land() in header (Laszlo)
v4:
  - fix 5.2 machine types so they won't apply pc_compat_5_1 (Laszlo)
v3:
  - rebase on top of "[PATCH v2] hw: add compat machines for 5.2"
    so apply that before this patch
v2:
  - AML: clean is_inserted flag only after SMI callback
  - make x-smi-cpu-hotunplug false by default
  - massage error hint on not supported unplug
v1:
  - fix typos and some phrases (Laszlo)
  - add unplug check (Laszlo)
  - redo AML scan logic to avoid race when adding multiple CPUs

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 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.

Igor Mammedov (11):
  x86: lpc9: let firmware negotiate 'CPU hotplug with SMI' features
  x86: cpuhp: prevent guest crash on CPU hotplug when broadcast SMI is
    in use
  x86: cpuhp: refuse cpu hot-unplug request earlier if not supported
  acpi: add aml_land() and aml_break() primitives
  tests: acpi: mark to be changed tables in
    bios-tables-test-allowed-diff
  x86: ich9: expose "smi_negotiated_features" as a QOM property
  x86: acpi: introduce AcpiPmInfo::smi_on_cpuhp
  x86: acpi: introduce the PCI0.SMI0 ACPI device
  x68: acpi: trigger SMI before sending hotplug Notify event to OSPM
  tests: acpi: update acpi blobs with new AML
  smp: drop support for deprecated (invalid topologies)

 include/hw/acpi/aml-build.h       |   2 +
 include/hw/acpi/cpu.h             |   1 +
 include/hw/i386/ich9.h            |   4 +
 docs/system/deprecated.rst        |  26 ++---
 hw/acpi/aml-build.c               |  16 +++
 hw/acpi/cpu.c                     | 165 +++++++++++++++++++++++++-----
 hw/acpi/ich9.c                    |  24 ++++-
 hw/core/machine.c                 |  16 +--
 hw/i386/acpi-build.c              |  35 ++++++-
 hw/i386/pc.c                      |  20 ++--
 hw/i386/x86.c                     |  11 ++
 hw/isa/lpc_ich9.c                 |  16 +++
 tests/data/acpi/pc/DSDT           | Bin 4934 -> 5060 bytes
 tests/data/acpi/pc/DSDT.acpihmat  | Bin 6258 -> 6385 bytes
 tests/data/acpi/pc/DSDT.bridge    | Bin 6793 -> 6919 bytes
 tests/data/acpi/pc/DSDT.cphp      | Bin 5397 -> 5524 bytes
 tests/data/acpi/pc/DSDT.dimmpxm   | Bin 6587 -> 6714 bytes
 tests/data/acpi/pc/DSDT.ipmikcs   | Bin 5006 -> 5132 bytes
 tests/data/acpi/pc/DSDT.memhp     | Bin 6293 -> 6419 bytes
 tests/data/acpi/pc/DSDT.numamem   | Bin 4940 -> 5066 bytes
 tests/data/acpi/q35/DSDT          | Bin 7678 -> 7804 bytes
 tests/data/acpi/q35/DSDT.acpihmat | Bin 9002 -> 9129 bytes
 tests/data/acpi/q35/DSDT.bridge   | Bin 7695 -> 7821 bytes
 tests/data/acpi/q35/DSDT.cphp     | Bin 8141 -> 8268 bytes
 tests/data/acpi/q35/DSDT.dimmpxm  | Bin 9331 -> 9458 bytes
 tests/data/acpi/q35/DSDT.ipmibt   | Bin 7753 -> 7879 bytes
 tests/data/acpi/q35/DSDT.memhp    | Bin 9037 -> 9163 bytes
 tests/data/acpi/q35/DSDT.mmio64   | Bin 8808 -> 8934 bytes
 tests/data/acpi/q35/DSDT.numamem  | Bin 7684 -> 7810 bytes
 tests/data/acpi/q35/DSDT.tis      | Bin 8283 -> 8409 bytes
 30 files changed, 269 insertions(+), 67 deletions(-)

-- 
2.27.0



^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v6 01/11] x86: lpc9: let firmware negotiate 'CPU hotplug with SMI' features
  2020-09-23  9:46 [PATCH v6 00/11] x86: fix cpu hotplug with secure boot Igor Mammedov
@ 2020-09-23  9:46 ` Igor Mammedov
  2020-09-23  9:46 ` [PATCH v6 02/11] x86: cpuhp: prevent guest crash on CPU hotplug when broadcast SMI is in use Igor Mammedov
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Igor Mammedov @ 2020-09-23  9:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: lersek, ankur.a.arora, mst

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>
Tested-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 48b442bc0b..703d56036a 100644
--- a/include/hw/i386/ich9.h
+++ b/include/hw/i386/ich9.h
@@ -247,5 +247,7 @@ 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 b55369357e..2af660c55e 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -97,7 +97,9 @@
 #include "trace.h"
 #include CONFIG_DEVICES
 
-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 3303d2eab6..a54b3ec8bd 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(),
 };
 
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v6 02/11] x86: cpuhp: prevent guest crash on CPU hotplug when broadcast SMI is in use
  2020-09-23  9:46 [PATCH v6 00/11] x86: fix cpu hotplug with secure boot Igor Mammedov
  2020-09-23  9:46 ` [PATCH v6 01/11] x86: lpc9: let firmware negotiate 'CPU hotplug with SMI' features Igor Mammedov
@ 2020-09-23  9:46 ` Igor Mammedov
  2020-09-23 17:16   ` Laszlo Ersek
  2020-09-23  9:46 ` [PATCH v6 03/11] x86: cpuhp: refuse cpu hot-unplug request earlier if not supported Igor Mammedov
                   ` (9 subsequent siblings)
  11 siblings, 1 reply; 16+ messages in thread
From: Igor Mammedov @ 2020-09-23  9:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: lersek, ankur.a.arora, mst

There were reports of guest crash on CPU hotplug, when using q35 machine
type and OVMF with SMM, due to hotplugged CPU trying to process SMI at
default SMI handler location without it being relocated by firmware first.

Fix it by refusing hotplug if firmware hasn't negotiated CPU hotplug with
SMI support while SMI broadcast is in use.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>
---
v6:
  - rebase on top of current master, due to non trivial conflicts
    caused by microvm series, which moved/renamed pc_cpu_pre_plug()
    (Ankur Arora <ankur.a.arora@oracle.com>)
v1:
   fix typos an use suggested wording in commit and error msg
   s/secure boot/smm/; s/hotplug SMI/hotplug with SMI/
      (Laszlo Ersek <lersek@redhat.com>)
---
 hw/acpi/ich9.c | 12 +++++++++++-
 hw/i386/x86.c  | 11 +++++++++++
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index 6a19070cec..0acc9a3107 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -408,10 +408,20 @@ void ich9_pm_device_pre_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
     ICH9LPCState *lpc = ICH9_LPC_DEVICE(hotplug_dev);
 
     if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) &&
-        !lpc->pm.acpi_memory_hotplug.is_enabled)
+        !lpc->pm.acpi_memory_hotplug.is_enabled) {
         error_setg(errp,
                    "memory hotplug is not enabled: %s.memory-hotplug-support "
                    "is not set", object_get_typename(OBJECT(lpc)));
+    } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
+        uint64_t negotiated = lpc->smi_negotiated_features;
+
+        if (negotiated & BIT_ULL(ICH9_LPC_SMI_F_BROADCAST_BIT) &&
+            !(negotiated & BIT_ULL(ICH9_LPC_SMI_F_CPU_HOTPLUG_BIT))) {
+            error_setg(errp, "cpu hotplug with SMI wasn't enabled by firmware");
+            error_append_hint(errp, "update machine type to newer than 5.1 "
+                "and firmware that suppors CPU hotplug with SMM");
+        }
+    }
 }
 
 void ich9_pm_device_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index c2ea989579..403c2b1dad 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -279,6 +279,17 @@ void x86_cpu_pre_plug(HotplugHandler *hotplug_dev,
         return;
     }
 
+    if (x86ms->acpi_dev) {
+        Error *local_err = NULL;
+
+        hotplug_handler_pre_plug(HOTPLUG_HANDLER(x86ms->acpi_dev), dev,
+                                 &local_err);
+        if (local_err) {
+            error_propagate(errp, local_err);
+            return;
+        }
+    }
+
     init_topo_info(&topo_info, x86ms);
 
     env->nr_dies = x86ms->smp_dies;
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v6 03/11] x86: cpuhp: refuse cpu hot-unplug request earlier if not supported
  2020-09-23  9:46 [PATCH v6 00/11] x86: fix cpu hotplug with secure boot Igor Mammedov
  2020-09-23  9:46 ` [PATCH v6 01/11] x86: lpc9: let firmware negotiate 'CPU hotplug with SMI' features Igor Mammedov
  2020-09-23  9:46 ` [PATCH v6 02/11] x86: cpuhp: prevent guest crash on CPU hotplug when broadcast SMI is in use Igor Mammedov
@ 2020-09-23  9:46 ` Igor Mammedov
  2020-09-23  9:46 ` [PATCH v6 04/11] acpi: add aml_land() and aml_break() primitives Igor Mammedov
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Igor Mammedov @ 2020-09-23  9:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: lersek, ankur.a.arora, mst

CPU hot-unplug with SMM requires firmware participation to prevent
guest crash (i.e. CPU can be removed only after OS _and_ firmware
were prepared for the action).
Previous patches introduced ICH9_LPC_SMI_F_CPU_HOT_UNPLUG_BIT
feature bit, which is advertised by firmware when it has support
for CPU hot-unplug. Use it to check if guest is able to handle
unplug and make device_del fail gracefully if hot-unplug feature
hasn't been negotiated.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
v2:
 - fix typo in commit message
 - drop 5.1 version from hint message (Laszlo)
---
 hw/acpi/ich9.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index 0acc9a3107..95cb0f935b 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -460,6 +460,18 @@ void ich9_pm_device_unplug_request_cb(HotplugHandler *hotplug_dev,
                                       errp);
     } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU) &&
                !lpc->pm.cpu_hotplug_legacy) {
+        uint64_t negotiated = lpc->smi_negotiated_features;
+
+        if (negotiated & BIT_ULL(ICH9_LPC_SMI_F_BROADCAST_BIT) &&
+            !(negotiated & BIT_ULL(ICH9_LPC_SMI_F_CPU_HOT_UNPLUG_BIT))) {
+            error_setg(errp, "cpu hot-unplug with SMI wasn't enabled "
+                             "by firmware");
+            error_append_hint(errp, "update machine type to a version having "
+                                    "x-smi-cpu-hotunplug=on and firmware that "
+                                    "supports CPU hot-unplug with SMM");
+            return;
+        }
+
         acpi_cpu_unplug_request_cb(hotplug_dev, &lpc->pm.cpuhp_state,
                                    dev, errp);
     } else {
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v6 04/11] acpi: add aml_land() and aml_break() primitives
  2020-09-23  9:46 [PATCH v6 00/11] x86: fix cpu hotplug with secure boot Igor Mammedov
                   ` (2 preceding siblings ...)
  2020-09-23  9:46 ` [PATCH v6 03/11] x86: cpuhp: refuse cpu hot-unplug request earlier if not supported Igor Mammedov
@ 2020-09-23  9:46 ` Igor Mammedov
  2020-09-23  9:46 ` [PATCH v6 05/11] tests: acpi: mark to be changed tables in bios-tables-test-allowed-diff Igor Mammedov
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Igor Mammedov @ 2020-09-23  9:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: Philippe Mathieu-Daudé, lersek, ankur.a.arora, mst

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>
---
v5:
 - fix comment to match spec (Laszlo Ersek <lersek@redhat.com>)
 - reorder aml_lor() and aml_land() in header (Laszlo Ersek <lersek@redhat.com>)
---
 include/hw/acpi/aml-build.h |  2 ++
 hw/acpi/aml-build.c         | 16 ++++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index d27da03d64..fe0055fffb 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -290,6 +290,7 @@ Aml *aml_to_buffer(Aml *src, Aml *dst);
 Aml *aml_store(Aml *val, Aml *target);
 Aml *aml_and(Aml *arg1, Aml *arg2, Aml *dst);
 Aml *aml_or(Aml *arg1, Aml *arg2, Aml *dst);
+Aml *aml_land(Aml *arg1, Aml *arg2);
 Aml *aml_lor(Aml *arg1, Aml *arg2);
 Aml *aml_shiftleft(Aml *arg1, Aml *count);
 Aml *aml_shiftright(Aml *arg1, Aml *count, Aml *dst);
@@ -300,6 +301,7 @@ Aml *aml_increment(Aml *arg);
 Aml *aml_decrement(Aml *arg);
 Aml *aml_index(Aml *arg1, Aml *idx);
 Aml *aml_notify(Aml *arg1, Aml *arg2);
+Aml *aml_break(void);
 Aml *aml_call0(const char *method);
 Aml *aml_call1(const char *method, Aml *arg1);
 Aml *aml_call2(const char *method, Aml *arg1, Aml *arg2);
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index f6fbc9b95d..3792ba96ce 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -556,6 +556,15 @@ Aml *aml_or(Aml *arg1, Aml *arg2, Aml *dst)
     return build_opcode_2arg_dst(0x7D /* OrOp */, arg1, arg2, dst);
 }
 
+/* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefLAnd */
+Aml *aml_land(Aml *arg1, Aml *arg2)
+{
+    Aml *var = aml_opcode(0x90 /* LAndOp */);
+    aml_append(var, arg1);
+    aml_append(var, arg2);
+    return var;
+}
+
 /* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefLOr */
 Aml *aml_lor(Aml *arg1, Aml *arg2)
 {
@@ -629,6 +638,13 @@ Aml *aml_notify(Aml *arg1, Aml *arg2)
     return var;
 }
 
+/* ACPI 1.0b: 16.2.5.3 Type 1 Opcodes Encoding: DefBreak */
+Aml *aml_break(void)
+{
+    Aml *var = aml_opcode(0xa5 /* BreakOp */);
+    return var;
+}
+
 /* helper to call method without argument */
 Aml *aml_call0(const char *method)
 {
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v6 05/11] tests: acpi: mark to be changed tables in bios-tables-test-allowed-diff
  2020-09-23  9:46 [PATCH v6 00/11] x86: fix cpu hotplug with secure boot Igor Mammedov
                   ` (3 preceding siblings ...)
  2020-09-23  9:46 ` [PATCH v6 04/11] acpi: add aml_land() and aml_break() primitives Igor Mammedov
@ 2020-09-23  9:46 ` Igor Mammedov
  2020-09-23  9:46 ` [PATCH v6 06/11] x86: ich9: expose "smi_negotiated_features" as a QOM property Igor Mammedov
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Igor Mammedov @ 2020-09-23  9:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: lersek, ankur.a.arora, mst

... to let tests pass until binary blobs are updated with new AML

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 tests/qtest/bios-tables-test-allowed-diff.h | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523c8b..dba32d5613 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1 +1,20 @@
 /* List of comma-separated changed AML files to ignore */
+"tests/data/acpi/pc/DSDT",
+"tests/data/acpi/q35/DSDT",
+"tests/data/acpi/q35/DSDT.tis",
+"tests/data/acpi/q35/DSDT.bridge",
+"tests/data/acpi/q35/DSDT.mmio64",
+"tests/data/acpi/q35/DSDT.ipmibt",
+"tests/data/acpi/q35/DSDT.cphp",
+"tests/data/acpi/q35/DSDT.memhp",
+"tests/data/acpi/q35/DSDT.numamem",
+"tests/data/acpi/q35/DSDT.dimmpxm",
+"tests/data/acpi/q35/DSDT.acpihmat",
+"tests/data/acpi/pc/DSDT.bridge",
+"tests/data/acpi/pc/DSDT.ipmikcs",
+"tests/data/acpi/pc/DSDT.cphp",
+"tests/data/acpi/pc/DSDT.memhp",
+"tests/data/acpi/pc/DSDT.numamem",
+"tests/data/acpi/pc/DSDT.dimmpxm",
+"tests/data/acpi/pc/DSDT.acpihmat",
+"tests/data/acpi/pc/DSDT.acpihmat",
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v6 06/11] x86: ich9: expose "smi_negotiated_features" as a QOM property
  2020-09-23  9:46 [PATCH v6 00/11] x86: fix cpu hotplug with secure boot Igor Mammedov
                   ` (4 preceding siblings ...)
  2020-09-23  9:46 ` [PATCH v6 05/11] tests: acpi: mark to be changed tables in bios-tables-test-allowed-diff Igor Mammedov
@ 2020-09-23  9:46 ` Igor Mammedov
  2020-09-23  9:46 ` [PATCH v6 07/11] x86: acpi: introduce AcpiPmInfo::smi_on_cpuhp Igor Mammedov
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Igor Mammedov @ 2020-09-23  9:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: lersek, ankur.a.arora, mst

Expose the "smi_negotiated_features" field of ICH9LPCState as
a QOM property.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>
---
 include/hw/i386/ich9.h | 2 ++
 hw/isa/lpc_ich9.c      | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h
index 703d56036a..294024be5f 100644
--- a/include/hw/i386/ich9.h
+++ b/include/hw/i386/ich9.h
@@ -245,6 +245,8 @@ struct ICH9LPCState {
 #define ICH9_SMB_HST_D1                         0x06
 #define ICH9_SMB_HOST_BLOCK_DB                  0x07
 
+#define ICH9_LPC_SMI_NEGOTIATED_FEAT_PROP "x-smi-negotiated-features"
+
 /* 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
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index a54b3ec8bd..04e5323140 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -647,6 +647,9 @@ static void ich9_lpc_initfn(Object *obj)
                                   &acpi_enable_cmd, OBJ_PROP_FLAG_READ);
     object_property_add_uint8_ptr(OBJECT(lpc), ACPI_PM_PROP_ACPI_DISABLE_CMD,
                                   &acpi_disable_cmd, OBJ_PROP_FLAG_READ);
+    object_property_add_uint64_ptr(obj, ICH9_LPC_SMI_NEGOTIATED_FEAT_PROP,
+                                   &lpc->smi_negotiated_features,
+                                   OBJ_PROP_FLAG_READ);
 
     ich9_pm_add_properties(obj, &lpc->pm);
 }
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v6 07/11] x86: acpi: introduce AcpiPmInfo::smi_on_cpuhp
  2020-09-23  9:46 [PATCH v6 00/11] x86: fix cpu hotplug with secure boot Igor Mammedov
                   ` (5 preceding siblings ...)
  2020-09-23  9:46 ` [PATCH v6 06/11] x86: ich9: expose "smi_negotiated_features" as a QOM property Igor Mammedov
@ 2020-09-23  9:46 ` Igor Mammedov
  2020-09-23  9:46 ` [PATCH v6 08/11] x86: acpi: introduce the PCI0.SMI0 ACPI device Igor Mammedov
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Igor Mammedov @ 2020-09-23  9:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: lersek, ankur.a.arora, mst

Translate the "CPU hotplug with SMI" feature bit, from the property
added in the last patch, to a dedicated boolean in AcpiPmInfo.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>
---
 hw/i386/acpi-build.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 0e0535d2e3..b408d62560 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -95,6 +95,7 @@ typedef struct AcpiPmInfo {
     bool s3_disabled;
     bool s4_disabled;
     bool pcihp_bridge_en;
+    bool smi_on_cpuhp;
     uint8_t s4_val;
     AcpiFadtData fadt;
     uint16_t cpu_hp_io_base;
@@ -194,6 +195,7 @@ static void acpi_get_pm_info(MachineState *machine, AcpiPmInfo *pm)
     pm->cpu_hp_io_base = 0;
     pm->pcihp_io_base = 0;
     pm->pcihp_io_len = 0;
+    pm->smi_on_cpuhp = false;
 
     assert(obj);
     init_common_fadt_data(machine, obj, &pm->fadt);
@@ -207,12 +209,16 @@ static void acpi_get_pm_info(MachineState *machine, AcpiPmInfo *pm)
             object_property_get_uint(obj, ACPI_PCIHP_IO_LEN_PROP, NULL);
     }
     if (lpc) {
+        uint64_t smi_features = object_property_get_uint(lpc,
+            ICH9_LPC_SMI_NEGOTIATED_FEAT_PROP, NULL);
         struct AcpiGenericAddress r = { .space_id = AML_AS_SYSTEM_IO,
             .bit_width = 8, .address = ICH9_RST_CNT_IOPORT };
         pm->fadt.reset_reg = r;
         pm->fadt.reset_val = 0xf;
         pm->fadt.flags |= 1 << ACPI_FADT_F_RESET_REG_SUP;
         pm->cpu_hp_io_base = ICH9_CPU_HOTPLUG_IO_BASE;
+        pm->smi_on_cpuhp =
+            !!(smi_features & BIT_ULL(ICH9_LPC_SMI_F_CPU_HOTPLUG_BIT));
     }
 
     /* The above need not be conditional on machine type because the reset port
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v6 08/11] x86: acpi: introduce the PCI0.SMI0 ACPI device
  2020-09-23  9:46 [PATCH v6 00/11] x86: fix cpu hotplug with secure boot Igor Mammedov
                   ` (6 preceding siblings ...)
  2020-09-23  9:46 ` [PATCH v6 07/11] x86: acpi: introduce AcpiPmInfo::smi_on_cpuhp Igor Mammedov
@ 2020-09-23  9:46 ` Igor Mammedov
  2020-09-23  9:46 ` [PATCH v6 09/11] x68: acpi: trigger SMI before sending hotplug Notify event to OSPM Igor Mammedov
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Igor Mammedov @ 2020-09-23  9:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: lersek, ankur.a.arora, mst

When CPU hotplug with SMI has been negotiated, describe the SMI
register block in the DSDT. Pass the ACPI name of the SMI control
register to build_cpus_aml(), so that CPU_SCAN_METHOD can access the
register in the next patch.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>
---
 include/hw/acpi/cpu.h |  1 +
 hw/i386/acpi-build.c  | 29 ++++++++++++++++++++++++++++-
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/include/hw/acpi/cpu.h b/include/hw/acpi/cpu.h
index 62f0278ba2..0eeedaa491 100644
--- a/include/hw/acpi/cpu.h
+++ b/include/hw/acpi/cpu.h
@@ -50,6 +50,7 @@ void cpu_hotplug_hw_init(MemoryRegion *as, Object *owner,
 typedef struct CPUHotplugFeatures {
     bool acpi_1_compatible;
     bool has_legacy_cphp;
+    const char *smi_path;
 } CPUHotplugFeatures;
 
 void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index b408d62560..b49d360ab2 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1521,6 +1521,32 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
         aml_append(dev, aml_name_decl("_UID", aml_int(0)));
         aml_append(dev, build_q35_osc_method());
         aml_append(sb_scope, dev);
+
+        if (pm->smi_on_cpuhp) {
+            /* reserve SMI block resources, IO ports 0xB2, 0xB3 */
+            dev = aml_device("PCI0.SMI0");
+            aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A06")));
+            aml_append(dev, aml_name_decl("_UID", aml_string("SMI resources")));
+            crs = aml_resource_template();
+            aml_append(crs,
+                aml_io(
+                       AML_DECODE16,
+                       ACPI_PORT_SMI_CMD,
+                       ACPI_PORT_SMI_CMD,
+                       1,
+                       2)
+            );
+            aml_append(dev, aml_name_decl("_CRS", crs));
+            aml_append(dev, aml_operation_region("SMIR", AML_SYSTEM_IO,
+                aml_int(ACPI_PORT_SMI_CMD), 2));
+            field = aml_field("SMIR", AML_BYTE_ACC, AML_NOLOCK,
+                              AML_WRITE_AS_ZEROS);
+            aml_append(field, aml_named_field("SMIC", 8));
+            aml_append(field, aml_reserved_field(8));
+            aml_append(dev, field);
+            aml_append(sb_scope, dev);
+        }
+
         aml_append(dsdt, sb_scope);
 
         build_hpet_aml(dsdt);
@@ -1536,7 +1562,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
         build_legacy_cpu_hotplug_aml(dsdt, machine, pm->cpu_hp_io_base);
     } else {
         CPUHotplugFeatures opts = {
-            .acpi_1_compatible = true, .has_legacy_cphp = true
+            .acpi_1_compatible = true, .has_legacy_cphp = true,
+            .smi_path = pm->smi_on_cpuhp ? "\\_SB.PCI0.SMI0.SMIC" : NULL,
         };
         build_cpus_aml(dsdt, machine, opts, pm->cpu_hp_io_base,
                        "\\_SB.PCI0", "\\_GPE._E02");
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v6 09/11] x68: acpi: trigger SMI before sending hotplug Notify event to OSPM
  2020-09-23  9:46 [PATCH v6 00/11] x86: fix cpu hotplug with secure boot Igor Mammedov
                   ` (7 preceding siblings ...)
  2020-09-23  9:46 ` [PATCH v6 08/11] x86: acpi: introduce the PCI0.SMI0 ACPI device Igor Mammedov
@ 2020-09-23  9:46 ` Igor Mammedov
  2020-09-23  9:46 ` [PATCH v6 10/11] tests: acpi: update acpi blobs with new AML Igor Mammedov
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Igor Mammedov @ 2020-09-23  9:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: lersek, ankur.a.arora, mst

In case firmware has negotiated CPU hotplug SMI feature, generate
AML to describe SMI IO port region and send SMI to firmware
on each CPU hotplug SCI in case new CPUs were hotplugged.

Since new CPUs can be hotplugged while CPU_SCAN_METHOD is running
we can't send SMI before new CPUs are fetched from QEMU as it
could cause sending Notify to a CPU that firmware hasn't seen yet.
So fetch new CPUs into local cache first, then send SMI and
after that send Notify events to cached CPUs. This should ensure
that Notify is sent only to CPUs which were processed by firmware
first.
Any CPUs that were hotplugged after caching will be processed
by the next CPU_SCAN_METHOD, when pending SCI is handled.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>
---
v6:
 - Add comment explaining why while_ctx2 restarts from the last processed CPU.
v5:
 - fix hotplug on Windows when there is more than 256 possible CPUs
   (Windows isn't able to handle VarPackage over 255 elements
    so process CPUs in batches)
 - (Laszlo Ersek <lersek@redhat.com>)
   - fix off-by-one in package length
   - fix not selecting CPU before clearing insert event
   - use aml_lgreater() instead of aml_lnot(aml_equal(num_added_cpus, zero)
   - split off in separate patches:
     - making smi_negotiated_features a property
     - introduction of AcpiPmInfo.smi_on_cpuhp
     - introduction of PCI0.SMI0 ACPI device
v2:
 - clear insert event after firmware has returned
   control from SMI. (Laszlo Ersek <lersek@redhat.com>)
v1:
 - make sure that Notify is sent only to CPUs seen by FW
 - (Laszlo Ersek <lersek@redhat.com>)
    - use macro instead of x-smi-negotiated-features
    - style fixes
    - reserve whole SMI block (0xB2, 0xB3)
v0:
 - s/aml_string/aml_eisaid/
   /fixes Windows BSOD, on nonsense value/ (Laszlo Ersek <lersek@redhat.com>)
 - put SMI device under PCI0 like the rest of hotplug IO port
 - do not generate SMI AML if CPU hotplug SMI feature hasn't been negotiated
---
 hw/acpi/cpu.c | 165 +++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 138 insertions(+), 27 deletions(-)

diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
index 3d6a500fb7..8dd4d8ebbf 100644
--- a/hw/acpi/cpu.c
+++ b/hw/acpi/cpu.c
@@ -14,6 +14,8 @@
 #define ACPI_CPU_CMD_DATA_OFFSET_RW 8
 #define ACPI_CPU_CMD_DATA2_OFFSET_R 0
 
+#define OVMF_CPUHP_SMI_CMD 4
+
 enum {
     CPHP_GET_NEXT_CPU_WITH_EVENT_CMD = 0,
     CPHP_OST_EVENT_CMD = 1,
@@ -321,6 +323,7 @@ const VMStateDescription vmstate_cpu_hotplug = {
 #define CPU_NOTIFY_METHOD "CTFY"
 #define CPU_EJECT_METHOD  "CEJ0"
 #define CPU_OST_METHOD    "COST"
+#define CPU_ADDED_LIST    "CNEW"
 
 #define CPU_ENABLED       "CPEN"
 #define CPU_SELECTOR      "CSEL"
@@ -465,42 +468,150 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
 
         method = aml_method(CPU_SCAN_METHOD, 0, AML_SERIALIZED);
         {
+            const uint8_t max_cpus_per_pass = 255;
             Aml *else_ctx;
-            Aml *while_ctx;
+            Aml *while_ctx, *while_ctx2;
             Aml *has_event = aml_local(0);
             Aml *dev_chk = aml_int(1);
             Aml *eject_req = aml_int(3);
             Aml *next_cpu_cmd = aml_int(CPHP_GET_NEXT_CPU_WITH_EVENT_CMD);
+            Aml *num_added_cpus = aml_local(1);
+            Aml *cpu_idx = aml_local(2);
+            Aml *uid = aml_local(3);
+            Aml *has_job = aml_local(4);
+            Aml *new_cpus = aml_name(CPU_ADDED_LIST);
 
             aml_append(method, aml_acquire(ctrl_lock, 0xFFFF));
-            aml_append(method, aml_store(one, has_event));
-            while_ctx = aml_while(aml_equal(has_event, one));
+
+            /*
+             * Windows versions newer than XP (including Windows 10/Windows
+             * Server 2019), do support* VarPackageOp but, it is cripled to hold
+             * the same elements number as old PackageOp.
+             * For compatibility with Windows XP (so it won't crash) use ACPI1.0
+             * PackageOp which can hold max 255 elements.
+             *
+             * use named package as old Windows don't support it in local var
+             */
+            aml_append(method, aml_name_decl(CPU_ADDED_LIST,
+                                             aml_package(max_cpus_per_pass)));
+
+            aml_append(method, aml_store(zero, uid));
+            aml_append(method, aml_store(one, has_job));
+            /*
+             * CPU_ADDED_LIST can hold limited number of elements, outer loop
+             * allows to process CPUs in batches which let us to handle more
+             * CPUs than CPU_ADDED_LIST can hold.
+             */
+            while_ctx2 = aml_while(aml_equal(has_job, one));
             {
-                 /* clear loop exit condition, ins_evt/rm_evt checks
-                  * will set it to 1 while next_cpu_cmd returns a CPU
-                  * with events */
-                 aml_append(while_ctx, aml_store(zero, has_event));
-                 aml_append(while_ctx, aml_store(next_cpu_cmd, cpu_cmd));
-                 ifctx = aml_if(aml_equal(ins_evt, one));
-                 {
-                     aml_append(ifctx,
-                         aml_call2(CPU_NOTIFY_METHOD, cpu_data, dev_chk));
-                     aml_append(ifctx, aml_store(one, ins_evt));
-                     aml_append(ifctx, aml_store(one, has_event));
-                 }
-                 aml_append(while_ctx, ifctx);
-                 else_ctx = aml_else();
-                 ifctx = aml_if(aml_equal(rm_evt, one));
-                 {
-                     aml_append(ifctx,
-                         aml_call2(CPU_NOTIFY_METHOD, cpu_data, eject_req));
-                     aml_append(ifctx, aml_store(one, rm_evt));
-                     aml_append(ifctx, aml_store(one, has_event));
-                 }
-                 aml_append(else_ctx, ifctx);
-                 aml_append(while_ctx, else_ctx);
+                aml_append(while_ctx2, aml_store(zero, has_job));
+
+                aml_append(while_ctx2, aml_store(one, has_event));
+                aml_append(while_ctx2, aml_store(zero, num_added_cpus));
+
+                /*
+                 * Scan CPUs, till there are CPUs with events or
+                 * CPU_ADDED_LIST capacity is exhausted
+                 */
+                while_ctx = aml_while(aml_land(aml_equal(has_event, one),
+                                      aml_lless(uid, aml_int(arch_ids->len))));
+                {
+                     /*
+                      * clear loop exit condition, ins_evt/rm_evt checks will
+                      * set it to 1 while next_cpu_cmd returns a CPU with events
+                      */
+                     aml_append(while_ctx, aml_store(zero, has_event));
+
+                     aml_append(while_ctx, aml_store(uid, cpu_selector));
+                     aml_append(while_ctx, aml_store(next_cpu_cmd, cpu_cmd));
+
+                     /*
+                      * wrap around case, scan is complete, exit loop.
+                      * It happens since events are not cleared in scan loop,
+                      * so next_cpu_cmd continues to find already processed CPUs
+                      */
+                     ifctx = aml_if(aml_lless(cpu_data, uid));
+                     {
+                         aml_append(ifctx, aml_break());
+                     }
+                     aml_append(while_ctx, ifctx);
+
+                     /*
+                      * if CPU_ADDED_LIST is full, exit inner loop and process
+                      * collected CPUs
+                      */
+                     ifctx = aml_if(
+                         aml_equal(num_added_cpus, aml_int(max_cpus_per_pass)));
+                     {
+                         aml_append(ifctx, aml_store(one, has_job));
+                         aml_append(ifctx, aml_break());
+                     }
+                     aml_append(while_ctx, ifctx);
+
+                     aml_append(while_ctx, aml_store(cpu_data, uid));
+                     ifctx = aml_if(aml_equal(ins_evt, one));
+                     {
+                         /* cache added CPUs to Notify/Wakeup later */
+                         aml_append(ifctx, aml_store(uid,
+                             aml_index(new_cpus, num_added_cpus)));
+                         aml_append(ifctx, aml_increment(num_added_cpus));
+                         aml_append(ifctx, aml_store(one, has_event));
+                     }
+                     aml_append(while_ctx, ifctx);
+                     else_ctx = aml_else();
+                     ifctx = aml_if(aml_equal(rm_evt, one));
+                     {
+                         aml_append(ifctx,
+                             aml_call2(CPU_NOTIFY_METHOD, uid, eject_req));
+                         aml_append(ifctx, aml_store(one, rm_evt));
+                         aml_append(ifctx, aml_store(one, has_event));
+                     }
+                     aml_append(else_ctx, ifctx);
+                     aml_append(while_ctx, else_ctx);
+                     aml_append(while_ctx, aml_increment(uid));
+                }
+                aml_append(while_ctx2, while_ctx);
+
+                /*
+                 * in case FW negotiated ICH9_LPC_SMI_F_CPU_HOTPLUG_BIT,
+                 * make upcall to FW, so it can pull in new CPUs before
+                 * OS is notified and wakes them up
+                 */
+                if (opts.smi_path) {
+                    ifctx = aml_if(aml_lgreater(num_added_cpus, zero));
+                    {
+                        aml_append(ifctx, aml_store(aml_int(OVMF_CPUHP_SMI_CMD),
+                            aml_name("%s", opts.smi_path)));
+                    }
+                    aml_append(while_ctx2, ifctx);
+                }
+
+                /* Notify OSPM about new CPUs and clear insert events */
+                aml_append(while_ctx2, aml_store(zero, cpu_idx));
+                while_ctx = aml_while(aml_lless(cpu_idx, num_added_cpus));
+                {
+                    aml_append(while_ctx,
+                        aml_store(aml_derefof(aml_index(new_cpus, cpu_idx)),
+                                  uid));
+                    aml_append(while_ctx,
+                        aml_call2(CPU_NOTIFY_METHOD, uid, dev_chk));
+                    aml_append(while_ctx, aml_store(uid, aml_debug()));
+                    aml_append(while_ctx, aml_store(uid, cpu_selector));
+                    aml_append(while_ctx, aml_store(one, ins_evt));
+                    aml_append(while_ctx, aml_increment(cpu_idx));
+                }
+                aml_append(while_ctx2, while_ctx);
+                /*
+                 * If another batch is needed, then it will resume scanning
+                 * exactly at -- and not after -- the last CPU that's currently
+                 * in CPU_ADDED_LIST. In other words, the last CPU in
+                 * CPU_ADDED_LIST is going to be re-checked. That's OK: we've
+                 * just cleared the insert event for *all* CPUs in
+                 * CPU_ADDED_LIST, including the last one. So the scan will
+                 * simply seek past it.
+                 */
             }
-            aml_append(method, while_ctx);
+            aml_append(method, while_ctx2);
             aml_append(method, aml_release(ctrl_lock));
         }
         aml_append(cpus_dev, method);
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v6 10/11] tests: acpi: update acpi blobs with new AML
  2020-09-23  9:46 [PATCH v6 00/11] x86: fix cpu hotplug with secure boot Igor Mammedov
                   ` (8 preceding siblings ...)
  2020-09-23  9:46 ` [PATCH v6 09/11] x68: acpi: trigger SMI before sending hotplug Notify event to OSPM Igor Mammedov
@ 2020-09-23  9:46 ` Igor Mammedov
  2020-09-23  9:46 ` [PATCH v6 11/11] smp: drop support for deprecated (invalid topologies) Igor Mammedov
  2020-09-23 16:44 ` [PATCH v6 00/11] x86: fix cpu hotplug with secure boot Laszlo Ersek
  11 siblings, 0 replies; 16+ messages in thread
From: Igor Mammedov @ 2020-09-23  9:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: lersek, ankur.a.arora, mst

here is diff against tests/data/acpi/q35/DSDT
for currently shipped ovmf binary.
(once firmware blob is updated, it will negotiate CPU hotplug
feature which will ad extra hunk sending SMI and Q35 tests will
need to be updated), but otherwise diff shows new CPU hotplug
AML that is shared between q35 and pc machines.

             Method (CSCN, 0, Serialized)
             {
                 Acquire (\_SB.PCI0.PRES.CPLK, 0xFFFF)
-                Local0 = One
-                While ((Local0 == One))
-                {
-                    Local0 = Zero
-                    \_SB.PCI0.PRES.CCMD = Zero
-                    If ((\_SB.PCI0.PRES.CINS == One))
+                Name (CNEW, Package (0xFF){})
+                Local3 = Zero
+                Local4 = One
+                While ((Local4 == One))
+                {
+                    Local4 = Zero
+                    Local0 = One
+                    Local1 = Zero
+                    While (((Local0 == One) && (Local3 < One)))
                     {
-                        CTFY (\_SB.PCI0.PRES.CDAT, One)
-                        \_SB.PCI0.PRES.CINS = One
-                        Local0 = One
+                        Local0 = Zero
+                        \_SB.PCI0.PRES.CSEL = Local3
+                        \_SB.PCI0.PRES.CCMD = Zero
+                        If ((\_SB.PCI0.PRES.CDAT < Local3))
+                        {
+                            Break
+                        }
+
+                        If ((Local1 == 0xFF))
+                        {
+                            Local4 = One
+                            Break
+                        }
+
+                        Local3 = \_SB.PCI0.PRES.CDAT
+                        If ((\_SB.PCI0.PRES.CINS == One))
+                        {
+                            CNEW [Local1] = Local3
+                            Local1++
+                            Local0 = One
+                        }
+                        ElseIf ((\_SB.PCI0.PRES.CRMV == One))
+                        {
+                            CTFY (Local3, 0x03)
+                            \_SB.PCI0.PRES.CRMV = One
+                            Local0 = One
+                        }
+
+                        Local3++
                     }
-                    ElseIf ((\_SB.PCI0.PRES.CRMV == One))
+
+                    Local2 = Zero
+                    While ((Local2 < Local1))
                     {
-                        CTFY (\_SB.PCI0.PRES.CDAT, 0x03)
-                        \_SB.PCI0.PRES.CRMV = One
-                        Local0 = One
+                        Local3 = DerefOf (CNEW [Local2])
+                        CTFY (Local3, One)
+                        Debug = Local3
+                        \_SB.PCI0.PRES.CSEL = Local3
+                        \_SB.PCI0.PRES.CINS = One
+                        Local2++
                     }
                 }

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 tests/qtest/bios-tables-test-allowed-diff.h |  19 -------------------
 tests/data/acpi/pc/DSDT                     | Bin 4934 -> 5060 bytes
 tests/data/acpi/pc/DSDT.acpihmat            | Bin 6258 -> 6385 bytes
 tests/data/acpi/pc/DSDT.bridge              | Bin 6793 -> 6919 bytes
 tests/data/acpi/pc/DSDT.cphp                | Bin 5397 -> 5524 bytes
 tests/data/acpi/pc/DSDT.dimmpxm             | Bin 6587 -> 6714 bytes
 tests/data/acpi/pc/DSDT.ipmikcs             | Bin 5006 -> 5132 bytes
 tests/data/acpi/pc/DSDT.memhp               | Bin 6293 -> 6419 bytes
 tests/data/acpi/pc/DSDT.numamem             | Bin 4940 -> 5066 bytes
 tests/data/acpi/q35/DSDT                    | Bin 7678 -> 7804 bytes
 tests/data/acpi/q35/DSDT.acpihmat           | Bin 9002 -> 9129 bytes
 tests/data/acpi/q35/DSDT.bridge             | Bin 7695 -> 7821 bytes
 tests/data/acpi/q35/DSDT.cphp               | Bin 8141 -> 8268 bytes
 tests/data/acpi/q35/DSDT.dimmpxm            | Bin 9331 -> 9458 bytes
 tests/data/acpi/q35/DSDT.ipmibt             | Bin 7753 -> 7879 bytes
 tests/data/acpi/q35/DSDT.memhp              | Bin 9037 -> 9163 bytes
 tests/data/acpi/q35/DSDT.mmio64             | Bin 8808 -> 8934 bytes
 tests/data/acpi/q35/DSDT.numamem            | Bin 7684 -> 7810 bytes
 tests/data/acpi/q35/DSDT.tis                | Bin 8283 -> 8409 bytes
 19 files changed, 19 deletions(-)

diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index dba32d5613..dfb8523c8b 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,20 +1 @@
 /* List of comma-separated changed AML files to ignore */
-"tests/data/acpi/pc/DSDT",
-"tests/data/acpi/q35/DSDT",
-"tests/data/acpi/q35/DSDT.tis",
-"tests/data/acpi/q35/DSDT.bridge",
-"tests/data/acpi/q35/DSDT.mmio64",
-"tests/data/acpi/q35/DSDT.ipmibt",
-"tests/data/acpi/q35/DSDT.cphp",
-"tests/data/acpi/q35/DSDT.memhp",
-"tests/data/acpi/q35/DSDT.numamem",
-"tests/data/acpi/q35/DSDT.dimmpxm",
-"tests/data/acpi/q35/DSDT.acpihmat",
-"tests/data/acpi/pc/DSDT.bridge",
-"tests/data/acpi/pc/DSDT.ipmikcs",
-"tests/data/acpi/pc/DSDT.cphp",
-"tests/data/acpi/pc/DSDT.memhp",
-"tests/data/acpi/pc/DSDT.numamem",
-"tests/data/acpi/pc/DSDT.dimmpxm",
-"tests/data/acpi/pc/DSDT.acpihmat",
-"tests/data/acpi/pc/DSDT.acpihmat",
diff --git a/tests/data/acpi/pc/DSDT b/tests/data/acpi/pc/DSDT
index b121bb5bc124be522e233516efb17cdc94de5a75..4ca46e5a2bdb1dfab79dd8630aeeb9a386d8b30e 100644
GIT binary patch
delta 323
zcmX@6c0`@aCD<k8h%f^K<C%?ITiBSq>?R*zTg~KaJUN&BFO!?t<Qxu}dJbnl*Ki@G
z{{;-m1&k?+T!bd4FcvVR6fh<fFeEPW;h8WwfpKawP$;1wIYyr)KG-Qfz}eFvAjmb?
zIoQ>wfB{|5+1F)(_*8UJ7srs~r3*MGCvyD<TC=nOCSMOT3#4Fy%4C>0OxV*en6V(a
z1LVj=hSEf!vlf~!FvU<7<QvB59O4$4%*9;5h;9Z@5~!~<xqu;Qkt@s8q{M<|uz^Vo
e$<Ev$O+Xu>4GWSd>u{*D0xc{}+HA|o$prw{_+k$L

delta 193
zcmX@2eoT$aCD<jzO_+g!@xVr|Eo@9aW|I%Ft!DDqo}A16m&t=~at?<KUjbvnB1i7Y
z35*2{2?Y$34Y`E<7Pzxaj?rg{4|a+VaP~9^2yzW}_Vf#8bPjQgL>G5)3}Gx_L>C3B
l2AaFjn`MCq%VZl)NnLaWKy6&inEHZz!+`oW@8;y>0szAaHLw5x

diff --git a/tests/data/acpi/pc/DSDT.acpihmat b/tests/data/acpi/pc/DSDT.acpihmat
index b0dbb943f4cea83a5adde23aefa54f1678c560a1..35a74bce8cc152ecb615cb38c4b7f63c7c7d3ab3 100644
GIT binary patch
delta 304
zcmexl@X?UVCD<k8qXYv3W9LS$Eo@AlE|U+it!DDE+?>nK!N}<@<{a$o#}TbOnUO=-
z>HmKYXFu0)A*TNY49NwIDT`c%CZ{kKFr*YPCKNCvF7oA>Fgby7YBCp70YgGTa*RGp
ze6Uk|fU~DTK#*&&bFiz=<oO(8_2N^}Wn3IXl9w*voSew@A872-0&MaNR3@Vfd-?@4
z79@9o+?2>rnh12oLh}Wt7|Mct!x)`I+#-{?m<t$V^e3BeiYS&Q7ce9(a%GvClvvOV
kHX(^28LS9sVzgmF^5i;BRaT${rAd?HIm9-{af<K*0E&TJnE(I)

delta 209
zcmexp_{o6FCD<jTNP>ZZv2P>S7B(g?o5=^*Rx|k;Zq8-rVC3}Ra}IX)<A_$C%*Y|E
z_WyqYW5Oaw?#T&^1q=xV3^DpF@xe~<0nVNV0YR?8&d$Cr3*1>ID{_iTIET1JqD#9t
vhA>Xvz$xkvG-aVT%K{IU$>>Uge8U)VXysxqV8o&vXx8L-4zbO5IYsyZgN-`x

diff --git a/tests/data/acpi/pc/DSDT.bridge b/tests/data/acpi/pc/DSDT.bridge
index 7b6c7a47875fc73b03fbe88807890f3867ddba1a..803d7a8e839ea8b7ac33c4490459ddaede584269 100644
GIT binary patch
delta 323
zcmeA)Z8zg`33dr#mu6sK6xqnNg^kI}Zt?-P)l9y|lXKbsGP#LO&f$=$=WzCO4Hshi
zU%-%Dz?ibgMQCygV*x`-0b@b|L*gPIo(Yo^7^fx!g%S#qWAs_#gPr07oIMQ!f?R{0
zgI#?J7|;cseO(raPem7XaSTabx`1<XBG-SQHA@R%^7SyYKnfP9OooZWggyO&84Hp-
zK#ojgC`|-9YoYl9Qw(K6zF~~cA#Rb$T+9WG=w<*Vf%;053mB3Xxw1@6N-Ssw8<@n9
d?92_)1hgUAupoJ|4u>i$(8AKB&9<CAf&i#*V2}U+

delta 193
zcmZoS>onzZ33dtTlwx3D<lM-$g^kI_Z1MrN)lB}{lXKbsGI{V#&f$>ZD_~4m<j6fa
zfw6!gp@3nsA(yb<0(X|lG5Rd=!A|i3&YlJVL9W5ho_@iM&LM7*=;AJpA&do#=%PT?
kKyw#*vn=pnnQX%;sf(@vsEvymQ(us87*OBl-JCvx05hyLF#rGn

diff --git a/tests/data/acpi/pc/DSDT.cphp b/tests/data/acpi/pc/DSDT.cphp
index c0e8aa5b32d84f39e5d6c9a5024505f818707c12..8bab2f506409f2b025a63d8b91c7bfdaa931e626 100644
GIT binary patch
delta 251
zcmbQLHAS1tCD<ioiYNmEqrpb5Eo@9)(UT9bt!DBK*qqC5$;j?5<{a$o$1&NTLtK}`
z+0Qjxi0OX;LvjIQ$|6^x$tjEl3@HVS2?Y#^i+p(|Oio~&n#{#kz>rXoJo!Jn=;TWr
zB23~_Co^&i^CmA{z&SaQ>pxKE(gKi(OY#Dh$uasY@xe~<0nVNV0YR?8&Yph3j0MRZ
zAd3<iN)v%bFEn3ZilHpXH;mCa#4R#;ay+M$Vrg;#L((EwmZ?dJ1<hc+NeszgX`orr
ZhCsKh=Tv0{8eN(+c>;&n=6=pLZUB`uOrrn*

delta 220
zcmbQDJyna#CD<iIRFr{%(Q+f#7B(jD;K>KrRx|l|Y|dr3WMuc?a}IX)<CyHvArZis
zu*i{nasp!kLqY*Vj6O?zuv2`1v!_8okZZ8Bv#-kncb3WM!k&J?jLso$k?7(sjv<Vb
sA99NN7ceF)^k!M$fvGjfHw>3nF6IJ8bQ6Kffo4sfz#+DokE@Lv0MS-FWdHyG

diff --git a/tests/data/acpi/pc/DSDT.dimmpxm b/tests/data/acpi/pc/DSDT.dimmpxm
index 1649953b6cccb933e4a440dc56507dc9197c4a8a..e015b4594c96a6e0f34c0668e3383b9a91dff38e 100644
GIT binary patch
delta 308
zcmdmOyvu~kCD<jzN{WGjQF0^K7B(j5fXN5gRx`P|Zq8*_U}SO^o7~SKQ_tb-=Nc}=
z^uK^1xqvZck*m<;6vhIElmf<t0*1szzC05qCooP;=3*&eNGM2-(PxPdc8U*h_B03x
zat(G4cJ(P>Ko@lOby*-j6<yTDF(i5E0?x^aT>pXgEG>Y_^TW*oDOjK~`8<bcdO>mr
z$W@69rHMdyEHqzWIvL%#Am1=X=McBZWG?0cMl6y*eWl3-3`vVzS*9i>7BqtmOkzj|
c>tQTNjy5bvp6t(|%IfJCT$;2wfiqeF0OOlnivR!s

delta 210
zcmdmGvfG%;CD<iow<H4tqvA%cEo@9K9+MBSt!8q!-JHv=z{upmH@TlfroMnNVUZ*E
z<OIe7hJ*r!7=4!bV5j&1XHSEGAlG1LXJ3~E?ktnhg+2X(8J$DiBGJWN977lj7|}(6
os)4pF^k!M$fnn6-0!~hGbPYgbxR?tVCns_W1F3|~_c@~l0B9gOg#Z8m

diff --git a/tests/data/acpi/pc/DSDT.ipmikcs b/tests/data/acpi/pc/DSDT.ipmikcs
index 92748d49dcd418e4a734da47e8d5c0268aedfc29..ca6630e39f60ebd5c056f57c4c03fdb9d5467577 100644
GIT binary patch
delta 304
zcmeBE@6q6L33dtL5n*6pjNi!hhmFb0ZZZe^Y9?Rf$y?d~*1L&02Rr+5L@USWv&08G
z#RoWh8UzHn20I7%c>n*;;q2!cF2wY|fFZenF=dg9(Bu@x0)~_V#)JZf#6><l6DB7x
zPE7_1B@`s18xZX3Q^0^O=<MsVKzu4fG#Fx;i(^Rg(gmE86S@8a?O9rYP=JtMpfVX<
z*wZhVu^_nv<j6#Z(nO%M7Md?Goovr3$(TGjn^Q`$G`WBwX^|_-)TG3MX0W;>hGgdu
cw@9FylcNm_k|*!xRAmJkQ<}87m6MYT0IdjJEdT%j

delta 202
zcmeCt=u_u%33dtT6J}sw4B5!_hmFa{Y%&M?Y9@c}$y?d~vU~732Rr+5Ojh8M;ATu%
z<j6fafw6!gVKP6bSiL*T<QRRH_+Y2_0B28wfFRdkXHUOiM&}T>NOW-*#}LK>Ms!i2
n>H@}uh2AU+JTQ$4@(sgf6c;ndtjS9_MJ6xg65TAp^@S4v*7-NU

diff --git a/tests/data/acpi/pc/DSDT.memhp b/tests/data/acpi/pc/DSDT.memhp
index 4026772906e910af514beb76de6e4cca0bc2171b..43f4e114e2cc48c68c35af47303fa87c9255db40 100644
GIT binary patch
delta 323
zcmbPgIN6BHCD<iISdxK(aqmX1Eo@8<_LC2=t!8pInVif1m&r|Rat?<~J%_WOYq${8
z{{n{O0>+d@E<%%27z-Fu3K$a#7!nux@JyJTz&JG-D3nl;9HY+?AM6w#;OuD-5ab%{
z9PH{-z<@63?CY{Xd@8!Ai(^Rg(gmE86S@8atyx+Cldp%F1yZm;Wim`0ChX}K%vg}z
z0diy_Lun$=Sqse<m|`dk@(p8j4snZ2=3*{jL^lH{3Dj4bT)>dD$dzSkQer_f*uW%)
eWM^)WCZG+`h6Tx!bvRU6ffkk~ZMNl%;0FMI?qN#+

delta 193
zcmbPiG}VyHCD<iosssZA<ED*VTiBSK%qJgUTg~LEGdY+2FOvt~<Qxtez5>RCMULE)
z6Br8^5(*e58*&NzEpTU<9HY+?AM6w#;OuD-5ab%{?CBTG=p5n}i7xKq7{XY<h%O3L
l4K#P5H_HMKmdQ4plDg;$fZDj2G4%!ch5_|$-pv`o4*;<`HS7QY

diff --git a/tests/data/acpi/pc/DSDT.numamem b/tests/data/acpi/pc/DSDT.numamem
index 4d9ba337a82954af094e739b8a83467f89a37cc0..ba8f7e6c33f9eb0f7a080144fcb4a27d36aa04ae 100644
GIT binary patch
delta 323
zcmX@3c1oSgCD<k8lrRGWW7$ToEo@8<_LC2=t!8pInVif1m&r|Rat?<~J%_WOYq${8
z{{n{O0>+d@E<%%27z-Fu3K$a#7!nux@JyJTz&JG-D3nl;9HY+?AM6w#;OuD-5ab%{
z9PH{-z<@63?CY{Xd@8!Ai(^Rg(gmE86S@8atyx+Cldp%F1yZm;Wim`0ChX}K%vg}z
z0diy_Lun$=Sqse<m|`dk@(p8j4snZ2=3*{jL^lH{3Dj4bT)>dD$dzSkQer_f*uW%)
eWM^)WCZG+`h6Tx!bvRU6ffkk~ZMNm)=K=t6nPI;G

delta 193
zcmX@5enySUCD<jzN0@<uF>52&7B(g)^T`L;Rx`QkOwMKh%jCf~Ifp}ruYfUOkt6ry
z1jYh}gaU@ihFrpa3*1>I$LO=f2Rp?FIC~ld1i1z~d-?@4I)}JLqKmsYhA<W|qKg7m
l1I=CN&9cCQWwH&Yq%OJwpf)aMOnpJVVL*MGcXRS{0RV_rHDCY$

diff --git a/tests/data/acpi/q35/DSDT b/tests/data/acpi/q35/DSDT
index bba8884073a27427b88ac0d733c9c87330a59366..d6c26940b1a68d4184f6f2335924099aa28b130a 100644
GIT binary patch
delta 249
zcmexo{l|vOCD<jTMvj4jar;KD2@*_Rc9Z8wtY-2xp6n_4m)%XwIoR2cWAb}RaUBk4
zKi6;}rvC*D$pwrli(G^zr!W>Uq!chF6fh(%^5K~<Ie~F%GEgX?AbIk9NzutgQX)*^
zQztK!66Q@_x`1<XBG-SQ#-#-y5trlzDwAXMS>l78;scyL4FZB(gPlG7f*A{vJ3#g%
zGL$9)O<riez!XDSkZ%~HbBJ4H^5p+gQi`R?1q?}xTv?_jB^ES;^(HYSgQbCHMH>R$
W;xDbr3N*SjX|jQo*k*m{^GpEqI!~+s

delta 203
zcmexk^Us>gCD<k8pDY6d<C=|J6C{{?%qGu~Sk2_GJ=s(8FS`eybFi}?$K>~t63UDT
ziyXNpComQ;Bor{j=(EHJJH-b$dm01;xduBs`?@S}XPNv_N>su*#4QqC+Ql)1adLyS
rsDA-t!a{GB1s*Ju(Uk=GhB4yM%Ees3h($TjtjPvaVw=}WpJxI9Z#6rr

diff --git a/tests/data/acpi/q35/DSDT.acpihmat b/tests/data/acpi/q35/DSDT.acpihmat
index 9cac92418b5fcc2767dc74603d599642b59623fe..2b67045d6add69574499b97d938cca01949231b4 100644
GIT binary patch
delta 288
zcmZ4Gw$h!;CD<ior7{BpW64IY2@*`6E|cd-tY-4D-0UgI!N~3|<{a$o$1!=Lq<B4t
zv!83Y5YztxhU5aqltr#WlT#QA7*Yxt6ABm-7y0r`n4G{kHJOX4fFYqEIYyr)KG-Qf
zz}eFvAjmb?IoQ>wfB{|5+1F)(_*8UJ7srs~r3*MGCvyD<+OxC(DsKii3#4Fy%4BqX
zo_@iM1<4&CS0*x)CIa2H(0qaE<a<(*jLDO^q@@%~lM5J<7P+!aO-d|i2CGYANOlf!
bi%bUUi#9Aso*X5u$_g~5G-<PnG#dv1HTG7>

delta 203
zcmZ4KzRHcuCD<iIOPPUzv2r8V1PLZDo5^z|Rx|k;ZuXSqU}X2;a}IX)<Cr{AQi7W?
zVUZ*E<OIe7hJ?xMrNrvpStiHmv&08G#RoWh8UzHn20MHD1v5H_xJ9CiyEuk07BHfV
o0#z3<CM@)3S>S<bRFH2NE~B`ZL1s;MkrtWkC?mRghxA8w05}9VZU6uP

diff --git a/tests/data/acpi/q35/DSDT.bridge b/tests/data/acpi/q35/DSDT.bridge
index f08b7245f59aad491fcaa60e2bab1085c369ea1c..0f6c9c68c81cddd2126eea8a7c336b8667202223 100644
GIT binary patch
delta 249
zcmeCT>9yr@33dtTm1AIF4ByB#L4wK4Zt@(7)l9y|lRYK>vb%{n2Rr+5OnxsZuEXK%
z=Nc}=^uK^1xqvZck&Dpe6vhIElmf<t0*1szK0Fg9CooP;1_~t<Bu}0%DLT1GN`y&#
z>g0t|!o0~#7jRBa<oXZPxU>Ky;*z{TWpa!@OMI|Xe1Nm3K|qjeu(PLMFk?Y-2gsg8
zhSEf!$qUUFm|`dk@(p8j4snZ2p8Q`*O0hJ#fFWs-E6dcR#DZq9-Xw-(ur$!DXhWb|
W{H0Y{fku}mO*W7c+pI7BfC&KA`%aPo

delta 203
zcmeCR?YH4_33dtLmt$aH^xnudL4wK0Z1Nn5)lB}{lRYK>vU~732Rr+5OnxsZq0E@D
z$dP+;0%HL~LIFdJK1+PCQ+$B4r$IoFYp}Dkugd~=mdPKbL?xU<+#=DXT^vIgCpSoo
q`WG-JEc9ks;K4E(T}hB{7$Xj?T+9WGSd;_Jnrt8?wt21e1112)r8$rQ

diff --git a/tests/data/acpi/q35/DSDT.cphp b/tests/data/acpi/q35/DSDT.cphp
index 57d859cef9fa16a8f125c4b338611c8472699f38..d7bedee7ff638f11b3bb84ef960364b409a49cce 100644
GIT binary patch
delta 306
zcmX?Wf5w5!CD<jzM}dKXQF<fS1PLau=*e>=Rx|kqZ1$A2WUO}=a}IX)<A_#{(PxPd
zc8U*h_B03xat(G4@bUitpTpVDHC%}4e*r^s0b|M{SE0!%j0FrS1&j#=42g?;c_vIw
zV4RxF#a6(OP>_snLa?h(0Ry_Av#-kn@u_gp5U6b~jv>iQ7jRBa<oXY^XlVgl0h0Uz
zmC5MBo_@iM1<4&CS0*x)CIa2H(0qaEWPWK$#^lM?(o%}0$ps8ai(FZzCM6a$gViN5
fBs+(=MJ5CFMH?0*PwtjhWd#~jnzT7ox`G7&GO=B7

delta 199
zcmX@(aMqs7CD<k8tULn)qv}Sk2@*`+!IS4mtY-4_*z753$;j@(=N#<p$1(Y%v;;R}
z!XiiR$q9@F3<;ATNQu?EvrLZBXNeDXiVtx1GzbWC4R-eQ3ubf<af?J3cX141EMP<z
k1*$G!Ojzj6vcLn=s36}kTt;y*gUp&-DlM}4igX1F09eU5u>b%7

diff --git a/tests/data/acpi/q35/DSDT.dimmpxm b/tests/data/acpi/q35/DSDT.dimmpxm
index 9d5bd5744e2ba2e0f6126c3aba0bb36af865e499..13e80ae2e5c7606a7260d4cb3ab776488d0697d6 100644
GIT binary patch
delta 325
zcmezD@yV0RCD<k8lL`X^<MWMN6C{|N118UrSk2_-y4h1wfsx5wY;v}gOg)FQpKG`f
z)Bgg7<O0T&MXo}VQy2>vQVJLo3K$X>`SMJdoWM9WnTw@>A)z2SMxP}<*eO21+0!5(
z$TiqG*wv?i0bS79*JXkDRCG}n$B^Wu3pghya{UL|v$OyzUk^76q+o%{WORL=e!+|d
z$sHh9CNh*J0^POHe1R#3vLN3uM&}T>$Yd_&0!DN*fRaFcrO5>hNsC-rrY0p8G=mLH
eVo2rzYXaI3ZCH>zSzAh#6=-2;(q<cJb4~zS_+p&^

delta 194
zcmez5`PqZZCD<jTScQRs@!dwQ2@*^$9+T%ttY&hz-Rvo;z{upmH#u8MhOdAzVUZ*E
z<OIe7hJ*r!$%Zn*ehb`LCdcTr#0NXY2RM5g1O&MTJA3*CGdhR3MWTzlIEFA5FrteB
mRRhgk=*_aggJrUfw4^S&0-!c7W=wrSzF|Opn|Di_a{>Sy^)^ue

diff --git a/tests/data/acpi/q35/DSDT.ipmibt b/tests/data/acpi/q35/DSDT.ipmibt
index 5cd11de6a8fe47324e5f922823a22746882f19f5..9a1b635dab776fb25e378a00e6ca0cadf9902c25 100644
GIT binary patch
delta 286
zcmX?UbKI88CD<k8xEuom<LixFw<Vao>?S{#Sk2^XJb8-bUv@V!=U`_)j>#7##p^ko
z{anL^nEn?qBo{EIEOHT=oWfYZkW#>yP{5G5$cJab<OIg4$v~lmg5(%|miS<&_yA{5
zgMc8{VCP_0p8^JSL1$l=1>#fDMO_?2l9w*voSew@A85_e0+_rh%q)<C1uBza;xJ)P
zzhK6K<PMM{6B$YqfzDcJzQA;{oU|lk@?<Y*DaF#{0*0hTt}IiN5(}Ea>XI0eokQFr
afo@KYHY`Y<JWE=Y6=+Op(&iNDr%V8%Ct5cE

delta 182
zcmX?Zd(wu>CD<jzQ;vaw@%~1x+Y(GZW|N;wtY-4po;*eJFS`eybFi}?$K;EW65Nal
ziyXNpComQ;BusuMCFbYOGC4+{B|g|GKET=2ARx#!*xA!Bn9(`JEfQVa#W94jfDv63
lsJehLVWBt60uPqSlN+UlC2(n;Tq`X#xj{yB^LObBOaQB)Hktqc

diff --git a/tests/data/acpi/q35/DSDT.memhp b/tests/data/acpi/q35/DSDT.memhp
index 05a7a73ec43130d5c3018bb462fd84981bfb151c..55ce4e2293624c2c0725c3bbaaa7ec29acfccfc9 100644
GIT binary patch
delta 249
zcmX@>cG{iGCD<k8v@!z&<I0U(6C{`%>?hBWSk2^YGTBq|FT0zVbFi}?$K>~t;yN78
zey-s{O#cfQk_#A97P$ybPGKxyNGV`UC}2oj<ij&zasuPjWS~$&LGtAJlA@D~q(qp+
zr%qldCCr<=bOGn&M6UlpjY|taA}+}bR3^vhv&08G#RoWh8UzHn20MHD1v3^TcYy3k
zWGGDpn!M0_fhmTvAm1=X=McBZ<jMb~q!de&3mB3Xxw1@6N-Ssw>rG-v21^6YiZ%qg
W#a~*L6=-y6(qscEvCaC@avT8KEKcbF

delta 203
zcmX@@e%6i4CD<jzSDAr<aqdR02@*_B=9A}0tY&i6nd~X~m)(QUIoR2cWAb}R31!BF
zMULE)6Br8^5(*e%^jYGAo#F$WJq-eaT!WpReO(r~vrPUdB`V<@;ueW6?cx~1IJrSu
q)W3i+VWBt60uPqS=t_co!x(XB<zg;i#G)K%)?@=IvCV6x<v0MCY&quu

diff --git a/tests/data/acpi/q35/DSDT.mmio64 b/tests/data/acpi/q35/DSDT.mmio64
index efd3f1188f2b55da1514212d4be081a61c2a96e9..99b7b2ae4ba36b8ca7901626c1561d29100087d2 100644
GIT binary patch
delta 249
zcmaFi^30XXCD<k8nGyp7W9mk(2@*^W_LJvGtY&gHnd~X~m)%XwIoR2cWAb}RaUBk4
zKi6;}rvC*D$pwrli(G^zr!W>Uq!chF6fh(%^5K~<Ie~F%GEgX?AbIk9NzutgQX)*^
zQztK!66Q@_x`1<XBG-SQ#-#-y5trlzDwAXMS>l78;scyL4FZB(gPlG7f*A{vJ3#g%
zGL$9)O<riez!XDSkZ%~HbBJ4H^5p+gQi`R?1q?}xTv?_jB^ES;^(HYSgQbCHMH>R$
W;xDbr3N*SjX|jQo*k*lcOLhR{%1;Xb

delta 203
zcmaFn`oe|FCD<h-Ly3WbF>)i<1PLZ5^T~50Rx`QkO!k!g%kIJF9PI4JG5Niugfe5o
zB1i7Y35*2{2?Y!>`YiFmPVoWGo(2IyuEEaEzAg*gStfs!5|wZcaf?Kkc5w`0oZKKS
q>R-T^u+W=jfd|WEbR|K)VT?GmaxoV$Vo?q>YqEiq*ygp;mh1qbnmP#p

diff --git a/tests/data/acpi/q35/DSDT.numamem b/tests/data/acpi/q35/DSDT.numamem
index 1978b55f1255402bf9bade0b91150b5cb49789a4..2b2433cc13ec3110abbc8440a0b1ad8c487edb6c 100644
GIT binary patch
delta 249
zcmZp%X|m;V33dr-l4D?COx(ydL4wJ_e)1fN)lAMNlRYK>vb%{n2Rr+5OnxsZuEXK%
z=Nc}=^uK^1xqvZck&Dpe6vhIElmf<t0*1szK0Fg9CooP;1_~t<Bu}0%DLT1GN`y&#
z>g0t|!o0~#7jRBa<oXZPxU>Ky;*z{TWpa!@OMI|Xe1Nm3K|qjeu(PLMFk?Y-2gsg8
zhSEf!$qUUFm|`dk@(p8j4snZ2p8Q`*O0hJ#fFWs-E6dcR#DZq9-Xw-(ur$!DXhWb|
W{H0Y{fku}mO*W7c+pI5rl?ec^mQG#(

delta 203
zcmZp&ZL#5U33dr#kz-(B4Bf~zL4wK2eDWNL)l9BBlRYK>vU~732Rr+5OnxsZq0E@D
z$dP+;0%HL~LIFdJK1+PCQ+$B4r$IoFYp}Dkugd~=mdPKbL?xU<+#=DXT^vIgCpSoo
q`WG-JEc9ks;K4E(T}hB{7$Xj?T+9WGSd;_Jnrt8?wt21eRVDzYo;h3q

diff --git a/tests/data/acpi/q35/DSDT.tis b/tests/data/acpi/q35/DSDT.tis
index 638de3872673d17b1958497d0e62c83653de1602..dd06ee4c348930b0684827ca05747b4f65dbd6b2 100644
GIT binary patch
delta 249
zcmccZaMO{?CD<k8rUC;4<AjY|6C{|t>?Y5VSk2^XJlRw7FT0zVbFi}?$K>~t;yN78
zey-s{O#cfQk_#A97P$ybPGKxyNGV`UC}2oj<ij&zasuPjWS~$&LGtAJlA@D~q(qp+
zr%qldCCr<=bOGn&M6UlpjY|taA}+}bR3^vhv&08G#RoWh8UzHn20MHD1v3^TcYy3k
zWGGDpn!M0_fhmTvAm1=X=McBZ<jMb~q!de&3mB3Xxw1@6N-Ssw>rG-v21^6YiZ%qg
W#a~*L6=-y6(qscEvCaC@I;;Tg4o?UG

delta 203
zcmccVc-w)?CD<h-T7iLqv1KFI1PLY|v&nNLRx|l)Pxh4j%kIJF9PI4JG5Niugfe5o
zB1i7Y35*2{2?Y!>`YiFmPVoWGo(2IyuEEaEzAg*gStfs!5|wZcaf?Kkc5w`0oZKKS
q>R-T^u+W=jfd|WEbR|K)VT?GmaxoV$Vo?q>YqEiq*ygp;I;;SvwmJp?

-- 
2.27.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v6 11/11] smp: drop support for deprecated (invalid topologies)
  2020-09-23  9:46 [PATCH v6 00/11] x86: fix cpu hotplug with secure boot Igor Mammedov
                   ` (9 preceding siblings ...)
  2020-09-23  9:46 ` [PATCH v6 10/11] tests: acpi: update acpi blobs with new AML Igor Mammedov
@ 2020-09-23  9:46 ` Igor Mammedov
  2020-09-23  9:55   ` Igor Mammedov
  2020-09-23 16:44 ` [PATCH v6 00/11] x86: fix cpu hotplug with secure boot Laszlo Ersek
  11 siblings, 1 reply; 16+ messages in thread
From: Igor Mammedov @ 2020-09-23  9:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: lersek, ankur.a.arora, mst

it's was deprecated since 3.1

Support for invalid topologies is removed, the user must ensure
that topologies described with -smp include all possible cpus,
i.e. (sockets * cores * threads) == maxcpus or QEMU will
exit with error.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 docs/system/deprecated.rst | 26 +++++++++++++-------------
 hw/core/machine.c          | 16 ++++------------
 hw/i386/pc.c               | 16 ++++------------
 3 files changed, 21 insertions(+), 37 deletions(-)

diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 808c334fe7..fb95d2ecc4 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -47,19 +47,6 @@ The 'file' driver for drives is no longer appropriate for character or host
 devices and will only accept regular files (S_IFREG). The correct driver
 for these file types is 'host_cdrom' or 'host_device' as appropriate.
 
-``-smp`` (invalid topologies) (since 3.1)
-'''''''''''''''''''''''''''''''''''''''''
-
-CPU topology properties should describe whole machine topology including
-possible CPUs.
-
-However, historically it was possible to start QEMU with an incorrect topology
-where *n* <= *sockets* * *cores* * *threads* < *maxcpus*,
-which could lead to an incorrect topology enumeration by the guest.
-Support for invalid topologies will be removed, the user must ensure
-topologies described with -smp include all possible cpus, i.e.
-*sockets* * *cores* * *threads* = *maxcpus*.
-
 ``-vnc acl`` (since 4.0.0)
 ''''''''''''''''''''''''''
 
@@ -642,6 +629,19 @@ New machine versions (since 5.1) will not accept the option but it will still
 work with old machine types. User can check the QAPI schema to see if the legacy
 option is supported by looking at MachineInfo::numa-mem-supported property.
 
+``-smp`` (invalid topologies) (removed 5.2)
+'''''''''''''''''''''''''''''''''''''''''''
+
+CPU topology properties should describe whole machine topology including
+possible CPUs.
+
+However, historically it was possible to start QEMU with an incorrect topology
+where *n* <= *sockets* * *cores* * *threads* < *maxcpus*,
+which could lead to an incorrect topology enumeration by the guest.
+Support for invalid topologies is removed, the user must ensure
+topologies described with -smp include all possible cpus, i.e.
+*sockets* * *cores* * *threads* = *maxcpus*.
+
 Block devices
 -------------
 
diff --git a/hw/core/machine.c b/hw/core/machine.c
index ea26d61237..09aee4ea52 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -754,23 +754,15 @@ static void smp_parse(MachineState *ms, QemuOpts *opts)
             exit(1);
         }
 
-        if (sockets * cores * threads > ms->smp.max_cpus) {
-            error_report("cpu topology: "
-                         "sockets (%u) * cores (%u) * threads (%u) > "
-                         "maxcpus (%u)",
+        if (sockets * cores * threads != ms->smp.max_cpus) {
+            error_report("Invalid CPU topology: "
+                         "sockets (%u) * cores (%u) * threads (%u) "
+                         "!= maxcpus (%u)",
                          sockets, cores, threads,
                          ms->smp.max_cpus);
             exit(1);
         }
 
-        if (sockets * cores * threads != ms->smp.max_cpus) {
-            warn_report("Invalid CPU topology deprecated: "
-                        "sockets (%u) * cores (%u) * threads (%u) "
-                        "!= maxcpus (%u)",
-                        sockets, cores, threads,
-                        ms->smp.max_cpus);
-        }
-
         ms->smp.cpus = cpus;
         ms->smp.cores = cores;
         ms->smp.threads = threads;
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 2af660c55e..53e21a186d 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -748,23 +748,15 @@ void pc_smp_parse(MachineState *ms, QemuOpts *opts)
             exit(1);
         }
 
-        if (sockets * dies * cores * threads > ms->smp.max_cpus) {
-            error_report("cpu topology: "
-                         "sockets (%u) * dies (%u) * cores (%u) * threads (%u) > "
-                         "maxcpus (%u)",
+        if (sockets * dies * cores * threads != ms->smp.max_cpus) {
+            error_report("Invalid CPU topology deprecated: "
+                         "sockets (%u) * dies (%u) * cores (%u) * threads (%u) "
+                         "!= maxcpus (%u)",
                          sockets, dies, cores, threads,
                          ms->smp.max_cpus);
             exit(1);
         }
 
-        if (sockets * dies * cores * threads != ms->smp.max_cpus) {
-            warn_report("Invalid CPU topology deprecated: "
-                        "sockets (%u) * dies (%u) * cores (%u) * threads (%u) "
-                        "!= maxcpus (%u)",
-                        sockets, dies, cores, threads,
-                        ms->smp.max_cpus);
-        }
-
         ms->smp.cpus = cpus;
         ms->smp.cores = cores;
         ms->smp.threads = threads;
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v6 11/11] smp: drop support for deprecated (invalid topologies)
  2020-09-23  9:46 ` [PATCH v6 11/11] smp: drop support for deprecated (invalid topologies) Igor Mammedov
@ 2020-09-23  9:55   ` Igor Mammedov
  0 siblings, 0 replies; 16+ messages in thread
From: Igor Mammedov @ 2020-09-23  9:55 UTC (permalink / raw)
  To: qemu-devel; +Cc: lersek, ankur.a.arora, mst

On Wed, 23 Sep 2020 05:46:50 -0400
Igor Mammedov <imammedo@redhat.com> wrote:

> it's was deprecated since 3.1
> 
> Support for invalid topologies is removed, the user must ensure
> that topologies described with -smp include all possible cpus,
> i.e. (sockets * cores * threads) == maxcpus or QEMU will
> exit with error.

ignore this stowaway, it's not part of the series 


> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  docs/system/deprecated.rst | 26 +++++++++++++-------------
>  hw/core/machine.c          | 16 ++++------------
>  hw/i386/pc.c               | 16 ++++------------
>  3 files changed, 21 insertions(+), 37 deletions(-)
> 
> diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
> index 808c334fe7..fb95d2ecc4 100644
> --- a/docs/system/deprecated.rst
> +++ b/docs/system/deprecated.rst
> @@ -47,19 +47,6 @@ The 'file' driver for drives is no longer appropriate for character or host
>  devices and will only accept regular files (S_IFREG). The correct driver
>  for these file types is 'host_cdrom' or 'host_device' as appropriate.
>  
> -``-smp`` (invalid topologies) (since 3.1)
> -'''''''''''''''''''''''''''''''''''''''''
> -
> -CPU topology properties should describe whole machine topology including
> -possible CPUs.
> -
> -However, historically it was possible to start QEMU with an incorrect topology
> -where *n* <= *sockets* * *cores* * *threads* < *maxcpus*,
> -which could lead to an incorrect topology enumeration by the guest.
> -Support for invalid topologies will be removed, the user must ensure
> -topologies described with -smp include all possible cpus, i.e.
> -*sockets* * *cores* * *threads* = *maxcpus*.
> -
>  ``-vnc acl`` (since 4.0.0)
>  ''''''''''''''''''''''''''
>  
> @@ -642,6 +629,19 @@ New machine versions (since 5.1) will not accept the option but it will still
>  work with old machine types. User can check the QAPI schema to see if the legacy
>  option is supported by looking at MachineInfo::numa-mem-supported property.
>  
> +``-smp`` (invalid topologies) (removed 5.2)
> +'''''''''''''''''''''''''''''''''''''''''''
> +
> +CPU topology properties should describe whole machine topology including
> +possible CPUs.
> +
> +However, historically it was possible to start QEMU with an incorrect topology
> +where *n* <= *sockets* * *cores* * *threads* < *maxcpus*,
> +which could lead to an incorrect topology enumeration by the guest.
> +Support for invalid topologies is removed, the user must ensure
> +topologies described with -smp include all possible cpus, i.e.
> +*sockets* * *cores* * *threads* = *maxcpus*.
> +
>  Block devices
>  -------------
>  
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index ea26d61237..09aee4ea52 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -754,23 +754,15 @@ static void smp_parse(MachineState *ms, QemuOpts *opts)
>              exit(1);
>          }
>  
> -        if (sockets * cores * threads > ms->smp.max_cpus) {
> -            error_report("cpu topology: "
> -                         "sockets (%u) * cores (%u) * threads (%u) > "
> -                         "maxcpus (%u)",
> +        if (sockets * cores * threads != ms->smp.max_cpus) {
> +            error_report("Invalid CPU topology: "
> +                         "sockets (%u) * cores (%u) * threads (%u) "
> +                         "!= maxcpus (%u)",
>                           sockets, cores, threads,
>                           ms->smp.max_cpus);
>              exit(1);
>          }
>  
> -        if (sockets * cores * threads != ms->smp.max_cpus) {
> -            warn_report("Invalid CPU topology deprecated: "
> -                        "sockets (%u) * cores (%u) * threads (%u) "
> -                        "!= maxcpus (%u)",
> -                        sockets, cores, threads,
> -                        ms->smp.max_cpus);
> -        }
> -
>          ms->smp.cpus = cpus;
>          ms->smp.cores = cores;
>          ms->smp.threads = threads;
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 2af660c55e..53e21a186d 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -748,23 +748,15 @@ void pc_smp_parse(MachineState *ms, QemuOpts *opts)
>              exit(1);
>          }
>  
> -        if (sockets * dies * cores * threads > ms->smp.max_cpus) {
> -            error_report("cpu topology: "
> -                         "sockets (%u) * dies (%u) * cores (%u) * threads (%u) > "
> -                         "maxcpus (%u)",
> +        if (sockets * dies * cores * threads != ms->smp.max_cpus) {
> +            error_report("Invalid CPU topology deprecated: "
> +                         "sockets (%u) * dies (%u) * cores (%u) * threads (%u) "
> +                         "!= maxcpus (%u)",
>                           sockets, dies, cores, threads,
>                           ms->smp.max_cpus);
>              exit(1);
>          }
>  
> -        if (sockets * dies * cores * threads != ms->smp.max_cpus) {
> -            warn_report("Invalid CPU topology deprecated: "
> -                        "sockets (%u) * dies (%u) * cores (%u) * threads (%u) "
> -                        "!= maxcpus (%u)",
> -                        sockets, dies, cores, threads,
> -                        ms->smp.max_cpus);
> -        }
> -
>          ms->smp.cpus = cpus;
>          ms->smp.cores = cores;
>          ms->smp.threads = threads;



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v6 00/11] x86: fix cpu hotplug with secure boot
  2020-09-23  9:46 [PATCH v6 00/11] x86: fix cpu hotplug with secure boot Igor Mammedov
                   ` (10 preceding siblings ...)
  2020-09-23  9:46 ` [PATCH v6 11/11] smp: drop support for deprecated (invalid topologies) Igor Mammedov
@ 2020-09-23 16:44 ` Laszlo Ersek
  2020-09-24  7:12   ` Igor Mammedov
  11 siblings, 1 reply; 16+ messages in thread
From: Laszlo Ersek @ 2020-09-23 16:44 UTC (permalink / raw)
  To: Igor Mammedov, qemu-devel; +Cc: ankur.a.arora, mst

On 09/23/20 11:46, Igor Mammedov wrote:
> v6:
>   - [9/10] Add comment explaining why while_ctx2 restarts from the last processed CPU.
>   - rebase on top of current master, due to non trivial conflict
>     caused by microvm series, which moved/renamed pc_cpu_pre_plug()

So, I went back to my local branch where I had applied your v5, *plus*
the comment fixup ("[PATCH v5 9/10] fixup! x68: acpi: trigger SMI before
sending hotplug Notify event to OSPM") on top. I rebased that branch to
its *same* base commit, only squashing the comment fixup into patch#9.

Then I applied your v6 series on top of current master, using a
different (new) local branch.

Then I ran git-range-diff on these two local branches.

In patches 6, 7, 8, and 9, you've picked up my feedback tags from the v5
review session; that's good, there was nothing else to do.

There is a trivial difference in patch 2 -- trivial to review, that is;
I'm not saying that it's so trivial that git-rebase should have coped
with it automatically on your end. Here's the git-range-diff output:

>  2:  e606a75432a8 !  2:  94702d2e3125 x86: cpuhp: prevent guest crash on CPU hotplug when broadcast SMI is in use
>     @@ -12,7 +12,7 @@
>          Signed-off-by: Igor Mammedov <imammedo@redhat.com>
>          Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>          Tested-by: Laszlo Ersek <lersek@redhat.com>
>     -    Message-Id: <20200907112348.530921-3-imammedo@redhat.com>
>     +    Message-Id: <20200923094650.1301166-3-imammedo@redhat.com>
>
>      diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
>      --- a/hw/acpi/ich9.c
>     @@ -40,17 +40,17 @@
>
>       void ich9_pm_device_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
>
>     -diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>     ---- a/hw/i386/pc.c
>     -+++ b/hw/i386/pc.c
>     +diff --git a/hw/i386/x86.c b/hw/i386/x86.c
>     +--- a/hw/i386/x86.c
>     ++++ b/hw/i386/x86.c
>      @@
>               return;
>           }
>
>     -+    if (pcms->acpi_dev) {
>     ++    if (x86ms->acpi_dev) {
>      +        Error *local_err = NULL;
>      +
>     -+        hotplug_handler_pre_plug(HOTPLUG_HANDLER(pcms->acpi_dev), dev,
>     ++        hotplug_handler_pre_plug(HOTPLUG_HANDLER(x86ms->acpi_dev), dev,
>      +                                 &local_err);
>      +        if (local_err) {
>      +            error_propagate(errp, local_err);

Meaning that, in v6, you had to refer to "x86ms", rather than to "pcms",
and that the code had to be introduced in a different file / function.

The need for that originates from 0cca1a918b85 ("x86: move cpu hotplug
from pc to x86", 2020-09-17).

It looks innocent enough, but I should still retest patch#2. I'll report
back under that patch in this series.

Thanks
Laszlo



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v6 02/11] x86: cpuhp: prevent guest crash on CPU hotplug when broadcast SMI is in use
  2020-09-23  9:46 ` [PATCH v6 02/11] x86: cpuhp: prevent guest crash on CPU hotplug when broadcast SMI is in use Igor Mammedov
@ 2020-09-23 17:16   ` Laszlo Ersek
  0 siblings, 0 replies; 16+ messages in thread
From: Laszlo Ersek @ 2020-09-23 17:16 UTC (permalink / raw)
  To: Igor Mammedov, qemu-devel; +Cc: ankur.a.arora, mst

On 09/23/20 11:46, Igor Mammedov wrote:
> There were reports of guest crash on CPU hotplug, when using q35 machine
> type and OVMF with SMM, due to hotplugged CPU trying to process SMI at
> default SMI handler location without it being relocated by firmware first.
> 
> Fix it by refusing hotplug if firmware hasn't negotiated CPU hotplug with
> SMI support while SMI broadcast is in use.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> Tested-by: Laszlo Ersek <lersek@redhat.com>
> ---
> v6:
>   - rebase on top of current master, due to non trivial conflicts
>     caused by microvm series, which moved/renamed pc_cpu_pre_plug()
>     (Ankur Arora <ankur.a.arora@oracle.com>)

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

Thanks
Laszlo

> v1:
>    fix typos an use suggested wording in commit and error msg
>    s/secure boot/smm/; s/hotplug SMI/hotplug with SMI/
>       (Laszlo Ersek <lersek@redhat.com>)
> ---
>  hw/acpi/ich9.c | 12 +++++++++++-
>  hw/i386/x86.c  | 11 +++++++++++
>  2 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
> index 6a19070cec..0acc9a3107 100644
> --- a/hw/acpi/ich9.c
> +++ b/hw/acpi/ich9.c
> @@ -408,10 +408,20 @@ void ich9_pm_device_pre_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
>      ICH9LPCState *lpc = ICH9_LPC_DEVICE(hotplug_dev);
>  
>      if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) &&
> -        !lpc->pm.acpi_memory_hotplug.is_enabled)
> +        !lpc->pm.acpi_memory_hotplug.is_enabled) {
>          error_setg(errp,
>                     "memory hotplug is not enabled: %s.memory-hotplug-support "
>                     "is not set", object_get_typename(OBJECT(lpc)));
> +    } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
> +        uint64_t negotiated = lpc->smi_negotiated_features;
> +
> +        if (negotiated & BIT_ULL(ICH9_LPC_SMI_F_BROADCAST_BIT) &&
> +            !(negotiated & BIT_ULL(ICH9_LPC_SMI_F_CPU_HOTPLUG_BIT))) {
> +            error_setg(errp, "cpu hotplug with SMI wasn't enabled by firmware");
> +            error_append_hint(errp, "update machine type to newer than 5.1 "
> +                "and firmware that suppors CPU hotplug with SMM");
> +        }
> +    }
>  }
>  
>  void ich9_pm_device_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
> diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> index c2ea989579..403c2b1dad 100644
> --- a/hw/i386/x86.c
> +++ b/hw/i386/x86.c
> @@ -279,6 +279,17 @@ void x86_cpu_pre_plug(HotplugHandler *hotplug_dev,
>          return;
>      }
>  
> +    if (x86ms->acpi_dev) {
> +        Error *local_err = NULL;
> +
> +        hotplug_handler_pre_plug(HOTPLUG_HANDLER(x86ms->acpi_dev), dev,
> +                                 &local_err);
> +        if (local_err) {
> +            error_propagate(errp, local_err);
> +            return;
> +        }
> +    }
> +
>      init_topo_info(&topo_info, x86ms);
>  
>      env->nr_dies = x86ms->smp_dies;
> 



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v6 00/11] x86: fix cpu hotplug with secure boot
  2020-09-23 16:44 ` [PATCH v6 00/11] x86: fix cpu hotplug with secure boot Laszlo Ersek
@ 2020-09-24  7:12   ` Igor Mammedov
  0 siblings, 0 replies; 16+ messages in thread
From: Igor Mammedov @ 2020-09-24  7:12 UTC (permalink / raw)
  To: Laszlo Ersek; +Cc: qemu-devel, ankur.a.arora, mst

On Wed, 23 Sep 2020 18:44:50 +0200
Laszlo Ersek <lersek@redhat.com> wrote:

> On 09/23/20 11:46, Igor Mammedov wrote:
> > v6:
> >   - [9/10] Add comment explaining why while_ctx2 restarts from the last processed CPU.
> >   - rebase on top of current master, due to non trivial conflict
> >     caused by microvm series, which moved/renamed pc_cpu_pre_plug()  
> 
> So, I went back to my local branch where I had applied your v5, *plus*
> the comment fixup ("[PATCH v5 9/10] fixup! x68: acpi: trigger SMI before
> sending hotplug Notify event to OSPM") on top. I rebased that branch to
> its *same* base commit, only squashing the comment fixup into patch#9.
> 
> Then I applied your v6 series on top of current master, using a
> different (new) local branch.
> 
> Then I ran git-range-diff on these two local branches.
> 
> In patches 6, 7, 8, and 9, you've picked up my feedback tags from the v5
> review session; that's good, there was nothing else to do.
> 
> There is a trivial difference in patch 2 -- trivial to review, that is;
> I'm not saying that it's so trivial that git-rebase should have coped
> with it automatically on your end. Here's the git-range-diff output:
> 
> >  2:  e606a75432a8 !  2:  94702d2e3125 x86: cpuhp: prevent guest crash on CPU hotplug when broadcast SMI is in use
> >     @@ -12,7 +12,7 @@
> >          Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> >          Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> >          Tested-by: Laszlo Ersek <lersek@redhat.com>
> >     -    Message-Id: <20200907112348.530921-3-imammedo@redhat.com>
> >     +    Message-Id: <20200923094650.1301166-3-imammedo@redhat.com>
> >
> >      diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
> >      --- a/hw/acpi/ich9.c
> >     @@ -40,17 +40,17 @@
> >
> >       void ich9_pm_device_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
> >
> >     -diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> >     ---- a/hw/i386/pc.c
> >     -+++ b/hw/i386/pc.c
> >     +diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> >     +--- a/hw/i386/x86.c
> >     ++++ b/hw/i386/x86.c
> >      @@
> >               return;
> >           }
> >
> >     -+    if (pcms->acpi_dev) {
> >     ++    if (x86ms->acpi_dev) {
> >      +        Error *local_err = NULL;
> >      +
> >     -+        hotplug_handler_pre_plug(HOTPLUG_HANDLER(pcms->acpi_dev), dev,
> >     ++        hotplug_handler_pre_plug(HOTPLUG_HANDLER(x86ms->acpi_dev), dev,
> >      +                                 &local_err);
> >      +        if (local_err) {
> >      +            error_propagate(errp, local_err);  
> 
> Meaning that, in v6, you had to refer to "x86ms", rather than to "pcms",
> and that the code had to be introduced in a different file / function.
> 
> The need for that originates from 0cca1a918b85 ("x86: move cpu hotplug
> from pc to x86", 2020-09-17).
I should have added this commit to change log to spare you
trouble figuring out what exactly has changed.

> 
> It looks innocent enough, but I should still retest patch#2. I'll report
> back under that patch in this series.
> 
> Thanks
> Laszlo



^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2020-09-24  7:13 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-09-23  9:46 [PATCH v6 00/11] x86: fix cpu hotplug with secure boot Igor Mammedov
2020-09-23  9:46 ` [PATCH v6 01/11] x86: lpc9: let firmware negotiate 'CPU hotplug with SMI' features Igor Mammedov
2020-09-23  9:46 ` [PATCH v6 02/11] x86: cpuhp: prevent guest crash on CPU hotplug when broadcast SMI is in use Igor Mammedov
2020-09-23 17:16   ` Laszlo Ersek
2020-09-23  9:46 ` [PATCH v6 03/11] x86: cpuhp: refuse cpu hot-unplug request earlier if not supported Igor Mammedov
2020-09-23  9:46 ` [PATCH v6 04/11] acpi: add aml_land() and aml_break() primitives Igor Mammedov
2020-09-23  9:46 ` [PATCH v6 05/11] tests: acpi: mark to be changed tables in bios-tables-test-allowed-diff Igor Mammedov
2020-09-23  9:46 ` [PATCH v6 06/11] x86: ich9: expose "smi_negotiated_features" as a QOM property Igor Mammedov
2020-09-23  9:46 ` [PATCH v6 07/11] x86: acpi: introduce AcpiPmInfo::smi_on_cpuhp Igor Mammedov
2020-09-23  9:46 ` [PATCH v6 08/11] x86: acpi: introduce the PCI0.SMI0 ACPI device Igor Mammedov
2020-09-23  9:46 ` [PATCH v6 09/11] x68: acpi: trigger SMI before sending hotplug Notify event to OSPM Igor Mammedov
2020-09-23  9:46 ` [PATCH v6 10/11] tests: acpi: update acpi blobs with new AML Igor Mammedov
2020-09-23  9:46 ` [PATCH v6 11/11] smp: drop support for deprecated (invalid topologies) Igor Mammedov
2020-09-23  9:55   ` Igor Mammedov
2020-09-23 16:44 ` [PATCH v6 00/11] x86: fix cpu hotplug with secure boot Laszlo Ersek
2020-09-24  7:12   ` Igor Mammedov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).