Re: [Xen-devel] Re: xen: use paging_mode_hap()

[Tim Deegan <Tim.Deegan@xxxxxxxxxx>]

At 08:53 +0100 on 07 Apr (1270630406), Xu, Dongxiao wrote:
> We found that guests with Intel EPT could not boot up with latest
> upstream Xen.
> 
> The root cause is: 
> When calling p2m_init() function, the PG_HAP_enable hasn't been
> written to paging mode, which Tim has pointed out in this mail.

True - I thought that reshuffling hap_enable() would be good enough, but
of course p2m_init() gets called before any hap code, so this patch is
needed.

Acked-by: Tim Deegan <Tim.Deegan@xxxxxxxxxx>

> After adding the following patch, HVM guest with hap=1 works well.
> 
> Signed-off-by: Dongxiao Xu <dongxiao.xu@xxxxxxxxx>
> 
> diff -r adce8bc43fcc xen/arch/x86/mm/p2m.c
> --- a/xen/arch/x86/mm/p2m.c     Tue Apr 06 07:16:47 2010 +0100
> +++ b/xen/arch/x86/mm/p2m.c     Wed Apr 07 23:48:14 2010 +0800
> @@ -1712,7 +1712,7 @@ int p2m_init(struct domain *d)
>      p2m->get_entry_current = p2m_gfn_to_mfn_current;
>      p2m->change_entry_type_global = p2m_change_type_global;
> 
> -    if ( is_hvm_domain(d) && paging_mode_hap(d) &&
> +    if ( is_hvm_domain(d) && d->arch.hvm_domain.hap_enabled &&
>           (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) )
>          ept_p2m_init(d);
> 
> 
> Thanks,
> Dongxiao
> 
> 
> Christoph Egger wrote:
> > On Wednesday 31 March 2010 11:29:32 Tim Deegan wrote:
> >> At 16:51 +0100 on 30 Mar (1269967863), Christoph Egger wrote:
> >>> Hi!
> >>> 
> >>> Attached patch makes xen consequently use paging_mode_hap().
> >>> 
> >>> Signed-off-by: Christoph Egger <Christoph.Egger@xxxxxxx>
> >>> 
> >>> P.S.: This is a resend. Needs ack from Tim.
> >> 
> >> Nack, at least in its current form - paging_mode_hap(d) is not the
> >> same as d->arch.hvm_domain.hap_enabled.  Especially in paging.c,
> >> where we test hap_enabled to gate whether we should call the
> >> function that makes paging_mode_hap(d) true! 
> >> 
> >> I think the p2m.c changes are wrong too, since the p2m init code is
> >> called from hap_enable before it writes PG_HAP_enable to the paging
> >> mode.  (That's probably a bug in its own right; it should set the
> >> mode and then call the p2m code, and shadow should do likewise, and
> >> then these changes would be correct and desirable).
> >> 
> >> The other substitutions are probably OK.
> >> 
> >> Cheers,
> >> 
> >> Tim.
> > 
> > Thanks for you comments. Attached is my updated patch.
> > 
> > Signed-off-by: Christoph Egger <Christoph.Egger@xxxxxxx>
> 

-- 
Tim Deegan <Tim.Deegan@xxxxxxxxxx>
Principal Software Engineer, XenServer Engineering
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel