From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1LztbW-0000RY-SP
	for qemu-devel@nongnu.org; Fri, 01 May 2009 10:20:42 -0400
Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43)
	id 1LztbS-0000L2-Ot
	for qemu-devel@nongnu.org; Fri, 01 May 2009 10:20:42 -0400
Received: from [199.232.76.173] (port=47431 helo=monty-python.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1LztbS-0000Kl-Jn
	for qemu-devel@nongnu.org; Fri, 01 May 2009 10:20:38 -0400
Received: from bsdimp.com ([199.45.160.85]:53882 helo=harmony.bsdimp.com)
	by monty-python.gnu.org with esmtps
	(TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60)
	(envelope-from <imp@bsdimp.com>) id 1LztbR-00009U-OC
	for qemu-devel@nongnu.org; Fri, 01 May 2009 10:20:38 -0400
Date: Fri, 01 May 2009 08:17:50 -0600 (MDT)
Message-Id: <20090501.081750.-2000945085.imp@bsdimp.com>
Subject: Re: [Qemu-devel] [PATCH] try to implement complete pci-to-pci
	bridge emulator.
From: "M. Warner Losh" <imp@bsdimp.com>
In-Reply-To: <1241170436-2800-5-git-send-email-yamahata@valinux.co.jp>
References: <1241170436-2800-1-git-send-email-yamahata@valinux.co.jp>
	<1241170436-2800-5-git-send-email-yamahata@valinux.co.jp>
Mime-Version: 1.0
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/pipermail/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: yamahata@valinux.co.jp
Cc: mst@redhat.com, qemu-devel@nongnu.org, armbru@redhat.com

In message: <1241170436-2800-5-git-send-email-yamahata@valinux.co.jp>
            Isaku Yamahata <yamahata@valinux.co.jp> writes:
: Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
: ---
:  hw/pci.c      |  175 +++++++++++++++++++++++++++++++++++++++------------------
:  hw/pci.h      |   30 ++++++++++
:  hw/unin_pci.c |    2 +-
:  3 files changed, 151 insertions(+), 56 deletions(-)
: 
: diff --git a/hw/pci.c b/hw/pci.c
: index 0be3662..a1123cb 100644
: --- a/hw/pci.c
: +++ b/hw/pci.c
...
: +        case 0x01:
: +        case 0x02:
: +        case 0x03:
: +        case 0x06:
: +        case 0x07:
: +        case 0x08:
: +        case 0x09:
: +        case 0x0a:
: +        case 0x0b:
: +        case 0x0e:
: +        case 0x10 ... 0x27: /* base */

This is wrong.  BARs are writable, and must be writable to determine
their size.

: +        case 0x2c ... 0x2f: /* read-only subsystem ID & vendor ID */
: +        case 0x30 ... 0x33: /* rom */

This is wrong too.  You have to be able to turn on and off this
mapping by writing bit 0.

: +        case 0x3d:
: +            can_write = 0;
:              break;

General comment: hard coded magic numbers like the above are very hard
to read.

Warner