Re: [PATCH v4 1/5] docs: IOMMU user API
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
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)
> 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
> 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
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'
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
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
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
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
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
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)
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
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
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
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
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
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
"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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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