Re: [PATCH 3/4] KVM: x86: SVM: use vmcb01 in avic_init_vmcb

[Sean Christopherson <seanjc@google.com>]

Just "KVM: SVM:" for the shortlog, please.

On Tue, Mar 01, 2022, Maxim Levitsky wrote:
> Out of precation use vmcb01 when enabling host AVIC.
> No functional change intended.
> 
> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> ---
>  arch/x86/kvm/svm/avic.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
> index e23159f3a62ba..9656e192c646b 100644
> --- a/arch/x86/kvm/svm/avic.c
> +++ b/arch/x86/kvm/svm/avic.c
> @@ -167,7 +167,7 @@ int avic_vm_init(struct kvm *kvm)
>  
>  void avic_init_vmcb(struct vcpu_svm *svm)
>  {
> -	struct vmcb *vmcb = svm->vmcb;
> +	struct vmcb *vmcb = svm->vmcb01.ptr;

I don't like this change.  It's not bad code, but it'll be confusing because it
implies that it's legal for svm->vmcb to be something other than svm->vmcb01.ptr
when this is called.

If we want to guard AVIC, I'd much rather we do something like:

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 7038c76fa841..dcc856bd628d 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -992,8 +992,12 @@ static inline void init_vmcb_after_set_cpuid(struct kvm_vcpu *vcpu)
 static void init_vmcb(struct kvm_vcpu *vcpu)
 {
        struct vcpu_svm *svm = to_svm(vcpu);
-       struct vmcb_control_area *control = &svm->vmcb->control;
-       struct vmcb_save_area *save = &svm->vmcb->save;
+       struct vmcb *vmcb = svm->vmcb01.ptr;
+       struct vmcb_control_area *control = &vmcb->control;
+       struct vmcb_save_area *save = &vmcb->save;
+
+       if (WARN_ON_ONCE(vmcb != svm->vmcb))
+               svm_leave_nested(vcpu);

        svm_set_intercept(svm, INTERCEPT_CR0_READ);
        svm_set_intercept(svm, INTERCEPT_CR3_READ);


On a related topic, init_vmcb_after_set_cpuid() is broken for nested, it needs to
play nice with being called when svm->vmcb == &svm->nested.vmcb02, e.g. update
vmcb01 and re-merge (or just recalc?) vmcb02's intercepts.

>  	struct kvm_svm *kvm_svm = to_kvm_svm(svm->vcpu.kvm);
>  	phys_addr_t bpa = __sme_set(page_to_phys(svm->avic_backing_page));
>  	phys_addr_t lpa = __sme_set(page_to_phys(kvm_svm->avic_logical_id_table_page));
> -- 
> 2.26.3
>