[PATCH 0/2] firmware: dmi-sysfs: add SMBIOS

[PATCH 0/2] firmware: dmi-sysfs: add SMBIOS

[Ivan Khoronzhuk]

Some utils, like dmidecode and smbios, needs to access SMBIOS entry
table area in order to get information like SMBIOS version, size, etc.
Currently it's done via /dev/mem. But for situation when /dev/mem
usage is disabled, the utils have to use dmi sysfs instead, which
doesn't represent SMBIOS entry. So this patch series adds SMBIOS
area to dmi sysfs in order to allow utils in question to work
correctly with dmi sysfs.

Ivan Khoronzhuk (2):
  firmware: dmi_scan: add symbol to get SMBIOS entry area
  firmware: dmi-sysfs: add SMBIOS entry point area attribute

 drivers/firmware/dmi-sysfs.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 drivers/firmware/dmi_scan.c  | 34 ++++++++++++++++++++++++++++++++++
 include/linux/dmi.h          |  2 ++
 3 files changed, 78 insertions(+)

-- 
1.9.1

[PATCH 1/2] firmware: dmi_scan: add symbol to get SMBIOS entry area

[Ivan Khoronzhuk]

There are situations when code needs to access SMBIOS entry table
area. For example, to pass it via sysfs to userspace when it's not
allowed to get SMBIOS info via /dev/mem.

Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---
 drivers/firmware/dmi_scan.c | 34 ++++++++++++++++++++++++++++++++++
 include/linux/dmi.h         |  2 ++
 2 files changed, 36 insertions(+)

diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
index 420c8d8..ae9204a 100644
--- a/drivers/firmware/dmi_scan.c
+++ b/drivers/firmware/dmi_scan.c
@@ -113,6 +113,7 @@ static void dmi_table(u8 *buf, int len, int num,
 	}
 }
 
+static unsigned char smbios_header[32];
 static phys_addr_t dmi_base;
 static u16 dmi_len;
 static u16 dmi_num;
@@ -474,6 +475,7 @@ static int __init dmi_present(const u8 *buf)
 	if (memcmp(buf, "_SM_", 4) == 0 &&
 	    buf[5] < 32 && dmi_checksum(buf, buf[5])) {
 		smbios_ver = get_unaligned_be16(buf + 6);
+		memcpy(smbios_header, buf, buf[5]);
 
 		/* Some BIOS report weird SMBIOS version, fix that up */
 		switch (smbios_ver) {
@@ -505,6 +507,7 @@ static int __init dmi_present(const u8 *buf)
 				pr_info("SMBIOS %d.%d present.\n",
 				       dmi_ver >> 8, dmi_ver & 0xFF);
 			} else {
+				memcpy(smbios_header, buf, 15);
 				dmi_ver = (buf[14] & 0xF0) << 4 |
 					   (buf[14] & 0x0F);
 				pr_info("Legacy DMI %d.%d present.\n",
@@ -531,6 +534,7 @@ static int __init dmi_smbios3_present(const u8 *buf)
 		dmi_ver &= 0xFFFFFF;
 		dmi_len = get_unaligned_le32(buf + 12);
 		dmi_base = get_unaligned_le64(buf + 16);
+		memcpy(smbios_header, buf, buf[6]);
 
 		/*
 		 * The 64-bit SMBIOS 3.0 entry point no longer has a field
@@ -944,3 +948,33 @@ void dmi_memdev_name(u16 handle, const char **bank, const char **device)
 	}
 }
 EXPORT_SYMBOL_GPL(dmi_memdev_name);
+
+/**
+ * dmi_get_smbios_entry_area - copy SMBIOS entry point area to array.
+ * @entry - pointer on array to read area in, current max size is 32 bytes.
+ *
+ * returns -ENODATA if table is not available, otherwise returns actual
+ * size of SMBIOS entry point area.
+ */
+int dmi_get_smbios_entry_area(char *table)
+{
+	int size = 0;
+
+	if (!dmi_available)
+		return -ENODATA;
+
+	if (memcmp(smbios_header, "_SM3_", 5) == 0)
+		size = smbios_header[6];
+	else if (memcmp(smbios_header, "_SM_", 4) == 0)
+		size = smbios_header[5];
+	else if (memcmp(smbios_header, "_DMI_", 5) == 0)
+		size = 15;
+
+	memcpy(table, smbios_header, size);
+
+	if (!size)
+		return -ENODATA;
+
+	return size;
+}
+EXPORT_SYMBOL_GPL(dmi_get_smbios_entry_area);
diff --git a/include/linux/dmi.h b/include/linux/dmi.h
index f820f0a..7cae713 100644
--- a/include/linux/dmi.h
+++ b/include/linux/dmi.h
@@ -109,6 +109,7 @@ extern int dmi_walk(void (*decode)(const struct dmi_header *, void *),
 	void *private_data);
 extern bool dmi_match(enum dmi_field f, const char *str);
 extern void dmi_memdev_name(u16 handle, const char **bank, const char **device);
+int dmi_get_smbios_entry_area(char *table);
 
 #else
 
@@ -140,6 +141,7 @@ static inline void dmi_memdev_name(u16 handle, const char **bank,
 		const char **device) { }
 static inline const struct dmi_system_id *
 	dmi_first_match(const struct dmi_system_id *list) { return NULL; }
+static inline int dmi_get_smbios_entry_area(char *table) { return -ENODATA; }
 
 #endif
 
-- 
1.9.1

[PATCH 2/2] firmware: dmi-sysfs: add SMBIOS entry point area attribute

[Ivan Khoronzhuk]

There are situations when code needs to access SMBIOS entry table
area, but cannot use /dev/mem for this. As the table format is
consistent only for a version, and can be changed, use binary
attribute to give access to raw SMBIOS entry table area.

Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---
 drivers/firmware/dmi-sysfs.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/drivers/firmware/dmi-sysfs.c b/drivers/firmware/dmi-sysfs.c
index e0f1cb3..b5c0558 100644
--- a/drivers/firmware/dmi-sysfs.c
+++ b/drivers/firmware/dmi-sysfs.c
@@ -46,6 +46,8 @@ struct dmi_sysfs_entry {
 static LIST_HEAD(entry_list);
 static DEFINE_SPINLOCK(entry_list_lock);
 
+static unsigned char smbios_raw_header[32];
+
 /* dmi_sysfs_attribute - Top level attribute. used by all entries. */
 struct dmi_sysfs_attribute {
 	struct attribute attr;
@@ -646,9 +648,37 @@ static void cleanup_entry_list(void)
 	}
 }
 
+static ssize_t smbios_entry_area_raw_read(struct file *filp,
+					  struct kobject *kobj,
+					  struct bin_attribute *bin_attr,
+					  char *buf, loff_t pos, size_t count)
+{
+	ssize_t size;
+
+	size = bin_attr->size;
+
+	if (size > pos)
+		size -= pos;
+	else
+		return 0;
+
+	if (count < size)
+		size = count;
+
+	memcpy(buf, &smbios_raw_header[pos], size);
+
+	return size;
+}
+
+static struct bin_attribute smbios_entry_area_raw_attr = {
+	.read = smbios_entry_area_raw_read,
+	.attr = {.name = "smbios_raw_header", .mode = 0400},
+};
+
 static int __init dmi_sysfs_init(void)
 {
 	int error = -ENOMEM;
+	int size;
 	int val;
 
 	/* Set up our directory */
@@ -669,6 +699,18 @@ static int __init dmi_sysfs_init(void)
 		goto err;
 	}
 
+	size = dmi_get_smbios_entry_area(smbios_raw_header);
+	if (size == -ENODATA) {
+		pr_debug("dmi-sysfs: SMBIOS raw data is not available.\n");
+		error = size;
+		goto err;
+	}
+
+	/* Create the raw binary file to access the entry area */
+	smbios_entry_area_raw_attr.size = size;
+	if (sysfs_create_bin_file(dmi_kobj, &smbios_entry_area_raw_attr))
+		goto err;
+
 	pr_debug("dmi-sysfs: loaded.\n");
 
 	return 0;
-- 
1.9.1

Re: [PATCH 1/2] firmware: dmi_scan: add symbol to get SMBIOS entry area

[Ard Biesheuvel]

On 23 January 2015 at 20:21, Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote:
> There are situations when code needs to access SMBIOS entry table
> area. For example, to pass it via sysfs to userspace when it's not
> allowed to get SMBIOS info via /dev/mem.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> ---
>  drivers/firmware/dmi_scan.c | 34 ++++++++++++++++++++++++++++++++++
>  include/linux/dmi.h         |  2 ++
>  2 files changed, 36 insertions(+)
>
> diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
> index 420c8d8..ae9204a 100644
> --- a/drivers/firmware/dmi_scan.c
> +++ b/drivers/firmware/dmi_scan.c
> @@ -113,6 +113,7 @@ static void dmi_table(u8 *buf, int len, int num,
>         }
>  }
>
> +static unsigned char smbios_header[32];
>  static phys_addr_t dmi_base;
>  static u16 dmi_len;
>  static u16 dmi_num;
> @@ -474,6 +475,7 @@ static int __init dmi_present(const u8 *buf)
>         if (memcmp(buf, "_SM_", 4) == 0 &&
>             buf[5] < 32 && dmi_checksum(buf, buf[5])) {
>                 smbios_ver = get_unaligned_be16(buf + 6);
> +               memcpy(smbios_header, buf, buf[5]);
>
>                 /* Some BIOS report weird SMBIOS version, fix that up */
>                 switch (smbios_ver) {
> @@ -505,6 +507,7 @@ static int __init dmi_present(const u8 *buf)
>                                 pr_info("SMBIOS %d.%d present.\n",
>                                        dmi_ver >> 8, dmi_ver & 0xFF);
>                         } else {
> +                               memcpy(smbios_header, buf, 15);
>                                 dmi_ver = (buf[14] & 0xF0) << 4 |
>                                            (buf[14] & 0x0F);
>                                 pr_info("Legacy DMI %d.%d present.\n",
> @@ -531,6 +534,7 @@ static int __init dmi_smbios3_present(const u8 *buf)
>                 dmi_ver &= 0xFFFFFF;
>                 dmi_len = get_unaligned_le32(buf + 12);
>                 dmi_base = get_unaligned_le64(buf + 16);
> +               memcpy(smbios_header, buf, buf[6]);
>
>                 /*
>                  * The 64-bit SMBIOS 3.0 entry point no longer has a field
> @@ -944,3 +948,33 @@ void dmi_memdev_name(u16 handle, const char **bank, const char **device)
>         }
>  }
>  EXPORT_SYMBOL_GPL(dmi_memdev_name);
> +
> +/**
> + * dmi_get_smbios_entry_area - copy SMBIOS entry point area to array.
> + * @entry - pointer on array to read area in, current max size is 32 bytes.
> + *
> + * returns -ENODATA if table is not available, otherwise returns actual
> + * size of SMBIOS entry point area.
> + */
> +int dmi_get_smbios_entry_area(char *table)
> +{

What about

const u8 *dmi_get_smbios_header(int *size)

and return the buffer (or NULL if no data) and the size previously
recorded in *size (see below)

The reason is that, in the second patch, you are copying the data into
yet another char[32], which doesn't make a great deal of sense is the
data is not captured right at that time

(I suggested earlier that the correct way to implement this was to
populate the header at the same time you traverse the dmi tree to
populate the sysfs entries. If you capture the data at init time,
there is no reason to copy it yet again at dmi_sysfs module_init time)

> +       int size = 0;
> +
> +       if (!dmi_available)
> +               return -ENODATA;
> +
> +       if (memcmp(smbios_header, "_SM3_", 5) == 0)
> +               size = smbios_header[6];
> +       else if (memcmp(smbios_header, "_SM_", 4) == 0)
> +               size = smbios_header[5];
> +       else if (memcmp(smbios_header, "_DMI_", 5) == 0)
> +               size = 15;
> +

You are duplicating work here: could we record smbios_header_size when
you capture the data itself?

> +       memcpy(table, smbios_header, size);
> +
> +       if (!size)
> +               return -ENODATA;
> +
> +       return size;
> +}
> +EXPORT_SYMBOL_GPL(dmi_get_smbios_entry_area);
> diff --git a/include/linux/dmi.h b/include/linux/dmi.h
> index f820f0a..7cae713 100644
> --- a/include/linux/dmi.h
> +++ b/include/linux/dmi.h
> @@ -109,6 +109,7 @@ extern int dmi_walk(void (*decode)(const struct dmi_header *, void *),
>         void *private_data);
>  extern bool dmi_match(enum dmi_field f, const char *str);
>  extern void dmi_memdev_name(u16 handle, const char **bank, const char **device);
> +int dmi_get_smbios_entry_area(char *table);
>
>  #else
>
> @@ -140,6 +141,7 @@ static inline void dmi_memdev_name(u16 handle, const char **bank,
>                 const char **device) { }
>  static inline const struct dmi_system_id *
>         dmi_first_match(const struct dmi_system_id *list) { return NULL; }
> +static inline int dmi_get_smbios_entry_area(char *table) { return -ENODATA; }
>
>  #endif
>
> --
> 1.9.1
>

Re: [PATCH 1/2] firmware: dmi_scan: add symbol to get SMBIOS entry area

[Ivan Khoronzhuk]

On 01/26/2015 10:44 AM, Ard Biesheuvel wrote:
> On 23 January 2015 at 20:21, Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote:
>> There are situations when code needs to access SMBIOS entry table
>> area. For example, to pass it via sysfs to userspace when it's not
>> allowed to get SMBIOS info via /dev/mem.
>>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>> ---
>>   drivers/firmware/dmi_scan.c | 34 ++++++++++++++++++++++++++++++++++
>>   include/linux/dmi.h         |  2 ++
>>   2 files changed, 36 insertions(+)
>>
>> diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
>> index 420c8d8..ae9204a 100644
>> --- a/drivers/firmware/dmi_scan.c
>> +++ b/drivers/firmware/dmi_scan.c
>> @@ -113,6 +113,7 @@ static void dmi_table(u8 *buf, int len, int num,
>>          }
>>   }
>>
>> +static unsigned char smbios_header[32];
>>   static phys_addr_t dmi_base;
>>   static u16 dmi_len;
>>   static u16 dmi_num;
>> @@ -474,6 +475,7 @@ static int __init dmi_present(const u8 *buf)
>>          if (memcmp(buf, "_SM_", 4) == 0 &&
>>              buf[5] < 32 && dmi_checksum(buf, buf[5])) {
>>                  smbios_ver = get_unaligned_be16(buf + 6);
>> +               memcpy(smbios_header, buf, buf[5]);
>>
>>                  /* Some BIOS report weird SMBIOS version, fix that up */
>>                  switch (smbios_ver) {
>> @@ -505,6 +507,7 @@ static int __init dmi_present(const u8 *buf)
>>                                  pr_info("SMBIOS %d.%d present.\n",
>>                                         dmi_ver >> 8, dmi_ver & 0xFF);
>>                          } else {
>> +                               memcpy(smbios_header, buf, 15);
>>                                  dmi_ver = (buf[14] & 0xF0) << 4 |
>>                                             (buf[14] & 0x0F);
>>                                  pr_info("Legacy DMI %d.%d present.\n",
>> @@ -531,6 +534,7 @@ static int __init dmi_smbios3_present(const u8 *buf)
>>                  dmi_ver &= 0xFFFFFF;
>>                  dmi_len = get_unaligned_le32(buf + 12);
>>                  dmi_base = get_unaligned_le64(buf + 16);
>> +               memcpy(smbios_header, buf, buf[6]);
>>
>>                  /*
>>                   * The 64-bit SMBIOS 3.0 entry point no longer has a field
>> @@ -944,3 +948,33 @@ void dmi_memdev_name(u16 handle, const char **bank, const char **device)
>>          }
>>   }
>>   EXPORT_SYMBOL_GPL(dmi_memdev_name);
>> +
>> +/**
>> + * dmi_get_smbios_entry_area - copy SMBIOS entry point area to array.
>> + * @entry - pointer on array to read area in, current max size is 32 bytes.
>> + *
>> + * returns -ENODATA if table is not available, otherwise returns actual
>> + * size of SMBIOS entry point area.
>> + */
>> +int dmi_get_smbios_entry_area(char *table)
>> +{
> What about
>
> const u8 *dmi_get_smbios_header(int *size)
>
> and return the buffer (or NULL if no data) and the size previously
> recorded in *size (see below)
>
> The reason is that, in the second patch, you are copying the data into
> yet another char[32], which doesn't make a great deal of sense is the
> data is not captured right at that time
>
> (I suggested earlier that the correct way to implement this was to
> populate the header at the same time you traverse the dmi tree to
> populate the sysfs entries. If you capture the data at init time,
> there is no reason to copy it yet again at dmi_sysfs module_init time)


yes. Why I don't trust to return pointer even in case of const.
Even don't remember.
I'll update with const.
Thanks.

>> +       int size = 0;
>> +
>> +       if (!dmi_available)
>> +               return -ENODATA;
>> +
>> +       if (memcmp(smbios_header, "_SM3_", 5) == 0)
>> +               size = smbios_header[6];
>> +       else if (memcmp(smbios_header, "_SM_", 4) == 0)
>> +               size = smbios_header[5];
>> +       else if (memcmp(smbios_header, "_DMI_", 5) == 0)
>> +               size = 15;
>> +
> You are duplicating work here: could we record smbios_header_size when
> you capture the data itself?

~
I'll update soon.

>
>> +       memcpy(table, smbios_header, size);
>> +
>> +       if (!size)
>> +               return -ENODATA;
>> +
>> +       return size;
>> +}
>> +EXPORT_SYMBOL_GPL(dmi_get_smbios_entry_area);
>> diff --git a/include/linux/dmi.h b/include/linux/dmi.h
>> index f820f0a..7cae713 100644
>> --- a/include/linux/dmi.h
>> +++ b/include/linux/dmi.h
>> @@ -109,6 +109,7 @@ extern int dmi_walk(void (*decode)(const struct dmi_header *, void *),
>>          void *private_data);
>>   extern bool dmi_match(enum dmi_field f, const char *str);
>>   extern void dmi_memdev_name(u16 handle, const char **bank, const char **device);
>> +int dmi_get_smbios_entry_area(char *table);
>>
>>   #else
>>
>> @@ -140,6 +141,7 @@ static inline void dmi_memdev_name(u16 handle, const char **bank,
>>                  const char **device) { }
>>   static inline const struct dmi_system_id *
>>          dmi_first_match(const struct dmi_system_id *list) { return NULL; }
>> +static inline int dmi_get_smbios_entry_area(char *table) { return -ENODATA; }
>>
>>   #endif
>>
>> --
>> 1.9.1
>>