[PATCH] ahci: don't ignore result code of ahci_reset_controller()

[PATCH] ahci: don't ignore result code of ahci_reset_controller()

[Loic]

Hello,

Please picked up this patch for linux 4.4 and 4.9.
Compiled/tested without problem.

[ Upstream commit d312fefea8387503375f728855c9a62de20c9665 ]

From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Date: Mon, 2 Oct 2017 19:31:24 +0100
Subject: [PATCH] ahci: don't ignore result code of ahci_reset_controller()

ahci_pci_reset_controller() calls ahci_reset_controller(), which may
fail, but ignores the result code and always returns success. This
may result in failures like below

  ahci 0000:02:00.0: version 3.0
  ahci 0000:02:00.0: enabling device (0000 -> 0003)
  ahci 0000:02:00.0: SSS flag set, parallel bus scan disabled
  ahci 0000:02:00.0: controller reset failed (0xffffffff)
  ahci 0000:02:00.0: failed to stop engine (-5)
    ... repeated many times ...
  ahci 0000:02:00.0: failed to stop engine (-5)
  Unable to handle kernel paging request at virtual address ffff0000093f9018
    ...
  PC is at ahci_stop_engine+0x5c/0xd8 [libahci]
  LR is at ahci_deinit_port.constprop.12+0x1c/0xc0 [libahci]
    ...
  [<ffff000000a17014>] ahci_stop_engine+0x5c/0xd8 [libahci]
  [<ffff000000a196b4>] ahci_deinit_port.constprop.12+0x1c/0xc0 [libahci]
  [<ffff000000a197d8>] ahci_init_controller+0x80/0x168 [libahci]
  [<ffff000000a260f8>] ahci_pci_init_controller+0x60/0x68 [ahci]
  [<ffff000000a26f94>] ahci_init_one+0x75c/0xd88 [ahci]
  [<ffff000008430324>] local_pci_probe+0x3c/0xb8
  [<ffff000008431728>] pci_device_probe+0x138/0x170
  [<ffff000008585e54>] driver_probe_device+0x2dc/0x458
  [<ffff0000085860e4>] __driver_attach+0x114/0x118
  [<ffff000008583ca8>] bus_for_each_dev+0x60/0xa0
  [<ffff000008585638>] driver_attach+0x20/0x28
  [<ffff0000085850b0>] bus_add_driver+0x1f0/0x2a8
  [<ffff000008586ae0>] driver_register+0x60/0xf8
  [<ffff00000842f9b4>] __pci_register_driver+0x3c/0x48
  [<ffff000000a3001c>] ahci_pci_driver_init+0x1c/0x1000 [ahci]
  [<ffff000008083918>] do_one_initcall+0x38/0x120

where an obvious hardware level failure results in an unnecessary 15 second
delay and a subsequent crash.

So record the result code of ahci_reset_controller() and relay it, rather
than ignoring it.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Tejun Heo <tj@kernel.org>
---
 drivers/ata/ahci.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index cb9b0e9090e3b..9f78bb03bb763 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -621,8 +621,11 @@ static void ahci_pci_save_initial_config(struct pci_dev *pdev,
 static int ahci_pci_reset_controller(struct ata_host *host)
 {
 	struct pci_dev *pdev = to_pci_dev(host->dev);
+	int rc;
 
-	ahci_reset_controller(host);
+	rc = ahci_reset_controller(host);
+	if (rc)
+		return rc;
 
 	if (pdev->vendor == PCI_VENDOR_ID_INTEL) {
 		struct ahci_host_priv *hpriv = host->private_data;

Re: [PATCH] ahci: don't ignore result code of ahci_reset_controller()

[Sasha Levin]

On Sat, Oct 20, 2018 at 07:55:57PM +0200, Loic wrote:
>Hello,
>
>Please picked up this patch for linux 4.4 and 4.9.
>Compiled/tested without problem.
>
>[ Upstream commit d312fefea8387503375f728855c9a62de20c9665 ]
>
>From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>Date: Mon, 2 Oct 2017 19:31:24 +0100
>Subject: [PATCH] ahci: don't ignore result code of ahci_reset_controller()
>
>ahci_pci_reset_controller() calls ahci_reset_controller(), which may
>fail, but ignores the result code and always returns success. This
>may result in failures like below
>
>  ahci 0000:02:00.0: version 3.0
>  ahci 0000:02:00.0: enabling device (0000 -> 0003)
>  ahci 0000:02:00.0: SSS flag set, parallel bus scan disabled
>  ahci 0000:02:00.0: controller reset failed (0xffffffff)
>  ahci 0000:02:00.0: failed to stop engine (-5)
>    ... repeated many times ...
>  ahci 0000:02:00.0: failed to stop engine (-5)
>  Unable to handle kernel paging request at virtual address ffff0000093f9018
>    ...
>  PC is at ahci_stop_engine+0x5c/0xd8 [libahci]
>  LR is at ahci_deinit_port.constprop.12+0x1c/0xc0 [libahci]
>    ...
>  [<ffff000000a17014>] ahci_stop_engine+0x5c/0xd8 [libahci]
>  [<ffff000000a196b4>] ahci_deinit_port.constprop.12+0x1c/0xc0 [libahci]
>  [<ffff000000a197d8>] ahci_init_controller+0x80/0x168 [libahci]
>  [<ffff000000a260f8>] ahci_pci_init_controller+0x60/0x68 [ahci]
>  [<ffff000000a26f94>] ahci_init_one+0x75c/0xd88 [ahci]
>  [<ffff000008430324>] local_pci_probe+0x3c/0xb8
>  [<ffff000008431728>] pci_device_probe+0x138/0x170
>  [<ffff000008585e54>] driver_probe_device+0x2dc/0x458
>  [<ffff0000085860e4>] __driver_attach+0x114/0x118
>  [<ffff000008583ca8>] bus_for_each_dev+0x60/0xa0
>  [<ffff000008585638>] driver_attach+0x20/0x28
>  [<ffff0000085850b0>] bus_add_driver+0x1f0/0x2a8
>  [<ffff000008586ae0>] driver_register+0x60/0xf8
>  [<ffff00000842f9b4>] __pci_register_driver+0x3c/0x48
>  [<ffff000000a3001c>] ahci_pci_driver_init+0x1c/0x1000 [ahci]
>  [<ffff000008083918>] do_one_initcall+0x38/0x120
>
>where an obvious hardware level failure results in an unnecessary 15 second
>delay and a subsequent crash.
>
>So record the result code of ahci_reset_controller() and relay it, rather
>than ignoring it.
>
>Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>Signed-off-by: Tejun Heo <tj@kernel.org>

Queued for 4.4 and 4.9, thanks.

--
Thanks,
Sasha