Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753179AbcJGEgT (ORCPT ); Fri, 7 Oct 2016 00:36:19 -0400 Received: from cn.fujitsu.com ([59.151.112.132]:64847 "EHLO heian.cn.fujitsu.com" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1750974AbcJGEgL (ORCPT ); Fri, 7 Oct 2016 00:36:11 -0400 X-IronPort-AV: E=Sophos;i="5.22,518,1449504000"; d="scan'208";a="11698149" Subject: Re: [tip:x86/apic] x86/acpi: Introduce persistent storage for cpuid <-> apicid mapping To: Yinghai Lu References: <1472114120-3281-4-git-send-email-douly.fnst@cn.fujitsu.com> <4608f474-c49e-550b-90e2-c5f4c25e00f5@cn.fujitsu.com> CC: Thomas Gleixner , Linux Kernel Mailing List , Ingo Molnar , "H. Peter Anvin" , "linux-tip-commits@vger.kernel.org" , Tony Luck , "Rafael J. Wysocki" , "lenb@kernel.org" , "Zheng, Lv" , , From: Dou Liyang Message-ID: Date: Fri, 7 Oct 2016 12:35:36 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.167.226.106] X-yoursite-MailScanner-ID: 4F71A466F721.AC1DE X-yoursite-MailScanner: Found to be clean X-yoursite-MailScanner-From: douly.fnst@cn.fujitsu.com Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2463 Lines: 83 Hi Yinghai At 10/07/2016 05:20 AM, Yinghai Lu wrote: > On Thu, Oct 6, 2016 at 1:06 AM, Dou Liyang 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 > >