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

Re: [Minios-devel] [UNIKRAFT PATCHv4 00/43] Add arm64/kvm support for Unikraft



Hi Wei,

The title says v4 but I don't seem to find the previous versions on the ML. Did I miss anything?

On 07/06/2018 10:03 AM, Wei Chen wrote:
This patch series enable Unikraft on arm64/kvm. As we
haven't implemented GIC libraries and full timer support,
this patch series can ONLY work without uksched.
What we have done in this patch series:
1. Modified the build scripts and restructured
    the folders to improve the  multi-arch and multi-plat
    support,
2. Added boot code for Arm64 QEMU-KVM platform,

So you are only targeting KVM with QEMU? kvmtools (quite useful for lighter development) or any other will not work?

3. Enabled MMU and setup a 1:1 mapping page table for
    physical memory and virtual memory,
4. Added an exception table to handle SYNC, IRQ and other
    exceptions (Just dumping registers in this stage),
5. Supported device tree,

I am a bit confused with the reason of adding DT here. I would have thought it was for getting the code as generic as possible, but a lot of this series makes the assumption on the memory layout.

I would rather avoid a mix of both world (DT vs Hardcoded). This is making the code more difficult to read and maintain.

6. A simple PSCI library for CPU suspend, reset and system
    shutdown
7. PL011 UART for console and STDIO
8. A simple virtual timer library for debug timestamp.
Wei Chen (43):

To help the review it would be nice if you could split in smaller series.

Also can you provide a git branch with your code?

   build: Adjust sed script to avoid treating arm64 as arm
   build: Introduce a new variable UK_FAMILY
   arch: Reorganize arch folder to reflect the CPU family schema
   include: Reorganize base include folder to reflect CPU family schema
   build: Add a makefile function to check GCC version
   build: Add a makefile function to warn user when GCC is too old
   build: Add a makefile function to check and set flags for valid gcc
   arch: Add arm64 architecture config to menuconfig
   arch/x86: Rename MARCH_* of x86_64 to MARCH_X86_64_*
   arch/arm: Rename MARCH_* of arm32 to MARCH_ARM32_*
   arch/arm64: Add processor optimization GCC flags for arm64
   arch/arm: Check gcc version and set processor flags for arm32
   arch/arm: Add more CPU models to processor optimization list
   build: Override default pie option of GCC if possible
   uk/arch: Add necessary header files for Arm64
   uk/arch: Implement ukarch_find_lsbit for Arm64
   plat/include: Define macros for Arm64 to access registers
   plat/include: Define address offsets of boot stack and pagetable
   plat/kvm: Add link script for Arm64
   plat/kvm: Add console library for Arm64
   plat/kvm: Add Arm64 basic entry code
   plat/kvm: Allow access to floating-point and Advanced SIMD registers
   plat/kvm: Add Arm64 virtual timer library to provide ticks
   plat/common: Common arm64 CPU interrupt flag handling
   plat/kvm: Add interrupt handle APIs for arm64
   plat/kvm: Add trap handler to dump registers
   plat/kvm: Add exception table for Arm64
   plat/kvm: Create page tables for Arm64
   plat/kvm: Enable MMU for Arm64
   plat/kvm: Initialize device tree for Arm64
   plat/kvm: Parse command line from device tree for Arm64
   plat/kvm: Parse memory info from device tree for Arm64
   plat/kvm: Get PSCI conduit method from DTB for arm64
   plat/common: Implement PSCI despatch functions for arm64
   plat/common: Implement CPU suspend for arm64
   plat/common: Implement CPU reset for arm64
   plat/common: Implement system off for arm64
   plat/kvm: Implement shutdown for Arm64
   plat/kvm: Swith away from boot stack
   plat/kvm: Add kvm to Arm64 supported platform list
   plat/kvm: Update linker.uk to link image for Arm64
   plat/kvm: Implement time_block_until for arm64
   plat/kvm: Add Arm64 support source code to build list

  Config.uk                                     |   2 +-
  Makefile                                      |  33 +-
  Makefile.uk                                   |  22 +-
  arch/Arch.uk                                  |   2 +
  arch/Config.uk                                |  10 +-
  arch/arm/Compiler.uk                          |   9 +-
  arch/arm/Makefile.uk                          |  25 +-
  arch/arm/arm/Compiler.uk                      |   6 +
  arch/arm/{ => arm}/Config.uk                  |   6 +-
  arch/arm/arm/Makefile.uk                      |  72 +++
  arch/arm/{ => arm}/divsi3.S                   |  17 +-
  arch/arm/{ => arm}/ldivmod.S                  |   2 +-
  arch/arm/{ => arm}/ldivmod_helper.c           |   0
  arch/arm/{ => arm}/qdivrem.c                  |   0
  arch/arm/arm64/Compiler.uk                    |   6 +
  arch/arm/arm64/Config.uk                      |  60 +++
  arch/arm/arm64/Makefile.uk                    |  43 ++
  arch/x86/Compiler.uk                          |   4 +
  arch/x86/Makefile.uk                          |   4 +
  arch/{ => x86}/x86_64/Compiler.uk             |   0
  arch/{ => x86}/x86_64/Config.uk               |  34 +-
  arch/x86/x86_64/Makefile.uk                   |  37 ++
  arch/x86_64/Makefile.uk                       |  37 --
  include/uk/arch/arm/{ => arm}/atomic.h        |   0
  include/uk/arch/arm/{ => arm}/intsizes.h      |   0
  include/uk/arch/arm/{ => arm}/lcpu.h          |   0
  include/uk/arch/arm/{ => arm}/limits.h        |   0
  include/uk/arch/arm/{ => arm}/types.h         |   0
  include/uk/arch/arm/arm64/atomic.h            |  64 +++
  include/uk/arch/arm/arm64/intsizes.h          |  47 ++
  include/uk/arch/arm/arm64/lcpu.h              |  85 ++++
  include/uk/arch/arm/arm64/limits.h            |  48 ++
  include/uk/arch/{x86_64 => arm/arm64}/types.h |   0
  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     |   0
  include/uk/arch/{ => x86}/x86_64/intsizes.h   |   0
  include/uk/arch/{ => x86}/x86_64/lcpu.h       |   0
  include/uk/arch/{ => x86}/x86_64/limits.h     |   0
  include/uk/arch/x86/x86_64/types.h            |  38 ++
  plat/common/arm/cpu_native.c                  |  60 +++
  plat/common/arm/psci_arm64.S                  |  20 +

Why not introduce a arm64/ directory here?

  plat/common/arm/traps.c                       |  72 +++
  plat/common/include/arm/arm64/cpu.h           |  87 ++++
  plat/common/include/arm/arm64/cpu_defs.h      | 387 ++++++++++++++++
  plat/common/include/arm/arm64/irq.h           |  79 ++++
  plat/common/include/arm/cpu.h                 |  46 ++
  plat/common/include/arm/cpu_defs.h            |  47 ++
  plat/common/include/arm/irq.h                 |  46 ++
  plat/common/include/cpu.h                     |   4 +-
  plat/common/include/irq.h                     |  47 ++
  plat/kvm/Config.uk                            |   3 +-
  plat/kvm/Linker.uk                            |   5 +-
  plat/kvm/Makefile.uk                          |  28 +-
  plat/kvm/arm/console.c                        | 156 +++++++
  plat/kvm/arm/entry64.S                        |  82 ++++
  plat/kvm/arm/exceptions.S                     | 209 +++++++++

Assembly code can unlikely be shared between 32-bit and 64-bit. So shouldn't this be in an arm64 directory?

  plat/kvm/arm/intctrl.c                        |  24 +
  plat/kvm/arm/lcpu.c                           |  75 ++++
  plat/kvm/arm/link64.ld                        | 111 +++++
  plat/kvm/arm/pagetable.S                      | 418 ++++++++++++++++++

Ditto.

  plat/kvm/arm/setup.c                          | 211 +++++++++
  plat/kvm/arm/time.c                           | 138 ++++++
  plat/kvm/shutdown.c                           |  11 +-
  support/build/Makefile.rules                  |  20 +
  67 files changed, 3006 insertions(+), 141 deletions(-)
  create mode 100644 arch/arm/arm/Compiler.uk
  rename arch/arm/{ => arm}/Config.uk (80%)
  create mode 100644 arch/arm/arm/Makefile.uk
  rename arch/arm/{ => arm}/divsi3.S (97%)
  rename arch/arm/{ => arm}/ldivmod.S (99%)
  rename arch/arm/{ => arm}/ldivmod_helper.c (100%)
  rename arch/arm/{ => arm}/qdivrem.c (100%)
  create mode 100644 arch/arm/arm64/Compiler.uk
  create mode 100644 arch/arm/arm64/Config.uk
  create mode 100644 arch/arm/arm64/Makefile.uk
  create mode 100644 arch/x86/Compiler.uk
  create mode 100644 arch/x86/Makefile.uk
  rename arch/{ => x86}/x86_64/Compiler.uk (100%)
  rename arch/{ => x86}/x86_64/Config.uk (87%)
  create mode 100644 arch/x86/x86_64/Makefile.uk
  delete mode 100644 arch/x86_64/Makefile.uk
  rename include/uk/arch/arm/{ => arm}/atomic.h (100%)
  rename include/uk/arch/arm/{ => arm}/intsizes.h (100%)
  rename include/uk/arch/arm/{ => arm}/lcpu.h (100%)
  rename include/uk/arch/arm/{ => arm}/limits.h (100%)
  rename include/uk/arch/arm/{ => arm}/types.h (100%)
  create mode 100644 include/uk/arch/arm/arm64/atomic.h
  create mode 100644 include/uk/arch/arm/arm64/intsizes.h
  create mode 100644 include/uk/arch/arm/arm64/lcpu.h
  create mode 100644 include/uk/arch/arm/arm64/limits.h
  rename include/uk/arch/{x86_64 => arm/arm64}/types.h (100%)
  rename include/uk/arch/{ => x86}/x86_64/atomic.h (100%)
  rename include/uk/arch/{ => x86}/x86_64/intsizes.h (100%)
  rename include/uk/arch/{ => x86}/x86_64/lcpu.h (100%)
  rename include/uk/arch/{ => x86}/x86_64/limits.h (100%)
  create mode 100644 include/uk/arch/x86/x86_64/types.h
  create mode 100644 plat/common/arm/cpu_native.c
  create mode 100644 plat/common/arm/psci_arm64.S
  create mode 100644 plat/common/arm/traps.c
  create mode 100644 plat/common/include/arm/arm64/cpu.h
  create mode 100644 plat/common/include/arm/arm64/cpu_defs.h
  create mode 100644 plat/common/include/arm/arm64/irq.h
  create mode 100644 plat/common/include/arm/cpu.h
  create mode 100644 plat/common/include/arm/cpu_defs.h
  create mode 100644 plat/common/include/arm/irq.h
  create mode 100644 plat/common/include/irq.h

What is the difference between plat/common/arm and arch/arm?

  create mode 100644 plat/kvm/arm/console.c
  create mode 100644 plat/kvm/arm/entry64.S

It would be nice to move this in a arm64/ directoyr. You already did that for arch/arm.

  create mode 100644 plat/kvm/arm/exceptions.S

Ditto.

  create mode 100644 plat/kvm/arm/intctrl.c
  create mode 100644 plat/kvm/arm/lcpu.c
  create mode 100644 plat/kvm/arm/link64.ld
  create mode 100644 plat/kvm/arm/pagetable.S
  create mode 100644 plat/kvm/arm/setup.c
  create mode 100644 plat/kvm/arm/time.c


Cheers,

--
Julien Grall

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