From: Stefano Stabellini <sstabellini@kernel.org>
To: Julien Grall <julien.grall@arm.com>
Cc: xen-devel@lists.xenproject.org,
Stefano Stabellini <sstabellini@kernel.org>,
Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
Subject: Re: [Xen-devel] [PATCH v3 02/28] xen/arm64: head: Remove 1:1 mapping as soon as it is not used
Date: Thu, 22 Aug 2019 10:58:53 -0700 (PDT) [thread overview]
Message-ID: <alpine.DEB.2.21.1908221034570.22783@sstabellini-ThinkPad-T480s> (raw)
In-Reply-To: <20190812173019.11956-3-julien.grall@arm.com>
On Mon, 12 Aug 2019, Julien Grall wrote:
> The 1:1 mapping may clash with other parts of the Xen virtual memory
> layout. At the moment, Xen is handling the clash by only creating a
> mapping to the runtime virtual address before enabling the MMU.
>
> The rest of the mappings (such as the fixmap) will be mapped after the
> MMU is enabled. However, the code doing the mapping is not safe as it
> replace mapping without using the Break-Before-Make sequence.
>
> As the 1:1 mapping can be anywhere in the memory, it is easier to remove
> all the entries added as soon as the 1:1 mapping is not used rather than
> adding the Break-Before-Make sequence everywhere.
>
> It is difficult to track where exactly the 1:1 mapping was created
> without a full rework of create_page_tables(). Instead, introduce a new
> function remove_identity_mapping() will look where is the top-level entry
> for the 1:1 mapping and remove it.
>
> The new function is only called for the boot CPU. Secondary CPUs will
> switch directly to the runtime page-tables so there are no need to
> remove the 1:1 mapping. Note that this still doesn't make the Secondary
> CPUs path safe but it is not making it worst.
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
>
> ---
> It is very likely we will need to re-introduce the 1:1 mapping to cater
> secondary CPUs boot and suspend/resume. For now, the attempt is to make
> boot CPU path fully Arm Arm compliant.
>
> Changes in v3:
> - Avoid hardcoding slots
>
> Changes in v2:
> - s/ID map/1:1 mapping/
> - Rename remove_id_map() to remove_identity_mapping()
> - Add missing signed-off-by
> ---
> xen/arch/arm/arm64/head.S | 94 +++++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 79 insertions(+), 15 deletions(-)
>
> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> index 50cff08756..ec138aae3e 100644
> --- a/xen/arch/arm/arm64/head.S
> +++ b/xen/arch/arm/arm64/head.S
> @@ -33,6 +33,11 @@
> #define PT_DEV 0xe71 /* nG=1 AF=1 SH=10 AP=01 NS=1 ATTR=100 T=0 P=1 */
> #define PT_DEV_L3 0xe73 /* nG=1 AF=1 SH=10 AP=01 NS=1 ATTR=100 T=1 P=1 */
>
> +/* Convenience defines to get slot used by Xen mapping. */
> +#define XEN_ZEROETH_SLOT zeroeth_table_offset(XEN_VIRT_START)
> +#define XEN_FIRST_SLOT first_table_offset(XEN_VIRT_START)
> +#define XEN_SECOND_SLOT second_table_offset(XEN_VIRT_START)
> +
> #define __HEAD_FLAG_PAGE_SIZE ((PAGE_SHIFT - 10) / 2)
>
> #define __HEAD_FLAG_PHYS_BASE 1
> @@ -301,6 +306,13 @@ real_start_efi:
> ldr x0, =primary_switched
> br x0
> primary_switched:
> + /*
> + * The 1:1 map may clash with other parts of the Xen virtual memory
> + * layout. As it is not used anymore, remove it completely to
> + * avoid having to worry about replacing existing mapping
> + * afterwards.
> + */
> + bl remove_identity_mapping
> bl setup_fixmap
> #ifdef CONFIG_EARLY_PRINTK
> /* Use a virtual address to access the UART. */
> @@ -626,10 +638,71 @@ enable_mmu:
> ret
> ENDPROC(enable_mmu)
>
> +/*
> + * Remove the 1:1 map for the page-tables. It is not easy to keep track
^ from
> + * where the 1:1 map was mapped, so we will look for the top-level entry
> + * exclusive to the 1:1 map and remove it.
> + *
> + * Inputs:
> + * x19: paddr(start)
> + *
> + * Clobbers x0 - x1
> + */
> +remove_identity_mapping:
> + /*
> + * Find the zeroeth slot used. Remove the entry from zeroeth
> + * table if the slot is not XEN_ZEROETH_SLOT.
This part of the comment is good
> + * For slot XEN_ZEROETH_SLOT, the 1:1 mapping was either done in first
> + * or second table.
I don't think this sentence is very useful now that the slot is not
hard-coded anymore. I would remove it. Instead, I would write something
like "The slot XEN_ZEROETH_SLOT is used for the XEN_VIRT_START mapping."
The same goes for all the other levels.
> + */
> + lsr x1, x19, #ZEROETH_SHIFT /* x1 := zeroeth slot */
> + cmp x1, #XEN_ZEROETH_SLOT
> + beq 1f
> + /* It is not in slot XEN_ZEROETH_SLOT, remove the entry. */
> + ldr x0, =boot_pgtable /* x0 := root table */
> + str xzr, [x0, x1, lsl #3]
> + b identity_mapping_removed
> +
> +1:
> + /*
> + * Find the first slot used. Remove the entry for the first
> + * table if the slot is not XEN_FIRST_SLOT. For slot XEN_FIRST_SLOT,
> + * the 1:1 mapping was done in the second table.
> + */
> + lsr x1, x19, #FIRST_SHIFT
> + and x1, x1, #LPAE_ENTRY_MASK /* x1 := first slot */
> + cmp x1, #XEN_FIRST_SLOT
> + beq 1f
> + /* It is not in slot XEN_FIRST_SLOT, remove the entry. */
> + ldr x0, =boot_first /* x0 := first table */
> + str xzr, [x0, x1, lsl #3]
> + b identity_mapping_removed
> +
> +1:
> + /*
> + * Find the second slot used. Remove the entry for the first
> + * table if the slot is not XEN_SECOND_SLOT. For slot XEN_SECOND_SLOT,
> + * it means the 1:1 mapping was not created.
> + */
> + lsr x1, x19, #SECOND_SHIFT
> + and x1, x1, #LPAE_ENTRY_MASK /* x1 := first slot */
> + cmp x1, #XEN_SECOND_SLOT
> + beq identity_mapping_removed
> + /* It is not in slot 1, remove the entry */
> + ldr x0, =boot_second /* x0 := second table */
> + str xzr, [x0, x1, lsl #3]
> +
> +identity_mapping_removed:
> + /* See asm-arm/arm64/flushtlb.h for the explanation of the sequence. */
> + dsb nshst
> + tlbi alle2
> + dsb nsh
> + isb
I just want to point out that asm-arm/arm64/flushtlb.h says to use:
* DSB ISHST // Ensure prior page-tables updates have completed
* TLBI... // Invalidate the TLB
* DSB ISH // Ensure the TLB invalidation has completed
* ISB // See explanation below
Also implemented as:
asm volatile( \
"dsb ishst;" \
"tlbi " # tlbop ";" \
"dsb ish;" \
"isb;" \
: : : "memory"); \
Why is non-shareable enough? Shouldn't it be inner shareable?
> + ret
> +ENDPROC(remove_identity_mapping)
> +
> setup_fixmap:
> - /* Now we can install the fixmap and dtb mappings, since we
> - * don't need the 1:1 map any more */
> - dsb sy
> #if defined(CONFIG_EARLY_PRINTK) /* Fixmap is only used by early printk */
> /* Add UART to the fixmap table */
> ldr x1, =xen_fixmap /* x1 := vaddr (xen_fixmap) */
> @@ -647,19 +720,10 @@ setup_fixmap:
> ldr x1, =FIXMAP_ADDR(0)
> lsr x1, x1, #(SECOND_SHIFT - 3) /* x1 := Slot for FIXMAP(0) */
> str x2, [x4, x1] /* Map it in the fixmap's slot */
> -#endif
>
> - /*
> - * Flush the TLB in case the 1:1 mapping happens to clash with
> - * the virtual addresses used by the fixmap or DTB.
> - */
> - dsb sy /* Ensure any page table updates made above
> - * have occurred. */
> -
> - isb
> - tlbi alle2
> - dsb sy /* Ensure completion of TLB flush */
> - isb
> + /* Ensure any page table updates made above have occurred. */
> + dsb nshst
Same here about shareability
> +#endif
> ret
> ENDPROC(setup_fixmap)
>
> --
> 2.11.0
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
next prev parent reply other threads:[~2019-08-22 17:59 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-12 17:29 [Xen-devel] [PATCH v3 00/28] xen/arm: Rework head.S to make it more compliant with the Arm Arm Julien Grall
2019-08-12 17:29 ` [Xen-devel] [PATCH v3 01/28] xen/arm: lpae: Allow more LPAE helpers to be used in assembly Julien Grall
2019-08-22 17:07 ` Stefano Stabellini
2019-08-12 17:29 ` [Xen-devel] [PATCH v3 02/28] xen/arm64: head: Remove 1:1 mapping as soon as it is not used Julien Grall
2019-08-22 17:58 ` Stefano Stabellini [this message]
2019-08-22 18:25 ` Julien Grall
2019-08-22 18:29 ` Stefano Stabellini
2019-08-12 17:29 ` [Xen-devel] [PATCH v3 03/28] xen/arm64: head: Rework and document setup_fixmap() Julien Grall
2019-08-12 17:29 ` [Xen-devel] [PATCH v3 04/28] xen/arm64: head: Rework and document launch() Julien Grall
2019-08-12 17:29 ` [Xen-devel] [PATCH v3 05/28] xen/arm64: head: Setup TTBR_EL2 in enable_mmu() and add missing isb Julien Grall
2019-08-12 17:29 ` [Xen-devel] [PATCH v3 06/28] xen/arm64: head: Introduce a macro to get a PC-relative address of a symbol Julien Grall
2019-08-12 17:29 ` [Xen-devel] [PATCH v3 07/28] xen/arm64: head: Fix typo in the documentation on top of init_uart() Julien Grall
2019-08-22 17:08 ` Stefano Stabellini
2019-08-12 17:29 ` [Xen-devel] [PATCH v3 08/28] xen/arm32: head: Add a macro to move an immediate constant into a 32-bit register Julien Grall
2019-08-12 17:30 ` [Xen-devel] [PATCH v3 09/28] xen/arm32: head: Mark the end of subroutines with ENDPROC Julien Grall
2019-08-12 17:33 ` Julien Grall
2019-08-12 17:30 ` [Xen-devel] [PATCH v3 10/28] xen/arm32: head: Don't clobber r14/lr in the macro PRINT Julien Grall
2019-08-12 17:30 ` [Xen-devel] [PATCH v3 11/28] xen/arm32: head: Rework UART initialization on boot CPU Julien Grall
2019-08-12 17:30 ` [Xen-devel] [PATCH v3 12/28] xen/arm32: head: Introduce print_reg Julien Grall
2019-08-12 17:30 ` [Xen-devel] [PATCH v3 13/28] xen/arm32: head: Introduce distinct paths for the boot CPU and secondary CPUs Julien Grall
2019-08-22 17:11 ` Stefano Stabellini
2019-08-12 17:30 ` [Xen-devel] [PATCH v3 14/28] xen/arm32: head: Rework and document check_cpu_mode() Julien Grall
2019-08-22 17:14 ` Stefano Stabellini
2019-09-07 10:35 ` Julien Grall
2019-08-12 17:30 ` [Xen-devel] [PATCH v3 15/28] xen/arm32: head: Rework and document zero_bss() Julien Grall
2019-08-12 17:30 ` [Xen-devel] [PATCH v3 16/28] xen/arm32: head: Document create_pages_tables() Julien Grall
2019-08-12 17:30 ` [Xen-devel] [PATCH v3 17/28] xen/arm32: head: Document enable_mmu() Julien Grall
2019-08-12 17:30 ` [Xen-devel] [PATCH v3 18/28] xen/arm32: head: Move assembly switch to the runtime PT in secondary CPUs path Julien Grall
2019-08-22 17:17 ` Stefano Stabellini
2019-08-12 17:30 ` [Xen-devel] [PATCH v3 19/28] xen/arm32: head: Don't setup the fixmap on secondary CPUs Julien Grall
2019-08-12 17:30 ` [Xen-devel] [PATCH v3 20/28] xen/arm32: head: Remove 1:1 mapping as soon as it is not used Julien Grall
2019-08-22 18:17 ` Stefano Stabellini
2019-08-22 18:31 ` Julien Grall
2019-08-22 22:53 ` Stefano Stabellini
2019-08-22 23:39 ` Julien Grall
2019-08-12 17:30 ` [Xen-devel] [PATCH v3 21/28] xen/arm32: head: Rework and document setup_fixmap() Julien Grall
2019-08-12 17:30 ` [Xen-devel] [PATCH v3 22/28] xen/arm32: head: Rework and document launch() Julien Grall
2019-08-12 17:30 ` [Xen-devel] [PATCH v3 23/28] xen/arm32: head: Setup HTTBR in enable_mmu() and add missing isb Julien Grall
2019-08-22 17:15 ` Stefano Stabellini
2019-08-12 17:30 ` [Xen-devel] [PATCH v3 24/28] xen/arm: Zero BSS after the MMU and D-cache is turned on Julien Grall
2019-08-12 17:30 ` [Xen-devel] [PATCH v3 25/28] xen/arm64: head: Introduce macros to create table and mapping entry Julien Grall
2019-08-22 23:31 ` Stefano Stabellini
2019-08-22 23:44 ` Julien Grall
2019-08-12 17:30 ` [Xen-devel] [PATCH v3 26/28] xen/arm64: head: Use a page mapping for the 1:1 mapping in create_page_tables() Julien Grall
2019-08-24 1:16 ` Stefano Stabellini
2019-09-17 17:53 ` Julien Grall
2019-08-12 17:30 ` [Xen-devel] [PATCH v3 27/28] xen/arm32: head: Introduce macros to create table and mapping entry Julien Grall
2019-08-23 1:10 ` Stefano Stabellini
2019-08-23 9:40 ` Julien Grall
2019-08-12 17:30 ` [Xen-devel] [PATCH v3 28/28] xen/arm32: head: Use a page mapping for the 1:1 mapping in create_page_tables() Julien Grall
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=alpine.DEB.2.21.1908221034570.22783@sstabellini-ThinkPad-T480s \
--to=sstabellini@kernel.org \
--cc=Volodymyr_Babchuk@epam.com \
--cc=julien.grall@arm.com \
--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).