Re: [PATCH v2 1/2] x86/mem_event: Deliver gla fault EPT violation information

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

On 07/08/2014 23:19, Tamas Lengyel wrote:
> On Fri, Aug 8, 2014 at 12:00 AM, Andrew Cooper <andrew.cooper3@citrix.com>
> wrote:
> 
>> On 07/08/2014 20:47, Tamas K Lengyel wrote:
>>> On Intel EPT the exit qualification generated by a violation also
>> includes a bit (EPT_GLA_FAULT) which describes the following information:
>>> Set if the access causing the EPT violation is to a guest-physical
>> address that is the translation of a linear address. Clear if the access
>> causing the EPT violation is to a paging-structure entry as part of a page
>> walk or the update of an accessed or dirty bit.
>>>
>>> For more information see Table 27-7 in the Intel SDM.
>>>
>>> This patch extends the mem_event system to deliver this extra
>> information, which could be useful for determining the cause of a violation.
>>>
>>> v2: Split gla_fault into fault_in_gpt and fault_gla to be more
>> compatible with the AMD implementation.
>>>
>>> Signed-off-by: Tamas K Lengyel <tamas.lengyel@zentific.com>
>>> ---
>>>  xen/arch/x86/hvm/hvm.c         |  8 ++++++--
>>>  xen/arch/x86/hvm/svm/svm.c     |  2 +-
>>>  xen/arch/x86/hvm/vmx/vmx.c     | 23 ++++++++++++++++++++++-
>>>  xen/arch/x86/mm/p2m.c          |  5 ++++-
>>>  xen/include/asm-x86/hvm/hvm.h  |  5 ++++-
>>>  xen/include/asm-x86/p2m.h      |  3 ++-
>>>  xen/include/public/mem_event.h |  4 +++-
>>>  7 files changed, 42 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
>>> index e834406..d7b5e2b 100644
>>> --- a/xen/arch/x86/hvm/hvm.c
>>> +++ b/xen/arch/x86/hvm/hvm.c
>>> @@ -2725,6 +2725,8 @@ void hvm_inject_page_fault(int errcode, unsigned
>> long cr2)
>>>  int hvm_hap_nested_page_fault(paddr_t gpa,
>>>                                bool_t gla_valid,
>>>                                unsigned long gla,
>>> +                              bool_t fault_in_gpt,
>>> +                              bool_t fault_gla,
>>>                                bool_t access_r,
>>>                                bool_t access_w,
>>>                                bool_t access_x)
>>> @@ -2832,8 +2834,10 @@ int hvm_hap_nested_page_fault(paddr_t gpa,
>>>
>>>          if ( violation )
>>>          {
>>> -            if ( p2m_mem_access_check(gpa, gla_valid, gla, access_r,
>>> -                                        access_w, access_x, &req_ptr) )
>>> +            if ( p2m_mem_access_check(gpa, gla_valid, gla,
>>> +                                        fault_in_gpt, fault_gla,
>>> +                                        access_r, access_w, access_x,
>>> +                                        &req_ptr) )
>>
>> Please fix your new indentation.
>>
> 
> I'm just guessing here but is it the preferred indentation to fill each
> line to about the same length before starting a new line?

Line up each line of arguments with the g of gpa.  The original patch
was two spaces out.

~Andrew