* [LTP] [PATCH v2 1/1] ptrace07: should not use a hard-coded xstate size and use CPUID specified instead [not found] <cover.1664418361.git.pengfei.xu@intel.com> @ 2022-09-29 2:30 ` Pengfei Xu 2022-10-14 6:38 ` Pengfei Xu 0 siblings, 1 reply; 6+ messages in thread From: Pengfei Xu @ 2022-09-29 2:30 UTC (permalink / raw) To: ltp; +Cc: chang.seok.bae, eric.devolder, Heng Su 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" #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; @@ -102,12 +112,15 @@ static void do_test(void) sched_yield(); TEST(ptrace(PTRACE_ATTACH, pid, 0, 0)); - if (TST_RET != 0) + if (TST_RET != 0) { + free(xstate); tst_brk(TBROK | TTERRNO, "PTRACE_ATTACH failed"); + } SAFE_WAITPID(pid, NULL, 0); TEST(ptrace(PTRACE_GETREGSET, pid, NT_X86_XSTATE, &iov)); if (TST_RET != 0) { + free(xstate); if (TST_ERR == EIO) tst_brk(TCONF, "GETREGSET/SETREGSET is unsupported"); @@ -138,6 +151,7 @@ static void do_test(void) tst_res(TINFO, "PTRACE_SETREGSET with reserved bits failed with EINVAL"); } else { + free(xstate); tst_brk(TBROK | TTERRNO, "PTRACE_SETREGSET failed with unexpected error"); } @@ -152,8 +166,10 @@ static void do_test(void) * worry about potential stops after this point. */ TEST(ptrace(PTRACE_DETACH, pid, 0, 0)); - if (TST_RET != 0) + if (TST_RET != 0) { + free(xstate); tst_brk(TBROK | TTERRNO, "PTRACE_DETACH failed"); + } /* If child 'pid' crashes, only report it as info. */ SAFE_WAITPID(pid, &status, 0); @@ -173,6 +189,7 @@ static void do_test(void) } if (okay) tst_res(TPASS, "wasn't able to set invalid FPU state"); + free(xstate); } static struct tst_test test = { -- 2.31.1 -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [LTP] [PATCH v2 1/1] ptrace07: should not use a hard-coded xstate size and use CPUID specified instead 2022-09-29 2:30 ` [LTP] [PATCH v2 1/1] ptrace07: should not use a hard-coded xstate size and use CPUID specified instead Pengfei Xu @ 2022-10-14 6:38 ` Pengfei Xu 2022-10-17 13:55 ` Richard Palethorpe 0 siblings, 1 reply; 6+ messages in thread From: Pengfei Xu @ 2022-10-14 6:38 UTC (permalink / raw) To: ltp; +Cc: Heng Su, eric.devolder, chang.seok.bae Hi, This patch fixes ptrace07 spurious failures when the platform xstate maxium size is bigger than 4096bytes(512*8 bytes). Thanks for comments! 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" > > #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; > > @@ -102,12 +112,15 @@ static void do_test(void) > sched_yield(); > > TEST(ptrace(PTRACE_ATTACH, pid, 0, 0)); > - if (TST_RET != 0) > + if (TST_RET != 0) { > + free(xstate); > tst_brk(TBROK | TTERRNO, "PTRACE_ATTACH failed"); > + } > > SAFE_WAITPID(pid, NULL, 0); > TEST(ptrace(PTRACE_GETREGSET, pid, NT_X86_XSTATE, &iov)); > if (TST_RET != 0) { > + free(xstate); > if (TST_ERR == EIO) > tst_brk(TCONF, "GETREGSET/SETREGSET is unsupported"); > > @@ -138,6 +151,7 @@ static void do_test(void) > tst_res(TINFO, > "PTRACE_SETREGSET with reserved bits failed with EINVAL"); > } else { > + free(xstate); > tst_brk(TBROK | TTERRNO, > "PTRACE_SETREGSET failed with unexpected error"); > } > @@ -152,8 +166,10 @@ static void do_test(void) > * worry about potential stops after this point. > */ > TEST(ptrace(PTRACE_DETACH, pid, 0, 0)); > - if (TST_RET != 0) > + if (TST_RET != 0) { > + free(xstate); > tst_brk(TBROK | TTERRNO, "PTRACE_DETACH failed"); > + } > > /* If child 'pid' crashes, only report it as info. */ > SAFE_WAITPID(pid, &status, 0); > @@ -173,6 +189,7 @@ static void do_test(void) > } > if (okay) > tst_res(TPASS, "wasn't able to set invalid FPU state"); > + free(xstate); > } > > static struct tst_test test = { > -- > 2.31.1 > -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [LTP] [PATCH v2 1/1] ptrace07: should not use a hard-coded xstate size and use CPUID specified instead 2022-10-14 6:38 ` Pengfei Xu @ 2022-10-17 13:55 ` Richard Palethorpe 2022-10-18 7:28 ` Pengfei Xu 0 siblings, 1 reply; 6+ messages in thread From: Richard Palethorpe @ 2022-10-17 13:55 UTC (permalink / raw) To: Pengfei Xu; +Cc: Heng Su, eric.devolder, chang.seok.bae, ltp 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [LTP] [PATCH v2 1/1] ptrace07: should not use a hard-coded xstate size and use CPUID specified instead 2022-10-17 13:55 ` Richard Palethorpe @ 2022-10-18 7:28 ` Pengfei Xu 2022-10-18 8:11 ` Richard Palethorpe 0 siblings, 1 reply; 6+ messages in thread From: Pengfei Xu @ 2022-10-18 7:28 UTC (permalink / raw) To: Richard Palethorpe; +Cc: Heng Su, eric.devolder, chang.seok.bae, ltp 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [LTP] [PATCH v2 1/1] ptrace07: should not use a hard-coded xstate size and use CPUID specified instead 2022-10-18 7:28 ` Pengfei Xu @ 2022-10-18 8:11 ` Richard Palethorpe 2022-10-18 8:49 ` Pengfei Xu 0 siblings, 1 reply; 6+ messages in thread From: Richard Palethorpe @ 2022-10-18 8:11 UTC (permalink / raw) To: Pengfei Xu; +Cc: Heng Su, eric.devolder, chang.seok.bae, ltp 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. -- Thank you, Richard. -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [LTP] [PATCH v2 1/1] ptrace07: should not use a hard-coded xstate size and use CPUID specified instead 2022-10-18 8:11 ` Richard Palethorpe @ 2022-10-18 8:49 ` Pengfei Xu 0 siblings, 0 replies; 6+ messages in thread From: Pengfei Xu @ 2022-10-18 8:49 UTC (permalink / raw) To: Richard Palethorpe; +Cc: Heng Su, eric.devolder, chang.seok.bae, ltp 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. -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-10-18 8:50 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <cover.1664418361.git.pengfei.xu@intel.com>
2022-09-29 2:30 ` [LTP] [PATCH v2 1/1] ptrace07: should not use a hard-coded xstate size and use CPUID specified instead Pengfei Xu
2022-10-14 6:38 ` Pengfei Xu
2022-10-17 13:55 ` Richard Palethorpe
2022-10-18 7:28 ` Pengfei Xu
2022-10-18 8:11 ` Richard Palethorpe
2022-10-18 8:49 ` Pengfei Xu
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.