From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753712AbcCJMPF (ORCPT ); Thu, 10 Mar 2016 07:15:05 -0500 Received: from mga02.intel.com ([134.134.136.20]:7476 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751588AbcCJMO7 (ORCPT ); Thu, 10 Mar 2016 07:14:59 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,315,1455004800"; d="scan'208";a="906935961" Subject: Re: [PATCH 1/2] KVM: MMU: fix ept=0/pte.u=0/pte.w=0/CR0.WP=0/CR4.SMEP=1/EFER.NX=0 combo To: Paolo Bonzini , linux-kernel@vger.kernel.org, kvm@vger.kernel.org References: <1457437467-65707-1-git-send-email-pbonzini@redhat.com> <1457437467-65707-2-git-send-email-pbonzini@redhat.com> <56E12FF0.90202@linux.intel.com> <56E147D7.3040409@redhat.com> Cc: stable@vger.kernel.org, Andy Lutomirski From: Xiao Guangrong Message-ID: <56E16527.4020908@linux.intel.com> Date: Thu, 10 Mar 2016 20:14:31 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <56E147D7.3040409@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 03/10/2016 06:09 PM, Paolo Bonzini wrote: > > > On 10/03/2016 09:27, Xiao Guangrong wrote: >>> >> >>> + if (!enable_ept) { >>> + guest_efer |= EFER_NX; >>> + ignore_bits |= EFER_NX; >> >> Update ignore_bits is not necessary i think. > > More precisely, ignore_bits is only needed if guest EFER.NX=0 and we're > not in this CR0.WP=1/CR4.SMEP=0 situation. In theory you could have > guest EFER.NX=1 and host EFER.NX=0. It is not in linux, the kernel always set EFER.NX if CPUID reports it, arch/x86/kernel/head_64.S: 204 /* Setup EFER (Extended Feature Enable Register) */ 205 movl $MSR_EFER, %ecx 206 rdmsr 207 btsl $_EFER_SCE, %eax /* Enable System Call */ 208 btl $20,%edi /* No Execute supported? */ 209 jnc 1f 210 btsl $_EFER_NX, %eax 211 btsq $_PAGE_BIT_NX,early_pmd_flags(%rip) 212 1: wrmsr /* Make changes effective */ So if guest sees NX in its cpuid then host EFER.NX should be 1. > > This is what I came up with (plus some comments :)): > > u64 guest_efer = vmx->vcpu.arch.efer; > u64 ignore_bits = 0; > > if (!enable_ept) { > if (boot_cpu_has(X86_FEATURE_SMEP)) > guest_efer |= EFER_NX; > else if (!(guest_efer & EFER_NX)) > ignore_bits |= EFER_NX; > } Your logic is very right. What my suggestion is we can keep ignore_bits = EFER_NX | EFER_SCE; (needn't conditionally adjust it) because EFER_NX must be the same between guest and host if we switch EFER manually. > My patch is bigger but the resulting code is smaller and easier to follow: > > guest_efer = vmx->vcpu.arch.efer; > if (!enable_ept) > guest_efer |= EFER_NX; > ... > if (...) { > ... > } else { > guest_efer &= ~ignore_bits; > guest_efer |= host_efer & ignore_bits; > } I agreed. :)