Re: [Xen-devel] [PATCH v1] x86/mm: Suppresses vm_events caused by page-walks

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 24.08.18 at 20:47, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
> On 8/24/18 8:49 PM, Andrew Cooper wrote:
>> On 24/08/18 15:11, Alexandru Isaila wrote:
>>> diff --git a/xen/arch/x86/mm/mem_access.c b/xen/arch/x86/mm/mem_access.c
>>> index 03a864156..b01194d 100644
>>> --- a/xen/arch/x86/mm/mem_access.c
>>> +++ b/xen/arch/x86/mm/mem_access.c
>>> @@ -212,7 +212,20 @@ bool p2m_mem_access_check(paddr_t gpa, unsigned long 
>>> gla,
>>>           d->arch.monitor.inguest_pagefault_disabled &&
>>>           npfec.kind != npfec_kind_with_gla ) /* don't send a mem_event */
>>>      {
>>> -        hvm_emulate_one_vm_event(EMUL_KIND_NORMAL, TRAP_invalid_op, 
>>> X86_EVENT_NO_EC);
>>> +        struct hvm_hw_cpu ctxt;
>>> +        uint32_t pfec = PFEC_page_present;
>>> +        unsigned long gfn;
>>> +        uint32_t gflags;
>>> +
>>> +        hvm_funcs.save_cpu_ctxt(v, &ctxt);
>>> +        paging_get_hostmode(v)->pte_flags(v, p2m, gla, 0, ctxt.cr3, 
>>> &gflags);
>>> +        if ( gflags & _PAGE_RW )
>>> +            pfec |= PFEC_write_access;
>>> +
>>> +        if ( gflags & _PAGE_USER )
>>> +            pfec |= PFEC_user_mode;
>> 
>> As I've tried to explain before, this is architecturally incorrect. 
>> Both need to be derived from the EPT violation, because they are
>> properties of instruction which caused the fault, not the mapping which
>> faulted.
> 
> Right, I assume you mean starting to use eff_read, eff_write, eff_exec,
> and eff_user_exec from ept_qual_t:
> 
> 619 /* EPT violation qualifications definitions */
> 620 typedef union ept_qual {
> 621     unsigned long raw;
> 622     struct {
> 623         bool read:1, write:1, fetch:1,
> 624             eff_read:1, eff_write:1, eff_exec:1, /* eff_user_exec */:1,
> 625             gla_valid:1,
> 626             gla_fault:1; /* Valid iff gla_valid. */
> 627         unsigned long /* pad */:55;
> 628     };
> 629 } __transparent__ ept_qual_t;
> 
> Unfortunately, I've been told that that's not the way to go since those
> fields will be relevant only on newer architectures, and we'd like to be
> able to support even older ones. And, of course, the other thing is that
> they're VMX / EPT specific, and we were hoping to be able to get SVM
> support for free like that.

If this was the case (I didn't check, and you didn't provide a
pointer along with the "I've been told"), at the very least on
newer hardware correct behavior should be implemented.

For NPT, isn't there an error code bit telling you whether the
request was a user or "system" one? If not, some cheating
would be needed (derive from CPL, accepting that e.g.
descriptor table accesses would get mis-attributed), but
that's still not going to involve looking at the PTE flags.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel