|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 2/2] x86/altp2m: Add a hvmop for setting the suppress #VE bit
On Thu, Jun 15, 2017 at 8:01 PM, Tamas K Lengyel <tamas@xxxxxxxxxxxxx> wrote:
> On Fri, Jun 9, 2017 at 10:51 AM, Adrian Pop <apop@xxxxxxxxxxxxxxx> wrote:
>> Introduce a new hvmop, HVMOP_altp2m_set_suppress_ve, which allows a
>> privileged domain to change the value of the #VE suppress bit for a
>> page.
>>
>> Add a libxc wrapper for invoking this hvmop.
>>
>> Signed-off-by: Adrian Pop <apop@xxxxxxxxxxxxxxx>
>> ---
>> tools/libxc/include/xenctrl.h | 2 ++
>> tools/libxc/xc_altp2m.c | 24 +++++++++++++++++++
>> xen/arch/x86/hvm/hvm.c | 14 +++++++++++
>> xen/arch/x86/mm/mem_access.c | 52
>> +++++++++++++++++++++++++++++++++++++++++
>> xen/include/public/hvm/hvm_op.h | 15 ++++++++++++
>> xen/include/xen/mem_access.h | 3 +++
>> 6 files changed, 110 insertions(+)
>>
>> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
>> index 1629f412dd..f6ba8635bf 100644
>> --- a/tools/libxc/include/xenctrl.h
>> +++ b/tools/libxc/include/xenctrl.h
>> @@ -1926,6 +1926,8 @@ int xc_altp2m_destroy_view(xc_interface *handle,
>> domid_t domid,
>> /* Switch all vCPUs of the domain to the specified altp2m view */
>> int xc_altp2m_switch_to_view(xc_interface *handle, domid_t domid,
>> uint16_t view_id);
>> +int xc_altp2m_set_suppress_ve(xc_interface *handle, domid_t domid,
>> + uint16_t view_id, xen_pfn_t gfn, bool sve);
>> int xc_altp2m_set_mem_access(xc_interface *handle, domid_t domid,
>> uint16_t view_id, xen_pfn_t gfn,
>> xenmem_access_t access);
>> diff --git a/tools/libxc/xc_altp2m.c b/tools/libxc/xc_altp2m.c
>> index 0639632477..4710133918 100644
>> --- a/tools/libxc/xc_altp2m.c
>> +++ b/tools/libxc/xc_altp2m.c
>> @@ -163,6 +163,30 @@ int xc_altp2m_switch_to_view(xc_interface *handle,
>> domid_t domid,
>> return rc;
>> }
>>
>> +int xc_altp2m_set_suppress_ve(xc_interface *handle, domid_t domid,
>> + uint16_t view_id, xen_pfn_t gfn, bool sve)
>> +{
>> + int rc;
>> + DECLARE_HYPERCALL_BUFFER(xen_hvm_altp2m_op_t, arg);
>> +
>> + arg = xc_hypercall_buffer_alloc(handle, arg, sizeof(*arg));
>> + if ( arg == NULL )
>> + return -1;
>> +
>> + arg->version = HVMOP_ALTP2M_INTERFACE_VERSION;
>> + arg->cmd = HVMOP_altp2m_set_suppress_ve;
>> + arg->domain = domid;
>> + arg->u.set_suppress_ve.view = view_id;
>> + arg->u.set_suppress_ve.gfn = gfn;
>> + arg->u.set_suppress_ve.suppress_ve = sve;
>> +
>> + rc = xencall2(handle->xcall, __HYPERVISOR_hvm_op, HVMOP_altp2m,
>> + HYPERCALL_BUFFER_AS_ARG(arg));
>> +
>> + xc_hypercall_buffer_free(handle, arg);
>> + return rc;
>> +}
>> +
>> int xc_altp2m_set_mem_access(xc_interface *handle, domid_t domid,
>> uint16_t view_id, xen_pfn_t gfn,
>> xenmem_access_t access)
>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
>> index 70ddc81d44..dd8e205551 100644
>> --- a/xen/arch/x86/hvm/hvm.c
>> +++ b/xen/arch/x86/hvm/hvm.c
>> @@ -4358,6 +4358,7 @@ static int do_altp2m_op(
>> case HVMOP_altp2m_destroy_p2m:
>> case HVMOP_altp2m_switch_p2m:
>> case HVMOP_altp2m_set_mem_access:
>> + case HVMOP_altp2m_set_suppress_ve:
>> case HVMOP_altp2m_change_gfn:
>> break;
>> default:
>> @@ -4475,6 +4476,19 @@ static int do_altp2m_op(
>> a.u.set_mem_access.view);
>> break;
>>
>> + case HVMOP_altp2m_set_suppress_ve:
>> + if ( a.u.set_suppress_ve.pad1 || a.u.set_suppress_ve.pad2 )
>> + rc = -EINVAL;
>> + else
>> + {
>> + gfn_t gfn = _gfn(a.u.set_mem_access.gfn);
>> + unsigned int altp2m_idx = a.u.set_mem_access.view;
>> + bool suppress_ve = a.u.set_suppress_ve.suppress_ve;
>> +
>> + rc = p2m_set_suppress_ve(d, gfn, suppress_ve, altp2m_idx);
>> + }
>> + break;
>> +
>> case HVMOP_altp2m_change_gfn:
>> if ( a.u.change_gfn.pad1 || a.u.change_gfn.pad2 )
>> rc = -EINVAL;
>> diff --git a/xen/arch/x86/mm/mem_access.c b/xen/arch/x86/mm/mem_access.c
>> index d0b0767855..8c39db13e3 100644
>> --- a/xen/arch/x86/mm/mem_access.c
>> +++ b/xen/arch/x86/mm/mem_access.c
>> @@ -466,6 +466,58 @@ int p2m_get_mem_access(struct domain *d, gfn_t gfn,
>> xenmem_access_t *access)
>> }
>>
>> /*
>> + * Set/clear the #VE suppress bit for a page. Only available on VMX.
>> + */
>> +int p2m_set_suppress_ve(struct domain *d, gfn_t gfn, bool suppress_ve,
>> + unsigned int altp2m_idx)
>> +{
>> + struct p2m_domain *host_p2m = p2m_get_hostp2m(d);
>> + struct p2m_domain *ap2m = NULL;
>> + struct p2m_domain *p2m;
>> + mfn_t mfn;
>> + p2m_access_t a;
>> + p2m_type_t t;
>> + int rc;
>> +
>> + if ( !cpu_has_vmx_virt_exceptions )
>> + return -EOPNOTSUPP;
>> +
>> + /* This subop should only be used from a privileged domain. */
>> + if ( !current->domain->is_privileged )
>> + return -EINVAL;
>
> This check looks wrong to me. If this subop should only be used by an
> external (privileged) domain then I don't think this should be
> implemented as an HVMOP, looks more like a domctl to me.
Well after patch 1, isn't it the case that a guest has no way of
clearing the suppress_ve bit?
I was going to say we want the XSM_TARGET "default action" (which
allows a guest to do things on itself, or a privileged domain to do it
to any domain); but I think really we probably we don't want a guest
to be able to *clear* the suppress_ve bit on a page for which a
privileged domain has *set*; this would allow a domain to prevent the
other domain from effectively introspecting on a page.
This is starting to sound like another conversation I think I remember
recently about making sure that *only* the guest *or* an introspection
engine can use the altp2m functionality, but I can't seem to find that
with a quick look. Tamas, does that ring any bells?
-George
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |