Re: [PATCH v4 1/5] docs: IOMMU user API

2020-07-13 Thread Jacob Pan
Hi Alex,

On Mon, 13 Jul 2020 16:48:42 -0600
Alex Williamson  wrote:

> On Tue,  7 Jul 2020 16:43:45 -0700
> Jacob Pan  wrote:
> 
> > IOMMU UAPI is newly introduced to support communications between
> > guest virtual IOMMU and host IOMMU. There has been lots of
> > discussions on how it should work with VFIO UAPI and userspace in
> > general.
> > 
> > This document is indended to clarify the UAPI design and usage. The
> > mechenics of how future extensions should be achieved are also
> > covered  
> 
> mechanics
> 
will fix.

> > in this documentation.
> > 
> > Signed-off-by: Liu Yi L 
> > Signed-off-by: Jacob Pan 
> > ---
> >  Documentation/userspace-api/iommu.rst | 312
> > ++ 1 file changed, 312 insertions(+)
> >  create mode 100644 Documentation/userspace-api/iommu.rst
> > 
> > diff --git a/Documentation/userspace-api/iommu.rst
> > b/Documentation/userspace-api/iommu.rst new file mode 100644
> > index ..581b462c2cec
> > --- /dev/null
> > +++ b/Documentation/userspace-api/iommu.rst
> > @@ -0,0 +1,312 @@
> > +.. SPDX-License-Identifier: GPL-2.0
> > +.. iommu:
> > +
> > +=
> > +IOMMU Userspace API
> > +=
> > +
> > +IOMMU UAPI is used for virtualization cases where communications
> > are +needed between physical and virtual IOMMU drivers. For native
> > +usage, IOMMU is a system device which does not need to communicate
> > +with user space directly.
> > +
> > +The primary use cases are guest Shared Virtual Address (SVA) and
> > +guest IO virtual address (IOVA), wherein a virtual IOMMU (vIOMMU)
> > is +required to communicate with the physical IOMMU in the host.
> > +
> > +.. contents:: :local:
> > +
> > +Functionalities
> > +===
> > +Communications of user and kernel involve both directions. The
> > +supported user-kernel APIs are as follows:
> > +
> > +1. Alloc/Free PASID
> > +2. Bind/unbind guest PASID (e.g. Intel VT-d)
> > +3. Bind/unbind guest PASID table (e.g. ARM sMMU)
> > +4. Invalidate IOMMU caches
> > +5. Service page requests
> > +
> > +Requirements
> > +
> > +The IOMMU UAPIs are generic and extensible to meet the following
> > +requirements:
> > +
> > +1. Emulated and para-virtualised vIOMMUs
> > +2. Multiple vendors (Intel VT-d, ARM sMMU, etc.)
> > +3. Extensions to the UAPI shall not break existing user space
> > +
> > +Interfaces
> > +==
> > +Although the data structures defined in IOMMU UAPI are
> > self-contained, +there is no user API functions introduced.
> > Instead, IOMMU UAPI is +designed to work with existing user driver
> > frameworks such as VFIO. +
> > +Extension Rules & Precautions
> > +-
> > +When IOMMU UAPI gets extended, the data structures can *only* be
> > +modified in two ways:
> > +
> > +1. Adding new fields by re-purposing the padding[] field. No size
> > change. +2. Adding new union members at the end. May increase in
> > size. +
> > +No new fields can be added *after* the variable sized union in
> > that it +will break backward compatibility when offset moves. In
> > both cases, a +new flag must be accompanied with a new field such
> > that the IOMMU +driver can process the data based on the new flag.
> > Version field is +only reserved for the unlikely event of UAPI
> > upgrade at its entirety. +
> > +It's *always* the caller's responsibility to indicate the size of
> > the +structure passed by setting argsz appropriately.
> > +Though at the same time, argsz is user provided data which is not
> > +trusted. The argsz field allows the user to indicate how much data
> > +they're providing, it's still the kernel's responsibility to
> > validate +whether it's correct and sufficient for the requested
> > operation. +
> > +Compatibility Checking
> > +--
> > +When IOMMU UAPI extension results in size increase, user such as
> > VFIO +has to handle the following cases:
> > +
> > +1. User and kernel has exact size match
> > +2. An older user with older kernel header (smaller UAPI size)
> > running on a
> > +   newer kernel (larger UAPI size)
> > +3. A newer user with newer kernel header (larger UAPI size) running
> > +   on an older kernel.
> > +4. A malicious/misbehaving user pass illegal/invalid size but
> > within
> > +   range. The data may contain garbage.  
> 
> I'm still not sure where VFIO has responsibility in managing any of
> these cases.  I think we've determined that VFIO is just the wrapper
> and call-through mechanism, it's the UAPI core implementation and
> IOMMU drivers that are responsible for this.
> 
That is right, I shall rewrite the responsibility to be held by IOMMU
core.
"When IOMMU UAPI extension results in size increase, IOMMU UAPI core
shall handle the following cases:"

> > +
> > +Feature Checking
> > +
> > +While launching a guest with vIOMMU, it is important to ensure
> > that host +can support the UAPI data structures to be used for
> > 

Re: [Freedreno] [PATCH] iommu/arm-smmu: Add a init_context_bank implementation hook

2020-07-13 Thread Sai Prakash Ranjan

On 2020-07-14 00:43, Jordan Crouse wrote:

On Mon, Jul 13, 2020 at 08:03:32PM +0100, Will Deacon wrote:

On Mon, Jul 13, 2020 at 11:00:32AM -0600, Jordan Crouse wrote:
> On Mon, Jul 13, 2020 at 04:11:23PM +0100, Will Deacon wrote:
> > On Thu, Jun 11, 2020 at 04:36:56PM -0600, Jordan Crouse wrote:
> > > Add a new implementation hook to allow the implementation specific code
> > > to tweek the context bank configuration just before it gets written.
> > > The first user will be the Adreno GPU implementation to turn on
> > > SCTLR.HUPCF to ensure that a page fault doesn't terminating pending
> > > transactions. Doing so could hang the GPU if one of the terminated
> > > transactions is a CP read.
> > >
> > > This depends on the arm-smmu adreno SMMU implementation [1].
> > >
> > > [1] https://patchwork.kernel.org/patch/11600943/
> > >
> > > Signed-off-by: Jordan Crouse 
> > > ---
> > >
> > >  drivers/iommu/arm-smmu-qcom.c | 13 +
> > >  drivers/iommu/arm-smmu.c  | 28 +---
> > >  drivers/iommu/arm-smmu.h  | 11 +++
> > >  3 files changed, 37 insertions(+), 15 deletions(-)
> >
> > This looks straightforward enough, but I don't want to merge this without
> > a user and Sai's series has open questions afaict.
>
> Not sure what you mean by a user in this context?
> Are you referring to https://patchwork.kernel.org/patch/11628541/?

Right, this post was just a single patch in isolation, whereas it was
reposted over at:

https://lore.kernel.org/r/cdcc6a1c95a84e774790389dc8b3b7f490dc.1593344119.git.saiprakash.ran...@codeaurora.org

so I'll ignore this one. Sorry, I'm just really struggling to keep 
track
of what is targetting 5.9, and I don't have tonnes of time to sift 
through

the backlog of duplicate postings :(


Yeah, that is our fault. There are too many cooks in the kitchen.

We need to pick either system cache or split pagetable and serialize
the other on top of it to get the impl code going and then build from 
there.
This particular patch can happily hang out in the background until the 
rest is

resolved.



My bad, sorry. Let us get split pagetable support reviewed first, then I 
can post
system cache support on top of it. As jordan said, this patch can 
hibernate until

those get resolved.

Thanks,
Sai

--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member

of Code Aurora Forum, hosted by The Linux Foundation
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


RE: [PATCH v6 03/12] docs: x86: Add documentation for SVA (Shared Virtual Addressing)

2020-07-13 Thread Liu, Yi L
> From: Fenghua Yu 
> Sent: Tuesday, July 14, 2020 7:48 AM
> 
> From: Ashok Raj 
> 
> ENQCMD and Data Streaming Accelerator (DSA) and all of their associated 
> features
> are a complicated stack with lots of interconnected pieces.
> This documentation provides a big picture overview for all of the features.
> 
> Signed-off-by: Ashok Raj 
> Co-developed-by: Fenghua Yu 
> Signed-off-by: Fenghua Yu 
> Reviewed-by: Tony Luck 
> ---
> v3:
> - Replace deprecated intel_svm_bind_mm() by iommu_sva_bind_mm() (Baolu)
> - Fix a couple of typos (Baolu)
> 
> v2:
> - Fix the doc format and add the doc in toctree (Thomas)
> - Modify the doc for better description (Thomas, Tony, Dave)
> 
>  Documentation/x86/index.rst |   1 +
>  Documentation/x86/sva.rst   | 287 
>  2 files changed, 288 insertions(+)
>  create mode 100644 Documentation/x86/sva.rst
> 
> diff --git a/Documentation/x86/index.rst b/Documentation/x86/index.rst index
> 265d9e9a093b..e5d5ff096685 100644
> --- a/Documentation/x86/index.rst
> +++ b/Documentation/x86/index.rst
> @@ -30,3 +30,4 @@ x86-specific Documentation
> usb-legacy-support
> i386/index
> x86_64/index
> +   sva
> diff --git a/Documentation/x86/sva.rst b/Documentation/x86/sva.rst new file 
> mode
> 100644 index ..7242a84169ef
> --- /dev/null
> +++ b/Documentation/x86/sva.rst
> @@ -0,0 +1,287 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +===
> +Shared Virtual Addressing (SVA) with ENQCMD
> +===
> +
> +Background
> +==
> +
> +Shared Virtual Addressing (SVA) allows the processor and device to use
> +the same virtual addresses avoiding the need for software to translate
> +virtual addresses to physical addresses. SVA is what PCIe calls Shared
> +Virtual Memory (SVM)
> +
> +In addition to the convenience of using application virtual addresses
> +by the device, it also doesn't require pinning pages for DMA.
> +PCIe Address Translation Services (ATS) along with Page Request
> +Interface
> +(PRI) allow devices to function much the same way as the CPU handling
> +application page-faults. For more information please refer to PCIe
> +specification Chapter 10: ATS Specification.
> +

nit: may be helpful to mention Chapter 10 of PCIe spec since 4.0. before that, 
ATS has its
own specification.

> +Use of SVA requires IOMMU support in the platform. IOMMU also is
> +required to support PCIe features ATS and PRI. ATS allows devices to
> +cache translations for the virtual address. IOMMU driver uses the
> +mmu_notifier() support to keep the device tlb cache and the CPU cache
> +in sync. PRI allows the device to request paging the virtual address
> +before using if they are not paged in the CPU page tables.
> +
> +
> +Shared Hardware Workqueues
> +==
> +
> +Unlike Single Root I/O Virtualization (SRIOV), Scalable IOV (SIOV)
> +permits the use of Shared Work Queues (SWQ) by both applications and
> +Virtual Machines (VM's). This allows better hardware utilization vs.
> +hard partitioning resources that could result in under utilization. In
> +order to allow the hardware to distinguish the context for which work
> +is being executed in the hardware by SWQ interface, SIOV uses Process
> +Address Space ID (PASID), which is a 20bit number defined by the PCIe SIG.
> +
> +PASID value is encoded in all transactions from the device. This allows
> +the IOMMU to track I/O on a per-PASID granularity in addition to using
> +the PCIe Resource Identifier (RID) which is the Bus/Device/Function.
> +
> +
> +ENQCMD
> +==
> +
> +ENQCMD is a new instruction on Intel platforms that atomically submits
> +a work descriptor to a device. The descriptor includes the operation to
> +be performed, virtual addresses of all parameters, virtual address of a
> +completion record, and the PASID (process address space ID) of the current 
> process.
> +
> +ENQCMD works with non-posted semantics and carries a status back if the
> +command was accepted by hardware. This allows the submitter to know if
> +the submission needs to be retried or other device specific mechanisms
> +to implement fairness or ensure forward progress can be made.
> +
> +ENQCMD is the glue that ensures applications can directly submit
> +commands to the hardware and also permit hardware to be aware of
> +application context to perform I/O operations via use of PASID.
> +

maybe a reader will ask about ENQCMDs after reading ENQCMD/S spec. :-)

> +Process Address Space Tagging
> +=
> +
> +A new thread scoped MSR (IA32_PASID) provides the connection between
> +user processes and the rest of the hardware. When an application first
> +accesses an SVA capable device this MSR is initialized with a newly
> +allocated PASID. The driver for the device calls an IOMMU specific api
> +that sets up the routing for DMA and page-requests.
> +
> +For example, the Intel Data Streaming 

RE: [PATCH v6 01/12] iommu: Change type of pasid to u32

2020-07-13 Thread Liu, Yi L
> From: Fenghua Yu 
> Sent: Tuesday, July 14, 2020 7:48 AM
> 
> PASID is defined as a few different types in iommu including "int",
> "u32", and "unsigned int". To be consistent and to match with uapi
> definitions, define PASID and its variations (e.g. max PASID) as "u32".
> "u32" is also shorter and a little more explicit than "unsigned int".
> 
> No PASID type change in uapi although it defines PASID as __u64 in
> some places.

just out of curious, why not using ioasid_t? In Linux kernel, PASID is
managed by ioasid.

Regards,
Yi Liu

> Suggested-by: Thomas Gleixner 
> Signed-off-by: Fenghua Yu 
> Reviewed-by: Tony Luck 
> Reviewed-by: Lu Baolu 
> Acked-by: Felix Kuehling 
> ---
> v6:
> - Change return type to u32 for kfd_pasid_alloc() (Felix)
> 
> v5:
> - Reviewed by Lu Baolu
> 
> v4:
> - Change PASID type from "unsigned int" to "u32" (Christoph)
> 
> v2:
> - Create this new patch to define PASID as "unsigned int" consistently in
>   iommu (Thomas)
> 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h|  4 +--
>  .../drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c|  2 +-
>  .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c |  2 +-
>  .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c |  2 +-
>  .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c |  2 +-
>  .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.h |  2 +-
>  .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  |  4 +--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c   |  6 ++--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h   |  4 +--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c   |  2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c|  8 ++---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h|  8 ++---
>  .../gpu/drm/amd/amdkfd/cik_event_interrupt.c  |  2 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c   |  2 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_dbgmgr.h   |  2 +-
>  .../drm/amd/amdkfd/kfd_device_queue_manager.c |  7 ++---
>  drivers/gpu/drm/amd/amdkfd/kfd_events.c   |  8 ++---
>  drivers/gpu/drm/amd/amdkfd/kfd_events.h   |  4 +--
>  drivers/gpu/drm/amd/amdkfd/kfd_iommu.c|  6 ++--
>  drivers/gpu/drm/amd/amdkfd/kfd_pasid.c|  4 +--
>  drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 20 ++--
>  drivers/gpu/drm/amd/amdkfd/kfd_process.c  |  2 +-
>  .../gpu/drm/amd/include/kgd_kfd_interface.h   |  2 +-
>  drivers/iommu/amd/amd_iommu.h | 10 +++---
>  drivers/iommu/amd/iommu.c | 31 ++-
>  drivers/iommu/amd/iommu_v2.c  | 20 ++--
>  drivers/iommu/intel/dmar.c|  7 +++--
>  drivers/iommu/intel/intel-pasid.h | 24 +++---
>  drivers/iommu/intel/iommu.c   |  4 +--
>  drivers/iommu/intel/pasid.c   | 31 +--
>  drivers/iommu/intel/svm.c | 12 +++
>  drivers/iommu/iommu.c |  2 +-
>  drivers/misc/uacce/uacce.c|  2 +-
>  include/linux/amd-iommu.h |  8 ++---
>  include/linux/intel-iommu.h   | 12 +++
>  include/linux/intel-svm.h |  2 +-
>  include/linux/iommu.h | 10 +++---
>  include/linux/uacce.h |  2 +-
>  38 files changed, 141 insertions(+), 141 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> index ffe149aafc39..dfef5a7e0f5a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> @@ -207,11 +207,11 @@ uint8_t amdgpu_amdkfd_get_xgmi_hops_count(struct
> kgd_dev *dst, struct kgd_dev *s
>   })
> 
>  /* GPUVM API */
> -int amdgpu_amdkfd_gpuvm_create_process_vm(struct kgd_dev *kgd, unsigned int
> pasid,
> +int amdgpu_amdkfd_gpuvm_create_process_vm(struct kgd_dev *kgd, u32 pasid,
>   void **vm, void **process_info,
>   struct dma_fence **ef);
>  int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct kgd_dev *kgd,
> - struct file *filp, unsigned int pasid,
> + struct file *filp, u32 pasid,
>   void **vm, void **process_info,
>   struct dma_fence **ef);
>  void amdgpu_amdkfd_gpuvm_destroy_cb(struct amdgpu_device *adev,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c
> index bf927f432506..ee531c3988d1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c
> @@ -105,7 +105,7 @@ static void kgd_program_sh_mem_settings(struct kgd_dev
> *kgd, uint32_t vmid,
>   unlock_srbm(kgd);
>  }
> 
> -static int kgd_set_pasid_vmid_mapping(struct kgd_dev *kgd, unsigned int 
> pasid,
> +static int kgd_set_pasid_vmid_mapping(struct kgd_dev *kgd, u32 pasid,
> 

Re: [git pull] IOMMU Fixes for Linux v5.8-rc5

2020-07-13 Thread pr-tracker-bot
The pull request you sent on Mon, 13 Jul 2020 17:36:54 +0200:

> git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git 
> tags/iommu-fixes-v5.8-rc5

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/0dc589da873b58b70f4caf4b070fb0cf70fdd1dc

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[iommu:core 18/19] drivers/iommu/exynos-iommu.c:724:20: error: conflicting types for 'update_pte'

2020-07-13 Thread kernel test robot
Hi Robin,

First bad commit (maybe != root cause):

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git core
head:   97215a7df4351fdd9141418568be872fb1032d6e
commit: b4ceb4a5359ed1c9ba4a20acf3a70d4bbead3248 [18/19] iommu: Tidy up Kconfig 
for SoC IOMMUs
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
git checkout b4ceb4a5359ed1c9ba4a20acf3a70d4bbead3248
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross 
ARCH=xtensa 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot 

All errors (new ones prefixed by >>):

   In file included from include/linux/err.h:5,
from include/linux/clk.h:12,
from drivers/iommu/exynos-iommu.c:11:
   include/linux/scatterlist.h: In function 'sg_set_buf':
   arch/xtensa/include/asm/page.h:193:9: warning: comparison of unsigned 
expression >= 0 is always true [-Wtype-limits]
 193 |  ((pfn) >= ARCH_PFN_OFFSET && ((pfn) - ARCH_PFN_OFFSET) < max_mapnr)
 | ^~
   include/linux/compiler.h:78:42: note: in definition of macro 'unlikely'
  78 | # define unlikely(x) __builtin_expect(!!(x), 0)
 |  ^
   include/linux/scatterlist.h:143:2: note: in expansion of macro 'BUG_ON'
 143 |  BUG_ON(!virt_addr_valid(buf));
 |  ^~
   arch/xtensa/include/asm/page.h:201:32: note: in expansion of macro 
'pfn_valid'
 201 | #define virt_addr_valid(kaddr) pfn_valid(__pa(kaddr) >> PAGE_SHIFT)
 |^
   include/linux/scatterlist.h:143:10: note: in expansion of macro 
'virt_addr_valid'
 143 |  BUG_ON(!virt_addr_valid(buf));
 |  ^~~
   In file included from ./arch/xtensa/include/generated/asm/bug.h:1,
from include/linux/bug.h:5,
from include/linux/thread_info.h:12,
from arch/xtensa/include/asm/current.h:18,
from include/linux/mutex.h:14,
from include/linux/notifier.h:14,
from include/linux/clk.h:14,
from drivers/iommu/exynos-iommu.c:11:
   include/linux/dma-mapping.h: In function 'dma_map_resource':
   arch/xtensa/include/asm/page.h:193:9: warning: comparison of unsigned 
expression >= 0 is always true [-Wtype-limits]
 193 |  ((pfn) >= ARCH_PFN_OFFSET && ((pfn) - ARCH_PFN_OFFSET) < max_mapnr)
 | ^~
   include/asm-generic/bug.h:144:27: note: in definition of macro 'WARN_ON_ONCE'
 144 |  int __ret_warn_once = !!(condition);   \
 |   ^
   include/linux/dma-mapping.h:352:19: note: in expansion of macro 'pfn_valid'
 352 |  if (WARN_ON_ONCE(pfn_valid(PHYS_PFN(phys_addr
 |   ^
   drivers/iommu/exynos-iommu.c: At top level:
>> drivers/iommu/exynos-iommu.c:724:20: error: conflicting types for 
>> 'update_pte'
 724 | static inline void update_pte(sysmmu_pte_t *ent, sysmmu_pte_t val)
 |^~
   In file included from include/linux/pgtable.h:6,
from include/linux/mm.h:32,
from include/linux/scatterlist.h:8,
from include/linux/dma-mapping.h:11,
from drivers/iommu/exynos-iommu.c:12:
   arch/xtensa/include/asm/pgtable.h:306:20: note: previous definition of 
'update_pte' was here
 306 | static inline void update_pte(pte_t *ptep, pte_t pteval)
 |^~

vim +/update_pte +724 drivers/iommu/exynos-iommu.c

2a96536e77b43c KyongHo Cho  2012-05-12  723  
5e3435eb7e1d8c Marek Szyprowski 2016-02-18 @724  static inline void 
update_pte(sysmmu_pte_t *ent, sysmmu_pte_t val)
2a96536e77b43c KyongHo Cho  2012-05-12  725  {
5e3435eb7e1d8c Marek Szyprowski 2016-02-18  726 
dma_sync_single_for_cpu(dma_dev, virt_to_phys(ent), sizeof(*ent),
5e3435eb7e1d8c Marek Szyprowski 2016-02-18  727 
DMA_TO_DEVICE);
6ae5343c26f9cb Ben Dooks2016-06-08  728 *ent = cpu_to_le32(val);
5e3435eb7e1d8c Marek Szyprowski 2016-02-18  729 
dma_sync_single_for_device(dma_dev, virt_to_phys(ent), sizeof(*ent),
5e3435eb7e1d8c Marek Szyprowski 2016-02-18  730 
   DMA_TO_DEVICE);
2a96536e77b43c KyongHo Cho  2012-05-12  731  }
2a96536e77b43c KyongHo Cho  2012-05-12  732  

:: The code at line 724 was first introduced by commit
:: 5e3435eb7e1d8c9431254f5e0053ce1ad654a591 iommu/exynos: Remove 
ARM-specific cache flush interface

:: TO: Marek Szyprowski 
:: CC: Joerg Roedel 

---
0-DAY CI Kernel Test Service, 

Re: [PATCH v7 07/36] drm: exynos: use common helper for a scatterlist contiguity check

2020-07-13 Thread Inki Dae


20. 6. 19. 오후 7:36에 Marek Szyprowski 이(가) 쓴 글:
> Use common helper for checking the contiguity of the imported dma-buf.
> 
> Signed-off-by: Marek Szyprowski 

Acked-by : Inki Dae 

Thanks,
Inki Dae

> ---
>  drivers/gpu/drm/exynos/exynos_drm_gem.c | 23 +++
>  1 file changed, 3 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c 
> b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> index efa476858db5..1716a023bca0 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> @@ -431,27 +431,10 @@ exynos_drm_gem_prime_import_sg_table(struct drm_device 
> *dev,
>  {
>   struct exynos_drm_gem *exynos_gem;
>  
> - if (sgt->nents < 1)
> + /* check if the entries in the sg_table are contiguous */
> + if (drm_prime_get_contiguous_size(sgt) < attach->dmabuf->size) {
> + DRM_ERROR("buffer chunks must be mapped contiguously");
>   return ERR_PTR(-EINVAL);
> -
> - /*
> -  * Check if the provided buffer has been mapped as contiguous
> -  * into DMA address space.
> -  */
> - if (sgt->nents > 1) {
> - dma_addr_t next_addr = sg_dma_address(sgt->sgl);
> - struct scatterlist *s;
> - unsigned int i;
> -
> - for_each_sg(sgt->sgl, s, sgt->nents, i) {
> - if (!sg_dma_len(s))
> - break;
> - if (sg_dma_address(s) != next_addr) {
> - DRM_ERROR("buffer chunks must be mapped 
> contiguously");
> - return ERR_PTR(-EINVAL);
> - }
> - next_addr = sg_dma_address(s) + sg_dma_len(s);
> - }
>   }
>  
>   exynos_gem = exynos_drm_gem_init(dev, attach->dmabuf->size);
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v7 08/36] drm: exynos: fix common struct sg_table related issues

2020-07-13 Thread Inki Dae


20. 6. 19. 오후 7:36에 Marek Szyprowski 이(가) 쓴 글:
> The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function
> returns the number of the created entries in the DMA address space.
> However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and
> dma_unmap_sg must be called with the original number of the entries
> passed to the dma_map_sg().
> 
> struct sg_table is a common structure used for describing a non-contiguous
> memory buffer, used commonly in the DRM and graphics subsystems. It
> consists of a scatterlist with memory pages and DMA addresses (sgl entry),
> as well as the number of scatterlist entries: CPU pages (orig_nents entry)
> and DMA mapped pages (nents entry).
> 
> It turned out that it was a common mistake to misuse nents and orig_nents
> entries, calling DMA-mapping functions with a wrong number of entries or
> ignoring the number of mapped entries returned by the dma_map_sg()
> function.
> 
> To avoid such issues, lets use a common dma-mapping wrappers operating
> directly on the struct sg_table objects and use scatterlist page
> iterators where possible. This, almost always, hides references to the
> nents and orig_nents entries, making the code robust, easier to follow
> and copy/paste safe.
> 
> Signed-off-by: Marek Szyprowski 

Acked-by : Inki Dae 

Thanks,
Inki Dae

> ---
>  drivers/gpu/drm/exynos/exynos_drm_g2d.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c 
> b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> index fcee33a43aca..7014a8cd971a 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> @@ -395,8 +395,8 @@ static void g2d_userptr_put_dma_addr(struct g2d_data *g2d,
>   return;
>  
>  out:
> - dma_unmap_sg(to_dma_dev(g2d->drm_dev), g2d_userptr->sgt->sgl,
> - g2d_userptr->sgt->nents, DMA_BIDIRECTIONAL);
> + dma_unmap_sgtable(to_dma_dev(g2d->drm_dev), g2d_userptr->sgt,
> +   DMA_BIDIRECTIONAL, 0);
>  
>   pages = frame_vector_pages(g2d_userptr->vec);
>   if (!IS_ERR(pages)) {
> @@ -511,10 +511,10 @@ static dma_addr_t *g2d_userptr_get_dma_addr(struct 
> g2d_data *g2d,
>  
>   g2d_userptr->sgt = sgt;
>  
> - if (!dma_map_sg(to_dma_dev(g2d->drm_dev), sgt->sgl, sgt->nents,
> - DMA_BIDIRECTIONAL)) {
> + ret = dma_map_sgtable(to_dma_dev(g2d->drm_dev), sgt,
> +   DMA_BIDIRECTIONAL, 0);
> + if (ret) {
>   DRM_DEV_ERROR(g2d->dev, "failed to map sgt with dma region.\n");
> - ret = -ENOMEM;
>   goto err_sg_free_table;
>   }
>  
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 06/12] of/iommu: Make of_map_rid() PCI agnostic

2020-07-13 Thread Rob Herring
On Fri, 19 Jun 2020 09:20:07 +0100, Lorenzo Pieralisi wrote:
> There is nothing PCI specific (other than the RID - requester ID)
> in the of_map_rid() implementation, so the same function can be
> reused for input/output IDs mapping for other busses just as well.
> 
> Rename the RID instances/names to a generic "id" tag.
> 
> No functionality change intended.
> 
> Signed-off-by: Lorenzo Pieralisi 
> Cc: Rob Herring 
> Cc: Joerg Roedel 
> Cc: Robin Murphy 
> Cc: Marc Zyngier 
> ---
>  drivers/iommu/of_iommu.c |  4 ++--
>  drivers/of/base.c| 42 
>  drivers/of/irq.c |  2 +-
>  include/linux/of.h   |  4 ++--
>  4 files changed, 26 insertions(+), 26 deletions(-)
> 

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


[PATCH v6 09/12] x86/process: Clear PASID state for a newly forked/cloned thread

2020-07-13 Thread Fenghua Yu
The PASID state has to be cleared on forks, since the child has a
different address space. The PASID is also cleared for thread clone. While
it would be correct to inherit the PASID in this case, it is unknown
whether the new task will use ENQCMD. Giving it the PASID "just in case"
would have the downside of increased context switch overhead to setting
the PASID MSR.

Since #GP faults have to be handled on any threads that were created before
the PASID was assigned to the mm of the process, newly created threads
might as well be treated in a consistent way.

Suggested-by: Thomas Gleixner 
Signed-off-by: Fenghua Yu 
Reviewed-by: Tony Luck 
---
v2:
- Modify init_task_pasid().

 arch/x86/kernel/process.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index f362ce0d5ac0..1b1492e337a6 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -121,6 +121,21 @@ static int set_new_tls(struct task_struct *p, unsigned 
long tls)
return do_set_thread_area_64(p, ARCH_SET_FS, tls);
 }
 
+/* Initialize the PASID state for the forked/cloned thread. */
+static void init_task_pasid(struct task_struct *task)
+{
+   struct ia32_pasid_state *ppasid;
+
+   /*
+* Initialize the PASID state so that the PASID MSR will be
+* initialized to its initial state (0) by XRSTORS when the task is
+* scheduled for the first time.
+*/
+   ppasid = get_xsave_addr(>thread.fpu.state.xsave, XFEATURE_PASID);
+   if (ppasid)
+   ppasid->pasid = INIT_PASID;
+}
+
 int copy_thread_tls(unsigned long clone_flags, unsigned long sp,
unsigned long arg, struct task_struct *p, unsigned long tls)
 {
@@ -174,6 +189,9 @@ int copy_thread_tls(unsigned long clone_flags, unsigned 
long sp,
task_user_gs(p) = get_user_gs(current_pt_regs());
 #endif
 
+   if (static_cpu_has(X86_FEATURE_ENQCMD))
+   init_task_pasid(p);
+
/* Set a new TLS for the child thread? */
if (clone_flags & CLONE_SETTLS)
ret = set_new_tls(p, tls);
-- 
2.19.1

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


[PATCH v6 00/12] x86: tag application address space for devices

2020-07-13 Thread Fenghua Yu
Hi, Thomas, Joerg, and other maintainers,

This series only has one change in patch 1 on top of v5 (see change log).
Could you please consider to merge it upstream?

Thanks.

-Fenghua

=

Typical hardware devices require a driver stack to translate application
buffers to hardware addresses, and a kernel-user transition to notify the
hardware of new work. What if both the translation and transition overhead
could be eliminated? This is what Shared Virtual Address (SVA) and ENQCMD
enabled hardware like Data Streaming Accelerator (DSA) aims to achieve.
Applications map portals in their local-address-space and directly submit
work to them using a new instruction.

This series enables ENQCMD and associated management of the new MSR
(MSR_IA32_PASID). This new MSR allows an application address space to be
associated with what the PCIe spec calls a Process Address Space ID (PASID).
This PASID tag is carried along with all requests between applications and
devices and allows devices to interact with the process address space.

SVA and ENQCMD enabled device drivers need this series. The phase 2 DSA
patches with SVA and ENQCMD support was released on the top of this series:
https://lore.kernel.org/patchwork/cover/1244060/

This series only provides simple and basic support for ENQCMD and the MSR:
1. Clean up type definitions (patch 1-2). These patches can be in a
   separate series.
   - Define "pasid" as "u32" consistently
   - Define "flags" as "unsigned int"
2. Explain different various technical terms used in the series (patch 3).
3. Enumerate support for ENQCMD in the processor (patch 4).
4. Handle FPU PASID state and the MSR during context switch (patches 5-6).
5. Define "pasid" in mm_struct (patch 7).
5. Clear PASID state for new mm and forked and cloned thread (patch 8-9).
6. Allocate and free PASID for a process (patch 10).
7. Fix up the PASID MSR in #GP handler when one thread in a process
   executes ENQCMD for the first time (patches 11-12).

This patch series and the DSA phase 2 series are in
https://github.com/intel/idxd-driver/tree/idxd-stage2

References:
1. Detailed information on the ENQCMD/ENQCMDS instructions and the
IA32_PASID MSR can be found in Intel Architecture Instruction Set
Extensions and Future Features Programming Reference:
https://software.intel.com/sites/default/files/managed/c5/15/architecture-instruction-set-extensions-programming-reference.pdf

2. Detailed information on DSA can be found in DSA specification:
https://software.intel.com/en-us/download/intel-data-streaming-accelerator-preliminary-architecture-specification

Chang log:
v6:
- Change return type to u32 for kfd_pasid_alloc() in patch 1 (Felix)

v5:
- Mark ENQCMD disabled when configured out and use cpu_feature_enabled()
  to simplify the feature checking code in patch 10 and 12 (PeterZ and
  Dave Hansen)
- Add Reviewed-by: Lu Baolu to patch 1, 2, 10, and 12.

v4:
- Define PASID as "u32" instead of "unsigned int" in patch 1, 7, 10, 12.
  (Christoph)
- Drop v3 patch 2 which changes PASID type in ocxl because it's not related
  to x86 and was rejected by ocxl maintainer Frederic Barrat
- A split patch which changes PASID type to u32 in crypto/hisilicon/qm.c
  was released separately to linux-crypto mailing list because it's not
  related to x86 and is a standalone patch:

v3:
- Change names of bind_mm() and unbind_mm() to match to new APIs in
  patch 4 (Baolu)
- Change CONFIG_PCI_PASID to CONFIG_IOMMU_SUPPORT because non-PCI device
  can have PASID in ARM in patch 8 (Jean)
- Add a few sanity checks in __free_pasid() and alloc_pasid() in
  patch 11 (Baolu)
- Add patch 12 to define a new flag "has_valid_pasid" for a task and
  use the flag to identify if the task has a valid PASID MSR (PeterZ)
- Add fpu__pasid_write() to update the MSR in fixup() in patch 13
- Check if mm->pasid can be found in fixup() in patch 13

v2:
- Add patches 1-3 to define "pasid" and "flags" as "unsigned int"
  consistently (Thomas)
  (these 3 patches could be in a separate patch set)
- Add patch 8 to move "pasid" to generic mm_struct (Christoph).
  Jean-Philippe Brucker released a virtually same patch. Upstream only
  needs one of the two.
- Add patch 9 to initialize PASID in a new mm.
- Plus other changes described in each patch (Thomas)

Ashok Raj (1):
  docs: x86: Add documentation for SVA (Shared Virtual Addressing)

Fenghua Yu (9):
  iommu: Change type of pasid to u32
  iommu/vt-d: Change flags type to unsigned int in binding mm
  x86/cpufeatures: Enumerate ENQCMD and ENQCMDS instructions
  x86/msr-index: Define IA32_PASID MSR
  mm: Define pasid in mm
  fork: Clear PASID for new mm
  x86/process: Clear PASID state for a newly forked/cloned thread
  x86/mmu: Allocate/free PASID
  x86/traps: Fix up invalid PASID

Peter Zijlstra (1):
  sched: Define and initialize a flag to identify valid PASID in the
task

Yu-cheng Yu (1):
  x86/fpu/xstate: Add supervisor PASID state for ENQCMD feature

 Documentation/x86/index.rst   |   1 

[PATCH v6 03/12] docs: x86: Add documentation for SVA (Shared Virtual Addressing)

2020-07-13 Thread Fenghua Yu
From: Ashok Raj 

ENQCMD and Data Streaming Accelerator (DSA) and all of their associated
features are a complicated stack with lots of interconnected pieces.
This documentation provides a big picture overview for all of the
features.

Signed-off-by: Ashok Raj 
Co-developed-by: Fenghua Yu 
Signed-off-by: Fenghua Yu 
Reviewed-by: Tony Luck 
---
v3:
- Replace deprecated intel_svm_bind_mm() by iommu_sva_bind_mm() (Baolu)
- Fix a couple of typos (Baolu)

v2:
- Fix the doc format and add the doc in toctree (Thomas)
- Modify the doc for better description (Thomas, Tony, Dave)

 Documentation/x86/index.rst |   1 +
 Documentation/x86/sva.rst   | 287 
 2 files changed, 288 insertions(+)
 create mode 100644 Documentation/x86/sva.rst

diff --git a/Documentation/x86/index.rst b/Documentation/x86/index.rst
index 265d9e9a093b..e5d5ff096685 100644
--- a/Documentation/x86/index.rst
+++ b/Documentation/x86/index.rst
@@ -30,3 +30,4 @@ x86-specific Documentation
usb-legacy-support
i386/index
x86_64/index
+   sva
diff --git a/Documentation/x86/sva.rst b/Documentation/x86/sva.rst
new file mode 100644
index ..7242a84169ef
--- /dev/null
+++ b/Documentation/x86/sva.rst
@@ -0,0 +1,287 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+===
+Shared Virtual Addressing (SVA) with ENQCMD
+===
+
+Background
+==
+
+Shared Virtual Addressing (SVA) allows the processor and device to use the
+same virtual addresses avoiding the need for software to translate virtual
+addresses to physical addresses. SVA is what PCIe calls Shared Virtual
+Memory (SVM)
+
+In addition to the convenience of using application virtual addresses
+by the device, it also doesn't require pinning pages for DMA.
+PCIe Address Translation Services (ATS) along with Page Request Interface
+(PRI) allow devices to function much the same way as the CPU handling
+application page-faults. For more information please refer to PCIe
+specification Chapter 10: ATS Specification.
+
+Use of SVA requires IOMMU support in the platform. IOMMU also is required
+to support PCIe features ATS and PRI. ATS allows devices to cache
+translations for the virtual address. IOMMU driver uses the mmu_notifier()
+support to keep the device tlb cache and the CPU cache in sync. PRI allows
+the device to request paging the virtual address before using if they are
+not paged in the CPU page tables.
+
+
+Shared Hardware Workqueues
+==
+
+Unlike Single Root I/O Virtualization (SRIOV), Scalable IOV (SIOV) permits
+the use of Shared Work Queues (SWQ) by both applications and Virtual
+Machines (VM's). This allows better hardware utilization vs. hard
+partitioning resources that could result in under utilization. In order to
+allow the hardware to distinguish the context for which work is being
+executed in the hardware by SWQ interface, SIOV uses Process Address Space
+ID (PASID), which is a 20bit number defined by the PCIe SIG.
+
+PASID value is encoded in all transactions from the device. This allows the
+IOMMU to track I/O on a per-PASID granularity in addition to using the PCIe
+Resource Identifier (RID) which is the Bus/Device/Function.
+
+
+ENQCMD
+==
+
+ENQCMD is a new instruction on Intel platforms that atomically submits a
+work descriptor to a device. The descriptor includes the operation to be
+performed, virtual addresses of all parameters, virtual address of a completion
+record, and the PASID (process address space ID) of the current process.
+
+ENQCMD works with non-posted semantics and carries a status back if the
+command was accepted by hardware. This allows the submitter to know if the
+submission needs to be retried or other device specific mechanisms to
+implement fairness or ensure forward progress can be made.
+
+ENQCMD is the glue that ensures applications can directly submit commands
+to the hardware and also permit hardware to be aware of application context
+to perform I/O operations via use of PASID.
+
+Process Address Space Tagging
+=
+
+A new thread scoped MSR (IA32_PASID) provides the connection between
+user processes and the rest of the hardware. When an application first
+accesses an SVA capable device this MSR is initialized with a newly
+allocated PASID. The driver for the device calls an IOMMU specific api
+that sets up the routing for DMA and page-requests.
+
+For example, the Intel Data Streaming Accelerator (DSA) uses
+iommu_sva_bind_device(), which will do the following.
+
+- Allocate the PASID, and program the process page-table (cr3) in the PASID
+  context entries.
+- Register for mmu_notifier() to track any page-table invalidations to keep
+  the device tlb in sync. For example, when a page-table entry is invalidated,
+  IOMMU propagates the invalidation to device tlb. This will force any
+  future access by the device to this virtual address to participate in
+  ATS. 

[PATCH v6 07/12] mm: Define pasid in mm

2020-07-13 Thread Fenghua Yu
PASID is shared by all threads in a process. So the logical place to keep
track of it is in the "mm". Both ARM and X86 need to use the PASID in the
"mm".

Suggested-by: Christoph Hellwig 
Signed-off-by: Fenghua Yu 
Reviewed-by: Tony Luck 
---
v4:
- Change PASID type to u32 (Christoph)

v3:
- Change CONFIG_PCI_PASID to CONFIG_IOMMU_SUPPORT because non-PCI device
  can have PASID in ARM (Jean)

v2:
- This new patch moves "pasid" from x86 specific mm_context_t to generic
  struct mm_struct per Christopher's comment: 
https://lore.kernel.org/linux-iommu/20200414170252.714402-1-jean-phili...@linaro.org/T/#mb57110ffe1aaa24750eeea4f93b611f0d1913911
- Jean-Philippe Brucker released a virtually same patch. I still put this
  patch in the series for better review. The upstream kernel only needs one
  of the two patches eventually.
https://lore.kernel.org/linux-iommu/20200519175502.2504091-2-jean-phili...@linaro.org/
- Change CONFIG_IOASID to CONFIG_PCI_PASID (Ashok)

 include/linux/mm_types.h | 4 
 1 file changed, 4 insertions(+)

diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 64ede5f150dc..d61285cfe027 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -538,6 +538,10 @@ struct mm_struct {
atomic_long_t hugetlb_usage;
 #endif
struct work_struct async_put_work;
+
+#ifdef CONFIG_IOMMU_SUPPORT
+   u32 pasid;
+#endif
} __randomize_layout;
 
/*
-- 
2.19.1

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


[PATCH v6 10/12] x86/mmu: Allocate/free PASID

2020-07-13 Thread Fenghua Yu
A PASID is allocated for an "mm" the first time any thread attaches
to an SVM capable device. Later device attachments (whether to the same
device or another SVM device) will re-use the same PASID.

The PASID is freed when the process exits (so no need to keep
reference counts on how many SVM devices are sharing the PASID).

Currently the ENQCMD feature cannot be used if CONFIG_INTEL_IOMMU_SVM
is not set. Add X86_FEATURE_ENQCMD to the disabled features mask as
appropriate and use cpu_feature_enabled() to check the feature.

Signed-off-by: Fenghua Yu 
Reviewed-by: Tony Luck 
Reviewed-by: Lu Baolu 
---
v5:
- Mark ENQCMD disabled when configured out and remove redundant
  CONFIG_INTEL_IOMMU_SVM check which is included in cpu_feature_enabled()
  in fixup_pasid_exception() (PeterZ and Dave Hansen)
- Reviewed by Lu Baolu

v4:
- Change PASID type to u32 (Christoph)

v3:
- Add sanity checks in alloc_pasid() and _free_pasid() (Baolu)
- Add a comment that the private PASID feature will be removed completely
  from IOMMU and don't track private PASID in mm (Thomas)

v2:
- Define a helper free_bind() to simplify error exit code in bind_mm()
  (Thomas)
- Fix a ret error code in bind_mm() (Thomas)
- Change pasid's type from "int" to "unsigned int" to have consistent
  pasid type in iommu (Thomas)
- Simplify alloc_pasid() a bit.

 arch/x86/include/asm/disabled-features.h |   9 +-
 arch/x86/include/asm/iommu.h |   2 +
 arch/x86/include/asm/mmu_context.h   |  11 ++
 drivers/iommu/intel/svm.c| 128 ---
 4 files changed, 137 insertions(+), 13 deletions(-)

diff --git a/arch/x86/include/asm/disabled-features.h 
b/arch/x86/include/asm/disabled-features.h
index 4ea8584682f9..588d83e9da49 100644
--- a/arch/x86/include/asm/disabled-features.h
+++ b/arch/x86/include/asm/disabled-features.h
@@ -56,6 +56,12 @@
 # define DISABLE_PTI   (1 << (X86_FEATURE_PTI & 31))
 #endif
 
+#ifdef CONFIG_INTEL_IOMMU_SVM
+# define DISABLE_ENQCMD0
+#else
+# define DISABLE_ENQCMD (1 << (X86_FEATURE_ENQCMD & 31))
+#endif
+
 /*
  * Make sure to add features to the correct mask
  */
@@ -75,7 +81,8 @@
 #define DISABLED_MASK130
 #define DISABLED_MASK140
 #define DISABLED_MASK150
-#define DISABLED_MASK16
(DISABLE_PKU|DISABLE_OSPKE|DISABLE_LA57|DISABLE_UMIP)
+#define DISABLED_MASK16
(DISABLE_PKU|DISABLE_OSPKE|DISABLE_LA57|DISABLE_UMIP| \
+DISABLE_ENQCMD)
 #define DISABLED_MASK170
 #define DISABLED_MASK180
 #define DISABLED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 19)
diff --git a/arch/x86/include/asm/iommu.h b/arch/x86/include/asm/iommu.h
index bf1ed2ddc74b..ed41259fe7ac 100644
--- a/arch/x86/include/asm/iommu.h
+++ b/arch/x86/include/asm/iommu.h
@@ -26,4 +26,6 @@ arch_rmrr_sanity_check(struct acpi_dmar_reserved_memory *rmrr)
return -EINVAL;
 }
 
+void __free_pasid(struct mm_struct *mm);
+
 #endif /* _ASM_X86_IOMMU_H */
diff --git a/arch/x86/include/asm/mmu_context.h 
b/arch/x86/include/asm/mmu_context.h
index 47562147e70b..e1e7f1df6829 100644
--- a/arch/x86/include/asm/mmu_context.h
+++ b/arch/x86/include/asm/mmu_context.h
@@ -13,6 +13,7 @@
 #include 
 #include 
 #include 
+#include 
 
 extern atomic64_t last_mm_ctx_id;
 
@@ -117,9 +118,19 @@ static inline int init_new_context(struct task_struct *tsk,
init_new_context_ldt(mm);
return 0;
 }
+
+static inline void free_pasid(struct mm_struct *mm)
+{
+   if (!cpu_feature_enabled(X86_FEATURE_ENQCMD))
+   return;
+
+   __free_pasid(mm);
+}
+
 static inline void destroy_context(struct mm_struct *mm)
 {
destroy_context_ldt(mm);
+   free_pasid(mm);
 }
 
 extern void switch_mm(struct mm_struct *prev, struct mm_struct *next,
diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
index 8a0cf2f0dd54..4c70b037 100644
--- a/drivers/iommu/intel/svm.c
+++ b/drivers/iommu/intel/svm.c
@@ -425,6 +425,69 @@ int intel_svm_unbind_gpasid(struct device *dev, u32 pasid)
return ret;
 }
 
+static void free_bind(struct intel_svm *svm, struct intel_svm_dev *sdev,
+ bool new_pasid)
+{
+   if (new_pasid)
+   ioasid_free(svm->pasid);
+   kfree(svm);
+   kfree(sdev);
+}
+
+/*
+ * If this mm already has a PASID, use it. Otherwise allocate a new one.
+ * Let the caller know if a new PASID is allocated via 'new_pasid'.
+ */
+static int alloc_pasid(struct intel_svm *svm, struct mm_struct *mm,
+  u32 pasid_max, bool *new_pasid,
+  unsigned int flags)
+{
+   u32 pasid;
+
+   *new_pasid = false;
+
+   /*
+* Reuse the PASID if the mm already has a PASID and not a private
+* PASID is requested.
+*/
+   if (mm && mm->pasid && !(flags & SVM_FLAG_PRIVATE_PASID)) {
+   void *p;
+
+   /*
+* Since the mm has a PASID already, the PASID should be
+* bound 

[PATCH v6 06/12] x86/msr-index: Define IA32_PASID MSR

2020-07-13 Thread Fenghua Yu
The IA32_PASID MSR (0xd93) contains the Process Address Space Identifier
(PASID), a 20-bit value. Bit 31 must be set to indicate the value
programmed in the MSR is valid. Hardware uses PASID to identify process
address space and direct responses to the right address space.

Signed-off-by: Fenghua Yu 
Reviewed-by: Tony Luck 
---
v2:
- Change "identify process" to "identify process address space" in the
  commit message (Thomas)

 arch/x86/include/asm/msr-index.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index e8370e64a155..e5f699ff1dd6 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -237,6 +237,9 @@
 #define MSR_IA32_LASTINTFROMIP 0x01dd
 #define MSR_IA32_LASTINTTOIP   0x01de
 
+#define MSR_IA32_PASID 0x0d93
+#define MSR_IA32_PASID_VALID   BIT_ULL(31)
+
 /* DEBUGCTLMSR bits (others vary by model): */
 #define DEBUGCTLMSR_LBR(1UL <<  0) /* last branch 
recording */
 #define DEBUGCTLMSR_BTF_SHIFT  1
-- 
2.19.1

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


[PATCH v6 11/12] sched: Define and initialize a flag to identify valid PASID in the task

2020-07-13 Thread Fenghua Yu
From: Peter Zijlstra 

The flag is defined for the task to identify if the task has a valid
PASID. Its initial value is 0 when the task is forked/cloned. It will
be used shortly.

Signed-off-by: Peter Zijlstra 
Co-developed-by: Fenghua Yu 
Signed-off-by: Fenghua Yu 
---
v2:
- Add this patch to define the flag to identify valid PASID MSR (PeterZ)

 include/linux/sched.h | 3 +++
 kernel/fork.c | 4 
 2 files changed, 7 insertions(+)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 692e327d7455..042d6f5cde6a 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -800,6 +800,9 @@ struct task_struct {
/* Stalled due to lack of memory */
unsignedin_memstall:1;
 #endif
+#ifdef CONFIG_IOMMU_SUPPORT
+   unsignedhas_valid_pasid:1;
+#endif
 
unsigned long   atomic_flags; /* Flags requiring atomic 
access. */
 
diff --git a/kernel/fork.c b/kernel/fork.c
index 43b5f112604d..0a962bebdf88 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -955,6 +955,10 @@ static struct task_struct *dup_task_struct(struct 
task_struct *orig, int node)
tsk->use_memdelay = 0;
 #endif
 
+#ifdef CONFIG_IOMMU_SUPPORT
+   tsk->has_valid_pasid = 0;
+#endif
+
 #ifdef CONFIG_MEMCG
tsk->active_memcg = NULL;
 #endif
-- 
2.19.1

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


[PATCH v6 12/12] x86/traps: Fix up invalid PASID

2020-07-13 Thread Fenghua Yu
A #GP fault is generated when ENQCMD instruction is executed without
a valid PASID value programmed in the current thread's PASID MSR. The
#GP fault handler will initialize the MSR if a PASID has been allocated
for this process.

Decoding the user instruction is ugly and sets a bad architecture
precedent. It may not function if the faulting instruction is modified
after #GP.

Thomas suggested to provide a reason for the #GP caused by executing ENQCMD
without a valid PASID value programmed. #GP error codes are 16 bits and all
16 bits are taken. Refer to SDM Vol 3, Chapter 16.13 for details. The other
choice was to reflect the error code in an MSR. ENQCMD can also cause #GP
when loading from the source operand, so its not fully comprehending all
the reasons. Rather than special case the ENQCMD, in future Intel may
choose a different fault mechanism for such cases if recovery is needed on
#GP.

The following heuristic is used to avoid decoding the user instructions
to determine the precise reason for the #GP fault:
1) If the mm for the process has not been allocated a PASID, this #GP
   cannot be fixed.
2) If the PASID MSR is already initialized, then the #GP was for some
   other reason
3) Try initializing the PASID MSR and returning. If the #GP was from
   an ENQCMD this will fix it. If not, the #GP fault will be repeated
   and will hit case "2".

Suggested-by: Thomas Gleixner 
Signed-off-by: Fenghua Yu 
Reviewed-by: Tony Luck 
Reviewed-by: Lu Baolu 
---
v5:
- Use cpu_feature_enabled() and remove redundant CONFIG_INTEL_IOMMU_SVM
  check which is included in cpu_feature_enabled() in
  fixup_pasid_exception() (PeterZ and Dave Hansen)
- Reviewed by Lu Baolu

v4:
- Change PASID type to u32 (Christoph)

v3:
- Check and set current->has_valid_pasid in fixup() (PeterZ)
- Add fpu__pasid_write() to update the MSR (PeterZ)
- Add ioasid_find() sanity check in fixup()

v2:
- Update the first paragraph of the commit message (Thomas)
- Add reasons why don't decode the user instruction and don't use
  #GP error code (Thomas)
- Change get_task_mm() to current->mm (Thomas)
- Add comments on why IRQ is disabled during PASID fixup (Thomas)
- Add comment in fixup() that the function is called when #GP is from
  user (so mm is not NULL) (Dave Hansen)

 arch/x86/include/asm/iommu.h |  1 +
 arch/x86/kernel/traps.c  | 12 ++
 drivers/iommu/intel/svm.c| 78 
 3 files changed, 91 insertions(+)

diff --git a/arch/x86/include/asm/iommu.h b/arch/x86/include/asm/iommu.h
index ed41259fe7ac..e9365a5d6f7d 100644
--- a/arch/x86/include/asm/iommu.h
+++ b/arch/x86/include/asm/iommu.h
@@ -27,5 +27,6 @@ arch_rmrr_sanity_check(struct acpi_dmar_reserved_memory *rmrr)
 }
 
 void __free_pasid(struct mm_struct *mm);
+bool __fixup_pasid_exception(void);
 
 #endif /* _ASM_X86_IOMMU_H */
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index f58679e487f6..fe0f7d00523b 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -59,6 +59,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #ifdef CONFIG_X86_64
 #include 
@@ -518,6 +519,14 @@ static enum kernel_gp_hint get_kernel_gp_address(struct 
pt_regs *regs,
return GP_CANONICAL;
 }
 
+static bool fixup_pasid_exception(void)
+{
+   if (!cpu_feature_enabled(X86_FEATURE_ENQCMD))
+   return false;
+
+   return __fixup_pasid_exception();
+}
+
 #define GPFSTR "general protection fault"
 
 DEFINE_IDTENTRY_ERRORCODE(exc_general_protection)
@@ -530,6 +539,9 @@ DEFINE_IDTENTRY_ERRORCODE(exc_general_protection)
 
cond_local_irq_enable(regs);
 
+   if (user_mode(regs) && fixup_pasid_exception())
+   goto exit;
+
if (static_cpu_has(X86_FEATURE_UMIP)) {
if (user_mode(regs) && fixup_umip_exception(regs))
goto exit;
diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
index 4c70b037..4a84c82a4f8c 100644
--- a/drivers/iommu/intel/svm.c
+++ b/drivers/iommu/intel/svm.c
@@ -1105,3 +1105,81 @@ void __free_pasid(struct mm_struct *mm)
 */
ioasid_free(pasid);
 }
+
+/*
+ * Write the current task's PASID MSR/state. This is called only when PASID
+ * is enabled.
+ */
+static void fpu__pasid_write(u32 pasid)
+{
+   u64 msr_val = pasid | MSR_IA32_PASID_VALID;
+
+   fpregs_lock();
+
+   /*
+* If the MSR is active and owned by the current task's FPU, it can
+* be directly written.
+*
+* Otherwise, write the fpstate.
+*/
+   if (!test_thread_flag(TIF_NEED_FPU_LOAD)) {
+   wrmsrl(MSR_IA32_PASID, msr_val);
+   } else {
+   struct ia32_pasid_state *ppasid_state;
+
+   ppasid_state = get_xsave_addr(>thread.fpu.state.xsave,
+ XFEATURE_PASID);
+   /*
+* ppasid_state shouldn't be NULL because XFEATURE_PASID
+* is enabled.
+*/
+  

[PATCH v6 02/12] iommu/vt-d: Change flags type to unsigned int in binding mm

2020-07-13 Thread Fenghua Yu
"flags" passed to intel_svm_bind_mm() is a bit mask and should be
defined as "unsigned int" instead of "int".

Change its type to "unsigned int".

Suggested-by: Thomas Gleixner 
Signed-off-by: Fenghua Yu 
Reviewed-by: Tony Luck 
Reviewed-by: Lu Baolu 
---
v5:
- Reviewed by Lu Baolu

v2:
- Add this new patch per Thomas' comment.

 drivers/iommu/intel/svm.c   | 7 ---
 include/linux/intel-iommu.h | 2 +-
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
index 778089d198eb..8a0cf2f0dd54 100644
--- a/drivers/iommu/intel/svm.c
+++ b/drivers/iommu/intel/svm.c
@@ -427,7 +427,8 @@ int intel_svm_unbind_gpasid(struct device *dev, u32 pasid)
 
 /* Caller must hold pasid_mutex, mm reference */
 static int
-intel_svm_bind_mm(struct device *dev, int flags, struct svm_dev_ops *ops,
+intel_svm_bind_mm(struct device *dev, unsigned int flags,
+ struct svm_dev_ops *ops,
  struct mm_struct *mm, struct intel_svm_dev **sd)
 {
struct intel_iommu *iommu = intel_svm_device_to_iommu(dev);
@@ -954,7 +955,7 @@ intel_svm_bind(struct device *dev, struct mm_struct *mm, 
void *drvdata)
 {
struct iommu_sva *sva = ERR_PTR(-EINVAL);
struct intel_svm_dev *sdev = NULL;
-   int flags = 0;
+   unsigned int flags = 0;
int ret;
 
/*
@@ -963,7 +964,7 @@ intel_svm_bind(struct device *dev, struct mm_struct *mm, 
void *drvdata)
 * and intel_svm etc.
 */
if (drvdata)
-   flags = *(int *)drvdata;
+   flags = *(unsigned int *)drvdata;
mutex_lock(_mutex);
ret = intel_svm_bind_mm(dev, flags, NULL, mm, );
if (ret)
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index 643951e28dd4..d129baf7e0b8 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -760,7 +760,7 @@ struct intel_svm {
struct mm_struct *mm;
 
struct intel_iommu *iommu;
-   int flags;
+   unsigned int flags;
u32 pasid;
int gpasid; /* In case that guest PASID is different from host PASID */
struct list_head devs;
-- 
2.19.1

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


[PATCH v6 08/12] fork: Clear PASID for new mm

2020-07-13 Thread Fenghua Yu
When a new mm is created, its PASID should be cleared, i.e. the PASID is
initialized to its init state 0 on both ARM and X86.

Signed-off-by: Fenghua Yu 
Reviewed-by: Tony Luck 
---
v2:
- Add this patch to initialize PASID value for a new mm.

 include/linux/mm_types.h | 2 ++
 kernel/fork.c| 8 
 2 files changed, 10 insertions(+)

diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index d61285cfe027..d60d2ec10881 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -22,6 +22,8 @@
 #endif
 #define AT_VECTOR_SIZE (2*(AT_VECTOR_SIZE_ARCH + AT_VECTOR_SIZE_BASE + 1))
 
+/* Initial PASID value is 0. */
+#define INIT_PASID 0
 
 struct address_space;
 struct mem_cgroup;
diff --git a/kernel/fork.c b/kernel/fork.c
index 142b23645d82..43b5f112604d 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1007,6 +1007,13 @@ static void mm_init_owner(struct mm_struct *mm, struct 
task_struct *p)
 #endif
 }
 
+static void mm_init_pasid(struct mm_struct *mm)
+{
+#ifdef CONFIG_IOMMU_SUPPORT
+   mm->pasid = INIT_PASID;
+#endif
+}
+
 static void mm_init_uprobes_state(struct mm_struct *mm)
 {
 #ifdef CONFIG_UPROBES
@@ -1035,6 +1042,7 @@ static struct mm_struct *mm_init(struct mm_struct *mm, 
struct task_struct *p,
mm_init_cpumask(mm);
mm_init_aio(mm);
mm_init_owner(mm, p);
+   mm_init_pasid(mm);
RCU_INIT_POINTER(mm->exe_file, NULL);
mmu_notifier_subscriptions_init(mm);
init_tlb_flush_pending(mm);
-- 
2.19.1

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


[PATCH v6 01/12] iommu: Change type of pasid to u32

2020-07-13 Thread Fenghua Yu
PASID is defined as a few different types in iommu including "int",
"u32", and "unsigned int". To be consistent and to match with uapi
definitions, define PASID and its variations (e.g. max PASID) as "u32".
"u32" is also shorter and a little more explicit than "unsigned int".

No PASID type change in uapi although it defines PASID as __u64 in
some places.

Suggested-by: Thomas Gleixner 
Signed-off-by: Fenghua Yu 
Reviewed-by: Tony Luck 
Reviewed-by: Lu Baolu 
Acked-by: Felix Kuehling 
---
v6:
- Change return type to u32 for kfd_pasid_alloc() (Felix)

v5:
- Reviewed by Lu Baolu

v4:
- Change PASID type from "unsigned int" to "u32" (Christoph)

v2:
- Create this new patch to define PASID as "unsigned int" consistently in
  iommu (Thomas)

 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h|  4 +--
 .../drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c|  2 +-
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c |  2 +-
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c |  2 +-
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c |  2 +-
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.h |  2 +-
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  |  4 +--
 drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c   |  6 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h   |  4 +--
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c   |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c|  8 ++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h|  8 ++---
 .../gpu/drm/amd/amdkfd/cik_event_interrupt.c  |  2 +-
 drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c   |  2 +-
 drivers/gpu/drm/amd/amdkfd/kfd_dbgmgr.h   |  2 +-
 .../drm/amd/amdkfd/kfd_device_queue_manager.c |  7 ++---
 drivers/gpu/drm/amd/amdkfd/kfd_events.c   |  8 ++---
 drivers/gpu/drm/amd/amdkfd/kfd_events.h   |  4 +--
 drivers/gpu/drm/amd/amdkfd/kfd_iommu.c|  6 ++--
 drivers/gpu/drm/amd/amdkfd/kfd_pasid.c|  4 +--
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 20 ++--
 drivers/gpu/drm/amd/amdkfd/kfd_process.c  |  2 +-
 .../gpu/drm/amd/include/kgd_kfd_interface.h   |  2 +-
 drivers/iommu/amd/amd_iommu.h | 10 +++---
 drivers/iommu/amd/iommu.c | 31 ++-
 drivers/iommu/amd/iommu_v2.c  | 20 ++--
 drivers/iommu/intel/dmar.c|  7 +++--
 drivers/iommu/intel/intel-pasid.h | 24 +++---
 drivers/iommu/intel/iommu.c   |  4 +--
 drivers/iommu/intel/pasid.c   | 31 +--
 drivers/iommu/intel/svm.c | 12 +++
 drivers/iommu/iommu.c |  2 +-
 drivers/misc/uacce/uacce.c|  2 +-
 include/linux/amd-iommu.h |  8 ++---
 include/linux/intel-iommu.h   | 12 +++
 include/linux/intel-svm.h |  2 +-
 include/linux/iommu.h | 10 +++---
 include/linux/uacce.h |  2 +-
 38 files changed, 141 insertions(+), 141 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
index ffe149aafc39..dfef5a7e0f5a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
@@ -207,11 +207,11 @@ uint8_t amdgpu_amdkfd_get_xgmi_hops_count(struct kgd_dev 
*dst, struct kgd_dev *s
})
 
 /* GPUVM API */
-int amdgpu_amdkfd_gpuvm_create_process_vm(struct kgd_dev *kgd, unsigned int 
pasid,
+int amdgpu_amdkfd_gpuvm_create_process_vm(struct kgd_dev *kgd, u32 pasid,
void **vm, void **process_info,
struct dma_fence **ef);
 int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct kgd_dev *kgd,
-   struct file *filp, unsigned int pasid,
+   struct file *filp, u32 pasid,
void **vm, void **process_info,
struct dma_fence **ef);
 void amdgpu_amdkfd_gpuvm_destroy_cb(struct amdgpu_device *adev,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c
index bf927f432506..ee531c3988d1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c
@@ -105,7 +105,7 @@ static void kgd_program_sh_mem_settings(struct kgd_dev 
*kgd, uint32_t vmid,
unlock_srbm(kgd);
 }
 
-static int kgd_set_pasid_vmid_mapping(struct kgd_dev *kgd, unsigned int pasid,
+static int kgd_set_pasid_vmid_mapping(struct kgd_dev *kgd, u32 pasid,
unsigned int vmid)
 {
struct amdgpu_device *adev = get_amdgpu_device(kgd);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c
index 744366c7ee85..4d41317b9292 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c
+++ 

[PATCH v6 05/12] x86/fpu/xstate: Add supervisor PASID state for ENQCMD feature

2020-07-13 Thread Fenghua Yu
From: Yu-cheng Yu 

ENQCMD instruction reads PASID from IA32_PASID MSR. The MSR is stored
in the task's supervisor FPU PASID state and is context switched by
XSAVES/XRSTORS.

Signed-off-by: Yu-cheng Yu 
Co-developed-by: Fenghua Yu 
Signed-off-by: Fenghua Yu 
Reviewed-by: Tony Luck 
---
v2:
- Modify the commit message (Thomas)

 arch/x86/include/asm/fpu/types.h  | 10 ++
 arch/x86/include/asm/fpu/xstate.h |  2 +-
 arch/x86/kernel/fpu/xstate.c  |  4 
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/fpu/types.h b/arch/x86/include/asm/fpu/types.h
index f098f6cab94b..00f8efd4c07d 100644
--- a/arch/x86/include/asm/fpu/types.h
+++ b/arch/x86/include/asm/fpu/types.h
@@ -114,6 +114,7 @@ enum xfeature {
XFEATURE_Hi16_ZMM,
XFEATURE_PT_UNIMPLEMENTED_SO_FAR,
XFEATURE_PKRU,
+   XFEATURE_PASID,
 
XFEATURE_MAX,
 };
@@ -128,6 +129,7 @@ enum xfeature {
 #define XFEATURE_MASK_Hi16_ZMM (1 << XFEATURE_Hi16_ZMM)
 #define XFEATURE_MASK_PT   (1 << XFEATURE_PT_UNIMPLEMENTED_SO_FAR)
 #define XFEATURE_MASK_PKRU (1 << XFEATURE_PKRU)
+#define XFEATURE_MASK_PASID(1 << XFEATURE_PASID)
 
 #define XFEATURE_MASK_FPSSE(XFEATURE_MASK_FP | XFEATURE_MASK_SSE)
 #define XFEATURE_MASK_AVX512   (XFEATURE_MASK_OPMASK \
@@ -229,6 +231,14 @@ struct pkru_state {
u32 pad;
 } __packed;
 
+/*
+ * State component 10 is supervisor state used for context-switching the
+ * PASID state.
+ */
+struct ia32_pasid_state {
+   u64 pasid;
+} __packed;
+
 struct xstate_header {
u64 xfeatures;
u64 xcomp_bv;
diff --git a/arch/x86/include/asm/fpu/xstate.h 
b/arch/x86/include/asm/fpu/xstate.h
index 422d8369012a..ab9833c57aaa 100644
--- a/arch/x86/include/asm/fpu/xstate.h
+++ b/arch/x86/include/asm/fpu/xstate.h
@@ -33,7 +33,7 @@
  XFEATURE_MASK_BNDCSR)
 
 /* All currently supported supervisor features */
-#define XFEATURE_MASK_SUPERVISOR_SUPPORTED (0)
+#define XFEATURE_MASK_SUPERVISOR_SUPPORTED (XFEATURE_MASK_PASID)
 
 /*
  * Unsupported supervisor features. When a supervisor feature in this mask is
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index bda2e5eaca0e..31629e43383c 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -37,6 +37,7 @@ static const char *xfeature_names[] =
"AVX-512 ZMM_Hi256" ,
"Processor Trace (unused)"  ,
"Protection Keys User registers",
+   "PASID state",
"unknown xstate feature",
 };
 
@@ -51,6 +52,7 @@ static short xsave_cpuid_features[] __initdata = {
X86_FEATURE_AVX512F,
X86_FEATURE_INTEL_PT,
X86_FEATURE_PKU,
+   X86_FEATURE_ENQCMD,
 };
 
 /*
@@ -316,6 +318,7 @@ static void __init print_xstate_features(void)
print_xstate_feature(XFEATURE_MASK_ZMM_Hi256);
print_xstate_feature(XFEATURE_MASK_Hi16_ZMM);
print_xstate_feature(XFEATURE_MASK_PKRU);
+   print_xstate_feature(XFEATURE_MASK_PASID);
 }
 
 /*
@@ -590,6 +593,7 @@ static void check_xstate_against_struct(int nr)
XCHECK_SZ(sz, nr, XFEATURE_ZMM_Hi256, struct avx_512_zmm_uppers_state);
XCHECK_SZ(sz, nr, XFEATURE_Hi16_ZMM,  struct avx_512_hi16_state);
XCHECK_SZ(sz, nr, XFEATURE_PKRU,  struct pkru_state);
+   XCHECK_SZ(sz, nr, XFEATURE_PASID, struct ia32_pasid_state);
 
/*
 * Make *SURE* to add any feature numbers in below if
-- 
2.19.1

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


[PATCH v6 04/12] x86/cpufeatures: Enumerate ENQCMD and ENQCMDS instructions

2020-07-13 Thread Fenghua Yu
Work submission instruction comes in two flavors. ENQCMD can be called
both in ring 3 and ring 0 and always uses the contents of PASID MSR when
shipping the command to the device. ENQCMDS allows a kernel driver to
submit commands on behalf of a user process. The driver supplies the
PASID value in ENQCMDS. There isn't any usage of ENQCMD in the kernel
as of now.

The CPU feature flag is shown as "enqcmd" in /proc/cpuinfo.

Signed-off-by: Fenghua Yu 
Reviewed-by: Tony Luck 
---
v2:
- Re-write commit message (Thomas)

 arch/x86/include/asm/cpufeatures.h | 1 +
 arch/x86/kernel/cpu/cpuid-deps.c   | 1 +
 2 files changed, 2 insertions(+)

diff --git a/arch/x86/include/asm/cpufeatures.h 
b/arch/x86/include/asm/cpufeatures.h
index 02dabc9e77b0..4469618c410f 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -351,6 +351,7 @@
 #define X86_FEATURE_CLDEMOTE   (16*32+25) /* CLDEMOTE instruction */
 #define X86_FEATURE_MOVDIRI(16*32+27) /* MOVDIRI instruction */
 #define X86_FEATURE_MOVDIR64B  (16*32+28) /* MOVDIR64B instruction */
+#define X86_FEATURE_ENQCMD (16*32+29) /* ENQCMD and ENQCMDS 
instructions */
 
 /* AMD-defined CPU features, CPUID level 0x8007 (EBX), word 17 */
 #define X86_FEATURE_OVERFLOW_RECOV (17*32+ 0) /* MCA overflow recovery 
support */
diff --git a/arch/x86/kernel/cpu/cpuid-deps.c b/arch/x86/kernel/cpu/cpuid-deps.c
index 3cbe24ca80ab..3a02707c1f4d 100644
--- a/arch/x86/kernel/cpu/cpuid-deps.c
+++ b/arch/x86/kernel/cpu/cpuid-deps.c
@@ -69,6 +69,7 @@ static const struct cpuid_dep cpuid_deps[] = {
{ X86_FEATURE_CQM_MBM_TOTAL,X86_FEATURE_CQM_LLC   },
{ X86_FEATURE_CQM_MBM_LOCAL,X86_FEATURE_CQM_LLC   },
{ X86_FEATURE_AVX512_BF16,  X86_FEATURE_AVX512VL  },
+   { X86_FEATURE_ENQCMD,   X86_FEATURE_XSAVES},
{}
 };
 
-- 
2.19.1

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


Re: [PATCH v4 1/5] docs: IOMMU user API

2020-07-13 Thread Alex Williamson
On Tue,  7 Jul 2020 16:43:45 -0700
Jacob Pan  wrote:

> IOMMU UAPI is newly introduced to support communications between guest
> virtual IOMMU and host IOMMU. There has been lots of discussions on how
> it should work with VFIO UAPI and userspace in general.
> 
> This document is indended to clarify the UAPI design and usage. The
> mechenics of how future extensions should be achieved are also covered

mechanics

> in this documentation.
> 
> Signed-off-by: Liu Yi L 
> Signed-off-by: Jacob Pan 
> ---
>  Documentation/userspace-api/iommu.rst | 312 
> ++
>  1 file changed, 312 insertions(+)
>  create mode 100644 Documentation/userspace-api/iommu.rst
> 
> diff --git a/Documentation/userspace-api/iommu.rst 
> b/Documentation/userspace-api/iommu.rst
> new file mode 100644
> index ..581b462c2cec
> --- /dev/null
> +++ b/Documentation/userspace-api/iommu.rst
> @@ -0,0 +1,312 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +.. iommu:
> +
> +=
> +IOMMU Userspace API
> +=
> +
> +IOMMU UAPI is used for virtualization cases where communications are
> +needed between physical and virtual IOMMU drivers. For native
> +usage, IOMMU is a system device which does not need to communicate
> +with user space directly.
> +
> +The primary use cases are guest Shared Virtual Address (SVA) and
> +guest IO virtual address (IOVA), wherein a virtual IOMMU (vIOMMU) is
> +required to communicate with the physical IOMMU in the host.
> +
> +.. contents:: :local:
> +
> +Functionalities
> +===
> +Communications of user and kernel involve both directions. The
> +supported user-kernel APIs are as follows:
> +
> +1. Alloc/Free PASID
> +2. Bind/unbind guest PASID (e.g. Intel VT-d)
> +3. Bind/unbind guest PASID table (e.g. ARM sMMU)
> +4. Invalidate IOMMU caches
> +5. Service page requests
> +
> +Requirements
> +
> +The IOMMU UAPIs are generic and extensible to meet the following
> +requirements:
> +
> +1. Emulated and para-virtualised vIOMMUs
> +2. Multiple vendors (Intel VT-d, ARM sMMU, etc.)
> +3. Extensions to the UAPI shall not break existing user space
> +
> +Interfaces
> +==
> +Although the data structures defined in IOMMU UAPI are self-contained,
> +there is no user API functions introduced. Instead, IOMMU UAPI is
> +designed to work with existing user driver frameworks such as VFIO.
> +
> +Extension Rules & Precautions
> +-
> +When IOMMU UAPI gets extended, the data structures can *only* be
> +modified in two ways:
> +
> +1. Adding new fields by re-purposing the padding[] field. No size change.
> +2. Adding new union members at the end. May increase in size.
> +
> +No new fields can be added *after* the variable sized union in that it
> +will break backward compatibility when offset moves. In both cases, a
> +new flag must be accompanied with a new field such that the IOMMU
> +driver can process the data based on the new flag. Version field is
> +only reserved for the unlikely event of UAPI upgrade at its entirety.
> +
> +It's *always* the caller's responsibility to indicate the size of the
> +structure passed by setting argsz appropriately.
> +Though at the same time, argsz is user provided data which is not
> +trusted. The argsz field allows the user to indicate how much data
> +they're providing, it's still the kernel's responsibility to validate
> +whether it's correct and sufficient for the requested operation.
> +
> +Compatibility Checking
> +--
> +When IOMMU UAPI extension results in size increase, user such as VFIO
> +has to handle the following cases:
> +
> +1. User and kernel has exact size match
> +2. An older user with older kernel header (smaller UAPI size) running on a
> +   newer kernel (larger UAPI size)
> +3. A newer user with newer kernel header (larger UAPI size) running
> +   on an older kernel.
> +4. A malicious/misbehaving user pass illegal/invalid size but within
> +   range. The data may contain garbage.

I'm still not sure where VFIO has responsibility in managing any of
these cases.  I think we've determined that VFIO is just the wrapper
and call-through mechanism, it's the UAPI core implementation and
IOMMU drivers that are responsible for this.

> +
> +Feature Checking
> +
> +While launching a guest with vIOMMU, it is important to ensure that host
> +can support the UAPI data structures to be used for vIOMMU-pIOMMU
> +communications. Without upfront compatibility checking, future faults
> +are difficult to report even in normal conditions. For example, TLB
> +invalidations should always succeed. There is no architectural way to
> +report back to the vIOMMU if the UAPI data is incompatible. If that
> +happens, in order to protect IOMMU iosolation guarantee, we have to
> +resort to not giving completion status in vIOMMU. This may result in
> +VM hang.
> +
> +For this reason the following IOMMU UAPIs cannot 

[PATCH 4/9] dt-bindings: dma: renesas, rcar-dmac: Document R8A774E1 bindings

2020-07-13 Thread Lad Prabhakar
Renesas RZ/G2H (R8A774E1) SoC also has the R-Car gen3 compatible
DMA controllers, therefore document RZ/G2H specific bindings.

Signed-off-by: Lad Prabhakar 
---
 Documentation/devicetree/bindings/dma/renesas,rcar-dmac.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/dma/renesas,rcar-dmac.yaml 
b/Documentation/devicetree/bindings/dma/renesas,rcar-dmac.yaml
index b842dfd96a89..13f1a46be40d 100644
--- a/Documentation/devicetree/bindings/dma/renesas,rcar-dmac.yaml
+++ b/Documentation/devicetree/bindings/dma/renesas,rcar-dmac.yaml
@@ -23,6 +23,7 @@ properties:
   - renesas,dmac-r8a774a1 # RZ/G2M
   - renesas,dmac-r8a774b1 # RZ/G2N
   - renesas,dmac-r8a774c0 # RZ/G2E
+  - renesas,dmac-r8a774e1 # RZ/G2H
   - renesas,dmac-r8a7790  # R-Car H2
   - renesas,dmac-r8a7791  # R-Car M2-W
   - renesas,dmac-r8a7792  # R-Car V2H
-- 
2.17.1

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


[PATCH 5/9] arm64: dts: renesas: r8a774e1: Add SYS-DMAC device nodes

2020-07-13 Thread Lad Prabhakar
From: Marian-Cristian Rotariu 

Add sys-dmac[0-2] device nodes for RZ/G2H (R8A774E1) SoC.

Signed-off-by: Marian-Cristian Rotariu 

Signed-off-by: Lad Prabhakar 
---
 arch/arm64/boot/dts/renesas/r8a774e1.dtsi | 126 ++
 1 file changed, 126 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a774e1.dtsi 
b/arch/arm64/boot/dts/renesas/r8a774e1.dtsi
index 0fc0d9ff5bc5..9e05d134a295 100644
--- a/arch/arm64/boot/dts/renesas/r8a774e1.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a774e1.dtsi
@@ -408,6 +408,132 @@
/* placeholder */
};
 
+   dmac0: dma-controller@e670 {
+   compatible = "renesas,dmac-r8a774e1",
+"renesas,rcar-dmac";
+   reg = <0 0xe670 0 0x1>;
+   interrupts = ,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+;
+   interrupt-names = "error",
+ "ch0", "ch1", "ch2", "ch3",
+ "ch4", "ch5", "ch6", "ch7",
+ "ch8", "ch9", "ch10", "ch11",
+ "ch12", "ch13", "ch14", "ch15";
+   clocks = < CPG_MOD 219>;
+   clock-names = "fck";
+   power-domains = < R8A774E1_PD_ALWAYS_ON>;
+   resets = < 219>;
+   #dma-cells = <1>;
+   dma-channels = <16>;
+   iommus = <_ds0 0>, <_ds0 1>,
+<_ds0 2>, <_ds0 3>,
+<_ds0 4>, <_ds0 5>,
+<_ds0 6>, <_ds0 7>,
+<_ds0 8>, <_ds0 9>,
+<_ds0 10>, <_ds0 11>,
+<_ds0 12>, <_ds0 13>,
+<_ds0 14>, <_ds0 15>;
+   };
+
+   dmac1: dma-controller@e730 {
+   compatible = "renesas,dmac-r8a774e1",
+"renesas,rcar-dmac";
+   reg = <0 0xe730 0 0x1>;
+   interrupts = ,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+;
+   interrupt-names = "error",
+ "ch0", "ch1", "ch2", "ch3",
+ "ch4", "ch5", "ch6", "ch7",
+ "ch8", "ch9", "ch10", "ch11",
+ "ch12", "ch13", "ch14", "ch15";
+   clocks = < CPG_MOD 218>;
+   clock-names = "fck";
+   power-domains = < R8A774E1_PD_ALWAYS_ON>;
+   resets = < 218>;
+   #dma-cells = <1>;
+   dma-channels = <16>;
+   iommus = <_ds1 0>, <_ds1 1>,
+<_ds1 2>, <_ds1 3>,
+<_ds1 4>, <_ds1 5>,
+<_ds1 6>, <_ds1 7>,
+<_ds1 8>, <_ds1 9>,
+<_ds1 10>, <_ds1 11>,
+<_ds1 12>, <_ds1 13>,
+<_ds1 14>, <_ds1 15>;
+   };
+
+   dmac2: dma-controller@e731 {
+   compatible = "renesas,dmac-r8a774e1",
+"renesas,rcar-dmac";
+   reg = <0 0xe731 0 0x1>;
+   interrupts = ,
+,
+,
+,
+,
+,
+

[PATCH 6/9] dt-bindings: gpio: renesas, rcar-gpio: Add r8a774e1 support

2020-07-13 Thread Lad Prabhakar
Document Renesas RZ/G2H (R8A774E1) GPIO blocks compatibility within the
relevant dt-bindings.

Signed-off-by: Lad Prabhakar 
---
 Documentation/devicetree/bindings/gpio/renesas,rcar-gpio.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/gpio/renesas,rcar-gpio.yaml 
b/Documentation/devicetree/bindings/gpio/renesas,rcar-gpio.yaml
index 397d9383d15a..a9a9dd0854e7 100644
--- a/Documentation/devicetree/bindings/gpio/renesas,rcar-gpio.yaml
+++ b/Documentation/devicetree/bindings/gpio/renesas,rcar-gpio.yaml
@@ -37,6 +37,7 @@ properties:
  - renesas,gpio-r8a774a1 # RZ/G2M
  - renesas,gpio-r8a774b1 # RZ/G2N
  - renesas,gpio-r8a774c0 # RZ/G2E
+ - renesas,gpio-r8a774e1 # RZ/G2H
  - renesas,gpio-r8a7795  # R-Car H3
  - renesas,gpio-r8a7796  # R-Car M3-W
  - renesas,gpio-r8a77961 # R-Car M3-W+
-- 
2.17.1

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


[PATCH 8/9] dt-bindings: net: renesas, ravb: Add support for r8a774e1 SoC

2020-07-13 Thread Lad Prabhakar
From: Marian-Cristian Rotariu 

Document RZ/G2H (R8A774E1) SoC bindings.

Signed-off-by: Marian-Cristian Rotariu 

Signed-off-by: Lad Prabhakar 
---
 Documentation/devicetree/bindings/net/renesas,ravb.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/net/renesas,ravb.txt 
b/Documentation/devicetree/bindings/net/renesas,ravb.txt
index 032b76f14f4f..9119f1caf391 100644
--- a/Documentation/devicetree/bindings/net/renesas,ravb.txt
+++ b/Documentation/devicetree/bindings/net/renesas,ravb.txt
@@ -21,6 +21,7 @@ Required properties:
   - "renesas,etheravb-r8a774a1" for the R8A774A1 SoC.
   - "renesas,etheravb-r8a774b1" for the R8A774B1 SoC.
   - "renesas,etheravb-r8a774c0" for the R8A774C0 SoC.
+  - "renesas,etheravb-r8a774e1" for the R8A774E1 SoC.
   - "renesas,etheravb-r8a7795" for the R8A7795 SoC.
   - "renesas,etheravb-r8a7796" for the R8A77960 SoC.
   - "renesas,etheravb-r8a77961" for the R8A77961 SoC.
-- 
2.17.1

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


[PATCH 3/9] arm64: dts: renesas: r8a774e1: Add IPMMU device nodes

2020-07-13 Thread Lad Prabhakar
From: Marian-Cristian Rotariu 

Add RZ/G2H (R8A774E1) IPMMU nodes.

Signed-off-by: Marian-Cristian Rotariu 

Signed-off-by: Lad Prabhakar 
---
 arch/arm64/boot/dts/renesas/r8a774e1.dtsi | 121 ++
 1 file changed, 121 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a774e1.dtsi 
b/arch/arm64/boot/dts/renesas/r8a774e1.dtsi
index 7484ad530068..0fc0d9ff5bc5 100644
--- a/arch/arm64/boot/dts/renesas/r8a774e1.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a774e1.dtsi
@@ -408,6 +408,127 @@
/* placeholder */
};
 
+   ipmmu_ds0: iommu@e674 {
+   compatible = "renesas,ipmmu-r8a774e1";
+   reg = <0 0xe674 0 0x1000>;
+   renesas,ipmmu-main = <_mm 0>;
+   power-domains = < R8A774E1_PD_ALWAYS_ON>;
+   #iommu-cells = <1>;
+   };
+
+   ipmmu_ds1: iommu@e774 {
+   compatible = "renesas,ipmmu-r8a774e1";
+   reg = <0 0xe774 0 0x1000>;
+   renesas,ipmmu-main = <_mm 1>;
+   power-domains = < R8A774E1_PD_ALWAYS_ON>;
+   #iommu-cells = <1>;
+   };
+
+   ipmmu_hc: iommu@e657 {
+   compatible = "renesas,ipmmu-r8a774e1";
+   reg = <0 0xe657 0 0x1000>;
+   renesas,ipmmu-main = <_mm 2>;
+   power-domains = < R8A774E1_PD_ALWAYS_ON>;
+   #iommu-cells = <1>;
+   };
+
+   ipmmu_mm: iommu@e67b {
+   compatible = "renesas,ipmmu-r8a774e1";
+   reg = <0 0xe67b 0 0x1000>;
+   interrupts = ,
+;
+   power-domains = < R8A774E1_PD_ALWAYS_ON>;
+   #iommu-cells = <1>;
+   };
+
+   ipmmu_mp0: iommu@ec67 {
+   compatible = "renesas,ipmmu-r8a774e1";
+   reg = <0 0xec67 0 0x1000>;
+   renesas,ipmmu-main = <_mm 4>;
+   power-domains = < R8A774E1_PD_ALWAYS_ON>;
+   #iommu-cells = <1>;
+   };
+
+   ipmmu_pv0: iommu@fd80 {
+   compatible = "renesas,ipmmu-r8a774e1";
+   reg = <0 0xfd80 0 0x1000>;
+   renesas,ipmmu-main = <_mm 6>;
+   power-domains = < R8A774E1_PD_ALWAYS_ON>;
+   #iommu-cells = <1>;
+   };
+
+   ipmmu_pv1: iommu@fd95 {
+   compatible = "renesas,ipmmu-r8a774e1";
+   reg = <0 0xfd95 0 0x1000>;
+   renesas,ipmmu-main = <_mm 7>;
+   power-domains = < R8A774E1_PD_ALWAYS_ON>;
+   #iommu-cells = <1>;
+   };
+
+   ipmmu_pv2: iommu@fd96 {
+   compatible = "renesas,ipmmu-r8a774e1";
+   reg = <0 0xfd96 0 0x1000>;
+   renesas,ipmmu-main = <_mm 8>;
+   power-domains = < R8A774E1_PD_ALWAYS_ON>;
+   #iommu-cells = <1>;
+   };
+
+   ipmmu_pv3: iommu@fd97 {
+   compatible = "renesas,ipmmu-r8a774e1";
+   reg = <0 0xfd97 0 0x1000>;
+   renesas,ipmmu-main = <_mm 9>;
+   power-domains = < R8A774E1_PD_ALWAYS_ON>;
+   #iommu-cells = <1>;
+   };
+
+   ipmmu_vc0: iommu@fe6b {
+   compatible = "renesas,ipmmu-r8a774e1";
+   reg = <0 0xfe6b 0 0x1000>;
+   renesas,ipmmu-main = <_mm 12>;
+   power-domains = < R8A774E1_PD_A3VC>;
+   #iommu-cells = <1>;
+   };
+
+   ipmmu_vc1: iommu@fe6f {
+   compatible = "renesas,ipmmu-r8a774e1";
+   reg = <0 0xfe6f 0 0x1000>;
+   renesas,ipmmu-main = <_mm 13>;
+   power-domains = < R8A774E1_PD_A3VC>;
+   #iommu-cells = <1>;
+   };
+
+   ipmmu_vi0: iommu@febd {
+   compatible = "renesas,ipmmu-r8a774e1";
+   reg = <0 0xfebd 0 0x1000>;
+   renesas,ipmmu-main = <_mm 14>;
+   power-domains = < R8A774E1_PD_ALWAYS_ON>;
+   #iommu-cells = <1>;
+   };
+
+   ipmmu_vi1: iommu@febe {
+   compatible = "renesas,ipmmu-r8a774e1";
+   reg = <0 0xfebe 0 0x1000>;
+   renesas,ipmmu-main = <_mm 15>;
+   

[PATCH 0/9] R8A774E1 SoC enable support for IPMMU, DMAC, GPIO and AVB

2020-07-13 Thread Lad Prabhakar
Hi All,

This patch series adds device nodes for IPMMU, DMAC, GPIO
and AVB nodes for RZ/G2H (R8A774E1) SoC.

Cheers,
Prabhakar

Lad Prabhakar (3):
  dt-bindings: iommu: renesas,ipmmu-vmsa: Add r8a774e1 support
  dt-bindings: dma: renesas,rcar-dmac: Document R8A774E1 bindings
  dt-bindings: gpio: renesas,rcar-gpio: Add r8a774e1 support

Marian-Cristian Rotariu (6):
  iommu/ipmmu-vmsa: Hook up R8A774E1 DT matching code
  arm64: dts: renesas: r8a774e1: Add IPMMU device nodes
  arm64: dts: renesas: r8a774e1: Add SYS-DMAC device nodes
  arm64: dts: renesas: r8a774e1: Add GPIO device nodes
  dt-bindings: net: renesas,ravb: Add support for r8a774e1 SoC
  arm64: dts: renesas: r8a774e1: Add Ethernet AVB node

 .../bindings/dma/renesas,rcar-dmac.yaml   |   1 +
 .../bindings/gpio/renesas,rcar-gpio.yaml  |   1 +
 .../bindings/iommu/renesas,ipmmu-vmsa.yaml|   1 +
 .../devicetree/bindings/net/renesas,ravb.txt  |   1 +
 arch/arm64/boot/dts/renesas/r8a774e1.dtsi | 361 +-
 drivers/iommu/ipmmu-vmsa.c|   4 +
 6 files changed, 350 insertions(+), 19 deletions(-)

-- 
2.17.1

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


[PATCH 1/9] dt-bindings: iommu: renesas, ipmmu-vmsa: Add r8a774e1 support

2020-07-13 Thread Lad Prabhakar
Document RZ/G2H (R8A774E1) SoC bindings.

Signed-off-by: Lad Prabhakar 
---
 Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.yaml 
b/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.yaml
index e9d28a4060fa..6bfa090fd73a 100644
--- a/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.yaml
+++ b/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.yaml
@@ -32,6 +32,7 @@ properties:
   - enum:
   - renesas,ipmmu-r8a774a1 # RZ/G2M
   - renesas,ipmmu-r8a774b1 # RZ/G2N
+  - renesas,ipmmu-r8a774e1 # RZ/G2H
   - renesas,ipmmu-r8a774c0 # RZ/G2E
   - renesas,ipmmu-r8a7795  # R-Car H3
   - renesas,ipmmu-r8a7796  # R-Car M3-W
-- 
2.17.1

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


[PATCH 9/9] arm64: dts: renesas: r8a774e1: Add Ethernet AVB node

2020-07-13 Thread Lad Prabhakar
From: Marian-Cristian Rotariu 

This patch adds the SoC specific part of the Ethernet AVB
device tree node.

Signed-off-by: Marian-Cristian Rotariu 

Signed-off-by: Lad Prabhakar 
---
 arch/arm64/boot/dts/renesas/r8a774e1.dtsi | 41 +--
 1 file changed, 39 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a774e1.dtsi 
b/arch/arm64/boot/dts/renesas/r8a774e1.dtsi
index 599703d87b56..caca319aafcf 100644
--- a/arch/arm64/boot/dts/renesas/r8a774e1.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a774e1.dtsi
@@ -695,12 +695,49 @@
};
 
avb: ethernet@e680 {
+   compatible = "renesas,etheravb-r8a774e1",
+"renesas,etheravb-rcar-gen3";
reg = <0 0xe680 0 0x800>;
+   interrupts = ,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+;
+   interrupt-names = "ch0", "ch1", "ch2", "ch3",
+ "ch4", "ch5", "ch6", "ch7",
+ "ch8", "ch9", "ch10", "ch11",
+ "ch12", "ch13", "ch14", "ch15",
+ "ch16", "ch17", "ch18", "ch19",
+ "ch20", "ch21", "ch22", "ch23",
+ "ch24";
+   clocks = < CPG_MOD 812>;
+   power-domains = < R8A774E1_PD_ALWAYS_ON>;
+   resets = < 812>;
+   phy-mode = "rgmii";
+   iommus = <_ds0 16>;
#address-cells = <1>;
#size-cells = <0>;
status = "disabled";
-
-   /* placeholder */
};
 
can0: can@e6c3 {
-- 
2.17.1

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


[PATCH 7/9] arm64: dts: renesas: r8a774e1: Add GPIO device nodes

2020-07-13 Thread Lad Prabhakar
From: Marian-Cristian Rotariu 

Add GPIO device nodes to the DT of the r8a774e1 SoC.

Signed-off-by: Marian-Cristian Rotariu 

Signed-off-by: Lad Prabhakar 
---
 arch/arm64/boot/dts/renesas/r8a774e1.dtsi | 73 +--
 1 file changed, 56 insertions(+), 17 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a774e1.dtsi 
b/arch/arm64/boot/dts/renesas/r8a774e1.dtsi
index 9e05d134a295..599703d87b56 100644
--- a/arch/arm64/boot/dts/renesas/r8a774e1.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a774e1.dtsi
@@ -246,84 +246,123 @@
};
 
gpio0: gpio@e605 {
+   compatible = "renesas,gpio-r8a774e1",
+"renesas,rcar-gen3-gpio";
reg = <0 0xe605 0 0x50>;
+   interrupts = ;
#gpio-cells = <2>;
gpio-controller;
+   gpio-ranges = < 0 0 16>;
#interrupt-cells = <2>;
interrupt-controller;
-
-   /* placeholder */
+   clocks = < CPG_MOD 912>;
+   power-domains = < R8A774E1_PD_ALWAYS_ON>;
+   resets = < 912>;
};
 
gpio1: gpio@e6051000 {
+   compatible = "renesas,gpio-r8a774e1",
+"renesas,rcar-gen3-gpio";
reg = <0 0xe6051000 0 0x50>;
+   interrupts = ;
#gpio-cells = <2>;
gpio-controller;
+   gpio-ranges = < 0 32 29>;
#interrupt-cells = <2>;
interrupt-controller;
-
-   /* placeholder */
+   clocks = < CPG_MOD 911>;
+   power-domains = < R8A774E1_PD_ALWAYS_ON>;
+   resets = < 911>;
};
 
gpio2: gpio@e6052000 {
+   compatible = "renesas,gpio-r8a774e1",
+"renesas,rcar-gen3-gpio";
reg = <0 0xe6052000 0 0x50>;
+   interrupts = ;
#gpio-cells = <2>;
gpio-controller;
+   gpio-ranges = < 0 64 15>;
#interrupt-cells = <2>;
interrupt-controller;
-
-   /* placeholder */
+   clocks = < CPG_MOD 910>;
+   power-domains = < R8A774E1_PD_ALWAYS_ON>;
+   resets = < 910>;
};
 
gpio3: gpio@e6053000 {
-   /* placeholder */
+   compatible = "renesas,gpio-r8a774e1",
+"renesas,rcar-gen3-gpio";
reg = <0 0xe6053000 0 0x50>;
+   interrupts = ;
#gpio-cells = <2>;
gpio-controller;
+   gpio-ranges = < 0 96 16>;
#interrupt-cells = <2>;
interrupt-controller;
-
-   /* placeholder */
+   clocks = < CPG_MOD 909>;
+   power-domains = < R8A774E1_PD_ALWAYS_ON>;
+   resets = < 909>;
};
 
gpio4: gpio@e6054000 {
+   compatible = "renesas,gpio-r8a774e1",
+"renesas,rcar-gen3-gpio";
reg = <0 0xe6054000 0 0x50>;
+   interrupts = ;
#gpio-cells = <2>;
gpio-controller;
+   gpio-ranges = < 0 128 18>;
#interrupt-cells = <2>;
interrupt-controller;
-
-   /* placeholder */
+   clocks = < CPG_MOD 908>;
+   power-domains = < R8A774E1_PD_ALWAYS_ON>;
+   resets = < 908>;
};
 
gpio5: gpio@e6055000 {
+   compatible = "renesas,gpio-r8a774e1",
+"renesas,rcar-gen3-gpio";
reg = <0 0xe6055000 0 0x50>;
+   interrupts = ;
#gpio-cells = <2>;
gpio-controller;
+   gpio-ranges = < 0 160 26>;
#interrupt-cells = <2>;
interrupt-controller;
-
-   /* placeholder */
+   clocks = < CPG_MOD 907>;
+   power-domains = < R8A774E1_PD_ALWAYS_ON>;
+   resets = < 907>;
};
 
gpio6: gpio@e6055400 {
+   compatible = "renesas,gpio-r8a774e1",
+

[PATCH 2/9] iommu/ipmmu-vmsa: Hook up R8A774E1 DT matching code

2020-07-13 Thread Lad Prabhakar
From: Marian-Cristian Rotariu 

Add support for RZ/G2H (R8A774E1) SoC IPMMUs.

Signed-off-by: Marian-Cristian Rotariu 

Signed-off-by: Lad Prabhakar 
---
 drivers/iommu/ipmmu-vmsa.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index b90cd9ff96f6..cbce88ac5a71 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -751,6 +751,7 @@ static const struct soc_device_attribute soc_rcar_gen3[] = {
 static const struct soc_device_attribute soc_rcar_gen3_whitelist[] = {
{ .soc_id = "r8a774b1", },
{ .soc_id = "r8a774c0", },
+   { .soc_id = "r8a774e1", },
{ .soc_id = "r8a7795", .revision = "ES3.*" },
{ .soc_id = "r8a77961", },
{ .soc_id = "r8a77965", },
@@ -963,6 +964,9 @@ static const struct of_device_id ipmmu_of_ids[] = {
}, {
.compatible = "renesas,ipmmu-r8a774c0",
.data = _features_rcar_gen3,
+   }, {
+   .compatible = "renesas,ipmmu-r8a774e1",
+   .data = _features_rcar_gen3,
}, {
.compatible = "renesas,ipmmu-r8a7795",
.data = _features_rcar_gen3,
-- 
2.17.1

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


Re: [PATCH v3 3/4] dt-bindings: arm-smmu: add compatible string for Marvell Armada-AP806 SMMU-500

2020-07-13 Thread Rob Herring
On Fri, Jul 03, 2020 at 11:26:32AM +0200, Tomasz Nowicki wrote:
> On 03.07.2020 11:05, Robin Murphy wrote:
> > On 2020-07-02 21:16, Tomasz Nowicki wrote:
> > > Add specific compatible string for Marvell usage due to errata of
> > > accessing 64bits registers of ARM SMMU, in AP806.
> > > 
> > > AP806 SoC uses the generic ARM-MMU500, and there's no specific
> > > implementation of Marvell, this compatible is used for errata only.
> > > 
> > > Signed-off-by: Hanna Hawa 
> > > Signed-off-by: Gregory CLEMENT 
> > > Signed-off-by: Tomasz Nowicki 
> > > ---
> > >   Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 5 +
> > >   1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
> > > b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
> > > index d7ceb4c34423..7beca9c00b12 100644
> > > --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
> > > +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
> > > @@ -38,6 +38,11 @@ properties:
> > >     - qcom,sc7180-smmu-500
> > >     - qcom,sdm845-smmu-500
> > >     - const: arm,mmu-500
> > > +  - description: Marvell SoCs implementing "arm,mmu-500"
> > > +    items:
> > > +  - enum:
> > > +  - marvell,ap806-smmu-500
> > 
> > Isn't a single-valued enum just a constant? :P
> 
> That's how copy-paste engineering ends up :)

It's fine like this if you expect more SoCs to be added.

Either way,

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


Re: [PATCH 1/5] iommu/arm-smmu: Make all valid stream mappings BYPASS

2020-07-13 Thread kernel test robot
Hi Bjorn,

I love your patch! Perhaps something to improve:

[auto build test WARNING on iommu/next]
[also build test WARNING on arm-perf/for-next/perf v5.8-rc5 next-20200713]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use  as documented in
https://git-scm.com/docs/git-format-patch]

url:
https://github.com/0day-ci/linux/commits/Bjorn-Andersson/iommu-arm-smmu-Support-maintaining-bootloader-mappings/20200709-130417
base:   https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
config: x86_64-randconfig-s021-20200713 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-14) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.2-37-gc9676a3b-dirty
# save the attached .config to linux build tree
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot 


sparse warnings: (new ones prefixed by >>)

>> drivers/iommu/arm-smmu.c:1927:5: sparse: sparse: symbol 
>> 'arm_smmu_setup_identity' was not declared. Should it be static?

Please review and possibly fold the followup patch.

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org


.config.gz
Description: application/gzip
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[RFC PATCH] iommu/arm-smmu: arm_smmu_setup_identity() can be static

2020-07-13 Thread kernel test robot


Signed-off-by: kernel test robot 
---
 arm-smmu.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 2e27cf9815ab6..fb85e716ae9ac 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1924,7 +1924,7 @@ static int arm_smmu_device_cfg_probe(struct 
arm_smmu_device *smmu)
return 0;
 }
 
-int arm_smmu_setup_identity(struct arm_smmu_device *smmu)
+static int arm_smmu_setup_identity(struct arm_smmu_device *smmu)
 {
int i;
 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2 5/5] firmware: QCOM_SCM: Allow qcom_scm driver to be loadable as a permenent module

2020-07-13 Thread John Stultz
On Mon, Jul 13, 2020 at 1:41 PM Will Deacon  wrote:
>
> On Fri, Jul 10, 2020 at 03:21:53PM -0700, John Stultz wrote:
> > On Fri, Jul 10, 2020 at 12:54 AM Will Deacon  wrote:
> > > On Thu, Jul 09, 2020 at 08:28:45PM -0700, John Stultz wrote:
> > > > On Thu, Jul 2, 2020 at 7:18 AM Will Deacon  wrote:
> > > > > On Thu, Jun 25, 2020 at 12:10:39AM +, John Stultz wrote:
> > > > > > diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> > > > > > index b510f67dfa49..714893535dd2 100644
> > > > > > --- a/drivers/iommu/Kconfig
> > > > > > +++ b/drivers/iommu/Kconfig
> > > > > > @@ -381,6 +381,7 @@ config SPAPR_TCE_IOMMU
> > > > > >  config ARM_SMMU
> > > > > >   tristate "ARM Ltd. System MMU (SMMU) Support"
> > > > > >   depends on (ARM64 || ARM || (COMPILE_TEST && 
> > > > > > !GENERIC_ATOMIC64)) && MMU
> > > > > > + depends on QCOM_SCM || !QCOM_SCM #if QCOM_SCM=m this can't be 
> > > > > > =y
> > > > > >   select IOMMU_API
> > > > > >   select IOMMU_IO_PGTABLE_LPAE
> > > > > >   select ARM_DMA_USE_IOMMU if ARM
> > > > >
> > > > > This looks like a giant hack. Is there another way to handle this?
> > > >
> > > > Sorry for the slow response here.
> > > >
> > > > So, I agree the syntax looks strange (requiring a comment obviously
> > > > isn't a good sign), but it's a fairly common way to ensure drivers
> > > > don't get built in if they optionally depend on another driver that
> > > > can be built as a module.
> > > >   See "RFKILL || !RFKILL", "EXTCON || !EXTCON", or "USB_GADGET ||
> > > > !USB_GADGET" in various Kconfig files.
> > > >
> > > > I'm open to using a different method, and in a different thread you
> > > > suggested using something like symbol_get(). I need to look into it
> > > > more, but that approach looks even more messy and prone to runtime
> > > > failures. Blocking the unwanted case at build time seems a bit cleaner
> > > > to me, even if the syntax is odd.
> > >
> > > Maybe just split it out then, so that the ARM_SMMU entry doesn't have 
> > > this,
> > > as that driver _really_ doesn't care about SoC details like this. In other
> > > words, add a new entry along the lines of:
> > >
> > > config ARM_SMMU_QCOM_IMPL
> > > default y
> > > #if QCOM_SCM=m this can't be =y
> > > depends on ARM_SMMU & (QCOM_SCM || !QCOM_SCM)
> > >
> > > and then have arm-smmu.h provide a static inline qcom_smmu_impl_init()
> > > which returns -ENODEV if CONFIG_ARM_SMMU_QCOM_IMPL=n and hack the Makefile
> > > so that we don't bother to compile arm-smmu-qcom.o in that case.
> > >
> > > Would that work?
> >
> > I think this proposal still has problems with the directionality of the 
> > call.
> >
> > The arm-smmu-impl.o calls to arm-smmu-qcom.o which calls qcom_scm.o
> > So if qcom_scm.o is part of a module, the calling code in
> > arm-smmu-qcom.o also needs to be a module, which means CONFIG_ARM_SMMU
> > needs to be a module.
> >
> > I know you said the arm-smmu driver doesn't care about SoC details,
> > but the trouble is that currently the arm-smmu driver does directly
> > call the qcom-scm code. So it is a real dependency. However, if
> > QCOM_SCM is not configured, it calls stubs and that's ok.  In that
> > way, the "depends on QCOM_SCM || !QCOM_SCM" line actually makes sense.
> > It looks terrible because we're used to boolean logic, but it's
> > ternary.
>
> Yes, it looks ugly, but the part I really have issues with is that building
> QCOM_SCM=m and ARM_SMMU=y is perfectly fine if you don't run on an SoC
> with the qcom implementation. I don't see why we need to enforce things
> here beyond making sure that all selectable permutations _build_ and
> fail gracefully at runtime on the qcom SoC if SCM isn't available.

Ok. Thanks, that context/rationale makes sense to me now!  I'll dig in
and see if I can figure out the runtime get_symbol handling you
suggested for the scm callout.

Thanks again for the feedback!
-john
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2 5/5] firmware: QCOM_SCM: Allow qcom_scm driver to be loadable as a permenent module

2020-07-13 Thread Will Deacon
On Fri, Jul 10, 2020 at 03:21:53PM -0700, John Stultz wrote:
> On Fri, Jul 10, 2020 at 12:54 AM Will Deacon  wrote:
> > On Thu, Jul 09, 2020 at 08:28:45PM -0700, John Stultz wrote:
> > > On Thu, Jul 2, 2020 at 7:18 AM Will Deacon  wrote:
> > > > On Thu, Jun 25, 2020 at 12:10:39AM +, John Stultz wrote:
> > > > > diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> > > > > index b510f67dfa49..714893535dd2 100644
> > > > > --- a/drivers/iommu/Kconfig
> > > > > +++ b/drivers/iommu/Kconfig
> > > > > @@ -381,6 +381,7 @@ config SPAPR_TCE_IOMMU
> > > > >  config ARM_SMMU
> > > > >   tristate "ARM Ltd. System MMU (SMMU) Support"
> > > > >   depends on (ARM64 || ARM || (COMPILE_TEST && 
> > > > > !GENERIC_ATOMIC64)) && MMU
> > > > > + depends on QCOM_SCM || !QCOM_SCM #if QCOM_SCM=m this can't be =y
> > > > >   select IOMMU_API
> > > > >   select IOMMU_IO_PGTABLE_LPAE
> > > > >   select ARM_DMA_USE_IOMMU if ARM
> > > >
> > > > This looks like a giant hack. Is there another way to handle this?
> > >
> > > Sorry for the slow response here.
> > >
> > > So, I agree the syntax looks strange (requiring a comment obviously
> > > isn't a good sign), but it's a fairly common way to ensure drivers
> > > don't get built in if they optionally depend on another driver that
> > > can be built as a module.
> > >   See "RFKILL || !RFKILL", "EXTCON || !EXTCON", or "USB_GADGET ||
> > > !USB_GADGET" in various Kconfig files.
> > >
> > > I'm open to using a different method, and in a different thread you
> > > suggested using something like symbol_get(). I need to look into it
> > > more, but that approach looks even more messy and prone to runtime
> > > failures. Blocking the unwanted case at build time seems a bit cleaner
> > > to me, even if the syntax is odd.
> >
> > Maybe just split it out then, so that the ARM_SMMU entry doesn't have this,
> > as that driver _really_ doesn't care about SoC details like this. In other
> > words, add a new entry along the lines of:
> >
> > config ARM_SMMU_QCOM_IMPL
> > default y
> > #if QCOM_SCM=m this can't be =y
> > depends on ARM_SMMU & (QCOM_SCM || !QCOM_SCM)
> >
> > and then have arm-smmu.h provide a static inline qcom_smmu_impl_init()
> > which returns -ENODEV if CONFIG_ARM_SMMU_QCOM_IMPL=n and hack the Makefile
> > so that we don't bother to compile arm-smmu-qcom.o in that case.
> >
> > Would that work?
> 
> I think this proposal still has problems with the directionality of the call.
> 
> The arm-smmu-impl.o calls to arm-smmu-qcom.o which calls qcom_scm.o
> So if qcom_scm.o is part of a module, the calling code in
> arm-smmu-qcom.o also needs to be a module, which means CONFIG_ARM_SMMU
> needs to be a module.
> 
> I know you said the arm-smmu driver doesn't care about SoC details,
> but the trouble is that currently the arm-smmu driver does directly
> call the qcom-scm code. So it is a real dependency. However, if
> QCOM_SCM is not configured, it calls stubs and that's ok.  In that
> way, the "depends on QCOM_SCM || !QCOM_SCM" line actually makes sense.
> It looks terrible because we're used to boolean logic, but it's
> ternary.

Yes, it looks ugly, but the part I really have issues with is that building
QCOM_SCM=m and ARM_SMMU=y is perfectly fine if you don't run on an SoC
with the qcom implementation. I don't see why we need to enforce things
here beyond making sure that all selectable permutations _build_ and
fail gracefully at runtime on the qcom SoC if SCM isn't available.

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


Re: [PATCH v8 07/12] iommu/arm-smmu-v3: Share process page tables

2020-07-13 Thread Will Deacon
On Thu, Jun 18, 2020 at 05:51:20PM +0200, Jean-Philippe Brucker wrote:
> With Shared Virtual Addressing (SVA), we need to mirror CPU TTBR, TCR,
> MAIR and ASIDs in SMMU contexts. Each SMMU has a single ASID space split
> into two sets, shared and private. Shared ASIDs correspond to those
> obtained from the arch ASID allocator, and private ASIDs are used for
> "classic" map/unmap DMA.
> 
> A possible conflict happens when trying to use a shared ASID that has
> already been allocated for private use by the SMMU driver. This will be
> addressed in a later patch by replacing the private ASID. At the
> moment we return -EBUSY.
> 
> Each mm_struct shared with the SMMU will have a single context
> descriptor. Add a refcount to keep track of this. It will be protected
> by the global SVA lock.
> 
> Acked-by: Suzuki K Poulose 
> Signed-off-by: Jean-Philippe Brucker 
> ---
>  drivers/iommu/arm-smmu-v3.c | 150 +++-
>  1 file changed, 146 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 937aa1af428d5..cabd942e4cbf3 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -22,6 +22,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -33,6 +34,8 @@
>  
>  #include 
>  
> +#include "io-pgtable-arm.h"
> +
>  /* MMIO registers */
>  #define ARM_SMMU_IDR00x0
>  #define IDR0_ST_LVL  GENMASK(28, 27)
> @@ -589,6 +592,9 @@ struct arm_smmu_ctx_desc {
>   u64 ttbr;
>   u64 tcr;
>   u64 mair;
> +
> + refcount_t  refs;
> + struct mm_struct*mm;
>  };
>  
>  struct arm_smmu_l1_ctx_desc {
> @@ -727,6 +733,7 @@ struct arm_smmu_option_prop {
>  };
>  
>  static DEFINE_XARRAY_ALLOC1(asid_xa);
> +static DEFINE_MUTEX(sva_lock);
>  
>  static struct arm_smmu_option_prop arm_smmu_options[] = {
>   { ARM_SMMU_OPT_SKIP_PREFETCH, "hisilicon,broken-prefetch-cmd" },
> @@ -1662,7 +1669,8 @@ static int arm_smmu_write_ctx_desc(struct 
> arm_smmu_domain *smmu_domain,
>  #ifdef __BIG_ENDIAN
>   CTXDESC_CD_0_ENDI |
>  #endif
> - CTXDESC_CD_0_R | CTXDESC_CD_0_A | CTXDESC_CD_0_ASET |
> + CTXDESC_CD_0_R | CTXDESC_CD_0_A |
> + (cd->mm ? 0 : CTXDESC_CD_0_ASET) |
>   CTXDESC_CD_0_AA64 |
>   FIELD_PREP(CTXDESC_CD_0_ASID, cd->asid) |
>   CTXDESC_CD_0_V;
> @@ -1766,12 +1774,144 @@ static void arm_smmu_free_cd_tables(struct 
> arm_smmu_domain *smmu_domain)
>   cdcfg->cdtab = NULL;
>  }
>  
> -static void arm_smmu_free_asid(struct arm_smmu_ctx_desc *cd)
> +static void arm_smmu_init_cd(struct arm_smmu_ctx_desc *cd)
>  {
> + refcount_set(>refs, 1);
> +}
> +
> +static bool arm_smmu_free_asid(struct arm_smmu_ctx_desc *cd)
> +{
> + bool free;
> + struct arm_smmu_ctx_desc *old_cd;
> +
>   if (!cd->asid)
> - return;
> + return false;
> +
> + free = refcount_dec_and_test(>refs);
> + if (free) {
> + old_cd = xa_erase(_xa, cd->asid);
> + WARN_ON(old_cd != cd);
> + }
> + return free;
> +}
> +
> +static struct arm_smmu_ctx_desc *arm_smmu_share_asid(u16 asid)
> +{
> + struct arm_smmu_ctx_desc *cd;
>  
> - xa_erase(_xa, cd->asid);
> + cd = xa_load(_xa, asid);
> + if (!cd)
> + return NULL;
> +
> + if (cd->mm) {
> + /* All devices bound to this mm use the same cd struct. */
> + refcount_inc(>refs);
> + return cd;
> + }

How do you handle racing against a concurrent arm_smmu_free_asid() here?

> +__maybe_unused
> +static struct arm_smmu_ctx_desc *arm_smmu_alloc_shared_cd(struct mm_struct 
> *mm)
> +{
> + u16 asid;
> + int ret = 0;
> + u64 tcr, par, reg;
> + struct arm_smmu_ctx_desc *cd;
> + struct arm_smmu_ctx_desc *old_cd = NULL;
> +
> + lockdep_assert_held(_lock);

Please don't bother with these for static functions (but I can see the
value in having them for functions with external callers).

> +
> + asid = mm_context_get(mm);
> + if (!asid)
> + return ERR_PTR(-ESRCH);
> +
> + cd = kzalloc(sizeof(*cd), GFP_KERNEL);
> + if (!cd) {
> + ret = -ENOMEM;
> + goto err_put_context;
> + }
> +
> + arm_smmu_init_cd(cd);
> +
> + old_cd = arm_smmu_share_asid(asid);
> + if (IS_ERR(old_cd)) {
> + ret = PTR_ERR(old_cd);
> + goto err_free_cd;
> + } else if (old_cd) {

Don't need the 'else'

> + if (WARN_ON(old_cd->mm != mm)) {
> + ret = -EINVAL;
> + goto err_free_cd;
> + }
> + kfree(cd);
> + mm_context_put(mm);
> + return old_cd;

This 

Re: [PATCH] iommu/mediatek: Include liunx/dma-mapping.h

2020-07-13 Thread Matthias Brugger




On 13/07/2020 12:16, Joerg Roedel wrote:

From: Joerg Roedel 

This fixes a compile error when cross-compiling the driver
on x86-32.

Signed-off-by: Joerg Roedel 


Reviewed-by: Matthias Brugger 


---
  drivers/iommu/mtk_iommu.h | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h
index 6ff62452bbf9..122925dbe547 100644
--- a/drivers/iommu/mtk_iommu.h
+++ b/drivers/iommu/mtk_iommu.h
@@ -15,6 +15,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  
  #define MTK_LARB_COM_MAX	8



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


Re: [Freedreno] [PATCH] iommu/arm-smmu: Add a init_context_bank implementation hook

2020-07-13 Thread Jordan Crouse
On Mon, Jul 13, 2020 at 08:03:32PM +0100, Will Deacon wrote:
> On Mon, Jul 13, 2020 at 11:00:32AM -0600, Jordan Crouse wrote:
> > On Mon, Jul 13, 2020 at 04:11:23PM +0100, Will Deacon wrote:
> > > On Thu, Jun 11, 2020 at 04:36:56PM -0600, Jordan Crouse wrote:
> > > > Add a new implementation hook to allow the implementation specific code
> > > > to tweek the context bank configuration just before it gets written.
> > > > The first user will be the Adreno GPU implementation to turn on
> > > > SCTLR.HUPCF to ensure that a page fault doesn't terminating pending
> > > > transactions. Doing so could hang the GPU if one of the terminated
> > > > transactions is a CP read.
> > > > 
> > > > This depends on the arm-smmu adreno SMMU implementation [1].
> > > > 
> > > > [1] https://patchwork.kernel.org/patch/11600943/
> > > > 
> > > > Signed-off-by: Jordan Crouse 
> > > > ---
> > > > 
> > > >  drivers/iommu/arm-smmu-qcom.c | 13 +
> > > >  drivers/iommu/arm-smmu.c  | 28 +---
> > > >  drivers/iommu/arm-smmu.h  | 11 +++
> > > >  3 files changed, 37 insertions(+), 15 deletions(-)
> > > 
> > > This looks straightforward enough, but I don't want to merge this without
> > > a user and Sai's series has open questions afaict.
> > 
> > Not sure what you mean by a user in this context?
> > Are you referring to https://patchwork.kernel.org/patch/11628541/?
> 
> Right, this post was just a single patch in isolation, whereas it was
> reposted over at:
> 
> https://lore.kernel.org/r/cdcc6a1c95a84e774790389dc8b3b7f490dc.1593344119.git.saiprakash.ran...@codeaurora.org
> 
> so I'll ignore this one. Sorry, I'm just really struggling to keep track
> of what is targetting 5.9, and I don't have tonnes of time to sift through
> the backlog of duplicate postings :(

Yeah, that is our fault. There are too many cooks in the kitchen.

We need to pick either system cache or split pagetable and serialize
the other on top of it to get the impl code going and then build from there. 
This particular patch can happily hang out in the background until the rest is
resolved.

Jordan

> Will
> ___
> Freedreno mailing list
> freedr...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/freedreno

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH] iommu/arm-smmu: Add a init_context_bank implementation hook

2020-07-13 Thread Will Deacon
On Mon, Jul 13, 2020 at 11:00:32AM -0600, Jordan Crouse wrote:
> On Mon, Jul 13, 2020 at 04:11:23PM +0100, Will Deacon wrote:
> > On Thu, Jun 11, 2020 at 04:36:56PM -0600, Jordan Crouse wrote:
> > > Add a new implementation hook to allow the implementation specific code
> > > to tweek the context bank configuration just before it gets written.
> > > The first user will be the Adreno GPU implementation to turn on
> > > SCTLR.HUPCF to ensure that a page fault doesn't terminating pending
> > > transactions. Doing so could hang the GPU if one of the terminated
> > > transactions is a CP read.
> > > 
> > > This depends on the arm-smmu adreno SMMU implementation [1].
> > > 
> > > [1] https://patchwork.kernel.org/patch/11600943/
> > > 
> > > Signed-off-by: Jordan Crouse 
> > > ---
> > > 
> > >  drivers/iommu/arm-smmu-qcom.c | 13 +
> > >  drivers/iommu/arm-smmu.c  | 28 +---
> > >  drivers/iommu/arm-smmu.h  | 11 +++
> > >  3 files changed, 37 insertions(+), 15 deletions(-)
> > 
> > This looks straightforward enough, but I don't want to merge this without
> > a user and Sai's series has open questions afaict.
> 
> Not sure what you mean by a user in this context?
> Are you referring to https://patchwork.kernel.org/patch/11628541/?

Right, this post was just a single patch in isolation, whereas it was
reposted over at:

https://lore.kernel.org/r/cdcc6a1c95a84e774790389dc8b3b7f490dc.1593344119.git.saiprakash.ran...@codeaurora.org

so I'll ignore this one. Sorry, I'm just really struggling to keep track
of what is targetting 5.9, and I don't have tonnes of time to sift through
the backlog of duplicate postings :(

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


Re: [Freedreno] [PATCH v2 1/6] iommu/arm-smmu: Add auxiliary domain support for arm-smmuv2

2020-07-13 Thread Jordan Crouse
On Tue, Jul 07, 2020 at 08:09:41AM -0700, Rob Clark wrote:
> On Tue, Jul 7, 2020 at 5:34 AM Robin Murphy  wrote:
> >
> > On 2020-06-26 21:04, Jordan Crouse wrote:
> > > Support auxiliary domains for arm-smmu-v2 to initialize and support
> > > multiple pagetables for a single SMMU context bank. Since the smmu-v2
> > > hardware doesn't have any built in support for switching the pagetable
> > > base it is left as an exercise to the caller to actually use the 
> > > pagetable.
> >
> > Hmm, I've still been thinking that we could model this as supporting
> > exactly 1 aux domain iff the device is currently attached to a primary
> > domain with TTBR1 enabled. Then supporting multiple aux domains with
> > magic TTBR0 switching is the Adreno-specific extension on top of that.
> >
> > And if we don't want to go to that length, then - as I think Will was
> > getting at - I'm not sure it's worth bothering at all. There doesn't
> > seem to be any point in half-implementing a pretend aux domain interface
> > while still driving a bus through the rest of the abstractions - it's
> > really the worst of both worlds. If we're going to hand over the guts of
> > io-pgtable to the GPU driver then couldn't it just use
> > DOMAIN_ATTR_PGTABLE_CFG bidirectionally to inject a TTBR0 table straight
> > into the TTBR1-ified domain?
> 
> So, something along the lines of:
> 
> 1) qcom_adreno_smmu_impl somehow tells core arms-smmu that we want
>to use TTBR1 instead of TTBR0
> 
> 2) gpu driver uses iommu_domain_get_attr(PGTABLE_CFG) to snapshot
>the initial pgtable cfg.  (Btw, I kinda feel like we should add
>io_pgtable_fmt to io_pgtable_cfg to make it self contained.)
> 
> 3) gpu driver constructs pgtable_ops for TTBR0, and then kicks
>arm-smmu to do the initial setup to enable TTBR0 with
>iommu_domain_set_attr(PGTABLE_CFG, _pgtable_cfg)


There being no objections, I'm going to start going in this direction.
I think we should have a quirk on the arm-smmu device to allow the PGTABLE_CFG
to be set otherwise there is a chance for abuse. Ideally we would filter this
behavior on a stream ID basis if we come up with a scheme to do that cleanly
based on Will's comments in [1].

[1] https://lists.linuxfoundation.org/pipermail/iommu/2020-July/046488.html

Jordan

> if I understood you properly, that sounds simpler.
> 
> > Much as I like the idea of the aux domain abstraction and making this
> > fit semi-transparently into the IOMMU API, if an almost entirely private
> > interface will be the simplest and cleanest way to get it done then at
> > this point also I'm starting to lean towards just getting it done. But
> > if some other mediated-device type case then turns up that doesn't quite
> > fit that private interface, we revisit the proper abstraction again and
> > I reserve the right to say "I told you so" ;)
> 
> I'm on board with not trying to design this too generically until
> there is a second user
> 
> BR,
> -R
> 
> 
> >
> > Robin.
> >
> > > Aux domains are supported if split pagetable (TTBR1) support has been
> > > enabled on the master domain.  Each auxiliary domain will reuse the
> > > configuration of the master domain. By default the a domain with TTBR1
> > > support will have the TTBR0 region disabled so the first attached aux
> > > domain will enable the TTBR0 region in the hardware and conversely the
> > > last domain to be detached will disable TTBR0 translations.  All 
> > > subsequent
> > > auxiliary domains create a pagetable but not touch the hardware.
> > >
> > > The leaf driver will be able to query the physical address of the
> > > pagetable with the DOMAIN_ATTR_PTBASE attribute so that it can use the
> > > address with whatever means it has to switch the pagetable base.
> > >
> > > Following is a pseudo code example of how a domain can be created
> > >
> > >   /* Check to see if aux domains are supported */
> > >   if (iommu_dev_has_feature(dev, IOMMU_DEV_FEAT_AUX)) {
> > >iommu = iommu_domain_alloc(...);
> > >
> > >if (iommu_aux_attach_device(domain, dev))
> > >return FAIL;
> > >
> > >   /* Save the base address of the pagetable for use by the driver
> > >   iommu_domain_get_attr(domain, DOMAIN_ATTR_PTBASE, );
> > >   }
> > >
> > > Then 'domain' can be used like any other iommu domain to map and
> > > unmap iova addresses in the pagetable.
> > >
> > > Signed-off-by: Jordan Crouse 
> > > ---
> > >
> > >   drivers/iommu/arm-smmu.c | 219 ---
> > >   drivers/iommu/arm-smmu.h |   1 +
> > >   2 files changed, 204 insertions(+), 16 deletions(-)
> > >
> > > diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> > > index 060139452c54..ce6d654301bf 100644
> > > --- a/drivers/iommu/arm-smmu.c
> > > +++ b/drivers/iommu/arm-smmu.c
> > > @@ -91,6 +91,7 @@ struct arm_smmu_cb {
> > >   u32 tcr[2];
> > >   u32 mair[2];
> > >   struct arm_smmu_cfg 

Re: [Freedreno] [PATCH v9 4/7] iommu/arm-smmu: Add a pointer to the attached device to smmu_domain

2020-07-13 Thread Jordan Crouse
On Mon, Jul 13, 2020 at 04:09:02PM +0100, Will Deacon wrote:
> On Fri, Jun 26, 2020 at 02:00:38PM -0600, Jordan Crouse wrote:
> > Add a link to the pointer to the struct device that is attached to a
> > domain. This makes it easy to get the pointer if it is needed in the
> > implementation specific code.
> > 
> > Signed-off-by: Jordan Crouse 
> > ---
> > 
> >  drivers/iommu/arm-smmu.c | 6 --
> >  drivers/iommu/arm-smmu.h | 1 +
> >  2 files changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> > index 048de2681670..060139452c54 100644
> > --- a/drivers/iommu/arm-smmu.c
> > +++ b/drivers/iommu/arm-smmu.c
> > @@ -668,7 +668,8 @@ static void arm_smmu_write_context_bank(struct 
> > arm_smmu_device *smmu, int idx)
> >  }
> >  
> >  static int arm_smmu_init_domain_context(struct iommu_domain *domain,
> > -   struct arm_smmu_device *smmu)
> > +   struct arm_smmu_device *smmu,
> > +   struct device *dev)
> >  {
> > int irq, start, ret = 0;
> > unsigned long ias, oas;
> > @@ -801,6 +802,7 @@ static int arm_smmu_init_domain_context(struct 
> > iommu_domain *domain,
> > cfg->asid = cfg->cbndx;
> >  
> > smmu_domain->smmu = smmu;
> > +   smmu_domain->dev = dev;
> >  
> > pgtbl_cfg = (struct io_pgtable_cfg) {
> > .pgsize_bitmap  = smmu->pgsize_bitmap,
> > @@ -1190,7 +1192,7 @@ static int arm_smmu_attach_dev(struct iommu_domain 
> > *domain, struct device *dev)
> > return ret;
> >  
> > /* Ensure that the domain is finalised */
> > -   ret = arm_smmu_init_domain_context(domain, smmu);
> > +   ret = arm_smmu_init_domain_context(domain, smmu, dev);
> > if (ret < 0)
> > goto rpm_put;
> >  
> > diff --git a/drivers/iommu/arm-smmu.h b/drivers/iommu/arm-smmu.h
> > index 5f2de20e883b..d33cfe26b2f5 100644
> > --- a/drivers/iommu/arm-smmu.h
> > +++ b/drivers/iommu/arm-smmu.h
> > @@ -345,6 +345,7 @@ struct arm_smmu_domain {
> > struct mutexinit_mutex; /* Protects smmu pointer */
> > spinlock_t  cb_lock; /* Serialises ATS1* ops and 
> > TLB syncs */
> > struct iommu_domain domain;
> > +   struct device   *dev;   /* Device attached to this 
> > domain */
> 
> This really doesn't feel right to me -- you can generally have multiple
> devices attached to a domain and they can come and go without the domain
> being destroyed. Perhaps you could instead identify the GPU during
> cfg_probe() and squirrel that information away somewhere?

I need some help here. The SMMU device (qcom,adreno-smmu) will have at least two
stream ids from two different platform devices (GPU and GMU) and I need to
configure split-pagetable and stall/terminate differently on the two domains.

I couldn't figure out a way to identify the platform device before it attached
itself with iommu_attach_device. I tried poking around in fwspec but got lost.

If there is a way we can uniquely identify the devices (by stream id maybe) then
we could use that though I have reservations about hard coding stream IDs in the
impl driver. That said, the stream IDs have never changed in the life of the
GPU so maybe it's not a problem that needs solving.

Jordan

> The rest of the series looks ok to me.
> 
> Will
> ___
> Freedreno mailing list
> freedr...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/freedreno

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH] iommu/arm-smmu: Add a init_context_bank implementation hook

2020-07-13 Thread Jordan Crouse
On Mon, Jul 13, 2020 at 04:11:23PM +0100, Will Deacon wrote:
> On Thu, Jun 11, 2020 at 04:36:56PM -0600, Jordan Crouse wrote:
> > Add a new implementation hook to allow the implementation specific code
> > to tweek the context bank configuration just before it gets written.
> > The first user will be the Adreno GPU implementation to turn on
> > SCTLR.HUPCF to ensure that a page fault doesn't terminating pending
> > transactions. Doing so could hang the GPU if one of the terminated
> > transactions is a CP read.
> > 
> > This depends on the arm-smmu adreno SMMU implementation [1].
> > 
> > [1] https://patchwork.kernel.org/patch/11600943/
> > 
> > Signed-off-by: Jordan Crouse 
> > ---
> > 
> >  drivers/iommu/arm-smmu-qcom.c | 13 +
> >  drivers/iommu/arm-smmu.c  | 28 +---
> >  drivers/iommu/arm-smmu.h  | 11 +++
> >  3 files changed, 37 insertions(+), 15 deletions(-)
> 
> This looks straightforward enough, but I don't want to merge this without
> a user and Sai's series has open questions afaict.

Not sure what you mean by a user in this context?
Are you referring to https://patchwork.kernel.org/patch/11628541/?

> Will

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v8 04/12] arm64: mm: Pin down ASIDs for sharing mm with devices

2020-07-13 Thread Will Deacon
On Thu, Jun 18, 2020 at 05:51:17PM +0200, Jean-Philippe Brucker wrote:
> To enable address space sharing with the IOMMU, introduce mm_context_get()
> and mm_context_put(), that pin down a context and ensure that it will keep
> its ASID after a rollover. Export the symbols to let the modular SMMUv3
> driver use them.
> 
> Pinning is necessary because a device constantly needs a valid ASID,
> unlike tasks that only require one when running. Without pinning, we would
> need to notify the IOMMU when we're about to use a new ASID for a task,
> and it would get complicated when a new task is assigned a shared ASID.
> Consider the following scenario with no ASID pinned:
> 
> 1. Task t1 is running on CPUx with shared ASID (gen=1, asid=1)
> 2. Task t2 is scheduled on CPUx, gets ASID (1, 2)
> 3. Task tn is scheduled on CPUy, a rollover occurs, tn gets ASID (2, 1)
>We would now have to immediately generate a new ASID for t1, notify
>the IOMMU, and finally enable task tn. We are holding the lock during
>all that time, since we can't afford having another CPU trigger a
>rollover. The IOMMU issues invalidation commands that can take tens of
>milliseconds.
> 
> It gets needlessly complicated. All we wanted to do was schedule task tn,
> that has no business with the IOMMU. By letting the IOMMU pin tasks when
> needed, we avoid stalling the slow path, and let the pinning fail when
> we're out of shareable ASIDs.
> 
> After a rollover, the allocator expects at least one ASID to be available
> in addition to the reserved ones (one per CPU). So (NR_ASIDS - NR_CPUS -
> 1) is the maximum number of ASIDs that can be shared with the IOMMU.
> 
> Signed-off-by: Jean-Philippe Brucker 
> ---
>  arch/arm64/include/asm/mmu.h |  1 +
>  arch/arm64/include/asm/mmu_context.h | 11 +++-
>  arch/arm64/mm/context.c  | 95 +++-
>  3 files changed, 104 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
> index 68140fdd89d6b..bbdd291e31d59 100644
> --- a/arch/arm64/include/asm/mmu.h
> +++ b/arch/arm64/include/asm/mmu.h
> @@ -19,6 +19,7 @@
>  
>  typedef struct {
>   atomic64_t  id;
> + unsigned long   pinned;

bool?

>   void*vdso;
>   unsigned long   flags;
>  } mm_context_t;
> diff --git a/arch/arm64/include/asm/mmu_context.h 
> b/arch/arm64/include/asm/mmu_context.h
> index b0bd9b55594c5..7b0e0f6cb7e87 100644
> --- a/arch/arm64/include/asm/mmu_context.h
> +++ b/arch/arm64/include/asm/mmu_context.h
> @@ -177,7 +177,13 @@ static inline void cpu_replace_ttbr1(pgd_t *pgdp)
>  #define destroy_context(mm)  do { } while(0)
>  void check_and_switch_context(struct mm_struct *mm, unsigned int cpu);
>  
> -#define init_new_context(tsk,mm) ({ atomic64_set(&(mm)->context.id, 0); 
> 0; })
> +static inline int
> +init_new_context(struct task_struct *tsk, struct mm_struct *mm)
> +{
> + atomic64_set(>context.id, 0);
> + mm->context.pinned = 0;
> + return 0;
> +}
>  
>  #ifdef CONFIG_ARM64_SW_TTBR0_PAN
>  static inline void update_saved_ttbr0(struct task_struct *tsk,
> @@ -250,6 +256,9 @@ switch_mm(struct mm_struct *prev, struct mm_struct *next,
>  void verify_cpu_asid_bits(void);
>  void post_ttbr_update_workaround(void);
>  
> +unsigned long mm_context_get(struct mm_struct *mm);
> +void mm_context_put(struct mm_struct *mm);
> +
>  #endif /* !__ASSEMBLY__ */
>  
>  #endif /* !__ASM_MMU_CONTEXT_H */
> diff --git a/arch/arm64/mm/context.c b/arch/arm64/mm/context.c
> index d702d60e64dab..d0ddd413f5645 100644
> --- a/arch/arm64/mm/context.c
> +++ b/arch/arm64/mm/context.c
> @@ -27,6 +27,10 @@ static DEFINE_PER_CPU(atomic64_t, active_asids);
>  static DEFINE_PER_CPU(u64, reserved_asids);
>  static cpumask_t tlb_flush_pending;
>  
> +static unsigned long max_pinned_asids;
> +static unsigned long nr_pinned_asids;
> +static unsigned long *pinned_asid_map;
> +
>  #define ASID_MASK(~GENMASK(asid_bits - 1, 0))
>  #define ASID_FIRST_VERSION   (1UL << asid_bits)
>  
> @@ -74,6 +78,9 @@ void verify_cpu_asid_bits(void)
>  
>  static void set_kpti_asid_bits(void)
>  {
> + unsigned int k;
> + u8 *dst = (u8 *)asid_map;
> + u8 *src = (u8 *)pinned_asid_map;
>   unsigned int len = BITS_TO_LONGS(NUM_USER_ASIDS) * sizeof(unsigned 
> long);
>   /*
>* In case of KPTI kernel/user ASIDs are allocated in
> @@ -81,7 +88,8 @@ static void set_kpti_asid_bits(void)
>* is set, then the ASID will map only userspace. Thus
>* mark even as reserved for kernel.
>*/
> - memset(asid_map, 0xaa, len);
> + for (k = 0; k < len; k++)
> + dst[k] = src[k] | 0xaa;

Can you use __bitmap_replace() here? I think it would be clearer to use the
bitmap API wherever possible, since casting 'unsigned long *' to 'u8 *'
just makes me worry about endianness issues (although in this case I don't
hink it's a problem).

>  }
>  
>  static void 

[git pull] IOMMU Fixes for Linux v5.8-rc5

2020-07-13 Thread Joerg Roedel
Hi Linus,

The following changes since commit 9ebcfadb0610322ac537dd7aa5d9cbc2b2894c68:

  Linux 5.8-rc3 (2020-06-28 15:00:24 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git 
tags/iommu-fixes-v5.8-rc5

for you to fetch changes up to a082121b55bac125f7d09d78de00607ea75a6903:

  iommu/arm-smmu: Mark qcom_smmu_client_of_match as possibly unused (2020-07-09 
17:20:22 +0200)


IOMMU Fixes for Linux v5.8-rc5:

Including:

- Fix for a use-after-free of the device iommu-group. Found in the
  arm-smmu driver, but the fix is in generic code.

- Fix for the new Allwinner IOMMU driver to use the atomic
  readl_timeout() variant in IO/TLB flushing code.

- A couple of cleanups to fix various compile warnings.


Geert Uytterhoeven (1):
  iommu: SUN50I_IOMMU should depend on HAS_DMA

Joerg Roedel (1):
  iommu/amd: Make amd_iommu_apply_ivrs_quirks() static inline

Jordan Crouse (1):
  iommu/arm-smmu: Mark qcom_smmu_client_of_match as possibly unused

Maxime Ripard (2):
  iommu/sun50i: Change the readl timeout to the atomic variant
  iommu/sun50i: Remove unused variable

Qian Cai (1):
  iommu: Fix use-after-free in iommu_release_device

 drivers/iommu/Kconfig | 1 +
 drivers/iommu/amd/amd_iommu.h | 2 +-
 drivers/iommu/arm-smmu-qcom.c | 2 +-
 drivers/iommu/iommu.c | 2 +-
 drivers/iommu/sun50i-iommu.c  | 8 +++-
 5 files changed, 7 insertions(+), 8 deletions(-)

Please pull.

Thanks,

Joerg


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

Re: [PATCH v2] iommu/arm-smmu: Mark qcom_smmu_client_of_match as possibly unused

2020-07-13 Thread Joerg Roedel
On Mon, Jul 13, 2020 at 02:33:26PM +0100, Will Deacon wrote:
> I can't see this in Joerg's tree or in linux-next. Joerg: did you pick this
> one up? (I thought you did, but I can't find it!).

Yes, its in the tree and and will be pushed soon. I'll also send it to
Linus today.


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


Re: [PATCH] iommu/arm-smmu: Add a init_context_bank implementation hook

2020-07-13 Thread Will Deacon
On Thu, Jun 11, 2020 at 04:36:56PM -0600, Jordan Crouse wrote:
> Add a new implementation hook to allow the implementation specific code
> to tweek the context bank configuration just before it gets written.
> The first user will be the Adreno GPU implementation to turn on
> SCTLR.HUPCF to ensure that a page fault doesn't terminating pending
> transactions. Doing so could hang the GPU if one of the terminated
> transactions is a CP read.
> 
> This depends on the arm-smmu adreno SMMU implementation [1].
> 
> [1] https://patchwork.kernel.org/patch/11600943/
> 
> Signed-off-by: Jordan Crouse 
> ---
> 
>  drivers/iommu/arm-smmu-qcom.c | 13 +
>  drivers/iommu/arm-smmu.c  | 28 +---
>  drivers/iommu/arm-smmu.h  | 11 +++
>  3 files changed, 37 insertions(+), 15 deletions(-)

This looks straightforward enough, but I don't want to merge this without
a user and Sai's series has open questions afaict.

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


Re: [PATCH v9 4/7] iommu/arm-smmu: Add a pointer to the attached device to smmu_domain

2020-07-13 Thread Will Deacon
On Fri, Jun 26, 2020 at 02:00:38PM -0600, Jordan Crouse wrote:
> Add a link to the pointer to the struct device that is attached to a
> domain. This makes it easy to get the pointer if it is needed in the
> implementation specific code.
> 
> Signed-off-by: Jordan Crouse 
> ---
> 
>  drivers/iommu/arm-smmu.c | 6 --
>  drivers/iommu/arm-smmu.h | 1 +
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 048de2681670..060139452c54 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -668,7 +668,8 @@ static void arm_smmu_write_context_bank(struct 
> arm_smmu_device *smmu, int idx)
>  }
>  
>  static int arm_smmu_init_domain_context(struct iommu_domain *domain,
> - struct arm_smmu_device *smmu)
> + struct arm_smmu_device *smmu,
> + struct device *dev)
>  {
>   int irq, start, ret = 0;
>   unsigned long ias, oas;
> @@ -801,6 +802,7 @@ static int arm_smmu_init_domain_context(struct 
> iommu_domain *domain,
>   cfg->asid = cfg->cbndx;
>  
>   smmu_domain->smmu = smmu;
> + smmu_domain->dev = dev;
>  
>   pgtbl_cfg = (struct io_pgtable_cfg) {
>   .pgsize_bitmap  = smmu->pgsize_bitmap,
> @@ -1190,7 +1192,7 @@ static int arm_smmu_attach_dev(struct iommu_domain 
> *domain, struct device *dev)
>   return ret;
>  
>   /* Ensure that the domain is finalised */
> - ret = arm_smmu_init_domain_context(domain, smmu);
> + ret = arm_smmu_init_domain_context(domain, smmu, dev);
>   if (ret < 0)
>   goto rpm_put;
>  
> diff --git a/drivers/iommu/arm-smmu.h b/drivers/iommu/arm-smmu.h
> index 5f2de20e883b..d33cfe26b2f5 100644
> --- a/drivers/iommu/arm-smmu.h
> +++ b/drivers/iommu/arm-smmu.h
> @@ -345,6 +345,7 @@ struct arm_smmu_domain {
>   struct mutexinit_mutex; /* Protects smmu pointer */
>   spinlock_t  cb_lock; /* Serialises ATS1* ops and 
> TLB syncs */
>   struct iommu_domain domain;
> + struct device   *dev;   /* Device attached to this 
> domain */

This really doesn't feel right to me -- you can generally have multiple
devices attached to a domain and they can come and go without the domain
being destroyed. Perhaps you could instead identify the GPU during
cfg_probe() and squirrel that information away somewhere?

The rest of the series looks ok to me.

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


[PATCH -next] iommu: Make some functions static

2020-07-13 Thread Wei Yongjun
The sparse tool complains as follows:

drivers/iommu/iommu.c:386:5: warning:
 symbol 'iommu_insert_resv_region' was not declared. Should it be static?
drivers/iommu/iommu.c:2182:5: warning:
 symbol '__iommu_map' was not declared. Should it be static?

Those functions are not used outside of iommu.c, so mark them static.

Reported-by: Hulk Robot 
Signed-off-by: Wei Yongjun 
---
 drivers/iommu/iommu.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 1ed1e14a1f0c..40947f5bc6c5 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -383,8 +383,8 @@ static ssize_t iommu_group_show_name(struct iommu_group 
*group, char *buf)
  * Elements are sorted by start address and overlapping segments
  * of the same type are merged.
  */
-int iommu_insert_resv_region(struct iommu_resv_region *new,
-struct list_head *regions)
+static int iommu_insert_resv_region(struct iommu_resv_region *new,
+   struct list_head *regions)
 {
struct iommu_resv_region *iter, *tmp, *nr, *top;
LIST_HEAD(stack);
@@ -2179,8 +2179,8 @@ static size_t iommu_pgsize(struct iommu_domain *domain,
return pgsize;
 }
 
-int __iommu_map(struct iommu_domain *domain, unsigned long iova,
- phys_addr_t paddr, size_t size, int prot, gfp_t gfp)
+static int __iommu_map(struct iommu_domain *domain, unsigned long iova,
+  phys_addr_t paddr, size_t size, int prot, gfp_t gfp)
 {
const struct iommu_ops *ops = domain->ops;
unsigned long orig_iova = iova;

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


Re: [PATCH v10 4/5] dt-bindings: arm-smmu: add binding for Tegra194 SMMU

2020-07-13 Thread Robin Murphy

On 2020-07-10 21:29, Krishna Reddy wrote:

Thanks Rob. One question on setting "minItems: ". Please see below.


+allOf:
+  - if:
+  properties:
+compatible:
+  contains:
+enum:
+  - nvidia,tegra194-smmu
+then:
+  properties:
+reg:
+  minItems: 2
+  maxItems: 2



This doesn't work. The main part of the schema already said there's only
1 reg region. This part is ANDed with that, not an override. You need to add an 
else clause with 'maxItems: 1' and change the base schema to
{minItems: 1, maxItems: 2}.


As the earlier version of base schema doesn't have "minItems: " set, should it 
be set to 0 for backward compatibility?  Or can it just be omitted setting in base schema 
as before?


We've always needed at least 1 "reg" specifier in practice, so I don't 
think being backwards-compatible with broken DTs is a concern :)


Robin.



"else" part to set "maxItems: 1" and setting "maxItems: 2" in base schema is 
clear to me.


-KR


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


Re: [PATCH v10 2/5] iommu/arm-smmu: ioremap smmu mmio region before implementation init

2020-07-13 Thread Robin Murphy

On 2020-07-08 06:00, Krishna Reddy wrote:

ioremap smmu mmio region before calling into implementation init.
This is necessary to allow mapped address available during vendor
specific implementation init.


Reviewed-by: Robin Murphy 


Signed-off-by: Krishna Reddy 
---
  drivers/iommu/arm-smmu.c | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index d2054178df35..e03e873d3bca 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -2120,10 +2120,6 @@ static int arm_smmu_device_probe(struct platform_device 
*pdev)
if (err)
return err;
  
-	smmu = arm_smmu_impl_init(smmu);

-   if (IS_ERR(smmu))
-   return PTR_ERR(smmu);
-
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
ioaddr = res->start;
smmu->base = devm_ioremap_resource(dev, res);
@@ -2135,6 +2131,10 @@ static int arm_smmu_device_probe(struct platform_device 
*pdev)
 */
smmu->numpage = resource_size(res);
  
+	smmu = arm_smmu_impl_init(smmu);

+   if (IS_ERR(smmu))
+   return PTR_ERR(smmu);
+
num_irqs = 0;
while ((res = platform_get_resource(pdev, IORESOURCE_IRQ, num_irqs))) {
num_irqs++;


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


Re: [PATCH v10 0/5] NVIDIA ARM SMMU Implementation

2020-07-13 Thread Will Deacon
On Tue, Jul 07, 2020 at 10:00:12PM -0700, Krishna Reddy wrote:
> Changes in v10:
> Perform SMMU base ioremap before calling implementation init.
> Check for Global faults across both ARM MMU-500s during global interrupt.
> Check for context faults across all contexts of both ARM MMU-500s during 
> context fault interrupt.
> Add new DT binding nvidia,smmu-500 for NVIDIA implementation.

Please repost based on my SMMU queue, as this doesn't currently apply.

https://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git/log/?h=for-joerg/arm-smmu/updates

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


Re: [PATCH v10 5/5] iommu/arm-smmu: Add global/context fault implementation hooks

2020-07-13 Thread Will Deacon
On Tue, Jul 07, 2020 at 10:00:17PM -0700, Krishna Reddy wrote:
> Add global/context fault hooks to allow vendor specific implementations
> override default fault interrupt handlers.
> 
> Update NVIDIA implementation to override the default global/context fault
> interrupt handlers and handle interrupts across the two ARM MMU-500s that
> are programmed identically.
> 
> Signed-off-by: Krishna Reddy 
> ---
>  drivers/iommu/arm-smmu-nvidia.c | 99 +
>  drivers/iommu/arm-smmu.c| 17 +-
>  drivers/iommu/arm-smmu.h|  3 +
>  3 files changed, 117 insertions(+), 2 deletions(-)

Given that faults shouldn't occur during normal operation, is this patch
actually necessary?

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


Re: [PATCH v2] iommu/arm-smmu: Mark qcom_smmu_client_of_match as possibly unused

2020-07-13 Thread Will Deacon
On Mon, Jun 08, 2020 at 04:13:08PM +0100, Will Deacon wrote:
> On Thu, Jun 04, 2020 at 02:39:04PM -0600, Jordan Crouse wrote:
> > When CONFIG_OF=n of_match_device() gets pre-processed out of existence
> > leaving qcom-smmu_client_of_match unused. Mark it as possibly unused to
> > keep the compiler from warning in that case.
> > 
> > Fixes: 0e764a01015d ("iommu/arm-smmu: Allow client devices to select direct 
> > mapping")
> > Reported-by: kbuild test robot 
> > Acked-by: Will Deacon 
> > Signed-off-by: Jordan Crouse 
> > ---
> > 
> >  drivers/iommu/arm-smmu-qcom.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/iommu/arm-smmu-qcom.c b/drivers/iommu/arm-smmu-qcom.c
> > index cf01d0215a39..be4318044f96 100644
> > --- a/drivers/iommu/arm-smmu-qcom.c
> > +++ b/drivers/iommu/arm-smmu-qcom.c
> > @@ -12,7 +12,7 @@ struct qcom_smmu {
> > struct arm_smmu_device smmu;
> >  };
> >  
> > -static const struct of_device_id qcom_smmu_client_of_match[] = {
> > +static const struct of_device_id qcom_smmu_client_of_match[] 
> > __maybe_unused = {
> > { .compatible = "qcom,adreno" },
> > { .compatible = "qcom,mdp4" },
> > { .compatible = "qcom,mdss" },
> 
> Thanks. Joerg -- can you pick this one up, please? I don't have any other
> SMMU fixes pending at the moment.

I can't see this in Joerg's tree or in linux-next. Joerg: did you pick this
one up? (I thought you did, but I can't find it!).

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


Re: [PATCH v6 00/10] MT6779 IOMMU SUPPORT

2020-07-13 Thread Joerg Roedel
On Sat, Jul 11, 2020 at 03:11:33PM +0800, Yong Wu wrote:
> The SMI part always go with the IOMMU, Could you also help apply the
> mt6779 SMI basical part [1][2]. Both has already got reviewed-by from
> Rob and Matthias. and the [3] in that patchset is for performance
> improvement, it's not so necessary, it can be send in another patchset.
> 
> 
> [1] https://lore.kernel.org/patchwork/patch/1176833/
> [2] https://lore.kernel.org/patchwork/patch/1176831/

Okay, applied these two to the arm/mediatek branch.


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


Re: [PATCH v5 03/15] iommu/smmu: Report empty domain nesting info

2020-07-13 Thread Will Deacon
On Sun, Jul 12, 2020 at 04:20:58AM -0700, Liu Yi L wrote:
> This patch is added as instead of returning a boolean for DOMAIN_ATTR_NESTING,
> iommu_domain_get_attr() should return an iommu_nesting_info handle.
> 
> Cc: Will Deacon 
> Cc: Robin Murphy 
> Cc: Eric Auger 
> Cc: Jean-Philippe Brucker 
> Suggested-by: Jean-Philippe Brucker 
> Signed-off-by: Liu Yi L 
> Signed-off-by: Jacob Pan 
> ---
> v4 -> v5:
> *) address comments from Eric Auger.
> ---
>  drivers/iommu/arm-smmu-v3.c | 29 +++--
>  drivers/iommu/arm-smmu.c| 29 +++--
>  2 files changed, 54 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index f578677..ec815d7 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -3019,6 +3019,32 @@ static struct iommu_group 
> *arm_smmu_device_group(struct device *dev)
>   return group;
>  }
>  
> +static int arm_smmu_domain_nesting_info(struct arm_smmu_domain *smmu_domain,
> + void *data)
> +{
> + struct iommu_nesting_info *info = (struct iommu_nesting_info *)data;
> + unsigned int size;
> +
> + if (!info || smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
> + return -ENODEV;
> +
> + size = sizeof(struct iommu_nesting_info);
> +
> + /*
> +  * if provided buffer size is smaller than expected, should
> +  * return 0 and also the expected buffer size to caller.
> +  */
> + if (info->size < size) {
> + info->size = size;
> + return 0;
> + }
> +
> + /* report an empty iommu_nesting_info for now */
> + memset(info, 0x0, size);
> + info->size = size;
> + return 0;
> +}

Have you verified that this doesn't break the existing usage of
DOMAIN_ATTR_NESTING in drivers/vfio/vfio_iommu_type1.c?

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


Re: [PATCH 2/2] iommu/dma: Avoid SAC address trick for PCIe devices

2020-07-13 Thread Joerg Roedel
On Wed, Jul 08, 2020 at 12:32:42PM +0100, Robin Murphy wrote:
> As for the intel-iommu implementation, relegate the opportunistic
> attempt to allocate a SAC address to the domain of conventional PCI
> devices only, to avoid it increasingly causing far more performance
> issues than possible benefits on modern PCI Express systems.
> 
> Signed-off-by: Robin Murphy 
> ---
>  drivers/iommu/dma-iommu.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index 4959f5df21bd..0ff124f16ad4 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -426,7 +426,8 @@ static dma_addr_t iommu_dma_alloc_iova(struct 
> iommu_domain *domain,
>   dma_limit = min(dma_limit, (u64)domain->geometry.aperture_end);
>  
>   /* Try to get PCI devices a SAC address */
> - if (dma_limit > DMA_BIT_MASK(32) && dev_is_pci(dev))
> + if (dma_limit > DMA_BIT_MASK(32) &&
> + dev_is_pci(dev) && !pci_is_pcie(to_pci_dev(dev)))
>   iova = alloc_iova_fast(iovad, iova_len,
>  DMA_BIT_MASK(32) >> shift, false);
>  

Unfortunatly this patch causes XHCI initialization failures on my AMD
Ryzen system. I will remove both from the IOMMU tree for now.

I guess the XHCI chip in my system does not support full 64bit dma
addresses and needs a quirk or something like that. But until this is
resolved its better to not change the IOVA allocation behavior.

Regards,

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


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

2020-07-13 Thread Robin Murphy

On 2020-07-13 10:12, Claire Chang wrote:

The bounced DMA ops provide an implementation of DMA ops that bounce
streaming DMA in and out of a specially allocated region. Only the
operations relevant to streaming DMA are supported.


I think there are too many implicit assumptions here - apparently that 
coherent allocations will always be intercepted by 
dma_*_from_dev_coherent(), and that calling into dma-direct won't 
actually bounce things a second time beyond where you thought they were 
going, manage coherency for a different address, and make it all go 
subtly wrong. Consider "swiotlb=force", for instance...


Again, plumbing this straight into dma-direct so that SWIOTLB can simply 
target a different buffer and always bounce regardless of masks would 
seem a far better option.


Robin.


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

diff --git a/include/linux/device.h b/include/linux/device.h
index 7322c51e9c0c..868b9a364003 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -588,6 +588,9 @@ struct device {
  
  	struct list_head	dma_pools;	/* dma pools (if dma'ble) */
  
+#ifdef CONFIG_DMA_BOUNCED

+   struct dma_bounced_mem  *dma_bounced_mem;
+#endif
  #ifdef CONFIG_DMA_DECLARE_COHERENT
struct dma_coherent_mem *dma_mem; /* internal for coherent mem
 override */
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 2328f451a45d..86089424dafd 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -135,6 +135,7 @@ struct dma_map_ops {
  
  extern const struct dma_map_ops dma_virt_ops;

  extern const struct dma_map_ops dma_dummy_ops;
+extern const struct dma_map_ops dma_bounced_ops;
  
  #define DMA_BIT_MASK(n)	(((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
  
diff --git a/kernel/dma/Kconfig b/kernel/dma/Kconfig

index 1da3f44f2565..148734c8748b 100644
--- a/kernel/dma/Kconfig
+++ b/kernel/dma/Kconfig
@@ -88,6 +88,23 @@ config DMA_DIRECT_REMAP
select DMA_REMAP
select DMA_COHERENT_POOL
  
+config DMA_BOUNCED

+   bool "DMA Bounced"
+   depends on !HIGHMEM
+   select OF_RESERVED_MEM
+   help
+ This enables support for bounced DMA pools which provide a level of
+ DMA memory protection on systems with limited hardware protection
+ capabilities, such as those lacking an IOMMU. It does so by bouncing
+ the data to a specially allocated DMA-accessible protected region
+ before mapping and unmapping. One can assign the protected memory
+ region in the device tree by using reserved-memory.
+
+ For more information see
+ 

+ and .
+ If unsure, say "n".
+
  config DMA_CMA
bool "DMA Contiguous Memory Allocator"
depends on HAVE_DMA_CONTIGUOUS && CMA
diff --git a/kernel/dma/Makefile b/kernel/dma/Makefile
index 370f63344e9c..f5fb4f42326a 100644
--- a/kernel/dma/Makefile
+++ b/kernel/dma/Makefile
@@ -1,6 +1,7 @@
  # SPDX-License-Identifier: GPL-2.0
  
  obj-$(CONFIG_HAS_DMA)			+= mapping.o direct.o dummy.o

+obj-$(CONFIG_DMA_BOUNCED)  += bounced.o
  obj-$(CONFIG_DMA_CMA) += contiguous.o
  obj-$(CONFIG_DMA_DECLARE_COHERENT)+= coherent.o
  obj-$(CONFIG_DMA_VIRT_OPS)+= virt.o
diff --git a/kernel/dma/bounced.c b/kernel/dma/bounced.c
new file mode 100644
index ..fcaabb5eccf2
--- /dev/null
+++ b/kernel/dma/bounced.c
@@ -0,0 +1,215 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Bounced DMA support.
+ *
+ * This implements the mitigations for lack of IOMMU by bouncing the data to a
+ * specially allocated region before mapping and unmapping.
+ *
+ * Copyright 2020 Google LLC.
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct dma_bounced_mem {
+   void**orig_addr;
+   void*virt_base;
+   dma_addr_t  device_base;
+   dma_addr_t  device_end;
+   struct gen_pool *pool;
+   size_t  size;
+};
+
+static void dma_bounced_set_orig_virt(struct device *dev, dma_addr_t dma_addr,
+ void *orig_addr)
+{
+   struct dma_bounced_mem *mem = dev->dma_bounced_mem;
+   int idx = (dma_addr - mem->device_base) >> PAGE_SHIFT;
+
+   if (dma_addr < mem->device_base || dma_addr >= mem->device_end)
+   return;
+
+   mem->orig_addr[idx] = orig_addr;
+}
+
+static void *dma_bounced_get_orig_virt(struct device *dev, dma_addr_t dma_addr)
+{
+   struct dma_bounced_mem *mem = dev->dma_bounced_mem;
+   int idx = (dma_addr - mem->device_base) >> PAGE_SHIFT;
+
+   if (dma_addr < mem->device_base || dma_addr >= 

Re: [PATCH 0/4] Bounced DMA support

2020-07-13 Thread Robin Murphy

On 2020-07-13 10:12, Claire Chang wrote:

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

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

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


More to the point, this seems to need some fairly special interconnect 
hardware too. On typical systems that just stick a TZASC directly in 
front of the memory controller it would be hard to block DMA access 
without also blocking CPU access. With something like Arm TZC-400 I 
guess you could set up a "secure" region for most of DRAM that allows NS 
accesses by NSAID from the CPUs, then similar regions for the pools with 
NSAID access for both the respective device and the CPUs, but by that 
point you've probably used up most of the available regions before even 
considering what the firmware and TEE might want for actual Secure memory.


In short, I don't foresee this being used by very many systems.

That said,, although the motivation is different, it appears to end up 
being almost exactly the same end result as the POWER secure 
virtualisation thingy (essentially: constrain DMA to a specific portion 
of RAM). The more code can be shared with that, the better.



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


Note that we do actually have the notion of bounced DMA with IOMMUs 
already (to avoid leakage of unrelated data in the same page). I think 
it's only implemented for intel-iommu so far, but shouldn't take much 
work to generalise to iommu-dma if anyone wanted to. That's already done 
a bunch of work to generalise the SWIOTLB routines to be more reusable, 
so building on top of that would be highly preferable.


Thirdly, the concept of device-private bounce buffers does in fact 
already exist to some degree too - there are various USB, crypto and 
other devices that can only DMA to a local SRAM buffer (not to mention 
subsystems doing their own bouncing for the sake of alignment/block 
merging/etc.). Again, a slightly more generalised solution that makes 
this a first-class notion for dma-direct itself and could help supplant 
some of those hacks would be really really good.


Robin.


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


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

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


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


[PATCH] iommu/mediatek: Include liunx/dma-mapping.h

2020-07-13 Thread Joerg Roedel
From: Joerg Roedel 

This fixes a compile error when cross-compiling the driver
on x86-32.

Signed-off-by: Joerg Roedel 
---
 drivers/iommu/mtk_iommu.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h
index 6ff62452bbf9..122925dbe547 100644
--- a/drivers/iommu/mtk_iommu.h
+++ b/drivers/iommu/mtk_iommu.h
@@ -15,6 +15,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #define MTK_LARB_COM_MAX   8
-- 
2.16.3

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


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

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

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

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

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


[PATCH 0/4] Bounced DMA support

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

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

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

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

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


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

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

-- 
2.27.0.383.g050319c2ae-goog

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


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

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

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

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

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

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

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

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

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

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


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

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

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

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

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

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


Re: [PATCH v4 4/4] PCI/ACS: Enable PCI_ACS_TB for untrusted/external-facing devices

2020-07-13 Thread Joerg Roedel
On Sat, Jul 11, 2020 at 09:58:38PM -0500, Bjorn Helgaas wrote:
> If BIOS handed off with ATS enabled and we somehow relied on it being
> already enabled, something might break if we start disabling ATS.
> Just a theoretical possibility, doesn't seem likely to me.

I don't think this will be a problem. When the BIOS enables ATS for a
device it also needs to enable the IOMMU already, an we are not handling
an already enabled IOMMU (outside of kdump kernels) very well.


Regards,

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


Re: [PATCH 12/21] iommu/mediatek: Add iova reserved function

2020-07-13 Thread Pi-Hsun Shih
On Sat, Jul 11, 2020 at 2:51 PM Yong Wu  wrote:
>
> For multiple iommu_domains, we need to reserve some iova regions, so we
> will add mtk_iommu_iova_region structure. It includes the base address
> and size of the range.
> This is a preparing patch for supporting multi-domain.
>
> Signed-off-by: Anan sun
> Signed-off-by: Hao Chao
> Signed-off-by: Yong Wu 
> ---
>  drivers/iommu/mtk_iommu.c | 37 +
>  drivers/iommu/mtk_iommu.h |  5 +
>  2 files changed, 42 insertions(+)
>
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index 03a6d66f4bef..fdfdb75706e0 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -151,6 +151,11 @@ static LIST_HEAD(m4ulist); /* List all the M4U HWs */
> ...
> +
> +static void mtk_iommu_put_resv_regions(struct device *dev,
> +  struct list_head *head)
> +{
> +   struct iommu_resv_region *entry, *next;
> +
> +   list_for_each_entry_safe(entry, next, head, list)
> +   kfree(entry);
> +}
> +

This is the same as generic_iommu_put_resv_regions, use that as the
.put_resv_regions callback instead?

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


Re: [PATCH 11/21] iommu/mediatek: Add power-domain operation

2020-07-13 Thread Pi-Hsun Shih
On Sat, Jul 11, 2020 at 2:51 PM Yong Wu  wrote:
>
> In the previous SoC, the M4U HW is in the EMI power domain which is
> always on. the latest M4U is in the display power domain which may be
> turned on/off, thus we have to add pm_runtime interface for it.
>
> we should enable its power before M4U hw initial. and disable it after HW
> initialize.
>
> When the engine work, the engine always enable the power and clocks for
> smi-larb/smi-common, then the M4U's power will always be powered on
> automatically via the device link with smi-common.
>
> Note: we don't enable the M4U power in iommu_map/unmap for tlb flush.
> If its power already is on, of course it is ok. if the power is off,
> the main tlb will be reset while M4U power on, thus the tlb flush while
> m4u power off is unnecessary, just skip it.
>
> Signed-off-by: Yong Wu 
> ---
>  drivers/iommu/mtk_iommu.c | 54 ++-
>  1 file changed, 47 insertions(+), 7 deletions(-)
> ...
> for_each_m4u(data) {
> +   /* skip tlb flush when pm is not active */
> +   if (pm_runtime_enabled(data->dev) &&
> +   !pm_runtime_active(data->dev))
> +   continue;
> +

pm_runtime_active(dev) == false implies dev->power.disable_depth == 0,
which implies pm_runtime_enabled(dev) == true, so the
pm_runtime_enabled(data->dev) can be omitted here.

> spin_lock_irqsave(>tlb_lock, flags);
> writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0,
>data->base + data->plat_data->inv_sel_reg);
> ...
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 04/21] dt-binding: mediatek: Add binding for mt8192 IOMMU and SMI

2020-07-13 Thread Yong Wu
On Mon, 2020-07-13 at 13:36 +0800, Pi-Hsun Shih wrote:
> On Sat, Jul 11, 2020 at 2:50 PM Yong Wu  wrote:
> >
> > This patch adds decriptions for mt8192 IOMMU and SMI.
> >
> > mt8192 also is MTK IOMMU gen2 which uses ARM Short-Descriptor translation
> > table format. The M4U-SMI HW diagram is as below:
> >
> >   EMI
> >|
> >   M4U
> >|
> >   
> >SMI Common
> >   
> >|
> >   +---+--+--+--+---+
> >   |   |  |  |   .. |   |
> >   |   |  |  |  |   |
> > larb0   larb1  larb2  larb4 ..  larb19   larb20
> > disp0   disp1   mdpvdec   IPE  IPE
> >
> > All the connections are HW fixed, SW can NOT adjust it.
> >
> > mt8192 M4U support 0~16GB iova range. we preassign different engines
> > into different iova ranges:
> >
> > domain-id  module iova-range  larbs
> >0   disp0 ~ 4G  larb0/1
> >1   vcodec  4G ~ 8G larb4/5/7
> >2   cam/mdp 8G ~ 12G larb2/9/11/13/14/16/17/18/19/20
> >3   CCU00x4000_ ~ 0x43ff_ larb13: port 9/10
> >4   CCU10x4400_ ~ 0x47ff_ larb14: port 4/5
> >
> > The iova range for CCU0/1(camera control unit) is HW requirement.
> >
> > Signed-off-by: Yong Wu 
> > ---
> >  .../bindings/iommu/mediatek,iommu.txt |   8 +-
> >  .../mediatek,smi-common.txt   |   5 +-
> >  .../memory-controllers/mediatek,smi-larb.txt  |   3 +-
> >  include/dt-bindings/memory/mt8192-larb-port.h | 237 ++
> >  4 files changed, 247 insertions(+), 6 deletions(-)
> >  create mode 100644 include/dt-bindings/memory/mt8192-larb-port.h
> > ...
> > diff --git a/include/dt-bindings/memory/mt8192-larb-port.h 
> > b/include/dt-bindings/memory/mt8192-larb-port.h
> > new file mode 100644
> > index ..fbe0d5d50f1c
> > --- /dev/null
> > +++ b/include/dt-bindings/memory/mt8192-larb-port.h
> > ...
> > +/* larb7 */
> > +#define M4U_PORT_L7_VENC_RCPU  MTK_M4U_DOM_ID(1, 7, 0)
> > +#define M4U_PORT_L7_VENC_REC   MTK_M4U_DOM_ID(1, 7, 1)
> > +#define M4U_PORT_L7_VENC_BSDMA MTK_M4U_DOM_ID(1, 7, 2)
> > +#define M4U_PORT_L7_VENC_SV_COMV   MTK_M4U_DOM_ID(1, 7, 3)
> > +#define M4U_PORT_L7_VENC_RD_COMV   MTK_M4U_DOM_ID(1, 7, 4)
> > +#define M4U_PORT_L7_VENC_CUR_LUMA  MTK_M4U_DOM_ID(1, 7, 5)
> > +#define M4U_PORT_L7_VENC_CUR_CHROMAMTK_M4U_DOM_ID(1, 7, 6)
> > +#define M4U_PORT_L7_VENC_REF_LUMA  MTK_M4U_DOM_ID(1, 7, 7)
> > +#define M4U_PORT_L7_VENC_REF_CHROMAMTK_M4U_DOM_ID(1, 7, 8)
> > +#define M4U_PORT_L7_JPGENC_Y_RDMA  MTK_M4U_DOM_ID(1, 7, 9)
> > +#define M4U_PORT_L7_JPGENC_Q_RDMA  MTK_M4U_DOM_ID(1, 7, 10)
> > +#define M4U_PORT_L7_JPGENC_C_TABLE MTK_M4U_DOM_ID(1, 7, 11)
> > +#define M4U_PORT_L7_JPGENC_BSDMA   MTK_M4U_DOM_ID(1, 7, 12)
> > +#define M4U_PORT_L7_VENC_SUB_R_LUMAMTK_M4U_DOM_ID(1, 7, 13)
> > +#define M4U_PORT_L7_VENC_SUB_W_LUMAMTK_M4U_DOM_ID(1, 7, 14)
> > +
> 
> Small nit, /* larb8: null */ is missing here.

oh. Yes. Thanks.
I will add it in next version.

> 
> > +/* larb9 */
> > +#define M4U_PORT_L9_IMG_IMGI_D1MTK_M4U_DOM_ID(2, 
> > 9, 0)
> > +#define M4U_PORT_L9_IMG_IMGBI_D1   MTK_M4U_DOM_ID(2, 9, 1)
> > +#define M4U_PORT_L9_IMG_DMGI_D1MTK_M4U_DOM_ID(2, 
> > 9, 2)
> > +#define M4U_PORT_L9_IMG_DEPI_D1MTK_M4U_DOM_ID(2, 
> > 9, 3)
> > +#define M4U_PORT_L9_IMG_ICE_D1 MTK_M4U_DOM_ID(2, 9, 4)
> > +#define M4U_PORT_L9_IMG_SMTI_D1MTK_M4U_DOM_ID(2, 
> > 9, 5)
> > +#define M4U_PORT_L9_IMG_SMTO_D2MTK_M4U_DOM_ID(2, 
> > 9, 6)
> > +#define M4U_PORT_L9_IMG_SMTO_D1MTK_M4U_DOM_ID(2, 
> > 9, 7)
> > +#define M4U_PORT_L9_IMG_CRZO_D1MTK_M4U_DOM_ID(2, 
> > 9, 8)
> > +#define M4U_PORT_L9_IMG_IMG3O_D1   MTK_M4U_DOM_ID(2, 9, 9)
> > +#define M4U_PORT_L9_IMG_VIPI_D1MTK_M4U_DOM_ID(2, 
> > 9, 10)
> > +#define M4U_PORT_L9_IMG_SMTI_D5MTK_M4U_DOM_ID(2, 
> > 9, 11)
> > +#define M4U_PORT_L9_IMG_TIMGO_D1   MTK_M4U_DOM_ID(2, 9, 12)
> > +#define M4U_PORT_L9_IMG_UFBC_W0MTK_M4U_DOM_ID(2, 
> > 9, 13)
> > +#define M4U_PORT_L9_IMG_UFBC_R0MTK_M4U_DOM_ID(2, 
> > 9, 14)
> > +
> > ...

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

Re: [PATCH 06/21] iommu/io-pgtable-arm-v7s: Use ias to check the valid iova in unmap

2020-07-13 Thread Yong Wu
On Mon, 2020-07-13 at 08:38 +0800, Nicolas Boichat wrote:
> On Sat, Jul 11, 2020 at 2:50 PM Yong Wu  wrote:
> >
> > As title.
> >
> > Signed-off-by: Yong Wu 
> > ---
> >  drivers/iommu/io-pgtable-arm-v7s.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/iommu/io-pgtable-arm-v7s.c 
> > b/drivers/iommu/io-pgtable-arm-v7s.c
> > index 4272fe4e17f4..01f2a8876808 100644
> > --- a/drivers/iommu/io-pgtable-arm-v7s.c
> > +++ b/drivers/iommu/io-pgtable-arm-v7s.c
> > @@ -717,7 +717,7 @@ static size_t arm_v7s_unmap(struct io_pgtable_ops *ops, 
> > unsigned long iova,
> >  {
> > struct arm_v7s_io_pgtable *data = io_pgtable_ops_to_data(ops);
> >
> > -   if (WARN_ON(upper_32_bits(iova)))
> > +   if (WARN_ON(iova >= (1ULL << data->iop.cfg.ias)))
> 
> This is a little odd as iova is unsigned long and 1ULL is unsigned long long.
> 
> Would it be better to keep the spirit of the previous test and do
> something like:
>  if (WARN_ON(iova >> data->iop.cfg.ias)) ?

Yes. Thanks. I will change it like this in next version.

Also change this in arm_v7s_map by the way.

> 
> > return 0;
> >
> > return __arm_v7s_unmap(data, gather, iova, size, 1, data->pgd);
> > --
> > 2.18.0

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


Re: [SPAM]Re: [PATCH 01/21] dt-binding: memory: mediatek: Add a common larb-port header file

2020-07-13 Thread Yong Wu
On Sun, 2020-07-12 at 20:06 +0200, Matthias Brugger wrote:
> 
> On 11/07/2020 08:48, Yong Wu wrote:
> > Put all the macros about smi larb/port togethers, this is a preparing
> > patch for extending LARB_NR and adding new dom-id support.
> > 
> > Signed-off-by: Yong Wu 

[...]

> > diff --git a/include/dt-bindings/memory/mtk-smi-larb-port.h 
> > b/include/dt-bindings/memory/mtk-smi-larb-port.h
> > new file mode 100644
> > index ..2ec7fe5ce4e9
> > --- /dev/null
> > +++ b/include/dt-bindings/memory/mtk-smi-larb-port.h
> > @@ -0,0 +1,15 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Copyright (c) 2020 MediaTek Inc.
> > + * Author: Yong Wu 
> > + */
> > +#ifndef __DTS_MTK_IOMMU_PORT_H_
> > +#define __DTS_MTK_IOMMU_PORT_H_
> > +
> > +#define MTK_LARB_NR_MAX16
> 
> include/soc/mediatek/smi.h has the very same define.
> Should smi.h include this file?

If smi.h include this file, it also is ok. then these two
files(mtk_iommu.h and mtk_smi.c) don't need include this.

the MTK_LARB_NR_MAX only is used in mtk_iommu.h and mtk_smi.c directly,
thus I use that two files include this file currently. locally I think
it may be helpful for readable.

is this ok?

> 
> Regards,
> Matthias
> 
> > +
> > +#define MTK_M4U_ID(larb, port) (((larb) << 5) | (port))
> > +#define MTK_M4U_TO_LARB(id)(((id) >> 5) & 0xf)
> > +#define MTK_M4U_TO_PORT(id)((id) & 0x1f)
> > +
> > +#endif
> > 
> 
> ___
> Linux-mediatek mailing list
> linux-media...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek

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


Re: [PATCH 01/21] dt-binding: memory: mediatek: Add a common larb-port header file

2020-07-13 Thread Yong Wu
On Mon, 2020-07-13 at 13:43 +0800, Pi-Hsun Shih wrote:
> On Mon, Jul 13, 2020 at 2:06 AM Matthias Brugger  
> wrote:
> >
> >
> >
> > On 11/07/2020 08:48, Yong Wu wrote:
> > > Put all the macros about smi larb/port togethers, this is a preparing
> > > patch for extending LARB_NR and adding new dom-id support.
> > >
> > > Signed-off-by: Yong Wu 
> > > ---
> > >   include/dt-bindings/memory/mt2712-larb-port.h  |  2 +-
> > >   include/dt-bindings/memory/mt6779-larb-port.h  |  2 +-
> > >   include/dt-bindings/memory/mt8173-larb-port.h  |  2 +-
> > >   include/dt-bindings/memory/mt8183-larb-port.h  |  2 +-
> > >   include/dt-bindings/memory/mtk-smi-larb-port.h | 15 +++
> > >   5 files changed, 19 insertions(+), 4 deletions(-)
> > >   create mode 100644 include/dt-bindings/memory/mtk-smi-larb-port.h
> > >
> > > ...
> > > diff --git a/include/dt-bindings/memory/mtk-smi-larb-port.h 
> > > b/include/dt-bindings/memory/mtk-smi-larb-port.h
> > > new file mode 100644
> > > index ..2ec7fe5ce4e9
> > > --- /dev/null
> > > +++ b/include/dt-bindings/memory/mtk-smi-larb-port.h
> > > @@ -0,0 +1,15 @@
> > > +/* SPDX-License-Identifier: GPL-2.0-only */
> > > +/*
> > > + * Copyright (c) 2020 MediaTek Inc.
> > > + * Author: Yong Wu 
> > > + */
> > > +#ifndef __DTS_MTK_IOMMU_PORT_H_
> > > +#define __DTS_MTK_IOMMU_PORT_H_
> > > +
> > > +#define MTK_LARB_NR_MAX  16
> >
> > include/soc/mediatek/smi.h has the very same define.
> > Should smi.h include this file?
> >
> > Regards,
> > Matthias
> >
> 
> Looks like this is being addressed in patch 5 in this series ([05/21]
> iommu/mediatek: Use the common mtk-smi-larb-port.h)
> That said, should that patch be merged into this one?

At the beginning, I really did like this. But checkpatch will complain
like that:

WARNING:DT_SPLIT_BINDING_PATCH: DT binding docs and includes should be a
separate patch. See:
Documentation/devicetree/bindings/submitting-patches.rst

> 
> 
> 
> > > +
> > > +#define MTK_M4U_ID(larb, port)   (((larb) << 5) | (port))
> > > +#define MTK_M4U_TO_LARB(id)  (((id) >> 5) & 0xf)
> > > +#define MTK_M4U_TO_PORT(id)  ((id) & 0x1f)
> > > +
> > > +#endif
> > >

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