Re: [RFC PATCH v3 4/9] hw/arm/virt: Initialize the present cpu members
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.
participants (2)
-
Andrew Jones -
Salil Mehta