| Message ID | 20230926100436.28284-19-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: > ARM arch does not allow CPUs presence to be changed [1] after kernel has booted. > Hence, firmware/ACPI/Qemu must ensure persistent view of the vCPUs to the Guest > kernel even when they are not present in the QoM i.e. are unplugged or are > yet-to-be-plugged > > References: > [1] Check comment 5 in the bugzilla entry > Link: https://bugzilla.tianocore.org/show_bug.cgi?id=4481#c5 > > Signed-off-by: Salil Mehta <salil.mehta@huawei.com> > --- > cpus-common.c | 6 ++++++ > hw/arm/virt.c | 7 +++++++ > include/hw/core/cpu.h | 20 ++++++++++++++++++++ > 3 files changed, 33 insertions(+) > hmm, it's another CPU state. There are 4 CPU states, plus other 3 CPU states: possible, present, enabled. Now we're having always-present state. I think those CPU states can be squeezed into the previous present state. What we need is to ensure all possible vCPUs are present from the beginning. How are to do something like below? /* * The flag is set for all possible vCPUs in hw/arm/virt.c::virt_possible_cpu_arch_ids() * The idea is the flag is managed by specific board because the always-present is * the special requirements from hw/arm/virt board. */ #define CPU_ARCH_ID_FLAG_ALWAYS_PRESENT (1UL << 0) typedef struct CPUArchId { uint64_t flags; : } static inline bool machine_has_possible_cpu(int index); static inline bool mahine_has_present_cpu(int index) { if (!machine_has_possible_cpu(inde)) { return false; } if (!ms->possible_cpus->cpus[index].flags & CPU_ARCH_ID_FLAG_ALWAYS_PRESENT) { return false; } return true; } static inline bool machine_has_enabled_cpu(int index) { CPUState *cs; if (!machine_has_present_cpu(index)) { return false; } /* I'm thinking of cpu.enabled can be replaced by another * flag in struct CPUArchID::flags due to the fact: * The vCPU's states are managed by board and changed at * creation time or hotplug handlers. */ cs = CPUSTATE(ms->possible_cpus->cpus[i].cpu); if (!cs || !cpu.enabled) { return false; } return true; } > diff --git a/cpus-common.c b/cpus-common.c > index 24c04199a1..d64aa63b19 100644 > --- a/cpus-common.c > +++ b/cpus-common.c > @@ -128,6 +128,12 @@ bool qemu_enabled_cpu(CPUState *cpu) > return cpu && !cpu->disabled; > } > > +bool qemu_persistent_cpu(CPUState *cpu) > +{ > + /* cpu state can be faked to the guest via acpi */ > + return cpu->acpi_persistent; > +} > + > uint64_t qemu_get_cpu_archid(int cpu_index) > { > MachineState *ms = MACHINE(qdev_get_machine()); > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index cbb6199ec6..f1bee569d5 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -3006,6 +3006,13 @@ static void virt_cpu_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev, > return; > } > virt_cpu_set_properties(OBJECT(cs), cpu_slot, errp); > + > + /* > + * To give persistent presence view of vCPUs to the guest, ACPI might need > + * to fake the presence of the vCPUs to the guest but keep them disabled. > + * This shall be used during the init of ACPI Hotplug state and hot-unplug > + */ > + cs->acpi_persistent = true; > } > > static void virt_cpu_plug(HotplugHandler *hotplug_dev, DeviceState *dev, > diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h > index b2201a98ee..dab572c9bd 100644 > --- a/include/hw/core/cpu.h > +++ b/include/hw/core/cpu.h > @@ -425,6 +425,13 @@ struct CPUState { > * By default every CPUState is enabled as of now across all archs. > */ > bool disabled; > + /* > + * On certain architectures, to give persistent view of the 'presence' of > + * vCPUs to the guest, ACPI might need to fake the 'presence' of the vCPUs > + * but keep them ACPI disabled to the guest. This is done by returning > + * _STA.PRES=True and _STA.Ena=False for the unplugged vCPUs in QEMU QoM. > + */ > + bool acpi_persistent; > /* TODO Move common fields from CPUArchState here. */ > int cpu_index; > int cluster_index; > @@ -814,6 +821,19 @@ bool qemu_present_cpu(CPUState *cpu); > */ > bool qemu_enabled_cpu(CPUState *cpu); > > +/** > + * qemu_persistent_cpu: > + * @cpu: The vCPU to check > + * > + * Checks if the vCPU state should always be reflected as *present* via ACPI > + * to the Guest. By default, this is False on all architectures and has to be > + * explicity set during initialization. > + * > + * Returns: True if it is ACPI 'persistent' CPU > + * > + */ > +bool qemu_persistent_cpu(CPUState *cpu); > + > /** > * qemu_get_cpu_archid: > * @cpu_index: possible vCPU for which arch-id needs to be retreived Thanks, Gavin
Hi Gavin, > From: Gavin Shan <gshan@redhat.com> > Sent: Friday, September 29, 2023 12:18 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 18/37] arm/virt: Make ARM vCPU *present* status > ACPI *persistent* > > Hi Salil, > > On 9/26/23 20:04, Salil Mehta wrote: > > ARM arch does not allow CPUs presence to be changed [1] after kernel has booted. > > Hence, firmware/ACPI/Qemu must ensure persistent view of the vCPUs to the Guest > > kernel even when they are not present in the QoM i.e. are unplugged or are > > yet-to-be-plugged > > > > References: > > [1] Check comment 5 in the bugzilla entry > > Link: https://bugzilla.tianocore.org/show_bug.cgi?id=4481#c5 > > > > Signed-off-by: Salil Mehta <salil.mehta@huawei.com> > > --- > > cpus-common.c | 6 ++++++ > > hw/arm/virt.c | 7 +++++++ > > include/hw/core/cpu.h | 20 ++++++++++++++++++++ > > 3 files changed, 33 insertions(+) > > > > hmm, it's another CPU state. There are 4 CPU states, plus other 3 CPU states: > possible, present, enabled. Now we're having always-present state. Possible vCPU is not a QOM CPUState. Neither it gets represented through ACPI to the guest OS through _STA method. A device which in this case is a CPU can be in ENABLED state or can just be PRESENT but not ENABLED. All possible vCPUs get detected by guest OS by the mere presence of GICC Entry (Enabled/online-capable) in the ACPI MADT Table. A plugged vCPU will be 'present' in QOM and will be ACPI _STA.PRESENT as well. A un-plugged vCPU will be 'not-present' in QOM i.e. its CPUState object will be NULL. This is akin to x86 or any other architecture and we are not changing any of this at QOM level. What changes is the representation of un-plugged vCPU to Guest OS via ACPI _STA method. For ARM, we *fake* QOM un-plugged vCPU presence to the Guest OS through the ACPI _STA method. To detect this we check the bool 'acpi_persistent' part of CPUState. This has to be set explicitly by Architectures like ARM which do not support hot-plug. Hence, CPUs are in 'always present' state ACPI wise but cannot be used as they are ACPI disabled i.e. ACPI _STA.ENABLED=0. > I think > those CPU states can be squeezed into the previous present state. What we > need is to ensure all possible vCPUs are present from the beginning. All of this is unnecessary, really. Thanks Salil.
diff --git a/cpus-common.c b/cpus-common.c index 24c04199a1..d64aa63b19 100644 --- a/cpus-common.c +++ b/cpus-common.c @@ -128,6 +128,12 @@ bool qemu_enabled_cpu(CPUState *cpu) return cpu && !cpu->disabled; } +bool qemu_persistent_cpu(CPUState *cpu) +{ + /* cpu state can be faked to the guest via acpi */ + return cpu->acpi_persistent; +} + uint64_t qemu_get_cpu_archid(int cpu_index) { MachineState *ms = MACHINE(qdev_get_machine()); diff --git a/hw/arm/virt.c b/hw/arm/virt.c index cbb6199ec6..f1bee569d5 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -3006,6 +3006,13 @@ static void virt_cpu_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev, return; } virt_cpu_set_properties(OBJECT(cs), cpu_slot, errp); + + /* + * To give persistent presence view of vCPUs to the guest, ACPI might need + * to fake the presence of the vCPUs to the guest but keep them disabled. + * This shall be used during the init of ACPI Hotplug state and hot-unplug + */ + cs->acpi_persistent = true; } static void virt_cpu_plug(HotplugHandler *hotplug_dev, DeviceState *dev, diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h index b2201a98ee..dab572c9bd 100644 --- a/include/hw/core/cpu.h +++ b/include/hw/core/cpu.h @@ -425,6 +425,13 @@ struct CPUState { * By default every CPUState is enabled as of now across all archs. */ bool disabled; + /* + * On certain architectures, to give persistent view of the 'presence' of + * vCPUs to the guest, ACPI might need to fake the 'presence' of the vCPUs + * but keep them ACPI disabled to the guest. This is done by returning + * _STA.PRES=True and _STA.Ena=False for the unplugged vCPUs in QEMU QoM. + */ + bool acpi_persistent; /* TODO Move common fields from CPUArchState here. */ int cpu_index; int cluster_index; @@ -814,6 +821,19 @@ bool qemu_present_cpu(CPUState *cpu); */ bool qemu_enabled_cpu(CPUState *cpu); +/** + * qemu_persistent_cpu: + * @cpu: The vCPU to check + * + * Checks if the vCPU state should always be reflected as *present* via ACPI + * to the Guest. By default, this is False on all architectures and has to be + * explicity set during initialization. + * + * Returns: True if it is ACPI 'persistent' CPU + * + */ +bool qemu_persistent_cpu(CPUState *cpu); + /** * qemu_get_cpu_archid: * @cpu_index: possible vCPU for which arch-id needs to be retreived
ARM arch does not allow CPUs presence to be changed [1] after kernel has booted. Hence, firmware/ACPI/Qemu must ensure persistent view of the vCPUs to the Guest kernel even when they are not present in the QoM i.e. are unplugged or are yet-to-be-plugged References: [1] Check comment 5 in the bugzilla entry Link: https://bugzilla.tianocore.org/show_bug.cgi?id=4481#c5 Signed-off-by: Salil Mehta <salil.mehta@huawei.com> --- cpus-common.c | 6 ++++++ hw/arm/virt.c | 7 +++++++ include/hw/core/cpu.h | 20 ++++++++++++++++++++ 3 files changed, 33 insertions(+)