qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-devel] Re: PCI: Fix bus address conversion (was Re: commit rules f


From: Blue Swirl
Subject: [Qemu-devel] Re: PCI: Fix bus address conversion (was Re: commit rules for common git tree)
Date: Mon, 4 Jan 2010 19:49:10 +0000

On Mon, Jan 4, 2010 at 7:10 PM, Michael S. Tsirkin <address@hidden> 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 <address@hidden> 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 <address@hidden>
>> >> 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 <address@hidden>
>> >
>> > 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.

>> 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.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]