[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Minios-devel] [PATCH 00/47] MINI-OS: enable the arm64 support
Hi,
On 16/03/18 02:15, Huang Shijie wrote:
On Thu, Mar 15, 2018 at 10:51:56AM +0000, Julien Grall wrote:
Hi,
On 15/03/18 04:48, Huang Shijie wrote:
On Wed, Mar 14, 2018 at 10:21:52AM +0000, Julien Grall wrote:
Hi Julien,
I feel sorry that the patch set was not sent outsides.
I checked the archive for minios, and I did not find the email.
It seems there is something wrong with my git config, I will check it,
and fix it, and send it again.
Are you registered on the minios mailing list?
I did not registered on the minios mailing list, I check it by the archive.
Few generic comments on this series.
On 03/14/2018 09:39 AM, Huang Shijie wrote:
2.) Tests
I tested this patch set on Softiron(arm64) and x86_64 platform.
How about arm32? What is the state after this series?
I did not test the arm32, since it even can not pass the compiler for arm32.
I suggest we do not care about the arm32, and fix it after the arm64 code is
merged
in future.
Well, we already had a discussion on this on the previous version and agreed
on a plan. I would like to understand why this was not followed?
I think I have followed the plan:
1.) change the DTC as a folder, not the submodule.
2.) refactor the arm32 code the separate folders.
Which is missing from the plan?
Dropping arm32 code, this is what I meant by "clean slate" and clarified
by various e-mail privately and on the mailing list. See Wei's answer [1]:
"If you're sure arm32 can't work, #2 is probably easier. Please stick a
patch at the beginning to rip out the old port. That can easily be
applied."
On the previous version, I clearly suggested 2 paths to add support for
arm64:
"I can see two solutions going forward:
1) The arm directory is first reshaped to welcome arm64. This means:
* moving out arm32 specific code
* switch to LPAE page-table
* introducing helpers for common code to call arch-specific
code
On the code is reshaped, the arm64 series is added on top.
2) Start the arm64 port from a clean slate and then port arm32 over.
Knowing the state of the arm32 port, I would lean towards 2). This would
allow more flexibility and make easier to review. At the moment, I have to
hunt down the code to see what is missing."
This series does not follow any of them and end up to have #if
defined(__aarch64__) in the common code. This really defeating the purpose
of the refactoring below.
To be clear, I am not suggesting to add arm32 port, I am just asking to not
make things worst than the current state.
The current state is already very worst for arm32 now. :)
Without this patch set, the arm32 is not work; with this patch set, the arm32
still cannot work...
So what's the point to keep that code around? This making this series nearly
I moved the arm32 code to the separate folder, and do not change it.
You didn't change that code but change quite heavily the common code by
spreading #ifdef aarch64 in it. This design looks completely wrong and
clearly show that the two port are currently not that compatible. Indeed
the memory management is quite different (LPAE vs short page-table).
I thought I have done it from a clear slate.
Now, I found I feel confused about the "clean slate"..
Clean slate means:
"a state in which you are starting an activity or process again, not
considering what has happened in the past at all"
In that particular context it means removing the arm32 port and start
from scratch.
There are clearly no point to keep code around that are completely wrong
and where I have no way to verify whether this is valid change without
having to spend a significant amount of time to hunt down what are missing.
Cheers,
[1]
https://lists.xenproject.org/archives/html/minios-devel/2017-11/msg00136.html
--
Julien Grall
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel
|
