Re: [RFC PATCH v1 0/8] liveupdate: kvm: Guest_memfd preservation

[tarunsahu@google.com]

Hi,

Ackerley Tng <ackerleytng@google.com> writes:

> Sean Christopherson <seanjc@google.com> writes:
>
>>>
>>> [...snip...]
>>>
>>> we have one open Question left:
>>> 1. How to check guest_memfd is fully shared.
>>>
>>> [...snip...]
>>>
>>
>> Given that lack of support isn't going to be limited to _just_ guest_memfd,
>> simply disallow preservation if the VM supports private memory:
>>
>> 	if (kvm_arch_has_private_mem(kvm))
>> 		return -EOPNOTSUPP;
>
This is a good idea, I have implemented it in V2 (sent it recently).

Below, I have mentioned a detailed analysis. Please let me know your thoughts.

> Makes sense. Tarun this was the other option that I was suggesting when
> we discussed offline.
>
> I think (?) it is possible to create a fully-private guest_memfd for a
> non-Confidential VM, and even after conversion lands, for both
> vm_memory_attributes=true and vm_memory_attributes=false.
>
> In that case, your preservation series can still preserve memory tracked
> as private by guest_memfd but not used as private, right?
>
> I don't think anyone will use this combination before guest_memfd
> write() support lands, we just need to make sure there's no kernel crash
> or corruption in this case.

IIUC, currently, private memory definition is where cocovm with HW based
private memory is supported. Which is directly checked by
kvm_arch_has_private_mem and this return false incase ARCH does not
support it in HW (SEV/SNP, TDX).

So about the combination where a guest_memfd is tracked as private but
not actually private. (Created without the INIT_SHARED flags). Even
though kvm_arch_has_private_mem is false.

In this case the luo will preserve the guest_memfd. But it will not
preserve any attributes, because
1. during creation, we dont populate any attributes, so by default
guest_memfd memory always considered to be shared. (Even though no
INIT_SHARED is passed). Conversion to private IOCTL will fail as
kvm_arch_has_private_mem check will fail for non-COCOVM. for COCOVM,
presevation will not be supported and conversion to private memory will
be succeded. (No corruption and kernel crash in this case)


==== WHAT IF CONVERSION SERIES LANDS =========

In this case, we have two scenerio
1. kvm->attributes
   In this case, Every logic remains same as above.
2. gmem->attributes
   if INIT_SHARED: memory is initially shared and no entry in maple tree, and if
   kvm_arch_has_private_mem returns false (non-COCOVM), this will just
   fails any conversion request. hence preserving the guest_memfd wont
   have any problem (fully shared case).

   if INIT_SHARED not set: memory is initially private, and there is an
   entry in maple tree, marked the memory as private. During conversion:
   A. To private: it fails as kvm_arch_has_private_mem return false.
                  Preservation is safe and there will be no problem. as in preservation
                  we dont preserve any attributes, but on retrieval, when a new gmem
                  file is created, identical entry to attributes will also be assigned
                  as INIT_SHARED flag is not set. So we wont have any issues in this case.
   B. TO shared: it passes, and update the maple tree. preservation will
   not be preserving the attributes with current patches, and it will
   also allow the preservation as kvm_arch_has_private_mem is false. So
   in this case, on retrieval, we will lose the data regarding the
   private vs shared (non-cocovm). So if host want to access the shared
   memory, it will try MMAP and which will fail in new kernel (which was
   successful in old kernel as conversion happened). But I dont see any
   kernel crash or corruption.
   This is an issue with only non-cocoVM support with conversion series
   having the gmem attribute enabled without INIT_SHARED flag. I am not
   sure, if there will be any user for this very soon (is it SW_PROTECTED_VM ?? ).

Let me know, If my understanding is correct. Should we add INIT_SHARED
along with kvm_arch_has_private_mem check to make Above case 2.B.
impossible in future for the current support of guest_memfd.