Messages in this thread | | | From | Huacai Chen <> | Date | Tue, 23 May 2023 09:47:19 +0800 | Subject | Re: [PATCH] perf symbol: Add LoongArch case in get_plt_sizes() |
| |
On Mon, May 22, 2023 at 3:59 PM Tiezhu Yang <yangtiezhu@loongson.cn> 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 > > <acme@kernel.org> 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 <leo.yan@linaro.org> 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 <leo.yan@linaro.org> 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 <leo.yan@linaro.org> > >> > >> 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 > >
| |