| Message ID | 1543876074-4372-1-git-send-email-jiong.wang@netronome.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v2,bpf] mips: bpf: fix encoding bug for mm_srlv32_op | expand |
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. ]
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.
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
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.
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,
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(-)