| Message ID | 20210627143405.77298-2-sven@svenpeter.dev (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Apple M1 DART IOMMU driver | expand |
On 27.06.21 16:34, Sven Peter wrote: > > Apple's DART iommu uses a pagetable format that shares some > similarities with the ones already implemented by io-pgtable.c. > Add a new format variant to support the required differences > so that we don't have to duplicate the pagetable handling code. > > Signed-off-by: Sven Peter <sven@svenpeter.dev> > --- > drivers/iommu/io-pgtable-arm.c | 62 ++++++++++++++++++++++++++++++++++ > drivers/iommu/io-pgtable.c | 1 + > include/linux/io-pgtable.h | 7 ++++ > 3 files changed, 70 insertions(+) > > diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c > index 87def58e79b5..1dd5c45b4b5b 100644 > --- a/drivers/iommu/io-pgtable-arm.c > +++ b/drivers/iommu/io-pgtable-arm.c > @@ -127,6 +127,9 @@ > #define ARM_MALI_LPAE_MEMATTR_IMP_DEF 0x88ULL > #define ARM_MALI_LPAE_MEMATTR_WRITE_ALLOC 0x8DULL > > +#define APPLE_DART_PTE_PROT_NO_WRITE (1<<7) > +#define APPLE_DART_PTE_PROT_NO_READ (1<<8) > + > /* IOPTE accessors */ > #define iopte_deref(pte,d) __va(iopte_to_paddr(pte, d)) > > @@ -381,6 +384,15 @@ static arm_lpae_iopte arm_lpae_prot_to_pte(struct arm_lpae_io_pgtable *data, > { > arm_lpae_iopte pte; > > + if (data->iop.fmt == ARM_APPLE_DART) { > + pte = 0; > + if (!(prot & IOMMU_WRITE)) > + pte |= APPLE_DART_PTE_PROT_NO_WRITE; > + if (!(prot & IOMMU_READ)) > + pte |= APPLE_DART_PTE_PROT_NO_READ; > + return pte; What about the other bits, such as sharability, XN, etc? Do they not exist on DART? Or have they not been reverse engineered and 0s happen to "just work"? > + } > + > if (data->iop.fmt == ARM_64_LPAE_S1 || > data->iop.fmt == ARM_32_LPAE_S1) { > pte = ARM_LPAE_PTE_nG; > @@ -1043,6 +1055,51 @@ arm_mali_lpae_alloc_pgtable(struct io_pgtable_cfg *cfg, void *cookie) > return NULL; > } > > +static struct io_pgtable * > +apple_dart_alloc_pgtable(struct io_pgtable_cfg *cfg, void *cookie) > +{ > + struct arm_lpae_io_pgtable *data; > + int i; > + > + if (cfg->oas > 36) > + return NULL; > + > + data = arm_lpae_alloc_pgtable(cfg); > + if (!data) > + return NULL; > + > + /* > + * Apple's DART always requires three levels with the first level being > + * stored in four MMIO registers. We always concatenate the first and > + * second level so that we only have to setup the MMIO registers once. > + * This results in an effective two level pagetable. > + */ > + if (data->start_level < 1) > + return NULL; > + if (data->start_level == 1 && data->pgd_bits > 2) > + return NULL; > + if (data->start_level > 1) > + data->pgd_bits = 0; > + data->start_level = 2; > + cfg->apple_dart_cfg.n_ttbrs = 1 << data->pgd_bits; Maybe add a BUG_ON if n_ttbrs > ARRAY_SIZE(ttbr)? Or alternatively, do a normal runtime check and bail out then. Alex Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B Sitz: Berlin Ust-ID: DE 289 237 879
On Mon, Jun 28, 2021, at 12:54, Alexander Graf wrote: > > > On 27.06.21 16:34, Sven Peter wrote: > > > > Apple's DART iommu uses a pagetable format that shares some > > similarities with the ones already implemented by io-pgtable.c. > > Add a new format variant to support the required differences > > so that we don't have to duplicate the pagetable handling code. > > > > Signed-off-by: Sven Peter <sven@svenpeter.dev> > > --- > > drivers/iommu/io-pgtable-arm.c | 62 ++++++++++++++++++++++++++++++++++ > > drivers/iommu/io-pgtable.c | 1 + > > include/linux/io-pgtable.h | 7 ++++ > > 3 files changed, 70 insertions(+) > > > > diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c > > index 87def58e79b5..1dd5c45b4b5b 100644 > > --- a/drivers/iommu/io-pgtable-arm.c > > +++ b/drivers/iommu/io-pgtable-arm.c > > @@ -127,6 +127,9 @@ > > #define ARM_MALI_LPAE_MEMATTR_IMP_DEF 0x88ULL > > #define ARM_MALI_LPAE_MEMATTR_WRITE_ALLOC 0x8DULL > > > > +#define APPLE_DART_PTE_PROT_NO_WRITE (1<<7) > > +#define APPLE_DART_PTE_PROT_NO_READ (1<<8) > > + > > /* IOPTE accessors */ > > #define iopte_deref(pte,d) __va(iopte_to_paddr(pte, d)) > > > > @@ -381,6 +384,15 @@ static arm_lpae_iopte arm_lpae_prot_to_pte(struct arm_lpae_io_pgtable *data, > > { > > arm_lpae_iopte pte; > > > > + if (data->iop.fmt == ARM_APPLE_DART) { > > + pte = 0; > > + if (!(prot & IOMMU_WRITE)) > > + pte |= APPLE_DART_PTE_PROT_NO_WRITE; > > + if (!(prot & IOMMU_READ)) > > + pte |= APPLE_DART_PTE_PROT_NO_READ; > > + return pte; > > What about the other bits, such as sharability, XN, etc? Do they not > exist on DART? Or have they not been reverse engineered and 0s happen to > "just work"? I'm fairly certain they don't exist (or are at least not used by XNU). The co-processors that can run code also either use an entire separate iommu (e.g. the GPU) or only use DART as a "second stage" and have their own MMU which e.g. handles XN (e.g. the SEP or AOP). > > > + } > > + > > if (data->iop.fmt == ARM_64_LPAE_S1 || > > data->iop.fmt == ARM_32_LPAE_S1) { > > pte = ARM_LPAE_PTE_nG; > > @@ -1043,6 +1055,51 @@ arm_mali_lpae_alloc_pgtable(struct io_pgtable_cfg *cfg, void *cookie) > > return NULL; > > } > > > > +static struct io_pgtable * > > +apple_dart_alloc_pgtable(struct io_pgtable_cfg *cfg, void *cookie) > > +{ > > + struct arm_lpae_io_pgtable *data; > > + int i; > > + > > + if (cfg->oas > 36) > > + return NULL; > > + > > + data = arm_lpae_alloc_pgtable(cfg); > > + if (!data) > > + return NULL; > > + > > + /* > > + * Apple's DART always requires three levels with the first level being > > + * stored in four MMIO registers. We always concatenate the first and > > + * second level so that we only have to setup the MMIO registers once. > > + * This results in an effective two level pagetable. > > + */ > > + if (data->start_level < 1) > > + return NULL; > > + if (data->start_level == 1 && data->pgd_bits > 2) > > + return NULL; > > + if (data->start_level > 1) > > + data->pgd_bits = 0; > > + data->start_level = 2; > > + cfg->apple_dart_cfg.n_ttbrs = 1 << data->pgd_bits; > > Maybe add a BUG_ON if n_ttbrs > ARRAY_SIZE(ttbr)? Or alternatively, do a > normal runtime check and bail out then. n_ttbrs can't actually be larger than 4 at this point already due to the previous checks. I can add a BUG_ON though just to make it explicit and be safe in case those checks or the array size ever change. Sven
On 29.06.21 09:37, Sven Peter wrote: > > On Mon, Jun 28, 2021, at 12:54, Alexander Graf wrote: >> >> >> On 27.06.21 16:34, Sven Peter wrote: >>> >>> Apple's DART iommu uses a pagetable format that shares some >>> similarities with the ones already implemented by io-pgtable.c. >>> Add a new format variant to support the required differences >>> so that we don't have to duplicate the pagetable handling code. >>> >>> Signed-off-by: Sven Peter <sven@svenpeter.dev> >>> --- >>> drivers/iommu/io-pgtable-arm.c | 62 ++++++++++++++++++++++++++++++++++ >>> drivers/iommu/io-pgtable.c | 1 + >>> include/linux/io-pgtable.h | 7 ++++ >>> 3 files changed, 70 insertions(+) >>> >>> diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c >>> index 87def58e79b5..1dd5c45b4b5b 100644 >>> --- a/drivers/iommu/io-pgtable-arm.c >>> +++ b/drivers/iommu/io-pgtable-arm.c >>> @@ -127,6 +127,9 @@ >>> #define ARM_MALI_LPAE_MEMATTR_IMP_DEF 0x88ULL >>> #define ARM_MALI_LPAE_MEMATTR_WRITE_ALLOC 0x8DULL >>> >>> +#define APPLE_DART_PTE_PROT_NO_WRITE (1<<7) >>> +#define APPLE_DART_PTE_PROT_NO_READ (1<<8) >>> + >>> /* IOPTE accessors */ >>> #define iopte_deref(pte,d) __va(iopte_to_paddr(pte, d)) >>> >>> @@ -381,6 +384,15 @@ static arm_lpae_iopte arm_lpae_prot_to_pte(struct arm_lpae_io_pgtable *data, >>> { >>> arm_lpae_iopte pte; >>> >>> + if (data->iop.fmt == ARM_APPLE_DART) { >>> + pte = 0; >>> + if (!(prot & IOMMU_WRITE)) >>> + pte |= APPLE_DART_PTE_PROT_NO_WRITE; >>> + if (!(prot & IOMMU_READ)) >>> + pte |= APPLE_DART_PTE_PROT_NO_READ; >>> + return pte; >> >> What about the other bits, such as sharability, XN, etc? Do they not >> exist on DART? Or have they not been reverse engineered and 0s happen to >> "just work"? > > I'm fairly certain they don't exist (or are at least not used by XNU). > > The co-processors that can run code also either use an entire separate iommu > (e.g. the GPU) or only use DART as a "second stage" and have their own > MMU which e.g. handles XN (e.g. the SEP or AOP). Ok :). > >> >>> + } >>> + >>> if (data->iop.fmt == ARM_64_LPAE_S1 || >>> data->iop.fmt == ARM_32_LPAE_S1) { >>> pte = ARM_LPAE_PTE_nG; >>> @@ -1043,6 +1055,51 @@ arm_mali_lpae_alloc_pgtable(struct io_pgtable_cfg *cfg, void *cookie) >>> return NULL; >>> } >>> >>> +static struct io_pgtable * >>> +apple_dart_alloc_pgtable(struct io_pgtable_cfg *cfg, void *cookie) >>> +{ >>> + struct arm_lpae_io_pgtable *data; >>> + int i; >>> + >>> + if (cfg->oas > 36) >>> + return NULL; >>> + >>> + data = arm_lpae_alloc_pgtable(cfg); >>> + if (!data) >>> + return NULL; >>> + >>> + /* >>> + * Apple's DART always requires three levels with the first level being >>> + * stored in four MMIO registers. We always concatenate the first and >>> + * second level so that we only have to setup the MMIO registers once. >>> + * This results in an effective two level pagetable. >>> + */ >>> + if (data->start_level < 1) >>> + return NULL; >>> + if (data->start_level == 1 && data->pgd_bits > 2) >>> + return NULL; >>> + if (data->start_level > 1) >>> + data->pgd_bits = 0; >>> + data->start_level = 2; >>> + cfg->apple_dart_cfg.n_ttbrs = 1 << data->pgd_bits; >> >> Maybe add a BUG_ON if n_ttbrs > ARRAY_SIZE(ttbr)? Or alternatively, do a >> normal runtime check and bail out then. > > n_ttbrs can't actually be larger than 4 at this point already due to the > previous checks. > I can add a BUG_ON though just to make it explicit and be safe in case those > checks or the array size ever change. Ah, now I see it too. No worries then - I agree that you have all cases covered. Reviewed-by: Alexander Graf <graf@amazon.com> Alex Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B Sitz: Berlin Ust-ID: DE 289 237 879
Reviewed-by: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
On 2021-06-27 15:34, Sven Peter wrote: > Apple's DART iommu uses a pagetable format that shares some > similarities with the ones already implemented by io-pgtable.c. > Add a new format variant to support the required differences > so that we don't have to duplicate the pagetable handling code. > > Signed-off-by: Sven Peter <sven@svenpeter.dev> > --- > drivers/iommu/io-pgtable-arm.c | 62 ++++++++++++++++++++++++++++++++++ > drivers/iommu/io-pgtable.c | 1 + > include/linux/io-pgtable.h | 7 ++++ > 3 files changed, 70 insertions(+) > > diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c > index 87def58e79b5..1dd5c45b4b5b 100644 > --- a/drivers/iommu/io-pgtable-arm.c > +++ b/drivers/iommu/io-pgtable-arm.c > @@ -127,6 +127,9 @@ > #define ARM_MALI_LPAE_MEMATTR_IMP_DEF 0x88ULL > #define ARM_MALI_LPAE_MEMATTR_WRITE_ALLOC 0x8DULL > > +#define APPLE_DART_PTE_PROT_NO_WRITE (1<<7) > +#define APPLE_DART_PTE_PROT_NO_READ (1<<8) > + > /* IOPTE accessors */ > #define iopte_deref(pte,d) __va(iopte_to_paddr(pte, d)) > > @@ -381,6 +384,15 @@ static arm_lpae_iopte arm_lpae_prot_to_pte(struct arm_lpae_io_pgtable *data, > { > arm_lpae_iopte pte; > > + if (data->iop.fmt == ARM_APPLE_DART) { > + pte = 0; > + if (!(prot & IOMMU_WRITE)) > + pte |= APPLE_DART_PTE_PROT_NO_WRITE; > + if (!(prot & IOMMU_READ)) > + pte |= APPLE_DART_PTE_PROT_NO_READ; > + return pte; > + } > + > if (data->iop.fmt == ARM_64_LPAE_S1 || > data->iop.fmt == ARM_32_LPAE_S1) { > pte = ARM_LPAE_PTE_nG; > @@ -1043,6 +1055,51 @@ arm_mali_lpae_alloc_pgtable(struct io_pgtable_cfg *cfg, void *cookie) > return NULL; > } > > +static struct io_pgtable * > +apple_dart_alloc_pgtable(struct io_pgtable_cfg *cfg, void *cookie) > +{ > + struct arm_lpae_io_pgtable *data; > + int i; > + > + if (cfg->oas > 36) > + return NULL; > + > + data = arm_lpae_alloc_pgtable(cfg); > + if (!data) > + return NULL; > + > + /* > + * Apple's DART always requires three levels with the first level being > + * stored in four MMIO registers. We always concatenate the first and > + * second level so that we only have to setup the MMIO registers once. > + * This results in an effective two level pagetable. > + */ Nit: I appreciate the effort to document the weirdness, but this comment did rather mislead me initially, and now that I (think I) understand how things work it seems a bit backwards. Could we say something like: "The table format itself always uses two levels, but the total VA space is mapped by four separate tables, making the MMIO registers an effective "level 1". For simplicity, though, we treat this equivalently to LPAE stage 2 concatenation at level 2, with the additional TTBRs each just pointing at consecutive pages." ? > + if (data->start_level < 1) > + return NULL; > + if (data->start_level == 1 && data->pgd_bits > 2) > + return NULL; > + if (data->start_level > 1) > + data->pgd_bits = 0; > + data->start_level = 2; > + cfg->apple_dart_cfg.n_ttbrs = 1 << data->pgd_bits; > + data->pgd_bits += data->bits_per_level; > + > + data->pgd = __arm_lpae_alloc_pages(ARM_LPAE_PGD_SIZE(data), GFP_KERNEL, > + cfg); > + if (!data->pgd) > + goto out_free_data; > + > + for (i = 0; i < cfg->apple_dart_cfg.n_ttbrs; ++i) > + cfg->apple_dart_cfg.ttbr[i] = > + virt_to_phys(data->pgd + i * ARM_LPAE_GRANULE(data)); > + > + return &data->iop; > + > +out_free_data: > + kfree(data); > + return NULL; > +} > + > struct io_pgtable_init_fns io_pgtable_arm_64_lpae_s1_init_fns = { > .alloc = arm_64_lpae_alloc_pgtable_s1, > .free = arm_lpae_free_pgtable, > @@ -1068,6 +1125,11 @@ struct io_pgtable_init_fns io_pgtable_arm_mali_lpae_init_fns = { > .free = arm_lpae_free_pgtable, > }; > > +struct io_pgtable_init_fns io_pgtable_apple_dart_init_fns = { > + .alloc = apple_dart_alloc_pgtable, > + .free = arm_lpae_free_pgtable, > +}; > + > #ifdef CONFIG_IOMMU_IO_PGTABLE_LPAE_SELFTEST > > static struct io_pgtable_cfg *cfg_cookie __initdata; > diff --git a/drivers/iommu/io-pgtable.c b/drivers/iommu/io-pgtable.c > index 6e9917ce980f..fd8e6bd6caf9 100644 > --- a/drivers/iommu/io-pgtable.c > +++ b/drivers/iommu/io-pgtable.c > @@ -20,6 +20,7 @@ io_pgtable_init_table[IO_PGTABLE_NUM_FMTS] = { > [ARM_64_LPAE_S1] = &io_pgtable_arm_64_lpae_s1_init_fns, > [ARM_64_LPAE_S2] = &io_pgtable_arm_64_lpae_s2_init_fns, > [ARM_MALI_LPAE] = &io_pgtable_arm_mali_lpae_init_fns, > + [ARM_APPLE_DART] = &io_pgtable_apple_dart_init_fns, > #endif > #ifdef CONFIG_IOMMU_IO_PGTABLE_ARMV7S > [ARM_V7S] = &io_pgtable_arm_v7s_init_fns, > diff --git a/include/linux/io-pgtable.h b/include/linux/io-pgtable.h > index 4d40dfa75b55..a4bfac7f85f7 100644 > --- a/include/linux/io-pgtable.h > +++ b/include/linux/io-pgtable.h > @@ -16,6 +16,7 @@ enum io_pgtable_fmt { > ARM_V7S, > ARM_MALI_LPAE, > AMD_IOMMU_V1, > + ARM_APPLE_DART, s/ARM_// - this is pure Apple ;) With that fixed and hopefully a somewhat clarified comment above, Reviewed-by: Robin Murphy <robin.murphy@arm.com> > IO_PGTABLE_NUM_FMTS, > }; > > @@ -136,6 +137,11 @@ struct io_pgtable_cfg { > u64 transtab; > u64 memattr; > } arm_mali_lpae_cfg; > + > + struct { > + u64 ttbr[4]; > + u32 n_ttbrs; > + } apple_dart_cfg; > }; > }; > > @@ -246,5 +252,6 @@ extern struct io_pgtable_init_fns io_pgtable_arm_64_lpae_s2_init_fns; > extern struct io_pgtable_init_fns io_pgtable_arm_v7s_init_fns; > extern struct io_pgtable_init_fns io_pgtable_arm_mali_lpae_init_fns; > extern struct io_pgtable_init_fns io_pgtable_amd_iommu_v1_init_fns; > +extern struct io_pgtable_init_fns io_pgtable_apple_dart_init_fns; > > #endif /* __IO_PGTABLE_H */ >
Hi, On Tue, Jul 13, 2021, at 21:17, Robin Murphy wrote: > On 2021-06-27 15:34, Sven Peter wrote: > > Apple's DART iommu uses a pagetable format that shares some > > similarities with the ones already implemented by io-pgtable.c. > > Add a new format variant to support the required differences > > so that we don't have to duplicate the pagetable handling code. > > > > Signed-off-by: Sven Peter <sven@svenpeter.dev> > > --- > > drivers/iommu/io-pgtable-arm.c | 62 ++++++++++++++++++++++++++++++++++ > > drivers/iommu/io-pgtable.c | 1 + > > include/linux/io-pgtable.h | 7 ++++ > > 3 files changed, 70 insertions(+) > > > > diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c > > index 87def58e79b5..1dd5c45b4b5b 100644 > > --- a/drivers/iommu/io-pgtable-arm.c > > +++ b/drivers/iommu/io-pgtable-arm.c > > @@ -127,6 +127,9 @@ > > #define ARM_MALI_LPAE_MEMATTR_IMP_DEF 0x88ULL > > #define ARM_MALI_LPAE_MEMATTR_WRITE_ALLOC 0x8DULL > > > > +#define APPLE_DART_PTE_PROT_NO_WRITE (1<<7) > > +#define APPLE_DART_PTE_PROT_NO_READ (1<<8) > > + > > /* IOPTE accessors */ > > #define iopte_deref(pte,d) __va(iopte_to_paddr(pte, d)) > > > > @@ -381,6 +384,15 @@ static arm_lpae_iopte arm_lpae_prot_to_pte(struct arm_lpae_io_pgtable *data, > > { > > arm_lpae_iopte pte; > > > > + if (data->iop.fmt == ARM_APPLE_DART) { > > + pte = 0; > > + if (!(prot & IOMMU_WRITE)) > > + pte |= APPLE_DART_PTE_PROT_NO_WRITE; > > + if (!(prot & IOMMU_READ)) > > + pte |= APPLE_DART_PTE_PROT_NO_READ; > > + return pte; > > + } > > + > > if (data->iop.fmt == ARM_64_LPAE_S1 || > > data->iop.fmt == ARM_32_LPAE_S1) { > > pte = ARM_LPAE_PTE_nG; > > @@ -1043,6 +1055,51 @@ arm_mali_lpae_alloc_pgtable(struct io_pgtable_cfg *cfg, void *cookie) > > return NULL; > > } > > > > +static struct io_pgtable * > > +apple_dart_alloc_pgtable(struct io_pgtable_cfg *cfg, void *cookie) > > +{ > > + struct arm_lpae_io_pgtable *data; > > + int i; > > + > > + if (cfg->oas > 36) > > + return NULL; > > + > > + data = arm_lpae_alloc_pgtable(cfg); > > + if (!data) > > + return NULL; > > + > > + /* > > + * Apple's DART always requires three levels with the first level being > > + * stored in four MMIO registers. We always concatenate the first and > > + * second level so that we only have to setup the MMIO registers once. > > + * This results in an effective two level pagetable. > > + */ > > Nit: I appreciate the effort to document the weirdness, but this comment > did rather mislead me initially, and now that I (think I) understand how > things work it seems a bit backwards. Could we say something like: > > "The table format itself always uses two levels, but the total VA > space is mapped by four separate tables, making the MMIO registers > an effective "level 1". For simplicity, though, we treat this > equivalently to LPAE stage 2 concatenation at level 2, with the > additional TTBRs each just pointing at consecutive pages." > > ? > Sure, your version is much easier to understand! Thanks. > > + if (data->start_level < 1) > > + return NULL; > > + if (data->start_level == 1 && data->pgd_bits > 2) > > + return NULL; > > + if (data->start_level > 1) > > + data->pgd_bits = 0; > > + data->start_level = 2; > > + cfg->apple_dart_cfg.n_ttbrs = 1 << data->pgd_bits; > > + data->pgd_bits += data->bits_per_level; > > + > > + data->pgd = __arm_lpae_alloc_pages(ARM_LPAE_PGD_SIZE(data), GFP_KERNEL, > > + cfg); > > + if (!data->pgd) > > + goto out_free_data; > > + > > + for (i = 0; i < cfg->apple_dart_cfg.n_ttbrs; ++i) > > + cfg->apple_dart_cfg.ttbr[i] = > > + virt_to_phys(data->pgd + i * ARM_LPAE_GRANULE(data)); > > + > > + return &data->iop; > > + > > +out_free_data: > > + kfree(data); > > + return NULL; > > +} > > + > > struct io_pgtable_init_fns io_pgtable_arm_64_lpae_s1_init_fns = { > > .alloc = arm_64_lpae_alloc_pgtable_s1, > > .free = arm_lpae_free_pgtable, > > @@ -1068,6 +1125,11 @@ struct io_pgtable_init_fns io_pgtable_arm_mali_lpae_init_fns = { > > .free = arm_lpae_free_pgtable, > > }; > > > > +struct io_pgtable_init_fns io_pgtable_apple_dart_init_fns = { > > + .alloc = apple_dart_alloc_pgtable, > > + .free = arm_lpae_free_pgtable, > > +}; > > + > > #ifdef CONFIG_IOMMU_IO_PGTABLE_LPAE_SELFTEST > > > > static struct io_pgtable_cfg *cfg_cookie __initdata; > > diff --git a/drivers/iommu/io-pgtable.c b/drivers/iommu/io-pgtable.c > > index 6e9917ce980f..fd8e6bd6caf9 100644 > > --- a/drivers/iommu/io-pgtable.c > > +++ b/drivers/iommu/io-pgtable.c > > @@ -20,6 +20,7 @@ io_pgtable_init_table[IO_PGTABLE_NUM_FMTS] = { > > [ARM_64_LPAE_S1] = &io_pgtable_arm_64_lpae_s1_init_fns, > > [ARM_64_LPAE_S2] = &io_pgtable_arm_64_lpae_s2_init_fns, > > [ARM_MALI_LPAE] = &io_pgtable_arm_mali_lpae_init_fns, > > + [ARM_APPLE_DART] = &io_pgtable_apple_dart_init_fns, > > #endif > > #ifdef CONFIG_IOMMU_IO_PGTABLE_ARMV7S > > [ARM_V7S] = &io_pgtable_arm_v7s_init_fns, > > diff --git a/include/linux/io-pgtable.h b/include/linux/io-pgtable.h > > index 4d40dfa75b55..a4bfac7f85f7 100644 > > --- a/include/linux/io-pgtable.h > > +++ b/include/linux/io-pgtable.h > > @@ -16,6 +16,7 @@ enum io_pgtable_fmt { > > ARM_V7S, > > ARM_MALI_LPAE, > > AMD_IOMMU_V1, > > + ARM_APPLE_DART, > > s/ARM_// - this is pure Apple ;) > > With that fixed and hopefully a somewhat clarified comment above, > > Reviewed-by: Robin Murphy <robin.murphy@arm.com> True, I'll remove the ARM_ prefix and also change the commit message to match the subsystem style as mentioned in your reply for the third patch. Thanks for the review! > > > IO_PGTABLE_NUM_FMTS, > > }; > > > > @@ -136,6 +137,11 @@ struct io_pgtable_cfg { > > u64 transtab; > > u64 memattr; > > } arm_mali_lpae_cfg; > > + > > + struct { > > + u64 ttbr[4]; > > + u32 n_ttbrs; > > + } apple_dart_cfg; > > }; > > }; > > > > @@ -246,5 +252,6 @@ extern struct io_pgtable_init_fns io_pgtable_arm_64_lpae_s2_init_fns; > > extern struct io_pgtable_init_fns io_pgtable_arm_v7s_init_fns; > > extern struct io_pgtable_init_fns io_pgtable_arm_mali_lpae_init_fns; > > extern struct io_pgtable_init_fns io_pgtable_amd_iommu_v1_init_fns; > > +extern struct io_pgtable_init_fns io_pgtable_apple_dart_init_fns; > > > > #endif /* __IO_PGTABLE_H */ > > > Sven
diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c index 87def58e79b5..1dd5c45b4b5b 100644 --- a/drivers/iommu/io-pgtable-arm.c +++ b/drivers/iommu/io-pgtable-arm.c @@ -127,6 +127,9 @@ #define ARM_MALI_LPAE_MEMATTR_IMP_DEF 0x88ULL #define ARM_MALI_LPAE_MEMATTR_WRITE_ALLOC 0x8DULL +#define APPLE_DART_PTE_PROT_NO_WRITE (1<<7) +#define APPLE_DART_PTE_PROT_NO_READ (1<<8) + /* IOPTE accessors */ #define iopte_deref(pte,d) __va(iopte_to_paddr(pte, d)) @@ -381,6 +384,15 @@ static arm_lpae_iopte arm_lpae_prot_to_pte(struct arm_lpae_io_pgtable *data, { arm_lpae_iopte pte; + if (data->iop.fmt == ARM_APPLE_DART) { + pte = 0; + if (!(prot & IOMMU_WRITE)) + pte |= APPLE_DART_PTE_PROT_NO_WRITE; + if (!(prot & IOMMU_READ)) + pte |= APPLE_DART_PTE_PROT_NO_READ; + return pte; + } + if (data->iop.fmt == ARM_64_LPAE_S1 || data->iop.fmt == ARM_32_LPAE_S1) { pte = ARM_LPAE_PTE_nG; @@ -1043,6 +1055,51 @@ arm_mali_lpae_alloc_pgtable(struct io_pgtable_cfg *cfg, void *cookie) return NULL; } +static struct io_pgtable * +apple_dart_alloc_pgtable(struct io_pgtable_cfg *cfg, void *cookie) +{ + struct arm_lpae_io_pgtable *data; + int i; + + if (cfg->oas > 36) + return NULL; + + data = arm_lpae_alloc_pgtable(cfg); + if (!data) + return NULL; + + /* + * Apple's DART always requires three levels with the first level being + * stored in four MMIO registers. We always concatenate the first and + * second level so that we only have to setup the MMIO registers once. + * This results in an effective two level pagetable. + */ + if (data->start_level < 1) + return NULL; + if (data->start_level == 1 && data->pgd_bits > 2) + return NULL; + if (data->start_level > 1) + data->pgd_bits = 0; + data->start_level = 2; + cfg->apple_dart_cfg.n_ttbrs = 1 << data->pgd_bits; + data->pgd_bits += data->bits_per_level; + + data->pgd = __arm_lpae_alloc_pages(ARM_LPAE_PGD_SIZE(data), GFP_KERNEL, + cfg); + if (!data->pgd) + goto out_free_data; + + for (i = 0; i < cfg->apple_dart_cfg.n_ttbrs; ++i) + cfg->apple_dart_cfg.ttbr[i] = + virt_to_phys(data->pgd + i * ARM_LPAE_GRANULE(data)); + + return &data->iop; + +out_free_data: + kfree(data); + return NULL; +} + struct io_pgtable_init_fns io_pgtable_arm_64_lpae_s1_init_fns = { .alloc = arm_64_lpae_alloc_pgtable_s1, .free = arm_lpae_free_pgtable, @@ -1068,6 +1125,11 @@ struct io_pgtable_init_fns io_pgtable_arm_mali_lpae_init_fns = { .free = arm_lpae_free_pgtable, }; +struct io_pgtable_init_fns io_pgtable_apple_dart_init_fns = { + .alloc = apple_dart_alloc_pgtable, + .free = arm_lpae_free_pgtable, +}; + #ifdef CONFIG_IOMMU_IO_PGTABLE_LPAE_SELFTEST static struct io_pgtable_cfg *cfg_cookie __initdata; diff --git a/drivers/iommu/io-pgtable.c b/drivers/iommu/io-pgtable.c index 6e9917ce980f..fd8e6bd6caf9 100644 --- a/drivers/iommu/io-pgtable.c +++ b/drivers/iommu/io-pgtable.c @@ -20,6 +20,7 @@ io_pgtable_init_table[IO_PGTABLE_NUM_FMTS] = { [ARM_64_LPAE_S1] = &io_pgtable_arm_64_lpae_s1_init_fns, [ARM_64_LPAE_S2] = &io_pgtable_arm_64_lpae_s2_init_fns, [ARM_MALI_LPAE] = &io_pgtable_arm_mali_lpae_init_fns, + [ARM_APPLE_DART] = &io_pgtable_apple_dart_init_fns, #endif #ifdef CONFIG_IOMMU_IO_PGTABLE_ARMV7S [ARM_V7S] = &io_pgtable_arm_v7s_init_fns, diff --git a/include/linux/io-pgtable.h b/include/linux/io-pgtable.h index 4d40dfa75b55..a4bfac7f85f7 100644 --- a/include/linux/io-pgtable.h +++ b/include/linux/io-pgtable.h @@ -16,6 +16,7 @@ enum io_pgtable_fmt { ARM_V7S, ARM_MALI_LPAE, AMD_IOMMU_V1, + ARM_APPLE_DART, IO_PGTABLE_NUM_FMTS, }; @@ -136,6 +137,11 @@ struct io_pgtable_cfg { u64 transtab; u64 memattr; } arm_mali_lpae_cfg; + + struct { + u64 ttbr[4]; + u32 n_ttbrs; + } apple_dart_cfg; }; }; @@ -246,5 +252,6 @@ extern struct io_pgtable_init_fns io_pgtable_arm_64_lpae_s2_init_fns; extern struct io_pgtable_init_fns io_pgtable_arm_v7s_init_fns; extern struct io_pgtable_init_fns io_pgtable_arm_mali_lpae_init_fns; extern struct io_pgtable_init_fns io_pgtable_amd_iommu_v1_init_fns; +extern struct io_pgtable_init_fns io_pgtable_apple_dart_init_fns; #endif /* __IO_PGTABLE_H */
Apple's DART iommu uses a pagetable format that shares some similarities with the ones already implemented by io-pgtable.c. Add a new format variant to support the required differences so that we don't have to duplicate the pagetable handling code. Signed-off-by: Sven Peter <sven@svenpeter.dev> --- drivers/iommu/io-pgtable-arm.c | 62 ++++++++++++++++++++++++++++++++++ drivers/iommu/io-pgtable.c | 1 + include/linux/io-pgtable.h | 7 ++++ 3 files changed, 70 insertions(+)