From patchwork Sun Mar 4 21:56:33 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mark Cave-Ayland X-Patchwork-Id: 144542 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 62E8BB6EEA for ; Mon, 5 Mar 2012 08:56:47 +1100 (EST) Received: from localhost ([::1]:35075 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S4JQ5-0002cQ-CP for incoming@patchwork.ozlabs.org; Sun, 04 Mar 2012 16:56:45 -0500 Received: from eggs.gnu.org ([208.118.235.92]:50831) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S4JPw-0002c9-Dq for qemu-devel@nongnu.org; Sun, 04 Mar 2012 16:56:38 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1S4JPu-0003Dj-88 for qemu-devel@nongnu.org; Sun, 04 Mar 2012 16:56:35 -0500 Received: from p15195424.pureserver.info ([82.165.34.74]:36269) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S4JPt-0003Dd-RX for qemu-devel@nongnu.org; Sun, 04 Mar 2012 16:56:34 -0500 Received: from 93-97-95-250.zone5.bethere.co.uk ([93.97.95.250] helo=[192.168.1.76]) by p15195424.pureserver.info with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.43) id 1S4JPp-00063Q-Qj for qemu-devel@nongnu.org; Sun, 04 Mar 2012 21:56:32 +0000 Message-ID: <4F53E511.5060801@ilande.co.uk> Date: Sun, 04 Mar 2012 21:56:33 +0000 From: Mark Cave-Ayland User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.16) Gecko/20120207 Icedove/3.0.11 MIME-Version: 1.0 To: qemu-devel@nongnu.org References: <20120304094614.GA8158@redhat.com> <20120304122100.GA11207@redhat.com> <20120304132805.GC12047@redhat.com> <20120304142333.GA12900@redhat.com> <20120304152201.GC12776@redhat.com> <20120304173428.GA16058@redhat.com> In-Reply-To: X-SA-Exim-Connect-IP: 93.97.95.250 X-SA-Exim-Mail-From: mark.cave-ayland@ilande.co.uk X-SA-Exim-Version: 4.1 (built Wed, 05 Jan 2005 10:54:05 -0500) X-SA-Exim-Scanned: Yes (on p15195424.pureserver.info) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6, seldom 2.4 (older, 4) X-Received-From: 82.165.34.74 Subject: Re: [Qemu-devel] [PATCH] pci: fix bridge IO/BASE 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 On 04/03/12 19:51, Blue Swirl wrote: > I now know the root cause of the problem. OpenBIOS programs the BARs > somewhat correctly just by accident. The initial io_base and mem_base > for BARs are not correct, but because the host bridge BARs (and also 6 > of which 4 are not even BARs!) are programmed first, the bases > happened to settle to values that happen to work. The commit revealed > the problem since the settling didn't happen. The mask changes just > let the host bridge setup continue to do the magic. > > By just changing OpenBIOS (see attached patch), I can get the devices > to work (assuming that VGA is a separate problem). There's no need to > change QEMU. Hi Blue/Michael, Thanks for debugging this. I can confirm that building OpenBIOS SVN trunk with Blue's patch and testing against QEMU git master with the attached patch (to temporarily back out 2b50aa1f14d8e0a40f905ad0c022443c15646914 and de58ac72b6a062d1a61478284c0c0f8a0428613e which breaks VGA on PPC/SPARC64) appears to fix the problem for me. Thanks a lot to both of you for taking the time to track this down :) ATB, Mark. diff --git a/ioport.c b/ioport.c index 8a474d3..36fa3a4 100644 --- a/ioport.c +++ b/ioport.c @@ -328,7 +328,6 @@ void portio_list_init(PortioList *piolist, piolist->ports = callbacks; piolist->nr = 0; piolist->regions = g_new0(MemoryRegion *, n); - piolist->aliases = g_new0(MemoryRegion *, n); piolist->address_space = NULL; piolist->opaque = opaque; piolist->name = name; @@ -337,7 +336,6 @@ void portio_list_init(PortioList *piolist, void portio_list_destroy(PortioList *piolist) { g_free(piolist->regions); - g_free(piolist->aliases); } static void portio_list_add_1(PortioList *piolist, @@ -347,7 +345,7 @@ static void portio_list_add_1(PortioList *piolist, { MemoryRegionPortio *pio; MemoryRegionOps *ops; - MemoryRegion *region, *alias; + MemoryRegion *region; unsigned i; /* Copy the sub-list and null-terminate it. */ @@ -364,20 +362,12 @@ static void portio_list_add_1(PortioList *piolist, ops->old_portio = pio; region = g_new(MemoryRegion, 1); - alias = g_new(MemoryRegion, 1); - /* - * Use an alias so that the callback is called with an absolute address, - * rather than an offset relative to to start + off_low. - */ memory_region_init_io(region, ops, piolist->opaque, piolist->name, - UINT64_MAX); - memory_region_init_alias(alias, piolist->name, - region, start + off_low, off_high - off_low); + off_high - off_low); + memory_region_set_offset(region, start + off_low); memory_region_add_subregion(piolist->address_space, - start + off_low, alias); - piolist->regions[piolist->nr] = region; - piolist->aliases[piolist->nr] = alias; - ++piolist->nr; + start + off_low, region); + piolist->regions[piolist->nr++] = region; } void portio_list_add(PortioList *piolist, @@ -419,19 +409,15 @@ void portio_list_add(PortioList *piolist, void portio_list_del(PortioList *piolist) { - MemoryRegion *mr, *alias; + MemoryRegion *mr; unsigned i; for (i = 0; i < piolist->nr; ++i) { mr = piolist->regions[i]; - alias = piolist->aliases[i]; - memory_region_del_subregion(piolist->address_space, alias); - memory_region_destroy(alias); + memory_region_del_subregion(piolist->address_space, mr); memory_region_destroy(mr); g_free((MemoryRegionOps *)mr->ops); g_free(mr); - g_free(alias); piolist->regions[i] = NULL; - piolist->aliases[i] = NULL; } } diff --git a/ioport.h b/ioport.h index ab29c89..ae3e9da 100644 --- a/ioport.h +++ b/ioport.h @@ -60,7 +60,6 @@ typedef struct PortioList { struct MemoryRegion *address_space; unsigned nr; struct MemoryRegion **regions; - struct MemoryRegion **aliases; void *opaque; const char *name; } PortioList; diff --git a/memory.c b/memory.c index 6565e2e..926201a 100644 --- a/memory.c +++ b/memory.c @@ -389,17 +389,17 @@ static void memory_region_iorange_read(IORange *iorange, *data = ((uint64_t)1 << (width * 8)) - 1; if (mrp) { - *data = mrp->read(mr->opaque, offset); + *data = mrp->read(mr->opaque, offset + mr->offset); } else if (width == 2) { mrp = find_portio(mr, offset, 1, false); assert(mrp); - *data = mrp->read(mr->opaque, offset) | - (mrp->read(mr->opaque, offset + 1) << 8); + *data = mrp->read(mr->opaque, offset + mr->offset) | + (mrp->read(mr->opaque, offset + mr->offset + 1) << 8); } return; } *data = 0; - access_with_adjusted_size(offset, data, width, + access_with_adjusted_size(offset + mr->offset, data, width, mr->ops->impl.min_access_size, mr->ops->impl.max_access_size, memory_region_read_accessor, mr); @@ -416,16 +416,16 @@ static void memory_region_iorange_write(IORange *iorange, const MemoryRegionPortio *mrp = find_portio(mr, offset, width, true); if (mrp) { - mrp->write(mr->opaque, offset, data); + mrp->write(mr->opaque, offset + mr->offset, data); } else if (width == 2) { mrp = find_portio(mr, offset, 1, false); assert(mrp); - mrp->write(mr->opaque, offset, data & 0xff); - mrp->write(mr->opaque, offset + 1, data >> 8); + mrp->write(mr->opaque, offset + mr->offset, data & 0xff); + mrp->write(mr->opaque, offset + mr->offset + 1, data >> 8); } return; } - access_with_adjusted_size(offset, &data, width, + access_with_adjusted_size(offset + mr->offset, &data, width, mr->ops->impl.min_access_size, mr->ops->impl.max_access_size, memory_region_write_accessor, mr); @@ -796,6 +796,7 @@ void memory_region_init(MemoryRegion *mr, mr->size = int128_2_64(); } mr->addr = 0; + mr->offset = 0; mr->subpage = false; mr->enabled = true; mr->terminates = false; @@ -857,7 +858,7 @@ static uint64_t memory_region_dispatch_read1(MemoryRegion *mr, } /* FIXME: support unaligned access */ - access_with_adjusted_size(addr, &data, size, + access_with_adjusted_size(addr + mr->offset, &data, size, mr->ops->impl.min_access_size, mr->ops->impl.max_access_size, memory_region_read_accessor, mr); @@ -911,7 +912,7 @@ static void memory_region_dispatch_write(MemoryRegion *mr, } /* FIXME: support unaligned access */ - access_with_adjusted_size(addr, &data, size, + access_with_adjusted_size(addr + mr->offset, &data, size, mr->ops->impl.min_access_size, mr->ops->impl.max_access_size, memory_region_write_accessor, mr); @@ -1054,6 +1055,11 @@ bool memory_region_is_rom(MemoryRegion *mr) return mr->ram && mr->readonly; } +void memory_region_set_offset(MemoryRegion *mr, target_phys_addr_t offset) +{ + mr->offset = offset; +} + void memory_region_set_log(MemoryRegion *mr, bool log, unsigned client) { uint8_t mask = 1 << client; diff --git a/memory.h b/memory.h index b7bccd1..a9cd586 100644 --- a/memory.h +++ b/memory.h @@ -115,6 +115,7 @@ struct MemoryRegion { MemoryRegion *parent; Int128 size; target_phys_addr_t addr; + target_phys_addr_t offset; void (*destructor)(MemoryRegion *mr); ram_addr_t ram_addr; IORange iorange; @@ -370,6 +371,14 @@ bool memory_region_is_rom(MemoryRegion *mr); void *memory_region_get_ram_ptr(MemoryRegion *mr); /** + * memory_region_set_offset: Sets an offset to be added to MemoryRegionOps + * callbacks. + * + * This function is deprecated and should not be used in new code. + */ +void memory_region_set_offset(MemoryRegion *mr, target_phys_addr_t offset); + +/** * memory_region_set_log: Turn dirty logging on or off for a region. * * Turns dirty logging on or off for a specified client (display, migration).