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

[Razvan Cojocaru <rcojocaru@bitdefender.com>]

On 07/23/2014 04:40 PM, Tamas Lengyel wrote:
> 
>     @@ -3137,6 +3172,8 @@ void vmx_vmenter_helper(const struct
>     cpu_user_regs *regs)
>          if ( unlikely(need_flush) )
>              vpid_sync_all();
> 
>     +    check_pf_injection();
> 
> 
> The function check_pf_injection isn't just 'checking', it does issue the
> page fault injection as well under the right circumstances, and as such
> the naming of the function is a bit misleading. Breaking it into two
> functions might be an improvement where the actual check itself could be

That's a good point. I'll split it into two functions (and add a vmx_
prefix to their names while I'm at it).

> wrapped into an unlikely(). But isn't this a bit of an overkill at every
> VMENTER for every domain? Surely there are less invasive mechanisms to
> trigger a VMEXIT when you know your VM will be in a state where you can
> inject your page fault, without incurring an overhead for every domain.

It's not much of an overhead, basically if
d->arch.hvm_domain.fault_info.virtual_address == 0 (which is almost
always the case), nothing happens.

> Another question is, how do you know when the page fault had been
> handled and the page can be inspected? You would need some other trigger
> just at the right point in the execution of the VM or the page could be
> swapped out again before you had a chance to inspect it.

In our case, this always happens while the VCPU is paused waiting for a
mem_event reply, so it fits quite well.

> Also, it might make sense to perform some sanity checks on the vaddr and
> address space before injection (ie. is the page really swapped out).
> There is no guarantee that the page is still swapped out, even if you
> checked before issuing xc_domain_set_pagefault_info, unless the domain
> had been paused for both checking and setting.

As said above, the particular VCPU is in our case paused and waiting for
a mem_event reply. The assumption is that other clients will work under
similar circumstances, however it's always a good idea to check
everything that can be checked.


Thanks,
Razvan Cojocaru