[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 |