| Message ID | 1714406135-451286-26-git-send-email-steven.sistare@oracle.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Live update: cpr-exec | expand |
Steve Sistare <steven.sistare@oracle.com> writes: > For cpr-exec mode, ramblock_is_ignored is always true, and the address of > each migrated memory region must match the address of the statically > initialized region on the target. However, for a PCI rom block, the region > address is set when the guest writes to a BAR on the source, which does not > occur on the target, causing a "Mismatched GPAs" error during cpr-exec > migration. > > To fix, unconditionally set the target's address to the source's address > if the region does not have an address yet. > > Signed-off-by: Steve Sistare <steven.sistare@oracle.com> Just a detail below. Reviewed-by: Fabiano Rosas <farosas@suse.de> > --- > include/exec/memory.h | 12 ++++++++++++ > migration/ram.c | 15 +++++++++------ > system/memory.c | 10 ++++++++-- > 3 files changed, 29 insertions(+), 8 deletions(-) > > diff --git a/include/exec/memory.h b/include/exec/memory.h > index d337737..4f654b0 100644 > --- a/include/exec/memory.h > +++ b/include/exec/memory.h > @@ -801,6 +801,7 @@ struct MemoryRegion { > bool unmergeable; > uint8_t dirty_log_mask; > bool is_iommu; > + bool has_addr; This field is not used during memory access, maybe move it down below to preserve the hole for future usage. > RAMBlock *ram_block; > Object *owner; > /* owner as TYPE_DEVICE. Used for re-entrancy checks in MR access hotpath */ > @@ -2402,6 +2403,17 @@ void memory_region_set_enabled(MemoryRegion *mr, bool enabled); > void memory_region_set_address(MemoryRegion *mr, hwaddr addr); > > /* > + * memory_region_set_address_only: set the address of a region. > + * > + * Same as memory_region_set_address, but without causing transaction side > + * effects. > + * > + * @mr: the region to be updated > + * @addr: new address, relative to container region > + */ > +void memory_region_set_address_only(MemoryRegion *mr, hwaddr addr); > + > +/* > * memory_region_set_size: dynamically update the size of a region. > * > * Dynamically updates the size of a region. > diff --git a/migration/ram.c b/migration/ram.c > index add285b..7b8d7f6 100644 > --- a/migration/ram.c > +++ b/migration/ram.c > @@ -4196,12 +4196,15 @@ static int parse_ramblock(QEMUFile *f, RAMBlock *block, ram_addr_t length) > } > if (migrate_ignore_shared()) { > hwaddr addr = qemu_get_be64(f); > - if (migrate_ram_is_ignored(block) && > - block->mr->addr != addr) { > - error_report("Mismatched GPAs for block %s " > - "%" PRId64 "!= %" PRId64, block->idstr, > - (uint64_t)addr, (uint64_t)block->mr->addr); > - return -EINVAL; > + if (migrate_ram_is_ignored(block)) { > + if (!block->mr->has_addr) { > + memory_region_set_address_only(block->mr, addr); > + } else if (block->mr->addr != addr) { > + error_report("Mismatched GPAs for block %s " > + "%" PRId64 "!= %" PRId64, block->idstr, > + (uint64_t)addr, (uint64_t)block->mr->addr); > + return -EINVAL; > + } > } > } > ret = rdma_block_notification_handle(f, block->idstr); > diff --git a/system/memory.c b/system/memory.c > index ca04a0e..3c72504 100644 > --- a/system/memory.c > +++ b/system/memory.c > @@ -2665,7 +2665,7 @@ static void memory_region_add_subregion_common(MemoryRegion *mr, > for (alias = subregion->alias; alias; alias = alias->alias) { > alias->mapped_via_alias++; > } > - subregion->addr = offset; > + memory_region_set_address_only(subregion, offset); > memory_region_update_container_subregions(subregion); > } > > @@ -2745,10 +2745,16 @@ static void memory_region_readd_subregion(MemoryRegion *mr) > } > } > > +void memory_region_set_address_only(MemoryRegion *mr, hwaddr addr) > +{ > + mr->addr = addr; > + mr->has_addr = true; > +} > + > void memory_region_set_address(MemoryRegion *mr, hwaddr addr) > { > if (addr != mr->addr) { > - mr->addr = addr; > + memory_region_set_address_only(mr, addr); > memory_region_readd_subregion(mr); > } > }
diff --git a/include/exec/memory.h b/include/exec/memory.h index d337737..4f654b0 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -801,6 +801,7 @@ struct MemoryRegion { bool unmergeable; uint8_t dirty_log_mask; bool is_iommu; + bool has_addr; RAMBlock *ram_block; Object *owner; /* owner as TYPE_DEVICE. Used for re-entrancy checks in MR access hotpath */ @@ -2402,6 +2403,17 @@ void memory_region_set_enabled(MemoryRegion *mr, bool enabled); void memory_region_set_address(MemoryRegion *mr, hwaddr addr); /* + * memory_region_set_address_only: set the address of a region. + * + * Same as memory_region_set_address, but without causing transaction side + * effects. + * + * @mr: the region to be updated + * @addr: new address, relative to container region + */ +void memory_region_set_address_only(MemoryRegion *mr, hwaddr addr); + +/* * memory_region_set_size: dynamically update the size of a region. * * Dynamically updates the size of a region. diff --git a/migration/ram.c b/migration/ram.c index add285b..7b8d7f6 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -4196,12 +4196,15 @@ static int parse_ramblock(QEMUFile *f, RAMBlock *block, ram_addr_t length) } if (migrate_ignore_shared()) { hwaddr addr = qemu_get_be64(f); - if (migrate_ram_is_ignored(block) && - block->mr->addr != addr) { - error_report("Mismatched GPAs for block %s " - "%" PRId64 "!= %" PRId64, block->idstr, - (uint64_t)addr, (uint64_t)block->mr->addr); - return -EINVAL; + if (migrate_ram_is_ignored(block)) { + if (!block->mr->has_addr) { + memory_region_set_address_only(block->mr, addr); + } else if (block->mr->addr != addr) { + error_report("Mismatched GPAs for block %s " + "%" PRId64 "!= %" PRId64, block->idstr, + (uint64_t)addr, (uint64_t)block->mr->addr); + return -EINVAL; + } } } ret = rdma_block_notification_handle(f, block->idstr); diff --git a/system/memory.c b/system/memory.c index ca04a0e..3c72504 100644 --- a/system/memory.c +++ b/system/memory.c @@ -2665,7 +2665,7 @@ static void memory_region_add_subregion_common(MemoryRegion *mr, for (alias = subregion->alias; alias; alias = alias->alias) { alias->mapped_via_alias++; } - subregion->addr = offset; + memory_region_set_address_only(subregion, offset); memory_region_update_container_subregions(subregion); } @@ -2745,10 +2745,16 @@ static void memory_region_readd_subregion(MemoryRegion *mr) } } +void memory_region_set_address_only(MemoryRegion *mr, hwaddr addr) +{ + mr->addr = addr; + mr->has_addr = true; +} + void memory_region_set_address(MemoryRegion *mr, hwaddr addr) { if (addr != mr->addr) { - mr->addr = addr; + memory_region_set_address_only(mr, addr); memory_region_readd_subregion(mr); } }
For cpr-exec mode, ramblock_is_ignored is always true, and the address of each migrated memory region must match the address of the statically initialized region on the target. However, for a PCI rom block, the region address is set when the guest writes to a BAR on the source, which does not occur on the target, causing a "Mismatched GPAs" error during cpr-exec migration. To fix, unconditionally set the target's address to the source's address if the region does not have an address yet. Signed-off-by: Steve Sistare <steven.sistare@oracle.com> --- include/exec/memory.h | 12 ++++++++++++ migration/ram.c | 15 +++++++++------ system/memory.c | 10 ++++++++-- 3 files changed, 29 insertions(+), 8 deletions(-)