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