diff mbox series

[v1,repost,4/4,DO,NOT,COMMIT] xen/arm: Create a trampoline for secondary boot CPUs

Message ID 20240116143709.86584-5-julien@xen.org (mailing list archive)
State New
Headers show
Series xen/arm64: Rework the MMU-off code (idmap) so it is self-contained | expand

Commit Message

Julien Grall Jan. 16, 2024, 2:37 p.m. UTC 
From: Julien Grall <jgrall@amazon.com>

In order to confirm the early boot code is self-contained, allocate a
separate trampoline region for secondary to boot from it.

Signed-off-by: Julien Grall <jgrall@amazon.com>
---
 xen/arch/arm/arm64/mmu/mm.c |  7 +++++++
 xen/arch/arm/mmu/smpboot.c  |  4 +++-
 xen/arch/arm/psci.c         |  5 ++++-
 xen/arch/arm/smpboot.c      | 15 ++++++++++++++-
 4 files changed, 28 insertions(+), 3 deletions(-)

Comments

Carlo Nonato Jan. 17, 2024, 5:38 p.m. UTC | #1 
Hi Julien

On Tue, Jan 16, 2024 at 3:37 PM Julien Grall <julien@xen.org> wrote:
>
> From: Julien Grall <jgrall@amazon.com>
>
> In order to confirm the early boot code is self-contained, allocate a
> separate trampoline region for secondary to boot from it.
>
> Signed-off-by: Julien Grall <jgrall@amazon.com>
> ---
>  xen/arch/arm/arm64/mmu/mm.c |  7 +++++++
>  xen/arch/arm/mmu/smpboot.c  |  4 +++-
>  xen/arch/arm/psci.c         |  5 ++++-
>  xen/arch/arm/smpboot.c      | 15 ++++++++++++++-
>  4 files changed, 28 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/arm/arm64/mmu/mm.c b/xen/arch/arm/arm64/mmu/mm.c
> index d2651c948698..3c4988dc75d1 100644
> --- a/xen/arch/arm/arm64/mmu/mm.c
> +++ b/xen/arch/arm/arm64/mmu/mm.c
> @@ -110,11 +110,18 @@ void __init arch_setup_page_tables(void)
>      prepare_runtime_identity_mapping();
>  }
>
> +extern mfn_t trampoline_start;
> +
>  void update_identity_mapping(bool enable)
>  {
>      paddr_t id_addr = virt_to_maddr(_start);
>      int rc;
>
> +    if ( !mfn_eq(trampoline_start, INVALID_MFN) )
> +    {
> +        id_addr = mfn_to_maddr(trampoline_start);
> +    }
> +
>      if ( enable )
>          rc = map_pages_to_xen(id_addr, maddr_to_mfn(id_addr), 1,
>                                PAGE_HYPERVISOR_RX);
> diff --git a/xen/arch/arm/mmu/smpboot.c b/xen/arch/arm/mmu/smpboot.c
> index f1cf9252710c..d768dfe065a5 100644
> --- a/xen/arch/arm/mmu/smpboot.c
> +++ b/xen/arch/arm/mmu/smpboot.c
> @@ -72,13 +72,15 @@ static void clear_boot_pagetables(void)
>      clear_table(boot_third);
>  }
>
> +extern mfn_t trampoline_start;
> +
>  static void set_init_ttbr(lpae_t *root)
>  {
>      /*
>       * init_ttbr is part of the identity mapping which is read-only. So
>       * We need to re-map the region so it can be updated
>       */
> -    void *ptr = map_domain_page(virt_to_mfn(&init_ttbr));
> +    void *ptr = map_domain_page(trampoline_start);
>
>      ptr += PAGE_OFFSET(&init_ttbr);
>
> diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c
> index 695d2fa1f1b5..a00574d559d6 100644
> --- a/xen/arch/arm/psci.c
> +++ b/xen/arch/arm/psci.c
> @@ -36,11 +36,14 @@ static uint32_t psci_cpu_on_nr;
>
>  #define PSCI_RET(res)   ((int32_t)(res).a0)
>
> +extern mfn_t trampoline_start;
> +
>  int call_psci_cpu_on(int cpu)
>  {
>      struct arm_smccc_res res;
>
> -    arm_smccc_smc(psci_cpu_on_nr, cpu_logical_map(cpu), __pa(init_secondary),
> +    arm_smccc_smc(psci_cpu_on_nr, cpu_logical_map(cpu),
> +                  mfn_to_maddr(trampoline_start) + PAGE_OFFSET(init_secondary),
>                    &res);
>
>      return PSCI_RET(res);
> diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
> index 8d508a1bb258..ef84b73ebd46 100644
> --- a/xen/arch/arm/smpboot.c
> +++ b/xen/arch/arm/smpboot.c
> @@ -293,10 +293,13 @@ unsigned int __init smp_get_max_cpus(void)
>      return cpus;
>  }
>
> +mfn_t trampoline_start = INVALID_MFN_INITIALIZER;
> +
>  void __init
>  smp_prepare_cpus(void)
>  {
>      int rc;
> +    void *trampoline;
>
>      cpumask_copy(&cpu_present_map, &cpu_possible_map);
>
> @@ -304,6 +307,16 @@ smp_prepare_cpus(void)
>      if ( rc )
>          panic("Unable to allocate CPU sibling/core maps\n");
>
> +    /* Create a trampoline to confirm early boot code is self-contained */
> +    trampoline = alloc_xenheap_page();
> +    BUG_ON(!trampoline);
> +
> +    memcpy(trampoline, _start, PAGE_SIZE);
> +    clean_dcache_va_range(trampoline, PAGE_SIZE);
> +    invalidate_icache();
> +
> +    printk("Trampoline 0x%lx\n", virt_to_maddr(trampoline));

Here PRIx64 is needed for arm32.

> +    trampoline_start = virt_to_mfn(trampoline);
>  }
>
>  /* Boot the current CPU */
> @@ -439,7 +452,7 @@ static void set_smp_up_cpu(unsigned long mpidr)
>       * smp_up_cpu is part of the identity mapping which is read-only. So
>       * We need to re-map the region so it can be updated.
>       */
> -    void *ptr = map_domain_page(virt_to_mfn(&smp_up_cpu));
> +    void *ptr = map_domain_page(trampoline_start);
>
>      ptr += PAGE_OFFSET(&smp_up_cpu);
>
> --
> 2.40.1
>

Thanks.
Julien Grall Jan. 18, 2024, 9:25 a.m. UTC | #2 
On 17/01/2024 17:38, Carlo Nonato wrote:
> Hi Julien

Hi Carlo,

> On Tue, Jan 16, 2024 at 3:37 PM Julien Grall <julien@xen.org> wrote:
>>
>> From: Julien Grall <jgrall@amazon.com>
>>
>> In order to confirm the early boot code is self-contained, allocate a
>> separate trampoline region for secondary to boot from it.


[...]

>> @@ -304,6 +307,16 @@ smp_prepare_cpus(void)
>>       if ( rc )
>>           panic("Unable to allocate CPU sibling/core maps\n");
>>
>> +    /* Create a trampoline to confirm early boot code is self-contained */
>> +    trampoline = alloc_xenheap_page();
>> +    BUG_ON(!trampoline);
>> +
>> +    memcpy(trampoline, _start, PAGE_SIZE);
>> +    clean_dcache_va_range(trampoline, PAGE_SIZE);
>> +    invalidate_icache();
>> +
>> +    printk("Trampoline 0x%lx\n", virt_to_maddr(trampoline));
> 
> Here PRIx64 is needed for arm32.

Just for clarification, none of this patch is meant to work on Arm32. In 
fact, this would break secondary CPUs bring up.

This is why I tagged with DO NOT COMMIT. It was only included to show I 
tested this code on arm32.

Cheers,
diff mbox series

Patch

diff --git a/xen/arch/arm/arm64/mmu/mm.c b/xen/arch/arm/arm64/mmu/mm.c
index d2651c948698..3c4988dc75d1 100644
--- a/xen/arch/arm/arm64/mmu/mm.c
+++ b/xen/arch/arm/arm64/mmu/mm.c
@@ -110,11 +110,18 @@  void __init arch_setup_page_tables(void)
     prepare_runtime_identity_mapping();
 }
 
+extern mfn_t trampoline_start;
+
 void update_identity_mapping(bool enable)
 {
     paddr_t id_addr = virt_to_maddr(_start);
     int rc;
 
+    if ( !mfn_eq(trampoline_start, INVALID_MFN) )
+    {
+        id_addr = mfn_to_maddr(trampoline_start);
+    }
+
     if ( enable )
         rc = map_pages_to_xen(id_addr, maddr_to_mfn(id_addr), 1,
                               PAGE_HYPERVISOR_RX);
diff --git a/xen/arch/arm/mmu/smpboot.c b/xen/arch/arm/mmu/smpboot.c
index f1cf9252710c..d768dfe065a5 100644
--- a/xen/arch/arm/mmu/smpboot.c
+++ b/xen/arch/arm/mmu/smpboot.c
@@ -72,13 +72,15 @@  static void clear_boot_pagetables(void)
     clear_table(boot_third);
 }
 
+extern mfn_t trampoline_start;
+
 static void set_init_ttbr(lpae_t *root)
 {
     /*
      * init_ttbr is part of the identity mapping which is read-only. So
      * We need to re-map the region so it can be updated
      */
-    void *ptr = map_domain_page(virt_to_mfn(&init_ttbr));
+    void *ptr = map_domain_page(trampoline_start);
 
     ptr += PAGE_OFFSET(&init_ttbr);
 
diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c
index 695d2fa1f1b5..a00574d559d6 100644
--- a/xen/arch/arm/psci.c
+++ b/xen/arch/arm/psci.c
@@ -36,11 +36,14 @@  static uint32_t psci_cpu_on_nr;
 
 #define PSCI_RET(res)   ((int32_t)(res).a0)
 
+extern mfn_t trampoline_start;
+
 int call_psci_cpu_on(int cpu)
 {
     struct arm_smccc_res res;
 
-    arm_smccc_smc(psci_cpu_on_nr, cpu_logical_map(cpu), __pa(init_secondary),
+    arm_smccc_smc(psci_cpu_on_nr, cpu_logical_map(cpu),
+                  mfn_to_maddr(trampoline_start) + PAGE_OFFSET(init_secondary),
                   &res);
 
     return PSCI_RET(res);
diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
index 8d508a1bb258..ef84b73ebd46 100644
--- a/xen/arch/arm/smpboot.c
+++ b/xen/arch/arm/smpboot.c
@@ -293,10 +293,13 @@  unsigned int __init smp_get_max_cpus(void)
     return cpus;
 }
 
+mfn_t trampoline_start = INVALID_MFN_INITIALIZER;
+
 void __init
 smp_prepare_cpus(void)
 {
     int rc;
+    void *trampoline;
 
     cpumask_copy(&cpu_present_map, &cpu_possible_map);
 
@@ -304,6 +307,16 @@  smp_prepare_cpus(void)
     if ( rc )
         panic("Unable to allocate CPU sibling/core maps\n");
 
+    /* Create a trampoline to confirm early boot code is self-contained */
+    trampoline = alloc_xenheap_page();
+    BUG_ON(!trampoline);
+
+    memcpy(trampoline, _start, PAGE_SIZE);
+    clean_dcache_va_range(trampoline, PAGE_SIZE);
+    invalidate_icache();
+
+    printk("Trampoline 0x%lx\n", virt_to_maddr(trampoline));
+    trampoline_start = virt_to_mfn(trampoline);
 }
 
 /* Boot the current CPU */
@@ -439,7 +452,7 @@  static void set_smp_up_cpu(unsigned long mpidr)
      * smp_up_cpu is part of the identity mapping which is read-only. So
      * We need to re-map the region so it can be updated.
      */
-    void *ptr = map_domain_page(virt_to_mfn(&smp_up_cpu));
+    void *ptr = map_domain_page(trampoline_start);
 
     ptr += PAGE_OFFSET(&smp_up_cpu);