Re: [PATCH 3/6] sparc: remove the sparc32_dma_ops indirection

2018-12-14 Thread Guenter Roeck 
On Sat, Dec 08, 2018 at 09:41:12AM -0800, Christoph Hellwig wrote:
> There is no good reason to have a double indirection for the sparc32
> dma ops, so remove the sparc32_dma_ops and define separate dma_map_ops
> instance for the different IOMMU types.
> 

Except maybe this:

scsi host0: esp
scsi host0: Data transfer overflow.
scsi host0: cur_residue[0] tot_residue[-36] len[36]
scsi 0:0:0:0: Direct-Access QEMU QEMU HARDDISK2.5+ PQ: 0 ANSI: 5
scsi target0:0:0: Beginning Domain Validation
scsi host0: Data transfer overflow.
scsi host0: cur_residue[0] tot_residue[-36] len[36]
scsi host0: Data transfer overflow.
scsi host0: cur_residue[0] tot_residue[-36] len[36]
scsi host0: Data transfer overflow.
scsi host0: cur_residue[0] tot_residue[-36] len[36]
scsi host0: Data transfer overflow.
scsi host0: cur_residue[0] tot_residue[-36] len[36]

and so on, until qemu is terminated. This is seen with all sparc32
qemu emulations. Reverting the patch fixes the problem.

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

Re: [RFC v2 14/20] iommu: introduce device fault data

2018-12-14 Thread Jacob Pan 
On Wed, 12 Dec 2018 09:21:43 +0100
Auger Eric  wrote:

> Hi Jacob,
> 
> On 9/21/18 12:06 AM, Jacob Pan wrote:
> > On Tue, 18 Sep 2018 16:24:51 +0200
> > Eric Auger  wrote:
> >   
> >> From: Jacob Pan 
> >>
> >> Device faults detected by IOMMU can be reported outside IOMMU
> >> subsystem for further processing. This patch intends to provide
> >> a generic device fault data such that device drivers can be
> >> communicated with IOMMU faults without model specific knowledge.
> >>
> >> The proposed format is the result of discussion at:
> >> https://lkml.org/lkml/2017/11/10/291
> >> Part of the code is based on Jean-Philippe Brucker's patchset
> >> (https://patchwork.kernel.org/patch/9989315/).
> >>
> >> The assumption is that model specific IOMMU driver can filter and
> >> handle most of the internal faults if the cause is within IOMMU
> >> driver control. Therefore, the fault reasons can be reported are
> >> grouped and generalized based common specifications such as PCI
> >> ATS.
> >>
> >> Signed-off-by: Jacob Pan 
> >> Signed-off-by: Jean-Philippe Brucker
> >>  Signed-off-by: Liu, Yi L
> >>  Signed-off-by: Ashok Raj
> >>  Signed-off-by: Eric Auger
> >>  [moved part of the iommu_fault_event
> >> struct in the uapi, enriched the fault reasons to be able to map
> >> unrecoverable SMMUv3 errors]  
> > Sounds good to me.
> > There are also other "enrichment" we need to do to support mdev or
> > finer granularity fault reporting below physical device. e.g. PASID
> > level.
> > 
> > The current scheme works for PCIe physical device level, where each
> > device registers a single handler only once. When device fault is
> > detected by the IOMMU, it will find the matching handler and private
> > data to report back. However, for devices partitioned by PASID and
> > represented by mdev this may not work. Since IOMMU is not mdev aware
> > and only works at physical device level.
> > So I am thinking we should allow multiple registration of fault
> > handler with different data and ID. i.e.
> > 
> > int iommu_register_device_fault_handler(struct device *dev,
> > iommu_dev_fault_handler_t
> > handler, int id,
> > void *data)
> > 
> > where the new "id field" is
> >  * @id: Identification of the handler private data, will be used by
> > fault
> >  *  reporting code to match the handler data to be returned.
> > For page
> >  *  request, this can be the PASID. ID must be unique per
> > device, i.e.
> >  *  each ID can only be registered once per device.
> >  *  - IOMMU_DEV_FAULT_ID_UNRECOVERY (~0U) is reserved for fault
> > reporting
> >  *  w/o ID. e.g. unrecoverable faults.
> > 
> > I am still testing, but just wanted to have feedback on this idea.  
> 
> I am currently respinning this series. Do you have a respin for this
> patch including iommu_register_device_fault_handler with the @id param
> as you suggested above? Otherwise 2 solutions: I keep the code as is
> or I do the modification myself implementing a list of fault_params?
> 
you can keep the code as is if it fits your current needs. Yi and I
have thought of some new cases for supporting mdev. We are thinking to
support many to many handler vs PASID relationship. i.e. allow
registration of many fault handlers per device, each associated with an
ID and data. The use case is that a physical device may register a
fault handler for its own PASID or non-PASID related faults. Such
physical device can also be partitioned into sub-device, e.g. mdev, but
fault handler registration is at physical device level in that IOMMU is
not mdev aware.
Anyway, still need some work to flush out the details.
> Besides do you plans for "[PATCH v5 00/23] IOMMU and VT-d driver
> support for Shared Virtual Address (SVA)" respin - hope I didn't miss
> anything? - ?
> 
You did not miss anything. Yes, we are still working on some internal
integration issues. It should not affect the common interface much. Or
I can send out a common API spin first once we have the functionality
tested.

Thanks for checking.

> Thanks
> 
> Eric
> > 
> > Thanks,
> > 
> > Jacob
> > 
> >   
> >> ---
> >>  include/linux/iommu.h  | 55 -
> >>  include/uapi/linux/iommu.h | 83
> >> ++ 2 files changed, 136
> >> insertions(+), 2 deletions(-)
> >>
> >> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> >> index 9bd3e63d562b..7529c14ff506 100644
> >> --- a/include/linux/iommu.h
> >> +++ b/include/linux/iommu.h
> >> @@ -49,13 +49,17 @@ struct bus_type;
> >>  struct device;
> >>  struct iommu_domain;
> >>  struct notifier_block;
> >> +struct iommu_fault_event;
> >>  
> >>  /* iommu fault flags */
> >> -#define IOMMU_FAULT_READ  0x0
> >> -#define IOMMU_FAULT_WRITE 0x1
> >> +#define IOMMU_FAULT_READ  (1 << 0)
> >> +#define IOMMU_FAULT_WRITE (1 << 1)
> >> +#define IOMMU_FAULT_EXEC  (1 << 2)
> >> +#define IOMMU_FAULT_PRIV  (1 <<

Re: [PATCH 14/15] vmd: use the proper dma_* APIs instead of direct methods calls

2018-12-14 Thread Derrick, Jonathan 
Looks good to me
Thanks Christoph

Acked-by: Jon Derrick 

On Fri, 2018-12-14 at 15:17 -0600, Bjorn Helgaas wrote:
> Conventional spelling in subject is
> 
>   PCI: vmd: Use dma_* APIs instead of direct method calls
> 
> On Fri, Dec 07, 2018 at 11:07:19AM -0800, Christoph Hellwig wrote:
> > With the bypass support for the direct mapping we might not always have
> > methods to call, so use the proper APIs instead.  The only downside is
> > that we will create two dma-debug entries for each mapping if
> > CONFIG_DMA_DEBUG is enabled.
> > 
> > Signed-off-by: Christoph Hellwig 
> 
> You cc'd the VMD maintainers already, and I have no objection to this
> from a PCI core point of view, so:
> 
> Acked-by: Bjorn Helgaas 
> 
> > ---
> >  drivers/pci/controller/vmd.c | 42 +++-
> >  1 file changed, 17 insertions(+), 25 deletions(-)
> > 
> > diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> > index 98ce79eac128..3890812cdf87 100644
> > --- a/drivers/pci/controller/vmd.c
> > +++ b/drivers/pci/controller/vmd.c
> > @@ -307,39 +307,32 @@ static struct device *to_vmd_dev(struct device *dev)
> > return >dev->dev;
> >  }
> >  
> > -static const struct dma_map_ops *vmd_dma_ops(struct device *dev)
> > -{
> > -   return get_dma_ops(to_vmd_dev(dev));
> > -}
> > -
> >  static void *vmd_alloc(struct device *dev, size_t size, dma_addr_t *addr,
> >gfp_t flag, unsigned long attrs)
> >  {
> > -   return vmd_dma_ops(dev)->alloc(to_vmd_dev(dev), size, addr, flag,
> > -  attrs);
> > +   return dma_alloc_attrs(to_vmd_dev(dev), size, addr, flag, attrs);
> >  }
> >  
> >  static void vmd_free(struct device *dev, size_t size, void *vaddr,
> >  dma_addr_t addr, unsigned long attrs)
> >  {
> > -   return vmd_dma_ops(dev)->free(to_vmd_dev(dev), size, vaddr, addr,
> > - attrs);
> > +   return dma_free_attrs(to_vmd_dev(dev), size, vaddr, addr, attrs);
> >  }
> >  
> >  static int vmd_mmap(struct device *dev, struct vm_area_struct *vma,
> > void *cpu_addr, dma_addr_t addr, size_t size,
> > unsigned long attrs)
> >  {
> > -   return vmd_dma_ops(dev)->mmap(to_vmd_dev(dev), vma, cpu_addr, addr,
> > - size, attrs);
> > +   return dma_mmap_attrs(to_vmd_dev(dev), vma, cpu_addr, addr, size,
> > +   attrs);
> >  }
> >  
> >  static int vmd_get_sgtable(struct device *dev, struct sg_table *sgt,
> >void *cpu_addr, dma_addr_t addr, size_t size,
> >unsigned long attrs)
> >  {
> > -   return vmd_dma_ops(dev)->get_sgtable(to_vmd_dev(dev), sgt, cpu_addr,
> > -addr, size, attrs);
> > +   return dma_get_sgtable_attrs(to_vmd_dev(dev), sgt, cpu_addr, addr, size,
> > +   attrs);
> >  }
> >  
> >  static dma_addr_t vmd_map_page(struct device *dev, struct page *page,
> > @@ -347,61 +340,60 @@ static dma_addr_t vmd_map_page(struct device *dev, 
> > struct page *page,
> >enum dma_data_direction dir,
> >unsigned long attrs)
> >  {
> > -   return vmd_dma_ops(dev)->map_page(to_vmd_dev(dev), page, offset, size,
> > - dir, attrs);
> > +   return dma_map_page_attrs(to_vmd_dev(dev), page, offset, size, dir,
> > +   attrs);
> >  }
> >  
> >  static void vmd_unmap_page(struct device *dev, dma_addr_t addr, size_t 
> > size,
> >enum dma_data_direction dir, unsigned long attrs)
> >  {
> > -   vmd_dma_ops(dev)->unmap_page(to_vmd_dev(dev), addr, size, dir, attrs);
> > +   dma_unmap_page_attrs(to_vmd_dev(dev), addr, size, dir, attrs);
> >  }
> >  
> >  static int vmd_map_sg(struct device *dev, struct scatterlist *sg, int 
> > nents,
> >   enum dma_data_direction dir, unsigned long attrs)
> >  {
> > -   return vmd_dma_ops(dev)->map_sg(to_vmd_dev(dev), sg, nents, dir, attrs);
> > +   return dma_map_sg_attrs(to_vmd_dev(dev), sg, nents, dir, attrs);
> >  }
> >  
> >  static void vmd_unmap_sg(struct device *dev, struct scatterlist *sg, int 
> > nents,
> >  enum dma_data_direction dir, unsigned long attrs)
> >  {
> > -   vmd_dma_ops(dev)->unmap_sg(to_vmd_dev(dev), sg, nents, dir, attrs);
> > +   dma_unmap_sg_attrs(to_vmd_dev(dev), sg, nents, dir, attrs);
> >  }
> >  
> >  static void vmd_sync_single_for_cpu(struct device *dev, dma_addr_t addr,
> > size_t size, enum dma_data_direction dir)
> >  {
> > -   vmd_dma_ops(dev)->sync_single_for_cpu(to_vmd_dev(dev), addr, size, dir);
> > +   dma_sync_single_for_cpu(to_vmd_dev(dev), addr, size, dir);
> >  }
> >  
> >  static void vmd_sync_single_for_device(struct device *dev, dma_addr_t addr,
> >size_t size, enum dma_data_direction dir)
> >  {
> > -

Re: [PATCH 14/15] vmd: use the proper dma_* APIs instead of direct methods calls

2018-12-14 Thread Bjorn Helgaas 
Conventional spelling in subject is

  PCI: vmd: Use dma_* APIs instead of direct method calls

On Fri, Dec 07, 2018 at 11:07:19AM -0800, Christoph Hellwig wrote:
> With the bypass support for the direct mapping we might not always have
> methods to call, so use the proper APIs instead.  The only downside is
> that we will create two dma-debug entries for each mapping if
> CONFIG_DMA_DEBUG is enabled.
> 
> Signed-off-by: Christoph Hellwig 

You cc'd the VMD maintainers already, and I have no objection to this
from a PCI core point of view, so:

Acked-by: Bjorn Helgaas 

> ---
>  drivers/pci/controller/vmd.c | 42 +++-
>  1 file changed, 17 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> index 98ce79eac128..3890812cdf87 100644
> --- a/drivers/pci/controller/vmd.c
> +++ b/drivers/pci/controller/vmd.c
> @@ -307,39 +307,32 @@ static struct device *to_vmd_dev(struct device *dev)
>   return >dev->dev;
>  }
>  
> -static const struct dma_map_ops *vmd_dma_ops(struct device *dev)
> -{
> - return get_dma_ops(to_vmd_dev(dev));
> -}
> -
>  static void *vmd_alloc(struct device *dev, size_t size, dma_addr_t *addr,
>  gfp_t flag, unsigned long attrs)
>  {
> - return vmd_dma_ops(dev)->alloc(to_vmd_dev(dev), size, addr, flag,
> -attrs);
> + return dma_alloc_attrs(to_vmd_dev(dev), size, addr, flag, attrs);
>  }
>  
>  static void vmd_free(struct device *dev, size_t size, void *vaddr,
>dma_addr_t addr, unsigned long attrs)
>  {
> - return vmd_dma_ops(dev)->free(to_vmd_dev(dev), size, vaddr, addr,
> -   attrs);
> + return dma_free_attrs(to_vmd_dev(dev), size, vaddr, addr, attrs);
>  }
>  
>  static int vmd_mmap(struct device *dev, struct vm_area_struct *vma,
>   void *cpu_addr, dma_addr_t addr, size_t size,
>   unsigned long attrs)
>  {
> - return vmd_dma_ops(dev)->mmap(to_vmd_dev(dev), vma, cpu_addr, addr,
> -   size, attrs);
> + return dma_mmap_attrs(to_vmd_dev(dev), vma, cpu_addr, addr, size,
> + attrs);
>  }
>  
>  static int vmd_get_sgtable(struct device *dev, struct sg_table *sgt,
>  void *cpu_addr, dma_addr_t addr, size_t size,
>  unsigned long attrs)
>  {
> - return vmd_dma_ops(dev)->get_sgtable(to_vmd_dev(dev), sgt, cpu_addr,
> -  addr, size, attrs);
> + return dma_get_sgtable_attrs(to_vmd_dev(dev), sgt, cpu_addr, addr, size,
> + attrs);
>  }
>  
>  static dma_addr_t vmd_map_page(struct device *dev, struct page *page,
> @@ -347,61 +340,60 @@ static dma_addr_t vmd_map_page(struct device *dev, 
> struct page *page,
>  enum dma_data_direction dir,
>  unsigned long attrs)
>  {
> - return vmd_dma_ops(dev)->map_page(to_vmd_dev(dev), page, offset, size,
> -   dir, attrs);
> + return dma_map_page_attrs(to_vmd_dev(dev), page, offset, size, dir,
> + attrs);
>  }
>  
>  static void vmd_unmap_page(struct device *dev, dma_addr_t addr, size_t size,
>  enum dma_data_direction dir, unsigned long attrs)
>  {
> - vmd_dma_ops(dev)->unmap_page(to_vmd_dev(dev), addr, size, dir, attrs);
> + dma_unmap_page_attrs(to_vmd_dev(dev), addr, size, dir, attrs);
>  }
>  
>  static int vmd_map_sg(struct device *dev, struct scatterlist *sg, int nents,
> enum dma_data_direction dir, unsigned long attrs)
>  {
> - return vmd_dma_ops(dev)->map_sg(to_vmd_dev(dev), sg, nents, dir, attrs);
> + return dma_map_sg_attrs(to_vmd_dev(dev), sg, nents, dir, attrs);
>  }
>  
>  static void vmd_unmap_sg(struct device *dev, struct scatterlist *sg, int 
> nents,
>enum dma_data_direction dir, unsigned long attrs)
>  {
> - vmd_dma_ops(dev)->unmap_sg(to_vmd_dev(dev), sg, nents, dir, attrs);
> + dma_unmap_sg_attrs(to_vmd_dev(dev), sg, nents, dir, attrs);
>  }
>  
>  static void vmd_sync_single_for_cpu(struct device *dev, dma_addr_t addr,
>   size_t size, enum dma_data_direction dir)
>  {
> - vmd_dma_ops(dev)->sync_single_for_cpu(to_vmd_dev(dev), addr, size, dir);
> + dma_sync_single_for_cpu(to_vmd_dev(dev), addr, size, dir);
>  }
>  
>  static void vmd_sync_single_for_device(struct device *dev, dma_addr_t addr,
>  size_t size, enum dma_data_direction dir)
>  {
> - vmd_dma_ops(dev)->sync_single_for_device(to_vmd_dev(dev), addr, size,
> -  dir);
> + dma_sync_single_for_device(to_vmd_dev(dev), addr, size, dir);
>  }
>  
>  static void vmd_sync_sg_for_cpu(struct device *dev, struct scatterlist *sg,
>

Re: [PATCH 13/15] ACPI / scan: Refactor _CCA enforcement

2018-12-14 Thread Bjorn Helgaas 
On Fri, Dec 07, 2018 at 11:07:18AM -0800, Christoph Hellwig wrote:
> From: Robin Murphy 
> 
> Rather than checking the DMA attribute at each callsite, just pass it
> through for acpi_dma_configure() to handle directly. That can then deal
> with the relatively exceptional DEV_DMA_NOT_SUPPORTED case by explicitly
> installing dummy DMA ops instead of just skipping setup entirely. This
> will then free up the dev->dma_ops == NULL case for some valuable
> fastpath optimisations.
> 
> Signed-off-by: Robin Murphy 
> Signed-off-by: Christoph Hellwig 
> ---
>  drivers/acpi/scan.c  | 5 +
>  drivers/base/platform.c  | 3 +--
>  drivers/pci/pci-driver.c | 3 +--

Acked-by: Bjorn Helgaas# drivers/pci part

>  3 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index bd1c59fb0e17..b75ae34ed188 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -1456,6 +1456,11 @@ int acpi_dma_configure(struct device *dev, enum 
> dev_dma_attr attr)
>   const struct iommu_ops *iommu;
>   u64 dma_addr = 0, size = 0;
>  
> + if (attr == DEV_DMA_NOT_SUPPORTED) {
> + set_dma_ops(dev, _dummy_ops);
> + return 0;
> + }
> +
>   iort_dma_setup(dev, _addr, );
>  
>   iommu = iort_iommu_configure(dev);
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index eae841935a45..c1ddf191711e 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -1138,8 +1138,7 @@ int platform_dma_configure(struct device *dev)
>   ret = of_dma_configure(dev, dev->of_node, true);
>   } else if (has_acpi_companion(dev)) {
>   attr = acpi_get_dma_attr(to_acpi_device_node(dev->fwnode));
> - if (attr != DEV_DMA_NOT_SUPPORTED)
> - ret = acpi_dma_configure(dev, attr);
> + ret = acpi_dma_configure(dev, attr);
>   }
>  
>   return ret;
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index bef17c3fca67..1b58e058b13f 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -1602,8 +1602,7 @@ static int pci_dma_configure(struct device *dev)
>   struct acpi_device *adev = to_acpi_device_node(bridge->fwnode);
>   enum dev_dma_attr attr = acpi_get_dma_attr(adev);
>  
> - if (attr != DEV_DMA_NOT_SUPPORTED)
> - ret = acpi_dma_configure(dev, attr);
> + ret = acpi_dma_configure(dev, acpi_get_dma_attr(adev));
>   }
>  
>   pci_put_host_bridge_device(bridge);
> -- 
> 2.19.1
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 1/2] dma-mapping: zero memory returned from dma_alloc_*

