Messages in this thread | | | Date | Mon, 26 Jan 2015 13:53:50 +0200 | From | Ivan Khoronzhuk <> | Subject | Re: [PATCH 1/2] firmware: dmi_scan: add symbol to get SMBIOS entry area |
| |
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 >>
| |