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 <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

Reply via email to