[Xen-devel,v3,20/28] xen/arm32: head: Remove 1:1 mapping as soon as it is not used

Message ID 20190812173019.11956-21-julien.grall@arm.com
State Superseded
Headers show
Series
  • xen/arm: Rework head.S to make it more compliant with the Arm Arm
Related show

Commit Message

Julien Grall Aug. 12, 2019, 5:30 p.m.
The 1:1 mapping may clash with other parts of the Xen virtual memory
layout. At the moment, Xen is handling the clash by only creating a
mapping to the runtime virtual address before enabling the MMU.

The rest of the mappings (such as the fixmap) will be mapped after the
MMU is enabled. However, the code doing the mapping is not safe as it
replace mapping without using the Break-Before-Make sequence.

As the 1:1 mapping can be anywhere in the memory, it is easier to remove
all the entries added as soon as the 1:1 mapping is not used rather than
adding the Break-Before-Make sequence everywhere.

It is difficult to track where exactly the 1:1 mapping was created
without a full rework of create_page_tables(). Instead, introduce a new
function remove_identity_mapping() will look where is the top-level entry
for the 1:1 mapping and remove it.

The new function is only called for the boot CPU. Secondary CPUs will
switch directly to the runtime page-tables so there are no need to
remove the 1:1 mapping. Note that this still doesn't make the Secondary
CPUs path safe but it is not making it worst.

Signed-off-by: Julien Grall <julien.grall@arm.com>

---
    It is very likely we will need to re-introduce the 1:1 mapping to cater
    secondary CPUs boot and suspend/resume. For now, the attempt is to make
    boot CPU path fully Arm Arm compliant.

    Changes in v3:
        - Remove unused label
        - Avoid harcoding slots

    Changes in v2:
        - Patch added
---
 xen/arch/arm/arm32/head.S | 86 +++++++++++++++++++++++++++++++++++++----------
 1 file changed, 69 insertions(+), 17 deletions(-)

Comments

