[PATCH v2 bpf] mips: bpf: fix encoding bug for mm_srlv32_op

[PATCH v2 bpf] mips: bpf: fix encoding bug for mm_srlv32_op

[Jiong Wang]

For micro-mips, srlv inside POOL32A encoding space should use 0x50
sub-opcode, NOT 0x90.

Some early version ISA doc describes the encoding as 0x90 for both srlv and
srav, this looks to me was a typo. I checked Binutils libopcode
implementation which is using 0x50 for srlv and 0x90 for srav.

v1->v2:
  - Keep mm_srlv32_op sorted by value.

Fixes: f31318fdf324 ("MIPS: uasm: Add srlv uasm instruction")
Cc: Markos Chandras <markos.chandras@imgtec.com>
Cc: Paul Burton <paul.burton@mips.com>
Cc: linux-mips@vger.kernel.org
Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Acked-by: Song Liu <songliubraving@fb.com>
Signed-off-by: Jiong Wang <jiong.wang@netronome.com>
---
 arch/mips/include/uapi/asm/inst.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/mips/include/uapi/asm/inst.h b/arch/mips/include/uapi/asm/inst.h
index c05dcf5..273ef58 100644
--- a/arch/mips/include/uapi/asm/inst.h
+++ b/arch/mips/include/uapi/asm/inst.h
@@ -369,8 +369,8 @@ enum mm_32a_minor_op {
 	mm_ext_op = 0x02c,
 	mm_pool32axf_op = 0x03c,
 	mm_srl32_op = 0x040,
+	mm_srlv32_op = 0x050,
 	mm_sra_op = 0x080,
-	mm_srlv32_op = 0x090,
 	mm_rotr_op = 0x0c0,
 	mm_lwxs_op = 0x118,
 	mm_addu32_op = 0x150,
-- 
2.7.4

Re: [PATCH v2 bpf] mips: bpf: fix encoding bug for mm_srlv32_op

[Paul Burton]

Hello,

Jiong Wang wrote:
> For micro-mips, srlv inside POOL32A encoding space should use 0x50
> sub-opcode, NOT 0x90.
> 
> Some early version ISA doc describes the encoding as 0x90 for both srlv and
> srav, this looks to me was a typo. I checked Binutils libopcode
> implementation which is using 0x50 for srlv and 0x90 for srav.
> 
> v1->v2:
> - Keep mm_srlv32_op sorted by value.
> 
> Fixes: f31318fdf324 ("MIPS: uasm: Add srlv uasm instruction")
> Cc: Markos Chandras <markos.chandras@imgtec.com>
> Cc: Paul Burton <paul.burton@mips.com>
> Cc: linux-mips@vger.kernel.org
> Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Acked-by: Song Liu <songliubraving@fb.com>
> Signed-off-by: Jiong Wang <jiong.wang@netronome.com>

Applied to mips-fixes.

Thanks,
    Paul

[ This message was auto-generated; if you believe anything is incorrect
  then please email paul.burton@mips.com to report it. ]

Re: [PATCH v2 bpf] mips: bpf: fix encoding bug for mm_srlv32_op

[Jakub Kicinski]

On Mon, 3 Dec 2018 22:42:04 +0000, Paul Burton wrote:
> Jiong Wang wrote:
> > For micro-mips, srlv inside POOL32A encoding space should use 0x50
> > sub-opcode, NOT 0x90.
> > 
> > Some early version ISA doc describes the encoding as 0x90 for both srlv and
> > srav, this looks to me was a typo. I checked Binutils libopcode
> > implementation which is using 0x50 for srlv and 0x90 for srav.
> > 
> > v1->v2:
> > - Keep mm_srlv32_op sorted by value.
> > 
> > Fixes: f31318fdf324 ("MIPS: uasm: Add srlv uasm instruction")
> > Cc: Markos Chandras <markos.chandras@imgtec.com>
> > Cc: Paul Burton <paul.burton@mips.com>
> > Cc: linux-mips@vger.kernel.org
> > Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> > Acked-by: Song Liu <songliubraving@fb.com>
> > Signed-off-by: Jiong Wang <jiong.wang@netronome.com>  
> 
> Applied to mips-fixes.

Newbie process related question - are the arch JIT patches routed via
arch trees or bpf-next?  Jiong has more (slightly conflicting) JIT
patches to send - I wonder how they'll get applied and whether to wait
for the mips -> Linus -> net -> bpf merge chain.

Re: [PATCH v2 bpf] mips: bpf: fix encoding bug for mm_srlv32_op

[Paul Burton]

Hi Jakub,

On Mon, Dec 03, 2018 at 03:55:45PM -0800, Jakub Kicinski wrote:
> On Mon, 3 Dec 2018 22:42:04 +0000, Paul Burton wrote:
> > Jiong Wang wrote:
> > > For micro-mips, srlv inside POOL32A encoding space should use 0x50
> > > sub-opcode, NOT 0x90.
> > > 
> > > Some early version ISA doc describes the encoding as 0x90 for both srlv and
> > > srav, this looks to me was a typo. I checked Binutils libopcode
> > > implementation which is using 0x50 for srlv and 0x90 for srav.
> > > 
> > > v1->v2:
> > > - Keep mm_srlv32_op sorted by value.
> > > 
> > > Fixes: f31318fdf324 ("MIPS: uasm: Add srlv uasm instruction")
> > > Cc: Markos Chandras <markos.chandras@imgtec.com>
> > > Cc: Paul Burton <paul.burton@mips.com>
> > > Cc: linux-mips@vger.kernel.org
> > > Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> > > Acked-by: Song Liu <songliubraving@fb.com>
> > > Signed-off-by: Jiong Wang <jiong.wang@netronome.com>  
> > 
> > Applied to mips-fixes.
> 
> Newbie process related question - are the arch JIT patches routed via
> arch trees or bpf-next?  Jiong has more (slightly conflicting) JIT
> patches to send - I wonder how they'll get applied and whether to wait
> for the mips -> Linus -> net -> bpf merge chain.

I'd expect that to be a case-by-case "what makes most sense this time?"
sort of question.

In this particular patch the code you're changing isn't specifically
BPF-related code, it's part of the MIPS uasm assembler which MIPS BPF
happens to use behind the scenes, so since it seemed like a pretty
standalone patch taking it through the MIPS tree made sense to me.

If you have related patches the best thing to do would be to submit them
together as a series. Then after the maintainers involved can see the
patches we can figure out the best way to apply them.

Thanks,
    Paul

Re: [PATCH v2 bpf] mips: bpf: fix encoding bug for mm_srlv32_op

[Jakub Kicinski]

On Tue, 4 Dec 2018 00:06:24 +0000, Paul Burton wrote:
> If you have related patches the best thing to do would be to submit them
> together as a series. Then after the maintainers involved can see the
> patches we can figure out the best way to apply them.

Right, in hindsight that could've worked better, but for netdev/bpf
patches posting fixes and features in one series is a no-no :)

I guess the best way forward would be for Jiong to post the dependent
set (BPF_ALU | BPF_ARSH support) as an RFC and then decide.  The
conflict will be trivial, yet avoidable if we wait for this to
propagate to bpf-next.