linux-kernel - Re: [REGRESSION][BISECTED] BU40N Blu-Ray drive broken since 7627a0edef54

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z4aLWmt1Y2jL1-pD@ryzen>
Date: Tue, 14 Jan 2025 17:05:46 +0100
From: Niklas Cassel <cassel@...nel.org>
To: Christian Heusel <christian@...sel.eu>
Cc: Mario Limonciello <mario.limonciello@....com>,
	Damien Le Moal <dlemoal@...nel.org>,
	Mika Westerberg <mika.westerberg@...ux.intel.com>,
	linux-ide@...r.kernel.org, linux-kernel@...r.kernel.org,
	regressions@...ts.linux.dev
Subject: Re: [REGRESSION][BISECTED] BU40N Blu-Ray drive broken since
 7627a0edef54

Hello Christian,

Looking at the Arch Linux thread, it seems that the person reporting the
problem says that even on a working kernel, you only see the device the
user presses the open button on the Blu-Ray player.

This suggests to me that it sends a hotplug/PhyRdy event when the user
presses the open button.

In Linux, if we don't detect anything connected to the port during initial
boot, and if the port is not marked (by firmware) as either Hotplug Capable,
or External, then we mask the PhyRdy interrupt.

We could reconsider this, and leave the PhyRdy interrupt enabled even for a
port that is not Hotplug Capable or External, if there is no devices detected
on the port.


However, a device should really show up after a COMRESET... A device that
does not do so seems to be non-spec compliant.
It would be nice if we could understand why this device does not show up
after a COMRESET. (If it did, we most likely would not need the workaround
suggested above.)


Could you please run with this debug prints, which might tell us what is
going on:


diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index fdfa7b266218..eab9fd3316a6 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -804,6 +804,18 @@ static void ahci_power_up(struct ata_port *ap)
        writel(cmd | PORT_CMD_ICC_ACTIVE, port_mmio + PORT_CMD);
 }
 
+static void ahci_dump_regs(struct ata_port *ap, char *str)
+{
+       void __iomem *port_mmio = ahci_port_base(ap);
+
+       ata_port_err(ap, "%s PxSSTS %#x PxSCTL %#x PxCMD %#x PxDEVSLP %#x\n",
+                    str,
+                    readl(port_mmio + PORT_SCR_STAT),
+                    readl(port_mmio + PORT_SCR_CTL),
+                    readl(port_mmio + PORT_CMD),
+                    readl(port_mmio + PORT_DEVSLP));
+}
+
 static int ahci_set_lpm(struct ata_link *link, enum ata_lpm_policy policy,
                        unsigned int hints)
 {
@@ -824,6 +836,9 @@ static int ahci_set_lpm(struct ata_link *link, enum ata_lpm_policy policy,
                pp->intr_mask &= ~PORT_IRQ_PHYRDY;
                writel(pp->intr_mask, port_mmio + PORT_IRQ_MASK);
 
+               if (!(ata_link_nr_enabled(link) > 0))
+                       ahci_dump_regs(ap, "before offline");
+
                sata_link_scr_lpm(link, policy, false);
        }
 
@@ -1607,8 +1622,15 @@ int ahci_do_hardreset(struct ata_link *link, unsigned int *class,
 
        ahci_port_clear_pending_irq(ap);
 
+       ahci_dump_regs(ap, "before wakeup");
+       ahci_power_up(ap);
+       ata_msleep(ap, 500);
+
+       ahci_dump_regs(ap, "before reset");
        rc = sata_link_hardreset(link, timing, deadline, online,
                                 ahci_check_ready);
+       ata_port_err(ap, "sata_link_hardreset returned: %d online: %d\n", rc, *online);
+       ahci_dump_regs(ap, "after reset");
 
        hpriv->start_engine(ap);
 
diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c
index 9c76fb1ad2ec..21ad2828385c 100644
--- a/drivers/ata/libata-sata.c
+++ b/drivers/ata/libata-sata.c
@@ -660,15 +660,17 @@ int sata_link_hardreset(struct ata_link *link, const unsigned int *timing,
        /* Couldn't find anything in SATA I/II specs, but AHCI-1.1
         * 10.4.2 says at least 1 ms.
         */
-       ata_msleep(link->ap, 1);
+       ata_msleep(link->ap, 500);
 
        /* bring link back */
        rc = sata_link_resume(link, timing, deadline);
        if (rc)
                goto out;
        /* if link is offline nothing more to do */
-       if (ata_phys_link_offline(link))
+       if (ata_phys_link_offline(link)) {
+               ata_port_err(link->ap, "phys link offline, no reason to wait\n");
                goto out;
+       }
 
        /* Link is online.  From this point, -ENODEV too is an error. */
        if (online)






Kind regards,
Niklas