Re: [dpdk-dev] [PATCH v2] eal: fix up bad asm in rte_cpu_get_features

["H. Peter Anvin" <hpa@zytor.com>]

On 03/19/2014 05:40 PM, Neil Horman wrote:
> So after some discussion with hpa, I need to self NAK this again, apologies for
> the noise.  Theres some clean up to be done in this area, and I'm still getting
> a segfault that is in some way related to this code, but I need to dig deeper to
> understand it.
> 
> Neil

I still believe we should add the patch I posted in the previous email;
I should clean it up and put a proper header on it.

This is, if there is actually a need to feed %ebx and %edx into CPUID
(the native instruction is sensitive to %eax and %ecx, but not %ebx or
%edx.)

For reference, this is a version of CPUID I personally often use:

struct cpuid {
	unsigned int eax, ecx, edx, ebx;
};

static inline void cpuid(unsigned int leaf, unsigned int subleaf,
			 struct cpuid *out)
{
#if defined(__i386__) && defined(__PIC__)
	/* %ebx is a forbidden register */
	asm volatile("movl %%ebx,%0 ; cpuid ; xchgl %%ebx,%0"
		: "=r" (out->ebx),
		  "=a" (out->eax),
		  "=c" (out->ecx),
		  "=d" (out->edx)
		: "a" (leaf), "c" (subleaf));
#else
	asm volatile("cpuid"
		: "=b" (out->ebx),
		  "=a" (out->eax),
		  "=c" (out->ecx),
		  "=d" (out->edx)
		: "a" (leaf), "c" (subleaf));
#endif
}

... but that is a pretty significant API change.

Making it an inline lets gcc elide the entire memory structure, so that
is definitely useful.

>>
>> diff --git a/lib/librte_eal/common/eal_common_cpuflags.c
>> b/lib/librte_eal/common/eal_common_cpuflags.c
>> index 1ebf78cc2a48..6b75992fec1a 100644
>> --- a/lib/librte_eal/common/eal_common_cpuflags.c
>> +++ b/lib/librte_eal/common/eal_common_cpuflags.c
>> @@ -206,16 +206,16 @@ rte_cpu_get_features(struct cpuid_parameters_t params)
>>                     "d" (params.edx));
>>  #else
>>         asm volatile (
>> -            "mov %%ebx, %%edi\n"
>> +            "xchgl %%ebx, %1\n"
>>              "cpuid\n"
>> -            "xchgl %%ebx, %%edi;\n"
>> +            "xchgl %%ebx, %1;\n"
>>              : "=a" (eax),
>> -              "=D" (ebx),
>> +              "=r" (ebx),
>>                "=c" (ecx),
>>                "=d" (edx)
>>              /* input */
>>              : "a" (params.eax),
>> -              "D" (params.ebx),
>> +              "1" (params.ebx),
>>                "c" (params.ecx),
>>                "d" (params.edx));
>>  #endif
>>

	-hpa