| Message ID | 1743087130-429075-1-git-send-email-steven.sistare@oracle.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [V2] migration: cpr breaks SNP guest | expand |
On 3/27/25 09:52, Steve Sistare wrote: > With aux-ram-share=off, booting an SNP guest fails with: > > ../util/error.c:68: error_setv: Assertion `*errp == NULL' failed. > > This is because a CPR blocker for the guest_memfd ramblock is added > twice, once in ram_block_add_cpr_blocker because aux-ram-share=off so > rb->fd < 0, and once in ram_block_add for a specific guest_memfd blocker. > > To fix, add the guest_memfd blocker iff a generic one would not be > added by ram_block_add_cpr_blocker. > > Fixes: 094a3dbc55df ("migration: ram block cpr blockers") > Reported-by: Tom Lendacky <thomas.lendacky@amd.com> > Reported-by: Michael Roth <michael.roth@amd.com> > Tested-by: Tom Lendacky <thomas.lendacky@amd.com> Re-testing shows this still fixes the SNP launch failure. So Tested-by: tag is still good. Thanks, Tom > Signed-off-by: Steve Sistare <steven.sistare@oracle.com> > --- > system/physmem.c | 20 +++++++++++++------- > 1 file changed, 13 insertions(+), 7 deletions(-) > > diff --git a/system/physmem.c b/system/physmem.c > index e97de3e..333a5eb 100644 > --- a/system/physmem.c > +++ b/system/physmem.c > @@ -158,6 +158,7 @@ static void io_mem_init(void); > static void memory_map_init(void); > static void tcg_log_global_after_sync(MemoryListener *listener); > static void tcg_commit(MemoryListener *listener); > +static bool ram_is_cpr_compatible(RAMBlock *rb); > > /** > * CPUAddressSpace: all the information a CPU needs about an AddressSpace > @@ -1908,13 +1909,18 @@ static void ram_block_add(RAMBlock *new_block, Error **errp) > goto out_free; > } > > - error_setg(&new_block->cpr_blocker, > - "Memory region %s uses guest_memfd, " > - "which is not supported with CPR.", > - memory_region_name(new_block->mr)); > - migrate_add_blocker_modes(&new_block->cpr_blocker, errp, > - MIG_MODE_CPR_TRANSFER, > - -1); > + /* > + * Add a specific guest_memfd blocker if a generic one would not be > + * added by ram_block_add_cpr_blocker. > + */ > + if (ram_is_cpr_compatible(new_block)) { > + error_setg(&new_block->cpr_blocker, > + "Memory region %s uses guest_memfd, " > + "which is not supported with CPR.", > + memory_region_name(new_block->mr)); > + migrate_add_blocker_modes(&new_block->cpr_blocker, errp, > + MIG_MODE_CPR_TRANSFER, -1); > + } > } > > ram_size = (new_block->offset + new_block->max_length) >> TARGET_PAGE_BITS;
Steve Sistare <steven.sistare@oracle.com> writes: > With aux-ram-share=off, booting an SNP guest fails with: > > ../util/error.c:68: error_setv: Assertion `*errp == NULL' failed. > > This is because a CPR blocker for the guest_memfd ramblock is added > twice, once in ram_block_add_cpr_blocker because aux-ram-share=off so > rb->fd < 0, and once in ram_block_add for a specific guest_memfd blocker. > > To fix, add the guest_memfd blocker iff a generic one would not be > added by ram_block_add_cpr_blocker. > > Fixes: 094a3dbc55df ("migration: ram block cpr blockers") > Reported-by: Tom Lendacky <thomas.lendacky@amd.com> > Reported-by: Michael Roth <michael.roth@amd.com> > Tested-by: Tom Lendacky <thomas.lendacky@amd.com> > Signed-off-by: Steve Sistare <steven.sistare@oracle.com> Queued, thanks!
diff --git a/system/physmem.c b/system/physmem.c index e97de3e..333a5eb 100644 --- a/system/physmem.c +++ b/system/physmem.c @@ -158,6 +158,7 @@ static void io_mem_init(void); static void memory_map_init(void); static void tcg_log_global_after_sync(MemoryListener *listener); static void tcg_commit(MemoryListener *listener); +static bool ram_is_cpr_compatible(RAMBlock *rb); /** * CPUAddressSpace: all the information a CPU needs about an AddressSpace @@ -1908,13 +1909,18 @@ static void ram_block_add(RAMBlock *new_block, Error **errp) goto out_free; } - error_setg(&new_block->cpr_blocker, - "Memory region %s uses guest_memfd, " - "which is not supported with CPR.", - memory_region_name(new_block->mr)); - migrate_add_blocker_modes(&new_block->cpr_blocker, errp, - MIG_MODE_CPR_TRANSFER, - -1); + /* + * Add a specific guest_memfd blocker if a generic one would not be + * added by ram_block_add_cpr_blocker. + */ + if (ram_is_cpr_compatible(new_block)) { + error_setg(&new_block->cpr_blocker, + "Memory region %s uses guest_memfd, " + "which is not supported with CPR.", + memory_region_name(new_block->mr)); + migrate_add_blocker_modes(&new_block->cpr_blocker, errp, + MIG_MODE_CPR_TRANSFER, -1); + } } ram_size = (new_block->offset + new_block->max_length) >> TARGET_PAGE_BITS;
With aux-ram-share=off, booting an SNP guest fails with: ../util/error.c:68: error_setv: Assertion `*errp == NULL' failed. This is because a CPR blocker for the guest_memfd ramblock is added twice, once in ram_block_add_cpr_blocker because aux-ram-share=off so rb->fd < 0, and once in ram_block_add for a specific guest_memfd blocker. To fix, add the guest_memfd blocker iff a generic one would not be added by ram_block_add_cpr_blocker. Fixes: 094a3dbc55df ("migration: ram block cpr blockers") Reported-by: Tom Lendacky <thomas.lendacky@amd.com> Reported-by: Michael Roth <michael.roth@amd.com> Tested-by: Tom Lendacky <thomas.lendacky@amd.com> Signed-off-by: Steve Sistare <steven.sistare@oracle.com> --- system/physmem.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-)