Re: [Xen-devel] [PATCH v2 1/2] xen/arm: link: Link proc_info_list in .data instead of .init.data

[Julien Grall <julien.grall@xxxxxxx>]

Hi Oleksandr,

Please configure your client to quote with > rather than indentation. Below is 
an example how it can get messy in just 3 answers, I let you imagine with longer 
thread :).
This is a general issue with gmail web-interface that quote using tabulation. It 
would be best if you use an external mail client (e.g thunderbird, mute...).
On 07/12/2018 10:32, Oleksandr Tyshchenko wrote:
> On Fri, Dec 7, 2018 at 12:05 PM Julien Grall <julien.grall@xxxxxxx 
> <mailto:julien.grall@xxxxxxx>> wrote:
>     Hi Oleksandr,
>
> Hi Julien
>
>
>     On 07/12/2018 09:45, Oleksandr Tyshchenko wrote:
>      > From: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx
>     <mailto:oleksandr_tyshchenko@xxxxxxxx>>
>      >
>      > 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
>     <http://init.proc.info>.
>
> ok
>
>
>      >
>      > Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx
>     <mailto:oleksandr_tyshchenko@xxxxxxxx>>
>      >
>      > ---
>      >      Changes in v2:
>      >          - Fix typoes
>      >          - Rename ".init.proc.info <http://init.proc.info>" to
>     ".data.proc.info <http://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 <http://init.proc.info>", #alloc,
>     #execinstr
>      > +        .section ".data.proc.info <http://data.proc.info>", #alloc,
>     #execinstr
>
>     NIT: .proc.info <http://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 <http://init.proc.info>", #alloc,
>     #execinstr
>      > +        .section ".data.proc.info <http://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 <http://init.proc.info>", #alloc,
>     #execinstr
>      > +        .section ".data.proc.info <http://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 <http://data.proc.info>)
>      > +       __proc_info_end = .;
>
>     Sorry I haven't noticed it in the previous version. proc.info
>     <http://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?
Yes please. I will commit the other soon.

Cheers,

--
Julien Grall

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