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

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

Posted by Kyrylo Biryuk 1 week, 6 days ago

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.

Fixes: 89dafa20f3da ("ahci: disabled FBS prior to issuing software reset")
Assisted-by: LLM
Signed-off-by: Kyrylo Biryuk <illofspeed@gmail.com>
---

Notes:
    Changes in v4 (Niklas Cassel):
    - dropped the ata_port_info() print; ahci_enable_fbs() already logs
      "FBS is enabled"
    - moved the testing details out of the commit message
    Changes in v3: author name only. Changes in v2: author address only.
    
    Tested with the patch built as an out-of-tree libahci module for 7.2.6
    (the stock ahci module binds to it, symbol CRCs unchanged) on the
    88SE9235 + 88SM9705 setup from the commit message, which is passed
    through (VFIO) to a KVM guest. Hot-plugging a drive into a free port of
    the multiplier logged "FBS is disabled", "softreset failed (device not
    ready)" and a PMP hardreset, then "FBS is enabled" from
    ahci_enable_fbs(). 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.
    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 | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index 08ab56e9556..5441fbf543c 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -2233,6 +2233,21 @@ 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)
+			ahci_enable_fbs(ap);
+	}
+
 	if (!ata_dev_enabled(ap->link.device))
 		hpriv->stop_engine(ap);
 }
-- 
2.43.0

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

Posted by Damien Le Moal 1 week, 3 days ago

On 2026/10/12 12:39, 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.
> 
> Fixes: 89dafa20f3da ("ahci: disabled FBS prior to issuing software reset")
> Assisted-by: LLM
> Signed-off-by: Kyrylo Biryuk <illofspeed@gmail.com>

Looks good.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

-- 
Damien Le Moal
Western Digital Research

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

Posted by Niklas Cassel 1 week, 3 days ago

On Thu, Oct 15, 2026 at 07:37:45AM +0200, Damien Le Moal wrote:
> On 2026/10/12 12:39, 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.
> > 
> > Fixes: 89dafa20f3da ("ahci: disabled FBS prior to issuing software reset")
> > Assisted-by: LLM
> > Signed-off-by: Kyrylo Biryuk <illofspeed@gmail.com>
> 
> Looks good.
> 
> Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

Commit was updated to pick up Damien's tag:
https://git.kernel.org/pub/scm/linux/kernel/git/libata/linux.git/commit/?id=23eab9e877497397a0e2394ee9174d576816209e


Kind regards,
Niklas

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

Posted by Niklas Cassel 1 week, 6 days ago

On Mon, 12 Oct 2026 12:39:17 +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.
> 
> [...]

Applied to libata/linux.git (for-7.4), thanks!

[1/1] ata: libahci: re-enable FBS after error handling on a port multiplier
      https://git.kernel.org/libata/linux/c/21d333ab

Kind regards,
Niklas