Add support for privileged mappings
Subsystems (8)
v9 cover Add support for privileged mappings
This series is a resend of the V5 that Mitch sent sometime back [2] All the patches are the same and i have just rebased. Redid patch [3], as it does not apply in this code base. Added a couple of more patches [4], [5] from Robin for adding the privileged attributes to armv7s format and arm-smmuv3 revert. Added a patch for passing in the privileged attributes from arm32 dma-mapping apis as well.
The following patch to the ARM SMMU driver:
commit d346180e70b91b3d5a1ae7e5603e65593d4622bc
Author: Robin Murphy <[email protected]>
Date: Tue Jan 26 18:06:34 2016 +0000
iommu/arm-smmu: Treat all device transactions as unprivileged
started forcing all SMMU transactions to come through as "unprivileged". The rationale given was that:
(1) There is no way in the IOMMU API to even request privileged mappings.
(2) It's difficult to implement a DMA mapper that correctly models the ARM VMSAv8 behavior of unprivileged-writeable => privileged-execute-never.
This series rectifies (1) by introducing an IOMMU API for privileged mappings and implements it in io-pgtable-arm.
This series rectifies (2) by introducing a new dma attribute (DMA_ATTR_PRIVILEGED) for users of the DMA API that need privileged mappings which are inaccessible to lesser-privileged execution levels, and implements it in the arm64 IOMMU DMA mapper. The one known user (pl330.c) is converted over to the new attribute.
Jordan and Jeremy can provide more info on the use case if needed, but the high level is that it's a security feature to prevent attacks such as [1].
Note that, i tested this on arm64 with arm-smmuv2, short descriptor changes, tested this on arm32 platform as well and do not have an platform to test this with arm-smmuv3.
[1] https://github.com/robclark/kilroy
[2] https://lkml.org/lkml/2016/7/27/590
[3] https://patchwork.kernel.org/patch/9250493/
[4] http://www.linux-arm.org/git?p=linux-rm.git;a=commit;h=1291bd74f05d31da1dab3df02987cba5bd25849b
[5] http://www.linux-arm.org/git?p=linux-rm.git;a=commit;h=a79c1c6333f26849dba418cd92de26b60f5954f3
Changelog:
v8..v9
- Added additional comment in patch 1 and added tag for patch 6.
v7..v8
- Added a patch for passing in the privileged attributes from arm32 dma-mapping apis as well.
v6..v7
- Added couple of more patches, picked up acks, updated commit log
v5..v6
- Rebased all the patches and redid 6/6 as it does not apply in this code base.
v4..v5
- Simplified patch 4/6 (suggested by Robin Murphy).
v3..v4
- Rebased and reworked on linux next due to the dma attrs rework going on over there. Patches changed: 3/6, 4/6, and 5/6.
v2..v3
- Incorporated feedback from Robin:
* Various comments and re-wordings.
* Use existing bit definitions for IOMMU_PRIV implementation in io-pgtable-arm.
* Renamed and redocumented dma_direction_to_prot.
* Don't worry about executability in new DMA attr.
v1..v2
- Added a new DMA attribute to make executable privileged mappings work, and use that in the pl330 driver (suggested by Will).
Jeremy Gebben (1):
iommu/io-pgtable-arm: add support for the IOMMU_PRIV flag
Mitchel Humpherys (4):
iommu: add IOMMU_PRIV attribute
common: DMA-mapping: add DMA_ATTR_PRIVILEGED attribute arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED
dmaengine: pl330: Make sure microcode is privileged
Robin Murphy (2):
iommu/io-pgtable-arm-v7s: Add support for the IOMMU_PRIV flag Revert "iommu/arm-smmu: Set PRIVCFG in stage 1 STEs"
Sricharan R (2):
arm/dma-mapping: Implement DMA_ATTR_PRIVILEGED iommu/arm-smmu: Set privileged attribute to 'default' instead of
'unprivileged'
Documentation/DMA-attributes.txt | 10 +++++++ arch/arm/mm/dma-mapping.c | 60 +++++++++++++++++++------------------- arch/arm64/mm/dma-mapping.c | 6 ++-- drivers/dma/pl330.c | 5 ++-- drivers/iommu/arm-smmu-v3.c | 7 +---- drivers/iommu/arm-smmu.c | 2 +- drivers/iommu/dma-iommu.c | 12 ++++++-- drivers/iommu/io-pgtable-arm-v7s.c | 6 +++- drivers/iommu/io-pgtable-arm.c | 5 +++- include/linux/dma-iommu.h | 3 +- include/linux/dma-mapping.h | 7 +++++ include/linux/iommu.h | 7 +++++ 12 files changed, 82 insertions(+), 48 deletions(-)
Signature · 1 line
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
PATCH v9 1/9 iommu: add IOMMU_PRIV attribute
From: Mitchel Humpherys <[email protected]>
Add the IOMMU_PRIV attribute, which is used to indicate privileged mappings.
--- include/linux/iommu.h | 7 +++++++ 1 file changed, 7 insertions(+)
include/linux/iommu.h +7 -0
@@ -31,6 +31,13 @@#define IOMMU_CACHE (1 << 2)/* DMA cache coherency */ #define IOMMU_NOEXEC (1 << 3)#define IOMMU_MMIO (1 << 4)/* e.g. things like MSI doorbells */ +/* +* This is to make the IOMMU API setup privileged +* mapppings accessible by the master only@higher +* privileged execution level and inaccessible at +* less privileged levels. +*/ +#define IOMMU_PRIV (1 << 5)struct iommu_ops ;struct iommu_group ;
Signature · 1 line
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Robin Murphy
maintainerPATCH 1/1 iommu: Better document the IOMMU_PRIV flag
This is a fairly subtle thing - let's make sure it's described as clearly as possible to avoid potential misunderstandings.
---
Having another look through the IOMMU_PRIV series, I wasn't convinced that the original comment was really all that helpful - I'm happy for this to be squashed in if you like.
Robin.
include/linux/iommu.h | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
include/linux/iommu.h +7 -4
@@ -32,10 +32,13 @@#define IOMMU_NOEXEC (1 << 3)#define IOMMU_MMIO (1 << 4)/* e.g. things like MSI doorbells */ /* -* This is to make the IOMMU API setup privileged -* mapppings accessible by the master only at higher -* privileged execution level and inaccessible at -* less privileged levels. +* Where the bus hardware includes a privilege level as part of its access type +* markings, and certain devices are capable of issuing transactions marked as +* either 'supervisor' or 'user', the IOMMU_PRIV flag requests that the other +* given permission flags only apply to accesses@the higher privilege level, +* and that unprivileged transactions should have as little access as possible. +* This would usually imply the same permissions as kernel mappings on the CPU, +* if the IOMMU page table format is equivalent. */ #define IOMMU_PRIV (1 << 5)
Signature · 1 line
2.11.0.dirty
PATCH 1/1 iommu: Better document the IOMMU_PRIV flag
Hi Robin,
Quoted 32 lines
-----Original Message-----
From: linux-arm-kernel [mailto:linux-arm-kernel-bounces at lists.infradead.org] On Behalf Of Robin Murphy
Sent: Friday, January 27, 2017 5:53 PM
To: will.deacon at arm.com
Cc: iommu at lists.linux-foundation.org; sricharan at codeaurora.org; linux-arm-kernel at lists.infradead.org
Subject: [PATCH] iommu: Better document theIOMMU_PRIVflagThis is a fairly subtle thing - let's make sure it's described as clearly as possible to avoid potential misunderstandings.
Signed-off-by Robin Murphy <[email protected]>---
Having another look through the
IOMMU_PRIVseries, I wasn't convinced that the original comment was really all that helpful - I'm happy for this to be squashed in if you like.Robin.
include/linux/iommu.h| 11 +++++++----1 file changed, 7 insertions(+), 4 deletions(-)include/linux/iommu.h +0
-2@@ -32,10 +32,13 @@#define IOMMU_NOEXEC (1 << 3)#define IOMMU_MMIO (1 << 4)/* e.g. things like MSI doorbells */ /* -* This is to make the IOMMU API setup privileged -* mapppings accessible by the master only at higher
include/linux/iommu.h line 42
Quoted 10 lines
include/linux/iommu.h +7
-2-* privileged execution level and inaccessible at -* less privileged levels. +* Where the bus hardware includes a privilege level as part of its access type +* markings, and certain devices are capable of issuing transactions marked as +* either 'supervisor' or 'user', the IOMMU_PRIV flag requests that the other +* given permission flags only apply to accesses at the higher privilege level, +* and that unprivileged transactions should have as little access as possible. +* This would usually imply the same permissions as kernel mappings on the CPU, +* if the IOMMU page table format is equivalent. */
Agree, gives much more insight.
Regards,
Sricharan
PATCH v9 2/9 iommu/io-pgtable-arm: add support for the IOMMU_PRIV flag
In reply to Sricharan RFrom: Jeremy Gebben <[email protected]>
Allow the creation of privileged mode mappings, for stage 1 only.
--- drivers/iommu/io-pgtable-arm.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/iommu/io-pgtable-arm.c +4 -1
@@ -350,11 +350,14 @@ static arm_lpae_iopte arm_lpae_prot_to_pte(struct arm_lpae_io_pgtable *data,if (data->iop.fmt ==ARM_64_LPAE_S1 || data->iop.fmt ==ARM_32_LPAE_S1 ) { - pte =ARM_LPAE_PTE_AP_UNPRIV | ARM_LPAE_PTE_nG; + pte = ARM_LPAE_PTE_nG;if (!(prot &IOMMU_WRITE ) && (prot &IOMMU_READ )) pte |=ARM_LPAE_PTE_AP_RDONLY ; +if (!(prot &IOMMU_PRIV )) + pte |=ARM_LPAE_PTE_AP_UNPRIV ; +if (prot &IOMMU_MMIO ) pte |= (ARM_LPAE_MAIR_ATTR_IDX_DEV <<ARM_LPAE_PTE_ATTRINDX_SHIFT );
Signature · 1 line
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
PATCH v9 3/9 iommu/io-pgtable-arm-v7s: Add support for the IOMMU_PRIV flag
In reply to Sricharan RFrom: Robin Murphy <[email protected]>
The short-descriptor format also allows privileged-only mappings, so let's wire it up.
--- drivers/iommu/io-pgtable-arm-v7s.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
drivers/iommu/io-pgtable-arm-v7s.c +5 -1
@@ -265,7 +265,9 @@ static arm_v7s_iopte arm_v7s_prot_to_pte(int prot, int lvl,if (!(prot &IOMMU_MMIO )) pte |=ARM_V7S_ATTR_TEX (1 );if (ap) { - pte |=ARM_V7S_PTE_AF |ARM_V7S_PTE_AP_UNPRIV ; + pte |=ARM_V7S_PTE_AF ; +if (!(prot &IOMMU_PRIV )) + pte |=ARM_V7S_PTE_AP_UNPRIV ;if (!(prot &IOMMU_WRITE )) pte |=ARM_V7S_PTE_AP_RDONLY ; }
@@ -288,6 +290,8 @@ static int arm_v7s_pte_to_prot(arm_v7s_iopte pte, int lvl)if (!(attr &ARM_V7S_PTE_AP_RDONLY )) prot |=IOMMU_WRITE ; +if (!(attr &ARM_V7S_PTE_AP_UNPRIV )) + prot |=IOMMU_PRIV ;if ((attr & (ARM_V7S_TEX_MASK <<ARM_V7S_TEX_SHIFT )) ==0 ) prot |=IOMMU_MMIO ;else if (pte &ARM_V7S_ATTR_C )
Signature · 1 line
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
PATCH v9 4/9 common: DMA-mapping: add DMA_ATTR_PRIVILEGED attribute
In reply to Sricharan RFrom: Mitchel Humpherys <[email protected]>
This patch adds the DMA_ATTR_PRIVILEGED attribute to the DMA-mapping subsystem.
Some advanced peripherals such as remote processors and GPUs perform accesses to DMA buffers in both privileged "supervisor" and unprivileged "user" modes. This attribute is used to indicate to the DMA-mapping subsystem that the buffer is fully accessible at the elevated privilege level (and ideally inaccessible or at least read-only at the lesser-privileged levels).
--- Documentation/DMA-attributes.txt | 10 ++++++++++ include/linux/dma-mapping.h | 7 +++++++ 2 files changed, 17 insertions(+)
Documentation/DMA-attributes.txt +10 -0
@@ -143,3 +143,13 @@ So, this provides a way for drivers to avoid those error messages on calls where allocation failures are not a problem, and shouldn't bother the logs. NOTE: At the moment DMA_ATTR_NO_WARN is only implemented on PowerPC. + +DMA_ATTR_PRIVILEGED +------------------------------ + +Some advanced peripherals such as remote processors and GPUs perform +accesses to DMA buffers in both privileged "supervisor" and unprivileged +"user" modes. This attribute is used to indicate to the DMA-mapping +subsystem that the buffer is fully accessible at the elevated privilege +level (and ideally inaccessible or at least read-only at the +lesser-privileged levels).
include/linux/dma-mapping.h +7 -0
@@ -63,6 +63,13 @@#define DMA_ATTR_NO_WARN (1UL << 8)/* +* DMA_ATTR_PRIVILEGED: used to indicate that the buffer is fully +* accessible at an elevated privilege level (and ideally inaccessible or +* at least read-only@lesser-privileged levels). +*/ +#define DMA_ATTR_PRIVILEGED (1UL << 9) + +/* * A dma_addr_t can hold any valid DMA or bus address for the platform. * It can be given to a device to use as a DMA source or target. A CPU cannot * reference a dma_addr_t directly because there may be translation between
Signature · 1 line
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
PATCH v9 5/9 arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED
In reply to Sricharan RFrom: Mitchel Humpherys <[email protected]>
The newly added DMA_ATTR_PRIVILEGED is useful for creating mappings that are only accessible to privileged DMA engines. Implement it in dma-iommu.c so that the ARM64 DMA IOMMU mapper can make use of it.
--- arch/arm64/mm/dma-mapping.c | 6 +++--- drivers/iommu/dma-iommu.c | 12 +++++++++--- include/linux/dma-iommu.h | 3 ++- 3 files changed, 14 insertions(+), 7 deletions(-)
arch/arm64/mm/dma-mapping.c +3 -3
@@ -557,7 +557,7 @@ static void *__iommu_alloc_attrs(struct device *dev, size_t size, unsigned long attrs) {bool coherent =is_device_dma_coherent (dev); -int ioprot =dma_direction_to_prot (DMA_BIDIRECTIONAL , coherent); +int ioprot =dma_info_to_prot (DMA_BIDIRECTIONAL , coherent, attrs);size_t iosize = size;void *addr;
@@ -711,7 +711,7 @@ static dma_addr_t __iommu_map_page(struct device *dev, struct page *page, unsigned long attrs) {bool coherent =is_device_dma_coherent (dev); -int prot =dma_direction_to_prot (dir, coherent); +int prot =dma_info_to_prot (dir, coherent, attrs);dma_addr_t dev_addr =iommu_dma_map_page (dev, page, offset, size, prot); if (!iommu_dma_mapping_error(dev, dev_addr) &&
@@ -769,7 +769,7 @@ static int __iommu_map_sg_attrs(struct device *dev, struct scatterlist *sgl,__iommu_sync_sg_for_device (dev, sgl, nelems, dir);return iommu_dma_map_sg (dev, sgl, nelems, -dma_direction_to_prot (dir, coherent)); +dma_info_to_prot (dir, coherent, attrs)); } static void __iommu_unmap_sg_attrs(struct device *dev,
drivers/iommu/dma-iommu.c +9 -3
@@ -181,16 +181,22 @@ int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,EXPORT_SYMBOL (iommu_dma_init_domain);/** -* dma_direction_to_prot - Translate DMA API directions to IOMMU API page flags +* dma_info_to_prot - Translate DMA API directions and attributes to IOMMU API +* page flags. * @dir: Direction of DMA transfer * @coherent: Is the DMA master cache-coherent? +* @attrs: DMA attributes for the mapping * * Return: corresponding IOMMU API page protection flags */ -int dma_direction_to_prot (enum dma_data_direction dir,bool coherent) +int dma_info_to_prot (enum dma_data_direction dir,bool coherent, +unsigned long attrs) {int prot = coherent ?IOMMU_CACHE :0 ; +if (attrs &DMA_ATTR_PRIVILEGED ) + prot |=IOMMU_PRIV ; +switch (dir) {case DMA_BIDIRECTIONAL :return prot |IOMMU_READ |IOMMU_WRITE ;
@@ -633,7 +639,7 @@ dma_addr_t iommu_dma_map_resource(struct device *dev, phys_addr_t phys, size_t size, enum dma_data_direction dir, unsigned long attrs) { return __iommu_dma_map(dev, phys, size, - dma_direction_to_prot(dir, false) | IOMMU_MMIO); + dma_info_to_prot(dir, false, attrs) | IOMMU_MMIO); } void iommu_dma_unmap_resource(struct device *dev, dma_addr_t handle,
include/linux/dma-iommu.h +2 -1
@@ -34,7 +34,8 @@ int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base, u64 size, struct device *dev);/* General helpers for DMA-API <-> IOMMU-API interaction */ -int dma_direction_to_prot (enum dma_data_direction dir,bool coherent); +int dma_info_to_prot (enum dma_data_direction dir,bool coherent, +unsigned long attrs);/* * These implement the bulk of the relevant DMA mapping callbacks, but require
Signature · 1 line
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
PATCH v9 6/9 arm/dma-mapping: Implement DMA_ATTR_PRIVILEGED
In reply to Sricharan RThe newly added DMA_ATTR_PRIVILEGED is useful for creating mappings that are only accessible to privileged DMA engines. Adding it to the arm dma-mapping.c so that the ARM32 DMA IOMMU mapper can make use of it.
--- arch/arm/mm/dma-mapping.c | 60 +++++++++++++++++++++++------------------------ 1 file changed, 30 insertions(+), 30 deletions(-)
arch/arm/mm/dma-mapping.c +30 -30
@@ -1171,6 +1171,25 @@ static int __init dma_debug_do_init(void)#ifdef CONFIG_ARM_DMA_USE_IOMMU +static int __dma_info_to_prot (enum dma_data_direction dir,unsigned long attrs) +{ +int prot =0 ; + +if (attrs &DMA_ATTR_PRIVILEGED ) + prot |=IOMMU_PRIV ; + +switch (dir) { +case DMA_BIDIRECTIONAL : +return prot |IOMMU_READ |IOMMU_WRITE ; +case DMA_TO_DEVICE : +return prot |IOMMU_READ ; +case DMA_FROM_DEVICE : +return prot |IOMMU_WRITE ; +default : +return prot; + } +} +/* IOMMU */ static int extend_iommu_mapping (struct dma_iommu_mapping *mapping);
@@ -1394,7 +1413,8 @@ static int __iommu_free_buffer(struct device *dev, struct page **pages,* Create a mapping in device IO address space for specified pages */ static dma_addr_t -__iommu_create_mapping (struct device *dev,struct page **pages,size_t size) +__iommu_create_mapping (struct device *dev,struct page **pages,size_t size, +unsigned long attrs) {struct dma_iommu_mapping *mapping =to_dma_iommu_mapping (dev);unsigned int count =PAGE_ALIGN (size) >>PAGE_SHIFT ;
@@ -1419,7 +1439,7 @@ static int __iommu_free_buffer(struct device *dev, struct page **pages, len = (j - i) <<PAGE_SHIFT ; ret =iommu_map (mapping->domain, iova, phys, len, -IOMMU_READ |IOMMU_WRITE ); +__dma_info_to_prot (DMA_BIDIRECTIONAL , attrs));if (ret <0 ) goto fail; iova += len;
@@ -1476,7 +1496,8 @@ static struct page **__iommu_get_pages(void *cpu_addr, unsigned long attrs) }static void *__iommu_alloc_simple (struct device *dev,size_t size,gfp_t gfp, -dma_addr_t *handle,int coherent_flag) +dma_addr_t *handle,int coherent_flag, +unsigned long attrs) {struct page *page;void *addr;
@@ -1488,7 +1509,7 @@ static void *__iommu_alloc_simple(struct device *dev, size_t size, gfp_t gfp,if (!addr)return NULL ; - *handle =__iommu_create_mapping (dev, &page, size); + *handle =__iommu_create_mapping (dev, &page, size, attrs);if (*handle ==DMA_ERROR_CODE ) goto err_mapping;
@@ -1522,7 +1543,7 @@ static void *__arm_iommu_alloc_attrs(struct device *dev, size_t size,if (coherent_flag ==COHERENT || !gfpflags_allow_blocking (gfp))return __iommu_alloc_simple (dev, size, gfp, handle, - coherent_flag); + coherent_flag, attrs);/* * Following is a work-around (a.k.a. hack) to prevent pages
@@ -1537,7 +1558,7 @@ static void *__arm_iommu_alloc_attrs(struct device *dev, size_t size,if (!pages)return NULL ; - *handle =__iommu_create_mapping (dev, pages, size); + *handle =__iommu_create_mapping (dev, pages, size, attrs);if (*handle ==DMA_ERROR_CODE ) goto err_buffer;
@@ -1672,27 +1693,6 @@ static int arm_iommu_get_sgtable(struct device *dev, struct sg_table *sgt, GFP_KERNEL); } -static int __dma_direction_to_prot (enum dma_data_direction dir) -{ -int prot; - -switch (dir) { -case DMA_BIDIRECTIONAL : - prot =IOMMU_READ |IOMMU_WRITE ; -break ; -case DMA_TO_DEVICE : - prot =IOMMU_READ ; -break ; -case DMA_FROM_DEVICE : - prot =IOMMU_WRITE ; -break ; -default : - prot =0 ; - } - -return prot; -} -/* * Map a part of the scatter-gather list into contiguous io address space */
@@ -1722,7 +1722,7 @@ static int __map_sg_chunk(struct device *dev, struct scatterlist *sg,if (!is_coherent && (attrs &DMA_ATTR_SKIP_CPU_SYNC ) ==0 )__dma_page_cpu_to_dev (sg_page (s), s->offset, s->length, dir); - prot =__dma_direction_to_prot (dir); + prot =__dma_info_to_prot (dir, attrs); ret =iommu_map (mapping->domain, iova, phys, len, prot);if (ret <0 )
@@ -1930,7 +1930,7 @@ static dma_addr_t arm_coherent_iommu_map_page(struct device *dev, struct page *pif (dma_addr ==DMA_ERROR_CODE )return dma_addr; - prot =__dma_direction_to_prot (dir); + prot =__dma_info_to_prot (dir, attrs); ret =iommu_map (mapping->domain, dma_addr,page_to_phys (page), len, prot);if (ret <0 )
@@ -2036,7 +2036,7 @@ static dma_addr_t arm_iommu_map_resource(struct device *dev,if (dma_addr ==DMA_ERROR_CODE )return dma_addr; - prot =__dma_direction_to_prot (dir) |IOMMU_MMIO ; + prot =__dma_info_to_prot (dir, attrs) |IOMMU_MMIO ; ret =iommu_map (mapping->domain, dma_addr, addr, len, prot);if (ret <0 )
Signature · 1 line
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
PATCH v9 7/9 dmaengine: pl330: Make sure microcode is privileged
In reply to Sricharan RFrom: Mitchel Humpherys <[email protected]>
The PL330 is hard-wired such that instruction fetches on both the manager and channel threads go out onto the bus with the "privileged" bit set. This can become troublesome once there is an IOMMU or other form of memory protection downstream, since those will typically be programmed by the DMA mapping subsystem in the expectation of normal unprivileged transactions (such as the PL330 channel threads' own data accesses as currently configured by this driver).
To avoid the case of, say, an IOMMU blocking an unexpected privileged transaction with a permission fault, use the newly-introduced DMA_ATTR_PRIVILEGED attribute for the mapping of our microcode buffer. That way the DMA layer can do whatever it needs to do to make things continue to work as expected on more complex systems.
--- drivers/dma/pl330.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/dma/pl330.c +3 -2
@@ -1864,9 +1864,10 @@ static int dmac_alloc_resources(struct pl330_dmac *pl330)* Alloc MicroCode buffer for 'chans' Channel threads. * A channel's buffer offset is (Channel_Id * MCODE_BUFF_PERCHAN) */ - pl330->mcode_cpu =dma_alloc_coherent (pl330->ddma.dev, + pl330->mcode_cpu =dma_alloc_attrs (pl330->ddma.dev, chans * pl330->mcbufsz, - &pl330->mcode_bus,GFP_KERNEL ); + &pl330->mcode_bus,GFP_KERNEL , +DMA_ATTR_PRIVILEGED );if (!pl330->mcode_cpu) {dev_err (pl330->ddma.dev,"%s:%d Can't allocate memory!\n" , __func__, __LINE__);
Signature · 1 line
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
PATCH v9 8/9 iommu/arm-smmu: Set privileged attribute to 'default' instead of 'unprivileged'
In reply to Sricharan RCurrently the driver sets all the device transactions privileges to UNPRIVILEGED, but there are cases where the iommu masters wants to isolate privileged supervisor and unprivileged user.
So don't override the privileged setting to unprivileged, instead set it to default as incoming and let it be controlled by the pagetable settings.
--- drivers/iommu/arm-smmu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/iommu/arm-smmu.c +1 -1
@@ -1214,7 +1214,7 @@ static int arm_smmu_domain_add_master(struct arm_smmu_domain *smmu_domain,continue ; s2cr[idx].type = type; - s2cr[idx].privcfg =S2CR_PRIVCFG_UNPRIV ; + s2cr[idx].privcfg =S2CR_PRIVCFG_DEFAULT ; s2cr[idx].cbndx = cbndx;arm_smmu_write_s2cr (smmu, idx); }
Signature · 1 line
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
PATCH v9 9/9 Revert "iommu/arm-smmu: Set PRIVCFG in stage 1 STEs"
In reply to Sricharan RFrom: Robin Murphy <[email protected]>
This reverts commit df5e1a0f2a2d779ad467a691203bcbc74d75690e.
Now that proper privileged mappings can be requested via IOMMU_PRIV, unconditionally overriding the incoming PRIVCFG becomes the wrong thing to do, so stop it.
--- drivers/iommu/arm-smmu-v3.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)
drivers/iommu/arm-smmu-v3.c +1 -6
@@ -269,9 +269,6 @@#define STRTAB_STE_1_SHCFG_INCOMING 1UL#define STRTAB_STE_1_SHCFG_SHIFT 44 -#define STRTAB_STE_1_PRIVCFG_UNPRIV 2UL -#define STRTAB_STE_1_PRIVCFG_SHIFT 48 -#define STRTAB_STE_2_S2VMID_SHIFT 0#define STRTAB_STE_2_S2VMID_MASK 0xffffUL#define STRTAB_STE_2_VTCR_SHIFT 32
@@ -1073,9 +1070,7 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_device *smmu, u32 sid,#ifdef CONFIG_PCI_ATS STRTAB_STE_1_EATS_TRANS <<STRTAB_STE_1_EATS_SHIFT | #endif -STRTAB_STE_1_STRW_NSEL1 <<STRTAB_STE_1_STRW_SHIFT | -STRTAB_STE_1_PRIVCFG_UNPRIV << - STRTAB_STE_1_PRIVCFG_SHIFT); + STRTAB_STE_1_STRW_NSEL1 << STRTAB_STE_1_STRW_SHIFT);if (smmu->features &ARM_SMMU_FEAT_STALLS ) dst[1 ] |=cpu_to_le64 (STRTAB_STE_1_S1STALLD );
Signature · 1 line
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation