via boot up hang during probing for no sata disks

via boot up hang during probing for no sata disks

[Andi Kleen]

One of my test machines hangs at boot when booting 2.6.18rc4.

It seems to busy loop somewhere because the CPU fans spin up.

The machine doesn't have any disks (just two PATAPI CD-ROMs) 
and boots over the network.

The SATA drivers are compiled into the kernel (x86-64 defconfig)

Strangely it seems to only hang with the 64bit kernel -- when
I boot a 32bit kernel it usually comes up. It is likely some timing
issue because booting with initcall_debug also seems to make 
the problem go away. I verified with a printk and without initcall_debug
that the hang still happens in svia_init.

I haven't seen that with earlier kernels.

libata version 2.00 loaded.
sata_via 0000:00:0f.0: version 2.0
GSI 17 sharing vector 0xB9 and IRQ 17
IOAPIC[0]: Set PCI routing entry (1-20 -> 0xb9 -> IRQ 17 Mode:1 Active:1)
ACPI: PCI Interrupt 0000:00:0f.0[B] -> GSI 20 (level, low) -> IRQ 185
sata_via 0000:00:0f.0: routed to hard irq line 9
ata1: SATA max UDMA/133 cmd 0x2C30 ctl 0x2C26 bmdma 0x2C00 irq 185
ata2: SATA max UDMA/133 cmd 0x2C28 ctl 0x2C22 bmdma 0x2C08 irq 185
scsi0 : sata_via
ata1: SATA link down (SStatus 0 SControl 310)
ATA: abnormal status 0x7F on port 0x2C37
scsi1 : sata_via
ata2: SATA link down (SStatus 0 SControl 310)
ATA: abnormal status 0x7F on port 0x2C2F
<hang>

Chipset is 
00:0f.0 IDE interface: VIA Technologies, Inc.: Unknown device 3149 (rev 80)
00:0f.1 IDE interface: VIA Technologies, Inc. VT82C586A/B/VT82C686/A/B/VT8233/A/C/VT8235 PIPC Bus Master IDE (rev 06)

I will disable the driver for now because I don't need it
(after all the machine has no disks), but I wanted to let you now.

-Andi

Re: via boot up hang during probing for no sata disks

[Tejun Heo]

On Mon, Aug 14, 2006 at 03:33:35PM +0200, Andi Kleen wrote:
> 
> One of my test machines hangs at boot when booting 2.6.18rc4.
> 
> It seems to busy loop somewhere because the CPU fans spin up.
> 
> The machine doesn't have any disks (just two PATAPI CD-ROMs) 
> and boots over the network.
> 
> The SATA drivers are compiled into the kernel (x86-64 defconfig)
> 
> Strangely it seems to only hang with the 64bit kernel -- when
> I boot a 32bit kernel it usually comes up. It is likely some timing
> issue because booting with initcall_debug also seems to make 
> the problem go away. I verified with a printk and without initcall_debug
> that the hang still happens in svia_init.

Hello, Andi.

Can you try the following patch?

