From 2f6a9c685a0b6490796c08ef8120cff1ea743dc5 Mon Sep 17 00:00:00 2001 From: Alexander von Gluck IV Date: Wed, 14 Oct 2015 00:06:27 -0500 Subject: [PATCH] ahci: Fix delays with large numbers of empty ports * Properly flag missing devices * Do away with shifts and define ssts and sctl masks * Tested working on 6 different systems with a combination of drive configurations. * Empty media on AHCI devices still cause port change storms. (the issue that was attempted fixed in 5584c22fdd) --- .../kernel/busses/scsi/ahci/ahci_defs.h | 33 ++++---- .../kernel/busses/scsi/ahci/ahci_port.cpp | 82 ++++++++++--------- 2 files changed, 61 insertions(+), 54 deletions(-) diff --git a/src/add-ons/kernel/busses/scsi/ahci/ahci_defs.h b/src/add-ons/kernel/busses/scsi/ahci/ahci_defs.h index 1e942cbe6b..578defa00b 100644 --- a/src/add-ons/kernel/busses/scsi/ahci/ahci_defs.h +++ b/src/add-ons/kernel/busses/scsi/ahci/ahci_defs.h @@ -80,23 +80,26 @@ enum { }; -// Device Detection Initialization -#define SATA_CONTROL_DET_SHIFT 0 -#define SATA_CONTROL_DET_MASK 0x0000000f +// Serial ATA Status and control +#define HBA_PORT_IPM_MASK 0x00000f00 // Interface Power Management +#define SSTS_PORT_IPM_ACTIVE 0x00000100 // active state +#define SSTS_PORT_IPM_PARTIAL 0x00000200 // partial state +#define SSTS_PORT_IPM_SLUMBER 0x00000600 // slumber power management state +#define SSTS_PORT_IPM_DEVSLEEP 0x00000800 // devsleep power management state +#define SCTL_PORT_IPM_NORES 0x00000000 // no power restrictions +#define SCTL_PORT_IPM_NOPART 0x00000100 // no transitions to partial +#define SCTL_PORT_IPM_NOSLUM 0x00000200 // no transitions to slumber -#define DET_NO_INITIALIZATION 0x0 -#define DET_INITIALIZATION 0x1 +#define HBA_PORT_SPD_MASK 0x000000f0 // Interface speed -// Speed Allowed -#define SATA_CONTROL_SPD_SHIFT 4 -#define SATA_CONTROL_SPD_MASK 0x000000f0 - -// Interface Power Management Transitions Allowed -#define SATA_CONTROL_IPM_SHIFT 8 -#define SATA_CONTROL_IPM_MASK 0x00000f00 - -#define IPM_TRANSITIONS_TO_PARTIAL_DISABLED 0x1 -#define IPM_TRANSITIONS_TO_SLUMBER_DISABLED 0x2 +#define HBA_PORT_DET_MASK 0x0000000f // Device Detection +#define SSTS_PORT_DET_NODEV 0x00000000 // no device detected +#define SSTS_PORT_DET_NOPHY 0x00000001 // device present but PHY not est. +#define SSTS_PORT_DET_PRESENT 0x00000003 // device present and PHY est. +#define SSTS_PORT_DET_OFFLINE 0x00000004 // device offline due to disabled +#define SCTL_PORT_DET_NOINIT 0x00000000 // no initalization request +#define SCTL_PORT_DET_INIT 0x00000001 // perform interface initalization +#define SCTL_PORT_DET_DISABLE 0x00000004 // disable phy // Device signatures #define SATA_SIG_ATA 0x00000101 // SATA drive diff --git a/src/add-ons/kernel/busses/scsi/ahci/ahci_port.cpp b/src/add-ons/kernel/busses/scsi/ahci/ahci_port.cpp index b1122fd82e..f33b783e31 100644 --- a/src/add-ons/kernel/busses/scsi/ahci/ahci_port.cpp +++ b/src/add-ons/kernel/busses/scsi/ahci/ahci_port.cpp @@ -117,9 +117,7 @@ AHCIPort::Init1() // prdt follows after command table // disable transitions to partial or slumber state - fRegs->sctl = (fRegs->sctl & ~SATA_CONTROL_IPM_MASK) - | (IPM_TRANSITIONS_TO_PARTIAL_DISABLED - | IPM_TRANSITIONS_TO_SLUMBER_DISABLED) << SATA_CONTROL_IPM_SHIFT; + fRegs->sctl |= (SCTL_PORT_IPM_NOPART | SCTL_PORT_IPM_NOSLUM); // clear IRQ status bits fRegs->is = fRegs->is; @@ -166,18 +164,13 @@ AHCIPort::Init2() TRACE("is 0x%08" B_PRIx32 "\n", fRegs->is); TRACE("cmd 0x%08" B_PRIx32 "\n", fRegs->cmd); TRACE("ssts 0x%08" B_PRIx32 "\n", fRegs->ssts); - TRACE("sctl.ipm 0x%02" B_PRIx32 "\n", - (fRegs->sctl & SATA_CONTROL_IPM_MASK) >> SATA_CONTROL_IPM_SHIFT); - TRACE("sctl.spd 0x%02" B_PRIx32 "\n", - (fRegs->sctl & SATA_CONTROL_SPD_MASK) >> SATA_CONTROL_SPD_SHIFT); - TRACE("sctl.det 0x%02" B_PRIx32 "\n", - (fRegs->sctl & SATA_CONTROL_DET_MASK) >> SATA_CONTROL_DET_SHIFT); + TRACE("sctl.ipm 0x%02" B_PRIx32 "\n", fRegs->sctl & HBA_PORT_IPM_MASK); + TRACE("sctl.spd 0x%02" B_PRIx32 "\n", fRegs->sctl & HBA_PORT_SPD_MASK); + TRACE("sctl.det 0x%02" B_PRIx32 "\n", fRegs->sctl & HBA_PORT_DET_MASK); TRACE("serr 0x%08" B_PRIx32 "\n", fRegs->serr); TRACE("sact 0x%08" B_PRIx32 "\n", fRegs->sact); TRACE("tfd 0x%08" B_PRIx32 "\n", fRegs->tfd); - fDevicePresent = (fRegs->ssts & 0xf) == 0x3; - TRACE("%s: port %d, device %s\n", __func__, fIndex, fDevicePresent ? "present" : "absent"); @@ -229,12 +222,12 @@ AHCIPort::ResetDevice() return; } - if (wait_until_set(&fRegs->ssts, 0x1, 100000) < B_OK) + if (wait_until_set(&fRegs->ssts, SSTS_PORT_DET_NODEV, 100000) < B_OK) TRACE("AHCIPort::ResetDevice port %d no device detected\n", fIndex); _ClearErrorRegister(); - if (fRegs->ssts & 1) { + if (fRegs->ssts & SSTS_PORT_DET_NOPHY) { if (wait_until_set(&fRegs->ssts, 0x3, 500000) < B_OK) { TRACE("AHCIPort::ResetDevice port %d device present but no phy " "communication\n", fIndex); @@ -328,26 +321,20 @@ AHCIPort::PortReset() return B_ERROR; } - fRegs->sctl = (fRegs->sctl & ~SATA_CONTROL_DET_MASK) - | DET_INITIALIZATION << SATA_CONTROL_DET_SHIFT; + fRegs->sctl |= SCTL_PORT_DET_INIT; FlushPostedWrites(); spin(1100); // You must wait 1ms at minimum - fRegs->sctl = (fRegs->sctl & ~SATA_CONTROL_DET_MASK) - | DET_NO_INITIALIZATION << SATA_CONTROL_DET_SHIFT; + fRegs->sctl = (fRegs->sctl & ~HBA_PORT_DET_MASK) | SCTL_PORT_DET_NOINIT; + FlushPostedWrites(); - if (wait_until_set(&fRegs->ssts, 0x3, 500000) < B_OK) { - TRACE("AHCIPort::PortReset port %d device present but no phy " - "communication\n", fIndex); - return B_ERROR; + if (wait_until_set(&fRegs->ssts, SSTS_PORT_DET_PRESENT, 500000) < B_OK) { + TRACE("%s: port %d: no device detected\n", __func__, fIndex); + fDevicePresent = false; + return B_OK; } - //if ((fRegs->tfd & 0xff) == 0x7f) { - // TRACE("AHCIPort::PostReset port %d: no device\n", fIndex); - // return B_OK; - //} - Enable(); return PostReset(); @@ -366,9 +353,26 @@ AHCIPort::PostReset() return B_ERROR; } + switch (fRegs->ssts & HBA_PORT_SPD_MASK) { + case 0x10: + TRACE("%s: port %d link speed 1.5Gb/s\n", __func__, fIndex); + break; + case 0x20: + TRACE("%s: port %d link speed 3.0Gb/s\n", __func__, fIndex); + break; + case 0x30: + TRACE("%s: port %d link speed 6.0Gb/s\n", __func__, fIndex); + break; + default: + TRACE("%s: port %d link speed unrestricted\n", __func__, fIndex); + break; + } + wait_until_clear(&fRegs->tfd, ATA_STATUS_BUSY, 31000000); + fDevicePresent = (fRegs->ssts & HBA_PORT_DET_MASK) == SSTS_PORT_DET_PRESENT; fIsATAPI = fRegs->sig == SATA_SIG_ATAPI; + if (fIsATAPI) fRegs->cmd |= PORT_CMD_ATAPI; else @@ -440,12 +444,9 @@ AHCIPort::InterruptErrorHandler(uint32 is) B_PRIx32 ", is 0x%08" B_PRIx32 ", ci 0x%08" B_PRIx32 "\n", fIndex, fCommandsActive, is, ci); TRACE("ssts 0x%08" B_PRIx32 "\n", fRegs->ssts); - TRACE("sctl.ipm 0x%02" B_PRIx32 "\n", - (fRegs->sctl & SATA_CONTROL_IPM_MASK) >> SATA_CONTROL_IPM_SHIFT); - TRACE("sctl.spd 0x%02" B_PRIx32 "\n", - (fRegs->sctl & SATA_CONTROL_SPD_MASK) >> SATA_CONTROL_SPD_SHIFT); - TRACE("sctl.det 0x%02" B_PRIx32 "\n", - (fRegs->sctl & SATA_CONTROL_DET_MASK) >> SATA_CONTROL_DET_SHIFT); + TRACE("sctl.ipm 0x%02" B_PRIx32 "\n", fRegs->sctl & HBA_PORT_IPM_MASK); + TRACE("sctl.spd 0x%02" B_PRIx32 "\n", fRegs->sctl & HBA_PORT_SPD_MASK); + TRACE("sctl.det 0x%02" B_PRIx32 "\n", fRegs->sctl & HBA_PORT_DET_MASK); TRACE("serr 0x%08" B_PRIx32 "\n", fRegs->serr); TRACE("sact 0x%08" B_PRIx32 "\n", fRegs->sact); } @@ -488,18 +489,21 @@ AHCIPort::InterruptErrorHandler(uint32 is) } if (is & PORT_INT_PRC) { TRACE("PhyReady Change\n"); -// fSoftReset = true; + //fSoftReset = true; } if (is & PORT_INT_PC) { TRACE("Port Connect Change\n"); - // TODO: check if the COMINIT is actually unsolicited! + // Unsolicited when we had a port connect change without us requesting // Spec v1.3, §6.2.2.3 Recovery of Unsolicited COMINIT - // Spec v1.3.1, §7.4 Interaction of command list and port change status - // TODO: Issue COMRESET ? - //PortReset(); - //HBAReset(); ??? - // clear error bits to clear PxSERR.DIAG.X + // TODO: This isn't enough + //if ((fRegs->sctl & SCTL_PORT_DET_INIT) == 0) { + // TRACE("%s: unsolicited port connect change\n", __func__); + // fSoftReset = true; + // fError = true; + //} + + // XXX: This shouldn't be needed here... but we can loop without it _ClearErrorRegister(); } if (is & PORT_INT_UF) {