From patchwork Fri Feb 19 23:39:55 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Al Stone X-Patchwork-Id: 62435 Delivered-To: patches@linaro.org Received: by 10.112.43.199 with SMTP id y7csp78734lbl; Fri, 19 Feb 2016 15:41:18 -0800 (PST) X-Received: by 10.60.47.195 with SMTP id f3mr14333069oen.1.1455925278527; Fri, 19 Feb 2016 15:41:18 -0800 (PST) Return-Path: Received: from mail-ob0-x22e.google.com (mail-ob0-x22e.google.com. [2607:f8b0:4003:c01::22e]) by mx.google.com with ESMTPS id bg4si6869865oec.74.2016.02.19.15.41.18 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 19 Feb 2016 15:41:18 -0800 (PST) Received-SPF: pass (google.com: domain of al.stone@linaro.org designates 2607:f8b0:4003:c01::22e as permitted sender) client-ip=2607:f8b0:4003:c01::22e; Authentication-Results: mx.google.com; spf=pass (google.com: domain of al.stone@linaro.org designates 2607:f8b0:4003:c01::22e as permitted sender) smtp.mailfrom=al.stone@linaro.org; dkim=pass header.i=@linaro.org Received: by mail-ob0-x22e.google.com with SMTP id xk3so124292346obc.2 for ; Fri, 19 Feb 2016 15:41:18 -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=9hVU1KFt3GolNf4gQnVvvq2bS7hR/WD77d610QCtQOI=; b=Qp14iYoOausQlXjqvSigiGZASFcwnc5Ot7JDWJ1we1XnmDyjEXpOffsV0kZd1ThZXW JgQcXwlM2KsKLeuMbLaGmPRXIjFWMg8/3Xcv/swlLTd6UPvK9uGQx3bKNgnD452i+qM0 gQrl4Kz/hu3dBMsrPnFpAkBVVQASLJTpCixBk= 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=9hVU1KFt3GolNf4gQnVvvq2bS7hR/WD77d610QCtQOI=; b=YgPMPeuOItz5h46GM2j9yRX6sPUvlhPQQcHyHZ2boD58PA7316Ec1mPtkqRVjXLHXm 81si+PzdP9nsEjUipeaIKIqDy/Okw50tYBRzFzM8zWBM1IShVTZD6C5O3TRs6XgDvNGr 0ki8kGJCLsXLyeBdgvGG579HveknyAEU7A7tIETqTeXj5WooyclsLNtcaHC2+EfMYFrF oACr3kKbT2ItY1bmN+BWZw05HgXeBpVaiKjIuGCKpD6l6lNuxEiy55IBef9WQ4nHV9vk 85VTaPmUlPhHqqhKEByD1Gvrq+o1Rf4MHgerAf5QIU7Mwl8JX3480c5eGjplNApH+UKk eoYg== X-Gm-Message-State: AG10YOTXpPD//RiAanvFxMbMfw6anEeedVUes+JrlyPU9NqyFbAI6N4GTf+Tt1xbFg+9rXILoqY= X-Received: by 10.60.147.137 with SMTP id tk9mr14292647oeb.45.1455925278162; Fri, 19 Feb 2016 15:41:18 -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.41.16 (version=TLSv1/SSLv3 cipher=OTHER); Fri, 19 Feb 2016 15:41:16 -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 19/23] FADT: add in compliance tests for C2/C3 latency fields Date: Fri, 19 Feb 2016 16:39:55 -0700 Message-Id: <1455925199-8587-20-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> Add in new compliance checks for the P_LVL2_LAT and P_LVL3_LAT fields. Both of these require knowing the value of an x86 P_BLK; this in turn requires examination of any Processor() declarations in the ACPI name space, which in turn requires the initialization of the ACPI interpreter. It is probable that these fields are seldom used; processor speeds and feeds are typically controlled via very different mechanisms these days. These tests are therefore for completeness sake and it may be difficult to find ACPI tables using these fields. Note that these tests allow us to remove commented out versions of simpler tests of these fields. Signed-off-by: Al Stone Acked-by: Colin Ian King Acked-by: Alex Hung --- src/acpi/fadt/fadt.c | 172 +++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 147 insertions(+), 25 deletions(-) -- 2.5.0 diff --git a/src/acpi/fadt/fadt.c b/src/acpi/fadt/fadt.c index 6954e49..34978e5 100644 --- a/src/acpi/fadt/fadt.c +++ b/src/acpi/fadt/fadt.c @@ -19,6 +19,7 @@ * */ #include "fwts.h" +#include "fwts_acpi_object_eval.h" #include #include @@ -1342,6 +1343,138 @@ static void acpi_table_check_fadt_cst_cnt(fwts_framework *fw) return; } +static uint64_t fadt_find_p_blk(fwts_framework *fw) +{ + uint64_t pblk; + fwts_list *objects; + + pblk = 0; + objects = fwts_acpi_object_get_names(); + if (objects) { + fwts_list_link *obj; + + fwts_list_foreach(obj, objects) { + char *name = fwts_list_data(char*, obj); + ACPI_OBJECT pr = { 0 }; + ACPI_BUFFER buf = { sizeof(ACPI_OBJECT), &pr }; + ACPI_HANDLE handle; + ACPI_OBJECT_TYPE type; + ACPI_STATUS status; + + status = AcpiGetHandle(NULL, name, &handle); + if (ACPI_FAILURE(status)) { + fwts_warning(fw, "Failed to get handle for " + "object %s.", name); + continue; + } + status = AcpiGetType(handle, &type); + if (ACPI_FAILURE(status)) { + fwts_warning(fw, "Failed to get type for " + "object %s.", name); + continue; + } + + /* + * If a CPU is not defined as a Processor object, + * we don't care here. Per section 8.1.2 and 8.1.3, + * defining a CPU with a Device object implies that + * a _CST method must be defined, and whatever is + * in the _CST overrides the P_BLK and P_LVL*_LAT + * values. Since we're only trying to validate the + * P_LVL*_LAT values, and they're only used if the + * CPUs are defined as Processor objects, we can + * ignore any CPUs defined as Device objects. + */ + if (type == ACPI_TYPE_PROCESSOR) { + status = AcpiEvaluateObject(handle, NULL, NULL, + &buf); + if (ACPI_FAILURE(status)) + fwts_warning(fw, "Could not evaluate " + "Processor %s", name); + else { + if (pr.Processor.PblkAddress) + pblk = pr.Processor.PblkAddress; + } + } + } + } + + fwts_log_info(fw, "Using P_BLK address of 0x%" PRIx64, pblk); + return pblk; +} + +static void acpi_table_check_fadt_p_lvl2_lat(fwts_framework *fw, uint64_t pblk) +{ + if (pblk) { + if (fadt->p_lvl2_lat <= 100) + fwts_passed(fw, + "FADT P_LVL2_LAT is within proper range " + "at %" PRIu16, fadt->p_lvl2_lat); + else + fwts_warning(fw, + "FADT P_LVL2_LAT is > 100 (%" PRIu16 ") " + "but a P_BLK is defined. This implies " + "a C2 state is not supported, but there " + "is a P_BLK register block defined which " + "implies there might be a C2 state that " + "works. There is not enough information " + "to determine if this is expected or not.", + fadt->p_lvl2_lat); + } else { + if (fadt->p_lvl2_lat <= 100) + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "PLvl2LatDefinedButNotUsable", + "FADT P_LVL2_LAT is <= 100 (%" PRIu16 ") " + "which implies a C2 state is supported " + "but there is no P_BLK register block " + "defined to enable the C2 transition.", + fadt->p_lvl2_lat); + else + fwts_passed(fw, + "FADT P_LVL2_LAT is > 100 (%" PRIu16 ") " + "and no P_BLK is defined.", + fadt->p_lvl2_lat); + } + + return; +} + +static void acpi_table_check_fadt_p_lvl3_lat(fwts_framework *fw, uint64_t pblk) +{ + if (pblk) { + if (fadt->p_lvl3_lat <= 1000) + fwts_passed(fw, + "FADT P_LVL3_LAT is within proper range " + "at %" PRIu16, fadt->p_lvl3_lat); + else + fwts_warning(fw, + "FADT P_LVL3_LAT is > 1000 (%" PRIu16 ") " + "but a P_BLK is defined. This implies " + "a C3 state is not supported, but there " + "is a P_BLK register block defined which " + "implies there might be a C3 state that " + "works. There is not enough information " + "to determine if this is expected or not.", + fadt->p_lvl3_lat); + } else { + if (fadt->p_lvl3_lat <= 1000) + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "PLvl3LatDefinedButNotUsable", + "FADT P_LVL3_LAT is <= 1000 (%" PRIu16 ") " + "which implies a C3 state is supported " + "but there is no P_BLK register block " + "defined to enable the C3 transition.", + fadt->p_lvl3_lat); + else + fwts_passed(fw, + "FADT P_LVL3_LAT is > 100 (%" PRIu16 ") " + "and no P_BLK is defined.", + fadt->p_lvl3_lat); + } + + return; +} + static int fadt_test1(fwts_framework *fw) { bool passed = true; @@ -1381,6 +1514,20 @@ static int fadt_test1(fwts_framework *fw) fwts_log_info(fw, "FADT GPE1_BASE is %" PRIu8, fadt->gpe1_base); acpi_table_check_fadt_cst_cnt(fw); + if (fwts_acpi_init(fw) == FWTS_OK) { + uint64_t pblk = fadt_find_p_blk(fw); + + acpi_table_check_fadt_p_lvl2_lat(fw, pblk); + acpi_table_check_fadt_p_lvl3_lat(fw, pblk); + fwts_acpi_deinit(fw); + } else { + fwts_log_error(fw, "Cannot initialize ACPI namespace."); + fwts_log_info(fw, + "Without a namespace, cannot test " + "values of P_LVL2_LAT or P_LVL3_LAT " + "for correctness."); + } + fwts_log_info(fw, "FADT FLUSH_SIZE is %" PRIu16, fadt->flush_size); fwts_log_info(fw, "FADT FLUSH_STRIDE is %" PRIu16, @@ -1395,31 +1542,6 @@ static int fadt_test1(fwts_framework *fw) } /* - * Bug LP: #833644 - * - * Remove these tests, really need to put more intelligence into it - * perhaps in the cstates test rather than here. For the moment we - * shall remove this warning as it's giving users false alarms - * See: https://bugs.launchpad.net/ubuntu/+source/fwts/+bug/833644 - */ - /* - if (fadt->p_lvl2_lat > 100) { - fwts_warning(fw, "FADT P_LVL2_LAT is %" PRIi16 ", a value > 100 indicates a " - "system not to support a C2 state.", fadt->p_lvl2_lat); - fwts_advice(fw, "The FADT P_LVL2_LAT setting specifies the C2 latency in microseconds. The ACPI specification " - "states that a value > 100 indicates that C2 is not supported and hence the " - "ACPI processor idle routine will not use C2 power states."); - } - if (fadt->p_lvl3_lat > 1000) { - fwts_warning(fw, "FADT P_LVL3_LAT is %" PRIu16 ", a value > 1000 indicates a " - "system not to support a C3 state.", fadt->p_lvl3_lat); - fwts_advice(fw, "The FADT P_LVL2_LAT setting specifies the C3 latency in microseconds. The ACPI specification " - "states that a value > 1000 indicates that C3 is not supported and hence the " - "ACPI processor idle routine will not use C3 power states."); - } - */ - - /* * Cannot really test the Hypervisor Vendor Identity since * the value is provided by the hypervisor to the OS (as a * sign that the ACPI tables have been fabricated), if it