linux-kernel - Re: [PATCH v4 00/11] KVM: VMX: Clean up Hyper-V PV TLB flush

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <YEbM0SYpnfcTnfdA@google.com>
Date:   Mon, 8 Mar 2021 17:18:09 -0800
From:   Sean Christopherson <seanjc@...gle.com>
To:     Paolo Bonzini <pbonzini@...hat.com>
Cc:     Vitaly Kuznetsov <vkuznets@...hat.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        Joerg Roedel <joro@...tes.org>, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Sean Christopherson <sean.j.christopherson@...el.com>
Subject: Re: [PATCH v4 00/11] KVM: VMX: Clean up Hyper-V PV TLB flush

On Mon, Mar 08, 2021, Paolo Bonzini wrote:
> On 05/03/21 19:31, Sean Christopherson wrote:
> > Sean Christopherson (11):
> >    KVM: x86: Get active PCID only when writing a CR3 value

...

> Huh, I was sure I had queued this already for 5.12.  Well, done so now.

Maybe this series is cursed.  The first patch got mangled and broke SME.
It shows up as two commits with the same changelog, so maybe you intended to
split the patch and things went sideways?

Anyways, commit a16241ae56fa ("KVM: x86: Get active PCID only when writing a
CR3 value") breaks SME and PCID.  The kvm/queue code looks like this:


	cr3 = __sme_set(root_hpa);
	if (npt_enabled) {
		svm->vmcb->control.nested_cr3 = root_hpa;
		vmcb_mark_dirty(svm->vmcb, VMCB_NPT);

		/* Loading L2's CR3 is handled by enter_svm_guest_mode.  */
		if (!test_bit(VCPU_EXREG_CR3, (ulong *)&vcpu->arch.regs_avail))
			return;
		cr3 = vcpu->arch.cr3;
	}

	svm->vmcb->save.cr3 = cr3;
	vmcb_mark_dirty(svm->vmcb, VMCB_CR);

but it should look like this:

	if (npt_enabled) {
		svm->vmcb->control.nested_cr3 = __sme_set(root);
		vmcb_mark_dirty(svm->vmcb, VMCB_NPT);

		/* Loading L2's CR3 is handled by enter_svm_guest_mode.  */
		if (!test_bit(VCPU_EXREG_CR3, (ulong *)&vcpu->arch.regs_avail))
			return;
		cr3 = vcpu->arch.cr3;
	} else if (vcpu->arch.mmu->shadow_root_level >= PT64_ROOT_4LEVEL) {
		cr3 = __sme_set(root);
	} else {
		cr3 = root;
	}

	svm->vmcb->save.cr3 = cr3;
	vmcb_mark_dirty(svm->vmcb, VMCB_CR);

I'll generate a delta patch, and test and post, just in case there is other
stuff that got lost.