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

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


  • To: Tamas K Lengyel <tamas@xxxxxxxxxxxxx>, Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>
  • From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • Date: Wed, 5 Sep 2018 19:45:29 +0100
  • Autocrypt: addr=andrew.cooper3@xxxxxxxxxx; prefer-encrypt=mutual; keydata= xsFNBFLhNn8BEADVhE+Hb8i0GV6mihnnr/uiQQdPF8kUoFzCOPXkf7jQ5sLYeJa0cQi6Penp VtiFYznTairnVsN5J+ujSTIb+OlMSJUWV4opS7WVNnxHbFTPYZVQ3erv7NKc2iVizCRZ2Kxn srM1oPXWRic8BIAdYOKOloF2300SL/bIpeD+x7h3w9B/qez7nOin5NzkxgFoaUeIal12pXSR Q354FKFoy6Vh96gc4VRqte3jw8mPuJQpfws+Pb+swvSf/i1q1+1I4jsRQQh2m6OTADHIqg2E ofTYAEh7R5HfPx0EXoEDMdRjOeKn8+vvkAwhviWXTHlG3R1QkbE5M/oywnZ83udJmi+lxjJ5 YhQ5IzomvJ16H0Bq+TLyVLO/VRksp1VR9HxCzItLNCS8PdpYYz5TC204ViycobYU65WMpzWe LFAGn8jSS25XIpqv0Y9k87dLbctKKA14Ifw2kq5OIVu2FuX+3i446JOa2vpCI9GcjCzi3oHV e00bzYiHMIl0FICrNJU0Kjho8pdo0m2uxkn6SYEpogAy9pnatUlO+erL4LqFUO7GXSdBRbw5 gNt25XTLdSFuZtMxkY3tq8MFss5QnjhehCVPEpE6y9ZjI4XB8ad1G4oBHVGK5LMsvg22PfMJ ISWFSHoF/B5+lHkCKWkFxZ0gZn33ju5n6/FOdEx4B8cMJt+cWwARAQABzSlBbmRyZXcgQ29v cGVyIDxhbmRyZXcuY29vcGVyM0BjaXRyaXguY29tPsLBegQTAQgAJAIbAwULCQgHAwUVCgkI CwUWAgMBAAIeAQIXgAUCWKD95wIZAQAKCRBlw/kGpdefoHbdD/9AIoR3k6fKl+RFiFpyAhvO 59ttDFI7nIAnlYngev2XUR3acFElJATHSDO0ju+hqWqAb8kVijXLops0gOfqt3VPZq9cuHlh IMDquatGLzAadfFx2eQYIYT+FYuMoPZy/aTUazmJIDVxP7L383grjIkn+7tAv+qeDfE+txL4 SAm1UHNvmdfgL2/lcmL3xRh7sub3nJilM93RWX1Pe5LBSDXO45uzCGEdst6uSlzYR/MEr+5Z JQQ32JV64zwvf/aKaagSQSQMYNX9JFgfZ3TKWC1KJQbX5ssoX/5hNLqxMcZV3TN7kU8I3kjK mPec9+1nECOjjJSO/h4P0sBZyIUGfguwzhEeGf4sMCuSEM4xjCnwiBwftR17sr0spYcOpqET ZGcAmyYcNjy6CYadNCnfR40vhhWuCfNCBzWnUW0lFoo12wb0YnzoOLjvfD6OL3JjIUJNOmJy RCsJ5IA/Iz33RhSVRmROu+TztwuThClw63g7+hoyewv7BemKyuU6FTVhjjW+XUWmS/FzknSi dAG+insr0746cTPpSkGl3KAXeWDGJzve7/SBBfyznWCMGaf8E2P1oOdIZRxHgWj0zNr1+ooF /PzgLPiCI4OMUttTlEKChgbUTQ+5o0P080JojqfXwbPAyumbaYcQNiH1/xYbJdOFSiBv9rpt TQTBLzDKXok86M7BTQRS4TZ/ARAAkgqudHsp+hd82UVkvgnlqZjzz2vyrYfz7bkPtXaGb9H4 Rfo7mQsEQavEBdWWjbga6eMnDqtu+FC+qeTGYebToxEyp2lKDSoAsvt8w82tIlP/EbmRbDVn 7bhjBlfRcFjVYw8uVDPptT0TV47vpoCVkTwcyb6OltJrvg/QzV9f07DJswuda1JH3/qvYu0p vjPnYvCq4NsqY2XSdAJ02HrdYPFtNyPEntu1n1KK+gJrstjtw7KsZ4ygXYrsm/oCBiVW/OgU g/XIlGErkrxe4vQvJyVwg6YH653YTX5hLLUEL1NS4TCo47RP+wi6y+TnuAL36UtK/uFyEuPy wwrDVcC4cIFhYSfsO0BumEI65yu7a8aHbGfq2lW251UcoU48Z27ZUUZd2Dr6O/n8poQHbaTd 6bJJSjzGGHZVbRP9UQ3lkmkmc0+XCHmj5WhwNNYjgbbmML7y0fsJT5RgvefAIFfHBg7fTY/i kBEimoUsTEQz+N4hbKwo1hULfVxDJStE4sbPhjbsPCrlXf6W9CxSyQ0qmZ2bXsLQYRj2xqd1 bpA+1o1j2N4/au1R/uSiUFjewJdT/LX1EklKDcQwpk06Af/N7VZtSfEJeRV04unbsKVXWZAk uAJyDDKN99ziC0Wz5kcPyVD1HNf8bgaqGDzrv3TfYjwqayRFcMf7xJaL9xXedMcAEQEAAcLB XwQYAQgACQUCUuE2fwIbDAAKCRBlw/kGpdefoG4XEACD1Qf/er8EA7g23HMxYWd3FXHThrVQ HgiGdk5Yh632vjOm9L4sd/GCEACVQKjsu98e8o3ysitFlznEns5EAAXEbITrgKWXDDUWGYxd pnjj2u+GkVdsOAGk0kxczX6s+VRBhpbBI2PWnOsRJgU2n10PZ3mZD4Xu9kU2IXYmuW+e5KCA vTArRUdCrAtIa1k01sPipPPw6dfxx2e5asy21YOytzxuWFfJTGnVxZZSCyLUO83sh6OZhJkk b9rxL9wPmpN/t2IPaEKoAc0FTQZS36wAMOXkBh24PQ9gaLJvfPKpNzGD8XWR5HHF0NLIJhgg 4ZlEXQ2fVp3XrtocHqhu4UZR4koCijgB8sB7Tb0GCpwK+C4UePdFLfhKyRdSXuvY3AHJd4CP 4JzW0Bzq/WXY3XMOzUTYApGQpnUpdOmuQSfpV9MQO+/jo7r6yPbxT7CwRS5dcQPzUiuHLK9i nvjREdh84qycnx0/6dDroYhp0DFv4udxuAvt1h4wGwTPRQZerSm4xaYegEFusyhbZrI0U9tJ B8WrhBLXDiYlyJT6zOV2yZFuW47VrLsjYnHwn27hmxTC/7tvG3euCklmkn9Sl9IAKFu29RSo d5bD8kMSCYsTqtTfT6W4A3qHGvIDta3ptLYpIAOD2sY3GYq2nf3Bbzx81wZK14JdDDHUX2Rs 6+ahAA==
  • Cc: Wei Liu <wei.liu2@xxxxxxxxxx>, Adrian Pop <apop@xxxxxxxxxxxxxxx>, Sergej Proskurin <proskurin@xxxxxxxxxxxxx>, Ian Jackson <ian.jackson@xxxxxxxxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Wed, 05 Sep 2018 18:45:38 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Openpgp: preference=signencrypt

On 05/09/18 19:40, Tamas K Lengyel wrote:
> On Wed, Sep 5, 2018 at 10:40 AM Razvan Cojocaru
> <rcojocaru@xxxxxxxxxxxxxxx> wrote:
>> On 9/5/18 7:28 PM, Tamas K Lengyel wrote:
>>> On Tue, Sep 4, 2018 at 2:58 PM Razvan Cojocaru
>>> <rcojocaru@xxxxxxxxxxxxxxx> wrote:
>>>> On 9/4/18 11:40 PM, Tamas K Lengyel wrote:
>>>>> On Mon, Sep 3, 2018 at 10:59 PM Adrian Pop <apop@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx>
>>>>>> ---
>>>>>> 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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

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