Stefano Stabellini Aug. 22, 2019, 6:17 p.m. | #1
On Mon, 12 Aug 2019, Julien Grall wrote:
> The 1:1 mapping may clash with other parts of the Xen virtual memory
> layout. At the moment, Xen is handling the clash by only creating a
> mapping to the runtime virtual address before enabling the MMU.
> 
> The rest of the mappings (such as the fixmap) will be mapped after the
> MMU is enabled. However, the code doing the mapping is not safe as it
> replace mapping without using the Break-Before-Make sequence.
> 
> As the 1:1 mapping can be anywhere in the memory, it is easier to remove
> all the entries added as soon as the 1:1 mapping is not used rather than
> adding the Break-Before-Make sequence everywhere.
> 
> It is difficult to track where exactly the 1:1 mapping was created
> without a full rework of create_page_tables(). Instead, introduce a new
> function remove_identity_mapping() will look where is the top-level entry
> for the 1:1 mapping and remove it.
> 
> The new function is only called for the boot CPU. Secondary CPUs will
> switch directly to the runtime page-tables so there are no need to
> remove the 1:1 mapping. Note that this still doesn't make the Secondary
> CPUs path safe but it is not making it worst.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> 
> ---
>     It is very likely we will need to re-introduce the 1:1 mapping to cater
>     secondary CPUs boot and suspend/resume. For now, the attempt is to make
>     boot CPU path fully Arm Arm compliant.
> 
>     Changes in v3:
>         - Remove unused label
>         - Avoid harcoding slots
> 
>     Changes in v2:
>         - Patch added
> ---
>  xen/arch/arm/arm32/head.S | 86 +++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 69 insertions(+), 17 deletions(-)
> 
> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> index 8f945d318a..34fc9ffcee 100644
> --- a/xen/arch/arm/arm32/head.S
> +++ b/xen/arch/arm/arm32/head.S
> @@ -32,6 +32,10 @@
>  #define PT_UPPER(x) (PT_##x & 0xf00)
>  #define PT_LOWER(x) (PT_##x & 0x0ff)
>  
> +/* Convenience defines to get slot used by Xen mapping. */
> +#define XEN_FIRST_SLOT      first_table_offset(XEN_VIRT_START)
> +#define XEN_SECOND_SLOT     second_table_offset(XEN_VIRT_START)
> +
>  #if (defined (CONFIG_EARLY_PRINTK)) && (defined (EARLY_PRINTK_INC))
>  #include EARLY_PRINTK_INC
>  #endif
> @@ -158,6 +162,13 @@ past_zImage:
>          ldr   r0, =primary_switched
>          mov   pc, r0
>  primary_switched:
> +        /*
> +         * The 1:1 map may clash with other parts of the Xen virtual memory
> +         * layout. As it is not used anymore, remove it completely to
> +         * avoid having to worry about replacing existing mapping
> +         * afterwards.
> +         */
> +        bl    remove_identity_mapping
>          bl    setup_fixmap
>  #ifdef CONFIG_EARLY_PRINTK
>          /* Use a virtual address to access the UART. */
> @@ -474,12 +485,63 @@ enable_mmu:
>          mov   pc, lr
>  ENDPROC(enable_mmu)
>  
> -setup_fixmap:
> +/*
> + * Remove the 1:1 map for the page-tables. It is not easy to keep track
> + * where the 1:1 map was mapped, so we will look for the top-level entry
> + * exclusive to the 1:1 map and remove it.
> + *
> + * Inputs:
> + *   r9 : paddr(start)
> + *
> + * Clobbers r0 - r3
> + */
> +remove_identity_mapping:
> +        /* r2:r3 := invalid page-table entry */
> +        mov   r2, #0x0
> +        mov   r3, #0x0
>          /*
> -         * Now we can install the fixmap and dtb mappings, since we
> -         * don't need the 1:1 map any more
> +         * Find the first slot used. Remove the entry for the first
> +         * table if the slot is not XEN_FIRST_SLOT. For slot XEN_FIRST_SLOT,
> +         * the 1:1 mapping was done in the second table.

May I suggest something more similar to the arm64 version as a comment?
Something like:

"Find the first slot used. Remove the entry for the first table if the
slot is not XEN_FIRST_SLOT. In case of XEN_FIRST_SLOT, we have to look
at the second table as the slot is shared with the XEN_VIRT_START
mapping."


>           */
> -        dsb
> +        lsr   r1, r9, #FIRST_SHIFT
> +        mov_w r0, LPAE_ENTRY_MASK

ldr?


> +        and  r1, r1, r0              /* r1 := first slot */
> +        cmp  r1, #XEN_FIRST_SLOT

NIT: align


> +        beq   1f
> +        /* It is not in slot 0, remove the entry */
> +        ldr   r0, =boot_pgtable      /* r0 := root table */
> +        lsl   r1, r1, #3             /* r1 := Slot offset */
> +        strd  r2, r3, [r0, r1]
> +        b     identity_mapping_removed
> +
> +1:
> +        /*
> +         * Find the second slot used. Remove the entry for the first
> +         * table if the slot is not XEN_SECOND_SLOT. For slot XEN_SECOND_SLOT,
> +         * it means the 1:1 mapping was not created.
> +         */
> +        lsr   r1, r9, #SECOND_SHIFT
> +        mov_w r0, LPAE_ENTRY_MASK
> +        and   r1, r1, r0             /* r1 := second slot */
> +        cmp   r1, #XEN_SECOND_SLOT
> +        beq   identity_mapping_removed
> +        /* It is not in slot 1, remove the entry */
> +        ldr   r0, =boot_second       /* r0 := second table */
> +        lsl   r1, r1, #3             /* r1 := Slot offset */
> +        strd  r2, r3, [r0, r1]
> +
> +identity_mapping_removed:
> +        /* See asm-arm/arm32/flushtlb.h for the explanation of the sequence. */
> +        dsb   nshst
> +        mcr   CP32(r0, TLBIALLH)
> +        dsb   nsh
> +        isb

As for the arm64 patch, I would like to understand dsb ishst vs. dsb
nshst.


> +        mov   pc, lr
> +ENDPROC(remove_identity_mapping)
> +
> +setup_fixmap:
>  #if defined(CONFIG_EARLY_PRINTK) /* Fixmap is only used by early printk */
>          /* Add UART to the fixmap table */
>          ldr   r1, =xen_fixmap        /* r1 := vaddr (xen_fixmap) */
> @@ -489,7 +551,6 @@ setup_fixmap:
>          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 */
> -1:
>  
>          /* Map fixmap into boot_second */
>          ldr   r1, =boot_second       /* r1 := vaddr (boot_second) */
> @@ -501,19 +562,10 @@ setup_fixmap:
>          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 */
> -#endif
> -
> -        /*
> -         * Flush the TLB in case the 1:1 mapping happens to clash with
> -         * the virtual addresses used by the fixmap or DTB.
> -         */
> -        dsb                          /* Ensure any page table updates made above
> -                                      * have occurred. */
>  
> -        isb
> -        mcr   CP32(r0, TLBIALLH)     /* Flush hypervisor TLB */
> -        dsb                          /* Ensure completion of TLB flush */
> -        isb
> +        /* Ensure any page table updates made above have occurred. */
> +        dsb   nshst

same here


> +#endif
>          mov   pc, lr
>  ENDPROC(setup_fixmap)
>  
> -- 
> 2.11.0
>
Julien Grall Aug. 22, 2019, 6:31 p.m. | #2
Hi Stefano,

On 8/22/19 7:17 PM, Stefano Stabellini wrote:
> On Mon, 12 Aug 2019, Julien Grall wrote:
>> The 1:1 mapping may clash with other parts of the Xen virtual memory
>> layout. At the moment, Xen is handling the clash by only creating a
>> mapping to the runtime virtual address before enabling the MMU.
>>
>> The rest of the mappings (such as the fixmap) will be mapped after the
>> MMU is enabled. However, the code doing the mapping is not safe as it
>> replace mapping without using the Break-Before-Make sequence.
>>
>> As the 1:1 mapping can be anywhere in the memory, it is easier to remove
>> all the entries added as soon as the 1:1 mapping is not used rather than
>> adding the Break-Before-Make sequence everywhere.
>>
>> It is difficult to track where exactly the 1:1 mapping was created
>> without a full rework of create_page_tables(). Instead, introduce a new
>> function remove_identity_mapping() will look where is the top-level entry
>> for the 1:1 mapping and remove it.
>>
>> The new function is only called for the boot CPU. Secondary CPUs will
>> switch directly to the runtime page-tables so there are no need to
>> remove the 1:1 mapping. Note that this still doesn't make the Secondary
>> CPUs path safe but it is not making it worst.
>>
>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>>
>> ---
>>      It is very likely we will need to re-introduce the 1:1 mapping to cater
>>      secondary CPUs boot and suspend/resume. For now, the attempt is to make
>>      boot CPU path fully Arm Arm compliant.
>>
>>      Changes in v3:
>>          - Remove unused label
>>          - Avoid harcoding slots
>>
>>      Changes in v2:
>>          - Patch added
>> ---
>>   xen/arch/arm/arm32/head.S | 86 +++++++++++++++++++++++++++++++++++++----------
>>   1 file changed, 69 insertions(+), 17 deletions(-)
>>
>> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
>> index 8f945d318a..34fc9ffcee 100644
>> --- a/xen/arch/arm/arm32/head.S
>> +++ b/xen/arch/arm/arm32/head.S
>> @@ -32,6 +32,10 @@
>>   #define PT_UPPER(x) (PT_##x & 0xf00)
>>   #define PT_LOWER(x) (PT_##x & 0x0ff)
>>   
>> +/* Convenience defines to get slot used by Xen mapping. */
>> +#define XEN_FIRST_SLOT      first_table_offset(XEN_VIRT_START)
>> +#define XEN_SECOND_SLOT     second_table_offset(XEN_VIRT_START)
>> +
>>   #if (defined (CONFIG_EARLY_PRINTK)) && (defined (EARLY_PRINTK_INC))
>>   #include EARLY_PRINTK_INC
>>   #endif
>> @@ -158,6 +162,13 @@ past_zImage:
>>           ldr   r0, =primary_switched
>>           mov   pc, r0
>>   primary_switched:
>> +        /*
>> +         * The 1:1 map may clash with other parts of the Xen virtual memory
>> +         * layout. As it is not used anymore, remove it completely to
>> +         * avoid having to worry about replacing existing mapping
>> +         * afterwards.
>> +         */
>> +        bl    remove_identity_mapping
>>           bl    setup_fixmap
>>   #ifdef CONFIG_EARLY_PRINTK
>>           /* Use a virtual address to access the UART. */
>> @@ -474,12 +485,63 @@ enable_mmu:
>>           mov   pc, lr
>>   ENDPROC(enable_mmu)
>>   
>> -setup_fixmap:
>> +/*
>> + * Remove the 1:1 map for the page-tables. It is not easy to keep track
>> + * where the 1:1 map was mapped, so we will look for the top-level entry
>> + * exclusive to the 1:1 map and remove it.
>> + *
>> + * Inputs:
>> + *   r9 : paddr(start)
>> + *
>> + * Clobbers r0 - r3
>> + */
>> +remove_identity_mapping:
>> +        /* r2:r3 := invalid page-table entry */
>> +        mov   r2, #0x0
>> +        mov   r3, #0x0
>>           /*
>> -         * Now we can install the fixmap and dtb mappings, since we
>> -         * don't need the 1:1 map any more
>> +         * Find the first slot used. Remove the entry for the first
>> +         * table if the slot is not XEN_FIRST_SLOT. For slot XEN_FIRST_SLOT,
>> +         * the 1:1 mapping was done in the second table.
> 
> May I suggest something more similar to the arm64 version as a comment?
> Something like:
> 
> "Find the first slot used. Remove the entry for the first table if the
> slot is not XEN_FIRST_SLOT. In case of XEN_FIRST_SLOT, we have to look
> at the second table as the slot is shared with the XEN_VIRT_START
> mapping."

I have answered that on the arm64 version. Let's continue the 
conversation there.

> 
> 
>>            */
>> -        dsb
>> +        lsr   r1, r9, #FIRST_SHIFT
>> +        mov_w r0, LPAE_ENTRY_MASK
> 
> ldr?

