[RESEND] target/riscv: fix RV128 lq encoding

[RESEND] target/riscv: fix RV128 lq encoding

[Christoph Muellner]

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

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
-- 
2.34.1

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

[Frédéric Pétrot]

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 |
+---------------------------------------------------------------------------+

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 |
> +---------------------------------------------------------------------------+

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

[Philipp Tomsich]

[-- Attachment #1: Type: text/plain, Size: 2709 bytes --]

The cbo.* mnemonics share their opcode space with lq for those cases where
rd == 0 ("brownfield" encodings).
"Major opcode" refers to inst[6:0] according to chapter 26.

In overlapping multi-group syntax, this would look like:

> {
>
>   # *** RV32 Zicbom Standard Extension ***
>
>   cbo_clean  0000000 00001 ..... 010 00000 0001111 @sfence_vm
>
>   cbo_flush  0000000 00010 ..... 010 00000 0001111 @sfence_vm
>
>   cbo_inval  0000000 00000 ..... 010 00000 0001111 @sfence_vm
>
>
>   # *** RV32 Zicboz Standard Extension ***
>
>   cbo_zero   0000000 00100 ..... 010 00000 0001111 @sfence_vm
>
>
>   # *** RVI128 lq ***
>
>   lq       ............   ..... 010 ..... 0001111 @i
>
> }
>

Instead of using a multigroup here, I would recommend that you take a look
at https://patchwork.kernel.org/project/qemu-devel/list/?series=605340
where we have added a table of optional decoders — this could be used to
split these off into separate decoders that are run before the regular
decoder, if & only if Zicboc and/or Zicboz are enabled.

Cheers,
Philipp.


On Tue, 18 Jan 2022 at 17:32, Christoph Muellner <cmuellner@linux.com>
wrote:

> 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
>
> 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
> --
> 2.34.1
>
>

[-- Attachment #2: Type: text/html, Size: 7593 bytes --]