1. Patchew
  2. Seabios
  3. View series

[SeaBIOS] [PATCH v2] ahci: Fix hangs due to controller reset

Jiaxun Yang posted 1 patch 6 months, 3 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/seabios tags/patchew/20250528-ahci-v2-1-9d7310217ca2@flygoat.com
src/hw/ahci.c | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)
[SeaBIOS] [PATCH v2] ahci: Fix hangs due to controller reset
Posted by Jiaxun Yang 6 months, 3 weeks ago 
After adding AHCI controller reset functionality there are multiple
reports on AHCI booting regression.

As per my experiments on various machines, to reset controller
properly it is necessary to poll HOST_CTL_RESET bit until it's
clear. It is also required to read back HOST_CTL after changing
HOST_CTL_AHCI_EN bits to ensure the controller has accepted write.

Tested on ASMedia ASM1061, Intel H61 native SATA and AMD Phoenix
native SATA.

Link: https://mail.coreboot.org/hyperkitty/list/seabios@seabios.org/thread/RDNRKWBN4N5XQX2TQMM5P4WZ2OOPPNAM/
Link: https://github.com/FlyGoat/csmwrap/issues/14
Fixes: 8863cbbd15a7 ("ahci: add controller reset")
Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
Acked-by: Paul Menzel <pmenzel@molgen.mpg.de>
---
To: seabios@seabios.org
---
Changes in v2:
- Commit message (Paul Menzel)
- Link to v1: https://lore.kernel.org/r/20250522-ahci-v1-1-a9de195854f5@flygoat.com
---
 src/hw/ahci.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/src/hw/ahci.c b/src/hw/ahci.c
index 2285d33d4ae21335e5222ab04608685b5bff3763..7e7a03ddd010d53fa439c8ca49505a2da00241f4 100644
--- a/src/hw/ahci.c
+++ b/src/hw/ahci.c
@@ -660,8 +660,23 @@ ahci_controller_setup(struct pci_device *pci)
 
     pci_enable_busmaster(pci);
 
-    ahci_ctrl_writel(ctrl, HOST_CTL, HOST_CTL_RESET);
-    ahci_ctrl_writel(ctrl, HOST_CTL, HOST_CTL_AHCI_EN);
+    u32 val = ahci_ctrl_readl(ctrl, HOST_CTL);
+    ahci_ctrl_writel(ctrl, HOST_CTL, val | HOST_CTL_RESET);
+    u32 end = timer_calc(AHCI_RESET_TIMEOUT);
+    for (;;) {
+        val = ahci_ctrl_readl(ctrl, HOST_CTL);
+        if (!(val & HOST_CTL_RESET))
+            break;
+        if (timer_check(end)) {
+            warn_timeout();
+            dprintf(1, "AHCI: controller reset failed\n");
+            free(ctrl);
+            return;
+        }
+        yield();
+    }
+    ahci_ctrl_writel(ctrl, HOST_CTL, val | HOST_CTL_AHCI_EN);
+    (void)ahci_ctrl_readl(ctrl, HOST_CTL); /* Flush */
 
     ctrl->caps = ahci_ctrl_readl(ctrl, HOST_CAP);
     ctrl->ports = ahci_ctrl_readl(ctrl, HOST_PORTS_IMPL);

---
base-commit: 9029a010ec413e6c3c0eb52c29c252a5b9a9f774
change-id: 20250522-ahci-8ac63999b084

Best regards,
-- 
Jiaxun Yang <jiaxun.yang@flygoat.com>

_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org
[SeaBIOS] Re: [PATCH v2] ahci: Fix hangs due to controller reset
Posted by Gerd Hoffmann via SeaBIOS 6 months, 2 weeks ago 
On Wed, May 28, 2025 at 12:02:49PM +0100, Jiaxun Yang wrote:
> After adding AHCI controller reset functionality there are multiple
> reports on AHCI booting regression.
> 
> As per my experiments on various machines, to reset controller
> properly it is necessary to poll HOST_CTL_RESET bit until it's
> clear. It is also required to read back HOST_CTL after changing
> HOST_CTL_AHCI_EN bits to ensure the controller has accepted write.
> 
> Tested on ASMedia ASM1061, Intel H61 native SATA and AMD Phoenix
> native SATA.
> 
> Link: https://mail.coreboot.org/hyperkitty/list/seabios@seabios.org/thread/RDNRKWBN4N5XQX2TQMM5P4WZ2OOPPNAM/
> Link: https://github.com/FlyGoat/csmwrap/issues/14
> Fixes: 8863cbbd15a7 ("ahci: add controller reset")
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> Acked-by: Paul Menzel <pmenzel@molgen.mpg.de>

Applied & pushed.

take care,
  Gerd

_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org
[SeaBIOS] Re: [PATCH v2] ahci: Fix hangs due to controller reset
Posted by Paul Menzel 6 months, 3 weeks ago 
Dear Jiaxun,


Thank you for v2.

Am 28.05.25 um 13:02 schrieb Jiaxun Yang:
> After adding AHCI controller reset functionality there are multiple
> reports on AHCI booting regression.
> 
> As per my experiments on various machines, to reset controller
> properly it is necessary to poll HOST_CTL_RESET bit until it's
> clear. It is also required to read back HOST_CTL after changing
> HOST_CTL_AHCI_EN bits to ensure the controller has accepted write.
> 
> Tested on ASMedia ASM1061, Intel H61 native SATA and AMD Phoenix
> native SATA.
> 
> Link: https://mail.coreboot.org/hyperkitty/list/seabios@seabios.org/thread/RDNRKWBN4N5XQX2TQMM5P4WZ2OOPPNAM/
> Link: https://github.com/FlyGoat/csmwrap/issues/14
> Fixes: 8863cbbd15a7 ("ahci: add controller reset")
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> Acked-by: Paul Menzel <pmenzel@molgen.mpg.de>
> ---
> To: seabios@seabios.org
> ---
> Changes in v2:
> - Commit message (Paul Menzel)
> - Link to v1: https://lore.kernel.org/r/20250522-ahci-v1-1-a9de195854f5@flygoat.com
> ---
>   src/hw/ahci.c | 19 +++++++++++++++++--
>   1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/src/hw/ahci.c b/src/hw/ahci.c
> index 2285d33d4ae21335e5222ab04608685b5bff3763..7e7a03ddd010d53fa439c8ca49505a2da00241f4 100644
> --- a/src/hw/ahci.c
> +++ b/src/hw/ahci.c
> @@ -660,8 +660,23 @@ ahci_controller_setup(struct pci_device *pci)
>   
>       pci_enable_busmaster(pci);
>   
> -    ahci_ctrl_writel(ctrl, HOST_CTL, HOST_CTL_RESET);
> -    ahci_ctrl_writel(ctrl, HOST_CTL, HOST_CTL_AHCI_EN);
> +    u32 val = ahci_ctrl_readl(ctrl, HOST_CTL);
> +    ahci_ctrl_writel(ctrl, HOST_CTL, val | HOST_CTL_RESET);
> +    u32 end = timer_calc(AHCI_RESET_TIMEOUT);
> +    for (;;) {
> +        val = ahci_ctrl_readl(ctrl, HOST_CTL);
> +        if (!(val & HOST_CTL_RESET))
> +            break;
> +        if (timer_check(end)) {
> +            warn_timeout();
> +            dprintf(1, "AHCI: controller reset failed\n");

Although not common in the current code base, it might be useful (for 
the user) to log `AHCI_RESET_TIMEOUT`, and maybe even `val`. But it 
could be added in a follow-up, should too many reports with this reset 
problem come in.

> +            free(ctrl);
> +            return;
> +        }
> +        yield();
> +    }
> +    ahci_ctrl_writel(ctrl, HOST_CTL, val | HOST_CTL_AHCI_EN);
> +    (void)ahci_ctrl_readl(ctrl, HOST_CTL); /* Flush */
>   
>       ctrl->caps = ahci_ctrl_readl(ctrl, HOST_CAP);
>       ctrl->ports = ahci_ctrl_readl(ctrl, HOST_PORTS_IMPL);
Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>


Kind regards,

Paul
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.