On Sun, Jun 23, 2024 at 11:43:41PM +0200, Mark Kettenis wrote:
> > Date: Sun, 23 Jun 2024 22:55:20 +0200
> > From: Patrick Wildt <patrick@blueri.se>
> > 
> > Hi,
> > 
> > the Qualcomm Snapdragon X Elite (X1E80100) claims to support
> > 32 device-id bits, but it also seems to only allow a 4k pagesize
> > with 8 bytes per entry.  Without support for INDIRECT (status quo)
> > this yields a maximum deviceid of 0x1ffff, which is way less than the
> > 32 devbits.  Even with INDIRECT we would only get 0x3ffffff.
> > 
> > Hence let's only allocate as much as possibly configurable, and bail out
> > when someone tries to establish a device with it.
> > 
> > With this establishing nvme(4) on the Lenovo Yoga Slim 7x is still being
> > attempted, but will fail.  It would be nicer if the MSI map failed, but
> > with this the machine at least doesn't hang anymore on boot.
> 
> If the hardware really generated deviceids that can't be handled, they
> probably would have noticed that the hardware is broken.  So I don't
> think we need to worry about that.
> 
> ok kettenis@
> 
> I still think we want INDIRECT support.  That'll come after this?

Oh, we definitely need it because NVMe's device-id is 0xe0000 which is
out of scope without INDIRECT.  Here's the diff for INDIRECT, but right
now NVMe MSIs don't seem to work with this enabled...  So either no
NVMe w/o INDIRECT, or a hanging system w/ INDIRECT.

Patrick

diff --git a/sys/arch/arm64/dev/agintc.c b/sys/arch/arm64/dev/agintc.c
index 86b8aefc384..53c4b37444f 100644
--- a/sys/arch/arm64/dev/agintc.c
+++ b/sys/arch/arm64/dev/agintc.c
@@ -1577,6 +1577,7 @@ struct agintc_msi_softc {
 	struct agintc_dmamem		*sc_dtt;
 	size_t				sc_dtt_pgsz;
 	uint8_t				sc_dte_sz;
+	int				sc_dtt_indirect;
 	int				sc_cidbits;
 	struct agintc_dmamem		*sc_ctt;
 	size_t				sc_ctt_pgsz;
@@ -1705,12 +1706,30 @@ agintc_msi_attach(struct device *parent, struct device *self, void *aux)
 		size = (1ULL << sc->sc_devbits) * sc->sc_dte_sz;
 		size = roundup(size, sc->sc_dtt_pgsz);
 
+		/* Might make sense to go indirect */
+		if (size > 2 * sc->sc_dtt_pgsz) {
+			bus_space_write_8(sc->sc_iot, sc->sc_ioh, GITS_BASER(i),
+			    baser | GITS_BASER_INDIRECT);
+			if (bus_space_read_8(sc->sc_iot, sc->sc_ioh,
+			    GITS_BASER(i)) & GITS_BASER_INDIRECT)
+				sc->sc_dtt_indirect = 1;
+		}
+		if (sc->sc_dtt_indirect) {
+			size = (1ULL << sc->sc_devbits);
+			size /= (sc->sc_dtt_pgsz / sc->sc_dte_sz);
+			size *= sizeof(uint64_t);
+			size = roundup(size, sc->sc_dtt_pgsz);
+		}
+
 		/* Clamp down to maximum configurable num pages */
 		if (size / sc->sc_dtt_pgsz > GITS_BASER_SZ_MASK + 1)
 			size = (GITS_BASER_SZ_MASK + 1) * sc->sc_dtt_pgsz;
 
 		/* Calculate max deviceid based off configured size */
 		sc->sc_deviceid_max = (size / sc->sc_dte_sz) - 1;
+		if (sc->sc_dtt_indirect)
+			sc->sc_deviceid_max = ((size / sizeof(uint64_t)) *
+			    (sc->sc_dtt_pgsz / sc->sc_dte_sz)) - 1;
 
 		/* Allocate table. */
 		sc->sc_dtt = agintc_dmamem_alloc(sc->sc_dmat,
@@ -1725,7 +1744,9 @@ agintc_msi_attach(struct device *parent, struct device *self, void *aux)
 		KASSERT((dtt_pa & GITS_BASER_PA_MASK) == dtt_pa);
 		bus_space_write_8(sc->sc_iot, sc->sc_ioh, GITS_BASER(i),
 		    GITS_BASER_IC_NORM_NC | baser & GITS_BASER_PGSZ_MASK | 
-		    dtt_pa | (size / sc->sc_dtt_pgsz) - 1 | GITS_BASER_VALID);
+		    dtt_pa | (size / sc->sc_dtt_pgsz) - 1 |
+		    (sc->sc_dtt_indirect ? GITS_BASER_INDIRECT : 0) |
+		    GITS_BASER_VALID);
 	}
 
 	/* Set up collection translation table. */
@@ -1861,6 +1882,40 @@ agintc_msi_wait_cmd(struct agintc_msi_softc *sc)
 		printf("%s: command queue timeout\n", sc->sc_dev.dv_xname);
 }
 
+int
+agintc_msi_create_device_table(struct agintc_msi_softc *sc, uint32_t deviceid)
+{
+	uint64_t *table = AGINTC_DMA_KVA(sc->sc_dtt);
+	uint32_t idx = deviceid / (sc->sc_dtt_pgsz / sc->sc_dte_sz);
+	struct agintc_dmamem *dtt;
+	paddr_t dtt_pa;
+
+	/* Out of bounds */
+	if (deviceid > sc->sc_deviceid_max)
+		return ENXIO;
+
+	/* No need to adjust */
+	if (!sc->sc_dtt_indirect)
+		return 0;
+
+	/* Table already allocated */
+	if (table[idx])
+		return 0;
+
+	/* FIXME: leaks */
+	dtt = agintc_dmamem_alloc(sc->sc_dmat,
+	    sc->sc_dtt_pgsz, sc->sc_dtt_pgsz);
+	if (dtt == NULL)
+		return ENOMEM;
+
+	dtt_pa = AGINTC_DMA_DVA(dtt);
+	KASSERT((dtt_pa & GITS_BASER_PA_MASK) == dtt_pa);
+	table[idx] = dtt_pa | GITS_BASER_VALID;
+	cpu_dcache_wb_range((vaddr_t)&table[idx], sizeof(table[idx]));
+	__asm volatile("dsb sy");
+	return 0;
+}
+
 struct agintc_msi_device *
 agintc_msi_create_device(struct agintc_msi_softc *sc, uint32_t deviceid)
 {
@@ -1870,6 +1925,9 @@ agintc_msi_create_device(struct agintc_msi_softc *sc, uint32_t deviceid)
 	if (deviceid > sc->sc_deviceid_max)
 		return NULL;
 
+	if (agintc_msi_create_device_table(sc, deviceid) != 0)
+		return NULL;
+
 	md = malloc(sizeof(*md), M_DEVBUF, M_ZERO | M_WAITOK);
 	md->md_deviceid = deviceid;
 	md->md_itt = agintc_dmamem_alloc(sc->sc_dmat,