Re: [PATCH v1] xen/arm: arm32: Add support to identify the Cortex-R52 processor

[Julien Grall <julien@xxxxxxx>]

Hi,

On 23/06/2023 22:26, Julien Grall wrote:
> > --- a/xen/arch/arm/arm32/head.S
> > +++ b/xen/arch/arm/arm32/head.S
> > @@ -322,7 +322,7 @@ cpu_init:
> >           PRINT("- Setting up control registers -\r\n")
> >
> >           mov   r5, lr                       /* r5 := return address */
> > -
> > +#ifndef CONFIG_ARM_NO_PROC_INIT
> >           /* Get processor specific proc info into r1 */
> >           bl    __lookup_processor_type
> >           teq   r1, #0
> > @@ -337,7 +337,7 @@ cpu_init:
> >           ldr   r1, [r1, #PROCINFO_cpu_init]  /* r1 := vaddr(init 
> > func) */
> >           adr   lr, cpu_init_done             /* Save return address */
> >           add   pc, r1, r10                   /* Call paddr(init func) */
> > -
> > +#endif
> I think it would be best if you just #ifdef the fail below. So if the 
> config selected, then you will still be able to have a Xen that can boot 
> Cortex-A15 or a core that don't need _init.
> Note that for now, we should only select this new config for Armv8-R 
> because there are some work to confirm it would be safe for us to boot 
> Xen 32-bit Arm on any CPUs. I vaguely remember that we were making some 
> assumptions on the cache type in the past. But maybe we other check in 
> place to check such assumption.
> If this can be confirm (I am not ask you to do it, but you can) then we 
> could even get rid of the #ifdef.
I had a look through the code. We have a check in the 32-bit version of 
setup_mm() for the instruction cache type. So I think it would be OK to 
relax the check in head.S.
Bertrand, Stefano, what do you think?

Cheers,

--
Julien Grall