ACK: [PATCH 1/2] dmi: dmicheck: fix SMBIOS issues on aarch64 systems
Alex Hung
alex.hung at canonical.com
Tue Oct 13 07:09:44 UTC 2015
On 10/08/2015 11:04 PM, Colin King wrote:
> From: Jiri Vohanka <jvohanka at redhat.com>
>
> fix two issues in the 'dmicheck' test:
>
> 1) If 'SMBIOS' tables are not found in 'dmicheck_test2' then the
> tests for SMBIOS 3.0 tables are skipped as well. But the
> SMBIOS reference specification (version 3.0.0) says in
> section 5 (Accessing SMBIOS information):
> 'An implementation may provide either the 32-bit entry
> point or the 64-bit entry point, or both.'.
> This bug affected aarch64 systems I worked with since the 32-bit
> tables were not provided.
>
> 2) As of commit https://git.kernel.org/cgit/linux/kernel/git/arm64/
> linux.git/commit/?id=d7f96f97c4031fa4ffdb7801f9aae23e96170a6f,
> the SMBIOS tables are accessible in
> /sys/firmware/dmi/tables/smbios_entry_point and
> /sys/firmware/dmi/tables/DMI. This bug affected aarch64 systems
> I worked with since the access to SMBIOS tables via /dev/mem
> was not allowed if kernel was compiled with 'CONFIG_STRICT_DEVMEM=y'.
>
> The attached patch includes the following changes:
>
> - The test 'dmicheck_test2' is split into 'dmicheck_test2' and
> 'dmicheck_test3', the first one checking SMBIOS table and the
> second one checking SMBIOS 3.0 table. Thus, if one of the
> SMBIOS tables is missing then only one of the tests is skipped.
>
> - If the access to SMBIOS tables via /dev/mem fails on UEFI systems
> then the tables are loaded from /sys/firmware/dmi/tables/.
> If /sys/firmware/dmi/tables/smbios_entry_point begins with '_SM_'
> then the tables from /sys/firmware/dmi/tables/ are used as SMBIOS
> tables.
> If /sys/firmware/dmi/tables/smbios_entry_point begins with '_SM3_'
> then the tables from /sys/firmware/dmi/tables/ are used as SMBIOS
> 3.0 tables. This change allows access to SMBIOS tables without
> using /dev/mem on UEFI systems. (The address of SMBIOS entry point
> structure can be taken form /sys/firmware/efi/systab and the SMBIOS
> tables can be read from /sys/firmware/dmi/tables/.)
>
> I tested the patch on:
> - aarch64 system (with UEFI) - with 4.2.0 kernel
> - x86_64 system without UEFI with access to /dev/mem -
> Fedora23 with kernel-4.2.2-300.fc23.x86_64
> - x86_64 system with UEFI without acess to /dev/mem -
> Fedora23 with kernel-4.2.2-300.fc23.x86_64
> - x86 system without UEFI with access to /dev/mem -
> Fedora23 with kernel-4.2.2-300.fc23.i686+PAE
>
> [ with minor commit message edits and some whitespace clean up by
> Colin Ian King ]
>
> Signed-off-by: Jiri Vohanka <jvohanka at redhat.com>
> Tested-by: Colin Ian King <colin.king at canonical.com>
> Signed-off-by: Colin Ian King <colin.king at canonical.com>
> ---
> src/dmi/dmicheck/dmicheck.c | 156 ++++++++++++++++++++++++++++++++------------
> src/lib/src/fwts_smbios.c | 66 +++++++++++++++----
> 2 files changed, 166 insertions(+), 56 deletions(-)
>
> diff --git a/src/dmi/dmicheck/dmicheck.c b/src/dmi/dmicheck/dmicheck.c
> index d0dc183..eeb227a 100644
> --- a/src/dmi/dmicheck/dmicheck.c
> +++ b/src/dmi/dmicheck/dmicheck.c
> @@ -33,6 +33,7 @@
> #include <sys/stat.h>
> #include <unistd.h>
> #include <limits.h>
> +#include <fcntl.h>
>
> #define DMI_VERSION (0x0300)
> #define VERSION_MAJOR(v) ((v) >> 8)
> @@ -309,6 +310,94 @@ static fwts_dmi_used_by_kernel dmi_used_by_kernel_table[] = {
> { TYPE_EOD, 0xff },
> };
>
> +static int dmi_load_file(const char* filename, void *buf, size_t size)
> +{
> + int fd;
> + ssize_t ret;
> +
> + if ((fd = open(filename, O_RDONLY)) < 0)
> + return FWTS_ERROR;
> + ret = read(fd, buf, size);
> + close(fd);
> + if (ret != (ssize_t)size)
> + return FWTS_ERROR;
> + return FWTS_OK;
> +}
> +
> +static void* dmi_table_smbios(fwts_framework *fw, fwts_smbios_entry *entry)
> +{
> + off_t addr = (off_t)entry->struct_table_address;
> + size_t length = (size_t)entry->struct_table_length;
> + void *table;
> + void *mem;
> + char anchor[8];
> +
> + mem = fwts_mmap(addr, length);
> + if (mem != FWTS_MAP_FAILED) {
> + table = malloc(length);
> + if (table)
> + memcpy(table, mem, length);
> + (void)fwts_munmap(mem, length);
> + return table;
> + }
> +
> + if (dmi_load_file("/sys/firmware/dmi/tables/smbios_entry_point", anchor, 4) == FWTS_OK
> + && strncmp(anchor, "_SM_", 4) == 0) {
> + table = malloc(length);
> + if (!table)
> + return NULL;
> + if (dmi_load_file("/sys/firmware/dmi/tables/DMI", table, length) == FWTS_OK) {
> + fwts_log_info(fw, "SMBIOS table loaded from /sys/firmware/dmi/tables/DMI\n");
> + return table;
> + }
> + free(table);
> + }
> +
> + fwts_log_error(fw, "Cannot mmap SMBIOS table from %8.8" PRIx32 "..%8.8" PRIx32 ".",
> + entry->struct_table_address, entry->struct_table_address + entry->struct_table_length);
> + return NULL;
> +}
> +
> +static void* dmi_table_smbios30(fwts_framework *fw, fwts_smbios30_entry *entry)
> +{
> + off_t addr = (off_t)entry->struct_table_address;
> + size_t length = (size_t)entry->struct_table_max_size;
> + void *table;
> + void *mem;
> + char anchor[8];
> +
> + mem = fwts_mmap(addr, length);
> + if (mem != FWTS_MAP_FAILED) {
> + table = malloc(length);
> + if (table)
> + memcpy(table, mem, length);
> + (void)fwts_munmap(mem, length);
> + return table;
> + }
> +
> + if (dmi_load_file("/sys/firmware/dmi/tables/smbios_entry_point", anchor, 5) == FWTS_OK
> + && strncmp(anchor, "_SM3_", 5) == 0) {
> + table = malloc(length);
> + if (!table)
> + return NULL;
> + if (dmi_load_file("/sys/firmware/dmi/tables/DMI", table, length) == FWTS_OK) {
> + fwts_log_info(fw, "SMBIOS30 table loaded from /sys/firmware/dmi/tables/DMI\n");
> + return table;
> + }
> + free(table);
> + }
> +
> + fwts_log_error(fw, "Cannot mmap SMBIOS 3.0 table from %16.16" PRIx64 "..%16.16" PRIx64 ".",
> + entry->struct_table_address, entry->struct_table_address + entry->struct_table_max_size);
> + return NULL;
> +}
> +
> +static void dmi_table_free(void* table)
> +{
> + if (table)
> + free(table);
> +}
> +
> static void dmi_dump_entry(fwts_framework *fw, fwts_smbios_entry *entry, fwts_smbios_type type)
> {
> if (type == FWTS_SMBIOS) {
> @@ -365,17 +454,9 @@ static int dmi_sane(fwts_framework *fw, fwts_smbios_entry *entry)
> uint16_t table_length = entry->struct_table_length;
> int ret = FWTS_OK;
>
> - ptr = table = fwts_mmap((off_t)entry->struct_table_address,
> - (size_t)table_length);
> - if (table == FWTS_MAP_FAILED) {
> - fwts_failed(fw, LOG_LEVEL_MEDIUM,
> - "SMBIOSTableAddressNotMapped",
> - "Cannot mmap SMBIOS tables from "
> - "%8.8" PRIx32 "..%8.8" PRIx32 ".",
> - entry->struct_table_address,
> - entry->struct_table_address + table_length);
> - return FWTS_ERROR;
> - }
> + ptr = table = dmi_table_smbios(fw, entry);
> + if (table == NULL)
> + return FWTS_ERROR;
>
> for (i = 0; i < entry->number_smbios_structures; i++) {
> if (ptr > table + table_length) {
> @@ -422,7 +503,7 @@ static int dmi_sane(fwts_framework *fw, fwts_smbios_entry *entry)
> ret = FWTS_ERROR;
> }
>
> - (void)fwts_munmap(table, (size_t)entry->struct_table_length);
> + dmi_table_free(table);
>
> return ret;
> }
> @@ -520,17 +601,9 @@ static int dmi_smbios30_sane(fwts_framework *fw, fwts_smbios30_entry *entry)
> uint32_t table_length = entry->struct_table_max_size;
> int ret = FWTS_OK;
>
> - ptr = table = fwts_mmap((off_t)entry->struct_table_address,
> - (size_t)table_length);
> - if (table == FWTS_MAP_FAILED) {
> - fwts_failed(fw, LOG_LEVEL_MEDIUM,
> - "SMBIOS30TableAddressNotMapped",
> - "Cannot mmap SMBIOS 3.0 tables from "
> - "%16.16" PRIx64 "..%16.16" PRIx64 ".",
> - entry->struct_table_address,
> - entry->struct_table_address + table_length);
> - return FWTS_ERROR;
> - }
> + ptr = table = dmi_table_smbios30(fw, entry);
> + if (table == NULL)
> + return FWTS_ERROR;
>
> while (1)
> {
> @@ -579,7 +652,7 @@ static int dmi_smbios30_sane(fwts_framework *fw, fwts_smbios30_entry *entry)
> }
> }
>
> - (void)fwts_munmap(table, (size_t)entry->struct_table_max_size);
> + dmi_table_free(table);
>
> return ret;
> }
> @@ -1796,7 +1869,6 @@ static int dmicheck_test2(fwts_framework *fw)
> {
> void *addr;
> fwts_smbios_entry entry;
> - fwts_smbios30_entry entry30;
> fwts_smbios_type type;
> uint16_t version = 0;
> uint8_t *table;
> @@ -1823,20 +1895,23 @@ static int dmicheck_test2(fwts_framework *fw)
> if (dmi_version_check(fw, version) != FWTS_OK)
> return FWTS_SKIP;
>
> - table = fwts_mmap((off_t)entry.struct_table_address,
> - (size_t)entry.struct_table_length);
> - if (table == FWTS_MAP_FAILED) {
> - fwts_log_error(fw, "Cannot mmap SMBIOS tables from "
> - "%8.8" PRIx32 "..%8.8" PRIx32 ".",
> - entry.struct_table_address,
> - entry.struct_table_address + entry.struct_table_length);
> + table = dmi_table_smbios(fw, &entry);
> + if (table == NULL)
> return FWTS_ERROR;
> - }
>
> dmi_scan_tables(fw, &entry, table);
>
> - (void)fwts_munmap(table, (size_t)entry.struct_table_length);
> + dmi_table_free(table);
>
> + return FWTS_OK;
> +}
> +
> +static int dmicheck_test3(fwts_framework *fw)
> +{
> + void *addr;
> + fwts_smbios30_entry entry30;
> + uint16_t version = 0;
> + uint8_t *table;
>
> if (!smbios30_found) {
> fwts_skipped(fw, "Cannot find SMBIOS30 table entry, skip the test.");
> @@ -1850,19 +1925,13 @@ static int dmicheck_test2(fwts_framework *fw)
> return FWTS_ERROR;
> }
>
> - table = fwts_mmap((off_t)entry30.struct_table_address,
> - (size_t)entry30.struct_table_max_size);
> - if (table == FWTS_MAP_FAILED) {
> - fwts_log_error(fw, "Cannot mmap SMBIOS 3.0 table from "
> - "%16.16" PRIx64 "..%16.16" PRIx64 ".",
> - entry30.struct_table_address,
> - entry30.struct_table_address + entry30.struct_table_max_size);
> + table = dmi_table_smbios30(fw, &entry30);
> + if (table == NULL)
> return FWTS_ERROR;
> - }
>
> dmi_scan_smbios30_table(fw, &entry30, table);
>
> - (void)fwts_munmap(table, (size_t)entry30.struct_table_max_size);
> + dmi_table_free(table);
>
> return FWTS_OK;
> }
> @@ -1870,6 +1939,7 @@ static int dmicheck_test2(fwts_framework *fw)
> static fwts_framework_minor_test dmicheck_tests[] = {
> { dmicheck_test1, "Find and test SMBIOS Table Entry Points." },
> { dmicheck_test2, "Test DMI/SMBIOS tables for errors." },
> + { dmicheck_test3, "Test DMI/SMBIOS3 tables for errors." },
> { NULL, NULL }
> };
>
> diff --git a/src/lib/src/fwts_smbios.c b/src/lib/src/fwts_smbios.c
> index 8b0d975..4cafb81 100644
> --- a/src/lib/src/fwts_smbios.c
> +++ b/src/lib/src/fwts_smbios.c
> @@ -17,9 +17,32 @@
> *
> */
>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +#include <unistd.h>
> #include "fwts.h"
>
> #if defined(FWTS_ARCH_INTEL) || defined(FWTS_ARCH_AARCH64)
> +
> +/*
> + * fwts_load_file()
> + * loads file to memory
> + */
> +static int fwts_load_file(const char* filename, void *buf, size_t size)
> +{
> + int fd;
> + ssize_t ret;
> +
> + if ((fd = open(filename, O_RDONLY)) < 0)
> + return FWTS_ERROR;
> + ret = read(fd, buf, size);
> + close(fd);
> + if (ret != (ssize_t)size)
> + return FWTS_ERROR;
> + return FWTS_OK;
> +}
> +
> /*
> * fwts_smbios_find_entry_uefi()
> * find SMBIOS structure table entry from UEFI systab
> @@ -30,15 +53,24 @@ static void *fwts_smbios_find_entry_uefi(fwts_framework *fw, fwts_smbios_entry *
> fwts_smbios_entry *mapped_entry;
>
> if ((addr = fwts_scan_efi_systab("SMBIOS")) != NULL) {
> - if ((mapped_entry = fwts_mmap((off_t)addr, sizeof(fwts_smbios_entry))) == FWTS_MAP_FAILED) {
> - fwts_log_error(fw, "Cannot mmap SMBIOS entry at %p\n", addr);
> - return NULL;
> +
> + if ((mapped_entry = fwts_mmap((off_t)addr, sizeof(fwts_smbios_entry))) != FWTS_MAP_FAILED) {
> + *entry = *mapped_entry;
> + (void)fwts_munmap(mapped_entry, sizeof(fwts_smbios_entry));
> + *type = FWTS_SMBIOS;
> + return addr;
> }
> - *entry = *mapped_entry;
> - *type = FWTS_SMBIOS;
> - (void)fwts_munmap(mapped_entry, sizeof(fwts_smbios_entry));
> +
> + if (fwts_load_file("/sys/firmware/dmi/tables/smbios_entry_point",
> + entry, sizeof(fwts_smbios_entry)) == FWTS_OK && !strncmp((char*)entry, "_SM_", 4)) {
> + fwts_log_info(fw, "SMBIOS entry loaded from /sys/firmware/dmi/tables/smbios_entry_point\n");
> + *type = FWTS_SMBIOS;
> + return addr;
> + }
> +
> + fwts_log_error(fw, "Cannot mmap SMBIOS entry at %p\n", addr);
> }
> - return addr;
> + return NULL;
> }
>
> /*
> @@ -51,14 +83,22 @@ static void *fwts_smbios30_find_entry_uefi(fwts_framework *fw, fwts_smbios30_ent
> fwts_smbios30_entry *mapped_entry;
>
> if ((addr = fwts_scan_efi_systab("SMBIOS3")) != NULL) {
> - if ((mapped_entry = fwts_mmap((off_t)addr, sizeof(fwts_smbios30_entry))) == FWTS_MAP_FAILED) {
> - fwts_log_error(fw, "Cannot mmap SMBIOS30 entry at %p\n", addr);
> - return NULL;
> +
> + if ((mapped_entry = fwts_mmap((off_t)addr, sizeof(fwts_smbios30_entry))) != FWTS_MAP_FAILED) {
> + *entry = *mapped_entry;
> + (void)fwts_munmap(mapped_entry, sizeof(fwts_smbios30_entry));
> + return addr;
> }
> - *entry = *mapped_entry;
> - (void)fwts_munmap(mapped_entry, sizeof(fwts_smbios30_entry));
> +
> + if (fwts_load_file("/sys/firmware/dmi/tables/smbios_entry_point",
> + entry, sizeof(fwts_smbios30_entry)) == FWTS_OK && !strncmp((char*)entry, "_SM3_", 5)) {
> + fwts_log_info(fw, "SMBIOS30 entry loaded from /sys/firmware/dmi/tables/smbios_entry_point\n");
> + return addr;
> + }
> +
> + fwts_log_error(fw, "Cannot mmap SMBIOS30 entry at %p\n", addr);
> }
> - return addr;
> + return NULL;
> }
>
> #if defined(FWTS_ARCH_INTEL)
>
Acked-by: Alex Hung <alex.hung at canonical.com>
More information about the fwts-devel
mailing list