linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* RE: [RFC PATCH v3 6/9] hw/arm/virt-acpi-build: Use possible cpus in generation of MADT
       [not found]     ` <14284387-dcf4-cfb2-127c-5d1e1d0ecb79@huawei.com>
@ 2021-05-18  6:47       ` Salil Mehta
  2021-05-18 11:58         ` wangyanan (Y)
  0 siblings, 1 reply; 5+ messages in thread
From: Salil Mehta @ 2021-05-18  6:47 UTC (permalink / raw)
  To: wangyanan (Y)
  Cc: Peter Maydell, Andrew Jones, Michael S . Tsirkin, Igor Mammedov,
	Shannon Zhao, Alistair Francis, David Gibson, qemu-devel,
	qemu-arm, Paolo Bonzini, Philippe Mathieu-Daudé,
	yangyicong, Zengtao (B), Song Bao Hua (Barry Song),
	Wanghaibin (D),
	zhukeqian, yuzenghui, linux-kernel, linuxarm

> From: wangyanan (Y)
> Sent: Tuesday, May 18, 2021 6:03 AM
> 
> Hi Salil,
> 
> On 2021/5/18 1:07, 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 6/9] hw/arm/virt-acpi-build: Use possible cpus in
> >> generation of MADT
> >>
> >> When building ACPI tables regarding CPUs we should always build
> >> them for the number of possible CPUs, not the number of present
> >> CPUs. So we create gicc nodes in MADT for possible cpus and then
> >> ensure only the present CPUs are marked ENABLED. Furthermore, it
> >> also needed if we are going to support CPU hotplug in the future.
> > Hi Yanan,
> > Yes, these changes are part of the QEMU patch-set I floated last year.
> >
> > Link: https://www.mail-archive.com/qemu-devel@nongnu.org/msg712018.html
> Yes, I noticed this. Thanks!
> >
> > Perhaps I am missing something, but how this patch is related to the vcpu
> > topology support?
> No related actually. But this patch together with patch 5 aim to provide
> complete information (all cpus including enabled and the others) to guest,
> which will be more consistent with requirement in ACPI spec.


Well, if it is not related to the cpu topology support then this and other
similar patches included with the same line of thought should not be
part of this patch-set. 

I am already working with ARM folks in this regard.

Thanks

> 
> We don't consider cpu hotplug at all in this patch, but it indeed pave way
> for cpu hotplug in the future.
> 
> Thanks,
> Yanan
> > Thanks
> >
> >> Co-developed-by: Andrew Jones <drjones@redhat.com>
> >> Signed-off-by: Andrew Jones <drjones@redhat.com>
> >> Co-developed-by: Ying Fang <fangying1@huawei.com>
> >> Signed-off-by: Ying Fang <fangying1@huawei.com>
> >> Co-developed-by: Yanan Wang <wangyanan55@huawei.com>
> >> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> >> ---
> >>   hw/arm/virt-acpi-build.c | 29 +++++++++++++++++++++++++----
> >>   1 file changed, 25 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> >> index a2d8e87616..4d64aeb865 100644
> >> --- a/hw/arm/virt-acpi-build.c
> >> +++ b/hw/arm/virt-acpi-build.c
> >> @@ -481,6 +481,9 @@ build_madt(GArray *table_data, BIOSLinker *linker,
> >> VirtMachineState *vms)
> >>       const int *irqmap = vms->irqmap;
> >>       AcpiMadtGenericDistributor *gicd;
> >>       AcpiMadtGenericMsiFrame *gic_msi;
> >> +    MachineClass *mc = MACHINE_GET_CLASS(vms);
> >> +    const CPUArchIdList *possible_cpus =
> >> mc->possible_cpu_arch_ids(MACHINE(vms));
> >> +    bool pmu;
> >>       int i;
> >>
> >>       acpi_data_push(table_data, sizeof(AcpiMultipleApicTable));
> >> @@ -491,11 +494,21 @@ build_madt(GArray *table_data, BIOSLinker *linker,
> >> VirtMachineState *vms)
> >>       gicd->base_address = cpu_to_le64(memmap[VIRT_GIC_DIST].base);
> >>       gicd->version = vms->gic_version;
> >>
> >> -    for (i = 0; i < MACHINE(vms)->smp.cpus; i++) {
> >> +    for (i = 0; i < possible_cpus->len; i++) {
> >>           AcpiMadtGenericCpuInterface *gicc = acpi_data_push(table_data,
> >>                                                              sizeof(*gicc));
> >>           ARMCPU *armcpu = ARM_CPU(qemu_get_cpu(i));
> >>
> >> +        /*
> >> +         * PMU should have been either implemented for all CPUs or not,
> >> +         * so we only get information from the first CPU, which could
> >> +         * represent the others.
> >> +         */
> >> +        if (i == 0) {
> >> +            pmu = arm_feature(&armcpu->env, ARM_FEATURE_PMU);
> >> +        }
> >> +        assert(!armcpu || arm_feature(&armcpu->env, ARM_FEATURE_PMU) ==
> pmu);
> >> +
> >>           gicc->type = ACPI_APIC_GENERIC_CPU_INTERFACE;
> >>           gicc->length = sizeof(*gicc);
> >>           if (vms->gic_version == 2) {
> >> @@ -504,11 +517,19 @@ build_madt(GArray *table_data, BIOSLinker *linker,
> >> VirtMachineState *vms)
> >>               gicc->gicv_base_address =
> >> cpu_to_le64(memmap[VIRT_GIC_VCPU].base);
> >>           }
> >>           gicc->cpu_interface_number = cpu_to_le32(i);
> >> -        gicc->arm_mpidr = cpu_to_le64(armcpu->mp_affinity);
> >> +        gicc->arm_mpidr = cpu_to_le64(possible_cpus->cpus[i].arch_id);
> >>           gicc->uid = cpu_to_le32(i);
> >> -        gicc->flags = cpu_to_le32(ACPI_MADT_GICC_ENABLED);
> >>
> >> -        if (arm_feature(&armcpu->env, ARM_FEATURE_PMU)) {
> >> +        /*
> >> +         * ACPI spec says that LAPIC entry for non present CPU may be
> >> +         * omitted from MADT or it must be marked as disabled. Here we
> >> +         * choose to also keep the disabled ones in MADT.
> >> +         */
> >> +        if (possible_cpus->cpus[i].cpu != NULL) {
> >> +            gicc->flags = cpu_to_le32(ACPI_MADT_GICC_ENABLED);
> >> +        }
> >> +
> >> +        if (pmu) {
> >>               gicc->performance_interrupt =
> cpu_to_le32(PPI(VIRTUAL_PMU_IRQ));
> >>           }
> >>           if (vms->virt) {
> >> --
> >> 2.19.1
> >>
> > .

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [RFC PATCH v3 4/9] hw/arm/virt: Initialize the present cpu members
       [not found]     ` <68883a1b-5303-da13-a051-e909e1d1f71b@huawei.com>
