* [PATCH 1/8] hw: add compat machines for 6.0
2020-12-04 17:09 [PATCH 0/8] add support for cpu hot-unplug with SMI broadcast enabled Igor Mammedov
@ 2020-12-04 17:09 ` Igor Mammedov
2020-12-04 17:09 ` [PATCH 2/8] acpi: cpuhp: introduce 'firmware performs eject' status/control bits Igor Mammedov
` (6 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Igor Mammedov @ 2020-12-04 17:09 UTC (permalink / raw)
To: qemu-devel; +Cc: lersek, ankur.a.arora, mst
From: Cornelia Huck <cohuck@redhat.com>
Add 6.0 machine types for arm/i440fx/q35/s390x/spapr.
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
include/hw/boards.h | 3 +++
include/hw/i386/pc.h | 3 +++
hw/arm/virt.c | 9 ++++++++-
hw/core/machine.c | 3 +++
hw/i386/pc.c | 3 +++
hw/i386/pc_piix.c | 14 +++++++++++++-
hw/i386/pc_q35.c | 13 ++++++++++++-
hw/ppc/spapr.c | 15 +++++++++++++--
hw/s390x/s390-virtio-ccw.c | 14 +++++++++++++-
9 files changed, 71 insertions(+), 6 deletions(-)
diff --git a/include/hw/boards.h b/include/hw/boards.h
index a49e3a6b44..f94f4ad5d8 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -310,6 +310,9 @@ struct MachineState {
} \
type_init(machine_initfn##_register_types)
+extern GlobalProperty hw_compat_5_2[];
+extern const size_t hw_compat_5_2_len;
+
extern GlobalProperty hw_compat_5_1[];
extern const size_t hw_compat_5_1_len;
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 911e460097..49dfa667de 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -191,6 +191,9 @@ void pc_system_firmware_init(PCMachineState *pcms, MemoryRegion *rom_memory);
void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
const CPUArchIdList *apic_ids, GArray *entry);
+extern GlobalProperty pc_compat_5_2[];
+extern const size_t pc_compat_5_2_len;
+
extern GlobalProperty pc_compat_5_1[];
extern const size_t pc_compat_5_1_len;
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 27dbeb549e..d21dad4491 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2587,10 +2587,17 @@ static void machvirt_machine_init(void)
}
type_init(machvirt_machine_init);
+static void virt_machine_6_0_options(MachineClass *mc)
+{
+}
+DEFINE_VIRT_MACHINE_AS_LATEST(6, 0)
+
static void virt_machine_5_2_options(MachineClass *mc)
{
+ virt_machine_6_0_options(mc);
+ compat_props_add(mc->compat_props, hw_compat_5_2, hw_compat_5_2_len);
}
-DEFINE_VIRT_MACHINE_AS_LATEST(5, 2)
+DEFINE_VIRT_MACHINE(5, 2)
static void virt_machine_5_1_options(MachineClass *mc)
{
diff --git a/hw/core/machine.c b/hw/core/machine.c
index d0408049b5..9c41b94e0d 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -28,6 +28,9 @@
#include "hw/mem/nvdimm.h"
#include "migration/vmstate.h"
+GlobalProperty hw_compat_5_2[] = {};
+const size_t hw_compat_5_2_len = G_N_ELEMENTS(hw_compat_5_2);
+
GlobalProperty hw_compat_5_1[] = {
{ "vhost-scsi", "num_queues", "1"},
{ "vhost-user-blk", "num-queues", "1"},
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 17b514d1da..781523684c 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -97,6 +97,9 @@
#include "trace.h"
#include CONFIG_DEVICES
+GlobalProperty pc_compat_5_2[] = {};
+const size_t pc_compat_5_2_len = G_N_ELEMENTS(pc_compat_5_2);
+
GlobalProperty pc_compat_5_1[] = {
{ "ICH9-LPC", "x-smi-cpu-hotplug", "off" },
};
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 13d1628f13..6188c3e97e 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -426,7 +426,7 @@ static void pc_i440fx_machine_options(MachineClass *m)
machine_class_allow_dynamic_sysbus_dev(m, TYPE_VMBUS_BRIDGE);
}
-static void pc_i440fx_5_2_machine_options(MachineClass *m)
+static void pc_i440fx_6_0_machine_options(MachineClass *m)
{
PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
pc_i440fx_machine_options(m);
@@ -435,6 +435,18 @@ static void pc_i440fx_5_2_machine_options(MachineClass *m)
pcmc->default_cpu_version = 1;
}
+DEFINE_I440FX_MACHINE(v6_0, "pc-i440fx-6.0", NULL,
+ pc_i440fx_6_0_machine_options);
+
+static void pc_i440fx_5_2_machine_options(MachineClass *m)
+{
+ pc_i440fx_6_0_machine_options(m);
+ m->alias = NULL;
+ m->is_default = false;
+ compat_props_add(m->compat_props, hw_compat_5_2, hw_compat_5_2_len);
+ compat_props_add(m->compat_props, pc_compat_5_2, pc_compat_5_2_len);
+}
+
DEFINE_I440FX_MACHINE(v5_2, "pc-i440fx-5.2", NULL,
pc_i440fx_5_2_machine_options);
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index a3f4959c43..0a212443aa 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -344,7 +344,7 @@ static void pc_q35_machine_options(MachineClass *m)
m->max_cpus = 288;
}
-static void pc_q35_5_2_machine_options(MachineClass *m)
+static void pc_q35_6_0_machine_options(MachineClass *m)
{
PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
pc_q35_machine_options(m);
@@ -352,6 +352,17 @@ static void pc_q35_5_2_machine_options(MachineClass *m)
pcmc->default_cpu_version = 1;
}
+DEFINE_Q35_MACHINE(v6_0, "pc-q35-6.0", NULL,
+ pc_q35_6_0_machine_options);
+
+static void pc_q35_5_2_machine_options(MachineClass *m)
+{
+ pc_q35_6_0_machine_options(m);
+ m->alias = NULL;
+ compat_props_add(m->compat_props, hw_compat_5_2, hw_compat_5_2_len);
+ compat_props_add(m->compat_props, pc_compat_5_2, pc_compat_5_2_len);
+}
+
DEFINE_Q35_MACHINE(v5_2, "pc-q35-5.2", NULL,
pc_q35_5_2_machine_options);
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 12a012d9dd..c060702013 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -4509,15 +4509,26 @@ static void spapr_machine_latest_class_options(MachineClass *mc)
} \
type_init(spapr_machine_register_##suffix)
+/*
+ * pseries-6.0
+ */
+static void spapr_machine_6_0_class_options(MachineClass *mc)
+{
+ /* Defaults for the latest behaviour inherited from the base class */
+}
+
+DEFINE_SPAPR_MACHINE(6_0, "6.0", true);
+
/*
* pseries-5.2
*/
static void spapr_machine_5_2_class_options(MachineClass *mc)
{
- /* Defaults for the latest behaviour inherited from the base class */
+ spapr_machine_6_0_class_options(mc);
+ compat_props_add(mc->compat_props, hw_compat_5_2, hw_compat_5_2_len);
}
-DEFINE_SPAPR_MACHINE(5_2, "5.2", true);
+DEFINE_SPAPR_MACHINE(5_2, "5.2", false);
/*
* pseries-5.1
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 4e140bbead..f0ee8dae68 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -789,14 +789,26 @@ bool css_migration_enabled(void)
} \
type_init(ccw_machine_register_##suffix)
+static void ccw_machine_6_0_instance_options(MachineState *machine)
+{
+}
+
+static void ccw_machine_6_0_class_options(MachineClass *mc)
+{
+}
+DEFINE_CCW_MACHINE(6_0, "6.0", true);
+
static void ccw_machine_5_2_instance_options(MachineState *machine)
{
+ ccw_machine_6_0_instance_options(machine);
}
static void ccw_machine_5_2_class_options(MachineClass *mc)
{
+ ccw_machine_6_0_class_options(mc);
+ compat_props_add(mc->compat_props, hw_compat_5_2, hw_compat_5_2_len);
}
-DEFINE_CCW_MACHINE(5_2, "5.2", true);
+DEFINE_CCW_MACHINE(5_2, "5.2", false);
static void ccw_machine_5_1_instance_options(MachineState *machine)
{
--
2.27.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 2/8] acpi: cpuhp: introduce 'firmware performs eject' status/control bits
2020-12-04 17:09 [PATCH 0/8] add support for cpu hot-unplug with SMI broadcast enabled Igor Mammedov
2020-12-04 17:09 ` [PATCH 1/8] hw: add compat machines for 6.0 Igor Mammedov
@ 2020-12-04 17:09 ` Igor Mammedov
2020-12-07 6:31 ` Ankur Arora
2020-12-04 17:09 ` [PATCH 3/8] x86: acpi: introduce AcpiPmInfo::smi_on_cpu_unplug Igor Mammedov
` (5 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Igor Mammedov @ 2020-12-04 17:09 UTC (permalink / raw)
To: qemu-devel; +Cc: lersek, ankur.a.arora, mst
Adds bit #4 to status/control field of CPU hotplug MMIO interface.
New bit will be used OSPM to mark CPUs as pending for removal by firmware,
when it calls _EJ0 method on CPU device node. Later on, when firmware
sees this bit set, it will perform CPU eject which will clear bit #4
as well.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v1:
- rearrange status/control bits description (Laszlo)
- add clear bit #4 on eject
- drop toggling logic from bit #4, it can be only set by guest
and clear as part of cpu eject
- exclude boot CPU from remove request
- add trace events for new bit
---
include/hw/acpi/cpu.h | 1 +
docs/specs/acpi_cpu_hotplug.txt | 19 ++++++++++++++-----
hw/acpi/cpu.c | 9 +++++++++
hw/acpi/trace-events | 2 ++
4 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/include/hw/acpi/cpu.h b/include/hw/acpi/cpu.h
index 0eeedaa491..d71edde456 100644
--- a/include/hw/acpi/cpu.h
+++ b/include/hw/acpi/cpu.h
@@ -22,6 +22,7 @@ typedef struct AcpiCpuStatus {
uint64_t arch_id;
bool is_inserting;
bool is_removing;
+ bool fw_remove;
uint32_t ost_event;
uint32_t ost_status;
} AcpiCpuStatus;
diff --git a/docs/specs/acpi_cpu_hotplug.txt b/docs/specs/acpi_cpu_hotplug.txt
index 9bb22d1270..9bd59ae0da 100644
--- a/docs/specs/acpi_cpu_hotplug.txt
+++ b/docs/specs/acpi_cpu_hotplug.txt
@@ -56,8 +56,11 @@ read access:
no device check event to OSPM was issued.
It's valid only when bit 0 is set.
2: Device remove event, used to distinguish device for which
- no device eject request to OSPM was issued.
- 3-7: reserved and should be ignored by OSPM
+ no device eject request to OSPM was issued. Firmware must
+ ignore this bit.
+ 3: reserved and should be ignored by OSPM
+ 4: if set to 1, OSPM requests firmware to perform device eject.
+ 5-7: reserved and should be ignored by OSPM
[0x5-0x7] reserved
[0x8] Command data: (DWORD access)
contains 0 unless value last stored in 'Command field' is one of:
@@ -79,10 +82,16 @@ write access:
selected CPU device
2: if set to 1 clears device remove event, set by OSPM
after it has emitted device eject request for the
- selected CPU device
+ selected CPU device.
3: if set to 1 initiates device eject, set by OSPM when it
- triggers CPU device removal and calls _EJ0 method
- 4-7: reserved, OSPM must clear them before writing to register
+ triggers CPU device removal and calls _EJ0 method or by firmware
+ when bit #4 is set. In case bit #4 were set, it's cleared as
+ part of device eject.
+ 4: if set to 1, OSPM hands over device eject to firmware.
+ Firmware shall issue device eject request as described above
+ (bit #3) and OSPM should not touch device eject bit (#3) in case
+ it's asked firmware to perform CPU device eject.
+ 5-7: reserved, OSPM must clear them before writing to register
[0x5] Command field: (1 byte access)
value:
0: selects a CPU device with inserting/removing events and
diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
index f099b50927..811218f673 100644
--- a/hw/acpi/cpu.c
+++ b/hw/acpi/cpu.c
@@ -71,6 +71,7 @@ static uint64_t cpu_hotplug_rd(void *opaque, hwaddr addr, unsigned size)
val |= cdev->cpu ? 1 : 0;
val |= cdev->is_inserting ? 2 : 0;
val |= cdev->is_removing ? 4 : 0;
+ val |= cdev->fw_remove ? 16 : 0;
trace_cpuhp_acpi_read_flags(cpu_st->selector, val);
break;
case ACPI_CPU_CMD_DATA_OFFSET_RW:
@@ -148,6 +149,14 @@ static void cpu_hotplug_wr(void *opaque, hwaddr addr, uint64_t data,
hotplug_ctrl = qdev_get_hotplug_handler(dev);
hotplug_handler_unplug(hotplug_ctrl, dev, NULL);
object_unparent(OBJECT(dev));
+ cdev->fw_remove = false;
+ } else if (data & 16) {
+ if (!cdev->cpu || cdev->cpu == first_cpu) {
+ trace_cpuhp_acpi_fw_remove_invalid_cpu(cpu_st->selector);
+ break;
+ }
+ trace_cpuhp_acpi_fw_remove_cpu(cpu_st->selector);
+ cdev->fw_remove = true;
}
break;
case ACPI_CPU_CMD_OFFSET_WR:
diff --git a/hw/acpi/trace-events b/hw/acpi/trace-events
index afbc77de1c..f91ced477d 100644
--- a/hw/acpi/trace-events
+++ b/hw/acpi/trace-events
@@ -29,6 +29,8 @@ cpuhp_acpi_clear_inserting_evt(uint32_t idx) "idx[0x%"PRIx32"]"
cpuhp_acpi_clear_remove_evt(uint32_t idx) "idx[0x%"PRIx32"]"
cpuhp_acpi_ejecting_invalid_cpu(uint32_t idx) "0x%"PRIx32
cpuhp_acpi_ejecting_cpu(uint32_t idx) "0x%"PRIx32
+cpuhp_acpi_fw_remove_invalid_cpu(uint32_t idx) "0x%"PRIx32
+cpuhp_acpi_fw_remove_cpu(uint32_t idx) "0x%"PRIx32
cpuhp_acpi_write_ost_ev(uint32_t slot, uint32_t ev) "idx[0x%"PRIx32"] OST EVENT: 0x%"PRIx32
cpuhp_acpi_write_ost_status(uint32_t slot, uint32_t st) "idx[0x%"PRIx32"] OST STATUS: 0x%"PRIx32
--
2.27.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 2/8] acpi: cpuhp: introduce 'firmware performs eject' status/control bits
2020-12-04 17:09 ` [PATCH 2/8] acpi: cpuhp: introduce 'firmware performs eject' status/control bits Igor Mammedov
@ 2020-12-07 6:31 ` Ankur Arora
2020-12-07 8:47 ` Ankur Arora
2020-12-07 12:42 ` Igor Mammedov
0 siblings, 2 replies; 17+ messages in thread
From: Ankur Arora @ 2020-12-07 6:31 UTC (permalink / raw)
To: Igor Mammedov, qemu-devel; +Cc: lersek, mst
On 2020-12-04 9:09 a.m., Igor Mammedov wrote:
> Adds bit #4 to status/control field of CPU hotplug MMIO interface.
> New bit will be used OSPM to mark CPUs as pending for removal by firmware,
> when it calls _EJ0 method on CPU device node. Later on, when firmware
> sees this bit set, it will perform CPU eject which will clear bit #4
> as well.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> v1:
> - rearrange status/control bits description (Laszlo)
> - add clear bit #4 on eject
> - drop toggling logic from bit #4, it can be only set by guest
> and clear as part of cpu eject
> - exclude boot CPU from remove request
> - add trace events for new bit
> ---
> include/hw/acpi/cpu.h | 1 +
> docs/specs/acpi_cpu_hotplug.txt | 19 ++++++++++++++-----
> hw/acpi/cpu.c | 9 +++++++++
> hw/acpi/trace-events | 2 ++
> 4 files changed, 26 insertions(+), 5 deletions(-)
>
> diff --git a/include/hw/acpi/cpu.h b/include/hw/acpi/cpu.h
> index 0eeedaa491..d71edde456 100644
> --- a/include/hw/acpi/cpu.h
> +++ b/include/hw/acpi/cpu.h
> @@ -22,6 +22,7 @@ typedef struct AcpiCpuStatus {
> uint64_t arch_id;
> bool is_inserting;
> bool is_removing;
> + bool fw_remove;
> uint32_t ost_event;
> uint32_t ost_status;
> } AcpiCpuStatus;
> diff --git a/docs/specs/acpi_cpu_hotplug.txt b/docs/specs/acpi_cpu_hotplug.txt
> index 9bb22d1270..9bd59ae0da 100644
> --- a/docs/specs/acpi_cpu_hotplug.txt
> +++ b/docs/specs/acpi_cpu_hotplug.txt
> @@ -56,8 +56,11 @@ read access:
> no device check event to OSPM was issued.
> It's valid only when bit 0 is set.
> 2: Device remove event, used to distinguish device for which
> - no device eject request to OSPM was issued.
> - 3-7: reserved and should be ignored by OSPM
> + no device eject request to OSPM was issued. Firmware must
> + ignore this bit.
> + 3: reserved and should be ignored by OSPM
> + 4: if set to 1, OSPM requests firmware to perform device eject.
> + 5-7: reserved and should be ignored by OSPM
> [0x5-0x7] reserved
> [0x8] Command data: (DWORD access)
> contains 0 unless value last stored in 'Command field' is one of:
> @@ -79,10 +82,16 @@ write access:
> selected CPU device
> 2: if set to 1 clears device remove event, set by OSPM
> after it has emitted device eject request for the
> - selected CPU device
> + selected CPU device.
> 3: if set to 1 initiates device eject, set by OSPM when it
> - triggers CPU device removal and calls _EJ0 method
> - 4-7: reserved, OSPM must clear them before writing to register
> + triggers CPU device removal and calls _EJ0 method or by firmware
> + when bit #4 is set. In case bit #4 were set, it's cleared as
> + part of device eject.
> + 4: if set to 1, OSPM hands over device eject to firmware.
> + Firmware shall issue device eject request as described above
> + (bit #3) and OSPM should not touch device eject bit (#3) in case
> + it's asked firmware to perform CPU device eject.
> + 5-7: reserved, OSPM must clear them before writing to register
> [0x5] Command field: (1 byte access)
> value:
> 0: selects a CPU device with inserting/removing events and
> diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
> index f099b50927..811218f673 100644
> --- a/hw/acpi/cpu.c
> +++ b/hw/acpi/cpu.c
> @@ -71,6 +71,7 @@ static uint64_t cpu_hotplug_rd(void *opaque, hwaddr addr, unsigned size)
> val |= cdev->cpu ? 1 : 0;
> val |= cdev->is_inserting ? 2 : 0;
> val |= cdev->is_removing ? 4 : 0;
> + val |= cdev->fw_remove ? 16 : 0;
> trace_cpuhp_acpi_read_flags(cpu_st->selector, val);
> break;
> case ACPI_CPU_CMD_DATA_OFFSET_RW:
> @@ -148,6 +149,14 @@ static void cpu_hotplug_wr(void *opaque, hwaddr addr, uint64_t data,
> hotplug_ctrl = qdev_get_hotplug_handler(dev);
> hotplug_handler_unplug(hotplug_ctrl, dev, NULL);
> object_unparent(OBJECT(dev));
> + cdev->fw_remove = false;
> + } else if (data & 16) {
> + if (!cdev->cpu || cdev->cpu == first_cpu) {
> + trace_cpuhp_acpi_fw_remove_invalid_cpu(cpu_st->selector);
> + break;
> + }
> + trace_cpuhp_acpi_fw_remove_cpu(cpu_st->selector);
> + cdev->fw_remove = true;
> }
> break;
By the time the firmware gets the MMI, cdev->is_removing == 0. So we probably
need the cdev->fw_remove clause as well.
@@ -168,7 +193,7 @@ static void cpu_hotplug_wr(void *opaque, hwaddr addr, uint64_t data,
do {
cdev = &cpu_st->devs[iter];
- if (cdev->is_inserting || cdev->is_removing) {
+ if (cdev->is_inserting || cdev->is_removing || cdev->fw_remove) {
cpu_st->selector = iter;
trace_cpuhp_acpi_cpu_has_events(cpu_st->selector,
cdev->is_inserting, cdev->is_removing);
Ankur
> case ACPI_CPU_CMD_OFFSET_WR:
> diff --git a/hw/acpi/trace-events b/hw/acpi/trace-events
> index afbc77de1c..f91ced477d 100644
> --- a/hw/acpi/trace-events
> +++ b/hw/acpi/trace-events
> @@ -29,6 +29,8 @@ cpuhp_acpi_clear_inserting_evt(uint32_t idx) "idx[0x%"PRIx32"]"
> cpuhp_acpi_clear_remove_evt(uint32_t idx) "idx[0x%"PRIx32"]"
> cpuhp_acpi_ejecting_invalid_cpu(uint32_t idx) "0x%"PRIx32
> cpuhp_acpi_ejecting_cpu(uint32_t idx) "0x%"PRIx32
> +cpuhp_acpi_fw_remove_invalid_cpu(uint32_t idx) "0x%"PRIx32
> +cpuhp_acpi_fw_remove_cpu(uint32_t idx) "0x%"PRIx32
> cpuhp_acpi_write_ost_ev(uint32_t slot, uint32_t ev) "idx[0x%"PRIx32"] OST EVENT: 0x%"PRIx32
> cpuhp_acpi_write_ost_status(uint32_t slot, uint32_t st) "idx[0x%"PRIx32"] OST STATUS: 0x%"PRIx32
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 2/8] acpi: cpuhp: introduce 'firmware performs eject' status/control bits
2020-12-07 6:31 ` Ankur Arora
@ 2020-12-07 8:47 ` Ankur Arora
2020-12-07 12:48 ` Igor Mammedov
2020-12-07 12:42 ` Igor Mammedov
1 sibling, 1 reply; 17+ messages in thread
From: Ankur Arora @ 2020-12-07 8:47 UTC (permalink / raw)
To: Igor Mammedov, qemu-devel; +Cc: lersek, mst
On 2020-12-06 10:31 p.m., Ankur Arora wrote:
> On 2020-12-04 9:09 a.m., Igor Mammedov wrote:
>> Adds bit #4 to status/control field of CPU hotplug MMIO interface.
>> New bit will be used OSPM to mark CPUs as pending for removal by firmware,
>> when it calls _EJ0 method on CPU device node. Later on, when firmware
>> sees this bit set, it will perform CPU eject which will clear bit #4
>> as well.
>>
>> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
>> ---
>> v1:
>> - rearrange status/control bits description (Laszlo)
>> - add clear bit #4 on eject
>> - drop toggling logic from bit #4, it can be only set by guest
>> and clear as part of cpu eject
>> - exclude boot CPU from remove request
>> - add trace events for new bit
>> ---
>> include/hw/acpi/cpu.h | 1 +
>> docs/specs/acpi_cpu_hotplug.txt | 19 ++++++++++++++-----
>> hw/acpi/cpu.c | 9 +++++++++
>> hw/acpi/trace-events | 2 ++
>> 4 files changed, 26 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/hw/acpi/cpu.h b/include/hw/acpi/cpu.h
>> index 0eeedaa491..d71edde456 100644
>> --- a/include/hw/acpi/cpu.h
>> +++ b/include/hw/acpi/cpu.h
>> @@ -22,6 +22,7 @@ typedef struct AcpiCpuStatus {
>> uint64_t arch_id;
>> bool is_inserting;
>> bool is_removing;
>> + bool fw_remove;
>> uint32_t ost_event;
>> uint32_t ost_status;
>> } AcpiCpuStatus;
>> diff --git a/docs/specs/acpi_cpu_hotplug.txt b/docs/specs/acpi_cpu_hotplug.txt
>> index 9bb22d1270..9bd59ae0da 100644
>> --- a/docs/specs/acpi_cpu_hotplug.txt
>> +++ b/docs/specs/acpi_cpu_hotplug.txt
>> @@ -56,8 +56,11 @@ read access:
>> no device check event to OSPM was issued.
>> It's valid only when bit 0 is set.
>> 2: Device remove event, used to distinguish device for which
>> - no device eject request to OSPM was issued.
>> - 3-7: reserved and should be ignored by OSPM
>> + no device eject request to OSPM was issued. Firmware must
>> + ignore this bit.
>> + 3: reserved and should be ignored by OSPM
>> + 4: if set to 1, OSPM requests firmware to perform device eject.
>> + 5-7: reserved and should be ignored by OSPM
>> [0x5-0x7] reserved
>> [0x8] Command data: (DWORD access)
>> contains 0 unless value last stored in 'Command field' is one of:
>> @@ -79,10 +82,16 @@ write access:
>> selected CPU device
>> 2: if set to 1 clears device remove event, set by OSPM
>> after it has emitted device eject request for the
>> - selected CPU device
>> + selected CPU device.
>> 3: if set to 1 initiates device eject, set by OSPM when it
>> - triggers CPU device removal and calls _EJ0 method
>> - 4-7: reserved, OSPM must clear them before writing to register
>> + triggers CPU device removal and calls _EJ0 method or by firmware
>> + when bit #4 is set. In case bit #4 were set, it's cleared as
>> + part of device eject.
So I spent some time testing my OVMF series alongside this.
Just sent out the code on the EDK2 list.
To do the eject, I'm setting bit#3 after queuing up the processor
for removal (via RemoveProcessor()).
This means, however, that the unplug in QEMU would happen before the
real firmware unplug (which'll happen in SmmCpuUpdate()).
Clearly, the right place for eject would be either in the appropriate
APHandler() or in the BSPHandler() after all the APs have been waited
for but from my reading of related code I don't see any infrastructure
for doing this (admittedly, I don't know the EDK2 source well enough
so it's likely I missed something.)
The other possibility might be to do it in the _EJ0 method itself
after we return from the SMI:
@@ -479,9 +515,8 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
aml_append(method, aml_store(one, fw_ej_evt));
aml_append(method, aml_store(aml_int(OVMF_CPUHP_SMI_CMD),
aml_name("%s", opts.smi_path)));
- } else {
- aml_append(method, aml_store(one, ej_evt));
- }
+ }
+ aml_append(method, aml_store(one, ej_evt));
aml_append(method, aml_release(ctrl_lock));
This seems to work but it is possible I'm missing something here.
Opinions?
Ankur
>> + 4: if set to 1, OSPM hands over device eject to firmware.
>> + Firmware shall issue device eject request as described above
>> + (bit #3) and OSPM should not touch device eject bit (#3) in case
>> + it's asked firmware to perform CPU device eject.
>> + 5-7: reserved, OSPM must clear them before writing to register
>> [0x5] Command field: (1 byte access)
>> value:
>> 0: selects a CPU device with inserting/removing events and
>> diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
>> index f099b50927..811218f673 100644
>> --- a/hw/acpi/cpu.c
>> +++ b/hw/acpi/cpu.c
>> @@ -71,6 +71,7 @@ static uint64_t cpu_hotplug_rd(void *opaque, hwaddr addr, unsigned size)
>> val |= cdev->cpu ? 1 : 0;
>> val |= cdev->is_inserting ? 2 : 0;
>> val |= cdev->is_removing ? 4 : 0;
>> + val |= cdev->fw_remove ? 16 : 0;
>> trace_cpuhp_acpi_read_flags(cpu_st->selector, val);
>> break;
>> case ACPI_CPU_CMD_DATA_OFFSET_RW:
>> @@ -148,6 +149,14 @@ static void cpu_hotplug_wr(void *opaque, hwaddr addr, uint64_t data,
>> hotplug_ctrl = qdev_get_hotplug_handler(dev);
>> hotplug_handler_unplug(hotplug_ctrl, dev, NULL);
>> object_unparent(OBJECT(dev));
>> + cdev->fw_remove = false;
>> + } else if (data & 16) {
>> + if (!cdev->cpu || cdev->cpu == first_cpu) {
>> + trace_cpuhp_acpi_fw_remove_invalid_cpu(cpu_st->selector);
>> + break;
>> + }
>> + trace_cpuhp_acpi_fw_remove_cpu(cpu_st->selector);
>> + cdev->fw_remove = true;
>> }
>> break;
>
> By the time the firmware gets the MMI, cdev->is_removing == 0. So we probably
> need the cdev->fw_remove clause as well.
>
> @@ -168,7 +193,7 @@ static void cpu_hotplug_wr(void *opaque, hwaddr addr, uint64_t data,
>
> do {
> cdev = &cpu_st->devs[iter];
> - if (cdev->is_inserting || cdev->is_removing) {
> + if (cdev->is_inserting || cdev->is_removing || cdev->fw_remove) {
> cpu_st->selector = iter;
> trace_cpuhp_acpi_cpu_has_events(cpu_st->selector,
> cdev->is_inserting, cdev->is_removing);
>
>
> Ankur
>
>> case ACPI_CPU_CMD_OFFSET_WR:
>> diff --git a/hw/acpi/trace-events b/hw/acpi/trace-events
>> index afbc77de1c..f91ced477d 100644
>> --- a/hw/acpi/trace-events
>> +++ b/hw/acpi/trace-events
>> @@ -29,6 +29,8 @@ cpuhp_acpi_clear_inserting_evt(uint32_t idx) "idx[0x%"PRIx32"]"
>> cpuhp_acpi_clear_remove_evt(uint32_t idx) "idx[0x%"PRIx32"]"
>> cpuhp_acpi_ejecting_invalid_cpu(uint32_t idx) "0x%"PRIx32
>> cpuhp_acpi_ejecting_cpu(uint32_t idx) "0x%"PRIx32
>> +cpuhp_acpi_fw_remove_invalid_cpu(uint32_t idx) "0x%"PRIx32
>> +cpuhp_acpi_fw_remove_cpu(uint32_t idx) "0x%"PRIx32
>> cpuhp_acpi_write_ost_ev(uint32_t slot, uint32_t ev) "idx[0x%"PRIx32"] OST EVENT: 0x%"PRIx32
>> cpuhp_acpi_write_ost_status(uint32_t slot, uint32_t st) "idx[0x%"PRIx32"] OST STATUS: 0x%"PRIx32
>>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 2/8] acpi: cpuhp: introduce 'firmware performs eject' status/control bits
2020-12-07 8:47 ` Ankur Arora
@ 2020-12-07 12:48 ` Igor Mammedov
2020-12-07 19:01 ` Ankur Arora
0 siblings, 1 reply; 17+ messages in thread
From: Igor Mammedov @ 2020-12-07 12:48 UTC (permalink / raw)
To: Ankur Arora; +Cc: lersek, qemu-devel, mst
On Mon, 7 Dec 2020 00:47:13 -0800
Ankur Arora <ankur.a.arora@oracle.com> wrote:
> On 2020-12-06 10:31 p.m., Ankur Arora wrote:
> > On 2020-12-04 9:09 a.m., Igor Mammedov wrote:
> >> Adds bit #4 to status/control field of CPU hotplug MMIO interface.
> >> New bit will be used OSPM to mark CPUs as pending for removal by firmware,
> >> when it calls _EJ0 method on CPU device node. Later on, when firmware
> >> sees this bit set, it will perform CPU eject which will clear bit #4
> >> as well.
> >>
> >> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> >> ---
> >> v1:
> >> - rearrange status/control bits description (Laszlo)
> >> - add clear bit #4 on eject
> >> - drop toggling logic from bit #4, it can be only set by guest
> >> and clear as part of cpu eject
> >> - exclude boot CPU from remove request
> >> - add trace events for new bit
> >> ---
> >> include/hw/acpi/cpu.h | 1 +
> >> docs/specs/acpi_cpu_hotplug.txt | 19 ++++++++++++++-----
> >> hw/acpi/cpu.c | 9 +++++++++
> >> hw/acpi/trace-events | 2 ++
> >> 4 files changed, 26 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/include/hw/acpi/cpu.h b/include/hw/acpi/cpu.h
> >> index 0eeedaa491..d71edde456 100644
> >> --- a/include/hw/acpi/cpu.h
> >> +++ b/include/hw/acpi/cpu.h
> >> @@ -22,6 +22,7 @@ typedef struct AcpiCpuStatus {
> >> uint64_t arch_id;
> >> bool is_inserting;
> >> bool is_removing;
> >> + bool fw_remove;
> >> uint32_t ost_event;
> >> uint32_t ost_status;
> >> } AcpiCpuStatus;
> >> diff --git a/docs/specs/acpi_cpu_hotplug.txt b/docs/specs/acpi_cpu_hotplug.txt
> >> index 9bb22d1270..9bd59ae0da 100644
> >> --- a/docs/specs/acpi_cpu_hotplug.txt
> >> +++ b/docs/specs/acpi_cpu_hotplug.txt
> >> @@ -56,8 +56,11 @@ read access:
> >> no device check event to OSPM was issued.
> >> It's valid only when bit 0 is set.
> >> 2: Device remove event, used to distinguish device for which
> >> - no device eject request to OSPM was issued.
> >> - 3-7: reserved and should be ignored by OSPM
> >> + no device eject request to OSPM was issued. Firmware must
> >> + ignore this bit.
> >> + 3: reserved and should be ignored by OSPM
> >> + 4: if set to 1, OSPM requests firmware to perform device eject.
> >> + 5-7: reserved and should be ignored by OSPM
> >> [0x5-0x7] reserved
> >> [0x8] Command data: (DWORD access)
> >> contains 0 unless value last stored in 'Command field' is one of:
> >> @@ -79,10 +82,16 @@ write access:
> >> selected CPU device
> >> 2: if set to 1 clears device remove event, set by OSPM
> >> after it has emitted device eject request for the
> >> - selected CPU device
> >> + selected CPU device.
> >> 3: if set to 1 initiates device eject, set by OSPM when it
> >> - triggers CPU device removal and calls _EJ0 method
> >> - 4-7: reserved, OSPM must clear them before writing to register
> >> + triggers CPU device removal and calls _EJ0 method or by firmware
> >> + when bit #4 is set. In case bit #4 were set, it's cleared as
> >> + part of device eject.
>
> So I spent some time testing my OVMF series alongside this.
> Just sent out the code on the EDK2 list.
>
> To do the eject, I'm setting bit#3 after queuing up the processor
> for removal (via RemoveProcessor()).
>
> This means, however, that the unplug in QEMU would happen before the
> real firmware unplug (which'll happen in SmmCpuUpdate()).
>
> Clearly, the right place for eject would be either in the appropriate
> APHandler() or in the BSPHandler() after all the APs have been waited
> for but from my reading of related code I don't see any infrastructure
> for doing this (admittedly, I don't know the EDK2 source well enough
> so it's likely I missed something.)
>
> The other possibility might be to do it in the _EJ0 method itself
> after we return from the SMI:
>
> @@ -479,9 +515,8 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
> aml_append(method, aml_store(one, fw_ej_evt));
> aml_append(method, aml_store(aml_int(OVMF_CPUHP_SMI_CMD),
> aml_name("%s", opts.smi_path)));
> - } else {
> - aml_append(method, aml_store(one, ej_evt));
> - }
> + }
> + aml_append(method, aml_store(one, ej_evt));
> aml_append(method, aml_release(ctrl_lock));
>
> This seems to work but it is possible I'm missing something here.
theoretically this leaves unaccounted CPU on fw side,
what if SMM is entered again before CPU is ejected or OS doesn't eject it on purpose?
I'd prefer firmware to remove CPU before returning from SMM.
>
>
> Opinions?
>
> Ankur
>
> >> + 4: if set to 1, OSPM hands over device eject to firmware.
> >> + Firmware shall issue device eject request as described above
> >> + (bit #3) and OSPM should not touch device eject bit (#3) in case
> >> + it's asked firmware to perform CPU device eject.
> >> + 5-7: reserved, OSPM must clear them before writing to register
> >> [0x5] Command field: (1 byte access)
> >> value:
> >> 0: selects a CPU device with inserting/removing events and
> >> diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
> >> index f099b50927..811218f673 100644
> >> --- a/hw/acpi/cpu.c
> >> +++ b/hw/acpi/cpu.c
> >> @@ -71,6 +71,7 @@ static uint64_t cpu_hotplug_rd(void *opaque, hwaddr addr, unsigned size)
> >> val |= cdev->cpu ? 1 : 0;
> >> val |= cdev->is_inserting ? 2 : 0;
> >> val |= cdev->is_removing ? 4 : 0;
> >> + val |= cdev->fw_remove ? 16 : 0;
> >> trace_cpuhp_acpi_read_flags(cpu_st->selector, val);
> >> break;
> >> case ACPI_CPU_CMD_DATA_OFFSET_RW:
> >> @@ -148,6 +149,14 @@ static void cpu_hotplug_wr(void *opaque, hwaddr addr, uint64_t data,
> >> hotplug_ctrl = qdev_get_hotplug_handler(dev);
> >> hotplug_handler_unplug(hotplug_ctrl, dev, NULL);
> >> object_unparent(OBJECT(dev));
> >> + cdev->fw_remove = false;
> >> + } else if (data & 16) {
> >> + if (!cdev->cpu || cdev->cpu == first_cpu) {
> >> + trace_cpuhp_acpi_fw_remove_invalid_cpu(cpu_st->selector);
> >> + break;
> >> + }
> >> + trace_cpuhp_acpi_fw_remove_cpu(cpu_st->selector);
> >> + cdev->fw_remove = true;
> >> }
> >> break;
> >
> > By the time the firmware gets the MMI, cdev->is_removing == 0. So we probably
> > need the cdev->fw_remove clause as well.
> >
> > @@ -168,7 +193,7 @@ static void cpu_hotplug_wr(void *opaque, hwaddr addr, uint64_t data,
> >
> > do {
> > cdev = &cpu_st->devs[iter];
> > - if (cdev->is_inserting || cdev->is_removing) {
> > + if (cdev->is_inserting || cdev->is_removing || cdev->fw_remove) {
> > cpu_st->selector = iter;
> > trace_cpuhp_acpi_cpu_has_events(cpu_st->selector,
> > cdev->is_inserting, cdev->is_removing);
> >
> >
> > Ankur
> >
> >> case ACPI_CPU_CMD_OFFSET_WR:
> >> diff --git a/hw/acpi/trace-events b/hw/acpi/trace-events
> >> index afbc77de1c..f91ced477d 100644
> >> --- a/hw/acpi/trace-events
> >> +++ b/hw/acpi/trace-events
> >> @@ -29,6 +29,8 @@ cpuhp_acpi_clear_inserting_evt(uint32_t idx) "idx[0x%"PRIx32"]"
> >> cpuhp_acpi_clear_remove_evt(uint32_t idx) "idx[0x%"PRIx32"]"
> >> cpuhp_acpi_ejecting_invalid_cpu(uint32_t idx) "0x%"PRIx32
> >> cpuhp_acpi_ejecting_cpu(uint32_t idx) "0x%"PRIx32
> >> +cpuhp_acpi_fw_remove_invalid_cpu(uint32_t idx) "0x%"PRIx32
> >> +cpuhp_acpi_fw_remove_cpu(uint32_t idx) "0x%"PRIx32
> >> cpuhp_acpi_write_ost_ev(uint32_t slot, uint32_t ev) "idx[0x%"PRIx32"] OST EVENT: 0x%"PRIx32
> >> cpuhp_acpi_write_ost_status(uint32_t slot, uint32_t st) "idx[0x%"PRIx32"] OST STATUS: 0x%"PRIx32
> >>
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 2/8] acpi: cpuhp: introduce 'firmware performs eject' status/control bits
2020-12-07 12:48 ` Igor Mammedov
@ 2020-12-07 19:01 ` Ankur Arora
0 siblings, 0 replies; 17+ messages in thread
From: Ankur Arora @ 2020-12-07 19:01 UTC (permalink / raw)
To: Igor Mammedov; +Cc: lersek, qemu-devel, mst
On 2020-12-07 4:48 a.m., Igor Mammedov wrote:
> On Mon, 7 Dec 2020 00:47:13 -0800
> Ankur Arora <ankur.a.arora@oracle.com> wrote:
>
>> On 2020-12-06 10:31 p.m., Ankur Arora wrote:
>>> On 2020-12-04 9:09 a.m., Igor Mammedov wrote:
>>>> Adds bit #4 to status/control field of CPU hotplug MMIO interface.
>>>> New bit will be used OSPM to mark CPUs as pending for removal by firmware,
>>>> when it calls _EJ0 method on CPU device node. Later on, when firmware
>>>> sees this bit set, it will perform CPU eject which will clear bit #4
>>>> as well.
>>>>
>>>> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
>>>> ---
>>>> v1:
>>>> - rearrange status/control bits description (Laszlo)
>>>> - add clear bit #4 on eject
>>>> - drop toggling logic from bit #4, it can be only set by guest
>>>> and clear as part of cpu eject
>>>> - exclude boot CPU from remove request
>>>> - add trace events for new bit
>>>> ---
>>>> include/hw/acpi/cpu.h | 1 +
>>>> docs/specs/acpi_cpu_hotplug.txt | 19 ++++++++++++++-----
>>>> hw/acpi/cpu.c | 9 +++++++++
>>>> hw/acpi/trace-events | 2 ++
>>>> 4 files changed, 26 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/include/hw/acpi/cpu.h b/include/hw/acpi/cpu.h
>>>> index 0eeedaa491..d71edde456 100644
>>>> --- a/include/hw/acpi/cpu.h
>>>> +++ b/include/hw/acpi/cpu.h
>>>> @@ -22,6 +22,7 @@ typedef struct AcpiCpuStatus {
>>>> uint64_t arch_id;
>>>> bool is_inserting;
>>>> bool is_removing;
>>>> + bool fw_remove;
>>>> uint32_t ost_event;
>>>> uint32_t ost_status;
>>>> } AcpiCpuStatus;
>>>> diff --git a/docs/specs/acpi_cpu_hotplug.txt b/docs/specs/acpi_cpu_hotplug.txt
>>>> index 9bb22d1270..9bd59ae0da 100644
>>>> --- a/docs/specs/acpi_cpu_hotplug.txt
>>>> +++ b/docs/specs/acpi_cpu_hotplug.txt
>>>> @@ -56,8 +56,11 @@ read access:
>>>> no device check event to OSPM was issued.
>>>> It's valid only when bit 0 is set.
>>>> 2: Device remove event, used to distinguish device for which
>>>> - no device eject request to OSPM was issued.
>>>> - 3-7: reserved and should be ignored by OSPM
>>>> + no device eject request to OSPM was issued. Firmware must
>>>> + ignore this bit.
>>>> + 3: reserved and should be ignored by OSPM
>>>> + 4: if set to 1, OSPM requests firmware to perform device eject.
>>>> + 5-7: reserved and should be ignored by OSPM
>>>> [0x5-0x7] reserved
>>>> [0x8] Command data: (DWORD access)
>>>> contains 0 unless value last stored in 'Command field' is one of:
>>>> @@ -79,10 +82,16 @@ write access:
>>>> selected CPU device
>>>> 2: if set to 1 clears device remove event, set by OSPM
>>>> after it has emitted device eject request for the
>>>> - selected CPU device
>>>> + selected CPU device.
>>>> 3: if set to 1 initiates device eject, set by OSPM when it
>>>> - triggers CPU device removal and calls _EJ0 method
>>>> - 4-7: reserved, OSPM must clear them before writing to register
>>>> + triggers CPU device removal and calls _EJ0 method or by firmware
>>>> + when bit #4 is set. In case bit #4 were set, it's cleared as
>>>> + part of device eject.
>>
>> So I spent some time testing my OVMF series alongside this.
>> Just sent out the code on the EDK2 list.
>>
>> To do the eject, I'm setting bit#3 after queuing up the processor
>> for removal (via RemoveProcessor()).
>>
>> This means, however, that the unplug in QEMU would happen before the
>> real firmware unplug (which'll happen in SmmCpuUpdate()).
>>
>> Clearly, the right place for eject would be either in the appropriate
>> APHandler() or in the BSPHandler() after all the APs have been waited
>> for but from my reading of related code I don't see any infrastructure
>> for doing this (admittedly, I don't know the EDK2 source well enough
>> so it's likely I missed something.)
>>
>> The other possibility might be to do it in the _EJ0 method itself
>> after we return from the SMI:
>>
>> @@ -479,9 +515,8 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
>> aml_append(method, aml_store(one, fw_ej_evt));
>> aml_append(method, aml_store(aml_int(OVMF_CPUHP_SMI_CMD),
>> aml_name("%s", opts.smi_path)));
>> - } else {
>> - aml_append(method, aml_store(one, ej_evt));
>> - }
>> + }
>> + aml_append(method, aml_store(one, ej_evt));
>> aml_append(method, aml_release(ctrl_lock));
>>
>> This seems to work but it is possible I'm missing something here.
>
> theoretically this leaves unaccounted CPU on fw side,
On the guest side right? As in the firmware has marked it gone but it's not
really gone.
> what if SMM is entered again before CPU is ejected or OS doesn't eject it on purpose?
Yeah both of those things would be a mess. I'm not even sure it would enter the
SMM again -- given that the SMM has marked it gone.
>
> I'd prefer firmware to remove CPU before returning from SMM.
That would be ideal though I don't yet see a mechanism for doing that. Laszlo
might have better ideas though.
Another possibility that might address your concerns would be deferred removal.
Need to work out the details, but here's a sketch of what I'm thinking:
When the firmware writes to bit#3, QEMU marks the CPU for deferred removal.
And, when the firmware exits from the SMI handler, we force a VMEXIT before
returning to the guest. And, as part of this VMEXIT, QEMU does the
actual unplug.
Ankur
>
>
>>
>>
>> Opinions?
>>
>> Ankur
>>
>>>> + 4: if set to 1, OSPM hands over device eject to firmware.
>>>> + Firmware shall issue device eject request as described above
>>>> + (bit #3) and OSPM should not touch device eject bit (#3) in case
>>>> + it's asked firmware to perform CPU device eject.
>>>> + 5-7: reserved, OSPM must clear them before writing to register
>>>> [0x5] Command field: (1 byte access)
>>>> value:
>>>> 0: selects a CPU device with inserting/removing events and
>>>> diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
>>>> index f099b50927..811218f673 100644
>>>> --- a/hw/acpi/cpu.c
>>>> +++ b/hw/acpi/cpu.c
>>>> @@ -71,6 +71,7 @@ static uint64_t cpu_hotplug_rd(void *opaque, hwaddr addr, unsigned size)
>>>> val |= cdev->cpu ? 1 : 0;
>>>> val |= cdev->is_inserting ? 2 : 0;
>>>> val |= cdev->is_removing ? 4 : 0;
>>>> + val |= cdev->fw_remove ? 16 : 0;
>>>> trace_cpuhp_acpi_read_flags(cpu_st->selector, val);
>>>> break;
>>>> case ACPI_CPU_CMD_DATA_OFFSET_RW:
>>>> @@ -148,6 +149,14 @@ static void cpu_hotplug_wr(void *opaque, hwaddr addr, uint64_t data,
>>>> hotplug_ctrl = qdev_get_hotplug_handler(dev);
>>>> hotplug_handler_unplug(hotplug_ctrl, dev, NULL);
>>>> object_unparent(OBJECT(dev));
>>>> + cdev->fw_remove = false;
>>>> + } else if (data & 16) {
>>>> + if (!cdev->cpu || cdev->cpu == first_cpu) {
>>>> + trace_cpuhp_acpi_fw_remove_invalid_cpu(cpu_st->selector);
>>>> + break;
>>>> + }
>>>> + trace_cpuhp_acpi_fw_remove_cpu(cpu_st->selector);
>>>> + cdev->fw_remove = true;
>>>> }
>>>> break;
>>>
>>> By the time the firmware gets the MMI, cdev->is_removing == 0. So we probably
>>> need the cdev->fw_remove clause as well.
>>>
>>> @@ -168,7 +193,7 @@ static void cpu_hotplug_wr(void *opaque, hwaddr addr, uint64_t data,
>>>
>>> do {
>>> cdev = &cpu_st->devs[iter];
>>> - if (cdev->is_inserting || cdev->is_removing) {
>>> + if (cdev->is_inserting || cdev->is_removing || cdev->fw_remove) {
>>> cpu_st->selector = iter;
>>> trace_cpuhp_acpi_cpu_has_events(cpu_st->selector,
>>> cdev->is_inserting, cdev->is_removing);
>>>
>>>
>>> Ankur
>>>
>>>> case ACPI_CPU_CMD_OFFSET_WR:
>>>> diff --git a/hw/acpi/trace-events b/hw/acpi/trace-events
>>>> index afbc77de1c..f91ced477d 100644
>>>> --- a/hw/acpi/trace-events
>>>> +++ b/hw/acpi/trace-events
>>>> @@ -29,6 +29,8 @@ cpuhp_acpi_clear_inserting_evt(uint32_t idx) "idx[0x%"PRIx32"]"
>>>> cpuhp_acpi_clear_remove_evt(uint32_t idx) "idx[0x%"PRIx32"]"
>>>> cpuhp_acpi_ejecting_invalid_cpu(uint32_t idx) "0x%"PRIx32
>>>> cpuhp_acpi_ejecting_cpu(uint32_t idx) "0x%"PRIx32
>>>> +cpuhp_acpi_fw_remove_invalid_cpu(uint32_t idx) "0x%"PRIx32
>>>> +cpuhp_acpi_fw_remove_cpu(uint32_t idx) "0x%"PRIx32
>>>> cpuhp_acpi_write_ost_ev(uint32_t slot, uint32_t ev) "idx[0x%"PRIx32"] OST EVENT: 0x%"PRIx32
>>>> cpuhp_acpi_write_ost_status(uint32_t slot, uint32_t st) "idx[0x%"PRIx32"] OST STATUS: 0x%"PRIx32
>>>>
>>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/8] acpi: cpuhp: introduce 'firmware performs eject' status/control bits
2020-12-07 6:31 ` Ankur Arora
2020-12-07 8:47 ` Ankur Arora
@ 2020-12-07 12:42 ` Igor Mammedov
1 sibling, 0 replies; 17+ messages in thread
From: Igor Mammedov @ 2020-12-07 12:42 UTC (permalink / raw)
To: Ankur Arora; +Cc: lersek, qemu-devel, mst
On Sun, 6 Dec 2020 22:31:50 -0800
Ankur Arora <ankur.a.arora@oracle.com> wrote:
> On 2020-12-04 9:09 a.m., Igor Mammedov wrote:
> > Adds bit #4 to status/control field of CPU hotplug MMIO interface.
> > New bit will be used OSPM to mark CPUs as pending for removal by firmware,
> > when it calls _EJ0 method on CPU device node. Later on, when firmware
> > sees this bit set, it will perform CPU eject which will clear bit #4
> > as well.
> >
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> > v1:
> > - rearrange status/control bits description (Laszlo)
> > - add clear bit #4 on eject
> > - drop toggling logic from bit #4, it can be only set by guest
> > and clear as part of cpu eject
> > - exclude boot CPU from remove request
> > - add trace events for new bit
> > ---
> > include/hw/acpi/cpu.h | 1 +
> > docs/specs/acpi_cpu_hotplug.txt | 19 ++++++++++++++-----
> > hw/acpi/cpu.c | 9 +++++++++
> > hw/acpi/trace-events | 2 ++
> > 4 files changed, 26 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/hw/acpi/cpu.h b/include/hw/acpi/cpu.h
> > index 0eeedaa491..d71edde456 100644
> > --- a/include/hw/acpi/cpu.h
> > +++ b/include/hw/acpi/cpu.h
> > @@ -22,6 +22,7 @@ typedef struct AcpiCpuStatus {
> > uint64_t arch_id;
> > bool is_inserting;
> > bool is_removing;
> > + bool fw_remove;
> > uint32_t ost_event;
> > uint32_t ost_status;
> > } AcpiCpuStatus;
> > diff --git a/docs/specs/acpi_cpu_hotplug.txt b/docs/specs/acpi_cpu_hotplug.txt
> > index 9bb22d1270..9bd59ae0da 100644
> > --- a/docs/specs/acpi_cpu_hotplug.txt
> > +++ b/docs/specs/acpi_cpu_hotplug.txt
> > @@ -56,8 +56,11 @@ read access:
> > no device check event to OSPM was issued.
> > It's valid only when bit 0 is set.
> > 2: Device remove event, used to distinguish device for which
> > - no device eject request to OSPM was issued.
> > - 3-7: reserved and should be ignored by OSPM
> > + no device eject request to OSPM was issued. Firmware must
> > + ignore this bit.
> > + 3: reserved and should be ignored by OSPM
> > + 4: if set to 1, OSPM requests firmware to perform device eject.
> > + 5-7: reserved and should be ignored by OSPM
> > [0x5-0x7] reserved
> > [0x8] Command data: (DWORD access)
> > contains 0 unless value last stored in 'Command field' is one of:
> > @@ -79,10 +82,16 @@ write access:
> > selected CPU device
> > 2: if set to 1 clears device remove event, set by OSPM
> > after it has emitted device eject request for the
> > - selected CPU device
> > + selected CPU device.
> > 3: if set to 1 initiates device eject, set by OSPM when it
> > - triggers CPU device removal and calls _EJ0 method
> > - 4-7: reserved, OSPM must clear them before writing to register
> > + triggers CPU device removal and calls _EJ0 method or by firmware
> > + when bit #4 is set. In case bit #4 were set, it's cleared as
> > + part of device eject.
> > + 4: if set to 1, OSPM hands over device eject to firmware.
> > + Firmware shall issue device eject request as described above
> > + (bit #3) and OSPM should not touch device eject bit (#3) in case
> > + it's asked firmware to perform CPU device eject.
> > + 5-7: reserved, OSPM must clear them before writing to register
> > [0x5] Command field: (1 byte access)
> > value:
> > 0: selects a CPU device with inserting/removing events and
> > diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
> > index f099b50927..811218f673 100644
> > --- a/hw/acpi/cpu.c
> > +++ b/hw/acpi/cpu.c
> > @@ -71,6 +71,7 @@ static uint64_t cpu_hotplug_rd(void *opaque, hwaddr addr, unsigned size)
> > val |= cdev->cpu ? 1 : 0;
> > val |= cdev->is_inserting ? 2 : 0;
> > val |= cdev->is_removing ? 4 : 0;
> > + val |= cdev->fw_remove ? 16 : 0;
> > trace_cpuhp_acpi_read_flags(cpu_st->selector, val);
> > break;
> > case ACPI_CPU_CMD_DATA_OFFSET_RW:
> > @@ -148,6 +149,14 @@ static void cpu_hotplug_wr(void *opaque, hwaddr addr, uint64_t data,
> > hotplug_ctrl = qdev_get_hotplug_handler(dev);
> > hotplug_handler_unplug(hotplug_ctrl, dev, NULL);
> > object_unparent(OBJECT(dev));
> > + cdev->fw_remove = false;
> > + } else if (data & 16) {
> > + if (!cdev->cpu || cdev->cpu == first_cpu) {
> > + trace_cpuhp_acpi_fw_remove_invalid_cpu(cpu_st->selector);
> > + break;
> > + }
> > + trace_cpuhp_acpi_fw_remove_cpu(cpu_st->selector);
> > + cdev->fw_remove = true;
> > }
> > break;
>
> By the time the firmware gets the MMI, cdev->is_removing == 0. So we probably
> need the cdev->fw_remove clause as well.
indeed, I'll fix in in v2
>
> @@ -168,7 +193,7 @@ static void cpu_hotplug_wr(void *opaque, hwaddr addr, uint64_t data,
>
> do {
> cdev = &cpu_st->devs[iter];
> - if (cdev->is_inserting || cdev->is_removing) {
> + if (cdev->is_inserting || cdev->is_removing || cdev->fw_remove) {
> cpu_st->selector = iter;
> trace_cpuhp_acpi_cpu_has_events(cpu_st->selector,
> cdev->is_inserting, cdev->is_removing);
>
>
> Ankur
>
> > case ACPI_CPU_CMD_OFFSET_WR:
> > diff --git a/hw/acpi/trace-events b/hw/acpi/trace-events
> > index afbc77de1c..f91ced477d 100644
> > --- a/hw/acpi/trace-events
> > +++ b/hw/acpi/trace-events
> > @@ -29,6 +29,8 @@ cpuhp_acpi_clear_inserting_evt(uint32_t idx) "idx[0x%"PRIx32"]"
> > cpuhp_acpi_clear_remove_evt(uint32_t idx) "idx[0x%"PRIx32"]"
> > cpuhp_acpi_ejecting_invalid_cpu(uint32_t idx) "0x%"PRIx32
> > cpuhp_acpi_ejecting_cpu(uint32_t idx) "0x%"PRIx32
> > +cpuhp_acpi_fw_remove_invalid_cpu(uint32_t idx) "0x%"PRIx32
> > +cpuhp_acpi_fw_remove_cpu(uint32_t idx) "0x%"PRIx32
> > cpuhp_acpi_write_ost_ev(uint32_t slot, uint32_t ev) "idx[0x%"PRIx32"] OST EVENT: 0x%"PRIx32
> > cpuhp_acpi_write_ost_status(uint32_t slot, uint32_t st) "idx[0x%"PRIx32"] OST STATUS: 0x%"PRIx32
> >
> >
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/8] x86: acpi: introduce AcpiPmInfo::smi_on_cpu_unplug
2020-12-04 17:09 [PATCH 0/8] add support for cpu hot-unplug with SMI broadcast enabled Igor Mammedov
2020-12-04 17:09 ` [PATCH 1/8] hw: add compat machines for 6.0 Igor Mammedov
2020-12-04 17:09 ` [PATCH 2/8] acpi: cpuhp: introduce 'firmware performs eject' status/control bits Igor Mammedov
@ 2020-12-04 17:09 ` Igor Mammedov
2020-12-04 17:09 ` [PATCH 4/8] tests/acpi: allow expected files change Igor Mammedov
` (4 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Igor Mammedov @ 2020-12-04 17:09 UTC (permalink / raw)
To: qemu-devel; +Cc: lersek, ankur.a.arora, mst
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
hw/i386/acpi-build.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 1f5c211245..9036e5594c 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -96,6 +96,7 @@ typedef struct AcpiPmInfo {
bool s4_disabled;
bool pcihp_bridge_en;
bool smi_on_cpuhp;
+ bool smi_on_cpu_unplug;
bool pcihp_root_en;
uint8_t s4_val;
AcpiFadtData fadt;
@@ -197,6 +198,7 @@ static void acpi_get_pm_info(MachineState *machine, AcpiPmInfo *pm)
pm->pcihp_io_base = 0;
pm->pcihp_io_len = 0;
pm->smi_on_cpuhp = false;
+ pm->smi_on_cpu_unplug = false;
assert(obj);
init_common_fadt_data(machine, obj, &pm->fadt);
@@ -220,6 +222,8 @@ static void acpi_get_pm_info(MachineState *machine, AcpiPmInfo *pm)
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));
+ pm->smi_on_cpu_unplug =
+ !!(smi_features & BIT_ULL(ICH9_LPC_SMI_F_CPU_HOT_UNPLUG_BIT));
}
/* The above need not be conditional on machine type because the reset port
--
2.27.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 4/8] tests/acpi: allow expected files change
2020-12-04 17:09 [PATCH 0/8] add support for cpu hot-unplug with SMI broadcast enabled Igor Mammedov
` (2 preceding siblings ...)
2020-12-04 17:09 ` [PATCH 3/8] x86: acpi: introduce AcpiPmInfo::smi_on_cpu_unplug Igor Mammedov
@ 2020-12-04 17:09 ` Igor Mammedov
2020-12-04 17:09 ` [PATCH 5/8] x86: acpi: let the firmware handle pending "CPU remove" events in SMM Igor Mammedov
` (3 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Igor Mammedov @ 2020-12-04 17:09 UTC (permalink / raw)
To: qemu-devel; +Cc: lersek, ankur.a.arora, mst
Change that will be introduced by following patch:
@@ -557,6 +557,8 @@ DefinitionBlock ("", "DSDT", 1, "BOCHS ", "BXPCDSDT", 0x00000001)
CINS, 1,
CRMV, 1,
CEJ0, 1,
+ , 1,
+ CEJF, 1,
Offset (0x05),
CCMD, 8
}
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
tests/qtest/bios-tables-test-allowed-diff.h | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523c8b..cc75f3fc46 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1 +1,22 @@
/* 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.roothp",
+"tests/data/acpi/pc/DSDT.hpbridge",
+"tests/data/acpi/pc/DSDT.hpbrroot",
--
2.27.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 5/8] x86: acpi: let the firmware handle pending "CPU remove" events in SMM
2020-12-04 17:09 [PATCH 0/8] add support for cpu hot-unplug with SMI broadcast enabled Igor Mammedov
` (3 preceding siblings ...)
2020-12-04 17:09 ` [PATCH 4/8] tests/acpi: allow expected files change Igor Mammedov
@ 2020-12-04 17:09 ` Igor Mammedov
2020-12-07 6:20 ` Ankur Arora
2020-12-04 17:09 ` [PATCH 6/8] tests/acpi: update expected files Igor Mammedov
` (2 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Igor Mammedov @ 2020-12-04 17:09 UTC (permalink / raw)
To: qemu-devel; +Cc: lersek, ankur.a.arora, mst
if firmware and QEMU negotiated CPU hotunplug support, generate
_EJ0 method so that it will mark CPU for removal by firmware and
pass control to it by triggering SMI.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
include/hw/acpi/cpu.h | 1 +
hw/acpi/cpu.c | 15 +++++++++++++--
hw/i386/acpi-build.c | 1 +
3 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/include/hw/acpi/cpu.h b/include/hw/acpi/cpu.h
index d71edde456..999caaf510 100644
--- a/include/hw/acpi/cpu.h
+++ b/include/hw/acpi/cpu.h
@@ -51,6 +51,7 @@ void cpu_hotplug_hw_init(MemoryRegion *as, Object *owner,
typedef struct CPUHotplugFeatures {
bool acpi_1_compatible;
bool has_legacy_cphp;
+ bool fw_unplugs_cpu;
const char *smi_path;
} CPUHotplugFeatures;
diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
index 811218f673..bded2a837f 100644
--- a/hw/acpi/cpu.c
+++ b/hw/acpi/cpu.c
@@ -341,6 +341,7 @@ const VMStateDescription vmstate_cpu_hotplug = {
#define CPU_INSERT_EVENT "CINS"
#define CPU_REMOVE_EVENT "CRMV"
#define CPU_EJECT_EVENT "CEJ0"
+#define CPU_FW_EJECT_EVENT "CEJF"
void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
hwaddr io_base,
@@ -393,7 +394,10 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
aml_append(field, aml_named_field(CPU_REMOVE_EVENT, 1));
/* initiates device eject, write only */
aml_append(field, aml_named_field(CPU_EJECT_EVENT, 1));
- aml_append(field, aml_reserved_field(4));
+ aml_append(field, aml_reserved_field(1));
+ /* tell firmware to do device eject, write only */
+ aml_append(field, aml_named_field(CPU_FW_EJECT_EVENT, 1));
+ aml_append(field, aml_reserved_field(2));
aml_append(field, aml_named_field(CPU_COMMAND, 8));
aml_append(cpu_ctrl_dev, field);
@@ -428,6 +432,7 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
Aml *ins_evt = aml_name("%s.%s", cphp_res_path, CPU_INSERT_EVENT);
Aml *rm_evt = aml_name("%s.%s", cphp_res_path, CPU_REMOVE_EVENT);
Aml *ej_evt = aml_name("%s.%s", cphp_res_path, CPU_EJECT_EVENT);
+ Aml *fw_ej_evt = aml_name("%s.%s", cphp_res_path, CPU_FW_EJECT_EVENT);
aml_append(cpus_dev, aml_name_decl("_HID", aml_string("ACPI0010")));
aml_append(cpus_dev, aml_name_decl("_CID", aml_eisaid("PNP0A05")));
@@ -470,7 +475,13 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
aml_append(method, aml_acquire(ctrl_lock, 0xFFFF));
aml_append(method, aml_store(idx, cpu_selector));
- aml_append(method, aml_store(one, ej_evt));
+ if (opts.fw_unplugs_cpu) {
+ aml_append(method, aml_store(one, fw_ej_evt));
+ aml_append(method, aml_store(aml_int(OVMF_CPUHP_SMI_CMD),
+ aml_name("%s", opts.smi_path)));
+ } else {
+ aml_append(method, aml_store(one, ej_evt));
+ }
aml_append(method, aml_release(ctrl_lock));
}
aml_append(cpus_dev, method);
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 9036e5594c..475e76f514 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1586,6 +1586,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
CPUHotplugFeatures opts = {
.acpi_1_compatible = true, .has_legacy_cphp = true,
.smi_path = pm->smi_on_cpuhp ? "\\_SB.PCI0.SMI0.SMIC" : NULL,
+ .fw_unplugs_cpu = pm->smi_on_cpu_unplug,
};
build_cpus_aml(dsdt, machine, opts, pm->cpu_hp_io_base,
"\\_SB.PCI0", "\\_GPE._E02");
--
2.27.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 5/8] x86: acpi: let the firmware handle pending "CPU remove" events in SMM
2020-12-04 17:09 ` [PATCH 5/8] x86: acpi: let the firmware handle pending "CPU remove" events in SMM Igor Mammedov
@ 2020-12-07 6:20 ` Ankur Arora
2020-12-07 6:24 ` Ankur Arora
2020-12-07 12:57 ` Igor Mammedov
0 siblings, 2 replies; 17+ messages in thread
From: Ankur Arora @ 2020-12-07 6:20 UTC (permalink / raw)
To: Igor Mammedov, qemu-devel; +Cc: lersek, mst
On 2020-12-04 9:09 a.m., Igor Mammedov wrote:
> if firmware and QEMU negotiated CPU hotunplug support, generate
> _EJ0 method so that it will mark CPU for removal by firmware and
> pass control to it by triggering SMI.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> include/hw/acpi/cpu.h | 1 +
> hw/acpi/cpu.c | 15 +++++++++++++--
> hw/i386/acpi-build.c | 1 +
> 3 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/include/hw/acpi/cpu.h b/include/hw/acpi/cpu.h
> index d71edde456..999caaf510 100644
> --- a/include/hw/acpi/cpu.h
> +++ b/include/hw/acpi/cpu.h
> @@ -51,6 +51,7 @@ void cpu_hotplug_hw_init(MemoryRegion *as, Object *owner,
> typedef struct CPUHotplugFeatures {
> bool acpi_1_compatible;
> bool has_legacy_cphp;
> + bool fw_unplugs_cpu;
> const char *smi_path;
> } CPUHotplugFeatures;
>
> diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
> index 811218f673..bded2a837f 100644
> --- a/hw/acpi/cpu.c
> +++ b/hw/acpi/cpu.c
> @@ -341,6 +341,7 @@ const VMStateDescription vmstate_cpu_hotplug = {
> #define CPU_INSERT_EVENT "CINS"
> #define CPU_REMOVE_EVENT "CRMV"
> #define CPU_EJECT_EVENT "CEJ0"
> +#define CPU_FW_EJECT_EVENT "CEJF"
>
> void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
> hwaddr io_base,
> @@ -393,7 +394,10 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
> aml_append(field, aml_named_field(CPU_REMOVE_EVENT, 1));
Bit 2: Device remove event, used to distinguish device for which
no device eject request to OSPM was issued. Firmware must
ignore this bit.
> /* initiates device eject, write only */
> aml_append(field, aml_named_field(CPU_EJECT_EVENT, 1));
Bit 3: reserved and should be ignored by OSPM
> - aml_append(field, aml_reserved_field(4));
> + aml_append(field, aml_reserved_field(1));
> + /* tell firmware to do device eject, write only */
> + aml_append(field, aml_named_field(CPU_FW_EJECT_EVENT, 1));
> + aml_append(field, aml_reserved_field(2));
Shouldn't this be instead:
> - aml_append(field, aml_reserved_field(4));
> + /* tell firmware to do device eject, write only */
> + aml_append(field, aml_named_field(CPU_FW_EJECT_EVENT, 1));
> + aml_append(field, aml_reserved_field(3));
Bit 4: if set to 1, OSPM requests firmware to perform device eject.
Bit 5-7: reserved and should be ignored by OSPM
Otherwise AFAICS CPU_FW_EJECT_EVENT would correspond to bit 5.
Ankur
> aml_append(field, aml_named_field(CPU_COMMAND, 8));
> aml_append(cpu_ctrl_dev, field);
>
> @@ -428,6 +432,7 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
> Aml *ins_evt = aml_name("%s.%s", cphp_res_path, CPU_INSERT_EVENT);
> Aml *rm_evt = aml_name("%s.%s", cphp_res_path, CPU_REMOVE_EVENT);
> Aml *ej_evt = aml_name("%s.%s", cphp_res_path, CPU_EJECT_EVENT);
> + Aml *fw_ej_evt = aml_name("%s.%s", cphp_res_path, CPU_FW_EJECT_EVENT);
>
> aml_append(cpus_dev, aml_name_decl("_HID", aml_string("ACPI0010")));
> aml_append(cpus_dev, aml_name_decl("_CID", aml_eisaid("PNP0A05")));
> @@ -470,7 +475,13 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
>
> aml_append(method, aml_acquire(ctrl_lock, 0xFFFF));
> aml_append(method, aml_store(idx, cpu_selector));
> - aml_append(method, aml_store(one, ej_evt));
> + if (opts.fw_unplugs_cpu) {
> + aml_append(method, aml_store(one, fw_ej_evt));
> + aml_append(method, aml_store(aml_int(OVMF_CPUHP_SMI_CMD),
> + aml_name("%s", opts.smi_path)));
> + } else {
> + aml_append(method, aml_store(one, ej_evt));
> + }
> aml_append(method, aml_release(ctrl_lock));
> }
> aml_append(cpus_dev, method);
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 9036e5594c..475e76f514 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1586,6 +1586,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> CPUHotplugFeatures opts = {
> .acpi_1_compatible = true, .has_legacy_cphp = true,
> .smi_path = pm->smi_on_cpuhp ? "\\_SB.PCI0.SMI0.SMIC" : NULL,
> + .fw_unplugs_cpu = pm->smi_on_cpu_unplug,
> };
> build_cpus_aml(dsdt, machine, opts, pm->cpu_hp_io_base,
> "\\_SB.PCI0", "\\_GPE._E02");
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 5/8] x86: acpi: let the firmware handle pending "CPU remove" events in SMM
2020-12-07 6:20 ` Ankur Arora
@ 2020-12-07 6:24 ` Ankur Arora
2020-12-07 12:57 ` Igor Mammedov
1 sibling, 0 replies; 17+ messages in thread
From: Ankur Arora @ 2020-12-07 6:24 UTC (permalink / raw)
To: Igor Mammedov, qemu-devel; +Cc: lersek, mst
On 2020-12-06 10:20 p.m., Ankur Arora wrote:
> On 2020-12-04 9:09 a.m., Igor Mammedov wrote:
>> if firmware and QEMU negotiated CPU hotunplug support, generate
>> _EJ0 method so that it will mark CPU for removal by firmware and
>> pass control to it by triggering SMI.
>>
>> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
>> ---
>> include/hw/acpi/cpu.h | 1 +
>> hw/acpi/cpu.c | 15 +++++++++++++--
>> hw/i386/acpi-build.c | 1 +
>> 3 files changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/hw/acpi/cpu.h b/include/hw/acpi/cpu.h
>> index d71edde456..999caaf510 100644
>> --- a/include/hw/acpi/cpu.h
>> +++ b/include/hw/acpi/cpu.h
>> @@ -51,6 +51,7 @@ void cpu_hotplug_hw_init(MemoryRegion *as, Object *owner,
>> typedef struct CPUHotplugFeatures {
>> bool acpi_1_compatible;
>> bool has_legacy_cphp;
>> + bool fw_unplugs_cpu;
>> const char *smi_path;
>> } CPUHotplugFeatures;
>> diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
>> index 811218f673..bded2a837f 100644
>> --- a/hw/acpi/cpu.c
>> +++ b/hw/acpi/cpu.c
>> @@ -341,6 +341,7 @@ const VMStateDescription vmstate_cpu_hotplug = {
>> #define CPU_INSERT_EVENT "CINS"
>> #define CPU_REMOVE_EVENT "CRMV"
>> #define CPU_EJECT_EVENT "CEJ0"
>> +#define CPU_FW_EJECT_EVENT "CEJF"
>> void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
>> hwaddr io_base,
>> @@ -393,7 +394,10 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
>> aml_append(field, aml_named_field(CPU_REMOVE_EVENT, 1));
>
> Bit 2: Device remove event, used to distinguish device for which
> no device eject request to OSPM was issued. Firmware must
> ignore this bit.
>
>> /* initiates device eject, write only */
>> aml_append(field, aml_named_field(CPU_EJECT_EVENT, 1));
>
> Bit 3: reserved and should be ignored by OSPM
In write-access mode:
Bit 3: if set to 1 initiates device eject, set by OSPM when it
triggers CPU device removal and calls _EJ0 method or by firmware
when bit #4 is set. In case bit #4 were set, it's cleared as
part of device eject.
(and so should not be a reserved_field() below)
>
>> - aml_append(field, aml_reserved_field(4));
>> + aml_append(field, aml_reserved_field(1));
>> + /* tell firmware to do device eject, write only */
>> + aml_append(field, aml_named_field(CPU_FW_EJECT_EVENT, 1));
>> + aml_append(field, aml_reserved_field(2));
>
> Shouldn't this be instead:
>
>> - aml_append(field, aml_reserved_field(4));
>> + /* tell firmware to do device eject, write only */
>> + aml_append(field, aml_named_field(CPU_FW_EJECT_EVENT, 1));
>> + aml_append(field, aml_reserved_field(3));
>
> Bit 4: if set to 1, OSPM requests firmware to perform device eject.
> Bit 5-7: reserved and should be ignored by OSPM
>
> Otherwise AFAICS CPU_FW_EJECT_EVENT would correspond to bit 5.
>
>
> Ankur
>
>> aml_append(field, aml_named_field(CPU_COMMAND, 8));
>> aml_append(cpu_ctrl_dev, field);
>> @@ -428,6 +432,7 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
>> Aml *ins_evt = aml_name("%s.%s", cphp_res_path, CPU_INSERT_EVENT);
>> Aml *rm_evt = aml_name("%s.%s", cphp_res_path, CPU_REMOVE_EVENT);
>> Aml *ej_evt = aml_name("%s.%s", cphp_res_path, CPU_EJECT_EVENT);
>> + Aml *fw_ej_evt = aml_name("%s.%s", cphp_res_path, CPU_FW_EJECT_EVENT);
>> aml_append(cpus_dev, aml_name_decl("_HID", aml_string("ACPI0010")));
>> aml_append(cpus_dev, aml_name_decl("_CID", aml_eisaid("PNP0A05")));
>> @@ -470,7 +475,13 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
>> aml_append(method, aml_acquire(ctrl_lock, 0xFFFF));
>> aml_append(method, aml_store(idx, cpu_selector));
>> - aml_append(method, aml_store(one, ej_evt));
>> + if (opts.fw_unplugs_cpu) {
>> + aml_append(method, aml_store(one, fw_ej_evt));
>> + aml_append(method, aml_store(aml_int(OVMF_CPUHP_SMI_CMD),
>> + aml_name("%s", opts.smi_path)));
>> + } else {
>> + aml_append(method, aml_store(one, ej_evt));
>> + }
>> aml_append(method, aml_release(ctrl_lock));
>> }
>> aml_append(cpus_dev, method);
>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> index 9036e5594c..475e76f514 100644
>> --- a/hw/i386/acpi-build.c
>> +++ b/hw/i386/acpi-build.c
>> @@ -1586,6 +1586,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>> CPUHotplugFeatures opts = {
>> .acpi_1_compatible = true, .has_legacy_cphp = true,
>> .smi_path = pm->smi_on_cpuhp ? "\\_SB.PCI0.SMI0.SMIC" : NULL,
>> + .fw_unplugs_cpu = pm->smi_on_cpu_unplug,
>> };
>> build_cpus_aml(dsdt, machine, opts, pm->cpu_hp_io_base,
>> "\\_SB.PCI0", "\\_GPE._E02");
>>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 5/8] x86: acpi: let the firmware handle pending "CPU remove" events in SMM
2020-12-07 6:20 ` Ankur Arora
2020-12-07 6:24 ` Ankur Arora
@ 2020-12-07 12:57 ` Igor Mammedov
1 sibling, 0 replies; 17+ messages in thread
From: Igor Mammedov @ 2020-12-07 12:57 UTC (permalink / raw)
To: Ankur Arora; +Cc: lersek, qemu-devel, mst
On Sun, 6 Dec 2020 22:20:27 -0800
Ankur Arora <ankur.a.arora@oracle.com> wrote:
> On 2020-12-04 9:09 a.m., Igor Mammedov wrote:
> > if firmware and QEMU negotiated CPU hotunplug support, generate
> > _EJ0 method so that it will mark CPU for removal by firmware and
> > pass control to it by triggering SMI.
> >
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> > include/hw/acpi/cpu.h | 1 +
> > hw/acpi/cpu.c | 15 +++++++++++++--
> > hw/i386/acpi-build.c | 1 +
> > 3 files changed, 15 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/hw/acpi/cpu.h b/include/hw/acpi/cpu.h
> > index d71edde456..999caaf510 100644
> > --- a/include/hw/acpi/cpu.h
> > +++ b/include/hw/acpi/cpu.h
> > @@ -51,6 +51,7 @@ void cpu_hotplug_hw_init(MemoryRegion *as, Object *owner,
> > typedef struct CPUHotplugFeatures {
> > bool acpi_1_compatible;
> > bool has_legacy_cphp;
> > + bool fw_unplugs_cpu;
> > const char *smi_path;
> > } CPUHotplugFeatures;
> >
> > diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
> > index 811218f673..bded2a837f 100644
> > --- a/hw/acpi/cpu.c
> > +++ b/hw/acpi/cpu.c
> > @@ -341,6 +341,7 @@ const VMStateDescription vmstate_cpu_hotplug = {
> > #define CPU_INSERT_EVENT "CINS"
> > #define CPU_REMOVE_EVENT "CRMV"
> > #define CPU_EJECT_EVENT "CEJ0"
> > +#define CPU_FW_EJECT_EVENT "CEJF"
> >
> > void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
> > hwaddr io_base,
> > @@ -393,7 +394,10 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
> > aml_append(field, aml_named_field(CPU_REMOVE_EVENT, 1));
>
> Bit 2: Device remove event, used to distinguish device for which
> no device eject request to OSPM was issued. Firmware must
> ignore this bit.
>
> > /* initiates device eject, write only */
> > aml_append(field, aml_named_field(CPU_EJECT_EVENT, 1));
>
> Bit 3: reserved and should be ignored by OSPM
>
> > - aml_append(field, aml_reserved_field(4));
> > + aml_append(field, aml_reserved_field(1));
> > + /* tell firmware to do device eject, write only */
> > + aml_append(field, aml_named_field(CPU_FW_EJECT_EVENT, 1));
> > + aml_append(field, aml_reserved_field(2));
>
> Shouldn't this be instead:
>
> > - aml_append(field, aml_reserved_field(4));
> > + /* tell firmware to do device eject, write only */
> > + aml_append(field, aml_named_field(CPU_FW_EJECT_EVENT, 1));
> > + aml_append(field, aml_reserved_field(3));
>
yes, it should be this way,
I'll fix in v2
> Bit 4: if set to 1, OSPM requests firmware to perform device eject.
> Bit 5-7: reserved and should be ignored by OSPM
>
> Otherwise AFAICS CPU_FW_EJECT_EVENT would correspond to bit 5.
>
>
> Ankur
>
> > aml_append(field, aml_named_field(CPU_COMMAND, 8));
> > aml_append(cpu_ctrl_dev, field);
> >
> > @@ -428,6 +432,7 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
> > Aml *ins_evt = aml_name("%s.%s", cphp_res_path, CPU_INSERT_EVENT);
> > Aml *rm_evt = aml_name("%s.%s", cphp_res_path, CPU_REMOVE_EVENT);
> > Aml *ej_evt = aml_name("%s.%s", cphp_res_path, CPU_EJECT_EVENT);
> > + Aml *fw_ej_evt = aml_name("%s.%s", cphp_res_path, CPU_FW_EJECT_EVENT);
> >
> > aml_append(cpus_dev, aml_name_decl("_HID", aml_string("ACPI0010")));
> > aml_append(cpus_dev, aml_name_decl("_CID", aml_eisaid("PNP0A05")));
> > @@ -470,7 +475,13 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
> >
> > aml_append(method, aml_acquire(ctrl_lock, 0xFFFF));
> > aml_append(method, aml_store(idx, cpu_selector));
> > - aml_append(method, aml_store(one, ej_evt));
> > + if (opts.fw_unplugs_cpu) {
> > + aml_append(method, aml_store(one, fw_ej_evt));
> > + aml_append(method, aml_store(aml_int(OVMF_CPUHP_SMI_CMD),
> > + aml_name("%s", opts.smi_path)));
> > + } else {
> > + aml_append(method, aml_store(one, ej_evt));
> > + }
> > aml_append(method, aml_release(ctrl_lock));
> > }
> > aml_append(cpus_dev, method);
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index 9036e5594c..475e76f514 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -1586,6 +1586,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> > CPUHotplugFeatures opts = {
> > .acpi_1_compatible = true, .has_legacy_cphp = true,
> > .smi_path = pm->smi_on_cpuhp ? "\\_SB.PCI0.SMI0.SMIC" : NULL,
> > + .fw_unplugs_cpu = pm->smi_on_cpu_unplug,
> > };
> > build_cpus_aml(dsdt, machine, opts, pm->cpu_hp_io_base,
> > "\\_SB.PCI0", "\\_GPE._E02");
> >
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 6/8] tests/acpi: update expected files
2020-12-04 17:09 [PATCH 0/8] add support for cpu hot-unplug with SMI broadcast enabled Igor Mammedov
` (4 preceding siblings ...)
2020-12-04 17:09 ` [PATCH 5/8] x86: acpi: let the firmware handle pending "CPU remove" events in SMM Igor Mammedov
@ 2020-12-04 17:09 ` Igor Mammedov
2020-12-04 17:09 ` [PATCH 7/8] x86: ich9: factor out "guest_cpu_hotplug_features" Igor Mammedov
2020-12-04 17:09 ` [PATCH 8/8] x86: ich9: let firmware negotiate 'CPU hot-unplug with SMI' feature Igor Mammedov
7 siblings, 0 replies; 17+ messages in thread
From: Igor Mammedov @ 2020-12-04 17:09 UTC (permalink / raw)
To: qemu-devel; +Cc: lersek, ankur.a.arora, mst
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
tests/qtest/bios-tables-test-allowed-diff.h | 21 --------------------
tests/data/acpi/pc/DSDT | Bin 5060 -> 5067 bytes
tests/data/acpi/pc/DSDT.acpihmat | Bin 6385 -> 6392 bytes
tests/data/acpi/pc/DSDT.bridge | Bin 6919 -> 6926 bytes
tests/data/acpi/pc/DSDT.cphp | Bin 5524 -> 5531 bytes
tests/data/acpi/pc/DSDT.dimmpxm | Bin 6714 -> 6721 bytes
tests/data/acpi/pc/DSDT.hpbridge | Bin 5021 -> 5028 bytes
tests/data/acpi/pc/DSDT.hpbrroot | Bin 3079 -> 3086 bytes
tests/data/acpi/pc/DSDT.ipmikcs | Bin 5132 -> 5139 bytes
tests/data/acpi/pc/DSDT.memhp | Bin 6419 -> 6426 bytes
tests/data/acpi/pc/DSDT.numamem | Bin 5066 -> 5073 bytes
tests/data/acpi/pc/DSDT.roothp | Bin 5256 -> 5263 bytes
tests/data/acpi/q35/DSDT | Bin 7796 -> 7803 bytes
tests/data/acpi/q35/DSDT.acpihmat | Bin 9121 -> 9128 bytes
tests/data/acpi/q35/DSDT.bridge | Bin 7814 -> 7821 bytes
tests/data/acpi/q35/DSDT.cphp | Bin 8260 -> 8267 bytes
tests/data/acpi/q35/DSDT.dimmpxm | Bin 9450 -> 9457 bytes
tests/data/acpi/q35/DSDT.ipmibt | Bin 7871 -> 7878 bytes
tests/data/acpi/q35/DSDT.memhp | Bin 9155 -> 9162 bytes
tests/data/acpi/q35/DSDT.mmio64 | Bin 8927 -> 8934 bytes
tests/data/acpi/q35/DSDT.numamem | Bin 7802 -> 7809 bytes
tests/data/acpi/q35/DSDT.tis | Bin 8402 -> 8409 bytes
22 files changed, 21 deletions(-)
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index cc75f3fc46..dfb8523c8b 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,22 +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.roothp",
-"tests/data/acpi/pc/DSDT.hpbridge",
-"tests/data/acpi/pc/DSDT.hpbrroot",
diff --git a/tests/data/acpi/pc/DSDT b/tests/data/acpi/pc/DSDT
index 4ca46e5a2bdb1dfab79dd8630aeeb9a386d8b30e..295819caa4a1bfafb7834b19649dac7e44d22839 100644
GIT binary patch
delta 76
zcmX@2ep;Q&CD<k8v@in$<MNGMTi7@q?c;-;;-j0qCLd>uQ`HIx3J!5(P;d@#^<#AQ
d^b2Nm4)P6SbawSJU}OMMZj1~}o1NJkc>q|H6axSN
delta 69
zcmX@Deng$iCD<k8h%f^K<C%?ITi7_g?Bau+;-j0KCm&~vlT!`|3J!5(P;d@#^<#AQ
Z^b2Nm4)P6SbawSJU}Rv~?8V;50|3}=69WJM
diff --git a/tests/data/acpi/pc/DSDT.acpihmat b/tests/data/acpi/pc/DSDT.acpihmat
index 35a74bce8cc152ecb615cb38c4b7f63c7c7d3ab3..0354a205657efce4957933b9da84bc599282a344 100644
GIT binary patch
delta 76
zcmexp_`{IPCD<k8hXexyW6DOZEo__)uJOT6@zG6QlaI5-scHoT1&25?C^!eW`Y}3t
d`UNvO2l<9EI=gxqFfxEBH%11g&CcxW1Oayf6qf)1
delta 69
zcmexi_|cHdCD<k8qXYv3W9LS$Eo_{gF7d%m@zG7rlaI5-$tec}1&25?C^!eW`Y}3t
Z`UNvO2l<9EI=gxqFfy=g_F`Wr2mtgh6PEx0
diff --git a/tests/data/acpi/pc/DSDT.bridge b/tests/data/acpi/pc/DSDT.bridge
index 803d7a8e839ea8b7ac33c4490459ddaede584269..7bdcb830b5f6bd5f4aca0528ae3dcca4e429c6fb 100644
GIT binary patch
delta 76
zcmZoS>oenW33dtLlV)IGe7})v3md1SeSEM}d~}o7<l}5{s#*a-!6A+e3eEwpevHnZ
de!+~+LB3&(&aPetj0_;kjgf(Avore<VE`Z)6NCT&
delta 69
zcmeA(Yd7O^33dr#mu6sK6xqnNg^kn8E<V^PKDx<y@^Q8}Ipu($;1EX!1?K=)KSpOy
YzhFk^Am1=XXIC!+Mh2G6UhGGN0d8>-ga7~l
diff --git a/tests/data/acpi/pc/DSDT.cphp b/tests/data/acpi/pc/DSDT.cphp
index 8bab2f506409f2b025a63d8b91c7bfdaa931e626..d62a3a3e39801348b9d590343355678c38e520f7 100644
GIT binary patch
delta 76
zcmbQDJzJa0CD<iowkQJwBlkwGEo_{QG4a7p@zG6QlaI5-scHoT1&25?C^!eW`Y}3t
d`UNvO2l<9EI=gxqFfxEBH%11g&CcwBd;k*763_qu
delta 69
zcmbQOJw=<#CD<ioiYNmEqrpb5Eo_`#(ec4f@zG7rlaI5-$tec}1&25?C^!eW`Y}3t
Z`UNvO2l<9EI=gxqFfy=g_F@m@0|1%I5zqht
diff --git a/tests/data/acpi/pc/DSDT.dimmpxm b/tests/data/acpi/pc/DSDT.dimmpxm
index e015b4594c96a6e0f34c0668e3383b9a91dff38e..9458d9a96aca5633cd5369f8daa4db8a2ce63b55 100644
GIT binary patch
delta 76
zcmdmGa?pg!CD<jzQHp_q@##jcEo_`#0rA03@zG6QlaI5-scHoT1&25?C^!eW`Y}3t
d`UNvO2l<9EI=gxqFfxEBH%11g&CcxKg#b-~6j}fP
delta 69
zcmX?Tvde_aCD<jzN{WGjQF0^K7B)`jfcRji_~<6*$;a8^<dg$~f<qh`6r2NG{TQ7+
Z{el^tgM7mnon5^Q7#UbLd$E5P0sxn{5?TNN
diff --git a/tests/data/acpi/pc/DSDT.hpbridge b/tests/data/acpi/pc/DSDT.hpbridge
index 56032bcf1ba4e251f16c9028429826090531efdd..34dcfeed22bc1fb3a611b866c13232b969a982a9 100644
GIT binary patch
delta 76
zcmbQMzC@kNCD<ioi7*2L<AaS{Ti7@q?c;-;;-j0qCLd>uQ`HIx3J!5(P;d@#^<#AQ
d^b2Nm4)P6SbawSJU}OMMZj1~}o1NLCcmPEO6QBS9
delta 69
zcmZ3YK3AQ~CD<iot}p`wBkM-4Eo_`#cJaYZ@zG7rlaI5-$tec}1&25?C^!eW`Y}3t
Z`UNvO2l<9EI=gxqFfy=g_F|9X0RWGK5ugA7
diff --git a/tests/data/acpi/pc/DSDT.hpbrroot b/tests/data/acpi/pc/DSDT.hpbrroot
index 36b0a8f2fbf93df47b66107125cd3ce01e017b92..09949201424ee98994ae650b9b76171dba6cb5cd 100644
GIT binary patch
delta 53
zcmZpd=#$`b33dtL<6&T6?AXYq&Bp0yA0O-#AKm0N*^VuaQET#CHW>~^XIC#bMh2$M
IyV!hL0ag?YtN;K2
delta 46
zcmeB^XqVt}33dr#=V4%AT)UA=n~l@UE<V^PKDx<yvK?C-qw?grY%+{2n-8-2vH}1J
Co(rr1
diff --git a/tests/data/acpi/pc/DSDT.ipmikcs b/tests/data/acpi/pc/DSDT.ipmikcs
index ca6630e39f60ebd5c056f57c4c03fdb9d5467577..60f4dde1e4d80d64eb8645218acee78daff45dc4 100644
GIT binary patch
delta 53
zcmeCtn5@C&66_KpEW*IRXtj~+4;!bWeSEM}d~}o7WPbKIMy<(t>@pmT&aPf=j0{Yh
IyV)Cg0BW`k7XSbN
delta 46
zcmbQN(WAlT66_MfBf`MI7{8J04;!bKU3{=pd~}oZWPbKIM&-$Q>@tijn<ue1@&EuE
Ct_&9d
diff --git a/tests/data/acpi/pc/DSDT.memhp b/tests/data/acpi/pc/DSDT.memhp
index 43f4e114e2cc48c68c35af47303fa87c9255db40..4302ff996f74bf6a3615b7a8885c6ef21bc318d6 100644
GIT binary patch
delta 76
zcmbPiG|Py~CD<iIN|J$rv2!EW7B)_I`}km|_~<6D$;a8^RJ8(vf<qh`6r2NG{TQ7+
d{el^tgM7mnon5^Q7#Tp68zTeLW@q*bf&dUA6G#96
delta 69
zcmbPbG}(yDCD<iISdxK(aqmX1Eo__)_VK|^@zG7rlaI5-$tec}1&25?C^!eW`Y}3t
Z`UNvO2l<9EI=gxqFfy=g_F}&v2mqJ95=a05
diff --git a/tests/data/acpi/pc/DSDT.numamem b/tests/data/acpi/pc/DSDT.numamem
index ba8f7e6c33f9eb0f7a080144fcb4a27d36aa04ae..b615c14a5e0f63e36f3d75eadf9d46e4dcea13d3 100644
GIT binary patch
delta 76
zcmX@5eo>vvCD<k8qA&vkqtiyNEo_|b_VK|^@zG6QlaI5-scHoT1&25?C^!eW`Y}3t
d`UNvO2l<9EI=gxqFfxEBH%11g&CcxYJOD<J6TAQb
delta 69
zcmcbpeoCFoCD<k8lrRGWW7$ToEo__)_VK|^@zG7rlaI5-$tec}1&25?C^!eW`Y}3t
Z`UNvO2l<9EI=gxqFfy=g_F`}60RYI}61)Ha
diff --git a/tests/data/acpi/pc/DSDT.roothp b/tests/data/acpi/pc/DSDT.roothp
index 18caa0765fc10adb29e01717390ead6c63cd0f3c..10a5bab1968dbf4ee844d7bcfc3876b58fdf54af 100644
GIT binary patch
delta 76
zcmeCs?APRS33dtT7hzyvJiC!=3md1SeSEM}d~}o7<l}5{s#*a-!6A+e3eEwpevHnZ
de!+~+LB3&(&aPetj0_;kjgf(AvopIZF90v!6F&d|
delta 69
zcmeCz?9k+L33dtT5Mf|o{Irp43md1GU3{=pd~}oZ<l}5{a>@Zg!6A+e3eEwpevHnZ
Ye!+~+LB3&(&aPetj0`NBz1UrO0ke1#KL7v#
diff --git a/tests/data/acpi/q35/DSDT b/tests/data/acpi/q35/DSDT
index e7414e78563372fca4d2aab9d16c58c0ff8468f4..931afc6f626a022d368011c412f36211d1d34031 100644
GIT binary patch
delta 76
zcmexj^V^2YCD<jTT8@E%@$W{i2@;%+_VK|^@zG6QlNU?GscHoT1&25?C^!eW`Y}3t
d`UNvO2l<9EI=gxqFfxEBH%11g&GM3=tN?pc6g2<<
delta 69
zcmexu^TmeCCD<jTM2>-hQE4OB1PM+ryZB(I_~<6*$%`f8<dg$~f<qh`6r2NG{TQ7+
Z{el^tgM7mnon5^Q7#UbLt4W5k0sz455;Xt-
diff --git a/tests/data/acpi/q35/DSDT.acpihmat b/tests/data/acpi/q35/DSDT.acpihmat
index 88434da261212b15264c892976775acd5c954aea..f884ede4efbf07dddd0049b9118679d049159bdc 100644
GIT binary patch
delta 76
zcmZ4JzQUc$CD<iog)##J<F1Wd6C^ktT;qeC;-j0qCNGwVQ`HIx3J!5(P;d@#^<#AQ
d^b2Nm4)P6SbawSJU}OMMZj1~}o8={&xByR86UzVq
delta 69
zcmZ4CzR;b^CD<iop)vyl<Nb|X6C^l2UE+hC;-j0KCoh(WlT!`|3J!5(P;d@#^<#AQ
Z^b2Nm4)P6SbawSJU}Rv~tR~sS1pv@863YMp
diff --git a/tests/data/acpi/q35/DSDT.bridge b/tests/data/acpi/q35/DSDT.bridge
index 118476ff6101e11d6b1f2d3399241d7fd1a6f634..d1ce6d9065318cd71edec489ed124c1eb22fd9b0 100644
GIT binary patch
delta 76
zcmZp(?X~4{33dtTm1AIFoVJl`f&{0deSEM}d~}o7<i!$ks#*a-!6A+e3eEwpevHnZ
de!+~+LB3&(&aPetj0_;kjgf(Av%F+FD*z{36Bqyh
delta 69
zcmeCRZL{Ta33dr-lVf0D+`W-&f&{0RU3{=pd~}oZ<i!$ka>@Zg!6A+e3eEwpevHnZ
Ye!+~+LB3&(&aPetj0`NB)g;qd0jxa|7ytkO
diff --git a/tests/data/acpi/q35/DSDT.cphp b/tests/data/acpi/q35/DSDT.cphp
index 69c5edf620529e995461ccba63b76a083f25b2b6..f5b411a54bb6942f59ed016d43c4c28a7f99eb6b 100644
GIT binary patch
delta 76
zcmX@&aN2>(CD<jzTY-UrF>xc;1PM;ZnD}6)_~<6D$%`f8RJ8(vf<qh`6r2NG{TQ7+
d{el^tgM7mnon5^Q7#Tp68zTeLW_d|fb^tYu69NDL
delta 69
zcmX@@aKwSjCD<jzMS+2Vv27#Q1PM;B==fl#_~<6*$%`f8<dg$~f<qh`6r2NG{TQ7+
Z{el^tgM7mnon5^Q7#UbLt4XS|0|2>55&{4K
diff --git a/tests/data/acpi/q35/DSDT.dimmpxm b/tests/data/acpi/q35/DSDT.dimmpxm
index af41acba6e0117191ad8495a30ded7b0acc4d2ca..4bb12250f9dcdb34b2c2801186746a3693398cea 100644
GIT binary patch
delta 76
zcmaFm`O%ZhCD<k8qY489qufTW2@;%M0rA03@zG6QlNU?GscHoT1&25?C^!eW`Y}3t
d`UNvO2l<9EI=gxqFfxEBH%11g&GM3`xdCnX6m9?j
delta 69
zcmez9`O1^aCD<k8l?np`W6(yf2@;&n0rA03@zG7rlNU?G$tec}1&25?C^!eW`Y}3t
Z`UNvO2l<9EI=gxqFfy=gR+Bu<4FK%F6K((i
diff --git a/tests/data/acpi/q35/DSDT.ipmibt b/tests/data/acpi/q35/DSDT.ipmibt
index a650c3041ab9d6688eda843a6a2ab418e1a7ce9b..3ef8170d2a7004e3098af9e675a52174329c39ae 100644
GIT binary patch
delta 53
zcmdmQd(4*0CD<k8m>dHGqv1xb+Y+3P_VK|^@zG6Qlix|iF=|crm6YLNbawS}V`O03
JoFy5`3IMor4(k8_
delta 46
zcmX?RyWf_}CD<iozZ?StW7tNn+Y+2!cJaYZ@zG7rlix|iF)B~?m6Ty**<2_Y$_fBl
C&<*PV
diff --git a/tests/data/acpi/q35/DSDT.memhp b/tests/data/acpi/q35/DSDT.memhp
index 85598ca3f68f437e8d5048e2cb9815f20b332152..7e68670cdef2f65083f0a2044eb86565f3da1f89 100644
GIT binary patch
delta 76
zcmX@?e#)K8CD<k8lrjSY<AaS{6C^m@?c;-;;-j0qCNGwVQ`HIx3J!5(P;d@#^<#AQ
e^b2Nm4)P6SbawSJU}OMMZj1~}o8={!Z~*{y-4uEN
delta 69
zcmX@*e%PJMCD<k8urdP!qsT_C2@;$R_VK|^@zG7rlNU?G$tec}1&25?C^!eW`Y}3t
Z`UNvO2l<9EI=gxqFfy=gR+C)91pvG_5_$jt
diff --git a/tests/data/acpi/q35/DSDT.mmio64 b/tests/data/acpi/q35/DSDT.mmio64
index 092fdc32628f5a145b510c2a46de8b02222b1951..462e3bd5b622b7da06b2eaf194cc55a07ce9f9f9 100644
GIT binary patch
delta 76
zcmccb`plKfCD<k8nGyp7<J65@6C^m@?c;-;;-j0qCNGwVQ`HIx3J!5(P;d@#^<#AQ
e^b2Nm4)P6SbawSJU}OMMZj1~}o8=`BZ~_2&JQSG#
delta 69
zcmaFndf%1HCD<k8z7hii<H?O&6C^kt?Bj!-;-j0KCoh(WlT!`|3J!5(P;d@#^<#AQ
Z^b2Nm4)P6SbawSJU}Rv~tR{JY69D&n6PW-2
diff --git a/tests/data/acpi/q35/DSDT.numamem b/tests/data/acpi/q35/DSDT.numamem
index 899946255b9111e077e06c5f78be860e863911b9..1bfae7b803a971d47eeba3d943b83b9862f0c3cf 100644
GIT binary patch
delta 76
zcmexm(`d`(66_MvD96CSIAbH%1PM-e`}km|_~<6D$%`f8RJ8(vf<qh`6r2NG{TQ7+
d{el^tgM7mnon5^Q7#Tp68zTeLW_ihIRsckp6K?<j
delta 69
zcmZp){bj@D66_LECC9+PcxEHl1PM+D`}km|_~<6*$%`f8<dg$~f<qh`6r2NG{TQ7+
Z{el^tgM7mnon5^Q7#UbLt4T()0szJ!5^n$i
diff --git a/tests/data/acpi/q35/DSDT.tis b/tests/data/acpi/q35/DSDT.tis
index 08802fbd12eae6ad99f03a8db9a0bc7f95e77cb4..24f390d3404640f4f2636bff54911e012f8c23c2 100644
GIT binary patch
delta 76
zcmccQc+-)~CD<k8rUC;4<N1wT6C^kt?c;-;;-j0qCNGwVQ`HIx3J!5(P;d@#^<#AQ
e^b2Nm4)P6SbawSJU}OMMZj1~}o8=`pvjYHm0~Cz_
delta 69
zcmccVc*&8=CD<k8k^%z*<Cl$G6C^mj?Bau+;-j0KCoh(WlT!`|3J!5(P;d@#^<#AQ
Z^b2Nm4)P6SbawSJU}Rv~tR}gc9RT&z6O8}>
--
2.27.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 7/8] x86: ich9: factor out "guest_cpu_hotplug_features"
2020-12-04 17:09 [PATCH 0/8] add support for cpu hot-unplug with SMI broadcast enabled Igor Mammedov
` (5 preceding siblings ...)
2020-12-04 17:09 ` [PATCH 6/8] tests/acpi: update expected files Igor Mammedov
@ 2020-12-04 17:09 ` Igor Mammedov
2020-12-04 17:09 ` [PATCH 8/8] x86: ich9: let firmware negotiate 'CPU hot-unplug with SMI' feature Igor Mammedov
7 siblings, 0 replies; 17+ messages in thread
From: Igor Mammedov @ 2020-12-04 17:09 UTC (permalink / raw)
To: qemu-devel; +Cc: lersek, ankur.a.arora, mst
it will be reused by next patch to check validity of unplug
feature.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
hw/isa/lpc_ich9.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index 087a18d04d..da80430144 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -366,6 +366,7 @@ static void smi_features_ok_callback(void *opaque)
{
ICH9LPCState *lpc = opaque;
uint64_t guest_features;
+ uint64_t guest_cpu_hotplug_features;
if (lpc->smi_features_ok) {
/* negotiation already complete, features locked */
@@ -378,9 +379,12 @@ static void smi_features_ok_callback(void *opaque)
/* guest requests invalid features, leave @features_ok at zero */
return;
}
+
+ guest_cpu_hotplug_features = guest_features &
+ (BIT_ULL(ICH9_LPC_SMI_F_CPU_HOTPLUG_BIT) |
+ BIT_ULL(ICH9_LPC_SMI_F_CPU_HOT_UNPLUG_BIT));
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))) {
+ guest_cpu_hotplug_features) {
/*
* cpu hot-[un]plug with SMI requires SMI broadcast,
* leave @features_ok at zero
--
2.27.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 8/8] x86: ich9: let firmware negotiate 'CPU hot-unplug with SMI' feature
2020-12-04 17:09 [PATCH 0/8] add support for cpu hot-unplug with SMI broadcast enabled Igor Mammedov
` (6 preceding siblings ...)
2020-12-04 17:09 ` [PATCH 7/8] x86: ich9: factor out "guest_cpu_hotplug_features" Igor Mammedov
@ 2020-12-04 17:09 ` Igor Mammedov
7 siblings, 0 replies; 17+ messages in thread
From: Igor Mammedov @ 2020-12-04 17:09 UTC (permalink / raw)
To: qemu-devel; +Cc: lersek, ankur.a.arora, mst
Keep CPU hotunplug with SMI disabled on 5.2 and older and enable
it by default on newer machine types.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v1:
- ensure that unplug can't be enabled without hotplug (Laszlo)
---
hw/i386/pc.c | 4 +++-
hw/isa/lpc_ich9.c | 8 +++++++-
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 781523684c..6476d8d853 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_2[] = {};
+GlobalProperty pc_compat_5_2[] = {
+ { "ICH9-LPC", "x-smi-cpu-hotunplug", "off" },
+};
const size_t pc_compat_5_2_len = G_N_ELEMENTS(pc_compat_5_2);
GlobalProperty pc_compat_5_1[] = {
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index da80430144..d3145bf014 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -392,6 +392,12 @@ static void smi_features_ok_callback(void *opaque)
return;
}
+ if (guest_cpu_hotplug_features ==
+ BIT_ULL(ICH9_LPC_SMI_F_CPU_HOT_UNPLUG_BIT)) {
+ /* cpu hot-unplug is unsupported without cpu-hotplug */
+ return;
+ }
+
/* valid feature subset requested, lock it down, report success */
lpc->smi_negotiated_features = guest_features;
lpc->smi_features_ok = 1;
@@ -774,7 +780,7 @@ static Property ich9_lpc_properties[] = {
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),
+ ICH9_LPC_SMI_F_CPU_HOT_UNPLUG_BIT, true),
DEFINE_PROP_END_OF_LIST(),
};
--
2.27.0
^ permalink raw reply related [flat|nested] 17+ messages in thread