ide: Split error status from status register

When adding the werror=stop mode, some flags were added to s->status
which are used to determine what kind of operation should be restarted
when the VM is continued.

Unfortunately, it turns out that s->status is in fact a device register
and as such is visible to the guest (some of the abused bits are even
writable for the guest).

For migration we keep on using the old VMState field (renamed to
migration_compat_status) if the status register doesn't use any of the
previously abused bits. If it does, we use a subsection with a clean copy of
the status register.

The error status is always sent in a subsection if there is any error. It can't
use the old field because errors happen even without PCI.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 95beb175b327bd9d1b498f01b9b322366e87f743..da250ac39bdd76044a43d71bdd5f8766646440f1 100644 (file)
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -446,7 +446,7 @@ static int ide_handle_rw_error(IDEState *s, int error, int op)
     if ((error == ENOSPC && action == BLOCK_ERR_STOP_ENOSPC)
             || action == BLOCK_ERR_STOP_ANY) {
         s->bus->dma->ops->set_unit(s->bus->dma, s->unit);
-        s->bus->dma->ops->add_status(s->bus->dma, op);
+        s->bus->error_status = op;
         bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read);
         vm_stop(VMSTOP_DISKFULL);
     } else {
@@ -1847,6 +1847,13 @@ static bool ide_atapi_gesn_needed(void *opaque)
     return s->events.new_media || s->events.eject_request;
 }
 
+static bool ide_error_needed(void *opaque)
+{
+    IDEBus *bus = opaque;
+
+    return (bus->error_status != 0);
+}
+
 /* Fields for GET_EVENT_STATUS_NOTIFICATION ATAPI command */
 const VMStateDescription vmstate_ide_atapi_gesn_state = {
     .name ="ide_drive/atapi/gesn_state",
@@ -1921,6 +1928,17 @@ const VMStateDescription vmstate_ide_drive = {
     }
 };
 
+const VMStateDescription vmstate_ide_error_status = {
+    .name ="ide_bus/error",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields = (VMStateField []) {
+        VMSTATE_INT32(error_status, IDEBus),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 const VMStateDescription vmstate_ide_bus = {
     .name = "ide_bus",
     .version_id = 1,
@@ -1930,6 +1948,14 @@ const VMStateDescription vmstate_ide_bus = {
         VMSTATE_UINT8(cmd, IDEBus),
         VMSTATE_UINT8(unit, IDEBus),
         VMSTATE_END_OF_LIST()
+    },
+    .subsections = (VMStateSubsection []) {
+        {
+            .vmsd = &vmstate_ide_error_status,
+            .needed = ide_error_needed,
+        }, {
+            /* empty */
+        }
     }
 };
 

diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index c2b35ec5e641d7d334f28115930972cb606ac354..8d18cc37334c36377a0bc191342a4529e2b4e84c 100644 (file)
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -486,6 +486,8 @@ struct IDEBus {
     uint8_t unit;
     uint8_t cmd;
     qemu_irq irq;
+
+    int error_status;
 };
 
 struct IDEDevice {
@@ -505,11 +507,17 @@ struct IDEDeviceInfo {
 #define BM_STATUS_DMAING 0x01
 #define BM_STATUS_ERROR  0x02
 #define BM_STATUS_INT    0x04
+
+/* FIXME These are not status register bits */
 #define BM_STATUS_DMA_RETRY  0x08
 #define BM_STATUS_PIO_RETRY  0x10
 #define BM_STATUS_RETRY_READ  0x20
 #define BM_STATUS_RETRY_FLUSH 0x40
 
+#define BM_MIGRATION_COMPAT_STATUS_BITS \
+        (BM_STATUS_DMA_RETRY | BM_STATUS_PIO_RETRY | \
+        BM_STATUS_RETRY_READ | BM_STATUS_RETRY_FLUSH)
+
 #define BM_CMD_START     0x01
 #define BM_CMD_READ      0x08
 

diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index a4726adbeac209fc8bf8548e48c06d74d90fdcfa..7fa32bdff1629a630530b2c2b3f97b85f56c9af3 100644 (file)
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -183,27 +183,33 @@ static void bmdma_restart_dma(BMDMAState *bm, int is_read)
     bmdma_start_dma(&bm->dma, s, bm->dma_cb);
 }
 
+/* TODO This should be common IDE code */
 static void bmdma_restart_bh(void *opaque)
 {
     BMDMAState *bm = opaque;
+    IDEBus *bus = bm->bus;
     int is_read;
 
     qemu_bh_delete(bm->bh);
     bm->bh = NULL;
 
-    is_read = !!(bm->status & BM_STATUS_RETRY_READ);
+    if (bm->unit == (uint8_t) -1) {
+        return;
+    }
 
-    if (bm->status & BM_STATUS_DMA_RETRY) {
-        bm->status &= ~(BM_STATUS_DMA_RETRY | BM_STATUS_RETRY_READ);
+    is_read = !!(bus->error_status & BM_STATUS_RETRY_READ);
+
+    if (bus->error_status & BM_STATUS_DMA_RETRY) {
+        bus->error_status &= ~(BM_STATUS_DMA_RETRY | BM_STATUS_RETRY_READ);
         bmdma_restart_dma(bm, is_read);
-    } else if (bm->status & BM_STATUS_PIO_RETRY) {
-        bm->status &= ~(BM_STATUS_PIO_RETRY | BM_STATUS_RETRY_READ);
+    } else if (bus->error_status & BM_STATUS_PIO_RETRY) {
+        bus->error_status &= ~(BM_STATUS_PIO_RETRY | BM_STATUS_RETRY_READ);
         if (is_read) {
             ide_sector_read(bmdma_active_if(bm));
         } else {
             ide_sector_write(bmdma_active_if(bm));
         }
-    } else if (bm->status & BM_STATUS_RETRY_FLUSH) {
+    } else if (bus->error_status & BM_STATUS_RETRY_FLUSH) {
         ide_flush_cache(bmdma_active_if(bm));
     }
 }
@@ -351,6 +357,43 @@ static bool ide_bmdma_current_needed(void *opaque)
     return (bm->cur_prd_len != 0);
 }
 
+static bool ide_bmdma_status_needed(void *opaque)
+{
+    BMDMAState *bm = opaque;
+
+    /* Older versions abused some bits in the status register for internal
+     * error state. If any of these bits are set, we must add a subsection to
+     * transfer the real status register */
+    uint8_t abused_bits = BM_MIGRATION_COMPAT_STATUS_BITS;
+
+    return ((bm->status & abused_bits) != 0);
+}
+
+static void ide_bmdma_pre_save(void *opaque)
+{
+    BMDMAState *bm = opaque;
+    uint8_t abused_bits = BM_MIGRATION_COMPAT_STATUS_BITS;
+
+    bm->migration_compat_status =
+        (bm->status & ~abused_bits) | (bm->bus->error_status & abused_bits);
+}
+
+/* This function accesses bm->bus->error_status which is loaded only after
+ * BMDMA itself. This is why the function is called from ide_pci_post_load
+ * instead of being registered with VMState where it would run too early. */
+static int ide_bmdma_post_load(void *opaque, int version_id)
+{
+    BMDMAState *bm = opaque;
+    uint8_t abused_bits = BM_MIGRATION_COMPAT_STATUS_BITS;
+
+    if (bm->status == 0) {
+        bm->status = bm->migration_compat_status & ~abused_bits;
+        bm->bus->error_status |= bm->migration_compat_status & abused_bits;
+    }
+
+    return 0;
+}
+
 static const VMStateDescription vmstate_bmdma_current = {
     .name = "ide bmdma_current",
     .version_id = 1,
@@ -365,15 +408,26 @@ static const VMStateDescription vmstate_bmdma_current = {
     }
 };
 
+const VMStateDescription vmstate_bmdma_status = {
+    .name ="ide bmdma/status",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields = (VMStateField []) {
+        VMSTATE_UINT8(status, BMDMAState),
+        VMSTATE_END_OF_LIST()
+    }
+};
 
 static const VMStateDescription vmstate_bmdma = {
     .name = "ide bmdma",
     .version_id = 3,
     .minimum_version_id = 0,
     .minimum_version_id_old = 0,
+    .pre_save  = ide_bmdma_pre_save,
     .fields      = (VMStateField []) {
         VMSTATE_UINT8(cmd, BMDMAState),
-        VMSTATE_UINT8(status, BMDMAState),
+        VMSTATE_UINT8(migration_compat_status, BMDMAState),
         VMSTATE_UINT32(addr, BMDMAState),
         VMSTATE_INT64(sector_num, BMDMAState),
         VMSTATE_UINT32(nsector, BMDMAState),
@@ -384,6 +438,9 @@ static const VMStateDescription vmstate_bmdma = {
         {
             .vmsd = &vmstate_bmdma_current,
             .needed = ide_bmdma_current_needed,
+        }, {
+            .vmsd = &vmstate_bmdma_status,
+            .needed = ide_bmdma_status_needed,
         }, {
             /* empty */
         }
@@ -399,7 +456,9 @@ static int ide_pci_post_load(void *opaque, int version_id)
         /* current versions always store 0/1, but older version
            stored bigger values. We only need last bit */
         d->bmdma[i].unit &= 1;
+        ide_bmdma_post_load(&d->bmdma[i], -1);
     }
+
     return 0;
 }
 

diff --git a/hw/ide/pci.h b/hw/ide/pci.h
index cd72cbaeb963ea64bc59c2517304bad70be2d1c6..b4f3691a5cd1c0977fca01af837c4d59225908eb 100644 (file)
--- a/hw/ide/pci.h
+++ b/hw/ide/pci.h
@@ -22,6 +22,10 @@ typedef struct BMDMAState {
     IORange addr_ioport;
     QEMUBH *bh;
     qemu_irq irq;
+
+    /* Bit 0-2 and 7:   BM status register
+     * Bit 3-6:         bus->error_status */
+    uint8_t migration_compat_status;
 } BMDMAState;
 
 typedef struct PCIIDEState {