From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH v3 3/6] KVM: nVMX: Load nEPT state after EFER Date: Mon, 02 Sep 2013 20:38:11 +0200 Message-ID: <5224DB13.8030305@siemens.com> References: <20130902131612.GT22899@redhat.com> <5224D1C6.7050706@siemens.com> <20130902180935.GJ10142@redhat.com> <5224D6E2.5040605@siemens.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: Paolo Bonzini , kvm , Xiao Guangrong , Jun Nakajima , Yang Zhang , Arthur Chunqi Li To: Gleb Natapov Return-path: Received: from david.siemens.de ([192.35.17.14]:24262 "EHLO david.siemens.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758824Ab3IBSiV (ORCPT ); Mon, 2 Sep 2013 14:38:21 -0400 In-Reply-To: <5224D6E2.5040605@siemens.com> Sender: kvm-owner@vger.kernel.org List-ID: On 2013-09-02 20:20, Jan Kiszka wrote: > On 2013-09-02 20:09, Gleb Natapov wrote: >> On Mon, Sep 02, 2013 at 07:58:30PM +0200, Jan Kiszka wrote: >>> On 2013-09-02 15:16, Gleb Natapov wrote: >>>> On Thu, Aug 08, 2013 at 04:26:30PM +0200, Jan Kiszka wrote: >>>>> We need to update EFER.NX before building the nEPT state via >>>>> nested_ept_init_mmu_context. Otherwise, we risk to create an MMU context >>>>> that claims to have NX disabled while the guest EPT used NX. This will >>>>> cause spurious faults for L2. >>>>> >>>> Hmm, I do not see how nested ept mmu depends on guests EFER.NX setting. >>>> It just sets mmu->nx to true. >>> >>> Don't ask me for the details behind this, but update_permission_bitmask >>> called by kvm_init_shadow_ept_mmu is using it e.g. And the >> It uses it only in !ept case though and never looks at a guest setting >> as far as I can tell. Is it possible that this was an artifact of all >> nEPT code and the latest one does not need this patch? > > Hmm, possibly. Let me recheck, hope I can find the reproduction pattern > again... Yeah, drop it. Things work fine without it, and I just found an old version of nEPT where kvm_init_shadow_EPT_mmu did this: context->nx = is_nx(vcpu); /* TODO: ? */ Obviously, this patch was a workaround for that issue. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux