linux-kernel - Re: [PATCH v5 4/5] KVM: VMX: Allow direct access to MSR_IA32_SPEC

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: <e3bb9f32-46fa-c20d-023a-79a1199403e8@amazon.com>
Date:   Thu, 1 Feb 2018 15:28:47 +0100
From:   KarimAllah Ahmed <karahmed@...zon.com>
To:     Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
CC:     Jim Mattson <jmattson@...gle.com>,
        KarimAllah Ahmed <karahmed@...zon.de>,
        kvm list <kvm@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        "the arch/x86 maintainers" <x86@...nel.org>,
        Asit Mallick <asit.k.mallick@...el.com>,
        Arjan Van De Ven <arjan.van.de.ven@...el.com>,
        Dave Hansen <dave.hansen@...el.com>,
        Andi Kleen <ak@...ux.intel.com>,
        "Andrea Arcangeli" <aarcange@...hat.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Tim Chen <tim.c.chen@...ux.intel.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Dan Williams <dan.j.williams@...el.com>,
        Jun Nakajima <jun.nakajima@...el.com>,
        "Paolo Bonzini" <pbonzini@...hat.com>,
        David Woodhouse <dwmw@...zon.co.uk>,
        Greg KH <gregkh@...uxfoundation.org>,
        Andy Lutomirski <luto@...nel.org>,
        Ashok Raj <ashok.raj@...el.com>
Subject: Re: [PATCH v5 4/5] KVM: VMX: Allow direct access to
 MSR_IA32_SPEC_CTRL

On 02/01/2018 03:19 PM, Konrad Rzeszutek Wilk wrote:
> .snip..
>>> +/* Is SPEC_CTRL intercepted for the currently running vCPU? */
>>> +static bool spec_ctrl_intercepted(struct kvm_vcpu *vcpu)
>>> +{
>>> +	unsigned long *msr_bitmap;
>>> +	int f = sizeof(unsigned long);
>>> +
>>> +	if (!cpu_has_vmx_msr_bitmap())
>>> +		return true;
>>> +
>>> +	msr_bitmap = is_guest_mode(vcpu) ?
>>> +			to_vmx(vcpu)->nested.vmcs02.msr_bitmap :
>>> +			to_vmx(vcpu)->vmcs01.msr_bitmap;
>>> +
>>> +	return !!test_bit(MSR_IA32_SPEC_CTRL, msr_bitmap + 0x800 / f);
>>> +}
>>> +
> ..snip..
>>> @@ -3359,6 +3393,34 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>>>   	case MSR_IA32_TSC:
>>>   		kvm_write_tsc(vcpu, msr_info);
>>>   		break;
>>> +	case MSR_IA32_SPEC_CTRL:
>>> +		if (!msr_info->host_initiated &&
>>> +		    !guest_cpuid_has(vcpu, X86_FEATURE_IBRS) &&
>>> +		    !guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL))
>>> +			return 1;
>>> +
>>> +		vmx->spec_ctrl_used = true;
>>> +
>>> +		/* The STIBP bit doesn't fault even if it's not advertised */
>>> +		if (data & ~(SPEC_CTRL_IBRS | SPEC_CTRL_STIBP))
>>> +			return 1;
>>> +
>>> +		vmx->spec_ctrl = data;
>>> +
>>> +		/*
>>> +		 * When it's written (to non-zero) for the first time, pass
>>> +		 * it through. This means we don't have to take the perf
>>
>> .. But only if it is a nested guest (as you have && is_guest_mode).
>>
>> Do you want to update the comment a bit?
>>
>>> +		 * hit of saving it on vmexit for the common case of guests
>>> +		 * that don't use it.
>>> +		 */
>>> +		if (cpu_has_vmx_msr_bitmap() && data &&
>>> +		    spec_ctrl_intercepted(vcpu) &&
>>> +		    is_guest_mode(vcpu))
>>                      ^^^^^^^^^^^^^^^^^^ <=== here
> 
> Would it be perhaps also good to mention the complexity of how
> we ought to be handling L1 and L2 guests in the commit?
> 
> We are all stressed and I am sure some of us haven't gotten much
> sleep - but it can help in say three months when some unluckly new
> soul is trying to understand this and gets utterly confused.

Yup, I will go through the patches and add as much details as possible.

And yes, the is_guest_mode(vcpu) here is inverted :D I blame the late
night :)

> 
>>> +			vmx_disable_intercept_for_msr(
>>> +					vmx->vmcs01.msr_bitmap,
>>> +					MSR_IA32_SPEC_CTRL,
>>> +					MSR_TYPE_RW);
>>> +		break;
>>>   	case MSR_IA32_PRED_CMD:
>>>   		if (!msr_info->host_initiated &&
>>>   		    !guest_cpuid_has(vcpu, X86_FEATURE_IBPB) &&
> 
Amazon Development Center Germany GmbH
Berlin - Dresden - Aachen
main office: Krausenstr. 38, 10117 Berlin
Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger
Ust-ID: DE289237879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B