Re: [Xen-devel] [PATCH 07/14] Nested Virtualization: trap

[Tim Deegan <Tim.Deegan@xxxxxxxxxx>]

At 14:53 +0100 on 19 Aug (1282229594), Dong, Eddie wrote:
> Tim Deegan wrote:
> > At 03:44 +0100 on 19 Aug (1282189499), Dong, Eddie wrote:
> >>> 
> >>> +int hvm_inject_exception(unsigned int trapnr, int errcode,
> >>> unsigned long cr2) +{ +    uint64_t exitcode;
> >>> +    bool_t is_intercepted;
> >>> +    struct vcpu *v = current;
> >>> +    struct nestedhvm *hvm = &VCPU_NESTEDHVM(v);
> >>> +
> >>> +    if ( !nestedhvm_enabled(v->domain) ) {
> >>> +        hvm_funcs.inject_exception(trapnr, errcode, cr2); +       
> >>> return 0; +    }
> >> 
> >> If it is not nested, we go from here to vendor specific injection
> >> code. If it is nested, I think we'd better to go to another vendoral
> >> specific handler too.
> > 
> > That's exactly what this wrapper does.  It's basically:
> >  if ( in L2 and L1 intercepts )
> >     force vmexit
> >  else
> >     inject directly.
> > 
> > It calls out to arch-specific code to do the intercept check and the
> > vmexit.  It could be tidied up a bit and the interfaces could change
> > but this looks like about the least amount of sharing there could be
> > on this path.  I can't see anything objectionable.
> 
> Is there any real data to show the saving LOC here?

Lines of code aren't important by themselves; it's duplication of logic
that I object to.  For pure LOC we could certainly compress this
function in other ways.

> I see the real
> code start from "exitcode = nestedhvm_exception2exitcode(trapnr);"
> (half of this function code is just for wrapper check.). The real work
> is ~20LOC. However we added at least 2 new wrapper APIs:
> nestedhvm_exception2exitcode & nestedhvm_exception2exitcode.

That's one. :)  And it wouldn't be needed if the call to arch-specific
code to cause a vmexit and the test for interception took trapnr
directly (which they probably could).

If the new namespace of vmexit codes goes away entirely, that's fine by
me, btw.  Maybe the generic code can pass exit codes as opaque numbers,
with a few flags to steer it?  Christoph, what do you think?  You'll
have the best idea of how useful the new namespace is.

Tim.

-- 
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