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

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

On 08/08/2014 08:00, Jan Beulich wrote:
>>>> On 08.08.14 at 01:12, <tamas.lengyel@zentific.com> wrote:
>> --- 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)
> Afaic it is out of question to have a function with _six_ boolean
> parameters. This needs to be consolidated into a single flags field. I
> have actually done that already, in a patch serving a different
> purpose (see attached), as discussed recently on this list. I would
> very much appreciate if you either re-based yours on top of that or
> modified it along those lines.
>
>> @@ -2371,11 +2372,19 @@ static void ept_handle_violation(unsigned long qualification, paddr_t gpa)
>>      }
>>  
>>      if ( qualification & EPT_GLA_VALID )
>> +    {
>>          __vmread(GUEST_LINEAR_ADDRESS, &gla);
>> +        fault_gla = !!(qualification & EPT_GLA_FAULT);
>> +        fault_in_gpt = !fault_gla;
> I am actually not agreeing with Andrew regarding the need for two
> flags here, if we already know that SVM also properly expresses the
> distinction between faults on page table accesses and faults on the
> actual translation. The attached patch is also coded in this way, and
> I agree with your earlier arguing for just a single flag.

I also agree.  My suggestion for two flags was on the (wrong) assumption
that AMD didn't currently support providing this information (although I
should have picked up on this and retracted my suggestion).

~Andrew