Index | Thread | Search

From:
Jonathan Matthew <jonathan@d14n.org>
Subject:
Re: aplns(4) NVMMU fix
To:
Mark Kettenis <mark.kettenis@xs4all.nl>
Cc:
dlg@openbsd.org, jmatthew@openbsd.org, tech@openbsd.org
Date:
Sat, 15 Aug 2026 07:07:00 +1000

Download raw body.

Thread
  • Mark Kettenis:

    aplns(4) NVMMU fix

    • Jonathan Matthew:

      aplns(4) NVMMU fix

On Mon, Aug 10, 2026 at 10:01:14PM +0200, Mark Kettenis wrote:
> Sven Peter over at Asahi Linux figured out that we're not programming
> the integrated IOMMU of the Apple Silicon NVMe controller the right
> way.  It doesn't really need to opcode and we do need to program the
> "read" and "write" bits correctly.  If we don't do that we need to set
> this magic "null check" bit.  And that's what we do.
> 
> Hopwever, newer firmware, including all firmware available for
> machines with M4 and later, no longer supports this magic bit.  So we
> should really do the correct thing here.  Diff below does this,
> recognizing that the low bits of all NVMe commands endoing whether
> there is a data transfer involved and its direction.
> 
> Tested on my M2 Pro mini.
> 
> ok?

No hardware to test with currently, but this looks correct to me and
matches what m1n1 is doing now.
ok jmatthew@

> 
> 
> Index: arch/arm64/dev/aplns.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/arm64/dev/aplns.c,v
> diff -u -p -r1.19 aplns.c
> --- arch/arm64/dev/aplns.c	30 Jun 2026 16:24:33 -0000	1.19
> +++ arch/arm64/dev/aplns.c	10 Aug 2026 19:46:13 -0000
> @@ -51,8 +51,6 @@
>  #define ANS_BOOT_STATUS		0x01300
>  #define  ANS_BOOT_STATUS_OK	0xde71ce55
>  #define ANS_MODESEL_REG		0x01304
> -#define ANS_UNKNOWN_CTRL	0x24008
> -#define  ANS_PRP_NULL_CHECK	(1 << 11)
>  #define ANS_LINEAR_SQ_CTRL	0x24908
>  #define  ANS_LINEAR_SQ_CTRL_EN	(1 << 0)
>  #define ANS_LINEAR_ASQ_DB	0x2490c
> @@ -306,10 +304,6 @@ nvme_ans_init(struct nvme_ans_softc *asc
>  	bus_space_write_4(sc->sc_iot, sc->sc_ioh, ANS_MAX_PEND_CMDS_CTRL,
>  	    (ANS_MAX_QUEUE_DEPTH << 16) | ANS_MAX_QUEUE_DEPTH);
>  
> -	ctrl = bus_space_read_4(sc->sc_iot, sc->sc_ioh, ANS_UNKNOWN_CTRL);
> -	bus_space_write_4(sc->sc_iot, sc->sc_ioh, ANS_UNKNOWN_CTRL,
> -	    ctrl & ~ANS_PRP_NULL_CHECK);
> -
>  	return 0;
>  }
>  
> @@ -429,8 +423,10 @@ nvme_ans_sq_leave(struct nvme_softc *sc,
>  	    ANS_NVMMU_TCB_PITCH * id, sizeof(*tcb), BUS_DMASYNC_POSTWRITE);
>  
>  	memset(tcb, 0, sizeof(*tcb));
> -	tcb->tcb_opcode = sqe->opcode;
> -	tcb->tcb_flags = ANS_NVMMU_TCB_WRITE | ANS_NVMMU_TCB_READ;
> +	if (sqe->opcode & NVM_CMD_WRITE)
> +		tcb->tcb_flags |= ANS_NVMMU_TCB_READ;
> +	if (sqe->opcode & NVM_CMD_READ)
> +		tcb->tcb_flags |= ANS_NVMMU_TCB_WRITE;
>  	tcb->tcb_cid = id;
>  	tcb->tcb_prpl_len = sqe->nlb;
>  	tcb->tcb_prp[0] = sqe->entry.prp[0];
>