Re: [PATCH 27/36] xen/arch: add coloring support for Xen

[Julien Grall <julien@xxxxxxx>]

Hi,

On 04/03/2022 17:46, Marco Solieri wrote:
> From: Luca Miccio <lucmiccio@xxxxxxxxx>
>
> Introduce a new implementation of setup_pagetables that uses coloring
> logic in order to isolate Xen code using its color selection.
> Page tables construction is essentially copied, except for the xenmap
> table, where coloring logic is needed.  Given the absence of a contiguous
> physical mapping, pointers to next level tables need to be manually
> calculated.
The implementation of setup_pagetables() is not compliant to the Arm 
Arm. And I have plan to completely get rid of it.
The main part that is not compliant is switch_ttbr() because it keeps 
the MMU on. We should switch the MMU off, update the TTBR and then 
switch on the MMU. This implies that we need an identity mapping of the 
part of Xen that will run with MMU off.
I understand that rebuilding the page-tables and therefore switching the 
TTBR will be necessary for cache coloring. So before any new use, I 
would like the implementation of switch_ttbr() to be fixed.
What we will need to do is find space in the virtual layout that also 
match a physical address. With that in place, we could use the mapping 
to switch between TTBR.
[...]

>   void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t 
> xen_paddr)
> @@ -721,6 +885,7 @@ void __init setup_pagetables(unsigned long 
> boot_phys_offset, paddr_t xen_paddr)
>       per_cpu(xen_dommap, 0) = cpu0_dommap;
>   #endif
>   }
> +#endif /* !CONFIG_COLORING */
>   
>   static void clear_boot_pagetables(void)
>   {
> @@ -735,6 +900,9 @@ static void clear_boot_pagetables(void)
>   #endif
>       clear_table(boot_second);
>       clear_table(boot_third);
> +#ifdef CONFIG_COLORING
> +    clear_table(boot_colored_xen);
> +#endif
AFAICT, this is going to clear the boot pagetables in the cache coloring 
version of Xen. However, the secondary CPUs will build their page-tables 
using the version in the old Xen.
So you will need to update the code to clear the correct boot page tables.

[...]

> diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c
> index 0c90c2305c..d443fac6a2 100644
> --- a/xen/arch/arm/psci.c
> +++ b/xen/arch/arm/psci.c
> @@ -25,6 +25,7 @@
>   #include <asm/cpufeature.h>
>   #include <asm/psci.h>
>   #include <asm/acpi.h>
> +#include <asm/coloring.h>
>   
>   /*
>    * While a 64-bit OS can make calls with SMC32 calling conventions, for
> @@ -49,7 +50,8 @@ 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),
> +                  __pa(virt_boot_xen((vaddr_t)init_secondary)),
>                     &res);
>   
>       return PSCI_RET(res);
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 13b10515a8..294b806120 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -924,6 +924,7 @@ void __init start_xen(unsigned long boot_phys_offset,
>       struct domain *d;
>       int rc;
>       paddr_t xen_paddr = (paddr_t)(_start + boot_phys_offset);
> +    uint32_t xen_size = (_end - _start);
>   
>       dcache_line_bytes = read_dcache_line_bytes();
>   
> @@ -952,13 +953,16 @@ void __init start_xen(unsigned long boot_phys_offset,
>       if ( !coloring_init() )
>           panic("Xen Coloring support: setup failed\n");
>   
> +    xen_size = XEN_COLOR_MAP_SIZE;
> +#ifdef CONFIG_COLORING
> +    xen_paddr = get_xen_paddr(xen_size);
> +#endif
> +
>       /* Register Xen's load address as a boot module. */
> -    xen_bootmodule = add_boot_module(BOOTMOD_XEN, xen_paddr,
> -                             (paddr_t)(uintptr_t)(_end - _start + 1), false);
> +    xen_bootmodule = add_boot_module(BOOTMOD_XEN, xen_paddr, xen_size, false);
How do you plan to exclude the memory allocate the cache coloring version?

Cheers,

--
Julien Grall