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 (PxFB) and Command List buffer (PxCLB). Because PCI bus mastering is disabled, DMA access to guest memory is blocked. Consequently, the memory mapping requests fail or incorrectly route through bounce buffers instead of direct guest memory, causing the destination QEMU process to abort the migration.
Starting DMA/FIS engines when bus mastering is disabled might lead to buffer mapping failures or improper bounce buffer allocations. Do not start the engines in the aforementioned scenario. When the guest re-enables bus mastering to access an idle drive, it should reset the port engine, safely allocating the buffers and starting the engines. 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]> --- 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
