Hello SeaBIOS community,
Please look at the patch I've attached to this email. It fixes a regression recently introduced in SeaBIOS, introduced by SeaBIOS git commit ID 8863cbbd. I request that this patch by merged, and I will now explain why.
A recent SeaBIOS patch (commit 8863cbbd) adds AHCI controller reset prior to enablement, as a way to make SeaBIOS's AHCI driver work correctly on CSM-based setups.
However, that commit (commit 8863cbbd) broke AHCI initialisation on the Lenovo ThinkPad T420 when testing SeaBIOS as a coreboot payload. It caused an AHCI timeout. I tried to include these logs on the mailing list but it came back telling me my message was too big.
AnywaysTo mitigate this, I have provided for SeaBIOS a patch to this email (0004-ahci-Only-reset-controller-on-CSM.patch). My patch retains the new behaviour, resetting the AHCI controller prior to enablement, so that it will continue to function correctly on CSM, but it *only* applies that new behaviour on CSM setups.
With my patch, the *old* behaviour is used (enable the controller from its current state), if a CSM is not in use. This was confirmed to work correctly, fixing the issue when re-tested on the same ThinkPad T420.
I am also using this fix in Libreboot, see: https://browse.libreboot.org/lbmk.git/commit/?id=c073ee9d4fc4a631c16ff681bb62c29b952878baThat's where the patch is from. I put it there first. SeaBIOS devs are welcome to merge this if they wish.
-- Company director, Minifree Ltd Registered in England, No. 9361826 | VAT No. GB202190462 Registered Office: 19 Hilton Road, Canvey Island, Essex SS8 9QA, UK
From 5fe2215bc5196d836b54e1e5fb00b63fa096fda7 Mon Sep 17 00:00:00 2001
From: Leah Rowe <l...@libreboot.org>
Date: Thu, 1 May 2025 15:46:54 +0100
Subject: [PATCH 4/4] ahci: Only reset controller on CSM
Please refer to this commit:
commit 8863cbbd15a73b03153553c562f5b1fb939ad4d7
Author: Gerd Hoffmann <kra...@redhat.com>
Date: Thu Feb 6 12:10:21 2025 +0100
ahci: add controller reset
This commit broke AHCI init on the Lenovo ThinkPad T420,
when tested with SeaBIOS as a coreboot payload.
Since the above commit was made with CSMs in mind, to make
the AHCI driver work there, that change has been re-worked
so as to only apply when a CSM is in use.
Signed-off-by: Leah Rowe <l...@libreboot.org>
---
src/hw/ahci.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/src/hw/ahci.c b/src/hw/ahci.c
index 2285d33d..b09b198a 100644
--- a/src/hw/ahci.c
+++ b/src/hw/ahci.c
@@ -637,7 +637,7 @@ static void
ahci_controller_setup(struct pci_device *pci)
{
struct ahci_port_s *port;
- u32 pnr, max;
+ u32 val, pnr, max;
if (create_bounce_buf() < 0)
return;
@@ -660,8 +660,19 @@ 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);
+ /* AHCI controller reset, but only for CSM. Commit 8863cbbd
+ introduced this universally, to make AHCI drivers work in
+ CSM mode, but it broke AHCI setup on the ThinkPad T420 when
+ SeaBIOS is used as a coreboot payload, hence the else clause: */
+ if (CONFIG_CSM) {
+ /* Enable AHCI controller after resetting its state */
+ ahci_ctrl_writel(ctrl, HOST_CTL, HOST_CTL_RESET);
+ ahci_ctrl_writel(ctrl, HOST_CTL, HOST_CTL_AHCI_EN);
+ } else {
+ /* Enable AHCI controller from its current state */
+ val = ahci_ctrl_readl(ctrl, HOST_CTL);
+ ahci_ctrl_writel(ctrl, HOST_CTL, val | HOST_CTL_AHCI_EN);
+ }
ctrl->caps = ahci_ctrl_readl(ctrl, HOST_CAP);
ctrl->ports = ahci_ctrl_readl(ctrl, HOST_PORTS_IMPL);
--
2.39.5
OpenPGP_0x5C654067D383B1FF.asc
Description: OpenPGP public key
OpenPGP_signature.asc
Description: OpenPGP digital signature
_______________________________________________ SeaBIOS mailing list -- seabios@seabios.org To unsubscribe send an email to seabios-le...@seabios.org
