| Message ID | 20230926100436.28284-20-salil.mehta@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Support of Virtual CPU Hotplug for ARMv8 Arch | expand |
Hi Salil, On 9/26/23 20:04, Salil Mehta wrote: > ACPI AML changes to properly reflect the _STA.PRES and _STA.ENA Bits to the > guest during initialzation, when CPUs are hotplugged and after CPUs are > hot-unplugged. > > Signed-off-by: Salil Mehta <salil.mehta@huawei.com> > --- > hw/acpi/cpu.c | 49 +++++++++++++++++++++++++++++++--- > hw/acpi/generic_event_device.c | 11 ++++++++ > include/hw/acpi/cpu.h | 2 ++ > 3 files changed, 58 insertions(+), 4 deletions(-) > > diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c > index 232720992d..e1299696d3 100644 > --- a/hw/acpi/cpu.c > +++ b/hw/acpi/cpu.c > @@ -63,10 +63,11 @@ static uint64_t cpu_hotplug_rd(void *opaque, hwaddr addr, unsigned size) > cdev = &cpu_st->devs[cpu_st->selector]; > switch (addr) { > case ACPI_CPU_FLAGS_OFFSET_RW: /* pack and return is_* fields */ > - val |= cdev->cpu ? 1 : 0; > + val |= cdev->is_enabled ? 1 : 0; > val |= cdev->is_inserting ? 2 : 0; > val |= cdev->is_removing ? 4 : 0; > val |= cdev->fw_remove ? 16 : 0; > + val |= cdev->is_present ? 32 : 0; > trace_cpuhp_acpi_read_flags(cpu_st->selector, val); > break; The vCPU states are synchronized to what we had. It means we're maintaining two set vCPU states, one for board level and another set for vCPU hotplug here. They look duplicate to each other. However, it will need too much code changes to combine them. > case ACPI_CPU_CMD_DATA_OFFSET_RW: > @@ -228,7 +229,21 @@ void cpu_hotplug_hw_init(MemoryRegion *as, Object *owner, > struct CPUState *cpu = CPU(id_list->cpus[i].cpu); > if (qemu_present_cpu(cpu)) { > state->devs[i].cpu = cpu; > + state->devs[i].is_present = true; > + } else { > + if (qemu_persistent_cpu(cpu)) { > + state->devs[i].is_present = true; > + } else { > + state->devs[i].is_present = false; > + } > } state->devs[i].is_present = qemu_persistent_cpu(cpu); > + > + if (qemu_enabled_cpu(cpu)) { > + state->devs[i].is_enabled = true; > + } else { > + state->devs[i].is_enabled = false; > + } > + state->dev[i].is_enabled = qemu_enabled_cpu(cpu); > state->devs[i].arch_id = id_list->cpus[i].arch_id; > } > memory_region_init_io(&state->ctrl_reg, owner, &cpu_hotplug_ops, state, > @@ -261,6 +276,8 @@ void acpi_cpu_plug_cb(HotplugHandler *hotplug_dev, > } > > cdev->cpu = CPU(dev); > + cdev->is_present = true; > + cdev->is_enabled = true; > if (dev->hotplugged) { > cdev->is_inserting = true; > acpi_send_event(DEVICE(hotplug_dev), ACPI_CPU_HOTPLUG_STATUS); > @@ -292,6 +309,11 @@ void acpi_cpu_unplug_cb(CPUHotplugState *cpu_st, > return; > } > > + cdev->is_enabled = false; > + if (!qemu_persistent_cpu(CPU(dev))) { > + cdev->is_present = false; > + } > + > cdev->cpu = NULL; > } > > @@ -302,6 +324,8 @@ static const VMStateDescription vmstate_cpuhp_sts = { > .fields = (VMStateField[]) { > VMSTATE_BOOL(is_inserting, AcpiCpuStatus), > VMSTATE_BOOL(is_removing, AcpiCpuStatus), > + VMSTATE_BOOL(is_present, AcpiCpuStatus), > + VMSTATE_BOOL(is_enabled, AcpiCpuStatus), > VMSTATE_UINT32(ost_event, AcpiCpuStatus), > VMSTATE_UINT32(ost_status, AcpiCpuStatus), > VMSTATE_END_OF_LIST() > @@ -339,6 +363,7 @@ const VMStateDescription vmstate_cpu_hotplug = { > #define CPU_REMOVE_EVENT "CRMV" > #define CPU_EJECT_EVENT "CEJ0" > #define CPU_FW_EJECT_EVENT "CEJF" > +#define CPU_PRESENT "CPRS" > > void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, > hwaddr base_addr, > @@ -399,7 +424,9 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, > aml_append(field, aml_named_field(CPU_EJECT_EVENT, 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(3)); > + /* 1 if present, read only */ > + aml_append(field, aml_named_field(CPU_PRESENT, 1)); > + aml_append(field, aml_reserved_field(2)); > aml_append(field, aml_named_field(CPU_COMMAND, 8)); > aml_append(cpu_ctrl_dev, field); > > @@ -429,6 +456,7 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, > Aml *ctrl_lock = aml_name("%s.%s", cphp_res_path, CPU_LOCK); > Aml *cpu_selector = aml_name("%s.%s", cphp_res_path, CPU_SELECTOR); > Aml *is_enabled = aml_name("%s.%s", cphp_res_path, CPU_ENABLED); > + Aml *is_present = aml_name("%s.%s", cphp_res_path, CPU_PRESENT); > Aml *cpu_cmd = aml_name("%s.%s", cphp_res_path, CPU_COMMAND); > Aml *cpu_data = aml_name("%s.%s", cphp_res_path, CPU_DATA); > Aml *ins_evt = aml_name("%s.%s", cphp_res_path, CPU_INSERT_EVENT); > @@ -457,13 +485,26 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, > { > Aml *idx = aml_arg(0); > Aml *sta = aml_local(0); > + Aml *ifctx2; > + Aml *else_ctx; > > aml_append(method, aml_acquire(ctrl_lock, 0xFFFF)); > aml_append(method, aml_store(idx, cpu_selector)); > aml_append(method, aml_store(zero, sta)); > - ifctx = aml_if(aml_equal(is_enabled, one)); > + ifctx = aml_if(aml_equal(is_present, one)); > { > - aml_append(ifctx, aml_store(aml_int(0xF), sta)); > + ifctx2 = aml_if(aml_equal(is_enabled, one)); > + { > + /* cpu is present and enabled */ > + aml_append(ifctx2, aml_store(aml_int(0xF), sta)); > + } > + aml_append(ifctx, ifctx2); > + else_ctx = aml_else(); > + { > + /* cpu is present but disabled */ > + aml_append(else_ctx, aml_store(aml_int(0xD), sta)); > + } > + aml_append(ifctx, else_ctx); > } > aml_append(method, ifctx); > aml_append(method, aml_release(ctrl_lock)); > diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c > index d2fa1d0e4a..b84602b238 100644 > --- a/hw/acpi/generic_event_device.c > +++ b/hw/acpi/generic_event_device.c > @@ -319,6 +319,16 @@ static const VMStateDescription vmstate_memhp_state = { > } > }; > > +static const VMStateDescription vmstate_cpuhp_state = { > + .name = "acpi-ged/cpuhp", > + .version_id = 1, > + .minimum_version_id = 1, > + .fields = (VMStateField[]) { > + VMSTATE_CPU_HOTPLUG(cpuhp_state, AcpiGedState), > + VMSTATE_END_OF_LIST() > + } > +}; > + > static const VMStateDescription vmstate_ged_state = { > .name = "acpi-ged-state", > .version_id = 1, > @@ -367,6 +377,7 @@ static const VMStateDescription vmstate_acpi_ged = { > }, > .subsections = (const VMStateDescription * []) { > &vmstate_memhp_state, > + &vmstate_cpuhp_state, > &vmstate_ghes_state, > NULL > } > diff --git a/include/hw/acpi/cpu.h b/include/hw/acpi/cpu.h > index b87ebfdf4b..786a30d6d4 100644 > --- a/include/hw/acpi/cpu.h > +++ b/include/hw/acpi/cpu.h > @@ -22,6 +22,8 @@ typedef struct AcpiCpuStatus { > uint64_t arch_id; > bool is_inserting; > bool is_removing; > + bool is_present; > + bool is_enabled; > bool fw_remove; > uint32_t ost_event; > uint32_t ost_status; Thanks, Gavin
Hi Gavin, > From: Gavin Shan <gshan@redhat.com> > Sent: Friday, September 29, 2023 12:34 AM > To: Salil Mehta <salil.mehta@huawei.com>; qemu-devel@nongnu.org; qemu-arm@nongnu.org > Cc: maz@kernel.org; jean-philippe@linaro.org; Jonathan Cameron > <jonathan.cameron@huawei.com>; lpieralisi@kernel.org; > peter.maydell@linaro.org; richard.henderson@linaro.org; > imammedo@redhat.com; andrew.jones@linux.dev; david@redhat.com; > philmd@linaro.org; eric.auger@redhat.com; will@kernel.org; ardb@kernel.org; > oliver.upton@linux.dev; pbonzini@redhat.com; mst@redhat.com; > rafael@kernel.org; borntraeger@linux.ibm.com; alex.bennee@linaro.org; > linux@armlinux.org.uk; darren@os.amperecomputing.com; > ilkka@os.amperecomputing.com; vishnu@os.amperecomputing.com; > karl.heubaum@oracle.com; miguel.luis@oracle.com; salil.mehta@opnsrc.net; > zhukeqian <zhukeqian1@huawei.com>; wangxiongfeng (C) > <wangxiongfeng2@huawei.com>; wangyanan (Y) <wangyanan55@huawei.com>; > jiakernel2@gmail.com; maobibo@loongson.cn; lixianglai@loongson.cn > Subject: Re: [PATCH RFC V2 19/37] hw/acpi: ACPI/AML Changes to reflect the > correct _STA.{PRES,ENA} Bits to Guest > > Hi Salil, > > On 9/26/23 20:04, Salil Mehta wrote: > > ACPI AML changes to properly reflect the _STA.PRES and _STA.ENA Bits to > the > > guest during initialzation, when CPUs are hotplugged and after CPUs are > > hot-unplugged. > > > > Signed-off-by: Salil Mehta <salil.mehta@huawei.com> > > --- > > hw/acpi/cpu.c | 49 +++++++++++++++++++++++++++++++--- > > hw/acpi/generic_event_device.c | 11 ++++++++ > > include/hw/acpi/cpu.h | 2 ++ > > 3 files changed, 58 insertions(+), 4 deletions(-) > > > > diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c > > index 232720992d..e1299696d3 100644 > > --- a/hw/acpi/cpu.c > > +++ b/hw/acpi/cpu.c > > @@ -63,10 +63,11 @@ static uint64_t cpu_hotplug_rd(void *opaque, hwaddr > addr, unsigned size) > > cdev = &cpu_st->devs[cpu_st->selector]; > > switch (addr) { > > case ACPI_CPU_FLAGS_OFFSET_RW: /* pack and return is_* fields */ > > - val |= cdev->cpu ? 1 : 0; > > + val |= cdev->is_enabled ? 1 : 0; > > val |= cdev->is_inserting ? 2 : 0; > > val |= cdev->is_removing ? 4 : 0; > > val |= cdev->fw_remove ? 16 : 0; > > + val |= cdev->is_present ? 32 : 0; > > trace_cpuhp_acpi_read_flags(cpu_st->selector, val); > > break; > > The vCPU states are synchronized to what we had. It means we're maintaining two set > vCPU states, one for board level and another set for vCPU hotplug here. They look > duplicate to each other. However, it will need too much code changes to combine > them. We need to distinguish between ACPI CPU Hotplug states and QOM CPU states. ACPI exposes the QOM CPU state to Guest OS. Till now ACPI CPU Hotplug states and QOM CPU states were consistent to each other. ACPI inferred the presence of QOM CPUState object as _STA.PRESENT=1 and _STA.ENABLED=1. But with the ARM CPU hot(un)plug changes, this assumption is no longer true. QOM CPUState object might still be present or NULL but ACPI CPU Hotplug State will always expose un-plugged CPU i.e. with NULL CPUState object as _STA.PRESENT=1 and _STA.ENABLED=0. This is a key change because of which we are able to enable hot(un)plug mechanism on ARM. > > > case ACPI_CPU_CMD_DATA_OFFSET_RW: > > @@ -228,7 +229,21 @@ void cpu_hotplug_hw_init(MemoryRegion *as, Object > *owner, > > struct CPUState *cpu = CPU(id_list->cpus[i].cpu); > > if (qemu_present_cpu(cpu)) { > > state->devs[i].cpu = cpu; > > + state->devs[i].is_present = true; > > + } else { > > + if (qemu_persistent_cpu(cpu)) { > > + state->devs[i].is_present = true; > > + } else { > > + state->devs[i].is_present = false; > > + } > > } > > state->devs[i].is_present = qemu_persistent_cpu(cpu); Sure. Thanks for pointing. :) > > + > > + if (qemu_enabled_cpu(cpu)) { > > + state->devs[i].is_enabled = true; > > + } else { > > + state->devs[i].is_enabled = false; > > + } > > + > > state->dev[i].is_enabled = qemu_enabled_cpu(cpu); Sure. Yes. Thanks Salil.
On Tue, 26 Sep 2023 11:04:18 +0100 Salil Mehta via <qemu-devel@nongnu.org> wrote: > ACPI AML changes to properly reflect the _STA.PRES and _STA.ENA Bits to the > guest during initialzation, when CPUs are hotplugged and after CPUs are > hot-unplugged. > > Signed-off-by: Salil Mehta <salil.mehta@huawei.com> Hi Salil, Just brought up a qemu on qemu test setup again to poke the kernel series and hopefully resolve a few questions there. Ran into a trivial problem in which the kernel was trying and failing to attach an ACPI handler before these were hotplugged. Came down to the kernel code now treating functional in _STA as meaning can be enumerated and effectively ignoring all the other bits. Requires a change to the value presented by default... See below. Fix may well be completely wrong even though it works ;) Thanks, Jonathan > --- > hw/acpi/cpu.c | 49 +++++++++++++++++++++++++++++++--- > hw/acpi/generic_event_device.c | 11 ++++++++ > include/hw/acpi/cpu.h | 2 ++ > 3 files changed, 58 insertions(+), 4 deletions(-) > > diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c > index 232720992d..e1299696d3 100644 > --- a/hw/acpi/cpu.c > +++ b/hw/acpi/cpu.c > @@ -63,10 +63,11 @@ static uint64_t cpu_hotplug_rd(void *opaque, hwaddr addr, unsigned size) > cdev = &cpu_st->devs[cpu_st->selector]; > switch (addr) { > case ACPI_CPU_FLAGS_OFFSET_RW: /* pack and return is_* fields */ > - val |= cdev->cpu ? 1 : 0; > + val |= cdev->is_enabled ? 1 : 0; > val |= cdev->is_inserting ? 2 : 0; > val |= cdev->is_removing ? 4 : 0; > val |= cdev->fw_remove ? 16 : 0; > + val |= cdev->is_present ? 32 : 0; > trace_cpuhp_acpi_read_flags(cpu_st->selector, val); > break; > case ACPI_CPU_CMD_DATA_OFFSET_RW: > @@ -228,7 +229,21 @@ void cpu_hotplug_hw_init(MemoryRegion *as, Object *owner, > struct CPUState *cpu = CPU(id_list->cpus[i].cpu); > if (qemu_present_cpu(cpu)) { > state->devs[i].cpu = cpu; > + state->devs[i].is_present = true; > + } else { > + if (qemu_persistent_cpu(cpu)) { > + state->devs[i].is_present = true; > + } else { > + state->devs[i].is_present = false; > + } > } > + > + if (qemu_enabled_cpu(cpu)) { > + state->devs[i].is_enabled = true; > + } else { > + state->devs[i].is_enabled = false; > + } > + > state->devs[i].arch_id = id_list->cpus[i].arch_id; > } > memory_region_init_io(&state->ctrl_reg, owner, &cpu_hotplug_ops, state, > @@ -261,6 +276,8 @@ void acpi_cpu_plug_cb(HotplugHandler *hotplug_dev, > } > > cdev->cpu = CPU(dev); > + cdev->is_present = true; > + cdev->is_enabled = true; > if (dev->hotplugged) { > cdev->is_inserting = true; > acpi_send_event(DEVICE(hotplug_dev), ACPI_CPU_HOTPLUG_STATUS); > @@ -292,6 +309,11 @@ void acpi_cpu_unplug_cb(CPUHotplugState *cpu_st, > return; > } > > + cdev->is_enabled = false; > + if (!qemu_persistent_cpu(CPU(dev))) { > + cdev->is_present = false; > + } > + > cdev->cpu = NULL; > } > > @@ -302,6 +324,8 @@ static const VMStateDescription vmstate_cpuhp_sts = { > .fields = (VMStateField[]) { > VMSTATE_BOOL(is_inserting, AcpiCpuStatus), > VMSTATE_BOOL(is_removing, AcpiCpuStatus), > + VMSTATE_BOOL(is_present, AcpiCpuStatus), > + VMSTATE_BOOL(is_enabled, AcpiCpuStatus), > VMSTATE_UINT32(ost_event, AcpiCpuStatus), > VMSTATE_UINT32(ost_status, AcpiCpuStatus), > VMSTATE_END_OF_LIST() > @@ -339,6 +363,7 @@ const VMStateDescription vmstate_cpu_hotplug = { > #define CPU_REMOVE_EVENT "CRMV" > #define CPU_EJECT_EVENT "CEJ0" > #define CPU_FW_EJECT_EVENT "CEJF" > +#define CPU_PRESENT "CPRS" > > void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, > hwaddr base_addr, > @@ -399,7 +424,9 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, > aml_append(field, aml_named_field(CPU_EJECT_EVENT, 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(3)); > + /* 1 if present, read only */ > + aml_append(field, aml_named_field(CPU_PRESENT, 1)); > + aml_append(field, aml_reserved_field(2)); > aml_append(field, aml_named_field(CPU_COMMAND, 8)); > aml_append(cpu_ctrl_dev, field); > > @@ -429,6 +456,7 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, > Aml *ctrl_lock = aml_name("%s.%s", cphp_res_path, CPU_LOCK); > Aml *cpu_selector = aml_name("%s.%s", cphp_res_path, CPU_SELECTOR); > Aml *is_enabled = aml_name("%s.%s", cphp_res_path, CPU_ENABLED); > + Aml *is_present = aml_name("%s.%s", cphp_res_path, CPU_PRESENT); > Aml *cpu_cmd = aml_name("%s.%s", cphp_res_path, CPU_COMMAND); > Aml *cpu_data = aml_name("%s.%s", cphp_res_path, CPU_DATA); > Aml *ins_evt = aml_name("%s.%s", cphp_res_path, CPU_INSERT_EVENT); > @@ -457,13 +485,26 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, > { > Aml *idx = aml_arg(0); > Aml *sta = aml_local(0); > + Aml *ifctx2; > + Aml *else_ctx; > > aml_append(method, aml_acquire(ctrl_lock, 0xFFFF)); > aml_append(method, aml_store(idx, cpu_selector)); > aml_append(method, aml_store(zero, sta)); > - ifctx = aml_if(aml_equal(is_enabled, one)); > + ifctx = aml_if(aml_equal(is_present, one)); > { > - aml_append(ifctx, aml_store(aml_int(0xF), sta)); > + ifctx2 = aml_if(aml_equal(is_enabled, one)); > + { > + /* cpu is present and enabled */ > + aml_append(ifctx2, aml_store(aml_int(0xF), sta)); > + } > + aml_append(ifctx, ifctx2); > + else_ctx = aml_else(); > + { > + /* cpu is present but disabled */ > + aml_append(else_ctx, aml_store(aml_int(0xD), sta)); For the current kernel patches, functional should not be set. So this should be something like 0x5. > + } > + aml_append(ifctx, else_ctx); > } > aml_append(method, ifctx); > aml_append(method, aml_release(ctrl_lock));
diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c index 232720992d..e1299696d3 100644 --- a/hw/acpi/cpu.c +++ b/hw/acpi/cpu.c @@ -63,10 +63,11 @@ static uint64_t cpu_hotplug_rd(void *opaque, hwaddr addr, unsigned size) cdev = &cpu_st->devs[cpu_st->selector]; switch (addr) { case ACPI_CPU_FLAGS_OFFSET_RW: /* pack and return is_* fields */ - val |= cdev->cpu ? 1 : 0; + val |= cdev->is_enabled ? 1 : 0; val |= cdev->is_inserting ? 2 : 0; val |= cdev->is_removing ? 4 : 0; val |= cdev->fw_remove ? 16 : 0; + val |= cdev->is_present ? 32 : 0; trace_cpuhp_acpi_read_flags(cpu_st->selector, val); break; case ACPI_CPU_CMD_DATA_OFFSET_RW: @@ -228,7 +229,21 @@ void cpu_hotplug_hw_init(MemoryRegion *as, Object *owner, struct CPUState *cpu = CPU(id_list->cpus[i].cpu); if (qemu_present_cpu(cpu)) { state->devs[i].cpu = cpu; + state->devs[i].is_present = true; + } else { + if (qemu_persistent_cpu(cpu)) { + state->devs[i].is_present = true; + } else { + state->devs[i].is_present = false; + } } + + if (qemu_enabled_cpu(cpu)) { + state->devs[i].is_enabled = true; + } else { + state->devs[i].is_enabled = false; + } + state->devs[i].arch_id = id_list->cpus[i].arch_id; } memory_region_init_io(&state->ctrl_reg, owner, &cpu_hotplug_ops, state, @@ -261,6 +276,8 @@ void acpi_cpu_plug_cb(HotplugHandler *hotplug_dev, } cdev->cpu = CPU(dev); + cdev->is_present = true; + cdev->is_enabled = true; if (dev->hotplugged) { cdev->is_inserting = true; acpi_send_event(DEVICE(hotplug_dev), ACPI_CPU_HOTPLUG_STATUS); @@ -292,6 +309,11 @@ void acpi_cpu_unplug_cb(CPUHotplugState *cpu_st, return; } + cdev->is_enabled = false; + if (!qemu_persistent_cpu(CPU(dev))) { + cdev->is_present = false; + } + cdev->cpu = NULL; } @@ -302,6 +324,8 @@ static const VMStateDescription vmstate_cpuhp_sts = { .fields = (VMStateField[]) { VMSTATE_BOOL(is_inserting, AcpiCpuStatus), VMSTATE_BOOL(is_removing, AcpiCpuStatus), + VMSTATE_BOOL(is_present, AcpiCpuStatus), + VMSTATE_BOOL(is_enabled, AcpiCpuStatus), VMSTATE_UINT32(ost_event, AcpiCpuStatus), VMSTATE_UINT32(ost_status, AcpiCpuStatus), VMSTATE_END_OF_LIST() @@ -339,6 +363,7 @@ const VMStateDescription vmstate_cpu_hotplug = { #define CPU_REMOVE_EVENT "CRMV" #define CPU_EJECT_EVENT "CEJ0" #define CPU_FW_EJECT_EVENT "CEJF" +#define CPU_PRESENT "CPRS" void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, hwaddr base_addr, @@ -399,7 +424,9 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, aml_append(field, aml_named_field(CPU_EJECT_EVENT, 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(3)); + /* 1 if present, read only */ + aml_append(field, aml_named_field(CPU_PRESENT, 1)); + aml_append(field, aml_reserved_field(2)); aml_append(field, aml_named_field(CPU_COMMAND, 8)); aml_append(cpu_ctrl_dev, field); @@ -429,6 +456,7 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, Aml *ctrl_lock = aml_name("%s.%s", cphp_res_path, CPU_LOCK); Aml *cpu_selector = aml_name("%s.%s", cphp_res_path, CPU_SELECTOR); Aml *is_enabled = aml_name("%s.%s", cphp_res_path, CPU_ENABLED); + Aml *is_present = aml_name("%s.%s", cphp_res_path, CPU_PRESENT); Aml *cpu_cmd = aml_name("%s.%s", cphp_res_path, CPU_COMMAND); Aml *cpu_data = aml_name("%s.%s", cphp_res_path, CPU_DATA); Aml *ins_evt = aml_name("%s.%s", cphp_res_path, CPU_INSERT_EVENT); @@ -457,13 +485,26 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, { Aml *idx = aml_arg(0); Aml *sta = aml_local(0); + Aml *ifctx2; + Aml *else_ctx; aml_append(method, aml_acquire(ctrl_lock, 0xFFFF)); aml_append(method, aml_store(idx, cpu_selector)); aml_append(method, aml_store(zero, sta)); - ifctx = aml_if(aml_equal(is_enabled, one)); + ifctx = aml_if(aml_equal(is_present, one)); { - aml_append(ifctx, aml_store(aml_int(0xF), sta)); + ifctx2 = aml_if(aml_equal(is_enabled, one)); + { + /* cpu is present and enabled */ + aml_append(ifctx2, aml_store(aml_int(0xF), sta)); + } + aml_append(ifctx, ifctx2); + else_ctx = aml_else(); + { + /* cpu is present but disabled */ + aml_append(else_ctx, aml_store(aml_int(0xD), sta)); + } + aml_append(ifctx, else_ctx); } aml_append(method, ifctx); aml_append(method, aml_release(ctrl_lock)); diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c index d2fa1d0e4a..b84602b238 100644 --- a/hw/acpi/generic_event_device.c +++ b/hw/acpi/generic_event_device.c @@ -319,6 +319,16 @@ static const VMStateDescription vmstate_memhp_state = { } }; +static const VMStateDescription vmstate_cpuhp_state = { + .name = "acpi-ged/cpuhp", + .version_id = 1, + .minimum_version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_CPU_HOTPLUG(cpuhp_state, AcpiGedState), + VMSTATE_END_OF_LIST() + } +}; + static const VMStateDescription vmstate_ged_state = { .name = "acpi-ged-state", .version_id = 1, @@ -367,6 +377,7 @@ static const VMStateDescription vmstate_acpi_ged = { }, .subsections = (const VMStateDescription * []) { &vmstate_memhp_state, + &vmstate_cpuhp_state, &vmstate_ghes_state, NULL } diff --git a/include/hw/acpi/cpu.h b/include/hw/acpi/cpu.h index b87ebfdf4b..786a30d6d4 100644 --- a/include/hw/acpi/cpu.h +++ b/include/hw/acpi/cpu.h @@ -22,6 +22,8 @@ typedef struct AcpiCpuStatus { uint64_t arch_id; bool is_inserting; bool is_removing; + bool is_present; + bool is_enabled; bool fw_remove; uint32_t ost_event; uint32_t ost_status;
ACPI AML changes to properly reflect the _STA.PRES and _STA.ENA Bits to the guest during initialzation, when CPUs are hotplugged and after CPUs are hot-unplugged. Signed-off-by: Salil Mehta <salil.mehta@huawei.com> --- hw/acpi/cpu.c | 49 +++++++++++++++++++++++++++++++--- hw/acpi/generic_event_device.c | 11 ++++++++ include/hw/acpi/cpu.h | 2 ++ 3 files changed, 58 insertions(+), 4 deletions(-)