2018-12-14 Thread Christoph Hellwig 
On Fri, Dec 14, 2018 at 07:10:56PM +0100, Sam Ravnborg wrote:
> Hi Christoph,
> 
> I stumbled upon this one:
> 
> #define __get_dma_pages(gfp_mask, order) \
> __get_free_pages((gfp_mask) | GFP_DMA, (order))

This isn't directly related to the dma mapping, but another place
that hides GFP_DMA allocations.  So no need for the treatment,
but we really should kill this obsfucating wrapper..
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 1/2] dma-mapping: zero memory returned from dma_alloc_*

2018-12-14 Thread Sam Ravnborg 
Hi Christoph,

I stumbled upon this one:

#define __get_dma_pages(gfp_mask, order) \
__get_free_pages((gfp_mask) | GFP_DMA, (order))

(include/linux/gfp.h)
Should it also have the __GFP_ZERO treatment?
Or maybe this is already done in your tree..

As for the sparc bits:
Acked-by: Sam Ravnborg  [sparc]

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

Re: use generic DMA mapping code in powerpc V4

2018-12-14 Thread Christoph Hellwig 
On Fri, Dec 14, 2018 at 01:00:26PM +0100, Christian Zigotzky wrote:
> On 12 December 2018 at 3:15PM, Christoph Hellwig wrote:
> > Thanks for bisecting.  I've spent some time going over the conversion
> > but can't really pinpoint it.  I have three little patches that switch
> > parts of the code to the generic version.  This is on top of the
> > last good commmit (977706f9755d2d697aa6f45b4f9f0e07516efeda).
> >
> > Can you check with whіch one things stop working?
>
> Hello Christoph,
>
> Great news! All your patches work!

No so great because that means I still have no idea what broke..

> I tested all your patches (including the patch '0004-alloc-free.patch' 
> today) and the PASEMI onboard ethernet works and the P5020 board boots 
> without any problems. Thank you for your work!
> I have a few days off. That means, I will work less and only for the A-EON 
> first level Linux support. I can test again on Thursday next week.

Enjoy your days off!

I think I'll need to prepare something new that is better bisectable,
and use that opportunity to reason about the changes a little more.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 12/34] powerpc/cell: move dma direct window setup out of dma_configure

2018-12-14 Thread Christoph Hellwig 
On Sat, Dec 15, 2018 at 12:29:11AM +1100, Michael Ellerman wrote:
> I think the problem is that we don't want to set iommu_bypass_supported
> unless cell_iommu_fixed_mapping_init() succeeds.
> 
> Yep. This makes it work for me on cell on top of your v5.

Thanks, this looks good.  I've folded it with the slight change of moving
the iommu_bypass_supported setup into cell_iommu_fixed_mapping_init.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH] iommu/ipmmu-vmsa: Hook up r8a774c0 DT matching code

2018-12-14 Thread Simon Horman 
On Thu, Dec 13, 2018 at 08:22:44PM +, Fabrizio Castro wrote:
> Support RZ/G2E (a.k.a. R8A774C0) IPMMU.
> 
> Signed-off-by: Fabrizio Castro 

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

Re: [PATCH 1/2] dma-mapping: zero memory returned from dma_alloc_*

2018-12-14 Thread Eugeniy Paltsev 
Hi Christoph,

do you have any public git repository with all your dma changes?

I want to test them for ARC.

Thanks.

On Fri, 2018-12-14 at 09:25 +0100, Christoph Hellwig wrote:
> If we want to map memory from the DMA allocator to userspace it must be
> zeroed at allocation time to prevent stale data leaks.   We already do
> this on most common architectures, but some architectures don't do this
> yet, fix them up, either by passing GFP_ZERO when we use the normal page
> allocator or doing a manual memset otherwise.
> 
> Signed-off-by: Christoph Hellwig 
> ---
-- 
 Eugeniy Paltsev
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 1/2] dma-mapping: zero memory returned from dma_alloc_*

2018-12-14 Thread Greg Ungerer 



On 14/12/18 9:47 pm, Christoph Hellwig wrote:

On Fri, Dec 14, 2018 at 10:54:32AM +0100, Geert Uytterhoeven wrote:

-   page = alloc_pages(flag, order);
+   page = alloc_pages(flag | GFP_ZERO, order);
 if (!page)
 return NULL;


There's second implementation below, which calls __get_free_pages() and
does an explicit memset().  As __get_free_pages() calls alloc_pages(), perhaps
it makes sense to replace the memset() by GFP_ZERO, to increase consistency?


It would, but this patch really tries to be minimally invasive to just
provide the zeroing everywhere.  There is plenty of opportunity
to improve the m68k dma allocator if I can get enough reviewers/testers:

  - for one the coldfire/nommu case absolutely does not make sense to
me as there is not work done at all to make sure the memory is
mapped uncached despite the architecture implementing cache
flushing for the map interface.  So this whole implementation
looks broken to me and will need some major work (I had a previous
discussion with Greg on that which needs to be dug out)


Yep, that is right. Certainly the MMU case is broken. Some noMMU cases work
by virtue of the SoC only having an instruction cache (the older V2 cores).

The MMU case is fixable, but I think it will mean changing away from
the fall-back virtual:physical 1:1 mapping it uses for the kernel address
space. So not completely trivial. Either that or a dedicated area of RAM
for coherent allocations that we can mark as non-cachable via the really
course grained and limited ACR registers - not really very appealing.

The noMMU case in general is probably limited to something like that same
type of dedicated RAM/ACR register mechamism.

The most commonly used periperal with DMA is the FEC ethernet module,
and it has some "special" (used very loosely) cache flushing for
parts like the 532x family which probably makes it mostly work right.
There is a PCI bus on the 54xx family of parts, and I know general
ethernet cards on it (like e1000's) have problems I am sure are
related to the fact that coherent memory allocations aren't.

I do plan to have a look at this for the MMU case some time soon.

Regards
Greg





  - the "regular" implementation in this patch should probably be replaced
with the generic remapping helpers that have been added for the 4.21
merge window:


http://git.infradead.org/users/hch/dma-mapping.git/commitdiff/0c3b3171ceccb8830c2bb5adff1b4e9b204c1450

Compile tested only patch below:

--

From ade86dc75b9850daf9111ebf9ce15825a6144f2d Mon Sep 17 00:00:00 2001

From: Christoph Hellwig 
Date: Fri, 14 Dec 2018 12:41:45 +0100
Subject: m68k: use the generic dma coherent remap allocator

This switche to using common code for the DMA allocations, including
potential use of the CMA allocator if configure.  Also add a few
comments where the existing behavior seems to be lacking.

Signed-off-by: Christoph Hellwig 
---
  arch/m68k/Kconfig  |  2 ++
  arch/m68k/kernel/dma.c | 64 --
  2 files changed, 20 insertions(+), 46 deletions(-)

diff --git a/arch/m68k/Kconfig b/arch/m68k/Kconfig
index 8a5868e9a3a0..60788cf02fbc 100644
--- a/arch/m68k/Kconfig
+++ b/arch/m68k/Kconfig
@@ -2,10 +2,12 @@
  config M68K
bool
default y
+   select ARCH_HAS_DMA_MMAP_PGPROT if MMU && !COLDFIRE
select ARCH_HAS_SYNC_DMA_FOR_DEVICE if HAS_DMA
select ARCH_MIGHT_HAVE_PC_PARPORT if ISA
select ARCH_NO_COHERENT_DMA_MMAP if !MMU
select ARCH_NO_PREEMPT if !COLDFIRE
+   select DMA_DIRECT_REMAP if MMU && !COLDFIRE
select HAVE_IDE
select HAVE_AOUT if MMU
select HAVE_DEBUG_BUGVERBOSE
diff --git a/arch/m68k/kernel/dma.c b/arch/m68k/kernel/dma.c
index dafe99d08a6a..16da5d96e228 100644
--- a/arch/m68k/kernel/dma.c
+++ b/arch/m68k/kernel/dma.c
@@ -18,57 +18,29 @@
  #include 
  
  #if defined(CONFIG_MMU) && !defined(CONFIG_COLDFIRE)

-
-void *arch_dma_alloc(struct device *dev, size_t size, dma_addr_t *handle,
-   gfp_t flag, unsigned long attrs)
+void arch_dma_prep_coherent(struct page *page, size_t size)
  {
-   struct page *page, **map;
-   pgprot_t pgprot;
-   void *addr;
-   int i, order;
-
-   pr_debug("dma_alloc_coherent: %d,%x\n", size, flag);
-
-   size = PAGE_ALIGN(size);
-   order = get_order(size);
-
-   page = alloc_pages(flag | GFP_ZERO, order);
-   if (!page)
-   return NULL;
-
-   *handle = page_to_phys(page);
-   map = kmalloc(sizeof(struct page *) << order, flag & ~__GFP_DMA);
-   if (!map) {
-   __free_pages(page, order);
-   return NULL;
-   }
-   split_page(page, order);
-
-   order = 1 << order;
-   size >>= PAGE_SHIFT;
-   map[0] = page;
-   for (i = 1; i < size; i++)
-   map[i] = page + i;
-   for (; i < order; i++)
-   __free_page(page + i);
-   pgprot =

[PATCH dma-mapping tree] arm64: default to the direct mapping in get_arch_dma_ops

2018-12-14 Thread Christoph Hellwig 
Otherwise the direct mapping won't work at all given that a NULL
dev->dma_ops causes a fallback.  Note that we already explicitly set
dev->dma_ops to dma_dummy_ops for dma-incapable devices, so this
fallback should not be needed anyway.

Fixes: 356da6d0cd ("dma-mapping: bypass indirect calls for dma-direct")
Signed-off-by: Christoph Hellwig 
Reported-by: Marek Szyprowski 
Tested-by: Marek Szyprowski 
---
 arch/arm64/include/asm/dma-mapping.h | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/arch/arm64/include/asm/dma-mapping.h 
b/arch/arm64/include/asm/dma-mapping.h
index 273e778f7de2..95dbf3ef735a 100644
--- a/arch/arm64/include/asm/dma-mapping.h
+++ b/arch/arm64/include/asm/dma-mapping.h
@@ -26,11 +26,7 @@
 
 static inline const struct dma_map_ops *get_arch_dma_ops(struct bus_type *bus)
 {
-   /*
-* We expect no ISA devices, and all other DMA masters are expected to
-* have someone call arch_setup_dma_ops at device creation time.
-*/
-   return _dummy_ops;
+   return NULL;
 }
 
 void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
-- 
2.19.2

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

Re: [PATCH 15/15] dma-mapping: bypass indirect calls for dma-direct

2018-12-14 Thread Marek Szyprowski 
Hi Christoph,

On 2018-12-14 15:24, Christoph Hellwig wrote:
> On Fri, Dec 14, 2018 at 03:11:37PM +0100, Marek Szyprowski wrote:
>> Hi Christoph,
>>
>> On 2018-12-07 20:07, Christoph Hellwig wrote:
>>> Avoid expensive indirect calls in the fast path DMA mapping
>>> operations by directly calling the dma_direct_* ops if we are using
>>> the directly mapped DMA operations.
>>>
>>> Signed-off-by: Christoph Hellwig 
>> This breaks direct DMA on ARM64 (also todays linux-next). NULL
>> dev->dma_ops fallbacks to get_arch_dma_ops(), which in turn returns
>> non-functional _dummy_ops on ARM64...
> Yeah, fallback from direct (NULL) dev->dma_ops to something else won't
> work with NULL as the indicator.
>
> Fortunately we shouldn't even need that thanks to the patch from Robin
> that explicitly set the dummy ops where needed.
>
> Can you try the patch below?

Yes, it fixes the problem.

> diff --git a/arch/arm64/include/asm/dma-mapping.h 
> b/arch/arm64/include/asm/dma-mapping.h
> index 273e778f7de2..95dbf3ef735a 100644
> --- a/arch/arm64/include/asm/dma-mapping.h
> +++ b/arch/arm64/include/asm/dma-mapping.h
> @@ -26,11 +26,7 @@
>  
>  static inline const struct dma_map_ops *get_arch_dma_ops(struct bus_type 
> *bus)
>  {
> - /*
> -  * We expect no ISA devices, and all other DMA masters are expected to
> -  * have someone call arch_setup_dma_ops at device creation time.
> -  */
> - return _dummy_ops;
> + return NULL;
>  }
>  
>  void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 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: [PATCH 15/15] dma-mapping: bypass indirect calls for dma-direct

2018-12-14 Thread Christoph Hellwig 
On Fri, Dec 14, 2018 at 03:11:37PM +0100, Marek Szyprowski wrote:
> Hi Christoph,
> 
> On 2018-12-07 20:07, Christoph Hellwig wrote:
> > Avoid expensive indirect calls in the fast path DMA mapping
> > operations by directly calling the dma_direct_* ops if we are using
> > the directly mapped DMA operations.
> >
> > Signed-off-by: Christoph Hellwig 
> 
> This breaks direct DMA on ARM64 (also todays linux-next). NULL
> dev->dma_ops fallbacks to get_arch_dma_ops(), which in turn returns
> non-functional _dummy_ops on ARM64...

Yeah, fallback from direct (NULL) dev->dma_ops to something else won't
work with NULL as the indicator.

Fortunately we shouldn't even need that thanks to the patch from Robin
that explicitly set the dummy ops where needed.

Can you try the patch below?

diff --git a/arch/arm64/include/asm/dma-mapping.h 
b/arch/arm64/include/asm/dma-mapping.h
index 273e778f7de2..95dbf3ef735a 100644
--- a/arch/arm64/include/asm/dma-mapping.h
+++ b/arch/arm64/include/asm/dma-mapping.h
@@ -26,11 +26,7 @@
 
 static inline const struct dma_map_ops *get_arch_dma_ops(struct bus_type *bus)
 {
-   /*
-* We expect no ISA devices, and all other DMA masters are expected to
-* have someone call arch_setup_dma_ops at device creation time.
-*/
-   return _dummy_ops;
+   return NULL;
 }
 
 void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 15/15] dma-mapping: bypass indirect calls for dma-direct

2018-12-14 Thread Marek Szyprowski 
Hi Christoph,

On 2018-12-07 20:07, Christoph Hellwig wrote:
> Avoid expensive indirect calls in the fast path DMA mapping
> operations by directly calling the dma_direct_* ops if we are using
> the directly mapped DMA operations.
>
> Signed-off-by: Christoph Hellwig 

This breaks direct DMA on ARM64 (also todays linux-next). NULL
dev->dma_ops fallbacks to get_arch_dma_ops(), which in turn returns
non-functional _dummy_ops on ARM64...

> ---
>  arch/alpha/include/asm/dma-mapping.h |   2 +-
>  arch/arc/mm/cache.c  |   2 +-
>  arch/arm/include/asm/dma-mapping.h   |   2 +-
>  arch/arm/mm/dma-mapping-nommu.c  |  14 +---
>  arch/arm64/mm/dma-mapping.c  |   3 -
>  arch/ia64/hp/common/hwsw_iommu.c |   2 +-
>  arch/ia64/hp/common/sba_iommu.c  |   4 +-
>  arch/ia64/kernel/dma-mapping.c   |   1 -
>  arch/mips/include/asm/dma-mapping.h  |   2 +-
>  arch/parisc/kernel/setup.c   |   4 -
>  arch/sparc/include/asm/dma-mapping.h |   4 +-
>  arch/x86/kernel/pci-dma.c|   2 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_drv.c  |   2 +-
>  drivers/iommu/amd_iommu.c|  13 +---
>  include/asm-generic/dma-mapping.h|   2 +-
>  include/linux/dma-direct.h   |  17 
>  include/linux/dma-mapping.h  | 111 +++
>  include/linux/dma-noncoherent.h  |   5 +-
>  kernel/dma/direct.c  |  37 ++---
>  kernel/dma/mapping.c |  40 ++
>  20 files changed, 150 insertions(+), 119 deletions(-)
>
> diff --git a/arch/alpha/include/asm/dma-mapping.h 
> b/arch/alpha/include/asm/dma-mapping.h
> index 8beeafd4f68e..0ee6a5c99b16 100644
> --- a/arch/alpha/include/asm/dma-mapping.h
> +++ b/arch/alpha/include/asm/dma-mapping.h
> @@ -7,7 +7,7 @@ extern const struct dma_map_ops alpha_pci_ops;
>  static inline const struct dma_map_ops *get_arch_dma_ops(struct bus_type 
> *bus)
>  {
>  #ifdef CONFIG_ALPHA_JENSEN
> - return _direct_ops;
> + return NULL;
>  #else
>   return _pci_ops;
>  #endif
> diff --git a/arch/arc/mm/cache.c b/arch/arc/mm/cache.c
> index f2701c13a66b..e188bb3ede53 100644
> --- a/arch/arc/mm/cache.c
> +++ b/arch/arc/mm/cache.c
> @@ -1280,7 +1280,7 @@ void __init arc_cache_init_master(void)
>   /*
>* In case of IOC (say IOC+SLC case), pointers above could still be set
>* but end up not being relevant as the first function in chain is not
> -  * called at all for @dma_direct_ops
> +  * called at all for devices using coherent DMA.
>* arch_sync_dma_for_cpu() -> dma_cache_*() -> __dma_cache_*()
>*/
>  }
> diff --git a/arch/arm/include/asm/dma-mapping.h 
> b/arch/arm/include/asm/dma-mapping.h
> index 965b7c846ecb..31d3b96f0f4b 100644
> --- a/arch/arm/include/asm/dma-mapping.h
> +++ b/arch/arm/include/asm/dma-mapping.h
> @@ -18,7 +18,7 @@ extern const struct dma_map_ops arm_coherent_dma_ops;
>  
>  static inline const struct dma_map_ops *get_arch_dma_ops(struct bus_type 
> *bus)
>  {
> - return IS_ENABLED(CONFIG_MMU) ? _dma_ops : _direct_ops;
> + return IS_ENABLED(CONFIG_MMU) ? _dma_ops : NULL;
>  }
>  
>  #ifdef __arch_page_to_dma
> diff --git a/arch/arm/mm/dma-mapping-nommu.c b/arch/arm/mm/dma-mapping-nommu.c
> index 712416ecd8e6..f304b10e23a4 100644
> --- a/arch/arm/mm/dma-mapping-nommu.c
> +++ b/arch/arm/mm/dma-mapping-nommu.c
> @@ -22,7 +22,7 @@
>  #include "dma.h"
>  
>  /*
> - *  dma_direct_ops is used if
> + *  The generic direct mapping code is used if
>   *   - MMU/MPU is off
>   *   - cpu is v7m w/o cache support
>   *   - device is coherent
> @@ -209,16 +209,9 @@ const struct dma_map_ops arm_nommu_dma_ops = {
>  };
>  EXPORT_SYMBOL(arm_nommu_dma_ops);
>  
> -static const struct dma_map_ops *arm_nommu_get_dma_map_ops(bool coherent)
> -{
> - return coherent ? _direct_ops : _nommu_dma_ops;
> -}
> -
>  void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
>   const struct iommu_ops *iommu, bool coherent)
>  {
> - const struct dma_map_ops *dma_ops;
> -
>   if (IS_ENABLED(CONFIG_CPU_V7M)) {
>   /*
>* Cache support for v7m is optional, so can be treated as
> @@ -234,7 +227,6 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base, 
> u64 size,
>   dev->archdata.dma_coherent = (get_cr() & CR_M) ? coherent : 
> true;
>   }
>  
> - dma_ops = arm_nommu_get_dma_map_ops(dev->archdata.dma_coherent);
> -
> - set_dma_ops(dev, dma_ops);
> + if (!dev->archdata.dma_coherent)
> + set_dma_ops(dev, _nommu_dma_ops);
>  }
> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
> index ab1e417204d0..95eda81e3f2d 100644
> --- a/arch/arm64/mm/dma-mapping.c
> +++ b/arch/arm64/mm/dma-mapping.c
> @@ -462,9 +462,6 @@ static void __iommu_setup_dma_ops(struct device *dev, u64 
> dma_base, u64 size,
>  void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
>   const struct iommu_ops *iommu,

Re: ensure dma_alloc_coherent always returns zeroed memory

2018-12-14 Thread Christoph Hellwig 
And in various places this used GFP_ZERO instead of __GFP_ZERO,
so won't compile.

The fixed version is available here:

http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/dma-alloc-always-zero

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

Re: [PATCH 12/34] powerpc/cell: move dma direct window setup out of dma_configure

2018-12-14 Thread Michael Ellerman 
Christoph Hellwig  writes:
> On Sun, Dec 09, 2018 at 09:23:39PM +1100, Michael Ellerman wrote:
>> Christoph Hellwig  writes:
>> 
>> > Configure the dma settings at device setup time, and stop playing games
>> > with get_pci_dma_ops.  This prepares for using the common dma_configure
>> > code later on.
>> >
>> > Signed-off-by: Christoph Hellwig 
>> > ---
>> >  arch/powerpc/platforms/cell/iommu.c | 20 +++-
>> >  1 file changed, 11 insertions(+), 9 deletions(-)
>> 
>> This one's crashing, haven't dug into why yet:
>
> Can you provide a gdb assembly of the exact crash site?  This looks
> like for some odd reason the DT structures aren't fully setup by the
> time we are probing the device, which seems odd.

It's dev->of_node which is NULL.

Because we were passed a platform_device which doesn't have an of_node.

It's the cbe-mic device created in cell_publish_devices().

I can fix that by simply checking for a NULL node, then the system boots
but then I have no network devices, due to:

  tg3 :00:01.0: enabling device (0140 -> 0142)
  tg3 :00:01.0: DMA engine test failed, aborting
  tg3: probe of :00:01.0 failed with error -12
  tg3 :00:01.1: enabling device (0140 -> 0142)
  tg3 :00:01.1: DMA engine test failed, aborting
  tg3: probe of :00:01.1 failed with error -12


I think the problem is that we don't want to set iommu_bypass_supported
unless cell_iommu_fixed_mapping_init() succeeds.

Yep. This makes it work for me on cell on top of your v5.

cheers


diff --git a/arch/powerpc/platforms/cell/iommu.c 
b/arch/powerpc/platforms/cell/iommu.c
index 348a815779c1..8329fda17cc8 100644
--- a/arch/powerpc/platforms/cell/iommu.c
+++ b/arch/powerpc/platforms/cell/iommu.c
@@ -813,6 +813,10 @@ static u64 cell_iommu_get_fixed_address(struct device *dev)
int i, len, best, naddr, nsize, pna, range_size;
 
np = of_node_get(dev->of_node);
+   if (!np)
+   /* We can be called for platform devices that have no of_node */
+   goto out;
+
while (1) {
naddr = of_n_addr_cells(np);
nsize = of_n_size_cells(np);
@@ -1065,8 +1069,11 @@ static int __init cell_iommu_init(void)
/* Setup various callbacks */
cell_pci_controller_ops.dma_dev_setup = cell_pci_dma_dev_setup;
 
-   if (!iommu_fixed_disabled && cell_iommu_fixed_mapping_init() == 0)
+   if (!iommu_fixed_disabled && cell_iommu_fixed_mapping_init() == 0) {
+   cell_pci_controller_ops.iommu_bypass_supported =
+   cell_pci_iommu_bypass_supported;
goto done;
+   }
 
/* Create an iommu for each /axon node.  */
for_each_node_by_name(np, "axon") {
@@ -1085,10 +1092,6 @@ static int __init cell_iommu_init(void)
}
  done:
/* Setup default PCI iommu ops */
-   if (!iommu_fixed_disabled) {
-   cell_pci_controller_ops.iommu_bypass_supported =
-   cell_pci_iommu_bypass_supported;
-   }
set_pci_dma_ops(_iommu_ops);
cell_iommu_enabled = true;
  bail:
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 1/2] dma-mapping: zero memory returned from dma_alloc_*

2018-12-14 Thread Geert Uytterhoeven 
Hi Christoph,

On Fri, Dec 14, 2018 at 12:47 PM Christoph Hellwig  wrote:
>
> On Fri, Dec 14, 2018 at 10:54:32AM +0100, Geert Uytterhoeven wrote:
> > > -   page = alloc_pages(flag, order);
> > > +   page = alloc_pages(flag | GFP_ZERO, order);
> > > if (!page)
> > > return NULL;
> >
> > There's second implementation below, which calls __get_free_pages() and
> > does an explicit memset().  As __get_free_pages() calls alloc_pages(), 
> > perhaps
> > it makes sense to replace the memset() by GFP_ZERO, to increase consistency?
>
> It would, but this patch really tries to be minimally invasive to just
> provide the zeroing everywhere.

Fair enough.

> There is plenty of opportunity
> to improve the m68k dma allocator if I can get enough reviewers/testers:
>
>  - for one the coldfire/nommu case absolutely does not make sense to
>me as there is not work done at all to make sure the memory is
>mapped uncached despite the architecture implementing cache
>flushing for the map interface.  So this whole implementation
>looks broken to me and will need some major work (I had a previous
>discussion with Greg on that which needs to be dug out)
>  - the "regular" implementation in this patch should probably be replaced
>with the generic remapping helpers that have been added for the 4.21
>merge window:
>
> 
> http://git.infradead.org/users/hch/dma-mapping.git/commitdiff/0c3b3171ceccb8830c2bb5adff1b4e9b204c1450
>
> Compile tested only patch below:
>
> --
> From ade86dc75b9850daf9111ebf9ce15825a6144f2d Mon Sep 17 00:00:00 2001
> From: Christoph Hellwig 
> Date: Fri, 14 Dec 2018 12:41:45 +0100
> Subject: m68k: use the generic dma coherent remap allocator
>
> This switche to using common code for the DMA allocations, including
> potential use of the CMA allocator if configure.  Also add a few
> comments where the existing behavior seems to be lacking.
>
> Signed-off-by: Christoph Hellwig 

Thanks, looks OK to me.
M68k doesn't have many drivers using the DMA framework, as most of them
predated that framework.

> --- a/arch/m68k/kernel/dma.c
> +++ b/arch/m68k/kernel/dma.c

>
> -void arch_dma_free(struct device *dev, size_t size, void *addr,
> -   dma_addr_t handle, unsigned long attrs)
> +pgprot_t arch_dma_mmap_pgprot(struct device *dev, pgprot_t prot,
> +   unsigned long attrs)
>  {
> -   pr_debug("dma_free_coherent: %p, %x\n", addr, handle);
> -   vfree(addr);
> +   /*
> +* XXX: this doesn't seem to handle the sun3 MMU at all.

Sun-3 selects NO_DMA, and this file is compiled for the HAS_DMA case only.

> +*/
> +   if (CPU_IS_040_OR_060) {
> +   pgprot_val(prot) &= ~_PAGE_CACHE040;
> +   pgprot_val(prot) |= _PAGE_GLOBAL040 | _PAGE_NOCACHE_S;
> +   } else {
> +   pgprot_val(prot) |= _PAGE_NOCACHE030;
> +   }
> +   return prot;
>  }

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: use generic DMA mapping code in powerpc V4

2018-12-14 Thread Christian Zigotzky 

On 12 December 2018 at 3:15PM, Christoph Hellwig wrote:
> Thanks for bisecting.  I've spent some time going over the conversion
> but can't really pinpoint it.  I have three little patches that switch
> parts of the code to the generic version.  This is on top of the
> last good commmit (977706f9755d2d697aa6f45b4f9f0e07516efeda).
>
> Can you check with whіch one things stop working?

Hello Christoph,

Great news! All your patches work!

I tested all your patches (including the patch '0004-alloc-free.patch' 
today) and the PASEMI onboard ethernet works and the P5020 board boots 
without any problems. Thank you for your work!
I have a few days off. That means, I will work less and only for the 
A-EON first level Linux support. I can test again on Thursday next week.


Have a nice weekend!

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

Re: [PATCH 1/2] dma-mapping: zero memory returned from dma_alloc_*

2018-12-14 Thread Christoph Hellwig 
On Fri, Dec 14, 2018 at 10:54:32AM +0100, Geert Uytterhoeven wrote:
> > -   page = alloc_pages(flag, order);
> > +   page = alloc_pages(flag | GFP_ZERO, order);
> > if (!page)
> > return NULL;
> 
> There's second implementation below, which calls __get_free_pages() and
> does an explicit memset().  As __get_free_pages() calls alloc_pages(), perhaps
> it makes sense to replace the memset() by GFP_ZERO, to increase consistency?

It would, but this patch really tries to be minimally invasive to just
provide the zeroing everywhere.  There is plenty of opportunity
to improve the m68k dma allocator if I can get enough reviewers/testers:

 - for one the coldfire/nommu case absolutely does not make sense to
   me as there is not work done at all to make sure the memory is
   mapped uncached despite the architecture implementing cache
   flushing for the map interface.  So this whole implementation
   looks broken to me and will need some major work (I had a previous
   discussion with Greg on that which needs to be dug out)
 - the "regular" implementation in this patch should probably be replaced
   with the generic remapping helpers that have been added for the 4.21
   merge window:


http://git.infradead.org/users/hch/dma-mapping.git/commitdiff/0c3b3171ceccb8830c2bb5adff1b4e9b204c1450

Compile tested only patch below:

--
>From ade86dc75b9850daf9111ebf9ce15825a6144f2d Mon Sep 17 00:00:00 2001
From: Christoph Hellwig 
Date: Fri, 14 Dec 2018 12:41:45 +0100
Subject: m68k: use the generic dma coherent remap allocator

This switche to using common code for the DMA allocations, including
potential use of the CMA allocator if configure.  Also add a few
comments where the existing behavior seems to be lacking.

Signed-off-by: Christoph Hellwig 
---
 arch/m68k/Kconfig  |  2 ++
 arch/m68k/kernel/dma.c | 64 --
 2 files changed, 20 insertions(+), 46 deletions(-)

diff --git a/arch/m68k/Kconfig b/arch/m68k/Kconfig
index 8a5868e9a3a0..60788cf02fbc 100644
--- a/arch/m68k/Kconfig
+++ b/arch/m68k/Kconfig
@@ -2,10 +2,12 @@
 config M68K
bool
default y
+   select ARCH_HAS_DMA_MMAP_PGPROT if MMU && !COLDFIRE
select ARCH_HAS_SYNC_DMA_FOR_DEVICE if HAS_DMA
select ARCH_MIGHT_HAVE_PC_PARPORT if ISA
select ARCH_NO_COHERENT_DMA_MMAP if !MMU
select ARCH_NO_PREEMPT if !COLDFIRE
+   select DMA_DIRECT_REMAP if MMU && !COLDFIRE
select HAVE_IDE
select HAVE_AOUT if MMU
select HAVE_DEBUG_BUGVERBOSE
diff --git a/arch/m68k/kernel/dma.c b/arch/m68k/kernel/dma.c
index dafe99d08a6a..16da5d96e228 100644
--- a/arch/m68k/kernel/dma.c
+++ b/arch/m68k/kernel/dma.c
@@ -18,57 +18,29 @@
 #include 
 
 #if defined(CONFIG_MMU) && !defined(CONFIG_COLDFIRE)
-
-void *arch_dma_alloc(struct device *dev, size_t size, dma_addr_t *handle,
-   gfp_t flag, unsigned long attrs)
+void arch_dma_prep_coherent(struct page *page, size_t size)
 {
-   struct page *page, **map;
-   pgprot_t pgprot;
-   void *addr;
-   int i, order;
-
-   pr_debug("dma_alloc_coherent: %d,%x\n", size, flag);
-
-   size = PAGE_ALIGN(size);
-   order = get_order(size);
-
-   page = alloc_pages(flag | GFP_ZERO, order);
-   if (!page)
-   return NULL;
-
-   *handle = page_to_phys(page);
-   map = kmalloc(sizeof(struct page *) << order, flag & ~__GFP_DMA);
-   if (!map) {
-   __free_pages(page, order);
-   return NULL;
-   }
-   split_page(page, order);
-
-   order = 1 << order;
-   size >>= PAGE_SHIFT;
-   map[0] = page;
-   for (i = 1; i < size; i++)
-   map[i] = page + i;
-   for (; i < order; i++)
-   __free_page(page + i);
-   pgprot = __pgprot(_PAGE_PRESENT | _PAGE_ACCESSED | _PAGE_DIRTY);
-   if (CPU_IS_040_OR_060)
-   pgprot_val(pgprot) |= _PAGE_GLOBAL040 | _PAGE_NOCACHE_S;
-   else
-   pgprot_val(pgprot) |= _PAGE_NOCACHE030;
-   addr = vmap(map, size, VM_MAP, pgprot);
-   kfree(map);
-
-   return addr;
+   /*
+* XXX: don't we need to flush and invalidate the caches before
+* creating a coherent mapping?
+* coherent?
+*/
 }
 
-void arch_dma_free(struct device *dev, size_t size, void *addr,
-   dma_addr_t handle, unsigned long attrs)
+pgprot_t arch_dma_mmap_pgprot(struct device *dev, pgprot_t prot,
+   unsigned long attrs)
 {
-   pr_debug("dma_free_coherent: %p, %x\n", addr, handle);
-   vfree(addr);
+   /*
+* XXX: this doesn't seem to handle the sun3 MMU at all.
+*/
+   if (CPU_IS_040_OR_060) {
+   pgprot_val(prot) &= ~_PAGE_CACHE040;
+   pgprot_val(prot) |= _PAGE_GLOBAL040 | _PAGE_NOCACHE_S;
+   } else {
+   pgprot_val(prot) |= _PAGE_NOCACHE030;
+   }
+   return prot;
 }
-
 #else
 
 #include 
-- 
2.19.2

Re: [PATCH] iommu/ipmmu-vmsa: Hook up r8a774c0 DT matching code

2018-12-14 Thread Geert Uytterhoeven 
On Thu, Dec 13, 2018 at 9:22 PM Fabrizio Castro
 wrote:
> Support RZ/G2E (a.k.a. R8A774C0) IPMMU.
>
> Signed-off-by: Fabrizio Castro 

Reviewed-by: Geert Uytterhoeven 

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] dt-bindings: iommu: ipmmu-vmsa: Add r8a774c0 support

