Patchwork [v2,20/21] q35: automatically load the q35 dsdt table

login
register
mail settings
Submitter Jason Baron
Date Oct. 9, 2012, 3:30 a.m.
Message ID <9cdff450169c1a7956528afe8be3a4caa592d2ab.1349749915.git.jbaron@redhat.com>
Download mbox | patch
Permalink /patch/190212/
State New
Headers show

Comments

Jason Baron - Oct. 9, 2012, 3:30 a.m.
From: Jason Baron <jbaron@redhat.com>

Automatically, locate the required q35 dsdt table on load. Otherwise we error
out. This could be done in the bios, but its harder to produce a good error
message.

Signed-off-by: Jason Baron <jbaron@redhat.com>
---
 hw/pc.c      |   19 +++++++++++++++++++
 hw/pc.h      |    2 ++
 hw/pc_piix.c |    7 +++++++
 hw/pc_q35.c  |    7 +++++++
 4 files changed, 35 insertions(+), 0 deletions(-)
Paolo Bonzini - Oct. 9, 2012, 8:02 a.m.
Il 09/10/2012 05:30, Jason Baron ha scritto:
> From: Jason Baron <jbaron@redhat.com>
> 
> Automatically, locate the required q35 dsdt table on load. Otherwise we error
> out. This could be done in the bios, but its harder to produce a good error
> message.

Not just for q35, so the commit message is wrong, but the patch is
otherwise good.

We should import the source code for the DSDT from SeaBIOS, and support
building it + installing it.  You made this a hard failure, so it has to
be done before this patch goes in, doesn't it?

Paolo

> Signed-off-by: Jason Baron <jbaron@redhat.com>
> ---
>  hw/pc.c      |   19 +++++++++++++++++++
>  hw/pc.h      |    2 ++
>  hw/pc_piix.c |    7 +++++++
>  hw/pc_q35.c  |    7 +++++++
>  4 files changed, 35 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/pc.c b/hw/pc.c
> index f5fbd0c..4ee41a1 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -1172,3 +1172,22 @@ void ioapic_init_gsi(GSIState *gsi_state, const char *parent_name)
>          gsi_state->ioapic_irq[i] = qdev_get_gpio_in(dev, i);
>      }
>  }
> +
> +int find_and_load_dsdt(const char *dsdt_name)
> +{
> +    char *filename;
> +    char buf[1024];
> +
> +    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, dsdt_name);
> +    if (!filename) {
> +        return -1;
> +    }
> +    snprintf(buf, 1024, "file=%s", filename);
> +    g_free(filename);
> +    if (acpi_table_add(buf) < 0) {
> +        fprintf(stderr, "Wrong acpi table provided\n");
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> diff --git a/hw/pc.h b/hw/pc.h
> index 125c1fd..5e93ae1 100644
> --- a/hw/pc.h
> +++ b/hw/pc.h
> @@ -227,5 +227,7 @@ void pc_system_firmware_init(MemoryRegion *rom_memory);
>  #define E820_UNUSABLE   5
>  
>  int e820_add_entry(uint64_t, uint64_t, uint32_t);
> +int find_and_load_dsdt(const char *dsdt_name);
> +
>  
>  #endif
> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> index e133630..06bb40e 100644
> --- a/hw/pc_piix.c
> +++ b/hw/pc_piix.c
> @@ -126,6 +126,13 @@ static void pc_init1(MemoryRegion *system_memory,
>      MemoryRegion *rom_memory;
>      void *fw_cfg = NULL;
>  
> +    /* let's first see if we can find the proper dsdt */
> +    if (find_and_load_dsdt("acpi-dsdt.aml")) {
> +        fprintf(stderr, "Couldn't find piix dsdt table!\n"
> +                        "Try updating your bios.\n");
> +        exit(1);
> +    }
> +
>      pc_cpus_init(cpu_model);
>  
>      if (kvmclock_enabled) {
> diff --git a/hw/pc_q35.c b/hw/pc_q35.c
> index 48083bb..1f96af0 100644
> --- a/hw/pc_q35.c
> +++ b/hw/pc_q35.c
> @@ -356,6 +356,13 @@ static void pc_q35_init(ram_addr_t ram_size,
>      qemu_irq *i8259;
>      int i;
>  
> +    /* let's first see if we can find the proper dsdt */
> +    if (find_and_load_dsdt("q35-acpi-dsdt.aml")) {
> +        fprintf(stderr, "Couldn't find q35 dsdt table!\n"
> +                        "Try updating your bios.\n");
> +        exit(1);
> +    }
> +
>      pc_cpus_init(cpu_model);
>  
>      kvmclock_create();
>
Paolo Bonzini - Oct. 9, 2012, 8:29 a.m.
Il 09/10/2012 10:02, Paolo Bonzini ha scritto:
> Il 09/10/2012 05:30, Jason Baron ha scritto:
>> From: Jason Baron <jbaron@redhat.com>
>>
>> Automatically, locate the required q35 dsdt table on load. Otherwise we error
>> out. This could be done in the bios, but its harder to produce a good error
>> message.
> 
> Not just for q35, so the commit message is wrong, but the patch is
> otherwise good.
> 
> We should import the source code for the DSDT from SeaBIOS, and support
> building it + installing it.  You made this a hard failure, so it has to
> be done before this patch goes in, doesn't it?

Ah, having reviewed the SeaBIOS series I now see that you're keeping the
code in SeaBIOS but installing it alongside bios.bin.  This makes sense
since, at least for backwards compatibility, the PIIX DSDT will remain
in bios.bin for a while.

However, you still need to patch QEMU's roms/Makefile to copy the .aml
files to pc-bios, and the Makefile to install it (in both cases, just
grep for bios.bin).

Paolo

> 
> Paolo
> 
>> Signed-off-by: Jason Baron <jbaron@redhat.com>
>> ---
>>  hw/pc.c      |   19 +++++++++++++++++++
>>  hw/pc.h      |    2 ++
>>  hw/pc_piix.c |    7 +++++++
>>  hw/pc_q35.c  |    7 +++++++
>>  4 files changed, 35 insertions(+), 0 deletions(-)
>>
>> diff --git a/hw/pc.c b/hw/pc.c
>> index f5fbd0c..4ee41a1 100644
>> --- a/hw/pc.c
>> +++ b/hw/pc.c
>> @@ -1172,3 +1172,22 @@ void ioapic_init_gsi(GSIState *gsi_state, const char *parent_name)
>>          gsi_state->ioapic_irq[i] = qdev_get_gpio_in(dev, i);
>>      }
>>  }
>> +
>> +int find_and_load_dsdt(const char *dsdt_name)
>> +{
>> +    char *filename;
>> +    char buf[1024];
>> +
>> +    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, dsdt_name);
>> +    if (!filename) {
>> +        return -1;
>> +    }
>> +    snprintf(buf, 1024, "file=%s", filename);
>> +    g_free(filename);
>> +    if (acpi_table_add(buf) < 0) {
>> +        fprintf(stderr, "Wrong acpi table provided\n");
>> +        return -1;
>> +    }
>> +
>> +    return 0;
>> +}
>> diff --git a/hw/pc.h b/hw/pc.h
>> index 125c1fd..5e93ae1 100644
>> --- a/hw/pc.h
>> +++ b/hw/pc.h
>> @@ -227,5 +227,7 @@ void pc_system_firmware_init(MemoryRegion *rom_memory);
>>  #define E820_UNUSABLE   5
>>  
>>  int e820_add_entry(uint64_t, uint64_t, uint32_t);
>> +int find_and_load_dsdt(const char *dsdt_name);
>> +
>>  
>>  #endif
>> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
>> index e133630..06bb40e 100644
>> --- a/hw/pc_piix.c
>> +++ b/hw/pc_piix.c
>> @@ -126,6 +126,13 @@ static void pc_init1(MemoryRegion *system_memory,
>>      MemoryRegion *rom_memory;
>>      void *fw_cfg = NULL;
>>  
>> +    /* let's first see if we can find the proper dsdt */
>> +    if (find_and_load_dsdt("acpi-dsdt.aml")) {
>> +        fprintf(stderr, "Couldn't find piix dsdt table!\n"
>> +                        "Try updating your bios.\n");
>> +        exit(1);
>> +    }
>> +
>>      pc_cpus_init(cpu_model);
>>  
>>      if (kvmclock_enabled) {
>> diff --git a/hw/pc_q35.c b/hw/pc_q35.c
>> index 48083bb..1f96af0 100644
>> --- a/hw/pc_q35.c
>> +++ b/hw/pc_q35.c
>> @@ -356,6 +356,13 @@ static void pc_q35_init(ram_addr_t ram_size,
>>      qemu_irq *i8259;
>>      int i;
>>  
>> +    /* let's first see if we can find the proper dsdt */
>> +    if (find_and_load_dsdt("q35-acpi-dsdt.aml")) {
>> +        fprintf(stderr, "Couldn't find q35 dsdt table!\n"
>> +                        "Try updating your bios.\n");
>> +        exit(1);
>> +    }
>> +
>>      pc_cpus_init(cpu_model);
>>  
>>      kvmclock_create();
>>
> 
> 
>
Jason Baron - Oct. 9, 2012, 8:06 p.m.
On Tue, Oct 09, 2012 at 10:02:50AM +0200, Paolo Bonzini wrote:
> Il 09/10/2012 05:30, Jason Baron ha scritto:
> > From: Jason Baron <jbaron@redhat.com>
> > 
> > Automatically, locate the required q35 dsdt table on load. Otherwise we error
> > out. This could be done in the bios, but its harder to produce a good error
> > message.
> 
> Not just for q35, so the commit message is wrong, but the patch is
> otherwise good.
> 
> We should import the source code for the DSDT from SeaBIOS, and support
> building it + installing it.  You made this a hard failure, so it has to
> be done before this patch goes in, doesn't it?
> 

Right, so perhaps this patch is held off until the SeaBIOS changes are
in place, and q35 can still be invoked via -acpitable file=<q35 dsdttable>,
in the meantime.

On the other hand, this patch will work with the '-L' option supplied to
the updated SeaBIOS dir, without the .aml built into qemu. That seems
reasonable to me as well.

Thanks,

-Jason


> Paolo
> 
> > Signed-off-by: Jason Baron <jbaron@redhat.com>
> > ---
> >  hw/pc.c      |   19 +++++++++++++++++++
> >  hw/pc.h      |    2 ++
> >  hw/pc_piix.c |    7 +++++++
> >  hw/pc_q35.c  |    7 +++++++
> >  4 files changed, 35 insertions(+), 0 deletions(-)
> > 
> > diff --git a/hw/pc.c b/hw/pc.c
> > index f5fbd0c..4ee41a1 100644
> > --- a/hw/pc.c
> > +++ b/hw/pc.c
> > @@ -1172,3 +1172,22 @@ void ioapic_init_gsi(GSIState *gsi_state, const char *parent_name)
> >          gsi_state->ioapic_irq[i] = qdev_get_gpio_in(dev, i);
> >      }
> >  }
> > +
> > +int find_and_load_dsdt(const char *dsdt_name)
> > +{
> > +    char *filename;
> > +    char buf[1024];
> > +
> > +    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, dsdt_name);
> > +    if (!filename) {
> > +        return -1;
> > +    }
> > +    snprintf(buf, 1024, "file=%s", filename);
> > +    g_free(filename);
> > +    if (acpi_table_add(buf) < 0) {
> > +        fprintf(stderr, "Wrong acpi table provided\n");
> > +        return -1;
> > +    }
> > +
> > +    return 0;
> > +}
> > diff --git a/hw/pc.h b/hw/pc.h
> > index 125c1fd..5e93ae1 100644
> > --- a/hw/pc.h
> > +++ b/hw/pc.h
> > @@ -227,5 +227,7 @@ void pc_system_firmware_init(MemoryRegion *rom_memory);
> >  #define E820_UNUSABLE   5
> >  
> >  int e820_add_entry(uint64_t, uint64_t, uint32_t);
> > +int find_and_load_dsdt(const char *dsdt_name);
> > +
> >  
> >  #endif
> > diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> > index e133630..06bb40e 100644
> > --- a/hw/pc_piix.c
> > +++ b/hw/pc_piix.c
> > @@ -126,6 +126,13 @@ static void pc_init1(MemoryRegion *system_memory,
> >      MemoryRegion *rom_memory;
> >      void *fw_cfg = NULL;
> >  
> > +    /* let's first see if we can find the proper dsdt */
> > +    if (find_and_load_dsdt("acpi-dsdt.aml")) {
> > +        fprintf(stderr, "Couldn't find piix dsdt table!\n"
> > +                        "Try updating your bios.\n");
> > +        exit(1);
> > +    }
> > +
> >      pc_cpus_init(cpu_model);
> >  
> >      if (kvmclock_enabled) {
> > diff --git a/hw/pc_q35.c b/hw/pc_q35.c
> > index 48083bb..1f96af0 100644
> > --- a/hw/pc_q35.c
> > +++ b/hw/pc_q35.c
> > @@ -356,6 +356,13 @@ static void pc_q35_init(ram_addr_t ram_size,
> >      qemu_irq *i8259;
> >      int i;
> >  
> > +    /* let's first see if we can find the proper dsdt */
> > +    if (find_and_load_dsdt("q35-acpi-dsdt.aml")) {
> > +        fprintf(stderr, "Couldn't find q35 dsdt table!\n"
> > +                        "Try updating your bios.\n");
> > +        exit(1);
> > +    }
> > +
> >      pc_cpus_init(cpu_model);
> >  
> >      kvmclock_create();
> > 
>
Blue Swirl - Oct. 13, 2012, 8:33 a.m.
On Tue, Oct 9, 2012 at 3:30 AM, Jason Baron <jbaron@redhat.com> wrote:
> From: Jason Baron <jbaron@redhat.com>
>
> Automatically, locate the required q35 dsdt table on load. Otherwise we error
> out. This could be done in the bios, but its harder to produce a good error
> message.
>
> Signed-off-by: Jason Baron <jbaron@redhat.com>
> ---
>  hw/pc.c      |   19 +++++++++++++++++++
>  hw/pc.h      |    2 ++
>  hw/pc_piix.c |    7 +++++++
>  hw/pc_q35.c  |    7 +++++++
>  4 files changed, 35 insertions(+), 0 deletions(-)
>
> diff --git a/hw/pc.c b/hw/pc.c
> index f5fbd0c..4ee41a1 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -1172,3 +1172,22 @@ void ioapic_init_gsi(GSIState *gsi_state, const char *parent_name)
>          gsi_state->ioapic_irq[i] = qdev_get_gpio_in(dev, i);
>      }
>  }
> +
> +int find_and_load_dsdt(const char *dsdt_name)
> +{
> +    char *filename;
> +    char buf[1024];
> +
> +    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, dsdt_name);
> +    if (!filename) {
> +        return -1;
> +    }
> +    snprintf(buf, 1024, "file=%s", filename);

Please use sizeof(buf) instead of 1024, or introduce a constant.

> +    g_free(filename);
> +    if (acpi_table_add(buf) < 0) {
> +        fprintf(stderr, "Wrong acpi table provided\n");
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> diff --git a/hw/pc.h b/hw/pc.h
> index 125c1fd..5e93ae1 100644
> --- a/hw/pc.h
> +++ b/hw/pc.h
> @@ -227,5 +227,7 @@ void pc_system_firmware_init(MemoryRegion *rom_memory);
>  #define E820_UNUSABLE   5
>
>  int e820_add_entry(uint64_t, uint64_t, uint32_t);
> +int find_and_load_dsdt(const char *dsdt_name);
> +
>
>  #endif
> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> index e133630..06bb40e 100644
> --- a/hw/pc_piix.c
> +++ b/hw/pc_piix.c
> @@ -126,6 +126,13 @@ static void pc_init1(MemoryRegion *system_memory,
>      MemoryRegion *rom_memory;
>      void *fw_cfg = NULL;
>
> +    /* let's first see if we can find the proper dsdt */
> +    if (find_and_load_dsdt("acpi-dsdt.aml")) {
> +        fprintf(stderr, "Couldn't find piix dsdt table!\n"
> +                        "Try updating your bios.\n");
> +        exit(1);
> +    }
> +
>      pc_cpus_init(cpu_model);
>
>      if (kvmclock_enabled) {
> diff --git a/hw/pc_q35.c b/hw/pc_q35.c
> index 48083bb..1f96af0 100644
> --- a/hw/pc_q35.c
> +++ b/hw/pc_q35.c
> @@ -356,6 +356,13 @@ static void pc_q35_init(ram_addr_t ram_size,
>      qemu_irq *i8259;
>      int i;
>
> +    /* let's first see if we can find the proper dsdt */
> +    if (find_and_load_dsdt("q35-acpi-dsdt.aml")) {
> +        fprintf(stderr, "Couldn't find q35 dsdt table!\n"
> +                        "Try updating your bios.\n");
> +        exit(1);
> +    }
> +
>      pc_cpus_init(cpu_model);
>
>      kvmclock_create();
> --
> 1.7.1
>

Patch

diff --git a/hw/pc.c b/hw/pc.c
index f5fbd0c..4ee41a1 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -1172,3 +1172,22 @@  void ioapic_init_gsi(GSIState *gsi_state, const char *parent_name)
         gsi_state->ioapic_irq[i] = qdev_get_gpio_in(dev, i);
     }
 }
+
+int find_and_load_dsdt(const char *dsdt_name)
+{
+    char *filename;
+    char buf[1024];
+
+    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, dsdt_name);
+    if (!filename) {
+        return -1;
+    }
+    snprintf(buf, 1024, "file=%s", filename);
+    g_free(filename);
+    if (acpi_table_add(buf) < 0) {
+        fprintf(stderr, "Wrong acpi table provided\n");
+        return -1;
+    }
+
+    return 0;
+}
diff --git a/hw/pc.h b/hw/pc.h
index 125c1fd..5e93ae1 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -227,5 +227,7 @@  void pc_system_firmware_init(MemoryRegion *rom_memory);
 #define E820_UNUSABLE   5
 
 int e820_add_entry(uint64_t, uint64_t, uint32_t);
+int find_and_load_dsdt(const char *dsdt_name);
+
 
 #endif
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index e133630..06bb40e 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -126,6 +126,13 @@  static void pc_init1(MemoryRegion *system_memory,
     MemoryRegion *rom_memory;
     void *fw_cfg = NULL;
 
+    /* let's first see if we can find the proper dsdt */
+    if (find_and_load_dsdt("acpi-dsdt.aml")) {
+        fprintf(stderr, "Couldn't find piix dsdt table!\n"
+                        "Try updating your bios.\n");
+        exit(1);
+    }
+
     pc_cpus_init(cpu_model);
 
     if (kvmclock_enabled) {
diff --git a/hw/pc_q35.c b/hw/pc_q35.c
index 48083bb..1f96af0 100644
--- a/hw/pc_q35.c
+++ b/hw/pc_q35.c
@@ -356,6 +356,13 @@  static void pc_q35_init(ram_addr_t ram_size,
     qemu_irq *i8259;
     int i;
 
+    /* let's first see if we can find the proper dsdt */
+    if (find_and_load_dsdt("q35-acpi-dsdt.aml")) {
+        fprintf(stderr, "Couldn't find q35 dsdt table!\n"
+                        "Try updating your bios.\n");
+        exit(1);
+    }
+
     pc_cpus_init(cpu_model);
 
     kvmclock_create();