2023-05-09 07:27:36

by Tiezhu Yang

[permalink] [raw]
Subject: [PATCH] perf symbol: Add LoongArch case in get_plt_sizes()

We can see the following definitions in bfd/elfnn-loongarch.c:

#define PLT_HEADER_INSNS 8
#define PLT_HEADER_SIZE (PLT_HEADER_INSNS * 4)

#define PLT_ENTRY_INSNS 4
#define PLT_ENTRY_SIZE (PLT_ENTRY_INSNS * 4)

so plt header size is 32 and plt entry size is 16 on LoongArch,
let us add LoongArch case in get_plt_sizes().

Link: https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=bfd/elfnn-loongarch.c
Signed-off-by: Tiezhu Yang <[email protected]>
---

This is based on 6.4-rc1

tools/perf/util/symbol-elf.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index b2ed9cc..5d409c2 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -411,6 +411,10 @@ static bool get_plt_sizes(struct dso *dso, GElf_Ehdr *ehdr, GElf_Shdr *shdr_plt,
*plt_header_size = 32;
*plt_entry_size = 16;
return true;
+ case EM_LOONGARCH:
+ *plt_header_size = 32;
+ *plt_entry_size = 16;
+ return true;
case EM_SPARC:
*plt_header_size = 48;
*plt_entry_size = 12;
--
2.1.0


2023-05-18 02:20:45

by Huacai Chen

[permalink] [raw]
Subject: Re: [PATCH] perf symbol: Add LoongArch case in get_plt_sizes()

Queued, thanks.

Huacai

On Tue, May 9, 2023 at 2:56 PM Tiezhu Yang <[email protected]> wrote:
>
> We can see the following definitions in bfd/elfnn-loongarch.c:
>
> #define PLT_HEADER_INSNS 8
> #define PLT_HEADER_SIZE (PLT_HEADER_INSNS * 4)
>
> #define PLT_ENTRY_INSNS 4
> #define PLT_ENTRY_SIZE (PLT_ENTRY_INSNS * 4)
>
> so plt header size is 32 and plt entry size is 16 on LoongArch,
> let us add LoongArch case in get_plt_sizes().
>
> Link: https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=bfd/elfnn-loongarch.c
> Signed-off-by: Tiezhu Yang <[email protected]>
> ---
>
> This is based on 6.4-rc1
>
> tools/perf/util/symbol-elf.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> index b2ed9cc..5d409c2 100644
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
> @@ -411,6 +411,10 @@ static bool get_plt_sizes(struct dso *dso, GElf_Ehdr *ehdr, GElf_Shdr *shdr_plt,
> *plt_header_size = 32;
> *plt_entry_size = 16;
> return true;
> + case EM_LOONGARCH:
> + *plt_header_size = 32;
> + *plt_entry_size = 16;
> + return true;
> case EM_SPARC:
> *plt_header_size = 48;
> *plt_entry_size = 12;
> --
> 2.1.0
>
>

2023-05-18 03:16:42

by Huacai Chen

[permalink] [raw]
Subject: Re: [PATCH] perf symbol: Add LoongArch case in get_plt_sizes()

On Thu, May 18, 2023 at 11:06 AM Leo Yan <[email protected]> wrote:
>
> On Thu, May 18, 2023 at 10:11:27AM +0800, Huacai Chen wrote:
> > Queued, thanks.
>
> The patch is fine for me.
>
> Should not perf patches are to be merged via Arnaldo's tree?
I think both are OK, if Arnaldo takes this patch, I will drop it.

Huacai
>
> Thanks,
> Leo
>
> > On Tue, May 9, 2023 at 2:56 PM Tiezhu Yang <[email protected]> wrote:
> > >
> > > We can see the following definitions in bfd/elfnn-loongarch.c:
> > >
> > > #define PLT_HEADER_INSNS 8
> > > #define PLT_HEADER_SIZE (PLT_HEADER_INSNS * 4)
> > >
> > > #define PLT_ENTRY_INSNS 4
> > > #define PLT_ENTRY_SIZE (PLT_ENTRY_INSNS * 4)
> > >
> > > so plt header size is 32 and plt entry size is 16 on LoongArch,
> > > let us add LoongArch case in get_plt_sizes().
> > >
> > > Link: https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=bfd/elfnn-loongarch.c
> > > Signed-off-by: Tiezhu Yang <[email protected]>
> > > ---
> > >
> > > This is based on 6.4-rc1
> > >
> > > tools/perf/util/symbol-elf.c | 4 ++++
> > > 1 file changed, 4 insertions(+)
> > >
> > > diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> > > index b2ed9cc..5d409c2 100644
> > > --- a/tools/perf/util/symbol-elf.c
> > > +++ b/tools/perf/util/symbol-elf.c
> > > @@ -411,6 +411,10 @@ static bool get_plt_sizes(struct dso *dso, GElf_Ehdr *ehdr, GElf_Shdr *shdr_plt,
> > > *plt_header_size = 32;
> > > *plt_entry_size = 16;
> > > return true;
> > > + case EM_LOONGARCH:
> > > + *plt_header_size = 32;
> > > + *plt_entry_size = 16;
> > > + return true;
> > > case EM_SPARC:
> > > *plt_header_size = 48;
> > > *plt_entry_size = 12;
> > > --
> > > 2.1.0
> > >
> > >

2023-05-18 03:18:02

by Leo Yan

[permalink] [raw]
Subject: Re: [PATCH] perf symbol: Add LoongArch case in get_plt_sizes()

On Thu, May 18, 2023 at 10:11:27AM +0800, Huacai Chen wrote:
> Queued, thanks.

The patch is fine for me.

Should not perf patches are to be merged via Arnaldo's tree?

Thanks,
Leo

> On Tue, May 9, 2023 at 2:56 PM Tiezhu Yang <[email protected]> wrote:
> >
> > We can see the following definitions in bfd/elfnn-loongarch.c:
> >
> > #define PLT_HEADER_INSNS 8
> > #define PLT_HEADER_SIZE (PLT_HEADER_INSNS * 4)
> >
> > #define PLT_ENTRY_INSNS 4
> > #define PLT_ENTRY_SIZE (PLT_ENTRY_INSNS * 4)
> >
> > so plt header size is 32 and plt entry size is 16 on LoongArch,
> > let us add LoongArch case in get_plt_sizes().
> >
> > Link: https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=bfd/elfnn-loongarch.c
> > Signed-off-by: Tiezhu Yang <[email protected]>
> > ---
> >
> > This is based on 6.4-rc1
> >
> > tools/perf/util/symbol-elf.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> > index b2ed9cc..5d409c2 100644
> > --- a/tools/perf/util/symbol-elf.c
> > +++ b/tools/perf/util/symbol-elf.c
> > @@ -411,6 +411,10 @@ static bool get_plt_sizes(struct dso *dso, GElf_Ehdr *ehdr, GElf_Shdr *shdr_plt,
> > *plt_header_size = 32;
> > *plt_entry_size = 16;
> > return true;
> > + case EM_LOONGARCH:
> > + *plt_header_size = 32;
> > + *plt_entry_size = 16;
> > + return true;
> > case EM_SPARC:
> > *plt_header_size = 48;
> > *plt_entry_size = 12;
> > --
> > 2.1.0
> >
> >

2023-05-18 03:32:04

by Leo Yan

[permalink] [raw]
Subject: Re: [PATCH] perf symbol: Add LoongArch case in get_plt_sizes()

On Thu, May 18, 2023 at 11:12:26AM +0800, Huacai Chen wrote:
> On Thu, May 18, 2023 at 11:06 AM Leo Yan <[email protected]> wrote:
> >
> > On Thu, May 18, 2023 at 10:11:27AM +0800, Huacai Chen wrote:
> > > Queued, thanks.
> >
> > The patch is fine for me.
> >
> > Should not perf patches are to be merged via Arnaldo's tree?
>
> I think both are OK, if Arnaldo takes this patch, I will drop it.

A good practice is to firstly inquiry the maintainers.

AFAIK, Arnaldo will test perf patches before sending out pull request;
if perf patches are scattered out, it might be out of the testing
radar.

Thanks,
Leo

2023-05-18 04:21:24

by Leo Yan

[permalink] [raw]
Subject: Re: [PATCH] perf symbol: Add LoongArch case in get_plt_sizes()

On Thu, May 18, 2023 at 11:57:29AM +0800, Huacai Chen wrote:
> On Thu, May 18, 2023 at 11:21 AM Leo Yan <[email protected]> wrote:
> >
> > On Thu, May 18, 2023 at 11:12:26AM +0800, Huacai Chen wrote:
> > > On Thu, May 18, 2023 at 11:06 AM Leo Yan <[email protected]> wrote:
> > > >
> > > > On Thu, May 18, 2023 at 10:11:27AM +0800, Huacai Chen wrote:
> > > > > Queued, thanks.
> > > >
> > > > The patch is fine for me.
> > > >
> > > > Should not perf patches are to be merged via Arnaldo's tree?
> > >
> > > I think both are OK, if Arnaldo takes this patch, I will drop it.
> >
> > A good practice is to firstly inquiry the maintainers.
> >
> > AFAIK, Arnaldo will test perf patches before sending out pull request;
> > if perf patches are scattered out, it might be out of the testing
> > radar.
> OK, I know, thank you very much.

You are welcome!

I found the code base for bfd:
https://github.com/bminor/binutils-gdb/blob/master/bfd/elfnn-loongarch.c

And this patch is consistent with above link, FWIW:

Reviewed-by: Leo Yan <[email protected]>

2023-05-18 04:26:09

by Huacai Chen

[permalink] [raw]
Subject: Re: [PATCH] perf symbol: Add LoongArch case in get_plt_sizes()

On Thu, May 18, 2023 at 11:21 AM Leo Yan <[email protected]> wrote:
>
> On Thu, May 18, 2023 at 11:12:26AM +0800, Huacai Chen wrote:
> > On Thu, May 18, 2023 at 11:06 AM Leo Yan <[email protected]> wrote:
> > >
> > > On Thu, May 18, 2023 at 10:11:27AM +0800, Huacai Chen wrote:
> > > > Queued, thanks.
> > >
> > > The patch is fine for me.
> > >
> > > Should not perf patches are to be merged via Arnaldo's tree?
> >
> > I think both are OK, if Arnaldo takes this patch, I will drop it.
>
> A good practice is to firstly inquiry the maintainers.
>
> AFAIK, Arnaldo will test perf patches before sending out pull request;
> if perf patches are scattered out, it might be out of the testing
> radar.
OK, I know, thank you very much.

Huacai
>
> Thanks,
> Leo

2023-05-18 12:42:22

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH] perf symbol: Add LoongArch case in get_plt_sizes()

Em Thu, May 18, 2023 at 12:05:53PM +0800, Leo Yan escreveu:
> On Thu, May 18, 2023 at 11:57:29AM +0800, Huacai Chen wrote:
> > On Thu, May 18, 2023 at 11:21 AM Leo Yan <[email protected]> wrote:
> > >
> > > On Thu, May 18, 2023 at 11:12:26AM +0800, Huacai Chen wrote:
> > > > On Thu, May 18, 2023 at 11:06 AM Leo Yan <[email protected]> wrote:
> > > > >
> > > > > On Thu, May 18, 2023 at 10:11:27AM +0800, Huacai Chen wrote:
> > > > > > Queued, thanks.
> > > > >
> > > > > The patch is fine for me.
> > > > >
> > > > > Should not perf patches are to be merged via Arnaldo's tree?
> > > >
> > > > I think both are OK, if Arnaldo takes this patch, I will drop it.
> > >
> > > A good practice is to firstly inquiry the maintainers.
> > >
> > > AFAIK, Arnaldo will test perf patches before sending out pull request;
> > > if perf patches are scattered out, it might be out of the testing
> > > radar.
> > OK, I know, thank you very much.
>
> You are welcome!
>
> I found the code base for bfd:
> https://github.com/bminor/binutils-gdb/blob/master/bfd/elfnn-loongarch.c
>
> And this patch is consistent with above link, FWIW:
>
> Reviewed-by: Leo Yan <[email protected]>

Thanks, applied.

- Arnaldo


2023-05-22 04:12:24

by Huacai Chen

[permalink] [raw]
Subject: Re: [PATCH] perf symbol: Add LoongArch case in get_plt_sizes()

Hi, Arnaldo,

On Thu, May 18, 2023 at 8:16 PM Arnaldo Carvalho de Melo
<[email protected]> wrote:
>
> Em Thu, May 18, 2023 at 12:05:53PM +0800, Leo Yan escreveu:
> > On Thu, May 18, 2023 at 11:57:29AM +0800, Huacai Chen wrote:
> > > On Thu, May 18, 2023 at 11:21 AM Leo Yan <[email protected]> wrote:
> > > >
> > > > On Thu, May 18, 2023 at 11:12:26AM +0800, Huacai Chen wrote:
> > > > > On Thu, May 18, 2023 at 11:06 AM Leo Yan <[email protected]> wrote:
> > > > > >
> > > > > > On Thu, May 18, 2023 at 10:11:27AM +0800, Huacai Chen wrote:
> > > > > > > Queued, thanks.
> > > > > >
> > > > > > The patch is fine for me.
> > > > > >
> > > > > > Should not perf patches are to be merged via Arnaldo's tree?
> > > > >
> > > > > I think both are OK, if Arnaldo takes this patch, I will drop it.
> > > >
> > > > A good practice is to firstly inquiry the maintainers.
> > > >
> > > > AFAIK, Arnaldo will test perf patches before sending out pull request;
> > > > if perf patches are scattered out, it might be out of the testing
> > > > radar.
> > > OK, I know, thank you very much.
> >
> > You are welcome!
> >
> > I found the code base for bfd:
> > https://github.com/bminor/binutils-gdb/blob/master/bfd/elfnn-loongarch.c
> >
> > And this patch is consistent with above link, FWIW:
> >
> > Reviewed-by: Leo Yan <[email protected]>
>
> Thanks, applied.
I'm very sorry that this patch breaks cross-build. We need some
additional modification.

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 5d409c26a22e..b3dbf6ca99a7 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -35,6 +35,10 @@
#define EM_AARCH64 183 /* ARM 64 bit */
#endif

+#ifndef EM_LOONGARCH
+#define EM_LOONGARCH 258
+#endif
+
#ifndef ELF32_ST_VISIBILITY
#define ELF32_ST_VISIBILITY(o) ((o) & 0x03)
#endif

Then, drop this patch to get an updated version, or let me send an
incremental patch?

Huacai

>
> - Arnaldo
>

2023-05-22 08:06:16

by Tiezhu Yang

[permalink] [raw]
Subject: Re: [PATCH] perf symbol: Add LoongArch case in get_plt_sizes()



On 05/22/2023 11:50 AM, Huacai Chen wrote:
> Hi, Arnaldo,
>
> On Thu, May 18, 2023 at 8:16 PM Arnaldo Carvalho de Melo
> <[email protected]> wrote:
>>
>> Em Thu, May 18, 2023 at 12:05:53PM +0800, Leo Yan escreveu:
>>> On Thu, May 18, 2023 at 11:57:29AM +0800, Huacai Chen wrote:
>>>> On Thu, May 18, 2023 at 11:21 AM Leo Yan <[email protected]> wrote:
>>>>>
>>>>> On Thu, May 18, 2023 at 11:12:26AM +0800, Huacai Chen wrote:
>>>>>> On Thu, May 18, 2023 at 11:06 AM Leo Yan <[email protected]> wrote:
>>>>>>>
>>>>>>> On Thu, May 18, 2023 at 10:11:27AM +0800, Huacai Chen wrote:
>>>>>>>> Queued, thanks.
>>>>>>>
>>>>>>> The patch is fine for me.
>>>>>>>
>>>>>>> Should not perf patches are to be merged via Arnaldo's tree?
>>>>>>
>>>>>> I think both are OK, if Arnaldo takes this patch, I will drop it.
>>>>>
>>>>> A good practice is to firstly inquiry the maintainers.
>>>>>
>>>>> AFAIK, Arnaldo will test perf patches before sending out pull request;
>>>>> if perf patches are scattered out, it might be out of the testing
>>>>> radar.
>>>> OK, I know, thank you very much.
>>>
>>> You are welcome!
>>>
>>> I found the code base for bfd:
>>> https://github.com/bminor/binutils-gdb/blob/master/bfd/elfnn-loongarch.c
>>>
>>> And this patch is consistent with above link, FWIW:
>>>
>>> Reviewed-by: Leo Yan <[email protected]>
>>
>> Thanks, applied.
> I'm very sorry that this patch breaks cross-build. We need some
> additional modification.
>
> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> index 5d409c26a22e..b3dbf6ca99a7 100644
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
> @@ -35,6 +35,10 @@
> #define EM_AARCH64 183 /* ARM 64 bit */
> #endif
>
> +#ifndef EM_LOONGARCH
> +#define EM_LOONGARCH 258
> +#endif
> +
> #ifndef ELF32_ST_VISIBILITY
> #define ELF32_ST_VISIBILITY(o) ((o) & 0x03)
> #endif
>
> Then, drop this patch to get an updated version, or let me send an
> incremental patch?
>

I tested this patch on native LoongArch and x86 system, I did not
hit the build error about undeclared EM_LOONGARCH both on LoongArch
and x86, because EM_LOONGARCH is defined in /usr/include/elf.h

Here is the x86 system info:

[root@fedora yangtiezhu]# cat /etc/fedora-release
Fedora release 38 (Thirty Eight)
[root@fedora yangtiezhu]# uname -m
x86_64
[root@fedora yangtiezhu]# grep -rn -w EM_LOONGARCH /usr/include/
/usr/include/linux/elf-em.h:54:#define EM_LOONGARCH 258 /*
LoongArch */
/usr/include/linux/audit.h:442:#define AUDIT_ARCH_LOONGARCH32
(EM_LOONGARCH|__AUDIT_ARCH_LE)
/usr/include/linux/audit.h:443:#define AUDIT_ARCH_LOONGARCH64
(EM_LOONGARCH|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
/usr/include/elf.h:361:#define EM_LOONGARCH 258 /* LoongArch */
[root@fedora yangtiezhu]# rpm -qf /usr/include/elf.h
glibc-headers-x86-2.37-1.fc38.noarch

If I am missing something, please let me know.

Anyway, it is not a bad thing to add the EM_LOONGARCH definition
to avoid the build error on some systems which have no EM_LOONGARCH
in the glibc header.

Thanks,
Tiezhu


2023-05-23 01:55:48

by Huacai Chen

[permalink] [raw]
Subject: Re: [PATCH] perf symbol: Add LoongArch case in get_plt_sizes()

On Mon, May 22, 2023 at 3:59 PM Tiezhu Yang <[email protected]> wrote:
>
>
>
> On 05/22/2023 11:50 AM, Huacai Chen wrote:
> > Hi, Arnaldo,
> >
> > On Thu, May 18, 2023 at 8:16 PM Arnaldo Carvalho de Melo
> > <[email protected]> wrote:
> >>
> >> Em Thu, May 18, 2023 at 12:05:53PM +0800, Leo Yan escreveu:
> >>> On Thu, May 18, 2023 at 11:57:29AM +0800, Huacai Chen wrote:
> >>>> On Thu, May 18, 2023 at 11:21 AM Leo Yan <[email protected]> wrote:
> >>>>>
> >>>>> On Thu, May 18, 2023 at 11:12:26AM +0800, Huacai Chen wrote:
> >>>>>> On Thu, May 18, 2023 at 11:06 AM Leo Yan <[email protected]> wrote:
> >>>>>>>
> >>>>>>> On Thu, May 18, 2023 at 10:11:27AM +0800, Huacai Chen wrote:
> >>>>>>>> Queued, thanks.
> >>>>>>>
> >>>>>>> The patch is fine for me.
> >>>>>>>
> >>>>>>> Should not perf patches are to be merged via Arnaldo's tree?
> >>>>>>
> >>>>>> I think both are OK, if Arnaldo takes this patch, I will drop it.
> >>>>>
> >>>>> A good practice is to firstly inquiry the maintainers.
> >>>>>
> >>>>> AFAIK, Arnaldo will test perf patches before sending out pull request;
> >>>>> if perf patches are scattered out, it might be out of the testing
> >>>>> radar.
> >>>> OK, I know, thank you very much.
> >>>
> >>> You are welcome!
> >>>
> >>> I found the code base for bfd:
> >>> https://github.com/bminor/binutils-gdb/blob/master/bfd/elfnn-loongarch.c
> >>>
> >>> And this patch is consistent with above link, FWIW:
> >>>
> >>> Reviewed-by: Leo Yan <[email protected]>
> >>
> >> Thanks, applied.
> > I'm very sorry that this patch breaks cross-build. We need some
> > additional modification.
> >
> > diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> > index 5d409c26a22e..b3dbf6ca99a7 100644
> > --- a/tools/perf/util/symbol-elf.c
> > +++ b/tools/perf/util/symbol-elf.c
> > @@ -35,6 +35,10 @@
> > #define EM_AARCH64 183 /* ARM 64 bit */
> > #endif
> >
> > +#ifndef EM_LOONGARCH
> > +#define EM_LOONGARCH 258
> > +#endif
> > +
> > #ifndef ELF32_ST_VISIBILITY
> > #define ELF32_ST_VISIBILITY(o) ((o) & 0x03)
> > #endif
> >
> > Then, drop this patch to get an updated version, or let me send an
> > incremental patch?
> >
>
> I tested this patch on native LoongArch and x86 system, I did not
> hit the build error about undeclared EM_LOONGARCH both on LoongArch
> and x86, because EM_LOONGARCH is defined in /usr/include/elf.h
>
> Here is the x86 system info:
>
> [root@fedora yangtiezhu]# cat /etc/fedora-release
> Fedora release 38 (Thirty Eight)
> [root@fedora yangtiezhu]# uname -m
> x86_64
> [root@fedora yangtiezhu]# grep -rn -w EM_LOONGARCH /usr/include/
> /usr/include/linux/elf-em.h:54:#define EM_LOONGARCH 258 /*
> LoongArch */
> /usr/include/linux/audit.h:442:#define AUDIT_ARCH_LOONGARCH32
> (EM_LOONGARCH|__AUDIT_ARCH_LE)
> /usr/include/linux/audit.h:443:#define AUDIT_ARCH_LOONGARCH64
> (EM_LOONGARCH|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
> /usr/include/elf.h:361:#define EM_LOONGARCH 258 /* LoongArch */
> [root@fedora yangtiezhu]# rpm -qf /usr/include/elf.h
> glibc-headers-x86-2.37-1.fc38.noarch
>
> If I am missing something, please let me know.
Fedora 38 has a very new toolchain, older distribution will meet build errors.

Huacai
>
> Anyway, it is not a bad thing to add the EM_LOONGARCH definition
> to avoid the build error on some systems which have no EM_LOONGARCH
> in the glibc header.
>
> Thanks,
> Tiezhu
>
>

2023-05-23 07:43:00

by Leo Yan

[permalink] [raw]
Subject: Re: [PATCH] perf symbol: Add LoongArch case in get_plt_sizes()

On Tue, May 23, 2023 at 09:47:19AM +0800, Huacai Chen wrote:

[...]

> > > I'm very sorry that this patch breaks cross-build. We need some
> > > additional modification.
> > >
> > > diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> > > index 5d409c26a22e..b3dbf6ca99a7 100644
> > > --- a/tools/perf/util/symbol-elf.c
> > > +++ b/tools/perf/util/symbol-elf.c
> > > @@ -35,6 +35,10 @@
> > > #define EM_AARCH64 183 /* ARM 64 bit */
> > > #endif
> > >
> > > +#ifndef EM_LOONGARCH
> > > +#define EM_LOONGARCH 258
> > > +#endif
> > > +

I confirmed that we must apply this change, otherwise, it will
introduce perf building failure on Ubuntu 22.04 (jammy) with my Arm64
machine.

Sorry I didn't detect the building failure when reviewed this patch!

Thanks,
Leo

2023-05-23 09:12:57

by Tiezhu Yang

[permalink] [raw]
Subject: Re: [PATCH] perf symbol: Add LoongArch case in get_plt_sizes()



On 05/23/2023 03:30 PM, Leo Yan wrote:
> On Tue, May 23, 2023 at 09:47:19AM +0800, Huacai Chen wrote:
>
> [...]
>
>>>> I'm very sorry that this patch breaks cross-build. We need some
>>>> additional modification.
>>>>
>>>> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
>>>> index 5d409c26a22e..b3dbf6ca99a7 100644
>>>> --- a/tools/perf/util/symbol-elf.c
>>>> +++ b/tools/perf/util/symbol-elf.c
>>>> @@ -35,6 +35,10 @@
>>>> #define EM_AARCH64 183 /* ARM 64 bit */
>>>> #endif
>>>>
>>>> +#ifndef EM_LOONGARCH
>>>> +#define EM_LOONGARCH 258
>>>> +#endif
>>>> +
>
> I confirmed that we must apply this change, otherwise, it will
> introduce perf building failure on Ubuntu 22.04 (jammy) with my Arm64
> machine.
>
> Sorry I didn't detect the building failure when reviewed this patch!
>

Sorry for that, EM_LOONGARCH was added in binutils in 2020 [1],
and then it was added in glibc in 2022 [2], so it may be not
exist on some systems.

For now, I do not find the related commit of this patch in any
branch of acme/linux.git, so it is better to send v2 to avoid
the build error, I will do it as soon as possible.

[1]
https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=4cf2ad720078
[2] https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=2d83247d90c9

Thanks,
Tiezhu