aboutsummaryrefslogtreecommitdiff
path: root/hw/ide
diff options
context:
space:
mode:
authorPavel Butsykin <pbutsykin@virtuozzo.com>2016-04-12 18:48:15 -0400
committerJohn Snow <jsnow@redhat.com>2016-04-12 18:48:15 -0400
commit502356eeeb5fd2bdd92b2d5156e511626c1c3814 (patch)
tree8aed873d0e5859db34e6a8d335367e26f15efb4a /hw/ide
parent9a41826f3843bf11bf8f52f176071f8c1fb0aa7e (diff)
downloadqemu-502356eeeb5fd2bdd92b2d5156e511626c1c3814.zip
qemu-502356eeeb5fd2bdd92b2d5156e511626c1c3814.tar.gz
qemu-502356eeeb5fd2bdd92b2d5156e511626c1c3814.tar.bz2
ide: really restart pending and in-flight atapi dma
Restart of ATAPI DMA used to be unreachable, because the request to do so wasn't indicated in bus->error_status due to the lack of spare bits, and ide_restart_bh() would return early doing nothing. This patch makes use of the observation that not all bit combinations were possible in ->error_status. In particular, IDE_RETRY_READ only made sense together with IDE_RETRY_DMA or IDE_RETRY_PIO. This allows to re-use IDE_RETRY_READ alone as an indicator of ATAPI DMA restart request. To makes things more uniform, ATAPI DMA gets its own value for ->dma_cmd. As a means against confusion, macros are added to test the state of ->error_status. The patch fixes the restart of both in-flight and pending ATAPI DMA, following the scheme similar to that of IDE DMA. [Including a fixup patch: Message-id: 1460465594-15777-1-git-send-email-pbutsykin@virtuozzo.com --js] Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com> Signed-off-by: Denis V. Lunev <den@openvz.org> Reviewed-by: Roman Kagan <rkagan@virtuozzo.com> Reviewed-by: John Snow <jsnow@redhat.com> Message-id: 1459924806-306-4-git-send-email-den@openvz.org Signed-off-by: John Snow <jsnow@redhat.com>
Diffstat (limited to 'hw/ide')
-rw-r--r--hw/ide/atapi.c13
-rw-r--r--hw/ide/core.c30
-rw-r--r--hw/ide/internal.h20
-rw-r--r--hw/ide/macio.c2
4 files changed, 43 insertions, 22 deletions
diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index acc52cd..2bb606c 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -375,15 +375,18 @@ static void ide_atapi_cmd_check_status(IDEState *s)
}
/* ATAPI DMA support */
-/* XXX: handle read errors */
static void ide_atapi_cmd_read_dma_cb(void *opaque, int ret)
{
IDEState *s = opaque;
int data_offset, n;
if (ret < 0) {
- ide_atapi_io_error(s, ret);
- goto eot;
+ if (ide_handle_rw_error(s, -ret, ide_dma_cmd_to_retry(s->dma_cmd))) {
+ if (s->bus->error_status) {
+ return;
+ }
+ goto eot;
+ }
}
if (s->io_buffer_size > 0) {
@@ -481,10 +484,6 @@ static void ide_atapi_cmd_read(IDEState *s, int lba, int nb_sectors,
}
}
-
-/* Called by *_restart_bh when the transfer function points
- * to ide_atapi_cmd
- */
void ide_atapi_dma_restart(IDEState *s)
{
/*
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 58d0687..41e6a2d 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -57,7 +57,6 @@ static const int smart_attributes[][12] = {
{ 190, 0x03, 0x00, 0x45, 0x45, 0x1f, 0x00, 0x1f, 0x1f, 0x00, 0x00, 0x32},
};
-static int ide_handle_rw_error(IDEState *s, int error, int op);
static void ide_dummy_transfer_stop(IDEState *s);
static void padstr(char *str, const char *src, int len)
@@ -773,7 +772,7 @@ void ide_dma_error(IDEState *s)
ide_set_irq(s->bus);
}
-static int ide_handle_rw_error(IDEState *s, int error, int op)
+int ide_handle_rw_error(IDEState *s, int error, int op)
{
bool is_read = (op & IDE_RETRY_READ) != 0;
BlockErrorAction action = blk_get_error_action(s->blk, is_read, error);
@@ -783,8 +782,10 @@ static int ide_handle_rw_error(IDEState *s, int error, int op)
s->bus->error_status = op;
} else if (action == BLOCK_ERROR_ACTION_REPORT) {
block_acct_failed(blk_get_stats(s->blk), &s->acct);
- if (op & IDE_RETRY_DMA) {
+ if (IS_IDE_RETRY_DMA(op)) {
ide_dma_error(s);
+ } else if (IS_IDE_RETRY_ATAPI(op)) {
+ ide_atapi_io_error(s, -error);
} else {
ide_rw_error(s);
}
@@ -872,6 +873,8 @@ static void ide_dma_cb(void *opaque, int ret)
ide_issue_trim, ide_dma_cb, s,
DMA_DIRECTION_TO_DEVICE);
break;
+ default:
+ abort();
}
return;
@@ -1634,6 +1637,9 @@ static bool cmd_packet(IDEState *s, uint8_t cmd)
s->status = READY_STAT | SEEK_STAT;
s->atapi_dma = s->feature & 1;
+ if (s->atapi_dma) {
+ s->dma_cmd = IDE_DMA_ATAPI;
+ }
s->nsector = 1;
ide_transfer_start(s, s->io_buffer, ATAPI_PACKET_SIZE,
ide_atapi_cmd);
@@ -2518,15 +2524,13 @@ static void ide_restart_bh(void *opaque)
if (s->bus->dma->ops->restart) {
s->bus->dma->ops->restart(s->bus->dma);
}
- }
-
- if (error_status & IDE_RETRY_DMA) {
+ } else if (IS_IDE_RETRY_DMA(error_status)) {
if (error_status & IDE_RETRY_TRIM) {
ide_restart_dma(s, IDE_DMA_TRIM);
} else {
ide_restart_dma(s, is_read ? IDE_DMA_READ : IDE_DMA_WRITE);
}
- } else if (error_status & IDE_RETRY_PIO) {
+ } else if (IS_IDE_RETRY_PIO(error_status)) {
if (is_read) {
ide_sector_read(s);
} else {
@@ -2534,15 +2538,11 @@ static void ide_restart_bh(void *opaque)
}
} else if (error_status & IDE_RETRY_FLUSH) {
ide_flush_cache(s);
+ } else if (IS_IDE_RETRY_ATAPI(error_status)) {
+ assert(s->end_transfer_func == ide_atapi_cmd);
+ ide_atapi_dma_restart(s);
} else {
- /*
- * We've not got any bits to tell us about ATAPI - but
- * we do have the end_transfer_func that tells us what
- * we're trying to do.
- */
- if (s->end_transfer_func == ide_atapi_cmd) {
- ide_atapi_dma_restart(s);
- }
+ abort();
}
}
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 68c7d0d..d2c458f 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -338,6 +338,7 @@ enum ide_dma_cmd {
IDE_DMA_READ,
IDE_DMA_WRITE,
IDE_DMA_TRIM,
+ IDE_DMA_ATAPI,
};
#define ide_cmd_is_read(s) \
@@ -506,13 +507,28 @@ struct IDEDevice {
};
/* These are used for the error_status field of IDEBus */
+#define IDE_RETRY_MASK 0xf8
#define IDE_RETRY_DMA 0x08
#define IDE_RETRY_PIO 0x10
+#define IDE_RETRY_ATAPI 0x20 /* reused IDE_RETRY_READ bit */
#define IDE_RETRY_READ 0x20
#define IDE_RETRY_FLUSH 0x40
#define IDE_RETRY_TRIM 0x80
#define IDE_RETRY_HBA 0x100
+#define IS_IDE_RETRY_DMA(_status) \
+ ((_status) & IDE_RETRY_DMA)
+
+#define IS_IDE_RETRY_PIO(_status) \
+ ((_status) & IDE_RETRY_PIO)
+
+/*
+ * The method of the IDE_RETRY_ATAPI determination is to use a previously
+ * impossible bit combination as a new status value.
+ */
+#define IS_IDE_RETRY_ATAPI(_status) \
+ (((_status) & IDE_RETRY_MASK) == IDE_RETRY_ATAPI)
+
static inline uint8_t ide_dma_cmd_to_retry(uint8_t dma_cmd)
{
switch (dma_cmd) {
@@ -522,6 +538,8 @@ static inline uint8_t ide_dma_cmd_to_retry(uint8_t dma_cmd)
return IDE_RETRY_DMA;
case IDE_DMA_TRIM:
return IDE_RETRY_DMA | IDE_RETRY_TRIM;
+ case IDE_DMA_ATAPI:
+ return IDE_RETRY_ATAPI;
default:
break;
}
@@ -612,4 +630,6 @@ void ide_bus_new(IDEBus *idebus, size_t idebus_size, DeviceState *dev,
int bus_id, int max_units);
IDEDevice *ide_create_drive(IDEBus *bus, int unit, DriveInfo *drive);
+int ide_handle_rw_error(IDEState *s, int error, int op);
+
#endif /* HW_IDE_INTERNAL_H */
diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index 1725e5b..76256eb 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -346,6 +346,8 @@ static void pmac_ide_transfer_cb(void *opaque, int ret)
case IDE_DMA_TRIM:
pmac_dma_trim(s->blk, offset, io->len, pmac_ide_transfer_cb, io);
break;
+ default:
+ abort();
}
return;