diff --git a/drivers/scsi/sata_via.c b/drivers/scsi/sata_via.c
index 03baec2..efa865d 100644
--- a/drivers/scsi/sata_via.c
+++ b/drivers/scsi/sata_via.c
@@ -74,6 +74,8 @@ enum {
 static int svia_init_one (struct pci_dev *pdev, const struct pci_device_id *ent);
 static u32 svia_scr_read (struct ata_port *ap, unsigned int sc_reg);
 static void svia_scr_write (struct ata_port *ap, unsigned int sc_reg, u32 val);
+static void vt6420_error_handler(struct ata_port *ap);
+static void vt6421_error_handler(struct ata_port *ap);
 
 static const struct pci_device_id svia_pci_tbl[] = {
 	{ 0x1106, 0x3149, PCI_ANY_ID, PCI_ANY_ID, 0, 0, vt6420 },
@@ -107,7 +109,7 @@ static struct scsi_host_template svia_sh
 	.bios_param		= ata_std_bios_param,
 };
 
-static const struct ata_port_operations svia_sata_ops = {
+static const struct ata_port_operations vt6420_sata_ops = {
 	.port_disable		= ata_port_disable,
 
 	.tf_load		= ata_tf_load,
@@ -127,7 +129,7 @@ static const struct ata_port_operations 
 
 	.freeze			= ata_bmdma_freeze,
 	.thaw			= ata_bmdma_thaw,
-	.error_handler		= ata_bmdma_error_handler,
+	.error_handler		= vt6420_error_handler,
 	.post_internal_cmd	= ata_bmdma_post_internal_cmd,
 
 	.irq_handler		= ata_interrupt,
@@ -141,13 +143,47 @@ static const struct ata_port_operations 
 	.host_stop		= ata_host_stop,
 };
 
-static struct ata_port_info svia_port_info = {
+static const struct ata_port_operations vt6421_sata_ops = {
+	.port_disable		= ata_port_disable,
+
+	.tf_load		= ata_tf_load,
+	.tf_read		= ata_tf_read,
+	.check_status		= ata_check_status,
+	.exec_command		= ata_exec_command,
+	.dev_select		= ata_std_dev_select,
+
+	.bmdma_setup            = ata_bmdma_setup,
+	.bmdma_start            = ata_bmdma_start,
+	.bmdma_stop		= ata_bmdma_stop,
+	.bmdma_status		= ata_bmdma_status,
+
+	.qc_prep		= ata_qc_prep,
+	.qc_issue		= ata_qc_issue_prot,
+	.data_xfer		= ata_pio_data_xfer,
+
+	.freeze			= ata_bmdma_freeze,
+	.thaw			= ata_bmdma_thaw,
+	.error_handler		= vt6421_error_handler,
+	.post_internal_cmd	= ata_bmdma_post_internal_cmd,
+
+	.irq_handler		= ata_interrupt,
+	.irq_clear		= ata_bmdma_irq_clear,
+
+	.scr_read		= svia_scr_read,
+	.scr_write		= svia_scr_write,
+
+	.port_start		= ata_port_start,
+	.port_stop		= ata_port_stop,
+	.host_stop		= ata_host_stop,
+};
+
+static struct ata_port_info vt6420_port_info = {
 	.sht		= &svia_sht,
 	.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY,
 	.pio_mask	= 0x1f,
 	.mwdma_mask	= 0x07,
 	.udma_mask	= 0x7f,
-	.port_ops	= &svia_sata_ops,
+	.port_ops	= &vt6420_sata_ops,
 };
 
 MODULE_AUTHOR("Jeff Garzik");
@@ -170,6 +206,48 @@ static void svia_scr_write (struct ata_p
 	outl(val, ap->ioaddr.scr_addr + (4 * sc_reg));
 }
 
+static int vt6420_prereset(struct ata_port *ap)
+{
+	struct ata_eh_context *ehc = &ap->eh_context;
+	unsigned long timeout = jiffies + (HZ * 5);
+	u32 sstatus;
+
+	/* if we're about to do hardreset, nothing more to do */
+	if (ehc->i.action & ATA_EH_HARDRESET)
+		return 0;
+
+	/* Resume phy.  SCR registers in vt6420 are super-fragile and
+	 * can cause system lock up depending on how it's accessed.
+	 * Use the old resume sequence from __sata_phy_reset().
+	 */
+	sata_scr_write_flush(ap, SCR_CONTROL, 0x300);
+
+	/* wait for phy to become ready, if necessary */
+	do {
+		msleep(200);
+		sata_scr_read(ap, SCR_STATUS, &sstatus);
+		if ((sstatus & 0xf) != 1)
+			break;
+	} while (time_before(jiffies, timeout));
+
+	/* wait for !BSY */
+	ata_busy_sleep(ap, ATA_TMOUT_BOOT_QUICK, ATA_TMOUT_BOOT);
+
+	return 0;
+}
+
+static void vt6420_error_handler(struct ata_port *ap)
+{
+	return ata_bmdma_drive_eh(ap, vt6420_prereset, ata_std_softreset,
+				  sata_std_hardreset, ata_std_postreset);
+}
+
+static void vt6421_error_handler(struct ata_port *ap)
+{
+	return ata_bmdma_drive_eh(ap, ata_std_prereset, NULL,
+				  sata_std_hardreset, ata_std_postreset);
+}
+
 static const unsigned int svia_bar_sizes[] = {
 	8, 4, 8, 4, 16, 256
 };
@@ -210,7 +288,7 @@ static void vt6421_init_addrs(struct ata
 static struct ata_probe_ent *vt6420_init_probe_ent(struct pci_dev *pdev)
 {
 	struct ata_probe_ent *probe_ent;
-	struct ata_port_info *ppi = &svia_port_info;
+	struct ata_port_info *ppi = &vt6420_port_info;
 
 	probe_ent = ata_pci_init_native_mode(pdev, &ppi, ATA_PORT_PRIMARY | ATA_PORT_SECONDARY);
 	if (!probe_ent)
@@ -239,7 +317,7 @@ static struct ata_probe_ent *vt6421_init
 
 	probe_ent->sht		= &svia_sht;
 	probe_ent->host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY;
-	probe_ent->port_ops	= &svia_sata_ops;
+	probe_ent->port_ops	= &vt6421_sata_ops;
 	probe_ent->n_ports	= N_PORTS;
 	probe_ent->irq		= pdev->irq;
 	probe_ent->irq_flags	= IRQF_SHARED;

Re: via boot up hang during probing for no sata disks

[Andi Kleen]

On Wed, 16 Aug 2006 14:12:28 +0900
Tejun Heo <htejun@gmail.com> wrote:

> On Mon, Aug 14, 2006 at 03:33:35PM +0200, Andi Kleen wrote:
> > 
> > One of my test machines hangs at boot when booting 2.6.18rc4.
> > 
> > It seems to busy loop somewhere because the CPU fans spin up.
> > 
> > The machine doesn't have any disks (just two PATAPI CD-ROMs) 
> > and boots over the network.
> > 
> > The SATA drivers are compiled into the kernel (x86-64 defconfig)
> > 
> > Strangely it seems to only hang with the 64bit kernel -- when
> > I boot a 32bit kernel it usually comes up. It is likely some timing
> > issue because booting with initcall_debug also seems to make 
> > the problem go away. I verified with a printk and without initcall_debug
> > that the hang still happens in svia_init.
> 
> Hello, Andi.
> 
> Can you try the following patch?

This patch seems to fix the problem. Thanks.

I get now just

libata version 2.00 loaded.
sata_via 0000:00:0f.0: version 2.0
GSI 17 sharing vector 0xB9 and IRQ 17
IOAPIC[0]: Set PCI routing entry (1-20 -> 0xb9 -> IRQ 17 Mode:1 Active:1)
ACPI: PCI Interrupt 0000:00:0f.0[B] -> GSI 20 (level, low) -> IRQ 185
sata_via 0000:00:0f.0: routed to hard irq line 9
ata1: SATA max UDMA/133 cmd 0x2C30 ctl 0x2C26 bmdma 0x2C00 irq 185
ata2: SATA max UDMA/133 cmd 0x2C28 ctl 0x2C22 bmdma 0x2C08 irq 185
scsi0 : sata_via
ata1: SATA link down (SStatus 0 SControl 300)
ATA: abnormal status 0x7F on port 0x2C37
scsi1 : sata_via
ata2: SATA link down (SStatus 0 SControl 300)
ATA: abnormal status 0x7F on port 0x2C2F
... continues booting without any noticeable delay...


-Andi

[PATCH] sata_via: use old SCR access pattern on vt6420

[Tejun Heo]

vt6420 has super-fragile SCR registers which can hang the whole
machine if accessed with the wrong timings.  This patch makes sata_via
use SCR registers only during probing and with the same timings as
before (pre new EH), which is proven to work.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---

Andi, can you please test this patch?  This adds two more SCR accesses
for link status printing (as in the pre EH code) and prevents all SCR
accesses once probing is complete.  This should work but I wanna make
sure.

Jeff, if Andi confirms, this one is good for #uptream-fixes.  It fixes
a apparent regression.  I'll post patches for vt6421 soon.

Thanks.

 drivers/scsi/sata_via.c |  117 +++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 112 insertions(+), 5 deletions(-)

Index: work-c3/drivers/scsi/sata_via.c
===================================================================
--- work-c3.orig/drivers/scsi/sata_via.c
+++ work-c3/drivers/scsi/sata_via.c
@@ -74,6 +74,7 @@ enum {
 static int svia_init_one (struct pci_dev *pdev, const struct pci_device_id *ent);
 static u32 svia_scr_read (struct ata_port *ap, unsigned int sc_reg);
 static void svia_scr_write (struct ata_port *ap, unsigned int sc_reg, u32 val);
+static void vt6420_error_handler(struct ata_port *ap);
 
 static const struct pci_device_id svia_pci_tbl[] = {
 	{ 0x1106, 0x3149, PCI_ANY_ID, PCI_ANY_ID, 0, 0, vt6420 },
@@ -107,7 +108,38 @@ static struct scsi_host_template svia_sh
 	.bios_param		= ata_std_bios_param,
 };
 
-static const struct ata_port_operations svia_sata_ops = {
+static const struct ata_port_operations vt6420_sata_ops = {
+	.port_disable		= ata_port_disable,
+
+	.tf_load		= ata_tf_load,
+	.tf_read		= ata_tf_read,
+	.check_status		= ata_check_status,
+	.exec_command		= ata_exec_command,
+	.dev_select		= ata_std_dev_select,
+
+	.bmdma_setup            = ata_bmdma_setup,
+	.bmdma_start            = ata_bmdma_start,
+	.bmdma_stop		= ata_bmdma_stop,
+	.bmdma_status		= ata_bmdma_status,
+
+	.qc_prep		= ata_qc_prep,
+	.qc_issue		= ata_qc_issue_prot,
+	.data_xfer		= ata_pio_data_xfer,
+
+	.freeze			= ata_bmdma_freeze,
+	.thaw			= ata_bmdma_thaw,
+	.error_handler		= vt6420_error_handler,
+	.post_internal_cmd	= ata_bmdma_post_internal_cmd,
+
+	.irq_handler		= ata_interrupt,
+	.irq_clear		= ata_bmdma_irq_clear,
+
+	.port_start		= ata_port_start,
+	.port_stop		= ata_port_stop,
+	.host_stop		= ata_host_stop,
+};
+
+static const struct ata_port_operations vt6421_sata_ops = {
 	.port_disable		= ata_port_disable,
 
 	.tf_load		= ata_tf_load,
@@ -141,13 +173,13 @@ static const struct ata_port_operations 
 	.host_stop		= ata_host_stop,
 };
 
-static struct ata_port_info svia_port_info = {
+static struct ata_port_info vt6420_port_info = {
 	.sht		= &svia_sht,
 	.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY,
 	.pio_mask	= 0x1f,
 	.mwdma_mask	= 0x07,
 	.udma_mask	= 0x7f,
-	.port_ops	= &svia_sata_ops,
+	.port_ops	= &vt6420_sata_ops,
 };
 
 MODULE_AUTHOR("Jeff Garzik");
@@ -170,6 +202,81 @@ static void svia_scr_write (struct ata_p
 	outl(val, ap->ioaddr.scr_addr + (4 * sc_reg));
 }
 
+/**
+ *	vt6420_prereset - prereset for vt6420
+ *	@ap: target ATA port
+ *
+ *	SCR registers on vt6420 are pieces of shit and may hang the
+ *	whole machine completely if accessed with the wrong timing.
+ *	To avoid such catastrophe, vt6420 doesn't provide generic SCR
+ *	access operations, but uses SStatus and SControl only during
+ *	boot probing in controlled way.
+ *
+ *	As the old (pre EH update) probing code is proven to work, we
+ *	strictly follow the access pattern.
+ *
+ *	LOCKING:
+ *	Kernel thread context (may sleep)
+ *
+ *	RETURNS:
+ *	0 on success, -errno otherwise.
+ */
+static int vt6420_prereset(struct ata_port *ap)
+{
+	struct ata_eh_context *ehc = &ap->eh_context;
+	unsigned long timeout = jiffies + (HZ * 5);
+	u32 sstatus, scontrol;
+	int online;
+
+	/* don't do any SCR stuff if we're not loading */
+	if (!ATA_PFLAG_LOADING)
+		goto skip_scr;
+
+	/* Resume phy.  This is the old resume sequence from
+	 * __sata_phy_reset().
+	 */
+	svia_scr_write(ap, SCR_CONTROL, 0x300);
+	svia_scr_read(ap, SCR_CONTROL); /* flush */
+
+	/* wait for phy to become ready, if necessary */
+	do {
+		msleep(200);
+		if ((svia_scr_read(ap, SCR_STATUS) & 0xf) != 1)
+			break;
+	} while (time_before(jiffies, timeout));
+
+	/* open code sata_print_link_status() */
+	sstatus = svia_scr_read(ap, SCR_STATUS);
+	scontrol = svia_scr_read(ap, SCR_CONTROL);
+
+	online = (sstatus & 0xf) == 0x3;
+
+	ata_port_printk(ap, KERN_INFO,
+			"SATA link %s 1.5 Gbps (SStatus %X SControl %X)\n",
+			online ? "up" : "down", sstatus, scontrol);
+
+	/* SStatus is read one more time */
+	svia_scr_read(ap, SCR_STATUS);
+
+	if (!online) {
+		/* tell EH to bail */
+		ehc->i.action &= ~ATA_EH_RESET_MASK;
+		return 0;
+	}
+
+ skip_scr:
+	/* wait for !BSY */
+	ata_busy_sleep(ap, ATA_TMOUT_BOOT_QUICK, ATA_TMOUT_BOOT);
+
+	return 0;
+}
+
+static void vt6420_error_handler(struct ata_port *ap)
+{
+	return ata_bmdma_drive_eh(ap, vt6420_prereset, ata_std_softreset,
+				  NULL, ata_std_postreset);
+}
+
 static const unsigned int svia_bar_sizes[] = {
 	8, 4, 8, 4, 16, 256
 };
@@ -210,7 +317,7 @@ static void vt6421_init_addrs(struct ata
 static struct ata_probe_ent *vt6420_init_probe_ent(struct pci_dev *pdev)
 {
 	struct ata_probe_ent *probe_ent;
-	struct ata_port_info *ppi = &svia_port_info;
+	struct ata_port_info *ppi = &vt6420_port_info;
 
 	probe_ent = ata_pci_init_native_mode(pdev, &ppi, ATA_PORT_PRIMARY | ATA_PORT_SECONDARY);
 	if (!probe_ent)
@@ -239,7 +346,7 @@ static struct ata_probe_ent *vt6421_init
 
 	probe_ent->sht		= &svia_sht;
 	probe_ent->host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY;
-	probe_ent->port_ops	= &svia_sata_ops;
+	probe_ent->port_ops	= &vt6421_sata_ops;
 	probe_ent->n_ports	= N_PORTS;
 	probe_ent->irq		= pdev->irq;
 	probe_ent->irq_flags	= IRQF_SHARED;

Re: [PATCH] sata_via: use old SCR access pattern on vt6420

[Andi Kleen]

On Wed, 23 Aug 2006 01:00:27 +0900
Tejun Heo <htejun@gmail.com> wrote:

 
> Andi, can you please test this patch?  This adds two more SCR accesses
> for link status printing (as in the pre EH code) and prevents all SCR
> accesses once probing is complete.  This should work but I wanna make
> sure.

I tested booted it both 32bit and 64bit and didn't see any hangs.

-Andi

Re: [PATCH] sata_via: use old SCR access pattern on vt6420

[Jeff Garzik]

applied