[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,

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.

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?

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

> 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

 


Rackspace

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