[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH v2 21/25] arm/altp2m: Add HVMOP_altp2m_change_gfn.
Hello Sergej,
On 01/08/16 18:10, Sergej Proskurin wrote:
This commit adds the functionality to change mfn mappings for specified
gfn's in altp2m views. This mechanism can be used within the context of
VMI, e.g., to establish stealthy debugging.
Signed-off-by: Sergej Proskurin <proskurin@xxxxxxxxxxxxx>
---
Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
Cc: Julien Grall <julien.grall@xxxxxxx>
---
xen/arch/arm/altp2m.c | 116 +++++++++++++++++++++++++++++++++++++++++++
xen/arch/arm/hvm.c | 7 ++-
xen/arch/arm/p2m.c | 14 ++++++
xen/include/asm-arm/altp2m.h | 6 +++
xen/include/asm-arm/p2m.h | 4 ++
5 files changed, 146 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/altp2m.c b/xen/arch/arm/altp2m.c
index 78fc1d5..db86c14 100644
--- a/xen/arch/arm/altp2m.c
+++ b/xen/arch/arm/altp2m.c
@@ -294,6 +294,122 @@ out:
altp2m_unlock(d);
}
+int altp2m_change_gfn(struct domain *d,
+ unsigned int idx,
+ gfn_t old_gfn,
+ gfn_t new_gfn)
+{
+ struct p2m_domain *hp2m, *ap2m;
+ paddr_t old_gpa = pfn_to_paddr(gfn_x(old_gfn));
+ mfn_t mfn;
+ xenmem_access_t xma;
+ p2m_type_t p2mt;
+ unsigned int level;
+ int rc = -EINVAL;
+
+ static const p2m_access_t memaccess[] = {
+#define ACCESS(ac) [XENMEM_access_##ac] = p2m_access_##ac
+ ACCESS(n),
+ ACCESS(r),
+ ACCESS(w),
+ ACCESS(rw),
+ ACCESS(x),
+ ACCESS(rx),
+ ACCESS(wx),
+ ACCESS(rwx),
+ ACCESS(rx2rw),
+ ACCESS(n2rwx),
+#undef ACCESS
+ };
+
+ if ( idx >= MAX_ALTP2M || d->arch.altp2m_vttbr[idx] == INVALID_VTTBR )
The second check is not safe. Another operation could destroy the altp2m
at the same time, but because of memory ordering this thread may still
see altp2m_vttbr as valid.
+ return rc;
+
+ hp2m = p2m_get_hostp2m(d);
+ ap2m = d->arch.altp2m_p2m[idx];
+
+ altp2m_lock(d);
+
+ /*
+ * Flip mem_access_enabled to true when a permission is set, as to prevent
+ * allocating or inserting super-pages.
+ */
+ ap2m->mem_access_enabled = true;
Can you give more details about why you need this?
+
+ mfn = p2m_lookup_attr(ap2m, old_gfn, &p2mt, &level, NULL, NULL);
+
+ /* Check whether the page needs to be reset. */
+ if ( gfn_eq(new_gfn, INVALID_GFN) )
+ {
+ /* If mfn is mapped by old_gpa, remove old_gpa from the altp2m table.
*/
+ if ( !mfn_eq(mfn, INVALID_MFN) )
+ {
+ rc = remove_altp2m_entry(d, ap2m, old_gpa,
pfn_to_paddr(mfn_x(mfn)), level);
remove_altp2m_entry should take a gfn and mfn in parameter and not an
address. The latter is a call for misusage of the API.
+ if ( rc )
+ {
+ rc = -EINVAL;
+ goto out;
+ }
+ }
+
+ rc = 0;
+ goto out;
+ }
+
+ /* Check host p2m if no valid entry in altp2m present. */
+ if ( mfn_eq(mfn, INVALID_MFN) )
+ {
+ mfn = p2m_lookup_attr(hp2m, old_gfn, &p2mt, &level, NULL, &xma);
+ if ( mfn_eq(mfn, INVALID_MFN) || (p2mt != p2m_ram_rw) )
Please add a comment to explain why the second check.
+ {
+ rc = -EINVAL;
+ goto out;
+ }
+
+ /* If this is a superpage, copy that first. */
+ if ( level != 3 )
+ {
+ rc = modify_altp2m_entry(d, ap2m, old_gpa,
pfn_to_paddr(mfn_x(mfn)),
+ level, p2mt, memaccess[xma]);
+ if ( rc )
+ {
+ rc = -EINVAL;
+ goto out;
+ }
+ }
+ }
+
+ mfn = p2m_lookup_attr(ap2m, new_gfn, &p2mt, &level, NULL, &xma);
+
+ /* If new_gfn is not part of altp2m, get the mapping information from hp2m
*/
+ if ( mfn_eq(mfn, INVALID_MFN) )
+ mfn = p2m_lookup_attr(hp2m, new_gfn, &p2mt, &level, NULL, &xma);
+
+ if ( mfn_eq(mfn, INVALID_MFN) || (p2mt != p2m_ram_rw) )
Please add a comment to explain why the second check.
+ {
+ rc = -EINVAL;
+ goto out;
+ }
+
+ /* Set mem access attributes - currently supporting only one (4K) page. */
+ level = 3;
+ rc = modify_altp2m_entry(d, ap2m, old_gpa, pfn_to_paddr(mfn_x(mfn)),
modify_altp2m_entry should take a gfn and mfn in parameter and not an
address. The latter is a call for misusage of the API.
+ level, p2mt, memaccess[xma]);
+ if ( rc )
+ {
+ rc = -EINVAL;
+ goto out;
+ }
+
+ rc = 0;
+
+out:
+ altp2m_unlock(d);
+
+ return rc;
+}
+
+
static void altp2m_vcpu_reset(struct vcpu *v)
{
struct altp2mvcpu *av = &vcpu_altp2m(v);
diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c
index 00a244a..38b32de 100644
--- a/xen/arch/arm/hvm.c
+++ b/xen/arch/arm/hvm.c
@@ -142,7 +142,12 @@ static int do_altp2m_op(XEN_GUEST_HANDLE_PARAM(void) arg)
break;
case HVMOP_altp2m_change_gfn:
- rc = -EOPNOTSUPP;
+ if ( a.u.change_gfn.pad1 || a.u.change_gfn.pad2 )
+ rc = -EINVAL;
+ else
+ rc = altp2m_change_gfn(d, a.u.change_gfn.view,
+ _gfn(a.u.change_gfn.old_gfn),
+ _gfn(a.u.change_gfn.new_gfn));
break;
}
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index bee8be7..2f4751b 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1321,6 +1321,20 @@ void guest_physmap_remove_page(struct domain *d,
p2m_remove_mapping(d, p2m_get_hostp2m(d), gfn, (1 << page_order), mfn);
}
+int remove_altp2m_entry(struct domain *d, struct p2m_domain *ap2m,
+ paddr_t gpa, paddr_t maddr, unsigned int level)
The interface should take mfn_t and gfn_t in parameter and not address.
+{
+ paddr_t size = level_sizes[level];
+ paddr_t mask = level_masks[level];
+ gfn_t gfn = _gfn(paddr_to_pfn(gpa & mask));
+ mfn_t mfn = _mfn(paddr_to_pfn(maddr & mask));
+ unsigned long nr = paddr_to_pfn(size);
+
+ ASSERT(p2m_is_altp2m(ap2m));
+
+ return p2m_remove_mapping(d, ap2m, gfn, nr, mfn);
+}
+
int modify_altp2m_entry(struct domain *d, struct p2m_domain *ap2m,
paddr_t gpa, paddr_t maddr, unsigned int level,
p2m_type_t t, p2m_access_t a)
diff --git a/xen/include/asm-arm/altp2m.h b/xen/include/asm-arm/altp2m.h
index 8bfbc6a..64fbff7 100644
--- a/xen/include/asm-arm/altp2m.h
+++ b/xen/include/asm-arm/altp2m.h
@@ -99,4 +99,10 @@ void altp2m_propagate_change(struct domain *d,
p2m_type_t p2mt,
p2m_access_t p2ma);
+/* Change a gfn->mfn mapping */
+int altp2m_change_gfn(struct domain *d,
+ unsigned int idx,
+ gfn_t old_gfn,
+ gfn_t new_gfn);
+
#endif /* __ASM_ARM_ALTP2M_H */
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 16e33ca..8433d66 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -182,6 +182,10 @@ mfn_t p2m_lookup_attr(struct p2m_domain *p2m, gfn_t gfn,
p2m_type_t *t,
unsigned int *level, unsigned int *mattr,
xenmem_access_t *xma);
+/* Remove an altp2m view's entry. */
+int remove_altp2m_entry(struct domain *d, struct p2m_domain *p2m,
+ paddr_t gpa, paddr_t maddr, unsigned int level);
+
/* Modify an altp2m view's entry or its attributes. */
int modify_altp2m_entry(struct domain *d, struct p2m_domain *p2m,
paddr_t gpa, paddr_t maddr, unsigned int level,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
