Re: [PATCH RFC V2 6/6] xen: Handle resumed instruction based on previous mem_event reply

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 11/07/14 19:41, Razvan Cojocaru wrote:
> On 07/11/2014 09:36 PM, Andrew Cooper wrote:
>> On 11/07/14 16:43, Razvan Cojocaru wrote:
>>> In a scenario where a page fault that triggered a mem_event occured,
>>> p2m_mem_access_check() will now be able to either 1) emulate the
>>> current instruction, or 2) emulate it, but don't allow it to perform
>>> any writes.
>>>
>>> Changes since V1:
>>>  - Removed the 'skip' code which required computing the current
>>>    instruction length.
>>>  - Removed the set_ad_bits() code that attempted to modify the
>>>    'accessed' and 'dirty' bits for instructions that the emulator
>>>    can't handle at the moment.
>>>
>>> Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
>>> ---
>>>  xen/arch/x86/domain.c          |    5 +++
>>>  xen/arch/x86/mm/p2m.c          |   90 ++++++++++++++++++++++++++++++++++++++++
>>>  xen/include/asm-x86/domain.h   |    9 ++++
>>>  xen/include/public/mem_event.h |   12 +++---
>>>  4 files changed, 111 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
>>> index e896210..5cd283b 100644
>>> --- a/xen/arch/x86/domain.c
>>> +++ b/xen/arch/x86/domain.c
>>> @@ -407,6 +407,11 @@ int vcpu_initialise(struct vcpu *v)
>>>  
>>>      v->arch.flags = TF_kernel_mode;
>>>  
>>> +    /* By default, do not emulate */
>>> +    v->arch.mem_event.emulate_flags = 0;
>>> +    v->arch.mem_event.gpa = 0;
>>> +    v->arch.mem_event.eip = 0;
>>> +
>>>      rc = mapcache_vcpu_init(v);
>>>      if ( rc )
>>>          return rc;
>>> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
>>> index 13fdf78..7aff480 100644
>>> --- a/xen/arch/x86/mm/p2m.c
>>> +++ b/xen/arch/x86/mm/p2m.c
>>> @@ -1377,6 +1377,7 @@ bool_t p2m_mem_access_check(paddr_t gpa, bool_t gla_valid, unsigned long gla,
>>>  {
>>>      struct vcpu *v = current;
>>>      unsigned long gfn = gpa >> PAGE_SHIFT;
>>> +    unsigned long exit_qualification;
>>>      struct domain *d = v->domain;    
>>>      struct p2m_domain* p2m = p2m_get_hostp2m(d);
>>>      mfn_t mfn;
>>> @@ -1384,6 +1385,9 @@ bool_t p2m_mem_access_check(paddr_t gpa, bool_t gla_valid, unsigned long gla,
>>>      p2m_access_t p2ma;
>>>      mem_event_request_t *req;
>>>      int rc;
>>> +    unsigned long eip = guest_cpu_user_regs()->eip;
>>> +
>>> +    __vmread(EXIT_QUALIFICATION, &exit_qualification);
>> This absolutely can't be put into common code.  I presume you haven't
>> attempted to run a Xen patched with this on an AMD cpu ?
> Indeed, we have not. I think that for a while there wasn't even support
> for mem_even when compiling for AMD, so there wasn't really even a choice.
>
> What would be the best way to get that value in a cross-CPU manner?
>
>
> Thanks,
> Razvan Cojocaru

The problem is that it will die with a fatal invalid opcode exception,
as AMD cpus won't know execute the vmread instruction.

Code using __vmread() must live in arch/x86/hvm/vmx/* and have some
generic hvm way of accessing the required information.  Also bear in
mind that the exit qualification is also architecture specific, so you
will need a more general way of representing "the guest exited because
of a pagefault".

~Andrew