Subject: [PATCH] x86: move vmware to hypervisor
Impact: cleanup
vmware is only required by hypervisor and having only two functions
by moving these two functions to hypervisor we get rid of 2 vmware files
Added support for CONFIG_X86_HYPERVISOR
Signed-off-by: Jaswinder Singh Rajput <[email protected]>
---
arch/x86/Kconfig | 12 ++++
arch/x86/include/asm/hypervisor.h | 34 ++++++++++-
arch/x86/include/asm/vmware.h | 27 ---------
arch/x86/kernel/cpu/Makefile | 2 +-
arch/x86/kernel/cpu/hypervisor.c | 80 ++++++++++++++++++++-------
arch/x86/kernel/cpu/vmware.c | 112 -------------------------------------
6 files changed, 104 insertions(+), 163 deletions(-)
delete mode 100644 arch/x86/include/asm/vmware.h
delete mode 100644 arch/x86/kernel/cpu/vmware.c
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 9bb9286..e75e578 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -553,6 +553,18 @@ config HPET_EMULATE_RTC
def_bool y
depends on HPET_TIMER && (RTC=y || RTC=m || RTC_DRV_CMOS=m || RTC_DRV_CMOS=y)
+config X86_HYPERVISOR
+ bool
+ default n
+ prompt "X86 Hypervisor Support"
+ ---help---
+ A hypervisor, also called virtual machine monitor (VMM),
+ is a computer hardware platform virtualization software
+ that allows multiple operating systems to run on a host
+ computer concurrently
+
+ Choose Y to use VMware Hypervisor
+
# Mark as embedded because too many people got it wrong.
# The code disables itself when not needed.
config DMI
diff --git a/arch/x86/include/asm/hypervisor.h b/arch/x86/include/asm/hypervisor.h
index 369f5c5..e0b1ee0 100644
--- a/arch/x86/include/asm/hypervisor.h
+++ b/arch/x86/include/asm/hypervisor.h
@@ -17,10 +17,38 @@
* Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
*/
-#ifndef ASM_X86__HYPERVISOR_H
-#define ASM_X86__HYPERVISOR_H
+#ifndef ASM_X86_HYPERVISOR_H
+#define ASM_X86_HYPERVISOR_H
+
+#ifdef CONFIG_X86_HYPERVISOR
+
+/* VMware */
+#define CPUID_VMWARE_INFO_LEAF 0x40000000
+#define VMWARE_HYPERVISOR_MAGIC 0x564D5868
+#define VMWARE_HYPERVISOR_PORT 0x5658
+
+#define VMWARE_PORT_CMD_GETVERSION 10
+#define VMWARE_PORT_CMD_GETHZ 45
+
+#define VMWARE_PORT(cmd, eax, ebx, ecx, edx) \
+ __asm__("inl (%%dx)": \
+ "=a"(eax), "=c"(ecx), "=d"(edx), "=b"(ebx) : \
+ "0"(VMWARE_HYPERVISOR_MAGIC), \
+ "1"(VMWARE_PORT_CMD_##cmd), \
+ "2"(VMWARE_HYPERVISOR_PORT), "3"(UINT_MAX) : \
+ "memory");
extern unsigned long get_hypervisor_tsc_freq(void);
extern void init_hypervisor(struct cpuinfo_x86 *c);
-#endif
+#else /* CONFIG_X86_HYPERVISOR */
+
+static inline unsigned long get_hypervisor_tsc_freq(void)
+{
+ return 0;
+}
+
+static inline void init_hypervisor(struct cpuinfo_x86 *c) {}
+#endif /* CONFIG_X86_HYPERVISOR */
+
+#endif /* ASM_X86_HYPERVISOR_H */
diff --git a/arch/x86/include/asm/vmware.h b/arch/x86/include/asm/vmware.h
deleted file mode 100644
index c11b7e1..0000000
--- a/arch/x86/include/asm/vmware.h
+++ /dev/null
@@ -1,27 +0,0 @@
-/*
- * Copyright (C) 2008, VMware, Inc.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
- * NON INFRINGEMENT. See the GNU General Public License for more
- * details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
- *
- */
-#ifndef ASM_X86__VMWARE_H
-#define ASM_X86__VMWARE_H
-
-extern unsigned long vmware_get_tsc_khz(void);
-extern int vmware_platform(void);
-extern void vmware_set_feature_bits(struct cpuinfo_x86 *c);
-
-#endif
diff --git a/arch/x86/kernel/cpu/Makefile b/arch/x86/kernel/cpu/Makefile
index 3efcb2b..55e251e 100644
--- a/arch/x86/kernel/cpu/Makefile
+++ b/arch/x86/kernel/cpu/Makefile
@@ -9,11 +9,11 @@ endif
obj-y := intel_cacheinfo.o addon_cpuid_features.o
obj-y += proc.o capflags.o powerflags.o common.o
-obj-y += vmware.o hypervisor.o
obj-$(CONFIG_X86_32) += bugs.o cmpxchg.o
obj-$(CONFIG_X86_64) += bugs_64.o
+obj-$(CONFIG_X86_HYPERVISOR) += hypervisor.o
obj-$(CONFIG_X86_CPU_DEBUG) += cpu_debug.o
obj-$(CONFIG_CPU_SUP_INTEL) += intel.o
diff --git a/arch/x86/kernel/cpu/hypervisor.c b/arch/x86/kernel/cpu/hypervisor.c
index fb5b86a..e8b3cb2 100644
--- a/arch/x86/kernel/cpu/hypervisor.c
+++ b/arch/x86/kernel/cpu/hypervisor.c
@@ -2,6 +2,7 @@
* Common hypervisor code
*
* Copyright (C) 2008, VMware, Inc.
+ * Copyright (C) 2009, Jaswinder Singh Rajput
* Author : Alok N Kataria <[email protected]>
*
* This program is free software; you can redistribute it and/or modify
@@ -21,38 +22,77 @@
*
*/
-#include <asm/processor.h>
-#include <asm/vmware.h>
+#include <linux/dmi.h>
+
#include <asm/hypervisor.h>
+#include <asm/processor.h>
+#include <asm/div64.h>
-static inline void __cpuinit
-detect_hypervisor_vendor(struct cpuinfo_x86 *c)
+/* VMware */
+static unsigned long vmware_get_tsc_khz(void)
{
- if (vmware_platform()) {
- c->x86_hyper_vendor = X86_HYPER_VENDOR_VMWARE;
- } else {
- c->x86_hyper_vendor = X86_HYPER_VENDOR_NONE;
- }
+ uint32_t eax, ebx, ecx, edx;
+ uint64_t tsc_hz;
+
+ VMWARE_PORT(GETHZ, eax, ebx, ecx, edx);
+
+ if (ebx == UINT_MAX)
+ return 0;
+ tsc_hz = eax | (((uint64_t)ebx) << 32);
+ do_div(tsc_hz, 1000);
+ BUG_ON(tsc_hz >> 32);
+
+ return tsc_hz;
}
-unsigned long get_hypervisor_tsc_freq(void)
+static inline int __vmware_platform(void)
{
- if (boot_cpu_data.x86_hyper_vendor == X86_HYPER_VENDOR_VMWARE)
- return vmware_get_tsc_khz();
+ uint32_t eax, ebx, ecx, edx;
+
+ VMWARE_PORT(GETVERSION, eax, ebx, ecx, edx);
+
+ return eax != (uint32_t)-1 && ebx == VMWARE_HYPERVISOR_MAGIC;
+}
+
+/*
+ * While checking the dmi string infomation, just checking the product
+ * serial key should be enough, as this will always have a VMware
+ * specific string when running under VMware hypervisor.
+ */
+static int vmware_platform(void)
+{
+ if (cpu_has_hypervisor) {
+ unsigned int eax, ebx, ecx, edx;
+ char hyper_vendor_id[13];
+
+ cpuid(CPUID_VMWARE_INFO_LEAF, &eax, &ebx, &ecx, &edx);
+ memcpy(hyper_vendor_id + 0, &ebx, 4);
+ memcpy(hyper_vendor_id + 4, &ecx, 4);
+ memcpy(hyper_vendor_id + 8, &edx, 4);
+ hyper_vendor_id[12] = '\0';
+ if (!strcmp(hyper_vendor_id, "VMwareVMware"))
+ return 1;
+ } else if (dmi_available && dmi_name_in_serial("VMware") &&
+ __vmware_platform())
+ return 1;
+
return 0;
}
-static inline void __cpuinit
-hypervisor_set_feature_bits(struct cpuinfo_x86 *c)
+unsigned long get_hypervisor_tsc_freq(void)
{
- if (boot_cpu_data.x86_hyper_vendor == X86_HYPER_VENDOR_VMWARE) {
- vmware_set_feature_bits(c);
- return;
- }
+ if (boot_cpu_data.x86_hyper_vendor == X86_HYPER_VENDOR_VMWARE)
+ return vmware_get_tsc_khz();
+
+ return 0;
}
void __cpuinit init_hypervisor(struct cpuinfo_x86 *c)
{
- detect_hypervisor_vendor(c);
- hypervisor_set_feature_bits(c);
+ if (vmware_platform()) {
+ c->x86_hyper_vendor = X86_HYPER_VENDOR_VMWARE;
+ set_cpu_cap(c, X86_FEATURE_CONSTANT_TSC);
+ set_cpu_cap(c, X86_FEATURE_TSC_RELIABLE);
+ } else
+ c->x86_hyper_vendor = X86_HYPER_VENDOR_NONE;
}
diff --git a/arch/x86/kernel/cpu/vmware.c b/arch/x86/kernel/cpu/vmware.c
deleted file mode 100644
index 284c399..0000000
--- a/arch/x86/kernel/cpu/vmware.c
+++ /dev/null
@@ -1,112 +0,0 @@
-/*
- * VMware Detection code.
- *
- * Copyright (C) 2008, VMware, Inc.
- * Author : Alok N Kataria <[email protected]>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
- * NON INFRINGEMENT. See the GNU General Public License for more
- * details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
- *
- */
-
-#include <linux/dmi.h>
-#include <asm/div64.h>
-#include <asm/vmware.h>
-
-#define CPUID_VMWARE_INFO_LEAF 0x40000000
-#define VMWARE_HYPERVISOR_MAGIC 0x564D5868
-#define VMWARE_HYPERVISOR_PORT 0x5658
-
-#define VMWARE_PORT_CMD_GETVERSION 10
-#define VMWARE_PORT_CMD_GETHZ 45
-
-#define VMWARE_PORT(cmd, eax, ebx, ecx, edx) \
- __asm__("inl (%%dx)" : \
- "=a"(eax), "=c"(ecx), "=d"(edx), "=b"(ebx) : \
- "0"(VMWARE_HYPERVISOR_MAGIC), \
- "1"(VMWARE_PORT_CMD_##cmd), \
- "2"(VMWARE_HYPERVISOR_PORT), "3"(UINT_MAX) : \
- "memory");
-
-static inline int __vmware_platform(void)
-{
- uint32_t eax, ebx, ecx, edx;
- VMWARE_PORT(GETVERSION, eax, ebx, ecx, edx);
- return eax != (uint32_t)-1 && ebx == VMWARE_HYPERVISOR_MAGIC;
-}
-
-static unsigned long __vmware_get_tsc_khz(void)
-{
- uint64_t tsc_hz;
- uint32_t eax, ebx, ecx, edx;
-
- VMWARE_PORT(GETHZ, eax, ebx, ecx, edx);
-
- if (ebx == UINT_MAX)
- return 0;
- tsc_hz = eax | (((uint64_t)ebx) << 32);
- do_div(tsc_hz, 1000);
- BUG_ON(tsc_hz >> 32);
- return tsc_hz;
-}
-
-/*
- * While checking the dmi string infomation, just checking the product
- * serial key should be enough, as this will always have a VMware
- * specific string when running under VMware hypervisor.
- */
-int vmware_platform(void)
-{
- if (cpu_has_hypervisor) {
- unsigned int eax, ebx, ecx, edx;
- char hyper_vendor_id[13];
-
- cpuid(CPUID_VMWARE_INFO_LEAF, &eax, &ebx, &ecx, &edx);
- memcpy(hyper_vendor_id + 0, &ebx, 4);
- memcpy(hyper_vendor_id + 4, &ecx, 4);
- memcpy(hyper_vendor_id + 8, &edx, 4);
- hyper_vendor_id[12] = '\0';
- if (!strcmp(hyper_vendor_id, "VMwareVMware"))
- return 1;
- } else if (dmi_available && dmi_name_in_serial("VMware") &&
- __vmware_platform())
- return 1;
-
- return 0;
-}
-
-unsigned long vmware_get_tsc_khz(void)
-{
- BUG_ON(!vmware_platform());
- return __vmware_get_tsc_khz();
-}
-
-/*
- * 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.
- */
-void __cpuinit vmware_set_feature_bits(struct cpuinfo_x86 *c)
-{
- set_cpu_cap(c, X86_FEATURE_CONSTANT_TSC);
- set_cpu_cap(c, X86_FEATURE_TSC_RELIABLE);
-}
--
1.6.0.6
* Jaswinder Singh Rajput <[email protected]> wrote:
> Subject: [PATCH] x86: move vmware to hypervisor
>
> Impact: cleanup
>
> vmware is only required by hypervisor and having only two functions
> by moving these two functions to hypervisor we get rid of 2 vmware files
>
> Added support for CONFIG_X86_HYPERVISOR
>
> Signed-off-by: Jaswinder Singh Rajput <[email protected]>
> ---
> arch/x86/Kconfig | 12 ++++
> arch/x86/include/asm/hypervisor.h | 34 ++++++++++-
> arch/x86/include/asm/vmware.h | 27 ---------
> arch/x86/kernel/cpu/Makefile | 2 +-
> arch/x86/kernel/cpu/hypervisor.c | 80 ++++++++++++++++++++-------
> arch/x86/kernel/cpu/vmware.c | 112 -------------------------------------
> 6 files changed, 104 insertions(+), 163 deletions(-)
> delete mode 100644 arch/x86/include/asm/vmware.h
> delete mode 100644 arch/x86/kernel/cpu/vmware.c
I dont really like this one. KVM is a hypervisor too, and so is
Xen and lguest. VMware is one of the many types of a
hypervisors.
Ingo
On Tue, 2009-03-17 at 10:39 +0100, Ingo Molnar wrote:
> * Jaswinder Singh Rajput <[email protected]> wrote:
>
> > Subject: [PATCH] x86: move vmware to hypervisor
> >
> > Impact: cleanup
> >
> > vmware is only required by hypervisor and having only two functions
> > by moving these two functions to hypervisor we get rid of 2 vmware files
> >
> > Added support for CONFIG_X86_HYPERVISOR
> >
> > Signed-off-by: Jaswinder Singh Rajput <[email protected]>
> > ---
> > arch/x86/Kconfig | 12 ++++
> > arch/x86/include/asm/hypervisor.h | 34 ++++++++++-
> > arch/x86/include/asm/vmware.h | 27 ---------
> > arch/x86/kernel/cpu/Makefile | 2 +-
> > arch/x86/kernel/cpu/hypervisor.c | 80 ++++++++++++++++++++-------
> > arch/x86/kernel/cpu/vmware.c | 112 -------------------------------------
> > 6 files changed, 104 insertions(+), 163 deletions(-)
> > delete mode 100644 arch/x86/include/asm/vmware.h
> > delete mode 100644 arch/x86/kernel/cpu/vmware.c
>
> I dont really like this one. KVM is a hypervisor too, and so is
> Xen and lguest. VMware is one of the many types of a
> hypervisors.
>
Can we use common hypervisor for Xen, lguest, vmware, etc.
And Xen, lguest, vmware will be the sub-options.
--
JSR
Jaswinder Singh Rajput wrote:
>> I dont really like this one. KVM is a hypervisor too, and so is
>> Xen and lguest. VMware is one of the many types of a
>> hypervisors.
>
> Can we use common hypervisor for Xen, lguest, vmware, etc.
> And Xen, lguest, vmware will be the sub-options.
Obviously. We spent a lot of time breaking up the cpu stuff into common
and vendor-specific portions, we shouldn't go backwards w.r.t. hypervisors.
-hpa
--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.
On Tue, 2009-03-17 at 02:19 -0700, Jaswinder Singh Rajput wrote:
> Subject: [PATCH] x86: move vmware to hypervisor
>
> Impact: cleanup
>
> vmware is only required by hypervisor and having only two functions
> by moving these two functions to hypervisor we get rid of 2 vmware files
>
Just to add to Ingo's and HPA's views...
Jaswinder, if you go back and look at the LKML discussions that we had
regarding these patches, you would notice, this breakdown was done
intentionally. Even if individual hypervisors implement things
differently, we still have a common way for the kernel to interact with
them, which is through the interface that hypervisor.c exports.
--
Alok
> Added support for CONFIG_X86_HYPERVISOR
>
> Signed-off-by: Jaswinder Singh Rajput <[email protected]>
> ---
> arch/x86/Kconfig | 12 ++++
> arch/x86/include/asm/hypervisor.h | 34 ++++++++++-
> arch/x86/include/asm/vmware.h | 27 ---------
> arch/x86/kernel/cpu/Makefile | 2 +-
> arch/x86/kernel/cpu/hypervisor.c | 80 ++++++++++++++++++++-------
> arch/x86/kernel/cpu/vmware.c | 112 -------------------------------------
> 6 files changed, 104 insertions(+), 163 deletions(-)
> delete mode 100644 arch/x86/include/asm/vmware.h
> delete mode 100644 arch/x86/kernel/cpu/vmware.c
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 9bb9286..e75e578 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -553,6 +553,18 @@ config HPET_EMULATE_RTC
> def_bool y
> depends on HPET_TIMER && (RTC=y || RTC=m || RTC_DRV_CMOS=m || RTC_DRV_CMOS=y)
>
> +config X86_HYPERVISOR
> + bool
> + default n
> + prompt "X86 Hypervisor Support"
> + ---help---
> + A hypervisor, also called virtual machine monitor (VMM),
> + is a computer hardware platform virtualization software
> + that allows multiple operating systems to run on a host
> + computer concurrently
> +
> + Choose Y to use VMware Hypervisor
> +
> # Mark as embedded because too many people got it wrong.
> # The code disables itself when not needed.
> config DMI
> diff --git a/arch/x86/include/asm/hypervisor.h b/arch/x86/include/asm/hypervisor.h
> index 369f5c5..e0b1ee0 100644
> --- a/arch/x86/include/asm/hypervisor.h
> +++ b/arch/x86/include/asm/hypervisor.h
> @@ -17,10 +17,38 @@
> * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
> *
> */
> -#ifndef ASM_X86__HYPERVISOR_H
> -#define ASM_X86__HYPERVISOR_H
> +#ifndef ASM_X86_HYPERVISOR_H
> +#define ASM_X86_HYPERVISOR_H
> +
> +#ifdef CONFIG_X86_HYPERVISOR
> +
> +/* VMware */
> +#define CPUID_VMWARE_INFO_LEAF 0x40000000
> +#define VMWARE_HYPERVISOR_MAGIC 0x564D5868
> +#define VMWARE_HYPERVISOR_PORT 0x5658
> +
> +#define VMWARE_PORT_CMD_GETVERSION 10
> +#define VMWARE_PORT_CMD_GETHZ 45
> +
> +#define VMWARE_PORT(cmd, eax, ebx, ecx, edx) \
> + __asm__("inl (%%dx)": \
> + "=a"(eax), "=c"(ecx), "=d"(edx), "=b"(ebx) : \
> + "0"(VMWARE_HYPERVISOR_MAGIC), \
> + "1"(VMWARE_PORT_CMD_##cmd), \
> + "2"(VMWARE_HYPERVISOR_PORT), "3"(UINT_MAX) : \
> + "memory");
>
> extern unsigned long get_hypervisor_tsc_freq(void);
> extern void init_hypervisor(struct cpuinfo_x86 *c);
>
> -#endif
> +#else /* CONFIG_X86_HYPERVISOR */
> +
> +static inline unsigned long get_hypervisor_tsc_freq(void)
> +{
> + return 0;
> +}
> +
> +static inline void init_hypervisor(struct cpuinfo_x86 *c) {}
> +#endif /* CONFIG_X86_HYPERVISOR */
> +
> +#endif /* ASM_X86_HYPERVISOR_H */
> diff --git a/arch/x86/include/asm/vmware.h b/arch/x86/include/asm/vmware.h
> deleted file mode 100644
> index c11b7e1..0000000
> --- a/arch/x86/include/asm/vmware.h
> +++ /dev/null
> @@ -1,27 +0,0 @@
> -/*
> - * Copyright (C) 2008, VMware, Inc.
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License as published by
> - * the Free Software Foundation; either version 2 of the License, or
> - * (at your option) any later version.
> - *
> - * This program is distributed in the hope that it will be useful, but
> - * WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
> - * NON INFRINGEMENT. See the GNU General Public License for more
> - * details.
> - *
> - * You should have received a copy of the GNU General Public License
> - * along with this program; if not, write to the Free Software
> - * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
> - *
> - */
> -#ifndef ASM_X86__VMWARE_H
> -#define ASM_X86__VMWARE_H
> -
> -extern unsigned long vmware_get_tsc_khz(void);
> -extern int vmware_platform(void);
> -extern void vmware_set_feature_bits(struct cpuinfo_x86 *c);
> -
> -#endif
> diff --git a/arch/x86/kernel/cpu/Makefile b/arch/x86/kernel/cpu/Makefile
> index 3efcb2b..55e251e 100644
> --- a/arch/x86/kernel/cpu/Makefile
> +++ b/arch/x86/kernel/cpu/Makefile
> @@ -9,11 +9,11 @@ endif
>
> obj-y := intel_cacheinfo.o addon_cpuid_features.o
> obj-y += proc.o capflags.o powerflags.o common.o
> -obj-y += vmware.o hypervisor.o
>
> obj-$(CONFIG_X86_32) += bugs.o cmpxchg.o
> obj-$(CONFIG_X86_64) += bugs_64.o
>
> +obj-$(CONFIG_X86_HYPERVISOR) += hypervisor.o
> obj-$(CONFIG_X86_CPU_DEBUG) += cpu_debug.o
>
> obj-$(CONFIG_CPU_SUP_INTEL) += intel.o
> diff --git a/arch/x86/kernel/cpu/hypervisor.c b/arch/x86/kernel/cpu/hypervisor.c
> index fb5b86a..e8b3cb2 100644
> --- a/arch/x86/kernel/cpu/hypervisor.c
> +++ b/arch/x86/kernel/cpu/hypervisor.c
> @@ -2,6 +2,7 @@
> * Common hypervisor code
> *
> * Copyright (C) 2008, VMware, Inc.
> + * Copyright (C) 2009, Jaswinder Singh Rajput
> * Author : Alok N Kataria <[email protected]>
> *
> * This program is free software; you can redistribute it and/or modify
> @@ -21,38 +22,77 @@
> *
> */
>
> -#include <asm/processor.h>
> -#include <asm/vmware.h>
> +#include <linux/dmi.h>
> +
> #include <asm/hypervisor.h>
> +#include <asm/processor.h>
> +#include <asm/div64.h>
>
> -static inline void __cpuinit
> -detect_hypervisor_vendor(struct cpuinfo_x86 *c)
> +/* VMware */
> +static unsigned long vmware_get_tsc_khz(void)
> {
> - if (vmware_platform()) {
> - c->x86_hyper_vendor = X86_HYPER_VENDOR_VMWARE;
> - } else {
> - c->x86_hyper_vendor = X86_HYPER_VENDOR_NONE;
> - }
> + uint32_t eax, ebx, ecx, edx;
> + uint64_t tsc_hz;
> +
> + VMWARE_PORT(GETHZ, eax, ebx, ecx, edx);
> +
> + if (ebx == UINT_MAX)
> + return 0;
> + tsc_hz = eax | (((uint64_t)ebx) << 32);
> + do_div(tsc_hz, 1000);
> + BUG_ON(tsc_hz >> 32);
> +
> + return tsc_hz;
> }
>
> -unsigned long get_hypervisor_tsc_freq(void)
> +static inline int __vmware_platform(void)
> {
> - if (boot_cpu_data.x86_hyper_vendor == X86_HYPER_VENDOR_VMWARE)
> - return vmware_get_tsc_khz();
> + uint32_t eax, ebx, ecx, edx;
> +
> + VMWARE_PORT(GETVERSION, eax, ebx, ecx, edx);
> +
> + return eax != (uint32_t)-1 && ebx == VMWARE_HYPERVISOR_MAGIC;
> +}
> +
> +/*
> + * While checking the dmi string infomation, just checking the product
> + * serial key should be enough, as this will always have a VMware
> + * specific string when running under VMware hypervisor.
> + */
> +static int vmware_platform(void)
> +{
> + if (cpu_has_hypervisor) {
> + unsigned int eax, ebx, ecx, edx;
> + char hyper_vendor_id[13];
> +
> + cpuid(CPUID_VMWARE_INFO_LEAF, &eax, &ebx, &ecx, &edx);
> + memcpy(hyper_vendor_id + 0, &ebx, 4);
> + memcpy(hyper_vendor_id + 4, &ecx, 4);
> + memcpy(hyper_vendor_id + 8, &edx, 4);
> + hyper_vendor_id[12] = '\0';
> + if (!strcmp(hyper_vendor_id, "VMwareVMware"))
> + return 1;
> + } else if (dmi_available && dmi_name_in_serial("VMware") &&
> + __vmware_platform())
> + return 1;
> +
> return 0;
> }
>
> -static inline void __cpuinit
> -hypervisor_set_feature_bits(struct cpuinfo_x86 *c)
> +unsigned long get_hypervisor_tsc_freq(void)
> {
> - if (boot_cpu_data.x86_hyper_vendor == X86_HYPER_VENDOR_VMWARE) {
> - vmware_set_feature_bits(c);
> - return;
> - }
> + if (boot_cpu_data.x86_hyper_vendor == X86_HYPER_VENDOR_VMWARE)
> + return vmware_get_tsc_khz();
> +
> + return 0;
> }
>
> void __cpuinit init_hypervisor(struct cpuinfo_x86 *c)
> {
> - detect_hypervisor_vendor(c);
> - hypervisor_set_feature_bits(c);
> + if (vmware_platform()) {
> + c->x86_hyper_vendor = X86_HYPER_VENDOR_VMWARE;
> + set_cpu_cap(c, X86_FEATURE_CONSTANT_TSC);
> + set_cpu_cap(c, X86_FEATURE_TSC_RELIABLE);
> + } else
> + c->x86_hyper_vendor = X86_HYPER_VENDOR_NONE;
> }
> diff --git a/arch/x86/kernel/cpu/vmware.c b/arch/x86/kernel/cpu/vmware.c
> deleted file mode 100644
> index 284c399..0000000
> --- a/arch/x86/kernel/cpu/vmware.c
> +++ /dev/null
> @@ -1,112 +0,0 @@
> -/*
> - * VMware Detection code.
> - *
> - * Copyright (C) 2008, VMware, Inc.
> - * Author : Alok N Kataria <[email protected]>
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License as published by
> - * the Free Software Foundation; either version 2 of the License, or
> - * (at your option) any later version.
> - *
> - * This program is distributed in the hope that it will be useful, but
> - * WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
> - * NON INFRINGEMENT. See the GNU General Public License for more
> - * details.
> - *
> - * You should have received a copy of the GNU General Public License
> - * along with this program; if not, write to the Free Software
> - * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
> - *
> - */
> -
> -#include <linux/dmi.h>
> -#include <asm/div64.h>
> -#include <asm/vmware.h>
> -
> -#define CPUID_VMWARE_INFO_LEAF 0x40000000
> -#define VMWARE_HYPERVISOR_MAGIC 0x564D5868
> -#define VMWARE_HYPERVISOR_PORT 0x5658
> -
> -#define VMWARE_PORT_CMD_GETVERSION 10
> -#define VMWARE_PORT_CMD_GETHZ 45
> -
> -#define VMWARE_PORT(cmd, eax, ebx, ecx, edx) \
> - __asm__("inl (%%dx)" : \
> - "=a"(eax), "=c"(ecx), "=d"(edx), "=b"(ebx) : \
> - "0"(VMWARE_HYPERVISOR_MAGIC), \
> - "1"(VMWARE_PORT_CMD_##cmd), \
> - "2"(VMWARE_HYPERVISOR_PORT), "3"(UINT_MAX) : \
> - "memory");
> -
> -static inline int __vmware_platform(void)
> -{
> - uint32_t eax, ebx, ecx, edx;
> - VMWARE_PORT(GETVERSION, eax, ebx, ecx, edx);
> - return eax != (uint32_t)-1 && ebx == VMWARE_HYPERVISOR_MAGIC;
> -}
> -
> -static unsigned long __vmware_get_tsc_khz(void)
> -{
> - uint64_t tsc_hz;
> - uint32_t eax, ebx, ecx, edx;
> -
> - VMWARE_PORT(GETHZ, eax, ebx, ecx, edx);
> -
> - if (ebx == UINT_MAX)
> - return 0;
> - tsc_hz = eax | (((uint64_t)ebx) << 32);
> - do_div(tsc_hz, 1000);
> - BUG_ON(tsc_hz >> 32);
> - return tsc_hz;
> -}
> -
> -/*
> - * While checking the dmi string infomation, just checking the product
> - * serial key should be enough, as this will always have a VMware
> - * specific string when running under VMware hypervisor.
> - */
> -int vmware_platform(void)
> -{
> - if (cpu_has_hypervisor) {
> - unsigned int eax, ebx, ecx, edx;
> - char hyper_vendor_id[13];
> -
> - cpuid(CPUID_VMWARE_INFO_LEAF, &eax, &ebx, &ecx, &edx);
> - memcpy(hyper_vendor_id + 0, &ebx, 4);
> - memcpy(hyper_vendor_id + 4, &ecx, 4);
> - memcpy(hyper_vendor_id + 8, &edx, 4);
> - hyper_vendor_id[12] = '\0';
> - if (!strcmp(hyper_vendor_id, "VMwareVMware"))
> - return 1;
> - } else if (dmi_available && dmi_name_in_serial("VMware") &&
> - __vmware_platform())
> - return 1;
> -
> - return 0;
> -}
> -
> -unsigned long vmware_get_tsc_khz(void)
> -{
> - BUG_ON(!vmware_platform());
> - return __vmware_get_tsc_khz();
> -}
> -
> -/*
> - * 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.
> - */
> -void __cpuinit vmware_set_feature_bits(struct cpuinfo_x86 *c)
> -{
> - set_cpu_cap(c, X86_FEATURE_CONSTANT_TSC);
> - set_cpu_cap(c, X86_FEATURE_TSC_RELIABLE);
> -}
On Tue, 2009-03-17 at 08:50 -0700, H. Peter Anvin wrote:
> Jaswinder Singh Rajput wrote:
> >> I dont really like this one. KVM is a hypervisor too, and so is
> >> Xen and lguest. VMware is one of the many types of a
> >> hypervisors.
> >
> > Can we use common hypervisor for Xen, lguest, vmware, etc.
> > And Xen, lguest, vmware will be the sub-options.
>
> Obviously. We spent a lot of time breaking up the cpu stuff into common
> and vendor-specific portions, we shouldn't go backwards w.r.t. hypervisors.
>
OK, agreed.
But atleast give freedom to users to disable this who are not willing to
use it.
Currently hypervisor is used only used by VMWARE, so is I prepare this
freedom patch:
From: Jaswinder Singh Rajput <[email protected]>
Date: Wed, 25 Mar 2009 10:40:01 +0530
Subject: [PATCH] x86: Introduce CONFIG_X86_VMWARE option
Impact: freedom to choose
Gives freedom to users to select or suppress CONFIG_X86_VMWARE option.
Signed-off-by: Jaswinder Singh Rajput <[email protected]>
---
arch/x86/Kconfig | 7 +++++++
arch/x86/include/asm/hypervisor.h | 9 +++++++++
arch/x86/kernel/cpu/Makefile | 2 +-
3 files changed, 17 insertions(+), 1 deletions(-)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index f5d7d29..56e1a9c 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -554,6 +554,13 @@ config HPET_EMULATE_RTC
def_bool y
depends on HPET_TIMER && (RTC=y || RTC=m || RTC_DRV_CMOS=m || RTC_DRV_CMOS=y)
+config X86_VMWARE
+ bool
+ default n
+ prompt "X86 VMware support"
+ ---help---
+ Enable X86 VMware Hypervisor support.
+
# Mark as embedded because too many people got it wrong.
# The code disables itself when not needed.
config DMI
diff --git a/arch/x86/include/asm/hypervisor.h b/arch/x86/include/asm/hypervisor.h
index 369f5c5..c08ae75 100644
--- a/arch/x86/include/asm/hypervisor.h
+++ b/arch/x86/include/asm/hypervisor.h
@@ -20,7 +20,16 @@
#ifndef ASM_X86__HYPERVISOR_H
#define ASM_X86__HYPERVISOR_H
+#ifdef CONFIG_X86_VMWARE
extern unsigned long get_hypervisor_tsc_freq(void);
extern void init_hypervisor(struct cpuinfo_x86 *c);
+#else
+static inline unsigned long get_hypervisor_tsc_freq(void)
+{
+ return 0;
+}
+
+static inline void init_hypervisor(struct cpuinfo_x86 *c) {}
+#endif
#endif
diff --git a/arch/x86/kernel/cpu/Makefile b/arch/x86/kernel/cpu/Makefile
index 3efcb2b..24372f7 100644
--- a/arch/x86/kernel/cpu/Makefile
+++ b/arch/x86/kernel/cpu/Makefile
@@ -9,11 +9,11 @@ endif
obj-y := intel_cacheinfo.o addon_cpuid_features.o
obj-y += proc.o capflags.o powerflags.o common.o
-obj-y += vmware.o hypervisor.o
obj-$(CONFIG_X86_32) += bugs.o cmpxchg.o
obj-$(CONFIG_X86_64) += bugs_64.o
+obj-$(CONFIG_X86_VMWARE) += vmware.o hypervisor.o
obj-$(CONFIG_X86_CPU_DEBUG) += cpu_debug.o
obj-$(CONFIG_CPU_SUP_INTEL) += intel.o
--
1.6.0.6
* Jaswinder Singh Rajput <[email protected]> wrote:
> On Tue, 2009-03-17 at 08:50 -0700, H. Peter Anvin wrote:
> > Jaswinder Singh Rajput wrote:
> > >> I dont really like this one. KVM is a hypervisor too, and so is
> > >> Xen and lguest. VMware is one of the many types of a
> > >> hypervisors.
> > >
> > > Can we use common hypervisor for Xen, lguest, vmware, etc.
> > > And Xen, lguest, vmware will be the sub-options.
> >
> > Obviously. We spent a lot of time breaking up the cpu stuff into common
> > and vendor-specific portions, we shouldn't go backwards w.r.t. hypervisors.
> >
>
> OK, agreed.
>
> But atleast give freedom to users to disable this who are not willing to
> use it.
>
> Currently hypervisor is used only used by VMWARE, so is I prepare this
> freedom patch:
>
> From: Jaswinder Singh Rajput <[email protected]>
> Date: Wed, 25 Mar 2009 10:40:01 +0530
> Subject: [PATCH] x86: Introduce CONFIG_X86_VMWARE option
>
> Impact: freedom to choose
>
> Gives freedom to users to select or suppress CONFIG_X86_VMWARE option.
>
> Signed-off-by: Jaswinder Singh Rajput <[email protected]>
> ---
> arch/x86/Kconfig | 7 +++++++
> arch/x86/include/asm/hypervisor.h | 9 +++++++++
> arch/x86/kernel/cpu/Makefile | 2 +-
> 3 files changed, 17 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index f5d7d29..56e1a9c 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -554,6 +554,13 @@ config HPET_EMULATE_RTC
> def_bool y
> depends on HPET_TIMER && (RTC=y || RTC=m || RTC_DRV_CMOS=m || RTC_DRV_CMOS=y)
>
> +config X86_VMWARE
> + bool
> + default n
> + prompt "X86 VMware support"
> + ---help---
> + Enable X86 VMware Hypervisor support.
> +
> # Mark as embedded because too many people got it wrong.
> # The code disables itself when not needed.
> config DMI
> diff --git a/arch/x86/include/asm/hypervisor.h b/arch/x86/include/asm/hypervisor.h
> index 369f5c5..c08ae75 100644
> --- a/arch/x86/include/asm/hypervisor.h
> +++ b/arch/x86/include/asm/hypervisor.h
> @@ -20,7 +20,16 @@
> #ifndef ASM_X86__HYPERVISOR_H
> #define ASM_X86__HYPERVISOR_H
>
> +#ifdef CONFIG_X86_VMWARE
> extern unsigned long get_hypervisor_tsc_freq(void);
> extern void init_hypervisor(struct cpuinfo_x86 *c);
> +#else
> +static inline unsigned long get_hypervisor_tsc_freq(void)
> +{
> + return 0;
> +}
> +
> +static inline void init_hypervisor(struct cpuinfo_x86 *c) {}
> +#endif
>
> #endif
> diff --git a/arch/x86/kernel/cpu/Makefile b/arch/x86/kernel/cpu/Makefile
> index 3efcb2b..24372f7 100644
> --- a/arch/x86/kernel/cpu/Makefile
> +++ b/arch/x86/kernel/cpu/Makefile
> @@ -9,11 +9,11 @@ endif
>
> obj-y := intel_cacheinfo.o addon_cpuid_features.o
> obj-y += proc.o capflags.o powerflags.o common.o
> -obj-y += vmware.o hypervisor.o
>
> obj-$(CONFIG_X86_32) += bugs.o cmpxchg.o
> obj-$(CONFIG_X86_64) += bugs_64.o
>
> +obj-$(CONFIG_X86_VMWARE) += vmware.o hypervisor.o
> obj-$(CONFIG_X86_CPU_DEBUG) += cpu_debug.o
vmware can be considered a CPU here, so i think making the disabling
also depend on PROCESSOR_SELECT.
Ingo
Jaswinder Singh Rajput wrote:
> On Tue, 2009-03-17 at 08:50 -0700, H. Peter Anvin wrote:
>> Jaswinder Singh Rajput wrote:
>>>> I dont really like this one. KVM is a hypervisor too, and so is
>>>> Xen and lguest. VMware is one of the many types of a
>>>> hypervisors.
>>> Can we use common hypervisor for Xen, lguest, vmware, etc.
>>> And Xen, lguest, vmware will be the sub-options.
>> Obviously. We spent a lot of time breaking up the cpu stuff into common
>> and vendor-specific portions, we shouldn't go backwards w.r.t. hypervisors.
>>
>
> OK, agreed.
>
> But atleast give freedom to users to disable this who are not willing to
> use it.
>
> Currently hypervisor is used only used by VMWARE, so is I prepare this
> freedom patch:
>
> From: Jaswinder Singh Rajput <[email protected]>
> Date: Wed, 25 Mar 2009 10:40:01 +0530
> Subject: [PATCH] x86: Introduce CONFIG_X86_VMWARE option
>
> Impact: freedom to choose
>
> Gives freedom to users to select or suppress CONFIG_X86_VMWARE option.
>
> Signed-off-by: Jaswinder Singh Rajput <[email protected]>
> ---
> arch/x86/Kconfig | 7 +++++++
> arch/x86/include/asm/hypervisor.h | 9 +++++++++
> arch/x86/kernel/cpu/Makefile | 2 +-
> 3 files changed, 17 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index f5d7d29..56e1a9c 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -554,6 +554,13 @@ config HPET_EMULATE_RTC
> def_bool y
> depends on HPET_TIMER && (RTC=y || RTC=m || RTC_DRV_CMOS=m || RTC_DRV_CMOS=y)
>
> +config X86_VMWARE
> + bool
> + default n
> + prompt "X86 VMware support"
> + ---help---
> + Enable X86 VMware Hypervisor support.
>
..
Could that perhaps be clarified somewhat, changing the prompt
string to either "X86 VMware host support"
or "X86 VMware guest support", to remove any confusion ?
thanks
On Wed, 2009-03-25 at 05:59 -0700, Ingo Molnar wrote:
> * Jaswinder Singh Rajput <[email protected]> wrote:
>
> > On Tue, 2009-03-17 at 08:50 -0700, H. Peter Anvin wrote:
> > > Jaswinder Singh Rajput wrote:
> > > >> I dont really like this one. KVM is a hypervisor too, and so is
> > > >> Xen and lguest. VMware is one of the many types of a
> > > >> hypervisors.
> > > >
> > > > Can we use common hypervisor for Xen, lguest, vmware, etc.
> > > > And Xen, lguest, vmware will be the sub-options.
> > >
> > > Obviously. We spent a lot of time breaking up the cpu stuff into common
> > > and vendor-specific portions, we shouldn't go backwards w.r.t. hypervisors.
> > >
> >
> > OK, agreed.
> >
> > But atleast give freedom to users to disable this who are not willing to
> > use it.
> >
> > Currently hypervisor is used only used by VMWARE, so is I prepare this
> > freedom patch:
> >
> > From: Jaswinder Singh Rajput <[email protected]>
> > Date: Wed, 25 Mar 2009 10:40:01 +0530
> > Subject: [PATCH] x86: Introduce CONFIG_X86_VMWARE option
> >
> > Impact: freedom to choose
> >
> > Gives freedom to users to select or suppress CONFIG_X86_VMWARE option.
> >
> > Signed-off-by: Jaswinder Singh Rajput <[email protected]>
> > ---
> > arch/x86/Kconfig | 7 +++++++
> > arch/x86/include/asm/hypervisor.h | 9 +++++++++
> > arch/x86/kernel/cpu/Makefile | 2 +-
> > 3 files changed, 17 insertions(+), 1 deletions(-)
> >
> > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > index f5d7d29..56e1a9c 100644
> > --- a/arch/x86/Kconfig
> > +++ b/arch/x86/Kconfig
> > @@ -554,6 +554,13 @@ config HPET_EMULATE_RTC
> > def_bool y
> > depends on HPET_TIMER && (RTC=y || RTC=m || RTC_DRV_CMOS=m || RTC_DRV_CMOS=y)
> >
> > +config X86_VMWARE
> > + bool
> > + default n
> > + prompt "X86 VMware support"
> > + ---help---
> > + Enable X86 VMware Hypervisor support.
> > +
> > # Mark as embedded because too many people got it wrong.
> > # The code disables itself when not needed.
> > config DMI
> > diff --git a/arch/x86/include/asm/hypervisor.h b/arch/x86/include/asm/hypervisor.h
> > index 369f5c5..c08ae75 100644
> > --- a/arch/x86/include/asm/hypervisor.h
> > +++ b/arch/x86/include/asm/hypervisor.h
> > @@ -20,7 +20,16 @@
> > #ifndef ASM_X86__HYPERVISOR_H
> > #define ASM_X86__HYPERVISOR_H
> >
> > +#ifdef CONFIG_X86_VMWARE
> > extern unsigned long get_hypervisor_tsc_freq(void);
> > extern void init_hypervisor(struct cpuinfo_x86 *c);
> > +#else
> > +static inline unsigned long get_hypervisor_tsc_freq(void)
> > +{
> > + return 0;
> > +}
> > +
> > +static inline void init_hypervisor(struct cpuinfo_x86 *c) {}
> > +#endif
> >
> > #endif
> > diff --git a/arch/x86/kernel/cpu/Makefile b/arch/x86/kernel/cpu/Makefile
> > index 3efcb2b..24372f7 100644
> > --- a/arch/x86/kernel/cpu/Makefile
> > +++ b/arch/x86/kernel/cpu/Makefile
> > @@ -9,11 +9,11 @@ endif
> >
> > obj-y := intel_cacheinfo.o addon_cpuid_features.o
> > obj-y += proc.o capflags.o powerflags.o common.o
> > -obj-y += vmware.o hypervisor.o
> >
> > obj-$(CONFIG_X86_32) += bugs.o cmpxchg.o
> > obj-$(CONFIG_X86_64) += bugs_64.o
> >
> > +obj-$(CONFIG_X86_VMWARE) += vmware.o hypervisor.o
> > obj-$(CONFIG_X86_CPU_DEBUG) += cpu_debug.o
>
> vmware can be considered a CPU here, so i think making the disabling
> also depend on PROCESSOR_SELECT.
Ingo, this code is not just to be used by VMware, the reason we did this
generically was so that a guest could run unaltered on *any* fully
virtualized hypervisor.
And give that this code is just a boot setup thing, the only thing this
patch saves over here is not running the detection code on native
systems. All the rest of the code is guarded by the
"boot_cpu_data.x86_hyper_vendor" checks anyways.
I don't really see the point of adding one more config option just for
this.
Thanks,
Alok
>
> Ingo
On Wed, 2009-03-25 at 09:52 -0700, Alok Kataria wrote:
> >
> > vmware can be considered a CPU here, so i think making the disabling
> > also depend on PROCESSOR_SELECT.
>
> Ingo, this code is not just to be used by VMware, the reason we did this
> generically was so that a guest could run unaltered on *any* fully
> virtualized hypervisor.
> And give that this code is just a boot setup thing, the only thing this
> patch saves over here is not running the detection code on native
> systems. All the rest of the code is guarded by the
> "boot_cpu_data.x86_hyper_vendor" checks anyways.
>
> I don't really see the point of adding one more config option just for
> this.
>
Can you please explain what is the point of adding this support all the
time if this is useless for 99.9% of cases. IMHO, it should be optional.
--
JSR
On Wed, 2009-03-25 at 10:07 -0700, Jaswinder Singh Rajput wrote:
> On Wed, 2009-03-25 at 09:52 -0700, Alok Kataria wrote:
> > >
> > > vmware can be considered a CPU here, so i think making the disabling
> > > also depend on PROCESSOR_SELECT.
> >
> > Ingo, this code is not just to be used by VMware, the reason we did this
> > generically was so that a guest could run unaltered on *any* fully
> > virtualized hypervisor.
> > And give that this code is just a boot setup thing, the only thing this
> > patch saves over here is not running the detection code on native
> > systems. All the rest of the code is guarded by the
> > "boot_cpu_data.x86_hyper_vendor" checks anyways.
> >
> > I don't really see the point of adding one more config option just for
> > this.
> >
>
> Can you please explain what is the point of adding this support all the
> time if this is useless for 99.9% of cases. IMHO, it should be optional.
First of all, I don't know how did you get to the 99.9% number, though I
think its not a point worth debating, just like to share some info with
you. More and more people are adopting virtualization now a days and
give the trend i don't see just 0.1% people running Linux on virtualized
hardware. So though its not a common case there is still a large user
base.
I am not saying we should not hide this behind a config at all. The
point is there is nothing that we save by adding a new config, so what's
the point at all. If you can give me a solid reason like, say, you save
1% code space with this config option, or 'n' sec in the boottime, I am
all ears for such an argument.
Thanks,
Alok
>
> --
> JSR
>
>
>
>
>
On Wed, 2009-03-25 at 10:24 -0700, Alok Kataria wrote:
> On Wed, 2009-03-25 at 10:07 -0700, Jaswinder Singh Rajput wrote:
> > On Wed, 2009-03-25 at 09:52 -0700, Alok Kataria wrote:
> > > >
> > > > vmware can be considered a CPU here, so i think making the disabling
> > > > also depend on PROCESSOR_SELECT.
> > >
> > > Ingo, this code is not just to be used by VMware, the reason we did this
> > > generically was so that a guest could run unaltered on *any* fully
> > > virtualized hypervisor.
> > > And give that this code is just a boot setup thing, the only thing this
> > > patch saves over here is not running the detection code on native
> > > systems. All the rest of the code is guarded by the
> > > "boot_cpu_data.x86_hyper_vendor" checks anyways.
> > >
> > > I don't really see the point of adding one more config option just for
> > > this.
> > >
> >
> > Can you please explain what is the point of adding this support all the
> > time if this is useless for 99.9% of cases. IMHO, it should be optional.
>
> First of all, I don't know how did you get to the 99.9% number, though I
> think its not a point worth debating, just like to share some info with
> you. More and more people are adopting virtualization now a days and
> give the trend i don't see just 0.1% people running Linux on virtualized
> hardware. So though its not a common case there is still a large user
> base.
I am agree with you there is no point for debate.
If someone need this option, she can enable it and use it.
Thanks,
--
JSR
On Wed, 2009-03-25 at 10:38 -0700, Jaswinder Singh Rajput wrote:
> On Wed, 2009-03-25 at 10:24 -0700, Alok Kataria wrote:
> > On Wed, 2009-03-25 at 10:07 -0700, Jaswinder Singh Rajput wrote:
> > > On Wed, 2009-03-25 at 09:52 -0700, Alok Kataria wrote:
> > > > >
> > > > > vmware can be considered a CPU here, so i think making the disabling
> > > > > also depend on PROCESSOR_SELECT.
> > > >
> > > > Ingo, this code is not just to be used by VMware, the reason we did this
> > > > generically was so that a guest could run unaltered on *any* fully
> > > > virtualized hypervisor.
> > > > And give that this code is just a boot setup thing, the only thing this
> > > > patch saves over here is not running the detection code on native
> > > > systems. All the rest of the code is guarded by the
> > > > "boot_cpu_data.x86_hyper_vendor" checks anyways.
> > > >
> > > > I don't really see the point of adding one more config option just for
> > > > this.
> > > >
> > >
> > > Can you please explain what is the point of adding this support all the
> > > time if this is useless for 99.9% of cases. IMHO, it should be optional.
> >
> > First of all, I don't know how did you get to the 99.9% number, though I
> > think its not a point worth debating, just like to share some info with
> > you. More and more people are adopting virtualization now a days and
> > give the trend i don't see just 0.1% people running Linux on virtualized
> > hardware. So though its not a common case there is still a large user
> > base.
>
> I am agree with you there is no point for debate.
>
> If someone need this option, she can enable it and use it.
You seem to be missing the point I raised in the previous mail. Its
below for your reference.
> I am not saying we should not hide this behind a config at all. The
> point is there is nothing that we save by adding a new config, so what's
> the point at all. If you can give me a solid reason like, say, you save
> 1% code space with this config option, or 'n' sec in the boottime, I am
> all ears for such an argument.
>
>
So, if there are any tangible benefits with doing this I am ok with it,
but your current argument about "Freedom to user" doesn't sound too
compelling.
--
Alok
On Wed, 25 Mar 2009, Alok Kataria wrote:
> On Wed, 2009-03-25 at 10:38 -0700, Jaswinder Singh Rajput wrote:
>> I am not saying we should not hide this behind a config at all. The
>> point is there is nothing that we save by adding a new config, so what's
>> the point at all. If you can give me a solid reason like, say, you save
>> 1% code space with this config option, or 'n' sec in the boottime, I am
>> all ears for such an argument.
>>
>>
>
> So, if there are any tangible benefits with doing this I am ok with it,
> but your current argument about "Freedom to user" doesn't sound too
> compelling.
isn't it the other way around? you are adding this in, it should be up to
you to show that it
A. has no impact
B. has a small enough impact that it's not worth the config option
C. has an impact, but is so valuble that it should be on for all systems
(even those that are absolutly not suitable for running virtual machines)
David Lang
On Thu, 2009-03-26 at 00:10 -0700, [email protected] wrote:
> On Wed, 25 Mar 2009, Alok Kataria wrote:
>
> > On Wed, 2009-03-25 at 10:38 -0700, Jaswinder Singh Rajput wrote:
> >> I am not saying we should not hide this behind a config at all. The
> >> point is there is nothing that we save by adding a new config, so what's
> >> the point at all. If you can give me a solid reason like, say, you save
> >> 1% code space with this config option, or 'n' sec in the boottime, I am
> >> all ears for such an argument.
> >>
> >>
> >
> > So, if there are any tangible benefits with doing this I am ok with it,
> > but your current argument about "Freedom to user" doesn't sound too
> > compelling.
>
> isn't it the other way around? you are adding this in, it should be up to
> you to show that it
>
> A. has no impact
>
> B. has a small enough impact that it's not worth the config option
This is what exactly I have mentioned in a mail earlier in this thread.
http://lkml.org/lkml/2009/3/25/333
Alok
Alok Kataria wrote:
>>>>
>>> So, if there are any tangible benefits with doing this I am ok with it,
>>> but your current argument about "Freedom to user" doesn't sound too
>>> compelling.
>> isn't it the other way around? you are adding this in, it should be up to
>> you to show that it
>>
>> A. has no impact
>>
>> B. has a small enough impact that it's not worth the config option
>
> This is what exactly I have mentioned in a mail earlier in this thread.
> http://lkml.org/lkml/2009/3/25/333
>
It sounds to me like the kind of things that gets buried under
CONFIG_EMBEDDED for the people who are absolutely paranoid (sometimes
even for very good reasons) about every additional byte in the kernel.
-hpa