* Re: [PATCH 1/2] vdpa: Allow to configure max data virtqueues [not found] ` <20211130094838.15489-2-elic@nvidia.com> @ 2021-12-01 1:24 ` Si-Wei Liu [not found] ` <20211201100307.GA239524@mtl-vdi-166.wap.labs.mlnx> 0 siblings, 1 reply; 8+ messages in thread From: Si-Wei Liu @ 2021-12-01 1:24 UTC (permalink / raw) To: Eli Cohen, mst, jasowang, virtualization; +Cc: lvivier, eperezma On 11/30/2021 1:48 AM, Eli Cohen wrote: > Allow to configure the max virtqueues for a device. > > Signed-off-by: Eli Cohen <elic@nvidia.com> > --- > drivers/vdpa/vdpa.c | 16 +++++++++++++++- > include/linux/vdpa.h | 1 + > 2 files changed, 16 insertions(+), 1 deletion(-) > > diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c > index 7332a74a4b00..e185ec2ee851 100644 > --- a/drivers/vdpa/vdpa.c > +++ b/drivers/vdpa/vdpa.c > @@ -480,7 +480,8 @@ vdpa_nl_cmd_mgmtdev_get_dumpit(struct sk_buff *msg, struct netlink_callback *cb) > } > > #define VDPA_DEV_NET_ATTRS_MASK ((1 << VDPA_ATTR_DEV_NET_CFG_MACADDR) | \ > - (1 << VDPA_ATTR_DEV_NET_CFG_MTU)) > + (1 << VDPA_ATTR_DEV_NET_CFG_MTU) | \ > + (1 << VDPA_ATTR_DEV_NET_CFG_MAX_VQP)) It seems VDPA_ATTR_DEV_MAX_VQS (u32) is what you want (# of data virtqueues instead of # of data virtqueue pairs)? Not sure what's possible use of VDPA_ATTR_DEV_NET_CFG_MAX_VQP, was it to dump/display the config space max_virtqueue_pairs value (u16, 1-32768) for virtio-net? Why there's such quasi-duplicate attribute introduced in the first place? Not even sure VDPA_ATTR_DEV_MAX_VQS by definition should include other virtqueues as well: such as control virtqueue or event virtqueue. Hence the name will be more applicable to vdpa devices of other virtio type than just virtio-net. Otherwise I would think this attribute is slightly misnamed (max_data_vqs might be a proper name). > > static int vdpa_nl_cmd_dev_add_set_doit(struct sk_buff *skb, struct genl_info *info) > { > @@ -506,6 +507,19 @@ static int vdpa_nl_cmd_dev_add_set_doit(struct sk_buff *skb, struct genl_info *i > nla_get_u16(nl_attrs[VDPA_ATTR_DEV_NET_CFG_MTU]); > config.mask |= (1 << VDPA_ATTR_DEV_NET_CFG_MTU); > } > + if (nl_attrs[VDPA_ATTR_DEV_NET_CFG_MAX_VQP]) { > + config.max_virtqueues = nla_get_u16(nl_attrs[VDPA_ATTR_DEV_NET_CFG_MAX_VQP]); > + if (config.max_virtqueues < 2) { > + NL_SET_ERR_MSG_MOD(info->extack, "At least two virtqueues are required"); > + return -EINVAL; > + } > + if ((config.max_virtqueues - 1) & config.max_virtqueues) { > + NL_SET_ERR_MSG_MOD(info->extack, > + "Must provide power of two number of virtqueues"); Why there's such limitation for the number of vDPA virtqueues? I thought the software virtio doesn't have this limitation (power of two). -Siwei > + return -EINVAL; > + } > + config.mask |= BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MAX_VQP); > + } > > /* Skip checking capability if user didn't prefer to configure any > * device networking attributes. It is likely that user might have used > diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h > index c3011ccda430..2f0b09c6d1ae 100644 > --- a/include/linux/vdpa.h > +++ b/include/linux/vdpa.h > @@ -101,6 +101,7 @@ struct vdpa_dev_set_config { > u16 mtu; > } net; > u64 mask; > + u16 max_virtqueues; > }; > > /** _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <20211201100307.GA239524@mtl-vdi-166.wap.labs.mlnx>]
* RE: [PATCH 1/2] vdpa: Allow to configure max data virtqueues [not found] ` <20211201100307.GA239524@mtl-vdi-166.wap.labs.mlnx> @ 2021-12-01 10:09 ` Parav Pandit via Virtualization [not found] ` <20211201115838.GA3465@mtl-vdi-166.wap.labs.mlnx> 2021-12-02 3:40 ` Jason Wang 2021-12-01 19:40 ` Si-Wei Liu 2021-12-01 19:46 ` Si-Wei Liu 2 siblings, 2 replies; 8+ messages in thread From: Parav Pandit via Virtualization @ 2021-12-01 10:09 UTC (permalink / raw) To: Eli Cohen, Si-Wei Liu; +Cc: lvivier, mst, virtualization, eperezma > From: Eli Cohen <elic@nvidia.com> > Sent: Wednesday, December 1, 2021 3:33 PM > > On Tue, Nov 30, 2021 at 05:24:03PM -0800, Si-Wei Liu wrote: > > > > > > On 11/30/2021 1:48 AM, Eli Cohen wrote: > > > Allow to configure the max virtqueues for a device. > > > > > > Signed-off-by: Eli Cohen <elic@nvidia.com> > > > --- > > > drivers/vdpa/vdpa.c | 16 +++++++++++++++- > > > include/linux/vdpa.h | 1 + > > > 2 files changed, 16 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c index > > > 7332a74a4b00..e185ec2ee851 100644 > > > --- a/drivers/vdpa/vdpa.c > > > +++ b/drivers/vdpa/vdpa.c > > > @@ -480,7 +480,8 @@ vdpa_nl_cmd_mgmtdev_get_dumpit(struct sk_buff > *msg, struct netlink_callback *cb) > > > } > > > #define VDPA_DEV_NET_ATTRS_MASK ((1 << > VDPA_ATTR_DEV_NET_CFG_MACADDR) | \ > > > - (1 << VDPA_ATTR_DEV_NET_CFG_MTU)) > > > + (1 << VDPA_ATTR_DEV_NET_CFG_MTU) | \ > > > + (1 << VDPA_ATTR_DEV_NET_CFG_MAX_VQP)) > > It seems VDPA_ATTR_DEV_MAX_VQS (u32) is what you want (# of data > > virtqueues instead of # of data virtqueue pairs)? Not sure what's > > possible use of VDPA_ATTR_DEV_NET_CFG_MAX_VQP, was it to > dump/display > > the config space max_virtqueue_pairs value (u16, 1-32768) for > > virtio-net? Why there's such quasi-duplicate attribute introduced in the first > place? > > > > VDPA_ATTR_DEV_MAX_VQS currently returns vdev->nvqs which equals > whatever passed to _vdpa_register_device(). The latter depends on the value > provided by (struct vdpa_dev_set_config).max_virtqueues. > Max VQs configuration should reuse VDPA_ATTR_DEV_MAX_VQS. it indicates what is the max vqs a given vdpa device is using. Until now it was driver's choice, now its users choice if provided. So no need for additional attribute. > Maybe we should add attributes to add aditional virtqueues like control > virtqueue and their index. They could be returned by > vdpa_dev_net_config_fill(). Yes. _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <20211201115838.GA3465@mtl-vdi-166.wap.labs.mlnx>]
* RE: [PATCH 1/2] vdpa: Allow to configure max data virtqueues [not found] ` <20211201115838.GA3465@mtl-vdi-166.wap.labs.mlnx> @ 2021-12-01 12:28 ` Parav Pandit via Virtualization 0 siblings, 0 replies; 8+ messages in thread From: Parav Pandit via Virtualization @ 2021-12-01 12:28 UTC (permalink / raw) To: Eli Cohen; +Cc: lvivier, mst, virtualization, eperezma, Si-Wei Liu > From: Eli Cohen <elic@nvidia.com> > Sent: Wednesday, December 1, 2021 5:29 PM > > On Wed, Dec 01, 2021 at 12:09:17PM +0200, Parav Pandit wrote: > > > > > > > From: Eli Cohen <elic@nvidia.com> > > > Sent: Wednesday, December 1, 2021 3:33 PM > > > > > > On Tue, Nov 30, 2021 at 05:24:03PM -0800, Si-Wei Liu wrote: > > > > > > > > > > > > On 11/30/2021 1:48 AM, Eli Cohen wrote: > > > > > Allow to configure the max virtqueues for a device. > > > > > > > > > > Signed-off-by: Eli Cohen <elic@nvidia.com> > > > > > --- > > > > > drivers/vdpa/vdpa.c | 16 +++++++++++++++- > > > > > include/linux/vdpa.h | 1 + > > > > > 2 files changed, 16 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c index > > > > > 7332a74a4b00..e185ec2ee851 100644 > > > > > --- a/drivers/vdpa/vdpa.c > > > > > +++ b/drivers/vdpa/vdpa.c > > > > > @@ -480,7 +480,8 @@ vdpa_nl_cmd_mgmtdev_get_dumpit(struct > > > > > sk_buff > > > *msg, struct netlink_callback *cb) > > > > > } > > > > > #define VDPA_DEV_NET_ATTRS_MASK ((1 << > > > VDPA_ATTR_DEV_NET_CFG_MACADDR) | \ > > > > > - (1 << > VDPA_ATTR_DEV_NET_CFG_MTU)) > > > > > + (1 << > VDPA_ATTR_DEV_NET_CFG_MTU) | \ > > > > > + (1 << > VDPA_ATTR_DEV_NET_CFG_MAX_VQP)) > > > > It seems VDPA_ATTR_DEV_MAX_VQS (u32) is what you want (# of data > > > > virtqueues instead of # of data virtqueue pairs)? Not sure what's > > > > possible use of VDPA_ATTR_DEV_NET_CFG_MAX_VQP, was it to > > > dump/display > > > > the config space max_virtqueue_pairs value (u16, 1-32768) for > > > > virtio-net? Why there's such quasi-duplicate attribute introduced > > > > in the first > > > place? > > > > > > > > > > VDPA_ATTR_DEV_MAX_VQS currently returns vdev->nvqs which equals > > > whatever passed to _vdpa_register_device(). The latter depends on > > > the value provided by (struct vdpa_dev_set_config).max_virtqueues. > > > > > But currently VDPA_ATTR_DEV_MAX_VQS returns the number of used queues, No. it doesn't. It reports nvqs passed in _vdpa_register_device(). And this API has clear definition of nvqs = number of virtqueues supported by this device nvqs != number of used vqs. So current MAX_VQS is doing its job right. For example nvqs can be be 8 and guest VM driver may just use two VQs (1 tx, 1 rx, and used vqs = 2). _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] vdpa: Allow to configure max data virtqueues 2021-12-01 10:09 ` Parav Pandit via Virtualization [not found] ` <20211201115838.GA3465@mtl-vdi-166.wap.labs.mlnx> @ 2021-12-02 3:40 ` Jason Wang 2021-12-02 3:42 ` Parav Pandit via Virtualization 1 sibling, 1 reply; 8+ messages in thread From: Jason Wang @ 2021-12-02 3:40 UTC (permalink / raw) To: Parav Pandit, Eli Cohen, Si-Wei Liu Cc: lvivier, eperezma, virtualization, mst 在 2021/12/1 下午6:09, Parav Pandit 写道: > >> From: Eli Cohen <elic@nvidia.com> >> Sent: Wednesday, December 1, 2021 3:33 PM >> >> On Tue, Nov 30, 2021 at 05:24:03PM -0800, Si-Wei Liu wrote: >>> >>> On 11/30/2021 1:48 AM, Eli Cohen wrote: >>>> Allow to configure the max virtqueues for a device. >>>> >>>> Signed-off-by: Eli Cohen <elic@nvidia.com> >>>> --- >>>> drivers/vdpa/vdpa.c | 16 +++++++++++++++- >>>> include/linux/vdpa.h | 1 + >>>> 2 files changed, 16 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c index >>>> 7332a74a4b00..e185ec2ee851 100644 >>>> --- a/drivers/vdpa/vdpa.c >>>> +++ b/drivers/vdpa/vdpa.c >>>> @@ -480,7 +480,8 @@ vdpa_nl_cmd_mgmtdev_get_dumpit(struct sk_buff >> *msg, struct netlink_callback *cb) >>>> } >>>> #define VDPA_DEV_NET_ATTRS_MASK ((1 << >> VDPA_ATTR_DEV_NET_CFG_MACADDR) | \ >>>> - (1 << VDPA_ATTR_DEV_NET_CFG_MTU)) >>>> + (1 << VDPA_ATTR_DEV_NET_CFG_MTU) | \ >>>> + (1 << VDPA_ATTR_DEV_NET_CFG_MAX_VQP)) >>> It seems VDPA_ATTR_DEV_MAX_VQS (u32) is what you want (# of data >>> virtqueues instead of # of data virtqueue pairs)? Not sure what's >>> possible use of VDPA_ATTR_DEV_NET_CFG_MAX_VQP, was it to >> dump/display >>> the config space max_virtqueue_pairs value (u16, 1-32768) for >>> virtio-net? Why there's such quasi-duplicate attribute introduced in the first >> place? >> VDPA_ATTR_DEV_MAX_VQS currently returns vdev->nvqs which equals >> whatever passed to _vdpa_register_device(). The latter depends on the value >> provided by (struct vdpa_dev_set_config).max_virtqueues. >> > Max VQs configuration should reuse VDPA_ATTR_DEV_MAX_VQS. > it indicates what is the max vqs a given vdpa device is using. Until now it was driver's choice, now its users choice if provided. > So no need for additional attribute. I think a device specific attribute might be better: 1) max_virt_queue_pairs is part of config space so it should work as mtu and mac 2) it's more convenient for the user to specifc qps instead of doing 2*N+1 calculation Thanks > >> Maybe we should add attributes to add aditional virtqueues like control >> virtqueue and their index. They could be returned by >> vdpa_dev_net_config_fill(). > Yes. > _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH 1/2] vdpa: Allow to configure max data virtqueues 2021-12-02 3:40 ` Jason Wang @ 2021-12-02 3:42 ` Parav Pandit via Virtualization 0 siblings, 0 replies; 8+ messages in thread From: Parav Pandit via Virtualization @ 2021-12-02 3:42 UTC (permalink / raw) To: Jason Wang, Eli Cohen, Si-Wei Liu; +Cc: lvivier, eperezma, virtualization, mst > From: Jason Wang <jasowang@redhat.com> > Sent: Thursday, December 2, 2021 9:11 AM > > 在 2021/12/1 下午6:09, Parav Pandit 写道: > > > >> From: Eli Cohen <elic@nvidia.com> > >> Sent: Wednesday, December 1, 2021 3:33 PM > >> > >> On Tue, Nov 30, 2021 at 05:24:03PM -0800, Si-Wei Liu wrote: > >>> > >>> On 11/30/2021 1:48 AM, Eli Cohen wrote: > >>>> Allow to configure the max virtqueues for a device. > >>>> > >>>> Signed-off-by: Eli Cohen <elic@nvidia.com> > >>>> --- > >>>> drivers/vdpa/vdpa.c | 16 +++++++++++++++- > >>>> include/linux/vdpa.h | 1 + > >>>> 2 files changed, 16 insertions(+), 1 deletion(-) > >>>> > >>>> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c index > >>>> 7332a74a4b00..e185ec2ee851 100644 > >>>> --- a/drivers/vdpa/vdpa.c > >>>> +++ b/drivers/vdpa/vdpa.c > >>>> @@ -480,7 +480,8 @@ vdpa_nl_cmd_mgmtdev_get_dumpit(struct > sk_buff > >> *msg, struct netlink_callback *cb) > >>>> } > >>>> #define VDPA_DEV_NET_ATTRS_MASK ((1 << > >> VDPA_ATTR_DEV_NET_CFG_MACADDR) | \ > >>>> - (1 << VDPA_ATTR_DEV_NET_CFG_MTU)) > >>>> + (1 << VDPA_ATTR_DEV_NET_CFG_MTU) | \ > >>>> + (1 << > VDPA_ATTR_DEV_NET_CFG_MAX_VQP)) > >>> It seems VDPA_ATTR_DEV_MAX_VQS (u32) is what you want (# of data > >>> virtqueues instead of # of data virtqueue pairs)? Not sure what's > >>> possible use of VDPA_ATTR_DEV_NET_CFG_MAX_VQP, was it to > >> dump/display > >>> the config space max_virtqueue_pairs value (u16, 1-32768) for > >>> virtio-net? Why there's such quasi-duplicate attribute introduced in > >>> the first > >> place? > >> VDPA_ATTR_DEV_MAX_VQS currently returns vdev->nvqs which equals > >> whatever passed to _vdpa_register_device(). The latter depends on the > >> value provided by (struct vdpa_dev_set_config).max_virtqueues. > >> > > Max VQs configuration should reuse VDPA_ATTR_DEV_MAX_VQS. > > it indicates what is the max vqs a given vdpa device is using. Until now it was > driver's choice, now its users choice if provided. > > So no need for additional attribute. > > > I think a device specific attribute might be better: > > 1) max_virt_queue_pairs is part of config space so it should work as mtu and > mac > > 2) it's more convenient for the user to specifc qps instead of doing > 2*N+1 calculation > Yes. Eli's new series already took care of this by reusing existing attribute. _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] vdpa: Allow to configure max data virtqueues [not found] ` <20211201100307.GA239524@mtl-vdi-166.wap.labs.mlnx> 2021-12-01 10:09 ` Parav Pandit via Virtualization @ 2021-12-01 19:40 ` Si-Wei Liu 2021-12-01 19:46 ` Si-Wei Liu 2 siblings, 0 replies; 8+ messages in thread From: Si-Wei Liu @ 2021-12-01 19:40 UTC (permalink / raw) To: Eli Cohen; +Cc: lvivier, mst, virtualization, eperezma On 12/1/2021 2:03 AM, Eli Cohen wrote: > On Tue, Nov 30, 2021 at 05:24:03PM -0800, Si-Wei Liu wrote: >> >> On 11/30/2021 1:48 AM, Eli Cohen wrote: >>> Allow to configure the max virtqueues for a device. >>> >>> Signed-off-by: Eli Cohen <elic@nvidia.com> >>> --- >>> drivers/vdpa/vdpa.c | 16 +++++++++++++++- >>> include/linux/vdpa.h | 1 + >>> 2 files changed, 16 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c >>> index 7332a74a4b00..e185ec2ee851 100644 >>> --- a/drivers/vdpa/vdpa.c >>> +++ b/drivers/vdpa/vdpa.c >>> @@ -480,7 +480,8 @@ vdpa_nl_cmd_mgmtdev_get_dumpit(struct sk_buff *msg, struct netlink_callback *cb) >>> } >>> #define VDPA_DEV_NET_ATTRS_MASK ((1 << VDPA_ATTR_DEV_NET_CFG_MACADDR) | \ >>> - (1 << VDPA_ATTR_DEV_NET_CFG_MTU)) >>> + (1 << VDPA_ATTR_DEV_NET_CFG_MTU) | \ >>> + (1 << VDPA_ATTR_DEV_NET_CFG_MAX_VQP)) >> It seems VDPA_ATTR_DEV_MAX_VQS (u32) is what you want (# of data virtqueues >> instead of # of data virtqueue pairs)? Not sure what's possible use of >> VDPA_ATTR_DEV_NET_CFG_MAX_VQP, was it to dump/display the config space >> max_virtqueue_pairs value (u16, 1-32768) for virtio-net? Why there's such >> quasi-duplicate attribute introduced in the first place? >> > VDPA_ATTR_DEV_MAX_VQS currently returns vdev->nvqs which equals whatever > passed to _vdpa_register_device(). The latter depends on the value > provided by (struct vdpa_dev_set_config).max_virtqueues. It's the only > way to let the user know if there are other virtqueues besides the data > virtqueues. For example, if we support control virtqueue and configured > 2 data QPs, we will return 5. That's right. Then let VDPA_ATTR_DEV_MAX_VQS continue to be read-only as is. > > Maybe we should add attributes to add aditional virtqueues like control > virtqueue and their index. They could be returned by > vdpa_dev_net_config_fill(). Probably this info would need to return to QEMU for proper virtq modeling via vDPA ioctls rather than just netlink API. Agreed it's virtio-net specific config. > VDPA_ATTR_DEV_NET_CFG_MAX_VQP seems the right choice since it is used to > provid the requested number of data virtqueues when creating the device. > Maybe we need to change the name to VDPA_ATTR_DEV_NET_CFG_MAX_VQ and > then this patch makes more sense. I would just keep the name for VDPA_ATTR_DEV_NET_CFG_MAX_VQP and have user configure the number of data queue pairs instead. Very few virtio types come with virtqueue in pairs, so this has to be vdpa net specific config. Thanks, -Siwei > > > I will post another series to address all these issues. > >> Not even sure VDPA_ATTR_DEV_MAX_VQS by definition should include other >> virtqueues as well: such as control virtqueue or event virtqueue. Hence the >> name will be more applicable to vdpa devices of other virtio type than just >> virtio-net. Otherwise I would think this attribute is slightly misnamed >> (max_data_vqs might be a proper name). >> >>> static int vdpa_nl_cmd_dev_add_set_doit(struct sk_buff *skb, struct genl_info *info) >>> { >>> @@ -506,6 +507,19 @@ static int vdpa_nl_cmd_dev_add_set_doit(struct sk_buff *skb, struct genl_info *i >>> nla_get_u16(nl_attrs[VDPA_ATTR_DEV_NET_CFG_MTU]); >>> config.mask |= (1 << VDPA_ATTR_DEV_NET_CFG_MTU); >>> } >>> + if (nl_attrs[VDPA_ATTR_DEV_NET_CFG_MAX_VQP]) { >>> + config.max_virtqueues = nla_get_u16(nl_attrs[VDPA_ATTR_DEV_NET_CFG_MAX_VQP]); >>> + if (config.max_virtqueues < 2) { >>> + NL_SET_ERR_MSG_MOD(info->extack, "At least two virtqueues are required"); >>> + return -EINVAL; >>> + } >>> + if ((config.max_virtqueues - 1) & config.max_virtqueues) { >>> + NL_SET_ERR_MSG_MOD(info->extack, >>> + "Must provide power of two number of virtqueues"); >> Why there's such limitation for the number of vDPA virtqueues? I thought the >> software virtio doesn't have this limitation (power of two). > Right, the limitation comes from mlx5_vdpa. I will post a patch later to > remove that limitation. > >> -Siwei >> >>> + return -EINVAL; >>> + } >>> + config.mask |= BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MAX_VQP); >>> + } >>> /* Skip checking capability if user didn't prefer to configure any >>> * device networking attributes. It is likely that user might have used >>> diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h >>> index c3011ccda430..2f0b09c6d1ae 100644 >>> --- a/include/linux/vdpa.h >>> +++ b/include/linux/vdpa.h >>> @@ -101,6 +101,7 @@ struct vdpa_dev_set_config { >>> u16 mtu; >>> } net; >>> u64 mask; >>> + u16 max_virtqueues; >>> }; >>> /** _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] vdpa: Allow to configure max data virtqueues [not found] ` <20211201100307.GA239524@mtl-vdi-166.wap.labs.mlnx> 2021-12-01 10:09 ` Parav Pandit via Virtualization 2021-12-01 19:40 ` Si-Wei Liu @ 2021-12-01 19:46 ` Si-Wei Liu 2 siblings, 0 replies; 8+ messages in thread From: Si-Wei Liu @ 2021-12-01 19:46 UTC (permalink / raw) To: Eli Cohen; +Cc: lvivier, mst, virtualization, eperezma [-- Attachment #1.1: Type: text/plain, Size: 568 bytes --] On 12/1/2021 2:03 AM, Eli Cohen wrote: >>> + if ((config.max_virtqueues - 1) & config.max_virtqueues) { >>> + NL_SET_ERR_MSG_MOD(info->extack, >>> + "Must provide power of two number of virtqueues"); >> Why there's such limitation for the number of vDPA virtqueues? I thought the >> software virtio doesn't have this limitation (power of two). > Right, the limitation comes from mlx5_vdpa. I will post a patch later to > remove that limitation. > I mean the check should not live here. Other vdpa vendor device/driver may not have this limitation. -Siwei [-- Attachment #1.2: Type: text/html, Size: 1183 bytes --] [-- Attachment #2: Type: text/plain, Size: 183 bytes --] _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <20211130094838.15489-3-elic@nvidia.com>]
* Re: [PATCH 2/2] vdpa/mlx5: Support configuring max data virtqueues [not found] ` <20211130094838.15489-3-elic@nvidia.com> @ 2021-12-01 1:43 ` Si-Wei Liu 0 siblings, 0 replies; 8+ messages in thread From: Si-Wei Liu @ 2021-12-01 1:43 UTC (permalink / raw) To: Eli Cohen, mst, jasowang, virtualization; +Cc: lvivier, eperezma On 11/30/2021 1:48 AM, Eli Cohen wrote: > Check if the required number of data virtqueues was provided when a > adding a new device and verify the new value does not exceed device > capabilities. > > In addition, change the arrays holding virtqueue and callback contexts > to be dynamically allocated. > > Signed-off-by: Eli Cohen <elic@nvidia.com> > --- > drivers/vdpa/mlx5/net/mlx5_vnet.c | 32 +++++++++++++++++++------------ > 1 file changed, 20 insertions(+), 12 deletions(-) > > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c > index 63813fbb5f62..54e276e0df18 100644 > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c > @@ -131,11 +131,6 @@ struct mlx5_vdpa_virtqueue { > struct mlx5_vq_restore_info ri; > }; > > -/* We will remove this limitation once mlx5_vdpa_alloc_resources() > - * provides for driver space allocation > - */ > -#define MLX5_MAX_SUPPORTED_VQS 16 > - > static bool is_index_valid(struct mlx5_vdpa_dev *mvdev, u16 idx) > { > if (unlikely(idx > mvdev->max_idx)) > @@ -148,8 +143,8 @@ struct mlx5_vdpa_net { > struct mlx5_vdpa_dev mvdev; > struct mlx5_vdpa_net_resources res; > struct virtio_net_config config; > - struct mlx5_vdpa_virtqueue vqs[MLX5_MAX_SUPPORTED_VQS]; > - struct vdpa_callback event_cbs[MLX5_MAX_SUPPORTED_VQS + 1]; > + struct mlx5_vdpa_virtqueue *vqs; > + struct vdpa_callback *event_cbs; > > /* Serialize vq resources creation and destruction. This is required > * since memory map might change and we need to destroy and create > @@ -1218,7 +1213,7 @@ static void suspend_vqs(struct mlx5_vdpa_net *ndev) > { > int i; > > - for (i = 0; i < MLX5_MAX_SUPPORTED_VQS; i++) > + for (i = 0; i < ndev->mvdev.max_vqs; i++) > suspend_vq(ndev, &ndev->vqs[i]); > } > > @@ -1245,7 +1240,7 @@ static int create_rqt(struct mlx5_vdpa_net *ndev) > int i, j; > int err; > > - max_rqt = min_t(int, MLX5_MAX_SUPPORTED_VQS / 2, > + max_rqt = min_t(int, ndev->mvdev.max_vqs / 2, > 1 << MLX5_CAP_GEN(ndev->mvdev.mdev, log_max_rqt_size)); Here the user configured number of data vqs may get silently capped to what the device can support (up to 2^log_max_rqt_size). Do you consider expose this device specific capacity to vdpa users so it doesn't get blindly misconfigured, and/or return appropriate error message to indicate the failure cause? If mlx5 vdpa really can't support power of 2 number of data vqs due to hardware limitation (?), an appropriate message should be returned here. -Siwei > if (max_rqt < 1) > return -EOPNOTSUPP; > @@ -2235,7 +2230,7 @@ static int mlx5_vdpa_reset(struct vdpa_device *vdev) > clear_vqs_ready(ndev); > mlx5_vdpa_destroy_mr(&ndev->mvdev); > ndev->mvdev.status = 0; > - memset(ndev->event_cbs, 0, sizeof(ndev->event_cbs)); > + memset(ndev->event_cbs, 0, sizeof(*ndev->event_cbs) * (mvdev->max_vqs + 1)); > ndev->mvdev.actual_features = 0; > ++mvdev->generation; > if (MLX5_CAP_GEN(mvdev->mdev, umem_uid_0)) { > @@ -2308,6 +2303,8 @@ static void mlx5_vdpa_free(struct vdpa_device *vdev) > } > mlx5_vdpa_free_resources(&ndev->mvdev); > mutex_destroy(&ndev->reslock); > + kfree(ndev->event_cbs); > + kfree(ndev->vqs); > } > > static struct vdpa_notification_area mlx5_get_vq_notification(struct vdpa_device *vdev, u16 idx) > @@ -2547,13 +2544,23 @@ static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name, > > /* we save one virtqueue for control virtqueue should we require it */ > max_vqs = MLX5_CAP_DEV_VDPA_EMULATION(mdev, max_num_virtio_queues); > - max_vqs = min_t(u32, max_vqs, MLX5_MAX_SUPPORTED_VQS); > + if (add_config->mask & BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MAX_VQP)) { > + if (add_config->max_virtqueues > max_vqs) > + return -EINVAL; > + max_vqs = min_t(u32, max_vqs, add_config->max_virtqueues); > + } > > ndev = vdpa_alloc_device(struct mlx5_vdpa_net, mvdev.vdev, mdev->device, &mlx5_vdpa_ops, > name, false); > if (IS_ERR(ndev)) > return PTR_ERR(ndev); > > + ndev->vqs = kcalloc(max_vqs, sizeof(*ndev->vqs), GFP_KERNEL); > + ndev->event_cbs = kcalloc(max_vqs + 1, sizeof(*ndev->event_cbs), GFP_KERNEL); > + if (!ndev->vqs || !ndev->event_cbs) { > + err = -ENOMEM; > + goto err_mtu; > + } > ndev->mvdev.max_vqs = max_vqs; > mvdev = &ndev->mvdev; > mvdev->mdev = mdev; > @@ -2676,7 +2683,8 @@ static int mlx5v_probe(struct auxiliary_device *adev, > mgtdev->mgtdev.ops = &mdev_ops; > mgtdev->mgtdev.device = mdev->device; > mgtdev->mgtdev.id_table = id_table; > - mgtdev->mgtdev.config_attr_mask = (1 << VDPA_ATTR_DEV_NET_CFG_MACADDR); > + mgtdev->mgtdev.config_attr_mask = BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MACADDR) | > + BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MAX_VQP); > mgtdev->madev = madev; > > err = vdpa_mgmtdev_register(&mgtdev->mgtdev); _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-12-02 3:42 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20211130094838.15489-1-elic@nvidia.com> [not found] ` <20211130094838.15489-2-elic@nvidia.com> 2021-12-01 1:24 ` [PATCH 1/2] vdpa: Allow to configure max data virtqueues Si-Wei Liu [not found] ` <20211201100307.GA239524@mtl-vdi-166.wap.labs.mlnx> 2021-12-01 10:09 ` Parav Pandit via Virtualization [not found] ` <20211201115838.GA3465@mtl-vdi-166.wap.labs.mlnx> 2021-12-01 12:28 ` Parav Pandit via Virtualization 2021-12-02 3:40 ` Jason Wang 2021-12-02 3:42 ` Parav Pandit via Virtualization 2021-12-01 19:40 ` Si-Wei Liu 2021-12-01 19:46 ` Si-Wei Liu [not found] ` <20211130094838.15489-3-elic@nvidia.com> 2021-12-01 1:43 ` [PATCH 2/2] vdpa/mlx5: Support configuring " Si-Wei Liu
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.