Re: [RESEND PATCH v2 2/3] iommu/io-pgtable-arm: Add support for AARCH64 split pagetables
On Wed, Jul 10, 2019 at 05:45:37PM +0100, Robin Murphy wrote:
> Hi Jordan,
>
> On 08/07/2019 20:00, Jordan Crouse wrote:
> >Add a new sub-format ARM_64_LPAE_SPLIT_S1 to create and set up split
> >pagetables (TTBR0 and TTBR1). The initialization function sets up the
> >correct va_size and sign extension bits and programs the TCR registers.
> >Split pagetable formats use their own own map/unmap wrappers to ensure
> >that the correct pagetable is selected based on the incoming iova but
> >most of the heavy lifting is common to the other formats.
>
> I'm somewhat concerned that this implementation is very closely tied to the
> current Adreno use-case, and won't easily generalise in future to other
> potential TTBR1 uses which have been tossed around, like
> SMMUv3-without-substream-ID.
I haven't heard about these use-cases. I figured v3 would be all PASID all the
time. If you have details, I can see if I can avoid painting v3 into a corner.
> Furthermore, even for the Adreno pretend-PASID case it appears to be a bit
> too fragile for comfort - given that a DOMAIN_ATTR_SPLIT_TABLES domain
> doesn't look any different from a regular one from the users' point of view,
> what's to stop them making "without PASID" mappings in the lower half of the
> address space, and thus unwittingly pulling the rug out from under their own
> feet upon attaching an aux domain? In fact allocating a TTBR0 table at all
> for the main domain seems like little more than a waste of memory.
That makes sense. Would it work better if we made a type ARM_64_LPAE_TTBR1_S1
that only allocated memory for TTBR1 and set TBR.EDP0 to trigger faults for
TTBR0?
Then we could get rid of most of the extra stuff in io-pgtable-arm.c and I think
that would meet most of your concerns.
Jordan
> >
> >Signed-off-by: Jordan Crouse
> >---
> >
> > drivers/iommu/io-pgtable-arm.c | 261
> > +
> > drivers/iommu/io-pgtable.c | 1 +
> > include/linux/io-pgtable.h | 2 +
> > 3 files changed, 240 insertions(+), 24 deletions(-)
> >
> >diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
> >index 161a7d56..aec35e5 100644
> >--- a/drivers/iommu/io-pgtable-arm.c
> >+++ b/drivers/iommu/io-pgtable-arm.c
> >@@ -118,7 +118,12 @@
> > #define ARM_LPAE_TCR_TG0_64K (1 << 14)
> > #define ARM_LPAE_TCR_TG0_16K (2 << 14)
> >+#define ARM_LPAE_TCR_TG1_4K (0 << 30)
> >+#define ARM_LPAE_TCR_TG1_64K(1 << 30)
> >+#define ARM_LPAE_TCR_TG1_16K(2 << 30)
> >+
> > #define ARM_LPAE_TCR_SH0_SHIFT 12
> >+#define ARM_LPAE_TCR_SH1_SHIFT 28
> > #define ARM_LPAE_TCR_SH0_MASK 0x3
> > #define ARM_LPAE_TCR_SH_NS 0
> > #define ARM_LPAE_TCR_SH_OS 2
> >@@ -126,6 +131,8 @@
> > #define ARM_LPAE_TCR_ORGN0_SHIFT 10
> > #define ARM_LPAE_TCR_IRGN0_SHIFT 8
> >+#define ARM_LPAE_TCR_ORGN1_SHIFT26
> >+#define ARM_LPAE_TCR_IRGN1_SHIFT24
> > #define ARM_LPAE_TCR_RGN_MASK 0x3
> > #define ARM_LPAE_TCR_RGN_NC0
> > #define ARM_LPAE_TCR_RGN_WBWA 1
> >@@ -136,6 +143,7 @@
> > #define ARM_LPAE_TCR_SL0_MASK 0x3
> > #define ARM_LPAE_TCR_T0SZ_SHIFT0
> >+#define ARM_LPAE_TCR_T1SZ_SHIFT 16
> > #define ARM_LPAE_TCR_SZ_MASK 0xf
> > #define ARM_LPAE_TCR_PS_SHIFT 16
> >@@ -152,6 +160,14 @@
> > #define ARM_LPAE_TCR_PS_48_BIT 0x5ULL
> > #define ARM_LPAE_TCR_PS_52_BIT 0x6ULL
> >+#define ARM_LPAE_TCR_SEP_SHIFT 47
> >+#define ARM_LPAE_TCR_SEP_31 (0x0ULL << ARM_LPAE_TCR_SEP_SHIFT)
> >+#define ARM_LPAE_TCR_SEP_35 (0x1ULL << ARM_LPAE_TCR_SEP_SHIFT)
> >+#define ARM_LPAE_TCR_SEP_39 (0x2ULL << ARM_LPAE_TCR_SEP_SHIFT)
> >+#define ARM_LPAE_TCR_SEP_41 (0x3ULL << ARM_LPAE_TCR_SEP_SHIFT)
> >+#define ARM_LPAE_TCR_SEP_43 (0x4ULL << ARM_LPAE_TCR_SEP_SHIFT)
> >+#define ARM_LPAE_TCR_SEP_UPSTREAM (0x7ULL << ARM_LPAE_TCR_SEP_SHIFT)
>
> This is a specific detail of SMMUv2, and nothing to do with the LPAE/AArch64
> VMSA formats.
>
> >+
> > #define ARM_LPAE_MAIR_ATTR_SHIFT(n)((n) << 3)
> > #define ARM_LPAE_MAIR_ATTR_MASK0xff
> > #define ARM_LPAE_MAIR_ATTR_DEVICE 0x04
> >@@ -179,11 +195,12 @@ struct arm_lpae_io_pgtable {
> > struct io_pgtable iop;
> > int levels;
> >+u32 sep;
> > size_t pgd_size;
> > unsigned long pg_shift;
> > unsigned long bits_per_level;
> >-void*pgd;
> >+void*pgd[2];
> > };
> > typedef u64 arm_lpae_iopte;
> >@@ -426,7 +443,8 @@ 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 ||
> >-data->iop.fmt == ARM_32_LPAE_S1) {
> >+data->io
Re: [RESEND PATCH v2 2/3] iommu/io-pgtable-arm: Add support for AARCH64 split pagetables
Hi Jordan,
On 08/07/2019 20:00, Jordan Crouse wrote:
Add a new sub-format ARM_64_LPAE_SPLIT_S1 to create and set up split
pagetables (TTBR0 and TTBR1). The initialization function sets up the
correct va_size and sign extension bits and programs the TCR registers.
Split pagetable formats use their own own map/unmap wrappers to ensure
that the correct pagetable is selected based on the incoming iova but
most of the heavy lifting is common to the other formats.
I'm somewhat concerned that this implementation is very closely tied to
the current Adreno use-case, and won't easily generalise in future to
other potential TTBR1 uses which have been tossed around, like
SMMUv3-without-substream-ID.
Furthermore, even for the Adreno pretend-PASID case it appears to be a
bit too fragile for comfort - given that a DOMAIN_ATTR_SPLIT_TABLES
domain doesn't look any different from a regular one from the users'
point of view, what's to stop them making "without PASID" mappings in
the lower half of the address space, and thus unwittingly pulling the
rug out from under their own feet upon attaching an aux domain? In fact
allocating a TTBR0 table at all for the main domain seems like little
more than a waste of memory.
Signed-off-by: Jordan Crouse
---
drivers/iommu/io-pgtable-arm.c | 261 +
drivers/iommu/io-pgtable.c | 1 +
include/linux/io-pgtable.h | 2 +
3 files changed, 240 insertions(+), 24 deletions(-)
diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
index 161a7d56..aec35e5 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c
@@ -118,7 +118,12 @@
#define ARM_LPAE_TCR_TG0_64K (1 << 14)
#define ARM_LPAE_TCR_TG0_16K (2 << 14)
+#define ARM_LPAE_TCR_TG1_4K (0 << 30)
+#define ARM_LPAE_TCR_TG1_64K (1 << 30)
+#define ARM_LPAE_TCR_TG1_16K (2 << 30)
+
#define ARM_LPAE_TCR_SH0_SHIFT12
+#define ARM_LPAE_TCR_SH1_SHIFT 28
#define ARM_LPAE_TCR_SH0_MASK 0x3
#define ARM_LPAE_TCR_SH_NS0
#define ARM_LPAE_TCR_SH_OS2
@@ -126,6 +131,8 @@
#define ARM_LPAE_TCR_ORGN0_SHIFT 10
#define ARM_LPAE_TCR_IRGN0_SHIFT 8
+#define ARM_LPAE_TCR_ORGN1_SHIFT 26
+#define ARM_LPAE_TCR_IRGN1_SHIFT 24
#define ARM_LPAE_TCR_RGN_MASK 0x3
#define ARM_LPAE_TCR_RGN_NC 0
#define ARM_LPAE_TCR_RGN_WBWA 1
@@ -136,6 +143,7 @@
#define ARM_LPAE_TCR_SL0_MASK 0x3
#define ARM_LPAE_TCR_T0SZ_SHIFT 0
+#define ARM_LPAE_TCR_T1SZ_SHIFT16
#define ARM_LPAE_TCR_SZ_MASK 0xf
#define ARM_LPAE_TCR_PS_SHIFT 16
@@ -152,6 +160,14 @@
#define ARM_LPAE_TCR_PS_48_BIT0x5ULL
#define ARM_LPAE_TCR_PS_52_BIT0x6ULL
+#define ARM_LPAE_TCR_SEP_SHIFT 47
+#define ARM_LPAE_TCR_SEP_31(0x0ULL << ARM_LPAE_TCR_SEP_SHIFT)
+#define ARM_LPAE_TCR_SEP_35(0x1ULL << ARM_LPAE_TCR_SEP_SHIFT)
+#define ARM_LPAE_TCR_SEP_39(0x2ULL << ARM_LPAE_TCR_SEP_SHIFT)
+#define ARM_LPAE_TCR_SEP_41(0x3ULL << ARM_LPAE_TCR_SEP_SHIFT)
+#define ARM_LPAE_TCR_SEP_43(0x4ULL << ARM_LPAE_TCR_SEP_SHIFT)
+#define ARM_LPAE_TCR_SEP_UPSTREAM (0x7ULL << ARM_LPAE_TCR_SEP_SHIFT)
This is a specific detail of SMMUv2, and nothing to do with the
LPAE/AArch64 VMSA formats.
+
#define ARM_LPAE_MAIR_ATTR_SHIFT(n) ((n) << 3)
#define ARM_LPAE_MAIR_ATTR_MASK 0xff
#define ARM_LPAE_MAIR_ATTR_DEVICE 0x04
@@ -179,11 +195,12 @@ struct arm_lpae_io_pgtable {
struct io_pgtable iop;
int levels;
+ u32 sep;
size_t pgd_size;
unsigned long pg_shift;
unsigned long bits_per_level;
- void *pgd;
+ void*pgd[2];
};
typedef u64 arm_lpae_iopte;
@@ -426,7 +443,8 @@ 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 ||
- data->iop.fmt == ARM_32_LPAE_S1) {
+ data->iop.fmt == ARM_32_LPAE_S1 ||
+ data->iop.fmt == ARM_64_LPAE_SPLIT_S1) {
pte = ARM_LPAE_PTE_nG;
if (!(prot & IOMMU_WRITE) && (prot & IOMMU_READ))
pte |= ARM_LPAE_PTE_AP_RDONLY;
@@ -470,11 +488,10 @@ static arm_lpae_iopte arm_lpae_prot_to_pte(struct
arm_lpae_io_pgtable *data,
return pte;
}
-static int arm_lpae_map(struct io_pgtable_ops *ops, unsigned long iova,
- phys_addr_t paddr, size_t size, int iommu_prot)
+static int _arm_lpae_map(struct arm_lpae_io_pgtable *data, unsigned long iova,
+ phys_addr_t paddr, size_t size, int iommu_prot,
+ arm_lpae_iopte *ptep)
{
- struct arm_lpae_io_pgtable *data = io_pgtable_ops_to_data(ops);
-
[RESEND PATCH v2 2/3] iommu/io-pgtable-arm: Add support for AARCH64 split pagetables
Add a new sub-format ARM_64_LPAE_SPLIT_S1 to create and set up split
pagetables (TTBR0 and TTBR1). The initialization function sets up the
correct va_size and sign extension bits and programs the TCR registers.
Split pagetable formats use their own own map/unmap wrappers to ensure
that the correct pagetable is selected based on the incoming iova but
most of the heavy lifting is common to the other formats.
Signed-off-by: Jordan Crouse
---
drivers/iommu/io-pgtable-arm.c | 261 +
drivers/iommu/io-pgtable.c | 1 +
include/linux/io-pgtable.h | 2 +
3 files changed, 240 insertions(+), 24 deletions(-)
diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
index 161a7d56..aec35e5 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c
@@ -118,7 +118,12 @@
#define ARM_LPAE_TCR_TG0_64K (1 << 14)
#define ARM_LPAE_TCR_TG0_16K (2 << 14)
+#define ARM_LPAE_TCR_TG1_4K(0 << 30)
+#define ARM_LPAE_TCR_TG1_64K (1 << 30)
+#define ARM_LPAE_TCR_TG1_16K (2 << 30)
+
#define ARM_LPAE_TCR_SH0_SHIFT 12
+#define ARM_LPAE_TCR_SH1_SHIFT 28
#define ARM_LPAE_TCR_SH0_MASK 0x3
#define ARM_LPAE_TCR_SH_NS 0
#define ARM_LPAE_TCR_SH_OS 2
@@ -126,6 +131,8 @@
#define ARM_LPAE_TCR_ORGN0_SHIFT 10
#define ARM_LPAE_TCR_IRGN0_SHIFT 8
+#define ARM_LPAE_TCR_ORGN1_SHIFT 26
+#define ARM_LPAE_TCR_IRGN1_SHIFT 24
#define ARM_LPAE_TCR_RGN_MASK 0x3
#define ARM_LPAE_TCR_RGN_NC0
#define ARM_LPAE_TCR_RGN_WBWA 1
@@ -136,6 +143,7 @@
#define ARM_LPAE_TCR_SL0_MASK 0x3
#define ARM_LPAE_TCR_T0SZ_SHIFT0
+#define ARM_LPAE_TCR_T1SZ_SHIFT16
#define ARM_LPAE_TCR_SZ_MASK 0xf
#define ARM_LPAE_TCR_PS_SHIFT 16
@@ -152,6 +160,14 @@
#define ARM_LPAE_TCR_PS_48_BIT 0x5ULL
#define ARM_LPAE_TCR_PS_52_BIT 0x6ULL
+#define ARM_LPAE_TCR_SEP_SHIFT 47
+#define ARM_LPAE_TCR_SEP_31(0x0ULL << ARM_LPAE_TCR_SEP_SHIFT)
+#define ARM_LPAE_TCR_SEP_35(0x1ULL << ARM_LPAE_TCR_SEP_SHIFT)
+#define ARM_LPAE_TCR_SEP_39(0x2ULL << ARM_LPAE_TCR_SEP_SHIFT)
+#define ARM_LPAE_TCR_SEP_41(0x3ULL << ARM_LPAE_TCR_SEP_SHIFT)
+#define ARM_LPAE_TCR_SEP_43(0x4ULL << ARM_LPAE_TCR_SEP_SHIFT)
+#define ARM_LPAE_TCR_SEP_UPSTREAM (0x7ULL << ARM_LPAE_TCR_SEP_SHIFT)
+
#define ARM_LPAE_MAIR_ATTR_SHIFT(n)((n) << 3)
#define ARM_LPAE_MAIR_ATTR_MASK0xff
#define ARM_LPAE_MAIR_ATTR_DEVICE 0x04
@@ -179,11 +195,12 @@ struct arm_lpae_io_pgtable {
struct io_pgtable iop;
int levels;
+ u32 sep;
size_t pgd_size;
unsigned long pg_shift;
unsigned long bits_per_level;
- void*pgd;
+ void*pgd[2];
};
typedef u64 arm_lpae_iopte;
@@ -426,7 +443,8 @@ 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 ||
- data->iop.fmt == ARM_32_LPAE_S1) {
+ data->iop.fmt == ARM_32_LPAE_S1 ||
+ data->iop.fmt == ARM_64_LPAE_SPLIT_S1) {
pte = ARM_LPAE_PTE_nG;
if (!(prot & IOMMU_WRITE) && (prot & IOMMU_READ))
pte |= ARM_LPAE_PTE_AP_RDONLY;
@@ -470,11 +488,10 @@ static arm_lpae_iopte arm_lpae_prot_to_pte(struct
arm_lpae_io_pgtable *data,
return pte;
}
-static int arm_lpae_map(struct io_pgtable_ops *ops, unsigned long iova,
- phys_addr_t paddr, size_t size, int iommu_prot)
+static int _arm_lpae_map(struct arm_lpae_io_pgtable *data, unsigned long iova,
+ phys_addr_t paddr, size_t size, int iommu_prot,
+ arm_lpae_iopte *ptep)
{
- struct arm_lpae_io_pgtable *data = io_pgtable_ops_to_data(ops);
- arm_lpae_iopte *ptep = data->pgd;
int ret, lvl = ARM_LPAE_START_LVL(data);
arm_lpae_iopte prot;
@@ -497,12 +514,39 @@ static int arm_lpae_map(struct io_pgtable_ops *ops,
unsigned long iova,
return ret;
}
+static int arm_lpae_split_map(struct io_pgtable_ops *ops, unsigned long iova,
+ phys_addr_t paddr, size_t size, int iommu_prot)
+{
+ struct arm_lpae_io_pgtable *data = io_pgtable_ops_to_data(ops);
+ unsigned long mask = 1UL << data->sep;
+ arm_lpae_iopte *ptep;
+
+ if (iova & mask) {
+ ptep = data->pgd[1];
+ iova &= (mask - 1);
+ } else
+ ptep = data->pgd[0];
+
+ return _arm_lpae_map(data, iova, paddr, size, iommu_prot, ptep);
+}
+
+static int arm_lpae_map(struct io_pgtable_ops *ops, unsigned long iova,
+ phys_addr_t paddr, siz
