Re: [Qemu-devel] [SeaBIOS] [PATCH v2 4/5] acpi: load and link tables from /etc/acpi/

["Michael S. Tsirkin" <mst@redhat.com>]

On Sun, Jul 14, 2013 at 02:27:14PM -0400, Kevin O'Connor wrote:
> On Sun, Jul 07, 2013 at 06:42:43PM +0300, Michael S. Tsirkin wrote:
> > Load files in /etc/acpi/, link them using
> > a linker script and use for acpi tables, including the RSDP.
> > Presense of RSDP in this directory completely disables
> > generating and loading legacy acpi tables.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  src/acpi.c | 39 +++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 39 insertions(+)
> > 
> > diff --git a/src/acpi.c b/src/acpi.c
> > index a81f0cb..07d311a 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"
> >  
> > @@ -599,6 +600,34 @@ static const struct pci_device_id acpi_find_tbl[] = {
> >  
> >  struct rsdp_descriptor *RsdpAddr;
> >  
> > +/* Look for RSDP signature in files loaded in FSEG */
> > +struct rsdp_descriptor *
> > +acpi_find_rsdp_rom(void)
> > +{
> > +    struct romfile_s *file = NULL;
> > +    struct rsdp_descriptor *rsdp = NULL;
> > +    for (;;) {
> > +        file = romfile_findprefix("", file);
> > +        if (!file)
> > +            break;
> > +
> > +        if (!file->data || !pmm_test_fseg(file->data) ||
> > +            file->size < sizeof(rsdp->signature))
> > +            continue;
> > +
> > +        void *data;
> > +
> > +        for (data = file->data;
> > +             data + sizeof(*rsdp) <= file->data + file->size;
> > +             data++) {
> > +            rsdp = data;
> > +            if (rsdp->signature == cpu_to_le64(RSDP_SIGNATURE))
> > +                return rsdp;
> > +        }
> > +    }
> > +    return NULL;
> > +}
> > +
> >  #define MAX_ACPI_TABLES 20
> >  void
> >  acpi_setup(void)
> > @@ -608,6 +637,16 @@ acpi_setup(void)
> >  
> >      dprintf(3, "init ACPI tables\n");
> >  
> > +    linker_loader_execute("/etc/linker-script");
> > +
> > +    RsdpAddr = acpi_find_rsdp_rom();
> > +
> > +    if (RsdpAddr) {
> > +        return;
> > +    }
> 
> I don't understand this.  Why not use the presence of
> "/etc/linker-script" to determine if ACPI should be produced.

We can do this but what do we do if linker script is there
but no rsdp?
Maybe just add
	test -e /etc/linker-script != RsdpAddr
	        dprintf(1, "linker script found but no rsdp. Falling back on builtin tables\n");

Would this address your comment?


> Also, /etc/linker-script is not very informative - how about something
> like "/etc/biostables".
> 
> -Kevin

Hmm bios seems redundant, and tables makes one think it's the tables
themselves.

"/etc/acpi-linker"
"/etc/acpi-loader"?


-- 
MST