Re: [Qemu-devel] [PATCH 14/15] acpi: Add hooks for adding things to the SSDT table

[Corey Minyard]

On 04/12/2015 11:17 AM, Michael S. Tsirkin wrote:
> On Tue, Apr 07, 2015 at 02:51:43PM -0500, address@hidden wrote:
>> From: Corey Minyard <address@hidden>
>>
>> This way devices can tie in when then SSDT is built and can add their
>> own entries.  This didn't seem to fit anyplace else, primarily because it
>> required the Aml type, so I added a new file for it.
>>
>> Signed-off-by: Corey Minyard <address@hidden>
> Hmm, I don't see patch 15/15 so I don't know how this is used.

Yeah, I resent and I don't know what's happened.  All the others were
sent exactly the same way.  Something doesn't like that patch.

>
> Presumably devices register using add_device_ssdt_encoder?
> Can't they, instead, add their tables to some list at that point?

Not if they are adding to a common SSDT.

>
> It also would seem a bit easier to debug to have devices add their own
> tables rather than patch SSDT.
> Somewhere near the line
>     /* Add tables supplied by user (if any) */
> would be a good place to stick this.

So you are suggesting each device add it's own SSDT?  I'm not sure how
that helps debugging, it seems simpler to add it to a common one.  But
it's not a big deal either way.

-corey

>
>
>> ---
>>  hw/acpi/Makefile.objs        |  1 +
>>  hw/acpi/acpi-hooks.c         | 55 
>> ++++++++++++++++++++++++++++++++++++++++++++
>>  hw/i386/acpi-build.c         |  3 +++
>>  include/hw/acpi/acpi-hooks.h | 31 +++++++++++++++++++++++++
>>  4 files changed, 90 insertions(+)
>>  create mode 100644 hw/acpi/acpi-hooks.c
>>  create mode 100644 include/hw/acpi/acpi-hooks.h
>>
>> diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
>> index b9fefa7..f60b12a 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-hooks.o
>> diff --git a/hw/acpi/acpi-hooks.c b/hw/acpi/acpi-hooks.c
>> new file mode 100644
>> index 0000000..c499208
>> --- /dev/null
>> +++ b/hw/acpi/acpi-hooks.c
>> @@ -0,0 +1,55 @@
>> +/*
>> + * ACPI hooks for inserting table entries from devices into the SSDT table.
>> + *
>> + * 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 <hw/acpi/acpi-hooks.h>
>> +#include <qemu/queue.h>
>> +
>> +struct ssdt_device_encoder {
>> +    void (*encode)(Aml *ssdt, void *opaque);
>> +    void *opaque;
>> +    QSLIST_ENTRY(ssdt_device_encoder) next;
>> +};
>> +
>> +static QSLIST_HEAD(ssdt_device_encoders, ssdt_device_encoder)
>> +     ssdt_device_encoders = QSLIST_HEAD_INITIALIZER(&ssdt_device_encoders);
>> +
>> +void
>> +add_device_ssdt_encoder(void (*encode)(Aml *ssdt, void *opaque), void 
>> *opaque)
>> +{
>> +    struct ssdt_device_encoder *e = g_new0(struct ssdt_device_encoder, 1);
>> +
>> +    e->encode = encode;
>> +    e->opaque = opaque;
>> +    QSLIST_INSERT_HEAD(&ssdt_device_encoders, e, next);
>> +}
>> +
>> +void
>> +call_device_ssdt_encoders(Aml *ssdt)
>> +{
>> +    struct ssdt_device_encoder *e;
>> +
>> +    QSLIST_FOREACH(e, &ssdt_device_encoders, next) {
>> +        e->encode(ssdt, e->opaque);
>> +    }
>> +}
>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> index e761005..3a4b1ce 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-hooks.h"
>>  #include "hw/nvram/fw_cfg.h"
>>  #include "hw/acpi/bios-linker-loader.h"
>>  #include "hw/loader.h"
>> @@ -1008,6 +1009,8 @@ build_ssdt(GArray *table_data, GArray *linker,
>>          aml_append(ssdt, sb_scope);
>>      }
>>  
>> +    call_device_ssdt_encoders(ssdt);
>> +
>>      /* copy AML table into ACPI tables blob and patch header there */
>>      g_array_append_vals(table_data, ssdt->buf->data, ssdt->buf->len);
>>      build_header(linker, table_data,
>> diff --git a/include/hw/acpi/acpi-hooks.h b/include/hw/acpi/acpi-hooks.h
>> new file mode 100644
>> index 0000000..ae66925
>> --- /dev/null
>> +++ b/include/hw/acpi/acpi-hooks.h
>> @@ -0,0 +1,31 @@
>> +/*
>> + * Hooks for dynamically construct ACPI tables in devices
>> + *
>> + * Copyright (C) 2015  Corey Minyard <address@hidden>
>> + *
>> + * 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>
>> +
>> +void add_device_ssdt_encoder(void (*encode)(Aml *ssdt, void *opaque),
>> +                             void *opaque);
>> +void call_device_ssdt_encoders(Aml *ssdt);
>> +
>> +#endif /* QEMU_HW_ACPI_HOOKS_H */
>> -- 
>> 1.8.3.1
>>