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++) {
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.