| Message ID | 20100302123326H.fujita.tomonori@lab.ntt.co.jp |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
On Tuesday 02 March 2010, FUJITA Tomonori wrote: > If we go with such approach, then we could use something like the > following. There are only two kinds of scatterlist definitions (use > dma_length or not) so we can cover all the architectures. > > diff --git a/include/asm-generic/scatterlist.h b/include/asm-generic/scatterlist.h > index 8b94544..1bf620d 100644 > --- a/include/asm-generic/scatterlist.h > +++ b/include/asm-generic/scatterlist.h > @@ -11,7 +11,9 @@ struct scatterlist { > unsigned int offset; > unsigned int length; > dma_addr_t dma_address; > +#ifdef CONFIG_NEED_SG_DMA_LENGTH > unsigned int dma_length; > +#endif > }; Yes, that sounds good. If the only reason to need dma_length is virtual merging, a clearer (from the Kconfig perspective, not from the implementation) name might be CONFIG_HAVE_IOMMU_VMERGE, similar to the CONFIG_IOMMU_VMERGE option on PPC64 that determines the default for the virtual merging runtime option. Arnd -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2 Mar 2010 13:03:25 +0100 Arnd Bergmann <arnd@arndb.de> wrote: > On Tuesday 02 March 2010, FUJITA Tomonori wrote: > > If we go with such approach, then we could use something like the > > following. There are only two kinds of scatterlist definitions (use > > dma_length or not) so we can cover all the architectures. > > > > diff --git a/include/asm-generic/scatterlist.h b/include/asm-generic/scatterlist.h > > index 8b94544..1bf620d 100644 > > --- a/include/asm-generic/scatterlist.h > > +++ b/include/asm-generic/scatterlist.h > > @@ -11,7 +11,9 @@ struct scatterlist { > > unsigned int offset; > > unsigned int length; > > dma_addr_t dma_address; > > +#ifdef CONFIG_NEED_SG_DMA_LENGTH > > unsigned int dma_length; > > +#endif > > }; > > Yes, that sounds good. If the only reason to need dma_length is virtual merging, > a clearer (from the Kconfig perspective, not from the implementation) name > might be CONFIG_HAVE_IOMMU_VMERGE, similar to the CONFIG_IOMMU_VMERGE option > on PPC64 that determines the default for the virtual merging runtime option. Yeah, but IIRC, Alpha, x86_64 GART, parisc, and IA64 don't have CONFIG_ option for IOMMU virtual merging. I prefer to avoid to adding something like CONFIG_HAVE_IOMMU_VMERGE for them. If we add CONFIG_HAVE_IOMMU_VMERGE to them, it's a bit strange not to add the feature to disable virtual merging for them (I guess GART already has the feature though). Actually, I want to use dma_length on all the architectures (if nobody complains). -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday 02 March 2010, FUJITA Tomonori wrote: > Yeah, but IIRC, Alpha, x86_64 GART, parisc, and IA64 don't have > CONFIG_ option for IOMMU virtual merging. I prefer to avoid to adding > something like CONFIG_HAVE_IOMMU_VMERGE for them. If we add > CONFIG_HAVE_IOMMU_VMERGE to them, it's a bit strange not to add the > feature to disable virtual merging for them (I guess GART already has > the feature though). While I think the runtime feature (actually a workaround for broken device drivers) should be consistently used on all architectures, or removed entirely, it's orthogonal to this discussion. I'm sure you'll come up with a reasonable name for a new option if you introduce one. CONFIG_HAVE_IOMMU_VMERGE and CONFIG_NEED_SG_DMA_LENGTH both seem ok to me. > Actually, I want to use dma_length on all the architectures (if nobody > complains). Fine with me as well. It wastes a small amount of memory but makes the code more consistent. Arnd -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2 Mar 2010 14:38:31 +0100 Arnd Bergmann <arnd@arndb.de> wrote: > On Tuesday 02 March 2010, FUJITA Tomonori wrote: > > Yeah, but IIRC, Alpha, x86_64 GART, parisc, and IA64 don't have > > CONFIG_ option for IOMMU virtual merging. I prefer to avoid to adding > > something like CONFIG_HAVE_IOMMU_VMERGE for them. If we add > > CONFIG_HAVE_IOMMU_VMERGE to them, it's a bit strange not to add the > > feature to disable virtual merging for them (I guess GART already has > > the feature though). > > While I think the runtime feature (actually a workaround for broken device > drivers) What does "broken device drivers" mean here? > should be consistently used on all architectures, or removed > entirely, it's orthogonal to this discussion. -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday 02 March 2010, FUJITA Tomonori wrote: > On Tue, 2 Mar 2010 14:38:31 +0100 > Arnd Bergmann <arnd@arndb.de> wrote: > > > On Tuesday 02 March 2010, FUJITA Tomonori wrote: > > > Yeah, but IIRC, Alpha, x86_64 GART, parisc, and IA64 don't have > > > CONFIG_ option for IOMMU virtual merging. I prefer to avoid to adding > > > something like CONFIG_HAVE_IOMMU_VMERGE for them. If we add > > > CONFIG_HAVE_IOMMU_VMERGE to them, it's a bit strange not to add the > > > feature to disable virtual merging for them (I guess GART already has > > > the feature though). > > > > While I think the runtime feature (actually a workaround for broken device > > drivers) > > What does "broken device drivers" mean here? Broken in the sense that arch/powerpc/Kconfig describes CONFIG_IOMMU_VMERGE: Cause IO segments sent to a device for DMA to be merged virtually by the IOMMU when they happen to have been allocated contiguously. This doesn't add pressure to the IOMMU allocator. However, some drivers don't support getting large merged segments coming back from *_map_sg(). Most drivers don't have this problem; it is safe to say Y here. I don't know if this comment still applies to any drivers in the mainline kernel, but it's possible. Arnd -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Arnd Bergmann <arnd@arndb.de> Date: Tue, 2 Mar 2010 14:54:11 +0100 > Broken in the sense that arch/powerpc/Kconfig describes CONFIG_IOMMU_VMERGE: > > Cause IO segments sent to a device for DMA to be merged virtually > by the IOMMU when they happen to have been allocated contiguously. > This doesn't add pressure to the IOMMU allocator. However, some > drivers don't support getting large merged segments coming back > from *_map_sg(). > > Most drivers don't have this problem; it is safe to say Y here. > > I don't know if this comment still applies to any drivers in the mainline > kernel, but it's possible. That really has to be out of date these days. -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 02 Mar 2010 05:55:34 -0800 (PST) David Miller <davem@davemloft.net> wrote: > From: Arnd Bergmann <arnd@arndb.de> > Date: Tue, 2 Mar 2010 14:54:11 +0100 > > > Broken in the sense that arch/powerpc/Kconfig describes CONFIG_IOMMU_VMERGE: > > > > Cause IO segments sent to a device for DMA to be merged virtually > > by the IOMMU when they happen to have been allocated contiguously. > > This doesn't add pressure to the IOMMU allocator. However, some > > drivers don't support getting large merged segments coming back > > from *_map_sg(). > > > > Most drivers don't have this problem; it is safe to say Y here. > > > > I don't know if this comment still applies to any drivers in the mainline > > kernel, but it's possible. > > That really has to be out of date these days. Yeah, I think so. I added dma_get_max_seg_size() several years ago so that device drives can tell IOMMU about the maximum segment length that they can handle. And the default limit (64K) should work for everyone, I think. I guess that the comment was written when IOMMU was able to merge as many segments as possible with ignoring the device limitation. -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/asm-generic/scatterlist.h b/include/asm-generic/scatterlist.h index 8b94544..1bf620d 100644 --- a/include/asm-generic/scatterlist.h +++ b/include/asm-generic/scatterlist.h @@ -11,7 +11,9 @@ struct scatterlist { unsigned int offset; unsigned int length; dma_addr_t dma_address; +#ifdef CONFIG_NEED_SG_DMA_LENGTH unsigned int dma_length; +#endif }; /* @@ -22,17 +24,11 @@ struct scatterlist { * is 0. */ #define sg_dma_address(sg) ((sg)->dma_address) -#ifndef sg_dma_len -/* - * Normally, you have an iommu on 64 bit machines, but not on 32 bit - * machines. Architectures that are differnt should override this. - */ -#if __BITS_PER_LONG == 64 +#ifdef CONFIG_NEED_SG_DMA_LENGTH #define sg_dma_len(sg) ((sg)->dma_length) #else #define sg_dma_len(sg) ((sg)->length) -#endif /* 64 bit */ -#endif /* sg_dma_len */ +#endif #ifndef ISA_DMA_THRESHOLD #define ISA_DMA_THRESHOLD (~0UL)