* Re: [dmidecode] [Patch v3] firmware: dmi-sysfs: add SMBIOS entry point area raw attribute
[not found] <1422448763-17583-1-git-send-email-ivan.khoronzhuk@linaro.org>
@ 2015-01-28 15:56 ` Ivan Khoronzhuk
[not found] ` <54C906A9.1050001-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
0 siblings, 1 reply; 3+ messages in thread
From: Ivan Khoronzhuk @ 2015-01-28 15:56 UTC (permalink / raw)
To: linux-kernel, ard.biesheuvel, grant.likely, matt.fleming,
linux-api, linux-doc@vger.kernel.org
Cc: dmidecode-devel, leif.lindholm
+ 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
>
_______________________________________________
https://lists.nongnu.org/mailman/listinfo/dmidecode-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Patch v3] firmware: dmi-sysfs: add SMBIOS entry point area raw attribute
[not found] ` <54C906A9.1050001-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
@ 2015-02-03 10:49 ` Matt Fleming
2015-02-03 14:47 ` Ivan Khoronzhuk
0 siblings, 1 reply; 3+ messages in thread
From: Matt Fleming @ 2015-02-03 10:49 UTC (permalink / raw)
To: Ivan Khoronzhuk
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A,
grant.likely-QSEj5FYQhm4dnm+yROfE0A,
matt.fleming-ral2JQCrhuEAvxtiuMwx3w,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
dmidecode-devel-qX2TKyscuCcdnm+yROfE0A,
leif.lindholm-QSEj5FYQhm4dnm+yROfE0A
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Patch v3] firmware: dmi-sysfs: add SMBIOS entry point area raw attribute
2015-02-03 10:49 ` Matt Fleming
@ 2015-02-03 14:47 ` Ivan Khoronzhuk
0 siblings, 0 replies; 3+ messages in thread
From: Ivan Khoronzhuk @ 2015-02-03 14:47 UTC (permalink / raw)
To: Matt Fleming
Cc: linux-kernel, ard.biesheuvel, grant.likely, matt.fleming,
linux-api, linux-doc@vger.kernel.org, dmidecode-devel,
leif.lindholm
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!
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-02-03 14:47 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1422448763-17583-1-git-send-email-ivan.khoronzhuk@linaro.org>
2015-01-28 15:56 ` [dmidecode] [Patch v3] firmware: dmi-sysfs: add SMBIOS entry point area raw attribute Ivan Khoronzhuk
[not found] ` <54C906A9.1050001-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2015-02-03 10:49 ` Matt Fleming
2015-02-03 14:47 ` 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).