Re: [Minios-devel] [UNIKRAFT PATCHv4 08/43] arch: Add arm64 architecture config to menuconfig

[Wei Chen <Wei.Chen@xxxxxxx>]

Hi Julien,

> -----Original Message-----
> From: Julien Grall <julien.grall@xxxxxxx>
> Sent: 2018年7月8日 5:56
> To: Wei Chen <Wei.Chen@xxxxxxx>; minios-devel@xxxxxxxxxxxxxxxxxxxx;
> simon.kuenzer@xxxxxxxxx
> Cc: Kaly Xin <Kaly.Xin@xxxxxxx>; nd <nd@xxxxxxx>
> Subject: Re: [Minios-devel] [UNIKRAFT PATCHv4 08/43] arch: Add arm64
> architecture config to menuconfig
> 
> Hi,
> 
> On 07/06/2018 10:03 AM, Wei Chen wrote:
> > Add the arm64 entry for menuconfig. As different silicon vendors may
> > have different 64-bit ARMv8 SoCs. If we want to add them to Config,
> 
> I know some people will find me very picky :). Based on the new

Sometimes ; )

> branding, this should be Armv8 (i.e no upper-case for r, m). I am not
> too fuss for the commit message, but I would like to be at list fixed in
> the Kconfig description.

Honestly, Arm looks very very awkward to me. But I think you're right, it's
the new branding, I would change them to Arm, although I still think arm or
ARM looks better. . Maybe I am a little Obsessive compulsive : )

> 
> > it will be a large list. So we only provide ARM's cortex A53~A75 CPUs
> 
> Sam here.

Ok.

> 
> > for "Processor Optimization"
> >
> > If we use MARCH_ as the prefix for ARM64 CPUs as x86, when we select
> > "generic", the MARCH_GENERIC will conflict with x86's MARCH_GENERIC.
> > So, we use MARCH_ARM64_ for ARM64 as the prefix.
> >
> > Current supported arm64 CPU models:
> > native, generic, cortex-a53, cortex-a57, cortex-a72, cortex-a73,
> > cortex-a55 and cortex-a75.
> >
> > Signed-off-by: Wei Chen <Wei.Chen@xxxxxxx>
> > ---
> >   Config.uk                |  2 +-
> >   arch/Arch.uk             |  2 ++
> >   arch/Config.uk           |  6 ++++
> >   arch/arm/arm64/Config.uk | 60 ++++++++++++++++++++++++++++++++++++++++
> >   4 files changed, 69 insertions(+), 1 deletion(-)
> >   create mode 100644 arch/arm/arm64/Config.uk
> >
> > diff --git a/Config.uk b/Config.uk
> > index 21cec9b..e7a26b2 100644
> > --- a/Config.uk
> > +++ b/Config.uk
> > @@ -65,7 +65,7 @@ config OPTIMIZE_SIZE
> >   endchoice
> >
> >   comment "Hint: Specify a CPU type to get most benefits from performance
> optimization"
> > -   depends on OPTIMIZE_PERF && MARCH_GENERIC
> > +   depends on OPTIMIZE_PERF && (MARCH_GENERIC || MARCH_ARM64_GENERIC)
> 
> Not even looking at the code, the naming looks wrong here. When I read
> MARCH_GENERIC, I would expect to be selected by anyone.
> 
> It feels like to me we want to introduce yet another Kconfig
> HAS_OPTIMIZE_PERF that will be selected by MARCH_GENERIC (x86) and
> MARCH_ARM64_GENERIC (Arm64).
> 

MARCH_GENERIC here should be MARCH_X86_64_GENERIC. When Simon released this
code, Unikraft only support x86_64. So he didn't add X86_64 to this
CONFIG_OPTION. I have renamed MARCH_GENERIC to MARCH_X86_64_GENERIC
in next patch.

