From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36887) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YhTD9-0000JJ-QM for qemu-devel@nongnu.org; Sun, 12 Apr 2015 21:30:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YhTD6-00014Z-HL for qemu-devel@nongnu.org; Sun, 12 Apr 2015 21:30:51 -0400 Received: from mail-vn0-x22c.google.com ([2607:f8b0:400c:c0f::22c]:47097) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YhTD6-00014V-Bb for qemu-devel@nongnu.org; Sun, 12 Apr 2015 21:30:48 -0400 Received: by vnbf62 with SMTP id f62so15125344vnb.13 for ; Sun, 12 Apr 2015 18:30:48 -0700 (PDT) Sender: Corey Minyard Message-ID: <552B1C45.1080304@acm.org> Date: Sun, 12 Apr 2015 20:30:45 -0500 From: Corey Minyard MIME-Version: 1.0 References: <1428436304-24044-1-git-send-email-minyard@acm.org> <1428436304-24044-15-git-send-email-minyard@acm.org> <20150412180834-mutt-send-email-mst@redhat.com> In-Reply-To: <20150412180834-mutt-send-email-mst@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 14/15] acpi: Add hooks for adding things to the SSDT table Reply-To: minyard@acm.org List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: Corey Minyard , qemu-devel@nongnu.org On 04/12/2015 11:17 AM, Michael S. Tsirkin wrote: > On Tue, Apr 07, 2015 at 02:51:43PM -0500, minyard@acm.org wrote: >> From: Corey Minyard >> >> 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 > 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 >> +#include >> + >> +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 >> + * >> + * 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 >> + * >> + * 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 >> + >> +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 >>