Hi Gavin, > From: Gavin Shan <gs...@redhat.com> > Sent: Wednesday, September 27, 2023 6:17 AM > To: Salil Mehta <salil.me...@huawei.com>; qemu-devel@nongnu.org; > qemu-...@nongnu.org > Cc: m...@kernel.org; jean-phili...@linaro.org; Jonathan Cameron > <jonathan.came...@huawei.com>; lpieral...@kernel.org; > peter.mayd...@linaro.org; richard.hender...@linaro.org; > imamm...@redhat.com; andrew.jo...@linux.dev; da...@redhat.com; > phi...@linaro.org; eric.au...@redhat.com; w...@kernel.org; a...@kernel.org; > oliver.up...@linux.dev; pbonz...@redhat.com; m...@redhat.com; > raf...@kernel.org; borntrae...@linux.ibm.com; alex.ben...@linaro.org; > li...@armlinux.org.uk; dar...@os.amperecomputing.com; > il...@os.amperecomputing.com; vis...@os.amperecomputing.com; > karl.heub...@oracle.com; miguel.l...@oracle.com; salil.me...@opnsrc.net; > zhukeqian <zhukeqi...@huawei.com>; wangxiongfeng (C) > <wangxiongfe...@huawei.com>; wangyanan (Y) <wangyana...@huawei.com>; > jiakern...@gmail.com; maob...@loongson.cn; lixiang...@loongson.cn > Subject: Re: [PATCH RFC V2 03/37] hw/arm/virt: Move setting of common CPU > properties in a function > > Hi Salil, > > On 9/26/23 20:04, Salil Mehta wrote: > > Factor out CPU properties code common for {hot,cold}-plugged CPUs. This > allows > > code reuse. > > > > Signed-off-by: Salil Mehta <salil.me...@huawei.com> > > --- > > hw/arm/virt.c | 220 ++++++++++++++++++++++++++---------------- > > include/hw/arm/virt.h | 4 + > > 2 files changed, 140 insertions(+), 84 deletions(-) > > > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > > index 57fe97c242..0eb6bf5a18 100644 > > --- a/hw/arm/virt.c > > +++ b/hw/arm/virt.c > > @@ -2018,16 +2018,130 @@ static void virt_cpu_post_init(VirtMachineState > *vms, MemoryRegion *sysmem) > > } > > } > >
[...] > > + } > > + > > + /* > > + * RFC: Question: this must only be called for the hotplugged cpus. > > For the > > + * cold booted secondary cpus this is being taken care in > > arm_load_kernel() > > + * in boot.c. Perhaps we should remove that code now? > > + */ > > + if (vms->psci_conduit != QEMU_PSCI_CONDUIT_DISABLED) { > > + object_property_set_int(cpuobj, "psci-conduit", vms->psci_conduit, > > + NULL); > > + > > + /* Secondary CPUs start in PSCI powered-down state */ > > + if (CPU(cpuobj)->cpu_index > 0) { > > + object_property_set_bool(cpuobj, "start-powered-off", true, > > NULL); > > + } > > + } > > + > > +out: > > + if (local_err) { > > + error_propagate(errp, local_err); > > + } > > + return; > ^^^^^^ > > It's not needed obviously :) Yep, will remove that. Thanks Salil.