Hi Mathieu and Sean,
On 8/10/22 7:38 AM, Sean Christopherson wrote:
On Tue, Aug 09, 2022, Mathieu Desnoyers wrote:
----- On Aug 9, 2022, at 8:21 AM, Mathieu Desnoyers mathieu.desnoyers@efficios.com wrote:
----- Gavin Shan gshan@redhat.com wrote:
On 8/9/22 5:16 PM, Florian Weimer wrote:
> __builtin_thread_pointer doesn't work on all architectures/GCC > versions. > Is this a problem for selftests? >
It's a problem as the test case is running on all architectures. I think I need introduce our own __builtin_thread_pointer() for where it's not supported: (1) PowerPC (2) x86 without GCC 11
Please let me know if I still have missed cases where __buitin_thread_pointer() isn't supported?
As far as I know, these are the two outliers that also have rseq support. The list is a bit longer if we also consider non-rseq architectures (csky, hppa, ia64, m68k, microblaze, sparc, don't know about the Linux architectures without glibc support).
For kvm/selftests, there are 3 architectures involved actually. So we just need consider 4 cases: aarch64, x86, s390 and other. For other case, we just use __builtin_thread_pointer() to maintain code's integrity, but it's not called at all.
I think kvm/selftest is always relying on glibc if I'm correct.
All those are handled in the rseq selftests and in librseq. Why duplicate all that logic again?
More to the point, considering that we have all the relevant rseq registration code in tools/testing/selftests/rseq/rseq.c already, and the relevant thread pointer getter code in tools/testing/selftests/rseq/rseq-*thread-pointer.h, is there an easy way to get test applications in tools/testing/selftests/kvm and in tools/testing/selftests/rseq to share that common code ?
Keeping duplicated compatibility code is bad for long-term maintainability.
Any reason not to simply add tools/lib/rseq.c and then expose a helper to get the registered rseq struct?
There are couple of reasons, not to share tools/testing/selftests/rseq/librseq.so or add tools/lib/librseq.so. Please let me know if the arguments making sense to you?
- By design, selftests/rseq and selftests/kvm are parallel. It's going to introduce unnecessary dependency for selftests/kvm to use selftests/rseq/librseq.so. To me, it makes the maintainability even harder.
- What selftests/kvm needs is rseq-thread-pointer.h, which accounts for ~5% of functionalities, provided by selftests/rseq/librseq.so.
- I'm not too much familiar with selftests/rseq, but it seems it need heavy rework before it can become tools/lib/librseq.so. However, I'm not sure if the effort is worthwhile. The newly added library is fully used by testtests/rseq. ~5% of that is going to be used by selftests/kvm. In this case, we still have cross-dependency issue.
I personally prefer not to use selftests/rseq/librseq.so or add tools/lib/librseq.so, but I need your feedback. Please share your thoughts. Thanks, Gavin