Re: [PATCH v3] ata: libahci: re-enable FBS after error handling on a port multiplier

[Niklas Cassel]

On Sun, Oct 11, 2026 at 08:02:36PM +0200, Kyrylo Biryuk wrote:
> ahci_do_softreset() disables FBS before resetting a device behind a port
> multiplier, as required by AHCI 1.2 section 9.3.9, and re-enables it only
> when that softreset succeeds. If it fails, for example because a
> hot-plugged drive is still spinning up ("softreset failed (device not
> ready)"), EH escalates to a hardreset of the PMP link, and nothing turns
> FBS back on: only ahci_pmp_attach() does, which runs at the next boot.
> The port keeps working with command-based switching until then.
> 
> Seen on a Marvell 88SE9235 (1b4b:9235) with Marvell 88SM9705 port
> multipliers (1b4b:9705, in a Synology DX1222 expansion unit): after
> hot-plugging a drive, the kernel logged "FBS is disabled" followed by
> "softreset failed (device not ready)" and a PMP hardreset, and never
> "FBS is enabled" again (twice, on two different ports).
> 
> At the end of ahci_error_handler(), re-enable FBS if a PMP is still
> attached and FBS is supported but disabled.
> 
> Tested on 7.2.6 with the patch built as an out-of-tree libahci (the stock
> ahci module binds to it, symbol CRCs are unchanged). Hot-plugging a drive
> into a free port of the multiplier then logged "FBS is disabled",
> "softreset failed (device not ready)" and a PMP hardreset, followed by
> "re-enabling FBS after error handling" and "FBS is enabled". Afterwards
> two other drives behind the same multiplier read 259 + 259 MB/s in
> parallel (O_DIRECT, 4 MiB blocks), against 268 MB/s for one drive alone,
> so their commands overlap again.

This whole testing paragraph belongs after the '---'.
Having specific information about exact kernel version, or if the module
was built as an out of tree module, or built-in, is not information that
should be in the commit log itself.


> 
> Fixes: 89dafa20f3da ("ahci: disabled FBS prior to issuing software reset")
> Assisted-by: LLM
> Signed-off-by: Kyrylo Biryuk <illofspeed@xxxxxxxxx>
> ---
> 
> Notes:
>     v3: no code changes. From and Signed-off-by now carry my full name
>     instead of the GitHub handle "illofspeed" used for v2 (same author).
>     
>     The controller is passed through (VFIO) to a KVM guest; the tested kernel is the
>     guest's. The two earlier occurrences without the patch were on ata8 and ata10 of the
>     same controller; this test was on ata7.
> 
>  drivers/ata/libahci.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
> index 08ab56e9556..6f374fd79f5 100644
> --- a/drivers/ata/libahci.c
> +++ b/drivers/ata/libahci.c
> @@ -2233,6 +2233,23 @@ void ahci_error_handler(struct ata_port *ap)
>  
>  	sata_pmp_error_handler(ap);
>  
> +	/*
> +	 * ahci_do_softreset() turns FBS off before resetting a fan-out link
> +	 * and turns it back on only if that reset succeeds. When it fails
> +	 * (e.g. a hot-plugged drive is still spinning up), EH escalates to a
> +	 * PMP hardreset, which never re-enables FBS, so the port would keep
> +	 * command-based switching until the PMP is attached again (reboot).
> +	 * Restore FBS once recovery is done and the PMP is still attached.
> +	 */
> +	if (sata_pmp_attached(ap) && ata_dev_enabled(ap->link.device)) {
> +		struct ahci_port_priv *pp = ap->private_data;
> +
> +		if (pp->fbs_supported && !pp->fbs_enabled) {
> +			ata_port_info(ap, "re-enabling FBS after error handling\n");

I would drop this print.


Kind regards,
Niklas