From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Sun, 8 Oct 2017 21:04:05 -0700
Subject: nfp: bpf: use the power of sparse to check we encode registers right
Patch-mainline: v4.15-rc1
Git-commit: b3f868df3c8904e964d7b257b47d7d90d93375e0
References: bsc#1109837
Define a new __bitwise type for software representation of registers.
This will allow us to catch incorrect parameter types using sparse.
Accessors we define also allow us to return correct enum type and
therefore ensure all switches handle all register types.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Acked-by: Thomas Bogendoerfer <tbogendoerfer@suse.de>
---
drivers/net/ethernet/netronome/nfp/bpf/jit.c | 99 +++++++++++++-------------
drivers/net/ethernet/netronome/nfp/bpf/main.h | 24 ------
drivers/net/ethernet/netronome/nfp/nfp_asm.h | 45 +++++++++++
3 files changed, 99 insertions(+), 69 deletions(-)
--- a/drivers/net/ethernet/netronome/nfp/bpf/jit.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
@@ -128,11 +128,11 @@ struct nfp_insn_re_regs {
bool i8;
};
-static u16 nfp_swreg_to_unreg(u32 swreg, bool is_dst)
+static u16 nfp_swreg_to_unreg(swreg reg, bool is_dst)
{
- u16 val = FIELD_GET(NN_REG_VAL, swreg);
+ u16 val = swreg_value(reg);
- switch (FIELD_GET(NN_REG_TYPE, swreg)) {
+ switch (swreg_type(reg)) {
case NN_REG_GPR_A:
case NN_REG_GPR_B:
case NN_REG_GPR_BOTH:
@@ -149,33 +149,34 @@ static u16 nfp_swreg_to_unreg(u32 swreg,
return UR_REG_IMM_encode(val);
case NN_REG_NONE:
return is_dst ? UR_REG_NO_DST : REG_NONE;
- default:
- pr_err("unrecognized reg encoding %08x\n", swreg);
- return 0;
}
+
+ pr_err("unrecognized reg encoding %08x\n", reg);
+ return 0;
}
static int
-swreg_to_unrestricted(u32 dst, u32 lreg, u32 rreg, struct nfp_insn_ur_regs *reg)
+swreg_to_unrestricted(swreg dst, swreg lreg, swreg rreg,
+ struct nfp_insn_ur_regs *reg)
{
memset(reg, 0, sizeof(*reg));
/* Decode destination */
- if (FIELD_GET(NN_REG_TYPE, dst) == NN_REG_IMM)
+ if (swreg_type(dst) == NN_REG_IMM)
return -EFAULT;
- if (FIELD_GET(NN_REG_TYPE, dst) == NN_REG_GPR_B)
+ if (swreg_type(dst) == NN_REG_GPR_B)
reg->dst_ab = ALU_DST_B;
- if (FIELD_GET(NN_REG_TYPE, dst) == NN_REG_GPR_BOTH)
+ if (swreg_type(dst) == NN_REG_GPR_BOTH)
reg->wr_both = true;
reg->dst = nfp_swreg_to_unreg(dst, true);
/* Decode source operands */
- if (FIELD_GET(NN_REG_TYPE, lreg) == FIELD_GET(NN_REG_TYPE, rreg))
+ if (swreg_type(lreg) == swreg_type(rreg))
return -EFAULT;
- if (FIELD_GET(NN_REG_TYPE, lreg) == NN_REG_GPR_B ||
- FIELD_GET(NN_REG_TYPE, rreg) == NN_REG_GPR_A) {
+ if (swreg_type(lreg) == NN_REG_GPR_B ||
+ swreg_type(rreg) == NN_REG_GPR_A) {
reg->areg = nfp_swreg_to_unreg(rreg, false);
reg->breg = nfp_swreg_to_unreg(lreg, false);
reg->swap = true;
@@ -187,11 +188,11 @@ swreg_to_unrestricted(u32 dst, u32 lreg,
return 0;
}
-static u16 nfp_swreg_to_rereg(u32 swreg, bool is_dst, bool has_imm8, bool *i8)
+static u16 nfp_swreg_to_rereg(swreg reg, bool is_dst, bool has_imm8, bool *i8)
{
- u16 val = FIELD_GET(NN_REG_VAL, swreg);
+ u16 val = swreg_value(reg);
- switch (FIELD_GET(NN_REG_TYPE, swreg)) {
+ switch (swreg_type(reg)) {
case NN_REG_GPR_A:
case NN_REG_GPR_B:
case NN_REG_GPR_BOTH:
@@ -207,34 +208,37 @@ static u16 nfp_swreg_to_rereg(u32 swreg,
return RE_REG_IMM_encode(val & 0x7f);
case NN_REG_NONE:
return is_dst ? RE_REG_NO_DST : REG_NONE;
- default:
- pr_err("unrecognized reg encoding\n");
+ case NN_REG_NNR:
+ pr_err("NNRs used with restricted encoding\n");
return 0;
}
+
+ pr_err("unrecognized reg encoding\n");
+ return 0;
}
static int
-swreg_to_restricted(u32 dst, u32 lreg, u32 rreg, struct nfp_insn_re_regs *reg,
- bool has_imm8)
+swreg_to_restricted(swreg dst, swreg lreg, swreg rreg,
+ struct nfp_insn_re_regs *reg, bool has_imm8)
{
memset(reg, 0, sizeof(*reg));
/* Decode destination */
- if (FIELD_GET(NN_REG_TYPE, dst) == NN_REG_IMM)
+ if (swreg_type(dst) == NN_REG_IMM)
return -EFAULT;
- if (FIELD_GET(NN_REG_TYPE, dst) == NN_REG_GPR_B)
+ if (swreg_type(dst) == NN_REG_GPR_B)
reg->dst_ab = ALU_DST_B;
- if (FIELD_GET(NN_REG_TYPE, dst) == NN_REG_GPR_BOTH)
+ if (swreg_type(dst) == NN_REG_GPR_BOTH)
reg->wr_both = true;
reg->dst = nfp_swreg_to_rereg(dst, true, false, NULL);
/* Decode source operands */
- if (FIELD_GET(NN_REG_TYPE, lreg) == FIELD_GET(NN_REG_TYPE, rreg))
+ if (swreg_type(lreg) == swreg_type(rreg))
return -EFAULT;
- if (FIELD_GET(NN_REG_TYPE, lreg) == NN_REG_GPR_B ||
- FIELD_GET(NN_REG_TYPE, rreg) == NN_REG_GPR_A) {
+ if (swreg_type(lreg) == NN_REG_GPR_B ||
+ swreg_type(rreg) == NN_REG_GPR_A) {
reg->areg = nfp_swreg_to_rereg(rreg, false, has_imm8, ®->i8);
reg->breg = nfp_swreg_to_rereg(lreg, false, has_imm8, ®->i8);
reg->swap = true;
@@ -281,7 +285,7 @@ __emit_cmd(struct nfp_prog *nfp_prog, en
static void
emit_cmd(struct nfp_prog *nfp_prog, enum cmd_tgt_map op,
- u8 mode, u8 xfer, u32 lreg, u32 rreg, u8 size, bool sync)
+ u8 mode, u8 xfer, swreg lreg, swreg rreg, u8 size, bool sync)
{
struct nfp_insn_re_regs reg;
int err;
@@ -364,7 +368,7 @@ __emit_br_byte(struct nfp_prog *nfp_prog
static void
emit_br_byte_neq(struct nfp_prog *nfp_prog,
- u32 dst, u8 imm, u8 byte, u16 addr, u8 defer)
+ swreg dst, u8 imm, u8 byte, u16 addr, u8 defer)
{
struct nfp_insn_re_regs reg;
int err;
@@ -399,13 +403,13 @@ __emit_immed(struct nfp_prog *nfp_prog,
}
static void
-emit_immed(struct nfp_prog *nfp_prog, u32 dst, u16 imm,
+emit_immed(struct nfp_prog *nfp_prog, swreg dst, u16 imm,
enum immed_width width, bool invert, enum immed_shift shift)
{
struct nfp_insn_ur_regs reg;
int err;
- if (FIELD_GET(NN_REG_TYPE, dst) == NN_REG_IMM) {
+ if (swreg_type(dst) == NN_REG_IMM) {
nfp_prog->error = -EFAULT;
return;
}
@@ -451,8 +455,8 @@ __emit_shf(struct nfp_prog *nfp_prog, u1
}
static void
-emit_shf(struct nfp_prog *nfp_prog, u32 dst, u32 lreg, enum shf_op op, u32 rreg,
- enum shf_sc sc, u8 shift)
+emit_shf(struct nfp_prog *nfp_prog, swreg dst,
+ swreg lreg, enum shf_op op, swreg rreg, enum shf_sc sc, u8 shift)
{
struct nfp_insn_re_regs reg;
int err;
@@ -486,7 +490,8 @@ __emit_alu(struct nfp_prog *nfp_prog, u1
}
static void
-emit_alu(struct nfp_prog *nfp_prog, u32 dst, u32 lreg, enum alu_op op, u32 rreg)
+emit_alu(struct nfp_prog *nfp_prog, swreg dst,
+ swreg lreg, enum alu_op op, swreg rreg)
{
struct nfp_insn_ur_regs reg;
int err;
@@ -524,7 +529,7 @@ __emit_ld_field(struct nfp_prog *nfp_pro
static void
emit_ld_field_any(struct nfp_prog *nfp_prog, enum shf_sc sc, u8 shift,
- u32 dst, u8 bmask, u32 src, bool zero)
+ swreg dst, u8 bmask, swreg src, bool zero)
{
struct nfp_insn_re_regs reg;
int err;
@@ -540,7 +545,7 @@ emit_ld_field_any(struct nfp_prog *nfp_p
}
static void
-emit_ld_field(struct nfp_prog *nfp_prog, u32 dst, u8 bmask, u32 src,
+emit_ld_field(struct nfp_prog *nfp_prog, swreg dst, u8 bmask, swreg src,
enum shf_sc sc, u8 shift)
{
emit_ld_field_any(nfp_prog, sc, shift, dst, bmask, src, false);
@@ -565,7 +570,7 @@ static bool pack_immed(u32 imm, u16 *val
return true;
}
-static void wrp_immed(struct nfp_prog *nfp_prog, u32 dst, u32 imm)
+static void wrp_immed(struct nfp_prog *nfp_prog, swreg dst, u32 imm)
{
enum immed_shift shift;
u16 val;
@@ -586,7 +591,7 @@ static void wrp_immed(struct nfp_prog *n
* If the @imm is small enough encode it directly in operand and return
* otherwise load @imm to a spare register and return its encoding.
*/
-static u32 ur_load_imm_any(struct nfp_prog *nfp_prog, u32 imm, u32 tmp_reg)
+static swreg ur_load_imm_any(struct nfp_prog *nfp_prog, u32 imm, swreg tmp_reg)
{
if (FIELD_FIT(UR_REG_IMM_MAX, imm))
return reg_imm(imm);
@@ -599,7 +604,7 @@ static u32 ur_load_imm_any(struct nfp_pr
* If the @imm is small enough encode it directly in operand and return
* otherwise load @imm to a spare register and return its encoding.
*/
-static u32 re_load_imm_any(struct nfp_prog *nfp_prog, u32 imm, u32 tmp_reg)
+static swreg re_load_imm_any(struct nfp_prog *nfp_prog, u32 imm, swreg tmp_reg)
{
if (FIELD_FIT(RE_REG_IMM_MAX, imm))
return reg_imm(imm);
@@ -629,7 +634,7 @@ construct_data_ind_ld(struct nfp_prog *n
{
unsigned int i;
u16 shift, sz;
- u32 tmp_reg;
+ swreg tmp_reg;
/* We load the value from the address indicated in @offset and then
* shift out the data we don't need. Note: this is big endian!
@@ -697,7 +702,7 @@ static int wrp_set_mark(struct nfp_prog
static void
wrp_alu_imm(struct nfp_prog *nfp_prog, u8 dst, enum alu_op alu_op, u32 imm)
{
- u32 tmp_reg;
+ swreg tmp_reg;
if (alu_op == ALU_OP_AND) {
if (!imm)
@@ -815,7 +820,7 @@ wrp_cmp_imm(struct nfp_prog *nfp_prog, s
const struct bpf_insn *insn = &meta->insn;
u64 imm = insn->imm; /* sign extend */
u8 reg = insn->dst_reg * 2;
- u32 tmp_reg;
+ swreg tmp_reg;
if (insn->off < 0) /* TODO */
return -EOPNOTSUPP;
@@ -1139,7 +1144,7 @@ static int mem_ldx4_skb(struct nfp_prog
static int mem_ldx4_xdp(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
{
- u32 dst = reg_both(meta->insn.dst_reg * 2);
+ swreg dst = reg_both(meta->insn.dst_reg * 2);
if (meta->insn.off != offsetof(struct xdp_md, data) &&
meta->insn.off != offsetof(struct xdp_md, data_end))
@@ -1202,8 +1207,10 @@ static int jeq_imm(struct nfp_prog *nfp_
{
const struct bpf_insn *insn = &meta->insn;
u64 imm = insn->imm; /* sign extend */
- u32 or1 = reg_a(insn->dst_reg * 2), or2 = reg_b(insn->dst_reg * 2 + 1);
- u32 tmp_reg;
+ swreg or1, or2, tmp_reg;
+
+ or1 = reg_a(insn->dst_reg * 2);
+ or2 = reg_b(insn->dst_reg * 2 + 1);
if (insn->off < 0) /* TODO */
return -EOPNOTSUPP;
@@ -1252,7 +1259,7 @@ static int jset_imm(struct nfp_prog *nfp
{
const struct bpf_insn *insn = &meta->insn;
u64 imm = insn->imm; /* sign extend */
- u32 tmp_reg;
+ swreg tmp_reg;
if (insn->off < 0) /* TODO */
return -EOPNOTSUPP;
@@ -1283,7 +1290,7 @@ static int jne_imm(struct nfp_prog *nfp_
{
const struct bpf_insn *insn = &meta->insn;
u64 imm = insn->imm; /* sign extend */
- u32 tmp_reg;
+ swreg tmp_reg;
if (insn->off < 0) /* TODO */
return -EOPNOTSUPP;
--- a/drivers/net/ethernet/netronome/nfp/bpf/main.h
+++ b/drivers/net/ethernet/netronome/nfp/bpf/main.h
@@ -39,6 +39,7 @@
#include <linux/list.h>
#include <linux/types.h>
+#include "../nfp_asm.h"
#include "../nfp_net.h"
/* For branch fixup logic use up-most byte of branch instruction as scratch
@@ -65,29 +66,6 @@ enum nfp_bpf_action_type {
NN_ACT_XDP,
};
-/* Software register representation, hardware encoding in asm.h */
-#define NN_REG_TYPE GENMASK(31, 24)
-#define NN_REG_VAL GENMASK(7, 0)
-
-enum nfp_bpf_reg_type {
- NN_REG_GPR_A = BIT(0),
- NN_REG_GPR_B = BIT(1),
- NN_REG_NNR = BIT(2),
- NN_REG_XFER = BIT(3),
- NN_REG_IMM = BIT(4),
- NN_REG_NONE = BIT(5),
-};
-
-#define NN_REG_GPR_BOTH (NN_REG_GPR_A | NN_REG_GPR_B)
-
-#define reg_both(x) ((x) | FIELD_PREP(NN_REG_TYPE, NN_REG_GPR_BOTH))
-#define reg_a(x) ((x) | FIELD_PREP(NN_REG_TYPE, NN_REG_GPR_A))
-#define reg_b(x) ((x) | FIELD_PREP(NN_REG_TYPE, NN_REG_GPR_B))
-#define reg_nnr(x) ((x) | FIELD_PREP(NN_REG_TYPE, NN_REG_NNR))
-#define reg_xfer(x) ((x) | FIELD_PREP(NN_REG_TYPE, NN_REG_XFER))
-#define reg_imm(x) ((x) | FIELD_PREP(NN_REG_TYPE, NN_REG_IMM))
-#define reg_none() (FIELD_PREP(NN_REG_TYPE, NN_REG_NONE))
-
#define pkt_reg(np) reg_a((np)->regs_per_thread - STATIC_REG_PKT)
#define imm_a(np) reg_a((np)->regs_per_thread - STATIC_REG_IMM)
#define imm_b(np) reg_b((np)->regs_per_thread - STATIC_REG_IMM)
--- a/drivers/net/ethernet/netronome/nfp/nfp_asm.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_asm.h
@@ -34,6 +34,7 @@
#ifndef __NFP_ASM_H__
#define __NFP_ASM_H__ 1
+#include <linux/bitfield.h>
#include <linux/types.h>
#define REG_NONE 0
@@ -230,4 +231,48 @@ enum lcsr_wr_src {
#define OP_CARB_BASE 0x0e000000000ULL
#define OP_CARB_OR 0x00000010000ULL
+/* Software register representation, independent of operand type */
+#define NN_REG_TYPE GENMASK(31, 24)
+#define NN_REG_VAL GENMASK(7, 0)
+
+enum nfp_bpf_reg_type {
+ NN_REG_GPR_A = BIT(0),
+ NN_REG_GPR_B = BIT(1),
+ NN_REG_GPR_BOTH = NN_REG_GPR_A | NN_REG_GPR_B,
+ NN_REG_NNR = BIT(2),
+ NN_REG_XFER = BIT(3),
+ NN_REG_IMM = BIT(4),
+ NN_REG_NONE = BIT(5),
+};
+
+#define reg_both(x) __enc_swreg((x), NN_REG_GPR_BOTH)
+#define reg_a(x) __enc_swreg((x), NN_REG_GPR_A)
+#define reg_b(x) __enc_swreg((x), NN_REG_GPR_B)
+#define reg_nnr(x) __enc_swreg((x), NN_REG_NNR)
+#define reg_xfer(x) __enc_swreg((x), NN_REG_XFER)
+#define reg_imm(x) __enc_swreg((x), NN_REG_IMM)
+#define reg_none() __enc_swreg(0, NN_REG_NONE)
+
+typedef __u32 __bitwise swreg;
+
+static inline swreg __enc_swreg(u16 id, u8 type)
+{
+ return (__force swreg)(id | FIELD_PREP(NN_REG_TYPE, type));
+}
+
+static inline u32 swreg_raw(swreg reg)
+{
+ return (__force u32)reg;
+}
+
+static inline enum nfp_bpf_reg_type swreg_type(swreg reg)
+{
+ return FIELD_GET(NN_REG_TYPE, swreg_raw(reg));
+}
+
+static inline u16 swreg_value(swreg reg)
+{
+ return FIELD_GET(NN_REG_VAL, swreg_raw(reg));
+}
+
#endif