Patchwork [RFC,2/3] acpi: load and link tables from /etc/acpi/

login
register
mail settings
Submitter Michael S. Tsirkin
Date April 25, 2013, 9:02 a.m.
Message ID <4fd91502526b9977807a8dff28dcba4db670af7c.1366879705.git.mst@redhat.com>
Download mbox | patch
Permalink /patch/239437/
State New
Headers show

Comments

Michael S. Tsirkin - April 25, 2013, 9:02 a.m.
Load files in /etc/acpi/ and use for acpi tables.
Any files in this directory completely disable
generating and loading legacy acpi tables.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 src/acpi.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 66 insertions(+), 1 deletion(-)
Laszlo Ersek - April 29, 2013, 11:41 a.m.
Not sure how much it counts, but I personally can agree with you on this
direction :)

One note below:

> @@ -603,8 +604,72 @@ acpi_setup(void)
>      if (! CONFIG_ACPI)
>          return;
>  
> +    int acpi_generate = 1;
> +
>      dprintf(3, "init ACPI tables\n");
>  
> +    struct romfile_s *file = NULL;
> +    for (;;) {
> +        file = romfile_findprefix("/etc/acpi/", file);
> +        if (!file)
> +            break;
> +
> +        /*
> +         * Disable ACPI table generation. All ACPI tables must come from
> +         * etc/acpi/ romfile entries.
> +         */
> +        acpi_generate = 0;
> +

[...]

> +    }
> +
> +    linker_link("/etc/linker-script");
> +
> +    if (!acpi_generate) {
> +        return;
> +    }
> +
> +    dprintf(3, "generate ACPI tables\n");
> +
>      // This code is hardcoded for PIIX4 Power Management device.
>      struct pci_device *pci = pci_find_init_device(acpi_find_tbl, NULL);
>      if (!pci)

Are you deliberately calling linker_link() independently from
"acpi_generate"? My hunch is that linker_link() only makes sense if
acpi_generate == 0 (ie. qemu has passed down at least one table to
repoint some pointer to), but I may be missing a use case.

Thanks
Laszlo
Michael S. Tsirkin - April 29, 2013, 1:25 p.m.
On Mon, Apr 29, 2013 at 01:41:01PM +0200, Laszlo Ersek wrote:
> Not sure how much it counts, but I personally can agree with you on this
> direction :)
> 
> One note below:
> 
> > @@ -603,8 +604,72 @@ acpi_setup(void)
> >      if (! CONFIG_ACPI)
> >          return;
> >  
> > +    int acpi_generate = 1;
> > +
> >      dprintf(3, "init ACPI tables\n");
> >  
> > +    struct romfile_s *file = NULL;
> > +    for (;;) {
> > +        file = romfile_findprefix("/etc/acpi/", file);
> > +        if (!file)
> > +            break;
> > +
> > +        /*
> > +         * Disable ACPI table generation. All ACPI tables must come from
> > +         * etc/acpi/ romfile entries.
> > +         */
> > +        acpi_generate = 0;
> > +
> 
> [...]
> 
> > +    }
> > +
> > +    linker_link("/etc/linker-script");
> > +
> > +    if (!acpi_generate) {
> > +        return;
> > +    }
> > +
> > +    dprintf(3, "generate ACPI tables\n");
> > +
> >      // This code is hardcoded for PIIX4 Power Management device.
> >      struct pci_device *pci = pci_find_init_device(acpi_find_tbl, NULL);
> >      if (!pci)
> 
> Are you deliberately calling linker_link() independently from
> "acpi_generate"? My hunch is that linker_link() only makes sense if
> acpi_generate == 0 (ie. qemu has passed down at least one table to
> repoint some pointer to), but I may be missing a use case.
> 
> Thanks
> Laszlo

In theory linker code is unrelated to acpi, and host
can the linker to patch any romfile.
I could add if (!acpi_generate) but it just seems
like adding code to remove something potentially useful.

Patch

diff --git a/src/acpi.c b/src/acpi.c
index 58cd6d7..16ea9f4 100644
--- a/src/acpi.c
+++ b/src/acpi.c
@@ -27,6 +27,7 @@ 
 #include "config.h" // CONFIG_*
 #include "paravirt.h" // RamSize
 #include "dev-q35.h"
+#include "linker.h"
 
 #include "acpi-dsdt.hex"
 
@@ -603,8 +604,72 @@  acpi_setup(void)
     if (! CONFIG_ACPI)
         return;
 
+    int acpi_generate = 1;
+
     dprintf(3, "init ACPI tables\n");
 
+    struct romfile_s *file = NULL;
+    for (;;) {
+        file = romfile_findprefix("/etc/acpi/", file);
+        if (!file)
+            break;
+
+        /*
+         * Disable ACPI table generation. All ACPI tables must come from
+         * etc/acpi/ romfile entries.
+         */
+        acpi_generate = 0;
+
+        if (!file->size)
+            continue;
+
+        void *data = malloc_high(file->size);
+        if (!data) {
+            warn_noalloc();
+            break;
+        }
+        int ret = file->copy(file, data, file->size);
+        if (ret < file->size) {
+            free(data);
+            continue;
+        }
+        /* Handle RSDP and FACS quirks */
+        if (file->size >= 8) {
+            struct rsdp_descriptor *rsdp = data;
+            struct acpi_table_header *hdr = data;
+            /* RSDP is in FSEG memory. */
+            if (rsdp->signature == cpu_to_le64(RSDP_SIGNATURE)) {
+                data = malloc_fseg(file->size);
+                if (!data) {
+                    warn_noalloc();
+                    break;
+                }
+                memcpy(data, rsdp, file->size);
+                free(rsdp);
+                /* Store Rsdp pointer for use by find_fadt. */
+                RsdpAddr = data;
+            } else if (hdr->signature == FACS_SIGNATURE) {
+                /* FACS is aligned to a 64 bit boundary. */
+                data = memalign_high(64, file->size);
+                if (!data) {
+                    warn_noalloc();
+                    break;
+                }
+                memcpy(data, hdr, file->size);
+                free(hdr);
+            }
+        }
+        file->data = data;
+    }
+
+    linker_link("/etc/linker-script");
+
+    if (!acpi_generate) {
+        return;
+    }
+
+    dprintf(3, "generate ACPI tables\n");
+
     // This code is hardcoded for PIIX4 Power Management device.
     struct pci_device *pci = pci_find_init_device(acpi_find_tbl, NULL);
     if (!pci)
@@ -630,7 +695,7 @@  acpi_setup(void)
     if (pci->device == PCI_DEVICE_ID_INTEL_ICH9_LPC)
         ACPI_INIT_TABLE(build_mcfg_q35());
 
-    struct romfile_s *file = NULL;
+    file = NULL;
     for (;;) {
         file = romfile_findprefix("acpi/", file);
         if (!file)