linux-kernel - Re: [PATCH v2 10/11] vmware: set cpu capabilities during platform initialization

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 13 Apr 2017 10:29:27 +0000
From:   Alok Kataria <akataria@...are.com>
To:     "jgross@...e.com" <jgross@...e.com>
CC:     "boris.ostrovsky@...cle.com" <boris.ostrovsky@...cle.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "virtualization@...ts.linux-foundation.org" 
        <virtualization@...ts.linux-foundation.org>,
        "x86@...nel.org" <x86@...nel.org>, "hpa@...or.com" <hpa@...or.com>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "xen-devel@...ts.xenproject.org" <xen-devel@...ts.xenproject.org>
Subject: Re: [PATCH v2 10/11] vmware: set cpu capabilities during platform
 initialization


On Thu, 2017-04-13 at 12:11 +0200, Juergen Gross wrote:
> There is no need to set the same capabilities for each cpu
> individually. This can be done for all cpus in platform initialization.

Looks reasonable to me.
Acked-by: Alok Kataria <akataria@...are.com>

Thanks,
Alok

> 
> Cc: Alok Kataria <akataria@...are.com>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: Ingo Molnar <mingo@...hat.com>
> Cc: "H. Peter Anvin" <hpa@...or.com>
> Cc: x86@...nel.org
> Cc: virtualization@...ts.linux-foundation.org
> Signed-off-by: Juergen Gross <jgross@...e.com>
> ---
>  arch/x86/kernel/cpu/vmware.c | 39 ++++++++++++++++++++-------------------
>  1 file changed, 20 insertions(+), 19 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/vmware.c b/arch/x86/kernel/cpu/vmware.c
> index 22403a28caf5..40ed26852ebd 100644
> --- a/arch/x86/kernel/cpu/vmware.c
> +++ b/arch/x86/kernel/cpu/vmware.c
> @@ -113,6 +113,24 @@ static void __init vmware_paravirt_ops_setup(void)
>  #define vmware_paravirt_ops_setup() do {} while (0)
>  #endif
>  
> +/*
> + * VMware hypervisor takes care of exporting a reliable TSC to the guest.
> + * Still, due to timing difference when running on virtual cpus, the TSC can
> + * be marked as unstable in some cases. For example, the TSC sync check at
> + * bootup can fail due to a marginal offset between vcpus' TSCs (though the
> + * TSCs do not drift from each other).  Also, the ACPI PM timer clocksource
> + * is not suitable as a watchdog when running on a hypervisor because the
> + * kernel may miss a wrap of the counter if the vcpu is descheduled for a
> + * long time. To skip these checks at runtime we set these capability bits,
> + * so that the kernel could just trust the hypervisor with providing a
> + * reliable virtual TSC that is suitable for timekeeping.
> + */
> +static void __init vmware_set_capabilities(void)
> +{
> +	setup_force_cpu_cap(X86_FEATURE_CONSTANT_TSC);
> +	setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE);
> +}
> +
>  static void __init vmware_platform_setup(void)
>  {
>  	uint32_t eax, ebx, ecx, edx;
> @@ -152,6 +170,8 @@ static void __init vmware_platform_setup(void)
>  #ifdef CONFIG_X86_IO_APIC
>  	no_timer_check = 1;
>  #endif
> +
> +	vmware_set_capabilities();
>  }
>  
>  /*
> @@ -176,24 +196,6 @@ static uint32_t __init vmware_platform(void)
>  	return 0;
>  }
>  
> -/*
> - * VMware hypervisor takes care of exporting a reliable TSC to the guest.
> - * Still, due to timing difference when running on virtual cpus, the TSC can
> - * be marked as unstable in some cases. For example, the TSC sync check at
> - * bootup can fail due to a marginal offset between vcpus' TSCs (though the
> - * TSCs do not drift from each other).  Also, the ACPI PM timer clocksource
> - * is not suitable as a watchdog when running on a hypervisor because the
> - * kernel may miss a wrap of the counter if the vcpu is descheduled for a
> - * long time. To skip these checks at runtime we set these capability bits,
> - * so that the kernel could just trust the hypervisor with providing a
> - * reliable virtual TSC that is suitable for timekeeping.
> - */
> -static void vmware_set_cpu_features(struct cpuinfo_x86 *c)
> -{
> -	set_cpu_cap(c, X86_FEATURE_CONSTANT_TSC);
> -	set_cpu_cap(c, X86_FEATURE_TSC_RELIABLE);
> -}
> -
>  /* Checks if hypervisor supports x2apic without VT-D interrupt remapping. */
>  static bool __init vmware_legacy_x2apic_available(void)
>  {
> @@ -206,7 +208,6 @@ static bool __init vmware_legacy_x2apic_available(void)
>  const __refconst struct hypervisor_x86 x86_hyper_vmware = {
>  	.name			= "VMware",
>  	.detect			= vmware_platform,
> -	.set_cpu_features	= vmware_set_cpu_features,
>  	.init_platform		= vmware_platform_setup,
>  	.x2apic_available	= vmware_legacy_x2apic_available,
>  };