Re: [PATCH v3 52/52] xen/arm: add Kconfig option CONFIG_HAS_MPU to enable MPU system support

[Julien Grall <julien@xxxxxxx>]

Hi,

On 05/07/2023 18:20, Ayan Kumar Halder wrote:
> On 26/06/2023 04:34, Penny Zheng wrote:
> > CAUTION: This message has originated from an External Source. Please 
> > use proper judgment and caution when opening attachments, clicking 
> > links, or responding to this email.
> >
> > Introduce a Kconfig option CONFIG_HAS_MPU to enable MPU architecture
> > support. STATIC_MEMORY, ARCH_MAP_DOMAIN_PAGE and ARM_SECURE_STATE will
> > be selected by MPU system by default. Also, features like, ARM_EFI, are
> > not supported right now.
> >
> > Current MPU system design is only for ARM 64-bit platform.
> >
> > Signed-off-by: Penny Zheng <penny.zheng@xxxxxxx>
> > Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
> > ---
> > v3:
> > - select ARCH_MAP_DOMAIN_PAGE and ARM_SECURE_STATE
> > - remove platform-specific config: CONFIG_ARM_V8R
> > ---
> >   xen/arch/arm/Kconfig | 14 +++++++++++++-
> >   1 file changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> > index 3f67aacbbf..2acdf39ec8 100644
> > --- a/xen/arch/arm/Kconfig
> > +++ b/xen/arch/arm/Kconfig
> > @@ -62,6 +62,7 @@ source "arch/Kconfig"
> >   config HAS_MMU
> >          bool "Memory Management Unit support in a VMSA system"
> >          default y
> > +       depends on !HAS_MPU
> >          select HAS_PAGING_MEMPOOL
> >          select HAS_PMAP
> >          select HAS_VMAP
> > @@ -70,6 +71,17 @@ config HAS_MMU
> >            a memory system through a set of virtual to physical 
> > address mappings and associated memory
> >            properties held in memory-mapped tables known as 
> > translation tables.
> > +config HAS_MPU
> > +       bool "Memory Protection Unit support in a PMSA system"
> > +       default n
> > +       depends on ARM_64
> This will not be true as R52 will support MPU
But the code will not compile until you work is added, correct? If so, I 
think it make sense to depend on ARM_64 for now.
> > +       select ARCH_MAP_DOMAIN_PAGE
> > +       select ARM_SECURE_STATE
> Also, R52 does not support secure state.
> > +       select STATIC_MEMORY
> > +       help
> > +         The PMSA is based on a Memory Protection Unit (MPU), which 
> > provides a much simpler
> > +         memory protection scheme than the MMU based VMSA.
> > +
> Instead, I will suggest something like this :-
>
> config ARM_V8R
>      bool "ARMv8-R AArch64 architecture support (UNSUPPORTED)" if 
> UNSUPPORTED
>      default n
>      select HAS_MPU
>      select ARM_SECURE_STATE
>      select STATIC_MEMORY
>      depends on ARM_64
>      help
>        This option enables Armv8-R profile for Arm64. Enabling this option
>        results in selecting MPU.
>
>
> So, that later on we can add :-
>
> config AARCH32_V8R
>      bool "AArch32 Arm V8R Support (UNSUPPORTED)" if UNSUPPORTED
>      default n
>      select HAS_MPU
>      select STATIC_MEMORY
>      depends on ARM_32
>      help
>        This option enables Armv8-R profile for Arm32.
I believe the the duplication can be avoided if you write:

config ARM_MPU
      bool "..." if  UNSUPPORTED
      default n
      select ARM_SECURE_STATE if ARM_64
      select STATIC_MEMORY
      help
        ...

Not that I didn't use your proposed name because in the future there 
might be an ArmVx-R and we don't want to rename everything. So a more 
generic name is better.
Cheers,

--
Julien Grall