[PATCH v2 0/4] Fix IOVA allocation in Apple dart iommu after global LMB mem map changes

The changes in "Make LMB memory map global and persistent" [1] break mapping DMA memory in the USB xHCI driver when using the apple_dart iommu present on Apple silicon systems.
The IOVA space used by the u-boot driver (low 4GB) and physical memory do not overlap. The physical memory on this systems starts depending on the SoC either at 0x10_0000_0000 or 0x100_0000_0000. It make no sense to manage these distinct regions in a single LMB map. In addition every device has its own iommu and IO address space so sharing a single memory map between all iommu instances is not necessary.
To fix this issue restore the used subset (add, alloc and free) of the previous pointer based LMB interface with "io_" as prefix.
To ensure that low level lmb functions do not use the global LMB variable reorder lib/lmb.c so that the variable is not visible.
Tested with patches from my "Fix device removal order for Apple dart iommu" series [2] to fix a separate issue.
The cosmetic commit has two checkpatch warnings in existing code which I ignored.
[1] https://lore.kernel.org/u-boot/20240826115940.3233167-1-sughosh.ganu@linaro.... [2] https://lore.kernel.org/u-boot/20241031-iommu_apple_dart_ordering-v1-0-8a687...
Signed-off-by: Janne Grunau j@jannau.net --- Changes in v2: - added io_lmb_teardown() and use it in dart's remove callback - removed leftover from copy-n-paste in io_lmb_setup() documentation - added Tom's Rb: - Link to v1: https://lore.kernel.org/r/20241101-io_lmb_apple_dart_iommu-v1-0-fe4b9a74d47c...
--- Janne Grunau (4): lmb: Do not use global LMB variable in _lmb_free() lmb: cosmetic: reorder functions and global LMB variable lmb: Add basic io_lmb functionality iommu: apple: Manage IOVA separately from global LMB mem map
drivers/iommu/apple_dart.c | 18 +- include/lmb.h | 51 ++++ lib/lmb.c | 614 ++++++++++++++++++++++++++------------------- 3 files changed, 418 insertions(+), 265 deletions(-) --- base-commit: 1d147b74f437fb0e85821e8271fe52bc5fd30194 change-id: 20241101-io_lmb_apple_dart_iommu-dc2674ef9036
Best regards,

From: Janne Grunau j@jannau.net
It will be re-used with a lmb list pointer as argument for IOVA allocations in the apple_dart iommu driver.
Reviewed-by: Tom Rini trini@konsulko.com Signed-off-by: Janne Grunau j@jannau.net --- lib/lmb.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/lib/lmb.c b/lib/lmb.c index 96a055f951e657bf3acc05c9fa804698ce90c312..5be0d9dd89a506dc1e6e6951cc54441a8d4bb0df 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -495,10 +495,10 @@ long lmb_add(phys_addr_t base, phys_size_t size) return lmb_map_update_notify(base, size, MAP_OP_ADD, LMB_NONE); }
-static long _lmb_free(phys_addr_t base, phys_size_t size) +static long _lmb_free(struct alist *lmb_rgn_lst, phys_addr_t base, + phys_size_t size) { struct lmb_region *rgn; - struct alist *lmb_rgn_lst = &lmb.used_mem; phys_addr_t rgnbegin, rgnend; phys_addr_t end = base + size - 1; int i; @@ -561,7 +561,7 @@ long lmb_free_flags(phys_addr_t base, phys_size_t size, { long ret;
- ret = _lmb_free(base, size); + ret = _lmb_free(&lmb.used_mem, base, size); if (ret < 0) return ret;

From: Janne Grunau j@jannau.net
Low lovel LMB functionality will be used to manage IOVA space in the Apple dart iommu driver. This reordering ensures that those function can not access the global LMB memory map variable.
Reviewed-by: Tom Rini trini@konsulko.com Signed-off-by: Janne Grunau j@jannau.net --- lib/lmb.c | 496 ++++++++++++++++++++++++++++++++------------------------------ 1 file changed, 254 insertions(+), 242 deletions(-)
diff --git a/lib/lmb.c b/lib/lmb.c index 5be0d9dd89a506dc1e6e6951cc54441a8d4bb0df..2ce91f7521cfe58577b092fdaea8004b17f0eeff 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -27,97 +27,11 @@ DECLARE_GLOBAL_DATA_PTR; #define MAP_OP_FREE (u8)0x2 #define MAP_OP_ADD (u8)0x3
-static struct lmb lmb; - -static bool lmb_should_notify(enum lmb_flags flags) -{ - return !lmb.test && !(flags & LMB_NONOTIFY) && - CONFIG_IS_ENABLED(EFI_LOADER); -} - -static int __maybe_unused lmb_map_update_notify(phys_addr_t addr, - phys_size_t size, - u8 op, enum lmb_flags flags) -{ - u64 efi_addr; - u64 pages; - efi_status_t status; - - if (op != MAP_OP_RESERVE && op != MAP_OP_FREE && op != MAP_OP_ADD) { - log_err("Invalid map update op received (%d)\n", op); - return -1; - } - - if (!lmb_should_notify(flags)) - return 0; - - efi_addr = (uintptr_t)map_sysmem(addr, 0); - pages = efi_size_in_pages(size + (efi_addr & EFI_PAGE_MASK)); - efi_addr &= ~EFI_PAGE_MASK; - - status = efi_add_memory_map_pg(efi_addr, pages, - op == MAP_OP_RESERVE ? - EFI_BOOT_SERVICES_DATA : - EFI_CONVENTIONAL_MEMORY, - false); - if (status != EFI_SUCCESS) { - log_err("%s: LMB Map notify failure %lu\n", __func__, - status & ~EFI_ERROR_MASK); - return -1; - } - unmap_sysmem((void *)(uintptr_t)efi_addr); - - return 0; -} - -static void lmb_print_region_flags(enum lmb_flags flags) -{ - u64 bitpos; - const char *flag_str[] = { "none", "no-map", "no-overwrite", "no-notify" }; - - do { - bitpos = flags ? fls(flags) - 1 : 0; - assert_noisy(bitpos < ARRAY_SIZE(flag_str)); - printf("%s", flag_str[bitpos]); - flags &= ~(1ull << bitpos); - puts(flags ? ", " : "\n"); - } while (flags); -} - -static void lmb_dump_region(struct alist *lmb_rgn_lst, char *name) -{ - struct lmb_region *rgn = lmb_rgn_lst->data; - unsigned long long base, size, end; - enum lmb_flags flags; - int i; - - printf(" %s.count = 0x%x\n", name, lmb_rgn_lst->count); - - for (i = 0; i < lmb_rgn_lst->count; i++) { - base = rgn[i].base; - size = rgn[i].size; - end = base + size - 1; - flags = rgn[i].flags; - - printf(" %s[%d]\t[0x%llx-0x%llx], 0x%08llx bytes flags: ", - name, i, base, end, size); - lmb_print_region_flags(flags); - } -} - -void lmb_dump_all_force(void) -{ - printf("lmb_dump_all:\n"); - lmb_dump_region(&lmb.free_mem, "memory"); - lmb_dump_region(&lmb.used_mem, "reserved"); -} - -void lmb_dump_all(void) -{ -#ifdef DEBUG - lmb_dump_all_force(); -#endif -} +/* + * The following low level LMB functions must not access the global LMB memory + * map since they are also used to manage IOVA memory maps in iommu drivers like + * apple_dart. + */
static long lmb_addrs_overlap(phys_addr_t base1, phys_size_t size1, phys_addr_t base2, phys_size_t size2) @@ -206,117 +120,6 @@ static void lmb_fix_over_lap_regions(struct alist *lmb_rgn_lst, lmb_remove_region(lmb_rgn_lst, r2); }
-static void lmb_reserve_uboot_region(void) -{ - int bank; - ulong end, bank_end; - phys_addr_t rsv_start; - - rsv_start = gd->start_addr_sp - CONFIG_STACK_SIZE; - end = gd->ram_top; - - /* - * Reserve memory from aligned address below the bottom of U-Boot stack - * until end of RAM area to prevent LMB from overwriting that memory. - */ - debug("## Current stack ends at 0x%08lx ", (ulong)rsv_start); - - /* adjust sp by 16K to be safe */ - rsv_start -= SZ_16K; - for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) { - if (!gd->bd->bi_dram[bank].size || - rsv_start < gd->bd->bi_dram[bank].start) - continue; - /* Watch out for RAM at end of address space! */ - bank_end = gd->bd->bi_dram[bank].start + - gd->bd->bi_dram[bank].size - 1; - if (rsv_start > bank_end) - continue; - if (bank_end > end) - bank_end = end - 1; - - lmb_reserve_flags(rsv_start, bank_end - rsv_start + 1, - LMB_NOOVERWRITE); - - if (gd->flags & GD_FLG_SKIP_RELOC) - lmb_reserve_flags((phys_addr_t)(uintptr_t)_start, - gd->mon_len, LMB_NOOVERWRITE); - - break; - } -} - -static void lmb_reserve_common(void *fdt_blob) -{ - lmb_reserve_uboot_region(); - - if (CONFIG_IS_ENABLED(OF_LIBFDT) && fdt_blob) - boot_fdt_add_mem_rsv_regions(fdt_blob); -} - -static __maybe_unused void lmb_reserve_common_spl(void) -{ - phys_addr_t rsv_start; - phys_size_t rsv_size; - - /* - * Assume a SPL stack of 16KB. This must be - * more than enough for the SPL stage. - */ - if (IS_ENABLED(CONFIG_SPL_STACK_R_ADDR)) { - rsv_start = gd->start_addr_sp - 16384; - rsv_size = 16384; - lmb_reserve_flags(rsv_start, rsv_size, LMB_NOOVERWRITE); - } - - if (IS_ENABLED(CONFIG_SPL_SEPARATE_BSS)) { - /* Reserve the bss region */ - rsv_start = (phys_addr_t)(uintptr_t)__bss_start; - rsv_size = (phys_addr_t)(uintptr_t)__bss_end - - (phys_addr_t)(uintptr_t)__bss_start; - lmb_reserve_flags(rsv_start, rsv_size, LMB_NOOVERWRITE); - } -} - -/** - * lmb_add_memory() - Add memory range for LMB allocations - * - * Add the entire available memory range to the pool of memory that - * can be used by the LMB module for allocations. - * - * Return: None - */ -void lmb_add_memory(void) -{ - int i; - phys_size_t size; - u64 ram_top = gd->ram_top; - struct bd_info *bd = gd->bd; - - if (CONFIG_IS_ENABLED(LMB_ARCH_MEM_MAP)) - return lmb_arch_add_memory(); - - /* Assume a 4GB ram_top if not defined */ - if (!ram_top) - ram_top = 0x100000000ULL; - - for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) { - size = bd->bi_dram[i].size; - if (size) { - lmb_add(bd->bi_dram[i].start, size); - - /* - * Reserve memory above ram_top as - * no-overwrite so that it cannot be - * allocated - */ - if (bd->bi_dram[i].start >= ram_top) - lmb_reserve_flags(bd->bi_dram[i].start, size, - LMB_NOOVERWRITE); - } - } -} - static long lmb_resize_regions(struct alist *lmb_rgn_lst, unsigned long idx_start, phys_addr_t base, phys_size_t size) @@ -476,25 +279,6 @@ static long lmb_add_region_flags(struct alist *lmb_rgn_lst, phys_addr_t base, return 0; }
-static long lmb_add_region(struct alist *lmb_rgn_lst, phys_addr_t base, - phys_size_t size) -{ - return lmb_add_region_flags(lmb_rgn_lst, base, size, LMB_NONE); -} - -/* This routine may be called with relocation disabled. */ -long lmb_add(phys_addr_t base, phys_size_t size) -{ - long ret; - struct alist *lmb_rgn_lst = &lmb.free_mem; - - ret = lmb_add_region(lmb_rgn_lst, base, size); - if (ret) - return ret; - - return lmb_map_update_notify(base, size, MAP_OP_ADD, LMB_NONE); -} - static long _lmb_free(struct alist *lmb_rgn_lst, phys_addr_t base, phys_size_t size) { @@ -546,6 +330,255 @@ static long _lmb_free(struct alist *lmb_rgn_lst, phys_addr_t base, rgn[i].flags); }
+static long lmb_overlaps_region(struct alist *lmb_rgn_lst, phys_addr_t base, + phys_size_t size) +{ + unsigned long i; + struct lmb_region *rgn = lmb_rgn_lst->data; + + for (i = 0; i < lmb_rgn_lst->count; i++) { + phys_addr_t rgnbase = rgn[i].base; + phys_size_t rgnsize = rgn[i].size; + if (lmb_addrs_overlap(base, size, rgnbase, rgnsize)) + break; + } + + return (i < lmb_rgn_lst->count) ? i : -1; +} + +static phys_addr_t lmb_align_down(phys_addr_t addr, phys_size_t size) +{ + return addr & ~(size - 1); +} + +/* + * Low level LMB functions are used to manage IOVA memory maps for the Apple + * dart iommu. They must not access the global LMB memory map. + * So keep the global LMB variable declaration unreachable from them. + */ + +static struct lmb lmb; + +static bool lmb_should_notify(enum lmb_flags flags) +{ + return !lmb.test && !(flags & LMB_NONOTIFY) && + CONFIG_IS_ENABLED(EFI_LOADER); +} + +static int __maybe_unused lmb_map_update_notify(phys_addr_t addr, + phys_size_t size, + u8 op, enum lmb_flags flags) +{ + u64 efi_addr; + u64 pages; + efi_status_t status; + + if (op != MAP_OP_RESERVE && op != MAP_OP_FREE && op != MAP_OP_ADD) { + log_err("Invalid map update op received (%d)\n", op); + return -1; + } + + if (!lmb_should_notify(flags)) + return 0; + + efi_addr = (uintptr_t)map_sysmem(addr, 0); + pages = efi_size_in_pages(size + (efi_addr & EFI_PAGE_MASK)); + efi_addr &= ~EFI_PAGE_MASK; + + status = efi_add_memory_map_pg(efi_addr, pages, + op == MAP_OP_RESERVE ? + EFI_BOOT_SERVICES_DATA : + EFI_CONVENTIONAL_MEMORY, + false); + if (status != EFI_SUCCESS) { + log_err("%s: LMB Map notify failure %lu\n", __func__, + status & ~EFI_ERROR_MASK); + return -1; + } + unmap_sysmem((void *)(uintptr_t)efi_addr); + + return 0; +} + +static void lmb_print_region_flags(enum lmb_flags flags) +{ + u64 bitpos; + const char *flag_str[] = { "none", "no-map", "no-overwrite", "no-notify" }; + + do { + bitpos = flags ? fls(flags) - 1 : 0; + assert_noisy(bitpos < ARRAY_SIZE(flag_str)); + printf("%s", flag_str[bitpos]); + flags &= ~(1ull << bitpos); + puts(flags ? ", " : "\n"); + } while (flags); +} + +static void lmb_dump_region(struct alist *lmb_rgn_lst, char *name) +{ + struct lmb_region *rgn = lmb_rgn_lst->data; + unsigned long long base, size, end; + enum lmb_flags flags; + int i; + + printf(" %s.count = 0x%x\n", name, lmb_rgn_lst->count); + + for (i = 0; i < lmb_rgn_lst->count; i++) { + base = rgn[i].base; + size = rgn[i].size; + end = base + size - 1; + flags = rgn[i].flags; + + printf(" %s[%d]\t[0x%llx-0x%llx], 0x%08llx bytes flags: ", + name, i, base, end, size); + lmb_print_region_flags(flags); + } +} + +void lmb_dump_all_force(void) +{ + printf("lmb_dump_all:\n"); + lmb_dump_region(&lmb.free_mem, "memory"); + lmb_dump_region(&lmb.used_mem, "reserved"); +} + +void lmb_dump_all(void) +{ +#ifdef DEBUG + lmb_dump_all_force(); +#endif +} + +static void lmb_reserve_uboot_region(void) +{ + int bank; + ulong end, bank_end; + phys_addr_t rsv_start; + + rsv_start = gd->start_addr_sp - CONFIG_STACK_SIZE; + end = gd->ram_top; + + /* + * Reserve memory from aligned address below the bottom of U-Boot stack + * until end of RAM area to prevent LMB from overwriting that memory. + */ + debug("## Current stack ends at 0x%08lx ", (ulong)rsv_start); + + /* adjust sp by 16K to be safe */ + rsv_start -= SZ_16K; + for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) { + if (!gd->bd->bi_dram[bank].size || + rsv_start < gd->bd->bi_dram[bank].start) + continue; + /* Watch out for RAM at end of address space! */ + bank_end = gd->bd->bi_dram[bank].start + + gd->bd->bi_dram[bank].size - 1; + if (rsv_start > bank_end) + continue; + if (bank_end > end) + bank_end = end - 1; + + lmb_reserve_flags(rsv_start, bank_end - rsv_start + 1, + LMB_NOOVERWRITE); + + if (gd->flags & GD_FLG_SKIP_RELOC) + lmb_reserve_flags((phys_addr_t)(uintptr_t)_start, + gd->mon_len, LMB_NOOVERWRITE); + + break; + } +} + +static void lmb_reserve_common(void *fdt_blob) +{ + lmb_reserve_uboot_region(); + + if (CONFIG_IS_ENABLED(OF_LIBFDT) && fdt_blob) + boot_fdt_add_mem_rsv_regions(fdt_blob); +} + +static __maybe_unused void lmb_reserve_common_spl(void) +{ + phys_addr_t rsv_start; + phys_size_t rsv_size; + + /* + * Assume a SPL stack of 16KB. This must be + * more than enough for the SPL stage. + */ + if (IS_ENABLED(CONFIG_SPL_STACK_R_ADDR)) { + rsv_start = gd->start_addr_sp - 16384; + rsv_size = 16384; + lmb_reserve_flags(rsv_start, rsv_size, LMB_NOOVERWRITE); + } + + if (IS_ENABLED(CONFIG_SPL_SEPARATE_BSS)) { + /* Reserve the bss region */ + rsv_start = (phys_addr_t)(uintptr_t)__bss_start; + rsv_size = (phys_addr_t)(uintptr_t)__bss_end - + (phys_addr_t)(uintptr_t)__bss_start; + lmb_reserve_flags(rsv_start, rsv_size, LMB_NOOVERWRITE); + } +} + +/** + * lmb_add_memory() - Add memory range for LMB allocations + * + * Add the entire available memory range to the pool of memory that + * can be used by the LMB module for allocations. + * + * Return: None + */ +void lmb_add_memory(void) +{ + int i; + phys_size_t size; + u64 ram_top = gd->ram_top; + struct bd_info *bd = gd->bd; + + if (CONFIG_IS_ENABLED(LMB_ARCH_MEM_MAP)) + return lmb_arch_add_memory(); + + /* Assume a 4GB ram_top if not defined */ + if (!ram_top) + ram_top = 0x100000000ULL; + + for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) { + size = bd->bi_dram[i].size; + if (size) { + lmb_add(bd->bi_dram[i].start, size); + + /* + * Reserve memory above ram_top as + * no-overwrite so that it cannot be + * allocated + */ + if (bd->bi_dram[i].start >= ram_top) + lmb_reserve_flags(bd->bi_dram[i].start, size, + LMB_NOOVERWRITE); + } + } +} + +static long lmb_add_region(struct alist *lmb_rgn_lst, phys_addr_t base, + phys_size_t size) +{ + return lmb_add_region_flags(lmb_rgn_lst, base, size, LMB_NONE); +} + +/* This routine may be called with relocation disabled. */ +long lmb_add(phys_addr_t base, phys_size_t size) +{ + long ret; + struct alist *lmb_rgn_lst = &lmb.free_mem; + + ret = lmb_add_region(lmb_rgn_lst, base, size); + if (ret) + return ret; + + return lmb_map_update_notify(base, size, MAP_OP_ADD, LMB_NONE); +} + /** * lmb_free_flags() - Free up a region of memory * @base: Base Address of region to be freed @@ -590,27 +623,6 @@ long lmb_reserve(phys_addr_t base, phys_size_t size) return lmb_reserve_flags(base, size, LMB_NONE); }
-static long lmb_overlaps_region(struct alist *lmb_rgn_lst, phys_addr_t base, - phys_size_t size) -{ - unsigned long i; - struct lmb_region *rgn = lmb_rgn_lst->data; - - for (i = 0; i < lmb_rgn_lst->count; i++) { - phys_addr_t rgnbase = rgn[i].base; - phys_size_t rgnsize = rgn[i].size; - if (lmb_addrs_overlap(base, size, rgnbase, rgnsize)) - break; - } - - return (i < lmb_rgn_lst->count) ? i : -1; -} - -static phys_addr_t lmb_align_down(phys_addr_t addr, phys_size_t size) -{ - return addr & ~(size - 1); -} - static phys_addr_t _lmb_alloc_base(phys_size_t size, ulong align, phys_addr_t max_addr, enum lmb_flags flags) {

From: Janne Grunau j@jannau.net
These functions can be used with struct lmb pointers and will be used to manage IOVA space in the apple_dart iommu driver. This restores part of the pointer base struct lmb API from before commit ed17a33fed29 ("lmb: make LMB memory map persistent and global"). io_lmb_add() and io_lmb_free() can trivially reuse exisiting lmb functions. io_lmb_setup() is separate for unique error log messages. io_lmb_alloc() is a simplified copy of _lmb_alloc_base() since the later has unused features and internal use of the global LMB memory map.
Signed-off-by: Janne Grunau j@jannau.net --- include/lmb.h | 51 +++++++++++++++++++++++++++++++++++++ lib/lmb.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 131 insertions(+)
diff --git a/include/lmb.h b/include/lmb.h index 2201d6f2b67bb605dbff015fa2a6a008b780c57a..fa91bf17adea84d335ba43ee7749e2a12a1d44c0 100644 --- a/include/lmb.h +++ b/include/lmb.h @@ -156,6 +156,57 @@ static inline int lmb_read_check(phys_addr_t addr, phys_size_t len) return lmb_alloc_addr(addr, len) == addr ? 0 : -1; }
+/** + * io_lmb_setup() - Initialize LMB struct + * @lmb: IO LMB to initialize + * + * Returns: 0 on success, negative error code on failure + */ +int io_lmb_setup(struct lmb *io_lmb); + +/** + * io_lmb_teardown() - Tear LMB struct down + * @lmb: IO LMB to teardown + */ +void io_lmb_teardown(struct lmb *io_lmb); + +/** + * io_lmb_add() - Add an IOVA range for allocations + * @io_lmb: LMB to add the space to + * @base: Base Address of region to add + * @size: Size of the region to add + * + * Add the IOVA space [base, base + size] to be managed by io_lmb. + * + * Returns: 0 if the region addition was successful, -1 on failure + */ +long io_lmb_add(struct lmb *io_lmb, phys_addr_t base, phys_size_t size); + +/** + * io_lmb_alloc() - Allocate specified IO memory address with specified alignment + * @io_lmb: LMB to alloc from + * @size: Size of the region requested + * @align: Required address and size alignment + * + * Allocate a region of IO memory. The base parameter is used to specify the + * base address of the requested region. + * + * Return: base IO address on success, 0 on error + */ +phys_addr_t io_lmb_alloc(struct lmb *io_lmb, phys_size_t size, ulong align); + +/** + * io_lmb_free() - Free up a region of IOVA space + * @io_lmb: LMB to return the IO address space to + * @base: Base Address of region to be freed + * @size: Size of the region to be freed + * + * Free up a region of IOVA space. + * + * Return: 0 if successful, -1 on failure + */ +long io_lmb_free(struct lmb *io_lmb, phys_addr_t base, phys_size_t size); + #endif /* __KERNEL__ */
#endif /* _LINUX_LMB_H */ diff --git a/lib/lmb.c b/lib/lmb.c index 2ce91f7521cfe58577b092fdaea8004b17f0eeff..cfc1763e49a76c6d4d9c85cd2b0342c6e0a7c4cf 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -351,6 +351,86 @@ static phys_addr_t lmb_align_down(phys_addr_t addr, phys_size_t size) return addr & ~(size - 1); }
+/* + * IOVA LMB memory maps using lmb pointers instead of the global LMB memory map. + */ + +int io_lmb_setup(struct lmb *io_lmb) +{ + int ret; + + ret = alist_init(&io_lmb->free_mem, sizeof(struct lmb_region), + (uint)LMB_ALIST_INITIAL_SIZE); + if (!ret) { + log_debug("Unable to initialise the list for LMB free IOVA\n"); + return -ENOMEM; + } + + ret = alist_init(&io_lmb->used_mem, sizeof(struct lmb_region), + (uint)LMB_ALIST_INITIAL_SIZE); + if (!ret) { + log_debug("Unable to initialise the list for LMB used IOVA\n"); + return -ENOMEM; + } + + io_lmb->test = false; + + return 0; +} + +void io_lmb_teardown(struct lmb *io_lmb) +{ + alist_uninit(&io_lmb->free_mem); + alist_uninit(&io_lmb->used_mem); +} + +long io_lmb_add(struct lmb *io_lmb, phys_addr_t base, phys_size_t size) +{ + return lmb_add_region_flags(&io_lmb->free_mem, base, size, LMB_NONE); +} + +/* derived and simplified from _lmb_alloc_base() */ +phys_addr_t io_lmb_alloc(struct lmb *io_lmb, phys_size_t size, ulong align) +{ + long i, rgn; + phys_addr_t base = 0; + phys_addr_t res_base; + struct lmb_region *lmb_used = io_lmb->used_mem.data; + struct lmb_region *lmb_memory = io_lmb->free_mem.data; + + for (i = io_lmb->free_mem.count - 1; i >= 0; i--) { + phys_addr_t lmbbase = lmb_memory[i].base; + phys_size_t lmbsize = lmb_memory[i].size; + + if (lmbsize < size) + continue; + base = lmb_align_down(lmbbase + lmbsize - size, align); + + while (base && lmbbase <= base) { + rgn = lmb_overlaps_region(&io_lmb->used_mem, base, size); + if (rgn < 0) { + /* This area isn't reserved, take it */ + if (lmb_add_region_flags(&io_lmb->used_mem, base, + size, LMB_NONE) < 0) + return 0; + + return base; + } + + res_base = lmb_used[rgn].base; + if (res_base < size) + break; + base = lmb_align_down(res_base - size, align); + } + } + return 0; +} + +long io_lmb_free(struct lmb *io_lmb, phys_addr_t base, phys_size_t size) +{ + return _lmb_free(&io_lmb->used_mem, base, size); +} + /* * Low level LMB functions are used to manage IOVA memory maps for the Apple * dart iommu. They must not access the global LMB memory map.

From: Janne Grunau j@jannau.net
There is no overlap between the IOVA space managed by the iommu (here the 32-bit address space) and physical RAM on Apple silicon systems. The RAM starts at 0x10_0000_0000 or 0x100_0000_0000 so it's not possible to manage the IOVA with the global memory LMB and use 1:1 translation. In addition each device has its own iommu and does not need to share the address space with all other devices. This should not be problem for u-boot's limited use and hardware support. Restore the private per instance LMB IOVA map.
Fixes: ed17a33fed2 ("lmb: make LMB memory map persistent and global") Signed-off-by: Janne Grunau j@jannau.net --- drivers/iommu/apple_dart.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/iommu/apple_dart.c b/drivers/iommu/apple_dart.c index 611ac7cd6deb4fccd45ccfe8cbb9b3e36e2af753..3e9e7819e517b8fe62b9e429b7a8ca3eca29741d 100644 --- a/drivers/iommu/apple_dart.c +++ b/drivers/iommu/apple_dart.c @@ -73,6 +73,8 @@ struct apple_dart_priv { u64 *l1, *l2; int bypass, shift;
+ struct lmb io_lmb; + dma_addr_t dvabase; dma_addr_t dvaend;
@@ -123,7 +125,7 @@ static dma_addr_t apple_dart_map(struct udevice *dev, void *addr, size_t size) off = (phys_addr_t)addr - paddr; psize = ALIGN(size + off, DART_PAGE_SIZE);
- dva = lmb_alloc(psize, DART_PAGE_SIZE); + dva = io_lmb_alloc(&priv->io_lmb, psize, DART_PAGE_SIZE);
idx = dva / DART_PAGE_SIZE; for (i = 0; i < psize / DART_PAGE_SIZE; i++) { @@ -159,7 +161,7 @@ static void apple_dart_unmap(struct udevice *dev, dma_addr_t addr, size_t size) (unsigned long)&priv->l2[idx + i]); priv->flush_tlb(priv);
- lmb_free(dva, psize); + io_lmb_free(&priv->io_lmb, dva, psize); }
static struct iommu_ops apple_dart_ops = { @@ -173,7 +175,7 @@ static int apple_dart_probe(struct udevice *dev) dma_addr_t addr; phys_addr_t l2; int ntte, nl1, nl2; - int sid, i; + int ret, sid, i; u32 params2, params4;
priv->base = dev_read_addr_ptr(dev); @@ -212,7 +214,13 @@ static int apple_dart_probe(struct udevice *dev) priv->dvabase = DART_PAGE_SIZE; priv->dvaend = SZ_4G - DART_PAGE_SIZE;
- lmb_add(priv->dvabase, priv->dvaend - priv->dvabase); + ret = io_lmb_setup(&priv->io_lmb); + if (ret) + return ret; + ret = io_lmb_add(&priv->io_lmb, priv->dvabase, + priv->dvaend - priv->dvabase); + if (ret) + return -EINVAL;
/* Disable translations. */ for (sid = 0; sid < priv->nsid; sid++) @@ -294,6 +302,8 @@ static int apple_dart_remove(struct udevice *dev) } priv->flush_tlb(priv);
+ io_lmb_teardown(&priv->io_lmb); + return 0; }

On Wed, Nov 06, 2024 at 10:10:08PM +0100, Janne Grunau via B4 Relay wrote:
The changes in "Make LMB memory map global and persistent" [1] break mapping DMA memory in the USB xHCI driver when using the apple_dart iommu present on Apple silicon systems.
The IOVA space used by the u-boot driver (low 4GB) and physical memory do not overlap. The physical memory on this systems starts depending on the SoC either at 0x10_0000_0000 or 0x100_0000_0000. It make no sense to manage these distinct regions in a single LMB map. In addition every device has its own iommu and IO address space so sharing a single memory map between all iommu instances is not necessary.
To fix this issue restore the used subset (add, alloc and free) of the previous pointer based LMB interface with "io_" as prefix.
To ensure that low level lmb functions do not use the global LMB variable reorder lib/lmb.c so that the variable is not visible.
Tested with patches from my "Fix device removal order for Apple dart iommu" series [2] to fix a separate issue.
The cosmetic commit has two checkpatch warnings in existing code which I ignored.
[1] https://lore.kernel.org/u-boot/20240826115940.3233167-1-sughosh.ganu@linaro.... [2] https://lore.kernel.org/u-boot/20241031-iommu_apple_dart_ordering-v1-0-8a687...
Signed-off-by: Janne Grunau j@jannau.net
Changes in v2:
- added io_lmb_teardown() and use it in dart's remove callback
- removed leftover from copy-n-paste in io_lmb_setup() documentation
- added Tom's Rb:
- Link to v1: https://lore.kernel.org/r/20241101-io_lmb_apple_dart_iommu-v1-0-fe4b9a74d47c...
Sorry for the late reply here, can you please rebase on top of current master? A few other changes / clean-ups have gone in and I can't easily rebase and confirm the changes right now. Thanks!

On Sun, Nov 10, 2024 at 08:52:11PM -0600, Tom Rini wrote:
On Wed, Nov 06, 2024 at 10:10:08PM +0100, Janne Grunau via B4 Relay wrote:
The changes in "Make LMB memory map global and persistent" [1] break mapping DMA memory in the USB xHCI driver when using the apple_dart iommu present on Apple silicon systems.
The IOVA space used by the u-boot driver (low 4GB) and physical memory do not overlap. The physical memory on this systems starts depending on the SoC either at 0x10_0000_0000 or 0x100_0000_0000. It make no sense to manage these distinct regions in a single LMB map. In addition every device has its own iommu and IO address space so sharing a single memory map between all iommu instances is not necessary.
To fix this issue restore the used subset (add, alloc and free) of the previous pointer based LMB interface with "io_" as prefix.
To ensure that low level lmb functions do not use the global LMB variable reorder lib/lmb.c so that the variable is not visible.
Tested with patches from my "Fix device removal order for Apple dart iommu" series [2] to fix a separate issue.
The cosmetic commit has two checkpatch warnings in existing code which I ignored.
[1] https://lore.kernel.org/u-boot/20240826115940.3233167-1-sughosh.ganu@linaro.... [2] https://lore.kernel.org/u-boot/20241031-iommu_apple_dart_ordering-v1-0-8a687...
Signed-off-by: Janne Grunau j@jannau.net
Changes in v2:
- added io_lmb_teardown() and use it in dart's remove callback
- removed leftover from copy-n-paste in io_lmb_setup() documentation
- added Tom's Rb:
- Link to v1: https://lore.kernel.org/r/20241101-io_lmb_apple_dart_iommu-v1-0-fe4b9a74d47c...
Sorry for the late reply here, can you please rebase on top of current master? A few other changes / clean-ups have gone in and I can't easily rebase and confirm the changes right now. Thanks!
done. I redid the cosmetic reorder patch to ensure the other changes to lib/lmb.c are not inadvertently reverted. Please apply Patch 1 + 2 quickly to avoid the risk of having to do it again.
best regards
Janne
participants (3)
-
Janne Grunau
-
Janne Grunau via B4 Relay
-
Tom Rini