[v2] kvm/selftests: Two rseq_test fixes

[PATCH v2 1/2] KVM: selftests: Make rseq compatible with glibc-2.35

Posted by Gavin Shan 3 years, 8 months ago

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;
 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;
 }
-- 
2.23.0

Re: [PATCH v2 1/2] KVM: selftests: Make rseq compatible with glibc-2.35

Posted by Mathieu Desnoyers 3 years, 8 months ago

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

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

Re: [PATCH v2 1/2] KVM: selftests: Make rseq compatible with glibc-2.35

Posted by Paolo Bonzini 3 years, 8 months ago

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

Re: [PATCH v2 1/2] KVM: selftests: Make rseq compatible with glibc-2.35

Posted by Gavin Shan 3 years, 8 months ago

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

Re: [PATCH v2 1/2] KVM: selftests: Make rseq compatible with glibc-2.35

Posted by Mathieu Desnoyers 3 years, 8 months ago

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

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com