Re: [PATCH 2/3] iommu/vt-d: Apply per-device dma_ops
Hi Lu, I really do like the switch to the per-device dma_map_ops, but: On Thu, Aug 01, 2019 at 02:01:55PM +0800, Lu Baolu wrote: > Current Intel IOMMU driver sets the system level dma_ops. This > implementation has at least the following drawbacks: 1) each > dma API will go through the IOMMU driver even the devices are > using identity mapped domains; 2) if user requests to use an > identity mapped domain (a.k.a. bypass iommu translation), the > driver might fall back to dma domain blindly if the device is > not able to address all system memory. This is very clearly a behavioral regression. The intel-iommu driver has always used the iommu mapping to provide decent support for devices that do not have the full 64-bit addressing capability, and changing this will make a lot of existing setups go slower. I don't think having to use swiotlb for these devices helps anyone.
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, 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.
Re: [PATCH] dma-direct: don't truncate dma_required_mask to bus addressing capabilities
On Mon, Aug 05, 2019 at 05:51:53PM +0200, Lucas Stach wrote: > The dma required_mask needs to reflect the actual addressing capabilities > needed to handle the whole system RAM. When truncated down to the bus > addressing capabilities dma_addressing_limited() will incorrectly signal > no limitations for devices which are restricted by the bus_dma_mask. > > Fixes: b4ebe6063204 (dma-direct: implement complete bus_dma_mask handling) > Signed-off-by: Lucas Stach Yeah, this looks sensible. Atish, can you check if this helps on the HiFive board as well? ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 5/6] fs/core/vmcore: Move sev_active() reference to x86 arch code
Secure Encrypted Virtualization is an x86-specific feature, so it shouldn't appear in generic kernel code because it forces non-x86 architectures to define the sev_active() function, which doesn't make a lot of sense. To solve this problem, add an x86 elfcorehdr_read() function to override the generic weak implementation. To do that, it's necessary to make read_from_oldmem() public so that it can be used outside of vmcore.c. Also, remove the export for sev_active() since it's only used in files that won't be built as modules. Signed-off-by: Thiago Jung Bauermann Reviewed-by: Christoph Hellwig Reviewed-by: Lianbo Jiang --- arch/x86/kernel/crash_dump_64.c | 5 + arch/x86/mm/mem_encrypt.c | 1 - fs/proc/vmcore.c| 8 include/linux/crash_dump.h | 14 ++ include/linux/mem_encrypt.h | 1 - 5 files changed, 23 insertions(+), 6 deletions(-) diff --git a/arch/x86/kernel/crash_dump_64.c b/arch/x86/kernel/crash_dump_64.c index 22369dd5de3b..045e82e8945b 100644 --- a/arch/x86/kernel/crash_dump_64.c +++ b/arch/x86/kernel/crash_dump_64.c @@ -70,3 +70,8 @@ ssize_t copy_oldmem_page_encrypted(unsigned long pfn, char *buf, size_t csize, { return __copy_oldmem_page(pfn, buf, csize, offset, userbuf, true); } + +ssize_t elfcorehdr_read(char *buf, size_t count, u64 *ppos) +{ + return read_from_oldmem(buf, count, ppos, 0, sev_active()); +} diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c index 94da5a88abe6..9268c12458c8 100644 --- a/arch/x86/mm/mem_encrypt.c +++ b/arch/x86/mm/mem_encrypt.c @@ -349,7 +349,6 @@ bool sev_active(void) { return sme_me_mask && sev_enabled; } -EXPORT_SYMBOL(sev_active); /* Override for DMA direct allocation check - ARCH_HAS_FORCE_DMA_UNENCRYPTED */ bool force_dma_unencrypted(struct device *dev) diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c index 7bcc92add72c..7b13988796e1 100644 --- a/fs/proc/vmcore.c +++ b/fs/proc/vmcore.c @@ -104,9 +104,9 @@ static int pfn_is_ram(unsigned long pfn) } /* Reads a page from the oldmem device from given offset. */ -static ssize_t read_from_oldmem(char *buf, size_t count, - u64 *ppos, int userbuf, - bool encrypted) +ssize_t read_from_oldmem(char *buf, size_t count, +u64 *ppos, int userbuf, +bool encrypted) { unsigned long pfn, offset; size_t nr_bytes; @@ -170,7 +170,7 @@ void __weak elfcorehdr_free(unsigned long long addr) */ ssize_t __weak elfcorehdr_read(char *buf, size_t count, u64 *ppos) { - return read_from_oldmem(buf, count, ppos, 0, sev_active()); + return read_from_oldmem(buf, count, ppos, 0, false); } /* diff --git a/include/linux/crash_dump.h b/include/linux/crash_dump.h index f774c5eb9e3c..4664fc1871de 100644 --- a/include/linux/crash_dump.h +++ b/include/linux/crash_dump.h @@ -115,4 +115,18 @@ static inline int vmcore_add_device_dump(struct vmcoredd_data *data) return -EOPNOTSUPP; } #endif /* CONFIG_PROC_VMCORE_DEVICE_DUMP */ + +#ifdef CONFIG_PROC_VMCORE +ssize_t read_from_oldmem(char *buf, size_t count, +u64 *ppos, int userbuf, +bool encrypted); +#else +static inline ssize_t read_from_oldmem(char *buf, size_t count, + u64 *ppos, int userbuf, + bool encrypted) +{ + return -EOPNOTSUPP; +} +#endif /* CONFIG_PROC_VMCORE */ + #endif /* LINUX_CRASHDUMP_H */ diff --git a/include/linux/mem_encrypt.h b/include/linux/mem_encrypt.h index 0c5b0ff9eb29..5c4a18a91f89 100644 --- a/include/linux/mem_encrypt.h +++ b/include/linux/mem_encrypt.h @@ -19,7 +19,6 @@ #else /* !CONFIG_ARCH_HAS_MEM_ENCRYPT */ static inline bool mem_encrypt_active(void) { return false; } -static inline bool sev_active(void) { return false; } #endif /* CONFIG_ARCH_HAS_MEM_ENCRYPT */ ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 3/6] dma-mapping: Remove dma_check_mask()
sme_active() is an x86-specific function so it's better not to call it from generic code. Christoph Hellwig mentioned that "There is no reason why we should have a special debug printk just for one specific reason why there is a requirement for a large DMA mask.", so just remove dma_check_mask(). Signed-off-by: Thiago Jung Bauermann Reviewed-by: Christoph Hellwig Reviewed-by: Tom Lendacky --- kernel/dma/mapping.c | 8 1 file changed, 8 deletions(-) diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c index 1f628e7ac709..61eeefbfcb36 100644 --- a/kernel/dma/mapping.c +++ b/kernel/dma/mapping.c @@ -291,12 +291,6 @@ void dma_free_attrs(struct device *dev, size_t size, void *cpu_addr, } EXPORT_SYMBOL(dma_free_attrs); -static inline void dma_check_mask(struct device *dev, u64 mask) -{ - if (sme_active() && (mask < (((u64)sme_get_me_mask() << 1) - 1))) - dev_warn(dev, "SME is active, device will require DMA bounce buffers\n"); -} - int dma_supported(struct device *dev, u64 mask) { const struct dma_map_ops *ops = get_dma_ops(dev); @@ -327,7 +321,6 @@ int dma_set_mask(struct device *dev, u64 mask) return -EIO; arch_dma_set_mask(dev, mask); - dma_check_mask(dev, mask); *dev->dma_mask = mask; return 0; } @@ -345,7 +338,6 @@ int dma_set_coherent_mask(struct device *dev, u64 mask) if (!dma_supported(dev, mask)) return -EIO; - dma_check_mask(dev, mask); dev->coherent_dma_mask = mask; return 0; }
[PATCH v4 6/6] s390/mm: Remove sev_active() function
All references to sev_active() were moved to arch/x86 so we don't need to define it for s390 anymore. Signed-off-by: Thiago Jung Bauermann Reviewed-by: Christoph Hellwig Reviewed-by: Halil Pasic --- arch/s390/include/asm/mem_encrypt.h | 1 - arch/s390/mm/init.c | 7 +-- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/arch/s390/include/asm/mem_encrypt.h b/arch/s390/include/asm/mem_encrypt.h index ff813a56bc30..2542cbf7e2d1 100644 --- a/arch/s390/include/asm/mem_encrypt.h +++ b/arch/s390/include/asm/mem_encrypt.h @@ -5,7 +5,6 @@ #ifndef __ASSEMBLY__ static inline bool mem_encrypt_active(void) { return false; } -extern bool sev_active(void); int set_memory_encrypted(unsigned long addr, int numpages); int set_memory_decrypted(unsigned long addr, int numpages); diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c index 20340a03ad90..a124f19f7b3c 100644 --- a/arch/s390/mm/init.c +++ b/arch/s390/mm/init.c @@ -156,14 +156,9 @@ int set_memory_decrypted(unsigned long addr, int numpages) } /* are we a protected virtualization guest? */ -bool sev_active(void) -{ - return is_prot_virt_guest(); -} - bool force_dma_unencrypted(struct device *dev) { - return sev_active(); + return is_prot_virt_guest(); } /* protected virtualization */
[PATCH v4 2/6] swiotlb: Remove call to sme_active()
sme_active() is an x86-specific function so it's better not to call it from generic code. There's no need to mention which memory encryption feature is active, so just use a more generic message. Besides, other architectures will have different names for similar technology. Signed-off-by: Thiago Jung Bauermann Reviewed-by: Christoph Hellwig Reviewed-by: Tom Lendacky --- kernel/dma/swiotlb.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c index 9de232229063..f29caad71e13 100644 --- a/kernel/dma/swiotlb.c +++ b/kernel/dma/swiotlb.c @@ -461,8 +461,7 @@ phys_addr_t swiotlb_tbl_map_single(struct device *hwdev, panic("Can not allocate SWIOTLB buffer earlier and can't now provide you with the DMA bounce buffer"); if (mem_encrypt_active()) - pr_warn_once("%s is active and system is using DMA bounce buffers\n", -sme_active() ? "SME" : "SEV"); + pr_warn_once("Memory encryption is active and system is using DMA bounce buffers\n"); mask = dma_get_seg_boundary(hwdev);
[PATCH v4 1/6] x86,s390: Move ARCH_HAS_MEM_ENCRYPT definition to arch/Kconfig
powerpc is also going to use this feature, so put it in a generic location. Signed-off-by: Thiago Jung Bauermann Reviewed-by: Thomas Gleixner Reviewed-by: Christoph Hellwig --- arch/Kconfig | 3 +++ arch/s390/Kconfig | 4 +--- arch/x86/Kconfig | 4 +--- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/arch/Kconfig b/arch/Kconfig index a7b57dd42c26..89e2e3f64f79 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -925,6 +925,9 @@ config LOCK_EVENT_COUNTS the chance of application behavior change because of timing differences. The counts are reported via debugfs. +config ARCH_HAS_MEM_ENCRYPT + bool + source "kernel/gcov/Kconfig" source "scripts/gcc-plugins/Kconfig" diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig index a4ad2733eedf..f43319c44454 100644 --- a/arch/s390/Kconfig +++ b/arch/s390/Kconfig @@ -1,7 +1,4 @@ # SPDX-License-Identifier: GPL-2.0 -config ARCH_HAS_MEM_ENCRYPT -def_bool y - config MMU def_bool y @@ -68,6 +65,7 @@ config S390 select ARCH_HAS_GCOV_PROFILE_ALL select ARCH_HAS_GIGANTIC_PAGE select ARCH_HAS_KCOV + select ARCH_HAS_MEM_ENCRYPT select ARCH_HAS_PTE_SPECIAL select ARCH_HAS_SET_MEMORY select ARCH_HAS_STRICT_KERNEL_RWX diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index 222855cc0158..06027809c599 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -68,6 +68,7 @@ config X86 select ARCH_HAS_FORTIFY_SOURCE select ARCH_HAS_GCOV_PROFILE_ALL select ARCH_HAS_KCOVif X86_64 + select ARCH_HAS_MEM_ENCRYPT select ARCH_HAS_MEMBARRIER_SYNC_CORE select ARCH_HAS_PMEM_APIif X86_64 select ARCH_HAS_PTE_DEVMAP if X86_64 @@ -1518,9 +1519,6 @@ config X86_CPA_STATISTICS helps to determine the effectiveness of preserving large and huge page mappings when mapping protections are changed. -config ARCH_HAS_MEM_ENCRYPT - def_bool y - config AMD_MEM_ENCRYPT bool "AMD Secure Memory Encryption (SME) support" depends on X86_64 && CPU_SUP_AMD
[PATCH v4 4/6] x86,s390/mm: Move sme_active() and sme_me_mask to x86-specific header
Now that generic code doesn't reference them, move sme_active() and sme_me_mask to x86's . Also remove the export for sme_active() since it's only used in files that won't be built as modules. sme_me_mask on the other hand is used in arch/x86/kvm/svm.c (via __sme_set() and __psp_pa()) which can be built as a module so its export needs to stay. Signed-off-by: Thiago Jung Bauermann Reviewed-by: Christoph Hellwig Reviewed-by: Tom Lendacky --- arch/s390/include/asm/mem_encrypt.h | 4 +--- arch/x86/include/asm/mem_encrypt.h | 10 ++ arch/x86/mm/mem_encrypt.c | 1 - include/linux/mem_encrypt.h | 14 +- 4 files changed, 12 insertions(+), 17 deletions(-) diff --git a/arch/s390/include/asm/mem_encrypt.h b/arch/s390/include/asm/mem_encrypt.h index 3eb018508190..ff813a56bc30 100644 --- a/arch/s390/include/asm/mem_encrypt.h +++ b/arch/s390/include/asm/mem_encrypt.h @@ -4,9 +4,7 @@ #ifndef __ASSEMBLY__ -#define sme_me_mask0ULL - -static inline bool sme_active(void) { return false; } +static inline bool mem_encrypt_active(void) { return false; } extern bool sev_active(void); int set_memory_encrypted(unsigned long addr, int numpages); diff --git a/arch/x86/include/asm/mem_encrypt.h b/arch/x86/include/asm/mem_encrypt.h index 0c196c47d621..848ce43b9040 100644 --- a/arch/x86/include/asm/mem_encrypt.h +++ b/arch/x86/include/asm/mem_encrypt.h @@ -92,6 +92,16 @@ early_set_memory_encrypted(unsigned long vaddr, unsigned long size) { return 0; extern char __start_bss_decrypted[], __end_bss_decrypted[], __start_bss_decrypted_unused[]; +static inline bool mem_encrypt_active(void) +{ + return sme_me_mask; +} + +static inline u64 sme_get_me_mask(void) +{ + return sme_me_mask; +} + #endif /* __ASSEMBLY__ */ #endif /* __X86_MEM_ENCRYPT_H__ */ diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c index fece30ca8b0c..94da5a88abe6 100644 --- a/arch/x86/mm/mem_encrypt.c +++ b/arch/x86/mm/mem_encrypt.c @@ -344,7 +344,6 @@ bool sme_active(void) { return sme_me_mask && !sev_enabled; } -EXPORT_SYMBOL(sme_active); bool sev_active(void) { diff --git a/include/linux/mem_encrypt.h b/include/linux/mem_encrypt.h index 470bd53a89df..0c5b0ff9eb29 100644 --- a/include/linux/mem_encrypt.h +++ b/include/linux/mem_encrypt.h @@ -18,23 +18,11 @@ #else /* !CONFIG_ARCH_HAS_MEM_ENCRYPT */ -#define sme_me_mask0ULL - -static inline bool sme_active(void) { return false; } +static inline bool mem_encrypt_active(void) { return false; } static inline bool sev_active(void) { return false; } #endif /* CONFIG_ARCH_HAS_MEM_ENCRYPT */ -static inline bool mem_encrypt_active(void) -{ - return sme_me_mask; -} - -static inline u64 sme_get_me_mask(void) -{ - return sme_me_mask; -} - #ifdef CONFIG_AMD_MEM_ENCRYPT /* * The __sme_set() and __sme_clr() macros are useful for adding or removing
[PATCH v4 0/6] Remove x86-specific code from generic headers
Hello, This version has only a small change in the last patch as requested by Christoph and Halil, and collects Reviewed-by's. These patches are applied on top of v5.3-rc2. I don't have a way to test SME, SEV, nor s390's PEF so the patches have only been build tested. Changelog Since v3: - Patch "s390/mm: Remove sev_active() function" - Preserve comment from sev_active() in force_dma_unencrypted(). Suggested by Christoph Hellwig. Since v2: - Patch "x86,s390: Move ARCH_HAS_MEM_ENCRYPT definition to arch/Kconfig" - Added "select ARCH_HAS_MEM_ENCRYPT" to config S390. Suggested by Janani. - Patch "DMA mapping: Move SME handling to x86-specific files" - Split up into 3 new patches. Suggested by Christoph Hellwig. - Patch "swiotlb: Remove call to sme_active()" - New patch. - Patch "dma-mapping: Remove dma_check_mask()" - New patch. - Patch "x86,s390/mm: Move sme_active() and sme_me_mask to x86-specific header" - New patch. - Removed export of sme_active symbol. Suggested by Christoph Hellwig. - Patch "fs/core/vmcore: Move sev_active() reference to x86 arch code" - Removed export of sev_active symbol. Suggested by Christoph Hellwig. - Patch "s390/mm: Remove sev_active() function" - New patch. Since v1: - Patch "x86,s390: Move ARCH_HAS_MEM_ENCRYPT definition to arch/Kconfig" - Remove definition of ARCH_HAS_MEM_ENCRYPT from s390/Kconfig as well. - Reworded patch title and message a little bit. - Patch "DMA mapping: Move SME handling to x86-specific files" - Adapt s390's as well. - Remove dma_check_mask() from kernel/dma/mapping.c. Suggested by Christoph Hellwig. Thiago Jung Bauermann (6): x86,s390: Move ARCH_HAS_MEM_ENCRYPT definition to arch/Kconfig swiotlb: Remove call to sme_active() dma-mapping: Remove dma_check_mask() x86,s390/mm: Move sme_active() and sme_me_mask to x86-specific header fs/core/vmcore: Move sev_active() reference to x86 arch code s390/mm: Remove sev_active() function arch/Kconfig| 3 +++ arch/s390/Kconfig | 4 +--- arch/s390/include/asm/mem_encrypt.h | 5 + arch/s390/mm/init.c | 7 +-- arch/x86/Kconfig| 4 +--- arch/x86/include/asm/mem_encrypt.h | 10 ++ arch/x86/kernel/crash_dump_64.c | 5 + arch/x86/mm/mem_encrypt.c | 2 -- fs/proc/vmcore.c| 8 include/linux/crash_dump.h | 14 ++ include/linux/mem_encrypt.h | 15 +-- kernel/dma/mapping.c| 8 kernel/dma/swiotlb.c| 3 +-- 13 files changed, 42 insertions(+), 46 deletions(-)
Re: [PATCH] iommu/iova: wait 'fq_timer' handler to finish before destroying 'fq'
+ (Robin) Hi Robin, Sorry to ping you... What's your suggestion for this patch? I'm looking forward to your reply. Thanks, Xiongfeng. On 2019/7/27 17:21, Xiongfeng Wang wrote: > Fix following crash that occurs when 'fq_flush_timeout()' access > 'fq->lock' while 'iovad->fq' has been cleared. This happens when the > 'fq_timer' handler is being executed and we call > 'free_iova_flush_queue()'. When the timer handler is being executed, > its pending state is cleared and it is detached. This patch use > 'del_timer_sync()' to wait for the timer handler 'fq_flush_timeout()' to > finish before destroying the flush queue. > > [ 9052.361840] Unable to handle kernel paging request at virtual address > a02fd6c66008 > [ 9052.361843] Mem abort info: > [ 9052.361845] ESR = 0x9604 > [ 9052.361847] Exception class = DABT (current EL), IL = 32 bits > [ 9052.361849] SET = 0, FnV = 0 > [ 9052.361850] EA = 0, S1PTW = 0 > [ 9052.361852] Data abort info: > [ 9052.361853] ISV = 0, ISS = 0x0004 > [ 9052.361855] CM = 0, WnR = 0 > [ 9052.361860] user pgtable: 4k pages, 48-bit VAs, pgdp = 9b665b91 > [ 9052.361863] [a02fd6c66008] pgd= > [ 9052.361870] Internal error: Oops: 9604 [#1] SMP > [ 9052.361873] Process rmmod (pid: 51122, stack limit = 0x3f5524f7) > [ 9052.361881] CPU: 69 PID: 51122 Comm: rmmod Kdump: loaded Tainted: G >OE 4.19.36-vhulk1906.3.0.h356.eulerosv2r8.aarch64 #1 > [ 9052.361882] Hardware name: Huawei TaiShan 2280 V2/BC82AMDC, BIOS 0.81 > 07/10/2019 > [ 9052.361885] pstate: 80400089 (Nzcv daIf +PAN -UAO) > [ 9052.361902] pc : fq_flush_timeout+0x9c/0x110 > [ 9052.361904] lr : (null) > [ 9052.361906] sp : 0965bd80 > [ 9052.361907] x29: 0965bd80 x28: 0202 > [ 9052.361912] x27: x26: 0053 > [ 9052.361915] x25: a026ed805008 x24: 09119810 > [ 9052.361919] x23: 0911b938 x22: 0911bc04 > [ 9052.361922] x21: a026ed804f28 x20: a02fd6c66008 > [ 9052.361926] x19: a02fd6c64000 x18: 09117000 > [ 9052.361929] x17: 0008 x16: > [ 9052.361933] x15: 09119708 x14: 0115 > [ 9052.361936] x13: 092f09d7 x12: > [ 9052.361940] x11: 0001 x10: 0965be98 > [ 9052.361943] x9 : x8 : 0007 > [ 9052.361947] x7 : 0010 x6 : 00d658b784ef > [ 9052.361950] x5 : 00ff x4 : > [ 9052.361954] x3 : 0013 x2 : 0001 > [ 9052.361957] x1 : x0 : a02fd6c66008 > [ 9052.361961] Call trace: > [ 9052.361967] fq_flush_timeout+0x9c/0x110 > [ 9052.361976] call_timer_fn+0x34/0x178 > [ 9052.361980] expire_timers+0xec/0x158 > [ 9052.361983] run_timer_softirq+0xc0/0x1f8 > [ 9052.361987] __do_softirq+0x120/0x324 > [ 9052.361995] irq_exit+0x11c/0x140 > [ 9052.362003] __handle_domain_irq+0x6c/0xc0 > [ 9052.362005] gic_handle_irq+0x6c/0x150 > [ 9052.362008] el1_irq+0xb8/0x140 > [ 9052.362010] vprintk_emit+0x2b4/0x320 > [ 9052.362013] vprintk_default+0x54/0x90 > [ 9052.362016] vprintk_func+0xa0/0x150 > [ 9052.362019] printk+0x74/0x94 > [ 9052.362034] nvme_get_smart+0x200/0x220 [nvme] > [ 9052.362041] nvme_remove+0x38/0x250 [nvme] > [ 9052.362051] pci_device_remove+0x48/0xd8 > [ 9052.362065] device_release_driver_internal+0x1b4/0x250 > [ 9052.362068] driver_detach+0x64/0xe8 > [ 9052.362072] bus_remove_driver+0x64/0x118 > [ 9052.362074] driver_unregister+0x34/0x60 > [ 9052.362077] pci_unregister_driver+0x24/0xd8 > [ 9052.362083] nvme_exit+0x24/0x1754 [nvme] > [ 9052.362094] __arm64_sys_delete_module+0x19c/0x2a0 > [ 9052.362102] el0_svc_common+0x78/0x130 > [ 9052.362106] el0_svc_handler+0x38/0x78 > [ 9052.362108] el0_svc+0x8/0xc > > Signed-off-by: Xiongfeng Wang > --- > drivers/iommu/iova.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c > index 3e1a8a6..90e8035 100644 > --- a/drivers/iommu/iova.c > +++ b/drivers/iommu/iova.c > @@ -64,8 +64,7 @@ static void free_iova_flush_queue(struct iova_domain *iovad) > if (!has_iova_flush_queue(iovad)) > return; > > - if (timer_pending(&iovad->fq_timer)) > - del_timer(&iovad->fq_timer); > + del_timer_sync(&iovad->fq_timer); > > fq_destroy_all_entries(iovad); > > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 2/2] iommu/vt-d: Fix possible use-after-free of private domain
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 --- 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(&domain->devices)) domain_exit(info->domain); free_devinfo_mem(info); -- 2.17.1
[PATCH 1/2] iommu/vt-d: Detach domain before using a private one
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 --- 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(&device_domain_lock, flags); info = dev->archdata.iommu; - __dmar_remove_one_dev_info(info); + if (info) + __dmar_remove_one_dev_info(info); spin_unlock_irqrestore(&device_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, -- 2.17.1
Re: [PATCH v4 12/15] iommu/vt-d: Cleanup get_valid_domain_for_dev()
Hi Alex, On 8/3/19 12:54 AM, Alex Williamson wrote: On Fri, 2 Aug 2019 15:17:45 +0800 Lu Baolu wrote: Hi Alex, Thanks for reporting this. I will try to find a machine with a pcie-to-pci bridge and get this issue fixed. I will update you later. Further debug below... On 8/2/19 9:30 AM, Alex Williamson wrote: DMAR: No ATSR found DMAR: dmar0: Using Queued invalidation DMAR: dmar1: Using Queued invalidation pci :00:00.0: DMAR: Software identity mapping pci :00:01.0: DMAR: Software identity mapping pci :00:02.0: DMAR: Software identity mapping pci :00:16.0: DMAR: Software identity mapping pci :00:1a.0: DMAR: Software identity mapping pci :00:1b.0: DMAR: Software identity mapping pci :00:1c.0: DMAR: Software identity mapping pci :00:1c.5: DMAR: Software identity mapping pci :00:1c.6: DMAR: Software identity mapping pci :00:1c.7: DMAR: Software identity mapping pci :00:1d.0: DMAR: Software identity mapping pci :00:1f.0: DMAR: Software identity mapping pci :00:1f.2: DMAR: Software identity mapping pci :00:1f.3: DMAR: Software identity mapping pci :01:00.0: DMAR: Software identity mapping pci :01:00.1: DMAR: Software identity mapping pci :03:00.0: DMAR: Software identity mapping pci :04:00.0: DMAR: Software identity mapping DMAR: Setting RMRR: pci :00:02.0: DMAR: Setting identity map [0xbf80 - 0xcf9f] pci :00:1a.0: DMAR: Setting identity map [0xbe8d1000 - 0xbe8d] pci :00:1d.0: DMAR: Setting identity map [0xbe8d1000 - 0xbe8d] DMAR: Prepare 0-16MiB unity mapping for LPC pci :00:1f.0: DMAR: Setting identity map [0x0 - 0xff] pci :00:00.0: Adding to iommu group 0 pci :00:00.0: Using iommu direct mapping pci :00:01.0: Adding to iommu group 1 pci :00:01.0: Using iommu direct mapping pci :00:02.0: Adding to iommu group 2 pci :00:02.0: Using iommu direct mapping pci :00:16.0: Adding to iommu group 3 pci :00:16.0: Using iommu direct mapping pci :00:1a.0: Adding to iommu group 4 pci :00:1a.0: Using iommu direct mapping pci :00:1b.0: Adding to iommu group 5 pci :00:1b.0: Using iommu direct mapping pci :00:1c.0: Adding to iommu group 6 pci :00:1c.0: Using iommu direct mapping pci :00:1c.5: Adding to iommu group 7 pci :00:1c.5: Using iommu direct mapping pci :00:1c.6: Adding to iommu group 8 pci :00:1c.6: Using iommu direct mapping pci :00:1c.7: Adding to iommu group 9 Note that group 9 contains device 00:1c.7 pci :00:1c.7: Using iommu direct mapping I'm booted with iommu=pt, so the domain type is IOMMU_DOMAIN_PT pci :00:1d.0: Adding to iommu group 10 pci :00:1d.0: Using iommu direct mapping pci :00:1f.0: Adding to iommu group 11 pci :00:1f.0: Using iommu direct mapping pci :00:1f.2: Adding to iommu group 11 pci :00:1f.3: Adding to iommu group 11 pci :01:00.0: Adding to iommu group 1 pci :01:00.1: Adding to iommu group 1 pci :03:00.0: Adding to iommu group 12 pci :03:00.0: Using iommu direct mapping pci :04:00.0: Adding to iommu group 13 pci :04:00.0: Using iommu direct mapping pci :05:00.0: Adding to iommu group 9 05:00.0 is downstream of 00:1c.7 and in the same group. As above, the domain is type IOMMU_DOMAIN_IDENTITY, so we take the following branch: } else { if (device_def_domain_type(dev) == IOMMU_DOMAIN_DMA) { Default domain type is IOMMU_DOMAIN_DMA because of the code block in device_def_domain_type() handling bridges to conventional PCI and conventional PCI endpoints. ret = iommu_request_dma_domain_for_dev(dev); This fails in request_default_domain_for_dev() because there's more than one device in the group. if (ret) { dmar_domain->flags |= DOMAIN_FLAG_LOSE_CHILDREN; if (!get_private_domain_for_dev(dev)) { With this commit, this now returns NULL because find_domain() does find a domain, the same one we found before this code block. dev_warn(dev, "Failed to get a private domain.\n"); return -ENOMEM; } So the key factors are that I'm booting with iommu=pt and I have a PCIe-to-PCI bridge grouped with its parent root port. The bridge wants an IOMMU_DOMAIN_DMA, but the group domain is already of type IOMMU_DOMAIN_IDENTITY. A temporary workaround is to not use passthrough mode, but this is a regression versus previous kernels. Thanks, I can reproduce this issue with a local setup. I will submit the fix and cc it to you. Please let me know if that fix doesn't solve this problem. Best regards, Baolu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 11/22] iommu: Introduce guest PASID bind function
On Tue, 16 Jul 2019 18:44:56 +0200 Auger Eric wrote: > > +struct gpasid_bind_data { > other structs in iommu.h are prefixed with iommu_? Good point, will add iommu_ prefix. Thanks, Jacob ___ 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 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&id=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(&qcom_scm_lock); > > > + > > > + __qcom_scm_call_do(desc, res, fn_id, x5, > > > +ARM_SMCCC_STD_CALL); > > > + > > > + mutex_unlock(&qcom_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? Regards, Bjorn ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 4/4] arm64: dts/sdm845: Enable FW implemented safe sequence handler on MTP
On Wed 12 Jun 00:15 PDT 2019, Vivek Gautam wrote: > Indicate on MTP SDM845 that firmware implements handler to > TLB invalidate erratum SCM call where SAFE sequence is toggled > to achieve optimum performance on real-time clients, such as > display and camera. > > Signed-off-by: Vivek Gautam > --- > arch/arm64/boot/dts/qcom/sdm845.dtsi | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi > b/arch/arm64/boot/dts/qcom/sdm845.dtsi > index 78ec373a2b18..6a73d9744a71 100644 > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi > @@ -2368,6 +2368,7 @@ > compatible = "qcom,sdm845-smmu-500", "arm,mmu-500"; > reg = <0 0x1500 0 0x8>; > #iommu-cells = <2>; > + qcom,smmu-500-fw-impl-safe-errata; Looked back at this series and started to wonder if there there is a case where this should not be set? I mean we're after all adding this to the top 845 dtsi... How about making it the default in the driver and opt out of the errata once there is a need? Regards, Bjorn > #global-interrupts = <1>; > interrupts = , >, > -- > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member > of Code Aurora Forum, hosted by The Linux Foundation > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 11/22] iommu: Introduce guest PASID bind function
On Tue, 16 Jul 2019 18:44:56 +0200 Auger Eric wrote: > Hi Jacob, > > On 6/9/19 3:44 PM, Jacob Pan wrote: > > Guest shared virtual address (SVA) may require host to shadow guest > > PASID tables. Guest PASID can also be allocated from the host via > > enlightened interfaces. In this case, guest needs to bind the guest > > mm, i.e. cr3 in guest physical address to the actual PASID table in > > the host IOMMU. Nesting will be turned on such that guest virtual > > address can go through a two level translation: > > - 1st level translates GVA to GPA > > - 2nd level translates GPA to HPA > > This patch introduces APIs to bind guest PASID data to the assigned > > device entry in the physical IOMMU. See the diagram below for usage > > explaination. > > > > .-. .---. > > | vIOMMU| | Guest process mm, FL only | > > | | '---' > > ./ > > | PASID Entry |--- PASID cache flush - > > '-' | > > | | V > > | | GP > > '-' > > Guest > > --| Shadow |--- GP->HP* - > > vv | > > Host v > > .-. .--. > > | pIOMMU| | Bind FL for GVA-GPA | > > | | '--' > > ./ | > > | PASID Entry | V (Nested xlate) > > '\.-. > > | | |Set SL to GPA-HPA| > > | | '-' > > '-' > > > > Where: > > - FL = First level/stage one page tables > > - SL = Second level/stage two page tables > > - GP = Guest PASID > > - HP = Host PASID > > * Conversion needed if non-identity GP-HP mapping option is chosen. > > > > Signed-off-by: Jacob Pan > > Signed-off-by: Liu Yi L > > --- > > drivers/iommu/iommu.c | 20 > > include/linux/iommu.h | 21 + > > include/uapi/linux/iommu.h | 58 > > ++ 3 files changed, 99 > > insertions(+) > > > > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c > > index 1758b57..d0416f60 100644 > > --- a/drivers/iommu/iommu.c > > +++ b/drivers/iommu/iommu.c > > @@ -1648,6 +1648,26 @@ int iommu_cache_invalidate(struct > > iommu_domain *domain, struct device *dev, } > > EXPORT_SYMBOL_GPL(iommu_cache_invalidate); > > > > +int iommu_sva_bind_gpasid(struct iommu_domain *domain, > > + struct device *dev, struct > > gpasid_bind_data *data) +{ > > + if (unlikely(!domain->ops->sva_bind_gpasid)) > > + return -ENODEV; > > + > > + return domain->ops->sva_bind_gpasid(domain, dev, data); > > +} > > +EXPORT_SYMBOL_GPL(iommu_sva_bind_gpasid); > > + > > +int iommu_sva_unbind_gpasid(struct iommu_domain *domain, struct > > device *dev, > > + ioasid_t pasid) > > +{ > > + if (unlikely(!domain->ops->sva_unbind_gpasid)) > > + return -ENODEV; > > + > > + return domain->ops->sva_unbind_gpasid(dev, pasid); > > +} > > +EXPORT_SYMBOL_GPL(iommu_sva_unbind_gpasid); > > + > > static void __iommu_detach_device(struct iommu_domain *domain, > > struct device *dev) > > { > > diff --git a/include/linux/iommu.h b/include/linux/iommu.h > > index 8d766a8..560c8c8 100644 > > --- a/include/linux/iommu.h > > +++ b/include/linux/iommu.h > > @@ -25,6 +25,7 @@ > > #include > > #include > > #include > > +#include > > #include > > > > #define IOMMU_READ (1 << 0) > > @@ -267,6 +268,8 @@ struct page_response_msg { > > * @detach_pasid_table: detach the pasid table > > * @cache_invalidate: invalidate translation caches > > * @pgsize_bitmap: bitmap of all possible supported page sizes > > + * @sva_bind_gpasid: bind guest pasid and mm > > + * @sva_unbind_gpasid: unbind guest pasid and mm > comment vs field ordering as pointed out by Jonathan on other patches > > */ > > struct iommu_ops { > > bool (*capable)(enum iommu_cap); > > @@ -332,6 +335,10 @@ struct iommu_ops { > > int (*page_response)(struct device *dev, struct > > page_response_msg *msg); int (*cache_invalidate)(struct > > iommu_domain *domain, struct device *dev, struct > > iommu_cache_invalidate_info *inv_info); > > + int (*sva_bind_gpasid)(struct iommu_domain *domain, > > + struct device *dev, struct > > gpasid_bind_data *data); + > > + int (*sva_unbind_gpasid)(struct device *dev, int pasid); > > > > unsigned long pgsize_bitmap; > > }; > > @@ -447,6 +454,10 @@ extern void iommu_detach_pasid_table(struct > > iommu_domain *domain); extern int iommu_cache_invalidate(struct > > iommu_domain *domain, struct device *dev, > > struct > > iommu_cache_invalidate_info *inv_info); +extern
Re: [PATCH v4 14/22] iommu/vt-d: Add custom allocator for IOASID
On Tue, 16 Jul 2019 11:30:14 +0200 Auger Eric wrote: > Hi Jacob, > > On 6/9/19 3:44 PM, Jacob Pan wrote: > > When VT-d driver runs in the guest, PASID allocation must be > > performed via virtual command interface. This patch registers a > > custom IOASID allocator which takes precedence over the default > > XArray based allocator. The resulting IOASID allocation will always > > come from the host. This ensures that PASID namespace is system- > > wide. > > > > Signed-off-by: Lu Baolu > > Signed-off-by: Liu, Yi L > > Signed-off-by: Jacob Pan > > --- > > drivers/iommu/Kconfig | 1 + > > drivers/iommu/intel-iommu.c | 60 > > + > > include/linux/intel-iommu.h | 2 ++ 3 files changed, 63 > > insertions(+) > > > > diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig > > index c40c4b5..5d1bc2a 100644 > > --- a/drivers/iommu/Kconfig > > +++ b/drivers/iommu/Kconfig > > @@ -213,6 +213,7 @@ config INTEL_IOMMU_SVM > > bool "Support for Shared Virtual Memory with Intel IOMMU" > > depends on INTEL_IOMMU && X86 > > select PCI_PASID > > + select IOASID > > select MMU_NOTIFIER > > select IOASID > > help > > diff --git a/drivers/iommu/intel-iommu.c > > b/drivers/iommu/intel-iommu.c index 09b8ff0..5b84994 100644 > > --- a/drivers/iommu/intel-iommu.c > > +++ b/drivers/iommu/intel-iommu.c > > @@ -1711,6 +1711,8 @@ static void free_dmar_iommu(struct > > intel_iommu *iommu) if (ecap_prs(iommu->ecap)) > > intel_svm_finish_prq(iommu); > > } > > + ioasid_unregister_allocator(&iommu->pasid_allocator); > > + > > #endif > > } > > > > @@ -4820,6 +4822,46 @@ static int __init > > platform_optin_force_iommu(void) return 1; > > } > > > > +#ifdef CONFIG_INTEL_IOMMU_SVM > > +static ioasid_t intel_ioasid_alloc(ioasid_t min, ioasid_t max, > > void *data) +{ > > + struct intel_iommu *iommu = data; > > + ioasid_t ioasid; > > + > > + /* > > +* VT-d virtual command interface always uses the full 20 > > bit > > +* PASID range. Host can partition guest PASID range based > > on > > +* policies but it is out of guest's control. > > +*/ > > + if (min < PASID_MIN || max > PASID_MAX) > > + return -EINVAL; > ioasid_alloc() does not handle that error value, use INVALID_IOASID? > > + > > + if (vcmd_alloc_pasid(iommu, &ioasid)) > > + return INVALID_IOASID; > > + > > + return ioasid; > > +} > > + > > +static void intel_ioasid_free(ioasid_t ioasid, void *data) > > +{ > > + struct iommu_pasid_alloc_info *svm; > > + struct intel_iommu *iommu = data; > > + > > + if (!iommu) > > + return; > > + /* > > +* Sanity check the ioasid owner is done at upper layer, > > e.g. VFIO > > +* We can only free the PASID when all the devices are > > unbond. > > +*/ > > + svm = ioasid_find(NULL, ioasid, NULL); > > + if (!svm) { > > + pr_warn("Freeing unbond IOASID %d\n", ioasid); > > + return; > > + } > > + vcmd_free_pasid(iommu, ioasid); > > +} > > +#endif > > + > > int __init intel_iommu_init(void) > > { > > int ret = -ENODEV; > > @@ -4924,6 +4966,24 @@ int __init intel_iommu_init(void) > >"%s", iommu->name); > > iommu_device_set_ops(&iommu->iommu, > > &intel_iommu_ops); iommu_device_register(&iommu->iommu); > > +#ifdef CONFIG_INTEL_IOMMU_SVM > > + if (cap_caching_mode(iommu->cap) && > > sm_supported(iommu)) { > > + /* > > +* Register a custom ASID allocator if we > > are running > > +* in a guest, the purpose is to have a > > system wide PASID > > +* namespace among all PASID users. > > +* There can be multiple vIOMMUs in each > > guest but only > > +* one allocator is active. All vIOMMU > > allocators will > > +* eventually be calling the same host > > allocator. > > +*/ > > + iommu->pasid_allocator.alloc = > > intel_ioasid_alloc; > > + iommu->pasid_allocator.free = > > intel_ioasid_free; > > + iommu->pasid_allocator.pdata = (void > > *)iommu; > > + ret = > > ioasid_register_allocator(&iommu->pasid_allocator); > > + if (ret) > > + pr_warn("Custom PASID allocator > > registeration failed\n"); > what if it fails, don't you want a tear down path? > Good point, we need to disable PASID usage, i.e. disable scalable mode if there is no virtual command based PASID allocator in the guest. Sorry for the late reply. > Thanks > > Eric > [...] [Jacob Pan]
Re: [PATCH 3/8] of/fdt: add function to get the SoC wide DMA addressable memory size
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. > > > Signed-off-by: Nicolas Saenz Julienne > > > --- > > > > > > drivers/of/fdt.c | 72 ++ > > > include/linux/of_fdt.h | 2 ++ > > > 2 files changed, 74 insertions(+) > > > > > > diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c > > > index 9cdf14b9aaab..f2444c61a136 100644 > > > --- a/drivers/of/fdt.c > > > +++ b/drivers/of/fdt.c > > > @@ -953,6 +953,78 @@ int __init early_init_dt_scan_chosen_stdout(void) > > > } > > > #endif > > > > > > +/** > > > + * early_init_dt_dma_zone_size - Look at all 'dma-ranges' and provide the > > > + * maximum common dmable memory size. > > > + * > > > + * Some devices might have multiple interconnects each with their own DMA > > > + * addressing limitations. For example the Raspberry Pi 4 has the > > > following: > > > + * > > > + * soc { > > > + * dma-ranges = <0xc000 0x0 0x 0x3c00>; > > > + * [...] > > > + * } > > > + * > > > + * v3dbus { > > > + * dma-ranges = <0x 0x0 0x 0x3c00>; > > > + * [...] > > > + * } > > > + * > > > + * scb { > > > + * dma-ranges = <0x0 0x 0x0 0x 0xfc00>; > > > + * [...] > > > + * } > > > + * > > > + * Here the area addressable by all devices is [0x-0x3bff]. > > > Hence > > > + * the function will write in 'data' a size of 0x3c00. > > > + * > > > + * Note that the implementation assumes all interconnects have the same > > > physical > > > + * memory view and that the mapping always start at the beginning of RAM. > > > > Not really a valid assumption for general code. > > Fair enough. On my defence I settled on that assumption after grepping all dts > and being unable to find a board that behaved otherwise. > > [...] > > > It's possible to have multiple levels of nodes and dma-ranges. You need to > > handle that case too. Doing that and handling differing address translations > > will be complicated. > > 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. Rob
Re: [PATCH 3/8] of/fdt: add function to get the SoC wide DMA addressable memory size
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. > > Signed-off-by: Nicolas Saenz Julienne > > --- > > > > drivers/of/fdt.c | 72 ++ > > include/linux/of_fdt.h | 2 ++ > > 2 files changed, 74 insertions(+) > > > > diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c > > index 9cdf14b9aaab..f2444c61a136 100644 > > --- a/drivers/of/fdt.c > > +++ b/drivers/of/fdt.c > > @@ -953,6 +953,78 @@ int __init early_init_dt_scan_chosen_stdout(void) > > } > > #endif > > > > +/** > > + * early_init_dt_dma_zone_size - Look at all 'dma-ranges' and provide the > > + * maximum common dmable memory size. > > + * > > + * Some devices might have multiple interconnects each with their own DMA > > + * addressing limitations. For example the Raspberry Pi 4 has the > > following: > > + * > > + * soc { > > + * dma-ranges = <0xc000 0x0 0x 0x3c00>; > > + * [...] > > + * } > > + * > > + * v3dbus { > > + * dma-ranges = <0x 0x0 0x 0x3c00>; > > + * [...] > > + * } > > + * > > + * scb { > > + * dma-ranges = <0x0 0x 0x0 0x 0xfc00>; > > + * [...] > > + * } > > + * > > + * Here the area addressable by all devices is [0x-0x3bff]. > > Hence > > + * the function will write in 'data' a size of 0x3c00. > > + * > > + * Note that the implementation assumes all interconnects have the same > > physical > > + * memory view and that the mapping always start at the beginning of RAM. > > Not really a valid assumption for general code. Fair enough. On my defence I settled on that assumption after grepping all dts and being unable to find a board that behaved otherwise. [...] > It's possible to have multiple levels of nodes and dma-ranges. You need to > handle that case too. Doing that and handling differing address translations > will be complicated. 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? signature.asc Description: This is a digitally signed message part
Re: large DMA segments vs SWIOTLB
Hi Christoph, Am Donnerstag, den 01.08.2019, 16:00 +0200 schrieb Christoph Hellwig: > On Thu, Aug 01, 2019 at 10:35:02AM +0200, Lucas Stach wrote: > > Hi Christoph, > > > > Am Donnerstag, den 01.08.2019, 09:29 +0200 schrieb Christoph Hellwig: > > > Hi Lukas, > > > > > > have you tried the latest 5.3-rc kernel, where we limited the NVMe > > > I/O size based on the swiotlb buffer size? > > > > Yes, the issue was reproduced on 5.3-rc2. I now see your commit > > limiting the request size, so I guess I need to dig in to see why I'm > > still getting requests larger than the SWIOTLB max segment size. Thanks > > for the pointer! > > a similar setup to yours the > dma_addressing_limited doesn't work, but if we changed it to a <= > it does. The result is counter to what I'd expect, but because I'm on > vacation I didn't have time to look into why it works. This is his > patch, let me know if this works for you: > > > diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h > index f7d1eea32c78..89ac1cf754cc 100644 > --- a/include/linux/dma-mapping.h > +++ b/include/linux/dma-mapping.h > @@ -689,7 +689,7 @@ static inline int dma_coerce_mask_and_coherent(struct > device *dev, u64 mask) > */ > static inline bool dma_addressing_limited(struct device *dev) > { > > - return min_not_zero(dma_get_mask(dev), dev->bus_dma_mask) < > > + return min_not_zero(dma_get_mask(dev), dev->bus_dma_mask) <= > > dma_get_required_mask(dev); > } From the patch I just sent it should be clear why the above works. With my patch applied I can't reproduce any issues with this NVMe device anymore. Thanks for pointing me into the right direction! Regards, Lucas ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] dma-direct: don't truncate dma_required_mask to bus addressing capabilities
The dma required_mask needs to reflect the actual addressing capabilities needed to handle the whole system RAM. When truncated down to the bus addressing capabilities dma_addressing_limited() will incorrectly signal no limitations for devices which are restricted by the bus_dma_mask. Fixes: b4ebe6063204 (dma-direct: implement complete bus_dma_mask handling) Signed-off-by: Lucas Stach --- kernel/dma/direct.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c index 59bdceea3737..7ba3480fc546 100644 --- a/kernel/dma/direct.c +++ b/kernel/dma/direct.c @@ -47,9 +47,6 @@ u64 dma_direct_get_required_mask(struct device *dev) { u64 max_dma = phys_to_dma_direct(dev, (max_pfn - 1) << PAGE_SHIFT); - if (dev->bus_dma_mask && dev->bus_dma_mask < max_dma) - max_dma = dev->bus_dma_mask; - return (1ULL << (fls64(max_dma) - 1)) * 2 - 1; } -- 2.20.1
[PATCH v3 1/8] dma: debug: Replace strncmp with str_has_prefix
strncmp(str, const, len) is error-prone because len is easy to have typo. The example is the hard-coded len has counting error or sizeof(const) forgets - 1. So we prefer using newly introduced str_has_prefix() to substitute such strncmp to make code better. Signed-off-by: Chuhong Yuan --- Changes in v3: - Revise the description. kernel/dma/debug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/dma/debug.c b/kernel/dma/debug.c index 099002d84f46..0f9e1aba3e1a 100644 --- a/kernel/dma/debug.c +++ b/kernel/dma/debug.c @@ -970,7 +970,7 @@ static __init int dma_debug_cmdline(char *str) if (!str) return -EINVAL; - if (strncmp(str, "off", 3) == 0) { + if (str_has_prefix(str, "off")) { pr_info("debugging disabled on kernel command line\n"); global_disable = true; } -- 2.20.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3 0/8] Replace strncmp with str_has_prefix
The commit 72921427d46b ("string.h: Add str_has_prefix() helper function") introduced str_has_prefix() to substitute error-prone strncmp(str, const, len). strncmp(str, const, len) is easy to have error in len because of counting error or sizeof(const) without - 1. These patches replace such pattern with str_has_prefix() to avoid hard coded constant length and sizeof. Besides, str_has_prefix() returns the length of prefix when the comparison returns true. We can use this return value to substitute some hard-coding. Changelog: v1 -> v2: - Revise the description. - Use the return value of str_has_prefix() to eliminate hard coding. - Remove possible false positives and add newly detected one in upstream. v2 -> v3: - Revise the description. - Remove else uses in printk.c. Chuhong Yuan (8): dma: debug: Replace strncmp with str_has_prefix module: Replace strncmp with str_has_prefix PM/sleep: Replace strncmp with str_has_prefix printk: Replace strncmp with str_has_prefix reboot: Replace strncmp with str_has_prefix sched: Replace strncmp with str_has_prefix userns: Replace strncmp with str_has_prefix watchdog: Replace strncmp with str_has_prefix kernel/dma/debug.c | 2 +- kernel/module.c | 2 +- kernel/power/main.c | 2 +- kernel/printk/braille.c | 10 ++ kernel/printk/printk.c | 19 +-- kernel/reboot.c | 6 -- kernel/sched/debug.c | 5 +++-- kernel/sched/isolation.c | 9 + kernel/user_namespace.c | 10 +- kernel/watchdog.c| 8 10 files changed, 43 insertions(+), 30 deletions(-) -- 2.20.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC,v3 6/9] media: platform: Add Mediatek ISP P1 V4L2 functions
Hi Jungo, On Tue, Jul 30, 2019 at 10:45 AM Jungo Lin wrote: > > On Mon, 2019-07-29 at 19:04 +0900, Tomasz Figa wrote: > > On Mon, Jul 29, 2019 at 10:18 AM Jungo Lin wrote: > > > On Fri, 2019-07-26 at 14:49 +0900, Tomasz Figa wrote: > > > > On Wed, Jul 24, 2019 at 1:31 PM Jungo Lin > > > > wrote: > > > > > On Tue, 2019-07-23 at 19:21 +0900, Tomasz Figa wrote: > > > > > > On Thu, Jul 18, 2019 at 1:39 PM Jungo Lin > > > > > > wrote: > > > > > > > On Wed, 2019-07-10 at 18:54 +0900, Tomasz Figa wrote: > > > > > > > > On Tue, Jun 11, 2019 at 11:53:41AM +0800, Jungo Lin wrote: [snip] > > > > > > > > > + > > > > > > > > > + dev_dbg(dev, "%s: node:%d fd:%d idx:%d\n", > > > > > > > > > + __func__, > > > > > > > > > + node->id, > > > > > > > > > + buf->vbb.request_fd, > > > > > > > > > + buf->vbb.vb2_buf.index); > > > > > > > > > + > > > > > > > > > + /* For request buffers en-queue, handled in > > > > > > > > > mtk_cam_req_try_queue */ > > > > > > > > > + if (vb->vb2_queue->uses_requests) > > > > > > > > > + return; > > > > > > > > > > > > > > > > I'd suggest removing non-request support from this driver. Even > > > > > > > > if we end up > > > > > > > > with a need to provide compatibility for non-request mode, then > > > > > > > > it should be > > > > > > > > built on top of the requests mode, so that the driver itself > > > > > > > > doesn't have to > > > > > > > > deal with two modes. > > > > > > > > > > > > > > > > > > > > > > The purpose of non-request function in this driver is needed by > > > > > > > our camera middle-ware design. It needs 3A statistics buffers > > > > > > > before > > > > > > > image buffers en-queue. So we need to en-queue 3A statistics with > > > > > > > non-request mode in this driver. After MW got the 3A statistics > > > > > > > data, it > > > > > > > will en-queue the images, tuning buffer and other meta buffers > > > > > > > with > > > > > > > request mode. Based on this requirement, do you have any > > > > > > > suggestion? > > > > > > > For upstream driver, should we only consider request mode? > > > > > > > > > > > > > > > > > > > Where does that requirement come from? Why the timing of queuing of > > > > > > the buffers to the driver is important? > > > > > > > > > > > > [snip] > > > > > > > > > > Basically, this requirement comes from our internal camera > > > > > middle-ware/3A hal in user space. Since this is not generic > > > > > requirement, > > > > > we will follow your original suggestion to keep the request mode only > > > > > and remove other non-request design in other files. For upstream > > > > > driver, > > > > > it should support request mode only. > > > > > > > > > > > > > Note that Chromium OS will use the "upstream driver" and we don't want > > > > to diverge, so please make the userspace also use only requests. I > > > > don't see a reason why there would be any need to submit any buffers > > > > outside of a request. > > > > > > > > [snip] > > > > > > Ok, I have raised your concern to our colleagues and let him to discuss > > > with you in another communication channel. > > > > > > > Thanks! > > > > Best regards, > > Tomasz > > Our colleague is preparing material to explain the our 3A/MW design. If > he is ready, he will discuss this with you. Thanks! > > In the original plan, we will deliver P1 v4 patch set tomorrow (31th > Jul.). But, there are some comments waiting for other experts' input. > Do you suggest it is better to resolve all comments before v4 patch set > submitting or continue to discuss these comments on v4? For the remaining v4l2-compliance issues, we can postpone them and keep on a TODO list in the next version. Best regards, Tomasz
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
[PATCH 6/7] dma-mapping: remove ARCH_NO_COHERENT_DMA_MMAP
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| 7 --- 8 files changed, 16 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 ab6aa355e1a0..47765c96fe09 100644 --- a/kernel/dma/mapping.c +++ b/kernel/dma/mapping.c @@ -169,7 +169,6 @@ 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 unsigned long user_count = vma_pages(vma); unsigned long count = PAGE_ALIGN(size) >> PAGE_SHIFT; unsigned long off = vma->vm_pgoff; @@ -198,9 +197,6 @@ int dma_common_mmap(struct device *dev, struct vm_area_struct *vma, return remap_pfn_range(vma, vma->vm_start, pfn + vma->vm_pgoff, user_count << PAGE_SHIFT, vma->vm_page_prot); -#else - return -ENXIO; -#endif /* !CONFIG_ARCH_NO_COHERENT_DMA_MMAP */ } /** @@ -214,9 +210,6 @@ 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_T
[PATCH 4/7] 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 --- 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 return true; } -- 2.20.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 7/7] 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 4410af33c5c4..9c9a23e5600d 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 70fcc9736a8c..3542b7bd6a46 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, .mmap = dma_common_mmap, @@ -710,7 +704,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, .mmap = dma_common_mmap, diff --git a/arch/x86/kernel/amd_gart_64.c b/arch/x86/kernel/amd_gart_64.c index a65b4a9c7f87..d02662238b57 100644 --- a/arch/x86/kernel/amd_gart_64.c +++ b/arch/x86/kernel/amd_gart_64.c @@ -680,6 +680,7 @@ static const struct dma_map_ops gart_dma_ops = { .dma_supported = dma_direct_supported, .mmap = dma_common_mmap, .get_sgtable= dma_common_get_sgtable, + .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 47765c96fe09..96599f39f67a 100644 --- a/kernel/dma/mapping.c +++ b/kernel/dma/mapping.c @@ -247,25 +247,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); @@ -274,7 +255,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
[PATCH 3/7] 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 cdb157cd70e7..ab6aa355e1a0 100644 --- a/kernel/dma/mapping.c +++ b/kernel/dma/mapping.c @@ -203,6 +203,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 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 5/7] m68knommu: add a pgprot_noncached stub
Provide a pgprot_noncached like all the other nommu ports so that common code can rely on it being able to be present. Note that this is generally code that is not actually run on nommu, but at least we can avoid nasty ifdefs by having a stub. Signed-off-by: Christoph Hellwig --- arch/m68k/include/asm/pgtable_no.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/m68k/include/asm/pgtable_no.h b/arch/m68k/include/asm/pgtable_no.h index fc3a96c77bd8..06194c7ba151 100644 --- a/arch/m68k/include/asm/pgtable_no.h +++ b/arch/m68k/include/asm/pgtable_no.h @@ -29,6 +29,8 @@ #define PAGE_READONLY __pgprot(0) #define PAGE_KERNEL__pgprot(0) +#define pgprot_noncached(prot) (prot) + extern void paging_init(void); #define swapper_pg_dir ((pgd_t *) 0) -- 2.20.1
[PATCH 1/7] 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 6774b03aa405..4410af33c5c4 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -877,17 +877,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 b945239621d8..4ceb5b9016d8 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
remove default fallbacks in dma_map_ops v2
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 v1: - add a dma_can_mmap helper for alsa
[PATCH 2/7] 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 ++ kernel/dma/mapping.c| 20 13 files changed, 38 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 98410119c47b..70fcc9736a8c 100644 --- a/arch/powerpc/platforms/ps3/system-bus.c +++ b/arch/powerpc/platforms/ps3/system-bus.c @@ -700,6 +700,8 @@ static const struct dma_map_ops ps3_sb_dma_ops = { .get_required_mask = ps3_dma_get_required_mask, .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 = { @@ -711,6 +713,8 @@ static const struct dma_map_ops ps3_ioc0_dma_ops = { .get_required_mask = ps3_dma_get_required_mask, .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 b/arch/powerpc/platforms/pseries/vio.c index 6601b9d404dc..3473eef7628c 100644 --- a/arch/powerp
Re: [PATCH 1/2] dma-mapping: fix page attributes for dma_mmap_*
On Mon, Aug 05, 2019 at 11:01:44AM +0300, Christoph Hellwig wrote: > All the way back to introducing dma_common_mmap we've defaulyed to mark s/defaultyed/defaulted/ > the pages as uncached. But this is wrong for DMA coherent devices. > Later on DMA_ATTR_WRITE_COMBINE also got incorrect treatment as that > flag is only treated special on the alloc side for non-coherent devices. > > Introduce a new dma_pgprot helper that deals with the check for coherent > devices so that only the remapping cases even reach arch_dma_mmap_pgprot s/even/ever/ > diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c > index 1d3f0b5a9940..bd2b039f43a6 100644 > --- a/arch/arm64/mm/dma-mapping.c > +++ b/arch/arm64/mm/dma-mapping.c > @@ -14,9 +14,7 @@ > pgprot_t arch_dma_mmap_pgprot(struct device *dev, pgprot_t prot, > unsigned long attrs) > { > - if (!dev_is_dma_coherent(dev) || (attrs & DMA_ATTR_WRITE_COMBINE)) > - return pgprot_writecombine(prot); > - return prot; > + return pgprot_writecombine(prot); > } For arm64: Acked-by: Catalin Marinas ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/2] MIPS: remove support for DMA_ATTR_WRITE_COMBINE
Hello! On 05.08.2019 11:01, Christoph Hellwig wrote: Mips uses the KSEG1 kernel memory segment do map dma coherent MIPS. s/do/to/? allocations for n on-coherent devices as uncachable, and does not have Uncacheable? any kind of special support for DMA_ATTR_WRITE_COMBINE in the allocation path. Thus supporting DMA_ATTR_WRITE_COMBINE in dma_mmap_attrs will lead to multiple mappings with different caching attributes. Fixes: 8c172467be36 ("MIPS: Add implementation of dma_map_ops.mmap()") Signed-off-by: Christoph Hellwig [...] MBR, Sergei
[PATCH 1/2] dma-mapping: fix page attributes for dma_mmap_*
All the way back to introducing dma_common_mmap we've defaulyed to mark the pages as uncached. But this is wrong for DMA coherent devices. Later on DMA_ATTR_WRITE_COMBINE also got incorrect treatment as that flag is only treated special on the alloc side for non-coherent devices. Introduce a new dma_pgprot helper that deals with the check for coherent devices so that only the remapping cases even reach arch_dma_mmap_pgprot and we thus ensure no aliasing of page attributes happens, which makes the powerpc version of arch_dma_mmap_pgprot obsolete and simplifies the remaining ones. Note that this means arch_dma_mmap_pgprot is a bit misnamed now, but we'll phase it out soon. Fixes: 64ccc9c033c6 ("common: dma-mapping: add support for generic dma_mmap_* calls") Reported-by: Shawn Anastasio Reported-by: Gavin Li Signed-off-by: Christoph Hellwig --- arch/arm/mm/dma-mapping.c| 4 +--- arch/arm64/mm/dma-mapping.c | 4 +--- arch/powerpc/Kconfig | 1 - arch/powerpc/kernel/dma-common.c | 17 - drivers/iommu/dma-iommu.c| 6 +++--- include/linux/dma-mapping.h | 1 + include/linux/dma-noncoherent.h | 5 - kernel/dma/mapping.c | 17 - kernel/dma/remap.c | 2 +- 9 files changed, 23 insertions(+), 34 deletions(-) delete mode 100644 arch/powerpc/kernel/dma-common.c diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index 6774b03aa405..d42557ee69c2 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -2405,9 +2405,7 @@ long arch_dma_coherent_to_pfn(struct device *dev, void *cpu_addr, pgprot_t arch_dma_mmap_pgprot(struct device *dev, pgprot_t prot, unsigned long attrs) { - if (!dev_is_dma_coherent(dev)) - return __get_dma_pgprot(attrs, prot); - return prot; + return __get_dma_pgprot(attrs, prot); } void *arch_dma_alloc(struct device *dev, size_t size, dma_addr_t *dma_handle, diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c index 1d3f0b5a9940..bd2b039f43a6 100644 --- a/arch/arm64/mm/dma-mapping.c +++ b/arch/arm64/mm/dma-mapping.c @@ -14,9 +14,7 @@ pgprot_t arch_dma_mmap_pgprot(struct device *dev, pgprot_t prot, unsigned long attrs) { - if (!dev_is_dma_coherent(dev) || (attrs & DMA_ATTR_WRITE_COMBINE)) - return pgprot_writecombine(prot); - return prot; + return pgprot_writecombine(prot); } void arch_sync_dma_for_device(struct device *dev, phys_addr_t paddr, diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index 77f6ebf97113..d8dcd8820369 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -121,7 +121,6 @@ config PPC select ARCH_32BIT_OFF_T if PPC32 select ARCH_HAS_DEBUG_VIRTUAL select ARCH_HAS_DEVMEM_IS_ALLOWED - select ARCH_HAS_DMA_MMAP_PGPROT select ARCH_HAS_ELF_RANDOMIZE select ARCH_HAS_FORTIFY_SOURCE select ARCH_HAS_GCOV_PROFILE_ALL diff --git a/arch/powerpc/kernel/dma-common.c b/arch/powerpc/kernel/dma-common.c deleted file mode 100644 index dc7ef6b17b69.. --- a/arch/powerpc/kernel/dma-common.c +++ /dev/null @@ -1,17 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0-or-later -/* - * Contains common dma routines for all powerpc platforms. - * - * Copyright (C) 2019 Shawn Anastasio. - */ - -#include -#include - -pgprot_t arch_dma_mmap_pgprot(struct device *dev, pgprot_t prot, - unsigned long attrs) -{ - if (!dev_is_dma_coherent(dev)) - return pgprot_noncached(prot); - return prot; -} diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c index a7f9c3edbcb2..0015fe610b23 100644 --- a/drivers/iommu/dma-iommu.c +++ b/drivers/iommu/dma-iommu.c @@ -574,7 +574,7 @@ static void *iommu_dma_alloc_remap(struct device *dev, size_t size, struct iova_domain *iovad = &cookie->iovad; bool coherent = dev_is_dma_coherent(dev); int ioprot = dma_info_to_prot(DMA_BIDIRECTIONAL, coherent, attrs); - pgprot_t prot = arch_dma_mmap_pgprot(dev, PAGE_KERNEL, attrs); + pgprot_t prot = dma_pgprot(dev, PAGE_KERNEL, attrs); unsigned int count, min_size, alloc_sizes = domain->pgsize_bitmap; struct page **pages; struct sg_table sgt; @@ -975,7 +975,7 @@ static void *iommu_dma_alloc_pages(struct device *dev, size_t size, return NULL; if (IS_ENABLED(CONFIG_DMA_REMAP) && (!coherent || PageHighMem(page))) { - pgprot_t prot = arch_dma_mmap_pgprot(dev, PAGE_KERNEL, attrs); + pgprot_t prot = dma_pgprot(dev, PAGE_KERNEL, attrs); cpu_addr = dma_common_contiguous_remap(page, alloc_size, VM_USERMAP, prot, __builtin_return_address(0)); @@ -1035,7 +1035,7 @@ static int iommu_dma_mmap(struct device *dev, struct vm_area_struct *vma, unsigned long pfn, off = vma->vm_pgoff; int ret;
[PATCH 2/2] MIPS: remove support for DMA_ATTR_WRITE_COMBINE
Mips uses the KSEG1 kernel memory segment do map dma coherent allocations for non-coherent devices as uncachable, and does not have any kind of special support for DMA_ATTR_WRITE_COMBINE in the allocation path. Thus supporting DMA_ATTR_WRITE_COMBINE in dma_mmap_attrs will lead to multiple mappings with different caching attributes. Fixes: 8c172467be36 ("MIPS: Add implementation of dma_map_ops.mmap()") Signed-off-by: Christoph Hellwig --- arch/mips/Kconfig | 1 - arch/mips/mm/dma-noncoherent.c | 8 2 files changed, 9 deletions(-) diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig index d50fafd7bf3a..86e6760ef0d0 100644 --- a/arch/mips/Kconfig +++ b/arch/mips/Kconfig @@ -1119,7 +1119,6 @@ config DMA_PERDEV_COHERENT config DMA_NONCOHERENT bool - select ARCH_HAS_DMA_MMAP_PGPROT select ARCH_HAS_SYNC_DMA_FOR_DEVICE select ARCH_HAS_UNCACHED_SEGMENT select NEED_DMA_MAP_STATE diff --git a/arch/mips/mm/dma-noncoherent.c b/arch/mips/mm/dma-noncoherent.c index ed56c6fa7be2..1d4d57dd9acf 100644 --- a/arch/mips/mm/dma-noncoherent.c +++ b/arch/mips/mm/dma-noncoherent.c @@ -65,14 +65,6 @@ long arch_dma_coherent_to_pfn(struct device *dev, void *cpu_addr, return page_to_pfn(virt_to_page(cached_kernel_address(cpu_addr))); } -pgprot_t arch_dma_mmap_pgprot(struct device *dev, pgprot_t prot, - unsigned long attrs) -{ - if (attrs & DMA_ATTR_WRITE_COMBINE) - return pgprot_writecombine(prot); - return pgprot_noncached(prot); -} - static inline void dma_sync_virt(void *addr, size_t size, enum dma_data_direction dir) { -- 2.20.1
fix default dma_mmap_* pgprot v2
Hi all, As Shawn pointed out we've had issues with the dma mmap pgprots ever since the dma_common_mmap helper was added beyong the initial architectures - we default to uncached mappings, but for devices that are DMA coherent, or if the DMA_ATTR_NON_CONSISTENT is set (and supported) this can lead to aliasing of cache attributes. This patch fixes that. My explanation of why this hasn't been much of an issue is that the dma_mmap_ helpers aren't used widely and mostly just in architecture specific drivers. Changes since v1: - fix handling of DMA_ATTR_NON_CONSISTENT where it is a no-op (which is most architectures) - remove DMA_ATTR_WRITE_COMBINE on mips, as it seem dangerous as-is ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] arm: initialize the dma-remap atomic pool for LPAE configs
When we use the generic dma-direct + remap code we also need to initialize the atomic pool that is used for GFP_ATOMIC allocations on non-coherent devices. Fixes: ad3c7b18c5b3 ("arm: use swiotlb for bounce buffering on LPAE configs") Signed-off-by: Christoph Hellwig --- arch/arm/mm/dma-mapping.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index 6774b03aa405..e509365cc9ca 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -2423,4 +2423,10 @@ void arch_dma_free(struct device *dev, size_t size, void *cpu_addr, { __arm_dma_free(dev, size, cpu_addr, dma_handle, attrs, false); } + +static int __init atomic_pool_init(void) +{ + return dma_atomic_pool_init(GFP_DMA, pgprot_noncached(PAGE_KERNEL)); +} +postcore_initcall(atomic_pool_init); #endif /* CONFIG_SWIOTLB */ -- 2.20.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
arm swiotlb fix
Hi all, my commit to add swiotlb to arm failed to initialize the atomic pool, which is needed for GFP_ATOMIC allocations on non-coherent devices. These are fairly rare, but exist so we should wire it up. For 5.4 I plan to move the initilization to the common dma-remap code so it won't be missed for other architectures. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu