Re: [RESEND] target/riscv: fix RV128 lq encoding

[Christoph Müllner]

Hi Frédéric,

you are right, I misunderstood the "LQ is added to the MISC-MEM major
opcode" part of the spec.
I saw the encoding conflict with the CBO instructions and thought of a
bug in qemu's LQ encoding.
Philipp already highlighted that cbo.* instructions are actually LQ with rd=0.

Thanks,
Christoph

On Wed, Jan 19, 2022 at 8:45 PM Frédéric Pétrot
<frederic.petrot@univ-grenoble-alpes.fr> wrote:
>
> Le 18/01/2022 à 17:32, Christoph Muellner a écrit :
> > If LQ has func3==010 and is located in the MISC-MEM opcodes,
> > then it conflicts with the CBO opcode space.
> > However, since LQ is specified as: "LQ is added to the MISC-MEM major
> > opcode", we have an implementation bug, because 'major opcode'
> > refers to func3, which must be 111.
> >
> > This results in the following instruction encodings:
> >
> > lq        ........ ........ .111.... .0001111
> > cbo_clean 00000000 0001.... .0100000 00001111
> > cbo_flush 00000000 0010.... .0100000 00001111
> > cbo_inval 00000000 0000.... .0100000 00001111
> > cbo_zero  00000000 0100.... .0100000 00001111
> >                               ^^^-func3
> >                                        ^^^^^^^-opcode
>
>    Hello Christoph,
>    I see page table 26.1 of the last riscv-isa-manual.pdf what is called major
>    opcodes in my understanding, and MISC-MEM is one of them with value 
> 00_111_11.
>    The value for func3 that I chose comes from
>    https://github.com/michaeljclark/riscv-meta/blob/master/opcodes
>    which admittedly is out-dated, but I don't see any particular value for
>    LQ/SQ in the new spec either (I mean, riscv-isa-manual.pdf, any pointer we
>    could refer to ?).
>    I have nothing against changing the opcode, but then we need to change
>    disas/riscv.c which also uses the previous opcode to dump instructions when
>    running with -d in_asm.
>
>    Frédéric
> >
> > Signed-off-by: Christoph Muellner <cmuellner@linux.com>
> > ---
> >   target/riscv/insn32.decode | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
> > index 5bbedc254c..d3f798ca10 100644
> > --- a/target/riscv/insn32.decode
> > +++ b/target/riscv/insn32.decode
> > @@ -168,7 +168,7 @@ sraw     0100000 .....  ..... 101 ..... 0111011 @r
> >
> >   # *** RV128I Base Instruction Set (in addition to RV64I) ***
> >   ldu      ............   ..... 111 ..... 0000011 @i
> > -lq       ............   ..... 010 ..... 0001111 @i
> > +lq       ............   ..... 111 ..... 0001111 @i
> >   sq       ............   ..... 100 ..... 0100011 @s
> >   addid    ............  .....  000 ..... 1011011 @i
> >   sllid    000000 ......  ..... 001 ..... 1011011 @sh6
>
> --
> +---------------------------------------------------------------------------+
> | Frédéric Pétrot, Pr. Grenoble INP-Ensimag/TIMA,   Ensimag deputy director |
> | Mob/Pho: +33 6 74 57 99 65/+33 4 76 57 48 70      Ad augusta  per angusta |
> | http://tima.univ-grenoble-alpes.fr frederic.petrot@univ-grenoble-alpes.fr |
> +---------------------------------------------------------------------------+