Isaku Yamahata
2006-Apr-25 04:30 UTC
[Xen-devel] [PATCH] pci-dma-xen.c change necessary for xen/ia64
xenLinux/ia64 with dom0 vp model uses linux-2.6-xen-sparse/arch/i386/kernel/pci-dma-xen.c. But it has its own dma_map_page(), dma_declare_coheremnt_memory() and their families. So those in pci-dma-xen.c are unnecessary. #ifdef out them. -- yamahata _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Muli Ben-Yehuda
2006-Apr-25 08:44 UTC
Re: [Xen-devel] [PATCH] pci-dma-xen.c change necessary for xen/ia64
On Tue, Apr 25, 2006 at 01:30:52PM +0900, Isaku Yamahata wrote:> > xenLinux/ia64 with dom0 vp model uses > linux-2.6-xen-sparse/arch/i386/kernel/pci-dma-xen.c.Err, why?> But it has its own dma_map_page(), dma_declare_coheremnt_memory() and their > families. So those in pci-dma-xen.c are unnecessary. #ifdef out > them.Won''t it be cleaner for IA64 to have a DMA API implementation for each different mode of operation? if code can be consolidated between different implementations it should be put in a common file, under arch/ia64 or in lib/ or drivers/, depending on the usage. The #ifdef approach (not to mention using one arch''s files from another) #is going to be severely frowned upon upstream. Cheers, Muli _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Christian Limpach
2006-Apr-25 10:00 UTC
Re: [Xen-devel] [PATCH] pci-dma-xen.c change necessary for xen/ia64
On 4/25/06, Isaku Yamahata <yamahata@valinux.co.jp> wrote:> > xenLinux/ia64 with dom0 vp model uses > linux-2.6-xen-sparse/arch/i386/kernel/pci-dma-xen.c. > But it has its own dma_map_page(), dma_declare_coheremnt_memory() and their > families. So those in pci-dma-xen.c are unnecessary. #ifdef out them.Does this work? If you have your own dma_declare_coherent_memory(), don''t you then need to #define ARCH_HAS_DMA_DECLARE_COHERENT_MEMORY so that you don''t get the default implementations in linux/dma-mapping.h? I also don''t like the ifdef on dma_map_page since it requires you to #define it to something else to override it instead of implementing it. I prefer to have #if defined(__i386__) || defined(__x86_64__) (maybe with a comment stating !defined(__ia64__)) which will make it clear why these are ifdef''ed out. I think this approach is ok for now since we haven''t decided whether to merge this file back into i386'' pci-dma or move it to drivers/xen/core. christian _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Isaku Yamahata
2006-Apr-26 02:35 UTC
Re: [Xen-devel] [PATCH] pci-dma-xen.c change necessary for xen/ia64
On Tue, Apr 25, 2006 at 11:00:52AM +0100, Christian Limpach wrote:> I also don''t like the ifdef on dma_map_page since it requires you to > #define it to something else to override it instead of implementing > it. > > I prefer to have #if defined(__i386__) || defined(__x86_64__) (maybe > with a comment stating !defined(__ia64__)) which will make it clear > why these are ifdef''ed out.I attached the updated patch. -- yamahata _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Christian Limpach
2006-Apr-26 09:19 UTC
Re: [Xen-devel] [PATCH] pci-dma-xen.c change necessary for xen/ia64
On 4/26/06, Isaku Yamahata <yamahata@valinux.co.jp> wrote:> On Tue, Apr 25, 2006 at 11:00:52AM +0100, Christian Limpach wrote: > > > I also don''t like the ifdef on dma_map_page since it requires you to > > #define it to something else to override it instead of implementing > > it. > > > > I prefer to have #if defined(__i386__) || defined(__x86_64__) (maybe > > with a comment stating !defined(__ia64__)) which will make it clear > > why these are ifdef''ed out. > > I attached the updated patch.Cool, although I''m still wondering about dma_declare_coherent_memory(): If you have your own dma_declare_coherent_memory(), don''t you then need to #define ARCH_HAS_DMA_DECLARE_COHERENT_MEMORY so that you don''t get the default implementations in linux/dma-mapping.h? Do you use the default dma_declare_coherent_memory() from linux/dma-mapping.h and thus don''t define ARCH_HAS_DMA_DECLARE_COHERENT_MEMORY? christian _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Isaku Yamahata
2006-Apr-26 09:34 UTC
Re: [Xen-devel] [PATCH] pci-dma-xen.c change necessary for xen/ia64
On Wed, Apr 26, 2006 at 10:19:49AM +0100, Christian Limpach wrote:> On 4/26/06, Isaku Yamahata <yamahata@valinux.co.jp> wrote: > >On Tue, Apr 25, 2006 at 11:00:52AM +0100, Christian Limpach wrote: > > > >> I also don''t like the ifdef on dma_map_page since it requires you to > >> #define it to something else to override it instead of implementing > >> it. > >> > >> I prefer to have #if defined(__i386__) || defined(__x86_64__) (maybe > >> with a comment stating !defined(__ia64__)) which will make it clear > >> why these are ifdef''ed out. > > > >I attached the updated patch. > > Cool, although I''m still wondering about dma_declare_coherent_memory(): > If you have your own dma_declare_coherent_memory(), don''t you then > need to #define ARCH_HAS_DMA_DECLARE_COHERENT_MEMORY so that you don''t > get the default implementations in linux/dma-mapping.h? > > Do you use the default dma_declare_coherent_memory() from > linux/dma-mapping.h and thus don''t define > ARCH_HAS_DMA_DECLARE_COHERENT_MEMORY?xenLinux/IA64 uses the default dma_declare_coherent_memory(). So ARCH_HAS_DMA_DECLARE_COHERENT_MEMORY isn''t defined. -- yamahata _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Christian Limpach
2006-Apr-26 09:40 UTC
Re: [Xen-devel] [PATCH] pci-dma-xen.c change necessary for xen/ia64
On Wed, Apr 26, 2006 at 06:34:24PM +0900, Isaku Yamahata wrote:> On Wed, Apr 26, 2006 at 10:19:49AM +0100, Christian Limpach wrote: > > On 4/26/06, Isaku Yamahata <yamahata@valinux.co.jp> wrote: > > >On Tue, Apr 25, 2006 at 11:00:52AM +0100, Christian Limpach wrote: > > > > > >> I also don''t like the ifdef on dma_map_page since it requires you to > > >> #define it to something else to override it instead of implementing > > >> it. > > >> > > >> I prefer to have #if defined(__i386__) || defined(__x86_64__) (maybe > > >> with a comment stating !defined(__ia64__)) which will make it clear > > >> why these are ifdef''ed out. > > > > > >I attached the updated patch. > > > > Cool, although I''m still wondering about dma_declare_coherent_memory(): > > If you have your own dma_declare_coherent_memory(), don''t you then > > need to #define ARCH_HAS_DMA_DECLARE_COHERENT_MEMORY so that you don''t > > get the default implementations in linux/dma-mapping.h? > > > > Do you use the default dma_declare_coherent_memory() from > > linux/dma-mapping.h and thus don''t define > > ARCH_HAS_DMA_DECLARE_COHERENT_MEMORY? > > xenLinux/IA64 uses the default dma_declare_coherent_memory(). > So ARCH_HAS_DMA_DECLARE_COHERENT_MEMORY isn''t defined.Ah ok, then it makes sense. I''ll apply the patch. Thanks. christian _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Muli Ben-Yehuda
2006-Apr-26 12:08 UTC
Re: [Xen-devel] [PATCH] pci-dma-xen.c change necessary for xen/ia64
On Wed, Apr 26, 2006 at 10:40:55AM +0100, Christian Limpach wrote:> Ah ok, then it makes sense. I''ll apply the patch. Thanks.Why not do it right from the get-go instead of doing something that will need to be re-done later for upstream inclusion? Cheers, Muli _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Christian Limpach
2006-Apr-26 17:17 UTC
Re: [Xen-devel] [PATCH] pci-dma-xen.c change necessary for xen/ia64
On 4/26/06, Muli Ben-Yehuda <muli@il.ibm.com> wrote:> On Wed, Apr 26, 2006 at 10:40:55AM +0100, Christian Limpach wrote: > > > Ah ok, then it makes sense. I''ll apply the patch. Thanks. > > Why not do it right from the get-go instead of doing something that > will need to be re-done later for upstream inclusion?Because we haven''t decided yet whether it''s preferable having xen specific changes to pci-dma on each architecture or having a xen specific pci-dma which would be shared by all architectures. christian _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Muli Ben-Yehuda
2006-Apr-27 07:41 UTC
Re: [Xen-devel] [PATCH] pci-dma-xen.c change necessary for xen/ia64
On Wed, Apr 26, 2006 at 06:17:01PM +0100, Christian Limpach wrote:> On 4/26/06, Muli Ben-Yehuda <muli@il.ibm.com> wrote: > >On Wed, Apr 26, 2006 at 10:40:55AM +0100, Christian Limpach wrote: > > > >> Ah ok, then it makes sense. I''ll apply the patch. Thanks. > > > >Why not do it right from the get-go instead of doing something that > >will need to be re-done later for upstream inclusion? > > Because we haven''t decided yet whether it''s preferable having xen > specific changes to pci-dma on each architecture or having a xen > specific pci-dma which would be shared by all architectures.>From an upstream inclusion point of view, the former is definitely asmoother approach. Once the different archs have Xen specific DMA API implementations, we can look at consolidating the shared bits under drivers/xen or some such. Cheers, Muli _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel