Hi,
Finally the update for the ahci code, featuring:
* A bunch of bugfixes, which makes the code work on real hardware (additionally to the qemu emulated ahci adapter). * Better timeout handling. * Support for unaligned access. * Support for boot ordering.
cheers, Gerd
The following changes since commit 77b8536e5c9908fbe99c88d01462a36a3deb05b1:
pci: set BUILD_PCIMEM_START to 0xe0000000 (2011-07-12 21:14:53 -0400)
are available in the git repository at: git://git.kraxel.org/seabios ahci
Gerd Hoffmann (6): ahci/sata: Fix FIS setup. ahci: use interrupt status register ahci: add error recovery code ahci: don't expect ATA_CB_STAT_DRQ being clear ahci: rework init workflow & fix timeout handling ahci: bootprio support
Scott Duplichan (1): ahci: handle unaligned buffers.
src/ahci.c | 350 +++++++++++++++++++++++++++++++++++++++++++++--------------- src/ahci.h | 3 +- 2 files changed, 264 insertions(+), 89 deletions(-)
FIS setup does't follow the SATA specs, fix it.
Credits go to Jonathan Kollasch and Scott Duplichan for finding those.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- src/ahci.c | 7 +++---- 1 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/ahci.c b/src/ahci.c index fb4b70c..058416c 100644 --- a/src/ahci.c +++ b/src/ahci.c @@ -125,12 +125,11 @@ static int ahci_command(struct ahci_port_s *port, int iswrite, int isatapi, return -1;
flags = ((1 << 16) | /* one prd entry */ - (1 << 10) | /* clear busy on ok */ (iswrite ? (1 << 6) : 0) | (isatapi ? (1 << 5) : 0) | - (4 << 0)); /* fis length (dwords) */ - SET_FLATPTR(list[0].flags, flags); - SET_FLATPTR(list[0].bytes, bsize); + (5 << 0)); /* fis length (dwords) */ + SET_FLATPTR(list[0].flags, flags); + SET_FLATPTR(list[0].bytes, 0); SET_FLATPTR(list[0].base, ((u32)(cmd))); SET_FLATPTR(list[0].baseu, 0);
Poll interrupt status register to figure when the device has updated the status and possibly finished the request, continue polling until BSY is clear as we might see multiple status updates per request.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- src/ahci.c | 43 ++++++++++++++++++++++++++++--------------- 1 files changed, 28 insertions(+), 15 deletions(-)
diff --git a/src/ahci.c b/src/ahci.c index 058416c..9ff1324 100644 --- a/src/ahci.c +++ b/src/ahci.c @@ -105,7 +105,7 @@ static void ahci_port_writel(struct ahci_ctrl_s *ctrl, u32 pnr, u32 reg, u32 val static int ahci_command(struct ahci_port_s *port, int iswrite, int isatapi, void *buffer, u32 bsize) { - u32 val, status, success, flags; + u32 val, status, success, flags, intbits; struct ahci_ctrl_s *ctrl = GET_GLOBAL(port->ctrl); struct ahci_cmd_s *cmd = GET_GLOBAL(port->cmd); struct ahci_fis_s *fis = GET_GLOBAL(port->fis); @@ -118,12 +118,6 @@ static int ahci_command(struct ahci_port_s *port, int iswrite, int isatapi, SET_FLATPTR(cmd->prdt[0].baseu, 0); SET_FLATPTR(cmd->prdt[0].flags, bsize-1);
- val = ahci_port_readl(ctrl, pnr, PORT_CMD); - ahci_port_writel(ctrl, pnr, PORT_CMD, val | PORT_CMD_START); - - if (ahci_port_readl(ctrl, pnr, PORT_CMD_ISSUE)) - return -1; - flags = ((1 << 16) | /* one prd entry */ (iswrite ? (1 << 6) : 0) | (isatapi ? (1 << 5) : 0) | @@ -134,15 +128,31 @@ static int ahci_command(struct ahci_port_s *port, int iswrite, int isatapi, SET_FLATPTR(list[0].baseu, 0);
dprintf(2, "AHCI/%d: send cmd ...\n", pnr); - SET_FLATPTR(fis->rfis[2], 0); + intbits = ahci_port_readl(ctrl, pnr, PORT_IRQ_STAT); + if (intbits) + ahci_port_writel(ctrl, pnr, PORT_IRQ_STAT, intbits); ahci_port_writel(ctrl, pnr, PORT_SCR_ACT, 1); ahci_port_writel(ctrl, pnr, PORT_CMD_ISSUE, 1); - while (ahci_port_readl(ctrl, pnr, PORT_CMD_ISSUE)) { - yield(); - } - while ((status = GET_FLATPTR(fis->rfis[2])) == 0) { - yield(); - } + + do { + for (;;) { + intbits = ahci_port_readl(ctrl, pnr, PORT_IRQ_STAT); + if (intbits) { + ahci_port_writel(ctrl, pnr, PORT_IRQ_STAT, intbits); + if (intbits & 0x02) { + status = GET_FLATPTR(fis->psfis[2]); + break; + } + if (intbits & 0x01) { + status = GET_FLATPTR(fis->rfis[2]); + break; + } + } + yield(); + } + dprintf(2, "AHCI/%d: ... intbits 0x%x, status 0x%x ...\n", + pnr, intbits, status); + } while (status & ATA_CB_STAT_BSY);
success = (0x00 == (status & (ATA_CB_STAT_BSY | ATA_CB_STAT_DF | ATA_CB_STAT_DRQ | ATA_CB_STAT_ERR)) && @@ -329,7 +339,10 @@ ahci_port_init(struct ahci_ctrl_s *ctrl, u32 pnr) ahci_port_writel(ctrl, pnr, PORT_LST_ADDR, (u32)port->list); ahci_port_writel(ctrl, pnr, PORT_FIS_ADDR, (u32)port->fis); val = ahci_port_readl(ctrl, pnr, PORT_CMD); - ahci_port_writel(ctrl, pnr, PORT_CMD, val | PORT_CMD_FIS_RX); + val |= PORT_CMD_FIS_RX; + ahci_port_writel(ctrl, pnr, PORT_CMD, val); + val |= PORT_CMD_START; + ahci_port_writel(ctrl, pnr, PORT_CMD, val);
sata_prep_simple(&port->cmd->fis, ATA_CMD_IDENTIFY_PACKET_DEVICE); rc = ahci_command(port, 0, 0, buffer, sizeof(buffer));
By Scott Duplichan.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- src/ahci.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++--- 1 files changed, 47 insertions(+), 3 deletions(-)
diff --git a/src/ahci.c b/src/ahci.c index 9ff1324..ec698ef 100644 --- a/src/ahci.c +++ b/src/ahci.c @@ -105,7 +105,7 @@ static void ahci_port_writel(struct ahci_ctrl_s *ctrl, u32 pnr, u32 reg, u32 val static int ahci_command(struct ahci_port_s *port, int iswrite, int isatapi, void *buffer, u32 bsize) { - u32 val, status, success, flags, intbits; + u32 val, status, success, flags, intbits, error; struct ahci_ctrl_s *ctrl = GET_GLOBAL(port->ctrl); struct ahci_cmd_s *cmd = GET_GLOBAL(port->cmd); struct ahci_fis_s *fis = GET_GLOBAL(port->fis); @@ -141,10 +141,12 @@ static int ahci_command(struct ahci_port_s *port, int iswrite, int isatapi, ahci_port_writel(ctrl, pnr, PORT_IRQ_STAT, intbits); if (intbits & 0x02) { status = GET_FLATPTR(fis->psfis[2]); + error = GET_FLATPTR(fis->psfis[3]); break; } if (intbits & 0x01) { status = GET_FLATPTR(fis->rfis[2]); + error = GET_FLATPTR(fis->rfis[3]); break; } } @@ -157,8 +159,50 @@ static int ahci_command(struct ahci_port_s *port, int iswrite, int isatapi, success = (0x00 == (status & (ATA_CB_STAT_BSY | ATA_CB_STAT_DF | ATA_CB_STAT_DRQ | ATA_CB_STAT_ERR)) && ATA_CB_STAT_RDY == (status & (ATA_CB_STAT_RDY))); - dprintf(2, "AHCI/%d: ... finished, status 0x%x, %s\n", pnr, - status, success ? "OK" : "ERROR"); + if (success) { + dprintf(2, "AHCI/%d: ... finished, status 0x%x, OK\n", pnr, + status); + } else { + dprintf(2, "AHCI/%d: ... finished, status 0x%x, ERROR 0x%x\n", pnr, + status, error); + + // non-queued error recovery (AHCI 1.3 section 6.2.2.1) + // Clears PxCMD.ST to 0 to reset the PxCI register + val = ahci_port_readl(ctrl, pnr, PORT_CMD); + ahci_port_writel(ctrl, pnr, PORT_CMD, val & ~PORT_CMD_START); + + // waits for PxCMD.CR to clear to 0 + while (1) { + val = ahci_port_readl(ctrl, pnr, PORT_CMD); + if ((val & PORT_CMD_LIST_ON) == 0) + break; + yield(); + } + + // Clears any error bits in PxSERR to enable capturing new errors + val = ahci_port_readl(ctrl, pnr, PORT_SCR_ERR); + ahci_port_writel(ctrl, pnr, PORT_SCR_ERR, val); + + // Clears status bits in PxIS as appropriate + val = ahci_port_readl(ctrl, pnr, PORT_IRQ_STAT); + ahci_port_writel(ctrl, pnr, PORT_IRQ_STAT, val); + + // If PxTFD.STS.BSY or PxTFD.STS.DRQ is set to 1, issue + // a COMRESET to the device to put it in an idle state + val = ahci_port_readl(ctrl, pnr, PORT_TFDATA); + if (val & (ATA_CB_STAT_BSY | ATA_CB_STAT_DRQ)) { + dprintf(2, "AHCI/%d: issue comreset\n", pnr); + val = ahci_port_readl(ctrl, pnr, PORT_SCR_CTL); + // set Device Detection Initialization (DET) to 1 for 1 ms for comreset + ahci_port_writel(ctrl, pnr, PORT_SCR_CTL, val | 1); + mdelay (1); + ahci_port_writel(ctrl, pnr, PORT_SCR_CTL, val); + } + + // Sets PxCMD.ST to 1 to enable issuing new commands + val = ahci_port_readl(ctrl, pnr, PORT_CMD); + ahci_port_writel(ctrl, pnr, PORT_CMD, val | PORT_CMD_START); + } return success ? 0 : -1; }
From: Scott Duplichan scott@notabs.org
This change allows unaligned buffers to be used for reads or writes to non-atapi devices. Currently only MS-DOS boot is known to need unaligned buffer support.
Signed-off-by: Scott Duplichan scott@notabs.org Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- src/ahci.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 51 insertions(+), 2 deletions(-)
diff --git a/src/ahci.c b/src/ahci.c index ec698ef..e2ec07a 100644 --- a/src/ahci.c +++ b/src/ahci.c @@ -22,6 +22,7 @@ /**************************************************************** * these bits must run in both 16bit and 32bit modes ****************************************************************/ +u8 *ahci_buf_fl VAR16VISIBLE;
// prepare sata command fis static void sata_prep_simple(struct sata_cmd_fis *fis, u8 command) @@ -230,9 +231,9 @@ int ahci_cmd_data(struct disk_op_s *op, void *cdbcmd, u16 blocksize) return DISK_RET_SUCCESS; }
-// read/write count blocks from a harddrive. +// read/write count blocks from a harddrive, op->buf_fl must be word aligned static int -ahci_disk_readwrite(struct disk_op_s *op, int iswrite) +ahci_disk_readwrite_aligned(struct disk_op_s *op, int iswrite) { struct ahci_port_s *port = container_of( op->drive_g, struct ahci_port_s, drive); @@ -249,6 +250,47 @@ ahci_disk_readwrite(struct disk_op_s *op, int iswrite) return DISK_RET_SUCCESS; }
+// read/write count blocks from a harddrive. +static int +ahci_disk_readwrite(struct disk_op_s *op, int iswrite) +{ + // if caller's buffer is word aligned, use it directly + if (((u32) op->buf_fl & 1) == 0) + return ahci_disk_readwrite_aligned(op, iswrite); + + // Use a word aligned buffer for AHCI I/O + int rc; + struct disk_op_s localop = *op; + u8 *alignedbuf_fl = GET_GLOBAL(ahci_buf_fl); + u8 *position = op->buf_fl; + + localop.buf_fl = alignedbuf_fl; + localop.count = 1; + + if (iswrite) { + u16 block; + for (block = 0; block < op->count; block++) { + memcpy_fl (alignedbuf_fl, position, DISK_SECTOR_SIZE); + rc = ahci_disk_readwrite_aligned (&localop, 1); + if (rc) + return rc; + position += DISK_SECTOR_SIZE; + localop.lba++; + } + } else { // read + u16 block; + for (block = 0; block < op->count; block++) { + rc = ahci_disk_readwrite_aligned (&localop, 0); + if (rc) + return rc; + memcpy_fl (position, alignedbuf_fl, DISK_SECTOR_SIZE); + position += DISK_SECTOR_SIZE; + localop.lba++; + } + } + return DISK_RET_SUCCESS; +} + // command demuxer int process_ahci_op(struct disk_op_s *op) { @@ -493,6 +535,13 @@ ahci_init_controller(int bdf) warn_noalloc(); return; } + + ahci_buf_fl = malloc_low(DISK_SECTOR_SIZE); + if (!ahci_buf_fl) { + warn_noalloc(); + return; + } + ctrl->pci_bdf = bdf; ctrl->iobase = pci_config_readl(bdf, PCI_BASE_ADDRESS_5); ctrl->irq = pci_config_readb(bdf, PCI_INTERRUPT_LINE);
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- src/ahci.c | 5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/ahci.c b/src/ahci.c index e2ec07a..7279199 100644 --- a/src/ahci.c +++ b/src/ahci.c @@ -158,7 +158,7 @@ static int ahci_command(struct ahci_port_s *port, int iswrite, int isatapi, } while (status & ATA_CB_STAT_BSY);
success = (0x00 == (status & (ATA_CB_STAT_BSY | ATA_CB_STAT_DF | - ATA_CB_STAT_DRQ | ATA_CB_STAT_ERR)) && + ATA_CB_STAT_ERR)) && ATA_CB_STAT_RDY == (status & (ATA_CB_STAT_RDY))); if (success) { dprintf(2, "AHCI/%d: ... finished, status 0x%x, OK\n", pnr, @@ -379,8 +379,7 @@ ahci_port_probe(struct ahci_ctrl_s *ctrl, u32 pnr) u32 val, count = 0;
val = ahci_port_readl(ctrl, pnr, PORT_TFDATA); - while (val & ((1 << 7) /* BSY */ | - (1 << 3) /* DRQ */)) { + while (val & ATA_CB_STAT_BSY) { ndelay(500); val = ahci_port_readl(ctrl, pnr, PORT_TFDATA); count++;
Rework init workflow to match suggestions in the ahci specs better, especially remove the shortcut which tries to detect drives without enabling FIS receiving. This makes memory allocation a bit complicated as we are using malloc_tmp() allocated memory now to probe the devices so we can free it when no drive is present. In case we detect a drive we have to free and realloc the memory with malloc_low() so it is available after POST when the boot loader wants read stuff via int13.
Also use TSC to calculate timeout instead of delays and loop counts.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- src/ahci.c | 189 ++++++++++++++++++++++++++++++++++++++++-------------------- 1 files changed, 127 insertions(+), 62 deletions(-)
diff --git a/src/ahci.c b/src/ahci.c index 7279199..1f119bb 100644 --- a/src/ahci.c +++ b/src/ahci.c @@ -17,7 +17,9 @@ #include "ahci.h" // CDB_CMD_READ_10 #include "blockcmd.h" // CDB_CMD_READ_10
-#define AHCI_MAX_RETRIES 5 +#define AHCI_REQUEST_TIMEOUT 32000 // 32 seconds max for IDE ops +#define AHCI_RESET_TIMEOUT 500 // 500 miliseconds +#define AHCI_LINK_TIMEOUT 10 // 10 miliseconds
/**************************************************************** * these bits must run in both 16bit and 32bit modes @@ -112,6 +114,7 @@ static int ahci_command(struct ahci_port_s *port, int iswrite, int isatapi, struct ahci_fis_s *fis = GET_GLOBAL(port->fis); struct ahci_list_s *list = GET_GLOBAL(port->list); u32 pnr = GET_GLOBAL(port->pnr); + u64 end;
SET_FLATPTR(cmd->fis.reg, 0x27); SET_FLATPTR(cmd->fis.pmp_type, (1 << 7)); /* cmd fis */ @@ -135,6 +138,7 @@ static int ahci_command(struct ahci_port_s *port, int iswrite, int isatapi, ahci_port_writel(ctrl, pnr, PORT_SCR_ACT, 1); ahci_port_writel(ctrl, pnr, PORT_CMD_ISSUE, 1);
+ end = calc_future_tsc(AHCI_REQUEST_TIMEOUT); do { for (;;) { intbits = ahci_port_readl(ctrl, pnr, PORT_IRQ_STAT); @@ -151,6 +155,10 @@ static int ahci_command(struct ahci_port_s *port, int iswrite, int isatapi, break; } } + if (check_tsc(end)) { + warn_timeout(); + return -1; + } yield(); } dprintf(2, "AHCI/%d: ... intbits 0x%x, status 0x%x ...\n", @@ -347,62 +355,36 @@ int process_ahci_op(struct disk_op_s *op) static void ahci_port_reset(struct ahci_ctrl_s *ctrl, u32 pnr) { - u32 val, count = 0; + u32 val; + u64 end;
/* disable FIS + CMD */ - val = ahci_port_readl(ctrl, pnr, PORT_CMD); - while (val & (PORT_CMD_FIS_RX | PORT_CMD_START | - PORT_CMD_FIS_ON | PORT_CMD_LIST_ON) && - count < AHCI_MAX_RETRIES) { + end = calc_future_tsc(AHCI_RESET_TIMEOUT); + for (;;) { + val = ahci_port_readl(ctrl, pnr, PORT_CMD); + if (!(val & (PORT_CMD_FIS_RX | PORT_CMD_START | + PORT_CMD_FIS_ON | PORT_CMD_LIST_ON))) + break; val &= ~(PORT_CMD_FIS_RX | PORT_CMD_START); ahci_port_writel(ctrl, pnr, PORT_CMD, val); - ndelay(500); - val = ahci_port_readl(ctrl, pnr, PORT_CMD); - count++; + if (check_tsc(end)) { + warn_timeout(); + break; + } + yield(); }
- /* clear status */ - val = ahci_port_readl(ctrl, pnr, PORT_SCR_ERR); - if (val) - ahci_port_writel(ctrl, pnr, PORT_SCR_ERR, val); - /* disable + clear IRQs */ - ahci_port_writel(ctrl, pnr, PORT_IRQ_MASK, val); + ahci_port_writel(ctrl, pnr, PORT_IRQ_MASK, 0); val = ahci_port_readl(ctrl, pnr, PORT_IRQ_STAT); if (val) ahci_port_writel(ctrl, pnr, PORT_IRQ_STAT, val); }
-static int -ahci_port_probe(struct ahci_ctrl_s *ctrl, u32 pnr) -{ - u32 val, count = 0; - - val = ahci_port_readl(ctrl, pnr, PORT_TFDATA); - while (val & ATA_CB_STAT_BSY) { - ndelay(500); - val = ahci_port_readl(ctrl, pnr, PORT_TFDATA); - count++; - if (count >= AHCI_MAX_RETRIES) - return -1; - } - - val = ahci_port_readl(ctrl, pnr, PORT_SCR_STAT); - if ((val & 0x07) != 0x03) - return -1; - return 0; -} - -#define MAXMODEL 40 - static struct ahci_port_s* -ahci_port_init(struct ahci_ctrl_s *ctrl, u32 pnr) +ahci_port_alloc(struct ahci_ctrl_s *ctrl, u32 pnr) { struct ahci_port_s *port = malloc_fseg(sizeof(*port)); - char model[MAXMODEL+1]; - u16 buffer[256]; - u32 val; - int rc;
if (!port) { warn_noalloc(); @@ -410,9 +392,9 @@ ahci_port_init(struct ahci_ctrl_s *ctrl, u32 pnr) } port->pnr = pnr; port->ctrl = ctrl; - port->list = memalign_low(1024, 1024); - port->fis = memalign_low(256, 256); - port->cmd = memalign_low(256, 256); + port->list = memalign_tmp(1024, 1024); + port->fis = memalign_tmp(256, 256); + port->cmd = memalign_tmp(256, 256); if (port->list == NULL || port->fis == NULL || port->cmd == NULL) { warn_noalloc(); return NULL; @@ -423,11 +405,97 @@ ahci_port_init(struct ahci_ctrl_s *ctrl, u32 pnr)
ahci_port_writel(ctrl, pnr, PORT_LST_ADDR, (u32)port->list); ahci_port_writel(ctrl, pnr, PORT_FIS_ADDR, (u32)port->fis); - val = ahci_port_readl(ctrl, pnr, PORT_CMD); - val |= PORT_CMD_FIS_RX; - ahci_port_writel(ctrl, pnr, PORT_CMD, val); - val |= PORT_CMD_START; - ahci_port_writel(ctrl, pnr, PORT_CMD, val); + return port; +} + +static void ahci_port_realloc(struct ahci_port_s *port) +{ + u32 cmd; + + ahci_port_reset(port->ctrl, port->pnr); + + free(port->list); + free(port->fis); + free(port->cmd); + port->list = memalign_low(1024, 1024); + port->fis = memalign_low(256, 256); + port->cmd = memalign_low(256, 256); + + ahci_port_writel(port->ctrl, port->pnr, PORT_LST_ADDR, (u32)port->list); + ahci_port_writel(port->ctrl, port->pnr, PORT_FIS_ADDR, (u32)port->fis); + + cmd = ahci_port_readl(port->ctrl, port->pnr, PORT_CMD); + cmd |= (PORT_CMD_FIS_RX|PORT_CMD_START); + ahci_port_writel(port->ctrl, port->pnr, PORT_CMD, cmd); +} + +static void ahci_port_release(struct ahci_port_s *port) +{ + ahci_port_reset(port->ctrl, port->pnr); + free(port->list); + free(port->fis); + free(port->cmd); + free(port); +} + +#define MAXMODEL 40 + +/* See ahci spec chapter 10.1 "Software Initialization of HBA" */ +static int ahci_port_init(struct ahci_port_s *port) +{ + struct ahci_ctrl_s *ctrl = port->ctrl; + u32 pnr = port->pnr; + char model[MAXMODEL+1]; + u16 buffer[256]; + u32 cmd, stat, err, tf; + u64 end; + int rc; + + /* enable FIS recv */ + cmd = ahci_port_readl(ctrl, pnr, PORT_CMD); + cmd |= PORT_CMD_FIS_RX; + ahci_port_writel(ctrl, pnr, PORT_CMD, cmd); + + /* spin up */ + cmd |= PORT_CMD_SPIN_UP; + ahci_port_writel(ctrl, pnr, PORT_CMD, cmd); + end = calc_future_tsc(AHCI_LINK_TIMEOUT); + for (;;) { + stat = ahci_port_readl(ctrl, pnr, PORT_SCR_STAT); + if ((stat & 0x07) == 0x03) { + dprintf(1, "AHCI/%d: link up\n", port->pnr); + break; + } + if (check_tsc(end)) { + dprintf(1, "AHCI/%d: link down\n", port->pnr); + return -1; + } + yield(); + } + + /* clear error status */ + err = ahci_port_readl(ctrl, pnr, PORT_SCR_ERR); + if (err) + ahci_port_writel(ctrl, pnr, PORT_SCR_ERR, err); + + /* wait for device becoming ready */ + end = calc_future_tsc(AHCI_REQUEST_TIMEOUT); + for (;;) { + tf = ahci_port_readl(ctrl, pnr, PORT_TFDATA); + if (!(tf & (ATA_CB_STAT_BSY | + ATA_CB_STAT_DRQ))) + break; + if (check_tsc(end)) { + warn_timeout(); + dprintf(1, "AHCI/%d: device not ready (tf 0x%x)\n", port->pnr, tf); + return -1; + } + yield(); + } + + /* start device */ + cmd |= PORT_CMD_START; + ahci_port_writel(ctrl, pnr, PORT_CMD, cmd);
sata_prep_simple(&port->cmd->fis, ATA_CMD_IDENTIFY_PACKET_DEVICE); rc = ahci_command(port, 0, 0, buffer, sizeof(buffer)); @@ -438,7 +506,7 @@ ahci_port_init(struct ahci_ctrl_s *ctrl, u32 pnr) sata_prep_simple(&port->cmd->fis, ATA_CMD_IDENTIFY_DEVICE); rc = ahci_command(port, 0, 0, buffer, sizeof(buffer)); if (rc < 0) - goto err; + return -1; }
port->drive.type = DTYPE_AHCI; @@ -491,13 +559,7 @@ ahci_port_init(struct ahci_ctrl_s *ctrl, u32 pnr) if (iscd) boot_add_cd(&port->drive, desc, -1); } - - return port; - -err: - dprintf(1, "AHCI/%d: init failure, reset\n", port->pnr); - ahci_port_reset(ctrl, pnr); - return NULL; + return 0; }
// Detect any drives attached to a given controller. @@ -515,11 +577,14 @@ ahci_detect(void *data) continue; dprintf(2, "AHCI/%d: probing\n", pnr); ahci_port_reset(ctrl, pnr); - rc = ahci_port_probe(ctrl, pnr); - dprintf(1, "AHCI/%d: link %s\n", pnr, rc == 0 ? "up" : "down"); - if (rc != 0) + port = ahci_port_alloc(ctrl, pnr); + if (port == NULL) continue; - port = ahci_port_init(ctrl, pnr); + rc = ahci_port_init(port); + if (rc < 0) + ahci_port_release(port); + else + ahci_port_realloc(port); } }
Wind up bootprio support in the ahci driver so boot device ordering works for ahci disks too. No extra work needed on qemu side.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- src/ahci.c | 15 ++++++++++----- src/ahci.h | 3 ++- 2 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/src/ahci.c b/src/ahci.c index 1f119bb..99bd0bb 100644 --- a/src/ahci.c +++ b/src/ahci.c @@ -541,7 +541,8 @@ static int ahci_port_init(struct ahci_port_s *port) dprintf(1, "%s\n", desc);
// Register with bcv system. - boot_add_hd(&port->drive, desc, -1); + int prio = bootprio_find_ata_device(ctrl->pci_tmp, pnr, 0); + boot_add_hd(&port->drive, desc, prio); } else { // found cdrom (atapi) port->drive.blksize = CDROM_SECTOR_SIZE; @@ -556,8 +557,10 @@ static int ahci_port_init(struct ahci_port_s *port) dprintf(1, "%s\n", desc);
// fill cdidmap - if (iscd) - boot_add_cd(&port->drive, desc, -1); + if (iscd) { + int prio = bootprio_find_ata_device(ctrl->pci_tmp, pnr, 0); + boot_add_cd(&port->drive, desc, prio); + } } return 0; } @@ -590,9 +593,10 @@ ahci_detect(void *data)
// Initialize an ata controller and detect its drives. static void -ahci_init_controller(int bdf) +ahci_init_controller(struct pci_device *pci) { struct ahci_ctrl_s *ctrl = malloc_fseg(sizeof(*ctrl)); + u16 bdf = pci->bdf; u32 val;
if (!ctrl) { @@ -606,6 +610,7 @@ ahci_init_controller(int bdf) return; }
+ ctrl->pci_tmp = pci; ctrl->pci_bdf = bdf; ctrl->iobase = pci_config_readl(bdf, PCI_BASE_ADDRESS_5); ctrl->irq = pci_config_readb(bdf, PCI_INTERRUPT_LINE); @@ -637,7 +642,7 @@ ahci_init(void) continue; if (pci->prog_if != 1 /* AHCI rev 1 */) continue; - ahci_init_controller(pci->bdf); + ahci_init_controller(pci); } }
diff --git a/src/ahci.h b/src/ahci.h index 0e13e00..98ade63 100644 --- a/src/ahci.h +++ b/src/ahci.h @@ -26,7 +26,8 @@ struct sata_cmd_fis { };
struct ahci_ctrl_s { - int pci_bdf; + struct pci_device *pci_tmp; + u16 pci_bdf; u8 irq; u32 iobase; u32 caps;
On Thu, Jul 14, 2011 at 04:23:58PM +0200, Gerd Hoffmann wrote:
Hi,
Finally the update for the ahci code, featuring:
- A bunch of bugfixes, which makes the code work on real hardware (additionally to the qemu emulated ahci adapter).
- Better timeout handling.
- Support for unaligned access.
- Support for boot ordering.
I've confirmed that with this patch series applied I can boot my Asrock e350m1 board in AHCI mode.
-Kevin
On Sun, Jul 17, 2011 at 8:16 AM, Kevin O'Connor kevin@koconnor.net wrote:
On Thu, Jul 14, 2011 at 04:23:58PM +0200, Gerd Hoffmann wrote:
Hi,
Finally the update for the ahci code, featuring:
* A bunch of bugfixes, which makes the code work on real hardware (additionally to the qemu emulated ahci adapter). * Better timeout handling. * Support for unaligned access. * Support for boot ordering.
I've confirmed that with this patch series applied I can boot my Asrock e350m1 board in AHCI mode.
-Kevin
Kevin,
You tested that by changing SATA_MODE in cfg.h? If it is working and commited, we should make ahci the default in coreboot.
Marc
On 07/17/11 16:16, Kevin O'Connor wrote:
On Thu, Jul 14, 2011 at 04:23:58PM +0200, Gerd Hoffmann wrote:
Hi,
Finally the update for the ahci code, featuring:
- A bunch of bugfixes, which makes the code work on real hardware (additionally to the qemu emulated ahci adapter).
- Better timeout handling.
- Support for unaligned access.
- Support for boot ordering.
I've confirmed that with this patch series applied I can boot my Asrock e350m1 board in AHCI mode.
Anything which holds this series from being committed? Do you wait for more test feedback?
cheers, Gerd
On Fri, Jul 22, 2011 at 05:10:21PM +0200, Gerd Hoffmann wrote:
On 07/17/11 16:16, Kevin O'Connor wrote:
On Thu, Jul 14, 2011 at 04:23:58PM +0200, Gerd Hoffmann wrote:
Hi,
Finally the update for the ahci code, featuring:
- A bunch of bugfixes, which makes the code work on real hardware (additionally to the qemu emulated ahci adapter).
- Better timeout handling.
- Support for unaligned access.
- Support for boot ordering.
I've confirmed that with this patch series applied I can boot my Asrock e350m1 board in AHCI mode.
Anything which holds this series from being committed? Do you wait for more test feedback?
Thanks. I applied the patch series.
I do have some comments, but I think they can be addressed on top if needed.
In patch 2 - there is a nested loop - the inside looping checking for an interrupt and the outside loop checking for BSY. I don't see how an interrupt could fire when the drive is reporting BSY - is this really needed?
In patch 4 - the buffer should really be shared with all AHCI controllers and the CDROM emulation code.
In patch 6 - it looks like each port will try to detect a drive by waiting 10ms. Ideally, this would be parallelized by launching a thread per-port. Also, ideally malloc_fseg() wouldn't be called unless the port was needed.
In patch 7 - the ahci code uses bootprio_find_ata_device() which tacks on "/disk@0" to the search pattern which is redundant as ahci doesn't have "slave" drives - is this really needed?
-Kevin
Hi,
Finally found the time to look at this.
I do have some comments, but I think they can be addressed on top if needed.
In patch 2 - there is a nested loop - the inside looping checking for an interrupt and the outside loop checking for BSY. I don't see how an interrupt could fire when the drive is reporting BSY - is this really needed?
I've seen qemu sending multiple updates, thats why the loop is there.
In patch 4 - the buffer should really be shared with all AHCI controllers and the CDROM emulation code.
Fixed.
In patch 6 - it looks like each port will try to detect a drive by waiting 10ms. Ideally, this would be parallelized by launching a thread per-port. Also, ideally malloc_fseg() wouldn't be called unless the port was needed.
Fixed.
In patch 7 - the ahci code uses bootprio_find_ata_device() which tacks on "/disk@0" to the search pattern which is redundant as ahci doesn't have "slave" drives - is this really needed?
It isn't required, but it also doesn't harm. qemu gives us a string with the redundant "/disk@0" included, thus bootprio_find_ata_device works as-is and I didn't bother creating a separate function for ahci/sata.
Patch series with the fixes mentioned above will go to the list in a moment.
cheers, Gerd
-
Gerd Hoffmann -
Kevin O'Connor -
Marc Jones