Re: [PATCH v4] eal_common_cpuflags: Fix %rbx corruption, and simplify the code

["H. Peter Anvin" <hpa-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>]

On 03/25/2014 10:03 AM, Neil Horman wrote:
>  int
>  rte_cpu_get_flag_enabled(enum rte_cpu_flag_t feature)
>  {
> -	int value;
> +	const struct feature_entry *feat;
> +	cpuid_registers_t regs;
> +	static uint32_t max_leaf = 0;
> +
> +	if (!max_leaf) {
> +		/* Get the max input leaf for this processor */
> +		rte_cpu_get_features(0, 0, regs);
> +		max_leaf = regs[REG_EAX];
> +	}
>  
>  	if (feature >= RTE_CPUFLAG_NUMFLAGS)
>  		/* Flag does not match anything in the feature tables */
>  		return -ENOENT;
>  
> -	/* get value of the register containing the desired feature */
> -	value = rte_cpu_get_features(cpu_feature_table[feature].params);
> +	feat = &cpu_feature_table[feature];
> +
> +	if (!feat->leaf)
> +		/* This entry in the table wasn't filled out! */
> +		return -EFAULT;

> +	if (feat->leaf > max_leaf)
> +		return -EINVAL;

This doesn't quite work.  The max_leaf is per CPUID "group", i.e. the
8000xxxx CPUID leaves have a different limit than 0000xxxx leaves.  So I
would just do this as:

	rte_cpu_get_features(feat->leaf & 0xffff0000, 0, regs);
	if (((regs[REG_EAX] ^ feat->leaf) & 0xffff0000) ||
	    regs[REG_EAX] < feat->leaf)
		return 0;

Returning 0 is the right thing, because this is a legitimate instance of
"this feature is not supported."

The first part is a sanity check that the CPUID leaf group is supported
at all; the second part is the actual limit check.

	-hpa