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

[Ackerley Tng]

tarunsahu at google.com writes:

> Hi,
>
> Ackerley Tng <ackerleytng at google.com> writes:
>
>> Sean Christopherson <seanjc at 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).
>

Will look at RFC v2 in a bit :) Thanks!

> 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 =========
>

I think there are quite many cases to consider if we consider both
before and after conversion series lands. For preservation, shall we
assume it will go after conversion? :) This way we can avoid discussing
some cases.

> In this case, we have two scenerio
> 1. kvm->attributes
>    In this case, Every logic remains same as above.

This is the vm_memory_attributes=true case.

Since kvm_supported_mem_attributes(kvm) returns 0 when
kvm_arch_has_private_mem(kvm) returns false, the vm's mem_attr_array
will always say shared for any vm where kvm_arch_has_private_mem() is
false.

So yup, preservation doesn't preserve attributes but that's also
effectively preserving always-shared attributes, so all is good.

> 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).
>

Yup, preservation doesn't preserve attributes but that's also
effectively preserving always-shared attributes, so all is good.

>    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.

Perhaps it should be this way: the process of preserving should first
freeze further conversions, then check for private/shared status, and if
private, return error to userspace.

I haven't looked at RFC v2 yet, but IIUC this is kind of in line with
the other issue where we freeze allocations:

+ When preserving, freeze further allocations, preserve all pages that
  are already allocated. If there are pages that weren't allocated, too
  bad - allocations are frozen until unpreserve.
+ When preserving, freeze further conversions, preserve all shared
  pages. If there are private pages, fail preservation.

This way, there's no loss of any private/shared state, since only shared
pages are preserved.

>    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 ?? ).
>

So if a non-CoCo VM does use an all-private guest_memfd (didn't specify
INIT_SHARED) (don't think anyone will use this before guest_memfd
write() support), it'll just always fail preservation unless before
preserving everything was converted to shared.

> 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.