[PATCH] util: directly query KVM for TSC scaling support

[Michal Prívozník]

On 8/4/21 7:09 PM, Daniel P. Berrangé wrote:
> We currently query the host MSRs to determine if TSC scaling is
> supported. This works OK when running privileged and can open
> the /dev/cpu/0/msr. When unprivileged we fallback to querying
> MSRs from /dev/kvm. This is incorrect because /dev/kvm only
> reports accurate info for MSRs that are valid to use from inside
> a guest.  The TSC scaling support MSR is not, thus we always end
> up reporting lack of TSC scaling when unprivileged.
> 
> The solution to this is easy, because KVM can directly report
> whether TSC scaling is available, which matches what QEMU will
> do at startup.
> 
> https://gitlab.com/libvirt/libvirt/-/issues/188

Perhaps, Closes: https://gitlab.com/... ?

> Reported-by: Roman Mohr <rmohr at redhat.com>
> Signed-off-by: Daniel P. Berrangé <berrange at redhat.com>
> ---
>  src/util/virhostcpu.c | 19 +++++++------------
>  1 file changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c
> index 7aa92ad11d..f140610b47 100644
> --- a/src/util/virhostcpu.c
> +++ b/src/util/virhostcpu.c
> @@ -1349,11 +1349,10 @@ virHostCPUGetMSR(unsigned long index,
>  virHostCPUTscInfo *
>  virHostCPUGetTscInfo(void)
>  {
> -    virHostCPUTscInfo *info;
> +    g_autofree virHostCPUTscInfo *info = g_new0(virHostCPUTscInfo, 1);
>      VIR_AUTOCLOSE kvmFd = -1;
>      VIR_AUTOCLOSE vmFd = -1;
>      VIR_AUTOCLOSE vcpuFd = -1;
> -    uint64_t msr = 0;
>      int rc;
>  
>      if ((kvmFd = open(KVM_DEVICE, O_RDONLY)) < 0) {
> @@ -1378,23 +1377,19 @@ virHostCPUGetTscInfo(void)
>                               _("Unable to probe TSC frequency"));
>          return NULL;
>      }
> -
> -    info = g_new0(virHostCPUTscInfo, 1);
> -
>      info->frequency = rc * 1000ULL;
>  
> -    if (virHostCPUGetMSR(VMX_PROCBASED_CTLS2_MSR, &msr) == 0) {
> -        /* High 32 bits of the MSR value indicate whether specific control
> -         * can be set to 1. */
> -        msr >>= 32;
> -
> -        info->scaling = virTristateBoolFromBool(!!(msr & VMX_USE_TSC_SCALING));
> +    if ((rc = ioctl(kvmFd, KVM_CHECK_EXTENSION, KVM_CAP_TSC_CONTROL)) < 0) {
> +        virReportSystemError(errno, "%s",
> +                             _("Unable to query TSC scaling support"));
> +        return NULL;
>      }
> +    info->scaling = rc ? VIR_TRISTATE_BOOL_YES : VIR_TRISTATE_BOOL_NO;
>  
>      VIR_DEBUG("Detected TSC frequency %llu Hz, scaling %s",
>                info->frequency, virTristateBoolTypeToString(info->scaling));
>  
> -    return info;
> +    return g_steal_pointer(&info);
>  }
>  
>  #else
> 

I think you can also remove defines of those VMX_ macros that are being
removed:

# define VMX_PROCBASED_CTLS2_MSR 0x48b
# define VMX_USE_TSC_SCALING (1 << 25)

Reviewed-by: Michal Privoznik <mprivozn at redhat.com>

Michal