[Qemu-devel] [PATCH 0/7] abstract chipset(i440fx) specific register operation.

[Qemu-devel] [PATCH 0/7] abstract chipset(i440fx) specific register operation.

[Isaku Yamahata]

This patch set abstract out chipset specific operation,
and spit out i440fx specific operation into dev-i440fx.c with it.
Thus q35 specific register value/operation will be added easily.

Isaku Yamahata (7):
  seabios: pci: introduce helper function to find device from table and
    initialize it.
  seabios: shadow: make device finding more generic.
  seabios: smm: move out piix4 specific logic to dev-i440fx.c
  seabios: smm_init: move out i440fx smram operation into dev-i440fx.c
  seabios: acpi: move acpi definitions to acpi.h from acpi.c
  seabios: acpi: split out piix4 pm logic.
  seabios: acpi: clean up of finding pm device.

 src/acpi.c       |  102 +++++++++--------------------------------------------
 src/acpi.h       |   81 ++++++++++++++++++++++++++++++++++++++++++
 src/dev-i440fx.c |   57 ++++++++++++++++++++++++++++++
 src/dev-i440fx.h |    4 ++
 src/pci.c        |   12 ++++++
 src/pci.h        |    1 +
 src/post.h       |   24 +++++++++++++
 src/shadow.c     |   44 +++++++++++++++--------
 src/smm.c        |   35 +++++++++++++-----
 9 files changed, 251 insertions(+), 109 deletions(-)
 create mode 100644 src/post.h

[Qemu-devel] [PATCH 1/7] seabios: pci: introduce helper function to find device from table and initialize it.

[Isaku Yamahata]

introduce helper function to find device from table and initialize it.
pci_find_init_device(). This will be used later.

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
 src/pci.c |   12 ++++++++++++
 src/pci.h |    1 +
 2 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/src/pci.c b/src/pci.c
index c54b084..611d0e2 100644
--- a/src/pci.c
+++ b/src/pci.c
@@ -203,3 +203,15 @@ int pci_init_device(const struct pci_device_id *ids, u16 bdf, void *arg)
     }
     return -1;
 }
+
+int pci_find_init_device(const struct pci_device_id *ids, void *arg)
+{
+    int bdf, max;
+
+    foreachpci(bdf, max) {
+        if (pci_init_device(ids, bdf, arg) == 0) {
+            return bdf;
+        }
+    }
+    return -1;
+}
diff --git a/src/pci.h b/src/pci.h
index fa6a32d..9c3108c 100644
--- a/src/pci.h
+++ b/src/pci.h
@@ -93,6 +93,7 @@ struct pci_device_id {
     }
 
 int pci_init_device(const struct pci_device_id *table, u16 bdf, void *arg);
+int pci_find_init_device(const struct pci_device_id *ids, void *arg);
 
 // pirtable.c
 void create_pirtable(void);
-- 
1.7.1.1

[Qemu-devel] [PATCH 2/7] seabios: shadow: make device finding more generic.

[Isaku Yamahata]

pam register offset is north bridge specific.
So determine the offset based on found north bridge.

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
 src/dev-i440fx.c |    9 +++++++++
 src/dev-i440fx.h |    1 +
 src/post.h       |   13 +++++++++++++
 src/shadow.c     |   44 +++++++++++++++++++++++++++++---------------
 4 files changed, 52 insertions(+), 15 deletions(-)
 create mode 100644 src/post.h

diff --git a/src/dev-i440fx.c b/src/dev-i440fx.c
index 864a52c..5961efc 100644
--- a/src/dev-i440fx.c
+++ b/src/dev-i440fx.c
@@ -14,8 +14,17 @@
 #include "ioport.h" // outb
 #include "pci.h" // pci_config_writeb
 #include "pci_regs.h" // PCI_INTERRUPT_LINE
+#include "post.h"
 #include "dev-i440fx.h"
 
+#define I440FX_PAM0     0x59
+
+void i440fx_shadow_detected(u16 bdf, void *arg)
+{
+    struct pam_regs *pam_regs = arg;
+    pam_regs->pam0 = I440FX_PAM0;
+}
+
 /* PIIX3/PIIX4 PCI to ISA bridge */
 void piix_isa_bridge_init(u16 bdf, void *arg)
 {
diff --git a/src/dev-i440fx.h b/src/dev-i440fx.h
index ded5740..abcd0d1 100644
--- a/src/dev-i440fx.h
+++ b/src/dev-i440fx.h
@@ -3,6 +3,7 @@
 
 #include "types.h" // u16
 
+void i440fx_shadow_detected(u16 bdf, void *arg);
 void piix_isa_bridge_init(u16 bdf, void *arg);
 void piix_ide_init(u16 bdf, void *arg);
 void piix4_pm_init(u16 bdf, void *arg);
diff --git a/src/post.h b/src/post.h
new file mode 100644
index 0000000..18f89fb
--- /dev/null
+++ b/src/post.h
@@ -0,0 +1,13 @@
+//
+// Copyright (C) 2010 Isaku Yamahata <yamahata at valinux co jp>
+//
+// This file may be distributed under the terms of the GNU LGPLv3 license.
+#ifndef __POST_H
+#define __POST_H
+
+struct pam_regs
+{
+    u32 pam0;
+};
+
+#endif /* __POST_H */
diff --git a/src/shadow.c b/src/shadow.c
index 978424e..2a360a8 100644
--- a/src/shadow.c
+++ b/src/shadow.c
@@ -9,6 +9,8 @@
 #include "pci.h" // pci_config_writeb
 #include "config.h" // CONFIG_*
 #include "pci_ids.h" // PCI_VENDOR_ID_INTEL
+#include "post.h"
+#include "dev-i440fx.h"
 
 // Test if 'addr' is in the range from 'start'..'start+size'
 #define IN_RANGE(addr, start, size) ({   \
@@ -18,35 +20,42 @@
             (__addr - __start < __size); \
         })
 
+static const struct pci_device_id dram_controller_tbl[] = {
+    PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82441,
+               i440fx_shadow_detected),
+    PCI_DEVICE_END
+};
+
 // On the emulators, the bios at 0xf0000 is also at 0xffff0000
 #define BIOS_SRC_ADDR 0xffff0000
 
 // Enable shadowing and copy bios.
 static void
-__make_bios_writable(u16 bdf)
+__make_bios_writable(u16 bdf, u32 pam0)
 {
     // Make ram from 0xc0000-0xf0000 writable
     int clear = 0;
     int i;
     for (i=0; i<6; i++) {
-        int reg = pci_config_readb(bdf, 0x5a + i);
+      u32 pam = pam0 + 1 + i;
+      int reg = pci_config_readb(bdf, pam);
         if ((reg & 0x11) != 0x11) {
             // Need to copy optionroms to work around qemu implementation
             void *mem = (void*)(BUILD_ROM_START + i * 32*1024);
             memcpy((void*)BUILD_BIOS_TMP_ADDR, mem, 32*1024);
-            pci_config_writeb(bdf, 0x5a + i, 0x33);
+            pci_config_writeb(bdf, pam, 0x33);
             memcpy(mem, (void*)BUILD_BIOS_TMP_ADDR, 32*1024);
             clear = 1;
         } else {
-            pci_config_writeb(bdf, 0x5a + i, 0x33);
+            pci_config_writeb(bdf, pam, 0x33);
         }
     }
     if (clear)
         memset((void*)BUILD_BIOS_TMP_ADDR, 0, 32*1024);
 
     // Make ram from 0xf0000-0x100000 writable
-    int reg = pci_config_readb(bdf, 0x59);
-    pci_config_writeb(bdf, 0x59, 0x30);
+    int reg = pci_config_readb(bdf, pam0);
+    pci_config_writeb(bdf, pam0, 0x30);
     if (reg & 0x10)
         // Ram already present.
         return;
@@ -64,26 +73,29 @@ make_bios_writable(void)
 
     dprintf(3, "enabling shadow ram\n");
 
+    // at this point, staticlly alloacted variable can't written.
+    // so stack should be used.
+    struct pam_regs pam_regs;
     // Locate chip controlling ram shadowing.
-    int bdf = pci_find_device(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82441);
+    int bdf = pci_find_init_device(dram_controller_tbl, &pam_regs);
     if (bdf < 0) {
         dprintf(1, "Unable to unlock ram - bridge not found\n");
         return;
     }
 
-    int reg = pci_config_readb(bdf, 0x59);
+    int reg = pci_config_readb(bdf, pam_regs.pam0);
     if (!(reg & 0x10)) {
         // QEMU doesn't fully implement the piix shadow capabilities -
         // if ram isn't backing the bios segment when shadowing is
         // disabled, the code itself wont be in memory.  So, run the
         // code from the high-memory flash location.
         u32 pos = (u32)__make_bios_writable - BUILD_BIOS_ADDR + BIOS_SRC_ADDR;
-        void (*func)(u16 bdf) = (void*)pos;
-        func(bdf);
+        void (*func)(u16 bdf, u32 pam0) = (void*)pos;
+        func(bdf, pam_regs.pam0);
         return;
     }
     // Ram already present - just enable writes
-    __make_bios_writable(bdf);
+    __make_bios_writable(bdf, pam_regs.pam0);
 }
 
 // Make the BIOS code segment area (0xf0000) read-only.
@@ -95,7 +107,8 @@ make_bios_readonly(void)
 
     dprintf(3, "locking shadow ram\n");
 
-    int bdf = pci_find_device(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82441);
+    struct pam_regs pam_regs;
+    int bdf = pci_find_init_device(dram_controller_tbl, &pam_regs);
     if (bdf < 0) {
         dprintf(1, "Unable to lock ram - bridge not found\n");
         return;
@@ -108,14 +121,15 @@ make_bios_readonly(void)
     int i;
     for (i=0; i<6; i++) {
         u32 mem = BUILD_ROM_START + i * 32*1024;
+        u32 pam = pam_regs.pam0 + 1 + i;
         if (RomEnd <= mem + 16*1024) {
             if (RomEnd > mem)
-                pci_config_writeb(bdf, 0x5a + i, 0x31);
+                pci_config_writeb(bdf, pam, 0x31);
             break;
         }
-        pci_config_writeb(bdf, 0x5a + i, 0x11);
+        pci_config_writeb(bdf, pam, 0x11);
     }
 
     // Write protect 0xf0000-0x100000
-    pci_config_writeb(bdf, 0x59, 0x10);
+    pci_config_writeb(bdf, pam_regs.pam0, 0x10);
 }
-- 
1.7.1.1

[Qemu-devel] Re: [SeaBIOS] [PATCH 2/7] seabios: shadow: make device finding more generic.

[Kevin O'Connor]

On Mon, Jul 12, 2010 at 08:47:47PM +0900, Isaku Yamahata wrote:
> pam register offset is north bridge specific.
> So determine the offset based on found north bridge.

Is it really just the offset that is north bridge specific?  I thought
the entire process was very north bridge specific.

If so, I don't think it makes sense to pass back the pam0 register -
instead the north bridge specific code should do the necessary work
(using helper functions if possible).

I have the same concern with part 3 and 4 of this series.

-Kevin

[Qemu-devel] Re: [SeaBIOS] [PATCH 2/7] seabios: shadow: make device finding more generic.

[Isaku Yamahata]

On Mon, Jul 12, 2010 at 08:59:14PM -0400, Kevin O'Connor wrote:
> On Mon, Jul 12, 2010 at 08:47:47PM +0900, Isaku Yamahata wrote:
> > pam register offset is north bridge specific.
> > So determine the offset based on found north bridge.
> 
> Is it really just the offset that is north bridge specific?  I thought
> the entire process was very north bridge specific.
>
> If so, I don't think it makes sense to pass back the pam0 register -
> instead the north bridge specific code should do the necessary work
> (using helper functions if possible).
> 
> I have the same concern with part 3 and 4 of this series.

I440fx and Q35 (all Intel chipset?) are similar in registers
which seabios programs, so I choice to abstract it at register offset level.
I don't expect that other vendor's chipset support is wanted.

If you want more high level abstract, I'll respin the patch set.
-- 
yamahata

[Qemu-devel] Re: [SeaBIOS] [PATCH 2/7] seabios: shadow: make device finding more generic.

[Kevin O'Connor]

On Tue, Jul 13, 2010 at 06:45:00PM +0900, Isaku Yamahata wrote:
> On Mon, Jul 12, 2010 at 08:59:14PM -0400, Kevin O'Connor wrote:
> > On Mon, Jul 12, 2010 at 08:47:47PM +0900, Isaku Yamahata wrote:
> > > pam register offset is north bridge specific.
> > > So determine the offset based on found north bridge.
> > 
> > Is it really just the offset that is north bridge specific?  I thought
> > the entire process was very north bridge specific.
> >
> > If so, I don't think it makes sense to pass back the pam0 register -
> > instead the north bridge specific code should do the necessary work
> > (using helper functions if possible).
> > 
> > I have the same concern with part 3 and 4 of this series.
> 
> I440fx and Q35 (all Intel chipset?) are similar in registers
> which seabios programs, so I choice to abstract it at register offset level.
> I don't expect that other vendor's chipset support is wanted.

Although it isn't currently used, the memory locking support is useful
on real machines too.  I'd prefer a solution that would work on both
qemu and real machines.

It's minor for part 2 of the series, but I found part 3/4 to be hard
to follow due to the way the flow of code jumps between machine
specific code in dev-i440fx.c and the smm code in smm.c.

> If you want more high level abstract, I'll respin the patch set.

I've been meaning to look through the full series of changes in your
repo, but have not yet had a chance to do so.  I hope to get to that
in the next few days.  Sorry for the delay.

-Kevin

[Qemu-devel] [PATCH 3/7] seabios: smm: move out piix4 specific logic to dev-i440fx.c

[Isaku Yamahata]

move out piix4 specific logic to dev-i440fx.c by using pci_find_init_device().

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
 src/dev-i440fx.c |   25 +++++++++++++++++++++++++
 src/dev-i440fx.h |    1 +
 src/post.h       |    6 ++++++
 src/smm.c        |   18 +++++++++++++-----
 4 files changed, 45 insertions(+), 5 deletions(-)

diff --git a/src/dev-i440fx.c b/src/dev-i440fx.c
index 5961efc..17d42ce 100644
--- a/src/dev-i440fx.c
+++ b/src/dev-i440fx.c
@@ -16,6 +16,7 @@
 #include "pci_regs.h" // PCI_INTERRUPT_LINE
 #include "post.h"
 #include "dev-i440fx.h"
+#include "post.h"
 
 #define I440FX_PAM0     0x59
 
@@ -64,3 +65,27 @@ void piix4_pm_init(u16 bdf, void *arg)
     pci_config_writel(bdf, 0x90, PORT_SMB_BASE | 1);
     pci_config_writeb(bdf, 0xd2, 0x09); /* enable SMBus io space */
 }
+
+#define PIIX_DEVACTB    0x58
+#define PIIX_APMC_EN    (1 << 25)
+
+static int piix4_apmc_is_enabled(u16 bdf)
+{
+    /* check if SMM init is already done */
+    u32 value = pci_config_readl(bdf, PIIX_DEVACTB);
+    return value & PIIX_APMC_EN;
+}
+
+static void piix4_apmc_enable(u16 bdf)
+{
+    /* enable SMI generation when writing to the APMC register */
+    u32 value = pci_config_readl(bdf, PIIX_DEVACTB);
+    pci_config_writel(bdf, PIIX_DEVACTB, value | PIIX_APMC_EN);
+}
+
+void piix4_apmc_detected(u16 bdf, void *arg)
+{
+    struct apmc_ops *ops = arg;
+    ops->is_enabled = piix4_apmc_is_enabled;
+    ops->enable = piix4_apmc_enable;
+}
diff --git a/src/dev-i440fx.h b/src/dev-i440fx.h
index abcd0d1..934e7f2 100644
--- a/src/dev-i440fx.h
+++ b/src/dev-i440fx.h
@@ -7,5 +7,6 @@ void i440fx_shadow_detected(u16 bdf, void *arg);
 void piix_isa_bridge_init(u16 bdf, void *arg);
 void piix_ide_init(u16 bdf, void *arg);
 void piix4_pm_init(u16 bdf, void *arg);
+void piix4_apmc_detected(u16 bdf, void *arg);
 
 #endif // __I440FX_H
diff --git a/src/post.h b/src/post.h
index 18f89fb..2996878 100644
--- a/src/post.h
+++ b/src/post.h
@@ -10,4 +10,10 @@ struct pam_regs
     u32 pam0;
 };
 
+struct apmc_ops
+{
+    int (*is_enabled)(u16 bdf);
+    void (*enable)(u16 bdf);
+};
+
 #endif /* __POST_H */
diff --git a/src/smm.c b/src/smm.c
index 3f53ef9..baa272f 100644
--- a/src/smm.c
+++ b/src/smm.c
@@ -10,6 +10,8 @@
 #include "config.h" // CONFIG_*
 #include "ioport.h" // outb
 #include "pci_ids.h" // PCI_VENDOR_ID_INTEL
+#include "post.h"
+#include "dev-i440fx.h"
 
 ASM32FLAT(
     ".global smm_relocation_start\n"
@@ -72,6 +74,13 @@ ASM32FLAT(
 extern u8 smm_relocation_start, smm_relocation_end;
 extern u8 smm_code_start, smm_code_end;
 
+static const struct pci_device_id apmc_ops_tbl[] = {
+    PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82371AB_3,
+               piix4_apmc_detected),
+
+    PCI_DEVICE_END,
+};
+
 void
 smm_init(void)
 {
@@ -84,8 +93,8 @@ smm_init(void)
     dprintf(3, "init smm\n");
 
     // This code is hardcoded for PIIX4 Power Management device.
-    int bdf = pci_find_device(PCI_VENDOR_ID_INTEL
-                              , PCI_DEVICE_ID_INTEL_82371AB_3);
+    struct apmc_ops apmc_ops;
+    int bdf = pci_find_init_device(apmc_ops_tbl, &apmc_ops);
     if (bdf < 0)
         // Device not found
         return;
@@ -95,8 +104,7 @@ smm_init(void)
         return;
 
     /* check if SMM init is already done */
-    u32 value = pci_config_readl(bdf, 0x58);
-    if (value & (1 << 25))
+    if (apmc_ops.is_enabled(bdf))
         return;
 
     /* enable the SMM memory window */
@@ -110,7 +118,7 @@ smm_init(void)
            &smm_relocation_end - &smm_relocation_start);
 
     /* enable SMI generation when writing to the APMC register */
-    pci_config_writel(bdf, 0x58, value | (1 << 25));
+    apmc_ops.enable(bdf);
 
     /* init APM status port */
     outb(0x01, PORT_SMI_STATUS);
-- 
1.7.1.1

[Qemu-devel] [PATCH 4/7] seabios: smm_init: move out i440fx smram operation into dev-i440fx.c

[Isaku Yamahata]

move out i440fx smram operation into dev-i440fx.c.

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
 src/dev-i440fx.c |    8 ++++++++
 src/dev-i440fx.h |    1 +
 src/post.h       |    5 +++++
 src/smm.c        |   17 ++++++++++++-----
 4 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/src/dev-i440fx.c b/src/dev-i440fx.c
index 17d42ce..c9e0c3e 100644
--- a/src/dev-i440fx.c
+++ b/src/dev-i440fx.c
@@ -26,6 +26,14 @@ void i440fx_shadow_detected(u16 bdf, void *arg)
     pam_regs->pam0 = I440FX_PAM0;
 }
 
+#define I440FX_SMRAM    0x72
+
+void i440fx_smram_detected(u16 bdf, void *arg)
+{
+    struct smram_regs *smram_regs = arg;
+    smram_regs->smram = I440FX_SMRAM;
+}
+
 /* PIIX3/PIIX4 PCI to ISA bridge */
 void piix_isa_bridge_init(u16 bdf, void *arg)
 {
diff --git a/src/dev-i440fx.h b/src/dev-i440fx.h
index 934e7f2..6250c5e 100644
--- a/src/dev-i440fx.h
+++ b/src/dev-i440fx.h
@@ -4,6 +4,7 @@
 #include "types.h" // u16
 
 void i440fx_shadow_detected(u16 bdf, void *arg);
+void i440fx_smram_detected(u16 bdf, void *arg);
 void piix_isa_bridge_init(u16 bdf, void *arg);
 void piix_ide_init(u16 bdf, void *arg);
 void piix4_pm_init(u16 bdf, void *arg);
diff --git a/src/post.h b/src/post.h
index 2996878..c82ee8f 100644
--- a/src/post.h
+++ b/src/post.h
@@ -16,4 +16,9 @@ struct apmc_ops
     void (*enable)(u16 bdf);
 };
 
+struct smram_regs
+{
+    u32 smram;
+};
+
 #endif /* __POST_H */
diff --git a/src/smm.c b/src/smm.c
index baa272f..4518d95 100644
--- a/src/smm.c
+++ b/src/smm.c
@@ -81,6 +81,13 @@ static const struct pci_device_id apmc_ops_tbl[] = {
     PCI_DEVICE_END,
 };
 
+static const struct pci_device_id smram_tbl[] = {
+    PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82441,
+               i440fx_smram_detected),
+
+    PCI_DEVICE_END,
+};
+
 void
 smm_init(void)
 {
@@ -98,9 +105,9 @@ smm_init(void)
     if (bdf < 0)
         // Device not found
         return;
-    int i440_bdf = pci_find_device(PCI_VENDOR_ID_INTEL
-                                   , PCI_DEVICE_ID_INTEL_82441);
-    if (i440_bdf < 0)
+    struct smram_regs smram_regs;
+    int smram_bdf = pci_find_init_device(smram_tbl, &smram_regs);
+    if (smram_bdf < 0)
         return;
 
     /* check if SMM init is already done */
@@ -108,7 +115,7 @@ smm_init(void)
         return;
 
     /* enable the SMM memory window */
-    pci_config_writeb(i440_bdf, 0x72, 0x02 | 0x48);
+    pci_config_writeb(smram_bdf, smram_regs.smram, 0x02 | 0x48);
 
     /* save original memory content */
     memcpy((void *)BUILD_SMM_ADDR, (void *)BUILD_SMM_INIT_ADDR, BUILD_SMM_SIZE);
@@ -139,5 +146,5 @@ smm_init(void)
     wbinvd();
 
     /* close the SMM memory window and enable normal SMM */
-    pci_config_writeb(i440_bdf, 0x72, 0x02 | 0x08);
+    pci_config_writeb(smram_bdf, smram_regs.smram, 0x02 | 0x08);
 }
-- 
1.7.1.1

[Qemu-devel] [PATCH 5/7] seabios: acpi: move acpi definitions to acpi.h from acpi.c

[Isaku Yamahata]

Move ACPI_TABLE_HEADER_DEF and struct fadt_descriptor_rev1
from acpi.h to acpi.c for later use.

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
 src/acpi.c |   73 ---------------------------------------------------------
 src/acpi.h |   76 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 76 insertions(+), 73 deletions(-)

diff --git a/src/acpi.c b/src/acpi.c
index 0559443..1179570 100644
--- a/src/acpi.c
+++ b/src/acpi.c
@@ -19,18 +19,6 @@
 /* Table structure from Linux kernel (the ACPI tables are under the
    BSD license) */
 
-#define ACPI_TABLE_HEADER_DEF   /* ACPI common table header */ \
-    u32 signature;          /* ACPI signature (4 ASCII characters) */ \
-    u32 length;                 /* Length of table, in bytes, including header */ \
-    u8  revision;               /* ACPI Specification minor version # */ \
-    u8  checksum;               /* To make sum of entire table == 0 */ \
-    u8  oem_id [6];             /* OEM identification */ \
-    u8  oem_table_id [8];       /* OEM table identification */ \
-    u32 oem_revision;           /* OEM revision number */ \
-    u8  asl_compiler_id [4];    /* ASL compiler vendor ID */ \
-    u32 asl_compiler_revision;  /* ASL compiler revision number */
-
-
 struct acpi_table_header         /* ACPI common table header */
 {
     ACPI_TABLE_HEADER_DEF
@@ -65,67 +53,6 @@ struct facs_descriptor_rev1
 
 
 /*
- * ACPI 1.0 Fixed ACPI Description Table (FADT)
- */
-#define FACP_SIGNATURE 0x50434146 // FACP
-struct fadt_descriptor_rev1
-{
-    ACPI_TABLE_HEADER_DEF     /* ACPI common table header */
-    u32 firmware_ctrl;          /* Physical address of FACS */
-    u32 dsdt;                   /* Physical address of DSDT */
-    u8  model;                  /* System Interrupt Model */
-    u8  reserved1;              /* Reserved */
-    u16 sci_int;                /* System vector of SCI interrupt */
-    u32 smi_cmd;                /* Port address of SMI command port */
-    u8  acpi_enable;            /* Value to write to smi_cmd to enable ACPI */
-    u8  acpi_disable;           /* Value to write to smi_cmd to disable ACPI */
-    u8  S4bios_req;             /* Value to write to SMI CMD to enter S4BIOS state */
-    u8  reserved2;              /* Reserved - must be zero */
-    u32 pm1a_evt_blk;           /* Port address of Power Mgt 1a acpi_event Reg Blk */
-    u32 pm1b_evt_blk;           /* Port address of Power Mgt 1b acpi_event Reg Blk */
-    u32 pm1a_cnt_blk;           /* Port address of Power Mgt 1a Control Reg Blk */
-    u32 pm1b_cnt_blk;           /* Port address of Power Mgt 1b Control Reg Blk */
-    u32 pm2_cnt_blk;            /* Port address of Power Mgt 2 Control Reg Blk */
-    u32 pm_tmr_blk;             /* Port address of Power Mgt Timer Ctrl Reg Blk */
-    u32 gpe0_blk;               /* Port addr of General Purpose acpi_event 0 Reg Blk */
-    u32 gpe1_blk;               /* Port addr of General Purpose acpi_event 1 Reg Blk */
-    u8  pm1_evt_len;            /* Byte length of ports at pm1_x_evt_blk */
-    u8  pm1_cnt_len;            /* Byte length of ports at pm1_x_cnt_blk */
-    u8  pm2_cnt_len;            /* Byte Length of ports at pm2_cnt_blk */
-    u8  pm_tmr_len;             /* Byte Length of ports at pm_tm_blk */
-    u8  gpe0_blk_len;           /* Byte Length of ports at gpe0_blk */
-    u8  gpe1_blk_len;           /* Byte Length of ports at gpe1_blk */
-    u8  gpe1_base;              /* Offset in gpe model where gpe1 events start */
-    u8  reserved3;              /* Reserved */
-    u16 plvl2_lat;              /* Worst case HW latency to enter/exit C2 state */
-    u16 plvl3_lat;              /* Worst case HW latency to enter/exit C3 state */
-    u16 flush_size;             /* Size of area read to flush caches */
-    u16 flush_stride;           /* Stride used in flushing caches */
-    u8  duty_offset;            /* Bit location of duty cycle field in p_cnt reg */
-    u8  duty_width;             /* Bit width of duty cycle field in p_cnt reg */
-    u8  day_alrm;               /* Index to day-of-month alarm in RTC CMOS RAM */
-    u8  mon_alrm;               /* Index to month-of-year alarm in RTC CMOS RAM */
-    u8  century;                /* Index to century in RTC CMOS RAM */
-    u8  reserved4;              /* Reserved */
-    u8  reserved4a;             /* Reserved */
-    u8  reserved4b;             /* Reserved */
-#if 0
-    u32 wb_invd         : 1;    /* The wbinvd instruction works properly */
-    u32 wb_invd_flush   : 1;    /* The wbinvd flushes but does not invalidate */
-    u32 proc_c1         : 1;    /* All processors support C1 state */
-    u32 plvl2_up        : 1;    /* C2 state works on MP system */
-    u32 pwr_button      : 1;    /* Power button is handled as a generic feature */
-    u32 sleep_button    : 1;    /* Sleep button is handled as a generic feature, or not present */
-    u32 fixed_rTC       : 1;    /* RTC wakeup stat not in fixed register space */
-    u32 rtcs4           : 1;    /* RTC wakeup stat not possible from S4 */
-    u32 tmr_val_ext     : 1;    /* The tmr_val width is 32 bits (0 = 24 bits) */
-    u32 reserved5       : 23;   /* Reserved - must be zero */
-#else
-    u32 flags;
-#endif
-} PACKED;
-
-/*
  * MADT values and structures
  */
 
diff --git a/src/acpi.h b/src/acpi.h
index c3ae84c..e01315a 100644
--- a/src/acpi.h
+++ b/src/acpi.h
@@ -22,4 +22,80 @@ struct rsdp_descriptor {        /* Root System Descriptor Pointer */
 
 extern struct rsdp_descriptor *RsdpAddr;
 
+/* Table structure from Linux kernel (the ACPI tables are under the
+   BSD license) */
+
+#define ACPI_TABLE_HEADER_DEF   /* ACPI common table header */ \
+    u32 signature;          /* ACPI signature (4 ASCII characters) */ \
+    u32 length;                 /* Length of table, in bytes, including header */ \
+    u8  revision;               /* ACPI Specification minor version # */ \
+    u8  checksum;               /* To make sum of entire table == 0 */ \
+    u8  oem_id [6];             /* OEM identification */ \
+    u8  oem_table_id [8];       /* OEM table identification */ \
+    u32 oem_revision;           /* OEM revision number */ \
+    u8  asl_compiler_id [4];    /* ASL compiler vendor ID */ \
+    u32 asl_compiler_revision;  /* ASL compiler revision number */
+
+
+/*
+ * ACPI 1.0 Fixed ACPI Description Table (FADT)
+ */
+#define FACP_SIGNATURE 0x50434146 // FACP
+struct fadt_descriptor_rev1
+{
+    ACPI_TABLE_HEADER_DEF     /* ACPI common table header */
+    u32 firmware_ctrl;          /* Physical address of FACS */
+    u32 dsdt;                   /* Physical address of DSDT */
+    u8  model;                  /* System Interrupt Model */
+    u8  reserved1;              /* Reserved */
+    u16 sci_int;                /* System vector of SCI interrupt */
+    u32 smi_cmd;                /* Port address of SMI command port */
+    u8  acpi_enable;            /* Value to write to smi_cmd to enable ACPI */
+    u8  acpi_disable;           /* Value to write to smi_cmd to disable ACPI */
+    u8  S4bios_req;             /* Value to write to SMI CMD to enter S4BIOS state */
+    u8  reserved2;              /* Reserved - must be zero */
+    u32 pm1a_evt_blk;           /* Port address of Power Mgt 1a acpi_event Reg Blk */
+    u32 pm1b_evt_blk;           /* Port address of Power Mgt 1b acpi_event Reg Blk */
+    u32 pm1a_cnt_blk;           /* Port address of Power Mgt 1a Control Reg Blk */
+    u32 pm1b_cnt_blk;           /* Port address of Power Mgt 1b Control Reg Blk */
+    u32 pm2_cnt_blk;            /* Port address of Power Mgt 2 Control Reg Blk */
+    u32 pm_tmr_blk;             /* Port address of Power Mgt Timer Ctrl Reg Blk */
+    u32 gpe0_blk;               /* Port addr of General Purpose acpi_event 0 Reg Blk */
+    u32 gpe1_blk;               /* Port addr of General Purpose acpi_event 1 Reg Blk */
+    u8  pm1_evt_len;            /* Byte length of ports at pm1_x_evt_blk */
+    u8  pm1_cnt_len;            /* Byte length of ports at pm1_x_cnt_blk */
+    u8  pm2_cnt_len;            /* Byte Length of ports at pm2_cnt_blk */
+    u8  pm_tmr_len;             /* Byte Length of ports at pm_tm_blk */
+    u8  gpe0_blk_len;           /* Byte Length of ports at gpe0_blk */
+    u8  gpe1_blk_len;           /* Byte Length of ports at gpe1_blk */
+    u8  gpe1_base;              /* Offset in gpe model where gpe1 events start */
+    u8  reserved3;              /* Reserved */
+    u16 plvl2_lat;              /* Worst case HW latency to enter/exit C2 state */
+    u16 plvl3_lat;              /* Worst case HW latency to enter/exit C3 state */
+    u16 flush_size;             /* Size of area read to flush caches */
+    u16 flush_stride;           /* Stride used in flushing caches */
+    u8  duty_offset;            /* Bit location of duty cycle field in p_cnt reg */
+    u8  duty_width;             /* Bit width of duty cycle field in p_cnt reg */
+    u8  day_alrm;               /* Index to day-of-month alarm in RTC CMOS RAM */
+    u8  mon_alrm;               /* Index to month-of-year alarm in RTC CMOS RAM */
+    u8  century;                /* Index to century in RTC CMOS RAM */
+    u8  reserved4;              /* Reserved */
+    u8  reserved4a;             /* Reserved */
+    u8  reserved4b;             /* Reserved */
+#if 0
+    u32 wb_invd         : 1;    /* The wbinvd instruction works properly */
+    u32 wb_invd_flush   : 1;    /* The wbinvd flushes but does not invalidate */
+    u32 proc_c1         : 1;    /* All processors support C1 state */
+    u32 plvl2_up        : 1;    /* C2 state works on MP system */
+    u32 pwr_button      : 1;    /* Power button is handled as a generic feature */
+    u32 sleep_button    : 1;    /* Sleep button is handled as a generic feature, or not present */
+    u32 fixed_rTC       : 1;    /* RTC wakeup stat not in fixed register space */
+    u32 rtcs4           : 1;    /* RTC wakeup stat not possible from S4 */
+    u32 tmr_val_ext     : 1;    /* The tmr_val width is 32 bits (0 = 24 bits) */
+    u32 reserved5       : 23;   /* Reserved - must be zero */
+#else
+    u32 flags;
+#endif
+} PACKED;
+
 #endif // acpi.h
-- 
1.7.1.1

[Qemu-devel] [PATCH 6/7] seabios: acpi: split out piix4 pm logic.

[Isaku Yamahata]

split out piix4 pm logic.

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
 src/acpi.c       |   19 ++++++++++---------
 src/acpi.h       |    5 +++++
 src/dev-i440fx.c |   15 +++++++++++++++
 src/dev-i440fx.h |    1 +
 4 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/src/acpi.c b/src/acpi.c
index 1179570..32d436f 100644
--- a/src/acpi.c
+++ b/src/acpi.c
@@ -12,6 +12,7 @@
 #include "pci_ids.h" // PCI_VENDOR_ID_INTEL
 #include "pci_regs.h" // PCI_INTERRUPT_LINE
 #include "paravirt.h"
+#include "dev-i440fx.h" // piix4_fadt_init
 
 /****************************************************/
 /* ACPI tables init */
@@ -202,11 +203,6 @@ static inline u16 cpu_to_le16(u16 x)
     return x;
 }
 
-static inline u32 cpu_to_le32(u32 x)
-{
-    return x;
-}
-
 static void
 build_header(struct acpi_table_header *h, u32 sig, int len, u8 rev)
 {
@@ -222,6 +218,14 @@ build_header(struct acpi_table_header *h, u32 sig, int len, u8 rev)
     h->checksum -= checksum(h, len);
 }
 
+static const struct pci_device_id fadt_init_tbl[] = {
+    /* PIIX4 Power Management device (for ACPI) */
+    PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82371AB_3,
+               piix4_fadt_init),
+
+    PCI_DEVICE_END
+};
+
 static void*
 build_fadt(int bdf)
 {
@@ -251,8 +255,6 @@ build_fadt(int bdf)
     int pm_sci_int = pci_config_readb(bdf, PCI_INTERRUPT_LINE);
     fadt->sci_int = cpu_to_le16(pm_sci_int);
     fadt->smi_cmd = cpu_to_le32(PORT_SMI_CMD);
-    fadt->acpi_enable = 0xf1;
-    fadt->acpi_disable = 0xf0;
     fadt->pm1a_evt_blk = cpu_to_le32(PORT_ACPI_PM_BASE);
     fadt->pm1a_cnt_blk = cpu_to_le32(PORT_ACPI_PM_BASE + 0x04);
     fadt->pm_tmr_blk = cpu_to_le32(PORT_ACPI_PM_BASE + 0x08);
@@ -261,8 +263,7 @@ build_fadt(int bdf)
     fadt->pm_tmr_len = 4;
     fadt->plvl2_lat = cpu_to_le16(0xfff); // C2 state not supported
     fadt->plvl3_lat = cpu_to_le16(0xfff); // C3 state not supported
-    fadt->gpe0_blk = cpu_to_le32(0xafe0);
-    fadt->gpe0_blk_len = 4;
+    pci_init_device(fadt_init_tbl, bdf, fadt);
     /* WBINVD + PROC_C1 + SLP_BUTTON + FIX_RTC */
     fadt->flags = cpu_to_le32((1 << 0) | (1 << 2) | (1 << 5) | (1 << 6));
 
diff --git a/src/acpi.h b/src/acpi.h
index e01315a..56add07 100644
--- a/src/acpi.h
+++ b/src/acpi.h
@@ -98,4 +98,9 @@ struct fadt_descriptor_rev1
 #endif
 } PACKED;
 
