Re: [PATCH v2] ALSA: emu10k1: Stop using iommu_present()
On Tue, 05 Apr 2022 15:27:54 +0200, Robin Murphy wrote: > > iommu_get_domain_for_dev() is already perfectly happy to return NULL > if the given device has no IOMMU. Drop the unnecessary check in favour > of just handling that condition appropriately. > > Signed-off-by: Robin Murphy > --- > > v2: Get "!domain" condition right Applied now. Thanks. Takashi ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] ALSA: emu10k1: Stop using iommu_present()
On Tue, 05 Apr 2022 14:13:33 +0200, Robin Murphy wrote: > > iommu_get_domain_for_dev() is already perfectly happy to return NULL > if the given device has no IOMMU. Drop the unnecessary check. > > Signed-off-by: Robin Murphy This will change the code behavior. The current code does nothing if no IOMMU is found, but after your removal of the check, the code reaches to emu->iommu_workaround = true incorrectly. thanks, Takashi > --- > sound/pci/emu10k1/emu10k1_main.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/sound/pci/emu10k1/emu10k1_main.c > b/sound/pci/emu10k1/emu10k1_main.c > index 86cc1ca025e4..5ffab343b89c 100644 > --- a/sound/pci/emu10k1/emu10k1_main.c > +++ b/sound/pci/emu10k1/emu10k1_main.c > @@ -1751,9 +1751,6 @@ static void snd_emu10k1_detect_iommu(struct snd_emu10k1 > *emu) > > emu->iommu_workaround = false; > > - if (!iommu_present(emu->card->dev->bus)) > - return; > - > domain = iommu_get_domain_for_dev(emu->card->dev); > if (domain && domain->type == IOMMU_DOMAIN_IDENTITY) > return; > -- > 2.28.0.dirty > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
dma_mmap_coherent() breakage with mem_encrypt on AMD Ryzen
Hi, we've got a bug report recently about the garbage playback sound from a PCI sound device with mem_encrypt on AMD Ryzen: https://bugzilla.kernel.org/show_bug.cgi?id=27 The debug session showed that this happens only with the mmap using dma_mmap_coherent() and mem_encrypt. The mmap with the legacy page fault (as done in ALSA PCM core) seems still working even with the mem_encrypt. Since the problem could be observed on two different PCI drivers, it looks like a generic problem of DMA-mmap implementation with AMD memory encryption. According to the reporter, it's seen on both 5.4.x and 5.10.x kernels, hence it doesn't like a recent regression. Can anyone take a look? Thanks! Takashi ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu/amd: Apply the same IVRS IOAPIC workaround to Acer Aspire A315-41
Acer Aspire A315-41 requires the very same workaround as the existing
quirk for Dell Latitude 5495. Add the new entry for that.
BugLink: https://bugzilla.suse.com/show_bug.cgi?id=1137799
Signed-off-by: Takashi Iwai
---
drivers/iommu/amd_iommu_quirks.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/drivers/iommu/amd_iommu_quirks.c b/drivers/iommu/amd_iommu_quirks.c
index c235f79b7a20..5120ce4fdce3 100644
--- a/drivers/iommu/amd_iommu_quirks.c
+++ b/drivers/iommu/amd_iommu_quirks.c
@@ -73,6 +73,19 @@ static const struct dmi_system_id ivrs_quirks[] __initconst
= {
},
.driver_data = (void *)_ioapic_quirks[DELL_LATITUDE_5495],
},
+ {
+ /*
+* Acer Aspire A315-41 requires the very same workaround as
+* Dell Latitude 5495
+*/
+ .callback = ivrs_ioapic_quirk_cb,
+ .ident = "Acer Aspire A315-41",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Acer"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Aspire A315-41"),
+ },
+ .driver_data = (void *)_ioapic_quirks[DELL_LATITUDE_5495],
+ },
{
.callback = ivrs_ioapic_quirk_cb,
.ident = "Lenovo ideapad 330S-15ARR",
--
2.16.4
Re: [PATCH 4/7] ALSA: pcm: use dma_can_mmap() to check if a device supports dma_mmap_*
On Tue, 06 Aug 2019 07:29:49 +0200, Christoph Hellwig wrote: > > On Mon, Aug 05, 2019 at 11:22:03AM +0200, Takashi Iwai wrote: > > This won't work as expected, unfortunately. It's a bit tricky check, > > since the driver may have its own mmap implementation via > > substream->ops->mmap, and the dma_buffer.dev.dev might point to > > another object depending on the dma_buffer.dev.type. > > > > So please replace with something like below: > > >From my gut feeling I'd reorder it a bit to make it more clear like > this: > > http://git.infradead.org/users/hch/misc.git/commitdiff/958fccf54d00c16740522f818d23f9350498e911 > > if this is fine it'll be in the next version, waiting for a little more > feedback on the other patches. Yes, the new one looks good. Reviewed-by: Takashi Iwai Thanks! Takashi
Re: [PATCH 4/7] ALSA: pcm: use dma_can_mmap() to check if a device supports dma_mmap_*
On Mon, 05 Aug 2019 11:11:56 +0200,
Christoph Hellwig wrote:
>
> Replace the local hack with the dma_can_mmap helper to check if
> a given device supports mapping DMA allocations to userspace.
>
> Signed-off-by: Christoph Hellwig
> ---
> sound/core/pcm_native.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
> index 703857aab00f..81c82c3ee8a2 100644
> --- a/sound/core/pcm_native.c
> +++ b/sound/core/pcm_native.c
> @@ -220,12 +220,11 @@ static bool hw_support_mmap(struct snd_pcm_substream
> *substream)
> {
> if (!(substream->runtime->hw.info & SNDRV_PCM_INFO_MMAP))
> return false;
> - /* architecture supports dma_mmap_coherent()? */
> -#if defined(CONFIG_ARCH_NO_COHERENT_DMA_MMAP) || !defined(CONFIG_HAS_DMA)
> + if (!dma_can_mmap(substream->dma_buffer.dev.dev))
> + return false;
> if (!substream->ops->mmap &&
> substream->dma_buffer.dev.type == SNDRV_DMA_TYPE_DEV)
> return false;
> -#endif
This won't work as expected, unfortunately. It's a bit tricky check,
since the driver may have its own mmap implementation via
substream->ops->mmap, and the dma_buffer.dev.dev might point to
another object depending on the dma_buffer.dev.type.
So please replace with something like below:
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -221,11 +221,10 @@ static bool hw_support_mmap(struct snd_pcm_substream
*substream)
if (!(substream->runtime->hw.info & SNDRV_PCM_INFO_MMAP))
return false;
/* architecture supports dma_mmap_coherent()? */
-#if defined(CONFIG_ARCH_NO_COHERENT_DMA_MMAP) || !defined(CONFIG_HAS_DMA)
if (!substream->ops->mmap &&
- substream->dma_buffer.dev.type == SNDRV_DMA_TYPE_DEV)
+ substream->dma_buffer.dev.type == SNDRV_DMA_TYPE_DEV &&
+ !dma_can_mmap(substream->dma_buffer.dev.dev))
return false;
-#endif
return true;
}
Thanks!
Takashi
Re: [PATCH 5/5] dma-mapping: remove ARCH_NO_COHERENT_DMA_MMAP
On Sat, 03 Aug 2019 12:30:24 +0200, Christoph Hellwig wrote: > > On Fri, Aug 02, 2019 at 10:24:02AM +0200, Takashi Iwai wrote: > > I wasn't careful enough to look at that change, sorry. > > > > The code there tries to check whether dma_mmap_coherent() would always > > fail on some platforms. Then the driver clears the mmap capability > > flag at the device open time and notifies user-space to fall back to > > the dumb read/write mode. > > > > So I'm afraid that simply dropping the check would cause the behavior > > regression, e.g. on PARISC. > > > > Is there any simple way to test whether dma_mmap_coherent() would work > > or not in general on the target platform? It's not necessarily in an > > ifdef at all. > > This isn't really a platform, but a per-device question. I can add a > "bool dma_can_mmap(struct device *dev)" helper to check that. Yes, this would fit perfect. > But how > do I get at a suitable struct device in hw_support_mmap()? substream->dma_buffer.dev.dev can be that, which is used in the mmap helper side, snd_pcm_lib_default_mmap(). Thanks! Takashi
Re: [PATCH 5/5] dma-mapping: remove ARCH_NO_COHERENT_DMA_MMAP
On Fri, 02 Aug 2019 09:03:54 +0200, Christoph Hellwig wrote: > > Takashi, > > any comments on the sounds/ side of this? I wasn't careful enough to look at that change, sorry. The code there tries to check whether dma_mmap_coherent() would always fail on some platforms. Then the driver clears the mmap capability flag at the device open time and notifies user-space to fall back to the dumb read/write mode. So I'm afraid that simply dropping the check would cause the behavior regression, e.g. on PARISC. Is there any simple way to test whether dma_mmap_coherent() would work or not in general on the target platform? It's not necessarily in an ifdef at all. thanks, Takashi > On Thu, Jul 25, 2019 at 08:34:01AM +0200, Christoph Hellwig wrote: > > Now that we never use a default ->mmap implementation, and non-coherent > > architectures can control the presence of ->mmap support by enabling > > ARCH_HAS_DMA_COHERENT_TO_PFN for the dma direct implementation there > > is no need for a global config option to control the availability > > of dma_common_mmap. > > > > Signed-off-by: Christoph Hellwig > > --- > > arch/Kconfig| 3 --- > > arch/c6x/Kconfig| 1 - > > arch/m68k/Kconfig | 1 - > > arch/microblaze/Kconfig | 1 - > > arch/parisc/Kconfig | 1 - > > arch/sh/Kconfig | 1 - > > arch/xtensa/Kconfig | 1 - > > kernel/dma/mapping.c| 4 > > sound/core/pcm_native.c | 10 +- > > 9 files changed, 1 insertion(+), 22 deletions(-) > > > > diff --git a/arch/Kconfig b/arch/Kconfig > > index a7b57dd42c26..ec2834206d08 100644 > > --- a/arch/Kconfig > > +++ b/arch/Kconfig > > @@ -790,9 +790,6 @@ config COMPAT_32BIT_TIME > > This is relevant on all 32-bit architectures, and 64-bit architectures > > as part of compat syscall handling. > > > > -config ARCH_NO_COHERENT_DMA_MMAP > > - bool > > - > > config ARCH_NO_PREEMPT > > bool > > > > diff --git a/arch/c6x/Kconfig b/arch/c6x/Kconfig > > index b4fb61c83494..e65e8d82442a 100644 > > --- a/arch/c6x/Kconfig > > +++ b/arch/c6x/Kconfig > > @@ -20,7 +20,6 @@ config C6X > > select OF_EARLY_FLATTREE > > select GENERIC_CLOCKEVENTS > > select MODULES_USE_ELF_RELA > > - select ARCH_NO_COHERENT_DMA_MMAP > > select MMU_GATHER_NO_RANGE if MMU > > > > config MMU > > diff --git a/arch/m68k/Kconfig b/arch/m68k/Kconfig > > index c518d695c376..614b355ae338 100644 > > --- a/arch/m68k/Kconfig > > +++ b/arch/m68k/Kconfig > > @@ -8,7 +8,6 @@ config M68K > > select ARCH_HAS_DMA_PREP_COHERENT if HAS_DMA && MMU && !COLDFIRE > > select ARCH_HAS_SYNC_DMA_FOR_DEVICE if HAS_DMA > > select ARCH_MIGHT_HAVE_PC_PARPORT if ISA > > - select ARCH_NO_COHERENT_DMA_MMAP if !MMU > > select ARCH_NO_PREEMPT if !COLDFIRE > > select BINFMT_FLAT_ARGVP_ENVP_ON_STACK > > select DMA_DIRECT_REMAP if HAS_DMA && MMU && !COLDFIRE > > diff --git a/arch/microblaze/Kconfig b/arch/microblaze/Kconfig > > index d411de05b628..632c9477a0f6 100644 > > --- a/arch/microblaze/Kconfig > > +++ b/arch/microblaze/Kconfig > > @@ -9,7 +9,6 @@ config MICROBLAZE > > select ARCH_HAS_SYNC_DMA_FOR_CPU > > select ARCH_HAS_SYNC_DMA_FOR_DEVICE > > select ARCH_MIGHT_HAVE_PC_PARPORT > > - select ARCH_NO_COHERENT_DMA_MMAP if !MMU > > select ARCH_WANT_IPC_PARSE_VERSION > > select BUILDTIME_EXTABLE_SORT > > select TIMER_OF > > diff --git a/arch/parisc/Kconfig b/arch/parisc/Kconfig > > index 6d732e451071..e9dd88b7f81e 100644 > > --- a/arch/parisc/Kconfig > > +++ b/arch/parisc/Kconfig > > @@ -52,7 +52,6 @@ config PARISC > > select GENERIC_SCHED_CLOCK > > select HAVE_UNSTABLE_SCHED_CLOCK if SMP > > select GENERIC_CLOCKEVENTS > > - select ARCH_NO_COHERENT_DMA_MMAP > > select CPU_NO_EFFICIENT_FFS > > select NEED_DMA_MAP_STATE > > select NEED_SG_DMA_LENGTH > > diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig > > index 6b1b5941b618..f356ee674d89 100644 > > --- a/arch/sh/Kconfig > > +++ b/arch/sh/Kconfig > > @@ -5,7 +5,6 @@ config SUPERH > > select ARCH_HAS_PTE_SPECIAL > > select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST > > select ARCH_MIGHT_HAVE_PC_PARPORT > > - select ARCH_NO_COHERENT_DMA_MMAP if !MMU > > select HAVE_PATA_PLATFORM > > select CLKDEV_LOOKUP > > select DMA_DECLARE_COHERENT > > diff --git a/arch/xtensa/Kconfig b/arch/xtensa/Kconfig > > index ebc135bda921..70653aed3005 100644 > > --- a/arch/xtensa/Kconfig > > +++ b/arch/xtensa/Kconfig > > @@ -5,7 +5,6 @@ config XTENSA > > select ARCH_HAS_BINFMT_FLAT if !MMU > > select ARCH_HAS_SYNC_DMA_FOR_CPU > > select ARCH_HAS_SYNC_DMA_FOR_DEVICE > > - select ARCH_NO_COHERENT_DMA_MMAP if !MMU > > select ARCH_USE_QUEUED_RWLOCKS > > select ARCH_USE_QUEUED_SPINLOCKS > > select ARCH_WANT_FRAME_POINTERS > > diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c > > index 7dff1829c8c5..815446f76995 100644 > > --- a/kernel/dma/mapping.c > > +++
Re: [alsa-devel] don't pass a NULL struct device to DMA API functions
On Fri, 01 Feb 2019 17:09:57 +0100, Christoph Hellwig wrote: > > On Fri, Feb 01, 2019 at 02:16:08PM +0100, Takashi Iwai wrote: > > Actually there are a bunch of ISA sound drivers that still call > > allocators with NULL device. > > > > The patch below should address it, although it's only compile-tested. > > Oh, I missed these "indirect" calls. This looks good to me: > > Reviewed-by: Christoph Hellwig OK, merged this one to for-next branch now as well. thanks, Takashi ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [alsa-devel] [PATCH 17/18] ALSA: hal2: pass struct device to DMA API functions
On Fri, 01 Feb 2019 17:09:06 +0100, Christoph Hellwig wrote: > > On Fri, Feb 01, 2019 at 02:12:45PM +0100, Takashi Iwai wrote: > > Shall I take this one through sound git tree or all through yours? > > Feel free to merge the sound bits through your tree! Alright, merged both patches 17 and 18 now. thanks, Takashi ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [alsa-devel] [PATCH 17/18] ALSA: hal2: pass struct device to DMA API functions
On Fri, 01 Feb 2019 09:48:00 +0100, Christoph Hellwig wrote: > > The DMA API generally relies on a struct device to work properly, and > only barely works without one for legacy reasons. Pass the easily > available struct device from the platform_device to remedy this. > > Signed-off-by: Christoph Hellwig Looks good to me: Reviewed-by: Takashi Iwai Shall I take this one through sound git tree or all through yours? thanks, Takashi ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH RFC] dma-direct: Try reallocation with GFP_DMA32 if possible
On Fri, 20 Apr 2018 11:47:02 +0200,
Christoph Hellwig wrote:
>
> On Mon, Apr 16, 2018 at 05:18:19PM +0200, Takashi Iwai wrote:
> > As the recent swiotlb bug revealed, we seem to have given up the
> > direct DMA allocation too early and felt back to swiotlb allocation.
> > The reason is that swiotlb allocator expected that dma_direct_alloc()
> > would try harder to get pages even below 64bit DMA mask with
> > GFP_DMA32, but the function doesn't do that but only deals with
> > GFP_DMA case.
> >
> > This patch adds a similar fallback reallocation with GFP_DMA32 as
> > we've done with GFP_DMA. The condition is that the coherent mask is
> > smaller than 64bit (i.e. some address limitation), and neither GFP_DMA
> > nor GFP_DMA32 is set beforehand.
> >
> > Signed-off-by: Takashi Iwai <ti...@suse.de>
> >
> > ---
> >
> > This is a resend of a test patch included in the previous thread
> > ("swiotlb: Fix unexpected swiotlb_alloc_coherent() failures").
>
> I like the patch, but as-is it doesn't apply. Can you resend it against
> latest Linus' tree?
It's because it's written on the tree with another fix patch I sent
beforehand ("[PATCH 1/2] dma-direct: Don't repeat allocation for no-op
GFP_DMA").
Could you check that one at first? I'm fine to rebase and resubmit
this one, if still preferred, though.
thanks,
Takashi
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH RFC] dma-direct: Try reallocation with GFP_DMA32 if possible
As the recent swiotlb bug revealed, we seem to have given up the
direct DMA allocation too early and felt back to swiotlb allocation.
The reason is that swiotlb allocator expected that dma_direct_alloc()
would try harder to get pages even below 64bit DMA mask with
GFP_DMA32, but the function doesn't do that but only deals with
GFP_DMA case.
This patch adds a similar fallback reallocation with GFP_DMA32 as
we've done with GFP_DMA. The condition is that the coherent mask is
smaller than 64bit (i.e. some address limitation), and neither GFP_DMA
nor GFP_DMA32 is set beforehand.
Signed-off-by: Takashi Iwai <ti...@suse.de>
---
This is a resend of a test patch included in the previous thread
("swiotlb: Fix unexpected swiotlb_alloc_coherent() failures").
lib/dma-direct.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/lib/dma-direct.c b/lib/dma-direct.c
index bbfb229aa067..970d39155618 100644
--- a/lib/dma-direct.c
+++ b/lib/dma-direct.c
@@ -84,6 +84,13 @@ void *dma_direct_alloc(struct device *dev, size_t size,
dma_addr_t *dma_handle,
__free_pages(page, page_order);
page = NULL;
+ if (IS_ENABLED(CONFIG_ZONE_DMA32) &&
+ dev->coherent_dma_mask < DMA_BIT_MASK(64) &&
+ !(gfp & (GFP_DMA32 | GFP_DMA))) {
+ gfp |= GFP_DMA32;
+ goto again;
+ }
+
if (IS_ENABLED(CONFIG_ZONE_DMA) &&
dev->coherent_dma_mask < DMA_BIT_MASK(32) &&
!(gfp & GFP_DMA)) {
--
2.16.3
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 1/2] dma-direct: Don't repeat allocation for no-op GFP_DMA
When an allocation with lower dma_coherent mask fails,
dma_direct_alloc() retries the allocation with GFP_DMA. But, it's
useless for architectures that has no ZONE_DMA, obviously.
Fix it by adding the check of CONFIG_ZONE_DMA before retrying the
allocation.
Fixes: 95f183916d4b ("dma-direct: retry allocations using GFP_DMA for small
masks")
Signed-off-by: Takashi Iwai <ti...@suse.de>
---
lib/dma-direct.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/dma-direct.c b/lib/dma-direct.c
index c0bba30fef0a..bbfb229aa067 100644
--- a/lib/dma-direct.c
+++ b/lib/dma-direct.c
@@ -84,7 +84,8 @@ void *dma_direct_alloc(struct device *dev, size_t size,
dma_addr_t *dma_handle,
__free_pages(page, page_order);
page = NULL;
- if (dev->coherent_dma_mask < DMA_BIT_MASK(32) &&
+ if (IS_ENABLED(CONFIG_ZONE_DMA) &&
+ dev->coherent_dma_mask < DMA_BIT_MASK(32) &&
!(gfp & GFP_DMA)) {
gfp = (gfp & ~GFP_DMA32) | GFP_DMA;
goto again;
--
2.16.3
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 0/2] swiotlb: Some regression fixes
Hi,
this is a couple of small regression fixes I stumbled on recently.
The first one is a trivial one, and it was already posted in another
thread ("swiotlb: Fix unexpected swiotlb_alloc_coherent() failures").
thanks,
Takashi
===
Takashi Iwai (2):
dma-direct: Don't repeat allocation for no-op GFP_DMA
swiotlb: Fix dma_supported() to consider direct allocation
lib/dma-direct.c | 3 ++-
lib/swiotlb.c| 4
2 files changed, 6 insertions(+), 1 deletion(-)
--
2.16.3
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 2/2] swiotlb: Fix dma_supported() to consider direct allocation
Along with the recent change to use the common swiotlb dma_ops, x86
also takes over the swiotlb_dma_supported(). This caused a regression
when a low bit DMA mask is set; e.g. parport_pc driver now fails to
set the 24bit DMA mask:
parport_pc parport_pc.956: Unable to set coherent dma mask: disabling DMA
It's because swiotlb_dma_supported() only checks the swiotlb range,
and the 24bit DMA mask is below io_tlb_end. OTOH, in the past kernel
versions, x86's swiotlb dma_supported() was NULL, which was
effectively evaluated as the direct DMA, hence the lower DMA mask was
allowed.
This patch fixes the regression by extending swiotlb_dma_supported()
to check the direct DMA at first for covering the primary allocation
before swiotlb.
Fixes: 6e4bf5867783 ("x86/dma: Use generic swiotlb_ops")
Signed-off-by: Takashi Iwai <ti...@suse.de>
---
lib/swiotlb.c | 4
1 file changed, 4 insertions(+)
diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index de7cc540450f..a81502ea79e7 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -1042,6 +1042,10 @@ swiotlb_dma_mapping_error(struct device *hwdev,
dma_addr_t dma_addr)
int
swiotlb_dma_supported(struct device *hwdev, u64 mask)
{
+#ifdef CONFIG_DMA_DIRECT_OPS
+ if (dma_direct_supported(hwdev, mask))
+ return 1;
+#endif
return __phys_to_dma(hwdev, io_tlb_end - 1) <= mask;
}
--
2.16.3
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] swiotlb: Fix unexpected swiotlb_alloc_coherent() failures
On Thu, 12 Apr 2018 12:32:54 +0200, Robin Murphy wrote: > > On 12/04/18 09:27, Takashi Iwai wrote: > > On Thu, 12 Apr 2018 10:19:05 +0200, > > Takashi Iwai wrote: > >> > >> On Thu, 12 Apr 2018 10:03:56 +0200, > >> Takashi Iwai wrote: > >>> > >>> On Thu, 12 Apr 2018 08:02:27 +0200, > >>> Christoph Hellwig wrote: > >>>> > >>>> On Wed, Apr 11, 2018 at 09:28:54AM +0200, Takashi Iwai wrote: > >>>>>> But we should try a GFP_DMA32 allocation first, so this is a bit > >>>>>> surprising. > >>>>> > >>>>> Hm, do we really try that? > >>>>> Through a quick glance, dma_alloc_coherent_gfp_flags() gives GFP_DMA32 > >>>>> only when coherent mask <= DMA_BIT_MASK(32); in the case of iwlwifi, > >>>>> it's 36bit, so GFP_DMA isn't set. > >>>> > >>>> Oh, yes - it is using an odd dma mask, and amdgpu seems to use an > >>>> just as odd 40-bit dma mask. > >>>> > >>>>> We had a fallback allocation with GFP_DMA32 in the past, but this > >>>>> seems gone long time ago along with cleanups (commit c647c3bb2d16). > >>>>> > >>>>> But I haven't followed about this topic for long time, so I might have > >>>>> missed obviously... > >>>> > >>>> I think a fallback would be much better here rather than relying on the > >>>> limited swiotlb buffer bool. dma_direct_alloc (which in 4.17 is also > >>>> used for x86) already has a GFP_DMA fallback, so extending this for > >>>> GFP_DMA32 as well would seem reasonable. > >>>> > >>>> Any volunteers? > >>> > >>> Below is a quick attempt, totally untested. Actually the retry with > >>> GFP_DMA is superfluous for archs without it, so the first patch > >>> corrects it. > >> > >> Gah, scratch this, it doesn't work. A different check is needed... > > > > The v2 patches are below, replaced with IS_ENABLED(CONFIG_ZONE_DMA). > > That looks pretty reasonable to me - I'd be tempted to try a factoring > out a helper function to avoid the "goto again" logic, but that's > hardly crucial. Right, this change won't make the code flow more complex than before, so I guess it's OK as a first step. I'll submit this second patch as RFC while the first one as a safe fix (in addition to another regression fix I stumbled on). > What I'd really love to see is something like the alloc_pages_mask() > proposal from years ago to come back such that GFP_DMA32 could die > entirely, but I don't know anywhere near enough about the mm layer to > consider taking that on myself :( Yeah, it'd be lovely, but I'm afraid it's hard to fly high... thanks, Takashi ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] swiotlb: Fix unexpected swiotlb_alloc_coherent() failures
On Thu, 12 Apr 2018 10:19:05 +0200, Takashi Iwai wrote: > > On Thu, 12 Apr 2018 10:03:56 +0200, > Takashi Iwai wrote: > > > > On Thu, 12 Apr 2018 08:02:27 +0200, > > Christoph Hellwig wrote: > > > > > > On Wed, Apr 11, 2018 at 09:28:54AM +0200, Takashi Iwai wrote: > > > > > But we should try a GFP_DMA32 allocation first, so this is a bit > > > > > surprising. > > > > > > > > Hm, do we really try that? > > > > Through a quick glance, dma_alloc_coherent_gfp_flags() gives GFP_DMA32 > > > > only when coherent mask <= DMA_BIT_MASK(32); in the case of iwlwifi, > > > > it's 36bit, so GFP_DMA isn't set. > > > > > > Oh, yes - it is using an odd dma mask, and amdgpu seems to use an > > > just as odd 40-bit dma mask. > > > > > > > We had a fallback allocation with GFP_DMA32 in the past, but this > > > > seems gone long time ago along with cleanups (commit c647c3bb2d16). > > > > > > > > But I haven't followed about this topic for long time, so I might have > > > > missed obviously... > > > > > > I think a fallback would be much better here rather than relying on the > > > limited swiotlb buffer bool. dma_direct_alloc (which in 4.17 is also > > > used for x86) already has a GFP_DMA fallback, so extending this for > > > GFP_DMA32 as well would seem reasonable. > > > > > > Any volunteers? > > > > Below is a quick attempt, totally untested. Actually the retry with > > GFP_DMA is superfluous for archs without it, so the first patch > > corrects it. > > Gah, scratch this, it doesn't work. A different check is needed... The v2 patches are below, replaced with IS_ENABLED(CONFIG_ZONE_DMA). Takashi 0001-dma-direct-Don-t-repeat-allocation-for-no-op-GFP_DMA.patch Description: Binary data 0002-dma-direct-Try-reallocation-with-GFP_DMA32-if-possib.patch Description: Binary data ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] swiotlb: Fix unexpected swiotlb_alloc_coherent() failures
On Thu, 12 Apr 2018 10:03:56 +0200, Takashi Iwai wrote: > > On Thu, 12 Apr 2018 08:02:27 +0200, > Christoph Hellwig wrote: > > > > On Wed, Apr 11, 2018 at 09:28:54AM +0200, Takashi Iwai wrote: > > > > But we should try a GFP_DMA32 allocation first, so this is a bit > > > > surprising. > > > > > > Hm, do we really try that? > > > Through a quick glance, dma_alloc_coherent_gfp_flags() gives GFP_DMA32 > > > only when coherent mask <= DMA_BIT_MASK(32); in the case of iwlwifi, > > > it's 36bit, so GFP_DMA isn't set. > > > > Oh, yes - it is using an odd dma mask, and amdgpu seems to use an > > just as odd 40-bit dma mask. > > > > > We had a fallback allocation with GFP_DMA32 in the past, but this > > > seems gone long time ago along with cleanups (commit c647c3bb2d16). > > > > > > But I haven't followed about this topic for long time, so I might have > > > missed obviously... > > > > I think a fallback would be much better here rather than relying on the > > limited swiotlb buffer bool. dma_direct_alloc (which in 4.17 is also > > used for x86) already has a GFP_DMA fallback, so extending this for > > GFP_DMA32 as well would seem reasonable. > > > > Any volunteers? > > Below is a quick attempt, totally untested. Actually the retry with > GFP_DMA is superfluous for archs without it, so the first patch > corrects it. Gah, scratch this, it doesn't work. A different check is needed... Takashi ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] swiotlb: Fix unexpected swiotlb_alloc_coherent() failures
On Thu, 12 Apr 2018 08:02:27 +0200, Christoph Hellwig wrote: > > On Wed, Apr 11, 2018 at 09:28:54AM +0200, Takashi Iwai wrote: > > > But we should try a GFP_DMA32 allocation first, so this is a bit > > > surprising. > > > > Hm, do we really try that? > > Through a quick glance, dma_alloc_coherent_gfp_flags() gives GFP_DMA32 > > only when coherent mask <= DMA_BIT_MASK(32); in the case of iwlwifi, > > it's 36bit, so GFP_DMA isn't set. > > Oh, yes - it is using an odd dma mask, and amdgpu seems to use an > just as odd 40-bit dma mask. > > > We had a fallback allocation with GFP_DMA32 in the past, but this > > seems gone long time ago along with cleanups (commit c647c3bb2d16). > > > > But I haven't followed about this topic for long time, so I might have > > missed obviously... > > I think a fallback would be much better here rather than relying on the > limited swiotlb buffer bool. dma_direct_alloc (which in 4.17 is also > used for x86) already has a GFP_DMA fallback, so extending this for > GFP_DMA32 as well would seem reasonable. > > Any volunteers? Below is a quick attempt, totally untested. Actually the retry with GFP_DMA is superfluous for archs without it, so the first patch corrects it. The second patch adds the retry with GFP_DMA32. I'll resubmit properly if these are OK (and better if anyone could test them :) thanks, Takashi 0001-dma-direct-Don-t-repeat-allocation-for-no-op-GFP_DMA.patch Description: Binary data 0002-dma-direct-Try-reallocation-with-GFP_DMA32-if-possib.patch Description: Binary data ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] swiotlb: Fix unexpected swiotlb_alloc_coherent() failures
On Tue, 10 Apr 2018 20:10:20 +0200, Christoph Hellwig wrote: > > On Tue, Apr 10, 2018 at 06:50:04PM +0100, Robin Murphy wrote: > > In the first one, the machine appears to have enough RAM that most of it is > > beyond the device's 36-bit DMA mask, thus it's fairly likely for the > > initial direct alloc to come back with something unsuitable. > > But we should try a GFP_DMA32 allocation first, so this is a bit > surprising. Hm, do we really try that? Through a quick glance, dma_alloc_coherent_gfp_flags() gives GFP_DMA32 only when coherent mask <= DMA_BIT_MASK(32); in the case of iwlwifi, it's 36bit, so GFP_DMA isn't set. We had a fallback allocation with GFP_DMA32 in the past, but this seems gone long time ago along with cleanups (commit c647c3bb2d16). But I haven't followed about this topic for long time, so I might have missed obviously... thanks, Takashi ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] swiotlb: Fix unexpected swiotlb_alloc_coherent() failures
The code refactoring by commit 0176adb00406 ("swiotlb: refactor
coherent buffer allocation") made swiotlb_alloc_buffer() almost always
failing due to a thinko: namely, the function evaluates the
dma_coherent_ok() call incorrectly and dealing as if it's invalid.
This ends up with weird errors like iwlwifi probe failure or amdgpu
screen flickering.
This patch corrects the logic error.
Bugzilla: https://bugzilla.suse.com/show_bug.cgi?id=1088658
Bugzilla: https://bugzilla.suse.com/show_bug.cgi?id=1088902
Fixes: 0176adb00406 ("swiotlb: refactor coherent buffer allocation")
Cc: <sta...@vger.kernel.org> # v4.16+
Signed-off-by: Takashi Iwai <ti...@suse.de>
---
lib/swiotlb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index 47aeb04c1997..de7cc540450f 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -719,7 +719,7 @@ swiotlb_alloc_buffer(struct device *dev, size_t size,
dma_addr_t *dma_handle,
goto out_warn;
*dma_handle = __phys_to_dma(dev, phys_addr);
- if (dma_coherent_ok(dev, *dma_handle, size))
+ if (!dma_coherent_ok(dev, *dma_handle, size))
goto out_unmap;
memset(phys_to_virt(phys_addr), 0, size);
--
2.16.3
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] swiotlb: Fix unexpected swiotlb_alloc_coherent() failures
On Tue, 10 Apr 2018 19:06:15 +0200,
Christoph Hellwig wrote:
>
> On Tue, Apr 10, 2018 at 07:05:13PM +0200, Takashi Iwai wrote:
> > The code refactoring by commit 0176adb00406 ("swiotlb: refactor
> > coherent buffer allocation") made swiotlb_alloc_buffer() almost always
> > failing due to a thinko: namely, the function evaluates the
> > dma_coherent_ok() call incorrectly and dealing as if it's invalid.
> > This ends up with weird errors like iwlwifi probe failure or amdgpu
> > screen flickering.
> >
> > This patch corrects the logic error.
>
> This looks ok, although even hitting this code is a bad sign. Do you
> know why the previous dma_direct_alloc didn't succeed?
That I have no idea, sorry. I just figured out the regression just
from the dmesg output of 4.16 kernel in bugzilla reports.
Could you take a look at the bugzilla reports there?
thanks,
Takashi
> > Bugzilla: https://bugzilla.suse.com/show_bug.cgi?id=1088658
> > Bugzilla: https://bugzilla.suse.com/show_bug.cgi?id=1088902
> > Fixes: 0176adb00406 ("swiotlb: refactor coherent buffer allocation")
> > Cc: <sta...@vger.kernel.org> # v4.16+
> > Signed-off-by: Takashi Iwai <ti...@suse.de>
> > ---
> > lib/swiotlb.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/lib/swiotlb.c b/lib/swiotlb.c
> > index 47aeb04c1997..de7cc540450f 100644
> > --- a/lib/swiotlb.c
> > +++ b/lib/swiotlb.c
> > @@ -719,7 +719,7 @@ swiotlb_alloc_buffer(struct device *dev, size_t size,
> > dma_addr_t *dma_handle,
> > goto out_warn;
> >
> > *dma_handle = __phys_to_dma(dev, phys_addr);
> > - if (dma_coherent_ok(dev, *dma_handle, size))
> > + if (!dma_coherent_ok(dev, *dma_handle, size))
> > goto out_unmap;
> >
> > memset(phys_to_virt(phys_addr), 0, size);
> > --
> > 2.16.3
> ---end quoted text---
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
