Re: [PATCH] net: ipa: fix some resource limit max values

From: Caleb Connolly
Date: Mon Oct 24 2022 - 18:07:17 EST




On 24/10/2022 20:15, Alex Elder wrote:
On 10/24/22 11:56 AM, Caleb Connolly wrote:
Some resource limits on IPA v3.1 and v3.5.1 have their max values set to
255, this causes a few splats in ipa_reg_encode and prevents it from booting.
The limits are all 6 bits wide so adjust the max values to 63.

Thank you for sending this Caleb.

On IPA v3.5.1 (SDM845) I confirm that these resource limit fields are
6 bits wide, while the values we assign are in some cases 255, which
cannot be represented in 6 bits.  Your fix in this case is proper,
changing the maximum limit from 255 to be 63.  (Just in case, I've
sent a note to Qualcomm to ask them to confirm this, but I think this
is fine.)

Great, thanks


I re-checked the definitions of the MIN_LIMIT and MAX_LIMIT fields
for IPA v3.1, and it turns out in that case the *register field*
definitions were wrong.  They should, in fact, be 8 bits wide rather
than just 6.  So in that case, 255 would be a reasonable limit value.

Heh, well that's fun... Thanks for checking


Did you observe these splats when doing actual testing on an msm8998
(which has IPA v3.1)?  Or did you just double-check the code?  I
looked at the other currently-supported platforms and didn't see
this sort of problem elsewhere (IPA v4.2, 4.5, 4.9, 4.11).

I found these just by 'grep'ing for "max = 255", none of the other versions had that and I didn't see anything obvious at a glance so I expect only these two platforms are affected. The same issue has been confirmed on MSM8998: https://gitlab.com/msm8998-mainline/linux/-/issues/39

Jami (CC'd) has offered to test the next revision of the fix there so we can be sure it works on v3.1 and v3.5.1.


Could you please send a new version of your patch, which fixes the
register definition in "ipa_reg-v3.1.c" instead?

It might be best to fix the two issues in separate patches, since
they will parts pf the code with different development histories.

That makes sense, will do.

Thanks!

                    -Alex

Fixes: 1c418c4a929c ("net: ipa: define resource group/type IPA register fields")
Signed-off-by: Caleb Connolly <caleb.connolly@xxxxxxxxxx>
---
  drivers/net/ipa/data/ipa_data-v3.1.c   | 62 +++++++++++++-------------
  drivers/net/ipa/data/ipa_data-v3.5.1.c |  4 +-
  2 files changed, 33 insertions(+), 33 deletions(-)

diff --git a/drivers/net/ipa/data/ipa_data-v3.1.c b/drivers/net/ipa/data/ipa_data-v3.1.c
index e0d71f609272..7ff093f982ad 100644
--- a/drivers/net/ipa/data/ipa_data-v3.1.c
+++ b/drivers/net/ipa/data/ipa_data-v3.1.c
@@ -187,53 +187,53 @@ static const struct ipa_gsi_endpoint_data ipa_gsi_endpoint_data[] = {
  static const struct ipa_resource ipa_resource_src[] = {
      [IPA_RESOURCE_TYPE_SRC_PKT_CONTEXTS] = {
          .limits[IPA_RSRC_GROUP_SRC_UL] = {
-            .min = 3,    .max = 255,
+            .min = 3,    .max = 63,
          },
          .limits[IPA_RSRC_GROUP_SRC_DL] = {
-            .min = 3,    .max = 255,
+            .min = 3,    .max = 63,
          },
          .limits[IPA_RSRC_GROUP_SRC_DIAG] = {
-            .min = 1,    .max = 255,
+            .min = 1,    .max = 63,
          },
          .limits[IPA_RSRC_GROUP_SRC_DMA] = {
-            .min = 1,    .max = 255,
+            .min = 1,    .max = 63,
          },
          .limits[IPA_RSRC_GROUP_SRC_UC_RX_Q] = {
-            .min = 2,    .max = 255,
+            .min = 2,    .max = 63,
          },
      },
      [IPA_RESOURCE_TYPE_SRC_HDR_SECTORS] = {
          .limits[IPA_RSRC_GROUP_SRC_UL] = {
-            .min = 0,    .max = 255,
+            .min = 0,    .max = 63,
          },
          .limits[IPA_RSRC_GROUP_SRC_DL] = {
-            .min = 0,    .max = 255,
+            .min = 0,    .max = 63,
          },
          .limits[IPA_RSRC_GROUP_SRC_DIAG] = {
-            .min = 0,    .max = 255,
+            .min = 0,    .max = 63,
          },
          .limits[IPA_RSRC_GROUP_SRC_DMA] = {
-            .min = 0,    .max = 255,
+            .min = 0,    .max = 63,
          },
          .limits[IPA_RSRC_GROUP_SRC_UC_RX_Q] = {
-            .min = 0,    .max = 255,
+            .min = 0,    .max = 63,
          },
      },
      [IPA_RESOURCE_TYPE_SRC_HDRI1_BUFFER] = {
          .limits[IPA_RSRC_GROUP_SRC_UL] = {
-            .min = 0,    .max = 255,
+            .min = 0,    .max = 63,
          },
          .limits[IPA_RSRC_GROUP_SRC_DL] = {
-            .min = 0,    .max = 255,
+            .min = 0,    .max = 63,
          },
          .limits[IPA_RSRC_GROUP_SRC_DIAG] = {
-            .min = 0,    .max = 255,
+            .min = 0,    .max = 63,
          },
          .limits[IPA_RSRC_GROUP_SRC_DMA] = {
-            .min = 0,    .max = 255,
+            .min = 0,    .max = 63,
          },
          .limits[IPA_RSRC_GROUP_SRC_UC_RX_Q] = {
-            .min = 0,    .max = 255,
+            .min = 0,    .max = 63,
          },
      },
      [IPA_RESOURCE_TYPE_SRC_DESCRIPTOR_LISTS] = {
@@ -272,36 +272,36 @@ static const struct ipa_resource ipa_resource_src[] = {
      },
      [IPA_RESOURCE_TYPE_SRC_HDRI2_BUFFERS] = {
          .limits[IPA_RSRC_GROUP_SRC_UL] = {
-            .min = 0,    .max = 255,
+            .min = 0,    .max = 63,
          },
          .limits[IPA_RSRC_GROUP_SRC_DL] = {
-            .min = 0,    .max = 255,
+            .min = 0,    .max = 63,
          },
          .limits[IPA_RSRC_GROUP_SRC_DIAG] = {
-            .min = 0,    .max = 255,
+            .min = 0,    .max = 63,
          },
          .limits[IPA_RSRC_GROUP_SRC_DMA] = {
-            .min = 0,    .max = 255,
+            .min = 0,    .max = 63,
          },
          .limits[IPA_RSRC_GROUP_SRC_UC_RX_Q] = {
-            .min = 0,    .max = 255,
+            .min = 0,    .max = 63,
          },
      },
      [IPA_RESOURCE_TYPE_SRC_HPS_DMARS] = {
          .limits[IPA_RSRC_GROUP_SRC_UL] = {
-            .min = 0,    .max = 255,
+            .min = 0,    .max = 63,
          },
          .limits[IPA_RSRC_GROUP_SRC_DL] = {
-            .min = 0,    .max = 255,
+            .min = 0,    .max = 63,
          },
          .limits[IPA_RSRC_GROUP_SRC_DIAG] = {
-            .min = 0,    .max = 255,
+            .min = 0,    .max = 63,
          },
          .limits[IPA_RSRC_GROUP_SRC_DMA] = {
-            .min = 0,    .max = 255,
+            .min = 0,    .max = 63,
          },
          .limits[IPA_RSRC_GROUP_SRC_UC_RX_Q] = {
-            .min = 0,    .max = 255,
+            .min = 0,    .max = 63,
          },
      },
      [IPA_RESOURCE_TYPE_SRC_ACK_ENTRIES] = {
@@ -345,22 +345,22 @@ static const struct ipa_resource ipa_resource_dst[] = {
      },
      [IPA_RESOURCE_TYPE_DST_DATA_SECTOR_LISTS] = {
          .limits[IPA_RSRC_GROUP_DST_UL] = {
-            .min = 0,    .max = 255,
+            .min = 0,    .max = 63,
          },
          .limits[IPA_RSRC_GROUP_DST_DL] = {
-            .min = 0,    .max = 255,
+            .min = 0,    .max = 63,
          },
          .limits[IPA_RSRC_GROUP_DST_DIAG_DPL] = {
-            .min = 0,    .max = 255,
+            .min = 0,    .max = 63,
          },
          .limits[IPA_RSRC_GROUP_DST_DMA] = {
-            .min = 0,    .max = 255,
+            .min = 0,    .max = 63,
          },
          .limits[IPA_RSRC_GROUP_DST_Q6ZIP_GENERAL] = {
-            .min = 0,    .max = 255,
+            .min = 0,    .max = 63,
          },
          .limits[IPA_RSRC_GROUP_DST_Q6ZIP_ENGINE] = {
-            .min = 0,    .max = 255,
+            .min = 0,    .max = 63,
          },
      },
      [IPA_RESOURCE_TYPE_DST_DPS_DMARS] = {
diff --git a/drivers/net/ipa/data/ipa_data-v3.5.1.c b/drivers/net/ipa/data/ipa_data-v3.5.1.c
index 383ef1890065..42f2c88a92d4 100644
--- a/drivers/net/ipa/data/ipa_data-v3.5.1.c
+++ b/drivers/net/ipa/data/ipa_data-v3.5.1.c
@@ -179,10 +179,10 @@ static const struct ipa_gsi_endpoint_data ipa_gsi_endpoint_data[] = {
  static const struct ipa_resource ipa_resource_src[] = {
      [IPA_RESOURCE_TYPE_SRC_PKT_CONTEXTS] = {
          .limits[IPA_RSRC_GROUP_SRC_LWA_DL] = {
-            .min = 1,    .max = 255,
+            .min = 1,    .max = 63,
          },
          .limits[IPA_RSRC_GROUP_SRC_UL_DL] = {
-            .min = 1,    .max = 255,
+            .min = 1,    .max = 63,
          },
          .limits[IPA_RSRC_GROUP_SRC_UC_RX_Q] = {
              .min = 1,    .max = 63,


--
Kind Regards,
Caleb (they/them)