All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
To: Gavin Shan <gshan@redhat.com>
Cc: Florian Weimer <fweimer@redhat.com>,
	shan gavin <shan.gavin@gmail.com>, KVM list <kvm@vger.kernel.org>,
	maz <maz@kernel.org>, linux-kernel <linux-kernel@vger.kernel.org>,
	andrew jones <andrew.jones@linux.dev>, yihyu <yihyu@redhat.com>,
	linux-kselftest <linux-kselftest@vger.kernel.org>,
	Paolo Bonzini <pbonzini@redhat.com>,
	kvmarm <kvmarm@lists.cs.columbia.edu>
Subject: Re: [PATCH v2 1/2] KVM: selftests: Make rseq compatible with glibc-2.35
Date: Wed, 10 Aug 2022 08:22:36 -0400 (EDT)	[thread overview]
Message-ID: <876568572.367.1660134156963.JavaMail.zimbra@efficios.com> (raw)
In-Reply-To: <20220810104114.6838-2-gshan@redhat.com>

----- 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
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

WARNING: multiple messages have this Message-ID (diff)
From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
To: Gavin Shan <gshan@redhat.com>
Cc: kvmarm <kvmarm@lists.cs.columbia.edu>,
	KVM list <kvm@vger.kernel.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	linux-kselftest <linux-kselftest@vger.kernel.org>,
	Florian Weimer <fweimer@redhat.com>,
	shan gavin <shan.gavin@gmail.com>, maz <maz@kernel.org>,
	andrew jones <andrew.jones@linux.dev>,
	Paolo Bonzini <pbonzini@redhat.com>, yihyu <yihyu@redhat.com>,
	Sean Christopherson <seanjc@google.com>,
	oliver upton <oliver.upton@linux.dev>
Subject: Re: [PATCH v2 1/2] KVM: selftests: Make rseq compatible with glibc-2.35
Date: Wed, 10 Aug 2022 08:22:36 -0400 (EDT)	[thread overview]
Message-ID: <876568572.367.1660134156963.JavaMail.zimbra@efficios.com> (raw)
In-Reply-To: <20220810104114.6838-2-gshan@redhat.com>

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

  reply	other threads:[~2022-08-10 12:22 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-10 10:41 [PATCH v2 0/2] kvm/selftests: Two rseq_test fixes Gavin Shan
2022-08-10 10:41 ` Gavin Shan
2022-08-10 10:41 ` [PATCH v2 1/2] KVM: selftests: Make rseq compatible with glibc-2.35 Gavin Shan
2022-08-10 10:41   ` Gavin Shan
2022-08-10 12:22   ` Mathieu Desnoyers [this message]
2022-08-10 12:22     ` Mathieu Desnoyers
2022-08-10 12:29     ` Paolo Bonzini
2022-08-10 12:29       ` Paolo Bonzini
2022-08-10 12:30       ` Mathieu Desnoyers
2022-08-10 12:30         ` Mathieu Desnoyers
2022-08-10 23:57       ` Gavin Shan
2022-08-10 23:57         ` Gavin Shan
2022-08-10 10:41 ` [PATCH v2 2/2] KVM: selftests: Use getcpu() instead of sched_getcpu() in rseq_test Gavin Shan
2022-08-10 10:41   ` Gavin Shan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=876568572.367.1660134156963.JavaMail.zimbra@efficios.com \
    --to=mathieu.desnoyers@efficios.com \
    --cc=andrew.jones@linux.dev \
    --cc=fweimer@redhat.com \
    --cc=gshan@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.cs.columbia.edu \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=maz@kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=shan.gavin@gmail.com \
    --cc=yihyu@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.