From patchwork Sat Oct 20 19:54:00 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Colin Ian King X-Patchwork-Id: 192946 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from chlorine.canonical.com (chlorine.canonical.com [91.189.94.204]) by ozlabs.org (Postfix) with ESMTP id 82F6A2C008D for ; Sun, 21 Oct 2012 06:54:07 +1100 (EST) Received: from localhost ([127.0.0.1] helo=chlorine.canonical.com) by chlorine.canonical.com with esmtp (Exim 4.71) (envelope-from ) id 1TPf7W-0004YC-BB; Sat, 20 Oct 2012 19:54:06 +0000 Received: from youngberry.canonical.com ([91.189.89.112]) by chlorine.canonical.com with esmtp (Exim 4.71) (envelope-from ) id 1TPf7U-0004Xf-2E for fwts-devel@lists.ubuntu.com; Sat, 20 Oct 2012 19:54:04 +0000 Received: from cpc3-craw6-2-0-cust180.croy.cable.virginmedia.com ([77.100.248.181] helo=localhost) by youngberry.canonical.com with esmtpsa (TLS1.0:DHE_RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1TPf7T-0000WQ-U3 for fwts-devel@lists.ubuntu.com; Sat, 20 Oct 2012 19:54:04 +0000 From: Colin King To: fwts-devel@lists.ubuntu.com Subject: [PATCH 3/4] acpi: mcfg: more code tidying. Date: Sat, 20 Oct 2012 20:54:00 +0100 Message-Id: <1350762841-5702-4-git-send-email-colin.king@canonical.com> X-Mailer: git-send-email 1.7.10.4 In-Reply-To: <1350762841-5702-1-git-send-email-colin.king@canonical.com> References: <1350762841-5702-1-git-send-email-colin.king@canonical.com> X-BeenThere: fwts-devel@lists.ubuntu.com X-Mailman-Version: 2.1.13 Precedence: list List-Id: Firmware Test Suite Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: fwts-devel-bounces@lists.ubuntu.com Errors-To: fwts-devel-bounces@lists.ubuntu.com From: Colin Ian King This code really needs a bit of a clean up. Clean up argument types in compare_config_space() and break up some overly long lines. Also improve the pretty printing of the configuration data. Signed-off-by: Colin Ian King Acked-by: Alex Hung Acked-by: Keng-Yu Lin --- src/acpi/mcfg/mcfg.c | 86 +++++++++++++++++++++++++++++++------------------- 1 file changed, 53 insertions(+), 33 deletions(-) diff --git a/src/acpi/mcfg/mcfg.c b/src/acpi/mcfg/mcfg.c index adc5e3a..1b96b26 100644 --- a/src/acpi/mcfg/mcfg.c +++ b/src/acpi/mcfg/mcfg.c @@ -27,17 +27,22 @@ #include #include #include +#include static fwts_list *memory_map_list; static fwts_acpi_table_info *mcfg_table; -static void compare_config_space(fwts_framework *fw, int segment, int device, unsigned char *space) +static void compare_config_space( + fwts_framework *fw, + const uint16_t segment, + const uint32_t device, + const uint8_t *space) { fwts_list *lspci_output; fwts_list_link *item; char command[PATH_MAX]; - char compare_line[1024]; + char compare_line[50]; snprintf(compare_line, sizeof(compare_line), "%02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x", @@ -46,7 +51,7 @@ static void compare_config_space(fwts_framework *fw, int segment, int device, un space[8], space[9], space[10], space[11], space[12], space[13], space[14], space[15]); - snprintf(command, sizeof(command), "%s -vxxx -s %i:%i", + snprintf(command, sizeof(command), "%s -vxxx -s %" PRIu16 ":%" PRIu32, fw->lspci, segment, device); if (fwts_pipe_exec(command, &lspci_output) == FWTS_EXEC_ERROR) { @@ -54,22 +59,31 @@ static void compare_config_space(fwts_framework *fw, int segment, int device, un return; } + if (lspci_output == NULL) { + fwts_log_warning(fw, + "Failed to get lspci info for %" PRIu16 ":%" PRIu32, + segment, device); + return; + } + fwts_list_foreach(item, lspci_output) { char *line = fwts_text_list_text(item); fwts_chop_newline(line); - if (strncmp(line, "00: ",4)==0) { + if (strncmp(line, "00: ", 4) ==0) { if (strcmp(&line[4], compare_line)) { - fwts_log_info(fw, "%s is read from MMCONFIG, but traditional gives :\n-%s-\n", &line[4], compare_line); + fwts_log_info(fw, + "%s is read from MMCONFIG, but config " + "space gives :\n-%s-\n", &line[4], compare_line); fwts_failed(fw, LOG_LEVEL_MEDIUM, "PCIConfigSpaceBad", - "PCI config space appears to not work"); + "PCI config space appears to not work."); } else - fwts_passed(fw, "PCI config space verified"); + fwts_passed(fw, "PCI config space verified."); fwts_text_list_free(lspci_output); - return ; + return; } } fwts_text_list_free(lspci_output); @@ -86,7 +100,9 @@ static int mcfg_init(fwts_framework *fw) return FWTS_ERROR; } if (mcfg_table == NULL) { - fwts_log_error(fw, "ACPI table MCFG not found. This table is required to check for PCI Express*"); + fwts_log_error(fw, + "ACPI table MCFG not found. This table is " + "required to check for PCI Express*"); return FWTS_ERROR; } @@ -104,10 +120,10 @@ static int mcfg_deinit(fwts_framework *fw) static int mcfg_test1(fwts_framework *fw) { int nr, i; - void *mapped_table_page; + uint8_t *mapped_table_page; fwts_acpi_table_mcfg *mcfg = (fwts_acpi_table_mcfg*)mcfg_table->data; fwts_acpi_mcfg_configuration *config; - int failed = 0; + bool failed = false; ssize_t mcfg_size; const char *memory_map_name; int page_size; @@ -143,9 +159,10 @@ static int mcfg_test1(fwts_framework *fw) mcfg_size + sizeof(fwts_acpi_table_mcfg)); fwts_tag_failed(fw, FWTS_TAG_ACPI_INVALID_TABLE); fwts_advice(fw, - "MCFG table must be least %d bytes (header size) with " - "multiples of %d bytes for each MCFG entry.", - 36+8, (int)sizeof(fwts_acpi_mcfg_configuration)); + "MCFG table must be least %zd bytes (header size) with " + "multiples of %zd bytes for each MCFG entry.", + sizeof(fwts_acpi_table_mcfg), + sizeof(fwts_acpi_mcfg_configuration)); return FWTS_ERROR; } nr = mcfg_size / sizeof(fwts_acpi_mcfg_configuration); @@ -163,8 +180,9 @@ static int mcfg_test1(fwts_framework *fw) return FWTS_ERROR; } - fwts_log_info(fw, "MCFG table found, size is %zd bytes (excluding header) (%i entries).", - mcfg_size, nr); + fwts_log_info(fw, + "MCFG table found, size is %zd bytes (excluding header) (%i entries).", + mcfg_size, nr); if (mcfg == NULL) { fwts_failed(fw, LOG_LEVEL_HIGH, "MCFGInvalidTable", @@ -179,7 +197,11 @@ static int mcfg_test1(fwts_framework *fw) config = &mcfg->configuration[0]; for (i = 0; i < nr; i++, config++) { - fwts_log_info(fw, "Configuration Entry #%d address : 0x%" PRIx64, i, config->base_address); + fwts_log_info_verbatum(fw, "Configuration Entry #%d:", i); + fwts_log_info_verbatum(fw, " Base Address : 0x%" PRIx64, config->base_address); + fwts_log_info_verbatum(fw, " Segment : %" PRIu8, config->pci_segment_group_number); + fwts_log_info_verbatum(fw, " Start bus : %" PRIu8, config->start_bus_number); + fwts_log_info_verbatum(fw, " End bus : %" PRIu8, config->end_bus_number); if ((memory_map_list != NULL) && (!fwts_memory_map_is_reserved(memory_map_list, config->base_address))) { @@ -189,21 +211,22 @@ static int mcfg_test1(fwts_framework *fw) " is not reserved in the memory map table", config->base_address); fwts_tag_failed(fw, FWTS_TAG_BIOS); + fwts_advice(fw, - "The PCI Express specification states that the PCI Express configuration space should " - "be defined in the MCFG table and *maybe* optionally defined in the %s " - "if ACPI MCFG is present. " - "Linux checks if the region is reserved in the memory map table and will reject the " - "MMCONFIG if there is a discrepency between MCFG and the memory map table for the " - "PCI Express region. [See arch/x86/pci/mmconfig-shared.c pci_mmcfg_reject_broken()]. " - "It is recommended that this is defined in the %s table for Linux.", + "The PCI Express specification states that the " + "PCI Express configuration space should " + "be defined in the MCFG table and *maybe* " + "optionally defined in the %s if ACPI MCFG is " + "present. Linux checks if the region is reserved " + "in the memory map table and will reject the " + "MMCONFIG if there is a discrepency between MCFG " + "and the memory map table for the PCI Express region. " + "[See arch/x86/pci/mmconfig-shared.c pci_mmcfg_reject_broken()]. " + "It is recommended that this is defined in the " + "%s table for Linux.", memory_map_name, memory_map_name); - failed++; + failed = true; } - - fwts_log_info_verbatum(fw, "Segment : %" PRIu8, config->pci_segment_group_number); - fwts_log_info_verbatum(fw, "Start bus : %" PRIu8, config->start_bus_number); - fwts_log_info_verbatum(fw, "End bus : %" PRIu8, config->end_bus_number); } if (!failed) fwts_passed(fw, "MCFG mmio config space is reserved in memory map table."); @@ -215,10 +238,7 @@ static int mcfg_test1(fwts_framework *fw) fwts_log_error(fw, "Cannot mmap table at 0x%" PRIx64 ".", config->base_address); return FWTS_ERROR; } - - compare_config_space(fw, config->pci_segment_group_number, - 0, (unsigned char *)mapped_table_page); - + compare_config_space(fw, config->pci_segment_group_number, 0, mapped_table_page); fwts_munmap(mapped_table_page, page_size); return FWTS_OK;