From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.zytor.com (terminus.zytor.com [198.137.202.10]) by dpdk.org (Postfix) with ESMTP id B6DA158F7 for ; Thu, 20 Mar 2014 05:20:41 +0100 (CET) Received: from tazenda.hos.anvin.org ([IPv6:2601:9:7280:8f0:cc79:79ff:fead:f559]) (authenticated bits=0) by mail.zytor.com (8.14.7/8.14.5) with ESMTP id s2K4M8iN028082 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Wed, 19 Mar 2014 21:22:10 -0700 Message-ID: <532A6CEB.1070106@zytor.com> Date: Wed, 19 Mar 2014 21:22:03 -0700 From: "H. Peter Anvin" 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 , dev@dpdk.org References: <1395175414-25232-1-git-send-email-nhorman@tuxdriver.com> <1395240524-412-1-git-send-email-nhorman@tuxdriver.com> <5329BB6E.8080509@zytor.com> <20140320004010.GA20693@neilslaptop.think-freely.org> In-Reply-To: <20140320004010.GA20693@neilslaptop.think-freely.org> X-Enigmail-Version: 1.6 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v2] eal: fix up bad asm in rte_cpu_get_features X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 20 Mar 2014 04:20:42 -0000 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