diff mbox series

[v3,25/28] xen/arm64: head: Introduce macros to create table and mapping entry

Message ID 20190812173019.11956-26-julien.grall@arm.com (mailing list archive)
State Superseded
Headers show
Series xen/arm: Rework head.S to make it more compliant with the Arm Arm | expand

Commit Message

Julien Grall Aug. 12, 2019, 5:30 p.m. UTC
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.

Furthermore, the two macros are capable to work independently of 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>

---
    Changes in v3:
        - Patch added
---
 xen/arch/arm/arm64/head.S | 83 ++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 67 insertions(+), 16 deletions(-)

Comments

Stefano Stabellini Aug. 22, 2019, 11:31 p.m. UTC | #1
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
                                                         ^ you meant
                                                         only?

>     mapping.
> 
> Furthermore, the two macros are capable to work independently of 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>
> 
> ---
>     Changes in v3:
>         - Patch added
> ---
>  xen/arch/arm/arm64/head.S | 83 ++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 67 insertions(+), 16 deletions(-)
> 
> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> index f2a0e1d3b0..f4177dbba1 100644
> --- a/xen/arch/arm/arm64/head.S
> +++ b/xen/arch/arm/arm64/head.S
> @@ -492,6 +492,68 @@ cpu_init:
>  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

Why not pass the virtual address as a symbol too?


> + * shift:   #imm page table shift
> + * tmp1:    scratch register
> + * tmp2:    scratch register
> + * tmp3:    scratch register
> + *
> + * Preserves \virt
> + * Clobbers \tmp1, \tmp2, \tmp3
> + *
> + * Also use x20 for the phys offset.
> + *
> + * Note that all parameters using registers should be distinct.
> + */
> +.macro create_table_entry, ptbl, tbl, virt, shift, tmp1, tmp2, tmp3
> +        lsr   \tmp1, \virt, #\shift
> +        and   \tmp1, \tmp1, #LPAE_ENTRY_MASK/* \tmp1 := slot in \tlb */
> +
> +        load_paddr \tmp2, \tbl
> +        mov   \tmp3, #PT_PT                 /* \tmp3 := right for linear PT */
> +        orr   \tmp3, \tmp3, \tmp2           /*          + \tlb paddr */
> +
> +        adr_l \tmp2, \ptbl
> +
> +        str   \tmp3, [\tmp2, \tmp1, lsl #3]
> +.endm
> +
> +/*
> + * Macro to create a mapping entry in \tbl to \phys. Only mapping in 3rd
> + * level table (i.e page granularity) is supported.
> + *
> + * tbl:     table symbol where the entry will be created

NIT: for consistency, I would prefer if you called it ptlb


> + * virt:    virtual address

It could be a symbol here, right?


> + * phys:    physical address (should be page aligned)
> + * tmp1:    scratch register
> + * tmp2:    scratch register
> + * tmp3:    scratch register
> + * type:    mapping type. If not specified it will be normal memory (PT_MEM_L3)
> + *
> + * Preserves \virt, \phys
> + * Clobbers \tmp1, \tmp2, \tmp3
> + *
> + * Note that all parameters using registers should be distinct.
> + */
> +.macro create_mapping_entry, tbl, virt, phys, tmp1, tmp2, tmp3, type=PT_MEM_L3
> +        and   \tmp3, \phys, #THIRD_MASK     /* \tmp3 := PAGE_ALIGNED(phys) */
> +
> +        lsr   \tmp1, \virt, #THIRD_SHIFT
> +        and   \tmp1, \tmp1, #LPAE_ENTRY_MASK/* \tmp1 := slot in \tlb */
> +
> +        mov   \tmp2, #\type                 /* \tmp2 := right for section PT */
> +        orr   \tmp2, \tmp2, \tmp3           /*          + PAGE_ALIGNED(phys) */
> +
> +        adr_l \tmp3, \tbl
> +
> +        str   \tmp2, [\tmp3, \tmp1, lsl #3]
> +.endm
> +
> +/*
>   * Rebuild the boot pagetable's first-level entries. The structure
>   * is described in mm.c.
>   *
> @@ -735,28 +797,17 @@ ENDPROC(remove_identity_mapping)
>   *   x20: Physical offset
>   *   x23: Early UART base physical address
>   *
> - * Clobbers x1 - x4
> + * Clobbers x0 - x3
>   */
>  setup_fixmap:
>  #ifdef CONFIG_EARLY_PRINTK
>          /* Add UART to the fixmap table */
> -        ldr   x1, =xen_fixmap        /* x1 := vaddr (xen_fixmap) */
> -        lsr   x2, x23, #THIRD_SHIFT
> -        lsl   x2, x2, #THIRD_SHIFT   /* 4K aligned paddr of UART */
> -        mov   x3, #PT_DEV_L3
> -        orr   x2, x2, x3             /* x2 := 4K dev map including UART */
> -        str   x2, [x1, #(FIXMAP_CONSOLE*8)] /* Map it in the first fixmap's slot */
> +        ldr   x0, =EARLY_UART_VIRTUAL_ADDRESS
> +        create_mapping_entry xen_fixmap, x0, x23, x1, x2, x3, type=PT_DEV_L3
>  #endif
> -
>          /* Map fixmap into boot_second */
> -        ldr   x4, =boot_second       /* x4 := vaddr (boot_second) */
> -        load_paddr x2, xen_fixmap
> -        mov   x3, #PT_PT
> -        orr   x2, x2, x3             /* x2 := table map of xen_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 */
> -
> +        ldr   x0, =FIXMAP_ADDR(0)
> +        create_table_entry boot_second, xen_fixmap, x0, SECOND_SHIFT, x1, x2, x3
>          /* Ensure any page table updates made above have occurred. */
>          dsb   nshst
>  
> -- 
> 2.11.0
>
Julien Grall Aug. 22, 2019, 11:44 p.m. UTC | #2
On Fri, 23 Aug 2019, 00:33 Stefano Stabellini, <sstabellini@kernel.org>
wrote:

> 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
>                                                          ^ you meant
>                                                          only?
>

Yes, I will fix it.


> >     mapping.
> >
> > Furthermore, the two macros are capable to work independently of 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>
> >
> > ---
> >     Changes in v3:
> >         - Patch added
> > ---
> >  xen/arch/arm/arm64/head.S | 83
> ++++++++++++++++++++++++++++++++++++++---------
> >  1 file changed, 67 insertions(+), 16 deletions(-)
> >
> > diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> > index f2a0e1d3b0..f4177dbba1 100644
> > --- a/xen/arch/arm/arm64/head.S
> > +++ b/xen/arch/arm/arm64/head.S
> > @@ -492,6 +492,68 @@ cpu_init:
> >  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
>
> Why not pass the virtual address as a symbol too?
>

Because we have no symbol for most of the virtual addresses. They are just
constant defined in config.h.


>
> > + * shift:   #imm page table shift
> > + * tmp1:    scratch register
> > + * tmp2:    scratch register
> > + * tmp3:    scratch register
> > + *
> > + * Preserves \virt
> > + * Clobbers \tmp1, \tmp2, \tmp3
> > + *
> > + * Also use x20 for the phys offset.
> > + *
> > + * Note that all parameters using registers should be distinct.
> > + */
> > +.macro create_table_entry, ptbl, tbl, virt, shift, tmp1, tmp2, tmp3
> > +        lsr   \tmp1, \virt, #\shift
> > +        and   \tmp1, \tmp1, #LPAE_ENTRY_MASK/* \tmp1 := slot in \tlb */
> > +
> > +        load_paddr \tmp2, \tbl
> > +        mov   \tmp3, #PT_PT                 /* \tmp3 := right for
> linear PT */
> > +        orr   \tmp3, \tmp3, \tmp2           /*          + \tlb paddr */
> > +
> > +        adr_l \tmp2, \ptbl
> > +
> > +        str   \tmp3, [\tmp2, \tmp1, lsl #3]
> > +.endm
> > +
> > +/*
> > + * Macro to create a mapping entry in \tbl to \phys. Only mapping in 3rd
> > + * level table (i.e page granularity) is supported.
> > + *
> > + * tbl:     table symbol where the entry will be created
>
> NIT: for consistency, I would prefer if you called it ptlb
>

Ok.


>
> > + * virt:    virtual address
>
> It could be a symbol here, right?
>

No. See above.


>
> > + * phys:    physical address (should be page aligned)
> > + * tmp1:    scratch register
> > + * tmp2:    scratch register
> > + * tmp3:    scratch register
> > + * type:    mapping type. If not specified it will be normal memory
> (PT_MEM_L3)
> > + *
> > + * Preserves \virt, \phys
> > + * Clobbers \tmp1, \tmp2, \tmp3
> > + *
> > + * Note that all parameters using registers should be distinct.
> > + */
> > +.macro create_mapping_entry, tbl, virt, phys, tmp1, tmp2, tmp3,
> type=PT_MEM_L3
> > +        and   \tmp3, \phys, #THIRD_MASK     /* \tmp3 :=
> PAGE_ALIGNED(phys) */
> > +
> > +        lsr   \tmp1, \virt, #THIRD_SHIFT
> > +        and   \tmp1, \tmp1, #LPAE_ENTRY_MASK/* \tmp1 := slot in \tlb */
> > +
> > +        mov   \tmp2, #\type                 /* \tmp2 := right for
> section PT */
> > +        orr   \tmp2, \tmp2, \tmp3           /*          +
> PAGE_ALIGNED(phys) */
> > +
> > +        adr_l \tmp3, \tbl
> > +
> > +        str   \tmp2, [\tmp3, \tmp1, lsl #3]
> > +.endm
> > +
> > +/*
> >   * Rebuild the boot pagetable's first-level entries. The structure
> >   * is described in mm.c.
> >   *
> > @@ -735,28 +797,17 @@ ENDPROC(remove_identity_mapping)
> >   *   x20: Physical offset
> >   *   x23: Early UART base physical address
> >   *
> > - * Clobbers x1 - x4
> > + * Clobbers x0 - x3
> >   */
> >  setup_fixmap:
> >  #ifdef CONFIG_EARLY_PRINTK
> >          /* Add UART to the fixmap table */
> > -        ldr   x1, =xen_fixmap        /* x1 := vaddr (xen_fixmap) */
> > -        lsr   x2, x23, #THIRD_SHIFT
> > -        lsl   x2, x2, #THIRD_SHIFT   /* 4K aligned paddr of UART */
> > -        mov   x3, #PT_DEV_L3
> > -        orr   x2, x2, x3             /* x2 := 4K dev map including UART
> */
> > -        str   x2, [x1, #(FIXMAP_CONSOLE*8)] /* Map it in the first
> fixmap's slot */
> > +        ldr   x0, =EARLY_UART_VIRTUAL_ADDRESS
> > +        create_mapping_entry xen_fixmap, x0, x23, x1, x2, x3,
> type=PT_DEV_L3
> >  #endif
> > -
> >          /* Map fixmap into boot_second */
> > -        ldr   x4, =boot_second       /* x4 := vaddr (boot_second) */
> > -        load_paddr x2, xen_fixmap
> > -        mov   x3, #PT_PT
> > -        orr   x2, x2, x3             /* x2 := table map of xen_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 */
> > -
> > +        ldr   x0, =FIXMAP_ADDR(0)
> > +        create_table_entry boot_second, xen_fixmap, x0, SECOND_SHIFT,
> x1, x2, x3
> >          /* 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
diff mbox series

Patch

diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
index f2a0e1d3b0..f4177dbba1 100644
--- a/xen/arch/arm/arm64/head.S
+++ b/xen/arch/arm/arm64/head.S
@@ -492,6 +492,68 @@  cpu_init:
 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
+ * tmp1:    scratch register
+ * tmp2:    scratch register
+ * tmp3:    scratch register
+ *
+ * Preserves \virt
+ * Clobbers \tmp1, \tmp2, \tmp3
+ *
+ * Also use x20 for the phys offset.
+ *
+ * Note that all parameters using registers should be distinct.
+ */
+.macro create_table_entry, ptbl, tbl, virt, shift, tmp1, tmp2, tmp3
+        lsr   \tmp1, \virt, #\shift
+        and   \tmp1, \tmp1, #LPAE_ENTRY_MASK/* \tmp1 := slot in \tlb */
+
+        load_paddr \tmp2, \tbl
+        mov   \tmp3, #PT_PT                 /* \tmp3 := right for linear PT */
+        orr   \tmp3, \tmp3, \tmp2           /*          + \tlb paddr */
+
+        adr_l \tmp2, \ptbl
+
+        str   \tmp3, [\tmp2, \tmp1, lsl #3]
+.endm
+
+/*
+ * Macro to create a mapping entry in \tbl to \phys. 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 (should be page aligned)
+ * tmp1:    scratch register
+ * tmp2:    scratch register
+ * tmp3:    scratch register
+ * type:    mapping type. If not specified it will be normal memory (PT_MEM_L3)
+ *
+ * Preserves \virt, \phys
+ * Clobbers \tmp1, \tmp2, \tmp3
+ *
+ * Note that all parameters using registers should be distinct.
+ */
+.macro create_mapping_entry, tbl, virt, phys, tmp1, tmp2, tmp3, type=PT_MEM_L3
+        and   \tmp3, \phys, #THIRD_MASK     /* \tmp3 := PAGE_ALIGNED(phys) */
+
+        lsr   \tmp1, \virt, #THIRD_SHIFT
+        and   \tmp1, \tmp1, #LPAE_ENTRY_MASK/* \tmp1 := slot in \tlb */
+
+        mov   \tmp2, #\type                 /* \tmp2 := right for section PT */
+        orr   \tmp2, \tmp2, \tmp3           /*          + PAGE_ALIGNED(phys) */
+
+        adr_l \tmp3, \tbl
+
+        str   \tmp2, [\tmp3, \tmp1, lsl #3]
+.endm
+
+/*
  * Rebuild the boot pagetable's first-level entries. The structure
  * is described in mm.c.
  *
@@ -735,28 +797,17 @@  ENDPROC(remove_identity_mapping)
  *   x20: Physical offset
  *   x23: Early UART base physical address
  *
- * Clobbers x1 - x4
+ * Clobbers x0 - x3
  */
 setup_fixmap:
 #ifdef CONFIG_EARLY_PRINTK
         /* Add UART to the fixmap table */
-        ldr   x1, =xen_fixmap        /* x1 := vaddr (xen_fixmap) */
-        lsr   x2, x23, #THIRD_SHIFT
-        lsl   x2, x2, #THIRD_SHIFT   /* 4K aligned paddr of UART */
-        mov   x3, #PT_DEV_L3
-        orr   x2, x2, x3             /* x2 := 4K dev map including UART */
-        str   x2, [x1, #(FIXMAP_CONSOLE*8)] /* Map it in the first fixmap's slot */
+        ldr   x0, =EARLY_UART_VIRTUAL_ADDRESS
+        create_mapping_entry xen_fixmap, x0, x23, x1, x2, x3, type=PT_DEV_L3
 #endif
-
         /* Map fixmap into boot_second */
-        ldr   x4, =boot_second       /* x4 := vaddr (boot_second) */
-        load_paddr x2, xen_fixmap
-        mov   x3, #PT_PT
-        orr   x2, x2, x3             /* x2 := table map of xen_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 */
-
+        ldr   x0, =FIXMAP_ADDR(0)
+        create_table_entry boot_second, xen_fixmap, x0, SECOND_SHIFT, x1, x2, x3
         /* Ensure any page table updates made above have occurred. */
         dsb   nshst