Hi Julien,
On 16/01/2024 15:37, Julien Grall wrote:
>
>
> From: Julien Grall <jgr...@amazon.com>
>
> With the upcoming work to color Xen, the binary will not be anymore
> physically contiguous. This will be a problem during boot as the
> assembly code will need to work out where each piece of Xen reside.
>
> An easy way to solve the issue is to have all code/data accessed
> by the secondary CPUs while the MMU is off within a single page.
>
> Right now, smp_up_cpu is used by secondary CPUs to wait there turn for
s/there/their ?
> booting before the MMU is on. Yet it is currently in .data which is
> unlikely to be within the same page as the rest of the idmap.
>
> Move smp_up_cpu to the recently create section .data.idmap. The idmap is
s/create/created
> currently part of the text section and therefore will be mapped read-onl
s/onl/only
> executable. This means that we need to temporarily remap
> smp_up_cpu in order to update it.
>
> Introduce a new function set_smp_up_cpu() for this purpose so the code
> is not duplicated between when opening and closing the gate.
>
> Signed-off-by: Julien Grall <jgr...@amazon.com>
Reviewed-by: Michal Orzel <michal.or...@amd.com>
> ---
> xen/arch/arm/smpboot.c | 36 +++++++++++++++++++++++++++++++-----
> 1 file changed, 31 insertions(+), 5 deletions(-)
>
> diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
> index 7110bc11fc05..8d508a1bb258 100644
> --- a/xen/arch/arm/smpboot.c
> +++ b/xen/arch/arm/smpboot.c
> @@ -29,6 +29,10 @@
> #include <asm/psci.h>
> #include <asm/acpi.h>
>
> +/* Override macros from asm/page.h to make them work with mfn_t */
> +#undef virt_to_mfn
> +#define virt_to_mfn(va) _mfn(__virt_to_mfn(va))
> +
> cpumask_t cpu_online_map;
> cpumask_t cpu_present_map;
> cpumask_t cpu_possible_map;
> @@ -56,7 +60,7 @@ struct init_info init_data =
> };
>
> /* Shared state for coordinating CPU bringup */
> -unsigned long smp_up_cpu = MPIDR_INVALID;
> +unsigned long __section(".data.idmap") smp_up_cpu = MPIDR_INVALID;
> /* Shared state for coordinating CPU teardown */
> static bool cpu_is_dead;
>
> @@ -429,6 +433,28 @@ void stop_cpu(void)
> wfi();
> }
>
> +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));
> +
> + ptr += PAGE_OFFSET(&smp_up_cpu);
> +
> + *(unsigned long *)ptr = mpidr;
> +
> + /*
> + * init_ttbr will be accessed with the MMU off, so ensure the update
smp_up_cpu instead of init_ttbr
> + * is visible by cleaning the cache.
> + */
> + clean_dcache(ptr);
> +
> + unmap_domain_page(ptr);
> +
> +}
> +
> int __init cpu_up_send_sgi(int cpu)
> {
> /* We don't know the GIC ID of the CPU until it has woken up, so just
> @@ -460,8 +486,7 @@ int __cpu_up(unsigned int cpu)
> init_data.cpuid = cpu;
>
> /* Open the gate for this CPU */
> - smp_up_cpu = cpu_logical_map(cpu);
> - clean_dcache(smp_up_cpu);
> + set_smp_up_cpu(cpu_logical_map(cpu));
>
> rc = arch_cpu_up(cpu);
>
> @@ -497,8 +522,9 @@ int __cpu_up(unsigned int cpu)
> */
> init_data.stack = NULL;
> init_data.cpuid = ~0;
> - smp_up_cpu = MPIDR_INVALID;
> - clean_dcache(smp_up_cpu);
> +
> + set_smp_up_cpu(MPIDR_INVALID);
> +
> arch_cpu_up_finish();
>
> if ( !cpu_online(cpu) )
> --
> 2.40.1
>
~Michal
