[LTP] [PATCH v2 1/1] ptrace07: should not use a hard-coded xstate size and use CPUID specified instead

[Pengfei Xu]

Hi Richard,

On 2022-10-18 at 09:11:47 +0100, Richard Palethorpe wrote:
> Hello,
> 
> Pengfei Xu <pengfei.xu@intel.com> writes:
> 
> > Hi Richard,
> >
> > On 2022-10-17 at 14:55:29 +0100, Richard Palethorpe wrote:
> >> Hello,
> >> 
> >> Pengfei Xu <pengfei.xu@intel.com> writes:
> >> 
> >> > Hi,
> >> >
> >> > This patch fixes ptrace07 spurious failures when the platform xstate maxium
> >> > size is bigger than 4096bytes(512*8 bytes).
> >> >
> >> > Thanks for comments!
> >> 
> >> This patch causes the test to fail on my Xeon workstation. The problem
> >> seems to be the cpuid function which just fills the args with zeros.
> >   Sorry, I didn't meet this issue,  I think I should use a new cpuid function.
> >   Thanks for the report!
> >
> >> 
> >> >
> >> > BR.
> >> >
> >> > On 2022-09-29 at 10:30:20 +0800, Pengfei Xu wrote:
> >> >> Should not use a hard-coded xstate size(512 * 8 = 4096 bytes) which is
> >> >> wrong, should use maximum XSAVE size specified by CPUID.(EAX=0DH, ECX=0H):EBX.
> >> >> If the CPU's maximum XSAVE size exceeds the hard-coded xstate size 4096 bytes,
> >> >> it will cause the ptrace07 case to fail as below:
> >> >> "
> >> >> ./ptrace07
> >> >> tst_test.c:1528: TINFO: Timeout per run is 0h 00m 30s
> >> >> ptrace07.c:142: TBROK: PTRACE_SETREGSET failed with unexpected error: EFAULT (14)
> >> >> tst_test.c:1571: TINFO: Killed the leftover descendant processes
> >> >> 
> >> >> Summary:
> >> >> passed   0
> >> >> failed   0
> >> >> broken   1
> >> >> skipped  0
> >> >> warnings 0
> >> >> "
> >> >> 
> >> >> Reported-by: Eric DeVolder <eric.devolder@oracle.com>
> >> >> Reviewed-by: Chang S. Bae <chang.seok.bae@intel.com>
> >> >> Signed-off-by: Pengfei Xu <pengfei.xu@intel.com>
> >> >> ---
> >> >>  testcases/kernel/syscalls/ptrace/ptrace07.c | 25 +++++++++++++++++----
> >> >>  1 file changed, 21 insertions(+), 4 deletions(-)
> >> >> 
> >> >> diff --git a/testcases/kernel/syscalls/ptrace/ptrace07.c b/testcases/kernel/syscalls/ptrace/ptrace07.c
> >> >> index da62cadb0..0accaceb5 100644
> >> >> --- a/testcases/kernel/syscalls/ptrace/ptrace07.c
> >> >> +++ b/testcases/kernel/syscalls/ptrace/ptrace07.c
> >> >> @@ -35,6 +35,7 @@
> >> >>  #include "config.h"
> >> >>  #include "ptrace.h"
> >> >>  #include "tst_test.h"
> >> >> +#include "ltp_cpuid.h"
> >> 
> >> This is from the old API (starts with ltp_) so we shouldn't use it
> >> anymore. If it is being used at all, then it's being used in a way that
> >> would allow it to silently fail AFAICT.
> >> 
> >   Thanks for the comments, I plan to add below __cpuid_count() macro function
> > as below in ltp/include/tst_cpu.h first, there seems to be some other place to
> > use the cpuid function.
> >
> > /*
> >  * gcc cpuid.h provides __cpuid_count() since v4.4.
> >  * Clang/LLVM cpuid.h provides  __cpuid_count() since v3.4.0.
> >  *
> >  * Provide local define for tests needing __cpuid_count() because
> >  * ltp needs to work in older environments that do not yet
> >  * have __cpuid_count().
> >  */
> > #ifndef __cpuid_count
> > #define __cpuid_count(level, count, a, b, c, d)				\
> > 	({								\
> > 	__asm__ __volatile__ ("cpuid\n\t"				\
> > 			      : "=a" (a), "=b" (b), "=c" (c), "=d" (d)	\
> > 			      : "0" (level), "2" (count));		\
> > 	})
> > #endif
> 
> Looks good. Although this should go in ltp/include/lapi/cpuid.h as this
> is where we put system header fallbacks.
> 
  Thanks for suggestion! Yes, it's better.
  I will write it in ltp/include/lapi/cpuid.h instead.

  Thanks!
  BR.

> -- 
> Thank you,
> Richard.