On 4/16/24 06:53, Paolo Bonzini wrote:
On Sat, Mar 30, 2024 at 10:01 PM Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote:
On 3/29/24 23:58, Michael Roth wrote:
From: Tom Lendacky<thomas.lendacky@xxxxxxx>
In preparation to support SEV-SNP AP Creation, use a variable that holds
the VMSA physical address rather than converting the virtual address.
This will allow SEV-SNP AP Creation to set the new physical address that
will be used should the vCPU reset path be taken.
Signed-off-by: Tom Lendacky<thomas.lendacky@xxxxxxx>
Signed-off-by: Ashish Kalra<ashish.kalra@xxxxxxx>
Signed-off-by: Michael Roth<michael.roth@xxxxxxx>
---
I'll get back to this one after Easter, but it looks like Sean had some
objections at https://lore.kernel.org/lkml/ZeCqnq7dLcJI41O9@xxxxxxxxxx/.
Note that AP create is called multiple times per vCPU under OVMF with
and added call by the kernel when booting the APs.
So IIUC the gist of the solution here would be to replace
/* Use the new VMSA */
svm->sev_es.vmsa_pa = pfn_to_hpa(pfn);
svm->vmcb->control.vmsa_pa = svm->sev_es.vmsa_pa;
with something like
/* Use the new VMSA */
__free_page(virt_to_page(svm->sev_es.vmsa));
This should only be called for the page that KVM allocated during vCPU
creation. After that, the VMSA page from an AP create is a guest page
and shouldn't be freed by KVM.
svm->sev_es.vmsa = pfn_to_kaddr(pfn);
svm->vmcb->control.vmsa_pa = __pa(svm->sev_es.vmsa);
and wrap the __free_page() in sev_free_vcpu() with "if
(!svm->sev_es.snp_ap_create)".
This should remove the need for svm->sev_es.vmsa_pa. It is always
equal to svm->vmcb->control.vmsa_pa anyway.
Yeah, a little bit of multiple VMPL support worked its way in there
where the VMSA per VMPL level is maintained.
But I believe that Sean wants a separate KVM object per VMPL level, so
that would disappear anyway (Joerg and I want to get on the PUCK
schedule to talk about multi-VMPL level support soon.)
Also, it's possible to remove
/*
* gmem pages aren't currently migratable, but if this ever
* changes then care should be taken to ensure
* svm->sev_es.vmsa_pa is pinned through some other means.
*/
kvm_release_pfn_clean(pfn);
Removing this here will cause any previous guest VMSA page(s) to remain
pinned, that's the reason for unpinning here. OVMF re-uses the VMSA, but
that isn't a requirement for a firmware, and the kernel will create a
new VMSA page.
if sev_free_vcpu() does
if (svm->sev_es.snp_ap_create) {
__free_page(virt_to_page(svm->sev_es.vmsa));
} else {
put_page(virt_to_page(svm->sev_es.vmsa));
}
and while at it, please reverse the polarity of snp_ap_create and
rename it to snp_ap_created.
The snp_ap_create flag gets cleared once the new VMSA is put in place,
it doesn't remain. So the flag usage will have to be altered in order
for this function to work properly.
Thanks,
Tom
Paolo
