From: "Michael S. Tsirkin" <mst@redhat.com>
To: Lin Ma <lma@suse.com>
Cc: imammedo@redhat.com, qemu-devel@nongnu.org, famz@redhat.com,
lersek@redhat.com
Subject: Re: [Qemu-devel] [PATCH] smbios: Add 1 terminator if there is any string field defined in given table.
Date: Thu, 10 Nov 2016 17:09:30 +0200 [thread overview]
Message-ID: <20161110170840-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <20160906082833.25428-1-lma@suse.com>
On Tue, Sep 06, 2016 at 04:28:33PM +0800, Lin Ma wrote:
> If user specifies binary file on command line to load smbios entries, then
> will get error messages while decoding them in guest.
>
> Reproducer:
> 1. dump a smbios table to a binary file from host or guest.(says table 1)
> 2. load the binary file through command line: 'qemu -smbios file=...'.
> 3. perform 'dmidecode' or 'dmidecode -t 1' in guest.
>
> It reports 'Invalid entry length...' because qemu doesn't add terminator(s) for
> the table correctly.
> For smbios tables which have string field provided, qemu should add 1 terminator.
> For smbios tables which dont have string field provided, qemu should add 2.
>
> This patch fixed the issue.
>
> Signed-off-by: Lin Ma <lma@suse.com>
Seems to make sense superficially
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Fam, would you like to take this?
> ---
> hw/smbios/smbios.c | 90 ++++++++++++++++++++++++++++++++++++++++++++++
> include/hw/smbios/smbios.h | 44 +++++++++++++++++++++++
> 2 files changed, 134 insertions(+)
>
> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
> index 74c7102..6293bc5 100644
> --- a/hw/smbios/smbios.c
> +++ b/hw/smbios/smbios.c
> @@ -885,6 +885,9 @@ void smbios_entry_add(QemuOpts *opts)
> {
> const char *val;
>
> + int i, terminator_count = 2, table_str_field_count = 0;
> + int *tables_str_field_offset = NULL;
> +
> assert(!smbios_immutable);
>
> val = qemu_opt_get(opts, "file");
> @@ -926,7 +929,94 @@ void smbios_entry_add(QemuOpts *opts)
> smbios_type4_count++;
> }
>
> + switch (header->type) {
> + case 0:
> + tables_str_field_offset = g_malloc0(sizeof(int) * \
> + TYPE_0_STR_FIELD_COUNT);
> + tables_str_field_offset = (int []){\
> + TYPE_0_STR_FIELD_OFFSET_VENDOR, \
> + TYPE_0_STR_FIELD_OFFSET_BIOS_VERSION, \
> + TYPE_0_STR_FIELD_OFFSET_BIOS_RELEASE_DATE};
> + table_str_field_count = sizeof(tables_str_field_offset) / \
> + sizeof(tables_str_field_offset[0]);
> + break;
> + case 1:
> + tables_str_field_offset = g_malloc0(sizeof(int) * \
> + TYPE_1_STR_FIELD_COUNT);
> + tables_str_field_offset = (int []){
> + TYPE_1_STR_FIELD_OFFSET_MANUFACTURER, \
> + TYPE_1_STR_FIELD_OFFSET_PRODUCT, \
> + TYPE_1_STR_FIELD_OFFSET_VERSION, \
> + TYPE_1_STR_FIELD_OFFSET_SERIAL, \
> + TYPE_1_STR_FIELD_OFFSET_SKU, \
> + TYPE_1_STR_FIELD_OFFSET_FAMILY};
> + table_str_field_count = sizeof(tables_str_field_offset) / \
> + sizeof(tables_str_field_offset[0]);
> + break;
> + case 2:
> + tables_str_field_offset = g_malloc0(sizeof(int) * \
> + TYPE_2_STR_FIELD_COUNT);
> + tables_str_field_offset = (int []){\
> + TYPE_2_STR_FIELD_OFFSET_MANUFACTURER, \
> + TYPE_2_STR_FIELD_OFFSET_PRODUCT, \
> + TYPE_2_STR_FIELD_OFFSET_VERSION, \
> + TYPE_2_STR_FIELD_OFFSET_SERIAL, \
> + TYPE_2_STR_FIELD_OFFSET_ASSET, \
> + TYPE_2_STR_FIELD_OFFSET_LOCATION};
> + table_str_field_count = sizeof(tables_str_field_offset) / \
> + sizeof(tables_str_field_offset[0]);
> + break;
> + case 3:
> + tables_str_field_offset = g_malloc0(sizeof(int) * \
> + TYPE_3_STR_FIELD_COUNT);
> + tables_str_field_offset = (int []){\
> + TYPE_3_STR_FIELD_OFFSET_MANUFACTURER, \
> + TYPE_3_STR_FIELD_OFFSET_VERSION, \
> + TYPE_3_STR_FIELD_OFFSET_SERIAL, \
> + TYPE_3_STR_FIELD_OFFSET_ASSET, \
> + TYPE_3_STR_FIELD_OFFSET_SKU};
> + table_str_field_count = sizeof(tables_str_field_offset) / \
> + sizeof(tables_str_field_offset[0]);
> + break;
> + case 4:
> + tables_str_field_offset = g_malloc0(sizeof(int) * \
> + TYPE_4_STR_FIELD_COUNT);
> + tables_str_field_offset = (int []){\
> + TYPE_4_STR_FIELD_OFFSET_SOCKET, \
> + TYPE_4_STR_FIELD_OFFSET_PROCESSOR_MANUFACTURER, \
> + TYPE_4_STR_FIELD_OFFSET_PROCESSOR_VERSION, \
> + TYPE_4_STR_FIELD_OFFSET_SERIAL, \
> + TYPE_4_STR_FIELD_OFFSET_ASSET, \
> + TYPE_4_STR_FIELD_OFFSET_PART};
> + table_str_field_count = sizeof(tables_str_field_offset) / \
> + sizeof(tables_str_field_offset[0]);
> + break;
> + case 17:
> + tables_str_field_offset = g_malloc0(sizeof(int) * \
> + TYPE_17_STR_FIELD_COUNT);
> + tables_str_field_offset = (int []){\
> + TYPE_17_STR_FIELD_OFFSET_DEVICE_LOCATOR, \
> + TYPE_17_STR_FIELD_OFFSET_BANK_LOCATOR, \
> + TYPE_17_STR_FIELD_OFFSET_MANUFACTURER, \
> + TYPE_17_STR_FIELD_OFFSET_SERIAL, \
> + TYPE_17_STR_FIELD_OFFSET_ASSET, \
> + TYPE_17_STR_FIELD_OFFSET_PART};
> + table_str_field_count = sizeof(tables_str_field_offset) / \
> + sizeof(tables_str_field_offset[0]);
> + break;
> + default:
> + break;
> + }
> +
> + for (i = 0; i < table_str_field_count; i++) {
> + if (*(uint8_t *)(smbios_tables + tables_str_field_offset[i]) > 0) {
> + terminator_count = 1;
> + break;
> + }
> + }
> +
> smbios_tables_len += size;
> + smbios_tables_len += terminator_count;
> if (size > smbios_table_max) {
> smbios_table_max = size;
> }
> diff --git a/include/hw/smbios/smbios.h b/include/hw/smbios/smbios.h
> index 1cd53cc..6d59c3d 100644
> --- a/include/hw/smbios/smbios.h
> +++ b/include/hw/smbios/smbios.h
> @@ -267,4 +267,48 @@ void smbios_get_tables(const struct smbios_phys_mem_area *mem_array,
> const unsigned int mem_array_size,
> uint8_t **tables, size_t *tables_len,
> uint8_t **anchor, size_t *anchor_len);
> +
> +#define TYPE_0_STR_FIELD_OFFSET_VENDOR 0x4
> +#define TYPE_0_STR_FIELD_OFFSET_BIOS_VERSION 0x5
> +#define TYPE_0_STR_FIELD_OFFSET_BIOS_RELEASE_DATE 0x8
> +#define TYPE_0_STR_FIELD_COUNT 3
> +
> +#define TYPE_1_STR_FIELD_OFFSET_MANUFACTURER 0x4
> +#define TYPE_1_STR_FIELD_OFFSET_PRODUCT 0x5
> +#define TYPE_1_STR_FIELD_OFFSET_VERSION 0x6
> +#define TYPE_1_STR_FIELD_OFFSET_SERIAL 0x7
> +#define TYPE_1_STR_FIELD_OFFSET_SKU 0x19
> +#define TYPE_1_STR_FIELD_OFFSET_FAMILY 0x1a
> +#define TYPE_1_STR_FIELD_COUNT 6
> +
> +#define TYPE_2_STR_FIELD_OFFSET_MANUFACTURER 0x4
> +#define TYPE_2_STR_FIELD_OFFSET_PRODUCT 0x5
> +#define TYPE_2_STR_FIELD_OFFSET_VERSION 0x6
> +#define TYPE_2_STR_FIELD_OFFSET_SERIAL 0x7
> +#define TYPE_2_STR_FIELD_OFFSET_ASSET 0x8
> +#define TYPE_2_STR_FIELD_OFFSET_LOCATION 0xa
> +#define TYPE_2_STR_FIELD_COUNT 6
> +
> +#define TYPE_3_STR_FIELD_OFFSET_MANUFACTURER 0x4
> +#define TYPE_3_STR_FIELD_OFFSET_VERSION 0x6
> +#define TYPE_3_STR_FIELD_OFFSET_SERIAL 0x7
> +#define TYPE_3_STR_FIELD_OFFSET_ASSET 0x8
> +#define TYPE_3_STR_FIELD_OFFSET_SKU 0x14
> +#define TYPE_3_STR_FIELD_COUNT 5
> +
> +#define TYPE_4_STR_FIELD_OFFSET_SOCKET 0x4
> +#define TYPE_4_STR_FIELD_OFFSET_PROCESSOR_MANUFACTURER 0x7
> +#define TYPE_4_STR_FIELD_OFFSET_PROCESSOR_VERSION 0x10
> +#define TYPE_4_STR_FIELD_OFFSET_SERIAL 0x20
> +#define TYPE_4_STR_FIELD_OFFSET_ASSET 0x21
> +#define TYPE_4_STR_FIELD_OFFSET_PART 0x22
> +#define TYPE_4_STR_FIELD_COUNT 6
> +
> +#define TYPE_17_STR_FIELD_OFFSET_DEVICE_LOCATOR 0x10
> +#define TYPE_17_STR_FIELD_OFFSET_BANK_LOCATOR 0x11
> +#define TYPE_17_STR_FIELD_OFFSET_MANUFACTURER 0x17
> +#define TYPE_17_STR_FIELD_OFFSET_SERIAL 0x18
> +#define TYPE_17_STR_FIELD_OFFSET_ASSET 0x19
> +#define TYPE_17_STR_FIELD_OFFSET_PART 0x1a
> +#define TYPE_17_STR_FIELD_COUNT 6
> #endif /* QEMU_SMBIOS_H */
> --
> 2.9.2
next prev parent reply other threads:[~2016-11-10 15:09 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-06 8:28 [Qemu-devel] [PATCH] smbios: Add 1 terminator if there is any string field defined in given table Lin Ma
2016-11-07 16:25 ` [Qemu-devel] 答复： " Lin Ma
2016-11-07 16:31 ` Daniel P. Berrange
2016-11-07 16:45 ` Lin Ma
2016-11-10 15:09 ` Michael S. Tsirkin [this message]
2016-11-10 17:18 ` [Qemu-devel] " Laszlo Ersek
2016-11-10 17:31 ` Laszlo Ersek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20161110170840-mutt-send-email-mst@kernel.org \
--to=mst@redhat.com \
--cc=famz@redhat.com \
--cc=imammedo@redhat.com \
--cc=lersek@redhat.com \
--cc=lma@suse.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).