| Message ID | 20220810104114.6838-2-gshan@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | kvm/selftests: Two rseq_test fixes | expand |
----- On Aug 10, 2022, at 6:41 AM, Gavin Shan gshan@redhat.com wrote: > The rseq information is registered by TLS, starting from glibc-2.35. > In this case, the test always fails due to syscall(__NR_rseq). For > example, on RHEL9.1 where upstream glibc-2.35 features are enabled > on downstream glibc-2.34, the test fails like below. > > # ./rseq_test > ==== Test Assertion Failure ==== > rseq_test.c:60: !r > pid=112043 tid=112043 errno=22 - Invalid argument > 1 0x0000000000401973: main at rseq_test.c:226 > 2 0x0000ffff84b6c79b: ?? ??:0 > 3 0x0000ffff84b6c86b: ?? ??:0 > 4 0x0000000000401b6f: _start at ??:? > rseq failed, errno = 22 (Invalid argument) > # rpm -aq | grep glibc-2 > glibc-2.34-39.el9.aarch64 > > Fix the issue by using "../rseq/rseq.c" to fetch the rseq information, > registred by TLS if it exists. Otherwise, we're going to register our > own rseq information as before. > > Reported-by: Yihuang Yu <yihyu@redhat.com> > Suggested-by: Florian Weimer <fweimer@redhat.com> > Suggested-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> > Suggested-by: Paolo Bonzini <pbonzini@redhat.com> > Signed-off-by: Gavin Shan <gshan@redhat.com> > --- > tools/testing/selftests/kvm/Makefile | 5 +++-- > tools/testing/selftests/kvm/rseq_test.c | 28 +++++++------------------ > 2 files changed, 11 insertions(+), 22 deletions(-) > > diff --git a/tools/testing/selftests/kvm/Makefile > b/tools/testing/selftests/kvm/Makefile > index c7f47429d6cd..89c9a8c52c5f 100644 > --- a/tools/testing/selftests/kvm/Makefile > +++ b/tools/testing/selftests/kvm/Makefile > @@ -197,7 +197,8 @@ endif > CFLAGS += -Wall -Wstrict-prototypes -Wuninitialized -O2 -g -std=gnu99 \ > -fno-stack-protector -fno-PIE -I$(LINUX_TOOL_INCLUDE) \ > -I$(LINUX_TOOL_ARCH_INCLUDE) -I$(LINUX_HDR_PATH) -Iinclude \ > - -I$(<D) -Iinclude/$(UNAME_M) -I.. $(EXTRA_CFLAGS) $(KHDR_INCLUDES) > + -I$(<D) -Iinclude/$(UNAME_M) -I ../rseq -I.. $(EXTRA_CFLAGS) \ > + $(KHDR_INCLUDES) > > no-pie-option := $(call try-run, echo 'int main() { return 0; }' | \ > $(CC) -Werror -no-pie -x c - -o "$$TMP", -no-pie) > @@ -206,7 +207,7 @@ no-pie-option := $(call try-run, echo 'int main() { return > 0; }' | \ > pgste-option = $(call try-run, echo 'int main() { return 0; }' | \ > $(CC) -Werror -Wl$(comma)--s390-pgste -x c - -o "$$TMP",-Wl$(comma)--s390-pgste) > > - > +LDLIBS += -ldl > LDFLAGS += -pthread $(no-pie-option) $(pgste-option) > > # After inclusion, $(OUTPUT) is defined and > diff --git a/tools/testing/selftests/kvm/rseq_test.c > b/tools/testing/selftests/kvm/rseq_test.c > index a54d4d05a058..2cd5fe49ac8b 100644 > --- a/tools/testing/selftests/kvm/rseq_test.c > +++ b/tools/testing/selftests/kvm/rseq_test.c > @@ -20,15 +20,7 @@ > #include "processor.h" > #include "test_util.h" > > -static __thread volatile struct rseq __rseq = { > - .cpu_id = RSEQ_CPU_ID_UNINITIALIZED, > -}; > - > -/* > - * Use an arbitrary, bogus signature for configuring rseq, this test does not > - * actually enter an rseq critical section. > - */ > -#define RSEQ_SIG 0xdeadbeef > +#include "../rseq/rseq.c" > > /* > * Any bug related to task migration is likely to be timing-dependent; perform > @@ -37,6 +29,7 @@ static __thread volatile struct rseq __rseq = { > #define NR_TASK_MIGRATIONS 100000 > > static pthread_t migration_thread; > +static struct rseq_abi *__rseq; What is this ? > static cpu_set_t possible_mask; > static int min_cpu, max_cpu; > static bool done; > @@ -49,14 +42,6 @@ static void guest_code(void) > GUEST_SYNC(0); > } > > -static void sys_rseq(int flags) > -{ > - int r; > - > - r = syscall(__NR_rseq, &__rseq, sizeof(__rseq), flags, RSEQ_SIG); > - TEST_ASSERT(!r, "rseq failed, errno = %d (%s)", errno, strerror(errno)); > -} > - > static int next_cpu(int cpu) > { > /* > @@ -218,7 +203,10 @@ int main(int argc, char *argv[]) > > calc_min_max_cpu(); > > - sys_rseq(0); > + r = rseq_register_current_thread(); > + TEST_ASSERT(!r, "rseq_register_current_thread failed, errno = %d (%s)", > + errno, strerror(errno)); > + __rseq = rseq_get_abi(); > > /* > * Create and run a dummy VM that immediately exits to userspace via > @@ -256,7 +244,7 @@ int main(int argc, char *argv[]) > */ > smp_rmb(); > cpu = sched_getcpu(); > - rseq_cpu = READ_ONCE(__rseq.cpu_id); > + rseq_cpu = READ_ONCE(__rseq->cpu_id); #include <rseq.h> and use rseq_current_cpu_raw(). Thanks, Mathieu > smp_rmb(); > } while (snapshot != atomic_read(&seq_cnt)); > > @@ -278,7 +266,7 @@ int main(int argc, char *argv[]) > > kvm_vm_free(vm); > > - sys_rseq(RSEQ_FLAG_UNREGISTER); > + rseq_unregister_current_thread(); > > return 0; > } > -- > 2.23.0
On 8/10/22 14:22, Mathieu Desnoyers wrote: >> >> /* >> * Create and run a dummy VM that immediately exits to userspace via >> @@ -256,7 +244,7 @@ int main(int argc, char *argv[]) >> */ >> smp_rmb(); >> cpu = sched_getcpu(); >> - rseq_cpu = READ_ONCE(__rseq.cpu_id); >> + rseq_cpu = READ_ONCE(__rseq->cpu_id); > #include <rseq.h> > > and use > > rseq_current_cpu_raw(). Thanks, I squashed it and queued it for -rc1 (tested on both glibc 2.34 and 2.35). diff --git a/tools/testing/selftests/kvm/rseq_test.c b/tools/testing/selftests/kvm/rseq_test.c index 84e8425edc2c..987a76674f4f 100644 --- a/tools/testing/selftests/kvm/rseq_test.c +++ b/tools/testing/selftests/kvm/rseq_test.c @@ -29,7 +29,6 @@ #define NR_TASK_MIGRATIONS 100000 static pthread_t migration_thread; -static struct rseq_abi *__rseq; static cpu_set_t possible_mask; static int min_cpu, max_cpu; static bool done; @@ -218,7 +217,6 @@ int main(int argc, char *argv[]) r = rseq_register_current_thread(); TEST_ASSERT(!r, "rseq_register_current_thread failed, errno = %d (%s)", errno, strerror(errno)); - __rseq = rseq_get_abi(); /* * Create and run a dummy VM that immediately exits to userspace via @@ -256,7 +254,7 @@ int main(int argc, char *argv[]) */ smp_rmb(); cpu = sched_getcpu(); - rseq_cpu = READ_ONCE(__rseq->cpu_id); + rseq_cpu = rseq_current_cpu_raw(); smp_rmb(); } while (snapshot != atomic_read(&seq_cnt)); Paolo
----- On Aug 10, 2022, at 8:29 AM, Paolo Bonzini pbonzini@redhat.com wrote: > On 8/10/22 14:22, Mathieu Desnoyers wrote: >>> >>> /* >>> * Create and run a dummy VM that immediately exits to userspace via >>> @@ -256,7 +244,7 @@ int main(int argc, char *argv[]) >>> */ >>> smp_rmb(); >>> cpu = sched_getcpu(); >>> - rseq_cpu = READ_ONCE(__rseq.cpu_id); >>> + rseq_cpu = READ_ONCE(__rseq->cpu_id); >> #include <rseq.h> >> >> and use >> >> rseq_current_cpu_raw(). > > Thanks, I squashed it and queued it for -rc1 (tested on both > glibc 2.34 and 2.35). Thanks a lot Paolo, Cheers! :) Mathieu
On 8/10/22 10:29 PM, Paolo Bonzini wrote: > On 8/10/22 14:22, Mathieu Desnoyers wrote: >>> >>> /* >>> * Create and run a dummy VM that immediately exits to userspace via >>> @@ -256,7 +244,7 @@ int main(int argc, char *argv[]) >>> */ >>> smp_rmb(); >>> cpu = sched_getcpu(); >>> - rseq_cpu = READ_ONCE(__rseq.cpu_id); >>> + rseq_cpu = READ_ONCE(__rseq->cpu_id); >> #include <rseq.h> >> >> and use >> >> rseq_current_cpu_raw(). > > Thanks, I squashed it and queued it for -rc1 (tested on both > glibc 2.34 and 2.35). > Paolo, Thanks for the makeup, which looks good to me :) > diff --git a/tools/testing/selftests/kvm/rseq_test.c b/tools/testing/selftests/kvm/rseq_test.c > index 84e8425edc2c..987a76674f4f 100644 > --- a/tools/testing/selftests/kvm/rseq_test.c > +++ b/tools/testing/selftests/kvm/rseq_test.c > @@ -29,7 +29,6 @@ > #define NR_TASK_MIGRATIONS 100000 > > static pthread_t migration_thread; > -static struct rseq_abi *__rseq; > static cpu_set_t possible_mask; > static int min_cpu, max_cpu; > static bool done; > @@ -218,7 +217,6 @@ int main(int argc, char *argv[]) > r = rseq_register_current_thread(); > TEST_ASSERT(!r, "rseq_register_current_thread failed, errno = %d (%s)", > errno, strerror(errno)); > - __rseq = rseq_get_abi(); > > /* > * Create and run a dummy VM that immediately exits to userspace via > @@ -256,7 +254,7 @@ int main(int argc, char *argv[]) > */ > smp_rmb(); > cpu = sched_getcpu(); > - rseq_cpu = READ_ONCE(__rseq->cpu_id); > + rseq_cpu = rseq_current_cpu_raw(); > smp_rmb(); > } while (snapshot != atomic_read(&seq_cnt)); >
diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile index c7f47429d6cd..89c9a8c52c5f 100644 --- a/tools/testing/selftests/kvm/Makefile +++ b/tools/testing/selftests/kvm/Makefile @@ -197,7 +197,8 @@ endif CFLAGS += -Wall -Wstrict-prototypes -Wuninitialized -O2 -g -std=gnu99 \ -fno-stack-protector -fno-PIE -I$(LINUX_TOOL_INCLUDE) \ -I$(LINUX_TOOL_ARCH_INCLUDE) -I$(LINUX_HDR_PATH) -Iinclude \ - -I$(<D) -Iinclude/$(UNAME_M) -I.. $(EXTRA_CFLAGS) $(KHDR_INCLUDES) + -I$(<D) -Iinclude/$(UNAME_M) -I ../rseq -I.. $(EXTRA_CFLAGS) \ + $(KHDR_INCLUDES) no-pie-option := $(call try-run, echo 'int main() { return 0; }' | \ $(CC) -Werror -no-pie -x c - -o "$$TMP", -no-pie) @@ -206,7 +207,7 @@ no-pie-option := $(call try-run, echo 'int main() { return 0; }' | \ pgste-option = $(call try-run, echo 'int main() { return 0; }' | \ $(CC) -Werror -Wl$(comma)--s390-pgste -x c - -o "$$TMP",-Wl$(comma)--s390-pgste) - +LDLIBS += -ldl LDFLAGS += -pthread $(no-pie-option) $(pgste-option) # After inclusion, $(OUTPUT) is defined and diff --git a/tools/testing/selftests/kvm/rseq_test.c b/tools/testing/selftests/kvm/rseq_test.c index a54d4d05a058..2cd5fe49ac8b 100644 --- a/tools/testing/selftests/kvm/rseq_test.c +++ b/tools/testing/selftests/kvm/rseq_test.c @@ -20,15 +20,7 @@ #include "processor.h" #include "test_util.h" -static __thread volatile struct rseq __rseq = { - .cpu_id = RSEQ_CPU_ID_UNINITIALIZED, -}; - -/* - * Use an arbitrary, bogus signature for configuring rseq, this test does not - * actually enter an rseq critical section. - */ -#define RSEQ_SIG 0xdeadbeef +#include "../rseq/rseq.c" /* * Any bug related to task migration is likely to be timing-dependent; perform @@ -37,6 +29,7 @@ static __thread volatile struct rseq __rseq = { #define NR_TASK_MIGRATIONS 100000 static pthread_t migration_thread; +static struct rseq_abi *__rseq; static cpu_set_t possible_mask; static int min_cpu, max_cpu; static bool done; @@ -49,14 +42,6 @@ static void guest_code(void) GUEST_SYNC(0); } -static void sys_rseq(int flags) -{ - int r; - - r = syscall(__NR_rseq, &__rseq, sizeof(__rseq), flags, RSEQ_SIG); - TEST_ASSERT(!r, "rseq failed, errno = %d (%s)", errno, strerror(errno)); -} - static int next_cpu(int cpu) { /* @@ -218,7 +203,10 @@ int main(int argc, char *argv[]) calc_min_max_cpu(); - sys_rseq(0); + r = rseq_register_current_thread(); + TEST_ASSERT(!r, "rseq_register_current_thread failed, errno = %d (%s)", + errno, strerror(errno)); + __rseq = rseq_get_abi(); /* * Create and run a dummy VM that immediately exits to userspace via @@ -256,7 +244,7 @@ int main(int argc, char *argv[]) */ smp_rmb(); cpu = sched_getcpu(); - rseq_cpu = READ_ONCE(__rseq.cpu_id); + rseq_cpu = READ_ONCE(__rseq->cpu_id); smp_rmb(); } while (snapshot != atomic_read(&seq_cnt)); @@ -278,7 +266,7 @@ int main(int argc, char *argv[]) kvm_vm_free(vm); - sys_rseq(RSEQ_FLAG_UNREGISTER); + rseq_unregister_current_thread(); return 0; }
The rseq information is registered by TLS, starting from glibc-2.35. In this case, the test always fails due to syscall(__NR_rseq). For example, on RHEL9.1 where upstream glibc-2.35 features are enabled on downstream glibc-2.34, the test fails like below. # ./rseq_test ==== Test Assertion Failure ==== rseq_test.c:60: !r pid=112043 tid=112043 errno=22 - Invalid argument 1 0x0000000000401973: main at rseq_test.c:226 2 0x0000ffff84b6c79b: ?? ??:0 3 0x0000ffff84b6c86b: ?? ??:0 4 0x0000000000401b6f: _start at ??:? rseq failed, errno = 22 (Invalid argument) # rpm -aq | grep glibc-2 glibc-2.34-39.el9.aarch64 Fix the issue by using "../rseq/rseq.c" to fetch the rseq information, registred by TLS if it exists. Otherwise, we're going to register our own rseq information as before. Reported-by: Yihuang Yu <yihyu@redhat.com> Suggested-by: Florian Weimer <fweimer@redhat.com> Suggested-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> Suggested-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Gavin Shan <gshan@redhat.com> --- tools/testing/selftests/kvm/Makefile | 5 +++-- tools/testing/selftests/kvm/rseq_test.c | 28 +++++++------------------ 2 files changed, 11 insertions(+), 22 deletions(-)