Re: [kvm-unit-tests PATCH v8 06/12] s390x: clock and delays caluculations

Pierre Morel
2020-06-08T16:16:56Z

On 2020-06-08 17:55, Thomas Huth wrote:
> On 08/06/2020 10.12, Pierre Morel wrote:
>> The hardware gives us a good definition of the microsecond,
>> let's keep this information and let the routine accessing
>> the hardware keep all the information and return microseconds.
>>
>> Calculate delays in microseconds and take care about wrapping
>> around zero.
>>
>> Define values with macros and use inlines to keep the
>> milliseconds interface.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>> lib/s390x/asm/time.h | 29 +++++++++++++++++++++++++++--
>> 1 file changed, 27 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/s390x/asm/time.h b/lib/s390x/asm/time.h
>> index 1791380..eb15941 100644
>> --- a/lib/s390x/asm/time.h
>> +++ b/lib/s390x/asm/time.h
>> @@ -13,14 +13,39 @@
>> #ifndef ASM_S390X_TIME_H
>> #define ASM_S390X_TIME_H
>>
>> -static inline uint64_t get_clock_ms(void)
>> +#define STCK_SHIFT (63 - 51)
>
> Could you maybe rather call this STCK_SHIFT_US instead, so that it is
> clear that we refer to the microsecond bit here?
>
>> +#define STCK_MAX ((1 << (STCK_SHIFT + 1)) - 1)
>
> Hmm, is that really right? Shouldn't that rather be
>
> ((1 << (51 + 1)) - 1)
>
> instead?

:) ooops, right.

>
> (you want to have a max. value for the 52 bits that count the
> microseconds, not a max value for the remainder bits, do you?)
>
>> +static inline uint64_t get_clock_us(void)
>> {
>> uint64_t clk;
>>
>> asm volatile(" stck %0 " : : "Q"(clk) : "memory");
>>
>> /* Bit 51 is incrememented each microsecond */
>> - return (clk >> (63 - 51)) / 1000;
>> + return clk >> STCK_SHIFT;
>> +}
>> +
>> +static inline void udelay(unsigned long us)
>> +{
>> + unsigned long startclk = get_clock_us();
>> + unsigned long c;
>> +
>> + do {
>> + c = get_clock_us();
>> + if (c < startclk)
>> + c += STCK_MAX;
>> + } while (c < (startclk + us));
>
> You could omit the parentheses around "startclk + us" here.

OK

Thanks,
Pierre

--
Pierre Morel
IBM Lab Boeblingen