Re: [PATCH RFC V9 4/5] xen, libxc: Request page fault injection via libxc

[Tim Deegan <tim@xen.org>]

At 12:30 +0300 on 10 Sep (1410348648), Razvan Cojocaru wrote:
> On 09/09/2014 11:14 PM, Tim Deegan wrote:
> > At 17:57 +0100 on 09 Sep (1410281829), George Dunlap wrote:
> >> On Tue, Sep 2, 2014 at 2:24 PM, Tim Deegan <tim@xen.org> wrote:
> >>> Hi,
> >>>
> >>> At 12:18 +0300 on 02 Sep (1409656686), Razvan Cojocaru wrote:
> >>>> While we need to set the data per-domain and have whatever VCPU inject
> >>>> the page fault - _but_only_if_ it's in usermode and its CR3 points to
> >>>> something interesting.
> >>>
> >>> That's a strange and specific thing to ask the hypervisor to do for
> >>> you.  Given that you can already trap CR3 changes as mem-events can
> >>> you trigger the fault injection in response to the contect switch?
> >>> I guess that would probably catch it in kernel mode. :(
> >>
> >> I was wondering, rather than special-casing inject_trap, would it make
> >> sense to be able for the memory controller to get notifications when
> >> certain more complex conditions happen (e.g., "some vcpu is in user
> >> mode with this CR3")?  Then the controller could ask to be notified
> >> when the event happens, and when it does, just call inject_fault.
> > 
> > Yes, that sounds like a better place to put this kind of test.  As
> > part of the mem_event trigger framework it doesn't seem nearly so out
> > of place (and it avoids many of the problems of clashes between
> > different event injection paths).
> 
> Do you mean someplace here (hvm.c)?
> 
> 3265 int hvm_set_cr3(unsigned long value)
> 3266 {
> 3267     struct vcpu *v = current;
> 3268     struct page_info *page;
> 3269     unsigned long old;
> 3270
> 3271     if ( hvm_paging_enabled(v) && !paging_mode_hap(v->domain) &&
> 3272          (value != v->arch.hvm_vcpu.guest_cr[3]) )
> 3273     {
> 3274         /* Shadow-mode CR3 change. Check PDBR and update refcounts. */
> 3275         HVM_DBG_LOG(DBG_LEVEL_VMMU, "CR3 value = %lx", value);
> 3276         page = get_page_from_gfn(v->domain, value >> PAGE_SHIFT,
> 3277                                  NULL, P2M_ALLOC);
> 3278         if ( !page )
> 3279             goto bad_cr3;
> 3280
> 3281         put_page(pagetable_get_page(v->arch.guest_table));
> 3282         v->arch.guest_table = pagetable_from_page(page);
> 3283
> 3284         HVM_DBG_LOG(DBG_LEVEL_VMMU, "Update CR3 value = %lx", value);
> 3285     }
> 3286
> 3287     old=v->arch.hvm_vcpu.guest_cr[3];
> 3288     v->arch.hvm_vcpu.guest_cr[3] = value;
> 3289     paging_update_cr3(v);
> 3290     hvm_memory_event_cr3(value, old);
> 3291     return X86EMUL_OKAY;
> 3292
> 3293  bad_cr3:
> 3294     gdprintk(XENLOG_ERR, "Invalid CR3\n");
> 3295     domain_crash(v->domain);
> 3296     return X86EMUL_UNHANDLEABLE;
> 3297 }
> 
> Alongside hvm_memory_event_cr3(value, old), have another function
> checking an array of CR3s and if v is in user mode and send out an event?

Nope, I mean trigger an event when the guets vcpu is in _user_ mode
with the right CR3.  You could detect that on the VMEXIT path for example.

> As I've explained in my earlier reply to Tamas, the way we use the
> injection request hypercall now, conditions normally apply for immediate
> injection.

Oh, so you don't actually need this delayed/triggered injection right
now?  In that case we should definitely drop all of this in favour of
a simple inject-this-fault-now API.

Once you have an actual user of the feature we can come back to it --
after all, then we'll know exactly what's needed.  And if it never
actually is needed we save ourselves a bunch of code. :)

> Also, I'm not sure the "is it user mode?" check would reliably work at
> the time of calling hvm_set_cr3(). Are CR3 loads not always happening in
> kernel-mode?

Yes, indeed.

> > Yeah; I was thinking about page-protecting the process's stack as an
> > approach to this.  Breakpointing the return address might work too but
> > would probably cause more false alarms -- you'd at least need to walk
> > up past the libc/win32 wrappers to avoid trapping every thread.
> > 
> > Ideally there'd be something vcpu-specific we could tinker with
> > (e.g. arranging MSRs so that SYSRET will fault) once we see the right
> > CR3 (assuming intercepting CR3 is cheap enough in this case).
> 
> All valid suggestions, however they would seem to have a greater impact
> on guest responsiveness. There would be quite a lot of CR3 loads and
> SYSRETs.

Well, you'd only be trapping the CR3 loads, and only to Xen; trapping
CR3 to Xen is measurable but not by any means dreadful -- I'd expect
it to be dwarfed by the other costs of the mem_event user.  The only
SYSRETs that would trap would be the ones already in the right address
space. 

Cheers,

Tim.