On 2024-07-10 11:28 p.m., Paul Menzel wrote:
Dear Ahmed, dear Junfeng,
Thank you for this patch.
Am 10.07.24 um 22:40 schrieb Ahmed Zaki:
From: Junfeng Guo <junfeng....@intel.com>
Parse the following DDP sections:
- ICE_SID_RXPARSER_IMEM into an array of struct ice_imem_item
- ICE_SID_RXPARSER_METADATA_INIT into an array of struct
ice_metainit_item
- ICE_SID_RXPARSER_CAM or ICE_SID_RXPARSER_PG_SPILL into an array of
struct ice_pg_cam_item
- ICE_SID_RXPARSER_NOMATCH_CAM or ICE_SID_RXPARSER_NOMATCH_SPILL
into an
array of struct ice_pg_nm_cam_item
- ICE_SID_RXPARSER_CAM into an array of ice_bst_tcam_item
- ICE_SID_LBL_RXPARSER_TMEM into an array of ice_lbl_item
- ICE_SID_RXPARSER_MARKER_PTYPE into an array of ice_ptype_mk_tcam_item
- ICE_SID_RXPARSER_MARKER_GRP into an array of ice_mk_grp_item
- ICE_SID_RXPARSER_PROTO_GRP into an array of ice_proto_grp_item
- ICE_SID_RXPARSER_FLAG_REDIR into an array of ice_flg_rd_item
- ICE_SID_XLT_KEY_BUILDER_SW, ICE_SID_XLT_KEY_BUILDER_ACL,
ICE_SID_XLT_KEY_BUILDER_FD and ICE_SID_XLT_KEY_BUILDER_RSS into
struct ice_xlt_kb
Did you write this from hand, or do you have some scripts to convert it
from some datasheet into code?
I believe this was all manually done.
As the parser is new infrastructure, are you planing on adding some
tests for the parser?
I am not aware of any plans to add testing for the parser stage.
In any case, what kind of tests do you have in mind? if you mean kerenl
self-tests, I don't see any other ice tests in
tools/testing/selftests/drivers/net/ for the rest of the already
supported h/w stages (ACL, FDIR, RSS, ..etc) that I can use as examples.
Could you also go into more detail on how to debug possible problems,
that means, how to make sure, that the parser works correctly and how to
debug it in case one suspects the parser has a problem.
The next patch in the set adds ice_debug() for almost all operations.
That includes dumping all DDP related sections and CAM entries.
Reviewed-by: Marcin Szycik <marcin.szy...@linux.intel.com>
Signed-off-by: Qi Zhang <qi.z.zh...@intel.com>
Signed-off-by: Junfeng Guo <junfeng....@intel.com>
Co-developed-by: Ahmed Zaki <ahmed.z...@intel.com>
Signed-off-by: Ahmed Zaki <ahmed.z...@intel.com>
---
drivers/net/ethernet/intel/ice/ice_ddp.c | 10 +-
drivers/net/ethernet/intel/ice/ice_ddp.h | 13 +
drivers/net/ethernet/intel/ice/ice_parser.c | 1396 +++++++++++++++++++
drivers/net/ethernet/intel/ice/ice_parser.h | 357 +++++
drivers/net/ethernet/intel/ice/ice_type.h | 1 +
5 files changed, 1772 insertions(+), 5 deletions(-)
<..>
+ * @length: number of items in the table to create
+ * @parse_item: the function to parse the item
+ * @no_offset: ignore header offset, calculate index from 0
+ *
+ * Return: a pointer to the allocated table or ERR_PTR.
+ */
+static
+void *ice_parser_create_table(struct ice_hw *hw, u32 sect_type,
+ u32 item_size, u32 length,
+ void (*parse_item)(struct ice_hw *hw, u16 idx,
+ void *item, void *data,
+ int size),
+ bool no_offset)
+{
+ struct ice_pkg_enum state = {};
+ struct ice_seg *seg = hw->seg;
+ void *table, *data, *item;
+ u16 idx = U16_MAX;
+
+ if (!seg)
+ return ERR_PTR(-EINVAL);
+
+ table = kzalloc(item_size * length, GFP_KERNEL);
+ if (!table)
+ return ERR_PTR(-ENOMEM);
+
+ do {
+ data = ice_pkg_enum_entry(seg, &state, sect_type, NULL,
+ ice_parser_sect_item_get);
+ seg = NULL;
+ if (data) {
+ struct ice_pkg_sect_hdr *hdr =
+ (struct ice_pkg_sect_hdr *)state.sect;
+
+ if (no_offset)
+ idx++;
Can’t `idx` initialized with `U16_MAX` now overflow?
Good catch, looking at the code, that seems to be intentionl so that
first call to parse_item() would start at 0.
+ else
+ idx = le16_to_cpu(hdr->offset) +
+ state.entry_idx;
+
+ item = (void *)((uintptr_t)table + idx * item_size);
+ parse_item(hw, idx, item, data, item_size);
I will change that to
u16 idx = 0;
..
parse_item(hw, idx, item, data, item_size);
if (no_offset)
idx++;
better readability.
+ }
+ } while (data);
+
+ return table;
+}
+
+/*** ICE_SID_RXPARSER_IMEM section ***/
+#define ICE_IM_BM_ALU0 BIT(0)
+#define ICE_IM_BM_ALU1 BIT(1)
+#define ICE_IM_BM_ALU2 BIT(2)
+#define ICE_IM_BM_PG BIT(3)
+
+/**
+ * ice_imem_bm_init - parse 4 bits of Boost Main
+ * @bm: pointer to the Boost Main structure
+ * @data: Boost Main data to be parsed
Excuse my ignorance, as you use `data` in `FIELD_GET()` is `data` the
right name? The `FIELD_GET()` example in `include/linux/bitfield.h` uses
`reg`. How do I know the valid values of `data`?
This and the following funcs parse the DDP sections. so 'data' is not
always register values.
+ */
+static void ice_imem_bm_init(struct ice_bst_main *bm, u8 data)
+{
+ bm->alu0 = FIELD_GET(ICE_IM_BM_ALU0, data);
+ bm->alu1 = FIELD_GET(ICE_IM_BM_ALU1, data);
+ bm->alu2 = FIELD_GET(ICE_IM_BM_ALU2, data);
+ bm->pg = FIELD_GET(ICE_IM_BM_PG, data);
+}
+
+#define ICE_IM_BKB_PRIO GENMASK(7, 0)
+#define ICE_IM_BKB_TSR_CTRL BIT(8)
<..>
diff --git a/drivers/net/ethernet/intel/ice/ice_parser.h
b/drivers/net/ethernet/intel/ice/ice_parser.h
index 09ed380eee32..26468b16202c 100644
--- a/drivers/net/ethernet/intel/ice/ice_parser.h
+++ b/drivers/net/ethernet/intel/ice/ice_parser.h
@@ -4,8 +4,365 @@
#ifndef _ICE_PARSER_H_
#define _ICE_PARSER_H_
+#define ICE_SEC_DATA_OFFSET 4
+#define ICE_SID_RXPARSER_IMEM_ENTRY_SIZE 48
+#define ICE_SID_RXPARSER_METADATA_INIT_ENTRY_SIZE 24
+#define ICE_SID_RXPARSER_CAM_ENTRY_SIZE 16
+#define ICE_SID_RXPARSER_PG_SPILL_ENTRY_SIZE 17
+#define ICE_SID_RXPARSER_NOMATCH_CAM_ENTRY_SIZE 12
+#define ICE_SID_RXPARSER_NOMATCH_SPILL_ENTRY_SIZE 13
+#define ICE_SID_RXPARSER_BOOST_TCAM_ENTRY_SIZE 88
+#define ICE_SID_RXPARSER_MARKER_TYPE_ENTRY_SIZE 24
+#define ICE_SID_RXPARSER_MARKER_GRP_ENTRY_SIZE 8
+#define ICE_SID_RXPARSER_PROTO_GRP_ENTRY_SIZE 24
+#define ICE_SID_RXPARSER_FLAG_REDIR_ENTRY_SIZE 1
+
+#define ICE_SEC_LBL_DATA_OFFSET 2
+#define ICE_SID_LBL_ENTRY_SIZE 66
+
+/*** ICE_SID_RXPARSER_IMEM section ***/
+#define ICE_IMEM_TABLE_SIZE 192
+
+/* TCAM boost Master; if bit is set, and TCAM hit, TCAM output
overrides iMEM
+ * output.
+ */
+struct ice_bst_main {
+ bool alu0;
+ bool alu1;
+ bool alu2;
+ bool pg;
+};
+
+struct ice_bst_keybuilder {
+ u8 prio;
Add a comment, what `prio` is and what values are allowed?
Will do.
thanks.
