From: Heyang Zhou Subject: apldart: stop using bypass mode To: tech@openbsd.org Date: Wed, 29 Jul 2026 22:43:54 +0800 Hi, DART configured in bypass mode silently hides DMA memory corruptions caused by early-boot misconfiguration. Specifically, on my Macbook Air 2020 (M1), I see unexplained crashes when running memory-intensive workloads (e.g. building a large Rust project), if a USB hub with two devices is plugged in when the machine is booted: panic: kernel diagnostic assertion "(pg->pg_flags & PG_DEV) == 0" failed: file "/usr/src/sys/uvm/uvm_page.c" db_enter() at panic+0x138 panic() at __assert+0x28 panic() at uvm_pagealloc+0x1c4 uvm_pagealloc() at uvmfault_promote+0xac uvmfault_promote() at uvm_fault_lower+0x2cc uvm_fault_lower() at uvm_fault+0x158 uvm_fault() at udata_abort+0x128 Patching the assertion to dump the page confirms it is indeed a corruption in the vm_page array (pg_flags clobbered and contains values like 0xc0e28ae1). This patch implements support for devices that need mappings in two IOMMUs and removes bypass. Now any invalid DMAs through a non-locked DART are detected and reported as a panic. For example, the crash turns out to be u-boot leaving the USB controller in a bad state when handing over to OpenBSD, if any usb device is connected during boot. Now it's caught by DART instead of silently corrupting (more) memory: panic: apldart11: error 0x81000404 addr 0x00000000dae15100 Running `usb stop` in u-boot before entering the kernel, or booting with no USB devices, enters the system fine & stays stable under stress. Any USB devices plugged in later work correctly. diff --git a/sys/arch/arm64/dev/apldart.c b/sys/arch/arm64/dev/apldart.c index 4d5d03b3400..89d99a26b21 100644 --- a/sys/arch/arm64/dev/apldart.c +++ b/sys/arch/arm64/dev/apldart.c @@ -140,10 +140,23 @@ apldart_trunc_offset(psize_t off) #define HWRITE4(sc, reg, val) \ bus_space_write_4((sc)->sc_iot, (sc)->sc_ioh, (reg), (val)) +/* + * A device can be listed against up to three DARTs, and may appear + * against the same DART more than once with different stream IDs, so + * a stream can end up with three mirrors: <&d0 0>, <&d0 1>, <&d1 0>, + * <&d1 1> is one stream plus three. + */ +#define APLDART_MAX_MIRRORS 3 + struct apldart_stream { struct apldart_softc *as_sc; int as_sid; + /* Stream whose translation tables we share, if any. */ + struct apldart_stream *as_borrowed; + struct apldart_stream *as_mirror[APLDART_MAX_MIRRORS]; + int as_nmirror; + struct extent *as_dvamap; struct mutex as_dvamap_mtx; struct apldart_dmamem *as_l1; @@ -219,7 +232,9 @@ struct cfdriver apldart_cd = { }; bus_dma_tag_t apldart_map(void *, uint32_t *, bus_dma_tag_t); +void apldart_mirror(void *, uint32_t *, bus_dma_tag_t); void apldart_reserve(void *, uint32_t *, bus_addr_t, bus_size_t); +void apldart_flush_tlb(struct apldart_stream *); int apldart_t8020_intr(void *); int apldart_t8110_intr(void *); @@ -257,7 +272,7 @@ apldart_attach(struct device *parent, struct device *self, void *aux) struct apldart_softc *sc = (struct apldart_softc *)self; struct fdt_attach_args *faa = aux; uint64_t dva_range[2]; - uint32_t config, maj, min, params2, params3, params4, tcr, ttbr; + uint32_t config, maj, min, params3, params4, tcr, ttbr; int sid, idx; if (faa->fa_nreg < 1) { @@ -359,21 +374,6 @@ apldart_attach(struct device *parent, struct device *self, void *aux) */ sc->sc_do_suspend = !sc->sc_locked && !sc->sc_translating; - /* - * Use bypass mode if supported. This avoids an issue with - * the USB3 controllers which need mappings entered into two - * IOMMUs, which is somewhat difficult to implement with our - * current kernel interfaces. - */ - params2 = HREAD4(sc, DART_PARAMS2); - if ((params2 & DART_PARAMS2_BYPASS_SUPPORT) && - !sc->sc_locked && !sc->sc_translating) { - for (sid = 0; sid < sc->sc_nsid; sid++) - HWRITE4(sc, DART_TCR(sc, sid), sc->sc_tcr_bypass); - printf(", bypass\n"); - return; - } - if (sc->sc_locked) printf(", locked\n"); else if (sc->sc_translating) @@ -428,6 +428,7 @@ apldart_attach(struct device *parent, struct device *self, void *aux) sc->sc_id.id_node = faa->fa_node; sc->sc_id.id_cookie = sc; sc->sc_id.id_map = apldart_map; + sc->sc_id.id_mirror = apldart_mirror; sc->sc_id.id_reserve = apldart_reserve; iommu_device_register(&sc->sc_id); } @@ -446,7 +447,6 @@ apldart_resume(struct apldart_softc *sc) { paddr_t pa; int ntte, nl1, nl2; - uint32_t params2; uint32_t mask; int sid, idx; @@ -455,13 +455,6 @@ apldart_resume(struct apldart_softc *sc) power_domain_enable(sc->sc_node); - params2 = HREAD4(sc, DART_PARAMS2); - if (params2 & DART_PARAMS2_BYPASS_SUPPORT) { - for (sid = 0; sid < sc->sc_nsid; sid++) - HWRITE4(sc, DART_TCR(sc, sid), sc->sc_tcr_bypass); - return; - } - ntte = howmany((sc->sc_dvaend & sc->sc_dvamask), DART_PAGE_SIZE); nl2 = howmany(ntte, DART_PAGE_SIZE / sizeof(uint64_t)); nl1 = howmany(nl2, DART_PAGE_SIZE / sizeof(uint64_t)); @@ -594,7 +587,7 @@ apldart_init_locked_stream(struct apldart_stream *as) l1[idx] = (pa >> sc->sc_shift) | DART_L1_TABLE; } } - sc->sc_flush_tlb(sc, as->as_sid); + apldart_flush_tlb(as); memcpy(&as->as_dmat, sc->sc_dmat, sizeof(*sc->sc_dmat)); as->as_dmat._cookie = as; @@ -698,14 +691,138 @@ bus_dma_tag_t apldart_map(void *cookie, uint32_t *cells, bus_dma_tag_t dmat) { struct apldart_softc *sc = cookie; + struct apldart_stream *as; uint32_t sid = cells[0]; KASSERT(sid < sc->sc_nsid); if (sc->sc_as[sid] == NULL) sc->sc_as[sid] = apldart_alloc_stream(sc, sid); + as = sc->sc_as[sid]; + + /* + * A stream that shares another stream's translation tables has + * no tag of its own; hand out the tag of the stream that owns + * them. + */ + if (as->as_borrowed) + as = as->as_borrowed; - return &sc->sc_as[sid]->as_dmat; + return &as->as_dmat; +} + +/* + * A device can sit behind more than one DART; Apple's USB controllers + * do. Such a device needs a DVA that resolves identically in each of + * them, so point this DART's stream at the translation tables that + * belong to the stream we were handed and keep both TLBs in sync. + */ +void +apldart_mirror(void *cookie, uint32_t *cells, bus_dma_tag_t dmat) +{ + struct apldart_softc *sc = cookie; + struct apldart_softc *psc; + struct apldart_stream *pas, *as; + uint32_t sid = cells[0]; + paddr_t pa; + uint32_t mask; + int idx, ntte, nl1, nl2; + + KASSERT(sid < sc->sc_nsid); + + if (dmat->_dmamap_load != apldart_dmamap_load) + return; + pas = dmat->_cookie; + psc = pas->as_sc; + + /* + * Already set up; either this is the stream we were handed, or + * we have mirrored it before. A node can be mapped more than + * once, and a DART can appear twice in the same "iommus" + * property. + */ + as = sc->sc_as[sid]; + if (as == pas || (as != NULL && as->as_borrowed == pas)) + return; + + /* + * Locked DARTs keep the mappings set up by the firmware, which + * has already arranged for this to work. Leave them alone. + */ + if (sc->sc_locked || psc->sc_locked) + return; + + /* + * Sharing tables only works if both DARTs are under our + * control and describe the same address space with the same + * table layout. + */ + if (sc->sc_translating || psc->sc_translating || + sc->sc_ias != psc->sc_ias || sc->sc_shift != psc->sc_shift || + sc->sc_nttbr != psc->sc_nttbr || + sc->sc_ttbr_valid != psc->sc_ttbr_valid || + sc->sc_dvabase != psc->sc_dvabase || + sc->sc_dvaend != psc->sc_dvaend || + pas->as_nmirror >= nitems(pas->as_mirror) || as != NULL) { + printf("%s: can't share tables with %s\n", + sc->sc_dev.dv_xname, psc->sc_dev.dv_xname); + return; + } + + /* + * The tables belong to the other stream. as_borrowed records + * that, which keeps us from freeing them and lets + * apldart_map() hand out the owner's tag instead of ours, + * which we never set up. + */ + as = malloc(sizeof(*as), M_DEVBUF, M_WAITOK | M_ZERO); + as->as_sc = sc; + as->as_sid = sid; + as->as_borrowed = pas; + as->as_l1 = pas->as_l1; + as->as_l2 = pas->as_l2; + + ntte = howmany((psc->sc_dvaend & psc->sc_dvamask), DART_PAGE_SIZE); + nl2 = howmany(ntte, DART_PAGE_SIZE / sizeof(uint64_t)); + nl1 = howmany(nl2, DART_PAGE_SIZE / sizeof(uint64_t)); + + /* Install the other DART's page tables. */ + pa = APLDART_DMA_DVA(as->as_l1); + for (idx = 0; idx < nl1; idx++) { + HWRITE4(sc, DART_TTBR(sc, sid, idx), + (pa >> DART_TTBR_SHIFT) | sc->sc_ttbr_valid); + pa += DART_PAGE_SIZE; + } + sc->sc_flush_tlb(sc, sid); + + /* Enable this stream. */ + mask = HREAD4(sc, DART_SID_ENABLE(sc, sid / 32)); + mask |= (1U << (sid % 32)); + HWRITE4(sc, DART_SID_ENABLE(sc, sid / 32), mask); + + /* Enable translations. */ + HWRITE4(sc, DART_TCR(sc, sid), sc->sc_tcr_translate_enable); + + /* + * Only ever reached from iommu_device_map() during autoconf, + * so apldart_flush_tlb() cannot be walking this array while we + * extend it. + */ + sc->sc_as[sid] = as; + pas->as_mirror[pas->as_nmirror++] = as; +} + +void +apldart_flush_tlb(struct apldart_stream *as) +{ + struct apldart_stream *mas; + int i; + + as->as_sc->sc_flush_tlb(as->as_sc, as->as_sid); + for (i = 0; i < as->as_nmirror; i++) { + mas = as->as_mirror[i]; + mas->as_sc->sc_flush_tlb(mas->as_sc, mas->as_sid); + } } void @@ -841,7 +958,7 @@ apldart_load_map(struct apldart_stream *as, bus_dmamap_t map, int flags) } } - sc->sc_flush_tlb(sc, as->as_sid); + apldart_flush_tlb(as); return 0; } @@ -849,7 +966,6 @@ apldart_load_map(struct apldart_stream *as, bus_dmamap_t map, int flags) void apldart_unload_map(struct apldart_stream *as, bus_dmamap_t map) { - struct apldart_softc *sc = as->as_sc; struct apldart_map_state *ams = map->_dm_cookie; volatile uint64_t *tte; int seg, error; @@ -883,7 +999,7 @@ apldart_unload_map(struct apldart_stream *as, bus_dmamap_t map) ams[seg].ams_len = 0; } - sc->sc_flush_tlb(sc, as->as_sid); + apldart_flush_tlb(as); } int diff --git a/sys/dev/ofw/ofw_misc.c b/sys/dev/ofw/ofw_misc.c index 1bc6c5d5473..f25c2e09449 100644 --- a/sys/dev/ofw/ofw_misc.c +++ b/sys/dev/ofw/ofw_misc.c @@ -1069,6 +1069,23 @@ iommu_device_do_map(uint32_t phandle, uint32_t *cells, bus_dma_tag_t dmat) return dmat; } +void +iommu_device_do_mirror(uint32_t phandle, uint32_t *cells, bus_dma_tag_t dmat) +{ + struct iommu_device *id; + + if (phandle == 0) + return; + + LIST_FOREACH(id, &iommu_devices, id_list) { + if (id->id_phandle == phandle) { + if (id->id_mirror) + id->id_mirror(id->id_cookie, cells, dmat); + return; + } + } +} + int iommu_device_lookup(int node, uint32_t *phandle, uint32_t *cells) { @@ -1165,15 +1182,65 @@ out: return ret; } +/* + * A device can be behind more than one IOMMU, in which case it needs + * the same address to be valid in all of them. The first IOMMU listed + * provides the tag; the remaining ones are asked to mirror it. + */ bus_dma_tag_t iommu_device_map(int node, bus_dma_tag_t dmat) { - uint32_t phandle, cells[2] = {0}; + uint32_t cells[2]; + uint32_t *cell; + uint32_t *map; + bus_dma_tag_t ndmat = dmat; + int len, icells, ncells, inode; + int mapped = 0; + int i; - if (iommu_device_lookup(node, &phandle, &cells[0])) + len = OF_getproplen(node, "iommus"); + if (len <= 0) return dmat; - return iommu_device_do_map(phandle, &cells[0], dmat); + map = malloc(len, M_TEMP, M_WAITOK); + OF_getpropintarray(node, "iommus", map, len); + + cell = map; + ncells = len / sizeof(uint32_t); + while (ncells > 0) { + inode = OF_getnodebyphandle(cell[0]); + if (inode == 0) + break; + + icells = OF_getpropint(inode, "#iommu-cells", 1); + if (icells > nitems(cells) || ncells < icells + 1) + break; + + memset(cells, 0, sizeof(cells)); + for (i = 0; i < icells; i++) + cells[i] = cell[1 + i]; + + if (!mapped) { + ndmat = iommu_device_do_map(cell[0], &cells[0], dmat); + + /* + * iommu_device_do_map() hands back the tag it + * was given if no IOMMU claimed the device. + * There is then nothing for the rest to mirror. + */ + if (ndmat == dmat) + break; + mapped = 1; + } else + iommu_device_do_mirror(cell[0], &cells[0], ndmat); + + cell += (1 + icells); + ncells -= (1 + icells); + } + + free(map, M_TEMP, len); + + return ndmat; } bus_dma_tag_t diff --git a/sys/dev/ofw/ofw_misc.h b/sys/dev/ofw/ofw_misc.h index a1ee7194550..53f61e46e07 100644 --- a/sys/dev/ofw/ofw_misc.h +++ b/sys/dev/ofw/ofw_misc.h @@ -271,6 +271,7 @@ struct iommu_device { int id_node; void *id_cookie; bus_dma_tag_t (*id_map)(void *, uint32_t *, bus_dma_tag_t); + void (*id_mirror)(void *, uint32_t *, bus_dma_tag_t); void (*id_reserve)(void *, uint32_t *, bus_addr_t, bus_size_t); LIST_ENTRY(iommu_device) id_list;