|
[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
Hi Simon, > -----Original Message----- > From: Simon Kuenzer <simon.kuenzer@xxxxxxxxx> > Sent: 2018年6月15日 5:17 > 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 > > 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 got it, thanks. I will adopt Linux code style in my patches. > > > > 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
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |