On 3/27/2014 9:58 AM, Andreas Färber wrote:
> Am 25.03.2014 08:29, schrieb Alex Bennée:
>>
>> Anton Blanchard <an...@samba.org> writes:
>>
>>> Add MSR_LE to the msr_mask for POWER8.
>>>
>> <snip>
>>> - pcc->msr_mask = 0x800000000284FF36ULL;
>>> + pcc->msr_mask = 0x800000000284FF37ULL;
>> <snip>
>>
>> Should we be adding some #define's for the various bit positions on this
>> mask? Looking at the current code it looks like a big ream of magic
>> numbers.
>
> In general I concur that defines would be nice, however for 2.0 that's
> too risky for me as temporary maintainer and I'm not sure if these
> values are being pieced together by contributors or whether this is
> coming directly from the manual?
There are defines in target-ppc/cpu.h that identify the bit position. So
something like this could/should be done (in 2.1):
pcc->msr_mask =
(1ULL << MSR_SF) |
(1ULL << MSR_TAG) |
(1ULL << MSR_ISF) |
(1ULL << MSR_DR /* MISSING */ ) |
(1ULL << MSR_IR /* MISSING */ ) |
(1ULL << MSR_FE1 /* MISSING */ ) |
(1ULL << MSR_BE /* MISSING */ ) |
(1ULL << MSR_ME /* MISSING */ ) |
(1ULL << MSR_SE /* MISSING */ ) |
(1ULL << MSR_FE0 /* MISSING */ ) |
(1ULL << MSR_FP /* MISSING */ ) |
(1ULL << MSR_PR /* MISSING */ ) |
(1ULL << MSR_EE /* MISSING */ );
The set of defines is incomplete -- the items marked MISSING are not currently
there.
They could, of course, easily be added.
Unfortunately, this behavior has been repeated over 50 times in the
target-ppc/translate_init.c
file and some MSR bit positions have multiple meanings based on processor
family and era. So
a thorough and accurate cleanup would provide a nice challenge :)
I'm willing to tackle this after I get done with some decimal floating point
work (probably 2
weeks).
> The other issue has been that adding a new family, even after the
> initial round of cleanups, still requires a chunk of code to be copied,
> which seems prone to forgetting little bits on the new one, then maybe
> fixing up the original template but not the derived models, etc.
