From patchwork Mon Aug 21 20:32:06 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?UmFkaW0gS3LEjW3DocWZ?= X-Patchwork-Id: 9913711 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id B25A8602A0 for ; Mon, 21 Aug 2017 20:32:14 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A48E520683 for ; Mon, 21 Aug 2017 20:32:14 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 9991D287D4; Mon, 21 Aug 2017 20:32:14 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,HK_RANDOM_FROM, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 29629287FD for ; Mon, 21 Aug 2017 20:32:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753610AbdHUUcM (ORCPT ); Mon, 21 Aug 2017 16:32:12 -0400 Received: from mx1.redhat.com ([209.132.183.28]:36890 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753041AbdHUUcL (ORCPT ); Mon, 21 Aug 2017 16:32:11 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 6657AC047B8A; Mon, 21 Aug 2017 20:32:11 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 6657AC047B8A Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=rkrcmar@redhat.com Received: from flask (unknown [10.43.2.80]) by smtp.corp.redhat.com (Postfix) with SMTP id 0AE4C17CD7; Mon, 21 Aug 2017 20:32:06 +0000 (UTC) Received: by flask (sSMTP sendmail emulation); Mon, 21 Aug 2017 22:32:06 +0200 Date: Mon, 21 Aug 2017 22:32:06 +0200 From: Radim =?utf-8?B?S3LEjW3DocWZ?= To: Jim Mattson Cc: David Hildenbrand , kvm@vger.kernel.org Subject: Re: [PATCH v5] kvm: vmx: Raise #UD on unsupported RDSEED Message-ID: <20170821203205.GA31206@flask> References: <20170821163858.64280-1-jmattson@google.com> <20170821192640.30817-1-jmattson@google.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20170821192640.30817-1-jmattson@google.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Mon, 21 Aug 2017 20:32:11 +0000 (UTC) Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 2017-08-21 12:26-0700, Jim Mattson: > A guest may not be configured to support RDSEED, even when the host > does. If the guest does not support RDSEED, intercept the instruction > and synthesize #UD. Also clear the "allowed-1" bit for RDSEED exiting > in the IA32_VMX_PROCBASED_CTLS2 MSR. (RDRAND looks the same.) > Signed-off-by: Jim Mattson > --- > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > @@ -5298,6 +5299,9 @@ static u32 vmx_secondary_exec_control(struct vcpu_vmx *vmx) > if (!enable_pml) > exec_control &= ~SECONDARY_EXEC_ENABLE_PML; > > + if (guest_cpuid_has(&vmx->vcpu, X86_FEATURE_RDSEED)) > + exec_control &= ~SECONDARY_EXEC_RDSEED; > + > return exec_control; > } > > @@ -9665,6 +9682,24 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu) > } > } > > + if (vmx_rdseed_supported()) { > + bool rdseed_enabled = guest_cpuid_has(vcpu, X86_FEATURE_RDSEED); > + > + if (rdseed_enabled) > + secondary_exec_ctl &= ~SECONDARY_EXEC_RDSEED; All other CPUID-controlled features use vmx_cpuid_update(), but I would actually prefer to have it in vmx_secondary_exec_control. In any case, combining those two is weird. > + else > + secondary_exec_ctl |= SECONDARY_EXEC_RDSEED; The feature can never be unset here, so we can have just the first branch, thanks. > + > + if (nested) { > + if (rdseed_enabled) > + vmx->nested.nested_vmx_secondary_ctls_high |= > + SECONDARY_EXEC_RDSEED; > + else > + vmx->nested.nested_vmx_secondary_ctls_high &= > + ~SECONDARY_EXEC_RDSEED; > + } > + } I think it would be nicer to generalize that pattern: (We can call it after updating the MSRs too.) ---8<--- Subject: [PATCH] KVM: nVMX: refactor secondary_ctls_high updates We should not enable a VMX feature if its instruction is not in guest CPUID or not provided by hardware. The change allows us to easily add more features. RDTSCP will always get configured with CPUID, so there is no need to set it from the beginning, just like INVPCID. Signed-off-by: Radim Krčmář --- arch/x86/kvm/vmx.c | 46 +++++++++++++++++++++------------------------- 1 file changed, 21 insertions(+), 25 deletions(-) diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 2b92c2de2b3a..7e2b33e0948d 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -2810,7 +2810,6 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx) vmx->nested.nested_vmx_secondary_ctls_high &= SECONDARY_EXEC_RDRAND | SECONDARY_EXEC_RDSEED | SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES | - SECONDARY_EXEC_RDTSCP | SECONDARY_EXEC_DESC | SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE | SECONDARY_EXEC_APIC_REGISTER_VIRT | @@ -9623,25 +9622,28 @@ static void nested_vmx_cr_fixed1_bits_update(struct kvm_vcpu *vcpu) #undef cr4_fixed1_update } +/* + * Update MSR_IA32_VMX_PROCBASED_CTLS2 according to CPUID. Selected features + * are enabled iff they are enabled in CPUID and supported by the host. + */ +static void nested_vmx_secondary_ctls_high_update(struct kvm_vcpu *vcpu, + u32 host_secondary_exec_ctl) +{ + u32 mask = SECONDARY_EXEC_RDTSCP | SECONDARY_EXEC_ENABLE_INVPCID; + + vcpu->vmx.nested.nested_vmx_secondary_ctls_high &= ~mask + vcpu->vmx.nested.nested_vmx_secondary_ctls_high |= + host_secondary_exec_ctl & mask; +} + static void vmx_cpuid_update(struct kvm_vcpu *vcpu) { struct vcpu_vmx *vmx = to_vmx(vcpu); u32 secondary_exec_ctl = vmx_secondary_exec_control(vmx); - if (vmx_rdtscp_supported()) { - bool rdtscp_enabled = guest_cpuid_has(vcpu, X86_FEATURE_RDTSCP); - if (!rdtscp_enabled) - secondary_exec_ctl &= ~SECONDARY_EXEC_RDTSCP; - - if (nested) { - if (rdtscp_enabled) - vmx->nested.nested_vmx_secondary_ctls_high |= - SECONDARY_EXEC_RDTSCP; - else - vmx->nested.nested_vmx_secondary_ctls_high &= - ~SECONDARY_EXEC_RDTSCP; - } - } + if (vmx_rdtscp_supported() && + !guest_cpuid_has(vcpu, X86_FEATURE_RDTSCP)) + secondary_exec_ctl &= ~SECONDARY_EXEC_RDTSCP; if (vmx_invpcid_supported()) { /* Exposing INVPCID only when PCID is exposed */ @@ -9653,15 +9655,6 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu) secondary_exec_ctl &= ~SECONDARY_EXEC_ENABLE_INVPCID; guest_cpuid_clear(vcpu, X86_FEATURE_INVPCID); } - - if (nested) { - if (invpcid_enabled) - vmx->nested.nested_vmx_secondary_ctls_high |= - SECONDARY_EXEC_ENABLE_INVPCID; - else - vmx->nested.nested_vmx_secondary_ctls_high &= - ~SECONDARY_EXEC_ENABLE_INVPCID; - } } if (cpu_has_secondary_exec_ctrls()) @@ -9674,8 +9667,11 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu) to_vmx(vcpu)->msr_ia32_feature_control_valid_bits &= ~FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX; - if (nested_vmx_allowed(vcpu)) + if (nested_vmx_allowed(vcpu)) { nested_vmx_cr_fixed1_bits_update(vcpu); + nested_vmx_secondary_ctls_high_update(vcpu, secondary_exec_ctl); + } + } static void vmx_set_supported_cpuid(u32 func, struct kvm_cpuid_entry2 *entry)