[PATCH v2] x86/altp2m: Allow setting the #VE info page for an arbitrary VCPU

[PATCH v2] x86/altp2m: Allow setting the #VE info page for an arbitrary VCPU

[Adrian Pop]

In a classic HVI + Xen setup, the introspection engine would monitor
legacy guest page-tables by marking them read-only inside the EPT; this
way any modification explicitly made by the guest or implicitly made by
the CPU page walker would trigger an EPT violation, which would be
forwarded by Xen to the SVA and thus the HVI agent.  The HVI agent would
analyse the modification, and act upon it - for example, a virtual page
may be remapped (its guest physical address changed inside the
page-table), in which case the introspection logic would update the
protection accordingly (remove EPT hook on the old gpa, and place a new
EPT hook on the new gpa).  In other cases, the modification may be of no
interest to the introspection engine - for example, the accessed/dirty
bits may be cleared by the operating system or the accessed/dirty bits
may be set by the CPU page walker.

In our tests we discovered that the vast majority of guest page-table
modifications fall in the second category (especially on Windows 10 RS4
x64 - more than 95% of ALL the page-table modifications are irrelevant to
us) - they are of no interest to the introspection logic, but they
trigger a very costly EPT violation nonetheless.  Therefore, we decided
to make use of the new #VE & VMFUNC features in recent Intel CPUs to
accelerate the guest page-tables monitoring in the following way:

1. Each monitored page-table would be flagged as being convertible
   inside the EPT, thus enabling the CPU to deliver a virtualization
   exception to he guest instead of generating a traditional EPT
   violation.
2. We inject a small filtering driver inside the protected guest VM,
   which would intercept the virtualization exception in order to handle
   guest page-table modifications.
3. We create a dedicated EPT view (altp2m) for the in-guest agent, which
   would isolate the agent from the rest of the operating system; the
   agent will switch in and out of the protected EPT view via the VMFUNC
   instruction placed inside a trampoline page, thus making the agent
   immune to malicious code inside the guest.

This way, all the page-table accesses would generate a
virtualization-exception inside the guest instead of a costly EPT
violation; the #VE agent would emulate and analyse the modification, and
decide whether it is relevant for the main introspection logic; if it is
relevant, it would do a VMCALL and notify the introspection engine
about the modification; otherwise, it would resume normal instruction
execution, thus avoiding a very costly VM exit.

Signed-off-by: Adrian Pop <apop@bitdefender.com>
---
Changes in v2:
- remove the "__get_vcpu()" helper
---
 tools/libxc/xc_altp2m.c |  1 -
 xen/arch/x86/hvm/hvm.c  | 19 ++++++++++---------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/tools/libxc/xc_altp2m.c b/tools/libxc/xc_altp2m.c
index ce4a1e4d60..528e929d7a 100644
--- a/tools/libxc/xc_altp2m.c
+++ b/tools/libxc/xc_altp2m.c
@@ -68,7 +68,6 @@ int xc_altp2m_set_domain_state(xc_interface *handle, uint32_t dom, bool state)
     return rc;
 }
 
