[Patch v3] firmware: dmi-sysfs: add SMBIOS entry point area raw attribute

[Patch v3] firmware: dmi-sysfs: add SMBIOS entry point area raw attribute

[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 adds SMBIOS area to
dmi-sysfs in order to allow utils in question to work correctly with
dmi sysfs interface.

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---

v1: https://lkml.org/lkml/2015/1/23/643
v2: https://lkml.org/lkml/2015/1/26/345

v3..v2:
  firmware: dmi_scan: add symbol to get SMBIOS entry area
  firmware: dmi-sysfs: add SMBIOS entry point area attribute
	combined in one patch
	added SMBIOS information to ABI sysfs-dmi documentaton

v2..v1:
  firmware: dmi_scan: add symbol to get SMBIOS entry area
	- used additional static var to hold SMBIOS raw table size
	- changed format of get_smbios_entry_area symbol
	  returned pointer on const smbios table

  firmware: dmi-sysfs: add SMBIOS entry point area attribute
	- adopted to updated get_smbios_entry_area symbol
  	- removed redundant array to save smbios table


 Documentation/ABI/testing/sysfs-firmware-dmi | 10 +++++++
 drivers/firmware/dmi-sysfs.c                 | 42 ++++++++++++++++++++++++++++
 drivers/firmware/dmi_scan.c                  | 26 +++++++++++++++++
 include/linux/dmi.h                          |  3 ++
 4 files changed, 81 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-firmware-dmi b/Documentation/ABI/testing/sysfs-firmware-dmi
index c78f9ab..3a9ffe8 100644
--- a/Documentation/ABI/testing/sysfs-firmware-dmi
+++ b/Documentation/ABI/testing/sysfs-firmware-dmi
@@ -12,6 +12,16 @@ Description:
 		cannot ensure that the data as exported to userland is
 		without error either.
 
+		The firmware provides DMI structures as a packed list of
+		data referenced by a SMBIOS table entry point. The SMBIOS
+		entry point contains general information, like SMBIOS
+		version, DMI table size, etc. The structure, content and
+		size of SMBIOS entry point is dependent on SMBIOS version.
+		That's why SMBIOS entry point is represented in dmi sysfs
+		like a raw attribute and is accessible via
+		/sys/firmware/dmi/smbios_raw_header. The format of SMBIOS
+		entry point header can be read in SMBIOS specification.
+
 		DMI is structured as a large table of entries, where
 		each entry has a common header indicating the type and
 		length of the entry, as well as a firmware-provided
diff --git a/drivers/firmware/dmi-sysfs.c b/drivers/firmware/dmi-sysfs.c
index e0f1cb3..61b6a38 100644
--- a/drivers/firmware/dmi-sysfs.c
+++ b/drivers/firmware/dmi-sysfs.c
@@ -29,6 +29,8 @@
 #define MAX_ENTRY_TYPE 255 /* Most of these aren't used, but we consider
 			      the top entry type is only 8 bits */
 
+static const u8 *smbios_raw_header;
+
 struct dmi_sysfs_entry {
 	struct dmi_header dh;
 	struct kobject kobj;
@@ -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_raw_area_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;
 	}
 
+	smbios_raw_header = dmi_get_smbios_entry_area(&size);
+	if (!smbios_raw_header) {
+		pr_debug("dmi-sysfs: SMBIOS raw data is not available.\n");
+		error = -ENODATA;
+		goto err;
+	}
+
+	/* Create the raw binary file to access the entry area */
+	smbios_raw_area_attr.size = size;
+	if (sysfs_create_bin_file(dmi_kobj, &smbios_raw_area_attr))
+		goto err;
+
 	pr_debug("dmi-sysfs: loaded.\n");
 
 	return 0;
diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
index 420c8d8..d94c6b7 100644
--- a/drivers/firmware/dmi_scan.c
+++ b/drivers/firmware/dmi_scan.c
@@ -113,6 +113,8 @@ static void dmi_table(u8 *buf, int len, int num,
 	}
 }
 
+static unsigned char smbios_header[32];
+static int smbios_header_size;
 static phys_addr_t dmi_base;
 static u16 dmi_len;
 static u16 dmi_num;
@@ -474,6 +476,8 @@ 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);
+		smbios_header_size = buf[5];
+		memcpy(smbios_header, buf, smbios_header_size);
 
 		/* Some BIOS report weird SMBIOS version, fix that up */
 		switch (smbios_ver) {
@@ -505,6 +509,8 @@ static int __init dmi_present(const u8 *buf)
 				pr_info("SMBIOS %d.%d present.\n",
 				       dmi_ver >> 8, dmi_ver & 0xFF);
 			} else {
+				smbios_header_size = 15;
+				memcpy(smbios_header, buf, smbios_header_size);
 				dmi_ver = (buf[14] & 0xF0) << 4 |
 					   (buf[14] & 0x0F);
 				pr_info("Legacy DMI %d.%d present.\n",
@@ -531,6 +537,8 @@ 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);
+		smbios_header_size = buf[6];
+		memcpy(smbios_header, buf, smbios_header_size);
 
 		/*
 		 * The 64-bit SMBIOS 3.0 entry point no longer has a field
@@ -944,3 +952,21 @@ 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.
+ * @size - pointer to assign actual size of SMBIOS entry point area.
+ *
+ * returns NULL if table is not available, otherwise returns pointer on
+ * SMBIOS entry point area array.
+ */
+const u8 *dmi_get_smbios_entry_area(int *size)
+{
+	if (!smbios_header_size || !dmi_available)
+		return NULL;
+
+	*size = smbios_header_size;
+
+	return smbios_header;
+}
+EXPORT_SYMBOL_GPL(dmi_get_smbios_entry_area);
diff --git a/include/linux/dmi.h b/include/linux/dmi.h
index f820f0a..8e1a28d 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);
+const u8 *dmi_get_smbios_entry_area(int *size);
 
 #else
 
@@ -140,6 +141,8 @@ 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 const u8 *dmi_get_smbios_entry_area(int *size)
+	{ return NULL; }
 
 #endif
 
-- 
1.9.1

Re: [Patch v3] firmware: dmi-sysfs: add SMBIOS entry point area raw attribute

[Ivan Khoronzhuk]

+ linux-api@vger.kernel.org
+ linux-doc@vger.kernel.org

