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. > > > > 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);
Ah, right, that makes sense.
>
> 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.
>
>
> 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 <mich...@walle.cc>
> > 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
>
--
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
signature.asc
Description: PGP signature
