From patchwork Sun Sep 4 18:13:13 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Michael S. Tsirkin" X-Patchwork-Id: 113293 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [140.186.70.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 357D4B6F75 for ; Mon, 5 Sep 2011 04:12:39 +1000 (EST) Received: from localhost ([::1]:43725 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R0HBI-0003TZ-E2 for incoming@patchwork.ozlabs.org; Sun, 04 Sep 2011 14:12:32 -0400 Received: from eggs.gnu.org ([140.186.70.92]:40996) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R0HBB-0003TJ-2f for qemu-devel@nongnu.org; Sun, 04 Sep 2011 14:12:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1R0HB9-0004in-Bf for qemu-devel@nongnu.org; Sun, 04 Sep 2011 14:12:25 -0400 Received: from mx1.redhat.com ([209.132.183.28]:12383) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R0HB8-0004ig-C6 for qemu-devel@nongnu.org; Sun, 04 Sep 2011 14:12:23 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p84ICHZI006354 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Sun, 4 Sep 2011 14:12:17 -0400 Received: from redhat.com (vpn-200-96.tlv.redhat.com [10.35.200.96]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with SMTP id p84ICD2h006904; Sun, 4 Sep 2011 14:12:14 -0400 Date: Sun, 4 Sep 2011 21:13:13 +0300 From: "Michael S. Tsirkin" To: Wen Congyang , qemu-devel@nongnu.org, Kevin Wolf , Anthony Liguori , Isaku Yamahata Message-ID: <20110904181313.GA14020@redhat.com> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH] pci: implement bridge filtering X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Support bridge filtering on top of the memory API as suggested by Avi Kivity: Create a memory region for the bridge's address space. This region is not directly added to system_memory or its descendants. Devices under the bridge see this region as its pci_address_space(). The region is as large as the entire address space - it does not take into account any windows. For each of the three windows (pref, non-pref, vga), create an alias with the appropriate start and size. Map the alias into the bridge's parent's pci_address_space(), as subregions. Signed-off-by: Michael S. Tsirkin --- The below seems to work fine for me so I applied this. Still need to test bridge filtering, any help with this appreciated. hw/pci.c | 70 +--------------------------------------- hw/pci.h | 2 - hw/pci_bridge.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++++--- hw/pci_internals.h | 3 ++ 4 files changed, 89 insertions(+), 75 deletions(-) diff --git a/hw/pci.c b/hw/pci.c index 57ff7b1..56dfa18 100644 --- a/hw/pci.c +++ b/hw/pci.c @@ -889,7 +889,6 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num, r = &pci_dev->io_regions[region_num]; r->addr = PCI_BAR_UNMAPPED; r->size = size; - r->filtered_size = size; r->type = type; r->memory = NULL; @@ -920,41 +919,6 @@ pcibus_t pci_get_bar_addr(PCIDevice *pci_dev, int region_num) return pci_dev->io_regions[region_num].addr; } -static void pci_bridge_filter(PCIDevice *d, pcibus_t *addr, pcibus_t *size, - uint8_t type) -{ - pcibus_t base = *addr; - pcibus_t limit = *addr + *size - 1; - PCIDevice *br; - - for (br = d->bus->parent_dev; br; br = br->bus->parent_dev) { - uint16_t cmd = pci_get_word(d->config + PCI_COMMAND); - - if (type & PCI_BASE_ADDRESS_SPACE_IO) { - if (!(cmd & PCI_COMMAND_IO)) { - goto no_map; - } - } else { - if (!(cmd & PCI_COMMAND_MEMORY)) { - goto no_map; - } - } - - base = MAX(base, pci_bridge_get_base(br, type)); - limit = MIN(limit, pci_bridge_get_limit(br, type)); - } - - if (base > limit) { - goto no_map; - } - *addr = base; - *size = limit - base + 1; - return; -no_map: - *addr = PCI_BAR_UNMAPPED; - *size = 0; -} - static pcibus_t pci_bar_address(PCIDevice *d, int reg, uint8_t type, pcibus_t size) { @@ -1024,7 +988,7 @@ static void pci_update_mappings(PCIDevice *d) { PCIIORegion *r; int i; - pcibus_t new_addr, filtered_size; + pcibus_t new_addr; for(i = 0; i < PCI_NUM_REGIONS; i++) { r = &d->io_regions[i]; @@ -1035,14 +999,8 @@ static void pci_update_mappings(PCIDevice *d) new_addr = pci_bar_address(d, i, r->type, r->size); - /* bridge filtering */ - filtered_size = r->size; - if (new_addr != PCI_BAR_UNMAPPED) { - pci_bridge_filter(d, &new_addr, &filtered_size, r->type); - } - /* This bar isn't changed */ - if (new_addr == r->addr && filtered_size == r->filtered_size) + if (new_addr == r->addr) continue; /* now do the real mapping */ @@ -1050,15 +1008,7 @@ static void pci_update_mappings(PCIDevice *d) memory_region_del_subregion(r->address_space, r->memory); } r->addr = new_addr; - r->filtered_size = filtered_size; if (r->addr != PCI_BAR_UNMAPPED) { - /* - * TODO: currently almost all the map funcions assumes - * filtered_size == size and addr & ~(size - 1) == addr. - * However with bridge filtering, they aren't always true. - * Teach them such cases, such that filtered_size < size and - * addr & (size - 1) != 0. - */ if (r->type & PCI_BASE_ADDRESS_SPACE_IO) { memory_region_add_subregion_overlap(r->address_space, r->addr, @@ -1576,22 +1526,6 @@ PCIDevice *pci_nic_init_nofail(NICInfo *nd, const char *default_model, return res; } -static void pci_bridge_update_mappings_fn(PCIBus *b, PCIDevice *d) -{ - pci_update_mappings(d); -} - -void pci_bridge_update_mappings(PCIBus *b) -{ - PCIBus *child; - - pci_for_each_device_under_bus(b, pci_bridge_update_mappings_fn); - - QLIST_FOREACH(child, &b->child, sibling) { - pci_bridge_update_mappings(child); - } -} - /* Whether a given bus number is in range of the secondary * bus of the given bridge device. */ static bool pci_secondary_bus_in_range(PCIDevice *dev, int bus_num) diff --git a/hw/pci.h b/hw/pci.h index 391217e..65e1568 100644 --- a/hw/pci.h +++ b/hw/pci.h @@ -90,7 +90,6 @@ typedef struct PCIIORegion { pcibus_t addr; /* current PCI mapping address. -1 means not mapped */ #define PCI_BAR_UNMAPPED (~(pcibus_t)0) pcibus_t size; - pcibus_t filtered_size; uint8_t type; MemoryRegion *memory; MemoryRegion *address_space; @@ -277,7 +276,6 @@ int pci_read_devaddr(Monitor *mon, const char *addr, int *domp, int *busp, void do_pci_info_print(Monitor *mon, const QObject *data); void do_pci_info(Monitor *mon, QObject **ret_data); -void pci_bridge_update_mappings(PCIBus *b); void pci_device_deassert_intx(PCIDevice *dev); diff --git a/hw/pci_bridge.c b/hw/pci_bridge.c index e0b339e..b488f06 100644 --- a/hw/pci_bridge.c +++ b/hw/pci_bridge.c @@ -135,6 +135,72 @@ pcibus_t pci_bridge_get_limit(const PCIDevice *bridge, uint8_t type) return limit; } +static void pci_bridge_init_alias(PCIBridge *bridge, MemoryRegion *alias, + uint8_t type, const char *name, + MemoryRegion *space, + MemoryRegion *parent_space) +{ + pcibus_t base = pci_bridge_get_base(&bridge->dev, type); + pcibus_t limit = pci_bridge_get_limit(&bridge->dev, type); + /* TODO: this doesn't handle base = 0 limit = 2^64 - 1 correctly. + * Apparently no way to do this with existing memory APIs. */ + pcibus_t size = limit >= base ? limit + 1 - base : 0; + + memory_region_init_alias(alias, name, space, base, size); + memory_region_add_subregion_overlap(parent_space, base, alias, 1); +} + +static void pci_bridge_cleanup_alias(MemoryRegion *alias, + MemoryRegion *parent_space) +{ + memory_region_del_subregion(parent_space, alias); + memory_region_destroy(alias); +} + +static void pci_bridge_region_init(PCIBridge *br) +{ + PCIBus *sec_bus = &br->sec_bus; + PCIBus *parent = br->dev.bus; + pci_bridge_init_alias(br, sec_bus->alias_pref_mem, + PCI_BASE_ADDRESS_MEM_PREFETCH, + "pci_bridge_pref_mem", + sec_bus->address_space_mem, + parent->address_space_mem); + pci_bridge_init_alias(br, sec_bus->alias_mem, + PCI_BASE_ADDRESS_SPACE_MEMORY, + "pci_bridge_mem", + sec_bus->address_space_mem, + parent->address_space_mem); + pci_bridge_init_alias(br, sec_bus->alias_io, + PCI_BASE_ADDRESS_SPACE_IO, + "pci_bridge_io", + sec_bus->address_space_io, + parent->address_space_io); + /* TODO: VGA and VGA palatte snooping support. */ +} + +static void pci_bridge_region_cleanup(PCIBridge *br) +{ + PCIBus *sec_bus = &br->sec_bus; + PCIBus *parent = br->dev.bus; + pci_bridge_cleanup_alias(sec_bus->alias_io, + parent->address_space_io); + pci_bridge_cleanup_alias(sec_bus->alias_mem, + parent->address_space_mem); + pci_bridge_cleanup_alias(sec_bus->alias_pref_mem, + parent->address_space_mem); +} + +static void pci_bridge_update_mappings(PCIBridge *br) +{ + /* Make updates atomic to: handle the case of one VCPU updating the bridge + * while another accesses an unaffected region. */ + memory_region_transaction_begin(); + pci_bridge_region_cleanup(br); + pci_bridge_region_init(br); + memory_region_transaction_commit(); +} + /* default write_config function for PCI-to-PCI bridge */ void pci_bridge_write_config(PCIDevice *d, uint32_t address, uint32_t val, int len) @@ -151,7 +217,7 @@ void pci_bridge_write_config(PCIDevice *d, /* memory base/limit, prefetchable base/limit and io base/limit upper 16 */ ranges_overlap(address, len, PCI_MEMORY_BASE, 20)) { - pci_bridge_update_mappings(&s->sec_bus); + pci_bridge_update_mappings(s); } newctl = pci_get_word(d->config + PCI_BRIDGE_CONTROL); @@ -246,10 +312,14 @@ int pci_bridge_initfn(PCIDevice *dev) br->bus_name); sec_bus->parent_dev = dev; sec_bus->map_irq = br->map_irq; - /* TODO: use memory API to perform memory filtering. */ - sec_bus->address_space_mem = parent->address_space_mem; - sec_bus->address_space_io = parent->address_space_io; - + sec_bus->address_space_mem = g_new(MemoryRegion, 1); + memory_region_init(sec_bus->address_space_mem, "pci_pridge_pci", INT64_MAX); + sec_bus->address_space_io = g_new(MemoryRegion, 1); + memory_region_init(sec_bus->address_space_io, "pci_bridge_io", 65536); + sec_bus->alias_pref_mem = g_new(MemoryRegion, 1); + sec_bus->alias_mem = g_new(MemoryRegion, 1); + sec_bus->alias_io = g_new(MemoryRegion, 1); + pci_bridge_region_init(br); QLIST_INIT(&sec_bus->child); QLIST_INSERT_HEAD(&parent->child, sec_bus, sibling); return 0; @@ -259,8 +329,17 @@ int pci_bridge_initfn(PCIDevice *dev) int pci_bridge_exitfn(PCIDevice *pci_dev) { PCIBridge *s = DO_UPCAST(PCIBridge, dev, pci_dev); + PCIBus *sec_bus = &s->sec_bus; assert(QLIST_EMPTY(&s->sec_bus.child)); QLIST_REMOVE(&s->sec_bus, sibling); + pci_bridge_region_cleanup(s); + g_free(sec_bus->alias_pref_mem); + g_free(sec_bus->alias_mem); + g_free(sec_bus->alias_io); + memory_region_destroy(sec_bus->address_space_mem); + g_free(sec_bus->address_space_mem); + memory_region_destroy(sec_bus->address_space_io); + g_free(sec_bus->address_space_io); /* qbus_free() is called automatically by qdev_free() */ return 0; } diff --git a/hw/pci_internals.h b/hw/pci_internals.h index c7fd23d..578c8d2 100644 --- a/hw/pci_internals.h +++ b/hw/pci_internals.h @@ -27,6 +27,9 @@ struct PCIBus { target_phys_addr_t mem_base; MemoryRegion *address_space_mem; MemoryRegion *address_space_io; + MemoryRegion *alias_pref_mem; + MemoryRegion *alias_mem; + MemoryRegion *alias_io; QLIST_HEAD(, PCIBus) child; /* this will be replaced by qdev later */ QLIST_ENTRY(PCIBus) sibling;/* this will be replaced by qdev later */