@ 2021-05-18  7:04       ` Salil Mehta
  2021-05-18  7:50         ` Andrew Jones
  0 siblings, 1 reply; 5+ messages in thread
From: Salil Mehta @ 2021-05-18  7:04 UTC (permalink / raw)
  To: wangyanan (Y)
  Cc: Peter Maydell, Andrew Jones, Michael S . Tsirkin, Igor Mammedov,
	Shannon Zhao, Alistair Francis, David Gibson, qemu-devel,
	qemu-arm, Paolo Bonzini, Philippe Mathieu-Daudé,
	yangyicong, Zengtao (B), Song Bao Hua (Barry Song),
	Wanghaibin (D),
	zhukeqian, yuzenghui, linux-kernel, linuxarm

> 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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC PATCH v3 4/9] hw/arm/virt: Initialize the present cpu members
  2021-05-18  7:04       ` [RFC PATCH v3 4/9] hw/arm/virt: Initialize the present cpu members Salil Mehta
@ 2021-05-18  7:50         ` Andrew Jones
  2021-05-18 18:50           ` Salil Mehta
  0 siblings, 1 reply; 5+ messages in thread
From: Andrew Jones @ 2021-05-18  7:50 UTC (permalink / raw)
  To: Salil Mehta
  Cc: wangyanan (Y),
	Peter Maydell, Michael S . Tsirkin, Igor Mammedov, Shannon Zhao,
	Alistair Francis, David Gibson, qemu-devel, qemu-arm,
	Paolo Bonzini, Philippe Mathieu-Daudé,
	yangyicong, Zengtao (B), Song Bao Hua (Barry Song),
	Wanghaibin (D),
	zhukeqian, yuzenghui, linux-kernel, linuxarm

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC PATCH v3 6/9] hw/arm/virt-acpi-build: Use possible cpus in generation of MADT
  2021-05-18  6:47       ` [RFC PATCH v3 6/9] hw/arm/virt-acpi-build: Use possible cpus in generation of MADT Salil Mehta
