Re: [Xen-devel] [PATCH v5] new config option vtsc_tolerance_khz to avoid TSC emulation

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Tue, Mar 27, 2018 at 11:26:55AM +0200, Olaf Hering wrote:
> Add an option to control when vTSC emulation will be activated for a
> domU with tsc_mode=default. Without such option each TSC access from
> domU will be emulated, which causes a significant perfomance drop for
> workloads that make use of rdtsc.

This is not always true. Intel hardware has a feature called TSC
scaling that allows to set a scaling factor in the VMCS so that TSC
values can be scaled without requiring emulation.

I'm not sure the way this newly introduced option interacts with the
hardware TSC scaling feature is correct.

IMO if hardware TSC scaling is supported vtsc_tolerance_khz should be
ignored, and the TSC should be scaled by the hardware always in order
to provide accurate values.

>      case TSC_MODE_DEFAULT:
> +        d->arch.vtsc_tolerance_khz = vtsc_tolerance_khz;
> +        /* Fallthrough. */
>      case TSC_MODE_ALWAYS_EMULATE:
>          d->arch.vtsc_offset = get_s_time() - elapsed_nsec;
>          d->arch.tsc_khz = gtsc_khz ?: cpu_khz;
> @@ -2149,8 +2154,26 @@ void tsc_set_info(struct domain *d,
>           * When a guest is created, gtsc_khz is passed in as zero, making
>           * d->arch.tsc_khz == cpu_khz. Thus no need to check incarnation.
>           */
> +        disable_vtsc = d->arch.tsc_khz == cpu_khz;
> +
> +        if ( tsc_mode == TSC_MODE_DEFAULT && gtsc_khz &&
> +             d->arch.vtsc_tolerance_khz )
> +        {
> +            uint32_t khz_diff;
> +
> +            khz_diff = cpu_khz > gtsc_khz ?
> +                       cpu_khz - gtsc_khz : gtsc_khz - cpu_khz;
> +            disable_vtsc = khz_diff <= d->arch.vtsc_tolerance_khz;
> +
> +            printk(XENLOG_G_INFO "%s: d%u: host has %lu kHz,"
> +                   " domU expects %u kHz,"
> +                   " difference of %u is %s tolerance of %u\n",
> +                   __func__, d->domain_id, cpu_khz, gtsc_khz, khz_diff,
> +                   disable_vtsc ? "within" : "outside",
> +                   d->arch.vtsc_tolerance_khz);
> +        }

AFAICT in the chunk above you will disable vtsc without checking if
the hardware supports TSC scaling, which leads to inaccurate TSC values
on hardware that could provide accurate results without the software
emulation overhead.

Roger.

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