diff mbox series

[RFC,v2,04/12] xen/arm32: head: Remove restriction where to load Xen

Message ID 20221022150422.17707-5-julien@xen.org (mailing list archive)
State Superseded
Headers show
Series xen/arm: Don't switch TTBR while the MMU is on | expand

Commit Message

Julien Grall Oct. 22, 2022, 3:04 p.m. UTC
From: Julien Grall <jgrall@amazon.com>

At the moment, bootloaders can load Xen anywhere in memory but the
region 2MB - 4MB. While I am not aware of any issue, we have no way
to tell the bootloader to avoid that region.

In addition to that, in the future, Xen may grow over 2MB if we
enable feature like UBSAN or GCOV. To avoid widening the restriction
on the load address, it would be better to get rid of it.

When the identity mapping is clashing with the Xen runtime mapping,
we need an extra indirection to be able to replace the identity
mapping with the Xen runtime mapping.

Reserve a new memory region that will be used to temporarily map Xen.
For convenience, the new area is re-using the same first slot as the
domheap which is used for per-cpu temporary mapping after a CPU has
booted.

Furthermore, directly map boot_second (which cover Xen and more)
to the temporary area. This will avoid to allocate an extra page-table
for the second-level and will helpful for follow-up patches (we will
want to use the fixmap whilst in the temporary mapping).

Lastly, some part of the code now needs to know whether the temporary
mapping was created. So reserve r12 to store this information.

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

    Changes in v2:
        - Patch added
---
 xen/arch/arm/arm32/head.S         | 139 ++++++++++++++++++++++++++----
 xen/arch/arm/domain_page.c        |   9 ++
 xen/arch/arm/include/asm/config.h |  14 +++
 xen/arch/arm/mm.c                 |  14 +++
 4 files changed, 161 insertions(+), 15 deletions(-)

Comments

Luca Fancellu Oct. 25, 2022, 11:56 a.m. UTC | #1
> On 22 Oct 2022, at 16:04, Julien Grall <julien@xen.org> wrote:
> 
> From: Julien Grall <jgrall@amazon.com>
> 
> At the moment, bootloaders can load Xen anywhere in memory but the
> region 2MB - 4MB. While I am not aware of any issue, we have no way
> to tell the bootloader to avoid that region.
> 
> In addition to that, in the future, Xen may grow over 2MB if we
> enable feature like UBSAN or GCOV. To avoid widening the restriction
> on the load address, it would be better to get rid of it.
> 
> When the identity mapping is clashing with the Xen runtime mapping,
> we need an extra indirection to be able to replace the identity
> mapping with the Xen runtime mapping.
> 
> Reserve a new memory region that will be used to temporarily map Xen.
> For convenience, the new area is re-using the same first slot as the
> domheap which is used for per-cpu temporary mapping after a CPU has
> booted.
> 
> Furthermore, directly map boot_second (which cover Xen and more)
> to the temporary area. This will avoid to allocate an extra page-table
> for the second-level and will helpful for follow-up patches (we will
> want to use the fixmap whilst in the temporary mapping).
> 
> Lastly, some part of the code now needs to know whether the temporary
> mapping was created. So reserve r12 to store this information.
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>
> ----
> 
>    Changes in v2:
>        - Patch added
> ---

Hi Julien,

I’m hitting an assert with this one, tested on qemu and fvp:

Xen 4.17-rc
(XEN) Xen version 4.17-rc (user@hostname) (arm-poky-linux-gnueabi-gcc (GCC) 11.3.0) debug=y Tue Oct 25 10:51:06 UTC 2022
(XEN) Latest ChangeSet:
(XEN) build-id: ab143b13f4394ced5331d6ff1cedebdb2ffadc07
(XEN) Processor: 412fc0f1: "ARM Limited", variant: 0x2, part 0xc0f,rev 0x1
(XEN) 32-bit Execution:
(XEN)   Processor Features: 00001131:00011001
(XEN)     Instruction Sets: AArch32 A32 Thumb Thumb-2 ThumbEE Jazelle
(XEN)     Extensions: GenericTimer
(XEN)   Debug Features: 02010555
(XEN)   Auxiliary Features: 00000000
(XEN)   Memory Model Features: 10201105 20000000
(XEN)                          01240000 02102211
(XEN)   ISA Features: 02101110 13112111 21232041
(XEN)                 11112131 10011142 00000000
(XEN) Using SMC Calling Convention v1.0
(XEN) Using PSCI v0.2
(XEN) SMP: Allowing 4 CPUs
(XEN) Generic Timer IRQ: phys=30 hyp=26 virt=27 Freq: 62500 KHz
(XEN) GICv2m extension register frame:
(XEN)         gic_v2m_addr=0000000008020000
(XEN)         gic_v2m_size=0000000000001000
(XEN)         gic_v2m_spi_base=80
(XEN)         gic_v2m_num_spis=64
(XEN) GICv2 initialization:
(XEN)         gic_dist_addr=0000000008000000
(XEN)         gic_cpu_addr=0000000008010000
(XEN)         gic_hyp_addr=0000000008030000
(XEN)         gic_vcpu_addr=0000000008040000
(XEN)         gic_maintenance_irq=25
(XEN) GICv2: 288 lines, 4 cpus (IID 00000000).
(XEN) XSM Framework v1.0.1 initialized
(XEN) Initialising XSM SILO mode
(XEN) Using scheduler: SMP Credit Scheduler rev2 (credit2)
(XEN) Initializing Credit2 scheduler
(XEN)  load_precision_shift: 18
(XEN)  load_window_shift: 30
(XEN)  underload_balance_tolerance: 0
(XEN)  overload_balance_tolerance: -3
(XEN)  runqueues arrangement: socket
(XEN)  cap enforcement granularity: 10ms
(XEN) load tracking window length 1073741824 ns
(XEN) Allocated console ring of 32 KiB.
(XEN) VFP implementer 0x41 architecture 4 part 0x30 variant 0xf rev 0x0
(XEN) CPU0: Guest atomics will try 1 times before pausing the domain
(XEN) Bringing up CPU1
(XEN) Assertion '!lpae_is_valid(*entry)' failed at arch/arm/domain_page.c:69
(XEN) ----[ Xen-4.17-rc  arm32  debug=y  Not tainted ]----
(XEN) CPU:    0
(XEN) PC:     00269af4 init_domheap_mappings+0x178/0x17c
(XEN) CPSR:   2000005a MODE:Hypervisor
(XEN)      R0: 002ffe48 R1: 0000007f R2: bfff8f7d R3: 00400000
(XEN)      R4: 000bfff8 R5: 43ffa010 R6: 00000001 R7: 43ffa000
(XEN)      R8: 43ff8000 R9: 00000000 R10:00000000 R11:002ffe6c R12:000bfff8
(XEN) HYP: SP: 002ffe44 LR: 00269a2c
(XEN)
(XEN)   VTCR_EL2: 00000000
(XEN)  VTTBR_EL2: 0000000000000000
(XEN)
(XEN)  SCTLR_EL2: 30cd187f
(XEN)    HCR_EL2: 00000038
(XEN)  TTBR0_EL2: 000000004014a000
(XEN)
(XEN)    ESR_EL2: 00000000
(XEN)  HPFAR_EL2: 00000000
(XEN)      HDFAR: 00000000
(XEN)      HIFAR: 00000000
(XEN)
(XEN) Xen stack trace from sp=002ffe44:
(XEN)    00000000 bfff8f7d 00400000 002ffe6c 43ffa000 00000001 002a6000 0033b224
(XEN)    002a2ec8 002ffe84 002700cc 00000001 00000000 002a6000 0033b224 002ffeb4
(XEN)    00276338 00000001 002a6000 0033b224 00000001 00000000 002a6000 0033b224
(XEN)    002a2ec8 00000000 00000000 002ffecc 0020185c 002ffecc 002ad978 00000001
(XEN)    00000001 002fff54 002c7d80 11112131 10011142 00000000 002a4744 00000000
(XEN)    00000000 00008f20 00004000 48000000 002e01f0 00000000 c0000000 00000000
(XEN)    40000000 00000001 002e0208 002a6e84 002a6e80 002afaa0 002e31f0 00000000
(XEN)    c0000000 00000003 ffffffff 00000000 002a9000 0020005c 00000000 00000000
(XEN)    48000000 40010000 3fe10000 00000000 00200064 00000000 00000000 00000000
(XEN)    00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
(XEN)    00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
(XEN)    00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
(XEN)    00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
(XEN)    00000000 00000000 00000000 00000000 00000000 00000000 00000000
(XEN) Xen call trace:
(XEN)    [<00269af4>] init_domheap_mappings+0x178/0x17c (PC)
(XEN)    [<00269a2c>] init_domheap_mappings+0xb0/0x17c (LR)
(XEN)    [<002700cc>] init_secondary_pagetables+0x60/0x168
(XEN)    [<00276338>] __cpu_up+0x38/0x1d8
(XEN)    [<0020185c>] cpu_up+0xe0/0x114
(XEN)    [<002c7d80>] start_xen+0xd40/0x1138
(XEN)    [<00200064>] head.o#primary_switched+0x8/0x18
(XEN)
(XEN)
(XEN) ****************************************
(XEN) Panic on CPU 0:
(XEN) Assertion '!lpae_is_valid(*entry)' failed at arch/arm/domain_page.c:69
(XEN) ****************************************
(XEN)
(XEN) Reboot in five seconds...
Julien Grall Nov. 17, 2022, 8:18 p.m. UTC | #2
Hi Luca,

On 25/10/2022 12:56, Luca Fancellu wrote:
> 
> 
>> On 22 Oct 2022, at 16:04, Julien Grall <julien@xen.org> wrote:
>>
>> From: Julien Grall <jgrall@amazon.com>
>>
>> At the moment, bootloaders can load Xen anywhere in memory but the
>> region 2MB - 4MB. While I am not aware of any issue, we have no way
>> to tell the bootloader to avoid that region.
>>
>> In addition to that, in the future, Xen may grow over 2MB if we
>> enable feature like UBSAN or GCOV. To avoid widening the restriction
>> on the load address, it would be better to get rid of it.
>>
>> When the identity mapping is clashing with the Xen runtime mapping,
>> we need an extra indirection to be able to replace the identity
>> mapping with the Xen runtime mapping.
>>
>> Reserve a new memory region that will be used to temporarily map Xen.
>> For convenience, the new area is re-using the same first slot as the
>> domheap which is used for per-cpu temporary mapping after a CPU has
>> booted.
>>
>> Furthermore, directly map boot_second (which cover Xen and more)
>> to the temporary area. This will avoid to allocate an extra page-table
>> for the second-level and will helpful for follow-up patches (we will
>> want to use the fixmap whilst in the temporary mapping).
>>
>> Lastly, some part of the code now needs to know whether the temporary
>> mapping was created. So reserve r12 to store this information.
>>
>> Signed-off-by: Julien Grall <jgrall@amazon.com>
>> ----
>>
>>     Changes in v2:
>>         - Patch added
>> ---
> 
> Hi Julien,
> 
> I’m hitting an assert with this one, tested on qemu and fvp:

Thanks for testing the series!

> 
> Xen 4.17-rc
> (XEN) Xen version 4.17-rc (user@hostname) (arm-poky-linux-gnueabi-gcc (GCC) 11.3.0) debug=y Tue Oct 25 10:51:06 UTC 2022
> (XEN) Latest ChangeSet:
> (XEN) build-id: ab143b13f4394ced5331d6ff1cedebdb2ffadc07
> (XEN) Processor: 412fc0f1: "ARM Limited", variant: 0x2, part 0xc0f,rev 0x1
> (XEN) 32-bit Execution:
> (XEN)   Processor Features: 00001131:00011001
> (XEN)     Instruction Sets: AArch32 A32 Thumb Thumb-2 ThumbEE Jazelle
> (XEN)     Extensions: GenericTimer
> (XEN)   Debug Features: 02010555
> (XEN)   Auxiliary Features: 00000000
> (XEN)   Memory Model Features: 10201105 20000000
> (XEN)                          01240000 02102211
> (XEN)   ISA Features: 02101110 13112111 21232041
> (XEN)                 11112131 10011142 00000000
> (XEN) Using SMC Calling Convention v1.0
> (XEN) Using PSCI v0.2
> (XEN) SMP: Allowing 4 CPUs
> (XEN) Generic Timer IRQ: phys=30 hyp=26 virt=27 Freq: 62500 KHz
> (XEN) GICv2m extension register frame:
> (XEN)         gic_v2m_addr=0000000008020000
> (XEN)         gic_v2m_size=0000000000001000
> (XEN)         gic_v2m_spi_base=80
> (XEN)         gic_v2m_num_spis=64
> (XEN) GICv2 initialization:
> (XEN)         gic_dist_addr=0000000008000000
> (XEN)         gic_cpu_addr=0000000008010000
> (XEN)         gic_hyp_addr=0000000008030000
> (XEN)         gic_vcpu_addr=0000000008040000
> (XEN)         gic_maintenance_irq=25
> (XEN) GICv2: 288 lines, 4 cpus (IID 00000000).
> (XEN) XSM Framework v1.0.1 initialized
> (XEN) Initialising XSM SILO mode
> (XEN) Using scheduler: SMP Credit Scheduler rev2 (credit2)
> (XEN) Initializing Credit2 scheduler
> (XEN)  load_precision_shift: 18
> (XEN)  load_window_shift: 30
> (XEN)  underload_balance_tolerance: 0
> (XEN)  overload_balance_tolerance: -3
> (XEN)  runqueues arrangement: socket
> (XEN)  cap enforcement granularity: 10ms
> (XEN) load tracking window length 1073741824 ns
> (XEN) Allocated console ring of 32 KiB.
> (XEN) VFP implementer 0x41 architecture 4 part 0x30 variant 0xf rev 0x0
> (XEN) CPU0: Guest atomics will try 1 times before pausing the domain
> (XEN) Bringing up CPU1
> (XEN) Assertion '!lpae_is_valid(*entry)' failed at arch/arm/domain_page.c:69

So this is asserting because, so far, for secondary CPUs, we are copying 
the content of the CPU0 root table to the secondary CPU root table and 
then update the entry.

So the entry would logical be valid. This is fine to be valid because 
the root able is not yet live.

I have follow-up patches (not yet sent) where the root table for 
secondary CPUs would also be live. I probably mistakenly tested with 
those patches.

Anyway, the ASSERT() here doesn't make sense in the context of this 
patch because we are still switching the CPU0 root table. So I will drop 
the ASSERT() for now.

I will re-introduce it in a follow-up series.

Before I send a new version, do you have any comments for the rest of 
the patches?

Cheers,
Luca Fancellu Nov. 25, 2022, 3:50 p.m. UTC | #3
> On 17 Nov 2022, at 20:18, Julien Grall <julien@xen.org> wrote:
> 
> Hi Luca,
> 
> On 25/10/2022 12:56, Luca Fancellu wrote:
>>> On 22 Oct 2022, at 16:04, Julien Grall <julien@xen.org> wrote:
>>> 
>>> From: Julien Grall <jgrall@amazon.com>
>>> 
>>> At the moment, bootloaders can load Xen anywhere in memory but the
>>> region 2MB - 4MB. While I am not aware of any issue, we have no way
>>> to tell the bootloader to avoid that region.
>>> 
>>> In addition to that, in the future, Xen may grow over 2MB if we
>>> enable feature like UBSAN or GCOV. To avoid widening the restriction
>>> on the load address, it would be better to get rid of it.
>>> 
>>> When the identity mapping is clashing with the Xen runtime mapping,
>>> we need an extra indirection to be able to replace the identity
>>> mapping with the Xen runtime mapping.
>>> 
>>> Reserve a new memory region that will be used to temporarily map Xen.
>>> For convenience, the new area is re-using the same first slot as the
>>> domheap which is used for per-cpu temporary mapping after a CPU has
>>> booted.
>>> 
>>> Furthermore, directly map boot_second (which cover Xen and more)
>>> to the temporary area. This will avoid to allocate an extra page-table
>>> for the second-level and will helpful for follow-up patches (we will
>>> want to use the fixmap whilst in the temporary mapping).
>>> 
>>> Lastly, some part of the code now needs to know whether the temporary
>>> mapping was created. So reserve r12 to store this information.
>>> 
>>> Signed-off-by: Julien Grall <jgrall@amazon.com>
>>> ----
>>> 
>>>    Changes in v2:
>>>        - Patch added
>>> ---
>> Hi Julien,
>> I’m hitting an assert with this one, tested on qemu and fvp:
> 
> Thanks for testing the series!
> 
>> Xen 4.17-rc
>> (XEN) Xen version 4.17-rc (user@hostname) (arm-poky-linux-gnueabi-gcc (GCC) 11.3.0) debug=y Tue Oct 25 10:51:06 UTC 2022
>> (XEN) Latest ChangeSet:
>> (XEN) build-id: ab143b13f4394ced5331d6ff1cedebdb2ffadc07
>> (XEN) Processor: 412fc0f1: "ARM Limited", variant: 0x2, part 0xc0f,rev 0x1
>> (XEN) 32-bit Execution:
>> (XEN)   Processor Features: 00001131:00011001
>> (XEN)     Instruction Sets: AArch32 A32 Thumb Thumb-2 ThumbEE Jazelle
>> (XEN)     Extensions: GenericTimer
>> (XEN)   Debug Features: 02010555
>> (XEN)   Auxiliary Features: 00000000
>> (XEN)   Memory Model Features: 10201105 20000000
>> (XEN)                          01240000 02102211
>> (XEN)   ISA Features: 02101110 13112111 21232041
>> (XEN)                 11112131 10011142 00000000
>> (XEN) Using SMC Calling Convention v1.0
>> (XEN) Using PSCI v0.2
>> (XEN) SMP: Allowing 4 CPUs
>> (XEN) Generic Timer IRQ: phys=30 hyp=26 virt=27 Freq: 62500 KHz
>> (XEN) GICv2m extension register frame:
>> (XEN)         gic_v2m_addr=0000000008020000
>> (XEN)         gic_v2m_size=0000000000001000
>> (XEN)         gic_v2m_spi_base=80
>> (XEN)         gic_v2m_num_spis=64
>> (XEN) GICv2 initialization:
>> (XEN)         gic_dist_addr=0000000008000000
>> (XEN)         gic_cpu_addr=0000000008010000
>> (XEN)         gic_hyp_addr=0000000008030000
>> (XEN)         gic_vcpu_addr=0000000008040000
>> (XEN)         gic_maintenance_irq=25
>> (XEN) GICv2: 288 lines, 4 cpus (IID 00000000).
>> (XEN) XSM Framework v1.0.1 initialized
>> (XEN) Initialising XSM SILO mode
>> (XEN) Using scheduler: SMP Credit Scheduler rev2 (credit2)
>> (XEN) Initializing Credit2 scheduler
>> (XEN)  load_precision_shift: 18
>> (XEN)  load_window_shift: 30
>> (XEN)  underload_balance_tolerance: 0
>> (XEN)  overload_balance_tolerance: -3
>> (XEN)  runqueues arrangement: socket
>> (XEN)  cap enforcement granularity: 10ms
>> (XEN) load tracking window length 1073741824 ns
>> (XEN) Allocated console ring of 32 KiB.
>> (XEN) VFP implementer 0x41 architecture 4 part 0x30 variant 0xf rev 0x0
>> (XEN) CPU0: Guest atomics will try 1 times before pausing the domain
>> (XEN) Bringing up CPU1
>> (XEN) Assertion '!lpae_is_valid(*entry)' failed at arch/arm/domain_page.c:69
> 
> So this is asserting because, so far, for secondary CPUs, we are copying the content of the CPU0 root table to the secondary CPU root table and then update the entry.
> 
> So the entry would logical be valid. This is fine to be valid because the root able is not yet live.
> 
> I have follow-up patches (not yet sent) where the root table for secondary CPUs would also be live. I probably mistakenly tested with those patches.
> 
> Anyway, the ASSERT() here doesn't make sense in the context of this patch because we are still switching the CPU0 root table. So I will drop the ASSERT() for now.
> 
> I will re-introduce it in a follow-up series.
> 
> Before I send a new version, do you have any comments for the rest of the patches?

Hi Julien,

Yes as you pointed out, the assert was not right in that context and it can be removed without issues, I’ve had a look on the serie and the changes looks ok to me, I’ve also tested
that it works on arm64 and arm32 using FVP.

Cheers,
Luca

> 
> Cheers,
> 
> -- 
> Julien Grall
Julien Grall Dec. 6, 2022, 6:29 p.m. UTC | #4
Hi Luca,

On 25/11/2022 15:50, Luca Fancellu wrote:
> 
> 
>> On 17 Nov 2022, at 20:18, Julien Grall <julien@xen.org> wrote:
>>
>> Hi Luca,
>>
>> On 25/10/2022 12:56, Luca Fancellu wrote:
>>>> On 22 Oct 2022, at 16:04, Julien Grall <julien@xen.org> wrote:
>>>>
>>>> From: Julien Grall <jgrall@amazon.com>
>>>>
>>>> At the moment, bootloaders can load Xen anywhere in memory but the
>>>> region 2MB - 4MB. While I am not aware of any issue, we have no way
>>>> to tell the bootloader to avoid that region.
>>>>
>>>> In addition to that, in the future, Xen may grow over 2MB if we
>>>> enable feature like UBSAN or GCOV. To avoid widening the restriction
>>>> on the load address, it would be better to get rid of it.
>>>>
>>>> When the identity mapping is clashing with the Xen runtime mapping,
>>>> we need an extra indirection to be able to replace the identity
>>>> mapping with the Xen runtime mapping.
>>>>
>>>> Reserve a new memory region that will be used to temporarily map Xen.
>>>> For convenience, the new area is re-using the same first slot as the
>>>> domheap which is used for per-cpu temporary mapping after a CPU has
>>>> booted.
>>>>
>>>> Furthermore, directly map boot_second (which cover Xen and more)
>>>> to the temporary area. This will avoid to allocate an extra page-table
>>>> for the second-level and will helpful for follow-up patches (we will
>>>> want to use the fixmap whilst in the temporary mapping).
>>>>
>>>> Lastly, some part of the code now needs to know whether the temporary
>>>> mapping was created. So reserve r12 to store this information.
>>>>
>>>> Signed-off-by: Julien Grall <jgrall@amazon.com>
>>>> ----
>>>>
>>>>     Changes in v2:
>>>>         - Patch added
>>>> ---
>>> Hi Julien,
>>> I’m hitting an assert with this one, tested on qemu and fvp:
>>
>> Thanks for testing the series!
>>
>>> Xen 4.17-rc
>>> (XEN) Xen version 4.17-rc (user@hostname) (arm-poky-linux-gnueabi-gcc (GCC) 11.3.0) debug=y Tue Oct 25 10:51:06 UTC 2022
>>> (XEN) Latest ChangeSet:
>>> (XEN) build-id: ab143b13f4394ced5331d6ff1cedebdb2ffadc07
>>> (XEN) Processor: 412fc0f1: "ARM Limited", variant: 0x2, part 0xc0f,rev 0x1
>>> (XEN) 32-bit Execution:
>>> (XEN)   Processor Features: 00001131:00011001
>>> (XEN)     Instruction Sets: AArch32 A32 Thumb Thumb-2 ThumbEE Jazelle
>>> (XEN)     Extensions: GenericTimer
>>> (XEN)   Debug Features: 02010555
>>> (XEN)   Auxiliary Features: 00000000
>>> (XEN)   Memory Model Features: 10201105 20000000
>>> (XEN)                          01240000 02102211
>>> (XEN)   ISA Features: 02101110 13112111 21232041
>>> (XEN)                 11112131 10011142 00000000
>>> (XEN) Using SMC Calling Convention v1.0
>>> (XEN) Using PSCI v0.2
>>> (XEN) SMP: Allowing 4 CPUs
>>> (XEN) Generic Timer IRQ: phys=30 hyp=26 virt=27 Freq: 62500 KHz
>>> (XEN) GICv2m extension register frame:
>>> (XEN)         gic_v2m_addr=0000000008020000
>>> (XEN)         gic_v2m_size=0000000000001000
>>> (XEN)         gic_v2m_spi_base=80
>>> (XEN)         gic_v2m_num_spis=64
>>> (XEN) GICv2 initialization:
>>> (XEN)         gic_dist_addr=0000000008000000
>>> (XEN)         gic_cpu_addr=0000000008010000
>>> (XEN)         gic_hyp_addr=0000000008030000
>>> (XEN)         gic_vcpu_addr=0000000008040000
>>> (XEN)         gic_maintenance_irq=25
>>> (XEN) GICv2: 288 lines, 4 cpus (IID 00000000).
>>> (XEN) XSM Framework v1.0.1 initialized
>>> (XEN) Initialising XSM SILO mode
>>> (XEN) Using scheduler: SMP Credit Scheduler rev2 (credit2)
>>> (XEN) Initializing Credit2 scheduler
>>> (XEN)  load_precision_shift: 18
>>> (XEN)  load_window_shift: 30
>>> (XEN)  underload_balance_tolerance: 0
>>> (XEN)  overload_balance_tolerance: -3
>>> (XEN)  runqueues arrangement: socket
>>> (XEN)  cap enforcement granularity: 10ms
>>> (XEN) load tracking window length 1073741824 ns
>>> (XEN) Allocated console ring of 32 KiB.
>>> (XEN) VFP implementer 0x41 architecture 4 part 0x30 variant 0xf rev 0x0
>>> (XEN) CPU0: Guest atomics will try 1 times before pausing the domain
>>> (XEN) Bringing up CPU1
>>> (XEN) Assertion '!lpae_is_valid(*entry)' failed at arch/arm/domain_page.c:69
>>
>> So this is asserting because, so far, for secondary CPUs, we are copying the content of the CPU0 root table to the secondary CPU root table and then update the entry.
>>
>> So the entry would logical be valid. This is fine to be valid because the root able is not yet live.
>>
>> I have follow-up patches (not yet sent) where the root table for secondary CPUs would also be live. I probably mistakenly tested with those patches.
>>
>> Anyway, the ASSERT() here doesn't make sense in the context of this patch because we are still switching the CPU0 root table. So I will drop the ASSERT() for now.
>>
>> I will re-introduce it in a follow-up series.
>>
>> Before I send a new version, do you have any comments for the rest of the patches?
> 
> Hi Julien,
> 
> Yes as you pointed out, the assert was not right in that context and it can be removed without issues, I’ve had a look on the serie and the changes looks ok to me, I’ve also tested
> that it works on arm64 and arm32 using FVP.

Thanks! I will aim to respin the series this week.

Cheers,
diff mbox series

Patch

diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
index aeaa8d105aeb..54725c90993c 100644
--- a/xen/arch/arm/arm32/head.S
+++ b/xen/arch/arm/arm32/head.S
@@ -35,6 +35,9 @@ 
 #define XEN_FIRST_SLOT      first_table_offset(XEN_VIRT_START)
 #define XEN_SECOND_SLOT     second_table_offset(XEN_VIRT_START)
 
+/* Offset between the early boot xen mapping and the runtime xen mapping */
+#define XEN_TEMPORARY_OFFSET      (TEMPORARY_XEN_VIRT_START - XEN_VIRT_START)
+
 #if defined(CONFIG_EARLY_PRINTK) && defined(CONFIG_EARLY_PRINTK_INC)
 #include CONFIG_EARLY_PRINTK_INC
 #endif
@@ -95,7 +98,7 @@ 
  *   r9  - paddr(start)
  *   r10 - phys offset
  *   r11 - UART address
- *   r12 -
+ *   r12 - Temporary mapping created
  *   r13 - SP
  *   r14 - LR
  *   r15 - PC
@@ -447,6 +450,9 @@  ENDPROC(cpu_init)
  *   r9 : paddr(start)
  *   r10: phys offset
  *
+ * Output:
+ *   r12: Was a temporary mapping created?
+ *
  * Clobbers r0 - r4, r6
  *
  * Register usage within this function:
@@ -486,7 +492,11 @@  create_page_tables:
         /*
          * Setup the 1:1 mapping so we can turn the MMU on. Note that
          * only the first page of Xen will be part of the 1:1 mapping.
+         *
+         * In all the cases, we will link boot_third_id. So create the
+         * mapping in advance.
          */
+        create_mapping_entry boot_third_id, r9, r9
 
         /*
          * Find the first slot used. If the slot is not XEN_FIRST_SLOT,
@@ -503,8 +513,7 @@  create_page_tables:
         /*
          * Find the second slot used. If the slot is XEN_SECOND_SLOT, then the
          * 1:1 mapping will use its own set of page-tables from the
-         * third level. For slot XEN_SECOND_SLOT, Xen is not yet able to handle
-         * it.
+         * third level.
          */
         get_table_slot r1, r9, 2     /* r1 := second slot */
         cmp   r1, #XEN_SECOND_SLOT
@@ -515,13 +524,33 @@  create_page_tables:
 link_from_second_id:
         create_table_entry boot_second_id, boot_third_id, r9, 2
 link_from_third_id:
-        create_mapping_entry boot_third_id, r9, r9
+        /* Good news, we are not clashing with Xen virtual mapping */
+        mov   r12, #0                /* r12 := temporary mapping not created */
         mov   pc, lr
 
 virtphys_clash:
-        /* Identity map clashes with boot_third, which we cannot handle yet */
-        PRINT("- Unable to build boot page tables - virt and phys addresses clash. -\r\n")
-        b     fail
+        /*
+         * The identity map clashes with boot_third. Link boot_first_id and
+         * map Xen to a temporary mapping. See switch_to_runtime_mapping
+         * for more details.
+         */
+        PRINT("- Virt and Phys addresses clash  -\r\n")
+        PRINT("- Create temporary mapping -\r\n")
+
+        /*
+         * This will override the link to boot_second in XEN_FIRST_SLOT.
+         * The page-tables are not live yet. So no need to use
+         * break-before-make.
+         */
+        create_table_entry boot_pgtable, boot_second_id, r9, 1
+        create_table_entry boot_second_id, boot_third_id, r9, 2
+
+        /* Map boot_second (cover Xen mappings) to the temporary 1st slot */
+        mov_w r0, TEMPORARY_XEN_VIRT_START
+        create_table_entry boot_pgtable, boot_second, r0, 1
+
+        mov   r12, #1                /* r12 := temporary mapping created */
+        mov   pc, lr
 ENDPROC(create_page_tables)
 
 /*
@@ -530,9 +559,10 @@  ENDPROC(create_page_tables)
  *
  * Inputs:
  *   r9 : paddr(start)
+ *  r12 : Was the temporary mapping created?
  *   lr : Virtual address to return to
  *
- * Clobbers r0 - r3
+ * Clobbers r0 - r5
  */
 enable_mmu:
         PRINT("- Turning on paging -\r\n")
@@ -560,21 +590,79 @@  enable_mmu:
          * The MMU is turned on and we are in the 1:1 mapping. Switch
          * to the runtime mapping.
          */
-        mov_w r0, 1f
-        mov   pc, r0
+        mov   r5, lr                /* Save LR before overwritting it */
+        mov_w lr, 1f                /* Virtual address in the runtime mapping */
+        b     switch_to_runtime_mapping
 1:
+        mov   lr, r5                /* Restore LR */
         /*
-         * 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.
+         * At this point, either the 1:1 map or the temporary mapping
+         * will be present. The former may clash with other parts of the
+         * Xen virtual memory layout. As both of them are not used
+         * anymore, remove them completely to avoid having to worry
+         * about replacing existing mapping afterwards.
          *
          * On return this will jump to the virtual address requested by
          * the caller.
          */
-        b     remove_identity_mapping
+        teq   r12, #0
+        beq   remove_identity_mapping
+        b     remove_temporary_mapping
 ENDPROC(enable_mmu)
 
+/*
+ * Switch to the runtime mapping. The logic depends on whether the
+ * runtime virtual region is clashing with the physical address
+ *
+ *  - If it is not clashing, we can directly jump to the address in
+ *    the runtime mapping.
+ *  - If it is clashing, create_page_tables() would have mapped Xen to
+ *    a temporary virtual address. We need to switch to the temporary
+ *    mapping so we can remove the identity mapping and map Xen at the
+ *    correct position.
+ *
+ * Inputs
+ *    r9: paddr(start)
+ *   r12: Was a temporary mapping created?
+ *    lr: Address in the runtime mapping to jump to
+ *
+ * Clobbers r0 - r4
+ */
+switch_to_runtime_mapping:
+        /*
+         * Jump to the runtime mapping if the virt and phys are not
+         * clashing
+         */
+        teq   r12, #0
+        beq   ready_to_switch
+
+        /* We are still in the 1:1 mapping. Jump to the temporary Virtual address. */
+        mov_w r0, 1f
+        add   r0, r0, #XEN_TEMPORARY_OFFSET /* r0 := address in temporary mapping */
+        mov   pc, r0
+
+1:
+        /* Remove boot_second_id */
+        mov   r2, #0
+        mov   r3, #0
+        adr_l r0, boot_pgtable
+        get_table_slot r1, r9, 1            /* r1 := first slot */
+        lsl   r1, r1, #3                    /* r1 := first slot offset */
+        strd  r2, r3, [r0, r1]
+
+        flush_xen_tlb_local r0
+
+        /* Map boot_second into boot_pgtable */
+        mov_w r0, XEN_VIRT_START
+        create_table_entry boot_pgtable, boot_second, r0, 1
+
+        /* Ensure any page table updates are visible before continuing */
+        dsb   nsh
+
+ready_to_switch:
+        mov   pc, lr
+ENDPROC(switch_to_runtime_mapping)
+
 /*
  * Remove the 1:1 map from 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
@@ -620,6 +708,27 @@  identity_mapping_removed:
         mov   pc, lr
 ENDPROC(remove_identity_mapping)
 
+/*
+ * Remove the temporary mapping of Xen starting at TEMPORARY_XEN_VIRT_START.
+ *
+ * Clobbers r0 - r1
+ */
+remove_temporary_mapping:
+        /* r2:r3 := invalid page-table entry */
+        mov   r2, #0
+        mov   r3, #0
+
+        adr_l r0, boot_pgtable
+        mov_w r1, TEMPORARY_XEN_VIRT_START
+        get_table_slot r1, r1, 1     /* r1 := first slot */
+        lsl   r1, r1, #3             /* r1 := first slot offset */
+        strd  r2, r3, [r0, r1]
+
+        flush_xen_tlb_local r0
+
+        mov  pc, lr
+ENDPROC(remove_temporary_mapping)
+
 /*
  * Map the UART in the fixmap (when earlyprintk is used) and hook the
  * fixmap table in the page tables.
diff --git a/xen/arch/arm/domain_page.c b/xen/arch/arm/domain_page.c
index 71182575f95a..3c59db697f26 100644
--- a/xen/arch/arm/domain_page.c
+++ b/xen/arch/arm/domain_page.c
@@ -58,7 +58,16 @@  bool init_domheap_mappings(unsigned int cpu)
     first_idx = first_table_offset(DOMHEAP_VIRT_START);
     for ( i = 0; i < DOMHEAP_SECOND_PAGES; i++ )
     {
+        lpae_t *entry = &root[first_idx + i];
         lpae_t pte = mfn_to_xen_entry(mfn_add(mfn, i), MT_NORMAL);
+
+        /*
+         * The domheap is overlapping with the temporary Xen mapping and
+         * the boot code is responsible to remove the latter mapping.
+         * So we can avoid break-before-make here.
+         */
+        ASSERT(!lpae_is_valid(*entry));
+
         pte.pt.table = 1;
         write_pte(&root[first_idx + i], pte);
     }
diff --git a/xen/arch/arm/include/asm/config.h b/xen/arch/arm/include/asm/config.h
index 80d81f127f7e..f381c471f67a 100644
--- a/xen/arch/arm/include/asm/config.h
+++ b/xen/arch/arm/include/asm/config.h
@@ -148,6 +148,20 @@ 
 /* Number of domheap pagetable pages required at the second level (2MB mappings) */
 #define DOMHEAP_SECOND_PAGES (DOMHEAP_VIRT_SIZE >> FIRST_SHIFT)
 
+/*
+ * The temporary area is overlapping with the domheap area. This may
+ * be used to create an alias of the first slot containing Xen mappings
+ * when turning on/off the MMU.
+ */
+#define TEMPORARY_AREA_FIRST_SLOT    (first_table_offset(DOMHEAP_VIRT_START))
+
+/* Calculate the address in the temporary area */
+#define TEMPORARY_AREA_ADDR(addr)                           \
+     (((addr) & ~XEN_PT_LEVEL_MASK(1)) |                    \
+      (TEMPORARY_AREA_FIRST_SLOT << XEN_PT_LEVEL_SHIFT(1)))
+
+#define TEMPORARY_XEN_VIRT_START    TEMPORARY_AREA_ADDR(XEN_VIRT_START)
+
 #else /* ARM_64 */
 
 #define SLOT0_ENTRY_BITS  39
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 6ccffeaea57d..726211c77917 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -176,6 +176,9 @@  static void __init __maybe_unused build_assertions(void)
 #define CHECK_SAME_SLOT(level, virt1, virt2) \
     BUILD_BUG_ON(level##_table_offset(virt1) != level##_table_offset(virt2))
 
+#define CHECK_DIFFERENT_SLOT(level, virt1, virt2) \
+    BUILD_BUG_ON(level##_table_offset(virt1) == level##_table_offset(virt2))
+
 #ifdef CONFIG_ARM_64
     CHECK_SAME_SLOT(zeroeth, XEN_VIRT_START, FIXMAP_ADDR(0));
     CHECK_SAME_SLOT(zeroeth, XEN_VIRT_START, BOOT_FDT_VIRT_START);
@@ -183,7 +186,18 @@  static void __init __maybe_unused build_assertions(void)
     CHECK_SAME_SLOT(first, XEN_VIRT_START, FIXMAP_ADDR(0));
     CHECK_SAME_SLOT(first, XEN_VIRT_START, BOOT_FDT_VIRT_START);
 
+    /*
+     * For arm32, the temporary mapping will re-use the domheap
+     * first slot and the second slots will match.
+     */
+#ifdef CONFIG_ARM_32
+    CHECK_SAME_SLOT(first, TEMPORARY_XEN_VIRT_START, DOMHEAP_VIRT_START);
+    CHECK_DIFFERENT_SLOT(first, XEN_VIRT_START, TEMPORARY_XEN_VIRT_START);
+    CHECK_SAME_SLOT(second, XEN_VIRT_START, TEMPORARY_XEN_VIRT_START);
+#endif
+
 #undef CHECK_SAME_SLOT
+#undef CHECK_DIFFERENT_SLOT
 }
 
 void dump_pt_walk(paddr_t ttbr, paddr_t addr,