RE: [PATCH v4 10/14] migration: add support to migrate shared regions list

[Wang, Wei W]

> From: Brijesh Singh <brijesh.singh@amd.com>
> 
> When memory encryption is enabled, the hypervisor maintains a shared
> regions list which is referred by hypervisor during migration to check if 
> page is
> private or shared. This list is built during the VM bootup and must be 
> migrated
> to the target host so that hypervisor on target host can use it for future
> migration.
> 
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> Co-developed-by: Ashish Kalra <ashish.kalra@amd.com>
> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> ---
>  include/sysemu/sev.h |  2 ++
>  target/i386/sev.c    | 43
> +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 45 insertions(+)
> 
> diff --git a/include/sysemu/sev.h b/include/sysemu/sev.h index
> 3b913518c0..118ee66406 100644
> --- a/include/sysemu/sev.h
> +++ b/include/sysemu/sev.h
> @@ -32,5 +32,7 @@ void sev_es_set_reset_vector(CPUState *cpu);  int
> sev_remove_shared_regions_list(unsigned long gfn_start,
>                                     unsigned long gfn_end);  int
> sev_add_shared_regions_list(unsigned long gfn_start, unsigned long gfn_end);
> +int sev_save_outgoing_shared_regions_list(QEMUFile *f); int
> +sev_load_incoming_shared_regions_list(QEMUFile *f);
> 
>  #endif
> diff --git a/target/i386/sev.c b/target/i386/sev.c index
> 6d44b7ad21..789051f7b4 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -135,10 +135,15 @@ static const char *const sev_fw_errlist[] = {
> 
>  #define SEV_FW_BLOB_MAX_SIZE            0x4000          /* 16KB
> */
> 
> +#define SHARED_REGION_LIST_CONT     0x1
> +#define SHARED_REGION_LIST_END      0x2
> +
>  static struct ConfidentialGuestMemoryEncryptionOps
> sev_memory_encryption_ops = {
>      .save_setup = sev_save_setup,
>      .save_outgoing_page = sev_save_outgoing_page,
>      .load_incoming_page = sev_load_incoming_page,
> +    .save_outgoing_shared_regions_list =
> sev_save_outgoing_shared_regions_list,
> +    .load_incoming_shared_regions_list =
> + sev_load_incoming_shared_regions_list,

Hi Ashish,
I have some questions about the callbacks:

1) why using a list of shared regions, instead of bitmaps to record 
private/shared state?
I saw that the KVM side implementation used bitmaps in the first place and 
changed to
shared regions since v10, but don't find the reason.

2) why is the save/load of shared region list (or bitmap) made vendor specific?
I think it can be a common interface and data structure, e.g. KVM maintains a 
per memory slot
bitmap to be obtained by QEMU.

Best,
Wei