xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
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 27/28] xen/arm32: head: Introduce macros to create table and mapping entry
Date: Thu, 22 Aug 2019 18:10:08 -0700 (PDT)	[thread overview]
Message-ID: <alpine.DEB.2.21.1908221645070.25445@sstabellini-ThinkPad-T480s> (raw)
In-Reply-To: <20190812173019.11956-28-julien.grall@arm.com>

On Mon, 12 Aug 2019, Julien Grall wrote:
> At the moment, any update to the boot-pages are open-coded. This is
> making more difficult to understand the logic of a function as each
> update roughly requires 6 instructions.
> 
> To ease the readability, two new macros are introduced:
>     - create_table_entry: Create a page-table entry in a given table.
>     This can work at any level.
>     - create_mapping_entry: Create a mapping entry in a given table.
>     None of the users will require to map at any other level than 3rd
>     (i.e page granularity). So the macro is supporting support 3rd level
>     mapping.
> 
> Unlike arm64, there are no easy way to have a PC relative address within
> the range -/+4GB. In order to have the possibility to use the macro in
> context with MMU on/off, the user needs to tell the state of the MMU.
> 
> Lastly, take the opportunity to replace open-coded version in
> setup_fixmap() by the two new macros. The ones in create_page_tables()
> will be replaced in a follow-up patch.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> 
> ---
>     The adr_l hack is a bit ugly, but I can't find nicer way to avoid
>     code duplication and improve readability.
> 
>     Changes in v3:
>         - Patch added
> ---
>  xen/arch/arm/arm32/head.S | 108 ++++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 89 insertions(+), 19 deletions(-)
> 
> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> index e86a9f95e7..6d03fecaf2 100644
> --- a/xen/arch/arm/arm32/head.S
> +++ b/xen/arch/arm/arm32/head.S
> @@ -50,6 +50,20 @@
>  .endm
>  
>  /*
> + * There are no easy way to have a PC relative address within the range
> + * +/- 4GB of the PC.
> + *
> + * This macro workaround it by asking the user to tell whether the MMU
> + * has been turned on or not.

I would add one statement saying why we are using r10 below in the
implementation. Just a suggestion to make things clearer.


> + */
> +.macro adr_l, dst, sym, mmu
> +        ldr   \dst, =\sym
> +        .if \mmu == 0
> +        add   \dst, \dst, r10
> +        .endif
> +.endm
> +
> +/*
>   * Common register usage in this file:
>   *   r0  -
>   *   r1  -
> @@ -342,6 +356,76 @@ cpu_init_done:
>  ENDPROC(cpu_init)
>  
>  /*
> + * Macro to create a page table entry in \ptbl to \tbl
> + *
> + * ptbl:    table symbol where the entry will be created
> + * tbl:     table symbol to point to
> + * virt:    virtual address
> + * shift:   #imm page table shift
> + * mmu:     Is the MMU turned on/off. If not specified it will be off
> + *
> + * Preserves \virt
> + * Clobbers r1 - r4

In the 64bit version you added the temp registers to the parameter list.
Why do things differently here, hard-coding the usage of r1-r4?


> + * Also use r10 for the phys offset.
> + *
> + * Note that \virt should be in a register other than r1 - r4
> + */
> +.macro create_table_entry, ptbl, tbl, virt, shift, mmu=0
> +        lsr   r1, \virt, #\shift
> +        mov_w r2, LPAE_ENTRY_MASK
> +        and   r1, r1, r2             /* r1 := slot in \tlb */
> +        lsl   r1, r1, #3             /* r1 := slot offset in \tlb */
> +
> +        ldr   r4, =\tbl
> +        add   r4, r4, r10            /* r4 := paddr(\tlb) */

you could use adr_l?


> +        mov   r2, #PT_PT             /* r2:r3 := right for linear PT */
> +        orr   r2, r2, r4             /*           + \tlb paddr */
> +        mov   r3, #0
> +
> +        adr_l r4, \ptbl, \mmu
> +
> +        strd  r2, r3, [r4, r1]
> +.endm
> +
> +/*
> + * Macro to create a mapping entry in \tbl to \paddr. Only mapping in 3rd
> + * level table (i.e page granularity) is supported.
> + *
> + * tbl:     table symbol where the entry will be created
> + * virt:    virtual address
> + * phys:    physical address
> + * type:    mapping type. If not specified it will be normal memory (PT_MEM_L3)
> + * mmu:     Is the MMU turned on/off. If not specified it will be off
> + *
> + * Preserves \virt, \phys
> + * Clobbers r1 - r4

Same here


> + * * Also use r10 for the phys offset.
> + *
> + * Note that \virt and \paddr should be in other registers than r1 - r4
> + * and be distinct.
> + */
> +.macro create_mapping_entry, tbl, virt, phys, type=PT_MEM_L3, mmu=0
> +        lsr   r4, \phys, #THIRD_SHIFT
> +        lsl   r4, r4, #THIRD_SHIFT   /* r4 := PAGE_ALIGNED(phys) */

and THIRD_MASK like for arm64? Doesn't really matter but it would be
nicer if we could keep them similar.


> +        mov_w r2, LPAE_ENTRY_MASK
> +        lsr   r1, \virt, #THIRD_SHIFT
> +        and   r1, r1, r2             /* r1 := slot in \tlb */
> +        lsl   r1, r1, #3             /* r1 := slot offset in \tlb */

I would prefer if you moved these for lines up, like you have down in
create_table_entry, it is clearer to read for me. Just an optional
suggestion.


> +        mov   r2, #\type             /* r2:r3 := right for section PT */
> +        orr   r2, r2, r4             /*          + PAGE_ALIGNED(phys) */
> +        mov   r3, #0
> +
> +        adr_l r4, \tbl, \mmu
> +
> +        strd  r2, r3, [r4, r1]
> +.endm
> +
> +/*
>   * Rebuild the boot pagetable's first-level entries. The structure
>   * is described in mm.c.
>   *
> @@ -559,31 +643,17 @@ ENDPROC(remove_identity_mapping)
>   *   r10: Physical offset
>   *   r11: Early UART base physical address
>   *
> - * Clobbers r1 - r4
> + * Clobbers r0 - r4
>   */
>  setup_fixmap:
>  #if defined(CONFIG_EARLY_PRINTK)
>          /* Add UART to the fixmap table */
> -        ldr   r1, =xen_fixmap        /* r1 := vaddr (xen_fixmap) */
> -        lsr   r2, r11, #THIRD_SHIFT
> -        lsl   r2, r2, #THIRD_SHIFT   /* 4K aligned paddr of UART */
> -        orr   r2, r2, #PT_UPPER(DEV_L3)
> -        orr   r2, r2, #PT_LOWER(DEV_L3) /* r2:r3 := 4K dev map including UART */
> -        mov   r3, #0x0
> -        strd  r2, r3, [r1, #(FIXMAP_CONSOLE*8)] /* Map it in the first fixmap's slot */
> +        ldr   r0, =EARLY_UART_VIRTUAL_ADDRESS
> +        create_mapping_entry xen_fixmap, r0, r11, type=PT_DEV_L3, mmu=1
>  #endif
> -
>          /* Map fixmap into boot_second */
> -        ldr   r1, =boot_second       /* r1 := vaddr (boot_second) */
> -        ldr   r2, =xen_fixmap
> -        add   r2, r2, r10            /* r2 := paddr (xen_fixmap) */
> -        orr   r2, r2, #PT_UPPER(PT)
> -        orr   r2, r2, #PT_LOWER(PT)  /* r2:r3 := table map of xen_fixmap */
> -        ldr   r4, =FIXMAP_ADDR(0)
> -        mov   r4, r4, lsr #(SECOND_SHIFT - 3)   /* r4 := Slot for FIXMAP(0) */
> -        mov   r3, #0x0
> -        strd  r2, r3, [r1, r4]       /* Map it in the fixmap's slot */
> -
> +        mov_w r0, FIXMAP_ADDR(0)
> +        create_table_entry boot_second, xen_fixmap, r0, SECOND_SHIFT, mmu=1
>          /* Ensure any page table updates made above have occurred. */
>          dsb   nshst
>  
> -- 
> 2.11.0
> 

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

  reply	other threads:[~2019-08-23  1:10 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
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 [this message]
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.1908221645070.25445@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).