eal/x86: add vendor ID checks for specific implementation
Checks
Commit Message
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
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
[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
@@ -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