[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

On Fri, Aug 8, 2014 at 9:00 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>> On 08.08.14 at 01:12, <tamas.lengyel@xxxxxxxxxxxx> 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.

Certainly. I'll split your consolidation part into its own patch and rebase on that.

> @@ -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.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.