Re: [Xen-devel] [PATCH 3/4] x86: provide and use hv_tsc_scale

["Durrant, Paul" <pdurrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Wei Liu <wei.liu.xen@xxxxxxxxx> On Behalf Of Wei Liu
> Sent: 20 December 2019 19:52
> To: Xen Development List <xen-devel@xxxxxxxxxxxxxxxxxxxx>
> Cc: Michael Kelley <mikelley@xxxxxxxxxxxxx>; Durrant, Paul
> <pdurrant@xxxxxxxxxx>; Wei Liu <liuwe@xxxxxxxxxxxxx>; Paul Durrant
> <paul@xxxxxxx>; Jan Beulich <jbeulich@xxxxxxxx>; Andrew Cooper
> <andrew.cooper3@xxxxxxxxxx>; Wei Liu <wl@xxxxxxx>; Roger Pau Monné
> <roger.pau@xxxxxxxxxx>
> Subject: [PATCH 3/4] x86: provide and use hv_tsc_scale
> 
> The Hyper-V clock source and Xen's own viridian code need the same
> functionality.
> 
> Move the function in viridian/time.c to hyperv.h and use it in both
> places.
> 
> No functional change.
> 
> Signed-off-by: Wei Liu <liuwe@xxxxxxxxxxxxx>

Reviewed-by: Paul Durrant <paul@xxxxxxx>

> ---
>  xen/arch/x86/hvm/viridian/time.c   | 30 ++--------------------------
>  xen/arch/x86/time.c                |  7 +------
>  xen/include/asm-x86/guest/hyperv.h | 32 ++++++++++++++++++++++++++++--
>  3 files changed, 33 insertions(+), 36 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/viridian/time.c
> b/xen/arch/x86/hvm/viridian/time.c
> index 32e79bbcc4..6b2d745f3a 100644
> --- a/xen/arch/x86/hvm/viridian/time.c
> +++ b/xen/arch/x86/hvm/viridian/time.c
> @@ -13,6 +13,7 @@
> 
>  #include <asm/apic.h>
>  #include <asm/event.h>
> +#include <asm/guest/hyperv.h>
>  #include <asm/guest/hyperv-tlfs.h>
>  #include <asm/hvm/support.h>
> 
> @@ -82,33 +83,6 @@ static void update_reference_tsc(const struct domain
> *d, bool initialize)
>      p->tsc_sequence = seq;
>  }
> 
> -/*
> - * The specification says: "The partition reference time is computed
> - * by the following formula:
> - *
> - * ReferenceTime = ((VirtualTsc * TscScale) >> 64) + TscOffset
> - *
> - * The multiplication is a 64 bit multiplication, which results in a
> - * 128 bit number which is then shifted 64 times to the right to obtain
> - * the high 64 bits."
> - */
> -static uint64_t scale_tsc(uint64_t tsc, uint64_t scale, int64_t offset)
> -{
> -    uint64_t result;
> -
> -    /*
> -     * Quadword MUL takes an implicit operand in RAX, and puts the result
> -     * in RDX:RAX. Because we only want the result of the multiplication
> -     * after shifting right by 64 bits, we therefore only need the
> content
> -     * of RDX.
> -     */
> -    asm ( "mulq %[scale]"
> -          : "+a" (tsc), "=d" (result)
> -          : [scale] "rm" (scale) );
> -
> -    return result + offset;
> -}
> -
>  static uint64_t trc_val(const struct domain *d, int64_t offset)
>  {
>      uint64_t tsc, scale;
> @@ -116,7 +90,7 @@ static uint64_t trc_val(const struct domain *d, int64_t
> offset)
>      tsc = hvm_get_guest_tsc(pt_global_vcpu_target(d));
>      scale = ((10000ul << 32) / d->arch.tsc_khz) << 32;
> 
> -    return scale_tsc(tsc, scale, offset);
> +    return hv_scale_tsc(tsc, scale, offset);
>  }
> 
>  static void time_ref_count_freeze(const struct domain *d)
> diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
> index bbcc9b10b8..d21875de9e 100644
> --- a/xen/arch/x86/time.c
> +++ b/xen/arch/x86/time.c
> @@ -725,12 +725,7 @@ static inline uint64_t read_hyperv_timer(void)
> 
>      } while ( tsc_page->tsc_sequence != seq );
> 
> -    /* ret = ((tsc * scale) >> 64) + offset; */
> -    asm ( "mul %[scale]; add %[offset], %[ret]"
> -          : "+a" (tsc), [ret] "=&d" (ret)
> -          : [scale] "rm" (scale), [offset] "rm" (offset) );
> -
> -    return ret;
> +    return hv_scale_tsc(tsc, scale, offset);
>  }
> 
>  static struct platform_timesource __initdata plt_hyperv_timer =
> diff --git a/xen/include/asm-x86/guest/hyperv.h b/xen/include/asm-
> x86/guest/hyperv.h
> index cc21b9abfc..c7a7f32bd5 100644
> --- a/xen/include/asm-x86/guest/hyperv.h
> +++ b/xen/include/asm-x86/guest/hyperv.h
> @@ -19,10 +19,38 @@
>  #ifndef __X86_GUEST_HYPERV_H__
>  #define __X86_GUEST_HYPERV_H__
> 
> -#ifdef CONFIG_HYPERV_GUEST
> -
>  #include <xen/types.h>
> 
> +/*
> + * The specification says: "The partition reference time is computed
> + * by the following formula:
> + *
> + * ReferenceTime = ((VirtualTsc * TscScale) >> 64) + TscOffset
> + *
> + * The multiplication is a 64 bit multiplication, which results in a
> + * 128 bit number which is then shifted 64 times to the right to obtain
> + * the high 64 bits."
> + */
> +static inline uint64_t hv_scale_tsc(uint64_t tsc, uint64_t scale,
> +                                    int64_t offset)
> +{
> +    uint64_t result;
> +
> +    /*
> +     * Quadword MUL takes an implicit operand in RAX, and puts the result
> +     * in RDX:RAX. Because we only want the result of the multiplication
> +     * after shifting right by 64 bits, we therefore only need the
> content
> +     * of RDX.
> +     */
> +    asm ( "mulq %[scale]"
> +          : "+a" (tsc), "=d" (result)
> +          : [scale] "rm" (scale) );
> +
> +    return result + offset;
> +}
> +
> +#ifdef CONFIG_HYPERV_GUEST
> +
>  #include <asm/guest/hypervisor.h>
> 
>  struct ms_hyperv_info {
> --
> 2.20.1


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel