[v5,2/7] bbdev: add device status info
Checks
Commit Message
Added device status information, so that the PMD can
expose information related to the underlying accelerator device status.
Minor order change in structure to fit into padding hole.
Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
---
drivers/baseband/acc100/rte_acc100_pmd.c | 1 +
drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c | 1 +
drivers/baseband/fpga_lte_fec/fpga_lte_fec.c | 1 +
drivers/baseband/la12xx/bbdev_la12xx.c | 1 +
drivers/baseband/null/bbdev_null.c | 1 +
drivers/baseband/turbo_sw/bbdev_turbo_software.c | 1 +
lib/bbdev/rte_bbdev.c | 22 ++++++++++++++
lib/bbdev/rte_bbdev.h | 35 ++++++++++++++++++++--
lib/bbdev/version.map | 6 ++++
9 files changed, 67 insertions(+), 2 deletions(-)
Comments
On 7/7/22 01:28, Nicolas Chautru wrote:
> Added device status information, so that the PMD can
> expose information related to the underlying accelerator device status.
> Minor order change in structure to fit into padding hole.
>
> Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
> ---
> drivers/baseband/acc100/rte_acc100_pmd.c | 1 +
> drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c | 1 +
> drivers/baseband/fpga_lte_fec/fpga_lte_fec.c | 1 +
> drivers/baseband/la12xx/bbdev_la12xx.c | 1 +
> drivers/baseband/null/bbdev_null.c | 1 +
> drivers/baseband/turbo_sw/bbdev_turbo_software.c | 1 +
> lib/bbdev/rte_bbdev.c | 22 ++++++++++++++
> lib/bbdev/rte_bbdev.h | 35 ++++++++++++++++++++--
> lib/bbdev/version.map | 6 ++++
> 9 files changed, 67 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/baseband/acc100/rte_acc100_pmd.c b/drivers/baseband/acc100/rte_acc100_pmd.c
> index de7e4bc..17ba798 100644
> --- a/drivers/baseband/acc100/rte_acc100_pmd.c
> +++ b/drivers/baseband/acc100/rte_acc100_pmd.c
> @@ -1060,6 +1060,7 @@
>
> /* Read and save the populated config from ACC100 registers */
> fetch_acc100_config(dev);
> + dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
>
> /* This isn't ideal because it reports the maximum number of queues but
> * does not provide info on how many can be uplink/downlink or different
> diff --git a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> index 82ae6ba..57b12af 100644
> --- a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> +++ b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> @@ -369,6 +369,7 @@
> dev_info->capabilities = bbdev_capabilities;
> dev_info->cpu_flag_reqs = NULL;
> dev_info->data_endianness = RTE_LITTLE_ENDIAN;
> + dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
>
> /* Calculates number of queues assigned to device */
> dev_info->max_num_queues = 0;
> diff --git a/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c b/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
> index 21d3529..2a330c4 100644
> --- a/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
> +++ b/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
> @@ -645,6 +645,7 @@ struct __rte_cache_aligned fpga_queue {
> dev_info->capabilities = bbdev_capabilities;
> dev_info->cpu_flag_reqs = NULL;
> dev_info->data_endianness = RTE_LITTLE_ENDIAN;
> + dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
>
> /* Calculates number of queues assigned to device */
> dev_info->max_num_queues = 0;
> diff --git a/drivers/baseband/la12xx/bbdev_la12xx.c b/drivers/baseband/la12xx/bbdev_la12xx.c
> index 4d1bd16..c1f88c6 100644
> --- a/drivers/baseband/la12xx/bbdev_la12xx.c
> +++ b/drivers/baseband/la12xx/bbdev_la12xx.c
> @@ -100,6 +100,7 @@ struct bbdev_la12xx_params {
> dev_info->capabilities = bbdev_capabilities;
> dev_info->cpu_flag_reqs = NULL;
> dev_info->min_alignment = 64;
> + dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
>
> rte_bbdev_log_debug("got device info from %u", dev->data->dev_id);
> }
> diff --git a/drivers/baseband/null/bbdev_null.c b/drivers/baseband/null/bbdev_null.c
> index 248e129..94a1976 100644
> --- a/drivers/baseband/null/bbdev_null.c
> +++ b/drivers/baseband/null/bbdev_null.c
> @@ -82,6 +82,7 @@ struct bbdev_queue {
> * here for code completeness.
> */
> dev_info->data_endianness = RTE_LITTLE_ENDIAN;
> + dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
>
> rte_bbdev_log_debug("got device info from %u", dev->data->dev_id);
> }
> diff --git a/drivers/baseband/turbo_sw/bbdev_turbo_software.c b/drivers/baseband/turbo_sw/bbdev_turbo_software.c
> index af7bc41..dbc5524 100644
> --- a/drivers/baseband/turbo_sw/bbdev_turbo_software.c
> +++ b/drivers/baseband/turbo_sw/bbdev_turbo_software.c
> @@ -254,6 +254,7 @@ struct turbo_sw_queue {
> dev_info->min_alignment = 64;
> dev_info->harq_buffer_size = 0;
> dev_info->data_endianness = RTE_LITTLE_ENDIAN;
> + dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
>
> rte_bbdev_log_debug("got device info from %u\n", dev->data->dev_id);
> }
> diff --git a/lib/bbdev/rte_bbdev.c b/lib/bbdev/rte_bbdev.c
> index 4da8047..38630a2 100644
> --- a/lib/bbdev/rte_bbdev.c
> +++ b/lib/bbdev/rte_bbdev.c
> @@ -1133,3 +1133,25 @@ struct rte_mempool *
> rte_bbdev_log(ERR, "Invalid operation type");
> return NULL;
> }
> +
> +const char *
> +rte_bbdev_device_status_str(enum rte_bbdev_device_status status)
> +{
> + static const char * const dev_sta_string[] = {
> + "RTE_BBDEV_DEV_NOSTATUS",
> + "RTE_BBDEV_DEV_NOT_SUPPORTED",
> + "RTE_BBDEV_DEV_RESET",
> + "RTE_BBDEV_DEV_CONFIGURED",
> + "RTE_BBDEV_DEV_ACTIVE",
> + "RTE_BBDEV_DEV_FATAL_ERR",
> + "RTE_BBDEV_DEV_RESTART_REQ",
> + "RTE_BBDEV_DEV_RECONFIG_REQ",
> + "RTE_BBDEV_DEV_CORRECT_ERR",
> + };
> +
> + if (status < sizeof(dev_sta_string) / sizeof(char *))
> + return dev_sta_string[status];
> +
> + rte_bbdev_log(ERR, "Invalid device status");
> + return NULL;
> +}
> diff --git a/lib/bbdev/rte_bbdev.h b/lib/bbdev/rte_bbdev.h
> index b88c881..9b1ffa4 100644
> --- a/lib/bbdev/rte_bbdev.h
> +++ b/lib/bbdev/rte_bbdev.h
> @@ -223,6 +223,21 @@ struct rte_bbdev_queue_conf {
> int
> rte_bbdev_queue_stop(uint16_t dev_id, uint16_t queue_id);
>
> +/**
> + * Flags indicate the status of the device
> + */
> +enum rte_bbdev_device_status {
> + RTE_BBDEV_DEV_NOSTATUS, /**< Nothing being reported */
> + RTE_BBDEV_DEV_NOT_SUPPORTED, /**< Device status is not supported on the PMD */
> + RTE_BBDEV_DEV_RESET, /**< Device in reset and un-configured state */
> + RTE_BBDEV_DEV_CONFIGURED, /**< Device is configured and ready to use */
> + RTE_BBDEV_DEV_ACTIVE, /**< Device is configured and VF is being used */
> + RTE_BBDEV_DEV_FATAL_ERR, /**< Device has hit a fatal uncorrectable error */
> + RTE_BBDEV_DEV_RESTART_REQ, /**< Device requires application to restart */
> + RTE_BBDEV_DEV_RECONFIG_REQ, /**< Device requires application to reconfigure queues */
> + RTE_BBDEV_DEV_CORRECT_ERR, /**< Warning of a correctable error event happened */
> +};
I don't have a strong opinion on this, but I think NOT_SUPPORTED should
be a special value. If you want to keep 0 value for NOSTATUS, maybe you
could do:
enum rte_bbdev_device_status {
RTE_BBDEV_DEV_NOT_SUPPORTED = -1, /**< Device status is not supported
on the PMD */
RTE_BBDEV_DEV_NOSTATUS = 0, /**< Nothing being reported */
RTE_BBDEV_DEV_RESET, /**< Device in reset and un-configured
state */
...
> +
> /** Device statistics. */
> struct rte_bbdev_stats {
> uint64_t enqueued_count; /**< Count of all operations enqueued */
> @@ -285,12 +300,14 @@ struct rte_bbdev_driver_info {
> /** Set if device supports per-queue interrupts */
> bool queue_intr_supported;
> /** Minimum alignment of buffers, in bytes */
> - uint16_t min_alignment;
> - /** HARQ memory available in kB */
> + /** Device Status */
> + enum rte_bbdev_device_status device_status;
> uint32_t harq_buffer_size;
> /** Byte endianness (RTE_BIG_ENDIAN/RTE_LITTLE_ENDIAN) supported
> * for input/output data
> */
> + uint16_t min_alignment;
> + /** HARQ memory available in kB */
> uint8_t data_endianness;
> /** Default queue configuration used if none is supplied */
> struct rte_bbdev_queue_conf default_queue_conf;
> @@ -827,6 +844,20 @@ typedef void (*rte_bbdev_cb_fn)(uint16_t dev_id,
> rte_bbdev_queue_intr_ctl(uint16_t dev_id, uint16_t queue_id, int epfd, int op,
> void *data);
>
> +/**
> + * Converts device status from enum to string
> + *
> + * @param status
> + * Device status as enum
> + *
> + * @returns
> + * Operation type as string or NULL if op_type is invalid
> + *
> + */
> +__rte_experimental
> +const char*
> +rte_bbdev_device_status_str(enum rte_bbdev_device_status status);
> +
> #ifdef __cplusplus
> }
> #endif
> diff --git a/lib/bbdev/version.map b/lib/bbdev/version.map
> index cce3f3c..9ac3643 100644
> --- a/lib/bbdev/version.map
> +++ b/lib/bbdev/version.map
> @@ -39,3 +39,9 @@ DPDK_22 {
>
> local: *;
> };
> +
> +EXPERIMENTAL {
> + global:
> +
We now add the version the new API was introduced in as a comment:
# added in 22.11
> + rte_bbdev_device_status_str;
> +};
Thanks Maxime,
> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Thursday, August 25, 2022 7:19 AM
> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org;
> thomas@monjalon.net; gakhil@marvell.com; hemant.agrawal@nxp.com
> Cc: trix@redhat.com; mdr@ashroe.eu; Richardson, Bruce
> <bruce.richardson@intel.com>; david.marchand@redhat.com;
> stephen@networkplumber.org
> Subject: Re: [PATCH v5 2/7] bbdev: add device status info
>
>
>
> On 7/7/22 01:28, Nicolas Chautru wrote:
> > Added device status information, so that the PMD can expose
> > information related to the underlying accelerator device status.
> > Minor order change in structure to fit into padding hole.
> >
> > Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
> > ---
> > drivers/baseband/acc100/rte_acc100_pmd.c | 1 +
> > drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c | 1 +
> > drivers/baseband/fpga_lte_fec/fpga_lte_fec.c | 1 +
> > drivers/baseband/la12xx/bbdev_la12xx.c | 1 +
> > drivers/baseband/null/bbdev_null.c | 1 +
> > drivers/baseband/turbo_sw/bbdev_turbo_software.c | 1 +
> > lib/bbdev/rte_bbdev.c | 22 ++++++++++++++
> > lib/bbdev/rte_bbdev.h | 35 ++++++++++++++++++++--
> > lib/bbdev/version.map | 6 ++++
> > 9 files changed, 67 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/baseband/acc100/rte_acc100_pmd.c
> > b/drivers/baseband/acc100/rte_acc100_pmd.c
> > index de7e4bc..17ba798 100644
> > --- a/drivers/baseband/acc100/rte_acc100_pmd.c
> > +++ b/drivers/baseband/acc100/rte_acc100_pmd.c
> > @@ -1060,6 +1060,7 @@
> >
> > /* Read and save the populated config from ACC100 registers */
> > fetch_acc100_config(dev);
> > + dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
> >
> > /* This isn't ideal because it reports the maximum number of queues
> but
> > * does not provide info on how many can be uplink/downlink or
> > different diff --git
> > a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> > b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> > index 82ae6ba..57b12af 100644
> > --- a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> > +++ b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> > @@ -369,6 +369,7 @@
> > dev_info->capabilities = bbdev_capabilities;
> > dev_info->cpu_flag_reqs = NULL;
> > dev_info->data_endianness = RTE_LITTLE_ENDIAN;
> > + dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
> >
> > /* Calculates number of queues assigned to device */
> > dev_info->max_num_queues = 0;
> > diff --git a/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
> > b/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
> > index 21d3529..2a330c4 100644
> > --- a/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
> > +++ b/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
> > @@ -645,6 +645,7 @@ struct __rte_cache_aligned fpga_queue {
> > dev_info->capabilities = bbdev_capabilities;
> > dev_info->cpu_flag_reqs = NULL;
> > dev_info->data_endianness = RTE_LITTLE_ENDIAN;
> > + dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
> >
> > /* Calculates number of queues assigned to device */
> > dev_info->max_num_queues = 0;
> > diff --git a/drivers/baseband/la12xx/bbdev_la12xx.c
> > b/drivers/baseband/la12xx/bbdev_la12xx.c
> > index 4d1bd16..c1f88c6 100644
> > --- a/drivers/baseband/la12xx/bbdev_la12xx.c
> > +++ b/drivers/baseband/la12xx/bbdev_la12xx.c
> > @@ -100,6 +100,7 @@ struct bbdev_la12xx_params {
> > dev_info->capabilities = bbdev_capabilities;
> > dev_info->cpu_flag_reqs = NULL;
> > dev_info->min_alignment = 64;
> > + dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
> >
> > rte_bbdev_log_debug("got device info from %u", dev->data-
> >dev_id);
> > }
> > diff --git a/drivers/baseband/null/bbdev_null.c
> > b/drivers/baseband/null/bbdev_null.c
> > index 248e129..94a1976 100644
> > --- a/drivers/baseband/null/bbdev_null.c
> > +++ b/drivers/baseband/null/bbdev_null.c
> > @@ -82,6 +82,7 @@ struct bbdev_queue {
> > * here for code completeness.
> > */
> > dev_info->data_endianness = RTE_LITTLE_ENDIAN;
> > + dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
> >
> > rte_bbdev_log_debug("got device info from %u", dev->data-
> >dev_id);
> > }
> > diff --git a/drivers/baseband/turbo_sw/bbdev_turbo_software.c
> > b/drivers/baseband/turbo_sw/bbdev_turbo_software.c
> > index af7bc41..dbc5524 100644
> > --- a/drivers/baseband/turbo_sw/bbdev_turbo_software.c
> > +++ b/drivers/baseband/turbo_sw/bbdev_turbo_software.c
> > @@ -254,6 +254,7 @@ struct turbo_sw_queue {
> > dev_info->min_alignment = 64;
> > dev_info->harq_buffer_size = 0;
> > dev_info->data_endianness = RTE_LITTLE_ENDIAN;
> > + dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
> >
> > rte_bbdev_log_debug("got device info from %u\n", dev->data-
> >dev_id);
> > }
> > diff --git a/lib/bbdev/rte_bbdev.c b/lib/bbdev/rte_bbdev.c index
> > 4da8047..38630a2 100644
> > --- a/lib/bbdev/rte_bbdev.c
> > +++ b/lib/bbdev/rte_bbdev.c
> > @@ -1133,3 +1133,25 @@ struct rte_mempool *
> > rte_bbdev_log(ERR, "Invalid operation type");
> > return NULL;
> > }
> > +
> > +const char *
> > +rte_bbdev_device_status_str(enum rte_bbdev_device_status status) {
> > + static const char * const dev_sta_string[] = {
> > + "RTE_BBDEV_DEV_NOSTATUS",
> > + "RTE_BBDEV_DEV_NOT_SUPPORTED",
> > + "RTE_BBDEV_DEV_RESET",
> > + "RTE_BBDEV_DEV_CONFIGURED",
> > + "RTE_BBDEV_DEV_ACTIVE",
> > + "RTE_BBDEV_DEV_FATAL_ERR",
> > + "RTE_BBDEV_DEV_RESTART_REQ",
> > + "RTE_BBDEV_DEV_RECONFIG_REQ",
> > + "RTE_BBDEV_DEV_CORRECT_ERR",
> > + };
> > +
> > + if (status < sizeof(dev_sta_string) / sizeof(char *))
> > + return dev_sta_string[status];
> > +
> > + rte_bbdev_log(ERR, "Invalid device status");
> > + return NULL;
> > +}
> > diff --git a/lib/bbdev/rte_bbdev.h b/lib/bbdev/rte_bbdev.h index
> > b88c881..9b1ffa4 100644
> > --- a/lib/bbdev/rte_bbdev.h
> > +++ b/lib/bbdev/rte_bbdev.h
> > @@ -223,6 +223,21 @@ struct rte_bbdev_queue_conf {
> > int
> > rte_bbdev_queue_stop(uint16_t dev_id, uint16_t queue_id);
> >
> > +/**
> > + * Flags indicate the status of the device */ enum
> > +rte_bbdev_device_status {
> > + RTE_BBDEV_DEV_NOSTATUS, /**< Nothing being reported */
> > + RTE_BBDEV_DEV_NOT_SUPPORTED, /**< Device status is not
> supported on the PMD */
> > + RTE_BBDEV_DEV_RESET, /**< Device in reset and un-
> configured state */
> > + RTE_BBDEV_DEV_CONFIGURED, /**< Device is configured and
> ready to use */
> > + RTE_BBDEV_DEV_ACTIVE, /**< Device is configured and VF is
> being used */
> > + RTE_BBDEV_DEV_FATAL_ERR, /**< Device has hit a fatal
> uncorrectable error */
> > + RTE_BBDEV_DEV_RESTART_REQ, /**< Device requires application
> to restart */
> > + RTE_BBDEV_DEV_RECONFIG_REQ, /**< Device requires
> application to reconfigure queues */
> > + RTE_BBDEV_DEV_CORRECT_ERR, /**< Warning of a correctable
> error event happened */
> > +};
>
> I don't have a strong opinion on this, but I think NOT_SUPPORTED should be
> a special value. If you want to keep 0 value for NOSTATUS, maybe you could
> do:
>
> enum rte_bbdev_device_status {
> RTE_BBDEV_DEV_NOT_SUPPORTED = -1, /**< Device status is not
> supported
> on the PMD */
> RTE_BBDEV_DEV_NOSTATUS = 0, /**< Nothing being reported
> */
> RTE_BBDEV_DEV_RESET, /**< Device in reset and un-
> configured
> state */
> ...
Thanks Maxime. My concern is that I am upstreaming in parallel in pf_bb_config in parallel hence would like to keep it unchanged if possible.
Given you don’t have a strong opinion is that okay to keep as is? Or I can force special value 1 for NOT_SUPPORTED so that this is explicitly defined. But really enum should always be used.
>
>
> > +
> > /** Device statistics. */
> > struct rte_bbdev_stats {
> > uint64_t enqueued_count; /**< Count of all operations enqueued */
> > @@ -285,12 +300,14 @@ struct rte_bbdev_driver_info {
> > /** Set if device supports per-queue interrupts */
> > bool queue_intr_supported;
> > /** Minimum alignment of buffers, in bytes */
> > - uint16_t min_alignment;
> > - /** HARQ memory available in kB */
> > + /** Device Status */
> > + enum rte_bbdev_device_status device_status;
> > uint32_t harq_buffer_size;
> > /** Byte endianness (RTE_BIG_ENDIAN/RTE_LITTLE_ENDIAN)
> supported
> > * for input/output data
> > */
> > + uint16_t min_alignment;
> > + /** HARQ memory available in kB */
> > uint8_t data_endianness;
> > /** Default queue configuration used if none is supplied */
> > struct rte_bbdev_queue_conf default_queue_conf; @@ -827,6
> +844,20
> > @@ typedef void (*rte_bbdev_cb_fn)(uint16_t dev_id,
> > rte_bbdev_queue_intr_ctl(uint16_t dev_id, uint16_t queue_id, int epfd, int
> op,
> > void *data);
> >
> > +/**
> > + * Converts device status from enum to string
> > + *
> > + * @param status
> > + * Device status as enum
> > + *
> > + * @returns
> > + * Operation type as string or NULL if op_type is invalid
> > + *
> > + */
> > +__rte_experimental
> > +const char*
> > +rte_bbdev_device_status_str(enum rte_bbdev_device_status status);
> > +
> > #ifdef __cplusplus
> > }
> > #endif
> > diff --git a/lib/bbdev/version.map b/lib/bbdev/version.map index
> > cce3f3c..9ac3643 100644
> > --- a/lib/bbdev/version.map
> > +++ b/lib/bbdev/version.map
> > @@ -39,3 +39,9 @@ DPDK_22 {
> >
> > local: *;
> > };
> > +
> > +EXPERIMENTAL {
> > + global:
> > +
>
> We now add the version the new API was introduced in as a comment:
>
> # added in 22.11
Thanks for this feedback, I will update this
> > + rte_bbdev_device_status_str;
> > +};
Hi,
On 8/25/22 20:30, Chautru, Nicolas wrote:
> Thanks Maxime,
>
>> -----Original Message-----
>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Sent: Thursday, August 25, 2022 7:19 AM
>> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org;
>> thomas@monjalon.net; gakhil@marvell.com; hemant.agrawal@nxp.com
>> Cc: trix@redhat.com; mdr@ashroe.eu; Richardson, Bruce
>> <bruce.richardson@intel.com>; david.marchand@redhat.com;
>> stephen@networkplumber.org
>> Subject: Re: [PATCH v5 2/7] bbdev: add device status info
>>
>>
>>
>> On 7/7/22 01:28, Nicolas Chautru wrote:
>>> Added device status information, so that the PMD can expose
>>> information related to the underlying accelerator device status.
>>> Minor order change in structure to fit into padding hole.
>>>
>>> Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
>>> ---
>>> drivers/baseband/acc100/rte_acc100_pmd.c | 1 +
>>> drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c | 1 +
>>> drivers/baseband/fpga_lte_fec/fpga_lte_fec.c | 1 +
>>> drivers/baseband/la12xx/bbdev_la12xx.c | 1 +
>>> drivers/baseband/null/bbdev_null.c | 1 +
>>> drivers/baseband/turbo_sw/bbdev_turbo_software.c | 1 +
>>> lib/bbdev/rte_bbdev.c | 22 ++++++++++++++
>>> lib/bbdev/rte_bbdev.h | 35 ++++++++++++++++++++--
>>> lib/bbdev/version.map | 6 ++++
>>> 9 files changed, 67 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/baseband/acc100/rte_acc100_pmd.c
>>> b/drivers/baseband/acc100/rte_acc100_pmd.c
>>> index de7e4bc..17ba798 100644
>>> --- a/drivers/baseband/acc100/rte_acc100_pmd.c
>>> +++ b/drivers/baseband/acc100/rte_acc100_pmd.c
>>> @@ -1060,6 +1060,7 @@
>>>
>>> /* Read and save the populated config from ACC100 registers */
>>> fetch_acc100_config(dev);
>>> + dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
>>>
>>> /* This isn't ideal because it reports the maximum number of queues
>> but
>>> * does not provide info on how many can be uplink/downlink or
>>> different diff --git
>>> a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
>>> b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
>>> index 82ae6ba..57b12af 100644
>>> --- a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
>>> +++ b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
>>> @@ -369,6 +369,7 @@
>>> dev_info->capabilities = bbdev_capabilities;
>>> dev_info->cpu_flag_reqs = NULL;
>>> dev_info->data_endianness = RTE_LITTLE_ENDIAN;
>>> + dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
>>>
>>> /* Calculates number of queues assigned to device */
>>> dev_info->max_num_queues = 0;
>>> diff --git a/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
>>> b/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
>>> index 21d3529..2a330c4 100644
>>> --- a/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
>>> +++ b/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
>>> @@ -645,6 +645,7 @@ struct __rte_cache_aligned fpga_queue {
>>> dev_info->capabilities = bbdev_capabilities;
>>> dev_info->cpu_flag_reqs = NULL;
>>> dev_info->data_endianness = RTE_LITTLE_ENDIAN;
>>> + dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
>>>
>>> /* Calculates number of queues assigned to device */
>>> dev_info->max_num_queues = 0;
>>> diff --git a/drivers/baseband/la12xx/bbdev_la12xx.c
>>> b/drivers/baseband/la12xx/bbdev_la12xx.c
>>> index 4d1bd16..c1f88c6 100644
>>> --- a/drivers/baseband/la12xx/bbdev_la12xx.c
>>> +++ b/drivers/baseband/la12xx/bbdev_la12xx.c
>>> @@ -100,6 +100,7 @@ struct bbdev_la12xx_params {
>>> dev_info->capabilities = bbdev_capabilities;
>>> dev_info->cpu_flag_reqs = NULL;
>>> dev_info->min_alignment = 64;
>>> + dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
>>>
>>> rte_bbdev_log_debug("got device info from %u", dev->data-
>>> dev_id);
>>> }
>>> diff --git a/drivers/baseband/null/bbdev_null.c
>>> b/drivers/baseband/null/bbdev_null.c
>>> index 248e129..94a1976 100644
>>> --- a/drivers/baseband/null/bbdev_null.c
>>> +++ b/drivers/baseband/null/bbdev_null.c
>>> @@ -82,6 +82,7 @@ struct bbdev_queue {
>>> * here for code completeness.
>>> */
>>> dev_info->data_endianness = RTE_LITTLE_ENDIAN;
>>> + dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
>>>
>>> rte_bbdev_log_debug("got device info from %u", dev->data-
>>> dev_id);
>>> }
>>> diff --git a/drivers/baseband/turbo_sw/bbdev_turbo_software.c
>>> b/drivers/baseband/turbo_sw/bbdev_turbo_software.c
>>> index af7bc41..dbc5524 100644
>>> --- a/drivers/baseband/turbo_sw/bbdev_turbo_software.c
>>> +++ b/drivers/baseband/turbo_sw/bbdev_turbo_software.c
>>> @@ -254,6 +254,7 @@ struct turbo_sw_queue {
>>> dev_info->min_alignment = 64;
>>> dev_info->harq_buffer_size = 0;
>>> dev_info->data_endianness = RTE_LITTLE_ENDIAN;
>>> + dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
>>>
>>> rte_bbdev_log_debug("got device info from %u\n", dev->data-
>>> dev_id);
>>> }
>>> diff --git a/lib/bbdev/rte_bbdev.c b/lib/bbdev/rte_bbdev.c index
>>> 4da8047..38630a2 100644
>>> --- a/lib/bbdev/rte_bbdev.c
>>> +++ b/lib/bbdev/rte_bbdev.c
>>> @@ -1133,3 +1133,25 @@ struct rte_mempool *
>>> rte_bbdev_log(ERR, "Invalid operation type");
>>> return NULL;
>>> }
>>> +
>>> +const char *
>>> +rte_bbdev_device_status_str(enum rte_bbdev_device_status status) {
>>> + static const char * const dev_sta_string[] = {
>>> + "RTE_BBDEV_DEV_NOSTATUS",
>>> + "RTE_BBDEV_DEV_NOT_SUPPORTED",
>>> + "RTE_BBDEV_DEV_RESET",
>>> + "RTE_BBDEV_DEV_CONFIGURED",
>>> + "RTE_BBDEV_DEV_ACTIVE",
>>> + "RTE_BBDEV_DEV_FATAL_ERR",
>>> + "RTE_BBDEV_DEV_RESTART_REQ",
>>> + "RTE_BBDEV_DEV_RECONFIG_REQ",
>>> + "RTE_BBDEV_DEV_CORRECT_ERR",
>>> + };
>>> +
>>> + if (status < sizeof(dev_sta_string) / sizeof(char *))
>>> + return dev_sta_string[status];
>>> +
>>> + rte_bbdev_log(ERR, "Invalid device status");
>>> + return NULL;
>>> +}
>>> diff --git a/lib/bbdev/rte_bbdev.h b/lib/bbdev/rte_bbdev.h index
>>> b88c881..9b1ffa4 100644
>>> --- a/lib/bbdev/rte_bbdev.h
>>> +++ b/lib/bbdev/rte_bbdev.h
>>> @@ -223,6 +223,21 @@ struct rte_bbdev_queue_conf {
>>> int
>>> rte_bbdev_queue_stop(uint16_t dev_id, uint16_t queue_id);
>>>
>>> +/**
>>> + * Flags indicate the status of the device */ enum
>>> +rte_bbdev_device_status {
>>> + RTE_BBDEV_DEV_NOSTATUS, /**< Nothing being reported */
>>> + RTE_BBDEV_DEV_NOT_SUPPORTED, /**< Device status is not
>> supported on the PMD */
>>> + RTE_BBDEV_DEV_RESET, /**< Device in reset and un-
>> configured state */
>>> + RTE_BBDEV_DEV_CONFIGURED, /**< Device is configured and
>> ready to use */
>>> + RTE_BBDEV_DEV_ACTIVE, /**< Device is configured and VF is
>> being used */
>>> + RTE_BBDEV_DEV_FATAL_ERR, /**< Device has hit a fatal
>> uncorrectable error */
>>> + RTE_BBDEV_DEV_RESTART_REQ, /**< Device requires application
>> to restart */
>>> + RTE_BBDEV_DEV_RECONFIG_REQ, /**< Device requires
>> application to reconfigure queues */
>>> + RTE_BBDEV_DEV_CORRECT_ERR, /**< Warning of a correctable
>> error event happened */
>>> +};
>>
>> I don't have a strong opinion on this, but I think NOT_SUPPORTED should be
>> a special value. If you want to keep 0 value for NOSTATUS, maybe you could
>> do:
>>
>> enum rte_bbdev_device_status {
>> RTE_BBDEV_DEV_NOT_SUPPORTED = -1, /**< Device status is not
>> supported
>> on the PMD */
>> RTE_BBDEV_DEV_NOSTATUS = 0, /**< Nothing being reported
>> */
>> RTE_BBDEV_DEV_RESET, /**< Device in reset and un-
>> configured
>> state */
>> ...
>
> Thanks Maxime. My concern is that I am upstreaming in parallel in pf_bb_config in parallel hence would like to keep it unchanged if possible.
> Given you don’t have a strong opinion is that okay to keep as is? Or I can force special value 1 for NOT_SUPPORTED so that this is explicitly defined. But really enum should always be used.
I don't understand. It should not have any impact on pf_bb_config, given
pf_bb_config does not use DPDK.
Maxime
Hi Maxime,
> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Friday, August 26, 2022 3:13 AM
> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org;
> thomas@monjalon.net; gakhil@marvell.com; hemant.agrawal@nxp.com
> Cc: trix@redhat.com; mdr@ashroe.eu; Richardson, Bruce
> <bruce.richardson@intel.com>; david.marchand@redhat.com;
> stephen@networkplumber.org
> Subject: Re: [PATCH v5 2/7] bbdev: add device status info
>
> Hi,
>
> On 8/25/22 20:30, Chautru, Nicolas wrote:
> > Thanks Maxime,
> >
> >> -----Original Message-----
> >> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> >> Sent: Thursday, August 25, 2022 7:19 AM
> >> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org;
> >> thomas@monjalon.net; gakhil@marvell.com; hemant.agrawal@nxp.com
> >> Cc: trix@redhat.com; mdr@ashroe.eu; Richardson, Bruce
> >> <bruce.richardson@intel.com>; david.marchand@redhat.com;
> >> stephen@networkplumber.org
> >> Subject: Re: [PATCH v5 2/7] bbdev: add device status info
> >>
> >>
> >>
> >> On 7/7/22 01:28, Nicolas Chautru wrote:
> >>> Added device status information, so that the PMD can expose
> >>> information related to the underlying accelerator device status.
> >>> Minor order change in structure to fit into padding hole.
> >>>
> >>> Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
> >>> ---
> >>> drivers/baseband/acc100/rte_acc100_pmd.c | 1 +
> >>> drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c | 1 +
> >>> drivers/baseband/fpga_lte_fec/fpga_lte_fec.c | 1 +
> >>> drivers/baseband/la12xx/bbdev_la12xx.c | 1 +
> >>> drivers/baseband/null/bbdev_null.c | 1 +
> >>> drivers/baseband/turbo_sw/bbdev_turbo_software.c | 1 +
> >>> lib/bbdev/rte_bbdev.c | 22 ++++++++++++++
> >>> lib/bbdev/rte_bbdev.h | 35 ++++++++++++++++++++--
> >>> lib/bbdev/version.map | 6 ++++
> >>> 9 files changed, 67 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/baseband/acc100/rte_acc100_pmd.c
> >>> b/drivers/baseband/acc100/rte_acc100_pmd.c
> >>> index de7e4bc..17ba798 100644
> >>> --- a/drivers/baseband/acc100/rte_acc100_pmd.c
> >>> +++ b/drivers/baseband/acc100/rte_acc100_pmd.c
> >>> @@ -1060,6 +1060,7 @@
> >>>
> >>> /* Read and save the populated config from ACC100 registers */
> >>> fetch_acc100_config(dev);
> >>> + dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
> >>>
> >>> /* This isn't ideal because it reports the maximum number of
> >>> queues
> >> but
> >>> * does not provide info on how many can be uplink/downlink or
> >>> different diff --git
> >>> a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> >>> b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> >>> index 82ae6ba..57b12af 100644
> >>> --- a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> >>> +++ b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> >>> @@ -369,6 +369,7 @@
> >>> dev_info->capabilities = bbdev_capabilities;
> >>> dev_info->cpu_flag_reqs = NULL;
> >>> dev_info->data_endianness = RTE_LITTLE_ENDIAN;
> >>> + dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
> >>>
> >>> /* Calculates number of queues assigned to device */
> >>> dev_info->max_num_queues = 0;
> >>> diff --git a/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
> >>> b/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
> >>> index 21d3529..2a330c4 100644
> >>> --- a/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
> >>> +++ b/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
> >>> @@ -645,6 +645,7 @@ struct __rte_cache_aligned fpga_queue {
> >>> dev_info->capabilities = bbdev_capabilities;
> >>> dev_info->cpu_flag_reqs = NULL;
> >>> dev_info->data_endianness = RTE_LITTLE_ENDIAN;
> >>> + dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
> >>>
> >>> /* Calculates number of queues assigned to device */
> >>> dev_info->max_num_queues = 0;
> >>> diff --git a/drivers/baseband/la12xx/bbdev_la12xx.c
> >>> b/drivers/baseband/la12xx/bbdev_la12xx.c
> >>> index 4d1bd16..c1f88c6 100644
> >>> --- a/drivers/baseband/la12xx/bbdev_la12xx.c
> >>> +++ b/drivers/baseband/la12xx/bbdev_la12xx.c
> >>> @@ -100,6 +100,7 @@ struct bbdev_la12xx_params {
> >>> dev_info->capabilities = bbdev_capabilities;
> >>> dev_info->cpu_flag_reqs = NULL;
> >>> dev_info->min_alignment = 64;
> >>> + dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
> >>>
> >>> rte_bbdev_log_debug("got device info from %u", dev->data-
> >>> dev_id);
> >>> }
> >>> diff --git a/drivers/baseband/null/bbdev_null.c
> >>> b/drivers/baseband/null/bbdev_null.c
> >>> index 248e129..94a1976 100644
> >>> --- a/drivers/baseband/null/bbdev_null.c
> >>> +++ b/drivers/baseband/null/bbdev_null.c
> >>> @@ -82,6 +82,7 @@ struct bbdev_queue {
> >>> * here for code completeness.
> >>> */
> >>> dev_info->data_endianness = RTE_LITTLE_ENDIAN;
> >>> + dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
> >>>
> >>> rte_bbdev_log_debug("got device info from %u", dev->data-
> >>> dev_id);
> >>> }
> >>> diff --git a/drivers/baseband/turbo_sw/bbdev_turbo_software.c
> >>> b/drivers/baseband/turbo_sw/bbdev_turbo_software.c
> >>> index af7bc41..dbc5524 100644
> >>> --- a/drivers/baseband/turbo_sw/bbdev_turbo_software.c
> >>> +++ b/drivers/baseband/turbo_sw/bbdev_turbo_software.c
> >>> @@ -254,6 +254,7 @@ struct turbo_sw_queue {
> >>> dev_info->min_alignment = 64;
> >>> dev_info->harq_buffer_size = 0;
> >>> dev_info->data_endianness = RTE_LITTLE_ENDIAN;
> >>> + dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
> >>>
> >>> rte_bbdev_log_debug("got device info from %u\n", dev->data-
> >>> dev_id);
> >>> }
> >>> diff --git a/lib/bbdev/rte_bbdev.c b/lib/bbdev/rte_bbdev.c index
> >>> 4da8047..38630a2 100644
> >>> --- a/lib/bbdev/rte_bbdev.c
> >>> +++ b/lib/bbdev/rte_bbdev.c
> >>> @@ -1133,3 +1133,25 @@ struct rte_mempool *
> >>> rte_bbdev_log(ERR, "Invalid operation type");
> >>> return NULL;
> >>> }
> >>> +
> >>> +const char *
> >>> +rte_bbdev_device_status_str(enum rte_bbdev_device_status status) {
> >>> + static const char * const dev_sta_string[] = {
> >>> + "RTE_BBDEV_DEV_NOSTATUS",
> >>> + "RTE_BBDEV_DEV_NOT_SUPPORTED",
> >>> + "RTE_BBDEV_DEV_RESET",
> >>> + "RTE_BBDEV_DEV_CONFIGURED",
> >>> + "RTE_BBDEV_DEV_ACTIVE",
> >>> + "RTE_BBDEV_DEV_FATAL_ERR",
> >>> + "RTE_BBDEV_DEV_RESTART_REQ",
> >>> + "RTE_BBDEV_DEV_RECONFIG_REQ",
> >>> + "RTE_BBDEV_DEV_CORRECT_ERR",
> >>> + };
> >>> +
> >>> + if (status < sizeof(dev_sta_string) / sizeof(char *))
> >>> + return dev_sta_string[status];
> >>> +
> >>> + rte_bbdev_log(ERR, "Invalid device status");
> >>> + return NULL;
> >>> +}
> >>> diff --git a/lib/bbdev/rte_bbdev.h b/lib/bbdev/rte_bbdev.h index
> >>> b88c881..9b1ffa4 100644
> >>> --- a/lib/bbdev/rte_bbdev.h
> >>> +++ b/lib/bbdev/rte_bbdev.h
> >>> @@ -223,6 +223,21 @@ struct rte_bbdev_queue_conf {
> >>> int
> >>> rte_bbdev_queue_stop(uint16_t dev_id, uint16_t queue_id);
> >>>
> >>> +/**
> >>> + * Flags indicate the status of the device */ enum
> >>> +rte_bbdev_device_status {
> >>> + RTE_BBDEV_DEV_NOSTATUS, /**< Nothing being reported */
> >>> + RTE_BBDEV_DEV_NOT_SUPPORTED, /**< Device status is not
> >> supported on the PMD */
> >>> + RTE_BBDEV_DEV_RESET, /**< Device in reset and un-
> >> configured state */
> >>> + RTE_BBDEV_DEV_CONFIGURED, /**< Device is configured and
> >> ready to use */
> >>> + RTE_BBDEV_DEV_ACTIVE, /**< Device is configured and VF is
> >> being used */
> >>> + RTE_BBDEV_DEV_FATAL_ERR, /**< Device has hit a fatal
> >> uncorrectable error */
> >>> + RTE_BBDEV_DEV_RESTART_REQ, /**< Device requires application
> >> to restart */
> >>> + RTE_BBDEV_DEV_RECONFIG_REQ, /**< Device requires
> >> application to reconfigure queues */
> >>> + RTE_BBDEV_DEV_CORRECT_ERR, /**< Warning of a correctable
> >> error event happened */
> >>> +};
> >>
> >> I don't have a strong opinion on this, but I think NOT_SUPPORTED
> >> should be a special value. If you want to keep 0 value for NOSTATUS,
> >> maybe you could
> >> do:
> >>
> >> enum rte_bbdev_device_status {
> >> RTE_BBDEV_DEV_NOT_SUPPORTED = -1, /**< Device status is not
> >> supported
> >> on the PMD */
> >> RTE_BBDEV_DEV_NOSTATUS = 0, /**< Nothing being reported
> >> */
> >> RTE_BBDEV_DEV_RESET, /**< Device in reset and un-
> >> configured
> >> state */
> >> ...
> >
> > Thanks Maxime. My concern is that I am upstreaming in parallel in
> pf_bb_config in parallel hence would like to keep it unchanged if possible.
> > Given you don’t have a strong opinion is that okay to keep as is? Or I can
> force special value 1 for NOT_SUPPORTED so that this is explicitly defined. But
> really enum should always be used.
>
> I don't understand. It should not have any impact on pf_bb_config, given
> pf_bb_config does not use DPDK.
>
> Maxime
That device status is being shared from pf_bb_config to the bbdev PMD through PF2VF communications, hence they share that same enum.
On 8/29/22 18:10, Chautru, Nicolas wrote:
> Hi Maxime,
>
>> -----Original Message-----
>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Sent: Friday, August 26, 2022 3:13 AM
>> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org;
>> thomas@monjalon.net; gakhil@marvell.com; hemant.agrawal@nxp.com
>> Cc: trix@redhat.com; mdr@ashroe.eu; Richardson, Bruce
>> <bruce.richardson@intel.com>; david.marchand@redhat.com;
>> stephen@networkplumber.org
>> Subject: Re: [PATCH v5 2/7] bbdev: add device status info
>>
>> Hi,
>>
>> On 8/25/22 20:30, Chautru, Nicolas wrote:
>>> Thanks Maxime,
>>>
>>>> -----Original Message-----
>>>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>>>> Sent: Thursday, August 25, 2022 7:19 AM
>>>> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org;
>>>> thomas@monjalon.net; gakhil@marvell.com; hemant.agrawal@nxp.com
>>>> Cc: trix@redhat.com; mdr@ashroe.eu; Richardson, Bruce
>>>> <bruce.richardson@intel.com>; david.marchand@redhat.com;
>>>> stephen@networkplumber.org
>>>> Subject: Re: [PATCH v5 2/7] bbdev: add device status info
>>>>
>>>>
>>>>
>>>> On 7/7/22 01:28, Nicolas Chautru wrote:
>>>>> Added device status information, so that the PMD can expose
>>>>> information related to the underlying accelerator device status.
>>>>> Minor order change in structure to fit into padding hole.
>>>>>
>>>>> Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
>>>>> ---
>>>>> drivers/baseband/acc100/rte_acc100_pmd.c | 1 +
>>>>> drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c | 1 +
>>>>> drivers/baseband/fpga_lte_fec/fpga_lte_fec.c | 1 +
>>>>> drivers/baseband/la12xx/bbdev_la12xx.c | 1 +
>>>>> drivers/baseband/null/bbdev_null.c | 1 +
>>>>> drivers/baseband/turbo_sw/bbdev_turbo_software.c | 1 +
>>>>> lib/bbdev/rte_bbdev.c | 22 ++++++++++++++
>>>>> lib/bbdev/rte_bbdev.h | 35 ++++++++++++++++++++--
>>>>> lib/bbdev/version.map | 6 ++++
>>>>> 9 files changed, 67 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/baseband/acc100/rte_acc100_pmd.c
>>>>> b/drivers/baseband/acc100/rte_acc100_pmd.c
>>>>> index de7e4bc..17ba798 100644
>>>>> --- a/drivers/baseband/acc100/rte_acc100_pmd.c
>>>>> +++ b/drivers/baseband/acc100/rte_acc100_pmd.c
>>>>> @@ -1060,6 +1060,7 @@
>>>>>
>>>>> /* Read and save the populated config from ACC100 registers */
>>>>> fetch_acc100_config(dev);
>>>>> + dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
>>>>>
>>>>> /* This isn't ideal because it reports the maximum number of
>>>>> queues
>>>> but
>>>>> * does not provide info on how many can be uplink/downlink or
>>>>> different diff --git
>>>>> a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
>>>>> b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
>>>>> index 82ae6ba..57b12af 100644
>>>>> --- a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
>>>>> +++ b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
>>>>> @@ -369,6 +369,7 @@
>>>>> dev_info->capabilities = bbdev_capabilities;
>>>>> dev_info->cpu_flag_reqs = NULL;
>>>>> dev_info->data_endianness = RTE_LITTLE_ENDIAN;
>>>>> + dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
>>>>>
>>>>> /* Calculates number of queues assigned to device */
>>>>> dev_info->max_num_queues = 0;
>>>>> diff --git a/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
>>>>> b/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
>>>>> index 21d3529..2a330c4 100644
>>>>> --- a/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
>>>>> +++ b/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
>>>>> @@ -645,6 +645,7 @@ struct __rte_cache_aligned fpga_queue {
>>>>> dev_info->capabilities = bbdev_capabilities;
>>>>> dev_info->cpu_flag_reqs = NULL;
>>>>> dev_info->data_endianness = RTE_LITTLE_ENDIAN;
>>>>> + dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
>>>>>
>>>>> /* Calculates number of queues assigned to device */
>>>>> dev_info->max_num_queues = 0;
>>>>> diff --git a/drivers/baseband/la12xx/bbdev_la12xx.c
>>>>> b/drivers/baseband/la12xx/bbdev_la12xx.c
>>>>> index 4d1bd16..c1f88c6 100644
>>>>> --- a/drivers/baseband/la12xx/bbdev_la12xx.c
>>>>> +++ b/drivers/baseband/la12xx/bbdev_la12xx.c
>>>>> @@ -100,6 +100,7 @@ struct bbdev_la12xx_params {
>>>>> dev_info->capabilities = bbdev_capabilities;
>>>>> dev_info->cpu_flag_reqs = NULL;
>>>>> dev_info->min_alignment = 64;
>>>>> + dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
>>>>>
>>>>> rte_bbdev_log_debug("got device info from %u", dev->data-
>>>>> dev_id);
>>>>> }
>>>>> diff --git a/drivers/baseband/null/bbdev_null.c
>>>>> b/drivers/baseband/null/bbdev_null.c
>>>>> index 248e129..94a1976 100644
>>>>> --- a/drivers/baseband/null/bbdev_null.c
>>>>> +++ b/drivers/baseband/null/bbdev_null.c
>>>>> @@ -82,6 +82,7 @@ struct bbdev_queue {
>>>>> * here for code completeness.
>>>>> */
>>>>> dev_info->data_endianness = RTE_LITTLE_ENDIAN;
>>>>> + dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
>>>>>
>>>>> rte_bbdev_log_debug("got device info from %u", dev->data-
>>>>> dev_id);
>>>>> }
>>>>> diff --git a/drivers/baseband/turbo_sw/bbdev_turbo_software.c
>>>>> b/drivers/baseband/turbo_sw/bbdev_turbo_software.c
>>>>> index af7bc41..dbc5524 100644
>>>>> --- a/drivers/baseband/turbo_sw/bbdev_turbo_software.c
>>>>> +++ b/drivers/baseband/turbo_sw/bbdev_turbo_software.c
>>>>> @@ -254,6 +254,7 @@ struct turbo_sw_queue {
>>>>> dev_info->min_alignment = 64;
>>>>> dev_info->harq_buffer_size = 0;
>>>>> dev_info->data_endianness = RTE_LITTLE_ENDIAN;
>>>>> + dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
>>>>>
>>>>> rte_bbdev_log_debug("got device info from %u\n", dev->data-
>>>>> dev_id);
>>>>> }
>>>>> diff --git a/lib/bbdev/rte_bbdev.c b/lib/bbdev/rte_bbdev.c index
>>>>> 4da8047..38630a2 100644
>>>>> --- a/lib/bbdev/rte_bbdev.c
>>>>> +++ b/lib/bbdev/rte_bbdev.c
>>>>> @@ -1133,3 +1133,25 @@ struct rte_mempool *
>>>>> rte_bbdev_log(ERR, "Invalid operation type");
>>>>> return NULL;
>>>>> }
>>>>> +
>>>>> +const char *
>>>>> +rte_bbdev_device_status_str(enum rte_bbdev_device_status status) {
>>>>> + static const char * const dev_sta_string[] = {
>>>>> + "RTE_BBDEV_DEV_NOSTATUS",
>>>>> + "RTE_BBDEV_DEV_NOT_SUPPORTED",
>>>>> + "RTE_BBDEV_DEV_RESET",
>>>>> + "RTE_BBDEV_DEV_CONFIGURED",
>>>>> + "RTE_BBDEV_DEV_ACTIVE",
>>>>> + "RTE_BBDEV_DEV_FATAL_ERR",
>>>>> + "RTE_BBDEV_DEV_RESTART_REQ",
>>>>> + "RTE_BBDEV_DEV_RECONFIG_REQ",
>>>>> + "RTE_BBDEV_DEV_CORRECT_ERR",
>>>>> + };
>>>>> +
>>>>> + if (status < sizeof(dev_sta_string) / sizeof(char *))
>>>>> + return dev_sta_string[status];
>>>>> +
>>>>> + rte_bbdev_log(ERR, "Invalid device status");
>>>>> + return NULL;
>>>>> +}
>>>>> diff --git a/lib/bbdev/rte_bbdev.h b/lib/bbdev/rte_bbdev.h index
>>>>> b88c881..9b1ffa4 100644
>>>>> --- a/lib/bbdev/rte_bbdev.h
>>>>> +++ b/lib/bbdev/rte_bbdev.h
>>>>> @@ -223,6 +223,21 @@ struct rte_bbdev_queue_conf {
>>>>> int
>>>>> rte_bbdev_queue_stop(uint16_t dev_id, uint16_t queue_id);
>>>>>
>>>>> +/**
>>>>> + * Flags indicate the status of the device */ enum
>>>>> +rte_bbdev_device_status {
>>>>> + RTE_BBDEV_DEV_NOSTATUS, /**< Nothing being reported */
>>>>> + RTE_BBDEV_DEV_NOT_SUPPORTED, /**< Device status is not
>>>> supported on the PMD */
>>>>> + RTE_BBDEV_DEV_RESET, /**< Device in reset and un-
>>>> configured state */
>>>>> + RTE_BBDEV_DEV_CONFIGURED, /**< Device is configured and
>>>> ready to use */
>>>>> + RTE_BBDEV_DEV_ACTIVE, /**< Device is configured and VF is
>>>> being used */
>>>>> + RTE_BBDEV_DEV_FATAL_ERR, /**< Device has hit a fatal
>>>> uncorrectable error */
>>>>> + RTE_BBDEV_DEV_RESTART_REQ, /**< Device requires application
>>>> to restart */
>>>>> + RTE_BBDEV_DEV_RECONFIG_REQ, /**< Device requires
>>>> application to reconfigure queues */
>>>>> + RTE_BBDEV_DEV_CORRECT_ERR, /**< Warning of a correctable
>>>> error event happened */
>>>>> +};
>>>>
>>>> I don't have a strong opinion on this, but I think NOT_SUPPORTED
>>>> should be a special value. If you want to keep 0 value for NOSTATUS,
>>>> maybe you could
>>>> do:
>>>>
>>>> enum rte_bbdev_device_status {
>>>> RTE_BBDEV_DEV_NOT_SUPPORTED = -1, /**< Device status is not
>>>> supported
>>>> on the PMD */
>>>> RTE_BBDEV_DEV_NOSTATUS = 0, /**< Nothing being reported
>>>> */
>>>> RTE_BBDEV_DEV_RESET, /**< Device in reset and un-
>>>> configured
>>>> state */
>>>> ...
>>>
>>> Thanks Maxime. My concern is that I am upstreaming in parallel in
>> pf_bb_config in parallel hence would like to keep it unchanged if possible.
>>> Given you don’t have a strong opinion is that okay to keep as is? Or I can
>> force special value 1 for NOT_SUPPORTED so that this is explicitly defined. But
>> really enum should always be used.
>>
>> I don't understand. It should not have any impact on pf_bb_config, given
>> pf_bb_config does not use DPDK.
>>
>> Maxime
>
> That device status is being shared from pf_bb_config to the bbdev PMD through PF2VF communications, hence they share that same enum.
>
Ok, but generic DPDK ABI should not be dependent on a vendor internal
implementation IMHO.
Maxime
Hi Maxime,
> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Tuesday, August 30, 2022 12:09 AM
> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org;
> thomas@monjalon.net; gakhil@marvell.com; hemant.agrawal@nxp.com
> Cc: trix@redhat.com; mdr@ashroe.eu; Richardson, Bruce
> <bruce.richardson@intel.com>; david.marchand@redhat.com;
> stephen@networkplumber.org
> Subject: Re: [PATCH v5 2/7] bbdev: add device status info
>
>
>
> On 8/29/22 18:10, Chautru, Nicolas wrote:
> > Hi Maxime,
> >
> >> -----Original Message-----
> >> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> >> Sent: Friday, August 26, 2022 3:13 AM
> >> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org;
> >> thomas@monjalon.net; gakhil@marvell.com; hemant.agrawal@nxp.com
> >> Cc: trix@redhat.com; mdr@ashroe.eu; Richardson, Bruce
> >> <bruce.richardson@intel.com>; david.marchand@redhat.com;
> >> stephen@networkplumber.org
> >> Subject: Re: [PATCH v5 2/7] bbdev: add device status info
> >>
> >> Hi,
> >>
> >> On 8/25/22 20:30, Chautru, Nicolas wrote:
> >>> Thanks Maxime,
> >>>
> >>>> -----Original Message-----
> >>>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> >>>> Sent: Thursday, August 25, 2022 7:19 AM
> >>>> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org;
> >>>> thomas@monjalon.net; gakhil@marvell.com;
> hemant.agrawal@nxp.com
> >>>> Cc: trix@redhat.com; mdr@ashroe.eu; Richardson, Bruce
> >>>> <bruce.richardson@intel.com>; david.marchand@redhat.com;
> >>>> stephen@networkplumber.org
> >>>> Subject: Re: [PATCH v5 2/7] bbdev: add device status info
> >>>>
> >>>>
> >>>>
> >>>> On 7/7/22 01:28, Nicolas Chautru wrote:
> >>>>> Added device status information, so that the PMD can expose
> >>>>> information related to the underlying accelerator device status.
> >>>>> Minor order change in structure to fit into padding hole.
> >>>>>
> >>>>> Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
> >>>>> ---
> >>>>> drivers/baseband/acc100/rte_acc100_pmd.c | 1 +
> >>>>> drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c | 1 +
> >>>>> drivers/baseband/fpga_lte_fec/fpga_lte_fec.c | 1 +
> >>>>> drivers/baseband/la12xx/bbdev_la12xx.c | 1 +
> >>>>> drivers/baseband/null/bbdev_null.c | 1 +
> >>>>> drivers/baseband/turbo_sw/bbdev_turbo_software.c | 1 +
> >>>>> lib/bbdev/rte_bbdev.c | 22 ++++++++++++++
> >>>>> lib/bbdev/rte_bbdev.h | 35
> ++++++++++++++++++++--
> >>>>> lib/bbdev/version.map | 6 ++++
> >>>>> 9 files changed, 67 insertions(+), 2 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/baseband/acc100/rte_acc100_pmd.c
> >>>>> b/drivers/baseband/acc100/rte_acc100_pmd.c
> >>>>> index de7e4bc..17ba798 100644
> >>>>> --- a/drivers/baseband/acc100/rte_acc100_pmd.c
> >>>>> +++ b/drivers/baseband/acc100/rte_acc100_pmd.c
> >>>>> @@ -1060,6 +1060,7 @@
> >>>>>
> >>>>> /* Read and save the populated config from ACC100
> registers */
> >>>>> fetch_acc100_config(dev);
> >>>>> + dev_info->device_status =
> RTE_BBDEV_DEV_NOT_SUPPORTED;
> >>>>>
> >>>>> /* This isn't ideal because it reports the maximum number of
> >>>>> queues
> >>>> but
> >>>>> * does not provide info on how many can be
> uplink/downlink
> >>>>> or different diff --git
> >>>>> a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> >>>>> b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> >>>>> index 82ae6ba..57b12af 100644
> >>>>> --- a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> >>>>> +++ b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> >>>>> @@ -369,6 +369,7 @@
> >>>>> dev_info->capabilities = bbdev_capabilities;
> >>>>> dev_info->cpu_flag_reqs = NULL;
> >>>>> dev_info->data_endianness = RTE_LITTLE_ENDIAN;
> >>>>> + dev_info->device_status =
> RTE_BBDEV_DEV_NOT_SUPPORTED;
> >>>>>
> >>>>> /* Calculates number of queues assigned to device */
> >>>>> dev_info->max_num_queues = 0; diff --git
> >>>>> a/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
> >>>>> b/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
> >>>>> index 21d3529..2a330c4 100644
> >>>>> --- a/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
> >>>>> +++ b/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
> >>>>> @@ -645,6 +645,7 @@ struct __rte_cache_aligned fpga_queue {
> >>>>> dev_info->capabilities = bbdev_capabilities;
> >>>>> dev_info->cpu_flag_reqs = NULL;
> >>>>> dev_info->data_endianness = RTE_LITTLE_ENDIAN;
> >>>>> + dev_info->device_status =
> RTE_BBDEV_DEV_NOT_SUPPORTED;
> >>>>>
> >>>>> /* Calculates number of queues assigned to device */
> >>>>> dev_info->max_num_queues = 0; diff --git
> >>>>> a/drivers/baseband/la12xx/bbdev_la12xx.c
> >>>>> b/drivers/baseband/la12xx/bbdev_la12xx.c
> >>>>> index 4d1bd16..c1f88c6 100644
> >>>>> --- a/drivers/baseband/la12xx/bbdev_la12xx.c
> >>>>> +++ b/drivers/baseband/la12xx/bbdev_la12xx.c
> >>>>> @@ -100,6 +100,7 @@ struct bbdev_la12xx_params {
> >>>>> dev_info->capabilities = bbdev_capabilities;
> >>>>> dev_info->cpu_flag_reqs = NULL;
> >>>>> dev_info->min_alignment = 64;
> >>>>> + dev_info->device_status =
> RTE_BBDEV_DEV_NOT_SUPPORTED;
> >>>>>
> >>>>> rte_bbdev_log_debug("got device info from %u", dev->data-
> >>>>> dev_id);
> >>>>> }
> >>>>> diff --git a/drivers/baseband/null/bbdev_null.c
> >>>>> b/drivers/baseband/null/bbdev_null.c
> >>>>> index 248e129..94a1976 100644
> >>>>> --- a/drivers/baseband/null/bbdev_null.c
> >>>>> +++ b/drivers/baseband/null/bbdev_null.c
> >>>>> @@ -82,6 +82,7 @@ struct bbdev_queue {
> >>>>> * here for code completeness.
> >>>>> */
> >>>>> dev_info->data_endianness = RTE_LITTLE_ENDIAN;
> >>>>> + dev_info->device_status =
> RTE_BBDEV_DEV_NOT_SUPPORTED;
> >>>>>
> >>>>> rte_bbdev_log_debug("got device info from %u", dev->data-
> >>>>> dev_id);
> >>>>> }
> >>>>> diff --git a/drivers/baseband/turbo_sw/bbdev_turbo_software.c
> >>>>> b/drivers/baseband/turbo_sw/bbdev_turbo_software.c
> >>>>> index af7bc41..dbc5524 100644
> >>>>> --- a/drivers/baseband/turbo_sw/bbdev_turbo_software.c
> >>>>> +++ b/drivers/baseband/turbo_sw/bbdev_turbo_software.c
> >>>>> @@ -254,6 +254,7 @@ struct turbo_sw_queue {
> >>>>> dev_info->min_alignment = 64;
> >>>>> dev_info->harq_buffer_size = 0;
> >>>>> dev_info->data_endianness = RTE_LITTLE_ENDIAN;
> >>>>> + dev_info->device_status =
> RTE_BBDEV_DEV_NOT_SUPPORTED;
> >>>>>
> >>>>> rte_bbdev_log_debug("got device info from %u\n", dev-
> >data-
> >>>>> dev_id);
> >>>>> }
> >>>>> diff --git a/lib/bbdev/rte_bbdev.c b/lib/bbdev/rte_bbdev.c index
> >>>>> 4da8047..38630a2 100644
> >>>>> --- a/lib/bbdev/rte_bbdev.c
> >>>>> +++ b/lib/bbdev/rte_bbdev.c
> >>>>> @@ -1133,3 +1133,25 @@ struct rte_mempool *
> >>>>> rte_bbdev_log(ERR, "Invalid operation type");
> >>>>> return NULL;
> >>>>> }
> >>>>> +
> >>>>> +const char *
> >>>>> +rte_bbdev_device_status_str(enum rte_bbdev_device_status status)
> {
> >>>>> + static const char * const dev_sta_string[] = {
> >>>>> + "RTE_BBDEV_DEV_NOSTATUS",
> >>>>> + "RTE_BBDEV_DEV_NOT_SUPPORTED",
> >>>>> + "RTE_BBDEV_DEV_RESET",
> >>>>> + "RTE_BBDEV_DEV_CONFIGURED",
> >>>>> + "RTE_BBDEV_DEV_ACTIVE",
> >>>>> + "RTE_BBDEV_DEV_FATAL_ERR",
> >>>>> + "RTE_BBDEV_DEV_RESTART_REQ",
> >>>>> + "RTE_BBDEV_DEV_RECONFIG_REQ",
> >>>>> + "RTE_BBDEV_DEV_CORRECT_ERR",
> >>>>> + };
> >>>>> +
> >>>>> + if (status < sizeof(dev_sta_string) / sizeof(char *))
> >>>>> + return dev_sta_string[status];
> >>>>> +
> >>>>> + rte_bbdev_log(ERR, "Invalid device status");
> >>>>> + return NULL;
> >>>>> +}
> >>>>> diff --git a/lib/bbdev/rte_bbdev.h b/lib/bbdev/rte_bbdev.h index
> >>>>> b88c881..9b1ffa4 100644
> >>>>> --- a/lib/bbdev/rte_bbdev.h
> >>>>> +++ b/lib/bbdev/rte_bbdev.h
> >>>>> @@ -223,6 +223,21 @@ struct rte_bbdev_queue_conf {
> >>>>> int
> >>>>> rte_bbdev_queue_stop(uint16_t dev_id, uint16_t queue_id);
> >>>>>
> >>>>> +/**
> >>>>> + * Flags indicate the status of the device */ enum
> >>>>> +rte_bbdev_device_status {
> >>>>> + RTE_BBDEV_DEV_NOSTATUS, /**< Nothing being
> reported */
> >>>>> + RTE_BBDEV_DEV_NOT_SUPPORTED, /**< Device status is
> not
> >>>> supported on the PMD */
> >>>>> + RTE_BBDEV_DEV_RESET, /**< Device in reset and un-
> >>>> configured state */
> >>>>> + RTE_BBDEV_DEV_CONFIGURED, /**< Device is
> configured and
> >>>> ready to use */
> >>>>> + RTE_BBDEV_DEV_ACTIVE, /**< Device is configured
> and VF is
> >>>> being used */
> >>>>> + RTE_BBDEV_DEV_FATAL_ERR, /**< Device has hit a fatal
> >>>> uncorrectable error */
> >>>>> + RTE_BBDEV_DEV_RESTART_REQ, /**< Device requires
> application
> >>>> to restart */
> >>>>> + RTE_BBDEV_DEV_RECONFIG_REQ, /**< Device requires
> >>>> application to reconfigure queues */
> >>>>> + RTE_BBDEV_DEV_CORRECT_ERR, /**< Warning of a
> correctable
> >>>> error event happened */
> >>>>> +};
> >>>>
> >>>> I don't have a strong opinion on this, but I think NOT_SUPPORTED
> >>>> should be a special value. If you want to keep 0 value for
> >>>> NOSTATUS, maybe you could
> >>>> do:
> >>>>
> >>>> enum rte_bbdev_device_status {
> >>>> RTE_BBDEV_DEV_NOT_SUPPORTED = -1, /**< Device status is not
> >>>> supported
> >>>> on the PMD */
> >>>> RTE_BBDEV_DEV_NOSTATUS = 0, /**< Nothing being reported
> >>>> */
> >>>> RTE_BBDEV_DEV_RESET, /**< Device in reset and un-
> >>>> configured
> >>>> state */
> >>>> ...
> >>>
> >>> Thanks Maxime. My concern is that I am upstreaming in parallel in
> >> pf_bb_config in parallel hence would like to keep it unchanged if possible.
> >>> Given you don’t have a strong opinion is that okay to keep as is? Or
> >>> I can
> >> force special value 1 for NOT_SUPPORTED so that this is explicitly
> >> defined. But really enum should always be used.
> >>
> >> I don't understand. It should not have any impact on pf_bb_config,
> >> given pf_bb_config does not use DPDK.
> >>
> >> Maxime
> >
> > That device status is being shared from pf_bb_config to the bbdev PMD
> through PF2VF communications, hence they share that same enum.
> >
>
> Ok, but generic DPDK ABI should not be dependent on a vendor internal
> implementation IMHO.
>
I agree. This is the opposite direction, pf_bb_config is reusing the same API enumeration for now the assumption is that enum is being applied.
In case we change here (for a fairly cosmetic reason) that would cause to change as well on the other ingredient, which is doable but has overhead.
If you really believe that there is a strong reason to do such a change let me know now, we would need to impact pf_bb_config release to have similar change and match that API change which is possible but not ideal.
Thanks
Nic
@@ -1060,6 +1060,7 @@
/* Read and save the populated config from ACC100 registers */
fetch_acc100_config(dev);
+ dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
/* This isn't ideal because it reports the maximum number of queues but
* does not provide info on how many can be uplink/downlink or different
@@ -369,6 +369,7 @@
dev_info->capabilities = bbdev_capabilities;
dev_info->cpu_flag_reqs = NULL;
dev_info->data_endianness = RTE_LITTLE_ENDIAN;
+ dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
/* Calculates number of queues assigned to device */
dev_info->max_num_queues = 0;
@@ -645,6 +645,7 @@ struct __rte_cache_aligned fpga_queue {
dev_info->capabilities = bbdev_capabilities;
dev_info->cpu_flag_reqs = NULL;
dev_info->data_endianness = RTE_LITTLE_ENDIAN;
+ dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
/* Calculates number of queues assigned to device */
dev_info->max_num_queues = 0;
@@ -100,6 +100,7 @@ struct bbdev_la12xx_params {
dev_info->capabilities = bbdev_capabilities;
dev_info->cpu_flag_reqs = NULL;
dev_info->min_alignment = 64;
+ dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
rte_bbdev_log_debug("got device info from %u", dev->data->dev_id);
}
@@ -82,6 +82,7 @@ struct bbdev_queue {
* here for code completeness.
*/
dev_info->data_endianness = RTE_LITTLE_ENDIAN;
+ dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
rte_bbdev_log_debug("got device info from %u", dev->data->dev_id);
}
@@ -254,6 +254,7 @@ struct turbo_sw_queue {
dev_info->min_alignment = 64;
dev_info->harq_buffer_size = 0;
dev_info->data_endianness = RTE_LITTLE_ENDIAN;
+ dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
rte_bbdev_log_debug("got device info from %u\n", dev->data->dev_id);
}
@@ -1133,3 +1133,25 @@ struct rte_mempool *
rte_bbdev_log(ERR, "Invalid operation type");
return NULL;
}
+
+const char *
+rte_bbdev_device_status_str(enum rte_bbdev_device_status status)
+{
+ static const char * const dev_sta_string[] = {
+ "RTE_BBDEV_DEV_NOSTATUS",
+ "RTE_BBDEV_DEV_NOT_SUPPORTED",
+ "RTE_BBDEV_DEV_RESET",
+ "RTE_BBDEV_DEV_CONFIGURED",
+ "RTE_BBDEV_DEV_ACTIVE",
+ "RTE_BBDEV_DEV_FATAL_ERR",
+ "RTE_BBDEV_DEV_RESTART_REQ",
+ "RTE_BBDEV_DEV_RECONFIG_REQ",
+ "RTE_BBDEV_DEV_CORRECT_ERR",
+ };
+
+ if (status < sizeof(dev_sta_string) / sizeof(char *))
+ return dev_sta_string[status];
+
+ rte_bbdev_log(ERR, "Invalid device status");
+ return NULL;
+}
@@ -223,6 +223,21 @@ struct rte_bbdev_queue_conf {
int
rte_bbdev_queue_stop(uint16_t dev_id, uint16_t queue_id);
+/**
+ * Flags indicate the status of the device
+ */
+enum rte_bbdev_device_status {
+ RTE_BBDEV_DEV_NOSTATUS, /**< Nothing being reported */
+ RTE_BBDEV_DEV_NOT_SUPPORTED, /**< Device status is not supported on the PMD */
+ RTE_BBDEV_DEV_RESET, /**< Device in reset and un-configured state */
+ RTE_BBDEV_DEV_CONFIGURED, /**< Device is configured and ready to use */
+ RTE_BBDEV_DEV_ACTIVE, /**< Device is configured and VF is being used */
+ RTE_BBDEV_DEV_FATAL_ERR, /**< Device has hit a fatal uncorrectable error */
+ RTE_BBDEV_DEV_RESTART_REQ, /**< Device requires application to restart */
+ RTE_BBDEV_DEV_RECONFIG_REQ, /**< Device requires application to reconfigure queues */
+ RTE_BBDEV_DEV_CORRECT_ERR, /**< Warning of a correctable error event happened */
+};
+
/** Device statistics. */
struct rte_bbdev_stats {
uint64_t enqueued_count; /**< Count of all operations enqueued */
@@ -285,12 +300,14 @@ struct rte_bbdev_driver_info {
/** Set if device supports per-queue interrupts */
bool queue_intr_supported;
/** Minimum alignment of buffers, in bytes */
- uint16_t min_alignment;
- /** HARQ memory available in kB */
+ /** Device Status */
+ enum rte_bbdev_device_status device_status;
uint32_t harq_buffer_size;
/** Byte endianness (RTE_BIG_ENDIAN/RTE_LITTLE_ENDIAN) supported
* for input/output data
*/
+ uint16_t min_alignment;
+ /** HARQ memory available in kB */
uint8_t data_endianness;
/** Default queue configuration used if none is supplied */
struct rte_bbdev_queue_conf default_queue_conf;
@@ -827,6 +844,20 @@ typedef void (*rte_bbdev_cb_fn)(uint16_t dev_id,
rte_bbdev_queue_intr_ctl(uint16_t dev_id, uint16_t queue_id, int epfd, int op,
void *data);
+/**
+ * Converts device status from enum to string
+ *
+ * @param status
+ * Device status as enum
+ *
+ * @returns
+ * Operation type as string or NULL if op_type is invalid
+ *
+ */
+__rte_experimental
+const char*
+rte_bbdev_device_status_str(enum rte_bbdev_device_status status);
+
#ifdef __cplusplus
}
#endif
@@ -39,3 +39,9 @@ DPDK_22 {
local: *;
};
+
+EXPERIMENTAL {
+ global:
+
+ rte_bbdev_device_status_str;
+};