| Message ID | 1422044507-22982-2-git-send-email-ivan.khoronzhuk@linaro.org |
|---|---|
| State | New |
| Headers | show |
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 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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 >> -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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
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(+)