Re: [Xen-devel] [PATCH for-next] x86/svm: Correct vm_event API for descriptor accesses
Hello, Andrew Cooper writes: > c/s d0a699a389f1 "x86/monitor: add support for descriptor access events" > introduced logic looking for what appeared to be exitinfo (not that this > exists in SVM - exitinfo1 or 2 do), but actually passed the exit IDT vectoring > information. There is never any IDT vectoring involved in these intercepts so > the value passed is always zero. > > In fact, SVM doesn't provide any information, even in exitinfo1 and 2. Note > the error in the public API and state that this field is always 0, and drop > the SVM logic in hvm_monitor_descriptor_access(). > > In the SVM vmexit handler itself, optimise the switch statement by observing > that there is a linear transformation between the SVM exit_reason and > VM_EVENT_DESC_* values. (Bloat-o-meter reports 6028 => 5877 for a saving of > 151 bytes). > > Signed-off-by: Andrew Cooper > --- > CC: Jan Beulich > CC: Wei Liu > CC: Roger Pau Monné > CC: Razvan Cojocaru > CC: Tamas K Lengyel > CC: Alexandru Isaila > CC: Petre Pircalabu > CC: Adrian Pop > > Adrian: Do you recall what information you were attempting to forward from the > VMCB? I can't locate anything which would plausibly be interesting. The SVM part was most likely meant to mirror the logic from VMX. But, as I recall, monitoring hadn't been implemented with SVM so this couldn't really be used on AMD. Unfortunately I'm not sure what information was supposed to be forwarded. The cleanup looks good to me. Acked-by: Adrian Pop > This is part of a longer cleanup series I gathered in the wake of the task > switch issues, but it is pulled out ahead due to its interaction with the > public interface. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v6 0/3] Add hvmops for setting and getting the suppress #VE bit
As the code stands right now, after DomU has enabled #VE using HVMOP_altp2m_vcpu_enable_notify, all its pages have the #VE suppress bit cleared, generating #VEs for any EPT violation. There is currently no way to change the value of the #VE suppress bit for a page from a domain; it can only be done in Xen internally using ept_set_entry(). Following the discussion from https://lists.xen.org/archives/html/xen-devel/2017-03/msg01312.html these patches introduce new hvmops for setting and getting this bit and thus Adrian Pop (2): x86/altp2m: Add a hvmop for setting the suppress #VE bit x86/altp2m: Add a hvmop for querying the suppress #VE bit Vlad Ioan Topan (1): x86/mm: Change default value for suppress #VE in set_mem_access() tools/libxc/include/xenctrl.h | 4 ++ tools/libxc/xc_altp2m.c | 50 +++ xen/arch/x86/hvm/hvm.c | 33 ++ xen/arch/x86/mm/mem_access.c| 107 +++- xen/include/public/hvm/hvm_op.h | 13 xen/include/xen/mem_access.h| 6 ++ 6 files changed, 211 insertions(+), 2 deletions(-) -- 2.18.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v6 1/3] x86/mm: Change default value for suppress #VE in set_mem_access()
From: Vlad Ioan Topan The default value for the "suppress #VE" bit set by set_mem_access() currently depends on whether the call is made from the same domain (the bit is set when called from another domain and cleared if called from the same domain). This patch changes that behavior to inherit the old suppress #VE bit value if it is already set and to set it to 1 otherwise, which is safer and more reliable. Signed-off-by: Vlad Ioan Topan Signed-off-by: Adrian Pop --- Changes in v6: - add a comment in the code to explain the meaning of the -1 magic value --- xen/arch/x86/mm/mem_access.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/xen/arch/x86/mm/mem_access.c b/xen/arch/x86/mm/mem_access.c index a8b3e99ec4..c980f1744d 100644 --- a/xen/arch/x86/mm/mem_access.c +++ b/xen/arch/x86/mm/mem_access.c @@ -285,8 +285,11 @@ int p2m_set_altp2m_mem_access(struct domain *d, struct p2m_domain *hp2m, } } -return ap2m->set_entry(ap2m, gfn, mfn, PAGE_ORDER_4K, t, a, - current->domain != d); +/* + * Inherit the old suppress #VE bit value if it is already set, or set it + * to 1 otherwise + */ +return ap2m->set_entry(ap2m, gfn, mfn, PAGE_ORDER_4K, t, a, -1); } static int set_mem_access(struct domain *d, struct p2m_domain *p2m, -- 2.18.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v6 2/3] x86/altp2m: Add a hvmop for setting the suppress #VE bit
Introduce a new hvmop, HVMOP_altp2m_set_suppress_ve, which allows a
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
Acked-by: Wei Liu
Acked-by: Tamas K Lengyel
---
Changes in v5:
- remove the "set_" from struct xen_hvm_altp2m_set_suppress_ve
Changes in v4:
- fix a deadlock:
If p2m_set_suppress_ve() is called on invalid pages the code path
wrongly returns without releasing the lock, resulting in a deadlock.
- remove the privileged domain check
Changes in v3:
- fix indentation (Wei Liu)
- use return values other than EINVAL where appropriate (Ian Beulich)
- remove the irrelevant comments from the
xen_hvm_altp2m_set_suppress_ve struct (Ian Beulich)
- add comment for the suppress_ve field in the struct above (Ian
Beulich)
- remove the typedef and DEFINE_XEN_GUEST_HANDLE for
xen_hvm_altp2m_set_suppress_ve (Ian Beulich)
- use XSM_DM_PRIV check instead of domain->is_privileged (Ian Beulich)
Changes in v2:
- check if #VE has been enabled on the target domain (Tamas K Lengyel)
- check if the cpu has the #VE feature
- make the suppress_ve argument boolean (Jan Beulich)
- initialize only local variables that need initializing (Jan Beulich)
- use fewer local variables (Jan Beulich)
- fix indentation (Jan Beulich)
- remove unnecessary braces (Jan Beulich)
- use gfn_lock() instead of p2m_lock() in the non-altp2m case (Jan
Beulich)
- merge patch #2 and patch #3 (Jan Beulich)
---
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| 55 +
xen/include/public/hvm/hvm_op.h | 11 +++
xen/include/xen/mem_access.h| 3 ++
6 files changed, 109 insertions(+)
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index bb75bcc84d..9b0f55c649 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1939,6 +1939,8 @@ int xc_altp2m_destroy_view(xc_interface *handle, uint32_t
domid,
/* Switch all vCPUs of the domain to the specified altp2m view */
int xc_altp2m_switch_to_view(xc_interface *handle, uint32_t domid,
uint16_t view_id);
+int xc_altp2m_set_suppress_ve(xc_interface *handle, uint32_t domid,
+ uint16_t view_id, xen_pfn_t gfn, bool sve);
int xc_altp2m_set_mem_access(xc_interface *handle, uint32_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 ce4a1e4d60..f883d0b392 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,
uint32_t domid,
return rc;
}
+int xc_altp2m_set_suppress_ve(xc_interface *handle, uint32_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.suppress_ve.view = view_id;
+arg->u.suppress_ve.gfn = gfn;
+arg->u.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, uint32_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 062872cb71..e2073b6b03 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4492,6 +4492,7 @@ static int do_altp2m_op(
case HVMOP_altp2m_create_p2m:
case HVMOP_altp2m_destroy_p2m:
case HVMOP_altp2m_switch_p2m:
+case HVMOP_altp2m_set_suppress_ve:
case HVMOP_altp2m_set_mem_access:
case HVMOP_altp2m_set_mem_access_multi:
case HVMOP_altp2m_change_gfn:
@@ -4609,6 +4610,19 @@ static int do_altp2m_op(
rc = p2m_switch_domain_altp2m_by_id(d, a.u.view.view);
break;
+case HVMOP_altp2m_set_suppress_ve:
+if ( a.u.suppress_ve.pad1 || a.u.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.suppress_ve.suppress_ve;
+
+rc = p2m_set_suppress_ve(d, gfn, suppress_ve, altp2m_idx);
+}
+break;
+
case HVMOP_altp2m_set_mem_access:
if ( a.u.set_mem_access.pad )
rc =
[Xen-devel] [PATCH v6 3/3] x86/altp2m: Add a hvmop for querying the suppress #VE bit
Signed-off-by: Adrian Pop
Acked-by: Tamas K Lengyel
---
tools/libxc/include/xenctrl.h | 2 ++
tools/libxc/xc_altp2m.c | 26 +++
xen/arch/x86/hvm/hvm.c | 19 ++
xen/arch/x86/mm/mem_access.c| 45 +
xen/include/public/hvm/hvm_op.h | 2 ++
xen/include/xen/mem_access.h| 3 +++
6 files changed, 97 insertions(+)
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 9b0f55c649..dad96a95a3 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1941,6 +1941,8 @@ int xc_altp2m_switch_to_view(xc_interface *handle,
uint32_t domid,
uint16_t view_id);
int xc_altp2m_set_suppress_ve(xc_interface *handle, uint32_t domid,
uint16_t view_id, xen_pfn_t gfn, bool sve);
+int xc_altp2m_get_suppress_ve(xc_interface *handle, uint32_t domid,
+ uint16_t view_id, xen_pfn_t gfn, bool *sve);
int xc_altp2m_set_mem_access(xc_interface *handle, uint32_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 f883d0b392..1c9b572e2b 100644
--- a/tools/libxc/xc_altp2m.c
+++ b/tools/libxc/xc_altp2m.c
@@ -163,6 +163,32 @@ int xc_altp2m_switch_to_view(xc_interface *handle,
uint32_t domid,
return rc;
}
+int xc_altp2m_get_suppress_ve(xc_interface *handle, uint32_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_get_suppress_ve;
+arg->domain = domid;
+arg->u.suppress_ve.view = view_id;
+arg->u.suppress_ve.gfn = gfn;
+
+rc = xencall2(handle->xcall, __HYPERVISOR_hvm_op, HVMOP_altp2m,
+ HYPERCALL_BUFFER_AS_ARG(arg));
+
+if ( !rc )
+*sve = arg->u.suppress_ve.suppress_ve;
+
+xc_hypercall_buffer_free(handle, arg);
+return rc;
+}
+
int xc_altp2m_set_suppress_ve(xc_interface *handle, uint32_t domid,
uint16_t view_id, xen_pfn_t gfn, bool sve)
{
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index e2073b6b03..f0b3e7b2c2 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4493,6 +4493,7 @@ static int do_altp2m_op(
case HVMOP_altp2m_destroy_p2m:
case HVMOP_altp2m_switch_p2m:
case HVMOP_altp2m_set_suppress_ve:
+case HVMOP_altp2m_get_suppress_ve:
case HVMOP_altp2m_set_mem_access:
case HVMOP_altp2m_set_mem_access_multi:
case HVMOP_altp2m_change_gfn:
@@ -4623,6 +4624,24 @@ static int do_altp2m_op(
}
break;
+case HVMOP_altp2m_get_suppress_ve:
+if ( a.u.suppress_ve.pad1 || a.u.suppress_ve.pad2 )
+rc = -EINVAL;
+else
+{
+gfn_t gfn = _gfn(a.u.suppress_ve.gfn);
+unsigned int altp2m_idx = a.u.suppress_ve.view;
+bool suppress_ve;
+
+rc = p2m_get_suppress_ve(d, gfn, _ve, altp2m_idx);
+if ( !rc )
+{
+a.u.suppress_ve.suppress_ve = suppress_ve;
+rc = __copy_to_guest(arg, , 1) ? -EFAULT : 0;
+}
+}
+break;
+
case HVMOP_altp2m_set_mem_access:
if ( a.u.set_mem_access.pad )
rc = -EINVAL;
diff --git a/xen/arch/x86/mm/mem_access.c b/xen/arch/x86/mm/mem_access.c
index 6ac9ef3575..2217bda4c5 100644
--- a/xen/arch/x86/mm/mem_access.c
+++ b/xen/arch/x86/mm/mem_access.c
@@ -556,6 +556,51 @@ out:
return rc;
}
+int p2m_get_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;
+
+if ( !cpu_has_vmx_virt_exceptions )
+return -EOPNOTSUPP;
+
+/* #VE should be enabled for this vcpu. */
+if ( gfn_eq(vcpu_altp2m(current).veinfo_gfn, INVALID_GFN) )
+return -ENXIO;
+
+if ( altp2m_idx > 0 )
+{
+if ( altp2m_idx >= MAX_ALTP2M ||
+ d->arch.altp2m_eptp[altp2m_idx] == mfn_x(INVALID_MFN) )
+return -EINVAL;
+
+p2m = ap2m = d->arch.altp2m_p2m[altp2m_idx];
+}
+else
+p2m = host_p2m;
+
+gfn_lock(host_p2m, gfn, 0);
+
+if ( ap2m )
+p2m_lock(ap2m);
+
+mfn = p2m->get_entry(p2m, gfn, , , 0, NULL, suppress_ve);
+if ( !mfn_valid(mfn) )
+return -ESRCH;
+
+if ( ap2m )
+p2m_unlock(ap2m);
+
+gfn_unlock(host_p2m, gfn, 0);
+
+return 0;
+}
+
/*
* Local variables:
Re: [Xen-devel] [PATCH v5 1/3] x86/mm: Change default value for suppress #VE in set_mem_access()
On Tue, Sep 11, 2018 at 12:08:42PM -0600, Tamas K Lengyel wrote: > On Mon, Sep 3, 2018 at 9:48 AM Adrian Pop wrote: > > > > From: Vlad Ioan Topan > > > > The default value for the "suppress #VE" bit set by set_mem_access() > > currently depends on whether the call is made from the same domain (the > > bit is set when called from another domain and cleared if called from > > the same domain). This patch changes that behavior to inherit the old > > suppress #VE bit value if it is already set and to set it to 1 > > otherwise, which is safer and more reliable. > > > > Signed-off-by: Vlad Ioan Topan > > Signed-off-by: Adrian Pop > > --- > > xen/arch/x86/mm/mem_access.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/xen/arch/x86/mm/mem_access.c b/xen/arch/x86/mm/mem_access.c > > index 84d260ebd8..e1522a0b75 100644 > > --- a/xen/arch/x86/mm/mem_access.c > > +++ b/xen/arch/x86/mm/mem_access.c > > @@ -284,8 +284,7 @@ int p2m_set_altp2m_mem_access(struct domain *d, struct > > p2m_domain *hp2m, > > } > > } > > > > -return ap2m->set_entry(ap2m, gfn, mfn, PAGE_ORDER_4K, t, a, > > - current->domain != d); > > Could you add a comment here saying "inherit the old suppress #VE bit > value if it is already set and to set it to 1 otherwise" to explain > the meaning of the magic -1 value? Sure. Thank you! ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 3/3] x86/altp2m: Add a hvmop for querying the suppress #VE bit
On Tue, Sep 11, 2018 at 12:02:39PM -0600, Tamas K Lengyel wrote: > On Mon, Sep 3, 2018 at 9:48 AM Adrian Pop wrote: > > > > Signed-off-by: Adrian Pop > > Acked-by: Tamas K Lengyel Thanks! ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2] x86/altp2m: Allow setting the #VE info page for an arbitrary VCPU
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
---
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, ), 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-
[Xen-devel] [PATCH v5 2/3] x86/altp2m: Add a hvmop for setting the suppress #VE bit
Introduce a new hvmop, HVMOP_altp2m_set_suppress_ve, which allows a
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
Acked-by: Wei Liu
Acked-by: Tamas K Lengyel
---
Changes in v5:
- remove the "set_" from struct xen_hvm_altp2m_set_suppress_ve
Changes in v4:
- fix a deadlock:
If p2m_set_suppress_ve() is called on invalid pages the code path
wrongly returns without releasing the lock, resulting in a deadlock.
- remove the privileged domain check
Changes in v3:
- fix indentation (Wei Liu)
- use return values other than EINVAL where appropriate (Ian Beulich)
- remove the irrelevant comments from the
xen_hvm_altp2m_set_suppress_ve struct (Ian Beulich)
- add comment for the suppress_ve field in the struct above (Ian
Beulich)
- remove the typedef and DEFINE_XEN_GUEST_HANDLE for
xen_hvm_altp2m_set_suppress_ve (Ian Beulich)
- use XSM_DM_PRIV check instead of domain->is_privileged (Ian Beulich)
Changes in v2:
- check if #VE has been enabled on the target domain (Tamas K Lengyel)
- check if the cpu has the #VE feature
- make the suppress_ve argument boolean (Jan Beulich)
- initialize only local variables that need initializing (Jan Beulich)
- use fewer local variables (Jan Beulich)
- fix indentation (Jan Beulich)
- remove unnecessary braces (Jan Beulich)
- use gfn_lock() instead of p2m_lock() in the non-altp2m case (Jan
Beulich)
- merge patch #2 and patch #3 (Jan Beulich)
---
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| 55 +
xen/include/public/hvm/hvm_op.h | 11 +++
xen/include/xen/mem_access.h| 3 ++
6 files changed, 109 insertions(+)
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index c626984aba..cc8b3e7dce 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1952,6 +1952,8 @@ int xc_altp2m_destroy_view(xc_interface *handle, uint32_t
domid,
/* Switch all vCPUs of the domain to the specified altp2m view */
int xc_altp2m_switch_to_view(xc_interface *handle, uint32_t domid,
uint16_t view_id);
+int xc_altp2m_set_suppress_ve(xc_interface *handle, uint32_t domid,
+ uint16_t view_id, xen_pfn_t gfn, bool sve);
int xc_altp2m_set_mem_access(xc_interface *handle, uint32_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 ce4a1e4d60..f883d0b392 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,
uint32_t domid,
return rc;
}
+int xc_altp2m_set_suppress_ve(xc_interface *handle, uint32_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.suppress_ve.view = view_id;
+arg->u.suppress_ve.gfn = gfn;
+arg->u.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, uint32_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 72c51faecb..64ab36ff53 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4524,6 +4524,7 @@ static int do_altp2m_op(
case HVMOP_altp2m_create_p2m:
case HVMOP_altp2m_destroy_p2m:
case HVMOP_altp2m_switch_p2m:
+case HVMOP_altp2m_set_suppress_ve:
case HVMOP_altp2m_set_mem_access:
case HVMOP_altp2m_set_mem_access_multi:
case HVMOP_altp2m_change_gfn:
@@ -4641,6 +4642,19 @@ static int do_altp2m_op(
rc = p2m_switch_domain_altp2m_by_id(d, a.u.view.view);
break;
+case HVMOP_altp2m_set_suppress_ve:
+if ( a.u.suppress_ve.pad1 || a.u.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.suppress_ve.suppress_ve;
+
+rc = p2m_set_suppress_ve(d, gfn, suppress_ve, altp2m_idx);
+}
+break;
+
case HVMOP_altp2m_set_mem_access:
if ( a.u.set_mem_access.pad )
rc =
[Xen-devel] [PATCH v5 0/3] Add hvmops for setting and getting the suppress #VE bit
As the code stands right now, after DomU has enabled #VE using HVMOP_altp2m_vcpu_enable_notify, all its pages have the #VE suppress bit cleared, generating #VEs for any EPT violation. There is currently no way to change the value of the #VE suppress bit for a page from a domain; it can only be done in Xen internally using ept_set_entry(). Following the discussion from https://lists.xen.org/archives/html/xen-devel/2017-03/msg01312.html these patches introduce new hvmops for setting and getting this bit and thus Adrian Pop (3): x86/altp2m: Add a hvmop for setting the suppress #VE bit x86/altp2m: Add a hvmop for querying the suppress #VE bit Vlad Ioan Topan (1): x86/mm: Change default value for suppress #VE in set_mem_access() tools/libxc/include/xenctrl.h | 4 ++ tools/libxc/xc_altp2m.c | 50 xen/arch/x86/hvm/hvm.c | 33 ++ xen/arch/x86/mm/mem_access.c| 103 +++- xen/include/public/hvm/hvm_op.h | 13 xen/include/xen/mem_access.h| 6 ++ 6 files changed, 207 insertions(+), 2 deletions(-) -- 2.18.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v5 1/3] x86/mm: Change default value for suppress #VE in set_mem_access()
From: Vlad Ioan Topan The default value for the "suppress #VE" bit set by set_mem_access() currently depends on whether the call is made from the same domain (the bit is set when called from another domain and cleared if called from the same domain). This patch changes that behavior to inherit the old suppress #VE bit value if it is already set and to set it to 1 otherwise, which is safer and more reliable. Signed-off-by: Vlad Ioan Topan Signed-off-by: Adrian Pop --- xen/arch/x86/mm/mem_access.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/xen/arch/x86/mm/mem_access.c b/xen/arch/x86/mm/mem_access.c index 84d260ebd8..e1522a0b75 100644 --- a/xen/arch/x86/mm/mem_access.c +++ b/xen/arch/x86/mm/mem_access.c @@ -284,8 +284,7 @@ int p2m_set_altp2m_mem_access(struct domain *d, struct p2m_domain *hp2m, } } -return ap2m->set_entry(ap2m, gfn, mfn, PAGE_ORDER_4K, t, a, - current->domain != d); +return ap2m->set_entry(ap2m, gfn, mfn, PAGE_ORDER_4K, t, a, -1); } static int set_mem_access(struct domain *d, struct p2m_domain *p2m, -- 2.18.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v4] x86/altp2m: Add a subop for obtaining the mem access of a page
Currently there is a subop for setting the memaccess of a page, but not
for consulting it. The new HVMOP_altp2m_get_mem_access adds this
functionality.
Both altp2m get/set mem access functions use the struct
xen_hvm_altp2m_mem_access which has now dropped the `set' part and has
been renamed from xen_hvm_altp2m_set_mem_access.
Signed-off-by: Adrian Pop
---
Changes in v4:
- don't break the stable interface
Changes in v3:
- remove the unrelated helper function
- simplify the locking in p2m_get_mem_access
Changes in v2:
- use the _p2m_get_mem_access helper from p2m_get_mem_access
- minor Arm adjustments
- move out the addition of a memaccess helper function to a separate
patch in the attempts of making the diff clearer
---
tools/libxc/include/xenctrl.h | 3 +++
tools/libxc/xc_altp2m.c | 33 +
xen/arch/arm/mem_access.c | 8 ++--
xen/arch/x86/hvm/hvm.c | 27 +++
xen/arch/x86/mm/mem_access.c| 21 +++--
xen/common/mem_access.c | 2 +-
xen/include/public/hvm/hvm_op.h | 19 +++
xen/include/public/xen-compat.h | 2 +-
xen/include/xen/mem_access.h| 3 ++-
9 files changed, 111 insertions(+), 7 deletions(-)
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index c626984aba..ae298899fc 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1958,6 +1958,9 @@ int xc_altp2m_set_mem_access(xc_interface *handle,
uint32_t domid,
int xc_altp2m_set_mem_access_multi(xc_interface *handle, uint32_t domid,
uint16_t view_id, uint8_t *access,
uint64_t *gfns, uint32_t nr);
+int xc_altp2m_get_mem_access(xc_interface *handle, uint32_t domid,
+ uint16_t view_id, xen_pfn_t gfn,
+ xenmem_access_t *access);
int xc_altp2m_change_gfn(xc_interface *handle, uint32_t domid,
uint16_t view_id, xen_pfn_t old_gfn,
xen_pfn_t new_gfn);
diff --git a/tools/libxc/xc_altp2m.c b/tools/libxc/xc_altp2m.c
index ce4a1e4d60..53754ff6d3 100644
--- a/tools/libxc/xc_altp2m.c
+++ b/tools/libxc/xc_altp2m.c
@@ -177,9 +177,15 @@ int xc_altp2m_set_mem_access(xc_interface *handle,
uint32_t domid,
arg->version = HVMOP_ALTP2M_INTERFACE_VERSION;
arg->cmd = HVMOP_altp2m_set_mem_access;
arg->domain = domid;
+#if __XEN_INTERFACE_VERSION__ < 0x00040a00
arg->u.set_mem_access.view = view_id;
arg->u.set_mem_access.hvmmem_access = access;
arg->u.set_mem_access.gfn = gfn;
+#else /* __XEN_INTERFACE_VERSION__ >= 0x00040a00 */
+arg->u.mem_access.view = view_id;
+arg->u.mem_access.hvmmem_access = access;
+arg->u.mem_access.gfn = gfn;
+#endif
rc = xencall2(handle->xcall, __HYPERVISOR_hvm_op, HVMOP_altp2m,
HYPERCALL_BUFFER_AS_ARG(arg));
@@ -254,3 +260,30 @@ int xc_altp2m_set_mem_access_multi(xc_interface *xch,
uint32_t domid,
return rc;
}
+
+int xc_altp2m_get_mem_access(xc_interface *handle, uint32_t domid,
+ uint16_t view_id, xen_pfn_t gfn,
+ xenmem_access_t *access)
+{
+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_get_mem_access;
+arg->domain = domid;
+arg->u.mem_access.view = view_id;
+arg->u.mem_access.gfn = gfn;
+
+rc = xencall2(handle->xcall, __HYPERVISOR_hvm_op, HVMOP_altp2m,
+ HYPERCALL_BUFFER_AS_ARG(arg));
+
+if ( !rc )
+*access = arg->u.mem_access.hvmmem_access;
+
+xc_hypercall_buffer_free(handle, arg);
+return rc;
+}
diff --git a/xen/arch/arm/mem_access.c b/xen/arch/arm/mem_access.c
index ba4ec780fd..178bc1a6c1 100644
--- a/xen/arch/arm/mem_access.c
+++ b/xen/arch/arm/mem_access.c
@@ -236,7 +236,7 @@ bool p2m_mem_access_check(paddr_t gpa, vaddr_t gla, const
struct npfec npfec)
if ( !p2m->mem_access_enabled )
return true;
-rc = p2m_get_mem_access(v->domain, gaddr_to_gfn(gpa), );
+rc = p2m_get_mem_access(v->domain, gaddr_to_gfn(gpa), , 0);
if ( rc )
return true;
@@ -441,11 +441,15 @@ long p2m_set_mem_access_multi(struct domain *d,
}
int p2m_get_mem_access(struct domain *d, gfn_t gfn,
- xenmem_access_t *access)
+ xenmem_access_t *access, unsigned int altp2m_idx)
{
int ret;
struct p2m_domain *p2m = p2m_get_hostp2m(d);
+/* altp2m is not yet implemented on Arm. The altp2m_idx should be 0. */
+if ( altp2m_idx )
+return -EINVAL;
+
p2m_read_lock(p2m);
ret = __p2m_get_mem_access(d, gfn, access);
p2m_read_unlock(p2m);
Re: [Xen-devel] [PATCH v3] x86/altp2m: Add a subop for obtaining the mem access of a page
On Mon, Aug 27, 2018 at 03:59:16AM -0600, Jan Beulich wrote:
> >>> On 27.08.18 at 11:38, wrote:
> > On Tue, Jul 31, 2018 at 05:37:30AM -0600, Jan Beulich wrote:
> >> >>> On 25.07.18 at 13:16, wrote:
> >> > --- a/xen/include/public/hvm/hvm_op.h
> >> > +++ b/xen/include/public/hvm/hvm_op.h
> >> > @@ -234,7 +234,7 @@ struct xen_hvm_altp2m_view {
> >> > typedef struct xen_hvm_altp2m_view xen_hvm_altp2m_view_t;
> >> > DEFINE_XEN_GUEST_HANDLE(xen_hvm_altp2m_view_t);
> >> >
> >> > -struct xen_hvm_altp2m_set_mem_access {
> >> > +struct xen_hvm_altp2m_mem_access {
> >> > /* view */
> >> > uint16_t view;
> >> > /* Memory type */
> >> > @@ -243,8 +243,8 @@ struct xen_hvm_altp2m_set_mem_access {
> >> > /* gfn */
> >> > uint64_t gfn;
> >> > };
> >> > -typedef struct xen_hvm_altp2m_set_mem_access
> >> > xen_hvm_altp2m_set_mem_access_t;
> >> > -DEFINE_XEN_GUEST_HANDLE(xen_hvm_altp2m_set_mem_access_t);
> >> > +typedef struct xen_hvm_altp2m_mem_access xen_hvm_altp2m_mem_access_t;
> >> > +DEFINE_XEN_GUEST_HANDLE(xen_hvm_altp2m_mem_access_t);
> >> >
> >> > struct xen_hvm_altp2m_set_mem_access_multi {
> >> > /* view */
> >> > @@ -296,6 +296,8 @@ struct xen_hvm_altp2m_op {
> >> > #define HVMOP_altp2m_change_gfn 8
> >> > /* Set access for an array of pages */
> >> > #define HVMOP_altp2m_set_mem_access_multi 9
> >> > +/* Get the access of a page of memory from a certain view */
> >> > +#define HVMOP_altp2m_get_mem_access 10
> >> > domid_t domain;
> >> > uint16_t pad1;
> >> > uint32_t pad2;
> >> > @@ -303,7 +305,7 @@ struct xen_hvm_altp2m_op {
> >> > struct xen_hvm_altp2m_domain_state domain_state;
> >> > struct xen_hvm_altp2m_vcpu_enable_notify enable_notify;
> >> > struct xen_hvm_altp2m_view view;
> >> > -struct xen_hvm_altp2m_set_mem_access set_mem_access;
> >> > +struct xen_hvm_altp2m_mem_access mem_access;
> >> > struct xen_hvm_altp2m_change_gfn change_gfn;
> >> > struct xen_hvm_altp2m_set_mem_access_multi set_mem_access_multi;
> >> > uint8_t pad[64];
> >>
> >> This being exposed to guests, the interface has to be considered
> >> stable imo, in which case you can't rename things like this. You'd
> >> need __XEN_INTERFACE_VERSION__ dependent logic (just like is the
> >> case further up in the file).
> >
> > Right. Sorry about that. Maybe just having separate structs for
> > get/set would be cleaner in this case, even though they would be
> > similar.
>
> Personally I'd prefer to avoid having two structures with identical
> layout but different tags. But if others think differently, I'm not
> meaning to stand in the way.
Ok then. I have no strong preference either way.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/altp2m: Allow setting the #VE info page for an arbitrary VCPU
On Tue, Jul 31, 2018 at 05:53:00AM -0600, Jan Beulich wrote:
> > +struct vcpu *v;
> > +
> > +dom = rcu_lock_domain_by_id(domain_id);
> > +
> > +for_each_vcpu( dom, v )
> > +{
> > +if ( vcpu_id == v->vcpu_id )
> > +{
> > +rcu_unlock_domain(dom);
> > +return v;
> > +}
> > +}
>
> for_each_vcpu() looks excessive here - all you need is a bounds
> check and an access into d->vcpus[]. Together with the fact
> that your caller has already identified and locked d I wonder
> whether this helper is needed in the first place.
All right. I'll remove it altogether.
> > @@ -4576,26 +4599,32 @@ 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 )
> > {
> > rc = -EINVAL;
> > break;
> > }
> >
> > -if ( !gfn_eq(vcpu_altp2m(curr).veinfo_gfn, INVALID_GFN) ||
> > - mfn_eq(get_gfn_query_unlocked(curr->domain,
> > +v = __get_vcpu(a.domain, a.u.enable_notify.vcpu_id);
> > +if ( !v )
> > +{
> > +rc = -EFAULT;
>
> Hardly an appropriate error indicator for the condition.
I'll change it to -EINVAL.
> > +break;
> > +}
> > +
> > +if ( !gfn_eq(vcpu_altp2m(v).veinfo_gfn, INVALID_GFN) ||
> > + mfn_eq(get_gfn_query_unlocked(v->domain,
> > a.u.enable_notify.gfn, ), 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);
>
> I'd like you to outline in the description how you mean an external
> agent to coordinate the use of this GFN with the guest (and in
> particular without in-guest agent).
I'll try to clarify this.
Thank you!
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v4 3/3] x86/altp2m: Add a hvmop for querying the suppress #VE bit
On Tue, Jul 31, 2018 at 05:44:03AM -0600, Jan Beulich wrote:
> >>> On 25.07.18 at 13:18, wrote:
> > --- a/xen/include/public/hvm/hvm_op.h
> > +++ b/xen/include/public/hvm/hvm_op.h
> > @@ -38,7 +38,7 @@ struct xen_hvm_param {
> > typedef struct xen_hvm_param xen_hvm_param_t;
> > DEFINE_XEN_GUEST_HANDLE(xen_hvm_param_t);
> >
> > -struct xen_hvm_altp2m_set_suppress_ve {
> > +struct xen_hvm_altp2m_suppress_ve {
>
> Please add this without the "set_" right away in patch 2.
Sure. Thanks!
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3] x86/altp2m: Add a subop for obtaining the mem access of a page
On Tue, Jul 31, 2018 at 05:37:30AM -0600, Jan Beulich wrote:
> >>> On 25.07.18 at 13:16, wrote:
> > --- a/xen/include/public/hvm/hvm_op.h
> > +++ b/xen/include/public/hvm/hvm_op.h
> > @@ -234,7 +234,7 @@ struct xen_hvm_altp2m_view {
> > typedef struct xen_hvm_altp2m_view xen_hvm_altp2m_view_t;
> > DEFINE_XEN_GUEST_HANDLE(xen_hvm_altp2m_view_t);
> >
> > -struct xen_hvm_altp2m_set_mem_access {
> > +struct xen_hvm_altp2m_mem_access {
> > /* view */
> > uint16_t view;
> > /* Memory type */
> > @@ -243,8 +243,8 @@ struct xen_hvm_altp2m_set_mem_access {
> > /* gfn */
> > uint64_t gfn;
> > };
> > -typedef struct xen_hvm_altp2m_set_mem_access
> > xen_hvm_altp2m_set_mem_access_t;
> > -DEFINE_XEN_GUEST_HANDLE(xen_hvm_altp2m_set_mem_access_t);
> > +typedef struct xen_hvm_altp2m_mem_access xen_hvm_altp2m_mem_access_t;
> > +DEFINE_XEN_GUEST_HANDLE(xen_hvm_altp2m_mem_access_t);
> >
> > struct xen_hvm_altp2m_set_mem_access_multi {
> > /* view */
> > @@ -296,6 +296,8 @@ struct xen_hvm_altp2m_op {
> > #define HVMOP_altp2m_change_gfn 8
> > /* Set access for an array of pages */
> > #define HVMOP_altp2m_set_mem_access_multi 9
> > +/* Get the access of a page of memory from a certain view */
> > +#define HVMOP_altp2m_get_mem_access 10
> > domid_t domain;
> > uint16_t pad1;
> > uint32_t pad2;
> > @@ -303,7 +305,7 @@ struct xen_hvm_altp2m_op {
> > struct xen_hvm_altp2m_domain_state domain_state;
> > struct xen_hvm_altp2m_vcpu_enable_notify enable_notify;
> > struct xen_hvm_altp2m_view view;
> > -struct xen_hvm_altp2m_set_mem_access set_mem_access;
> > +struct xen_hvm_altp2m_mem_access mem_access;
> > struct xen_hvm_altp2m_change_gfn change_gfn;
> > struct xen_hvm_altp2m_set_mem_access_multi set_mem_access_multi;
> > uint8_t pad[64];
>
> This being exposed to guests, the interface has to be considered
> stable imo, in which case you can't rename things like this. You'd
> need __XEN_INTERFACE_VERSION__ dependent logic (just like is the
> case further up in the file).
Right. Sorry about that. Maybe just having separate structs for
get/set would be cleaner in this case, even though they would be
similar.
> Also, to you, George, and whoever else advocates for this, another
> remark regarding the guest accessibility here (at the risk of getting
> flamed once again): The less capable (afaict)
> XENMEM_access_op_{g,s}et_access (previously
> HVMOP_{g,s}et_mem_access) are tools accessible only. I find such
> an inconsistency rather odd.
I don't really know what to reply. While there probably isn't, as far
as I'm aware, any use-case for the non-altp2m OPs to be accessed from
the guest via HVMOPs directly, if my understanding is correct, the
scenario involving #VE and an in-guest agent would require this
functionality.
Thanks.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] x86/altp2m: Allow setting the #VE info page for an arbitrary VCPU
The intended use-case of this patch is to allow either Dom0 or a control
domain to activate #VE for an introspected guest, and not having to do
this necessarily from an in-guest agent.
Signed-off-by: Adrian Pop
---
tools/libxc/xc_altp2m.c | 1 -
xen/arch/x86/hvm/hvm.c | 47 +
2 files changed, 38 insertions(+), 10 deletions(-)
diff --git a/tools/libxc/xc_altp2m.c b/tools/libxc/xc_altp2m.c
index 1c9b572e2b..be5bfd28ed 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 4e318cede4..0157611384 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4467,6 +4467,30 @@ static int hvmop_get_param(
return rc;
}
+/*
+ * Find the struct vcpu given a dom_id and vcpu_id.
+ * Return NULL if not found.
+ */
+static struct vcpu *__get_vcpu(domid_t domain_id, uint32_t vcpu_id)
+{
+struct domain *dom;
+struct vcpu *v;
+
+dom = rcu_lock_domain_by_id(domain_id);
+
+for_each_vcpu( dom, v )
+{
+if ( vcpu_id == v->vcpu_id )
+{
+rcu_unlock_domain(dom);
+return v;
+}
+}
+
+rcu_unlock_domain(dom);
+return NULL;
+}
+
static int do_altp2m_op(
XEN_GUEST_HANDLE_PARAM(void) arg)
{
@@ -4504,8 +4528,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;
@@ -4576,26 +4599,32 @@ 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 )
{
rc = -EINVAL;
break;
}
-if ( !gfn_eq(vcpu_altp2m(curr).veinfo_gfn, INVALID_GFN) ||
- mfn_eq(get_gfn_query_unlocked(curr->domain,
+v = __get_vcpu(a.domain, a.u.enable_notify.vcpu_id);
+if ( !v )
+{
+rc = -EFAULT;
+break;
+}
+
+if ( !gfn_eq(vcpu_altp2m(v).veinfo_gfn, INVALID_GFN) ||
+ mfn_eq(get_gfn_query_unlocked(v->domain,
a.u.enable_notify.gfn, ), 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.17.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v4 3/3] x86/altp2m: Add a hvmop for querying the suppress #VE bit
Signed-off-by: Adrian Pop
---
tools/libxc/include/xenctrl.h | 2 ++
tools/libxc/xc_altp2m.c | 32 ---
xen/arch/x86/hvm/hvm.c | 23 +++--
xen/arch/x86/mm/mem_access.c| 45 +
xen/include/public/hvm/hvm_op.h | 6 +++--
xen/include/xen/mem_access.h| 3 +++
6 files changed, 104 insertions(+), 7 deletions(-)
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 177d4e6903..f100f9cff3 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1968,6 +1968,8 @@ int xc_altp2m_switch_to_view(xc_interface *handle,
uint32_t domid,
uint16_t view_id);
int xc_altp2m_set_suppress_ve(xc_interface *handle, uint32_t domid,
uint16_t view_id, xen_pfn_t gfn, bool sve);
+int xc_altp2m_get_suppress_ve(xc_interface *handle, uint32_t domid,
+ uint16_t view_id, xen_pfn_t gfn, bool *sve);
int xc_altp2m_set_mem_access(xc_interface *handle, uint32_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 f44d0fffa8..1c9b572e2b 100644
--- a/tools/libxc/xc_altp2m.c
+++ b/tools/libxc/xc_altp2m.c
@@ -163,6 +163,32 @@ int xc_altp2m_switch_to_view(xc_interface *handle,
uint32_t domid,
return rc;
}
+int xc_altp2m_get_suppress_ve(xc_interface *handle, uint32_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_get_suppress_ve;
+arg->domain = domid;
+arg->u.suppress_ve.view = view_id;
+arg->u.suppress_ve.gfn = gfn;
+
+rc = xencall2(handle->xcall, __HYPERVISOR_hvm_op, HVMOP_altp2m,
+ HYPERCALL_BUFFER_AS_ARG(arg));
+
+if ( !rc )
+*sve = arg->u.suppress_ve.suppress_ve;
+
+xc_hypercall_buffer_free(handle, arg);
+return rc;
+}
+
int xc_altp2m_set_suppress_ve(xc_interface *handle, uint32_t domid,
uint16_t view_id, xen_pfn_t gfn, bool sve)
{
@@ -176,9 +202,9 @@ int xc_altp2m_set_suppress_ve(xc_interface *handle,
uint32_t domid,
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;
+arg->u.suppress_ve.view = view_id;
+arg->u.suppress_ve.gfn = gfn;
+arg->u.suppress_ve.suppress_ve = sve;
rc = xencall2(handle->xcall, __HYPERVISOR_hvm_op, HVMOP_altp2m,
HYPERCALL_BUFFER_AS_ARG(arg));
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 5689191a56..4e318cede4 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4494,6 +4494,7 @@ static int do_altp2m_op(
case HVMOP_altp2m_destroy_p2m:
case HVMOP_altp2m_switch_p2m:
case HVMOP_altp2m_set_suppress_ve:
+case HVMOP_altp2m_get_suppress_ve:
case HVMOP_altp2m_set_mem_access:
case HVMOP_altp2m_set_mem_access_multi:
case HVMOP_altp2m_change_gfn:
@@ -4612,18 +4613,36 @@ static int do_altp2m_op(
break;
case HVMOP_altp2m_set_suppress_ve:
-if ( a.u.set_suppress_ve.pad1 || a.u.set_suppress_ve.pad2 )
+if ( a.u.suppress_ve.pad1 || a.u.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;
+bool suppress_ve = a.u.suppress_ve.suppress_ve;
rc = p2m_set_suppress_ve(d, gfn, suppress_ve, altp2m_idx);
}
break;
+case HVMOP_altp2m_get_suppress_ve:
+if ( a.u.suppress_ve.pad1 || a.u.suppress_ve.pad2 )
+rc = -EINVAL;
+else
+{
+gfn_t gfn = _gfn(a.u.suppress_ve.gfn);
+unsigned int altp2m_idx = a.u.suppress_ve.view;
+bool suppress_ve;
+
+rc = p2m_get_suppress_ve(d, gfn, _ve, altp2m_idx);
+if ( !rc )
+{
+a.u.suppress_ve.suppress_ve = suppress_ve;
+rc = __copy_to_guest(arg, , 1) ? -EFAULT : 0;
+}
+}
+break;
+
case HVMOP_altp2m_set_mem_access:
if ( a.u.set_mem_access.pad )
rc = -EINVAL;
diff --git a/xen/arch/x86/mm/mem_access.c b/xen/arch/x86/mm/mem_access.c
index 7cd93ed6b0..c8766835da 100644
--- a/xen/arch/x86/mm/mem_access.c
+++ b/xen/arch/x86/mm/mem_access.c
[Xen-devel] [PATCH v4 1/3] x86/mm: Change default value for suppress #VE in set_mem_access()
From: Vlad Ioan Topan The default value for the "suppress #VE" bit set by set_mem_access() currently depends on whether the call is made from the same domain (the bit is set when called from another domain and cleared if called from the same domain). This patch changes that behavior to inherit the old suppress #VE bit value if it is already set and to set it to 1 otherwise, which is safer and more reliable. Signed-off-by: Vlad Ioan Topan Signed-off-by: Adrian Pop --- xen/arch/x86/mm/mem_access.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/xen/arch/x86/mm/mem_access.c b/xen/arch/x86/mm/mem_access.c index 03a8641569..c469fdcd57 100644 --- a/xen/arch/x86/mm/mem_access.c +++ b/xen/arch/x86/mm/mem_access.c @@ -282,8 +282,7 @@ int p2m_set_altp2m_mem_access(struct domain *d, struct p2m_domain *hp2m, } } -return ap2m->set_entry(ap2m, gfn, mfn, PAGE_ORDER_4K, t, a, - current->domain != d); +return ap2m->set_entry(ap2m, gfn, mfn, PAGE_ORDER_4K, t, a, -1); } static int set_mem_access(struct domain *d, struct p2m_domain *p2m, -- 2.17.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v4 0/3] Add hvmops for setting and getting the suppress #VE bit
As the code stands right now, after DomU has enabled #VE using HVMOP_altp2m_vcpu_enable_notify, all its pages have the #VE suppress bit cleared, generating #VEs for any EPT violation. There is currently no way to change the value of the #VE suppress bit for a page from a domain; it can only be done in Xen internally using ept_set_entry(). Following the discussion from https://lists.xen.org/archives/html/xen-devel/2017-03/msg01312.html these patches introduce new hvmops for setting and getting this bit and thus having control over which pages generate #VE and which VM-Exit. Adrian Pop (2): x86/altp2m: Add a hvmop for setting the suppress #VE bit x86/altp2m: Add a hvmop for querying the suppress #VE bit Vlad Ioan Topan (1): x86/mm: Change default value for suppress #VE in set_mem_access() tools/libxc/include/xenctrl.h | 4 ++ tools/libxc/xc_altp2m.c | 50 xen/arch/x86/hvm/hvm.c | 33 ++ xen/arch/x86/mm/mem_access.c| 103 +++- xen/include/public/hvm/hvm_op.h | 13 xen/include/xen/mem_access.h| 6 ++ 6 files changed, 207 insertions(+), 2 deletions(-) -- 2.17.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v4 2/3] x86/altp2m: Add a hvmop for setting the suppress #VE bit
Introduce a new hvmop, HVMOP_altp2m_set_suppress_ve, which allows a
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
Acked-by: Wei Liu
Acked-by: Tamas K Lengyel
---
Changes in v4:
- fix a deadlock:
If p2m_set_suppress_ve() is called by with invalid pages the code
path wrongly returns without releasing the lock, resulting in a
deadlock.
- remove the privileged domain check
Changes in v3:
- fix indentation (Wei Liu)
- use return values other than EINVAL where appropriate (Ian Beulich)
- remove the irrelevant comments from the
xen_hvm_altp2m_set_suppress_ve struct (Ian Beulich)
- add comment for the suppress_ve field in the struct above (Ian
Beulich)
- remove the typedef and DEFINE_XEN_GUEST_HANDLE for
xen_hvm_altp2m_set_suppress_ve (Ian Beulich)
- use XSM_DM_PRIV check instead of domain->is_privileged (Ian Beulich)
Changes in v2:
- check if #VE has been enabled on the target domain (Tamas K Lengyel)
- check if the cpu has the #VE feature
- make the suppress_ve argument boolean (Jan Beulich)
- initialize only local variables that need initializing (Jan Beulich)
- use fewer local variables (Jan Beulich)
- fix indentation (Jan Beulich)
- remove unnecessary braces (Jan Beulich)
- use gfn_lock() instead of p2m_lock() in the non-altp2m case (Jan
Beulich)
- merge patch #2 and patch #3 (Jan Beulich)
---
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| 55 +
xen/include/public/hvm/hvm_op.h | 11 +++
xen/include/xen/mem_access.h| 3 ++
6 files changed, 109 insertions(+)
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index dd7d8a9724..177d4e6903 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1966,6 +1966,8 @@ int xc_altp2m_destroy_view(xc_interface *handle, uint32_t
domid,
/* Switch all vCPUs of the domain to the specified altp2m view */
int xc_altp2m_switch_to_view(xc_interface *handle, uint32_t domid,
uint16_t view_id);
+int xc_altp2m_set_suppress_ve(xc_interface *handle, uint32_t domid,
+ uint16_t view_id, xen_pfn_t gfn, bool sve);
int xc_altp2m_set_mem_access(xc_interface *handle, uint32_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 ce4a1e4d60..f44d0fffa8 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,
uint32_t domid,
return rc;
}
+int xc_altp2m_set_suppress_ve(xc_interface *handle, uint32_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, uint32_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 67b99af334..5689191a56 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4493,6 +4493,7 @@ static int do_altp2m_op(
case HVMOP_altp2m_create_p2m:
case HVMOP_altp2m_destroy_p2m:
case HVMOP_altp2m_switch_p2m:
+case HVMOP_altp2m_set_suppress_ve:
case HVMOP_altp2m_set_mem_access:
case HVMOP_altp2m_set_mem_access_multi:
case HVMOP_altp2m_change_gfn:
@@ -4610,6 +4611,19 @@ static int do_altp2m_op(
rc = p2m_switch_domain_altp2m_by_id(d, a.u.view.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_set_mem_access:
if ( a.u.set_mem_access.pad )
rc = -EINVAL;
diff --git a/xen/arch/x86/mm/mem_access.c b/xen/
[Xen-devel] [PATCH v3] x86/altp2m: Add a subop for obtaining the mem access of a page
Currently there is a subop for setting the memaccess of a page, but not
for consulting it. The new HVMOP_altp2m_get_mem_access adds this
functionality.
Both altp2m get/set mem access functions use the struct
xen_hvm_altp2m_mem_access which has now dropped the `set' part and has
been renamed from xen_hvm_altp2m_set_mem_access.
Signed-off-by: Adrian Pop
---
Changes in v3:
- remove the unrelated helper function
- simplify the locking in p2m_get_mem_access
Changes in v2:
- use the _p2m_get_mem_access helper from p2m_get_mem_access
- minor Arm adjustments
- move out the addition of a memaccess helper function to a separate
patch in the attempts of making the diff clearer
---
tools/libxc/include/xenctrl.h | 3 +++
tools/libxc/xc_altp2m.c | 33 ++---
xen/arch/arm/mem_access.c | 8 ++--
xen/arch/x86/hvm/hvm.c | 26 ++
xen/arch/x86/mm/mem_access.c| 21 +++--
xen/common/mem_access.c | 2 +-
xen/include/public/hvm/hvm_op.h | 10 ++
xen/include/xen/mem_access.h| 3 ++-
8 files changed, 89 insertions(+), 17 deletions(-)
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index dd7d8a9724..82286c2f52 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1972,6 +1972,9 @@ int xc_altp2m_set_mem_access(xc_interface *handle,
uint32_t domid,
int xc_altp2m_set_mem_access_multi(xc_interface *handle, uint32_t domid,
uint16_t view_id, uint8_t *access,
uint64_t *gfns, uint32_t nr);
+int xc_altp2m_get_mem_access(xc_interface *handle, uint32_t domid,
+ uint16_t view_id, xen_pfn_t gfn,
+ xenmem_access_t *access);
int xc_altp2m_change_gfn(xc_interface *handle, uint32_t domid,
uint16_t view_id, xen_pfn_t old_gfn,
xen_pfn_t new_gfn);
diff --git a/tools/libxc/xc_altp2m.c b/tools/libxc/xc_altp2m.c
index ce4a1e4d60..0ddb18fa2c 100644
--- a/tools/libxc/xc_altp2m.c
+++ b/tools/libxc/xc_altp2m.c
@@ -177,9 +177,9 @@ int xc_altp2m_set_mem_access(xc_interface *handle, uint32_t
domid,
arg->version = HVMOP_ALTP2M_INTERFACE_VERSION;
arg->cmd = HVMOP_altp2m_set_mem_access;
arg->domain = domid;
-arg->u.set_mem_access.view = view_id;
-arg->u.set_mem_access.hvmmem_access = access;
-arg->u.set_mem_access.gfn = gfn;
+arg->u.mem_access.view = view_id;
+arg->u.mem_access.hvmmem_access = access;
+arg->u.mem_access.gfn = gfn;
rc = xencall2(handle->xcall, __HYPERVISOR_hvm_op, HVMOP_altp2m,
HYPERCALL_BUFFER_AS_ARG(arg));
@@ -254,3 +254,30 @@ int xc_altp2m_set_mem_access_multi(xc_interface *xch,
uint32_t domid,
return rc;
}
+
+int xc_altp2m_get_mem_access(xc_interface *handle, uint32_t domid,
+ uint16_t view_id, xen_pfn_t gfn,
+ xenmem_access_t *access)
+{
+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_get_mem_access;
+arg->domain = domid;
+arg->u.mem_access.view = view_id;
+arg->u.mem_access.gfn = gfn;
+
+rc = xencall2(handle->xcall, __HYPERVISOR_hvm_op, HVMOP_altp2m,
+ HYPERCALL_BUFFER_AS_ARG(arg));
+
+if ( !rc )
+*access = arg->u.mem_access.hvmmem_access;
+
+xc_hypercall_buffer_free(handle, arg);
+return rc;
+}
diff --git a/xen/arch/arm/mem_access.c b/xen/arch/arm/mem_access.c
index ba4ec780fd..178bc1a6c1 100644
--- a/xen/arch/arm/mem_access.c
+++ b/xen/arch/arm/mem_access.c
@@ -236,7 +236,7 @@ bool p2m_mem_access_check(paddr_t gpa, vaddr_t gla, const
struct npfec npfec)
if ( !p2m->mem_access_enabled )
return true;
-rc = p2m_get_mem_access(v->domain, gaddr_to_gfn(gpa), );
+rc = p2m_get_mem_access(v->domain, gaddr_to_gfn(gpa), , 0);
if ( rc )
return true;
@@ -441,11 +441,15 @@ long p2m_set_mem_access_multi(struct domain *d,
}
int p2m_get_mem_access(struct domain *d, gfn_t gfn,
- xenmem_access_t *access)
+ xenmem_access_t *access, unsigned int altp2m_idx)
{
int ret;
struct p2m_domain *p2m = p2m_get_hostp2m(d);
+/* altp2m is not yet implemented on Arm. The altp2m_idx should be 0. */
+if ( altp2m_idx )
+return -EINVAL;
+
p2m_read_lock(p2m);
ret = __p2m_get_mem_access(d, gfn, access);
p2m_read_unlock(p2m);
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 67b99af334..49f4afabac 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4495,6 +4495,7 @@ static int do_altp2m_op(
case HV
Re: [Xen-devel] [PATCH v2 2/2] x86/altp2m: Add a subop for obtaining the memaccess of a page
On Mon, Jul 09, 2018 at 02:35:30AM -0600, Jan Beulich wrote: > >>> On 05.07.18 at 12:44, wrote: > > Both altp2m get/set memaccess functions use the struct > > xen_hvm_altp2m_mem_access which has now dropped the `set' part and has > > been renamed from xen_hvm_altp2m_set_mem_access. > > > > Signed-off-by: Adrian Pop > > Looking through this patch - what's the point of patch 1? There's no > further user being introduced here. It's not entirely necessary. It just cleans up _p2m_get_mem_access() slightly, which would be the only user of the new function. I could drop patch 1 and if it's deemed desirable I could send it separately. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 1/2] x86/mem_access: Add a helper function for memaccess conversion
On Thu, Jul 05, 2018 at 10:53:22AM -0600, Tamas K Lengyel wrote: > On Thu, Jul 5, 2018 at 4:44 AM Adrian Pop wrote: > > @@ -72,11 +83,7 @@ static int _p2m_get_mem_access(struct p2m_domain *p2m, > > gfn_t gfn, > > if ( mfn_eq(mfn, INVALID_MFN) ) > > return -ESRCH; > > > > -if ( (unsigned int)a >= ARRAY_SIZE(memaccess) ) > > -return -ERANGE; > > - > > -*access = memaccess[a]; > > -return 0; > > +return access_to_xenmem_access(p2m, a, access); > > Shouldn't this be p2m_access_to_xenmem_access? Did you compile check > this patch before sending? Oh, that's right! It seems I've mistakenly used an intermediary version of the patch when testing unfortunately. Thank you! ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 1/2] x86/mem_access: Add a helper function for memaccess conversion
The p2m_access_to_xenmem_access() converts a p2m_access_t to a
xenmem_access_t. It is complementary to xenmem_access_to_p2m_access().
It is currently only used by _p2m_get_mem_access().
Signed-off-by: Adrian Pop
---
xen/arch/x86/mm/mem_access.c | 41 +---
1 file changed, 24 insertions(+), 17 deletions(-)
diff --git a/xen/arch/x86/mm/mem_access.c b/xen/arch/x86/mm/mem_access.c
index c0cd0174cf..e75ff0c735 100644
--- a/xen/arch/x86/mm/mem_access.c
+++ b/xen/arch/x86/mm/mem_access.c
@@ -33,16 +33,12 @@
#include "mm-locks.h"
/*
- * Get access type for a gfn.
- * If gfn == INVALID_GFN, gets the default access type.
+ * Convert a p2m_access_t to a xenmem_access_t.
*/
-static int _p2m_get_mem_access(struct p2m_domain *p2m, gfn_t gfn,
- xenmem_access_t *access)
+static int p2m_access_to_xenmem_access(struct p2m_domain *p2m,
+ p2m_access_t paccess,
+ xenmem_access_t *xaccess)
{
-p2m_type_t t;
-p2m_access_t a;
-mfn_t mfn;
-
static const xenmem_access_t memaccess[] = {
#define ACCESS(ac) [p2m_access_##ac] = XENMEM_access_##ac
ACCESS(n),
@@ -58,12 +54,27 @@ static int _p2m_get_mem_access(struct p2m_domain *p2m,
gfn_t gfn,
#undef ACCESS
};
+if ( (unsigned int)paccess >= ARRAY_SIZE(memaccess) )
+return -ERANGE;
+
+*xaccess = memaccess[paccess];
+return 0;
+}
+
+/*
+ * Get access type for a gfn.
+ * If gfn == INVALID_GFN, gets the default access type.
+ */
+static int _p2m_get_mem_access(struct p2m_domain *p2m, gfn_t gfn,
+ xenmem_access_t *access)
+{
+p2m_type_t t;
+p2m_access_t a;
+mfn_t mfn;
+
/* If request to get default access. */
if ( gfn_eq(gfn, INVALID_GFN) )
-{
-*access = memaccess[p2m->default_access];
-return 0;
-}
+return p2m_access_to_xenmem_access(p2m, p2m->default_access, access);
gfn_lock(p2m, gfn, 0);
mfn = p2m->get_entry(p2m, gfn, , , 0, NULL, NULL);
@@ -72,11 +83,7 @@ static int _p2m_get_mem_access(struct p2m_domain *p2m, gfn_t
gfn,
if ( mfn_eq(mfn, INVALID_MFN) )
return -ESRCH;
-if ( (unsigned int)a >= ARRAY_SIZE(memaccess) )
-return -ERANGE;
-
-*access = memaccess[a];
-return 0;
+return access_to_xenmem_access(p2m, a, access);
}
bool p2m_mem_access_emulate_check(struct vcpu *v,
--
2.17.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 0/2] Add an altp2m subop for obtaining the page mem access
Currently there is a subop for setting the memaccess of a page, but not for consulting it. The new HVMOP_altp2m_get_mem_access adds this functionality. Changes in v2: - use the _p2m_get_mem_access helper from p2m_get_mem_access - minor Arm adjustments - move out the addition of a memaccess helper function to a separate patch in the attempts of making the diff clearer Adrian Pop (2): x86/mem_access: Add a helper function for memaccess conversion x86/altp2m: Add a subop for obtaining the mem access of a page tools/libxc/include/xenctrl.h | 3 ++ tools/libxc/xc_altp2m.c | 33 +-- xen/arch/arm/mem_access.c | 8 +++- xen/arch/x86/hvm/hvm.c | 26 ++-- xen/arch/x86/mm/mem_access.c| 73 +++-- xen/common/mem_access.c | 2 +- xen/include/public/hvm/hvm_op.h | 10 +++-- xen/include/xen/mem_access.h| 3 +- 8 files changed, 121 insertions(+), 37 deletions(-) -- 2.17.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/altp2m: Add a subop for obtaining the mem access of a page
On Wed, Jul 04, 2018 at 03:20:45PM +0300, Adrian POP wrote: > On Fri, Jun 29, 2018 at 09:38:58AM -0600, Jan Beulich wrote: > > > --- a/xen/arch/x86/mm/mem_access.c > > > +++ b/xen/arch/x86/mm/mem_access.c > > > @@ -32,17 +32,10 @@ > > > > > > #include "mm-locks.h" > > > > > > -/* > > > - * Get access type for a gfn. > > > - * If gfn == INVALID_GFN, gets the default access type. > > > - */ > > > -static int _p2m_get_mem_access(struct p2m_domain *p2m, gfn_t gfn, > > > - xenmem_access_t *access) > > > +static int p2m_access_to_xenmem_access(struct p2m_domain *p2m, > > > > This is not even p2m code - why the p2m_ prefix? > > There's indeed no reason for this to have the p2m_ prefix. Will remove > it. Oh I remember now. The p2m_ prefix was added there because the helper function converts a p2m_access_t to a xenmem_access_t, so p2m_access_to_xenmem_access(). Took the naming from the complementary function, xenmem_access_to_p2m_access(). I could add a comment to clarify this. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/altp2m: Add a subop for obtaining the mem access of a page
Hello, On Mon, Jul 02, 2018 at 12:01:01PM +0100, Julien Grall wrote: > Hi, > > On 06/28/2018 02:00 PM, Adrian Pop wrote: > > diff --git a/xen/arch/arm/mem_access.c b/xen/arch/arm/mem_access.c > > index ae2686ffa2..ba9e50e7f6 100644 > > --- a/xen/arch/arm/mem_access.c > > +++ b/xen/arch/arm/mem_access.c > > @@ -236,7 +236,7 @@ bool p2m_mem_access_check(paddr_t gpa, vaddr_t gla, > > const struct npfec npfec) > > if ( !p2m->mem_access_enabled ) > > return true; > > -rc = p2m_get_mem_access(v->domain, gaddr_to_gfn(gpa), ); > > +rc = p2m_get_mem_access(v->domain, gaddr_to_gfn(gpa), , 0); > > if ( rc ) > > return true; > > @@ -440,8 +440,9 @@ long p2m_set_mem_access_multi(struct domain *d, > > return -EOPNOTSUPP; > > } > > +/* The altp2m_idx argument is not used on ARM. */ > > s/ARM/Arm/ please. > > Also, I guess altp2m_idx should always be 0 until altp2m is implemented for > Arm. If so, I would like to see a check on the variable and return -EINVAL > on error. All right. Thank you! ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/altp2m: Add a subop for obtaining the mem access of a page
On Fri, Jun 29, 2018 at 09:38:58AM -0600, Jan Beulich wrote:
> >>> On 28.06.18 at 15:00, wrote:
> > @@ -4666,6 +4667,23 @@ static int do_altp2m_op(
> > }
> > break;
> >
> > +case HVMOP_altp2m_get_mem_access:
> > +if ( a.u.mem_access.pad )
> > +rc = -EINVAL;
> > +else
> > +{
> > +xenmem_access_t access;
> > +
> > +rc = p2m_get_mem_access(d, _gfn(a.u.mem_access.gfn), ,
> > +a.u.mem_access.view);
> > +if ( !rc )
> > +{
> > +a.u.mem_access.hvmmem_access = access;
> > +rc = __copy_to_guest(arg, , 1) ? -EFAULT : 0;
>
> __copy_field_to_guest()? Or wait, no, the function argument is still a
> handle of void.
I'll then leave the __copy_to_guest() in place as it is for now.
> And then - here we are again: Is it reasonable to permit a domain inquiring
> for itself?
Yes, this is a questionable aspect of altp2m that warrants further
discussion. I'll resend a version with the other problems addressed to
have them out of the way.
> > --- a/xen/arch/x86/mm/mem_access.c
> > +++ b/xen/arch/x86/mm/mem_access.c
> > @@ -32,17 +32,10 @@
> >
> > #include "mm-locks.h"
> >
> > -/*
> > - * Get access type for a gfn.
> > - * If gfn == INVALID_GFN, gets the default access type.
> > - */
> > -static int _p2m_get_mem_access(struct p2m_domain *p2m, gfn_t gfn,
> > - xenmem_access_t *access)
> > +static int p2m_access_to_xenmem_access(struct p2m_domain *p2m,
>
> This is not even p2m code - why the p2m_ prefix?
There's indeed no reason for this to have the p2m_ prefix. Will remove
it.
> > @@ -458,11 +462,41 @@ long p2m_set_mem_access_multi(struct domain *d,
> > return rc;
> > }
> >
> > -int p2m_get_mem_access(struct domain *d, gfn_t gfn, xenmem_access_t
> > *access)
> > +int p2m_get_mem_access(struct domain *d, gfn_t gfn, xenmem_access_t
> > *access,
> > + unsigned int altp2m_idx)
> > {
> > -struct p2m_domain *p2m = p2m_get_hostp2m(d);
> > +struct p2m_domain *host_p2m = p2m_get_hostp2m(d);
> > +struct p2m_domain *ap2m = NULL;
> > +struct p2m_domain *p2m;
> > +p2m_access_t a;
> > +p2m_type_t t;
> > +mfn_t mfn;
> > +
> > +if ( altp2m_idx )
> > +{
> > +if ( altp2m_idx >= MAX_ALTP2M ||
> > + d->arch.altp2m_eptp[altp2m_idx] == mfn_x(INVALID_MFN) )
> > +return -EINVAL;
> > +
> > +p2m = ap2m = d->arch.altp2m_p2m[altp2m_idx];
> > +}
> > +else
> > +p2m = host_p2m;
> > +
> > +p2m_read_lock(host_p2m);
> > +if (ap2m)
>
> Missing blanks (also below).
All right.
> > +p2m_read_lock(ap2m);
> > +
> > +mfn = p2m->get_entry(p2m, gfn, , , 0, NULL, NULL);
> > +
> > +if (ap2m)
> > +p2m_read_unlock(ap2m);
> > +p2m_read_unlock(host_p2m);
> > +
> > +if ( mfn_eq(mfn, INVALID_MFN) )
> > +return -ESRCH;
> >
> > -return _p2m_get_mem_access(p2m, gfn, access);
> > +return p2m_access_to_xenmem_access(p2m, a, access);
>
> I'm confused: Why does p2m_get_mem_access() not use its helper
> function p2m_get_mem_access() (which you retain) anymore? I
> guess the description is a little too terse. It might also have helped
> if some of the mechanical preparation steps were broken out.
Ok, I'll fix this and attempt to provide more information with the
commit message.
Thank you!
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] x86/altp2m: Add a subop for obtaining the mem access of a page
A new function, p2m_access_to_xenmem_access(), has been added to avoid
code duplication. The existing _p2m_get_mem_access() now uses this
function as does p2m_get_mem_access().
---
tools/libxc/include/xenctrl.h | 3 ++
tools/libxc/xc_altp2m.c | 33 --
xen/arch/arm/mem_access.c | 5 ++-
xen/arch/x86/hvm/hvm.c | 26 +--
xen/arch/x86/mm/mem_access.c| 80 +++--
xen/common/mem_access.c | 2 +-
xen/include/public/hvm/hvm_op.h | 10 +++--
xen/include/xen/mem_access.h| 3 +-
8 files changed, 124 insertions(+), 38 deletions(-)
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 408fa1c6a4..f326bcbb4a 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1964,6 +1964,9 @@ int xc_altp2m_set_mem_access(xc_interface *handle,
uint32_t domid,
int xc_altp2m_set_mem_access_multi(xc_interface *handle, uint32_t domid,
uint16_t view_id, uint8_t *access,
uint64_t *gfns, uint32_t nr);
+int xc_altp2m_get_mem_access(xc_interface *handle, uint32_t domid,
+ uint16_t view_id, xen_pfn_t gfn,
+ xenmem_access_t *access);
int xc_altp2m_change_gfn(xc_interface *handle, uint32_t domid,
uint16_t view_id, xen_pfn_t old_gfn,
xen_pfn_t new_gfn);
diff --git a/tools/libxc/xc_altp2m.c b/tools/libxc/xc_altp2m.c
index ce4a1e4d60..0ddb18fa2c 100644
--- a/tools/libxc/xc_altp2m.c
+++ b/tools/libxc/xc_altp2m.c
@@ -177,9 +177,9 @@ int xc_altp2m_set_mem_access(xc_interface *handle, uint32_t
domid,
arg->version = HVMOP_ALTP2M_INTERFACE_VERSION;
arg->cmd = HVMOP_altp2m_set_mem_access;
arg->domain = domid;
-arg->u.set_mem_access.view = view_id;
-arg->u.set_mem_access.hvmmem_access = access;
-arg->u.set_mem_access.gfn = gfn;
+arg->u.mem_access.view = view_id;
+arg->u.mem_access.hvmmem_access = access;
+arg->u.mem_access.gfn = gfn;
rc = xencall2(handle->xcall, __HYPERVISOR_hvm_op, HVMOP_altp2m,
HYPERCALL_BUFFER_AS_ARG(arg));
@@ -254,3 +254,30 @@ int xc_altp2m_set_mem_access_multi(xc_interface *xch,
uint32_t domid,
return rc;
}
+
+int xc_altp2m_get_mem_access(xc_interface *handle, uint32_t domid,
+ uint16_t view_id, xen_pfn_t gfn,
+ xenmem_access_t *access)
+{
+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_get_mem_access;
+arg->domain = domid;
+arg->u.mem_access.view = view_id;
+arg->u.mem_access.gfn = gfn;
+
+rc = xencall2(handle->xcall, __HYPERVISOR_hvm_op, HVMOP_altp2m,
+ HYPERCALL_BUFFER_AS_ARG(arg));
+
+if ( !rc )
+*access = arg->u.mem_access.hvmmem_access;
+
+xc_hypercall_buffer_free(handle, arg);
+return rc;
+}
diff --git a/xen/arch/arm/mem_access.c b/xen/arch/arm/mem_access.c
index ae2686ffa2..ba9e50e7f6 100644
--- a/xen/arch/arm/mem_access.c
+++ b/xen/arch/arm/mem_access.c
@@ -236,7 +236,7 @@ bool p2m_mem_access_check(paddr_t gpa, vaddr_t gla, const
struct npfec npfec)
if ( !p2m->mem_access_enabled )
return true;
-rc = p2m_get_mem_access(v->domain, gaddr_to_gfn(gpa), );
+rc = p2m_get_mem_access(v->domain, gaddr_to_gfn(gpa), , 0);
if ( rc )
return true;
@@ -440,8 +440,9 @@ long p2m_set_mem_access_multi(struct domain *d,
return -EOPNOTSUPP;
}
+/* The altp2m_idx argument is not used on ARM. */
int p2m_get_mem_access(struct domain *d, gfn_t gfn,
- xenmem_access_t *access)
+ xenmem_access_t *access, unsigned int altp2m_idx)
{
int ret;
struct p2m_domain *p2m = p2m_get_hostp2m(d);
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index c23983cdff..28e1719751 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4509,6 +4509,7 @@ static int do_altp2m_op(
case HVMOP_altp2m_switch_p2m:
case HVMOP_altp2m_set_mem_access:
case HVMOP_altp2m_set_mem_access_multi:
+case HVMOP_altp2m_get_mem_access:
case HVMOP_altp2m_change_gfn:
break;
@@ -4625,12 +4626,12 @@ static int do_altp2m_op(
break;
case HVMOP_altp2m_set_mem_access:
-if ( a.u.set_mem_access.pad )
+if ( a.u.mem_access.pad )
rc = -EINVAL;
else
-rc = p2m_set_mem_access(d, _gfn(a.u.set_mem_access.gfn), 1, 0, 0,
-a.u.set_mem_access.hvmmem_access,
-a.u.set_mem_access.view);
+rc = p2m_set_mem_access(d, _gfn(a.u.mem_access.gfn), 1, 0, 0,
+
