linux-kernel - Re: [tip:x86/apic] x86/acpi: Introduce persistent storage for cpuid <-> apicid mapping

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] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 7 Oct 2016 12:35:36 +0800
From:   Dou Liyang <douly.fnst@...fujitsu.com>
To:     Yinghai Lu <yinghai@...nel.org>
CC:     Thomas Gleixner <tglx@...utronix.de>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Ingo Molnar <mingo@...nel.org>,
        "H. Peter Anvin" <hpa@...or.com>,
        "linux-tip-commits@...r.kernel.org" 
        <linux-tip-commits@...r.kernel.org>,
        Tony Luck <tony.luck@...el.com>,
        "Rafael J. Wysocki" <rjw@...ysocki.net>,
        "lenb@...nel.org" <lenb@...nel.org>,
        "Zheng, Lv" <lv.zheng@...el.com>, <robert.moore@...el.com>,
        <linux-acpi@...r.kernel.org>
Subject: Re: [tip:x86/apic] x86/acpi: Introduce persistent storage for cpuid
 <-> apicid mapping

Hi Yinghai

At 10/07/2016 05:20 AM, Yinghai Lu wrote:
> On Thu, Oct 6, 2016 at 1:06 AM, Dou Liyang <douly.fnst@...fujitsu.com> wrote:
>
>> I seem to remember that in x2APIC Spec the x2APIC ID may be at 255 or
>> greater.
>
> Good to know. Maybe later when one package have more cores like 30 cores etc.
>
>> If we do that judgment, it may be affect x2APIC's work in some other places.
>>
>> I saw the MADT, the main reason may be that we define 0xff to acpi_id
>> in LAPIC mode.
>> As you said, it was like:
>> [   42.107902] ACPI: LAPIC (acpi_id[0xff] lapic_id[0xff] disabled)
>> [   42.120125] ACPI: LAPIC (acpi_id[0xff] lapic_id[0xff] disabled)
>> [   42.132361] ACPI: LAPIC (acpi_id[0xff] lapic_id[0xff] disabled)
>> ...
>>
>> How about doing the acpi_id check when we parse it in
>> acpi_parse_lapic().
>>
>> 8<----------------
>>
>> --- a/arch/x86/kernel/acpi/boot.c
>> +++ b/arch/x86/kernel/acpi/boot.c
>> @@ -233,6 +233,11 @@ acpi_parse_lapic(struct acpi_subtable_header * header,
>> const unsigned long end)
>>
>>         acpi_table_print_madt_entry(header);
>>
>> +       if (processor->id >= 255) {
>> +               ++disabled_cpus;
>> +               return -EINVAL;
>> +       }
>> +
>>         /*
>>          * We need to register disabled CPU as well to permit
>>          * counting disabled CPUs. This allows us to size
>
> Yes, that should work. but should do the same thing for x2apic
>
> in acpi_parse_x2apic should have
>
>> +       if (processor->local_apic_id == -1) {
>> +               ++disabled_cpus;
>> +               return -EINVAL;
>> +       }
>
> that is the reason why i want to extend acpi_register_lapic()
> to take extra disabled_id (one is 0xff and another is 0xffffffff)
> so could save some lines.
>

Yes, I understood.
But I think adding an extra disabled_id is not a good way for
validating the apic_id. If the disabled_id is not just one id(-1 or
255), may be two or more, even be a range. what should we do for
extending our code?

Firstly, I am not sure that the "-1" could appear in the MADT, even if
the ACPI tables is unreasonable.

Seondly, I guess if we need the check, there are some reserved methods
in the kernel, such as "default_apic_id_valid", "x2apic_apic_id_valid"
and so on. we should extend all of them and use them for check.


CC'ed: Rafael and Lv

May I ask a question?

Is it possible that the "-1/oxffffffff" could appear in the MADT which 
is one of the ACPI tables?


> Thanks
>
> Yinghai
>
>