On Tue, Apr 24, 2018 at 08:55:49AM +0200, Christoph Hellwig wrote:
> On Tue, Apr 24, 2018 at 12:52:05AM +0100, Russell King - ARM Linux wrote:
> > On Mon, Apr 23, 2018 at 07:04:18PM +0200, Christoph Hellwig wrote:
> > > This way we have one central definition of it, and user can select it as
> > > needed. Note that we also add a second ARCH_HAS_SWIOTLB symbol to
> > > indicate the architecture supports swiotlb at all, so that we can still
> > > make the usage optional for a few architectures that want this feature
> > > to be user selectable.
> > >
> > > Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> >
> > Hmm, this looks like we end up with NEED_SG_DMA_LENGTH=y on ARM by
> > default, which probably isn't a good idea - ARM pre-dates the dma_length
> > parameter in scatterlists, and I don't think all code is guaranteed to
> > do the right thing if this is enabled.
>
> We shouldn't end up with NEED_SG_DMA_LENGTH=y on ARM by default.
Your patch as sent would end up with:
ARM selects ARCH_HAS_SWIOTLB
SWIOTLB is defaulted to ARCH_HAS_SWIOTLB
SWIOTLB selects NEED_SG_DMA_LENGTH
due to:
@@ -106,6 +106,7 @@ config ARM
select REFCOUNT_FULL
select RTC_LIB
select SYS_SUPPORTS_APM_EMULATION
+ select ARCH_HAS_SWIOTLB
and:
+config SWIOTLB
+ bool "SWIOTLB support"
+ default ARCH_HAS_SWIOTLB
+ select NEED_SG_DMA_LENGTH
Therefore, the default state for SWIOTLB and hence NEED_SG_DMA_LENGTH
becomes 'y' on ARM, and any defconfig file that does not mention SWIOTLB
explicitly ends up with both these enabled.
> It is only select by ARM_DMA_USE_IOMMU before the patch, and it will
> now also be selected by SWIOTLB, which for arm is never used or seleted
> directly by anything but xen-swiotlb.
See above.
> Then again looking at the series there shouldn't be any need to
> even select NEED_SG_DMA_LENGTH for swiotlb, as we'll never merge segments,
> so I'll fix that up.
That would help to avoid any regressions along the lines I've spotted
by review.
It does look a bit weird though - patch 10 arranged stuff so that we
didn't end up with SWIOTLB always enabled, but this patch reintroduces
that with the allowance that the user can disable if so desired.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
