Re: [Xen-devel] [PATCH] x86/hvm: simplify emulation triggered by vm_event response

[Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>]

On 02/04/2016 02:36 PM, Andrew Cooper wrote:
> On 04/02/16 12:27, Razvan Cojocaru wrote:
>> Currently, after receiving a vm_event reply requesting emulation,
>> the actual emulation is triggered in p2m_mem_access_check(),
>> which means that we're waiting for the page fault to occur again
>> before emulating.
> 
> Presumably this means that we re-enter the guest and exit immediately
> for (hopefully) the same violation?

Yes, something along those lines: the original page fault occurs, a
vm_event is being sent to the application, which replies with the
EMULATE flag set (but does not lift the page restrictions). Now we're
hoping that the same instruction that has caused the first page fault
runs, which triggers a new page fault (thus a new call of
p2m_mem_access_check()). But in p2m_mem_access_check() we check to see
if emulate flags are set for the current VCPU, and if they are, we check
to see that the instruction is really the one that has caused the
original page fault, and if it is (i.e. both EIP and GPA match), we emulate.

The ideal case is the one where the second page fault is being caused by
the same instruction hitting the same page, and that happens most of the
time, but it unfortunately does not happen all of the time.

So when the second page fault is _not_ caused by the same instruction,
we just reset the emulate flags and carry on with regular processing,
which means that a new vm_event will be sent out about this new page
fault. But even though the application has reuquested that the page
fault that has triggered the last page fault be emulated, it wasn't (as
a design limitation). So now, when / if the old instruction hits the
page again, it will be received by the monitoring application as a new
hit, not still the old, unemulated one.

There are safeguards possible for this in the monitoring application,
but they too have limitations, and it is ultimately less efficient and
more error-prone that the alternative hopefully is.

>>  Aside from the performance impact, this
>> complicates the code since between hvm_do_resume() and the second
>> page fault it is possible that the latter becomes a completely
>> new page fault - hence checking that EIP and the GPA match with
>> the ones in the original page fault.
> 
> Presumably this occurs when we injected an event on the vmentry?

Any asynchronous-type cause of a page fault will do, hopefully I've been
able to explain somewhat above.

>>  If they don't, duplicate
>> EPT fault vm_events will occur, of which a monitoring application
>> needs to be aware.
>> This patch makes struct arch_vm_event smaller (since we no longer
>> need to track eip and gpa), removes the checking code from
>> p2m_mem_access_check(), and moves the emulation in hvm_do_resume().
>>
>> Signed-off-by: Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>
>> ---
>>  xen/arch/x86/hvm/hvm.c         | 17 +++++++++++++++++
>>  xen/arch/x86/mm/p2m.c          | 34 ----------------------------------
>>  xen/include/asm-x86/vm_event.h |  2 --
>>  3 files changed, 17 insertions(+), 36 deletions(-)
> 
> Gotta love that diffstat!
> 
> The logic makes sense, so Acked-by: Andrew Cooper
> <andrew.cooper3@xxxxxxxxxx> for the x86-related nature, but it would be
> nice to have a review from Tamas for the vm_event side of things.

Thanks! Of course. Hopefully Tamas will like this, based on a
conversation we've had a few days ago here.


Thanks,
Razvan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel