linux-kernel - Re: [PATCH 6/8] kvm: svm: pass MSR_IA32_SPEC_CTRL and MSR_IA32_PRED

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: <7d96bfb5-99c3-da31-0094-23097e3faec2@redhat.com>
Date:   Tue, 9 Jan 2018 17:08:13 +0100
From:   Paolo Bonzini <pbonzini@...hat.com>
To:     Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
Cc:     linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        rkrcmar@...hat.com, liran.alon@...cle.com, jmattson@...gle.com,
        aliguori@...zon.com, thomas.lendacky@....com, dwmw@...zon.co.uk,
        bp@...en8.de, x86@...nel.org
Subject: Re: [PATCH 6/8] kvm: svm: pass MSR_IA32_SPEC_CTRL and
 MSR_IA32_PRED_CMD down to guest

Oops, I missed these.

On 09/01/2018 15:22, Konrad Rzeszutek Wilk wrote:
>> +	if (have_spec_ctrl) {
>> +		rdmsrl(MSR_IA32_SPEC_CTRL, svm->spec_ctrl);
>> +		if (svm->spec_ctrl != 0)
> Perhaps just :
> 
> 	if (svm->spec_ctrl) ?
> 
> And above too?

These will become != SPEC_CTRL_ENABLE_IBRS or something like that.

>> +			wrmsrl(MSR_IA32_SPEC_CTRL, 0);
>> +	}
>> +	/*
>> +	 * Speculative execution past the above wrmsrl might encounter
>> +	 * an indirect branch and use guest-controlled contents of the
>> +	 * indirect branch predictor; block it.
>> +	 */
>> +	asm("lfence");
> Don't you want this to be part of the if () .. else part?

Not right now, because the processor could speculate that have_spec_ctrl
== 0 and skip the wrmsrl.  After it becomes a static_cpu_has, it could
move inside, but only if the kernel is compiled with static keys enabled.

> Meaning:
> 
> 	if (have_spec_ctrl && svm->spec_ctrl)
> 		wrmsrl(MSR_IA32_SPEC_CTRL, 0);
> 	else
> 		asm("lfence");
> 
> But .. I am missing something - AMD don't expose 0x48. They expose only 0x49.
> 
> That is only the IPBP is needed on AMD? (I haven't actually seen any official
> docs from AMD).

AMD is not exposing 0x48 yet, but they plan to based on my information
from a few weeks ago.

Paolo