From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([140.186.70.92]:60872)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <michael@walle.cc>) id 1Qz4Fv-00076n-8R
	for qemu-devel@nongnu.org; Thu, 01 Sep 2011 06:12:20 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <michael@walle.cc>) id 1Qz44E-0007AR-J9
	for qemu-devel@nongnu.org; Thu, 01 Sep 2011 06:00:18 -0400
Received: from mail.serverraum.org ([78.47.150.89]:44923)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <michael@walle.cc>) id 1Qz44E-00079I-D9
	for qemu-devel@nongnu.org; Thu, 01 Sep 2011 06:00:14 -0400
From: Michael Walle <michael@walle.cc>
Date: Thu, 1 Sep 2011 12:00:11 +0200
References: <201109010054.53424.michael@walle.cc> <4E5F1E14.3060606@redhat.com>
In-Reply-To: <4E5F1E14.3060606@redhat.com>
MIME-Version: 1.0
Content-Type: Text/Plain;
  charset="iso-8859-1"
Content-Transfer-Encoding: 7bit
Message-Id: <201109011200.12035.michael@walle.cc>
Subject: Re: [Qemu-devel] new memory api: offsets?
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Avi Kivity <avi@redhat.com>
Cc: qemu-devel@nongnu.org

Am Donnerstag 01 September 2011, 07:54:28 schrieb Avi Kivity:
> On 09/01/2011 01:54 AM, Michael Walle wrote:
> > Hi Avi,
> > 
> > while debugging, i noticed, that mr->offset is never set, expect in the
> > initializer. (The subregion collision warning is printed although the
> > regions are not overlapping.) Is this intended?
> 
> Did you miss memory_region_set_offset()?
I saw that function but it's never called. Now i noticed that this is a 
deprecated public function.

> The subregion collision warning is unrelated?
you are walking along the subregions and use that offset property to detect 
collisions. Shouldn't you use the addr property instead?

@@ -1190,16 +1190,18 @@ static void 
memory_region_add_subregion_common(MemoryRegion *mr,
         if (subregion->may_overlap || other->may_overlap) {
             continue;
         }
-        if (offset >= other->offset + other->size
-            || offset + subregion->size <= other->offset) {
+        if (offset >= other->addr + other->size
+            || offset + subregion->size <= other->addr) {
             continue;
         }

> > btw. you may include the memory region name in the warning.
> 
> sure, patches welcome.

-        printf("warning: subregion collision %llx/%llx vs %llx/%llx\n",
+        printf("warning: subregion collision %llx/%llx (%s) "
+               "vs %llx/%llx (%s)\n",
                (unsigned long long)offset,
                (unsigned long long)subregion->size,
-               (unsigned long long)other->offset,
-               (unsigned long long)other->size);
+               subregion->name,
+               (unsigned long long)other->addr,
+               (unsigned long long)other->size,
+               other->name);

let me know if i should post a git patch instead :)

-- 
Michael