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

[Oleksandr <olekstysh@gmail.com>]

On 07.12.18 12:34, Julien Grall wrote:
> Hi Oleksandr,

Hi Julien


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

Have already configured, sorry for the inconvenience.


>
> On 07/12/2018 10:32, Oleksandr Tyshchenko wrote:
>>
>>
>> On Fri, Dec 7, 2018 at 12:05 PM Julien Grall <julien.grall@arm.com 
>> <mailto: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
>>     <mailto: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
>>     <http://init.proc.info>.
>>
>> ok
>>
>>
>>      >
>>      > Signed-off-by: Oleksandr Tyshchenko 
>> <oleksandr_tyshchenko@epam.com
>>     <mailto:oleksandr_tyshchenko@epam.com>>
>>      >
>>      > ---
>>      >      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.

Done.

https://lists.xenproject.org/archives/html/xen-devel/2018-12/msg00777.html


>
> Cheers,
>
-- 
Regards,

Oleksandr Tyshchenko


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