[PATCH net-next 0/8] net: ipa: abstract status parsing

[PATCH net-next 0/8] net: ipa: abstract status parsing

[Alex Elder]

Under some circumstances, IPA generates a "packet status" structure
that describes information about a packet.  This is used, for
example, when offload hardware detects an error in a packet, or
otherwise discovers a packet needs special handling.  In this case,
the status is delivered (along with the packet it describes) to a
"default" endpoint so that it can be handled by the AP.

Until now, the structure of this status information hasn't changed.
However, to support more than 32 endpoints, this structure required
some changes, such that some fields are rearranged in ways that are
tricky to represent using C code.

This series updates code related to the IPA status structure.  The
first patch uses a local variable to avoid recomputing a packet
length more than once.  The second stops using sizeof() to determine
the size of an IPA packet status structure.  Patches 3-5 extend the
definitions for values held in packet status fields.  Patch 6 does a
little general cleanup to make patch 7 simpler.  Patch 7 stops using
a C structure to represent packet status; instead, a new function
fetches values "by name" from a buffer containing such a structure.
The last patch updates this function so it also supports IPA v5.0+.

					-Alex

Alex Elder (8):
  net: ipa: refactor status buffer parsing
  net: ipa: stop using sizeof(status)
  net: ipa: define all IPA status mask bits
  net: ipa: rename the NAT enumerated type
  net: ipa: define remaining IPA status field values
  net: ipa: IPA status preparatory cleanups
  net: ipa: introduce generalized status decoder
  net: ipa: add IPA v5.0 packet status support

 drivers/net/ipa/ipa_endpoint.c | 280 +++++++++++++++++++++++++--------
 drivers/net/ipa/ipa_reg.h      |  10 +-
 2 files changed, 217 insertions(+), 73 deletions(-)

-- 
2.34.1

[PATCH net-next 1/8] net: ipa: refactor status buffer parsing

[Alex Elder]

The packet length encoded in an IPA packet status buffer is computed
more than once in ipa_endpoint_status_parse().  It is also checked
again in ipa_endpoint_status_skip(), which that function calls.

Compute the length once, and use that computed value later rather
than recomputing it.  Check for it being zero in the parse function
rather than in ipa_endpoint_status_skip().

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/net/ipa/ipa_endpoint.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ipa/ipa_endpoint.c b/drivers/net/ipa/ipa_endpoint.c
index 136932464261c..3756ce5f3f310 100644
--- a/drivers/net/ipa/ipa_endpoint.c
+++ b/drivers/net/ipa/ipa_endpoint.c
@@ -1325,8 +1325,7 @@ static bool ipa_endpoint_status_skip(struct ipa_endpoint *endpoint,
 
 	if (!ipa_status_format_packet(status->opcode))
 		return true;
-	if (!status->pkt_len)
-		return true;
+
 	endpoint_id = u8_get_bits(status->endp_dst_idx,
 				  IPA_STATUS_DST_IDX_FMASK);
 	if (endpoint_id != endpoint->endpoint_id)
@@ -1394,6 +1393,7 @@ static void ipa_endpoint_status_parse(struct ipa_endpoint *endpoint,
 
 	while (resid) {
 		const struct ipa_status *status = data;
+		u32 length;
 		u32 align;
 		u32 len;
 
@@ -1405,7 +1405,8 @@ static void ipa_endpoint_status_parse(struct ipa_endpoint *endpoint,
 		}
 
 		/* Skip over status packets that lack packet data */
-		if (ipa_endpoint_status_skip(endpoint, status)) {
+		length = le16_to_cpu(status->pkt_len);
+		if (!length || ipa_endpoint_status_skip(endpoint, status)) {
 			data += sizeof(*status);
 			resid -= sizeof(*status);
 			continue;
@@ -1418,19 +1419,16 @@ static void ipa_endpoint_status_parse(struct ipa_endpoint *endpoint,
 		 * computed checksum information will be appended.
 		 */
 		align = endpoint->config.rx.pad_align ? : 1;
-		len = le16_to_cpu(status->pkt_len);
-		len = sizeof(*status) + ALIGN(len, align);
+		len = sizeof(*status) + ALIGN(length, align);
 		if (endpoint->config.checksum)
 			len += sizeof(struct rmnet_map_dl_csum_trailer);
 
 		if (!ipa_endpoint_status_drop(endpoint, status)) {
 			void *data2;
 			u32 extra;
-			u32 len2;
 
 			/* Client receives only packet data (no status) */
 			data2 = data + sizeof(*status);
-			len2 = le16_to_cpu(status->pkt_len);
 
 			/* Have the true size reflect the extra unused space in
 			 * the original receive buffer.  Distribute the "cost"
@@ -1438,7 +1436,7 @@ static void ipa_endpoint_status_parse(struct ipa_endpoint *endpoint,
 			 * buffer.
 			 */
 			extra = DIV_ROUND_CLOSEST(unused * len, total_len);
-			ipa_endpoint_skb_copy(endpoint, data2, len2, extra);
+			ipa_endpoint_skb_copy(endpoint, data2, length, extra);
 		}
 
 		/* Consume status and the full packet it describes */
-- 
2.34.1

[PATCH net-next 2/8] net: ipa: stop using sizeof(status)

[Alex Elder]

The IPA packet status structure changes in IPA v5.0 in ways that are
difficult to represent cleanly.  As a small step toward redefining
it as a parsed block of data, use a constant to define its size,
rather than the size of the IPA status structure type.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/net/ipa/ipa_endpoint.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ipa/ipa_endpoint.c b/drivers/net/ipa/ipa_endpoint.c
index 3756ce5f3f310..dd4b2b073aae9 100644
--- a/drivers/net/ipa/ipa_endpoint.c
+++ b/drivers/net/ipa/ipa_endpoint.c
@@ -70,6 +70,9 @@ struct ipa_status {
 #define IPA_STATUS_FLAGS1_RT_RULE_ID_FMASK	GENMASK(31, 22)
 #define IPA_STATUS_FLAGS2_TAG_FMASK		GENMASK_ULL(63, 16)
 
+/* Size in bytes of an IPA packet status structure */
+#define IPA_STATUS_SIZE			sizeof(__le32[4])
+
 /* Compute the aggregation size value to use for a given buffer size */
 static u32 ipa_aggr_size_kb(u32 rx_buffer_size, bool aggr_hard_limit)
 {
@@ -1397,18 +1400,18 @@ static void ipa_endpoint_status_parse(struct ipa_endpoint *endpoint,
 		u32 align;
 		u32 len;
 
-		if (resid < sizeof(*status)) {
+		if (resid < IPA_STATUS_SIZE) {
 			dev_err(&endpoint->ipa->pdev->dev,
 				"short message (%u bytes < %zu byte status)\n",
-				resid, sizeof(*status));
+				resid, IPA_STATUS_SIZE);
 			break;
 		}
 
 		/* Skip over status packets that lack packet data */
 		length = le16_to_cpu(status->pkt_len);
 		if (!length || ipa_endpoint_status_skip(endpoint, status)) {
-			data += sizeof(*status);
-			resid -= sizeof(*status);
+			data += IPA_STATUS_SIZE;
+			resid -= IPA_STATUS_SIZE;
 			continue;
 		}
 
@@ -1419,7 +1422,7 @@ static void ipa_endpoint_status_parse(struct ipa_endpoint *endpoint,
 		 * computed checksum information will be appended.
 		 */
 		align = endpoint->config.rx.pad_align ? : 1;
-		len = sizeof(*status) + ALIGN(length, align);
+		len = IPA_STATUS_SIZE + ALIGN(length, align);
 		if (endpoint->config.checksum)
 			len += sizeof(struct rmnet_map_dl_csum_trailer);
 
@@ -1428,7 +1431,7 @@ static void ipa_endpoint_status_parse(struct ipa_endpoint *endpoint,
 			u32 extra;
 
 			/* Client receives only packet data (no status) */
-			data2 = data + sizeof(*status);
+			data2 = data + IPA_STATUS_SIZE;
 
 			/* Have the true size reflect the extra unused space in
 			 * the original receive buffer.  Distribute the "cost"
-- 
2.34.1

[PATCH net-next 3/8] net: ipa: define all IPA status mask bits

[Alex Elder]

There is a 16 bit status mask defined in the IPA packet status
structure, of which only one (TAG_VALID) is currently used.

Define all other IPA status mask values in an enumerated type whose
numeric values are bit mask values (in CPU byte order) in the status
mask.  Use the TAG_VALID value from that type rather than defining a
separate field mask.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/net/ipa/ipa_endpoint.c | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ipa/ipa_endpoint.c b/drivers/net/ipa/ipa_endpoint.c
index dd4b2b073aae9..5cf3ac2b5c85a 100644
--- a/drivers/net/ipa/ipa_endpoint.c
+++ b/drivers/net/ipa/ipa_endpoint.c
@@ -48,11 +48,31 @@ enum ipa_status_exception {
 	IPA_STATUS_EXCEPTION_DEAGGR		= 0x01,
 };
 
+/** enum ipa_status_mask - IPA status mask field bitmask hardware values */
+enum ipa_status_mask {
+	IPA_STATUS_MASK_FRAG_PROCESS		= BIT(0),
+	IPA_STATUS_MASK_FILT_PROCESS		= BIT(1),
+	IPA_STATUS_MASK_NAT_PROCESS		= BIT(2),
+	IPA_STATUS_MASK_ROUTE_PROCESS		= BIT(3),
+	IPA_STATUS_MASK_TAG_VALID		= BIT(4),
+	IPA_STATUS_MASK_FRAGMENT		= BIT(5),
+	IPA_STATUS_MASK_FIRST_FRAGMENT		= BIT(6),
+	IPA_STATUS_MASK_V4			= BIT(7),
+	IPA_STATUS_MASK_CKSUM_PROCESS		= BIT(8),
+	IPA_STATUS_MASK_AGGR_PROCESS		= BIT(9),
+	IPA_STATUS_MASK_DEST_EOT		= BIT(10),
+	IPA_STATUS_MASK_DEAGGR_PROCESS		= BIT(11),
+	IPA_STATUS_MASK_DEAGG_FIRST		= BIT(12),
+	IPA_STATUS_MASK_SRC_EOT			= BIT(13),
+	IPA_STATUS_MASK_PREV_EOT		= BIT(14),
+	IPA_STATUS_MASK_BYTE_LIMIT		= BIT(15),
+};
+
 /* Status element provided by hardware */
 struct ipa_status {
 	u8 opcode;		/* enum ipa_status_opcode */
 	u8 exception;		/* enum ipa_status_exception */
-	__le16 mask;
+	__le16 mask;		/* enum ipa_status_bit (bitmask) */
 	__le16 pkt_len;
 	u8 endp_src_idx;
 	u8 endp_dst_idx;
@@ -64,7 +84,6 @@ struct ipa_status {
 };
 
 /* Field masks for struct ipa_status structure fields */
-#define IPA_STATUS_MASK_TAG_VALID_FMASK		GENMASK(4, 4)
 #define IPA_STATUS_SRC_IDX_FMASK		GENMASK(4, 0)
 #define IPA_STATUS_DST_IDX_FMASK		GENMASK(4, 0)
 #define IPA_STATUS_FLAGS1_RT_RULE_ID_FMASK	GENMASK(31, 22)
@@ -1344,7 +1363,7 @@ static bool ipa_endpoint_status_tag(struct ipa_endpoint *endpoint,
 	struct ipa *ipa = endpoint->ipa;
 	u32 endpoint_id;
 
-	if (!le16_get_bits(status->mask, IPA_STATUS_MASK_TAG_VALID_FMASK))
+	if (!le16_get_bits(status->mask, IPA_STATUS_MASK_TAG_VALID))
 		return false;	/* No valid tag */
 
 	/* The status contains a valid tag.  We know the packet was sent to
-- 
2.34.1

[PATCH net-next 4/8] net: ipa: rename the NAT enumerated type

[Alex Elder]

Rename the ipa_nat_en enumerated type to be ipa_nat_type, and rename
its symbols accordingly.  Add a comment indicating those values are
also used in the IPA status nat_type field.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/net/ipa/ipa_endpoint.c |  4 +++-
 drivers/net/ipa/ipa_reg.h      | 10 +++++-----
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ipa/ipa_endpoint.c b/drivers/net/ipa/ipa_endpoint.c
index 5cf3ac2b5c85a..178934f131be5 100644
--- a/drivers/net/ipa/ipa_endpoint.c
+++ b/drivers/net/ipa/ipa_endpoint.c
@@ -68,6 +68,8 @@ enum ipa_status_mask {
 	IPA_STATUS_MASK_BYTE_LIMIT		= BIT(15),
 };
 
+/** The IPA status nat_type field uses enum ipa_nat_type hardware values */
+
 /* Status element provided by hardware */
 struct ipa_status {
 	u8 opcode;		/* enum ipa_status_opcode */
@@ -570,7 +572,7 @@ static void ipa_endpoint_init_nat(struct ipa_endpoint *endpoint)
 		return;
 
 	reg = ipa_reg(ipa, ENDP_INIT_NAT);
-	val = ipa_reg_encode(reg, NAT_EN, IPA_NAT_BYPASS);
+	val = ipa_reg_encode(reg, NAT_EN, IPA_NAT_TYPE_BYPASS);
 
 	iowrite32(val, ipa->reg_virt + ipa_reg_n_offset(reg, endpoint_id));
 }
diff --git a/drivers/net/ipa/ipa_reg.h b/drivers/net/ipa/ipa_reg.h
index ff64b19a4df8b..b1a3c2c7e1674 100644
--- a/drivers/net/ipa/ipa_reg.h
+++ b/drivers/net/ipa/ipa_reg.h
@@ -382,11 +382,11 @@ enum ipa_reg_endp_init_nat_field_id {
 	NAT_EN,
 };
 
-/** enum ipa_nat_en - ENDP_INIT_NAT register NAT_EN field value */
-enum ipa_nat_en {
-	IPA_NAT_BYPASS				= 0x0,
-	IPA_NAT_SRC				= 0x1,
-	IPA_NAT_DST				= 0x2,
+/** enum ipa_nat_type - ENDP_INIT_NAT register NAT_EN field value */
+enum ipa_nat_type {
+	IPA_NAT_TYPE_BYPASS			= 0,
+	IPA_NAT_TYPE_SRC			= 1,
+	IPA_NAT_TYPE_DST			= 2,
 };
 
 /* ENDP_INIT_HDR register */
-- 
2.34.1

[PATCH net-next 5/8] net: ipa: define remaining IPA status field values

[Alex Elder]

Define the remaining values for opcode and exception fields in the
IPA packet status structure.  Most of these values are powers-of-2,
suggesting they are meant to be used as bitmasks, but that is not
the case.  Add comments to be clear about this, and express the
values in decimal format.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/net/ipa/ipa_endpoint.c | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ipa/ipa_endpoint.c b/drivers/net/ipa/ipa_endpoint.c
index 178934f131be5..ee3c29b1efea9 100644
--- a/drivers/net/ipa/ipa_endpoint.c
+++ b/drivers/net/ipa/ipa_endpoint.c
@@ -34,18 +34,31 @@
 
 #define IPA_ENDPOINT_RESET_AGGR_RETRY_MAX	3
 
-/** enum ipa_status_opcode - status element opcode hardware values */
-enum ipa_status_opcode {
-	IPA_STATUS_OPCODE_PACKET		= 0x01,
-	IPA_STATUS_OPCODE_DROPPED_PACKET	= 0x04,
-	IPA_STATUS_OPCODE_SUSPENDED_PACKET	= 0x08,
-	IPA_STATUS_OPCODE_PACKET_2ND_PASS	= 0x40,
+/** enum ipa_status_opcode - IPA status opcode field hardware values */
+enum ipa_status_opcode {				/* *Not* a bitmask */
+	IPA_STATUS_OPCODE_PACKET		= 1,
+	IPA_STATUS_OPCODE_NEW_RULE_PACKET	= 2,
+	IPA_STATUS_OPCODE_DROPPED_PACKET	= 4,
+	IPA_STATUS_OPCODE_SUSPENDED_PACKET	= 8,
+	IPA_STATUS_OPCODE_LOG			= 16,
+	IPA_STATUS_OPCODE_DCMP			= 32,
+	IPA_STATUS_OPCODE_PACKET_2ND_PASS	= 64,
 };
 
-/** enum ipa_status_exception - status element exception type */
-enum ipa_status_exception {
+/** enum ipa_status_exception - IPA status exception field hardware values */
+enum ipa_status_exception {				/* *Not* a bitmask */
 	/* 0 means no exception */
-	IPA_STATUS_EXCEPTION_DEAGGR		= 0x01,
+	IPA_STATUS_EXCEPTION_DEAGGR		= 1,
+	IPA_STATUS_EXCEPTION_IPTYPE		= 4,
+	IPA_STATUS_EXCEPTION_PACKET_LENGTH	= 8,
+	IPA_STATUS_EXCEPTION_FRAG_RULE_MISS	= 16,
+	IPA_STATUS_EXCEPTION_SW_FILTER		= 32,
+	IPA_STATUS_EXCEPTION_NAT		= 64,		/* IPv4 */
+	IPA_STATUS_EXCEPTION_IPV6_CONN_TRACK	= 64,		/* IPv6 */
+	IPA_STATUS_EXCEPTION_UC			= 128,
+	IPA_STATUS_EXCEPTION_INVALID_ENDPOINT	= 129,
+	IPA_STATUS_EXCEPTION_HEADER_INSERT	= 136,
+	IPA_STATUS_EXCEPTION_CHEKCSUM		= 229,
 };
 
 /** enum ipa_status_mask - IPA status mask field bitmask hardware values */
-- 
2.34.1

[PATCH net-next 6/8] net: ipa: IPA status preparatory cleanups

[Alex Elder]

The next patch reworks how the IPA packet status structure is
interpreted.  This patch does some preparatory work, to make it
easier to see the effect of that change:
  - Change a few functions that access fields in a IPA packet status
    structure to store field values in local variables with names
    related to the field.
  - Pass a void pointer rather than an (equivalent) status pointer
    to two functions called by ipa_endpoint_status_parse().
  - Use "rule" rather than "val" as the name of a variable that
    holds a routing rule ID.
  - Consistently use "IPA packet status" rather than "status
    element" when referring to this data structure.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/net/ipa/ipa_endpoint.c | 43 ++++++++++++++++++++--------------
 1 file changed, 25 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ipa/ipa_endpoint.c b/drivers/net/ipa/ipa_endpoint.c
index ee3c29b1efea9..5097eb1bbadb0 100644
--- a/drivers/net/ipa/ipa_endpoint.c
+++ b/drivers/net/ipa/ipa_endpoint.c
@@ -1182,8 +1182,8 @@ static void ipa_endpoint_status(struct ipa_endpoint *endpoint)
 			val |= ipa_reg_encode(reg, STATUS_ENDP,
 					      status_endpoint_id);
 		}
-		/* STATUS_LOCATION is 0, meaning status element precedes
-		 * packet (not present for IPA v4.5+)
+		/* STATUS_LOCATION is 0, meaning IPA packet status
+		 * precedes the packet (not present for IPA v4.5+)
 		 */
 		/* STATUS_PKT_SUPPRESS_FMASK is 0 (not present for v4.0+) */
 	}
@@ -1339,8 +1339,8 @@ static bool ipa_endpoint_skb_build(struct ipa_endpoint *endpoint,
 	return skb != NULL;
 }
 
-/* The format of a packet status element is the same for several status
- * types (opcodes).  Other types aren't currently supported.
+ /* The format of an IPA packet status structure is the same for several
+  * status types (opcodes).  Other types aren't currently supported.
  */
 static bool ipa_status_format_packet(enum ipa_status_opcode opcode)
 {
@@ -1358,9 +1358,11 @@ static bool ipa_status_format_packet(enum ipa_status_opcode opcode)
 static bool ipa_endpoint_status_skip(struct ipa_endpoint *endpoint,
 				     const struct ipa_status *status)
 {
+	enum ipa_status_opcode opcode;
 	u32 endpoint_id;
 
-	if (!ipa_status_format_packet(status->opcode))
+	opcode = status->opcode;
+	if (!ipa_status_format_packet(opcode))
 		return true;
 
 	endpoint_id = u8_get_bits(status->endp_dst_idx,
@@ -1371,14 +1373,16 @@ static bool ipa_endpoint_status_skip(struct ipa_endpoint *endpoint,
 	return false;	/* Don't skip this packet, process it */
 }
 
-static bool ipa_endpoint_status_tag(struct ipa_endpoint *endpoint,
-				    const struct ipa_status *status)
+static bool ipa_endpoint_status_tag_valid(struct ipa_endpoint *endpoint,
+					  const struct ipa_status *status)
 {
 	struct ipa_endpoint *command_endpoint;
+	enum ipa_status_mask status_mask;
 	struct ipa *ipa = endpoint->ipa;
 	u32 endpoint_id;
 
-	if (!le16_get_bits(status->mask, IPA_STATUS_MASK_TAG_VALID))
+	status_mask = le16_get_bits(status->mask, IPA_STATUS_MASK_TAG_VALID);
+	if (!status_mask)
 		return false;	/* No valid tag */
 
 	/* The status contains a valid tag.  We know the packet was sent to
@@ -1404,20 +1408,23 @@ static bool ipa_endpoint_status_tag(struct ipa_endpoint *endpoint,
 static bool ipa_endpoint_status_drop(struct ipa_endpoint *endpoint,
 				     const struct ipa_status *status)
 {
-	u32 val;
+	enum ipa_status_exception exception;
+	u32 rule;
 
 	/* If the status indicates a tagged transfer, we'll drop the packet */
-	if (ipa_endpoint_status_tag(endpoint, status))
+	if (ipa_endpoint_status_tag_valid(endpoint, status))
 		return true;
 
 	/* Deaggregation exceptions we drop; all other types we consume */
-	if (status->exception)
-		return status->exception == IPA_STATUS_EXCEPTION_DEAGGR;
+	exception = status->exception;
+	if (exception)
+		return exception == IPA_STATUS_EXCEPTION_DEAGGR;
 
 	/* Drop the packet if it fails to match a routing rule; otherwise no */
-	val = le32_get_bits(status->flags1, IPA_STATUS_FLAGS1_RT_RULE_ID_FMASK);
+	rule = le32_get_bits(status->flags1,
+			     IPA_STATUS_FLAGS1_RT_RULE_ID_FMASK);
 
-	return val == field_max(IPA_STATUS_FLAGS1_RT_RULE_ID_FMASK);
+	return rule == field_max(IPA_STATUS_FLAGS1_RT_RULE_ID_FMASK);
 }
 
 static void ipa_endpoint_status_parse(struct ipa_endpoint *endpoint,
@@ -1443,15 +1450,15 @@ static void ipa_endpoint_status_parse(struct ipa_endpoint *endpoint,
 
 		/* Skip over status packets that lack packet data */
 		length = le16_to_cpu(status->pkt_len);
-		if (!length || ipa_endpoint_status_skip(endpoint, status)) {
+		if (!length || ipa_endpoint_status_skip(endpoint, data)) {
 			data += IPA_STATUS_SIZE;
 			resid -= IPA_STATUS_SIZE;
 			continue;
 		}
 
 		/* Compute the amount of buffer space consumed by the packet,
-		 * including the status element.  If the hardware is configured
-		 * to pad packet data to an aligned boundary, account for that.
+		 * including the status.  If the hardware is configured to
+		 * pad packet data to an aligned boundary, account for that.
 		 * And if checksum offload is enabled a trailer containing
 		 * computed checksum information will be appended.
 		 */
@@ -1460,7 +1467,7 @@ static void ipa_endpoint_status_parse(struct ipa_endpoint *endpoint,
 		if (endpoint->config.checksum)
 			len += sizeof(struct rmnet_map_dl_csum_trailer);
 
-		if (!ipa_endpoint_status_drop(endpoint, status)) {
+		if (!ipa_endpoint_status_drop(endpoint, data)) {
 			void *data2;
 			u32 extra;
 
-- 
2.34.1

[PATCH net-next 7/8] net: ipa: introduce generalized status decoder

[Alex Elder]

Stop assuming the IPA packet status has a fixed format (defined by
a C structure).  Instead, use a function to extract each field from
a block of data interpreted as an IPA packet status.  Define an
enumerated type that identifies the fields that can be extracted.
The current function extracts fields based on the existing
ipa_status structure format (which is no longer used).

Define IPA_STATUS_RULE_MISS, to replace the calls to field_max() to
represent that condition; those depended on the knowing the width of
a filter or router rule in the IPA packet status structure.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/net/ipa/ipa_endpoint.c | 158 +++++++++++++++++++++++++--------
 1 file changed, 120 insertions(+), 38 deletions(-)

diff --git a/drivers/net/ipa/ipa_endpoint.c b/drivers/net/ipa/ipa_endpoint.c
index 5097eb1bbadb0..3f6c3e2b6ec95 100644
--- a/drivers/net/ipa/ipa_endpoint.c
+++ b/drivers/net/ipa/ipa_endpoint.c
@@ -81,32 +81,118 @@ enum ipa_status_mask {
 	IPA_STATUS_MASK_BYTE_LIMIT		= BIT(15),
 };
 
+/* Special IPA filter/router rule field value indicating "rule miss" */
+#define IPA_STATUS_RULE_MISS	0x3ff	/* 10-bit filter/router rule fields */
+
 /** The IPA status nat_type field uses enum ipa_nat_type hardware values */
 
-/* Status element provided by hardware */
-struct ipa_status {
-	u8 opcode;		/* enum ipa_status_opcode */
-	u8 exception;		/* enum ipa_status_exception */
-	__le16 mask;		/* enum ipa_status_bit (bitmask) */
-	__le16 pkt_len;
-	u8 endp_src_idx;
-	u8 endp_dst_idx;
-	__le32 metadata;
-	__le32 flags1;
-	__le64 flags2;
-	__le32 flags3;
-	__le32 flags4;
+/* enum ipa_status_field_id - IPA packet status structure field identifiers */
+enum ipa_status_field_id {
+	STATUS_OPCODE,			/* enum ipa_status_opcode */
+	STATUS_EXCEPTION,		/* enum ipa_status_exception */
+	STATUS_MASK,			/* enum ipa_status_mask (bitmask) */
+	STATUS_LENGTH,
+	STATUS_SRC_ENDPOINT,
+	STATUS_DST_ENDPOINT,
+	STATUS_METADATA,
+	STATUS_FILTER_LOCAL,		/* Boolean */
+	STATUS_FILTER_HASH,		/* Boolean */
+	STATUS_FILTER_GLOBAL,		/* Boolean */
+	STATUS_FILTER_RETAIN,		/* Boolean */
+	STATUS_FILTER_RULE_INDEX,
+	STATUS_ROUTER_LOCAL,		/* Boolean */
+	STATUS_ROUTER_HASH,		/* Boolean */
+	STATUS_UCP,			/* Boolean */
+	STATUS_ROUTER_TABLE,
+	STATUS_ROUTER_RULE_INDEX,
+	STATUS_NAT_HIT,			/* Boolean */
+	STATUS_NAT_INDEX,
+	STATUS_NAT_TYPE,		/* enum ipa_nat_type */
+	STATUS_TAG_LOW32,		/* Low-order 32 bits of 48-bit tag */
+	STATUS_TAG_HIGH16,		/* High-order 16 bits of 48-bit tag */
+	STATUS_SEQUENCE,
+	STATUS_TIME_OF_DAY,
+	STATUS_HEADER_LOCAL,		/* Boolean */
+	STATUS_HEADER_OFFSET,
+	STATUS_FRAG_HIT,		/* Boolean */
+	STATUS_FRAG_RULE_INDEX,
 };
 
-/* Field masks for struct ipa_status structure fields */
-#define IPA_STATUS_SRC_IDX_FMASK		GENMASK(4, 0)
-#define IPA_STATUS_DST_IDX_FMASK		GENMASK(4, 0)
-#define IPA_STATUS_FLAGS1_RT_RULE_ID_FMASK	GENMASK(31, 22)
-#define IPA_STATUS_FLAGS2_TAG_FMASK		GENMASK_ULL(63, 16)
-
 /* Size in bytes of an IPA packet status structure */
 #define IPA_STATUS_SIZE			sizeof(__le32[4])
 
+/* IPA status structure decoder; looks up field values for a structure */
+static u32 ipa_status_extract(const void *data, enum ipa_status_field_id field)
+{
+	const __le32 *word = data;
+
+	switch (field) {
+	case STATUS_OPCODE:
+		return le32_get_bits(word[0], GENMASK(7, 0));
+	case STATUS_EXCEPTION:
+		return le32_get_bits(word[0], GENMASK(15, 8));
+	case STATUS_MASK:
+		return le32_get_bits(word[0], GENMASK(31, 16));
+	case STATUS_LENGTH:
+		return le32_get_bits(word[1], GENMASK(15, 0));
+	case STATUS_SRC_ENDPOINT:
+		return le32_get_bits(word[1], GENMASK(20, 16));
+	/* Status word 1, bits 21-23 are reserved */
+	case STATUS_DST_ENDPOINT:
+		return le32_get_bits(word[1], GENMASK(28, 24));
+	/* Status word 1, bits 29-31 are reserved */
+	case STATUS_METADATA:
+		return le32_to_cpu(word[2]);
+	case STATUS_FILTER_LOCAL:
+		return le32_get_bits(word[3], GENMASK(0, 0));
+	case STATUS_FILTER_HASH:
+		return le32_get_bits(word[3], GENMASK(1, 1));
+	case STATUS_FILTER_GLOBAL:
+		return le32_get_bits(word[3], GENMASK(2, 2));
+	case STATUS_FILTER_RETAIN:
+		return le32_get_bits(word[3], GENMASK(3, 3));
+	case STATUS_FILTER_RULE_INDEX:
+		return le32_get_bits(word[3], GENMASK(13, 4));
+	case STATUS_ROUTER_LOCAL:
+		return le32_get_bits(word[3], GENMASK(14, 14));
+	case STATUS_ROUTER_HASH:
+		return le32_get_bits(word[3], GENMASK(15, 15));
+	case STATUS_UCP:
+		return le32_get_bits(word[3], GENMASK(16, 16));
+	case STATUS_ROUTER_TABLE:
+		return le32_get_bits(word[3], GENMASK(21, 17));
+	case STATUS_ROUTER_RULE_INDEX:
+		return le32_get_bits(word[3], GENMASK(31, 22));
+	case STATUS_NAT_HIT:
+		return le32_get_bits(word[4], GENMASK(0, 0));
+	case STATUS_NAT_INDEX:
+		return le32_get_bits(word[4], GENMASK(13, 1));
+	case STATUS_NAT_TYPE:
+		return le32_get_bits(word[4], GENMASK(15, 14));
+	case STATUS_TAG_LOW32:
+		return le32_get_bits(word[4], GENMASK(31, 16)) |
+			(le32_get_bits(word[5], GENMASK(15, 0)) << 16);
+	case STATUS_TAG_HIGH16:
+		return le32_get_bits(word[5], GENMASK(31, 16));
+	case STATUS_SEQUENCE:
+		return le32_get_bits(word[6], GENMASK(7, 0));
+	case STATUS_TIME_OF_DAY:
+		return le32_get_bits(word[6], GENMASK(31, 8));
+	case STATUS_HEADER_LOCAL:
+		return le32_get_bits(word[7], GENMASK(0, 0));
+	case STATUS_HEADER_OFFSET:
+		return le32_get_bits(word[7], GENMASK(10, 1));
+	case STATUS_FRAG_HIT:
+		return le32_get_bits(word[7], GENMASK(11, 11));
+	case STATUS_FRAG_RULE_INDEX:
+		return le32_get_bits(word[7], GENMASK(15, 12));
+	/* Status word 7, bits 16-31 are reserved */
+	default:
+		WARN(true, "%s: bad field_id %u\n", __func__, field);
+		return 0;
+	}
+}
+
 /* Compute the aggregation size value to use for a given buffer size */
 static u32 ipa_aggr_size_kb(u32 rx_buffer_size, bool aggr_hard_limit)
 {
@@ -1355,33 +1441,32 @@ static bool ipa_status_format_packet(enum ipa_status_opcode opcode)
 	}
 }
 
-static bool ipa_endpoint_status_skip(struct ipa_endpoint *endpoint,
-				     const struct ipa_status *status)
+static bool
+ipa_endpoint_status_skip(struct ipa_endpoint *endpoint, const void *data)
 {
 	enum ipa_status_opcode opcode;
 	u32 endpoint_id;
 
-	opcode = status->opcode;
+	opcode = ipa_status_extract(data, STATUS_OPCODE);
 	if (!ipa_status_format_packet(opcode))
 		return true;
 
-	endpoint_id = u8_get_bits(status->endp_dst_idx,
-				  IPA_STATUS_DST_IDX_FMASK);
+	endpoint_id = ipa_status_extract(data, STATUS_DST_ENDPOINT);
 	if (endpoint_id != endpoint->endpoint_id)
 		return true;
 
 	return false;	/* Don't skip this packet, process it */
 }
 
-static bool ipa_endpoint_status_tag_valid(struct ipa_endpoint *endpoint,
-					  const struct ipa_status *status)
+static bool
+ipa_endpoint_status_tag_valid(struct ipa_endpoint *endpoint, const void *data)
 {
 	struct ipa_endpoint *command_endpoint;
 	enum ipa_status_mask status_mask;
 	struct ipa *ipa = endpoint->ipa;
 	u32 endpoint_id;
 
-	status_mask = le16_get_bits(status->mask, IPA_STATUS_MASK_TAG_VALID);
+	status_mask = ipa_status_extract(data, STATUS_MASK);
 	if (!status_mask)
 		return false;	/* No valid tag */
 
@@ -1390,8 +1475,7 @@ static bool ipa_endpoint_status_tag_valid(struct ipa_endpoint *endpoint,
 	 * If the packet came from the AP->command TX endpoint we know
 	 * this packet was sent as part of the pipeline clear process.
 	 */
-	endpoint_id = u8_get_bits(status->endp_src_idx,
-				  IPA_STATUS_SRC_IDX_FMASK);
+	endpoint_id = ipa_status_extract(data, STATUS_SRC_ENDPOINT);
 	command_endpoint = ipa->name_map[IPA_ENDPOINT_AP_COMMAND_TX];
 	if (endpoint_id == command_endpoint->endpoint_id) {
 		complete(&ipa->completion);
@@ -1405,26 +1489,25 @@ static bool ipa_endpoint_status_tag_valid(struct ipa_endpoint *endpoint,
 }
 
 /* Return whether the status indicates the packet should be dropped */
-static bool ipa_endpoint_status_drop(struct ipa_endpoint *endpoint,
-				     const struct ipa_status *status)
+static bool
+ipa_endpoint_status_drop(struct ipa_endpoint *endpoint, const void *data)
 {
 	enum ipa_status_exception exception;
 	u32 rule;
 
 	/* If the status indicates a tagged transfer, we'll drop the packet */
-	if (ipa_endpoint_status_tag_valid(endpoint, status))
+	if (ipa_endpoint_status_tag_valid(endpoint, data))
 		return true;
 
 	/* Deaggregation exceptions we drop; all other types we consume */
-	exception = status->exception;
+	exception = ipa_status_extract(data, STATUS_EXCEPTION);
 	if (exception)
 		return exception == IPA_STATUS_EXCEPTION_DEAGGR;
 
 	/* Drop the packet if it fails to match a routing rule; otherwise no */
-	rule = le32_get_bits(status->flags1,
-			     IPA_STATUS_FLAGS1_RT_RULE_ID_FMASK);
+	rule = ipa_status_extract(data, STATUS_ROUTER_RULE_INDEX);
 
-	return rule == field_max(IPA_STATUS_FLAGS1_RT_RULE_ID_FMASK);
+	return rule == IPA_STATUS_RULE_MISS;
 }
 
 static void ipa_endpoint_status_parse(struct ipa_endpoint *endpoint,
@@ -1436,7 +1519,6 @@ static void ipa_endpoint_status_parse(struct ipa_endpoint *endpoint,
 	u32 resid = total_len;
 
 	while (resid) {
-		const struct ipa_status *status = data;
 		u32 length;
 		u32 align;
 		u32 len;
@@ -1449,7 +1531,7 @@ static void ipa_endpoint_status_parse(struct ipa_endpoint *endpoint,
 		}
 
 		/* Skip over status packets that lack packet data */
-		length = le16_to_cpu(status->pkt_len);
+		length = ipa_status_extract(data, STATUS_LENGTH);
 		if (!length || ipa_endpoint_status_skip(endpoint, data)) {
 			data += IPA_STATUS_SIZE;
 			resid -= IPA_STATUS_SIZE;
-- 
2.34.1

[PATCH net-next 8/8] net: ipa: add IPA v5.0 packet status support

[Alex Elder]

Update ipa_status_extract() to support IPA v5.0 and beyond.  Because
the format of the IPA packet status depends on the version, pass an
IPA pointer to the function.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/net/ipa/ipa_endpoint.c | 52 +++++++++++++++++++++++-----------
 1 file changed, 36 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ipa/ipa_endpoint.c b/drivers/net/ipa/ipa_endpoint.c
index 3f6c3e2b6ec95..ce7f2d6e447ed 100644
--- a/drivers/net/ipa/ipa_endpoint.c
+++ b/drivers/net/ipa/ipa_endpoint.c
@@ -122,8 +122,10 @@ enum ipa_status_field_id {
 #define IPA_STATUS_SIZE			sizeof(__le32[4])
 
 /* IPA status structure decoder; looks up field values for a structure */
-static u32 ipa_status_extract(const void *data, enum ipa_status_field_id field)
+static u32 ipa_status_extract(struct ipa *ipa, const void *data,
+			      enum ipa_status_field_id field)
 {
+	enum ipa_version version = ipa->version;
 	const __le32 *word = data;
 
 	switch (field) {
@@ -136,10 +138,15 @@ static u32 ipa_status_extract(const void *data, enum ipa_status_field_id field)
 	case STATUS_LENGTH:
 		return le32_get_bits(word[1], GENMASK(15, 0));
 	case STATUS_SRC_ENDPOINT:
-		return le32_get_bits(word[1], GENMASK(20, 16));
-	/* Status word 1, bits 21-23 are reserved */
+		if (version < IPA_VERSION_5_0)
+			return le32_get_bits(word[1], GENMASK(20, 16));
+		return le32_get_bits(word[1], GENMASK(23, 16));
+	/* Status word 1, bits 21-23 are reserved (not IPA v5.0+) */
+	/* Status word 1, bits 24-26 are reserved (IPA v5.0+) */
 	case STATUS_DST_ENDPOINT:
-		return le32_get_bits(word[1], GENMASK(28, 24));
+		if (version < IPA_VERSION_5_0)
+			return le32_get_bits(word[1], GENMASK(28, 24));
+		return le32_get_bits(word[7], GENMASK(23, 16));
 	/* Status word 1, bits 29-31 are reserved */
 	case STATUS_METADATA:
 		return le32_to_cpu(word[2]);
@@ -153,14 +160,23 @@ static u32 ipa_status_extract(const void *data, enum ipa_status_field_id field)
 		return le32_get_bits(word[3], GENMASK(3, 3));
 	case STATUS_FILTER_RULE_INDEX:
 		return le32_get_bits(word[3], GENMASK(13, 4));
+	/* ROUTER_TABLE is in word 3, bits 14-21 (IPA v5.0+) */
 	case STATUS_ROUTER_LOCAL:
-		return le32_get_bits(word[3], GENMASK(14, 14));
+		if (version < IPA_VERSION_5_0)
+			return le32_get_bits(word[3], GENMASK(14, 14));
+		return le32_get_bits(word[1], GENMASK(27, 27));
 	case STATUS_ROUTER_HASH:
-		return le32_get_bits(word[3], GENMASK(15, 15));
+		if (version < IPA_VERSION_5_0)
+			return le32_get_bits(word[3], GENMASK(15, 15));
+		return le32_get_bits(word[1], GENMASK(28, 28));
 	case STATUS_UCP:
-		return le32_get_bits(word[3], GENMASK(16, 16));
+		if (version < IPA_VERSION_5_0)
+			return le32_get_bits(word[3], GENMASK(16, 16));
+		return le32_get_bits(word[7], GENMASK(31, 31));
 	case STATUS_ROUTER_TABLE:
-		return le32_get_bits(word[3], GENMASK(21, 17));
+		if (version < IPA_VERSION_5_0)
+			return le32_get_bits(word[3], GENMASK(21, 17));
+		return le32_get_bits(word[3], GENMASK(21, 14));
 	case STATUS_ROUTER_RULE_INDEX:
 		return le32_get_bits(word[3], GENMASK(31, 22));
 	case STATUS_NAT_HIT:
@@ -186,7 +202,8 @@ static u32 ipa_status_extract(const void *data, enum ipa_status_field_id field)
 		return le32_get_bits(word[7], GENMASK(11, 11));
 	case STATUS_FRAG_RULE_INDEX:
 		return le32_get_bits(word[7], GENMASK(15, 12));
-	/* Status word 7, bits 16-31 are reserved */
+	/* Status word 7, bits 16-30 are reserved */
+	/* Status word 7, bit 31 is reserved (not IPA v5.0+) */
 	default:
 		WARN(true, "%s: bad field_id %u\n", __func__, field);
 		return 0;
@@ -1444,14 +1461,15 @@ static bool ipa_status_format_packet(enum ipa_status_opcode opcode)
 static bool
 ipa_endpoint_status_skip(struct ipa_endpoint *endpoint, const void *data)
 {
+	struct ipa *ipa = endpoint->ipa;
 	enum ipa_status_opcode opcode;
 	u32 endpoint_id;
 
-	opcode = ipa_status_extract(data, STATUS_OPCODE);
+	opcode = ipa_status_extract(ipa, data, STATUS_OPCODE);
 	if (!ipa_status_format_packet(opcode))
 		return true;
 
-	endpoint_id = ipa_status_extract(data, STATUS_DST_ENDPOINT);
+	endpoint_id = ipa_status_extract(ipa, data, STATUS_DST_ENDPOINT);
 	if (endpoint_id != endpoint->endpoint_id)
 		return true;
 
@@ -1466,7 +1484,7 @@ ipa_endpoint_status_tag_valid(struct ipa_endpoint *endpoint, const void *data)
 	struct ipa *ipa = endpoint->ipa;
 	u32 endpoint_id;
 
-	status_mask = ipa_status_extract(data, STATUS_MASK);
+	status_mask = ipa_status_extract(ipa, data, STATUS_MASK);
 	if (!status_mask)
 		return false;	/* No valid tag */
 
@@ -1475,7 +1493,7 @@ ipa_endpoint_status_tag_valid(struct ipa_endpoint *endpoint, const void *data)
 	 * If the packet came from the AP->command TX endpoint we know
 	 * this packet was sent as part of the pipeline clear process.
 	 */
-	endpoint_id = ipa_status_extract(data, STATUS_SRC_ENDPOINT);
+	endpoint_id = ipa_status_extract(ipa, data, STATUS_SRC_ENDPOINT);
 	command_endpoint = ipa->name_map[IPA_ENDPOINT_AP_COMMAND_TX];
 	if (endpoint_id == command_endpoint->endpoint_id) {
 		complete(&ipa->completion);
@@ -1493,6 +1511,7 @@ static bool
 ipa_endpoint_status_drop(struct ipa_endpoint *endpoint, const void *data)
 {
 	enum ipa_status_exception exception;
+	struct ipa *ipa = endpoint->ipa;
 	u32 rule;
 
 	/* If the status indicates a tagged transfer, we'll drop the packet */
@@ -1500,12 +1519,12 @@ ipa_endpoint_status_drop(struct ipa_endpoint *endpoint, const void *data)
 		return true;
 
 	/* Deaggregation exceptions we drop; all other types we consume */
-	exception = ipa_status_extract(data, STATUS_EXCEPTION);
+	exception = ipa_status_extract(ipa, data, STATUS_EXCEPTION);
 	if (exception)
 		return exception == IPA_STATUS_EXCEPTION_DEAGGR;
 
 	/* Drop the packet if it fails to match a routing rule; otherwise no */
-	rule = ipa_status_extract(data, STATUS_ROUTER_RULE_INDEX);
+	rule = ipa_status_extract(ipa, data, STATUS_ROUTER_RULE_INDEX);
 
 	return rule == IPA_STATUS_RULE_MISS;
 }
@@ -1516,6 +1535,7 @@ static void ipa_endpoint_status_parse(struct ipa_endpoint *endpoint,
 	u32 buffer_size = endpoint->config.rx.buffer_size;
 	void *data = page_address(page) + NET_SKB_PAD;
 	u32 unused = buffer_size - total_len;
+	struct ipa *ipa = endpoint->ipa;
 	u32 resid = total_len;
 
 	while (resid) {
@@ -1531,7 +1551,7 @@ static void ipa_endpoint_status_parse(struct ipa_endpoint *endpoint,
 		}
 
 		/* Skip over status packets that lack packet data */
-		length = ipa_status_extract(data, STATUS_LENGTH);
+		length = ipa_status_extract(ipa, data, STATUS_LENGTH);
 		if (!length || ipa_endpoint_status_skip(endpoint, data)) {
 			data += IPA_STATUS_SIZE;
 			resid -= IPA_STATUS_SIZE;
-- 
2.34.1

Re: [PATCH net-next 0/8] net: ipa: abstract status parsing

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (master)
by David S. Miller <davem@davemloft.net>:

On Wed, 25 Jan 2023 14:45:37 -0600 you wrote:
> Under some circumstances, IPA generates a "packet status" structure
> that describes information about a packet.  This is used, for
> example, when offload hardware detects an error in a packet, or
> otherwise discovers a packet needs special handling.  In this case,
> the status is delivered (along with the packet it describes) to a
> "default" endpoint so that it can be handled by the AP.
> 
> [...]

Here is the summary with links:
  - [net-next,1/8] net: ipa: refactor status buffer parsing
    https://git.kernel.org/netdev/net-next/c/63a560b5289a
  - [net-next,2/8] net: ipa: stop using sizeof(status)
    https://git.kernel.org/netdev/net-next/c/b8dc7d0eea5a
  - [net-next,3/8] net: ipa: define all IPA status mask bits
    https://git.kernel.org/netdev/net-next/c/8e71708bb25e
  - [net-next,4/8] net: ipa: rename the NAT enumerated type
    https://git.kernel.org/netdev/net-next/c/cbea4761173d
  - [net-next,5/8] net: ipa: define remaining IPA status field values
    https://git.kernel.org/netdev/net-next/c/ec4c24f6a511
  - [net-next,6/8] net: ipa: IPA status preparatory cleanups
    https://git.kernel.org/netdev/net-next/c/02c5077439fc
  - [net-next,7/8] net: ipa: introduce generalized status decoder
    https://git.kernel.org/netdev/net-next/c/ebd2a82ecea8
  - [net-next,8/8] net: ipa: add IPA v5.0 packet status support
    https://git.kernel.org/netdev/net-next/c/55c6eae70ff1

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html