From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35366) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bmEy6-0007Mt-Vr for qemu-devel@nongnu.org; Tue, 20 Sep 2016 02:55:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bmEy1-0005Yu-0w for qemu-devel@nongnu.org; Tue, 20 Sep 2016 02:55:50 -0400 MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Tue, 20 Sep 2016 08:55:09 +0200 From: Michael Walle In-Reply-To: <20160920022342.GH20488@umbus> References: <1471354850-5549-1-git-send-email-michael@walle.cc> <20160920022342.GH20488@umbus> Message-ID: Subject: Re: [Qemu-devel] [PATCH] linux-user: ppc64: fix ARCH_206 bit in AT_HWCAP List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson Cc: Riku Voipio , Tom Musta , qemu-ppc@nongnu.org, Alexander Graf , qemu-devel@nongnu.org Am 2016-09-20 04:23, schrieb David Gibson: > On Tue, Aug 16, 2016 at 03:40:50PM +0200, Michael Walle wrote: >> Only the POWER[789] CPUs should have the ARCH_206 bit set. This is >> what the >> linux kernel does. I guess this was also the intention of commit >> 0e019746. >> We have to make sure all *206 bits are set. > > Hrm.. it's not clear to me how this patch fixes things. What was > incorrect with the previous logic? ah, i guess the patch has too few context. in the previous logic, only one bit in "flag" had to be set and the expression would evaluate to true. This is correct if there is only one bit set in "flag", but GET_FEATURE2 is also used with multiple bits set in "flag": GET_FEATURE2((PPC2_BCTAR_ISA207 | PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 | PPC2_ISA207S), QEMU_PPC_FEATURE2_ARCH_2_07); Ahh, I've just seen that Tom's mail wasn't CCed to the mailinglist: Am 2016-08-01 14:17, schrieb Tom Musta: > I took a quick look at the qemu and linux code and I think I agree > with Michael's analysis. The HWCAP bit seems to mean that the entire > ISA is supported and therefore excludes all of these implementations > (like e5500) which picked and chose some aspects of that ISA version. > > Hope all is well with you, Alex! > > On Mon, Jul 25, 2016 at 10:14 AM, Michael Walle > wrote: > >> Hi, >> >> ok this was a tough one. Programs which links to ceil() aborts with >> invalid instruction. The instruction was "frip", which isn't >> supported on e5500 or e500mc. Turns out the libc checks the AT_HWCAP >> field (dunno if that has to do with multilib support) and uses the >> optimized power5 code if the ARCH_2_06 bit is set. linux-user sets >> the ARCH_2_06 bit in case of a e5500 core, which is wrong IMHO. In >> fact the linux kernel sets this bit only for POWER[789] CPUs. >> >> The line which sets this bit in linux-user is: >> >> #define GET_FEATURE2(flag, feature) >> \ >> do { if (cpu->env.insns_flags2 & flag) { features |= feature; } >> } while (0) >> >> GET_FEATURE2((PPC2_PERM_ISA206 | PPC2_DIVE_ISA206 | >> PPC2_ATOMIC_ISA206 | >> PPC2_FP_CVT_ISA206 | PPC2_FP_TST_ISA206), >> QEMU_PPC_FEATURE_ARCH_2_06); >> >> PPC2_PERM_ISA206 is set for the e5500/e500mc core. Is this really >> intended to set the ARCH_2_06 bit if _any_ of the listed bits is set >> or should it set the ARCH_2_06 bit only if _all_ bits are set. Eg. >> >> #define GET_FEATURES(flag, feature) >> do { if (cpu->env.insns_flags2 & flag == flag) { features |= >> feature; } } while (0) -michael