From patchwork Fri Feb 19 23:39:45 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Al Stone X-Patchwork-Id: 62425 Delivered-To: patches@linaro.org Received: by 10.112.43.199 with SMTP id y7csp78580lbl; Fri, 19 Feb 2016 15:40:48 -0800 (PST) X-Received: by 10.60.138.67 with SMTP id qo3mr13719439oeb.80.1455925248560; Fri, 19 Feb 2016 15:40:48 -0800 (PST) Return-Path: Received: from mail-ob0-x22f.google.com (mail-ob0-x22f.google.com. [2607:f8b0:4003:c01::22f]) by mx.google.com with ESMTPS id rv11si7755677oec.82.2016.02.19.15.40.48 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 19 Feb 2016 15:40:48 -0800 (PST) Received-SPF: pass (google.com: domain of al.stone@linaro.org designates 2607:f8b0:4003:c01::22f as permitted sender) client-ip=2607:f8b0:4003:c01::22f; Authentication-Results: mx.google.com; spf=pass (google.com: domain of al.stone@linaro.org designates 2607:f8b0:4003:c01::22f as permitted sender) smtp.mailfrom=al.stone@linaro.org; dkim=pass header.i=@linaro.org Received: by mail-ob0-x22f.google.com with SMTP id gc3so121027183obb.3 for ; Fri, 19 Feb 2016 15:40:48 -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=CWwzCE++wNMmlhG1JRV6SuRPRcSLf6rZKCtZ7DMV134=; b=ca/opLtKPOaYWp37+U2NfKpeFUmLr7OZdCOgiQf/MYuaAyzwu2MUtxhp8uvPnboYWF TzNX2NTN+1OIYL8anMYCkpUBHzVj9bxq+DGU4E8TfPgNMI6mmqE21uU+uNiosPSVKuU3 OV8oqaERdtDZEIOOLJd7jF2Amr1wKkNOc7AcM= 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=CWwzCE++wNMmlhG1JRV6SuRPRcSLf6rZKCtZ7DMV134=; b=ExZUvzMV+9LWrIMIutn8Esn2dpf1dQGqWdHiZDm9wpRWHoGsAyy33o06jra93AoEdT KAjInTBdHGkhcczwjeMtED5Ci6lgJHeb2Hzzeh6ZJJU8x7X0Jja71GEy7aCbJsTmP0eI 2UmFXwjaYvR4zGIRmZb6sxyk+/Ftqj0BDlLgXmGtnZN2m4oDpq0brp2CVxIG7WFkjOO4 9qllzIoL2kpBLP0LqOi8tx3KkwYHJoHeXUOCK2Kmdp0eLTYVXgN51yGEZM7hrLhNjiNn PjWOMf1VQZ49k43YY5/9unkS8Uxx49LcmwMDuixX5iYv9JLK18niFmCUvrVZbp2i+2Es WHQg== X-Gm-Message-State: AG10YOS21F/yYhfrOKrh79ZO00OHRgD8nybDlYORC/dptSjQSKkkwi6ReVeBzOZ65JGqvymLKGo= X-Received: by 10.60.76.10 with SMTP id g10mr13953914oew.50.1455925248256; Fri, 19 Feb 2016 15:40:48 -0800 (PST) Return-Path: 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.46 (version=TLSv1/SSLv3 cipher=OTHER); Fri, 19 Feb 2016 15:40:47 -0800 (PST) From: Al Stone To: fwts-devel@lists.ubuntu.com Cc: linaro-acpi@lists.linaro.org, patches@linaro.org, Al Stone Subject: [PATCH v2 09/23] FADT: expand the compliance test for FIRMWARE_CTRL fields Date: Fri, 19 Feb 2016 16:39:45 -0700 Message-Id: <1455925199-8587-10-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> Expand the testing of the FIRMWARE_CTRL -- and by extension, the X_FIRMWARE_CTRL 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: Colin Ian King Acked-by: Alex Hung --- src/acpi/fadt/fadt.c | 146 +++++++++++++++++++++++++++++++++------------------ 1 file changed, 95 insertions(+), 51 deletions(-) -- 2.5.0 diff --git a/src/acpi/fadt/fadt.c b/src/acpi/fadt/fadt.c index afe52f3..b9e50a4 100644 --- a/src/acpi/fadt/fadt.c +++ b/src/acpi/fadt/fadt.c @@ -203,61 +203,105 @@ static int fadt_revision(fwts_framework *fw) return FWTS_OK; } - -static void acpi_table_check_fadt_firmware_control( - fwts_framework *fw, - const fwts_acpi_table_fadt *fadt, - bool *passed) +static void acpi_table_check_fadt_firmware_ctrl(fwts_framework *fw) { - if (fadt->firmware_control == 0) { - if (fadt->header.length >= 140) { - if ((fadt->x_firmware_ctrl == 0) && - !(fwts_acpi_is_reduced_hardware(fadt))) { - *passed = false; - fwts_failed(fw, LOG_LEVEL_CRITICAL, - "FADTFACSZero", - "FADT 32 bit FIRMWARE_CONTROL and " - "64 bit X_FIRMWARE_CONTROL (FACS " - "address) are null."); - fwts_advice(fw, - "The 32 bit FIRMWARE_CTRL or 64 " - "bit X_FIRMWARE_CTRL should point " - "to a valid Firmware ACPI Control " - "Structure (FACS) when ACPI hardware " - "reduced mode is not set. "); - } + /* tests for very old FADTs */ + if (fadt->header.length < 140 && fadt->firmware_control == 0) { + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "FADT32BitFACSNull", + "FADT 32 bit FIRMWARE_CONTROL is null."); + fwts_advice(fw, + "The ACPI version 1.0 FADT has a NULL " + "FIRMWARE_CTRL and it needs to be defined " + "to point to a valid Firmware ACPI Control " + "Structure (FACS)."); + + /* can't do much else */ + return; + } + + /* for more recent FADTs, things get more complicated */ + if (fadt->firmware_control == 0 && fadt->x_firmware_ctrl == 0) { + if (fwts_acpi_is_reduced_hardware(fadt)) { + fwts_passed(fw, + "FADT 32 bit FIRMWARE_CONTROL and " + "64 bit X_FIRMWARE_CONTROL (FACS " + "address) are null in hardware reduced " + "mode."); + fwts_advice(fw, + "When in hardware reduced mode, it is " + "allowed to have both the 32-bit " + "FIRMWARE_CTRL and 64-bit X_FIRMWARE_CTRL " + "fields set to zero as they are. This " + "indicates that no FACS is available."); } else { - *passed = false; - fwts_failed(fw, LOG_LEVEL_MEDIUM, - "FADT32BitFACSNull", - "FADT 32 bit FIRMWARE_CONTROL is null."); + fwts_failed(fw, LOG_LEVEL_CRITICAL, + "FADTFACSZero", + "FADT 32 bit FIRMWARE_CONTROL and " + "64 bit X_FIRMWARE_CONTROL (FACS " + "address) are both null."); fwts_advice(fw, - "The ACPI version 1.0 FADT has a NULL " - "FIRMWARE_CTRL and it needs to be defined " - "to point to a valid Firmware ACPI Control " - "Structure (FACS)."); + "The 32 bit FIRMWARE_CTRL or 64 " + "bit X_FIRMWARE_CTRL should point " + "to a valid Firmware ACPI Control " + "Structure (FACS) when ACPI hardware " + "reduced mode is not set. "); } + + } + + if ((fadt->firmware_control != 0 && fadt->x_firmware_ctrl == 0) || + (fadt->firmware_control == 0 && fadt->x_firmware_ctrl != 0)) { + fwts_passed(fw, + "Only one of FIRMWARE_CTRL and X_FIRMWARE_CTRL " + "is non-zero."); } else { - if (fadt->header.length >= 140) { - if (fadt->x_firmware_ctrl != 0) { - if (((uint64_t)fadt->firmware_control != fadt->x_firmware_ctrl)) { - *passed = false; - fwts_failed(fw, LOG_LEVEL_MEDIUM, - "FwCtrl32and64Differ", - "FIRMWARE_CONTROL is 0x%" PRIx32 - " and differs from X_FIRMWARE_CONTROL " - "0x%" PRIx64, - fadt->firmware_control, - fadt->x_firmware_ctrl); - fwts_advice(fw, - "One would expect the 32 bit FIRMWARE_CTRL " - "and 64 bit X_FIRMWARE_CTRL pointers to " - "point to the same FACS, however they don't " - "which is clearly ambiguous and wrong. " - "The kernel works around this by using the " - "64 bit X_FIRMWARE_CTRL pointer to the FACS. "); - } - } + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "FADTFACSBothSet", + "Both 32-bit FIRMWARE_CTRL and 64-bit " + "X_FIRMWARE_CTRL pointers to the FACS are " + "set but only one is allowed."); + fwts_advice(fw, + "Having both FACS pointers set is ambiguous; " + "there is no way to determine which one is " + "the correct address. The kernel workaround " + "is to always use the 64-bit X_FIRMWARE_CTRL."); + } + + + if (fadt->firmware_control != 0 && fadt->x_firmware_ctrl != 0) { + if ((uint64_t)fadt->firmware_control == fadt->x_firmware_ctrl) { + fwts_passed(fw, + "Both FIRMWARE_CTRL and X_FIRMWARE_CTRL " + "are being used and contain the same FACS " + "address."); + fwts_advice(fw, + "While having FIRMWARE_CTRL and " + "X_FIRMWARE_CTRL both set to an address " + "is not compliant with the ACPI " + "specification, they are both set to " + "the same address, which at least " + "mitigates the ambiguity in determining " + "which address is the correct one to use " + "for the FACS. Ideally, only one of the " + "two addresses should be set but as a " + "practical matter, this will work."); + } else { + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "FwCtrl32and64Differ", + "FIRMWARE_CONTROL is 0x%" PRIx32 + " and differs from X_FIRMWARE_CONTROL " + "0x%" PRIx64, + fadt->firmware_control, + fadt->x_firmware_ctrl); + fwts_advice(fw, + "One would expect the 32 bit FIRMWARE_CTRL " + "and 64 bit X_FIRMWARE_CTRL pointers to " + "point to the same FACS, however they do " + "not which is clearly ambiguous and wrong. " + "The kernel works around this by using the " + "64-bit X_FIRMWARE_CTRL pointer to the " + "FACS. "); } } } @@ -497,7 +541,7 @@ static int fadt_test1(fwts_framework *fw) { bool passed = true; - acpi_table_check_fadt_firmware_control(fw, fadt, &passed); + acpi_table_check_fadt_firmware_ctrl(fw); acpi_table_check_fadt_dsdt(fw, fadt, &passed); acpi_table_check_fadt_smi(fw, fadt, &passed); acpi_table_check_fadt_pm_tmr(fw, fadt, &passed);