[PATCH v1 1/3] iommu/vt-d: Using pasid_pte_is_present() helper function

2021-08-16 Thread Liu Yi L
Use pasid_pte_is_present() for present bit check in 
intel_pasid_tear_down_entry().

Cc: Lu Baolu 
Signed-off-by: Liu Yi L 
---
 drivers/iommu/intel/pasid.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
index c6cf44a6c923..02e10491184b 100644
--- a/drivers/iommu/intel/pasid.c
+++ b/drivers/iommu/intel/pasid.c
@@ -517,7 +517,7 @@ void intel_pasid_tear_down_entry(struct intel_iommu *iommu, 
struct device *dev,
if (WARN_ON(!pte))
return;
 
-   if (!(pte->val[0] & PASID_PTE_PRESENT))
+   if (!pasid_pte_is_present(pte))
return;
 
did = pasid_get_domain_id(pte);
-- 
2.25.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH v1 3/3] iommu/vt-d: Fix Unexpected page request in Privilege Mode

2021-08-16 Thread Liu Yi L
This patch fixes the below error. This is due to improper iotlb invalidation
in intel_pasid_tear_down_entry().

[  180.187556] Unexpected page request in Privilege Mode
[  180.187565] Unexpected page request in Privilege Mode
[  180.279933] Unexpected page request in Privilege Mode
[  180.279937] Unexpected page request in Privilege Mode

Per chapter 6.5.3.3 of VT-d spec 3.3, when tear down a pasid entry, software
should use Domain selective IOTLB flush if the PGTT of the pasid entry is
SL only or Nested, while for the pasid entries whose PGTT is FL only or PT
using PASID-based IOTLB flush is enough.

Fixes: 1c4f88b7f1f9 ("iommu/vt-d: Shared virtual address in scalable mode")
Cc: Lu Baolu 
Signed-off-by: Kumar Sanjay K 
Signed-off-by: Liu Yi L 
Tested-by: Yi Sun 
---
 drivers/iommu/intel/pasid.c | 10 --
 drivers/iommu/intel/pasid.h |  5 +
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
index b1d0c2945c9a..07c390aed1fe 100644
--- a/drivers/iommu/intel/pasid.c
+++ b/drivers/iommu/intel/pasid.c
@@ -511,7 +511,7 @@ void intel_pasid_tear_down_entry(struct intel_iommu *iommu, 
struct device *dev,
 u32 pasid, bool fault_ignore)
 {
struct pasid_entry *pte;
-   u16 did;
+   u16 did, pgtt;
 
pte = intel_pasid_get_entry(dev, pasid);
if (WARN_ON(!pte))
@@ -521,13 +521,19 @@ void intel_pasid_tear_down_entry(struct intel_iommu 
*iommu, struct device *dev,
return;
 
did = pasid_get_domain_id(pte);
+   pgtt = pasid_pte_get_pgtt(pte);
+
intel_pasid_clear_entry(dev, pasid, fault_ignore);
 
if (!ecap_coherent(iommu->ecap))
clflush_cache_range(pte, sizeof(*pte));
 
pasid_cache_invalidation_with_pasid(iommu, did, pasid);
-   qi_flush_piotlb(iommu, did, pasid, 0, -1, 0);
+
+   if (pgtt == PASID_ENTRY_PGTT_PT || pgtt == PASID_ENTRY_PGTT_FL_ONLY)
+   qi_flush_piotlb(iommu, did, pasid, 0, -1, 0);
+   else
+   iommu->flush.flush_iotlb(iommu, did, 0, 0, DMA_TLB_DSI_FLUSH);
 
/* Device IOTLB doesn't need to be flushed in caching mode. */
if (!cap_caching_mode(iommu->cap))
diff --git a/drivers/iommu/intel/pasid.h b/drivers/iommu/intel/pasid.h
index 5ff61c3d401f..637141d71092 100644
--- a/drivers/iommu/intel/pasid.h
+++ b/drivers/iommu/intel/pasid.h
@@ -99,6 +99,11 @@ static inline bool pasid_pte_is_present(struct pasid_entry 
*pte)
return READ_ONCE(pte->val[0]) & PASID_PTE_PRESENT;
 }
 
+static inline u16 pasid_pte_get_pgtt(struct pasid_entry *pte)
+{
+   return (READ_ONCE(pte->val[0]) >> 6) & 0x7;
+}
+
 extern unsigned int intel_pasid_max_id;
 int intel_pasid_alloc_table(struct device *dev);
 void intel_pasid_free_table(struct device *dev);
-- 
2.25.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH v1 2/3] iommu/vt-d: Add present bit check in pasid entry setup helper functions

2021-08-16 Thread Liu Yi L
The helper functions is not capable to modify the pasid entries which
are still in use. So should have a check against present bit.

Cc: Lu Baolu 
Signed-off-by: Liu Yi L 
---
 drivers/iommu/intel/pasid.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
index 02e10491184b..b1d0c2945c9a 100644
--- a/drivers/iommu/intel/pasid.c
+++ b/drivers/iommu/intel/pasid.c
@@ -534,6 +534,10 @@ void intel_pasid_tear_down_entry(struct intel_iommu 
*iommu, struct device *dev,
devtlb_invalidation_with_pasid(iommu, dev, pasid);
 }
 
+/*
+ * This function flushes cache for a newly setup pasid table entry.
+ * Caller of it should not modify the in-use pasid table entries.
+ */
 static void pasid_flush_caches(struct intel_iommu *iommu,
struct pasid_entry *pte,
   u32 pasid, u16 did)
@@ -585,6 +589,10 @@ int intel_pasid_setup_first_level(struct intel_iommu 
*iommu,
if (WARN_ON(!pte))
return -EINVAL;
 
+   /* Caller must ensure PASID entry is not in use. */
+   if (pasid_pte_is_present(pte))
+   return -EBUSY;
+
pasid_clear_entry(pte);
 
/* Setup the first level page table pointer: */
@@ -684,6 +692,10 @@ int intel_pasid_setup_second_level(struct intel_iommu 
*iommu,
return -ENODEV;
}
 
+   /* Caller must ensure PASID entry is not in use. */
+   if (pasid_pte_is_present(pte))
+   return -EBUSY;
+
pasid_clear_entry(pte);
pasid_set_domain_id(pte, did);
pasid_set_slptr(pte, pgd_val);
@@ -723,6 +735,10 @@ int intel_pasid_setup_pass_through(struct intel_iommu 
*iommu,
return -ENODEV;
}
 
+   /* Caller must ensure PASID entry is not in use. */
+   if (pasid_pte_is_present(pte))
+   return -EBUSY;
+
pasid_clear_entry(pte);
pasid_set_domain_id(pte, did);
pasid_set_address_width(pte, iommu->agaw);
-- 
2.25.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH v1 0/3] Misc fixes to intel iommu driver

2021-08-16 Thread Liu Yi L
Hi,

This series includes two minor enhancements and one bug fix. Please have
a review.

Thanks,
Yi Liu
---

Liu Yi L (3):
  iommu/vt-d: Using pasid_pte_is_present() helper function
  iommu/vt-d: Add present bit check in pasid entry setup helper
functions
  iommu/vt-d: Fix Unexpected page request in Privilege Mode

 drivers/iommu/intel/pasid.c | 28 +---
 drivers/iommu/intel/pasid.h |  5 +
 2 files changed, 30 insertions(+), 3 deletions(-)

-- 
2.25.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [RFC 0/5] Support non-coherent DMA on RISC-V using a global pool

2021-08-16 Thread Atish Patra
On Mon, Aug 16, 2021 at 6:37 PM Guo Ren  wrote:
>
> 1
>
> On Thu, Jul 29, 2021 at 2:19 PM Atish Patra  wrote:
> >
> > On Wed, Jul 28, 2021 at 9:30 PM Palmer Dabbelt  wrote:
> > >
> > > On Fri, 23 Jul 2021 14:40:26 PDT (-0700), Atish Patra wrote:
> > > > RISC-V privilege specification doesn't define an IOMMU or any method 
> > > > modify
> > > > PMA attributes or PTE entries to allow non-coherent mappings yet. In
> > > > the beginning, this approach was adopted assuming that most of the 
> > > > RISC-V
> > > > platforms would support full cache-coherent IO. Here is excerpt from the
> > > > priv spec section 3.6.5
> > > >
> > > > "In RISC-V platforms, the use of hardware-incoherent regions is 
> > > > discouraged
> > > > due to software complexity, performance, and energy impacts."
> > > >
> > > > While some of the RISC-V platforms adhere to the above suggestion, not 
> > > > all
> > > > platforms can afford to build to fully cache-coherent I/O devices. To
> > > > address DMA for non-coherent I/O devices, we need to mark a region of 
> > > > memory
> > > > as non-cacheable. Some of the platforms rely on a fixed region of 
> > > > uncached
> > > > memory that is remapped to the system memory while some other modify
> > > > the PTE entries to achieve that.
> > > >
> > > > The patch3 solves the issue for the fist use case by using a global
> > > > pool of memory that is reserved for DMA. The device access the reserved 
> > > > area
> > > > of the region while corresponding CPU address will be from uncached 
> > > > region
> > > > As the uncached region is remapped to the beginning of the system ram, 
> > > > both
> > > > CPU and device get the same view.
> > > >
> > > > To facilitate streaming DMA APIs, patch 1 introduces a set of generic
> > > > cache operations. Any platform can use the generic ops to provide 
> > > > platform
> > > > specific cache management operations. Once the standard RISC-V CMO 
> > > > extension
> > > > is available, it will also use these generic ops.
> > > >
> > > > To address the second use case, Page Based Memory Attribute (PBMT) 
> > > > extension
> > > > is proposed. Once the extension is in good shape, we can leverage that
> > > > using CONFIG_DIRECT_REMAP. Currently, it is selected via a compile time 
> > > > config
> > > > option. We will probably need another arch specific hooks to know if the
> > > > platform supports direct remap at runtime. For RISC-V, it will check the
> > > > presence of PBMT extension while other arch function will simply return 
> > > > true
> > >
> > > IIUC this is another extension that's not yet frozen or implemented in
> > > hardware?  Is this one compatible with what's in the c906, or is it
> > > doing things its own way?
> >
> > Hi Palmer,
> > This series doesn't implement the PBMT extension which is still in 
> > discussion.
> > It simply reuse the existing non-coherent dma mapping support in
> > upstream kernel and enable
> > it for RISC-V. The current version uses a non-coherent global pool. I
> > will update it to use arch_set_uncached
> > as per Christoph's suggestion. It solves the non-coherent DMA problem
> > for beagleV and not c906.
> >
> > I briefly mentioned the PBMT extension just to provide an idea how the
> > RISC-V Linux kernel
> > can support both unached window and PBMT extension. PBMT extension is
> > planned to be frozen
> > by the end of this year and none of the hardware has implemented it.
> >
> > The implementation in c906 is a non-standard one and will not be
> > supported by the default PBMT
> > extension implementation.
> The default PBMT & c908 PBMT are similar, only BIT definitions are
> different. I propose to support default PBMT first and give the back
> door to modify the PBMT definition during boot.
> The "protection_map[] = (__P000, __P001 ..__S000, __S001)" in
> mm/mmap.c has been modified by arch/mips, arm, sparc, x86, so I think
> it's proper solution direction.
>
> The reset problem is how to passing custom PBMT at the very early boot
> stage. Now I'm trying to use the DTS parsing instead of boot param hdr
> which Anup objected to.
>

IIRC, c906 has a compatible mode that has the compliant PTE bit modifications.
Can you use that mode in the Allwinner D1 board to boot Linux ? I am
not sure if you have any fallback method for non-coherent DMA
if custom DMA_COHERENT bits are not enabled through enhanced mode ?

> ref: 
> https://lore.kernel.org/linux-riscv/1623693067-53886-1-git-send-email-guo...@kernel.org/
>
> Any comments are welcome.
>
> >
> >
> > >
> > > > if DIRECT_REMAP is enabled. This is required as arious different config
> > > > (DIRECT_REMAP, GLOBAL_POOL) will be enabled in the defconfig so that a
> > > > unified kernel image can boot on all RISC-V platforms.
> > > >
> > > > This patch series depends on Christoph's global pool support series[1].
> > > > Tested on Qemu, HiFive unleashed and beagleV. This series is also 
> > > > available
> > > > at [2].
> > > > This series also solves the 

Re: [RFC 1/5] RISC-V: Implement arch_sync_dma* functions

2021-08-16 Thread Atish Patra
On Mon, Aug 16, 2021 at 6:48 PM Guo Ren  wrote:
>
> On Sat, Jul 24, 2021 at 5:40 AM Atish Patra  wrote:
> >
> > To facilitate streaming DMA APIs, this patch introduces a set of generic
> > cache operations related dma sync. Any platform can use the generic ops
> > to provide platform specific cache management operations. Once the
> > standard RISC-V CMO extension is available, it can be built on top of it.
> >
> > Signed-off-by: Atish Patra 
> > ---
> >  arch/riscv/include/asm/dma-noncoherent.h | 19 +++
> >  arch/riscv/mm/Makefile   |  1 +
> >  arch/riscv/mm/dma-noncoherent.c  | 66 
> >  3 files changed, 86 insertions(+)
> >  create mode 100644 arch/riscv/include/asm/dma-noncoherent.h
> >  create mode 100644 arch/riscv/mm/dma-noncoherent.c
> >
> > diff --git a/arch/riscv/include/asm/dma-noncoherent.h 
> > b/arch/riscv/include/asm/dma-noncoherent.h
> > new file mode 100644
> > index ..5bdb03c9c427
> > --- /dev/null
> > +++ b/arch/riscv/include/asm/dma-noncoherent.h
> > @@ -0,0 +1,19 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Copyright (c) 2021 Western Digital Corporation or its affiliates.
> > + */
> > +
> > +#ifndef __ASM_RISCV_DMA_NON_COHERENT_H
> > +#define __ASM_RISCV_DMA_NON_COHERENT_H
> > +
> > +#ifdef CONFIG_RISCV_DMA_NONCOHERENT
> > +struct riscv_dma_cache_sync {
> > +   void (*cache_invalidate)(phys_addr_t paddr, size_t size);
> > +   void (*cache_clean)(phys_addr_t paddr, size_t size);
> > +   void (*cache_flush)(phys_addr_t paddr, size_t size);
> > +};
> I like the style like this than my previous patch which using
> sbi_call. The c906 has custom instructions that could be called in
> S-mode directly.
>

How are you going to include the custom instructions in the upstream kernel ?

> Hope the patch could be soon merged, after correct the
> DMA_FROM/TO_DEVICE/BIDIRECTIONAL and alternatives ops_set.
>
> > +
> > +void riscv_dma_cache_sync_set(struct riscv_dma_cache_sync *ops);
> > +#endif
> > +
> > +#endif
> > diff --git a/arch/riscv/mm/Makefile b/arch/riscv/mm/Makefile
> > index 7ebaef10ea1b..959bef49098b 100644
> > --- a/arch/riscv/mm/Makefile
> > +++ b/arch/riscv/mm/Makefile
> > @@ -27,3 +27,4 @@ KASAN_SANITIZE_init.o := n
> >  endif
> >
> >  obj-$(CONFIG_DEBUG_VIRTUAL) += physaddr.o
> > +obj-$(CONFIG_RISCV_DMA_NONCOHERENT) += dma-noncoherent.o
> > diff --git a/arch/riscv/mm/dma-noncoherent.c 
> > b/arch/riscv/mm/dma-noncoherent.c
> > new file mode 100644
> > index ..2f6e9627c4aa
> > --- /dev/null
> > +++ b/arch/riscv/mm/dma-noncoherent.c
> > @@ -0,0 +1,66 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * RISC-V specific functions to support DMA for non-coherent devices
> > + *
> > + * Copyright (c) 2021 Western Digital Corporation or its affiliates.
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +static struct riscv_dma_cache_sync *dma_cache_sync;
> > +unsigned long riscv_dma_uc_offset;
> > +
> > +static void __dma_sync(phys_addr_t paddr, size_t size, enum 
> > dma_data_direction dir)
> > +{
> > +   if ((dir == DMA_FROM_DEVICE) && (dma_cache_sync->cache_invalidate))
> > +   dma_cache_sync->cache_invalidate(paddr, size);
> > +   else if ((dir == DMA_TO_DEVICE) && (dma_cache_sync->cache_clean))
> > +   dma_cache_sync->cache_clean(paddr, size);
> > +   else if ((dir == DMA_BIDIRECTIONAL) && dma_cache_sync->cache_flush)
> > +   dma_cache_sync->cache_flush(paddr, size);
> > +}
> > +
> > +void arch_sync_dma_for_device(phys_addr_t paddr, size_t size, enum 
> > dma_data_direction dir)
> > +{
> > +   if (!dma_cache_sync)
> > +   return;
> > +
> > +   __dma_sync(paddr, size, dir);
> > +}
> > +
> > +void arch_sync_dma_for_cpu(phys_addr_t paddr, size_t size, enum 
> > dma_data_direction dir)
> > +{
> > +   if (!dma_cache_sync)
> > +   return;
> > +
> > +   __dma_sync(paddr, size, dir);
> > +}
> > +
> > +void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
> > +   const struct iommu_ops *iommu, bool coherent)
> > +{
> > +   /* If a specific device is dma-coherent, set it here */
> > +   dev->dma_coherent = coherent;
> > +}
> > +
> > +void arch_dma_prep_coherent(struct page *page, size_t size)
> > +{
> > +   void *flush_addr = page_address(page);
> > +
> > +   memset(flush_addr, 0, size);
> > +   if (dma_cache_sync && dma_cache_sync->cache_flush)
> > +   dma_cache_sync->cache_flush(__pa(flush_addr), size);
> > +}
> > +
> > +void riscv_dma_cache_sync_set(struct riscv_dma_cache_sync *ops)
> > +{
> > +   dma_cache_sync = ops;
> > +}
> > --
> > 2.31.1
> >
> > ___
> > iommu mailing list
> > iommu@lists.linux-foundation.org
> > https://lists.linuxfoundation.org/mailman/listinfo/iommu
>

Re: [RFC 1/5] RISC-V: Implement arch_sync_dma* functions

2021-08-16 Thread Guo Ren
On Sat, Jul 24, 2021 at 5:40 AM Atish Patra  wrote:
>
> To facilitate streaming DMA APIs, this patch introduces a set of generic
> cache operations related dma sync. Any platform can use the generic ops
> to provide platform specific cache management operations. Once the
> standard RISC-V CMO extension is available, it can be built on top of it.
>
> Signed-off-by: Atish Patra 
> ---
>  arch/riscv/include/asm/dma-noncoherent.h | 19 +++
>  arch/riscv/mm/Makefile   |  1 +
>  arch/riscv/mm/dma-noncoherent.c  | 66 
>  3 files changed, 86 insertions(+)
>  create mode 100644 arch/riscv/include/asm/dma-noncoherent.h
>  create mode 100644 arch/riscv/mm/dma-noncoherent.c
>
> diff --git a/arch/riscv/include/asm/dma-noncoherent.h 
> b/arch/riscv/include/asm/dma-noncoherent.h
> new file mode 100644
> index ..5bdb03c9c427
> --- /dev/null
> +++ b/arch/riscv/include/asm/dma-noncoherent.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (c) 2021 Western Digital Corporation or its affiliates.
> + */
> +
> +#ifndef __ASM_RISCV_DMA_NON_COHERENT_H
> +#define __ASM_RISCV_DMA_NON_COHERENT_H
> +
> +#ifdef CONFIG_RISCV_DMA_NONCOHERENT
> +struct riscv_dma_cache_sync {
> +   void (*cache_invalidate)(phys_addr_t paddr, size_t size);
> +   void (*cache_clean)(phys_addr_t paddr, size_t size);
> +   void (*cache_flush)(phys_addr_t paddr, size_t size);
> +};
I like the style like this than my previous patch which using
sbi_call. The c906 has custom instructions that could be called in
S-mode directly.

Hope the patch could be soon merged, after correct the
DMA_FROM/TO_DEVICE/BIDIRECTIONAL and alternatives ops_set.

> +
> +void riscv_dma_cache_sync_set(struct riscv_dma_cache_sync *ops);
> +#endif
> +
> +#endif
> diff --git a/arch/riscv/mm/Makefile b/arch/riscv/mm/Makefile
> index 7ebaef10ea1b..959bef49098b 100644
> --- a/arch/riscv/mm/Makefile
> +++ b/arch/riscv/mm/Makefile
> @@ -27,3 +27,4 @@ KASAN_SANITIZE_init.o := n
>  endif
>
>  obj-$(CONFIG_DEBUG_VIRTUAL) += physaddr.o
> +obj-$(CONFIG_RISCV_DMA_NONCOHERENT) += dma-noncoherent.o
> diff --git a/arch/riscv/mm/dma-noncoherent.c b/arch/riscv/mm/dma-noncoherent.c
> new file mode 100644
> index ..2f6e9627c4aa
> --- /dev/null
> +++ b/arch/riscv/mm/dma-noncoherent.c
> @@ -0,0 +1,66 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * RISC-V specific functions to support DMA for non-coherent devices
> + *
> + * Copyright (c) 2021 Western Digital Corporation or its affiliates.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +static struct riscv_dma_cache_sync *dma_cache_sync;
> +unsigned long riscv_dma_uc_offset;
> +
> +static void __dma_sync(phys_addr_t paddr, size_t size, enum 
> dma_data_direction dir)
> +{
> +   if ((dir == DMA_FROM_DEVICE) && (dma_cache_sync->cache_invalidate))
> +   dma_cache_sync->cache_invalidate(paddr, size);
> +   else if ((dir == DMA_TO_DEVICE) && (dma_cache_sync->cache_clean))
> +   dma_cache_sync->cache_clean(paddr, size);
> +   else if ((dir == DMA_BIDIRECTIONAL) && dma_cache_sync->cache_flush)
> +   dma_cache_sync->cache_flush(paddr, size);
> +}
> +
> +void arch_sync_dma_for_device(phys_addr_t paddr, size_t size, enum 
> dma_data_direction dir)
> +{
> +   if (!dma_cache_sync)
> +   return;
> +
> +   __dma_sync(paddr, size, dir);
> +}
> +
> +void arch_sync_dma_for_cpu(phys_addr_t paddr, size_t size, enum 
> dma_data_direction dir)
> +{
> +   if (!dma_cache_sync)
> +   return;
> +
> +   __dma_sync(paddr, size, dir);
> +}
> +
> +void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
> +   const struct iommu_ops *iommu, bool coherent)
> +{
> +   /* If a specific device is dma-coherent, set it here */
> +   dev->dma_coherent = coherent;
> +}
> +
> +void arch_dma_prep_coherent(struct page *page, size_t size)
> +{
> +   void *flush_addr = page_address(page);
> +
> +   memset(flush_addr, 0, size);
> +   if (dma_cache_sync && dma_cache_sync->cache_flush)
> +   dma_cache_sync->cache_flush(__pa(flush_addr), size);
> +}
> +
> +void riscv_dma_cache_sync_set(struct riscv_dma_cache_sync *ops)
> +{
> +   dma_cache_sync = ops;
> +}
> --
> 2.31.1
>
> ___
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu



-- 
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH v6 7/7] dma-iommu: account for min_align_mask

2021-08-16 Thread David Stevens
From: David Stevens 

For devices which set min_align_mask, swiotlb preserves the offset of
the original physical address within that mask. Since __iommu_dma_map
accounts for non-aligned addresses, passing a non-aligned swiotlb
address with the swiotlb aligned size results in the offset being
accounted for twice in the size passed to iommu_map_atomic. The extra
page exposed to DMA is also not cleaned up by __iommu_dma_unmap, since
that function unmaps with the correct size. This causes mapping failures
if the iova gets reused, due to collisions in the iommu page tables.

To fix this, pass the original size to __iommu_dma_map, since that
function already handles alignment.

Additionally, when swiotlb returns non-aligned addresses, there is
padding at the start of the bounce buffer that needs to be cleared.

Fixes: 1f221a0d0dbf ("swiotlb: respect min_align_mask")
Signed-off-by: David Stevens 
---
 drivers/iommu/dma-iommu.c | 24 +---
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 6738420fc081..f2fb360c2907 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -788,7 +788,6 @@ static dma_addr_t iommu_dma_map_page(struct device *dev, 
struct page *page,
struct iommu_domain *domain = iommu_get_dma_domain(dev);
struct iommu_dma_cookie *cookie = domain->iova_cookie;
struct iova_domain *iovad = >iovad;
-   size_t aligned_size = size;
dma_addr_t iova, dma_mask = dma_get_mask(dev);
 
/*
@@ -796,8 +795,8 @@ static dma_addr_t iommu_dma_map_page(struct device *dev, 
struct page *page,
 * page aligned, we don't need to use a bounce page.
 */
if (dev_use_swiotlb(dev) && iova_offset(iovad, phys | size)) {
-   void *padding_start;
-   size_t padding_size;
+   void *tlb_start;
+   size_t aligned_size, iova_off, mapping_end_off;
 
aligned_size = iova_align(iovad, size);
phys = swiotlb_tbl_map_single(dev, phys, size, aligned_size,
@@ -806,23 +805,26 @@ static dma_addr_t iommu_dma_map_page(struct device *dev, 
struct page *page,
if (phys == DMA_MAPPING_ERROR)
return DMA_MAPPING_ERROR;
 
-   /* Cleanup the padding area. */
-   padding_start = phys_to_virt(phys);
-   padding_size = aligned_size;
+   iova_off = iova_offset(iovad, phys);
+   tlb_start = phys_to_virt(phys - iova_off);
 
if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC) &&
(dir == DMA_TO_DEVICE || dir == DMA_BIDIRECTIONAL)) {
-   padding_start += size;
-   padding_size -= size;
+   /* Cleanup the padding area. */
+   mapping_end_off = iova_off + size;
+   memset(tlb_start, 0, iova_off);
+   memset(tlb_start + mapping_end_off, 0,
+  aligned_size - mapping_end_off);
+   } else {
+   /* Nothing was sync'ed, so clear the whole buffer. */
+   memset(tlb_start, 0, aligned_size);
}
-
-   memset(padding_start, 0, padding_size);
}
 
if (!coherent && !(attrs & DMA_ATTR_SKIP_CPU_SYNC))
arch_sync_dma_for_device(phys, size, dir);
 
-   iova = __iommu_dma_map(dev, phys, aligned_size, prot, dma_mask);
+   iova = __iommu_dma_map(dev, phys, size, prot, dma_mask);
if (iova == DMA_MAPPING_ERROR && is_swiotlb_buffer(phys))
swiotlb_tbl_unmap_single(dev, phys, size, dir, attrs);
return iova;
-- 
2.33.0.rc1.237.g0d66db33f3-goog

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH v6 6/7] swiotlb: support aligned swiotlb buffers

2021-08-16 Thread David Stevens
From: David Stevens 

Add an argument to swiotlb_tbl_map_single that specifies the desired
alignment of the allocated buffer. This is used by dma-iommu to ensure
the buffer is aligned to the iova granule size when using swiotlb with
untrusted sub-granule mappings. This addresses an issue where adjacent
slots could be exposed to the untrusted device if IO_TLB_SIZE < iova
granule < PAGE_SIZE.

Signed-off-by: David Stevens 
Reviewed-by: Christoph Hellwig 
---
 drivers/iommu/dma-iommu.c |  4 ++--
 drivers/xen/swiotlb-xen.c |  2 +-
 include/linux/swiotlb.h   |  3 ++-
 kernel/dma/swiotlb.c  | 11 +++
 4 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 49a0d4de5f6c..6738420fc081 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -800,8 +800,8 @@ static dma_addr_t iommu_dma_map_page(struct device *dev, 
struct page *page,
size_t padding_size;
 
aligned_size = iova_align(iovad, size);
-   phys = swiotlb_tbl_map_single(dev, phys, size,
- aligned_size, dir, attrs);
+   phys = swiotlb_tbl_map_single(dev, phys, size, aligned_size,
+ iova_mask(iovad), dir, attrs);
 
if (phys == DMA_MAPPING_ERROR)
return DMA_MAPPING_ERROR;
diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index 24d11861ac7d..8b03d2c93428 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -382,7 +382,7 @@ static dma_addr_t xen_swiotlb_map_page(struct device *dev, 
struct page *page,
 */
trace_swiotlb_bounced(dev, dev_addr, size, swiotlb_force);
 
-   map = swiotlb_tbl_map_single(dev, phys, size, size, dir, attrs);
+   map = swiotlb_tbl_map_single(dev, phys, size, size, 0, dir, attrs);
if (map == (phys_addr_t)DMA_MAPPING_ERROR)
return DMA_MAPPING_ERROR;
 
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index 216854a5e513..93d82e43eb3a 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -44,7 +44,8 @@ extern void __init swiotlb_update_mem_attributes(void);
 
 phys_addr_t swiotlb_tbl_map_single(struct device *hwdev, phys_addr_t phys,
size_t mapping_size, size_t alloc_size,
-   enum dma_data_direction dir, unsigned long attrs);
+   unsigned int alloc_aligned_mask, enum dma_data_direction dir,
+   unsigned long attrs);
 
 extern void swiotlb_tbl_unmap_single(struct device *hwdev,
 phys_addr_t tlb_addr,
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index e50df8d8f87e..d4c45d8cd1fa 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -427,7 +427,7 @@ static unsigned int wrap_index(struct io_tlb_mem *mem, 
unsigned int index)
  * allocate a buffer from that IO TLB pool.
  */
 static int find_slots(struct device *dev, phys_addr_t orig_addr,
-   size_t alloc_size)
+   size_t alloc_size, unsigned int alloc_align_mask)
 {
struct io_tlb_mem *mem = io_tlb_default_mem;
unsigned long boundary_mask = dma_get_seg_boundary(dev);
@@ -450,6 +450,7 @@ static int find_slots(struct device *dev, phys_addr_t 
orig_addr,
stride = (iotlb_align_mask >> IO_TLB_SHIFT) + 1;
if (alloc_size >= PAGE_SIZE)
stride = max(stride, stride << (PAGE_SHIFT - IO_TLB_SHIFT));
+   stride = max(stride, (alloc_align_mask >> IO_TLB_SHIFT) + 1);
 
spin_lock_irqsave(>lock, flags);
if (unlikely(nslots > mem->nslabs - mem->used))
@@ -504,7 +505,8 @@ static int find_slots(struct device *dev, phys_addr_t 
orig_addr,
 
 phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr,
size_t mapping_size, size_t alloc_size,
-   enum dma_data_direction dir, unsigned long attrs)
+   unsigned int alloc_align_mask, enum dma_data_direction dir,
+   unsigned long attrs)
 {
struct io_tlb_mem *mem = io_tlb_default_mem;
unsigned int offset = swiotlb_align_offset(dev, orig_addr);
@@ -524,7 +526,8 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, 
phys_addr_t orig_addr,
return (phys_addr_t)DMA_MAPPING_ERROR;
}
 
-   index = find_slots(dev, orig_addr, alloc_size + offset);
+   index = find_slots(dev, orig_addr,
+  alloc_size + offset, alloc_align_mask);
if (index == -1) {
if (!(attrs & DMA_ATTR_NO_WARN))
dev_warn_ratelimited(dev,
@@ -636,7 +639,7 @@ dma_addr_t swiotlb_map(struct device *dev, phys_addr_t 
paddr, size_t size,
trace_swiotlb_bounced(dev, phys_to_dma(dev, paddr), size,
  swiotlb_force);
 
-   swiotlb_addr = swiotlb_tbl_map_single(dev, paddr, size, size, dir,
+   swiotlb_addr = 

[PATCH v6 5/7] dma-iommu: Check CONFIG_SWIOTLB more broadly

2021-08-16 Thread David Stevens
From: David Stevens 

Introduce a new dev_use_swiotlb function to guard swiotlb code, instead
of overloading dev_is_untrusted. This allows CONFIG_SWIOTLB to be
checked more broadly, so the swiotlb related code can be removed more
aggressively.

Signed-off-by: David Stevens 
Reviewed-by: Robin Murphy 
Reviewed-by: Christoph Hellwig 
---
 drivers/iommu/dma-iommu.c | 20 
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 8152efada8b2..49a0d4de5f6c 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -317,6 +317,11 @@ static bool dev_is_untrusted(struct device *dev)
return dev_is_pci(dev) && to_pci_dev(dev)->untrusted;
 }
 
+static bool dev_use_swiotlb(struct device *dev)
+{
+   return IS_ENABLED(CONFIG_SWIOTLB) && dev_is_untrusted(dev);
+}
+
 /**
  * iommu_dma_init_domain - Initialise a DMA mapping domain
  * @domain: IOMMU domain previously prepared by iommu_get_dma_cookie()
@@ -713,7 +718,7 @@ static void iommu_dma_sync_single_for_cpu(struct device 
*dev,
 {
phys_addr_t phys;
 
-   if (dev_is_dma_coherent(dev) && !dev_is_untrusted(dev))
+   if (dev_is_dma_coherent(dev) && !dev_use_swiotlb(dev))
return;
 
phys = iommu_iova_to_phys(iommu_get_dma_domain(dev), dma_handle);
@@ -729,7 +734,7 @@ static void iommu_dma_sync_single_for_device(struct device 
*dev,
 {
phys_addr_t phys;
 
-   if (dev_is_dma_coherent(dev) && !dev_is_untrusted(dev))
+   if (dev_is_dma_coherent(dev) && !dev_use_swiotlb(dev))
return;
 
phys = iommu_iova_to_phys(iommu_get_dma_domain(dev), dma_handle);
@@ -747,7 +752,7 @@ static void iommu_dma_sync_sg_for_cpu(struct device *dev,
struct scatterlist *sg;
int i;
 
-   if (dev_is_untrusted(dev))
+   if (dev_use_swiotlb(dev))
for_each_sg(sgl, sg, nelems, i)
iommu_dma_sync_single_for_cpu(dev, sg_dma_address(sg),
  sg->length, dir);
@@ -763,7 +768,7 @@ static void iommu_dma_sync_sg_for_device(struct device *dev,
struct scatterlist *sg;
int i;
 
-   if (dev_is_untrusted(dev))
+   if (dev_use_swiotlb(dev))
for_each_sg(sgl, sg, nelems, i)
iommu_dma_sync_single_for_device(dev,
 sg_dma_address(sg),
@@ -790,8 +795,7 @@ static dma_addr_t iommu_dma_map_page(struct device *dev, 
struct page *page,
 * If both the physical buffer start address and size are
 * page aligned, we don't need to use a bounce page.
 */
-   if (IS_ENABLED(CONFIG_SWIOTLB) && dev_is_untrusted(dev) &&
-   iova_offset(iovad, phys | size)) {
+   if (dev_use_swiotlb(dev) && iova_offset(iovad, phys | size)) {
void *padding_start;
size_t padding_size;
 
@@ -974,7 +978,7 @@ static int iommu_dma_map_sg(struct device *dev, struct 
scatterlist *sg,
iommu_deferred_attach(dev, domain))
return 0;
 
-   if (dev_is_untrusted(dev))
+   if (dev_use_swiotlb(dev))
return iommu_dma_map_sg_swiotlb(dev, sg, nents, dir, attrs);
 
if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
@@ -1046,7 +1050,7 @@ static void iommu_dma_unmap_sg(struct device *dev, struct 
scatterlist *sg,
struct scatterlist *tmp;
int i;
 
-   if (dev_is_untrusted(dev)) {
+   if (dev_use_swiotlb(dev)) {
iommu_dma_unmap_sg_swiotlb(dev, sg, nents, dir, attrs);
return;
}
-- 
2.33.0.rc1.237.g0d66db33f3-goog

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH v6 4/7] dma-iommu: fold _swiotlb helpers into callers

2021-08-16 Thread David Stevens
From: David Stevens 

Fold the _swiotlb helper functions into the respective _page functions,
since recent fixes have moved all logic from the _page functions to the
_swiotlb functions.

Signed-off-by: David Stevens 
Reviewed-by: Christoph Hellwig 
---
 drivers/iommu/dma-iommu.c | 135 +-
 1 file changed, 59 insertions(+), 76 deletions(-)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 5dd2c517dbf5..8152efada8b2 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -493,26 +493,6 @@ static void __iommu_dma_unmap(struct device *dev, 
dma_addr_t dma_addr,
iommu_dma_free_iova(cookie, dma_addr, size, iotlb_gather.freelist);
 }
 
-static void __iommu_dma_unmap_swiotlb(struct device *dev, dma_addr_t dma_addr,
-   size_t size, enum dma_data_direction dir,
-   unsigned long attrs)
-{
-   struct iommu_domain *domain = iommu_get_dma_domain(dev);
-   phys_addr_t phys;
-
-   phys = iommu_iova_to_phys(domain, dma_addr);
-   if (WARN_ON(!phys))
-   return;
-
-   if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC) && !dev_is_dma_coherent(dev))
-   arch_sync_dma_for_cpu(phys, size, dir);
-
-   __iommu_dma_unmap(dev, dma_addr, size);
-
-   if (unlikely(is_swiotlb_buffer(phys)))
-   swiotlb_tbl_unmap_single(dev, phys, size, dir, attrs);
-}
-
 static dma_addr_t __iommu_dma_map(struct device *dev, phys_addr_t phys,
size_t size, int prot, u64 dma_mask)
 {
@@ -539,55 +519,6 @@ static dma_addr_t __iommu_dma_map(struct device *dev, 
phys_addr_t phys,
return iova + iova_off;
 }
 
-static dma_addr_t __iommu_dma_map_swiotlb(struct device *dev, phys_addr_t phys,
-   size_t org_size, dma_addr_t dma_mask, bool coherent,
-   enum dma_data_direction dir, unsigned long attrs)
-{
-   int prot = dma_info_to_prot(dir, coherent, attrs);
-   struct iommu_domain *domain = iommu_get_dma_domain(dev);
-   struct iommu_dma_cookie *cookie = domain->iova_cookie;
-   struct iova_domain *iovad = >iovad;
-   size_t aligned_size = org_size;
-   void *padding_start;
-   size_t padding_size;
-   dma_addr_t iova;
-
-   /*
-* If both the physical buffer start address and size are
-* page aligned, we don't need to use a bounce page.
-*/
-   if (IS_ENABLED(CONFIG_SWIOTLB) && dev_is_untrusted(dev) &&
-   iova_offset(iovad, phys | org_size)) {
-   aligned_size = iova_align(iovad, org_size);
-   phys = swiotlb_tbl_map_single(dev, phys, org_size,
- aligned_size, dir, attrs);
-
-   if (phys == DMA_MAPPING_ERROR)
-   return DMA_MAPPING_ERROR;
-
-   /* Cleanup the padding area. */
-   padding_start = phys_to_virt(phys);
-   padding_size = aligned_size;
-
-   if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC) &&
-   (dir == DMA_TO_DEVICE ||
-dir == DMA_BIDIRECTIONAL)) {
-   padding_start += org_size;
-   padding_size -= org_size;
-   }
-
-   memset(padding_start, 0, padding_size);
-   }
-
-   if (!coherent && !(attrs & DMA_ATTR_SKIP_CPU_SYNC))
-   arch_sync_dma_for_device(phys, org_size, dir);
-
-   iova = __iommu_dma_map(dev, phys, aligned_size, prot, dma_mask);
-   if (iova == DMA_MAPPING_ERROR && is_swiotlb_buffer(phys))
-   swiotlb_tbl_unmap_single(dev, phys, org_size, dir, attrs);
-   return iova;
-}
-
 static void __iommu_dma_free_pages(struct page **pages, int count)
 {
while (count--)
@@ -848,15 +779,68 @@ static dma_addr_t iommu_dma_map_page(struct device *dev, 
struct page *page,
 {
phys_addr_t phys = page_to_phys(page) + offset;
bool coherent = dev_is_dma_coherent(dev);
+   int prot = dma_info_to_prot(dir, coherent, attrs);
+   struct iommu_domain *domain = iommu_get_dma_domain(dev);
+   struct iommu_dma_cookie *cookie = domain->iova_cookie;
+   struct iova_domain *iovad = >iovad;
+   size_t aligned_size = size;
+   dma_addr_t iova, dma_mask = dma_get_mask(dev);
+
+   /*
+* If both the physical buffer start address and size are
+* page aligned, we don't need to use a bounce page.
+*/
+   if (IS_ENABLED(CONFIG_SWIOTLB) && dev_is_untrusted(dev) &&
+   iova_offset(iovad, phys | size)) {
+   void *padding_start;
+   size_t padding_size;
+
+   aligned_size = iova_align(iovad, size);
+   phys = swiotlb_tbl_map_single(dev, phys, size,
+ aligned_size, dir, attrs);
+
+   if (phys == DMA_MAPPING_ERROR)
+   return DMA_MAPPING_ERROR;
 
-   return __iommu_dma_map_swiotlb(dev, phys, size, dma_get_mask(dev),

[PATCH v6 1/7] dma-iommu: fix sync_sg with swiotlb

2021-08-16 Thread David Stevens
From: David Stevens 

The is_swiotlb_buffer function takes the physical address of the swiotlb
buffer, not the physical address of the original buffer. The sglist
contains the physical addresses of the original buffer, so for the
sync_sg functions to work properly when a bounce buffer might have been
used, we need to use iommu_iova_to_phys to look up the physical address.
This is what sync_single does, so call that function on each sglist
segment.

The previous code mostly worked because swiotlb does the transfer on map
and unmap. However, any callers which use DMA_ATTR_SKIP_CPU_SYNC with
sglists or which call sync_sg would not have had anything copied to the
bounce buffer.

Fixes: 82612d66d51d ("iommu: Allow the dma-iommu api to use bounce buffers")
Signed-off-by: David Stevens 
Reviewed-by: Robin Murphy 
Reviewed-by: Christoph Hellwig 
---
 drivers/iommu/dma-iommu.c | 33 +
 1 file changed, 13 insertions(+), 20 deletions(-)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 98ba927aee1a..968e0150c95e 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -810,17 +810,13 @@ static void iommu_dma_sync_sg_for_cpu(struct device *dev,
struct scatterlist *sg;
int i;
 
-   if (dev_is_dma_coherent(dev) && !dev_is_untrusted(dev))
-   return;
-
-   for_each_sg(sgl, sg, nelems, i) {
-   if (!dev_is_dma_coherent(dev))
+   if (dev_is_untrusted(dev))
+   for_each_sg(sgl, sg, nelems, i)
+   iommu_dma_sync_single_for_cpu(dev, sg_dma_address(sg),
+ sg->length, dir);
+   else if (!dev_is_dma_coherent(dev))
+   for_each_sg(sgl, sg, nelems, i)
arch_sync_dma_for_cpu(sg_phys(sg), sg->length, dir);
-
-   if (is_swiotlb_buffer(sg_phys(sg)))
-   swiotlb_sync_single_for_cpu(dev, sg_phys(sg),
-   sg->length, dir);
-   }
 }
 
 static void iommu_dma_sync_sg_for_device(struct device *dev,
@@ -830,17 +826,14 @@ static void iommu_dma_sync_sg_for_device(struct device 
*dev,
struct scatterlist *sg;
int i;
 
-   if (dev_is_dma_coherent(dev) && !dev_is_untrusted(dev))
-   return;
-
-   for_each_sg(sgl, sg, nelems, i) {
-   if (is_swiotlb_buffer(sg_phys(sg)))
-   swiotlb_sync_single_for_device(dev, sg_phys(sg),
-  sg->length, dir);
-
-   if (!dev_is_dma_coherent(dev))
+   if (dev_is_untrusted(dev))
+   for_each_sg(sgl, sg, nelems, i)
+   iommu_dma_sync_single_for_device(dev,
+sg_dma_address(sg),
+sg->length, dir);
+   else if (!dev_is_dma_coherent(dev))
+   for_each_sg(sgl, sg, nelems, i)
arch_sync_dma_for_device(sg_phys(sg), sg->length, dir);
-   }
 }
 
 static dma_addr_t iommu_dma_map_page(struct device *dev, struct page *page,
-- 
2.33.0.rc1.237.g0d66db33f3-goog

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH v6 3/7] dma-iommu: skip extra sync during unmap w/swiotlb

2021-08-16 Thread David Stevens
From: David Stevens 

Calling the iommu_dma_sync_*_for_cpu functions during unmap can cause
two copies out of the swiotlb buffer. Do the arch sync directly in
__iommu_dma_unmap_swiotlb instead to avoid this. This makes the call to
iommu_dma_sync_sg_for_cpu for untrusted devices in iommu_dma_unmap_sg no
longer necessary, so move that invocation later in the function.

Signed-off-by: David Stevens 
Reviewed-by: Christoph Hellwig 
---
 drivers/iommu/dma-iommu.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 8098ce593640..5dd2c517dbf5 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -504,6 +504,9 @@ static void __iommu_dma_unmap_swiotlb(struct device *dev, 
dma_addr_t dma_addr,
if (WARN_ON(!phys))
return;
 
+   if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC) && !dev_is_dma_coherent(dev))
+   arch_sync_dma_for_cpu(phys, size, dir);
+
__iommu_dma_unmap(dev, dma_addr, size);
 
if (unlikely(is_swiotlb_buffer(phys)))
@@ -853,8 +856,6 @@ static dma_addr_t iommu_dma_map_page(struct device *dev, 
struct page *page,
 static void iommu_dma_unmap_page(struct device *dev, dma_addr_t dma_handle,
size_t size, enum dma_data_direction dir, unsigned long attrs)
 {
-   if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
-   iommu_dma_sync_single_for_cpu(dev, dma_handle, size, dir);
__iommu_dma_unmap_swiotlb(dev, dma_handle, size, dir, attrs);
 }
 
@@ -1062,14 +1063,14 @@ static void iommu_dma_unmap_sg(struct device *dev, 
struct scatterlist *sg,
struct scatterlist *tmp;
int i;
 
-   if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
-   iommu_dma_sync_sg_for_cpu(dev, sg, nents, dir);
-
if (dev_is_untrusted(dev)) {
iommu_dma_unmap_sg_swiotlb(dev, sg, nents, dir, attrs);
return;
}
 
+   if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
+   iommu_dma_sync_sg_for_cpu(dev, sg, nents, dir);
+
/*
 * The scatterlist segments are mapped into a single
 * contiguous IOVA allocation, so this is incredibly easy.
-- 
2.33.0.rc1.237.g0d66db33f3-goog

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH v6 2/7] dma-iommu: fix arch_sync_dma for map

2021-08-16 Thread David Stevens
From: David Stevens 

When calling arch_sync_dma, we need to pass it the memory that's
actually being used for dma. When using swiotlb bounce buffers, this is
the bounce buffer. Move arch_sync_dma into the __iommu_dma_map_swiotlb
helper, so it can use the bounce buffer address if necessary.

Now that iommu_dma_map_sg delegates to a function which takes care of
architectural syncing in the untrusted device case, the call to
iommu_dma_sync_sg_for_device can be moved so it only occurs for trusted
devices. Doing the sync for untrusted devices before mapping never
really worked, since it needs to be able to target swiotlb buffers.

This also moves the architectural sync to before the call to
__iommu_dma_map, to guarantee that untrusted devices can't see stale
data they shouldn't see.

Fixes: 82612d66d51d ("iommu: Allow the dma-iommu api to use bounce buffers")
Signed-off-by: David Stevens 
Reviewed-by: Christoph Hellwig 
---
 drivers/iommu/dma-iommu.c | 16 +++-
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 968e0150c95e..8098ce593640 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -576,6 +576,9 @@ static dma_addr_t __iommu_dma_map_swiotlb(struct device 
*dev, phys_addr_t phys,
memset(padding_start, 0, padding_size);
}
 
+   if (!coherent && !(attrs & DMA_ATTR_SKIP_CPU_SYNC))
+   arch_sync_dma_for_device(phys, org_size, dir);
+
iova = __iommu_dma_map(dev, phys, aligned_size, prot, dma_mask);
if (iova == DMA_MAPPING_ERROR && is_swiotlb_buffer(phys))
swiotlb_tbl_unmap_single(dev, phys, org_size, dir, attrs);
@@ -842,14 +845,9 @@ static dma_addr_t iommu_dma_map_page(struct device *dev, 
struct page *page,
 {
phys_addr_t phys = page_to_phys(page) + offset;
bool coherent = dev_is_dma_coherent(dev);
-   dma_addr_t dma_handle;
 
-   dma_handle = __iommu_dma_map_swiotlb(dev, phys, size, dma_get_mask(dev),
+   return __iommu_dma_map_swiotlb(dev, phys, size, dma_get_mask(dev),
coherent, dir, attrs);
-   if (!coherent && !(attrs & DMA_ATTR_SKIP_CPU_SYNC) &&
-   dma_handle != DMA_MAPPING_ERROR)
-   arch_sync_dma_for_device(phys, size, dir);
-   return dma_handle;
 }
 
 static void iommu_dma_unmap_page(struct device *dev, dma_addr_t dma_handle,
@@ -992,12 +990,12 @@ static int iommu_dma_map_sg(struct device *dev, struct 
scatterlist *sg,
iommu_deferred_attach(dev, domain))
return 0;
 
-   if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
-   iommu_dma_sync_sg_for_device(dev, sg, nents, dir);
-
if (dev_is_untrusted(dev))
return iommu_dma_map_sg_swiotlb(dev, sg, nents, dir, attrs);
 
+   if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
+   iommu_dma_sync_sg_for_device(dev, sg, nents, dir);
+
/*
 * Work out how much IOVA space we need, and align the segments to
 * IOVA granules for the IOMMU driver to handle. With some clever
-- 
2.33.0.rc1.237.g0d66db33f3-goog

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH v6 0/7] Fixes for dma-iommu swiotlb bounce buffers

2021-08-16 Thread David Stevens
From: David Stevens 

This patch set includes various fixes for dma-iommu's swiotlb bounce
buffers for untrusted devices. There are four fixes for correctness
issues, one for a performance issue, and one for general cleanup.

The min_align_mask issue was found when running fio on an untrusted nvme
device with bs=512. The other issues were found via code inspection, so
I don't have any specific use cases where things were not working, nor
any concrete performance numbers.

v5 -> v6:
 - Remove unnecessary line break
 - Remove redundant config check

v4 -> v5:
 - Fix xen build error
 - Move _swiotlb refactor into its own patch

v3 -> v4:
 - Fold _swiotlb functions into _page functions
 - Add patch to align swiotlb buffer to iovad granule
 - Combine if checks in iommu_dma_sync_sg_* functions

v2 -> v3:
 - Add new patch to address min_align_mask bug
 - Set SKIP_CPU_SYNC flag after syncing in map/unmap
 - Properly call arch_sync_dma_for_cpu in iommu_dma_sync_sg_for_cpu

v1 -> v2:
 - Split fixes into dedicated patches
 - Less invasive changes to fix arch_sync when mapping
 - Leave dev_is_untrusted check for strict iommu

David Stevens (7):
  dma-iommu: fix sync_sg with swiotlb
  dma-iommu: fix arch_sync_dma for map
  dma-iommu: skip extra sync during unmap w/swiotlb
  dma-iommu: fold _swiotlb helpers into callers
  dma-iommu: Check CONFIG_SWIOTLB more broadly
  swiotlb: support aligned swiotlb buffers
  dma-iommu: account for min_align_mask

 drivers/iommu/dma-iommu.c | 191 +-
 drivers/xen/swiotlb-xen.c |   2 +-
 include/linux/swiotlb.h   |   3 +-
 kernel/dma/swiotlb.c  |  11 ++-
 4 files changed, 96 insertions(+), 111 deletions(-)

-- 
2.33.0.rc1.237.g0d66db33f3-goog

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [RFC 0/5] Support non-coherent DMA on RISC-V using a global pool

2021-08-16 Thread Guo Ren
1

On Thu, Jul 29, 2021 at 2:19 PM Atish Patra  wrote:
>
> On Wed, Jul 28, 2021 at 9:30 PM Palmer Dabbelt  wrote:
> >
> > On Fri, 23 Jul 2021 14:40:26 PDT (-0700), Atish Patra wrote:
> > > RISC-V privilege specification doesn't define an IOMMU or any method 
> > > modify
> > > PMA attributes or PTE entries to allow non-coherent mappings yet. In
> > > the beginning, this approach was adopted assuming that most of the RISC-V
> > > platforms would support full cache-coherent IO. Here is excerpt from the
> > > priv spec section 3.6.5
> > >
> > > "In RISC-V platforms, the use of hardware-incoherent regions is 
> > > discouraged
> > > due to software complexity, performance, and energy impacts."
> > >
> > > While some of the RISC-V platforms adhere to the above suggestion, not all
> > > platforms can afford to build to fully cache-coherent I/O devices. To
> > > address DMA for non-coherent I/O devices, we need to mark a region of 
> > > memory
> > > as non-cacheable. Some of the platforms rely on a fixed region of uncached
> > > memory that is remapped to the system memory while some other modify
> > > the PTE entries to achieve that.
> > >
> > > The patch3 solves the issue for the fist use case by using a global
> > > pool of memory that is reserved for DMA. The device access the reserved 
> > > area
> > > of the region while corresponding CPU address will be from uncached region
> > > As the uncached region is remapped to the beginning of the system ram, 
> > > both
> > > CPU and device get the same view.
> > >
> > > To facilitate streaming DMA APIs, patch 1 introduces a set of generic
> > > cache operations. Any platform can use the generic ops to provide platform
> > > specific cache management operations. Once the standard RISC-V CMO 
> > > extension
> > > is available, it will also use these generic ops.
> > >
> > > To address the second use case, Page Based Memory Attribute (PBMT) 
> > > extension
> > > is proposed. Once the extension is in good shape, we can leverage that
> > > using CONFIG_DIRECT_REMAP. Currently, it is selected via a compile time 
> > > config
> > > option. We will probably need another arch specific hooks to know if the
> > > platform supports direct remap at runtime. For RISC-V, it will check the
> > > presence of PBMT extension while other arch function will simply return 
> > > true
> >
> > IIUC this is another extension that's not yet frozen or implemented in
> > hardware?  Is this one compatible with what's in the c906, or is it
> > doing things its own way?
>
> Hi Palmer,
> This series doesn't implement the PBMT extension which is still in discussion.
> It simply reuse the existing non-coherent dma mapping support in
> upstream kernel and enable
> it for RISC-V. The current version uses a non-coherent global pool. I
> will update it to use arch_set_uncached
> as per Christoph's suggestion. It solves the non-coherent DMA problem
> for beagleV and not c906.
>
> I briefly mentioned the PBMT extension just to provide an idea how the
> RISC-V Linux kernel
> can support both unached window and PBMT extension. PBMT extension is
> planned to be frozen
> by the end of this year and none of the hardware has implemented it.
>
> The implementation in c906 is a non-standard one and will not be
> supported by the default PBMT
> extension implementation.
The default PBMT & c908 PBMT are similar, only BIT definitions are
different. I propose to support default PBMT first and give the back
door to modify the PBMT definition during boot.
The "protection_map[] = (__P000, __P001 ..__S000, __S001)" in
mm/mmap.c has been modified by arch/mips, arm, sparc, x86, so I think
it's proper solution direction.

The reset problem is how to passing custom PBMT at the very early boot
stage. Now I'm trying to use the DTS parsing instead of boot param hdr
which Anup objected to.

ref: 
https://lore.kernel.org/linux-riscv/1623693067-53886-1-git-send-email-guo...@kernel.org/

Any comments are welcome.

>
>
> >
> > > if DIRECT_REMAP is enabled. This is required as arious different config
> > > (DIRECT_REMAP, GLOBAL_POOL) will be enabled in the defconfig so that a
> > > unified kernel image can boot on all RISC-V platforms.
> > >
> > > This patch series depends on Christoph's global pool support series[1].
> > > Tested on Qemu, HiFive unleashed and beagleV. This series is also 
> > > available
> > > at [2].
> > > This series also solves the non-coherent DMA support on beagleV but 
> > > requires
> > > additional beagleV specific patches[3] which will be upstreamed soon.
> > >
> > >
> > > [1] 
> > > https://lists.linuxfoundation.org/pipermail/iommu/2021-July/057266.html
> > > [2] https://github.com/atishp04/linux/tree/riscv_nc_global_pool
> > > [3] https://github.com/atishp04/linux/tree/wip_beaglev_dma_nc_global
> > >
> > > Atish Patra (5):
> > > RISC-V: Implement arch_sync_dma* functions
> > > of: Move of_dma_get_range to of_address.h
> > > dma-mapping: Enable global non-coherent pool support for RISC-V
> > > 

Re: [PATCH v2 0/2] Don't fail device probing due to of_dma_set_restricted_buffer()

2021-08-16 Thread Rob Herring
On Mon, Aug 16, 2021 at 8:26 AM Will Deacon  wrote:
>
> Hi all,
>
> This is v2 of the patch I previously posted here:
>
>   https://lore.kernel.org/r/20210805094736.902-1-w...@kernel.org
>
> Changes since v1 are:
>
>   * Move of_dma_set_restricted_buffer() into of/device.c (Rob)
>   * Use IS_ENABLED() instead of 'static inline' stub (Rob)
>
> This applies on Konrad's devel/for-linus-5.15 branch in swiotlb.git
>
> Cheers,
>
> Will
>
> Cc: Claire Chang 
> Cc: Konrad Rzeszutek Wilk 
> Cc: Christoph Hellwig 
> Cc: Rob Herring 
> Cc: Robin Murphy 
>
> --->8
>
> Will Deacon (2):
>   of: Move of_dma_set_restricted_buffer() into device.c
>   of: restricted dma: Don't fail device probe on rmem init failure
>
>  drivers/of/address.c| 33 -
>  drivers/of/device.c | 39 ++-
>  drivers/of/of_private.h |  7 ---
>  3 files changed, 38 insertions(+), 41 deletions(-)

Reviewed-by: Rob Herring 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


RE: [PATCH V3 08/13] HV/Vmbus: Initialize VMbus ring buffer for Isolation VM

2021-08-16 Thread Michael Kelley via iommu
From: Tianyu Lan  Sent: Monday, August 9, 2021 10:56 AM
> 
> VMbus ring buffer are shared with host and it's need to

s/it's need/it needs/

> be accessed via extra address space of Isolation VM with
> SNP support. This patch is to map the ring buffer
> address in extra address space via ioremap(). HV host

It's actually using vmap_pfn(), not ioremap().

> visibility hvcall smears data in the ring buffer and
> so reset the ring buffer memory to zero after calling
> visibility hvcall.
> 
> Signed-off-by: Tianyu Lan 
> ---
>  drivers/hv/Kconfig|  1 +
>  drivers/hv/channel.c  | 10 +
>  drivers/hv/hyperv_vmbus.h |  2 +
>  drivers/hv/ring_buffer.c  | 84 ++-
>  4 files changed, 79 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig
> index d1123ceb38f3..dd12af20e467 100644
> --- a/drivers/hv/Kconfig
> +++ b/drivers/hv/Kconfig
> @@ -8,6 +8,7 @@ config HYPERV
>   || (ARM64 && !CPU_BIG_ENDIAN))
>   select PARAVIRT
>   select X86_HV_CALLBACK_VECTOR if X86
> + select VMAP_PFN
>   help
> Select this option to run Linux as a Hyper-V client operating
> system.
> diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
> index 4c4717c26240..60ef881a700c 100644
> --- a/drivers/hv/channel.c
> +++ b/drivers/hv/channel.c
> @@ -712,6 +712,16 @@ static int __vmbus_open(struct vmbus_channel *newchannel,
>   if (err)
>   goto error_clean_ring;
> 
> + err = hv_ringbuffer_post_init(>outbound,
> +   page, send_pages);
> + if (err)
> + goto error_free_gpadl;
> +
> + err = hv_ringbuffer_post_init(>inbound,
> +   [send_pages], recv_pages);
> + if (err)
> + goto error_free_gpadl;
> +
>   /* Create and init the channel open message */
>   open_info = kzalloc(sizeof(*open_info) +
>  sizeof(struct vmbus_channel_open_channel),
> diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
> index 40bc0eff6665..15cd23a561f3 100644
> --- a/drivers/hv/hyperv_vmbus.h
> +++ b/drivers/hv/hyperv_vmbus.h
> @@ -172,6 +172,8 @@ extern int hv_synic_cleanup(unsigned int cpu);
>  /* Interface */
> 
>  void hv_ringbuffer_pre_init(struct vmbus_channel *channel);
> +int hv_ringbuffer_post_init(struct hv_ring_buffer_info *ring_info,
> + struct page *pages, u32 page_cnt);
> 
>  int hv_ringbuffer_init(struct hv_ring_buffer_info *ring_info,
>  struct page *pages, u32 pagecnt, u32 max_pkt_size);
> diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
> index 2aee356840a2..d4f93fca1108 100644
> --- a/drivers/hv/ring_buffer.c
> +++ b/drivers/hv/ring_buffer.c
> @@ -17,6 +17,8 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
> 
>  #include "hyperv_vmbus.h"
> 
> @@ -179,43 +181,89 @@ void hv_ringbuffer_pre_init(struct vmbus_channel 
> *channel)
>   mutex_init(>outbound.ring_buffer_mutex);
>  }
> 
> -/* Initialize the ring buffer. */
> -int hv_ringbuffer_init(struct hv_ring_buffer_info *ring_info,
> -struct page *pages, u32 page_cnt, u32 max_pkt_size)
> +int hv_ringbuffer_post_init(struct hv_ring_buffer_info *ring_info,
> +struct page *pages, u32 page_cnt)
>  {
> + u64 physic_addr = page_to_pfn(pages) << PAGE_SHIFT;
> + unsigned long *pfns_wraparound;
> + void *vaddr;
>   int i;
> - struct page **pages_wraparound;
> 
> - BUILD_BUG_ON((sizeof(struct hv_ring_buffer) != PAGE_SIZE));
> + if (!hv_isolation_type_snp())
> + return 0;
> +
> + physic_addr += ms_hyperv.shared_gpa_boundary;
> 
>   /*
>* First page holds struct hv_ring_buffer, do wraparound mapping for
>* the rest.
>*/
> - pages_wraparound = kcalloc(page_cnt * 2 - 1, sizeof(struct page *),
> + pfns_wraparound = kcalloc(page_cnt * 2 - 1, sizeof(unsigned long),
>  GFP_KERNEL);
> - if (!pages_wraparound)
> + if (!pfns_wraparound)
>   return -ENOMEM;
> 
> - pages_wraparound[0] = pages;
> + pfns_wraparound[0] = physic_addr >> PAGE_SHIFT;
>   for (i = 0; i < 2 * (page_cnt - 1); i++)
> - pages_wraparound[i + 1] = [i % (page_cnt - 1) + 1];
> -
> - ring_info->ring_buffer = (struct hv_ring_buffer *)
> - vmap(pages_wraparound, page_cnt * 2 - 1, VM_MAP, PAGE_KERNEL);
> -
> - kfree(pages_wraparound);
> + pfns_wraparound[i + 1] = (physic_addr >> PAGE_SHIFT) +
> + i % (page_cnt - 1) + 1;
> 
> -
> - if (!ring_info->ring_buffer)
> + vaddr = vmap_pfn(pfns_wraparound, page_cnt * 2 - 1, PAGE_KERNEL_IO);
> + kfree(pfns_wraparound);
> + if (!vaddr)
>   return -ENOMEM;
> 
> - ring_info->ring_buffer->read_index =
> - ring_info->ring_buffer->write_index = 0;
> + /* Clean memory after setting 

Re: [iommu:apple/dart 3/3] drivers/iommu/apple-dart.c:730:17: error: initialization of 'size_t (*)(struct iommu_domain *, long unsigned int, size_t, struct iommu_iotlb_gather *)' {aka 'long unsigned i

2021-08-16 Thread Robin Murphy

On 2021-08-16 07:48, Sven Peter wrote:



On Thu, Aug 12, 2021, at 13:29, Joerg Roedel wrote:

Hi Sven,

On Tue, Aug 10, 2021 at 08:09:53AM +0200, Sven Peter wrote:

This happens because apple/dart is missing the "Optimizing iommu_[map/unmap] 
performance"
series which is already in the core branch [1].
The same commit works fine in iommu/next since that branch merges both 
iommu/core and
apple/dart.


Okay, thanks. I re-based the DART patches on-top of my core branch,
which contains the changes for iommu_[map/unmap] performance. I
generally don't like rebasing topic branches, but made an exception here
to not break bisectability.

Thanks,

Joerg



Hi Joerg,

Thanks, and sorry about that! I'll try to make it more clear if anything depends
on another series in the future or just try to avoid it altogether if possible.


Just a heads up about a similar situation you may already be aware of: Once 
Robin's
DMA domain strictness refactoring [1] is merged, the current DART driver will 
fail due
to patch 12 there, which unexports iommu_get_dma_cookie. It'll need a small
adjustment just like all the other drivers (which will also fix two small bugs
it just made me notice: I never use iommu_put_dma_cookie and also 
unconditionally
grab a DMA cookie for all domain types).

Unless I'm mistaken I can't make that adjustment before the first patch of
that series has been merged, and Robin can't make that adjustment in his series
because it'll presumably go through another topic branch.


Hmm, the exports only affect modules though - the cookie functions are 
still public so that the core code can call them - so it should only 
break certain configs. Possibly the most self-contained way to avoid 
issues in -next would be to switch APPLE_DART from tristate to bool for 
now, then flip it back along with cleaning up the iommu-dma remnants at 
rc1. That should avoid any more rebasing at least.


At worst, my patch #12 could in principle be held back and applied at 
rc1 alongside the DART fixup - functionally it shouldn't be an issue, 
but patch #16 would need fixing up to apply without the context change.


Robin.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


RE: [PATCH V3 00/13] x86/Hyper-V: Add Hyper-V Isolation VM support

2021-08-16 Thread Michael Kelley via iommu
From: Tianyu Lan  Sent: Monday, August 9, 2021 10:56 AM
> 
> Hyper-V provides two kinds of Isolation VMs. VBS(Virtualization-based
> security) and AMD SEV-SNP unenlightened Isolation VMs. This patchset
> is to add support for these Isolation VM support in Linux.
> 

A general comment about this series:  I have not seen any statements
made about whether either type of Isolated VM is supported for 32-bit
Linux guests.   arch/x86/Kconfig has CONFIG_AMD_MEM_ENCRYPT as
64-bit only, so evidently SEV-SNP Isolated VMs would be 64-bit only.
But I don't know if VBS VMs are any different.

I didn't track down what happens if a 32-bit Linux is booted in
a VM that supports SEV-SNP.  Presumably some kind of message
is output that no encryption is being done.  But at a slightly
higher level, the Hyper-V initialization path should probably
also check for 32-bit and output a clear message that no isolation
is being provided.  At that point, I don't know if it is possible to
continue in non-isolated mode or whether the only choice is to
panic.  Continuing in non-isolated mode might be a bad idea
anyway since presumably the user has explicitly requested an
Isolated VM.

Related, I noticed usage of "unsigned long" for holding physical
addresses, which works when running 64-bit, but not when running
32-bit.  But even if Isolated VMs are always 64-bit, it would be still be
better to clean this up and use phys_addr_t instead.  Unfortunately,
more generic functions like set_memory_encrypted() and
set_memory_decrypted() have physical address arguments that
are of type unsigned long.

Michael
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH V3 10/13] x86/Swiotlb: Add Swiotlb bounce buffer remap function for HV IVM

2021-08-16 Thread Tianyu Lan

On 8/14/2021 1:58 AM, Tianyu Lan wrote:

On 8/12/2021 8:27 PM, Christoph Hellwig wrote:

This is still broken.  You need to make sure the actual DMA allocations
do have struct page backing.



Hi Christoph:
  swiotlb_tbl_map_single() still returns PA below vTOM/share_gpa_ > 
boundary. These PAs has backing pages and belong to system memory.
In other word, all PAs passed to DMA API have backing pages and these is 
no difference between Isolation guest and traditional guest for DMA API.
The new mapped VA for PA above vTOM here is just to access the bounce 
buffer in the swiotlb code and isn't exposed to outside.


Hi Christoph:
  Sorry to bother you.Please double check with these two patches
" [PATCH V3 10/13] x86/Swiotlb: Add Swiotlb bounce buffer remap function 
for HV IVM" and "[PATCH V3 09/13] DMA: Add dma_map_decrypted/dma_

unmap_encrypted() function".
  The swiotlb bounce buffer in the isolation VM are allocated in the
low end memory and these memory has struct page backing. All dma address
returned by swiotlb/DMA API are low end memory and this is as same as 
what happen in the traditional VM.So this means all PAs passed to DMA 
API have struct page backing. The difference in Isolation VM is to 
access bounce buffer via address space above vTOM/shared_guest_memory

_boundary. To access bounce buffer shared with host, the guest needs to
mark the memory visible to host via hypercall and map bounce buffer in 
the extra address space(PA + shared_guest_memory_boundary). The vstart
introduced in this patch is to store va of extra address space and it's 
only used to access bounce buffer in the swiotlb_bounce(). The PA in 
extra space is only in the Hyper-V map function and won't be passed to 
DMA API or other components.
  The API dma_map_decrypted() introduced in the patch 9 is to map 
the bounce buffer in the extra space and these memory in the low end 
space are used as DMA memory in the driver. Do you prefer these APIs

still in the set_memory.c? I move the API to dma/mapping.c due to the
suggested name arch_dma_map_decrypted() in the previous mail
(https://lore.kernel.org/netdev/20210720135437.ga13...@lst.de/).
  If there are something unclear, please let me know. Hope this
still can catch the merge window.

Thanks.




___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v3 14/14] tpm: Allow locality 2 to be set when initializing the TPM for Secure Launch

2021-08-16 Thread Ross Philipson

On 8/10/21 12:21 PM, Jarkko Sakkinen wrote:

On Mon, Aug 09, 2021 at 12:38:56PM -0400, Ross Philipson wrote:

The Secure Launch MLE environment uses PCRs that are only accessible from
the DRTM locality 2. By default the TPM drivers always initialize the
locality to 0. When a Secure Launch is in progress, initialize the
locality to 2.

Signed-off-by: Ross Philipson 
---
  drivers/char/tpm/tpm-chip.c | 13 +++--
  1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index ddaeceb..48b9351 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -23,6 +23,7 @@
  #include 
  #include 
  #include 
+#include 
  #include "tpm.h"
  
  DEFINE_IDR(dev_nums_idr);

@@ -34,12 +35,20 @@
  
  static int tpm_request_locality(struct tpm_chip *chip)

  {
-   int rc;
+   int rc, locality;


 int locality;
 int rc;


Will do.



  
  	if (!chip->ops->request_locality)

return 0;
  
-	rc = chip->ops->request_locality(chip, 0);

+   if (slaunch_get_flags() & SL_FLAG_ACTIVE) {
+   dev_dbg(>dev, "setting TPM locality to 2 for MLE\n");
+   locality = 2;
+   } else {
+   dev_dbg(>dev, "setting TPM locality to 0\n");
+   locality = 0;
+   }


Please, remove dev_dbg()'s.


Will do.

Thanks
Ross




+
+   rc = chip->ops->request_locality(chip, locality);
if (rc < 0)
return rc;
  
--

1.8.3.1


/Jarkko



___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v3 02/14] x86/boot: Add missing handling of setup_indirect structures

2021-08-16 Thread Ross Philipson

On 8/10/21 12:19 PM, Jarkko Sakkinen wrote:

On Mon, Aug 09, 2021 at 12:38:44PM -0400, Ross Philipson wrote:

One of the two functions in ioremap.c that handles setup_data was
missing the correct handling of setup_indirect structures.


What is "correct handling", and how was it broken?

What is 'setup_indirect'?


Functionality missing from original commit:


Remove this sentence.


commit b3c72fc9a78e (x86/boot: Introduce setup_indirect)


Should be.

Fixes: b3c72fc9a78e ("x86/boot: Introduce setup_indirect")


I will fix these things and make the commit message clearer.

Thanks,
Ross



  

Signed-off-by: Ross Philipson 
---
  arch/x86/mm/ioremap.c | 21 +++--
  1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index ab74e4f..f2b34c5 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -669,17 +669,34 @@ static bool __init 
early_memremap_is_setup_data(resource_size_t phys_addr,
  
  	paddr = boot_params.hdr.setup_data;

while (paddr) {
-   unsigned int len;
+   unsigned int len, size;
  
  		if (phys_addr == paddr)

return true;
  
  		data = early_memremap_decrypted(paddr, sizeof(*data));

+   size = sizeof(*data);
  
  		paddr_next = data->next;

len = data->len;
  
-		early_memunmap(data, sizeof(*data));

+   if ((phys_addr > paddr) && (phys_addr < (paddr + len))) {
+   early_memunmap(data, sizeof(*data));
+   return true;
+   }
+
+   if (data->type == SETUP_INDIRECT) {
+   size += len;
+   early_memunmap(data, sizeof(*data));
+   data = early_memremap_decrypted(paddr, size);
+
+   if (((struct setup_indirect *)data->data)->type != 
SETUP_INDIRECT) {
+   paddr = ((struct setup_indirect 
*)data->data)->addr;
+   len = ((struct setup_indirect 
*)data->data)->len;
+   }
+   }
+
+   early_memunmap(data, size);
  
  		if ((phys_addr > paddr) && (phys_addr < (paddr + len)))

return true;
--
1.8.3.1




/Jarkko



___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH v2 1/2] of: Move of_dma_set_restricted_buffer() into device.c

2021-08-16 Thread Will Deacon
Rob observes that:

  | of_dma_set_restricted_buffer() [...] should also be moved to
  | of/device.c. There's no reason for it to be in of/address.c. It has
  | nothing to do with address parsing.

Move it to of/device.c, as he suggests.

Cc: Claire Chang 
Cc: Konrad Rzeszutek Wilk 
Cc: Christoph Hellwig 
Cc: Robin Murphy 
Suggested-by: Rob Herring 
Link: 
https://lore.kernel.org/r/CAL_JsqJ7ROWWJX84x2kEex9NQ8G+2=ybrunoobx+j8bjzzs...@mail.gmail.com
Signed-off-by: Will Deacon 
---
 drivers/of/address.c| 33 -
 drivers/of/device.c | 34 ++
 drivers/of/of_private.h |  7 ---
 3 files changed, 34 insertions(+), 40 deletions(-)

diff --git a/drivers/of/address.c b/drivers/of/address.c
index 973257434398..94f017d808c4 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -8,7 +8,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -996,38 +995,6 @@ int of_dma_get_range(struct device_node *np, const struct 
bus_dma_region **map)
of_node_put(node);
return ret;
 }
-
-int of_dma_set_restricted_buffer(struct device *dev, struct device_node *np)
-{
-   struct device_node *node, *of_node = dev->of_node;
-   int count, i;
-
-   count = of_property_count_elems_of_size(of_node, "memory-region",
-   sizeof(u32));
-   /*
-* If dev->of_node doesn't exist or doesn't contain memory-region, try
-* the OF node having DMA configuration.
-*/
-   if (count <= 0) {
-   of_node = np;
-   count = of_property_count_elems_of_size(
-   of_node, "memory-region", sizeof(u32));
-   }
-
-   for (i = 0; i < count; i++) {
-   node = of_parse_phandle(of_node, "memory-region", i);
-   /*
-* There might be multiple memory regions, but only one
-* restricted-dma-pool region is allowed.
-*/
-   if (of_device_is_compatible(node, "restricted-dma-pool") &&
-   of_device_is_available(node))
-   return of_reserved_mem_device_init_by_idx(dev, of_node,
- i);
-   }
-
-   return 0;
-}
 #endif /* CONFIG_HAS_DMA */
 
 /**
diff --git a/drivers/of/device.c b/drivers/of/device.c
index 2defdca418ec..089c5b4b97fb 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -5,6 +5,7 @@
 #include 
 #include 
 #include 
+#include 
 #include  /* for bus_dma_region */
 #include 
 #include 
@@ -52,6 +53,39 @@ int of_device_add(struct platform_device *ofdev)
return device_add(>dev);
 }
 
+static int
+of_dma_set_restricted_buffer(struct device *dev, struct device_node *np)
+{
+   struct device_node *node, *of_node = dev->of_node;
+   int count, i;
+
+   count = of_property_count_elems_of_size(of_node, "memory-region",
+   sizeof(u32));
+   /*
+* If dev->of_node doesn't exist or doesn't contain memory-region, try
+* the OF node having DMA configuration.
+*/
+   if (count <= 0) {
+   of_node = np;
+   count = of_property_count_elems_of_size(
+   of_node, "memory-region", sizeof(u32));
+   }
+
+   for (i = 0; i < count; i++) {
+   node = of_parse_phandle(of_node, "memory-region", i);
+   /*
+* There might be multiple memory regions, but only one
+* restricted-dma-pool region is allowed.
+*/
+   if (of_device_is_compatible(node, "restricted-dma-pool") &&
+   of_device_is_available(node))
+   return of_reserved_mem_device_init_by_idx(dev, of_node,
+ i);
+   }
+
+   return 0;
+}
+
 /**
  * of_dma_configure_id - Setup DMA configuration
  * @dev:   Device to apply DMA configuration
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index f557bd22b0cf..631489f7f8c0 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -163,19 +163,12 @@ struct bus_dma_region;
 #if defined(CONFIG_OF_ADDRESS) && defined(CONFIG_HAS_DMA)
 int of_dma_get_range(struct device_node *np,
const struct bus_dma_region **map);
-int of_dma_set_restricted_buffer(struct device *dev, struct device_node *np);
 #else
 static inline int of_dma_get_range(struct device_node *np,
const struct bus_dma_region **map)
 {
return -ENODEV;
 }
-static inline int of_dma_set_restricted_buffer(struct device *dev,
-  struct device_node *np)
-{
-   /* Do nothing, successfully. */
-   return 0;
-}
 #endif
 
 void fdt_init_reserved_mem(void);
-- 
2.33.0.rc1.237.g0d66db33f3-goog


[PATCH v2 2/2] of: restricted dma: Don't fail device probe on rmem init failure

2021-08-16 Thread Will Deacon
If CONFIG_DMA_RESTRICTED_POOL=n then probing a device with a reference
to a "restricted-dma-pool" will fail with a reasonably cryptic error:

  | pci-host-generic: probe of 1.pci failed with error -22

Rework of_dma_set_restricted_buffer() so that it does not cause probing
failure and instead either returns early if CONFIG_DMA_RESTRICTED_POOL=n
or emits a diagnostic if the reserved DMA pool fails to initialise.

Cc: Claire Chang 
Cc: Konrad Rzeszutek Wilk 
Cc: Christoph Hellwig 
Cc: Rob Herring 
Cc: Robin Murphy 
Signed-off-by: Will Deacon 
---
 drivers/of/device.c | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/of/device.c b/drivers/of/device.c
index 089c5b4b97fb..5b043ee30824 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -53,12 +53,15 @@ int of_device_add(struct platform_device *ofdev)
return device_add(>dev);
 }
 
-static int
+static void
 of_dma_set_restricted_buffer(struct device *dev, struct device_node *np)
 {
struct device_node *node, *of_node = dev->of_node;
int count, i;
 
+   if (!IS_ENABLED(CONFIG_DMA_RESTRICTED_POOL))
+   return;
+
count = of_property_count_elems_of_size(of_node, "memory-region",
sizeof(u32));
/*
@@ -79,11 +82,11 @@ of_dma_set_restricted_buffer(struct device *dev, struct 
device_node *np)
 */
if (of_device_is_compatible(node, "restricted-dma-pool") &&
of_device_is_available(node))
-   return of_reserved_mem_device_init_by_idx(dev, of_node,
- i);
+   break;
}
 
-   return 0;
+   if (i != count && of_reserved_mem_device_init_by_idx(dev, of_node, i))
+   dev_warn(dev, "failed to initialise \"restricted-dma-pool\" 
memory node\n");
 }
 
 /**
@@ -200,7 +203,7 @@ int of_dma_configure_id(struct device *dev, struct 
device_node *np,
arch_setup_dma_ops(dev, dma_start, size, iommu, coherent);
 
if (!iommu)
-   return of_dma_set_restricted_buffer(dev, np);
+   of_dma_set_restricted_buffer(dev, np);
 
return 0;
 }
-- 
2.33.0.rc1.237.g0d66db33f3-goog

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH v2 0/2] Don't fail device probing due to of_dma_set_restricted_buffer()

2021-08-16 Thread Will Deacon
Hi all,

This is v2 of the patch I previously posted here:

  https://lore.kernel.org/r/20210805094736.902-1-w...@kernel.org

Changes since v1 are:

  * Move of_dma_set_restricted_buffer() into of/device.c (Rob)
  * Use IS_ENABLED() instead of 'static inline' stub (Rob)

This applies on Konrad's devel/for-linus-5.15 branch in swiotlb.git

Cheers,

Will

Cc: Claire Chang 
Cc: Konrad Rzeszutek Wilk 
Cc: Christoph Hellwig 
Cc: Rob Herring 
Cc: Robin Murphy 

--->8

Will Deacon (2):
  of: Move of_dma_set_restricted_buffer() into device.c
  of: restricted dma: Don't fail device probe on rmem init failure

 drivers/of/address.c| 33 -
 drivers/of/device.c | 39 ++-
 drivers/of/of_private.h |  7 ---
 3 files changed, 38 insertions(+), 41 deletions(-)

-- 
2.33.0.rc1.237.g0d66db33f3-goog

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v5 5/7] dma-iommu: Check CONFIG_SWIOTLB more broadly

2021-08-16 Thread Christoph Hellwig
> - if (IS_ENABLED(CONFIG_SWIOTLB) && dev_is_untrusted(dev) &&
> + if (IS_ENABLED(CONFIG_SWIOTLB) && dev_use_swiotlb(dev) &&
>   iova_offset(iovad, phys | size)) {

This can drop the explicit CONFIG_SWIOTLB check now (and make the remaining
conditional fit onto a single line).
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v5 4/7] dma-iommu: fold _swiotlb helpers into callers

2021-08-16 Thread Christoph Hellwig
> + if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC) &&
> + (dir == DMA_TO_DEVICE ||
> +  dir == DMA_BIDIRECTIONAL)) {

Nit: the two dire checks easily fit onto a single line and are easier to
follow that way.

Otherwise looks good:

Reviewed-by: Christoph Hellwig 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v5 3/7] dma-iommu: skip extra sync during unmap w/swiotlb

2021-08-16 Thread Christoph Hellwig
Looks good,

Reviewed-by: Christoph Hellwig 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v5 2/7] dma-iommu: fix arch_sync_dma for map

2021-08-16 Thread Christoph Hellwig
Looks good,

Reviewed-by: Christoph Hellwig 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 1/4] iommu/arm-smmu-v3: Use command queue batching helpers to improve performance

2021-08-16 Thread Leizhen (ThunderTown)



On 2021/8/16 16:21, Will Deacon wrote:
> On Mon, Aug 16, 2021 at 03:47:58PM +0800, Leizhen (ThunderTown) wrote:
>>
>>
>> On 2021/8/16 15:24, John Garry wrote:
 In addition, I find that function arm_smmu_cmdq_build_cmd() can also be 
 optimized
 slightly, three useless instructions can be reduced.
>>>
>>> I think that you could optimise further by pre-building commonly used 
>>> commands.
>>>
>>> For example, CMD_SYNC without MSI polling is always the same. And then only 
>>> different in 1 field for MSI polling.
>>>
>>> But you need to check if the performance gain is worth the change.
>>
>> Good advice. I can give it a try.
> 
> Please send it as a new patch on top. I've queued the old one and sent
> it to Joerg. Since this is just further cleanup, it can be done separately.

OK, I'll post a separate patch later.

> 
> Will
> .
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 1/4] iommu/arm-smmu-v3: Use command queue batching helpers to improve performance

2021-08-16 Thread Will Deacon
On Mon, Aug 16, 2021 at 03:47:58PM +0800, Leizhen (ThunderTown) wrote:
> 
> 
> On 2021/8/16 15:24, John Garry wrote:
> >> In addition, I find that function arm_smmu_cmdq_build_cmd() can also be 
> >> optimized
> >> slightly, three useless instructions can be reduced.
> > 
> > I think that you could optimise further by pre-building commonly used 
> > commands.
> > 
> > For example, CMD_SYNC without MSI polling is always the same. And then only 
> > different in 1 field for MSI polling.
> > 
> > But you need to check if the performance gain is worth the change.
> 
> Good advice. I can give it a try.

Please send it as a new patch on top. I've queued the old one and sent
it to Joerg. Since this is just further cleanup, it can be done separately.

Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 1/4] iommu/arm-smmu-v3: Use command queue batching helpers to improve performance

2021-08-16 Thread Leizhen (ThunderTown)


On 2021/8/16 15:24, John Garry wrote:
>> In addition, I find that function arm_smmu_cmdq_build_cmd() can also be 
>> optimized
>> slightly, three useless instructions can be reduced.
> 
> I think that you could optimise further by pre-building commonly used 
> commands.
> 
> For example, CMD_SYNC without MSI polling is always the same. And then only 
> different in 1 field for MSI polling.
> 
> But you need to check if the performance gain is worth the change.

Good advice. I can give it a try.

> 
>>
>> Case 1):
>> void arm_smmu_cmdq_build_cmd_tst1(u64 *cmd, struct arm_smmu_cmdq_ent *ent)
>> {
>>  memset(cmd, 0, 1 << CMDQ_ENT_SZ_SHIFT);
>>  cmd[0] |= FIELD_PREP(CMDQ_0_OP, ent->opcode);
>> }
>> 4608 :
>>  4608:   a9007c1f    stp xzr, xzr, [x0]
>>  460c:   39400022    ldrb    w2, [x1]
>>  4610:   f941    ldr x1, [x0]
>>  4614:   aa020021    orr x1, x1, x2
>>  4618:   f901    str x1, [x0]
>>  461c:   d65f03c0    ret
>>
>> Case 2):
>> void arm_smmu_cmdq_build_cmd_tst2(u64 *cmd, struct arm_smmu_cmdq_ent *ent)
>> {
>>  int i;
>>
>>  cmd[0] = FIELD_PREP(CMDQ_0_OP, ent->opcode);
>>  for (i = 1; i < CMDQ_ENT_DWORDS; i++)
>>  cmd[i] = 0;
>> }
>> 4620 :
>>  4620:   39400021    ldrb    w1, [x1]
>>  4624:   a9007c01    stp x1, xzr, [x0]
>>  4628:   d65f03c0    ret
>>  462c:   d503201f    nop
>>
>> Case 3):
>> void arm_smmu_cmdq_build_cmd_tst3(u64 *cmd, struct arm_smmu_cmdq_ent *ent)
>> {
>>  memset(cmd, 0, 1 << CMDQ_ENT_SZ_SHIFT);
>>  cmd[0] = FIELD_PREP(CMDQ_0_OP, ent->opcode);
>> }
>> 4630 :
>>  4630:   a9007c1f    stp xzr, xzr, [x0]
>>  4634:   39400021    ldrb    w1, [x1]
>>  4638:   f901    str x1, [x0]
>>  463c:   d65f03c0    ret
>>
> 
> .
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v2 3/4] iommu/arm-smmu-v3: Add and use static helper function arm_smmu_get_cmdq()

2021-08-16 Thread Zhen Lei
One SMMU has only one normal CMDQ. Therefore, this CMDQ is used regardless
of the core on which the command is inserted. It can be referenced
directly through "smmu->cmdq". However, one SMMU has multiple ECMDQs, and
the ECMDQ used by the core on which the command insertion is executed may
be different. So the helper function arm_smmu_get_cmdq() is added, which
returns the CMDQ/ECMDQ that the current core should use. Currently, the
code that supports ECMDQ is not added. just simply returns ">cmdq".

Many subfunctions of arm_smmu_cmdq_issue_cmdlist() use ">cmdq" or
">cmdq.q" directly. To support ECMDQ, they need to call the newly
added function arm_smmu_get_cmdq() instead.

Note that normal CMDQ is still required until ECMDQ is available.

Signed-off-by: Zhen Lei 
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 22 -
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c 
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 9be07f6915cc3c8..7814366778fda35 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -335,10 +335,14 @@ static int arm_smmu_cmdq_build_cmd(u64 *cmd, struct 
arm_smmu_cmdq_ent *ent)
return 0;
 }
 
+static struct arm_smmu_cmdq *arm_smmu_get_cmdq(struct arm_smmu_device *smmu)
+{
+   return >cmdq;
+}
+
 static void arm_smmu_cmdq_build_sync_cmd(u64 *cmd, struct arm_smmu_device 
*smmu,
-u32 prod)
+struct arm_smmu_queue *q, u32 prod)
 {
-   struct arm_smmu_queue *q = >cmdq.q;
struct arm_smmu_cmdq_ent ent = {
.opcode = CMDQ_OP_CMD_SYNC,
};
@@ -579,7 +583,7 @@ static int arm_smmu_cmdq_poll_until_not_full(struct 
arm_smmu_device *smmu,
 {
unsigned long flags;
struct arm_smmu_queue_poll qp;
-   struct arm_smmu_cmdq *cmdq = >cmdq;
+   struct arm_smmu_cmdq *cmdq = arm_smmu_get_cmdq(smmu);
int ret = 0;
 
/*
@@ -595,7 +599,7 @@ static int arm_smmu_cmdq_poll_until_not_full(struct 
arm_smmu_device *smmu,
 
queue_poll_init(smmu, );
do {
-   llq->val = READ_ONCE(smmu->cmdq.q.llq.val);
+   llq->val = READ_ONCE(cmdq->q.llq.val);
if (!queue_full(llq))
break;
 
@@ -614,7 +618,7 @@ static int __arm_smmu_cmdq_poll_until_msi(struct 
arm_smmu_device *smmu,
 {
int ret = 0;
struct arm_smmu_queue_poll qp;
-   struct arm_smmu_cmdq *cmdq = >cmdq;
+   struct arm_smmu_cmdq *cmdq = arm_smmu_get_cmdq(smmu);
u32 *cmd = (u32 *)(Q_ENT(>q, llq->prod));
 
queue_poll_init(smmu, );
@@ -637,12 +641,12 @@ static int __arm_smmu_cmdq_poll_until_consumed(struct 
arm_smmu_device *smmu,
   struct arm_smmu_ll_queue *llq)
 {
struct arm_smmu_queue_poll qp;
-   struct arm_smmu_cmdq *cmdq = >cmdq;
+   struct arm_smmu_cmdq *cmdq = arm_smmu_get_cmdq(smmu);
u32 prod = llq->prod;
int ret = 0;
 
queue_poll_init(smmu, );
-   llq->val = READ_ONCE(smmu->cmdq.q.llq.val);
+   llq->val = READ_ONCE(cmdq->q.llq.val);
do {
if (queue_consumed(llq, prod))
break;
@@ -732,7 +736,7 @@ static int arm_smmu_cmdq_issue_cmdlist(struct 
arm_smmu_device *smmu,
u32 prod;
unsigned long flags;
bool owner;
-   struct arm_smmu_cmdq *cmdq = >cmdq;
+   struct arm_smmu_cmdq *cmdq = arm_smmu_get_cmdq(smmu);
struct arm_smmu_ll_queue llq = {
.max_n_shift = cmdq->q.llq.max_n_shift,
}, head = llq;
@@ -772,7 +776,7 @@ static int arm_smmu_cmdq_issue_cmdlist(struct 
arm_smmu_device *smmu,
arm_smmu_cmdq_write_entries(cmdq, cmds, llq.prod, n);
if (sync) {
prod = queue_inc_prod_n(, n);
-   arm_smmu_cmdq_build_sync_cmd(cmd_sync, smmu, prod);
+   arm_smmu_cmdq_build_sync_cmd(cmd_sync, smmu, >q, prod);
queue_write(Q_ENT(>q, prod), cmd_sync, CMDQ_ENT_DWORDS);
 
/*
-- 
2.26.0.106.g9fadedd

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH v2 1/4] iommu/arm-smmu-v3: Use command queue batching helpers to improve performance

2021-08-16 Thread Zhen Lei
The obvious key to the performance optimization of commit 587e6c10a7ce
("iommu/arm-smmu-v3: Reduce contention during command-queue insertion") is
to allow multiple cores to insert commands in parallel after a brief mutex
contention.

Obviously, inserting as many commands at a time as possible can reduce the
number of times the mutex contention participates, thereby improving the
overall performance. At least it reduces the number of calls to function
arm_smmu_cmdq_issue_cmdlist().

Therefore, use command queue batching helpers to insert multiple commands
at a time.

Signed-off-by: Zhen Lei 
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c 
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 235f9bdaeaf223b..5eedb46aaceece8 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1747,15 +1747,17 @@ static int arm_smmu_atc_inv_master(struct 
arm_smmu_master *master)
 {
int i;
struct arm_smmu_cmdq_ent cmd;
+   struct arm_smmu_cmdq_batch cmds;
 
arm_smmu_atc_inv_to_cmd(0, 0, 0, );
 
+   cmds.num = 0;
for (i = 0; i < master->num_streams; i++) {
cmd.atc.sid = master->streams[i].id;
-   arm_smmu_cmdq_issue_cmd(master->smmu, );
+   arm_smmu_cmdq_batch_add(master->smmu, , );
}
 
-   return arm_smmu_cmdq_issue_sync(master->smmu);
+   return arm_smmu_cmdq_batch_submit(master->smmu, );
 }
 
 int arm_smmu_atc_inv_domain(struct arm_smmu_domain *smmu_domain, int ssid,
-- 
2.26.0.106.g9fadedd

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH v2 0/4] Prepare for ECMDQ support

2021-08-16 Thread Zhen Lei
v1 --> v2:
1. Stop pre-zeroing batch commands. Patch 1 is modified. Other patches remain 
unchanged.
   Before:
   struct arm_smmu_cmdq_batch cmds = {};

   After:
   struct arm_smmu_cmdq_batch cmds;
   cmds.num = 0;

RFC --> v1
1. Resend the patches for ECMDQ preparation and remove the patches for ECMDQ 
implementation.
2. Patch 2 is modified. Other patches remain unchanged.
   1) Add static helper __arm_smmu_cmdq_issue_cmd(), and make 
arm_smmu_cmdq_issue_cmd()
  and arm_smmu_cmdq_issue_cmd_with_sync() implement based on it.
   2) Remove unused arm_smmu_cmdq_issue_sync().

RFC:
https://www.spinics.net/lists/arm-kernel/msg904879.html


Zhen Lei (4):
  iommu/arm-smmu-v3: Use command queue batching helpers to improve
performance
  iommu/arm-smmu-v3: Add and use static helper function
arm_smmu_cmdq_issue_cmd_with_sync()
  iommu/arm-smmu-v3: Add and use static helper function
arm_smmu_get_cmdq()
  iommu/arm-smmu-v3: Extract reusable function
__arm_smmu_cmdq_skip_err()

 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 72 -
 1 file changed, 43 insertions(+), 29 deletions(-)

-- 
2.26.0.106.g9fadedd

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH v2 4/4] iommu/arm-smmu-v3: Extract reusable function __arm_smmu_cmdq_skip_err()

2021-08-16 Thread Zhen Lei
When SMMU_GERROR.CMDQP_ERR is different to SMMU_GERRORN.CMDQP_ERR, it
indicates that one or more errors have been encountered on a command queue
control page interface. We need to traverse all ECMDQs in that control
page to find all errors. For each ECMDQ error handling, it is much the
same as the CMDQ error handling. This common processing part is extracted
as a new function __arm_smmu_cmdq_skip_err().

Signed-off-by: Zhen Lei 
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c 
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 7814366778fda35..f3824c37f1832a2 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -359,7 +359,8 @@ static void arm_smmu_cmdq_build_sync_cmd(u64 *cmd, struct 
arm_smmu_device *smmu,
arm_smmu_cmdq_build_cmd(cmd, );
 }
 
-static void arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu)
+static void __arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu,
+struct arm_smmu_queue *q)
 {
static const char * const cerror_str[] = {
[CMDQ_ERR_CERROR_NONE_IDX]  = "No error",
@@ -370,7 +371,6 @@ static void arm_smmu_cmdq_skip_err(struct arm_smmu_device 
*smmu)
 
int i;
u64 cmd[CMDQ_ENT_DWORDS];
-   struct arm_smmu_queue *q = >cmdq.q;
u32 cons = readl_relaxed(q->cons_reg);
u32 idx = FIELD_GET(CMDQ_CONS_ERR, cons);
struct arm_smmu_cmdq_ent cmd_sync = {
@@ -417,6 +417,11 @@ static void arm_smmu_cmdq_skip_err(struct arm_smmu_device 
*smmu)
queue_write(Q_ENT(q, cons), cmd, q->ent_dwords);
 }
 
+static void arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu)
+{
+   __arm_smmu_cmdq_skip_err(smmu, >cmdq.q);
+}
+
 /*
  * Command queue locking.
  * This is a form of bastardised rwlock with the following major changes:
-- 
2.26.0.106.g9fadedd

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH v2 2/4] iommu/arm-smmu-v3: Add and use static helper function arm_smmu_cmdq_issue_cmd_with_sync()

2021-08-16 Thread Zhen Lei
The obvious key to the performance optimization of commit 587e6c10a7ce
("iommu/arm-smmu-v3: Reduce contention during command-queue insertion") is
to allow multiple cores to insert commands in parallel after a brief mutex
contention.

Obviously, inserting as many commands at a time as possible can reduce the
number of times the mutex contention participates, thereby improving the
overall performance. At least it reduces the number of calls to function
arm_smmu_cmdq_issue_cmdlist().

Therefore, function arm_smmu_cmdq_issue_cmd_with_sync() is added to insert
the 'cmd+sync' commands at a time.

Signed-off-by: Zhen Lei 
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 35 +++--
 1 file changed, 19 insertions(+), 16 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c 
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 5eedb46aaceece8..9be07f6915cc3c8 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -845,8 +845,9 @@ static int arm_smmu_cmdq_issue_cmdlist(struct 
arm_smmu_device *smmu,
return ret;
 }
 
-static int arm_smmu_cmdq_issue_cmd(struct arm_smmu_device *smmu,
-  struct arm_smmu_cmdq_ent *ent)
+static int __arm_smmu_cmdq_issue_cmd(struct arm_smmu_device *smmu,
+struct arm_smmu_cmdq_ent *ent,
+bool sync)
 {
u64 cmd[CMDQ_ENT_DWORDS];
 
@@ -856,12 +857,19 @@ static int arm_smmu_cmdq_issue_cmd(struct arm_smmu_device 
*smmu,
return -EINVAL;
}
 
-   return arm_smmu_cmdq_issue_cmdlist(smmu, cmd, 1, false);
+   return arm_smmu_cmdq_issue_cmdlist(smmu, cmd, 1, sync);
 }
 
-static int arm_smmu_cmdq_issue_sync(struct arm_smmu_device *smmu)
+static int arm_smmu_cmdq_issue_cmd(struct arm_smmu_device *smmu,
+  struct arm_smmu_cmdq_ent *ent)
 {
-   return arm_smmu_cmdq_issue_cmdlist(smmu, NULL, 0, true);
+   return __arm_smmu_cmdq_issue_cmd(smmu, ent, false);
+}
+
+static int arm_smmu_cmdq_issue_cmd_with_sync(struct arm_smmu_device *smmu,
+struct arm_smmu_cmdq_ent *ent)
+{
+   return __arm_smmu_cmdq_issue_cmd(smmu, ent, true);
 }
 
 static void arm_smmu_cmdq_batch_add(struct arm_smmu_device *smmu,
@@ -929,8 +937,7 @@ void arm_smmu_tlb_inv_asid(struct arm_smmu_device *smmu, 
u16 asid)
.tlbi.asid = asid,
};
 
-   arm_smmu_cmdq_issue_cmd(smmu, );
-   arm_smmu_cmdq_issue_sync(smmu);
+   arm_smmu_cmdq_issue_cmd_with_sync(smmu, );
 }
 
 static void arm_smmu_sync_cd(struct arm_smmu_domain *smmu_domain,
@@ -1211,8 +1218,7 @@ static void arm_smmu_sync_ste_for_sid(struct 
arm_smmu_device *smmu, u32 sid)
},
};
 
-   arm_smmu_cmdq_issue_cmd(smmu, );
-   arm_smmu_cmdq_issue_sync(smmu);
+   arm_smmu_cmdq_issue_cmd_with_sync(smmu, );
 }
 
 static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
@@ -1825,8 +1831,7 @@ static void arm_smmu_tlb_inv_context(void *cookie)
} else {
cmd.opcode  = CMDQ_OP_TLBI_S12_VMALL;
cmd.tlbi.vmid   = smmu_domain->s2_cfg.vmid;
-   arm_smmu_cmdq_issue_cmd(smmu, );
-   arm_smmu_cmdq_issue_sync(smmu);
+   arm_smmu_cmdq_issue_cmd_with_sync(smmu, );
}
arm_smmu_atc_inv_domain(smmu_domain, 0, 0, 0);
 }
@@ -3340,18 +3345,16 @@ static int arm_smmu_device_reset(struct arm_smmu_device 
*smmu, bool bypass)
 
/* Invalidate any cached configuration */
cmd.opcode = CMDQ_OP_CFGI_ALL;
-   arm_smmu_cmdq_issue_cmd(smmu, );
-   arm_smmu_cmdq_issue_sync(smmu);
+   arm_smmu_cmdq_issue_cmd_with_sync(smmu, );
 
/* Invalidate any stale TLB entries */
if (smmu->features & ARM_SMMU_FEAT_HYP) {
cmd.opcode = CMDQ_OP_TLBI_EL2_ALL;
-   arm_smmu_cmdq_issue_cmd(smmu, );
+   arm_smmu_cmdq_issue_cmd_with_sync(smmu, );
}
 
cmd.opcode = CMDQ_OP_TLBI_NSNH_ALL;
-   arm_smmu_cmdq_issue_cmd(smmu, );
-   arm_smmu_cmdq_issue_sync(smmu);
+   arm_smmu_cmdq_issue_cmd_with_sync(smmu, );
 
/* Event queue */
writeq_relaxed(smmu->evtq.q.q_base, smmu->base + ARM_SMMU_EVTQ_BASE);
-- 
2.26.0.106.g9fadedd

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 1/4] iommu/arm-smmu-v3: Use command queue batching helpers to improve performance

2021-08-16 Thread John Garry

In addition, I find that function arm_smmu_cmdq_build_cmd() can also be 
optimized
slightly, three useless instructions can be reduced.


I think that you could optimise further by pre-building commonly used 
commands.


For example, CMD_SYNC without MSI polling is always the same. And then 
only different in 1 field for MSI polling.


But you need to check if the performance gain is worth the change.



Case 1):
void arm_smmu_cmdq_build_cmd_tst1(u64 *cmd, struct arm_smmu_cmdq_ent *ent)
{
 memset(cmd, 0, 1 << CMDQ_ENT_SZ_SHIFT);
 cmd[0] |= FIELD_PREP(CMDQ_0_OP, ent->opcode);
}
4608 :
 4608:   a9007c1fstp xzr, xzr, [x0]
 460c:   39400022ldrbw2, [x1]
 4610:   f941ldr x1, [x0]
 4614:   aa020021orr x1, x1, x2
 4618:   f901str x1, [x0]
 461c:   d65f03c0ret

Case 2):
void arm_smmu_cmdq_build_cmd_tst2(u64 *cmd, struct arm_smmu_cmdq_ent *ent)
{
 int i;

 cmd[0] = FIELD_PREP(CMDQ_0_OP, ent->opcode);
 for (i = 1; i < CMDQ_ENT_DWORDS; i++)
 cmd[i] = 0;
}
4620 :
 4620:   39400021ldrbw1, [x1]
 4624:   a9007c01stp x1, xzr, [x0]
 4628:   d65f03c0ret
 462c:   d503201fnop

Case 3):
void arm_smmu_cmdq_build_cmd_tst3(u64 *cmd, struct arm_smmu_cmdq_ent *ent)
{
 memset(cmd, 0, 1 << CMDQ_ENT_SZ_SHIFT);
 cmd[0] = FIELD_PREP(CMDQ_0_OP, ent->opcode);
}
4630 :
 4630:   a9007c1fstp xzr, xzr, [x0]
 4634:   39400021ldrbw1, [x1]
 4638:   f901str x1, [x0]
 463c:   d65f03c0ret



___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [iommu:apple/dart 3/3] drivers/iommu/apple-dart.c:730:17: error: initialization of 'size_t (*)(struct iommu_domain *, long unsigned int, size_t, struct iommu_iotlb_gather *)' {aka 'long unsigned

2021-08-16 Thread Sven Peter via iommu



On Thu, Aug 12, 2021, at 13:29, Joerg Roedel wrote:
> Hi Sven,
> 
> On Tue, Aug 10, 2021 at 08:09:53AM +0200, Sven Peter wrote:
> > This happens because apple/dart is missing the "Optimizing 
> > iommu_[map/unmap] performance"
> > series which is already in the core branch [1].
> > The same commit works fine in iommu/next since that branch merges both 
> > iommu/core and
> > apple/dart.
> 
> Okay, thanks. I re-based the DART patches on-top of my core branch,
> which contains the changes for iommu_[map/unmap] performance. I
> generally don't like rebasing topic branches, but made an exception here
> to not break bisectability.
> 
> Thanks,
> 
>   Joerg
> 

Hi Joerg,

Thanks, and sorry about that! I'll try to make it more clear if anything depends
on another series in the future or just try to avoid it altogether if possible.


Just a heads up about a similar situation you may already be aware of: Once 
Robin's
DMA domain strictness refactoring [1] is merged, the current DART driver will 
fail due
to patch 12 there, which unexports iommu_get_dma_cookie. It'll need a small
adjustment just like all the other drivers (which will also fix two small bugs
it just made me notice: I never use iommu_put_dma_cookie and also 
unconditionally
grab a DMA cookie for all domain types).

Unless I'm mistaken I can't make that adjustment before the first patch of
that series has been merged, and Robin can't make that adjustment in his series
because it'll presumably go through another topic branch.


Best,


Sven

[1] 
https://lore.kernel.org/linux-iommu/cover.1628682048.git.robin.mur...@arm.com/


___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu