On Tue, Apr 07, 2015 at 02:51:39PM -0500, miny...@acm.org wrote:
> From: Corey Minyard <cminy...@mvista.com>
>
> There was no way to directly add a table entry to the SMBIOS table,
> even though the BIOS supports this. So add a function to do this.
> This is in preparation for the IPMI handler adding it's SMBIOS table
> entry.
>
> Signed-off-by: Corey Minyard <cminy...@mvista.com>
Do we have to use a callback for this?
It seems that if smbios_table_entry_add just added the table
itself to some array or a linked list, then devices could just call
smbios_table_entry_add instead of registering a handler.
And smbios_get_tables would scan that and get the tables.
On systems without smbios, this function could be a nop stub.
Did I miss something?
> ---
> hw/i386/smbios.c | 149
> ++++++++++++++++++++++++++++++-----------------
> include/hw/i386/smbios.h | 13 +++++
> 2 files changed, 110 insertions(+), 52 deletions(-)
>
> diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
> index 1341e02..fe15325 100644
> --- a/hw/i386/smbios.c
> +++ b/hw/i386/smbios.c
> @@ -831,6 +831,25 @@ static void smbios_entry_point_setup(void)
> ep.structure_table_address = cpu_to_le32(0);
> }
>
> +struct smbios_device_handler {
> + void (*handle_device_table)(void *opaque);
> + void *opaque;
> + struct smbios_device_handler *prev;
> +};
> +static struct smbios_device_handler *device_handlers;
> +
> +void smbios_register_device_table_handler(void (*handle_device_table)
> + (void *opaque),
> + void *opaque)
> +{
> + struct smbios_device_handler *handler = g_malloc(sizeof(*handler));
> +
> + handler->handle_device_table = handle_device_table;
> + handler->opaque = opaque;
> + handler->prev = device_handlers;
> + device_handlers = handler;
> +}
> +
> void smbios_get_tables(uint8_t **tables, size_t *tables_len,
> uint8_t **anchor, size_t *anchor_len)
> {
> @@ -875,6 +894,14 @@ void smbios_get_tables(uint8_t **tables, size_t
> *tables_len,
> }
>
> smbios_build_type_32_table();
> +
> + while (device_handlers) {
> + struct smbios_device_handler *handler = device_handlers;
> + device_handlers = handler->prev;
> + handler->handle_device_table(handler->opaque);
> + g_free(handler);
> + }
> +
> smbios_build_type_127_table();
>
> smbios_validate_table();
> @@ -898,6 +925,71 @@ static void save_opt(const char **dest, QemuOpts *opts,
> const char *name)
> }
> }
>
> +int smbios_table_entry_add(void *data, int size, bool append_zeros)
> +{
> + struct smbios_structure_header *header;
> + struct smbios_table *table; /* legacy mode only */
> +
> + /*
> + * NOTE: standard double '\0' terminator expected, per smbios spec.
> + * (except in legacy mode, where the second '\0' is implicit and
> + * will be inserted by the BIOS).
> + */
> + if (append_zeros) {
> + size += 2;
> + }
> + smbios_tables = g_realloc(smbios_tables, smbios_tables_len + size);
> + header = (struct smbios_structure_header *)(smbios_tables +
> + smbios_tables_len);
> +
> + memcpy(header, data, size);
> +
> + if (test_bit(header->type, have_fields_bitmap)) {
> + error_report("can't load type %d struct, fields already specified!",
> + header->type);
> + exit(1);
> + }
> + set_bit(header->type, have_binfile_bitmap);
> +
> + if (header->type == 4) {
> + smbios_type4_count++;
> + }
> +
> + smbios_tables_len += size;
> + if (size > smbios_table_max) {
> + smbios_table_max = size;
> + }
> + smbios_table_cnt++;
> +
> + /* add a copy of the newly loaded blob to legacy smbios_entries */
> + /* NOTE: This code runs before smbios_set_defaults(), so we don't
> + * yet know which mode (legacy vs. aggregate-table) will be
> + * required. We therefore add the binary blob to both legacy
> + * (smbios_entries) and aggregate (smbios_tables) tables, and
> + * delete the one we don't need from smbios_set_defaults(),
> + * once we know which machine version has been requested.
> + */
> + if (!smbios_entries) {
> + smbios_entries_len = sizeof(uint16_t);
> + smbios_entries = g_malloc0(smbios_entries_len);
> + }
> + if (append_zeros) {
> + size -= 1; /* The BIOS adds the second zero in legacy mode. */
> + }
> + smbios_entries = g_realloc(smbios_entries, smbios_entries_len +
> + size + sizeof(*table));
> + table = (struct smbios_table *)(smbios_entries + smbios_entries_len);
> + table->header.type = SMBIOS_TABLE_ENTRY;
> + table->header.length = cpu_to_le16(sizeof(*table) + size);
> + memcpy(table->data, header, size);
> + smbios_entries_len += sizeof(*table) + size;
> + (*(uint16_t *)smbios_entries) =
> + cpu_to_le16(le16_to_cpu(*(uint16_t *)smbios_entries) + 1);
> + /* end: add a copy of the newly loaded blob to legacy smbios_entries */
> +
> + return 0;
> +}
> +
> void smbios_entry_add(QemuOpts *opts)
> {
> Error *local_err = NULL;
> @@ -907,9 +999,8 @@ void smbios_entry_add(QemuOpts *opts)
>
> val = qemu_opt_get(opts, "file");
> if (val) {
> - struct smbios_structure_header *header;
> int size;
> - struct smbios_table *table; /* legacy mode only */
> + uint8_t *data;
>
> qemu_opts_validate(opts, qemu_smbios_file_opts, &local_err);
> if (local_err) {
> @@ -923,60 +1014,14 @@ void smbios_entry_add(QemuOpts *opts)
> exit(1);
> }
>
> - /*
> - * NOTE: standard double '\0' terminator expected, per smbios spec.
> - * (except in legacy mode, where the second '\0' is implicit and
> - * will be inserted by the BIOS).
> - */
> - smbios_tables = g_realloc(smbios_tables, smbios_tables_len + size);
> - header = (struct smbios_structure_header *)(smbios_tables +
> - smbios_tables_len);
> -
> - if (load_image(val, (uint8_t *)header) != size) {
> + data = g_malloc(size);
> + if (load_image(val, data) != size) {
> error_report("Failed to load SMBIOS file %s", val);
> exit(1);
> }
>
> - if (test_bit(header->type, have_fields_bitmap)) {
> - error_report("can't load type %d struct, fields already
> specified!",
> - header->type);
> - exit(1);
> - }
> - set_bit(header->type, have_binfile_bitmap);
> -
> - if (header->type == 4) {
> - smbios_type4_count++;
> - }
> -
> - smbios_tables_len += size;
> - if (size > smbios_table_max) {
> - smbios_table_max = size;
> - }
> - smbios_table_cnt++;
> -
> - /* add a copy of the newly loaded blob to legacy smbios_entries */
> - /* NOTE: This code runs before smbios_set_defaults(), so we don't
> - * yet know which mode (legacy vs. aggregate-table) will be
> - * required. We therefore add the binary blob to both legacy
> - * (smbios_entries) and aggregate (smbios_tables) tables, and
> - * delete the one we don't need from smbios_set_defaults(),
> - * once we know which machine version has been requested.
> - */
> - if (!smbios_entries) {
> - smbios_entries_len = sizeof(uint16_t);
> - smbios_entries = g_malloc0(smbios_entries_len);
> - }
> - smbios_entries = g_realloc(smbios_entries, smbios_entries_len +
> - size + sizeof(*table));
> - table = (struct smbios_table *)(smbios_entries + smbios_entries_len);
> - table->header.type = SMBIOS_TABLE_ENTRY;
> - table->header.length = cpu_to_le16(sizeof(*table) + size);
> - memcpy(table->data, header, size);
> - smbios_entries_len += sizeof(*table) + size;
> - (*(uint16_t *)smbios_entries) =
> - cpu_to_le16(le16_to_cpu(*(uint16_t *)smbios_entries) + 1);
> - /* end: add a copy of the newly loaded blob to legacy smbios_entries
> */
> -
> + smbios_table_entry_add(data, size, false);
> + g_free(data);
> return;
> }
>
> diff --git a/include/hw/i386/smbios.h b/include/hw/i386/smbios.h
> index d2850be..656327d 100644
> --- a/include/hw/i386/smbios.h
> +++ b/include/hw/i386/smbios.h
> @@ -27,6 +27,19 @@ void smbios_get_tables(uint8_t **tables, size_t
> *tables_len,
> uint8_t **anchor, size_t *anchor_len);
>
> /*
> + * Add an external entry to the SMBIOS table. Can only be called
> + * from a registered device table handler.
> + */
> +int smbios_table_entry_add(void *data, int size, bool append_zeros);
> +
> +/*
> + * When constructing SMBIOS tables, call a function at the end of the
> + * add process to allow devices to add their own SMBIOS table entries.
> + */
> +void smbios_register_device_table_handler(void (*handle_device_table)
> + (void *opaque),
> + void *opaque);
> +/*
> * SMBIOS spec defined tables
> */
>
> --
> 1.8.3.1
>
