From: Michael Roth <[email protected]>

For vm_memory_attributes=1, in-place conversion/population is not
supported, so the initial contents necessarily must need to come
from a separate src address, which is enforced by the current
implementation. However, for vm_memory_attributes=0, it is possible for
guest memory to be initialized directly from userspace by mmap()'ing the
guest_memfd and writing to it while the corresponding GPA ranges are in
a 'shared' state before converting them to the 'private' state expected
by KVM_SEV_SNP_LAUNCH_UPDATE.

Update the handling/documentation for KVM_SEV_SNP_LAUNCH_UPDATE to allow
for 'uaddr' to be set to NULL when vm_memory_attributes=0, which
SNP_LAUNCH_UPDATE will then use to determine when it should/shouldn't
copy in data from a separate memory location. Continue to enforce
non-NULL for the original vm_memory_attributes=1 case.

Signed-off-by: Michael Roth <[email protected]>
[Added src_page check in error handling path when the firmware command fails]
[Dropped ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES]
Signed-off-by: Ackerley Tng <[email protected]>
---
 Documentation/virt/kvm/x86/amd-memory-encryption.rst | 19 +++++++++++++++----
 arch/x86/kvm/svm/sev.c                               | 18 +++++++++++++-----
 virt/kvm/kvm_main.c                                  |  1 +
 3 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/Documentation/virt/kvm/x86/amd-memory-encryption.rst 
b/Documentation/virt/kvm/x86/amd-memory-encryption.rst
index b2395dd4769de..3b9f36a55a95b 100644
--- a/Documentation/virt/kvm/x86/amd-memory-encryption.rst
+++ b/Documentation/virt/kvm/x86/amd-memory-encryption.rst
@@ -503,7 +503,12 @@ secrets.
 
 It is required that the GPA ranges initialized by this command have had the
 KVM_MEMORY_ATTRIBUTE_PRIVATE attribute set in advance. See the documentation
-for KVM_SET_MEMORY_ATTRIBUTES for more details on this aspect.
+for KVM_SET_MEMORY_ATTRIBUTES/KVM_SET_MEMORY_ATTRIBUTES2 for more details on
+this aspect. If running with kvm.vm_memory_attributes=0 (to allow for
+guest_memfd to handle memory attributes and allow for in-place conversion of
+pages between shared/private), the 'PRESERVED' flag/content mode (which is
+only available via the KVM_SET_MEMORY_ATTRIBUTES2 interface) must be used
+when setting the range to private prior to issuing this ioctl.
 
 Upon success, this command is not guaranteed to have processed the entire
 range requested. Instead, the ``gfn_start``, ``uaddr``, and ``len`` fields of
@@ -511,9 +516,15 @@ range requested. Instead, the ``gfn_start``, ``uaddr``, 
and ``len`` fields of
 remaining range that has yet to be processed. The caller should continue
 calling this command until those fields indicate the entire range has been
 processed, e.g. ``len`` is 0, ``gfn_start`` is equal to the last GFN in the
-range plus 1, and ``uaddr`` is the last byte of the userspace-provided source
-buffer address plus 1. In the case where ``type`` is 
KVM_SEV_SNP_PAGE_TYPE_ZERO,
-``uaddr`` will be ignored completely.
+range plus 1, and ``uaddr`` (if specified) is the last byte of the
+userspace-provided source buffer address plus 1.
+
+In the case where ``type`` is KVM_SEV_SNP_PAGE_TYPE_ZERO, ``uaddr`` will be
+ignored completely. Otherwise, ``uaddr`` is required if
+kvm.vm_memory_attributes=0 and optional if kvm.vm_memory_attributes=1, since
+in the latter case guest memory can be initialized directly from userspace
+prior to converting it to private and passing the GPA range on to this
+interface.
 
 Parameters (in): struct  kvm_sev_snp_launch_update
 
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index c2126b3c30724..bf10d24907a00 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2343,7 +2343,15 @@ static int sev_gmem_post_populate(struct kvm *kvm, gfn_t 
gfn, kvm_pfn_t pfn,
        int level;
        int ret;
 
-       if (WARN_ON_ONCE(sev_populate_args->type != KVM_SEV_SNP_PAGE_TYPE_ZERO 
&& !src_page))
+       /*
+        * For vm_memory_attributes=1, in-place conversion/population is not
+        * supported, so the initial contents necessarily need to come from a
+        * separate src address. For vm_memory_attributes=0, this isn't
+        * necessarily the case, since the pages may have been populated
+        * directly from userspace before calling KVM_SEV_SNP_LAUNCH_UPDATE.
+        */
+       if (vm_memory_attributes &&
+           sev_populate_args->type != KVM_SEV_SNP_PAGE_TYPE_ZERO && !src_page)
                return -EINVAL;
 
        ret = snp_lookup_rmpentry((u64)pfn, &assigned, &level);
@@ -2390,7 +2398,7 @@ static int sev_gmem_post_populate(struct kvm *kvm, gfn_t 
gfn, kvm_pfn_t pfn,
         */
        if (ret && !snp_page_reclaim(kvm, pfn) &&
            sev_populate_args->type == KVM_SEV_SNP_PAGE_TYPE_CPUID &&
-           sev_populate_args->fw_error == SEV_RET_INVALID_PARAM) {
+           sev_populate_args->fw_error == SEV_RET_INVALID_PARAM && src_page) {
                void *src_vaddr = kmap_local_page(src_page);
                void *dst_vaddr = kmap_local_pfn(pfn);
 
@@ -2422,8 +2430,8 @@ static int snp_launch_update(struct kvm *kvm, struct 
kvm_sev_cmd *argp)
        if (copy_from_user(&params, u64_to_user_ptr(argp->data), 
sizeof(params)))
                return -EFAULT;
 
-       pr_debug("%s: GFN start 0x%llx length 0x%llx type %d flags %d\n", 
__func__,
-                params.gfn_start, params.len, params.type, params.flags);
+       pr_debug("%s: GFN start 0x%llx length 0x%llx type %d flags %d src 
%llx\n", __func__,
+                params.gfn_start, params.len, params.type, params.flags, 
params.uaddr);
 
        if (!params.len || !PAGE_ALIGNED(params.len) || params.flags ||
            (params.type != KVM_SEV_SNP_PAGE_TYPE_NORMAL &&
@@ -2479,7 +2487,7 @@ static int snp_launch_update(struct kvm *kvm, struct 
kvm_sev_cmd *argp)
 
        params.gfn_start += count;
        params.len -= count * PAGE_SIZE;
-       if (params.type != KVM_SEV_SNP_PAGE_TYPE_ZERO)
+       if (src && params.type != KVM_SEV_SNP_PAGE_TYPE_ZERO)
                params.uaddr += count * PAGE_SIZE;
 
        if (copy_to_user(u64_to_user_ptr(argp->data), &params, sizeof(params)))
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index ba195bb239aaa..3bf212fd99193 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -105,6 +105,7 @@ module_param(allow_unsafe_mappings, bool, 0444);
 #ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES
 bool vm_memory_attributes = true;
 module_param(vm_memory_attributes, bool, 0444);
+EXPORT_SYMBOL_FOR_KVM_INTERNAL(vm_memory_attributes);
 #endif
 DEFINE_STATIC_CALL_RET0(__kvm_get_memory_attributes, 
kvm_get_memory_attributes_t);
 EXPORT_SYMBOL_FOR_KVM_INTERNAL(STATIC_CALL_KEY(__kvm_get_memory_attributes));

-- 
2.54.0.545.g6539524ca2-goog



Reply via email to