Re: [PATCH] eal/x86: add vendor ID checks for specific implementation

[Ferruh Yigit <ferruh.yigit@amd.com>]

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