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@xxxxxxxxxxxx wrote:
----- Gavin Shan <gshan@xxxxxxxxxx> 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
