Re: [PATCH v4 0/3] Replace private domain with per-group default domain

2020-05-05 Thread Daniel Drake
On Wed, May 6, 2020 at 10:03 AM Lu Baolu  wrote:
> https://lkml.org/lkml/2020/4/14/616
> [This has been applied in iommu/next.]
>
> Hence, there is no need to keep the private domain implementation
> in the Intel IOMMU driver. This patch series aims to remove it.

I applied these patches on top of Joerg's branch and confirmed that
they fix the issue discussed in the thread:

[PATCH v2] iommu/vt-d: consider real PCI device when checking if
mapping is needed
(the patch there is no longer needed)

Tested-by: Daniel Drake 

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


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

2020-05-05 Thread Lu Baolu
Current Intel IOMMU driver sets the system level dma_ops. This causes
each dma API to go through the IOMMU driver even the devices are using
identity mapped domains. This sets per-device dma_ops only if a device
is using a DMA domain. Otherwise, use the default system level dma_ops
for direct dma.

Signed-off-by: Lu Baolu 
---
 drivers/iommu/intel-iommu.c | 82 -
 1 file changed, 26 insertions(+), 56 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index af309e8fa6f5..29d3940847d3 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -2720,17 +2720,6 @@ static int __init si_domain_init(int hw)
return 0;
 }
 
