Re: [Xen-devel] [RESEND] nested EPT: fix the handling of nested EPT.

["Li, Liang Z" <liang.z.li@xxxxxxxxx>]

> -----Original Message-----
> From: Tim Deegan [mailto:tim@xxxxxxx]
> Sent: Thursday, June 04, 2015 9:11 PM
> To: Li, Liang Z
> Cc: xen-devel@xxxxxxxxxxxxx; keir@xxxxxxx; jbeulich@xxxxxxxx;
> andrew.cooper3@xxxxxxxxxx; Tian, Kevin; Zhang, Yang Z
> Subject: Re: [RESEND] nested EPT: fix the handling of nested EPT.
> 
> At 05:06 +0800 on 03 Jun (1433307971), Liang Li wrote:
> > If the host EPT entry is changed, the nested EPT should be updated.
> > The current code does not do this, and it's wrong.
> >
> > Reported-by: Tim Deegan <tim@xxxxxxx>
> > Signed-off-by: Liang Li <liang.z.li@xxxxxxxxx>
> > Signed-off-by: Yang Zhang <yang.z.zhang@xxxxxxxxx>
> > ---
> >  xen/arch/x86/mm/p2m-ept.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
> > index 5133eb6..26293a0 100644
> > --- a/xen/arch/x86/mm/p2m-ept.c
> > +++ b/xen/arch/x86/mm/p2m-ept.c
> > @@ -26,6 +26,7 @@
> >  #include <asm/p2m.h>
> >  #include <asm/hvm/vmx/vmx.h>
> >  #include <asm/hvm/vmx/vmcs.h>
> > +#include <asm/hvm/nestedhvm.h>
> >  #include <xen/iommu.h>
> >  #include <asm/mtrr.h>
> >  #include <asm/hvm/cacheattr.h>
> > @@ -1076,6 +1077,9 @@ void ept_sync_domain(struct p2m_domain *p2m)
> >
> >      ASSERT(local_irq_is_enabled());
> >
> > +    if ( nestedhvm_enabled(d) )
> > +        p2m_flush_nestedp2m(d);
> 
> 
> It seems like this should trigger when the nested EPT table itself is being
> populated, e.g.
> 
>   hvm_hap_nested_page_fault
>   --> nestedhvm_hap_nested_page_fault
>   --> nestedhap_fix_p2m (--> p2m_lock(p2m))
>   --> p2m_set_entry
>   --> ept_set_entry
>   --> ept_sync_domain
>   --> p2m_flush_nestedp2m
>   --> p2m_flush_table (--> p2m_lock(p2m))
> 
> which would deadlock on the nested table's p2m lock.  What have I missed?

Yes, you are right. 

Maybe we should add another condition checking, just like:

struct vcpu *v = current;

If(nestedhvm_enabled(d)  && !nestedhvm_vcpu_in_guestmode(v) )  
    p2m_flush_nestedp2m(d);


Or add a flag(is_nested_p2m) in the struct p2m_domain to mark a nested EPT, and 
change the condition to:
      If(nestedhvm_enabled(d)  && p2m->is_nested_p2m )  

to exclude the case of nested EPT, is that enough?

Liang

> Tim.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel