Re: [PATCH v12 18/29] KVM: SEV: Use a VMSA physical address variable for populating VMCB

[Paolo Bonzini]

On Tue, Apr 16, 2024 at 4:25 PM Tom Lendacky <thomas.lendacky@xxxxxxx> wrote:
>
> On 4/16/24 06:53, Paolo Bonzini wrote:
> > On Sat, Mar 30, 2024 at 10:01 PM Paolo Bonzini <pbonzini@redhatcom> 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.

Oooh, I somehow thought that

+ target_svm->sev_es.snp_vmsa_gpa = INVALID_PAGE;
+ target_svm->sev_es.snp_ap_create = true;

was in svm_create_vcpu().

So there should be separate "snp_ap_waiting_for_reset" and
"snp_has_guest_vmsa" flags. The latter is set once in
__sev_snp_update_protected_guest_state and is what governs whether the
VMSA page was allocated or just refcounted.

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

Yes, agreed on both counts.

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

Yes, and once you understand that I was thinking of a set-once flag
"snp_has_guest_vmsa" it should all make a lot more sense.

Paolo