What's wrong with the mov_w? Ok it is two instructions but... the 
constant will be stored in a literal and therefore induce a memory load 
(see patch #8).


> 
> 
>> +        and  r1, r1, r0              /* r1 := first slot */
>> +        cmp  r1, #XEN_FIRST_SLOT
> 
> NIT: align

align of?

> 
> 
>> +        beq   1f
>> +        /* It is not in slot 0, remove the entry */
>> +        ldr   r0, =boot_pgtable      /* r0 := root table */
>> +        lsl   r1, r1, #3             /* r1 := Slot offset */
>> +        strd  r2, r3, [r0, r1]
>> +        b     identity_mapping_removed
>> +
>> +1:
>> +        /*
>> +         * Find the second slot used. Remove the entry for the first
>> +         * table if the slot is not XEN_SECOND_SLOT. For slot XEN_SECOND_SLOT,
>> +         * it means the 1:1 mapping was not created.
>> +         */
>> +        lsr   r1, r9, #SECOND_SHIFT
>> +        mov_w r0, LPAE_ENTRY_MASK
>> +        and   r1, r1, r0             /* r1 := second slot */
>> +        cmp   r1, #XEN_SECOND_SLOT
>> +        beq   identity_mapping_removed
>> +        /* It is not in slot 1, remove the entry */
>> +        ldr   r0, =boot_second       /* r0 := second table */
>> +        lsl   r1, r1, #3             /* r1 := Slot offset */
>> +        strd  r2, r3, [r0, r1]
>> +
>> +identity_mapping_removed:
>> +        /* See asm-arm/arm32/flushtlb.h for the explanation of the sequence. */
>> +        dsb   nshst
>> +        mcr   CP32(r0, TLBIALLH)
>> +        dsb   nsh
>> +        isb
> 
> As for the arm64 patch, I would like to understand dsb ishst vs. dsb
> nshst.


I have answered that on the arm64 version. Let's continue the 
conversation there.

> 
> 
>> +        mov   pc, lr
>> +ENDPROC(remove_identity_mapping)
>> +
>> +setup_fixmap:
>>   #if defined(CONFIG_EARLY_PRINTK) /* Fixmap is only used by early printk */
>>           /* Add UART to the fixmap table */
>>           ldr   r1, =xen_fixmap        /* r1 := vaddr (xen_fixmap) */
>> @@ -489,7 +551,6 @@ setup_fixmap:
>>           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 */
>> -1:
>>   
>>           /* Map fixmap into boot_second */
>>           ldr   r1, =boot_second       /* r1 := vaddr (boot_second) */
>> @@ -501,19 +562,10 @@ setup_fixmap:
>>           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 */
>> -#endif
>> -
>> -        /*
>> -         * Flush the TLB in case the 1:1 mapping happens to clash with
>> -         * the virtual addresses used by the fixmap or DTB.
>> -         */
>> -        dsb                          /* Ensure any page table updates made above
>> -                                      * have occurred. */
>>   
>> -        isb
>> -        mcr   CP32(r0, TLBIALLH)     /* Flush hypervisor TLB */
>> -        dsb                          /* Ensure completion of TLB flush */
>> -        isb
>> +        /* Ensure any page table updates made above have occurred. */
>> +        dsb   nshst
> 
> same here


I have answered that on the arm64 version. Let's continue the 
conversation there.

> 
> 
>> +#endif
>>           mov   pc, lr
>>   ENDPROC(setup_fixmap)
>>   
>> -- 
>> 2.11.0
>>

Cheers,
Stefano Stabellini Aug. 22, 2019, 10:53 p.m. | #3
On Thu, 22 Aug 2019, Julien Grall wrote:
> > >            */
> > > -        dsb
> > > +        lsr   r1, r9, #FIRST_SHIFT
> > > +        mov_w r0, LPAE_ENTRY_MASK
> > 
> > ldr?
> 
> What's wrong with the mov_w? Ok it is two instructions but... the constant
> will be stored in a literal and therefore induce a memory load (see patch #8).

I am just wondering why you would choose mov_w when you can just do a
single simple ldr.
 

> > 
> > > +        and  r1, r1, r0              /* r1 := first slot */
> > > +        cmp  r1, #XEN_FIRST_SLOT
> > 
> > NIT: align
> 
> align of?

This is just a code style thing. It doesn't show here but we usually
have 3 spaces between the command the the register, while here there are
only 2.
Julien Grall Aug. 22, 2019, 11:39 p.m. | #4
Sorry for the formatting.

On Thu, 22 Aug 2019, 23:55 Stefano Stabellini, <sstabellini@kernel.org>
wrote:

> On Thu, 22 Aug 2019, Julien Grall wrote:

> > > >            */

> > > > -        dsb

> > > > +        lsr   r1, r9, #FIRST_SHIFT

> > > > +        mov_w r0, LPAE_ENTRY_MASK

> > >

> > > ldr?

> >

> > What's wrong with the mov_w? Ok it is two instructions but... the

> constant

> > will be stored in a literal and therefore induce a memory load (see

> patch #8).

>

> I am just wondering why you would choose mov_w when you can just do a

> single simple ldr.

>


Well, I have just explained it and you likely saw the explanation in patch
#8 as you acked it. So I am not sure what other explanation you are looking
for.

The choice between ldr rX, =... and mov_w is pretty much a matter of taste
from our perspective.

They will both take 64-bit in memory, the former because of one instruction
and the constant stored in the literal pool. The latter because it is using
two instructions.

Technically, we could also reduce the number of instructions to one for
mov_w depending on the constant size. But that's micro-optimization.

mov_w will be slightly more efficient because you don't have an extra
memory load (from loading from a pool). Although it is pretty much
insignificant here.

I also have to admit that the syntax for ldr is slightly confusing. I
always have to look up in other place how it can be used and works.

So I would like to keep the ldr use to the strict minimum, i.e. for
non-cons value (such as symbol).

Cheers,
<div dir="auto"><div>Sorry for the formatting.<br><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, 22 Aug 2019, 23:55 Stefano Stabellini, &lt;<a href="mailto:sstabellini@kernel.org">sstabellini@kernel.org</a>&gt; wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Thu, 22 Aug 2019, Julien Grall wrote:<br>
&gt; &gt; &gt;            */<br>
&gt; &gt; &gt; -        dsb<br>
&gt; &gt; &gt; +        lsr   r1, r9, #FIRST_SHIFT<br>
&gt; &gt; &gt; +        mov_w r0, LPAE_ENTRY_MASK<br>
&gt; &gt; <br>
&gt; &gt; ldr?<br>
&gt; <br>
&gt; What&#39;s wrong with the mov_w? Ok it is two instructions but... the constant<br>
&gt; will be stored in a literal and therefore induce a memory load (see patch #8).<br>
<br>
I am just wondering why you would choose mov_w when you can just do a<br>
single simple ldr.<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">Well, I have just explained it and you likely saw the explanation in patch #8 as you acked it. So I am not sure what other explanation you are looking for.</div><div dir="auto"><br></div><div dir="auto">The choice between ldr rX, =... and mov_w is pretty much a matter of taste from our perspective.</div><div dir="auto"><br></div><div dir="auto">They will both take 64-bit in memory, the former because of one instruction and the constant stored in the literal pool. The latter because it is using two instructions.</div><div dir="auto"><br></div><div dir="auto">Technically, we could also reduce the number of instructions to one for mov_w depending on the constant size. But that&#39;s micro-optimization.</div><div dir="auto"><br></div><div dir="auto">mov_w will be slightly more efficient because you don&#39;t have an extra memory load (from loading from a pool). Although it is pretty much insignificant here.</div><div dir="auto"><br></div><div dir="auto">I also have to admit that the syntax for ldr is slightly confusing. I always have to look up in other place how it can be used and works.</div><div dir="auto"><br></div><div dir="auto">So I would like to keep the ldr use to the strict minimum, i.e. for non-cons value (such as symbol).</div><div dir="auto"><br></div><div dir="auto">Cheers,</div></div>

Patch

diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
index 8f945d318a..34fc9ffcee 100644
--- a/xen/arch/arm/arm32/head.S
+++ b/xen/arch/arm/arm32/head.S
@@ -32,6 +32,10 @@ 
 #define PT_UPPER(x) (PT_##x & 0xf00)
 #define PT_LOWER(x) (PT_##x & 0x0ff)
 
+/* Convenience defines to get slot used by Xen mapping. */
+#define XEN_FIRST_SLOT      first_table_offset(XEN_VIRT_START)
+#define XEN_SECOND_SLOT     second_table_offset(XEN_VIRT_START)
+
 #if (defined (CONFIG_EARLY_PRINTK)) && (defined (EARLY_PRINTK_INC))
 #include EARLY_PRINTK_INC
 #endif
@@ -158,6 +162,13 @@  past_zImage:
         ldr   r0, =primary_switched
         mov   pc, r0
 primary_switched:
+        /*
+         * The 1:1 map may clash with other parts of the Xen virtual memory
+         * layout. As it is not used anymore, remove it completely to
+         * avoid having to worry about replacing existing mapping
+         * afterwards.
+         */
+        bl    remove_identity_mapping
         bl    setup_fixmap
 #ifdef CONFIG_EARLY_PRINTK
         /* Use a virtual address to access the UART. */
@@ -474,12 +485,63 @@  enable_mmu:
         mov   pc, lr
 ENDPROC(enable_mmu)
 
-setup_fixmap:
+/*
+ * Remove the 1:1 map for the page-tables. It is not easy to keep track
+ * where the 1:1 map was mapped, so we will look for the top-level entry
+ * exclusive to the 1:1 map and remove it.
+ *
+ * Inputs:
+ *   r9 : paddr(start)
+ *
+ * Clobbers r0 - r3
+ */
+remove_identity_mapping:
+        /* r2:r3 := invalid page-table entry */
+        mov   r2, #0x0
+        mov   r3, #0x0
         /*
-         * Now we can install the fixmap and dtb mappings, since we
-         * don't need the 1:1 map any more
+         * Find the first slot used. Remove the entry for the first
+         * table if the slot is not XEN_FIRST_SLOT. For slot XEN_FIRST_SLOT,
+         * the 1:1 mapping was done in the second table.
          */
-        dsb
+        lsr   r1, r9, #FIRST_SHIFT
+        mov_w r0, LPAE_ENTRY_MASK
+        and  r1, r1, r0              /* r1 := first slot */
+        cmp  r1, #XEN_FIRST_SLOT
+        beq   1f
+        /* It is not in slot 0, remove the entry */
+        ldr   r0, =boot_pgtable      /* r0 := root table */
+        lsl   r1, r1, #3             /* r1 := Slot offset */
+        strd  r2, r3, [r0, r1]
+        b     identity_mapping_removed
+
+1:
+        /*
+         * Find the second slot used. Remove the entry for the first
+         * table if the slot is not XEN_SECOND_SLOT. For slot XEN_SECOND_SLOT,
+         * it means the 1:1 mapping was not created.
+         */
+        lsr   r1, r9, #SECOND_SHIFT
+        mov_w r0, LPAE_ENTRY_MASK
+        and   r1, r1, r0             /* r1 := second slot */
+        cmp   r1, #XEN_SECOND_SLOT
+        beq   identity_mapping_removed
+        /* It is not in slot 1, remove the entry */
+        ldr   r0, =boot_second       /* r0 := second table */
+        lsl   r1, r1, #3             /* r1 := Slot offset */
+        strd  r2, r3, [r0, r1]
+
+identity_mapping_removed:
+        /* See asm-arm/arm32/flushtlb.h for the explanation of the sequence. */
+        dsb   nshst
+        mcr   CP32(r0, TLBIALLH)
+        dsb   nsh
+        isb
+
+        mov   pc, lr
+ENDPROC(remove_identity_mapping)
+
+setup_fixmap:
 #if defined(CONFIG_EARLY_PRINTK) /* Fixmap is only used by early printk */
         /* Add UART to the fixmap table */
         ldr   r1, =xen_fixmap        /* r1 := vaddr (xen_fixmap) */
@@ -489,7 +551,6 @@  setup_fixmap:
         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 */
-1:
 
         /* Map fixmap into boot_second */
         ldr   r1, =boot_second       /* r1 := vaddr (boot_second) */
@@ -501,19 +562,10 @@  setup_fixmap:
         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 */
-#endif
-
-        /*
-         * Flush the TLB in case the 1:1 mapping happens to clash with
-         * the virtual addresses used by the fixmap or DTB.
-         */
-        dsb                          /* Ensure any page table updates made above
-                                      * have occurred. */
 
-        isb
-        mcr   CP32(r0, TLBIALLH)     /* Flush hypervisor TLB */
-        dsb                          /* Ensure completion of TLB flush */
-        isb
+        /* Ensure any page table updates made above have occurred. */
+        dsb   nshst
+#endif
         mov   pc, lr
 ENDPROC(setup_fixmap)