RE: [PATCH RFC V2 12/37] hw/acpi: Use qemu_present_cpu() API in ACPI CPU hotplug init

[Salil Mehta]

> From: Gavin Shan <gshan@redhat.com>
> Sent: Thursday, September 28, 2023 1:41 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 12/37] hw/acpi: Use qemu_present_cpu() API in
> ACPI CPU hotplug init
> 
> Hi Salil,
> 
> On 9/26/23 20:04, Salil Mehta wrote:
> > ACPI CPU Hotplug code assumes a virtual CPU is unplugged if the CPUState 
> > object
> > is absent in the list of ths possible CPUs(CPUArchIdList *possible_cpus)
> > maintained on per-machine basis. Use the earlier introduced 
> > qemu_present_cpu()
> > API to check this state.
> >
> > This change should have no bearing on the functionality of any architecture 
> > and
> > is mere a representational change.
> >
> > Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> > ---
> >   hw/acpi/cpu.c | 5 ++++-
> >   1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
> > index 45defdc0e2..d5ba37b209 100644
> > --- a/hw/acpi/cpu.c
> > +++ b/hw/acpi/cpu.c
> > @@ -225,7 +225,10 @@ void cpu_hotplug_hw_init(MemoryRegion *as, Object 
> > *owner,
> >       state->dev_count = id_list->len;
> >       state->devs = g_new0(typeof(*state->devs), state->dev_count);
> >       for (i = 0; i < id_list->len; i++) {
> > -        state->devs[i].cpu =  CPU(id_list->cpus[i].cpu);
> > +        struct CPUState *cpu = CPU(id_list->cpus[i].cpu);
> > +        if (qemu_present_cpu(cpu)) {
> > +            state->devs[i].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,
> 
> I don't think qemu_present_cpu() is needed because all possible vCPUs are 
> present
> for x86 and arm64 at this point? Besides, we have the assumption all 
> hotpluggable
> vCPUs are present, looking at James' kernel series where 
> ACPI_HOTPLUG_PRESENT_CPU
> exists in linux/drivers/acpi/Kconfig :)

No, for x86 not all possible vCPUs need to be present at VM init. IOAPIC has
got no such limitation like GIC. Hot-pluggable vCPUs can be deferred created.
Hence, QOM CPUState objects will not be present and ACPI will expose them as
NOT PRESENT to the Guest OS.

This is not the case with ARM. Not all possible vCPUs are present at QOM
level but QEMU *fakes* the presence of vCPUs i.e. the unplugged ones to
the Guest OS by exposing them as PRESENT in ACPI _STA method. Later is
required due to the architectural limitation of ARM. GIC CPU interfaces
need to be present in MADT for all possible vCPUs at guest boot time.

Hence, we require above change.


Thanks
Salil.