* [Qemu-devel] [PATCH] exec: fix access to ram_list.dirty_memory when sync dirty bitmap @ 2017-06-28 2:43 Haozhong Zhang 2017-06-28 7:30 ` Juan Quintela 2017-06-28 9:09 ` Juan Quintela 0 siblings, 2 replies; 6+ messages in thread From: Haozhong Zhang @ 2017-06-28 2:43 UTC (permalink / raw) To: qemu-devel Cc: Paolo Bonzini, Stefan Hajnoczi, Juan Quintela, Xiao Guangrong, Haozhong Zhang In cpu_physical_memory_sync_dirty_bitmap(rb, start, ...), the 2nd argument 'start' is relative to the start of the ramblock 'rb'. When it's used to access the dirty memory bitmap of ram_list (i.e. ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION]->blocks[]), an offset to the start of all RAM (i.e. rb->offset) should be added to it, which has however been missed since c/s 6b6712efcc. For a ramblock of host memory backend whose offset is not zero, cpu_physical_memory_sync_dirty_bitmap() synchronizes the incorrect part of the dirty memory bitmap of ram_list to the per ramblock dirty bitmap. As a result, a guest with host memory backend may crash after migration. Fix it by adding the offset of ramblock when accessing the dirty memory bitmap of ram_list in cpu_physical_memory_sync_dirty_bitmap(). Reported-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> --- include/exec/ram_addr.h | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h index 73d1bea8b6..cbc797ed05 100644 --- a/include/exec/ram_addr.h +++ b/include/exec/ram_addr.h @@ -377,6 +377,7 @@ uint64_t cpu_physical_memory_sync_dirty_bitmap(RAMBlock *rb, uint64_t *real_dirty_pages) { ram_addr_t addr; + ram_addr_t offset = rb->offset; unsigned long page = BIT_WORD(start >> TARGET_PAGE_BITS); uint64_t num_dirty = 0; unsigned long *dest = rb->bmap; @@ -386,8 +387,9 @@ uint64_t cpu_physical_memory_sync_dirty_bitmap(RAMBlock *rb, int k; int nr = BITS_TO_LONGS(length >> TARGET_PAGE_BITS); unsigned long * const *src; - unsigned long idx = (page * BITS_PER_LONG) / DIRTY_MEMORY_BLOCK_SIZE; - unsigned long offset = BIT_WORD((page * BITS_PER_LONG) % + unsigned long word = BIT_WORD((start + offset) >> TARGET_PAGE_BITS); + unsigned long idx = (word * BITS_PER_LONG) / DIRTY_MEMORY_BLOCK_SIZE; + unsigned long offset = BIT_WORD((word * BITS_PER_LONG) % DIRTY_MEMORY_BLOCK_SIZE); rcu_read_lock(); @@ -416,7 +418,7 @@ uint64_t cpu_physical_memory_sync_dirty_bitmap(RAMBlock *rb, } else { for (addr = 0; addr < length; addr += TARGET_PAGE_SIZE) { if (cpu_physical_memory_test_and_clear_dirty( - start + addr, + start + addr + offset, TARGET_PAGE_SIZE, DIRTY_MEMORY_MIGRATION)) { *real_dirty_pages += 1; -- 2.11.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] exec: fix access to ram_list.dirty_memory when sync dirty bitmap 2017-06-28 2:43 [Qemu-devel] [PATCH] exec: fix access to ram_list.dirty_memory when sync dirty bitmap Haozhong Zhang @ 2017-06-28 7:30 ` Juan Quintela 2017-06-28 8:23 ` Paolo Bonzini 2017-06-28 9:09 ` Juan Quintela 1 sibling, 1 reply; 6+ messages in thread From: Juan Quintela @ 2017-06-28 7:30 UTC (permalink / raw) To: Haozhong Zhang; +Cc: qemu-devel, Paolo Bonzini, Stefan Hajnoczi, Xiao Guangrong Haozhong Zhang <haozhong.zhang@intel.com> wrote: > In cpu_physical_memory_sync_dirty_bitmap(rb, start, ...), the 2nd > argument 'start' is relative to the start of the ramblock 'rb'. When > it's used to access the dirty memory bitmap of ram_list (i.e. > ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION]->blocks[]), an offset to > the start of all RAM (i.e. rb->offset) should be added to it, which has > however been missed since c/s 6b6712efcc. For a ramblock of host memory > backend whose offset is not zero, cpu_physical_memory_sync_dirty_bitmap() > synchronizes the incorrect part of the dirty memory bitmap of ram_list > to the per ramblock dirty bitmap. As a result, a guest with host > memory backend may crash after migration. > > Fix it by adding the offset of ramblock when accessing the dirty memory > bitmap of ram_list in cpu_physical_memory_sync_dirty_bitmap(). > > Reported-by: Stefan Hajnoczi <stefanha@redhat.com> > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> Reviewed-by: Juan Quintela <quintela@redhat.com> As this function is only used on migration, should I integrate it on my next push, or do you want to pull it, Paolo? Later, Juan. > --- > include/exec/ram_addr.h | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h > index 73d1bea8b6..cbc797ed05 100644 > --- a/include/exec/ram_addr.h > +++ b/include/exec/ram_addr.h > @@ -377,6 +377,7 @@ uint64_t cpu_physical_memory_sync_dirty_bitmap(RAMBlock *rb, > uint64_t *real_dirty_pages) > { > ram_addr_t addr; > + ram_addr_t offset = rb->offset; > unsigned long page = BIT_WORD(start >> TARGET_PAGE_BITS); > uint64_t num_dirty = 0; > unsigned long *dest = rb->bmap; > @@ -386,8 +387,9 @@ uint64_t cpu_physical_memory_sync_dirty_bitmap(RAMBlock *rb, > int k; > int nr = BITS_TO_LONGS(length >> TARGET_PAGE_BITS); > unsigned long * const *src; > - unsigned long idx = (page * BITS_PER_LONG) / DIRTY_MEMORY_BLOCK_SIZE; > - unsigned long offset = BIT_WORD((page * BITS_PER_LONG) % > + unsigned long word = BIT_WORD((start + offset) >> TARGET_PAGE_BITS); > + unsigned long idx = (word * BITS_PER_LONG) / DIRTY_MEMORY_BLOCK_SIZE; > + unsigned long offset = BIT_WORD((word * BITS_PER_LONG) % > DIRTY_MEMORY_BLOCK_SIZE); > > rcu_read_lock(); > @@ -416,7 +418,7 @@ uint64_t cpu_physical_memory_sync_dirty_bitmap(RAMBlock *rb, > } else { > for (addr = 0; addr < length; addr += TARGET_PAGE_SIZE) { > if (cpu_physical_memory_test_and_clear_dirty( > - start + addr, > + start + addr + offset, > TARGET_PAGE_SIZE, > DIRTY_MEMORY_MIGRATION)) { > *real_dirty_pages += 1; ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] exec: fix access to ram_list.dirty_memory when sync dirty bitmap 2017-06-28 7:30 ` Juan Quintela @ 2017-06-28 8:23 ` Paolo Bonzini 0 siblings, 0 replies; 6+ messages in thread From: Paolo Bonzini @ 2017-06-28 8:23 UTC (permalink / raw) To: quintela, Haozhong Zhang; +Cc: qemu-devel, Stefan Hajnoczi, Xiao Guangrong On 28/06/2017 09:30, Juan Quintela wrote: > Haozhong Zhang <haozhong.zhang@intel.com> wrote: >> In cpu_physical_memory_sync_dirty_bitmap(rb, start, ...), the 2nd >> argument 'start' is relative to the start of the ramblock 'rb'. When >> it's used to access the dirty memory bitmap of ram_list (i.e. >> ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION]->blocks[]), an offset to >> the start of all RAM (i.e. rb->offset) should be added to it, which has >> however been missed since c/s 6b6712efcc. For a ramblock of host memory >> backend whose offset is not zero, cpu_physical_memory_sync_dirty_bitmap() >> synchronizes the incorrect part of the dirty memory bitmap of ram_list >> to the per ramblock dirty bitmap. As a result, a guest with host >> memory backend may crash after migration. >> >> Fix it by adding the offset of ramblock when accessing the dirty memory >> bitmap of ram_list in cpu_physical_memory_sync_dirty_bitmap(). >> >> Reported-by: Stefan Hajnoczi <stefanha@redhat.com> >> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> > > Reviewed-by: Juan Quintela <quintela@redhat.com> > > As this function is only used on migration, should I integrate it on my > next push, or do you want to pull it, Paolo? > > Later, Juan. > > >> --- >> include/exec/ram_addr.h | 8 +++++--- >> 1 file changed, 5 insertions(+), 3 deletions(-) >> >> diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h >> index 73d1bea8b6..cbc797ed05 100644 >> --- a/include/exec/ram_addr.h >> +++ b/include/exec/ram_addr.h >> @@ -377,6 +377,7 @@ uint64_t cpu_physical_memory_sync_dirty_bitmap(RAMBlock *rb, >> uint64_t *real_dirty_pages) >> { >> ram_addr_t addr; >> + ram_addr_t offset = rb->offset; >> unsigned long page = BIT_WORD(start >> TARGET_PAGE_BITS); >> uint64_t num_dirty = 0; >> unsigned long *dest = rb->bmap; >> @@ -386,8 +387,9 @@ uint64_t cpu_physical_memory_sync_dirty_bitmap(RAMBlock *rb, >> int k; >> int nr = BITS_TO_LONGS(length >> TARGET_PAGE_BITS); >> unsigned long * const *src; >> - unsigned long idx = (page * BITS_PER_LONG) / DIRTY_MEMORY_BLOCK_SIZE; >> - unsigned long offset = BIT_WORD((page * BITS_PER_LONG) % >> + unsigned long word = BIT_WORD((start + offset) >> TARGET_PAGE_BITS); >> + unsigned long idx = (word * BITS_PER_LONG) / DIRTY_MEMORY_BLOCK_SIZE; >> + unsigned long offset = BIT_WORD((word * BITS_PER_LONG) % >> DIRTY_MEMORY_BLOCK_SIZE); The shadowing between the two variables named offset is a bit ugly. Please use rb->offset in the initialization of "word", and declare... >> >> rcu_read_lock(); >> @@ -416,7 +418,7 @@ uint64_t cpu_physical_memory_sync_dirty_bitmap(RAMBlock *rb, >> } else { ... "ram_addr_t offset = offset" here. Paolo >> for (addr = 0; addr < length; addr += TARGET_PAGE_SIZE) { >> if (cpu_physical_memory_test_and_clear_dirty( >> - start + addr, >> + start + addr + offset, >> TARGET_PAGE_SIZE, >> DIRTY_MEMORY_MIGRATION)) { >> *real_dirty_pages += 1; ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] exec: fix access to ram_list.dirty_memory when sync dirty bitmap 2017-06-28 2:43 [Qemu-devel] [PATCH] exec: fix access to ram_list.dirty_memory when sync dirty bitmap Haozhong Zhang 2017-06-28 7:30 ` Juan Quintela @ 2017-06-28 9:09 ` Juan Quintela 2017-06-28 11:12 ` Haozhong Zhang 1 sibling, 1 reply; 6+ messages in thread From: Juan Quintela @ 2017-06-28 9:09 UTC (permalink / raw) To: Haozhong Zhang; +Cc: qemu-devel, Paolo Bonzini, Stefan Hajnoczi, Xiao Guangrong Haozhong Zhang <haozhong.zhang@intel.com> wrote: > In cpu_physical_memory_sync_dirty_bitmap(rb, start, ...), the 2nd > argument 'start' is relative to the start of the ramblock 'rb'. When > it's used to access the dirty memory bitmap of ram_list (i.e. > ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION]->blocks[]), an offset to > the start of all RAM (i.e. rb->offset) should be added to it, which has > however been missed since c/s 6b6712efcc. For a ramblock of host memory > backend whose offset is not zero, cpu_physical_memory_sync_dirty_bitmap() > synchronizes the incorrect part of the dirty memory bitmap of ram_list > to the per ramblock dirty bitmap. As a result, a guest with host > memory backend may crash after migration. > > Fix it by adding the offset of ramblock when accessing the dirty memory > bitmap of ram_list in cpu_physical_memory_sync_dirty_bitmap(). > > Reported-by: Stefan Hajnoczi <stefanha@redhat.com> > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> Hi I need to add this patch to make it compile for me with all architectures enabled. I am adding that to you patch, are you ok? Thanks, Juan. >From 74573a4190af8a357054493d5451e2b51c553498 Mon Sep 17 00:00:00 2001 From: Juan Quintela <quintela@redhat.com> Date: Wed, 28 Jun 2017 10:19:25 +0200 Subject: [PATCH] fix missing Macro applications Signed-off-by: Juan Quintela <quintela@redhat.com> --- hw/block/fdc.c | 2 +- hw/display/qxl.c | 4 ++-- hw/display/vga.c | 2 +- hw/display/vmware_vga.c | 2 +- hw/ide/ahci.c | 2 +- hw/input/vmmouse.c | 2 +- hw/intc/openpic.c | 2 +- hw/intc/xics.c | 2 +- hw/misc/max111x.c | 2 +- hw/ppc/spapr_iommu.c | 2 +- hw/ppc/spapr_pci.c | 4 ++-- hw/ppc/spapr_vio.c | 4 ++-- hw/usb/hcd-uhci.c | 2 +- target/ppc/machine.c | 8 ++++---- 14 files changed, 20 insertions(+), 20 deletions(-) diff --git a/hw/block/fdc.c b/hw/block/fdc.c index 28f6b6e..4011290 100644 --- a/hw/block/fdc.c +++ b/hw/block/fdc.c @@ -1217,7 +1217,7 @@ static const VMStateDescription vmstate_fdc = { VMSTATE_UINT8(config, FDCtrl), VMSTATE_UINT8(lock, FDCtrl), VMSTATE_UINT8(pwrd, FDCtrl), - VMSTATE_UINT8_EQUAL(num_floppies, FDCtrl), + VMSTATE_UINT8_EQUAL(num_floppies, FDCtrl, NULL), VMSTATE_STRUCT_ARRAY(drives, FDCtrl, MAX_FD, 1, vmstate_fdrive, FDrive), VMSTATE_END_OF_LIST() diff --git a/hw/display/qxl.c b/hw/display/qxl.c index ad09bb9..3c1688e 100644 --- a/hw/display/qxl.c +++ b/hw/display/qxl.c @@ -2373,12 +2373,12 @@ static VMStateDescription qxl_vmstate = { VMSTATE_UINT32(last_release_offset, PCIQXLDevice), VMSTATE_UINT32(mode, PCIQXLDevice), VMSTATE_UINT32(ssd.unique, PCIQXLDevice), - VMSTATE_INT32_EQUAL(num_memslots, PCIQXLDevice), + VMSTATE_INT32_EQUAL(num_memslots, PCIQXLDevice, NULL), VMSTATE_STRUCT_ARRAY(guest_slots, PCIQXLDevice, NUM_MEMSLOTS, 0, qxl_memslot, struct guest_slots), VMSTATE_STRUCT(guest_primary.surface, PCIQXLDevice, 0, qxl_surface, QXLSurfaceCreate), - VMSTATE_INT32_EQUAL(ssd.num_surfaces, PCIQXLDevice), + VMSTATE_INT32_EQUAL(ssd.num_surfaces, PCIQXLDevice, NULL), VMSTATE_VARRAY_INT32(guest_surfaces.cmds, PCIQXLDevice, ssd.num_surfaces, 0, vmstate_info_uint64, uint64_t), diff --git a/hw/display/vga.c b/hw/display/vga.c index dcc95f8..80508b8 100644 --- a/hw/display/vga.c +++ b/hw/display/vga.c @@ -2099,7 +2099,7 @@ const VMStateDescription vmstate_vga_common = { VMSTATE_BUFFER(palette, VGACommonState), VMSTATE_INT32(bank_offset, VGACommonState), - VMSTATE_UINT8_EQUAL(is_vbe_vmstate, VGACommonState), + VMSTATE_UINT8_EQUAL(is_vbe_vmstate, VGACommonState, NULL), VMSTATE_UINT16(vbe_index, VGACommonState), VMSTATE_UINT16_ARRAY(vbe_regs, VGACommonState, VBE_DISPI_INDEX_NB), VMSTATE_UINT32(vbe_start_addr, VGACommonState), diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c index ec5f27d..c989cef 100644 --- a/hw/display/vmware_vga.c +++ b/hw/display/vmware_vga.c @@ -1192,7 +1192,7 @@ static const VMStateDescription vmstate_vmware_vga_internal = { .minimum_version_id = 0, .post_load = vmsvga_post_load, .fields = (VMStateField[]) { - VMSTATE_INT32_EQUAL(new_depth, struct vmsvga_state_s), + VMSTATE_INT32_EQUAL(new_depth, struct vmsvga_state_s, NULL), VMSTATE_INT32(enable, struct vmsvga_state_s), VMSTATE_INT32(config, struct vmsvga_state_s), VMSTATE_INT32(cursor.id, struct vmsvga_state_s), diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index f60826d..874d3fe 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -1669,7 +1669,7 @@ const VMStateDescription vmstate_ahci = { VMSTATE_UINT32(control_regs.impl, AHCIState), VMSTATE_UINT32(control_regs.version, AHCIState), VMSTATE_UINT32(idp_index, AHCIState), - VMSTATE_INT32_EQUAL(ports, AHCIState), + VMSTATE_INT32_EQUAL(ports, AHCIState, NULL), VMSTATE_END_OF_LIST() }, }; diff --git a/hw/input/vmmouse.c b/hw/input/vmmouse.c index 4747da9..b6d2208 100644 --- a/hw/input/vmmouse.c +++ b/hw/input/vmmouse.c @@ -243,7 +243,7 @@ static const VMStateDescription vmstate_vmmouse = { .minimum_version_id = 0, .post_load = vmmouse_post_load, .fields = (VMStateField[]) { - VMSTATE_INT32_EQUAL(queue_size, VMMouseState), + VMSTATE_INT32_EQUAL(queue_size, VMMouseState, NULL), VMSTATE_UINT32_ARRAY(queue, VMMouseState, VMMOUSE_QUEUE_SIZE), VMSTATE_UINT16(nb_queue, VMMouseState), VMSTATE_UINT16(status, VMMouseState), diff --git a/hw/intc/openpic.c b/hw/intc/openpic.c index f966d06..5595bb2 100644 --- a/hw/intc/openpic.c +++ b/hw/intc/openpic.c @@ -1499,7 +1499,7 @@ static const VMStateDescription vmstate_openpic = { VMSTATE_UINT32(max_irq, OpenPICState), VMSTATE_STRUCT_VARRAY_UINT32(src, OpenPICState, max_irq, 0, vmstate_openpic_irqsource, IRQSource), - VMSTATE_UINT32_EQUAL(nb_cpus, OpenPICState), + VMSTATE_UINT32_EQUAL(nb_cpus, OpenPICState, NULL), VMSTATE_STRUCT_VARRAY_UINT32(dst, OpenPICState, nb_cpus, 0, vmstate_openpic_irqdest, IRQDest), VMSTATE_STRUCT_ARRAY(timers, OpenPICState, OPENPIC_MAX_TMR, 0, diff --git a/hw/intc/xics.c b/hw/intc/xics.c index 7ccfb53..d4194d6 100644 --- a/hw/intc/xics.c +++ b/hw/intc/xics.c @@ -574,7 +574,7 @@ static const VMStateDescription vmstate_ics_simple = { .post_load = ics_simple_dispatch_post_load, .fields = (VMStateField[]) { /* Sanity check */ - VMSTATE_UINT32_EQUAL(nr_irqs, ICSState), + VMSTATE_UINT32_EQUAL(nr_irqs, ICSState, NULL), VMSTATE_STRUCT_VARRAY_POINTER_UINT32(irqs, ICSState, nr_irqs, vmstate_ics_simple_irq, diff --git a/hw/misc/max111x.c b/hw/misc/max111x.c index 2a277bd..6dbdc03 100644 --- a/hw/misc/max111x.c +++ b/hw/misc/max111x.c @@ -116,7 +116,7 @@ static const VMStateDescription vmstate_max111x = { VMSTATE_UINT8(tb1, MAX111xState), VMSTATE_UINT8(rb2, MAX111xState), VMSTATE_UINT8(rb3, MAX111xState), - VMSTATE_INT32_EQUAL(inputs, MAX111xState), + VMSTATE_INT32_EQUAL(inputs, MAX111xState, NULL), VMSTATE_INT32(com, MAX111xState), VMSTATE_ARRAY_INT32_UNSAFE(input, MAX111xState, inputs, vmstate_info_uint8, uint8_t), diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c index 0341bc0..8656a54 100644 --- a/hw/ppc/spapr_iommu.c +++ b/hw/ppc/spapr_iommu.c @@ -231,7 +231,7 @@ static const VMStateDescription vmstate_spapr_tce_table = { .post_load = spapr_tce_table_post_load, .fields = (VMStateField []) { /* Sanity check */ - VMSTATE_UINT32_EQUAL(liobn, sPAPRTCETable), + VMSTATE_UINT32_EQUAL(liobn, sPAPRTCETable, NULL), /* IOMMU state */ VMSTATE_UINT32(mig_nb_table, sPAPRTCETable), diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index 0b447f2..3b37dcd 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -1848,7 +1848,7 @@ static const VMStateDescription vmstate_spapr_pci_lsi = { .version_id = 1, .minimum_version_id = 1, .fields = (VMStateField[]) { - VMSTATE_UINT32_EQUAL(irq, struct spapr_pci_lsi), + VMSTATE_UINT32_EQUAL(irq, struct spapr_pci_lsi, NULL), VMSTATE_END_OF_LIST() }, @@ -1936,7 +1936,7 @@ static const VMStateDescription vmstate_spapr_pci = { .pre_save = spapr_pci_pre_save, .post_load = spapr_pci_post_load, .fields = (VMStateField[]) { - VMSTATE_UINT64_EQUAL(buid, sPAPRPHBState), + VMSTATE_UINT64_EQUAL(buid, sPAPRPHBState, NULL), VMSTATE_UINT32_TEST(mig_liobn, sPAPRPHBState, pre_2_8_migration), VMSTATE_UINT64_TEST(mig_mem_win_addr, sPAPRPHBState, pre_2_8_migration), VMSTATE_UINT64_TEST(mig_mem_win_size, sPAPRPHBState, pre_2_8_migration), diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c index a0ee4fd..ea3bc8b 100644 --- a/hw/ppc/spapr_vio.c +++ b/hw/ppc/spapr_vio.c @@ -557,8 +557,8 @@ const VMStateDescription vmstate_spapr_vio = { .minimum_version_id = 1, .fields = (VMStateField[]) { /* Sanity check */ - VMSTATE_UINT32_EQUAL(reg, VIOsPAPRDevice), - VMSTATE_UINT32_EQUAL(irq, VIOsPAPRDevice), + VMSTATE_UINT32_EQUAL(reg, VIOsPAPRDevice, NULL), + VMSTATE_UINT32_EQUAL(irq, VIOsPAPRDevice, NULL), /* General VIO device state */ VMSTATE_UINT64(signal_state, VIOsPAPRDevice), diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c index ca72a80..e3562a4 100644 --- a/hw/usb/hcd-uhci.c +++ b/hw/usb/hcd-uhci.c @@ -415,7 +415,7 @@ static const VMStateDescription vmstate_uhci = { .post_load = uhci_post_load, .fields = (VMStateField[]) { VMSTATE_PCI_DEVICE(dev, UHCIState), - VMSTATE_UINT8_EQUAL(num_ports_vmstate, UHCIState), + VMSTATE_UINT8_EQUAL(num_ports_vmstate, UHCIState, NULL), VMSTATE_STRUCT_ARRAY(ports, UHCIState, NB_PORTS, 1, vmstate_uhci_port, UHCIPort), VMSTATE_UINT16(cmd, UHCIState), diff --git a/target/ppc/machine.c b/target/ppc/machine.c index 6cb3a48..445f489 100644 --- a/target/ppc/machine.c +++ b/target/ppc/machine.c @@ -419,7 +419,7 @@ static const VMStateDescription vmstate_slb = { .needed = slb_needed, .post_load = slb_post_load, .fields = (VMStateField[]) { - VMSTATE_INT32_EQUAL(env.slb_nr, PowerPCCPU), + VMSTATE_INT32_EQUAL(env.slb_nr, PowerPCCPU, NULL), VMSTATE_SLB_ARRAY(env.slb, PowerPCCPU, MAX_SLB_ENTRIES), VMSTATE_END_OF_LIST() } @@ -452,7 +452,7 @@ static const VMStateDescription vmstate_tlb6xx = { .minimum_version_id = 1, .needed = tlb6xx_needed, .fields = (VMStateField[]) { - VMSTATE_INT32_EQUAL(env.nb_tlb, PowerPCCPU), + VMSTATE_INT32_EQUAL(env.nb_tlb, PowerPCCPU, NULL), VMSTATE_STRUCT_VARRAY_POINTER_INT32(env.tlb.tlb6, PowerPCCPU, env.nb_tlb, vmstate_tlb6xx_entry, @@ -510,7 +510,7 @@ static const VMStateDescription vmstate_tlbemb = { .minimum_version_id = 1, .needed = tlbemb_needed, .fields = (VMStateField[]) { - VMSTATE_INT32_EQUAL(env.nb_tlb, PowerPCCPU), + VMSTATE_INT32_EQUAL(env.nb_tlb, PowerPCCPU, NULL), VMSTATE_STRUCT_VARRAY_POINTER_INT32(env.tlb.tlbe, PowerPCCPU, env.nb_tlb, vmstate_tlbemb_entry, @@ -551,7 +551,7 @@ static const VMStateDescription vmstate_tlbmas = { .minimum_version_id = 1, .needed = tlbmas_needed, .fields = (VMStateField[]) { - VMSTATE_INT32_EQUAL(env.nb_tlb, PowerPCCPU), + VMSTATE_INT32_EQUAL(env.nb_tlb, PowerPCCPU, NULL), VMSTATE_STRUCT_VARRAY_POINTER_INT32(env.tlb.tlbm, PowerPCCPU, env.nb_tlb, vmstate_tlbmas_entry, -- 2.9.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] exec: fix access to ram_list.dirty_memory when sync dirty bitmap 2017-06-28 9:09 ` Juan Quintela @ 2017-06-28 11:12 ` Haozhong Zhang 2017-06-28 11:32 ` Juan Quintela 0 siblings, 1 reply; 6+ messages in thread From: Haozhong Zhang @ 2017-06-28 11:12 UTC (permalink / raw) To: Juan Quintela; +Cc: qemu-devel, Paolo Bonzini, Stefan Hajnoczi, Xiao Guangrong On 06/28/17 11:09 +0200, Juan Quintela wrote: > Haozhong Zhang <haozhong.zhang@intel.com> wrote: > > In cpu_physical_memory_sync_dirty_bitmap(rb, start, ...), the 2nd > > argument 'start' is relative to the start of the ramblock 'rb'. When > > it's used to access the dirty memory bitmap of ram_list (i.e. > > ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION]->blocks[]), an offset to > > the start of all RAM (i.e. rb->offset) should be added to it, which has > > however been missed since c/s 6b6712efcc. For a ramblock of host memory > > backend whose offset is not zero, cpu_physical_memory_sync_dirty_bitmap() > > synchronizes the incorrect part of the dirty memory bitmap of ram_list > > to the per ramblock dirty bitmap. As a result, a guest with host > > memory backend may crash after migration. > > > > Fix it by adding the offset of ramblock when accessing the dirty memory > > bitmap of ram_list in cpu_physical_memory_sync_dirty_bitmap(). > > > > Reported-by: Stefan Hajnoczi <stefanha@redhat.com> > > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> > > > Hi > > I need to add this patch to make it compile for me with all > architectures enabled. > > I am adding that to you patch, are you ok? > Remind me why your following patch is related to mine? My patch does not touch any vmstate. Thanks, Haozhong > Thanks, Juan. > > From 74573a4190af8a357054493d5451e2b51c553498 Mon Sep 17 00:00:00 2001 > From: Juan Quintela <quintela@redhat.com> > Date: Wed, 28 Jun 2017 10:19:25 +0200 > Subject: [PATCH] fix missing Macro applications > > > Signed-off-by: Juan Quintela <quintela@redhat.com> > --- > hw/block/fdc.c | 2 +- > hw/display/qxl.c | 4 ++-- > hw/display/vga.c | 2 +- > hw/display/vmware_vga.c | 2 +- > hw/ide/ahci.c | 2 +- > hw/input/vmmouse.c | 2 +- > hw/intc/openpic.c | 2 +- > hw/intc/xics.c | 2 +- > hw/misc/max111x.c | 2 +- > hw/ppc/spapr_iommu.c | 2 +- > hw/ppc/spapr_pci.c | 4 ++-- > hw/ppc/spapr_vio.c | 4 ++-- > hw/usb/hcd-uhci.c | 2 +- > target/ppc/machine.c | 8 ++++---- > 14 files changed, 20 insertions(+), 20 deletions(-) > > diff --git a/hw/block/fdc.c b/hw/block/fdc.c > index 28f6b6e..4011290 100644 > --- a/hw/block/fdc.c > +++ b/hw/block/fdc.c > @@ -1217,7 +1217,7 @@ static const VMStateDescription vmstate_fdc = { > VMSTATE_UINT8(config, FDCtrl), > VMSTATE_UINT8(lock, FDCtrl), > VMSTATE_UINT8(pwrd, FDCtrl), > - VMSTATE_UINT8_EQUAL(num_floppies, FDCtrl), > + VMSTATE_UINT8_EQUAL(num_floppies, FDCtrl, NULL), > VMSTATE_STRUCT_ARRAY(drives, FDCtrl, MAX_FD, 1, > vmstate_fdrive, FDrive), > VMSTATE_END_OF_LIST() > diff --git a/hw/display/qxl.c b/hw/display/qxl.c > index ad09bb9..3c1688e 100644 > --- a/hw/display/qxl.c > +++ b/hw/display/qxl.c > @@ -2373,12 +2373,12 @@ static VMStateDescription qxl_vmstate = { > VMSTATE_UINT32(last_release_offset, PCIQXLDevice), > VMSTATE_UINT32(mode, PCIQXLDevice), > VMSTATE_UINT32(ssd.unique, PCIQXLDevice), > - VMSTATE_INT32_EQUAL(num_memslots, PCIQXLDevice), > + VMSTATE_INT32_EQUAL(num_memslots, PCIQXLDevice, NULL), > VMSTATE_STRUCT_ARRAY(guest_slots, PCIQXLDevice, NUM_MEMSLOTS, 0, > qxl_memslot, struct guest_slots), > VMSTATE_STRUCT(guest_primary.surface, PCIQXLDevice, 0, > qxl_surface, QXLSurfaceCreate), > - VMSTATE_INT32_EQUAL(ssd.num_surfaces, PCIQXLDevice), > + VMSTATE_INT32_EQUAL(ssd.num_surfaces, PCIQXLDevice, NULL), > VMSTATE_VARRAY_INT32(guest_surfaces.cmds, PCIQXLDevice, > ssd.num_surfaces, 0, > vmstate_info_uint64, uint64_t), > diff --git a/hw/display/vga.c b/hw/display/vga.c > index dcc95f8..80508b8 100644 > --- a/hw/display/vga.c > +++ b/hw/display/vga.c > @@ -2099,7 +2099,7 @@ const VMStateDescription vmstate_vga_common = { > VMSTATE_BUFFER(palette, VGACommonState), > > VMSTATE_INT32(bank_offset, VGACommonState), > - VMSTATE_UINT8_EQUAL(is_vbe_vmstate, VGACommonState), > + VMSTATE_UINT8_EQUAL(is_vbe_vmstate, VGACommonState, NULL), > VMSTATE_UINT16(vbe_index, VGACommonState), > VMSTATE_UINT16_ARRAY(vbe_regs, VGACommonState, VBE_DISPI_INDEX_NB), > VMSTATE_UINT32(vbe_start_addr, VGACommonState), > diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c > index ec5f27d..c989cef 100644 > --- a/hw/display/vmware_vga.c > +++ b/hw/display/vmware_vga.c > @@ -1192,7 +1192,7 @@ static const VMStateDescription vmstate_vmware_vga_internal = { > .minimum_version_id = 0, > .post_load = vmsvga_post_load, > .fields = (VMStateField[]) { > - VMSTATE_INT32_EQUAL(new_depth, struct vmsvga_state_s), > + VMSTATE_INT32_EQUAL(new_depth, struct vmsvga_state_s, NULL), > VMSTATE_INT32(enable, struct vmsvga_state_s), > VMSTATE_INT32(config, struct vmsvga_state_s), > VMSTATE_INT32(cursor.id, struct vmsvga_state_s), > diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c > index f60826d..874d3fe 100644 > --- a/hw/ide/ahci.c > +++ b/hw/ide/ahci.c > @@ -1669,7 +1669,7 @@ const VMStateDescription vmstate_ahci = { > VMSTATE_UINT32(control_regs.impl, AHCIState), > VMSTATE_UINT32(control_regs.version, AHCIState), > VMSTATE_UINT32(idp_index, AHCIState), > - VMSTATE_INT32_EQUAL(ports, AHCIState), > + VMSTATE_INT32_EQUAL(ports, AHCIState, NULL), > VMSTATE_END_OF_LIST() > }, > }; > diff --git a/hw/input/vmmouse.c b/hw/input/vmmouse.c > index 4747da9..b6d2208 100644 > --- a/hw/input/vmmouse.c > +++ b/hw/input/vmmouse.c > @@ -243,7 +243,7 @@ static const VMStateDescription vmstate_vmmouse = { > .minimum_version_id = 0, > .post_load = vmmouse_post_load, > .fields = (VMStateField[]) { > - VMSTATE_INT32_EQUAL(queue_size, VMMouseState), > + VMSTATE_INT32_EQUAL(queue_size, VMMouseState, NULL), > VMSTATE_UINT32_ARRAY(queue, VMMouseState, VMMOUSE_QUEUE_SIZE), > VMSTATE_UINT16(nb_queue, VMMouseState), > VMSTATE_UINT16(status, VMMouseState), > diff --git a/hw/intc/openpic.c b/hw/intc/openpic.c > index f966d06..5595bb2 100644 > --- a/hw/intc/openpic.c > +++ b/hw/intc/openpic.c > @@ -1499,7 +1499,7 @@ static const VMStateDescription vmstate_openpic = { > VMSTATE_UINT32(max_irq, OpenPICState), > VMSTATE_STRUCT_VARRAY_UINT32(src, OpenPICState, max_irq, 0, > vmstate_openpic_irqsource, IRQSource), > - VMSTATE_UINT32_EQUAL(nb_cpus, OpenPICState), > + VMSTATE_UINT32_EQUAL(nb_cpus, OpenPICState, NULL), > VMSTATE_STRUCT_VARRAY_UINT32(dst, OpenPICState, nb_cpus, 0, > vmstate_openpic_irqdest, IRQDest), > VMSTATE_STRUCT_ARRAY(timers, OpenPICState, OPENPIC_MAX_TMR, 0, > diff --git a/hw/intc/xics.c b/hw/intc/xics.c > index 7ccfb53..d4194d6 100644 > --- a/hw/intc/xics.c > +++ b/hw/intc/xics.c > @@ -574,7 +574,7 @@ static const VMStateDescription vmstate_ics_simple = { > .post_load = ics_simple_dispatch_post_load, > .fields = (VMStateField[]) { > /* Sanity check */ > - VMSTATE_UINT32_EQUAL(nr_irqs, ICSState), > + VMSTATE_UINT32_EQUAL(nr_irqs, ICSState, NULL), > > VMSTATE_STRUCT_VARRAY_POINTER_UINT32(irqs, ICSState, nr_irqs, > vmstate_ics_simple_irq, > diff --git a/hw/misc/max111x.c b/hw/misc/max111x.c > index 2a277bd..6dbdc03 100644 > --- a/hw/misc/max111x.c > +++ b/hw/misc/max111x.c > @@ -116,7 +116,7 @@ static const VMStateDescription vmstate_max111x = { > VMSTATE_UINT8(tb1, MAX111xState), > VMSTATE_UINT8(rb2, MAX111xState), > VMSTATE_UINT8(rb3, MAX111xState), > - VMSTATE_INT32_EQUAL(inputs, MAX111xState), > + VMSTATE_INT32_EQUAL(inputs, MAX111xState, NULL), > VMSTATE_INT32(com, MAX111xState), > VMSTATE_ARRAY_INT32_UNSAFE(input, MAX111xState, inputs, > vmstate_info_uint8, uint8_t), > diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c > index 0341bc0..8656a54 100644 > --- a/hw/ppc/spapr_iommu.c > +++ b/hw/ppc/spapr_iommu.c > @@ -231,7 +231,7 @@ static const VMStateDescription vmstate_spapr_tce_table = { > .post_load = spapr_tce_table_post_load, > .fields = (VMStateField []) { > /* Sanity check */ > - VMSTATE_UINT32_EQUAL(liobn, sPAPRTCETable), > + VMSTATE_UINT32_EQUAL(liobn, sPAPRTCETable, NULL), > > /* IOMMU state */ > VMSTATE_UINT32(mig_nb_table, sPAPRTCETable), > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c > index 0b447f2..3b37dcd 100644 > --- a/hw/ppc/spapr_pci.c > +++ b/hw/ppc/spapr_pci.c > @@ -1848,7 +1848,7 @@ static const VMStateDescription vmstate_spapr_pci_lsi = { > .version_id = 1, > .minimum_version_id = 1, > .fields = (VMStateField[]) { > - VMSTATE_UINT32_EQUAL(irq, struct spapr_pci_lsi), > + VMSTATE_UINT32_EQUAL(irq, struct spapr_pci_lsi, NULL), > > VMSTATE_END_OF_LIST() > }, > @@ -1936,7 +1936,7 @@ static const VMStateDescription vmstate_spapr_pci = { > .pre_save = spapr_pci_pre_save, > .post_load = spapr_pci_post_load, > .fields = (VMStateField[]) { > - VMSTATE_UINT64_EQUAL(buid, sPAPRPHBState), > + VMSTATE_UINT64_EQUAL(buid, sPAPRPHBState, NULL), > VMSTATE_UINT32_TEST(mig_liobn, sPAPRPHBState, pre_2_8_migration), > VMSTATE_UINT64_TEST(mig_mem_win_addr, sPAPRPHBState, pre_2_8_migration), > VMSTATE_UINT64_TEST(mig_mem_win_size, sPAPRPHBState, pre_2_8_migration), > diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c > index a0ee4fd..ea3bc8b 100644 > --- a/hw/ppc/spapr_vio.c > +++ b/hw/ppc/spapr_vio.c > @@ -557,8 +557,8 @@ const VMStateDescription vmstate_spapr_vio = { > .minimum_version_id = 1, > .fields = (VMStateField[]) { > /* Sanity check */ > - VMSTATE_UINT32_EQUAL(reg, VIOsPAPRDevice), > - VMSTATE_UINT32_EQUAL(irq, VIOsPAPRDevice), > + VMSTATE_UINT32_EQUAL(reg, VIOsPAPRDevice, NULL), > + VMSTATE_UINT32_EQUAL(irq, VIOsPAPRDevice, NULL), > > /* General VIO device state */ > VMSTATE_UINT64(signal_state, VIOsPAPRDevice), > diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c > index ca72a80..e3562a4 100644 > --- a/hw/usb/hcd-uhci.c > +++ b/hw/usb/hcd-uhci.c > @@ -415,7 +415,7 @@ static const VMStateDescription vmstate_uhci = { > .post_load = uhci_post_load, > .fields = (VMStateField[]) { > VMSTATE_PCI_DEVICE(dev, UHCIState), > - VMSTATE_UINT8_EQUAL(num_ports_vmstate, UHCIState), > + VMSTATE_UINT8_EQUAL(num_ports_vmstate, UHCIState, NULL), > VMSTATE_STRUCT_ARRAY(ports, UHCIState, NB_PORTS, 1, > vmstate_uhci_port, UHCIPort), > VMSTATE_UINT16(cmd, UHCIState), > diff --git a/target/ppc/machine.c b/target/ppc/machine.c > index 6cb3a48..445f489 100644 > --- a/target/ppc/machine.c > +++ b/target/ppc/machine.c > @@ -419,7 +419,7 @@ static const VMStateDescription vmstate_slb = { > .needed = slb_needed, > .post_load = slb_post_load, > .fields = (VMStateField[]) { > - VMSTATE_INT32_EQUAL(env.slb_nr, PowerPCCPU), > + VMSTATE_INT32_EQUAL(env.slb_nr, PowerPCCPU, NULL), > VMSTATE_SLB_ARRAY(env.slb, PowerPCCPU, MAX_SLB_ENTRIES), > VMSTATE_END_OF_LIST() > } > @@ -452,7 +452,7 @@ static const VMStateDescription vmstate_tlb6xx = { > .minimum_version_id = 1, > .needed = tlb6xx_needed, > .fields = (VMStateField[]) { > - VMSTATE_INT32_EQUAL(env.nb_tlb, PowerPCCPU), > + VMSTATE_INT32_EQUAL(env.nb_tlb, PowerPCCPU, NULL), > VMSTATE_STRUCT_VARRAY_POINTER_INT32(env.tlb.tlb6, PowerPCCPU, > env.nb_tlb, > vmstate_tlb6xx_entry, > @@ -510,7 +510,7 @@ static const VMStateDescription vmstate_tlbemb = { > .minimum_version_id = 1, > .needed = tlbemb_needed, > .fields = (VMStateField[]) { > - VMSTATE_INT32_EQUAL(env.nb_tlb, PowerPCCPU), > + VMSTATE_INT32_EQUAL(env.nb_tlb, PowerPCCPU, NULL), > VMSTATE_STRUCT_VARRAY_POINTER_INT32(env.tlb.tlbe, PowerPCCPU, > env.nb_tlb, > vmstate_tlbemb_entry, > @@ -551,7 +551,7 @@ static const VMStateDescription vmstate_tlbmas = { > .minimum_version_id = 1, > .needed = tlbmas_needed, > .fields = (VMStateField[]) { > - VMSTATE_INT32_EQUAL(env.nb_tlb, PowerPCCPU), > + VMSTATE_INT32_EQUAL(env.nb_tlb, PowerPCCPU, NULL), > VMSTATE_STRUCT_VARRAY_POINTER_INT32(env.tlb.tlbm, PowerPCCPU, > env.nb_tlb, > vmstate_tlbmas_entry, > -- > 2.9.4 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] exec: fix access to ram_list.dirty_memory when sync dirty bitmap 2017-06-28 11:12 ` Haozhong Zhang @ 2017-06-28 11:32 ` Juan Quintela 0 siblings, 0 replies; 6+ messages in thread From: Juan Quintela @ 2017-06-28 11:32 UTC (permalink / raw) To: qemu-devel; +Cc: Paolo Bonzini, Stefan Hajnoczi, Xiao Guangrong Haozhong Zhang <haozhong.zhang@intel.com> wrote: > On 06/28/17 11:09 +0200, Juan Quintela wrote: >> Haozhong Zhang <haozhong.zhang@intel.com> wrote: >> > In cpu_physical_memory_sync_dirty_bitmap(rb, start, ...), the 2nd >> > argument 'start' is relative to the start of the ramblock 'rb'. When >> > it's used to access the dirty memory bitmap of ram_list (i.e. >> > ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION]->blocks[]), an offset to >> > the start of all RAM (i.e. rb->offset) should be added to it, which has >> > however been missed since c/s 6b6712efcc. For a ramblock of host memory >> > backend whose offset is not zero, cpu_physical_memory_sync_dirty_bitmap() >> > synchronizes the incorrect part of the dirty memory bitmap of ram_list >> > to the per ramblock dirty bitmap. As a result, a guest with host >> > memory backend may crash after migration. >> > >> > Fix it by adding the offset of ramblock when accessing the dirty memory >> > bitmap of ram_list in cpu_physical_memory_sync_dirty_bitmap(). >> > >> > Reported-by: Stefan Hajnoczi <stefanha@redhat.com> >> > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> >> >> >> Hi >> >> I need to add this patch to make it compile for me with all >> architectures enabled. >> >> I am adding that to you patch, are you ok? >> > > Remind me why your following patch is related to mine? My patch does > not touch any vmstate. O:-) Because sometimes I got a bit sloppy. Sorry. Later, Juan. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-06-28 11:32 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-06-28 2:43 [Qemu-devel] [PATCH] exec: fix access to ram_list.dirty_memory when sync dirty bitmap Haozhong Zhang 2017-06-28 7:30 ` Juan Quintela 2017-06-28 8:23 ` Paolo Bonzini 2017-06-28 9:09 ` Juan Quintela 2017-06-28 11:12 ` Haozhong Zhang 2017-06-28 11:32 ` Juan Quintela
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).