Linux Virtualization - Apr 2017 - [PATCH v2 00/11] x86: xen cpuid() cleanup

Reduce special casing of xen_cpuid() by using cpu capabilities instead
of faked cpuid nodes.

This cleanup enables us remove the hypervisor specific set_cpu_features
callback as the same effect can be reached via
setup_[clear|force]_cpu_cap().

Removing the rest faked nodes from xen_cpuid() requires some more work
as the remaining cases (mwait leafs and extended topology info) have
to be handled at the consumer sides of this information.

Changes in V2:
- added several features to this scheme
- removed hypervisor specific set_cpu_features() callbacks

Cc: Alok Kataria <akataria at vmware.com> 
Cc: Thomas Gleixner <tglx at linutronix.de> 
Cc: Ingo Molnar <mingo at redhat.com> 
Cc: "H. Peter Anvin" <hpa at zytor.com> 
Cc: x86 at kernel.org 
Cc: virtualization at lists.linux-foundation.org

Juergen Gross (11):
  xen: set cpu capabilities from xen_start_kernel()
  x86/xen: don't indicate DCA support in pv domains
  x86/xen: use capabilities instead of fake cpuid values for aperf
  x86/xen: use capabilities instead of fake cpuid values for mtrr
  x86/xen: use capabilities instead of fake cpuid values for acc
  x86/xen: use capabilities instead of fake cpuid values for acpi
  x86/xen: use capabilities instead of fake cpuid values for mwait
  x86/xen: use capabilities instead of fake cpuid values for x2apic
  x86/xen: use capabilities instead of fake cpuid values for xsave
  vmware: set cpu capabilities during platform initialization
  x86/cpu: remove hypervisor specific set_cpu_features

 arch/x86/include/asm/hypervisor.h |  5 ---
 arch/x86/kernel/cpu/common.c      |  1 -
 arch/x86/kernel/cpu/hypervisor.c  |  8 ----
 arch/x86/kernel/cpu/vmware.c      | 39 ++++++++++---------
 arch/x86/xen/enlighten_pv.c       | 81 +++++++++++++++------------------------
 5 files changed, 50 insertions(+), 84 deletions(-)

-- 
2.12.0

There is no need to set the same capabilities for each cpu
individually. This can be done for all cpus in platform initialization.

Cc: Alok Kataria <akataria at vmware.com>
Cc: Thomas Gleixner <tglx at linutronix.de>
Cc: Ingo Molnar <mingo at redhat.com>
Cc: "H. Peter Anvin" <hpa at zytor.com>
Cc: x86 at kernel.org
Cc: virtualization at lists.linux-foundation.org
Signed-off-by: Juergen Gross <jgross at suse.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,
 };
-- 
2.12.0

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 at vmware.com>

Thanks,
Alok
> 
> Cc: Alok Kataria <akataria at vmware.com>
> Cc: Thomas Gleixner <tglx at linutronix.de>
> Cc: Ingo Molnar <mingo at redhat.com>
> Cc: "H. Peter Anvin" <hpa at zytor.com>
> Cc: x86 at kernel.org
> Cc: virtualization at lists.linux-foundation.org
> Signed-off-by: Juergen Gross <jgross at suse.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,
>  };

On 04/13/2017 06:11 AM, Juergen Gross wrote:
> Reduce special casing of xen_cpuid() by using cpu capabilities instead
> of faked cpuid nodes.
>
> This cleanup enables us remove the hypervisor specific set_cpu_features
> callback as the same effect can be reached via
> setup_[clear|force]_cpu_cap().
>
> Removing the rest faked nodes from xen_cpuid() requires some more work
> as the remaining cases (mwait leafs and extended topology info) have
> to be handled at the consumer sides of this information.
Reviewed-by: Boris Ostrovsky <boris.ostrovsky at oracle.com>


-boris