Question about out_unmap section of map_sg in amd_iommu.c

2019-01-18 Thread Jerry Snitselaar




 out_unmap:
 pr_err("%s: IOMMU mapping error in map_sg (io-pages: %d)\n",
dev_name(dev), npages);

 for_each_sg(sglist, s, nelems, i) {
 int j, pages = iommu_num_pages(sg_phys(s), s->length, 
PAGE_SIZE);

 for (j = 0; j < pages; ++j) {
 unsigned long bus_addr;

 bus_addr  = address + s->dma_address + (j << 
PAGE_SHIFT);
 iommu_unmap_page(domain, bus_addr, PAGE_SIZE);

 if (--mapped_pages)
 goto out_free_iova;

Is this condition correct?  My thought is this was meant to break out of the 
loop early if
all the mapped pages have been unmapped. So if (--mapped == 0) instead?

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


Re: [PATCH] arm64/xen: fix xen-swiotlb cache flushing

2019-01-18 Thread Stefano Stabellini
On Thu, 17 Jan 2019, Christoph Hellwig wrote:
> Xen-swiotlb hooks into the arm/arm64 arch code through a copy of the
> DMA mapping operations stored in the struct device arch data.
> 
> Switching arm64 to use the direct calls for the merged DMA direct /
> swiotlb code broke this scheme.  Replace the indirect calls with
> direct-calls in xen-swiotlb as well to fix this problem.
> 
> Fixes: 356da6d0cd ("dma-mapping: bypass indirect calls for dma-direct")
> Reported-by: Julien Grall 
> Signed-off-by: Christoph Hellwig 
> ---
>  arch/arm/include/asm/xen/page-coherent.h   | 94 +
>  arch/arm64/include/asm/device.h|  3 -
>  arch/arm64/include/asm/xen/page-coherent.h | 76 +
>  arch/arm64/mm/dma-mapping.c|  4 +-
>  drivers/xen/swiotlb-xen.c  |  4 +-
>  include/xen/arm/page-coherent.h| 97 +-
>  6 files changed, 176 insertions(+), 102 deletions(-)
> 
> diff --git a/arch/arm/include/asm/xen/page-coherent.h 
> b/arch/arm/include/asm/xen/page-coherent.h
> index b3ef061d8b74..2c403e7c782d 100644
> --- a/arch/arm/include/asm/xen/page-coherent.h
> +++ b/arch/arm/include/asm/xen/page-coherent.h
> @@ -1 +1,95 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _ASM_ARM_XEN_PAGE_COHERENT_H
> +#define _ASM_ARM_XEN_PAGE_COHERENT_H
> +
> +#include 
> +#include 
>  #include 
> +
> +static inline const struct dma_map_ops *xen_get_dma_ops(struct device *dev)
> +{
> + if (dev && dev->archdata.dev_dma_ops)
> + return dev->archdata.dev_dma_ops;
> + return get_arch_dma_ops(NULL);
> +}
> +
> +static inline void *xen_alloc_coherent_pages(struct device *hwdev, size_t 
> size,
> + dma_addr_t *dma_handle, gfp_t flags, unsigned long attrs)
> +{
> + return xen_get_dma_ops(hwdev)->alloc(hwdev, size, dma_handle, flags, 
> attrs);
> +}
> +
> +static inline void xen_free_coherent_pages(struct device *hwdev, size_t size,
> + void *cpu_addr, dma_addr_t dma_handle, unsigned long attrs)
> +{
> + xen_get_dma_ops(hwdev)->free(hwdev, size, cpu_addr, dma_handle, attrs);
> +}
> +
> +static inline void xen_dma_map_page(struct device *hwdev, struct page *page,
> +  dma_addr_t dev_addr, unsigned long offset, size_t size,
> +  enum dma_data_direction dir, unsigned long attrs)
> +{
> + unsigned long page_pfn = page_to_xen_pfn(page);
> + unsigned long dev_pfn = XEN_PFN_DOWN(dev_addr);
> + unsigned long compound_pages =
> + (1< + bool local = (page_pfn <= dev_pfn) &&
> + (dev_pfn - page_pfn < compound_pages);
> +
> + /*
> +  * Dom0 is mapped 1:1, while the Linux page can span across
> +  * multiple Xen pages, it's not possible for it to contain a
> +  * mix of local and foreign Xen pages. So if the first xen_pfn
> +  * == mfn the page is local otherwise it's a foreign page
> +  * grant-mapped in dom0. If the page is local we can safely
> +  * call the native dma_ops function, otherwise we call the xen
> +  * specific function.
> +  */
> + if (local)
> + xen_get_dma_ops(hwdev)->map_page(hwdev, page, offset, size, 
> dir, attrs);
> + else
> + __xen_dma_map_page(hwdev, page, dev_addr, offset, size, dir, 
> attrs);
> +}
> +
> +static inline void xen_dma_unmap_page(struct device *hwdev, dma_addr_t 
> handle,
> + size_t size, enum dma_data_direction dir, unsigned long attrs)
> +{
> + unsigned long pfn = PFN_DOWN(handle);
> + /*
> +  * Dom0 is mapped 1:1, while the Linux page can be spanned accross
> +  * multiple Xen page, it's not possible to have a mix of local and
> +  * foreign Xen page. Dom0 is mapped 1:1, so calling pfn_valid on a
> +  * foreign mfn will always return false. If the page is local we can
> +  * safely call the native dma_ops function, otherwise we call the xen
> +  * specific function.
> +  */
> + if (pfn_valid(pfn)) {
> + if (xen_get_dma_ops(hwdev)->unmap_page)
> + xen_get_dma_ops(hwdev)->unmap_page(hwdev, handle, size, 
> dir, attrs);
> + } else
> + __xen_dma_unmap_page(hwdev, handle, size, dir, attrs);
> +}
> +
> +static inline void xen_dma_sync_single_for_cpu(struct device *hwdev,
> + dma_addr_t handle, size_t size, enum dma_data_direction dir)
> +{
> + unsigned long pfn = PFN_DOWN(handle);
> + if (pfn_valid(pfn)) {
> + if (xen_get_dma_ops(hwdev)->sync_single_for_cpu)
> + xen_get_dma_ops(hwdev)->sync_single_for_cpu(hwdev, 
> handle, size, dir);
> + } else
> + __xen_dma_sync_single_for_cpu(hwdev, handle, size, dir);
> +}
> +
> +static inline void xen_dma_sync_single_for_device(struct device *hwdev,
> + dma_addr_t handle, size_t size, enum dma_data_direction dir)
> +{
> + unsigned long pfn = PFN_DOWN(handle);
> + if (pfn_valid(pfn)) {
> + if 

Re: Fail to boot Dom0 on Xen Arm64 after "dma-mapping: bypass indirect calls for dma-direct"

2019-01-18 Thread Stefano Stabellini
On Thu, 17 Jan 2019, Christoph Hellwig wrote:
> On Thu, Jan 17, 2019 at 11:43:49AM +, Julien Grall wrote:
> > Looking at the change for arm64, you will always call dma-direct API. In
> > previous Linux version, xen-swiotlb will call dev->archdata.dev_dma_ops (a
> > copy of dev->dma_ops before setting Xen DMA ops) if not NULL. Does it mean
> > we expect dev->dma_ops to always be NULL and hence using dma-direct API?
> 
> The way I understood the code from inspecting it and sking the
> maintainers a few askings is that for DOM0 we always use xen-swiotlb
> as the actual dma_map_ops, but then use the functions in page-coherent.h
> only to deal with cache maintainance, so it should be safe.

Yes, what you wrote is correct, the stuff in
include/xen/arm/page-coherent.h is only to deal with cache maintenance,
specifically any cache maintenance operations that might be required by
map/unmap_page, dma_sync_single_for_cpu/device.

It looks like it is safe today to make the dma_direct calls directly,
especially considering that on arm64 it looks like the only other option
is iommu_dma_ops which has never been used with Xen so far (the IOMMU
has not been exposed to guests yet).

On arm32 we don't have this problem because dev->dma_ops is always !=
NULL with MMU enable (required on Xen), right?

So the patch looks fine, I only have an optional suggestion to add a
check on dma_ops being unset. I'll reply to the patch. 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH] iommu/vt-d: Implement dma_[un]map_resource()

2019-01-18 Thread Logan Gunthorpe
Currently the Intel IOMMU uses the default dma_[un]map_resource()
implementations does nothing and simply returns the physical address
unmodified.

However, this doesn't create the IOVA entries necessary for addresses
mapped this way to work when the IOMMU is enabled. Thus, when the
IOMMU is enabled, drivers relying on dma_map_resource() will trigger
DMAR errors. We see this when running ntb_transport with the IOMMU
enabled, DMA, and switchtec hardware.

The implementation for intel_map_resource() is nearly identical to
intel_map_page(), we just have to re-create __intel_map_single().
dma_unmap_resource() uses intel_unmap_page() directly as the
functions are identical.

Signed-off-by: Logan Gunthorpe 
Cc: David Woodhouse 
Cc: Joerg Roedel 
---
 drivers/iommu/intel-iommu.c | 25 -
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 2bd9ac285c0d..81fb265f7730 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -3649,11 +3649,9 @@ static int iommu_no_mapping(struct device *dev)
return 0;
 }
 
-static dma_addr_t __intel_map_page(struct device *dev, struct page *page,
-  unsigned long offset, size_t size, int dir,
-  u64 dma_mask)
+static dma_addr_t __intel_map_single(struct device *dev, phys_addr_t paddr,
+size_t size, int dir, u64 dma_mask)
 {
-   phys_addr_t paddr = page_to_phys(page) + offset;
struct dmar_domain *domain;
phys_addr_t start_paddr;
unsigned long iova_pfn;
@@ -3710,6 +3708,14 @@ static dma_addr_t __intel_map_page(struct device *dev, 
struct page *page,
return DMA_MAPPING_ERROR;
 }
 
+static dma_addr_t __intel_map_page(struct device *dev, struct page *page,
+  unsigned long offset, size_t size, int dir,
+  u64 dma_mask)
+{
+   return __intel_map_single(dev, page_to_phys(page) + offset, size, dir,
+ dma_mask);
+}
+
 static dma_addr_t intel_map_page(struct device *dev, struct page *page,
 unsigned long offset, size_t size,
 enum dma_data_direction dir,
@@ -3718,6 +3724,13 @@ static dma_addr_t intel_map_page(struct device *dev, 
struct page *page,
return __intel_map_page(dev, 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)
+{
+   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)
 {
struct dmar_domain *domain;
@@ -3924,6 +3937,8 @@ static const struct dma_map_ops intel_dma_ops = {
.unmap_sg = intel_unmap_sg,
.map_page = intel_map_page,
.unmap_page = intel_unmap_page,
+   .map_resource = intel_map_resource,
+   .unmap_resource = intel_unmap_page,
.dma_supported = dma_direct_supported,
 };
 
@@ -4080,7 +4095,7 @@ static int init_iommu_hw(void)
iommu_disable_protect_mem_regions(iommu);
continue;
}
-   
+
iommu_flush_write_buffer(iommu);
 
iommu_set_root_entry(iommu);
-- 
2.19.0

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


Re: [PATCH] dmaengine: fsl-edma: dma map slave device address

2019-01-18 Thread Angelo Dureghello
Hi Laurentiu,

On Fri, Jan 18, 2019 at 12:06:23PM +0200, Laurentiu Tudor wrote:
> This mapping needs to be created in order for slave dma transfers
> to work on systems with SMMU. The implementation mostly mimics the
> one in pl330 dma driver, authored by Robin Murphy.
> 
> Signed-off-by: Laurentiu Tudor 
> Suggested-by: Robin Murphy 
> ---
> Original approach was to add the missing mappings in the i2c client driver,
> see here for discussion: https://patchwork.ozlabs.org/patch/1026013/
> 
>  drivers/dma/fsl-edma-common.c | 66 ---
>  drivers/dma/fsl-edma-common.h |  4 +++
>  drivers/dma/fsl-edma.c|  1 +
>  drivers/dma/mcf-edma.c|  1 +
>  4 files changed, 68 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/dma/fsl-edma-common.c b/drivers/dma/fsl-edma-common.c
> index 8876c4c1bb2c..0e95ee24b6d4 100644
> --- a/drivers/dma/fsl-edma-common.c
> +++ b/drivers/dma/fsl-edma-common.c
> @@ -6,6 +6,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "fsl-edma-common.h"
>  
> @@ -173,12 +174,62 @@ int fsl_edma_resume(struct dma_chan *chan)
>  }
>  EXPORT_SYMBOL_GPL(fsl_edma_resume);
>  
> +static void fsl_edma_unprep_slave_dma(struct fsl_edma_chan *fsl_chan)
> +{
> + if (fsl_chan->dma_dir != DMA_NONE)
> + dma_unmap_resource(fsl_chan->vchan.chan.device->dev,
> +fsl_chan->dma_dev_addr,
> +fsl_chan->dma_dev_size,
> +fsl_chan->dma_dir, 0);
> + fsl_chan->dma_dir = DMA_NONE;
> +}
> +
> +static bool fsl_edma_prep_slave_dma(struct fsl_edma_chan *fsl_chan,
> + enum dma_transfer_direction dir)
> +{
> + struct device *dev = fsl_chan->vchan.chan.device->dev;
> + enum dma_data_direction dma_dir;
> + phys_addr_t addr = 0;
> + u32 size = 0;
> +
> + switch (dir) {
> + case DMA_MEM_TO_DEV:
> + dma_dir = DMA_FROM_DEVICE;
> + addr = fsl_chan->cfg.dst_addr;
> + size = fsl_chan->cfg.dst_maxburst;
> + break;
> + case DMA_DEV_TO_MEM:
> + dma_dir = DMA_TO_DEVICE;
> + addr = fsl_chan->cfg.src_addr;
> + size = fsl_chan->cfg.src_maxburst;
> + break;
> + default:
> + dma_dir = DMA_NONE;
> + break;
> + }
> +
> + /* Already mapped for this config? */
> + if (fsl_chan->dma_dir == dma_dir)
> + return true;
> +
> + fsl_edma_unprep_slave_dma(fsl_chan);
> +
> + fsl_chan->dma_dev_addr = dma_map_resource(dev, addr, size, dma_dir, 0);
> + if (dma_mapping_error(dev, fsl_chan->dma_dev_addr))
> + return false;
> + fsl_chan->dma_dev_size = size;
> + fsl_chan->dma_dir = dma_dir;
> +
> + return true;
> +}
> +
>  int fsl_edma_slave_config(struct dma_chan *chan,
>struct dma_slave_config *cfg)
>  {
>   struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
>  
>   memcpy(_chan->cfg, cfg, sizeof(*cfg));
> + fsl_edma_unprep_slave_dma(fsl_chan);
>  
>   return 0;
>  }
> @@ -378,6 +429,9 @@ struct dma_async_tx_descriptor *fsl_edma_prep_dma_cyclic(
>   if (!is_slave_direction(direction))
>   return NULL;
>  
> + if (!fsl_edma_prep_slave_dma(fsl_chan, direction))
> + return NULL;
> +
>   sg_len = buf_len / period_len;
>   fsl_desc = fsl_edma_alloc_desc(fsl_chan, sg_len);
>   if (!fsl_desc)
> @@ -409,11 +463,11 @@ struct dma_async_tx_descriptor 
> *fsl_edma_prep_dma_cyclic(
>  
>   if (direction == DMA_MEM_TO_DEV) {
>   src_addr = dma_buf_next;
> - dst_addr = fsl_chan->cfg.dst_addr;
> + dst_addr = fsl_chan->dma_dev_addr;
>   soff = fsl_chan->cfg.dst_addr_width;
>   doff = 0;
>   } else {
> - src_addr = fsl_chan->cfg.src_addr;
> + src_addr = fsl_chan->dma_dev_addr;
>   dst_addr = dma_buf_next;
>   soff = 0;
>   doff = fsl_chan->cfg.src_addr_width;
> @@ -444,6 +498,9 @@ struct dma_async_tx_descriptor *fsl_edma_prep_slave_sg(
>   if (!is_slave_direction(direction))
>   return NULL;
>  
> + if (!fsl_edma_prep_slave_dma(fsl_chan, direction))
> + return NULL;
> +
>   fsl_desc = fsl_edma_alloc_desc(fsl_chan, sg_len);
>   if (!fsl_desc)
>   return NULL;
> @@ -468,11 +525,11 @@ struct dma_async_tx_descriptor *fsl_edma_prep_slave_sg(
>  
>   if (direction == DMA_MEM_TO_DEV) {
>   src_addr = sg_dma_address(sg);
> - dst_addr = fsl_chan->cfg.dst_addr;
> + dst_addr = fsl_chan->dma_dev_addr;
>   soff = fsl_chan->cfg.dst_addr_width;
>   doff = 0;
>   } else {
> - 

Re: [PATCH v7 0/7] Add virtio-iommu driver

2019-01-18 Thread Michael S. Tsirkin


On Tue, Jan 15, 2019 at 12:19:52PM +, Jean-Philippe Brucker wrote:
> Implement the virtio-iommu driver, following specification v0.9 [1].
> 
> This is a simple rebase onto Linux v5.0-rc2. We now use the
> dev_iommu_fwspec_get() helper introduced in v5.0 instead of accessing
> dev->iommu_fwspec, but there aren't any functional change from v6 [2].
> 
> Our current goal for virtio-iommu is to get a paravirtual IOMMU working
> on Arm, and enable device assignment to guest userspace. In this
> use-case the mappings are static, and don't require optimal performance,
> so this series tries to keep things simple. However there is plenty more
> to do for features and optimizations, and having this base in v5.1 would
> be good. Given that most of the changes are to drivers/iommu, I believe
> the driver and future changes should go via the IOMMU tree.
> 
> You can find Linux driver and kvmtool device on v0.9.2 branches [3],
> module and x86 support on virtio-iommu/devel. Also tested with Eric's
> QEMU device [4]. Please note that the series depends on Robin's
> probe-deferral fix [5], which will hopefully land in v5.0.
> 
> [1] Virtio-iommu specification v0.9, sources and pdf
> git://linux-arm.org/virtio-iommu.git virtio-iommu/v0.9
> http://jpbrucker.net/virtio-iommu/spec/v0.9/virtio-iommu-v0.9.pdf
> 
> [2] [PATCH v6 0/7] Add virtio-iommu driver
> 
> https://lists.linuxfoundation.org/pipermail/iommu/2018-December/032127.html
> 
> [3] git://linux-arm.org/linux-jpb.git virtio-iommu/v0.9.2
> git://linux-arm.org/kvmtool-jpb.git virtio-iommu/v0.9.2
> 
> [4] [RFC v9 00/17] VIRTIO-IOMMU device
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg575578.html
> 
> [5] [PATCH] iommu/of: Fix probe-deferral
> https://www.spinics.net/lists/arm-kernel/msg698371.html

Thanks for the work!
So really my only issue with this is that there's no
way for the IOMMU to describe the devices that it
covers.

As a result that is then done in a platform-specific way.

And this means that for example it does not solve the problem that e.g.
some power people have in that their platform simply does not have a way
to specify which devices are covered by the IOMMU.

Solving that problem would make me much more excited about
this device.

On the other hand I can see that while there have been some
developments most of the code has been stable for quite a while now.

So what I am trying to do right about now, is making a small module that
loads early and pokes at the IOMMU sufficiently to get the data about
which devices use the IOMMU out of it using standard virtio config
space.  IIUC it's claimed to be impossible without messy changes to the
boot sequence.

If I succeed at least on some platforms I'll ask that this design is
worked into this device, minimizing info that goes through DT/ACPI.  If
I see I can't make it in time to meet the next merge window, I plan
merging the existing patches using DT (barring surprises).

As I only have a very small amount of time to spend on this attempt, If
someone else wants to try doing that in parallel, that would be great!


> Jean-Philippe Brucker (7):
>   dt-bindings: virtio-mmio: Add IOMMU description
>   dt-bindings: virtio: Add virtio-pci-iommu node
>   of: Allow the iommu-map property to omit untranslated devices
>   PCI: OF: Initialize dev->fwnode appropriately
>   iommu: Add virtio-iommu driver
>   iommu/virtio: Add probe request
>   iommu/virtio: Add event queue
> 
>  .../devicetree/bindings/virtio/iommu.txt  |   66 +
>  .../devicetree/bindings/virtio/mmio.txt   |   30 +
>  MAINTAINERS   |7 +
>  drivers/iommu/Kconfig |   11 +
>  drivers/iommu/Makefile|1 +
>  drivers/iommu/virtio-iommu.c  | 1158 +
>  drivers/of/base.c |   10 +-
>  drivers/pci/of.c  |7 +
>  include/uapi/linux/virtio_ids.h   |1 +
>  include/uapi/linux/virtio_iommu.h |  161 +++
>  10 files changed, 1449 insertions(+), 3 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/virtio/iommu.txt
>  create mode 100644 drivers/iommu/virtio-iommu.c
>  create mode 100644 include/uapi/linux/virtio_iommu.h
> 
> -- 
> 2.19.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 2/3] PCI: iproc: CRS state check in config request

2019-01-18 Thread Bjorn Helgaas
On Fri, Jan 18, 2019 at 09:53:22AM +0530, Srinath Mannam wrote:
> In the current implementation, config read of 0x0001 data
> is assumed as CRS completion. but sometimes 0x0001 can be
> a valid data.
> IPROC PCIe RC has a register to show config request status flags
> like SC, UR, CRS and CA.
> So that extra check is added in the code to confirm the CRS
> state using this register before reissue config request.

s/. but/.  But/  (Sentences start with a capital letter.)

Please wrap this text correctly.  If it's a single paragraph, wrap it so
the lines are filled.  It *looks* like it's intended to be separate
paragraphs; they should be separated by blank lines.

> Signed-off-by: Srinath Mannam 
> Reviewed-by: Ray Jui 
> ---
>  drivers/pci/controller/pcie-iproc.c | 23 +--
>  1 file changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/pcie-iproc.c 
> b/drivers/pci/controller/pcie-iproc.c
> index 13ce80f..ee89d56 100644
> --- a/drivers/pci/controller/pcie-iproc.c
> +++ b/drivers/pci/controller/pcie-iproc.c
> @@ -63,6 +63,10 @@
>  #define APB_ERR_EN_SHIFT 0
>  #define APB_ERR_EN   BIT(APB_ERR_EN_SHIFT)
>  
> +#define CFG_RD_SUCCESS   0
> +#define CFG_RD_UR1
> +#define CFG_RD_CRS   2
> +#define CFG_RD_CA3
>  #define CFG_RETRY_STATUS 0x0001
>  #define CFG_RETRY_STATUS_TIMEOUT_US  50 /* 500 milliseconds */
>  
> @@ -300,6 +304,9 @@ enum iproc_pcie_reg {
>   IPROC_PCIE_IARR4,
>   IPROC_PCIE_IMAP4,
>  
> + /* config read status */
> + IPROC_PCIE_CFG_RD_STATUS,
> +
>   /* link status */
>   IPROC_PCIE_LINK_STATUS,
>  
> @@ -370,6 +377,7 @@ static const u16 iproc_pcie_reg_paxb_v2[] = {
>   [IPROC_PCIE_IMAP3]  = 0xe08,
>   [IPROC_PCIE_IARR4]  = 0xe68,
>   [IPROC_PCIE_IMAP4]  = 0xe70,
> + [IPROC_PCIE_CFG_RD_STATUS]  = 0xee0,
>   [IPROC_PCIE_LINK_STATUS]= 0xf0c,
>   [IPROC_PCIE_APB_ERR_EN] = 0xf40,
>   [IPROC_PCIE_ORDERING_CFG]   = 0x2000,
> @@ -501,10 +509,12 @@ static void __iomem *iproc_pcie_map_ep_cfg_reg(struct 
> iproc_pcie *pcie,
>   return (pcie->base + offset);
>  }
>  
> -static unsigned int iproc_pcie_cfg_retry(void __iomem *cfg_data_p)
> +static unsigned int iproc_pcie_cfg_retry(struct iproc_pcie *pcie,
> +  void __iomem *cfg_data_p)
>  {
>   int timeout = CFG_RETRY_STATUS_TIMEOUT_US;
>   unsigned int data;
> + u32 status;
>  
>   /*
>* As per PCIe spec r3.1, sec 2.3.2, CRS Software Visibility only
> @@ -525,6 +535,15 @@ static unsigned int iproc_pcie_cfg_retry(void __iomem 
> *cfg_data_p)
>*/
>   data = readl(cfg_data_p);
>   while (data == CFG_RETRY_STATUS && timeout--) {
> + /*
> +  * CRS state is set in CFG_RD status register
> +  * This will handle the case where CFG_RETRY_STATUS is
> +  * valid config data.
> +  */
> + status = iproc_pcie_read_reg(pcie, IPROC_PCIE_CFG_RD_STATUS);
> + if (status != CFG_RD_CRS)
> + return data;
> +
>   udelay(1);
>   data = readl(cfg_data_p);
>   }
> @@ -603,7 +622,7 @@ static int iproc_pcie_config_read(struct pci_bus *bus, 
> unsigned int devfn,
>   if (!cfg_data_p)
>   return PCIBIOS_DEVICE_NOT_FOUND;
>  
> - data = iproc_pcie_cfg_retry(cfg_data_p);
> + data = iproc_pcie_cfg_retry(pcie, cfg_data_p);
>  
>   *val = data;
>   if (size <= 2)
> -- 
> 2.7.4
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 1/3] PCI: iproc: Add feature to set order mode

2019-01-18 Thread Bjorn Helgaas
On Fri, Jan 18, 2019 at 09:53:21AM +0530, Srinath Mannam wrote:
> Order mode in RX header of incoming pcie packets can be override to
> strict or loose order based on requirement.
> Sysfs entry is provided to set dynamic and default order modes of upstream
> traffic.

s/pcie/PCIe/

If this is two paragraphs, insert a blank line between.  If it's one
paragraph, reflow it so it reads like one paragraph.

Are you talking about the Relaxed Ordering bit in the TLP Attributes
field?  If so, please use the exact language used in the spec and
include a reference, e.g., to PCIe r4.0, sec 2.2.6.4, 2.4, etc.

I'm hesitant about a new sysfs file for this.  That automatically
creates a user-space dependency on this iProc feature.  Who would use
this file?

> To improve performance in few endpoints we required to modify the
> ordering attributes. Using this feature we can override order modes of RX
> packets at IPROC RC.
> 
> Ex:
> In PCIe based NVMe SSD endpoints data read/writes from disk is using
> Queue pairs (submit/completion). After completion of block read/write,
> EP writes completion command to completion queue to notify RC.
> So that all data transfers before completion command write are not
> required to strict order except completion command. It means previous all
> packets before completion command write to RC should be written to memory
> and acknowledged.

IIUC, if Enable Relaxed Ordering in the endpoint's Device Control
register is set, the device is permitted to set the Relaxed Ordering
bit in TLPs it initiates.  So I would think that if we set Enable
Relaxed Ordering correctly, the NVMe endpoint should be able to
use Relaxed Ordering for the data transfers and strict ordering (the
default) for the completion command.  What am I missing?

This sysfs file apparently affects the Root Port/Root Complex
behavior, not the Endpoint's behavior.  Does that mean iProc ignores
the Relaxed Ordering bit by default, and you're providing a knob that
makes it pay attention to it?  If that's the case, why wouldn't you
enable iProc support for Relaxed Ordering always, by default?

> Signed-off-by: Srinath Mannam 
> Reviewed-by: Ray Jui 
> ---
>  drivers/pci/controller/pcie-iproc.c | 128 
> 
>  drivers/pci/controller/pcie-iproc.h |  16 +
>  2 files changed, 144 insertions(+)
> 
> diff --git a/drivers/pci/controller/pcie-iproc.c 
> b/drivers/pci/controller/pcie-iproc.c
> index c20fd6b..13ce80f 100644
> --- a/drivers/pci/controller/pcie-iproc.c
> +++ b/drivers/pci/controller/pcie-iproc.c
> @@ -57,6 +57,9 @@
>  #define PCIE_DL_ACTIVE_SHIFT 2
>  #define PCIE_DL_ACTIVE   BIT(PCIE_DL_ACTIVE_SHIFT)
>  
> +#define MPS_MRRS_CFG_MPS_SHIFT   0
> +#define MPS_MRRS_CFG_MRRS_SHIFT  16
> +
>  #define APB_ERR_EN_SHIFT 0
>  #define APB_ERR_EN   BIT(APB_ERR_EN_SHIFT)
>  
> @@ -91,6 +94,14 @@
>  
>  #define IPROC_PCIE_REG_INVALID   0x
>  
> +#define RO_FIELD(window) BIT((window) << 1)
> +#define RO_VALUE(window) BIT(((window) << 1) + 1)
> +/* All Windows are allowed */
> +#define RO_ALL_WINDOW0x
> +/* Wait on All Windows */
> +#define RO_FIELD_ALL_WINDOW  0x
> +#define DYNAMIC_ORDER_MODE   0x5
> +
>  /**
>   * iProc PCIe outbound mapping controller specific parameters
>   *
> @@ -295,6 +306,15 @@ enum iproc_pcie_reg {
>   /* enable APB error for unsupported requests */
>   IPROC_PCIE_APB_ERR_EN,
>  
> + /* Ordering Mode configuration registers */
> + IPROC_PCIE_ORDERING_CFG,
> + IPROC_PCIE_MPS_MRRS_CFG,
> + IPROC_PCIE_IMAP0_RO_CONTROL,
> + IPROC_PCIE_IMAP1_RO_CONTROL,
> + IPROC_PCIE_IMAP2_RO_CONTROL,
> + IPROC_PCIE_IMAP3_RO_CONTROL,
> + IPROC_PCIE_IMAP4_RO_CONTROL,
> +
>   /* total number of core registers */
>   IPROC_PCIE_MAX_NUM_REG,
>  };
> @@ -352,6 +372,13 @@ static const u16 iproc_pcie_reg_paxb_v2[] = {
>   [IPROC_PCIE_IMAP4]  = 0xe70,
>   [IPROC_PCIE_LINK_STATUS]= 0xf0c,
>   [IPROC_PCIE_APB_ERR_EN] = 0xf40,
> + [IPROC_PCIE_ORDERING_CFG]   = 0x2000,
> + [IPROC_PCIE_MPS_MRRS_CFG]   = 0x2008,
> + [IPROC_PCIE_IMAP0_RO_CONTROL]   = 0x201c,
> + [IPROC_PCIE_IMAP1_RO_CONTROL]   = 0x2020,
> + [IPROC_PCIE_IMAP2_RO_CONTROL]   = 0x2024,
> + [IPROC_PCIE_IMAP3_RO_CONTROL]   = 0x2028,
> + [IPROC_PCIE_IMAP4_RO_CONTROL]   = 0x202c,
>  };
>  
>  /* iProc PCIe PAXC v1 registers */
> @@ -1401,6 +1428,97 @@ static int iproc_pcie_rev_init(struct iproc_pcie *pcie)
>   return 0;
>  }
>  
> +static
> +ssize_t order_mode_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buff)

Please format the same as the rest of the file, i.e.,

  static ssize_t order_mode_show(struct device *dev, ...

> +{
> + struct pci_host_bridge *host = to_pci_host_bridge(dev);
> + 

Re: use generic DMA mapping code in powerpc V4

2019-01-18 Thread Christian Zigotzky

Hello Christoph,

I was able to compile 257002094bc5935dd63207a380d9698ab81f0775 from your 
Git powerpc-dma.6-debug today.


Unfortunately I don't see any error messages (kernel ring buffer) and I 
don't have a RS232 serial null modem cable to get them.


Cheers,
Christian

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


Re: [PATCH 0/3] Add IPROC PCIe new features

2019-01-18 Thread Bjorn Helgaas
On Fri, Jan 18, 2019 at 09:53:20AM +0530, Srinath Mannam wrote:
> Add changes related to IPROC PCIe RC IP new features.
> 
> This patch set is based on Linux-5.0-rc2.
> 
> Srinath Mannam (3):
>   PCI: iproc: Add feature to set order mode

Since this apparently refers to a PCIe feature, the subject should use the
exact terminology used in the PCIe spec.

>   PCI: iproc: CRS state check in config request

This subject should start with a verb, not "CRS".

>   PCI: iproc: Add PCIe 32bit outbound memory configuration
> 
>  drivers/pci/controller/pcie-iproc.c | 172 
> +++-
>  drivers/pci/controller/pcie-iproc.h |  16 
>  2 files changed, 184 insertions(+), 4 deletions(-)
> 
> -- 
> 2.7.4
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v1] iommu/s390: Declare s390 iommu reserved regions

2019-01-18 Thread Jean-Philippe Brucker
Hi Pierre,

On 18/01/2019 13:29, Pierre Morel wrote:
> On 17/01/2019 14:02, Robin Murphy wrote:
>> On 15/01/2019 17:37, Pierre Morel wrote:
>>> The s390 iommu can only allow DMA transactions between the zPCI device
>>> entries start_dma and end_dma.
>>>
>>> Let's declare the regions before start_dma and after end_dma as
>>> reserved regions using the appropriate callback in iommu_ops.
>>>
>>> The reserved region may later be retrieved from sysfs or from
>>> the vfio iommu internal interface.
>>
>> For this particular case, I think the best solution is to give VFIO
>> the ability to directly interrogate the domain geometry (which s390
>> appears to set correctly already). The idea of reserved regions was
>> really for 'unexpected' holes inside the usable address space - using
>> them to also describe places that are entirely outside that address
>> space rather confuses things IMO.
>>
>> Furthermore, even if we *did* end up going down the route of actively
>> reserving addresses beyond the usable aperture, it doesn't seem
>> sensible for individual drivers to do it themselves when the core API
>> already describes the relevant information generically.
>>
>> Robin.
> 
> Robin,
> 
> I already posted a patch retrieving the geometry through
> VFIO_IOMMU_GET_INFO using a specific capability for the geometry [1],
> and AFAIU, Alex did not agree with this.

On arm we also need to report the IOMMU geometry to userspace (max IOVA
size in particular). Shameer has been working on a solution [2] that
presents a unified view of both geometry and reserved regions into the
VFIO_IOMMU_GET_INFO call, and I think we should go with that. If I
understand correctly it's currently blocked on the RMRR problem and
we're waiting for Jacob or Ashok to take a look at it, as Kevin pinged
them on thread [1]?

[2] https://lkml.org/lkml/2018/4/18/293

Thanks,
Jean

> 
> What is different in what you propose?
> 
> @Alex: I was hoping that this patch goes in your direction. What do you
> think?
> 
> Thanks,
> Pierre
> 
> [1]: https://lore.kernel.org/patchwork/patch/1030369/
> 
>>
>>>
>>> This seems to me related with the work Shameer has started on
>>> vfio_iommu_type1 so I add Alex and Shameer to the CC list.
>>>
>>>
>>> Pierre Morel (1):
>>>    iommu/s390: Declare s390 iommu reserved regions
>>>
>>>   drivers/iommu/s390-iommu.c | 29 +
>>>   1 file changed, 29 insertions(+)
>>>
>>
> 
> 

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

[PATCH v2] dma-debug: add dumping facility via debugfs

2019-01-18 Thread Corentin Labbe
While debugging a DMA mapping leak, I needed to access
debug_dma_dump_mappings() but easily from user space.

This patch adds a /sys/kernel/debug/dma-api/dump file which contain all
current DMA mapping.

Signed-off-by: Corentin Labbe 
---
Changes since v1:
- Use DEFINE_SHOW_ATTRIBUTE
- Add Documentation

 Documentation/DMA-API.txt |  3 +++
 kernel/dma/debug.c| 36 
 2 files changed, 39 insertions(+)

diff --git a/Documentation/DMA-API.txt b/Documentation/DMA-API.txt
index e133ccd60228..78114ee63057 100644
--- a/Documentation/DMA-API.txt
+++ b/Documentation/DMA-API.txt
@@ -696,6 +696,9 @@ dma-api/disabledThis read-only file contains 
the character 'Y'
happen when it runs out of memory or if it was
disabled at boot time
 
+dma-api/dump   This read-only file contains current DMA
+   mappings.
+
 dma-api/error_countThis file is read-only and shows the total
numbers of errors found.
 
diff --git a/kernel/dma/debug.c b/kernel/dma/debug.c
index 23cf5361bcf1..b4827b1c9070 100644
--- a/kernel/dma/debug.c
+++ b/kernel/dma/debug.c
@@ -144,6 +144,7 @@ static struct dentry *num_free_entries_dent __read_mostly;
 static struct dentry *min_free_entries_dent __read_mostly;
 static struct dentry *nr_total_entries_dent __read_mostly;
 static struct dentry *filter_dent   __read_mostly;
+static struct dentry *dump_dent __read_mostly;
 
 /* per-driver filter related state */
 
@@ -840,6 +841,36 @@ static const struct file_operations filter_fops = {
.llseek = default_llseek,
 };
 
+static int dump_show(struct seq_file *seq, void *v)
+{
+   int idx;
+
+   for (idx = 0; idx < HASH_SIZE; idx++) {
+   struct hash_bucket *bucket = _entry_hash[idx];
+   struct dma_debug_entry *entry;
+   unsigned long flags;
+
+   spin_lock_irqsave(>lock, flags);
+
+   list_for_each_entry(entry, >list, list) {
+   seq_printf(seq,
+  "%s %s %s idx %d P=%llx N=%lx D=%llx L=%llx 
%s %s\n",
+  dev_name(entry->dev),
+  dev_driver_string(entry->dev),
+  type2name[entry->type], idx,
+  phys_addr(entry), entry->pfn,
+  entry->dev_addr, entry->size,
+  dir2name[entry->direction],
+  maperr2str[entry->map_err_type]);
+   }
+
+   spin_unlock_irqrestore(>lock, flags);
+   }
+   return 0;
+}
+
+DEFINE_SHOW_ATTRIBUTE(dump);
+
 static int dma_debug_fs_init(void)
 {
dma_debug_dent = debugfs_create_dir("dma-api", NULL);
@@ -894,6 +925,11 @@ static int dma_debug_fs_init(void)
if (!filter_dent)
goto out_err;
 
+   dump_dent = debugfs_create_file("dump", 0444,
+   dma_debug_dent, NULL, _fops);
+   if (!dump_dent)
+   goto out_err;
+
return 0;
 
 out_err:
-- 
2.19.2

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


Re: [PATCH] dma-debug: add dumping facility via debugfs

2019-01-18 Thread LABBE Corentin
On Wed, Jan 16, 2019 at 06:10:13PM +, Robin Murphy wrote:
> On 16/01/2019 13:44, Corentin Labbe wrote:
> > While debugging a DMA mapping leak, I needed to access
> > debug_dma_dump_mappings() but easily from user space.
> > 
> > This patch adds a /sys/kernel/debug/dma-api/dump file which contain all
> > current DMA mapping.
> > 
> > Signed-off-by: Corentin Labbe 
> > ---
> >   kernel/dma/debug.c | 47 ++
> >   1 file changed, 47 insertions(+)
> > 
> > diff --git a/kernel/dma/debug.c b/kernel/dma/debug.c
> > index 23cf5361bcf1..9253382f5729 100644
> > --- a/kernel/dma/debug.c
> > +++ b/kernel/dma/debug.c
> > @@ -144,6 +144,7 @@ static struct dentry *num_free_entries_dent 
> > __read_mostly;
> >   static struct dentry *min_free_entries_dent __read_mostly;
> >   static struct dentry *nr_total_entries_dent __read_mostly;
> >   static struct dentry *filter_dent   __read_mostly;
> > +static struct dentry *dump_dent __read_mostly;
> >   
> >   /* per-driver filter related state */
> >   
> > @@ -840,6 +841,47 @@ static const struct file_operations filter_fops = {
> > .llseek = default_llseek,
> >   };
> >   
> > +static int dump_read(struct seq_file *seq, void *v)
> > +{
> > +   int idx;
> > +
> > +   for (idx = 0; idx < HASH_SIZE; idx++) {
> > +   struct hash_bucket *bucket = _entry_hash[idx];
> > +   struct dma_debug_entry *entry;
> > +   unsigned long flags;
> > +
> > +   spin_lock_irqsave(>lock, flags);
> > +
> > +   list_for_each_entry(entry, >list, list) {
> > +   seq_printf(seq,
> > +  "%s %s %s idx %d P=%llx N=%lx D=%llx L=%llx 
> > %s %s\n",
> > +  dev_name(entry->dev),
> > +  dev_driver_string(entry->dev),
> > +  type2name[entry->type], idx,
> > +  phys_addr(entry), entry->pfn,
> > +  entry->dev_addr, entry->size,
> > +  dir2name[entry->direction],
> > +  maperr2str[entry->map_err_type]);
> > +   }
> > +
> > +   spin_unlock_irqrestore(>lock, flags);
> > +   }
> > +   return 0;
> > +}
> 
> It's a shame that this is pretty much a duplication of 
> debug_dma_dump_mappings(), but there seems no straightforward way to 
> define one in terms of the other :/
> 
> (although given that we'd rather not have that weird external interface 
> anyway, maybe "now you can use debugfs instead" might be justification 
> enough for cleaning it up...)
> 
> > +
> > +static int dump_open(struct inode *inode, struct file *file)
> > +{
> > +   return single_open(file, dump_read, inode->i_private);
> > +}
> > +
> > +static const struct file_operations dump_fops = {
> > +   .owner = THIS_MODULE,
> > +   .open = dump_open,
> > +   .read = seq_read,
> > +   .llseek = seq_lseek,
> > +   .release = single_release,
> > +};
> > +
> 
> DEFINE_SHOW_ATTRIBUTE()?
> 

I will use it

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


Re: [PATCH v1] iommu/s390: Declare s390 iommu reserved regions

2019-01-18 Thread Pierre Morel

On 17/01/2019 14:02, Robin Murphy wrote:

On 15/01/2019 17:37, Pierre Morel wrote:

The s390 iommu can only allow DMA transactions between the zPCI device
entries start_dma and end_dma.

Let's declare the regions before start_dma and after end_dma as
reserved regions using the appropriate callback in iommu_ops.

The reserved region may later be retrieved from sysfs or from
the vfio iommu internal interface.


For this particular case, I think the best solution is to give VFIO the 
ability to directly interrogate the domain geometry (which s390 appears 
to set correctly already). The idea of reserved regions was really for 
'unexpected' holes inside the usable address space - using them to also 
describe places that are entirely outside that address space rather 
confuses things IMO.


Furthermore, even if we *did* end up going down the route of actively 
reserving addresses beyond the usable aperture, it doesn't seem sensible 
for individual drivers to do it themselves when the core API already 
describes the relevant information generically.


Robin.


Robin,

I already posted a patch retrieving the geometry through 
VFIO_IOMMU_GET_INFO using a specific capability for the geometry [1], 
and AFAIU, Alex did not agree with this.


What is different in what you propose?

@Alex: I was hoping that this patch goes in your direction. What do you 
think?


Thanks,
Pierre

[1]: https://lore.kernel.org/patchwork/patch/1030369/





This seems to me related with the work Shameer has started on
vfio_iommu_type1 so I add Alex and Shameer to the CC list.


Pierre Morel (1):
   iommu/s390: Declare s390 iommu reserved regions

  drivers/iommu/s390-iommu.c | 29 +
  1 file changed, 29 insertions(+)






--
Pierre Morel
Linux/KVM/QEMU in Böblingen - Germany

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

Re: use generic DMA mapping code in powerpc V4

2019-01-18 Thread Christoph Hellwig
On Fri, Jan 18, 2019 at 01:46:46PM +0100, Christian Zigotzky wrote:
> Sorry, it's not possible to patch 
> '257002094bc5935dd63207a380d9698ab81f0775' with your patch. I also tried it 
> manually but without any success.

Weird:

hch@carbon:~/work/linux$ git checkout 257002094bc5935dd63207a380d9698ab81f0775
HEAD is now at 257002094bc5 powerpc/dma: use the dma-direct allocator for 
coherent platforms
hch@carbon:~/work/linux$ patch -p1 < dbg.diff 
patching file kernel/dma/direct.c

I've pushed the result to

git://git.infradead.org/users/hch/misc.git

as a new powerpc-dma.6-debug branch
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 355d16acee6d..e46c9b64ec0d 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -118,8 +118,11 @@ struct page *__dma_direct_alloc_pages(struct device *dev, size_t size,
 			page = NULL;
 		}
 	}
-	if (!page)
+	if (!page) {
 		page = alloc_pages_node(dev_to_node(dev), gfp, page_order);
+		if (!page)
+			pr_warn("failed to allocate memory with gfp 0x%x\n", gfp);
+	}
 
 	if (page && !dma_coherent_ok(dev, page_to_phys(page), size)) {
 		__free_pages(page, page_order);
@@ -139,6 +142,10 @@ struct page *__dma_direct_alloc_pages(struct device *dev, size_t size,
 		}
 	}
 
+	if (!page) {
+		pr_warn("failed to allocate DMA memory!\n");
+		dump_stack();
+	}
 	return page;
 }
 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: use generic DMA mapping code in powerpc V4

2019-01-18 Thread Christian Zigotzky
Sorry, it's not possible to patch 
'257002094bc5935dd63207a380d9698ab81f0775' with your patch. I also tried 
it manually but without any success.


-- Christian

On 18 January 2019 at 1:18PM, Christoph Hellwig wrote:

On Fri, Jan 18, 2019 at 01:07:54PM +0100, Christian Zigotzky wrote:

git clone git://git.infradead.org/users/hch/misc.git -b powerpc-dma.6 a

git checkout 257002094bc5935dd63207a380d9698ab81f0775


I get the following error message with your patch:

Hmm.  Did I attached the wrong patch?  Here is the one I want and just
applied to that revision:

diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 355d16acee6d..e46c9b64ec0d 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -118,8 +118,11 @@ struct page *__dma_direct_alloc_pages(struct device *dev, 
size_t size,
page = NULL;
}
}
-   if (!page)
+   if (!page) {
page = alloc_pages_node(dev_to_node(dev), gfp, page_order);
+   if (!page)
+   pr_warn("failed to allocate memory with gfp 0x%x\n", 
gfp);
+   }
  
  	if (page && !dma_coherent_ok(dev, page_to_phys(page), size)) {

__free_pages(page, page_order);
@@ -139,6 +142,10 @@ struct page *__dma_direct_alloc_pages(struct device *dev, 
size_t size,
}
}
  
+	if (!page) {

+   pr_warn("failed to allocate DMA memory!\n");
+   dump_stack();
+   }
return page;
  }
  



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


Re: use generic DMA mapping code in powerpc V4

2019-01-18 Thread Christoph Hellwig
On Fri, Jan 18, 2019 at 01:07:54PM +0100, Christian Zigotzky wrote:
> git clone git://git.infradead.org/users/hch/misc.git -b powerpc-dma.6 a
>
> git checkout 257002094bc5935dd63207a380d9698ab81f0775
>
>
> I get the following error message with your patch:

Hmm.  Did I attached the wrong patch?  Here is the one I want and just
applied to that revision:

diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 355d16acee6d..e46c9b64ec0d 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -118,8 +118,11 @@ struct page *__dma_direct_alloc_pages(struct device *dev, 
size_t size,
page = NULL;
}
}
-   if (!page)
+   if (!page) {
page = alloc_pages_node(dev_to_node(dev), gfp, page_order);
+   if (!page)
+   pr_warn("failed to allocate memory with gfp 0x%x\n", 
gfp);
+   }
 
if (page && !dma_coherent_ok(dev, page_to_phys(page), size)) {
__free_pages(page, page_order);
@@ -139,6 +142,10 @@ struct page *__dma_direct_alloc_pages(struct device *dev, 
size_t size,
}
}
 
+   if (!page) {
+   pr_warn("failed to allocate DMA memory!\n");
+   dump_stack();
+   }
return page;
 }
 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH] iommu/intel: quirk to disable DMAR for QM57 igfx

2019-01-18 Thread Eric Wong
Joonas Lahtinen  wrote:
> Quoting Eric Wong (2019-01-04 03:06:26)
> > Yeah, so the Debian bpo 4.17(.17) kernel did not set
> > CONFIG_INTEL_IOMMU_DEFAULT_ON, so I didn't encounter problems.
> > My self-built kernels all set CONFIG_INTEL_IOMMU_DEFAULT_ON.
> 
> So it's the case that IOMMU never worked on your machine.
> 
> My recommendation would be to simply use intel_iommu=igfx_off if you
> need IOMMU.
> 
> Old hardware is known to have issues with IOMMU, and retroactively
> enabling IOMMU on those machines just brings them up :/

How about we use a quirk in case distros make IOMMU the default
one day?

8<
Subject: [PATCH] iommu/intel: quirk to disable DMAR for QM57 igfx

Like the GM45, it seems the integrated graphics on QM57 seems
broken and hanging graphics with "intel_iommu=on".  So allow
future users to unconditionally enable DMAR support and not have
to remember or know to specify "intel_iommu=igfx_off"

cf. https://lore.kernel.org/lkml/20181227114948.ev4b3jte3ubsc5us@dcvr/
cf. 
https://lore.kernel.org/lkml/154659116310.4596.13613897418163029...@jlahtine-desk.ger.corp.intel.com/

Signed-off-by: Eric Wong 
---
 drivers/iommu/intel-iommu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 048b5ab36a02..dc2507a01580 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -5399,7 +5399,7 @@ const struct iommu_ops intel_iommu_ops = {
 
 static void quirk_iommu_g4x_gfx(struct pci_dev *dev)
 {
-   /* G4x/GM45 integrated gfx dmar support is totally busted. */
+   /* G4x/GM45/QM57 integrated gfx dmar support is totally busted. */
pr_info("Disabling IOMMU for graphics on this chipset\n");
dmar_map_gfx = 0;
 }
@@ -5411,6 +5411,7 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e20, 
quirk_iommu_g4x_gfx);
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e30, quirk_iommu_g4x_gfx);
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e40, quirk_iommu_g4x_gfx);
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e90, quirk_iommu_g4x_gfx);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x0044, quirk_iommu_g4x_gfx);
 
 static void quirk_iommu_rwbf(struct pci_dev *dev)
 {
@@ -5457,7 +5458,6 @@ static void quirk_calpella_no_shadow_gtt(struct pci_dev 
*dev)
}
 }
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x0040, 
quirk_calpella_no_shadow_gtt);
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x0044, 
quirk_calpella_no_shadow_gtt);
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x0062, 
quirk_calpella_no_shadow_gtt);
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x006a, 
quirk_calpella_no_shadow_gtt);
 
-- 
EW
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: use generic DMA mapping code in powerpc V4

2019-01-18 Thread Christian Zigotzky

git clone git://git.infradead.org/users/hch/misc.git -b powerpc-dma.6 a

git checkout 257002094bc5935dd63207a380d9698ab81f0775


I get the following error message with your patch:

patching file a/kernel/dma/direct.c
Hunk #1 FAILED at 118.
Hunk #2 FAILED at 139.
2 out of 2 hunks FAILED -- saving rejects to file a/kernel/dma/direct.c.rej

-- Christian

On 18 January 2019 at 12:28PM, Christoph Hellwig wrote:

On Fri, Jan 18, 2019 at 12:10:26PM +0100, Christian Zigotzky wrote:

For which commit?

On top of 257002094bc5935dd63207a380d9698ab81f0775, that is the first
one you identified as breaking the detection of the SATA disks.



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


[PATCH 3/3] videobuf2: replace a layering violation with dma_map_resource

2019-01-18 Thread Christoph Hellwig
vb2_dc_get_userptr pokes into arm direct mapping details to get the
resemblance of a dma address for a a physical address that does is
not backed by a page struct.  Not only is this not portable to other
architectures with dma direct mapping offsets, but also not to uses
of IOMMUs of any kind.  Switch to the proper dma_map_resource /
dma_unmap_resource interface instead.

Signed-off-by: Christoph Hellwig 
Acked-by: Mauro Carvalho Chehab 
---
 .../common/videobuf2/videobuf2-dma-contig.c   | 41 ---
 1 file changed, 9 insertions(+), 32 deletions(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c 
b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
index aff0ab7bf83d..82389aead6ed 100644
--- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c
+++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
@@ -439,42 +439,14 @@ static void vb2_dc_put_userptr(void *buf_priv)
set_page_dirty_lock(pages[i]);
sg_free_table(sgt);
kfree(sgt);
+   } else {
+   dma_unmap_resource(buf->dev, buf->dma_addr, buf->size,
+  buf->dma_dir, 0);
}
vb2_destroy_framevec(buf->vec);
kfree(buf);
 }
 
-/*
- * For some kind of reserved memory there might be no struct page available,
- * so all that can be done to support such 'pages' is to try to convert
- * pfn to dma address or at the last resort just assume that
- * dma address == physical address (like it has been assumed in earlier version
- * of videobuf2-dma-contig
- */
-
-#ifdef __arch_pfn_to_dma
-static inline dma_addr_t vb2_dc_pfn_to_dma(struct device *dev, unsigned long 
pfn)
-{
-   return (dma_addr_t)__arch_pfn_to_dma(dev, pfn);
-}
-#elif defined(__pfn_to_bus)
-static inline dma_addr_t vb2_dc_pfn_to_dma(struct device *dev, unsigned long 
pfn)
-{
-   return (dma_addr_t)__pfn_to_bus(pfn);
-}
-#elif defined(__pfn_to_phys)
-static inline dma_addr_t vb2_dc_pfn_to_dma(struct device *dev, unsigned long 
pfn)
-{
-   return (dma_addr_t)__pfn_to_phys(pfn);
-}
-#else
-static inline dma_addr_t vb2_dc_pfn_to_dma(struct device *dev, unsigned long 
pfn)
-{
-   /* really, we cannot do anything better at this point */
-   return (dma_addr_t)(pfn) << PAGE_SHIFT;
-}
-#endif
-
 static void *vb2_dc_get_userptr(struct device *dev, unsigned long vaddr,
unsigned long size, enum dma_data_direction dma_dir)
 {
@@ -528,7 +500,12 @@ static void *vb2_dc_get_userptr(struct device *dev, 
unsigned long vaddr,
for (i = 1; i < n_pages; i++)
if (nums[i-1] + 1 != nums[i])
goto fail_pfnvec;
-   buf->dma_addr = vb2_dc_pfn_to_dma(buf->dev, nums[0]);
+   buf->dma_addr = dma_map_resource(buf->dev,
+   __pfn_to_phys(nums[0]), size, buf->dma_dir, 0);
+   if (dma_mapping_error(buf->dev, buf->dma_addr)) {
+   ret = -ENOMEM;
+   goto fail_pfnvec;
+   }
goto out;
}
 
-- 
2.20.1

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


[PATCH 1/3] dma-mapping: remove the default map_resource implementation

2019-01-18 Thread Christoph Hellwig
Instead provide a proper implementation in the direct mapping code, and
also wire it up for arm and powerpc, leaving an error return for all the
IOMMU or virtual mapping instances for which we'd have to wire up an
actual implementation

Signed-off-by: Christoph Hellwig 
---
 arch/arm/mm/dma-mapping.c |  2 ++
 arch/powerpc/kernel/dma-swiotlb.c |  1 +
 arch/powerpc/kernel/dma.c |  1 +
 include/linux/dma-mapping.h   | 12 +++-
 kernel/dma/direct.c   | 14 ++
 5 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index f1e2922e447c..3c8534904209 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -188,6 +188,7 @@ const struct dma_map_ops arm_dma_ops = {
.unmap_page = arm_dma_unmap_page,
.map_sg = arm_dma_map_sg,
.unmap_sg   = arm_dma_unmap_sg,
+   .map_resource   = dma_direct_map_resource,
.sync_single_for_cpu= arm_dma_sync_single_for_cpu,
.sync_single_for_device = arm_dma_sync_single_for_device,
.sync_sg_for_cpu= arm_dma_sync_sg_for_cpu,
@@ -211,6 +212,7 @@ const struct dma_map_ops arm_coherent_dma_ops = {
.get_sgtable= arm_dma_get_sgtable,
.map_page   = arm_coherent_dma_map_page,
.map_sg = arm_dma_map_sg,
+   .map_resource   = dma_direct_map_resource,
.dma_supported  = arm_dma_supported,
 };
 EXPORT_SYMBOL(arm_coherent_dma_ops);
diff --git a/arch/powerpc/kernel/dma-swiotlb.c 
b/arch/powerpc/kernel/dma-swiotlb.c
index 7d5fc9751622..fbb2506a414e 100644
--- a/arch/powerpc/kernel/dma-swiotlb.c
+++ b/arch/powerpc/kernel/dma-swiotlb.c
@@ -55,6 +55,7 @@ const struct dma_map_ops powerpc_swiotlb_dma_ops = {
.dma_supported = swiotlb_dma_supported,
.map_page = dma_direct_map_page,
.unmap_page = dma_direct_unmap_page,
+   .map_resource = dma_direct_map_resource,
.sync_single_for_cpu = dma_direct_sync_single_for_cpu,
.sync_single_for_device = dma_direct_sync_single_for_device,
.sync_sg_for_cpu = dma_direct_sync_sg_for_cpu,
diff --git a/arch/powerpc/kernel/dma.c b/arch/powerpc/kernel/dma.c
index b1903ebb2e9c..258b9e8ebb99 100644
--- a/arch/powerpc/kernel/dma.c
+++ b/arch/powerpc/kernel/dma.c
@@ -273,6 +273,7 @@ const struct dma_map_ops dma_nommu_ops = {
.dma_supported  = dma_nommu_dma_supported,
.map_page   = dma_nommu_map_page,
.unmap_page = dma_nommu_unmap_page,
+   .map_resource   = dma_direct_map_resource,
.get_required_mask  = dma_nommu_get_required_mask,
 #ifdef CONFIG_NOT_COHERENT_CACHE
.sync_single_for_cpu= dma_nommu_sync_single,
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index f6ded992c183..9842085e6774 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -208,6 +208,8 @@ dma_addr_t dma_direct_map_page(struct device *dev, struct 
page *page,
unsigned long attrs);
 int dma_direct_map_sg(struct device *dev, struct scatterlist *sgl, int nents,
enum dma_data_direction dir, unsigned long attrs);
+dma_addr_t dma_direct_map_resource(struct device *dev, phys_addr_t paddr,
+   size_t size, enum dma_data_direction dir, unsigned long attrs);
 
 #if defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_DEVICE) || \
 defined(CONFIG_SWIOTLB)
@@ -346,19 +348,19 @@ static inline dma_addr_t dma_map_resource(struct device 
*dev,
  unsigned long attrs)
 {
const struct dma_map_ops *ops = get_dma_ops(dev);
-   dma_addr_t addr;
+   dma_addr_t addr = DMA_MAPPING_ERROR;
 
BUG_ON(!valid_dma_direction(dir));
 
/* Don't allow RAM to be mapped */
BUG_ON(pfn_valid(PHYS_PFN(phys_addr)));
 
-   addr = phys_addr;
-   if (ops && ops->map_resource)
+   if (dma_is_direct(ops))
+   addr = dma_direct_map_resource(dev, phys_addr, size, dir, 
attrs);
+   else if (ops->map_resource)
addr = ops->map_resource(dev, phys_addr, size, dir, attrs);
 
debug_dma_map_resource(dev, phys_addr, size, dir, addr);
-
return addr;
 }
 
@@ -369,7 +371,7 @@ static inline void dma_unmap_resource(struct device *dev, 
dma_addr_t addr,
const struct dma_map_ops *ops = get_dma_ops(dev);
 
BUG_ON(!valid_dma_direction(dir));
-   if (ops && ops->unmap_resource)
+   if (!dma_is_direct(ops) && ops->unmap_resource)
ops->unmap_resource(dev, addr, size, dir, attrs);
debug_dma_unmap_resource(dev, addr, size, dir);
 }
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 355d16acee6d..25bd19974223 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -356,6 +356,20 @@ int 

[PATCH 2/3] dma-mapping: don't BUG when calling dma_map_resource on RAM

2019-01-18 Thread Christoph Hellwig
Use WARN_ON_ONCE to print a stack trace and return a proper error
code instead.

Signed-off-by: Christoph Hellwig 
Reviewed-by: Robin Murphy 
---
 include/linux/dma-mapping.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 9842085e6774..b904d55247ab 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -353,7 +353,8 @@ static inline dma_addr_t dma_map_resource(struct device 
*dev,
BUG_ON(!valid_dma_direction(dir));
 
/* Don't allow RAM to be mapped */
-   BUG_ON(pfn_valid(PHYS_PFN(phys_addr)));
+   if (WARN_ON_ONCE(pfn_valid(PHYS_PFN(phys_addr
+   return DMA_MAPPING_ERROR;
 
if (dma_is_direct(ops))
addr = dma_direct_map_resource(dev, phys_addr, size, dir, 
attrs);
-- 
2.20.1

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


fix a layering violation in videobuf2 and improve dma_map_resource v2

2019-01-18 Thread Christoph Hellwig
Hi all,

this series fixes a rather gross layering violation in videobuf2, which
pokes into arm DMA mapping internals to get a DMA address for memory that
does not have a page structure, and to do so fixes up the dma_map_resource
implementation to not provide a somewhat dangerous default and improve
the error handling.

Changes since v1:
 - don't apply bus offsets in dma_direct_map_resource
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH] dma-debug: add dumping facility via debugfs

2019-01-18 Thread Christoph Hellwig
On Wed, Jan 16, 2019 at 06:10:13PM +, Robin Murphy wrote:
> It's a shame that this is pretty much a duplication of 
> debug_dma_dump_mappings(), but there seems no straightforward way to define 
> one in terms of the other :/

We could always play some macro magic, but I think that is worse than
duplicating a little bit of functionality.

Btw, the dev argument to debug_dma_dump_mappings is always NULL and
could be removed..

> (although given that we'd rather not have that weird external interface 
> anyway, maybe "now you can use debugfs instead" might be justification 
> enough for cleaning it up...)

One argument against that is the system might be in a shape where you
can't easily read a file when it is in that shape.  The argument for
that is that in many systems the full list of mappings might overwhelm
the kernel log.

Adding Joerg, who originally wrote the code in case he has an opinion.

>
>> +
>> +static int dump_open(struct inode *inode, struct file *file)
>> +{
>> +return single_open(file, dump_read, inode->i_private);
>> +}
>> +
>> +static const struct file_operations dump_fops = {
>> +.owner = THIS_MODULE,
>> +.open = dump_open,
>> +.read = seq_read,
>> +.llseek = seq_lseek,
>> +.release = single_release,
>> +};
>> +
>
> DEFINE_SHOW_ATTRIBUTE()?
>
> Robin.
>
>>   static int dma_debug_fs_init(void)
>>   {
>>  dma_debug_dent = debugfs_create_dir("dma-api", NULL);
>> @@ -894,6 +936,11 @@ static int dma_debug_fs_init(void)
>>  if (!filter_dent)
>>  goto out_err;
>>   +  dump_dent = debugfs_create_file("dump", 0444,
>> +dma_debug_dent, NULL, _fops);
>> +if (!dump_dent)
>> +goto out_err;
>> +
>>  return 0;
>> out_err:
>>
---end quoted text---
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: use generic DMA mapping code in powerpc V4

2019-01-18 Thread Christoph Hellwig
On Fri, Jan 18, 2019 at 12:10:26PM +0100, Christian Zigotzky wrote:
> For which commit?

On top of 257002094bc5935dd63207a380d9698ab81f0775, that is the first
one you identified as breaking the detection of the SATA disks.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: use generic DMA mapping code in powerpc V4

2019-01-18 Thread Christian Zigotzky

For which commit?

-- Christian

On 18 January 2019 at 09:35AM, Christoph Hellwig wrote:

Hi Christian,

can you check if the debug printks in this patch trigger?

diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 355d16acee6d..e46c9b64ec0d 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -118,8 +118,11 @@ struct page *__dma_direct_alloc_pages(struct device *dev, 
size_t size,
page = NULL;
}
}
-   if (!page)
+   if (!page) {
page = alloc_pages_node(dev_to_node(dev), gfp, page_order);
+   if (!page)
+   pr_warn("failed to allocate memory with gfp 0x%x\n", 
gfp);
+   }
  
  	if (page && !dma_coherent_ok(dev, page_to_phys(page), size)) {

__free_pages(page, page_order);
@@ -139,6 +142,10 @@ struct page *__dma_direct_alloc_pages(struct device *dev, 
size_t size,
}
}
  
+	if (!page) {

+   pr_warn("failed to allocate DMA memory!\n");
+   dump_stack();
+   }
return page;
  }
  



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


[PATCH] dmaengine: fsl-edma: dma map slave device address

2019-01-18 Thread Laurentiu Tudor
This mapping needs to be created in order for slave dma transfers
to work on systems with SMMU. The implementation mostly mimics the
one in pl330 dma driver, authored by Robin Murphy.

Signed-off-by: Laurentiu Tudor 
Suggested-by: Robin Murphy 
---
Original approach was to add the missing mappings in the i2c client driver,
see here for discussion: https://patchwork.ozlabs.org/patch/1026013/

 drivers/dma/fsl-edma-common.c | 66 ---
 drivers/dma/fsl-edma-common.h |  4 +++
 drivers/dma/fsl-edma.c|  1 +
 drivers/dma/mcf-edma.c|  1 +
 4 files changed, 68 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/fsl-edma-common.c b/drivers/dma/fsl-edma-common.c
index 8876c4c1bb2c..0e95ee24b6d4 100644
--- a/drivers/dma/fsl-edma-common.c
+++ b/drivers/dma/fsl-edma-common.c
@@ -6,6 +6,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "fsl-edma-common.h"
 
@@ -173,12 +174,62 @@ int fsl_edma_resume(struct dma_chan *chan)
 }
 EXPORT_SYMBOL_GPL(fsl_edma_resume);
 
+static void fsl_edma_unprep_slave_dma(struct fsl_edma_chan *fsl_chan)
+{
+   if (fsl_chan->dma_dir != DMA_NONE)
+   dma_unmap_resource(fsl_chan->vchan.chan.device->dev,
+  fsl_chan->dma_dev_addr,
+  fsl_chan->dma_dev_size,
+  fsl_chan->dma_dir, 0);
+   fsl_chan->dma_dir = DMA_NONE;
+}
+
+static bool fsl_edma_prep_slave_dma(struct fsl_edma_chan *fsl_chan,
+   enum dma_transfer_direction dir)
+{
+   struct device *dev = fsl_chan->vchan.chan.device->dev;
+   enum dma_data_direction dma_dir;
+   phys_addr_t addr = 0;
+   u32 size = 0;
+
+   switch (dir) {
+   case DMA_MEM_TO_DEV:
+   dma_dir = DMA_FROM_DEVICE;
+   addr = fsl_chan->cfg.dst_addr;
+   size = fsl_chan->cfg.dst_maxburst;
+   break;
+   case DMA_DEV_TO_MEM:
+   dma_dir = DMA_TO_DEVICE;
+   addr = fsl_chan->cfg.src_addr;
+   size = fsl_chan->cfg.src_maxburst;
+   break;
+   default:
+   dma_dir = DMA_NONE;
+   break;
+   }
+
+   /* Already mapped for this config? */
+   if (fsl_chan->dma_dir == dma_dir)
+   return true;
+
+   fsl_edma_unprep_slave_dma(fsl_chan);
+
+   fsl_chan->dma_dev_addr = dma_map_resource(dev, addr, size, dma_dir, 0);
+   if (dma_mapping_error(dev, fsl_chan->dma_dev_addr))
+   return false;
+   fsl_chan->dma_dev_size = size;
+   fsl_chan->dma_dir = dma_dir;
+
+   return true;
+}
+
 int fsl_edma_slave_config(struct dma_chan *chan,
 struct dma_slave_config *cfg)
 {
struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
 
memcpy(_chan->cfg, cfg, sizeof(*cfg));
+   fsl_edma_unprep_slave_dma(fsl_chan);
 
return 0;
 }
@@ -378,6 +429,9 @@ struct dma_async_tx_descriptor *fsl_edma_prep_dma_cyclic(
if (!is_slave_direction(direction))
return NULL;
 
+   if (!fsl_edma_prep_slave_dma(fsl_chan, direction))
+   return NULL;
+
sg_len = buf_len / period_len;
fsl_desc = fsl_edma_alloc_desc(fsl_chan, sg_len);
if (!fsl_desc)
@@ -409,11 +463,11 @@ struct dma_async_tx_descriptor *fsl_edma_prep_dma_cyclic(
 
if (direction == DMA_MEM_TO_DEV) {
src_addr = dma_buf_next;
-   dst_addr = fsl_chan->cfg.dst_addr;
+   dst_addr = fsl_chan->dma_dev_addr;
soff = fsl_chan->cfg.dst_addr_width;
doff = 0;
} else {
-   src_addr = fsl_chan->cfg.src_addr;
+   src_addr = fsl_chan->dma_dev_addr;
dst_addr = dma_buf_next;
soff = 0;
doff = fsl_chan->cfg.src_addr_width;
@@ -444,6 +498,9 @@ struct dma_async_tx_descriptor *fsl_edma_prep_slave_sg(
if (!is_slave_direction(direction))
return NULL;
 
+   if (!fsl_edma_prep_slave_dma(fsl_chan, direction))
+   return NULL;
+
fsl_desc = fsl_edma_alloc_desc(fsl_chan, sg_len);
if (!fsl_desc)
return NULL;
@@ -468,11 +525,11 @@ struct dma_async_tx_descriptor *fsl_edma_prep_slave_sg(
 
if (direction == DMA_MEM_TO_DEV) {
src_addr = sg_dma_address(sg);
-   dst_addr = fsl_chan->cfg.dst_addr;
+   dst_addr = fsl_chan->dma_dev_addr;
soff = fsl_chan->cfg.dst_addr_width;
doff = 0;
} else {
-   src_addr = fsl_chan->cfg.src_addr;
+   src_addr = fsl_chan->dma_dev_addr;
dst_addr = sg_dma_address(sg);
soff = 0;

Re: [RFC v3 00/21] SMMUv3 Nested Stage Setup

2019-01-18 Thread Auger Eric
Hi,
On 1/8/19 11:26 AM, Eric Auger wrote:
> This series allows a virtualizer to program the nested stage mode.
> This is useful when both the host and the guest are exposed with
> an SMMUv3 and a PCI device is assigned to the guest using VFIO.
> 
> In this mode, the physical IOMMU must be programmed to translate
> the two stages: the one set up by the guest (IOVA -> GPA) and the
> one set up by the host VFIO driver as part of the assignment process
> (GPA -> HPA).
> 
> On Intel, this is traditionnaly achieved by combining the 2 stages
> into a single physical stage. However this relies on the capability
> to trap on each guest translation structure update. This is possible
> by using the VTD Caching Mode. Unfortunately the ARM SMMUv3 does
> not offer a similar mechanism.
> 
> However, the ARM SMMUv3 architecture supports 2 physical stages! Those
> were devised exactly with that use case in mind. Assuming the HW
> implements both stages (optional), the guest now can use stage 1
> while the host uses stage 2.
> 
> This assumes the virtualizer has means to propagate guest settings
> to the host SMMUv3 driver. This series brings this VFIO/IOMMU
> infrastructure.  Those services are:
> - bind the guest stage 1 configuration to the stream table entry
> - propagate guest TLB invalidations
> - bind MSI IOVAs
> - propagate faults collected at physical level up to the virtualizer
> 
> This series largely reuses the user API and infrastructure originally
> devised for SVA/SVM and patches submitted by Jacob, Yi Liu, Tianyu in
> [1-2] and Jean-Philippe [3-4].
> 
> Best Regards
> 
> Eric
> 
> This series can be found at:
> https://github.com/eauger/linux/tree/v5.0-rc1-2stage-rfc-v3
If someone is willing to test this series with QEMU you can use the
branch below, until I send a formal respin.

https://github.com/eauger/qemu/commits/v3.1.0-rc5-2stage-v3-for-rfc3-test-only.


Thanks

Eric


> 
> This was tested on Qualcomm HW featuring SMMUv3 and with adapted QEMU
> vSMMUv3.
> 
> References:
> [1] [PATCH v5 00/23] IOMMU and VT-d driver support for Shared Virtual
> Address (SVA)
> https://lwn.net/Articles/754331/
> [2] [RFC PATCH 0/8] Shared Virtual Memory virtualization for VT-d
> (VFIO part)
> https://lists.linuxfoundation.org/pipermail/iommu/2017-April/021475.html
> [3] [v2,00/40] Shared Virtual Addressing for the IOMMU
> https://patchwork.ozlabs.org/cover/912129/
> [4] [PATCH v3 00/10] Shared Virtual Addressing for the IOMMU
> https://patchwork.kernel.org/cover/10608299/
> 
> History:
> 
> v2 -> v3:
> - When registering the S1 MSI binding we now store the device handle. This
>   addresses Robin's comment about discimination of devices beonging to
>   different S1 groups and using different physical MSI doorbells.
> - Change the fault reporting API: use VFIO_PCI_DMA_FAULT_IRQ_INDEX to
>   set the eventfd and expose the faults through an mmappable fault region
> 
> v1 -> v2:
> - Added the fault reporting capability
> - asid properly passed on invalidation (fix assignment of multiple
>   devices)
> - see individual change logs for more info
> 
> Eric Auger (12):
>   iommu: Introduce bind_guest_msi
>   vfio: VFIO_IOMMU_BIND_MSI
>   iommu/smmuv3: Get prepared for nested stage support
>   iommu/smmuv3: Implement set_pasid_table
>   iommu/smmuv3: Implement cache_invalidate
>   dma-iommu: Implement NESTED_MSI cookie
>   iommu/smmuv3: Implement bind_guest_msi
>   iommu/smmuv3: Report non recoverable faults
>   vfio-pci: Add a new VFIO_REGION_TYPE_NESTED region type
>   vfio-pci: Register an iommu fault handler
>   vfio-pci: Add VFIO_PCI_DMA_FAULT_IRQ_INDEX
>   vfio: Document nested stage control
> 
> Jacob Pan (4):
>   iommu: Introduce set_pasid_table API
>   iommu: introduce device fault data
>   driver core: add per device iommu param
>   iommu: introduce device fault report API
> 
> Jean-Philippe Brucker (2):
>   iommu/arm-smmu-v3: Link domains and devices
>   iommu/arm-smmu-v3: Maintain a SID->device structure
> 
> Liu, Yi L (3):
>   iommu: Introduce cache_invalidate API
>   vfio: VFIO_IOMMU_SET_PASID_TABLE
>   vfio: VFIO_IOMMU_CACHE_INVALIDATE
> 
>  Documentation/vfio.txt  |  62 
>  drivers/iommu/arm-smmu-v3.c | 460 ++--
>  drivers/iommu/dma-iommu.c   | 112 ++-
>  drivers/iommu/iommu.c   | 187 ++-
>  drivers/vfio/pci/vfio_pci.c | 147 -
>  drivers/vfio/pci/vfio_pci_intrs.c   |  19 ++
>  drivers/vfio/pci/vfio_pci_private.h |   3 +
>  drivers/vfio/vfio_iommu_type1.c | 105 +++
>  include/linux/device.h  |   3 +
>  include/linux/dma-iommu.h   |  11 +
>  include/linux/iommu.h   | 127 +++-
>  include/uapi/linux/iommu.h  | 234 ++
>  include/uapi/linux/vfio.h   |  38 +++
>  13 files changed, 1476 insertions(+), 32 deletions(-)
>  create mode 100644 include/uapi/linux/iommu.h
> 
___
iommu mailing list

Re: [PATCH 3/3] videobuf2: replace a layering violation with dma_map_resource

2019-01-18 Thread Marek Szyprowski
Hi Christoph,

On 2019-01-17 18:21, Christoph Hellwig wrote:
> On Mon, Jan 14, 2019 at 01:42:26PM +0100, Marek Szyprowski wrote:
>> On 2019-01-11 19:17, Christoph Hellwig wrote:
>>> vb2_dc_get_userptr pokes into arm direct mapping details to get the
>>> resemblance of a dma address for a a physical address that does is
>>> not backed by a page struct.  Not only is this not portable to other
>>> architectures with dma direct mapping offsets, but also not to uses
>>> of IOMMUs of any kind.  Switch to the proper dma_map_resource /
>>> dma_unmap_resource interface instead.
>>>
>>> Signed-off-by: Christoph Hellwig 
>> There are checks for IOMMU presence in other places in vb2-dma-contig,
>> so it was used only when no IOMMU is available, but I agree that the
>> hacky code should be replaced by a generic code if possible.
>>
>> Tested-by: Marek Szyprowski 
>>
>> V4L2 pipeline works fine on older Exynos-based boards with CMA and IOMMU
>> disabled.
> Do you know if these rely on the offsets?  E.g. would they still work
> with the patch below applied on top.  That would keep the map_resource
> semantics as-is as solve the issue pointed out by Robin for now.

AFAIK that code was only used for sharing buffers between hardware
modules that are a part of the same SoC, usually implemented as platform
devices. AFAIR this never worked for devices on different buses. So far
I wasn't aware on ANY which would require an offset for the DMA access.

The first version of videobuf2-dc code even incorrectly used paddr
instead of dma_addr as a buffer 'address' returned to the client
drivers, because in case of those SoC this was exactly the same (see
commits 472af2b05bdefcaee7e754e22cbf131110017ad6 and
ba7fcb0c954921534707f08ebc4d8beeb2eb17e7).

> If not I can only think of a flag to bypass the offseting for now, but
> that would be pretty ugly.  Or go for the long-term solution of
> discovering the relationship between the two devices, as done by the
> PCIe P2P code..
>
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index 8e0359b04957..25bd19974223 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -359,7 +359,7 @@ EXPORT_SYMBOL(dma_direct_map_sg);
>  dma_addr_t dma_direct_map_resource(struct device *dev, phys_addr_t paddr,
>   size_t size, enum dma_data_direction dir, unsigned long attrs)
>  {
> - dma_addr_t dma_addr = phys_to_dma(dev, paddr);
> + dma_addr_t dma_addr = paddr;
>  
>   if (unlikely(!dma_direct_possible(dev, dma_addr, size))) {
>   report_addr(dev, dma_addr, size);
>
>
>
Best regards
-- 
Marek Szyprowski, PhD
Samsung R Institute Poland

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


Re: use generic DMA mapping code in powerpc V4

2019-01-18 Thread Christoph Hellwig
Hi Christian,

can you check if the debug printks in this patch trigger?

diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 355d16acee6d..e46c9b64ec0d 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -118,8 +118,11 @@ struct page *__dma_direct_alloc_pages(struct device *dev, 
size_t size,
page = NULL;
}
}
-   if (!page)
+   if (!page) {
page = alloc_pages_node(dev_to_node(dev), gfp, page_order);
+   if (!page)
+   pr_warn("failed to allocate memory with gfp 0x%x\n", 
gfp);
+   }
 
if (page && !dma_coherent_ok(dev, page_to_phys(page), size)) {
__free_pages(page, page_order);
@@ -139,6 +142,10 @@ struct page *__dma_direct_alloc_pages(struct device *dev, 
size_t size,
}
}
 
+   if (!page) {
+   pr_warn("failed to allocate DMA memory!\n");
+   dump_stack();
+   }
return page;
 }
 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu