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

[Richard Palethorpe <rpalethorpe@suse.de>]

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.

>
> 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.

>>  
>>  #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);

prints:

ptrace07.c:101: TINFO: EAX=0, ECX=0, EBX=0


-- 
Thank you,
Richard.

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