On 01/28/2015 02:39 PM, Ivan Khoronzhuk wrote:
> 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 adds SMBIOS area to
> dmi-sysfs in order to allow utils in question to work correctly with
> dmi sysfs interface.
>
> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> ---
>
> v1: https://lkml.org/lkml/2015/1/23/643
> v2: https://lkml.org/lkml/2015/1/26/345
>
> v3..v2:
>    firmware: dmi_scan: add symbol to get SMBIOS entry area
>    firmware: dmi-sysfs: add SMBIOS entry point area attribute
> 	combined in one patch
> 	added SMBIOS information to ABI sysfs-dmi documentaton
>
> v2..v1:
>    firmware: dmi_scan: add symbol to get SMBIOS entry area
> 	- used additional static var to hold SMBIOS raw table size
> 	- changed format of get_smbios_entry_area symbol
> 	  returned pointer on const smbios table
>
>    firmware: dmi-sysfs: add SMBIOS entry point area attribute
> 	- adopted to updated get_smbios_entry_area symbol
>    	- removed redundant array to save smbios table
>
>
>   Documentation/ABI/testing/sysfs-firmware-dmi | 10 +++++++
>   drivers/firmware/dmi-sysfs.c                 | 42 ++++++++++++++++++++++++++++
>   drivers/firmware/dmi_scan.c                  | 26 +++++++++++++++++
>   include/linux/dmi.h                          |  3 ++
>   4 files changed, 81 insertions(+)
>
> diff --git a/Documentation/ABI/testing/sysfs-firmware-dmi b/Documentation/ABI/testing/sysfs-firmware-dmi
> index c78f9ab..3a9ffe8 100644
> --- a/Documentation/ABI/testing/sysfs-firmware-dmi
> +++ b/Documentation/ABI/testing/sysfs-firmware-dmi
> @@ -12,6 +12,16 @@ Description:
>   		cannot ensure that the data as exported to userland is
>   		without error either.
>   
> +		The firmware provides DMI structures as a packed list of
> +		data referenced by a SMBIOS table entry point. The SMBIOS
> +		entry point contains general information, like SMBIOS
> +		version, DMI table size, etc. The structure, content and
> +		size of SMBIOS entry point is dependent on SMBIOS version.
> +		That's why SMBIOS entry point is represented in dmi sysfs
> +		like a raw attribute and is accessible via
> +		/sys/firmware/dmi/smbios_raw_header. The format of SMBIOS
> +		entry point header can be read in SMBIOS specification.
> +
>   		DMI is structured as a large table of entries, where
>   		each entry has a common header indicating the type and
>   		length of the entry, as well as a firmware-provided
> diff --git a/drivers/firmware/dmi-sysfs.c b/drivers/firmware/dmi-sysfs.c
> index e0f1cb3..61b6a38 100644
> --- a/drivers/firmware/dmi-sysfs.c
> +++ b/drivers/firmware/dmi-sysfs.c
> @@ -29,6 +29,8 @@
>   #define MAX_ENTRY_TYPE 255 /* Most of these aren't used, but we consider
>   			      the top entry type is only 8 bits */
>   
> +static const u8 *smbios_raw_header;
> +
>   struct dmi_sysfs_entry {
>   	struct dmi_header dh;
>   	struct kobject kobj;
> @@ -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_raw_area_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;
>   	}
>   
> +	smbios_raw_header = dmi_get_smbios_entry_area(&size);
> +	if (!smbios_raw_header) {
> +		pr_debug("dmi-sysfs: SMBIOS raw data is not available.\n");
> +		error = -ENODATA;
> +		goto err;
> +	}
> +
> +	/* Create the raw binary file to access the entry area */
> +	smbios_raw_area_attr.size = size;
> +	if (sysfs_create_bin_file(dmi_kobj, &smbios_raw_area_attr))
> +		goto err;
> +
>   	pr_debug("dmi-sysfs: loaded.\n");
>   
>   	return 0;
> diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
> index 420c8d8..d94c6b7 100644
> --- a/drivers/firmware/dmi_scan.c
> +++ b/drivers/firmware/dmi_scan.c
> @@ -113,6 +113,8 @@ static void dmi_table(u8 *buf, int len, int num,
>   	}
>   }
>   
> +static unsigned char smbios_header[32];
> +static int smbios_header_size;
>   static phys_addr_t dmi_base;
>   static u16 dmi_len;
>   static u16 dmi_num;
> @@ -474,6 +476,8 @@ 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);
> +		smbios_header_size = buf[5];
> +		memcpy(smbios_header, buf, smbios_header_size);
>   
>   		/* Some BIOS report weird SMBIOS version, fix that up */
>   		switch (smbios_ver) {
> @@ -505,6 +509,8 @@ static int __init dmi_present(const u8 *buf)
>   				pr_info("SMBIOS %d.%d present.\n",
>   				       dmi_ver >> 8, dmi_ver & 0xFF);
>   			} else {
> +				smbios_header_size = 15;
> +				memcpy(smbios_header, buf, smbios_header_size);
>   				dmi_ver = (buf[14] & 0xF0) << 4 |
>   					   (buf[14] & 0x0F);
>   				pr_info("Legacy DMI %d.%d present.\n",
> @@ -531,6 +537,8 @@ 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);
> +		smbios_header_size = buf[6];
> +		memcpy(smbios_header, buf, smbios_header_size);
>   
>   		/*
>   		 * The 64-bit SMBIOS 3.0 entry point no longer has a field
> @@ -944,3 +952,21 @@ 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.
> + * @size - pointer to assign actual size of SMBIOS entry point area.
> + *
> + * returns NULL if table is not available, otherwise returns pointer on
> + * SMBIOS entry point area array.
> + */
> +const u8 *dmi_get_smbios_entry_area(int *size)
> +{
> +	if (!smbios_header_size || !dmi_available)
> +		return NULL;
> +
> +	*size = smbios_header_size;
> +
> +	return smbios_header;
> +}
> +EXPORT_SYMBOL_GPL(dmi_get_smbios_entry_area);
> diff --git a/include/linux/dmi.h b/include/linux/dmi.h
> index f820f0a..8e1a28d 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);
> +const u8 *dmi_get_smbios_entry_area(int *size);
>   
>   #else
>   
> @@ -140,6 +141,8 @@ 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 const u8 *dmi_get_smbios_entry_area(int *size)
> +	{ return NULL; }
>   
>   #endif
>   

Re: [Patch v3] firmware: dmi-sysfs: add SMBIOS entry point area raw attribute

[Matt Fleming]

On Wed, 28 Jan, at 05:56:25PM, Ivan Khoronzhuk wrote:
> >diff --git a/drivers/firmware/dmi-sysfs.c b/drivers/firmware/dmi-sysfs.c
> >index e0f1cb3..61b6a38 100644
> >--- a/drivers/firmware/dmi-sysfs.c
> >+++ b/drivers/firmware/dmi-sysfs.c
> >@@ -29,6 +29,8 @@
> >  #define MAX_ENTRY_TYPE 255 /* Most of these aren't used, but we consider
> >  			      the top entry type is only 8 bits */
> >+static const u8 *smbios_raw_header;

There appears to be a mixture of u8 and unsigned char going on here, cf.
'smbios_header'.

While I'm pretty sure all architectures typedef them to be equivalent,
semantically, as a reviewer this makes me think there are type issues.

Is there any way to use one data type for the SMBIOS header?

> >@@ -669,6 +699,18 @@ static int __init dmi_sysfs_init(void)
> >  		goto err;
> >  	}
> >+	smbios_raw_header = dmi_get_smbios_entry_area(&size);
> >+	if (!smbios_raw_header) {
> >+		pr_debug("dmi-sysfs: SMBIOS raw data is not available.\n");
> >+		error = -ENODATA;
> >+		goto err;

Perhaps this should be -EINVAL? -ENODATA implies that if you try again
in the future data might be available, i.e. it's a temporary failure.
That's not the case here since the header is invalid.

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [Patch v3] firmware: dmi-sysfs: add SMBIOS entry point area raw attribute

[Ivan Khoronzhuk]

On 02/03/2015 12:49 PM, Matt Fleming wrote:
> On Wed, 28 Jan, at 05:56:25PM, Ivan Khoronzhuk wrote:
>>> diff --git a/drivers/firmware/dmi-sysfs.c b/drivers/firmware/dmi-sysfs.c
>>> index e0f1cb3..61b6a38 100644
>>> --- a/drivers/firmware/dmi-sysfs.c
>>> +++ b/drivers/firmware/dmi-sysfs.c
>>> @@ -29,6 +29,8 @@
>>>   #define MAX_ENTRY_TYPE 255 /* Most of these aren't used, but we consider
>>>   			      the top entry type is only 8 bits */
>>> +static const u8 *smbios_raw_header;
> There appears to be a mixture of u8 and unsigned char going on here, cf.
> 'smbios_header'.
>
> While I'm pretty sure all architectures typedef them to be equivalent,
> semantically, as a reviewer this makes me think there are type issues.
>
> Is there any way to use one data type for the SMBIOS header?

Let it be u8 in both cases.

>>> @@ -669,6 +699,18 @@ static int __init dmi_sysfs_init(void)
>>>   		goto err;
>>>   	}
>>> +	smbios_raw_header = dmi_get_smbios_entry_area(&size);
>>> +	if (!smbios_raw_header) {
>>> +		pr_debug("dmi-sysfs: SMBIOS raw data is not available.\n");
>>> +		error = -ENODATA;
>>> +		goto err;
> Perhaps this should be -EINVAL? -ENODATA implies that if you try again
> in the future data might be available, i.e. it's a temporary failure.
> That's not the case here since the header is invalid.
>

Yes, -EINVAL is better.
I'll send new patch soon.
Thanks!

Re: [Patch v3] firmware: dmi-sysfs: add SMBIOS entry point area raw attribute

[Mark Salter]

On Wed, 2015-01-28 at 14:39 +0200, Ivan Khoronzhuk wrote:
> 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 adds SMBIOS area to
> dmi-sysfs in order to allow utils in question to work correctly with
> dmi sysfs interface.
> 
> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>

Sorry for coming in late, here. Why expose the raw header
instead of exposing the pieces as individual files like
the kernel does for the other dmi info? That way the kernel
decodes the header and presents it in an easy to read
format for dmidecode or even a shell script.

> ---
> 
> v1: https://lkml.org/lkml/2015/1/23/643
> v2: https://lkml.org/lkml/2015/1/26/345
> 
> v3..v2:
>   firmware: dmi_scan: add symbol to get SMBIOS entry area
>   firmware: dmi-sysfs: add SMBIOS entry point area attribute
> 	combined in one patch
> 	added SMBIOS information to ABI sysfs-dmi documentaton
> 
> v2..v1:
>   firmware: dmi_scan: add symbol to get SMBIOS entry area
> 	- used additional static var to hold SMBIOS raw table size
> 	- changed format of get_smbios_entry_area symbol
> 	  returned pointer on const smbios table
> 
>   firmware: dmi-sysfs: add SMBIOS entry point area attribute
> 	- adopted to updated get_smbios_entry_area symbol
>   	- removed redundant array to save smbios table
> 
> 
>  Documentation/ABI/testing/sysfs-firmware-dmi | 10 +++++++
>  drivers/firmware/dmi-sysfs.c                 | 42 ++++++++++++++++++++++++++++
>  drivers/firmware/dmi_scan.c                  | 26 +++++++++++++++++
>  include/linux/dmi.h                          |  3 ++
>  4 files changed, 81 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-firmware-dmi b/Documentation/ABI/testing/sysfs-firmware-dmi
> index c78f9ab..3a9ffe8 100644
> --- a/Documentation/ABI/testing/sysfs-firmware-dmi
> +++ b/Documentation/ABI/testing/sysfs-firmware-dmi
> @@ -12,6 +12,16 @@ Description:
>  		cannot ensure that the data as exported to userland is
>  		without error either.
>  
> +		The firmware provides DMI structures as a packed list of
> +		data referenced by a SMBIOS table entry point. The SMBIOS
> +		entry point contains general information, like SMBIOS
> +		version, DMI table size, etc. The structure, content and
> +		size of SMBIOS entry point is dependent on SMBIOS version.
> +		That's why SMBIOS entry point is represented in dmi sysfs
> +		like a raw attribute and is accessible via
> +		/sys/firmware/dmi/smbios_raw_header. The format of SMBIOS
> +		entry point header can be read in SMBIOS specification.
> +
>  		DMI is structured as a large table of entries, where
>  		each entry has a common header indicating the type and
>  		length of the entry, as well as a firmware-provided
> diff --git a/drivers/firmware/dmi-sysfs.c b/drivers/firmware/dmi-sysfs.c
> index e0f1cb3..61b6a38 100644
> --- a/drivers/firmware/dmi-sysfs.c
> +++ b/drivers/firmware/dmi-sysfs.c
> @@ -29,6 +29,8 @@
>  #define MAX_ENTRY_TYPE 255 /* Most of these aren't used, but we consider
>  			      the top entry type is only 8 bits */
>  
> +static const u8 *smbios_raw_header;
> +
>  struct dmi_sysfs_entry {
>  	struct dmi_header dh;
>  	struct kobject kobj;
> @@ -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_raw_area_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;
>  	}
>  
> +	smbios_raw_header = dmi_get_smbios_entry_area(&size);
> +	if (!smbios_raw_header) {
> +		pr_debug("dmi-sysfs: SMBIOS raw data is not available.\n");
> +		error = -ENODATA;
> +		goto err;
> +	}
> +
> +	/* Create the raw binary file to access the entry area */
> +	smbios_raw_area_attr.size = size;
> +	if (sysfs_create_bin_file(dmi_kobj, &smbios_raw_area_attr))
> +		goto err;
> +
>  	pr_debug("dmi-sysfs: loaded.\n");
>  
>  	return 0;
> diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
> index 420c8d8..d94c6b7 100644
> --- a/drivers/firmware/dmi_scan.c
> +++ b/drivers/firmware/dmi_scan.c
> @@ -113,6 +113,8 @@ static void dmi_table(u8 *buf, int len, int num,
>  	}
>  }
>  
> +static unsigned char smbios_header[32];
> +static int smbios_header_size;
>  static phys_addr_t dmi_base;
>  static u16 dmi_len;
>  static u16 dmi_num;
> @@ -474,6 +476,8 @@ 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);
> +		smbios_header_size = buf[5];
> +		memcpy(smbios_header, buf, smbios_header_size);
>  
>  		/* Some BIOS report weird SMBIOS version, fix that up */
>  		switch (smbios_ver) {
> @@ -505,6 +509,8 @@ static int __init dmi_present(const u8 *buf)
>  				pr_info("SMBIOS %d.%d present.\n",
>  				       dmi_ver >> 8, dmi_ver & 0xFF);
>  			} else {
> +				smbios_header_size = 15;
> +				memcpy(smbios_header, buf, smbios_header_size);
>  				dmi_ver = (buf[14] & 0xF0) << 4 |
>  					   (buf[14] & 0x0F);
>  				pr_info("Legacy DMI %d.%d present.\n",
> @@ -531,6 +537,8 @@ 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);
> +		smbios_header_size = buf[6];
> +		memcpy(smbios_header, buf, smbios_header_size);
>  
>  		/*
>  		 * The 64-bit SMBIOS 3.0 entry point no longer has a field
> @@ -944,3 +952,21 @@ 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.
> + * @size - pointer to assign actual size of SMBIOS entry point area.
> + *
> + * returns NULL if table is not available, otherwise returns pointer on
> + * SMBIOS entry point area array.
> + */
> +const u8 *dmi_get_smbios_entry_area(int *size)
> +{
> +	if (!smbios_header_size || !dmi_available)
> +		return NULL;
> +
> +	*size = smbios_header_size;
> +
> +	return smbios_header;
> +}
> +EXPORT_SYMBOL_GPL(dmi_get_smbios_entry_area);
> diff --git a/include/linux/dmi.h b/include/linux/dmi.h
> index f820f0a..8e1a28d 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);
> +const u8 *dmi_get_smbios_entry_area(int *size);
>  
>  #else
>  
> @@ -140,6 +141,8 @@ 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 const u8 *dmi_get_smbios_entry_area(int *size)
> +	{ return NULL; }
>  
>  #endif
>  

Re: [Patch v3] firmware: dmi-sysfs: add SMBIOS entry point area raw attribute

[Ivan Khoronzhuk]

Hi, Mark

On 02/03/2015 04:58 PM, Mark Salter wrote:
> On Wed, 2015-01-28 at 14:39 +0200, Ivan Khoronzhuk wrote:
>> 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 adds SMBIOS area to
>> dmi-sysfs in order to allow utils in question to work correctly with
>> dmi sysfs interface.
>>
>> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> Sorry for coming in late, here. Why expose the raw header
> instead of exposing the pieces as individual files like
> the kernel does for the other dmi info? That way the kernel
> decodes the header and presents it in an easy to read
> format for dmidecode or even a shell script.

The SMBIOS entry point can contain specific fields depending on it's
version. In the specification I didn't find any rules concerning this.

Only field that probably will be available is version number, but
the version number is not only var that can be required by utils.
For example, dmidecode needs also print some additional info like
phys address where dmitable is placed.

