From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NSFfb-0006aO-Mm for qemu-devel@nongnu.org; Tue, 05 Jan 2010 15:06:23 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NSFfX-0006YY-Ns for qemu-devel@nongnu.org; Tue, 05 Jan 2010 15:06:23 -0500 Received: from [199.232.76.173] (port=42724 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NSFfX-0006YU-IE for qemu-devel@nongnu.org; Tue, 05 Jan 2010 15:06:19 -0500 Received: from mx20.gnu.org ([199.232.41.8]:36892) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1NSFfX-00011b-3S for qemu-devel@nongnu.org; Tue, 05 Jan 2010 15:06:19 -0500 Received: from mail-pw0-f43.google.com ([209.85.160.43]) by mx20.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NRswh-0004EY-7X for qemu-devel@nongnu.org; Mon, 04 Jan 2010 14:50:31 -0500 Received: by pwj11 with SMTP id 11so10324241pwj.2 for ; Mon, 04 Jan 2010 11:49:30 -0800 (PST) MIME-Version: 1.0 In-Reply-To: <20100104191008.GB20721@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> From: Blue Swirl Date: Mon, 4 Jan 2010 19:49:10 +0000 Message-ID: Content-Type: text/plain; charset=UTF-8 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: "Michael S. Tsirkin" Cc: Aurelien Jarno , qemu-devel@nongnu.org On Mon, Jan 4, 2010 at 7:10 PM, Michael S. Tsirkin wrote: > 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 wrot= e: >> >> 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 benefi= ted >> >> > from being on the list, point it out. =C2=A0People are reasonable a= nd if you >> >> > have a good suggestion, they'll value your input and be likely to s= eek >> >> > it out in the future. >> > >> > Here is another patch that would have benefitted from review >> > before commit: >> > >> >> commit cf616802171905a9b6d087a69caa3b978b9cd741 >> >> Author: Blue Swirl >> >> Date: =C2=A0 Sun Dec 27 20:52:36 2009 +0000 >> >> >> >> =C2=A0 =C2=A0 PCI: Fix bus address conversion >> >> >> >> =C2=A0 =C2=A0 Pass physical addresses to map functions instead of PCI= bus addresses. >> >> >> >> =C2=A0 =C2=A0 Signed-off-by: Blue Swirl >> > >> > and previous related patches. =C2=A0The issues here that I see are: >> > >> > - IMO mem_base should really be pci_bus_t, as pci address might be >> > =C2=A064 bit mapped into 32 bit target >> > >> > - I think pci to pci bridges need mem_base copied from parent to child= , >> > =C2=A0this does not seem to be done? >> > >> > - map functions need to get pci_bus_t (for io), and now they get >> > =C2=A0a cpu address there. The real fix IMO is moving the mapping >> > =C2=A0to within pci.c. I think Avi had a patch to do this - >> > =C2=A0any objections to refreshing it? >> > >> > Blue Swirl, could you comment please? >> >> The issues you point out (which may well be valid) are not related to >> the change made by the patch and should be addressed by new patches. > > Yes, there's no harm in fixing them separately. =C2=A0The point I was mak= ing > 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. 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. >> 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 the or= iginal patch > btw? I couldn't find it from the archives, maybe I didn't remember correctly. I think the discussions were about generic DMA.