[U-Boot] [PATCH 1/2] smbios: add parsing API

[Alexander Graf]

On 16.04.19 13:38, Christian Gmeiner wrote:
> Add an very simple API to be able to access SMBIOS strings
> like vendor, model and bios version.
>
> Signed-off-by: Christian Gmeiner <christian.gmeiner at gmail.com>
> ---
>  include/smbios.h    |  37 ++++++++++
>  lib/Kconfig         |   6 ++
>  lib/Makefile        |   1 +
>  lib/smbios-parser.c | 162 ++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 206 insertions(+)
>  create mode 100644 lib/smbios-parser.c
>
> diff --git a/include/smbios.h b/include/smbios.h
> index 97b9ddce23..ad44e54245 100644
> --- a/include/smbios.h
> +++ b/include/smbios.h
> @@ -237,4 +237,41 @@ typedef int (*smbios_write_type)(ulong *addr, int handle);
>   */
>  ulong write_smbios_table(ulong addr);
>  
> +
> +struct smbios_parser;
> +
> +/**
> + * smbios_parser_create() - Create smbios parser
> + *
> + * @addr:	start address to search for SMBIOS structure
> + * @len:	size of search area
> + * @return:	NULL or valid pointer to a struct smbios_parser
> + */
> +struct smbios_parser *smbios_parser_create(unsigned int addr, unsigned int len);
> +
> +/**
> + * smbios_parser_destroy() - Destroy smbios parser
> + *
> + * @p:	pointer to a struct smbios_parser
> + */
> +void smbios_parser_destroy(struct smbios_parser *p);
> +
> +/**
> + * smbios_header() - Search for SMBIOS header type
> + *
> + * @p:	pointer to a struct smbios_parser
> + * @type:	SMBIOS type
> + * @return:	NULL or a valid pointer to a struct smbios_header
> + */
> +struct smbios_header *smbios_header(const struct smbios_parser *p, int type);
> +
> +/**
> + * smbios_string() - Return string from SMBIOS
> + *
> + * @header:	pointer to struct smbios_header
> + * @index:	string index
> + * @return:	NULL or a valid const char pointer
> + */
> +const char *smbios_string(const struct smbios_header *header, int index);
> +
>  #endif /* _SMBIOS_H_ */
> diff --git a/lib/Kconfig b/lib/Kconfig
> index 2120216593..59aafc63a7 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -432,6 +432,12 @@ config SMBIOS_PRODUCT_NAME
>  
>  endmenu
>  
> +config SMBIOS_PARSER
> +	bool "SMBIOS parser"
> +	default n
> +	help
> +	  A simple parser for SMBIOS data.
> +
>  source lib/efi/Kconfig
>  source lib/efi_loader/Kconfig
>  source lib/optee/Kconfig
> diff --git a/lib/Makefile b/lib/Makefile
> index 47829bfed5..f095bd420a 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -34,6 +34,7 @@ obj-$(CONFIG_FIT) += fdtdec_common.o
>  obj-$(CONFIG_TEST_FDTDEC) += fdtdec_test.o
>  obj-$(CONFIG_GZIP_COMPRESSED) += gzip.o
>  obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += smbios.o
> +obj-$(CONFIG_SMBIOS_PARSER) += smbios-parser.o
>  obj-$(CONFIG_IMAGE_SPARSE) += image-sparse.o
>  obj-y += ldiv.o
>  obj-$(CONFIG_MD5) += md5.o
> diff --git a/lib/smbios-parser.c b/lib/smbios-parser.c
> new file mode 100644
> index 0000000000..b24fd7210c
> --- /dev/null
> +++ b/lib/smbios-parser.c
> @@ -0,0 +1,162 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2019, Bachmann electronic GmbH
> + */
> +
> +#include <common.h>
> +#include <malloc.h>
> +#include <mapmem.h>
> +#include <smbios.h>
> +
> +struct smbios_parser {
> +	void *search_area;
> +	unsigned int search_area_len;
> +	struct smbios_entry *entry;
> +	void *data;
> +};
> +
> +static inline int verify_checksum(const struct smbios_entry *e)
> +{
> +	/*
> +	 * Checksums for SMBIOS tables are calculated to have a value, so that
> +	 * the sum over all bytes yields zero (using unsigned 8 bit arithmetic).
> +	 */
> +	u8 *byte = (u8 *)e;
> +	u8 sum = 0;
> +
> +	for (int i = 0; i < e->length; i++)
> +		sum += byte[i];
> +
> +	return sum;
> +}
> +
> +static struct smbios_entry *find_smbios_entry(const struct smbios_parser *p)
> +{
> +	static const char sm_sig[] = "_SM_";
> +	u8 *buf = (u8 *)p->search_area;
> +	const u8 *end = (u8 *)p->search_area + p->search_area_len;
> +	struct smbios_entry *entry = NULL;
> +
> +	/* look for smbios entry */
> +	while (buf < end) {


Don't you have a way to pass the DMI pointer directly, maybe using cbfs?
Searching through the SMBIOS search area is so 1990s (and pretty much
only works on x86).


I also don't think we really need all the smbios_parser logic. No need
to create special pseudo objects. IMHO you should either create an
actual UCLASS for SMBIOS with genuine callbacks and expose the table as
DM object or just make all search functions in this patch stateless. I'd
opt for the latter - it's how the SMBIOS creation works too.


Alex


> +		if (memcmp(sm_sig, buf, 4) == 0) {
> +			entry = (struct smbios_entry *)buf;
> +
> +			if (verify_checksum(entry)) {
> +				entry = NULL;
> +				buf++;
> +				continue;
> +			}
> +
> +			break;
> +		}
> +
> +		buf++;
> +	}
> +
> +	return entry;
> +}
> +
> +void smbios_parser_destroy(struct smbios_parser *p)
> +{
> +	if (!p)
> +		return;
> +
> +	unmap_sysmem(p->data);
> +	unmap_sysmem(p->search_area);
> +
> +	free(p);
> +}
> +
> +struct smbios_parser *smbios_parser_create(unsigned int addr, unsigned int len)
> +{
> +	struct smbios_parser *p = malloc(sizeof(struct smbios_parser));
> +
> +	if (!p)
> +		return NULL;
> +
> +	p->search_area_len = len;
> +	p->search_area = map_sysmem(addr, len);
> +
> +	if (!p->search_area)
> +		goto fail;
> +
> +	/* find main entry smbios table */
> +	p->entry = find_smbios_entry(p);
> +
> +	if (!p->entry)
> +		goto fail;
> +
> +	/* map smbios data area */
> +	p->data = map_sysmem(p->entry->struct_table_address,
> +			     p->entry->struct_table_length);
> +
> +	if (!p->data)
> +		goto fail;
> +
> +	return p;
> +
> +fail:
> +	smbios_parser_destroy(p);
> +
> +	return NULL;
> +}
> +
> +static struct smbios_header *next_header(const struct smbios_header *curr)
> +{
> +	u8 *pos = ((u8 *)curr) + curr->length;
> +
> +	/* search for _double_ NULL bytes */
> +	while (!((*pos == 0) && (*(pos + 1) == 0)))
> +		pos++;
> +
> +	/* step behind the double NULL bytes */
> +	pos += 2;
> +
> +	return (struct smbios_header *)pos;
> +}
> +
> +struct smbios_header *smbios_header(const struct smbios_parser *p, int type)
> +{
> +	const struct smbios_entry *entry = p->entry;
> +	const unsigned int num_header = entry->struct_count;
> +	struct smbios_header *header = (struct smbios_header *)p->data;
> +
> +	for (unsigned int i = 0; i < num_header; i++) {
> +		if (header->type == type)
> +			return header;
> +
> +		header = next_header(header);
> +	}
> +
> +	return NULL;
> +}
> +
> +static const char *string_from_smbios_table(const struct smbios_header *header,
> +					    int idx)
> +{
> +	unsigned int i = 1;
> +	u8 *pos;
> +
> +	if (!header)
> +		return NULL;
> +
> +	pos = ((u8 *)header) + header->length;
> +
> +	while (i < idx) {
> +		if (*pos == 0x0)
> +			i++;
> +
> +		pos++;
> +	}
> +
> +	return (const char *)pos;
> +}
> +
> +const char *smbios_string(const struct smbios_header *header, int index)
> +{
> +	if (!header)
> +		return NULL;
> +
> +	return string_from_smbios_table(header, index);
> +}