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

[Miles Glenn <milesg@linux.ibm.com>]

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?

Thanks,

Glenn