From patchwork Fri Feb 19 23:39:46 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Al Stone X-Patchwork-Id: 585485 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) by ozlabs.org (Postfix) with ESMTP id 875731402D8; Sat, 20 Feb 2016 10:41:04 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=linaro.org header.i=@linaro.org header.b=cLMazpnQ; dkim-atps=neutral Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.76) (envelope-from ) id 1aWufX-0005LG-DD; Fri, 19 Feb 2016 23:41:03 +0000 Received: from mail-ob0-f170.google.com ([209.85.214.170]) by huckleberry.canonical.com with esmtps (TLS1.0:RSA_ARCFOUR_SHA1:16) (Exim 4.76) (envelope-from ) id 1aWufM-0005Fh-Dt for fwts-devel@lists.ubuntu.com; Fri, 19 Feb 2016 23:40:52 +0000 Received: by mail-ob0-f170.google.com with SMTP id xk3so124285457obc.2 for ; Fri, 19 Feb 2016 15:40:52 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=HFgpIeAjJmWIo/0LzATjNC37vg8NvgNwwFwwZdNBWLI=; b=cLMazpnQWhHiksC9u9WlxJCG9Jh9FIWS0+AcZCGmfR1jMa1Bm/yCZ73i4kLtGa8ZrV 4ev2HV5nPAj3Ms7VvHdBX9F7g7OE7NJXx/Ctu/LvyQO7RHFqg5oKEgKFQ4owqmPXhf3m GfWgkcN6xqki2DGVySOFUTxCa5Q3QBqqsEvYc= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=HFgpIeAjJmWIo/0LzATjNC37vg8NvgNwwFwwZdNBWLI=; b=WFedoHJx8q0tptf45BUf9FV7+xuA/UyaWgvnpb679T8yKGXbkwiDA8BffuDKonTdEE AG38/Z9DMWPcNYn5z+HJrNz33wSX3kTCQFLMNlQhKcmfiB1T8qaNW7L6YwwjKJEIP3zO DqMB1Qgtj2j/u+EX8vTmUCWaMiea6EPlkDf0tQtFLyOqRbFVg4GBQYXNGcjgH1bZkq3W fjwX6xXiGW5NtNJEBJrIBQl7kJo+7lBu1fYmx0aQ2NYHvcCcdgUFS9EiyHl25dnh+hKO ioGsoeMWoIRbWKQreaNEhppuULpNWWdnNSBpNXZ24hUwKOAKoBi7ZbvVGhgCJ1YiUJ7V Ps/Q== X-Gm-Message-State: AG10YOQKiEWVXvFjfI9RXXzPUYmo+dXhf8gpG0zy5xVv37gXVYRD+TOYkUsW3J3Cjmp6OMCQ X-Received: by 10.60.60.65 with SMTP id f1mr10628003oer.28.1455925251369; Fri, 19 Feb 2016 15:40:51 -0800 (PST) Received: from fidelio.ahs3.com (c-50-134-239-249.hsd1.co.comcast.net. [50.134.239.249]) by smtp.googlemail.com with ESMTPSA id kg7sm8655217obb.27.2016.02.19.15.40.49 (version=TLSv1/SSLv3 cipher=OTHER); Fri, 19 Feb 2016 15:40:49 -0800 (PST) From: Al Stone To: fwts-devel@lists.ubuntu.com Subject: [PATCH v2 10/23] FADT: expand compliance checks for DSDT and X_DSDT fields Date: Fri, 19 Feb 2016 16:39:46 -0700 Message-Id: <1455925199-8587-11-git-send-email-al.stone@linaro.org> X-Mailer: git-send-email 2.5.0 In-Reply-To: <1455925199-8587-1-git-send-email-al.stone@linaro.org> References: <1455925199-8587-1-git-send-email-al.stone@linaro.org> Cc: patches@linaro.org, linaro-acpi@lists.linaro.org X-BeenThere: fwts-devel@lists.ubuntu.com X-Mailman-Version: 2.1.14 Precedence: list List-Id: Firmware Test Suite Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: fwts-devel-bounces@lists.ubuntu.com Sender: fwts-devel-bounces@lists.ubuntu.com Expand the testing of the DSDT address -- and by extension, the X_DSDT address field -- to check for full compliance with the spec. Only one or the other may be used at any one time, per 6.1, but we also have to acknowledge there are tables that do use both the 32- and 64-bit values. At that point, we re-use parts of the existing test to verify that they are at least consistent. Signed-off-by: Al Stone Acked-by: Ivan Hu Acked-by: Alex Hung --- src/acpi/fadt/fadt.c | 112 +++++++++++++++++++++++++++++++-------------------- 1 file changed, 69 insertions(+), 43 deletions(-) diff --git a/src/acpi/fadt/fadt.c b/src/acpi/fadt/fadt.c index b9e50a4..4a39c01 100644 --- a/src/acpi/fadt/fadt.c +++ b/src/acpi/fadt/fadt.c @@ -306,54 +306,80 @@ static void acpi_table_check_fadt_firmware_ctrl(fwts_framework *fw) } } -static void acpi_table_check_fadt_dsdt( - fwts_framework *fw, - const fwts_acpi_table_fadt *fadt, - bool *passed) +static void acpi_table_check_fadt_dsdt(fwts_framework *fw) { - + /* check out older FADTs */ if (fadt->header.length < 148) { - if (fadt->dsdt == 0) { - *passed = false; + if (fadt->dsdt == 0) fwts_failed(fw, LOG_LEVEL_MEDIUM, "FADTDSDTNull", "FADT DSDT address is null."); - } - } else { - if (fadt->x_dsdt == 0) { - if (fadt->dsdt == 0) { - *passed = false; - fwts_failed(fw, LOG_LEVEL_MEDIUM, - "FADTXDSDTNull", - "FADT X_DSDT and DSDT address are null."); - } else { - *passed = false; - fwts_failed(fw, LOG_LEVEL_MEDIUM, - "FADTXDSDTNull", - "FADT X_DSDT address is null."); - fwts_advice(fw, - "An ACPI 2.0 FADT is being used however " - "the 64 bit X_DSDT is null." - "The kernel will fall back to using " - "the 32 bit DSDT pointer instead."); - } - } else if ((uint64_t)fadt->dsdt != fadt->x_dsdt && fadt->dsdt != 0) { - *passed = false; + } + + /* if one field is being used, the other should be null */ + if ((fadt->dsdt != 0 && fadt->x_dsdt == 0) || + (fadt->dsdt == 0 && fadt->x_dsdt != 0)) + fwts_passed(fw, + "FADT has only one of X_DSDT or DSDT addresses " + "being used."); + else { + if (fadt->dsdt == 0 && fadt->x_dsdt == 0) fwts_failed(fw, LOG_LEVEL_MEDIUM, - "FADT32And64Mismatch", - "FADT 32 bit DSDT (0x%" PRIx32 ") " - "does not point to same " - "physical address as 64 bit X_DSDT " - "(0x%" PRIx64 ").", - fadt->dsdt, fadt->x_dsdt); - fwts_advice(fw, - "One would expect the 32 bit DSDT and " - "64 bit X_DSDT pointers to point to the " - "same DSDT, however they don't which is " - "clearly ambiguous and wrong. " - "The kernel works around this by using the " - "64 bit X_DSDT pointer to the DSDT. "); - } + "FADTOneDSDTNull", + "FADT X_DSDT and DSDT addresses cannot " + "both be null."); + } + + /* unexpected use of addresses */ + if (fadt->dsdt != 0 && fadt->x_dsdt == 0) { + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "FADTXDSDTNull", + "FADT X_DSDT address is null."); + fwts_advice(fw, + "An ACPI 2.0 or newer FADT is being used but " + "the 64 bit X_DSDT is null. " + "The kernel will fall back to using " + "the 32 bit DSDT pointer instead."); + } + + /* + * If you are going to insist on using both fields, even though + * that is incorrect, at least make it unambiguous as to which + * address is the one to use, by using the same value in both + * fields. + */ + if ((uint64_t)fadt->dsdt == fadt->x_dsdt && fadt->dsdt != 0) { + fwts_passed(fw, + "FADT 32 bit DSDT and 64 bit X_DSDT " + "both point to the same physical address " + "(0x%" PRIx64 ").", fadt->x_dsdt); + fwts_advice(fw, + "While it is not correct to use both of the " + "32- and 64-bit DSDT address fields in recent " + "versions of ACPI, they are at least the same " + "address, which keeps the kernel from getting " + "confused. At some point, the 32-bit DSDT " + "address may get ignored so it is recommended " + "that the FADT be upgraded to only use the 64-" + "bit X_DSDT field. In the meantime, however, " + "ACPI will still behave correctly."); + } + if ((uint64_t)fadt->dsdt != fadt->x_dsdt && fadt->dsdt != 0) { + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "FADT32And64Mismatch", + "FADT 32 bit DSDT (0x%" PRIx32 ") " + "does not point to same " + "physical address as 64 bit X_DSDT " + "(0x%" PRIx64 ").", + fadt->dsdt, fadt->x_dsdt); + fwts_advice(fw, + "One would expect the 32 bit DSDT and " + "64 bit X_DSDT pointers to point to the " + "same DSDT, however they don't which is " + "clearly ambiguous and wrong. " + "The kernel works around this by assuming the " + "64 bit X_DSDT pointer to the DSDT is the correct " + "one to use."); } } @@ -542,7 +568,7 @@ static int fadt_test1(fwts_framework *fw) bool passed = true; acpi_table_check_fadt_firmware_ctrl(fw); - acpi_table_check_fadt_dsdt(fw, fadt, &passed); + acpi_table_check_fadt_dsdt(fw); acpi_table_check_fadt_smi(fw, fadt, &passed); acpi_table_check_fadt_pm_tmr(fw, fadt, &passed); acpi_table_check_fadt_gpe(fw, fadt, &passed);