[Qemu-devel] [PATCH 1/2] Cleanup acpi table creation.

[Qemu-devel] [PATCH 1/2] Cleanup acpi table creation.

[Gleb Natapov]

Makes dynamic number of acpi table possible.

Signed-off-by: Gleb Natapov <gleb@redhat.com>
---
 src/acpi.c |   82 +++++++++++++++++++++++++++++++++--------------------------
 1 files changed, 46 insertions(+), 36 deletions(-)

diff --git a/src/acpi.c b/src/acpi.c
index d0c9201..b9d449f 100644
--- a/src/acpi.c
+++ b/src/acpi.c
@@ -43,7 +43,7 @@ struct acpi_table_header         /* ACPI common table header */
 struct rsdt_descriptor_rev1
 {
     ACPI_TABLE_HEADER_DEF       /* ACPI common table header */
-    u32 table_offset_entry[3];  /* Array of pointers to other */
+    u32 table_offset_entry[0];  /* Array of pointers to other */
     /* ACPI tables */
 } PACKED;
 
@@ -224,8 +224,7 @@ static inline u32 cpu_to_le32(u32 x)
 }
 
 static void
-build_header(struct acpi_table_header *h, u32 sig, int len, u8 rev
-             , struct rsdt_descriptor_rev1 *rsdt)
+build_header(struct acpi_table_header *h, u32 sig, int len, u8 rev)
 {
     h->signature = sig;
     h->length = cpu_to_le32(len);
@@ -237,21 +236,10 @@ build_header(struct acpi_table_header *h, u32 sig, int len, u8 rev
     h->oem_revision = cpu_to_le32(1);
     h->asl_compiler_revision = cpu_to_le32(1);
     h->checksum -= checksum(h, len);
-
-    // Add to rsdt table
-    if (!rsdt)
-        return;
-    if (rsdt->length >= sizeof(*rsdt)) {
-        dprintf(1, "No more room for rsdt entry!\n");
-        return;
-    }
-    u32 *p = (void*)rsdt + rsdt->length;
-    *p = (u32)h;
-    rsdt->length += sizeof(*p);
 }
 
-static void
-build_fadt(struct rsdt_descriptor_rev1 *rsdt, int bdf)
+static void*
+build_fadt(int bdf)
 {
     struct fadt_descriptor_rev1 *fadt = malloc_high(sizeof(*fadt));
     struct facs_descriptor_rev1 *facs = memalign_high(64, sizeof(*facs));
@@ -259,7 +247,7 @@ build_fadt(struct rsdt_descriptor_rev1 *rsdt, int bdf)
 
     if (!fadt || !facs || !dsdt) {
         dprintf(1, "Not enough memory for fadt!\n");
-        return;
+        return NULL;
     }
 
     /* FACS */
@@ -292,11 +280,13 @@ build_fadt(struct rsdt_descriptor_rev1 *rsdt, int bdf)
     /* WBINVD + PROC_C1 + PWR_BUTTON + SLP_BUTTON + FIX_RTC */
     fadt->flags = cpu_to_le32((1 << 0) | (1 << 2) | (1 << 4) | (1 << 5) | (1 << 6));
 
-    build_header((void*)fadt, FACP_SIGNATURE, sizeof(*fadt), 1, rsdt);
+    build_header((void*)fadt, FACP_SIGNATURE, sizeof(*fadt), 1);
+
+    return fadt;
 }
 
-static void
-build_madt(struct rsdt_descriptor_rev1 *rsdt)
+static void*
+build_madt(void)
 {
     int smp_cpus = CountCPUs;
     int madt_size = (sizeof(struct multiple_apic_table)
@@ -306,7 +296,7 @@ build_madt(struct rsdt_descriptor_rev1 *rsdt)
     struct multiple_apic_table *madt = malloc_high(madt_size);
     if (!madt) {
         dprintf(1, "Not enough memory for madt!\n");
-        return;
+        return NULL;
     }
     memset(madt, 0, madt_size);
     madt->local_apic_address = cpu_to_le32(BUILD_APIC_ADDR);
@@ -351,13 +341,13 @@ build_madt(struct rsdt_descriptor_rev1 *rsdt)
         intsrcovr++;
     }
 
-    build_header((void*)madt, APIC_SIGNATURE, (void*)intsrcovr - (void*)madt
-                 , 1, rsdt);
+    build_header((void*)madt, APIC_SIGNATURE, (void*)intsrcovr - (void*)madt, 1);
+    return madt;
 }
 
 #define SSDT_SIGNATURE 0x54445353 // SSDT
-static void
-build_ssdt(struct rsdt_descriptor_rev1 *rsdt)
+static void*
+build_ssdt(void)
 {
     int smp_cpus = CountCPUs;
     int acpi_cpus = smp_cpus > 0xff ? 0xff : smp_cpus;
@@ -369,7 +359,7 @@ build_ssdt(struct rsdt_descriptor_rev1 *rsdt)
     u8 *ssdt = malloc_high(length);
     if (! ssdt) {
         dprintf(1, "No space for ssdt!\n");
-        return;
+        return NULL;
     }
 
     u8 *ssdt_ptr = ssdt;
@@ -410,11 +400,14 @@ build_ssdt(struct rsdt_descriptor_rev1 *rsdt)
         *(ssdt_ptr++) = 6;    // Processor block length
     }
 
-    build_header((void*)ssdt, SSDT_SIGNATURE, ssdt_ptr - ssdt, 1, rsdt);
+    build_header((void*)ssdt, SSDT_SIGNATURE, ssdt_ptr - ssdt, 1);
+
+    return ssdt;
 }
 
 struct rsdp_descriptor *RsdpAddr;
 
+#define MAX_ACPI_TABLES 20
 void
 acpi_bios_init(void)
 {
@@ -432,20 +425,37 @@ acpi_bios_init(void)
 
     // Create initial rsdt table
     struct rsdp_descriptor *rsdp = malloc_fseg(sizeof(*rsdp));
-    struct rsdt_descriptor_rev1 *rsdt = malloc_high(sizeof(*rsdt));
-    if (!rsdp || !rsdt) {
-        dprintf(1, "Not enough memory for acpi rsdp/rsdt table!\n");
+    if (!rsdp) {
+        dprintf(1, "Not enough memory for acpi rsdp table!\n");
         return;
     }
-    memset(rsdt, 0, sizeof(*rsdt));
-    rsdt->length = offsetof(struct rsdt_descriptor_rev1, table_offset_entry[0]);
+
+    u32 tables[MAX_ACPI_TABLES], tbl_idx = 0;
+
+#define ACPI_INIT_TABLE(X)                                   \
+    do {                                                     \
+        tables[tbl_idx] = (u32)(X);                          \
+        if (tables[tbl_idx])                                 \
+            tbl_idx++;                                       \
+    } while(0)
 
     // Add tables
-    build_fadt(rsdt, bdf);
-    build_ssdt(rsdt);
-    build_madt(rsdt);
+    ACPI_INIT_TABLE(build_fadt(bdf));
+    ACPI_INIT_TABLE(build_ssdt());
+    ACPI_INIT_TABLE(build_madt());
+
+    struct rsdt_descriptor_rev1 *rsdt;
+    size_t rsdt_len = sizeof(*rsdt) + sizeof(u32) * tbl_idx;
+    rsdt = malloc_high(rsdt_len);
+
+    if (!rsdt) {
+        dprintf(1, "Not enough memory for acpi rsdt table!\n");
+        return;
+    }
+    memset(rsdt, 0, rsdt_len);
+    memcpy(rsdt->table_offset_entry, tables, sizeof(u32) * tbl_idx);
 
-    build_header((void*)rsdt, RSDT_SIGNATURE, rsdt->length, 1, NULL);
+    build_header((void*)rsdt, RSDT_SIGNATURE, rsdt_len, 1);
 
     // Build rsdp pointer table
     memset(rsdp, 0, sizeof(*rsdp));
-- 
1.6.3.3

[Qemu-devel] [PATCH 2/2] Add support for passing additional acpi tables from qemu.

[Gleb Natapov]

Signed-off-by: Gleb Natapov <gleb@redhat.com>
---
 src/acpi.c     |   18 +++++++++++++++++-
 src/config.h   |    2 +-
 src/paravirt.c |   27 +++++++++++++++++++++++++++
 src/paravirt.h |    3 +++
 4 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/src/acpi.c b/src/acpi.c
index b9d449f..dafd8c8 100644
--- a/src/acpi.c
+++ b/src/acpi.c
@@ -11,7 +11,7 @@
 #include "biosvar.h" // GET_EBDA
 #include "pci_ids.h" // PCI_VENDOR_ID_INTEL
 #include "pci_regs.h" // PCI_INTERRUPT_LINE
-
+#include "paravirt.h"
 
 /****************************************************/
 /* ACPI tables init */
@@ -444,6 +444,22 @@ acpi_bios_init(void)
     ACPI_INIT_TABLE(build_ssdt());
     ACPI_INIT_TABLE(build_madt());
 
+    u16 i, external_tables = qemu_cfg_acpi_additional_tables();
+
+    for(i = 0; i < external_tables; i++) {
+        u16 len = qemu_cfg_next_acpi_table_len();
+        void *addr = malloc_high(len);
+        if (!addr) {
+            dprintf(1, "Not enogh memory of ext acpi table of size %d!\n", len);
+            continue;
+        }
+        ACPI_INIT_TABLE(qemu_cfg_next_acpi_table_load(addr, len));
+        if (tbl_idx == MAX_ACPI_TABLES) {
+            dprintf(1, "To many external table!\n");
+            break;
+        }
+    }
+
     struct rsdt_descriptor_rev1 *rsdt;
     size_t rsdt_len = sizeof(*rsdt) + sizeof(u32) * tbl_idx;
     rsdt = malloc_high(rsdt_len);
diff --git a/src/config.h b/src/config.h
index e93b080..10b2232 100644
--- a/src/config.h
+++ b/src/config.h
@@ -13,7 +13,7 @@
 #define CONFIG_APPNAME4 "BXPC"
 
 // Configure for use with KVM.
-#define CONFIG_KVM 0
+#define CONFIG_KVM 1
 // Configure as a coreboot payload.
 #define CONFIG_COREBOOT 0
 
diff --git a/src/paravirt.c b/src/paravirt.c
index 56d8421..cd1f263 100644
--- a/src/paravirt.c
+++ b/src/paravirt.c
@@ -72,3 +72,30 @@ int qemu_cfg_show_boot_menu(void)
     return v;
 }
 
+u16 qemu_cfg_acpi_additional_tables(void)
+{
+    u16 cnt;
+
+    if (!qemu_cfg_present)
+        return 0;
+
+    qemu_cfg_read_entry(&cnt, QEMU_CFG_ACPI_TABLES, sizeof(cnt));
+
+    return cnt;
+}
+
+u16 qemu_cfg_next_acpi_table_len(void)
+{
+    u16 len;
+
+    qemu_cfg_read((u8*)&len, sizeof(len));
+
+    return len;
+}
+
+void* qemu_cfg_next_acpi_table_load(void *addr, u16 len)
+{
+    qemu_cfg_read(addr, len);
+    return addr;
+}
+
diff --git a/src/paravirt.h b/src/paravirt.h
index 6997cff..c2bab71 100644
--- a/src/paravirt.h
+++ b/src/paravirt.h
@@ -40,5 +40,8 @@ extern int qemu_cfg_present;
 void qemu_cfg_port_probe(void);
 int qemu_cfg_show_boot_menu(void);
 void qemu_cfg_get_uuid(u8 *uuid);
+u16 qemu_cfg_acpi_additional_tables(void);
+u16 qemu_cfg_next_acpi_table_len(void);
+void *qemu_cfg_next_acpi_table_load(void *addr, u16 len);
 
 #endif
-- 
1.6.3.3

[Qemu-devel] Re: [PATCH 1/2] Cleanup acpi table creation.

[Kevin O'Connor]

Thanks - commit a9242a2afb8865610cc359af118404f24decec37

On Wed, Oct 07, 2009 at 03:21:43PM +0200, Gleb Natapov wrote:
> Makes dynamic number of acpi table possible.
> 
> Signed-off-by: Gleb Natapov <gleb@redhat.com>

[Qemu-devel] Re: [PATCH 2/2] Add support for passing additional acpi tables from qemu.

[Kevin O'Connor]

On Wed, Oct 07, 2009 at 03:21:44PM +0200, Gleb Natapov wrote:
> --- a/src/config.h
> +++ b/src/config.h
> @@ -13,7 +13,7 @@
>  #define CONFIG_APPNAME4 "BXPC"
>  
>  // Configure for use with KVM.
> -#define CONFIG_KVM 0
> +#define CONFIG_KVM 1

Was this change intentional?  If so, please explain why it's needed.

I have noticed that qemu-0.11 only works properly if CONFIG_KVM is
enabled.  However, this option pulls in some settings that really are
specific to kvm.

Otherwise, the patch looks good to me.

-Kevin

[Qemu-devel] Re: [PATCH 2/2] Add support for passing additional acpi tables from qemu.

[Gleb Natapov]

On Wed, Oct 07, 2009 at 07:48:49PM -0400, Kevin O'Connor wrote:
> On Wed, Oct 07, 2009 at 03:21:44PM +0200, Gleb Natapov wrote:
> > --- a/src/config.h
> > +++ b/src/config.h
> > @@ -13,7 +13,7 @@
> >  #define CONFIG_APPNAME4 "BXPC"
> >  
> >  // Configure for use with KVM.
> > -#define CONFIG_KVM 0
> > +#define CONFIG_KVM 1
> 
> Was this change intentional?  If so, please explain why it's needed.
> 
It was not!

> I have noticed that qemu-0.11 only works properly if CONFIG_KVM is
> enabled.  However, this option pulls in some settings that really are
> specific to kvm.
> 
I plan to get rid of CONFIG_KVM entirely.

> Otherwise, the patch looks good to me.
> 
> -Kevin

--
			Gleb.