Re: [kvm-unit-tests PATCH v1 1/2] lib/s390x: time: add wrapper for stckf

Thomas Huth
2022-08-30T12:27:47Z

On 30/08/2022 13.56, Nico Boehr wrote:
> Upcoming changes will do performance measurements of instructions. Since
> stck is designed to return unique values even on concurrent calls, it is
> unsuited for performance measurements. stckf should be used in this
> case.
>
> Hence, add a nice wrapper for stckf to the time library.
>
> Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
> ---
> lib/s390x/asm/time.h | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/lib/s390x/asm/time.h b/lib/s390x/asm/time.h
> index 7652a151e87a..d7c2bcb4f306 100644
> --- a/lib/s390x/asm/time.h
> +++ b/lib/s390x/asm/time.h
> @@ -14,6 +14,15 @@
> #define STCK_SHIFT_US (63 - 51)
> #define STCK_MAX ((1UL << 52) - 1)
>
> +static inline uint64_t get_clock_fast(void)
> +{
> + uint64_t clk;
> +
> + asm volatile(" stckf %0 " : : "Q"(clk) : "memory");
> +
> + return clk;
> +}

Using clk as input parameter together with memory clobbing sounds like a bad
solution to me here. The Linux kernel properly uses it as output parameter
instead:

static inline unsigned long get_tod_clock_fast(void)
{
unsigned long clk;

asm volatile("stckf %0" : "=Q" (clk) : : "cc");
return clk;
}

(see arch/s390/include/asm/timex.h in the kernel sources)

As you can see, also the "cc" should be in the clobber list here.

Thomas