Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Reply-To: Gavin Shan <gshan@redhat.com>
Subject: Re: [PATCH 1/2] KVM: selftests: Make rseq compatible with glibc-2.35
To:     Florian Weimer <fweimer@redhat.com>
Cc:     kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org,
        linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org,
        pbonzini@redhat.com, maz@kernel.org, oliver.upton@linux.dev,
        andrew.jones@linux.dev, seanjc@google.com,
        mathieu.desnoyers@efficios.com, yihyu@redhat.com,
        shan.gavin@gmail.com
References: <20220809060627.115847-1-gshan@redhat.com>
 <20220809060627.115847-2-gshan@redhat.com>
 <8735e6ncxw.fsf@oldenburg.str.redhat.com>
From:   Gavin Shan <gshan@redhat.com>
Message-ID: <7844e3fa-e49e-de75-e424-e82d3a023dd6@redhat.com>
Date:   Tue, 9 Aug 2022 18:45:26 +1000
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.2.0
MIME-Version: 1.0
In-Reply-To: <8735e6ncxw.fsf@oldenburg.str.redhat.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi Florian,

On 8/9/22 4:33 PM, Florian Weimer wrote:
>> diff --git a/tools/testing/selftests/kvm/rseq_test.c b/tools/testing/selftests/kvm/rseq_test.c
>> index a54d4d05a058..acb1bf1f06b3 100644
>> --- a/tools/testing/selftests/kvm/rseq_test.c
>> +++ b/tools/testing/selftests/kvm/rseq_test.c
>> @@ -9,6 +9,7 @@
>>   #include <string.h>
>>   #include <signal.h>
>>   #include <syscall.h>
>> +#include <dlfcn.h>
>>   #include <sys/ioctl.h>
>>   #include <sys/sysinfo.h>
>>   #include <asm/barrier.h>
> 
> I'm surprised that there isn't a Makefile update to link with -ldl
> (still required for glibc 2.33 and earlier).
> 

In next revision, I will add '-ldl' into tools/testing/selftests/kvm/Makefile.


>> @@ -36,6 +37,8 @@ static __thread volatile struct rseq __rseq = {
>>    */
>>   #define NR_TASK_MIGRATIONS 100000
>>   
>> +static bool __rseq_ownership;
>> +static volatile struct rseq *__rseq_info;
>>   static pthread_t migration_thread;
>>   static cpu_set_t possible_mask;
>>   static int min_cpu, max_cpu;
>> @@ -49,11 +52,33 @@ static void guest_code(void)
>>   		GUEST_SYNC(0);
>>   }
>>   
>> +static void sys_rseq_ownership(void)
>> +{
>> +	long *offset;
>> +	unsigned int *size, *flags;
>> +
>> +	offset = dlsym(RTLD_NEXT, "__rseq_offset");
>> +	size = dlsym(RTLD_NEXT, "__rseq_size");
>> +	flags = dlsym(RTLD_NEXT, "__rseq_flags");
>> +
>> +	if (offset && size && *size && flags) {
>> +		__rseq_ownership = false;
>> +		__rseq_info = (struct rseq *)((uintptr_t)__builtin_thread_pointer() +
>> +					      *offset);
> 
> __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?

Thanks,
Gavin