Re: [Xen-devel] [PATCH 17/17] xen/arm64: Zero BSS after the MMU and D-cache is turned on

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 10 Jun 2019, Julien Grall wrote:
> At the moment BSS is zeroed before the MMU and D-Cache is turned on.
> In other words, the cache will be bypassed when zeroing the BSS section.
> 
> Per the Image protocol [1], the state of the cache for BSS region is not
> known because it is not part of the "loaded kernel image".
> 
> This means that the cache will need to be invalidated twice for the BSS
> region:
>     1) Before zeroing to remove any dirty cache line. Otherwise they may
>     get evicted while zeroing and therefore overriding the value.
>     2) After zeroing to remove any cache line that may have been
>     speculated. Otherwise when turning on MMU and D-Cache, the CPU may
>     see old values.
> 
> However, the only reason to have the BSS zeroed early is because the
> boot page tables are part of BSS. To avoid the two cache invalidations,
> it is possible to move the page tables in the section .data.page_aligned.

I am not following the last part. How is moving the boot page tables to
.data.page_aligned solving the problem? Do we need to zero
.data.page_aligned early or can we skip it because it is guaranteed to
already be zero?


> A new macro DEFINE_BOOT_PAGE_TABLE is introduced to create and mark
> page-tables used before BSS is zeroed. This includes all boot_* but also
> xen_fixmap as zero_bss() will print a message when earlyprintk is
> enabled.

On a similar note, and continuing from what I wrote above, do we need to
make sure to zero the xen_fixmap before hooking it up setup_fixmap?


> [1] linux/Documentation/arm64/booting.txt
> 
> ---
> 
>     Note that the arm32 support is not there yet. This will need to be
>     addressed here or separately depending on when the Arm32 boot rework
>     is sent.
> ---
>  xen/arch/arm/arm64/head.S |  6 +++---
>  xen/arch/arm/mm.c         | 23 +++++++++++++++++------
>  2 files changed, 20 insertions(+), 9 deletions(-)
> 
> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> index 130ab66d8e..6c3edbbc81 100644
> --- a/xen/arch/arm/arm64/head.S
> +++ b/xen/arch/arm/arm64/head.S
> @@ -291,7 +291,6 @@ real_start_efi:
>          mov   x22, #0                /* x22 := is_secondary_cpu */
>  
>          bl    check_cpu_mode
> -        bl    zero_bss
>          bl    cpu_init
>          bl    create_page_tables
>          bl    enable_mmu
> @@ -312,6 +311,7 @@ primary_switched:
>          /* Use a virtual address to access the UART. */
>          ldr   x23, =EARLY_UART_VIRTUAL_ADDRESS
>  #endif
> +        bl    zero_bss
>          PRINT("- Ready -\r\n")
>          /* Setup the arguments for start_xen and jump to C world */
>          mov   x0, x20                /* x0 := phys_offset */
> @@ -423,8 +423,8 @@ zero_bss:
>          cbnz  x26, skip_bss
>  
>          PRINT("- Zero BSS -\r\n")
> -        load_paddr x0, __bss_start    /* Load paddr of start & end of bss */
> -        load_paddr x1, __bss_end
> +        ldr   x0, =__bss_start       /* x0 := vaddr(__bss_start) */
> +        ldr   x1, =__bss_end         /* x1 := vaddr(__bss_start) */
>  
>  1:      str   xzr, [x0], #8
>          cmp   x0, x1
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 6a549e9283..0b2d07a258 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -48,6 +48,17 @@
>  #undef mfn_to_virt
>  #define mfn_to_virt(mfn) __mfn_to_virt(mfn_x(mfn))
>  
> +/*
> + * Macros to define page-tables:
> + *  - DEFINE_BOOT_PAGE_TABLE is used to define page-table that are used
> + *  in assembly code before BSS is zeroed.
> + *  - DEFINE_PAGE_TABLE{,S} are used to define one or multiple
> + *  page-tables to be used after BSS is zeroed (typically they are only used
> + *  in C).
> + */
> +#define DEFINE_BOOT_PAGE_TABLE(name)                                         
>  \
> +lpae_t __aligned(PAGE_SIZE) __section(".data.page_aligned") 
> name[LPAE_ENTRIES]
> +
>  #define DEFINE_PAGE_TABLES(name, nr)                    \
>  lpae_t __aligned(PAGE_SIZE) name[LPAE_ENTRIES * (nr)]
>  
> @@ -76,13 +87,13 @@ lpae_t __aligned(PAGE_SIZE) name[LPAE_ENTRIES * (nr)]
>   * Finally, if EARLY_PRINTK is enabled then xen_fixmap will be mapped
>   * by the CPU once it has moved off the 1:1 mapping.
>   */
> -DEFINE_PAGE_TABLE(boot_pgtable);
> +DEFINE_BOOT_PAGE_TABLE(boot_pgtable);
>  #ifdef CONFIG_ARM_64
> -DEFINE_PAGE_TABLE(boot_first);
> -DEFINE_PAGE_TABLE(boot_first_id);
> +DEFINE_BOOT_PAGE_TABLE(boot_first);
> +DEFINE_BOOT_PAGE_TABLE(boot_first_id);
>  #endif
> -DEFINE_PAGE_TABLE(boot_second);
> -DEFINE_PAGE_TABLE(boot_third);
> +DEFINE_BOOT_PAGE_TABLE(boot_second);
> +DEFINE_BOOT_PAGE_TABLE(boot_third);
>  
>  /* Main runtime page tables */
>  
> @@ -135,7 +146,7 @@ static __initdata int xenheap_first_first_slot = -1;
>   */
>  static DEFINE_PAGE_TABLES(xen_second, 2);
>  /* First level page table used for fixmap */
> -DEFINE_PAGE_TABLE(xen_fixmap);
> +DEFINE_BOOT_PAGE_TABLE(xen_fixmap);
>  /* First level page table used to map Xen itself with the XN bit set
>   * as appropriate. */
>  static DEFINE_PAGE_TABLE(xen_xenmap);
> -- 
> 2.11.0
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel