Re: [PATCH v4 6/9] target/ppc: Add IBM PPE42 special instructions

[Thomas Huth <thuth@redhat.com>]

On 17/09/2025 18.07, BALATON Zoltan wrote:
> On Wed, 17 Sep 2025, Miles Glenn wrote:
>> On Wed, 2025-09-17 at 08:20 +0200, Thomas Huth wrote:
>>> On 17/09/2025 06.57, Harsh Prateek Bora wrote:
>>>>
>>>> On 9/12/25 22:17, Glenn Miles wrote:
>>>>> Adds the following instructions exclusively for
>>>>> IBM PPE42 processors:
>>>>>
>>>>>    LSKU
>>>>>    LCXU
>>>>>    STSKU
>>>>>    STCXU
>>>>>    LVD
>>>>>    LVDU
>>>>>    LVDX
>>>>>    STVD
>>>>>    STVDU
>>>>>    STVDX
>>>>>    SLVD
>>>>>    SRVD
>>>>>    CMPWBC
>>>>>    CMPLWBC
>>>>>    CMPWIBC
>>>>>    BNBWI
>>>>>    BNBW
>>>>>    CLRBWIBC
>>>>>    CLRWBC
>>>>>    DCBQ
>>>>>    RLDICL
>>>>>    RLDICR
>>>>>    RLDIMI
>>>>>
>>>>> A PPE42 GCC compiler is available here:
>>>>> https://github.com/open-power/ppe42-gcc
>>>>>
>>>>> For more information on the PPE42 processors please visit:
>>>>> https://wiki.raptorcs.com/w/images/a/a3/PPE_42X_Core_Users_Manual.pdf
>>>>>
>>>>> Signed-off-by: Glenn Miles <milesg@linux.ibm.com>
>>>>> ---
>>>>> Changes from v3:
>>>>>    - Removed copy of CHECK_VDR
>>>>>    - Refactored ld/st instructions
>>>>>
>>>>>   target/ppc/insn32.decode            |  66 ++-
>>>>>   target/ppc/translate.c              |  29 +-
>>>>>   target/ppc/translate/ppe-impl.c.inc | 665 ++++++++++++++++++++++++++++
>>>>>   3 files changed, 750 insertions(+), 10 deletions(-)
>>>>>   create mode 100644 target/ppc/translate/ppe-impl.c.inc
>>>>>
>>>>
>>>> <snip>
>>>>
>>>>> diff --git a/target/ppc/translate/ppe-impl.c.inc b/target/ppc/translate/
>>>>> ppe-impl.c.inc
>>>>> new file mode 100644
>>>>> index 0000000000..792103d7c2
>>>>> --- /dev/null
>>>>> +++ b/target/ppc/translate/ppe-impl.c.inc
>>>>> @@ -0,0 +1,665 @@
>>>>> +/*
>>>>> + * IBM PPE Instructions
>>>>> + *
>>>>> + * Copyright (c) 2025, IBM Corporation.
>>>>> + *
>>>>> + * SPDX-License-Identifier: GPL-2.0-or-later
>>>>> + */
>>>>> +
>>>>> +
>>>>> +#if !defined(TARGET_PPC64)
>>>>> +static bool vdr_is_valid(uint32_t vdr)
>>>>> +{
>>>>> +    const uint32_t valid_bitmap = 0xf00003ff;
>>>>> +    return !!((1ul << (vdr & 0x1f)) & valid_bitmap);
>>>>> +}
>>>>> +
>>>>> +static bool ppe_gpr_is_valid(uint32_t reg)
>>>>> +{
>>>>> +    const uint32_t valid_bitmap = 0xf00027ff;
>>>>> +    return !!((1ul << (reg & 0x1f)) & valid_bitmap);
>>>>> +}
>>>>> +#endif
>>>>> +
>>>>> +#define CHECK_VDR(CTX, VDR)                             \
>>>>> +    do {                                                \
>>>>> +        if (unlikely(!vdr_is_valid(VDR))) {             \
>>>>> +            gen_invalid(CTX);                           \
>>>>> +            return true;                                \
>>>>> +        }                                               \
>>>>> +    } while (0)
>>>>> +
>>>>> +#define CHECK_PPE_GPR(CTX, REG)                         \
>>>>> +    do {                                                \
>>>>> +        if (unlikely(!ppe_gpr_is_valid(REG))) {         \
>>>>> +            gen_invalid(CTX);                           \
>>>>> +            return true;                                \
>>>>> +        }                                               \
>>>>> +    } while (0)
>>>>> +
>>>>> +#define VDR_PAIR_REG(VDR) (((VDR) + 1) & 0x1f)
>>>>> +
>>>>> +#define CHECK_PPE_LEVEL(CTX, LVL)                       \
>>>>> +    do {                                                \
>>>>> +        if (unlikely(!((CTX)->insns_flags2 & (LVL)))) { \
>>>>> +            gen_invalid(CTX);                           \
>>>>> +            return true;                                \
>>>>> +        }                                               \
>>>>> +    } while (0)
>>>>> +
>>>>> +static bool trans_LCXU(DisasContext *ctx, arg_LCXU *a)
>>>>> +{
>>>>> +#if defined(TARGET_PPC64)
>>>>> +    return false;
>>>>> +#else
>>>>
>>>> If we are building the PPE42 instructions only for !TARGET_PPC64, does
>>>> it still make it usable with qemu-system-ppc64?
>>>
>>> As explained in an earlier thread already, qemu-system-ppc64 is a superset
>>> of qemu-system-ppc. Thus the ppe42 stuff should work in qemu-system- 
>>> ppc64, too.
>>>
>>>   Thomas
>>>
>>
>> Ah, yes, I don't think I fully understood the ramifications of Thomas's
>> statements earlier.  Looks like I'll need to scrub the code to ensure
>> that PPE42 can run even if TARGET_PPC64 is defined.
>>
>> Cedric, this requires me to change my response to your request to add
>> the check for TARGET_PPC64 inside the is_ppe() function.  I will need
>> to leave that function as-is if we want PPE42 to be supported in both
>> targets.  Will you be ok with that?
> 
> Does it make sense to support it with !TARGET_PPC64 if its only use is in 
> qemu-system-ppc64? Even if the CPU is 32-bit it has some 64-bit instructions 
> IIUC so does that make it TARGET_PPC64 only?

As long as the GPRs are 32 bit only (which is the case here, I assume), it's 
a 32-bit PPC CPU, isn't it? So that certainly should not go into the ppc64 
binary only.

  Thomas