Re: apldart: stop using bypass mode
Mark Kettenis <[email protected]>
| Newsgroups | gmane.os.openbsd.tech |
|---|---|
| Message-ID | <[email protected]> |
> Date: Wed, 29 Jul 2026 22:43:54 +0800 (CST) > From: Heyang Zhou <[email protected]> > > 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. Thanks a lot, this has been on my list of things to do for a long time. I think it needs to be done a little bit different though. The fact that the "iommus" property specifies multiple IOMMU streams doesn't necessarily mean that we should mirror them. As far as I know the DWC3 controller is the only one we need to do this for. Therefore the decision to mirror needs to be moved into the driver for that controller. The maximum number of IOMMU streams is actually two; this limit is enforced in the device tree binding for the Apple DWC3 controller. So I tweaked the way you handled mirrored streams in apldart(4) a bit. Diff below seems to work on my M2 Pro mini. I want to look a bit closer at the U-Boot issue before I commit this though. I thought U-Boot already stopped the USB controller before handing over control to the OS. Index: dev/fdt/xhci_fdt.c =================================================================== RCS file: /cvs/src/sys/dev/fdt/xhci_fdt.c,v diff -u -p -r1.31 xhci_fdt.c --- dev/fdt/xhci_fdt.c 21 Jul 2026 11:15:56 -0000 1.31 +++ dev/fdt/xhci_fdt.c 11 Aug 2026 20:57:12 -0000 @@ -300,11 +300,16 @@ int xhci_snps_attach(struct xhci_fdt_softc *sc) { /* - * On Apple hardware we need to reset the controller when we - * see a new connection. + * Apple hardware uses two IOMMUs in parallel, so we have to + * mirror the streams. We also need to reset the controller + * when we see a new connection on this hardware.. */ if (OF_is_compatible(sc->sc_node, "apple,dwc3") || OF_is_compatible(sc->sc_node, "apple,t8103-dwc3")) { + sc->sc.sc_bus.dmatag = + iommu_device_mirror_idx(sc->sc_node, + sc->sc.sc_bus.dmatag, 1); + sc->sc_usb_controller_port.up_cookie = sc; sc->sc_usb_controller_port.up_connect = xhci_snps_connect; task_set(&sc->sc_snps_connect_task, xhci_snps_do_connect, sc); Index: dev/ofw/ofw_misc.c =================================================================== RCS file: /cvs/src/sys/dev/ofw/ofw_misc.c,v diff -u -p -r1.44 ofw_misc.c --- dev/ofw/ofw_misc.c 25 Jan 2026 11:56:57 -0000 1.44 +++ dev/ofw/ofw_misc.c 11 Aug 2026 20:57:12 -0000 @@ -1058,9 +1058,6 @@ iommu_device_do_map(uint32_t phandle, ui { struct iommu_device *id; - if (phandle == 0) - return dmat; - LIST_FOREACH(id, &iommu_devices, id_list) { if (id->id_phandle == phandle) return id->id_map(id->id_cookie, cells, dmat); @@ -1070,7 +1067,7 @@ iommu_device_do_map(uint32_t phandle, ui } int -iommu_device_lookup(int node, uint32_t *phandle, uint32_t *cells) +iommu_device_lookup_idx(int node, uint32_t *phandle, uint32_t *cells, int idx) { uint32_t *cell; uint32_t *map; @@ -1078,6 +1075,8 @@ iommu_device_lookup(int node, uint32_t * int ret = 1; int i; + KASSERT(idx >= 0); + len = OF_getproplen(node, "iommus"); if (len <= 0) return ret; @@ -1098,14 +1097,17 @@ iommu_device_lookup(int node, uint32_t * KASSERT(icells <= 2); - *phandle = cell[0]; - for (i = 0; i < icells; i++) - cells[i] = cell[1 + i]; - ret = 0; - break; + if (idx == 0) { + *phandle = cell[0]; + for (i = 0; i < icells; i++) + cells[i] = cell[1 + i]; + ret = 0; + break; + } cell += (1 + icells); ncells -= (1 + icells); + idx--; } out: @@ -1115,6 +1117,12 @@ out: } int +iommu_device_lookup(int node, uint32_t *phandle, uint32_t *cells) +{ + return iommu_device_lookup_idx(node, phandle, cells, 0); +} + +int iommu_device_lookup_pci(int node, uint32_t rid, uint32_t *phandle, uint32_t *cells) { @@ -1166,14 +1174,37 @@ out: } bus_dma_tag_t -iommu_device_map(int node, bus_dma_tag_t dmat) +iommu_device_map_idx(int node, bus_dma_tag_t dmat, int idx) { uint32_t phandle, cells[2] = {0}; - if (iommu_device_lookup(node, &phandle, &cells[0])) + if (iommu_device_lookup_idx(node, &phandle, &cells[0], idx)) return dmat; return iommu_device_do_map(phandle, &cells[0], dmat); +} + +bus_dma_tag_t +iommu_device_map(int node, bus_dma_tag_t dmat) +{ + return iommu_device_map_idx(node, dmat, 0); +} + +bus_dma_tag_t +iommu_device_mirror_idx(int node, bus_dma_tag_t dmat, int idx) +{ + uint32_t phandle, cells[2] = {0}; + struct iommu_device *id; + + if (iommu_device_lookup_idx(node, &phandle, &cells[0], idx)) + return dmat; + + LIST_FOREACH(id, &iommu_devices, id_list) { + if (id->id_phandle == phandle) + return id->id_mirror(id->id_cookie, cells, dmat); + } + + return dmat; } bus_dma_tag_t Index: dev/ofw/ofw_misc.h =================================================================== RCS file: /cvs/src/sys/dev/ofw/ofw_misc.h,v diff -u -p -r1.31 ofw_misc.h --- dev/ofw/ofw_misc.h 21 Sep 2023 20:26:17 -0000 1.31 +++ dev/ofw/ofw_misc.h 11 Aug 2026 20:57:12 -0000 @@ -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); + bus_dma_tag_t (*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; @@ -281,6 +282,7 @@ void iommu_device_register(struct iommu_ int iommu_device_lookup(int, uint32_t *, uint32_t *); int iommu_device_lookup_pci(int, uint32_t, uint32_t *, uint32_t *); bus_dma_tag_t iommu_device_map(int, bus_dma_tag_t); +bus_dma_tag_t iommu_device_mirror_idx(int, bus_dma_tag_t, int); bus_dma_tag_t iommu_device_map_pci(int, uint32_t, bus_dma_tag_t); void iommu_reserve_region_pci(int, uint32_t, bus_addr_t, bus_size_t); Index: arch/arm64/dev/apldart.c =================================================================== RCS file: /cvs/src/sys/arch/arm64/dev/apldart.c,v diff -u -p -r1.22 apldart.c --- arch/arm64/dev/apldart.c 22 Jun 2026 07:54:19 -0000 1.22 +++ arch/arm64/dev/apldart.c 11 Aug 2026 20:57:14 -0000 @@ -143,6 +143,7 @@ apldart_trunc_offset(psize_t off) struct apldart_stream { struct apldart_softc *as_sc; int as_sid; + struct apldart_stream *as_mirror; struct extent *as_dvamap; struct mutex as_dvamap_mtx; @@ -219,6 +220,7 @@ struct cfdriver apldart_cd = { }; bus_dma_tag_t apldart_map(void *, uint32_t *, bus_dma_tag_t); +bus_dma_tag_t apldart_mirror(void *, uint32_t *, bus_dma_tag_t); void apldart_reserve(void *, uint32_t *, bus_addr_t, bus_size_t); int apldart_t8020_intr(void *); int apldart_t8110_intr(void *); @@ -257,7 +259,7 @@ apldart_attach(struct device *parent, st 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 +361,6 @@ apldart_attach(struct device *parent, st */ 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 +415,7 @@ apldart_attach(struct device *parent, st 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); } @@ -694,6 +682,63 @@ apldart_alloc_stream(struct apldart_soft return as; } +struct apldart_stream * +apldart_mirror_stream(struct apldart_softc *sc, int sid, + struct apldart_stream *mirror) +{ + struct apldart_stream *as; + paddr_t pa; + int idx, ntte, nl1, nl2; + uint32_t mask; + + as = malloc(sizeof(*as), M_DEVBUF, M_WAITOK | M_ZERO); + + as->as_sc = sc; + as->as_sid = sid; + as->as_mirror = mirror; + + KASSERT(!sc->sc_locked); + KASSERT(!sc->sc_translating); + + /* We shouldn't do mirrors of a mirror. */ + KASSERT(mirror->as_mirror == NULL); + + 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)); + KASSERT(nl1 <= sc->sc_nttbr); + + /* Install page tables from the mirrored stream. */ + pa = APLDART_DMA_DVA(mirror->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); + + memcpy(&as->as_dmat, sc->sc_dmat, sizeof(*sc->sc_dmat)); + as->as_dmat._cookie = as; + as->as_dmat._dmamap_create = apldart_dmamap_create; + as->as_dmat._dmamap_destroy = apldart_dmamap_destroy; + as->as_dmat._dmamap_load = apldart_dmamap_load; + as->as_dmat._dmamap_load_mbuf = apldart_dmamap_load_mbuf; + as->as_dmat._dmamap_load_uio = apldart_dmamap_load_uio; + as->as_dmat._dmamap_load_raw = apldart_dmamap_load_raw; + as->as_dmat._dmamap_unload = apldart_dmamap_unload; + as->as_dmat._flags |= BUS_DMA_COHERENT; + + return as; +} + bus_dma_tag_t apldart_map(void *cookie, uint32_t *cells, bus_dma_tag_t dmat) { @@ -704,7 +749,22 @@ apldart_map(void *cookie, uint32_t *cell if (sc->sc_as[sid] == NULL) sc->sc_as[sid] = apldart_alloc_stream(sc, sid); + + return &sc->sc_as[sid]->as_dmat; +} + +bus_dma_tag_t +apldart_mirror(void *cookie, uint32_t *cells, bus_dma_tag_t dmat) +{ + struct apldart_softc *sc = cookie; + uint32_t sid = cells[0]; + + KASSERT(sid < sc->sc_nsid); + KASSERT(dmat->_dmamap_create == apldart_dmamap_create); + if (sc->sc_as[sid] == NULL) + sc->sc_as[sid] = apldart_mirror_stream(sc, sid, dmat->_cookie); + return &sc->sc_as[sid]->as_dmat; } @@ -790,6 +850,14 @@ apldart_load_map(struct apldart_stream * volatile uint64_t *tte; int seg, error; + if (as->as_mirror) { + error = apldart_load_map(as->as_mirror, map, flags); + if (error) + return error; + sc->sc_flush_tlb(sc, as->as_sid); + return 0; + } + /* For each segment. */ for (seg = 0; seg < map->dm_nsegs; seg++) { paddr_t pa = map->dm_segs[seg]._ds_paddr; @@ -842,7 +910,6 @@ apldart_load_map(struct apldart_stream * } sc->sc_flush_tlb(sc, as->as_sid); - return 0; } @@ -853,6 +920,12 @@ apldart_unload_map(struct apldart_stream struct apldart_map_state *ams = map->_dm_cookie; volatile uint64_t *tte; int seg, error; + + if (as->as_mirror) { + apldart_unload_map(as->as_mirror, map); + sc->sc_flush_tlb(sc, as->as_sid); + return; + } /* For each segment. */ for (seg = 0; seg < map->dm_nsegs; seg++) {