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)
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.
-
Andrew Jones -
Salil Mehta