Re: [RFC PATCH 02/17] net: ipa: revert to IPA_TABLE_ENTRY_SIZE for 32-bit IPA support

From: Sireesh Kodali
Date: Mon Oct 18 2021 - 12:22:22 EST


On Thu Oct 14, 2021 at 3:58 AM IST, Alex Elder wrote:
> On 9/19/21 10:07 PM, Sireesh Kodali wrote:
> > From: Vladimir Lypak <vladimir.lypak@xxxxxxxxx>
> >
> > IPA v2.x is 32 bit. Having an IPA_TABLE_ENTRY size makes it easier to
> > deal with supporting both 32 bit and 64 bit IPA versions
>
> This looks reasonable. At this point filter/route tables aren't
> really used, so this is a simple fix. You use IPA_IS_64BIT()
> here, but it isn't defined until patch 7, which I expect is a
> build problem.

Oof, I probably messed this up while re-ordering the commits... will fix

Regards,
Sireesh
>
> -Alex
>
> > Signed-off-by: Vladimir Lypak <vladimir.lypak@xxxxxxxxx>
> > Signed-off-by: Sireesh Kodali <sireeshkodali1@xxxxxxxxx>
> > ---
> > drivers/net/ipa/ipa_qmi.c | 10 ++++++----
> > drivers/net/ipa/ipa_table.c | 29 +++++++++++++----------------
> > drivers/net/ipa/ipa_table.h | 4 ++++
> > 3 files changed, 23 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/net/ipa/ipa_qmi.c b/drivers/net/ipa/ipa_qmi.c
> > index 90f3aec55b36..7e2fe701cc4d 100644
> > --- a/drivers/net/ipa/ipa_qmi.c
> > +++ b/drivers/net/ipa/ipa_qmi.c
> > @@ -308,12 +308,12 @@ init_modem_driver_req(struct ipa_qmi *ipa_qmi)
> > mem = ipa_mem_find(ipa, IPA_MEM_V4_ROUTE);
> > req.v4_route_tbl_info_valid = 1;
> > req.v4_route_tbl_info.start = ipa->mem_offset + mem->offset;
> > - req.v4_route_tbl_info.count = mem->size / sizeof(__le64);
> > + req.v4_route_tbl_info.count = mem->size / IPA_TABLE_ENTRY_SIZE(ipa->version);
> >
> > mem = ipa_mem_find(ipa, IPA_MEM_V6_ROUTE);
> > req.v6_route_tbl_info_valid = 1;
> > req.v6_route_tbl_info.start = ipa->mem_offset + mem->offset;
> > - req.v6_route_tbl_info.count = mem->size / sizeof(__le64);
> > + req.v6_route_tbl_info.count = mem->size / IPA_TABLE_ENTRY_SIZE(ipa->version);
> >
> > mem = ipa_mem_find(ipa, IPA_MEM_V4_FILTER);
> > req.v4_filter_tbl_start_valid = 1;
> > @@ -352,7 +352,8 @@ init_modem_driver_req(struct ipa_qmi *ipa_qmi)
> > req.v4_hash_route_tbl_info_valid = 1;
> > req.v4_hash_route_tbl_info.start =
> > ipa->mem_offset + mem->offset;
> > - req.v4_hash_route_tbl_info.count = mem->size / sizeof(__le64);
> > + req.v4_hash_route_tbl_info.count =
> > + mem->size / IPA_TABLE_ENTRY_SIZE(ipa->version);
> > }
> >
> > mem = ipa_mem_find(ipa, IPA_MEM_V6_ROUTE_HASHED);
> > @@ -360,7 +361,8 @@ init_modem_driver_req(struct ipa_qmi *ipa_qmi)
> > req.v6_hash_route_tbl_info_valid = 1;
> > req.v6_hash_route_tbl_info.start =
> > ipa->mem_offset + mem->offset;
> > - req.v6_hash_route_tbl_info.count = mem->size / sizeof(__le64);
> > + req.v6_hash_route_tbl_info.count =
> > + mem->size / IPA_TABLE_ENTRY_SIZE(ipa->version);
> > }
> >
> > mem = ipa_mem_find(ipa, IPA_MEM_V4_FILTER_HASHED);
> > diff --git a/drivers/net/ipa/ipa_table.c b/drivers/net/ipa/ipa_table.c
> > index 1da334f54944..96c467c80a2e 100644
> > --- a/drivers/net/ipa/ipa_table.c
> > +++ b/drivers/net/ipa/ipa_table.c
> > @@ -118,7 +118,8 @@
> > * 32-bit all-zero rule list terminator. The "zero rule" is simply an
> > * all-zero rule followed by the list terminator.
> > */
> > -#define IPA_ZERO_RULE_SIZE (2 * sizeof(__le32))
> > +#define IPA_ZERO_RULE_SIZE(version) \
> > + (IPA_IS_64BIT(version) ? 2 * sizeof(__le32) : sizeof(__le32))
> >
> > /* Check things that can be validated at build time. */
> > static void ipa_table_validate_build(void)
> > @@ -132,12 +133,6 @@ static void ipa_table_validate_build(void)
> > */
> > BUILD_BUG_ON(sizeof(dma_addr_t) > sizeof(__le64));
> >
> > - /* A "zero rule" is used to represent no filtering or no routing.
> > - * It is a 64-bit block of zeroed memory. Code in ipa_table_init()
> > - * assumes that it can be written using a pointer to __le64.
> > - */
> > - BUILD_BUG_ON(IPA_ZERO_RULE_SIZE != sizeof(__le64));
> > -
> > /* Impose a practical limit on the number of routes */
> > BUILD_BUG_ON(IPA_ROUTE_COUNT_MAX > 32);
> > /* The modem must be allotted at least one route table entry */
> > @@ -236,7 +231,7 @@ static dma_addr_t ipa_table_addr(struct ipa *ipa, bool filter_mask, u16 count)
> > /* Skip over the zero rule and possibly the filter mask */
> > skip = filter_mask ? 1 : 2;
> >
> > - return ipa->table_addr + skip * sizeof(*ipa->table_virt);
> > + return ipa->table_addr + skip * IPA_TABLE_ENTRY_SIZE(ipa->version);
> > }
> >
> > static void ipa_table_reset_add(struct gsi_trans *trans, bool filter,
> > @@ -255,8 +250,8 @@ static void ipa_table_reset_add(struct gsi_trans *trans, bool filter,
> > if (filter)
> > first++; /* skip over bitmap */
> >
> > - offset = mem->offset + first * sizeof(__le64);
> > - size = count * sizeof(__le64);
> > + offset = mem->offset + first * IPA_TABLE_ENTRY_SIZE(ipa->version);
> > + size = count * IPA_TABLE_ENTRY_SIZE(ipa->version);
> > addr = ipa_table_addr(ipa, false, count);
> >
> > ipa_cmd_dma_shared_mem_add(trans, offset, size, addr, true);
> > @@ -434,11 +429,11 @@ static void ipa_table_init_add(struct gsi_trans *trans, bool filter,
> > count = 1 + hweight32(ipa->filter_map);
> > hash_count = hash_mem->size ? count : 0;
> > } else {
> > - count = mem->size / sizeof(__le64);
> > - hash_count = hash_mem->size / sizeof(__le64);
> > + count = mem->size / IPA_TABLE_ENTRY_SIZE(ipa->version);
> > + hash_count = hash_mem->size / IPA_TABLE_ENTRY_SIZE(ipa->version);
> > }
> > - size = count * sizeof(__le64);
> > - hash_size = hash_count * sizeof(__le64);
> > + size = count * IPA_TABLE_ENTRY_SIZE(ipa->version);
> > + hash_size = hash_count * IPA_TABLE_ENTRY_SIZE(ipa->version);
> >
> > addr = ipa_table_addr(ipa, filter, count);
> > hash_addr = ipa_table_addr(ipa, filter, hash_count);
> > @@ -621,7 +616,8 @@ int ipa_table_init(struct ipa *ipa)
> > * by dma_alloc_coherent() is guaranteed to be a power-of-2 number
> > * of pages, which satisfies the rule alignment requirement.
> > */
> > - size = IPA_ZERO_RULE_SIZE + (1 + count) * sizeof(__le64);
> > + size = IPA_ZERO_RULE_SIZE(ipa->version) +
> > + (1 + count) * IPA_TABLE_ENTRY_SIZE(ipa->version);
> > virt = dma_alloc_coherent(dev, size, &addr, GFP_KERNEL);
> > if (!virt)
> > return -ENOMEM;
> > @@ -653,7 +649,8 @@ void ipa_table_exit(struct ipa *ipa)
> > struct device *dev = &ipa->pdev->dev;
> > size_t size;
> >
> > - size = IPA_ZERO_RULE_SIZE + (1 + count) * sizeof(__le64);
> > + size = IPA_ZERO_RULE_SIZE(ipa->version) +
> > + (1 + count) * IPA_TABLE_ENTRY_SIZE(ipa->version);
> >
> > dma_free_coherent(dev, size, ipa->table_virt, ipa->table_addr);
> > ipa->table_addr = 0;
> > diff --git a/drivers/net/ipa/ipa_table.h b/drivers/net/ipa/ipa_table.h
> > index b6a9a0d79d68..78a168ce6558 100644
> > --- a/drivers/net/ipa/ipa_table.h
> > +++ b/drivers/net/ipa/ipa_table.h
> > @@ -10,6 +10,10 @@
> >
> > struct ipa;
> >
> > +/* The size of a filter or route table entry */
> > +#define IPA_TABLE_ENTRY_SIZE(version) \
> > + (IPA_IS_64BIT(version) ? sizeof(__le64) : sizeof(__le32))
> > +
> > /* The maximum number of filter table entries (IPv4, IPv6; hashed or not) */
> > #define IPA_FILTER_COUNT_MAX 14
> >
> >