> >
> >   config OPTIMIZE_DEADELIM
> >     bool "Drop unused functions and data"
> > diff --git a/arch/Arch.uk b/arch/Arch.uk
> > index f11308b..a8b3ca2 100644
> > --- a/arch/Arch.uk
> > +++ b/arch/Arch.uk
> > @@ -1,6 +1,8 @@
> >   # Selects architecture according to .config
> >   ifeq ($(CONFIG_ARCH_X86_64),y)
> >   CONFIG_UK_ARCH := x86_64
> > +else ifeq ($(CONFIG_ARCH_ARM_64),y)
> > +CONFIG_UK_ARCH := arm64
> >   else ifeq ($(CONFIG_ARCH_ARM_32),y)
> >   CONFIG_UK_ARCH := arm
> >   endif
> > diff --git a/arch/Config.uk b/arch/Config.uk
> > index 9236273..f08274d 100644
> > --- a/arch/Config.uk
> > +++ b/arch/Config.uk
> > @@ -1,12 +1,15 @@
> >   choice
> >     prompt "Architecture"
> >     default ARCH_ARM_32 if (UK_ARCH = "arm")
> > +   default ARCH_ARM_64 if (UK_ARCH = "arm64")
> >     default ARCH_X86_64
> >     help
> >       Select the target CPU architecture.
> >
> >   config ARCH_X86_64
> >          bool "x86 compatible (64 bits)"
> > +config ARCH_ARM_64
> > +       bool "ARMv8 compatible (64 bits)"
> >   config ARCH_ARM_32
> >          bool "ARMv7 compatible (32 bits)"
> >
> > @@ -18,3 +21,6 @@ endif
> >   if (ARCH_ARM_32)
> >     source "arch/arm/arm/Config.uk"
> >   endif
> > +if (ARCH_ARM_64)
> > +   source "arch/arm/arm64/Config.uk"
> > +endif
> > diff --git a/arch/arm/arm64/Config.uk b/arch/arm/arm64/Config.uk
> > new file mode 100644
> > index 0000000..634ec50
> > --- /dev/null
> > +++ b/arch/arm/arm64/Config.uk
> > @@ -0,0 +1,60 @@
> > +choice
> > +   prompt "Processor Optimization"
> > +   default MARCH_ARM64_GENERIC
> 
> Do we really need to have ARM64 in the name?
> 

Yes, we have MARCH_X86_64_GENERIC, MARCH_ARM64_GENERIC now. And in
The future we may have MARCH_ARM_GENERIC, MARCH_PPC64_GENERIC and
etc. Without them, in some cases, we have to use following similar
combination: CONFIG_ARM64 && CONFIG_MARCH_GENERIC


> > +   help
> > +           Optimize the code for selected target processor
> > +
> > +config MARCH_ARM64_NATIVE
> > +   bool "Auto-detect host CPU"
> > +   help
> > +           Optimize compilation to host CPU. Please note that this
> > +           option will fail in case of cross-compilation
> > +
> > +config MARCH_ARM64_GENERIC
> > +   bool "Generic ARMv8 CPU"
> 
> s/ARM/Arm/

Ok.

> 
> > +   help
> > +           Compile for Generic ARMv8 compatible CPUs
> > +
> > +config MARCH_ARM64_CORTEXA53
> > +   bool "Generic ARMv8 Cortex A53"
> > +   help
> > +           Compile for ARMv8 Cortex-A53 CPUs. Support TrustZone, NEON
> 
> Ditto.

Ok

> 
> > +           advanced SIMD, VFPv4, hardware virtualization, dual issue,
> 
> How virtualization matters for Unikraft? Shouldn't this just describe
> what will be the benefits for Unikraft?

This is just a description for the Cortex-A53. I copy them from wiki.

> 
> > +           in-order pipeline
> > +
> > +config MARCH_ARM64_CORTEXA57
> > +   bool "Generic ARMv8 Cortex A57"
> > +   help
> > +           Compile for ARMv8 Cortex-A57 CPUs. Support TrustZone, NEON
> > +           advanced SIMD, VFPv4, hardware virtualization, 3-way
> 
> Same here and everywhere below.

As above.

> 
> > +           superscalar, deeply out-of-order pipeline
> > +
> > +config MARCH_ARM64_CORTEXA72
> > +   bool "Generic ARMv8 Cortex A72"
> > +   help
> > +           Compile for ARMv8 Cortex-A72 CPUs. Support TrustZone, NEON
> > +           advanced SIMD, VFPv4, hardware virtualization, 3-way
> > +           superscalar, deeply out-of-order pipeline
> > +
> > +config MARCH_ARM64_CORTEXA73
> > +   bool "Generic ARMv8 Cortex A73"
> > +   help
> > +           Compile for ARMv8 Cortex-A73 CPUs. Support TrustZone, NEON
> > +           advanced SIMD, VFPv4, hardware virtualization, 2-way
> > +           superscalar, deeply out-of-order pipeline
> > +
> > +config MARCH_ARM64_CORTEXA55
> > +   bool "Generic ARMv8.2 Cortex A55"
> > +   help
> > +           Compile for ARMv8.2 Cortex-A55 CPUs. Support TrustZone, NEON
> > +           advanced SIMD, VFPv4, hardware virtualization, triple issue,
> > +           in-order pipeline
> > +
> > +config MARCH_ARM64_CORTEXA75
> > +   bool "Generic ARMv8.2 Cortex A75"
> > +   help
> > +           Compile for ARMv8.2 Cortex-A75 CPUs. Support TrustZone, NEON
> > +           advanced SIMD, VFPv4, hardware virtualization, 3-way
> > +           superscalar, deeply out-of-order pipeline
> > +
> > +endchoice
> >
> 
> Cheers,
> 
> --
> Julien Grall
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel