From mboxrd@z Thu Jan 1 00:00:00 1970 From: George Dunlap Subject: Re: [V10 PATCH 14/23] PVH xen: additional changes to support PVH guest creation and execution. Date: Wed, 21 Aug 2013 09:37:35 +0100 Message-ID: <52147C4F.9040005@eu.citrix.com> References: <1374631171-15224-1-git-send-email-mukesh.rathor@oracle.com> <1374631171-15224-15-git-send-email-mukesh.rathor@oracle.com> <20130820143200.5f631769@mantra.us.oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20130820143200.5f631769@mantra.us.oracle.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: Mukesh Rathor Cc: "xen-devel@lists.xensource.com" , "keir.xen@gmail.com" List-Id: xen-devel@lists.xenproject.org On 20/08/13 22:32, Mukesh Rathor wrote: > On Tue, 20 Aug 2013 15:13:10 +0100 > George Dunlap wrote: > >> On Wed, Jul 24, 2013 at 2:59 AM, Mukesh Rathor >> wrote: >>> diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c >>> index bff05d9..19a085c 100644 >>> --- a/xen/arch/x86/mm/hap/hap.c >>> +++ b/xen/arch/x86/mm/hap/hap.c >>> @@ -639,7 +639,9 @@ static void hap_update_cr3(struct vcpu *v, int >>> do_locking) const struct paging_mode * >>> hap_paging_get_mode(struct vcpu *v) >>> { >>> - return !hvm_paging_enabled(v) ? &hap_paging_real_mode : >>> + /* PVH 32bitfixme. */ >>> + return is_pvh_vcpu(v) ? &hap_paging_long_mode : >>> + !hvm_paging_enabled(v) ? &hap_paging_real_mode : >>> hvm_long_mode_enabled(v) ? &hap_paging_long_mode : >>> hvm_pae_enabled(v) ? &hap_paging_pae_mode : >>> &hap_paging_protected_mode; >> This shouldn't be necessary, right? The PVH code should ensure that >> for 64-bit PVH guests, hvm_long_mode_enabled() is always true, right? > Right, 64bit PVH always will be in long mode. However, with 32bit PVH, > this check will change, so best to leave it here. How will it change? In that case, won't hvm_long_mode() return false, but hvm_pae_enabled() return true, and you'll get hap_paging_pae_mode (which I assume is what you would want)? In any case, if it's not needed now, it shouldn't be introduced now. I've taken it out of my copy. -George