[v2,3/8] vdpa/ifc: set max queues according to HW spec
Checks
Commit Message
Set max_queues according to virtio HW spec.
For virtio BLK device, set max_queues to the value of "num_queues".
"num_queues" is element of struct virtio_blk_config.
Signed-off-by: Andy Pei <andy.pei@intel.com>
Signed-off-by: Huang Wei <wei_huang@intel.com>
---
drivers/vdpa/ifc/base/ifcvf.h | 2 +-
drivers/vdpa/ifc/ifcvf_vdpa.c | 21 ++++++++++++++++++++-
2 files changed, 21 insertions(+), 2 deletions(-)
Comments
> -----Original Message-----
> From: Pei, Andy <andy.pei@intel.com>
> Sent: Thursday, September 8, 2022 1:54 PM
> To: dev@dpdk.org
> Cc: Xia, Chenbo <chenbo.xia@intel.com>; Xu, Rosen <rosen.xu@intel.com>;
> Huang, Wei <wei.huang@intel.com>; Cao, Gang <gang.cao@intel.com>;
> maxime.coquelin@redhat.com; Huang Wei <wei_huang@intel.com>
> Subject: [PATCH v2 3/8] vdpa/ifc: set max queues according to HW spec
vdpa/ifc: set max queues based on virtio spec
>
> Set max_queues according to virtio HW spec.
> For virtio BLK device, set max_queues to the value of "num_queues".
> "num_queues" is element of struct virtio_blk_config.
Both virtio-net/blk should be described.
>
> Signed-off-by: Andy Pei <andy.pei@intel.com>
> Signed-off-by: Huang Wei <wei_huang@intel.com>
Email is wrong, please fix all in next version
> ---
> drivers/vdpa/ifc/base/ifcvf.h | 2 +-
> drivers/vdpa/ifc/ifcvf_vdpa.c | 21 ++++++++++++++++++++-
> 2 files changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/vdpa/ifc/base/ifcvf.h b/drivers/vdpa/ifc/base/ifcvf.h
> index ad505f1..c17bf2a 100644
> --- a/drivers/vdpa/ifc/base/ifcvf.h
> +++ b/drivers/vdpa/ifc/base/ifcvf.h
> @@ -21,7 +21,7 @@
> #define IFCVF_NET_DEVICE_ID 0x0001
> #define IFCVF_BLK_DEVICE_ID 0x0002
>
> -#define IFCVF_MAX_QUEUES 1
> +#define IFCVF_MAX_QUEUES 32
>
> #ifndef VIRTIO_F_IOMMU_PLATFORM
> #define VIRTIO_F_IOMMU_PLATFORM 33
> diff --git a/drivers/vdpa/ifc/ifcvf_vdpa.c b/drivers/vdpa/ifc/ifcvf_vdpa.c
> index 2d165c0..2b42850 100644
> --- a/drivers/vdpa/ifc/ifcvf_vdpa.c
> +++ b/drivers/vdpa/ifc/ifcvf_vdpa.c
> @@ -26,6 +26,18 @@
>
> #include "base/ifcvf.h"
>
> +/*
> + * RTE_MAX() and RTE_MIN() cannot be used since braced-group within
> + * expression allowed only inside a function, but MAX() is used as
> + * a number of elements in array.
> + */
> +#ifndef MAX
> +#define MAX(v1, v2) ((v1) > (v2) ? (v1) : (v2))
> +#endif
> +#ifndef MIN
> +#define MIN(v1, v2) ((v1) < (v2) ? (v1) : (v2))
> +#endif
Above ifndef is not needed?
Seems MAX is not used, so remove it
> +
> RTE_LOG_REGISTER(ifcvf_vdpa_logtype, pmd.vdpa.ifcvf, NOTICE);
> #define DRV_LOG(level, fmt, args...) \
> rte_log(RTE_LOG_ ## level, ifcvf_vdpa_logtype, \
> @@ -1512,6 +1524,7 @@ struct rte_vdpa_dev_info dev_info[] = {
> uint64_t capacity = 0;
> uint8_t *byte;
> uint32_t i;
> + uint16_t queue_pairs;
>
> if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> return 0;
> @@ -1559,7 +1572,6 @@ struct rte_vdpa_dev_info dev_info[] = {
> }
>
> internal->configured = 0;
> - internal->max_queues = IFCVF_MAX_QUEUES;
> features = ifcvf_get_features(&internal->hw);
>
> device_id = ifcvf_pci_get_device_type(pci_dev);
> @@ -1570,6 +1582,10 @@ struct rte_vdpa_dev_info dev_info[] = {
>
> if (device_id == VIRTIO_ID_NET) {
> internal->hw.device_type = IFCVF_NET;
> + queue_pairs = (internal->hw.common_cfg->num_queues - 1) / 2;
Please note this logic assumes CTRL_VQ is always there, if for the read hardware,
that is the case, then it's fine. You can decide yourself to check CTRL_VQ feature
is there or not.
Thanks,
Chenbo
> + DRV_LOG(INFO, "%s support %u queue pairs", pci_dev->name,
> + queue_pairs);
> + internal->max_queues = MIN(IFCVF_MAX_QUEUES, queue_pairs);
> internal->features = features &
> ~(1ULL << VIRTIO_F_IOMMU_PLATFORM);
> internal->features |= dev_info[IFCVF_NET].features;
> @@ -1609,6 +1625,9 @@ struct rte_vdpa_dev_info dev_info[] = {
> internal->hw.blk_cfg->geometry.sectors);
> DRV_LOG(DEBUG, "num_queues: 0x%08x",
> internal->hw.blk_cfg->num_queues);
> +
> + internal->max_queues = MIN(IFCVF_MAX_QUEUES,
> + internal->hw.blk_cfg->num_queues);
> }
>
> list->internal = internal;
> --
> 1.8.3.1
Hi Chenbo,
Thanks for your reply.
My reply is inline.
> -----Original Message-----
> From: Xia, Chenbo <chenbo.xia@intel.com>
> Sent: Friday, September 9, 2022 1:56 PM
> To: Pei, Andy <andy.pei@intel.com>; dev@dpdk.org
> Cc: Xu, Rosen <rosen.xu@intel.com>; Huang, Wei <wei.huang@intel.com>; Cao,
> Gang <gang.cao@intel.com>; maxime.coquelin@redhat.com
> Subject: RE: [PATCH v2 3/8] vdpa/ifc: set max queues according to HW spec
>
> > -----Original Message-----
> > From: Pei, Andy <andy.pei@intel.com>
> > Sent: Thursday, September 8, 2022 1:54 PM
> > To: dev@dpdk.org
> > Cc: Xia, Chenbo <chenbo.xia@intel.com>; Xu, Rosen
> > <rosen.xu@intel.com>; Huang, Wei <wei.huang@intel.com>; Cao, Gang
> > <gang.cao@intel.com>; maxime.coquelin@redhat.com; Huang Wei
> > <wei_huang@intel.com>
> > Subject: [PATCH v2 3/8] vdpa/ifc: set max queues according to HW spec
>
> vdpa/ifc: set max queues based on virtio spec
>
OK
> >
> > Set max_queues according to virtio HW spec.
> > For virtio BLK device, set max_queues to the value of "num_queues".
> > "num_queues" is element of struct virtio_blk_config.
>
> Both virtio-net/blk should be described.
>
OK
> >
> > Signed-off-by: Andy Pei <andy.pei@intel.com>
> > Signed-off-by: Huang Wei <wei_huang@intel.com>
>
> Email is wrong, please fix all in next version
>
OK
> > ---
> > drivers/vdpa/ifc/base/ifcvf.h | 2 +- drivers/vdpa/ifc/ifcvf_vdpa.c
> > | 21 ++++++++++++++++++++-
> > 2 files changed, 21 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/vdpa/ifc/base/ifcvf.h
> > b/drivers/vdpa/ifc/base/ifcvf.h index ad505f1..c17bf2a 100644
> > --- a/drivers/vdpa/ifc/base/ifcvf.h
> > +++ b/drivers/vdpa/ifc/base/ifcvf.h
> > @@ -21,7 +21,7 @@
> > #define IFCVF_NET_DEVICE_ID 0x0001
> > #define IFCVF_BLK_DEVICE_ID 0x0002
> >
> > -#define IFCVF_MAX_QUEUES 1
> > +#define IFCVF_MAX_QUEUES 32
> >
> > #ifndef VIRTIO_F_IOMMU_PLATFORM
> > #define VIRTIO_F_IOMMU_PLATFORM 33
> > diff --git a/drivers/vdpa/ifc/ifcvf_vdpa.c
> > b/drivers/vdpa/ifc/ifcvf_vdpa.c index 2d165c0..2b42850 100644
> > --- a/drivers/vdpa/ifc/ifcvf_vdpa.c
> > +++ b/drivers/vdpa/ifc/ifcvf_vdpa.c
> > @@ -26,6 +26,18 @@
> >
> > #include "base/ifcvf.h"
> >
> > +/*
> > + * RTE_MAX() and RTE_MIN() cannot be used since braced-group within
> > + * expression allowed only inside a function, but MAX() is used as
> > + * a number of elements in array.
> > + */
> > +#ifndef MAX
> > +#define MAX(v1, v2) ((v1) > (v2) ? (v1) : (v2))
> > +#endif
> > +#ifndef MIN
> > +#define MIN(v1, v2) ((v1) < (v2) ? (v1) : (v2))
> > +#endif
>
> Above ifndef is not needed?
>
> Seems MAX is not used, so remove it
>
OK
> > +
> > RTE_LOG_REGISTER(ifcvf_vdpa_logtype, pmd.vdpa.ifcvf, NOTICE);
> > #define DRV_LOG(level, fmt, args...) \
> > rte_log(RTE_LOG_ ## level, ifcvf_vdpa_logtype, \ @@ -1512,6 +1524,7
> > @@ struct rte_vdpa_dev_info dev_info[] = {
> > uint64_t capacity = 0;
> > uint8_t *byte;
> > uint32_t i;
> > + uint16_t queue_pairs;
> >
> > if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> > return 0;
> > @@ -1559,7 +1572,6 @@ struct rte_vdpa_dev_info dev_info[] = {
> > }
> >
> > internal->configured = 0;
> > - internal->max_queues = IFCVF_MAX_QUEUES;
> > features = ifcvf_get_features(&internal->hw);
> >
> > device_id = ifcvf_pci_get_device_type(pci_dev);
> > @@ -1570,6 +1582,10 @@ struct rte_vdpa_dev_info dev_info[] = {
> >
> > if (device_id == VIRTIO_ID_NET) {
> > internal->hw.device_type = IFCVF_NET;
> > + queue_pairs = (internal->hw.common_cfg->num_queues - 1) / 2;
>
> Please note this logic assumes CTRL_VQ is always there, if for the read hardware,
> that is the case, then it's fine. You can decide yourself to check CTRL_VQ feature
> is there or not.
>
I will keep the code as it is now, and add some comments to explain ifc driver assumes CTRL_VQ is always there.
> Thanks,
> Chenbo
>
> > + DRV_LOG(INFO, "%s support %u queue pairs", pci_dev->name,
> > + queue_pairs);
> > + internal->max_queues = MIN(IFCVF_MAX_QUEUES,
> queue_pairs);
> > internal->features = features &
> > ~(1ULL <<
> VIRTIO_F_IOMMU_PLATFORM);
> > internal->features |= dev_info[IFCVF_NET].features; @@ -
> 1609,6
> > +1625,9 @@ struct rte_vdpa_dev_info dev_info[] = {
> > internal->hw.blk_cfg->geometry.sectors);
> > DRV_LOG(DEBUG, "num_queues: 0x%08x",
> > internal->hw.blk_cfg->num_queues);
> > +
> > + internal->max_queues = MIN(IFCVF_MAX_QUEUES,
> > + internal->hw.blk_cfg->num_queues);
> > }
> >
> > list->internal = internal;
> > --
> > 1.8.3.1
@@ -21,7 +21,7 @@
#define IFCVF_NET_DEVICE_ID 0x0001
#define IFCVF_BLK_DEVICE_ID 0x0002
-#define IFCVF_MAX_QUEUES 1
+#define IFCVF_MAX_QUEUES 32
#ifndef VIRTIO_F_IOMMU_PLATFORM
#define VIRTIO_F_IOMMU_PLATFORM 33
@@ -26,6 +26,18 @@
#include "base/ifcvf.h"
+/*
+ * RTE_MAX() and RTE_MIN() cannot be used since braced-group within
+ * expression allowed only inside a function, but MAX() is used as
+ * a number of elements in array.
+ */
+#ifndef MAX
+#define MAX(v1, v2) ((v1) > (v2) ? (v1) : (v2))
+#endif
+#ifndef MIN
+#define MIN(v1, v2) ((v1) < (v2) ? (v1) : (v2))
+#endif
+
RTE_LOG_REGISTER(ifcvf_vdpa_logtype, pmd.vdpa.ifcvf, NOTICE);
#define DRV_LOG(level, fmt, args...) \
rte_log(RTE_LOG_ ## level, ifcvf_vdpa_logtype, \
@@ -1512,6 +1524,7 @@ struct rte_vdpa_dev_info dev_info[] = {
uint64_t capacity = 0;
uint8_t *byte;
uint32_t i;
+ uint16_t queue_pairs;
if (rte_eal_process_type() != RTE_PROC_PRIMARY)
return 0;
@@ -1559,7 +1572,6 @@ struct rte_vdpa_dev_info dev_info[] = {
}
internal->configured = 0;
- internal->max_queues = IFCVF_MAX_QUEUES;
features = ifcvf_get_features(&internal->hw);
device_id = ifcvf_pci_get_device_type(pci_dev);
@@ -1570,6 +1582,10 @@ struct rte_vdpa_dev_info dev_info[] = {
if (device_id == VIRTIO_ID_NET) {
internal->hw.device_type = IFCVF_NET;
+ queue_pairs = (internal->hw.common_cfg->num_queues - 1) / 2;
+ DRV_LOG(INFO, "%s support %u queue pairs", pci_dev->name,
+ queue_pairs);
+ internal->max_queues = MIN(IFCVF_MAX_QUEUES, queue_pairs);
internal->features = features &
~(1ULL << VIRTIO_F_IOMMU_PLATFORM);
internal->features |= dev_info[IFCVF_NET].features;
@@ -1609,6 +1625,9 @@ struct rte_vdpa_dev_info dev_info[] = {
internal->hw.blk_cfg->geometry.sectors);
DRV_LOG(DEBUG, "num_queues: 0x%08x",
internal->hw.blk_cfg->num_queues);
+
+ internal->max_queues = MIN(IFCVF_MAX_QUEUES,
+ internal->hw.blk_cfg->num_queues);
}
list->internal = internal;