From: Julien Grall <julien.grall@arm.com>
To: Sergej Proskurin <proskurin@sec.in.tum.de>,
xen-devel@lists.xenproject.org
Cc: Stefano Stabellini <sstabellini@kernel.org>
Subject: Re: [RFC PATCH v2 5/8] arm/mem_access: Add software guest-page-table walk
Date: Fri, 2 Jun 2017 10:02:20 +0100 [thread overview]
Message-ID: <3a019bbf-6ebe-85b0-4d97-90ccb6295035@arm.com> (raw)
In-Reply-To: <20170601151906.10213-6-proskurin@sec.in.tum.de>
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 <proskurin@sec.in.tum.de>
> ---
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Julien Grall <julien.grall@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
next prev parent reply other threads:[~2017-06-02 9:02 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-06-01 15:18 [RFC PATCH v2 0/8] arm/mem_access: Walk guest page tables in SW if mem_access is active Sergej Proskurin
2017-06-01 15:18 ` [RFC PATCH v2 1/8] arm/mem_access: Add (TCR_|TTBCR_)* defines Sergej Proskurin
2017-06-02 7:31 ` Julien Grall
2017-06-07 14:56 ` Sergej Proskurin
2017-06-07 15:07 ` Julien Grall
2017-06-07 15:11 ` Julien Grall
2017-06-01 15:18 ` [RFC PATCH v2 2/8] arm/mem_access: Add defines holding the width of 32/64bit regs Sergej Proskurin
2017-06-01 15:18 ` [RFC PATCH v2 3/8] arm/mem_access: Add defines supporting PTs with varying page sizes Sergej Proskurin
2017-06-02 8:27 ` Julien Grall
2017-06-01 15:18 ` [RFC PATCH v2 4/8] arm/mem_access: Add short-descriptor pte typedefs Sergej Proskurin
2017-06-02 8:50 ` Julien Grall
2017-06-01 15:18 ` [RFC PATCH v2 5/8] arm/mem_access: Add software guest-page-table walk Sergej Proskurin
2017-06-02 9:02 ` Julien Grall [this message]
2017-06-08 12:43 ` Sergej Proskurin
2017-06-09 8:19 ` Julien Grall
2017-06-01 15:18 ` [RFC PATCH v2 6/8] arm/mem_access: Add long-descriptor based gpt Sergej Proskurin
2017-06-02 12:55 ` Julien Grall
2017-06-09 11:50 ` Sergej Proskurin
2017-06-09 12:39 ` Julien Grall
2017-06-12 10:12 ` Sergej Proskurin
2017-06-12 10:44 ` Julien Grall
2017-06-12 12:31 ` Sergej Proskurin
2017-06-01 15:18 ` [RFC PATCH v2 7/8] arm/mem_access: Add short-descriptor " Sergej Proskurin
2017-06-02 15:11 ` Julien Grall
2017-06-01 15:18 ` [RFC PATCH v2 8/8] arm/mem_access: Walk the guest's pt in software Sergej Proskurin
2017-06-01 15:18 ` [PATCH 1/8] arm/mem_access: Add (TCR_|TTBCR_)* defines Sergej Proskurin
2017-06-02 15:13 ` Julien Grall
2017-06-03 8:56 ` Sergej Proskurin
2017-06-01 15:19 ` [PATCH 2/8] arm/mem_access: Add defines holding the width of 32/64bit regs Sergej Proskurin
2017-06-01 15:19 ` [PATCH 3/8] arm/mem_access: Add defines supporting PTs with varying page sizes Sergej Proskurin
2017-06-01 15:19 ` [PATCH 4/8] arm/mem_access: Add short-descriptor pte typedefs Sergej Proskurin
2017-06-01 15:19 ` [PATCH 5/8] arm/mem_access: Add software guest-page-table walk Sergej Proskurin
2017-06-01 15:19 ` [PATCH 6/8] arm/mem_access: Add long-descriptor based gpt Sergej Proskurin
2017-06-01 15:19 ` [PATCH 7/8] arm/mem_access: Add short-descriptor " Sergej Proskurin
2017-06-01 15:19 ` [PATCH 8/8] arm/mem_access: Walk the guest's pt in software Sergej Proskurin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3a019bbf-6ebe-85b0-4d97-90ccb6295035@arm.com \
--to=julien.grall@arm.com \
--cc=proskurin@sec.in.tum.de \
--cc=sstabellini@kernel.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).