From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:47906) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TSs3c-0006e5-Id for qemu-devel@nongnu.org; Mon, 29 Oct 2012 12:19:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TSs3U-0005jD-7g for qemu-devel@nongnu.org; Mon, 29 Oct 2012 12:19:20 -0400 Received: from mx1.redhat.com ([209.132.183.28]:19200) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TSs3T-0005j0-St for qemu-devel@nongnu.org; Mon, 29 Oct 2012 12:19:12 -0400 Date: Mon, 29 Oct 2012 18:21:26 +0200 From: "Michael S. Tsirkin" Message-ID: <20121029162126.GB21425@redhat.com> References: <402b9d2b657a7ac080b7892355970572c81be7e6.1350677361.git.jbaron@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <402b9d2b657a7ac080b7892355970572c81be7e6.1350677361.git.jbaron@redhat.com> Subject: Re: [Qemu-devel] [PATCH v3 07/26] pc/piix_pci: factor out smram/pam logic List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jason Baron Cc: agraf@suse.de, aliguori@us.ibm.com, juzhang@redhat.com, jan.kiszka@siemens.com, qemu-devel@nongnu.org, armbru@redhat.com, blauwirbel@gmail.com, yamahata@valinux.co.jp, alex.williamson@redhat.com, kevin@koconnor.net, avi@redhat.com, mkletzan@redhat.com, pbonzini@redhat.com, lcapitulino@redhat.com, afaerber@suse.de, kraxel@redhat.com On Fri, Oct 19, 2012 at 04:43:30PM -0400, Jason Baron wrote: > From: Isaku Yamahata > > Factor out smram/pam logic for later use. > Which will be used by q35 too. > > Reviewed-by: Paolo Bonzini > [jbaron@redhat.com: changes for updated memory API] > Signed-off-by: Isaku Yamahata > Signed-off-by: Jason Baron I dropped this patch from pci branch for now as it needs to be rebased. Can you do this pls? > --- > hw/i386/Makefile.objs | 1 + > hw/pam.c | 120 +++++++++++++++++++++++++++++++++++++++++++++++++ > hw/pam.h | 98 ++++++++++++++++++++++++++++++++++++++++ > hw/piix_pci.c | 65 ++++---------------------- > 4 files changed, 229 insertions(+), 55 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 8c764bb..2f0c172 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-$(CONFIG_XEN) += xen_platform.o xen_apic.o > obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen-host-pci-device.o > obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pt.o xen_pt_config_init.o xen_pt_msi.o > diff --git a/hw/pam.c b/hw/pam.c > new file mode 100644 > index 0000000..9ec5861 > --- /dev/null > +++ b/hw/pam.c > @@ -0,0 +1,120 @@ > +/* > + * 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 > + * VA Linux Systems Japan K.K. > + * Copyright (c) 2012 Jason Baron > + * > + */ > + > +#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); > + } > +} > + > +static void pam_update_seg(PAMMemoryRegion *mem, uint32_t start, uint32_t size, > + MemoryRegion *ram_memory, > + MemoryRegion *pci_address_space, > + MemoryRegion *system_memory, uint8_t attr) > +{ > + if (mem->initialized) { > + memory_region_del_subregion(system_memory, &mem->mem); > + memory_region_destroy(&mem->mem); > + } > + > + switch (attr) { > + case PAM_ATTR_WE | PAM_ATTR_RE: > + /* RAM */ > + memory_region_init_alias(&mem->mem, "pam-ram", ram_memory, > + start, size); > + break; > + case PAM_ATTR_RE: > + /* ROM (XXX: not quite correct) */ > + memory_region_init_alias(&mem->mem, "pam-rom", ram_memory, > + start, size); > + memory_region_set_readonly(&mem->mem, true); > + break; > + case PAM_ATTR_WE: > + case 0: > + /* XXX: should distinguish read/write cases */ > + memory_region_init_alias(&mem->mem, "pam-pci", pci_address_space, > + start, size); > + break; > + default: > + abort(); > + break; > + } > + memory_region_add_subregion_overlap(system_memory, start, &mem->mem, 1); > + mem->initialized = true; > + > +} > + > +static uint8_t pam_attr(uint8_t val, int hi) > +{ > + return (val >> ((!!hi) * 4)) & PAM_ATTR_MASK; > +} > + > +void pam_update(PAMMemoryRegion *mem, int idx, uint8_t val, > + MemoryRegion *ram_memory, MemoryRegion *pci_address_space, > + MemoryRegion *system_memory) > +{ > + uint32_t phys_addr; > + int map_idx; > + > + assert(0 <= idx && idx <= PAM_IDX_MAX); > + > + if (idx == 0) { > + pam_update_seg(&mem[0], PAM_BIOS_BASE, PAM_BIOS_SIZE, ram_memory, > + pci_address_space, system_memory, pam_attr(val, 1)); > + return; > + } > + > + map_idx = (idx - 1) * 2; > + > + phys_addr = PAM_EXPAN_BASE + PAM_EXPAN_SIZE * map_idx; > + pam_update_seg(&mem[map_idx + 1], phys_addr, PAM_EXPAN_SIZE, ram_memory, > + pci_address_space, system_memory, pam_attr(val, 0)); > + > + phys_addr += PAM_EXPAN_SIZE; > + pam_update_seg(&mem[map_idx + 2], phys_addr, PAM_EXPAN_SIZE, ram_memory, > + pci_address_space, system_memory, pam_attr(val, 1)); > +} > diff --git a/hw/pam.h b/hw/pam.h > new file mode 100644 > index 0000000..ce89a2a > --- /dev/null > +++ b/hw/pam.h > @@ -0,0 +1,98 @@ > +#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 > + * VA Linux Systems Japan K.K. > + * Copyright (c) 2012 Jason Baron > + * > + * 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" > +#include "memory.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 0xfffff > +/* 64KB: Intel 3 series express chipset family p. 58*/ > +#define PAM_BIOS_SIZE 0x10000 > + > +/* 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 */ > + > +typedef struct PAMMemoryRegion { > + MemoryRegion mem; > + bool initialized; > +} PAMMemoryRegion; > + > +void smram_update(MemoryRegion *smram_region, uint8_t smram, > + uint8_t smm_enabled); > +void smram_set_smm(uint8_t *host_smm_enabled, int smm, uint8_t smram, > + MemoryRegion *smram_region); > +void pam_update(PAMMemoryRegion *mem, int idx, uint8_t val, > + MemoryRegion *ram_memory, MemoryRegion *pci_address_space, > + MemoryRegion *system_memory); > + > +#endif /* QEMU_PAM_H */ > diff --git a/hw/piix_pci.c b/hw/piix_pci.c > index 537fc19..02b161d 100644 > --- a/hw/piix_pci.c > +++ b/hw/piix_pci.c > @@ -30,6 +30,7 @@ > #include "sysbus.h" > #include "range.h" > #include "xen.h" > +#include "pam.h" > > /* > * I440FX chipset data sheet. > @@ -68,11 +69,6 @@ typedef struct PIIX3State { > int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS]; > } PIIX3State; > > -typedef struct PAMMemoryRegion { > - MemoryRegion mem; > - bool initialized; > -} PAMMemoryRegion; > - > struct PCII440FXState { > PCIDevice dev; > MemoryRegion *system_memory; > @@ -105,56 +101,16 @@ static int pci_slot_get_pirq(PCIDevice *pci_dev, int pci_intx) > return (pci_intx + slot_addend) & 3; > } > > -static void update_pam(PCII440FXState *d, uint32_t start, uint32_t end, int r, > - PAMMemoryRegion *mem) > -{ > - if (mem->initialized) { > - memory_region_del_subregion(d->system_memory, &mem->mem); > - memory_region_destroy(&mem->mem); > - } > - > - // printf("ISA mapping %08x-0x%08x: %d\n", start, end, r); > - switch(r) { > - case 3: > - /* RAM */ > - memory_region_init_alias(&mem->mem, "pam-ram", d->ram_memory, > - start, end - start); > - break; > - case 1: > - /* ROM (XXX: not quite correct) */ > - memory_region_init_alias(&mem->mem, "pam-rom", d->ram_memory, > - start, end - start); > - memory_region_set_readonly(&mem->mem, true); > - break; > - case 2: > - case 0: > - /* XXX: should distinguish read/write cases */ > - memory_region_init_alias(&mem->mem, "pam-pci", d->pci_address_space, > - start, end - start); > - break; > - } > - memory_region_add_subregion_overlap(d->system_memory, > - start, &mem->mem, 1); > - mem->initialized = true; > -} > - > static void i440fx_update_memory_mappings(PCII440FXState *d) > { > - int i, r; > - uint32_t smram; > - bool smram_enabled; > + int i; > > memory_region_transaction_begin(); > - update_pam(d, 0xf0000, 0x100000, (d->dev.config[I440FX_PAM] >> 4) & 3, > - &d->pam_regions[0]); > - 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, > - &d->pam_regions[i+1]); > + for (i = 0; i <= PAM_IDX_MAX; i++) { > + pam_update(&d->pam_regions[0], i, d->dev.config[I440FX_PAM + i], > + d->ram_memory, d->pci_address_space, d->system_memory); > } > - smram = d->dev.config[I440FX_SMRAM]; > - smram_enabled = (d->smm_enabled && (smram & 0x08)) || (smram & 0x40); > - memory_region_set_enabled(&d->smram_region, !smram_enabled); > + smram_update(&d->smram_region, d->dev.config[I440FX_SMRAM], d->smm_enabled); > memory_region_transaction_commit(); > } > > @@ -162,11 +118,10 @@ 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); > - } > + memory_region_transaction_begin(); > + smram_set_smm(&d->smm_enabled, val, d->dev.config[I440FX_SMRAM], > + &d->smram_region); > + memory_region_transaction_commit(); > } > > > -- > 1.7.1