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