Re: [PATCH 1/1] iommu/vt-d: Correctly check format of page table in debugfs
Hi Joerg, Just a friendly reminder. What do you think of this fix? Best regards, Lu Baolu On 7/20/19 10:01 AM, Lu Baolu wrote: PASID support and enable bit in the context entry isn't the right indicator for the type of tables (legacy or scalable mode). Check the DMA_RTADDR_SMT bit in the root context pointer instead. Cc: Ashok Raj Cc: Jacob Pan Cc: Kevin Tian Cc: Sai Praneeth Fixes: dd5142ca5d24b ("iommu/vt-d: Add debugfs support to show scalable mode DMAR table internals") Signed-off-by: Lu Baolu --- drivers/iommu/intel-iommu-debugfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iommu/intel-iommu-debugfs.c b/drivers/iommu/intel-iommu-debugfs.c index 73a552914455..e31c3b416351 100644 --- a/drivers/iommu/intel-iommu-debugfs.c +++ b/drivers/iommu/intel-iommu-debugfs.c @@ -235,7 +235,7 @@ static void ctx_tbl_walk(struct seq_file *m, struct intel_iommu *iommu, u16 bus) tbl_wlk.ctx_entry = context; m->private = _wlk; - if (pasid_supported(iommu) && is_pasid_enabled(context)) { + if (dmar_readq(iommu->reg + DMAR_RTADDR_REG) & DMA_RTADDR_SMT) { pasid_dir_ptr = context->lo & VTD_PAGE_MASK; pasid_dir_size = get_pasid_dir_size(context); pasid_dir_walk(m, pasid_dir_ptr, pasid_dir_size);
Re: [PATCH 1/2] iommu/vt-d: Detach domain before using a private one
Hi Alex, On 8/9/19 4:10 AM, Alex Williamson wrote: On Tue, 6 Aug 2019 08:14:08 +0800 Lu Baolu wrote: When the default domain of a group doesn't work for a device, the iommu driver will try to use a private domain. The domain which was previously attached to the device must be detached. Cc: Ashok Raj Cc: Jacob Pan Cc: Kevin Tian Cc: Alex Williamson Fixes: 942067f1b6b97 ("iommu/vt-d: Identify default domains replaced with private") Reported-by: Alex Williamson Link: https://lkml.org/lkml/2019/8/2/1379 Signed-off-by: Lu Baolu --- Tested-by: Alex Williamson This series resolves the issue I reported against: iommu/vt-d: Cleanup get_valid_domain_for_dev() Thank you for testing it. Best regards, Baolu Thanks, Alex drivers/iommu/intel-iommu.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c index 3e22fa6ae8c8..37259b7f95a7 100644 --- a/drivers/iommu/intel-iommu.c +++ b/drivers/iommu/intel-iommu.c @@ -3449,6 +3449,7 @@ static bool iommu_need_mapping(struct device *dev) dmar_domain = to_dmar_domain(domain); dmar_domain->flags |= DOMAIN_FLAG_LOSE_CHILDREN; } + dmar_remove_one_dev_info(dev); get_private_domain_for_dev(dev); } @@ -4803,7 +4804,8 @@ static void dmar_remove_one_dev_info(struct device *dev) spin_lock_irqsave(_domain_lock, flags); info = dev->archdata.iommu; - __dmar_remove_one_dev_info(info); + if (info) + __dmar_remove_one_dev_info(info); spin_unlock_irqrestore(_domain_lock, flags); } @@ -5281,6 +5283,7 @@ static int intel_iommu_add_device(struct device *dev) if (device_def_domain_type(dev) == IOMMU_DOMAIN_IDENTITY) { ret = iommu_request_dm_for_dev(dev); if (ret) { + dmar_remove_one_dev_info(dev); dmar_domain->flags |= DOMAIN_FLAG_LOSE_CHILDREN; domain_add_dev_info(si_domain, dev); dev_info(dev, @@ -5291,6 +5294,7 @@ static int intel_iommu_add_device(struct device *dev) if (device_def_domain_type(dev) == IOMMU_DOMAIN_DMA) { ret = iommu_request_dma_domain_for_dev(dev); if (ret) { + dmar_remove_one_dev_info(dev); dmar_domain->flags |= DOMAIN_FLAG_LOSE_CHILDREN; if (!get_private_domain_for_dev(dev)) { dev_warn(dev, ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/2] iommu/vt-d: Fix possible use-after-free of private domain
On Tue, 6 Aug 2019 08:14:09 +0800 Lu Baolu wrote: > Multiple devices might share a private domain. One real example > is a pci bridge and all devices behind it. When remove a private > domain, make sure that it has been detached from all devices to > avoid use-after-free case. > > Cc: Ashok Raj > Cc: Jacob Pan > Cc: Kevin Tian > Cc: Alex Williamson > Fixes: 942067f1b6b97 ("iommu/vt-d: Identify default domains replaced with > private") > Signed-off-by: Lu Baolu > --- Tested-by: Alex Williamson > drivers/iommu/intel-iommu.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c > index 37259b7f95a7..12d094d08c0a 100644 > --- a/drivers/iommu/intel-iommu.c > +++ b/drivers/iommu/intel-iommu.c > @@ -4791,7 +4791,8 @@ static void __dmar_remove_one_dev_info(struct > device_domain_info *info) > > /* free the private domain */ > if (domain->flags & DOMAIN_FLAG_LOSE_CHILDREN && > - !(domain->flags & DOMAIN_FLAG_STATIC_IDENTITY)) > + !(domain->flags & DOMAIN_FLAG_STATIC_IDENTITY) && > + list_empty(>devices)) > domain_exit(info->domain); > > free_devinfo_mem(info); ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 3/8] of/fdt: add function to get the SoC wide DMA addressable memory size
On Thu, 2019-08-08 at 09:02 -0600, Rob Herring wrote: > On Tue, Aug 6, 2019 at 12:12 PM Nicolas Saenz Julienne > wrote: > > Hi Rob, > > > > On Mon, 2019-08-05 at 13:23 -0600, Rob Herring wrote: > > > On Mon, Aug 5, 2019 at 10:03 AM Nicolas Saenz Julienne > > > wrote: > > > > Hi Rob, > > > > Thanks for the review! > > > > > > > > On Fri, 2019-08-02 at 11:17 -0600, Rob Herring wrote: > > > > > On Wed, Jul 31, 2019 at 9:48 AM Nicolas Saenz Julienne > > > > > wrote: > > > > > > Some SoCs might have multiple interconnects each with their own DMA > > > > > > addressing limitations. This function parses the 'dma-ranges' on > > > > > > each of > > > > > > them and tries to guess the maximum SoC wide DMA addressable memory > > > > > > size. > > > > > > > > > > > > This is specially useful for arch code in order to properly setup > > > > > > CMA > > > > > > and memory zones. > > > > > > > > > > We already have a way to setup CMA in reserved-memory, so why is this > > > > > needed for that? > > > > > > > > Correct me if I'm wrong but I got the feeling you got the point of the > > > > patch > > > > later on. > > > > > > No, for CMA I don't. Can't we already pass a size and location for CMA > > > region under /reserved-memory. The only advantage here is perhaps the > > > CMA range could be anywhere in the DMA zone vs. a fixed location. > > > > Now I get it, sorry I wasn't aware of that interface. > > > > Still, I'm not convinced it matches RPi's use case as this would hard-code > > CMA's size. Most people won't care, but for the ones that do, it's nicer to > > change the value from the kernel command line than editing the dtb. > > Sure, I fully agree and am not a fan of the CMA DT overlays I've seen. > > > I get that > > if you need to, for example, reserve some memory for the video to work, it's > > silly not to hard-code it. Yet due to the board's nature and users base I > > say > > it's important to favor flexibility. It would also break compatibility with > > earlier versions of the board and diverge from the downstream kernel > > behaviour. > > Which is a bigger issue than it seems as most users don't always understand > > which kernel they are running and unknowingly copy configuration options > > from > > forums. > > > > As I also need to know the DMA addressing limitations to properly configure > > memory zones and dma-direct. Setting up the proper CMA constraints during > > the > > arch's init will be trivial anyway. > > It was really just commentary on commit text as for CMA alone we have > a solution already. I agree on the need for zones. Ok, understood :) > > > > > IMO, I'd just do: > > > > > > > > > > if (of_fdt_machine_is_compatible(blob, "brcm,bcm2711")) > > > > > dma_zone_size = XX; > > > > > > > > > > 2 lines of code is much easier to maintain than 10s of incomplete code > > > > > and is clearer who needs this. Maybe if we have dozens of SoCs with > > > > > this problem we should start parsing dma-ranges. > > > > > > > > FYI that's what arm32 is doing at the moment and was my first instinct. > > > > But > > > > it > > > > seems that arm64 has been able to survive so far without any machine > > > > specific > > > > code and I have the feeling Catalin and Will will not be happy about > > > > this > > > > solution. Am I wrong? > > > > > > No doubt. I'm fine if the 2 lines live in drivers/of/. > > > > > > Note that I'm trying to reduce the number of early_init_dt_scan_* > > > calls from arch code into the DT code so there's more commonality > > > across architectures in the early DT scans. So ideally, this can all > > > be handled under early_init_dt_scan() call. > > > > How does this look? (I'll split it in two patches and add a comment > > explaining > > why dt_dma_zone_size is needed) > > > > diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c > > index f2444c61a136..1395be40b722 100644 > > --- a/drivers/of/fdt.c > > +++ b/drivers/of/fdt.c > > @@ -30,6 +30,8 @@ > > > > #include "of_private.h" > > > > +u64 dt_dma_zone_size __ro_after_init; > > Avoiding a call from arch code by just having a variable isn't really > better. I'd rather see a common, non DT specific variable that can be > adjusted. Something similar to initrd_start/end. Then the arch code > doesn't have to care what hardware description code adjusted the > value. Way better, I'll update it. signature.asc Description: This is a digitally signed message part
[PATCH 8/8] dma-mapping: remove CONFIG_ARCH_NO_COHERENT_DMA_MMAP
CONFIG_ARCH_NO_COHERENT_DMA_MMAP is now functionally identical to !CONFIG_MMU, so remove the separate symbol. The only difference is that arm did not set it for !CONFIG_MMU, but arm uses a separate dma mapping implementation including its own mmap method, which is handled by moving the CONFIG_MMU check in dma_can_mmap so that is only applies to the dma-direct case, just as the other ifdefs for it. Signed-off-by: Christoph Hellwig --- arch/Kconfig| 3 --- arch/c6x/Kconfig| 1 - arch/m68k/Kconfig | 1 - arch/microblaze/Kconfig | 1 - arch/sh/Kconfig | 1 - arch/xtensa/Kconfig | 1 - kernel/dma/mapping.c| 12 +--- 7 files changed, 5 insertions(+), 15 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/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 64d1de59e133..fc17016b0871 100644 --- a/kernel/dma/mapping.c +++ b/kernel/dma/mapping.c @@ -186,7 +186,7 @@ int dma_common_mmap(struct device *dev, struct vm_area_struct *vma, void *cpu_addr, dma_addr_t dma_addr, size_t size, unsigned long attrs) { -#ifndef CONFIG_ARCH_NO_COHERENT_DMA_MMAP +#ifdef CONFIG_MMU unsigned long user_count = vma_pages(vma); unsigned long count = PAGE_ALIGN(size) >> PAGE_SHIFT; unsigned long off = vma->vm_pgoff; @@ -217,7 +217,7 @@ int dma_common_mmap(struct device *dev, struct vm_area_struct *vma, user_count << PAGE_SHIFT, vma->vm_page_prot); #else return -ENXIO; -#endif /* !CONFIG_ARCH_NO_COHERENT_DMA_MMAP */ +#endif /* CONFIG_MMU */ } /** @@ -231,12 +231,10 @@ bool dma_can_mmap(struct device *dev) { const struct dma_map_ops *ops = get_dma_ops(dev); - if (IS_ENABLED(CONFIG_ARCH_NO_COHERENT_DMA_MMAP)) - return false; - if (dma_is_direct(ops)) { - return dev_is_dma_coherent(dev) || - IS_ENABLED(CONFIG_ARCH_HAS_DMA_COHERENT_TO_PFN); + return IS_ENABLED(CONFIG_MMU) && + (dev_is_dma_coherent(dev) || + IS_ENABLED(CONFIG_ARCH_HAS_DMA_COHERENT_TO_PFN)); } return ops->mmap != NULL; -- 2.20.1
[PATCH 7/8] parisc: don't set ARCH_NO_COHERENT_DMA_MMAP
parisc is the only architecture that sets ARCH_NO_COHERENT_DMA_MMAP when an MMU is enabled. AFAIK this is because parisc CPUs use VIVT caches, which means exporting normally cachable memory to userspace is relatively dangrous due to cache aliasing. But normally cachable memory is only allocated by dma_alloc_coherent on parisc when using the sba_iommu or ccio_iommu drivers, so just remove the .mmap implementation for them so that we don't have to set ARCH_NO_COHERENT_DMA_MMAP, which I plan to get rid of. Signed-off-by: Christoph Hellwig --- arch/parisc/Kconfig| 1 - drivers/parisc/ccio-dma.c | 1 - drivers/parisc/sba_iommu.c | 1 - 3 files changed, 3 deletions(-) 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/drivers/parisc/ccio-dma.c b/drivers/parisc/ccio-dma.c index 1d7125d29bee..ad290f79983b 100644 --- a/drivers/parisc/ccio-dma.c +++ b/drivers/parisc/ccio-dma.c @@ -1024,7 +1024,6 @@ static const struct dma_map_ops ccio_ops = { .unmap_page = ccio_unmap_page, .map_sg = ccio_map_sg, .unmap_sg = ccio_unmap_sg, - .mmap = dma_common_mmap, .get_sgtable = dma_common_get_sgtable, }; diff --git a/drivers/parisc/sba_iommu.c b/drivers/parisc/sba_iommu.c index fa4df65b7e28..ed50502cc65a 100644 --- a/drivers/parisc/sba_iommu.c +++ b/drivers/parisc/sba_iommu.c @@ -1084,7 +1084,6 @@ static const struct dma_map_ops sba_ops = { .unmap_page = sba_unmap_page, .map_sg = sba_map_sg, .unmap_sg = sba_unmap_sg, - .mmap = dma_common_mmap, .get_sgtable = dma_common_get_sgtable, }; -- 2.20.1
[PATCH 2/8] dma-mapping: move the dma_get_sgtable API comments from arm to common code
The comments are spot on and should be near the central API, not just near a single implementation. Signed-off-by: Christoph Hellwig --- arch/arm/mm/dma-mapping.c | 11 --- kernel/dma/mapping.c | 11 +++ 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index ad64d32fb39a..b4d65da76393 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -880,17 +880,6 @@ static void arm_coherent_dma_free(struct device *dev, size_t size, void *cpu_add __arm_dma_free(dev, size, cpu_addr, handle, attrs, true); } -/* - * The whole dma_get_sgtable() idea is fundamentally unsafe - it seems - * that the intention is to allow exporting memory allocated via the - * coherent DMA APIs through the dma_buf API, which only accepts a - * scattertable. This presents a couple of problems: - * 1. Not all memory allocated via the coherent DMA APIs is backed by - *a struct page - * 2. Passing coherent DMA memory into the streaming APIs is not allowed - *as we will try to flush the memory through a different alias to that - *actually being used (and the flushes are redundant.) - */ int arm_dma_get_sgtable(struct device *dev, struct sg_table *sgt, void *cpu_addr, dma_addr_t handle, size_t size, unsigned long attrs) diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c index 9c0f6a8eb5cb..41590d003465 100644 --- a/kernel/dma/mapping.c +++ b/kernel/dma/mapping.c @@ -136,6 +136,17 @@ int dma_common_get_sgtable(struct device *dev, struct sg_table *sgt, return ret; } +/* + * The whole dma_get_sgtable() idea is fundamentally unsafe - it seems + * that the intention is to allow exporting memory allocated via the + * coherent DMA APIs through the dma_buf API, which only accepts a + * scattertable. This presents a couple of problems: + * 1. Not all memory allocated via the coherent DMA APIs is backed by + *a struct page + * 2. Passing coherent DMA memory into the streaming APIs is not allowed + *as we will try to flush the memory through a different alias to that + *actually being used (and the flushes are redundant.) + */ int dma_get_sgtable_attrs(struct device *dev, struct sg_table *sgt, void *cpu_addr, dma_addr_t dma_addr, size_t size, unsigned long attrs) -- 2.20.1
[PATCH 6/8] arm-nommu: call dma_mmap_from_dev_coherent directly
Ther is no need to go through dma_common_mmap for the arm-nommu dma mmap implementation as the only possible memory not handled above could be that from the per-device coherent pool. Signed-off-by: Christoph Hellwig --- arch/arm/mm/dma-mapping-nommu.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/arch/arm/mm/dma-mapping-nommu.c b/arch/arm/mm/dma-mapping-nommu.c index 52b82559d99b..db9247898300 100644 --- a/arch/arm/mm/dma-mapping-nommu.c +++ b/arch/arm/mm/dma-mapping-nommu.c @@ -68,8 +68,9 @@ static int arm_nommu_dma_mmap(struct device *dev, struct vm_area_struct *vma, if (dma_mmap_from_global_coherent(vma, cpu_addr, size, )) return ret; - - return dma_common_mmap(dev, vma, cpu_addr, dma_addr, size, attrs); + if (dma_mmap_from_dev_coherent(dev, vma, cpu_addr, size, )) + return ret; + return -ENXIO; } -- 2.20.1
[PATCH 5/8] ALSA: pcm: use dma_can_mmap() to check if a device supports dma_mmap_*
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 Reviewed-by: Takashi Iwai --- sound/core/pcm_native.c | 13 ++--- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 703857aab00f..9763c18e176a 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -220,13 +220,12 @@ 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) - return false; -#endif - return true; + + if (substream->ops->mmap || + substream->dma_buffer.dev.type != SNDRV_DMA_TYPE_DEV) + return true; + + return dma_can_mmap(substream->dma_buffer.dev.dev); } static int constrain_mask_params(struct snd_pcm_substream *substream, -- 2.20.1
[PATCH 4/8] dma-mapping: add a dma_can_mmap helper
Add a helper to check if DMA allocations for a specific device can be mapped to userspace using dma_mmap_*. Signed-off-by: Christoph Hellwig --- include/linux/dma-mapping.h | 5 + kernel/dma/mapping.c| 23 +++ 2 files changed, 28 insertions(+) diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h index f7d1eea32c78..17271857be5d 100644 --- a/include/linux/dma-mapping.h +++ b/include/linux/dma-mapping.h @@ -462,6 +462,7 @@ int dma_get_sgtable_attrs(struct device *dev, struct sg_table *sgt, int dma_mmap_attrs(struct device *dev, struct vm_area_struct *vma, void *cpu_addr, dma_addr_t dma_addr, size_t size, unsigned long attrs); +bool dma_can_mmap(struct device *dev); int dma_supported(struct device *dev, u64 mask); int dma_set_mask(struct device *dev, u64 mask); int dma_set_coherent_mask(struct device *dev, u64 mask); @@ -552,6 +553,10 @@ static inline int dma_mmap_attrs(struct device *dev, struct vm_area_struct *vma, { return -ENXIO; } +static inline bool dma_can_mmap(struct device *dev) +{ + return false; +} static inline int dma_supported(struct device *dev, u64 mask) { return 0; diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c index 67b900ad0836..64d1de59e133 100644 --- a/kernel/dma/mapping.c +++ b/kernel/dma/mapping.c @@ -220,6 +220,29 @@ int dma_common_mmap(struct device *dev, struct vm_area_struct *vma, #endif /* !CONFIG_ARCH_NO_COHERENT_DMA_MMAP */ } +/** + * dma_can_mmap - check if a given device supports dma_mmap_* + * @dev: device to check + * + * Returns %true if @dev supports dma_mmap_coherent() and dma_mmap_attrs() to + * map DMA allocations to userspace. + */ +bool dma_can_mmap(struct device *dev) +{ + const struct dma_map_ops *ops = get_dma_ops(dev); + + if (IS_ENABLED(CONFIG_ARCH_NO_COHERENT_DMA_MMAP)) + return false; + + if (dma_is_direct(ops)) { + return dev_is_dma_coherent(dev) || + IS_ENABLED(CONFIG_ARCH_HAS_DMA_COHERENT_TO_PFN); + } + + return ops->mmap != NULL; +} +EXPORT_SYMBOL_GPL(dma_can_mmap); + /** * dma_mmap_attrs - map a coherent DMA allocation into user space * @dev: valid struct device pointer, or NULL for ISA and EISA-like devices -- 2.20.1
[PATCH 3/8] dma-mapping: explicitly wire up ->mmap and ->get_sgtable
While the default ->mmap and ->get_sgtable implementations work for the majority of our dma_map_ops impementations they are inherently safe for others that don't use the page allocator or CMA and/or use their own way of remapping not covered by the common code. So remove the defaults if these methods are not wired up, but instead wire up the default implementations for all safe instances. Fixes: e1c7e324539a ("dma-mapping: always provide the dma_map_ops based implementation") Signed-off-by: Christoph Hellwig --- arch/alpha/kernel/pci_iommu.c | 2 ++ arch/ia64/hp/common/sba_iommu.c | 2 ++ arch/ia64/sn/pci/pci_dma.c | 2 ++ arch/mips/jazz/jazzdma.c| 2 ++ arch/powerpc/kernel/dma-iommu.c | 2 ++ arch/powerpc/platforms/ps3/system-bus.c | 4 arch/powerpc/platforms/pseries/vio.c| 2 ++ arch/s390/pci/pci_dma.c | 2 ++ arch/x86/kernel/amd_gart_64.c | 2 ++ arch/x86/kernel/pci-calgary_64.c| 2 ++ drivers/iommu/amd_iommu.c | 2 ++ drivers/iommu/intel-iommu.c | 2 ++ drivers/parisc/ccio-dma.c | 2 ++ drivers/parisc/sba_iommu.c | 2 ++ kernel/dma/mapping.c| 20 15 files changed, 42 insertions(+), 8 deletions(-) diff --git a/arch/alpha/kernel/pci_iommu.c b/arch/alpha/kernel/pci_iommu.c index 242108439f42..7f1925a32c99 100644 --- a/arch/alpha/kernel/pci_iommu.c +++ b/arch/alpha/kernel/pci_iommu.c @@ -955,5 +955,7 @@ const struct dma_map_ops alpha_pci_ops = { .map_sg = alpha_pci_map_sg, .unmap_sg = alpha_pci_unmap_sg, .dma_supported = alpha_pci_supported, + .mmap = dma_common_mmap, + .get_sgtable= dma_common_get_sgtable, }; EXPORT_SYMBOL(alpha_pci_ops); diff --git a/arch/ia64/hp/common/sba_iommu.c b/arch/ia64/hp/common/sba_iommu.c index 3d24cc43385b..4c0ea6c2833d 100644 --- a/arch/ia64/hp/common/sba_iommu.c +++ b/arch/ia64/hp/common/sba_iommu.c @@ -2183,6 +2183,8 @@ const struct dma_map_ops sba_dma_ops = { .map_sg = sba_map_sg_attrs, .unmap_sg = sba_unmap_sg_attrs, .dma_supported = sba_dma_supported, + .mmap = dma_common_mmap, + .get_sgtable= dma_common_get_sgtable, }; void sba_dma_init(void) diff --git a/arch/ia64/sn/pci/pci_dma.c b/arch/ia64/sn/pci/pci_dma.c index b7d42e4edc1f..12ffb9c0d738 100644 --- a/arch/ia64/sn/pci/pci_dma.c +++ b/arch/ia64/sn/pci/pci_dma.c @@ -438,6 +438,8 @@ static struct dma_map_ops sn_dma_ops = { .unmap_sg = sn_dma_unmap_sg, .dma_supported = sn_dma_supported, .get_required_mask = sn_dma_get_required_mask, + .mmap = dma_common_mmap, + .get_sgtable= dma_common_get_sgtable, }; void sn_dma_init(void) diff --git a/arch/mips/jazz/jazzdma.c b/arch/mips/jazz/jazzdma.c index 1804dc9d8136..a01e14955187 100644 --- a/arch/mips/jazz/jazzdma.c +++ b/arch/mips/jazz/jazzdma.c @@ -682,5 +682,7 @@ const struct dma_map_ops jazz_dma_ops = { .sync_sg_for_device = jazz_dma_sync_sg_for_device, .dma_supported = dma_direct_supported, .cache_sync = arch_dma_cache_sync, + .mmap = dma_common_mmap, + .get_sgtable= dma_common_get_sgtable, }; EXPORT_SYMBOL(jazz_dma_ops); diff --git a/arch/powerpc/kernel/dma-iommu.c b/arch/powerpc/kernel/dma-iommu.c index a0879674a9c8..2f5a53874f6d 100644 --- a/arch/powerpc/kernel/dma-iommu.c +++ b/arch/powerpc/kernel/dma-iommu.c @@ -208,4 +208,6 @@ const struct dma_map_ops dma_iommu_ops = { .sync_single_for_device = dma_iommu_sync_for_device, .sync_sg_for_cpu= dma_iommu_sync_sg_for_cpu, .sync_sg_for_device = dma_iommu_sync_sg_for_device, + .mmap = dma_common_mmap, + .get_sgtable= dma_common_get_sgtable, }; diff --git a/arch/powerpc/platforms/ps3/system-bus.c b/arch/powerpc/platforms/ps3/system-bus.c index 6818317073b9..3542b7bd6a46 100644 --- a/arch/powerpc/platforms/ps3/system-bus.c +++ b/arch/powerpc/platforms/ps3/system-bus.c @@ -694,6 +694,8 @@ static const struct dma_map_ops ps3_sb_dma_ops = { .dma_supported = ps3_dma_supported, .map_page = ps3_sb_map_page, .unmap_page = ps3_unmap_page, + .mmap = dma_common_mmap, + .get_sgtable = dma_common_get_sgtable, }; static const struct dma_map_ops ps3_ioc0_dma_ops = { @@ -704,6 +706,8 @@ static const struct dma_map_ops ps3_ioc0_dma_ops = { .dma_supported = ps3_dma_supported, .map_page = ps3_ioc0_map_page, .unmap_page = ps3_unmap_page, + .mmap = dma_common_mmap, + .get_sgtable = dma_common_get_sgtable, }; /** diff --git a/arch/powerpc/platforms/pseries/vio.c
remove default fallbacks in dma_map_ops v3
Hi all, we have a few places where the DMA mapping layer has non-trivial default actions that are questionable and/or dangerous. This series instead wires up the mmap, get_sgtable and get_required_mask methods explicitly and cleans up some surrounding areas. This also means we could get rid of the ARCH_NO_COHERENT_DMA_MMAP kconfig option, as we now require a mmap method wired up, or in case of non-coherent dma-direct the presence of the arch_dma_coherent_to_pfn hook. The only interesting case is that the sound code also checked the ARCH_NO_COHERENT_DMA_MMAP symbol in somewhat odd ways, so I'd like to see a review of the sound situation before going forward with that patch. Changes since v2: - fix use of dma_can_mmap in alsa - improve the CONFIG_* mess a little more Changes since v1: - add a dma_can_mmap helper for alsa
[PATCH 1/8] dma-mapping: provide a better default ->get_required_mask
Most dma_map_ops instances are IOMMUs that work perfectly fine in 32-bits of IOVA space, and the generic direct mapping code already provides its own routines that is intelligent based on the amount of memory actually present. Wire up the dma-direct routine for the ARM direct mapping code as well, and otherwise default to the constant 32-bit mask. This way we only need to override it for the occasional odd IOMMU that requires 64-bit IOVA support, or IOMMU drivers that are more efficient if they can fall back to the direct mapping. Signed-off-by: Christoph Hellwig --- arch/arm/mm/dma-mapping.c | 3 +++ arch/powerpc/platforms/ps3/system-bus.c | 7 -- arch/x86/kernel/amd_gart_64.c | 1 + kernel/dma/mapping.c| 30 + 4 files changed, 14 insertions(+), 27 deletions(-) diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index 8c0b0baeb398..ad64d32fb39a 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -192,6 +193,7 @@ const struct dma_map_ops arm_dma_ops = { .sync_sg_for_cpu= arm_dma_sync_sg_for_cpu, .sync_sg_for_device = arm_dma_sync_sg_for_device, .dma_supported = arm_dma_supported, + .get_required_mask = dma_direct_get_required_mask, }; EXPORT_SYMBOL(arm_dma_ops); @@ -212,6 +214,7 @@ const struct dma_map_ops arm_coherent_dma_ops = { .map_sg = arm_dma_map_sg, .map_resource = dma_direct_map_resource, .dma_supported = arm_dma_supported, + .get_required_mask = dma_direct_get_required_mask, }; EXPORT_SYMBOL(arm_coherent_dma_ops); diff --git a/arch/powerpc/platforms/ps3/system-bus.c b/arch/powerpc/platforms/ps3/system-bus.c index 98410119c47b..6818317073b9 100644 --- a/arch/powerpc/platforms/ps3/system-bus.c +++ b/arch/powerpc/platforms/ps3/system-bus.c @@ -686,18 +686,12 @@ static int ps3_dma_supported(struct device *_dev, u64 mask) return mask >= DMA_BIT_MASK(32); } -static u64 ps3_dma_get_required_mask(struct device *_dev) -{ - return DMA_BIT_MASK(32); -} - static const struct dma_map_ops ps3_sb_dma_ops = { .alloc = ps3_alloc_coherent, .free = ps3_free_coherent, .map_sg = ps3_sb_map_sg, .unmap_sg = ps3_sb_unmap_sg, .dma_supported = ps3_dma_supported, - .get_required_mask = ps3_dma_get_required_mask, .map_page = ps3_sb_map_page, .unmap_page = ps3_unmap_page, }; @@ -708,7 +702,6 @@ static const struct dma_map_ops ps3_ioc0_dma_ops = { .map_sg = ps3_ioc0_map_sg, .unmap_sg = ps3_ioc0_unmap_sg, .dma_supported = ps3_dma_supported, - .get_required_mask = ps3_dma_get_required_mask, .map_page = ps3_ioc0_map_page, .unmap_page = ps3_unmap_page, }; diff --git a/arch/x86/kernel/amd_gart_64.c b/arch/x86/kernel/amd_gart_64.c index a585ea6f686a..03ed9675f954 100644 --- a/arch/x86/kernel/amd_gart_64.c +++ b/arch/x86/kernel/amd_gart_64.c @@ -678,6 +678,7 @@ static const struct dma_map_ops gart_dma_ops = { .alloc = gart_alloc_coherent, .free = gart_free_coherent, .dma_supported = dma_direct_supported, + .get_required_mask = dma_direct_get_required_mask, }; static void gart_iommu_shutdown(void) diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c index b0038ca3aa92..9c0f6a8eb5cb 100644 --- a/kernel/dma/mapping.c +++ b/kernel/dma/mapping.c @@ -233,25 +233,6 @@ int dma_mmap_attrs(struct device *dev, struct vm_area_struct *vma, } EXPORT_SYMBOL(dma_mmap_attrs); -static u64 dma_default_get_required_mask(struct device *dev) -{ - u32 low_totalram = ((max_pfn - 1) << PAGE_SHIFT); - u32 high_totalram = ((max_pfn - 1) >> (32 - PAGE_SHIFT)); - u64 mask; - - if (!high_totalram) { - /* convert to mask just covering totalram */ - low_totalram = (1 << (fls(low_totalram) - 1)); - low_totalram += low_totalram - 1; - mask = low_totalram; - } else { - high_totalram = (1 << (fls(high_totalram) - 1)); - high_totalram += high_totalram - 1; - mask = (((u64)high_totalram) << 32) + 0x; - } - return mask; -} - u64 dma_get_required_mask(struct device *dev) { const struct dma_map_ops *ops = get_dma_ops(dev); @@ -260,7 +241,16 @@ u64 dma_get_required_mask(struct device *dev) return dma_direct_get_required_mask(dev); if (ops->get_required_mask) return ops->get_required_mask(dev); - return dma_default_get_required_mask(dev); + + /* +* We require every DMA ops implementation to at least support a 32-bit +* DMA mask (and use bounce buffering if that
Re: [PATCH 3/8] of/fdt: add function to get the SoC wide DMA addressable memory size
On Tue, Aug 6, 2019 at 12:12 PM Nicolas Saenz Julienne wrote: > > Hi Rob, > > On Mon, 2019-08-05 at 13:23 -0600, Rob Herring wrote: > > On Mon, Aug 5, 2019 at 10:03 AM Nicolas Saenz Julienne > > wrote: > > > Hi Rob, > > > Thanks for the review! > > > > > > On Fri, 2019-08-02 at 11:17 -0600, Rob Herring wrote: > > > > On Wed, Jul 31, 2019 at 9:48 AM Nicolas Saenz Julienne > > > > wrote: > > > > > Some SoCs might have multiple interconnects each with their own DMA > > > > > addressing limitations. This function parses the 'dma-ranges' on each > > > > > of > > > > > them and tries to guess the maximum SoC wide DMA addressable memory > > > > > size. > > > > > > > > > > This is specially useful for arch code in order to properly setup CMA > > > > > and memory zones. > > > > > > > > We already have a way to setup CMA in reserved-memory, so why is this > > > > needed for that? > > > > > > Correct me if I'm wrong but I got the feeling you got the point of the > > > patch > > > later on. > > > > No, for CMA I don't. Can't we already pass a size and location for CMA > > region under /reserved-memory. The only advantage here is perhaps the > > CMA range could be anywhere in the DMA zone vs. a fixed location. > > Now I get it, sorry I wasn't aware of that interface. > > Still, I'm not convinced it matches RPi's use case as this would hard-code > CMA's size. Most people won't care, but for the ones that do, it's nicer to > change the value from the kernel command line than editing the dtb. Sure, I fully agree and am not a fan of the CMA DT overlays I've seen. > I get that > if you need to, for example, reserve some memory for the video to work, it's > silly not to hard-code it. Yet due to the board's nature and users base I say > it's important to favor flexibility. It would also break compatibility with > earlier versions of the board and diverge from the downstream kernel > behaviour. > Which is a bigger issue than it seems as most users don't always understand > which kernel they are running and unknowingly copy configuration options from > forums. > > As I also need to know the DMA addressing limitations to properly configure > memory zones and dma-direct. Setting up the proper CMA constraints during the > arch's init will be trivial anyway. It was really just commentary on commit text as for CMA alone we have a solution already. I agree on the need for zones. > > > > > IMO, I'd just do: > > > > > > > > if (of_fdt_machine_is_compatible(blob, "brcm,bcm2711")) > > > > dma_zone_size = XX; > > > > > > > > 2 lines of code is much easier to maintain than 10s of incomplete code > > > > and is clearer who needs this. Maybe if we have dozens of SoCs with > > > > this problem we should start parsing dma-ranges. > > > > > > FYI that's what arm32 is doing at the moment and was my first instinct. > > > But > > > it > > > seems that arm64 has been able to survive so far without any machine > > > specific > > > code and I have the feeling Catalin and Will will not be happy about this > > > solution. Am I wrong? > > > > No doubt. I'm fine if the 2 lines live in drivers/of/. > > > > Note that I'm trying to reduce the number of early_init_dt_scan_* > > calls from arch code into the DT code so there's more commonality > > across architectures in the early DT scans. So ideally, this can all > > be handled under early_init_dt_scan() call. > > How does this look? (I'll split it in two patches and add a comment explaining > why dt_dma_zone_size is needed) > > diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c > index f2444c61a136..1395be40b722 100644 > --- a/drivers/of/fdt.c > +++ b/drivers/of/fdt.c > @@ -30,6 +30,8 @@ > > #include "of_private.h" > > +u64 dt_dma_zone_size __ro_after_init; Avoiding a call from arch code by just having a variable isn't really better. I'd rather see a common, non DT specific variable that can be adjusted. Something similar to initrd_start/end. Then the arch code doesn't have to care what hardware description code adjusted the value. Rob ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 1/4] firmware: qcom_scm-64: Add atomic version of qcom_scm_call
On Tue, Aug 6, 2019 at 3:58 AM Bjorn Andersson wrote: > > On Wed 19 Jun 04:34 PDT 2019, Vivek Gautam wrote: > > > On Tue, Jun 18, 2019 at 11:25 PM Will Deacon wrote: > > > > > > On Wed, Jun 12, 2019 at 12:45:51PM +0530, Vivek Gautam wrote: > > > > There are scnenarios where drivers are required to make a > > > > scm call in atomic context, such as in one of the qcom's > > > > arm-smmu-500 errata [1]. > > > > > > > > [1] ("https://source.codeaurora.org/quic/la/kernel/msm-4.9/commit/ > > > > drivers/iommu/arm-smmu.c?h=CogSystems-msm-49/ > > > > msm-4.9=da765c6c75266b38191b38ef086274943f353ea7") > > > > > > > > Signed-off-by: Vivek Gautam > > > > Reviewed-by: Bjorn Andersson > > > > --- > > > > drivers/firmware/qcom_scm-64.c | 136 > > > > - > > > > 1 file changed, 92 insertions(+), 44 deletions(-) > > > > > > > > diff --git a/drivers/firmware/qcom_scm-64.c > > > > b/drivers/firmware/qcom_scm-64.c > > > > index 91d5ad7cf58b..b6dca32c5ac4 100644 > > > > --- a/drivers/firmware/qcom_scm-64.c > > > > +++ b/drivers/firmware/qcom_scm-64.c > > > > [snip] > > > > > > + > > > > +static void qcom_scm_call_do(const struct qcom_scm_desc *desc, > > > > + struct arm_smccc_res *res, u32 fn_id, > > > > + u64 x5, bool atomic) > > > > +{ > > > > > > Maybe pass in the call type (ARM_SMCCC_FAST_CALL vs ARM_SMCCC_STD_CALL) > > > instead of "bool atomic"? Would certainly make the callsites easier to > > > understand. > > > > Sure, will do that. > > > > > > > > > + int retry_count = 0; > > > > + > > > > + if (!atomic) { > > > > + do { > > > > + mutex_lock(_scm_lock); > > > > + > > > > + __qcom_scm_call_do(desc, res, fn_id, x5, > > > > +ARM_SMCCC_STD_CALL); > > > > + > > > > + mutex_unlock(_scm_lock); > > > > + > > > > + if (res->a0 == QCOM_SCM_V2_EBUSY) { > > > > + if (retry_count++ > > > > > QCOM_SCM_EBUSY_MAX_RETRY) > > > > + break; > > > > + msleep(QCOM_SCM_EBUSY_WAIT_MS); > > > > + } > > > > + } while (res->a0 == QCOM_SCM_V2_EBUSY); > > > > + } else { > > > > + __qcom_scm_call_do(desc, res, fn_id, x5, > > > > ARM_SMCCC_FAST_CALL); > > > > + } > > > > > > Is it safe to make concurrent FAST calls? > > > > I better add a spinlock here. > > > > Hi Vivek, > > Would you be able to respin this patch, so that we could unblock the > introduction of the display nodes in the various device? Will pointed [1] to the restructuring of arm-smmu to support implementation specific details. That hasn't been posted yet, and I haven't yet been able to work on that either. I will be happy to respin this series with the comments addressed if Will is okay to pull changes to unblock sdm845 devices. :) [1] https://lore.kernel.org/patchwork/patch/1087457/ Thanks & Regards Vivek > > Regards, > Bjorn > ___ > iommu mailing list > iommu@lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/iommu -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH v3 hmm 11/11] mm/mmu_notifiers: remove unregister_no_release
On Tue, Aug 06, 2019 at 08:15:48PM -0300, Jason Gunthorpe wrote: > From: Jason Gunthorpe > > mmu_notifier_unregister_no_release() and mmu_notifier_call_srcu() no > longer have any users, they have all been converted to use > mmu_notifier_put(). > > So delete this difficult to use interface. > > Signed-off-by: Jason Gunthorpe Looks good: Reviewed-by: Christoph Hellwig ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 hmm 05/11] hmm: use mmu_notifier_get/put for 'struct hmm'
Looks good, Reviewed-by: Christoph Hellwig ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 hmm 02/11] mm/mmu_notifiers: do not speculatively allocate a mmu_notifier_mm
On Tue, Aug 06, 2019 at 08:15:39PM -0300, Jason Gunthorpe wrote: > From: Jason Gunthorpe > > A prior commit e0f3c3f78da2 ("mm/mmu_notifier: init notifier if necessary") > made an attempt at doing this, but had to be reverted as calling > the GFP_KERNEL allocator under the i_mmap_mutex causes deadlock, see > commit 35cfa2b0b491 ("mm/mmu_notifier: allocate mmu_notifier in advance"). > > However, we can avoid that problem by doing the allocation only under > the mmap_sem, which is already happening. > > Since all writers to mm->mmu_notifier_mm hold the write side of the > mmap_sem reading it under that sem is deterministic and we can use that to > decide if the allocation path is required, without speculation. > > The actual update to mmu_notifier_mm must still be done under the > mm_take_all_locks() to ensure read-side coherency. > > Signed-off-by: Jason Gunthorpe Looks good, Reviewed-by: Christoph Hellwig ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 hmm 04/11] misc/sgi-gru: use mmu_notifier_get/put for struct gru_mm_struct
Looks good, Reviewed-by: Christoph Hellwig ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 hmm 01/11] mm/mmu_notifiers: hoist do_mmu_notifier_register down_write to the caller
On Tue, Aug 06, 2019 at 08:15:38PM -0300, Jason Gunthorpe wrote: > From: Jason Gunthorpe > > This simplifies the code to not have so many one line functions and extra > logic. __mmu_notifier_register() simply becomes the entry point to > register the notifier, and the other one calls it under lock. > > Also add a lockdep_assert to check that the callers are holding the lock > as expected. > > Suggested-by: Christoph Hellwig > Signed-off-by: Jason Gunthorpe Looks good: Reviewed-by: Christoph Hellwig ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu/amd: Override wrong IVRS IOAPIC on Raven Ridge systems
Raven Ridge systems may have malfunction touchpad or hang at boot if incorrect IVRS IOAPIC is provided by BIOS. Users already found correct "ivrs_ioapic=" values, let's put them inside kernel to workaround buggy BIOS. BugLink: https://bugs.launchpad.net/bugs/1795292 BugLink: https://bugs.launchpad.net/bugs/1837688 Signed-off-by: Kai-Heng Feng --- drivers/iommu/amd_iommu_init.c | 75 ++ 1 file changed, 75 insertions(+) diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c index 4413aa67000e..06fd008281e5 100644 --- a/drivers/iommu/amd_iommu_init.c +++ b/drivers/iommu/amd_iommu_init.c @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -1109,6 +1110,78 @@ static int __init add_early_maps(void) return 0; } +struct quirk_entry { + u8 id; + u16 devid; +}; + +enum { + DELL_INSPIRON_7375 = 0, + DELL_LATITUDE_5495, + LENOVO_IDEAPAD_330S_15ARR, +}; + +static const struct quirk_entry ivrs_ioapic_quirks[][3] __initconst = { + /* ivrs_ioapic[4]=00:14.0 ivrs_ioapic[5]=00:00.2 */ + [DELL_INSPIRON_7375] = { + { .id = 4, .devid = 0xa0 }, + { .id = 5, .devid = 0x2 }, + {} + }, + /* ivrs_ioapic[4]=00:14.0 */ + [DELL_LATITUDE_5495] = { + { .id = 4, .devid = 0xa0 }, + {} + }, + /* ivrs_ioapic[32]=00:14.0 */ + [LENOVO_IDEAPAD_330S_15ARR] = { + { .id = 32, .devid = 0xa0 }, + {} + }, + {} +}; + +static int __init ivrs_ioapic_quirk_cb(const struct dmi_system_id *d) +{ + const struct quirk_entry *i; + + for (i = d->driver_data; i->id != 0 && i->devid != 0; i++) + add_special_device(IVHD_SPECIAL_IOAPIC, i->id, >devid, 0); + + return 0; +} + +static const struct dmi_system_id ivrs_quirks[] __initconst = { + { + .callback = ivrs_ioapic_quirk_cb, + .ident = "Dell Inspiron 7375", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."), + DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 7375"), + }, + .driver_data = (void *)_ioapic_quirks[DELL_INSPIRON_7375], + }, + { + .callback = ivrs_ioapic_quirk_cb, + .ident = "Dell Latitude 5495", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."), + DMI_MATCH(DMI_PRODUCT_NAME, "Latitude 5495"), + }, + .driver_data = (void *)_ioapic_quirks[DELL_LATITUDE_5495], + }, + { + .callback = ivrs_ioapic_quirk_cb, + .ident = "Lenovo ideapad 330S-15ARR", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_NAME, "81FB"), + }, + .driver_data = (void *)_ioapic_quirks[LENOVO_IDEAPAD_330S_15ARR], + }, + {} +}; + /* * Reads the device exclusion range from ACPI and initializes the IOMMU with * it @@ -1153,6 +1226,8 @@ static int __init init_iommu_from_acpi(struct amd_iommu *iommu, if (ret) return ret; + dmi_check_system(ivrs_quirks); + /* * First save the recommended feature enable bits from ACPI */ -- 2.17.1
Re: Regression due to d98849aff879 (dma-direct: handle DMA_ATTR_NO_KERNEL_MAPPING in common code)
On Wed, Aug 07, 2019 at 05:24:17PM +0200, Lucas Stach wrote: > I would suggest to place this line above the comment, as the comment > only really applies to the return value. Other than this nitpick, this > matches my understanding of the required changes, so: > > Reviewed-by: Lucas Stach Thanks, I've applied it with that fixed up. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu