From: Stefano Stabellini <sstabellini@kernel.org>
To: Julien Grall <julien@xen.org>
Cc: xen-devel@lists.xenproject.org, marco.solieri@minervasys.tech,
lucmiccio@gmail.com, Julien Grall <jgrall@amazon.com>,
Stefano Stabellini <sstabellini@kernel.org>,
Bertrand Marquis <bertrand.marquis@arm.com>,
Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
Subject: Re: [PATCH early-RFC 4/5] xen/arm: mm: Rework switch_ttbr()
Date: Fri, 11 Mar 2022 17:31:53 -0800 (PST) [thread overview]
Message-ID: <alpine.DEB.2.22.394.2203111721130.3497@ubuntu-linux-20-04-desktop> (raw)
In-Reply-To: <20220309112048.17377-5-julien@xen.org>
On Wed, 9 Mar 2022, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
>
> At the moment, switch_ttbr() is switching the TTBR whilst the MMU is
> still on.
>
> Switching TTBR is like replacing existing mappings with new ones. So
> we need to follow the break-before-make sequence.
>
> In this case, it means the MMU needs to be switched off while the
> TTBR is updated. In order to disable the MMU, we need to first
> jump to an identity mapping.
>
> Rename switch_ttbr() to switch_ttbr_id() and create an helper on
> top to temporary map the identity mapping and call switch_ttbr()
> via the identity address.
>
> switch_ttbr_id() is now reworked to temporarily turn off the MMU
> before updating the TTBR.
>
> We also need to make sure the helper switch_ttbr() is part of the
> identity mapping. So move _end_boot past it.
>
> Take the opportunity to instruction cache flush as the operation is
> only necessary when the memory is updated.
>
> Signed-off-by: Julien Grall <jgrall@amazon.com>
>
> ---
>
> TODO:
> * Rename _end_boot to _end_id_mapping or similar
> * Check the memory barriers
> * I suspect the instruction cache flush will be necessary
> for cache coloring.
> ---
> xen/arch/arm/arm64/head.S | 31 ++++++++++++++++++++-----------
> xen/arch/arm/mm.c | 14 +++++++++++++-
> 2 files changed, 33 insertions(+), 12 deletions(-)
>
> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> index 878649280d73..c5cc72b8fe6f 100644
> --- a/xen/arch/arm/arm64/head.S
> +++ b/xen/arch/arm/arm64/head.S
> @@ -803,36 +803,45 @@ fail: PRINT("- Boot failed -\r\n")
> b 1b
> ENDPROC(fail)
>
> -GLOBAL(_end_boot)
> -
> /*
> * Switch TTBR
> *
> * x0 ttbr
> *
> - * TODO: This code does not comply with break-before-make.
> + * XXX: Check the barriers
> */
> -ENTRY(switch_ttbr)
> +ENTRY(switch_ttbr_id)
> dsb sy /* Ensure the flushes happen before
> * continuing */
> isb /* Ensure synchronization with previous
> * changes to text */
> +
> + /* Turn off MMU */
> + mrs x1, SCTLR_EL2
> + bic x1, x1, #SCTLR_Axx_ELx_M
> + msr SCTLR_EL2, x1
> + dsb sy
> + isb
> +
> tlbi alle2 /* Flush hypervisor TLB */
> - ic iallu /* Flush I-cache */
> dsb sy /* Ensure completion of TLB flush */
> isb
>
> - msr TTBR0_EL2, x0
> + msr TTBR0_EL2, x0
> +
> + mrs x1, SCTLR_EL2
> + orr x1, x1, #SCTLR_Axx_ELx_M /* Enable MMU */
> + msr SCTLR_EL2, x1
>
> isb /* Ensure synchronization with previous
> * changes to text */
> - tlbi alle2 /* Flush hypervisor TLB */
> - ic iallu /* Flush I-cache */
> - dsb sy /* Ensure completion of TLB flush */
> - isb
> + /* Turn on the MMU */
> +
>
> ret
> -ENDPROC(switch_ttbr)
> +ENDPROC(switch_ttbr_id)
> +
> +GLOBAL(_end_boot)
>
> #ifdef CONFIG_EARLY_PRINTK
> /*
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 5c4dece16f7f..a53760af7af0 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -660,7 +660,19 @@ static void xen_pt_enforce_wnx(void)
> flush_xen_tlb_local();
> }
>
> -extern void switch_ttbr(uint64_t ttbr);
> +extern void switch_ttbr_id(uint64_t ttbr);
> +
> +typedef void (switch_ttbr_fn)(uint64_t ttbr);
> +
> +static void switch_ttbr(uint64_t ttbr)
> +{
> + vaddr_t id_addr = virt_to_maddr(switch_ttbr_id);
> + switch_ttbr_fn *fn = (switch_ttbr_fn *)id_addr;
> +
> + update_identity_mapping(true);
> + fn(ttbr);
> + update_identity_mapping(false);
> +}
Controversial question: does it really matter that XEN_VIRT_START >
512GB and that _start < 512GB?
I am totally fine with the limit, I am just brainstorming: given that
the mapping is used very temporarely, it wouldn't really be an issue if
it conflicts with something important. Let's say that it conflicts with
the VMAP or the FRAMETABLE. As long as:
- we save the current mapping
- update it with the Xen 1:1
- switch_ttbr
- remove Xen 1:1
- restore mapping
It should work, right? Basically, a mapping conflict shouldn't be an
issue given that the mapping has only to live long enough to call
switch_ttbr_id.
I am less sure about patch #5 but it doesn't seem it would be a problem
there either.
That said, I am totally fine with _start < 512GB.
next prev parent reply other threads:[~2022-03-12 1:32 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-09 11:20 [PATCH early-RFC 0/5] xen/arm: Don't switch TTBR while the MMU is on Julien Grall
2022-03-09 11:20 ` [PATCH early-RFC 1/5] xen/arm: Clean-up the memory layout Julien Grall
2022-03-17 15:23 ` Bertrand Marquis
2022-03-17 20:32 ` Julien Grall
2022-03-18 8:45 ` Bertrand Marquis
2022-03-09 11:20 ` [PATCH early-RFC 2/5] xen/arm64: Rework " Julien Grall
2022-03-17 20:46 ` Julien Grall
2022-03-25 13:17 ` Bertrand Marquis
2022-03-25 13:35 ` Julien Grall
2022-03-25 14:05 ` Bertrand Marquis
2022-03-25 14:36 ` Julien Grall
2022-05-21 15:49 ` Julien Grall
2022-03-09 11:20 ` [PATCH early-RFC 3/5] xen/arm: mm: Introduce helpers to prepare/enable/disable the identity mapping Julien Grall
2022-03-25 13:32 ` Bertrand Marquis
2022-03-25 13:48 ` Julien Grall
2022-03-25 14:11 ` Bertrand Marquis
2022-03-09 11:20 ` [PATCH early-RFC 4/5] xen/arm: mm: Rework switch_ttbr() Julien Grall
2022-03-12 1:17 ` Stefano Stabellini
2022-03-12 18:20 ` Julien Grall
2022-03-14 23:27 ` Stefano Stabellini
2022-03-12 1:31 ` Stefano Stabellini [this message]
2022-03-12 18:54 ` Julien Grall
2022-03-14 23:48 ` Stefano Stabellini
2022-03-15 19:01 ` Julien Grall
2022-03-16 21:58 ` Stefano Stabellini
2022-03-25 13:47 ` Bertrand Marquis
2022-03-25 14:24 ` Julien Grall
2022-03-25 14:35 ` Bertrand Marquis
2022-03-25 14:42 ` Julien Grall
2022-03-25 14:48 ` Bertrand Marquis
2022-04-07 15:38 ` Julien Grall
2022-04-13 14:02 ` Bertrand Marquis
2022-03-09 11:20 ` [PATCH early-RFC 5/5] xen/arm: smpboot: Directly switch to the runtime 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.22.394.2203111721130.3497@ubuntu-linux-20-04-desktop \
--to=sstabellini@kernel.org \
--cc=Volodymyr_Babchuk@epam.com \
--cc=bertrand.marquis@arm.com \
--cc=jgrall@amazon.com \
--cc=julien@xen.org \
--cc=lucmiccio@gmail.com \
--cc=marco.solieri@minervasys.tech \
--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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.