From: Jonathan Matthew <jonathan@d14n.org>
Subject: Re: ahci: fix interrupt issues
To: Jan Klemkow <jan@openbsd.org>
Cc: tech@openbsd.org
Date: Fri, 22 Nov 2024 17:01:26 +1000

On Thu, Nov 21, 2024 at 06:20:44PM +0100, Jan Klemkow wrote:
> Hi,
> 
> Attaching ahci(4) with newer AMD 600 Series AHCI controler hangs.  This is
> caused via atascsi_attach() which creates residual bits in the interrupt status
> register.  atascsi_attach() runs in polling mode, but sets the interrupt status
> bits anyway.  When these bits are already set, we never get an interrupt and
> run in a 60 second timeout for every ATA command send from sd(4) layer.  Which
> causes a kernel boot to last over an hour.
> 
> The SATA AHCI 1.3.1 specification also saids:
> 
> 	10.1.2 System Software Specific Initialization
> 
> 	[...] system software must always ensure that the PxIS (clear
> 	this first) and IS.IPS (clear this second) registers are cleared to ‘0’
> 	before programming the PxIE and GHC.IE registers.  This will prevent any
> 	residual bits set in these registers from causing an interrupt to be
> 	asserted.
> 
> The diff below simply clears the interupt status register, which fixes this
> issue.

We probably should clear the bit for the port in AHCI_REG_IS when we
complete a command in ahci_poll() (calling ahci_port_intr() will only
clear AHCI_PREG_IS), but since the spec directly tells us to do this,
we should do it either way.

ok jmatthew@

> 
> Tested with:
> ahci0 at pci12 dev 0 function 0 "AMD 600 Series AHCI" rev 0x01: msi, AHCI 1.3.1
> 
> ok?
> 
> bye,
> Jan
> 
> Index: dev/ic/ahci.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/ic/ahci.c,v
> diff -u -p -r1.42 ahci.c
> --- dev/ic/ahci.c	4 Sep 2024 07:54:52 -0000	1.42
> +++ dev/ic/ahci.c	21 Nov 2024 16:39:16 -0000
> @@ -321,6 +321,9 @@ noccc:
>  
>  	sc->sc_atascsi = atascsi_attach(&sc->sc_dev, &aaa);
>  
> +	/* Flush all residual bits of the interrupt status register */
> +	ahci_write(sc, AHCI_REG_IS, ahci_read(sc, AHCI_REG_IS));
> +
>  	/* Enable interrupts */
>  	ahci_write(sc, AHCI_REG_GHC, AHCI_REG_GHC_AE | AHCI_REG_GHC_IE);
>  
>