linux-kernel - Re: [PATCH] ahci: libahci: clear pending interrupt status

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <23cc546e-bf87-b3c7-6b75-b895c1241a4b@kernel.org>
Date:   Thu, 10 Aug 2023 14:12:12 +0900
From:   Damien Le Moal <dlemoal@...nel.org>
To:     Szuying Chen <chensiying21@...il.com>, linux-ide@...r.kernel.org,
        linux-kernel@...r.kernel.org
Cc:     Jesse1_Chang@...edia.com.tw, Richard_Hsu@...edia.com.tw,
        Chloe_chen@...edia.com.tw
Subject: Re: [PATCH] ahci: libahci: clear pending interrupt status

On 8/10/23 14:05, Szuying Chen wrote:
> When ISR handle interface fatal error with error recovery after clear PxIS
> and PxIE. Another FIS(SDB FIS with err) that set PxIS.IFS to 1 is recevied
> during error recovery, which causing the HBA not issue any new commands
> after cmd.ST set 1.

This is not very clear. If there was a fatal error, the drive should be in
error state and no other SDB FIS can be received as the drive does absolutely
nothing while in error state (it only waits for a read log 10h command to be
issued to get it out of error state). So if you are seeing 2 SDB FIS with
errors one after the other, you have a buggy device...

However, I may be misunderstanding your issue here. Could you provide more
details and a dmesg output example of the issue ?

> 
> Signed-off-by: Szuying Chen <Chloe_Chen@...edia.com.tw>
> ---
>  drivers/ata/libahci.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
> index 06aec35f88f2..0ae51fd95d46 100644
> --- a/drivers/ata/libahci.c
> +++ b/drivers/ata/libahci.c
> @@ -679,9 +679,21 @@ static int ahci_scr_write(struct ata_link *link, unsigned int sc_reg, u32 val)
> 
>  void ahci_start_engine(struct ata_port *ap)
>  {
> +	struct ahci_host_priv *hpriv = ap->host->private_data;
>  	void __iomem *port_mmio = ahci_port_base(ap);
>  	u32 tmp;
> 
> +	/* clear SError */
> +	tmp = readl(port_mmio + PORT_SCR_ERR);
> +	writel(tmp, port_mmio + PORT_SCR_ERR);
> +
> +	/* clear port IRQ */
> +	tmp = readl(port_mmio + PORT_IRQ_STAT);
> +	if (tmp)
> +		writel(tmp, port_mmio + PORT_IRQ_STAT);
> +
> +	writel(1 << ap->port_no, hpriv->mmio + PORT_IRQ_STAT);
> +
>  	/* start DMA */
>  	tmp = readl(port_mmio + PORT_CMD);
>  	tmp |= PORT_CMD_START;
> --
> 2.39.2
> 

-- 
Damien Le Moal
Western Digital Research