Re: [PATCH 07/14] Nested Virtualization: trap

[Christoph Egger <Christoph.Egger@amd.com>]

On Monday 09 August 2010 14:44:57 Tim Deegan 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 ( nestedhvm_vmentry_emulate(v) ) {
> > +        hvm_funcs.inject_exception(trapnr, errcode, cr2);
> > +        return 0;
> > +    }
> > +
> > +    if ( !nestedhvm_vcpu_in_guestmode(v) ) {
> > +        hvm_funcs.inject_exception(trapnr, errcode, cr2);
> > +        return 0;
> > +    }
> > +
> > +    exitcode = nestedhvm_exception2exitcode(trapnr);
> > +    hvm->nh_hostflags.fields.forcevmexit = 1;
> > +    hvm->nh_forcevmexit.exitcode = exitcode;
> > +    is_intercepted = hvm_nestedhvm_vm_intercepted_by_guest(v, exitcode);
> > +    hvm->nh_hostflags.fields.forcevmexit = 0;
> > +
> > +    if ( is_intercepted )
> > +    {
> > +        enum nestedhvm_vmexits nsret;
> > +
> > +        hvm->nh_forcevmexit.exitcode = exitcode;
> > +        hvm->nh_forcevmexit.exitinfo1 = errcode;
> > +        hvm->nh_forcevmexit.exitinfo2 = cr2;
> > +
> > +        hvm->nh_hostflags.fields.forcevmexit = 1;
> > +        nsret = nestedhvm_vcpu_vmexit(v, guest_cpu_user_regs(), 0 /*
> > dummy */); +        hvm->nh_hostflags.fields.forcevmexit = 0;
> > +
> > +        switch (nsret) {
> > +        case NESTEDHVM_VMEXIT_DONE:
> > +        case NESTEDHVM_VMEXIT_ERROR: /* L1 guest will crash L2 guest */
> > +            return 1;
> > +        case NESTEDHVM_VMEXIT_HOST:
> > +        case NESTEDHVM_VMEXIT_CONTINUE:
> > +        case NESTEDHVM_VMEXIT_FATALERROR:
> > +        default:
> > +            gdprintk(XENLOG_ERR, "unexpected nestedhvm error %i\n",
> > nsret); +            return -1;
>
> This new [-1,0,1] return value is ignored by almost all callers.  Would
> it be possible to get rid of the special case for #PF, and go back to
> returning void?

The return value of hvm_inject_exception() is only meaningful for code
sections that is "aware" of nested virtualization.

hvm_inject_exception() is mostly called from code that is not "aware"
of nested virtualization.

Hence, yes, you are right, most callers will ignore the return value.

There is exactly one reason to have them: Intel seems to want
"shadow-on-shadow". In this case the page fault handler
walks the guests shadow page table. If that fails the page
fault handler wants to inject a VMEXIT(#PF) into the guest to
let the guest fix its shadow page table. If the guest page walk
is successfull the page fault intercept handler wants to inject the
page fault exception into the nested guest.

The page fault intercept handler in
SVM (see [PATCH 10/14] Nested Virtualization: svm specific implementation)
assumes that the guest intercepts a page fault.
It uses the return value to check if hvm_inject_exception() did what is 
expected: Injecting a VMEXIT(#PF), which is the case when the assumption
is correct.
The page fault intercept handler calls svm_inject_exception() to inject
a page fault into the nested guest.

If you can invalidate this error check reason then yes, I can go back
to make hvm_inject_exception() return void.

Christoph


-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632