[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

On 09/07/18 08:20, Wei Chen wrote:
Hi Julien,

Hi Wei,

-----Original Message-----
From: Julien Grall <julien.grall@xxxxxxx>
Sent: 2018年7月8日 5:39
To: Wei Chen <Wei.Chen@xxxxxxx>; minios-devel@xxxxxxxxxxxxxxxxxxxx;
Cc: Kaly Xin <Kaly.Xin@xxxxxxx>; nd <nd@xxxxxxx>
Subject: Re: [Minios-devel] [UNIKRAFT PATCHv4 00/43] Add arm64/kvm support for

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.

Please mention such renaming in the cover letter in the future.

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

kvmtools and ukvm are not different hypervisors. They are different tools/monitor to create your VM.

So your Unikraft binary would be tailored to a given monitor? Or you do you plan to support all of them in one?

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.

The main goal of Device-Tree is to abstract the platform and therefore avoiding hardcoded and make your kernel image more generic.

If we take the example of the PL011, you are mapping the region at a fixed address when creating the page-tables but get the base-address from the Device-Tree. What is the benefits to have some time use DT and sometimes hardcode the value?

My point is, if you know your platform layout, then you have no need to use Device-Tree to get the base address for PL011 or whether PSCI will use HVC/SMC.

If you plan to tailor your kernel image to a given platform then you should not use DT for other bits than memory size/command line.

If you plan to have generic image, then you should use Device-Tree everywhere.

Both of them have pros/cons, I would prefer the latter but also understand the reason of using the latter. What I would like to avoid is a mix of both solutions, this is the worst of all.

And I hardcored the offsets of DTB, page table and stack. But I think
these Offsets couldn't be parsed from device tree.

I wasn't referring to that :).

6. A simple PSCI library for CPU suspend, reset and system
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.

TBH, I don't see any benefits to have the arm64 code for the reworking. The main issue with big series is the length of reviewing and can also be discouraging for the contributors as it requires to be spotless on changes.


Julien Grall

Minios-devel mailing list



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