-static int identity_mapping(struct device *dev)
-{
-   struct device_domain_info *info;
-
-   info = dev->archdata.iommu;
-   if (info)
-   return (info->domain == si_domain);
-
-   return 0;
-}
-
 static int domain_add_dev_info(struct dmar_domain *domain, struct device *dev)
 {
struct dmar_domain *ndomain;
@@ -3315,18 +3304,6 @@ static unsigned long intel_alloc_iova(struct device *dev,
return iova_pfn;
 }
 
-/* Check if the dev needs to go through non-identity map and unmap process.*/
-static bool iommu_need_mapping(struct device *dev)
-{
-   if (iommu_dummy(dev))
-   return false;
-
-   if (unlikely(attach_deferred(dev)))
-   do_deferred_attach(dev);
-
-   return !identity_mapping(dev);
-}
-
 static dma_addr_t __intel_map_single(struct device *dev, phys_addr_t paddr,
 size_t size, int dir, u64 dma_mask)
 {
@@ -3340,6 +3317,9 @@ static dma_addr_t __intel_map_single(struct device *dev, 
phys_addr_t paddr,
 
BUG_ON(dir == DMA_NONE);
 
+   if (unlikely(attach_deferred(dev)))
+   do_deferred_attach(dev);
+
domain = find_domain(dev);
if (!domain)
return DMA_MAPPING_ERROR;
@@ -3391,20 +3371,15 @@ static dma_addr_t intel_map_page(struct device *dev, 
struct page *page,
 enum dma_data_direction dir,
 unsigned long attrs)
 {
-   if (iommu_need_mapping(dev))
-   return __intel_map_single(dev, page_to_phys(page) + offset,
-   size, dir, *dev->dma_mask);
-   return dma_direct_map_page(dev, page, offset, size, dir, attrs);
+   return __intel_map_single(dev, page_to_phys(page) + offset,
+ size, dir, *dev->dma_mask);
 }
 
 static dma_addr_t intel_map_resource(struct device *dev, phys_addr_t phys_addr,
 size_t size, enum dma_data_direction dir,
 unsigned long attrs)
 {
-   if (iommu_need_mapping(dev))
-   return __intel_map_single(dev, phys_addr, size, dir,
-   *dev->dma_mask);
-   return dma_direct_map_resource(dev, phys_addr, size, dir, attrs);
+   return __intel_map_single(dev, phys_addr, size, dir, *dev->dma_mask);
 }
 
 static void intel_unmap(struct device *dev, dma_addr_t dev_addr, size_t size)
@@ -3455,17 +3430,13 @@ static void intel_unmap_page(struct device *dev, 
dma_addr_t dev_addr,
 size_t size, enum dma_data_direction dir,
 unsigned long attrs)
 {
-   if (iommu_need_mapping(dev))
-   intel_unmap(dev, dev_addr, size);
-   else
-   dma_direct_unmap_page(dev, dev_addr, size, dir, attrs);
+   intel_unmap(dev, dev_addr, size);
 }
 
 static void intel_unmap_resource(struct device *dev, dma_addr_t dev_addr,
size_t size, enum dma_data_direction dir, unsigned long attrs)
 {
-   if (iommu_need_mapping(dev))
-   intel_unmap(dev, dev_addr, size);
+   intel_unmap(dev, dev_addr, size);
 }
 
 static void *intel_alloc_coherent(struct device *dev, size_t size,
@@ -3475,8 +3446,8 @@ static void *intel_alloc_coherent(struct device *dev, 
size_t size,
struct page *page = NULL;
int order;
 
-   if (!iommu_need_mapping(dev))
-   return dma_direct_alloc(dev, size, dma_handle, flags, attrs);
+   if (unlikely(attach_deferred(dev)))
+   do_deferred_attach(dev);
 
size = PAGE_ALIGN(size);
order = get_order(size);
@@ -3511,9 +3482,6 @@ static void intel_free_coherent(struct device *dev, 
size_t size, void *vaddr,
int order;
struct page *page = virt_to_page(vaddr);
 
-   if (!iommu_need_mapping(dev))
-   return dma_direct_free(dev, size, vaddr, dma_handle, attrs);
-
size = PAGE_ALIGN(size);
order = get_order(size);
 
@@ -3531,9 +3499,6 @@ static void intel_unmap_sg(struct device *dev, struct 
scatterlist *sglist,
struct scatterlist *sg;
int i;
 
-   if (!iommu_need_mapping(dev))
-   return dma_direct_unmap_sg(dev, sglist, nelem

[PATCH v4 2/3] iommu/vt-d: Allow PCI sub-hierarchy to use DMA domain

2020-05-05 Thread Lu Baolu
Before commit fa954e6831789 ("iommu/vt-d: Delegate the dma domain
to upper layer"), Intel IOMMU started off with all devices in the
identity domain, and took them out later if it found they couldn't
access all of memory. This required devices behind a PCI bridge to
use a DMA domain at the beginning because all PCI devices behind
the bridge use the same source-id in their transactions and the
domain couldn't be changed at run-time.

Intel IOMMU driver is now aligned with the default domain framework,
there's no need to keep this requirement anymore.

Signed-off-by: Lu Baolu 
---
 drivers/iommu/intel-iommu.c | 25 -
 1 file changed, 25 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 16ba7add0f72..af309e8fa6f5 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -2857,31 +2857,6 @@ static int device_def_domain_type(struct device *dev)
 
if ((iommu_identity_mapping & IDENTMAP_GFX) && 
IS_GFX_DEVICE(pdev))
return IOMMU_DOMAIN_IDENTITY;
-
-   /*
-* We want to start off with all devices in the 1:1 domain, and
-* take them out later if we find they can't access all of 
memory.
-*
-* However, we can't do this for PCI devices behind bridges,
-* because all PCI devices behind the same bridge will end up
-* with the same source-id on their transactions.
-*
-* Practically speaking, we can't change things around for these
-* devices at run-time, because we can't be sure there'll be no
-* DMA transactions in flight for any of their siblings.
-*
-* So PCI devices (unless they're on the root bus) as well as
-* their parent PCI-PCI or PCIe-PCI bridges must be left _out_ 
of
-* the 1:1 domain, just in _case_ one of their siblings turns 
out
-* not to be able to map all of memory.
-*/
-   if (!pci_is_pcie(pdev)) {
-   if (!pci_is_root_bus(pdev->bus))
-   return IOMMU_DOMAIN_DMA;
-   if (pdev->class >> 8 == PCI_CLASS_BRIDGE_PCI)
-   return IOMMU_DOMAIN_DMA;
-   } else if (pci_pcie_type(pdev) == PCI_EXP_TYPE_PCI_BRIDGE)
-   return IOMMU_DOMAIN_DMA;
}
 
return 0;
-- 
2.17.1

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


[PATCH v4 0/3] Replace private domain with per-group default domain

2020-05-05 Thread Lu Baolu
Some devices are required to use a specific type (identity or dma) of
default domain when they are used with a vendor iommu. When the system
level default domain type is different from it, the vendor iommu driver
has to request a new default domain with either
iommu_request_dma_domain_for_dev() or iommu_request_dm_for_dev() in the
add_dev() callback. Unfortunately, these two helpers only work when the
group hasn't been assigned to any other devices, hence, some vendor iommu
driver has to use a private domain if it fails to request a new default
one.

Joerg proposed an on-going proposal which makes the default domain
framework to support configuring per-group default domain during boot
process.

https://lkml.org/lkml/2020/4/14/616
[This has been applied in iommu/next.]

Hence, there is no need to keep the private domain implementation
in the Intel IOMMU driver. This patch series aims to remove it.

Best regards,
baolu

Change log:
v3->v4:
 - Make the commit message of the first patch more comprehensive.

v2->v3:
 - Port necessary patches on the top of Joerg's new proposal.
   https://lkml.org/lkml/2020/4/14/616
   The per-group default domain proposed previously in this series
   will be deprecated due to a race concern between domain switching
   and device driver probing.

v1->v2:
 - Rename the iommu ops callback to def_domain_type

Lu Baolu (3):
  iommu/vt-d: Allow 32bit devices to uses DMA domain
  iommu/vt-d: Allow PCI sub-hierarchy to use DMA domain
  iommu/vt-d: Apply per-device dma_ops

 drivers/iommu/intel-iommu.c | 396 +++-
 1 file changed, 26 insertions(+), 370 deletions(-)

-- 
2.17.1

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


[PATCH v4 1/3] iommu/vt-d: Allow 32bit devices to uses DMA domain

2020-05-05 Thread Lu Baolu
Currently, if a 32bit device initially uses an identity domain, Intel
IOMMU driver will convert it forcibly to a DMA one if its address
capability is not enough for the whole system memory. The motivation was
to overcome the overhead caused by possible bounced buffer.

Unfortunately, this improvement has led to many problems. For example,
some 32bit devices are required to use an identity domain, forcing them
to use DMA domain will cause the device not to work anymore. On the
other hand, the VMD sub-devices share a domain but each sub-device might
have different address capability. Forcing a VMD sub-device to use DMA
domain blindly will impact the operation of other sub-devices without
any notification. Further more, PCI aliased devices (PCI bridge and all
devices beneath it, VMD devices and various devices quirked with
pci_add_dma_alias()) must use the same domain. Forcing one device to
switch to DMA domain during runtime will cause in-fligh DMAs for other
devices to abort or target to other memory which might cause undefind
system behavior.

With the last private domain usage in iommu_need_mapping() removed, all
private domain helpers are also cleaned in this patch. Otherwise, the
compiler will complain that some functions are defined but not used.

Cc: Daniel Drake 
Cc: Derrick Jonathan 
Cc: Jerry Snitselaar 
Signed-off-by: Lu Baolu 
---
 drivers/iommu/intel-iommu.c | 291 +---
 1 file changed, 1 insertion(+), 290 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 34e08fa2ce3a..16ba7add0f72 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -355,11 +355,6 @@ static void domain_exit(struct dmar_domain *domain);
 static void domain_remove_dev_info(struct dmar_domain *domain);
 static void dmar_remove_one_dev_info(struct device *dev);
 static void __dmar_remove_one_dev_info(struct device_domain_info *info);
-static void domain_context_clear(struct intel_iommu *iommu,
-struct device *dev);
-static int domain_detach_iommu(struct dmar_domain *domain,
-  struct intel_iommu *iommu);
-static bool device_is_rmrr_locked(struct device *dev);
 static int intel_iommu_attach_device(struct iommu_domain *domain,
 struct device *dev);
 static phys_addr_t intel_iommu_iova_to_phys(struct iommu_domain *domain,
@@ -1930,65 +1925,6 @@ static inline int guestwidth_to_adjustwidth(int gaw)
return agaw;
 }
 
-static int domain_init(struct dmar_domain *domain, struct intel_iommu *iommu,
-  int guest_width)
-{
-   int adjust_width, agaw;
-   unsigned long sagaw;
-   int ret;
-
-   init_iova_domain(&domain->iovad, VTD_PAGE_SIZE, IOVA_START_PFN);
-
-   if (!intel_iommu_strict) {
-   ret = init_iova_flush_queue(&domain->iovad,
-   iommu_flush_iova, iova_entry_free);
-   if (ret)
-   pr_info("iova flush queue initialization failed\n");
-   }
-
-   domain_reserve_special_ranges(domain);
-
-   /* calculate AGAW */
-   if (guest_width > cap_mgaw(iommu->cap))
-   guest_width = cap_mgaw(iommu->cap);
-   domain->gaw = guest_width;
-   adjust_width = guestwidth_to_adjustwidth(guest_width);
-   agaw = width_to_agaw(adjust_width);
-   sagaw = cap_sagaw(iommu->cap);
-   if (!test_bit(agaw, &sagaw)) {
-   /* hardware doesn't support it, choose a bigger one */
-   pr_debug("Hardware doesn't support agaw %d\n", agaw);
-   agaw = find_next_bit(&sagaw, 5, agaw);
-   if (agaw >= 5)
-   return -ENODEV;
-   }
-   domain->agaw = agaw;
-
-   if (ecap_coherent(iommu->ecap))
-   domain->iommu_coherency = 1;
-   else
-   domain->iommu_coherency = 0;
-
-   if (ecap_sc_support(iommu->ecap))
-   domain->iommu_snooping = 1;
-   else
-   domain->iommu_snooping = 0;
-
-   if (intel_iommu_superpage)
-   domain->iommu_superpage = fls(cap_super_page_val(iommu->cap));
-   else
-   domain->iommu_superpage = 0;
-
-   domain->nid = iommu->node;
-
-   /* always allocate the top pgd */
-   domain->pgd = (struct dma_pte *)alloc_pgtable_page(domain->nid);
-   if (!domain->pgd)
-   return -ENOMEM;
-   __iommu_flush_cache(iommu, domain->pgd, PAGE_SIZE);
-   return 0;
-}
-
 static void domain_exit(struct dmar_domain *domain)
 {
 
@@ -2704,94 +2640,6 @@ static struct dmar_domain 
*dmar_insert_one_dev_info(struct intel_iommu *iommu,
return domain;
 }
 
-static int get_last_alias(struct pci_dev *pdev, u16 alias, void *opaque)
-{
-   *(u16 *)opaque = alias;
-   return 0;
-}
-
-static struct dmar_domain *find_or_alloc_domain(struct device *dev, int gaw)
-{
-   struct device_domain_info *info;

Re: [PATCH v5] iommu/virtio: Use page size bitmap supported by endpoint

2020-05-05 Thread Michael S. Tsirkin
On Tue, May 05, 2020 at 03:00:04PM +0530, Bharat Bhushan wrote:
> Different endpoint can support different page size, probe
> endpoint if it supports specific page size otherwise use
> global page sizes.
> 
> Signed-off-by: Bharat Bhushan 
> ---
> v4->v5:
>  - Rebase to Linux v5.7-rc4
> 
> v3->v4:
>  - Fix whitespace error
> 
> v2->v3:
>  - Fixed error return for incompatible endpoint
>  - __u64 changed to __le64 in header file
> 
>  drivers/iommu/virtio-iommu.c  | 48 ---
>  include/uapi/linux/virtio_iommu.h |  7 +
>  2 files changed, 51 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
> index d5cac4f46ca5..9513d2ab819e 100644
> --- a/drivers/iommu/virtio-iommu.c
> +++ b/drivers/iommu/virtio-iommu.c
> @@ -78,6 +78,7 @@ struct viommu_endpoint {
>   struct viommu_dev   *viommu;
>   struct viommu_domain*vdomain;
>   struct list_headresv_regions;
> + u64 pgsize_bitmap;
>  };
>  
>  struct viommu_request {
> @@ -415,6 +416,19 @@ static int viommu_replay_mappings(struct viommu_domain 
> *vdomain)
>   return ret;
>  }
>  
> +static int viommu_set_pgsize_bitmap(struct viommu_endpoint *vdev,
> + struct virtio_iommu_probe_pgsize_mask *mask,
> + size_t len)
> +{
> + u64 pgsize_bitmap = le64_to_cpu(mask->pgsize_bitmap);
> +
> + if (len < sizeof(*mask))

This is too late to validate length, you have dereferenced it already.
do it before the read pls.

> + return -EINVAL;

OK but note that guest will then just proceed to ignore the
property. Is that really OK? Wouldn't host want to know?


> +
> + vdev->pgsize_bitmap = pgsize_bitmap;

what if bitmap is 0? Is that a valid size? I see a bunch of
BUG_ON with that value ...

I also see a bunch of code like e.g. this:

pg_size = 1UL << __ffs(pgsize_bitmap);

which probably won't DTRT on a 32 bit guest if the bitmap has bits
set in the high word.



> + return 0;
> +}
> +
>  static int viommu_add_resv_mem(struct viommu_endpoint *vdev,
>  struct virtio_iommu_probe_resv_mem *mem,
>  size_t len)
> @@ -499,6 +513,9 @@ static int viommu_probe_endpoint(struct viommu_dev 
> *viommu, struct device *dev)
>   case VIRTIO_IOMMU_PROBE_T_RESV_MEM:
>   ret = viommu_add_resv_mem(vdev, (void *)prop, len);
>   break;
> + case VIRTIO_IOMMU_PROBE_T_PAGE_SIZE_MASK:
> + ret = viommu_set_pgsize_bitmap(vdev, (void *)prop, len);
> + break;
>   default:
>   dev_err(dev, "unknown viommu prop 0x%x\n", type);
>   }
> @@ -630,7 +647,7 @@ static int viommu_domain_finalise(struct viommu_endpoint 
> *vdev,
>  
>   vdomain->id = (unsigned int)ret;
>  
> - domain->pgsize_bitmap   = viommu->pgsize_bitmap;
> + domain->pgsize_bitmap   = vdev->pgsize_bitmap;
>   domain->geometry= viommu->geometry;
>  
>   vdomain->map_flags  = viommu->map_flags;
> @@ -654,6 +671,29 @@ static void viommu_domain_free(struct iommu_domain 
> *domain)
>   kfree(vdomain);
>  }
>  
> +/*
> + * Check whether the endpoint's capabilities are compatible with other
> + * endpoints in the domain. Report any inconsistency.
> + */
> +static bool viommu_endpoint_is_compatible(struct viommu_endpoint *vdev,
> +   struct viommu_domain *vdomain)
> +{
> + struct device *dev = vdev->dev;
> +
> + if (vdomain->viommu != vdev->viommu) {
> + dev_err(dev, "cannot attach to foreign vIOMMU\n");
> + return false;
> + }
> +
> + if (vdomain->domain.pgsize_bitmap != vdev->pgsize_bitmap) {
> + dev_err(dev, "incompatible domain bitmap 0x%lx != 0x%llx\n",
> + vdomain->domain.pgsize_bitmap, vdev->pgsize_bitmap);
> + return false;
> + }

I'm confused by this. So let's assume host supports pages sizes
of 4k, 2M, 1G. It signals this in the properties. Nice.
Now domain supports 4k, 2M and that's all. Why is that a problem?
Just don't use 1G ...


> +
> + return true;
> +}
> +
>  static int viommu_attach_dev(struct iommu_domain *domain, struct device *dev)
>  {
>   int i;
> @@ -670,9 +710,8 @@ static int viommu_attach_dev(struct iommu_domain *domain, 
> struct device *dev)
>* owns it.
>*/
>   ret = viommu_domain_finalise(vdev, domain);
> - } else if (vdomain->viommu != vdev->viommu) {
> - dev_err(dev, "cannot attach to foreign vIOMMU\n");
> - ret = -EXDEV;
> + } else if (!viommu_endpoint_is_compatible(vdev, vdomain)) {
> + ret = -EINVAL;
>   }
>   mutex_unlock(&vdomain->mutex);
>  
> @@ -886,6 +925,7 @@ static int viommu_a

[PATCH] iommu/virtio: reverse arguments to list_add

2020-05-05 Thread Julia Lawall
Elsewhere in the file, there is a list_for_each_entry with
&vdev->resv_regions as the second argument, suggesting that
&vdev->resv_regions is the list head.  So exchange the
arguments on the list_add call to put the list head in the
second argument.

Fixes: 2a5a31487445 ("iommu/virtio: Add probe request")
Signed-off-by: Julia Lawall 

---
 drivers/iommu/virtio-iommu.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index d5cac4f46ca5..4e1d11af23c8 100644
--- a/drivers/iommu/virtio-iommu.c
+++ b/drivers/iommu/virtio-iommu.c
@@ -453,7 +453,7 @@ static int viommu_add_resv_mem(struct viommu_endpoint *vdev,
if (!region)
return -ENOMEM;
 
-   list_add(&vdev->resv_regions, ®ion->list);
+   list_add(®ion->list, &vdev->resv_regions);
return 0;
 }
 

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


Re: [PATCH v5] dt-bindings: iommu: renesas,ipmmu-vmsa: convert to json-schema

2020-05-05 Thread Rob Herring
On Tue, 21 Apr 2020 14:15:52 +0900, Yoshihiro Shimoda wrote:
> Convert Renesas VMSA-Compatible IOMMU bindings documentation
> to json-schema.
> 
> Note that original documentation doesn't mention renesas,ipmmu-vmsa
> for R-Mobile APE6. But, R-Mobile APE6 is similar to the R-Car
> Gen2. So, renesas,ipmmu-r8a73a4 belongs the renesas,ipmmu-vmsa
> section.
> 
> Signed-off-by: Yoshihiro Shimoda 
> ---
>  Changes from v4:
>  - Fix description about cell counts on #iommu-cells and renesas,ipmmu-main.
>  - Fix node name on the example. 
>  https://patchwork.kernel.org/patch/11494231/
> 
>  Changes from v3:
>  - Fix renesas,ipmmu-r8a7795's section
>  https://patchwork.kernel.org/patch/11494079/
> 
>  Changes from v2:
>  - Add a description for R-Mobile APE6 on the commit log.
>  - Change renesas,ipmmu-r8a73a4 section on the compatible.
>  - Add items on the interrupts.
>  - Add power-domains to required.
>  - Add oneOf for interrupts and renesas,ipmmu-main
>  https://patchwork.kernel.org/patch/11490581/
> 
>  Changes from v1:
>  - Fix typo in the subject.
>  - Add a description on #iommu-cells.
>  https://patchwork.kernel.org/patch/11485415/
> 
>  .../bindings/iommu/renesas,ipmmu-vmsa.txt  | 73 
>  .../bindings/iommu/renesas,ipmmu-vmsa.yaml | 98 
> ++
>  2 files changed, 98 insertions(+), 73 deletions(-)
>  delete mode 100644 
> Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt
>  create mode 100644 
> Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.yaml
> 

Applied, thanks.

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


Re: [PATCH 00/16] dts/dt-bindings: Fix Arm Ltd. ARMv8 "boards"

2020-05-05 Thread André Przywara
On 05/05/2020 18:06, Robin Murphy wrote:
> On 2020-05-05 5:51 pm, Andre Przywara wrote:
>> Date: Mon, 4 May 2020 12:42:49 +0100
>> Subject: [PATCH 02/16] dt-bindings: arm-smmu: Allow mmu-400,smmu-v1
>> compatible
> 
> ^^ not sure how you managed that...

Impressive, huh? ;-)
Actually just an accidental empty line when adding Cc: lines to the
header (copy & paste with EOL). Using the previous subject line is
probably an artefact of how git send-email works.

Sorry for that!
I figured that replying would create more churn, as the actual subject
line is still in the email.

> 
>> The Arm SMMUv1 DT binding only allows combining arm,mmu-401 with
>> arm,smmu-v1, even though the MMU-400 is compatible as well.
>>
>> Allow this combination as well to let the Arm Juno board pass the test.
> 
> Acked-by: Robin Murphy 

Thanks!

Cheers,
Andre

> 
>> Signed-off-by: Andre Przywara 
>> ---
>>   Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
>> b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
>> index 6515dbe47508..e3ef1c69d132 100644
>> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
>> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
>> @@ -41,7 +41,9 @@ properties:
>>     - const: arm,mmu-500
>>     - const: arm,smmu-v2
>>     - items:
>> -  - const: arm,mmu-401
>> +  - enum:
>> +  - arm,mmu-400
>> +  - arm,mmu-401
>>     - const: arm,smmu-v1
>>     - enum:
>>     - arm,smmu-v1
>>

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

Re: [PATCH 00/16] dts/dt-bindings: Fix Arm Ltd. ARMv8 "boards"

2020-05-05 Thread Robin Murphy

On 2020-05-05 5:51 pm, Andre Przywara wrote:

Date: Mon, 4 May 2020 12:42:49 +0100
Subject: [PATCH 02/16] dt-bindings: arm-smmu: Allow mmu-400,smmu-v1 compatible


^^ not sure how you managed that...


The Arm SMMUv1 DT binding only allows combining arm,mmu-401 with
arm,smmu-v1, even though the MMU-400 is compatible as well.

Allow this combination as well to let the Arm Juno board pass the test.


Acked-by: Robin Murphy 


Signed-off-by: Andre Przywara 
---
  Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml 
b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
index 6515dbe47508..e3ef1c69d132 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
@@ -41,7 +41,9 @@ properties:
- const: arm,mmu-500
- const: arm,smmu-v2
- items:
-  - const: arm,mmu-401
+  - enum:
+  - arm,mmu-400
+  - arm,mmu-401
- const: arm,smmu-v1
- enum:
- arm,smmu-v1


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


[PATCH 00/16] dts/dt-bindings: Fix Arm Ltd. ARMv8 "boards"

2020-05-05 Thread Andre Przywara
Date: Mon, 4 May 2020 12:42:49 +0100
Subject: [PATCH 02/16] dt-bindings: arm-smmu: Allow mmu-400,smmu-v1 compatible

The Arm SMMUv1 DT binding only allows combining arm,mmu-401 with
arm,smmu-v1, even though the MMU-400 is compatible as well.

Allow this combination as well to let the Arm Juno board pass the test.

Signed-off-by: Andre Przywara 
---
 Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml 
b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
index 6515dbe47508..e3ef1c69d132 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
@@ -41,7 +41,9 @@ properties:
   - const: arm,mmu-500
   - const: arm,smmu-v2
   - items:
-  - const: arm,mmu-401
+  - enum:
+  - arm,mmu-400
+  - arm,mmu-401
   - const: arm,smmu-v1
   - enum:
   - arm,smmu-v1
-- 
2.17.1

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


Re: [PATCH] iommu: Relax ACS requirement for RCiEP devices.

2020-05-05 Thread Alex Williamson
On Tue, 5 May 2020 07:56:06 -0700
"Raj, Ashok"  wrote:

> On Tue, May 05, 2020 at 08:05:14AM -0600, Alex Williamson wrote:
> > On Mon, 4 May 2020 23:11:07 -0700
> > "Raj, Ashok"  wrote:
> >   
> > > Hi Alex
> > > 
> > > + Joerg, accidently missed in the Cc.
> > > 
> > > On Mon, May 04, 2020 at 11:19:36PM -0600, Alex Williamson wrote:  
> > > > On Mon,  4 May 2020 21:42:16 -0700
> > > > Ashok Raj  wrote:
> > > > 
> > > > > PCIe Spec recommends we can relax ACS requirement for RCIEP devices.
> > > > > 
> > > > > PCIe 5.0 Specification.
> > > > > 6.12 Access Control Services (ACS)
> > > > > Implementation of ACS in RCiEPs is permitted but not required. It is
> > > > > explicitly permitted that, within a single Root Complex, some RCiEPs
> > > > > implement ACS and some do not. It is strongly recommended that Root 
> > > > > Complex
> > > > > implementations ensure that all accesses originating from RCiEPs
> > > > > (PFs and VFs) without ACS capability are first subjected to 
> > > > > processing by
> > > > > the Translation Agent (TA) in the Root Complex before further 
> > > > > decoding and
> > > > > processing. The details of such Root Complex handling are outside the 
> > > > > scope
> > > > > of this specification.
> > > > >   
> > > > 
> > > > Is the language here really strong enough to make this change?  ACS is
> > > > an optional feature, so being permitted but not required is rather
> > > > meaningless.  The spec is also specifically avoiding the words "must"
> > > > or "shall" and even when emphasized with "strongly", we still only have
> > > > a recommendation that may or may not be honored.  This seems like a
> > > > weak basis for assuming that RCiEPs universally honor this
> > > > recommendation.  Thanks,
> > > > 
> > > 
> > > We are speaking about PCIe spec, where people write it about 5 years ahead
> > > and every vendor tries to massage their product behavior with vague
> > > words like this..  :)
> > > 
> > > But honestly for any any RCiEP, or even integrated endpoints, there 
> > > is no way to send them except up north. These aren't behind a RP.  
> > 
> > But they are multi-function devices and the spec doesn't define routing
> > within multifunction packages.  A single function RCiEP will already be
> > assumed isolated within its own group.  
> 
> That's right. The other two devices only have legacy PCI headers. So 
> they can't claim to be RCiEP's but just integrated endpoints. The legacy
> devices don't even have a PCIe header.
> 
> I honestly don't know why these are groped as MFD's in the first place.
> 
> >
> > > I did check with couple folks who are part of the SIG, and seem to agree
> > > that ACS treatment for RCiEP's doesn't mean much. 
> > > 
> > > I understand the language isn't strong, but it doesn't seem like ACS 
> > > should
> > > be a strong requirement for RCiEP's and reasonable to relax.
> > > 
> > > What are your thoughts?   
> > 
> > I think hardware vendors have ACS at their disposal to clarify when
> > isolation is provided, otherwise vendors can submit quirks, but I don't
> > see that the "strongly recommended" phrasing is sufficient to assume
> > isolation between multifunction RCiEPs.  Thanks,  
> 
> You point is that integrated MFD endpoints, without ACS, there is no 
> gaurantee to SW that they are isolated.
> 
> As far as a quirk, do you think:
>   - a cmdline optput for integrated endpoints, and RCiEP's suffice?
> along with a compile time default that is strict enforcement
>   - typical vid/did type exception list?
> 
> A more generic way to ask for exception would be scalable until we can stop
> those type of integrated devices. Or we need to maintain these device lists
> for eternity. 

I don't think the language in the spec is anything sufficient to handle
RCiEP uniquely.  We've previously rejected kernel command line opt-outs
for ACS, and the extent to which those patches still float around the
user community and are blindly used to separate IOMMU groups are a
testament to the failure of this approach.  Users do not have a basis
for enabling this sort of opt-out.  The benefit is obvious in the IOMMU
grouping, but the risk is entirely unknown.  A kconfig option is even
worse as that means if you consume a downstream kernel, the downstream
maintainers might have decided universally that isolation is less
important than functionality.

I think the only solution is that the hardware vendors need to step up
to indicate where devices are isolated.  The hardware can do this
itself by implementing ACS, otherwise we need quirks.  I think we've
also generally been reluctant to accept quirks that provide a blanket
opt-out for a vendor because doing so is akin to trying to predict the
future (determining the behavior of all current and previous hardware
is generally a sufficiently impossible task already).  Perhaps if a
vendor has a published internal policy regarding RCiEP isolation and is
willing to stand by a quirk, there might be roo

Re: [PATCH] iommu: Relax ACS requirement for RCiEP devices.

2020-05-05 Thread Raj, Ashok
On Tue, May 05, 2020 at 08:05:14AM -0600, Alex Williamson wrote:
> On Mon, 4 May 2020 23:11:07 -0700
> "Raj, Ashok"  wrote:
> 
> > Hi Alex
> > 
> > + Joerg, accidently missed in the Cc.
> > 
> > On Mon, May 04, 2020 at 11:19:36PM -0600, Alex Williamson wrote:
> > > On Mon,  4 May 2020 21:42:16 -0700
> > > Ashok Raj  wrote:
> > >   
> > > > PCIe Spec recommends we can relax ACS requirement for RCIEP devices.
> > > > 
> > > > PCIe 5.0 Specification.
> > > > 6.12 Access Control Services (ACS)
> > > > Implementation of ACS in RCiEPs is permitted but not required. It is
> > > > explicitly permitted that, within a single Root Complex, some RCiEPs
> > > > implement ACS and some do not. It is strongly recommended that Root 
> > > > Complex
> > > > implementations ensure that all accesses originating from RCiEPs
> > > > (PFs and VFs) without ACS capability are first subjected to processing 
> > > > by
> > > > the Translation Agent (TA) in the Root Complex before further decoding 
> > > > and
> > > > processing. The details of such Root Complex handling are outside the 
> > > > scope
> > > > of this specification.
> > > > 
> > > 
> > > Is the language here really strong enough to make this change?  ACS is
> > > an optional feature, so being permitted but not required is rather
> > > meaningless.  The spec is also specifically avoiding the words "must"
> > > or "shall" and even when emphasized with "strongly", we still only have
> > > a recommendation that may or may not be honored.  This seems like a
> > > weak basis for assuming that RCiEPs universally honor this
> > > recommendation.  Thanks,
> > >   
> > 
> > We are speaking about PCIe spec, where people write it about 5 years ahead
> > and every vendor tries to massage their product behavior with vague
> > words like this..  :)
> > 
> > But honestly for any any RCiEP, or even integrated endpoints, there 
> > is no way to send them except up north. These aren't behind a RP.
> 
> But they are multi-function devices and the spec doesn't define routing
> within multifunction packages.  A single function RCiEP will already be
> assumed isolated within its own group.

That's right. The other two devices only have legacy PCI headers. So 
they can't claim to be RCiEP's but just integrated endpoints. The legacy
devices don't even have a PCIe header.

I honestly don't know why these are groped as MFD's in the first place.

>  
> > I did check with couple folks who are part of the SIG, and seem to agree
> > that ACS treatment for RCiEP's doesn't mean much. 
> > 
> > I understand the language isn't strong, but it doesn't seem like ACS should
> > be a strong requirement for RCiEP's and reasonable to relax.
> > 
> > What are your thoughts? 
> 
> I think hardware vendors have ACS at their disposal to clarify when
> isolation is provided, otherwise vendors can submit quirks, but I don't
> see that the "strongly recommended" phrasing is sufficient to assume
> isolation between multifunction RCiEPs.  Thanks,

You point is that integrated MFD endpoints, without ACS, there is no 
gaurantee to SW that they are isolated.

As far as a quirk, do you think:
- a cmdline optput for integrated endpoints, and RCiEP's suffice?
  along with a compile time default that is strict enforcement
- typical vid/did type exception list?

A more generic way to ask for exception would be scalable until we can stop
those type of integrated devices. Or we need to maintain these device lists
for eternity. 

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


Re: [PATCH] iommu: Relax ACS requirement for RCiEP devices.

2020-05-05 Thread Alex Williamson
On Mon, 4 May 2020 23:11:07 -0700
"Raj, Ashok"  wrote:

> Hi Alex
> 
> + Joerg, accidently missed in the Cc.
> 
> On Mon, May 04, 2020 at 11:19:36PM -0600, Alex Williamson wrote:
> > On Mon,  4 May 2020 21:42:16 -0700
> > Ashok Raj  wrote:
> >   
> > > PCIe Spec recommends we can relax ACS requirement for RCIEP devices.
> > > 
> > > PCIe 5.0 Specification.
> > > 6.12 Access Control Services (ACS)
> > > Implementation of ACS in RCiEPs is permitted but not required. It is
> > > explicitly permitted that, within a single Root Complex, some RCiEPs
> > > implement ACS and some do not. It is strongly recommended that Root 
> > > Complex
> > > implementations ensure that all accesses originating from RCiEPs
> > > (PFs and VFs) without ACS capability are first subjected to processing by
> > > the Translation Agent (TA) in the Root Complex before further decoding and
> > > processing. The details of such Root Complex handling are outside the 
> > > scope
> > > of this specification.
> > > 
> > > Since Linux didn't give special treatment to allow this exception, certain
> > > RCiEP MFD devices are getting grouped in a single iommu group. This
> > > doesn't permit a single device to be assigned to a guest for instance.
> > > 
> > > In one vendor system: Device 14.x were grouped in a single IOMMU group.
> > > 
> > > /sys/kernel/iommu_groups/5/devices/:00:14.0
> > > /sys/kernel/iommu_groups/5/devices/:00:14.2
> > > /sys/kernel/iommu_groups/5/devices/:00:14.3
> > > 
> > > After the patch:
> > > /sys/kernel/iommu_groups/5/devices/:00:14.0
> > > /sys/kernel/iommu_groups/5/devices/:00:14.2
> > > /sys/kernel/iommu_groups/6/devices/:00:14.3 <<< new group
> > > 
> > > 14.0 and 14.2 are integrated devices, but legacy end points.
> > > Whereas 14.3 was a PCIe compliant RCiEP.
> > > 
> > > 00:14.3 Network controller: Intel Corporation Device 9df0 (rev 30)
> > > Capabilities: [40] Express (v2) Root Complex Integrated Endpoint, MSI 00
> > > 
> > > This permits assigning this device to a guest VM.
> > > 
> > > Fixes: f096c061f552 ("iommu: Rework iommu_group_get_for_pci_dev()")
> > > Signed-off-by: Ashok Raj 
> > > To: Joerg Roedel 
> > > To: Bjorn Helgaas 
> > > Cc: linux-ker...@vger.kernel.org
> > > Cc: iommu@lists.linux-foundation.org
> > > Cc: Lu Baolu 
> > > Cc: Alex Williamson 
> > > Cc: Darrel Goeddel 
> > > Cc: Mark Scott ,
> > > Cc: Romil Sharma 
> > > Cc: Ashok Raj 
> > > ---
> > >  drivers/iommu/iommu.c | 15 ++-
> > >  1 file changed, 14 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> > > index 2b471419e26c..5744bd65f3e2 100644
> > > --- a/drivers/iommu/iommu.c
> > > +++ b/drivers/iommu/iommu.c
> > > @@ -1187,7 +1187,20 @@ static struct iommu_group 
> > > *get_pci_function_alias_group(struct pci_dev *pdev,
> > >   struct pci_dev *tmp = NULL;
> > >   struct iommu_group *group;
> > >  
> > > - if (!pdev->multifunction || pci_acs_enabled(pdev, REQ_ACS_FLAGS))
> > > + /*
> > > +  * PCI Spec 5.0, Section 6.12 Access Control Service
> > > +  * Implementation of ACS in RCiEPs is permitted but not required.
> > > +  * It is explicitly permitted that, within a single Root
> > > +  * Complex, some RCiEPs implement ACS and some do not. It is
> > > +  * strongly recommended that Root Complex implementations ensure
> > > +  * that all accesses originating from RCiEPs (PFs and VFs) without
> > > +  * ACS capability are first subjected to processing by the Translation
> > > +  * Agent (TA) in the Root Complex before further decoding and
> > > +  * processing.
> > > +  */  
> > 
> > Is the language here really strong enough to make this change?  ACS is
> > an optional feature, so being permitted but not required is rather
> > meaningless.  The spec is also specifically avoiding the words "must"
> > or "shall" and even when emphasized with "strongly", we still only have
> > a recommendation that may or may not be honored.  This seems like a
> > weak basis for assuming that RCiEPs universally honor this
> > recommendation.  Thanks,
> >   
> 
> We are speaking about PCIe spec, where people write it about 5 years ahead
> and every vendor tries to massage their product behavior with vague
> words like this..  :)
> 
> But honestly for any any RCiEP, or even integrated endpoints, there 
> is no way to send them except up north. These aren't behind a RP.

But they are multi-function devices and the spec doesn't define routing
within multifunction packages.  A single function RCiEP will already be
assumed isolated within its own group.
 
> I did check with couple folks who are part of the SIG, and seem to agree
> that ACS treatment for RCiEP's doesn't mean much. 
> 
> I understand the language isn't strong, but it doesn't seem like ACS should
> be a strong requirement for RCiEP's and reasonable to relax.
> 
> What are your thoughts? 

I think hardware vendors have ACS at their disposal to clarify when
isolation is provided, otherwise vendors can submit quir

Re: [PATCH 0/5] iommu/amd: Fix race conditions around increase_address_space()

2020-05-05 Thread Joerg Roedel
On Mon, May 04, 2020 at 02:54:08PM +0200, Joerg Roedel wrote:
> Joerg Roedel (5):
>   iommu/amd: Fix race in increase_address_space()/fetch_pte()
>   iommu/amd: Do not loop forever when trying to increase address space
>   iommu/amd: Call domain_flush_complete() in update_domain()
>   iommu/amd: Update Device Table in increase_address_space()
>   iommu/amd: Do not flush Device Table in iommu_map_page()
> 
>  drivers/iommu/amd_iommu.c   | 198 +---
>  drivers/iommu/amd_iommu_types.h |   9 +-
>  2 files changed, 161 insertions(+), 46 deletions(-)

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


Re: [PATCH v3 00/34] iommu: Move iommu_group setup to IOMMU core code

2020-05-05 Thread Joerg Roedel
On Wed, Apr 29, 2020 at 03:36:38PM +0200, Joerg Roedel wrote:
> Please review. If there are no objections I plan to put these patches
> into the IOMMU tree early next week.

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


Re: [PATCH v3 02/25] drm: core: fix common struct sg_table related issues

2020-05-05 Thread Christoph Hellwig
On Tue, May 05, 2020 at 12:51:58PM +0200, Marek Szyprowski wrote:
> Hi Christoph,
> 
> On 05.05.2020 12:15, Christoph Hellwig wrote:
> >> -  for_each_sg_page(st->sgl, &sg_iter, st->nents, 0)
> >> +  for_each_sg_page(st->sgl, &sg_iter, st->orig_nents, 0)
> > Would it make sense to also add a for_each_sgtable_page helper that
> > hides the use of orig_nents?  To be used like:
> >
> > for_each_sgtable_page(st, &sg_iter, 0) {
> 
> We would need two helpers:
> 
> for_each_sgtable_cpu_page() and for_each_sgtable_dma_page().
> 
> I considered them, but then I found that there are already 
> for_each_sg_page(), for_each_sg_dma_page() and various special iterators 
> like sg_page_iter, sg_dma_page_iter and sg_mapping_iter. Too bad that 
> they are almost not used, at least in the DRM subsystem. I wonder if it 
> make sense to apply them or simply provide the two above mentioned 
> wrappers?

None of the helpers helps with passing the right parameters from the
sg_table.  So in doube we'd need wrappers for all of the above, or
none..
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v3 02/25] drm: core: fix common struct sg_table related issues

2020-05-05 Thread Marek Szyprowski
Hi Christoph,

On 05.05.2020 12:15, Christoph Hellwig wrote:
>> -for_each_sg_page(st->sgl, &sg_iter, st->nents, 0)
>> +for_each_sg_page(st->sgl, &sg_iter, st->orig_nents, 0)
> Would it make sense to also add a for_each_sgtable_page helper that
> hides the use of orig_nents?  To be used like:
>
>   for_each_sgtable_page(st, &sg_iter, 0) {

We would need two helpers:

for_each_sgtable_cpu_page() and for_each_sgtable_dma_page().

I considered them, but then I found that there are already 
for_each_sg_page(), for_each_sg_dma_page() and various special iterators 
like sg_page_iter, sg_dma_page_iter and sg_mapping_iter. Too bad that 
they are almost not used, at least in the DRM subsystem. I wonder if it 
make sense to apply them or simply provide the two above mentioned 
wrappers?

>> +for_each_sg(sgt->sgl, sg, sgt->orig_nents, count) {
> Same here, e.g.
>
>   for_each_sgtable_entry(sgt, sg, count) {
>
> ?
>
Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

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


Re: [PATCH v3 01/25] dma-mapping: add generic helpers for mapping sgtable objects

2020-05-05 Thread Marek Szyprowski
Hi Christoph,

On 05.05.2020 12:22, Christoph Hellwig wrote:
>> +static inline int dma_map_sgtable_attrs(struct device *dev,
>> +struct sg_table *sgt, enum dma_data_direction dir, unsigned long attrs)
> Two tab indents for parameter continuation, please.
>
> Can we also skip the separate _attrs version?  The existing ones have the
> separate _attrs variant as there were pre-existing versions without the
> attrs argument and lots of users, but that doesn't really apply here as
> an extra 0 argument isn't really an issue.

Okay.

>> +static inline size_t iommu_map_sgtable(struct iommu_domain *domain,
>> +unsigned long iova, struct sg_table *sgt, int prot)
>> +{
>> +return iommu_map_sg(domain, iova, sgt->sgl, sgt->orig_nents, prot);
>> +}
> Should this be a separate patch due to the different subsystems?
>
> FYI, I'll happily pick up the prep patches in an immutable branch of
> the dma-mapping tree one we have settled on the details.

Okay.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

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


Re: [PATCH v3 01/25] dma-mapping: add generic helpers for mapping sgtable objects

2020-05-05 Thread Christoph Hellwig
> +static inline int dma_map_sgtable_attrs(struct device *dev,
> + struct sg_table *sgt, enum dma_data_direction dir, unsigned long attrs)

Two tab indents for parameter continuation, please.

Can we also skip the separate _attrs version?  The existing ones have the
separate _attrs variant as there were pre-existing versions without the
attrs argument and lots of users, but that doesn't really apply here as
an extra 0 argument isn't really an issue.

> +static inline size_t iommu_map_sgtable(struct iommu_domain *domain,
> + unsigned long iova, struct sg_table *sgt, int prot)
> +{
> + return iommu_map_sg(domain, iova, sgt->sgl, sgt->orig_nents, prot);
> +}

Should this be a separate patch due to the different subsystems?

FYI, I'll happily pick up the prep patches in an immutable branch of
the dma-mapping tree one we have settled on the details.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v3 02/25] drm: core: fix common struct sg_table related issues

2020-05-05 Thread Christoph Hellwig
> - for_each_sg_page(st->sgl, &sg_iter, st->nents, 0)
> + for_each_sg_page(st->sgl, &sg_iter, st->orig_nents, 0)

Would it make sense to also add a for_each_sgtable_page helper that
hides the use of orig_nents?  To be used like:

for_each_sgtable_page(st, &sg_iter, 0) {

> + for_each_sg(sgt->sgl, sg, sgt->orig_nents, count) {

Same here, e.g.

for_each_sgtable_entry(sgt, sg, count) {

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


[PATCH v3 5/5] drm/sun4i: mixer: Call of_dma_configure if there's an IOMMU

2020-05-05 Thread Maxime Ripard
The main DRM device is actually a virtual device so it doesn't have the
iommus property, which is instead on the DMA masters, in this case the
mixers.

Add a call to of_dma_configure with the mixers DT node but on the DRM
virtual device to configure it in the same way than the mixers.

Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/sun4i/sun8i_mixer.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c 
b/drivers/gpu/drm/sun4i/sun8i_mixer.c
index 4a64f7ae437a..19b3b4184704 100644
--- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
@@ -452,6 +452,19 @@ static int sun8i_mixer_bind(struct device *dev, struct 
device *master,
mixer->engine.ops = &sun8i_engine_ops;
mixer->engine.node = dev->of_node;
 
+   if (of_find_property(dev->of_node, "iommus", NULL)) {
+   /*
+* This assume we have the same DMA constraints for
+* all our the mixers in our pipeline. This sounds
+* bad, but it has always been the case for us, and
+* DRM doesn't do per-device allocation either, so we
+* would need to fix DRM first...
+*/
+   ret = of_dma_configure(drm->dev, dev->of_node, true);
+   if (ret)
+   return ret;
+   }
+
/*
 * While this function can fail, we shouldn't do anything
 * if this happens. Some early DE2 DT entries don't provide
-- 
git-series 0.9.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH v3 0/5] iommu: Add Allwinner H6 IOMMU driver

2020-05-05 Thread Maxime Ripard
Hi,

Here's a series adding support for the IOMMU introduced in the Allwinner
H6. The driver from Allwinner hints at more SoCs using it in the future
(with more masters), so we can bet on that IOMMU becoming pretty much
standard in new SoCs from Allwinner.

One thing I wasn't really sure about was how to expose the statistics
reported by the IOMMU PMU (TLB hit rates, latencies, and so on). The
Allwinner driver exposes them through custom sysfs files, while they would
be best represented through perf I guess? Anyway, I'm planning to support
them later on.

Let me know what you think,
Maxime

Changes from v2:
  - Rebased on 5.7
  - Add dt bindings patch
  - Allow the identity domain to be allocated
  - Add an unlikely to the check on whether a PTE already exists in map
  - Remove locking and adjust the PT installation to use an atomic
operation instead
  - Switch to iotlb_sync / flush_iotlb_all callback instead of flushing by
ourselves.

Changes from v1:
  - Add a patch to configure the IOMMU on the virtual DRM device
  - Rework the domain allocation / freeing
  - Remove the runtime_pm handling to power up the device and rely on
refcounting
  - use map gfp argument for kmem cache allocation
  - Removed unused macros
  - Switched from BIT(0) to 1 for the page table entry valid flag to make
it more obvious that it's over multiple bits.
  - Switch to module_initcall
  - Make accesses to the fwspec more consistant
  - Removed dev_info logs
  - Reworked invalidation / flushing
  - Allow for compilation with COMPILE_TEST

Maxime Ripard (5):
  dt-bindings: iommu: Add Allwinner H6 IOMMU bindings
  dt-bindings: display: sun8i-mixer: Allow for an iommu property
  iommu: Add Allwinner H6 IOMMU driver
  arm64: dts: allwinner: h6: Add IOMMU
  drm/sun4i: mixer: Call of_dma_configure if there's an IOMMU

 Documentation/devicetree/bindings/display/allwinner,sun8i-a83t-de2-mixer.yaml 
|3 +-
 Documentation/devicetree/bindings/iommu/allwinner,sun50i-h6-iommu.yaml
|   61 -
 arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi  
|   10 +-
 drivers/gpu/drm/sun4i/sun8i_mixer.c   
|   13 +-
 drivers/iommu/Kconfig 
|9 +-
 drivers/iommu/Makefile
|1 +-
 drivers/iommu/sun50i-iommu.c  
| 1065 -
 7 files changed, 1162 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/iommu/allwinner,sun50i-h6-iommu.yaml
 create mode 100644 drivers/iommu/sun50i-iommu.c

base-commit: 8f3d9f354286745c751374f5f1fcafee6b3f3136
-- 
git-series 0.9.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH v3 3/5] iommu: Add Allwinner H6 IOMMU driver

2020-05-05 Thread Maxime Ripard
The Allwinner H6 has introduced an IOMMU for a few DMA controllers, mostly
video related: the display engine, the video decoders / encoders, the
camera capture controller, etc.

The design is pretty simple compared to other IOMMUs found in SoCs: there's
a single instance, controlling all the masters, with a single address
space.

It also features a performance monitoring unit that allows to retrieve
various informations (per-master and global TLB accesses, hits and misses,
access latency, etc) that isn't supported at the moment.

Signed-off-by: Maxime Ripard 
---
 drivers/iommu/Kconfig|9 +-
 drivers/iommu/Makefile   |1 +-
 drivers/iommu/sun50i-iommu.c | 1065 +++-
 3 files changed, 1075 insertions(+)
 create mode 100644 drivers/iommu/sun50i-iommu.c

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 58b4a4dbfc78..6e80b0ea7228 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -303,6 +303,15 @@ config ROCKCHIP_IOMMU
  Say Y here if you are using a Rockchip SoC that includes an IOMMU
  device.
 
+config SUN50I_IOMMU
+   bool "Allwinner H6 IOMMU Support"
+   depends on ARCH_SUNXI || COMPILE_TEST
+   select ARM_DMA_USE_IOMMU
+   select IOMMU_API
+   select IOMMU_DMA
+   help
+ Support for the IOMMU introduced in the Allwinner H6 SoCs.
+
 config TEGRA_IOMMU_GART
bool "Tegra GART IOMMU Support"
depends on ARCH_TEGRA_2x_SOC
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index 9f33fdb3bb05..57cf4ba5e27c 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_MTK_IOMMU_V1) += mtk_iommu_v1.o
 obj-$(CONFIG_OMAP_IOMMU) += omap-iommu.o
 obj-$(CONFIG_OMAP_IOMMU_DEBUG) += omap-iommu-debug.o
 obj-$(CONFIG_ROCKCHIP_IOMMU) += rockchip-iommu.o
+obj-$(CONFIG_SUN50I_IOMMU) += sun50i-iommu.o
 obj-$(CONFIG_TEGRA_IOMMU_GART) += tegra-gart.o
 obj-$(CONFIG_TEGRA_IOMMU_SMMU) += tegra-smmu.o
 obj-$(CONFIG_EXYNOS_IOMMU) += exynos-iommu.o
diff --git a/drivers/iommu/sun50i-iommu.c b/drivers/iommu/sun50i-iommu.c
new file mode 100644
index ..d4c3fa2f0f82
--- /dev/null
+++ b/drivers/iommu/sun50i-iommu.c
@@ -0,0 +1,1065 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+// Copyright (C) 2016-2018, Allwinner Technology CO., LTD.
+// Copyright (C) 2019-2020, Cerno
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define IOMMU_RESET_REG0x010
+#define IOMMU_ENABLE_REG   0x020
+#define IOMMU_ENABLE_ENABLEBIT(0)
+
+#define IOMMU_BYPASS_REG   0x030
+#define IOMMU_AUTO_GATING_REG  0x040
+#define IOMMU_AUTO_GATING_ENABLE   BIT(0)
+
+#define IOMMU_WBUF_CTRL_REG0x044
+#define IOMMU_OOO_CTRL_REG 0x048
+#define IOMMU_4KB_BDY_PRT_CTRL_REG 0x04c
+#define IOMMU_TTB_REG  0x050
+#define IOMMU_TLB_ENABLE_REG   0x060
+#define IOMMU_TLB_PREFETCH_REG 0x070
+#define IOMMU_TLB_PREFETCH_MASTER_ENABLE(m)BIT(m)
+
+#define IOMMU_TLB_FLUSH_REG0x080
+#define IOMMU_TLB_FLUSH_PTW_CACHE  BIT(17)
+#define IOMMU_TLB_FLUSH_MACRO_TLB  BIT(16)
+#define IOMMU_TLB_FLUSH_MICRO_TLB(i)   (BIT(i) & GENMASK(5, 0))
+
+#define IOMMU_TLB_IVLD_ADDR_REG0x090
+#define IOMMU_TLB_IVLD_ADDR_MASK_REG   0x094
+#define IOMMU_TLB_IVLD_ENABLE_REG  0x098
+#define IOMMU_TLB_IVLD_ENABLE_ENABLE   BIT(0)
+
+#define IOMMU_PC_IVLD_ADDR_REG 0x0a0
+#define IOMMU_PC_IVLD_ENABLE_REG   0x0a8
+#define IOMMU_PC_IVLD_ENABLE_ENABLEBIT(0)
+
+#define IOMMU_DM_AUT_CTRL_REG(d)   (0x0b0 + ((d) / 2) * 4)
+#define IOMMU_DM_AUT_CTRL_RD_UNAVAIL(d, m) (1 << (((d & 1) * 16) + ((m) * 
2)))
+#define IOMMU_DM_AUT_CTRL_WR_UNAVAIL(d, m) (1 << (((d & 1) * 16) + ((m) * 
2) + 1))
+
+#define IOMMU_DM_AUT_OVWT_REG  0x0d0
+#define IOMMU_INT_ENABLE_REG   0x100
+#define IOMMU_INT_CLR_REG  0x104
+#define IOMMU_INT_STA_REG  0x108
+#define IOMMU_INT_ERR_ADDR_REG(i)  (0x110 + (i) * 4)
+#define IOMMU_INT_ERR_ADDR_L1_REG  0x130
+#define IOMMU_INT_ERR_ADDR_L2_REG  0x134
+#define IOMMU_INT_ERR_DATA_REG(i)  (0x150 + (i) * 4)
+#define IOMMU_L1PG_INT_REG 0x0180
+#define IOMMU_L2PG_INT_REG 0x0184
+
+#define IOMMU_INT_INVALID_L2PG BIT(17)
+#define IOMMU_INT_INVALID_L1PG BIT(16)
+#define IOMMU_INT_MASTER_PERMISSION(m) BIT(m)
+#define IOMMU_INT_MASTER_MASK  (IOMMU_INT_MASTER_PERMISSION(0) 
| \
+IOMMU_INT_MASTER_PERMISSION(1) 
| \
+IOMM

[PATCH v3 2/5] dt-bindings: display: sun8i-mixer: Allow for an iommu property

2020-05-05 Thread Maxime Ripard
The H6 mixer is attached to an IOMMU, so let's allow that property to be
set in the bindings.

Signed-off-by: Maxime Ripard 
---
 Documentation/devicetree/bindings/display/allwinner,sun8i-a83t-de2-mixer.yaml 
| 3 +++
 1 file changed, 3 insertions(+)

diff --git 
a/Documentation/devicetree/bindings/display/allwinner,sun8i-a83t-de2-mixer.yaml 
b/Documentation/devicetree/bindings/display/allwinner,sun8i-a83t-de2-mixer.yaml
index 1dee641e3ea1..c040eef56518 100644
--- 
a/Documentation/devicetree/bindings/display/allwinner,sun8i-a83t-de2-mixer.yaml
+++ 
b/Documentation/devicetree/bindings/display/allwinner,sun8i-a83t-de2-mixer.yaml
@@ -36,6 +36,9 @@ properties:
   - const: bus
   - const: mod
 
+  iommus:
+maxItems: 1
+
   resets:
 maxItems: 1
 
-- 
git-series 0.9.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH v3 1/5] dt-bindings: iommu: Add Allwinner H6 IOMMU bindings

2020-05-05 Thread Maxime Ripard
The Allwinner H6 has introduced an IOMMU. Let's add a device tree binding
for it.

Reviewed-by: Rob Herring 
Signed-off-by: Maxime Ripard 
---
 Documentation/devicetree/bindings/iommu/allwinner,sun50i-h6-iommu.yaml | 61 
+
 1 file changed, 61 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/iommu/allwinner,sun50i-h6-iommu.yaml

diff --git 
a/Documentation/devicetree/bindings/iommu/allwinner,sun50i-h6-iommu.yaml 
b/Documentation/devicetree/bindings/iommu/allwinner,sun50i-h6-iommu.yaml
new file mode 100644
index ..5e125cf2a88b
--- /dev/null
+++ b/Documentation/devicetree/bindings/iommu/allwinner,sun50i-h6-iommu.yaml
@@ -0,0 +1,61 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iommu/allwinner,sun50i-h6-iommu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Allwinner H6 IOMMU Device Tree Bindings
+
+maintainers:
+  - Chen-Yu Tsai 
+  - Maxime Ripard 
+
+properties:
+  "#iommu-cells":
+const: 1
+description:
+  The content of the cell is the master ID.
+
+  compatible:
+const: allwinner,sun50i-h6-iommu
+
+  reg:
+maxItems: 1
+
+  interrupts:
+maxItems: 1
+
+  clocks:
+maxItems: 1
+
+  resets:
+maxItems: 1
+
+required:
+  - "#iommu-cells"
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - resets
+
+additionalProperties: false
+
+examples:
+  - |
+  #include 
+  #include 
+
+  #include 
+  #include 
+
+  iommu: iommu@30f {
+  compatible = "allwinner,sun50i-h6-iommu";
+  reg = <0x030f 0x1>;
+  interrupts = ;
+  clocks = <&ccu CLK_BUS_IOMMU>;
+  resets = <&ccu RST_BUS_IOMMU>;
+  #iommu-cells = <1>;
+  };
+
+...
-- 
git-series 0.9.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH v3 4/5] arm64: dts: allwinner: h6: Add IOMMU

2020-05-05 Thread Maxime Ripard
Now that we have a driver for the IOMMU, let's start using it.

Signed-off-by: Maxime Ripard 
---
 arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi 
b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
index b9ab7d8fa8af..bba64a4030e2 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
@@ -123,6 +123,7 @@
clock-names = "bus",
  "mod";
resets = <&display_clocks RST_MIXER0>;
+   iommus = <&iommu 0>;
 
ports {
#address-cells = <1>;
@@ -387,6 +388,15 @@
#interrupt-cells = <3>;
};
 
+   iommu: iommu@30f {
+   compatible = "allwinner,sun50i-h6-iommu";
+   reg = <0x030f 0x1>;
+   interrupts = ;
+   clocks = <&ccu CLK_BUS_IOMMU>;
+   resets = <&ccu RST_BUS_IOMMU>;
+   #iommu-cells = <1>;
+   };
+
mmc0: mmc@402 {
compatible = "allwinner,sun50i-h6-mmc",
 "allwinner,sun50i-a64-mmc";
-- 
git-series 0.9.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2] dma: Fix max PFN arithmetic overflow on 32 bit systems

2020-05-05 Thread Alexander Dahl
Hei hei,

I would like to kindly ask about the status of this patch.

On Wed, Apr 15, 2020 at 04:35:21PM +0200, Alexander Dahl wrote:
> now after v5.7-rc1 is out, I would kindly ask, if anyone had time to
> review this one line patch? Is anything wrong with that fix?

Did it maybe not reach the right maintainers? I used
scripts/get_maintainer.pl to get my recipient list.

> (I added the current fli4l kernel package maintainer Florian to Cc to
> let him know I'm still having an eye on this.)
> 
> Greets
> Alex
> 
> On Sat, Mar 21, 2020 at 07:28:23PM +0100, Alexander Dahl wrote:
> > For ARCH=x86 (32 bit) when you set CONFIG_IOMMU_INTEL since c5a5dc4cbbf4
> > ("iommu/vt-d: Don't switch off swiotlb if bounce page is used") there's
> > a dependency on CONFIG_SWIOTLB, which was not necessarily active before.
> > 
> > The init code for swiotlb in 'pci_swiotlb_detect_4gb()' compares
> > something against MAX_DMA32_PFN to decide if it should be active.
> > However that define suffers from an arithmetic overflow since
> > 1b7e03ef7570 ("x86, NUMA: Enable emulation on 32bit too") when it was
> > first made visible to x86_32.
> > 
> > The effect is at boot time 64 MiB (default size) were allocated for
> > bounce buffers now, which is a noticeable amount of memory on small
> > systems. We noticed this effect on the fli4l Linux distribution when
> > migrating from kernel v4.19 (LTS) to v5.4 (LTS) on boards like pcengines
> > ALIX 2D3 with 256 MiB memory for example:
> > 
> >   Linux version 5.4.22 (buildroot@buildroot) (gcc version 7.3.0 (Buildroot 
> > 2018.02.8)) #1 SMP Mon Nov 26 23:40:00 CET 2018
> >   …
> >   Memory: 183484K/261756K available (4594K kernel code, 393K rwdata, 1660K 
> > rodata, 536K init, 456K bss , 78272K reserved, 0K cma-reserved, 0K highmem)
> >   …
> >   PCI-DMA: Using software bounce buffering for IO (SWIOTLB)
> >   software IO TLB: mapped [mem 0x0bb78000-0x0fb78000] (64MB)
> > 
> > The initial analysis and the suggested fix was done by user 'sourcejedi'
> > at stackoverflow and explicitly marked as GPLv2 for inclusion in the
> > Linux kernel:
> > 
> >   https://unix.stackexchange.com/a/520525/50007
> > 
> > The actual calculation however is the same as for arch/mips now as
> > suggested by Robin Murphy.
> > 
> > Fixes: https://web.nettworks.org/bugs/browse/FFL-2560
> > Fixes: https://unix.stackexchange.com/q/520065/50007
> > Reported-by: Alan Jenkins 
> > Suggested-by: Robin Murphy 
> > Signed-off-by: Alexander Dahl 
> > ---
> > 
> > Notes:
> > v1 -> v2:
> >   - use the same calculation as with arch/mips (Robin Murphy)
> > 
> >  arch/x86/include/asm/dma.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/x86/include/asm/dma.h b/arch/x86/include/asm/dma.h
> > index 00f7cf45e699..8e95aa4b0d17 100644
> > --- a/arch/x86/include/asm/dma.h
> > +++ b/arch/x86/include/asm/dma.h
> > @@ -74,7 +74,7 @@
> >  #define MAX_DMA_PFN   ((16UL * 1024 * 1024) >> PAGE_SHIFT)
> >  
> >  /* 4GB broken PCI/AGP hardware bus master zone */
> > -#define MAX_DMA32_PFN ((4UL * 1024 * 1024 * 1024) >> PAGE_SHIFT)
> > +#define MAX_DMA32_PFN (1UL << (32 - PAGE_SHIFT))
> >  
> >  #ifdef CONFIG_X86_32
> >  /* The maximum address that we can perform a DMA transfer to on this 
> > platform */
> > -- 
> > 2.20.1

Greets
Alex

-- 
/"\ ASCII RIBBON | »With the first link, the chain is forged. The first
\ / CAMPAIGN | speech censured, the first thought forbidden, the
 X  AGAINST  | first freedom denied, chains us all irrevocably.«
/ \ HTML MAIL| (Jean-Luc Picard, quoting Judge Aaron Satie)


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

[PATCH v5] iommu/virtio: Use page size bitmap supported by endpoint

2020-05-05 Thread Bharat Bhushan
Different endpoint can support different page size, probe
endpoint if it supports specific page size otherwise use
global page sizes.

Signed-off-by: Bharat Bhushan 
---
v4->v5:
 - Rebase to Linux v5.7-rc4

v3->v4:
 - Fix whitespace error

v2->v3:
 - Fixed error return for incompatible endpoint
 - __u64 changed to __le64 in header file

 drivers/iommu/virtio-iommu.c  | 48 ---
 include/uapi/linux/virtio_iommu.h |  7 +
 2 files changed, 51 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index d5cac4f46ca5..9513d2ab819e 100644
--- a/drivers/iommu/virtio-iommu.c
+++ b/drivers/iommu/virtio-iommu.c
@@ -78,6 +78,7 @@ struct viommu_endpoint {
struct viommu_dev   *viommu;
struct viommu_domain*vdomain;
struct list_headresv_regions;
+   u64 pgsize_bitmap;
 };
 
 struct viommu_request {
@@ -415,6 +416,19 @@ static int viommu_replay_mappings(struct viommu_domain 
*vdomain)
return ret;
 }
 
+static int viommu_set_pgsize_bitmap(struct viommu_endpoint *vdev,
+   struct virtio_iommu_probe_pgsize_mask *mask,
+   size_t len)
+{
+   u64 pgsize_bitmap = le64_to_cpu(mask->pgsize_bitmap);
+
+   if (len < sizeof(*mask))
+   return -EINVAL;
+
+   vdev->pgsize_bitmap = pgsize_bitmap;
+   return 0;
+}
+
 static int viommu_add_resv_mem(struct viommu_endpoint *vdev,
   struct virtio_iommu_probe_resv_mem *mem,
   size_t len)
@@ -499,6 +513,9 @@ static int viommu_probe_endpoint(struct viommu_dev *viommu, 
struct device *dev)
case VIRTIO_IOMMU_PROBE_T_RESV_MEM:
ret = viommu_add_resv_mem(vdev, (void *)prop, len);
break;
+   case VIRTIO_IOMMU_PROBE_T_PAGE_SIZE_MASK:
+   ret = viommu_set_pgsize_bitmap(vdev, (void *)prop, len);
+   break;
default:
dev_err(dev, "unknown viommu prop 0x%x\n", type);
}
@@ -630,7 +647,7 @@ static int viommu_domain_finalise(struct viommu_endpoint 
*vdev,
 
vdomain->id = (unsigned int)ret;
 
-   domain->pgsize_bitmap   = viommu->pgsize_bitmap;
+   domain->pgsize_bitmap   = vdev->pgsize_bitmap;
domain->geometry= viommu->geometry;
 
vdomain->map_flags  = viommu->map_flags;
@@ -654,6 +671,29 @@ static void viommu_domain_free(struct iommu_domain *domain)
kfree(vdomain);
 }
 
+/*
+ * Check whether the endpoint's capabilities are compatible with other
+ * endpoints in the domain. Report any inconsistency.
+ */
+static bool viommu_endpoint_is_compatible(struct viommu_endpoint *vdev,
+ struct viommu_domain *vdomain)
+{
+   struct device *dev = vdev->dev;
+
+   if (vdomain->viommu != vdev->viommu) {
+   dev_err(dev, "cannot attach to foreign vIOMMU\n");
+   return false;
+   }
+
+   if (vdomain->domain.pgsize_bitmap != vdev->pgsize_bitmap) {
+   dev_err(dev, "incompatible domain bitmap 0x%lx != 0x%llx\n",
+   vdomain->domain.pgsize_bitmap, vdev->pgsize_bitmap);
+   return false;
+   }
+
+   return true;
+}
+
 static int viommu_attach_dev(struct iommu_domain *domain, struct device *dev)
 {
int i;
@@ -670,9 +710,8 @@ static int viommu_attach_dev(struct iommu_domain *domain, 
struct device *dev)
 * owns it.
 */
ret = viommu_domain_finalise(vdev, domain);
-   } else if (vdomain->viommu != vdev->viommu) {
-   dev_err(dev, "cannot attach to foreign vIOMMU\n");
-   ret = -EXDEV;
+   } else if (!viommu_endpoint_is_compatible(vdev, vdomain)) {
+   ret = -EINVAL;
}
mutex_unlock(&vdomain->mutex);
 
@@ -886,6 +925,7 @@ static int viommu_add_device(struct device *dev)
 
vdev->dev = dev;
vdev->viommu = viommu;
+   vdev->pgsize_bitmap = viommu->pgsize_bitmap;
INIT_LIST_HEAD(&vdev->resv_regions);
dev_iommu_priv_set(dev, vdev);
 
diff --git a/include/uapi/linux/virtio_iommu.h 
b/include/uapi/linux/virtio_iommu.h
index 48e3c29223b5..2cced7accc99 100644
--- a/include/uapi/linux/virtio_iommu.h
+++ b/include/uapi/linux/virtio_iommu.h
@@ -111,6 +111,7 @@ struct virtio_iommu_req_unmap {
 
 #define VIRTIO_IOMMU_PROBE_T_NONE  0
 #define VIRTIO_IOMMU_PROBE_T_RESV_MEM  1
+#define VIRTIO_IOMMU_PROBE_T_PAGE_SIZE_MASK2
 
 #define VIRTIO_IOMMU_PROBE_T_MASK  0xfff
 
@@ -119,6 +120,12 @@ struct virtio_iommu_probe_property {
__le16  length;
 };
 
+struct virtio_iommu_probe_pgsize_mask {
+   struct virtio_iommu_probe_property  h

Re: [PATCH v3 18/25] drm: rcar-du: fix common struct sg_table related issues

2020-05-05 Thread Geert Uytterhoeven
Hi Marek,

On Tue, May 5, 2020 at 10:48 AM Marek Szyprowski
 wrote:
> The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
> numer of the created entries in the DMA address space. However the
> subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
> called with the original number of the entries passed to dma_map_sg. The
> sg_table->nents in turn holds the result of the dma_map_sg call as stated
> in include/linux/scatterlist.h. A common mistake was to ignore a result
> of the dma_map_sg function and don't use the sg_table->orig_nents at all.
>
> To avoid such issues, lets use common dma-mapping wrappers operating
> directly on the struct sg_table objects and adjust references to the
> nents and orig_nents respectively.
>
> Signed-off-by: Marek Szyprowski 
> ---
> For more information, see '[PATCH v3 00/25] DRM: fix struct sg_table nents
> vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/5/187

For the modern lore-users:
https://lore.kernel.org/r/20200505083926.28503-1-m.szyprow...@samsung.com/

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v6 17/25] iommu/arm-smmu-v3: Implement iommu_sva_bind/unbind()

2020-05-05 Thread Jean-Philippe Brucker
On Mon, May 04, 2020 at 01:47:23PM -0700, Jacob Pan wrote:
> > > > +   arm_smmu_write_ctx_desc(smmu_domain, mm->pasid,
> > > > &invalid_cd); +
> > > > +   arm_smmu_tlb_inv_asid(smmu_domain->smmu,
> > > > smmu_mn->cd->asid);
> > > > +   /* TODO: invalidate ATS */
> > > > +  
> > > If mm release is called after tlb invalidate range, is it still
> > > necessary to invalidate again?  
> > 
> > No, provided all mappings from the address space are unmapped and
> > invalidated. I'll double check, but in my tests invalidate range
> > didn't seem to be called for all mappings on mm exit, so I believe we
> > do need this.
> > 
> I think it is safe to invalidate again. There was a concern that mm
> release may delete IOMMU driver from the notification list and miss tlb
> invalidate range. I had a hard time to confirm that with ftrace while
> killing a process, many lost events.
> 

If it helps, I have a test that generates small DMA transactions on a SMMU
model. This is the trace for a job on a 8kB mmap'd buffer:

  smmu_bind_alloc: dev=:00:03.0 pasid=1
  dev_fault: IOMMU::00:03.0 type=2 reason=0 addr=0x860e6000 pasid=1 
group=74 flags=3 prot=2
  dev_page_response: IOMMU::00:03.0 code=0 pasid=1 group=74
  dev_fault: IOMMU::00:03.0 type=2 reason=0 addr=0x860e7000 pasid=1 
group=143 flags=3 prot=2
  dev_page_response: IOMMU::00:03.0 code=0 pasid=1 group=143
  smmu_mm_invalidate: pasid=1 start=0x860e6000 end=0x860e8000
  smmu_mm_invalidate: pasid=1 start=0x860e6000 end=0x860e8000
  smmu_mm_invalidate: pasid=1 start=0x860e8000 end=0x860ea000
  smmu_mm_invalidate: pasid=1 start=0x860e8000 end=0x860ea000
  smmu_unbind_free: dev=:00:03.0 pasid=1

And this is the same job, but the process immediately kills itself after
launching it.

  smmu_bind_alloc: dev=:00:03.0 pasid=1
  dev_fault: IOMMU::00:03.0 type=2 reason=0 addr=0xb9d15000 pasid=1 
group=259 flags=3 prot=2
  smmu_mm_release: pasid=1
  dev_page_response: IOMMU::00:03.0 code=0 pasid=1 group=259
  dev_fault: IOMMU::00:03.0 type=2 reason=0 addr=0xb9d15000 pasid=1 
group=383 flags=3 prot=2
  dev_page_response: IOMMU::00:03.0 code=1 pasid=1 group=383
  smmu_unbind_free: dev=:00:03.0 pasid=1

We don't get any invalidate_range notification in this case.

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


[PATCH v3 18/25] drm: rcar-du: fix common struct sg_table related issues

2020-05-05 Thread Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of the entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. A common mistake was to ignore a result
of the dma_map_sg function and don't use the sg_table->orig_nents at all.

To avoid such issues, lets use common dma-mapping wrappers operating
directly on the struct sg_table objects and adjust references to the
nents and orig_nents respectively.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v3 00/25] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/5/187
---
 drivers/media/platform/vsp1/vsp1_drm.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_drm.c 
b/drivers/media/platform/vsp1/vsp1_drm.c
index a4a45d6..8a2624b 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -912,8 +912,9 @@ int vsp1_du_map_sg(struct device *dev, struct sg_table *sgt)
 * skip cache sync. This will need to be revisited when support for
 * non-coherent buffers will be added to the DU driver.
 */
-   return dma_map_sg_attrs(vsp1->bus_master, sgt->sgl, sgt->nents,
-   DMA_TO_DEVICE, DMA_ATTR_SKIP_CPU_SYNC);
+   return dma_map_sgtable_attrs(vsp1->bus_master, sgt, DMA_TO_DEVICE,
+DMA_ATTR_SKIP_CPU_SYNC);
+   return sgt->nents;
 }
 EXPORT_SYMBOL_GPL(vsp1_du_map_sg);
 
@@ -921,8 +922,8 @@ void vsp1_du_unmap_sg(struct device *dev, struct sg_table 
*sgt)
 {
struct vsp1_device *vsp1 = dev_get_drvdata(dev);
 
-   dma_unmap_sg_attrs(vsp1->bus_master, sgt->sgl, sgt->nents,
-  DMA_TO_DEVICE, DMA_ATTR_SKIP_CPU_SYNC);
+   dma_unmap_sgtable_attrs(vsp1->bus_master, sgt, DMA_TO_DEVICE,
+   DMA_ATTR_SKIP_CPU_SYNC);
 }
 EXPORT_SYMBOL_GPL(vsp1_du_unmap_sg);
 
-- 
1.9.1

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


[PATCH v3 07/25] drm: i915: fix common struct sg_table related issues

2020-05-05 Thread Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of the entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h.

This driver creatively uses sg_table->orig_nents to store the size of the
allocate scatterlist and ignores the number of the entries returned by
dma_map_sg function. The sg_table->orig_nents is (mis)used to properly
free the (over)allocated scatterlist.

This patch only introduces common dma-mapping wrappers operating directly
on the struct sg_table objects to the dmabuf related functions, so the
other drivers, which might share buffers with i915 could rely on the
properly set nents and orig_nents values.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v3 00/25] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/5/187
---
 drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c   | 13 +
 drivers/gpu/drm/i915/gem/selftests/mock_dmabuf.c |  7 +++
 2 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c 
b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
index 7db5a79..7e8583e 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
@@ -48,12 +48,10 @@ static struct sg_table *i915_gem_map_dma_buf(struct 
dma_buf_attachment *attachme
src = sg_next(src);
}
 
-   if (!dma_map_sg_attrs(attachment->dev,
- st->sgl, st->nents, dir,
- DMA_ATTR_SKIP_CPU_SYNC)) {
-   ret = -ENOMEM;
+   ret = dma_map_sgtable_attrs(attachment->dev, st, dir,
+   DMA_ATTR_SKIP_CPU_SYNC);
+   if (ret)
goto err_free_sg;
-   }
 
return st;
 
@@ -73,9 +71,8 @@ static void i915_gem_unmap_dma_buf(struct dma_buf_attachment 
*attachment,
 {
struct drm_i915_gem_object *obj = dma_buf_to_obj(attachment->dmabuf);
 
-   dma_unmap_sg_attrs(attachment->dev,
-  sg->sgl, sg->nents, dir,
-  DMA_ATTR_SKIP_CPU_SYNC);
+   dma_unmap_sgtable_attrs(attachment->dev, sg, dir,
+   DMA_ATTR_SKIP_CPU_SYNC);
sg_free_table(sg);
kfree(sg);
 
diff --git a/drivers/gpu/drm/i915/gem/selftests/mock_dmabuf.c 
b/drivers/gpu/drm/i915/gem/selftests/mock_dmabuf.c
index debaf7b..756cb76 100644
--- a/drivers/gpu/drm/i915/gem/selftests/mock_dmabuf.c
+++ b/drivers/gpu/drm/i915/gem/selftests/mock_dmabuf.c
@@ -28,10 +28,9 @@ static struct sg_table *mock_map_dma_buf(struct 
dma_buf_attachment *attachment,
sg = sg_next(sg);
}
 
-   if (!dma_map_sg(attachment->dev, st->sgl, st->nents, dir)) {
-   err = -ENOMEM;
+   err = dma_map_sgtable(attachment->dev, st, dir);
+   if (err)
goto err_st;
-   }
 
return st;
 
@@ -46,7 +45,7 @@ static void mock_unmap_dma_buf(struct dma_buf_attachment 
*attachment,
   struct sg_table *st,
   enum dma_data_direction dir)
 {
-   dma_unmap_sg(attachment->dev, st->sgl, st->nents, dir);
+   dma_unmap_sgtable(attachment->dev, st, dir);
sg_free_table(st);
kfree(st);
 }
-- 
1.9.1

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


[PATCH v3 01/25] dma-mapping: add generic helpers for mapping sgtable objects

2020-05-05 Thread Marek Szyprowski
struct sg_table is a common structure used for describing a memory
buffer. It consists of a scatterlist with memory pages and DMA addresses
(sgl entry), as well as the number of scatterlist entries: CPU pages
(orig_nents entry) and DMA pages (nents entry).

It turned out that it was a common mistake to misuse nents and orig_nents
entries, call dma-mapping functions with a wrong number of entries or
ignoring the number of mapped entries returned by the dma_map_sg
function.

To avoid such issues, lets introduce a common wrappers operating directly
on the struct sg_table objects, which take care of the proper use of
the nents and orig_nents entries.

Signed-off-by: Marek Szyprowski 
---
 include/linux/dma-mapping.h | 32 
 include/linux/iommu.h   |  6 ++
 2 files changed, 38 insertions(+)

diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index b43116a..8364c20d 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -609,6 +609,36 @@ static inline void dma_sync_single_range_for_device(struct 
device *dev,
return dma_sync_single_for_device(dev, addr + offset, size, dir);
 }
 
+static inline int dma_map_sgtable_attrs(struct device *dev,
+   struct sg_table *sgt, enum dma_data_direction dir, unsigned long attrs)
+{
+   int n = dma_map_sg_attrs(dev, sgt->sgl, sgt->orig_nents, dir, attrs);
+
+   if (n > 0) {
+   sgt->nents = n;
+   return 0;
+   }
+   return -EINVAL;
+}
+
+static inline void dma_unmap_sgtable_attrs(struct device *dev,
+   struct sg_table *sgt, enum dma_data_direction dir, unsigned long attrs)
+{
+   dma_unmap_sg_attrs(dev, sgt->sgl, sgt->orig_nents, dir, attrs);
+}
+
+static inline void dma_sync_sgtable_for_cpu(struct device *dev,
+   struct sg_table *sgt, enum dma_data_direction dir)
+{
+   dma_sync_sg_for_cpu(dev, sgt->sgl, sgt->orig_nents, dir);
+}
+
+static inline void dma_sync_sgtable_for_device(struct device *dev,
+   struct sg_table *sgt, enum dma_data_direction dir)
+{
+   dma_sync_sg_for_device(dev, sgt->sgl, sgt->orig_nents, dir);
+}
+
 #define dma_map_single(d, a, s, r) dma_map_single_attrs(d, a, s, r, 0)
 #define dma_unmap_single(d, a, s, r) dma_unmap_single_attrs(d, a, s, r, 0)
 #define dma_map_sg(d, s, n, r) dma_map_sg_attrs(d, s, n, r, 0)
@@ -617,6 +647,8 @@ static inline void dma_sync_single_range_for_device(struct 
device *dev,
 #define dma_unmap_page(d, a, s, r) dma_unmap_page_attrs(d, a, s, r, 0)
 #define dma_get_sgtable(d, t, v, h, s) dma_get_sgtable_attrs(d, t, v, h, s, 0)
 #define dma_mmap_coherent(d, v, c, h, s) dma_mmap_attrs(d, v, c, h, s, 0)
+#define dma_map_sgtable(d, s, r) dma_map_sgtable_attrs(d, s, r, 0)
+#define dma_unmap_sgtable(d, s, r) dma_unmap_sgtable_attrs(d, s, r, 0)
 
 extern int dma_common_mmap(struct device *dev, struct vm_area_struct *vma,
void *cpu_addr, dma_addr_t dma_addr, size_t size,
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 7ef8b0b..5106b65 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -466,6 +466,12 @@ extern size_t iommu_map_sg_atomic(struct iommu_domain 
*domain,
 extern void iommu_set_fault_handler(struct iommu_domain *domain,
iommu_fault_handler_t handler, void *token);
 
+static inline size_t iommu_map_sgtable(struct iommu_domain *domain,
+   unsigned long iova, struct sg_table *sgt, int prot)
+{
+   return iommu_map_sg(domain, iova, sgt->sgl, sgt->orig_nents, prot);
+}
+
 extern void iommu_get_resv_regions(struct device *dev, struct list_head *list);
 extern void iommu_put_resv_regions(struct device *dev, struct list_head *list);
 extern void generic_iommu_put_resv_regions(struct device *dev,
-- 
1.9.1

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


[PATCH v3 03/25] drm: amdgpu: fix common struct sg_table related issues

2020-05-05 Thread Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of the entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. A common mistake was to ignore a result
of the dma_map_sg function and don't use the sg_table->orig_nents at all.

To avoid such issues, lets use common dma-mapping wrappers operating
directly on the struct sg_table objects and adjust references to the
nents and orig_nents respectively.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v3 00/25] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/5/187
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 6 +++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 9 +++--
 2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index 43d8ed7..eca5628 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -307,8 +307,8 @@ static struct sg_table *amdgpu_dma_buf_map(struct 
dma_buf_attachment *attach,
if (IS_ERR(sgt))
return sgt;
 
-   if (!dma_map_sg_attrs(attach->dev, sgt->sgl, sgt->nents, dir,
- DMA_ATTR_SKIP_CPU_SYNC))
+   if (dma_map_sgtable_attrs(attach->dev, sgt, dir,
+ DMA_ATTR_SKIP_CPU_SYNC))
goto error_free;
break;
 
@@ -349,7 +349,7 @@ static void amdgpu_dma_buf_unmap(struct dma_buf_attachment 
*attach,
struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
 
if (sgt->sgl->page_link) {
-   dma_unmap_sg(attach->dev, sgt->sgl, sgt->nents, dir);
+   dma_unmap_sgtable(attach->dev, sgt, dir);
sg_free_table(sgt);
kfree(sgt);
} else {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index eff1f73..f71f97f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1043,7 +1043,6 @@ static int amdgpu_ttm_tt_pin_userptr(struct ttm_tt *ttm)
 {
struct amdgpu_device *adev = amdgpu_ttm_adev(ttm->bdev);
struct amdgpu_ttm_tt *gtt = (void *)ttm;
-   unsigned nents;
int r;
 
int write = !(gtt->userflags & AMDGPU_GEM_USERPTR_READONLY);
@@ -1058,9 +1057,8 @@ static int amdgpu_ttm_tt_pin_userptr(struct ttm_tt *ttm)
goto release_sg;
 
/* Map SG to device */
-   r = -ENOMEM;
-   nents = dma_map_sg(adev->dev, ttm->sg->sgl, ttm->sg->nents, direction);
-   if (nents == 0)
+   r = dma_map_sgtable(adev->dev, ttm->sg, direction);
+   if (r)
goto release_sg;
 
/* convert SG to linear array of pages and dma addresses */
@@ -1091,8 +1089,7 @@ static void amdgpu_ttm_tt_unpin_userptr(struct ttm_tt 
*ttm)
return;
 
/* unmap the pages mapped to the device */
-   dma_unmap_sg(adev->dev, ttm->sg->sgl, ttm->sg->nents, direction);
-
+   dma_unmap_sgtable(adev->dev, ttm->sg, direction);
sg_free_table(ttm->sg);
 
 #if IS_ENABLED(CONFIG_DRM_AMDGPU_USERPTR)
-- 
1.9.1

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


[PATCH v3 15/25] drm: vmwgfx: fix common struct sg_table related issues

2020-05-05 Thread Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of the entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. A common mistake was to ignore a result
of the dma_map_sg function and don't use the sg_table->orig_nents at all.

To avoid such issues, lets use common dma-mapping wrappers operating
directly on the struct sg_table objects and adjust references to the
nents and orig_nents respectively.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v3 00/25] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/5/187
---
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 17 -
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
index bf0bc46..e50ae8b 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
@@ -362,8 +362,7 @@ static void vmw_ttm_unmap_from_dma(struct vmw_ttm_tt 
*vmw_tt)
 {
struct device *dev = vmw_tt->dev_priv->dev->dev;
 
-   dma_unmap_sg(dev, vmw_tt->sgt.sgl, vmw_tt->sgt.nents,
-   DMA_BIDIRECTIONAL);
+   dma_unmap_sgtable(dev, vmw_tt->sgt, DMA_BIDIRECTIONAL);
vmw_tt->sgt.nents = vmw_tt->sgt.orig_nents;
 }
 
@@ -383,16 +382,8 @@ static void vmw_ttm_unmap_from_dma(struct vmw_ttm_tt 
*vmw_tt)
 static int vmw_ttm_map_for_dma(struct vmw_ttm_tt *vmw_tt)
 {
struct device *dev = vmw_tt->dev_priv->dev->dev;
-   int ret;
-
-   ret = dma_map_sg(dev, vmw_tt->sgt.sgl, vmw_tt->sgt.orig_nents,
-DMA_BIDIRECTIONAL);
-   if (unlikely(ret == 0))
-   return -ENOMEM;
 
-   vmw_tt->sgt.nents = ret;
-
-   return 0;
+   return dma_map_sgtable(dev, vmw_tt->sgt, DMA_BIDIRECTIONAL);
 }
 
 /**
@@ -449,10 +440,10 @@ static int vmw_ttm_map_dma(struct vmw_ttm_tt *vmw_tt)
if (unlikely(ret != 0))
goto out_sg_alloc_fail;
 
-   if (vsgt->num_pages > vmw_tt->sgt.nents) {
+   if (vsgt->num_pages > vmw_tt->sgt.orig_nents) {
uint64_t over_alloc =
sgl_size * (vsgt->num_pages -
-   vmw_tt->sgt.nents);
+   vmw_tt->sgt.orig_nents);
 
ttm_mem_global_free(glob, over_alloc);
vmw_tt->sg_alloc_size -= over_alloc;
-- 
1.9.1

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


[PATCH v3 20/25] staging: ion: fix common struct sg_table related issues

2020-05-05 Thread Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of the entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. A common mistake was to ignore a result
of the dma_map_sg function and don't use the sg_table->orig_nents at all.

To avoid such issues, lets use common dma-mapping wrappers operating
directly on the struct sg_table objects and adjust references to the
nents and orig_nents respectively.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v3 00/25] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/5/187
---
 drivers/staging/android/ion/ion.c | 25 +++--
 drivers/staging/android/ion/ion_heap.c|  6 +++---
 drivers/staging/android/ion/ion_system_heap.c |  2 +-
 3 files changed, 15 insertions(+), 18 deletions(-)

diff --git a/drivers/staging/android/ion/ion.c 
b/drivers/staging/android/ion/ion.c
index 38b51ea..9274298 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -147,14 +147,14 @@ static struct sg_table *dup_sg_table(struct sg_table 
*table)
if (!new_table)
return ERR_PTR(-ENOMEM);
 
-   ret = sg_alloc_table(new_table, table->nents, GFP_KERNEL);
+   ret = sg_alloc_table(new_table, table->orig_nents, GFP_KERNEL);
if (ret) {
kfree(new_table);
return ERR_PTR(-ENOMEM);
}
 
new_sg = new_table->sgl;
-   for_each_sg(table->sgl, sg, table->nents, i) {
+   for_each_sg(table->sgl, sg, table->orig_nents, i) {
memcpy(new_sg, sg, sizeof(*sg));
new_sg->dma_address = 0;
new_sg = sg_next(new_sg);
@@ -224,12 +224,13 @@ static struct sg_table *ion_map_dma_buf(struct 
dma_buf_attachment *attachment,
 {
struct ion_dma_buf_attachment *a = attachment->priv;
struct sg_table *table;
+   int ret;
 
table = a->table;
 
-   if (!dma_map_sg(attachment->dev, table->sgl, table->nents,
-   direction))
-   return ERR_PTR(-ENOMEM);
+   ret = dma_map_sgtable(attachment->dev, table, direction);
+   if (ret)
+   return ERR_PTR(ret);
 
return table;
 }
@@ -238,7 +239,7 @@ static void ion_unmap_dma_buf(struct dma_buf_attachment 
*attachment,
  struct sg_table *table,
  enum dma_data_direction direction)
 {
-   dma_unmap_sg(attachment->dev, table->sgl, table->nents, direction);
+   dma_unmap_sgtable(attachment->dev, table, direction);
 }
 
 static int ion_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma)
@@ -296,10 +297,8 @@ static int ion_dma_buf_begin_cpu_access(struct dma_buf 
*dmabuf,
}
 
mutex_lock(&buffer->lock);
-   list_for_each_entry(a, &buffer->attachments, list) {
-   dma_sync_sg_for_cpu(a->dev, a->table->sgl, a->table->nents,
-   direction);
-   }
+   list_for_each_entry(a, &buffer->attachments, list)
+   dma_sync_sgtable_for_cpu(a->dev, a->table, direction);
 
 unlock:
mutex_unlock(&buffer->lock);
@@ -319,10 +318,8 @@ static int ion_dma_buf_end_cpu_access(struct dma_buf 
*dmabuf,
}
 
mutex_lock(&buffer->lock);
-   list_for_each_entry(a, &buffer->attachments, list) {
-   dma_sync_sg_for_device(a->dev, a->table->sgl, a->table->nents,
-  direction);
-   }
+   list_for_each_entry(a, &buffer->attachments, list)
+   dma_sync_sgtable_for_device(a->dev, a->table, direction);
mutex_unlock(&buffer->lock);
 
return 0;
diff --git a/drivers/staging/android/ion/ion_heap.c 
b/drivers/staging/android/ion/ion_heap.c
index 0755b11..f2f7ca7 100644
--- a/drivers/staging/android/ion/ion_heap.c
+++ b/drivers/staging/android/ion/ion_heap.c
@@ -38,7 +38,7 @@ void *ion_heap_map_kernel(struct ion_heap *heap,
else
pgprot = pgprot_writecombine(PAGE_KERNEL);
 
-   for_each_sg(table->sgl, sg, table->nents, i) {
+   for_each_sg(table->sgl, sg, table->orig_nents, i) {
int npages_this_entry = PAGE_ALIGN(sg->length) / PAGE_SIZE;
struct page *page = sg_page(sg);
 
@@ -71,7 +71,7 @@ int ion_heap_map_user(struct ion_heap *heap, struct 
ion_buffer *buffer,
int i;
int ret;
 
-   for_each_sg(table->sgl, sg, table->nents, i) {
+   for_each_sg(table->sgl, sg, table->orig_nents, i) {
struct page *page = sg_page(sg);
unsigned long remainder = vma->vm_end - addr;
unsigned long len = sg->length;
@@ -142,7 +142,7 @@ int ion_heap_b

[PATCH v3 12/25] drm: rockchip: fix common struct sg_table related issues

2020-05-05 Thread Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of the entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. A common mistake was to ignore a result
of the dma_map_sg function and don't use the sg_table->orig_nents at all.

To avoid such issues, lets use common dma-mapping wrappers operating
directly on the struct sg_table objects and adjust references to the
nents and orig_nents respectively.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v3 00/25] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/5/187
---
 drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 26 --
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c 
b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
index 0d18846..9df7d7d 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
@@ -36,8 +36,8 @@ static int rockchip_gem_iommu_map(struct rockchip_gem_object 
*rk_obj)
 
rk_obj->dma_addr = rk_obj->mm.start;
 
-   ret = iommu_map_sg(private->domain, rk_obj->dma_addr, rk_obj->sgt->sgl,
-  rk_obj->sgt->nents, prot);
+   ret = iommu_map_sgtable(private->domain, rk_obj->dma_addr, rk_obj->sgt,
+   prot);
if (ret < rk_obj->base.size) {
DRM_ERROR("failed to map buffer: size=%zd request_size=%zd\n",
  ret, rk_obj->base.size);
@@ -98,11 +98,10 @@ static int rockchip_gem_get_pages(struct 
rockchip_gem_object *rk_obj)
 * TODO: Replace this by drm_clflush_sg() once it can be implemented
 * without relying on symbols that are not exported.
 */
-   for_each_sg(rk_obj->sgt->sgl, s, rk_obj->sgt->nents, i)
+   for_each_sg(rk_obj->sgt->sgl, s, rk_obj->sgt->orig_nents, i)
sg_dma_address(s) = sg_phys(s);
 
-   dma_sync_sg_for_device(drm->dev, rk_obj->sgt->sgl, rk_obj->sgt->nents,
-  DMA_TO_DEVICE);
+   dma_sync_sgtable_for_device(drm->dev, rk_obj->sgt, DMA_TO_DEVICE);
 
return 0;
 
@@ -350,8 +349,8 @@ void rockchip_gem_free_object(struct drm_gem_object *obj)
if (private->domain) {
rockchip_gem_iommu_unmap(rk_obj);
} else {
-   dma_unmap_sg(drm->dev, rk_obj->sgt->sgl,
-rk_obj->sgt->nents, DMA_BIDIRECTIONAL);
+   dma_unmap_sgtable(drm->dev, rk_obj->sgt,
+ DMA_BIDIRECTIONAL);
}
drm_prime_gem_destroy(obj, rk_obj->sgt);
} else {
@@ -493,15 +492,14 @@ static unsigned long 
rockchip_sg_get_contiguous_size(struct sg_table *sgt,
struct sg_table *sg,
struct rockchip_gem_object *rk_obj)
 {
-   int count = dma_map_sg(drm->dev, sg->sgl, sg->nents,
-  DMA_BIDIRECTIONAL);
-   if (!count)
-   return -EINVAL;
+   int err = dma_map_sgtable(drm->dev, sg, DMA_BIDIRECTIONAL);
+   if (err)
+   return err;
 
-   if (rockchip_sg_get_contiguous_size(sg, count) < attach->dmabuf->size) {
+   if (rockchip_sg_get_contiguous_size(sg, sg->nents) <
+   attach->dmabuf->size) {
DRM_ERROR("failed to map sg_table to contiguous linear 
address.\n");
-   dma_unmap_sg(drm->dev, sg->sgl, sg->nents,
-DMA_BIDIRECTIONAL);
+   dma_unmap_sgtable(drm->dev, sg, DMA_BIDIRECTIONAL);
return -EINVAL;
}
 
-- 
1.9.1

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


[PATCH v3 08/25] drm: lima: fix common struct sg_table related issues

2020-05-05 Thread Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of the entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. A common mistake was to ignore a result
of the dma_map_sg function and don't use the sg_table->orig_nents at all.

To avoid such issues, lets use common dma-mapping wrappers operating
directly on the struct sg_table objects and adjust references to the
nents and orig_nents respectively.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v3 00/25] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/5/187
---
 drivers/gpu/drm/lima/lima_gem.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/lima/lima_gem.c b/drivers/gpu/drm/lima/lima_gem.c
index 5404e0d..fca359d 100644
--- a/drivers/gpu/drm/lima/lima_gem.c
+++ b/drivers/gpu/drm/lima/lima_gem.c
@@ -69,8 +69,7 @@ int lima_heap_alloc(struct lima_bo *bo, struct lima_vm *vm)
return ret;
 
if (bo->base.sgt) {
-   dma_unmap_sg(dev, bo->base.sgt->sgl,
-bo->base.sgt->nents, DMA_BIDIRECTIONAL);
+   dma_unmap_sgtable(dev, bo->base.sgt, DMA_BIDIRECTIONAL);
sg_free_table(bo->base.sgt);
} else {
bo->base.sgt = kmalloc(sizeof(*bo->base.sgt), GFP_KERNEL);
@@ -80,7 +79,13 @@ int lima_heap_alloc(struct lima_bo *bo, struct lima_vm *vm)
}
}
 
-   dma_map_sg(dev, sgt.sgl, sgt.nents, DMA_BIDIRECTIONAL);
+   ret = dma_map_sgtable(dev, &sgt, DMA_BIDIRECTIONAL);
+   if (ret) {
+   sg_free_table(&sgt);
+   kfree(bo->base.sgt);
+   bo->base.sgt = NULL;
+   return ret;
+   }
 
*bo->base.sgt = sgt;
 
-- 
1.9.1

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


[PATCH v3 17/25] drm: host1x: fix common struct sg_table related issues

2020-05-05 Thread Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of the entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. A common mistake was to ignore a result
of the dma_map_sg function and don't use the sg_table->orig_nents at all.

To avoid such issues, lets use common dma-mapping wrappers operating
directly on the struct sg_table objects and adjust references to the
nents and orig_nents respectively.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v3 00/25] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/5/187
---
 drivers/gpu/host1x/job.c | 22 --
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/host1x/job.c b/drivers/gpu/host1x/job.c
index a10643a..850115e 100644
--- a/drivers/gpu/host1x/job.c
+++ b/drivers/gpu/host1x/job.c
@@ -166,11 +166,9 @@ static unsigned int pin_job(struct host1x *host, struct 
host1x_job *job)
goto unpin;
}
 
-   err = dma_map_sg(dev, sgt->sgl, sgt->nents, dir);
-   if (!err) {
-   err = -ENOMEM;
+   err = dma_map_sgtable(dev, sgt, dir);
+   if (err)
goto unpin;
-   }
 
job->unpins[job->num_unpins].dev = dev;
job->unpins[job->num_unpins].dir = dir;
@@ -217,7 +215,7 @@ static unsigned int pin_job(struct host1x *host, struct 
host1x_job *job)
}
 
if (!IS_ENABLED(CONFIG_TEGRA_HOST1X_FIREWALL) && host->domain) {
-   for_each_sg(sgt->sgl, sg, sgt->nents, j)
+   for_each_sg(sgt->sgl, sg, sgt->orig_nents, j)
gather_size += sg->length;
gather_size = iova_align(&host->iova, gather_size);
 
@@ -229,9 +227,9 @@ static unsigned int pin_job(struct host1x *host, struct 
host1x_job *job)
goto unpin;
}
 
-   err = iommu_map_sg(host->domain,
+   err = iommu_map_sgtable(host->domain,
iova_dma_addr(&host->iova, alloc),
-   sgt->sgl, sgt->nents, IOMMU_READ);
+   sgt, IOMMU_READ);
if (err == 0) {
__free_iova(&host->iova, alloc);
err = -EINVAL;
@@ -241,12 +239,9 @@ static unsigned int pin_job(struct host1x *host, struct 
host1x_job *job)
job->unpins[job->num_unpins].size = gather_size;
phys_addr = iova_dma_addr(&host->iova, alloc);
} else if (sgt) {
-   err = dma_map_sg(host->dev, sgt->sgl, sgt->nents,
-DMA_TO_DEVICE);
-   if (!err) {
-   err = -ENOMEM;
+   err = dma_map_sgtable(host->dev, sgt, DMA_TO_DEVICE);
+   if (err)
goto unpin;
-   }
 
job->unpins[job->num_unpins].dir = DMA_TO_DEVICE;
job->unpins[job->num_unpins].dev = host->dev;
@@ -647,8 +642,7 @@ void host1x_job_unpin(struct host1x_job *job)
}
 
if (unpin->dev && sgt)
-   dma_unmap_sg(unpin->dev, sgt->sgl, sgt->nents,
-unpin->dir);
+   dma_unmap_sgtable(unpin->dev, sgt, unpin->dir);
 
host1x_bo_unpin(dev, unpin->bo, sgt);
host1x_bo_put(unpin->bo);
-- 
1.9.1

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


[PATCH v3 09/25] drm: msm: fix common struct sg_table related issues

2020-05-05 Thread Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of the entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. A common mistake was to ignore a result
of the dma_map_sg function and don't use the sg_table->orig_nents at all.

To avoid such issues, lets use common dma-mapping wrappers operating
directly on the struct sg_table objects and adjust references to the
nents and orig_nents respectively.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v3 00/25] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/5/187
---
 drivers/gpu/drm/msm/msm_gem.c   | 13 +
 drivers/gpu/drm/msm/msm_iommu.c |  2 +-
 2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
index 5a6a79f..ab952d6 100644
--- a/drivers/gpu/drm/msm/msm_gem.c
+++ b/drivers/gpu/drm/msm/msm_gem.c
@@ -53,11 +53,10 @@ static void sync_for_device(struct msm_gem_object *msm_obj)
struct device *dev = msm_obj->base.dev->dev;
 
if (get_dma_ops(dev) && IS_ENABLED(CONFIG_ARM64)) {
-   dma_sync_sg_for_device(dev, msm_obj->sgt->sgl,
-   msm_obj->sgt->nents, DMA_BIDIRECTIONAL);
+   dma_sync_sgtable_for_device(dev, msm_obj->sgt,
+   DMA_BIDIRECTIONAL);
} else {
-   dma_map_sg(dev, msm_obj->sgt->sgl,
-   msm_obj->sgt->nents, DMA_BIDIRECTIONAL);
+   dma_map_sgtable(dev, msm_obj->sgt, DMA_BIDIRECTIONAL);
}
 }
 
@@ -66,11 +65,9 @@ static void sync_for_cpu(struct msm_gem_object *msm_obj)
struct device *dev = msm_obj->base.dev->dev;
 
if (get_dma_ops(dev) && IS_ENABLED(CONFIG_ARM64)) {
-   dma_sync_sg_for_cpu(dev, msm_obj->sgt->sgl,
-   msm_obj->sgt->nents, DMA_BIDIRECTIONAL);
+   dma_sync_sgtable_for_cpu(dev, msm_obj->sgt, DMA_BIDIRECTIONAL);
} else {
-   dma_unmap_sg(dev, msm_obj->sgt->sgl,
-   msm_obj->sgt->nents, DMA_BIDIRECTIONAL);
+   dma_unmap_sgtable(dev, msm_obj->sgt, DMA_BIDIRECTIONAL);
}
 }
 
diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c
index ad58cfe..d322b39 100644
--- a/drivers/gpu/drm/msm/msm_iommu.c
+++ b/drivers/gpu/drm/msm/msm_iommu.c
@@ -43,7 +43,7 @@ static int msm_iommu_map(struct msm_mmu *mmu, uint64_t iova,
struct msm_iommu *iommu = to_msm_iommu(mmu);
size_t ret;
 
-   ret = iommu_map_sg(iommu->domain, iova, sgt->sgl, sgt->nents, prot);
+   ret = iommu_map_sgtable(iommu->domain, iova, sgt, prot);
WARN_ON(!ret);
 
return (ret == len) ? 0 : -EINVAL;
-- 
1.9.1

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


[PATCH v3 10/25] drm: panfrost: fix common struct sg_table related issues

2020-05-05 Thread Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of the entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. A common mistake was to ignore a result
of the dma_map_sg function and don't use the sg_table->orig_nents at all.

To avoid such issues, lets use common dma-mapping wrappers operating
directly on the struct sg_table objects and adjust references to the
nents and orig_nents respectively.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v3 00/25] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/5/187
---
 drivers/gpu/drm/panfrost/panfrost_gem.c | 4 ++--
 drivers/gpu/drm/panfrost/panfrost_mmu.c | 5 ++---
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c 
b/drivers/gpu/drm/panfrost/panfrost_gem.c
index 17b654e..95d7e80 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gem.c
+++ b/drivers/gpu/drm/panfrost/panfrost_gem.c
@@ -41,8 +41,8 @@ static void panfrost_gem_free_object(struct drm_gem_object 
*obj)
 
for (i = 0; i < n_sgt; i++) {
if (bo->sgts[i].sgl) {
-   dma_unmap_sg(pfdev->dev, bo->sgts[i].sgl,
-bo->sgts[i].nents, 
DMA_BIDIRECTIONAL);
+   dma_unmap_sgtable(pfdev->dev, &bo->sgts[i],
+ DMA_BIDIRECTIONAL);
sg_free_table(&bo->sgts[i]);
}
}
diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c 
b/drivers/gpu/drm/panfrost/panfrost_mmu.c
index ed28aeb..9926111 100644
--- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
+++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
@@ -517,10 +517,9 @@ static int panfrost_mmu_map_fault_addr(struct 
panfrost_device *pfdev, int as,
if (ret)
goto err_pages;
 
-   if (!dma_map_sg(pfdev->dev, sgt->sgl, sgt->nents, DMA_BIDIRECTIONAL)) {
-   ret = -EINVAL;
+   ret = dma_map_sgtable(pfdev->dev, sgt, DMA_BIDIRECTIONAL);
+   if (ret)
goto err_map;
-   }
 
mmu_map_sg(pfdev, bomapping->mmu, addr,
   IOMMU_WRITE | IOMMU_READ | IOMMU_NOEXEC, sgt);
-- 
1.9.1

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


[PATCH v3 25/25] media: pci: fix common ALSA DMA-mapping related codes

2020-05-05 Thread Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. Adapt the code to obey those rules.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v3 00/25] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/5/187
---
 drivers/media/pci/cx23885/cx23885-alsa.c | 2 +-
 drivers/media/pci/cx25821/cx25821-alsa.c | 2 +-
 drivers/media/pci/cx88/cx88-alsa.c   | 2 +-
 drivers/media/pci/saa7134/saa7134-alsa.c | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/pci/cx23885/cx23885-alsa.c 
b/drivers/media/pci/cx23885/cx23885-alsa.c
index df44ed7..3f366e4 100644
--- a/drivers/media/pci/cx23885/cx23885-alsa.c
+++ b/drivers/media/pci/cx23885/cx23885-alsa.c
@@ -129,7 +129,7 @@ static int cx23885_alsa_dma_unmap(struct cx23885_audio_dev 
*dev)
if (!buf->sglen)
return 0;
 
-   dma_unmap_sg(&dev->pci->dev, buf->sglist, buf->sglen, 
PCI_DMA_FROMDEVICE);
+   dma_unmap_sg(&dev->pci->dev, buf->sglist, buf->nr_pages, 
PCI_DMA_FROMDEVICE);
buf->sglen = 0;
return 0;
 }
diff --git a/drivers/media/pci/cx25821/cx25821-alsa.c 
b/drivers/media/pci/cx25821/cx25821-alsa.c
index 3016164..c40304d 100644
--- a/drivers/media/pci/cx25821/cx25821-alsa.c
+++ b/drivers/media/pci/cx25821/cx25821-alsa.c
@@ -193,7 +193,7 @@ static int cx25821_alsa_dma_unmap(struct cx25821_audio_dev 
*dev)
if (!buf->sglen)
return 0;
 
-   dma_unmap_sg(&dev->pci->dev, buf->sglist, buf->sglen, 
PCI_DMA_FROMDEVICE);
+   dma_unmap_sg(&dev->pci->dev, buf->sglist, buf->nr_pages, 
PCI_DMA_FROMDEVICE);
buf->sglen = 0;
return 0;
 }
diff --git a/drivers/media/pci/cx88/cx88-alsa.c 
b/drivers/media/pci/cx88/cx88-alsa.c
index 7d7acee..3c6fe6c 100644
--- a/drivers/media/pci/cx88/cx88-alsa.c
+++ b/drivers/media/pci/cx88/cx88-alsa.c
@@ -332,7 +332,7 @@ static int cx88_alsa_dma_unmap(struct cx88_audio_dev *dev)
if (!buf->sglen)
return 0;
 
-   dma_unmap_sg(&dev->pci->dev, buf->sglist, buf->sglen,
+   dma_unmap_sg(&dev->pci->dev, buf->sglist, buf->nr_pages,
 PCI_DMA_FROMDEVICE);
buf->sglen = 0;
return 0;
diff --git a/drivers/media/pci/saa7134/saa7134-alsa.c 
b/drivers/media/pci/saa7134/saa7134-alsa.c
index 544ca57..398c47f 100644
--- a/drivers/media/pci/saa7134/saa7134-alsa.c
+++ b/drivers/media/pci/saa7134/saa7134-alsa.c
@@ -313,7 +313,7 @@ static int saa7134_alsa_dma_unmap(struct saa7134_dev *dev)
if (!dma->sglen)
return 0;
 
-   dma_unmap_sg(&dev->pci->dev, dma->sglist, dma->sglen, 
PCI_DMA_FROMDEVICE);
+   dma_unmap_sg(&dev->pci->dev, dma->sglist, dma->nr_pages, 
PCI_DMA_FROMDEVICE);
dma->sglen = 0;
return 0;
 }
-- 
1.9.1

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


[PATCH v3 06/25] drm: exynos: fix common struct sg_table related issues

2020-05-05 Thread Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of the entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. A common mistake was to ignore a result
of the dma_map_sg function and don't use the sg_table->orig_nents at all.

To avoid such issues, lets use common dma-mapping wrappers operating
directly on the struct sg_table objects and adjust references to the
nents and orig_nents respectively.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v3 00/25] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/5/187
---
 drivers/gpu/drm/exynos/exynos_drm_g2d.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c 
b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
index fcee33a..6a655d3 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
@@ -395,8 +395,8 @@ static void g2d_userptr_put_dma_addr(struct g2d_data *g2d,
return;
 
 out:
-   dma_unmap_sg(to_dma_dev(g2d->drm_dev), g2d_userptr->sgt->sgl,
-   g2d_userptr->sgt->nents, DMA_BIDIRECTIONAL);
+   dma_unmap_sgtable(to_dma_dev(g2d->drm_dev), g2d_userptr->sgt,
+ DMA_BIDIRECTIONAL);
 
pages = frame_vector_pages(g2d_userptr->vec);
if (!IS_ERR(pages)) {
@@ -511,10 +511,9 @@ static dma_addr_t *g2d_userptr_get_dma_addr(struct 
g2d_data *g2d,
 
g2d_userptr->sgt = sgt;
 
-   if (!dma_map_sg(to_dma_dev(g2d->drm_dev), sgt->sgl, sgt->nents,
-   DMA_BIDIRECTIONAL)) {
+   ret = dma_map_sgtable(to_dma_dev(g2d->drm_dev), sgt, DMA_BIDIRECTIONAL);
+   if (ret) {
DRM_DEV_ERROR(g2d->dev, "failed to map sgt with dma region.\n");
-   ret = -ENOMEM;
goto err_sg_free_table;
}
 
-- 
1.9.1

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


[PATCH v3 05/25] drm: etnaviv: fix common struct sg_table related issues

2020-05-05 Thread Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of the entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. A common mistake was to ignore a result
of the dma_map_sg function and don't use the sg_table->orig_nents at all.

To avoid such issues, lets use common dma-mapping wrappers operating
directly on the struct sg_table objects and adjust references to the
nents and orig_nents respectively.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v3 00/25] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/5/187
---
 drivers/gpu/drm/etnaviv/etnaviv_gem.c | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c 
b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
index dc9ef30..340026b 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
@@ -27,7 +27,7 @@ static void etnaviv_gem_scatter_map(struct etnaviv_gem_object 
*etnaviv_obj)
 * because display controller, GPU, etc. are not coherent.
 */
if (etnaviv_obj->flags & ETNA_BO_CACHE_MASK)
-   dma_map_sg(dev->dev, sgt->sgl, sgt->nents, DMA_BIDIRECTIONAL);
+   dma_map_sgtable(dev->dev, sgt, DMA_BIDIRECTIONAL);
 }
 
 static void etnaviv_gem_scatterlist_unmap(struct etnaviv_gem_object 
*etnaviv_obj)
@@ -51,7 +51,7 @@ static void etnaviv_gem_scatterlist_unmap(struct 
etnaviv_gem_object *etnaviv_obj
 * discard those writes.
 */
if (etnaviv_obj->flags & ETNA_BO_CACHE_MASK)
-   dma_unmap_sg(dev->dev, sgt->sgl, sgt->nents, DMA_BIDIRECTIONAL);
+   dma_unmap_sgtable(dev->dev, sgt, DMA_BIDIRECTIONAL);
 }
 
 /* called with etnaviv_obj->lock held */
@@ -404,9 +404,8 @@ int etnaviv_gem_cpu_prep(struct drm_gem_object *obj, u32 op,
}
 
if (etnaviv_obj->flags & ETNA_BO_CACHED) {
-   dma_sync_sg_for_cpu(dev->dev, etnaviv_obj->sgt->sgl,
-   etnaviv_obj->sgt->nents,
-   etnaviv_op_to_dma_dir(op));
+   dma_sync_sgtable_for_cpu(dev->dev, etnaviv_obj->sgt,
+etnaviv_op_to_dma_dir(op));
etnaviv_obj->last_cpu_prep_op = op;
}
 
@@ -421,8 +420,7 @@ int etnaviv_gem_cpu_fini(struct drm_gem_object *obj)
if (etnaviv_obj->flags & ETNA_BO_CACHED) {
/* fini without a prep is almost certainly a userspace error */
WARN_ON(etnaviv_obj->last_cpu_prep_op == 0);
-   dma_sync_sg_for_device(dev->dev, etnaviv_obj->sgt->sgl,
-   etnaviv_obj->sgt->nents,
+   dma_sync_sgtable_for_device(dev->dev, etnaviv_obj->sgt,
etnaviv_op_to_dma_dir(etnaviv_obj->last_cpu_prep_op));
etnaviv_obj->last_cpu_prep_op = 0;
}
-- 
1.9.1

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


[PATCH v3 19/25] dmabuf: fix common struct sg_table related issues

2020-05-05 Thread Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of the entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. A common mistake was to ignore a result
of the dma_map_sg function and don't use the sg_table->orig_nents at all.

To avoid such issues, lets use common dma-mapping wrappers operating
directly on the struct sg_table objects and adjust references to the
nents and orig_nents respectively.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v3 00/25] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/5/187
---
 drivers/dma-buf/heaps/heap-helpers.c | 13 ++---
 drivers/dma-buf/udmabuf.c|  7 +++
 2 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/dma-buf/heaps/heap-helpers.c 
b/drivers/dma-buf/heaps/heap-helpers.c
index 9f964ca..be9523a 100644
--- a/drivers/dma-buf/heaps/heap-helpers.c
+++ b/drivers/dma-buf/heaps/heap-helpers.c
@@ -140,13 +140,12 @@ struct sg_table *dma_heap_map_dma_buf(struct 
dma_buf_attachment *attachment,
  enum dma_data_direction direction)
 {
struct dma_heaps_attachment *a = attachment->priv;
-   struct sg_table *table;
-
-   table = &a->table;
+   struct sg_table *table = &a->table;
+   int ret;
 
-   if (!dma_map_sg(attachment->dev, table->sgl, table->nents,
-   direction))
-   table = ERR_PTR(-ENOMEM);
+   ret = dma_map_sgtable(attachment->dev, table, direction);
+   if (ret)
+   table = ERR_PTR(ret);
return table;
 }
 
@@ -154,7 +153,7 @@ static void dma_heap_unmap_dma_buf(struct 
dma_buf_attachment *attachment,
   struct sg_table *table,
   enum dma_data_direction direction)
 {
-   dma_unmap_sg(attachment->dev, table->sgl, table->nents, direction);
+   dma_unmap_sgtable(attachment->dev, table, direction);
 }
 
 static vm_fault_t dma_heap_vm_fault(struct vm_fault *vmf)
diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
index acb26c6..5bcbf7a 100644
--- a/drivers/dma-buf/udmabuf.c
+++ b/drivers/dma-buf/udmabuf.c
@@ -63,10 +63,9 @@ static struct sg_table *get_sg_table(struct device *dev, 
struct dma_buf *buf,
GFP_KERNEL);
if (ret < 0)
goto err;
-   if (!dma_map_sg(dev, sg->sgl, sg->nents, direction)) {
-   ret = -EINVAL;
+   ret = dma_map_sgtable(dev, sg, direction);
+   if (ret < 0)
goto err;
-   }
return sg;
 
 err:
@@ -78,7 +77,7 @@ static struct sg_table *get_sg_table(struct device *dev, 
struct dma_buf *buf,
 static void put_sg_table(struct device *dev, struct sg_table *sg,
 enum dma_data_direction direction)
 {
-   dma_unmap_sg(dev, sg->sgl, sg->nents, direction);
+   dma_unmap_sgtable(dev, sg, direction);
sg_free_table(sg);
kfree(sg);
 }
-- 
1.9.1

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


[PATCH v3 24/25] samples: vfio-mdev/mbochs: fix common struct sg_table related issues

2020-05-05 Thread Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of the entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. A common mistake was to ignore a result
of the dma_map_sg function and don't use the sg_table->orig_nents at all.

To avoid such issues, lets use common dma-mapping wrappers operating
directly on the struct sg_table objects and adjust references to the
nents and orig_nents respectively.

While touching this code, also add missing call to dma_unmap_sgtable.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v3 00/25] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/5/187
---
 samples/vfio-mdev/mbochs.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/samples/vfio-mdev/mbochs.c b/samples/vfio-mdev/mbochs.c
index 3cc5e59..f2c62e0 100644
--- a/samples/vfio-mdev/mbochs.c
+++ b/samples/vfio-mdev/mbochs.c
@@ -846,7 +846,7 @@ static struct sg_table *mbochs_map_dmabuf(struct 
dma_buf_attachment *at,
if (sg_alloc_table_from_pages(sg, dmabuf->pages, dmabuf->pagecount,
  0, dmabuf->mode.size, GFP_KERNEL) < 0)
goto err2;
-   if (!dma_map_sg(at->dev, sg->sgl, sg->nents, direction))
+   if (dma_map_sgtable(at->dev, sg, direction))
goto err3;
 
return sg;
@@ -868,6 +868,7 @@ static void mbochs_unmap_dmabuf(struct dma_buf_attachment 
*at,
 
dev_dbg(dev, "%s: %d\n", __func__, dmabuf->id);
 
+   dma_unmap_sgtable(at->dev, sg, direction);
sg_free_table(sg);
kfree(sg);
 }
-- 
1.9.1

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


[PATCH v3 14/25] drm: virtio: fix common struct sg_table related issues

2020-05-05 Thread Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of the entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. A common mistake was to ignore a result
of the dma_map_sg function and don't use the sg_table->orig_nents at all.

To avoid such issues, lets use common dma-mapping wrappers operating
directly on the struct sg_table objects and adjust references to the
nents and orig_nents respectively.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v3 00/25] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/5/187
---
 drivers/gpu/drm/virtio/virtgpu_object.c | 17 -
 drivers/gpu/drm/virtio/virtgpu_vq.c | 10 --
 2 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c 
b/drivers/gpu/drm/virtio/virtgpu_object.c
index 6ccbd01..5fe153c 100644
--- a/drivers/gpu/drm/virtio/virtgpu_object.c
+++ b/drivers/gpu/drm/virtio/virtgpu_object.c
@@ -72,9 +72,8 @@ void virtio_gpu_cleanup_object(struct virtio_gpu_object *bo)
 
if (shmem->pages) {
if (shmem->mapped) {
-   dma_unmap_sg(vgdev->vdev->dev.parent,
-shmem->pages->sgl, shmem->mapped,
-DMA_TO_DEVICE);
+   dma_unmap_sgtable(vgdev->vdev->dev.parent,
+shmem->pages, DMA_TO_DEVICE);
shmem->mapped = 0;
}
 
@@ -157,13 +156,13 @@ static int virtio_gpu_object_shmem_init(struct 
virtio_gpu_device *vgdev,
}
 
if (use_dma_api) {
-   shmem->mapped = dma_map_sg(vgdev->vdev->dev.parent,
-  shmem->pages->sgl,
-  shmem->pages->nents,
-  DMA_TO_DEVICE);
-   *nents = shmem->mapped;
+   ret = dma_map_sgtable(vgdev->vdev->dev.parent,
+ shmem->pages, DMA_TO_DEVICE);
+   if (ret)
+   return ret;
+   *nents = shmem->mapped = shmem->pages->nents;
} else {
-   *nents = shmem->pages->nents;
+   *nents = shmem->pages->orig_nents;
}
 
*ents = kmalloc_array(*nents, sizeof(struct virtio_gpu_mem_entry),
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c 
b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 9e663a5..c329147 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -603,9 +603,8 @@ void virtio_gpu_cmd_transfer_to_host_2d(struct 
virtio_gpu_device *vgdev,
struct virtio_gpu_object_shmem *shmem = to_virtio_gpu_shmem(bo);
 
if (use_dma_api)
-   dma_sync_sg_for_device(vgdev->vdev->dev.parent,
-  shmem->pages->sgl, shmem->pages->nents,
-  DMA_TO_DEVICE);
+   dma_sync_sgtable_for_device(vgdev->vdev->dev.parent,
+   shmem->pages, DMA_TO_DEVICE);
 
cmd_p = virtio_gpu_alloc_cmd(vgdev, &vbuf, sizeof(*cmd_p));
memset(cmd_p, 0, sizeof(*cmd_p));
@@ -1019,9 +1018,8 @@ void virtio_gpu_cmd_transfer_to_host_3d(struct 
virtio_gpu_device *vgdev,
struct virtio_gpu_object_shmem *shmem = to_virtio_gpu_shmem(bo);
 
if (use_dma_api)
-   dma_sync_sg_for_device(vgdev->vdev->dev.parent,
-  shmem->pages->sgl, shmem->pages->nents,
-  DMA_TO_DEVICE);
+   dma_sync_sgtable_for_device(vgdev->vdev->dev.parent,
+   shmem->pages, DMA_TO_DEVICE);
 
cmd_p = virtio_gpu_alloc_cmd(vgdev, &vbuf, sizeof(*cmd_p));
memset(cmd_p, 0, sizeof(*cmd_p));
-- 
1.9.1

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


[PATCH v3 02/25] drm: core: fix common struct sg_table related issues

2020-05-05 Thread Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of the entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. A common mistake was to ignore a result
of the dma_map_sg function and don't use the sg_table->orig_nents at all.

To avoid such issues, lets use common dma-mapping wrappers operating
directly on the struct sg_table objects and adjust references to the
nents and orig_nents respectively.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v3 00/25] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/5/187
---
 drivers/gpu/drm/drm_cache.c|  2 +-
 drivers/gpu/drm/drm_gem_shmem_helper.c | 14 +-
 drivers/gpu/drm/drm_prime.c| 13 +++--
 3 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c
index 03e01b0..63bd497 100644
--- a/drivers/gpu/drm/drm_cache.c
+++ b/drivers/gpu/drm/drm_cache.c
@@ -127,7 +127,7 @@ static void drm_cache_flush_clflush(struct page *pages[],
struct sg_page_iter sg_iter;
 
mb(); /*CLFLUSH is ordered only by using memory barriers*/
-   for_each_sg_page(st->sgl, &sg_iter, st->nents, 0)
+   for_each_sg_page(st->sgl, &sg_iter, st->orig_nents, 0)
drm_clflush_page(sg_page_iter_page(&sg_iter));
mb(); /*Make sure that all cache line entry is flushed*/
 
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c 
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index df31e57..cfcfc0d 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -117,8 +117,8 @@ void drm_gem_shmem_free_object(struct drm_gem_object *obj)
kvfree(shmem->pages);
} else {
if (shmem->sgt) {
-   dma_unmap_sg(obj->dev->dev, shmem->sgt->sgl,
-shmem->sgt->nents, DMA_BIDIRECTIONAL);
+   dma_unmap_sgtable(obj->dev->dev, shmem->sgt,
+ DMA_BIDIRECTIONAL);
sg_free_table(shmem->sgt);
kfree(shmem->sgt);
}
@@ -395,8 +395,7 @@ void drm_gem_shmem_purge_locked(struct drm_gem_object *obj)
 
WARN_ON(!drm_gem_shmem_is_purgeable(shmem));
 
-   dma_unmap_sg(obj->dev->dev, shmem->sgt->sgl,
-shmem->sgt->nents, DMA_BIDIRECTIONAL);
+   dma_unmap_sgtable(obj->dev->dev, shmem->sgt, DMA_BIDIRECTIONAL);
sg_free_table(shmem->sgt);
kfree(shmem->sgt);
shmem->sgt = NULL;
@@ -623,12 +622,17 @@ struct sg_table *drm_gem_shmem_get_pages_sgt(struct 
drm_gem_object *obj)
goto err_put_pages;
}
/* Map the pages for use by the h/w. */
-   dma_map_sg(obj->dev->dev, sgt->sgl, sgt->nents, DMA_BIDIRECTIONAL);
+   ret = dma_map_sgtable(obj->dev->dev, sgt, DMA_BIDIRECTIONAL);
+   if (ret)
+   goto err_free_sgt;
 
shmem->sgt = sgt;
 
return sgt;
 
+err_free_sgt:
+   sg_free_table(sgt);
+   kfree(sgt);
 err_put_pages:
drm_gem_shmem_put_pages(shmem);
return ERR_PTR(ret);
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 282774e..3e7cb02 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -617,6 +617,7 @@ struct sg_table *drm_gem_map_dma_buf(struct 
dma_buf_attachment *attach,
 {
struct drm_gem_object *obj = attach->dmabuf->priv;
struct sg_table *sgt;
+   int ret;
 
if (WARN_ON(dir == DMA_NONE))
return ERR_PTR(-EINVAL);
@@ -626,11 +627,12 @@ struct sg_table *drm_gem_map_dma_buf(struct 
dma_buf_attachment *attach,
else
sgt = obj->dev->driver->gem_prime_get_sg_table(obj);
 
-   if (!dma_map_sg_attrs(attach->dev, sgt->sgl, sgt->nents, dir,
- DMA_ATTR_SKIP_CPU_SYNC)) {
+   ret = dma_map_sgtable_attrs(attach->dev, sgt, dir,
+   DMA_ATTR_SKIP_CPU_SYNC);
+   if (ret) {
sg_free_table(sgt);
kfree(sgt);
-   sgt = ERR_PTR(-ENOMEM);
+   sgt = ERR_PTR(ret);
}
 
return sgt;
@@ -652,8 +654,7 @@ void drm_gem_unmap_dma_buf(struct dma_buf_attachment 
*attach,
if (!sgt)
return;
 
-   dma_unmap_sg_attrs(attach->dev, sgt->sgl, sgt->nents, dir,
-  DMA_ATTR_SKIP_CPU_SYNC);
+   dma_unmap_sgtable_attrs(attach->dev, sgt, dir, DMA_ATTR_SKIP_CPU_SYNC);
sg_free_table(sgt);
kfree(sgt);
 }
@@ -975,7 +976,7 @@ int drm_prime_sg_to_page_addr

[PATCH v3 21/25] staging: tegra-vde: fix common struct sg_table related issues

2020-05-05 Thread Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of the entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. A common mistake was to ignore a result
of the dma_map_sg function and don't use the sg_table->orig_nents at all.

To avoid such issues, lets use common dma-mapping wrappers operating
directly on the struct sg_table objects and adjust references to the
nents and orig_nents respectively.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v3 00/25] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/5/187
---
 drivers/staging/media/tegra-vde/iommu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/tegra-vde/iommu.c 
b/drivers/staging/media/tegra-vde/iommu.c
index 6af863d..adf8dc7 100644
--- a/drivers/staging/media/tegra-vde/iommu.c
+++ b/drivers/staging/media/tegra-vde/iommu.c
@@ -36,8 +36,8 @@ int tegra_vde_iommu_map(struct tegra_vde *vde,
 
addr = iova_dma_addr(&vde->iova, iova);
 
-   size = iommu_map_sg(vde->domain, addr, sgt->sgl, sgt->nents,
-   IOMMU_READ | IOMMU_WRITE);
+   size = iommu_map_sgtable(vde->domain, addr, sgt,
+IOMMU_READ | IOMMU_WRITE);
if (!size) {
__free_iova(&vde->iova, iova);
return -ENXIO;
-- 
1.9.1

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


[PATCH v3 13/25] drm: tegra: fix common struct sg_table related issues

2020-05-05 Thread Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of the entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. A common mistake was to ignore a result
of the dma_map_sg function and don't use the sg_table->orig_nents at all.

To avoid such issues, lets use common dma-mapping wrappers operating
directly on the struct sg_table objects and adjust references to the
nents and orig_nents respectively.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v3 00/25] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/5/187
---
 drivers/gpu/drm/tegra/gem.c   | 27 ++-
 drivers/gpu/drm/tegra/plane.c | 15 +--
 2 files changed, 15 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c
index 6237681..4554fbb 100644
--- a/drivers/gpu/drm/tegra/gem.c
+++ b/drivers/gpu/drm/tegra/gem.c
@@ -98,8 +98,8 @@ static struct sg_table *tegra_bo_pin(struct device *dev, 
struct host1x_bo *bo,
 * the SG table needs to be copied to avoid overwriting any
 * other potential users of the original SG table.
 */
-   err = sg_alloc_table_from_sg(sgt, obj->sgt->sgl, 
obj->sgt->nents,
-GFP_KERNEL);
+   err = sg_alloc_table_from_sg(sgt, obj->sgt->sgl,
+obj->sgt->orig_nents, GFP_KERNEL);
if (err < 0)
goto free;
} else {
@@ -196,8 +196,7 @@ static int tegra_bo_iommu_map(struct tegra_drm *tegra, 
struct tegra_bo *bo)
 
bo->iova = bo->mm->start;
 
-   bo->size = iommu_map_sg(tegra->domain, bo->iova, bo->sgt->sgl,
-   bo->sgt->nents, prot);
+   bo->size = iommu_map_sgtable(tegra->domain, bo->iova, bo->sgt, prot);
if (!bo->size) {
dev_err(tegra->drm->dev, "failed to map buffer\n");
err = -ENOMEM;
@@ -264,8 +263,7 @@ static struct tegra_bo *tegra_bo_alloc_object(struct 
drm_device *drm,
 static void tegra_bo_free(struct drm_device *drm, struct tegra_bo *bo)
 {
if (bo->pages) {
-   dma_unmap_sg(drm->dev, bo->sgt->sgl, bo->sgt->nents,
-DMA_FROM_DEVICE);
+   dma_unmap_sgtable(drm->dev, bo->sgt, DMA_FROM_DEVICE);
drm_gem_put_pages(&bo->gem, bo->pages, true, true);
sg_free_table(bo->sgt);
kfree(bo->sgt);
@@ -290,12 +288,9 @@ static int tegra_bo_get_pages(struct drm_device *drm, 
struct tegra_bo *bo)
goto put_pages;
}
 
-   err = dma_map_sg(drm->dev, bo->sgt->sgl, bo->sgt->nents,
-DMA_FROM_DEVICE);
-   if (err == 0) {
-   err = -EFAULT;
+   err = dma_map_sgtable(drm->dev, bo->sgt, DMA_FROM_DEVICE);
+   if (err)
goto free_sgt;
-   }
 
return 0;
 
@@ -571,7 +566,7 @@ int tegra_drm_mmap(struct file *file, struct vm_area_struct 
*vma)
goto free;
}
 
-   if (dma_map_sg(attach->dev, sgt->sgl, sgt->nents, dir) == 0)
+   if (dma_map_sgtable(attach->dev, sgt, dir))
goto free;
 
return sgt;
@@ -590,7 +585,7 @@ static void tegra_gem_prime_unmap_dma_buf(struct 
dma_buf_attachment *attach,
struct tegra_bo *bo = to_tegra_bo(gem);
 
if (bo->pages)
-   dma_unmap_sg(attach->dev, sgt->sgl, sgt->nents, dir);
+   dma_unmap_sgtable(attach->dev, sgt, dir);
 
sg_free_table(sgt);
kfree(sgt);
@@ -609,8 +604,7 @@ static int tegra_gem_prime_begin_cpu_access(struct dma_buf 
*buf,
struct drm_device *drm = gem->dev;
 
if (bo->pages)
-   dma_sync_sg_for_cpu(drm->dev, bo->sgt->sgl, bo->sgt->nents,
-   DMA_FROM_DEVICE);
+   dma_sync_sgtable_for_cpu(drm->dev, bo->sgt, DMA_FROM_DEVICE);
 
return 0;
 }
@@ -623,8 +617,7 @@ static int tegra_gem_prime_end_cpu_access(struct dma_buf 
*buf,
struct drm_device *drm = gem->dev;
 
if (bo->pages)
-   dma_sync_sg_for_device(drm->dev, bo->sgt->sgl, bo->sgt->nents,
-  DMA_TO_DEVICE);
+   dma_sync_sgtable_for_device(drm->dev, bo->sgt, DMA_TO_DEVICE);
 
return 0;
 }
diff --git a/drivers/gpu/drm/tegra/plane.c b/drivers/gpu/drm/tegra/plane.c
index 9ccfb56..4b1c2ae6 100644
--- a/drivers/gpu/drm/tegra/plane.c
+++ b/drivers/gpu/drm/tegra/plane.c
@@ -130,12 +130,9 @@ static int tegra_dc_pin(struct tegra_dc *dc, struct 
tegra_plane_state *state)
}
 
   

[PATCH v3 22/25] misc: fastrpc: fix common struct sg_table related issues

2020-05-05 Thread Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of the entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. A common mistake was to ignore a result
of the dma_map_sg function and don't use the sg_table->orig_nents at all.

To avoid such issues, lets use common dma-mapping wrappers operating
directly on the struct sg_table objects and adjust references to the
nents and orig_nents respectively.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v3 00/25] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/5/187
---
 drivers/misc/fastrpc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index e3e085e..0a3e02aa 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -518,7 +518,7 @@ static struct fastrpc_invoke_ctx *fastrpc_context_alloc(
 
table = &a->sgt;
 
-   if (!dma_map_sg(attachment->dev, table->sgl, table->nents, dir))
+   if (!dma_map_sgtable(attachment->dev, table, dir))
return ERR_PTR(-ENOMEM);
 
return table;
@@ -528,7 +528,7 @@ static void fastrpc_unmap_dma_buf(struct dma_buf_attachment 
*attach,
  struct sg_table *table,
  enum dma_data_direction dir)
 {
-   dma_unmap_sg(attach->dev, table->sgl, table->nents, dir);
+   dma_unmap_sgtable(attach->dev, table, dir);
 }
 
 static void fastrpc_release(struct dma_buf *dmabuf)
-- 
1.9.1

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


[PATCH v3 11/25] drm: radeon: fix common struct sg_table related issues

2020-05-05 Thread Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of the entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. A common mistake was to ignore a result
of the dma_map_sg function and don't use the sg_table->orig_nents at all.

To avoid such issues, lets use common dma-mapping wrappers operating
directly on the struct sg_table objects and adjust references to the
nents and orig_nents respectively.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v3 00/25] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/5/187
---
 drivers/gpu/drm/radeon/radeon_ttm.c | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c 
b/drivers/gpu/drm/radeon/radeon_ttm.c
index 5d50c9e..166f84e 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -481,7 +481,7 @@ static int radeon_ttm_tt_pin_userptr(struct ttm_tt *ttm)
 {
struct radeon_device *rdev = radeon_get_rdev(ttm->bdev);
struct radeon_ttm_tt *gtt = (void *)ttm;
-   unsigned pinned = 0, nents;
+   unsigned pinned = 0;
int r;
 
int write = !(gtt->userflags & RADEON_GEM_USERPTR_READONLY);
@@ -521,9 +521,8 @@ static int radeon_ttm_tt_pin_userptr(struct ttm_tt *ttm)
if (r)
goto release_sg;
 
-   r = -ENOMEM;
-   nents = dma_map_sg(rdev->dev, ttm->sg->sgl, ttm->sg->nents, direction);
-   if (nents == 0)
+   r = dma_map_sgtable(rdev->dev, ttm->sg, direction);
+   if (r)
goto release_sg;
 
drm_prime_sg_to_page_addr_arrays(ttm->sg, ttm->pages,
@@ -554,9 +553,9 @@ static void radeon_ttm_tt_unpin_userptr(struct ttm_tt *ttm)
return;
 
/* free the sg table and pages again */
-   dma_unmap_sg(rdev->dev, ttm->sg->sgl, ttm->sg->nents, direction);
+   dma_unmap_sgtable(rdev->dev, ttm->sg, direction);
 
-   for_each_sg_page(ttm->sg->sgl, &sg_iter, ttm->sg->nents, 0) {
+   for_each_sg_page(ttm->sg->sgl, &sg_iter, ttm->sg->orig_nents, 0) {
struct page *page = sg_page_iter_page(&sg_iter);
if (!(gtt->userflags & RADEON_GEM_USERPTR_READONLY))
set_page_dirty(page);
-- 
1.9.1

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


[PATCH v3 04/25] drm: armada: fix common struct sg_table related issues

2020-05-05 Thread Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of the entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. A common mistake was to ignore a result
of the dma_map_sg function and don't use the sg_table->orig_nents at all.

To avoid such issues, lets use common dma-mapping wrappers operating
directly on the struct sg_table objects and adjust references to the
nents and orig_nents respectively.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v3 00/25] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/5/187
---
 drivers/gpu/drm/armada/armada_gem.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/armada/armada_gem.c 
b/drivers/gpu/drm/armada/armada_gem.c
index 976685f..5b4f48c 100644
--- a/drivers/gpu/drm/armada/armada_gem.c
+++ b/drivers/gpu/drm/armada/armada_gem.c
@@ -407,8 +407,8 @@ int armada_gem_pwrite_ioctl(struct drm_device *dev, void 
*data,
sg_set_page(sg, page, PAGE_SIZE, 0);
}
 
-   if (dma_map_sg(attach->dev, sgt->sgl, sgt->nents, dir) == 0) {
-   num = sgt->nents;
+   if (dma_map_sgtable(attach->dev, sgt, dir)) {
+   num = count;
goto release;
}
} else if (dobj->page) {
@@ -418,7 +418,7 @@ int armada_gem_pwrite_ioctl(struct drm_device *dev, void 
*data,
 
sg_set_page(sgt->sgl, dobj->page, dobj->obj.size, 0);
 
-   if (dma_map_sg(attach->dev, sgt->sgl, sgt->nents, dir) == 0)
+   if (dma_map_sgtable(attach->dev, sgt, dir))
goto free_table;
} else if (dobj->linear) {
/* Single contiguous physical region - no struct page */
@@ -449,11 +449,11 @@ static void armada_gem_prime_unmap_dma_buf(struct 
dma_buf_attachment *attach,
int i;
 
if (!dobj->linear)
-   dma_unmap_sg(attach->dev, sgt->sgl, sgt->nents, dir);
+   dma_unmap_sgtable(attach->dev, sgt, dir);
 
if (dobj->obj.filp) {
struct scatterlist *sg;
-   for_each_sg(sgt->sgl, sg, sgt->nents, i)
+   for_each_sg(sgt->sgl, sg, sgt->orig_nents, i)
put_page(sg_page(sg));
}
 
-- 
1.9.1

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


[PATCH v3 23/25] rapidio: fix common struct sg_table related issues

2020-05-05 Thread Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of the entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. A common mistake was to ignore a result
of the dma_map_sg function and don't use the sg_table->orig_nents at all.

To avoid such issues, lets use common dma-mapping wrappers operating
directly on the struct sg_table objects and adjust references to the
nents and orig_nents respectively.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v3 00/25] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/5/187
---
 drivers/rapidio/devices/rio_mport_cdev.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/rapidio/devices/rio_mport_cdev.c 
b/drivers/rapidio/devices/rio_mport_cdev.c
index 4029637..df7dba8 100644
--- a/drivers/rapidio/devices/rio_mport_cdev.c
+++ b/drivers/rapidio/devices/rio_mport_cdev.c
@@ -574,8 +574,7 @@ static void dma_req_free(struct kref *ref)
struct mport_cdev_priv *priv = req->priv;
unsigned int i;
 
-   dma_unmap_sg(req->dmach->device->dev,
-req->sgt.sgl, req->sgt.nents, req->dir);
+   dma_unmap_sgtable(req->dmach->device->dev, req->sgt, req->dir);
sg_free_table(&req->sgt);
if (req->page_list) {
for (i = 0; i < req->nr_pages; i++)
@@ -927,9 +926,8 @@ static int do_dma_request(struct mport_dma_req *req,
xfer->offset, xfer->length);
}
 
-   nents = dma_map_sg(chan->device->dev,
-  req->sgt.sgl, req->sgt.nents, dir);
-   if (nents == 0) {
+   ret = dma_map_sgtable(chan->device->dev, req->sgt, dir);
+   if (ret) {
rmcd_error("Failed to map SG list");
ret = -EFAULT;
goto err_pg;
-- 
1.9.1

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


[PATCH v3 16/25] xen: gntdev: fix common struct sg_table related issues

2020-05-05 Thread Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of the entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. A common mistake was to ignore a result
of the dma_map_sg function and don't use the sg_table->orig_nents at all.

To avoid such issues, lets use common dma-mapping wrappers operating
directly on the struct sg_table objects and adjust references to the
nents and orig_nents respectively.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v3 00/25] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/5/187
---
 drivers/xen/gntdev-dmabuf.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/xen/gntdev-dmabuf.c b/drivers/xen/gntdev-dmabuf.c
index 75d3bb9..4b22785 100644
--- a/drivers/xen/gntdev-dmabuf.c
+++ b/drivers/xen/gntdev-dmabuf.c
@@ -247,8 +247,7 @@ static void dmabuf_exp_ops_detach(struct dma_buf *dma_buf,
 
if (sgt) {
if (gntdev_dmabuf_attach->dir != DMA_NONE)
-   dma_unmap_sg_attrs(attach->dev, sgt->sgl,
-  sgt->nents,
+   dma_unmap_sgtable_attrs(attach->dev, sgt,
   gntdev_dmabuf_attach->dir,
   DMA_ATTR_SKIP_CPU_SYNC);
sg_free_table(sgt);
@@ -288,7 +287,7 @@ static void dmabuf_exp_ops_detach(struct dma_buf *dma_buf,
sgt = dmabuf_pages_to_sgt(gntdev_dmabuf->pages,
  gntdev_dmabuf->nr_pages);
if (!IS_ERR(sgt)) {
-   if (!dma_map_sg_attrs(attach->dev, sgt->sgl, sgt->nents, dir,
+   if (dma_map_sgtable_attrs(attach->dev, sgt, dir,
  DMA_ATTR_SKIP_CPU_SYNC)) {
sg_free_table(sgt);
kfree(sgt);
@@ -625,7 +624,7 @@ static struct gntdev_dmabuf *dmabuf_imp_alloc_storage(int 
count)
 
/* Now convert sgt to array of pages and check for page validity. */
i = 0;
-   for_each_sg_page(sgt->sgl, &sg_iter, sgt->nents, 0) {
+   for_each_sg_page(sgt->sgl, &sg_iter, sgt->orig_nents, 0) {
struct page *page = sg_page_iter_page(&sg_iter);
/*
 * Check if page is valid: this can happen if we are given
-- 
1.9.1

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


[PATCH v3 00/25] DRM: fix struct sg_table nents vs. orig_nents misuse

2020-05-05 Thread Marek Szyprowski
Dear All,

During the Exynos DRM GEM rework and fixing the issues in the 
drm_prime_sg_to_page_addr_arrays() function [1] I've noticed that most
drivers in DRM framework incorrectly use nents and orig_nents entries of
the struct sg_table.

In case of the most DMA-mapping implementations exchanging those two
entries or using nents for all loops on the scatterlist is harmless,
because they both have the same value. There exists however a DMA-mapping
implementations, for which such incorrect usage breaks things. The nents
returned by dma_map_sg() might be lower than the nents passed as its
parameter and this is perfectly fine. DMA framework or IOMMU is allowed
to join consecutive chunks while mapping if such operation is supported
by the underlying HW (bus, bridge, IOMMU, etc). Example of the case
where dma_map_sg() might return 1 'DMA' chunk for the 4 'physical' pages
is described here [2]

The DMA-mapping framework documentation [3] states that dma_map_sg()
returns the numer of the created entries in the DMA address space.
However the subsequent calls to dma_sync_sg_for_{device,cpu} and
dma_unmap_sg must be called with the original number of entries passed to
dma_map_sg. The common pattern in DRM drivers were to assign the
dma_map_sg() return value to sg_table->nents and use that value for
the subsequent calls to dma_sync_sg_* or dma_unmap_sg functions. Also
the code iterated over nents times to access the pages stored in the
processed scatterlist, while it should use orig_nents as the numer of
the page entries.

I've tried to identify all such incorrect usage of sg_table->nents and
this is a result of my research. It looks that the incorrect pattern has
been copied over the many drivers mainly in the DRM subsystem. Too bad in
most cases it even worked correctly if the system used simple, linear
DMA-mapping implementation, for which swapping nents and orig_nents
doesn't make any difference. To avoid similar issues in the future, I've
introduced a common dma-mapping wrappers, which operate directly on the
sg_table objects.

The biggest TODO is DRM/i915 driver and I don't feel brave enough to fix
it fully. The driver creatively uses sg_table->orig_nents to store the
size of the allocate scatterlist and ignores the number of the entries
returned by dma_map_sg function. In this patchset I only fixed the
sg_table objects exported by dmabuf related functions. I hope that I
didn't break anything there.

Patches are based on top of Linux next-20200504.

Best regards,
Marek Szyprowski


References:

[1] https://lkml.org/lkml/2020/3/27/555 
[2] https://lkml.org/lkml/2020/3/29/65
[3] Documentation/DMA-API-HOWTO.txt


Changelog:

v3:
- introduce dma_*_sgtable_* wrappers and use them in all patches

v2: 
https://lore.kernel.org/linux-iommu/c01c9766-9778-fd1f-f36e-2dc7bd376...@arm.com/T/
- dropped most of the changes to drm/i915
- added fixes for rcar-du, xen, media and ion
- fixed a few issues pointed by kbuild test robot
- added wide cc: list for each patch

v1: 
https://lore.kernel.org/linux-iommu/c01c9766-9778-fd1f-f36e-2dc7bd376...@arm.com/T/
- initial version


Patch summary:

Marek Szyprowski (25):
  dma-mapping: add generic helpers for mapping sgtable objects
  drm: core: fix common struct sg_table related issues
  drm: amdgpu: fix common struct sg_table related issues
  drm: armada: fix common struct sg_table related issues
  drm: etnaviv: fix common struct sg_table related issues
  drm: exynos: fix common struct sg_table related issues
  drm: i915: fix common struct sg_table related issues
  drm: lima: fix common struct sg_table related issues
  drm: msm: fix common struct sg_table related issues
  drm: panfrost: fix common struct sg_table related issues
  drm: radeon: fix common struct sg_table related issues
  drm: rockchip: fix common struct sg_table related issues
  drm: tegra: fix common struct sg_table related issues
  drm: virtio: fix common struct sg_table related issues
  drm: vmwgfx: fix common struct sg_table related issues
  xen: gntdev: fix common struct sg_table related issues
  drm: host1x: fix common struct sg_table related issues
  drm: rcar-du: fix common struct sg_table related issues
  dmabuf: fix common struct sg_table related issues
  staging: ion: fix common struct sg_table related issues
  staging: tegra-vde: fix common struct sg_table related issues
  misc: fastrpc: fix common struct sg_table related issues
  rapidio: fix common struct sg_table related issues
  samples: vfio-mdev/mbochs: fix common struct sg_table related issues
  media: pci: fix common ALSA DMA-mapping related codes

 drivers/dma-buf/heaps/heap-helpers.c | 13 +-
 drivers/dma-buf/udmabuf.c|  7 +++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c  |  6 ++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c  |  9 +++
 drivers/gpu/drm/armada/armada_gem.c  | 10 
 drivers/gpu/drm/drm_cache.c  |  2 +-
 drivers/gpu/drm/drm_gem_shmem_help