Re: [PATCH RFC 1/1] iommu: set the default iommu-dma mode as non-strict
On 2019/3/1 19:07, Jean-Philippe Brucker wrote: > Hi Leizhen, > > On 01/03/2019 04:44, Leizhen (ThunderTown) wrote: >> >> >> On 2019/2/26 20:36, Hanjun Guo wrote: >>> Hi Jean, >>> >>> On 2019/1/31 22:55, Jean-Philippe Brucker wrote: Hi, On 31/01/2019 13:52, Zhen Lei wrote: > Currently, many peripherals are faster than before. For example, the top > speed of the older netcard is 10Gb/s, and now it's more than 25Gb/s. But > when iommu page-table mapping enabled, it's hard to reach the top speed > in strict mode, because of frequently map and unmap operations. In order > to keep abreast of the times, I think it's better to set non-strict as > default. Most users won't be aware of this relaxation and will have their system vulnerable to e.g. thunderbolt hotplug. See for example 4.3 Deferred Invalidation in http://www.cs.technion.ac.il/users/wwwb/cgi-bin/tr-get.cgi/2018/MSC/MSC-2018-21.pdf >> Hi Jean, >> >>In fact, we have discussed the vulnerable of deferred invalidation before >> upstream >> the non-strict patches. The attacks maybe possible because of an untrusted >> device or >> the mistake of the device driver. And we limited the VFIO to still use >> strict mode. >>As mentioned in the pdf, limit the freed memory with deferred >> invalidation only to >> be reused by the device, can mitigate the vulnerability. But it's too hard >> to implement >> it now. >>A compromise maybe we only apply non-strict to (1) dma_free_coherent, >> because the >> memory is controlled by DMA common module, so we can make the memory to be >> freed after >> the global invalidation in the timer handler. (2) And provide some new APIs >> related to >> iommu_unmap_page/sg, these new APIs deferred invalidation. And the candiate >> device >> drivers update the APIs if they want to improve performance. (3) Make sure >> that only >> the trusted devices and trusted drivers can apply (1) and (2). For example, >> the driver >> must be built into kernel Image. > > Do we have a notion of untrusted kernel drivers? A userspace driver It seems impossible to have such driver. The modules insmod by root users should be guaranteed by themselves. > (VFIO) is untrusted, ok. But a malicious driver loaded into the kernel > address space would have much easier ways to corrupt the system than to > exploit lazy mode... Yes, so that we have no need to consider untrusted drivers. > > For (3), I agree that we should at least disallow lazy mode if > pci_dev->untrusted is set. At the moment it means that we require the > strictest IOMMU configuration for external-facing PCI ports, but it can > be extended to blacklist other vulnerable devices or locations. I plan to add an attribute file for each device, espcially for hotplug devices. And let the root users to decide which mode should be used, strict or non-strict. Becasue they should known whether the hot-plug divice is trusted or not. > > If you do (3) then maybe we don't need (1) and (2), which require a > tonne of work in the DMA and IOMMU layers (but would certainly be nice > to see, since it would also help handle ATS invalidation timeouts) > > Thanks, > Jean > >>So that some high-end trusted devices use non-strict mode, and keep >> others still using >> strict mode. The drivers who want to use non-strict mode, should change to >> use new APIs >> by themselves. >> >> Why not keep the policy to secure by default, as we do for iommu.passthrough? And maybe add something similar to CONFIG_IOMMU_DEFAULT_PASSTRHOUGH? It's easy enough for experts to pass a command-line argument or change the default config. >>> >>> Sorry for the late reply, it was Chinese new year, and we had a long >>> discussion >>> internally, we are fine to add a Kconfig but not sure OS vendors will set it >>> to default y. >>> >>> OS vendors seems not happy to pass a command-line argument, to be honest, >>> this is our motivation to enable non-strict as default. Hope OS vendors >>> can see this email thread, and give some input here. >>> >>> Thanks >>> Hanjun >>> >>> >>> . >>> >> > > > . > -- Thanks! BestRegards ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH RFC/RFT] dma-contiguous: Get normal pages for single-page allocations
The addresses within a single page are always contiguous, so it's
not so necessary to always allocate one single page from CMA area.
Since the CMA area has a limited predefined size of space, it may
run out of space in heavy use cases, where there might be quite a
lot CMA pages being allocated for single pages.
However, there is also a concern that a device might care where a
page comes from -- it might expect the page from CMA area and act
differently if the page doesn't.
This patch tries to get normal pages for single-page allocations
unless the device has its own CMA area. This would save resources
from the CMA area for more CMA allocations. And it'd also reduce
CMA fragmentations resulted from trivial allocations.
Also, it updates the API and its callers so as to pass gfp flags.
Signed-off-by: Nicolin Chen
---
arch/arm/mm/dma-mapping.c | 5 ++---
arch/arm64/mm/dma-mapping.c| 2 +-
arch/xtensa/kernel/pci-dma.c | 2 +-
drivers/iommu/amd_iommu.c | 2 +-
drivers/iommu/intel-iommu.c| 3 +--
include/linux/dma-contiguous.h | 4 ++--
kernel/dma/contiguous.c| 23 +++
7 files changed, 27 insertions(+), 14 deletions(-)
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 8a90f298af96..c39fc2d97712 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -588,7 +588,7 @@ static void *__alloc_from_contiguous(struct device *dev,
size_t size,
struct page *page;
void *ptr = NULL;
- page = dma_alloc_from_contiguous(dev, count, order, gfp & __GFP_NOWARN);
+ page = dma_alloc_from_contiguous(dev, count, order, gfp);
if (!page)
return NULL;
@@ -1293,8 +1293,7 @@ static struct page **__iommu_alloc_buffer(struct device
*dev, size_t size,
unsigned long order = get_order(size);
struct page *page;
- page = dma_alloc_from_contiguous(dev, count, order,
-gfp & __GFP_NOWARN);
+ page = dma_alloc_from_contiguous(dev, count, order, gfp);
if (!page)
goto error;
diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index 78c0a72f822c..660adedaab5d 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -159,7 +159,7 @@ static void *__iommu_alloc_attrs(struct device *dev, size_t
size,
struct page *page;
page = dma_alloc_from_contiguous(dev, size >> PAGE_SHIFT,
- get_order(size), gfp & __GFP_NOWARN);
+get_order(size), gfp);
if (!page)
return NULL;
diff --git a/arch/xtensa/kernel/pci-dma.c b/arch/xtensa/kernel/pci-dma.c
index 9171bff76fc4..e15b893caadb 100644
--- a/arch/xtensa/kernel/pci-dma.c
+++ b/arch/xtensa/kernel/pci-dma.c
@@ -157,7 +157,7 @@ void *arch_dma_alloc(struct device *dev, size_t size,
dma_addr_t *handle,
if (gfpflags_allow_blocking(flag))
page = dma_alloc_from_contiguous(dev, count, get_order(size),
-flag & __GFP_NOWARN);
+flag);
if (!page)
page = alloc_pages(flag | __GFP_ZERO, get_order(size));
diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 6b0760dafb3e..c54923a9e31f 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -2691,7 +2691,7 @@ static void *alloc_coherent(struct device *dev, size_t
size,
return NULL;
page = dma_alloc_from_contiguous(dev, size >> PAGE_SHIFT,
- get_order(size), flag & __GFP_NOWARN);
+get_order(size), flag);
if (!page)
return NULL;
}
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index a0ef7c5e5dfe..cd3483d03f3b 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -3789,8 +3789,7 @@ static void *intel_alloc_coherent(struct device *dev,
size_t size,
if (gfpflags_allow_blocking(flags)) {
unsigned int count = size >> PAGE_SHIFT;
- page = dma_alloc_from_contiguous(dev, count, order,
-flags & __GFP_NOWARN);
+ page = dma_alloc_from_contiguous(dev, count, order, flags);
if (page && iommu_no_mapping(dev) &&
page_to_phys(page) + size > dev->coherent_dma_mask) {
dma_release_from_contiguous(dev, page, count);
diff --git a/include/linux/dma-contiguous.h b/include/linux/dma-contiguous.h
index f247e8aa5e3d..b166e8a8740f 100644
--- a/include/linux/dma-contiguous.h
+++ b/include/linux/dma-contiguous.h
@@ -112,7 +112,7 @@ static inline int
Re: [git pull] IOMMU Fixes for Linux v5.0-rc8
The pull request you sent on Fri, 1 Mar 2019 17:57:05 +0100: > git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git > tags/iommu-fix-v5.0-rc8 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/a215ce8f0e00c2d707080236f1aafec337371043 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
Re: [Freedreno] [RFC PATCH v1 02/15] iommu/arm-smmu: Add split pagetable support for arm-smmu-v2
On Fri, Mar 1, 2019 at 2:38 PM Jordan Crouse wrote:
>
> Add support for a split pagetable (TTBR0/TTBR1) scheme for
> arm-smmu-v2. If split pagetables are enabled, create a
> pagetable for TTBR1 and set up the sign extension bit so
> that all IOVAs with that bit set are mapped and translated
> from the TTBR1 pagetable.
>
> Signed-off-by: Jordan Crouse
> ---
>
> drivers/iommu/arm-smmu-regs.h | 18 +
> drivers/iommu/arm-smmu.c | 149
> +
> drivers/iommu/io-pgtable-arm.c | 3 +-
> 3 files changed, 154 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/iommu/arm-smmu-regs.h b/drivers/iommu/arm-smmu-regs.h
> index a1226e4..56f9709 100644
> --- a/drivers/iommu/arm-smmu-regs.h
> +++ b/drivers/iommu/arm-smmu-regs.h
> @@ -193,7 +193,25 @@ enum arm_smmu_s2cr_privcfg {
> #define RESUME_RETRY (0 << 0)
> #define RESUME_TERMINATE (1 << 0)
>
> +#define TTBCR_EPD1 (1 << 23)
> +#define TTBCR_T1SZ_SHIFT 16
> +#define TTBCR_IRGN1_SHIFT 24
> +#define TTBCR_ORGN1_SHIFT 26
> +#define TTBCR_RGN_WBWA 1
> +#define TTBCR_SH1_SHIFT28
> +#define TTBCR_SH_IS3
> +
> +#define TTBCR_TG1_16K (1 << 30)
> +#define TTBCR_TG1_4K (2 << 30)
> +#define TTBCR_TG1_64K (3 << 30)
> +
> #define TTBCR2_SEP_SHIFT 15
> +#define TTBCR2_SEP_31 (0x0 << TTBCR2_SEP_SHIFT)
> +#define TTBCR2_SEP_35 (0x1 << TTBCR2_SEP_SHIFT)
> +#define TTBCR2_SEP_39 (0x2 << TTBCR2_SEP_SHIFT)
> +#define TTBCR2_SEP_41 (0x3 << TTBCR2_SEP_SHIFT)
> +#define TTBCR2_SEP_43 (0x4 << TTBCR2_SEP_SHIFT)
> +#define TTBCR2_SEP_47 (0x5 << TTBCR2_SEP_SHIFT)
> #define TTBCR2_SEP_UPSTREAM(0x7 << TTBCR2_SEP_SHIFT)
> #define TTBCR2_AS (1 << 4)
>
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index af18a7e..05eb126 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -151,6 +151,7 @@ struct arm_smmu_cb {
> u32 tcr[2];
> u32 mair[2];
> struct arm_smmu_cfg *cfg;
> + u64 split_table_mask;
> };
>
> struct arm_smmu_master_cfg {
> @@ -208,6 +209,7 @@ struct arm_smmu_device {
> unsigned long va_size;
> unsigned long ipa_size;
> unsigned long pa_size;
> + unsigned long ubs_size;
> unsigned long pgsize_bitmap;
>
> u32 num_global_irqs;
> @@ -252,13 +254,14 @@ enum arm_smmu_domain_stage {
>
> struct arm_smmu_domain {
> struct arm_smmu_device *smmu;
> - struct io_pgtable_ops *pgtbl_ops;
> + struct io_pgtable_ops *pgtbl_ops[2];
> const struct iommu_gather_ops *tlb_ops;
> struct arm_smmu_cfg cfg;
> enum arm_smmu_domain_stage stage;
> boolnon_strict;
> struct mutexinit_mutex; /* Protects smmu pointer
> */
> spinlock_t cb_lock; /* Serialises ATS1* ops and
> TLB syncs */
> + u32 attributes;
> struct iommu_domain domain;
> };
>
> @@ -618,6 +621,69 @@ static irqreturn_t arm_smmu_global_fault(int irq, void
> *dev)
> return IRQ_HANDLED;
> }
>
> +static void arm_smmu_init_ttbr1(struct arm_smmu_domain *smmu_domain,
> + struct io_pgtable_cfg *pgtbl_cfg)
> +{
> + struct arm_smmu_device *smmu = smmu_domain->smmu;
> + struct arm_smmu_cfg *cfg = _domain->cfg;
> + struct arm_smmu_cb *cb = _domain->smmu->cbs[cfg->cbndx];
> + int pgsize = 1 << __ffs(pgtbl_cfg->pgsize_bitmap);
> +
> + /* Enable speculative walks through the TTBR1 */
> + cb->tcr[0] &= ~TTBCR_EPD1;
> +
> + cb->tcr[0] |= TTBCR_SH_IS << TTBCR_SH1_SHIFT;
> + cb->tcr[0] |= TTBCR_RGN_WBWA << TTBCR_IRGN1_SHIFT;
> + cb->tcr[0] |= TTBCR_RGN_WBWA << TTBCR_ORGN1_SHIFT;
> +
> + switch (pgsize) {
> + case SZ_4K:
> + cb->tcr[0] |= TTBCR_TG1_4K;
> + break;
> + case SZ_16K:
> + cb->tcr[0] |= TTBCR_TG1_16K;
> + break;
> + case SZ_64K:
> + cb->tcr[0] |= TTBCR_TG1_64K;
> + break;
> + }
> +
> + cb->tcr[0] |= (64ULL - smmu->va_size) << TTBCR_T1SZ_SHIFT;
> +
> + /* Clear the existing SEP configuration */
> + cb->tcr[1] &= ~TTBCR2_SEP_UPSTREAM;
> +
> + /* Set up the sign extend bit */
> + switch (smmu->va_size) {
> + case 32:
> + cb->tcr[1] |= TTBCR2_SEP_31;
> +
[PATCH] iommu/mediatek: fix semicolon code style issue
From: Yang Wei Delete a superfluous semicolon in mtk_iommu_add_device(). Signed-off-by: Yang Wei --- drivers/iommu/mtk_iommu_v1.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iommu/mtk_iommu_v1.c b/drivers/iommu/mtk_iommu_v1.c index 7e0df67..52b01e3 100644 --- a/drivers/iommu/mtk_iommu_v1.c +++ b/drivers/iommu/mtk_iommu_v1.c @@ -474,7 +474,7 @@ static int mtk_iommu_add_device(struct device *dev) return err; } - return iommu_device_link(>iommu, dev);; + return iommu_device_link(>iommu, dev); } static void mtk_iommu_remove_device(struct device *dev) -- 2.7.4 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[RFC PATCH v1 03/15] iommu/io-pgtable: Allow TLB operations to be optional
An upcoming change to arm-smmu will add auxiliary domains that will allow
a leaf driver to create and map additional pagetables for device
specific uses. By definition aux arm-smmu domains will not be allowed
to touch the hardware directly so allow for the TLB operations for
a given pagetable configuration to be NULL just in case the caller
accidentally calls for a flush with the wrong device.
Signed-off-by: Jordan Crouse
---
drivers/iommu/io-pgtable.h | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/io-pgtable.h b/drivers/iommu/io-pgtable.h
index 47d5ae5..fbfd3c9 100644
--- a/drivers/iommu/io-pgtable.h
+++ b/drivers/iommu/io-pgtable.h
@@ -178,18 +178,22 @@ struct io_pgtable {
static inline void io_pgtable_tlb_flush_all(struct io_pgtable *iop)
{
- iop->cfg.tlb->tlb_flush_all(iop->cookie);
+ if (iop->cfg.tlb)
+ iop->cfg.tlb->tlb_flush_all(iop->cookie);
}
static inline void io_pgtable_tlb_add_flush(struct io_pgtable *iop,
unsigned long iova, size_t size, size_t granule, bool leaf)
{
- iop->cfg.tlb->tlb_add_flush(iova, size, granule, leaf, iop->cookie);
+ if (iop->cfg.tlb)
+ iop->cfg.tlb->tlb_add_flush(iova, size, granule, leaf,
+ iop->cookie);
}
static inline void io_pgtable_tlb_sync(struct io_pgtable *iop)
{
- iop->cfg.tlb->tlb_sync(iop->cookie);
+ if (iop->cfg.tlb)
+ iop->cfg.tlb->tlb_sync(iop->cookie);
}
/**
--
2.7.4
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[RFC PATCH v1 04/15] iommu: Add DOMAIN_ATTR_PTBASE
Add an attribute to return the base address of the pagetable. This is used
by auxiliary domains from arm-smmu to return the address of the pagetable
to the leaf driver so that it can set the appropriate pagetable through
it's own means.
Signed-off-by: Jordan Crouse
---
include/linux/iommu.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 3f2250b..dc60a71 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -126,6 +126,7 @@ enum iommu_attr {
DOMAIN_ATTR_NESTING,/* two stages of translation */
DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE,
DOMAIN_ATTR_SPLIT_TABLES,
+ DOMAIN_ATTR_PTBASE,
DOMAIN_ATTR_MAX,
};
--
2.7.4
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[RFC PATCH v1 02/15] iommu/arm-smmu: Add split pagetable support for arm-smmu-v2
Add support for a split pagetable (TTBR0/TTBR1) scheme for
arm-smmu-v2. If split pagetables are enabled, create a
pagetable for TTBR1 and set up the sign extension bit so
that all IOVAs with that bit set are mapped and translated
from the TTBR1 pagetable.
Signed-off-by: Jordan Crouse
---
drivers/iommu/arm-smmu-regs.h | 18 +
drivers/iommu/arm-smmu.c | 149 +
drivers/iommu/io-pgtable-arm.c | 3 +-
3 files changed, 154 insertions(+), 16 deletions(-)
diff --git a/drivers/iommu/arm-smmu-regs.h b/drivers/iommu/arm-smmu-regs.h
index a1226e4..56f9709 100644
--- a/drivers/iommu/arm-smmu-regs.h
+++ b/drivers/iommu/arm-smmu-regs.h
@@ -193,7 +193,25 @@ enum arm_smmu_s2cr_privcfg {
#define RESUME_RETRY (0 << 0)
#define RESUME_TERMINATE (1 << 0)
+#define TTBCR_EPD1 (1 << 23)
+#define TTBCR_T1SZ_SHIFT 16
+#define TTBCR_IRGN1_SHIFT 24
+#define TTBCR_ORGN1_SHIFT 26
+#define TTBCR_RGN_WBWA 1
+#define TTBCR_SH1_SHIFT28
+#define TTBCR_SH_IS3
+
+#define TTBCR_TG1_16K (1 << 30)
+#define TTBCR_TG1_4K (2 << 30)
+#define TTBCR_TG1_64K (3 << 30)
+
#define TTBCR2_SEP_SHIFT 15
+#define TTBCR2_SEP_31 (0x0 << TTBCR2_SEP_SHIFT)
+#define TTBCR2_SEP_35 (0x1 << TTBCR2_SEP_SHIFT)
+#define TTBCR2_SEP_39 (0x2 << TTBCR2_SEP_SHIFT)
+#define TTBCR2_SEP_41 (0x3 << TTBCR2_SEP_SHIFT)
+#define TTBCR2_SEP_43 (0x4 << TTBCR2_SEP_SHIFT)
+#define TTBCR2_SEP_47 (0x5 << TTBCR2_SEP_SHIFT)
#define TTBCR2_SEP_UPSTREAM(0x7 << TTBCR2_SEP_SHIFT)
#define TTBCR2_AS (1 << 4)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index af18a7e..05eb126 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -151,6 +151,7 @@ struct arm_smmu_cb {
u32 tcr[2];
u32 mair[2];
struct arm_smmu_cfg *cfg;
+ u64 split_table_mask;
};
struct arm_smmu_master_cfg {
@@ -208,6 +209,7 @@ struct arm_smmu_device {
unsigned long va_size;
unsigned long ipa_size;
unsigned long pa_size;
+ unsigned long ubs_size;
unsigned long pgsize_bitmap;
u32 num_global_irqs;
@@ -252,13 +254,14 @@ enum arm_smmu_domain_stage {
struct arm_smmu_domain {
struct arm_smmu_device *smmu;
- struct io_pgtable_ops *pgtbl_ops;
+ struct io_pgtable_ops *pgtbl_ops[2];
const struct iommu_gather_ops *tlb_ops;
struct arm_smmu_cfg cfg;
enum arm_smmu_domain_stage stage;
boolnon_strict;
struct mutexinit_mutex; /* Protects smmu pointer */
spinlock_t cb_lock; /* Serialises ATS1* ops and
TLB syncs */
+ u32 attributes;
struct iommu_domain domain;
};
@@ -618,6 +621,69 @@ static irqreturn_t arm_smmu_global_fault(int irq, void
*dev)
return IRQ_HANDLED;
}
+static void arm_smmu_init_ttbr1(struct arm_smmu_domain *smmu_domain,
+ struct io_pgtable_cfg *pgtbl_cfg)
+{
+ struct arm_smmu_device *smmu = smmu_domain->smmu;
+ struct arm_smmu_cfg *cfg = _domain->cfg;
+ struct arm_smmu_cb *cb = _domain->smmu->cbs[cfg->cbndx];
+ int pgsize = 1 << __ffs(pgtbl_cfg->pgsize_bitmap);
+
+ /* Enable speculative walks through the TTBR1 */
+ cb->tcr[0] &= ~TTBCR_EPD1;
+
+ cb->tcr[0] |= TTBCR_SH_IS << TTBCR_SH1_SHIFT;
+ cb->tcr[0] |= TTBCR_RGN_WBWA << TTBCR_IRGN1_SHIFT;
+ cb->tcr[0] |= TTBCR_RGN_WBWA << TTBCR_ORGN1_SHIFT;
+
+ switch (pgsize) {
+ case SZ_4K:
+ cb->tcr[0] |= TTBCR_TG1_4K;
+ break;
+ case SZ_16K:
+ cb->tcr[0] |= TTBCR_TG1_16K;
+ break;
+ case SZ_64K:
+ cb->tcr[0] |= TTBCR_TG1_64K;
+ break;
+ }
+
+ cb->tcr[0] |= (64ULL - smmu->va_size) << TTBCR_T1SZ_SHIFT;
+
+ /* Clear the existing SEP configuration */
+ cb->tcr[1] &= ~TTBCR2_SEP_UPSTREAM;
+
+ /* Set up the sign extend bit */
+ switch (smmu->va_size) {
+ case 32:
+ cb->tcr[1] |= TTBCR2_SEP_31;
+ cb->split_table_mask = (1ULL << 31);
+ break;
+ case 36:
+ cb->tcr[1] |= TTBCR2_SEP_35;
+ cb->split_table_mask = (1ULL << 35);
+ break;
+ case 40:
+ cb->tcr[1] |= TTBCR2_SEP_39;
+ cb->split_table_mask = (1ULL <<
[RFC PATCH v1 05/15] iommu/arm-smmu: Add auxiliary domain support for arm-smmuv2
Support the new auxiliary domain API for arm-smmuv2 to initialize and
support multiple pagetables for a SMMU device. 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; aux
domains in the IOMMU driver are only preoccupied with creating and managing
the pagetable memory.
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, );
}
After this 'domain' can be used like any other iommu domain to map and
unmap iova addresses in the pagetable. The driver/hardware can be used
to switch the pagetable according to its own specific implementation.
Signed-off-by: Jordan Crouse
---
drivers/iommu/arm-smmu.c | 135 ++-
1 file changed, 111 insertions(+), 24 deletions(-)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 05eb126..b7b508e 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -263,6 +263,8 @@ struct arm_smmu_domain {
spinlock_t cb_lock; /* Serialises ATS1* ops and
TLB syncs */
u32 attributes;
struct iommu_domain domain;
+ boolis_aux;
+ u64 ttbr0;
};
struct arm_smmu_option_prop {
@@ -874,6 +876,12 @@ static int arm_smmu_init_domain_context(struct
iommu_domain *domain,
if (!(smmu->features & ARM_SMMU_FEAT_TRANS_S2))
smmu_domain->stage = ARM_SMMU_DOMAIN_S1;
+ /* Aux domains can only be created for stage-1 tables */
+ if (smmu_domain->is_aux && smmu_domain->stage != ARM_SMMU_DOMAIN_S1) {
+ ret = -EINVAL;
+ goto out_unlock;
+ }
+
/*
* Choosing a suitable context format is even more fiddly. Until we
* grow some way for the caller to express a preference, and/or move
@@ -920,7 +928,10 @@ static int arm_smmu_init_domain_context(struct
iommu_domain *domain,
ias = min(ias, 32UL);
oas = min(oas, 32UL);
}
- smmu_domain->tlb_ops = _smmu_s1_tlb_ops;
+
+ /* aux domains shouldn't touch hardware so no TLB ops */
+ if (!smmu_domain->is_aux)
+ smmu_domain->tlb_ops = _smmu_s1_tlb_ops;
break;
case ARM_SMMU_DOMAIN_NESTED:
/*
@@ -939,32 +950,42 @@ static int arm_smmu_init_domain_context(struct
iommu_domain *domain,
ias = min(ias, 40UL);
oas = min(oas, 40UL);
}
- if (smmu->version == ARM_SMMU_V2)
- smmu_domain->tlb_ops = _smmu_s2_tlb_ops_v2;
- else
- smmu_domain->tlb_ops = _smmu_s2_tlb_ops_v1;
+
+ if (!smmu_domain->is_aux) {
+ if (smmu->version == ARM_SMMU_V2)
+ smmu_domain->tlb_ops = _smmu_s2_tlb_ops_v2;
+ else
+ smmu_domain->tlb_ops = _smmu_s2_tlb_ops_v1;
+ }
break;
default:
ret = -EINVAL;
goto out_unlock;
}
- ret = __arm_smmu_alloc_bitmap(smmu->context_map, start,
- smmu->num_context_banks);
- if (ret < 0)
- goto out_unlock;
- cfg->cbndx = ret;
- if (smmu->version < ARM_SMMU_V2) {
- cfg->irptndx = atomic_inc_return(>irptndx);
- cfg->irptndx %= smmu->num_context_irqs;
- } else {
- cfg->irptndx = cfg->cbndx;
- }
+ /*
+* Aux domains will use the same context bank assigned to the master
+* domain for the device
+*/
+ if (!smmu_domain->is_aux) {
+ ret = __arm_smmu_alloc_bitmap(smmu->context_map, start,
+ smmu->num_context_banks);
+ if (ret < 0)
+ goto out_unlock;
- if (smmu_domain->stage == ARM_SMMU_DOMAIN_S2)
- cfg->vmid = cfg->cbndx + 1 + smmu->cavium_id_base;
- else
- cfg->asid = cfg->cbndx + smmu->cavium_id_base;
+ cfg->cbndx = ret;
+ if (smmu->version < ARM_SMMU_V2) {
+ cfg->irptndx = atomic_inc_return(>irptndx);
+ cfg->irptndx %= smmu->num_context_irqs;
+ } else {
+ cfg->irptndx = cfg->cbndx;
+ }
+
+ if
[RFC PATCH v1 01/15] iommu: Add DOMAIN_ATTR_SPLIT_TABLES
Add a new domain attribute to enable split pagetable support for devices
devices that support it.
Signed-off-by: Jordan Crouse
---
include/linux/iommu.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index e90da6b..3f2250b 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -125,6 +125,7 @@ enum iommu_attr {
DOMAIN_ATTR_FSL_PAMUV1,
DOMAIN_ATTR_NESTING,/* two stages of translation */
DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE,
+ DOMAIN_ATTR_SPLIT_TABLES,
DOMAIN_ATTR_MAX,
};
--
2.7.4
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[RFC PATCH v1 00/15] drm/msm: Per-instance pagetable support
This is the latest incarnation of per-instance pagetable support for the MSM GPU driver. Some of these have been seen before, most recently [1]. Per-instance pagetables allow the target GPU driver to create and manage an individual pagetable for each file descriptor instance and switch between them asynchronously using the GPU to reprogram the pagetable registers on the fly. This is accomplished in this series by taking advantage of the multiple IOMMU domain API from Lu Baolu [2] and all these patches are based on that patch. This series is split into three parts: Part one adds support for split pagetables. These are the same patches from the previous attempts [1]. Split pagetables allow the hardware to switch out the lower pagetable (TTBR0) without affecting the global allocations in the upper one (TTBR1). Part 2 adds aux domain support for arm-smmu-v2. New aux domains create a new pagetable but do not touch the underlying hardware. The target driver uses the new aux domain to map and unmap memory through the usual mechanisms. The final part is the support in the GPU driver to enable 64 bit addressing for a5xx and a6xx, set up the support for split pagetables, create new per-instance pagetables for a new instance and submit the GPU command to switch the pagetable at the appropriate time. This is compile tested but I haven't done much target testing as of yet. I wanted to get this out in the world for debate while we work on fixing up the minor issues. In particular, I want to make sure that this fits with the current thinking about how aux domains should look and feel. [1] https://patchwork.freedesktop.org/series/43447/ [2] https://patchwork.kernel.org/patch/10825061/ Jordan Crouse (15): iommu: Add DOMAIN_ATTR_SPLIT_TABLES iommu/arm-smmu: Add split pagetable support for arm-smmu-v2 iommu/io-pgtable: Allow TLB operations to be optional iommu: Add DOMAIN_ATTR_PTBASE iommu/arm-smmu: Add auxiliary domain support for arm-smmuv2 drm/msm/adreno: Enable 64 bit mode by default on a5xx and a6xx targets drm/msm: Print all 64 bits of the faulting IOMMU address drm/msm: Pass the MMU domain index in struct msm_file_private drm/msm/gpu: Move address space setup to the GPU targets drm/msm: Add support for IOMMU auxiliary domains drm/msm: Add a helper function for a per-instance address space drm/msm: Add support to create target specific address spaces drm/msm/gpu: Add ttbr0 to the memptrs drm/msm/a6xx: Support per-instance pagetables drm/msm/a5xx: Support per-instance pagetables drivers/gpu/drm/msm/adreno/a2xx_gpu.c | 37 ++-- drivers/gpu/drm/msm/adreno/a3xx_gpu.c | 50 -- drivers/gpu/drm/msm/adreno/a4xx_gpu.c | 51 -- drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 163 +- drivers/gpu/drm/msm/adreno/a5xx_gpu.h | 19 ++ drivers/gpu/drm/msm/adreno/a5xx_preempt.c | 70 ++-- drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 167 +- drivers/gpu/drm/msm/adreno/a6xx_gpu.h | 1 + drivers/gpu/drm/msm/adreno/adreno_gpu.c | 7 - drivers/gpu/drm/msm/msm_drv.c | 25 ++- drivers/gpu/drm/msm/msm_drv.h | 5 + drivers/gpu/drm/msm/msm_gem.h | 2 + drivers/gpu/drm/msm/msm_gem_submit.c | 13 +- drivers/gpu/drm/msm/msm_gem_vma.c | 53 +++--- drivers/gpu/drm/msm/msm_gpu.c | 59 +-- drivers/gpu/drm/msm/msm_gpu.h | 3 + drivers/gpu/drm/msm/msm_iommu.c | 99 ++- drivers/gpu/drm/msm/msm_mmu.h | 4 + drivers/gpu/drm/msm/msm_ringbuffer.h | 1 + drivers/iommu/arm-smmu-regs.h | 18 ++ drivers/iommu/arm-smmu.c | 278 ++ drivers/iommu/io-pgtable-arm.c| 3 +- drivers/iommu/io-pgtable.h| 10 +- include/linux/iommu.h | 2 + 24 files changed, 952 insertions(+), 188 deletions(-) -- 2.7.4 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2] iommu/arm-smmu: Break insecure users by disabling bypass by default
If you're bisecting why your peripherals stopped working, it's probably this CL. Specifically if you see this in your dmesg: Unexpected global fault, this could be serious ...then it's almost certainly this CL. Running your IOMMU-enabled peripherals with the IOMMU in bypass mode is insecure and effectively disables the protection they provide. There are few reasons to allow unmatched stream bypass, and even fewer good ones. This patch starts the transition over to make it much harder to run your system insecurely. Expected steps: 1. By default disable bypass (so anyone insecure will notice) but make it easy for someone to re-enable bypass with just a KConfig change. That's this patch. 2. After people have had a little time to come to grips with the fact that they need to set their IOMMUs properly and have had time to dig into how to do this, the KConfig will be eliminated and bypass will simply be disabled. Folks who are truly upset and still haven't fixed their system can either figure out how to add 'arm-smmu.disable_bypass=n' to their command line or revert the patch in their own private kernel. Of course these folks will be less secure. Suggested-by: Robin Murphy Signed-off-by: Douglas Anderson --- Changes in v2: - Flipped default to 'yes' and changed comments a lot. drivers/iommu/Kconfig| 25 + drivers/iommu/arm-smmu.c | 3 ++- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig index 1ca1fa107b21..a4210672804a 100644 --- a/drivers/iommu/Kconfig +++ b/drivers/iommu/Kconfig @@ -359,6 +359,31 @@ config ARM_SMMU Say Y here if your SoC includes an IOMMU device implementing the ARM SMMU architecture. +config ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT + bool "Default to disabling bypass on ARM SMMU v1 and v2" + depends on ARM_SMMU + default y + help + Say Y here to (by default) disable bypass streams such that + incoming transactions from devices that are not attached to + an iommu domain will report an abort back to the device and + will not be allowed to pass through the SMMU. + + Any old kernels that existed before this KConfig was + introduced would default to _allowing_ bypass (AKA the + equivalent of NO for this config). However the default for + this option is YES because the old behavior is insecure. + + There are few reasons to allow unmatched stream bypass, and + even fewer good ones. If saying YES here breaks your board + you should work on fixing your board. This KConfig option + is expected to be removed in the future and we'll simply + hardcode the bypass disable in the code. + + NOTE: the kernel command line parameter + 'arm-smmu.disable_bypass' will continue to override this + config. + config ARM_SMMU_V3 bool "ARM Ltd. System MMU Version 3 (SMMUv3) Support" depends on ARM64 diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index 045d93884164..930c07635956 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -110,7 +110,8 @@ static int force_stage; module_param(force_stage, int, S_IRUGO); MODULE_PARM_DESC(force_stage, "Force SMMU mappings to be installed at a particular stage of translation. A value of '1' or '2' forces the corresponding stage. All other values are ignored (i.e. no stage is forced). Note that selecting a specific stage will disable support for nested translation."); -static bool disable_bypass; +static bool disable_bypass = + IS_ENABLED(CONFIG_ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT); module_param(disable_bypass, bool, S_IRUGO); MODULE_PARM_DESC(disable_bypass, "Disable bypass streams such that incoming transactions from devices that are not attached to an iommu domain will report an abort back to the device and will not be allowed to pass through the SMMU."); -- 2.21.0.352.gf09ad66450-goog ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/arm-smmu: Allow disabling bypass via kernel config
Hi, On Fri, Feb 15, 2019 at 2:37 PM Rob Clark wrote: > > On Thu, Feb 14, 2019 at 7:40 PM Doug Anderson wrote: > > > > Hi, > > > > On Thu, Feb 14, 2019 at 1:32 PM Robin Murphy wrote: > > > > > > Hi Doug, > > > > > > On 2019-02-14 8:44 pm, Douglas Anderson wrote: > > > > Right now the only way to disable the iommu bypass for the ARM SMMU is > > > > with the kernel command line parameter 'arm-smmu.disable_bypass'. > > > > > > > > In general kernel command line parameters make sense for things that > > > > someone would like to tweak without rebuilding the kernel or for very > > > > basic communication between the bootloader and the kernel, but are > > > > awkward for other things. Specifically: > > > > * Human parsing of the kernel command line can be difficult since it's > > > >just a big runon space separated line of text. > > > > * If every bit of the system was configured via the kernel command > > > >line the kernel command line would get very large and even more > > > >unwieldly. > > > > * Typically there are not easy ways in build systems to adjust the > > > >kernel command line for config-like options. > > > > > > > > Let's introduce a new config option that allows us to disable the > > > > iommu bypass without affecting the existing default nor the existing > > > > ability to adjust the configuration via kernel command line. > > > > > > I say let's just flip the default - for a while now it's been one of > > > those "oh yeah, we should probably do that" things that gets instantly > > > forgotten again, so some 3rd-party demand is plenty to convince me :) > > > > > > There are few reasons to allow unmatched stream bypass, and even fewer > > > good ones, so I'd be happy to shift the command-line burden over to the > > > esoteric cases at this point, and consider the config option in future > > > if anyone from that camp pops up and screams hard enough. > > > > Sure, I can submit that patch if we want. I presume I'll get lots of > > screaming but I'm used to that. ;-) > > > > ...specifically I found that when I turned on "disably bypass" on my > > board (sdm845-cheza, which is not yet upstream) that a bunch of things > > that used to work broke. That's a good thing because all the things > > that broke need to be fixed properly (by adding the IOMMUs) but > > presumably my board is not special in relying on the old insecure > > behavior. > > So one niggly bit, beyond the drivers that aren't but should be using > iommu, is the case where bootloader lights up the display. We > actually still have a lot of work to do for this (in that clk and > genpd drivers need to be aware of handover, in addition to just > iommu).. But I'd rather not make a hard problem harder just yet. > > (it gets worse, afaict so far on the windows-arm laptops since in that > case uefi/edk is actually taking the iommu out of bypass and things go > badly when arm-smmu disables clks after probe..) > > But I might recommend making the default a kconfig option for now, so > in the distro kernel case, where display driver is a kernel module, > you aren't making a hard problem harder. For cases where bootloader > isn't enabling display, things are much easier, and I think we could > switch the default sooner. But I fear for cases where bootloader is > enabling display it is a much harder problem :-( OK, so after thinking about this and playing with it a bit, here's what I'm planning to do: I'm going to send out a v2 of my patch where I basically just flip it to "default y". Originally I was going to post a patch like Robin suggested that just changed the default in the code without introducing a KConfig option. Then I looked at all the things I needed to do on my own board to get things working again once the IOMMU disallowed bypass and I just couldn't bring myself to respond to everyone else I was about to break "now figure out how to add a new kernel command line option until you fix this more correctly". In my mind a change that by default breaks all these insecure people (effectively notifying them that they were insecure) but that allows them to get back to the old state quickly seems like a good first step. In my commit message I'll mention that in a kernel version or two we should probably fully take out the KConfig option since people will have (presumably) had a chance to wean themselves off it. One last thought on all of this is the question about the whole "device tree as a stable ABI". Presumably we're are supposed to make some attempt to be able to run old device trees and presumably those old device trees will break because the proper way to hook up the IOMMU is by specifying it in the device tree. Thus we shouldn't be too quick to totally break all those old devices and we should make sure that there is some easy path if someone comes up with an old device tree that is "unfixable". ;-) Robin: hopefully it's OK that I'm co-opting a bit of your wording and adding you as a "Suggested-by".
[git pull] IOMMU Fixes for Linux v5.0-rc8
Hi Linus, The following changes since commit 8950dcd83ae7d62bdc2a60507949acebd85399f2: iommu/vt-d: Leave scalable mode default off (2019-01-30 17:23:58 +0100) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git tags/iommu-fix-v5.0-rc8 for you to fetch changes up to cffaaf0c816238c45cd2d06913476c83eb50f682: iommu/dmar: Fix buffer overflow during PCI bus notification (2019-02-26 11:24:37 +0100) IOMMU Fix for Linux v5.0-rc8 One important patch: - Fix for a memory corruption issue in the Intel VT-d driver that triggers on hardware with deep PCI hierarchies Julia Cartwright (1): iommu/dmar: Fix buffer overflow during PCI bus notification drivers/iommu/dmar.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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: io-pgtable: Add ARM Mali midgard MMU page table format
On 27/02/2019 23:22, Rob Herring wrote:
ARM Mali midgard GPU page tables are similar to standard 64-bit stage 1
page tables, but have a few differences. Add a new format type to
represent the format. The input address size is 48-bits and the output
address size is 40-bits (and possibly less?). Note that the later
bifrost GPUs follow the standard 64-bit stage 1 format.
The differences in the format compared to 64-bit stage 1 format are:
The 3rd level page entry bits are 0x1 instead of 0x3 for page entries.
The access flags are not read-only and unprivileged, but read and write.
This is similar to stage 2 entries, but the memory attributes field matches
stage 1 being an index.
The nG bit is not set by the vendor driver. This one didn't seem to matter,
but we'll keep it aligned to the vendor driver.
Cc: Will Deacon
Cc: Robin Murphy
Cc: Joerg Roedel
Cc: linux-arm-ker...@lists.infradead.org
Cc: iommu@lists.linux-foundation.org
Signed-off-by: Rob Herring
---
Robin, Hopefully this is what you had in mind.
Getting there, for sure :)
drivers/iommu/io-pgtable-arm.c | 70 +++---
drivers/iommu/io-pgtable.c | 1 +
include/linux/io-pgtable.h | 2 +
3 files changed, 59 insertions(+), 14 deletions(-)
diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
index d3700ec15cbd..84beea1f47a7 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c
@@ -180,11 +180,6 @@
#define iopte_prot(pte) ((pte) & ARM_LPAE_PTE_ATTR_MASK)
-#define iopte_leaf(pte,l) \
- (l == (ARM_LPAE_MAX_LEVELS - 1) ? \
- (iopte_type(pte,l) == ARM_LPAE_PTE_TYPE_PAGE) : \
- (iopte_type(pte,l) == ARM_LPAE_PTE_TYPE_BLOCK))
-
struct arm_lpae_io_pgtable {
struct io_pgtable iop;
@@ -198,6 +193,14 @@ struct arm_lpae_io_pgtable {
typedef u64 arm_lpae_iopte;
+static inline bool iopte_leaf(arm_lpae_iopte pte, int l, enum io_pgtable_fmt fmt)
+{
+ if ((l == (ARM_LPAE_MAX_LEVELS - 1)) && (fmt != ARM_MALI_LPAE))
+ return iopte_type(pte,l) == ARM_LPAE_PTE_TYPE_PAGE;
+
+ return iopte_type(pte,l) == ARM_LPAE_PTE_TYPE_BLOCK;
+}
+
static arm_lpae_iopte paddr_to_iopte(phys_addr_t paddr,
struct arm_lpae_io_pgtable *data)
{
@@ -304,11 +307,14 @@ static void __arm_lpae_init_pte(struct
arm_lpae_io_pgtable *data,
pte |= ARM_LPAE_PTE_NS;
if (lvl == ARM_LPAE_MAX_LEVELS - 1)
- pte |= ARM_LPAE_PTE_TYPE_PAGE;
+ pte |= (data->iop.fmt == ARM_MALI_LPAE) ?
+ ARM_LPAE_PTE_TYPE_BLOCK : ARM_LPAE_PTE_TYPE_PAGE;
Yuck at that ternary... Made worse by the previous hunk which proves you
already know the nice reasonable way to structure this logic ;)
else
pte |= ARM_LPAE_PTE_TYPE_BLOCK;
- pte |= ARM_LPAE_PTE_AF | ARM_LPAE_PTE_SH_IS;
+ if (data->iop.fmt != ARM_MALI_LPAE)
+ pte |= ARM_LPAE_PTE_AF;
So ENTRY_ACCESS_BIT is something different from the VMSA access flag then?
+ pte |= ARM_LPAE_PTE_SH_IS;
pte |= paddr_to_iopte(paddr, data);
__arm_lpae_set_pte(ptep, pte, >iop.cfg);
@@ -321,7 +327,7 @@ static int arm_lpae_init_pte(struct arm_lpae_io_pgtable
*data,
{
arm_lpae_iopte pte = *ptep;
- if (iopte_leaf(pte, lvl)) {
+ if (iopte_leaf(pte, lvl, data->iop.fmt)) {
/* We require an unmap first */
WARN_ON(!selftest_running);
return -EEXIST;
@@ -409,7 +415,7 @@ static int __arm_lpae_map(struct arm_lpae_io_pgtable *data,
unsigned long iova,
__arm_lpae_sync_pte(ptep, cfg);
}
- if (pte && !iopte_leaf(pte, lvl)) {
+ if (pte && !iopte_leaf(pte, lvl, data->iop.fmt)) {
cptep = iopte_deref(pte, data);
} else if (pte) {
/* We require an unmap first */
@@ -426,7 +432,22 @@ static arm_lpae_iopte arm_lpae_prot_to_pte(struct
arm_lpae_io_pgtable *data,
{
arm_lpae_iopte pte;
- if (data->iop.fmt == ARM_64_LPAE_S1 ||
+ if (data->iop.fmt == ARM_MALI_LPAE) {
+ pte = 0;
+
+ if (prot & IOMMU_WRITE)
+ pte |= ARM_LPAE_PTE_AP_RDONLY;
This one in particular I will never be able to look at without
instinctively thinking "hang on, how did I ever let that bug slip
through?"...
+
+ if (prot & IOMMU_READ)
+ pte |= ARM_LPAE_PTE_AP_UNPRIV;
...while this one only looks utterly insane. Can we please at least use
the stage 2 permission definitions for these so that they're actually
readable.
+
+ if (prot & IOMMU_MMIO)
+ pte |= (ARM_LPAE_MAIR_ATTR_IDX_DEV
+ << ARM_LPAE_PTE_ATTRINDX_SHIFT);
+ else if (prot & IOMMU_CACHE)
+ pte |= (ARM_LPAE_MAIR_ATTR_IDX_CACHE
+
Re: [PATCH RFC 1/1] iommu: set the default iommu-dma mode as non-strict
Hi Leizhen, On 01/03/2019 04:44, Leizhen (ThunderTown) wrote: > > > On 2019/2/26 20:36, Hanjun Guo wrote: >> Hi Jean, >> >> On 2019/1/31 22:55, Jean-Philippe Brucker wrote: >>> Hi, >>> >>> On 31/01/2019 13:52, Zhen Lei wrote: Currently, many peripherals are faster than before. For example, the top speed of the older netcard is 10Gb/s, and now it's more than 25Gb/s. But when iommu page-table mapping enabled, it's hard to reach the top speed in strict mode, because of frequently map and unmap operations. In order to keep abreast of the times, I think it's better to set non-strict as default. >>> >>> Most users won't be aware of this relaxation and will have their system >>> vulnerable to e.g. thunderbolt hotplug. See for example 4.3 Deferred >>> Invalidation in >>> http://www.cs.technion.ac.il/users/wwwb/cgi-bin/tr-get.cgi/2018/MSC/MSC-2018-21.pdf > Hi Jean, > >In fact, we have discussed the vulnerable of deferred invalidation before > upstream > the non-strict patches. The attacks maybe possible because of an untrusted > device or > the mistake of the device driver. And we limited the VFIO to still use strict > mode. >As mentioned in the pdf, limit the freed memory with deferred invalidation > only to > be reused by the device, can mitigate the vulnerability. But it's too hard to > implement > it now. >A compromise maybe we only apply non-strict to (1) dma_free_coherent, > because the > memory is controlled by DMA common module, so we can make the memory to be > freed after > the global invalidation in the timer handler. (2) And provide some new APIs > related to > iommu_unmap_page/sg, these new APIs deferred invalidation. And the candiate > device > drivers update the APIs if they want to improve performance. (3) Make sure > that only > the trusted devices and trusted drivers can apply (1) and (2). For example, > the driver > must be built into kernel Image. Do we have a notion of untrusted kernel drivers? A userspace driver (VFIO) is untrusted, ok. But a malicious driver loaded into the kernel address space would have much easier ways to corrupt the system than to exploit lazy mode... For (3), I agree that we should at least disallow lazy mode if pci_dev->untrusted is set. At the moment it means that we require the strictest IOMMU configuration for external-facing PCI ports, but it can be extended to blacklist other vulnerable devices or locations. If you do (3) then maybe we don't need (1) and (2), which require a tonne of work in the DMA and IOMMU layers (but would certainly be nice to see, since it would also help handle ATS invalidation timeouts) Thanks, Jean >So that some high-end trusted devices use non-strict mode, and keep others > still using > strict mode. The drivers who want to use non-strict mode, should change to > use new APIs > by themselves. > > >>> >>> Why not keep the policy to secure by default, as we do for >>> iommu.passthrough? And maybe add something similar to >>> CONFIG_IOMMU_DEFAULT_PASSTRHOUGH? It's easy enough for experts to pass a >>> command-line argument or change the default config. >> >> Sorry for the late reply, it was Chinese new year, and we had a long >> discussion >> internally, we are fine to add a Kconfig but not sure OS vendors will set it >> to default y. >> >> OS vendors seems not happy to pass a command-line argument, to be honest, >> this is our motivation to enable non-strict as default. Hope OS vendors >> can see this email thread, and give some input here. >> >> Thanks >> Hanjun >> >> >> . >> > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 0/4] iommu/vt-d: Several fixes for 5.1
On Fri, Mar 01, 2019 at 11:23:09AM +0800, Lu Baolu wrote: > Hi Joerg, > > This includes several small fixes. All are stable kernel irrelated. > Please consider it for 5.1-rc1. > > Best regards, > Lu Baolu > > Lu Baolu (4): > iommu/vt-d: Disable ATS support on untrusted devices > iommu/vt-d: Set context field after value initialized > iommu/vt-d: Fix NULL pointer reference in intel_svm_bind_mm() > iommu/vt-d: Get domain ID before clear pasid entry Applied, thanks. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/5] iommu/arm-smmu-v3: make smmu can be enabled in kdump kernel
It seems that the picture is too big, I change it from jpg to png.
On 2019/3/1 17:02, Leizhen (ThunderTown) wrote:
> Hi All,
> I drew a flowchart, hope this can help you to understand my method.
>
> On 2019/2/19 15:54, Zhen Lei wrote:
>> To reduce the risk of further crash, the device_shutdown() was not called
>> by the first kernel. That means some devices may still working in the
>> secondary kernel. For example, a netcard may still using ring buffer to
>> receive the broadcast messages in the kdump kernel. No events are reported
>> utill the related smmu reinitialized by the kdump kernel.
>>
>> commit b63b3439b856 ("iommu/arm-smmu-v3: Abort all transactions if SMMU is
>> enabled in kdump kernel") set SMMU_GBPA.ABORT to prevent the unexpected
>> devices accessing, but it also prevent the devices accessing which we
>> needed, like hard disk, netcard.
>>
>> In fact, we can use STE.config=0b000 to abort the unexpected devices
>> accessing only. As below:
>> 1. In the first kernel, all buffers used by the "unexpected" devices are
>>correctly mapped, and it will not be used by the secondary kernel
>>because the latter has its dedicated reserved memory.
>> 2. In the secondary kernel, set SMMU_GBPA.ABORT=1 before "disable smmu".
>> 3. In the secondary kernel, after the smmu was disabled, preset all
>>STE.config=0b000. For 2-level Stream Table, make all L1STD.l2ptr
>>pointer to a dummy L2ST. The dummy L2ST is shared by all L1STDs.
>> 4. In the secondary kernel, enable smmu. For the needed devices, allocate
>>new L2STs accordingly.
>>
>> For phase 1 and 2, the unexpected devices base the old mapping access
>> memory, it will not corrupt others. For phase 3, SMMU_GBPA abort it. For
>> phase 4 STE abort it.
>>
>> Fixes: commit b63b3439b856 ("iommu/arm-smmu-v3: Abort all transactions ...")
>> Signed-off-by: Zhen Lei
>> ---
>> drivers/iommu/arm-smmu-v3.c | 72
>> -
>> 1 file changed, 51 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
>> index 2072897..c3c4ff2 100644
>> --- a/drivers/iommu/arm-smmu-v3.c
>> +++ b/drivers/iommu/arm-smmu-v3.c
>> @@ -1219,35 +1219,57 @@ static void arm_smmu_init_bypass_stes(u64 *strtab,
>> unsigned int nent)
>> }
>> }
>>
>> -static int arm_smmu_init_l2_strtab(struct arm_smmu_device *smmu, u32 sid)
>> +static int __arm_smmu_init_l2_strtab(struct arm_smmu_device *smmu, u32 sid,
>> + struct arm_smmu_strtab_l1_desc *desc)
>> {
>> -size_t size;
>> void *strtab;
>> struct arm_smmu_strtab_cfg *cfg = >strtab_cfg;
>> -struct arm_smmu_strtab_l1_desc *desc = >l1_desc[sid >>
>> STRTAB_SPLIT];
>>
>> -if (desc->l2ptr)
>> -return 0;
>> -
>> -size = 1 << (STRTAB_SPLIT + ilog2(STRTAB_STE_DWORDS) + 3);
>> strtab = >strtab[(sid >> STRTAB_SPLIT) * STRTAB_L1_DESC_DWORDS];
>>
>> -desc->span = STRTAB_SPLIT + 1;
>> -desc->l2ptr = dmam_alloc_coherent(smmu->dev, size, >l2ptr_dma,
>> - GFP_KERNEL | __GFP_ZERO);
>> if (!desc->l2ptr) {
>> -dev_err(smmu->dev,
>> -"failed to allocate l2 stream table for SID %u\n",
>> -sid);
>> -return -ENOMEM;
>> +size_t size;
>> +
>> +size = 1 << (STRTAB_SPLIT + ilog2(STRTAB_STE_DWORDS) + 3);
>> +desc->l2ptr = dmam_alloc_coherent(smmu->dev, size,
>> + >l2ptr_dma,
>> + GFP_KERNEL | __GFP_ZERO);
>> +if (!desc->l2ptr) {
>> +dev_err(smmu->dev,
>> +"failed to allocate l2 stream table for SID
>> %u\n",
>> +sid);
>> +return -ENOMEM;
>> +}
>> +
>> +desc->span = STRTAB_SPLIT + 1;
>> +arm_smmu_init_bypass_stes(desc->l2ptr, 1 << STRTAB_SPLIT);
>> }
>>
>> -arm_smmu_init_bypass_stes(desc->l2ptr, 1 << STRTAB_SPLIT);
>> arm_smmu_write_strtab_l1_desc(strtab, desc);
>> +return 0;
>> +}
>> +
>> +static int arm_smmu_init_l2_strtab(struct arm_smmu_device *smmu, u32 sid)
>> +{
>> +int ret;
>> +struct arm_smmu_strtab_cfg *cfg = >strtab_cfg;
>> +struct arm_smmu_strtab_l1_desc *desc = >l1_desc[sid >>
>> STRTAB_SPLIT];
>> +
>> +ret = __arm_smmu_init_l2_strtab(smmu, sid, desc);
>> +if (ret)
>> +return ret;
>> +
>> arm_smmu_sync_std_for_sid(smmu, sid);
>> return 0;
>> }
>>
>> +static int arm_smmu_init_dummy_l2_strtab(struct arm_smmu_device *smmu, u32
>> sid)
>> +{
>> +static struct arm_smmu_strtab_l1_desc dummy_desc;
>> +
>> +return __arm_smmu_init_l2_strtab(smmu, sid, _desc);
>> +}
>> +
>> /* IRQ and event handlers */
>> static irqreturn_t arm_smmu_evtq_thread(int irq, void *dev)
>> {
>> @@ -2150,8 +2172,12 @@ static int
Re: [PATCH] iommu/mediatek: fix semicolon code style issue
On Thu, Feb 28, 2019 at 10:45:01PM +0800, Yang Wei wrote: > From: Yang Wei > > Delete a superfluous semicolon in mtk_iommu_add_device(). > > Signed-off-by: Yang Wei > --- > drivers/iommu/mtk_iommu_v1.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Applied, thanks. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
