[PATCH 5/5] lib/cobalt/clock.c:dispatch clock_settime

[PATCH 5/5] lib/cobalt/clock.c:dispatch clock_settime

[chensong]

Regarding sizeof time_t, dispatch 32bit timespec to clock_settime
and 64bit timespec to clock_settime64.

Signed-off-by: chensong <chensong@tj.kylinos.cn>
---
 lib/cobalt/clock.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/lib/cobalt/clock.c b/lib/cobalt/clock.c
index 11fd1aa..44b2f3f 100644
--- a/lib/cobalt/clock.c
+++ b/lib/cobalt/clock.c
@@ -237,7 +237,11 @@ COBALT_IMPL(int, clock_settime, (clockid_t clock_id, const struct timespec *tp))
 {
 	int ret;
 
-	ret = -XENOMAI_SYSCALL2(sc_cobalt_clock_settime, clock_id, tp);
+	if (sizeof(time_t) > 4)
+		ret = -XENOMAI_SYSCALL2(sc_cobalt_clock_settime64,
+					clock_id, tp);
+	else
+		ret = -XENOMAI_SYSCALL2(sc_cobalt_clock_settime, clock_id, tp);
 	if (ret) {
 		errno = ret;
 		return -1;
-- 
2.7.4

Re: [PATCH 5/5] lib/cobalt/clock.c:dispatch clock_settime

[Jan Kiszka]

On 03.11.20 04:05, chensong wrote:
> Regarding sizeof time_t, dispatch 32bit timespec to clock_settime
> and 64bit timespec to clock_settime64.
> 
> Signed-off-by: chensong <chensong@tj.kylinos.cn>
> ---
>  lib/cobalt/clock.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/cobalt/clock.c b/lib/cobalt/clock.c
> index 11fd1aa..44b2f3f 100644
> --- a/lib/cobalt/clock.c
> +++ b/lib/cobalt/clock.c
> @@ -237,7 +237,11 @@ COBALT_IMPL(int, clock_settime, (clockid_t clock_id, const struct timespec *tp))
>  {
>  	int ret;
>  
> -	ret = -XENOMAI_SYSCALL2(sc_cobalt_clock_settime, clock_id, tp);
> +	if (sizeof(time_t) > 4)
> +		ret = -XENOMAI_SYSCALL2(sc_cobalt_clock_settime64,
> +					clock_id, tp);
> +	else
> +		ret = -XENOMAI_SYSCALL2(sc_cobalt_clock_settime, clock_id, tp);

Maybe do a single

XENOMAI_SYSCALL2(sizeof(time_t) > 4 ? sc_cobalt_clock_settime64
                                    : sc_cobalt_clock_settime, ...)

But we need ABI revision negotiation here. If a new lib is run on a
kernel that does not have settime64, things will fail (the other way
around is fine). We need to ensure that new userspace can check for this
upfront while old userspace will happily run (hard requirement for
making these patches part of 3.1.x).

Jan

>  	if (ret) {
>  		errno = ret;
>  		return -1;
> 

-- 
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux

Re: [PATCH 5/5] lib/cobalt/clock.c:dispatch clock_settime

[chensong]

On 2020年11月10日 18:24, Jan Kiszka wrote:
>
> On 03.11.20 04:05, chensong wrote:
>> Regarding sizeof time_t, dispatch 32bit timespec to clock_settime
>> and 64bit timespec to clock_settime64.
>>
>> Signed-off-by: chensong <chensong@tj.kylinos.cn>
>> ---
>>   lib/cobalt/clock.c | 6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/cobalt/clock.c b/lib/cobalt/clock.c
>> index 11fd1aa..44b2f3f 100644
>> --- a/lib/cobalt/clock.c
>> +++ b/lib/cobalt/clock.c
>> @@ -237,7 +237,11 @@ COBALT_IMPL(int, clock_settime, (clockid_t clock_id, const struct timespec *tp))
>>   {
>>   	int ret;
>>
>> -	ret = -XENOMAI_SYSCALL2(sc_cobalt_clock_settime, clock_id, tp);
>> +	if (sizeof(time_t) > 4)
>> +		ret = -XENOMAI_SYSCALL2(sc_cobalt_clock_settime64,
>> +					clock_id, tp);
>> +	else
>> +		ret = -XENOMAI_SYSCALL2(sc_cobalt_clock_settime, clock_id, tp);
>
> Maybe do a single
>
> XENOMAI_SYSCALL2(sizeof(time_t) > 4 ? sc_cobalt_clock_settime64
>                                      : sc_cobalt_clock_settime, ...)
>
> But we need ABI revision negotiation here. If a new lib is run on a
> kernel that does not have settime64, things will fail (the other way
> around is fine). We need to ensure that new userspace can check for this
> upfront while old userspace will happily run (hard requirement for
> making these patches part of 3.1.x).
>
> Jan

As far as my understanding, ABI revision negotiation happens in low_init 
already, it reminds userspace process that lib and kernel are not 
compatible at the first place. Therefore, it addresses the scenario you 
are talking about(new userspace, old kernel or the other way around).

what's more, do we or customers release lib without kernel. About y2038, 
vanilla kernel, glibc, libcobalt, xenomai, too many scenarios to be 
addressed, to complicated, we'd better simplify it if possible.

chensong
>
>>   	if (ret) {
>>   		errno = ret;
>>   		return -1;
>>
>

Re: [PATCH 5/5] lib/cobalt/clock.c:dispatch clock_settime

[Jan Kiszka]

On 11.11.20 04:55, chensong wrote:
> 
> 
> On 2020年11月10日 18:24, Jan Kiszka wrote:
>>
>> On 03.11.20 04:05, chensong wrote:
>>> Regarding sizeof time_t, dispatch 32bit timespec to clock_settime
>>> and 64bit timespec to clock_settime64.
>>>
>>> Signed-off-by: chensong <chensong@tj.kylinos.cn>
>>> ---
>>>   lib/cobalt/clock.c | 6 +++++-
>>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/lib/cobalt/clock.c b/lib/cobalt/clock.c
>>> index 11fd1aa..44b2f3f 100644
>>> --- a/lib/cobalt/clock.c
>>> +++ b/lib/cobalt/clock.c
>>> @@ -237,7 +237,11 @@ COBALT_IMPL(int, clock_settime, (clockid_t
>>> clock_id, const struct timespec *tp))
>>>   {
>>>       int ret;
>>>
>>> -    ret = -XENOMAI_SYSCALL2(sc_cobalt_clock_settime, clock_id, tp);
>>> +    if (sizeof(time_t) > 4)
>>> +        ret = -XENOMAI_SYSCALL2(sc_cobalt_clock_settime64,
>>> +                    clock_id, tp);
>>> +    else
>>> +        ret = -XENOMAI_SYSCALL2(sc_cobalt_clock_settime, clock_id, tp);
>>
>> Maybe do a single
>>
>> XENOMAI_SYSCALL2(sizeof(time_t) > 4 ? sc_cobalt_clock_settime64
>>                                      : sc_cobalt_clock_settime, ...)
>>
>> But we need ABI revision negotiation here. If a new lib is run on a
>> kernel that does not have settime64, things will fail (the other way
>> around is fine). We need to ensure that new userspace can check for this
>> upfront while old userspace will happily run (hard requirement for
>> making these patches part of 3.1.x).
>>
>> Jan
> 
> As far as my understanding, ABI revision negotiation happens in low_init
> already, it reminds userspace process that lib and kernel are not
> compatible at the first place. Therefore, it addresses the scenario you
> are talking about(new userspace, old kernel or the other way around).
> 

Ah, right. Perfect. Then also update those revisions make detection work
as required.

> what's more, do we or customers release lib without kernel. About y2038,
> vanilla kernel, glibc, libcobalt, xenomai, too many scenarios to be
> addressed, to complicated, we'd better simplify it if possible.

Xenomai does not issue separate releases for the kernel part
implementing the syscalls and for the xenomai libs using them.

However, applications may have been bundled with libs from previous
releases of *the same stable series*, and users can expect (bugs aside,
new features excluded) that their xenomai lib from, say, 3.1 will still
work with a core from 3.1.1.

Jan
-- 
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux

Re: [PATCH 5/5] lib/cobalt/clock.c:dispatch clock_settime

[chensong]

On 2020年11月11日 15:29, Jan Kiszka wrote:
> On 11.11.20 04:55, chensong wrote:
>>
>>
>> On 2020年11月10日 18:24, Jan Kiszka wrote:
>>>
>>> On 03.11.20 04:05, chensong wrote:
>>>> Regarding sizeof time_t, dispatch 32bit timespec to clock_settime
>>>> and 64bit timespec to clock_settime64.
>>>>
>>>> Signed-off-by: chensong <chensong@tj.kylinos.cn>
>>>> ---
>>>>    lib/cobalt/clock.c | 6 +++++-
>>>>    1 file changed, 5 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/lib/cobalt/clock.c b/lib/cobalt/clock.c
>>>> index 11fd1aa..44b2f3f 100644
>>>> --- a/lib/cobalt/clock.c
>>>> +++ b/lib/cobalt/clock.c
>>>> @@ -237,7 +237,11 @@ COBALT_IMPL(int, clock_settime, (clockid_t
>>>> clock_id, const struct timespec *tp))
>>>>    {
>>>>        int ret;
>>>>
>>>> -    ret = -XENOMAI_SYSCALL2(sc_cobalt_clock_settime, clock_id, tp);
>>>> +    if (sizeof(time_t) > 4)
>>>> +        ret = -XENOMAI_SYSCALL2(sc_cobalt_clock_settime64,
>>>> +                    clock_id, tp);
>>>> +    else
>>>> +        ret = -XENOMAI_SYSCALL2(sc_cobalt_clock_settime, clock_id, tp);
>>>
>>> Maybe do a single
>>>
>>> XENOMAI_SYSCALL2(sizeof(time_t) > 4 ? sc_cobalt_clock_settime64
>>>                                       : sc_cobalt_clock_settime, ...)
>>>
>>> But we need ABI revision negotiation here. If a new lib is run on a
>>> kernel that does not have settime64, things will fail (the other way
>>> around is fine). We need to ensure that new userspace can check for this
>>> upfront while old userspace will happily run (hard requirement for
>>> making these patches part of 3.1.x).
>>>
>>> Jan
>>
>> As far as my understanding, ABI revision negotiation happens in low_init
>> already, it reminds userspace process that lib and kernel are not
>> compatible at the first place. Therefore, it addresses the scenario you
>> are talking about(new userspace, old kernel or the other way around).
>>
>
> Ah, right. Perfect. Then also update those revisions make detection work
> as required.
>
ok, will do.

XENOMAI_ABI_REV is defined in those files:

./kernel/cobalt/arch/arm/include/asm/xenomai/uapi/features.h:25:#define 
XENOMAI_ABI_REV   17UL
./kernel/cobalt/arch/x86/include/asm/xenomai/uapi/features.h:22:#define 
XENOMAI_ABI_REV   17UL
./kernel/cobalt/arch/arm64/include/asm/xenomai/uapi/features.h:25:#define XENOMAI_ABI_REV 
   1UL
./kernel/cobalt/arch/powerpc/include/asm/xenomai/uapi/features.h:22:#define 
XENOMAI_ABI_REV   17UL


increase them to 18UL?

if you agree, i will submit a v2 for this one.

>> what's more, do we or customers release lib without kernel. About y2038,
>> vanilla kernel, glibc, libcobalt, xenomai, too many scenarios to be
>> addressed, to complicated, we'd better simplify it if possible.
>
> Xenomai does not issue separate releases for the kernel part
> implementing the syscalls and for the xenomai libs using them.
>
> However, applications may have been bundled with libs from previous
> releases of *the same stable series*, and users can expect (bugs aside,
> new features excluded) that their xenomai lib from, say, 3.1 will still
> work with a core from 3.1.1.
>
> Jan
>

In those cases, they will get "ABI mismatch, required xxx, provided 
xxx", user will realize they need new kernel with new lib.

My point is, ignore those too old things. arnd Bergmann dropped 
something as well.

chensong

Re: [PATCH 5/5] lib/cobalt/clock.c:dispatch clock_settime

[Jan Kiszka]

On 11.11.20 10:23, chensong wrote:
> 
> 
> On 2020年11月11日 15:29, Jan Kiszka wrote:
>> On 11.11.20 04:55, chensong wrote:
>>>
>>>
>>> On 2020年11月10日 18:24, Jan Kiszka wrote:
>>>>
>>>> On 03.11.20 04:05, chensong wrote:
>>>>> Regarding sizeof time_t, dispatch 32bit timespec to clock_settime
>>>>> and 64bit timespec to clock_settime64.
>>>>>
>>>>> Signed-off-by: chensong <chensong@tj.kylinos.cn>
>>>>> ---
>>>>>    lib/cobalt/clock.c | 6 +++++-
>>>>>    1 file changed, 5 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/lib/cobalt/clock.c b/lib/cobalt/clock.c
>>>>> index 11fd1aa..44b2f3f 100644
>>>>> --- a/lib/cobalt/clock.c
>>>>> +++ b/lib/cobalt/clock.c
>>>>> @@ -237,7 +237,11 @@ COBALT_IMPL(int, clock_settime, (clockid_t
>>>>> clock_id, const struct timespec *tp))
>>>>>    {
>>>>>        int ret;
>>>>>
>>>>> -    ret = -XENOMAI_SYSCALL2(sc_cobalt_clock_settime, clock_id, tp);
>>>>> +    if (sizeof(time_t) > 4)
>>>>> +        ret = -XENOMAI_SYSCALL2(sc_cobalt_clock_settime64,
>>>>> +                    clock_id, tp);
>>>>> +    else
>>>>> +        ret = -XENOMAI_SYSCALL2(sc_cobalt_clock_settime, clock_id,
>>>>> tp);
>>>>
>>>> Maybe do a single
>>>>
>>>> XENOMAI_SYSCALL2(sizeof(time_t) > 4 ? sc_cobalt_clock_settime64
>>>>                                       : sc_cobalt_clock_settime, ...)
>>>>
>>>> But we need ABI revision negotiation here. If a new lib is run on a
>>>> kernel that does not have settime64, things will fail (the other way
>>>> around is fine). We need to ensure that new userspace can check for
>>>> this
>>>> upfront while old userspace will happily run (hard requirement for
>>>> making these patches part of 3.1.x).
>>>>
>>>> Jan
>>>
>>> As far as my understanding, ABI revision negotiation happens in low_init
>>> already, it reminds userspace process that lib and kernel are not
>>> compatible at the first place. Therefore, it addresses the scenario you
>>> are talking about(new userspace, old kernel or the other way around).
>>>
>>
>> Ah, right. Perfect. Then also update those revisions make detection work
>> as required.
>>
> ok, will do.
> 
> XENOMAI_ABI_REV is defined in those files:
> 
> ./kernel/cobalt/arch/arm/include/asm/xenomai/uapi/features.h:25:#define
> XENOMAI_ABI_REV   17UL
> ./kernel/cobalt/arch/x86/include/asm/xenomai/uapi/features.h:22:#define
> XENOMAI_ABI_REV   17UL
> ./kernel/cobalt/arch/arm64/include/asm/xenomai/uapi/features.h:25:#define XENOMAI_ABI_REV
>   1UL
> ./kernel/cobalt/arch/powerpc/include/asm/xenomai/uapi/features.h:22:#define
> XENOMAI_ABI_REV   17UL
> 
> 
> increase them to 18UL?
> 
> if you agree, i will submit a v2 for this one.

We need to increase, yes, but we need to keep previous revision userland
working.

> 
>>> what's more, do we or customers release lib without kernel. About y2038,
>>> vanilla kernel, glibc, libcobalt, xenomai, too many scenarios to be
>>> addressed, to complicated, we'd better simplify it if possible.
>>
>> Xenomai does not issue separate releases for the kernel part
>> implementing the syscalls and for the xenomai libs using them.
>>
>> However, applications may have been bundled with libs from previous
>> releases of *the same stable series*, and users can expect (bugs aside,
>> new features excluded) that their xenomai lib from, say, 3.1 will still
>> work with a core from 3.1.1.
>>
>> Jan
>>
> 
> In those cases, they will get "ABI mismatch, required xxx, provided
> xxx", user will realize they need new kernel with new lib.
> 
> My point is, ignore those too old things. arnd Bergmann dropped
> something as well.
> 

We can't break the scenario described above. That's in fact in line with
what kernel and libc do, just that they also account for newer libc
running on older kernels (while we deny that).

Jan
-- 
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux

Re: [PATCH 5/5] lib/cobalt/clock.c:dispatch clock_settime

[chensong]

On 2020年11月11日 17:43, Jan Kiszka wrote:
> On 11.11.20 10:23, chensong wrote:
>>
>>
>> On 2020年11月11日 15:29, Jan Kiszka wrote:
>>> On 11.11.20 04:55, chensong wrote:
>>>>
>>>>
>>>> On 2020年11月10日 18:24, Jan Kiszka wrote:
>>>>>
>>>>> On 03.11.20 04:05, chensong wrote:
>>>>>> Regarding sizeof time_t, dispatch 32bit timespec to clock_settime
>>>>>> and 64bit timespec to clock_settime64.
>>>>>>
>>>>>> Signed-off-by: chensong <chensong@tj.kylinos.cn>
>>>>>> ---
>>>>>>     lib/cobalt/clock.c | 6 +++++-
>>>>>>     1 file changed, 5 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/lib/cobalt/clock.c b/lib/cobalt/clock.c
>>>>>> index 11fd1aa..44b2f3f 100644
>>>>>> --- a/lib/cobalt/clock.c
>>>>>> +++ b/lib/cobalt/clock.c
>>>>>> @@ -237,7 +237,11 @@ COBALT_IMPL(int, clock_settime, (clockid_t
>>>>>> clock_id, const struct timespec *tp))
>>>>>>     {
>>>>>>         int ret;
>>>>>>
>>>>>> -    ret = -XENOMAI_SYSCALL2(sc_cobalt_clock_settime, clock_id, tp);
>>>>>> +    if (sizeof(time_t) > 4)
>>>>>> +        ret = -XENOMAI_SYSCALL2(sc_cobalt_clock_settime64,
>>>>>> +                    clock_id, tp);
>>>>>> +    else
>>>>>> +        ret = -XENOMAI_SYSCALL2(sc_cobalt_clock_settime, clock_id,
>>>>>> tp);
>>>>>
>>>>> Maybe do a single
>>>>>
>>>>> XENOMAI_SYSCALL2(sizeof(time_t) > 4 ? sc_cobalt_clock_settime64
>>>>>                                        : sc_cobalt_clock_settime, ...)
>>>>>
>>>>> But we need ABI revision negotiation here. If a new lib is run on a
>>>>> kernel that does not have settime64, things will fail (the other way
>>>>> around is fine). We need to ensure that new userspace can check for
>>>>> this
>>>>> upfront while old userspace will happily run (hard requirement for
>>>>> making these patches part of 3.1.x).
>>>>>
>>>>> Jan
>>>>
>>>> As far as my understanding, ABI revision negotiation happens in low_init
>>>> already, it reminds userspace process that lib and kernel are not
>>>> compatible at the first place. Therefore, it addresses the scenario you
>>>> are talking about(new userspace, old kernel or the other way around).
>>>>
>>>
>>> Ah, right. Perfect. Then also update those revisions make detection work
>>> as required.
>>>
>> ok, will do.
>>
>> XENOMAI_ABI_REV is defined in those files:
>>
>> ./kernel/cobalt/arch/arm/include/asm/xenomai/uapi/features.h:25:#define
>> XENOMAI_ABI_REV   17UL
>> ./kernel/cobalt/arch/x86/include/asm/xenomai/uapi/features.h:22:#define
>> XENOMAI_ABI_REV   17UL
>> ./kernel/cobalt/arch/arm64/include/asm/xenomai/uapi/features.h:25:#define XENOMAI_ABI_REV
>>    1UL
>> ./kernel/cobalt/arch/powerpc/include/asm/xenomai/uapi/features.h:22:#define
>> XENOMAI_ABI_REV   17UL
>>
>>
>> increase them to 18UL?
>>
>> if you agree, i will submit a v2 for this one.
>
> We need to increase, yes, but we need to keep previous revision userland
> working.

low_init will stop previous revision userland working once we increase it.

>
>>
>>>> what's more, do we or customers release lib without kernel. About y2038,
>>>> vanilla kernel, glibc, libcobalt, xenomai, too many scenarios to be
>>>> addressed, to complicated, we'd better simplify it if possible.
>>>
>>> Xenomai does not issue separate releases for the kernel part
>>> implementing the syscalls and for the xenomai libs using them.
>>>
>>> However, applications may have been bundled with libs from previous
>>> releases of *the same stable series*, and users can expect (bugs aside,
>>> new features excluded) that their xenomai lib from, say, 3.1 will still
>>> work with a core from 3.1.1.
if XENOMAI_ABI_REV in lib is different with XENOMAI_ABI_REV in kernel, 
low_init will stop it running at the first place.

Even if it doesn't, old application, old lib, will go to clock_settime, 
i can't see any impact here.

chensong

>>>
>>> Jan
>>>
>>
>> In those cases, they will get "ABI mismatch, required xxx, provided
>> xxx", user will realize they need new kernel with new lib.
>>
>> My point is, ignore those too old things. arnd Bergmann dropped
>> something as well.
>>
>
> We can't break the scenario described above. That's in fact in line with
> what kernel and libc do, just that they also account for newer libc
> running on older kernels (while we deny that).
>
> Jan
>

Re: [PATCH 5/5] lib/cobalt/clock.c:dispatch clock_settime

[Florian Bezdeka]

Hi all,

I reviewed all the timeset64 related patches.
Please note the comments below.

Did anyone look into testing yet? There are some combinations of kernel
and library versions we should look at and make sure they all behave as
expected. Maybe xenomai-images "project" could help here?

On 11.11.20 11:13, chensong via Xenomai wrote:
> 
> 
> On 2020年11月11日 17:43, Jan Kiszka wrote:
>> On 11.11.20 10:23, chensong wrote:
>>>
>>>
>>> On 2020年11月11日 15:29, Jan Kiszka wrote:
>>>> On 11.11.20 04:55, chensong wrote:
>>>>>
>>>>>
>>>>> On 2020年11月10日 18:24, Jan Kiszka wrote:
>>>>>>
>>>>>> On 03.11.20 04:05, chensong wrote:
>>>>>>> Regarding sizeof time_t, dispatch 32bit timespec to clock_settime
>>>>>>> and 64bit timespec to clock_settime64.
>>>>>>>
>>>>>>> Signed-off-by: chensong <chensong@tj.kylinos.cn>
>>>>>>> ---
>>>>>>>     lib/cobalt/clock.c | 6 +++++-
>>>>>>>     1 file changed, 5 insertions(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/lib/cobalt/clock.c b/lib/cobalt/clock.c
>>>>>>> index 11fd1aa..44b2f3f 100644
>>>>>>> --- a/lib/cobalt/clock.c
>>>>>>> +++ b/lib/cobalt/clock.c
>>>>>>> @@ -237,7 +237,11 @@ COBALT_IMPL(int, clock_settime, (clockid_t
>>>>>>> clock_id, const struct timespec *tp))
>>>>>>>     {
>>>>>>>         int ret;
>>>>>>>
>>>>>>> -    ret = -XENOMAI_SYSCALL2(sc_cobalt_clock_settime, clock_id, tp);
>>>>>>> +    if (sizeof(time_t) > 4)
>>>>>>> +        ret = -XENOMAI_SYSCALL2(sc_cobalt_clock_settime64,
>>>>>>> +                    clock_id, tp);
>>>>>>> +    else
>>>>>>> +        ret = -XENOMAI_SYSCALL2(sc_cobalt_clock_settime, clock_id,
>>>>>>> tp);
>>>>>>
>>>>>> Maybe do a single
>>>>>>
>>>>>> XENOMAI_SYSCALL2(sizeof(time_t) > 4 ? sc_cobalt_clock_settime64
>>>>>>                                        : sc_cobalt_clock_settime,
>>>>>> ...)
>>>>>>
>>>>>> But we need ABI revision negotiation here. If a new lib is run on a
>>>>>> kernel that does not have settime64, things will fail (the other way
>>>>>> around is fine). We need to ensure that new userspace can check for
>>>>>> this
>>>>>> upfront while old userspace will happily run (hard requirement for
>>>>>> making these patches part of 3.1.x).
>>>>>>
>>>>>> Jan
>>>>>
>>>>> As far as my understanding, ABI revision negotiation happens in
>>>>> low_init
>>>>> already, it reminds userspace process that lib and kernel are not
>>>>> compatible at the first place. Therefore, it addresses the scenario
>>>>> you
>>>>> are talking about(new userspace, old kernel or the other way around).
>>>>>
>>>>
>>>> Ah, right. Perfect. Then also update those revisions make detection
>>>> work
>>>> as required.
>>>>
>>> ok, will do.
>>>
>>> XENOMAI_ABI_REV is defined in those files:
>>>
>>> ./kernel/cobalt/arch/arm/include/asm/xenomai/uapi/features.h:25:#define
>>> XENOMAI_ABI_REV   17UL
>>> ./kernel/cobalt/arch/x86/include/asm/xenomai/uapi/features.h:22:#define
>>> XENOMAI_ABI_REV   17UL
>>> ./kernel/cobalt/arch/arm64/include/asm/xenomai/uapi/features.h:25:#define
>>> XENOMAI_ABI_REV
>>>    1UL
>>> ./kernel/cobalt/arch/powerpc/include/asm/xenomai/uapi/features.h:22:#define
>>>
>>> XENOMAI_ABI_REV   17UL
>>>
>>>
>>> increase them to 18UL?
>>>
>>> if you agree, i will submit a v2 for this one.
>>
>> We need to increase, yes, but we need to keep previous revision userland
>> working.

Do we really need to increase the ABI revision?
My understanding is, that once the kernel has been updated applications
using an old library will be locked out. I guess this is not allowed
when bumping the xenomai minor revision only.

I will come up with a small patch set that allows some feature detection
during runtime. That should help with the identification which syscall
should be used.

Would be glad if you both could have a look at it.

> 
> low_init will stop previous revision userland working once we increase it.
> 
>>
>>>
>>>>> what's more, do we or customers release lib without kernel. About
>>>>> y2038,
>>>>> vanilla kernel, glibc, libcobalt, xenomai, too many scenarios to be
>>>>> addressed, to complicated, we'd better simplify it if possible.
>>>>
>>>> Xenomai does not issue separate releases for the kernel part
>>>> implementing the syscalls and for the xenomai libs using them.
>>>>
>>>> However, applications may have been bundled with libs from previous
>>>> releases of *the same stable series*, and users can expect (bugs aside,
>>>> new features excluded) that their xenomai lib from, say, 3.1 will still
>>>> work with a core from 3.1.1.
> if XENOMAI_ABI_REV in lib is different with XENOMAI_ABI_REV in kernel,
> low_init will stop it running at the first place.
> 
> Even if it doesn't, old application, old lib, will go to clock_settime,
> i can't see any impact here.
> 
> chensong
> 
>>>>
>>>> Jan
>>>>
>>>
>>> In those cases, they will get "ABI mismatch, required xxx, provided
>>> xxx", user will realize they need new kernel with new lib.
>>>
>>> My point is, ignore those too old things. arnd Bergmann dropped
>>> something as well.
>>>
>>
>> We can't break the scenario described above. That's in fact in line with
>> what kernel and libc do, just that they also account for newer libc
>> running on older kernels (while we deny that).
>>
>> Jan
>>
> 
> 
> 
> 


-- 
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux

Re: [PATCH 5/5] lib/cobalt/clock.c:dispatch clock_settime

[Jan Kiszka]

On 13.11.20 12:47, Florian Bezdeka wrote:
> Hi all,
> 
> I reviewed all the timeset64 related patches.
> Please note the comments below.
> 
> Did anyone look into testing yet? There are some combinations of kernel
> and library versions we should look at and make sure they all behave as
> expected. Maybe xenomai-images "project" could help here?
> 

At least we need something in smokey that can be used - in different
build configurations - to stress things. But as this is also about these
different configurations (32/64-bit old/new apps on old/new kernels), a
pre-integration via xenomai-image may help. There, we currently lack the
compat case as target.

> On 11.11.20 11:13, chensong via Xenomai wrote:
>>
>>
>> On 2020年11月11日 17:43, Jan Kiszka wrote:
>>> On 11.11.20 10:23, chensong wrote:
>>>>
>>>>
>>>> On 2020年11月11日 15:29, Jan Kiszka wrote:
>>>>> On 11.11.20 04:55, chensong wrote:
>>>>>>
>>>>>>
>>>>>> On 2020年11月10日 18:24, Jan Kiszka wrote:
>>>>>>>
>>>>>>> On 03.11.20 04:05, chensong wrote:
>>>>>>>> Regarding sizeof time_t, dispatch 32bit timespec to clock_settime
>>>>>>>> and 64bit timespec to clock_settime64.
>>>>>>>>
>>>>>>>> Signed-off-by: chensong <chensong@tj.kylinos.cn>
>>>>>>>> ---
>>>>>>>>     lib/cobalt/clock.c | 6 +++++-
>>>>>>>>     1 file changed, 5 insertions(+), 1 deletion(-)
>>>>>>>>
>>>>>>>> diff --git a/lib/cobalt/clock.c b/lib/cobalt/clock.c
>>>>>>>> index 11fd1aa..44b2f3f 100644
>>>>>>>> --- a/lib/cobalt/clock.c
>>>>>>>> +++ b/lib/cobalt/clock.c
>>>>>>>> @@ -237,7 +237,11 @@ COBALT_IMPL(int, clock_settime, (clockid_t
>>>>>>>> clock_id, const struct timespec *tp))
>>>>>>>>     {
>>>>>>>>         int ret;
>>>>>>>>
>>>>>>>> -    ret = -XENOMAI_SYSCALL2(sc_cobalt_clock_settime, clock_id, tp);
>>>>>>>> +    if (sizeof(time_t) > 4)
>>>>>>>> +        ret = -XENOMAI_SYSCALL2(sc_cobalt_clock_settime64,
>>>>>>>> +                    clock_id, tp);
>>>>>>>> +    else
>>>>>>>> +        ret = -XENOMAI_SYSCALL2(sc_cobalt_clock_settime, clock_id,
>>>>>>>> tp);
>>>>>>>
>>>>>>> Maybe do a single
>>>>>>>
>>>>>>> XENOMAI_SYSCALL2(sizeof(time_t) > 4 ? sc_cobalt_clock_settime64
>>>>>>>                                        : sc_cobalt_clock_settime,
>>>>>>> ...)
>>>>>>>
>>>>>>> But we need ABI revision negotiation here. If a new lib is run on a
>>>>>>> kernel that does not have settime64, things will fail (the other way
>>>>>>> around is fine). We need to ensure that new userspace can check for
>>>>>>> this
>>>>>>> upfront while old userspace will happily run (hard requirement for
>>>>>>> making these patches part of 3.1.x).
>>>>>>>
>>>>>>> Jan
>>>>>>
>>>>>> As far as my understanding, ABI revision negotiation happens in
>>>>>> low_init
>>>>>> already, it reminds userspace process that lib and kernel are not
>>>>>> compatible at the first place. Therefore, it addresses the scenario
>>>>>> you
>>>>>> are talking about(new userspace, old kernel or the other way around).
>>>>>>
>>>>>
>>>>> Ah, right. Perfect. Then also update those revisions make detection
>>>>> work
>>>>> as required.
>>>>>
>>>> ok, will do.
>>>>
>>>> XENOMAI_ABI_REV is defined in those files:
>>>>
>>>> ./kernel/cobalt/arch/arm/include/asm/xenomai/uapi/features.h:25:#define
>>>> XENOMAI_ABI_REV   17UL
>>>> ./kernel/cobalt/arch/x86/include/asm/xenomai/uapi/features.h:22:#define
>>>> XENOMAI_ABI_REV   17UL
>>>> ./kernel/cobalt/arch/arm64/include/asm/xenomai/uapi/features.h:25:#define
>>>> XENOMAI_ABI_REV
>>>>    1UL
>>>> ./kernel/cobalt/arch/powerpc/include/asm/xenomai/uapi/features.h:22:#define
>>>>
>>>> XENOMAI_ABI_REV   17UL
>>>>
>>>>
>>>> increase them to 18UL?
>>>>
>>>> if you agree, i will submit a v2 for this one.
>>>
>>> We need to increase, yes, but we need to keep previous revision userland
>>> working.
> 
> Do we really need to increase the ABI revision?
> My understanding is, that once the kernel has been updated applications
> using an old library will be locked out. I guess this is not allowed
> when bumping the xenomai minor revision only.
> 
> I will come up with a small patch set that allows some feature detection
> during runtime. That should help with the identification which syscall
> should be used.
> 
> Would be glad if you both could have a look at it.
> 

We could grant a new feature bit
(include/cobalt/uapi/asm-generic/features.h) to the 64-bit time ABI,
dropping it again on next ABI revision bump (as it will then become
mandatory).

Jan
-- 
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux

[PATCH 0/3] Make offered kernel features accessible during

[florian.bezdeka]

From: Florian Bezdeka <florian.bezdeka@siemens.com>

Hi all,

while reviewing the settime64 syscall patches I noticed that we do not
have any infrastructure that allows us to check which features are
offered by the currently running kernel.

With this patch series I try to fix this gap. Everything is compile
time tested only so far. I'm currently trying to bring my test
infrastructure up and running.

Comments welcome...

Florian Bezdeka (3):
  cobalt uapi: Introducing new feature flag for settime64 availability
  lib/cobalt: Rename cobalt_check_features to cobalt_arch_check_features
  lib/cobalt: Introduce generic feature initialization and check API

 include/cobalt/uapi/asm-generic/features.h |  6 +++-
 lib/cobalt/arch/arm/features.c             |  2 +-
 lib/cobalt/arch/arm64/features.c           |  2 +-
 lib/cobalt/arch/powerpc/features.c         |  2 +-
 lib/cobalt/arch/x86/features.c             |  2 +-
 lib/cobalt/init.c                          |  2 +-
 lib/cobalt/internal.c                      | 10 ++++++
 lib/cobalt/internal.h                      | 39 +++++++++++++++++++---
 8 files changed, 55 insertions(+), 10 deletions(-)

-- 
2.26.2

[PATCH 1/3] cobalt uapi: Introducing new feature flag for settime64 availability

[florian.bezdeka]

From: Florian Bezdeka <florian.bezdeka@siemens.com>

Adding a new feature flag to allow the library asking for settime64
support. That will allow the library to use the new system call if it
is available / supported by the kernel.

Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
 include/cobalt/uapi/asm-generic/features.h | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/cobalt/uapi/asm-generic/features.h b/include/cobalt/uapi/asm-generic/features.h
index 8a4927c49..93a96670a 100644
--- a/include/cobalt/uapi/asm-generic/features.h
+++ b/include/cobalt/uapi/asm-generic/features.h
@@ -51,6 +51,7 @@ struct cobalt_featinfo {
 #define __xn_feat_nofastsynch 0x10000000
 #define __xn_feat_control     0x08000000
 #define __xn_feat_prioceiling 0x04000000
+#define __xn_feat_settime64   0x02000000
 
 #ifdef CONFIG_SMP
 #define __xn_feat_smp_mask __xn_feat_smp
@@ -70,7 +71,8 @@ struct cobalt_featinfo {
 #define __xn_feat_generic_mask			\
 	(__xn_feat_smp_mask		|	\
 	 __xn_feat_fastsynch_mask 	|	\
-	 __xn_feat_prioceiling)
+	 __xn_feat_prioceiling		|	\
+	 __xn_feat_settime64 )
 
 /*
  * List of features both sides have to agree on: If userland supports
@@ -101,6 +103,8 @@ const char *get_generic_feature_label(unsigned int feature)
 		return "control";
 	case __xn_feat_prioceiling:
 		return "prioceiling";
+	case __xn_feat_settime64:
+		return "settime64";
 	default:
 		return 0;
 	}
-- 
2.26.2

[PATCH 2/3] lib/cobalt: Rename cobalt_check_features to cobalt_arch_check_features

[florian.bezdeka]

From: Florian Bezdeka <florian.bezdeka@siemens.com>

cobalt_check_features is implemented for each platform. As further
feature initialization will arrive the name of the function should
clarify that.

Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
 lib/cobalt/arch/arm/features.c     | 2 +-
 lib/cobalt/arch/arm64/features.c   | 2 +-
 lib/cobalt/arch/powerpc/features.c | 2 +-
 lib/cobalt/arch/x86/features.c     | 2 +-
 lib/cobalt/init.c                  | 2 +-
 lib/cobalt/internal.h              | 2 +-
 6 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/cobalt/arch/arm/features.c b/lib/cobalt/arch/arm/features.c
index edabcf25b..d7b50f73f 100644
--- a/lib/cobalt/arch/arm/features.c
+++ b/lib/cobalt/arch/arm/features.c
@@ -37,7 +37,7 @@ struct __xn_full_tscinfo __xn_tscinfo = {
 	},
 };
 
-void cobalt_check_features(struct cobalt_featinfo *finfo)
+void cobalt_arch_check_features(struct cobalt_featinfo *finfo)
 {
 	unsigned long phys_addr;
 	unsigned page_size;
diff --git a/lib/cobalt/arch/arm64/features.c b/lib/cobalt/arch/arm64/features.c
index 254c8aeae..8b85a27ea 100644
--- a/lib/cobalt/arch/arm64/features.c
+++ b/lib/cobalt/arch/arm64/features.c
@@ -37,7 +37,7 @@ struct __xn_full_tscinfo __xn_tscinfo = {
 	},
 };
 
-void cobalt_check_features(struct cobalt_featinfo *finfo)
+void cobalt_arch_check_features(struct cobalt_featinfo *finfo)
 {
 	unsigned long phys_addr;
 	unsigned page_size;
diff --git a/lib/cobalt/arch/powerpc/features.c b/lib/cobalt/arch/powerpc/features.c
index 6fc7afa64..d795a198a 100644
--- a/lib/cobalt/arch/powerpc/features.c
+++ b/lib/cobalt/arch/powerpc/features.c
@@ -19,6 +19,6 @@
 #include <asm/xenomai/syscall.h>
 #include "internal.h"
 
-void cobalt_check_features(struct cobalt_featinfo *finfo)
+void cobalt_arch_check_features(struct cobalt_featinfo *finfo)
 {
 }
diff --git a/lib/cobalt/arch/x86/features.c b/lib/cobalt/arch/x86/features.c
index f8ba4ee2c..e7e62da39 100644
--- a/lib/cobalt/arch/x86/features.c
+++ b/lib/cobalt/arch/x86/features.c
@@ -25,7 +25,7 @@
 #include <asm/xenomai/uapi/fptest.h>
 #include "internal.h"
 
-void cobalt_check_features(struct cobalt_featinfo *finfo)
+void cobalt_arch_check_features(struct cobalt_featinfo *finfo)
 {
 #if defined(__i386__) && defined(CONFIG_XENO_X86_VSYSCALL)
 	size_t n = confstr(_CS_GNU_LIBPTHREAD_VERSION, NULL, 0);
diff --git a/lib/cobalt/init.c b/lib/cobalt/init.c
index 9b506d872..6907ace36 100644
--- a/lib/cobalt/init.c
+++ b/lib/cobalt/init.c
@@ -177,7 +177,7 @@ static void low_init(void)
 		early_panic("mlockall: %s", strerror(errno));
 
 	trace_me("memory locked");
-	cobalt_check_features(f);
+	cobalt_arch_check_features(f);
 	cobalt_init_umm(f->vdso_offset);
 	trace_me("memory heaps mapped");
 	cobalt_init_current_keys();
diff --git a/lib/cobalt/internal.h b/lib/cobalt/internal.h
index c45a54d9f..4ca99d927 100644
--- a/lib/cobalt/internal.h
+++ b/lib/cobalt/internal.h
@@ -88,7 +88,7 @@ int cobalt_init(void);
 
 struct cobalt_featinfo;
 
-void cobalt_check_features(struct cobalt_featinfo *finfo);
+void cobalt_arch_check_features(struct cobalt_featinfo *finfo);
 
 extern struct sigaction __cobalt_orig_sigdebug;
 
-- 
2.26.2

[PATCH 3/3] lib/cobalt: Introduce generic feature initialization and check API

[florian.bezdeka]

From: Florian Bezdeka <florian.bezdeka@siemens.com>

The feature initialization was arch specific up to now and the
available features (= features offered by the currently running kernel)
were no longer accessible once the bind syscall finished.

This patch introduces a simple API for fetching the offered features
during runtime.

Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
 lib/cobalt/init.c     |  2 +-
 lib/cobalt/internal.c | 10 ++++++++++
 lib/cobalt/internal.h | 39 +++++++++++++++++++++++++++++++++++----
 3 files changed, 46 insertions(+), 5 deletions(-)

diff --git a/lib/cobalt/init.c b/lib/cobalt/init.c
index 6907ace36..20e28ccb0 100644
--- a/lib/cobalt/init.c
+++ b/lib/cobalt/init.c
@@ -177,7 +177,7 @@ static void low_init(void)
 		early_panic("mlockall: %s", strerror(errno));
 
 	trace_me("memory locked");
-	cobalt_arch_check_features(f);
+	cobalt_features_init(f);
 	cobalt_init_umm(f->vdso_offset);
 	trace_me("memory heaps mapped");
 	cobalt_init_current_keys();
diff --git a/lib/cobalt/internal.c b/lib/cobalt/internal.c
index 43330060a..303d86f99 100644
--- a/lib/cobalt/internal.c
+++ b/lib/cobalt/internal.c
@@ -565,3 +565,13 @@ void cobalt_assert_nrt(void)
 	if (cobalt_should_warn())
 		pthread_kill(pthread_self(), SIGDEBUG);
 }
+
+unsigned int cobalt_features;
+
+void cobalt_features_init(struct cobalt_featinfo *f)
+{
+	cobalt_features = f->feat_all;
+
+	/* Trigger arch specific feature initialization */
+	cobalt_arch_check_features(f);
+}
\ No newline at end of file
diff --git a/lib/cobalt/internal.h b/lib/cobalt/internal.h
index 4ca99d927..ac7ce24f3 100644
--- a/lib/cobalt/internal.h
+++ b/lib/cobalt/internal.h
@@ -86,10 +86,6 @@ int cobalt_xlate_schedparam(int policy,
 			    struct sched_param *param);
 int cobalt_init(void);
 
-struct cobalt_featinfo;
-
-void cobalt_arch_check_features(struct cobalt_featinfo *finfo);
-
 extern struct sigaction __cobalt_orig_sigdebug;
 
 extern int __cobalt_std_fifo_minpri,
@@ -98,4 +94,39 @@ extern int __cobalt_std_fifo_minpri,
 extern int __cobalt_std_rr_minpri,
 	   __cobalt_std_rr_maxpri;
 
+extern unsigned int cobalt_features;
+
+struct cobalt_featinfo;
+
+/**
+ * Arch specific feature initialization
+ *
+ * @param finfo
+ */
+void cobalt_arch_check_features(struct cobalt_featinfo *finfo);
+
+/**
+ * Initialize the feature handling.
+ *
+ * @param f Feature info that will be cached for future feature checks
+ */
+void cobalt_features_init(struct cobalt_featinfo *f)
+	__attribute__((visibility("hidden")));
+
+/**
+ * Check if a given set of features is available / provided by the kernel
+ *
+ * @param feat_mask A bit mask of features to check availability for. See
+ * __xn_feat_* macros for a list of defined features
+ *
+ * @return 1 if all features are available, 0 otherwise
+ */
+inline int cobalt_features_available(unsigned int feat_mask)
+{
+	if ((cobalt_features & feat_mask) == feat_mask)
+		return 1;
+
+	return 0;
+}
+
 #endif /* _LIB_COBALT_INTERNAL_H */
-- 
2.26.2

Re: [PATCH 1/3] cobalt uapi: Introducing new feature flag for settime64 availability

[Jan Kiszka]

On 13.11.20 13:00, florian.bezdeka--- via Xenomai wrote:
> From: Florian Bezdeka <florian.bezdeka@siemens.com>
> 
> Adding a new feature flag to allow the library asking for settime64
> support. That will allow the library to use the new system call if it
> is available / supported by the kernel.
> 

As proposed in the other thread, we should state that this flag is
supposed to be dropped on the next major ABI revision update.

> Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
> ---
>  include/cobalt/uapi/asm-generic/features.h | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/include/cobalt/uapi/asm-generic/features.h b/include/cobalt/uapi/asm-generic/features.h
> index 8a4927c49..93a96670a 100644
> --- a/include/cobalt/uapi/asm-generic/features.h
> +++ b/include/cobalt/uapi/asm-generic/features.h
> @@ -51,6 +51,7 @@ struct cobalt_featinfo {
>  #define __xn_feat_nofastsynch 0x10000000
>  #define __xn_feat_control     0x08000000
>  #define __xn_feat_prioceiling 0x04000000
> +#define __xn_feat_settime64   0x02000000
>  
>  #ifdef CONFIG_SMP
>  #define __xn_feat_smp_mask __xn_feat_smp
> @@ -70,7 +71,8 @@ struct cobalt_featinfo {
>  #define __xn_feat_generic_mask			\
>  	(__xn_feat_smp_mask		|	\
>  	 __xn_feat_fastsynch_mask 	|	\
> -	 __xn_feat_prioceiling)
> +	 __xn_feat_prioceiling		|	\
> +	 __xn_feat_settime64 )
>  
>  /*
>   * List of features both sides have to agree on: If userland supports
> @@ -101,6 +103,8 @@ const char *get_generic_feature_label(unsigned int feature)
>  		return "control";
>  	case __xn_feat_prioceiling:
>  		return "prioceiling";
> +	case __xn_feat_settime64:
> +		return "settime64";
>  	default:
>  		return 0;
>  	}
> 

Jan
-- 
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux

Re: [PATCH 3/3] lib/cobalt: Introduce generic feature initialization and check API

[Jan Kiszka]

On 13.11.20 13:00, florian.bezdeka--- via Xenomai wrote:
> From: Florian Bezdeka <florian.bezdeka@siemens.com>
> 
> The feature initialization was arch specific up to now and the
> available features (= features offered by the currently running kernel)
> were no longer accessible once the bind syscall finished.
> 
> This patch introduces a simple API for fetching the offered features
> during runtime.
> 
> Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
> ---
>  lib/cobalt/init.c     |  2 +-
>  lib/cobalt/internal.c | 10 ++++++++++
>  lib/cobalt/internal.h | 39 +++++++++++++++++++++++++++++++++++----
>  3 files changed, 46 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/cobalt/init.c b/lib/cobalt/init.c
> index 6907ace36..20e28ccb0 100644
> --- a/lib/cobalt/init.c
> +++ b/lib/cobalt/init.c
> @@ -177,7 +177,7 @@ static void low_init(void)
>  		early_panic("mlockall: %s", strerror(errno));
>  
>  	trace_me("memory locked");
> -	cobalt_arch_check_features(f);
> +	cobalt_features_init(f);
>  	cobalt_init_umm(f->vdso_offset);
>  	trace_me("memory heaps mapped");
>  	cobalt_init_current_keys();
> diff --git a/lib/cobalt/internal.c b/lib/cobalt/internal.c
> index 43330060a..303d86f99 100644
> --- a/lib/cobalt/internal.c
> +++ b/lib/cobalt/internal.c
> @@ -565,3 +565,13 @@ void cobalt_assert_nrt(void)
>  	if (cobalt_should_warn())
>  		pthread_kill(pthread_self(), SIGDEBUG);
>  }
> +
> +unsigned int cobalt_features;
> +
> +void cobalt_features_init(struct cobalt_featinfo *f)
> +{
> +	cobalt_features = f->feat_all;
> +
> +	/* Trigger arch specific feature initialization */
> +	cobalt_arch_check_features(f);
> +}
> \ No newline at end of file
> diff --git a/lib/cobalt/internal.h b/lib/cobalt/internal.h
> index 4ca99d927..ac7ce24f3 100644
> --- a/lib/cobalt/internal.h
> +++ b/lib/cobalt/internal.h
> @@ -86,10 +86,6 @@ int cobalt_xlate_schedparam(int policy,
>  			    struct sched_param *param);
>  int cobalt_init(void);
>  
> -struct cobalt_featinfo;
> -
> -void cobalt_arch_check_features(struct cobalt_featinfo *finfo);
> -
>  extern struct sigaction __cobalt_orig_sigdebug;
>  
>  extern int __cobalt_std_fifo_minpri,
> @@ -98,4 +94,39 @@ extern int __cobalt_std_fifo_minpri,
>  extern int __cobalt_std_rr_minpri,
>  	   __cobalt_std_rr_maxpri;
>  
> +extern unsigned int cobalt_features;
> +
> +struct cobalt_featinfo;
> +
> +/**
> + * Arch specific feature initialization
> + *
> + * @param finfo
> + */
> +void cobalt_arch_check_features(struct cobalt_featinfo *finfo);
> +
> +/**
> + * Initialize the feature handling.
> + *
> + * @param f Feature info that will be cached for future feature checks
> + */
> +void cobalt_features_init(struct cobalt_featinfo *f)
> +	__attribute__((visibility("hidden")));

What will "hidden" do here?

> +
> +/**
> + * Check if a given set of features is available / provided by the kernel
> + *
> + * @param feat_mask A bit mask of features to check availability for. See
> + * __xn_feat_* macros for a list of defined features
> + *
> + * @return 1 if all features are available, 0 otherwise
> + */
> +inline int cobalt_features_available(unsigned int feat_mask)

Why not "static"?

And "bool" would be a better return type.

> +{
> +	if ((cobalt_features & feat_mask) == feat_mask)
> +		return 1;
> +
> +	return 0;

return ((cobalt_features & feat_mask) == feat_mask);

> +}
> +
>  #endif /* _LIB_COBALT_INTERNAL_H */
> 

Jan
-- 
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux

Re: [PATCH 3/3] lib/cobalt: Introduce generic feature initialization and check API

[florian.bezdeka]

On Fri, 2020-11-13 at 14:25 +0100, Jan Kiszka wrote:
> On 13.11.20 13:00, florian.bezdeka--- via Xenomai wrote:
> > From: Florian Bezdeka <florian.bezdeka@siemens.com>
> > 
> > The feature initialization was arch specific up to now and the
> > available features (= features offered by the currently running
> > kernel)
> > were no longer accessible once the bind syscall finished.
> > 
> > This patch introduces a simple API for fetching the offered
> > features
> > during runtime.
> > 
> > Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
> > ---
> >  lib/cobalt/init.c     |  2 +-
> >  lib/cobalt/internal.c | 10 ++++++++++
> >  lib/cobalt/internal.h | 39 +++++++++++++++++++++++++++++++++++----
> >  3 files changed, 46 insertions(+), 5 deletions(-)
> > 
> > diff --git a/lib/cobalt/init.c b/lib/cobalt/init.c
> > index 6907ace36..20e28ccb0 100644
> > --- a/lib/cobalt/init.c
> > +++ b/lib/cobalt/init.c
> > @@ -177,7 +177,7 @@ static void low_init(void)
> >  		early_panic("mlockall: %s", strerror(errno));
> >  
> >  	trace_me("memory locked");
> > -	cobalt_arch_check_features(f);
> > +	cobalt_features_init(f);
> >  	cobalt_init_umm(f->vdso_offset);
> >  	trace_me("memory heaps mapped");
> >  	cobalt_init_current_keys();
> > diff --git a/lib/cobalt/internal.c b/lib/cobalt/internal.c
> > index 43330060a..303d86f99 100644
> > --- a/lib/cobalt/internal.c
> > +++ b/lib/cobalt/internal.c
> > @@ -565,3 +565,13 @@ void cobalt_assert_nrt(void)
> >  	if (cobalt_should_warn())
> >  		pthread_kill(pthread_self(), SIGDEBUG);
> >  }
> > +
> > +unsigned int cobalt_features;
> > +
> > +void cobalt_features_init(struct cobalt_featinfo *f)
> > +{
> > +	cobalt_features = f->feat_all;
> > +
> > +	/* Trigger arch specific feature initialization */
> > +	cobalt_arch_check_features(f);
> > +}
> > \ No newline at end of file
> > diff --git a/lib/cobalt/internal.h b/lib/cobalt/internal.h
> > index 4ca99d927..ac7ce24f3 100644
> > --- a/lib/cobalt/internal.h
> > +++ b/lib/cobalt/internal.h
> > @@ -86,10 +86,6 @@ int cobalt_xlate_schedparam(int policy,
> >  			    struct sched_param *param);
> >  int cobalt_init(void);
> >  
> > -struct cobalt_featinfo;
> > -
> > -void cobalt_arch_check_features(struct cobalt_featinfo *finfo);
> > -
> >  extern struct sigaction __cobalt_orig_sigdebug;
> >  
> >  extern int __cobalt_std_fifo_minpri,
> > @@ -98,4 +94,39 @@ extern int __cobalt_std_fifo_minpri,
> >  extern int __cobalt_std_rr_minpri,
> >  	   __cobalt_std_rr_maxpri;
> >  
> > +extern unsigned int cobalt_features;
> > +
> > +struct cobalt_featinfo;
> > +
> > +/**
> > + * Arch specific feature initialization
> > + *
> > + * @param finfo
> > + */
> > +void cobalt_arch_check_features(struct cobalt_featinfo *finfo);
> > +
> > +/**
> > + * Initialize the feature handling.
> > + *
> > + * @param f Feature info that will be cached for future feature
> > checks
> > + */
> > +void cobalt_features_init(struct cobalt_featinfo *f)
> > +	__attribute__((visibility("hidden")));
> 
> What will "hidden" do here?

I will remove it. It was added by accident.
It would prevent the function from being added to the linking
interface. As a result it would not be possible to use this function
from outside the library. 

Reducing the exported symbols is a follow up topic if relevant at all.

> 
> > +
> > +/**
> > + * Check if a given set of features is available / provided by the
> > kernel
> > + *
> > + * @param feat_mask A bit mask of features to check availability
> > for. See
> > + * __xn_feat_* macros for a list of defined features
> > + *
> > + * @return 1 if all features are available, 0 otherwise
> > + */
> > +inline int cobalt_features_available(unsigned int feat_mask)
> 
> Why not "static"?
> 
> And "bool" would be a better return type.

Right. The first implementation was bool, but bool seems to be rarely
used in the codebase. Will take that into v2 as well as the missing
"static".

> 
> > +{
> > +	if ((cobalt_features & feat_mask) == feat_mask)
> > +		return 1;
> > +
> > +	return 0;
> 
> return ((cobalt_features & feat_mask) == feat_mask);
> 
> > +}
> > +
> >  #endif /* _LIB_COBALT_INTERNAL_H */
> > 
> 
> Jan

Re: [PATCH 1/3] cobalt uapi: Introducing new feature flag for settime64 availability

[florian.bezdeka]

On Fri, 2020-11-13 at 14:25 +0100, Jan Kiszka wrote:
> On 13.11.20 13:00, florian.bezdeka--- via Xenomai wrote:
> > From: Florian Bezdeka <florian.bezdeka@siemens.com>
> > 
> > Adding a new feature flag to allow the library asking for settime64
> > support. That will allow the library to use the new system call if
> > it
> > is available / supported by the kernel.
> > 
> 
> As proposed in the other thread, we should state that this flag is
> supposed to be dropped on the next major ABI revision update.

I agree. The ABI revision of all supported architectures has to be
incremented when removing this flag. 

How do we make sure that the necessary steps are taken once the next
major release process starts? Is there some kind of "next major branch"
available where we could apply patches to after settime64 stuff was
merged to "next"?

> 
> > Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
> > ---
> >  include/cobalt/uapi/asm-generic/features.h | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/include/cobalt/uapi/asm-generic/features.h
> > b/include/cobalt/uapi/asm-generic/features.h
> > index 8a4927c49..93a96670a 100644
> > --- a/include/cobalt/uapi/asm-generic/features.h
> > +++ b/include/cobalt/uapi/asm-generic/features.h
> > @@ -51,6 +51,7 @@ struct cobalt_featinfo {
> >  #define __xn_feat_nofastsynch 0x10000000
> >  #define __xn_feat_control     0x08000000
> >  #define __xn_feat_prioceiling 0x04000000
> > +#define __xn_feat_settime64   0x02000000
> >  
> >  #ifdef CONFIG_SMP
> >  #define __xn_feat_smp_mask __xn_feat_smp
> > @@ -70,7 +71,8 @@ struct cobalt_featinfo {
> >  #define __xn_feat_generic_mask			\
> >  	(__xn_feat_smp_mask		|	\
> >  	 __xn_feat_fastsynch_mask 	|	\
> > -	 __xn_feat_prioceiling)
> > +	 __xn_feat_prioceiling		|	\
> > +	 __xn_feat_settime64 )
> >  
> >  /*
> >   * List of features both sides have to agree on: If userland
> > supports
> > @@ -101,6 +103,8 @@ const char *get_generic_feature_label(unsigned
> > int feature)
> >  		return "control";
> >  	case __xn_feat_prioceiling:
> >  		return "prioceiling";
> > +	case __xn_feat_settime64:
> > +		return "settime64";
> >  	default:
> >  		return 0;
> >  	}
> > 
> 
> Jan

Re: [PATCH 1/3] cobalt uapi: Introducing new feature flag for settime64 availability

[song]

On 2020/11/13 下午8:00, florian.bezdeka@siemens.com wrote:
> From: Florian Bezdeka <florian.bezdeka@siemens.com>
> 
> Adding a new feature flag to allow the library asking for settime64
> support. That will allow the library to use the new system call if it
> is available / supported by the kernel.
> 
> Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
> ---
>   include/cobalt/uapi/asm-generic/features.h | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/include/cobalt/uapi/asm-generic/features.h b/include/cobalt/uapi/asm-generic/features.h
> index 8a4927c49..93a96670a 100644
> --- a/include/cobalt/uapi/asm-generic/features.h
> +++ b/include/cobalt/uapi/asm-generic/features.h
> @@ -51,6 +51,7 @@ struct cobalt_featinfo {
>   #define __xn_feat_nofastsynch 0x10000000
>   #define __xn_feat_control     0x08000000
>   #define __xn_feat_prioceiling 0x04000000
> +#define __xn_feat_settime64   0x02000000
>   
>   #ifdef CONFIG_SMP
>   #define __xn_feat_smp_mask __xn_feat_smp
> @@ -70,7 +71,8 @@ struct cobalt_featinfo {
>   #define __xn_feat_generic_mask			\
>   	(__xn_feat_smp_mask		|	\
>   	 __xn_feat_fastsynch_mask 	|	\
> -	 __xn_feat_prioceiling)
> +	 __xn_feat_prioceiling		|	\
> +	 __xn_feat_settime64 )
>   
>   /*
>    * List of features both sides have to agree on: If userland supports
> @@ -101,6 +103,8 @@ const char *get_generic_feature_label(unsigned int feature)
>   		return "control";
>   	case __xn_feat_prioceiling:
>   		return "prioceiling";
> +	case __xn_feat_settime64:
> +		return "settime64";
>   	default:
>   		return 0;
>   	}
> 

If i understand it correctly, we negotiate xn_feature instead of abi 
revision? In lib/cobalt/clock.c?

Another question is settime64 is the first step of y2038, clock_gettime 
and others will come up, shall we have xn_feat_time64 for all of them?

chensong

Re: [PATCH 5/5] lib/cobalt/clock.c:dispatch clock_settime

[song]

On 2020/11/13 下午7:47, Florian Bezdeka wrote:
> Hi all,
> 
> I reviewed all the timeset64 related patches.
> Please note the comments below.
> 
> Did anyone look into testing yet? There are some combinations of kernel
> and library versions we should look at and make sure they all behave as
> expected. Maybe xenomai-images "project" could help here?

That would be great. y2038 is not as difficult as other realtime part, 
but it will bring a lot of compatible problems if we're not cautious enough.


> 
> On 11.11.20 11:13, chensong via Xenomai wrote:
>>
>>
>> On 2020年11月11日 17:43, Jan Kiszka wrote:
>>> On 11.11.20 10:23, chensong wrote:
>>>>
>>>>
>>>> On 2020年11月11日 15:29, Jan Kiszka wrote:
>>>>> On 11.11.20 04:55, chensong wrote:
>>>>>>
>>>>>>
>>>>>> On 2020年11月10日 18:24, Jan Kiszka wrote:
>>>>>>>
>>>>>>> On 03.11.20 04:05, chensong wrote:
>>>>>>>> Regarding sizeof time_t, dispatch 32bit timespec to clock_settime
>>>>>>>> and 64bit timespec to clock_settime64.
>>>>>>>>
>>>>>>>> Signed-off-by: chensong <chensong@tj.kylinos.cn>
>>>>>>>> ---
>>>>>>>>      lib/cobalt/clock.c | 6 +++++-
>>>>>>>>      1 file changed, 5 insertions(+), 1 deletion(-)
>>>>>>>>
>>>>>>>> diff --git a/lib/cobalt/clock.c b/lib/cobalt/clock.c
>>>>>>>> index 11fd1aa..44b2f3f 100644
>>>>>>>> --- a/lib/cobalt/clock.c
>>>>>>>> +++ b/lib/cobalt/clock.c
>>>>>>>> @@ -237,7 +237,11 @@ COBALT_IMPL(int, clock_settime, (clockid_t
>>>>>>>> clock_id, const struct timespec *tp))
>>>>>>>>      {
>>>>>>>>          int ret;
>>>>>>>>
>>>>>>>> -    ret = -XENOMAI_SYSCALL2(sc_cobalt_clock_settime, clock_id, tp);
>>>>>>>> +    if (sizeof(time_t) > 4)
>>>>>>>> +        ret = -XENOMAI_SYSCALL2(sc_cobalt_clock_settime64,
>>>>>>>> +                    clock_id, tp);
>>>>>>>> +    else
>>>>>>>> +        ret = -XENOMAI_SYSCALL2(sc_cobalt_clock_settime, clock_id,
>>>>>>>> tp);
>>>>>>>
>>>>>>> Maybe do a single
>>>>>>>
>>>>>>> XENOMAI_SYSCALL2(sizeof(time_t) > 4 ? sc_cobalt_clock_settime64
>>>>>>>                                         : sc_cobalt_clock_settime,
>>>>>>> ...)
>>>>>>>
>>>>>>> But we need ABI revision negotiation here. If a new lib is run on a
>>>>>>> kernel that does not have settime64, things will fail (the other way
>>>>>>> around is fine). We need to ensure that new userspace can check for
>>>>>>> this
>>>>>>> upfront while old userspace will happily run (hard requirement for
>>>>>>> making these patches part of 3.1.x).
>>>>>>>
>>>>>>> Jan
>>>>>>
>>>>>> As far as my understanding, ABI revision negotiation happens in
>>>>>> low_init
>>>>>> already, it reminds userspace process that lib and kernel are not
>>>>>> compatible at the first place. Therefore, it addresses the scenario
>>>>>> you
>>>>>> are talking about(new userspace, old kernel or the other way around).
>>>>>>
>>>>>
>>>>> Ah, right. Perfect. Then also update those revisions make detection
>>>>> work
>>>>> as required.
>>>>>
>>>> ok, will do.
>>>>
>>>> XENOMAI_ABI_REV is defined in those files:
>>>>
>>>> ./kernel/cobalt/arch/arm/include/asm/xenomai/uapi/features.h:25:#define
>>>> XENOMAI_ABI_REV   17UL
>>>> ./kernel/cobalt/arch/x86/include/asm/xenomai/uapi/features.h:22:#define
>>>> XENOMAI_ABI_REV   17UL
>>>> ./kernel/cobalt/arch/arm64/include/asm/xenomai/uapi/features.h:25:#define
>>>> XENOMAI_ABI_REV
>>>>     1UL
>>>> ./kernel/cobalt/arch/powerpc/include/asm/xenomai/uapi/features.h:22:#define
>>>>
>>>> XENOMAI_ABI_REV   17UL
>>>>
>>>>
>>>> increase them to 18UL?
>>>>
>>>> if you agree, i will submit a v2 for this one.
>>>
>>> We need to increase, yes, but we need to keep previous revision userland
>>> working.
> 
> Do we really need to increase the ABI revision?
Agree, we don't have to.
Regarding the case, old app old lib and new kernel, my understanding is 
when old app calls clock_settime, old lib will lead it to 
sc_cobalt_clock_settime(24), nothing changes, as a result, nothing broke.

I will give it a try.

> My understanding is, that once the kernel has been updated applications
> using an old library will be locked out. I guess this is not allowed
> when bumping the xenomai minor revision only.
> 
> I will come up with a small patch set that allows some feature detection
> during runtime. That should help with the identification which syscall
> should be used.
> 
> Would be glad if you both could have a look at it.
> 
>>
>> low_init will stop previous revision userland working once we increase it.
>>
>>>
>>>>
>>>>>> what's more, do we or customers release lib without kernel. About
>>>>>> y2038,
>>>>>> vanilla kernel, glibc, libcobalt, xenomai, too many scenarios to be
>>>>>> addressed, to complicated, we'd better simplify it if possible.
>>>>>
>>>>> Xenomai does not issue separate releases for the kernel part
>>>>> implementing the syscalls and for the xenomai libs using them.
>>>>>
>>>>> However, applications may have been bundled with libs from previous
>>>>> releases of *the same stable series*, and users can expect (bugs aside,
>>>>> new features excluded) that their xenomai lib from, say, 3.1 will still
>>>>> work with a core from 3.1.1.
>> if XENOMAI_ABI_REV in lib is different with XENOMAI_ABI_REV in kernel,
>> low_init will stop it running at the first place.
>>
>> Even if it doesn't, old application, old lib, will go to clock_settime,
>> i can't see any impact here.
>>
>> chensong
>>
>>>>>
>>>>> Jan
>>>>>
>>>>
>>>> In those cases, they will get "ABI mismatch, required xxx, provided
>>>> xxx", user will realize they need new kernel with new lib.
>>>>
>>>> My point is, ignore those too old things. arnd Bergmann dropped
>>>> something as well.
>>>>
>>>
>>> We can't break the scenario described above. That's in fact in line with
>>> what kernel and libc do, just that they also account for newer libc
>>> running on older kernels (while we deny that).
>>>
>>> Jan
>>>
>>
>>
>>
>>
> 
> 

Re: [PATCH 1/3] cobalt uapi: Introducing new feature flag for settime64 availability

[Jan Kiszka]

On 14.11.20 07:57, song via Xenomai wrote:
> 
> 
> On 2020/11/13 下午8:00, florian.bezdeka@siemens.com wrote:
>> From: Florian Bezdeka <florian.bezdeka@siemens.com>
>>
>> Adding a new feature flag to allow the library asking for settime64
>> support. That will allow the library to use the new system call if it
>> is available / supported by the kernel.
>>
>> Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
>> ---
>>   include/cobalt/uapi/asm-generic/features.h | 6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/cobalt/uapi/asm-generic/features.h
>> b/include/cobalt/uapi/asm-generic/features.h
>> index 8a4927c49..93a96670a 100644
>> --- a/include/cobalt/uapi/asm-generic/features.h
>> +++ b/include/cobalt/uapi/asm-generic/features.h
>> @@ -51,6 +51,7 @@ struct cobalt_featinfo {
>>   #define __xn_feat_nofastsynch 0x10000000
>>   #define __xn_feat_control     0x08000000
>>   #define __xn_feat_prioceiling 0x04000000
>> +#define __xn_feat_settime64   0x02000000
>>     #ifdef CONFIG_SMP
>>   #define __xn_feat_smp_mask __xn_feat_smp
>> @@ -70,7 +71,8 @@ struct cobalt_featinfo {
>>   #define __xn_feat_generic_mask            \
>>       (__xn_feat_smp_mask        |    \
>>        __xn_feat_fastsynch_mask     |    \
>> -     __xn_feat_prioceiling)
>> +     __xn_feat_prioceiling        |    \
>> +     __xn_feat_settime64 )
>>     /*
>>    * List of features both sides have to agree on: If userland supports
>> @@ -101,6 +103,8 @@ const char *get_generic_feature_label(unsigned int
>> feature)
>>           return "control";
>>       case __xn_feat_prioceiling:
>>           return "prioceiling";
>> +    case __xn_feat_settime64:
>> +        return "settime64";
>>       default:
>>           return 0;
>>       }
>>
> 
> If i understand it correctly, we negotiate xn_feature instead of abi
> revision? In lib/cobalt/clock.c?
> 

Yes. New userspace will mandate the feature, old will not require it.
Kernel can handle both cases, passively.

> Another question is settime64 is the first step of y2038, clock_gettime
> and others will come up, shall we have xn_feat_time64 for all of them?

Yes, good point.

Jan
-- 
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux

Re: [PATCH 1/3] cobalt uapi: Introducing new feature flag for settime64 availability

[Jan Kiszka]

On 13.11.20 17:18, Bezdeka, Florian (T RDA IOT SES-DE) wrote:
> On Fri, 2020-11-13 at 14:25 +0100, Jan Kiszka wrote:
>> On 13.11.20 13:00, florian.bezdeka--- via Xenomai wrote:
>>> From: Florian Bezdeka <florian.bezdeka@siemens.com>
>>>
>>> Adding a new feature flag to allow the library asking for settime64
>>> support. That will allow the library to use the new system call if
>>> it
>>> is available / supported by the kernel.
>>>
>>
>> As proposed in the other thread, we should state that this flag is
>> supposed to be dropped on the next major ABI revision update.
> 
> I agree. The ABI revision of all supported architectures has to be
> incremented when removing this flag.
> 
> How do we make sure that the necessary steps are taken once the next
> major release process starts? Is there some kind of "next major branch"
> available where we could apply patches to after settime64 stuff was
> merged to "next"?

next and master are currently "stable 3.1.x". There is no branch for a
next major release yet. But you could leave a comment at a line that
defines the ABI revision and, thus, needs to be touched on next bump.

Jan
-- 
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux

Re: [PATCH 1/3] cobalt uapi: Introducing new feature flag for settime64 availability

[song]

On 2020/11/14 下午3:32, Jan Kiszka wrote:
> On 14.11.20 07:57, song via Xenomai wrote:
>>
>>
>> On 2020/11/13 下午8:00, florian.bezdeka@siemens.com wrote:
>>> From: Florian Bezdeka <florian.bezdeka@siemens.com>
>>>
>>> Adding a new feature flag to allow the library asking for settime64
>>> support. That will allow the library to use the new system call if it
>>> is available / supported by the kernel.
>>>
>>> Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
>>> ---
>>>    include/cobalt/uapi/asm-generic/features.h | 6 +++++-
>>>    1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/include/cobalt/uapi/asm-generic/features.h
>>> b/include/cobalt/uapi/asm-generic/features.h
>>> index 8a4927c49..93a96670a 100644
>>> --- a/include/cobalt/uapi/asm-generic/features.h
>>> +++ b/include/cobalt/uapi/asm-generic/features.h
>>> @@ -51,6 +51,7 @@ struct cobalt_featinfo {
>>>    #define __xn_feat_nofastsynch 0x10000000
>>>    #define __xn_feat_control     0x08000000
>>>    #define __xn_feat_prioceiling 0x04000000
>>> +#define __xn_feat_settime64   0x02000000
>>>      #ifdef CONFIG_SMP
>>>    #define __xn_feat_smp_mask __xn_feat_smp
>>> @@ -70,7 +71,8 @@ struct cobalt_featinfo {
>>>    #define __xn_feat_generic_mask            \
>>>        (__xn_feat_smp_mask        |    \
>>>         __xn_feat_fastsynch_mask     |    \
>>> -     __xn_feat_prioceiling)
>>> +     __xn_feat_prioceiling        |    \
>>> +     __xn_feat_settime64 )
>>>      /*
>>>     * List of features both sides have to agree on: If userland supports
>>> @@ -101,6 +103,8 @@ const char *get_generic_feature_label(unsigned int
>>> feature)
>>>            return "control";
>>>        case __xn_feat_prioceiling:
>>>            return "prioceiling";
>>> +    case __xn_feat_settime64:
>>> +        return "settime64";
>>>        default:
>>>            return 0;
>>>        }
>>>
>>
>> If i understand it correctly, we negotiate xn_feature instead of abi
>> revision? In lib/cobalt/clock.c?
>>
> 
> Yes. New userspace will mandate the feature, old will not require it.
> Kernel can handle both cases, passively.
Ok, i will submit a new patch in lib/cobalt/clock.c after this one is 
accepted.

By the way, the other patches in my patch serial have no dependency on 
this, can they be accepted? Or any other comments?

chensong

> 
>> Another question is settime64 is the first step of y2038, clock_gettime
>> and others will come up, shall we have xn_feat_time64 for all of them?
> 
> Yes, good point.
> 
> Jan
> 

[PATCH v2 0/3] Make offered kernel features accessible during

[florian.bezdeka]

From: Florian Bezdeka <florian.bezdeka@siemens.com>

Hi all,                                                                             
                                                                                    
while reviewing the settime64 syscall patches I noticed that we do not
have any infrastructure that allows us to check which features are
offered by the currently running kernel.
                                                                                    
With this patch series I try to fix this gap. Everything is compile
time tested only so far. I'm currently trying to bring my test
infrastructure up and running.

The first two patches are not related to the time64 topic, so they might be
applied to 'next' already. The third patch should make its way into the 
time64 patch queue maintained by Chengson.

Changes from v1:
 - reordered patches
 - cobalt_features_available is now returning a bool value instead of an
   integer
 - cobalt_features_available is now marked as "inline static" instead of 
   inline only
 - Renamed feature flag from '__xn_feat_settime64' to '__xn_feat_time64'
 - The third patch now includes some "TODO"s that should remind us to
   remove the feature flag when the next ABI revision dump is done

Florian Bezdeka (3):
  lib/cobalt: Rename cobalt_check_features to cobalt_arch_check_features
  lib/cobalt: Introduce generic feature initialization and check API
  cobalt uapi: Introducing new feature flag for time64 availability

 include/cobalt/uapi/asm-generic/features.h    |  6 ++-
 .../arm64/include/asm/xenomai/uapi/features.h |  1 +
 .../include/asm/xenomai/uapi/features.h       |  1 +
 .../x86/include/asm/xenomai/uapi/features.h   |  1 +
 lib/cobalt/arch/arm/features.c                |  2 +-
 lib/cobalt/arch/arm64/features.c              |  2 +-
 lib/cobalt/arch/powerpc/features.c            |  2 +-
 lib/cobalt/arch/x86/features.c                |  2 +-
 lib/cobalt/init.c                             |  2 +-
 lib/cobalt/internal.c                         | 10 +++++
 lib/cobalt/internal.h                         | 39 +++++++++++++++++--
 11 files changed, 58 insertions(+), 10 deletions(-)

-- 
2.26.2

[PATCH v2 1/3] lib/cobalt: Rename cobalt_check_features to cobalt_arch_check_features

[florian.bezdeka]

From: Florian Bezdeka <florian.bezdeka@siemens.com>

cobalt_check_features is implemented for each architecture. As
further feature initialization will arrive the name of the function
should clarify that.

Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
 lib/cobalt/arch/arm/features.c     | 2 +-
 lib/cobalt/arch/arm64/features.c   | 2 +-
 lib/cobalt/arch/powerpc/features.c | 2 +-
 lib/cobalt/arch/x86/features.c     | 2 +-
 lib/cobalt/init.c                  | 2 +-
 lib/cobalt/internal.h              | 2 +-
 6 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/cobalt/arch/arm/features.c b/lib/cobalt/arch/arm/features.c
index edabcf25b..d7b50f73f 100644
--- a/lib/cobalt/arch/arm/features.c
+++ b/lib/cobalt/arch/arm/features.c
@@ -37,7 +37,7 @@ struct __xn_full_tscinfo __xn_tscinfo = {
 	},
 };
 
-void cobalt_check_features(struct cobalt_featinfo *finfo)
+void cobalt_arch_check_features(struct cobalt_featinfo *finfo)
 {
 	unsigned long phys_addr;
 	unsigned page_size;
diff --git a/lib/cobalt/arch/arm64/features.c b/lib/cobalt/arch/arm64/features.c
index 254c8aeae..8b85a27ea 100644
--- a/lib/cobalt/arch/arm64/features.c
+++ b/lib/cobalt/arch/arm64/features.c
@@ -37,7 +37,7 @@ struct __xn_full_tscinfo __xn_tscinfo = {
 	},
 };
 
-void cobalt_check_features(struct cobalt_featinfo *finfo)
+void cobalt_arch_check_features(struct cobalt_featinfo *finfo)
 {
 	unsigned long phys_addr;
 	unsigned page_size;
diff --git a/lib/cobalt/arch/powerpc/features.c b/lib/cobalt/arch/powerpc/features.c
index 6fc7afa64..d795a198a 100644
--- a/lib/cobalt/arch/powerpc/features.c
+++ b/lib/cobalt/arch/powerpc/features.c
@@ -19,6 +19,6 @@
 #include <asm/xenomai/syscall.h>
 #include "internal.h"
 
-void cobalt_check_features(struct cobalt_featinfo *finfo)
+void cobalt_arch_check_features(struct cobalt_featinfo *finfo)
 {
 }
diff --git a/lib/cobalt/arch/x86/features.c b/lib/cobalt/arch/x86/features.c
index f8ba4ee2c..e7e62da39 100644
--- a/lib/cobalt/arch/x86/features.c
+++ b/lib/cobalt/arch/x86/features.c
@@ -25,7 +25,7 @@
 #include <asm/xenomai/uapi/fptest.h>
 #include "internal.h"
 
-void cobalt_check_features(struct cobalt_featinfo *finfo)
+void cobalt_arch_check_features(struct cobalt_featinfo *finfo)
 {
 #if defined(__i386__) && defined(CONFIG_XENO_X86_VSYSCALL)
 	size_t n = confstr(_CS_GNU_LIBPTHREAD_VERSION, NULL, 0);
diff --git a/lib/cobalt/init.c b/lib/cobalt/init.c
index 9b506d872..6907ace36 100644
--- a/lib/cobalt/init.c
+++ b/lib/cobalt/init.c
@@ -177,7 +177,7 @@ static void low_init(void)
 		early_panic("mlockall: %s", strerror(errno));
 
 	trace_me("memory locked");
-	cobalt_check_features(f);
+	cobalt_arch_check_features(f);
 	cobalt_init_umm(f->vdso_offset);
 	trace_me("memory heaps mapped");
 	cobalt_init_current_keys();
diff --git a/lib/cobalt/internal.h b/lib/cobalt/internal.h
index c45a54d9f..4ca99d927 100644
--- a/lib/cobalt/internal.h
+++ b/lib/cobalt/internal.h
@@ -88,7 +88,7 @@ int cobalt_init(void);
 
 struct cobalt_featinfo;
 
-void cobalt_check_features(struct cobalt_featinfo *finfo);
+void cobalt_arch_check_features(struct cobalt_featinfo *finfo);
 
 extern struct sigaction __cobalt_orig_sigdebug;
 
-- 
2.26.2

[PATCH v2 2/3] lib/cobalt: Introduce generic feature initialization and check API

[florian.bezdeka]

From: Florian Bezdeka <florian.bezdeka@siemens.com>

The feature initialization was arch specific up to now and the
available features (= features offered by the currently running kernel)
were no longer accessible once the bind syscall finished.

This patch introduces a simple API for fetching the offered features
during runtime.

Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
 lib/cobalt/init.c     |  2 +-
 lib/cobalt/internal.c | 10 ++++++++++
 lib/cobalt/internal.h | 39 +++++++++++++++++++++++++++++++++++----
 3 files changed, 46 insertions(+), 5 deletions(-)

diff --git a/lib/cobalt/init.c b/lib/cobalt/init.c
index 6907ace36..20e28ccb0 100644
--- a/lib/cobalt/init.c
+++ b/lib/cobalt/init.c
@@ -177,7 +177,7 @@ static void low_init(void)
 		early_panic("mlockall: %s", strerror(errno));
 
 	trace_me("memory locked");
-	cobalt_arch_check_features(f);
+	cobalt_features_init(f);
 	cobalt_init_umm(f->vdso_offset);
 	trace_me("memory heaps mapped");
 	cobalt_init_current_keys();
diff --git a/lib/cobalt/internal.c b/lib/cobalt/internal.c
index 43330060a..303d86f99 100644
--- a/lib/cobalt/internal.c
+++ b/lib/cobalt/internal.c
@@ -565,3 +565,13 @@ void cobalt_assert_nrt(void)
 	if (cobalt_should_warn())
 		pthread_kill(pthread_self(), SIGDEBUG);
 }
+
+unsigned int cobalt_features;
+
+void cobalt_features_init(struct cobalt_featinfo *f)
+{
+	cobalt_features = f->feat_all;
+
+	/* Trigger arch specific feature initialization */
+	cobalt_arch_check_features(f);
+}
\ No newline at end of file
diff --git a/lib/cobalt/internal.h b/lib/cobalt/internal.h
index 4ca99d927..a0dff30cd 100644
--- a/lib/cobalt/internal.h
+++ b/lib/cobalt/internal.h
@@ -19,6 +19,7 @@
 #define _LIB_COBALT_INTERNAL_H
 
 #include <limits.h>
+#include <stdbool.h>
 #include <boilerplate/ancillaries.h>
 #include <cobalt/sys/cobalt.h>
 #include "current.h"
@@ -86,10 +87,6 @@ int cobalt_xlate_schedparam(int policy,
 			    struct sched_param *param);
 int cobalt_init(void);
 
-struct cobalt_featinfo;
-
-void cobalt_arch_check_features(struct cobalt_featinfo *finfo);
-
 extern struct sigaction __cobalt_orig_sigdebug;
 
 extern int __cobalt_std_fifo_minpri,
@@ -98,4 +95,38 @@ extern int __cobalt_std_fifo_minpri,
 extern int __cobalt_std_rr_minpri,
 	   __cobalt_std_rr_maxpri;
 
+extern unsigned int cobalt_features;
+
+struct cobalt_featinfo;
+
+/**
+ * Arch specific feature initialization
+ *
+ * @param finfo
+ */
+void cobalt_arch_check_features(struct cobalt_featinfo *finfo);
+
+/**
+ * Initialize the feature handling.
+ *
+ * @param f Feature info that will be cached for future feature checks
+ */
+void cobalt_features_init(struct cobalt_featinfo *f);
+
+/**
+ * Check if a given set of features is available / provided by the kernel
+ *
+ * @param feat_mask A bit mask of features to check availability for. See
+ * __xn_feat_* macros for a list of defined features
+ *
+ * @return true if all features are available, false otherwise
+ */
+static inline bool cobalt_features_available(unsigned int feat_mask)
+{
+	if ((cobalt_features & feat_mask) == feat_mask)
+		return true;
+
+	return false;
+}
+
 #endif /* _LIB_COBALT_INTERNAL_H */
-- 
2.26.2

[PATCH v2 3/3] cobalt uapi: Introducing new feature flag for time64 availability

[florian.bezdeka]

From: Florian Bezdeka <florian.bezdeka@siemens.com>

Adding a new feature flag to allow the library asking for time64
support. That will allow the library to use the new system calls when
available / supported by the kernel.

The feature flag should be removed during next ABI revision bump.

Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
 include/cobalt/uapi/asm-generic/features.h                  | 6 +++++-
 .../cobalt/arch/arm64/include/asm/xenomai/uapi/features.h   | 1 +
 .../cobalt/arch/powerpc/include/asm/xenomai/uapi/features.h | 1 +
 kernel/cobalt/arch/x86/include/asm/xenomai/uapi/features.h  | 1 +
 4 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/cobalt/uapi/asm-generic/features.h b/include/cobalt/uapi/asm-generic/features.h
index 8a4927c49..705729aae 100644
--- a/include/cobalt/uapi/asm-generic/features.h
+++ b/include/cobalt/uapi/asm-generic/features.h
@@ -51,6 +51,7 @@ struct cobalt_featinfo {
 #define __xn_feat_nofastsynch 0x10000000
 #define __xn_feat_control     0x08000000
 #define __xn_feat_prioceiling 0x04000000
+#define __xn_feat_time64      0x02000000
 
 #ifdef CONFIG_SMP
 #define __xn_feat_smp_mask __xn_feat_smp
@@ -70,7 +71,8 @@ struct cobalt_featinfo {
 #define __xn_feat_generic_mask			\
 	(__xn_feat_smp_mask		|	\
 	 __xn_feat_fastsynch_mask 	|	\
-	 __xn_feat_prioceiling)
+	 __xn_feat_prioceiling		|	\
+	 __xn_feat_time64)
 
 /*
  * List of features both sides have to agree on: If userland supports
@@ -101,6 +103,8 @@ const char *get_generic_feature_label(unsigned int feature)
 		return "control";
 	case __xn_feat_prioceiling:
 		return "prioceiling";
+	case __xn_feat_time64:
+		return "time64";
 	default:
 		return 0;
 	}
diff --git a/kernel/cobalt/arch/arm64/include/asm/xenomai/uapi/features.h b/kernel/cobalt/arch/arm64/include/asm/xenomai/uapi/features.h
index 74423f68b..f03f483f6 100644
--- a/kernel/cobalt/arch/arm64/include/asm/xenomai/uapi/features.h
+++ b/kernel/cobalt/arch/arm64/include/asm/xenomai/uapi/features.h
@@ -22,6 +22,7 @@
 #define _COBALT_ARM64_ASM_UAPI_FEATURES_H
 
 /* The ABI revision level we use on this arch. */
+// TODO: Reminder: Remove __xn_feat_time64 feature flag on next ABI_REV bump
 #define XENOMAI_ABI_REV   1UL
 
 #define XENOMAI_FEAT_DEP (__xn_feat_generic_mask)
diff --git a/kernel/cobalt/arch/powerpc/include/asm/xenomai/uapi/features.h b/kernel/cobalt/arch/powerpc/include/asm/xenomai/uapi/features.h
index f6e67d884..db6dbed24 100644
--- a/kernel/cobalt/arch/powerpc/include/asm/xenomai/uapi/features.h
+++ b/kernel/cobalt/arch/powerpc/include/asm/xenomai/uapi/features.h
@@ -19,6 +19,7 @@
 #define _COBALT_POWERPC_ASM_UAPI_FEATURES_H
 
 /* The ABI revision level we use on this arch. */
+// TODO: Reminder: Remove __xn_feat_time64 feature flag on next ABI_REV bump
 #define XENOMAI_ABI_REV   17UL
 
 #define XENOMAI_FEAT_DEP  __xn_feat_generic_mask
diff --git a/kernel/cobalt/arch/x86/include/asm/xenomai/uapi/features.h b/kernel/cobalt/arch/x86/include/asm/xenomai/uapi/features.h
index 67c7625f8..30539a5d1 100644
--- a/kernel/cobalt/arch/x86/include/asm/xenomai/uapi/features.h
+++ b/kernel/cobalt/arch/x86/include/asm/xenomai/uapi/features.h
@@ -19,6 +19,7 @@
 #define _COBALT_X86_ASM_UAPI_FEATURES_H
 
 /* The ABI revision level we use on this arch. */
+// TODO: Reminder: Remove __xn_feat_time64 feature flag on next ABI_REV bump
 #define XENOMAI_ABI_REV   17UL
 
 #define XENOMAI_FEAT_DEP  __xn_feat_generic_mask
-- 
2.26.2

Re: [PATCH v2 2/3] lib/cobalt: Introduce generic feature initialization and check API

[song]

On 2020/11/16 下午10:07, florian.bezdeka@siemens.com wrote:
> From: Florian Bezdeka <florian.bezdeka@siemens.com>
> 
> The feature initialization was arch specific up to now and the
> available features (= features offered by the currently running kernel)
> were no longer accessible once the bind syscall finished.
> 
> This patch introduces a simple API for fetching the offered features
> during runtime.
> 
> Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
> ---
>   lib/cobalt/init.c     |  2 +-
>   lib/cobalt/internal.c | 10 ++++++++++
>   lib/cobalt/internal.h | 39 +++++++++++++++++++++++++++++++++++----
>   3 files changed, 46 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/cobalt/init.c b/lib/cobalt/init.c
> index 6907ace36..20e28ccb0 100644
> --- a/lib/cobalt/init.c
> +++ b/lib/cobalt/init.c
> @@ -177,7 +177,7 @@ static void low_init(void)
>   		early_panic("mlockall: %s", strerror(errno));
>   
>   	trace_me("memory locked");
> -	cobalt_arch_check_features(f);
> +	cobalt_features_init(f);
>   	cobalt_init_umm(f->vdso_offset);
>   	trace_me("memory heaps mapped");
>   	cobalt_init_current_keys();
> diff --git a/lib/cobalt/internal.c b/lib/cobalt/internal.c
> index 43330060a..303d86f99 100644
> --- a/lib/cobalt/internal.c
> +++ b/lib/cobalt/internal.c
> @@ -565,3 +565,13 @@ void cobalt_assert_nrt(void)
>   	if (cobalt_should_warn())
>   		pthread_kill(pthread_self(), SIGDEBUG);
>   }
> +
> +unsigned int cobalt_features;
> +
> +void cobalt_features_init(struct cobalt_featinfo *f)
> +{
> +	cobalt_features = f->feat_all;
> +
> +	/* Trigger arch specific feature initialization */
> +	cobalt_arch_check_features(f);
> +}
> \ No newline at end of file
> diff --git a/lib/cobalt/internal.h b/lib/cobalt/internal.h
> index 4ca99d927..a0dff30cd 100644
> --- a/lib/cobalt/internal.h
> +++ b/lib/cobalt/internal.h
> @@ -19,6 +19,7 @@
>   #define _LIB_COBALT_INTERNAL_H
>   
>   #include <limits.h>
> +#include <stdbool.h>
>   #include <boilerplate/ancillaries.h>
>   #include <cobalt/sys/cobalt.h>
>   #include "current.h"
> @@ -86,10 +87,6 @@ int cobalt_xlate_schedparam(int policy,
>   			    struct sched_param *param);
>   int cobalt_init(void);
>   
> -struct cobalt_featinfo;
> -
> -void cobalt_arch_check_features(struct cobalt_featinfo *finfo);
> -
>   extern struct sigaction __cobalt_orig_sigdebug;
>   
>   extern int __cobalt_std_fifo_minpri,
> @@ -98,4 +95,38 @@ extern int __cobalt_std_fifo_minpri,
>   extern int __cobalt_std_rr_minpri,
>   	   __cobalt_std_rr_maxpri;
>   
> +extern unsigned int cobalt_features;
> +
> +struct cobalt_featinfo;
> +
> +/**
> + * Arch specific feature initialization
> + *
> + * @param finfo
> + */
> +void cobalt_arch_check_features(struct cobalt_featinfo *finfo);
> +
> +/**
> + * Initialize the feature handling.
> + *
> + * @param f Feature info that will be cached for future feature checks
> + */
> +void cobalt_features_init(struct cobalt_featinfo *f);
> +
> +/**
> + * Check if a given set of features is available / provided by the kernel
> + *
> + * @param feat_mask A bit mask of features to check availability for. See
> + * __xn_feat_* macros for a list of defined features
> + *
> + * @return true if all features are available, false otherwise
> + */
> +static inline bool cobalt_features_available(unsigned int feat_mask)
> +{
> +	if ((cobalt_features & feat_mask) == feat_mask)
> +		return true;
> +
> +	return false;
> +}
> +
Regarding the case of clock_settime(lib/cobalt/clock.c), we are going to 
call cobalt_features_available(__xn_feat_time64) before
"ret = -XENOMAI_SYSCALL2(sc_cobalt_clock_settime, clock_id, tp);"

right?

>   #endif /* _LIB_COBALT_INTERNAL_H */
> 

Re: [PATCH v2 2/3] lib/cobalt: Introduce generic feature initialization and check API

[florian.bezdeka]

On Tue, 2020-11-17 at 12:31 +0800, song wrote:
> 
> On 2020/11/16 下午10:07, florian.bezdeka@siemens.com wrote:
> > From: Florian Bezdeka <florian.bezdeka@siemens.com>
> > 
> > The feature initialization was arch specific up to now and the
> > available features (= features offered by the currently running kernel)
> > were no longer accessible once the bind syscall finished.
> > 
> > This patch introduces a simple API for fetching the offered features
> > during runtime.
> > 
> > Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
> > ---
> >   lib/cobalt/init.c     |  2 +-
> >   lib/cobalt/internal.c | 10 ++++++++++
> >   lib/cobalt/internal.h | 39 +++++++++++++++++++++++++++++++++++----
> >   3 files changed, 46 insertions(+), 5 deletions(-)
> > 
> > diff --git a/lib/cobalt/init.c b/lib/cobalt/init.c
> > index 6907ace36..20e28ccb0 100644
> > --- a/lib/cobalt/init.c
> > +++ b/lib/cobalt/init.c
> > @@ -177,7 +177,7 @@ static void low_init(void)
> >   		early_panic("mlockall: %s", strerror(errno));
> >   
> >   	trace_me("memory locked");
> > -	cobalt_arch_check_features(f);
> > +	cobalt_features_init(f);
> >   	cobalt_init_umm(f->vdso_offset);
> >   	trace_me("memory heaps mapped");
> >   	cobalt_init_current_keys();
> > diff --git a/lib/cobalt/internal.c b/lib/cobalt/internal.c
> > index 43330060a..303d86f99 100644
> > --- a/lib/cobalt/internal.c
> > +++ b/lib/cobalt/internal.c
> > @@ -565,3 +565,13 @@ void cobalt_assert_nrt(void)
> >   	if (cobalt_should_warn())
> >   		pthread_kill(pthread_self(), SIGDEBUG);
> >   }
> > +
> > +unsigned int cobalt_features;
> > +
> > +void cobalt_features_init(struct cobalt_featinfo *f)
> > +{
> > +	cobalt_features = f->feat_all;
> > +
> > +	/* Trigger arch specific feature initialization */
> > +	cobalt_arch_check_features(f);
> > +}
> > \ No newline at end of file
> > diff --git a/lib/cobalt/internal.h b/lib/cobalt/internal.h
> > index 4ca99d927..a0dff30cd 100644
> > --- a/lib/cobalt/internal.h
> > +++ b/lib/cobalt/internal.h
> > @@ -19,6 +19,7 @@
> >   #define _LIB_COBALT_INTERNAL_H
> >   
> >   #include <limits.h>
> > +#include <stdbool.h>
> >   #include <boilerplate/ancillaries.h>
> >   #include <cobalt/sys/cobalt.h>
> >   #include "current.h"
> > @@ -86,10 +87,6 @@ int cobalt_xlate_schedparam(int policy,
> >   			    struct sched_param *param);
> >   int cobalt_init(void);
> >   
> > -struct cobalt_featinfo;
> > -
> > -void cobalt_arch_check_features(struct cobalt_featinfo *finfo);
> > -
> >   extern struct sigaction __cobalt_orig_sigdebug;
> >   
> >   extern int __cobalt_std_fifo_minpri,
> > @@ -98,4 +95,38 @@ extern int __cobalt_std_fifo_minpri,
> >   extern int __cobalt_std_rr_minpri,
> >   	   __cobalt_std_rr_maxpri;
> >   
> > +extern unsigned int cobalt_features;
> > +
> > +struct cobalt_featinfo;
> > +
> > +/**
> > + * Arch specific feature initialization
> > + *
> > + * @param finfo
> > + */
> > +void cobalt_arch_check_features(struct cobalt_featinfo *finfo);
> > +
> > +/**
> > + * Initialize the feature handling.
> > + *
> > + * @param f Feature info that will be cached for future feature checks
> > + */
> > +void cobalt_features_init(struct cobalt_featinfo *f);
> > +
> > +/**
> > + * Check if a given set of features is available / provided by the kernel
> > + *
> > + * @param feat_mask A bit mask of features to check availability for. See
> > + * __xn_feat_* macros for a list of defined features
> > + *
> > + * @return true if all features are available, false otherwise
> > + */
> > +static inline bool cobalt_features_available(unsigned int feat_mask)
> > +{
> > +	if ((cobalt_features & feat_mask) == feat_mask)
> > +		return true;
> > +
> > +	return false;
> > +}
> > +
> Regarding the case of clock_settime(lib/cobalt/clock.c), we are going to 
> call cobalt_features_available(__xn_feat_time64) before
> "ret = -XENOMAI_SYSCALL2(sc_cobalt_clock_settime, clock_id, tp);"
> 
> right?

Right. The idea is to have something like

if (cobalt_features_available(__xn_feat_time64)
	/* Use the new syscall */
	ret = -XENOMAI_SYSCALL2(sc_cobalt_clock_settime64, ...);
else
	/* Use the old syscall */
	ret = -XENOMAI_SYSCALL2(sc_cobalt_clock_settime, ...);
> 
> >   #endif /* _LIB_COBALT_INTERNAL_H */
> > 
> 
> 

Re: [PATCH v2 2/3] lib/cobalt: Introduce generic feature initialization and check API

[song]

On 2020/11/17 下午4:29, florian.bezdeka@siemens.com wrote:
> On Tue, 2020-11-17 at 12:31 +0800, song wrote:
>>
>> On 2020/11/16 下午10:07, florian.bezdeka@siemens.com wrote:
>>> From: Florian Bezdeka <florian.bezdeka@siemens.com>
>>>
>>> The feature initialization was arch specific up to now and the
>>> available features (= features offered by the currently running kernel)
>>> were no longer accessible once the bind syscall finished.
>>>
>>> This patch introduces a simple API for fetching the offered features
>>> during runtime.
>>>
>>> Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
>>> ---
>>>    lib/cobalt/init.c     |  2 +-
>>>    lib/cobalt/internal.c | 10 ++++++++++
>>>    lib/cobalt/internal.h | 39 +++++++++++++++++++++++++++++++++++----
>>>    3 files changed, 46 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/lib/cobalt/init.c b/lib/cobalt/init.c
>>> index 6907ace36..20e28ccb0 100644
>>> --- a/lib/cobalt/init.c
>>> +++ b/lib/cobalt/init.c
>>> @@ -177,7 +177,7 @@ static void low_init(void)
>>>    		early_panic("mlockall: %s", strerror(errno));
>>>    
>>>    	trace_me("memory locked");
>>> -	cobalt_arch_check_features(f);
>>> +	cobalt_features_init(f);
>>>    	cobalt_init_umm(f->vdso_offset);
>>>    	trace_me("memory heaps mapped");
>>>    	cobalt_init_current_keys();
>>> diff --git a/lib/cobalt/internal.c b/lib/cobalt/internal.c
>>> index 43330060a..303d86f99 100644
>>> --- a/lib/cobalt/internal.c
>>> +++ b/lib/cobalt/internal.c
>>> @@ -565,3 +565,13 @@ void cobalt_assert_nrt(void)
>>>    	if (cobalt_should_warn())
>>>    		pthread_kill(pthread_self(), SIGDEBUG);
>>>    }
>>> +
>>> +unsigned int cobalt_features;
>>> +
>>> +void cobalt_features_init(struct cobalt_featinfo *f)
>>> +{
>>> +	cobalt_features = f->feat_all;
>>> +
>>> +	/* Trigger arch specific feature initialization */
>>> +	cobalt_arch_check_features(f);
>>> +}
>>> \ No newline at end of file
>>> diff --git a/lib/cobalt/internal.h b/lib/cobalt/internal.h
>>> index 4ca99d927..a0dff30cd 100644
>>> --- a/lib/cobalt/internal.h
>>> +++ b/lib/cobalt/internal.h
>>> @@ -19,6 +19,7 @@
>>>    #define _LIB_COBALT_INTERNAL_H
>>>    
>>>    #include <limits.h>
>>> +#include <stdbool.h>
>>>    #include <boilerplate/ancillaries.h>
>>>    #include <cobalt/sys/cobalt.h>
>>>    #include "current.h"
>>> @@ -86,10 +87,6 @@ int cobalt_xlate_schedparam(int policy,
>>>    			    struct sched_param *param);
>>>    int cobalt_init(void);
>>>    
>>> -struct cobalt_featinfo;
>>> -
>>> -void cobalt_arch_check_features(struct cobalt_featinfo *finfo);
>>> -
>>>    extern struct sigaction __cobalt_orig_sigdebug;
>>>    
>>>    extern int __cobalt_std_fifo_minpri,
>>> @@ -98,4 +95,38 @@ extern int __cobalt_std_fifo_minpri,
>>>    extern int __cobalt_std_rr_minpri,
>>>    	   __cobalt_std_rr_maxpri;
>>>    
>>> +extern unsigned int cobalt_features;
>>> +
>>> +struct cobalt_featinfo;
>>> +
>>> +/**
>>> + * Arch specific feature initialization
>>> + *
>>> + * @param finfo
>>> + */
>>> +void cobalt_arch_check_features(struct cobalt_featinfo *finfo);
>>> +
>>> +/**
>>> + * Initialize the feature handling.
>>> + *
>>> + * @param f Feature info that will be cached for future feature checks
>>> + */
>>> +void cobalt_features_init(struct cobalt_featinfo *f);
>>> +
>>> +/**
>>> + * Check if a given set of features is available / provided by the kernel
>>> + *
>>> + * @param feat_mask A bit mask of features to check availability for. See
>>> + * __xn_feat_* macros for a list of defined features
>>> + *
>>> + * @return true if all features are available, false otherwise
>>> + */
>>> +static inline bool cobalt_features_available(unsigned int feat_mask)
>>> +{
>>> +	if ((cobalt_features & feat_mask) == feat_mask)
>>> +		return true;
>>> +
>>> +	return false;
>>> +}
>>> +
>> Regarding the case of clock_settime(lib/cobalt/clock.c), we are going to
>> call cobalt_features_available(__xn_feat_time64) before
>> "ret = -XENOMAI_SYSCALL2(sc_cobalt_clock_settime, clock_id, tp);"
>>
>> right?
> 
> Right. The idea is to have something like
> 
> if (cobalt_features_available(__xn_feat_time64)
> 	/* Use the new syscall */
> 	ret = -XENOMAI_SYSCALL2(sc_cobalt_clock_settime64, ...);
> else
> 	/* Use the old syscall */
> 	ret = -XENOMAI_SYSCALL2(sc_cobalt_clock_settime, ...);

great, i will submit a new one after this one is accepted.
>>
>>>    #endif /* _LIB_COBALT_INTERNAL_H */
>>>
>>
>>

Re: [PATCH v2 2/3] lib/cobalt: Introduce generic feature initialization and check API

[Jan Kiszka]

On 16.11.20 15:07, florian.bezdeka--- via Xenomai wrote:
> From: Florian Bezdeka <florian.bezdeka@siemens.com>
> 
> The feature initialization was arch specific up to now and the
> available features (= features offered by the currently running kernel)
> were no longer accessible once the bind syscall finished.
> 
> This patch introduces a simple API for fetching the offered features
> during runtime.
> 
> Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
> ---
>  lib/cobalt/init.c     |  2 +-
>  lib/cobalt/internal.c | 10 ++++++++++
>  lib/cobalt/internal.h | 39 +++++++++++++++++++++++++++++++++++----
>  3 files changed, 46 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/cobalt/init.c b/lib/cobalt/init.c
> index 6907ace36..20e28ccb0 100644
> --- a/lib/cobalt/init.c
> +++ b/lib/cobalt/init.c
> @@ -177,7 +177,7 @@ static void low_init(void)
>  		early_panic("mlockall: %s", strerror(errno));
>  
>  	trace_me("memory locked");
> -	cobalt_arch_check_features(f);
> +	cobalt_features_init(f);
>  	cobalt_init_umm(f->vdso_offset);
>  	trace_me("memory heaps mapped");
>  	cobalt_init_current_keys();
> diff --git a/lib/cobalt/internal.c b/lib/cobalt/internal.c
> index 43330060a..303d86f99 100644
> --- a/lib/cobalt/internal.c
> +++ b/lib/cobalt/internal.c
> @@ -565,3 +565,13 @@ void cobalt_assert_nrt(void)
>  	if (cobalt_should_warn())
>  		pthread_kill(pthread_self(), SIGDEBUG);
>  }
> +
> +unsigned int cobalt_features;
> +
> +void cobalt_features_init(struct cobalt_featinfo *f)
> +{
> +	cobalt_features = f->feat_all;
> +
> +	/* Trigger arch specific feature initialization */
> +	cobalt_arch_check_features(f);
> +}
> \ No newline at end of file
> diff --git a/lib/cobalt/internal.h b/lib/cobalt/internal.h
> index 4ca99d927..a0dff30cd 100644
> --- a/lib/cobalt/internal.h
> +++ b/lib/cobalt/internal.h
> @@ -19,6 +19,7 @@
>  #define _LIB_COBALT_INTERNAL_H
>  
>  #include <limits.h>
> +#include <stdbool.h>
>  #include <boilerplate/ancillaries.h>
>  #include <cobalt/sys/cobalt.h>
>  #include "current.h"
> @@ -86,10 +87,6 @@ int cobalt_xlate_schedparam(int policy,
>  			    struct sched_param *param);
>  int cobalt_init(void);
>  
> -struct cobalt_featinfo;
> -
> -void cobalt_arch_check_features(struct cobalt_featinfo *finfo);
> -
>  extern struct sigaction __cobalt_orig_sigdebug;
>  
>  extern int __cobalt_std_fifo_minpri,
> @@ -98,4 +95,38 @@ extern int __cobalt_std_fifo_minpri,
>  extern int __cobalt_std_rr_minpri,
>  	   __cobalt_std_rr_maxpri;
>  
> +extern unsigned int cobalt_features;
> +
> +struct cobalt_featinfo;
> +
> +/**
> + * Arch specific feature initialization
> + *
> + * @param finfo
> + */
> +void cobalt_arch_check_features(struct cobalt_featinfo *finfo);
> +
> +/**
> + * Initialize the feature handling.
> + *
> + * @param f Feature info that will be cached for future feature checks
> + */
> +void cobalt_features_init(struct cobalt_featinfo *f);
> +
> +/**
> + * Check if a given set of features is available / provided by the kernel
> + *
> + * @param feat_mask A bit mask of features to check availability for. See
> + * __xn_feat_* macros for a list of defined features
> + *
> + * @return true if all features are available, false otherwise
> + */
> +static inline bool cobalt_features_available(unsigned int feat_mask)
> +{
> +	if ((cobalt_features & feat_mask) == feat_mask)
> +		return true;
> +
> +	return false;

Collapsed to

return (cobalt_features & feat_mask) == feat_mask;

> +}
> +
>  #endif /* _LIB_COBALT_INTERNAL_H */
> 

All 3 merged, thanks.

Jan
-- 
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux

Re: [PATCH v2 3/3] cobalt uapi: Introducing new feature flag for time64 availability

[Jan Kiszka]

On 16.11.20 15:07, florian.bezdeka--- via Xenomai wrote:
> From: Florian Bezdeka <florian.bezdeka@siemens.com>
> 
> Adding a new feature flag to allow the library asking for time64
> support. That will allow the library to use the new system calls when
> available / supported by the kernel.
> 
> The feature flag should be removed during next ABI revision bump.
> 
> Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
> ---
>  include/cobalt/uapi/asm-generic/features.h                  | 6 +++++-
>  .../cobalt/arch/arm64/include/asm/xenomai/uapi/features.h   | 1 +
>  .../cobalt/arch/powerpc/include/asm/xenomai/uapi/features.h | 1 +
>  kernel/cobalt/arch/x86/include/asm/xenomai/uapi/features.h  | 1 +
>  4 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/include/cobalt/uapi/asm-generic/features.h b/include/cobalt/uapi/asm-generic/features.h
> index 8a4927c49..705729aae 100644
> --- a/include/cobalt/uapi/asm-generic/features.h
> +++ b/include/cobalt/uapi/asm-generic/features.h
> @@ -51,6 +51,7 @@ struct cobalt_featinfo {
>  #define __xn_feat_nofastsynch 0x10000000
>  #define __xn_feat_control     0x08000000
>  #define __xn_feat_prioceiling 0x04000000
> +#define __xn_feat_time64      0x02000000
>  
>  #ifdef CONFIG_SMP
>  #define __xn_feat_smp_mask __xn_feat_smp
> @@ -70,7 +71,8 @@ struct cobalt_featinfo {
>  #define __xn_feat_generic_mask			\
>  	(__xn_feat_smp_mask		|	\
>  	 __xn_feat_fastsynch_mask 	|	\
> -	 __xn_feat_prioceiling)
> +	 __xn_feat_prioceiling		|	\
> +	 __xn_feat_time64)
>  
>  /*
>   * List of features both sides have to agree on: If userland supports
> @@ -101,6 +103,8 @@ const char *get_generic_feature_label(unsigned int feature)
>  		return "control";
>  	case __xn_feat_prioceiling:
>  		return "prioceiling";
> +	case __xn_feat_time64:
> +		return "time64";
>  	default:
>  		return 0;
>  	}
> diff --git a/kernel/cobalt/arch/arm64/include/asm/xenomai/uapi/features.h b/kernel/cobalt/arch/arm64/include/asm/xenomai/uapi/features.h
> index 74423f68b..f03f483f6 100644
> --- a/kernel/cobalt/arch/arm64/include/asm/xenomai/uapi/features.h
> +++ b/kernel/cobalt/arch/arm64/include/asm/xenomai/uapi/features.h
> @@ -22,6 +22,7 @@
>  #define _COBALT_ARM64_ASM_UAPI_FEATURES_H
>  
>  /* The ABI revision level we use on this arch. */
> +// TODO: Reminder: Remove __xn_feat_time64 feature flag on next ABI_REV bump
>  #define XENOMAI_ABI_REV   1UL
>  
>  #define XENOMAI_FEAT_DEP (__xn_feat_generic_mask)
> diff --git a/kernel/cobalt/arch/powerpc/include/asm/xenomai/uapi/features.h b/kernel/cobalt/arch/powerpc/include/asm/xenomai/uapi/features.h
> index f6e67d884..db6dbed24 100644
> --- a/kernel/cobalt/arch/powerpc/include/asm/xenomai/uapi/features.h
> +++ b/kernel/cobalt/arch/powerpc/include/asm/xenomai/uapi/features.h
> @@ -19,6 +19,7 @@
>  #define _COBALT_POWERPC_ASM_UAPI_FEATURES_H
>  
>  /* The ABI revision level we use on this arch. */
> +// TODO: Reminder: Remove __xn_feat_time64 feature flag on next ABI_REV bump
>  #define XENOMAI_ABI_REV   17UL
>  
>  #define XENOMAI_FEAT_DEP  __xn_feat_generic_mask
> diff --git a/kernel/cobalt/arch/x86/include/asm/xenomai/uapi/features.h b/kernel/cobalt/arch/x86/include/asm/xenomai/uapi/features.h
> index 67c7625f8..30539a5d1 100644
> --- a/kernel/cobalt/arch/x86/include/asm/xenomai/uapi/features.h
> +++ b/kernel/cobalt/arch/x86/include/asm/xenomai/uapi/features.h
> @@ -19,6 +19,7 @@
>  #define _COBALT_X86_ASM_UAPI_FEATURES_H
>  
>  /* The ABI revision level we use on this arch. */
> +// TODO: Reminder: Remove __xn_feat_time64 feature flag on next ABI_REV bump
>  #define XENOMAI_ABI_REV   17UL
>  
>  #define XENOMAI_FEAT_DEP  __xn_feat_generic_mask
> 

There is a larger pile of refactorings sitting in Philippe's queue that
likely require us to move to 3.2 rather sooner than later. Therefore I'm
pulling this our until we have the patches that add time32 also to 3.1.

The other changes can stay, though.

Jan

-- 
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux