From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tim Deegan Subject: Re: [V3 PATCH 7/9] x86/hvm: pkeys, add pkeys support for guest_walk_tables Date: Wed, 16 Dec 2015 16:28:27 +0000 Message-ID: <20151216162827.GA65025@deinos.phlegethon.org> References: <1449479780-19146-1-git-send-email-huaitong.han@intel.com> <1449479780-19146-8-git-send-email-huaitong.han@intel.com> <5669C23F.6080203@citrix.com> <567184FF.9030702@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <567184FF.9030702@citrix.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: George Dunlap Cc: "wei.liu2@citrix.com" , "Tian, Kevin" , "Wu, Feng" , "jbeulich@suse.com" , "stefano.stabellini@eu.citrix.com" , "george.dunlap@eu.citrix.com" , "andrew.cooper3@citrix.com" , "Dong, Eddie" , "xen-devel@lists.xen.org" , "Nakajima, Jun" , "Han, Huaitong" , "keir@xen.org" , "ian.jackson@eu.citrix.com" , "ian.campbell@citrix.com" List-Id: xen-devel@lists.xenproject.org Hi, At 15:36 +0000 on 16 Dec (1450280191), George Dunlap wrote: > (hvm_fetch_from_guest_virt() seems to only set PFEC_insn_fetch if nx or > smep are enabled in the guest. This seems inconsistent to me with the > treatment of PFEC_reserved_bit: it seems like > hvm_fetch_from_guest_virt() should always pass in PFEC_insn_fetch, > particularly as guest_walk_tables() will already gate the checks based > on whether nx or smep is enabled in the guest. Tim, you know of any > reason for this?) This code is following the hardware, IIRC: on real CPUs without NX enabled, pagefault error codes will not have that bit set even when caused by instruction fetches. Looking at the SDM (3A.4.7) the current rules are for that bit to be set iff ((PAE && NXE) || SMEP). It seems unfortunate to have this check duplicated between guest_walk_tables() and its callers, so potentially g_w_t() should explicitly _clear_ PFEC_insn_fetch in cases where it should not be returned, and then its callers can set it unconditionally for all instruction fetches. (There are similar rules for the new PK bit, which should not be shown to the guest if CR4.PKE is clear.) Cheers, Tim.