| Message ID | 1428436304-24044-15-git-send-email-minyard@acm.org |
|---|---|
| State | New |
| Headers | show |
On 07/04/2015 21:51, minyard@acm.org wrote: > From: Corey Minyard <cminyard@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 <cminyard@mvista.com> > --- > 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 <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> > + > +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 */ > I like this. Michael, can you please review patches 10/11/14 in this series? Paolo
On Tue, Apr 07, 2015 at 02:51:43PM -0500, minyard@acm.org wrote: > From: Corey Minyard <cminyard@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 <cminyard@mvista.com> Hmm, I don't see patch 15/15 so I don't know how this is used. Presumably devices register using add_device_ssdt_encoder? Can't they, instead, add their tables to some list at that point? 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. > --- > 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 <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> > + > +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 >
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 <cminyard@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 <cminyard@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. > > 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 <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> >> + >> +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 >>
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, minyard@acm.org wrote: > >> From: Corey Minyard <cminyard@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 <cminyard@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 <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> > >> + > >> +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 > >>
On 13/04/2015 08:36, Michael S. Tsirkin wrote: >> > 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. A unit test for what? Paolo
On Mon, Apr 13, 2015 at 10:39:39AM +0200, Paolo Bonzini wrote: > > > On 13/04/2015 08:36, Michael S. Tsirkin wrote: > >> > 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. > > A unit test for what? > > Paolo The ACPI code added. With an extra table it would be easy to test that it's there.
On 13/04/2015 03:30, Corey Minyard wrote: > > 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. Can you send that privately? Or put it somewhere on the web. Paolo
On 13/04/2015 13:32, Michael S. Tsirkin wrote: >>>> 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. >> >> A unit test for what? > > The ACPI code added. With an extra table it would be easy > to test that it's there. Ah, I agree that patch 15 should add something like commit 3a9c86df/71096d6c, but that doesn't need a separate table. Paolo
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 <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> + +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 */