[PATCH v2 1/7] baseband/acc: acc100 free harq layout pointer

[PATCH v2 1/7] baseband/acc: acc100 free harq layout pointer

[Hernan Vargas]

Explicitly call rte_free for harq_layout pointer in dev_close function
to prevent memory leak.

Fixes: ba2262fe16c ("baseband/acc100: fix close cleanup")
Cc: stable@dpdk.org

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
---
 drivers/baseband/acc/rte_acc100_pmd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/baseband/acc/rte_acc100_pmd.c b/drivers/baseband/acc/rte_acc100_pmd.c
index ba8247d47e..0177a9e0ea 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -622,6 +622,7 @@ acc100_dev_close(struct rte_bbdev *dev)
 		rte_free(d->tail_ptrs);
 		rte_free(d->info_ring);
 		rte_free(d->sw_rings_base);
+		rte_free(d->harq_layout);
 		d->sw_rings_base = NULL;
 		d->tail_ptrs = NULL;
 		d->info_ring = NULL;
-- 
2.37.1

[PATCH v2 2/7] baseband/acc: acc100 fix iteration counter in TB

[Hernan Vargas]

Use ldpc or turbo iteration counter based on the operation type.

Fixes: 5ad5060f8f7 ("baseband/acc100: add LDPC processing functions")
Cc: stable@dpdk.org

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
---
 drivers/baseband/acc/rte_acc100_pmd.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/baseband/acc/rte_acc100_pmd.c b/drivers/baseband/acc/rte_acc100_pmd.c
index 0177a9e0ea..d77d3b77b5 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -4035,8 +4035,12 @@ dequeue_dec_one_op_tb(struct acc_queue *q, struct rte_bbdev_dec_op **ref_op,
 		/* CRC invalid if error exists */
 		if (!op->status)
 			op->status |= rsp.crc_status << RTE_BBDEV_CRC_ERROR;
-		op->turbo_dec.iter_count = RTE_MAX((uint8_t) rsp.iter_cnt,
-				op->turbo_dec.iter_count);
+		if (q->op_type == RTE_BBDEV_OP_LDPC_DEC)
+			op->ldpc_dec.iter_count = RTE_MAX((uint8_t) rsp.iter_cnt,
+					op->ldpc_dec.iter_count);
+		else
+			op->turbo_dec.iter_count = RTE_MAX((uint8_t) rsp.iter_cnt,
+					op->turbo_dec.iter_count);
 
 		/* Check if this is the last desc in batch (Atomic Queue) */
 		if (desc->req.last_desc_in_batch) {
-- 
2.37.1

Re: [PATCH v2 1/7] baseband/acc: acc100 free harq layout pointer

[Maxime Coquelin]

On 1/6/23 06:44, Hernan Vargas wrote:
> Explicitly call rte_free for harq_layout pointer in dev_close function
> to prevent memory leak.
> 
> Fixes: ba2262fe16c ("baseband/acc100: fix close cleanup")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> ---
>   drivers/baseband/acc/rte_acc100_pmd.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/baseband/acc/rte_acc100_pmd.c b/drivers/baseband/acc/rte_acc100_pmd.c
> index ba8247d47e..0177a9e0ea 100644
> --- a/drivers/baseband/acc/rte_acc100_pmd.c
> +++ b/drivers/baseband/acc/rte_acc100_pmd.c
> @@ -622,6 +622,7 @@ acc100_dev_close(struct rte_bbdev *dev)
>   		rte_free(d->tail_ptrs);
>   		rte_free(d->info_ring);
>   		rte_free(d->sw_rings_base);
> +		rte_free(d->harq_layout);
>   		d->sw_rings_base = NULL;
>   		d->tail_ptrs = NULL;
>   		d->info_ring = NULL;

Please set d->harq_layout to NULL for consistency and to prevent double
free.

Maxime

Re: [PATCH v2 2/7] baseband/acc: acc100 fix iteration counter in TB

[Maxime Coquelin]

On 1/6/23 06:44, Hernan Vargas wrote:
> Use ldpc or turbo iteration counter based on the operation type.
> 
> Fixes: 5ad5060f8f7 ("baseband/acc100: add LDPC processing functions")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> ---
>   drivers/baseband/acc/rte_acc100_pmd.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/baseband/acc/rte_acc100_pmd.c b/drivers/baseband/acc/rte_acc100_pmd.c
> index 0177a9e0ea..d77d3b77b5 100644
> --- a/drivers/baseband/acc/rte_acc100_pmd.c
> +++ b/drivers/baseband/acc/rte_acc100_pmd.c
> @@ -4035,8 +4035,12 @@ dequeue_dec_one_op_tb(struct acc_queue *q, struct rte_bbdev_dec_op **ref_op,
>   		/* CRC invalid if error exists */
>   		if (!op->status)
>   			op->status |= rsp.crc_status << RTE_BBDEV_CRC_ERROR;
> -		op->turbo_dec.iter_count = RTE_MAX((uint8_t) rsp.iter_cnt,
> -				op->turbo_dec.iter_count);
> +		if (q->op_type == RTE_BBDEV_OP_LDPC_DEC)
> +			op->ldpc_dec.iter_count = RTE_MAX((uint8_t) rsp.iter_cnt,
> +					op->ldpc_dec.iter_count);
> +		else
> +			op->turbo_dec.iter_count = RTE_MAX((uint8_t) rsp.iter_cnt,
> +					op->turbo_dec.iter_count);
>   
>   		/* Check if this is the last desc in batch (Atomic Queue) */
>   		if (desc->req.last_desc_in_batch) {

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime

Re: [PATCH v2 4/7] baseband/acc: acc100 fix queue mapping to 64 bits

[Maxime Coquelin]

On 1/6/23 10:12, Maxime Coquelin wrote:
> 
> 
> On 1/6/23 06:44, Hernan Vargas wrote:
>> Fix potential overflow for Q mapping extension to 64 bits.
>>
>> Fixes: 32e8b7ea35d ("baseband/acc100: refactor to segregate common code")
>>
>> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
>> ---
>>   drivers/baseband/acc/rte_acc100_pmd.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/baseband/acc/rte_acc100_pmd.c 
>> b/drivers/baseband/acc/rte_acc100_pmd.c
>> index 2f65f290b5..1ca95f4440 100644
>> --- a/drivers/baseband/acc/rte_acc100_pmd.c
>> +++ b/drivers/baseband/acc/rte_acc100_pmd.c
>> @@ -664,7 +664,7 @@ acc100_find_free_queue_idx(struct rte_bbdev *dev,
>>       for (aq_idx = 0; aq_idx < qtop->num_aqs_per_groups; aq_idx++) {
>>           if (((d->q_assigned_bit_map[group_idx] >> aq_idx) & 0x1) == 
>> 0) {
>>               /* Mark the Queue as assigned */
>> -            d->q_assigned_bit_map[group_idx] |= (1 << aq_idx);
>> +            d->q_assigned_bit_map[group_idx] |= (1ULL << aq_idx);
>>               /* Report the AQ Index */
>>               return (group_idx << ACC100_GRP_ID_SHIFT) + aq_idx;
>>           }
> 
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Also cc'ing stable since it is in v22.11 at least.

> Thanks,
> Maxime

RE: [PATCH v2 4/7] baseband/acc: acc100 fix queue mapping to 64 bits

[Vargas, Hernan]

Hi Maxime,

aq_idx is uint16_t in the stable branch so I don't think that this fix is needed there.

Thanks,
Hernan

-----Original Message-----
From: Maxime Coquelin <maxime.coquelin@redhat.com> 
Sent: Friday, January 6, 2023 3:13 AM
To: Vargas, Hernan <hernan.vargas@intel.com>; dev@dpdk.org; gakhil@marvell.com; trix@redhat.com
Cc: Chautru, Nicolas <nicolas.chautru@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; stable@dpdk.org
Subject: Re: [PATCH v2 4/7] baseband/acc: acc100 fix queue mapping to 64 bits



On 1/6/23 10:12, Maxime Coquelin wrote:
> 
> 
> On 1/6/23 06:44, Hernan Vargas wrote:
>> Fix potential overflow for Q mapping extension to 64 bits.
>>
>> Fixes: 32e8b7ea35d ("baseband/acc100: refactor to segregate common code")
>>
>> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
>> ---
>>   drivers/baseband/acc/rte_acc100_pmd.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/baseband/acc/rte_acc100_pmd.c 
>> b/drivers/baseband/acc/rte_acc100_pmd.c
>> index 2f65f290b5..1ca95f4440 100644
>> --- a/drivers/baseband/acc/rte_acc100_pmd.c
>> +++ b/drivers/baseband/acc/rte_acc100_pmd.c
>> @@ -664,7 +664,7 @@ acc100_find_free_queue_idx(struct rte_bbdev *dev,
>>       for (aq_idx = 0; aq_idx < qtop->num_aqs_per_groups; aq_idx++) {
>>           if (((d->q_assigned_bit_map[group_idx] >> aq_idx) & 0x1) == 
>> 0) {
>>               /* Mark the Queue as assigned */
>> -            d->q_assigned_bit_map[group_idx] |= (1 << aq_idx);
>> +            d->q_assigned_bit_map[group_idx] |= (1ULL << aq_idx);
>>               /* Report the AQ Index */
>>               return (group_idx << ACC100_GRP_ID_SHIFT) + aq_idx;
>>           }
> 
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Also cc'ing stable since it is in v22.11 at least.

> Thanks,
> Maxime

RE: [PATCH v2 1/7] baseband/acc: acc100 free harq layout pointer

[Vargas, Hernan]

Hi Maxime,

d->harq_layout = NULL is already in de acc100_dev_close function.

Thanks,
Hernan

-----Original Message-----
From: Maxime Coquelin <maxime.coquelin@redhat.com> 
Sent: Friday, January 6, 2023 2:53 AM
To: Vargas, Hernan <hernan.vargas@intel.com>; dev@dpdk.org; gakhil@marvell.com; trix@redhat.com
Cc: Chautru, Nicolas <nicolas.chautru@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; stable@dpdk.org
Subject: Re: [PATCH v2 1/7] baseband/acc: acc100 free harq layout pointer



On 1/6/23 06:44, Hernan Vargas wrote:
> Explicitly call rte_free for harq_layout pointer in dev_close function 
> to prevent memory leak.
> 
> Fixes: ba2262fe16c ("baseband/acc100: fix close cleanup")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> ---
>   drivers/baseband/acc/rte_acc100_pmd.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/baseband/acc/rte_acc100_pmd.c 
> b/drivers/baseband/acc/rte_acc100_pmd.c
> index ba8247d47e..0177a9e0ea 100644
> --- a/drivers/baseband/acc/rte_acc100_pmd.c
> +++ b/drivers/baseband/acc/rte_acc100_pmd.c
> @@ -622,6 +622,7 @@ acc100_dev_close(struct rte_bbdev *dev)
>   		rte_free(d->tail_ptrs);
>   		rte_free(d->info_ring);
>   		rte_free(d->sw_rings_base);
> +		rte_free(d->harq_layout);
>   		d->sw_rings_base = NULL;
>   		d->tail_ptrs = NULL;
>   		d->info_ring = NULL;

Please set d->harq_layout to NULL for consistency and to prevent double free.

Maxime