Re: [PATCH v3 3/7] memory: Unify the definiton of ReplayRamPopulate() and ReplayRamDiscard()

[Chenyi Qiang <chenyi.qiang@intel.com>]

On 3/17/2025 11:00 AM, Kishen Maloor wrote:
> On 3/10/25 1:18 AM, Chenyi Qiang wrote:
>> ...
>> diff --git a/migration/ram.c b/migration/ram.c
>> index ce28328141..053730367b 100644
>> --- a/migration/ram.c
>> +++ b/migration/ram.c
>> @@ -816,8 +816,8 @@ static inline bool
>> migration_bitmap_clear_dirty(RAMState *rs,
>>       return ret;
>>   }
>>   -static void dirty_bitmap_clear_section(MemoryRegionSection *section,
>> -                                       void *opaque)
>> +static int dirty_bitmap_clear_section(MemoryRegionSection *section,
>> +                                      void *opaque)
>>   {
>>       const hwaddr offset = section->offset_within_region;
>>       const hwaddr size = int128_get64(section->size);
>> @@ -836,6 +836,7 @@ static void
>> dirty_bitmap_clear_section(MemoryRegionSection *section,
>>       }
>>       *cleared_bits += bitmap_count_one_with_offset(rb->bmap, start,
>> npages);
>>       bitmap_clear(rb->bmap, start, npages);
> 
> It appears that the ram_discard_manager_replay_discarded() path would
> clear all private pages
> from the dirty bitmap over here, and thus break migration.

Exactly, but migration for confidential VM support with guest_memfd is
not supported yet.

> 
> Perhaps the MemoryAttributeManager should be excluded from this flow,
> something like below?

I think it would already fail before reaching this path if initiating
confidential VMs migration. But adding such check or assert for some
hints is also OK for me. No strong preference.

> 
> @@ -910,6 +909,9 @@ static uint64_t
> ramblock_dirty_bitmap_clear_discarded_pages(RAMBlock *rb)
>              .size = int128_make64(qemu_ram_get_used_length(rb)),
>          };
>  
> +        if (object_dynamic_cast(OBJECT(rdm),
> TYPE_MEMORY_ATTRIBUTE_MANAGER))
> +            return 0;
> +
>          ram_discard_manager_replay_discarded(rdm, &section,
>                                               dirty_bitmap_clear_section,
>                                               &cleared_bits);
>