[Qemu-devel] [PATCH v2 0/2] abstract shadow and smm opreration.

[Qemu-devel] [PATCH v2 0/2] abstract shadow and smm opreration.

[Isaku Yamahata]

This is on top of the patch series of
PATCH v2 "abstract chipset(i440fx) specific register operation."

Patch descriontion:
This patch set abstract out shadow and smm operation which are
specific to chipset. and spit out i440fx specific part into dev-i440fx.c.
Thus q35 specific register value/operation will be added easily.


Changes v1 -> v2:
- separated non-arguable part to make the merge easy.
- factor out intel specific shadow memory operation.
- factor out smm operation.
- abstract at device level, not register level.

Isaku Yamahata (2):
  seabios: shadow: make device finding more generic.
  seabios: smm: move out piix4 specific smram logic to dev-i440fx.c

 src/dev-i440fx.c |   44 ++++++++++++++++++++++
 src/dev-i440fx.h |    3 ++
 src/shadow.c     |  106 +++++++++++++++++++++++++++++++++---------------------
 src/smm.c        |   58 +++++++++++++-----------------
 src/util.h       |    6 +++
 5 files changed, 143 insertions(+), 74 deletions(-)

[Qemu-devel] [PATCH v2 1/2] 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>
---
changes v2 -> v3.
- abstract at device level, not at register offset level.
- factor out intel chipset operation.
---
 src/dev-i440fx.c |   12 ++++++
 src/dev-i440fx.h |    2 +
 src/shadow.c     |  106 +++++++++++++++++++++++++++++++++---------------------
 src/util.h       |    2 +
 4 files changed, 81 insertions(+), 41 deletions(-)

diff --git a/src/dev-i440fx.c b/src/dev-i440fx.c
index 15c6cac..366a2db 100644
--- a/src/dev-i440fx.c
+++ b/src/dev-i440fx.c
@@ -17,6 +17,18 @@
 #include "acpi.h"
 #include "dev-i440fx.h"
 
+#define I440FX_PAM0     0x59
+
+void i440fx_bios_make_writable(u16 bdf, void *arg)
+{
+    make_bios_writable_intel(bdf, I440FX_PAM0);
+}
+
+void i440fx_bios_make_readonly(u16 bdf, void *arg)
+{
+    make_bios_readonly_intel(bdf, 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 661860a..6d1b687 100644
--- a/src/dev-i440fx.h
+++ b/src/dev-i440fx.h
@@ -3,6 +3,8 @@
 
 #include "types.h" // u16
 
+void i440fx_bios_make_writable(u16 bdf, void *arg);
+void i440fx_bios_make_readonly(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/shadow.c b/src/shadow.c
index 978424e..e91e54e 100644
--- a/src/shadow.c
+++ b/src/shadow.c
@@ -9,6 +9,7 @@
 #include "pci.h" // pci_config_writeb
 #include "config.h" // CONFIG_*
 #include "pci_ids.h" // PCI_VENDOR_ID_INTEL
+#include "dev-i440fx.h"
 
 // Test if 'addr' is in the range from 'start'..'start+size'
 #define IN_RANGE(addr, start, size) ({   \
@@ -23,30 +24,31 @@
 
 // Enable shadowing and copy bios.
 static void
-__make_bios_writable(u16 bdf)
+__make_bios_writable_intel(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;
@@ -55,52 +57,28 @@ __make_bios_writable(u16 bdf)
     memcpy((void*)BUILD_BIOS_ADDR, (void*)BIOS_SRC_ADDR, BUILD_BIOS_SIZE);
 }
 
-// Make the 0xc0000-0x100000 area read/writable.
 void
-make_bios_writable(void)
+make_bios_writable_intel(u16 bdf, u32 pam0)
 {
-    if (CONFIG_COREBOOT)
-        return;
-
-    dprintf(3, "enabling shadow ram\n");
-
-    // Locate chip controlling ram shadowing.
-    int bdf = pci_find_device(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82441);
-    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, 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);
+        u32 pos = (u32)__make_bios_writable_intel - BUILD_BIOS_ADDR +
+            BIOS_SRC_ADDR;
+        void (*func)(u16 bdf, u32 pam0) = (void*)pos;
+        func(bdf, pam0);
         return;
     }
     // Ram already present - just enable writes
-    __make_bios_writable(bdf);
+    __make_bios_writable_intel(bdf, pam0);
 }
 
-// Make the BIOS code segment area (0xf0000) read-only.
 void
-make_bios_readonly(void)
+make_bios_readonly_intel(u16 bdf, u32 pam0)
 {
-    if (CONFIG_COREBOOT)
-        return;
-
-    dprintf(3, "locking shadow ram\n");
-
-    int bdf = pci_find_device(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82441);
-    if (bdf < 0) {
-        dprintf(1, "Unable to lock ram - bridge not found\n");
-        return;
-    }
-
     // Flush any pending writes before locking memory.
     wbinvd();
 
@@ -108,14 +86,60 @@ make_bios_readonly(void)
     int i;
     for (i=0; i<6; i++) {
         u32 mem = BUILD_ROM_START + i * 32*1024;
+        u32 pam = 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, pam0, 0x10);
+}
+
+static const struct pci_device_id dram_controller_make_writable_tbl[] = {
+    PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82441,
+               i440fx_bios_make_writable),
+    PCI_DEVICE_END
+};
+
+// Make the 0xc0000-0x100000 area read/writable.
+void
+make_bios_writable(void)
+{
+    if (CONFIG_COREBOOT)
+        return;
+
+    dprintf(3, "enabling shadow ram\n");
+
+    // at this point, staticlly alloacted variable can't written.
+    // so stack should be used.
+
+    // Locate chip controlling ram shadowing.
+    int bdf = pci_find_init_device(dram_controller_make_writable_tbl, NULL);
+    if (bdf < 0) {
+        dprintf(1, "Unable to unlock ram - bridge not found\n");
+    }
+}
+
+static const struct pci_device_id dram_controller_make_readonly_tbl[] = {
+    PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82441,
+               i440fx_bios_make_readonly),
+    PCI_DEVICE_END
+};
+
+// Make the BIOS code segment area (0xf0000) read-only.
+void
+make_bios_readonly(void)
+{
+    if (CONFIG_COREBOOT)
+        return;
+
+    dprintf(3, "locking shadow ram\n");
+    int bdf = pci_find_init_device(dram_controller_make_readonly_tbl, NULL);
+    if (bdf < 0) {
+        dprintf(1, "Unable to lock ram - bridge not found\n");
+    }
 }
diff --git a/src/util.h b/src/util.h
index 6c08a3c..96f4ff7 100644
--- a/src/util.h
+++ b/src/util.h
@@ -332,6 +332,8 @@ void bios32_setup(void);
 // shadow.c
 void make_bios_writable(void);
 void make_bios_readonly(void);
+void make_bios_writable_intel(u16 bdf, u32 pam0);
+void make_bios_readonly_intel(u16 bdf, u32 pam0);
 
 // pciinit.c
 extern const u8 pci_irqs[4];
-- 
1.7.1.1

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

[Isaku Yamahata]

move out piix4 specific smram logic to dev-i440fx.c.

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
changes v2 -> v3.
- abstract device level and simplified it.
- factor out smm ram relocation.
---
 src/dev-i440fx.c |   32 +++++++++++++++++++++++++++++
 src/dev-i440fx.h |    1 +
 src/smm.c        |   58 +++++++++++++++++++++++------------------------------
 src/util.h       |    4 +++
 4 files changed, 62 insertions(+), 33 deletions(-)

diff --git a/src/dev-i440fx.c b/src/dev-i440fx.c
index 366a2db..346f6d0 100644
--- a/src/dev-i440fx.c
+++ b/src/dev-i440fx.c
@@ -13,6 +13,7 @@
 #include "util.h" // dprintf
 #include "ioport.h" // outb
 #include "pci.h" // pci_config_writeb
+#include "pci_ids.h"
 #include "pci_regs.h" // PCI_INTERRUPT_LINE
 #include "acpi.h"
 #include "dev-i440fx.h"
@@ -82,3 +83,34 @@ void piix4_fadt_init(u16 bdf, void *arg)
     fadt->gpe0_blk = cpu_to_le32(PIIX4_GPE0_BLK);
     fadt->gpe0_blk_len = PIIX4_GPE0_BLK_LEN;
 }
+
+#define I440FX_SMRAM    0x72
+#define PIIX_DEVACTB    0x58
+#define PIIX_APMC_EN    (1 << 25)
+
+// This code is hardcoded for PIIX4 Power Management device.
+void piix4_apmc_smm_init(u16 bdf, void *arg)
+{
+    int i440_bdf = pci_find_device(PCI_VENDOR_ID_INTEL
+                                   , PCI_DEVICE_ID_INTEL_82441);
+    if (i440_bdf < 0)
+        return;
+
+    /* check if SMM init is already done */
+    u32 value = pci_config_readl(bdf, PIIX_DEVACTB);
+    if (value & PIIX_APMC_EN)
+        return;
+
+    /* enable the SMM memory window */
+    pci_config_writeb(i440_bdf, I440FX_SMRAM, 0x02 | 0x48);
+
+    smm_save_and_copy();
+
+    /* enable SMI generation when writing to the APMC register */
+    pci_config_writel(bdf, PIIX_DEVACTB, value | PIIX_APMC_EN);
+
+    smm_relocate_and_restore();
+
+    /* close the SMM memory window and enable normal SMM */
+    pci_config_writeb(i440_bdf, I440FX_SMRAM, 0x02 | 0x08);
+}
diff --git a/src/dev-i440fx.h b/src/dev-i440fx.h
index 6d1b687..ab5a4d1 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_fadt_init(u16 bdf, void *arg);
+void piix4_apmc_smm_init(u16 bdf, void *arg);
 
 #endif // __I440FX_H
diff --git a/src/smm.c b/src/smm.c
index 3f53ef9..7e52892 100644
--- a/src/smm.c
+++ b/src/smm.c
@@ -10,6 +10,7 @@
 #include "config.h" // CONFIG_*
 #include "ioport.h" // outb
 #include "pci_ids.h" // PCI_VENDOR_ID_INTEL
+#include "dev-i440fx.h"
 
 ASM32FLAT(
     ".global smm_relocation_start\n"
@@ -73,45 +74,19 @@ extern u8 smm_relocation_start, smm_relocation_end;
 extern u8 smm_code_start, smm_code_end;
 
 void
-smm_init(void)
+smm_save_and_copy(void)
 {
-    if (CONFIG_COREBOOT)
-        // SMM only supported on emulators.
-        return;
-    if (!CONFIG_USE_SMM)
-        return;
-
-    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);
-    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)
-        return;
-
-    /* check if SMM init is already done */
-    u32 value = pci_config_readl(bdf, 0x58);
-    if (value & (1 << 25))
-        return;
-
-    /* enable the SMM memory window */
-    pci_config_writeb(i440_bdf, 0x72, 0x02 | 0x48);
-
     /* save original memory content */
     memcpy((void *)BUILD_SMM_ADDR, (void *)BUILD_SMM_INIT_ADDR, BUILD_SMM_SIZE);
 
     /* copy the SMM relocation code */
     memcpy((void *)BUILD_SMM_INIT_ADDR, &smm_relocation_start,
            &smm_relocation_end - &smm_relocation_start);
+}
 
-    /* enable SMI generation when writing to the APMC register */
-    pci_config_writel(bdf, 0x58, value | (1 << 25));
-
+void
+smm_relocate_and_restore(void)
+{
     /* init APM status port */
     outb(0x01, PORT_SMI_STATUS);
 
@@ -129,7 +104,24 @@ smm_init(void)
     memcpy((void *)BUILD_SMM_ADDR, &smm_code_start
            , &smm_code_end - &smm_code_start);
     wbinvd();
+}
+
+static const struct pci_device_id smm_init_tbl[] = {
+    PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82371AB_3,
+               piix4_apmc_smm_init),
+
+    PCI_DEVICE_END,
+};
+
+void
+smm_init(void)
+{
+    if (CONFIG_COREBOOT)
+        // SMM only supported on emulators.
+        return;
+    if (!CONFIG_USE_SMM)
+        return;
 
-    /* close the SMM memory window and enable normal SMM */
-    pci_config_writeb(i440_bdf, 0x72, 0x02 | 0x08);
+    dprintf(3, "init smm\n");
+    pci_find_init_device(smm_init_tbl, NULL);
 }
diff --git a/src/util.h b/src/util.h
index 96f4ff7..85cf3f5 100644
--- a/src/util.h
+++ b/src/util.h
@@ -335,6 +335,10 @@ void make_bios_readonly(void);
 void make_bios_writable_intel(u16 bdf, u32 pam0);
 void make_bios_readonly_intel(u16 bdf, u32 pam0);
 
+// smm.c
+void smm_save_and_copy(void);
+void smm_relocate_and_restore(void);
+
 // pciinit.c
 extern const u8 pci_irqs[4];
 void pci_bios_allocate_regions(u16 bdf, void *arg);
-- 
1.7.1.1

[Qemu-devel] Re: [SeaBIOS] [PATCH v2 0/2] abstract shadow and smm opreration.

[Kevin O'Connor]

On Tue, Jul 20, 2010 at 04:50:44PM +0900, Isaku Yamahata wrote:
> This is on top of the patch series of
> PATCH v2 "abstract chipset(i440fx) specific register operation."
> 
> Patch descriontion:
> This patch set abstract out shadow and smm operation which are
> specific to chipset. and spit out i440fx specific part into dev-i440fx.c.
> Thus q35 specific register value/operation will be added easily.

Thanks!  I've committed both series.

As a side note, I'm not sure if moving the smm/shadow code into
dev-i440fx.c is best - the code seems more specific to smm/shadow then
to the i440fx.  Lets see how it works out.

-Kevin