* Re: [PATCH] add iommu_num_pages helper function [not found] ` <20080729104722H.fujita.tomonori@lab.ntt.co.jp> @ 2008-07-29 3:56 ` Stephen Rothwell 2008-07-29 4:38 ` FUJITA Tomonori 2008-07-29 5:24 ` David Miller 0 siblings, 2 replies; 23+ messages in thread From: Stephen Rothwell @ 2008-07-29 3:56 UTC (permalink / raw) To: FUJITA Tomonori Cc: mingo, joerg.roedel, mingo, tglx, linux-kernel, iommu, bhavna.sarathy, robert.richter, Linus, Andrew Morton, Jesse Barnes, linux-next [-- Attachment #1: Type: text/plain, Size: 1296 bytes --] Hi, On Tue, 29 Jul 2008 10:46:31 +0900 FUJITA Tomonori <fujita.tomonori@lab•ntt.co.jp> wrote: > > > So, I assume a patch created on July 25 is not destined for 2.6.27, > > Hmm, seems that they will go into 2.6.27: > > http://marc.info/?l=linux-kernel&m=121729275722287&w=2 > > > > right? Especially since it breaks another architecture build and has > > only made it to linux-next on July 29 ... and needs more work given the > > comments already on this patch. > > It doesn't break another architecture build since only x86 IOMMUs use > the helper function. But as I pointed out, it doesn't work for some > architectures. Um, did you see the linux-next build report I posted (possibly not): arch/powerpc/kernel/iommu.c:56: error: static declaration of 'iommu_num_pages' follows non-static declaration include/linux/iommu-helper.h:11: error: previous declaration of 'iommu_num_pages' was here This is now a build of 2.6.27-rc1 for powerpc64 ppc64_defconfig ... So we have a patch first posted to lkml on July 25 first hits linux-next on July 29 and is no in Linus' tree on July 29. A grep would have found the problem. More care required ... -- Cheers, Stephen Rothwell sfr@canb•auug.org.au http://www.canb.auug.org.au/~sfr/ [-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --] ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] add iommu_num_pages helper function 2008-07-29 3:56 ` [PATCH] add iommu_num_pages helper function Stephen Rothwell @ 2008-07-29 4:38 ` FUJITA Tomonori 2008-07-29 5:26 ` David Miller 2008-07-29 5:24 ` David Miller 1 sibling, 1 reply; 23+ messages in thread From: FUJITA Tomonori @ 2008-07-29 4:38 UTC (permalink / raw) To: sfr Cc: fujita.tomonori, mingo, joerg.roedel, mingo, tglx, linux-kernel, iommu, bhavna.sarathy, robert.richter, torvalds, akpm, jbarnes, linux-next On Tue, 29 Jul 2008 13:56:52 +1000 Stephen Rothwell <sfr@canb•auug.org.au> wrote: > Hi, > > On Tue, 29 Jul 2008 10:46:31 +0900 FUJITA Tomonori <fujita.tomonori@lab•ntt.co.jp> wrote: > > > > > So, I assume a patch created on July 25 is not destined for 2.6.27, > > > > Hmm, seems that they will go into 2.6.27: > > > > http://marc.info/?l=linux-kernel&m=121729275722287&w=2 > > > > > > > right? Especially since it breaks another architecture build and has > > > only made it to linux-next on July 29 ... and needs more work given the > > > comments already on this patch. > > > > It doesn't break another architecture build since only x86 IOMMUs use > > the helper function. But as I pointed out, it doesn't work for some > > architectures. > > Um, did you see the linux-next build report I posted (possibly not): > > arch/powerpc/kernel/iommu.c:56: error: static declaration of 'iommu_num_pages' follows non-static declaration > include/linux/iommu-helper.h:11: error: previous declaration of 'iommu_num_pages' was here > > This is now a build of 2.6.27-rc1 for powerpc64 ppc64_defconfig ... Ah, really sorry. Yeah, it breaks POWER and SPARC builds. > So we have a patch first posted to lkml on July 25 first hits linux-next > on July 29 and is no in Linus' tree on July 29. A grep would have found > the problem. > > More care required ... Agreed... As I said before, this is not urgent at all so we didn't need to rush. A fix has already been posted? == From: FUJITA Tomonori <fujita.tomonori@lab•ntt.co.jp> Subject: [PATCH] move iommu_num_pages helper to x86 This IOMMU helper function doesn't work for some architectures: http://marc.info/?l=linux-kernel&m=121699304403202&w=2 It also breaks POWER and SPARC builds: http://marc.info/?l=linux-kernel&m=121730388001890&w=2 Currently, only x86 IOMMUs use this so let's move it to x86 for now. Reported-by: Stephen Rothwell <sfr@canb•auug.org.au> Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab•ntt.co.jp> --- arch/x86/kernel/pci-dma.c | 8 ++++++++ include/asm-x86/iommu.h | 2 ++ include/linux/iommu-helper.h | 1 - lib/iommu-helper.c | 8 -------- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c index 8dbffb8..87d4d69 100644 --- a/arch/x86/kernel/pci-dma.c +++ b/arch/x86/kernel/pci-dma.c @@ -123,6 +123,14 @@ void __init pci_iommu_alloc(void) pci_swiotlb_init(); } + +unsigned long iommu_num_pages(unsigned long addr, unsigned long len) +{ + unsigned long size = roundup((addr & ~PAGE_MASK) + len, PAGE_SIZE); + + return size >> PAGE_SHIFT; +} +EXPORT_SYMBOL(iommu_num_pages); #endif /* diff --git a/include/asm-x86/iommu.h b/include/asm-x86/iommu.h index ecc8061..5f888cc 100644 --- a/include/asm-x86/iommu.h +++ b/include/asm-x86/iommu.h @@ -7,6 +7,8 @@ extern struct dma_mapping_ops nommu_dma_ops; extern int force_iommu, no_iommu; extern int iommu_detected; +extern unsigned long iommu_num_pages(unsigned long addr, unsigned long len); + #ifdef CONFIG_GART_IOMMU extern int gart_iommu_aperture; extern int gart_iommu_aperture_allowed; diff --git a/include/linux/iommu-helper.h b/include/linux/iommu-helper.h index f8598f5..c975caf 100644 --- a/include/linux/iommu-helper.h +++ b/include/linux/iommu-helper.h @@ -8,4 +8,3 @@ extern unsigned long iommu_area_alloc(unsigned long *map, unsigned long size, unsigned long align_mask); extern void iommu_area_free(unsigned long *map, unsigned long start, unsigned int nr); -extern unsigned long iommu_num_pages(unsigned long addr, unsigned long len); diff --git a/lib/iommu-helper.c b/lib/iommu-helper.c index 889ddce..a3b8d4c 100644 --- a/lib/iommu-helper.c +++ b/lib/iommu-helper.c @@ -80,11 +80,3 @@ void iommu_area_free(unsigned long *map, unsigned long start, unsigned int nr) } } EXPORT_SYMBOL(iommu_area_free); - -unsigned long iommu_num_pages(unsigned long addr, unsigned long len) -{ - unsigned long size = roundup((addr & ~PAGE_MASK) + len, PAGE_SIZE); - - return size >> PAGE_SHIFT; -} -EXPORT_SYMBOL(iommu_num_pages); -- 1.5.5.GIT ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH] add iommu_num_pages helper function 2008-07-29 4:38 ` FUJITA Tomonori @ 2008-07-29 5:26 ` David Miller 2008-07-29 5:42 ` FUJITA Tomonori 0 siblings, 1 reply; 23+ messages in thread From: David Miller @ 2008-07-29 5:26 UTC (permalink / raw) To: fujita.tomonori Cc: sfr, mingo, joerg.roedel, mingo, tglx, linux-kernel, iommu, bhavna.sarathy, robert.richter, torvalds, akpm, jbarnes, linux-next From: FUJITA Tomonori <fujita.tomonori@lab•ntt.co.jp> Date: Tue, 29 Jul 2008 13:38:53 +0900 > Agreed... As I said before, this is not urgent at all so we didn't > need to rush. Please, if you are going to add such interface, add "const unsigned long io_page_size" argument so that powerpc and sparc can use it too. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] add iommu_num_pages helper function 2008-07-29 5:26 ` David Miller @ 2008-07-29 5:42 ` FUJITA Tomonori 0 siblings, 0 replies; 23+ messages in thread From: FUJITA Tomonori @ 2008-07-29 5:42 UTC (permalink / raw) To: davem Cc: fujita.tomonori, sfr, mingo, joerg.roedel, mingo, tglx, linux-kernel, iommu, bhavna.sarathy, robert.richter, torvalds, akpm, jbarnes, linux-next On Mon, 28 Jul 2008 22:26:18 -0700 (PDT) David Miller <davem@davemloft•net> wrote: > From: FUJITA Tomonori <fujita.tomonori@lab•ntt.co.jp> > Date: Tue, 29 Jul 2008 13:38:53 +0900 > > > Agreed... As I said before, this is not urgent at all so we didn't > > need to rush. > > Please, if you are going to add such interface, add > "const unsigned long io_page_size" argument so that > powerpc and sparc can use it too. I didn't add this function: http://marc.info/?l=linux-kernel&m=121699080831750&w=2 I pointed out that it doesn't work for some architectures: http://marc.info/?l=linux-kernel&m=121699304403202&w=2 But sorry about not noticing that it breaks SPARC and POWER. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] add iommu_num_pages helper function 2008-07-29 3:56 ` [PATCH] add iommu_num_pages helper function Stephen Rothwell 2008-07-29 4:38 ` FUJITA Tomonori @ 2008-07-29 5:24 ` David Miller 2008-07-29 5:42 ` FUJITA Tomonori 2008-07-29 6:45 ` Stephen Rothwell 1 sibling, 2 replies; 23+ messages in thread From: David Miller @ 2008-07-29 5:24 UTC (permalink / raw) To: sfr Cc: fujita.tomonori, mingo, joerg.roedel, mingo, tglx, linux-kernel, iommu, bhavna.sarathy, robert.richter, torvalds, akpm, jbarnes, linux-next From: Stephen Rothwell <sfr@canb•auug.org.au> Date: Tue, 29 Jul 2008 13:56:52 +1000 > Um, did you see the linux-next build report I posted (possibly not): > > arch/powerpc/kernel/iommu.c:56: error: static declaration of 'iommu_num_pages' follows non-static declaration > include/linux/iommu-helper.h:11: error: previous declaration of 'iommu_num_pages' was here > > This is now a build of 2.6.27-rc1 for powerpc64 ppc64_defconfig ... It breaks sparc64 the same exact way. :-/ sparc64: Fix build after addition of iommu_num_pages() helper. It would have been much more pleasant if that new interface: 1) Took an "io_page_size" argument so powerpc and sparc64 could make use of it. 2) Was build tested on something other than x86. Signed-off-by: David S. Miller <davem@davemloft•net> diff --git a/arch/sparc64/kernel/iommu.c b/arch/sparc64/kernel/iommu.c index 2a37a6c..600f712 100644 --- a/arch/sparc64/kernel/iommu.c +++ b/arch/sparc64/kernel/iommu.c @@ -575,7 +575,7 @@ static int dma_4u_map_sg(struct device *dev, struct scatterlist *sglist, } /* Allocate iommu entries for that segment */ paddr = (unsigned long) SG_ENT_PHYS_ADDRESS(s); - npages = iommu_num_pages(paddr, slen); + npages = sparc64_iommu_num_pages(paddr, slen); entry = iommu_range_alloc(dev, iommu, npages, &handle); /* Handle failure */ @@ -647,7 +647,7 @@ iommu_map_failed: iopte_t *base; vaddr = s->dma_address & IO_PAGE_MASK; - npages = iommu_num_pages(s->dma_address, s->dma_length); + npages = sparc64_iommu_num_pages(s->dma_address, s->dma_length); iommu_range_free(iommu, vaddr, npages); entry = (vaddr - iommu->page_table_map_base) @@ -715,7 +715,7 @@ static void dma_4u_unmap_sg(struct device *dev, struct scatterlist *sglist, if (!len) break; - npages = iommu_num_pages(dma_handle, len); + npages = sparc64_iommu_num_pages(dma_handle, len); iommu_range_free(iommu, dma_handle, npages); entry = ((dma_handle - iommu->page_table_map_base) diff --git a/arch/sparc64/kernel/iommu_common.h b/arch/sparc64/kernel/iommu_common.h index 53b19c8..e64152b 100644 --- a/arch/sparc64/kernel/iommu_common.h +++ b/arch/sparc64/kernel/iommu_common.h @@ -35,8 +35,8 @@ #define SG_ENT_PHYS_ADDRESS(SG) (__pa(sg_virt((SG)))) -static inline unsigned long iommu_num_pages(unsigned long vaddr, - unsigned long slen) +static inline unsigned long sparc64_iommu_num_pages(unsigned long vaddr, + unsigned long slen) { unsigned long npages; @@ -53,7 +53,7 @@ static inline int is_span_boundary(unsigned long entry, struct scatterlist *sg) { unsigned long paddr = SG_ENT_PHYS_ADDRESS(outs); - int nr = iommu_num_pages(paddr, outs->dma_length + sg->length); + int nr = sparc64_iommu_num_pages(paddr, outs->dma_length + sg->length); return iommu_is_span_boundary(entry, nr, shift, boundary_size); } diff --git a/arch/sparc64/kernel/pci_sun4v.c b/arch/sparc64/kernel/pci_sun4v.c index a104c80..08dcdbd 100644 --- a/arch/sparc64/kernel/pci_sun4v.c +++ b/arch/sparc64/kernel/pci_sun4v.c @@ -382,7 +382,7 @@ static int dma_4v_map_sg(struct device *dev, struct scatterlist *sglist, } /* Allocate iommu entries for that segment */ paddr = (unsigned long) SG_ENT_PHYS_ADDRESS(s); - npages = iommu_num_pages(paddr, slen); + npages = sparc64_iommu_num_pages(paddr, slen); entry = iommu_range_alloc(dev, iommu, npages, &handle); /* Handle failure */ @@ -459,7 +459,7 @@ iommu_map_failed: unsigned long vaddr, npages; vaddr = s->dma_address & IO_PAGE_MASK; - npages = iommu_num_pages(s->dma_address, s->dma_length); + npages = sparc64_iommu_num_pages(s->dma_address, s->dma_length); iommu_range_free(iommu, vaddr, npages); /* XXX demap? XXX */ s->dma_address = DMA_ERROR_CODE; @@ -498,7 +498,7 @@ static void dma_4v_unmap_sg(struct device *dev, struct scatterlist *sglist, if (!len) break; - npages = iommu_num_pages(dma_handle, len); + npages = sparc64_iommu_num_pages(dma_handle, len); iommu_range_free(iommu, dma_handle, npages); entry = ((dma_handle - iommu->page_table_map_base) >> IO_PAGE_SHIFT); ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH] add iommu_num_pages helper function 2008-07-29 5:24 ` David Miller @ 2008-07-29 5:42 ` FUJITA Tomonori 2008-07-29 6:45 ` Stephen Rothwell 1 sibling, 0 replies; 23+ messages in thread From: FUJITA Tomonori @ 2008-07-29 5:42 UTC (permalink / raw) To: davem Cc: sfr, fujita.tomonori, mingo, joerg.roedel, mingo, tglx, linux-kernel, iommu, bhavna.sarathy, robert.richter, torvalds, akpm, jbarnes, linux-next On Mon, 28 Jul 2008 22:24:39 -0700 (PDT) David Miller <davem@davemloft•net> wrote: > From: Stephen Rothwell <sfr@canb•auug.org.au> > Date: Tue, 29 Jul 2008 13:56:52 +1000 > > > Um, did you see the linux-next build report I posted (possibly not): > > > > arch/powerpc/kernel/iommu.c:56: error: static declaration of 'iommu_num_pages' follows non-static declaration > > include/linux/iommu-helper.h:11: error: previous declaration of 'iommu_num_pages' was here > > > > This is now a build of 2.6.27-rc1 for powerpc64 ppc64_defconfig ... > > It breaks sparc64 the same exact way. :-/ Yeah, it breaks both. It's not a proper helper function that all the architectures can use so how about making it x86 stuff? http://marc.info/?l=linux-kernel&m=121730644704448&w=2 ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] add iommu_num_pages helper function 2008-07-29 5:24 ` David Miller 2008-07-29 5:42 ` FUJITA Tomonori @ 2008-07-29 6:45 ` Stephen Rothwell 2008-07-29 9:50 ` Joerg Roedel 1 sibling, 1 reply; 23+ messages in thread From: Stephen Rothwell @ 2008-07-29 6:45 UTC (permalink / raw) To: David Miller Cc: fujita.tomonori, mingo, joerg.roedel, mingo, tglx, linux-kernel, iommu, bhavna.sarathy, robert.richter, torvalds, akpm, jbarnes, linux-next [-- Attachment #1: Type: text/plain, Size: 382 bytes --] Hi Dave, On Mon, 28 Jul 2008 22:24:39 -0700 (PDT) David Miller <davem@davemloft•net> wrote: > > sparc64: Fix build after addition of iommu_num_pages() helper. I added this to the end of linux-next for today. Hopefully we will have a fix in Linus' tree tomorrow. -- Cheers, Stephen Rothwell sfr@canb•auug.org.au http://www.canb.auug.org.au/~sfr/ [-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --] ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] add iommu_num_pages helper function 2008-07-29 6:45 ` Stephen Rothwell @ 2008-07-29 9:50 ` Joerg Roedel 2008-07-29 9:58 ` FUJITA Tomonori 2008-07-29 10:10 ` David Miller 0 siblings, 2 replies; 23+ messages in thread From: Joerg Roedel @ 2008-07-29 9:50 UTC (permalink / raw) To: Stephen Rothwell Cc: David Miller, fujita.tomonori, mingo, mingo, tglx, linux-kernel, iommu, bhavna.sarathy, robert.richter, torvalds, akpm, jbarnes, linux-next On Tue, Jul 29, 2008 at 04:45:23PM +1000, Stephen Rothwell wrote: > Hi Dave, > > On Mon, 28 Jul 2008 22:24:39 -0700 (PDT) David Miller <davem@davemloft•net> wrote: > > > > sparc64: Fix build after addition of iommu_num_pages() helper. > > I added this to the end of linux-next for today. Hopefully we will have > a fix in Linus' tree tomorrow. Yes, my fault, sorry for that breakage :-/ I think moving that function to x86-only code is the best solution for now. Before moving it back to generic code we need an architecture independent way to find out the IOMMU page size. FUJITA, do you want to work on that or should I try to do this and send it to you for review? Joerg -- | AMD Saxony Limited Liability Company & Co. KG Operating | Wilschdorfer Landstr. 101, 01109 Dresden, Germany System | Register Court Dresden: HRA 4896 Research | General Partner authorized to represent: Center | AMD Saxony LLC (Wilmington, Delaware, US) | General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] add iommu_num_pages helper function 2008-07-29 9:50 ` Joerg Roedel @ 2008-07-29 9:58 ` FUJITA Tomonori 2008-07-29 10:02 ` Joerg Roedel 2008-07-29 10:10 ` David Miller 1 sibling, 1 reply; 23+ messages in thread From: FUJITA Tomonori @ 2008-07-29 9:58 UTC (permalink / raw) To: joerg.roedel Cc: sfr, davem, fujita.tomonori, mingo, mingo, tglx, linux-kernel, iommu, bhavna.sarathy, robert.richter, torvalds, akpm, jbarnes, linux-next On Tue, 29 Jul 2008 11:50:27 +0200 Joerg Roedel <joerg.roedel@amd•com> wrote: > On Tue, Jul 29, 2008 at 04:45:23PM +1000, Stephen Rothwell wrote: > > Hi Dave, > > > > On Mon, 28 Jul 2008 22:24:39 -0700 (PDT) David Miller <davem@davemloft•net> wrote: > > > > > > sparc64: Fix build after addition of iommu_num_pages() helper. > > > > I added this to the end of linux-next for today. Hopefully we will have > > a fix in Linus' tree tomorrow. > > Yes, my fault, sorry for that breakage :-/ > I think moving that function to x86-only code is the best solution for > now. Before moving it back to generic code we need an architecture > independent way to find out the IOMMU page size. Agreed, it fixes both SPARC and POWER breakages. > FUJITA, > > do you want to work on that or should I try to do this and send it to > you for review? I've already sent a patch that fixes the breakages in such way: http://marc.info/?l=linux-kernel&m=121730644704448&w=2 ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] add iommu_num_pages helper function 2008-07-29 9:58 ` FUJITA Tomonori @ 2008-07-29 10:02 ` Joerg Roedel 2008-07-30 22:44 ` Benjamin Herrenschmidt 0 siblings, 1 reply; 23+ messages in thread From: Joerg Roedel @ 2008-07-29 10:02 UTC (permalink / raw) To: FUJITA Tomonori Cc: sfr, davem, mingo, mingo, tglx, linux-kernel, iommu, bhavna.sarathy, robert.richter, torvalds, akpm, jbarnes, linux-next On Tue, Jul 29, 2008 at 06:58:08PM +0900, FUJITA Tomonori wrote: > On Tue, 29 Jul 2008 11:50:27 +0200 > Joerg Roedel <joerg.roedel@amd•com> wrote: > > do you want to work on that or should I try to do this and send it to > > you for review? > > I've already sent a patch that fixes the breakages in such way: > > http://marc.info/?l=linux-kernel&m=121730644704448&w=2 Yes I have seen this patch. I meant the patches that introduce something like IOMMU_PAGE_SIZE for all architectures so that we can make this function generic and move it back to lib/. Joerg -- | AMD Saxony Limited Liability Company & Co. KG Operating | Wilschdorfer Landstr. 101, 01109 Dresden, Germany System | Register Court Dresden: HRA 4896 Research | General Partner authorized to represent: Center | AMD Saxony LLC (Wilmington, Delaware, US) | General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] add iommu_num_pages helper function 2008-07-29 10:02 ` Joerg Roedel @ 2008-07-30 22:44 ` Benjamin Herrenschmidt 2008-07-31 2:07 ` FUJITA Tomonori 0 siblings, 1 reply; 23+ messages in thread From: Benjamin Herrenschmidt @ 2008-07-30 22:44 UTC (permalink / raw) To: Joerg Roedel Cc: FUJITA Tomonori, sfr, davem, mingo, mingo, tglx, linux-kernel, iommu, bhavna.sarathy, robert.richter, torvalds, akpm, jbarnes, linux-next On Tue, 2008-07-29 at 12:02 +0200, Joerg Roedel wrote: > On Tue, Jul 29, 2008 at 06:58:08PM +0900, FUJITA Tomonori wrote: > > On Tue, 29 Jul 2008 11:50:27 +0200 > > Joerg Roedel <joerg.roedel@amd•com> wrote: > > > do you want to work on that or should I try to do this and send it to > > > you for review? > > > > I've already sent a patch that fixes the breakages in such way: > > > > http://marc.info/?l=linux-kernel&m=121730644704448&w=2 > > Yes I have seen this patch. I meant the patches that introduce something > like IOMMU_PAGE_SIZE for all architectures so that we can make this > function generic and move it back to lib/. That will not work in the light of some new stuff coming on powerpc in the future. The iommu page size may/will be dynamic per instance of iommu. IE, it will be mostly decided at runtime, I don't expect machines to have multiple iommus of different page sizes off hand but it -can- happen. Ben. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] add iommu_num_pages helper function 2008-07-30 22:44 ` Benjamin Herrenschmidt @ 2008-07-31 2:07 ` FUJITA Tomonori 0 siblings, 0 replies; 23+ messages in thread From: FUJITA Tomonori @ 2008-07-31 2:07 UTC (permalink / raw) To: benh Cc: joerg.roedel, fujita.tomonori, sfr, davem, mingo, mingo, tglx, linux-kernel, iommu, bhavna.sarathy, robert.richter, torvalds, akpm, jbarnes, linux-next On Thu, 31 Jul 2008 08:44:14 +1000 Benjamin Herrenschmidt <benh@kernel•crashing.org> wrote: > On Tue, 2008-07-29 at 12:02 +0200, Joerg Roedel wrote: > > On Tue, Jul 29, 2008 at 06:58:08PM +0900, FUJITA Tomonori wrote: > > > On Tue, 29 Jul 2008 11:50:27 +0200 > > > Joerg Roedel <joerg.roedel@amd•com> wrote: > > > > do you want to work on that or should I try to do this and send it to > > > > you for review? > > > > > > I've already sent a patch that fixes the breakages in such way: > > > > > > http://marc.info/?l=linux-kernel&m=121730644704448&w=2 > > > > Yes I have seen this patch. I meant the patches that introduce something > > like IOMMU_PAGE_SIZE for all architectures so that we can make this > > function generic and move it back to lib/. > > That will not work in the light of some new stuff coming on powerpc in > the future. The iommu page size may/will be dynamic per instance of > iommu. IE, it will be mostly decided at runtime, I don't expect machines > to have multiple iommus of different page sizes off hand but it -can- > happen. You could do something like this: #define IOMMU_PAGE_SIZE power_iommu_page_size But I prefer the current proposal to have the size as an argument. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] add iommu_num_pages helper function 2008-07-29 9:50 ` Joerg Roedel 2008-07-29 9:58 ` FUJITA Tomonori @ 2008-07-29 10:10 ` David Miller 2008-07-29 10:26 ` Joerg Roedel ` (3 more replies) 1 sibling, 4 replies; 23+ messages in thread From: David Miller @ 2008-07-29 10:10 UTC (permalink / raw) To: joerg.roedel Cc: sfr, fujita.tomonori, mingo, mingo, tglx, linux-kernel, iommu, bhavna.sarathy, robert.richter, torvalds, akpm, jbarnes, linux-next From: Joerg Roedel <joerg.roedel@amd•com> Date: Tue, 29 Jul 2008 11:50:27 +0200 > I think moving that function to x86-only code is the best solution for > now. Before moving it back to generic code we need an architecture > independent way to find out the IOMMU page size. My suggestion (again) is to pass it in as a "const unsigned long io_page_size" argument and update the callers. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] add iommu_num_pages helper function 2008-07-29 10:10 ` David Miller @ 2008-07-29 10:26 ` Joerg Roedel 2008-07-29 10:27 ` FUJITA Tomonori ` (2 subsequent siblings) 3 siblings, 0 replies; 23+ messages in thread From: Joerg Roedel @ 2008-07-29 10:26 UTC (permalink / raw) To: David Miller Cc: sfr, fujita.tomonori, mingo, mingo, tglx, linux-kernel, iommu, bhavna.sarathy, robert.richter, torvalds, akpm, jbarnes, linux-next On Tue, Jul 29, 2008 at 03:10:05AM -0700, David Miller wrote: > From: Joerg Roedel <joerg.roedel@amd•com> > Date: Tue, 29 Jul 2008 11:50:27 +0200 > > > I think moving that function to x86-only code is the best solution for > > now. Before moving it back to generic code we need an architecture > > independent way to find out the IOMMU page size. > > My suggestion (again) is to pass it in as a "const unsigned long > io_page_size" argument and update the callers. Ok, this has the advantage that it supports more than one IOMMU page size per architecture and its also less intrusive. Joerg -- | AMD Saxony Limited Liability Company & Co. KG Operating | Wilschdorfer Landstr. 101, 01109 Dresden, Germany System | Register Court Dresden: HRA 4896 Research | General Partner authorized to represent: Center | AMD Saxony LLC (Wilmington, Delaware, US) | General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] add iommu_num_pages helper function 2008-07-29 10:10 ` David Miller 2008-07-29 10:26 ` Joerg Roedel @ 2008-07-29 10:27 ` FUJITA Tomonori 2008-07-29 10:31 ` Ingo Molnar 2008-07-30 12:18 ` Joerg Roedel 2008-07-30 22:44 ` Benjamin Herrenschmidt 3 siblings, 1 reply; 23+ messages in thread From: FUJITA Tomonori @ 2008-07-29 10:27 UTC (permalink / raw) To: davem Cc: joerg.roedel, sfr, fujita.tomonori, mingo, mingo, tglx, linux-kernel, iommu, bhavna.sarathy, robert.richter, torvalds, akpm, jbarnes, linux-next On Tue, 29 Jul 2008 03:10:05 -0700 (PDT) David Miller <davem@davemloft•net> wrote: > From: Joerg Roedel <joerg.roedel@amd•com> > Date: Tue, 29 Jul 2008 11:50:27 +0200 > > > I think moving that function to x86-only code is the best solution for > > now. Before moving it back to generic code we need an architecture > > independent way to find out the IOMMU page size. > > My suggestion (again) is to pass it in as a "const unsigned long > io_page_size" argument and update the callers. Either is fine by me. Joerg, if you like, please finish this (for all the architectures, please). As I said before, I planed to work on this cleanup after rc1 (and this should have been 2.6.28 stuff). But I'd appreciate if you would do. Thanks, ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] add iommu_num_pages helper function 2008-07-29 10:27 ` FUJITA Tomonori @ 2008-07-29 10:31 ` Ingo Molnar 2008-07-29 10:33 ` David Miller 0 siblings, 1 reply; 23+ messages in thread From: Ingo Molnar @ 2008-07-29 10:31 UTC (permalink / raw) To: FUJITA Tomonori Cc: davem, joerg.roedel, sfr, mingo, tglx, linux-kernel, iommu, bhavna.sarathy, robert.richter, torvalds, akpm, jbarnes, linux-next * FUJITA Tomonori <fujita.tomonori@lab•ntt.co.jp> wrote: > On Tue, 29 Jul 2008 03:10:05 -0700 (PDT) > David Miller <davem@davemloft•net> wrote: > > > From: Joerg Roedel <joerg.roedel@amd•com> > > Date: Tue, 29 Jul 2008 11:50:27 +0200 > > > > > I think moving that function to x86-only code is the best solution for > > > now. Before moving it back to generic code we need an architecture > > > independent way to find out the IOMMU page size. > > > > My suggestion (again) is to pass it in as a "const unsigned long > > io_page_size" argument and update the callers. > > Either is fine by me. > > Joerg, if you like, please finish this (for all the architectures, > please). As I said before, I planed to work on this cleanup after rc1 > (and this should have been 2.6.28 stuff). But I'd appreciate if you > would do. i've queued up your quick fix in tip/x86/urgent to resolve the immediate -git build breakage. The wider patch can then be done on top of that, without being rushed. Any objections to this approach? Ingo ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] add iommu_num_pages helper function 2008-07-29 10:31 ` Ingo Molnar @ 2008-07-29 10:33 ` David Miller 2008-07-29 12:45 ` Stephen Rothwell 0 siblings, 1 reply; 23+ messages in thread From: David Miller @ 2008-07-29 10:33 UTC (permalink / raw) To: mingo Cc: fujita.tomonori, joerg.roedel, sfr, mingo, tglx, linux-kernel, iommu, bhavna.sarathy, robert.richter, torvalds, akpm, jbarnes, linux-next From: Ingo Molnar <mingo@elte•hu> Date: Tue, 29 Jul 2008 12:31:46 +0200 > i've queued up your quick fix in tip/x86/urgent to resolve the immediate > -git build breakage. The wider patch can then be done on top of that, > without being rushed. Any objections to this approach? I do not object :-) ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] add iommu_num_pages helper function 2008-07-29 10:33 ` David Miller @ 2008-07-29 12:45 ` Stephen Rothwell 0 siblings, 0 replies; 23+ messages in thread From: Stephen Rothwell @ 2008-07-29 12:45 UTC (permalink / raw) To: David Miller Cc: mingo, fujita.tomonori, joerg.roedel, mingo, tglx, linux-kernel, iommu, bhavna.sarathy, robert.richter, torvalds, akpm, jbarnes, linux-next [-- Attachment #1: Type: text/plain, Size: 541 bytes --] On Tue, 29 Jul 2008 03:33:42 -0700 (PDT) David Miller <davem@davemloft•net> wrote: > > From: Ingo Molnar <mingo@elte•hu> > Date: Tue, 29 Jul 2008 12:31:46 +0200 > > > i've queued up your quick fix in tip/x86/urgent to resolve the immediate > > -git build breakage. The wider patch can then be done on top of that, > > without being rushed. Any objections to this approach? > > I do not object :-) Neither do I. :-) -- Cheers, Stephen Rothwell sfr@canb•auug.org.au http://www.canb.auug.org.au/~sfr/ [-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --] ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] add iommu_num_pages helper function 2008-07-29 10:10 ` David Miller 2008-07-29 10:26 ` Joerg Roedel 2008-07-29 10:27 ` FUJITA Tomonori @ 2008-07-30 12:18 ` Joerg Roedel 2008-07-30 13:16 ` FUJITA Tomonori 2008-07-30 22:44 ` Benjamin Herrenschmidt 3 siblings, 1 reply; 23+ messages in thread From: Joerg Roedel @ 2008-07-30 12:18 UTC (permalink / raw) To: David Miller Cc: sfr, fujita.tomonori, mingo, mingo, tglx, linux-kernel, iommu, bhavna.sarathy, robert.richter, torvalds, akpm, jbarnes, linux-next On Tue, Jul 29, 2008 at 03:10:05AM -0700, David Miller wrote: > From: Joerg Roedel <joerg.roedel@amd•com> > Date: Tue, 29 Jul 2008 11:50:27 +0200 > > > I think moving that function to x86-only code is the best solution for > > now. Before moving it back to generic code we need an architecture > > independent way to find out the IOMMU page size. > > My suggestion (again) is to pass it in as a "const unsigned long > io_page_size" argument and update the callers. Ok, after setting up cross compile environments for sparc64 and powerpc64 I wrote a patch which adds the io_page_size argument to the iommu_num pages function and moves the function back to lib/iommu-helper.c. The patch touches all 3 architectures and can not be split in a bisectable way. Who is the best person to send this patch to? Joerg -- | AMD Saxony Limited Liability Company & Co. KG Operating | Wilschdorfer Landstr. 101, 01109 Dresden, Germany System | Register Court Dresden: HRA 4896 Research | General Partner authorized to represent: Center | AMD Saxony LLC (Wilmington, Delaware, US) | General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] add iommu_num_pages helper function 2008-07-30 12:18 ` Joerg Roedel @ 2008-07-30 13:16 ` FUJITA Tomonori 2008-07-30 14:03 ` Joerg Roedel 2008-07-30 18:52 ` Joerg Roedel 0 siblings, 2 replies; 23+ messages in thread From: FUJITA Tomonori @ 2008-07-30 13:16 UTC (permalink / raw) To: joerg.roedel Cc: davem, sfr, fujita.tomonori, mingo, mingo, tglx, linux-kernel, iommu, bhavna.sarathy, robert.richter, torvalds, akpm, jbarnes, linux-next On Wed, 30 Jul 2008 14:18:05 +0200 Joerg Roedel <joerg.roedel@amd•com> wrote: > On Tue, Jul 29, 2008 at 03:10:05AM -0700, David Miller wrote: > > From: Joerg Roedel <joerg.roedel@amd•com> > > Date: Tue, 29 Jul 2008 11:50:27 +0200 > > > > > I think moving that function to x86-only code is the best solution for > > > now. Before moving it back to generic code we need an architecture > > > independent way to find out the IOMMU page size. > > > > My suggestion (again) is to pass it in as a "const unsigned long > > io_page_size" argument and update the callers. > > Ok, after setting up cross compile environments for sparc64 and > powerpc64 I wrote a patch which adds the io_page_size argument to the > iommu_num pages function and moves the function back to > lib/iommu-helper.c. You need ia64, parisc, and alpha too. :) I have cross compile environments for them so I can take care of them if you like. > The patch touches all 3 architectures and can not be > split in a bisectable way. How about naming a new helper function iommu_nr_pages (or something)? Then you can do the conversion independently. If we like, we use iommu_num_pages name after the conversion. > Who is the best person to send this patch to? The -mm is an appropriate tree, I think. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] add iommu_num_pages helper function 2008-07-30 13:16 ` FUJITA Tomonori @ 2008-07-30 14:03 ` Joerg Roedel 2008-07-30 18:52 ` Joerg Roedel 1 sibling, 0 replies; 23+ messages in thread From: Joerg Roedel @ 2008-07-30 14:03 UTC (permalink / raw) To: FUJITA Tomonori Cc: davem, sfr, mingo, mingo, tglx, linux-kernel, iommu, bhavna.sarathy, robert.richter, torvalds, akpm, jbarnes, linux-next On Wed, Jul 30, 2008 at 10:16:05PM +0900, FUJITA Tomonori wrote: > On Wed, 30 Jul 2008 14:18:05 +0200 > Joerg Roedel <joerg.roedel@amd•com> wrote: > > Ok, after setting up cross compile environments for sparc64 and > > powerpc64 I wrote a patch which adds the io_page_size argument to the > > iommu_num pages function and moves the function back to > > lib/iommu-helper.c. > > You need ia64, parisc, and alpha too. :) > > I have cross compile environments for them so I can take care of them > if you like. Ok, so you suggest to create a patchset to convert the other IOMMU implementations too. Thats fine for me, I will try to do it :) Do you have hardware to do runtime tests of the code? I have hardware only for x86, sparc64 and alpha. > > > The patch touches all 3 architectures and can not be > > split in a bisectable way. > > How about naming a new helper function iommu_nr_pages (or something)? > > Then you can do the conversion independently. If we like, we use > iommu_num_pages name after the conversion. Ok, this would mean only 3 renaming patches to keep the patch series bisectable. This should be acceptable. > > > Who is the best person to send this patch to? > > The -mm is an appropriate tree, I think. Ok, then I submit it to Andrew once its tested. Thanks, Joerg -- | AMD Saxony Limited Liability Company & Co. KG Operating | Wilschdorfer Landstr. 101, 01109 Dresden, Germany System | Register Court Dresden: HRA 4896 Research | General Partner authorized to represent: Center | AMD Saxony LLC (Wilmington, Delaware, US) | General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] add iommu_num_pages helper function 2008-07-30 13:16 ` FUJITA Tomonori 2008-07-30 14:03 ` Joerg Roedel @ 2008-07-30 18:52 ` Joerg Roedel 1 sibling, 0 replies; 23+ messages in thread From: Joerg Roedel @ 2008-07-30 18:52 UTC (permalink / raw) To: FUJITA Tomonori Cc: davem, sfr, mingo, mingo, tglx, linux-kernel, iommu, bhavna.sarathy, robert.richter, torvalds, akpm, jbarnes, linux-next On Wed, Jul 30, 2008 at 10:16:05PM +0900, FUJITA Tomonori wrote: > On Wed, 30 Jul 2008 14:18:05 +0200 > Joerg Roedel <joerg.roedel@amd•com> wrote: > > > > Ok, after setting up cross compile environments for sparc64 and > > powerpc64 I wrote a patch which adds the io_page_size argument to the > > iommu_num pages function and moves the function back to > > lib/iommu-helper.c. > > You need ia64, parisc, and alpha too. :) Ok, I looked into the different IOMMU drivers for pa-risc and ia64. The changes needed there are non-trival and I can't test them on real hardware (I only have an pa-risc 1.1). So I think I skip these 2 architectures for now. They can be changed by somebody who has the hardware :) (Alpha instead was trival. I changed this driver) Joerg -- | AMD Saxony Limited Liability Company & Co. KG Operating | Wilschdorfer Landstr. 101, 01109 Dresden, Germany System | Register Court Dresden: HRA 4896 Research | General Partner authorized to represent: Center | AMD Saxony LLC (Wilmington, Delaware, US) | General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] add iommu_num_pages helper function 2008-07-29 10:10 ` David Miller ` (2 preceding siblings ...) 2008-07-30 12:18 ` Joerg Roedel @ 2008-07-30 22:44 ` Benjamin Herrenschmidt 3 siblings, 0 replies; 23+ messages in thread From: Benjamin Herrenschmidt @ 2008-07-30 22:44 UTC (permalink / raw) To: David Miller Cc: joerg.roedel, sfr, fujita.tomonori, mingo, mingo, tglx, linux-kernel, iommu, bhavna.sarathy, robert.richter, torvalds, akpm, jbarnes, linux-next On Tue, 2008-07-29 at 03:10 -0700, David Miller wrote: > From: Joerg Roedel <joerg.roedel@amd•com> > Date: Tue, 29 Jul 2008 11:50:27 +0200 > > > I think moving that function to x86-only code is the best solution for > > now. Before moving it back to generic code we need an architecture > > independent way to find out the IOMMU page size. > > My suggestion (again) is to pass it in as a "const unsigned long > io_page_size" argument and update the callers. Yeah. That's better Ben. ^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2008-07-31 2:15 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20080725233325W.fujita.tomonori@lab.ntt.co.jp>
[not found] ` <20080726134156.GC4401@elte.hu>
[not found] ` <20080729110749.728c54e6.sfr@canb.auug.org.au>
[not found] ` <20080729104722H.fujita.tomonori@lab.ntt.co.jp>
2008-07-29 3:56 ` [PATCH] add iommu_num_pages helper function Stephen Rothwell
2008-07-29 4:38 ` FUJITA Tomonori
2008-07-29 5:26 ` David Miller
2008-07-29 5:42 ` FUJITA Tomonori
2008-07-29 5:24 ` David Miller
2008-07-29 5:42 ` FUJITA Tomonori
2008-07-29 6:45 ` Stephen Rothwell
2008-07-29 9:50 ` Joerg Roedel
2008-07-29 9:58 ` FUJITA Tomonori
2008-07-29 10:02 ` Joerg Roedel
2008-07-30 22:44 ` Benjamin Herrenschmidt
2008-07-31 2:07 ` FUJITA Tomonori
2008-07-29 10:10 ` David Miller
2008-07-29 10:26 ` Joerg Roedel
2008-07-29 10:27 ` FUJITA Tomonori
2008-07-29 10:31 ` Ingo Molnar
2008-07-29 10:33 ` David Miller
2008-07-29 12:45 ` Stephen Rothwell
2008-07-30 12:18 ` Joerg Roedel
2008-07-30 13:16 ` FUJITA Tomonori
2008-07-30 14:03 ` Joerg Roedel
2008-07-30 18:52 ` Joerg Roedel
2008-07-30 22:44 ` Benjamin Herrenschmidt
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox