diff mbox

[dpdk-dev] eal/x86: get hypervisor name

Message ID 20171130214720.30098-1-thomas@monjalon.net (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues

Commit Message

Thomas Monjalon Nov. 30, 2017, 9:47 p.m. UTC 
  The CPUID instruction is catched by hypervisor which can return
a flag indicating one is running, and its name.

Suggested-by: Stephen Hemminger <sthemmin@microsoft.com>
Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
warning: to be tested
---
 lib/librte_eal/common/arch/arm/rte_cpuflags.c      |  6 +++++
 lib/librte_eal/common/arch/ppc_64/rte_cpuflags.c   |  6 +++++
 lib/librte_eal/common/arch/x86/rte_cpuflags.c      | 30 ++++++++++++++++++++++
 .../common/include/arch/x86/rte_cpuflags.h         |  1 +
 .../common/include/generic/rte_cpuflags.h          | 14 ++++++++++
 lib/librte_eal/rte_eal_version.map                 |  9 ++++++-
 6 files changed, 65 insertions(+), 1 deletion(-)

Comments

Stephen Hemminger Nov. 30, 2017, 9:55 p.m. UTC | #1 
On Thu, 30 Nov 2017 22:47:20 +0100
Thomas Monjalon <thomas@monjalon.net> wrote:

> +	for (int reg = 1; reg < 4; reg++)
> +		memcpy(name + (reg - 1) * 4, &regs[reg], 4);
> +	name[12] = '\0';

C99 style declarations are not generally used in DPDK.
Thomas Monjalon Nov. 30, 2017, 10:03 p.m. UTC | #2 
30/11/2017 22:55, Stephen Hemminger:
> On Thu, 30 Nov 2017 22:47:20 +0100
> Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> > +	for (int reg = 1; reg < 4; reg++)
> > +		memcpy(name + (reg - 1) * 4, &regs[reg], 4);
> > +	name[12] = '\0';
> 
> C99 style declarations are not generally used in DPDK.

True :)
It will be fixed in v2.

I think I should add a log somewhere at initialization in order
to test this function.
Do you think it deserves a table of name strings in the .h?
Stephen Hemminger Nov. 30, 2017, 10:11 p.m. UTC | #3 
On Thu, 30 Nov 2017 23:03:57 +0100
Thomas Monjalon <thomas@monjalon.net> wrote:

> 30/11/2017 22:55, Stephen Hemminger:
> > On Thu, 30 Nov 2017 22:47:20 +0100
> > Thomas Monjalon <thomas@monjalon.net> wrote:
> >   
> > > +	for (int reg = 1; reg < 4; reg++)
> > > +		memcpy(name + (reg - 1) * 4, &regs[reg], 4);
> > > +	name[12] = '\0';  
> > 
> > C99 style declarations are not generally used in DPDK.  
> 
> True :)
> It will be fixed in v2.
> 
> I think I should add a log somewhere at initialization in order
> to test this function.
> Do you think it deserves a table of name strings in the .h?

Same table is short-listed in util-linux (lscpu).
You might also want to handle Xen, Xen PV, VirtualBox, 
Windows and WSL at some point.
Jerin Jacob Dec. 1, 2017, 8:12 a.m. UTC | #4 
-----Original Message-----
> Date: Thu, 30 Nov 2017 22:47:20 +0100
> From: Thomas Monjalon <thomas@monjalon.net>
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH] eal/x86: get hypervisor name
> X-Mailer: git-send-email 2.15.0
> 
> The CPUID instruction is catched by hypervisor which can return
> a flag indicating one is running, and its name.
> 
> Suggested-by: Stephen Hemminger <sthemmin@microsoft.com>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
> warning: to be tested
> ---
>  lib/librte_eal/common/arch/arm/rte_cpuflags.c      |  6 +++++
>  lib/librte_eal/common/arch/ppc_64/rte_cpuflags.c   |  6 +++++
>  lib/librte_eal/common/arch/x86/rte_cpuflags.c      | 30 ++++++++++++++++++++++
>  .../common/include/arch/x86/rte_cpuflags.h         |  1 +
>  .../common/include/generic/rte_cpuflags.h          | 14 ++++++++++
>  lib/librte_eal/rte_eal_version.map                 |  9 ++++++-
>  6 files changed, 65 insertions(+), 1 deletion(-)
>  	RTE_CPUFLAG_FPU,                    /**< FPU */
> diff --git a/lib/librte_eal/common/include/generic/rte_cpuflags.h b/lib/librte_eal/common/include/generic/rte_cpuflags.h
> index c1c5551fc..3832fb851 100644
> --- a/lib/librte_eal/common/include/generic/rte_cpuflags.h
> +++ b/lib/librte_eal/common/include/generic/rte_cpuflags.h
> @@ -93,4 +93,18 @@ rte_cpu_check_supported(void);
>  int
>  rte_cpu_is_supported(void);
>  
> +enum rte_hypervisor {
> +	RTE_HYPERVISOR_NONE,
> +	RTE_HYPERVISOR_KVM,
> +	RTE_HYPERVISOR_HYPERV,
> +	RTE_HYPERVISOR_VMWARE,
> +	RTE_HYPERVISOR_UNKNOWN
> +};
> +
> +/**
> + * Get the type of hypervisor it is running on.
> + */
> +enum rte_hypervisor
> +rte_hypervisor_get_name(void);

Cc: chaozhu@linux.vnet.ibm.com

IMO, cpu_flag area is the not the correct abstraction to get
the hypervisor name. It is x86 specific. I think, correct
usage will be to call hypervisor specific APIs like KVM_GET_API_VERSION
https://lwn.net/Articles/658511/

BTW, What is the need for an DPDK application to know the 
hypervisor name? What action an DPDK application should
take based on hypervisor name? if is not interest of data plane
application why it needs to be abstracted in DPDK?
Thomas Monjalon Dec. 1, 2017, 8:52 a.m. UTC | #5 
01/12/2017 09:12, Jerin Jacob:
> -----Original Message-----
> > Date: Thu, 30 Nov 2017 22:47:20 +0100
> > From: Thomas Monjalon <thomas@monjalon.net>
> > To: dev@dpdk.org
> > Subject: [dpdk-dev] [PATCH] eal/x86: get hypervisor name
> > X-Mailer: git-send-email 2.15.0
> > 
> > The CPUID instruction is catched by hypervisor which can return
> > a flag indicating one is running, and its name.
> > 
> > Suggested-by: Stephen Hemminger <sthemmin@microsoft.com>
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > ---
> > warning: to be tested
> > ---
> >  lib/librte_eal/common/arch/arm/rte_cpuflags.c      |  6 +++++
> >  lib/librte_eal/common/arch/ppc_64/rte_cpuflags.c   |  6 +++++
> >  lib/librte_eal/common/arch/x86/rte_cpuflags.c      | 30 ++++++++++++++++++++++
> >  .../common/include/arch/x86/rte_cpuflags.h         |  1 +
> >  .../common/include/generic/rte_cpuflags.h          | 14 ++++++++++
> >  lib/librte_eal/rte_eal_version.map                 |  9 ++++++-
> >  6 files changed, 65 insertions(+), 1 deletion(-)
> >  	RTE_CPUFLAG_FPU,                    /**< FPU */
> > diff --git a/lib/librte_eal/common/include/generic/rte_cpuflags.h b/lib/librte_eal/common/include/generic/rte_cpuflags.h
> > index c1c5551fc..3832fb851 100644
> > --- a/lib/librte_eal/common/include/generic/rte_cpuflags.h
> > +++ b/lib/librte_eal/common/include/generic/rte_cpuflags.h
> > @@ -93,4 +93,18 @@ rte_cpu_check_supported(void);
> >  int
> >  rte_cpu_is_supported(void);
> >  
> > +enum rte_hypervisor {
> > +	RTE_HYPERVISOR_NONE,
> > +	RTE_HYPERVISOR_KVM,
> > +	RTE_HYPERVISOR_HYPERV,
> > +	RTE_HYPERVISOR_VMWARE,
> > +	RTE_HYPERVISOR_UNKNOWN
> > +};
> > +
> > +/**
> > + * Get the type of hypervisor it is running on.
> > + */
> > +enum rte_hypervisor
> > +rte_hypervisor_get_name(void);
> 
> Cc: chaozhu@linux.vnet.ibm.com
> 
> IMO, cpu_flag area is the not the correct abstraction to get
> the hypervisor name. It is x86 specific. I think, correct
> usage will be to call hypervisor specific APIs like KVM_GET_API_VERSION
> https://lwn.net/Articles/658511/

I think it is quite logical because the CPU is virtualized by the hypervisor.
It is similar to get the CPU model, but for a different ring level.

> BTW, What is the need for an DPDK application to know the 
> hypervisor name? What action an DPDK application should
> take based on hypervisor name? if is not interest of data plane
> application why it needs to be abstracted in DPDK?

I see two usages for now.
We can automate the use of the specific VMware TSC (it is an option currently).
We can adapt the device management policy on Hyper-V without waiting a device scan.
Jerin Jacob Dec. 1, 2017, 9:17 a.m. UTC | #6 
-----Original Message-----
> Date: Fri, 01 Dec 2017 09:52:26 +0100
> From: Thomas Monjalon <thomas@monjalon.net>
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> Cc: dev@dpdk.org, chaozhu@linux.vnet.ibm.com
> Subject: Re: [dpdk-dev] [PATCH] eal/x86: get hypervisor name
> 
> 01/12/2017 09:12, Jerin Jacob:
> > -----Original Message-----
> > > Date: Thu, 30 Nov 2017 22:47:20 +0100
> > > From: Thomas Monjalon <thomas@monjalon.net>
> > > To: dev@dpdk.org
> > > Subject: [dpdk-dev] [PATCH] eal/x86: get hypervisor name
> > > X-Mailer: git-send-email 2.15.0
> > > 
> > > The CPUID instruction is catched by hypervisor which can return
> > > a flag indicating one is running, and its name.
> > > 
> > > Suggested-by: Stephen Hemminger <sthemmin@microsoft.com>
> > > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > > ---
> > > warning: to be tested
> > > ---
> > >  lib/librte_eal/common/arch/arm/rte_cpuflags.c      |  6 +++++
> > >  lib/librte_eal/common/arch/ppc_64/rte_cpuflags.c   |  6 +++++
> > >  lib/librte_eal/common/arch/x86/rte_cpuflags.c      | 30 ++++++++++++++++++++++
> > >  .../common/include/arch/x86/rte_cpuflags.h         |  1 +
> > >  .../common/include/generic/rte_cpuflags.h          | 14 ++++++++++
> > >  lib/librte_eal/rte_eal_version.map                 |  9 ++++++-
> > >  6 files changed, 65 insertions(+), 1 deletion(-)
> > >  	RTE_CPUFLAG_FPU,                    /**< FPU */
> > > diff --git a/lib/librte_eal/common/include/generic/rte_cpuflags.h b/lib/librte_eal/common/include/generic/rte_cpuflags.h
> > > index c1c5551fc..3832fb851 100644
> > > --- a/lib/librte_eal/common/include/generic/rte_cpuflags.h
> > > +++ b/lib/librte_eal/common/include/generic/rte_cpuflags.h
> > > @@ -93,4 +93,18 @@ rte_cpu_check_supported(void);
> > >  int
> > >  rte_cpu_is_supported(void);
> > >  
> > > +enum rte_hypervisor {
> > > +	RTE_HYPERVISOR_NONE,
> > > +	RTE_HYPERVISOR_KVM,
> > > +	RTE_HYPERVISOR_HYPERV,
> > > +	RTE_HYPERVISOR_VMWARE,
> > > +	RTE_HYPERVISOR_UNKNOWN
> > > +};
> > > +
> > > +/**
> > > + * Get the type of hypervisor it is running on.
> > > + */
> > > +enum rte_hypervisor
> > > +rte_hypervisor_get_name(void);
> > 
> > Cc: chaozhu@linux.vnet.ibm.com
> > 
> > IMO, cpu_flag area is the not the correct abstraction to get
> > the hypervisor name. It is x86 specific. I think, correct
> > usage will be to call hypervisor specific APIs like KVM_GET_API_VERSION
> > https://lwn.net/Articles/658511/
> 
> I think it is quite logical because the CPU is virtualized by the hypervisor.
> It is similar to get the CPU model, but for a different ring level.

At least on non x86, it is not exposed as CPU flags. IMO,
*hypervisor*.h file makes more sense rather than rte_cpuflags.h.

> 
> > BTW, What is the need for an DPDK application to know the 
> > hypervisor name? What action an DPDK application should
> > take based on hypervisor name? if is not interest of data plane
> > application why it needs to be abstracted in DPDK?
> 
> I see two usages for now.
> We can automate the use of the specific VMware TSC (it is an option currently).

In my view this use case needed by the DPDK "implementation" not the
"application". So internal API makes more sense to me.

> We can adapt the device management policy on Hyper-V without waiting a device scan.

I am not sure about this? Looks like a candidate for library implementation
not for the "application"
diff mbox

Patch

diff --git a/lib/librte_eal/common/arch/arm/rte_cpuflags.c b/lib/librte_eal/common/arch/arm/rte_cpuflags.c
index 88f1cbe37..f1f0cb51d 100644
--- a/lib/librte_eal/common/arch/arm/rte_cpuflags.c
+++ b/lib/librte_eal/common/arch/arm/rte_cpuflags.c
@@ -178,3 +178,9 @@  rte_cpu_get_flag_name(enum rte_cpu_flag_t feature)
 		return NULL;
 	return rte_cpu_feature_table[feature].name;
 }
+
+enum rte_hypervisor
+rte_hypervisor_get_name(void)
+{
+	return RTE_HYPERVISOR_UNKNOWN;
+}
diff --git a/lib/librte_eal/common/arch/ppc_64/rte_cpuflags.c b/lib/librte_eal/common/arch/ppc_64/rte_cpuflags.c
index 970a61c5e..b905cbd2c 100644
--- a/lib/librte_eal/common/arch/ppc_64/rte_cpuflags.c
+++ b/lib/librte_eal/common/arch/ppc_64/rte_cpuflags.c
@@ -146,3 +146,9 @@  rte_cpu_get_flag_name(enum rte_cpu_flag_t feature)
 		return NULL;
 	return rte_cpu_feature_table[feature].name;
 }
+
+enum rte_hypervisor
+rte_hypervisor_get_name(void)
+{
+	return RTE_HYPERVISOR_UNKNOWN;
+}
diff --git a/lib/librte_eal/common/arch/x86/rte_cpuflags.c b/lib/librte_eal/common/arch/x86/rte_cpuflags.c
index 7d4a0fefb..76893b8f1 100644
--- a/lib/librte_eal/common/arch/x86/rte_cpuflags.c
+++ b/lib/librte_eal/common/arch/x86/rte_cpuflags.c
@@ -34,6 +34,7 @@ 
 #include "rte_cpuflags.h"
 
 #include <stdio.h>
+#include <string.h>
 #include <errno.h>
 #include <stdint.h>
 #include <cpuid.h>
@@ -92,6 +93,7 @@  const struct feature_entry rte_cpu_feature_table[] = {
 	FEAT_DEF(AVX, 0x00000001, 0, RTE_REG_ECX, 28)
 	FEAT_DEF(F16C, 0x00000001, 0, RTE_REG_ECX, 29)
 	FEAT_DEF(RDRAND, 0x00000001, 0, RTE_REG_ECX, 30)
+	FEAT_DEF(HYPERVISOR, 0x00000001, 0, RTE_REG_ECX, 31)
 
 	FEAT_DEF(FPU, 0x00000001, 0, RTE_REG_EDX,  0)
 	FEAT_DEF(VME, 0x00000001, 0, RTE_REG_EDX,  1)
@@ -194,3 +196,31 @@  rte_cpu_get_flag_name(enum rte_cpu_flag_t feature)
 		return NULL;
 	return rte_cpu_feature_table[feature].name;
 }
+
+/* See http://lwn.net/Articles/301888/ */
+#define HYPERVISOR_INFO_LEAF 0x40000000
+
+enum rte_hypervisor
+rte_hypervisor_get_name(void)
+{
+	cpuid_registers_t regs;
+	char name[13];
+
+	if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_HYPERVISOR))
+		return RTE_HYPERVISOR_NONE;
+
+	__cpuid(HYPERVISOR_INFO_LEAF,
+			regs[RTE_REG_EAX], regs[RTE_REG_EBX],
+			regs[RTE_REG_ECX], regs[RTE_REG_EDX]);
+	for (int reg = 1; reg < 4; reg++)
+		memcpy(name + (reg - 1) * 4, &regs[reg], 4);
+	name[12] = '\0';
+
+	if (strcmp("KVMKVMKVM", name) == 0)
+		return RTE_HYPERVISOR_KVM;
+	if (strcmp("Microsoft Hv", name) == 0)
+		return RTE_HYPERVISOR_HYPERV;
+	if (strcmp("VMwareVMware", name) == 0)
+		return RTE_HYPERVISOR_VMWARE;
+	return RTE_HYPERVISOR_UNKNOWN;
+}
diff --git a/lib/librte_eal/common/include/arch/x86/rte_cpuflags.h b/lib/librte_eal/common/include/arch/x86/rte_cpuflags.h
index 26204fabb..686ac07f3 100644
--- a/lib/librte_eal/common/include/arch/x86/rte_cpuflags.h
+++ b/lib/librte_eal/common/include/arch/x86/rte_cpuflags.h
@@ -69,6 +69,7 @@  enum rte_cpu_flag_t {
 	RTE_CPUFLAG_AVX,                    /**< AVX */
 	RTE_CPUFLAG_F16C,                   /**< F16C */
 	RTE_CPUFLAG_RDRAND,                 /**< RDRAND */
+	RTE_CPUFLAG_HYPERVISOR,             /**< Running on a hypervisor */
 
 	/* (EAX 01h) EDX features */
 	RTE_CPUFLAG_FPU,                    /**< FPU */
diff --git a/lib/librte_eal/common/include/generic/rte_cpuflags.h b/lib/librte_eal/common/include/generic/rte_cpuflags.h
index c1c5551fc..3832fb851 100644
--- a/lib/librte_eal/common/include/generic/rte_cpuflags.h
+++ b/lib/librte_eal/common/include/generic/rte_cpuflags.h
@@ -93,4 +93,18 @@  rte_cpu_check_supported(void);
 int
 rte_cpu_is_supported(void);
 
+enum rte_hypervisor {
+	RTE_HYPERVISOR_NONE,
+	RTE_HYPERVISOR_KVM,
+	RTE_HYPERVISOR_HYPERV,
+	RTE_HYPERVISOR_VMWARE,
+	RTE_HYPERVISOR_UNKNOWN
+};
+
+/**
+ * Get the type of hypervisor it is running on.
+ */
+enum rte_hypervisor
+rte_hypervisor_get_name(void);
+
 #endif /* _RTE_CPUFLAGS_H_ */
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index f4f46c1be..3f8d81577 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -200,6 +200,13 @@  DPDK_17.11 {
 
 } DPDK_17.08;
 
+DPDK_18.02 {
+	global:
+
+	rte_hypervisor_get_name;
+
+} DPDK_17.11;
+
 EXPERIMENTAL {
 	global:
 
@@ -235,4 +242,4 @@  EXPERIMENTAL {
 	rte_service_set_stats_enable;
 	rte_service_start_with_defaults;
 
-} DPDK_17.11;
+} DPDK_18.02;