From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49945) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bmLvj-0003ud-OE for qemu-devel@nongnu.org; Tue, 20 Sep 2016 10:21:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bmLve-0003zV-8R for qemu-devel@nongnu.org; Tue, 20 Sep 2016 10:21:50 -0400 Date: Tue, 20 Sep 2016 23:12:33 +1000 From: David Gibson Message-ID: <20160920131233.GF20488@umbus> References: <1471354850-5549-1-git-send-email-michael@walle.cc> <20160920022342.GH20488@umbus> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="KJKxYQkVikLexIKR" Content-Disposition: inline In-Reply-To: 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: Michael Walle Cc: Riku Voipio , Tom Musta , qemu-ppc@nongnu.org, Alexander Graf , qemu-devel@nongnu.org --KJKxYQkVikLexIKR Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Sep 20, 2016 at 08:55:09AM +0200, Michael Walle wrote: > 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. > >=20 > > Hrm.. it's not clear to me how this patch fixes things. What was > > incorrect with the previous logic? >=20 > 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. Th= is > is correct if there is only one bit set in "flag", but GET_FEATURE2 is al= so > used with multiple bits set in "flag": >=20 > GET_FEATURE2((PPC2_BCTAR_ISA207 | PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 | > PPC2_ISA207S), QEMU_PPC_FEATURE2_ARCH_2_07); Ah, right, that makes sense. >=20 > Ahh, I've just seen that Tom's mail wasn't CCed to the mailinglist: Ok. But regardless of that, this explanation needs to be in the commit message for the benefit of people looking back in future. Please resend with a commit message expanded to include the above explanation, and I expect I'll commit it. >=20 >=20 > 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. > >=20 > > Hope all is well with you, Alex! > >=20 > > On Mon, Jul 25, 2016 at 10:14 AM, Michael Walle > > wrote: > >=20 > > > Hi, > > >=20 > > > 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. > > >=20 > > > The line which sets this bit in linux-user is: > > >=20 > > > #define GET_FEATURE2(flag, feature) > > > \ > > > do { if (cpu->env.insns_flags2 & flag) { features |=3D feature; } > > > } while (0) > > >=20 > > > 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); > > >=20 > > > 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. > > >=20 > > > #define GET_FEATURES(flag, feature) > > > do { if (cpu->env.insns_flags2 & flag =3D=3D flag) { features |=3D > > > feature; } } while (0) >=20 > -michael >=20 --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --KJKxYQkVikLexIKR Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJX4TW+AAoJEGw4ysog2bOSUmkQAL6hGt+o425wK6BdGCHE8tXw 4DHOSgZvbXa1ikPuLLou/wLBOsDl1hSXE9SnfD6enmSZP0Qkz2Rd7k3vSVL1xqIN wIo+J/2mWT55W3iT55hPBWkCvcb7ZwMue765uxo88zkznP5f56JPeVRb51kVvuZl TxsOSvIGQ9uSaWPSbHuyihF3yax97bL1dTaUG7fE0cV0TiSzvNR58OG2ixtis6dG yn4tGROAo+uO93VjWJsWV7P/ZLAJGHuH03iGlIk4tzOyBFOI06ctWNSbpOyZZc3T afDbDFt2aohi3OUiZe0HocJS9vT7H9xkvea9Vj12H3yg+CrV89XbVOaSjnr4xPNy rXOzzjJpv0S1vlUXgUJsGHo1Isv1K7KHEGsvw8FkEPp2emFPzlZR3/f7oy0xoy/1 4LYjxRmbdyl1kaRaMR6Bruz2JQG0GGOYX2diWJt9GGJ1OuR2g2g7YVbwyYcV15C4 zDfb8Gp3r8JkxzPM+3MQ2SzCYx4PIzl40tSLEgraYjwPI6TGd7rQDbqymxSEpamP bIwgefpPsVFzE0+slEzmMAlBy1I+owW8pNXkt9zc5Cx4Advwn/ys186P7IFttN0X +bc3lv00B+TOO0NS3P/soGQl0iPPkmJz1xwuajatMJ/jns67jP7e32WNCu/uKGUY S1AjHms9FzS3RGNDziKb =Ml9g -----END PGP SIGNATURE----- --KJKxYQkVikLexIKR--