Re: [PATCH v1 1/4] xen/arm: mpu: Introduce choice between MMU and MPU

[Ayan Kumar Halder <ayankuma@xxxxxxx>]

On 27/08/2024 11:37, Julien Grall wrote:
> Hi,
Hi Julien,
> On 23/08/2024 17:31, Ayan Kumar Halder wrote:
> > There are features in the forthcoming patches which are dependent on
> > MPU. For eg fixed start address.
> > Also, some of the Xen features (eg STATIC_MEMORY) will be selected
> > by the MPU configuration.
> >
> > Thus, this patch introduces a choice between MMU and MPU for the type
> > of memory management system. By default, MMU is selected.
> > All the current platforms are now made dependent on MMU.
> >
> > Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@xxxxxxx>
> > ---
> >   xen/arch/arm/Kconfig           | 17 ++++++++++++++++-
> >   xen/arch/arm/platforms/Kconfig |  2 +-
> >   2 files changed, 17 insertions(+), 2 deletions(-)
> >
> > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> > index 323c967361..686948cefd 100644
> > --- a/xen/arch/arm/Kconfig
> > +++ b/xen/arch/arm/Kconfig
> > @@ -58,10 +58,25 @@ config PADDR_BITS
> >       default 40 if ARM_PA_BITS_40
> >       default 48 if ARM_64
> >   +choice
> > +    prompt "Memory management system"
> > +    default MMU if ARM
> > +    help
> > +      User can choose between the different forms of memory 
> > management system.
> > +      Most of the Arm based systems support MMU.
> I feel this is quite subjective. So I would drop it.
Agreed.
> > +
> >   config MMU
> > -    def_bool y
> > +    bool "MMU"
> >       select HAS_PMAP
> >       select HAS_VMAP
> > +    help
> > +      Memory management unit is supported on most Arm based systems.
> For this case, I would write "Select it you are plan to run Xen on 
> Armv7-A or Armv8-A".
Ack.
> > +
> > +config MPU
> > +    bool "MPU"
> I think it should be gated by UNSUPPORTED. SUPPORT.md also needs to be 
> updated.
Ack.
> > +    help
> > +      Memory protection unit is supported on some Cortex-R systems.
> Why do you make this specific to Cortex-R? Shouldn't this instead be 
> Armv8-R?
Yes, you are correct. It should be Armv8-R. Otherwise, someone might 
think it is applicable for Cortex-R5, R7, etc.
- Ayan