On Sun, Apr 12, 2015 at 08:30:45PM -0500, Corey Minyard wrote:
> On 04/12/2015 11:17 AM, Michael S. Tsirkin wrote:
> > On Tue, Apr 07, 2015 at 02:51:43PM -0500, miny...@acm.org wrote:
> >> From: Corey Minyard <cminy...@mvista.com>
> >>
> >> 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 <cminy...@mvista.com>
> > 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.
Why not? Just walk the list after building the rest of the 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.
For example, it's easier to write a unit test: encode something specific
in one of the IDs, then look it up.
Which is, btw, something that's missing in this patchset.
> But
> it's not a big deal either way.
>
> -corey
Yes, there isn't a lot of difference really. Seems slightly neater.
> >
> >
> >> ---
> >> 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 <cminy...@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>
> >> +
> >> +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
> >>
