Messages in this thread | | | Date | Tue, 29 Jun 2021 14:04:03 +0200 | Subject | Re: [PATCH v4 1/3] iommu: io-pgtable: add DART pagetable format | From | Alexander Graf <> |
| |
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
| |