From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH v3 05/13] x86/altp2m: basic data structures and support routines. Date: Fri, 3 Jul 2015 17:22:58 +0100 Message-ID: <5596B6E2.7010601@citrix.com> References: <1435774177-6345-1-git-send-email-edmund.h.white@intel.com> <1435774177-6345-6-git-send-email-edmund.h.white@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1435774177-6345-6-git-send-email-edmund.h.white@intel.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: Ed White , xen-devel@lists.xen.org Cc: Ravi Sahita , Wei Liu , George Dunlap , Ian Jackson , Tim Deegan , Jan Beulich , tlengyel@novetta.com, Daniel De Graaf List-Id: xen-devel@lists.xenproject.org On 01/07/15 19:09, Ed White wrote: > Add the basic data structures needed to support alternate p2m's and > the functions to initialise them and tear them down. > > Although Intel hardware can handle 512 EPTP's per hardware thread > concurrently, only 10 per domain are supported in this patch for > performance reasons. > > The iterator in hap_enable() does need to handle 512, so that is now > uint16_t. > > This change also splits the p2m lock into one lock type for altp2m's > and another type for all other p2m's. The purpose of this is to place > the altp2m list lock between the types, so the list lock can be > acquired whilst holding the host p2m lock. > > Signed-off-by: Ed White Only some style issues. Otherwise, Reviewed-by: Andrew Cooper > diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c > index 6faf3f4..f21d34d 100644 > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -6502,6 +6504,25 @@ enum hvm_intblk nhvm_interrupt_blocked(struct vcpu *v) > return hvm_funcs.nhvm_intr_blocked(v); > } > > +void ap2m_vcpu_update_eptp(struct vcpu *v) > +{ > + if (hvm_funcs.ap2m_vcpu_update_eptp) spaces inside brackets > + hvm_funcs.ap2m_vcpu_update_eptp(v); > +} > + > +void ap2m_vcpu_update_vmfunc_ve(struct vcpu *v) > +{ > + if (hvm_funcs.ap2m_vcpu_update_vmfunc_ve) spaces inside brackets > + hvm_funcs.ap2m_vcpu_update_vmfunc_ve(v); > +} > + > +bool_t ap2m_vcpu_emulate_ve(struct vcpu *v) > +{ > + if (hvm_funcs.ap2m_vcpu_emulate_ve) spaces inside brackets > + return hvm_funcs.ap2m_vcpu_emulate_ve(v); > + return 0; > +} > + > /* > * Local variables: > * mode: C > diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c > index d0d3f1e..c00282c 100644 > --- a/xen/arch/x86/mm/hap/hap.c > +++ b/xen/arch/x86/mm/hap/hap.c > @@ -459,7 +459,7 @@ void hap_domain_init(struct domain *d) > int hap_enable(struct domain *d, u32 mode) > { > unsigned int old_pages; > - uint8_t i; > + uint16_t i; > int rv = 0; > > domain_pause(d); > @@ -498,6 +498,24 @@ int hap_enable(struct domain *d, u32 mode) > goto out; > } > > + /* Init alternate p2m data */ > + if ( (d->arch.altp2m_eptp = alloc_xenheap_page()) == NULL ) > + { > + rv = -ENOMEM; > + goto out; > + } > + > + for (i = 0; i < MAX_EPTP; i++) > + d->arch.altp2m_eptp[i] = INVALID_MFN; > + > + for (i = 0; i < MAX_ALTP2M; i++) { braces > + rv = p2m_alloc_table(d->arch.altp2m_p2m[i]); > + if ( rv != 0 ) > + goto out; > + } > + > + d->arch.altp2m_active = 0; > + > /* Now let other users see the new mode */ > d->arch.paging.mode = mode | PG_HAP_enable; > > @@ -510,6 +528,17 @@ void hap_final_teardown(struct domain *d) > { > uint8_t i; > > + d->arch.altp2m_active = 0; > + > + if ( d->arch.altp2m_eptp ) { braces > + free_xenheap_page(d->arch.altp2m_eptp); > + d->arch.altp2m_eptp = NULL; > + } > + > + for (i = 0; i < MAX_ALTP2M; i++) { braces > diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c > index 1fd1194..58d4951 100644 > --- a/xen/arch/x86/mm/p2m.c > +++ b/xen/arch/x86/mm/p2m.c > @@ -35,6 +35,7 @@ > #include /* ept_p2m_init() */ > #include > #include > +#include > #include > #include > > @@ -183,6 +184,43 @@ static void p2m_teardown_nestedp2m(struct domain *d) > } > } > > +static void p2m_teardown_altp2m(struct domain *d) > +{ > + uint8_t i; A plain unsigned int here would suffice. It also looks curios as you use uint16 for the same iteration elsewhere. > + struct p2m_domain *p2m; > + > + for (i = 0; i < MAX_ALTP2M; i++) spaces inside brackets > + { > + if ( !d->arch.altp2m_p2m[i] ) > + continue; > + p2m = d->arch.altp2m_p2m[i]; > + p2m_free_one(p2m); > + d->arch.altp2m_p2m[i] = NULL; > + } > +} > + > +static int p2m_init_altp2m(struct domain *d) > +{ > + uint8_t i; > + struct p2m_domain *p2m; > + > + mm_lock_init(&d->arch.altp2m_lock); > + for (i = 0; i < MAX_ALTP2M; i++) spaces inside brackets