+static inline u32 cpu_to_le32(u32 x)
+{
+    return x;
+}
+
 #endif // acpi.h
diff --git a/src/dev-i440fx.c b/src/dev-i440fx.c
index c9e0c3e..e5dd7dd 100644
--- a/src/dev-i440fx.c
+++ b/src/dev-i440fx.c
@@ -17,6 +17,7 @@
 #include "post.h"
 #include "dev-i440fx.h"
 #include "post.h"
+#include "acpi.h" // cpu_to_le32
 
 #define I440FX_PAM0     0x59
 
@@ -97,3 +98,17 @@ void piix4_apmc_detected(u16 bdf, void *arg)
     ops->is_enabled = piix4_apmc_is_enabled;
     ops->enable = piix4_apmc_enable;
 }
+
+#define PIIX4_ACPI_ENABLE       0xf1
+#define PIIX4_ACPI_DISABLE      0xf0
+#define PIIX4_GPE0_BLK          0xafe0
+#define PIIX4_GPE0_BLK_LEN      4
+
+void piix4_fadt_init(u16 bdf, void *arg)
+{
+    struct fadt_descriptor_rev1 *fadt = arg;
+    fadt->acpi_enable = PIIX4_ACPI_ENABLE;
+    fadt->acpi_disable = PIIX4_ACPI_DISABLE;
+    fadt->gpe0_blk = cpu_to_le32(PIIX4_GPE0_BLK);
+    fadt->gpe0_blk_len = PIIX4_GPE0_BLK_LEN;
+}
diff --git a/src/dev-i440fx.h b/src/dev-i440fx.h
index 6250c5e..ee89c23 100644
--- a/src/dev-i440fx.h
+++ b/src/dev-i440fx.h
@@ -9,5 +9,6 @@ void piix_isa_bridge_init(u16 bdf, void *arg);
 void piix_ide_init(u16 bdf, void *arg);
 void piix4_pm_init(u16 bdf, void *arg);
 void piix4_apmc_detected(u16 bdf, void *arg);
+void piix4_fadt_init(u16 bdf, void *arg);
 
 #endif // __I440FX_H
-- 
1.7.1.1

[Qemu-devel] Re: [SeaBIOS] [PATCH 6/7] seabios: acpi: split out piix4 pm logic.

[Kevin O'Connor]

On Mon, Jul 12, 2010 at 08:47:51PM +0900, Isaku Yamahata wrote:
> split out piix4 pm logic.
[...]
> @@ -202,11 +203,6 @@ static inline u16 cpu_to_le16(u16 x)
>      return x;
>  }
>  
> -static inline u32 cpu_to_le32(u32 x)
> -{
> -    return x;
> -}

This is a nitpick, but if cpu_to_le32() is moved to a header then
cpu_to_le16() needs to be moved to.  It should also be moved to where
ntohl() is defined (util.h).

-Kevin

[Qemu-devel] [PATCH 7/7] seabios: acpi: clean up of finding pm device.

[Isaku Yamahata]

Make it table driven to other chip set.

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
 src/acpi.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/acpi.c b/src/acpi.c
index 32d436f..fa07c37 100644
--- a/src/acpi.c
+++ b/src/acpi.c
@@ -527,6 +527,13 @@ build_srat(void)
     return srat;
 }
 
+static const struct pci_device_id acpi_find_tbl[] = {
+    /* PIIX4 Power Management device. */
+    PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82371AB_3, NULL),
+
+    PCI_DEVICE_END,
+};
+
 struct rsdp_descriptor *RsdpAddr;
 
 #define MAX_ACPI_TABLES 20
@@ -539,8 +546,7 @@ acpi_bios_init(void)
     dprintf(3, "init ACPI tables\n");
 
     // This code is hardcoded for PIIX4 Power Management device.
-    int bdf = pci_find_device(PCI_VENDOR_ID_INTEL
-                              , PCI_DEVICE_ID_INTEL_82371AB_3);
+    int bdf = pci_find_init_device(acpi_find_tbl, NULL);
     if (bdf < 0)
         // Device not found
         return;
-- 
1.7.1.1

[Qemu-devel] Re: [SeaBIOS] [PATCH 0/7] abstract chipset(i440fx) specific register operation.

[Kevin O'Connor]

On Mon, Jul 12, 2010 at 08:47:45PM +0900, Isaku Yamahata wrote:
> This patch set abstract out chipset specific operation,
> and spit out i440fx specific operation into dev-i440fx.c with it.
> Thus q35 specific register value/operation will be added easily.

Hi Isaku,

Can you give a brief overview on what patch series are remaining, and
what the final "q35" support will entail?

-Kevin

[Qemu-devel] Re: [SeaBIOS] [PATCH 0/7] abstract chipset(i440fx) specific register operation.

[Isaku Yamahata]

On Mon, Jul 12, 2010 at 08:50:55PM -0400, Kevin O'Connor wrote:
> On Mon, Jul 12, 2010 at 08:47:45PM +0900, Isaku Yamahata wrote:
> > This patch set abstract out chipset specific operation,
> > and spit out i440fx specific operation into dev-i440fx.c with it.
> > Thus q35 specific register value/operation will be added easily.
> 
> Hi Isaku,
> 
> Can you give a brief overview on what patch series are remaining, and
> what the final "q35" support will entail?

Oh yes, I should have depict the overview.
You can get my local seabios repo from the below.

    git clone http://people.valinux.co.jp/~yamahata/qemu/q35/seabios

This is not for review, but for those who want to try qemu q35/pcie.
So it contains change sets which won't be accepted to the upstream.


I have 3 patches in my posting with what I already posted.

overriding DSDT: I already posted it.
  seabios: acpi: allow qemu to load dsdt as external acpi table.
  Due to rom size limit, it isn't an option to have 2 DSDT in seabios,
  one for i440fx, one for q35.

split out i440fx specific part: This patch series.
  I suppose, you want redesign.

acpi MCFG support: single patch
  seabios: acpi: add mcfg table.

q35 device specific part: single patch
  seabios: add q35 initialization functions.
  This patch adds 2 files(dev-q35.[ch]) and inserts q35 specific entries
  into initialization tables.

q35 DSDT: single patch
  seabios: q35: add dsdt.
  I'm not sure this should go into seabios or qemu because it isn't
  complied into seabios.


I have other patches, but they would need discussion about how they
should work. So I don't plan to push those soon.
I have 3 issues.

- paravirtualize pci bus numbering
  Currently pci bus is numbered continuously.
  i.e. (*pci_bus)++ in change set of f441666dbdf0e9f78442a6b33b086699ff6f5a21.
  On the other hand in real hardware case, bus numbers might be
  assigned non-contiguously. And some surely does. Probably those
  numbers are hard-coded in bios.
  It is also convenient for DSDT writers to assign pci bus number
  without the constraint that bus numbers be contiguously assigned.

  So I'd like to pass the bus numbering hits from qemu to seabios.
  This requires qemu enhancement which would require discussion.

- PCI bar assignment clean up
  Clean up of pci_bios_{io, mem, prefmem}_addr.
  This isn't a big issue.
  
- vga bios
  remove hard coded VBE physical address
  Gerd sent patches to address this, but there seems no progress yet.
  This is for the original vgabios. vgabios in seabioa seem under
  development.

-- 
yamahata