[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [dmidecode] [PATCH 2/4] Add passing of 64 bit table address to dmi_t
|
From: |
Jean Delvare |
|
Subject: |
Re: [dmidecode] [PATCH 2/4] Add passing of 64 bit table address to dmi_table() |
|
Date: |
Fri, 17 Apr 2015 13:22:06 +0200 |
Hi Roy,
On Thu, 16 Apr 2015 21:13:59 -0700, Roy Franz wrote:
> This allows dmi_table() to print the address that the table was read from,
> even when reading from a sysfs file. A 64 bit address is used in preparation
> for supporting SMBIOS v3 64-bit entry points with 64 bit table addresses.
> If the table address is only 32 bits, retain the 32 bit print size so that
> this change doesn't change the output for 32 bit table addresses.
64-bit, 64-bit, 32-bit, 32-bit (missing dashes)
> dmi_table() used to use the file offset to print the table address,
> since the offset in /dev/mem was the table address. When reading
> tables from sysfs files, the offset will be 0, as the file contains
> just the table.
>
> Signed-off-by: Roy Franz <address@hidden>
> ---
> dmidecode.c | 45 +++++++++++++++++++++++++++++++--------------
> 1 file changed, 31 insertions(+), 14 deletions(-)
>
> diff --git a/dmidecode.c b/dmidecode.c
> index a5304a7..1d19a1b 100644
> --- a/dmidecode.c
> +++ b/dmidecode.c
> @@ -4331,7 +4331,7 @@ static void dmi_table_dump(u32 base, u16 len, const
> char *devmem)
> free(buf);
> }
>
> -static void dmi_table(u32 base, u16 len, u16 num, u16 ver, const char
> *devmem)
> +static void dmi_table(u32 base, u16 len, u16 num, u16 ver, const char
> *devmem, u64 dmi_table_addr)
> {
> u8 *buf;
> u8 *data;
> @@ -4357,7 +4357,16 @@ static void dmi_table(u32 base, u16 len, u16 num, u16
> ver, const char *devmem)
> printf("%u structures occupying %u bytes.\n",
> num, len);
> if (!(opt.flags & FLAG_FROM_DUMP))
> - printf("Table at 0x%08X.\n", base);
> + {
> + if (dmi_table_addr.h)
> + printf("Table at 0x%08X%08X.\n",
> + dmi_table_addr.h,
> + dmi_table_addr.l);
> + else
> + printf("Table at 0x%08X.\n",
> + dmi_table_addr.l);
> + }
> +
> }
> printf("\n");
> }
> @@ -4468,9 +4477,10 @@ static void overwrite_dmi_address(u8 *buf)
> buf[0x0B] = 0;
> }
>
> -static int smbios_decode(u8 *buf, const char *devmem)
> +static int smbios_decode(u8 *buf, const char *devmem, u32 offset)
> {
> u16 ver;
> + u64 dmi_table_addr;
>
> if (!checksum(buf, buf[0x05])
> || memcmp(buf + 0x10, "_DMI_", 5) != 0
> @@ -4499,8 +4509,10 @@ static int smbios_decode(u8 *buf, const char *devmem)
> printf("SMBIOS %u.%u present.\n",
> ver >> 8, ver & 0xFF);
>
> - dmi_table(DWORD(buf + 0x18), WORD(buf + 0x16), WORD(buf + 0x1C),
> - ver, devmem);
> + dmi_table_addr.h = 0;
> + dmi_table_addr.l = DWORD(buf + 0x18);
> + dmi_table(offset, WORD(buf + 0x16), WORD(buf + 0x1C),
> + ver, devmem, dmi_table_addr);
>
> if (opt.flags & FLAG_DUMP_BIN)
> {
> @@ -4518,8 +4530,10 @@ static int smbios_decode(u8 *buf, const char *devmem)
> return 1;
> }
>
> -static int legacy_decode(u8 *buf, const char *devmem)
> +static int legacy_decode(u8 *buf, const char *devmem, u32 offset)
> {
> + u64 dmi_table_addr;
> +
> if (!checksum(buf, 0x0F))
> return 0;
>
> @@ -4527,8 +4541,11 @@ static int legacy_decode(u8 *buf, const char *devmem)
> printf("Legacy DMI %u.%u present.\n",
> buf[0x0E] >> 4, buf[0x0E] & 0x0F);
>
> - dmi_table(DWORD(buf + 0x08), WORD(buf + 0x06), WORD(buf + 0x0C),
> - ((buf[0x0E] & 0xF0) << 4) + (buf[0x0E] & 0x0F), devmem);
> + dmi_table_addr.h = 0;
> + dmi_table_addr.l = DWORD(buf + 0x08);
> + dmi_table(offset, WORD(buf + 0x06), WORD(buf + 0x0C),
> + ((buf[0x0E] & 0xF0) << 4) + (buf[0x0E] & 0x0F), devmem,
> + dmi_table_addr);
>
> if (opt.flags & FLAG_DUMP_BIN)
> {
> @@ -4646,18 +4663,18 @@ int main(int argc, char * const argv[])
>
> if (memcmp(buf, "_SM_", 4) == 0)
> {
> - if (smbios_decode(buf, opt.dumpfile))
> + if (smbios_decode(buf, opt.dumpfile, DWORD(buf + 0x18)))
> found++;
> }
> else if (memcmp(buf, "_DMI_", 5) == 0)
> {
> - if (legacy_decode(buf, opt.dumpfile))
> + if (legacy_decode(buf, opt.dumpfile, DWORD(buf + 0x08)))
> found++;
> }
> goto done;
> }
>
> - /* First try EFI (ia64, Intel-based Mac) */
> + /* Next try EFI (ia64, Intel-based Mac) */
> efi = address_from_efi(&fp);
> switch (efi)
> {
This change rather belongs to the next patch.
> @@ -4674,7 +4691,7 @@ int main(int argc, char * const argv[])
> goto exit_free;
> }
>
> - if (smbios_decode(buf, opt.devmem))
> + if (smbios_decode(buf, opt.devmem, DWORD(buf + 0x18)))
> found++;
> goto done;
>
> @@ -4690,7 +4707,7 @@ memory_scan:
> {
> if (memcmp(buf + fp, "_SM_", 4) == 0 && fp <= 0xFFE0)
> {
> - if (smbios_decode(buf+fp, opt.devmem))
> + if (smbios_decode(buf+fp, opt.devmem, DWORD(buf + fp +
> 0x18)))
> {
> found++;
> fp += 16;
> @@ -4698,7 +4715,7 @@ memory_scan:
> }
> else if (memcmp(buf + fp, "_DMI_", 5) == 0)
> {
> - if (legacy_decode(buf + fp, opt.devmem))
> + if (legacy_decode(buf + fp, opt.devmem, DWORD(buf + fp
> + 0x08)))
> found++;
> }
> }
This approach introduces some redundancy. It is small and certainly
acceptable, but this made me think, wouldn't it make more sense to pass
a boolean flag as the extra parameter, no_offset = 1 for the sysfs case
and 0? Just a thought, maybe worth trying.
--
Jean Delvare
SUSE L3 Support