Re: [PATCH 12/12] xen/arm: add cache coloring support for Xen

[Julien Grall <julien@xxxxxxx>]

On 10/09/2022 16:22, Julien Grall wrote:
> Hi Carlo,
>
> I haven't fully reviewed the patch yet. Just point out a couple of 
> things I noticed while looking how you dealt with the relocation (I need 
> some code to test my series :)).
> On 26/08/2022 13:51, Carlo Nonato wrote:
> > -#ifdef CONFIG_ARM_64
> > +#ifdef CONFIG_CACHE_COLORING
> > +    /* Copy Xen to the new location */
> > +    memcpy((void *)BOOT_RELOC_VIRT_START, (const void *)XEN_VIRT_START,
> > +           (_end - _start));
> If I am not mistaken, at this point, Xen will still be using the stack 
> that is part of Xen binary (see cpu0_boot_stack). However, until the 
> point switch_ttbr() is called the stack can still be used. In particular...
> > +    clean_dcache_va_range((void *)BOOT_RELOC_VIRT_START, (_end - 
> > _start)); > +
> > +    ttbr = virt_to_maddr_colored((vaddr_t)xen_pgtable);
> > +#elif CONFIG_ARM_64
> >       ttbr = (uintptr_t) xen_pgtable + phys_offset;
> >   #else
> >       ttbr = (uintptr_t) cpu0_pgtable + phys_offset;
> > @@ -530,6 +610,18 @@ void __init setup_pagetables(unsigned long 
> > boot_phys_offset)
> ... the compiler may need to save some information on the stack. And 
> then...
> >       switch_ttbr(ttbr);
> ... restore it after switch_ttbr(). Xen will be using a different stack 
> (same virtual address but different physical address) so the wrong value 
> will be loaded.
> I am not aware of any guaranteed from the AAPCS{32, 64} that this can 
> point (please provide a pointer if I am wrong). So I think we either 
s/point/not happen/

> want to copy the new Xen to the correct position in the assembly.
>
> This means we would want to revive partially f60658c6ae47 "xen/arm: Stop 
> relocating Xen".
> Cheers,
--
Julien Grall