diff mbox series

eal/x86: add vendor ID checks for specific implementation

Message ID 20231109052820.2129745-1-sivaprasad.tummala@amd.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series eal/x86: add vendor ID checks for specific implementation |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/github-robot: build success github build: passed
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-sample-apps-testing success Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/intel-Functional success Functional PASS

Commit Message

Sivaprasad Tummala Nov. 9, 2023, 5:28 a.m. UTC 
  Current get_tsc_freq_arch() implementation is specific for
Intel processors.

Added vendor checks to gracefully return on AMD EPYC processors.

Signed-off-by: Sivaprasad Tummala <sivaprasad.tummala@amd.com>
---
 lib/eal/x86/rte_cycles.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

Comments

Ferruh Yigit Nov. 22, 2023, 11:25 a.m. UTC | #1 
On 11/9/2023 5:28 AM, Sivaprasad Tummala wrote:
> Current get_tsc_freq_arch() implementation is specific for
> Intel processors.
> 
> Added vendor checks to gracefully return on AMD EPYC processors.
> 

Hi Siva,

Is this fixing an issue in AMD platform, if so can you please describe
the impact of the issue and add fixes tag?


> Signed-off-by: Sivaprasad Tummala <sivaprasad.tummala@amd.com>
> ---
>  lib/eal/x86/rte_cycles.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/lib/eal/x86/rte_cycles.c b/lib/eal/x86/rte_cycles.c
> index 69ed59b4f0..f147a5231d 100644
> --- a/lib/eal/x86/rte_cycles.c
> +++ b/lib/eal/x86/rte_cycles.c
> @@ -10,6 +10,10 @@
>  #include <cpuid.h>
>  #endif
>  
> +#define x86_vendor_amd(t1, t2, t3)        \
> +	((t1 == 0x68747541) && /* htuA */   \
> +	 (t2 == 0x444d4163) && /* DMAc */   \
> +	 (t3 == 0x69746e65))   /* itne */
>  
>  #include "eal_private.h"
>  
> @@ -110,6 +114,18 @@ get_tsc_freq_arch(void)
>  	uint8_t mult, model;
>  	int32_t ret;
>  
> +#ifdef RTE_TOOLCHAIN_MSVC
> +	__cpuid(cpuinfo, 0);
>

We already discussed in the past to abstract the cpuid(), even David
sent a patch for it [1].

If this is customer facing issue, OK to get it as it is for the release,
but in long term I think better idea to switch to abstract.


[1]
https://patchwork.dpdk.org/project/dpdk/list/?series=29605&state=*


> +	a = cpuinfo[0];
> +	b = cpuinfo[1];
> +	c = cpuinfo[2];
> +	d = cpuinfo[3];
> +#else
> +	__cpuid(0, a, b, c, d);
> +#endif
> +	if (x86_vendor_amd(b, c, d))
> +		return 0;
> +
>  	/*
>  	 * Time Stamp Counter and Nominal Core Crystal Clock
>  	 * Information Leaf
Sivaprasad Tummala Nov. 22, 2023, 5:45 p.m. UTC | #2 
[AMD Official Use Only - General]

Hi Ferruh,

> -----Original Message-----
> From: Yigit, Ferruh <Ferruh.Yigit@amd.com>
> Sent: Wednesday, November 22, 2023 4:56 PM
> To: Tummala, Sivaprasad <Sivaprasad.Tummala@amd.com>;
> bruce.richardson@intel.com; konstantin.v.ananyev@yandex.ru
> Cc: dev@dpdk.org; David Marchand <david.marchand@redhat.com>
> Subject: Re: [PATCH] eal/x86: add vendor ID checks for specific implementation
>
> On 11/9/2023 5:28 AM, Sivaprasad Tummala wrote:
> > Current get_tsc_freq_arch() implementation is specific for Intel
> > processors.
> >
> > Added vendor checks to gracefully return on AMD EPYC processors.
> >
>
> Hi Siva,
>
> Is this fixing an issue in AMD platform, if so can you please describe the impact of
> the issue and add fixes tag?
Yes, this patch fixes incorrect TSC issue on AMD platforms. Sure, will add the fixes tag in next version.
>
>
> > Signed-off-by: Sivaprasad Tummala <sivaprasad.tummala@amd.com>
> > ---
> >  lib/eal/x86/rte_cycles.c | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> >
> > diff --git a/lib/eal/x86/rte_cycles.c b/lib/eal/x86/rte_cycles.c index
> > 69ed59b4f0..f147a5231d 100644
> > --- a/lib/eal/x86/rte_cycles.c
> > +++ b/lib/eal/x86/rte_cycles.c
> > @@ -10,6 +10,10 @@
> >  #include <cpuid.h>
> >  #endif
> >
> > +#define x86_vendor_amd(t1, t2, t3)        \
> > +   ((t1 == 0x68747541) && /* htuA */   \
> > +    (t2 == 0x444d4163) && /* DMAc */   \
> > +    (t3 == 0x69746e65))   /* itne */
> >
> >  #include "eal_private.h"
> >
> > @@ -110,6 +114,18 @@ get_tsc_freq_arch(void)
> >     uint8_t mult, model;
> >     int32_t ret;
> >
> > +#ifdef RTE_TOOLCHAIN_MSVC
> > +   __cpuid(cpuinfo, 0);
> >
>
> We already discussed in the past to abstract the cpuid(), even David sent a patch
> for it [1].
>
> If this is customer facing issue, OK to get it as it is for the release, but in long term
> I think better idea to switch to abstract.
Yes, this fixes a customer issue. Sure, will look into this in the long term as the below patch series in "Deferred"

>
>
> [1]
> https://patchwork.dpdk.org/project/dpdk/list/?series=29605&state=*
>
>
> > +   a = cpuinfo[0];
> > +   b = cpuinfo[1];
> > +   c = cpuinfo[2];
> > +   d = cpuinfo[3];
> > +#else
> > +   __cpuid(0, a, b, c, d);
> > +#endif
> > +   if (x86_vendor_amd(b, c, d))
> > +           return 0;
> > +
> >     /*
> >      * Time Stamp Counter and Nominal Core Crystal Clock
> >      * Information Leaf
diff mbox series

Patch

diff --git a/lib/eal/x86/rte_cycles.c b/lib/eal/x86/rte_cycles.c
index 69ed59b4f0..f147a5231d 100644
--- a/lib/eal/x86/rte_cycles.c
+++ b/lib/eal/x86/rte_cycles.c
@@ -10,6 +10,10 @@ 
 #include <cpuid.h>
 #endif
 
+#define x86_vendor_amd(t1, t2, t3)        \
+	((t1 == 0x68747541) && /* htuA */   \
+	 (t2 == 0x444d4163) && /* DMAc */   \
+	 (t3 == 0x69746e65))   /* itne */
 
 #include "eal_private.h"
 
@@ -110,6 +114,18 @@  get_tsc_freq_arch(void)
 	uint8_t mult, model;
 	int32_t ret;
 
+#ifdef RTE_TOOLCHAIN_MSVC
+	__cpuid(cpuinfo, 0);
+	a = cpuinfo[0];
+	b = cpuinfo[1];
+	c = cpuinfo[2];
+	d = cpuinfo[3];
+#else
+	__cpuid(0, a, b, c, d);
+#endif
+	if (x86_vendor_amd(b, c, d))
+		return 0;
+
 	/*
 	 * Time Stamp Counter and Nominal Core Crystal Clock
 	 * Information Leaf