From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754887AbbI3JBd (ORCPT ); Wed, 30 Sep 2015 05:01:33 -0400 Received: from szxga01-in.huawei.com ([58.251.152.64]:21495 "EHLO szxga01-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752646AbbI3JBT (ORCPT ); Wed, 30 Sep 2015 05:01:19 -0400 Message-ID: <560BA4C9.7090607@huawei.com> Date: Wed, 30 Sep 2015 17:00:57 +0800 From: Hanjun Guo User-Agent: Mozilla/5.0 (Windows NT 6.1; rv:24.0) Gecko/20100101 Thunderbird/24.0.1 MIME-Version: 1.0 To: Al Stone , , CC: , , , , , Subject: Re: [PATCH v5 0/5] Provide better MADT subtable sanity checks References: <1443570346-15378-1-git-send-email-al.stone@linaro.org> In-Reply-To: <1443570346-15378-1-git-send-email-al.stone@linaro.org> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.177.17.188] X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2015/9/30 7:45, Al Stone wrote: > NB: this patch set is for use against the linux-pm bleeding edge branch. > > Currently, the BAD_MADT_ENTRY macro is used to do a very simple sanity > check on the various subtables that are defined for the MADT. The check > compares the size of the subtable data structure as defined by ACPICA to > the length entry in the subtable. If they are not the same, the assumption > is that the subtable is incorrect. > > Over time, the ACPI spec has allowed for MADT subtables where this can > never be true (the local SAPIC subtable, for example). Or, more recently, > the spec has accumulated some minor flaws where there are three possible > sizes for a subtable, all of which are valid, but only for specific versions > of the spec (the GICC subtable). In both cases, BAD_MADT_ENTRY reports these > subtables as bad when they are not. In order to retain some sanity check > on the MADT subtables, we now have to special case these subtables. Of > necessity, these special cases have ended up in arch-dependent code (arm64) > or an arch has simply decided to forgo the check (ia64). > > This patch set replaces the BAD_MADT_ENTRY macro with a function called > bad_madt_entry(). This function uses a data set of details about the > subtables to provide more sanity checking than before: > > -- is the subtable legal for the version given in the FADT? > > -- is the subtable legal for the revision of the MADT in use? > > -- is the subtable of the proper length (including checking > on the one variable length subtable that is currently ignored), > given the FADT version and the MADT revision? > > Further, this patch set adds in the call to bad_madt_entry() from the > acpi_table_parse_madt() function, allowing it to be used consistently > by all architectures, for all subtables, and removing the need for each > of the subtable traversal callback functions to use BAD_MADT_ENTRY. > > In theory, as the ACPI specification changes, we would only have to add > additional information to the data set describing the MADT subtables in > order to continue providing sanity checks, even when new subtables are > added. > > These patches have been tested on an APM Mustang (arm64) and are known to > work there. They have also been cross-compiled for x86 and ia64 with no > known failures. > > Changes for v5: > -- 0-day found incorrect data in the table describing allowed MADT > subtables; this only affected ACPI 1.0 firmware. Corrected the > data to meet the 1.0b spec. > -- Rebase to bleeding-edge branch for Rafael Wysocki; this patch set > now requires that a patch set from Marc Zyngier be applied first: > https://lkml.org/lkml/2015/9/28/421 > -- Tested on AMD Seattle (linux-pm tree) also > > Changes for v4: > -- Remove extraneous white space change (Graeme Gregory) > -- acpi_parse_entries() changes also needed a check to make sure that > only MADT entries used bad_madt_entry() (Sudeep Holla) > -- inadvertent use of 01day build noted that bad_madt_entry() can be > static, so added it (Sudeep Holla, Fengguang Wu) > > Changes for v3: > -- Reviewed-and-tested-by from Sudeep Holla for arm64 parts > -- Clearer language in error messages (Graeme Gregory, Timur Tabi) > -- Double checked that inserting call to bad_madt_entry() into the > function acpi_parse_entries() does not impact current behavior > (Sudeep Holla) > > Changes for v2: > -- Acked-by on 2/5 from Marc Zyngier and Catalin Marinas for ARM > -- Correct faulty end of loop test found by Timur Tabi > > > Al Stone (5): > ACPI: add in a bad_madt_entry() function to eventually replace the > macro > ACPI / ARM64: remove usage of BAD_MADT_ENTRY/BAD_MADT_GICC_ENTRY > ACPI / IA64: remove usage of BAD_MADT_ENTRY > ACPI / X86: remove usage of BAD_MADT_ENTRY > ACPI: remove definition of BAD_MADT_ENTRY macro For this patch set, Reviewed-by: Hanjun Guo Thanks Hanjun