eal: add umonitor umwait to x86 cpuflags

Message ID 1599841589-9150-1-git-send-email-timothy.mcdaniel@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series eal: add umonitor umwait to x86 cpuflags |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-testing success Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/travis-robot success Travis build: passed
ci/iol-mellanox-Performance success Performance Testing PASS
ci/Intel-compilation success Compilation OK

Commit Message

Timothy McDaniel Sept. 11, 2020, 4:26 p.m. UTC
  umonitor/umwait are user-level versions of the monitor/mwait
instructions. These allow the core to wait in a low-power state
until a specified cache line is accessed, a timeout occurs,
or the core is interrupted.

Signed-off-by: Timothy McDaniel <timothy.mcdaniel@intel.com>
---
 lib/librte_eal/x86/include/rte_cpuflags.h | 1 +
 lib/librte_eal/x86/rte_cpuflags.c         | 1 +
 2 files changed, 2 insertions(+)
  

Comments

Ananyev, Konstantin Sept. 11, 2020, 4:37 p.m. UTC | #1
> 
> umonitor/umwait are user-level versions of the monitor/mwait
> instructions. These allow the core to wait in a low-power state
> until a specified cache line is accessed, a timeout occurs,
> or the core is interrupted.

Isn't it a duplicate to:
https://patches.dpdk.org/patch/76554/
?

> 
> Signed-off-by: Timothy McDaniel <timothy.mcdaniel@intel.com>
> ---
>  lib/librte_eal/x86/include/rte_cpuflags.h | 1 +
>  lib/librte_eal/x86/rte_cpuflags.c         | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/lib/librte_eal/x86/include/rte_cpuflags.h b/lib/librte_eal/x86/include/rte_cpuflags.h
> index c1d2036..ab2c3b3 100644
> --- a/lib/librte_eal/x86/include/rte_cpuflags.h
> +++ b/lib/librte_eal/x86/include/rte_cpuflags.h
> @@ -130,6 +130,7 @@ enum rte_cpu_flag_t {
>  	RTE_CPUFLAG_CLDEMOTE,               /**< Cache Line Demote */
>  	RTE_CPUFLAG_MOVDIRI,                /**< Direct Store Instructions */
>  	RTE_CPUFLAG_MOVDIR64B,              /**< Direct Store Instructions 64B */
> +	RTE_CPUFLAG_UMWAIT,                 /**< UMONITOR/UMWAIT */
>  	RTE_CPUFLAG_AVX512VP2INTERSECT,     /**< AVX512 Two Register Intersection */
> 
>  	/* The last item */
> diff --git a/lib/librte_eal/x86/rte_cpuflags.c b/lib/librte_eal/x86/rte_cpuflags.c
> index 30439e7..6bed3eb 100644
> --- a/lib/librte_eal/x86/rte_cpuflags.c
> +++ b/lib/librte_eal/x86/rte_cpuflags.c
> @@ -137,6 +137,7 @@ const struct feature_entry rte_cpu_feature_table[] = {
>  	FEAT_DEF(CLDEMOTE, 0x00000007, 0, RTE_REG_ECX, 25)
>  	FEAT_DEF(MOVDIRI, 0x00000007, 0, RTE_REG_ECX, 27)
>  	FEAT_DEF(MOVDIR64B, 0x00000007, 0, RTE_REG_ECX, 28)
> +	FEAT_DEF(UMWAIT, 0x00000007, 0, RTE_REG_ECX, 5)
>  	FEAT_DEF(AVX512VP2INTERSECT, 0x00000007, 0, RTE_REG_EDX, 8)
>  };
> 
> --
> 2.6.4
  
Timothy McDaniel Sept. 14, 2020, 4:45 p.m. UTC | #2
> -----Original Message-----
> From: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Sent: Friday, September 11, 2020 11:37 AM
> To: McDaniel, Timothy <timothy.mcdaniel@intel.com>; Richardson, Bruce
> <bruce.richardson@intel.com>
> Cc: dev@dpdk.org; Carrillo, Erik G <erik.g.carrillo@intel.com>; Eads, Gage
> <gage.eads@intel.com>; Van Haaren, Harry <harry.van.haaren@intel.com>;
> Ma, Liang J <liang.j.ma@intel.com>; Burakov, Anatoly
> <anatoly.burakov@intel.com>
> Subject: RE: [PATCH] eal: add umonitor umwait to x86 cpuflags
> 
> 
> >
> > umonitor/umwait are user-level versions of the monitor/mwait
> > instructions. These allow the core to wait in a low-power state
> > until a specified cache line is accessed, a timeout occurs,
> > or the core is interrupted.
> 
> Isn't it a duplicate to:
> https://patches.dpdk.org/patch/76554/
> ?
Yes, the new rte_cpuflags.[ch] bit definitions for umonitor/umwait appear to be duplicates. I am not opposed to
dropping my patch, so long as this new bit gets defined in some other patch that makes it into 20.11.
> 
> >
> > Signed-off-by: Timothy McDaniel <timothy.mcdaniel@intel.com>
> > ---
> >  lib/librte_eal/x86/include/rte_cpuflags.h | 1 +
> >  lib/librte_eal/x86/rte_cpuflags.c         | 1 +
> >  2 files changed, 2 insertions(+)
> >
> > diff --git a/lib/librte_eal/x86/include/rte_cpuflags.h
> b/lib/librte_eal/x86/include/rte_cpuflags.h
> > index c1d2036..ab2c3b3 100644
> > --- a/lib/librte_eal/x86/include/rte_cpuflags.h
> > +++ b/lib/librte_eal/x86/include/rte_cpuflags.h
> > @@ -130,6 +130,7 @@ enum rte_cpu_flag_t {
> >  	RTE_CPUFLAG_CLDEMOTE,               /**< Cache Line Demote */
> >  	RTE_CPUFLAG_MOVDIRI,                /**< Direct Store Instructions */
> >  	RTE_CPUFLAG_MOVDIR64B,              /**< Direct Store Instructions 64B
> */
> > +	RTE_CPUFLAG_UMWAIT,                 /**< UMONITOR/UMWAIT */
> >  	RTE_CPUFLAG_AVX512VP2INTERSECT,     /**< AVX512 Two Register
> Intersection */
> >
> >  	/* The last item */
> > diff --git a/lib/librte_eal/x86/rte_cpuflags.c b/lib/librte_eal/x86/rte_cpuflags.c
> > index 30439e7..6bed3eb 100644
> > --- a/lib/librte_eal/x86/rte_cpuflags.c
> > +++ b/lib/librte_eal/x86/rte_cpuflags.c
> > @@ -137,6 +137,7 @@ const struct feature_entry rte_cpu_feature_table[] = {
> >  	FEAT_DEF(CLDEMOTE, 0x00000007, 0, RTE_REG_ECX, 25)
> >  	FEAT_DEF(MOVDIRI, 0x00000007, 0, RTE_REG_ECX, 27)
> >  	FEAT_DEF(MOVDIR64B, 0x00000007, 0, RTE_REG_ECX, 28)
> > +	FEAT_DEF(UMWAIT, 0x00000007, 0, RTE_REG_ECX, 5)
> >  	FEAT_DEF(AVX512VP2INTERSECT, 0x00000007, 0, RTE_REG_EDX, 8)
> >  };
> >
> > --
> > 2.6.4
  
Timothy McDaniel Oct. 9, 2020, 3:21 p.m. UTC | #3
> -----Original Message-----
> From: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Sent: Friday, September 11, 2020 11:37 AM
> To: McDaniel, Timothy <timothy.mcdaniel@intel.com>; Richardson, Bruce
> <bruce.richardson@intel.com>
> Cc: dev@dpdk.org; Carrillo, Erik G <erik.g.carrillo@intel.com>; Eads, Gage
> <gage.eads@intel.com>; Van Haaren, Harry <harry.van.haaren@intel.com>;
> Ma, Liang J <liang.j.ma@intel.com>; Burakov, Anatoly
> <anatoly.burakov@intel.com>
> Subject: RE: [PATCH] eal: add umonitor umwait to x86 cpuflags
> 
> 
> >
> > umonitor/umwait are user-level versions of the monitor/mwait
> > instructions. These allow the core to wait in a low-power state
> > until a specified cache line is accessed, a timeout occurs,
> > or the core is interrupted.
> 
> Isn't it a duplicate to:
> https://patches.dpdk.org/patch/76554/
> ?
> 
> >
> > Signed-off-by: Timothy McDaniel <timothy.mcdaniel@intel.com>
> > ---
> >  lib/librte_eal/x86/include/rte_cpuflags.h | 1 +
> >  lib/librte_eal/x86/rte_cpuflags.c         | 1 +
> >  2 files changed, 2 insertions(+)
> >
> > diff --git a/lib/librte_eal/x86/include/rte_cpuflags.h
> b/lib/librte_eal/x86/include/rte_cpuflags.h
> > index c1d2036..ab2c3b3 100644
> > --- a/lib/librte_eal/x86/include/rte_cpuflags.h
> > +++ b/lib/librte_eal/x86/include/rte_cpuflags.h
> > @@ -130,6 +130,7 @@ enum rte_cpu_flag_t {
> >  	RTE_CPUFLAG_CLDEMOTE,               /**< Cache Line Demote */
> >  	RTE_CPUFLAG_MOVDIRI,                /**< Direct Store Instructions */
> >  	RTE_CPUFLAG_MOVDIR64B,              /**< Direct Store Instructions 64B
> */
> > +	RTE_CPUFLAG_UMWAIT,                 /**< UMONITOR/UMWAIT */
> >  	RTE_CPUFLAG_AVX512VP2INTERSECT,     /**< AVX512 Two Register
> Intersection */
> >
> >  	/* The last item */
> > diff --git a/lib/librte_eal/x86/rte_cpuflags.c b/lib/librte_eal/x86/rte_cpuflags.c
> > index 30439e7..6bed3eb 100644
> > --- a/lib/librte_eal/x86/rte_cpuflags.c
> > +++ b/lib/librte_eal/x86/rte_cpuflags.c
> > @@ -137,6 +137,7 @@ const struct feature_entry rte_cpu_feature_table[] = {
> >  	FEAT_DEF(CLDEMOTE, 0x00000007, 0, RTE_REG_ECX, 25)
> >  	FEAT_DEF(MOVDIRI, 0x00000007, 0, RTE_REG_ECX, 27)
> >  	FEAT_DEF(MOVDIR64B, 0x00000007, 0, RTE_REG_ECX, 28)
> > +	FEAT_DEF(UMWAIT, 0x00000007, 0, RTE_REG_ECX, 5)
> >  	FEAT_DEF(AVX512VP2INTERSECT, 0x00000007, 0, RTE_REG_EDX, 8)
> >  };
> >
> > --
> > 2.6.4

With regards to the dependency on part of the power patch set (first patch in the WAITPKG patch series  http://patches.dpdk.org/project/dpdk/list/?series=12669 ), we understand the power team are addressing any outstanding issues today.
This should allow the DLB (https://patches.dpdk.org/project/dpdk/list/?series=12163) and DLB2 (https://patches.dpdk.org/project/dpdk/list/?series=12164) patch sets to make use of the CPU ID patch as planned.
Let us know if there is anything else outstanding. We are currently working towards the PMD merge deadline date.

Thanks, Tim
  

Patch

diff --git a/lib/librte_eal/x86/include/rte_cpuflags.h b/lib/librte_eal/x86/include/rte_cpuflags.h
index c1d2036..ab2c3b3 100644
--- a/lib/librte_eal/x86/include/rte_cpuflags.h
+++ b/lib/librte_eal/x86/include/rte_cpuflags.h
@@ -130,6 +130,7 @@  enum rte_cpu_flag_t {
 	RTE_CPUFLAG_CLDEMOTE,               /**< Cache Line Demote */
 	RTE_CPUFLAG_MOVDIRI,                /**< Direct Store Instructions */
 	RTE_CPUFLAG_MOVDIR64B,              /**< Direct Store Instructions 64B */
+	RTE_CPUFLAG_UMWAIT,                 /**< UMONITOR/UMWAIT */
 	RTE_CPUFLAG_AVX512VP2INTERSECT,     /**< AVX512 Two Register Intersection */
 
 	/* The last item */
diff --git a/lib/librte_eal/x86/rte_cpuflags.c b/lib/librte_eal/x86/rte_cpuflags.c
index 30439e7..6bed3eb 100644
--- a/lib/librte_eal/x86/rte_cpuflags.c
+++ b/lib/librte_eal/x86/rte_cpuflags.c
@@ -137,6 +137,7 @@  const struct feature_entry rte_cpu_feature_table[] = {
 	FEAT_DEF(CLDEMOTE, 0x00000007, 0, RTE_REG_ECX, 25)
 	FEAT_DEF(MOVDIRI, 0x00000007, 0, RTE_REG_ECX, 27)
 	FEAT_DEF(MOVDIR64B, 0x00000007, 0, RTE_REG_ECX, 28)
+	FEAT_DEF(UMWAIT, 0x00000007, 0, RTE_REG_ECX, 5)
 	FEAT_DEF(AVX512VP2INTERSECT, 0x00000007, 0, RTE_REG_EDX, 8)
 };