Hi Sergej,
On 06/01/2017 04:18 PM, Sergej Proskurin wrote:
The function p2m_mem_access_check_and_get_page in mem_access.c translates a gva
to an ipa by means of the hardware functionality of the ARM architecture. This
is implemented in the function gva_to_ipa. If mem_access is active,
hardware-based gva to ipa translation might fail, as gva_to_ipa uses the
guest's translation tables, access to which might be restricted by the active
VTTBR. To address this issue, in this commit we add a software-based
guest-page-table walk, which will be used by the function
p2m_mem_access_check_and_get_page perform the gva to ipa translation in
software in one of the following commits.
Note: This function p2m_walk_gpt assumes that the domain, the gva of
which is to be translated, is running on the currently active vCPU. To
walk the guest's page table on a different vCPU, the following registers
would need to be loaded: TCR_EL1, TTBR0_EL1, TTBR1_EL1, and SCTLR_EL1.
Signed-off-by: Sergej Proskurin <prosku...@sec.in.tum.de>
---
Cc: Stefano Stabellini <sstabell...@kernel.org>
Cc: Julien Grall <julien.gr...@arm.com>
---
v2: Rename p2m_gva_to_ipa to p2m_walk_gpt and move it to p2m.c.
Move the functionality responsible for walking long-descriptor based
translation tables out of the function p2m_walk_gpt. Also move out the
long-descriptor based translation out of this commit.
Change function parameters in order to return access access rights to a
requested gva.
Cosmetic fixes.
---
xen/arch/arm/p2m.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++
I know I suggested to move in p2m.c. Looking at the diff stat, this will
increase quite a lot p2m.c which is already big.
How about introducing a file guest_walk.c which contain the new functions?
xen/include/asm-arm/p2m.h | 6 +++++
2 files changed, 64 insertions(+)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 752e948070..0337d83581 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1548,6 +1548,64 @@ void __init setup_virt_paging(void)
}
/*
+ * The function __p2m_walk_gpt_sd translates a given GVA into an IPA using the
+ * short-descriptor translation table format in software. This function assumes
+ * that the domain is running on the currently active vCPU. To walk the guest's
+ * page table on a different vCPU, the following registers would need to be
+ * loaded: TCR_EL1, TTBR0_EL1, TTBR1_EL1, and SCTLR_EL1.
+ */
+static int __p2m_walk_gpt_sd(struct p2m_domain *p2m,
The __ are not necessary here. You don't use the name p2m_walk_gpt_sd
anywhere.
+ vaddr_t gva, paddr_t *ipa,
+ unsigned int *perm_ro)
I am a bit confused with perm_ro. Will you only return 0/1? If so it
should be a bool.
But we likely want to know more permission such as the execution bit...
+{
+ /* Not implemented yet. */
+ return -EFAULT;
+}
+
+/*
+ * The function __p2m_walk_gpt_ld translates a given GVA into an IPA using the
+ * long-descriptor translation table format in software. This function assumes
+ * that the domain is running on the currently active vCPU. To walk the guest's
+ * page table on a different vCPU, the following registers would need to be
+ * loaded: TCR_EL1, TTBR0_EL1, TTBR1_EL1, and SCTLR_EL1.
+ */
+static int __p2m_walk_gpt_ld(struct p2m_domain *p2m,
Ditto.
+ vaddr_t gva, paddr_t *ipa,
+ unsigned int *perm_ro)
+{
+ /* Not implemented yet. */
+ return -EFAULT;
+}
+
+int p2m_walk_gpt(struct p2m_domain *p2m, vaddr_t gva,
So you mix 2 things, p2m and gpt. P2M is used to refer to stage-2 page
table. Whilst gpt stands I guess for guest page table.
The name gpt is not very used in Xen and would prefer a clearer name
such as the x86 one "guest_walk_tables".
+ paddr_t *ipa, unsigned int *perm_ro)
+{
+ uint32_t sctlr = READ_SYSREG(SCTLR_EL1);
+ register_t tcr = READ_SYSREG(TCR_EL1);
+#ifdef CONFIG_ARM_64
+ struct domain *d = p2m->domain;
+#endif
The only place use *d is in the is_32bit_domain, so no need to add some
#ifdef and define the variable.
+
+ /* If the MMU is disabled, there is no need to translate the gva. */
+ if ( !(sctlr & SCTLR_M) )
+ {
+ *ipa = gva;
Why *perm_ro is not set here?
+
+ return 0;
+ }
+
+#ifdef CONFIG_ARM_64
+ if ( is_32bit_domain(d) )
+#endif
is_32bit_domain exists for 32-bit Xen. So not need to have this #ifdef.
+ {
+ if ( !(tcr & TTBCR_EAE) )
+ return __p2m_walk_gpt_sd(p2m, gva, ipa, perm_ro);
+ }
+
+ return __p2m_walk_gpt_ld(p2m, gva, ipa, perm_ro);
+}
+
+/*
* Local variables:
* mode: C
* c-file-style: "BSD"
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 18c57f936e..cc9f4bf225 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -264,6 +264,12 @@ void guest_physmap_remove_page(struct domain *d,
mfn_t gfn_to_mfn(struct domain *d, gfn_t gfn);
+/* Walk the guest's page tables in software. */
+int p2m_walk_gpt(struct p2m_domain *p2m,
+ vaddr_t gva,
+ paddr_t *ipa,
+ unsigned int *perm_ro);
+
/*
* Populate-on-demand
*/
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
