Hi Julien,
> -----Original Message-----
> From: Xen-devel <xen-devel-boun...@lists.xenproject.org> On Behalf Of Julien
> Grall
> Sent: 2022年2月21日 18:22
> To: xen-devel@lists.xenproject.org
> Cc: jul...@xen.org; Julien Grall <jgr...@amazon.com>; Stefano Stabellini
> <sstabell...@kernel.org>; Bertrand Marquis <bertrand.marq...@arm.com>;
> Volodymyr Babchuk <volodymyr_babc...@epam.com>
> Subject: [PATCH v3 09/19] xen/arm32: mm: Check if the virtual address is
> shared
> before updating it
>
> From: Julien Grall <jgr...@amazon.com>
>
> Only the first 2GB of the virtual address space is shared between all
> the page-tables on Arm32.
>
> There is a long outstanding TODO in xen_pt_update() stating that the
> function can only work with shared mapping. Nobody has ever called
> the function with private mapping, however as we add more callers
> there is a risk to mess things up.
>
> Introduce a new define to mark the end of the shared mappings and use
> it in xen_pt_update() to verify if the address is correct.
>
> Note that on Arm64, all the mappings are shared. Some compiler may
> complain about an always true check, so the new define is not introduced
> for arm64 and the code is protected with an #ifdef.
>
> Signed-off-by: Julien Grall <jgr...@amazon.com>
>
> ---
> Changes in v2:
> - New patch
> ---
> xen/arch/arm/include/asm/config.h | 4 ++++
> xen/arch/arm/mm.c | 11 +++++++++--
> 2 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/arm/include/asm/config.h
> b/xen/arch/arm/include/asm/config.h
> index c7b77912013e..85d4a510ce8a 100644
> --- a/xen/arch/arm/include/asm/config.h
> +++ b/xen/arch/arm/include/asm/config.h
> @@ -137,6 +137,10 @@
>
> #define XENHEAP_VIRT_START _AT(vaddr_t,0x40000000)
> #define XENHEAP_VIRT_END _AT(vaddr_t,0x7fffffff)
> +
> +/* The first 2GB is always shared between all the page-tables. */
> +#define SHARED_VIRT_END _AT(vaddr_t, 0x7fffffff)
> +
> #define DOMHEAP_VIRT_START _AT(vaddr_t,0x80000000)
> #define DOMHEAP_VIRT_END _AT(vaddr_t,0xffffffff)
>
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 24de8dcb9042..f18f65745595 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -1365,11 +1365,18 @@ static int xen_pt_update(unsigned long virt,
> * For arm32, page-tables are different on each CPUs. Yet, they share
> * some common mappings. It is assumed that only common mappings
> * will be modified with this function.
> - *
> - * XXX: Add a check.
> */
> const mfn_t root = virt_to_mfn(THIS_CPU_PGTABLE);
>
> +#ifdef SHARED_VIRT_END
> + if ( virt > SHARED_VIRT_END ||
> + (SHARED_VIRT_END - virt) < nr_mfns )
Why not convert (SHARED_VIRT_END - virt) to page number before comparation?
I think nr_mfns is something related to page numbers, so maybe something like
PAGE_SHIFT or round_pgdown is needed.
I am just wondering, and forgive me if I am wrong.
> + {
> + mm_printk("Trying to map outside of the shared area.\n");
> + return -EINVAL;
> + }
> +#endif
> +
> /*
> * The hardware was configured to forbid mapping both writeable and
> * executable.
> --
> 2.32.0
>
Cheers,
---
Hongda
