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

[Pengfei Xu <pengfei.xu@intel.com>]

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


> >>  
> >>  #ifndef PTRACE_GETREGSET
> >>  # define PTRACE_GETREGSET 0x4204
> >> @@ -48,6 +49,8 @@
> >>  # define NT_X86_XSTATE 0x202
> >>  #endif
> >>  
> >> +#define CPUID_LEAF_XSTATE 0xd
> >> +
> >>  static void check_regs_loop(uint32_t initval)
> >>  {
> >>  	const unsigned long num_iters = 1000000000;
> >> @@ -83,8 +86,15 @@ static void do_test(void)
> >>  	int i;
> >>  	int num_cpus = tst_ncpus();
> >>  	pid_t pid;
> >> -	uint64_t xstate[512];
> >> -	struct iovec iov = { .iov_base = xstate, .iov_len = sizeof(xstate) };
> >> +	uint32_t eax, ebx, ecx = 0, edx;
> >> +	uint64_t *xstate;
> >> +	/*
> >> +	 * CPUID.(EAX=0DH, ECX=0H):EBX: maximum size (bytes, from the beginning
> >> +	 * of the XSAVE/XRSTOR save area) required by enabled features in XCR0.
> >> +	 */
> >> +	cpuid(CPUID_LEAF_XSTATE, &eax, &ebx, &ecx, &edx);
> >> +	xstate = aligned_alloc(64, ebx);
> >> +	struct iovec iov = { .iov_base = xstate, .iov_len = ebx };
> >>  	int status;
> >>  	bool okay;
> 
> Adding:
> 
> tst_res(TINFO, "EAX=%u, ECX=%u, EBX=%u", eax, ecx, ebx);
> 
  Thanks, I will add it.

Thanks!
BR.

> prints:
> 
> ptrace07.c:101: TINFO: EAX=0, ECX=0, EBX=0
> 
> 
> -- 
> Thank you,
> Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp