From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tim Deegan Subject: Re: [PATCH v2 4/6] x86/NPT: don't walk entire page tables when changing types on a range Date: Fri, 25 Apr 2014 10:43:21 +0200 Message-ID: <20140425084321.GA33897@deinos.phlegethon.org> References: <53567B1C020000780000AB8C@nat28.tlf.novell.com> <53567D16020000780000ABD6@nat28.tlf.novell.com> <20140424162541.GK48969@deinos.phlegethon.org> <535A192E020000780000C234@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1WdbjK-0004XP-7t for xen-devel@lists.xenproject.org; Fri, 25 Apr 2014 08:43:34 +0000 Content-Disposition: inline In-Reply-To: <535A192E020000780000C234@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: Kevin Tian , Keir Fraser , suravee.suthikulpanit@amd.com, Eddie Dong , Jun Nakajima , xen-devel , Boris Ostrovsky List-Id: xen-devel@lists.xenproject.org At 07:13 +0100 on 25 Apr (1398406414), Jan Beulich wrote: > >>> On 24.04.14 at 18:25, wrote: > > At 13:30 +0100 on 22 Apr (1398169846), Jan Beulich wrote: > >> This builds on the fact that in order for no NPF VM exit to occur, > >> _PAGE_USER must always be set. I.e. by clearing the flag we can force a > >> VM exit allowing us to do similar lazy type changes as on EPT. > >> > >> That way, the generic entry-wise code can go away, and we could remove > >> the range restriction in enforced on HVMOP_track_dirty_vram for XSA-27. > >> > >> Signed-off-by: Jan Beulich > >> > >> --- a/xen/arch/x86/hvm/svm/svm.c > >> +++ b/xen/arch/x86/hvm/svm/svm.c > >> @@ -2557,7 +2557,16 @@ void svm_vmexit_handler(struct cpu_user_ > >> perfc_incra(svmexits, VMEXIT_NPF_PERFC); > >> if ( cpu_has_svm_decode ) > >> v->arch.hvm_svm.cached_insn_len = vmcb->guest_ins_len & 0xf; > >> - svm_do_nested_pgfault(v, regs, vmcb->exitinfo1, vmcb->exitinfo2); > >> + rc = p2m_npt_fault(vmcb->exitinfo2); > > > > Can we limit the classes of fault that we need to check for > > recalc on? e.g. if bit 0 of the error code is clear, then the page is > > not mapped at all. > > That's a good suggestion (albeit I'm not sure if there really are > frequent faults with P clear - with paging perhaps, but that's a slow > path anyway). Ideally we would have an indication that the fault > was because of the U bit clear, but sadly that doesn't exist. Yeah, that's a shame. > > I'm unsure about having two different fixup paths here anyway -- one > > for log-dirty and one for everything else (PoD, sharing &c). This > > call should probably go _inside_ svm_do_nested_pgfault() at least. > > I intentionally kept it separate, so it's not getting farther away than > necessary from how EPT handling is structured. Hmm. Well, in that case I guess it's OK here. The alternative would be to put it right into the HVM nested fault handler and replumb the ERPT path to go the same way, but I can see that would be inefficient... > > Also, maybe it wants a more descriptive name than p2m_npt_fault(). > > p2m_pt_hnadle_misconfig(), to match the EPT equivalent? > > I first considered naming it that way, but it's not really a mis- > configuration. But if you think that's a better name despite not > describing what it really does, I'm okay changing it... I think I do prefer it, but maybe we can find a better name for both of them: {p2m_pt,ept}_handle_deferred_changes()? > > This looks like it could be merged with the EPT equivalent. Or would > > that be too unwieldy? > > I considered unifying both this and/or the helpers of it, but decided > that the result would be uglier (too many parameters just to tell one > from the other) than the duplication that results from this approach. OK, fair enough. Tim.