net: ipa: back-port concurrent replenish fix

PATCH In review Alex Elder 2 participants 4 messages Feb 1, 2022 – Feb 3, 2022

Subsystems (3)

Network drivers Stable branch Qualcomm IPA driver

lore

Share & tools

lore

Alex Elder

author

2022-02-01 01:17 UTC #0 lore raw

cover net: ipa: back-port concurrent replenish fix

Attached are two patches that take the place of one IPA fix that was requested for back-port. The original did not apply cleanly, so one prerequisite was back-ported, followed by the "real" fix. They are built upon Linux v5.10.95.

I wasn't sure how I was supposed to indicate this was a manual back-port. I added the upstream commit ID in each patch but it needs to be edited. And I added a "References" tag indicating the e-mail to which this small series responds.

Please let me know if there's something else/more I should do.

Thanks.

-Alex

Alex Elder (2):
net: ipa: use a bitmap for endpoint replenish_enabled
net: ipa: prevent concurrent replenish

 drivers/net/ipa/ipa_endpoint.c | 20 ++++++++++++++++----
 drivers/net/ipa/ipa_endpoint.h | 15 ++++++++++++++-
 2 files changed, 30 insertions(+), 5 deletions(-)

Signature · 1 line

2.32.0

Alex Elder

author

2022-02-01 01:17 UTC #1 lore raw

PATCH 1/2 net: ipa: use a bitmap for endpoint replenish_enabled

    XXX commit c1aaa01dbf4cef95af3e04a5a43986c290e06ea3 upstream.

Define a new replenish_flags bitmap to contain Boolean flags associated with an endpoint's replenishing state. Replace the replenish_enabled field with a flag in that bitmap. This is to prepare for the next patch, which adds another flag.

References: https://lore.kernel.org/stable/[email protected]

Signed-off-by Alex Elder <[email protected]>

Signed-off-by David S. Miller <[email protected]>

---
 drivers/net/ipa/ipa_endpoint.c |  8 ++++----
 drivers/net/ipa/ipa_endpoint.h | 13 ++++++++++++-
 2 files changed, 16 insertions(+), 5 deletions(-)

drivers/net/ipa/ipa_endpoint.c +4 -4

index a37aae00e128f..80380f5966142 100644
@@ -901,7 +901,7 @@ static void ipa_endpoint_replenish(struct ipa_endpoint *endpoint, u32 count)
 	struct gsi *gsi;
 	u32 backlog;
 
-	if (!endpoint->replenish_enabled) {
+	if (!test_bit(IPA_REPLENISH_ENABLED, endpoint->replenish_flags)) {
 		if (count)
 			atomic_add(count, &endpoint->replenish_saved);
 		return;

@@ -941,7 +941,7 @@ static void ipa_endpoint_replenish_enable(struct ipa_endpoint *endpoint)
 	u32 max_backlog;
 	u32 saved;
 
-	endpoint->replenish_enabled = true;
+	set_bit(IPA_REPLENISH_ENABLED, endpoint->replenish_flags);
 	while ((saved = atomic_xchg(&endpoint->replenish_saved, 0)))
 		atomic_add(saved, &endpoint->replenish_backlog);

@@ -955,7 +955,7 @@ static void ipa_endpoint_replenish_disable(struct ipa_endpoint *endpoint)
 {
 	u32 backlog;
 
-	endpoint->replenish_enabled = false;
+	clear_bit(IPA_REPLENISH_ENABLED, endpoint->replenish_flags);
 	while ((backlog = atomic_xchg(&endpoint->replenish_backlog, 0)))
 		atomic_add(backlog, &endpoint->replenish_saved);
 }

@@ -1472,7 +1472,7 @@ static void ipa_endpoint_setup_one(struct ipa_endpoint *endpoint)
 		/* RX transactions require a single TRE, so the maximum
 		 * backlog is the same as the maximum outstanding TREs.
 		 */
-		endpoint->replenish_enabled = false;
+		clear_bit(IPA_REPLENISH_ENABLED, endpoint->replenish_flags);
 		atomic_set(&endpoint->replenish_saved,
 			   gsi_channel_tre_max(gsi, endpoint->channel_id));
 		atomic_set(&endpoint->replenish_backlog, 0);

drivers/net/ipa/ipa_endpoint.h +12 -1

index 58a245de488e8..ffae393500d4f 100644
@@ -39,6 +39,17 @@ enum ipa_endpoint_name {
 
 #define IPA_ENDPOINT_MAX		32	/* Max supported by driver */
 
+/**
+ * enum ipa_replenish_flag:	RX buffer replenish flags
+ *
+ * @IPA_REPLENISH_ENABLED:	Whether receive buffer replenishing is enabled
+ * @IPA_REPLENISH_COUNT:	Number of defined replenish flags
+ */
+enum ipa_replenish_flag {
+	IPA_REPLENISH_ENABLED,
+	IPA_REPLENISH_COUNT,	/* Number of flags (must be last) */
+};
+
 /**
  * struct ipa_endpoint - IPA endpoint information
  * @channel_id:	EP's GSI channel

@@ -60,7 +71,7 @@ struct ipa_endpoint {
 	struct net_device *netdev;
 
 	/* Receive buffer replenishing for RX endpoints */
-	bool replenish_enabled;
+	DECLARE_BITMAP(replenish_flags, IPA_REPLENISH_COUNT);
 	u32 replenish_ready;
 	atomic_t replenish_saved;
 	atomic_t replenish_backlog;

Signature · 1 line

2.32.0

Alex Elder

author

2022-02-01 01:17 UTC #2 lore raw

PATCH 2/2 net: ipa: prevent concurrent replenish

In reply to Alex Elder

    XXX commit 998c0bd2b3715244da7639cc4e6a2062cb79c3f4 upstream.

We have seen cases where an endpoint RX completion interrupt arrives while replenishing for the endpoint is underway. This causes another instance of replenishing to begin as part of completing the receive transaction. If this occurs it can lead to transaction corruption.

Use a new flag to ensure only one replenish instance for an endpoint executes at a time.

References: https://lore.kernel.org/stable/[email protected]

Fixes 84f9bd12d46db ("soc: qcom: ipa: IPA endpoints")

Signed-off-by Alex Elder <[email protected]>

Signed-off-by David S. Miller <[email protected]>

---
 drivers/net/ipa/ipa_endpoint.c | 12 ++++++++++++
 drivers/net/ipa/ipa_endpoint.h |  2 ++
 2 files changed, 14 insertions(+)

drivers/net/ipa/ipa_endpoint.c +12 -0

index 80380f5966142..2e1e558275b1c 100644
@@ -907,16 +907,27 @@ static void ipa_endpoint_replenish(struct ipa_endpoint *endpoint, u32 count)
 		return;
 	}
 
+	/* If already active, just update the backlog */
+	if (test_and_set_bit(IPA_REPLENISH_ACTIVE, endpoint->replenish_flags)) {
+		if (count)
+			atomic_add(count, &endpoint->replenish_backlog);
+		return;
+	}
 
 	while (atomic_dec_not_zero(&endpoint->replenish_backlog))
 		if (ipa_endpoint_replenish_one(endpoint))
 			goto try_again_later;
+
+	clear_bit(IPA_REPLENISH_ACTIVE, endpoint->replenish_flags);
+
 	if (count)
 		atomic_add(count, &endpoint->replenish_backlog);
 
 	return;
 
 try_again_later:
+	clear_bit(IPA_REPLENISH_ACTIVE, endpoint->replenish_flags);
+
 	/* The last one didn't succeed, so fix the backlog */
 	backlog = atomic_inc_return(&endpoint->replenish_backlog);

@@ -1473,6 +1484,7 @@ static void ipa_endpoint_setup_one(struct ipa_endpoint *endpoint)
 		 * backlog is the same as the maximum outstanding TREs.
 		 */
 		clear_bit(IPA_REPLENISH_ENABLED, endpoint->replenish_flags);
+		clear_bit(IPA_REPLENISH_ACTIVE, endpoint->replenish_flags);
 		atomic_set(&endpoint->replenish_saved,
 			   gsi_channel_tre_max(gsi, endpoint->channel_id));
 		atomic_set(&endpoint->replenish_backlog, 0);

drivers/net/ipa/ipa_endpoint.h +2 -0

index ffae393500d4f..823c4a1296587 100644
@@ -43,10 +43,12 @@ enum ipa_endpoint_name {
  * enum ipa_replenish_flag:	RX buffer replenish flags
  *
  * @IPA_REPLENISH_ENABLED:	Whether receive buffer replenishing is enabled
+ * @IPA_REPLENISH_ACTIVE:	Whether replenishing is underway
  * @IPA_REPLENISH_COUNT:	Number of defined replenish flags
  */
 enum ipa_replenish_flag {
 	IPA_REPLENISH_ENABLED,
+	IPA_REPLENISH_ACTIVE,
 	IPA_REPLENISH_COUNT,	/* Number of flags (must be last) */
 };

Signature · 1 line

2.32.0

Greg KH

maintainer

2022-02-03 16:59 UTC #3 lore raw

In reply to Alex Elder

Quoted 16 lines · Alex Elder

Attached are two patches that take the place of one IPA fix that was requested for back-port. The original did not apply cleanly, so one prerequisite was back-ported, followed by the "real" fix. They are built upon Linux v5.10.95.
I wasn't sure how I was supposed to indicate this was a manual back-port. I added the upstream commit ID in each patch but it needs to be edited. And I added a "References" tag indicating the e-mail to which this small series responds.
Please let me know if there's something else/more I should do.
Thanks.
-Alex

Quoted 11 lines

Alex Elder (2):
net: ipa: use a bitmap for endpoint replenish_enabled
net: ipa: prevent concurrent replenish
 drivers/net/ipa/ipa_endpoint.c | 20 ++++++++++++++++----
 drivers/net/ipa/ipa_endpoint.h | 15 ++++++++++++++-
 2 files changed, 30 insertions(+), 5 deletions(-)
Signature · 1 line
2.32.0

All now queued up, thanks!