Re: [PATCH v1 3/6] baseband/acc: fix to acc200 access corner case

[Maxime Coquelin <maxime.coquelin@redhat.com>]

On 11/2/22 00:04, Nicolas Chautru wrote:
> To enforce safe access to the ACC200 device, the PMD requires
> to explicitly check that the device is in configured and
> enabled state prior to accessing queue resources.
> This is done by checking the Qmgr ingress queue status.
> 
> Fixes: 40e3adbdd3 ("baseband/acc200: add queue configuration")
> 
> Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
> ---
>   drivers/baseband/acc/rte_acc200_pmd.c | 27 +++++++++++++++++++++++++++
>   1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/baseband/acc/rte_acc200_pmd.c b/drivers/baseband/acc/rte_acc200_pmd.c
> index 76a5986448..10511d8b56 100644
> --- a/drivers/baseband/acc/rte_acc200_pmd.c
> +++ b/drivers/baseband/acc/rte_acc200_pmd.c
> @@ -172,6 +172,21 @@ updateQtop(uint8_t acc, uint8_t qg, struct rte_acc_conf *acc_conf, struct acc_de
>   	}
>   }
>   
> +/* Check device Qmgr is enabled for protection */
> +static inline bool
> +acc200_check_device_enable(struct rte_bbdev *dev)
> +{
> +	uint32_t reg_aq, qg;
> +	struct acc_device *d = dev->data->dev_private;
> +
> +	for (qg = 0; qg < ACC200_NUM_QGRPS; qg++) {
> +		reg_aq = acc_reg_read(d, queue_offset(d->pf_device, 0, qg, 0));
> +		if (reg_aq & ACC_QUEUE_ENABLE)
> +			return true;
> +	}
> +	return false;
> +}
> +
>   /* Fetch configuration enabled for the PF/VF using MMIO Read (slow). */
>   static inline void
>   fetch_acc200_config(struct rte_bbdev *dev)
> @@ -190,6 +205,12 @@ fetch_acc200_config(struct rte_bbdev *dev)
>   	if (d->configured)
>   		return;
>   
> +	if (!acc200_check_device_enable(dev)) {
> +		rte_bbdev_log(NOTICE, "%s has no queue enabled and can't be used.",
> +				dev->data->name);
> +		return;
> +	}
> +
>   	/* Choose correct registry addresses for the device type. */
>   	if (d->pf_device)
>   		reg_addr = &pf_reg_addr;
> @@ -454,6 +475,12 @@ acc200_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id)
>   		return -ENODEV;
>   	}
>   
> +	if (!acc200_check_device_enable(dev)) {
> +		rte_bbdev_log(NOTICE, "%s has no queue enabled and can't be used.",
> +				dev->data->name);
> +		return -ENODEV;
> +	}
> +
>   	alloc_sw_rings_min_mem(dev, d, num_queues, socket_id);
>   
>   	/* If minimal memory space approach failed, then allocate

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

Thanks,
Maxime