Skip to contentSkip to conversation

via boot up hang during probing for no sata disks

PATCH Applied Andi Kleen 3 participants 6 messages Aug 14, 2006 – Aug 24, 2006

Subsystems (2)
lore
Share & tools
lore

Andi Kleen

author
#0 lore raw

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

#1 lore raw
Quoted 8 lines · 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.

Quoted 7 lines

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?

drivers/scsi/sata_via.c +84 -6
index 03baec2..efa865d 100644
@@ -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;
author
#2 lore raw
Quoted 17 lines · On Wed, 16 Aug 2006 14:12:28 +0900 Tejun Heo <[email protected]> wrote:
Quoted 8 lines · 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.

Quoted 7 lines

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.

Quoted 3 lines

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

#3 lore raw

PATCH 1/1 sata_via: use old SCR access pattern on vt6420

In reply to 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.

---

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(-)
drivers/scsi/sata_via.c +112 -5
Index: 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;
author
#4 lore raw

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

Quoted 4 lines · On Wed, 23 Aug 2006 01:00:27 +0900 Tejun Heo <[email protected]> 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