[Qemu-devel] [PATCH v1 01/13] pc/piix_pci: factor out smram/pam logic

2012-10-29 Thread Jason Baron
From: Isaku Yamahata 

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

[jba...@redhat.com: changes for updated memory API]
Signed-off-by: Isaku Yamahata 
Signed-off-by: Jason Baron 
---
 hw/i386/Makefile.objs |1 +
 hw/pam.c  |   87 
 hw/pam.h  |   97 +
 hw/piix_pci.c |   68 ---
 4 files changed, 200 insertions(+), 53 deletions(-)
 create mode 100644 hw/pam.c
 create mode 100644 hw/pam.h

diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
index d400d1a..693bd18 100644
--- a/hw/i386/Makefile.objs
+++ b/hw/i386/Makefile.objs
@@ -6,6 +6,7 @@ obj-y += pci-hotplug.o smbios.o wdt_ib700.o
 obj-y += debugcon.o multiboot.o
 obj-y += pc_piix.o
 obj-y += pc_sysfw.o
+obj-y += pam.o
 obj-y += acpi_ich9.o lpc_ich9.o smbus_ich9.o
 obj-$(CONFIG_XEN) += xen_platform.o xen_apic.o
 obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen-host-pci-device.o
diff --git a/hw/pam.c b/hw/pam.c
new file mode 100644
index 000..a95e2cf
--- /dev/null
+++ b/hw/pam.c
@@ -0,0 +1,87 @@
+/*
+ * QEMU i440FX/PIIX3 PCI Bridge Emulation
+ *
+ * Copyright (c) 2006 Fabrice Bellard
+ * Copyright (c) 2011 Isaku Yamahata 
+ *VA Linux Systems Japan K.K.
+ * Copyright (c) 2012 Jason Baron 
+ *
+ * Split out from piix_pci.c
+ *
+ * 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.
+ */
+#include "sysemu.h"
+#include "pam.h"
+
+void smram_update(MemoryRegion *smram_region, uint8_t smram,
+  uint8_t smm_enabled)
+{
+bool smram_enabled;
+
+smram_enabled = ((smm_enabled && (smram & SMRAM_G_SMRAME)) ||
+(smram & SMRAM_D_OPEN));
+memory_region_set_enabled(smram_region, !smram_enabled);
+}
+
+void smram_set_smm(uint8_t *host_smm_enabled, int smm, uint8_t smram,
+   MemoryRegion *smram_region)
+{
+uint8_t smm_enabled = (smm != 0);
+if (*host_smm_enabled != smm_enabled) {
+*host_smm_enabled = smm_enabled;
+smram_update(smram_region, smram, *host_smm_enabled);
+}
+}
+
+void init_pam(MemoryRegion *ram_memory, MemoryRegion *system_memory,
+  MemoryRegion *pci_address_space, PAMMemoryRegion *mem,
+  uint32_t start, uint32_t size)
+{
+int i;
+
+/* RAM */
+memory_region_init_alias(&mem->alias[3], "pam-ram", ram_memory,
+ start, size);
+/* ROM (XXX: not quite correct) */
+memory_region_init_alias(&mem->alias[1], "pam-rom", ram_memory,
+ start, size);
+memory_region_set_readonly(&mem->alias[1], true);
+
+/* XXX: should distinguish read/write cases */
+memory_region_init_alias(&mem->alias[0], "pam-pci", pci_address_space,
+ start, size);
+memory_region_init_alias(&mem->alias[2], "pam-pci", pci_address_space,
+ start, size);
+
+for (i = 0; i < 4; ++i) {
+memory_region_set_enabled(&mem->alias[i], false);
+memory_region_add_subregion_overlap(system_memory, start,
+&mem->alias[i], 1);
+}
+mem->current = 0;
+}
+
+void pam_update(PAMMemoryRegion *pam, int idx, uint8_t val)
+{
+assert(0 <= idx && idx <= 12);
+
+memory_region_set_enabled(&pam->alias[pam->current], false);
+pam->current = (val >> ((!(idx & 1)) * 4)) & PAM_ATTR_MASK;
+memory_region_set_enabled(&pam->alias[pam->current], true);
+}
diff --git a/hw/pam.h b/hw/pam.h
new file mode 100644
index 000..2d77ebe
--- /dev/null
+++ b/hw/pam.h
@@ -0,0 +1,97 @@
+#ifndef QEMU_PAM_H
+#define QEMU_PAM_H
+
+/*
+ * Copyright (c) 2006 Fabrice Bellard
+ * Copyright (c) 2011 Isaku Yamahata 
+ *   VA Linux Systems Japan K.K.
+ * Copyright (c) 2012 Jason Baron 
+ *
+ * Split out from piix_pci.c
+ *
+ * Permission is hereby granted, free of charge, to any per

Re: [Qemu-devel] [PATCH v1 01/13] pc/piix_pci: factor out smram/pam logic

2012-10-30 Thread Anthony Liguori
Jason Baron  writes:

> From: Isaku Yamahata 
>
> Factor out smram/pam logic for later use.
> Which will be used by q35 too.
>
> [jba...@redhat.com: changes for updated memory API]
> Signed-off-by: Isaku Yamahata 
> Signed-off-by: Jason Baron 

This is really not the right approach to solving this problem.

"pam" is not a device.

Instead, the common bits here are essentially the northbridge logic.
That's what we should model here.

It's pretty simple actually.  Just throw this all into a device where
the memory regions are owned by the device.  But also make the device
own creation of all RAM (not just the areas controlled by PAM).

Then embed the northbridge into the i440fx and q35.

Regards,

Anthony Liguori

> ---
>  hw/i386/Makefile.objs |1 +
>  hw/pam.c  |   87 
>  hw/pam.h  |   97 
> +
>  hw/piix_pci.c |   68 ---
>  4 files changed, 200 insertions(+), 53 deletions(-)
>  create mode 100644 hw/pam.c
>  create mode 100644 hw/pam.h
>
> diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
> index d400d1a..693bd18 100644
> --- a/hw/i386/Makefile.objs
> +++ b/hw/i386/Makefile.objs
> @@ -6,6 +6,7 @@ obj-y += pci-hotplug.o smbios.o wdt_ib700.o
>  obj-y += debugcon.o multiboot.o
>  obj-y += pc_piix.o
>  obj-y += pc_sysfw.o
> +obj-y += pam.o
>  obj-y += acpi_ich9.o lpc_ich9.o smbus_ich9.o
>  obj-$(CONFIG_XEN) += xen_platform.o xen_apic.o
>  obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen-host-pci-device.o
> diff --git a/hw/pam.c b/hw/pam.c
> new file mode 100644
> index 000..a95e2cf
> --- /dev/null
> +++ b/hw/pam.c
> @@ -0,0 +1,87 @@
> +/*
> + * QEMU i440FX/PIIX3 PCI Bridge Emulation
> + *
> + * Copyright (c) 2006 Fabrice Bellard
> + * Copyright (c) 2011 Isaku Yamahata 
> + *VA Linux Systems Japan K.K.
> + * Copyright (c) 2012 Jason Baron 
> + *
> + * Split out from piix_pci.c
> + *
> + * 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.
> + */
> +#include "sysemu.h"
> +#include "pam.h"
> +
> +void smram_update(MemoryRegion *smram_region, uint8_t smram,
> +  uint8_t smm_enabled)
> +{
> +bool smram_enabled;
> +
> +smram_enabled = ((smm_enabled && (smram & SMRAM_G_SMRAME)) ||
> +(smram & SMRAM_D_OPEN));
> +memory_region_set_enabled(smram_region, !smram_enabled);
> +}
> +
> +void smram_set_smm(uint8_t *host_smm_enabled, int smm, uint8_t smram,
> +   MemoryRegion *smram_region)
> +{
> +uint8_t smm_enabled = (smm != 0);
> +if (*host_smm_enabled != smm_enabled) {
> +*host_smm_enabled = smm_enabled;
> +smram_update(smram_region, smram, *host_smm_enabled);
> +}
> +}
> +
> +void init_pam(MemoryRegion *ram_memory, MemoryRegion *system_memory,
> +  MemoryRegion *pci_address_space, PAMMemoryRegion *mem,
> +  uint32_t start, uint32_t size)
> +{
> +int i;
> +
> +/* RAM */
> +memory_region_init_alias(&mem->alias[3], "pam-ram", ram_memory,
> + start, size);
> +/* ROM (XXX: not quite correct) */
> +memory_region_init_alias(&mem->alias[1], "pam-rom", ram_memory,
> + start, size);
> +memory_region_set_readonly(&mem->alias[1], true);
> +
> +/* XXX: should distinguish read/write cases */
> +memory_region_init_alias(&mem->alias[0], "pam-pci", pci_address_space,
> + start, size);
> +memory_region_init_alias(&mem->alias[2], "pam-pci", pci_address_space,
> + start, size);
> +
> +for (i = 0; i < 4; ++i) {
> +memory_region_set_enabled(&mem->alias[i], false);
> +memory_region_add_subregion_overlap(system_memory, start,
> +&mem->alias[i], 1);
> +}
> +mem->current = 0;

Re: [Qemu-devel] [PATCH v1 01/13] pc/piix_pci: factor out smram/pam logic

2012-10-30 Thread Andreas Färber
Am 30.10.2012 20:07, schrieb Anthony Liguori:
> Jason Baron  writes:
> 
>> From: Isaku Yamahata 
>>
>> Factor out smram/pam logic for later use.
>> Which will be used by q35 too.
>>
>> [jba...@redhat.com: changes for updated memory API]
>> Signed-off-by: Isaku Yamahata 
>> Signed-off-by: Jason Baron 
> 
> This is really not the right approach to solving this problem.
> 
> "pam" is not a device.
> 
> Instead, the common bits here are essentially the northbridge logic.
> That's what we should model here.
> 
> It's pretty simple actually.  Just throw this all into a device where
> the memory regions are owned by the device.  But also make the device
> own creation of all RAM (not just the areas controlled by PAM).
> 
> Then embed the northbridge into the i440fx and q35.

Anthony/Wanpeng, any update on i440fx? Haven't heard of it since we
finally merged the PCI host bridge stuff for v1.2...

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg