LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 0/2] firmware: dmi-sysfs: add SMBIOS
@ 2015-01-23 20:21 Ivan Khoronzhuk
  2015-01-23 20:21 ` [PATCH 1/2] firmware: dmi_scan: add symbol to get SMBIOS entry area Ivan Khoronzhuk
  2015-01-23 20:21 ` [PATCH 2/2] firmware: dmi-sysfs: add SMBIOS entry point area attribute Ivan Khoronzhuk
  0 siblings, 2 replies; 5+ messages in thread
From: Ivan Khoronzhuk @ 2015-01-23 20:21 UTC (permalink / raw)
  To: linux-kernel, ard.biesheuvel
  Cc: dmidecode-devel, grant.likely, leif.lindholm, matt.fleming,
	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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] firmware: dmi_scan: add symbol to get SMBIOS entry area
  2015-01-23 20:21 [PATCH 0/2] firmware: dmi-sysfs: add SMBIOS Ivan Khoronzhuk
@ 2015-01-23 20:21 ` Ivan Khoronzhuk
  2015-01-26  8:44   ` Ard Biesheuvel
  2015-01-23 20:21 ` [PATCH 2/2] firmware: dmi-sysfs: add SMBIOS entry point area attribute Ivan Khoronzhuk
  1 sibling, 1 reply; 5+ messages in thread
From: Ivan Khoronzhuk @ 2015-01-23 20:21 UTC (permalink / raw)
  To: linux-kernel, ard.biesheuvel
  Cc: dmidecode-devel, grant.likely, leif.lindholm, matt.fleming,
	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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 2/2] firmware: dmi-sysfs: add SMBIOS entry point area attribute
  2015-01-23 20:21 [PATCH 0/2] firmware: dmi-sysfs: add SMBIOS Ivan Khoronzhuk
  2015-01-23 20:21 ` [PATCH 1/2] firmware: dmi_scan: add symbol to get SMBIOS entry area Ivan Khoronzhuk
@ 2015-01-23 20:21 ` Ivan Khoronzhuk
  1 sibling, 0 replies; 5+ messages in thread
From: Ivan Khoronzhuk @ 2015-01-23 20:21 UTC (permalink / raw)
  To: linux-kernel, ard.biesheuvel
  Cc: dmidecode-devel, grant.likely, leif.lindholm, matt.fleming,
	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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] firmware: dmi_scan: add symbol to get SMBIOS entry area
  2015-01-23 20:21 ` [PATCH 1/2] firmware: dmi_scan: add symbol to get SMBIOS entry area Ivan Khoronzhuk
@ 2015-01-26  8:44   ` Ard Biesheuvel
  2015-01-26 11:53     ` Ivan Khoronzhuk
  0 siblings, 1 reply; 5+ messages in thread
From: Ard Biesheuvel @ 2015-01-26  8:44 UTC (permalink / raw)
  To: Ivan Khoronzhuk
  Cc: linux-kernel, dmidecode-devel, Grant Likely, Leif Lindholm,
	Matt Fleming, x86

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
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] firmware: dmi_scan: add symbol to get SMBIOS entry area
  2015-01-26  8:44   ` Ard Biesheuvel
@ 2015-01-26 11:53     ` Ivan Khoronzhuk
  0 siblings, 0 replies; 5+ messages in thread
From: Ivan Khoronzhuk @ 2015-01-26 11:53 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-kernel, dmidecode-devel, Grant Likely, Leif Lindholm,
	Matt Fleming, x86


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
>>


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-01-26 11:53 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-23 20:21 [PATCH 0/2] firmware: dmi-sysfs: add SMBIOS Ivan Khoronzhuk
2015-01-23 20:21 ` [PATCH 1/2] firmware: dmi_scan: add symbol to get SMBIOS entry area Ivan Khoronzhuk
2015-01-26  8:44   ` Ard Biesheuvel
2015-01-26 11:53     ` Ivan Khoronzhuk
2015-01-23 20:21 ` [PATCH 2/2] firmware: dmi-sysfs: add SMBIOS entry point area attribute Ivan Khoronzhuk

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).