-/* This is a bit odd to me that it acts on current.. */
 int xc_altp2m_set_vcpu_enable_notify(xc_interface *handle, uint32_t domid,
                                      uint32_t vcpuid, xen_pfn_t gfn)
 {
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 72c51faecb..49c3bbee94 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4533,8 +4533,7 @@ static int do_altp2m_op(
         return -EOPNOTSUPP;
     }
 
-    d = ( a.cmd != HVMOP_altp2m_vcpu_enable_notify ) ?
-        rcu_lock_domain_by_any_id(a.domain) : rcu_lock_current_domain();
+    d = rcu_lock_domain_by_any_id(a.domain);
 
     if ( d == NULL )
         return -ESRCH;
@@ -4605,26 +4604,28 @@ static int do_altp2m_op(
 
     case HVMOP_altp2m_vcpu_enable_notify:
     {
-        struct vcpu *curr = current;
+        struct vcpu *v;
         p2m_type_t p2mt;
 
-        if ( a.u.enable_notify.pad || a.domain != DOMID_SELF ||
-             a.u.enable_notify.vcpu_id != curr->vcpu_id )
+        if ( a.u.enable_notify.pad ||
+             a.u.enable_notify.vcpu_id >= d->max_vcpus )
         {
             rc = -EINVAL;
             break;
         }
 
-        if ( !gfn_eq(vcpu_altp2m(curr).veinfo_gfn, INVALID_GFN) ||
-             mfn_eq(get_gfn_query_unlocked(curr->domain,
+        v = d->vcpu[a.u.enable_notify.vcpu_id];
+
+        if ( !gfn_eq(vcpu_altp2m(v).veinfo_gfn, INVALID_GFN) ||
+             mfn_eq(get_gfn_query_unlocked(v->domain,
                     a.u.enable_notify.gfn, &p2mt), INVALID_MFN) )
         {
             rc = -EINVAL;
             break;
         }
 
-        vcpu_altp2m(curr).veinfo_gfn = _gfn(a.u.enable_notify.gfn);
-        altp2m_vcpu_update_vmfunc_ve(curr);
+        vcpu_altp2m(v).veinfo_gfn = _gfn(a.u.enable_notify.gfn);
+        altp2m_vcpu_update_vmfunc_ve(v);
         break;
     }
 
-- 
2.18.0


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

Re: [PATCH v2] x86/altp2m: Allow setting the #VE info page for an arbitrary VCPU

[Tamas K Lengyel]

On Mon, Sep 3, 2018 at 10:59 PM Adrian Pop <apop@bitdefender.com> wrote:
>
> In a classic HVI + Xen setup, the introspection engine would monitor
> legacy guest page-tables by marking them read-only inside the EPT; this
> way any modification explicitly made by the guest or implicitly made by
> the CPU page walker would trigger an EPT violation, which would be
> forwarded by Xen to the SVA and thus the HVI agent.  The HVI agent would
> analyse the modification, and act upon it - for example, a virtual page
> may be remapped (its guest physical address changed inside the
> page-table), in which case the introspection logic would update the
> protection accordingly (remove EPT hook on the old gpa, and place a new
> EPT hook on the new gpa).  In other cases, the modification may be of no
> interest to the introspection engine - for example, the accessed/dirty
> bits may be cleared by the operating system or the accessed/dirty bits
> may be set by the CPU page walker.
>
> In our tests we discovered that the vast majority of guest page-table
> modifications fall in the second category (especially on Windows 10 RS4
> x64 - more than 95% of ALL the page-table modifications are irrelevant to
> us) - they are of no interest to the introspection logic, but they
> trigger a very costly EPT violation nonetheless.  Therefore, we decided
> to make use of the new #VE & VMFUNC features in recent Intel CPUs to
> accelerate the guest page-tables monitoring in the following way:
>
> 1. Each monitored page-table would be flagged as being convertible
>    inside the EPT, thus enabling the CPU to deliver a virtualization
>    exception to he guest instead of generating a traditional EPT
>    violation.
> 2. We inject a small filtering driver inside the protected guest VM,
>    which would intercept the virtualization exception in order to handle
>    guest page-table modifications.
> 3. We create a dedicated EPT view (altp2m) for the in-guest agent, which
>    would isolate the agent from the rest of the operating system; the
>    agent will switch in and out of the protected EPT view via the VMFUNC
>    instruction placed inside a trampoline page, thus making the agent
>    immune to malicious code inside the guest.
>
> This way, all the page-table accesses would generate a
> virtualization-exception inside the guest instead of a costly EPT
> violation; the #VE agent would emulate and analyse the modification, and
> decide whether it is relevant for the main introspection logic; if it is
> relevant, it would do a VMCALL and notify the introspection engine
> about the modification; otherwise, it would resume normal instruction
> execution, thus avoiding a very costly VM exit.
>
> Signed-off-by: Adrian Pop <apop@bitdefender.com>
> ---
> Changes in v2:
> - remove the "__get_vcpu()" helper
> ---
>  tools/libxc/xc_altp2m.c |  1 -
>  xen/arch/x86/hvm/hvm.c  | 19 ++++++++++---------
>  2 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/tools/libxc/xc_altp2m.c b/tools/libxc/xc_altp2m.c
> index ce4a1e4d60..528e929d7a 100644
> --- a/tools/libxc/xc_altp2m.c
> +++ b/tools/libxc/xc_altp2m.c
> @@ -68,7 +68,6 @@ int xc_altp2m_set_domain_state(xc_interface *handle, uint32_t dom, bool state)
>      return rc;
>  }
>
> -/* This is a bit odd to me that it acts on current.. */
>  int xc_altp2m_set_vcpu_enable_notify(xc_interface *handle, uint32_t domid,
>                                       uint32_t vcpuid, xen_pfn_t gfn)
>  {
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index 72c51faecb..49c3bbee94 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -4533,8 +4533,7 @@ static int do_altp2m_op(
>          return -EOPNOTSUPP;
>      }
>
> -    d = ( a.cmd != HVMOP_altp2m_vcpu_enable_notify ) ?
> -        rcu_lock_domain_by_any_id(a.domain) : rcu_lock_current_domain();
> +    d = rcu_lock_domain_by_any_id(a.domain);

Does rcu_lock_domain_by_any_id work if its from the current domain? If
not, doesn't that change this function's accessibility to be from
exclusively usable only by the outside agent?

>
>      if ( d == NULL )
>          return -ESRCH;
> @@ -4605,26 +4604,28 @@ static int do_altp2m_op(
>
>      case HVMOP_altp2m_vcpu_enable_notify:
>      {
> -        struct vcpu *curr = current;
> +        struct vcpu *v;
>          p2m_type_t p2mt;
>
> -        if ( a.u.enable_notify.pad || a.domain != DOMID_SELF ||
> -             a.u.enable_notify.vcpu_id != curr->vcpu_id )
> +        if ( a.u.enable_notify.pad ||
> +             a.u.enable_notify.vcpu_id >= d->max_vcpus )
>          {
>              rc = -EINVAL;
>              break;
>          }
>
> -        if ( !gfn_eq(vcpu_altp2m(curr).veinfo_gfn, INVALID_GFN) ||
> -             mfn_eq(get_gfn_query_unlocked(curr->domain,
> +        v = d->vcpu[a.u.enable_notify.vcpu_id];
> +
> +        if ( !gfn_eq(vcpu_altp2m(v).veinfo_gfn, INVALID_GFN) ||
> +             mfn_eq(get_gfn_query_unlocked(v->domain,
>                      a.u.enable_notify.gfn, &p2mt), INVALID_MFN) )
>          {
>              rc = -EINVAL;
>              break;
>          }
>
> -        vcpu_altp2m(curr).veinfo_gfn = _gfn(a.u.enable_notify.gfn);
> -        altp2m_vcpu_update_vmfunc_ve(curr);
> +        vcpu_altp2m(v).veinfo_gfn = _gfn(a.u.enable_notify.gfn);
> +        altp2m_vcpu_update_vmfunc_ve(v);
>          break;
>      }
>
> --
> 2.18.0

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

Re: [PATCH v2] x86/altp2m: Allow setting the #VE info page for an arbitrary VCPU

[Razvan Cojocaru]

On 9/4/18 11:40 PM, Tamas K Lengyel wrote:
> On Mon, Sep 3, 2018 at 10:59 PM Adrian Pop <apop@bitdefender.com> wrote:
>>
>> In a classic HVI + Xen setup, the introspection engine would monitor
>> legacy guest page-tables by marking them read-only inside the EPT; this
>> way any modification explicitly made by the guest or implicitly made by
>> the CPU page walker would trigger an EPT violation, which would be
>> forwarded by Xen to the SVA and thus the HVI agent.  The HVI agent would
>> analyse the modification, and act upon it - for example, a virtual page
>> may be remapped (its guest physical address changed inside the
>> page-table), in which case the introspection logic would update the
>> protection accordingly (remove EPT hook on the old gpa, and place a new
>> EPT hook on the new gpa).  In other cases, the modification may be of no
>> interest to the introspection engine - for example, the accessed/dirty
>> bits may be cleared by the operating system or the accessed/dirty bits
>> may be set by the CPU page walker.
>>
>> In our tests we discovered that the vast majority of guest page-table
>> modifications fall in the second category (especially on Windows 10 RS4
>> x64 - more than 95% of ALL the page-table modifications are irrelevant to
>> us) - they are of no interest to the introspection logic, but they
>> trigger a very costly EPT violation nonetheless.  Therefore, we decided
>> to make use of the new #VE & VMFUNC features in recent Intel CPUs to
>> accelerate the guest page-tables monitoring in the following way:
>>
>> 1. Each monitored page-table would be flagged as being convertible
>>    inside the EPT, thus enabling the CPU to deliver a virtualization
>>    exception to he guest instead of generating a traditional EPT
>>    violation.
>> 2. We inject a small filtering driver inside the protected guest VM,
>>    which would intercept the virtualization exception in order to handle
>>    guest page-table modifications.
>> 3. We create a dedicated EPT view (altp2m) for the in-guest agent, which
>>    would isolate the agent from the rest of the operating system; the
>>    agent will switch in and out of the protected EPT view via the VMFUNC
>>    instruction placed inside a trampoline page, thus making the agent
>>    immune to malicious code inside the guest.
>>
>> This way, all the page-table accesses would generate a
>> virtualization-exception inside the guest instead of a costly EPT
>> violation; the #VE agent would emulate and analyse the modification, and
>> decide whether it is relevant for the main introspection logic; if it is
>> relevant, it would do a VMCALL and notify the introspection engine
>> about the modification; otherwise, it would resume normal instruction
>> execution, thus avoiding a very costly VM exit.
>>
>> Signed-off-by: Adrian Pop <apop@bitdefender.com>
>> ---
>> Changes in v2:
>> - remove the "__get_vcpu()" helper
>> ---
>>  tools/libxc/xc_altp2m.c |  1 -
>>  xen/arch/x86/hvm/hvm.c  | 19 ++++++++++---------
>>  2 files changed, 10 insertions(+), 10 deletions(-)
>>
>> diff --git a/tools/libxc/xc_altp2m.c b/tools/libxc/xc_altp2m.c
>> index ce4a1e4d60..528e929d7a 100644
>> --- a/tools/libxc/xc_altp2m.c
>> +++ b/tools/libxc/xc_altp2m.c
>> @@ -68,7 +68,6 @@ int xc_altp2m_set_domain_state(xc_interface *handle, uint32_t dom, bool state)
>>      return rc;
>>  }
>>
>> -/* This is a bit odd to me that it acts on current.. */
>>  int xc_altp2m_set_vcpu_enable_notify(xc_interface *handle, uint32_t domid,
>>                                       uint32_t vcpuid, xen_pfn_t gfn)
>>  {
>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
>> index 72c51faecb..49c3bbee94 100644
>> --- a/xen/arch/x86/hvm/hvm.c
>> +++ b/xen/arch/x86/hvm/hvm.c
>> @@ -4533,8 +4533,7 @@ static int do_altp2m_op(
>>          return -EOPNOTSUPP;
>>      }
>>
>> -    d = ( a.cmd != HVMOP_altp2m_vcpu_enable_notify ) ?
>> -        rcu_lock_domain_by_any_id(a.domain) : rcu_lock_current_domain();
>> +    d = rcu_lock_domain_by_any_id(a.domain);
> 
> Does rcu_lock_domain_by_any_id work if its from the current domain? If
> not, doesn't that change this function's accessibility to be from
> exclusively usable only by the outside agent?
The code says it should be safe:

 633 struct domain *rcu_lock_domain_by_any_id(domid_t dom)
 634 {
 635     if ( dom == DOMID_SELF )
 636         return rcu_lock_current_domain();
 637     return rcu_lock_domain_by_id(dom);
 638 }

as long as dom == DOMID_SELF. I think the old behaviour assumed that
HVMOP_altp2m_vcpu_enable_notify alone would only ever be used from the
current domain, and this change expands its usability (Adrian should
correct me if I'm wrong here).


Thanks,
Razvan

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

Re: [PATCH v2] x86/altp2m: Allow setting the #VE info page for an arbitrary VCPU

[Jan Beulich]

>>> On 04.09.18 at 22:58, <rcojocaru@bitdefender.com> wrote:
> On 9/4/18 11:40 PM, Tamas K Lengyel wrote:
>> On Mon, Sep 3, 2018 at 10:59 PM Adrian Pop <apop@bitdefender.com> wrote:
>>>
>>> In a classic HVI + Xen setup, the introspection engine would monitor
>>> legacy guest page-tables by marking them read-only inside the EPT; this
>>> way any modification explicitly made by the guest or implicitly made by
>>> the CPU page walker would trigger an EPT violation, which would be
>>> forwarded by Xen to the SVA and thus the HVI agent.  The HVI agent would
>>> analyse the modification, and act upon it - for example, a virtual page
>>> may be remapped (its guest physical address changed inside the
>>> page-table), in which case the introspection logic would update the
>>> protection accordingly (remove EPT hook on the old gpa, and place a new
>>> EPT hook on the new gpa).  In other cases, the modification may be of no
>>> interest to the introspection engine - for example, the accessed/dirty
>>> bits may be cleared by the operating system or the accessed/dirty bits
>>> may be set by the CPU page walker.
>>>
>>> In our tests we discovered that the vast majority of guest page-table
>>> modifications fall in the second category (especially on Windows 10 RS4
>>> x64 - more than 95% of ALL the page-table modifications are irrelevant to
>>> us) - they are of no interest to the introspection logic, but they
>>> trigger a very costly EPT violation nonetheless.  Therefore, we decided
>>> to make use of the new #VE & VMFUNC features in recent Intel CPUs to
>>> accelerate the guest page-tables monitoring in the following way:
>>>
>>> 1. Each monitored page-table would be flagged as being convertible
>>>    inside the EPT, thus enabling the CPU to deliver a virtualization
>>>    exception to he guest instead of generating a traditional EPT
>>>    violation.
>>> 2. We inject a small filtering driver inside the protected guest VM,
>>>    which would intercept the virtualization exception in order to handle
>>>    guest page-table modifications.
>>> 3. We create a dedicated EPT view (altp2m) for the in-guest agent, which
>>>    would isolate the agent from the rest of the operating system; the
>>>    agent will switch in and out of the protected EPT view via the VMFUNC
>>>    instruction placed inside a trampoline page, thus making the agent
>>>    immune to malicious code inside the guest.
>>>
>>> This way, all the page-table accesses would generate a
>>> virtualization-exception inside the guest instead of a costly EPT
>>> violation; the #VE agent would emulate and analyse the modification, and
>>> decide whether it is relevant for the main introspection logic; if it is
>>> relevant, it would do a VMCALL and notify the introspection engine
>>> about the modification; otherwise, it would resume normal instruction
>>> execution, thus avoiding a very costly VM exit.
>>>
>>> Signed-off-by: Adrian Pop <apop@bitdefender.com>
>>> ---
>>> Changes in v2:
>>> - remove the "__get_vcpu()" helper
>>> ---
>>>  tools/libxc/xc_altp2m.c |  1 -
>>>  xen/arch/x86/hvm/hvm.c  | 19 ++++++++++---------
>>>  2 files changed, 10 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/tools/libxc/xc_altp2m.c b/tools/libxc/xc_altp2m.c
>>> index ce4a1e4d60..528e929d7a 100644
>>> --- a/tools/libxc/xc_altp2m.c
>>> +++ b/tools/libxc/xc_altp2m.c
>>> @@ -68,7 +68,6 @@ int xc_altp2m_set_domain_state(xc_interface *handle, 
> uint32_t dom, bool state)
>>>      return rc;
>>>  }
>>>
>>> -/* This is a bit odd to me that it acts on current.. */
>>>  int xc_altp2m_set_vcpu_enable_notify(xc_interface *handle, uint32_t domid,
>>>                                       uint32_t vcpuid, xen_pfn_t gfn)
>>>  {
>>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
>>> index 72c51faecb..49c3bbee94 100644
>>> --- a/xen/arch/x86/hvm/hvm.c
>>> +++ b/xen/arch/x86/hvm/hvm.c
>>> @@ -4533,8 +4533,7 @@ static int do_altp2m_op(
>>>          return -EOPNOTSUPP;
>>>      }
>>>
>>> -    d = ( a.cmd != HVMOP_altp2m_vcpu_enable_notify ) ?
>>> -        rcu_lock_domain_by_any_id(a.domain) : rcu_lock_current_domain();
>>> +    d = rcu_lock_domain_by_any_id(a.domain);
>> 
>> Does rcu_lock_domain_by_any_id work if its from the current domain? If
>> not, doesn't that change this function's accessibility to be from
>> exclusively usable only by the outside agent?
> The code says it should be safe:
> 
>  633 struct domain *rcu_lock_domain_by_any_id(domid_t dom)
>  634 {
>  635     if ( dom == DOMID_SELF )
>  636         return rcu_lock_current_domain();
>  637     return rcu_lock_domain_by_id(dom);
>  638 }
> 
> as long as dom == DOMID_SELF. I think the old behaviour assumed that
> HVMOP_altp2m_vcpu_enable_notify alone would only ever be used from the
> current domain, and this change expands its usability (Adrian should
> correct me if I'm wrong here).

But a guest exposed interface can't be changed like this: If a.domain
was ignored for this sub-op before, it needs to continue to be ignored.

Jan


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

Re: [PATCH v2] x86/altp2m: Allow setting the #VE info page for an arbitrary VCPU

[Razvan Cojocaru]

On 9/5/18 9:56 AM, Jan Beulich wrote:
>>>> On 04.09.18 at 22:58, <rcojocaru@bitdefender.com> wrote:
>> On 9/4/18 11:40 PM, Tamas K Lengyel wrote:
>>> On Mon, Sep 3, 2018 at 10:59 PM Adrian Pop <apop@bitdefender.com> wrote:
>>>>
>>>> In a classic HVI + Xen setup, the introspection engine would monitor
>>>> legacy guest page-tables by marking them read-only inside the EPT; this
>>>> way any modification explicitly made by the guest or implicitly made by
>>>> the CPU page walker would trigger an EPT violation, which would be
>>>> forwarded by Xen to the SVA and thus the HVI agent.  The HVI agent would
>>>> analyse the modification, and act upon it - for example, a virtual page
>>>> may be remapped (its guest physical address changed inside the
>>>> page-table), in which case the introspection logic would update the
>>>> protection accordingly (remove EPT hook on the old gpa, and place a new
>>>> EPT hook on the new gpa).  In other cases, the modification may be of no
>>>> interest to the introspection engine - for example, the accessed/dirty
>>>> bits may be cleared by the operating system or the accessed/dirty bits
>>>> may be set by the CPU page walker.
>>>>
>>>> In our tests we discovered that the vast majority of guest page-table
>>>> modifications fall in the second category (especially on Windows 10 RS4
>>>> x64 - more than 95% of ALL the page-table modifications are irrelevant to
>>>> us) - they are of no interest to the introspection logic, but they
>>>> trigger a very costly EPT violation nonetheless.  Therefore, we decided
>>>> to make use of the new #VE & VMFUNC features in recent Intel CPUs to
>>>> accelerate the guest page-tables monitoring in the following way:
>>>>
>>>> 1. Each monitored page-table would be flagged as being convertible
>>>>    inside the EPT, thus enabling the CPU to deliver a virtualization
>>>>    exception to he guest instead of generating a traditional EPT
>>>>    violation.
>>>> 2. We inject a small filtering driver inside the protected guest VM,
>>>>    which would intercept the virtualization exception in order to handle
>>>>    guest page-table modifications.
>>>> 3. We create a dedicated EPT view (altp2m) for the in-guest agent, which
>>>>    would isolate the agent from the rest of the operating system; the
>>>>    agent will switch in and out of the protected EPT view via the VMFUNC
>>>>    instruction placed inside a trampoline page, thus making the agent
>>>>    immune to malicious code inside the guest.
>>>>
>>>> This way, all the page-table accesses would generate a
>>>> virtualization-exception inside the guest instead of a costly EPT
>>>> violation; the #VE agent would emulate and analyse the modification, and
>>>> decide whether it is relevant for the main introspection logic; if it is
>>>> relevant, it would do a VMCALL and notify the introspection engine
>>>> about the modification; otherwise, it would resume normal instruction
>>>> execution, thus avoiding a very costly VM exit.
>>>>
>>>> Signed-off-by: Adrian Pop <apop@bitdefender.com>
>>>> ---
>>>> Changes in v2:
>>>> - remove the "__get_vcpu()" helper
>>>> ---
>>>>  tools/libxc/xc_altp2m.c |  1 -
>>>>  xen/arch/x86/hvm/hvm.c  | 19 ++++++++++---------
>>>>  2 files changed, 10 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/tools/libxc/xc_altp2m.c b/tools/libxc/xc_altp2m.c
>>>> index ce4a1e4d60..528e929d7a 100644
>>>> --- a/tools/libxc/xc_altp2m.c
>>>> +++ b/tools/libxc/xc_altp2m.c
>>>> @@ -68,7 +68,6 @@ int xc_altp2m_set_domain_state(xc_interface *handle, 
>> uint32_t dom, bool state)
>>>>      return rc;
>>>>  }
>>>>
>>>> -/* This is a bit odd to me that it acts on current.. */
>>>>  int xc_altp2m_set_vcpu_enable_notify(xc_interface *handle, uint32_t domid,
>>>>                                       uint32_t vcpuid, xen_pfn_t gfn)
>>>>  {
>>>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
>>>> index 72c51faecb..49c3bbee94 100644
>>>> --- a/xen/arch/x86/hvm/hvm.c
>>>> +++ b/xen/arch/x86/hvm/hvm.c
>>>> @@ -4533,8 +4533,7 @@ static int do_altp2m_op(
>>>>          return -EOPNOTSUPP;
>>>>      }
>>>>
>>>> -    d = ( a.cmd != HVMOP_altp2m_vcpu_enable_notify ) ?
>>>> -        rcu_lock_domain_by_any_id(a.domain) : rcu_lock_current_domain();
>>>> +    d = rcu_lock_domain_by_any_id(a.domain);
>>>
>>> Does rcu_lock_domain_by_any_id work if its from the current domain? If
>>> not, doesn't that change this function's accessibility to be from
>>> exclusively usable only by the outside agent?
>> The code says it should be safe:
>>
>>  633 struct domain *rcu_lock_domain_by_any_id(domid_t dom)
>>  634 {
>>  635     if ( dom == DOMID_SELF )
>>  636         return rcu_lock_current_domain();
>>  637     return rcu_lock_domain_by_id(dom);
>>  638 }
>>
>> as long as dom == DOMID_SELF. I think the old behaviour assumed that
>> HVMOP_altp2m_vcpu_enable_notify alone would only ever be used from the
>> current domain, and this change expands its usability (Adrian should
>> correct me if I'm wrong here).
> 
> But a guest exposed interface can't be changed like this: If a.domain
> was ignored for this sub-op before, it needs to continue to be ignored.

The point of this patch is twofold (as hopefully explained, or at least
implied, by the use-case in the description): one is to now allow
setting the #VE information for any VCPU, not just current; the second,
to be able to make the call from any sufficiently privileged domain (so
as to allow the introspection agent to control behaviour).


Thanks,
Razvan

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

Re: [PATCH v2] x86/altp2m: Allow setting the #VE info page for an arbitrary VCPU

[Razvan Cojocaru]

On 9/5/18 9:56 AM, Jan Beulich wrote:
>>>> On 04.09.18 at 22:58, <rcojocaru@bitdefender.com> wrote:
>> On 9/4/18 11:40 PM, Tamas K Lengyel wrote:
>>> On Mon, Sep 3, 2018 at 10:59 PM Adrian Pop <apop@bitdefender.com> wrote:
>>>>
>>>> In a classic HVI + Xen setup, the introspection engine would monitor
>>>> legacy guest page-tables by marking them read-only inside the EPT; this
>>>> way any modification explicitly made by the guest or implicitly made by
>>>> the CPU page walker would trigger an EPT violation, which would be
>>>> forwarded by Xen to the SVA and thus the HVI agent.  The HVI agent would
>>>> analyse the modification, and act upon it - for example, a virtual page
>>>> may be remapped (its guest physical address changed inside the
>>>> page-table), in which case the introspection logic would update the
>>>> protection accordingly (remove EPT hook on the old gpa, and place a new
>>>> EPT hook on the new gpa).  In other cases, the modification may be of no
>>>> interest to the introspection engine - for example, the accessed/dirty
>>>> bits may be cleared by the operating system or the accessed/dirty bits
>>>> may be set by the CPU page walker.
>>>>
>>>> In our tests we discovered that the vast majority of guest page-table
>>>> modifications fall in the second category (especially on Windows 10 RS4
>>>> x64 - more than 95% of ALL the page-table modifications are irrelevant to
>>>> us) - they are of no interest to the introspection logic, but they
>>>> trigger a very costly EPT violation nonetheless.  Therefore, we decided
>>>> to make use of the new #VE & VMFUNC features in recent Intel CPUs to
>>>> accelerate the guest page-tables monitoring in the following way:
>>>>
>>>> 1. Each monitored page-table would be flagged as being convertible
>>>>    inside the EPT, thus enabling the CPU to deliver a virtualization
>>>>    exception to he guest instead of generating a traditional EPT
>>>>    violation.
>>>> 2. We inject a small filtering driver inside the protected guest VM,
>>>>    which would intercept the virtualization exception in order to handle
>>>>    guest page-table modifications.
>>>> 3. We create a dedicated EPT view (altp2m) for the in-guest agent, which
>>>>    would isolate the agent from the rest of the operating system; the
>>>>    agent will switch in and out of the protected EPT view via the VMFUNC
>>>>    instruction placed inside a trampoline page, thus making the agent
>>>>    immune to malicious code inside the guest.
>>>>
>>>> This way, all the page-table accesses would generate a
>>>> virtualization-exception inside the guest instead of a costly EPT
>>>> violation; the #VE agent would emulate and analyse the modification, and
>>>> decide whether it is relevant for the main introspection logic; if it is
>>>> relevant, it would do a VMCALL and notify the introspection engine
>>>> about the modification; otherwise, it would resume normal instruction
>>>> execution, thus avoiding a very costly VM exit.
>>>>
>>>> Signed-off-by: Adrian Pop <apop@bitdefender.com>
>>>> ---
>>>> Changes in v2:
>>>> - remove the "__get_vcpu()" helper
>>>> ---
>>>>  tools/libxc/xc_altp2m.c |  1 -
>>>>  xen/arch/x86/hvm/hvm.c  | 19 ++++++++++---------
>>>>  2 files changed, 10 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/tools/libxc/xc_altp2m.c b/tools/libxc/xc_altp2m.c
>>>> index ce4a1e4d60..528e929d7a 100644
>>>> --- a/tools/libxc/xc_altp2m.c
>>>> +++ b/tools/libxc/xc_altp2m.c
>>>> @@ -68,7 +68,6 @@ int xc_altp2m_set_domain_state(xc_interface *handle, 
>> uint32_t dom, bool state)
>>>>      return rc;
>>>>  }
>>>>
>>>> -/* This is a bit odd to me that it acts on current.. */
>>>>  int xc_altp2m_set_vcpu_enable_notify(xc_interface *handle, uint32_t domid,
>>>>                                       uint32_t vcpuid, xen_pfn_t gfn)
>>>>  {
>>>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
>>>> index 72c51faecb..49c3bbee94 100644
>>>> --- a/xen/arch/x86/hvm/hvm.c
>>>> +++ b/xen/arch/x86/hvm/hvm.c
>>>> @@ -4533,8 +4533,7 @@ static int do_altp2m_op(
>>>>          return -EOPNOTSUPP;
>>>>      }
>>>>
>>>> -    d = ( a.cmd != HVMOP_altp2m_vcpu_enable_notify ) ?
>>>> -        rcu_lock_domain_by_any_id(a.domain) : rcu_lock_current_domain();
>>>> +    d = rcu_lock_domain_by_any_id(a.domain);
>>>
>>> Does rcu_lock_domain_by_any_id work if its from the current domain? If
>>> not, doesn't that change this function's accessibility to be from
>>> exclusively usable only by the outside agent?
>> The code says it should be safe:
>>
>>  633 struct domain *rcu_lock_domain_by_any_id(domid_t dom)
>>  634 {
>>  635     if ( dom == DOMID_SELF )
>>  636         return rcu_lock_current_domain();
>>  637     return rcu_lock_domain_by_id(dom);
>>  638 }
>>
>> as long as dom == DOMID_SELF. I think the old behaviour assumed that
>> HVMOP_altp2m_vcpu_enable_notify alone would only ever be used from the
>> current domain, and this change expands its usability (Adrian should
>> correct me if I'm wrong here).
> 
> But a guest exposed interface can't be changed like this: If a.domain
> was ignored for this sub-op before, it needs to continue to be ignored.

Also, technically speaking a.domain is not currently ignored - it's just
checked against DOMID_SELF:

4596     case HVMOP_altp2m_vcpu_enable_notify:
4597     {
4598         struct vcpu *curr = current;
4599         p2m_type_t p2mt;
4600
4601         if ( a.u.enable_notify.pad || a.domain != DOMID_SELF ||
4602              a.u.enable_notify.vcpu_id != curr->vcpu_id )
4603         {
4604             rc = -EINVAL;
4605             break;
4606         }
4607
4608         if ( !gfn_eq(vcpu_altp2m(curr).veinfo_gfn, INVALID_GFN) ||
4609              mfn_eq(get_gfn_query_unlocked(curr->domain,
4610                     a.u.enable_notify.gfn, &p2mt), INVALID_MFN) )
4611         {
4612             rc = -EINVAL;
4613             break;
4614         }
4615
4616         vcpu_altp2m(curr).veinfo_gfn = _gfn(a.u.enable_notify.gfn);
4617         altp2m_vcpu_update_vmfunc_ve(curr);
4618         break;
4619     }
4620


Thanks,
Razvan

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

Re: [PATCH v2] x86/altp2m: Allow setting the #VE info page for an arbitrary VCPU

[Jan Beulich]

>>> On 05.09.18 at 10:14, <rcojocaru@bitdefender.com> wrote:
> On 9/5/18 9:56 AM, Jan Beulich wrote:
>>>>> On 04.09.18 at 22:58, <rcojocaru@bitdefender.com> wrote:
>>> On 9/4/18 11:40 PM, Tamas K Lengyel wrote:
>>>> On Mon, Sep 3, 2018 at 10:59 PM Adrian Pop <apop@bitdefender.com> wrote:
>>>>> --- a/xen/arch/x86/hvm/hvm.c
>>>>> +++ b/xen/arch/x86/hvm/hvm.c
>>>>> @@ -4533,8 +4533,7 @@ static int do_altp2m_op(
>>>>>          return -EOPNOTSUPP;
>>>>>      }
>>>>>
>>>>> -    d = ( a.cmd != HVMOP_altp2m_vcpu_enable_notify ) ?
>>>>> -        rcu_lock_domain_by_any_id(a.domain) : rcu_lock_current_domain();
>>>>> +    d = rcu_lock_domain_by_any_id(a.domain);
>>>>
>>>> Does rcu_lock_domain_by_any_id work if its from the current domain? If
>>>> not, doesn't that change this function's accessibility to be from
>>>> exclusively usable only by the outside agent?
>>> The code says it should be safe:
>>>
>>>  633 struct domain *rcu_lock_domain_by_any_id(domid_t dom)
>>>  634 {
>>>  635     if ( dom == DOMID_SELF )
>>>  636         return rcu_lock_current_domain();
>>>  637     return rcu_lock_domain_by_id(dom);
>>>  638 }
>>>
>>> as long as dom == DOMID_SELF. I think the old behaviour assumed that
>>> HVMOP_altp2m_vcpu_enable_notify alone would only ever be used from the
>>> current domain, and this change expands its usability (Adrian should
>>> correct me if I'm wrong here).
>> 
>> But a guest exposed interface can't be changed like this: If a.domain
>> was ignored for this sub-op before, it needs to continue to be ignored.
> 
> Also, technically speaking a.domain is not currently ignored - it's just
> checked against DOMID_SELF:
> 
> 4596     case HVMOP_altp2m_vcpu_enable_notify:
> 4597     {
> 4598         struct vcpu *curr = current;
> 4599         p2m_type_t p2mt;
> 4600
> 4601         if ( a.u.enable_notify.pad || a.domain != DOMID_SELF ||
> 4602              a.u.enable_notify.vcpu_id != curr->vcpu_id )
> 4603         {
> 4604             rc = -EINVAL;
> 4605             break;
> 4606         }

Ah, yes, I think that's fine then.

Jan



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

Re: [PATCH v2] x86/altp2m: Allow setting the #VE info page for an arbitrary VCPU

[Tamas K Lengyel]

On Tue, Sep 4, 2018 at 2:58 PM Razvan Cojocaru
<rcojocaru@bitdefender.com> wrote:
>
> On 9/4/18 11:40 PM, Tamas K Lengyel wrote:
> > On Mon, Sep 3, 2018 at 10:59 PM Adrian Pop <apop@bitdefender.com> wrote:
> >>
> >> In a classic HVI + Xen setup, the introspection engine would monitor
> >> legacy guest page-tables by marking them read-only inside the EPT; this
> >> way any modification explicitly made by the guest or implicitly made by
> >> the CPU page walker would trigger an EPT violation, which would be
> >> forwarded by Xen to the SVA and thus the HVI agent.  The HVI agent would
> >> analyse the modification, and act upon it - for example, a virtual page
> >> may be remapped (its guest physical address changed inside the
> >> page-table), in which case the introspection logic would update the
> >> protection accordingly (remove EPT hook on the old gpa, and place a new
> >> EPT hook on the new gpa).  In other cases, the modification may be of no
> >> interest to the introspection engine - for example, the accessed/dirty
> >> bits may be cleared by the operating system or the accessed/dirty bits
> >> may be set by the CPU page walker.
> >>
> >> In our tests we discovered that the vast majority of guest page-table
> >> modifications fall in the second category (especially on Windows 10 RS4
> >> x64 - more than 95% of ALL the page-table modifications are irrelevant to
> >> us) - they are of no interest to the introspection logic, but they
> >> trigger a very costly EPT violation nonetheless.  Therefore, we decided
> >> to make use of the new #VE & VMFUNC features in recent Intel CPUs to
> >> accelerate the guest page-tables monitoring in the following way:
> >>
> >> 1. Each monitored page-table would be flagged as being convertible
> >>    inside the EPT, thus enabling the CPU to deliver a virtualization
> >>    exception to he guest instead of generating a traditional EPT
> >>    violation.
> >> 2. We inject a small filtering driver inside the protected guest VM,
> >>    which would intercept the virtualization exception in order to handle
> >>    guest page-table modifications.
> >> 3. We create a dedicated EPT view (altp2m) for the in-guest agent, which
> >>    would isolate the agent from the rest of the operating system; the
> >>    agent will switch in and out of the protected EPT view via the VMFUNC
> >>    instruction placed inside a trampoline page, thus making the agent
> >>    immune to malicious code inside the guest.
> >>
> >> This way, all the page-table accesses would generate a
> >> virtualization-exception inside the guest instead of a costly EPT
> >> violation; the #VE agent would emulate and analyse the modification, and
> >> decide whether it is relevant for the main introspection logic; if it is
> >> relevant, it would do a VMCALL and notify the introspection engine
> >> about the modification; otherwise, it would resume normal instruction
> >> execution, thus avoiding a very costly VM exit.
> >>
> >> Signed-off-by: Adrian Pop <apop@bitdefender.com>
> >> ---
> >> Changes in v2:
> >> - remove the "__get_vcpu()" helper
> >> ---
> >>  tools/libxc/xc_altp2m.c |  1 -
> >>  xen/arch/x86/hvm/hvm.c  | 19 ++++++++++---------
> >>  2 files changed, 10 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/tools/libxc/xc_altp2m.c b/tools/libxc/xc_altp2m.c
> >> index ce4a1e4d60..528e929d7a 100644
> >> --- a/tools/libxc/xc_altp2m.c
> >> +++ b/tools/libxc/xc_altp2m.c
> >> @@ -68,7 +68,6 @@ int xc_altp2m_set_domain_state(xc_interface *handle, uint32_t dom, bool state)
> >>      return rc;
> >>  }
> >>
> >> -/* This is a bit odd to me that it acts on current.. */
> >>  int xc_altp2m_set_vcpu_enable_notify(xc_interface *handle, uint32_t domid,
> >>                                       uint32_t vcpuid, xen_pfn_t gfn)
> >>  {
> >> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> >> index 72c51faecb..49c3bbee94 100644
> >> --- a/xen/arch/x86/hvm/hvm.c
> >> +++ b/xen/arch/x86/hvm/hvm.c
> >> @@ -4533,8 +4533,7 @@ static int do_altp2m_op(
> >>          return -EOPNOTSUPP;
> >>      }
> >>
> >> -    d = ( a.cmd != HVMOP_altp2m_vcpu_enable_notify ) ?
> >> -        rcu_lock_domain_by_any_id(a.domain) : rcu_lock_current_domain();
> >> +    d = rcu_lock_domain_by_any_id(a.domain);
> >
> > Does rcu_lock_domain_by_any_id work if its from the current domain? If
> > not, doesn't that change this function's accessibility to be from
> > exclusively usable only by the outside agent?
> The code says it should be safe:
>
>  633 struct domain *rcu_lock_domain_by_any_id(domid_t dom)
>  634 {
>  635     if ( dom == DOMID_SELF )
>  636         return rcu_lock_current_domain();
>  637     return rcu_lock_domain_by_id(dom);
>  638 }
>
> as long as dom == DOMID_SELF. I think the old behaviour assumed that
> HVMOP_altp2m_vcpu_enable_notify alone would only ever be used from the
> current domain, and this change expands its usability (Adrian should
> correct me if I'm wrong here).

Sounds good, thanks!

Tamas

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

Re: [PATCH v2] x86/altp2m: Allow setting the #VE info page for an arbitrary VCPU

[Razvan Cojocaru]

On 9/5/18 7:28 PM, Tamas K Lengyel wrote:
> On Tue, Sep 4, 2018 at 2:58 PM Razvan Cojocaru
> <rcojocaru@bitdefender.com> wrote:
>>
>> On 9/4/18 11:40 PM, Tamas K Lengyel wrote:
>>> On Mon, Sep 3, 2018 at 10:59 PM Adrian Pop <apop@bitdefender.com> wrote:
>>>>
>>>> In a classic HVI + Xen setup, the introspection engine would monitor
>>>> legacy guest page-tables by marking them read-only inside the EPT; this
>>>> way any modification explicitly made by the guest or implicitly made by
>>>> the CPU page walker would trigger an EPT violation, which would be
>>>> forwarded by Xen to the SVA and thus the HVI agent.  The HVI agent would
>>>> analyse the modification, and act upon it - for example, a virtual page
>>>> may be remapped (its guest physical address changed inside the
>>>> page-table), in which case the introspection logic would update the
>>>> protection accordingly (remove EPT hook on the old gpa, and place a new
>>>> EPT hook on the new gpa).  In other cases, the modification may be of no
>>>> interest to the introspection engine - for example, the accessed/dirty
>>>> bits may be cleared by the operating system or the accessed/dirty bits
>>>> may be set by the CPU page walker.
>>>>
>>>> In our tests we discovered that the vast majority of guest page-table
>>>> modifications fall in the second category (especially on Windows 10 RS4
>>>> x64 - more than 95% of ALL the page-table modifications are irrelevant to
>>>> us) - they are of no interest to the introspection logic, but they
>>>> trigger a very costly EPT violation nonetheless.  Therefore, we decided
>>>> to make use of the new #VE & VMFUNC features in recent Intel CPUs to
>>>> accelerate the guest page-tables monitoring in the following way:
>>>>
>>>> 1. Each monitored page-table would be flagged as being convertible
>>>>    inside the EPT, thus enabling the CPU to deliver a virtualization
>>>>    exception to he guest instead of generating a traditional EPT
>>>>    violation.
>>>> 2. We inject a small filtering driver inside the protected guest VM,
>>>>    which would intercept the virtualization exception in order to handle
>>>>    guest page-table modifications.
>>>> 3. We create a dedicated EPT view (altp2m) for the in-guest agent, which
>>>>    would isolate the agent from the rest of the operating system; the
>>>>    agent will switch in and out of the protected EPT view via the VMFUNC
>>>>    instruction placed inside a trampoline page, thus making the agent
>>>>    immune to malicious code inside the guest.
>>>>
>>>> This way, all the page-table accesses would generate a
>>>> virtualization-exception inside the guest instead of a costly EPT
>>>> violation; the #VE agent would emulate and analyse the modification, and
>>>> decide whether it is relevant for the main introspection logic; if it is
>>>> relevant, it would do a VMCALL and notify the introspection engine
>>>> about the modification; otherwise, it would resume normal instruction
>>>> execution, thus avoiding a very costly VM exit.
>>>>
>>>> Signed-off-by: Adrian Pop <apop@bitdefender.com>
>>>> ---
>>>> Changes in v2:
>>>> - remove the "__get_vcpu()" helper
>>>> ---
>>>>  tools/libxc/xc_altp2m.c |  1 -
>>>>  xen/arch/x86/hvm/hvm.c  | 19 ++++++++++---------
>>>>  2 files changed, 10 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/tools/libxc/xc_altp2m.c b/tools/libxc/xc_altp2m.c
>>>> index ce4a1e4d60..528e929d7a 100644
>>>> --- a/tools/libxc/xc_altp2m.c
>>>> +++ b/tools/libxc/xc_altp2m.c
>>>> @@ -68,7 +68,6 @@ int xc_altp2m_set_domain_state(xc_interface *handle, uint32_t dom, bool state)
>>>>      return rc;
>>>>  }
>>>>
>>>> -/* This is a bit odd to me that it acts on current.. */
>>>>  int xc_altp2m_set_vcpu_enable_notify(xc_interface *handle, uint32_t domid,
>>>>                                       uint32_t vcpuid, xen_pfn_t gfn)
>>>>  {
>>>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
>>>> index 72c51faecb..49c3bbee94 100644
>>>> --- a/xen/arch/x86/hvm/hvm.c
>>>> +++ b/xen/arch/x86/hvm/hvm.c
>>>> @@ -4533,8 +4533,7 @@ static int do_altp2m_op(
>>>>          return -EOPNOTSUPP;
>>>>      }
>>>>
>>>> -    d = ( a.cmd != HVMOP_altp2m_vcpu_enable_notify ) ?
>>>> -        rcu_lock_domain_by_any_id(a.domain) : rcu_lock_current_domain();
>>>> +    d = rcu_lock_domain_by_any_id(a.domain);
>>>
>>> Does rcu_lock_domain_by_any_id work if its from the current domain? If
>>> not, doesn't that change this function's accessibility to be from
>>> exclusively usable only by the outside agent?
>> The code says it should be safe:
>>
>>  633 struct domain *rcu_lock_domain_by_any_id(domid_t dom)
>>  634 {
>>  635     if ( dom == DOMID_SELF )
>>  636         return rcu_lock_current_domain();
>>  637     return rcu_lock_domain_by_id(dom);
>>  638 }
>>
>> as long as dom == DOMID_SELF. I think the old behaviour assumed that
>> HVMOP_altp2m_vcpu_enable_notify alone would only ever be used from the
>> current domain, and this change expands its usability (Adrian should
>> correct me if I'm wrong here).
> 
> Sounds good, thanks!

May we take that as an Acked-by, or are there are other things you think
we should address?


Thanks,
Razvan

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

Re: [PATCH v2] x86/altp2m: Allow setting the #VE info page for an arbitrary VCPU

[Tamas K Lengyel]

On Wed, Sep 5, 2018 at 10:40 AM Razvan Cojocaru
<rcojocaru@bitdefender.com> wrote:
>
> On 9/5/18 7:28 PM, Tamas K Lengyel wrote:
> > On Tue, Sep 4, 2018 at 2:58 PM Razvan Cojocaru
> > <rcojocaru@bitdefender.com> wrote:
> >>
> >> On 9/4/18 11:40 PM, Tamas K Lengyel wrote:
> >>> On Mon, Sep 3, 2018 at 10:59 PM Adrian Pop <apop@bitdefender.com> wrote:
> >>>>
> >>>> In a classic HVI + Xen setup, the introspection engine would monitor
> >>>> legacy guest page-tables by marking them read-only inside the EPT; this
> >>>> way any modification explicitly made by the guest or implicitly made by
> >>>> the CPU page walker would trigger an EPT violation, which would be
> >>>> forwarded by Xen to the SVA and thus the HVI agent.  The HVI agent would
> >>>> analyse the modification, and act upon it - for example, a virtual page
> >>>> may be remapped (its guest physical address changed inside the
> >>>> page-table), in which case the introspection logic would update the
> >>>> protection accordingly (remove EPT hook on the old gpa, and place a new
> >>>> EPT hook on the new gpa).  In other cases, the modification may be of no
> >>>> interest to the introspection engine - for example, the accessed/dirty
> >>>> bits may be cleared by the operating system or the accessed/dirty bits
> >>>> may be set by the CPU page walker.
> >>>>
> >>>> In our tests we discovered that the vast majority of guest page-table
> >>>> modifications fall in the second category (especially on Windows 10 RS4
> >>>> x64 - more than 95% of ALL the page-table modifications are irrelevant to
> >>>> us) - they are of no interest to the introspection logic, but they
> >>>> trigger a very costly EPT violation nonetheless.  Therefore, we decided
> >>>> to make use of the new #VE & VMFUNC features in recent Intel CPUs to
> >>>> accelerate the guest page-tables monitoring in the following way:
> >>>>
> >>>> 1. Each monitored page-table would be flagged as being convertible
> >>>>    inside the EPT, thus enabling the CPU to deliver a virtualization
> >>>>    exception to he guest instead of generating a traditional EPT
> >>>>    violation.
> >>>> 2. We inject a small filtering driver inside the protected guest VM,
> >>>>    which would intercept the virtualization exception in order to handle
> >>>>    guest page-table modifications.
> >>>> 3. We create a dedicated EPT view (altp2m) for the in-guest agent, which
> >>>>    would isolate the agent from the rest of the operating system; the
> >>>>    agent will switch in and out of the protected EPT view via the VMFUNC
> >>>>    instruction placed inside a trampoline page, thus making the agent
> >>>>    immune to malicious code inside the guest.
> >>>>
> >>>> This way, all the page-table accesses would generate a
> >>>> virtualization-exception inside the guest instead of a costly EPT
> >>>> violation; the #VE agent would emulate and analyse the modification, and
> >>>> decide whether it is relevant for the main introspection logic; if it is
> >>>> relevant, it would do a VMCALL and notify the introspection engine
> >>>> about the modification; otherwise, it would resume normal instruction
> >>>> execution, thus avoiding a very costly VM exit.
> >>>>
> >>>> Signed-off-by: Adrian Pop <apop@bitdefender.com>
> >>>> ---
> >>>> Changes in v2:
> >>>> - remove the "__get_vcpu()" helper
> >>>> ---
> >>>>  tools/libxc/xc_altp2m.c |  1 -
> >>>>  xen/arch/x86/hvm/hvm.c  | 19 ++++++++++---------
> >>>>  2 files changed, 10 insertions(+), 10 deletions(-)
> >>>>
> >>>> diff --git a/tools/libxc/xc_altp2m.c b/tools/libxc/xc_altp2m.c
> >>>> index ce4a1e4d60..528e929d7a 100644
> >>>> --- a/tools/libxc/xc_altp2m.c
> >>>> +++ b/tools/libxc/xc_altp2m.c
> >>>> @@ -68,7 +68,6 @@ int xc_altp2m_set_domain_state(xc_interface *handle, uint32_t dom, bool state)
> >>>>      return rc;
> >>>>  }
> >>>>
> >>>> -/* This is a bit odd to me that it acts on current.. */
> >>>>  int xc_altp2m_set_vcpu_enable_notify(xc_interface *handle, uint32_t domid,
> >>>>                                       uint32_t vcpuid, xen_pfn_t gfn)
> >>>>  {
> >>>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> >>>> index 72c51faecb..49c3bbee94 100644
> >>>> --- a/xen/arch/x86/hvm/hvm.c
> >>>> +++ b/xen/arch/x86/hvm/hvm.c
> >>>> @@ -4533,8 +4533,7 @@ static int do_altp2m_op(
> >>>>          return -EOPNOTSUPP;
> >>>>      }
> >>>>
> >>>> -    d = ( a.cmd != HVMOP_altp2m_vcpu_enable_notify ) ?
> >>>> -        rcu_lock_domain_by_any_id(a.domain) : rcu_lock_current_domain();
> >>>> +    d = rcu_lock_domain_by_any_id(a.domain);
> >>>
> >>> Does rcu_lock_domain_by_any_id work if its from the current domain? If
> >>> not, doesn't that change this function's accessibility to be from
> >>> exclusively usable only by the outside agent?
> >> The code says it should be safe:
> >>
> >>  633 struct domain *rcu_lock_domain_by_any_id(domid_t dom)
> >>  634 {
> >>  635     if ( dom == DOMID_SELF )
> >>  636         return rcu_lock_current_domain();
> >>  637     return rcu_lock_domain_by_id(dom);
> >>  638 }
> >>
> >> as long as dom == DOMID_SELF. I think the old behaviour assumed that
> >> HVMOP_altp2m_vcpu_enable_notify alone would only ever be used from the
> >> current domain, and this change expands its usability (Adrian should
> >> correct me if I'm wrong here).
> >
> > Sounds good, thanks!
>
> May we take that as an Acked-by, or are there are other things you think
> we should address?

A Reviewed-by would be appropriate, I don't think the files touched in
this patch fall under our umbrella.

Tamas

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

Re: [PATCH v2] x86/altp2m: Allow setting the #VE info page for an arbitrary VCPU

[Andrew Cooper]

On 05/09/18 19:40, Tamas K Lengyel wrote:
> On Wed, Sep 5, 2018 at 10:40 AM Razvan Cojocaru
> <rcojocaru@bitdefender.com> wrote:
>> On 9/5/18 7:28 PM, Tamas K Lengyel wrote:
>>> On Tue, Sep 4, 2018 at 2:58 PM Razvan Cojocaru
>>> <rcojocaru@bitdefender.com> wrote:
>>>> On 9/4/18 11:40 PM, Tamas K Lengyel wrote:
>>>>> On Mon, Sep 3, 2018 at 10:59 PM Adrian Pop <apop@bitdefender.com> wrote:
>>>>>> In a classic HVI + Xen setup, the introspection engine would monitor
>>>>>> legacy guest page-tables by marking them read-only inside the EPT; this
>>>>>> way any modification explicitly made by the guest or implicitly made by
>>>>>> the CPU page walker would trigger an EPT violation, which would be
>>>>>> forwarded by Xen to the SVA and thus the HVI agent.  The HVI agent would
>>>>>> analyse the modification, and act upon it - for example, a virtual page
>>>>>> may be remapped (its guest physical address changed inside the
>>>>>> page-table), in which case the introspection logic would update the
>>>>>> protection accordingly (remove EPT hook on the old gpa, and place a new
>>>>>> EPT hook on the new gpa).  In other cases, the modification may be of no
>>>>>> interest to the introspection engine - for example, the accessed/dirty
>>>>>> bits may be cleared by the operating system or the accessed/dirty bits
>>>>>> may be set by the CPU page walker.
>>>>>>
>>>>>> In our tests we discovered that the vast majority of guest page-table
>>>>>> modifications fall in the second category (especially on Windows 10 RS4
>>>>>> x64 - more than 95% of ALL the page-table modifications are irrelevant to
>>>>>> us) - they are of no interest to the introspection logic, but they
>>>>>> trigger a very costly EPT violation nonetheless.  Therefore, we decided
>>>>>> to make use of the new #VE & VMFUNC features in recent Intel CPUs to
>>>>>> accelerate the guest page-tables monitoring in the following way:
>>>>>>
>>>>>> 1. Each monitored page-table would be flagged as being convertible
>>>>>>    inside the EPT, thus enabling the CPU to deliver a virtualization
>>>>>>    exception to he guest instead of generating a traditional EPT
>>>>>>    violation.
>>>>>> 2. We inject a small filtering driver inside the protected guest VM,
>>>>>>    which would intercept the virtualization exception in order to handle
>>>>>>    guest page-table modifications.
>>>>>> 3. We create a dedicated EPT view (altp2m) for the in-guest agent, which
>>>>>>    would isolate the agent from the rest of the operating system; the
>>>>>>    agent will switch in and out of the protected EPT view via the VMFUNC
>>>>>>    instruction placed inside a trampoline page, thus making the agent
>>>>>>    immune to malicious code inside the guest.
>>>>>>
>>>>>> This way, all the page-table accesses would generate a
>>>>>> virtualization-exception inside the guest instead of a costly EPT
>>>>>> violation; the #VE agent would emulate and analyse the modification, and
>>>>>> decide whether it is relevant for the main introspection logic; if it is
>>>>>> relevant, it would do a VMCALL and notify the introspection engine
>>>>>> about the modification; otherwise, it would resume normal instruction
>>>>>> execution, thus avoiding a very costly VM exit.
>>>>>>
>>>>>> Signed-off-by: Adrian Pop <apop@bitdefender.com>
>>>>>> ---
>>>>>> Changes in v2:
>>>>>> - remove the "__get_vcpu()" helper
>>>>>> ---
>>>>>>  tools/libxc/xc_altp2m.c |  1 -
>>>>>>  xen/arch/x86/hvm/hvm.c  | 19 ++++++++++---------
>>>>>>  2 files changed, 10 insertions(+), 10 deletions(-)
>>>>>>
>>>>>> diff --git a/tools/libxc/xc_altp2m.c b/tools/libxc/xc_altp2m.c
>>>>>> index ce4a1e4d60..528e929d7a 100644
>>>>>> --- a/tools/libxc/xc_altp2m.c
>>>>>> +++ b/tools/libxc/xc_altp2m.c
>>>>>> @@ -68,7 +68,6 @@ int xc_altp2m_set_domain_state(xc_interface *handle, uint32_t dom, bool state)
>>>>>>      return rc;
>>>>>>  }
>>>>>>
>>>>>> -/* This is a bit odd to me that it acts on current.. */
>>>>>>  int xc_altp2m_set_vcpu_enable_notify(xc_interface *handle, uint32_t domid,
>>>>>>                                       uint32_t vcpuid, xen_pfn_t gfn)
>>>>>>  {
>>>>>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
>>>>>> index 72c51faecb..49c3bbee94 100644
>>>>>> --- a/xen/arch/x86/hvm/hvm.c
>>>>>> +++ b/xen/arch/x86/hvm/hvm.c
>>>>>> @@ -4533,8 +4533,7 @@ static int do_altp2m_op(
>>>>>>          return -EOPNOTSUPP;
>>>>>>      }
>>>>>>
>>>>>> -    d = ( a.cmd != HVMOP_altp2m_vcpu_enable_notify ) ?
>>>>>> -        rcu_lock_domain_by_any_id(a.domain) : rcu_lock_current_domain();
>>>>>> +    d = rcu_lock_domain_by_any_id(a.domain);
>>>>> Does rcu_lock_domain_by_any_id work if its from the current domain? If
>>>>> not, doesn't that change this function's accessibility to be from
>>>>> exclusively usable only by the outside agent?
>>>> The code says it should be safe:
>>>>
>>>>  633 struct domain *rcu_lock_domain_by_any_id(domid_t dom)
>>>>  634 {
>>>>  635     if ( dom == DOMID_SELF )
>>>>  636         return rcu_lock_current_domain();
>>>>  637     return rcu_lock_domain_by_id(dom);
>>>>  638 }
>>>>
>>>> as long as dom == DOMID_SELF. I think the old behaviour assumed that
>>>> HVMOP_altp2m_vcpu_enable_notify alone would only ever be used from the
>>>> current domain, and this change expands its usability (Adrian should
>>>> correct me if I'm wrong here).
>>> Sounds good, thanks!
>> May we take that as an Acked-by, or are there are other things you think
>> we should address?
> A Reviewed-by would be appropriate, I don't think the files touched in
> this patch fall under our umbrella.

That doesn't prohibit you providing a Reviewed-by: tag :)

The statement itself is useful and hold weight, even if it isn't in code
you are a maintainer of.

~Andrew

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

Re: [PATCH v2] x86/altp2m: Allow setting the #VE info page for an arbitrary VCPU

[Razvan Cojocaru]

On 9/5/18 9:40 PM, Tamas K Lengyel wrote:
>>> Sounds good, thanks!
>> May we take that as an Acked-by, or are there are other things you think
>> we should address?
> A Reviewed-by would be appropriate, I don't think the files touched in
> this patch fall under our umbrella.

You're right, I just saw the Cc but didn't check.


Thanks,
Razvan

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

Re: [PATCH v2] x86/altp2m: Allow setting the #VE info page for an arbitrary VCPU

[Tamas K Lengyel]

On Wed, Sep 5, 2018 at 12:45 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>
> On 05/09/18 19:40, Tamas K Lengyel wrote:
> > On Wed, Sep 5, 2018 at 10:40 AM Razvan Cojocaru
> > <rcojocaru@bitdefender.com> wrote:
> >> On 9/5/18 7:28 PM, Tamas K Lengyel wrote:
> >>> On Tue, Sep 4, 2018 at 2:58 PM Razvan Cojocaru
> >>> <rcojocaru@bitdefender.com> wrote:
> >>>> On 9/4/18 11:40 PM, Tamas K Lengyel wrote:
> >>>>> On Mon, Sep 3, 2018 at 10:59 PM Adrian Pop <apop@bitdefender.com> wrote:
> >>>>>> In a classic HVI + Xen setup, the introspection engine would monitor
> >>>>>> legacy guest page-tables by marking them read-only inside the EPT; this
> >>>>>> way any modification explicitly made by the guest or implicitly made by
> >>>>>> the CPU page walker would trigger an EPT violation, which would be
> >>>>>> forwarded by Xen to the SVA and thus the HVI agent.  The HVI agent would
> >>>>>> analyse the modification, and act upon it - for example, a virtual page
> >>>>>> may be remapped (its guest physical address changed inside the
> >>>>>> page-table), in which case the introspection logic would update the
> >>>>>> protection accordingly (remove EPT hook on the old gpa, and place a new
> >>>>>> EPT hook on the new gpa).  In other cases, the modification may be of no
> >>>>>> interest to the introspection engine - for example, the accessed/dirty
> >>>>>> bits may be cleared by the operating system or the accessed/dirty bits
> >>>>>> may be set by the CPU page walker.
> >>>>>>
> >>>>>> In our tests we discovered that the vast majority of guest page-table
> >>>>>> modifications fall in the second category (especially on Windows 10 RS4
> >>>>>> x64 - more than 95% of ALL the page-table modifications are irrelevant to
> >>>>>> us) - they are of no interest to the introspection logic, but they
> >>>>>> trigger a very costly EPT violation nonetheless.  Therefore, we decided
> >>>>>> to make use of the new #VE & VMFUNC features in recent Intel CPUs to
> >>>>>> accelerate the guest page-tables monitoring in the following way:
> >>>>>>
> >>>>>> 1. Each monitored page-table would be flagged as being convertible
> >>>>>>    inside the EPT, thus enabling the CPU to deliver a virtualization
> >>>>>>    exception to he guest instead of generating a traditional EPT
> >>>>>>    violation.
> >>>>>> 2. We inject a small filtering driver inside the protected guest VM,
> >>>>>>    which would intercept the virtualization exception in order to handle
> >>>>>>    guest page-table modifications.
> >>>>>> 3. We create a dedicated EPT view (altp2m) for the in-guest agent, which
> >>>>>>    would isolate the agent from the rest of the operating system; the
> >>>>>>    agent will switch in and out of the protected EPT view via the VMFUNC
> >>>>>>    instruction placed inside a trampoline page, thus making the agent
> >>>>>>    immune to malicious code inside the guest.
> >>>>>>
> >>>>>> This way, all the page-table accesses would generate a
> >>>>>> virtualization-exception inside the guest instead of a costly EPT
> >>>>>> violation; the #VE agent would emulate and analyse the modification, and
> >>>>>> decide whether it is relevant for the main introspection logic; if it is
> >>>>>> relevant, it would do a VMCALL and notify the introspection engine
> >>>>>> about the modification; otherwise, it would resume normal instruction
> >>>>>> execution, thus avoiding a very costly VM exit.
> >>>>>>
> >>>>>> Signed-off-by: Adrian Pop <apop@bitdefender.com>
> >>>>>> ---
> >>>>>> Changes in v2:
> >>>>>> - remove the "__get_vcpu()" helper
> >>>>>> ---
> >>>>>>  tools/libxc/xc_altp2m.c |  1 -
> >>>>>>  xen/arch/x86/hvm/hvm.c  | 19 ++++++++++---------
> >>>>>>  2 files changed, 10 insertions(+), 10 deletions(-)
> >>>>>>
> >>>>>> diff --git a/tools/libxc/xc_altp2m.c b/tools/libxc/xc_altp2m.c
> >>>>>> index ce4a1e4d60..528e929d7a 100644
> >>>>>> --- a/tools/libxc/xc_altp2m.c
> >>>>>> +++ b/tools/libxc/xc_altp2m.c
> >>>>>> @@ -68,7 +68,6 @@ int xc_altp2m_set_domain_state(xc_interface *handle, uint32_t dom, bool state)
> >>>>>>      return rc;
> >>>>>>  }
> >>>>>>
> >>>>>> -/* This is a bit odd to me that it acts on current.. */
> >>>>>>  int xc_altp2m_set_vcpu_enable_notify(xc_interface *handle, uint32_t domid,
> >>>>>>                                       uint32_t vcpuid, xen_pfn_t gfn)
> >>>>>>  {
> >>>>>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> >>>>>> index 72c51faecb..49c3bbee94 100644
> >>>>>> --- a/xen/arch/x86/hvm/hvm.c
> >>>>>> +++ b/xen/arch/x86/hvm/hvm.c
> >>>>>> @@ -4533,8 +4533,7 @@ static int do_altp2m_op(
> >>>>>>          return -EOPNOTSUPP;
> >>>>>>      }
> >>>>>>
> >>>>>> -    d = ( a.cmd != HVMOP_altp2m_vcpu_enable_notify ) ?
> >>>>>> -        rcu_lock_domain_by_any_id(a.domain) : rcu_lock_current_domain();
> >>>>>> +    d = rcu_lock_domain_by_any_id(a.domain);
> >>>>> Does rcu_lock_domain_by_any_id work if its from the current domain? If
> >>>>> not, doesn't that change this function's accessibility to be from
> >>>>> exclusively usable only by the outside agent?
> >>>> The code says it should be safe:
> >>>>
> >>>>  633 struct domain *rcu_lock_domain_by_any_id(domid_t dom)
> >>>>  634 {
> >>>>  635     if ( dom == DOMID_SELF )
> >>>>  636         return rcu_lock_current_domain();
> >>>>  637     return rcu_lock_domain_by_id(dom);
> >>>>  638 }
> >>>>
> >>>> as long as dom == DOMID_SELF. I think the old behaviour assumed that
> >>>> HVMOP_altp2m_vcpu_enable_notify alone would only ever be used from the
> >>>> current domain, and this change expands its usability (Adrian should
> >>>> correct me if I'm wrong here).
> >>> Sounds good, thanks!
> >> May we take that as an Acked-by, or are there are other things you think
> >> we should address?
> > A Reviewed-by would be appropriate, I don't think the files touched in
> > this patch fall under our umbrella.
>
> That doesn't prohibit you providing a Reviewed-by: tag :)
>
> The statement itself is useful and hold weight, even if it isn't in code
> you are a maintainer of.

Indeed :)

Reviewed-by: Tamas K Lengyel <tamas@tklengyel.com>

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

Re: [PATCH v2] x86/altp2m: Allow setting the #VE info page for an arbitrary VCPU

[Razvan Cojocaru]

On 9/6/18 1:27 AM, Tamas K Lengyel wrote:
> On Wed, Sep 5, 2018 at 12:45 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>>
>> On 05/09/18 19:40, Tamas K Lengyel wrote:
>>> On Wed, Sep 5, 2018 at 10:40 AM Razvan Cojocaru
>>> <rcojocaru@bitdefender.com> wrote:
>>>> On 9/5/18 7:28 PM, Tamas K Lengyel wrote:
>>>>> On Tue, Sep 4, 2018 at 2:58 PM Razvan Cojocaru
>>>>> <rcojocaru@bitdefender.com> wrote:
>>>>>> On 9/4/18 11:40 PM, Tamas K Lengyel wrote:
>>>>>>> On Mon, Sep 3, 2018 at 10:59 PM Adrian Pop <apop@bitdefender.com> wrote:
>>>>>>>> In a classic HVI + Xen setup, the introspection engine would monitor
>>>>>>>> legacy guest page-tables by marking them read-only inside the EPT; this
>>>>>>>> way any modification explicitly made by the guest or implicitly made by
>>>>>>>> the CPU page walker would trigger an EPT violation, which would be
>>>>>>>> forwarded by Xen to the SVA and thus the HVI agent.  The HVI agent would
>>>>>>>> analyse the modification, and act upon it - for example, a virtual page
>>>>>>>> may be remapped (its guest physical address changed inside the
>>>>>>>> page-table), in which case the introspection logic would update the
>>>>>>>> protection accordingly (remove EPT hook on the old gpa, and place a new
>>>>>>>> EPT hook on the new gpa).  In other cases, the modification may be of no
>>>>>>>> interest to the introspection engine - for example, the accessed/dirty
>>>>>>>> bits may be cleared by the operating system or the accessed/dirty bits
>>>>>>>> may be set by the CPU page walker.
>>>>>>>>
>>>>>>>> In our tests we discovered that the vast majority of guest page-table
>>>>>>>> modifications fall in the second category (especially on Windows 10 RS4
>>>>>>>> x64 - more than 95% of ALL the page-table modifications are irrelevant to
>>>>>>>> us) - they are of no interest to the introspection logic, but they
>>>>>>>> trigger a very costly EPT violation nonetheless.  Therefore, we decided
>>>>>>>> to make use of the new #VE & VMFUNC features in recent Intel CPUs to
>>>>>>>> accelerate the guest page-tables monitoring in the following way:
>>>>>>>>
>>>>>>>> 1. Each monitored page-table would be flagged as being convertible
>>>>>>>>    inside the EPT, thus enabling the CPU to deliver a virtualization
>>>>>>>>    exception to he guest instead of generating a traditional EPT
>>>>>>>>    violation.
>>>>>>>> 2. We inject a small filtering driver inside the protected guest VM,
>>>>>>>>    which would intercept the virtualization exception in order to handle
>>>>>>>>    guest page-table modifications.
>>>>>>>> 3. We create a dedicated EPT view (altp2m) for the in-guest agent, which
>>>>>>>>    would isolate the agent from the rest of the operating system; the
>>>>>>>>    agent will switch in and out of the protected EPT view via the VMFUNC
>>>>>>>>    instruction placed inside a trampoline page, thus making the agent
>>>>>>>>    immune to malicious code inside the guest.
>>>>>>>>
>>>>>>>> This way, all the page-table accesses would generate a
>>>>>>>> virtualization-exception inside the guest instead of a costly EPT
>>>>>>>> violation; the #VE agent would emulate and analyse the modification, and
>>>>>>>> decide whether it is relevant for the main introspection logic; if it is
>>>>>>>> relevant, it would do a VMCALL and notify the introspection engine
>>>>>>>> about the modification; otherwise, it would resume normal instruction
>>>>>>>> execution, thus avoiding a very costly VM exit.
>>>>>>>>
>>>>>>>> Signed-off-by: Adrian Pop <apop@bitdefender.com>
>>>>>>>> ---
>>>>>>>> Changes in v2:
>>>>>>>> - remove the "__get_vcpu()" helper
>>>>>>>> ---
>>>>>>>>  tools/libxc/xc_altp2m.c |  1 -
>>>>>>>>  xen/arch/x86/hvm/hvm.c  | 19 ++++++++++---------
>>>>>>>>  2 files changed, 10 insertions(+), 10 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/tools/libxc/xc_altp2m.c b/tools/libxc/xc_altp2m.c
>>>>>>>> index ce4a1e4d60..528e929d7a 100644
>>>>>>>> --- a/tools/libxc/xc_altp2m.c
>>>>>>>> +++ b/tools/libxc/xc_altp2m.c
>>>>>>>> @@ -68,7 +68,6 @@ int xc_altp2m_set_domain_state(xc_interface *handle, uint32_t dom, bool state)
>>>>>>>>      return rc;
>>>>>>>>  }
>>>>>>>>
>>>>>>>> -/* This is a bit odd to me that it acts on current.. */
>>>>>>>>  int xc_altp2m_set_vcpu_enable_notify(xc_interface *handle, uint32_t domid,
>>>>>>>>                                       uint32_t vcpuid, xen_pfn_t gfn)
>>>>>>>>  {
>>>>>>>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
>>>>>>>> index 72c51faecb..49c3bbee94 100644
>>>>>>>> --- a/xen/arch/x86/hvm/hvm.c
>>>>>>>> +++ b/xen/arch/x86/hvm/hvm.c
>>>>>>>> @@ -4533,8 +4533,7 @@ static int do_altp2m_op(
>>>>>>>>          return -EOPNOTSUPP;
>>>>>>>>      }
>>>>>>>>
>>>>>>>> -    d = ( a.cmd != HVMOP_altp2m_vcpu_enable_notify ) ?
>>>>>>>> -        rcu_lock_domain_by_any_id(a.domain) : rcu_lock_current_domain();
>>>>>>>> +    d = rcu_lock_domain_by_any_id(a.domain);
>>>>>>> Does rcu_lock_domain_by_any_id work if its from the current domain? If
>>>>>>> not, doesn't that change this function's accessibility to be from
>>>>>>> exclusively usable only by the outside agent?
>>>>>> The code says it should be safe:
>>>>>>
>>>>>>  633 struct domain *rcu_lock_domain_by_any_id(domid_t dom)
>>>>>>  634 {
>>>>>>  635     if ( dom == DOMID_SELF )
>>>>>>  636         return rcu_lock_current_domain();
>>>>>>  637     return rcu_lock_domain_by_id(dom);
>>>>>>  638 }
>>>>>>
>>>>>> as long as dom == DOMID_SELF. I think the old behaviour assumed that
>>>>>> HVMOP_altp2m_vcpu_enable_notify alone would only ever be used from the
>>>>>> current domain, and this change expands its usability (Adrian should
>>>>>> correct me if I'm wrong here).
>>>>> Sounds good, thanks!
>>>> May we take that as an Acked-by, or are there are other things you think
>>>> we should address?
>>> A Reviewed-by would be appropriate, I don't think the files touched in
>>> this patch fall under our umbrella.
>>
>> That doesn't prohibit you providing a Reviewed-by: tag :)
>>
>> The statement itself is useful and hold weight, even if it isn't in code
>> you are a maintainer of.
> 
> Indeed :)
> 
> Reviewed-by: Tamas K Lengyel <tamas@tklengyel.com>

Are there any issues preventing this patch to go in? Possibly missing acks?


Thanks,
Razvan

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

Re: [PATCH v2] x86/altp2m: Allow setting the #VE info page for an arbitrary VCPU

[Wei Liu]

On Thu, Sep 20, 2018 at 11:11:37AM +0300, Razvan Cojocaru wrote:
> On 9/6/18 1:27 AM, Tamas K Lengyel wrote:
> > On Wed, Sep 5, 2018 at 12:45 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> >>
> >> On 05/09/18 19:40, Tamas K Lengyel wrote:
> >>> On Wed, Sep 5, 2018 at 10:40 AM Razvan Cojocaru
> >>> <rcojocaru@bitdefender.com> wrote:
> >>>> On 9/5/18 7:28 PM, Tamas K Lengyel wrote:
> >>>>> On Tue, Sep 4, 2018 at 2:58 PM Razvan Cojocaru
> >>>>> <rcojocaru@bitdefender.com> wrote:
> >>>>>> On 9/4/18 11:40 PM, Tamas K Lengyel wrote:
> >>>>>>> On Mon, Sep 3, 2018 at 10:59 PM Adrian Pop <apop@bitdefender.com> wrote:
> >>>>>>>> In a classic HVI + Xen setup, the introspection engine would monitor
> >>>>>>>> legacy guest page-tables by marking them read-only inside the EPT; this
> >>>>>>>> way any modification explicitly made by the guest or implicitly made by
> >>>>>>>> the CPU page walker would trigger an EPT violation, which would be
> >>>>>>>> forwarded by Xen to the SVA and thus the HVI agent.  The HVI agent would
> >>>>>>>> analyse the modification, and act upon it - for example, a virtual page
> >>>>>>>> may be remapped (its guest physical address changed inside the
> >>>>>>>> page-table), in which case the introspection logic would update the
> >>>>>>>> protection accordingly (remove EPT hook on the old gpa, and place a new
> >>>>>>>> EPT hook on the new gpa).  In other cases, the modification may be of no
> >>>>>>>> interest to the introspection engine - for example, the accessed/dirty
> >>>>>>>> bits may be cleared by the operating system or the accessed/dirty bits
> >>>>>>>> may be set by the CPU page walker.
> >>>>>>>>
> >>>>>>>> In our tests we discovered that the vast majority of guest page-table
> >>>>>>>> modifications fall in the second category (especially on Windows 10 RS4
> >>>>>>>> x64 - more than 95% of ALL the page-table modifications are irrelevant to
> >>>>>>>> us) - they are of no interest to the introspection logic, but they
> >>>>>>>> trigger a very costly EPT violation nonetheless.  Therefore, we decided
> >>>>>>>> to make use of the new #VE & VMFUNC features in recent Intel CPUs to
> >>>>>>>> accelerate the guest page-tables monitoring in the following way:
> >>>>>>>>
> >>>>>>>> 1. Each monitored page-table would be flagged as being convertible
> >>>>>>>>    inside the EPT, thus enabling the CPU to deliver a virtualization
> >>>>>>>>    exception to he guest instead of generating a traditional EPT
> >>>>>>>>    violation.
> >>>>>>>> 2. We inject a small filtering driver inside the protected guest VM,
> >>>>>>>>    which would intercept the virtualization exception in order to handle
> >>>>>>>>    guest page-table modifications.
> >>>>>>>> 3. We create a dedicated EPT view (altp2m) for the in-guest agent, which
> >>>>>>>>    would isolate the agent from the rest of the operating system; the
> >>>>>>>>    agent will switch in and out of the protected EPT view via the VMFUNC
> >>>>>>>>    instruction placed inside a trampoline page, thus making the agent
> >>>>>>>>    immune to malicious code inside the guest.
> >>>>>>>>
> >>>>>>>> This way, all the page-table accesses would generate a
> >>>>>>>> virtualization-exception inside the guest instead of a costly EPT
> >>>>>>>> violation; the #VE agent would emulate and analyse the modification, and
> >>>>>>>> decide whether it is relevant for the main introspection logic; if it is
> >>>>>>>> relevant, it would do a VMCALL and notify the introspection engine
> >>>>>>>> about the modification; otherwise, it would resume normal instruction
> >>>>>>>> execution, thus avoiding a very costly VM exit.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Adrian Pop <apop@bitdefender.com>
> >>>>>>>> ---
> >>>>>>>> Changes in v2:
> >>>>>>>> - remove the "__get_vcpu()" helper
> >>>>>>>> ---
> >>>>>>>>  tools/libxc/xc_altp2m.c |  1 -
> >>>>>>>>  xen/arch/x86/hvm/hvm.c  | 19 ++++++++++---------
> >>>>>>>>  2 files changed, 10 insertions(+), 10 deletions(-)
> >>>>>>>>
> >>>>>>>> diff --git a/tools/libxc/xc_altp2m.c b/tools/libxc/xc_altp2m.c
> >>>>>>>> index ce4a1e4d60..528e929d7a 100644
> >>>>>>>> --- a/tools/libxc/xc_altp2m.c
> >>>>>>>> +++ b/tools/libxc/xc_altp2m.c
> >>>>>>>> @@ -68,7 +68,6 @@ int xc_altp2m_set_domain_state(xc_interface *handle, uint32_t dom, bool state)
> >>>>>>>>      return rc;
> >>>>>>>>  }
> >>>>>>>>
> >>>>>>>> -/* This is a bit odd to me that it acts on current.. */
> >>>>>>>>  int xc_altp2m_set_vcpu_enable_notify(xc_interface *handle, uint32_t domid,
> >>>>>>>>                                       uint32_t vcpuid, xen_pfn_t gfn)
> >>>>>>>>  {
> >>>>>>>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> >>>>>>>> index 72c51faecb..49c3bbee94 100644
> >>>>>>>> --- a/xen/arch/x86/hvm/hvm.c
> >>>>>>>> +++ b/xen/arch/x86/hvm/hvm.c
> >>>>>>>> @@ -4533,8 +4533,7 @@ static int do_altp2m_op(
> >>>>>>>>          return -EOPNOTSUPP;
> >>>>>>>>      }
> >>>>>>>>
> >>>>>>>> -    d = ( a.cmd != HVMOP_altp2m_vcpu_enable_notify ) ?
> >>>>>>>> -        rcu_lock_domain_by_any_id(a.domain) : rcu_lock_current_domain();
> >>>>>>>> +    d = rcu_lock_domain_by_any_id(a.domain);
> >>>>>>> Does rcu_lock_domain_by_any_id work if its from the current domain? If
> >>>>>>> not, doesn't that change this function's accessibility to be from
> >>>>>>> exclusively usable only by the outside agent?
> >>>>>> The code says it should be safe:
> >>>>>>
> >>>>>>  633 struct domain *rcu_lock_domain_by_any_id(domid_t dom)
> >>>>>>  634 {
> >>>>>>  635     if ( dom == DOMID_SELF )
> >>>>>>  636         return rcu_lock_current_domain();
> >>>>>>  637     return rcu_lock_domain_by_id(dom);
> >>>>>>  638 }
> >>>>>>
> >>>>>> as long as dom == DOMID_SELF. I think the old behaviour assumed that
> >>>>>> HVMOP_altp2m_vcpu_enable_notify alone would only ever be used from the
> >>>>>> current domain, and this change expands its usability (Adrian should
> >>>>>> correct me if I'm wrong here).
> >>>>> Sounds good, thanks!
> >>>> May we take that as an Acked-by, or are there are other things you think
> >>>> we should address?
> >>> A Reviewed-by would be appropriate, I don't think the files touched in
> >>> this patch fall under our umbrella.
> >>
> >> That doesn't prohibit you providing a Reviewed-by: tag :)
> >>
> >> The statement itself is useful and hold weight, even if it isn't in code
> >> you are a maintainer of.
> > 
> > Indeed :)
> > 
> > Reviewed-by: Tamas K Lengyel <tamas@tklengyel.com>
> 
> Are there any issues preventing this patch to go in? Possibly missing acks?

I don't think so, unless an explicit ack for the deletion of that
comment is required. In any case:

Acked-by: Wei Liu <wei.liu2@citrix.com>

I will commit this patch shortly.

Wei.

> 
> 
> Thanks,
> Razvan

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

Re: [PATCH v2] x86/altp2m: Allow setting the #VE info page for an arbitrary VCPU

[Razvan Cojocaru]

On 9/20/18 11:14 AM, Wei Liu wrote:
>>> Reviewed-by: Tamas K Lengyel <tamas@tklengyel.com>
>> Are there any issues preventing this patch to go in? Possibly missing acks?
> I don't think so, unless an explicit ack for the deletion of that
> comment is required. In any case:
> 
> Acked-by: Wei Liu <wei.liu2@citrix.com>
> 
> I will commit this patch shortly.

Thanks!

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

Re: [PATCH v2] x86/altp2m: Allow setting the #VE info page for an arbitrary VCPU

[Jan Beulich]

>>> On 20.09.18 at 10:11, <rcojocaru@bitdefender.com> wrote:
> On 9/6/18 1:27 AM, Tamas K Lengyel wrote:
>> On Wed, Sep 5, 2018 at 12:45 PM Andrew Cooper <andrew.cooper3@citrix.com> 
> wrote:
>>>
>>> On 05/09/18 19:40, Tamas K Lengyel wrote:
>>>> On Wed, Sep 5, 2018 at 10:40 AM Razvan Cojocaru
>>>> <rcojocaru@bitdefender.com> wrote:
>>>>> On 9/5/18 7:28 PM, Tamas K Lengyel wrote:
>>>>>> On Tue, Sep 4, 2018 at 2:58 PM Razvan Cojocaru
>>>>>> <rcojocaru@bitdefender.com> wrote:
>>>>>>> On 9/4/18 11:40 PM, Tamas K Lengyel wrote:
>>>>>>>> On Mon, Sep 3, 2018 at 10:59 PM Adrian Pop <apop@bitdefender.com> wrote:
>>>>>>>>> In a classic HVI + Xen setup, the introspection engine would monitor
>>>>>>>>> legacy guest page-tables by marking them read-only inside the EPT; this
>>>>>>>>> way any modification explicitly made by the guest or implicitly made by
>>>>>>>>> the CPU page walker would trigger an EPT violation, which would be
>>>>>>>>> forwarded by Xen to the SVA and thus the HVI agent.  The HVI agent would
>>>>>>>>> analyse the modification, and act upon it - for example, a virtual page
>>>>>>>>> may be remapped (its guest physical address changed inside the
>>>>>>>>> page-table), in which case the introspection logic would update the
>>>>>>>>> protection accordingly (remove EPT hook on the old gpa, and place a new
>>>>>>>>> EPT hook on the new gpa).  In other cases, the modification may be of no
>>>>>>>>> interest to the introspection engine - for example, the accessed/dirty
>>>>>>>>> bits may be cleared by the operating system or the accessed/dirty bits
>>>>>>>>> may be set by the CPU page walker.
>>>>>>>>>
>>>>>>>>> In our tests we discovered that the vast majority of guest page-table
>>>>>>>>> modifications fall in the second category (especially on Windows 10 RS4
>>>>>>>>> x64 - more than 95% of ALL the page-table modifications are irrelevant to
>>>>>>>>> us) - they are of no interest to the introspection logic, but they
>>>>>>>>> trigger a very costly EPT violation nonetheless.  Therefore, we decided
>>>>>>>>> to make use of the new #VE & VMFUNC features in recent Intel CPUs to
>>>>>>>>> accelerate the guest page-tables monitoring in the following way:
>>>>>>>>>
>>>>>>>>> 1. Each monitored page-table would be flagged as being convertible
>>>>>>>>>    inside the EPT, thus enabling the CPU to deliver a virtualization
>>>>>>>>>    exception to he guest instead of generating a traditional EPT
>>>>>>>>>    violation.
>>>>>>>>> 2. We inject a small filtering driver inside the protected guest VM,
>>>>>>>>>    which would intercept the virtualization exception in order to handle
>>>>>>>>>    guest page-table modifications.
>>>>>>>>> 3. We create a dedicated EPT view (altp2m) for the in-guest agent, which
>>>>>>>>>    would isolate the agent from the rest of the operating system; the
>>>>>>>>>    agent will switch in and out of the protected EPT view via the VMFUNC
>>>>>>>>>    instruction placed inside a trampoline page, thus making the agent
>>>>>>>>>    immune to malicious code inside the guest.
>>>>>>>>>
>>>>>>>>> This way, all the page-table accesses would generate a
>>>>>>>>> virtualization-exception inside the guest instead of a costly EPT
>>>>>>>>> violation; the #VE agent would emulate and analyse the modification, and
>>>>>>>>> decide whether it is relevant for the main introspection logic; if it is
>>>>>>>>> relevant, it would do a VMCALL and notify the introspection engine
>>>>>>>>> about the modification; otherwise, it would resume normal instruction
>>>>>>>>> execution, thus avoiding a very costly VM exit.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Adrian Pop <apop@bitdefender.com>
>>>>>>>>> ---
>>>>>>>>> Changes in v2:
>>>>>>>>> - remove the "__get_vcpu()" helper
>>>>>>>>> ---
>>>>>>>>>  tools/libxc/xc_altp2m.c |  1 -
>>>>>>>>>  xen/arch/x86/hvm/hvm.c  | 19 ++++++++++---------
>>>>>>>>>  2 files changed, 10 insertions(+), 10 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/tools/libxc/xc_altp2m.c b/tools/libxc/xc_altp2m.c
>>>>>>>>> index ce4a1e4d60..528e929d7a 100644
>>>>>>>>> --- a/tools/libxc/xc_altp2m.c
>>>>>>>>> +++ b/tools/libxc/xc_altp2m.c
>>>>>>>>> @@ -68,7 +68,6 @@ int xc_altp2m_set_domain_state(xc_interface *handle, 
> uint32_t dom, bool state)
>>>>>>>>>      return rc;
>>>>>>>>>  }
>>>>>>>>>
>>>>>>>>> -/* This is a bit odd to me that it acts on current.. */
>>>>>>>>>  int xc_altp2m_set_vcpu_enable_notify(xc_interface *handle, uint32_t domid,
>>>>>>>>>                                       uint32_t vcpuid, xen_pfn_t gfn)
>>>>>>>>>  {
>>>>>>>>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
>>>>>>>>> index 72c51faecb..49c3bbee94 100644
>>>>>>>>> --- a/xen/arch/x86/hvm/hvm.c
>>>>>>>>> +++ b/xen/arch/x86/hvm/hvm.c
>>>>>>>>> @@ -4533,8 +4533,7 @@ static int do_altp2m_op(
>>>>>>>>>          return -EOPNOTSUPP;
>>>>>>>>>      }
>>>>>>>>>
>>>>>>>>> -    d = ( a.cmd != HVMOP_altp2m_vcpu_enable_notify ) ?
>>>>>>>>> -        rcu_lock_domain_by_any_id(a.domain) : rcu_lock_current_domain();
>>>>>>>>> +    d = rcu_lock_domain_by_any_id(a.domain);
>>>>>>>> Does rcu_lock_domain_by_any_id work if its from the current domain? If
>>>>>>>> not, doesn't that change this function's accessibility to be from
>>>>>>>> exclusively usable only by the outside agent?
>>>>>>> The code says it should be safe:
>>>>>>>
>>>>>>>  633 struct domain *rcu_lock_domain_by_any_id(domid_t dom)
>>>>>>>  634 {
>>>>>>>  635     if ( dom == DOMID_SELF )
>>>>>>>  636         return rcu_lock_current_domain();
>>>>>>>  637     return rcu_lock_domain_by_id(dom);
>>>>>>>  638 }
>>>>>>>
>>>>>>> as long as dom == DOMID_SELF. I think the old behaviour assumed that
>>>>>>> HVMOP_altp2m_vcpu_enable_notify alone would only ever be used from the
>>>>>>> current domain, and this change expands its usability (Adrian should
>>>>>>> correct me if I'm wrong here).
>>>>>> Sounds good, thanks!
>>>>> May we take that as an Acked-by, or are there are other things you think
>>>>> we should address?
>>>> A Reviewed-by would be appropriate, I don't think the files touched in
>>>> this patch fall under our umbrella.
>>>
>>> That doesn't prohibit you providing a Reviewed-by: tag :)
>>>
>>> The statement itself is useful and hold weight, even if it isn't in code
>>> you are a maintainer of.
>> 
>> Indeed :)
>> 
>> Reviewed-by: Tamas K Lengyel <tamas@tklengyel.com>
> 
> Are there any issues preventing this patch to go in? Possibly missing acks?

Well, afaics the patch has no x86 maintainer ack, nor - considering it's
an mm function sitting in the "wrong" file, at least one from the mm
maintainer. As mentioned a number of times before, it is the submitter's
responsibility to chase acks, not the committers' or maintainers'.

Jan


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

Re: [PATCH v2] x86/altp2m: Allow setting the #VE info page for an arbitrary VCPU

[Wei Liu]

On Thu, Sep 20, 2018 at 02:22:29AM -0600, Jan Beulich wrote:
> >>> On 20.09.18 at 10:11, <rcojocaru@bitdefender.com> wrote:
> > On 9/6/18 1:27 AM, Tamas K Lengyel wrote:
> >> On Wed, Sep 5, 2018 at 12:45 PM Andrew Cooper <andrew.cooper3@citrix.com> 
> > wrote:
> >>>
> >>> On 05/09/18 19:40, Tamas K Lengyel wrote:
> >>>> On Wed, Sep 5, 2018 at 10:40 AM Razvan Cojocaru
> >>>> <rcojocaru@bitdefender.com> wrote:
> >>>>> On 9/5/18 7:28 PM, Tamas K Lengyel wrote:
> >>>>>> On Tue, Sep 4, 2018 at 2:58 PM Razvan Cojocaru
> >>>>>> <rcojocaru@bitdefender.com> wrote:
> >>>>>>> On 9/4/18 11:40 PM, Tamas K Lengyel wrote:
> >>>>>>>> On Mon, Sep 3, 2018 at 10:59 PM Adrian Pop <apop@bitdefender.com> wrote:
> >>>>>>>>> In a classic HVI + Xen setup, the introspection engine would monitor
> >>>>>>>>> legacy guest page-tables by marking them read-only inside the EPT; this
> >>>>>>>>> way any modification explicitly made by the guest or implicitly made by
> >>>>>>>>> the CPU page walker would trigger an EPT violation, which would be
> >>>>>>>>> forwarded by Xen to the SVA and thus the HVI agent.  The HVI agent would
> >>>>>>>>> analyse the modification, and act upon it - for example, a virtual page
> >>>>>>>>> may be remapped (its guest physical address changed inside the
> >>>>>>>>> page-table), in which case the introspection logic would update the
> >>>>>>>>> protection accordingly (remove EPT hook on the old gpa, and place a new
> >>>>>>>>> EPT hook on the new gpa).  In other cases, the modification may be of no
> >>>>>>>>> interest to the introspection engine - for example, the accessed/dirty
> >>>>>>>>> bits may be cleared by the operating system or the accessed/dirty bits
> >>>>>>>>> may be set by the CPU page walker.
> >>>>>>>>>
> >>>>>>>>> In our tests we discovered that the vast majority of guest page-table
> >>>>>>>>> modifications fall in the second category (especially on Windows 10 RS4
> >>>>>>>>> x64 - more than 95% of ALL the page-table modifications are irrelevant to
> >>>>>>>>> us) - they are of no interest to the introspection logic, but they
> >>>>>>>>> trigger a very costly EPT violation nonetheless.  Therefore, we decided
> >>>>>>>>> to make use of the new #VE & VMFUNC features in recent Intel CPUs to
> >>>>>>>>> accelerate the guest page-tables monitoring in the following way:
> >>>>>>>>>
> >>>>>>>>> 1. Each monitored page-table would be flagged as being convertible
> >>>>>>>>>    inside the EPT, thus enabling the CPU to deliver a virtualization
> >>>>>>>>>    exception to he guest instead of generating a traditional EPT
> >>>>>>>>>    violation.
> >>>>>>>>> 2. We inject a small filtering driver inside the protected guest VM,
> >>>>>>>>>    which would intercept the virtualization exception in order to handle
> >>>>>>>>>    guest page-table modifications.
> >>>>>>>>> 3. We create a dedicated EPT view (altp2m) for the in-guest agent, which
> >>>>>>>>>    would isolate the agent from the rest of the operating system; the
> >>>>>>>>>    agent will switch in and out of the protected EPT view via the VMFUNC
> >>>>>>>>>    instruction placed inside a trampoline page, thus making the agent
> >>>>>>>>>    immune to malicious code inside the guest.
> >>>>>>>>>
> >>>>>>>>> This way, all the page-table accesses would generate a
> >>>>>>>>> virtualization-exception inside the guest instead of a costly EPT
> >>>>>>>>> violation; the #VE agent would emulate and analyse the modification, and
> >>>>>>>>> decide whether it is relevant for the main introspection logic; if it is
> >>>>>>>>> relevant, it would do a VMCALL and notify the introspection engine
> >>>>>>>>> about the modification; otherwise, it would resume normal instruction
> >>>>>>>>> execution, thus avoiding a very costly VM exit.
> >>>>>>>>>
> >>>>>>>>> Signed-off-by: Adrian Pop <apop@bitdefender.com>
> >>>>>>>>> ---
> >>>>>>>>> Changes in v2:
> >>>>>>>>> - remove the "__get_vcpu()" helper
> >>>>>>>>> ---
> >>>>>>>>>  tools/libxc/xc_altp2m.c |  1 -
> >>>>>>>>>  xen/arch/x86/hvm/hvm.c  | 19 ++++++++++---------
> >>>>>>>>>  2 files changed, 10 insertions(+), 10 deletions(-)
> >>>>>>>>>
> >>>>>>>>> diff --git a/tools/libxc/xc_altp2m.c b/tools/libxc/xc_altp2m.c
> >>>>>>>>> index ce4a1e4d60..528e929d7a 100644
> >>>>>>>>> --- a/tools/libxc/xc_altp2m.c
> >>>>>>>>> +++ b/tools/libxc/xc_altp2m.c
> >>>>>>>>> @@ -68,7 +68,6 @@ int xc_altp2m_set_domain_state(xc_interface *handle, 
> > uint32_t dom, bool state)
> >>>>>>>>>      return rc;
> >>>>>>>>>  }
> >>>>>>>>>
> >>>>>>>>> -/* This is a bit odd to me that it acts on current.. */
> >>>>>>>>>  int xc_altp2m_set_vcpu_enable_notify(xc_interface *handle, uint32_t domid,
> >>>>>>>>>                                       uint32_t vcpuid, xen_pfn_t gfn)
> >>>>>>>>>  {
> >>>>>>>>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> >>>>>>>>> index 72c51faecb..49c3bbee94 100644
> >>>>>>>>> --- a/xen/arch/x86/hvm/hvm.c
> >>>>>>>>> +++ b/xen/arch/x86/hvm/hvm.c
> >>>>>>>>> @@ -4533,8 +4533,7 @@ static int do_altp2m_op(
> >>>>>>>>>          return -EOPNOTSUPP;
> >>>>>>>>>      }
> >>>>>>>>>
> >>>>>>>>> -    d = ( a.cmd != HVMOP_altp2m_vcpu_enable_notify ) ?
> >>>>>>>>> -        rcu_lock_domain_by_any_id(a.domain) : rcu_lock_current_domain();
> >>>>>>>>> +    d = rcu_lock_domain_by_any_id(a.domain);
> >>>>>>>> Does rcu_lock_domain_by_any_id work if its from the current domain? If
> >>>>>>>> not, doesn't that change this function's accessibility to be from
> >>>>>>>> exclusively usable only by the outside agent?
> >>>>>>> The code says it should be safe:
> >>>>>>>
> >>>>>>>  633 struct domain *rcu_lock_domain_by_any_id(domid_t dom)
> >>>>>>>  634 {
> >>>>>>>  635     if ( dom == DOMID_SELF )
> >>>>>>>  636         return rcu_lock_current_domain();
> >>>>>>>  637     return rcu_lock_domain_by_id(dom);
> >>>>>>>  638 }
> >>>>>>>
> >>>>>>> as long as dom == DOMID_SELF. I think the old behaviour assumed that
> >>>>>>> HVMOP_altp2m_vcpu_enable_notify alone would only ever be used from the
> >>>>>>> current domain, and this change expands its usability (Adrian should
> >>>>>>> correct me if I'm wrong here).
> >>>>>> Sounds good, thanks!
> >>>>> May we take that as an Acked-by, or are there are other things you think
> >>>>> we should address?
> >>>> A Reviewed-by would be appropriate, I don't think the files touched in
> >>>> this patch fall under our umbrella.
> >>>
> >>> That doesn't prohibit you providing a Reviewed-by: tag :)
> >>>
> >>> The statement itself is useful and hold weight, even if it isn't in code
> >>> you are a maintainer of.
> >> 
> >> Indeed :)
> >> 
> >> Reviewed-by: Tamas K Lengyel <tamas@tklengyel.com>
> > 
> > Are there any issues preventing this patch to go in? Possibly missing acks?
> 
> Well, afaics the patch has no x86 maintainer ack, nor - considering it's
> an mm function sitting in the "wrong" file, at least one from the mm
> maintainer. As mentioned a number of times before, it is the submitter's
> responsibility to chase acks, not the committers' or maintainers'.


Oh, sorry. I have missed that this at least needs an ack from George.
I will wait until EOD for George to give an ack. If there isn't one by
then I will revert the patch in staging.

Wei.

> 
> Jan
> 

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

Re: [PATCH v2] x86/altp2m: Allow setting the #VE info page for an arbitrary VCPU

[Jan Beulich]

>>> On 20.09.18 at 10:11, <rcojocaru@bitdefender.com> wrote:
> Are there any issues preventing this patch to go in? Possibly missing acks?

Oh, and btw - irrespective what MAINTAINERS says - patches to this
function would better also be Cc-ed to George as the mm maintainer.
Even more so that I think it has become clear from past discussions
that I'm not going to ack any patches to this function, the existence /
placement of which I question in the first place. It's just that, with
recent clarifications, I won't object to any such additions anymore.

Jan



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

Re: [PATCH v2] x86/altp2m: Allow setting the #VE info page for an arbitrary VCPU

[Razvan Cojocaru]

On 9/20/18 11:31 AM, Wei Liu wrote:
>>>> Reviewed-by: Tamas K Lengyel <tamas@tklengyel.com>
>>> Are there any issues preventing this patch to go in? Possibly missing acks?
>> Well, afaics the patch has no x86 maintainer ack, nor - considering it's
>> an mm function sitting in the "wrong" file, at least one from the mm
>> maintainer. As mentioned a number of times before, it is the submitter's
>> responsibility to chase acks, not the committers' or maintainers'.
> 
> Oh, sorry. I have missed that this at least needs an ack from George.
> I will wait until EOD for George to give an ack. If there isn't one by
> then I will revert the patch in staging.

Added George to the email recipients.


Thanks,
Razvan

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

Re: [PATCH v2] x86/altp2m: Allow setting the #VE info page for an arbitrary VCPU

[George Dunlap]

On Tue, Sep 4, 2018 at 6:00 AM Adrian Pop <apop@bitdefender.com> wrote:
>
> In a classic HVI + Xen setup, the introspection engine would monitor
> legacy guest page-tables by marking them read-only inside the EPT; this
> way any modification explicitly made by the guest or implicitly made by
> the CPU page walker would trigger an EPT violation, which would be
> forwarded by Xen to the SVA and thus the HVI agent.  The HVI agent would
> analyse the modification, and act upon it - for example, a virtual page
> may be remapped (its guest physical address changed inside the
> page-table), in which case the introspection logic would update the
> protection accordingly (remove EPT hook on the old gpa, and place a new
> EPT hook on the new gpa).  In other cases, the modification may be of no
> interest to the introspection engine - for example, the accessed/dirty
> bits may be cleared by the operating system or the accessed/dirty bits
> may be set by the CPU page walker.
>
> In our tests we discovered that the vast majority of guest page-table
> modifications fall in the second category (especially on Windows 10 RS4
> x64 - more than 95% of ALL the page-table modifications are irrelevant to
> us) - they are of no interest to the introspection logic, but they
> trigger a very costly EPT violation nonetheless.  Therefore, we decided
> to make use of the new #VE & VMFUNC features in recent Intel CPUs to
> accelerate the guest page-tables monitoring in the following way:
>
> 1. Each monitored page-table would be flagged as being convertible
>    inside the EPT, thus enabling the CPU to deliver a virtualization
>    exception to he guest instead of generating a traditional EPT
>    violation.
> 2. We inject a small filtering driver inside the protected guest VM,
>    which would intercept the virtualization exception in order to handle
>    guest page-table modifications.
> 3. We create a dedicated EPT view (altp2m) for the in-guest agent, which
>    would isolate the agent from the rest of the operating system; the
>    agent will switch in and out of the protected EPT view via the VMFUNC
>    instruction placed inside a trampoline page, thus making the agent
>    immune to malicious code inside the guest.
>
> This way, all the page-table accesses would generate a
> virtualization-exception inside the guest instead of a costly EPT
> violation; the #VE agent would emulate and analyse the modification, and
> decide whether it is relevant for the main introspection logic; if it is
> relevant, it would do a VMCALL and notify the introspection engine
> about the modification; otherwise, it would resume normal instruction
> execution, thus avoiding a very costly VM exit.
>
> Signed-off-by: Adrian Pop <apop@bitdefender.com>

I don't have any objections to the code; I think it can stay in the
tree as far as I'm concerned.

I do have a comment on the commit message, just for future reference
at this point.  It goes into a lot of detail about the architecture of
what you're trying to accomplish, but not what this patch actually
does.  I think something like "Allow a dom0 agent to enable
vcpu_notify on guest vcpus, to enable an out-of-guest introspection
agent to insert an in-guest agent into a guest" would have been
enough.

I do have a question about your proposed use case.  You're running
this in 'mixed' mode, right, and using the altp2m to hide a secure bit
of code from the operating system?  What's to stop a rogue operating
system that doesn't want to be introspected from calling
HVMOP_altp2m_vcpu_enable_notify with INVALID_GFN to disable this?

 -George

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

Re: [PATCH v2] x86/altp2m: Allow setting the #VE info page for an arbitrary VCPU

[Razvan Cojocaru]

On 9/20/18 5:42 PM, George Dunlap wrote:
> I do have a question about your proposed use case.  You're running
> this in 'mixed' mode, right, and using the altp2m to hide a secure bit
> of code from the operating system?  What's to stop a rogue operating
> system that doesn't want to be introspected from calling
> HVMOP_altp2m_vcpu_enable_notify with INVALID_GFN to disable this?

Nothing, but we're not running this in mixed mode. :)
We're after 'external', for the very same reasons you've mentioned.

Everything important is done in dom0-only. If there's something to be
done that the in-guest agent would like, it has to ask the introspection
agent in dom0 via VMCALL events.


Thanks,
Razvan

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

Re: [PATCH v2] x86/altp2m: Allow setting the #VE info page for an arbitrary VCPU

[George Dunlap]

On Thu, Sep 20, 2018 at 3:55 PM Razvan Cojocaru
<rcojocaru@bitdefender.com> wrote:
>
> On 9/20/18 5:42 PM, George Dunlap wrote:
> > I do have a question about your proposed use case.  You're running
> > this in 'mixed' mode, right, and using the altp2m to hide a secure bit
> > of code from the operating system?  What's to stop a rogue operating
> > system that doesn't want to be introspected from calling
> > HVMOP_altp2m_vcpu_enable_notify with INVALID_GFN to disable this?
>
> Nothing, but we're not running this in mixed mode. :)
> We're after 'external', for the very same reasons you've mentioned.
>
> Everything important is done in dom0-only. If there's something to be
> done that the in-guest agent would like, it has to ask the introspection
> agent in dom0 via VMCALL events.

OK, got it, thanks.

 -George

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