|
[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 Julien, Thanks for your comments. > -----Original Message----- > From: Julien Grall <julien.grall@xxxxxxx> > Sent: 2018年7月8日 5:39 > To: Wei Chen <Wei.Chen@xxxxxxx>; minios-devel@xxxxxxxxxxxxxxxxxxxx; > simon.kuenzer@xxxxxxxxx > Cc: Kaly Xin <Kaly.Xin@xxxxxxx>; nd <nd@xxxxxxx> > Subject: 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? Sorry, I sent two versions in mailing list using the name "Prepare build scripts to support ARM64". And sent v3 in github including Arm64 code for demo. Because at that time, Simon was very busy, and he Wasn't ready to review my code. The v4 was generated by my local script, I think it should be v3 in this ML. > > 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? > As I had discussed with Simon before, we planned to enable Arm64/KVM on QEMU first. Because QEMU-KVM is the de-facto implementation of KVM. After features on QEMU-KVM become mature, we will add other hypervisors like kvmtools and ukvm later. > > 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. > Sorry, I don't understand your comment here clear. What did "assumption on the memory layout" mean here? My platform is QEMU-KVM virt, I think the memory layout is clear and fixed. Except the PL011 UART for early debug, I parsed memory, command line, PSCI and PL011 for console from device tree. I will remove the PL011 early Debug later, if we will not need it anymore. And I hardcored the offsets of DTB, page table and stack. But I think these Offsets couldn't be parsed from device tree. > > 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. At the very beginning, Simon and I decided to send the first series Without any arm64/KVM support code, just including the folder layout and scripts modification. But after that, Unikraft added lots of code. The multi-arch modification inevitably affected the plat/ folders. We moved some code to plat/common. So I combined two series into one. I think this would give reviewers an direct overview. > > Also can you provide a git branch with your code? Of course, you can find the branch on github: https://github.com/Weichen81/unikraft/tree/upstream > > > 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
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |