Index | Thread | Search

From:
Heyang Zhou <hello@su3.io>
Subject:
apldart: stop using bypass mode
To:
tech@openbsd.org
Date:
Wed, 29 Jul 2026 22:43:54 +0800

Download raw body.

Thread
  • Heyang Zhou:

    apldart: stop using bypass mode

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;