2018-12-14 Thread Geert Uytterhoeven 
On Thu, Dec 13, 2018 at 9:19 PM Fabrizio Castro
 wrote:
> Document RZ/G2E (R8A774C0) SoC bindings.
>
> Signed-off-by: Fabrizio Castro 

Reviewed-by: Geert Uytterhoeven 

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 1/2] dma-mapping: zero memory returned from dma_alloc_*

2018-12-14 Thread Geert Uytterhoeven 
On Fri, Dec 14, 2018 at 10:54 AM Geert Uytterhoeven
 wrote:
> On Fri, Dec 14, 2018 at 9:26 AM Christoph Hellwig  wrote:
> > If we want to map memory from the DMA allocator to userspace it must be
> > zeroed at allocation time to prevent stale data leaks.   We already do
> > this on most common architectures, but some architectures don't do this
> > yet, fix them up, either by passing GFP_ZERO when we use the normal page
> > allocator or doing a manual memset otherwise.
> >
> > Signed-off-by: Christoph Hellwig 
>
> Thanks for your patch!
>
> > --- a/arch/m68k/kernel/dma.c
> > +++ b/arch/m68k/kernel/dma.c
> > @@ -32,7 +32,7 @@ void *arch_dma_alloc(struct device *dev, size_t size, 
> > dma_addr_t *handle,
> > size = PAGE_ALIGN(size);
> > order = get_order(size);
> >
> > -   page = alloc_pages(flag, order);
> > +   page = alloc_pages(flag | GFP_ZERO, order);
> > if (!page)
> > return NULL;
>
> There's second implementation below, which calls __get_free_pages() and
> does an explicit memset().  As __get_free_pages() calls alloc_pages(), perhaps
> it makes sense to replace the memset() by GFP_ZERO, to increase consistency?

Regardless, for m68k:
Acked-by: Geert Uytterhoeven 

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

[PATCH 1/2] dma-mapping: zero memory returned from dma_alloc_*

2018-12-14 Thread Christoph Hellwig 
If we want to map memory from the DMA allocator to userspace it must be
zeroed at allocation time to prevent stale data leaks.   We already do
this on most common architectures, but some architectures don't do this
yet, fix them up, either by passing GFP_ZERO when we use the normal page
allocator or doing a manual memset otherwise.

Signed-off-by: Christoph Hellwig 
---
 arch/alpha/kernel/pci_iommu.c| 2 +-
 arch/arc/mm/dma.c| 2 +-
 arch/c6x/mm/dma-coherent.c   | 5 -
 arch/m68k/kernel/dma.c   | 2 +-
 arch/microblaze/mm/consistent.c  | 2 +-
 arch/openrisc/kernel/dma.c   | 2 +-
 arch/parisc/kernel/pci-dma.c | 4 ++--
 arch/s390/pci/pci_dma.c  | 2 +-
 arch/sparc/kernel/ioport.c   | 2 +-
 arch/sparc/mm/io-unit.c  | 2 +-
 arch/sparc/mm/iommu.c| 2 +-
 arch/xtensa/kernel/pci-dma.c | 2 +-
 drivers/misc/mic/host/mic_boot.c | 2 +-
 kernel/dma/virt.c| 2 +-
 14 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/arch/alpha/kernel/pci_iommu.c b/arch/alpha/kernel/pci_iommu.c
index e1716e0d92fd..28a025eda80d 100644
--- a/arch/alpha/kernel/pci_iommu.c
+++ b/arch/alpha/kernel/pci_iommu.c
@@ -443,7 +443,7 @@ static void *alpha_pci_alloc_coherent(struct device *dev, 
size_t size,
gfp &= ~GFP_DMA;
 
 try_again:
-   cpu_addr = (void *)__get_free_pages(gfp, order);
+   cpu_addr = (void *)__get_free_pages(gfp | GFP_ZERO, order);
if (! cpu_addr) {
printk(KERN_INFO "pci_alloc_consistent: "
   "get_free_pages failed from %pf\n",
diff --git a/arch/arc/mm/dma.c b/arch/arc/mm/dma.c
index db203ff69ccf..b0754581efc6 100644
--- a/arch/arc/mm/dma.c
+++ b/arch/arc/mm/dma.c
@@ -33,7 +33,7 @@ void *arch_dma_alloc(struct device *dev, size_t size, 
dma_addr_t *dma_handle,
 */
BUG_ON(gfp & __GFP_HIGHMEM);
 
-   page = alloc_pages(gfp, order);
+   page = alloc_pages(gfp | GFP_ZERO, order);
if (!page)
return NULL;
 
diff --git a/arch/c6x/mm/dma-coherent.c b/arch/c6x/mm/dma-coherent.c
index 01305c787201..75b79571732c 100644
--- a/arch/c6x/mm/dma-coherent.c
+++ b/arch/c6x/mm/dma-coherent.c
@@ -78,6 +78,7 @@ static void __free_dma_pages(u32 addr, int order)
 void *arch_dma_alloc(struct device *dev, size_t size, dma_addr_t *handle,
gfp_t gfp, unsigned long attrs)
 {
+   void *ret;
u32 paddr;
int order;
 
@@ -94,7 +95,9 @@ void *arch_dma_alloc(struct device *dev, size_t size, 
dma_addr_t *handle,
if (!paddr)
return NULL;
 
-   return phys_to_virt(paddr);
+   ret = phys_to_virt(paddr);
+   memset(ret, 0, 1 << order);
+   return ret;
 }
 
 /*
diff --git a/arch/m68k/kernel/dma.c b/arch/m68k/kernel/dma.c
index e3c57d6b..dafe99d08a6a 100644
--- a/arch/m68k/kernel/dma.c
+++ b/arch/m68k/kernel/dma.c
@@ -32,7 +32,7 @@ void *arch_dma_alloc(struct device *dev, size_t size, 
dma_addr_t *handle,
size = PAGE_ALIGN(size);
order = get_order(size);
 
-   page = alloc_pages(flag, order);
+   page = alloc_pages(flag | GFP_ZERO, order);
if (!page)
return NULL;
 
diff --git a/arch/microblaze/mm/consistent.c b/arch/microblaze/mm/consistent.c
index 45e0a1aa9357..79b9f4695a1b 100644
--- a/arch/microblaze/mm/consistent.c
+++ b/arch/microblaze/mm/consistent.c
@@ -81,7 +81,7 @@ void *arch_dma_alloc(struct device *dev, size_t size, 
dma_addr_t *dma_handle,
size = PAGE_ALIGN(size);
order = get_order(size);
 
-   vaddr = __get_free_pages(gfp, order);
+   vaddr = __get_free_pages(gfp | GFP_ZERO, order);
if (!vaddr)
return NULL;
 
diff --git a/arch/openrisc/kernel/dma.c b/arch/openrisc/kernel/dma.c
index 159336adfa2f..cdd03f63207c 100644
--- a/arch/openrisc/kernel/dma.c
+++ b/arch/openrisc/kernel/dma.c
@@ -89,7 +89,7 @@ arch_dma_alloc(struct device *dev, size_t size, dma_addr_t 
*dma_handle,
.mm = _mm
};
 
-   page = alloc_pages_exact(size, gfp);
+   page = alloc_pages_exact(size, gfp | GFP_ZERO);
if (!page)
return NULL;
 
diff --git a/arch/parisc/kernel/pci-dma.c b/arch/parisc/kernel/pci-dma.c
index 04c48f1ef3fb..7fa396714b5a 100644
--- a/arch/parisc/kernel/pci-dma.c
+++ b/arch/parisc/kernel/pci-dma.c
@@ -404,7 +404,7 @@ static void *pcxl_dma_alloc(struct device *dev, size_t size,
order = get_order(size);
size = 1 << (order + PAGE_SHIFT);
vaddr = pcxl_alloc_range(size);
-   paddr = __get_free_pages(flag, order);
+   paddr = __get_free_pages(flag | GFP_ZERO, order);
flush_kernel_dcache_range(paddr, size);
paddr = __pa(paddr);
map_uncached_pages(vaddr, size, paddr);
@@ -429,7 +429,7 @@ static void *pcx_dma_alloc(struct device *dev, size_t size,
if ((attrs & DMA_ATTR_NON_CONSISTENT) == 0)
return NULL;
 
-   addr = (void *)__get_free_pages(flag, get_order(size));
+

[PATCH 2/2] dma-mapping: deprecate dma_zalloc_coherent

2018-12-14 Thread Christoph Hellwig 
We now always return zeroed memory from dma_alloc_coherent.  Note that
simply passing GFP_ZERO to dma_alloc_coherent wasn't always doing the
right thing to start with given that various allocators are not backed
by the page allocator and thus would ignore GFP_ZERO.

Signed-off-by: Christoph Hellwig 
---
 Documentation/DMA-API.txt   | 9 -
 include/linux/dma-mapping.h | 7 ---
 2 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/Documentation/DMA-API.txt b/Documentation/DMA-API.txt
index 016eb6909b8a..e133ccd60228 100644
--- a/Documentation/DMA-API.txt
+++ b/Documentation/DMA-API.txt
@@ -58,15 +58,6 @@ specify the ``GFP_`` flags (see kmalloc()) for the 
allocation (the
 implementation may choose to ignore flags that affect the location of
 the returned memory, like GFP_DMA).
 
-::
-
-   void *
-   dma_zalloc_coherent(struct device *dev, size_t size,
-   dma_addr_t *dma_handle, gfp_t flag)
-
-Wraps dma_alloc_coherent() and also zeroes the returned memory if the
-allocation attempt succeeded.
-
 ::
 
void
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index f422aec0f53c..a52c6409bdc2 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -644,12 +644,13 @@ static inline unsigned long dma_max_pfn(struct device 
*dev)
 }
 #endif
 
+/*
+ * Please always use dma_alloc_coherent instead as it already zeroes the 
memory!
+ */
 static inline void *dma_zalloc_coherent(struct device *dev, size_t size,
dma_addr_t *dma_handle, gfp_t flag)
 {
-   void *ret = dma_alloc_coherent(dev, size, dma_handle,
-  flag | __GFP_ZERO);
-   return ret;
+   return dma_alloc_coherent(dev, size, dma_handle, flag);
 }
 
 static inline int dma_get_cache_alignment(void)
-- 
2.19.2

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

ensure dma_alloc_coherent always returns zeroed memory

2018-12-14 Thread Christoph Hellwig 
For security reasons we already returned zeroed memory from
dma_alloc_coherent on most common platforms, but some implementation
missed out.  Make sure we provide a consistent behavior.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu