On Wed, Dec 14, 2022 at 01:40:02PM -0600, Michael Roth wrote: > From: Vishal Annapurve <vannapurve@xxxxxxxxxx> > > This change adds handling of HVA ranges to copy contents s/This change adds handling of/Handle/ > +static int sev_launch_update_priv_gfn_handler(struct kvm *kvm, > + struct kvm_gfn_range *range, > + struct kvm_sev_cmd *argp) > +{ > + struct sev_data_launch_update_data data; > + struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info; > + gfn_t gfn; > + kvm_pfn_t pfn; > + struct kvm_memory_slot *memslot = range->slot; > + int ret = 0; > + > + data.reserved = 0; > + data.handle = sev->handle; > + > + for (gfn = range->start; gfn < range->end; gfn++) { > + int order; > + void *kvaddr; > + > + ret = kvm_restricted_mem_get_pfn(memslot, gfn, &pfn, &order); > + if (ret) > + return ret; > + > + kvaddr = pfn_to_kaddr(pfn); > + if (!virt_addr_valid(kvaddr)) { > + pr_err("Invalid kvaddr 0x%llx\n", (uint64_t)kvaddr); Is that some debugging help leftover or what is that printk issued for? > + ret = -EINVAL; > + goto e_ret; > + } > + > + ret = kvm_read_guest_page(kvm, gfn, kvaddr, 0, PAGE_SIZE); > + if (ret) { > + pr_err("guest read failed 0x%x\n", ret); > + goto e_ret; > + } > + > + if (!this_cpu_has(X86_FEATURE_SME_COHERENT)) check_for_deprecated_apis: WARNING: arch/x86/kvm/svm/sev.c:602: Do not use this_cpu_has() - use cpu_feature_enabled() instead. > + clflush_cache_range(kvaddr, PAGE_SIZE); > + > + data.len = PAGE_SIZE; > + data.address = __sme_set(pfn << PAGE_SHIFT); > + ret = sev_issue_cmd(kvm, SEV_CMD_LAUNCH_UPDATE_DATA, &data, &argp->error); > + if (ret) > + goto e_ret; > + kvm_release_pfn_clean(pfn); > + } > + kvm_vm_set_region_attr(kvm, range->start, range->end, > + true /* priv_attr */); No need to break that line. > + > +e_ret: > + return ret; > +} > + > +static int sev_launch_update_gfn_handler(struct kvm *kvm, struct kvm_gfn_range *range, > + void *data) > +{ > + struct kvm_sev_cmd *argp = (struct kvm_sev_cmd *)data; > + > + if (kvm_slot_can_be_private(range->slot)) > + return sev_launch_update_priv_gfn_handler(kvm, range, argp); > + > + return sev_launch_update_shared_gfn_handler(kvm, range, argp); > +} > + > +static int sev_launch_update_data(struct kvm *kvm, > + struct kvm_sev_cmd *argp) > +{ > + struct kvm_sev_launch_update_data params; > + > + if (!sev_guest(kvm)) > + return -ENOTTY; > + > + if (copy_from_user(¶ms, (void __user *)(uintptr_t)argp->data, sizeof(params))) > + return -EFAULT; Not gonna check those user-supplied values for sanity? Or is this check if (WARN_ON_ONCE(hva_end <= hva_start)) return -EINVAL; in kvm_vm_do_hva_range_op() enough? Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette