From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <hpa@linux.intel.com>
Received: from mga01.intel.com (mga01.intel.com [192.55.52.88])
 by dpdk.org (Postfix) with ESMTP id 6103F5938
 for <dev@dpdk.org>; Tue, 25 Mar 2014 18:46:11 +0100 (CET)
Received: from fmsmga001.fm.intel.com ([10.253.24.23])
 by fmsmga101.fm.intel.com with ESMTP; 25 Mar 2014 10:37:18 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="4.97,730,1389772800"; d="scan'208";a="499277320"
Received: from unknown (HELO hanvin-mobl6.amr.corp.intel.com) ([10.255.40.24])
 by fmsmga001.fm.intel.com with ESMTP; 25 Mar 2014 10:37:18 -0700
Message-ID: <5331BECE.3070601@linux.intel.com>
Date: Tue, 25 Mar 2014 10:37:18 -0700
From: "H. Peter Anvin" <hpa@linux.intel.com>
User-Agent: Mozilla/5.0 (X11; Linux x86_64;
 rv:24.0) Gecko/20100101 Thunderbird/24.3.0
MIME-Version: 1.0
To: Neil Horman <nhorman@tuxdriver.com>, dev@dpdk.org
References: <20140320163921.GC7721@hmsreliant.think-freely.org>
 <1395767000-28709-1-git-send-email-nhorman@tuxdriver.com>
In-Reply-To: <1395767000-28709-1-git-send-email-nhorman@tuxdriver.com>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Subject: Re: [dpdk-dev] [PATCH v4] eal_common_cpuflags: Fix %rbx corruption,
 and simplify the code
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Tue, 25 Mar 2014 17:46:11 -0000

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