On Fri, Dec 7, 2018 at 12:05 PM Julien Grall <julien.grall@arm.com> wrote:

> Hi Oleksandr,
>
Hi Julien


>
> On 07/12/2018 09:45, Oleksandr Tyshchenko wrote:
> > From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> >
> > To be able to use it for the hot-plugged CPUs as well.
>
> You need to explain in the commit message why you rename .init.proc.info.
>
ok


>
> >
> > Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> >
> > ---
> >      Changes in v2:
> >          - Fix typoes
> >          - Rename ".init.proc.info" to ".data.proc.info"
> > ---
> >   xen/arch/arm/arm32/proc-v7.S |  6 +++---
> >   xen/arch/arm/xen.lds.S       | 10 ++++++----
> >   2 files changed, 9 insertions(+), 7 deletions(-)
> >
> > diff --git a/xen/arch/arm/arm32/proc-v7.S b/xen/arch/arm/arm32/proc-v7.S
> > index 80a250d..a0c0c9e 100644
> > --- a/xen/arch/arm/arm32/proc-v7.S
> > +++ b/xen/arch/arm/arm32/proc-v7.S
> > @@ -30,7 +30,7 @@ brahma15mp_init:
> >           mcr   CP32(r0, ACTLR)
> >           mov   pc, lr
> >
> > -        .section ".init.proc.info", #alloc, #execinstr
> > +        .section ".data.proc.info", #alloc, #execinstr
>
> NIT: .proc.info would be sufficient. This would avoid to rename again the
> section if we decide to move it in a separate section.
>
ok


>
> >           .type __v7_ca15mp_proc_info, #object
> >   __v7_ca15mp_proc_info:
> >           .long 0x410FC0F0             /* Cortex-A15 */
> > @@ -39,7 +39,7 @@ __v7_ca15mp_proc_info:
> >           .long caxx_processor
> >           .size __v7_ca15mp_proc_info, . - __v7_ca15mp_proc_info
> >
> > -        .section ".init.proc.info", #alloc, #execinstr
> > +        .section ".data.proc.info", #alloc, #execinstr
> >           .type __v7_ca7mp_proc_info, #object
> >   __v7_ca7mp_proc_info:
> >           .long 0x410FC070             /* Cortex-A7 */
> > @@ -48,7 +48,7 @@ __v7_ca7mp_proc_info:
> >           .long caxx_processor
> >           .size __v7_ca7mp_proc_info, . - __v7_ca7mp_proc_info
> >
> > -        .section ".init.proc.info", #alloc, #execinstr
> > +        .section ".data.proc.info", #alloc, #execinstr
> >           .type __v7_brahma15mp_proc_info, #object
> >   __v7_brahma15mp_proc_info:
> >           .long 0x420F00F0             /* Broadcom Brahma-B15 */
> > diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
> > index 245a0e0..624da9b 100644
> > --- a/xen/arch/arm/xen.lds.S
> > +++ b/xen/arch/arm/xen.lds.S
> > @@ -88,6 +88,12 @@ SECTIONS
> >          . = ALIGN(PAGE_SIZE);
> >          *(.data.page_aligned)
> >          *(.data)
> > +
> > +       . = ALIGN(POINTER_ALIGN);
> > +       __proc_info_start = .;
> > +       *(.data.proc.info)
> > +       __proc_info_end = .;
>
> Sorry I haven't noticed it in the previous version. proc.info should
> never
> modified, right? So it should be in rodata in that case.
>
agree, will link it in rodata.

May I resend just this patch after address your comments?


>
> > +
> >          . = ALIGN(8);
> >          __start_schedulers_array = .;
> >          *(.data.schedulers)
> > @@ -151,10 +157,6 @@ SECTIONS
> >          *(.init.setup)
> >          __setup_end = .;
> >
> > -       __proc_info_start = .;
> > -       *(.init.proc.info)
> > -       __proc_info_end = .;
> > -
> >          __initcall_start = .;
> >          *(.initcallpresmp.init)
> >          __presmp_initcall_end = .;
> >
>
> Cheers,
>
> --
> Julien Grall
>


-- 
Regards,

Oleksandr Tyshchenko