LinuxLists.cc - [PATCH v2 1/2] net: ipa: fix v3.5.1 resource limit max values

2022-10-24 23:05:58

Subject: [PATCH v2 1/2] net: ipa: fix v3.5.1 resource limit max values

Some resource limits on IPA v3.5.1 have their max values set to
255, this causes a few splats in ipa_reg_encode and prevents the
IPA from booting properly. The limits are all 6 bits wide so
adjust the max values to 63.

Fixes: 1c418c4a929c ("net: ipa: define resource group/type IPA register fields")
Signed-off-by: Caleb Connolly <[email protected]>
---
V1: https://lore.kernel.org/netdev/[email protected]/
Changes since v1:
* Apply the correct fix for v3.1 which has the opposite issue where the masks
are wrong rather than the values.
* Split into two patches
---
drivers/net/ipa/data/ipa_data-v3.5.1.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

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,
--
2.38.1

2022-10-24 23:35:05

by Alex Elder

[permalink] [raw]

Subject: Re: [PATCH v2 1/2] net: ipa: fix v3.5.1 resource limit max values

On 10/24/22 4:03 PM, Caleb Connolly wrote:
> Some resource limits on IPA v3.5.1 have their max values set to
> 255, this causes a few splats in ipa_reg_encode and prevents the
> IPA from booting properly. The limits are all 6 bits wide so
> adjust the max values to 63.
>
> Fixes: 1c418c4a929c ("net: ipa: define resource group/type IPA register fields")
> Signed-off-by: Caleb Connolly <[email protected]>

Thanks Caleb, this looks good.

David et al, in case it isn't obvious, this is for net/master,
for back-port (only to 6.0.y).

Reviewed-by: Alex Elder <[email protected]>

> ---
> V1: https://lore.kernel.org/netdev/[email protected]/
> Changes since v1:
> * Apply the correct fix for v3.1 which has the opposite issue where the masks
> are wrong rather than the values.
> * Split into two patches
> ---
> drivers/net/ipa/data/ipa_data-v3.5.1.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> 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,

2022-10-24 23:39:38

by Caleb Connolly

[permalink] [raw]

Subject: [PATCH v2 2/2] net: ipa: fix v3.1 resource limit masks

The resource group limits for IPA v3.1 mistakenly used 6 bit wide mask
values, when the hardware actually uses 8. Out of range values were
silently ignored before, so the IPA worked as expected. However the
new generalised register definitions introduce stricter checking here,
they now cause some splats and result in the value 0 being written
instead. Fix the limit bitmask widths so that the correct values can be
written.

Fixes: 1c418c4a929c ("net: ipa: define resource group/type IPA register fields")
Signed-off-by: Caleb Connolly <[email protected]>
---
drivers/net/ipa/reg/ipa_reg-v3.1.c | 96 ++++++++++--------------------
1 file changed, 32 insertions(+), 64 deletions(-)

diff --git a/drivers/net/ipa/reg/ipa_reg-v3.1.c b/drivers/net/ipa/reg/ipa_reg-v3.1.c
index 116b27717e3d..0d002c3c38a2 100644
--- a/drivers/net/ipa/reg/ipa_reg-v3.1.c
+++ b/drivers/net/ipa/reg/ipa_reg-v3.1.c
@@ -127,112 +127,80 @@ static const u32 ipa_reg_counter_cfg_fmask[] = {
IPA_REG_FIELDS(COUNTER_CFG, counter_cfg, 0x000001f0);

static const u32 ipa_reg_src_rsrc_grp_01_rsrc_type_fmask[] = {
- [X_MIN_LIM] = GENMASK(5, 0),
- /* Bits 6-7 reserved */
- [X_MAX_LIM] = GENMASK(13, 8),
- /* Bits 14-15 reserved */
- [Y_MIN_LIM] = GENMASK(21, 16),
- /* Bits 22-23 reserved */
- [Y_MAX_LIM] = GENMASK(29, 24),
- /* Bits 30-31 reserved */
+ [X_MIN_LIM] = GENMASK(7, 0),
+ [X_MAX_LIM] = GENMASK(15, 8),
+ [Y_MIN_LIM] = GENMASK(23, 16),
+ [Y_MAX_LIM] = GENMASK(31, 24),
};

IPA_REG_STRIDE_FIELDS(SRC_RSRC_GRP_01_RSRC_TYPE, src_rsrc_grp_01_rsrc_type,
0x00000400, 0x0020);

static const u32 ipa_reg_src_rsrc_grp_23_rsrc_type_fmask[] = {
- [X_MIN_LIM] = GENMASK(5, 0),
- /* Bits 6-7 reserved */
- [X_MAX_LIM] = GENMASK(13, 8),
- /* Bits 14-15 reserved */
- [Y_MIN_LIM] = GENMASK(21, 16),
- /* Bits 22-23 reserved */
- [Y_MAX_LIM] = GENMASK(29, 24),
- /* Bits 30-31 reserved */
+ [X_MIN_LIM] = GENMASK(7, 0),
+ [X_MAX_LIM] = GENMASK(15, 8),
+ [Y_MIN_LIM] = GENMASK(23, 16),
+ [Y_MAX_LIM] = GENMASK(31, 24),
};

IPA_REG_STRIDE_FIELDS(SRC_RSRC_GRP_23_RSRC_TYPE, src_rsrc_grp_23_rsrc_type,
0x00000404, 0x0020);

static const u32 ipa_reg_src_rsrc_grp_45_rsrc_type_fmask[] = {
- [X_MIN_LIM] = GENMASK(5, 0),
- /* Bits 6-7 reserved */
- [X_MAX_LIM] = GENMASK(13, 8),
- /* Bits 14-15 reserved */
- [Y_MIN_LIM] = GENMASK(21, 16),
- /* Bits 22-23 reserved */
- [Y_MAX_LIM] = GENMASK(29, 24),
- /* Bits 30-31 reserved */
+ [X_MIN_LIM] = GENMASK(7, 0),
+ [X_MAX_LIM] = GENMASK(15, 8),
+ [Y_MIN_LIM] = GENMASK(23, 16),
+ [Y_MAX_LIM] = GENMASK(31, 24),
};

IPA_REG_STRIDE_FIELDS(SRC_RSRC_GRP_45_RSRC_TYPE, src_rsrc_grp_45_rsrc_type,
0x00000408, 0x0020);

static const u32 ipa_reg_src_rsrc_grp_67_rsrc_type_fmask[] = {
- [X_MIN_LIM] = GENMASK(5, 0),
- /* Bits 6-7 reserved */
- [X_MAX_LIM] = GENMASK(13, 8),
- /* Bits 14-15 reserved */
- [Y_MIN_LIM] = GENMASK(21, 16),
- /* Bits 22-23 reserved */
- [Y_MAX_LIM] = GENMASK(29, 24),
- /* Bits 30-31 reserved */
+ [X_MIN_LIM] = GENMASK(7, 0),
+ [X_MAX_LIM] = GENMASK(15, 8),
+ [Y_MIN_LIM] = GENMASK(23, 16),
+ [Y_MAX_LIM] = GENMASK(31, 24),
};

IPA_REG_STRIDE_FIELDS(SRC_RSRC_GRP_67_RSRC_TYPE, src_rsrc_grp_67_rsrc_type,
0x0000040c, 0x0020);

static const u32 ipa_reg_dst_rsrc_grp_01_rsrc_type_fmask[] = {
- [X_MIN_LIM] = GENMASK(5, 0),
- /* Bits 6-7 reserved */
- [X_MAX_LIM] = GENMASK(13, 8),
- /* Bits 14-15 reserved */
- [Y_MIN_LIM] = GENMASK(21, 16),
- /* Bits 22-23 reserved */
- [Y_MAX_LIM] = GENMASK(29, 24),
- /* Bits 30-31 reserved */
+ [X_MIN_LIM] = GENMASK(7, 0),
+ [X_MAX_LIM] = GENMASK(15, 8),
+ [Y_MIN_LIM] = GENMASK(23, 16),
+ [Y_MAX_LIM] = GENMASK(31, 24),
};

IPA_REG_STRIDE_FIELDS(DST_RSRC_GRP_01_RSRC_TYPE, dst_rsrc_grp_01_rsrc_type,
0x00000500, 0x0020);

static const u32 ipa_reg_dst_rsrc_grp_23_rsrc_type_fmask[] = {
- [X_MIN_LIM] = GENMASK(5, 0),
- /* Bits 6-7 reserved */
- [X_MAX_LIM] = GENMASK(13, 8),
- /* Bits 14-15 reserved */
- [Y_MIN_LIM] = GENMASK(21, 16),
- /* Bits 22-23 reserved */
- [Y_MAX_LIM] = GENMASK(29, 24),
- /* Bits 30-31 reserved */
+ [X_MIN_LIM] = GENMASK(7, 0),
+ [X_MAX_LIM] = GENMASK(15, 8),
+ [Y_MIN_LIM] = GENMASK(23, 16),
+ [Y_MAX_LIM] = GENMASK(31, 24),
};

IPA_REG_STRIDE_FIELDS(DST_RSRC_GRP_23_RSRC_TYPE, dst_rsrc_grp_23_rsrc_type,
0x00000504, 0x0020);

static const u32 ipa_reg_dst_rsrc_grp_45_rsrc_type_fmask[] = {
- [X_MIN_LIM] = GENMASK(5, 0),
- /* Bits 6-7 reserved */
- [X_MAX_LIM] = GENMASK(13, 8),
- /* Bits 14-15 reserved */
- [Y_MIN_LIM] = GENMASK(21, 16),
- /* Bits 22-23 reserved */
- [Y_MAX_LIM] = GENMASK(29, 24),
- /* Bits 30-31 reserved */
+ [X_MIN_LIM] = GENMASK(7, 0),
+ [X_MAX_LIM] = GENMASK(15, 8),
+ [Y_MIN_LIM] = GENMASK(23, 16),
+ [Y_MAX_LIM] = GENMASK(31, 24),
};

IPA_REG_STRIDE_FIELDS(DST_RSRC_GRP_45_RSRC_TYPE, dst_rsrc_grp_45_rsrc_type,
0x00000508, 0x0020);

static const u32 ipa_reg_dst_rsrc_grp_67_rsrc_type_fmask[] = {
- [X_MIN_LIM] = GENMASK(5, 0),
- /* Bits 6-7 reserved */
- [X_MAX_LIM] = GENMASK(13, 8),
- /* Bits 14-15 reserved */
- [Y_MIN_LIM] = GENMASK(21, 16),
- /* Bits 22-23 reserved */
- [Y_MAX_LIM] = GENMASK(29, 24),
- /* Bits 30-31 reserved */
+ [X_MIN_LIM] = GENMASK(7, 0),
+ [X_MAX_LIM] = GENMASK(15, 8),
+ [Y_MIN_LIM] = GENMASK(23, 16),
+ [Y_MAX_LIM] = GENMASK(31, 24),
};

IPA_REG_STRIDE_FIELDS(DST_RSRC_GRP_67_RSRC_TYPE, dst_rsrc_grp_67_rsrc_type,
--
2.38.1

2022-10-25 20:37:13

by Jami Kettunen

[permalink] [raw]

Subject: Re: [PATCH v2 2/2] net: ipa: fix v3.1 resource limit masks

On 25.10.2022 00:03, Caleb Connolly wrote:
> The resource group limits for IPA v3.1 mistakenly used 6 bit wide mask
> values, when the hardware actually uses 8. Out of range values were
> silently ignored before, so the IPA worked as expected. However the
> new generalised register definitions introduce stricter checking here,
> they now cause some splats and result in the value 0 being written
> instead. Fix the limit bitmask widths so that the correct values can be
> written.
>
> Fixes: 1c418c4a929c ("net: ipa: define resource group/type IPA register
> fields")
> Signed-off-by: Caleb Connolly <[email protected]>

Tested-by: Jami Kettunen <[email protected]>

> ---
> drivers/net/ipa/reg/ipa_reg-v3.1.c | 96 ++++++++++--------------------
> 1 file changed, 32 insertions(+), 64 deletions(-)
>
> diff --git a/drivers/net/ipa/reg/ipa_reg-v3.1.c
> b/drivers/net/ipa/reg/ipa_reg-v3.1.c
> index 116b27717e3d..0d002c3c38a2 100644
> --- a/drivers/net/ipa/reg/ipa_reg-v3.1.c
> +++ b/drivers/net/ipa/reg/ipa_reg-v3.1.c
> @@ -127,112 +127,80 @@ static const u32 ipa_reg_counter_cfg_fmask[] = {
> IPA_REG_FIELDS(COUNTER_CFG, counter_cfg, 0x000001f0);
>
> static const u32 ipa_reg_src_rsrc_grp_01_rsrc_type_fmask[] = {
> - [X_MIN_LIM] = GENMASK(5, 0),
> - /* Bits 6-7 reserved */
> - [X_MAX_LIM] = GENMASK(13, 8),
> - /* Bits 14-15 reserved */
> - [Y_MIN_LIM] = GENMASK(21, 16),
> - /* Bits 22-23 reserved */
> - [Y_MAX_LIM] = GENMASK(29, 24),
> - /* Bits 30-31 reserved */
> + [X_MIN_LIM] = GENMASK(7, 0),
> + [X_MAX_LIM] = GENMASK(15, 8),
> + [Y_MIN_LIM] = GENMASK(23, 16),
> + [Y_MAX_LIM] = GENMASK(31, 24),
> };
>
> IPA_REG_STRIDE_FIELDS(SRC_RSRC_GRP_01_RSRC_TYPE,
> src_rsrc_grp_01_rsrc_type,
> 0x00000400, 0x0020);
>
> static const u32 ipa_reg_src_rsrc_grp_23_rsrc_type_fmask[] = {
> - [X_MIN_LIM] = GENMASK(5, 0),
> - /* Bits 6-7 reserved */
> - [X_MAX_LIM] = GENMASK(13, 8),
> - /* Bits 14-15 reserved */
> - [Y_MIN_LIM] = GENMASK(21, 16),
> - /* Bits 22-23 reserved */
> - [Y_MAX_LIM] = GENMASK(29, 24),
> - /* Bits 30-31 reserved */
> + [X_MIN_LIM] = GENMASK(7, 0),
> + [X_MAX_LIM] = GENMASK(15, 8),
> + [Y_MIN_LIM] = GENMASK(23, 16),
> + [Y_MAX_LIM] = GENMASK(31, 24),
> };
>
> IPA_REG_STRIDE_FIELDS(SRC_RSRC_GRP_23_RSRC_TYPE,
> src_rsrc_grp_23_rsrc_type,
> 0x00000404, 0x0020);
>
> static const u32 ipa_reg_src_rsrc_grp_45_rsrc_type_fmask[] = {
> - [X_MIN_LIM] = GENMASK(5, 0),
> - /* Bits 6-7 reserved */
> - [X_MAX_LIM] = GENMASK(13, 8),
> - /* Bits 14-15 reserved */
> - [Y_MIN_LIM] = GENMASK(21, 16),
> - /* Bits 22-23 reserved */
> - [Y_MAX_LIM] = GENMASK(29, 24),
> - /* Bits 30-31 reserved */
> + [X_MIN_LIM] = GENMASK(7, 0),
> + [X_MAX_LIM] = GENMASK(15, 8),
> + [Y_MIN_LIM] = GENMASK(23, 16),
> + [Y_MAX_LIM] = GENMASK(31, 24),
> };
>
> IPA_REG_STRIDE_FIELDS(SRC_RSRC_GRP_45_RSRC_TYPE,
> src_rsrc_grp_45_rsrc_type,
> 0x00000408, 0x0020);
>
> static const u32 ipa_reg_src_rsrc_grp_67_rsrc_type_fmask[] = {
> - [X_MIN_LIM] = GENMASK(5, 0),
> - /* Bits 6-7 reserved */
> - [X_MAX_LIM] = GENMASK(13, 8),
> - /* Bits 14-15 reserved */
> - [Y_MIN_LIM] = GENMASK(21, 16),
> - /* Bits 22-23 reserved */
> - [Y_MAX_LIM] = GENMASK(29, 24),
> - /* Bits 30-31 reserved */
> + [X_MIN_LIM] = GENMASK(7, 0),
> + [X_MAX_LIM] = GENMASK(15, 8),
> + [Y_MIN_LIM] = GENMASK(23, 16),
> + [Y_MAX_LIM] = GENMASK(31, 24),
> };
>
> IPA_REG_STRIDE_FIELDS(SRC_RSRC_GRP_67_RSRC_TYPE,
> src_rsrc_grp_67_rsrc_type,
> 0x0000040c, 0x0020);
>
> static const u32 ipa_reg_dst_rsrc_grp_01_rsrc_type_fmask[] = {
> - [X_MIN_LIM] = GENMASK(5, 0),
> - /* Bits 6-7 reserved */
> - [X_MAX_LIM] = GENMASK(13, 8),
> - /* Bits 14-15 reserved */
> - [Y_MIN_LIM] = GENMASK(21, 16),
> - /* Bits 22-23 reserved */
> - [Y_MAX_LIM] = GENMASK(29, 24),
> - /* Bits 30-31 reserved */
> + [X_MIN_LIM] = GENMASK(7, 0),
> + [X_MAX_LIM] = GENMASK(15, 8),
> + [Y_MIN_LIM] = GENMASK(23, 16),
> + [Y_MAX_LIM] = GENMASK(31, 24),
> };
>
> IPA_REG_STRIDE_FIELDS(DST_RSRC_GRP_01_RSRC_TYPE,
> dst_rsrc_grp_01_rsrc_type,
> 0x00000500, 0x0020);
>
> static const u32 ipa_reg_dst_rsrc_grp_23_rsrc_type_fmask[] = {
> - [X_MIN_LIM] = GENMASK(5, 0),
> - /* Bits 6-7 reserved */
> - [X_MAX_LIM] = GENMASK(13, 8),
> - /* Bits 14-15 reserved */
> - [Y_MIN_LIM] = GENMASK(21, 16),
> - /* Bits 22-23 reserved */
> - [Y_MAX_LIM] = GENMASK(29, 24),
> - /* Bits 30-31 reserved */
> + [X_MIN_LIM] = GENMASK(7, 0),
> + [X_MAX_LIM] = GENMASK(15, 8),
> + [Y_MIN_LIM] = GENMASK(23, 16),
> + [Y_MAX_LIM] = GENMASK(31, 24),
> };
>
> IPA_REG_STRIDE_FIELDS(DST_RSRC_GRP_23_RSRC_TYPE,
> dst_rsrc_grp_23_rsrc_type,
> 0x00000504, 0x0020);
>
> static const u32 ipa_reg_dst_rsrc_grp_45_rsrc_type_fmask[] = {
> - [X_MIN_LIM] = GENMASK(5, 0),
> - /* Bits 6-7 reserved */
> - [X_MAX_LIM] = GENMASK(13, 8),
> - /* Bits 14-15 reserved */
> - [Y_MIN_LIM] = GENMASK(21, 16),
> - /* Bits 22-23 reserved */
> - [Y_MAX_LIM] = GENMASK(29, 24),
> - /* Bits 30-31 reserved */
> + [X_MIN_LIM] = GENMASK(7, 0),
> + [X_MAX_LIM] = GENMASK(15, 8),
> + [Y_MIN_LIM] = GENMASK(23, 16),
> + [Y_MAX_LIM] = GENMASK(31, 24),
> };
>
> IPA_REG_STRIDE_FIELDS(DST_RSRC_GRP_45_RSRC_TYPE,
> dst_rsrc_grp_45_rsrc_type,
> 0x00000508, 0x0020);
>
> static const u32 ipa_reg_dst_rsrc_grp_67_rsrc_type_fmask[] = {
> - [X_MIN_LIM] = GENMASK(5, 0),
> - /* Bits 6-7 reserved */
> - [X_MAX_LIM] = GENMASK(13, 8),
> - /* Bits 14-15 reserved */
> - [Y_MIN_LIM] = GENMASK(21, 16),
> - /* Bits 22-23 reserved */
> - [Y_MAX_LIM] = GENMASK(29, 24),
> - /* Bits 30-31 reserved */
> + [X_MIN_LIM] = GENMASK(7, 0),
> + [X_MAX_LIM] = GENMASK(15, 8),
> + [Y_MIN_LIM] = GENMASK(23, 16),
> + [Y_MAX_LIM] = GENMASK(31, 24),
> };
>
> IPA_REG_STRIDE_FIELDS(DST_RSRC_GRP_67_RSRC_TYPE,
> dst_rsrc_grp_67_rsrc_type,

2022-10-26 03:05:33

by patchwork-bot+netdevbpf

[permalink] [raw]

Subject: Re: [PATCH v2 1/2] net: ipa: fix v3.5.1 resource limit max values

Hello:

This series was applied to netdev/net.git (master)
by Jakub Kicinski <[email protected]>:

On Mon, 24 Oct 2022 22:03:31 +0100 you wrote:
> Some resource limits on IPA v3.5.1 have their max values set to
> 255, this causes a few splats in ipa_reg_encode and prevents the
> IPA from booting properly. The limits are all 6 bits wide so
> adjust the max values to 63.
>
> Fixes: 1c418c4a929c ("net: ipa: define resource group/type IPA register fields")
> Signed-off-by: Caleb Connolly <[email protected]>
>
> [...]

Here is the summary with links:
- [v2,1/2] net: ipa: fix v3.5.1 resource limit max values
https://git.kernel.org/netdev/net/c/f23a566bbfc0
- [v2,2/2] net: ipa: fix v3.1 resource limit masks
https://git.kernel.org/netdev/net/c/05a31b94af32

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