[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Minios-devel] [UNIKRAFT PATCH 0/9] Prepare build scripts to support ARM64
Hey Wei,
On 09.04.2018 08:37, Wei Chen wrote:
Hi Simon,
Sorry for replying later, I had public holidays last week.
-----Original Message-----
From: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
Sent: 2018年4月5日 5:24
To: Wei Chen <Wei.Chen@xxxxxxx>; minios-devel@xxxxxxxxxxxxxxxxxxxx
Cc: Shijie Huang <Shijie.Huang@xxxxxxx>; Kaly Xin <Kaly.Xin@xxxxxxx>; nd
<nd@xxxxxxx>
Subject: Re: [UNIKRAFT PATCH 0/9] Prepare build scripts to support ARM64
Thanks a lot for this patch series!
I added my comments inline to each patch and look forward for v2. ;-)
I have some general minor notes:
Can you confirm that your formatting of source files is correct? (e.g.,
use checkpatch.pl). Patch 3 threw a warning that another blank line was
introduced at EOF.
About the code-style, I also have some concerns. While I was modifying the
code, I found some code was using "tab" and some code was using "spaces" for
indent. So I wasn't sure which code-style we should use.
We usually tried to adopt the new code style (Linux) for files we
ported. In fact, this may reduce traceability of what we changed in the
code from the original file since you can't do a simple diff anymore. I
understand your concerns. However, we preferred to unify the code style
since we expect that we are going to diverge anyways from the original
(which was MiniOS and Solo5 mainly).
I just checked the code again, the "tab" indent is use by those files copied
from Xen. I understand we should keep the original code-style for those
files. Except such kind of files, should we use the Linux code-style for
Unikraft?
Yes, in general, Linux style.
Please double check the selectors in your short commit messages. Patch
4, for instance, should use 'include' instead of 'build'.
Oh, that's right. Though this series is for 'build', but patch#4 modified
The "include". I should use the 'include:' as subject prefix.
Great ;-)
Since I saw it in several commit messages: Remove the "have to"s in your
commit messages. They are still fine without ;-). The message of each
patch should just describe what it does - independent of any
circumstances or weightings.
Thanks. I would refine the commit messages to address this comment : )
Thanks,
Simon
On 15.03.2018 04:39, Wei Chen wrote:
Currently, Unikraft only supports arm32 and x86_64. The folder layout
is not very convenient to add arm64 or x86_32 support to it. In this
case we will modify the folder layout to support common code for the
architectures of the same CPU families. We also have to modify the
build scripts which corresponding to this change at the same time.
Wei Chen (9):
build: Adjust sed script to avoid treating arm64 as arm
build: Introduce a new variable UK_FAMILY
build: Move arm32 libraries to new family/architecture folder
build: Move architecture headers to family/architecture folder
build: Add a makefile rule to check valid gcc version
build: Add arm64 architecture config to menuconfig
build: Add a macro to check and add gcc flags for target CPU
build: Add compiler and flags for arm64
build: Check the minimum GCC version for arm32
Makefile | 36 ++-
arch/Arch.uk | 2 +
arch/Config.uk | 7 +-
arch/arm/Compiler.uk | 4 +
arch/arm/Config.uk | 67 +++++-
arch/arm/Makefile.uk | 83 ++++++-
arch/arm/arm32/divsi3.S | 404
++++++++++++++++++++++++++++++++++
arch/arm/arm32/ldivmod.S | 68 ++++++
arch/arm/arm32/ldivmod_helper.c | 67 ++++++
arch/arm/arm32/qdivrem.c | 324 +++++++++++++++++++++++++++
arch/arm/divsi3.S | 404 ------------------------------
----
arch/arm/ldivmod.S | 68 ------
arch/arm/ldivmod_helper.c | 67 ------
arch/arm/qdivrem.c | 324 ---------------------------
arch/x86/Compiler.uk | 6 +
arch/x86/Config.uk | 89 ++++++++
arch/x86/Makefile.uk | 37 ++++
arch/x86_64/Compiler.uk | 6 -
arch/x86_64/Config.uk | 89 --------
arch/x86_64/Makefile.uk | 37 ----
include/uk/arch/arm/arm32/atomic.h | 64 ++++++
include/uk/arch/arm/arm32/intsizes.h | 45 ++++
include/uk/arch/arm/arm32/lcpu.h | 59 +++++
include/uk/arch/arm/arm32/limits.h | 45 ++++
include/uk/arch/arm/arm32/types.h | 35 +++
include/uk/arch/arm/atomic.h | 64 ------
include/uk/arch/arm/intsizes.h | 45 ----
include/uk/arch/arm/lcpu.h | 59 -----
include/uk/arch/arm/limits.h | 45 ----
include/uk/arch/arm/types.h | 35 ---
include/uk/arch/atomic.h | 8 +-
include/uk/arch/lcpu.h | 8 +-
include/uk/arch/limits.h | 16 +-
include/uk/arch/types.h | 16 +-
include/uk/arch/x86/x86_64/atomic.h | 45 ++++
include/uk/arch/x86/x86_64/intsizes.h | 45 ++++
include/uk/arch/x86/x86_64/lcpu.h | 73 ++++++
include/uk/arch/x86/x86_64/limits.h | 46 ++++
include/uk/arch/x86/x86_64/types.h | 38 ++++
include/uk/arch/x86_64/atomic.h | 45 ----
include/uk/arch/x86_64/intsizes.h | 45 ----
include/uk/arch/x86_64/lcpu.h | 73 ------
include/uk/arch/x86_64/limits.h | 46 ----
include/uk/arch/x86_64/types.h | 38 ----
support/build/Makefile.rules | 8 +
45 files changed, 1698 insertions(+), 1537 deletions(-)
create mode 100644 arch/arm/arm32/divsi3.S
create mode 100644 arch/arm/arm32/ldivmod.S
create mode 100644 arch/arm/arm32/ldivmod_helper.c
create mode 100644 arch/arm/arm32/qdivrem.c
delete mode 100644 arch/arm/divsi3.S
delete mode 100644 arch/arm/ldivmod.S
delete mode 100644 arch/arm/ldivmod_helper.c
delete mode 100644 arch/arm/qdivrem.c
create mode 100644 arch/x86/Compiler.uk
create mode 100644 arch/x86/Config.uk
create mode 100644 arch/x86/Makefile.uk
delete mode 100644 arch/x86_64/Compiler.uk
delete mode 100644 arch/x86_64/Config.uk
delete mode 100644 arch/x86_64/Makefile.uk
create mode 100644 include/uk/arch/arm/arm32/atomic.h
create mode 100644 include/uk/arch/arm/arm32/intsizes.h
create mode 100644 include/uk/arch/arm/arm32/lcpu.h
create mode 100644 include/uk/arch/arm/arm32/limits.h
create mode 100644 include/uk/arch/arm/arm32/types.h
delete mode 100644 include/uk/arch/arm/atomic.h
delete mode 100644 include/uk/arch/arm/intsizes.h
delete mode 100644 include/uk/arch/arm/lcpu.h
delete mode 100644 include/uk/arch/arm/limits.h
delete mode 100644 include/uk/arch/arm/types.h
create mode 100644 include/uk/arch/x86/x86_64/atomic.h
create mode 100644 include/uk/arch/x86/x86_64/intsizes.h
create mode 100644 include/uk/arch/x86/x86_64/lcpu.h
create mode 100644 include/uk/arch/x86/x86_64/limits.h
create mode 100644 include/uk/arch/x86/x86_64/types.h
delete mode 100644 include/uk/arch/x86_64/atomic.h
delete mode 100644 include/uk/arch/x86_64/intsizes.h
delete mode 100644 include/uk/arch/x86_64/lcpu.h
delete mode 100644 include/uk/arch/x86_64/limits.h
delete mode 100644 include/uk/arch/x86_64/types.h
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel
|
