Hi Julien,
> -----Original Message-----
> >>>
> >>> In this patch we introduce .text.idmap to head_mmu.S, and
> >>> add this section after .text.header. to ensure code of
> >>> head_mmu.S after the code of header.S.
> >>>
> >>> After this, we will still include some code that does not
> >>> belong to identity map before _end_boot. Because we have
> >>> moved _end_boot to head_mmu.S.
> >>
> >> I dislike this approach because you are expecting that only head_mmu.S
> >> will be part of .text.idmap. If it is not, everything could blow up
> again.
> >>
> >
> > I agree.
> >
> >> That said, if you look at staging, you will notice that now _end_boot
> is
> >> defined in the linker script to avoid any issue.
> >>
> >
> > Sorry, I am not quite clear about this comment. The _end_boot of
> original
> > staging branch is defined in head.S. And I am not quite sure how this
> > _end_boot solve multiple files contain idmap code.
>
> If you look at the latest staging, there is a commit (229ebd517b9d) that
> now define _end_boot in the linker script.
>
> The .text.idmap section can be added before the definition of _end_boot.
>
Oh, my branch was a little old, I have seen this new definition in xen.ld.S
after I update the branch. I understand now.
> >
> > Cheers,
> > Wei Chen
> >
> >>> That means all code in head.S
> >>> will be included before _end_boot. In this patch, we also
> >>> added .text flag in the place of original _end_boot in head.S.
> >>> All the code after .text in head.S will not be included in
> >>> identity map section.
> >>>
> >>> Signed-off-by: Wei Chen <wei.c...@arm.com>
> >>> ---
> >>> v1 -> v2:
> >>> 1. New patch.
> >>> ---
> >>> xen/arch/arm/arm64/head.S | 6 ++++++
> >>> xen/arch/arm/arm64/head_mmu.S | 2 +-
> >>> xen/arch/arm/xen.lds.S | 1 +
> >>> 3 files changed, 8 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> >>> index 5cfa47279b..782bd1f94c 100644
> >>> --- a/xen/arch/arm/arm64/head.S
> >>> +++ b/xen/arch/arm/arm64/head.S
> >>> @@ -466,6 +466,12 @@ fail: PRINT("- Boot failed -\r\n")
> >>> b 1b
> >>> ENDPROC(fail)
> >>>
> >>> +/*
> >>> + * For the code that do not need in indentity map section,
> >>> + * we put them back to normal .text section
> >>> + */
> >>> +.section .text, "ax", %progbits
> >>> +
> >>
> >> I would argue that puts wants to be part of the idmap.
> >>
> >
> > I am ok to move puts to idmap. But from the original head.S, puts is
> > placed after _end_boot, and from the xen.ld.S, we can see idmap is
> > area is the section of "_end_boot - start".
>
> The original position of _end_boot is wrong. It didn't take into account
> the literal pool (there are at the end of the unit). So they would be
> past _end_boot.
>
Ok.
> > The reason of moving puts
> > to idmap is because we're using it in idmap?
>
> I guess it depends of what idmap really mean here. If you only interpret
> as the MMU is on and VA == PA. Then not yet (I was thinking to introduce
> a few calls).
>
> If you also include the MMU off. Then yes.
>
> Also, in the context of cache coloring, we will need to have a
> trampoline for cache coloring. So it would be better to keep everything
> close together as it is easier to copy.
>
Understand, thanks!
Cheers,
Wei Chen
> Cheers,
>
> --
> Julien Grall
