Re: [PATCH v2 4/4] xen/arm: mpu: Implement a dummy enable_secondary_cpu_mm

[Julien Grall <julien@xxxxxxx>]

Hi Jan,

On 24/09/2024 13:22, Jan Beulich wrote:
> On 24.09.2024 13:54, Ayan Kumar Halder wrote:
> > Hi Jan,
> >
> > On 23/09/2024 11:23, Jan Beulich wrote:
> > > On 18.09.2024 19:51, Ayan Kumar Halder wrote:
> > > > Secondary cpus initialization is not yet supported. Thus, we print an
> > > > appropriate message and put the secondary cpus in WFE state.
> > > >
> > > > Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@xxxxxxx>
> > > > ---
> > > > Changes from :-
> > > >
> > > > v1 - 1. NR_CPUS is defined as 1 for MPU
> > > Not quite, what you do is ...
> > >
> > > > --- a/xen/arch/Kconfig
> > > > +++ b/xen/arch/Kconfig
> > > > @@ -11,6 +11,7 @@ config NR_CPUS
> > > >         default "8" if ARM && RCAR3
> > > >         default "4" if ARM && QEMU
> > > >         default "4" if ARM && MPSOC
> > > > +       default "1" if ARM && MPU
> > > >         default "128" if ARM
> > > >         help
> > > >           Controls the build-time size of various arrays and bitmaps
> > > ... merely set the default. I wonder how useful that is, the more that
> > > - as per the description - this is temporary only anyway.
> > Yes, I can enforce a build time check like this.
> >
> > --- a/xen/arch/arm/setup.c
> > +++ b/xen/arch/arm/setup.c
> > @@ -295,6 +295,12 @@ void asmlinkage __init start_xen(unsigned long
> > fdt_paddr)
> >        struct domain *d;
> >        int rc, i;
> >
> > +    /*
> > +     * Currently MPU does not support SMP.
> > +     */
> > +#ifdef CONFIG_MPU
> > +    BUILD_BUG_ON(NR_CPUS > 1);
> > +#endif
> >        dcache_line_bytes = read_dcache_line_bytes();
> >
> > Does this look ok ?
> Well, I'd still want to understand the purpose (if I was maintainer of
> this code at least). You can't bring up secondary processors yet - fine.
> But why does that mean you want to limit the build to NR_CPUS=1? Any
> number is fine, but just not useful?
I have suggested to restrict NR_CPUS because it is not clear whether 
cpu_up() would even work on MPU. In its current state, it may return an 
error but could also corrupt the system.
At least for Xen on Arm, we have no other way to temporarly disable SMP 
support (aside modify the provided DTB, but I would like to avoid it).
Cheers,

--
Julien Grall