From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH 4/4] VMX: x86: Only reset MMU when necessary Date: Wed, 12 May 2010 11:11:47 +0300 Message-ID: <4BEA62C3.3090505@redhat.com> References: <4BEA4B2A.9060907@redhat.com> <1273645986-21526-4-git-send-email-sheng@linux.intel.com> <4BEA51C2.5000708@redhat.com> <201005121531.21541.sheng@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Cc: Marcelo Tosatti , kvm@vger.kernel.org To: Sheng Yang Return-path: Received: from mx1.redhat.com ([209.132.183.28]:52429 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753228Ab0ELILw (ORCPT ); Wed, 12 May 2010 04:11:52 -0400 In-Reply-To: <201005121531.21541.sheng@linux.intel.com> Sender: kvm-owner@vger.kernel.org List-ID: On 05/12/2010 10:31 AM, Sheng Yang wrote: > >>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >>> index b59fc67..971a295 100644 >>> --- a/arch/x86/kvm/x86.c >>> +++ b/arch/x86/kvm/x86.c >>> >>> @@ -416,6 +416,10 @@ out: >>> static int __kvm_set_cr0(struct kvm_vcpu *vcpu, unsigned long cr0) >>> { >>> >>> + unsigned long old_cr0 = kvm_read_cr0(vcpu); >>> + unsigned long update_bits = X86_CR0_PG | X86_CR0_PE | >>> + X86_CR0_CD | X86_CR0_NW; >>> >> PE doesn't affect paging, CD, NW don't either? >> > Yes, PE can't affect alone. > > Marcelo has commented on CD/NW, because we need to reload pdptrs if they changed, > then we need to reload MMU. > Ah, correct. >> What about WP? >> > How WP would affect? > If cr0.wp=0 then we can have a pte with gpte.rw=0 but spte.rw=1 (since the guest always runs with cr0.wp=1). So we need to reload the mmu to switch page tables. This won't work now, I'll post a patch adding cr0.wp to sp->role. But please add cr0.wp to the set of bits requiring reload so we won't have a regression. >>> @@ -722,6 +730,9 @@ static int set_efer(struct kvm_vcpu *vcpu, u64 efer) >>> >>> vcpu->arch.mmu.base_role.nxe = (efer& EFER_NX)&& !tdp_enabled; >>> >>> + if ((efer ^ old_efer)& EFER_NX) >>> + update_rsvd_bits_mask(vcpu); >>> + >>> >>> return 0; >>> >>> } >>> >> I think it's fine to reset the entire mmu context here, most guests >> won't toggle nx all the time. But it needs to be in patch 3, otherwise >> we have a regression between 3 and 4. >> > OK. Would drop patch 3 and keep mmu reset if you like... > Yes please. -- Do not meddle in the internals of kernels, for they are subtle and quick to panic.