| Message ID | 569D2D7B.30101@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
Ping? On 1/18/16 4:22 PM, Carlos Eduardo Seo wrote: > > Hi > > This is a bug fix. There are 4 unused bits in HWCAP and space must be > reserved for them in _dl_powerpc_cap_flags so the code iterates through > all the 32 capabilities, otherwise you may get a wrong AT_HWCAP > displayed when LD_SHOW_AUXV=1. > > If possible, I'd like this in 2.23. > > Regards, >
On 18-01-2016 16:22, Carlos Eduardo Seo wrote: > Hi > > This is a bug fix. There are 4 unused bits in HWCAP and space must be reserved for them in _dl_powerpc_cap_flags so the code iterates through all the 32 capabilities, otherwise you may get a wrong AT_HWCAP displayed when LD_SHOW_AUXV=1. > > If possible, I'd like this in 2.23. I think you should not it might be wrong for hwcap2, since the logic is correct for hwcap1 (I assume this hitting the new fields for POWER9). LGTM. > > Regards, > > -- > Carlos Eduardo Seo > Software Engineer - Linux on Power Toolchain > cseo@linux.vnet.ibm.com > > 0001-powerpc-Fix-dl-procinfo-HWCAP.patch > > > From c90f857f571e222ac60478e3ed26f978042947b8 Mon Sep 17 00:00:00 2001 > From: Carlos Eduardo Seo <cseo@linux.vnet.ibm.com> > Date: Mon, 28 Dec 2015 16:36:46 -0200 > Subject: [PATCH] powerpc: Fix dl-procinfo HWCAP. > > HWCAP-related code should had been updated when the 32 bits of HWCAP were > used. This patch updates the code in dl-procinfo.h to loop through all the 32 > bits in HWCAP and updates _dl_powerpc_cap_flags accordignly. > > 2016-01-18 Carlos Eduardo Seo <cseo@linux.vnet.ibm.com> > > * sysdeps/powerpc/dl-procinfo.c: > (_dl_powerpc_cap_flags): Updated to reflect the entire 32-bit HWCAP. > * sysdeps/powerpc/dl-procinfo.h: Code cleanup. > (_DL_HWCAP_FIRST): Removed. Replaced by 0 accordignly. > --- > sysdeps/powerpc/dl-procinfo.c | 5 +++-- > sysdeps/powerpc/dl-procinfo.h | 9 +++------ > 2 files changed, 6 insertions(+), 8 deletions(-) > > diff --git a/sysdeps/powerpc/dl-procinfo.c b/sysdeps/powerpc/dl-procinfo.c > index a8df5b8..5604366 100644 > --- a/sysdeps/powerpc/dl-procinfo.c > +++ b/sysdeps/powerpc/dl-procinfo.c > @@ -45,11 +45,12 @@ > #if !defined PROCINFO_DECL && defined SHARED > ._dl_powerpc_cap_flags > #else > -PROCINFO_CLASS const char _dl_powerpc_cap_flags[60][10] > +PROCINFO_CLASS const char _dl_powerpc_cap_flags[64][10] > #endif > #ifndef PROCINFO_DECL > = { > - "ppcle", "true_le", "archpmu", "vsx", > + "ppcle", "true_le", "", "", > + "", "", "archpmu", "vsx", > "arch_2_06", "power6x", "dfp", "pa6t", > "arch_2_05", "ic_snoop", "smt", "booke", > "cellbe", "power5+", "power5", "power4", > diff --git a/sysdeps/powerpc/dl-procinfo.h b/sysdeps/powerpc/dl-procinfo.h > index 407149b..80e70e5 100644 > --- a/sysdeps/powerpc/dl-procinfo.h > +++ b/sysdeps/powerpc/dl-procinfo.h > @@ -22,9 +22,6 @@ > #include <ldsodefs.h> > #include <sysdep.h> /* This defines the PPC_FEATURE[2]_* macros. */ > > -/* There are 28 bits used, but they are bits 4..31. */ > -#define _DL_HWCAP_FIRST 4 > - > /* The total number of available bits (including those prior to > _DL_HWCAP_FIRST). Some of these bits might not be used. */ > #define _DL_HWCAP_COUNT 64 > @@ -68,7 +65,7 @@ static inline const char * > __attribute__ ((unused)) > _dl_hwcap_string (int idx) > { > - return GLRO(dl_powerpc_cap_flags)[idx - _DL_HWCAP_FIRST]; > + return GLRO(dl_powerpc_cap_flags)[idx]; > } > > static inline const char * > @@ -82,7 +79,7 @@ static inline int > __attribute__ ((unused)) > _dl_string_hwcap (const char *str) > { > - for (int i = _DL_HWCAP_FIRST; i < _DL_HWCAP_COUNT; ++i) > + for (int i = 0; i < _DL_HWCAP_COUNT; ++i) > if (strcmp (str, _dl_hwcap_string (i)) == 0) > return i; > return -1; > @@ -180,7 +177,7 @@ _dl_procinfo (unsigned int type, unsigned long int word) > case AT_HWCAP: > _dl_printf ("AT_HWCAP: "); > > - for (int i = _DL_HWCAP_FIRST; i <= _DL_HWCAP_LAST; ++i) > + for (int i = 0; i <= _DL_HWCAP_LAST; ++i) > if (word & (1 << i)) > _dl_printf (" %s", _dl_hwcap_string (i)); > break; > -- 2.5.4 (Apple Git-61)
On 1/25/16 4:04 PM, Adhemerval Zanella wrote: > > I think you should not it might be wrong for hwcap2, since the logic > is correct for hwcap1 (I assume this hitting the new fields for > POWER9). > No, this actually hits HWCAP1. If you look at hwcap.h, you will see: #define PPC_FEATURE_PSERIES_PERFMON_COMPAT 0x00000040 #define PPC_FEATURE_TRUE_LE 0x00000002 There's a 4 bit gap between PPC_FEATURE_PSERIES_PERFMON_COMPAT and PPC_FEATURE_TRUE_LE that has to be padded in _dl_powerpc_cap_flags. The patch also assumes HWCAP1 is 64-bit long and all bits are filled, which is true now (if you pad those vacant 4 bits).
Ping Is this OK for 2.23? Thanks On 1/25/16 4:15 PM, Carlos Eduardo Seo wrote: > > > On 1/25/16 4:04 PM, Adhemerval Zanella wrote: >> >> I think you should not it might be wrong for hwcap2, since the logic >> is correct for hwcap1 (I assume this hitting the new fields for >> POWER9). >> > > No, this actually hits HWCAP1. If you look at hwcap.h, you will see: > > #define PPC_FEATURE_PSERIES_PERFMON_COMPAT 0x00000040 > #define PPC_FEATURE_TRUE_LE 0x00000002 > > There's a 4 bit gap between PPC_FEATURE_PSERIES_PERFMON_COMPAT and > PPC_FEATURE_TRUE_LE that has to be padded in _dl_powerpc_cap_flags. > > The patch also assumes HWCAP1 is 64-bit long and all bits are filled, > which is true now (if you pad those vacant 4 bits). >
LGTM but it does not characterize a critical bug or CVE to lift the freeze. Please apply when 2.24 open and we can backport to 2.23 after release. On 03-02-2016 12:02, Carlos Eduardo Seo wrote: > Ping > > Is this OK for 2.23? > > Thanks > > On 1/25/16 4:15 PM, Carlos Eduardo Seo wrote: >> >> >> On 1/25/16 4:04 PM, Adhemerval Zanella wrote: >>> >>> I think you should not it might be wrong for hwcap2, since the logic >>> is correct for hwcap1 (I assume this hitting the new fields for >>> POWER9). >>> >> >> No, this actually hits HWCAP1. If you look at hwcap.h, you will see: >> >> #define PPC_FEATURE_PSERIES_PERFMON_COMPAT 0x00000040 >> #define PPC_FEATURE_TRUE_LE 0x00000002 >> >> There's a 4 bit gap between PPC_FEATURE_PSERIES_PERFMON_COMPAT and >> PPC_FEATURE_TRUE_LE that has to be padded in _dl_powerpc_cap_flags. >> >> The patch also assumes HWCAP1 is 64-bit long and all bits are filled, >> which is true now (if you pad those vacant 4 bits). >> >
On 2/3/16 2:51 PM, Adhemerval Zanella wrote: > LGTM but it does not characterize a critical bug or CVE to lift the > freeze. Please apply when 2.24 open and we can backport to 2.23 > after release. > Sounds good. Thanks
Carlos Eduardo Seo <cseo@linux.vnet.ibm.com> writes: > Subject: [PATCH] powerpc: Fix dl-procinfo HWCAP. > > HWCAP-related code should had been updated when the 32 bits of HWCAP were > used. This patch updates the code in dl-procinfo.h to loop through all the 32 > bits in HWCAP and updates _dl_powerpc_cap_flags accordignly. Typo here -----------------------------------------------^ Fixed. > > 2016-01-18 Carlos Eduardo Seo <cseo@linux.vnet.ibm.com> > > * sysdeps/powerpc/dl-procinfo.c: > (_dl_powerpc_cap_flags): Updated to reflect the entire 32-bit HWCAP. > * sysdeps/powerpc/dl-procinfo.h: Code cleanup. > (_DL_HWCAP_FIRST): Removed. Replaced by 0 accordignly. Likewise + 2 spaces here ----------^ Fixed too. Pushed as 911569d. Thanks!
diff --git a/sysdeps/powerpc/dl-procinfo.c b/sysdeps/powerpc/dl-procinfo.c index a8df5b8..5604366 100644 --- a/sysdeps/powerpc/dl-procinfo.c +++ b/sysdeps/powerpc/dl-procinfo.c @@ -45,11 +45,12 @@ #if !defined PROCINFO_DECL && defined SHARED ._dl_powerpc_cap_flags #else -PROCINFO_CLASS const char _dl_powerpc_cap_flags[60][10] +PROCINFO_CLASS const char _dl_powerpc_cap_flags[64][10] #endif #ifndef PROCINFO_DECL = { - "ppcle", "true_le", "archpmu", "vsx", + "ppcle", "true_le", "", "", + "", "", "archpmu", "vsx", "arch_2_06", "power6x", "dfp", "pa6t", "arch_2_05", "ic_snoop", "smt", "booke", "cellbe", "power5+", "power5", "power4", diff --git a/sysdeps/powerpc/dl-procinfo.h b/sysdeps/powerpc/dl-procinfo.h index 407149b..80e70e5 100644 --- a/sysdeps/powerpc/dl-procinfo.h +++ b/sysdeps/powerpc/dl-procinfo.h @@ -22,9 +22,6 @@ #include <ldsodefs.h> #include <sysdep.h> /* This defines the PPC_FEATURE[2]_* macros. */ -/* There are 28 bits used, but they are bits 4..31. */ -#define _DL_HWCAP_FIRST 4 - /* The total number of available bits (including those prior to _DL_HWCAP_FIRST). Some of these bits might not be used. */ #define _DL_HWCAP_COUNT 64 @@ -68,7 +65,7 @@ static inline const char * __attribute__ ((unused)) _dl_hwcap_string (int idx) { - return GLRO(dl_powerpc_cap_flags)[idx - _DL_HWCAP_FIRST]; + return GLRO(dl_powerpc_cap_flags)[idx]; } static inline const char * @@ -82,7 +79,7 @@ static inline int __attribute__ ((unused)) _dl_string_hwcap (const char *str) { - for (int i = _DL_HWCAP_FIRST; i < _DL_HWCAP_COUNT; ++i) + for (int i = 0; i < _DL_HWCAP_COUNT; ++i) if (strcmp (str, _dl_hwcap_string (i)) == 0) return i; return -1; @@ -180,7 +177,7 @@ _dl_procinfo (unsigned int type, unsigned long int word) case AT_HWCAP: _dl_printf ("AT_HWCAP: "); - for (int i = _DL_HWCAP_FIRST; i <= _DL_HWCAP_LAST; ++i) + for (int i = 0; i <= _DL_HWCAP_LAST; ++i) if (word & (1 << i)) _dl_printf (" %s", _dl_hwcap_string (i)); break;