Download raw body.
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;
apldart: stop using bypass mode