I don't sure how exactly next SMBIOS version will be changed.
It can happen that some new data is available...and some old is removed.
It's better to export it as raw data like it was done for dmi entries
via raw attribute and It's better to pass the whole entry table
instead of each time modify the dmi sysfs interface when new SMBIOS
version is issued.


>
>> ---
>>
>> v1: https://lkml.org/lkml/2015/1/23/643
>> v2: https://lkml.org/lkml/2015/1/26/345
>>
>> v3..v2:
>>    firmware: dmi_scan: add symbol to get SMBIOS entry area
>>    firmware: dmi-sysfs: add SMBIOS entry point area attribute
>> 	combined in one patch
>> 	added SMBIOS information to ABI sysfs-dmi documentaton
>>
>> v2..v1:
>>    firmware: dmi_scan: add symbol to get SMBIOS entry area
>> 	- used additional static var to hold SMBIOS raw table size
>> 	- changed format of get_smbios_entry_area symbol
>> 	  returned pointer on const smbios table
>>
>>    firmware: dmi-sysfs: add SMBIOS entry point area attribute
>> 	- adopted to updated get_smbios_entry_area symbol
>>    	- removed redundant array to save smbios table
>>
>>
>>   Documentation/ABI/testing/sysfs-firmware-dmi | 10 +++++++
>>   drivers/firmware/dmi-sysfs.c                 | 42 ++++++++++++++++++++++++++++
>>   drivers/firmware/dmi_scan.c                  | 26 +++++++++++++++++
>>   include/linux/dmi.h                          |  3 ++
>>   4 files changed, 81 insertions(+)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-firmware-dmi b/Documentation/ABI/testing/sysfs-firmware-dmi
>> index c78f9ab..3a9ffe8 100644
>> --- a/Documentation/ABI/testing/sysfs-firmware-dmi
>> +++ b/Documentation/ABI/testing/sysfs-firmware-dmi
>> @@ -12,6 +12,16 @@ Description:
>>   		cannot ensure that the data as exported to userland is
>>   		without error either.
>>   
>> +		The firmware provides DMI structures as a packed list of
>> +		data referenced by a SMBIOS table entry point. The SMBIOS
>> +		entry point contains general information, like SMBIOS
>> +		version, DMI table size, etc. The structure, content and
>> +		size of SMBIOS entry point is dependent on SMBIOS version.
>> +		That's why SMBIOS entry point is represented in dmi sysfs
>> +		like a raw attribute and is accessible via
>> +		/sys/firmware/dmi/smbios_raw_header. The format of SMBIOS
>> +		entry point header can be read in SMBIOS specification.
>> +
>>   		DMI is structured as a large table of entries, where
>>   		each entry has a common header indicating the type and
>>   		length of the entry, as well as a firmware-provided
>> diff --git a/drivers/firmware/dmi-sysfs.c b/drivers/firmware/dmi-sysfs.c
>> index e0f1cb3..61b6a38 100644
>> --- a/drivers/firmware/dmi-sysfs.c
>> +++ b/drivers/firmware/dmi-sysfs.c
>> @@ -29,6 +29,8 @@
>>   #define MAX_ENTRY_TYPE 255 /* Most of these aren't used, but we consider
>>   			      the top entry type is only 8 bits */
>>   
>> +static const u8 *smbios_raw_header;
>> +
>>   struct dmi_sysfs_entry {
>>   	struct dmi_header dh;
>>   	struct kobject kobj;
>> @@ -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_raw_area_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;
>>   	}
>>   
>> +	smbios_raw_header = dmi_get_smbios_entry_area(&size);
>> +	if (!smbios_raw_header) {
>> +		pr_debug("dmi-sysfs: SMBIOS raw data is not available.\n");
>> +		error = -ENODATA;
>> +		goto err;
>> +	}
>> +
>> +	/* Create the raw binary file to access the entry area */
>> +	smbios_raw_area_attr.size = size;
>> +	if (sysfs_create_bin_file(dmi_kobj, &smbios_raw_area_attr))
>> +		goto err;
>> +
>>   	pr_debug("dmi-sysfs: loaded.\n");
>>   
>>   	return 0;
>> diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
>> index 420c8d8..d94c6b7 100644
>> --- a/drivers/firmware/dmi_scan.c
>> +++ b/drivers/firmware/dmi_scan.c
>> @@ -113,6 +113,8 @@ static void dmi_table(u8 *buf, int len, int num,
>>   	}
>>   }
>>   
>> +static unsigned char smbios_header[32];
>> +static int smbios_header_size;
>>   static phys_addr_t dmi_base;
>>   static u16 dmi_len;
>>   static u16 dmi_num;
>> @@ -474,6 +476,8 @@ 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);
>> +		smbios_header_size = buf[5];
>> +		memcpy(smbios_header, buf, smbios_header_size);
>>   
>>   		/* Some BIOS report weird SMBIOS version, fix that up */
>>   		switch (smbios_ver) {
>> @@ -505,6 +509,8 @@ static int __init dmi_present(const u8 *buf)
>>   				pr_info("SMBIOS %d.%d present.\n",
>>   				       dmi_ver >> 8, dmi_ver & 0xFF);
>>   			} else {
>> +				smbios_header_size = 15;
>> +				memcpy(smbios_header, buf, smbios_header_size);
>>   				dmi_ver = (buf[14] & 0xF0) << 4 |
>>   					   (buf[14] & 0x0F);
>>   				pr_info("Legacy DMI %d.%d present.\n",
>> @@ -531,6 +537,8 @@ 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);
>> +		smbios_header_size = buf[6];
>> +		memcpy(smbios_header, buf, smbios_header_size);
>>   
>>   		/*
>>   		 * The 64-bit SMBIOS 3.0 entry point no longer has a field
>> @@ -944,3 +952,21 @@ 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.
>> + * @size - pointer to assign actual size of SMBIOS entry point area.
>> + *
>> + * returns NULL if table is not available, otherwise returns pointer on
>> + * SMBIOS entry point area array.
>> + */
>> +const u8 *dmi_get_smbios_entry_area(int *size)
>> +{
>> +	if (!smbios_header_size || !dmi_available)
>> +		return NULL;
>> +
>> +	*size = smbios_header_size;
>> +
>> +	return smbios_header;
>> +}
>> +EXPORT_SYMBOL_GPL(dmi_get_smbios_entry_area);
>> diff --git a/include/linux/dmi.h b/include/linux/dmi.h
>> index f820f0a..8e1a28d 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);
>> +const u8 *dmi_get_smbios_entry_area(int *size);
>>   
>>   #else
>>   
>> @@ -140,6 +141,8 @@ 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 const u8 *dmi_get_smbios_entry_area(int *size)
>> +	{ return NULL; }
>>   
>>   #endif
>>   
>

Re: [dmidecode] [Patch v3] firmware: dmi-sysfs: add SMBIOS entry point area raw attribute

[Jean Delvare]

Hi Ivan, Mark and all,

Le Tuesday 03 February 2015 à 17:48 +0200, Ivan Khoronzhuk a écrit :
> On 02/03/2015 04:58 PM, Mark Salter wrote:
> > On Wed, 2015-01-28 at 14:39 +0200, Ivan Khoronzhuk wrote:
> >> 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 adds SMBIOS area to
> >> dmi-sysfs in order to allow utils in question to work correctly with
> >> dmi sysfs interface.
> >>
> >> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> > Sorry for coming in late, here. Why expose the raw header
> > instead of exposing the pieces as individual files like
> > the kernel does for the other dmi info? That way the kernel
> > decodes the header and presents it in an easy to read
> > format for dmidecode or even a shell script.

Sorry for coming in even later.

This is a common misconception that dmidecode would be happier with
pre-processed data. In fact it's exactly the opposite, dmidecode will be
much happier with raw data because it already has all the code to decode
the raw data, and that code will have to stay in place anyway as not all
systems have sysfs with the proper information exposed to avoid reading
the raw data from /dev/mem directly.

In a particular it should be noted that the
current /sys/firmware/dmi/entries interface is completely unsuited for
consumption by dmidecode. It would require a significant amount of extra
code in dmidecode to walk and parse the hundreds of files in this
directory. And there would be additional problems, such as slower
execution (500 file open/close cycles, thank you very much) and entries
not being displayed in the same order as when dmidecode reads the table
directly.

So not only Ivan is right with his idea of exposing the raw DMI/SMBIOS
entry point in sysfs, but I think we need to go even further and also
expose the raw DMI data table itself through sysfs. It should go
under /sys/firmware/dmi/tables/, much like ACPI tables live
under /sys/firmware/acpi/tables/.

I would also suggest that both the raw entry point and the raw table
data should be presented regardless of CONFIG_DMI_SYSFS. The code should
be small enough that it should be OK to include it unconditionally. Most
systems don't need the dmi-sysfs code, the use cases seem rather limited
to me, and on distributions it's generally built as a module and not
loaded by default.

> The SMBIOS entry point can contain specific fields depending on it's
> version. In the specification I didn't find any rules concerning this.
> 
> Only field that probably will be available is version number, but
> the version number is not only var that can be required by utils.
> For example, dmidecode needs also print some additional info like
> phys address where dmitable is placed.
> 
> I don't sure how exactly next SMBIOS version will be changed.
> It can happen that some new data is available...and some old is removed.
> It's better to export it as raw data like it was done for dmi entries
> via raw attribute and It's better to pass the whole entry table
> instead of each time modify the dmi sysfs interface when new SMBIOS
> version is issued.
> 
> 
> >
> >> ---
> >>
> >> v1: https://lkml.org/lkml/2015/1/23/643
> >> v2: https://lkml.org/lkml/2015/1/26/345
> >>
> >> v3..v2:

This notation is confusing, being opposite to the git syntax. Please
just write "Changes since v2".

> >>    firmware: dmi_scan: add symbol to get SMBIOS entry area
> >>    firmware: dmi-sysfs: add SMBIOS entry point area attribute
> >> 	combined in one patch
> >> 	added SMBIOS information to ABI sysfs-dmi documentaton
> >>
> >> v2..v1:
> >>    firmware: dmi_scan: add symbol to get SMBIOS entry area
> >> 	- used additional static var to hold SMBIOS raw table size
> >> 	- changed format of get_smbios_entry_area symbol
> >> 	  returned pointer on const smbios table
> >>
> >>    firmware: dmi-sysfs: add SMBIOS entry point area attribute
> >> 	- adopted to updated get_smbios_entry_area symbol
> >>    	- removed redundant array to save smbios table
> >>
> >>
> >>   Documentation/ABI/testing/sysfs-firmware-dmi | 10 +++++++
> >>   drivers/firmware/dmi-sysfs.c                 | 42 ++++++++++++++++++++++++++++
> >>   drivers/firmware/dmi_scan.c                  | 26 +++++++++++++++++
> >>   include/linux/dmi.h                          |  3 ++
> >>   4 files changed, 81 insertions(+)
> >>
> >> diff --git a/Documentation/ABI/testing/sysfs-firmware-dmi b/Documentation/ABI/testing/sysfs-firmware-dmi
> >> index c78f9ab..3a9ffe8 100644
> >> --- a/Documentation/ABI/testing/sysfs-firmware-dmi
> >> +++ b/Documentation/ABI/testing/sysfs-firmware-dmi
> >> @@ -12,6 +12,16 @@ Description:
> >>   		cannot ensure that the data as exported to userland is
> >>   		without error either.
> >>   
> >> +		The firmware provides DMI structures as a packed list of
> >> +		data referenced by a SMBIOS table entry point. The SMBIOS
> >> +		entry point contains general information, like SMBIOS
> >> +		version, DMI table size, etc. The structure, content and
> >> +		size of SMBIOS entry point is dependent on SMBIOS version.
> >> +		That's why SMBIOS entry point is represented in dmi sysfs
> >> +		like a raw attribute and is accessible via
> >> +		/sys/firmware/dmi/smbios_raw_header. The format of SMBIOS
> >> +		entry point header can be read in SMBIOS specification.
> >> +
> >>   		DMI is structured as a large table of entries, where
> >>   		each entry has a common header indicating the type and
> >>   		length of the entry, as well as a firmware-provided
> >> diff --git a/drivers/firmware/dmi-sysfs.c b/drivers/firmware/dmi-sysfs.c
> >> index e0f1cb3..61b6a38 100644
> >> --- a/drivers/firmware/dmi-sysfs.c
> >> +++ b/drivers/firmware/dmi-sysfs.c
> >> @@ -29,6 +29,8 @@
> >>   #define MAX_ENTRY_TYPE 255 /* Most of these aren't used, but we consider
> >>   			      the top entry type is only 8 bits */
> >>   
> >> +static const u8 *smbios_raw_header;

It's called an entry point in the specification and every document out
there (including your own text above), why do you want to suddenly call
it a "header"? The term "header" is used to designate something
completely different in the context of DMI/SMBIOS data so I find it
quite confusing.

Please also note that the recently released version 3.0.0 of the SMBIOS
specification introduces a new entry point format, and the firmware is
allowed to implement both the old and the new format. It may be
desirable to expose both to user-space under different names.

Thanks,
-- 
Jean Delvare
SUSE L3 Support

Re: [dmidecode] [Patch v3] firmware: dmi-sysfs: add SMBIOS entry point area raw attribute

[Jean Delvare]

Replying to myself...

On Thu, 26 Feb 2015 09:50:42 +0100, Jean Delvare wrote:
> Please also note that the recently released version 3.0.0 of the SMBIOS
> specification introduces a new entry point format, and the firmware is
> allowed to implement both the old and the new format. It may be
> desirable to expose both to user-space under different names.

Having read the kernel code meanwhile, I see we will call either
dmi_smbios3_present or dmi_present successfully, not both, so
presenting both entry points to user-space would be non-trivial. So I'm
fine with presenting only one entry point in sysfs for the time being.
We can always revisit later if it turns out that dmidecode really needs
both in some cases.

-- 
Jean Delvare
SUSE L3 Support

Re: [dmidecode] [Patch v3] firmware: dmi-sysfs: add SMBIOS entry point area raw attribute

[Ivan.khoronzhuk]

Hi Jean,
See reply at v4.

On 02/26/2015 10:50 AM, Jean Delvare wrote:
> Hi Ivan, Mark and all,
>
> Le Tuesday 03 February 2015 à 17:48 +0200, Ivan Khoronzhuk a écrit :
>> On 02/03/2015 04:58 PM, Mark Salter wrote:
>>> On Wed, 2015-01-28 at 14:39 +0200, Ivan Khoronzhuk wrote:
>>>> 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 adds SMBIOS area to
>>>> dmi-sysfs in order to allow utils in question to work correctly with
>>>> dmi sysfs interface.
>>>>
>>>> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>>> Sorry for coming in late, here. Why expose the raw header
>>> instead of exposing the pieces as individual files like
>>> the kernel does for the other dmi info? That way the kernel
>>> decodes the header and presents it in an easy to read
>>> format for dmidecode or even a shell script.
> Sorry for coming in even later.
>
> This is a common misconception that dmidecode would be happier with
> pre-processed data. In fact it's exactly the opposite, dmidecode will be
> much happier with raw data because it already has all the code to decode
> the raw data, and that code will have to stay in place anyway as not all
> systems have sysfs with the proper information exposed to avoid reading
> the raw data from /dev/mem directly.
>
> In a particular it should be noted that the
> current /sys/firmware/dmi/entries interface is completely unsuited for
> consumption by dmidecode. It would require a significant amount of extra
> code in dmidecode to walk and parse the hundreds of files in this
> directory. And there would be additional problems, such as slower
> execution (500 file open/close cycles, thank you very much) and entries
> not being displayed in the same order as when dmidecode reads the table
> directly.
>
> So not only Ivan is right with his idea of exposing the raw DMI/SMBIOS
> entry point in sysfs, but I think we need to go even further and also
> expose the raw DMI data table itself through sysfs. It should go
> under /sys/firmware/dmi/tables/, much like ACPI tables live
> under /sys/firmware/acpi/tables/.
>
> I would also suggest that both the raw entry point and the raw table
> data should be presented regardless of CONFIG_DMI_SYSFS. The code should
> be small enough that it should be OK to include it unconditionally. Most
> systems don't need the dmi-sysfs code, the use cases seem rather limited
> to me, and on distributions it's generally built as a module and not
> loaded by default.
>
>> The SMBIOS entry point can contain specific fields depending on it's
>> version. In the specification I didn't find any rules concerning this.
>>
>> Only field that probably will be available is version number, but
>> the version number is not only var that can be required by utils.
>> For example, dmidecode needs also print some additional info like
>> phys address where dmitable is placed.
>>
>> I don't sure how exactly next SMBIOS version will be changed.
>> It can happen that some new data is available...and some old is removed.
>> It's better to export it as raw data like it was done for dmi entries
>> via raw attribute and It's better to pass the whole entry table
>> instead of each time modify the dmi sysfs interface when new SMBIOS
>> version is issued.
>>
>>
>>>> ---
>>>>
>>>> v1: https://lkml.org/lkml/2015/1/23/643
>>>> v2: https://lkml.org/lkml/2015/1/26/345
>>>>
>>>> v3..v2:
> This notation is confusing, being opposite to the git syntax. Please
> just write "Changes since v2".
>
>>>>     firmware: dmi_scan: add symbol to get SMBIOS entry area
>>>>     firmware: dmi-sysfs: add SMBIOS entry point area attribute
>>>> 	combined in one patch
>>>> 	added SMBIOS information to ABI sysfs-dmi documentaton
>>>>
>>>> v2..v1:
>>>>     firmware: dmi_scan: add symbol to get SMBIOS entry area
>>>> 	- used additional static var to hold SMBIOS raw table size
>>>> 	- changed format of get_smbios_entry_area symbol
>>>> 	  returned pointer on const smbios table
>>>>
>>>>     firmware: dmi-sysfs: add SMBIOS entry point area attribute
>>>> 	- adopted to updated get_smbios_entry_area symbol
>>>>     	- removed redundant array to save smbios table
>>>>
>>>>
>>>>    Documentation/ABI/testing/sysfs-firmware-dmi | 10 +++++++
>>>>    drivers/firmware/dmi-sysfs.c                 | 42 ++++++++++++++++++++++++++++
>>>>    drivers/firmware/dmi_scan.c                  | 26 +++++++++++++++++
>>>>    include/linux/dmi.h                          |  3 ++
>>>>    4 files changed, 81 insertions(+)
>>>>
>>>> diff --git a/Documentation/ABI/testing/sysfs-firmware-dmi b/Documentation/ABI/testing/sysfs-firmware-dmi
>>>> index c78f9ab..3a9ffe8 100644
>>>> --- a/Documentation/ABI/testing/sysfs-firmware-dmi
>>>> +++ b/Documentation/ABI/testing/sysfs-firmware-dmi
>>>> @@ -12,6 +12,16 @@ Description:
>>>>    		cannot ensure that the data as exported to userland is
>>>>    		without error either.
>>>>    
>>>> +		The firmware provides DMI structures as a packed list of
>>>> +		data referenced by a SMBIOS table entry point. The SMBIOS
>>>> +		entry point contains general information, like SMBIOS
>>>> +		version, DMI table size, etc. The structure, content and
>>>> +		size of SMBIOS entry point is dependent on SMBIOS version.
>>>> +		That's why SMBIOS entry point is represented in dmi sysfs
>>>> +		like a raw attribute and is accessible via
>>>> +		/sys/firmware/dmi/smbios_raw_header. The format of SMBIOS
>>>> +		entry point header can be read in SMBIOS specification.
>>>> +
>>>>    		DMI is structured as a large table of entries, where
>>>>    		each entry has a common header indicating the type and
>>>>    		length of the entry, as well as a firmware-provided
>>>> diff --git a/drivers/firmware/dmi-sysfs.c b/drivers/firmware/dmi-sysfs.c
>>>> index e0f1cb3..61b6a38 100644
>>>> --- a/drivers/firmware/dmi-sysfs.c
>>>> +++ b/drivers/firmware/dmi-sysfs.c
>>>> @@ -29,6 +29,8 @@
>>>>    #define MAX_ENTRY_TYPE 255 /* Most of these aren't used, but we consider
>>>>    			      the top entry type is only 8 bits */
>>>>    
>>>> +static const u8 *smbios_raw_header;
> It's called an entry point in the specification and every document out
> there (including your own text above), why do you want to suddenly call
> it a "header"? The term "header" is used to designate something
> completely different in the context of DMI/SMBIOS data so I find it
> quite confusing.
>
> Please also note that the recently released version 3.0.0 of the SMBIOS
> specification introduces a new entry point format, and the firmware is
> allowed to implement both the old and the new format. It may be
> desirable to expose both to user-space under different names.
>
> Thanks,

-- 
Regards,
Ivan Khoronzhuk

Re: [dmidecode] [Patch v3] firmware: dmi-sysfs: add SMBIOS entry point area raw attribute

[Ivan.khoronzhuk]

Hi Jean.

On 02/26/2015 11:41 AM, Jean Delvare wrote:
> Replying to myself...
>
> On Thu, 26 Feb 2015 09:50:42 +0100, Jean Delvare wrote:
>> Please also note that the recently released version 3.0.0 of the SMBIOS
>> specification introduces a new entry point format, and the firmware is
>> allowed to implement both the old and the new format. It may be
>> desirable to expose both to user-space under different names.
> Having read the kernel code meanwhile, I see we will call either
> dmi_smbios3_present or dmi_present successfully, not both, so
> presenting both entry points to user-space would be non-trivial. So I'm
> fine with presenting only one entry point in sysfs for the time being.
> We can always revisit later if it turns out that dmidecode really needs
> both in some cases.
>

Why do you need two tables ? If kernel can parse the best one why
it should present the old one? The old is subset of new, so the new must
contain all required information, only format can be slightly different.
If dmidecode has problems in reading new version then dmidecode should
be modified, not kernel.

-- 
Regards,
Ivan Khoronzhuk

Re: [dmidecode] [Patch v3] firmware: dmi-sysfs: add SMBIOS entry point area raw attribute

[Jean Delvare]

Hi Ivan,

On Wed, 04 Mar 2015 14:28:20 +0200, Ivan.khoronzhuk wrote:
> On 02/26/2015 11:41 AM, Jean Delvare wrote:
> > On Thu, 26 Feb 2015 09:50:42 +0100, Jean Delvare wrote:
> >> Please also note that the recently released version 3.0.0 of the SMBIOS
> >> specification introduces a new entry point format, and the firmware is
> >> allowed to implement both the old and the new format. It may be
> >> desirable to expose both to user-space under different names.
> >
> > Having read the kernel code meanwhile, I see we will call either
> > dmi_smbios3_present or dmi_present successfully, not both, so
> > presenting both entry points to user-space would be non-trivial. So I'm
> > fine with presenting only one entry point in sysfs for the time being.
> > We can always revisit later if it turns out that dmidecode really needs
> > both in some cases.
> 
> Why do you need two tables ? If kernel can parse the best one why
> it should present the old one? The old is subset of new, so the new must
> contain all required information, only format can be slightly different.
> If dmidecode has problems in reading new version then dmidecode should
> be modified, not kernel.

It's not just two tables (I don't expect a lot of BIOSes to provide two
tables in practice, and they would have essentially the same format
anyway) but more importantly two entry points. The _SM3_ entry point is
brand new and most applications (including dmidecode) don't support it
yet. It doesn't matter if the kernel itself can parse it, as it passes
the raw entry point to applications anyway.

It happens that we are introducing this new sysfs raw interface at the
same time _SM3_ is being introduced, so we do not have to care about
backwards compatibility. Both the kernel and dmidecode will need to be
updated to support the new interface, so we can keep things simple and
let the kernel expose only the best entry point.

If the sysfs raw interface was already present at the time _SM3_
support was being added, then we would have had to present both entry
points for backwards compatibility. And if some _SM4_ entry point is
ever added in the future with a new format, we will have to export it
as a new sysfs attribute so as to not break compatibility.

As a summary, I agree that a single entry point file is OK for now, but
only because we are lucky that the timing is right.

-- 
Jean Delvare
SUSE L3 Support

Re: [dmidecode] [Patch v3] firmware: dmi-sysfs: add SMBIOS entry point area raw attribute

[Ivan.khoronzhuk]

Hi Jean,

On 03/05/2015 09:46 AM, Jean Delvare wrote:
> Hi Ivan,
>
> On Wed, 04 Mar 2015 14:28:20 +0200, Ivan.khoronzhuk wrote:
>> On 02/26/2015 11:41 AM, Jean Delvare wrote:
>>> On Thu, 26 Feb 2015 09:50:42 +0100, Jean Delvare wrote:
>>>> Please also note that the recently released version 3.0.0 of the SMBIOS
>>>> specification introduces a new entry point format, and the firmware is
>>>> allowed to implement both the old and the new format. It may be
>>>> desirable to expose both to user-space under different names.
>>> Having read the kernel code meanwhile, I see we will call either
>>> dmi_smbios3_present or dmi_present successfully, not both, so
>>> presenting both entry points to user-space would be non-trivial. So I'm
>>> fine with presenting only one entry point in sysfs for the time being.
>>> We can always revisit later if it turns out that dmidecode really needs
>>> both in some cases.
>> Why do you need two tables ? If kernel can parse the best one why
>> it should present the old one? The old is subset of new, so the new must
>> contain all required information, only format can be slightly different.
>> If dmidecode has problems in reading new version then dmidecode should
>> be modified, not kernel.
> It's not just two tables (I don't expect a lot of BIOSes to provide two
> tables in practice, and they would have essentially the same format
> anyway) but more importantly two entry points. The _SM3_ entry point is
> brand new and most applications (including dmidecode) don't support it
> yet. It doesn't matter if the kernel itself can parse it, as it passes
> the raw entry point to applications anyway.
>
> It happens that we are introducing this new sysfs raw interface at the
> same time _SM3_ is being introduced, so we do not have to care about
> backwards compatibility. Both the kernel and dmidecode will need to be
> updated to support the new interface, so we can keep things simple and
> let the kernel expose only the best entry point.
>
> If the sysfs raw interface was already present at the time _SM3_
> support was being added, then we would have had to present both entry
> points for backwards compatibility. And if some _SM4_ entry point is
> ever added in the future with a new format, we will have to export it
> as a new sysfs attribute so as to not break compatibility.
>
> As a summary, I agree that a single entry point file is OK for now, but
> only because we are lucky that the timing is right.
>

Despite of timing is right.

The specification doesn't oblige firmware to provide two entry points.
An implementation may provide either the 32-bit entry point or the 64-bit
entry point, or both. For compatibility with existing SMBIOS parsers, an
implementation should provide the 32-bit entry point, but it's not required.

Another case if specification requires to provide two entry points. Then 
you can
be sure in backward compatibility. But at least for now you can't.

It's obvious, if kernel found two entry points then it can create two 
sysfs attributes.
But, what kernel should do in case if only one new entry point is present.
Generate entry point of old version..., sorry but it's bad idea. At 
least because
where guarantee that we have enough information for this. Only field we 
can bring
thought entry point versions is magic string _SM*_, and based on it, if util
don't support new version it can warn. It's used for differ versions and
there is nothing we can do more.

-- 
Regards,
Ivan Khoronzhuk

Re: [dmidecode] [Patch v3] firmware: dmi-sysfs: add SMBIOS entry point area raw attribute

[Jean Delvare]

Hi Ivan,

On Sat, 07 Mar 2015 22:53:32 +0200, Ivan.khoronzhuk wrote:
> On 03/05/2015 09:46 AM, Jean Delvare wrote:
> > It's not just two tables (I don't expect a lot of BIOSes to provide two
> > tables in practice, and they would have essentially the same format
> > anyway) but more importantly two entry points. The _SM3_ entry point is
> > brand new and most applications (including dmidecode) don't support it
> > yet. It doesn't matter if the kernel itself can parse it, as it passes
> > the raw entry point to applications anyway.
> >
> > It happens that we are introducing this new sysfs raw interface at the
> > same time _SM3_ is being introduced, so we do not have to care about
> > backwards compatibility. Both the kernel and dmidecode will need to be
> > updated to support the new interface, so we can keep things simple and
> > let the kernel expose only the best entry point.
> >
> > If the sysfs raw interface was already present at the time _SM3_
> > support was being added, then we would have had to present both entry
> > points for backwards compatibility. And if some _SM4_ entry point is
> > ever added in the future with a new format, we will have to export it
> > as a new sysfs attribute so as to not break compatibility.
> >
> > As a summary, I agree that a single entry point file is OK for now, but
> > only because we are lucky that the timing is right.
> 
> Despite of timing is right.
> 
> The specification doesn't oblige firmware to provide two entry points.
> An implementation may provide either the 32-bit entry point or the 64-bit
> entry point, or both. For compatibility with existing SMBIOS parsers, an
> implementation should provide the 32-bit entry point, but it's not required.

I expect most implementations will do, as it's trivial to implement.

> Another case if specification requires to provide two entry points. Then 
> you can
> be sure in backward compatibility. But at least for now you can't.
> 
> It's obvious, if kernel found two entry points then it can create two 
> sysfs attributes.
> But, what kernel should do in case if only one new entry point is present.
> Generate entry point of old version..., sorry but it's bad idea. At 
> least because
> where guarantee that we have enough information for this. Only field we 
> can bring
> thought entry point versions is magic string _SM*_, and based on it, if util
> don't support new version it can warn. It's used for differ versions and
> there is nothing we can do more.

I agree that the kernel should not fake an entry point which does not
exist (I'm not sure if you misunderstood me but I never suggested that.)

-- 
Jean Delvare
SUSE L3 Support

Re: [dmidecode] [Patch v3] firmware: dmi-sysfs: add SMBIOS entry point area raw attribute

[Ard Biesheuvel]

On 8 March 2015 at 12:31, Jean Delvare <jdelvare@suse.de> wrote:
> Hi Ivan,
>
> On Sat, 07 Mar 2015 22:53:32 +0200, Ivan.khoronzhuk wrote:
>> On 03/05/2015 09:46 AM, Jean Delvare wrote:
>> > It's not just two tables (I don't expect a lot of BIOSes to provide two
>> > tables in practice, and they would have essentially the same format
>> > anyway) but more importantly two entry points. The _SM3_ entry point is
>> > brand new and most applications (including dmidecode) don't support it
>> > yet. It doesn't matter if the kernel itself can parse it, as it passes
>> > the raw entry point to applications anyway.
>> >
>> > It happens that we are introducing this new sysfs raw interface at the
>> > same time _SM3_ is being introduced, so we do not have to care about
>> > backwards compatibility. Both the kernel and dmidecode will need to be
>> > updated to support the new interface, so we can keep things simple and
>> > let the kernel expose only the best entry point.
>> >
>> > If the sysfs raw interface was already present at the time _SM3_
>> > support was being added, then we would have had to present both entry
>> > points for backwards compatibility. And if some _SM4_ entry point is
>> > ever added in the future with a new format, we will have to export it
>> > as a new sysfs attribute so as to not break compatibility.
>> >
>> > As a summary, I agree that a single entry point file is OK for now, but
>> > only because we are lucky that the timing is right.
>>
>> Despite of timing is right.
>>
>> The specification doesn't oblige firmware to provide two entry points.
>> An implementation may provide either the 32-bit entry point or the 64-bit
>> entry point, or both. For compatibility with existing SMBIOS parsers, an
>> implementation should provide the 32-bit entry point, but it's not required.
>
> I expect most implementations will do, as it's trivial to implement.
>

Not quite. First of all, some 64-bit ARM systems do not have any
system RAM below 4 GB, so there is not way they can implement the
32-bit entry point. Also, the 64-bit entry point does not limit the
structure size or the entire table to 64 KB like the 32-bit one does,
so it may be necessary to create a whole separate table with a subset
of the contents of the real table to stay within limits for the 32-bit
entry point. And the 32-bit entry point could well be 3.0 anyway, if
it uses any of the new enum values for the data items that were
undefined before 3.0.

More info here:
http://sourceforge.net/p/edk2/mailman/message/33550425/

Regards,
Ard.


>> you can
>> be sure in backward compatibility. But at least for now you can't.
>>
>> It's obvious, if kernel found two entry points then it can create two
>> sysfs attributes.
>> But, what kernel should do in case if only one new entry point is present.
>> Generate entry point of old version..., sorry but it's bad idea. At
>> least because
>> where guarantee that we have enough information for this. Only field we
>> can bring
>> thought entry point versions is magic string _SM*_, and based on it, if util
>> don't support new version it can warn. It's used for differ versions and
>> there is nothing we can do more.
>
> I agree that the kernel should not fake an entry point which does not
> exist (I'm not sure if you misunderstood me but I never suggested that.)
>
> --
> Jean Delvare
> SUSE L3 Support

Re: [dmidecode] [Patch v3] firmware: dmi-sysfs: add SMBIOS entry point area raw attribute

[Jean Delvare]

On Sun, 8 Mar 2015 14:53:04 +0100, Ard Biesheuvel wrote:
> On 8 March 2015 at 12:31, Jean Delvare <jdelvare@suse.de> wrote:
> > On Sat, 07 Mar 2015 22:53:32 +0200, Ivan.khoronzhuk wrote:
> >> The specification doesn't oblige firmware to provide two entry points.
> >> An implementation may provide either the 32-bit entry point or the 64-bit
> >> entry point, or both. For compatibility with existing SMBIOS parsers, an
> >> implementation should provide the 32-bit entry point, but it's not required.
> >
> > I expect most implementations will do, as it's trivial to implement.
> 
> Not quite. First of all, some 64-bit ARM systems do not have any
> system RAM below 4 GB, so there is not way they can implement the
> 32-bit entry point.

I didn't know that, thanks for the notice. No big deal anyway, these
systems did not support SMBIOS before version 3.0 so there cannot be
any regression on these systems.

> Also, the 64-bit entry point does not limit the
> structure size or the entire table to 64 KB like the 32-bit one does,
> so it may be necessary to create a whole separate table with a subset
> of the contents of the real table to stay within limits for the 32-bit
> entry point.

I doubt this is an issue in practice. I have been around for quite some
time now and the largest table I've ever seen was 9043 byte long, which
is nowhere close to the limit.

> And the 32-bit entry point could well be 3.0 anyway, if
> it uses any of the new enum values for the data items that were
> undefined before 3.0.

This is true but irrelevant to the discussion.

Thanks,
-- 
Jean Delvare
SUSE L3 Support

Re: [dmidecode] [Patch v3] firmware: dmi-sysfs: add SMBIOS entry point area raw attribute

[Ard Biesheuvel]

On 8 March 2015 at 18:11, Jean Delvare <jdelvare@suse.de> wrote:
> On Sun, 8 Mar 2015 14:53:04 +0100, Ard Biesheuvel wrote:
>> On 8 March 2015 at 12:31, Jean Delvare <jdelvare@suse.de> wrote:
>> > On Sat, 07 Mar 2015 22:53:32 +0200, Ivan.khoronzhuk wrote:
>> >> The specification doesn't oblige firmware to provide two entry points.
>> >> An implementation may provide either the 32-bit entry point or the 64-bit
>> >> entry point, or both. For compatibility with existing SMBIOS parsers, an
>> >> implementation should provide the 32-bit entry point, but it's not required.
>> >
>> > I expect most implementations will do, as it's trivial to implement.
>>
>> Not quite. First of all, some 64-bit ARM systems do not have any
>> system RAM below 4 GB, so there is not way they can implement the
>> 32-bit entry point.
>
> I didn't know that, thanks for the notice. No big deal anyway, these
> systems did not support SMBIOS before version 3.0 so there cannot be
> any regression on these systems.
>
>> Also, the 64-bit entry point does not limit the
>> structure size or the entire table to 64 KB like the 32-bit one does,
>> so it may be necessary to create a whole separate table with a subset
>> of the contents of the real table to stay within limits for the 32-bit
>> entry point.
>
> I doubt this is an issue in practice. I have been around for quite some
> time now and the largest table I've ever seen was 9043 byte long, which
> is nowhere close to the limit.
>
>> And the 32-bit entry point could well be 3.0 anyway, if
>> it uses any of the new enum values for the data items that were
>> undefined before 3.0.
>
> This is true but irrelevant to the discussion.
>

To clarify, the SMBIOS 3.0 spec explicitly allows the 32-bit entry
point to either point to the same table as the 64-bit entry point, or
point to a separate table, in which case the contents of the latter
should be a subset of the contents of the former. It doesn't specify
anything about the version number to be used in the 32-bit entry point
in case they point to separate tables. This means the presence of the
32-bit entry point does not guarantee that the table contents are
compatible with the pre-3.0 tools. So perhaps it would make sense to
export the 32-bit entry point separately only if it points to a
different table, and has a different version number?

-- 
Ard.

Re: [dmidecode] [Patch v3] firmware: dmi-sysfs: add SMBIOS entry point area raw attribute

[Jean Delvare]

On Sun, 8 Mar 2015 18:38:57 +0100, Ard Biesheuvel wrote:
> On 8 March 2015 at 18:11, Jean Delvare <jdelvare@suse.de> wrote:
> > On Sun, 8 Mar 2015 14:53:04 +0100, Ard Biesheuvel wrote:
> >> And the 32-bit entry point could well be 3.0 anyway, if
> >> it uses any of the new enum values for the data items that were
> >> undefined before 3.0.
> >
> > This is true but irrelevant to the discussion.
> 
> To clarify, the SMBIOS 3.0 spec explicitly allows the 32-bit entry
> point to either point to the same table as the 64-bit entry point, or
> point to a separate table, in which case the contents of the latter
> should be a subset of the contents of the former. It doesn't specify
> anything about the version number to be used in the 32-bit entry point
> in case they point to separate tables. This means the presence of the
> 32-bit entry point does not guarantee that the table contents are
> compatible with the pre-3.0 tools. So perhaps it would make sense to
> export the 32-bit entry point separately only if it points to a
> different table, and has a different version number?

The situation is exactly the same as with every new version of the
SMBIOS specification: tools need to be updated to support the new
enumerated values and the new fields, but are able to decode all the
rest just fine. The only thing that would make it a different situation
is if something in the 3.0 specification is incompatible with previous
specifications. But I'd be very surprised if this is the case, as I am
sure the DMTF people care about compatibility.

And I can't see any practical case where the vendor would want to not
implement version 3.0 in the _SM_-pointed table if they do so for the
_SM3_-pointed table. That's more work for them and serves no purpose,
as everything that could be encoded in old versions can also be encoded
in newer versions.

So, no, I still don't think there is any value in exposing two entry
points in sysfs.

-- 
Jean Delvare
SUSE L3 Support