On Wed, 9 Mar 2022, Julien Grall wrote:
> From: Julien Grall <jgr...@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 <jgr...@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);
> +}
As far as I can tell this should work for coloring too. In the case of
coloring:
/* running on the old mapping, same as non-coloring */
update_identity_mapping(true);
/* jumping to the 1:1 mapping of the old Xen and switching to the
* new pagetable */
fn(ttbr);
/* new pagetable is enabled, now we are back to addresses greater
* than XEN_VIRT_START, which correspond to new cache-colored Xen */
update_identity_mapping(false);
The only doubt that I have is: are we sure than a single page of 1:1
mapping is enough? What if:
virt_to_maddr(switch_ttbr_id) - virt_to_maddr(_start) > PAGE_SIZE
We might have to do a 1:1 mapping of size = _end-_start. It would work
with coloring too because we are doing a 1:1 mapping of the old copy of
Xen which is non-colored and contiguous (not the new copy which is
colored and fragmented).
Thanks Julien very much for your help!
