Re: [Qemu-devel] [PATCH v3 14/16] acpi: Add a way for devices to add ACPI tables

["Michael S. Tsirkin" <mst@redhat.com>]

On Mon, Jun 08, 2015 at 08:12:09PM -0500, minyard@acm.org wrote:
> From: Corey Minyard <cminyard@mvista.com>
> 
> Some devices, like IPMI, need to add ACPI table entries to report
> their presence.  Add a method for adding these entries.
> 
> Signed-off-by: Corey Minyard <cminyard@mvista.com>
> ---
>  hw/acpi/Makefile.objs             |  1 +
>  hw/acpi/acpi-dev-tables.c         | 80 +++++++++++++++++++++++++++++++++++++++
>  hw/i386/acpi-build.c              | 17 +++++++++
>  include/hw/acpi/acpi-dev-tables.h | 38 +++++++++++++++++++
>  4 files changed, 136 insertions(+)
>  create mode 100644 hw/acpi/acpi-dev-tables.c
>  create mode 100644 include/hw/acpi/acpi-dev-tables.h
> 
> diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
> index b9fefa7..2b84f08 100644
> --- a/hw/acpi/Makefile.objs
> +++ b/hw/acpi/Makefile.objs
> @@ -3,3 +3,4 @@ common-obj-$(CONFIG_ACPI) += memory_hotplug.o
>  common-obj-$(CONFIG_ACPI) += acpi_interface.o
>  common-obj-$(CONFIG_ACPI) += bios-linker-loader.o
>  common-obj-$(CONFIG_ACPI) += aml-build.o
> +common-obj-$(CONFIG_ACPI) += acpi-dev-tables.o
> diff --git a/hw/acpi/acpi-dev-tables.c b/hw/acpi/acpi-dev-tables.c
> new file mode 100644
> index 0000000..7f07a3d
> --- /dev/null
> +++ b/hw/acpi/acpi-dev-tables.c
> @@ -0,0 +1,80 @@
> +/*
> + * Add and get ACPI tables registered by devices.
> + *
> + * Copyright (c) 2015 Corey Minyard, MontaVista Software, LLC
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#include <string.h>
> +#include <hw/acpi/acpi-dev-tables.h>
> +#include <qemu/queue.h>
> +
> +struct acpi_dev_table {
> +    void *data;
> +    int size;
> +    const char *sig;
> +    uint8_t rev;
> +    QSLIST_ENTRY(acpi_dev_table) next;
> +};
> +
> +static QSLIST_HEAD(, acpi_dev_table) acpi_table_entries;
> +
> +void
> +add_acpi_dev_table(void *data, int size, const char *sig, uint8_t rev)
> +{
> +    struct acpi_dev_table *e = g_malloc(sizeof(*e));
> +
> +    e->data = g_malloc(size);
> +    memcpy(e->data, data, size);
> +    e->size = size;
> +    e->sig = sig;
> +    e->rev = rev;
> +    QSLIST_INSERT_HEAD(&acpi_table_entries, e, next);
> +}
> +
> +struct acpi_dev_table *acpi_dev_table_first(void)
> +{
> +    return QSLIST_FIRST(&acpi_table_entries);
> +}
> +
> +struct acpi_dev_table *acpi_dev_table_next(struct acpi_dev_table *current)
> +{
> +    return QSLIST_NEXT(current, next);
> +}
> +
> +uint8_t *acpi_dev_table_data(struct acpi_dev_table *e)
> +{
> +    return e->data;
> +}
> +
> +unsigned acpi_dev_table_len(struct acpi_dev_table *e)
> +{
> +    return e->size;
> +}
> +
> +const char *acpi_dev_table_sig(struct acpi_dev_table *e)
> +{
> +    return e->sig;
> +}
> +
> +uint8_t acpi_dev_table_rev(struct acpi_dev_table *e)
> +{
> +    return e->rev;
> +}
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index e761005..0b4d195 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -35,6 +35,7 @@
>  #include "hw/timer/hpet.h"
>  #include "hw/i386/acpi-defs.h"
>  #include "hw/acpi/acpi.h"
> +#include "hw/acpi/acpi-dev-tables.h"
>  #include "hw/nvram/fw_cfg.h"
>  #include "hw/acpi/bios-linker-loader.h"
>  #include "hw/loader.h"
> @@ -1377,6 +1378,7 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
>      AcpiMcfgInfo mcfg;
>      PcPciInfo pci;
>      uint8_t *u;
> +    struct acpi_dev_table *dt;
>      size_t aml_len = 0;
>      GArray *tables_blob = tables->table_data;
>  
> @@ -1448,6 +1450,21 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
>          build_dmar_q35(tables_blob, tables->linker);
>      }
>  
> +    /* Add tables from devices. */
> +    for (dt = acpi_dev_table_first(); dt; dt = acpi_dev_table_next(dt)) {
> +        unsigned len = acpi_dev_table_len(dt);
> +        void *data = acpi_dev_table_data(dt);
> +
> +        acpi_add_table(table_offsets, tables_blob);
> +        acpi_data_push(tables_blob, sizeof(AcpiTableHeader));
> +        g_array_append_vals(tables_blob, data, len);
> +        build_header(tables->linker, tables_blob,
> +            (void *)(tables_blob->data + tables_blob->len -
> +                     len - sizeof(AcpiTableHeader)),
> +            acpi_dev_table_sig(dt), len + sizeof(AcpiTableHeader),
> +            acpi_dev_table_rev(dt));
> +    }
> +
>      /* Add tables supplied by user (if any) */
>      for (u = acpi_table_first(); u; u = acpi_table_next(u)) {
>          unsigned len = acpi_table_len(u);
> diff --git a/include/hw/acpi/acpi-dev-tables.h b/include/hw/acpi/acpi-dev-tables.h
> new file mode 100644
> index 0000000..6dd18a5
> --- /dev/null
> +++ b/include/hw/acpi/acpi-dev-tables.h
> @@ -0,0 +1,38 @@
> +/*
> + * Add and get ACPI tables registered by devices.
> + *
> + * Copyright (C) 2015  Corey Minyard <cminyard@mvista.com>
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License version 2 as published by the Free Software Foundation.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see <http://www.gnu.org/licenses/>
> + *
> + * Contributions after 2012-01-13 are licensed under the terms of the
> + * GNU GPL, version 2 or (at your option) any later version.
> + */
> +
> +#ifndef QEMU_HW_ACPI_HOOKS_H
> +#define QEMU_HW_ACPI_HOOKS_H
> +
> +#include <hw/acpi/aml-build.h>

you don't seem to do anything with this header.
why include it?

> +
> +void add_acpi_dev_table(void *data, int size, const char *sig, uint8_t rev);
> +
> +struct acpi_dev_table;

Any particular reason to violate the coding style with
the name of this structure.

> +
> +struct acpi_dev_table *acpi_dev_table_first(void);
> +struct acpi_dev_table *acpi_dev_table_next(struct acpi_dev_table *current);
> +uint8_t *acpi_dev_table_data(struct acpi_dev_table *);
> +unsigned acpi_dev_table_len(struct acpi_dev_table *);
> +const char *acpi_dev_table_sig(struct acpi_dev_table *);
> +uint8_t acpi_dev_table_rev(struct acpi_dev_table *);


Pls add documentation for the APIs.

> +
> +#endif /* QEMU_HW_ACPI_HOOKS_H */
> -- 
> 1.8.3.1
>