All of lore.kernel.org
 help / color / mirror / Atom feed
* SVM NPT mem_access and npfec.insn_fetch
@ 2018-03-30 21:06 Razvan Cojocaru
  2018-03-30 21:14 ` Andrew Cooper
  0 siblings, 1 reply; 5+ messages in thread
From: Razvan Cojocaru @ 2018-03-30 21:06 UTC (permalink / raw)
  To: xen-devel; +Cc: Boris Ostrovsky, Tamas K Lengyel, suravee.suthikulpanit

Hello,

We've been trying to enable mem_access events for SVM, and we do receive
some events with xen-access if we set the pages read-only. However, the
exec xen-access tests fails, because it appears that npfec.insn_fetch is
always 0 here (or at least we haven't seen it to be 1 in a lot of testing):

index 569b124..d518c94 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1835,6 +1835,13 @@ int hvm_hap_nested_page_fault(paddr_t gpa,
unsigned long gla,
             break;
         }

+        printk("pae: %d, nx: %d, gfn: 0x%lx access %d violation %d read %d"
+               " write %d insn %d present %d glav %d kind %d \n",
+               !!(curr->arch.hvm_vcpu.guest_cr[4] & X86_CR4_PAE),
+               !!(curr->arch.hvm_vcpu.guest_efer & EFER_NX), gfn,
+               p2ma, violation, npfec.read_access, npfec.write_access,
+               npfec.insn_fetch, npfec.present, npfec.gla_valid,
npfec.kind);
+
         if ( violation )
         {
             /* Should #VE be emulated for this fault? */

This patch does not require anything else, just start an HVM guest on an
AMD host. It outputs things like:

(XEN) pae: 1, nx: 1, gfn: 0xf0235 access 7 violation 0 read 1 write 1
insn 0 present 1 glav 0 kind 2

The dom0 CPU does list "pae" and "nx" among its capabilities:

# cat /proc/cpuinfo | grep nx
flags		: fpu de tsc msr *pae* mce cx8 apic mca cmov pat clflush mmx fxsr
sse sse2 ht syscall *nx* mmxext fxsr_opt lm constant_tsc rep_good nopl
nonstop_tsc extd_apicid eagerfpu pni pclmulqdq ssse3 fma cx16 sse4_1
sse4_2 movbe popcnt aes xsave avx f16c rdrand hypervisor lahf_lm
cmp_legacy abm sse4a misalignsse 3dnowprefetch bpext retpoline
retpoline_amd vmmcall fsgsbase bmi1 avx2 bmi2 rdseed adx clflushopt
sha_ni xsaveopt xsavec xgetbv1 clzero arat

In svm.c, this is what happens:

static void svm_do_nested_pgfault(struct vcpu *v,
    struct cpu_user_regs *regs, uint64_t pfec, paddr_t gpa)
{
    int ret;
    unsigned long gfn = gpa >> PAGE_SHIFT;
    mfn_t mfn;
    p2m_type_t p2mt;
    p2m_access_t p2ma;
    struct p2m_domain *p2m = NULL;

    /*
     * Since HW doesn't explicitly provide a read access bit and we need to
     * somehow describe read-modify-write instructions we will
conservatively
     * set read_access for all memory accesses that are not instruction
fetches.
     */
    struct npfec npfec = {
        .read_access = !(pfec & PFEC_insn_fetch),
        .write_access = !!(pfec & PFEC_write_access),
        .insn_fetch = !!(pfec & PFEC_insn_fetch),
        .present = !!(pfec & PFEC_page_present),
    };

    /* These bits are mutually exclusive */
    if ( pfec & NPT_PFEC_with_gla )
        npfec.kind = npfec_kind_with_gla;
    else if ( pfec & NPT_PFEC_in_gpt )
        npfec.kind = npfec_kind_in_gpt;

    ret = hvm_hap_nested_page_fault(gpa, ~0ul, npfec);

(One thing to notice here is that the gla is always invalid with SVM -
is there perhaps _some_ way of getting it in Xen even though the manual
says we only have the GPA and error code here?)

svm_do_nested_pgfault() is being called from the NPF VMEXIT code:

    case VMEXIT_NPF:
        perfc_incra(svmexits, VMEXIT_NPF_PERFC);
        if ( cpu_has_svm_decode )
            v->arch.hvm_svm.cached_insn_len = vmcb->guest_ins_len & 0xf;
        rc = vmcb->exitinfo1 & PFEC_page_present
             ? p2m_pt_handle_deferred_changes(vmcb->exitinfo2) : 0;
        if ( rc >= 0 )
            svm_do_nested_pgfault(v, regs, vmcb->exitinfo1,
vmcb->exitinfo2);
        else
        {
            printk(XENLOG_G_ERR
                   "%pv: Error %d handling NPF (gpa=%08lx ec=%04lx)\n",
                   v, rc, vmcb->exitinfo2, vmcb->exitinfo1);
            domain_crash(v->domain);
        }
        v->arch.hvm_svm.cached_insn_len = 0;
        break;

Surely with xen-access setting _all_ the guest's pages to
XENMEM_access_rw I should have at least seen one event coming from an
execute fault.

What are we missing?


Thanks,
Razvan

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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: SVM NPT mem_access and npfec.insn_fetch
  2018-03-30 21:06 SVM NPT mem_access and npfec.insn_fetch Razvan Cojocaru
@ 2018-03-30 21:14 ` Andrew Cooper
  2018-03-30 21:47   ` Razvan Cojocaru
  0 siblings, 1 reply; 5+ messages in thread
From: Andrew Cooper @ 2018-03-30 21:14 UTC (permalink / raw)
  To: Razvan Cojocaru, xen-devel
  Cc: Boris Ostrovsky, Tamas K Lengyel, suravee.suthikulpanit

On 30/03/2018 22:06, Razvan Cojocaru wrote:
> Hello,
>
> We've been trying to enable mem_access events for SVM, and we do receive
> some events with xen-access if we set the pages read-only. However, the
> exec xen-access tests fails, because it appears that npfec.insn_fetch is
> always 0 here (or at least we haven't seen it to be 1 in a lot of testing):
>
> index 569b124..d518c94 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -1835,6 +1835,13 @@ int hvm_hap_nested_page_fault(paddr_t gpa,
> unsigned long gla,
>              break;
>          }
>
> +        printk("pae: %d, nx: %d, gfn: 0x%lx access %d violation %d read %d"
> +               " write %d insn %d present %d glav %d kind %d \n",
> +               !!(curr->arch.hvm_vcpu.guest_cr[4] & X86_CR4_PAE),
> +               !!(curr->arch.hvm_vcpu.guest_efer & EFER_NX), gfn,
> +               p2ma, violation, npfec.read_access, npfec.write_access,
> +               npfec.insn_fetch, npfec.present, npfec.gla_valid,
> npfec.kind);
> +
>          if ( violation )
>          {
>              /* Should #VE be emulated for this fault? */
>
> This patch does not require anything else, just start an HVM guest on an
> AMD host. It outputs things like:
>
> (XEN) pae: 1, nx: 1, gfn: 0xf0235 access 7 violation 0 read 1 write 1
> insn 0 present 1 glav 0 kind 2
>
> The dom0 CPU does list "pae" and "nx" among its capabilities:
>
> # cat /proc/cpuinfo | grep nx
> flags		: fpu de tsc msr *pae* mce cx8 apic mca cmov pat clflush mmx fxsr
> sse sse2 ht syscall *nx* mmxext fxsr_opt lm constant_tsc rep_good nopl
> nonstop_tsc extd_apicid eagerfpu pni pclmulqdq ssse3 fma cx16 sse4_1
> sse4_2 movbe popcnt aes xsave avx f16c rdrand hypervisor lahf_lm
> cmp_legacy abm sse4a misalignsse 3dnowprefetch bpext retpoline
> retpoline_amd vmmcall fsgsbase bmi1 avx2 bmi2 rdseed adx clflushopt
> sha_ni xsaveopt xsavec xgetbv1 clzero arat
>
> In svm.c, this is what happens:
>
> static void svm_do_nested_pgfault(struct vcpu *v,
>     struct cpu_user_regs *regs, uint64_t pfec, paddr_t gpa)
> {
>     int ret;
>     unsigned long gfn = gpa >> PAGE_SHIFT;
>     mfn_t mfn;
>     p2m_type_t p2mt;
>     p2m_access_t p2ma;
>     struct p2m_domain *p2m = NULL;
>
>     /*
>      * Since HW doesn't explicitly provide a read access bit and we need to
>      * somehow describe read-modify-write instructions we will
> conservatively
>      * set read_access for all memory accesses that are not instruction
> fetches.
>      */
>     struct npfec npfec = {
>         .read_access = !(pfec & PFEC_insn_fetch),
>         .write_access = !!(pfec & PFEC_write_access),
>         .insn_fetch = !!(pfec & PFEC_insn_fetch),
>         .present = !!(pfec & PFEC_page_present),
>     };
>
>     /* These bits are mutually exclusive */
>     if ( pfec & NPT_PFEC_with_gla )
>         npfec.kind = npfec_kind_with_gla;
>     else if ( pfec & NPT_PFEC_in_gpt )
>         npfec.kind = npfec_kind_in_gpt;
>
>     ret = hvm_hap_nested_page_fault(gpa, ~0ul, npfec);
>
> (One thing to notice here is that the gla is always invalid with SVM -
> is there perhaps _some_ way of getting it in Xen even though the manual
> says we only have the GPA and error code here?)
>
> svm_do_nested_pgfault() is being called from the NPF VMEXIT code:
>
>     case VMEXIT_NPF:
>         perfc_incra(svmexits, VMEXIT_NPF_PERFC);
>         if ( cpu_has_svm_decode )
>             v->arch.hvm_svm.cached_insn_len = vmcb->guest_ins_len & 0xf;
>         rc = vmcb->exitinfo1 & PFEC_page_present
>              ? p2m_pt_handle_deferred_changes(vmcb->exitinfo2) : 0;
>         if ( rc >= 0 )
>             svm_do_nested_pgfault(v, regs, vmcb->exitinfo1,
> vmcb->exitinfo2);
>         else
>         {
>             printk(XENLOG_G_ERR
>                    "%pv: Error %d handling NPF (gpa=%08lx ec=%04lx)\n",
>                    v, rc, vmcb->exitinfo2, vmcb->exitinfo1);
>             domain_crash(v->domain);
>         }
>         v->arch.hvm_svm.cached_insn_len = 0;
>         break;
>
> Surely with xen-access setting _all_ the guest's pages to
> XENMEM_access_rw I should have at least seen one event coming from an
> execute fault.
>
> What are we missing?

Look at p2m-pt.c and check whether NX is being properly set in the NPT
tables when you select XENMEM_access_rw ?  NPT pagetables have an
identical format and layout to regular PAE pagetables.

~Andrew

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: SVM NPT mem_access and npfec.insn_fetch
  2018-03-30 21:14 ` Andrew Cooper
@ 2018-03-30 21:47   ` Razvan Cojocaru
  2018-03-30 21:52     ` Andrew Cooper
  0 siblings, 1 reply; 5+ messages in thread
From: Razvan Cojocaru @ 2018-03-30 21:47 UTC (permalink / raw)
  To: Andrew Cooper, xen-devel
  Cc: Boris Ostrovsky, Tamas K Lengyel, suravee.suthikulpanit

On 03/31/2018 12:14 AM, Andrew Cooper wrote:
> On 30/03/2018 22:06, Razvan Cojocaru wrote:
>> Hello,
>>
>> We've been trying to enable mem_access events for SVM, and we do receive
>> some events with xen-access if we set the pages read-only. However, the
>> exec xen-access tests fails, because it appears that npfec.insn_fetch is
>> always 0 here (or at least we haven't seen it to be 1 in a lot of testing):
>>
>> index 569b124..d518c94 100644
>> --- a/xen/arch/x86/hvm/hvm.c
>> +++ b/xen/arch/x86/hvm/hvm.c
>> @@ -1835,6 +1835,13 @@ int hvm_hap_nested_page_fault(paddr_t gpa,
>> unsigned long gla,
>>              break;
>>          }
>>
>> +        printk("pae: %d, nx: %d, gfn: 0x%lx access %d violation %d read %d"
>> +               " write %d insn %d present %d glav %d kind %d \n",
>> +               !!(curr->arch.hvm_vcpu.guest_cr[4] & X86_CR4_PAE),
>> +               !!(curr->arch.hvm_vcpu.guest_efer & EFER_NX), gfn,
>> +               p2ma, violation, npfec.read_access, npfec.write_access,
>> +               npfec.insn_fetch, npfec.present, npfec.gla_valid,
>> npfec.kind);
>> +
>>          if ( violation )
>>          {
>>              /* Should #VE be emulated for this fault? */
>>
>> This patch does not require anything else, just start an HVM guest on an
>> AMD host. It outputs things like:
>>
>> (XEN) pae: 1, nx: 1, gfn: 0xf0235 access 7 violation 0 read 1 write 1
>> insn 0 present 1 glav 0 kind 2
>> [...]
>> What are we missing?
> 
> Look at p2m-pt.c and check whether NX is being properly set in the NPT
> tables when you select XENMEM_access_rw ?  NPT pagetables have an
> identical format and layout to regular PAE pagetables.
We weren't setting it at all. Just stored the restrictions in a
newly-added integer in struct page_info (suggestions for the best place
to store them are welcome), and fed them back to whatever function was
asking for them from there.

Did a quick test and setting _PAGE_NX_BIT as well seems to be the way to
go indeed.


Thanks,
Razvan

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: SVM NPT mem_access and npfec.insn_fetch
  2018-03-30 21:47   ` Razvan Cojocaru
@ 2018-03-30 21:52     ` Andrew Cooper
  2018-03-30 22:04       ` Razvan Cojocaru
  0 siblings, 1 reply; 5+ messages in thread
From: Andrew Cooper @ 2018-03-30 21:52 UTC (permalink / raw)
  To: Razvan Cojocaru, xen-devel
  Cc: Boris Ostrovsky, Tamas K Lengyel, suravee.suthikulpanit

On 30/03/2018 22:47, Razvan Cojocaru wrote:
> On 03/31/2018 12:14 AM, Andrew Cooper wrote:
>> On 30/03/2018 22:06, Razvan Cojocaru wrote:
>>> Hello,
>>>
>>> We've been trying to enable mem_access events for SVM, and we do receive
>>> some events with xen-access if we set the pages read-only. However, the
>>> exec xen-access tests fails, because it appears that npfec.insn_fetch is
>>> always 0 here (or at least we haven't seen it to be 1 in a lot of testing):
>>>
>>> index 569b124..d518c94 100644
>>> --- a/xen/arch/x86/hvm/hvm.c
>>> +++ b/xen/arch/x86/hvm/hvm.c
>>> @@ -1835,6 +1835,13 @@ int hvm_hap_nested_page_fault(paddr_t gpa,
>>> unsigned long gla,
>>>              break;
>>>          }
>>>
>>> +        printk("pae: %d, nx: %d, gfn: 0x%lx access %d violation %d read %d"
>>> +               " write %d insn %d present %d glav %d kind %d \n",
>>> +               !!(curr->arch.hvm_vcpu.guest_cr[4] & X86_CR4_PAE),
>>> +               !!(curr->arch.hvm_vcpu.guest_efer & EFER_NX), gfn,
>>> +               p2ma, violation, npfec.read_access, npfec.write_access,
>>> +               npfec.insn_fetch, npfec.present, npfec.gla_valid,
>>> npfec.kind);
>>> +
>>>          if ( violation )
>>>          {
>>>              /* Should #VE be emulated for this fault? */
>>>
>>> This patch does not require anything else, just start an HVM guest on an
>>> AMD host. It outputs things like:
>>>
>>> (XEN) pae: 1, nx: 1, gfn: 0xf0235 access 7 violation 0 read 1 write 1
>>> insn 0 present 1 glav 0 kind 2
>>> [...]
>>> What are we missing?
>> Look at p2m-pt.c and check whether NX is being properly set in the NPT
>> tables when you select XENMEM_access_rw ?  NPT pagetables have an
>> identical format and layout to regular PAE pagetables.
> We weren't setting it at all. Just stored the restrictions in a
> newly-added integer in struct page_info (suggestions for the best place
> to store them are welcome), and fed them back to whatever function was
> asking for them from there.
>
> Did a quick test and setting _PAGE_NX_BIT as well seems to be the way to
> go indeed.

Well... If you don't set the real permissions bits, how is hardware
going to know that you don't want this page to be executed, and raise a
nested page fault in the first place?

~Andrew

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: SVM NPT mem_access and npfec.insn_fetch
  2018-03-30 21:52     ` Andrew Cooper
@ 2018-03-30 22:04       ` Razvan Cojocaru
  0 siblings, 0 replies; 5+ messages in thread
From: Razvan Cojocaru @ 2018-03-30 22:04 UTC (permalink / raw)
  To: Andrew Cooper, xen-devel
  Cc: Boris Ostrovsky, Tamas K Lengyel, suravee.suthikulpanit

On 03/31/2018 12:52 AM, Andrew Cooper wrote:
> Well... If you don't set the real permissions bits, how is hardware
> going to know that you don't want this page to be executed, and raise a
> nested page fault in the first place?

Right, it's obvious in retrospect. We were fooled by the way it seemed
to have worked for page writes out-of-the-box.


Thanks,
Razvan

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-03-30 22:04 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-30 21:06 SVM NPT mem_access and npfec.insn_fetch Razvan Cojocaru
2018-03-30 21:14 ` Andrew Cooper
2018-03-30 21:47   ` Razvan Cojocaru
2018-03-30 21:52     ` Andrew Cooper
2018-03-30 22:04       ` Razvan Cojocaru

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.