| Message ID | 20230127025237.269680-4-jarkko@profian.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC,1/8] KVM: SVM: fix: calculate end instead of passing size | expand |
On 1/26/23 20:52, Jarkko Sakkinen wrote: > From: Tom Dohrmann <erbse.13@gmx.de> > > When doing a launch update for a CPUID page the firmware checks that the values > conform to the policy laid out in the processor programming manual. If the > values don't conform, the firmware will return SEV_RET_INVALID_PARAM. > In addition to returning an error the firmware will choose some acceptable > values and write them back to the page that was used for the launch update, so > that the VMM can inspect the changes and try again with the corrected values. > This is specified in section 8.17.2.6 in the SEV-SNP Firmware ABI spec. > Because launch updates are always done on the private UPM mappings, the pages > are first copied from the shared mappings to the private mappings. When the > firmware corrects the values, the corrected values are in the private mappings, > inaccessible to userspace. In order to make the corrected values accessible to > userspace, the page containing them must be copied from the private mappings > back to the shared mappings. > > [jarkko@profian.com: fixed checkpatch.pl errors] > Link: https://lore.kernel.org/lkml/Y76%2FI6Nrh7xEAAwv@notebook/ > Signed-off-by: Tom Dohrmann <erbse.13@gmx.de> > Signed-off-by: Jarkko Sakkinen <jarkko@profian.com> > --- > arch/x86/kvm/svm/sev.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c > index 6d3162853c33..4a8e552d8cfe 100644 > --- a/arch/x86/kvm/svm/sev.c > +++ b/arch/x86/kvm/svm/sev.c > @@ -2230,6 +2230,23 @@ static int snp_launch_update_gfn_handler(struct kvm *kvm, > pr_err("SEV-SNP launch update failed, ret: 0x%x, fw_error: 0x%x\n", > ret, *error); > snp_page_reclaim(pfns[i]); > + > + /* > + * When invalid CPUID function entries are detected, the firmware > + * corrects these entries. In that case write the page back to > + * userspace. I would additionally add that the firmware does not encrypt the page, which allows the hypervisor to copy the page back to userspace. Thanks, Tom > + */ > + if (params.page_type == SNP_PAGE_TYPE_CPUID && > + *error == SEV_RET_INVALID_PARAM) { > + int ret; > + > + host_rmp_make_shared(pfns[i], PG_LEVEL_4K, true); > + > + ret = kvm_write_guest_page(kvm, gfn, kvaddr, 0, PAGE_SIZE); > + if (ret) > + pr_err("Guest write failed, ret: 0x%x\n", ret); > + } > + > goto e_release; > } > }
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c index 6d3162853c33..4a8e552d8cfe 100644 --- a/arch/x86/kvm/svm/sev.c +++ b/arch/x86/kvm/svm/sev.c @@ -2230,6 +2230,23 @@ static int snp_launch_update_gfn_handler(struct kvm *kvm, pr_err("SEV-SNP launch update failed, ret: 0x%x, fw_error: 0x%x\n", ret, *error); snp_page_reclaim(pfns[i]); + + /* + * When invalid CPUID function entries are detected, the firmware + * corrects these entries. In that case write the page back to + * userspace. + */ + if (params.page_type == SNP_PAGE_TYPE_CPUID && + *error == SEV_RET_INVALID_PARAM) { + int ret; + + host_rmp_make_shared(pfns[i], PG_LEVEL_4K, true); + + ret = kvm_write_guest_page(kvm, gfn, kvaddr, 0, PAGE_SIZE); + if (ret) + pr_err("Guest write failed, ret: 0x%x\n", ret); + } + goto e_release; } }