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

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

On Tuesday 10 August 2010 12:48:27 Tim Deegan wrote:
> At 09:55 +0100 on 10 Aug (1281434149), Christoph Egger wrote:
> > 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.
>
> OK, I think I'm getting tied up in the naming scheme.  Let me try to lay
> out what I think is going on and you can tell me where I'm going wrong.
>
> If the L0 Xen is using shadow pagetables, then it must be intercepting
> #PF.  We expect the guest to be using shadows (and intercepting #PF) too.
>
> When an actual #PF occurs when L2 is running, we VMEXIT to L0, which
> walks the pagetables provided by L1.  In the interesting case, L0 sees
> that the L1 pagetables justify the fault. It injects #PF into the VM. 

> If the L1 guest is using shadows, it intercepts #PF and does its own
> shadow pagetable tricks (which might involve it injecting #PF into the
> L2 guest).  If it's not, the injected #PF goes straight to the L2.

Correct.

> > 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.
>
> The new return code from hvm_inject_exception() is
> 0: Normal injection into the running L1 or the running L2.
> 1: VMEXIT from the running L2 to the L1, caused by injecting an
>    intercepted exception. (This is the expected case in the scenario
>    above).
> -1: Any other case.

Correct.

> The logic in svm_vmexit_handler() when the L0 shadow code decides to
> inject #PF is now:
>
> if ( L2 is running and L1 is not using HAP (i.e. sh-on-hap or sh-on-sh) )
> {
>     Inject #PF (allowing the L1 to intercept);
>     If the L1 didn't intercept (or injection failed)
>         then crash the L1.
> } else (i.e L1 running directly or hap-on-hap) {
>     Inject directly, ignoring the L1 intercepts.
> }

Correct.

> I don't see why this change is needed.  AFAICS, all cases are covered by
> unconditionally calling hvm_inject_exception() here.  *-on-hap never
> takes this path at all because L0 doesn't intercept #PF when it uses
> HAP, so the only difference this makes is to catch the case where L1
> didn't intercept #PF and it was injected directly into L2.

Correct. How should L1 fix its shadow page table in this case?
I expect L2 to go wild because of that.

> But this is an acceptable thing for the L1 to do (even though Xen doesn't
> ever behave that way), so I think it's wrong to crash the guest.

Ok, that is the point where our opinions differ. Can you explain me why
this is acceptable? As I said above, I expect the L2 guest to go wild in
this case.

> What was the reason for calling svm_inject_exception() directly in some
> cases?

svm_inject_exception() always injects a #PF into running L1 or L2. It never
injects a VMEXIT into L1.

> Cheers,
>
> Tim.
>
> > 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