Re: [Minios-devel] [PATCH 00/47] MINI-OS: enable the arm64 support

[Huang Shijie <shijie.huang@xxxxxxx>]

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.

> Hi Shijie,
> 
> 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.

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

> 
> >   4.) The comments from Julien which I did not follow:
> >      0) Use p2m_set_entry to setup the page table.
> >         I did not have good understanding of the p2m_set_entry().
> >     So I use my familiar way to setup the page table.
> 
> I haven't seen any discussion from you since November about misunderstanding
> in my suggestions...
I added the map/munmap which you concerned before.
> 
> >
> >      1) Move dsb/dmb/mb to common place.
> >         I did not do so, because I found the older arm32 archs
> >         use "mcr" to do the memory barriers.
> 
> Hu? B4.2.5 in ARM DDI  0406C.c says:
> 
> "In ARMv7, ARM deprecates any use of these CP15 c7 operations, and strongly
> recommends that software uses the ISB , DSB , and DMB instructions instead."
> 
> Mini-OS will always run on ARMv7 or later architecture.

okay, I can add extra patch to fix it.

Thanks
Huang Shijie

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel