diff mbox series

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

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

Commit Message

Julien Grall Jan. 16, 2024, 11:55 a.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. 16, 2024, 2:24 p.m. UTC | #1 
Hi Julien,

On Tue, Jan 16, 2024 at 12:55 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)

Isn't this function not present in the patch series?

>  {
>      /*
>       * 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);
>
> --
> 2.40.1
>

Thank you very much for your help.
Julien Grall Jan. 16, 2024, 2:35 p.m. UTC | #2 
On 16/01/2024 14:24, Carlo Nonato wrote:
> Hi Julien,
> 
> On Tue, Jan 16, 2024 at 12:55 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)
> 
> Isn't this function not present in the patch series?

Oh. It looks like I forgot to post one patch. Let me resend it.

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);