[PATCH] arm32: Avoid using solaris syntax for .section directive

Khem Raj posted 1 patch 9 months, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20230801002853.556437-1-raj.khem@gmail.com
There is a newer version of this series
xen/arch/arm/arm32/proc-v7.S | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
[PATCH] arm32: Avoid using solaris syntax for .section directive
Posted by Khem Raj 9 months, 1 week ago
Assembler from binutils 2.41 rejects this syntax

.section "name"[, flags...]

where flags could be #alloc, #write, #execstr
Switch to using ELF syntax

.section name[, "flags"[, @type]]

[1] https://ftp.gnu.org/old-gnu/Manuals/gas-2.9.1/html_chapter/as_7.html#SEC119

Signed-off-by: Khem Raj <raj.khem@gmail.com>
---
 xen/arch/arm/arm32/proc-v7.S | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/arm32/proc-v7.S b/xen/arch/arm/arm32/proc-v7.S
index c90a31d80f..6d3d19b873 100644
--- a/xen/arch/arm/arm32/proc-v7.S
+++ b/xen/arch/arm/arm32/proc-v7.S
@@ -29,7 +29,7 @@ brahma15mp_init:
         mcr   CP32(r0, ACTLR)
         mov   pc, lr
 
-        .section ".proc.info", #alloc
+        .section .proc.info, "a"
         .type __v7_ca15mp_proc_info, #object
 __v7_ca15mp_proc_info:
         .long 0x410FC0F0             /* Cortex-A15 */
@@ -38,7 +38,7 @@ __v7_ca15mp_proc_info:
         .long caxx_processor
         .size __v7_ca15mp_proc_info, . - __v7_ca15mp_proc_info
 
-        .section ".proc.info", #alloc
+        .section .proc.info, "a"
         .type __v7_ca7mp_proc_info, #object
 __v7_ca7mp_proc_info:
         .long 0x410FC070             /* Cortex-A7 */
@@ -47,7 +47,7 @@ __v7_ca7mp_proc_info:
         .long caxx_processor
         .size __v7_ca7mp_proc_info, . - __v7_ca7mp_proc_info
 
-        .section ".proc.info", #alloc
+        .section .proc.info, "a"
         .type __v7_brahma15mp_proc_info, #object
 __v7_brahma15mp_proc_info:
         .long 0x420F00F0             /* Broadcom Brahma-B15 */
-- 
2.41.0
Re: [PATCH] arm32: Avoid using solaris syntax for .section directive
Posted by Michal Orzel 9 months, 1 week ago
Hi,

On 01/08/2023 02:28, Khem Raj wrote:
> 
> 
> Assembler from binutils 2.41 rejects this syntax
> 
> .section "name"[, flags...]
> 
> where flags could be #alloc, #write, #execstr
s/execstr/execinstr + there is also #exclude and #tls if you want to list them all

> Switch to using ELF syntax
> 
> .section name[, "flags"[, @type]]
> 
> [1] https://ftp.gnu.org/old-gnu/Manuals/gas-2.9.1/html_chapter/as_7.html#SEC119
I think it would be better to add a link to 2.41 docs instead or to refer to the following commit
of binutils:
4cb88cfae843 "PR11601, Solaris assembler compatibility doesn't work"


> 
> Signed-off-by: Khem Raj <raj.khem@gmail.com>
> ---
>  xen/arch/arm/arm32/proc-v7.S | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/arm/arm32/proc-v7.S b/xen/arch/arm/arm32/proc-v7.S
> index c90a31d80f..6d3d19b873 100644
> --- a/xen/arch/arm/arm32/proc-v7.S
> +++ b/xen/arch/arm/arm32/proc-v7.S
> @@ -29,7 +29,7 @@ brahma15mp_init:
>          mcr   CP32(r0, ACTLR)
>          mov   pc, lr
> 
> -        .section ".proc.info", #alloc
> +        .section .proc.info, "a"
>          .type __v7_ca15mp_proc_info, #object
>  __v7_ca15mp_proc_info:
>          .long 0x410FC0F0             /* Cortex-A15 */
> @@ -38,7 +38,7 @@ __v7_ca15mp_proc_info:
>          .long caxx_processor
>          .size __v7_ca15mp_proc_info, . - __v7_ca15mp_proc_info
> 
> -        .section ".proc.info", #alloc
> +        .section .proc.info, "a"
>          .type __v7_ca7mp_proc_info, #object
>  __v7_ca7mp_proc_info:
>          .long 0x410FC070             /* Cortex-A7 */
> @@ -47,7 +47,7 @@ __v7_ca7mp_proc_info:
>          .long caxx_processor
>          .size __v7_ca7mp_proc_info, . - __v7_ca7mp_proc_info
> 
> -        .section ".proc.info", #alloc
> +        .section .proc.info, "a"
>          .type __v7_brahma15mp_proc_info, #object
>  __v7_brahma15mp_proc_info:
>          .long 0x420F00F0             /* Broadcom Brahma-B15 */
> --
> 2.41.0
> 
> 

The patch looks good but a fast grep shows that ".section .dtb,#alloc" in arch/arm/dtb.S would also want
to be changed (I do not have gas 2.41, so you can check it by specifying dtb to be included in Xen image through
"menuconfig->Common Features->Absolute path to device tree blob")

~Michal
Re: [PATCH] arm32: Avoid using solaris syntax for .section directive
Posted by Khem Raj 9 months, 1 week ago
On Tue, Aug 1, 2023 at 12:39 AM Michal Orzel <michal.orzel@amd.com> wrote:
>
> Hi,
>
> On 01/08/2023 02:28, Khem Raj wrote:
> >
> >
> > Assembler from binutils 2.41 rejects this syntax
> >
> > .section "name"[, flags...]
> >
> > where flags could be #alloc, #write, #execstr
> s/execstr/execinstr + there is also #exclude and #tls if you want to list them all
>
> > Switch to using ELF syntax
> >
> > .section name[, "flags"[, @type]]
> >
> > [1] https://ftp.gnu.org/old-gnu/Manuals/gas-2.9.1/html_chapter/as_7.html#SEC119
> I think it would be better to add a link to 2.41 docs instead or to refer to the following commit
> of binutils:
> 4cb88cfae843 "PR11601, Solaris assembler compatibility doesn't work"
>
>
> >
> > Signed-off-by: Khem Raj <raj.khem@gmail.com>
> > ---
> >  xen/arch/arm/arm32/proc-v7.S | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/xen/arch/arm/arm32/proc-v7.S b/xen/arch/arm/arm32/proc-v7.S
> > index c90a31d80f..6d3d19b873 100644
> > --- a/xen/arch/arm/arm32/proc-v7.S
> > +++ b/xen/arch/arm/arm32/proc-v7.S
> > @@ -29,7 +29,7 @@ brahma15mp_init:
> >          mcr   CP32(r0, ACTLR)
> >          mov   pc, lr
> >
> > -        .section ".proc.info", #alloc
> > +        .section .proc.info, "a"
> >          .type __v7_ca15mp_proc_info, #object
> >  __v7_ca15mp_proc_info:
> >          .long 0x410FC0F0             /* Cortex-A15 */
> > @@ -38,7 +38,7 @@ __v7_ca15mp_proc_info:
> >          .long caxx_processor
> >          .size __v7_ca15mp_proc_info, . - __v7_ca15mp_proc_info
> >
> > -        .section ".proc.info", #alloc
> > +        .section .proc.info, "a"
> >          .type __v7_ca7mp_proc_info, #object
> >  __v7_ca7mp_proc_info:
> >          .long 0x410FC070             /* Cortex-A7 */
> > @@ -47,7 +47,7 @@ __v7_ca7mp_proc_info:
> >          .long caxx_processor
> >          .size __v7_ca7mp_proc_info, . - __v7_ca7mp_proc_info
> >
> > -        .section ".proc.info", #alloc
> > +        .section .proc.info, "a"
> >          .type __v7_brahma15mp_proc_info, #object
> >  __v7_brahma15mp_proc_info:
> >          .long 0x420F00F0             /* Broadcom Brahma-B15 */
> > --
> > 2.41.0
> >
> >
>
> The patch looks good but a fast grep shows that ".section .dtb,#alloc" in arch/arm/dtb.S would also want
> to be changed (I do not have gas 2.41, so you can check it by specifying dtb to be included in Xen image through
> "menuconfig->Common Features->Absolute path to device tree blob")

thanks will add it to v2 as well

>
> ~Michal