[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

RE: [Xen-devel] [PATCH] remove useless super page recomputing in ept_set_entry



> Would it make sense to turn this check on in DEBUG mode?  It can't be
> that expensive if it's currently on all the time.

Put the check in debug mode is of course no problem, the problem to me is the 
code
implies:

Say the original page is a super page, we want to change a page inside it.  
When we
get here we need to split the original super page, but somehow smaller super 
page is
not supported, we need split the page further.

Although hardware spec doesn't say smaller super page will also be supported, 
but the
code will be much simpler can cleaner.

Actually it is the p2m layer managing guest memory allocation/free and mapping, 
so it
is not reasonable to me to put part of the logic like splitting a page into EPT 
layer.  How
current code manages the info when a super page is split?  Will the split page 
be changed
to some newly allocated page or freed casually?

Anyway I prefer to keep such info in p2m layer.

Thanks!
-Xin


> 
> It's OK to code with assumptions about what some other part of the
> code is doing, but it's always good to add checks (in DEBUG mode) to
> catch problems if the assumptions change.
> 
>  -George
> 
> On Tue, Jun 29, 2010 at 7:06 PM, Li, Xin <xin.li@xxxxxxxxx> wrote:
> > VMX: remove useless super page recomputing in ept_set_entry.
> >
> > Caller to ept_set_entry must give a valid order that hardware supports, see
> set_p2m_entry in p2m.c.
> >
> > Signed-off-by: Xin Li <xin.li@xxxxxxxxx>
> >
> > diff -r a6d5b76abdcb xen/arch/x86/mm/hap/p2m-ept.c
> > --- a/xen/arch/x86/mm/hap/p2m-ept.c Tue Jun 29 18:20:14 2010 +0100
> > +++ b/xen/arch/x86/mm/hap/p2m-ept.c Wed Jun 30 09:59:40 2010 +0800
> > @@ -307,18 +307,10 @@
> >     }
> >     else
> >     {
> > -        int num = order / EPT_TABLE_ORDER;
> >         int level;
> >         ept_entry_t *split_ept_entry;
> >
> > -        if ( (num >= 2) && hvm_hap_has_1gb(d) )
> > -            num = 2;
> > -        else if ( (num >= 1) && hvm_hap_has_2mb(d) )
> > -            num = 1;
> > -        else
> > -            num = 0;
> > -
> > -        for ( level = split_level; level > num ; level-- )
> > +        for ( level = split_level; level > walk_level ; level-- )
> >         {
> >             rv = ept_split_large_page(d, &table, &index, gfn, level);
> >             if ( !rv )
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@xxxxxxxxxxxxxxxxxxx
> > http://lists.xensource.com/xen-devel
> >

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.