When a Windows guest puts an idle SATA drive into standby, it disables
PCI bus mastering on the AHCI controller but fails to clear the port
engine bits in the port command register. During post-migration
handling, QEMU's ahci_state_post_load function checks only these port
engine bits to determine whether to start the engines and map the FIS
receive buffer and Command List buffer.

Because PCI bus mastering is disabled, DMA access to guest memory is
blocked. The consequences of this allocation depend on the bounce
buffer budget introduced in commit 637b0aa139 ("softmmu: Support
concurrent bounce buffers"), where each PCI device receives a 4 KiB
bounce buffer limit:

1. Silent Guest Memory Corruption (Budget Not Exceeded):
   To start the engine, QEMU allocates 1024 bytes for the command list
   and 256 bytes for the FIS receive buffer (1280 bytes total). With
   one or two active drives, mapping requests succeed by allocating
   private, zero-filled bounce buffers, allowing migration to
   complete cleanly. However, when the guest later resumes the drive
   and stops the port engines, unmapping writes those buffers
   back to the guest memory regions referenced by PxCLB and PxFB
   with zeroes. This leads to silent guest memory corruption.

2. Migration Failure (Budget Exceeded):
   When enough ports are active the 4 KiB bounce buffer budget is
   exceeded. The mapping call fails (e.g., "bad FIS receive
   buffer address") and the destination QEMU aborts migration.

To resolve this issue, skip port engine startup in
ahci_state_post_load() if PCI bus mastering is disabled. When Windows
10 re-enables bus mastering to access the idle drive, traces
show that it explicitly restores the state: it sets the Bus Master bit,
rewrites PxCLB and PxFB, toggles PxCMD to restart the engine, and issues
a COMRESET. Windows 7 does not hit this path because it does not disable
bus mastering during standby.

Reported-by: andychiu <[email protected]>
Suggested-by: Michael S. Tsirkin <[email protected]>
Suggested-by: John Snow <[email protected]>
Link: https://lists.gnu.org/archive/html/qemu-devel/2019-09/msg01544.html
Signed-off-by: Abhisek Panda <[email protected]>
Tested-by: Denis V. Lunev <[email protected]>
Reviewed-by: Denis V. Lunev <[email protected]>
---
 hw/ide/ahci.c         | 17 ++++++++++++++---
 hw/ide/ich.c          |  8 ++++++++
 include/hw/ide/ahci.h |  1 +
 3 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 86f6297dc9..5f842eff13 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -172,6 +172,11 @@ static uint32_t ahci_port_read(AHCIState *s, int port, int 
offset)
     return val;
 }
 
+static inline int ahci_bus_master_enabled(AHCIState *s)
+{
+    return !s->bus_master_enabled || s->bus_master_enabled(s);
+}
+
 static void ahci_check_irq(AHCIState *s)
 {
     int i;
@@ -1625,6 +1630,8 @@ static const IDEDMAOps ahci_dma_ops = {
 
 void ahci_init(AHCIState *s, DeviceState *qdev)
 {
+    s->bus_master_enabled = NULL;
+
     /* XXX BAR size should be 1k, but that breaks, so bump it to 4k for now */
     memory_region_init_io(&s->mem, OBJECT(qdev), &ahci_mem_ops, s,
                           "ahci", AHCI_MEM_BAR_SIZE);
@@ -1792,10 +1799,14 @@ static int ahci_state_post_load(void *opaque, int 
version_id)
             return -1;
         }
 
-        /* After a migrate, the DMA/FIS engines are "off" and
-         * need to be conditionally restarted */
+        /*
+         * After a migrate, the DMA/FIS engines are "off" and need to be
+         * conditionally restarted. If PCI bus mastering is disabled, leave
+         * the engines stopped because DMA is inactive.
+         */
         pr->cmd &= ~(PORT_CMD_LIST_ON | PORT_CMD_FIS_ON);
-        if (ahci_cond_start_engines(ad) != 0) {
+        if (ahci_bus_master_enabled(s) &&
+            ahci_cond_start_engines(ad) != 0) {
             return -1;
         }
 
diff --git a/hw/ide/ich.c b/hw/ide/ich.c
index b00987f08d..8e8c2e108d 100644
--- a/hw/ide/ich.c
+++ b/hw/ide/ich.c
@@ -110,6 +110,13 @@ static void pci_ich9_reset(DeviceState *dev)
     ahci_reset(&d->ahci);
 }
 
+static bool pci_ich9_ahci_bus_master_enabled(AHCIState *s)
+{
+    AHCIPCIState *d = container_of(s, AHCIPCIState, ahci);
+
+    return d->parent_obj.is_master;
+}
+
 static void pci_ich9_ahci_init(Object *obj)
 {
     AHCIPCIState *d = ICH9_AHCI(obj);
@@ -117,6 +124,7 @@ static void pci_ich9_ahci_init(Object *obj)
     qemu_init_irq_child(obj, "update-irq", &d->irq,
                         pci_ich9_ahci_update_irq, d, 0);
     ahci_init(&d->ahci, DEVICE(obj));
+    d->ahci.bus_master_enabled = pci_ich9_ahci_bus_master_enabled;
     d->ahci.irq = &d->irq;
 }
 
diff --git a/include/hw/ide/ahci.h b/include/hw/ide/ahci.h
index cd07b87811..434a6457b5 100644
--- a/include/hw/ide/ahci.h
+++ b/include/hw/ide/ahci.h
@@ -46,6 +46,7 @@ typedef struct AHCIState {
     uint32_t ports;
     qemu_irq irq;
     AddressSpace *as;
+    bool (*bus_master_enabled)(struct AHCIState *s);
 } AHCIState;
 
 
-- 
2.43.7


Reply via email to