From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NSFYI-00048Z-7v for qemu-devel@nongnu.org; Tue, 05 Jan 2010 14:58:50 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NSFYD-00045j-6K for qemu-devel@nongnu.org; Tue, 05 Jan 2010 14:58:49 -0500 Received: from [199.232.76.173] (port=43293 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NSFYC-00045V-FU for qemu-devel@nongnu.org; Tue, 05 Jan 2010 14:58:44 -0500 Received: from mx20.gnu.org ([199.232.41.8]:36649) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1NSFYC-0006i5-1J for qemu-devel@nongnu.org; Tue, 05 Jan 2010 14:58:44 -0500 Received: from mx1.redhat.com ([209.132.183.28]) by mx20.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NRt6H-00056v-S7 for qemu-devel@nongnu.org; Mon, 04 Jan 2010 15:00:26 -0500 Date: Mon, 4 Jan 2010 21:57:09 +0200 From: "Michael S. Tsirkin" Message-ID: <20100104195709.GA21311@redhat.com> References: <20091227113732.GA3833@redhat.com> <20091227203453.GA30873@volta.aurel32.net> <20091227215023.GB3278@redhat.com> <20091227222326.GA10602@hall.aurel32.net> <20091227224021.GA3443@redhat.com> <4B37E752.1020000@codemonkey.ws> <20100104183356.GA18873@redhat.com> <20100104191008.GB20721@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: Content-Transfer-Encoding: quoted-printable Subject: [Qemu-devel] Re: PCI: Fix bus address conversion (was Re: commit rules for common git tree) List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: Aurelien Jarno , qemu-devel@nongnu.org On Mon, Jan 04, 2010 at 07:49:10PM +0000, Blue Swirl wrote: > On Mon, Jan 4, 2010 at 7:10 PM, Michael S. Tsirkin wro= te: > > On Mon, Jan 04, 2010 at 07:04:38PM +0000, Blue Swirl wrote: > >> On Mon, Jan 4, 2010 at 6:33 PM, Michael S. Tsirkin = wrote: > >> >> On Sun, Dec 27, 2009 at 05:01:38PM -0600, Anthony Liguori wrote: > >> >> > Likewise, if you see a patch go in that you think would have be= nefited > >> >> > from being on the list, point it out. =A0People are reasonable = and if you > >> >> > have a good suggestion, they'll value your input and be likely = to seek > >> >> > it out in the future. > >> > > >> > Here is another patch that would have benefitted from review > >> > before commit: > >> > > >> >> commit cf616802171905a9b6d087a69caa3b978b9cd741 > >> >> Author: Blue Swirl > >> >> Date: =A0 Sun Dec 27 20:52:36 2009 +0000 > >> >> > >> >> =A0 =A0 PCI: Fix bus address conversion > >> >> > >> >> =A0 =A0 Pass physical addresses to map functions instead of PCI b= us addresses. > >> >> > >> >> =A0 =A0 Signed-off-by: Blue Swirl > >> > > >> > and previous related patches. =A0The issues here that I see are: > >> > > >> > - IMO mem_base should really be pci_bus_t, as pci address might be > >> > =A064 bit mapped into 32 bit target > >> > > >> > - I think pci to pci bridges need mem_base copied from parent to c= hild, > >> > =A0this does not seem to be done? > >> > > >> > - map functions need to get pci_bus_t (for io), and now they get > >> > =A0a cpu address there. The real fix IMO is moving the mapping > >> > =A0to within pci.c. I think Avi had a patch to do this - > >> > =A0any objections to refreshing it? > >> > > >> > Blue Swirl, could you comment please? > >> > >> The issues you point out (which may well be valid) are not related t= o > >> the change made by the patch and should be addressed by new patches. > > > > Yes, there's no harm in fixing them separately. =A0The point I was ma= king > > is it is better to post patches on list so issues can be pointed out = and > > discussed without the need to dig through git history. >=20 > This may happen in any case, for example if you are busy and have no > time to review the patch in time before it's committed. It has > happened to me many times. Also patches that seem harmless can and > will break stuff. Yes, it may. But hey, give people a chance. > >> IIRC Avi promised to refresh his patch but never delivered. I think > >> Paul also wanted that the bus translation would be handled in a more > >> generic way (eliminate map functions). > > > > I'd like to eliminate map functions as well. Do you have a link to th= e original patch > > btw? >=20 > I couldn't find it from the archives, maybe I didn't remember > correctly. I think the discussions were about generic DMA. DMA? Sounds strange ... these are PCI memory/io/ROM mappings. --=20 MST