[Qemu-devel] [PATCH 0/2] piix_pci: factor out pam, smram logic

[Qemu-devel] [PATCH 0/2] piix_pci: factor out pam, smram logic

[Isaku Yamahata]

Factor out PC pam, smram logic. which will also be used for q35 later.
All the magic numbers are replaced with symbolic constants at the same.
As by product, pc_piix initalization is simplified a bit.

Isaku Yamahata (2):
  pc/piix_pci: factor out smram/pam logic
  pc, i440fx: simplify i440fx initialization

 Makefile.target |    2 +-
 hw/pam.c        |  128 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/pam.h        |   96 +++++++++++++++++++++++++++++++++++++++++
 hw/pc.h         |    2 +-
 hw/pc_piix.c    |    8 +---
 hw/piix_pci.c   |   84 +++++++-----------------------------
 6 files changed, 243 insertions(+), 77 deletions(-)
 create mode 100644 hw/pam.c
 create mode 100644 hw/pam.h

[Qemu-devel] [PATCH 1/2] pc/piix_pci: factor out smram/pam logic

[Isaku Yamahata]

Factor out smram/pam logic for later use.
Which will be used by q35 too.

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
 Makefile.target |    2 +-
 hw/pam.c        |  128 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/pam.h        |   96 +++++++++++++++++++++++++++++++++++++++++
 hw/piix_pci.c   |   71 +++++--------------------------
 4 files changed, 236 insertions(+), 61 deletions(-)
 create mode 100644 hw/pam.c
 create mode 100644 hw/pam.h

diff --git a/Makefile.target b/Makefile.target
index ace5608..03c23d7 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -214,7 +214,7 @@ obj-$(CONFIG_KVM) += ivshmem.o
 # Hardware support
 obj-i386-y += vga.o
 obj-i386-y += mc146818rtc.o i8259.o pc.o
-obj-i386-y += cirrus_vga.o apic.o ioapic.o piix_pci.o
+obj-i386-y += cirrus_vga.o apic.o ioapic.o piix_pci.o pam.o
 obj-i386-y += vmport.o
 obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
 obj-i386-y += debugcon.o multiboot.o
diff --git a/hw/pam.c b/hw/pam.c
new file mode 100644
index 0000000..cd85faf
--- /dev/null
+++ b/hw/pam.c
@@ -0,0 +1,128 @@
+/*
+ * QEMU i440FX/PIIX3 PCI Bridge Emulation
+ *
+ * Copyright (c) 2006 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ * Split out from piix_pci.c
+ * Copyright (c) 2011 Isaku Yamahata <yamahata at valinux co jp>
+ *                    VA Linux Systems Japan K.K.
+ */
+
+#include "sysemu.h"
+#include "pam.h"
+
+/* XXX: suppress when better memory API. We make the assumption that
+   no device (in particular the VGA) changes the memory mappings in
+   the 0xa0000-0x100000 range */
+void pam_init_memory_mappings(PAM *pam)
+{
+    int i;
+    for(i = 0; i < ARRAY_SIZE(pam->isa_page_descs); i++) {
+        pam->isa_page_descs[i] =
+            cpu_get_physical_page_desc(SMRAM_C_BASE + (i << TARGET_PAGE_BITS));
+    }
+}
+
+static target_phys_addr_t isa_page_descs_get(PAM *pam, uint32_t addr)
+{
+    return pam->isa_page_descs[(addr - SMRAM_C_BASE) >> TARGET_PAGE_BITS];
+}
+
+void smram_update(PAM *pam, uint8_t smram)
+{
+    if ((pam->smm_enabled && (smram & SMRAM_G_SMRAME)) ||
+        (smram & SMRAM_D_OPEN)) {
+        cpu_register_physical_memory(SMRAM_C_BASE, SMRAM_C_SIZE, SMRAM_C_BASE);
+    } else {
+        uint32_t addr;
+        for(addr = SMRAM_C_BASE;
+            addr < SMRAM_C_END; addr += TARGET_PAGE_SIZE) {
+            cpu_register_physical_memory(addr, TARGET_PAGE_SIZE,
+                                         isa_page_descs_get(pam, addr));
+        }
+    }
+}
+
+void smram_set_smm(PAM *pam, int smm, uint8_t smram)
+{
+    uint8_t smm_enabled = (smm != 0);
+    if (pam->smm_enabled != smm_enabled) {
+        pam->smm_enabled = smm_enabled;
+        smram_update(pam, smram);
+    }
+}
+
+static void pam_update_seg(struct PAM *pam,
+                           uint32_t start, uint32_t size, uint8_t attr)
+{
+    uint32_t addr;
+
+#if 0
+    printf("ISA mapping %08"PRIx32"-0x%08"PRIx32": %"PRId32"\n",
+           start, start + size, attr);
+#endif
+    switch(attr) {
+    case PAM_ATTR_WE | PAM_ATTR_RE:
+        /* RAM */
+        cpu_register_physical_memory(start, size, start);
+        break;
+
+    case PAM_ATTR_RE:
+        /* ROM (XXX: not quite correct) */
+        cpu_register_physical_memory(start, size, start | IO_MEM_ROM);
+        break;
+
+    case PAM_ATTR_WE:
+    case 0: /* XXX: should distinguish read/write cases */
+        for(addr = start; addr < start + size; addr += TARGET_PAGE_SIZE) {
+            cpu_register_physical_memory(addr, TARGET_PAGE_SIZE,
+                                         isa_page_descs_get(pam, addr));
+        }
+        break;
+
+    default:
+        abort();
+        break;
+    }
+}
+
+static uint8_t pam_attr(uint8_t val, int hi)
+{
+    return (val >> ((!!hi) * 4)) & PAM_ATTR_MASK;
+}
+
+void pam_update(PAM *pam, int idx, uint8_t val)
+{
+    uint32_t phys_addr;
+
+    assert(0 <= idx && idx <= PAM_IDX_MAX);
+
+    if (idx == 0) {
+        pam_update_seg(pam, PAM_BIOS_BASE, PAM_BIOS_SIZE, pam_attr(val, 1));
+        return;
+    }
+
+    phys_addr = PAM_EXPAN_BASE + PAM_EXPAN_SIZE * (idx - 1) * 2;
+    pam_update_seg(pam, phys_addr, PAM_EXPAN_SIZE, pam_attr(val, 0));
+
+    phys_addr += PAM_EXPAN_SIZE;
+    pam_update_seg(pam, phys_addr, PAM_EXPAN_SIZE, pam_attr(val, 1));
+}
diff --git a/hw/pam.h b/hw/pam.h
new file mode 100644
index 0000000..cb66056
--- /dev/null
+++ b/hw/pam.h
@@ -0,0 +1,96 @@
+#ifndef QEMU_PAM_H
+#define QEMU_PAM_H
+
+/*
+ * Copyright (c) 2006 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+/*
+ * Split out from piix_pci.c
+ * Copyright (c) 2011 Isaku Yamahata <yamahata at valinux co jp>
+ *                    VA Linux Systems Japan K.K.
+ *
+ * SMRAM memory area and PAM memory area in Legacy address range for PC.
+ * PAM: Programmable Attribute Map registers
+ *
+ * 0xa0000 - 0xbffff compatible SMRAM
+ *
+ * 0xc0000 - 0xc3fff Expansion area memory segments
+ * 0xc4000 - 0xc7fff
+ * 0xc8000 - 0xcbfff
+ * 0xcc000 - 0xcffff
+ * 0xd0000 - 0xd7fff
+ * 0xd8000 - 0xdbfff
+ * 0xdc000 - 0xdffff
+ * 0xe0000 - 0xe3fff Extended System BIOS Area Memory Segments
+ * 0xe4000 - 0xe7fff
+ * 0xe8000 - 0xebfff
+ * 0xec000 - 0xeffff
+ *
+ * 0xf0000 - 0xfffff System BIOS Area Memory Segments
+ */
+
+#include "qemu-common.h"
+
+#define SMRAM_C_BASE    0xa0000
+#define SMRAM_C_END     0xc0000
+#define SMRAM_C_SIZE    0x20000
+
+
+#define PAM_EXPAN_BASE  0xc0000
+#define PAM_EXPAN_SIZE  0x04000
+
+#define PAM_EXBIOS_BASE 0xe0000
+#define PAM_EXBIOS_SIZE 0x04000
+
+#define PAM_BIOS_BASE   0xf0000
+#define PAM_BIOS_END    0x100000
+#define PAM_BIOS_SIZE   0x10000 /* 16KB */
+
+/* PAM registers: log nibble and high nibble*/
+#define PAM_ATTR_WE     ((uint8_t)2)
+#define PAM_ATTR_RE     ((uint8_t)1)
+#define PAM_ATTR_MASK   ((uint8_t)3)
+
+#define PAM_IDX_MAX     6       /* pam0 - pam6 */
+
+/* SMRAM register */
+#define SMRAM_D_OPEN           ((uint8_t)(1 << 6))
+#define SMRAM_D_CLS            ((uint8_t)(1 << 5))
+#define SMRAM_D_LCK            ((uint8_t)(1 << 4))
+#define SMRAM_G_SMRAME         ((uint8_t)(1 << 3))
+#define SMRAM_C_BASE_SEG_MASK  ((uint8_t)0x7)
+#define SMRAM_C_BASE_SEG       ((uint8_t)0x2)  /* hardwired to b010 */
+
+
+struct PAM {
+    target_phys_addr_t
+    isa_page_descs[(PAM_BIOS_END - SMRAM_C_BASE) >> TARGET_PAGE_BITS];
+    uint8_t smm_enabled;
+};
+
+typedef struct PAM PAM;
+
+void pam_init_memory_mappings(PAM *pam);
+void smram_update(PAM *pam, uint8_t smram);
+void smram_set_smm(PAM *pam, int smm, uint8_t smram);
+void pam_update(PAM *pam, int idx, uint8_t val);
+
+#endif /* QEMU_PAM_H */
diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index 892c576..4bcbce6 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -29,6 +29,7 @@
 #include "isa.h"
 #include "sysbus.h"
 #include "range.h"
+#include "pam.h"
 
 /*
  * I440FX chipset data sheet.
@@ -45,8 +46,7 @@ typedef struct PIIX3State {
 
 struct PCII440FXState {
     PCIDevice dev;
-    target_phys_addr_t isa_page_descs[384 / 4];
-    uint8_t smm_enabled;
+    PAM pam;
     PIIX3State *piix3;
 };
 
@@ -67,75 +67,26 @@ static int pci_slot_get_pirq(void *opaque, PCIDevice *pci_dev, int irq_num)
     return (irq_num + slot_addend) & 3;
 }
 
-static void update_pam(PCII440FXState *d, uint32_t start, uint32_t end, int r)
-{
-    uint32_t addr;
-
-    //    printf("ISA mapping %08x-0x%08x: %d\n", start, end, r);
-    switch(r) {
-    case 3:
-        /* RAM */
-        cpu_register_physical_memory(start, end - start,
-                                     start);
-        break;
-    case 1:
-        /* ROM (XXX: not quite correct) */
-        cpu_register_physical_memory(start, end - start,
-                                     start | IO_MEM_ROM);
-        break;
-    case 2:
-    case 0:
-        /* XXX: should distinguish read/write cases */
-        for(addr = start; addr < end; addr += 4096) {
-            cpu_register_physical_memory(addr, 4096,
-                                         d->isa_page_descs[(addr - 0xa0000) >> 12]);
-        }
-        break;
-    }
-}
-
 static void i440fx_update_memory_mappings(PCII440FXState *d)
 {
-    int i, r;
-    uint32_t smram, addr;
+    int i;
 
-    update_pam(d, 0xf0000, 0x100000, (d->dev.config[I440FX_PAM] >> 4) & 3);
-    for(i = 0; i < 12; i++) {
-        r = (d->dev.config[(i >> 1) + (I440FX_PAM + 1)] >> ((i & 1) * 4)) & 3;
-        update_pam(d, 0xc0000 + 0x4000 * i, 0xc0000 + 0x4000 * (i + 1), r);
-    }
-    smram = d->dev.config[I440FX_SMRAM];
-    if ((d->smm_enabled && (smram & 0x08)) || (smram & 0x40)) {
-        cpu_register_physical_memory(0xa0000, 0x20000, 0xa0000);
-    } else {
-        for(addr = 0xa0000; addr < 0xc0000; addr += 4096) {
-            cpu_register_physical_memory(addr, 4096,
-                                         d->isa_page_descs[(addr - 0xa0000) >> 12]);
-        }
+    for (i = 0; i <= PAM_IDX_MAX; i++) {
+        pam_update(&d->pam, i, d->dev.config[I440FX_PAM + i]);
     }
+
+    smram_update(&d->pam, d->dev.config[I440FX_SMRAM]);
 }
 
 static void i440fx_set_smm(int val, void *arg)
 {
     PCII440FXState *d = arg;
-
-    val = (val != 0);
-    if (d->smm_enabled != val) {
-        d->smm_enabled = val;
-        i440fx_update_memory_mappings(d);
-    }
+    smram_set_smm(&d->pam, val, d->dev.config[I440FX_SMRAM]);
 }
 
-
-/* XXX: suppress when better memory API. We make the assumption that
-   no device (in particular the VGA) changes the memory mappings in
-   the 0xa0000-0x100000 range */
 void i440fx_init_memory_mappings(PCII440FXState *d)
 {
-    int i;
-    for(i = 0; i < 96; i++) {
-        d->isa_page_descs[i] = cpu_get_physical_page_desc(0xa0000 + i * 0x1000);
-    }
+    pam_init_memory_mappings(&d->pam);
 }
 
 static void i440fx_write_config(PCIDevice *dev,
@@ -160,7 +111,7 @@ static int i440fx_load_old(QEMUFile* f, void *opaque, int version_id)
     if (ret < 0)
         return ret;
     i440fx_update_memory_mappings(d);
-    qemu_get_8s(f, &d->smm_enabled);
+    qemu_get_8s(f, &d->pam.smm_enabled);
 
     if (version_id == 2)
         for (i = 0; i < 4; i++)
@@ -186,7 +137,7 @@ static const VMStateDescription vmstate_i440fx = {
     .post_load = i440fx_post_load,
     .fields      = (VMStateField []) {
         VMSTATE_PCI_DEVICE(dev, PCII440FXState),
-        VMSTATE_UINT8(smm_enabled, PCII440FXState),
+        VMSTATE_UINT8(pam.smm_enabled, PCII440FXState),
         VMSTATE_END_OF_LIST()
     }
 };
-- 
1.7.1.1

[Qemu-devel] [PATCH 2/2] pc, i440fx: simplify i440fx initialization

[Isaku Yamahata]

simplify i440fx initialization by eliminating
i440fx_init_memory_mappings().

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
 hw/pc.h       |    2 +-
 hw/pc_piix.c  |    8 +-------
 hw/piix_pci.c |   15 ++++++---------
 3 files changed, 8 insertions(+), 17 deletions(-)

diff --git a/hw/pc.h b/hw/pc.h
index feb8a7a..38a0952 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -175,7 +175,7 @@ int pcspk_audio_init(qemu_irq *pic);
 struct PCII440FXState;
 typedef struct PCII440FXState PCII440FXState;
 
-PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix_devfn, qemu_irq *pic, ram_addr_t ram_size);
+PCIBus *i440fx_init(int *piix_devfn, qemu_irq *pic, ram_addr_t ram_size);
 void i440fx_init_memory_mappings(PCII440FXState *d);
 
 /* piix4.c */
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index a85214b..43fac55 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -74,7 +74,6 @@ static void pc_init1(ram_addr_t ram_size,
     int i;
     ram_addr_t below_4g_mem_size, above_4g_mem_size;
     PCIBus *pci_bus;
-    PCII440FXState *i440fx_state;
     int piix3_devfn = -1;
     qemu_irq *cpu_irq;
     qemu_irq *isa_irq;
@@ -106,10 +105,9 @@ static void pc_init1(ram_addr_t ram_size,
     isa_irq = qemu_allocate_irqs(isa_irq_handler, isa_irq_state, 24);
 
     if (pci_enabled) {
-        pci_bus = i440fx_init(&i440fx_state, &piix3_devfn, isa_irq, ram_size);
+        pci_bus = i440fx_init(&piix3_devfn, isa_irq, ram_size);
     } else {
         pci_bus = NULL;
-        i440fx_state = NULL;
         isa_bus_new(NULL);
     }
     isa_bus_irqs(isa_irq);
@@ -166,10 +164,6 @@ static void pc_init1(ram_addr_t ram_size,
         smbus_eeprom_init(smbus, 8, NULL, 0);
     }
 
-    if (i440fx_state) {
-        i440fx_init_memory_mappings(i440fx_state);
-    }
-
     if (pci_enabled) {
         pc_pci_device_init(pci_bus);
     }
diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index 4bcbce6..a668a31 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -84,11 +84,6 @@ static void i440fx_set_smm(int val, void *arg)
     smram_set_smm(&d->pam, val, d->dev.config[I440FX_SMRAM]);
 }
 
-void i440fx_init_memory_mappings(PCII440FXState *d)
-{
-    pam_init_memory_mappings(&d->pam);
-}
-
 static void i440fx_write_config(PCIDevice *dev,
                                 uint32_t address, uint32_t val, int len)
 {
@@ -164,15 +159,17 @@ static int i440fx_initfn(PCIDevice *dev)
     d->dev.config[I440FX_SMRAM] = 0x02;
 
     cpu_smm_register(&i440fx_set_smm, d);
+    pam_init_memory_mappings(&d->pam);
     return 0;
 }
 
-PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix3_devfn, qemu_irq *pic, ram_addr_t ram_size)
+PCIBus *i440fx_init(int *piix3_devfn, qemu_irq *pic, ram_addr_t ram_size)
 {
     DeviceState *dev;
     PCIBus *b;
     PCIDevice *d;
     I440FXState *s;
+    PCII440FXState *i440fx_state;
     PIIX3State *piix3;
 
     dev = qdev_create(NULL, "i440FX-pcihost");
@@ -182,20 +179,20 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix3_devfn, qemu_irq *
     qdev_init_nofail(dev);
 
     d = pci_create_simple(b, 0, "i440FX");
-    *pi440fx_state = DO_UPCAST(PCII440FXState, dev, d);
+    i440fx_state = DO_UPCAST(PCII440FXState, dev, d);
 
     piix3 = DO_UPCAST(PIIX3State, dev,
                       pci_create_simple_multifunction(b, -1, true, "PIIX3"));
     piix3->pic = pic;
     pci_bus_irqs(b, piix3_set_irq, pci_slot_get_pirq, piix3, 4);
-    (*pi440fx_state)->piix3 = piix3;
+    i440fx_state->piix3 = piix3;
 
     *piix3_devfn = piix3->dev.devfn;
 
     ram_size = ram_size / 8 / 1024 / 1024;
     if (ram_size > 255)
         ram_size = 255;
-    (*pi440fx_state)->dev.config[0x57]=ram_size;
+    i440fx_state->dev.config[0x57] = ram_size;
 
     return b;
 }
-- 
1.7.1.1