Re: [Xen-devel] [PATCH v2 6/6] x86: implement Hyper-V clock source

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

> -----Original Message-----
> From: Wei Liu <wei.liu.xen@xxxxxxxxx> On Behalf Of Wei Liu
> Sent: 18 December 2019 14:43
> To: Xen Development List <xen-devel@xxxxxxxxxxxxxxxxxxxx>
> Cc: Michael Kelley <mikelley@xxxxxxxxxxxxx>; Durrant, Paul
> <pdurrant@xxxxxxxxxx>; Wei Liu <liuwe@xxxxxxxxxxxxx>; Jan Beulich
> <jbeulich@xxxxxxxx>; Andrew Cooper <andrew.cooper3@xxxxxxxxxx>; Wei Liu
> <wl@xxxxxxx>; Roger Pau Monné <roger.pau@xxxxxxxxxx>
> Subject: [PATCH v2 6/6] x86: implement Hyper-V clock source
> 
> Implement a clock source using Hyper-V's reference TSC page.
> 
> Signed-off-by: Wei Liu <liuwe@xxxxxxxxxxxxx>
> ---
> v2:
> 1. Address Jan's comments.
> 
> Relevant spec:
> 
> https://github.com/MicrosoftDocs/Virtualization-
> Documentation/raw/live/tlfs/Hypervisor%20Top%20Level%20Functional%20Specif
> ication%20v5.0C.pdf
> 
> Section 12.6.
> ---
>  xen/arch/x86/time.c | 101 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 101 insertions(+)
> 
> diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
> index 216169a025..8b96b2e9a5 100644
> --- a/xen/arch/x86/time.c
> +++ b/xen/arch/x86/time.c
> @@ -31,6 +31,7 @@
>  #include <asm/processor.h>
>  #include <asm/fixmap.h>
>  #include <asm/guest.h>
> +#include <asm/guest/hyperv-tlfs.h>
>  #include <asm/mc146818rtc.h>
>  #include <asm/div64.h>
>  #include <asm/acpi.h>
> @@ -644,6 +645,103 @@ static struct platform_timesource __initdata
> plt_xen_timer =
>  };
>  #endif
> 
> +#ifdef CONFIG_HYPERV_GUEST
> +/************************************************************
> + * HYPER-V REFERENCE TSC
> + */
> +
> +static struct ms_hyperv_tsc_page *hyperv_tsc;
> +static struct page_info *hyperv_tsc_page;
> +
> +static int64_t __init init_hyperv_timer(struct platform_timesource *pts)
> +{
> +    paddr_t maddr;
> +    uint64_t tsc_msr, freq;
> +
> +    if ( !(ms_hyperv.features & HV_MSR_REFERENCE_TSC_AVAILABLE) )
> +        return 0;
> +
> +    hyperv_tsc_page = alloc_domheap_page(NULL, 0);
> +    if ( !hyperv_tsc_page )
> +        return 0;
> +
> +    hyperv_tsc = __map_domain_page_global(hyperv_tsc_page);
> +    if ( !hyperv_tsc )
> +    {
> +        free_domheap_page(hyperv_tsc_page);
> +        hyperv_tsc_page = NULL;
> +        return 0;
> +    }
> +
> +    maddr = page_to_maddr(hyperv_tsc_page);
> +
> +    /*
> +     * Per Hyper-V TLFS:
> +     *   1. Read existing MSR value
> +     *   2. Preserve bits [11:1]
> +     *   3. Set bits [63:12] to be guest physical address of tsc page
> +     *   4. Set enabled bit (0)
> +     *   5. Write back new MSR value
> +     */
> +    rdmsrl(HV_X64_MSR_REFERENCE_TSC, tsc_msr);
> +    tsc_msr &= 0xffeULL;
> +    tsc_msr |=  maddr | 1 /* enabled */;
> +    wrmsrl(HV_X64_MSR_REFERENCE_TSC, tsc_msr);
> +

You need to check for the HV_X64_ACCESS_FREQUENCY_MSRS feature or you risk a 
#GP below I think.

> +    /* Get TSC frequency from Hyper-V */
> +    rdmsrl(HV_X64_MSR_TSC_FREQUENCY, freq);
> +    pts->frequency = freq;
> +
> +    return freq;
> +}
> +
> +static inline uint64_t read_hyperv_timer(void)
> +{
> +    uint64_t scale, offset, ret, tsc;
> +    uint32_t seq;
> +    const struct ms_hyperv_tsc_page *tsc_page = hyperv_tsc;
> +
> +    do {
> +        seq = tsc_page->tsc_sequence;
> +
> +        /* Seq 0 is special. It means the TSC enlightenment is not
> +         * available at the moment. The reference time can only be
> +         * obtained from the Reference Counter MSR.
> +         */
> +        if ( seq == 0 )

Older versions of the spec used to use 0xFFFFFFFF I think, although when I look 
again they seem to have been retro-actively fixed. In any case I think you 
should treat both 0xFFFFFFFF and 0 as invalid.

> +        {
> +            rdmsrl(HV_X64_MSR_TIME_REF_COUNT, ret);
> +            return ret;
> +        }
> +
> +        /* rdtsc_ordered already contains a load fence */
> +        tsc = rdtsc_ordered();
> +        scale = tsc_page->tsc_scale;
> +        offset = tsc_page->tsc_offset;
> +
> +        smp_rmb();
> +
> +    } 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) );
> +

It would be nice to common this up with scale_tsc() in viridian/time.c.

  Paul

> +    return ret;
> +}
> +
> +static struct platform_timesource __initdata plt_hyperv_timer =
> +{
> +    .id = "hyperv",
> +    .name = "HYPER-V REFERENCE TSC",
> +    .read_counter = read_hyperv_timer,
> +    .init = init_hyperv_timer,
> +    /* See TSC time source for why counter_bits is set to 63 */
> +    .counter_bits = 63,
> +};
> +#endif
> +
>  /************************************************************
>   * GENERIC PLATFORM TIMER INFRASTRUCTURE
>   */
> @@ -793,6 +891,9 @@ static u64 __init init_platform_timer(void)
>      static struct platform_timesource * __initdata plt_timers[] = {
>  #ifdef CONFIG_XEN_GUEST
>          &plt_xen_timer,
> +#endif
> +#ifdef CONFIG_HYPERV_GUEST
> +        &plt_hyperv_timer,
>  #endif
>          &plt_hpet, &plt_pmtimer, &plt_pit
>      };
> --
> 2.20.1


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