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 <m...@redhat.com> 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 <m...@redhat.com> 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 benefited > >> >> > from being on the list, point it out. People 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 <blauwir...@gmail.com> > >> >> Date: Sun Dec 27 20:52:36 2009 +0000 > >> >> > >> >> PCI: Fix bus address conversion > >> >> > >> >> Pass physical addresses to map functions instead of PCI bus > >> >> addresses. > >> >> > >> >> Signed-off-by: Blue Swirl <blauwir...@gmail.com> > >> > > >> > and previous related patches. The issues here that I see are: > >> > > >> > - IMO mem_base should really be pci_bus_t, as pci address might be > >> > 64 bit mapped into 32 bit target > >> > > >> > - I think pci to pci bridges need mem_base copied from parent to child, > >> > this does not seem to be done? > >> > > >> > - map functions need to get pci_bus_t (for io), and now they get > >> > a cpu address there. The real fix IMO is moving the mapping > >> > to within pci.c. I think Avi had a patch to do this - > >> > any 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. The point I was making > > 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.
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 the > > original patch > > btw? > > 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. -- MST
