From e9ebb2f76778d19227476e34c3d7aa6b8975c1b6 Mon Sep 17 00:00:00 2001 From: John Snow Date: Sat, 4 Jul 2015 02:06:02 -0400 Subject: ahci: Do not ignore memory access read size The only guidance the AHCI specification gives on memory access is: "Register accesses shall have a maximum size of 64-bits; 64-bit access must not cross an 8-byte alignment boundary." I interpret this to mean that aligned or unaligned 1, 2 and 4 byte accesses should work, as well as aligned 8 byte accesses. In practice, a real Q35/ICH9 responds to 1, 2, 4 and 8 byte reads regardless of alignment. Windows 7 can be observed making 1 byte reads to the middle of 32 bit registers to fetch error codes. Introduce a wrapper to support unaligned accesses to AHCI. This wrapper will support aligned 8 byte reads, but will make no effort to support unaligned 8 byte reads, which although they will work on real hardware, are not guaranteed to work and do not appear to be used by either Windows or Linux. Signed-off-by: John Snow Reviewed-by: Eric Blake Message-id: 1434470575-21625-2-git-send-email-jsnow@redhat.com --- hw/ide/ahci.c | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) (limited to 'hw/ide/ahci.c') diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index b4b65c100a..0d6a2d8b4c 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -331,8 +331,7 @@ static void ahci_port_write(AHCIState *s, int port, int offset, uint32_t val) } } -static uint64_t ahci_mem_read(void *opaque, hwaddr addr, - unsigned size) +static uint64_t ahci_mem_read_32(void *opaque, hwaddr addr) { AHCIState *s = opaque; uint32_t val = 0; @@ -368,6 +367,30 @@ static uint64_t ahci_mem_read(void *opaque, hwaddr addr, } +/** + * AHCI 1.3 section 3 ("HBA Memory Registers") + * Support unaligned 8/16/32 bit reads, and 64 bit aligned reads. + * Caller is responsible for masking unwanted higher order bytes. + */ +static uint64_t ahci_mem_read(void *opaque, hwaddr addr, unsigned size) +{ + hwaddr aligned = addr & ~0x3; + int ofst = addr - aligned; + uint64_t lo = ahci_mem_read_32(opaque, aligned); + uint64_t hi; + + /* if < 8 byte read does not cross 4 byte boundary */ + if (ofst + size <= 4) { + return lo >> (ofst * 8); + } + g_assert_cmpint(size, >, 1); + + /* If the 64bit read is unaligned, we will produce undefined + * results. AHCI does not support unaligned 64bit reads. */ + hi = ahci_mem_read_32(opaque, aligned + 4); + return (hi << 32 | lo) >> (ofst * 8); +} + static void ahci_mem_write(void *opaque, hwaddr addr, uint64_t val, unsigned size) -- cgit 1.4.1 From b6fe41fa6dbdf7b92b76cd4848ef442de18e03d3 Mon Sep 17 00:00:00 2001 From: John Snow Date: Sat, 4 Jul 2015 02:06:03 -0400 Subject: ahci: use shorter variables Trivial cleanup that I didn't want to tack-on to anything else. Signed-off-by: John Snow Reviewed-by: Stefan Hajnoczi Message-id: 1435016308-6150-3-git-send-email-jsnow@redhat.com --- hw/ide/ahci.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) (limited to 'hw/ide/ahci.c') diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index 0d6a2d8b4c..3a8bff7e12 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -972,9 +972,11 @@ static int is_ncq(uint8_t ata_cmd) static void process_ncq_command(AHCIState *s, int port, uint8_t *cmd_fis, int slot) { + AHCIDevice *ad = &s->dev[port]; + IDEState *ide_state = &ad->port.ifs[0]; NCQFrame *ncq_fis = (NCQFrame*)cmd_fis; uint8_t tag = ncq_fis->tag >> 3; - NCQTransferState *ncq_tfs = &s->dev[port].ncq_tfs[tag]; + NCQTransferState *ncq_tfs = &ad->ncq_tfs[tag]; if (ncq_tfs->used) { /* error - already in use */ @@ -983,7 +985,7 @@ static void process_ncq_command(AHCIState *s, int port, uint8_t *cmd_fis, } ncq_tfs->used = 1; - ncq_tfs->drive = &s->dev[port]; + ncq_tfs->drive = ad; ncq_tfs->slot = slot; ncq_tfs->lba = ((uint64_t)ncq_fis->lba5 << 40) | ((uint64_t)ncq_fis->lba4 << 32) | @@ -1000,9 +1002,9 @@ static void process_ncq_command(AHCIState *s, int port, uint8_t *cmd_fis, DPRINTF(port, "NCQ transfer LBA from %"PRId64" to %"PRId64", " "drive max %"PRId64"\n", ncq_tfs->lba, ncq_tfs->lba + ncq_tfs->sector_count - 2, - s->dev[port].port.ifs[0].nb_sectors - 1); + ide_state->nb_sectors - 1); - ahci_populate_sglist(&s->dev[port], &ncq_tfs->sglist, 0); + ahci_populate_sglist(ad, &ncq_tfs->sglist, 0); ncq_tfs->tag = tag; switch(ncq_fis->command) { @@ -1014,9 +1016,9 @@ static void process_ncq_command(AHCIState *s, int port, uint8_t *cmd_fis, DPRINTF(port, "tag %d aio read %"PRId64"\n", ncq_tfs->tag, ncq_tfs->lba); - dma_acct_start(ncq_tfs->drive->port.ifs[0].blk, &ncq_tfs->acct, + dma_acct_start(ide_state->blk, &ncq_tfs->acct, &ncq_tfs->sglist, BLOCK_ACCT_READ); - ncq_tfs->aiocb = dma_blk_read(ncq_tfs->drive->port.ifs[0].blk, + ncq_tfs->aiocb = dma_blk_read(ide_state->blk, &ncq_tfs->sglist, ncq_tfs->lba, ncq_cb, ncq_tfs); break; @@ -1027,9 +1029,9 @@ static void process_ncq_command(AHCIState *s, int port, uint8_t *cmd_fis, DPRINTF(port, "tag %d aio write %"PRId64"\n", ncq_tfs->tag, ncq_tfs->lba); - dma_acct_start(ncq_tfs->drive->port.ifs[0].blk, &ncq_tfs->acct, + dma_acct_start(ide_state->blk, &ncq_tfs->acct, &ncq_tfs->sglist, BLOCK_ACCT_WRITE); - ncq_tfs->aiocb = dma_blk_write(ncq_tfs->drive->port.ifs[0].blk, + ncq_tfs->aiocb = dma_blk_write(ide_state->blk, &ncq_tfs->sglist, ncq_tfs->lba, ncq_cb, ncq_tfs); break; -- cgit 1.4.1 From a55c8231d04e3023bc5c3da9290f01e7d6989a94 Mon Sep 17 00:00:00 2001 From: John Snow Date: Sat, 4 Jul 2015 02:06:03 -0400 Subject: ahci: add ncq_err helper Set some appropriate error bits for NCQ for us. Signed-off-by: John Snow Reviewed-by: Stefan Hajnoczi Message-id: 1435016308-6150-4-git-send-email-jsnow@redhat.com --- hw/ide/ahci.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) (limited to 'hw/ide/ahci.c') diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index 3a8bff7e12..7b286a2652 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -922,6 +922,15 @@ out: return r; } +static void ncq_err(NCQTransferState *ncq_tfs) +{ + IDEState *ide_state = &ncq_tfs->drive->port.ifs[0]; + + ide_state->error = ABRT_ERR; + ide_state->status = READY_STAT | ERR_STAT; + ncq_tfs->drive->port_regs.scr_err |= (1 << ncq_tfs->tag); +} + static void ncq_cb(void *opaque, int ret) { NCQTransferState *ncq_tfs = (NCQTransferState *)opaque; @@ -934,10 +943,7 @@ static void ncq_cb(void *opaque, int ret) ncq_tfs->drive->port_regs.scr_act &= ~(1 << ncq_tfs->tag); if (ret < 0) { - /* error */ - ide_state->error = ABRT_ERR; - ide_state->status = READY_STAT | ERR_STAT; - ncq_tfs->drive->port_regs.scr_err |= (1 << ncq_tfs->tag); + ncq_err(ncq_tfs); } else { ide_state->status = READY_STAT | SEEK_STAT; } -- cgit 1.4.1 From 3bcbe4aa803c1a41e5392ecac7b4fc3c99a42f89 Mon Sep 17 00:00:00 2001 From: John Snow Date: Sat, 4 Jul 2015 02:06:03 -0400 Subject: ahci: check for ncq prdtl overflow Don't attempt the NCQ transfer if the PRDT we were given is not big enough to perform the entire transfer. Signed-off-by: John Snow Reviewed-by: Stefan Hajnoczi Message-id: 1435016308-6150-5-git-send-email-jsnow@redhat.com --- hw/ide/ahci.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) (limited to 'hw/ide/ahci.c') diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index 7b286a2652..f18d1f9b0b 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -983,6 +983,7 @@ static void process_ncq_command(AHCIState *s, int port, uint8_t *cmd_fis, NCQFrame *ncq_fis = (NCQFrame*)cmd_fis; uint8_t tag = ncq_fis->tag >> 3; NCQTransferState *ncq_tfs = &ad->ncq_tfs[tag]; + size_t size; if (ncq_tfs->used) { /* error - already in use */ @@ -999,20 +1000,28 @@ static void process_ncq_command(AHCIState *s, int port, uint8_t *cmd_fis, ((uint64_t)ncq_fis->lba2 << 16) | ((uint64_t)ncq_fis->lba1 << 8) | (uint64_t)ncq_fis->lba0; + ncq_tfs->tag = tag; - /* Note: We calculate the sector count, but don't currently rely on it. - * The total size of the DMA buffer tells us the transfer size instead. */ ncq_tfs->sector_count = ((uint16_t)ncq_fis->sector_count_high << 8) | ncq_fis->sector_count_low; + ahci_populate_sglist(ad, &ncq_tfs->sglist, 0); + size = ncq_tfs->sector_count * 512; + + if (ncq_tfs->sglist.size < size) { + error_report("ahci: PRDT length for NCQ command (0x%zx) " + "is smaller than the requested size (0x%zx)", + ncq_tfs->sglist.size, size); + qemu_sglist_destroy(&ncq_tfs->sglist); + ncq_err(ncq_tfs); + ahci_trigger_irq(ad->hba, ad, PORT_IRQ_OVERFLOW); + return; + } DPRINTF(port, "NCQ transfer LBA from %"PRId64" to %"PRId64", " "drive max %"PRId64"\n", ncq_tfs->lba, ncq_tfs->lba + ncq_tfs->sector_count - 2, ide_state->nb_sectors - 1); - ahci_populate_sglist(ad, &ncq_tfs->sglist, 0); - ncq_tfs->tag = tag; - switch(ncq_fis->command) { case READ_FPDMA_QUEUED: DPRINTF(port, "NCQ reading %d sectors from LBA %"PRId64", " @@ -1051,6 +1060,7 @@ static void process_ncq_command(AHCIState *s, int port, uint8_t *cmd_fis, "error: tried to process non-NCQ command as NCQ\n"); } qemu_sglist_destroy(&ncq_tfs->sglist); + ncq_err(ncq_tfs); } } -- cgit 1.4.1 From d56f4d6965ebcf8f3c496845c286e3a66496fdff Mon Sep 17 00:00:00 2001 From: John Snow Date: Sat, 4 Jul 2015 02:06:03 -0400 Subject: ahci: separate prdtl from opts There's no real reason to have it bundled together, and this way is a little nicer to follow if you have the AHCI spec pulled up. Signed-off-by: John Snow Reviewed-by: Stefan Hajnoczi Message-id: 1435016308-6150-6-git-send-email-jsnow@redhat.com --- hw/ide/ahci.c | 23 ++++++++++++----------- hw/ide/ahci.h | 3 ++- 2 files changed, 14 insertions(+), 12 deletions(-) (limited to 'hw/ide/ahci.c') diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index f18d1f9b0b..4d0615b82b 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -834,10 +834,11 @@ static int ahci_populate_sglist(AHCIDevice *ad, QEMUSGList *sglist, int32_t offset) { AHCICmdHdr *cmd = ad->cur_cmd; - uint32_t opts = le32_to_cpu(cmd->opts); - uint64_t prdt_addr = le64_to_cpu(cmd->tbl_addr) + 0x80; - int sglist_alloc_hint = opts >> AHCI_CMD_HDR_PRDT_LEN; - dma_addr_t prdt_len = (sglist_alloc_hint * sizeof(AHCI_SG)); + uint16_t opts = le16_to_cpu(cmd->opts); + uint16_t prdtl = le16_to_cpu(cmd->prdtl); + uint64_t cfis_addr = le64_to_cpu(cmd->tbl_addr); + uint64_t prdt_addr = cfis_addr + 0x80; + dma_addr_t prdt_len = (prdtl * sizeof(AHCI_SG)); dma_addr_t real_prdt_len = prdt_len; uint8_t *prdt; int i; @@ -857,7 +858,7 @@ static int ahci_populate_sglist(AHCIDevice *ad, QEMUSGList *sglist, * request for sector sizes up to 32K. */ - if (!sglist_alloc_hint) { + if (!prdtl) { DPRINTF(ad->port_no, "no sg list given by guest: 0x%08x\n", opts); return -1; } @@ -876,10 +877,10 @@ static int ahci_populate_sglist(AHCIDevice *ad, QEMUSGList *sglist, } /* Get entries in the PRDT, init a qemu sglist accordingly */ - if (sglist_alloc_hint > 0) { + if (prdtl > 0) { AHCI_SG *tbl = (AHCI_SG *)prdt; sum = 0; - for (i = 0; i < sglist_alloc_hint; i++) { + for (i = 0; i < prdtl; i++) { /* flags_size is zero-based */ tbl_entry_size = prdt_tbl_entry_size(&tbl[i]); if (offset <= (sum + tbl_entry_size)) { @@ -897,12 +898,12 @@ static int ahci_populate_sglist(AHCIDevice *ad, QEMUSGList *sglist, goto out; } - qemu_sglist_init(sglist, qbus->parent, (sglist_alloc_hint - off_idx), + qemu_sglist_init(sglist, qbus->parent, (prdtl - off_idx), ad->hba->as); qemu_sglist_add(sglist, le64_to_cpu(tbl[off_idx].addr) + off_pos, prdt_tbl_entry_size(&tbl[off_idx]) - off_pos); - for (i = off_idx + 1; i < sglist_alloc_hint; i++) { + for (i = off_idx + 1; i < prdtl; i++) { /* flags_size is zero-based */ qemu_sglist_add(sglist, le64_to_cpu(tbl[i].addr), prdt_tbl_entry_size(&tbl[i])); @@ -1069,7 +1070,7 @@ static void handle_reg_h2d_fis(AHCIState *s, int port, { IDEState *ide_state = &s->dev[port].port.ifs[0]; AHCICmdHdr *cmd = s->dev[port].cur_cmd; - uint32_t opts = le32_to_cpu(cmd->opts); + uint16_t opts = le16_to_cpu(cmd->opts); if (cmd_fis[1] & 0x0F) { DPRINTF(port, "Port Multiplier not supported." @@ -1226,7 +1227,7 @@ static void ahci_start_transfer(IDEDMA *dma) IDEState *s = &ad->port.ifs[0]; uint32_t size = (uint32_t)(s->data_end - s->data_ptr); /* write == ram -> device */ - uint32_t opts = le32_to_cpu(ad->cur_cmd->opts); + uint16_t opts = le16_to_cpu(ad->cur_cmd->opts); int is_write = opts & AHCI_CMD_WRITE; int is_atapi = opts & AHCI_CMD_ATAPI; int has_sglist = 0; diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h index 6d167f2736..b8872a4e4d 100644 --- a/hw/ide/ahci.h +++ b/hw/ide/ahci.h @@ -236,7 +236,8 @@ typedef struct AHCIPortRegs { } AHCIPortRegs; typedef struct AHCICmdHdr { - uint32_t opts; + uint16_t opts; + uint16_t prdtl; uint32_t status; uint64_t tbl_addr; uint32_t reserved[4]; -- cgit 1.4.1 From 5d5f89212f19e3d7d3da1328137ca9e33eead7bf Mon Sep 17 00:00:00 2001 From: John Snow Date: Sat, 4 Jul 2015 02:06:03 -0400 Subject: ahci: add ncq debug checks Most of the time, these bits can be safely ignored. For the purposes of debugging however, it's nice to know that they're not being used. Signed-off-by: John Snow Reviewed-by: Stefan Hajnoczi Message-id: 1435016308-6150-7-git-send-email-jsnow@redhat.com --- hw/ide/ahci.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) (limited to 'hw/ide/ahci.c') diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index 4d0615b82b..b78017a9dd 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -1003,6 +1003,25 @@ static void process_ncq_command(AHCIState *s, int port, uint8_t *cmd_fis, (uint64_t)ncq_fis->lba0; ncq_tfs->tag = tag; + /* Sanity-check the NCQ packet */ + if (tag != slot) { + DPRINTF(port, "Warn: NCQ slot (%d) did not match the given tag (%d)\n", + slot, tag); + } + + if (ncq_fis->aux0 || ncq_fis->aux1 || ncq_fis->aux2 || ncq_fis->aux3) { + DPRINTF(port, "Warn: Attempt to use NCQ auxiliary fields.\n"); + } + if (ncq_fis->prio || ncq_fis->icc) { + DPRINTF(port, "Warn: Unsupported attempt to use PRIO/ICC fields\n"); + } + if (ncq_fis->fua & NCQ_FIS_FUA_MASK) { + DPRINTF(port, "Warn: Unsupported attempt to use Force Unit Access\n"); + } + if (ncq_fis->tag & NCQ_FIS_RARC_MASK) { + DPRINTF(port, "Warn: Unsupported attempt to use Rebuild Assist\n"); + } + ncq_tfs->sector_count = ((uint16_t)ncq_fis->sector_count_high << 8) | ncq_fis->sector_count_low; ahci_populate_sglist(ad, &ncq_tfs->sglist, 0); @@ -1016,6 +1035,10 @@ static void process_ncq_command(AHCIState *s, int port, uint8_t *cmd_fis, ncq_err(ncq_tfs); ahci_trigger_irq(ad->hba, ad, PORT_IRQ_OVERFLOW); return; + } else if (ncq_tfs->sglist.size != size) { + DPRINTF(port, "Warn: PRDTL (0x%zx)" + " does not match requested size (0x%zx)", + ncq_tfs->sglist.size, size); } DPRINTF(port, "NCQ transfer LBA from %"PRId64" to %"PRId64", " -- cgit 1.4.1 From 0437d32ae232af37d3b94064a563eb51d4eedd62 Mon Sep 17 00:00:00 2001 From: John Snow Date: Sat, 4 Jul 2015 02:06:03 -0400 Subject: ahci: ncq sector count correction This value should not be size-corrected, 0 sectors does not imply 1 sector(s). This is just debug information, but it's misleading! Signed-off-by: John Snow Reviewed-by: Stefan Hajnoczi Message-id: 1435016308-6150-8-git-send-email-jsnow@redhat.com --- hw/ide/ahci.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'hw/ide/ahci.c') diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index b78017a9dd..b73a2e4784 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -1043,14 +1043,14 @@ static void process_ncq_command(AHCIState *s, int port, uint8_t *cmd_fis, DPRINTF(port, "NCQ transfer LBA from %"PRId64" to %"PRId64", " "drive max %"PRId64"\n", - ncq_tfs->lba, ncq_tfs->lba + ncq_tfs->sector_count - 2, + ncq_tfs->lba, ncq_tfs->lba + ncq_tfs->sector_count - 1, ide_state->nb_sectors - 1); switch(ncq_fis->command) { case READ_FPDMA_QUEUED: DPRINTF(port, "NCQ reading %d sectors from LBA %"PRId64", " "tag %d\n", - ncq_tfs->sector_count-1, ncq_tfs->lba, ncq_tfs->tag); + ncq_tfs->sector_count, ncq_tfs->lba, ncq_tfs->tag); DPRINTF(port, "tag %d aio read %"PRId64"\n", ncq_tfs->tag, ncq_tfs->lba); @@ -1063,7 +1063,7 @@ static void process_ncq_command(AHCIState *s, int port, uint8_t *cmd_fis, break; case WRITE_FPDMA_QUEUED: DPRINTF(port, "NCQ writing %d sectors to LBA %"PRId64", tag %d\n", - ncq_tfs->sector_count-1, ncq_tfs->lba, ncq_tfs->tag); + ncq_tfs->sector_count, ncq_tfs->lba, ncq_tfs->tag); DPRINTF(port, "tag %d aio write %"PRId64"\n", ncq_tfs->tag, ncq_tfs->lba); -- cgit 1.4.1 From a718978ed58abc1ad92567a9c17525136be02a71 Mon Sep 17 00:00:00 2001 From: John Snow Date: Sat, 4 Jul 2015 02:06:04 -0400 Subject: ide: add limit to .prepare_buf() prepare_buf should not always grab as many descriptors as it can, sometimes it should self-limit. For example, an NCQ transfer of 1 sector with a PRDT that describes 4GiB of data should not copy 4GiB of data, it should just transfer that first 512 bytes. PIO is not affected, because the dma_buf_rw dma helpers already have a byte limit built-in to them, but DMA/NCQ will exhaust the entire list regardless of requested size. AHCI 1.3 specifies in section 6.1.6 Command List Underflow that NCQ is not required to detect underflow conditions. Non-NCQ pathways signal underflow by writing to the PRDBC field, which will already occur by writing the actual transferred byte count to the PRDBC, signaling the underflow. Our NCQ pathways aren't required to detect underflow, but since our DMA backend uses the size of the PRDT to determine the size of the transer, if our PRDT is bigger than the transaction (the underflow condition) it doesn't cost us anything to detect it and truncate the PRDT. This is a recoverable error and is not signaled to the guest, in either NCQ or normal DMA cases. For BMDMA, the existing pathways should see no guest-visible difference, but any bytes described in the overage will no longer be transferred before indicating to the guest that there was an underflow. Signed-off-by: John Snow Reviewed-by: Stefan Hajnoczi Message-id: 1435767578-32743-2-git-send-email-jsnow@redhat.com --- hw/ide/ahci.c | 27 ++++++++++++++------------- hw/ide/core.c | 8 ++++---- hw/ide/internal.h | 2 +- hw/ide/macio.c | 2 +- hw/ide/pci.c | 21 ++++++++++++++++----- 5 files changed, 36 insertions(+), 24 deletions(-) (limited to 'hw/ide/ahci.c') diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index b73a2e4784..de1759a24d 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -49,7 +49,7 @@ static int handle_cmd(AHCIState *s,int port,int slot); static void ahci_reset_port(AHCIState *s, int port); static void ahci_write_fis_d2h(AHCIDevice *ad, uint8_t *cmd_fis); static void ahci_init_d2h(AHCIDevice *ad); -static int ahci_dma_prepare_buf(IDEDMA *dma, int is_write); +static int ahci_dma_prepare_buf(IDEDMA *dma, int32_t limit); static void ahci_commit_buf(IDEDMA *dma, uint32_t tx_bytes); static bool ahci_map_clb_address(AHCIDevice *ad); static bool ahci_map_fis_address(AHCIDevice *ad); @@ -827,11 +827,12 @@ static void ahci_write_fis_d2h(AHCIDevice *ad, uint8_t *cmd_fis) static int prdt_tbl_entry_size(const AHCI_SG *tbl) { + /* flags_size is zero-based */ return (le32_to_cpu(tbl->flags_size) & AHCI_PRDT_SIZE_MASK) + 1; } static int ahci_populate_sglist(AHCIDevice *ad, QEMUSGList *sglist, - int32_t offset) + int64_t limit, int32_t offset) { AHCICmdHdr *cmd = ad->cur_cmd; uint16_t opts = le16_to_cpu(cmd->opts); @@ -881,9 +882,8 @@ static int ahci_populate_sglist(AHCIDevice *ad, QEMUSGList *sglist, AHCI_SG *tbl = (AHCI_SG *)prdt; sum = 0; for (i = 0; i < prdtl; i++) { - /* flags_size is zero-based */ tbl_entry_size = prdt_tbl_entry_size(&tbl[i]); - if (offset <= (sum + tbl_entry_size)) { + if (offset < (sum + tbl_entry_size)) { off_idx = i; off_pos = offset - sum; break; @@ -901,12 +901,13 @@ static int ahci_populate_sglist(AHCIDevice *ad, QEMUSGList *sglist, qemu_sglist_init(sglist, qbus->parent, (prdtl - off_idx), ad->hba->as); qemu_sglist_add(sglist, le64_to_cpu(tbl[off_idx].addr) + off_pos, - prdt_tbl_entry_size(&tbl[off_idx]) - off_pos); + MIN(prdt_tbl_entry_size(&tbl[off_idx]) - off_pos, + limit)); - for (i = off_idx + 1; i < prdtl; i++) { - /* flags_size is zero-based */ + for (i = off_idx + 1; i < prdtl && sglist->size < limit; i++) { qemu_sglist_add(sglist, le64_to_cpu(tbl[i].addr), - prdt_tbl_entry_size(&tbl[i])); + MIN(prdt_tbl_entry_size(&tbl[i]), + limit - sglist->size)); if (sglist->size > INT32_MAX) { error_report("AHCI Physical Region Descriptor Table describes " "more than 2 GiB.\n"); @@ -1024,8 +1025,8 @@ static void process_ncq_command(AHCIState *s, int port, uint8_t *cmd_fis, ncq_tfs->sector_count = ((uint16_t)ncq_fis->sector_count_high << 8) | ncq_fis->sector_count_low; - ahci_populate_sglist(ad, &ncq_tfs->sglist, 0); size = ncq_tfs->sector_count * 512; + ahci_populate_sglist(ad, &ncq_tfs->sglist, size, 0); if (ncq_tfs->sglist.size < size) { error_report("ahci: PRDT length for NCQ command (0x%zx) " @@ -1262,7 +1263,7 @@ static void ahci_start_transfer(IDEDMA *dma) goto out; } - if (ahci_dma_prepare_buf(dma, is_write)) { + if (ahci_dma_prepare_buf(dma, size)) { has_sglist = 1; } @@ -1312,12 +1313,12 @@ static void ahci_restart_dma(IDEDMA *dma) * Not currently invoked by PIO R/W chains, * which invoke ahci_populate_sglist via ahci_start_transfer. */ -static int32_t ahci_dma_prepare_buf(IDEDMA *dma, int is_write) +static int32_t ahci_dma_prepare_buf(IDEDMA *dma, int32_t limit) { AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma); IDEState *s = &ad->port.ifs[0]; - if (ahci_populate_sglist(ad, &s->sg, s->io_buffer_offset) == -1) { + if (ahci_populate_sglist(ad, &s->sg, limit, s->io_buffer_offset) == -1) { DPRINTF(ad->port_no, "ahci_dma_prepare_buf failed.\n"); return -1; } @@ -1352,7 +1353,7 @@ static int ahci_dma_rw_buf(IDEDMA *dma, int is_write) uint8_t *p = s->io_buffer + s->io_buffer_index; int l = s->io_buffer_size - s->io_buffer_index; - if (ahci_populate_sglist(ad, &s->sg, s->io_buffer_offset)) { + if (ahci_populate_sglist(ad, &s->sg, l, s->io_buffer_offset)) { return 0; } diff --git a/hw/ide/core.c b/hw/ide/core.c index 1efd98af63..be7c350b87 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -716,8 +716,8 @@ static void ide_dma_cb(void *opaque, int ret) sector_num = ide_get_sector(s); if (n > 0) { - assert(s->io_buffer_size == s->sg.size); - dma_buf_commit(s, s->io_buffer_size); + assert(n * 512 == s->sg.size); + dma_buf_commit(s, s->sg.size); sector_num += n; ide_set_sector(s, sector_num); s->nsector -= n; @@ -734,7 +734,7 @@ static void ide_dma_cb(void *opaque, int ret) n = s->nsector; s->io_buffer_index = 0; s->io_buffer_size = n * 512; - if (s->bus->dma->ops->prepare_buf(s->bus->dma, ide_cmd_is_read(s)) < 512) { + if (s->bus->dma->ops->prepare_buf(s->bus->dma, s->io_buffer_size) < 512) { /* The PRDs were too short. Reset the Active bit, but don't raise an * interrupt. */ s->status = READY_STAT | SEEK_STAT; @@ -2326,7 +2326,7 @@ static void ide_nop(IDEDMA *dma) { } -static int32_t ide_nop_int32(IDEDMA *dma, int x) +static int32_t ide_nop_int32(IDEDMA *dma, int32_t l) { return 0; } diff --git a/hw/ide/internal.h b/hw/ide/internal.h index 965cc55cb8..3736e1bbfe 100644 --- a/hw/ide/internal.h +++ b/hw/ide/internal.h @@ -324,7 +324,7 @@ typedef void EndTransferFunc(IDEState *); typedef void DMAStartFunc(IDEDMA *, IDEState *, BlockCompletionFunc *); typedef void DMAVoidFunc(IDEDMA *); typedef int DMAIntFunc(IDEDMA *, int); -typedef int32_t DMAInt32Func(IDEDMA *, int); +typedef int32_t DMAInt32Func(IDEDMA *, int32_t len); typedef void DMAu32Func(IDEDMA *, uint32_t); typedef void DMAStopFunc(IDEDMA *, bool); typedef void DMARestartFunc(void *, int, RunState); diff --git a/hw/ide/macio.c b/hw/ide/macio.c index dd52d50732..a55a479da6 100644 --- a/hw/ide/macio.c +++ b/hw/ide/macio.c @@ -499,7 +499,7 @@ static int ide_nop_int(IDEDMA *dma, int x) return 0; } -static int32_t ide_nop_int32(IDEDMA *dma, int x) +static int32_t ide_nop_int32(IDEDMA *dma, int32_t l) { return 0; } diff --git a/hw/ide/pci.c b/hw/ide/pci.c index 4afd0cfe8c..d31ff885b7 100644 --- a/hw/ide/pci.c +++ b/hw/ide/pci.c @@ -53,10 +53,14 @@ static void bmdma_start_dma(IDEDMA *dma, IDEState *s, } /** - * Return the number of bytes successfully prepared. - * -1 on error. + * Prepare an sglist based on available PRDs. + * @limit: How many bytes to prepare total. + * + * Returns the number of bytes prepared, -1 on error. + * IDEState.io_buffer_size will contain the number of bytes described + * by the PRDs, whether or not we added them to the sglist. */ -static int32_t bmdma_prepare_buf(IDEDMA *dma, int is_write) +static int32_t bmdma_prepare_buf(IDEDMA *dma, int32_t limit) { BMDMAState *bm = DO_UPCAST(BMDMAState, dma, dma); IDEState *s = bmdma_active_if(bm); @@ -75,7 +79,7 @@ static int32_t bmdma_prepare_buf(IDEDMA *dma, int is_write) /* end of table (with a fail safe of one page) */ if (bm->cur_prd_last || (bm->cur_addr - bm->addr) >= BMDMA_PAGE_SIZE) { - return s->io_buffer_size; + return s->sg.size; } pci_dma_read(pci_dev, bm->cur_addr, &prd, 8); bm->cur_addr += 8; @@ -90,7 +94,14 @@ static int32_t bmdma_prepare_buf(IDEDMA *dma, int is_write) } l = bm->cur_prd_len; if (l > 0) { - qemu_sglist_add(&s->sg, bm->cur_prd_addr, l); + uint64_t sg_len; + + /* Don't add extra bytes to the SGList; consume any remaining + * PRDs from the guest, but ignore them. */ + sg_len = MIN(limit - s->sg.size, bm->cur_prd_len); + if (sg_len) { + qemu_sglist_add(&s->sg, bm->cur_prd_addr, sg_len); + } /* Note: We limit the max transfer to be 2GiB. * This should accommodate the largest ATA transaction -- cgit 1.4.1 From 4614619ee4ad96d2715dc41f9430fb43335c15d2 Mon Sep 17 00:00:00 2001 From: John Snow Date: Sat, 4 Jul 2015 02:06:04 -0400 Subject: ahci: stash ncq command For migration and werror=stop/rerror=stop resume purposes, it will be convenient to have the command handy inside of ncq_tfs. Eventually, we'd like to avoid reading from the FIS entirely after the initial read, so this is a byte (hah!) sized step in that direction. Signed-off-by: John Snow Reviewed-by: Stefan Hajnoczi Message-id: 1435767578-32743-3-git-send-email-jsnow@redhat.com --- hw/ide/ahci.c | 3 ++- hw/ide/ahci.h | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) (limited to 'hw/ide/ahci.c') diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index de1759a24d..9540a64a39 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -996,6 +996,7 @@ static void process_ncq_command(AHCIState *s, int port, uint8_t *cmd_fis, ncq_tfs->used = 1; ncq_tfs->drive = ad; ncq_tfs->slot = slot; + ncq_tfs->cmd = ncq_fis->command; ncq_tfs->lba = ((uint64_t)ncq_fis->lba5 << 40) | ((uint64_t)ncq_fis->lba4 << 32) | ((uint64_t)ncq_fis->lba3 << 24) | @@ -1047,7 +1048,7 @@ static void process_ncq_command(AHCIState *s, int port, uint8_t *cmd_fis, ncq_tfs->lba, ncq_tfs->lba + ncq_tfs->sector_count - 1, ide_state->nb_sectors - 1); - switch(ncq_fis->command) { + switch (ncq_tfs->cmd) { case READ_FPDMA_QUEUED: DPRINTF(port, "NCQ reading %d sectors from LBA %"PRId64", " "tag %d\n", diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h index b8872a4e4d..33607d7fad 100644 --- a/hw/ide/ahci.h +++ b/hw/ide/ahci.h @@ -259,6 +259,7 @@ typedef struct NCQTransferState { uint16_t sector_count; uint64_t lba; uint8_t tag; + uint8_t cmd; int slot; int used; } NCQTransferState; -- cgit 1.4.1 From 922f893e57da24bc80db3e79bea56485d1c111fa Mon Sep 17 00:00:00 2001 From: John Snow Date: Sat, 4 Jul 2015 02:06:04 -0400 Subject: ahci: assert is_ncq for process_ncq We already checked this in the handle_cmd phase, so just change this to an assertion and simplify the error logic. (Also, fix the switch indent, because checkpatch.pl yelled.) ((Sorry for churn.)) Signed-off-by: John Snow Reviewed-by: Stefan Hajnoczi Message-id: 1435767578-32743-4-git-send-email-jsnow@redhat.com --- hw/ide/ahci.c | 68 ++++++++++++++++++++++++++--------------------------------- 1 file changed, 30 insertions(+), 38 deletions(-) (limited to 'hw/ide/ahci.c') diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index 9540a64a39..8171f39aeb 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -987,6 +987,7 @@ static void process_ncq_command(AHCIState *s, int port, uint8_t *cmd_fis, NCQTransferState *ncq_tfs = &ad->ncq_tfs[tag]; size_t size; + g_assert(is_ncq(ncq_fis->command)); if (ncq_tfs->used) { /* error - already in use */ fprintf(stderr, "%s: tag %d already used\n", __FUNCTION__, tag); @@ -1049,44 +1050,35 @@ static void process_ncq_command(AHCIState *s, int port, uint8_t *cmd_fis, ide_state->nb_sectors - 1); switch (ncq_tfs->cmd) { - case READ_FPDMA_QUEUED: - DPRINTF(port, "NCQ reading %d sectors from LBA %"PRId64", " - "tag %d\n", - ncq_tfs->sector_count, ncq_tfs->lba, ncq_tfs->tag); - - DPRINTF(port, "tag %d aio read %"PRId64"\n", - ncq_tfs->tag, ncq_tfs->lba); - - dma_acct_start(ide_state->blk, &ncq_tfs->acct, - &ncq_tfs->sglist, BLOCK_ACCT_READ); - ncq_tfs->aiocb = dma_blk_read(ide_state->blk, - &ncq_tfs->sglist, ncq_tfs->lba, - ncq_cb, ncq_tfs); - break; - case WRITE_FPDMA_QUEUED: - DPRINTF(port, "NCQ writing %d sectors to LBA %"PRId64", tag %d\n", - ncq_tfs->sector_count, ncq_tfs->lba, ncq_tfs->tag); - - DPRINTF(port, "tag %d aio write %"PRId64"\n", - ncq_tfs->tag, ncq_tfs->lba); - - dma_acct_start(ide_state->blk, &ncq_tfs->acct, - &ncq_tfs->sglist, BLOCK_ACCT_WRITE); - ncq_tfs->aiocb = dma_blk_write(ide_state->blk, - &ncq_tfs->sglist, ncq_tfs->lba, - ncq_cb, ncq_tfs); - break; - default: - if (is_ncq(cmd_fis[2])) { - DPRINTF(port, - "error: unsupported NCQ command (0x%02x) received\n", - cmd_fis[2]); - } else { - DPRINTF(port, - "error: tried to process non-NCQ command as NCQ\n"); - } - qemu_sglist_destroy(&ncq_tfs->sglist); - ncq_err(ncq_tfs); + case READ_FPDMA_QUEUED: + DPRINTF(port, "NCQ reading %d sectors from LBA %"PRId64", tag %d\n", + ncq_tfs->sector_count, ncq_tfs->lba, ncq_tfs->tag); + + DPRINTF(port, "tag %d aio read %"PRId64"\n", + ncq_tfs->tag, ncq_tfs->lba); + + dma_acct_start(ide_state->blk, &ncq_tfs->acct, + &ncq_tfs->sglist, BLOCK_ACCT_READ); + ncq_tfs->aiocb = dma_blk_read(ide_state->blk, &ncq_tfs->sglist, + ncq_tfs->lba, ncq_cb, ncq_tfs); + break; + case WRITE_FPDMA_QUEUED: + DPRINTF(port, "NCQ writing %d sectors to LBA %"PRId64", tag %d\n", + ncq_tfs->sector_count, ncq_tfs->lba, ncq_tfs->tag); + + DPRINTF(port, "tag %d aio write %"PRId64"\n", + ncq_tfs->tag, ncq_tfs->lba); + + dma_acct_start(ide_state->blk, &ncq_tfs->acct, + &ncq_tfs->sglist, BLOCK_ACCT_WRITE); + ncq_tfs->aiocb = dma_blk_write(ide_state->blk, &ncq_tfs->sglist, + ncq_tfs->lba, ncq_cb, ncq_tfs); + break; + default: + DPRINTF(port, "error: unsupported NCQ command (0x%02x) received\n", + ncq_tfs->cmd); + qemu_sglist_destroy(&ncq_tfs->sglist); + ncq_err(ncq_tfs); } } -- cgit 1.4.1 From 631ddc22cbb401f2777dc8b117196f0988df4eea Mon Sep 17 00:00:00 2001 From: John Snow Date: Sat, 4 Jul 2015 02:06:04 -0400 Subject: ahci: refactor process_ncq_command Split off execute_ncq_command so that we can call it separately later if we desire. Signed-off-by: John Snow Reviewed-by: Stefan Hajnoczi Message-id: 1435767578-32743-5-git-send-email-jsnow@redhat.com --- hw/ide/ahci.c | 73 ++++++++++++++++++++++++++++++++++------------------------- 1 file changed, 42 insertions(+), 31 deletions(-) (limited to 'hw/ide/ahci.c') diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index 8171f39aeb..b3a6a91dbb 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -977,6 +977,47 @@ static int is_ncq(uint8_t ata_cmd) } } +static void execute_ncq_command(NCQTransferState *ncq_tfs) +{ + AHCIDevice *ad = ncq_tfs->drive; + IDEState *ide_state = &ad->port.ifs[0]; + int port = ad->port_no; + g_assert(is_ncq(ncq_tfs->cmd)); + + switch (ncq_tfs->cmd) { + case READ_FPDMA_QUEUED: + DPRINTF(port, "NCQ reading %d sectors from LBA %"PRId64", tag %d\n", + ncq_tfs->sector_count, ncq_tfs->lba, ncq_tfs->tag); + + DPRINTF(port, "tag %d aio read %"PRId64"\n", + ncq_tfs->tag, ncq_tfs->lba); + + dma_acct_start(ide_state->blk, &ncq_tfs->acct, + &ncq_tfs->sglist, BLOCK_ACCT_READ); + ncq_tfs->aiocb = dma_blk_read(ide_state->blk, &ncq_tfs->sglist, + ncq_tfs->lba, ncq_cb, ncq_tfs); + break; + case WRITE_FPDMA_QUEUED: + DPRINTF(port, "NCQ writing %d sectors to LBA %"PRId64", tag %d\n", + ncq_tfs->sector_count, ncq_tfs->lba, ncq_tfs->tag); + + DPRINTF(port, "tag %d aio write %"PRId64"\n", + ncq_tfs->tag, ncq_tfs->lba); + + dma_acct_start(ide_state->blk, &ncq_tfs->acct, + &ncq_tfs->sglist, BLOCK_ACCT_WRITE); + ncq_tfs->aiocb = dma_blk_write(ide_state->blk, &ncq_tfs->sglist, + ncq_tfs->lba, ncq_cb, ncq_tfs); + break; + default: + DPRINTF(port, "error: unsupported NCQ command (0x%02x) received\n", + ncq_tfs->cmd); + qemu_sglist_destroy(&ncq_tfs->sglist); + ncq_err(ncq_tfs); + } +} + + static void process_ncq_command(AHCIState *s, int port, uint8_t *cmd_fis, int slot) { @@ -1049,37 +1090,7 @@ static void process_ncq_command(AHCIState *s, int port, uint8_t *cmd_fis, ncq_tfs->lba, ncq_tfs->lba + ncq_tfs->sector_count - 1, ide_state->nb_sectors - 1); - switch (ncq_tfs->cmd) { - case READ_FPDMA_QUEUED: - DPRINTF(port, "NCQ reading %d sectors from LBA %"PRId64", tag %d\n", - ncq_tfs->sector_count, ncq_tfs->lba, ncq_tfs->tag); - - DPRINTF(port, "tag %d aio read %"PRId64"\n", - ncq_tfs->tag, ncq_tfs->lba); - - dma_acct_start(ide_state->blk, &ncq_tfs->acct, - &ncq_tfs->sglist, BLOCK_ACCT_READ); - ncq_tfs->aiocb = dma_blk_read(ide_state->blk, &ncq_tfs->sglist, - ncq_tfs->lba, ncq_cb, ncq_tfs); - break; - case WRITE_FPDMA_QUEUED: - DPRINTF(port, "NCQ writing %d sectors to LBA %"PRId64", tag %d\n", - ncq_tfs->sector_count, ncq_tfs->lba, ncq_tfs->tag); - - DPRINTF(port, "tag %d aio write %"PRId64"\n", - ncq_tfs->tag, ncq_tfs->lba); - - dma_acct_start(ide_state->blk, &ncq_tfs->acct, - &ncq_tfs->sglist, BLOCK_ACCT_WRITE); - ncq_tfs->aiocb = dma_blk_write(ide_state->blk, &ncq_tfs->sglist, - ncq_tfs->lba, ncq_cb, ncq_tfs); - break; - default: - DPRINTF(port, "error: unsupported NCQ command (0x%02x) received\n", - ncq_tfs->cmd); - qemu_sglist_destroy(&ncq_tfs->sglist); - ncq_err(ncq_tfs); - } + execute_ncq_command(ncq_tfs); } static void handle_reg_h2d_fis(AHCIState *s, int port, -- cgit 1.4.1 From 54f3223730736fca1e6e89bb7f99c4f8432fdabb Mon Sep 17 00:00:00 2001 From: John Snow Date: Sat, 4 Jul 2015 02:06:04 -0400 Subject: ahci: factor ncq_finish out of ncq_cb When we add werror=stop or rerror=stop support to NCQ, we'll want to take a codepath where we don't actually complete the command, so factor that out into a new routine. Signed-off-by: John Snow Reviewed-by: Stefan Hajnoczi Message-id: 1435767578-32743-6-git-send-email-jsnow@redhat.com --- hw/ide/ahci.c | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) (limited to 'hw/ide/ahci.c') diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index b3a6a91dbb..b0b9b41ed0 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -933,6 +933,23 @@ static void ncq_err(NCQTransferState *ncq_tfs) ncq_tfs->drive->port_regs.scr_err |= (1 << ncq_tfs->tag); } +static void ncq_finish(NCQTransferState *ncq_tfs) +{ + /* Clear bit for this tag in SActive */ + ncq_tfs->drive->port_regs.scr_act &= ~(1 << ncq_tfs->tag); + + ahci_write_fis_sdb(ncq_tfs->drive->hba, ncq_tfs->drive->port_no, + (1 << ncq_tfs->tag)); + + DPRINTF(ncq_tfs->drive->port_no, "NCQ transfer tag %d finished\n", + ncq_tfs->tag); + + block_acct_done(blk_get_stats(ncq_tfs->drive->port.ifs[0].blk), + &ncq_tfs->acct); + qemu_sglist_destroy(&ncq_tfs->sglist); + ncq_tfs->used = 0; +} + static void ncq_cb(void *opaque, int ret) { NCQTransferState *ncq_tfs = (NCQTransferState *)opaque; @@ -941,8 +958,6 @@ static void ncq_cb(void *opaque, int ret) if (ret == -ECANCELED) { return; } - /* Clear bit for this tag in SActive */ - ncq_tfs->drive->port_regs.scr_act &= ~(1 << ncq_tfs->tag); if (ret < 0) { ncq_err(ncq_tfs); @@ -950,16 +965,7 @@ static void ncq_cb(void *opaque, int ret) ide_state->status = READY_STAT | SEEK_STAT; } - ahci_write_fis_sdb(ncq_tfs->drive->hba, ncq_tfs->drive->port_no, - (1 << ncq_tfs->tag)); - - DPRINTF(ncq_tfs->drive->port_no, "NCQ transfer tag %d finished\n", - ncq_tfs->tag); - - block_acct_done(blk_get_stats(ncq_tfs->drive->port.ifs[0].blk), - &ncq_tfs->acct); - qemu_sglist_destroy(&ncq_tfs->sglist); - ncq_tfs->used = 0; + ncq_finish(ncq_tfs); } static int is_ncq(uint8_t ata_cmd) -- cgit 1.4.1 From 7c03a691077e71a08bbca06568cd97f09537458c Mon Sep 17 00:00:00 2001 From: John Snow Date: Sat, 4 Jul 2015 02:06:04 -0400 Subject: ahci: add rwerror=stop support for ncq Handle NCQ failures for cases where we want to halt the VM on IO errors. Upon a VM state change, retry the halted NCQ commands. Signed-off-by: John Snow Reviewed-by: Stefan Hajnoczi Message-id: 1435767578-32743-7-git-send-email-jsnow@redhat.com --- hw/ide/ahci.c | 36 ++++++++++++++++++++++++++++++++++-- hw/ide/ahci.h | 1 + hw/ide/core.c | 7 +++++++ hw/ide/internal.h | 2 ++ 4 files changed, 44 insertions(+), 2 deletions(-) (limited to 'hw/ide/ahci.c') diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index b0b9b41ed0..d996f37b36 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -581,6 +581,7 @@ static void ahci_reset_port(AHCIState *s, int port) /* reset ncq queue */ for (i = 0; i < AHCI_MAX_CMDS; i++) { NCQTransferState *ncq_tfs = &s->dev[port].ncq_tfs[i]; + ncq_tfs->halt = false; if (!ncq_tfs->used) { continue; } @@ -960,12 +961,23 @@ static void ncq_cb(void *opaque, int ret) } if (ret < 0) { - ncq_err(ncq_tfs); + bool is_read = ncq_tfs->cmd == READ_FPDMA_QUEUED; + BlockErrorAction action = blk_get_error_action(ide_state->blk, + is_read, -ret); + if (action == BLOCK_ERROR_ACTION_STOP) { + ncq_tfs->halt = true; + ide_state->bus->error_status = IDE_RETRY_HBA; + } else if (action == BLOCK_ERROR_ACTION_REPORT) { + ncq_err(ncq_tfs); + } + blk_error_action(ide_state->blk, action, is_read, -ret); } else { ide_state->status = READY_STAT | SEEK_STAT; } - ncq_finish(ncq_tfs); + if (!ncq_tfs->halt) { + ncq_finish(ncq_tfs); + } } static int is_ncq(uint8_t ata_cmd) @@ -988,7 +1000,9 @@ static void execute_ncq_command(NCQTransferState *ncq_tfs) AHCIDevice *ad = ncq_tfs->drive; IDEState *ide_state = &ad->port.ifs[0]; int port = ad->port_no; + g_assert(is_ncq(ncq_tfs->cmd)); + ncq_tfs->halt = false; switch (ncq_tfs->cmd) { case READ_FPDMA_QUEUED: @@ -1318,6 +1332,23 @@ static void ahci_restart_dma(IDEDMA *dma) /* Nothing to do, ahci_start_dma already resets s->io_buffer_offset. */ } +/** + * IDE/PIO restarts are handled by the core layer, but NCQ commands + * need an extra kick from the AHCI HBA. + */ +static void ahci_restart(IDEDMA *dma) +{ + AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma); + int i; + + for (i = 0; i < AHCI_MAX_CMDS; i++) { + NCQTransferState *ncq_tfs = &ad->ncq_tfs[i]; + if (ncq_tfs->halt) { + execute_ncq_command(ncq_tfs); + } + } +} + /** * Called in DMA R/W chains to read the PRDT, utilizing ahci_populate_sglist. * Not currently invoked by PIO R/W chains, @@ -1406,6 +1437,7 @@ static void ahci_irq_set(void *opaque, int n, int level) static const IDEDMAOps ahci_dma_ops = { .start_dma = ahci_start_dma, + .restart = ahci_restart, .restart_dma = ahci_restart_dma, .start_transfer = ahci_start_transfer, .prepare_buf = ahci_dma_prepare_buf, diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h index 33607d7fad..47a3122270 100644 --- a/hw/ide/ahci.h +++ b/hw/ide/ahci.h @@ -262,6 +262,7 @@ typedef struct NCQTransferState { uint8_t cmd; int slot; int used; + bool halt; } NCQTransferState; struct AHCIDevice { diff --git a/hw/ide/core.c b/hw/ide/core.c index be7c350b87..122e955084 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -2371,6 +2371,13 @@ static void ide_restart_bh(void *opaque) * called function can set a new error status. */ bus->error_status = 0; + /* The HBA has generically asked to be kicked on retry */ + if (error_status & IDE_RETRY_HBA) { + if (s->bus->dma->ops->restart) { + s->bus->dma->ops->restart(s->bus->dma); + } + } + if (error_status & IDE_RETRY_DMA) { if (error_status & IDE_RETRY_TRIM) { ide_restart_dma(s, IDE_DMA_TRIM); diff --git a/hw/ide/internal.h b/hw/ide/internal.h index 3736e1bbfe..30fdcbc5fa 100644 --- a/hw/ide/internal.h +++ b/hw/ide/internal.h @@ -436,6 +436,7 @@ struct IDEDMAOps { DMAInt32Func *prepare_buf; DMAu32Func *commit_buf; DMAIntFunc *rw_buf; + DMAVoidFunc *restart; DMAVoidFunc *restart_dma; DMAStopFunc *set_inactive; DMAVoidFunc *cmd_done; @@ -499,6 +500,7 @@ struct IDEDevice { #define IDE_RETRY_READ 0x20 #define IDE_RETRY_FLUSH 0x40 #define IDE_RETRY_TRIM 0x80 +#define IDE_RETRY_HBA 0x100 static inline IDEState *idebus_active_if(IDEBus *bus) { -- cgit 1.4.1 From 9364384de0e3b8a5bdea67ba49bee9ea7f1b8f26 Mon Sep 17 00:00:00 2001 From: John Snow Date: Sat, 4 Jul 2015 02:06:05 -0400 Subject: ahci: correct types in NCQTransferState Signed-off-by: John Snow Reviewed-by: Stefan Hajnoczi Message-id: 1435767578-32743-8-git-send-email-jsnow@redhat.com --- hw/ide/ahci.c | 10 +++++----- hw/ide/ahci.h | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) (limited to 'hw/ide/ahci.c') diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index d996f37b36..efd07ac804 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -45,7 +45,7 @@ do { \ } while (0) static void check_cmd(AHCIState *s, int port); -static int handle_cmd(AHCIState *s,int port,int slot); +static int handle_cmd(AHCIState *s, int port, uint8_t slot); static void ahci_reset_port(AHCIState *s, int port); static void ahci_write_fis_d2h(AHCIDevice *ad, uint8_t *cmd_fis); static void ahci_init_d2h(AHCIDevice *ad); @@ -506,7 +506,7 @@ static void ahci_reg_init(AHCIState *s) static void check_cmd(AHCIState *s, int port) { AHCIPortRegs *pr = &s->dev[port].port_regs; - int slot; + uint8_t slot; if ((pr->cmd & PORT_CMD_START) && pr->cmd_issue) { for (slot = 0; (slot < 32) && pr->cmd_issue; slot++) { @@ -1039,7 +1039,7 @@ static void execute_ncq_command(NCQTransferState *ncq_tfs) static void process_ncq_command(AHCIState *s, int port, uint8_t *cmd_fis, - int slot) + uint8_t slot) { AHCIDevice *ad = &s->dev[port]; IDEState *ide_state = &ad->port.ifs[0]; @@ -1114,7 +1114,7 @@ static void process_ncq_command(AHCIState *s, int port, uint8_t *cmd_fis, } static void handle_reg_h2d_fis(AHCIState *s, int port, - int slot, uint8_t *cmd_fis) + uint8_t slot, uint8_t *cmd_fis) { IDEState *ide_state = &s->dev[port].port.ifs[0]; AHCICmdHdr *cmd = s->dev[port].cur_cmd; @@ -1198,7 +1198,7 @@ static void handle_reg_h2d_fis(AHCIState *s, int port, ide_exec_cmd(&s->dev[port].port, cmd_fis[2]); } -static int handle_cmd(AHCIState *s, int port, int slot) +static int handle_cmd(AHCIState *s, int port, uint8_t slot) { IDEState *ide_state; uint64_t tbl_addr; diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h index 47a3122270..c728e3a07d 100644 --- a/hw/ide/ahci.h +++ b/hw/ide/ahci.h @@ -260,8 +260,8 @@ typedef struct NCQTransferState { uint64_t lba; uint8_t tag; uint8_t cmd; - int slot; - int used; + uint8_t slot; + bool used; bool halt; } NCQTransferState; -- cgit 1.4.1 From e08a98357b5811e7933ff077f6da4b85175caf8a Mon Sep 17 00:00:00 2001 From: John Snow Date: Sat, 4 Jul 2015 02:06:05 -0400 Subject: ahci: correct ncq sector count uint16_t isn't enough to hold the real sector count, since a value of zero implies a full 64K sectors, so we need a uint32_t here. We *could* cheat and pretend that this value is 0-based and fit it in a uint16_t, but I'd rather waste 2 bytes instead of a future dev's 10 minutes when they forget to +1/-1 accordingly somewhere. See SATA 3.2, section 13.6.4.1 "READ FPDMA QUEUED". Signed-off-by: John Snow Reviewed-by: Stefan Hajnoczi Message-id: 1435767578-32743-9-git-send-email-jsnow@redhat.com --- hw/ide/ahci.c | 7 +++++-- hw/ide/ahci.h | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) (limited to 'hw/ide/ahci.c') diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index efd07ac804..1027a60a9b 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -1086,8 +1086,11 @@ static void process_ncq_command(AHCIState *s, int port, uint8_t *cmd_fis, DPRINTF(port, "Warn: Unsupported attempt to use Rebuild Assist\n"); } - ncq_tfs->sector_count = ((uint16_t)ncq_fis->sector_count_high << 8) | - ncq_fis->sector_count_low; + ncq_tfs->sector_count = ((ncq_fis->sector_count_high << 8) | + ncq_fis->sector_count_low); + if (!ncq_tfs->sector_count) { + ncq_tfs->sector_count = 0x10000; + } size = ncq_tfs->sector_count * 512; ahci_populate_sglist(ad, &ncq_tfs->sglist, size, 0); diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h index c728e3a07d..9090d3d882 100644 --- a/hw/ide/ahci.h +++ b/hw/ide/ahci.h @@ -256,7 +256,7 @@ typedef struct NCQTransferState { BlockAIOCB *aiocb; QEMUSGList sglist; BlockAcctCookie acct; - uint16_t sector_count; + uint32_t sector_count; uint64_t lba; uint8_t tag; uint8_t cmd; -- cgit 1.4.1 From c82bd3c893825fc76af3634f5461f5eabd94e9df Mon Sep 17 00:00:00 2001 From: John Snow Date: Sat, 4 Jul 2015 02:06:05 -0400 Subject: ahci: add cmd header to ncq transfer state While the rest of the AHCI device can rely on a single bookmarked pointer for the AHCI Command Header currently being processed, NCQ is asynchronous and may have many commands in flight simultaneously. Add a cmdh pointer to the ncq_tfs object and make the sglist prepare function take an AHCICmdHeader pointer so we can be explicit about where we'd like to build SGlists from. Signed-off-by: John Snow Reviewed-by: Stefan Hajnoczi Message-id: 1435767578-32743-11-git-send-email-jsnow@redhat.com --- hw/ide/ahci.c | 11 ++++++----- hw/ide/ahci.h | 1 + 2 files changed, 7 insertions(+), 5 deletions(-) (limited to 'hw/ide/ahci.c') diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index 1027a60a9b..b77512b0a0 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -833,9 +833,8 @@ static int prdt_tbl_entry_size(const AHCI_SG *tbl) } static int ahci_populate_sglist(AHCIDevice *ad, QEMUSGList *sglist, - int64_t limit, int32_t offset) + AHCICmdHdr *cmd, int64_t limit, int32_t offset) { - AHCICmdHdr *cmd = ad->cur_cmd; uint16_t opts = le16_to_cpu(cmd->opts); uint16_t prdtl = le16_to_cpu(cmd->prdtl); uint64_t cfis_addr = le64_to_cpu(cmd->tbl_addr); @@ -1058,6 +1057,7 @@ static void process_ncq_command(AHCIState *s, int port, uint8_t *cmd_fis, ncq_tfs->used = 1; ncq_tfs->drive = ad; ncq_tfs->slot = slot; + ncq_tfs->cmdh = &((AHCICmdHdr *)ad->lst)[slot]; ncq_tfs->cmd = ncq_fis->command; ncq_tfs->lba = ((uint64_t)ncq_fis->lba5 << 40) | ((uint64_t)ncq_fis->lba4 << 32) | @@ -1092,7 +1092,7 @@ static void process_ncq_command(AHCIState *s, int port, uint8_t *cmd_fis, ncq_tfs->sector_count = 0x10000; } size = ncq_tfs->sector_count * 512; - ahci_populate_sglist(ad, &ncq_tfs->sglist, size, 0); + ahci_populate_sglist(ad, &ncq_tfs->sglist, ncq_tfs->cmdh, size, 0); if (ncq_tfs->sglist.size < size) { error_report("ahci: PRDT length for NCQ command (0x%zx) " @@ -1362,7 +1362,8 @@ static int32_t ahci_dma_prepare_buf(IDEDMA *dma, int32_t limit) AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma); IDEState *s = &ad->port.ifs[0]; - if (ahci_populate_sglist(ad, &s->sg, limit, s->io_buffer_offset) == -1) { + if (ahci_populate_sglist(ad, &s->sg, ad->cur_cmd, + limit, s->io_buffer_offset) == -1) { DPRINTF(ad->port_no, "ahci_dma_prepare_buf failed.\n"); return -1; } @@ -1397,7 +1398,7 @@ static int ahci_dma_rw_buf(IDEDMA *dma, int is_write) uint8_t *p = s->io_buffer + s->io_buffer_index; int l = s->io_buffer_size - s->io_buffer_index; - if (ahci_populate_sglist(ad, &s->sg, l, s->io_buffer_offset)) { + if (ahci_populate_sglist(ad, &s->sg, ad->cur_cmd, l, s->io_buffer_offset)) { return 0; } diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h index 9090d3d882..9f5b4d20b5 100644 --- a/hw/ide/ahci.h +++ b/hw/ide/ahci.h @@ -254,6 +254,7 @@ typedef struct AHCIDevice AHCIDevice; typedef struct NCQTransferState { AHCIDevice *drive; BlockAIOCB *aiocb; + AHCICmdHdr *cmdh; QEMUSGList sglist; BlockAcctCookie acct; uint32_t sector_count; -- cgit 1.4.1 From ee364416c1b5ed1adc779ca7197451a131666236 Mon Sep 17 00:00:00 2001 From: John Snow Date: Sat, 4 Jul 2015 02:06:05 -0400 Subject: ahci: add get_cmd_header helper cur_cmd is an internal bookmark that points to the current AHCI Command Header being processed by the AHCI state machine. With NCQ needing to occasionally rely on some of the same AHCI helpers, we cannot use cur_cmd and will need to grab explicit pointers instead. In an attempt to begin relying on the cur_cmd pointer less, add a helper to let us specifically get the pointer to the command header of particular interest. Signed-off-by: John Snow Reviewed-by: Stefan Hajnoczi Message-id: 1435767578-32743-12-git-send-email-jsnow@redhat.com --- hw/ide/ahci.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) (limited to 'hw/ide/ahci.c') diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index b77512b0a0..13b0157cbe 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -1116,11 +1116,20 @@ static void process_ncq_command(AHCIState *s, int port, uint8_t *cmd_fis, execute_ncq_command(ncq_tfs); } +static AHCICmdHdr *get_cmd_header(AHCIState *s, uint8_t port, uint8_t slot) +{ + if (port >= s->ports || slot >= AHCI_MAX_CMDS) { + return NULL; + } + + return s->dev[port].lst ? &((AHCICmdHdr *)s->dev[port].lst)[slot] : NULL; +} + static void handle_reg_h2d_fis(AHCIState *s, int port, uint8_t slot, uint8_t *cmd_fis) { IDEState *ide_state = &s->dev[port].port.ifs[0]; - AHCICmdHdr *cmd = s->dev[port].cur_cmd; + AHCICmdHdr *cmd = get_cmd_header(s, port, slot); uint16_t opts = le16_to_cpu(cmd->opts); if (cmd_fis[1] & 0x0F) { @@ -1219,7 +1228,7 @@ static int handle_cmd(AHCIState *s, int port, uint8_t slot) DPRINTF(port, "error: lst not given but cmd handled"); return -1; } - cmd = &((AHCICmdHdr *)s->dev[port].lst)[slot]; + cmd = get_cmd_header(s, port, slot); /* remember current slot handle for later */ s->dev[port].cur_cmd = cmd; @@ -1572,7 +1581,7 @@ static int ahci_state_post_load(void *opaque, int version_id) if (ad->busy_slot < 0 || ad->busy_slot >= AHCI_MAX_CMDS) { return -1; } - ad->cur_cmd = &((AHCICmdHdr *)ad->lst)[ad->busy_slot]; + ad->cur_cmd = get_cmd_header(s, i, ad->busy_slot); } } -- cgit 1.4.1 From 684d50132fdd68f4c2cba9e65b50f9b8ef4c5164 Mon Sep 17 00:00:00 2001 From: John Snow Date: Sat, 4 Jul 2015 02:06:05 -0400 Subject: ahci: ncq migration Migrate the NCQ queue. This is solely for the benefit of halted commands, since anything else should have completed and had any relevant status flushed to the HBA registers already. Signed-off-by: John Snow Reviewed-by: Stefan Hajnoczi Message-id: 1435767578-32743-13-git-send-email-jsnow@redhat.com --- hw/ide/ahci.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-) (limited to 'hw/ide/ahci.c') diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index 13b0157cbe..a2620f6514 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -1521,6 +1521,21 @@ void ahci_reset(AHCIState *s) } } +static const VMStateDescription vmstate_ncq_tfs = { + .name = "ncq state", + .version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_UINT32(sector_count, NCQTransferState), + VMSTATE_UINT64(lba, NCQTransferState), + VMSTATE_UINT8(tag, NCQTransferState), + VMSTATE_UINT8(cmd, NCQTransferState), + VMSTATE_UINT8(slot, NCQTransferState), + VMSTATE_BOOL(used, NCQTransferState), + VMSTATE_BOOL(halt, NCQTransferState), + VMSTATE_END_OF_LIST() + }, +}; + static const VMStateDescription vmstate_ahci_device = { .name = "ahci port", .version_id = 1, @@ -1546,14 +1561,17 @@ static const VMStateDescription vmstate_ahci_device = { VMSTATE_BOOL(done_atapi_packet, AHCIDevice), VMSTATE_INT32(busy_slot, AHCIDevice), VMSTATE_BOOL(init_d2h_sent, AHCIDevice), + VMSTATE_STRUCT_ARRAY(ncq_tfs, AHCIDevice, AHCI_MAX_CMDS, + 1, vmstate_ncq_tfs, NCQTransferState), VMSTATE_END_OF_LIST() }, }; static int ahci_state_post_load(void *opaque, int version_id) { - int i; + int i, j; struct AHCIDevice *ad; + NCQTransferState *ncq_tfs; AHCIState *s = opaque; for (i = 0; i < s->ports; i++) { @@ -1565,6 +1583,37 @@ static int ahci_state_post_load(void *opaque, int version_id) return -1; } + for (j = 0; j < AHCI_MAX_CMDS; j++) { + ncq_tfs = &ad->ncq_tfs[j]; + ncq_tfs->drive = ad; + + if (ncq_tfs->used != ncq_tfs->halt) { + return -1; + } + if (!ncq_tfs->halt) { + continue; + } + if (!is_ncq(ncq_tfs->cmd)) { + return -1; + } + if (ncq_tfs->slot != ncq_tfs->tag) { + return -1; + } + /* If ncq_tfs->halt is justly set, the engine should be engaged, + * and the command list buffer should be mapped. */ + ncq_tfs->cmdh = get_cmd_header(s, i, ncq_tfs->slot); + if (!ncq_tfs->cmdh) { + return -1; + } + ahci_populate_sglist(ncq_tfs->drive, &ncq_tfs->sglist, + ncq_tfs->cmdh, ncq_tfs->sector_count * 512, + 0); + if (ncq_tfs->sector_count != ncq_tfs->sglist.size >> 9) { + return -1; + } + } + + /* * If an error is present, ad->busy_slot will be valid and not -1. * In this case, an operation is waiting to resume and will re-check -- cgit 1.4.1 From dd6282217d8fee36e3854eab2635bec9cc5d5236 Mon Sep 17 00:00:00 2001 From: John Snow Date: Sat, 4 Jul 2015 02:06:05 -0400 Subject: ahci: Do not map cmd_fis to generate response The Register D2H FIS should copy the current values of the registers instead of just parroting back the same values the guest sent back to it. In this case, the SECTOR COUNT variables are actually not generally meaningful in terms of standard commands (See ATA8-AC3 Section 9.2 Normal Outputs), so it actually probably doesn't matter what we put in here. Meanwhile, we do need to use the Register update FIS from the NCQ pathways (in error cases), so getting rid of references to cur_cmd here is a win for AHCI concurrency. Signed-off-by: John Snow Reviewed-by: Stefan Hajnoczi Message-id: 1435767578-32743-14-git-send-email-jsnow@redhat.com --- hw/ide/ahci.c | 50 +++++--------------------------------------------- 1 file changed, 5 insertions(+), 45 deletions(-) (limited to 'hw/ide/ahci.c') diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index a2620f6514..eadd8b32c6 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -701,35 +701,13 @@ static void ahci_write_fis_sdb(AHCIState *s, int port, uint32_t finished) static void ahci_write_fis_pio(AHCIDevice *ad, uint16_t len) { AHCIPortRegs *pr = &ad->port_regs; - uint8_t *pio_fis, *cmd_fis; - uint64_t tbl_addr; - dma_addr_t cmd_len = 0x80; + uint8_t *pio_fis; IDEState *s = &ad->port.ifs[0]; if (!ad->res_fis || !(pr->cmd & PORT_CMD_FIS_RX)) { return; } - /* map cmd_fis */ - tbl_addr = le64_to_cpu(ad->cur_cmd->tbl_addr); - cmd_fis = dma_memory_map(ad->hba->as, tbl_addr, &cmd_len, - DMA_DIRECTION_TO_DEVICE); - - if (cmd_fis == NULL) { - DPRINTF(ad->port_no, "dma_memory_map failed in ahci_write_fis_pio"); - ahci_trigger_irq(ad->hba, ad, PORT_IRQ_HBUS_ERR); - return; - } - - if (cmd_len != 0x80) { - DPRINTF(ad->port_no, - "dma_memory_map mapped too few bytes in ahci_write_fis_pio"); - dma_memory_unmap(ad->hba->as, cmd_fis, cmd_len, - DMA_DIRECTION_TO_DEVICE, cmd_len); - ahci_trigger_irq(ad->hba, ad, PORT_IRQ_HBUS_ERR); - return; - } - pio_fis = &ad->res_fis[RES_FIS_PSFIS]; pio_fis[0] = SATA_FIS_TYPE_PIO_SETUP; @@ -745,8 +723,8 @@ static void ahci_write_fis_pio(AHCIDevice *ad, uint16_t len) pio_fis[9] = s->hob_lcyl; pio_fis[10] = s->hob_hcyl; pio_fis[11] = 0; - pio_fis[12] = cmd_fis[12]; - pio_fis[13] = cmd_fis[13]; + pio_fis[12] = s->nsector & 0xFF; + pio_fis[13] = (s->nsector >> 8) & 0xFF; pio_fis[14] = 0; pio_fis[15] = s->status; pio_fis[16] = len & 255; @@ -763,9 +741,6 @@ static void ahci_write_fis_pio(AHCIDevice *ad, uint16_t len) } ahci_trigger_irq(ad->hba, ad, PORT_IRQ_PIOS_FIS); - - dma_memory_unmap(ad->hba->as, cmd_fis, cmd_len, - DMA_DIRECTION_TO_DEVICE, cmd_len); } static void ahci_write_fis_d2h(AHCIDevice *ad, uint8_t *cmd_fis) @@ -773,22 +748,12 @@ static void ahci_write_fis_d2h(AHCIDevice *ad, uint8_t *cmd_fis) AHCIPortRegs *pr = &ad->port_regs; uint8_t *d2h_fis; int i; - dma_addr_t cmd_len = 0x80; - int cmd_mapped = 0; IDEState *s = &ad->port.ifs[0]; if (!ad->res_fis || !(pr->cmd & PORT_CMD_FIS_RX)) { return; } - if (!cmd_fis) { - /* map cmd_fis */ - uint64_t tbl_addr = le64_to_cpu(ad->cur_cmd->tbl_addr); - cmd_fis = dma_memory_map(ad->hba->as, tbl_addr, &cmd_len, - DMA_DIRECTION_TO_DEVICE); - cmd_mapped = 1; - } - d2h_fis = &ad->res_fis[RES_FIS_RFIS]; d2h_fis[0] = SATA_FIS_TYPE_REGISTER_D2H; @@ -804,8 +769,8 @@ static void ahci_write_fis_d2h(AHCIDevice *ad, uint8_t *cmd_fis) d2h_fis[9] = s->hob_lcyl; d2h_fis[10] = s->hob_hcyl; d2h_fis[11] = 0; - d2h_fis[12] = cmd_fis[12]; - d2h_fis[13] = cmd_fis[13]; + d2h_fis[12] = s->nsector & 0xFF; + d2h_fis[13] = (s->nsector >> 8) & 0xFF; for (i = 14; i < 20; i++) { d2h_fis[i] = 0; } @@ -819,11 +784,6 @@ static void ahci_write_fis_d2h(AHCIDevice *ad, uint8_t *cmd_fis) } ahci_trigger_irq(ad->hba, ad, PORT_IRQ_D2H_REG_FIS); - - if (cmd_mapped) { - dma_memory_unmap(ad->hba->as, cmd_fis, cmd_len, - DMA_DIRECTION_TO_DEVICE, cmd_len); - } } static int prdt_tbl_entry_size(const AHCI_SG *tbl) -- cgit 1.4.1 From 7c649ac5b607e2339fb54fc0fc01311ba5eacadd Mon Sep 17 00:00:00 2001 From: John Snow Date: Sat, 4 Jul 2015 02:06:05 -0400 Subject: ahci: fix sdb fis semantics There are two things to fix here: The first one is subtle: the PxSACT register in the AHCI HBA has different semantics from the field it is shadowing, the ACT field in the Set Device Bits FIS. In the HBA register, PxSACT acts as a bitfield indicating outstanding NCQ commands where a set bit indicates a pending NCQ operation. The FIS field however operates as an RWC register update to PxSACT, where a set bit indicates a *successfully* completed command. Correct the FIS semantics. At the same time, move the "clear finished" action to the SDB FIS generation instead of the register read to mimick how the other shadow registers work, which always just report the last reported value from a FIS, and not the most current values which may not have been reported by a FIS yet. Lastly and more simply, SATA 3.2 section 13.6.4.2 (and later sections) all specify that the Interrupt bit for the SDB FIS should always be set to one for NCQ commands. That's currently the only time we generate this FIS, so set it on all the time. Signed-off-by: John Snow Reviewed-by: Stefan Hajnoczi Message-id: 1435767578-32743-16-git-send-email-jsnow@redhat.com --- hw/ide/ahci.c | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) (limited to 'hw/ide/ahci.c') diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index eadd8b32c6..bb6a92f7f4 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -106,8 +106,6 @@ static uint32_t ahci_port_read(AHCIState *s, int port, int offset) val = pr->scr_err; break; case PORT_SCR_ACT: - pr->scr_act &= ~s->dev[port].finished; - s->dev[port].finished = 0; val = pr->scr_act; break; case PORT_CMD_ISSUE: @@ -666,14 +664,14 @@ static void ahci_unmap_clb_address(AHCIDevice *ad) ad->lst = NULL; } -static void ahci_write_fis_sdb(AHCIState *s, int port, uint32_t finished) +static void ahci_write_fis_sdb(AHCIState *s, NCQTransferState *ncq_tfs) { - AHCIDevice *ad = &s->dev[port]; + AHCIDevice *ad = ncq_tfs->drive; AHCIPortRegs *pr = &ad->port_regs; IDEState *ide_state; SDBFIS *sdb_fis; - if (!s->dev[port].res_fis || + if (!ad->res_fis || !(pr->cmd & PORT_CMD_FIS_RX)) { return; } @@ -683,19 +681,23 @@ static void ahci_write_fis_sdb(AHCIState *s, int port, uint32_t finished) sdb_fis->type = SATA_FIS_TYPE_SDB; /* Interrupt pending & Notification bit */ - sdb_fis->flags = (ad->hba->control_regs.irqstatus ? (1 << 6) : 0); + sdb_fis->flags = 0x40; /* Interrupt bit, always 1 for NCQ */ sdb_fis->status = ide_state->status & 0x77; sdb_fis->error = ide_state->error; /* update SAct field in SDB_FIS */ - s->dev[port].finished |= finished; sdb_fis->payload = cpu_to_le32(ad->finished); /* Update shadow registers (except BSY 0x80 and DRQ 0x08) */ pr->tfdata = (ad->port.ifs[0].error << 8) | (ad->port.ifs[0].status & 0x77) | (pr->tfdata & 0x88); + pr->scr_act &= ~ad->finished; + ad->finished = 0; - ahci_trigger_irq(s, ad, PORT_IRQ_SDB_FIS); + /* Trigger IRQ if interrupt bit is set (which currently, it always is) */ + if (sdb_fis->flags & 0x40) { + ahci_trigger_irq(s, ad, PORT_IRQ_SDB_FIS); + } } static void ahci_write_fis_pio(AHCIDevice *ad, uint16_t len) @@ -895,11 +897,14 @@ static void ncq_err(NCQTransferState *ncq_tfs) static void ncq_finish(NCQTransferState *ncq_tfs) { - /* Clear bit for this tag in SActive */ - ncq_tfs->drive->port_regs.scr_act &= ~(1 << ncq_tfs->tag); + /* If we didn't error out, set our finished bit. Errored commands + * do not get a bit set for the SDB FIS ACT register, nor do they + * clear the outstanding bit in scr_act (PxSACT). */ + if (!(ncq_tfs->drive->port_regs.scr_err & (1 << ncq_tfs->tag))) { + ncq_tfs->drive->finished |= (1 << ncq_tfs->tag); + } - ahci_write_fis_sdb(ncq_tfs->drive->hba, ncq_tfs->drive->port_no, - (1 << ncq_tfs->tag)); + ahci_write_fis_sdb(ncq_tfs->drive->hba, ncq_tfs); DPRINTF(ncq_tfs->drive->port_no, "NCQ transfer tag %d finished\n", ncq_tfs->tag); -- cgit 1.4.1