From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:41472) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WBNuA-0004gP-Vu for qemu-devel@nongnu.org; Thu, 06 Feb 2014 07:18:13 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WBNu4-0008Ki-To for qemu-devel@nongnu.org; Thu, 06 Feb 2014 07:18:06 -0500 Received: from mx1.redhat.com ([209.132.183.28]:44841) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WBNu4-0008Ke-Ir for qemu-devel@nongnu.org; Thu, 06 Feb 2014 07:18:00 -0500 Date: Thu, 6 Feb 2014 14:22:54 +0200 From: "Michael S. Tsirkin" Message-ID: <20140206122254.GA12290@redhat.com> References: <1391682273-8457-1-git-send-email-imammedo@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1391682273-8457-1-git-send-email-imammedo@redhat.com> Subject: Re: [Qemu-devel] [PATCH] memory_region_present: return false if address is not found in child MemoryRegion List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: peter.maydell@linaro.org, jan.kiszka@siemens.com, qemu-devel@nongnu.org, anthony@codemonkey.ws, pbonzini@redhat.com, rth@twiddle.net On Thu, Feb 06, 2014 at 11:24:33AM +0100, Igor Mammedov wrote: > Windows XP shows COM2 port as non functional in > "Device Manager" although no COM2 port backing device > is present in QEMU. > > That is caused by the fact that QEMU reports to > OSPM that device is present by setting 5th bit in > PII4XPM.pci_conf[0x67] register when COM2 doesn't > exist. > > It happens due to memory_region_present(io_as, 0x2f8) > returning false positive since 0x2f8 address eventually > translates into catchall io_as address space. > > Fix memory_region_present(parent, addr) by returning > true only if addr maps into a MemoryRegion within > parent (excluding parent itself), to match its > doc comment. > > While at it fix copy/paste error in > memory_region_present() doc comment. > > Signed-off-by: Igor Mammedov Hmm, I wonder whether this still should return true if parent is not a pure container. Also what if MR isn't under parent at all? Maybe the right thing to do is return mr->ops == &unassigned_mem_ops ? > --- > include/exec/memory.h | 6 +++--- > memory.c | 2 +- > 2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/include/exec/memory.h b/include/exec/memory.h > index 296d6ab..a5eb4c8 100644 > --- a/include/exec/memory.h > +++ b/include/exec/memory.h > @@ -838,13 +838,13 @@ void memory_region_set_alias_offset(MemoryRegion *mr, > hwaddr offset); > > /** > - * memory_region_present: translate an address/size relative to a > - * MemoryRegion into a #MemoryRegionSection. > + * memory_region_present: checks if an address relative to a @parent > + * translates into #MemoryRegion within @parent > * > * Answer whether a #MemoryRegion within @parent covers the address > * @addr. > * > - * @parent: a MemoryRegion within which @addr is a relative address > + * @parent: a #MemoryRegion within which @addr is a relative address > * @addr: the area within @parent to be searched > */ > bool memory_region_present(MemoryRegion *parent, hwaddr addr); > diff --git a/memory.c b/memory.c > index 59ecc28..3f1df23 100644 > --- a/memory.c > +++ b/memory.c > @@ -1562,7 +1562,7 @@ static FlatRange *flatview_lookup(FlatView *view, AddrRange addr) > bool memory_region_present(MemoryRegion *parent, hwaddr addr) > { > MemoryRegion *mr = memory_region_find(parent, addr, 1).mr; > - if (!mr) { > + if (!mr || (mr == parent)) { > return false; > } > memory_region_unref(mr); > -- > 1.7.1