linux-kernel - Re: [RFC PATCH v3 6/9] hw/arm/virt-acpi-build: Use possible cpus in generation of MADT

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <d99a32e6-99f7-6977-5f74-0d9f9a06b5ea@huawei.com>
Date:   Tue, 18 May 2021 19:58:21 +0800
From:   "wangyanan (Y)" <wangyanan55@...wei.com>
To:     Salil Mehta <salil.mehta@...wei.com>
CC:     Peter Maydell <peter.maydell@...aro.org>,
        Andrew Jones <drjones@...hat.com>,
        "Michael S . Tsirkin" <mst@...hat.com>,
        Igor Mammedov <imammedo@...hat.com>,
        Shannon Zhao <shannon.zhaosl@...il.com>,
        "Alistair Francis" <alistair.francis@....com>,
        David Gibson <david@...son.dropbear.id.au>,
        "qemu-devel@...gnu.org" <qemu-devel@...gnu.org>,
        "qemu-arm@...gnu.org" <qemu-arm@...gnu.org>,
        "Paolo Bonzini" <pbonzini@...hat.com>,
        Philippe Mathieu-Daudé <philmd@...hat.com>,
        yangyicong <yangyicong@...wei.com>,
        "Zengtao (B)" <prime.zeng@...ilicon.com>,
        "Song Bao Hua (Barry Song)" <song.bao.hua@...ilicon.com>,
        "Wanghaibin (D)" <wanghaibin.wang@...wei.com>,
        zhukeqian <zhukeqian1@...wei.com>,
        yuzenghui <yuzenghui@...wei.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linuxarm@...neuler.org" <linuxarm@...neuler.org>
Subject: Re: [RFC PATCH v3 6/9] hw/arm/virt-acpi-build: Use possible cpus in
 generation of MADT


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@...gnu.org]
>>>> On Behalf Of Yanan Wang
>>>> Sent: Sunday, May 16, 2021 11:29 AM
>>>> To: Peter Maydell <peter.maydell@...aro.org>; Andrew Jones
>>>> <drjones@...hat.com>; Michael S . Tsirkin <mst@...hat.com>; Igor Mammedov
>>>> <imammedo@...hat.com>; Shannon Zhao <shannon.zhaosl@...il.com>; Alistair
>>>> Francis <alistair.francis@....com>; David Gibson
>>>> <david@...son.dropbear.id.au>; qemu-devel@...gnu.org; qemu-arm@...gnu.org
>>>> Cc: Song Bao Hua (Barry Song) <song.bao.hua@...ilicon.com>; zhukeqian
>>>> <zhukeqian1@...wei.com>; yangyicong <yangyicong@...wei.com>; Zengtao (B)
>>>> <prime.zeng@...ilicon.com>; Wanghaibin (D) <wanghaibin.wang@...wei.com>;
>>>> yuzenghui <yuzenghui@...wei.com>; Paolo Bonzini <pbonzini@...hat.com>;
>>>> Philippe Mathieu-Daudé <philmd@...hat.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@...hat.com>
>>>> Signed-off-by: Andrew Jones <drjones@...hat.com>
>>>> Co-developed-by: Ying Fang <fangying1@...wei.com>
>>>> Signed-off-by: Ying Fang <fangying1@...wei.com>
>>>> Co-developed-by: Yanan Wang <wangyanan55@...wei.com>
>>>> Signed-off-by: Yanan Wang <wangyanan55@...wei.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
>>>>
>>> .