From mboxrd@z Thu Jan 1 00:00:00 1970 From: Oleksandr Tyshchenko Subject: Re: [PATCH v2 1/2] xen/arm: link: Link proc_info_list in .data instead of .init.data Date: Fri, 7 Dec 2018 12:32:11 +0200 Message-ID: References: <1544175931-17047-1-git-send-email-olekstysh@gmail.com> <1544175931-17047-2-git-send-email-olekstysh@gmail.com> <5a524560-109e-55b2-922f-36d9449adab1@arm.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============8491397781482598907==" Return-path: Received: from us1-rack-dfw2.inumbo.com ([104.130.134.6]) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1gVDQm-0007ob-VE for xen-devel@lists.xenproject.org; Fri, 07 Dec 2018 10:32:25 +0000 Received: by mail-io1-xd43.google.com with SMTP id l14so2863084ioj.5 for ; Fri, 07 Dec 2018 02:32:23 -0800 (PST) In-Reply-To: <5a524560-109e-55b2-922f-36d9449adab1@arm.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" To: Julien Grall Cc: xen-devel , Stefano Stabellini , Mirela Simonovic , Oleksandr Tyshchenko List-Id: xen-devel@lists.xenproject.org --===============8491397781482598907== Content-Type: multipart/alternative; boundary="0000000000006af316057c6c21e5" --0000000000006af316057c6c21e5 Content-Type: text/plain; charset="UTF-8" On Fri, Dec 7, 2018 at 12:05 PM Julien Grall wrote: > Hi Oleksandr, > Hi Julien > > On 07/12/2018 09:45, Oleksandr Tyshchenko wrote: > > From: Oleksandr Tyshchenko > > > > 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 > > > > --- > > 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 --0000000000006af316057c6c21e5 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


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

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
=C2=A0

>
> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com><= br> >
> ---
>=C2=A0 =C2=A0 =C2=A0 Changes in v2:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 - Fix typoes
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 - Rename ".init.proc.info"= to ".data.proc.info"
> ---
>=C2=A0 =C2=A0xen/arch/arm/arm32/proc-v7.S |=C2=A0 6 +++---
>=C2=A0 =C2=A0xen/arch/arm/xen.lds.S=C2=A0 =C2=A0 =C2=A0 =C2=A0| 10 ++++= ++----
>=C2=A0 =C2=A02 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:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0mcr=C2=A0 =C2=A0CP32(r0, ACTLR= )
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0mov=C2=A0 =C2=A0pc, lr
>=C2=A0 =C2=A0
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 .section ".init.proc.info", #al= loc, #execinstr
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 .section ".data.proc.info", #al= loc, #execinstr

NIT: .pro= c.info would be sufficient. This would avoid to rename again the
section if we decide to move it in a separate section.
ok
=C2=A0

>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0.type __v7_ca15mp_proc_info, #= object
>=C2=A0 =C2=A0__v7_ca15mp_proc_info:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0.long 0x410FC0F0=C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* Cortex-A15 */
> @@ -39,7 +39,7 @@ __v7_ca15mp_proc_info:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0.long caxx_processor
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0.size __v7_ca15mp_proc_info, .= - __v7_ca15mp_proc_info
>=C2=A0 =C2=A0
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 .section ".init.proc.info", #al= loc, #execinstr
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 .section ".data.proc.info", #al= loc, #execinstr
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0.type __v7_ca7mp_proc_info, #o= bject
>=C2=A0 =C2=A0__v7_ca7mp_proc_info:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0.long 0x410FC070=C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* Cortex-A7 */
> @@ -48,7 +48,7 @@ __v7_ca7mp_proc_info:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0.long caxx_processor
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0.size __v7_ca7mp_proc_info, . = - __v7_ca7mp_proc_info
>=C2=A0 =C2=A0
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 .section ".init.proc.info", #al= loc, #execinstr
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 .section ".data.proc.info", #al= loc, #execinstr
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0.type __v7_brahma15mp_proc_inf= o, #object
>=C2=A0 =C2=A0__v7_brahma15mp_proc_info:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0.long 0x420F00F0=C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* 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
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 . =3D ALIGN(PAGE_SIZE);
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 *(.data.page_aligned)
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 *(.data)
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0. =3D ALIGN(POINTER_ALIGN);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0__proc_info_start =3D .;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0*(.data.proc.info)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0__proc_info_end =3D .;

Sorry I haven't noticed it in the previous version. proc.info should never <= br> modified, right? So it should be in rodata in that case.
agree, will link it in=C2=A0rodata.

May I resen= d just this patch after address your comments?
=C2=A0

> +
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 . =3D ALIGN(8);
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 __start_schedulers_array =3D .;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 *(.data.schedulers)
> @@ -151,10 +157,6 @@ SECTIONS
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 *(.init.setup)
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 __setup_end =3D .;
>=C2=A0 =C2=A0
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0__proc_info_start =3D .;
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0*(.init.proc.info)
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0__proc_info_end =3D .;
> -
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 __initcall_start =3D .;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 *(.initcallpresmp.init)
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 __presmp_initcall_end =3D .;
>

Cheers,

--
Julien Grall


--
Reg= ards,

Ol= eksandr Tyshchenko
<= /div> --0000000000006af316057c6c21e5-- --===============8491397781482598907== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVucHJvamVjdC5vcmcKaHR0cHM6Ly9saXN0 cy54ZW5wcm9qZWN0Lm9yZy9tYWlsbWFuL2xpc3RpbmZvL3hlbi1kZXZlbA== --===============8491397781482598907==--