Index | Thread | Search

From:
Jonathan Matthew <jonathan@d14n.org>
Subject:
Re: nvme: use I/O submission queue entry size reported by controller
To:
tech@openbsd.org
Cc:
jcs@jcs.org
Date:
Tue, 19 May 2026 10:35:03 +1000

Download raw body.

Thread
On Sun, May 17, 2026 at 12:25:57PM -0500, joshua stein wrote:
> On at least the Apple T2 NVMe, 128-byte submission queue entries on
> I/O queues are required instead of the standard 64 bytes.
> 
> This gets NVMe working on the 2018 Mac Mini.  Also tested on a 
> non-Apple NVMe but more tests would be helpful.

I've tested this on a few different nvme devices (real and virtual) and
haven't seen any problems.

I'm curious as to why you're using the maximum supported sqe size rather
than the minimum.  Does the apple T2 nvme not report 128 as the minimum
size?

> 
> 
> diff --git sys/dev/ic/nvme.c sys/dev/ic/nvme.c
> index 67f88e1ecb9..de3607e2972 100644
> --- sys/dev/ic/nvme.c
> +++ sys/dev/ic/nvme.c
> @@ -1090,20 +1090,25 @@ void
>  nvme_q_submit(struct nvme_softc *sc, struct nvme_queue *q, struct nvme_ccb *ccb,
>      void (*fill)(struct nvme_softc *, struct nvme_ccb *, void *))
>  {
> -	struct nvme_sqe *sqe = NVME_DMA_KVA(q->q_sq_dmamem);
> +	struct nvme_sqe *sqe;
>  	u_int32_t tail;
> +	u_int sqe_size;
> +
> +	sqe_size = (q->q_id == NVME_ADMIN_Q) ? sizeof(struct nvme_sqe) :
> +	    sc->sc_sqe_size;
>  
>  	tail = sc->sc_ops->op_sq_enter(sc, q, ccb);
>  
> -	sqe += tail;
> +	sqe = (struct nvme_sqe *)((char *)NVME_DMA_KVA(q->q_sq_dmamem) +
> +	    (tail * sqe_size));
>  
>  	bus_dmamap_sync(sc->sc_dmat, NVME_DMA_MAP(q->q_sq_dmamem),
> -	    sizeof(*sqe) * tail, sizeof(*sqe), BUS_DMASYNC_POSTWRITE);
> -	memset(sqe, 0, sizeof(*sqe));
> +	    sqe_size * tail, sqe_size, BUS_DMASYNC_POSTWRITE);
> +	memset(sqe, 0, sqe_size);
>  	(*fill)(sc, ccb, sqe);
>  	sqe->cid = ccb->ccb_id;
>  	bus_dmamap_sync(sc->sc_dmat, NVME_DMA_MAP(q->q_sq_dmamem),
> -	    sizeof(*sqe) * tail, sizeof(*sqe), BUS_DMASYNC_PREWRITE);
> +	    sqe_size * tail, sqe_size, BUS_DMASYNC_PREWRITE);
>  
>  	sc->sc_ops->op_sq_leave(sc, q, ccb);
>  }
> @@ -1283,6 +1288,9 @@ nvme_identify(struct nvme_softc *sc, u_int mpsmin)
>  
>  	sc->sc_nn = lemtoh32(&identify->nn);
>  
> +	/* use maximum I/O SQE size reported */
> +	sc->sc_sqe_size = 1 << (identify->sqes >> 4);
> +
>  	/*
>  	 * At least one Apple NVMe device presents a second, bogus disk that is
>  	 * inaccessible, so cap targets at 1.
> @@ -1495,7 +1503,8 @@ nvme_q_alloc(struct nvme_softc *sc, u_int16_t id, u_int entries, u_int dstrd)
>  		return (NULL);
>  
>  	q->q_sq_dmamem = nvme_dmamem_alloc(sc,
> -	    sizeof(struct nvme_sqe) * entries);
> +	    (id == NVME_ADMIN_Q ? sizeof(struct nvme_sqe) : sc->sc_sqe_size) *
> +	    entries);
>  	if (q->q_sq_dmamem == NULL)
>  		goto free;
>  
> diff --git sys/dev/ic/nvmevar.h sys/dev/ic/nvmevar.h
> index efbf737ed23..a5292f51f4f 100644
> --- sys/dev/ic/nvmevar.h
> +++ sys/dev/ic/nvmevar.h
> @@ -111,6 +111,7 @@ struct nvme_softc {
>  	size_t			sc_mdts;
>  	u_int			sc_max_prpl;
>  	u_int			sc_dstrd;
> +	u_int			sc_sqe_size;
>  
>  	struct nvm_identify_controller
>  				sc_identify;
> diff --git sys/dev/pci/nvme_pci.c sys/dev/pci/nvme_pci.c
> index 7ad4daffb9f..a42065274cc 100644
> --- sys/dev/pci/nvme_pci.c
> +++ sys/dev/pci/nvme_pci.c
> @@ -67,7 +67,8 @@ nvme_pci_match(struct device *parent, void *match, void *aux)
>  
>  	if (PCI_VENDOR(pa->pa_id) == PCI_VENDOR_APPLE &&
>  	    (PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_APPLE_NVME1 ||
> -	    PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_APPLE_NVME2))
> +	    PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_APPLE_NVME2 ||
> +	    PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_APPLE_NVME3))
>  	    	return (1);
>  
>  	return (0);
>