Re: [PATCH 2/3] iommu/vt-d: Apply per-device dma_ops

2019-08-05 Thread Christoph Hellwig
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_*

2019-08-05 Thread Takashi Iwai
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_*

2019-08-05 Thread Christoph Hellwig
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

2019-08-05 Thread Christoph Hellwig
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

2019-08-05 Thread Thiago Jung Bauermann
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()

2019-08-05 Thread Thiago Jung Bauermann
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

2019-08-05 Thread Thiago Jung Bauermann
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()

2019-08-05 Thread Thiago Jung Bauermann
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

2019-08-05 Thread Thiago Jung Bauermann
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

2019-08-05 Thread Thiago Jung Bauermann
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

2019-08-05 Thread Thiago Jung Bauermann
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'

2019-08-05 Thread Xiongfeng Wang
+ (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

2019-08-05 Thread Lu Baolu
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

2019-08-05 Thread Lu Baolu
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()

2019-08-05 Thread Lu Baolu

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

2019-08-05 Thread Jacob Pan
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

2019-08-05 Thread Bjorn Andersson
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

2019-08-05 Thread Bjorn Andersson
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

2019-08-05 Thread Jacob Pan
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

2019-08-05 Thread Jacob Pan
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

2019-08-05 Thread Rob Herring
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

2019-08-05 Thread Nicolas Saenz Julienne
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

2019-08-05 Thread Lucas Stach
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

2019-08-05 Thread Lucas Stach
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

2019-08-05 Thread Chuhong Yuan
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

2019-08-05 Thread Chuhong Yuan
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

2019-08-05 Thread Tomasz Figa
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_*

2019-08-05 Thread Takashi Iwai
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

2019-08-05 Thread Christoph Hellwig
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_*

2019-08-05 Thread Christoph Hellwig
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

2019-08-05 Thread Christoph Hellwig
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

2019-08-05 Thread Christoph Hellwig
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

2019-08-05 Thread Christoph Hellwig
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

2019-08-05 Thread Christoph Hellwig
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

2019-08-05 Thread Christoph Hellwig
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

2019-08-05 Thread Christoph Hellwig
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_*

2019-08-05 Thread Catalin Marinas
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

2019-08-05 Thread Sergei Shtylyov

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_*

2019-08-05 Thread Christoph Hellwig
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

2019-08-05 Thread Christoph Hellwig
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

2019-08-05 Thread Christoph Hellwig
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

2019-08-05 Thread Christoph Hellwig
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

2019-08-05 Thread Christoph Hellwig
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