Message ID | 20210516102900.28036-5-wangyanan55@huawei.com |
---|---|
State | New |
Headers | show |
Series | hw/arm/virt: Introduce cpu topology support | expand |
On Sun, May 16, 2021 at 06:28:55PM +0800, Yanan Wang wrote: > We create and initialize a cpuobj for each present cpu in > machvirt_init(). Now we also initialize the cpu member of > structure CPUArchId for each present cpu in the function. > > This will be used to determine whether a cpu is present > when generating ACPI tables in later patches. > > Co-developed-by: Ying Fang <fangying1@huawei.com> > Signed-off-by: Ying Fang <fangying1@huawei.com> > Signed-off-by: Yanan Wang <wangyanan55@huawei.com> > --- > hw/arm/virt.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index e5dcdebdbc..50e324975f 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -2061,6 +2061,13 @@ static void machvirt_init(MachineState *machine) > } > > qdev_realize(DEVICE(cpuobj), NULL, &error_fatal); > + > + /* > + * As ARM cpu hotplug is not supported yet, we initialize > + * the present cpu members here. > + */ > + machine->possible_cpus->cpus[n].cpu = cpuobj; > + > object_unref(cpuobj); > } > fdt_add_timer_nodes(vms); > -- > 2.19.1 > Reviewed-by: Andrew Jones <drjones@redhat.com>
> From: Qemu-arm [mailto:qemu-arm-bounces+salil.mehta=huawei.com@nongnu.org] > On Behalf Of Yanan Wang > Sent: Sunday, May 16, 2021 11:29 AM > To: Peter Maydell <peter.maydell@linaro.org>; Andrew Jones > <drjones@redhat.com>; Michael S . Tsirkin <mst@redhat.com>; Igor Mammedov > <imammedo@redhat.com>; Shannon Zhao <shannon.zhaosl@gmail.com>; Alistair > Francis <alistair.francis@wdc.com>; David Gibson > <david@gibson.dropbear.id.au>; qemu-devel@nongnu.org; qemu-arm@nongnu.org > Cc: Song Bao Hua (Barry Song) <song.bao.hua@hisilicon.com>; zhukeqian > <zhukeqian1@huawei.com>; yangyicong <yangyicong@huawei.com>; Zengtao (B) > <prime.zeng@hisilicon.com>; Wanghaibin (D) <wanghaibin.wang@huawei.com>; > yuzenghui <yuzenghui@huawei.com>; Paolo Bonzini <pbonzini@redhat.com>; > Philippe Mathieu-Daudé <philmd@redhat.com> > Subject: [RFC PATCH v3 4/9] hw/arm/virt: Initialize the present cpu members > > We create and initialize a cpuobj for each present cpu in > machvirt_init(). Now we also initialize the cpu member of > structure CPUArchId for each present cpu in the function. [...] > qdev_realize(DEVICE(cpuobj), NULL, &error_fatal); > + > + /* > + * As ARM cpu hotplug is not supported yet, we initialize > + * the present cpu members here. > + */ > + machine->possible_cpus->cpus[n].cpu = cpuobj; when vcpu Hotplug is not supported yet, what necessitates this change now?
Hi Salil, On 2021/5/18 4:48, Salil Mehta wrote: >> From: Qemu-arm [mailto:qemu-arm-bounces+salil.mehta=huawei.com@nongnu.org] >> On Behalf Of Yanan Wang >> Sent: Sunday, May 16, 2021 11:29 AM >> To: Peter Maydell <peter.maydell@linaro.org>; Andrew Jones >> <drjones@redhat.com>; Michael S . Tsirkin <mst@redhat.com>; Igor Mammedov >> <imammedo@redhat.com>; Shannon Zhao <shannon.zhaosl@gmail.com>; Alistair >> Francis <alistair.francis@wdc.com>; David Gibson >> <david@gibson.dropbear.id.au>; qemu-devel@nongnu.org; qemu-arm@nongnu.org >> Cc: Song Bao Hua (Barry Song) <song.bao.hua@hisilicon.com>; zhukeqian >> <zhukeqian1@huawei.com>; yangyicong <yangyicong@huawei.com>; Zengtao (B) >> <prime.zeng@hisilicon.com>; Wanghaibin (D) <wanghaibin.wang@huawei.com>; >> yuzenghui <yuzenghui@huawei.com>; Paolo Bonzini <pbonzini@redhat.com>; >> Philippe Mathieu-Daudé <philmd@redhat.com> >> Subject: [RFC PATCH v3 4/9] hw/arm/virt: Initialize the present cpu members >> >> We create and initialize a cpuobj for each present cpu in >> machvirt_init(). Now we also initialize the cpu member of >> structure CPUArchId for each present cpu in the function. > [...] > >> qdev_realize(DEVICE(cpuobj), NULL, &error_fatal); >> + >> + /* >> + * As ARM cpu hotplug is not supported yet, we initialize >> + * the present cpu members here. >> + */ >> + machine->possible_cpus->cpus[n].cpu = cpuobj; > > when vcpu Hotplug is not supported yet, what necessitates this change now? > The initialization will gives a way to determine whether a CPU is present or not. At least, for now it will be used when generating ACPI tables, e.g. DSDT, MADT. See patch 5 and 6. Thanks, Yanan > .
> From: wangyanan (Y) > Sent: Tuesday, May 18, 2021 5:43 AM > > Hi Salil, > > On 2021/5/18 4:48, Salil Mehta wrote: > >> From: Qemu-arm > [mailto:qemu-arm-bounces+salil.mehta=huawei.com@nongnu.org] > >> On Behalf Of Yanan Wang > >> Sent: Sunday, May 16, 2021 11:29 AM > >> To: Peter Maydell <peter.maydell@linaro.org>; Andrew Jones > >> <drjones@redhat.com>; Michael S . Tsirkin <mst@redhat.com>; Igor Mammedov > >> <imammedo@redhat.com>; Shannon Zhao <shannon.zhaosl@gmail.com>; Alistair > >> Francis <alistair.francis@wdc.com>; David Gibson > >> <david@gibson.dropbear.id.au>; qemu-devel@nongnu.org; qemu-arm@nongnu.org > >> Cc: Song Bao Hua (Barry Song) <song.bao.hua@hisilicon.com>; zhukeqian > >> <zhukeqian1@huawei.com>; yangyicong <yangyicong@huawei.com>; Zengtao (B) > >> <prime.zeng@hisilicon.com>; Wanghaibin (D) <wanghaibin.wang@huawei.com>; > >> yuzenghui <yuzenghui@huawei.com>; Paolo Bonzini <pbonzini@redhat.com>; > >> Philippe Mathieu-Daudé <philmd@redhat.com> > >> Subject: [RFC PATCH v3 4/9] hw/arm/virt: Initialize the present cpu members > >> > >> We create and initialize a cpuobj for each present cpu in > >> machvirt_init(). Now we also initialize the cpu member of > >> structure CPUArchId for each present cpu in the function. > > [...] > > > >> qdev_realize(DEVICE(cpuobj), NULL, &error_fatal); > >> + > >> + /* > >> + * As ARM cpu hotplug is not supported yet, we initialize > >> + * the present cpu members here. > >> + */ > >> + machine->possible_cpus->cpus[n].cpu = cpuobj; > > > > when vcpu Hotplug is not supported yet, what necessitates this change now? > > > The initialization will gives a way to determine whether a CPU is > present or not. > At least, for now it will be used when generating ACPI tables, e.g. > DSDT, MADT. > See patch 5 and 6. yes, but why do you require it now as part of the vcpu topology change? As-far-as-i-can-see, PPTT table changes(part of patch 5/9) do not require this change. Change in Patch 5/9 has also been done in anticipation of some future requirement(vcpu Hotplug?). Please correct me here if I am wrong? Thanks
On Tue, May 18, 2021 at 07:04:51AM +0000, Salil Mehta wrote: > > From: wangyanan (Y) > > Sent: Tuesday, May 18, 2021 5:43 AM > > > > Hi Salil, > > > > On 2021/5/18 4:48, Salil Mehta wrote: > > >> From: Qemu-arm > > [mailto:qemu-arm-bounces+salil.mehta=huawei.com@nongnu.org] > > >> On Behalf Of Yanan Wang > > >> Sent: Sunday, May 16, 2021 11:29 AM > > >> To: Peter Maydell <peter.maydell@linaro.org>; Andrew Jones > > >> <drjones@redhat.com>; Michael S . Tsirkin <mst@redhat.com>; Igor Mammedov > > >> <imammedo@redhat.com>; Shannon Zhao <shannon.zhaosl@gmail.com>; Alistair > > >> Francis <alistair.francis@wdc.com>; David Gibson > > >> <david@gibson.dropbear.id.au>; qemu-devel@nongnu.org; qemu-arm@nongnu.org > > >> Cc: Song Bao Hua (Barry Song) <song.bao.hua@hisilicon.com>; zhukeqian > > >> <zhukeqian1@huawei.com>; yangyicong <yangyicong@huawei.com>; Zengtao (B) > > >> <prime.zeng@hisilicon.com>; Wanghaibin (D) <wanghaibin.wang@huawei.com>; > > >> yuzenghui <yuzenghui@huawei.com>; Paolo Bonzini <pbonzini@redhat.com>; > > >> Philippe Mathieu-Daudé <philmd@redhat.com> > > >> Subject: [RFC PATCH v3 4/9] hw/arm/virt: Initialize the present cpu members > > >> > > >> We create and initialize a cpuobj for each present cpu in > > >> machvirt_init(). Now we also initialize the cpu member of > > >> structure CPUArchId for each present cpu in the function. > > > [...] > > > > > >> qdev_realize(DEVICE(cpuobj), NULL, &error_fatal); > > >> + > > >> + /* > > >> + * As ARM cpu hotplug is not supported yet, we initialize > > >> + * the present cpu members here. > > >> + */ > > >> + machine->possible_cpus->cpus[n].cpu = cpuobj; > > > > > > when vcpu Hotplug is not supported yet, what necessitates this change now? > > > > > The initialization will gives a way to determine whether a CPU is > > present or not. > > At least, for now it will be used when generating ACPI tables, e.g. > > DSDT, MADT. > > See patch 5 and 6. > > yes, but why do you require it now as part of the vcpu topology change? > > As-far-as-i-can-see, PPTT table changes(part of patch 5/9) do not require > this change. Change in Patch 5/9 has also been done in anticipation of > some future requirement(vcpu Hotplug?). > > Please correct me here if I am wrong? > Hi Salil, The problem is that we've never required smp.cpus == smp.maxcpus, so a user could have smp.cpus < smp.maxcpus. We want the topology to match maxcpus, but only enable cpus. However, if you think we should just not allow cpus < maxcpus until hot plug is sorted out, then we could discuss a way of trying to enforce cpus == maxcpus, but I'm not sure how we can without breaking existing command lines. Thanks, drew
> From: Andrew Jones [mailto:drjones@redhat.com] > Sent: Tuesday, May 18, 2021 8:50 AM > > On Tue, May 18, 2021 at 07:04:51AM +0000, Salil Mehta wrote: > > > From: wangyanan (Y) > > > Sent: Tuesday, May 18, 2021 5:43 AM > > > > > > Hi Salil, > > > > > > On 2021/5/18 4:48, Salil Mehta wrote: > > > >> From: Qemu-arm > > > [mailto:qemu-arm-bounces+salil.mehta=huawei.com@nongnu.org] > > > >> On Behalf Of Yanan Wang > > > >> Sent: Sunday, May 16, 2021 11:29 AM > > > >> To: Peter Maydell <peter.maydell@linaro.org>; Andrew Jones > > > >> <drjones@redhat.com>; Michael S . Tsirkin <mst@redhat.com>; Igor Mammedov > > > >> <imammedo@redhat.com>; Shannon Zhao <shannon.zhaosl@gmail.com>; Alistair > > > >> Francis <alistair.francis@wdc.com>; David Gibson > > > >> <david@gibson.dropbear.id.au>; qemu-devel@nongnu.org; > qemu-arm@nongnu.org > > > >> Cc: Song Bao Hua (Barry Song) <song.bao.hua@hisilicon.com>; zhukeqian > > > >> <zhukeqian1@huawei.com>; yangyicong <yangyicong@huawei.com>; Zengtao (B) > > > >> <prime.zeng@hisilicon.com>; Wanghaibin (D) > <wanghaibin.wang@huawei.com>; > > > >> yuzenghui <yuzenghui@huawei.com>; Paolo Bonzini <pbonzini@redhat.com>; > > > >> Philippe Mathieu-Daudé <philmd@redhat.com> > > > >> Subject: [RFC PATCH v3 4/9] hw/arm/virt: Initialize the present cpu members > > > >> > > > >> We create and initialize a cpuobj for each present cpu in > > > >> machvirt_init(). Now we also initialize the cpu member of > > > >> structure CPUArchId for each present cpu in the function. > > > > [...] > > > > > > > >> qdev_realize(DEVICE(cpuobj), NULL, &error_fatal); > > > >> + > > > >> + /* > > > >> + * As ARM cpu hotplug is not supported yet, we initialize > > > >> + * the present cpu members here. > > > >> + */ > > > >> + machine->possible_cpus->cpus[n].cpu = cpuobj; > > > > > > > > when vcpu Hotplug is not supported yet, what necessitates this change now? > > > > > > > The initialization will gives a way to determine whether a CPU is > > > present or not. > > > At least, for now it will be used when generating ACPI tables, e.g. > > > DSDT, MADT. > > > See patch 5 and 6. > > > > yes, but why do you require it now as part of the vcpu topology change? > > > > As-far-as-i-can-see, PPTT table changes(part of patch 5/9) do not require > > this change. Change in Patch 5/9 has also been done in anticipation of > > some future requirement(vcpu Hotplug?). > > > > Please correct me here if I am wrong? > > > > Hi Salil, > > The problem is that we've never required smp.cpus == smp.maxcpus, so > a user could have smp.cpus < smp.maxcpus. We want the topology to match > maxcpus, but only enable cpus. However, if you think we should just not > allow cpus < maxcpus until hot plug is sorted out, then we could discuss > a way of trying to enforce cpus == maxcpus, but I'm not sure how we can > without breaking existing command lines. Hi Andrew, Ideally, if the vcpu Hotplug is not supported the check in the smp_parse() should impose (cpus == maxcpus). This as of now is just a warning of invalid configuration I think. Beside this does not breaks any prior usages which you suggested might happen? Again, this is not a blocking issue from my side but just a humble suggestion. You might want to take a call on this :) Thanks Salil.
diff --git a/hw/arm/virt.c b/hw/arm/virt.c index e5dcdebdbc..50e324975f 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -2061,6 +2061,13 @@ static void machvirt_init(MachineState *machine) } qdev_realize(DEVICE(cpuobj), NULL, &error_fatal); + + /* + * As ARM cpu hotplug is not supported yet, we initialize + * the present cpu members here. + */ + machine->possible_cpus->cpus[n].cpu = cpuobj; + object_unref(cpuobj); } fdt_add_timer_nodes(vms);