Re: swiotlb: remove __weak hooks in favour ofarchitecture-specific functions

[FUJITA Tomonori]

On Fri, 22 May 2009 15:02:04 +0100
Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx> wrote:

> On Fri, 2009-05-22 at 07:55 -0400, FUJITA Tomonori wrote:
> > On Fri, 22 May 2009 12:43:16 +0100
> > Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx> wrote:
> > 
> > > On Fri, 2009-05-22 at 07:13 -0400, FUJITA Tomonori wrote:
> > > > On Thu, 21 May 2009 17:15:21 +0100
> > > > Ian Campbell <ian.campbell@xxxxxxxxxx> wrote:
> > > > Please go with the following way (that I posted yesterday):
> > > > 
> > > > http://marc.info/?l=xen-devel&m=124292666214380&w=2
> > > > 
> > > > 
> > > > Export the core feature of swiotlb, managing iotlb buffer and
> > > > implement the Xen mapping functions.
> > > 
> > > I feel that should be a last resort, before we go down that path we
> > > should try and find a way for us to use the generic code in a clean way
> > > which makes everyone happy.
> > >
> > > We have had several attempts at this and admittedly have not yet come up
> > > with something that satisfies everyone but I don't really think we have
> > > gotten to the point of admitting defeat and just duplicating the code.
> > 
> > There should not be much duplication.
> > 
> > 
> > > I think the proposal to use a dma_map_range-like function which I sent a
> > > few minutes ago I think gets us closer to something which satisfies
> > > everyone's requirements, including yours for a clean abstraction.
> > 
> > Seems you don't understand the point; with dom0, we can't cleanly use
> > arch/*/include/asm/.
> 
> I understand precisely what you are saying, I just fundamentally
> disagree with you. It is perfectly possible for an arch/*/include/asm
> interface for this stuff to be defined which is completely abstracted
> from the POV of the swiotlb code (and any other arch-external code).
> 
> > You need to insert Xen's hook like this:
> 
> As I said in the email this snippet was contained in:
> >       * The Xen specific stuff in arch/x86/include/asm/dma-mapping.h
> >         clearly needs to be properly abstracted away (I just stuck it
> >         there for testing).
> 
> So obviously I am well aware that it is unacceptable as it stands.

I don't think that you can't clean up the above much.

After all, you need a hook for Xen in
arch/x86/include/asm/dma-mapping.h and define it in Xen's land.

That will not be much different from:

http://marc.info/?l=linux-kernel&m=124299344606890&w=2


And adding 'if xen' hook in common header files in arch/x86/include/ is a
wrong design to me. Do you add a similar 'if xen' hook in other x86's
common headers files?


> I think we can find a way to implement this functionality which is
k> contained entirely within the arch/x86 code and is also acceptable to
> the x86 maintainers. There are plenty of cases where we define similar

I'm not sure about it (hopefully, not). But if the x86 maintainers ACK
the above code, I have no complaint.


> interfaces where arch code implements an API with different backends for
> different configurations, hardware configurations etc etc.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/