Re: [Qemu-devel] [Qemu-ppc] [PATCH 01/18] target-ppc: Add Flag for Power ISA V2.06

[Tom Musta <tommusta@gmail.com>]

On 12/18/2013 4:37 PM, Alexander Graf wrote:
> 
> 
>> Am 18.12.2013 um 23:11 schrieb Scott Wood <scottwood@freescale.com>:
>>
>>> On Wed, 2013-12-18 at 23:09 +0100, Alexander Graf wrote:
>>>> On 18.12.2013, at 23:02, Scott Wood <scottwood@freescale.com> wrote:
>>>>
>>>>> On Mon, 2013-12-09 at 09:46 -0600, Tom Musta wrote:
>>>>> This patch adds a flag for base instruction additions to Power ISA
>>>>> 2.06B.  The flag will be used to identify/select basic Book I and
>>>>> Book II instructions that were newly added in that revision of the
>>>>> architecture.  The flag will not be used for VSX or Altivec.
>>>>>
>>>>> Signed-off-by: Tom Musta <tommusta@gmail.com>
>>>>> ---
>>>>> target-ppc/cpu.h            |    4 +++-
>>>>> target-ppc/translate_init.c |    6 ++++--
>>>>> 2 files changed, 7 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
>>>>> index 0abc848..fe3aace 100644
>>>>> --- a/target-ppc/cpu.h
>>>>> +++ b/target-ppc/cpu.h
>>>>> @@ -1877,9 +1877,11 @@ enum {
>>>>>    PPC2_ISA205        = 0x0000000000000020ULL,
>>>>>    /* VSX additions in ISA 2.07                                             */
>>>>>    PPC2_VSX207        = 0x0000000000000040ULL,
>>>>> +    /* Book I 2.06B PowerPC specification (base instructions)                */
>>>>> +    PPC2_ISA206        = 0x0000000000000080ULL,
>>>>>
>>>>> #define PPC_TCG_INSNS2 (PPC2_BOOKE206 | PPC2_VSX | PPC2_PRCNTL | PPC2_DBRX | \
>>>>> -                        PPC2_ISA205 | PPC2_VSX207)
>>>>> +                        PPC2_ISA205 | PPC2_VSX207 | PPC2_ISA206)
>>>>> };
>>>>>
>>>>> /*****************************************************************************/
>>>>> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
>>>>> index e14ab63..491e56c 100644
>>>>> --- a/target-ppc/translate_init.c
>>>>> +++ b/target-ppc/translate_init.c
>>>>> @@ -7234,7 +7234,8 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
>>>>>                       PPC_64B | PPC_ALTIVEC |
>>>>>                       PPC_SEGMENT_64B | PPC_SLBI |
>>>>>                       PPC_POPCNTB | PPC_POPCNTWD;
>>>>> -    pcc->insns_flags2 = PPC2_VSX | PPC2_DFP | PPC2_DBRX | PPC2_ISA205;
>>>>> +    pcc->insns_flags2 = PPC2_VSX | PPC2_DFP | PPC2_DBRX | PPC2_ISA205 |
>>>>> +                        PPC2_ISA206;
>>>>>    pcc->msr_mask = 0x800000000284FF37ULL;
>>>>>    pcc->mmu_model = POWERPC_MMU_2_06;
>>>>> #if defined(CONFIG_SOFTMMU)
>>>>> @@ -7270,7 +7271,8 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
>>>>>                       PPC_64B | PPC_ALTIVEC |
>>>>>                       PPC_SEGMENT_64B | PPC_SLBI |
>>>>>                       PPC_POPCNTB | PPC_POPCNTWD;
>>>>> -    pcc->insns_flags2 = PPC2_VSX | PPC2_VSX207 | PPC2_DFP | PPC2_DBRX;
>>>>> +    pcc->insns_flags2 = PPC2_VSX | PPC2_VSX207 | PPC2_DFP | PPC2_DBRX |
>>>>> +                        PPC2_ISA206;
>>>>>    pcc->msr_mask = 0x800000000284FF36ULL;
>>>>>    pcc->mmu_model = POWERPC_MMU_2_06;
>>>>> #if defined(CONFIG_SOFTMMU)
>>>>
>>>> e500mc/e5500 are also ISA 2.06 (minus some of these new instructions,
>>>> but not all of them -- better to emulate instructions that don't exist
>>>> than to not emulate instructions that do exist).
>>>
>>> Well, that's what the nice categories in the ISA are there for, no?
>>> They should tell us quite explicitly which instructions are available
>>> in embedded and which are not.
>>
>> They should, but they don't.  In the core reference manual for e500mc
>> and derivatives, it lists the ISA categories supported... and then has a
>> table that says "oh, except for these instructions".
> 
> Hooray :). Can we please split the feature bit nevertheless?
> 
> Alex
> 
>>
>> -Scott
>>
>>

It turns out that all of the instructions added in this series are *not* implemented
in e500mc.  So the patch, as is, is correct for e500mc as well as P7/P8.

The problem with this patch series seems to be that previously implemented ISA2.06
instructions were given their own flags (e.g. PPC_POPCNTWD).  But this series attempted
to use a single flag for all of base ISA 2.06.

Based on Alex's suggestion, I propose that I re-implement, eliminating PPC2_ISA206 and
replacing it with the following new flags:

  - PPC2_PERMUTE_206 (bpermd)
  - PPC2_DIV_EXT_206 (div[wd]e[u])
  - PPC2_ATOMIC_206 (l[bh]arx, st[bh]cx.)
  - PPC2_FP_CVT_206 (fcfid*, fcti[dw]u[z], lfiwzx)
  - PPC2_FP_TEST_206 (ftdiv, ftsqrt)

Comments?