lkml.org 
[lkml]   [2009]   [May]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patches in this message
/
Date
SubjectRe: swiotlb: remove __weak hooks in favour of architecture-specific functions
From
On Fri, 22 May 2009 12:43:16 +0100
Ian Campbell <Ian.Campbell@eu.citrix.com> 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@citrix.com> 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/.

You need to insert Xen's hook like this:


+++ b/arch/x86/include/asm/dma-mapping.h
@@ -309,4 +309,20 @@ static inline void dma_free_coherent(struct device *dev, size_t size,
ops->free_coherent(dev, size, vaddr, bus);
}

+static inline dma_addr_t dma_map_range(struct device *dev, u64 mask,
+ phys_addr_t addr, size_t size)
+{
+ dma_addr_t dma_addr;
+#ifdef CONFIG_XEN
+ extern int xen_range_needs_mapping(phys_addr_t paddr, size_t size);
+ if (xen_pv_domain() && xen_range_needs_mapping(addr, size))
+ return 0;
+#endif
+
+ dma_addr = swiotlb_phys_to_bus(dev, addr);
+ if (dma_addr + size <= mask)
+ return 0;
+ return dma_addr;
+}

Or you need to use a function pointer in a strange way.
--- a/arch/x86/include/asm/dma-mapping.h
+++ b/arch/x86/include/asm/dma-mapping.h
@@ -315,4 +315,13 @@ static inline int is_buffer_dma_capable(struct device *dev, u64 mask,
return addr + size <= mask;
}

+extern int (*x86_swiotlb_force_mapping)(phys_addr_t paddr, size_t size);
+
+static inline int swiotlb_force_mapping(phys_addr_t paddr, size_t size)
+{
+ if (x86_swiotlb_force_mapping)
+ return x86_swiotlb_force_mapping(paddr, size);
+ return 0;
+}
+

Or you could invent something more.

As you said in another mail, it's up to the x86 maintainer :

> This case is internal to the x86 arch code and I'd really like to hear
> the x86 maintainer's opinion of the general approach.


But the above code looks really ugly to me.


> > With that approach, there is not
> > much code duplication and there is no need for ugly hooks for dom0;
> > the phys/bus address conversion and address checking.
>
> The phys/bus address conversion is also needed for powerpc.
>
> I think the two address checking functions can be collapsed into a
> single one which satisfies the needs of both Xen and powerpc.
>
> What dom0 specific "ugly" hooks does that leave? The alloc one? I've
> discussed that in another mail.

See above. POWERPC can use arch/*/include/asm/ cleanly for the
phys/bus address conversion while dom0 can't. That's what I said again
and again.

\
 
 \ /
  Last update: 2009-05-22 13:59    [W:0.083 / U:0.128 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site