linux-kernel - Re: [PATCH v3 2/7] KVM: x86: nSVM: implement nested LBR virtualization

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 22 Mar 2022 18:53:57 +0200
From:   Maxim Levitsky <mlevitsk@...hat.com>
To:     Paolo Bonzini <pbonzini@...hat.com>, kvm@...r.kernel.org
Cc:     Ingo Molnar <mingo@...hat.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Sean Christopherson <seanjc@...gle.com>,
        Borislav Petkov <bp@...en8.de>,
        "H. Peter Anvin" <hpa@...or.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Jim Mattson <jmattson@...gle.com>, x86@...nel.org,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Joerg Roedel <joro@...tes.org>, linux-kernel@...r.kernel.org,
        Wanpeng Li <wanpengli@...cent.com>
Subject: Re: [PATCH v3 2/7] KVM: x86: nSVM: implement nested LBR
 virtualization

On Wed, 2022-03-09 at 14:00 +0100, Paolo Bonzini wrote:
> On 3/1/22 15:36, Maxim Levitsky wrote:
> > @@ -565,8 +565,19 @@ static void nested_vmcb02_prepare_save(struct vcpu_svm *svm, struct vmcb *vmcb12
> >   		vmcb_mark_dirty(svm->vmcb, VMCB_DR);
> >   	}
> >   
> > -	if (unlikely(svm->vmcb01.ptr->control.virt_ext & LBR_CTL_ENABLE_MASK))
> > +	if (unlikely(svm->lbrv_enabled && (svm->nested.ctl.virt_ext & LBR_CTL_ENABLE_MASK))) {
> > +
> > +		/* Copy LBR related registers from vmcb12,
> > +		 * but make sure that we only pick LBR enable bit from the guest.
> > +		 */
> > +
> > +		svm_copy_lbrs(vmcb12, svm->vmcb);
> > +		svm->vmcb->save.dbgctl &= LBR_CTL_ENABLE_MASK;
> 
> This should be checked against DEBUGCTL_RESERVED_BITS instead in 
> __nested_vmcb_check_save; remember to add dbgctl to struct 
> vmcb_save_area_cached too.

Actually the AMD's manual doesn't specify what happens when this field contains reserved bits.

I did the following test and I see that CPU ignores the reserved bits:


diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index fa7366fab3bd6..6a9f41941d9ea 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -3875,6 +3875,9 @@ static __no_kcsan fastpath_t svm_vcpu_run(struct kvm_vcpu *vcpu)
        if (!static_cpu_has(X86_FEATURE_V_SPEC_CTRL))
                x86_spec_ctrl_set_guest(svm->spec_ctrl, svm->virt_spec_ctrl);
 
+       svm->vmcb->control.virt_ext |= LBR_CTL_ENABLE_MASK;
+       svm->vmcb->save.dbgctl = 0xFFFFFFFFFFFFFFFFUL;
+
        svm_vcpu_enter_exit(vcpu);
 
        /*


The VM booted fine and reading DEBUGCTL in the guest returned 0x3f.

Its true that DEBUGCTL has few more bits that are defined on AMD which
I zero here, but for practical purposes only bit that matters is BTF,
and I decided that for now to leave it alone, as currently KVM doesn't
emulate it correctly anyway when LBRs are not enabled.
(look at svm_set_msr, MSR_IA32_DEBUGCTLMSR)

In theory this bit should be passed through by setting host's DEBUGCTL,
just prior to entry to the guest, when only it is enabled,
without LBR virtualization. I'll fix this later.

Best regards,
	Maxim Levitsky

> 
> Paolo
> 
> > +		svm_update_lbrv(&svm->vcpu);
> > +
> > +	} else if (unlikely(svm->vmcb01.ptr->control.virt_ext & LBR_CTL_ENABLE_MASK)) {
> >   		svm_copy_lbrs(svm->vmcb01.ptr, svm->vmcb);
> > +	}
> >   }