Re: [PATCH v4 12/14] swiotlb: Add restricted DMA alloc/free support.

2021-02-26 Thread Claire Chang
On Fri, Feb 26, 2021 at 1:17 PM Christoph Hellwig  wrote:
>
> On Fri, Feb 26, 2021 at 12:17:50PM +0800, Claire Chang wrote:
> > Do you think I should fix this and rebase on the latest linux-next
> > now? I wonder if there are more factor and clean up coming and I
> > should wait after that.
>
> Here is my preferred plan:
>
>  1) wait for my series to support the min alignment in swiotlb to
> land in Linus tree
>  2) I'll resend my series with the further swiotlb cleanup and
> refactoring, which includes a slightly rebased version of your
> patch to add the io_tlb_mem structure
>  3) resend your series on top of that as a baseline
>
> This is my current WIP tree for 2:
>
>   
> http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/swiotlb-struct

Sounds good to me. Thanks!


Re: [PATCH v4 12/14] swiotlb: Add restricted DMA alloc/free support.

2021-02-25 Thread Claire Chang
> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> index fd9c1bd183ac..8b77fd64199e 100644
> --- a/kernel/dma/swiotlb.c
> +++ b/kernel/dma/swiotlb.c
> @@ -836,6 +836,40 @@ late_initcall(swiotlb_create_default_debugfs);
>  #endif
>
>  #ifdef CONFIG_DMA_RESTRICTED_POOL
> +struct page *dev_swiotlb_alloc(struct device *dev, size_t size, gfp_t gfp)
> +{
> +   struct swiotlb *swiotlb;
> +   phys_addr_t tlb_addr;
> +   unsigned int index;
> +
> +   /* dev_swiotlb_alloc can be used only in the context which permits 
> sleeping. */
> +   if (!dev->dev_swiotlb || !gfpflags_allow_blocking(gfp))

Just noticed that !gfpflags_allow_blocking(gfp) shouldn't be here.

Hi Christoph,

Do you think I should fix this and rebase on the latest linux-next
now? I wonder if there are more factor and clean up coming and I
should wait after that.

Thanks,
Claire


Re: [RFC PATCH v3 0/6] Restricted DMA

2021-02-08 Thread Claire Chang
v4 here: https://lore.kernel.org/patchwork/cover/1378113/


[PATCH v4 14/14] of: Add plumbing for restricted DMA pool

2021-02-08 Thread Claire Chang
If a device is not behind an IOMMU, we look up the device node and set
up the restricted DMA when the restricted-dma-pool is presented.

Signed-off-by: Claire Chang 
---
 drivers/of/address.c| 25 +
 drivers/of/device.c |  3 +++
 drivers/of/of_private.h |  5 +
 3 files changed, 33 insertions(+)

diff --git a/drivers/of/address.c b/drivers/of/address.c
index 73ddf2540f3f..b6093c9b135d 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -8,6 +8,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -1094,3 +1095,27 @@ bool of_dma_is_coherent(struct device_node *np)
return false;
 }
 EXPORT_SYMBOL_GPL(of_dma_is_coherent);
+
+int of_dma_set_restricted_buffer(struct device *dev)
+{
+   struct device_node *node;
+   int count, i;
+
+   if (!dev->of_node)
+   return 0;
+
+   count = of_property_count_elems_of_size(dev->of_node, "memory-region",
+   sizeof(phandle));
+   for (i = 0; i < count; i++) {
+   node = of_parse_phandle(dev->of_node, "memory-region", i);
+   /* There might be multiple memory regions, but only one
+* restriced-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, dev->of_node, i);
+   }
+
+   return 0;
+}
diff --git a/drivers/of/device.c b/drivers/of/device.c
index 1122daa8e273..38c631f1fafa 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -186,6 +186,9 @@ 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);
+
return 0;
 }
 EXPORT_SYMBOL_GPL(of_dma_configure_id);
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index d9e6a324de0a..28a2dfa197ba 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -161,12 +161,17 @@ 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);
 #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_get_restricted_buffer(struct device *dev)
+{
+   return -ENODEV;
+}
 #endif
 
 #endif /* _LINUX_OF_PRIVATE_H */
-- 
2.30.0.478.g8a0d178c01-goog



[PATCH v4 12/14] swiotlb: Add restricted DMA alloc/free support.

2021-02-08 Thread Claire Chang
Add the functions, dev_swiotlb_{alloc,free} to support the memory
allocation from restricted DMA pool.

Signed-off-by: Claire Chang 
---
 include/linux/swiotlb.h |  2 ++
 kernel/dma/direct.c | 30 ++
 kernel/dma/swiotlb.c| 34 ++
 3 files changed, 58 insertions(+), 8 deletions(-)

diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index b9f2a250c8da..2cd39e102915 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -74,6 +74,8 @@ extern enum swiotlb_force swiotlb_force;
 #ifdef CONFIG_DMA_RESTRICTED_POOL
 bool is_swiotlb_force(struct device *dev);
 bool is_dev_swiotlb_force(struct device *dev);
+struct page *dev_swiotlb_alloc(struct device *dev, size_t size, gfp_t gfp);
+bool dev_swiotlb_free(struct device *dev, struct page *page, size_t size);
 #else
 static inline bool is_swiotlb_force(struct device *dev)
 {
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index a76a1a2f24da..f9a9321f7559 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -12,6 +12,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include "direct.h"
 
@@ -77,6 +78,10 @@ static bool dma_coherent_ok(struct device *dev, phys_addr_t 
phys, size_t size)
 
 static void __dma_direct_free_pages(struct device *dev, struct page *page, 
size_t size)
 {
+#ifdef CONFIG_DMA_RESTRICTED_POOL
+   if (dev_swiotlb_free(dev, page, size))
+   return;
+#endif
dma_free_contiguous(dev, page, size);
 }
 
@@ -89,6 +94,12 @@ static struct page *__dma_direct_alloc_pages(struct device 
*dev, size_t size,
 
WARN_ON_ONCE(!PAGE_ALIGNED(size));
 
+#ifdef CONFIG_DMA_RESTRICTED_POOL
+   page = dev_swiotlb_alloc(dev, size, gfp);
+   if (page)
+   return page;
+#endif
+
gfp |= dma_direct_optimal_gfp_mask(dev, dev->coherent_dma_mask,
   _limit);
page = dma_alloc_contiguous(dev, size, gfp);
@@ -147,7 +158,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
gfp |= __GFP_NOWARN;
 
if ((attrs & DMA_ATTR_NO_KERNEL_MAPPING) &&
-   !force_dma_unencrypted(dev)) {
+   !force_dma_unencrypted(dev) && !is_dev_swiotlb_force(dev)) {
page = __dma_direct_alloc_pages(dev, size, gfp & ~__GFP_ZERO);
if (!page)
return NULL;
@@ -160,8 +171,8 @@ void *dma_direct_alloc(struct device *dev, size_t size,
}
 
if (!IS_ENABLED(CONFIG_ARCH_HAS_DMA_SET_UNCACHED) &&
-   !IS_ENABLED(CONFIG_DMA_DIRECT_REMAP) &&
-   !dev_is_dma_coherent(dev))
+   !IS_ENABLED(CONFIG_DMA_DIRECT_REMAP) && !dev_is_dma_coherent(dev) &&
+   !is_dev_swiotlb_force(dev))
return arch_dma_alloc(dev, size, dma_handle, gfp, attrs);
 
/*
@@ -171,7 +182,9 @@ void *dma_direct_alloc(struct device *dev, size_t size,
if (IS_ENABLED(CONFIG_DMA_COHERENT_POOL) &&
!gfpflags_allow_blocking(gfp) &&
(force_dma_unencrypted(dev) ||
-(IS_ENABLED(CONFIG_DMA_DIRECT_REMAP) && 
!dev_is_dma_coherent(dev
+(IS_ENABLED(CONFIG_DMA_DIRECT_REMAP) &&
+ !dev_is_dma_coherent(dev))) &&
+   !is_dev_swiotlb_force(dev))
return dma_direct_alloc_from_pool(dev, size, dma_handle, gfp);
 
/* we always manually zero the memory once we are done */
@@ -252,15 +265,15 @@ void dma_direct_free(struct device *dev, size_t size,
unsigned int page_order = get_order(size);
 
if ((attrs & DMA_ATTR_NO_KERNEL_MAPPING) &&
-   !force_dma_unencrypted(dev)) {
+   !force_dma_unencrypted(dev) && !is_dev_swiotlb_force(dev)) {
/* cpu_addr is a struct page cookie, not a kernel address */
dma_free_contiguous(dev, cpu_addr, size);
return;
}
 
if (!IS_ENABLED(CONFIG_ARCH_HAS_DMA_SET_UNCACHED) &&
-   !IS_ENABLED(CONFIG_DMA_DIRECT_REMAP) &&
-   !dev_is_dma_coherent(dev)) {
+   !IS_ENABLED(CONFIG_DMA_DIRECT_REMAP) && !dev_is_dma_coherent(dev) &&
+   !is_dev_swiotlb_force(dev)) {
arch_dma_free(dev, size, cpu_addr, dma_addr, attrs);
return;
}
@@ -288,7 +301,8 @@ struct page *dma_direct_alloc_pages(struct device *dev, 
size_t size,
void *ret;
 
if (IS_ENABLED(CONFIG_DMA_COHERENT_POOL) &&
-   force_dma_unencrypted(dev) && !gfpflags_allow_blocking(gfp))
+   force_dma_unencrypted(dev) && !gfpflags_allow_blocking(gfp) &&
+   !is_dev_swiotlb_force(dev))
return dma_direct_alloc_from_pool(dev, size, dma_handle, gfp);
 
page = __dma_direct_alloc_pages(dev, size, gfp);
diff --git a/kernel/dma

[PATCH v4 11/14] swiotlb: Add is_dev_swiotlb_force()

2021-02-08 Thread Claire Chang
Add is_dev_swiotlb_force() which returns true if the device has
restricted DMA pool (e.g. dev->dev_swiotlb is set).

Signed-off-by: Claire Chang 
---
 include/linux/swiotlb.h | 9 +
 kernel/dma/swiotlb.c| 5 +
 2 files changed, 14 insertions(+)

diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index 76f86c684524..b9f2a250c8da 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -73,11 +73,16 @@ extern enum swiotlb_force swiotlb_force;
 
 #ifdef CONFIG_DMA_RESTRICTED_POOL
 bool is_swiotlb_force(struct device *dev);
+bool is_dev_swiotlb_force(struct device *dev);
 #else
 static inline bool is_swiotlb_force(struct device *dev)
 {
return unlikely(swiotlb_force == SWIOTLB_FORCE);
 }
+static inline bool is_dev_swiotlb_force(struct device *dev)
+{
+   return false;
+}
 #endif /* CONFIG_DMA_RESTRICTED_POOL */
 
 bool is_swiotlb_buffer(struct device *dev, phys_addr_t paddr);
@@ -93,6 +98,10 @@ static inline bool is_swiotlb_force(struct device *dev)
 {
return false;
 }
+static inline bool is_dev_swiotlb_force(struct device *dev)
+{
+   return false;
+}
 static inline bool is_swiotlb_buffer(struct device *dev, phys_addr_t paddr)
 {
return false;
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index f64cbe6e84cc..fd9c1bd183ac 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -841,6 +841,11 @@ bool is_swiotlb_force(struct device *dev)
return unlikely(swiotlb_force == SWIOTLB_FORCE) || dev->dev_swiotlb;
 }
 
+bool is_dev_swiotlb_force(struct device *dev)
+{
+   return dev->dev_swiotlb;
+}
+
 static int rmem_swiotlb_device_init(struct reserved_mem *rmem,
struct device *dev)
 {
-- 
2.30.0.478.g8a0d178c01-goog



[PATCH v4 08/14] swiotlb: Use restricted DMA pool if available

2021-02-08 Thread Claire Chang
Regardless of swiotlb setting, the restricted DMA pool is preferred if
available.

The restricted DMA pools provide a basic level of protection against the
DMA overwriting buffer contents at unexpected times. However, to protect
against general data leakage and system memory corruption, the system
needs to provide a way to lock down the memory access, e.g., MPU.

Signed-off-by: Claire Chang 
---
 include/linux/swiotlb.h | 13 +
 kernel/dma/direct.h |  2 +-
 kernel/dma/swiotlb.c| 20 +---
 3 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index f13a52a97382..76f86c684524 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -71,6 +71,15 @@ dma_addr_t swiotlb_map(struct device *dev, phys_addr_t phys,
 #ifdef CONFIG_SWIOTLB
 extern enum swiotlb_force swiotlb_force;
 
+#ifdef CONFIG_DMA_RESTRICTED_POOL
+bool is_swiotlb_force(struct device *dev);
+#else
+static inline bool is_swiotlb_force(struct device *dev)
+{
+   return unlikely(swiotlb_force == SWIOTLB_FORCE);
+}
+#endif /* CONFIG_DMA_RESTRICTED_POOL */
+
 bool is_swiotlb_buffer(struct device *dev, phys_addr_t paddr);
 void __init swiotlb_exit(void);
 unsigned int swiotlb_max_segment(void);
@@ -80,6 +89,10 @@ phys_addr_t get_swiotlb_start(struct device *dev);
 void __init swiotlb_adjust_size(unsigned long new_size);
 #else
 #define swiotlb_force SWIOTLB_NO_FORCE
+static inline bool is_swiotlb_force(struct device *dev)
+{
+   return false;
+}
 static inline bool is_swiotlb_buffer(struct device *dev, phys_addr_t paddr)
 {
return false;
diff --git a/kernel/dma/direct.h b/kernel/dma/direct.h
index 7b83b1595989..b011db1b625d 100644
--- a/kernel/dma/direct.h
+++ b/kernel/dma/direct.h
@@ -87,7 +87,7 @@ static inline dma_addr_t dma_direct_map_page(struct device 
*dev,
phys_addr_t phys = page_to_phys(page) + offset;
dma_addr_t dma_addr = phys_to_dma(dev, phys);
 
-   if (unlikely(swiotlb_force == SWIOTLB_FORCE))
+   if (is_swiotlb_force(dev))
return swiotlb_map(dev, phys, size, dir, attrs);
 
if (unlikely(!dma_capable(dev, dma_addr, size, true))) {
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index e22e7ae75f1c..6fdebde8fb1f 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -40,6 +40,7 @@
 #include 
 #endif
 #ifdef CONFIG_DMA_RESTRICTED_POOL
+#include 
 #include 
 #include 
 #include 
@@ -109,6 +110,10 @@ static struct swiotlb default_swiotlb;
 
 static inline struct swiotlb *get_swiotlb(struct device *dev)
 {
+#ifdef CONFIG_DMA_RESTRICTED_POOL
+   if (dev && dev->dev_swiotlb)
+   return dev->dev_swiotlb;
+#endif
return _swiotlb;
 }
 
@@ -508,7 +513,7 @@ phys_addr_t swiotlb_tbl_map_single(struct device *hwdev, 
phys_addr_t orig_addr,
size_t mapping_size, size_t alloc_size,
enum dma_data_direction dir, unsigned long attrs)
 {
-   struct swiotlb *swiotlb = _swiotlb;
+   struct swiotlb *swiotlb = get_swiotlb(hwdev);
dma_addr_t tbl_dma_addr = phys_to_dma_unencrypted(hwdev, 
swiotlb->start);
unsigned long flags;
phys_addr_t tlb_addr;
@@ -519,7 +524,11 @@ phys_addr_t swiotlb_tbl_map_single(struct device *hwdev, 
phys_addr_t orig_addr,
unsigned long max_slots;
unsigned long tmp_io_tlb_used;
 
+#ifdef CONFIG_DMA_RESTRICTED_POOL
+   if (no_iotlb_memory && !hwdev->dev_swiotlb)
+#else
if (no_iotlb_memory)
+#endif
panic("Can not allocate SWIOTLB buffer earlier and can't now 
provide you with the DMA bounce buffer");
 
if (mem_encrypt_active())
@@ -641,7 +650,7 @@ void swiotlb_tbl_unmap_single(struct device *hwdev, 
phys_addr_t tlb_addr,
  size_t mapping_size, size_t alloc_size,
  enum dma_data_direction dir, unsigned long attrs)
 {
-   struct swiotlb *swiotlb = _swiotlb;
+   struct swiotlb *swiotlb = get_swiotlb(hwdev);
unsigned long flags;
int i, count, nslots = ALIGN(alloc_size, 1 << IO_TLB_SHIFT) >> 
IO_TLB_SHIFT;
int index = (tlb_addr - swiotlb->start) >> IO_TLB_SHIFT;
@@ -689,7 +698,7 @@ void swiotlb_tbl_sync_single(struct device *hwdev, 
phys_addr_t tlb_addr,
 size_t size, enum dma_data_direction dir,
 enum dma_sync_target target)
 {
-   struct swiotlb *swiotlb = _swiotlb;
+   struct swiotlb *swiotlb = get_swiotlb(hwdev);
int index = (tlb_addr - swiotlb->start) >> IO_TLB_SHIFT;
phys_addr_t orig_addr = swiotlb->orig_addr[index];
 
@@ -801,6 +810,11 @@ late_initcall(swiotlb_create_default_debugfs);
 #endif
 
 #ifdef CONFIG_DMA_RESTRICTED_POOL
+bool is_swiotlb_force(struct device *dev)
+{
+   return unlikely(swiotlb_force == SWIOTLB_FORCE) || dev->dev_sw

[PATCH v4 09/14] swiotlb: Refactor swiotlb_tbl_{map,unmap}_single

2021-02-08 Thread Claire Chang
Refactor swiotlb_tbl_{map,unmap}_single to make the code reusable for
dev_swiotlb_{alloc,free}.

Signed-off-by: Claire Chang 
---
 kernel/dma/swiotlb.c | 116 ++-
 1 file changed, 71 insertions(+), 45 deletions(-)

diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 6fdebde8fb1f..f64cbe6e84cc 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -509,14 +509,12 @@ static void swiotlb_bounce(phys_addr_t orig_addr, 
phys_addr_t tlb_addr,
}
 }
 
-phys_addr_t swiotlb_tbl_map_single(struct device *hwdev, phys_addr_t orig_addr,
-   size_t mapping_size, size_t alloc_size,
-   enum dma_data_direction dir, unsigned long attrs)
+static int swiotlb_tbl_find_free_region(struct device *hwdev,
+   dma_addr_t tbl_dma_addr,
+   size_t alloc_size, unsigned long attrs)
 {
struct swiotlb *swiotlb = get_swiotlb(hwdev);
-   dma_addr_t tbl_dma_addr = phys_to_dma_unencrypted(hwdev, 
swiotlb->start);
unsigned long flags;
-   phys_addr_t tlb_addr;
unsigned int nslots, stride, index, wrap;
int i;
unsigned long mask;
@@ -531,15 +529,6 @@ phys_addr_t swiotlb_tbl_map_single(struct device *hwdev, 
phys_addr_t orig_addr,
 #endif
panic("Can not allocate SWIOTLB buffer earlier and can't now 
provide you with the DMA bounce buffer");
 
-   if (mem_encrypt_active())
-   pr_warn_once("Memory encryption is active and system is using 
DMA bounce buffers\n");
-
-   if (mapping_size > alloc_size) {
-   dev_warn_once(hwdev, "Invalid sizes (mapping: %zd bytes, alloc: 
%zd bytes)",
- mapping_size, alloc_size);
-   return (phys_addr_t)DMA_MAPPING_ERROR;
-   }
-
mask = dma_get_seg_boundary(hwdev);
 
tbl_dma_addr &= mask;
@@ -601,7 +590,6 @@ phys_addr_t swiotlb_tbl_map_single(struct device *hwdev, 
phys_addr_t orig_addr,
swiotlb->list[i] = 0;
for (i = index - 1; (OFFSET(i, IO_TLB_SEGSIZE) != 
IO_TLB_SEGSIZE - 1) && swiotlb->list[i]; i--)
swiotlb->list[i] = ++count;
-   tlb_addr = swiotlb->start + (index << IO_TLB_SHIFT);
 
/*
 * Update the indices to avoid searching in the next
@@ -624,45 +612,20 @@ phys_addr_t swiotlb_tbl_map_single(struct device *hwdev, 
phys_addr_t orig_addr,
if (!(attrs & DMA_ATTR_NO_WARN) && printk_ratelimit())
dev_warn(hwdev, "swiotlb buffer is full (sz: %zd bytes), total 
%lu (slots), used %lu (slots)\n",
 alloc_size, swiotlb->nslabs, tmp_io_tlb_used);
-   return (phys_addr_t)DMA_MAPPING_ERROR;
+   return -ENOMEM;
+
 found:
swiotlb->used += nslots;
spin_unlock_irqrestore(>lock, flags);
 
-   /*
-* Save away the mapping from the original address to the DMA address.
-* This is needed when we sync the memory.  Then we sync the buffer if
-* needed.
-*/
-   for (i = 0; i < nslots; i++)
-   swiotlb->orig_addr[index+i] = orig_addr + (i << IO_TLB_SHIFT);
-   if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC) &&
-   (dir == DMA_TO_DEVICE || dir == DMA_BIDIRECTIONAL))
-   swiotlb_bounce(orig_addr, tlb_addr, mapping_size, 
DMA_TO_DEVICE);
-
-   return tlb_addr;
+   return index;
 }
 
-/*
- * tlb_addr is the physical address of the bounce buffer to unmap.
- */
-void swiotlb_tbl_unmap_single(struct device *hwdev, phys_addr_t tlb_addr,
- size_t mapping_size, size_t alloc_size,
- enum dma_data_direction dir, unsigned long attrs)
+static void swiotlb_tbl_release_region(struct device *hwdev, int index, size_t 
size)
 {
struct swiotlb *swiotlb = get_swiotlb(hwdev);
unsigned long flags;
-   int i, count, nslots = ALIGN(alloc_size, 1 << IO_TLB_SHIFT) >> 
IO_TLB_SHIFT;
-   int index = (tlb_addr - swiotlb->start) >> IO_TLB_SHIFT;
-   phys_addr_t orig_addr = swiotlb->orig_addr[index];
-
-   /*
-* First, sync the memory before unmapping the entry
-*/
-   if (orig_addr != INVALID_PHYS_ADDR &&
-   !(attrs & DMA_ATTR_SKIP_CPU_SYNC) &&
-   ((dir == DMA_FROM_DEVICE) || (dir == DMA_BIDIRECTIONAL)))
-   swiotlb_bounce(orig_addr, tlb_addr, mapping_size, 
DMA_FROM_DEVICE);
+   int i, count, nslots = ALIGN(size, 1 << IO_TLB_SHIFT) >> IO_TLB_SHIFT;
 
/*
 * Return the buffer to the free list by setting the corresponding
@@ -694,6 +657,69 @@ void swiotlb_tbl_unmap_single(struct device *hwdev, 
phys_addr_t tlb_addr,
s

[PATCH v4 10/14] dma-direct: Add a new wrapper __dma_direct_free_pages()

2021-02-08 Thread Claire Chang
Add a new wrapper __dma_direct_free_pages() that will be useful later
for dev_swiotlb_free().

Signed-off-by: Claire Chang 
---
 kernel/dma/direct.c | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 30ccbc08e229..a76a1a2f24da 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -75,6 +75,11 @@ static bool dma_coherent_ok(struct device *dev, phys_addr_t 
phys, size_t size)
min_not_zero(dev->coherent_dma_mask, dev->bus_dma_limit);
 }
 
+static void __dma_direct_free_pages(struct device *dev, struct page *page, 
size_t size)
+{
+   dma_free_contiguous(dev, page, size);
+}
+
 static struct page *__dma_direct_alloc_pages(struct device *dev, size_t size,
gfp_t gfp)
 {
@@ -237,7 +242,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
return NULL;
}
 out_free_pages:
-   dma_free_contiguous(dev, page, size);
+   __dma_direct_free_pages(dev, page, size);
return NULL;
 }
 
@@ -273,7 +278,7 @@ void dma_direct_free(struct device *dev, size_t size,
else if (IS_ENABLED(CONFIG_ARCH_HAS_DMA_CLEAR_UNCACHED))
arch_dma_clear_uncached(cpu_addr, size);
 
-   dma_free_contiguous(dev, dma_direct_to_page(dev, dma_addr), size);
+   __dma_direct_free_pages(dev, dma_direct_to_page(dev, dma_addr), size);
 }
 
 struct page *dma_direct_alloc_pages(struct device *dev, size_t size,
@@ -310,7 +315,7 @@ struct page *dma_direct_alloc_pages(struct device *dev, 
size_t size,
*dma_handle = phys_to_dma_direct(dev, page_to_phys(page));
return page;
 out_free_pages:
-   dma_free_contiguous(dev, page, size);
+   __dma_direct_free_pages(dev, page, size);
return NULL;
 }
 
@@ -329,7 +334,7 @@ void dma_direct_free_pages(struct device *dev, size_t size,
if (force_dma_unencrypted(dev))
set_memory_encrypted((unsigned long)vaddr, 1 << page_order);
 
-   dma_free_contiguous(dev, page, size);
+   __dma_direct_free_pages(dev, page, size);
 }
 
 #if defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_DEVICE) || \
-- 
2.30.0.478.g8a0d178c01-goog



[PATCH v4 13/14] dt-bindings: of: Add restricted DMA pool

2021-02-08 Thread Claire Chang
Introduce the new compatible string, restricted-dma-pool, for restricted
DMA. One can specify the address and length of the restricted DMA memory
region by restricted-dma-pool in the reserved-memory node.

Signed-off-by: Claire Chang 
---
 .../reserved-memory/reserved-memory.txt   | 24 +++
 1 file changed, 24 insertions(+)

diff --git 
a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt 
b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
index e8d3096d922c..fc9a12c2f679 100644
--- a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
+++ b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
@@ -51,6 +51,20 @@ compatible (optional) - standard definition
   used as a shared pool of DMA buffers for a set of devices. It can
   be used by an operating system to instantiate the necessary pool
   management subsystem if necessary.
+- restricted-dma-pool: This indicates a region of memory meant to be
+  used as a pool of restricted DMA buffers for a set of devices. The
+  memory region would be the only region accessible to those devices.
+  When using this, the no-map and reusable properties must not be set,
+  so the operating system can create a virtual mapping that will be 
used
+  for synchronization. The main purpose for restricted DMA is to
+  mitigate the lack of DMA access control on systems without an IOMMU,
+  which could result in the DMA accessing the system memory at
+  unexpected times and/or unexpected addresses, possibly leading to 
data
+  leakage or corruption. The feature on its own provides a basic level
+  of protection against the DMA overwriting buffer contents at
+  unexpected times. However, to protect against general data leakage 
and
+  system memory corruption, the system needs to provide way to lock 
down
+  the memory access, e.g., MPU.
 - vendor specific string in the form ,[-]
 no-map (optional) - empty property
 - Indicates the operating system must not create a virtual mapping
@@ -120,6 +134,11 @@ one for multimedia processing (named 
multimedia-memory@7700, 64MiB).
compatible = "acme,multimedia-memory";
reg = <0x7700 0x400>;
};
+
+   restricted_dma_mem_reserved: restricted_dma_mem_reserved {
+   compatible = "restricted-dma-pool";
+   reg = <0x5000 0x40>;
+   };
};
 
/* ... */
@@ -138,4 +157,9 @@ one for multimedia processing (named 
multimedia-memory@7700, 64MiB).
memory-region = <_reserved>;
/* ... */
};
+
+   pcie_device: pcie_device@0,0 {
+   memory-region = <_dma_mem_reserved>;
+   /* ... */
+   };
 };
-- 
2.30.0.478.g8a0d178c01-goog



[PATCH v4 07/14] swiotlb: Update swiotlb API to gain a struct device argument

2021-02-08 Thread Claire Chang
Introduce the get_swiotlb() getter and update all callers of
is_swiotlb_active(), is_swiotlb_buffer() and get_swiotlb_start() to gain
a struct device argument.

Signed-off-by: Claire Chang 
---
 drivers/iommu/dma-iommu.c | 12 ++--
 drivers/xen/swiotlb-xen.c |  4 ++--
 include/linux/swiotlb.h   | 10 +-
 kernel/dma/direct.c   |  8 
 kernel/dma/direct.h   |  6 +++---
 kernel/dma/swiotlb.c  | 23 +--
 6 files changed, 37 insertions(+), 26 deletions(-)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index f659395e7959..abdbe14472cc 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -503,7 +503,7 @@ static void __iommu_dma_unmap_swiotlb(struct device *dev, 
dma_addr_t dma_addr,
 
__iommu_dma_unmap(dev, dma_addr, size);
 
-   if (unlikely(is_swiotlb_buffer(phys)))
+   if (unlikely(is_swiotlb_buffer(dev, phys)))
swiotlb_tbl_unmap_single(dev, phys, size,
iova_align(iovad, size), dir, attrs);
 }
@@ -580,7 +580,7 @@ static dma_addr_t __iommu_dma_map_swiotlb(struct device 
*dev, phys_addr_t phys,
}
 
iova = __iommu_dma_map(dev, phys, aligned_size, prot, dma_mask);
-   if ((iova == DMA_MAPPING_ERROR) && is_swiotlb_buffer(phys))
+   if ((iova == DMA_MAPPING_ERROR) && is_swiotlb_buffer(dev, phys))
swiotlb_tbl_unmap_single(dev, phys, org_size,
aligned_size, dir, attrs);
 
@@ -753,7 +753,7 @@ static void iommu_dma_sync_single_for_cpu(struct device 
*dev,
if (!dev_is_dma_coherent(dev))
arch_sync_dma_for_cpu(phys, size, dir);
 
-   if (is_swiotlb_buffer(phys))
+   if (is_swiotlb_buffer(dev, phys))
swiotlb_tbl_sync_single(dev, phys, size, dir, SYNC_FOR_CPU);
 }
 
@@ -766,7 +766,7 @@ static void iommu_dma_sync_single_for_device(struct device 
*dev,
return;
 
phys = iommu_iova_to_phys(iommu_get_dma_domain(dev), dma_handle);
-   if (is_swiotlb_buffer(phys))
+   if (is_swiotlb_buffer(dev, phys))
swiotlb_tbl_sync_single(dev, phys, size, dir, SYNC_FOR_DEVICE);
 
if (!dev_is_dma_coherent(dev))
@@ -787,7 +787,7 @@ static void iommu_dma_sync_sg_for_cpu(struct device *dev,
if (!dev_is_dma_coherent(dev))
arch_sync_dma_for_cpu(sg_phys(sg), sg->length, dir);
 
-   if (is_swiotlb_buffer(sg_phys(sg)))
+   if (is_swiotlb_buffer(dev, sg_phys(sg)))
swiotlb_tbl_sync_single(dev, sg_phys(sg), sg->length,
dir, SYNC_FOR_CPU);
}
@@ -804,7 +804,7 @@ static void iommu_dma_sync_sg_for_device(struct device *dev,
return;
 
for_each_sg(sgl, sg, nelems, i) {
-   if (is_swiotlb_buffer(sg_phys(sg)))
+   if (is_swiotlb_buffer(dev, sg_phys(sg)))
swiotlb_tbl_sync_single(dev, sg_phys(sg), sg->length,
dir, SYNC_FOR_DEVICE);
 
diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index 91f8c68d1a9b..f424d46756b1 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -192,8 +192,8 @@ int __ref xen_swiotlb_init(int verbose, bool early)
/*
 * IO TLB memory already allocated. Just use it.
 */
-   if (is_swiotlb_active()) {
-   xen_io_tlb_start = phys_to_virt(get_swiotlb_start());
+   if (is_swiotlb_active(NULL)) {
+   xen_io_tlb_start = phys_to_virt(get_swiotlb_start(NULL));
goto end;
}
 
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index 041611bf3c2a..f13a52a97382 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -71,16 +71,16 @@ dma_addr_t swiotlb_map(struct device *dev, phys_addr_t phys,
 #ifdef CONFIG_SWIOTLB
 extern enum swiotlb_force swiotlb_force;
 
-bool is_swiotlb_buffer(phys_addr_t paddr);
+bool is_swiotlb_buffer(struct device *dev, phys_addr_t paddr);
 void __init swiotlb_exit(void);
 unsigned int swiotlb_max_segment(void);
 size_t swiotlb_max_mapping_size(struct device *dev);
-bool is_swiotlb_active(void);
-phys_addr_t get_swiotlb_start(void);
+bool is_swiotlb_active(struct device *dev);
+phys_addr_t get_swiotlb_start(struct device *dev);
 void __init swiotlb_adjust_size(unsigned long new_size);
 #else
 #define swiotlb_force SWIOTLB_NO_FORCE
-static inline bool is_swiotlb_buffer(phys_addr_t paddr)
+static inline bool is_swiotlb_buffer(struct device *dev, phys_addr_t paddr)
 {
return false;
 }
@@ -96,7 +96,7 @@ static inline size_t swiotlb_max_mapping_size(struct device 
*dev)
return SIZE_MAX;
 }
 
-static inline bool is_swiotlb_active(void)
+static inline bool is_swiotlb_active(struct device *dev)
 {
return false;
 }
diff --git a/kernel/dma/direct.c b/kernel/dma/di

[PATCH v4 06/14] swiotlb: Add restricted DMA pool

2021-02-08 Thread Claire Chang
Add the initialization function to create restricted DMA pools from
matching reserved-memory nodes.

Signed-off-by: Claire Chang 
---
 include/linux/device.h |  4 ++
 kernel/dma/swiotlb.c   | 94 +-
 2 files changed, 97 insertions(+), 1 deletion(-)

diff --git a/include/linux/device.h b/include/linux/device.h
index 7619a84f8ce4..08d440627b93 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -415,6 +415,7 @@ struct dev_links_info {
  * @dma_pools: Dma pools (if dma'ble device).
  * @dma_mem:   Internal for coherent mem override.
  * @cma_area:  Contiguous memory area for dma allocations
+ * @dev_swiotlb: Internal for swiotlb override.
  * @archdata:  For arch-specific additions.
  * @of_node:   Associated device tree node.
  * @fwnode:Associated device node supplied by platform firmware.
@@ -517,6 +518,9 @@ struct device {
 #ifdef CONFIG_DMA_CMA
struct cma *cma_area;   /* contiguous memory area for dma
   allocations */
+#endif
+#ifdef CONFIG_DMA_RESTRICTED_POOL
+   struct swiotlb *dev_swiotlb;
 #endif
/* arch specific additions */
struct dev_archdata archdata;
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index dc37951c6924..3a17451c5981 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -39,6 +39,13 @@
 #ifdef CONFIG_DEBUG_FS
 #include 
 #endif
+#ifdef CONFIG_DMA_RESTRICTED_POOL
+#include 
+#include 
+#include 
+#include 
+#include 
+#endif
 
 #include 
 #include 
@@ -75,7 +82,8 @@ enum swiotlb_force swiotlb_force;
  *  range check to see if the memory was in fact allocated by this
  *  API.
  * @nslabs: The number of IO TLB blocks (in groups of 64) between @start 
and
- *  @end. This is command line adjustable via setup_io_tlb_npages.
+ *  @end. For default swiotlb, this is command line adjustable via
+ *  setup_io_tlb_npages.
  * @used:   The number of used IO TLB block.
  * @list:   The free list describing the number of free entries available
  *  from each index.
@@ -780,3 +788,87 @@ static int __init swiotlb_create_default_debugfs(void)
 
 late_initcall(swiotlb_create_default_debugfs);
 #endif
+
+#ifdef CONFIG_DMA_RESTRICTED_POOL
+static int rmem_swiotlb_device_init(struct reserved_mem *rmem,
+   struct device *dev)
+{
+   struct swiotlb *swiotlb = rmem->priv;
+   int ret;
+
+   if (dev->dev_swiotlb)
+   return -EBUSY;
+
+   /* Since multiple devices can share the same pool, the private data,
+* swiotlb struct, will be initialized by the first device attached
+* to it.
+*/
+   if (!swiotlb) {
+   swiotlb = kzalloc(sizeof(*swiotlb), GFP_KERNEL);
+   if (!swiotlb)
+   return -ENOMEM;
+#ifdef CONFIG_ARM
+   unsigned long pfn = PHYS_PFN(reme->base);
+
+   if (!PageHighMem(pfn_to_page(pfn))) {
+   ret = -EINVAL;
+   goto cleanup;
+   }
+#endif /* CONFIG_ARM */
+
+   ret = swiotlb_init_tlb_pool(swiotlb, rmem->base, rmem->size);
+   if (ret)
+   goto cleanup;
+
+   rmem->priv = swiotlb;
+   }
+
+#ifdef CONFIG_DEBUG_FS
+   swiotlb_create_debugfs(swiotlb, rmem->name, default_swiotlb.debugfs);
+#endif /* CONFIG_DEBUG_FS */
+
+   dev->dev_swiotlb = swiotlb;
+
+   return 0;
+
+cleanup:
+   kfree(swiotlb);
+
+   return ret;
+}
+
+static void rmem_swiotlb_device_release(struct reserved_mem *rmem,
+   struct device *dev)
+{
+   if (!dev)
+   return;
+
+#ifdef CONFIG_DEBUG_FS
+   debugfs_remove_recursive(dev->dev_swiotlb->debugfs);
+#endif /* CONFIG_DEBUG_FS */
+   dev->dev_swiotlb = NULL;
+}
+
+static const struct reserved_mem_ops rmem_swiotlb_ops = {
+   .device_init = rmem_swiotlb_device_init,
+   .device_release = rmem_swiotlb_device_release,
+};
+
+static int __init rmem_swiotlb_setup(struct reserved_mem *rmem)
+{
+   unsigned long node = rmem->fdt_node;
+
+   if (of_get_flat_dt_prop(node, "reusable", NULL) ||
+   of_get_flat_dt_prop(node, "linux,cma-default", NULL) ||
+   of_get_flat_dt_prop(node, "linux,dma-default", NULL) ||
+   of_get_flat_dt_prop(node, "no-map", NULL))
+   return -EINVAL;
+
+   rmem->ops = _swiotlb_ops;
+   pr_info("Reserved memory: created device swiotlb memory pool at %pa, 
size %ld MiB\n",
+   >base, (unsigned long)rmem->size / SZ_1M);
+   return 0;
+}
+
+RESERVEDMEM_OF_DECLARE(dma, "restricted-dma-pool", rmem_swiotlb_setup);
+#endif /* CONFIG_DMA_RESTRICTED_POOL */
-- 
2.30.0.478.g8a0d178c01-goog



[PATCH v4 05/14] swiotlb: Add DMA_RESTRICTED_POOL

2021-02-08 Thread Claire Chang
Add a new kconfig symbol, DMA_RESTRICTED_POOL, for restricted DMA pool.

Signed-off-by: Claire Chang 
---
 kernel/dma/Kconfig | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/kernel/dma/Kconfig b/kernel/dma/Kconfig
index 479fc145acfc..97ff9f8dd3c8 100644
--- a/kernel/dma/Kconfig
+++ b/kernel/dma/Kconfig
@@ -83,6 +83,20 @@ config SWIOTLB
bool
select NEED_DMA_MAP_STATE
 
+config DMA_RESTRICTED_POOL
+   bool "DMA Restricted Pool"
+   depends on OF && OF_RESERVED_MEM
+   select SWIOTLB
+   help
+ This enables support for restricted DMA pools which provide a level of
+ DMA memory protection on systems with limited hardware protection
+ capabilities, such as those lacking an IOMMU.
+
+ For more information see
+ 

+ and .
+ If unsure, say "n".
+
 #
 # Should be selected if we can mmap non-coherent mappings to userspace.
 # The only thing that is really required is a way to set an uncached bit
-- 
2.30.0.478.g8a0d178c01-goog



[PATCH v4 03/14] swiotlb: Add struct swiotlb

2021-02-08 Thread Claire Chang
Added a new struct, swiotlb, as the IO TLB memory pool descriptor and
moved relevant global variables into that struct.
This will be useful later to allow for restricted DMA pool.

Signed-off-by: Claire Chang 
---
 kernel/dma/swiotlb.c | 327 +++
 1 file changed, 172 insertions(+), 155 deletions(-)

diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 678490d39e55..28b7bfe7a2a8 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -61,33 +61,43 @@
  * allocate a contiguous 1MB, we're probably in trouble anyway.
  */
 #define IO_TLB_MIN_SLABS ((1<<20) >> IO_TLB_SHIFT)
+#define INVALID_PHYS_ADDR (~(phys_addr_t)0)
 
 enum swiotlb_force swiotlb_force;
 
 /*
- * Used to do a quick range check in swiotlb_tbl_unmap_single and
- * swiotlb_tbl_sync_single_*, to see if the memory was in fact allocated by 
this
- * API.
- */
-static phys_addr_t io_tlb_start, io_tlb_end;
-
-/*
- * The number of IO TLB blocks (in groups of 64) between io_tlb_start and
- * io_tlb_end.  This is command line adjustable via setup_io_tlb_npages.
- */
-static unsigned long io_tlb_nslabs;
-
-/*
- * The number of used IO TLB block
- */
-static unsigned long io_tlb_used;
-
-/*
- * This is a free list describing the number of free entries available from
- * each index
+ * struct swiotlb - Software IO TLB Memory Pool Descriptor
+ *
+ * @start:  The start address of the swiotlb memory pool. Used to do a 
quick
+ *  range check to see if the memory was in fact allocated by this
+ *  API.
+ * @end:The end address of the swiotlb memory pool. Used to do a quick
+ *  range check to see if the memory was in fact allocated by this
+ *  API.
+ * @nslabs: The number of IO TLB blocks (in groups of 64) between @start 
and
+ *  @end. This is command line adjustable via setup_io_tlb_npages.
+ * @used:   The number of used IO TLB block.
+ * @list:   The free list describing the number of free entries available
+ *  from each index.
+ * @index:  The index to start searching in the next round.
+ * @orig_addr:  The original address corresponding to a mapped entry for the
+ *  sync operations.
+ * @lock:   The lock to protect the above data structures in the map and
+ *  unmap calls.
+ * @debugfs:The dentry to debugfs.
  */
-static unsigned int *io_tlb_list;
-static unsigned int io_tlb_index;
+struct swiotlb {
+   phys_addr_t start;
+   phys_addr_t end;
+   unsigned long nslabs;
+   unsigned long used;
+   unsigned int *list;
+   unsigned int index;
+   phys_addr_t *orig_addr;
+   spinlock_t lock;
+   struct dentry *debugfs;
+};
+static struct swiotlb default_swiotlb;
 
 /*
  * Max segment that we can provide which (if pages are contingous) will
@@ -95,27 +105,17 @@ static unsigned int io_tlb_index;
  */
 static unsigned int max_segment;
 
-/*
- * We need to save away the original address corresponding to a mapped entry
- * for the sync operations.
- */
-#define INVALID_PHYS_ADDR (~(phys_addr_t)0)
-static phys_addr_t *io_tlb_orig_addr;
-
-/*
- * Protect the above data structures in the map and unmap calls
- */
-static DEFINE_SPINLOCK(io_tlb_lock);
-
 static int late_alloc;
 
 static int __init
 setup_io_tlb_npages(char *str)
 {
+   struct swiotlb *swiotlb = _swiotlb;
+
if (isdigit(*str)) {
-   io_tlb_nslabs = simple_strtoul(str, , 0);
+   swiotlb->nslabs = simple_strtoul(str, , 0);
/* avoid tail segment of size < IO_TLB_SEGSIZE */
-   io_tlb_nslabs = ALIGN(io_tlb_nslabs, IO_TLB_SEGSIZE);
+   swiotlb->nslabs = ALIGN(swiotlb->nslabs, IO_TLB_SEGSIZE);
}
if (*str == ',')
++str;
@@ -123,7 +123,7 @@ setup_io_tlb_npages(char *str)
swiotlb_force = SWIOTLB_FORCE;
} else if (!strcmp(str, "noforce")) {
swiotlb_force = SWIOTLB_NO_FORCE;
-   io_tlb_nslabs = 1;
+   swiotlb->nslabs = 1;
}
 
return 0;
@@ -134,7 +134,7 @@ static bool no_iotlb_memory;
 
 unsigned long swiotlb_nr_tbl(void)
 {
-   return unlikely(no_iotlb_memory) ? 0 : io_tlb_nslabs;
+   return unlikely(no_iotlb_memory) ? 0 : default_swiotlb.nslabs;
 }
 EXPORT_SYMBOL_GPL(swiotlb_nr_tbl);
 
@@ -156,13 +156,14 @@ unsigned long swiotlb_size_or_default(void)
 {
unsigned long size;
 
-   size = io_tlb_nslabs << IO_TLB_SHIFT;
+   size = default_swiotlb.nslabs << IO_TLB_SHIFT;
 
return size ? size : (IO_TLB_DEFAULT_SIZE);
 }
 
 void __init swiotlb_adjust_size(unsigned long new_size)
 {
+   struct swiotlb *swiotlb = _swiotlb;
unsigned long size;
 
/*
@@ -170,10 +171,10 @@ void __init swiotlb_adjust_size(unsigned long new_size)
 * architectures such as those supporting memory encryption to
  

[PATCH v4 04/14] swiotlb: Refactor swiotlb_late_init_with_tbl

2021-02-08 Thread Claire Chang
Refactor swiotlb_late_init_with_tbl to make the code reusable for
restricted DMA pool initialization.

Signed-off-by: Claire Chang 
---
 kernel/dma/swiotlb.c | 65 
 1 file changed, 42 insertions(+), 23 deletions(-)

diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 28b7bfe7a2a8..dc37951c6924 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -353,20 +353,21 @@ static void swiotlb_cleanup(void)
max_segment = 0;
 }
 
-int
-swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs)
+static int swiotlb_init_tlb_pool(struct swiotlb *swiotlb, phys_addr_t start,
+   size_t size)
 {
-   struct swiotlb *swiotlb = _swiotlb;
-   unsigned long i, bytes;
+   unsigned long i;
+   void *vaddr = phys_to_virt(start);
 
-   bytes = nslabs << IO_TLB_SHIFT;
+   size = ALIGN(size, 1 << IO_TLB_SHIFT);
+   swiotlb->nslabs = size >> IO_TLB_SHIFT;
+   swiotlb->nslabs = ALIGN(swiotlb->nslabs, IO_TLB_SEGSIZE);
 
-   swiotlb->nslabs = nslabs;
-   swiotlb->start = virt_to_phys(tlb);
-   swiotlb->end = swiotlb->start + bytes;
+   swiotlb->start = start;
+   swiotlb->end = swiotlb->start + size;
 
-   set_memory_decrypted((unsigned long)tlb, bytes >> PAGE_SHIFT);
-   memset(tlb, 0, bytes);
+   set_memory_decrypted((unsigned long)vaddr, size >> PAGE_SHIFT);
+   memset(vaddr, 0, size);
 
/*
 * Allocate and initialize the free list array.  This array is used
@@ -390,13 +391,7 @@ swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs)
swiotlb->orig_addr[i] = INVALID_PHYS_ADDR;
}
swiotlb->index = 0;
-   no_iotlb_memory = false;
-
-   swiotlb_print_info();
 
-   late_alloc = 1;
-
-   swiotlb_set_max_segment(swiotlb->nslabs << IO_TLB_SHIFT);
spin_lock_init(>lock);
 
return 0;
@@ -410,6 +405,27 @@ swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs)
return -ENOMEM;
 }
 
+int swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs)
+{
+   struct swiotlb *swiotlb = _swiotlb;
+   unsigned long bytes = nslabs << IO_TLB_SHIFT;
+   int ret;
+
+   ret = swiotlb_init_tlb_pool(swiotlb, virt_to_phys(tlb), bytes);
+   if (ret)
+   return ret;
+
+   no_iotlb_memory = false;
+
+   swiotlb_print_info();
+
+   late_alloc = 1;
+
+   swiotlb_set_max_segment(bytes);
+
+   return 0;
+}
+
 void __init swiotlb_exit(void)
 {
struct swiotlb *swiotlb = _swiotlb;
@@ -747,17 +763,20 @@ phys_addr_t get_swiotlb_start(void)
 }
 
 #ifdef CONFIG_DEBUG_FS
-
-static int __init swiotlb_create_debugfs(void)
+static void swiotlb_create_debugfs(struct swiotlb *swiotlb, const char *name,
+  struct dentry *node)
 {
-   struct swiotlb *swiotlb = _swiotlb;
-
-   swiotlb->debugfs = debugfs_create_dir("swiotlb", NULL);
+   swiotlb->debugfs = debugfs_create_dir(name, node);
debugfs_create_ulong("io_tlb_nslabs", 0400, swiotlb->debugfs, 
>nslabs);
debugfs_create_ulong("io_tlb_used", 0400, swiotlb->debugfs, 
>used);
-   return 0;
 }
 
-late_initcall(swiotlb_create_debugfs);
+static int __init swiotlb_create_default_debugfs(void)
+{
+   swiotlb_create_debugfs(_swiotlb, "swiotlb", NULL);
+
+   return 0;
+}
 
+late_initcall(swiotlb_create_default_debugfs);
 #endif
-- 
2.30.0.478.g8a0d178c01-goog



[PATCH v4 02/14] swiotlb: Move is_swiotlb_buffer() to swiotlb.c

2021-02-08 Thread Claire Chang
Move is_swiotlb_buffer() to swiotlb.c and make io_tlb_{start,end}
static, so we can entirely hide struct swiotlb inside of swiotlb.c in
the following patches.

Signed-off-by: Claire Chang 
---
 include/linux/swiotlb.h | 7 +--
 kernel/dma/swiotlb.c| 7 ++-
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index 83200f3b042a..041611bf3c2a 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -70,13 +70,8 @@ dma_addr_t swiotlb_map(struct device *dev, phys_addr_t phys,
 
 #ifdef CONFIG_SWIOTLB
 extern enum swiotlb_force swiotlb_force;
-extern phys_addr_t io_tlb_start, io_tlb_end;
-
-static inline bool is_swiotlb_buffer(phys_addr_t paddr)
-{
-   return paddr >= io_tlb_start && paddr < io_tlb_end;
-}
 
+bool is_swiotlb_buffer(phys_addr_t paddr);
 void __init swiotlb_exit(void);
 unsigned int swiotlb_max_segment(void);
 size_t swiotlb_max_mapping_size(struct device *dev);
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index e180211f6ad9..678490d39e55 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -69,7 +69,7 @@ enum swiotlb_force swiotlb_force;
  * swiotlb_tbl_sync_single_*, to see if the memory was in fact allocated by 
this
  * API.
  */
-phys_addr_t io_tlb_start, io_tlb_end;
+static phys_addr_t io_tlb_start, io_tlb_end;
 
 /*
  * The number of IO TLB blocks (in groups of 64) between io_tlb_start and
@@ -719,6 +719,11 @@ bool is_swiotlb_active(void)
return io_tlb_end != 0;
 }
 
+bool is_swiotlb_buffer(phys_addr_t paddr)
+{
+   return paddr >= io_tlb_start && paddr < io_tlb_end;
+}
+
 phys_addr_t get_swiotlb_start(void)
 {
return io_tlb_start;
-- 
2.30.0.478.g8a0d178c01-goog



[PATCH v4 00/14] Restricted DMA

2021-02-08 Thread Claire Chang
This series implements mitigations for lack of DMA access control on
systems without an IOMMU, which could result in the DMA accessing the
system memory at unexpected times and/or unexpected addresses, possibly
leading to data leakage or corruption.

For example, we plan to use the PCI-e bus for Wi-Fi and that PCI-e bus is
not behind an IOMMU. As PCI-e, by design, gives the device full access to
system memory, a vulnerability in the Wi-Fi firmware could easily escalate
to a full system exploit (remote wifi exploits: [1a], [1b] that shows a
full chain of exploits; [2], [3]).

To mitigate the security concerns, we introduce restricted DMA. Restricted
DMA utilizes the existing swiotlb to bounce streaming DMA in and out of a
specially allocated region and does memory allocation from the same region.
The feature on its own provides a basic level of protection against the DMA
overwriting buffer contents at unexpected times. However, to protect
against general data leakage and system memory corruption, the system needs
to provide a way to restrict the DMA to a predefined memory region (this is
usually done at firmware level, e.g. MPU in ATF on some ARM platforms [4]).

[1a] 
https://googleprojectzero.blogspot.com/2017/04/over-air-exploiting-broadcoms-wi-fi_4.html
[1b] 
https://googleprojectzero.blogspot.com/2017/04/over-air-exploiting-broadcoms-wi-fi_11.html
[2] https://blade.tencent.com/en/advisories/qualpwn/
[3] 
https://www.bleepingcomputer.com/news/security/vulnerabilities-found-in-highly-popular-firmware-for-wifi-chips/
[4] 
https://github.com/ARM-software/arm-trusted-firmware/blob/master/plat/mediatek/mt8183/drivers/emi_mpu/emi_mpu.c#L132

Claire Chang (14):
  swiotlb: Remove external access to io_tlb_start
  swiotlb: Move is_swiotlb_buffer() to swiotlb.c
  swiotlb: Add struct swiotlb
  swiotlb: Refactor swiotlb_late_init_with_tbl
  swiotlb: Add DMA_RESTRICTED_POOL
  swiotlb: Add restricted DMA pool
  swiotlb: Update swiotlb API to gain a struct device argument
  swiotlb: Use restricted DMA pool if available
  swiotlb: Refactor swiotlb_tbl_{map,unmap}_single
  dma-direct: Add a new wrapper __dma_direct_free_pages()
  swiotlb: Add is_dev_swiotlb_force()
  swiotlb: Add restricted DMA alloc/free support.
  dt-bindings: of: Add restricted DMA pool
  of: Add plumbing for restricted DMA pool

 .../reserved-memory/reserved-memory.txt   |  24 +
 arch/powerpc/platforms/pseries/svm.c  |   4 +-
 drivers/iommu/dma-iommu.c |  12 +-
 drivers/of/address.c  |  25 +
 drivers/of/device.c   |   3 +
 drivers/of/of_private.h   |   5 +
 drivers/xen/swiotlb-xen.c |   4 +-
 include/linux/device.h|   4 +
 include/linux/swiotlb.h   |  32 +-
 kernel/dma/Kconfig|  14 +
 kernel/dma/direct.c   |  51 +-
 kernel/dma/direct.h   |   8 +-
 kernel/dma/swiotlb.c  | 636 --
 13 files changed, 582 insertions(+), 240 deletions(-)

-- 

v4:
  - Fix spinlock bad magic
  - Use rmem->name for debugfs entry
  - Address the comments in v3

v3:
  Using only one reserved memory region for both streaming DMA and memory
  allocation.
  https://lore.kernel.org/patchwork/cover/1360992/

v2:
  Building on top of swiotlb.
  https://lore.kernel.org/patchwork/cover/1280705/

v1:
  Using dma_map_ops.
  https://lore.kernel.org/patchwork/cover/1271660/

2.30.0.478.g8a0d178c01-goog



[PATCH v4 01/14] swiotlb: Remove external access to io_tlb_start

2021-02-08 Thread Claire Chang
Add a new function, get_swiotlb_start(), and remove external access to
io_tlb_start, so we can entirely hide struct swiotlb inside of swiotlb.c
in the following patches.

Signed-off-by: Claire Chang 
---
 arch/powerpc/platforms/pseries/svm.c | 4 ++--
 drivers/xen/swiotlb-xen.c| 4 ++--
 include/linux/swiotlb.h  | 1 +
 kernel/dma/swiotlb.c | 5 +
 4 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/svm.c 
b/arch/powerpc/platforms/pseries/svm.c
index 7b739cc7a8a9..c10c51d49f3d 100644
--- a/arch/powerpc/platforms/pseries/svm.c
+++ b/arch/powerpc/platforms/pseries/svm.c
@@ -55,8 +55,8 @@ void __init svm_swiotlb_init(void)
if (vstart && !swiotlb_init_with_tbl(vstart, io_tlb_nslabs, false))
return;
 
-   if (io_tlb_start)
-   memblock_free_early(io_tlb_start,
+   if (vstart)
+   memblock_free_early(vstart,
PAGE_ALIGN(io_tlb_nslabs << IO_TLB_SHIFT));
panic("SVM: Cannot allocate SWIOTLB buffer");
 }
diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index 2b385c1b4a99..91f8c68d1a9b 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -192,8 +192,8 @@ int __ref xen_swiotlb_init(int verbose, bool early)
/*
 * IO TLB memory already allocated. Just use it.
 */
-   if (io_tlb_start != 0) {
-   xen_io_tlb_start = phys_to_virt(io_tlb_start);
+   if (is_swiotlb_active()) {
+   xen_io_tlb_start = phys_to_virt(get_swiotlb_start());
goto end;
}
 
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index d9c9fc9ca5d2..83200f3b042a 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -81,6 +81,7 @@ void __init swiotlb_exit(void);
 unsigned int swiotlb_max_segment(void);
 size_t swiotlb_max_mapping_size(struct device *dev);
 bool is_swiotlb_active(void);
+phys_addr_t get_swiotlb_start(void);
 void __init swiotlb_adjust_size(unsigned long new_size);
 #else
 #define swiotlb_force SWIOTLB_NO_FORCE
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 7c42df6e6100..e180211f6ad9 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -719,6 +719,11 @@ bool is_swiotlb_active(void)
return io_tlb_end != 0;
 }
 
+phys_addr_t get_swiotlb_start(void)
+{
+   return io_tlb_start;
+}
+
 #ifdef CONFIG_DEBUG_FS
 
 static int __init swiotlb_create_debugfs(void)
--

This can be dropped if Christoph's swiotlb cleanups are landed.
https://lore.kernel.org/linux-iommu/20210207160934.2955931-1-...@lst.de/T/#m7124f29b6076d462101fcff6433295157621da09
 

2.30.0.478.g8a0d178c01-goog



[PATCH] Bluetooth: hci_h5: Set HCI_QUIRK_SIMULTANEOUS_DISCOVERY for btrtl

2021-01-19 Thread Claire Chang
Realtek Bluetooth controllers can do both LE scan and BR/EDR inquiry
at once, need to set HCI_QUIRK_SIMULTANEOUS_DISCOVERY quirk.

Signed-off-by: Claire Chang 
---
 drivers/bluetooth/hci_h5.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/bluetooth/hci_h5.c b/drivers/bluetooth/hci_h5.c
index fb9817f97d45..27e96681d583 100644
--- a/drivers/bluetooth/hci_h5.c
+++ b/drivers/bluetooth/hci_h5.c
@@ -906,6 +906,11 @@ static int h5_btrtl_setup(struct h5 *h5)
/* Give the device some time before the hci-core sends it a reset */
usleep_range(1, 2);
 
+   /* Enable controller to do both LE scan and BR/EDR inquiry
+* simultaneously.
+*/
+   set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, >hu->hdev->quirks);
+
 out_free:
btrtl_free(btrtl_dev);
 
-- 
2.30.0.284.gd98b1dd5eaa7-goog



Re: [RFC PATCH v3 6/6] of: Add plumbing for restricted DMA pool

2021-01-14 Thread Claire Chang
On Fri, Jan 15, 2021 at 2:52 AM Florian Fainelli  wrote:
>
> On 1/14/21 1:08 AM, Claire Chang wrote:
> > On Wed, Jan 13, 2021 at 7:48 AM Florian Fainelli  
> > wrote:
> >>
> >> On 1/5/21 7:41 PM, Claire Chang wrote:
> >>> If a device is not behind an IOMMU, we look up the device node and set
> >>> up the restricted DMA when the restricted-dma-pool is presented.
> >>>
> >>> Signed-off-by: Claire Chang 
> >>> ---
> >>
> >> [snip]
> >>
> >>> +int of_dma_set_restricted_buffer(struct device *dev)
> >>> +{
> >>> + struct device_node *node;
> >>> + int count, i;
> >>> +
> >>> + if (!dev->of_node)
> >>> + return 0;
> >>> +
> >>> + count = of_property_count_elems_of_size(dev->of_node, 
> >>> "memory-region",
> >>> + sizeof(phandle));
> >>
> >> You could have an early check for count < 0, along with an error
> >> message, if that is deemed useful.
> >>
> >>> + for (i = 0; i < count; i++) {
> >>> + node = of_parse_phandle(dev->of_node, "memory-region", i);
> >>> + if (of_device_is_compatible(node, "restricted-dma-pool"))
> >>
> >> And you may want to add here an of_device_is_available(node). A platform
> >> that provides the Device Tree firmware and try to support multiple
> >> different SoCs may try to determine if an IOMMU is present, and if it
> >> is, it could be marking the restriced-dma-pool region with a 'status =
> >> "disabled"' property, or any variant of that scheme.
> >
> > This function is called only when there is no IOMMU present (check in
> > drivers/of/device.c). I can still add of_device_is_available(node)
> > here if you think it's helpful.
>
> I believe it is, since boot loader can have a shared Device Tree blob
> skeleton and do various adaptations based on the chip (that's what we
> do) and adding a status property is much simpler than insertion new
> nodes are run time.
>
> >
> >>
> >>> + return of_reserved_mem_device_init_by_idx(
> >>> + dev, dev->of_node, i);
> >>
> >> This does not seem to be supporting more than one memory region, did not
> >> you want something like instead:
> >>
> >> ret = of_reserved_mem_device_init_by_idx(...);
> >> if (ret)
> >> return ret;
> >>
> >
> > Yes. This implement only supports one restriced-dma-pool memory region
> > with the assumption that there is only one memory region with the
> > compatible string, restricted-dma-pool, in the dts. IIUC, it's similar
> > to shared-dma-pool.
>
> Then if here is such a known limitation it should be both documented and
> enforced here, you shouldn ot be iterating over all of the phandles that
> you find, stop at the first one and issue a warning if count > 1?

What I have in mind is there might be multiple memory regions, but
only one is for restriced-dma-pool.
Say, if you want a separated region for coherent DMA and only do
streaming DMA in this restriced-dma-pool region, you can add another
reserved-memory node with shared-dma-pool in dts and the current
implementation will try to allocate the memory via
dma_alloc_from_dev_coherent() first (see dma_alloc_attrs() in
/kernel/dma/mapping.c).
Or if you have vendor specific memory region, you can still set up
restriced-dma-pool by adding another reserved-memory node in dts.
Dose this make sense to you? I'll document this for sure.

> --
> Florian


Re: [RFC PATCH v3 2/6] swiotlb: Add restricted DMA pool

2021-01-14 Thread Claire Chang
On Wed, Jan 13, 2021 at 8:42 PM Christoph Hellwig  wrote:
>
> > +#ifdef CONFIG_SWIOTLB
> > + struct io_tlb_mem   *dma_io_tlb_mem;
> >  #endif
>
> Please add a new config option for this code instead of always building
> it when swiotlb is enabled.
>
> > +static int swiotlb_init_io_tlb_mem(struct io_tlb_mem *mem, phys_addr_t 
> > start,
> > +size_t size)
>
> Can you split the refactoring in swiotlb.c into one or more prep
> patches?
>
> > +static int rmem_swiotlb_device_init(struct reserved_mem *rmem,
> > + struct device *dev)
> > +{
> > + struct io_tlb_mem *mem = rmem->priv;
> > + int ret;
> > +
> > + if (dev->dma_io_tlb_mem)
> > + return -EBUSY;
> > +
> > + if (!mem) {
> > + mem = kzalloc(sizeof(*mem), GFP_KERNEL);
> > + if (!mem)
> > + return -ENOMEM;
>
> What is the calling convention here that allows for a NULL and non-NULL
> private data?

Since multiple devices can share the same pool, the private data,
io_tlb_mem struct, will be initialized by the first device attached to
it.
This is similar to rmem_dma_device_init() in kernel/dma/coherent.c.
I'll add a comment for it in next version.


Re: [RFC PATCH v3 6/6] of: Add plumbing for restricted DMA pool

2021-01-14 Thread Claire Chang
On Wed, Jan 13, 2021 at 7:48 AM Florian Fainelli  wrote:
>
> On 1/5/21 7:41 PM, Claire Chang wrote:
> > If a device is not behind an IOMMU, we look up the device node and set
> > up the restricted DMA when the restricted-dma-pool is presented.
> >
> > Signed-off-by: Claire Chang 
> > ---
>
> [snip]
>
> > +int of_dma_set_restricted_buffer(struct device *dev)
> > +{
> > + struct device_node *node;
> > + int count, i;
> > +
> > + if (!dev->of_node)
> > + return 0;
> > +
> > + count = of_property_count_elems_of_size(dev->of_node, "memory-region",
> > + sizeof(phandle));
>
> You could have an early check for count < 0, along with an error
> message, if that is deemed useful.
>
> > + for (i = 0; i < count; i++) {
> > + node = of_parse_phandle(dev->of_node, "memory-region", i);
> > + if (of_device_is_compatible(node, "restricted-dma-pool"))
>
> And you may want to add here an of_device_is_available(node). A platform
> that provides the Device Tree firmware and try to support multiple
> different SoCs may try to determine if an IOMMU is present, and if it
> is, it could be marking the restriced-dma-pool region with a 'status =
> "disabled"' property, or any variant of that scheme.

This function is called only when there is no IOMMU present (check in
drivers/of/device.c). I can still add of_device_is_available(node)
here if you think it's helpful.

>
> > + return of_reserved_mem_device_init_by_idx(
> > + dev, dev->of_node, i);
>
> This does not seem to be supporting more than one memory region, did not
> you want something like instead:
>
> ret = of_reserved_mem_device_init_by_idx(...);
> if (ret)
> return ret;
>

Yes. This implement only supports one restriced-dma-pool memory region
with the assumption that there is only one memory region with the
compatible string, restricted-dma-pool, in the dts. IIUC, it's similar
to shared-dma-pool.


> > + }
> > +
> > + return 0;
> > +}
> > diff --git a/drivers/of/device.c b/drivers/of/device.c
> > index aedfaaafd3e7..e2c7409956ab 100644
> > --- a/drivers/of/device.c
> > +++ b/drivers/of/device.c
> > @@ -182,6 +182,10 @@ int of_dma_configure_id(struct device *dev, struct 
> > device_node *np,
> >   arch_setup_dma_ops(dev, dma_start, size, iommu, coherent);
> >
> >   dev->dma_range_map = map;
> > +
> > + if (!iommu)
> > + return of_dma_set_restricted_buffer(dev);
> > +
> >   return 0;
> >  }
> >  EXPORT_SYMBOL_GPL(of_dma_configure_id);
> > diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
> > index d9e6a324de0a..28a2dfa197ba 100644
> > --- a/drivers/of/of_private.h
> > +++ b/drivers/of/of_private.h
> > @@ -161,12 +161,17 @@ 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);
> >  #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_get_restricted_buffer(struct device *dev)
> > +{
> > + return -ENODEV;
> > +}
> >  #endif
> >
> >  #endif /* _LINUX_OF_PRIVATE_H */
> >
>
>
> --
> Florian


Re: [RFC PATCH v3 5/6] dt-bindings: of: Add restricted DMA pool

2021-01-11 Thread Claire Chang
On Fri, Jan 8, 2021 at 2:15 AM Florian Fainelli  wrote:
>
> On 1/7/21 10:00 AM, Konrad Rzeszutek Wilk wrote:
> >>>
> >>>
> >>>  - Nothing stops the physical device from bypassing the SWIOTLB buffer.
> >>>That is if an errant device screwed up the length or DMA address, the
> >>>SWIOTLB would gladly do what the device told it do?
> >>
> >> So the system needs to provide a way to lock down the memory access, e.g. 
> >> MPU.
> >
> > OK! Would it be prudent to have this in the description above perhaps?
>
> Yes this is something that must be documented as a requirement for the
> restricted DMA pool users, otherwise attempting to do restricted DMA
> pool is no different than say, using a device private CMA region.
> Without the enforcement, this is just a best effort.

Will add in the next version.

> --
> Florian


Re: [RFC PATCH v3 0/6] Restricted DMA

2021-01-11 Thread Claire Chang
On Fri, Jan 8, 2021 at 1:59 AM Florian Fainelli  wrote:
>
> On 1/7/21 9:42 AM, Claire Chang wrote:
>
> >> Can you explain how ATF gets involved and to what extent it does help,
> >> besides enforcing a secure region from the ARM CPU's perpsective? Does
> >> the PCIe root complex not have an IOMMU but can somehow be denied access
> >> to a region that is marked NS=0 in the ARM CPU's MMU? If so, that is
> >> still some sort of basic protection that the HW enforces, right?
> >
> > We need the ATF support for memory MPU (memory protection unit).
> > Restricted DMA (with reserved-memory in dts) makes sure the predefined 
> > memory
> > region is for PCIe DMA only, but we still need MPU to locks down PCIe 
> > access to
> > that specific regions.
>
> OK so you do have a protection unit of some sort to enforce which region
> in DRAM the PCIE bridge is allowed to access, that makes sense,
> otherwise the restricted DMA region would only be a hint but nothing you
> can really enforce. This is almost entirely analogous to our systems then.

Here is the example of setting the MPU:
https://github.com/ARM-software/arm-trusted-firmware/blob/master/plat/mediatek/mt8183/drivers/emi_mpu/emi_mpu.c#L132

>
> There may be some value in standardizing on an ARM SMCCC call then since
> you already support two different SoC vendors.
>
> >
> >>
> >> On Broadcom STB SoCs we have had something similar for a while however
> >> and while we don't have an IOMMU for the PCIe bridge, we do have a a
> >> basic protection mechanism whereby we can configure a region in DRAM to
> >> be PCIe read/write and CPU read/write which then gets used as the PCIe
> >> inbound region for the PCIe EP. By default the PCIe bridge is not
> >> allowed access to DRAM so we must call into a security agent to allow
> >> the PCIe bridge to access the designated DRAM region.
> >>
> >> We have done this using a private CMA area region assigned via Device
> >> Tree, assigned with a and requiring the PCIe EP driver to use
> >> dma_alloc_from_contiguous() in order to allocate from this device
> >> private CMA area. The only drawback with that approach is that it
> >> requires knowing how much memory you need up front for buffers and DMA
> >> descriptors that the PCIe EP will need to process. The problem is that
> >> it requires driver modifications and that does not scale over the number
> >> of PCIe EP drivers, some we absolutely do not control, but there is no
> >> need to bounce buffer. Your approach scales better across PCIe EP
> >> drivers however it does require bounce buffering which could be a
> >> performance hit.
> >
> > Only the streaming DMA (map/unmap) needs bounce buffering.
>
> True, and typically only on transmit since you don't really control
> where the sk_buff are allocated from, right? On RX since you need to
> hand buffer addresses to the WLAN chip prior to DMA, you can allocate
> them from a pool that already falls within the restricted DMA region, right?
>

Right, but applying bounce buffering to RX will make it more secure.
The device won't be able to modify the content after unmap. Just like what
iommu_unmap does.

> > I also added alloc/free support in this series
> > (https://lore.kernel.org/patchwork/patch/1360995/), so dma_direct_alloc() 
> > will
> > try to allocate memory from the predefined memory region.
> >
> > As for the performance hit, it should be similar to the default swiotlb.
> > Here are my experiment results. Both SoCs lack IOMMU for PCIe.
> >
> > PCIe wifi vht80 throughput -
> >
> >   MTK SoC  tcp_tx tcp_rxudp_tx   udp_rx
> >   w/o Restricted DMA  244.1 134.66   312.56   350.79
> >   w/ Restricted DMA246.95   136.59   363.21   351.99
> >
> >   Rockchip SoC   tcp_tx tcp_rxudp_tx   udp_rx
> >   w/o Restricted DMA  237.87   133.86   288.28   361.88
> >   w/ Restricted DMA256.01   130.95   292.28   353.19
>
> How come you get better throughput with restricted DMA? Is it because
> doing DMA to/from a contiguous region allows for better grouping of
> transactions from the DRAM controller's perspective somehow?

I'm not sure, but actually, enabling the default swiotlb for wifi also helps the
throughput a little bit for me.

>
> >
> > The CPU usage doesn't increase too much either.
> > Although I didn't measure the CPU usage very precisely, it's ~3% with a 
> > single
> > big core (Cortex-A72) and ~5% with a single small core (Cortex-A53).
> >
> > Thanks!
> >
> >>
> >> Thanks!
> >> --
> >> Florian
>
>
> --
> Florian


Re: [RFC PATCH v3 0/6] Restricted DMA

2021-01-07 Thread Claire Chang
On Thu, Jan 7, 2021 at 2:48 AM Florian Fainelli  wrote:
>
> Hi,
>
> First of all let me say that I am glad that someone is working on a
> upstream solution for this issue, would appreciate if you could CC and
> Jim Quinlan on subsequent submissions.

Sure!

>
> On 1/5/21 7:41 PM, Claire Chang wrote:
> > This series implements mitigations for lack of DMA access control on
> > systems without an IOMMU, which could result in the DMA accessing the
> > system memory at unexpected times and/or unexpected addresses, possibly
> > leading to data leakage or corruption.
> >
> > For example, we plan to use the PCI-e bus for Wi-Fi and that PCI-e bus is
> > not behind an IOMMU. As PCI-e, by design, gives the device full access to
> > system memory, a vulnerability in the Wi-Fi firmware could easily escalate
> > to a full system exploit (remote wifi exploits: [1a], [1b] that shows a
> > full chain of exploits; [2], [3]).
> >
> > To mitigate the security concerns, we introduce restricted DMA. Restricted
> > DMA utilizes the existing swiotlb to bounce streaming DMA in and out of a
> > specially allocated region and does memory allocation from the same region.
> > The feature on its own provides a basic level of protection against the DMA
> > overwriting buffer contents at unexpected times. However, to protect
> > against general data leakage and system memory corruption, the system needs
> > to provide a way to restrict the DMA to a predefined memory region (this is
> > usually done at firmware level, e.g. in ATF on some ARM platforms).
>
> Can you explain how ATF gets involved and to what extent it does help,
> besides enforcing a secure region from the ARM CPU's perpsective? Does
> the PCIe root complex not have an IOMMU but can somehow be denied access
> to a region that is marked NS=0 in the ARM CPU's MMU? If so, that is
> still some sort of basic protection that the HW enforces, right?

We need the ATF support for memory MPU (memory protection unit).
Restricted DMA (with reserved-memory in dts) makes sure the predefined memory
region is for PCIe DMA only, but we still need MPU to locks down PCIe access to
that specific regions.

>
> On Broadcom STB SoCs we have had something similar for a while however
> and while we don't have an IOMMU for the PCIe bridge, we do have a a
> basic protection mechanism whereby we can configure a region in DRAM to
> be PCIe read/write and CPU read/write which then gets used as the PCIe
> inbound region for the PCIe EP. By default the PCIe bridge is not
> allowed access to DRAM so we must call into a security agent to allow
> the PCIe bridge to access the designated DRAM region.
>
> We have done this using a private CMA area region assigned via Device
> Tree, assigned with a and requiring the PCIe EP driver to use
> dma_alloc_from_contiguous() in order to allocate from this device
> private CMA area. The only drawback with that approach is that it
> requires knowing how much memory you need up front for buffers and DMA
> descriptors that the PCIe EP will need to process. The problem is that
> it requires driver modifications and that does not scale over the number
> of PCIe EP drivers, some we absolutely do not control, but there is no
> need to bounce buffer. Your approach scales better across PCIe EP
> drivers however it does require bounce buffering which could be a
> performance hit.

Only the streaming DMA (map/unmap) needs bounce buffering.
I also added alloc/free support in this series
(https://lore.kernel.org/patchwork/patch/1360995/), so dma_direct_alloc() will
try to allocate memory from the predefined memory region.

As for the performance hit, it should be similar to the default swiotlb.
Here are my experiment results. Both SoCs lack IOMMU for PCIe.

PCIe wifi vht80 throughput -

  MTK SoC  tcp_tx tcp_rxudp_tx   udp_rx
  w/o Restricted DMA  244.1 134.66   312.56   350.79
  w/ Restricted DMA246.95   136.59   363.21   351.99

  Rockchip SoC   tcp_tx tcp_rxudp_tx   udp_rx
  w/o Restricted DMA  237.87   133.86   288.28   361.88
  w/ Restricted DMA256.01   130.95   292.28   353.19

The CPU usage doesn't increase too much either.
Although I didn't measure the CPU usage very precisely, it's ~3% with a single
big core (Cortex-A72) and ~5% with a single small core (Cortex-A53).

Thanks!

>
> Thanks!
> --
> Florian


Re: [RFC PATCH v3 5/6] dt-bindings: of: Add restricted DMA pool

2021-01-07 Thread Claire Chang
On Thu, Jan 7, 2021 at 2:58 AM Konrad Rzeszutek Wilk
 wrote:
>
> On Wed, Jan 06, 2021 at 11:41:23AM +0800, Claire Chang wrote:
> > Introduce the new compatible string, restricted-dma-pool, for restricted
> > DMA. One can specify the address and length of the restricted DMA memory
> > region by restricted-dma-pool in the device tree.
> >
> > Signed-off-by: Claire Chang 
> > ---
> >  .../reserved-memory/reserved-memory.txt   | 24 +++
> >  1 file changed, 24 insertions(+)
> >
> > diff --git 
> > a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt 
> > b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> > index e8d3096d922c..44975e2a1fd2 100644
> > --- a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> > +++ b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> > @@ -51,6 +51,20 @@ compatible (optional) - standard definition
> >used as a shared pool of DMA buffers for a set of devices. It can
> >be used by an operating system to instantiate the necessary pool
> >management subsystem if necessary.
> > +- restricted-dma-pool: This indicates a region of memory meant to 
> > be
> > +  used as a pool of restricted DMA buffers for a set of devices. 
> > The
> > +  memory region would be the only region accessible to those 
> > devices.
> > +  When using this, the no-map and reusable properties must not be 
> > set,
> > +  so the operating system can create a virtual mapping that will 
> > be used
> > +  for synchronization. The main purpose for restricted DMA is to
> > +  mitigate the lack of DMA access control on systems without an 
> > IOMMU,
> > +  which could result in the DMA accessing the system memory at
> > +  unexpected times and/or unexpected addresses, possibly leading 
> > to data
> > +  leakage or corruption. The feature on its own provides a basic 
> > level
> > +  of protection against the DMA overwriting buffer contents at
> > +  unexpected times. However, to protect against general data 
> > leakage and
> > +  system memory corruption, the system needs to provide way to 
> > restrict
> > +  the DMA to a predefined memory region.
>
> Heya!
>
> I think I am missing something obvious here so please bear with my
> questions:
>
>  - This code adds the means of having the SWIOTLB pool tied to a specific
>memory correct?

It doesn't affect the existing SWIOTLB. It just utilizes the existing SWIOTLB
code to create another DMA pool tied to a specific memory region for a given set
of devices. It bounces the streaming DMA (map/unmap) in and out of that region
and does the memory allocation (dma_direct_alloc) from the same region.

>
>
>  - Nothing stops the physical device from bypassing the SWIOTLB buffer.
>That is if an errant device screwed up the length or DMA address, the
>SWIOTLB would gladly do what the device told it do?

So the system needs to provide a way to lock down the memory access, e.g. MPU.

>
>  - This has to be combined with SWIOTLB-force-ish to always use the
>bounce buffer, otherwise you could still do DMA without using
>SWIOTLB (by not hitting the criteria for needing to use SWIOTLB)?

Since restricted DMA is for the devices that are not behind an IOMMU, I change
the criteria
`if (unlikely(swiotlb_force == SWIOTLB_FORCE))`
to
`if (unlikely(swiotlb_force == SWIOTLB_FORCE) || dev->dma_io_tlb_mem)`
in dma_direct_map_page().

Also, even if SWIOTLB=force, the restricted DMA pool is preferred if available
(get_io_tlb_mem in https://lore.kernel.org/patchwork/patch/1360995/).

Thanks!


Re: [RFC PATCH v3 2/6] swiotlb: Add restricted DMA pool

2021-01-07 Thread Claire Chang
Hi Greg and Konrad,

This change is intended to be non-arch specific. Any arch that lacks DMA access
control and has devices not behind an IOMMU can make use of it. Could you share
why you think this should be arch specific?

Thanks!


[RFC PATCH v3 1/6] swiotlb: Add io_tlb_mem struct

2021-01-05 Thread Claire Chang
Added a new struct, io_tlb_mem, as the IO TLB memory pool descriptor and
moved relevant global variables into that struct.
This will be useful later to allow for restricted DMA pool.

Signed-off-by: Claire Chang 
---
 arch/powerpc/platforms/pseries/svm.c |   4 +-
 drivers/xen/swiotlb-xen.c|   4 +-
 include/linux/swiotlb.h  |  39 +++-
 kernel/dma/swiotlb.c | 292 +--
 4 files changed, 178 insertions(+), 161 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/svm.c 
b/arch/powerpc/platforms/pseries/svm.c
index 7b739cc7a8a9..2b767f1ca5fd 100644
--- a/arch/powerpc/platforms/pseries/svm.c
+++ b/arch/powerpc/platforms/pseries/svm.c
@@ -55,8 +55,8 @@ void __init svm_swiotlb_init(void)
if (vstart && !swiotlb_init_with_tbl(vstart, io_tlb_nslabs, false))
return;
 
-   if (io_tlb_start)
-   memblock_free_early(io_tlb_start,
+   if (io_tlb_default_mem.start)
+   memblock_free_early(io_tlb_default_mem.start,
PAGE_ALIGN(io_tlb_nslabs << IO_TLB_SHIFT));
panic("SVM: Cannot allocate SWIOTLB buffer");
 }
diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index 2b385c1b4a99..4d17dff7ffd2 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -192,8 +192,8 @@ int __ref xen_swiotlb_init(int verbose, bool early)
/*
 * IO TLB memory already allocated. Just use it.
 */
-   if (io_tlb_start != 0) {
-   xen_io_tlb_start = phys_to_virt(io_tlb_start);
+   if (io_tlb_default_mem.start != 0) {
+   xen_io_tlb_start = phys_to_virt(io_tlb_default_mem.start);
goto end;
}
 
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index d9c9fc9ca5d2..dd8eb57cbb8f 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -70,11 +70,46 @@ dma_addr_t swiotlb_map(struct device *dev, phys_addr_t phys,
 
 #ifdef CONFIG_SWIOTLB
 extern enum swiotlb_force swiotlb_force;
-extern phys_addr_t io_tlb_start, io_tlb_end;
+
+/**
+ * struct io_tlb_mem - IO TLB Memory Pool Descriptor
+ *
+ * @start: The start address of the swiotlb memory pool. Used to do a quick
+ * range check to see if the memory was in fact allocated by this
+ * API.
+ * @end:   The end address of the swiotlb memory pool. Used to do a quick
+ * range check to see if the memory was in fact allocated by this
+ * API.
+ * @nslabs:The number of IO TLB blocks (in groups of 64) between @start and
+ * @end. This is command line adjustable via setup_io_tlb_npages.
+ * @used:  The number of used IO TLB block.
+ * @list:  The free list describing the number of free entries available
+ * from each index.
+ * @index: The index to start searching in the next round.
+ * @orig_addr: The original address corresponding to a mapped entry for the
+ * sync operations.
+ * @lock:  The lock to protect the above data structures in the map and
+ * unmap calls.
+ * @debugfs:   The dentry to debugfs.
+ */
+struct io_tlb_mem {
+   phys_addr_t start;
+   phys_addr_t end;
+   unsigned long nslabs;
+   unsigned long used;
+   unsigned int *list;
+   unsigned int index;
+   phys_addr_t *orig_addr;
+   spinlock_t lock;
+   struct dentry *debugfs;
+};
+extern struct io_tlb_mem io_tlb_default_mem;
 
 static inline bool is_swiotlb_buffer(phys_addr_t paddr)
 {
-   return paddr >= io_tlb_start && paddr < io_tlb_end;
+   struct io_tlb_mem *mem = _tlb_default_mem;
+
+   return paddr >= mem->start && paddr < mem->end;
 }
 
 void __init swiotlb_exit(void);
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 7c42df6e6100..e4368159f88a 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -61,33 +61,11 @@
  * allocate a contiguous 1MB, we're probably in trouble anyway.
  */
 #define IO_TLB_MIN_SLABS ((1<<20) >> IO_TLB_SHIFT)
+#define INVALID_PHYS_ADDR (~(phys_addr_t)0)
 
 enum swiotlb_force swiotlb_force;
 
-/*
- * Used to do a quick range check in swiotlb_tbl_unmap_single and
- * swiotlb_tbl_sync_single_*, to see if the memory was in fact allocated by 
this
- * API.
- */
-phys_addr_t io_tlb_start, io_tlb_end;
-
-/*
- * The number of IO TLB blocks (in groups of 64) between io_tlb_start and
- * io_tlb_end.  This is command line adjustable via setup_io_tlb_npages.
- */
-static unsigned long io_tlb_nslabs;
-
-/*
- * The number of used IO TLB block
- */
-static unsigned long io_tlb_used;
-
-/*
- * This is a free list describing the number of free entries available from
- * each index
- */
-static unsigned int *io_tlb_list;
-static unsigned int io_tlb_index;
+struct io_tlb_mem io_tlb_default_mem;
 
 /*
  * Max segment that we can provide which (if pages are contingous) will
@@ -9

[RFC PATCH v3 6/6] of: Add plumbing for restricted DMA pool

2021-01-05 Thread Claire Chang
If a device is not behind an IOMMU, we look up the device node and set
up the restricted DMA when the restricted-dma-pool is presented.

Signed-off-by: Claire Chang 
---
 drivers/of/address.c| 21 +
 drivers/of/device.c |  4 
 drivers/of/of_private.h |  5 +
 3 files changed, 30 insertions(+)

diff --git a/drivers/of/address.c b/drivers/of/address.c
index 73ddf2540f3f..94eca8249854 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -8,6 +8,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -1094,3 +1095,23 @@ bool of_dma_is_coherent(struct device_node *np)
return false;
 }
 EXPORT_SYMBOL_GPL(of_dma_is_coherent);
+
+int of_dma_set_restricted_buffer(struct device *dev)
+{
+   struct device_node *node;
+   int count, i;
+
+   if (!dev->of_node)
+   return 0;
+
+   count = of_property_count_elems_of_size(dev->of_node, "memory-region",
+   sizeof(phandle));
+   for (i = 0; i < count; i++) {
+   node = of_parse_phandle(dev->of_node, "memory-region", i);
+   if (of_device_is_compatible(node, "restricted-dma-pool"))
+   return of_reserved_mem_device_init_by_idx(
+   dev, dev->of_node, i);
+   }
+
+   return 0;
+}
diff --git a/drivers/of/device.c b/drivers/of/device.c
index aedfaaafd3e7..e2c7409956ab 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -182,6 +182,10 @@ int of_dma_configure_id(struct device *dev, struct 
device_node *np,
arch_setup_dma_ops(dev, dma_start, size, iommu, coherent);
 
dev->dma_range_map = map;
+
+   if (!iommu)
+   return of_dma_set_restricted_buffer(dev);
+
return 0;
 }
 EXPORT_SYMBOL_GPL(of_dma_configure_id);
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index d9e6a324de0a..28a2dfa197ba 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -161,12 +161,17 @@ 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);
 #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_get_restricted_buffer(struct device *dev)
+{
+   return -ENODEV;
+}
 #endif
 
 #endif /* _LINUX_OF_PRIVATE_H */
-- 
2.29.2.729.g45daf8777d-goog



[RFC PATCH v3 5/6] dt-bindings: of: Add restricted DMA pool

2021-01-05 Thread Claire Chang
Introduce the new compatible string, restricted-dma-pool, for restricted
DMA. One can specify the address and length of the restricted DMA memory
region by restricted-dma-pool in the device tree.

Signed-off-by: Claire Chang 
---
 .../reserved-memory/reserved-memory.txt   | 24 +++
 1 file changed, 24 insertions(+)

diff --git 
a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt 
b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
index e8d3096d922c..44975e2a1fd2 100644
--- a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
+++ b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
@@ -51,6 +51,20 @@ compatible (optional) - standard definition
   used as a shared pool of DMA buffers for a set of devices. It can
   be used by an operating system to instantiate the necessary pool
   management subsystem if necessary.
+- restricted-dma-pool: This indicates a region of memory meant to be
+  used as a pool of restricted DMA buffers for a set of devices. The
+  memory region would be the only region accessible to those devices.
+  When using this, the no-map and reusable properties must not be set,
+  so the operating system can create a virtual mapping that will be 
used
+  for synchronization. The main purpose for restricted DMA is to
+  mitigate the lack of DMA access control on systems without an IOMMU,
+  which could result in the DMA accessing the system memory at
+  unexpected times and/or unexpected addresses, possibly leading to 
data
+  leakage or corruption. The feature on its own provides a basic level
+  of protection against the DMA overwriting buffer contents at
+  unexpected times. However, to protect against general data leakage 
and
+  system memory corruption, the system needs to provide way to restrict
+  the DMA to a predefined memory region.
 - vendor specific string in the form ,[-]
 no-map (optional) - empty property
 - Indicates the operating system must not create a virtual mapping
@@ -120,6 +134,11 @@ one for multimedia processing (named 
multimedia-memory@7700, 64MiB).
compatible = "acme,multimedia-memory";
reg = <0x7700 0x400>;
};
+
+   restricted_dma_mem_reserved: restricted_dma_mem_reserved {
+   compatible = "restricted-dma-pool";
+   reg = <0x5000 0x40>;
+   };
};
 
/* ... */
@@ -138,4 +157,9 @@ one for multimedia processing (named 
multimedia-memory@7700, 64MiB).
memory-region = <_reserved>;
/* ... */
};
+
+   pcie_device: pcie_device@0,0 {
+   memory-region = <_dma_mem_reserved>;
+   /* ... */
+   };
 };
-- 
2.29.2.729.g45daf8777d-goog



[RFC PATCH v3 4/6] swiotlb: Add restricted DMA alloc/free support.

2021-01-05 Thread Claire Chang
Add the functions, swiotlb_alloc and swiotlb_free to support the
memory allocation from restricted DMA pool.

Signed-off-by: Claire Chang 
---
 include/linux/swiotlb.h |   6 ++
 kernel/dma/direct.c |  12 +++
 kernel/dma/swiotlb.c| 171 +---
 3 files changed, 144 insertions(+), 45 deletions(-)

diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index 5135e5636042..84fe96e40685 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -68,6 +68,12 @@ extern void swiotlb_tbl_sync_single(struct device *hwdev,
 dma_addr_t swiotlb_map(struct device *dev, phys_addr_t phys,
size_t size, enum dma_data_direction dir, unsigned long attrs);
 
+void *swiotlb_alloc(struct device *dev, size_t size, dma_addr_t *dma_handle,
+   unsigned long attrs);
+
+void swiotlb_free(struct device *dev, size_t size, void *vaddr,
+ dma_addr_t dma_addr, unsigned long attrs);
+
 #ifdef CONFIG_SWIOTLB
 extern enum swiotlb_force swiotlb_force;
 
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 30ccbc08e229..126e9b3354d6 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -137,6 +137,11 @@ void *dma_direct_alloc(struct device *dev, size_t size,
void *ret;
int err;
 
+#ifdef CONFIG_SWIOTLB
+   if (unlikely(dev->dma_io_tlb_mem))
+   return swiotlb_alloc(dev, size, dma_handle, attrs);
+#endif
+
size = PAGE_ALIGN(size);
if (attrs & DMA_ATTR_NO_WARN)
gfp |= __GFP_NOWARN;
@@ -246,6 +251,13 @@ void dma_direct_free(struct device *dev, size_t size,
 {
unsigned int page_order = get_order(size);
 
+#ifdef CONFIG_SWIOTLB
+   if (unlikely(dev->dma_io_tlb_mem)) {
+   swiotlb_free(dev, size, cpu_addr, dma_addr, attrs);
+   return;
+   }
+#endif
+
if ((attrs & DMA_ATTR_NO_KERNEL_MAPPING) &&
!force_dma_unencrypted(dev)) {
/* cpu_addr is a struct page cookie, not a kernel address */
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 1f05af09e61a..ca88ef59435d 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -459,14 +459,13 @@ static void swiotlb_bounce(phys_addr_t orig_addr, 
phys_addr_t tlb_addr,
}
 }
 
-phys_addr_t swiotlb_tbl_map_single(struct device *hwdev, phys_addr_t orig_addr,
-   size_t mapping_size, size_t alloc_size,
-   enum dma_data_direction dir, unsigned long attrs)
+static int swiotlb_tbl_find_free_region(struct device *hwdev,
+   dma_addr_t tbl_dma_addr,
+   size_t alloc_size,
+   unsigned long attrs)
 {
struct io_tlb_mem *mem = get_io_tlb_mem(hwdev);
-   dma_addr_t tbl_dma_addr = phys_to_dma_unencrypted(hwdev, mem->start);
unsigned long flags;
-   phys_addr_t tlb_addr;
unsigned int nslots, stride, index, wrap;
int i;
unsigned long mask;
@@ -477,15 +476,6 @@ phys_addr_t swiotlb_tbl_map_single(struct device *hwdev, 
phys_addr_t orig_addr,
if (no_iotlb_memory && !hwdev->dma_io_tlb_mem)
panic("Can not allocate SWIOTLB buffer earlier and can't now 
provide you with the DMA bounce buffer");
 
-   if (mem_encrypt_active())
-   pr_warn_once("Memory encryption is active and system is using 
DMA bounce buffers\n");
-
-   if (mapping_size > alloc_size) {
-   dev_warn_once(hwdev, "Invalid sizes (mapping: %zd bytes, alloc: 
%zd bytes)",
- mapping_size, alloc_size);
-   return (phys_addr_t)DMA_MAPPING_ERROR;
-   }
-
mask = dma_get_seg_boundary(hwdev);
 
tbl_dma_addr &= mask;
@@ -547,7 +537,6 @@ phys_addr_t swiotlb_tbl_map_single(struct device *hwdev, 
phys_addr_t orig_addr,
mem->list[i] = 0;
for (i = index - 1; (OFFSET(i, IO_TLB_SEGSIZE) != 
IO_TLB_SEGSIZE - 1) && mem->list[i]; i--)
mem->list[i] = ++count;
-   tlb_addr = mem->start + (index << IO_TLB_SHIFT);
 
/*
 * Update the indices to avoid searching in the next
@@ -570,45 +559,21 @@ phys_addr_t swiotlb_tbl_map_single(struct device *hwdev, 
phys_addr_t orig_addr,
if (!(attrs & DMA_ATTR_NO_WARN) && printk_ratelimit())
dev_warn(hwdev, "swiotlb buffer is full (sz: %zd bytes), total 
%lu (slots), used %lu (slots)\n",
 alloc_size, mem->nslabs, tmp_io_tlb_used);
-   return (phys_addr_t)DMA_MAPPING_ERROR;
+   return -ENOMEM;
+
 found:
mem->used += nslots;
spin_unlock_irqrestore(>lock, flags);
 
-   /*
-* Save away the mapping from the ori

[RFC PATCH v3 2/6] swiotlb: Add restricted DMA pool

2021-01-05 Thread Claire Chang
Add the initialization function to create restricted DMA pools from
matching reserved-memory nodes in the device tree.

Signed-off-by: Claire Chang 
---
 include/linux/device.h  |   4 ++
 include/linux/swiotlb.h |   7 +-
 kernel/dma/Kconfig  |   1 +
 kernel/dma/swiotlb.c| 144 ++--
 4 files changed, 131 insertions(+), 25 deletions(-)

diff --git a/include/linux/device.h b/include/linux/device.h
index 89bb8b84173e..ca6f71ec8871 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -413,6 +413,7 @@ struct dev_links_info {
  * @dma_pools: Dma pools (if dma'ble device).
  * @dma_mem:   Internal for coherent mem override.
  * @cma_area:  Contiguous memory area for dma allocations
+ * @dma_io_tlb_mem: Internal for swiotlb io_tlb_mem override.
  * @archdata:  For arch-specific additions.
  * @of_node:   Associated device tree node.
  * @fwnode:Associated device node supplied by platform firmware.
@@ -515,6 +516,9 @@ struct device {
 #ifdef CONFIG_DMA_CMA
struct cma *cma_area;   /* contiguous memory area for dma
   allocations */
+#endif
+#ifdef CONFIG_SWIOTLB
+   struct io_tlb_mem   *dma_io_tlb_mem;
 #endif
/* arch specific additions */
struct dev_archdata archdata;
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index dd8eb57cbb8f..a1bbd775 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -76,12 +76,13 @@ extern enum swiotlb_force swiotlb_force;
  *
  * @start: The start address of the swiotlb memory pool. Used to do a quick
  * range check to see if the memory was in fact allocated by this
- * API.
+ * API. For restricted DMA pool, this is device tree adjustable.
  * @end:   The end address of the swiotlb memory pool. Used to do a quick
  * range check to see if the memory was in fact allocated by this
- * API.
+ * API. For restricted DMA pool, this is device tree adjustable.
  * @nslabs:The number of IO TLB blocks (in groups of 64) between @start and
- * @end. This is command line adjustable via setup_io_tlb_npages.
+ * @end. For default swiotlb, this is command line adjustable via
+ * setup_io_tlb_npages.
  * @used:  The number of used IO TLB block.
  * @list:  The free list describing the number of free entries available
  * from each index.
diff --git a/kernel/dma/Kconfig b/kernel/dma/Kconfig
index 479fc145acfc..131a0a66781b 100644
--- a/kernel/dma/Kconfig
+++ b/kernel/dma/Kconfig
@@ -82,6 +82,7 @@ config ARCH_HAS_FORCE_DMA_UNENCRYPTED
 config SWIOTLB
bool
select NEED_DMA_MAP_STATE
+   select OF_EARLY_FLATTREE
 
 #
 # Should be selected if we can mmap non-coherent mappings to userspace.
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index e4368159f88a..7fb2ac087d23 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -36,6 +36,11 @@
 #include 
 #include 
 #include 
+#include 
+#include 
+#include 
+#include 
+#include 
 #ifdef CONFIG_DEBUG_FS
 #include 
 #endif
@@ -319,20 +324,21 @@ static void swiotlb_cleanup(void)
max_segment = 0;
 }
 
-int
-swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs)
+static int swiotlb_init_io_tlb_mem(struct io_tlb_mem *mem, phys_addr_t start,
+  size_t size)
 {
-   struct io_tlb_mem *mem = _tlb_default_mem;
-   unsigned long i, bytes;
+   unsigned long i;
+   void *vaddr = phys_to_virt(start);
 
-   bytes = nslabs << IO_TLB_SHIFT;
+   size = ALIGN(size, 1 << IO_TLB_SHIFT);
+   mem->nslabs = size >> IO_TLB_SHIFT;
+   mem->nslabs = ALIGN(mem->nslabs, IO_TLB_SEGSIZE);
 
-   mem->nslabs = nslabs;
-   mem->start = virt_to_phys(tlb);
-   mem->end = mem->start + bytes;
+   mem->start = start;
+   mem->end = mem->start + size;
 
-   set_memory_decrypted((unsigned long)tlb, bytes >> PAGE_SHIFT);
-   memset(tlb, 0, bytes);
+   set_memory_decrypted((unsigned long)vaddr, size >> PAGE_SHIFT);
+   memset(vaddr, 0, size);
 
/*
 * Allocate and initialize the free list array.  This array is used
@@ -356,13 +362,6 @@ swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs)
mem->orig_addr[i] = INVALID_PHYS_ADDR;
}
mem->index = 0;
-   no_iotlb_memory = false;
-
-   swiotlb_print_info();
-
-   late_alloc = 1;
-
-   swiotlb_set_max_segment(mem->nslabs << IO_TLB_SHIFT);
 
return 0;
 
@@ -375,6 +374,27 @@ swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs)
return -ENOMEM;
 }
 
+int swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs)
+{
+   struct io_tlb_mem *mem = _tlb_default_mem;
+   unsigned long bytes = nslabs << IO_TLB_SHIFT;
+

[RFC PATCH v3 3/6] swiotlb: Use restricted DMA pool if available

2021-01-05 Thread Claire Chang
Regardless of swiotlb setting, the restricted DMA pool is preferred if
available.

The restricted DMA pools provide a basic level of protection against
the DMA overwriting buffer contents at unexpected times. However, to
protect against general data leakage and system memory corruption, the
system needs to provide a way to restrict the DMA to a predefined memory
region.

Signed-off-by: Claire Chang 
---
 drivers/iommu/dma-iommu.c | 12 ++--
 include/linux/swiotlb.h   | 17 +++--
 kernel/dma/direct.c   |  8 
 kernel/dma/direct.h   | 10 ++
 kernel/dma/swiotlb.c  | 13 ++---
 5 files changed, 33 insertions(+), 27 deletions(-)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index f0305e6aac1b..1343cc2ef27a 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -516,7 +516,7 @@ static void __iommu_dma_unmap_swiotlb(struct device *dev, 
dma_addr_t dma_addr,
 
__iommu_dma_unmap(dev, dma_addr, size);
 
-   if (unlikely(is_swiotlb_buffer(phys)))
+   if (unlikely(is_swiotlb_buffer(dev, phys)))
swiotlb_tbl_unmap_single(dev, phys, size,
iova_align(iovad, size), dir, attrs);
 }
@@ -592,7 +592,7 @@ static dma_addr_t __iommu_dma_map_swiotlb(struct device 
*dev, phys_addr_t phys,
}
 
iova = __iommu_dma_map(dev, phys, aligned_size, prot, dma_mask);
-   if ((iova == DMA_MAPPING_ERROR) && is_swiotlb_buffer(phys))
+   if ((iova == DMA_MAPPING_ERROR) && is_swiotlb_buffer(dev, phys))
swiotlb_tbl_unmap_single(dev, phys, org_size,
aligned_size, dir, attrs);
 
@@ -764,7 +764,7 @@ static void iommu_dma_sync_single_for_cpu(struct device 
*dev,
if (!dev_is_dma_coherent(dev))
arch_sync_dma_for_cpu(phys, size, dir);
 
-   if (is_swiotlb_buffer(phys))
+   if (is_swiotlb_buffer(dev, phys))
swiotlb_tbl_sync_single(dev, phys, size, dir, SYNC_FOR_CPU);
 }
 
@@ -777,7 +777,7 @@ static void iommu_dma_sync_single_for_device(struct device 
*dev,
return;
 
phys = iommu_iova_to_phys(iommu_get_dma_domain(dev), dma_handle);
-   if (is_swiotlb_buffer(phys))
+   if (is_swiotlb_buffer(dev, phys))
swiotlb_tbl_sync_single(dev, phys, size, dir, SYNC_FOR_DEVICE);
 
if (!dev_is_dma_coherent(dev))
@@ -798,7 +798,7 @@ static void iommu_dma_sync_sg_for_cpu(struct device *dev,
if (!dev_is_dma_coherent(dev))
arch_sync_dma_for_cpu(sg_phys(sg), sg->length, dir);
 
-   if (is_swiotlb_buffer(sg_phys(sg)))
+   if (is_swiotlb_buffer(dev, sg_phys(sg)))
swiotlb_tbl_sync_single(dev, sg_phys(sg), sg->length,
dir, SYNC_FOR_CPU);
}
@@ -815,7 +815,7 @@ static void iommu_dma_sync_sg_for_device(struct device *dev,
return;
 
for_each_sg(sgl, sg, nelems, i) {
-   if (is_swiotlb_buffer(sg_phys(sg)))
+   if (is_swiotlb_buffer(dev, sg_phys(sg)))
swiotlb_tbl_sync_single(dev, sg_phys(sg), sg->length,
dir, SYNC_FOR_DEVICE);
 
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index a1bbd775..5135e5636042 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -2,12 +2,12 @@
 #ifndef __LINUX_SWIOTLB_H
 #define __LINUX_SWIOTLB_H
 
+#include 
 #include 
 #include 
 #include 
 #include 
 
-struct device;
 struct page;
 struct scatterlist;
 
@@ -106,9 +106,14 @@ struct io_tlb_mem {
 };
 extern struct io_tlb_mem io_tlb_default_mem;
 
-static inline bool is_swiotlb_buffer(phys_addr_t paddr)
+static inline struct io_tlb_mem *get_io_tlb_mem(struct device *dev)
 {
-   struct io_tlb_mem *mem = _tlb_default_mem;
+   return dev->dma_io_tlb_mem ? dev->dma_io_tlb_mem : _tlb_default_mem;
+}
+
+static inline bool is_swiotlb_buffer(struct device *dev, phys_addr_t paddr)
+{
+   struct io_tlb_mem *mem = get_io_tlb_mem(dev);
 
return paddr >= mem->start && paddr < mem->end;
 }
@@ -116,11 +121,11 @@ static inline bool is_swiotlb_buffer(phys_addr_t paddr)
 void __init swiotlb_exit(void);
 unsigned int swiotlb_max_segment(void);
 size_t swiotlb_max_mapping_size(struct device *dev);
-bool is_swiotlb_active(void);
+bool is_swiotlb_active(struct device *dev);
 void __init swiotlb_adjust_size(unsigned long new_size);
 #else
 #define swiotlb_force SWIOTLB_NO_FORCE
-static inline bool is_swiotlb_buffer(phys_addr_t paddr)
+static inline bool is_swiotlb_buffer(struct device *dev, phys_addr_t paddr)
 {
return false;
 }
@@ -136,7 +141,7 @@ static inline size_t swiotlb_max_mapping_size(struct device 
*dev)
return SIZE_MAX;
 }
 
-static inline bool is_swiotlb_active(void)
+static inline bool is_swiotlb_active(struct 

[RFC PATCH v3 0/6] Restricted DMA

2021-01-05 Thread Claire Chang
This series implements mitigations for lack of DMA access control on
systems without an IOMMU, which could result in the DMA accessing the
system memory at unexpected times and/or unexpected addresses, possibly
leading to data leakage or corruption.

For example, we plan to use the PCI-e bus for Wi-Fi and that PCI-e bus is
not behind an IOMMU. As PCI-e, by design, gives the device full access to
system memory, a vulnerability in the Wi-Fi firmware could easily escalate
to a full system exploit (remote wifi exploits: [1a], [1b] that shows a
full chain of exploits; [2], [3]).

To mitigate the security concerns, we introduce restricted DMA. Restricted
DMA utilizes the existing swiotlb to bounce streaming DMA in and out of a
specially allocated region and does memory allocation from the same region.
The feature on its own provides a basic level of protection against the DMA
overwriting buffer contents at unexpected times. However, to protect
against general data leakage and system memory corruption, the system needs
to provide a way to restrict the DMA to a predefined memory region (this is
usually done at firmware level, e.g. in ATF on some ARM platforms).

[1a] 
https://googleprojectzero.blogspot.com/2017/04/over-air-exploiting-broadcoms-wi-fi_4.html
[1b] 
https://googleprojectzero.blogspot.com/2017/04/over-air-exploiting-broadcoms-wi-fi_11.html
[2] https://blade.tencent.com/en/advisories/qualpwn/
[3] 
https://www.bleepingcomputer.com/news/security/vulnerabilities-found-in-highly-popular-firmware-for-wifi-chips/

Claire Chang (6):
  swiotlb: Add io_tlb_mem struct
  swiotlb: Add restricted DMA pool
  swiotlb: Use restricted DMA pool if available
  swiotlb: Add restricted DMA alloc/free support.
  dt-bindings: of: Add restricted DMA pool
  of: Add plumbing for restricted DMA pool

 .../reserved-memory/reserved-memory.txt   |  24 +
 arch/powerpc/platforms/pseries/svm.c  |   4 +-
 drivers/iommu/dma-iommu.c |  12 +-
 drivers/of/address.c  |  21 +
 drivers/of/device.c   |   4 +
 drivers/of/of_private.h   |   5 +
 drivers/xen/swiotlb-xen.c |   4 +-
 include/linux/device.h|   4 +
 include/linux/swiotlb.h   |  61 +-
 kernel/dma/Kconfig|   1 +
 kernel/dma/direct.c   |  20 +-
 kernel/dma/direct.h   |  10 +-
 kernel/dma/swiotlb.c  | 576 +++---
 13 files changed, 514 insertions(+), 232 deletions(-)

-- 
2.29.2.729.g45daf8777d-goog

v3: 
  Using only one reserved memory region for both streaming DMA and memory
  allocation.

v2:
  Building on top of swiotlb.
  https://lore.kernel.org/patchwork/cover/1280705/

v1:
  Using dma_map_ops.
  https://lore.kernel.org/patchwork/cover/1271660/


[PATCH] Bluetooth: hci_uart: Fix a race for write_work scheduling

2020-12-13 Thread Claire Chang
In hci_uart_write_work, there is a loop/goto checking the value of
HCI_UART_TX_WAKEUP. If HCI_UART_TX_WAKEUP is set again, it keeps trying
hci_uart_dequeue; otherwise, it clears HCI_UART_SENDING and returns.

In hci_uart_tx_wakeup, if HCI_UART_SENDING is already set, it sets
HCI_UART_TX_WAKEUP, skips schedule_work and assumes the running/pending
hci_uart_write_work worker will do hci_uart_dequeue properly.

However, if the HCI_UART_SENDING check in hci_uart_tx_wakeup is done after
the loop breaks, but before HCI_UART_SENDING is cleared in
hci_uart_write_work, the schedule_work is skipped incorrectly.

Fix this race by changing the order of HCI_UART_SENDING and
HCI_UART_TX_WAKEUP modification.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Fixes: 82f5169bf3d3 ("Bluetooth: hci_uart: add serdev driver support library")
Signed-off-by: Claire Chang 
---
 drivers/bluetooth/hci_ldisc.c  | 7 +++
 drivers/bluetooth/hci_serdev.c | 4 ++--
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index f83d67eafc9f..8be4d807d137 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -127,10 +127,9 @@ int hci_uart_tx_wakeup(struct hci_uart *hu)
if (!test_bit(HCI_UART_PROTO_READY, >flags))
goto no_schedule;
 
-   if (test_and_set_bit(HCI_UART_SENDING, >tx_state)) {
-   set_bit(HCI_UART_TX_WAKEUP, >tx_state);
+   set_bit(HCI_UART_TX_WAKEUP, >tx_state);
+   if (test_and_set_bit(HCI_UART_SENDING, >tx_state))
goto no_schedule;
-   }
 
BT_DBG("");
 
@@ -174,10 +173,10 @@ static void hci_uart_write_work(struct work_struct *work)
kfree_skb(skb);
}
 
+   clear_bit(HCI_UART_SENDING, >tx_state);
if (test_bit(HCI_UART_TX_WAKEUP, >tx_state))
goto restart;
 
-   clear_bit(HCI_UART_SENDING, >tx_state);
wake_up_bit(>tx_state, HCI_UART_SENDING);
 }
 
diff --git a/drivers/bluetooth/hci_serdev.c b/drivers/bluetooth/hci_serdev.c
index ef96ad06fa54..9e03402ef1b3 100644
--- a/drivers/bluetooth/hci_serdev.c
+++ b/drivers/bluetooth/hci_serdev.c
@@ -83,9 +83,9 @@ static void hci_uart_write_work(struct work_struct *work)
hci_uart_tx_complete(hu, hci_skb_pkt_type(skb));
kfree_skb(skb);
}
-   } while (test_bit(HCI_UART_TX_WAKEUP, >tx_state));
 
-   clear_bit(HCI_UART_SENDING, >tx_state);
+   clear_bit(HCI_UART_SENDING, >tx_state);
+   } while (test_bit(HCI_UART_TX_WAKEUP, >tx_state));
 }
 
 /* --- Interface to HCI layer -- */
-- 
2.29.2.576.ga3fc446d84-goog



Re: [PATCH] rfkill: Fix use-after-free in rfkill_resume()

2020-11-10 Thread Claire Chang
On Wed, Nov 11, 2020 at 1:35 AM Johannes Berg  wrote:
>
> On Tue, 2020-11-10 at 16:49 +0800, Claire Chang wrote:
> > If a device is getting removed or reprobed during resume, use-after-free
> > might happen. For example, h5_btrtl_resume()[drivers/bluetooth/hci_h5.c]
> > schedules a work queue for device reprobing. During the reprobing, if
> > rfkill_set_block() in rfkill_resume() is called after the corresponding
> > *_unregister() and kfree() are called, there will be an use-after-free
> > in hci_rfkill_set_block()[net/bluetooth/hci_core.c].
>
>
> Not sure I understand. So you're saying
>
>  * something (h5_btrtl_resume) schedules a worker
>  * said worker run, when it runs, calls rfkill_unregister()
>  * somehow rfkill_resume() still gets called after this
>
> But that can't really be right, device_del() removes it from the PM
> lists?

If device_del() is called right before the device_lock() in device_resume()[1],
it's possible the rfkill device is unregistered, but rfkill_resume is
still called.
We actually hit this during the suspend/resume stress test, although it's rare.

I also have a patch with multiple msleep that can 100% reproduce this
use-after-free. Happy to share here if needed.

[1] 
https://elixir.bootlin.com/linux/v5.10-rc3/source/drivers/base/power/main.c#L919

Thanks,
Claire

>
>
> johannes
>
>


[PATCH] rfkill: Fix use-after-free in rfkill_resume()

2020-11-10 Thread Claire Chang
If a device is getting removed or reprobed during resume, use-after-free
might happen. For example, h5_btrtl_resume()[drivers/bluetooth/hci_h5.c]
schedules a work queue for device reprobing. During the reprobing, if
rfkill_set_block() in rfkill_resume() is called after the corresponding
*_unregister() and kfree() are called, there will be an use-after-free
in hci_rfkill_set_block()[net/bluetooth/hci_core.c].

BUG: KASAN: use-after-free in hci_rfkill_set_block+0x58/0xc0 [bluetooth]
...
Call trace:
  dump_backtrace+0x0/0x154
  show_stack+0x20/0x2c
  dump_stack+0xbc/0x12c
  print_address_description+0x88/0x4b0
  __kasan_report+0x144/0x168
  kasan_report+0x10/0x18
  check_memory_region+0x19c/0x1ac
  __kasan_check_write+0x18/0x24
  hci_rfkill_set_block+0x58/0xc0 [bluetooth]
  rfkill_set_block+0x9c/0x120
  rfkill_resume+0x34/0x70
  dpm_run_callback+0xf0/0x1f4
  device_resume+0x210/0x22c

Fix this by checking rfkill->registered in rfkill_resume().
Since device_del() in rfkill_unregister() requires device_lock() and the
whole rfkill_resume() is also protected by the same lock in
device_resume()[drivers/base/power/main.c], we can make sure either the
rfkill->registered is false before rfkill_resume() starts or the rfkill
device won't be unregistered before rfkill_resume() returns.

Fixes: 8589086f4efd ("Bluetooth: hci_h5: Turn off RTL8723BS on suspend, reprobe 
on resume")
Signed-off-by: Claire Chang 
---
 net/rfkill/core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index 971c73c7d34c..97101c55763d 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -876,6 +876,9 @@ static int rfkill_resume(struct device *dev)
 
rfkill->suspended = false;
 
+   if (!rfkill->registered)
+   return 0;
+
if (!rfkill->persistent) {
cur = !!(rfkill->state & RFKILL_BLOCK_SW);
rfkill_set_block(rfkill, cur);
-- 
2.29.2.222.g5d2a92d10f8-goog



[PATCH] serial: 8250_mtk: Fix uart_get_baud_rate warning

2020-11-02 Thread Claire Chang
Mediatek 8250 port supports speed higher than uartclk / 16. If the baud
rates in both the new and the old termios setting are higher than
uartclk / 16, the WARN_ON in uart_get_baud_rate() will be triggered.
Passing NULL as the old termios so uart_get_baud_rate() will use
uartclk / 16 - 1 as the new baud rate which will be replaced by the
original baud rate later by tty_termios_encode_baud_rate() in
mtk8250_set_termios().

Fixes: 551e553f0d4a ("serial: 8250_mtk: Fix high-speed baud rates clamping")
Signed-off-by: Claire Chang 
---
 drivers/tty/serial/8250/8250_mtk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/8250/8250_mtk.c 
b/drivers/tty/serial/8250/8250_mtk.c
index 41f4120abdf2..fa876e2c13e5 100644
--- a/drivers/tty/serial/8250/8250_mtk.c
+++ b/drivers/tty/serial/8250/8250_mtk.c
@@ -317,7 +317,7 @@ mtk8250_set_termios(struct uart_port *port, struct ktermios 
*termios,
 */
baud = tty_termios_baud_rate(termios);
 
-   serial8250_do_set_termios(port, termios, old);
+   serial8250_do_set_termios(port, termios, NULL);
 
tty_termios_encode_baud_rate(termios, baud, baud);
 
-- 
2.29.1.341.ge80a0c044ae-goog



Re: [PATCH v2] Bluetooth: Move force_bredr_smp debugfs into hci_debugfs_create_bredr

2020-10-06 Thread Claire Chang
Hi,

This patch is to fix the kernel error
[   46.271811] debugfs: File 'force_bredr_smp' in directory 'hci0'
already present!

When powering off and on the bluetooth, the smp_register will try to create the
force_bredr_smp entry again.
Move the creation to hci_debugfs_create_bredr so the force_bredr_smp entry will
only be created when HCI_SETUP and HCI_CONFIG are not set.

Thanks,
Claire

On Tue, Sep 29, 2020 at 4:03 PM Claire Chang  wrote:
>
> Avoid multiple attempts to create the debugfs entry, force_bredr_smp,
> by moving it from the SMP registration to the BR/EDR controller init
> section. hci_debugfs_create_bredr is only called when HCI_SETUP and
> HCI_CONFIG is not set.
>
> Signed-off-by: Claire Chang 
> ---
> v2: correct a typo in commit message
>
>  net/bluetooth/hci_debugfs.c | 50 +
>  net/bluetooth/smp.c | 44 ++--
>  net/bluetooth/smp.h |  2 ++
>  3 files changed, 54 insertions(+), 42 deletions(-)
>
> diff --git a/net/bluetooth/hci_debugfs.c b/net/bluetooth/hci_debugfs.c
> index 5e8af2658e44..4626e0289a97 100644
> --- a/net/bluetooth/hci_debugfs.c
> +++ b/net/bluetooth/hci_debugfs.c
> @@ -494,6 +494,45 @@ static int auto_accept_delay_get(void *data, u64 *val)
>  DEFINE_SIMPLE_ATTRIBUTE(auto_accept_delay_fops, auto_accept_delay_get,
> auto_accept_delay_set, "%llu\n");
>
> +static ssize_t force_bredr_smp_read(struct file *file,
> +   char __user *user_buf,
> +   size_t count, loff_t *ppos)
> +{
> +   struct hci_dev *hdev = file->private_data;
> +   char buf[3];
> +
> +   buf[0] = hci_dev_test_flag(hdev, HCI_FORCE_BREDR_SMP) ? 'Y' : 'N';
> +   buf[1] = '\n';
> +   buf[2] = '\0';
> +   return simple_read_from_buffer(user_buf, count, ppos, buf, 2);
> +}
> +
> +static ssize_t force_bredr_smp_write(struct file *file,
> +const char __user *user_buf,
> +size_t count, loff_t *ppos)
> +{
> +   struct hci_dev *hdev = file->private_data;
> +   bool enable;
> +   int err;
> +
> +   err = kstrtobool_from_user(user_buf, count, );
> +   if (err)
> +   return err;
> +
> +   err = smp_force_bredr(hdev, enable);
> +   if (err)
> +   return err;
> +
> +   return count;
> +}
> +
> +static const struct file_operations force_bredr_smp_fops = {
> +   .open   = simple_open,
> +   .read   = force_bredr_smp_read,
> +   .write  = force_bredr_smp_write,
> +   .llseek = default_llseek,
> +};
> +
>  static int idle_timeout_set(void *data, u64 val)
>  {
> struct hci_dev *hdev = data;
> @@ -589,6 +628,17 @@ void hci_debugfs_create_bredr(struct hci_dev *hdev)
> debugfs_create_file("voice_setting", 0444, hdev->debugfs, hdev,
> _setting_fops);
>
> +   /* If the controller does not support BR/EDR Secure Connections
> +* feature, then the BR/EDR SMP channel shall not be present.
> +*
> +* To test this with Bluetooth 4.0 controllers, create a debugfs
> +* switch that allows forcing BR/EDR SMP support and accepting
> +* cross-transport pairing on non-AES encrypted connections.
> +*/
> +   if (!lmp_sc_capable(hdev))
> +   debugfs_create_file("force_bredr_smp", 0644, hdev->debugfs,
> +   hdev, _bredr_smp_fops);
> +
> if (lmp_ssp_capable(hdev)) {
> debugfs_create_file("ssp_debug_mode", 0444, hdev->debugfs,
> hdev, _debug_mode_fops);
> diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
> index 433227f96c73..8b817e4358fd 100644
> --- a/net/bluetooth/smp.c
> +++ b/net/bluetooth/smp.c
> @@ -3353,31 +3353,8 @@ static void smp_del_chan(struct l2cap_chan *chan)
> l2cap_chan_put(chan);
>  }
>
> -static ssize_t force_bredr_smp_read(struct file *file,
> -   char __user *user_buf,
> -   size_t count, loff_t *ppos)
> +int smp_force_bredr(struct hci_dev *hdev, bool enable)
>  {
> -   struct hci_dev *hdev = file->private_data;
> -   char buf[3];
> -
> -   buf[0] = hci_dev_test_flag(hdev, HCI_FORCE_BREDR_SMP) ? 'Y': 'N';
> -   buf[1] = '\n';
> -   buf[2] = '\0';
> -   return simple_read_from_buffer(user_buf, count, ppos, buf, 2);
> -}
> -
> -static ssize_t force_bredr_smp_write(struct file *file,
> - 

[PATCH v2] Bluetooth: Move force_bredr_smp debugfs into hci_debugfs_create_bredr

2020-09-29 Thread Claire Chang
Avoid multiple attempts to create the debugfs entry, force_bredr_smp,
by moving it from the SMP registration to the BR/EDR controller init
section. hci_debugfs_create_bredr is only called when HCI_SETUP and
HCI_CONFIG is not set.

Signed-off-by: Claire Chang 
---
v2: correct a typo in commit message

 net/bluetooth/hci_debugfs.c | 50 +
 net/bluetooth/smp.c | 44 ++--
 net/bluetooth/smp.h |  2 ++
 3 files changed, 54 insertions(+), 42 deletions(-)

diff --git a/net/bluetooth/hci_debugfs.c b/net/bluetooth/hci_debugfs.c
index 5e8af2658e44..4626e0289a97 100644
--- a/net/bluetooth/hci_debugfs.c
+++ b/net/bluetooth/hci_debugfs.c
@@ -494,6 +494,45 @@ static int auto_accept_delay_get(void *data, u64 *val)
 DEFINE_SIMPLE_ATTRIBUTE(auto_accept_delay_fops, auto_accept_delay_get,
auto_accept_delay_set, "%llu\n");
 
+static ssize_t force_bredr_smp_read(struct file *file,
+   char __user *user_buf,
+   size_t count, loff_t *ppos)
+{
+   struct hci_dev *hdev = file->private_data;
+   char buf[3];
+
+   buf[0] = hci_dev_test_flag(hdev, HCI_FORCE_BREDR_SMP) ? 'Y' : 'N';
+   buf[1] = '\n';
+   buf[2] = '\0';
+   return simple_read_from_buffer(user_buf, count, ppos, buf, 2);
+}
+
+static ssize_t force_bredr_smp_write(struct file *file,
+const char __user *user_buf,
+size_t count, loff_t *ppos)
+{
+   struct hci_dev *hdev = file->private_data;
+   bool enable;
+   int err;
+
+   err = kstrtobool_from_user(user_buf, count, );
+   if (err)
+   return err;
+
+   err = smp_force_bredr(hdev, enable);
+   if (err)
+   return err;
+
+   return count;
+}
+
+static const struct file_operations force_bredr_smp_fops = {
+   .open   = simple_open,
+   .read   = force_bredr_smp_read,
+   .write  = force_bredr_smp_write,
+   .llseek = default_llseek,
+};
+
 static int idle_timeout_set(void *data, u64 val)
 {
struct hci_dev *hdev = data;
@@ -589,6 +628,17 @@ void hci_debugfs_create_bredr(struct hci_dev *hdev)
debugfs_create_file("voice_setting", 0444, hdev->debugfs, hdev,
_setting_fops);
 
+   /* If the controller does not support BR/EDR Secure Connections
+* feature, then the BR/EDR SMP channel shall not be present.
+*
+* To test this with Bluetooth 4.0 controllers, create a debugfs
+* switch that allows forcing BR/EDR SMP support and accepting
+* cross-transport pairing on non-AES encrypted connections.
+*/
+   if (!lmp_sc_capable(hdev))
+   debugfs_create_file("force_bredr_smp", 0644, hdev->debugfs,
+   hdev, _bredr_smp_fops);
+
if (lmp_ssp_capable(hdev)) {
debugfs_create_file("ssp_debug_mode", 0444, hdev->debugfs,
hdev, _debug_mode_fops);
diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index 433227f96c73..8b817e4358fd 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -3353,31 +3353,8 @@ static void smp_del_chan(struct l2cap_chan *chan)
l2cap_chan_put(chan);
 }
 
-static ssize_t force_bredr_smp_read(struct file *file,
-   char __user *user_buf,
-   size_t count, loff_t *ppos)
+int smp_force_bredr(struct hci_dev *hdev, bool enable)
 {
-   struct hci_dev *hdev = file->private_data;
-   char buf[3];
-
-   buf[0] = hci_dev_test_flag(hdev, HCI_FORCE_BREDR_SMP) ? 'Y': 'N';
-   buf[1] = '\n';
-   buf[2] = '\0';
-   return simple_read_from_buffer(user_buf, count, ppos, buf, 2);
-}
-
-static ssize_t force_bredr_smp_write(struct file *file,
-const char __user *user_buf,
-size_t count, loff_t *ppos)
-{
-   struct hci_dev *hdev = file->private_data;
-   bool enable;
-   int err;
-
-   err = kstrtobool_from_user(user_buf, count, );
-   if (err)
-   return err;
-
if (enable == hci_dev_test_flag(hdev, HCI_FORCE_BREDR_SMP))
return -EALREADY;
 
@@ -3399,16 +3376,9 @@ static ssize_t force_bredr_smp_write(struct file *file,
 
hci_dev_change_flag(hdev, HCI_FORCE_BREDR_SMP);
 
-   return count;
+   return 0;
 }
 
-static const struct file_operations force_bredr_smp_fops = {
-   .open   = simple_open,
-   .read   = force_bredr_smp_read,
-   .write  = force_bredr_smp_write,
-   .llseek = default_llseek,
-};
-
 int smp_register(struct hci_dev *hdev)
 {
struct l2cap_chan *chan;
@@ -3433,17 +3403,7 @@ int smp_r

[PATCH] Bluetooth: Move force_bredr_smp debugfs into hci_debugfs_create_bredr

2020-09-15 Thread Claire Chang
Avoid multiple attempts to create the debugfs entery, force_bredr_smp,
by moving it from the SMP registration to the BR/EDR controller init
section. hci_debugfs_create_bredr is only called when HCI_SETUP and
HCI_CONFIG is not set.

Signed-off-by: Claire Chang 
---
 net/bluetooth/hci_debugfs.c | 50 +
 net/bluetooth/smp.c | 44 ++--
 net/bluetooth/smp.h |  2 ++
 3 files changed, 54 insertions(+), 42 deletions(-)

diff --git a/net/bluetooth/hci_debugfs.c b/net/bluetooth/hci_debugfs.c
index 5e8af2658e44..4626e0289a97 100644
--- a/net/bluetooth/hci_debugfs.c
+++ b/net/bluetooth/hci_debugfs.c
@@ -494,6 +494,45 @@ static int auto_accept_delay_get(void *data, u64 *val)
 DEFINE_SIMPLE_ATTRIBUTE(auto_accept_delay_fops, auto_accept_delay_get,
auto_accept_delay_set, "%llu\n");
 
+static ssize_t force_bredr_smp_read(struct file *file,
+   char __user *user_buf,
+   size_t count, loff_t *ppos)
+{
+   struct hci_dev *hdev = file->private_data;
+   char buf[3];
+
+   buf[0] = hci_dev_test_flag(hdev, HCI_FORCE_BREDR_SMP) ? 'Y' : 'N';
+   buf[1] = '\n';
+   buf[2] = '\0';
+   return simple_read_from_buffer(user_buf, count, ppos, buf, 2);
+}
+
+static ssize_t force_bredr_smp_write(struct file *file,
+const char __user *user_buf,
+size_t count, loff_t *ppos)
+{
+   struct hci_dev *hdev = file->private_data;
+   bool enable;
+   int err;
+
+   err = kstrtobool_from_user(user_buf, count, );
+   if (err)
+   return err;
+
+   err = smp_force_bredr(hdev, enable);
+   if (err)
+   return err;
+
+   return count;
+}
+
+static const struct file_operations force_bredr_smp_fops = {
+   .open   = simple_open,
+   .read   = force_bredr_smp_read,
+   .write  = force_bredr_smp_write,
+   .llseek = default_llseek,
+};
+
 static int idle_timeout_set(void *data, u64 val)
 {
struct hci_dev *hdev = data;
@@ -589,6 +628,17 @@ void hci_debugfs_create_bredr(struct hci_dev *hdev)
debugfs_create_file("voice_setting", 0444, hdev->debugfs, hdev,
_setting_fops);
 
+   /* If the controller does not support BR/EDR Secure Connections
+* feature, then the BR/EDR SMP channel shall not be present.
+*
+* To test this with Bluetooth 4.0 controllers, create a debugfs
+* switch that allows forcing BR/EDR SMP support and accepting
+* cross-transport pairing on non-AES encrypted connections.
+*/
+   if (!lmp_sc_capable(hdev))
+   debugfs_create_file("force_bredr_smp", 0644, hdev->debugfs,
+   hdev, _bredr_smp_fops);
+
if (lmp_ssp_capable(hdev)) {
debugfs_create_file("ssp_debug_mode", 0444, hdev->debugfs,
hdev, _debug_mode_fops);
diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index 433227f96c73..8b817e4358fd 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -3353,31 +3353,8 @@ static void smp_del_chan(struct l2cap_chan *chan)
l2cap_chan_put(chan);
 }
 
-static ssize_t force_bredr_smp_read(struct file *file,
-   char __user *user_buf,
-   size_t count, loff_t *ppos)
+int smp_force_bredr(struct hci_dev *hdev, bool enable)
 {
-   struct hci_dev *hdev = file->private_data;
-   char buf[3];
-
-   buf[0] = hci_dev_test_flag(hdev, HCI_FORCE_BREDR_SMP) ? 'Y': 'N';
-   buf[1] = '\n';
-   buf[2] = '\0';
-   return simple_read_from_buffer(user_buf, count, ppos, buf, 2);
-}
-
-static ssize_t force_bredr_smp_write(struct file *file,
-const char __user *user_buf,
-size_t count, loff_t *ppos)
-{
-   struct hci_dev *hdev = file->private_data;
-   bool enable;
-   int err;
-
-   err = kstrtobool_from_user(user_buf, count, );
-   if (err)
-   return err;
-
if (enable == hci_dev_test_flag(hdev, HCI_FORCE_BREDR_SMP))
return -EALREADY;
 
@@ -3399,16 +3376,9 @@ static ssize_t force_bredr_smp_write(struct file *file,
 
hci_dev_change_flag(hdev, HCI_FORCE_BREDR_SMP);
 
-   return count;
+   return 0;
 }
 
-static const struct file_operations force_bredr_smp_fops = {
-   .open   = simple_open,
-   .read   = force_bredr_smp_read,
-   .write  = force_bredr_smp_write,
-   .llseek = default_llseek,
-};
-
 int smp_register(struct hci_dev *hdev)
 {
struct l2cap_chan *chan;
@@ -3433,17 +3403,7 @@ int smp_register(struct hci_dev *hdev)
 
hdev->smp_dat

Re: [RFC v2 4/5] dt-bindings: of: Add plumbing for restricted DMA pool

2020-09-08 Thread Claire Chang
On Tue, Aug 25, 2020 at 1:30 AM Tomasz Figa  wrote:
>
> On Tue, Aug 11, 2020 at 11:15 AM Tomasz Figa  wrote:
> >
> > On Mon, Aug 3, 2020 at 5:15 PM Tomasz Figa  wrote:
> > >
> > > Hi Claire and Rob,
> > >
> > > On Mon, Aug 3, 2020 at 4:26 PM Claire Chang  wrote:
> > > >
> > > > On Sat, Aug 1, 2020 at 4:58 AM Rob Herring  wrote:
> > > > >
> > > > > On Tue, Jul 28, 2020 at 01:01:39PM +0800, Claire Chang wrote:
> > > > > > Introduce the new compatible string, device-swiotlb-pool, for 
> > > > > > restricted
> > > > > > DMA. One can specify the address and length of the device swiotlb 
> > > > > > memory
> > > > > > region by device-swiotlb-pool in the device tree.
> > > > > >
> > > > > > Signed-off-by: Claire Chang 
> > > > > > ---
> > > > > >  .../reserved-memory/reserved-memory.txt   | 35 
> > > > > > +++
> > > > > >  1 file changed, 35 insertions(+)
> > > > > >
> > > > > > diff --git 
> > > > > > a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> > > > > >  
> > > > > > b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> > > > > > index 4dd20de6977f..78850896e1d0 100644
> > > > > > --- 
> > > > > > a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> > > > > > +++ 
> > > > > > b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> > > > > > @@ -51,6 +51,24 @@ compatible (optional) - standard definition
> > > > > >used as a shared pool of DMA buffers for a set of 
> > > > > > devices. It can
> > > > > >be used by an operating system to instantiate the 
> > > > > > necessary pool
> > > > > >management subsystem if necessary.
> > > > > > +- device-swiotlb-pool: This indicates a region of memory 
> > > > > > meant to be
> > > > >
> > > > > swiotlb is a Linux thing. The binding should be independent.
> > > > Got it. Thanks for pointing this out.
> > > >
> > > > >
> > > > > > +  used as a pool of device swiotlb buffers for a given 
> > > > > > device. When
> > > > > > +  using this, the no-map and reusable properties must not 
> > > > > > be set, so the
> > > > > > +  operating system can create a virtual mapping that will 
> > > > > > be used for
> > > > > > +  synchronization. Also, there must be a restricted-dma 
> > > > > > property in the
> > > > > > +  device node to specify the indexes of reserved-memory 
> > > > > > nodes. One can
> > > > > > +  specify two reserved-memory nodes in the device tree. 
> > > > > > One with
> > > > > > +  shared-dma-pool to handle the coherent DMA buffer 
> > > > > > allocation, and
> > > > > > +  another one with device-swiotlb-pool for regular DMA 
> > > > > > to/from system
> > > > > > +  memory, which would be subject to bouncing. The main 
> > > > > > purpose for
> > > > > > +  restricted DMA is to mitigate the lack of DMA access 
> > > > > > control on
> > > > > > +  systems without an IOMMU, which could result in the DMA 
> > > > > > accessing the
> > > > > > +  system memory at unexpected times and/or unexpected 
> > > > > > addresses,
> > > > > > +  possibly leading to data leakage or corruption. The 
> > > > > > feature on its own
> > > > > > +  provides a basic level of protection against the DMA 
> > > > > > overwriting buffer
> > > > > > +  contents at unexpected times. However, to protect 
> > > > > > against general data
> > > > > > +  leakage and system memory corruption, the system needs 
> > > > > > to provide a
> > > > > > +  way to restrict the DMA to a predefined memory region.
> > > > >

Re: [RFC v2 4/5] dt-bindings: of: Add plumbing for restricted DMA pool

2020-08-03 Thread Claire Chang
On Sat, Aug 1, 2020 at 4:58 AM Rob Herring  wrote:
>
> On Tue, Jul 28, 2020 at 01:01:39PM +0800, Claire Chang wrote:
> > Introduce the new compatible string, device-swiotlb-pool, for restricted
> > DMA. One can specify the address and length of the device swiotlb memory
> > region by device-swiotlb-pool in the device tree.
> >
> > Signed-off-by: Claire Chang 
> > ---
> >  .../reserved-memory/reserved-memory.txt   | 35 +++
> >  1 file changed, 35 insertions(+)
> >
> > diff --git 
> > a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt 
> > b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> > index 4dd20de6977f..78850896e1d0 100644
> > --- a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> > +++ b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> > @@ -51,6 +51,24 @@ compatible (optional) - standard definition
> >used as a shared pool of DMA buffers for a set of devices. It can
> >be used by an operating system to instantiate the necessary pool
> >management subsystem if necessary.
> > +- device-swiotlb-pool: This indicates a region of memory meant to 
> > be
>
> swiotlb is a Linux thing. The binding should be independent.
Got it. Thanks for pointing this out.

>
> > +  used as a pool of device swiotlb buffers for a given device. When
> > +  using this, the no-map and reusable properties must not be set, 
> > so the
> > +  operating system can create a virtual mapping that will be used 
> > for
> > +  synchronization. Also, there must be a restricted-dma property 
> > in the
> > +  device node to specify the indexes of reserved-memory nodes. One 
> > can
> > +  specify two reserved-memory nodes in the device tree. One with
> > +  shared-dma-pool to handle the coherent DMA buffer allocation, and
> > +  another one with device-swiotlb-pool for regular DMA to/from 
> > system
> > +  memory, which would be subject to bouncing. The main purpose for
> > +  restricted DMA is to mitigate the lack of DMA access control on
> > +  systems without an IOMMU, which could result in the DMA 
> > accessing the
> > +  system memory at unexpected times and/or unexpected addresses,
> > +  possibly leading to data leakage or corruption. The feature on 
> > its own
> > +  provides a basic level of protection against the DMA overwriting 
> > buffer
> > +  contents at unexpected times. However, to protect against 
> > general data
> > +  leakage and system memory corruption, the system needs to 
> > provide a
> > +  way to restrict the DMA to a predefined memory region.
>
> I'm pretty sure we already support per device carveouts and I don't
> understand how this is different.
We use this to bounce streaming DMA in and out of a specially allocated region.
I'll try to merge this with the existing one (i.e., shared-dma-pool)
to see if that
makes things clearer.

>
> What is the last sentence supposed to imply? You need an IOMMU?
The main purpose is to mitigate the lack of DMA access control on
systems without an IOMMU.
For example, we plan to use this plus a MPU for our PCIe WiFi which is
not behind an IOMMU.

>
> >  - vendor specific string in the form ,[-]
> >  no-map (optional) - empty property
> >  - Indicates the operating system must not create a virtual mapping
> > @@ -117,6 +135,16 @@ one for multimedia processing (named 
> > multimedia-memory@7700, 64MiB).
> >   compatible = "acme,multimedia-memory";
> >   reg = <0x7700 0x400>;
> >   };
> > +
> > + wifi_coherent_mem_region: wifi_coherent_mem_region {
> > + compatible = "shared-dma-pool";
> > + reg = <0x5000 0x40>;
> > + };
> > +
> > + wifi_device_swiotlb_region: wifi_device_swiotlb_region {
> > + compatible = "device-swiotlb-pool";
> > + reg = <0x5040 0x400>;
> > + };
> >   };
> >
> >   /* ... */
> > @@ -135,4 +163,11 @@ one for multimedia processing (named 
> > multimedia-memory@7700, 64MiB).
> >   memory-region = <_reserved>;
> >   /* ... */
> >   };
> > +
> > + pcie_wifi: pcie_wifi@0,0 {
> > + memory-region = <_coherent_mem_region>,
> > +  <_device_swiotlb_region>;
> > + restricted-dma = <0>, <1>;
> > + /* ... */
> > + };
> >  };
> > --
> > 2.28.0.rc0.142.g3c755180ce-goog
> >


Re: [RFC v2 0/5] Restricted DMA

2020-07-28 Thread Claire Chang
It seems that I didn't rebase the patchset properly. There are some
build test errors.
Sorry about that. Please kindly ignore those rebase issues. I'll fix
them in the next version.


On Tue, Jul 28, 2020 at 1:01 PM Claire Chang  wrote:
>
> This series implements mitigations for lack of DMA access control on
> systems without an IOMMU, which could result in the DMA accessing the
> system memory at unexpected times and/or unexpected addresses, possibly
> leading to data leakage or corruption.
>
> For example, we plan to use the PCI-e bus for Wi-Fi on one MTK platform and
> that PCI-e bus is not behind an IOMMU. As PCI-e, by design, gives the
> device full access to system memory, a vulnerability in the Wi-Fi firmware
> could easily escalate to a full system exploit (remote wifi exploits: [1a],
> [1b] that shows a full chain of exploits; [2], [3]).
>
> To mitigate the security concerns, we introduce restricted DMA. The
> restricted DMA is implemented by per-device swiotlb and coherent memory
> pools. The feature on its own provides a basic level of protection against
> the DMA overwriting buffer contents at unexpected times. However, to
> protect against general data leakage and system memory corruption, the
> system needs to provide a way to restrict the DMA to a predefined memory
> region (this is usually done at firmware level, e.g. in ATF on some ARM
> platforms).
>
> [1a] 
> https://googleprojectzero.blogspot.com/2017/04/over-air-exploiting-broadcoms-wi-fi_4.html
> [1b] 
> https://googleprojectzero.blogspot.com/2017/04/over-air-exploiting-broadcoms-wi-fi_11.html
> [2] https://blade.tencent.com/en/advisories/qualpwn/
> [3] 
> https://www.bleepingcomputer.com/news/security/vulnerabilities-found-in-highly-popular-firmware-for-wifi-chips/
>
>
> Claire Chang (5):
>   swiotlb: Add io_tlb_mem struct
>   swiotlb: Add device swiotlb pool
>   swiotlb: Use device swiotlb pool if available
>   dt-bindings: of: Add plumbing for restricted DMA pool
>   of: Add plumbing for restricted DMA pool
>
>  .../reserved-memory/reserved-memory.txt   |  35 ++
>  drivers/iommu/intel/iommu.c   |   8 +-
>  drivers/of/address.c  |  39 ++
>  drivers/of/device.c   |   3 +
>  drivers/of/of_private.h   |   6 +
>  drivers/xen/swiotlb-xen.c |   4 +-
>  include/linux/device.h|   4 +
>  include/linux/dma-direct.h|   8 +-
>  include/linux/swiotlb.h   |  49 +-
>  kernel/dma/direct.c   |   8 +-
>  kernel/dma/swiotlb.c  | 418 +++---
>  11 files changed, 393 insertions(+), 189 deletions(-)
>
> --
> v1: https://lore.kernel.org/patchwork/cover/1271660/
> Changes in v2:
> - build on top of swiotlb
>
> 2.28.0.rc0.142.g3c755180ce-goog
>


Re: [PATCH 1/4] dma-mapping: Add bounced DMA ops

2020-07-27 Thread Claire Chang
v2 that reuses SWIOTLB here: https://lore.kernel.org/patchwork/cover/1280705/

Thanks,
Claire


[RFC v2 5/5] of: Add plumbing for restricted DMA pool

2020-07-27 Thread Claire Chang
If a device is not behind an IOMMU, we look up the device node and set
up the restricted DMA when the restricted-dma property is presented.
One can specify two reserved-memory nodes in the device tree. One with
shared-dma-pool to handle the coherent DMA buffer allocation, and
another one with device-swiotlb-pool for regular DMA to/from system
memory, which would be subject to bouncing.

Signed-off-by: Claire Chang 
---
 drivers/of/address.c| 39 +++
 drivers/of/device.c |  3 +++
 drivers/of/of_private.h |  6 ++
 3 files changed, 48 insertions(+)

diff --git a/drivers/of/address.c b/drivers/of/address.c
index 381dc9be7b22..1285f914481f 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -8,6 +8,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -1009,6 +1010,44 @@ int of_dma_get_range(struct device_node *np, u64 
*dma_addr, u64 *paddr, u64 *siz
return ret;
 }
 
+int of_dma_set_restricted_buffer(struct device *dev)
+{
+   int length, size, ret, i;
+   u32 idx[2];
+
+   if (!dev || !dev->of_node)
+   return -EINVAL;
+
+   if (!of_get_property(dev->of_node, "restricted-dma", ))
+   return 0;
+
+   size = length / sizeof(idx[0]);
+   if (size > ARRAY_SIZE(idx)) {
+   dev_err(dev,
+   "restricted-dma expected less than or equal to %d 
indexs, but got %d\n",
+   ARRAY_SIZE(idx), size);
+   return -EINVAL;
+   }
+
+   ret = of_property_read_u32_array(dev->of_node, "restricted-dma", idx,
+size);
+   if (ret)
+   return ret;
+
+   for (i = 0; i < size; i++) {
+   ret = of_reserved_mem_device_init_by_idx(dev, dev->of_node,
+idx[i]);
+   if (ret) {
+   dev_err(dev,
+   "of_reserved_mem_device_init_by_idx() failed 
with %d\n",
+   ret);
+   return ret;
+   }
+   }
+
+   return 0;
+}
+
 /**
  * of_dma_is_coherent - Check if device is coherent
  * @np:device node
diff --git a/drivers/of/device.c b/drivers/of/device.c
index 27203bfd0b22..83d6cf8a8256 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -169,6 +169,9 @@ int of_dma_configure(struct device *dev, struct device_node 
*np, bool force_dma)
 
arch_setup_dma_ops(dev, dma_addr, size, iommu, coherent);
 
+   if (!iommu)
+   return of_dma_set_restricted_buffer(dev);
+
return 0;
 }
 EXPORT_SYMBOL_GPL(of_dma_configure);
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index edc682249c00..f2e3adfb7d85 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -160,12 +160,18 @@ extern int of_bus_n_size_cells(struct device_node *np);
 #ifdef CONFIG_OF_ADDRESS
 extern int of_dma_get_range(struct device_node *np, u64 *dma_addr,
u64 *paddr, u64 *size);
+extern int of_dma_set_restricted_buffer(struct device *dev);
 #else
 static inline int of_dma_get_range(struct device_node *np, u64 *dma_addr,
   u64 *paddr, u64 *size)
 {
return -ENODEV;
 }
+
+static inline int of_dma_get_restricted_buffer(struct device *dev)
+{
+   return -ENODEV;
+}
 #endif
 
 #endif /* _LINUX_OF_PRIVATE_H */
-- 
2.28.0.rc0.142.g3c755180ce-goog



[RFC v2 4/5] dt-bindings: of: Add plumbing for restricted DMA pool

2020-07-27 Thread Claire Chang
Introduce the new compatible string, device-swiotlb-pool, for restricted
DMA. One can specify the address and length of the device swiotlb memory
region by device-swiotlb-pool in the device tree.

Signed-off-by: Claire Chang 
---
 .../reserved-memory/reserved-memory.txt   | 35 +++
 1 file changed, 35 insertions(+)

diff --git 
a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt 
b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
index 4dd20de6977f..78850896e1d0 100644
--- a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
+++ b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
@@ -51,6 +51,24 @@ compatible (optional) - standard definition
   used as a shared pool of DMA buffers for a set of devices. It can
   be used by an operating system to instantiate the necessary pool
   management subsystem if necessary.
+- device-swiotlb-pool: This indicates a region of memory meant to be
+  used as a pool of device swiotlb buffers for a given device. When
+  using this, the no-map and reusable properties must not be set, so 
the
+  operating system can create a virtual mapping that will be used for
+  synchronization. Also, there must be a restricted-dma property in the
+  device node to specify the indexes of reserved-memory nodes. One can
+  specify two reserved-memory nodes in the device tree. One with
+  shared-dma-pool to handle the coherent DMA buffer allocation, and
+  another one with device-swiotlb-pool for regular DMA to/from system
+  memory, which would be subject to bouncing. The main purpose for
+  restricted DMA is to mitigate the lack of DMA access control on
+  systems without an IOMMU, which could result in the DMA accessing the
+  system memory at unexpected times and/or unexpected addresses,
+  possibly leading to data leakage or corruption. The feature on its 
own
+  provides a basic level of protection against the DMA overwriting 
buffer
+  contents at unexpected times. However, to protect against general 
data
+  leakage and system memory corruption, the system needs to provide a
+  way to restrict the DMA to a predefined memory region.
 - vendor specific string in the form ,[-]
 no-map (optional) - empty property
 - Indicates the operating system must not create a virtual mapping
@@ -117,6 +135,16 @@ one for multimedia processing (named 
multimedia-memory@7700, 64MiB).
compatible = "acme,multimedia-memory";
reg = <0x7700 0x400>;
};
+
+   wifi_coherent_mem_region: wifi_coherent_mem_region {
+   compatible = "shared-dma-pool";
+   reg = <0x5000 0x40>;
+   };
+
+   wifi_device_swiotlb_region: wifi_device_swiotlb_region {
+   compatible = "device-swiotlb-pool";
+   reg = <0x5040 0x400>;
+   };
};
 
/* ... */
@@ -135,4 +163,11 @@ one for multimedia processing (named 
multimedia-memory@7700, 64MiB).
memory-region = <_reserved>;
/* ... */
};
+
+   pcie_wifi: pcie_wifi@0,0 {
+   memory-region = <_coherent_mem_region>,
+<_device_swiotlb_region>;
+   restricted-dma = <0>, <1>;
+   /* ... */
+   };
 };
-- 
2.28.0.rc0.142.g3c755180ce-goog



[RFC v2 3/5] swiotlb: Use device swiotlb pool if available

2020-07-27 Thread Claire Chang
Regardless of swiotlb setting, the device swiotlb pool is preferred if
available.

The device swiotlb pools provide a basic level of protection against
the DMA overwriting buffer contents at unexpected times. However, to
protect against general data leakage and system memory corruption, the
system needs to provide a way to restrict the DMA to a predefined memory
region.

Signed-off-by: Claire Chang 
---
 drivers/iommu/intel/iommu.c |  6 +++---
 include/linux/dma-direct.h  |  8 
 include/linux/swiotlb.h | 13 -
 kernel/dma/direct.c |  8 
 kernel/dma/swiotlb.c| 18 +++---
 5 files changed, 30 insertions(+), 23 deletions(-)

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 44c9230251eb..37d6583cf628 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -3684,7 +3684,7 @@ bounce_sync_single(struct device *dev, dma_addr_t addr, 
size_t size,
return;
 
tlb_addr = intel_iommu_iova_to_phys(>domain, addr);
-   if (is_swiotlb_buffer(tlb_addr))
+   if (is_swiotlb_buffer(dev, tlb_addr))
swiotlb_tbl_sync_single(dev, tlb_addr, size, dir, target);
 }
 
@@ -3768,7 +3768,7 @@ bounce_map_single(struct device *dev, phys_addr_t paddr, 
size_t size,
return (phys_addr_t)iova_pfn << PAGE_SHIFT;
 
 mapping_error:
-   if (is_swiotlb_buffer(tlb_addr))
+   if (is_swiotlb_buffer(dev, tlb_addr))
swiotlb_tbl_unmap_single(dev, tlb_addr, size,
 aligned_size, dir, attrs);
 swiotlb_error:
@@ -3796,7 +3796,7 @@ bounce_unmap_single(struct device *dev, dma_addr_t 
dev_addr, size_t size,
return;
 
intel_unmap(dev, dev_addr, size);
-   if (is_swiotlb_buffer(tlb_addr))
+   if (is_swiotlb_buffer(dev, tlb_addr))
swiotlb_tbl_unmap_single(dev, tlb_addr, size,
 aligned_size, dir, attrs);
 
diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
index 5a3ce2a24794..1cf920ddb2f6 100644
--- a/include/linux/dma-direct.h
+++ b/include/linux/dma-direct.h
@@ -134,7 +134,7 @@ static inline void dma_direct_sync_single_for_device(struct 
device *dev,
 {
phys_addr_t paddr = dma_to_phys(dev, addr);
 
-   if (unlikely(is_swiotlb_buffer(paddr)))
+   if (unlikely(is_swiotlb_buffer(dev, paddr)))
swiotlb_tbl_sync_single(dev, paddr, size, dir, SYNC_FOR_DEVICE);
 
if (!dev_is_dma_coherent(dev))
@@ -151,7 +151,7 @@ static inline void dma_direct_sync_single_for_cpu(struct 
device *dev,
arch_sync_dma_for_cpu_all();
}
 
-   if (unlikely(is_swiotlb_buffer(paddr)))
+   if (unlikely(is_swiotlb_buffer(dev, paddr)))
swiotlb_tbl_sync_single(dev, paddr, size, dir, SYNC_FOR_CPU);
 }
 
@@ -162,7 +162,7 @@ static inline dma_addr_t dma_direct_map_page(struct device 
*dev,
phys_addr_t phys = page_to_phys(page) + offset;
dma_addr_t dma_addr = phys_to_dma(dev, phys);
 
-   if (unlikely(swiotlb_force == SWIOTLB_FORCE))
+   if (unlikely(swiotlb_force == SWIOTLB_FORCE || dev->dma_io_tlb_mem))
return swiotlb_map(dev, phys, size, dir, attrs);
 
if (unlikely(!dma_capable(dev, dma_addr, size, true))) {
@@ -188,7 +188,7 @@ static inline void dma_direct_unmap_page(struct device 
*dev, dma_addr_t addr,
if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
dma_direct_sync_single_for_cpu(dev, addr, size, dir);
 
-   if (unlikely(is_swiotlb_buffer(phys)))
+   if (unlikely(is_swiotlb_buffer(dev, phys)))
swiotlb_tbl_unmap_single(dev, phys, size, size, dir, attrs);
 }
 #endif /* _LINUX_DMA_DIRECT_H */
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index ab0d571d0826..8a50b3af7c3f 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -105,18 +105,21 @@ struct io_tlb_mem {
 };
 extern struct io_tlb_mem io_tlb_default_mem;
 
-static inline bool is_swiotlb_buffer(phys_addr_t paddr)
+static inline bool is_swiotlb_buffer(struct device *dev, phys_addr_t paddr)
 {
-   return paddr >= io_tlb_mem.start && paddr < io_tlb_mem.end;
+   struct io_tlb_mem *mem =
+   dev->dma_io_tlb_mem ? dev->dma_io_tlb_mem : _tlb_default_mem;
+
+   return paddr >= mem->start && paddr < mem->end;
 }
 
 void __init swiotlb_exit(void);
 unsigned int swiotlb_max_segment(void);
 size_t swiotlb_max_mapping_size(struct device *dev);
-bool is_swiotlb_active(void);
+bool is_swiotlb_active(struct device *dev);
 #else
 #define swiotlb_force SWIOTLB_NO_FORCE
-static inline bool is_swiotlb_buffer(phys_addr_t paddr)
+static inline bool is_swiotlb_buffer(struct device *dev, phys_addr_t paddr)
 {
return false;
 }
@@ -132,7 +135,7 @@ static inline size_t swiotlb_max_mapping_size(struct device 
*dev)
return SIZE_MAX;
 }
 
-static

[RFC v2 0/5] Restricted DMA

2020-07-27 Thread Claire Chang
This series implements mitigations for lack of DMA access control on
systems without an IOMMU, which could result in the DMA accessing the
system memory at unexpected times and/or unexpected addresses, possibly
leading to data leakage or corruption.

For example, we plan to use the PCI-e bus for Wi-Fi on one MTK platform and
that PCI-e bus is not behind an IOMMU. As PCI-e, by design, gives the
device full access to system memory, a vulnerability in the Wi-Fi firmware
could easily escalate to a full system exploit (remote wifi exploits: [1a],
[1b] that shows a full chain of exploits; [2], [3]).

To mitigate the security concerns, we introduce restricted DMA. The
restricted DMA is implemented by per-device swiotlb and coherent memory
pools. The feature on its own provides a basic level of protection against
the DMA overwriting buffer contents at unexpected times. However, to
protect against general data leakage and system memory corruption, the
system needs to provide a way to restrict the DMA to a predefined memory
region (this is usually done at firmware level, e.g. in ATF on some ARM
platforms).

[1a] 
https://googleprojectzero.blogspot.com/2017/04/over-air-exploiting-broadcoms-wi-fi_4.html
[1b] 
https://googleprojectzero.blogspot.com/2017/04/over-air-exploiting-broadcoms-wi-fi_11.html
[2] https://blade.tencent.com/en/advisories/qualpwn/
[3] 
https://www.bleepingcomputer.com/news/security/vulnerabilities-found-in-highly-popular-firmware-for-wifi-chips/


Claire Chang (5):
  swiotlb: Add io_tlb_mem struct
  swiotlb: Add device swiotlb pool
  swiotlb: Use device swiotlb pool if available
  dt-bindings: of: Add plumbing for restricted DMA pool
  of: Add plumbing for restricted DMA pool

 .../reserved-memory/reserved-memory.txt   |  35 ++
 drivers/iommu/intel/iommu.c   |   8 +-
 drivers/of/address.c  |  39 ++
 drivers/of/device.c   |   3 +
 drivers/of/of_private.h   |   6 +
 drivers/xen/swiotlb-xen.c |   4 +-
 include/linux/device.h|   4 +
 include/linux/dma-direct.h|   8 +-
 include/linux/swiotlb.h   |  49 +-
 kernel/dma/direct.c   |   8 +-
 kernel/dma/swiotlb.c  | 418 +++---
 11 files changed, 393 insertions(+), 189 deletions(-)

--
v1: https://lore.kernel.org/patchwork/cover/1271660/
Changes in v2:
- build on top of swiotlb
 
2.28.0.rc0.142.g3c755180ce-goog



[RFC v2 1/5] swiotlb: Add io_tlb_mem struct

2020-07-27 Thread Claire Chang
Added a new struct, io_tlb_mem, as the IO TLB memory pool descriptor and
moved relevant global variables into that struct.
This will be useful later to allow for per-device swiotlb regions.

Signed-off-by: Claire Chang 
---
 drivers/iommu/intel/iommu.c |   2 +-
 drivers/xen/swiotlb-xen.c   |   4 +-
 include/linux/swiotlb.h |  38 -
 kernel/dma/swiotlb.c| 286 +---
 4 files changed, 172 insertions(+), 158 deletions(-)

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 3f7c04cf89b3..44c9230251eb 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -3736,7 +3736,7 @@ bounce_map_single(struct device *dev, phys_addr_t paddr, 
size_t size,
 */
if (!IS_ALIGNED(paddr | size, VTD_PAGE_SIZE)) {
tlb_addr = swiotlb_tbl_map_single(dev,
-   __phys_to_dma(dev, io_tlb_start),
+   __phys_to_dma(dev, io_tlb_default_mem.start),
paddr, size, aligned_size, dir, attrs);
if (tlb_addr == DMA_MAPPING_ERROR) {
goto swiotlb_error;
diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index b6d27762c6f8..62452424ec8a 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -190,8 +190,8 @@ int __ref xen_swiotlb_init(int verbose, bool early)
/*
 * IO TLB memory already allocated. Just use it.
 */
-   if (io_tlb_start != 0) {
-   xen_io_tlb_start = phys_to_virt(io_tlb_start);
+   if (io_tlb_default_mem.start != 0) {
+   xen_io_tlb_start = phys_to_virt(io_tlb_default_mem.start);
goto end;
}
 
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index 046bb94bd4d6..ab0d571d0826 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -69,11 +69,45 @@ dma_addr_t swiotlb_map(struct device *dev, phys_addr_t phys,
 
 #ifdef CONFIG_SWIOTLB
 extern enum swiotlb_force swiotlb_force;
-extern phys_addr_t io_tlb_start, io_tlb_end;
+
+/**
+ * struct io_tlb_mem - IO TLB Memory Pool Descriptor
+ *
+ * @start: The start address of the swiotlb memory pool. Used to do a quick
+ * range check to see if the memory was in fact allocated by this
+ * API. For device private swiotlb, this is device tree adjustable.
+ * @end:   The end address of the swiotlb memory pool. Used to do a quick
+ * range check to see if the memory was in fact allocated by this
+ * API. For device private swiotlb, this is device tree adjustable.
+ * @nslabs:The number of IO TLB blocks (in groups of 64) between @start and
+ * @end. For system swiotlb, this is command line adjustable via
+ * setup_io_tlb_npages.
+ * @used:  The number of used IO TLB block.
+ * @list:  The free list describing the number of free entries available
+ * from each index.
+ * @index: The index to start searching in the next round.
+ * @orig_addr: The original address corresponding to a mapped entry for the
+ * sync operations.
+ * @lock:  The lock to protect the above data structures in the map and
+ * unmap calls.
+ * @debugfs:   The dentry to debugfs.
+ */
+struct io_tlb_mem {
+   phys_addr_t start;
+   phys_addr_t end;
+   unsigned long nslabs;
+   unsigned long used;
+   unsigned int *list;
+   unsigned int index;
+   phys_addr_t *orig_addr;
+   spinlock_t lock;
+   struct dentry *debugfs;
+};
+extern struct io_tlb_mem io_tlb_default_mem;
 
 static inline bool is_swiotlb_buffer(phys_addr_t paddr)
 {
-   return paddr >= io_tlb_start && paddr < io_tlb_end;
+   return paddr >= io_tlb_mem.start && paddr < io_tlb_mem.end;
 }
 
 void __init swiotlb_exit(void);
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index c19379fabd20..f83911fa14ce 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -61,33 +61,11 @@
  * allocate a contiguous 1MB, we're probably in trouble anyway.
  */
 #define IO_TLB_MIN_SLABS ((1<<20) >> IO_TLB_SHIFT)
+#define INVALID_PHYS_ADDR (~(phys_addr_t)0)
 
 enum swiotlb_force swiotlb_force;
 
-/*
- * Used to do a quick range check in swiotlb_tbl_unmap_single and
- * swiotlb_tbl_sync_single_*, to see if the memory was in fact allocated by 
this
- * API.
- */
-phys_addr_t io_tlb_start, io_tlb_end;
-
-/*
- * The number of IO TLB blocks (in groups of 64) between io_tlb_start and
- * io_tlb_end.  This is command line adjustable via setup_io_tlb_npages.
- */
-static unsigned long io_tlb_nslabs;
-
-/*
- * The number of used IO TLB block
- */
-static unsigned long io_tlb_used;
-
-/*
- * This is a free list describing the number of free entries available from
- * each index
- */
-static unsigned int *io_tlb_list;
-static unsigned int io_tlb_index;
+struct io_tlb_mem io_tlb_defaul

[RFC v2 2/5] swiotlb: Add device swiotlb pool

2020-07-27 Thread Claire Chang
Add the initialization function to create device swiotlb pools from
matching reserved-memory nodes in the device tree.

Signed-off-by: Claire Chang 
---
 include/linux/device.h |   4 ++
 kernel/dma/swiotlb.c   | 148 +
 2 files changed, 126 insertions(+), 26 deletions(-)

diff --git a/include/linux/device.h b/include/linux/device.h
index 79ce404619e6..f40f711e43e9 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -575,6 +575,10 @@ struct device {
struct cma *cma_area;   /* contiguous memory area for dma
   allocations */
 #endif
+#ifdef CONFIG_SWIOTLB
+   struct io_tlb_mem   *dma_io_tlb_mem;
+#endif
+
/* arch specific additions */
struct dev_archdata archdata;
 
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index f83911fa14ce..eaa101b3e75b 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -36,6 +36,10 @@
 #include 
 #include 
 #include 
+#include 
+#include 
+#include 
+#include 
 #ifdef CONFIG_DEBUG_FS
 #include 
 #endif
@@ -298,20 +302,14 @@ static void swiotlb_cleanup(void)
max_segment = 0;
 }
 
-int
-swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs)
+static void swiotlb_init_io_tlb_mem(struct io_tlb_mem *mem, phys_addr_t start,
+   size_t size)
 {
-   struct io_tlb_mem *mem = _tlb_default_mem;
-   unsigned long i, bytes;
-
-   bytes = nslabs << IO_TLB_SHIFT;
+   unsigned long i;
 
-   mem->nslabs = nslabs;
-   mem->start = virt_to_phys(tlb);
-   mem->end = mem->start + bytes;
-
-   set_memory_decrypted((unsigned long)tlb, bytes >> PAGE_SHIFT);
-   memset(tlb, 0, bytes);
+   mem->nslabs = size >> IO_TLB_SHIFT;
+   mem->start = start;
+   mem->end = mem->start + size;
 
/*
 * Allocate and initialize the free list array.  This array is used
@@ -336,11 +334,6 @@ swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs)
}
mem->index = 0;
 
-   swiotlb_print_info();
-
-   late_alloc = 1;
-
-   swiotlb_set_max_segment(mem->nslabs << IO_TLB_SHIFT);
spin_lock_init(>lock);
 
return 0;
@@ -354,6 +347,38 @@ swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs)
return -ENOMEM;
 }
 
+int swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs)
+{
+   struct io_tlb_mem *mem = _tlb_default_mem;
+   unsigned long bytes;
+   int ret;
+
+   bytes = nslabs << IO_TLB_SHIFT;
+
+   set_memory_decrypted((unsigned long)tlb, bytes >> PAGE_SHIFT);
+   memset(tlb, 0, bytes);
+
+   ret = swiotlb_init_io_tlb_mem(mem, virt_to_phys(tlb), bytes);
+   if (ret)
+   return ret;
+
+   swiotlb_print_info();
+
+   late_alloc = 1;
+
+   swiotlb_set_max_segment(mem->nslabs << IO_TLB_SHIFT);
+
+   return 0;
+}
+
+static void swiotlb_free_pages(struct io_tlb_mem *mem)
+{
+   free_pages((unsigned long)mem->orig_addr,
+  get_order(mem->nslabs * sizeof(phys_addr_t)));
+   free_pages((unsigned long)mem->list,
+  get_order(mem->nslabs * sizeof(int)));
+}
+
 void __init swiotlb_exit(void)
 {
struct io_tlb_mem *mem = _tlb_default_mem;
@@ -362,10 +387,7 @@ void __init swiotlb_exit(void)
return;
 
if (late_alloc) {
-   free_pages((unsigned long)mem->orig_addr,
-  get_order(mem->nslabs * sizeof(phys_addr_t)));
-   free_pages((unsigned long)mem->list, get_order(mem->nslabs *
-  sizeof(int)));
+   swiotlb_free_pages(mem);
free_pages((unsigned long)phys_to_virt(mem->start),
   get_order(mem->nslabs << IO_TLB_SHIFT));
} else {
@@ -687,16 +709,90 @@ bool is_swiotlb_active(void)
 
 #ifdef CONFIG_DEBUG_FS
 
-static int __init swiotlb_create_debugfs(void)
+static void swiotlb_create_debugfs(struct io_tlb_mem *mem, const char *name,
+  struct dentry *node)
 {
-   struct io_tlb_mem *mem = _tlb_default_mem;
-
-   mem->debugfs = debugfs_create_dir("swiotlb", NULL);
+   mem->debugfs = debugfs_create_dir(name, node);
debugfs_create_ulong("io_tlb_nslabs", 0400, mem->debugfs, >nslabs);
debugfs_create_ulong("io_tlb_used", 0400, mem->debugfs, >used);
+}
+
+static int __init swiotlb_create_default_debugfs(void)
+{
+   swiotlb_create_debugfs(_tlb_default_mem, "swiotlb", NULL);
+
return 0;
 }
 
-late_initcall(swiotlb_create_debugfs);
+late_initcall(swiotlb_create_default_debugfs);
 
 #endif
+
+static int device_swiotlb_init(struct reserved_mem *rmem,
+  struct de

Re: [PATCH 1/4] dma-mapping: Add bounced DMA ops

2020-07-15 Thread Claire Chang
On Wed, Jul 15, 2020 at 11:46 AM Claire Chang  wrote:
>
> On Tue, Jul 14, 2020 at 7:01 PM Christoph Hellwig  wrote:
> >
> > On Mon, Jul 13, 2020 at 12:55:43PM +0100, Robin Murphy wrote:
> > > On 2020-07-13 10:12, Claire Chang wrote:
> > >> The bounced DMA ops provide an implementation of DMA ops that bounce
> > >> streaming DMA in and out of a specially allocated region. Only the
> > >> operations relevant to streaming DMA are supported.
> > >
> > > I think there are too many implicit assumptions here - apparently that
> > > coherent allocations will always be intercepted by
> > > dma_*_from_dev_coherent(), and that calling into dma-direct won't actually
> > > bounce things a second time beyond where you thought they were going,
> > > manage coherency for a different address, and make it all go subtly wrong.
> > > Consider "swiotlb=force", for instance...
If I understand it correctly, reusing SWIOTLB won't prevent the
coherent allocations
from always being intercepted by dma_*_from_dev_coherent(), right?
Since we can't bounce the coherent memory, we still need to rely on
dma_*_from_dev_coherent() and a reserved-memory region for coherent DMA to
restrict the device DMA access.

As for calling into dma-direct, in this version, I use set_dma_ops to set the
dma_bounced_ops, so I just bypass dma-direct and SWIOTLB. "swiotlb=force"
won't bounce things a second time and the data will still be bounced
to the region
set in dts.
Besides, I did a quick search and found that only two *-iommu.c directly use
dma_direct_map_page.
https://elixir.bootlin.com/linux/latest/C/ident/dma_direct_map_page
Since bounced DMA is to mitigate the lack of DMA access control on systems
without an IOMMU (see patch#4, only call of_dma_set_bounce_buffer for the
devices not behind an IOMMU), can we assume no one will use dma-direct?
(I understand that if we build bounced DMA on top of SWIOTLB, we don't need
to worry about this.)

> > >
> > > Again, plumbing this straight into dma-direct so that SWIOTLB can simply
> > > target a different buffer and always bounce regardless of masks would seem
> > > a far better option.
> >
> > I haven't really had time to read through the details, but I agree that
> > any bouncing scheme should reuse the swiotlb code and not invent a
> > parallel infrastructure.
> Thanks for the feedback. I'll try to reuse SWIOTLB.
My current plan is to first change the buffers management logic in SWIOTLB to
use gen_pool like this patch (i.e., gen_pool_dma_alloc, gen_pool_free, ect), and
then make SWIOTLB use the device's private pool for regular DMA to/from system
memory if possible.
Does this sound right?

Thanks!


Re: [PATCH] serial: 8250_mtk: Fix high-speed baud rates clamping

2020-07-14 Thread Claire Chang
On Wed, Jul 15, 2020 at 4:45 AM Daniel Winkler  wrote:
>
> Thank you Sergey for looking into this. Adding folks working on this
> platform to perform validation of the proposed patch.
>
> Best,
> Daniel
>
> On Tue, Jul 14, 2020 at 5:41 AM Serge Semin
>  wrote:
> >
> > Commit 7b668c064ec3 ("serial: 8250: Fix max baud limit in generic 8250
> > port") fixed limits of a baud rate setting for a generic 8250 port.
> > In other words since that commit the baud rate has been permitted to be
> > within [uartclk / 16 / UART_DIV_MAX; uartclk / 16], which is absolutely
> > normal for a standard 8250 UART port. But there are custom 8250 ports,
> > which provide extended baud rate limits. In particular the Mediatek 8250
> > port can work with baud rates up to "uartclk" speed.
> >
> > Normally that and any other peculiarity is supposed to be handled in a
> > custom set_termios() callback implemented in the vendor-specific
> > 8250-port glue-driver. Currently that is how it's done for the most of
> > the vendor-specific 8250 ports, but for some reason for Mediatek a
> > solution has been spread out to both the glue-driver and to the generic
> > 8250-port code. Due to that a bug has been introduced, which permitted the
> > extended baud rate limit for all even for standard 8250-ports. The bug
> > has been fixed by the commit 7b668c064ec3 ("serial: 8250: Fix max baud
> > limit in generic 8250 port") by narrowing the baud rates limit back down to
> > the normal bounds. Unfortunately by doing so we also broke the
> > Mediatek-specific extended bauds feature.
> >
> > A fix of the problem described above is twofold. First since we can't get
> > back the extended baud rate limits feature to the generic set_termios()
> > function and that method supports only a standard baud rates range, the
> > requested baud rate must be locally stored before calling it and then
> > restored back to the new termios structure after the generic set_termios()
> > finished its magic business. By doing so we still use the
> > serial8250_do_set_termios() method to set the LCR/MCR/FCR/etc. registers,
> > while the extended baud rate setting procedure will be performed later in
> > the custom Mediatek-specific set_termios() callback. Second since a true
> > baud rate is now fully calculated in the custom set_termios() method we
> > need to locally update the port timeout by calling the
> > uart_update_timeout() function. After the fixes described above are
> > implemented in the 8250_mtk.c driver, the Mediatek 8250-port should
> > get back to normally working with extended baud rates.
> >
> > Link: 
> > https://lore.kernel.org/linux-serial/20200701211337.3027448-1-danielwink...@google.com
> >
> > Fixes: 7b668c064ec3 ("serial: 8250: Fix max baud limit in generic 8250 
> > port")
> > Reported-by: Daniel Winkler 
> > Signed-off-by: Serge Semin 
Tested-by: Claire Chang 
> >
> > ---
> >
> > Folks, sorry for a delay with the problem fix. A solution is turned out to
> > be a bit more complicated than I originally thought in my comment to the
> > Daniel revert-patch.
> >
> > Please also note, that I don't have a Mediatek hardware to test the
> > solution suggested in the patch. The code is written as on so called
> > the tip of the pen after digging into the 8250_mtk.c and 8250_port.c
> > drivers code. So please Daniel or someone with Mediatek 8250-port
> > available on a board test this patch first and report about the results in
> > reply to this emailing thread. After that, if your conclusion is positive
> > and there is no objection against the solution design the patch can be
> > merged in.
I tested it with mt8183 + QCA6174.
The UART Bluetooth works fine with this fix.
Thanks!
> >
> > Cc: Alexey Malahov 
> > Cc: Daniel Winkler 
> > Cc: Aaron Sierra 
> > Cc: Andy Shevchenko 
> > Cc: Lukas Wunner 
> > Cc: Vignesh Raghavendra 
> > Cc: linux-ser...@vger.kernel.org
> > Cc: linux-media...@lists.infradead.org
> > Cc: BlueZ 
> > Cc: chromeos-bluetooth-upstreaming 
> > 
> > Cc: abhishekpan...@chromium.org
> > Cc: sta...@vger.kernel.org
> > ---
> >  drivers/tty/serial/8250/8250_mtk.c | 18 ++
> >  1 file changed, 18 insertions(+)
> >
> > diff --git a/drivers/tty/serial/8250/8250_mtk.c 
> > b/drivers/tty/serial/8250/8250_mtk.c
> > index f839380c2f4c..98b8a3e30733 100644
> > --- a/drivers/tty/serial/8250/8250_mtk.c
> > +++ b/drivers/tty/serial/8250/8250_mtk.c
> > @@ -306,8 +306,21 @@ mtk8250_set_term

Re: [PATCH 1/4] dma-mapping: Add bounced DMA ops

2020-07-14 Thread Claire Chang
On Tue, Jul 14, 2020 at 7:01 PM Christoph Hellwig  wrote:
>
> On Mon, Jul 13, 2020 at 12:55:43PM +0100, Robin Murphy wrote:
> > On 2020-07-13 10:12, Claire Chang wrote:
> >> The bounced DMA ops provide an implementation of DMA ops that bounce
> >> streaming DMA in and out of a specially allocated region. Only the
> >> operations relevant to streaming DMA are supported.
> >
> > I think there are too many implicit assumptions here - apparently that
> > coherent allocations will always be intercepted by
> > dma_*_from_dev_coherent(), and that calling into dma-direct won't actually
> > bounce things a second time beyond where you thought they were going,
> > manage coherency for a different address, and make it all go subtly wrong.
> > Consider "swiotlb=force", for instance...
> >
> > Again, plumbing this straight into dma-direct so that SWIOTLB can simply
> > target a different buffer and always bounce regardless of masks would seem
> > a far better option.
>
> I haven't really had time to read through the details, but I agree that
> any bouncing scheme should reuse the swiotlb code and not invent a
> parallel infrastructure.
Thanks for the feedback. I'll try to reuse SWIOTLB.


Re: [PATCH 0/4] Bounced DMA support

2020-07-14 Thread Claire Chang
On Mon, Jul 13, 2020 at 7:40 PM Robin Murphy  wrote:
>
> On 2020-07-13 10:12, Claire Chang wrote:
> > This series implements mitigations for lack of DMA access control on
> > systems without an IOMMU, which could result in the DMA accessing the
> > system memory at unexpected times and/or unexpected addresses, possibly
> > leading to data leakage or corruption.
> >
> > For example, we plan to use the PCI-e bus for Wi-Fi and that PCI-e bus
> > is not behind an IOMMU. As PCI-e, by design, gives the device full
> > access to system memory, a vulnerability in the Wi-Fi firmware could
> > easily escalate to a full system exploit (remote wifi exploits: [1a],
> > [1b] that shows a full chain of exploits; [2], [3]).
> >
> > To mitigate the security concerns, we introduce bounced DMA. The bounced
> > DMA ops provide an implementation of DMA ops that bounce streaming DMA
> > in and out of a specially allocated region. The feature on its own
> > provides a basic level of protection against the DMA overwriting buffer
> > contents at unexpected times. However, to protect against general data
> > leakage and system memory corruption, the system needs to provide a way
> > to restrict the DMA to a predefined memory region (this is usually done
> > at firmware level, e.g. in ATF on some ARM platforms).
>
> More to the point, this seems to need some fairly special interconnect
> hardware too. On typical systems that just stick a TZASC directly in
> front of the memory controller it would be hard to block DMA access
> without also blocking CPU access. With something like Arm TZC-400 I
> guess you could set up a "secure" region for most of DRAM that allows NS
> accesses by NSAID from the CPUs, then similar regions for the pools with
> NSAID access for both the respective device and the CPUs, but by that
> point you've probably used up most of the available regions before even
> considering what the firmware and TEE might want for actual Secure memory.
>
> In short, I don't foresee this being used by very many systems.
We're going to use this on MTK SoC with MPU (memory protection unit) to
restrict the DMA access for PCI-e Wi-Fi.
>
> That said,, although the motivation is different, it appears to end up
> being almost exactly the same end result as the POWER secure
> virtualisation thingy (essentially: constrain DMA to a specific portion
> of RAM). The more code can be shared with that, the better.
Could you share a bit more about the POWER secure virtualisation thingy?
>
> > Currently, 32-bit architectures are not supported because of the need to
> > handle HIGHMEM, which increases code complexity and adds more
> > performance penalty for such platforms. Also, bounced DMA can not be
> > enabled on devices behind an IOMMU, as those require an IOMMU-aware
> > implementation of DMA ops and do not require this kind of mitigation
> > anyway.
>
> Note that we do actually have the notion of bounced DMA with IOMMUs
> already (to avoid leakage of unrelated data in the same page). I think
> it's only implemented for intel-iommu so far, but shouldn't take much
> work to generalise to iommu-dma if anyone wanted to. That's already done
> a bunch of work to generalise the SWIOTLB routines to be more reusable,
> so building on top of that would be highly preferable.
Yes, I'm aware of that and I'll try to put this on top of SWIOTLB.
>
> Thirdly, the concept of device-private bounce buffers does in fact
> already exist to some degree too - there are various USB, crypto and
> other devices that can only DMA to a local SRAM buffer (not to mention
> subsystems doing their own bouncing for the sake of alignment/block
> merging/etc.). Again, a slightly more generalised solution that makes
> this a first-class notion for dma-direct itself and could help supplant
> some of those hacks would be really really good.
>
> Robin.
>
> > [1a] 
> > https://googleprojectzero.blogspot.com/2017/04/over-air-exploiting-broadcoms-wi-fi_4.html
> > [1b] 
> > https://googleprojectzero.blogspot.com/2017/04/over-air-exploiting-broadcoms-wi-fi_11.html
> > [2] https://blade.tencent.com/en/advisories/qualpwn/
> > [3] 
> > https://www.bleepingcomputer.com/news/security/vulnerabilities-found-in-highly-popular-firmware-for-wifi-chips/
> >
> >
> > Claire Chang (4):
> >dma-mapping: Add bounced DMA ops
> >dma-mapping: Add bounced DMA pool
> >dt-bindings: of: Add plumbing for bounced DMA pool
> >of: Add plumbing for bounced DMA pool
> >
> >   .../reserved-memory/reserved-memory.txt   |  36 +++
> >   drivers/of/address.c  |  37 +++
> > 

[PATCH 4/4] of: Add plumbing for bounced DMA pool

2020-07-13 Thread Claire Chang
If a device is not behind an IOMMU, we look up the device node and set
up the bounced DMA when the bounced-dma property is presented. One can
specify two reserved-memory nodes in the device tree. One with
shared-dma-pool to handle the coherent DMA buffer allocation, and
another one with bounced-dma-pool for regular DMA to/from system memory,
which would be subject to bouncing.

Signed-off-by: Claire Chang 
---
 drivers/of/address.c| 37 +
 drivers/of/device.c |  3 +++
 drivers/of/of_private.h |  6 ++
 3 files changed, 46 insertions(+)

diff --git a/drivers/of/address.c b/drivers/of/address.c
index 8eea3f6e29a4..a767b80f8862 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -8,6 +8,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -1009,6 +1010,42 @@ int of_dma_get_range(struct device_node *np, u64 
*dma_addr, u64 *paddr, u64 *siz
return ret;
 }
 
+int of_dma_set_bounce_buffer(struct device *dev)
+{
+   int length, size, ret, i;
+   u32 idx[2];
+
+   if (!dev || !dev->of_node)
+   return -EINVAL;
+
+   if (!of_get_property(dev->of_node, "bounced-dma", ))
+   return 0;
+
+   size = length / sizeof(idx[0]);
+   if (size > ARRAY_SIZE(idx)) {
+   dev_err(dev,
+   "bounced-dma expected less than or equal to 2 indexs, 
but got %d\n",
+   size);
+   return -EINVAL;
+   }
+
+   ret = of_property_read_u32_array(dev->of_node, "bounced-dma", idx,
+size);
+
+   for (i = 0; i < size; i++) {
+   ret = of_reserved_mem_device_init_by_idx(dev, dev->of_node,
+idx[i]);
+   if (ret) {
+   dev_err(dev,
+   "of_reserved_mem_device_init_by_idx() failed 
with %d\n",
+   ret);
+   return ret;
+   }
+   }
+
+   return 0;
+}
+
 /**
  * of_dma_is_coherent - Check if device is coherent
  * @np:device node
diff --git a/drivers/of/device.c b/drivers/of/device.c
index 27203bfd0b22..238beef48a50 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -169,6 +169,9 @@ int of_dma_configure(struct device *dev, struct device_node 
*np, bool force_dma)
 
arch_setup_dma_ops(dev, dma_addr, size, iommu, coherent);
 
+   if (!iommu)
+   return of_dma_set_bounce_buffer(dev);
+
return 0;
 }
 EXPORT_SYMBOL_GPL(of_dma_configure);
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index edc682249c00..3d1b8cca1519 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -160,12 +160,18 @@ extern int of_bus_n_size_cells(struct device_node *np);
 #ifdef CONFIG_OF_ADDRESS
 extern int of_dma_get_range(struct device_node *np, u64 *dma_addr,
u64 *paddr, u64 *size);
+extern int of_dma_set_bounce_buffer(struct device *dev);
 #else
 static inline int of_dma_get_range(struct device_node *np, u64 *dma_addr,
   u64 *paddr, u64 *size)
 {
return -ENODEV;
 }
+
+static inline int of_dma_get_bounce_buffer(struct device *dev)
+{
+   return -ENODEV;
+}
 #endif
 
 #endif /* _LINUX_OF_PRIVATE_H */
-- 
2.27.0.383.g050319c2ae-goog



[PATCH 3/4] dt-bindings: of: Add plumbing for bounced DMA pool

2020-07-13 Thread Claire Chang
Introduce the new compatible string, bounced-dma-pool, for bounced DMA.
One can specify the address and length of the bounced memory region by
bounced-dma-pool in the device tree.

Signed-off-by: Claire Chang 
---
 .../reserved-memory/reserved-memory.txt   | 36 +++
 1 file changed, 36 insertions(+)

diff --git 
a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt 
b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
index 4dd20de6977f..45b3134193ea 100644
--- a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
+++ b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
@@ -51,6 +51,24 @@ compatible (optional) - standard definition
   used as a shared pool of DMA buffers for a set of devices. It can
   be used by an operating system to instantiate the necessary pool
   management subsystem if necessary.
+- bounced-dma-pool: This indicates a region of memory meant to be used
+  as a pool of DMA bounce buffers for a given device. When using this,
+  the no-map and reusable properties must not be set, so the operating
+  system can create a virtual mapping that will be used for
+  synchronization. Also, there must be a bounced-dma property in the
+  device node to specify the indexes of reserved-memory nodes. One can
+  specify two reserved-memory nodes in the device tree. One with
+  shared-dma-pool to handle the coherent DMA buffer allocation, and
+  another one with bounced-dma-pool for regular DMA to/from system
+  memory, which would be subject to bouncing. The main purpose for
+  bounced DMA is to mitigate the lack of DMA access control on systems
+  without an IOMMU, which could result in the DMA accessing the system
+  memory at unexpected times and/or unexpected addresses, possibly
+  leading to data leakage or corruption. The feature on its own 
provides
+  a basic level of protection against the DMA overwriting buffer
+  contents at unexpected times. However, to protect against general 
data
+  leakage and system memory corruption, the system needs to provide a
+  way to restrict the DMA to a predefined memory region.
 - vendor specific string in the form ,[-]
 no-map (optional) - empty property
 - Indicates the operating system must not create a virtual mapping
@@ -117,6 +135,17 @@ one for multimedia processing (named 
multimedia-memory@7700, 64MiB).
compatible = "acme,multimedia-memory";
reg = <0x7700 0x400>;
};
+
+   wifi_bounced_dma_mem_region: wifi_bounced_dma_mem_region {
+   compatible = "bounced-dma-pool";
+   reg = <0x5000 0x400>;
+   };
+
+   wifi_coherent_mem_region: wifi_coherent_mem_region {
+   compatible = "shared-dma-pool";
+   reg = <0x5400 0x40>;
+   };
+
};
 
/* ... */
@@ -135,4 +164,11 @@ one for multimedia processing (named 
multimedia-memory@7700, 64MiB).
memory-region = <_reserved>;
/* ... */
};
+
+   pcie_wifi: pcie_wifi@0,0 {
+   memory-region = <_bounced_dma_mem_region>,
+<_coherent_mem_region>;
+   bounced-dma = <0>, <1>;
+   /* ... */
+   };
 };
-- 
2.27.0.383.g050319c2ae-goog



[PATCH 2/4] dma-mapping: Add bounced DMA pool

2020-07-13 Thread Claire Chang
Add the initialization function to create bounce buffer pools from
matching reserved-memory nodes in the device tree.

The bounce buffer pools provide a basic level of protection against
the DMA overwriting buffer contents at unexpected times. However, to
protect against general data leakage and system memory corruption, the
system needs to provide a way to restrict the DMA to a predefined memory
region.

Signed-off-by: Claire Chang 
---
 kernel/dma/bounced.c | 89 
 1 file changed, 89 insertions(+)

diff --git a/kernel/dma/bounced.c b/kernel/dma/bounced.c
index fcaabb5eccf2..0bfd6cf90aee 100644
--- a/kernel/dma/bounced.c
+++ b/kernel/dma/bounced.c
@@ -12,6 +12,9 @@
 #include 
 #include 
 #include 
+#include 
+#include 
+#include 
 #include 
 
 struct dma_bounced_mem {
@@ -213,3 +216,89 @@ const struct dma_map_ops dma_bounced_ops = {
.max_mapping_size   = dma_bounced_max_mapping_size,
.get_merge_boundary = NULL,
 };
+
+static int dma_bounced_device_init(struct reserved_mem *rmem,
+  struct device *dev)
+{
+   struct dma_bounced_mem *mem;
+   int ret;
+
+   mem = kzalloc(sizeof(*mem), GFP_KERNEL);
+   if (!mem)
+   return -ENOMEM;
+
+   mem->virt_base =
+   devm_memremap(dev, rmem->base, rmem->size, MEMREMAP_WB);
+   if (!mem->virt_base) {
+   ret = -EINVAL;
+   goto error;
+   }
+
+   mem->size = rmem->size;
+   mem->device_base = phys_to_dma(dev, rmem->base);
+   mem->device_end = mem->device_base + rmem->size;
+
+   mem->orig_addr = kcalloc(mem->size >> PAGE_SHIFT,
+sizeof(*mem->orig_addr), GFP_KERNEL);
+   if (!mem->orig_addr) {
+   ret = -ENOMEM;
+   goto error;
+   }
+
+   mem->pool = devm_gen_pool_create(dev, PAGE_SHIFT, NUMA_NO_NODE,
+"bounced DMA");
+   if (!mem->pool) {
+   ret = -ENOMEM;
+   goto error;
+   }
+
+   ret = gen_pool_add_virt(mem->pool, (unsigned long)mem->virt_base,
+   rmem->base, rmem->size, NUMA_NO_NODE);
+   if (ret)
+   goto error;
+
+   dev->dma_bounced_mem = mem;
+   set_dma_ops(dev, _bounced_ops);
+
+   return 0;
+
+error:
+   kfree(mem);
+
+   return ret;
+}
+
+static void dma_bounced_device_release(struct reserved_mem *rmem,
+  struct device *dev)
+{
+   struct dma_bounced_mem *mem = dev->dma_bounced_mem;
+
+   set_dma_ops(dev, NULL);
+   dev->dma_bounced_mem = NULL;
+
+   kfree(mem->orig_addr);
+   kfree(mem);
+}
+
+static const struct reserved_mem_ops rmem_dma_bounced_ops = {
+   .device_init= dma_bounced_device_init,
+   .device_release = dma_bounced_device_release,
+};
+
+static int __init dma_bounced_setup(struct reserved_mem *rmem)
+{
+   unsigned long node = rmem->fdt_node;
+
+   if (of_get_flat_dt_prop(node, "reusable", NULL) ||
+   of_get_flat_dt_prop(node, "linux,cma-default", NULL) ||
+   of_get_flat_dt_prop(node, "linux,dma-default", NULL) ||
+   of_get_flat_dt_prop(node, "no-map", NULL))
+   return -EINVAL;
+
+   rmem->ops = _dma_bounced_ops;
+   pr_info("Reserved memory: created DMA bounced memory pool at %pa, size 
%ld MiB\n",
+   >base, (unsigned long)rmem->size / SZ_1M);
+   return 0;
+}
+
+RESERVEDMEM_OF_DECLARE(dma, "bounced-dma-pool", dma_bounced_setup);
-- 
2.27.0.383.g050319c2ae-goog



[PATCH 0/4] Bounced DMA support

2020-07-13 Thread Claire Chang
This series implements mitigations for lack of DMA access control on
systems without an IOMMU, which could result in the DMA accessing the
system memory at unexpected times and/or unexpected addresses, possibly
leading to data leakage or corruption.

For example, we plan to use the PCI-e bus for Wi-Fi and that PCI-e bus
is not behind an IOMMU. As PCI-e, by design, gives the device full
access to system memory, a vulnerability in the Wi-Fi firmware could
easily escalate to a full system exploit (remote wifi exploits: [1a],
[1b] that shows a full chain of exploits; [2], [3]).

To mitigate the security concerns, we introduce bounced DMA. The bounced
DMA ops provide an implementation of DMA ops that bounce streaming DMA
in and out of a specially allocated region. The feature on its own
provides a basic level of protection against the DMA overwriting buffer
contents at unexpected times. However, to protect against general data
leakage and system memory corruption, the system needs to provide a way
to restrict the DMA to a predefined memory region (this is usually done
at firmware level, e.g. in ATF on some ARM platforms).

Currently, 32-bit architectures are not supported because of the need to
handle HIGHMEM, which increases code complexity and adds more
performance penalty for such platforms. Also, bounced DMA can not be
enabled on devices behind an IOMMU, as those require an IOMMU-aware
implementation of DMA ops and do not require this kind of mitigation
anyway.

[1a] 
https://googleprojectzero.blogspot.com/2017/04/over-air-exploiting-broadcoms-wi-fi_4.html
[1b] 
https://googleprojectzero.blogspot.com/2017/04/over-air-exploiting-broadcoms-wi-fi_11.html
[2] https://blade.tencent.com/en/advisories/qualpwn/
[3] 
https://www.bleepingcomputer.com/news/security/vulnerabilities-found-in-highly-popular-firmware-for-wifi-chips/


Claire Chang (4):
  dma-mapping: Add bounced DMA ops
  dma-mapping: Add bounced DMA pool
  dt-bindings: of: Add plumbing for bounced DMA pool
  of: Add plumbing for bounced DMA pool

 .../reserved-memory/reserved-memory.txt   |  36 +++
 drivers/of/address.c  |  37 +++
 drivers/of/device.c   |   3 +
 drivers/of/of_private.h   |   6 +
 include/linux/device.h|   3 +
 include/linux/dma-mapping.h   |   1 +
 kernel/dma/Kconfig|  17 +
 kernel/dma/Makefile   |   1 +
 kernel/dma/bounced.c  | 304 ++
 9 files changed, 408 insertions(+)
 create mode 100644 kernel/dma/bounced.c

-- 
2.27.0.383.g050319c2ae-goog



[PATCH 1/4] dma-mapping: Add bounced DMA ops

2020-07-13 Thread Claire Chang
The bounced DMA ops provide an implementation of DMA ops that bounce
streaming DMA in and out of a specially allocated region. Only the
operations relevant to streaming DMA are supported.

Signed-off-by: Claire Chang 
---
 include/linux/device.h  |   3 +
 include/linux/dma-mapping.h |   1 +
 kernel/dma/Kconfig  |  17 +++
 kernel/dma/Makefile |   1 +
 kernel/dma/bounced.c| 215 
 5 files changed, 237 insertions(+)
 create mode 100644 kernel/dma/bounced.c

diff --git a/include/linux/device.h b/include/linux/device.h
index 7322c51e9c0c..868b9a364003 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -588,6 +588,9 @@ struct device {
 
struct list_headdma_pools;  /* dma pools (if dma'ble) */
 
+#ifdef CONFIG_DMA_BOUNCED
+   struct dma_bounced_mem  *dma_bounced_mem;
+#endif
 #ifdef CONFIG_DMA_DECLARE_COHERENT
struct dma_coherent_mem *dma_mem; /* internal for coherent mem
 override */
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 2328f451a45d..86089424dafd 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -135,6 +135,7 @@ struct dma_map_ops {
 
 extern const struct dma_map_ops dma_virt_ops;
 extern const struct dma_map_ops dma_dummy_ops;
+extern const struct dma_map_ops dma_bounced_ops;
 
 #define DMA_BIT_MASK(n)(((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
 
diff --git a/kernel/dma/Kconfig b/kernel/dma/Kconfig
index 1da3f44f2565..148734c8748b 100644
--- a/kernel/dma/Kconfig
+++ b/kernel/dma/Kconfig
@@ -88,6 +88,23 @@ config DMA_DIRECT_REMAP
select DMA_REMAP
select DMA_COHERENT_POOL
 
+config DMA_BOUNCED
+   bool "DMA Bounced"
+   depends on !HIGHMEM
+   select OF_RESERVED_MEM
+   help
+ This enables support for bounced DMA pools which provide a level of
+ DMA memory protection on systems with limited hardware protection
+ capabilities, such as those lacking an IOMMU. It does so by bouncing
+ the data to a specially allocated DMA-accessible protected region
+ before mapping and unmapping. One can assign the protected memory
+ region in the device tree by using reserved-memory.
+
+ For more information see
+ 

+ and .
+ If unsure, say "n".
+
 config DMA_CMA
bool "DMA Contiguous Memory Allocator"
depends on HAVE_DMA_CONTIGUOUS && CMA
diff --git a/kernel/dma/Makefile b/kernel/dma/Makefile
index 370f63344e9c..f5fb4f42326a 100644
--- a/kernel/dma/Makefile
+++ b/kernel/dma/Makefile
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 
 obj-$(CONFIG_HAS_DMA)  += mapping.o direct.o dummy.o
+obj-$(CONFIG_DMA_BOUNCED)  += bounced.o
 obj-$(CONFIG_DMA_CMA)  += contiguous.o
 obj-$(CONFIG_DMA_DECLARE_COHERENT) += coherent.o
 obj-$(CONFIG_DMA_VIRT_OPS) += virt.o
diff --git a/kernel/dma/bounced.c b/kernel/dma/bounced.c
new file mode 100644
index ..fcaabb5eccf2
--- /dev/null
+++ b/kernel/dma/bounced.c
@@ -0,0 +1,215 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Bounced DMA support.
+ *
+ * This implements the mitigations for lack of IOMMU by bouncing the data to a
+ * specially allocated region before mapping and unmapping.
+ *
+ * Copyright 2020 Google LLC.
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct dma_bounced_mem {
+   void**orig_addr;
+   void*virt_base;
+   dma_addr_t  device_base;
+   dma_addr_t  device_end;
+   struct gen_pool *pool;
+   size_t  size;
+};
+
+static void dma_bounced_set_orig_virt(struct device *dev, dma_addr_t dma_addr,
+ void *orig_addr)
+{
+   struct dma_bounced_mem *mem = dev->dma_bounced_mem;
+   int idx = (dma_addr - mem->device_base) >> PAGE_SHIFT;
+
+   if (dma_addr < mem->device_base || dma_addr >= mem->device_end)
+   return;
+
+   mem->orig_addr[idx] = orig_addr;
+}
+
+static void *dma_bounced_get_orig_virt(struct device *dev, dma_addr_t dma_addr)
+{
+   struct dma_bounced_mem *mem = dev->dma_bounced_mem;
+   int idx = (dma_addr - mem->device_base) >> PAGE_SHIFT;
+
+   if (dma_addr < mem->device_base || dma_addr >= mem->device_end)
+   return NULL;
+
+   return mem->orig_addr[idx];
+}
+
+static void *dma_bounced_get_virt(struct device *dev, dma_addr_t dma_addr)
+{
+   struct dma_bounced_mem *mem = dev->dma_bounced_mem;
+
+   if (dma_addr < mem->device_base || dma_addr >= mem->device_end)
+   return NULL;
+
+   return (dma_addr - mem->device_base) + mem->virt_base;
+}
+
+static void dma_bounced_sync_single_for_cpu(struct device *dev,
+  

Re: [PATCH 1/3] serdev: ttyport: add devt for tty port

2020-05-18 Thread Claire Chang
On Fri, May 15, 2020 at 8:46 PM Greg KH  wrote:
>
> On Wed, May 06, 2020 at 03:23:12PM +0800, Claire Chang wrote:
> > serial_match_port() uses devt to match devices. However, when serdev
> > registers a tty port, devt has never been set. This makes
> > device_find_child() always return NULL.
> >
> > Assign devt in serdev_tty_port_register() to fix this.
> >
> > Signed-off-by: Claire Chang 
> > ---
> >  drivers/tty/serdev/serdev-ttyport.c | 2 ++
> >  1 file changed, 2 insertions(+)
>
> So is existing code broken because of this?  Or does no one ever call
> device_find_child() on this?  Who needs/uses this?
>
> thanks,
>
> greg k-h

I'm not sure. Our use case is to control the wake on bluetooth
behavior by the power/wakeup node.

`readlink -f /sys/class/bluetooth/hci0`
/sys/devices/platform/soc/11003000.serial/serial0/serial0-0/bluetooth/hci0

and we'd like to use
`/sys/devices/platform/soc/11003000.serial/serial0/power/wakeup` to
decide whether to enable the in-band wakeup on uart host side.

Thanks,
Claire


[PATCH 3/3] uart: mediatek: move the in-band wakeup logic to core

2020-05-06 Thread Claire Chang
Move the in-band wakeup logic to core so that we can control the wakeup
behavior by serdev controller's power/wakeup node and align with other
serial drivers.

Signed-off-by: Claire Chang 
---
 drivers/tty/serial/8250/8250_mtk.c | 24 +++-
 1 file changed, 3 insertions(+), 21 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_mtk.c 
b/drivers/tty/serial/8250/8250_mtk.c
index f839380c2f4c1..52cb41e4e493d 100644
--- a/drivers/tty/serial/8250/8250_mtk.c
+++ b/drivers/tty/serial/8250/8250_mtk.c
@@ -71,7 +71,6 @@ struct mtk8250_data {
 #ifdef CONFIG_SERIAL_8250_DMA
enum dma_rx_status  rx_status;
 #endif
-   int rx_wakeup_irq;
 };
 
 /* flow control mode */
@@ -496,6 +495,8 @@ static int mtk8250_probe(struct platform_device *pdev)
struct uart_8250_port uart = {};
struct resource *regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
struct resource *irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+   struct resource *wakeup_irq =
+   platform_get_resource(pdev, IORESOURCE_IRQ, 1);
struct mtk8250_data *data;
int err;
 
@@ -525,6 +526,7 @@ static int mtk8250_probe(struct platform_device *pdev)
spin_lock_init();
uart.port.mapbase = regs->start;
uart.port.irq = irq->start;
+   uart.port.wakeup_irq = wakeup_irq ? wakeup_irq->start : -ENXIO;
uart.port.pm = mtk8250_do_pm;
uart.port.type = PORT_16550;
uart.port.flags = UPF_BOOT_AUTOCONF | UPF_FIXED_PORT;
@@ -556,8 +558,6 @@ static int mtk8250_probe(struct platform_device *pdev)
if (data->line < 0)
return data->line;
 
-   data->rx_wakeup_irq = platform_get_irq_optional(pdev, 1);
-
return 0;
 }
 
@@ -581,23 +581,9 @@ static int mtk8250_remove(struct platform_device *pdev)
 static int __maybe_unused mtk8250_suspend(struct device *dev)
 {
struct mtk8250_data *data = dev_get_drvdata(dev);
-   int irq = data->rx_wakeup_irq;
-   int err;
 
serial8250_suspend_port(data->line);
-
pinctrl_pm_select_sleep_state(dev);
-   if (irq >= 0) {
-   err = enable_irq_wake(irq);
-   if (err) {
-   dev_err(dev,
-   "failed to enable irq wake on IRQ %d: %d\n",
-   irq, err);
-   pinctrl_pm_select_default_state(dev);
-   serial8250_resume_port(data->line);
-   return err;
-   }
-   }
 
return 0;
 }
@@ -605,12 +591,8 @@ static int __maybe_unused mtk8250_suspend(struct device 
*dev)
 static int __maybe_unused mtk8250_resume(struct device *dev)
 {
struct mtk8250_data *data = dev_get_drvdata(dev);
-   int irq = data->rx_wakeup_irq;
 
-   if (irq >= 0)
-   disable_irq_wake(irq);
pinctrl_pm_select_default_state(dev);
-
serial8250_resume_port(data->line);
 
return 0;
-- 
2.26.2.526.g744177e7f7-goog



[PATCH 2/3] tty: serial_core: add wakeup_irq to support in-band wakeup

2020-05-06 Thread Claire Chang
Since some uart controllers may be off in S3, we won't be able to use
the normal in-band wakeup.

Take 8250_mtk.c as an example. The driver needs to allocate an edge
sensitive interrupt as the wakeup_irq and use an addtional pinctrl to
reconfigure Rx pin to normal GPIO in sleep state. Once host detects Rx
falling, an interrupt is triggered, and the system leaves sleep state.

Add the wakeup_irq logic in core to simplify and make the code more
generic. Also, we can align with the original wakeup behavior and
power/wakeup node.

Signed-off-by: Claire Chang 
---
 drivers/tty/serial/8250/8250_core.c | 1 +
 drivers/tty/serial/serial_core.c| 8 +---
 include/linux/serial_core.h | 1 +
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_core.c 
b/drivers/tty/serial/8250/8250_core.c
index 45d9117cab680..06214e9fdc8ff 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -1001,6 +1001,7 @@ int serial8250_register_8250_port(struct uart_8250_port 
*up)
uart->port.membase  = up->port.membase;
uart->port.irq  = up->port.irq;
uart->port.irqflags = up->port.irqflags;
+   uart->port.wakeup_irq   = up->port.wakeup_irq;
uart->port.uartclk  = up->port.uartclk;
uart->port.fifosize = up->port.fifosize;
uart->port.regshift = up->port.regshift;
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 66a5e2faf57ea..1796a33986613 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -2165,12 +2165,13 @@ int uart_suspend_port(struct uart_driver *drv, struct 
uart_port *uport)
struct tty_port *port = >port;
struct device *tty_dev;
struct uart_match match = {uport, drv};
+   int irq = uport->wakeup_irq > 0 ? uport->wakeup_irq : uport->irq;
 
mutex_lock(>mutex);
 
tty_dev = device_find_child(uport->dev, , serial_match_port);
if (tty_dev && device_may_wakeup(tty_dev)) {
-   enable_irq_wake(uport->irq);
+   enable_irq_wake(irq);
put_device(tty_dev);
mutex_unlock(>mutex);
return 0;
@@ -2228,13 +2229,14 @@ int uart_resume_port(struct uart_driver *drv, struct 
uart_port *uport)
struct device *tty_dev;
struct uart_match match = {uport, drv};
struct ktermios termios;
+   int irq = uport->wakeup_irq > 0 ? uport->wakeup_irq : uport->irq;
 
mutex_lock(>mutex);
 
tty_dev = device_find_child(uport->dev, , serial_match_port);
if (!uport->suspended && device_may_wakeup(tty_dev)) {
-   if (irqd_is_wakeup_set(irq_get_irq_data((uport->irq
-   disable_irq_wake(uport->irq);
+   if (irqd_is_wakeup_set(irq_get_irq_data((irq
+   disable_irq_wake(irq);
put_device(tty_dev);
mutex_unlock(>mutex);
return 0;
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 92f5eba860528..5764687b90a36 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -136,6 +136,7 @@ struct uart_port {
  struct serial_iso7816 
*iso7816);
unsigned intirq;/* irq number */
unsigned long   irqflags;   /* irq flags  */
+   unsigned intwakeup_irq; /* wakeup irq number */
unsigned intuartclk;/* base uart clock */
unsigned intfifosize;   /* tx fifo size */
unsigned char   x_char; /* xon/xoff char */
-- 
2.26.2.526.g744177e7f7-goog



[PATCH 0/3] add wakeup_irq for in-band wakeup support

2020-05-06 Thread Claire Chang
Since some uart controllers may be off in S3, add additional wakeup_irq
to support in-band wakeup.

Claire Chang (3):
  serdev: ttyport: add devt for tty port
  tty: serial_core: add wakeup_irq to support in-band wakeup
  uart: mediatek: move the in-band wakeup logic to core

 drivers/tty/serdev/serdev-ttyport.c |  2 ++
 drivers/tty/serial/8250/8250_core.c |  1 +
 drivers/tty/serial/8250/8250_mtk.c  | 24 +++-
 drivers/tty/serial/serial_core.c|  8 +---
 include/linux/serial_core.h |  1 +
 5 files changed, 12 insertions(+), 24 deletions(-)

-- 
2.26.2.526.g744177e7f7-goog



[PATCH 1/3] serdev: ttyport: add devt for tty port

2020-05-06 Thread Claire Chang
serial_match_port() uses devt to match devices. However, when serdev
registers a tty port, devt has never been set. This makes
device_find_child() always return NULL.

Assign devt in serdev_tty_port_register() to fix this.

Signed-off-by: Claire Chang 
---
 drivers/tty/serdev/serdev-ttyport.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/tty/serdev/serdev-ttyport.c 
b/drivers/tty/serdev/serdev-ttyport.c
index d367803e2044f..9238119173a47 100644
--- a/drivers/tty/serdev/serdev-ttyport.c
+++ b/drivers/tty/serdev/serdev-ttyport.c
@@ -267,6 +267,7 @@ struct device *serdev_tty_port_register(struct tty_port 
*port,
 {
struct serdev_controller *ctrl;
struct serport *serport;
+   dev_t devt = MKDEV(drv->major, drv->minor_start) + idx;
int ret;
 
if (!port || !drv || !parent)
@@ -282,6 +283,7 @@ struct device *serdev_tty_port_register(struct tty_port 
*port,
serport->tty_drv = drv;
 
ctrl->ops = _ops;
+   ctrl->dev.devt = devt;
 
port->client_ops = _ops;
port->client_data = ctrl;
-- 
2.26.2.526.g744177e7f7-goog



Re: [PATCH] Bluetooth: hci_qca: fix in-band sleep enablement

2019-10-17 Thread Claire Chang
Hi Balakrishna,

Sorry for the late reply. I was on vacation for the past few days.
The chipset we use is QCA6174A-3 and rjliao has already confirmed that
IBS won't work without RAM fw downloaded.
Please ignore this change.

Thanks,
Claire


On Fri, Oct 11, 2019 at 3:21 PM Balakrishna Godavarthi
 wrote:
>
> Hi Claire,
>
> This change will not work  as we need fw files to be loaded tofor IBS to
> active.
> may i know on which chipset you have this issue of IBS active even with
> out fw download.
>
> On 2019-10-11 12:31, Harish Bandi wrote:
> > ++ Balakrishna
> >
> > On 2019-10-09 14:21, Claire Chang wrote:
> >> Enabling in-band sleep when there is no patch/nvm-config found and
> >> bluetooth is running with the original fw/config.
> >>
> >> Fixes: ba8f35979002 ("Bluetooth: hci_qca: Avoid setup failure on
> >> missing rampatch")
> >> Fixes: 7dc5fe0814c3 ("Bluetooth: hci_qca: Avoid missing rampatch
> >> failure with userspace fw loader")
> >> Signed-off-by: Claire Chang 
> >> ---
> >>  drivers/bluetooth/hci_qca.c | 11 +++
> >>  1 file changed, 7 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> >> index e3164c200eac..367eef893a11 100644
> >> --- a/drivers/bluetooth/hci_qca.c
> >> +++ b/drivers/bluetooth/hci_qca.c
> >> @@ -1291,10 +1291,8 @@ static int qca_setup(struct hci_uart *hu)
> >>  /* Setup patch / NVM configurations */
> >>  ret = qca_uart_setup(hdev, qca_baudrate, soc_type, soc_ver,
> >>  firmware_name);
> >> -if (!ret) {
> >> -set_bit(QCA_IBS_ENABLED, >flags);
> >> -qca_debugfs_init(hdev);
> >> -} else if (ret == -ENOENT) {
> >> +
> >> +if (ret == -ENOENT) {
> >>  /* No patch/nvm-config found, run with original fw/config */
> >>  ret = 0;
> >>  } else if (ret == -EAGAIN) {
> >> @@ -1305,6 +1303,11 @@ static int qca_setup(struct hci_uart *hu)
> >>  ret = 0;
> >>  }
> >>
> >> +if (!ret) {
> >> +set_bit(QCA_IBS_ENABLED, >flags);
> >> +qca_debugfs_init(hdev);
> >> +}
> >> +
> >>  /* Setup bdaddr */
> >>  if (qca_is_wcn399x(soc_type))
> >>  hu->hdev->set_bdaddr = qca_set_bdaddr;
>
> --
> Regards
> Balakrishna.


[PATCH] Bluetooth: hci_qca: fix in-band sleep enablement

2019-10-09 Thread Claire Chang
Enabling in-band sleep when there is no patch/nvm-config found and
bluetooth is running with the original fw/config.

Fixes: ba8f35979002 ("Bluetooth: hci_qca: Avoid setup failure on missing 
rampatch")
Fixes: 7dc5fe0814c3 ("Bluetooth: hci_qca: Avoid missing rampatch failure with 
userspace fw loader")
Signed-off-by: Claire Chang 
---
 drivers/bluetooth/hci_qca.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index e3164c200eac..367eef893a11 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -1291,10 +1291,8 @@ static int qca_setup(struct hci_uart *hu)
/* Setup patch / NVM configurations */
ret = qca_uart_setup(hdev, qca_baudrate, soc_type, soc_ver,
firmware_name);
-   if (!ret) {
-   set_bit(QCA_IBS_ENABLED, >flags);
-   qca_debugfs_init(hdev);
-   } else if (ret == -ENOENT) {
+
+   if (ret == -ENOENT) {
/* No patch/nvm-config found, run with original fw/config */
ret = 0;
} else if (ret == -EAGAIN) {
@@ -1305,6 +1303,11 @@ static int qca_setup(struct hci_uart *hu)
ret = 0;
}
 
+   if (!ret) {
+   set_bit(QCA_IBS_ENABLED, >flags);
+   qca_debugfs_init(hdev);
+   }
+
/* Setup bdaddr */
if (qca_is_wcn399x(soc_type))
hu->hdev->set_bdaddr = qca_set_bdaddr;
-- 
2.23.0.581.g78d2f28ef7-goog



[PATCH] Bluetooth: btqca: release_firmware after qca_inject_cmd_complete_event

2019-08-06 Thread Claire Chang
commit 32646db8cc28 ("Bluetooth: btqca: inject command complete event
during fw download") added qca_inject_cmd_complete_event() for certain
qualcomm chips. However, qca_download_firmware() will return without
calling release_firmware() in this case.

This leads to a memory leak like the following found by kmemleak:

unreferenced object 0xfff3868a5880 (size 128):
  comm "kworker/u17:5", pid 347, jiffies 4294676481 (age 312.157s)
  hex dump (first 32 bytes):
ac fd 00 00 00 00 00 00 00 d0 7e 17 80 ff ff ff  ..~.
00 00 00 00 00 00 00 00 00 59 8a 86 f3 ff ff ff  .Y..
  backtrace:
[<978ce31d>] kmem_cache_alloc_trace+0x194/0x298
[<6ea0398c>] _request_firmware+0x74/0x4e4
[<4da31ca0>] request_firmware+0x44/0x64
[<94572996>] qca_download_firmware+0x74/0x6e4 [btqca]
[<b24d615a>] qca_uart_setup+0xc0/0x2b0 [btqca]
[<364a6d5a>] qca_setup+0x204/0x570 [hci_uart]
[<6be1a544>] hci_uart_setup+0xa8/0x148 [hci_uart]
[<d64c0f4f>] hci_dev_do_open+0x144/0x530 [bluetooth]
[<f69f5110>] hci_power_on+0x84/0x288 [bluetooth]
[<d4151583>] process_one_work+0x210/0x420
[<3cf3dcfb>] worker_thread+0x2c4/0x3e4
[<7ccaf055>] kthread+0x124/0x134
[<bef1f723>] ret_from_fork+0x10/0x18
[<c36ee3dd>] 0x
unreferenced object 0xfff37b16de00 (size 128):
  comm "kworker/u17:5", pid 347, jiffies 4294676873 (age 311.766s)
  hex dump (first 32 bytes):
da 07 00 00 00 00 00 00 00 50 ff 0b 80 ff ff ff  .P..
00 00 00 00 00 00 00 00 00 dd 16 7b f3 ff ff ff  ...{
  backtrace:
[<978ce31d>] kmem_cache_alloc_trace+0x194/0x298
[<6ea0398c>] _request_firmware+0x74/0x4e4
[<4da31ca0>] request_firmware+0x44/0x64
[<94572996>] qca_download_firmware+0x74/0x6e4 [btqca]
[<0cde20a9>] qca_uart_setup+0x144/0x2b0 [btqca]
[<364a6d5a>] qca_setup+0x204/0x570 [hci_uart]
[<6be1a544>] hci_uart_setup+0xa8/0x148 [hci_uart]
[<d64c0f4f>] hci_dev_do_open+0x144/0x530 [bluetooth]
[<f69f5110>] hci_power_on+0x84/0x288 [bluetooth]
[<d4151583>] process_one_work+0x210/0x420
[<3cf3dcfb>] worker_thread+0x2c4/0x3e4
[<7ccaf055>] kthread+0x124/0x134
[<bef1f723>] ret_from_fork+0x10/0x18
[<c36ee3dd>] 0x

Make sure release_firmware() is called aftre
qca_inject_cmd_complete_event() to avoid the memory leak.

Fixes: 32646db8cc28 ("Bluetooth: btqca: inject command complete event during fw 
download")
Signed-off-by: Claire Chang 
---
 drivers/bluetooth/btqca.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
index 2221935fac7e..8f0fec5acade 100644
--- a/drivers/bluetooth/btqca.c
+++ b/drivers/bluetooth/btqca.c
@@ -344,7 +344,7 @@ static int qca_download_firmware(struct hci_dev *hdev,
 */
if (config->dnld_type == ROME_SKIP_EVT_VSE_CC ||
config->dnld_type == ROME_SKIP_EVT_VSE)
-   return qca_inject_cmd_complete_event(hdev);
+   ret = qca_inject_cmd_complete_event(hdev);
 
 out:
release_firmware(fw);
-- 
2.22.0.770.g0f2c4a37fd-goog



Re: [PATCH v1] Bluetooth: hci_qca: Give enough time to ROME controller to bootup.

2019-03-18 Thread Claire Chang
> > >>> Signed-off-by: Balakrishna Godavarthi 
Tested-by: Claire Chang 


Re: [PATCH v2 5/9] mfd: Add support for the MediaTek MT6358 PMIC

2019-03-15 Thread Claire Chang
On Fri, Mar 15, 2019 at 3:10 PM Nicolas Boichat  wrote:
>
> +Claire Chang who found some issue with this patch.
>
> On Mon, Mar 11, 2019 at 11:48 AM Hsin-Hsiung Wang
>  wrote:
> >
> > This adds support for the MediaTek MT6358 PMIC. This is a
> > multifunction device with the following sub modules:
> >
> > - Regulator
> > - RTC
> > - Codec
> > - Interrupt
> >
> > It is interfaced to the host controller using SPI interface
> > by a proprietary hardware called PMIC wrapper or pwrap.
> > MT6358 MFD is a child device of the pwrap.
> >
> > Signed-off-by: Hsin-Hsiung Wang 
> > ---
> >  drivers/mfd/Makefile |2 +-
> >  drivers/mfd/mt6358-irq.c |  236 +
> >  drivers/mfd/mt6397-core.c|   63 +-
> >  include/linux/mfd/mt6358/core.h  |  158 +++
> >  include/linux/mfd/mt6358/registers.h | 1926 
> > ++
> >  include/linux/mfd/mt6397/core.h  |3 +
> >  6 files changed, 2386 insertions(+), 2 deletions(-)
> >  create mode 100644 drivers/mfd/mt6358-irq.c
> >  create mode 100644 include/linux/mfd/mt6358/core.h
> >  create mode 100644 include/linux/mfd/mt6358/registers.h
> >
> > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> > index 088e249..50be021 100644
> > --- a/drivers/mfd/Makefile
> > +++ b/drivers/mfd/Makefile
> > @@ -230,7 +230,7 @@ obj-$(CONFIG_INTEL_SOC_PMIC)+= intel-soc-pmic.o
> >  obj-$(CONFIG_INTEL_SOC_PMIC_BXTWC) += intel_soc_pmic_bxtwc.o
> >  obj-$(CONFIG_INTEL_SOC_PMIC_CHTWC) += intel_soc_pmic_chtwc.o
> >  obj-$(CONFIG_INTEL_SOC_PMIC_CHTDC_TI)  += intel_soc_pmic_chtdc_ti.o
> > -obj-$(CONFIG_MFD_MT6397)   += mt6397-core.o mt6397-irq.o
> > +obj-$(CONFIG_MFD_MT6397)   += mt6397-core.o mt6397-irq.o mt6358-irq.o
> >
> >  obj-$(CONFIG_MFD_ALTERA_A10SR) += altera-a10sr.o
> >  obj-$(CONFIG_MFD_SUN4I_GPADC)  += sun4i-gpadc.o
> > diff --git a/drivers/mfd/mt6358-irq.c b/drivers/mfd/mt6358-irq.c
> > new file mode 100644
> > index 000..2941d87
> > --- /dev/null
> > +++ b/drivers/mfd/mt6358-irq.c
> > @@ -0,0 +1,236 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +//
> > +// Copyright (c) 2019 MediaTek Inc.
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +static struct irq_top_t mt6358_ints[] = {
> > +   MT6358_TOP_GEN(BUCK),
> > +   MT6358_TOP_GEN(LDO),
> > +   MT6358_TOP_GEN(PSC),
> > +   MT6358_TOP_GEN(SCK),
> > +   MT6358_TOP_GEN(BM),
> > +   MT6358_TOP_GEN(HK),
> > +   MT6358_TOP_GEN(AUD),
> > +   MT6358_TOP_GEN(MISC),
> > +};
> > +
> > +static int parsing_hwirq_to_top_group(unsigned int hwirq)
> > +{
> > +   int top_group;
> > +
> > +   for (top_group = 1; top_group < ARRAY_SIZE(mt6358_ints); 
> > top_group++) {
> > +   if (mt6358_ints[top_group].hwirq_base > hwirq) {
> > +   top_group--;
> > +   break;
> > +   }
> > +   }
> > +   return top_group;
> > +}
> > +
> > +static void pmic_irq_enable(struct irq_data *data)
> > +{
> > +   unsigned int hwirq = irqd_to_hwirq(data);
> > +   struct mt6397_chip *chip = irq_data_get_irq_chip_data(data);
> > +   struct pmic_irq_data *irqd = chip->irq_data;
> > +
> > +   irqd->enable_hwirq[hwirq] = true;
> > +}
> > +
> > +static void pmic_irq_disable(struct irq_data *data)
> > +{
> > +   unsigned int hwirq = irqd_to_hwirq(data);
> > +   struct mt6397_chip *chip = irq_data_get_irq_chip_data(data);
> > +   struct pmic_irq_data *irqd = chip->irq_data;
> > +
> > +   irqd->enable_hwirq[hwirq] = false;
> > +}
> > +
> > +static void pmic_irq_lock(struct irq_data *data)
> > +{
> > +   struct mt6397_chip *chip = irq_data_get_irq_chip_data(data);
> > +
> > +   mutex_lock(>irqlock);
> > +}
> > +
> > +static void pmic_irq_sync_unlock(struct irq_data *data)
> > +{
> > +   unsigned int i, top_gp, en_reg, int_regs, shift;
> > +   struct mt6397_chip *chip = irq_data_get_irq_chip_data(data);
> > +   struct pmic_irq_data *irqd = chip->irq_data;
> > +
> > +   for (i = 0; i < irqd->num_pmic_irqs; i++) {
> > +   if (irqd->enable_hwirq[i