[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

Hey Wei, Julien,

On 10.07.2018 09:16, Wei Chen wrote:
Hi Julien,

-----Original Message-----
From: Julien Grall <julien.grall@xxxxxxx>
Sent: 2018年7月9日 18:24
To: Wei Chen <Wei.Chen@xxxxxxx>; minios-devel@xxxxxxxxxxxxxxxxxxxx;
Cc: Kaly Xin <Kaly.Xin@xxxxxxx>; nd <nd@xxxxxxx>
Subject: Re: [Minios-devel] [UNIKRAFT PATCHv4 08/43] arch: Add arm64
architecture config to menuconfig

On 09/07/18 10:03, Wei Chen wrote:
Hi Julien,

Hi Wei,

-----Original Message-----
From: Julien Grall <julien.grall@xxxxxxx>
Sent: 2018年7月8日 5:56
To: Wei Chen <Wei.Chen@xxxxxxx>; minios-devel@xxxxxxxxxxxxxxxxxxxx;
Cc: Kaly Xin <Kaly.Xin@xxxxxxx>; nd <nd@xxxxxxx>
Subject: Re: [Minios-devel] [UNIKRAFT PATCHv4 08/43] arch: Add arm64
architecture config to menuconfig


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 : )

Sadly, 'Arm' or 'arm' is the way to go nowadays. The latter is preferred
in sentence to avoid confusion with another close word ;).

'ARM' should not be used anymore.

Now I learned also something new to me ;-). You are the Arm guys, I trust you about the proper typing of Arm.

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

Sam here.


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

    comment "Hint: Specify a CPU type to get most benefits from performance
-       depends on OPTIMIZE_PERF && MARCH_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_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
in next patch.

Can you move this next patch before? This would make clearer this patch.

Ok, I will adjust the order.

For me, both orders are fine.

        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
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 @@
        prompt "Architecture"
        default ARCH_ARM_32 if (UK_ARCH = "arm")
+       default ARCH_ARM_64 if (UK_ARCH = "arm64")
        default ARCH_X86_64
          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"
+if (ARCH_ARM_64)
+       source "arch/arm/arm64/Config.uk"
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 @@
+       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

I agree.

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



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



+               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.

Hum, I retrieved the x86 descriptions from GCC. They are not talking about virtualization but if it is properly worded it is fine to mention it here (same for TrustZone). It just would need to be clear that we are talking about features that a particular CPU/SoC has - and not that Unikraft needs it or is going to use it:

I would do something along the lines. Basically, remove "the compile for" to tell which features an Arm v8 SoC/CPU has:

bool "Generic Armv8 Cortex A53"
        ARMv8 Cortex-A53 CPUs with NEON, advanced SIMD,
        VFPv4, TrustZone, hardware virtualization, [...] support

However, it is also fine to do:

bool "Generic Armv8 Cortex A53"
        Compile for ARMv8 Cortex-A53 (and compatible) CPUs

Cortex-A53 actually defines a particular feature set, right?
What do you think?

Which wiki? In general, the description of a config should explain why a
user should select the option. It does not need to know that the
Cortex-A53 supports virtualization (or even allow 32-bit).

I forget, maybe from Wikipedia.


Julien Grall

Minios-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.