diff mbox series

[v5,06/13] xen/arm64: Fold setup_fixmap() to create_page_tables()

Message ID 20230814042536.878720-7-Henry.Wang@arm.com (mailing list archive)
State Superseded
Headers show
Series xen/arm: Split MMU code as the prepration of MPU work | expand

Commit Message

Henry Wang Aug. 14, 2023, 4:25 a.m. UTC
The original assembly setup_fixmap() is actually doing two seperate
tasks, one is enabling the early UART when earlyprintk on, and the
other is to set up the fixmap (even when earlyprintk is off).

Per discussion in [1], since commit
9d267c049d92 ("xen/arm64: Rework the memory layout"), there is no
chance that the fixmap and the mapping of early UART will clash with
the 1:1 mapping. Therefore the mapping of both the fixmap and the
early UART can be moved to the end of create_pagetables().

No functional change intended.

[1] https://lore.kernel.org/xen-devel/78862bb8-fd7f-5a51-a7ae-3c5b5998ed80@xen.org/

Signed-off-by: Henry Wang <Henry.Wang@arm.com>
---
v5:
- Refine the title and commit message.
- Drop the "not applied" in-code comment about the 1:1 mapping clash on
  top of create_page_tables().
- Drop the unnecessary dsb and isb from the original setup_fixmap().
v4:
- Rework "[v3,12/52] xen/mmu: extract early uart mapping from setup_fixmap"
---
 xen/arch/arm/arm64/head.S     |  1 -
 xen/arch/arm/arm64/mmu/head.S | 48 ++++++++---------------------------
 2 files changed, 10 insertions(+), 39 deletions(-)

Comments

Julien Grall Aug. 21, 2023, 9:22 a.m. UTC | #1
Hi Henry,

On 14/08/2023 05:25, Henry Wang wrote:
> The original assembly setup_fixmap() is actually doing two seperate
> tasks, one is enabling the early UART when earlyprintk on, and the
> other is to set up the fixmap (even when earlyprintk is off).
> 
> Per discussion in [1], since commit
> 9d267c049d92 ("xen/arm64: Rework the memory layout"), there is no
> chance that the fixmap and the mapping of early UART will clash with
> the 1:1 mapping. Therefore the mapping of both the fixmap and the
> early UART can be moved to the end of create_pagetables().
> 
> No functional change intended.

I would drop this sentence because the fixmap is now prepared much 
earlier in the code. So there is technically some functional change.

> 
> [1] https://lore.kernel.org/xen-devel/78862bb8-fd7f-5a51-a7ae-3c5b5998ed80@xen.org/
> 
> Signed-off-by: Henry Wang <Henry.Wang@arm.com>

Reviewed-by: Julien Grall <jgrall@amazon.com>

Cheers,
Henry Wang Aug. 21, 2023, 9:30 a.m. UTC | #2
> On Aug 21, 2023, at 17:22, Julien Grall <julien@xen.org> wrote:
> 
> Hi Henry,
> 
> On 14/08/2023 05:25, Henry Wang wrote:
>> The original assembly setup_fixmap() is actually doing two seperate
>> tasks, one is enabling the early UART when earlyprintk on, and the
>> other is to set up the fixmap (even when earlyprintk is off).
>> Per discussion in [1], since commit
>> 9d267c049d92 ("xen/arm64: Rework the memory layout"), there is no
>> chance that the fixmap and the mapping of early UART will clash with
>> the 1:1 mapping. Therefore the mapping of both the fixmap and the
>> early UART can be moved to the end of create_pagetables().
>> No functional change intended.
> 
> I would drop this sentence because the fixmap is now prepared much earlier in the code. So there is technically some functional change.

Sure, I will drop this sentence in v6.

> 
>> [1] https://lore.kernel.org/xen-devel/78862bb8-fd7f-5a51-a7ae-3c5b5998ed80@xen.org/
>> Signed-off-by: Henry Wang <Henry.Wang@arm.com>
> 
> Reviewed-by: Julien Grall <jgrall@amazon.com>

Thanks!

Kind regards,
Henry

> 
> Cheers,
> 
> -- 
> Julien Grall
diff mbox series

Patch

diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
index 3c8a12eda7..4ad85dcf58 100644
--- a/xen/arch/arm/arm64/head.S
+++ b/xen/arch/arm/arm64/head.S
@@ -275,7 +275,6 @@  real_start_efi:
         b     enable_boot_cpu_mm
 
 primary_switched:
-        bl    setup_fixmap
 #ifdef CONFIG_EARLY_PRINTK
         /* Use a virtual address to access the UART. */
         ldr   x23, =EARLY_UART_VIRTUAL_ADDRESS
diff --git a/xen/arch/arm/arm64/mmu/head.S b/xen/arch/arm/arm64/mmu/head.S
index 97d872c3cb..ba2ddd7e67 100644
--- a/xen/arch/arm/arm64/mmu/head.S
+++ b/xen/arch/arm/arm64/mmu/head.S
@@ -126,11 +126,6 @@ 
  * Rebuild the boot pagetable's first-level entries. The structure
  * is described in mm.c.
  *
- * After the CPU enables paging it will add the fixmap mapping
- * to these page tables, however this may clash with the 1:1
- * mapping. So each CPU must rebuild the page tables here with
- * the 1:1 in place.
- *
  * Inputs:
  *   x19: paddr(start)
  *   x20: phys offset
@@ -243,6 +238,16 @@  link_from_second_id:
         create_table_entry boot_second_id, boot_third_id, x19, 2, x0, x1, x2
 link_from_third_id:
         create_mapping_entry boot_third_id, x19, x19, x0, x1, x2
+
+#ifdef CONFIG_EARLY_PRINTK
+        /* Add UART to the fixmap table */
+        ldr   x0, =EARLY_UART_VIRTUAL_ADDRESS
+        /* x23: Early UART base physical address */
+        create_mapping_entry xen_fixmap, x0, x23, x1, x2, x3, type=PT_DEV_L3
+#endif
+        /* Map fixmap into boot_second */
+        ldr   x0, =FIXMAP_ADDR(0)
+        create_table_entry boot_second, xen_fixmap, x0, 2, x1, x2, x3
         ret
 
 virtphys_clash:
@@ -402,39 +407,6 @@  identity_mapping_removed:
         ret
 ENDPROC(remove_identity_mapping)
 
-/*
- * Map the UART in the fixmap (when earlyprintk is used) and hook the
- * fixmap table in the page tables.
- *
- * The fixmap cannot be mapped in create_page_tables because it may
- * clash with the 1:1 mapping.
- *
- * Inputs:
- *   x20: Physical offset
- *   x23: Early UART base physical address
- *
- * Clobbers x0 - x3
- */
-ENTRY(setup_fixmap)
-#ifdef CONFIG_EARLY_PRINTK
-        /* Add UART to the fixmap table */
-        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   x0, =FIXMAP_ADDR(0)
-        create_table_entry boot_second, xen_fixmap, x0, 2, x1, x2, x3
-        /* Ensure any page table updates made above have occurred. */
-        dsb   nshst
-        /*
-         * The fixmap area will be used soon after. So ensure no hardware
-         * translation happens before the dsb completes.
-         */
-        isb
-
-        ret
-ENDPROC(setup_fixmap)
-
 /* Fail-stop */
 fail:   PRINT("- Boot failed -\r\n")
 1:      wfe