@ 2021-05-18 11:58         ` wangyanan (Y)
  0 siblings, 0 replies; 5+ messages in thread
From: wangyanan (Y) @ 2021-05-18 11:58 UTC (permalink / raw)
  To: Salil Mehta
  Cc: Peter Maydell, Andrew Jones, Michael S . Tsirkin, Igor Mammedov,
	Shannon Zhao, Alistair Francis, David Gibson, qemu-devel,
	qemu-arm, Paolo Bonzini, Philippe Mathieu-Daudé,
	yangyicong, Zengtao (B), Song Bao Hua (Barry Song),
	Wanghaibin (D),
	zhukeqian, yuzenghui, linux-kernel, linuxarm


On 2021/5/18 14:47, Salil Mehta wrote:
>> From: wangyanan (Y)
>> Sent: Tuesday, May 18, 2021 6:03 AM
>>
>> Hi Salil,
>>
>> On 2021/5/18 1:07, 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 6/9] hw/arm/virt-acpi-build: Use possible cpus in
>>>> generation of MADT
>>>>
>>>> When building ACPI tables regarding CPUs we should always build
>>>> them for the number of possible CPUs, not the number of present
>>>> CPUs. So we create gicc nodes in MADT for possible cpus and then
>>>> ensure only the present CPUs are marked ENABLED. Furthermore, it
>>>> also needed if we are going to support CPU hotplug in the future.
>>> Hi Yanan,
>>> Yes, these changes are part of the QEMU patch-set I floated last year.
>>>
>>> Link: https://www.mail-archive.com/qemu-devel@nongnu.org/msg712018.html
>> Yes, I noticed this. Thanks!
>>> Perhaps I am missing something, but how this patch is related to the vcpu
>>> topology support?
>> No related actually. But this patch together with patch 5 aim to provide
>> complete information (all cpus including enabled and the others) to guest,
>> which will be more consistent with requirement in ACPI spec.
>
> Well, if it is not related to the cpu topology support then this and other
> similar patches included with the same line of thought should not be
> part of this patch-set.
>
> I am already working with ARM folks in this regard.
Hi Salil,

I'm planning to pack this part into a separate patchset and may repost
it another time, given that there are still some issues to solve.

Thanks,
Yanan
> Thanks
>
>> We don't consider cpu hotplug at all in this patch, but it indeed pave way
>> for cpu hotplug in the future.
>>
>> Thanks,
>> Yanan
>>> Thanks
>>>
>>>> Co-developed-by: Andrew Jones <drjones@redhat.com>
>>>> Signed-off-by: Andrew Jones <drjones@redhat.com>
>>>> Co-developed-by: Ying Fang <fangying1@huawei.com>
>>>> Signed-off-by: Ying Fang <fangying1@huawei.com>
>>>> Co-developed-by: Yanan Wang <wangyanan55@huawei.com>
>>>> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
>>>> ---
>>>>    hw/arm/virt-acpi-build.c | 29 +++++++++++++++++++++++++----
>>>>    1 file changed, 25 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>>>> index a2d8e87616..4d64aeb865 100644
>>>> --- a/hw/arm/virt-acpi-build.c
>>>> +++ b/hw/arm/virt-acpi-build.c
>>>> @@ -481,6 +481,9 @@ build_madt(GArray *table_data, BIOSLinker *linker,
>>>> VirtMachineState *vms)
>>>>        const int *irqmap = vms->irqmap;
>>>>        AcpiMadtGenericDistributor *gicd;
>>>>        AcpiMadtGenericMsiFrame *gic_msi;
>>>> +    MachineClass *mc = MACHINE_GET_CLASS(vms);
>>>> +    const CPUArchIdList *possible_cpus =
>>>> mc->possible_cpu_arch_ids(MACHINE(vms));
>>>> +    bool pmu;
>>>>        int i;
>>>>
>>>>        acpi_data_push(table_data, sizeof(AcpiMultipleApicTable));
>>>> @@ -491,11 +494,21 @@ build_madt(GArray *table_data, BIOSLinker *linker,
>>>> VirtMachineState *vms)
>>>>        gicd->base_address = cpu_to_le64(memmap[VIRT_GIC_DIST].base);
>>>>        gicd->version = vms->gic_version;
>>>>
>>>> -    for (i = 0; i < MACHINE(vms)->smp.cpus; i++) {
>>>> +    for (i = 0; i < possible_cpus->len; i++) {
>>>>            AcpiMadtGenericCpuInterface *gicc = acpi_data_push(table_data,
>>>>                                                               sizeof(*gicc));
>>>>            ARMCPU *armcpu = ARM_CPU(qemu_get_cpu(i));
>>>>
>>>> +        /*
>>>> +         * PMU should have been either implemented for all CPUs or not,
>>>> +         * so we only get information from the first CPU, which could
>>>> +         * represent the others.
>>>> +         */
>>>> +        if (i == 0) {
>>>> +            pmu = arm_feature(&armcpu->env, ARM_FEATURE_PMU);
>>>> +        }
>>>> +        assert(!armcpu || arm_feature(&armcpu->env, ARM_FEATURE_PMU) ==
>> pmu);
>>>> +
>>>>            gicc->type = ACPI_APIC_GENERIC_CPU_INTERFACE;
>>>>            gicc->length = sizeof(*gicc);
>>>>            if (vms->gic_version == 2) {
>>>> @@ -504,11 +517,19 @@ build_madt(GArray *table_data, BIOSLinker *linker,
>>>> VirtMachineState *vms)
>>>>                gicc->gicv_base_address =
>>>> cpu_to_le64(memmap[VIRT_GIC_VCPU].base);
>>>>            }
>>>>            gicc->cpu_interface_number = cpu_to_le32(i);
>>>> -        gicc->arm_mpidr = cpu_to_le64(armcpu->mp_affinity);
>>>> +        gicc->arm_mpidr = cpu_to_le64(possible_cpus->cpus[i].arch_id);
>>>>            gicc->uid = cpu_to_le32(i);
>>>> -        gicc->flags = cpu_to_le32(ACPI_MADT_GICC_ENABLED);
>>>>
>>>> -        if (arm_feature(&armcpu->env, ARM_FEATURE_PMU)) {
>>>> +        /*
>>>> +         * ACPI spec says that LAPIC entry for non present CPU may be
>>>> +         * omitted from MADT or it must be marked as disabled. Here we
>>>> +         * choose to also keep the disabled ones in MADT.
>>>> +         */
>>>> +        if (possible_cpus->cpus[i].cpu != NULL) {
>>>> +            gicc->flags = cpu_to_le32(ACPI_MADT_GICC_ENABLED);
>>>> +        }
>>>> +
>>>> +        if (pmu) {
>>>>                gicc->performance_interrupt =
>> cpu_to_le32(PPI(VIRTUAL_PMU_IRQ));
>>>>            }
>>>>            if (vms->virt) {
>>>> --
>>>> 2.19.1
>>>>
>>> .

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [RFC PATCH v3 4/9] hw/arm/virt: Initialize the present cpu members
  2021-05-18  7:50         ` Andrew Jones
@ 2021-05-18 18:50           ` Salil Mehta
  0 siblings, 0 replies; 5+ messages in thread
From: Salil Mehta @ 2021-05-18 18:50 UTC (permalink / raw)
  To: Andrew Jones
  Cc: wangyanan (Y),
	Peter Maydell, Michael S . Tsirkin, Igor Mammedov, Shannon Zhao,
	Alistair Francis, David Gibson, qemu-devel, qemu-arm,
	Paolo Bonzini, Philippe Mathieu-Daudé,
	yangyicong, Zengtao (B), Song Bao Hua (Barry Song),
	Wanghaibin (D),
	zhukeqian, yuzenghui, linux-kernel, linuxarm

> 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.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-05-18 18:50 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20210516102900.28036-1-wangyanan55@huawei.com>
     [not found] ` <20210516102900.28036-7-wangyanan55@huawei.com>
     [not found]   ` <6d3c643dc9174f8199ea9422f9e995e5@huawei.com>
     [not found]     ` <14284387-dcf4-cfb2-127c-5d1e1d0ecb79@huawei.com>
2021-05-18  6:47       ` [RFC PATCH v3 6/9] hw/arm/virt-acpi-build: Use possible cpus in generation of MADT Salil Mehta
2021-05-18 11:58         ` wangyanan (Y)
     [not found] ` <20210516102900.28036-5-wangyanan55@huawei.com>
     [not found]   ` <6c8f9c3502384f648f30c7381e87dda9@huawei.com>
     [not found]     ` <68883a1b-5303-da13-a051-e909e1d1f71b@huawei.com>
2021-05-18  7:04       ` [RFC PATCH v3 4/9] hw/arm/virt: Initialize the present cpu members Salil Mehta
2021-05-18  7:50         ` Andrew Jones
2021-05-18 18:50           ` Salil Mehta

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).