Jason Wang
2021-Dec-08 06:29 UTC
[PATCH 5/7] vdpa: Add support for querying control virtqueue index
On Wed, Dec 8, 2021 at 2:07 PM Eli Cohen <elic at nvidia.com> wrote:> > On Wed, Dec 08, 2021 at 11:25:01AM +0800, Jason Wang wrote: > > On Tue, Dec 7, 2021 at 3:57 PM Eli Cohen <elic at nvidia.com> wrote: > > > > > > On Fri, Dec 03, 2021 at 10:35:30AM +0800, Jason Wang wrote: > > > > On Thu, Dec 2, 2021 at 3:58 AM Eli Cohen <elic at nvidia.com> wrote: > > > > > > > > > > Add netlink attribute and callback function to query the control VQ > > > > > index of a device. > > > > > > > > It's better to explain the motivation of this. Actually we can deduce > > > > it from the max_virtqueue_paris if I was not wrong. > > > Right, we can deduce it. But I think, let's let the kernel deduce it. > > > e.g., we can avoid the new get_ctrl_vq_idx() callback, but still return > > > the value to userspace with VDPA_ATTR_DEV_CTRL_VQ_IDX. > > > > > > > So it means the > > > > value varies depending on if VIRTIO_NET_F_MQ > > > I guest you meant VIRTIO_NET_F_CTRL_VQ > > > > is negotiated which can > > > > be changed after a query. This may probably confuse the userspace > > > > sometime. > > > > > > No sure I follow you on this. The CVQ index is dictated after the call > > > to _vdpa_register_device() and cannot change. Whether there is a CVQ or > > > isn't depends on VIRTIO_NET_F_CTRL_VQ but the index doesn't change. > > > > Looks not, according to the spec, consider we have a device with 2 > > queue pairs plus cvq. > > > > When MQ is negotiated, cvq index is 4. > > When MQ is not negotiated, cvq index is 2. > > > > When you say MQ negotiated, do you mean both ways or just offered by the > upstream driver?I meant MQ feature is enabled from the driver and accepted by the device.> > I think I saw different behavior and I am going to check again. > > BTW, could you send reference to where the specs refers to the index of > the control virtqueue?You may refer spec chapter 5.1.2: https://docs.oasis-open.org/virtio/virtio/v1.1/csprd01/virtio-v1.1-csprd01.html#x1-1960002 Thanks> > > Thanks > > > > > > > > > > > > > Thanks > > > > > > > > > > > > > > Signed-off-by: Eli Cohen <elic at nvidia.com> > > > > > --- > > > > > drivers/vdpa/vdpa.c | 18 ++++++++++++++++++ > > > > > include/linux/vdpa.h | 1 + > > > > > include/uapi/linux/vdpa.h | 1 + > > > > > 3 files changed, 20 insertions(+) > > > > > > > > > > diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c > > > > > index f06f949d5fa1..ca3ab0f46188 100644 > > > > > --- a/drivers/vdpa/vdpa.c > > > > > +++ b/drivers/vdpa/vdpa.c > > > > > @@ -712,6 +712,20 @@ static int vdpa_nl_cmd_dev_get_dumpit(struct sk_buff *msg, struct netlink_callba > > > > > return msg->len; > > > > > } > > > > > > > > > > +static int vdpa_dev_net_ctrl_vq_fill(struct vdpa_device *vdev, > > > > > + struct sk_buff *msg, u64 features) > > > > > +{ > > > > > + u16 val_u16 = 0; > > > > > + > > > > > + if (features & BIT_ULL(VIRTIO_NET_F_CTRL_VQ) && > > > > > + vdev->config->get_ctrl_vq_idx) { > > > > > + val_u16 = vdev->config->get_ctrl_vq_idx(vdev); > > > > > + return nla_put_u16(msg, VDPA_ATTR_DEV_CTRL_VQ_IDX, val_u16); > > > > > + } > > > > > + > > > > > + return 0; > > > > > +} > > > > > + > > > > > static int vdpa_dev_net_mq_config_fill(struct vdpa_device *vdev, > > > > > struct sk_buff *msg, u64 features, > > > > > const struct virtio_net_config *config) > > > > > @@ -730,6 +744,7 @@ static int vdpa_dev_net_config_fill(struct vdpa_device *vdev, struct sk_buff *ms > > > > > struct virtio_net_config config = {}; > > > > > u64 features; > > > > > u16 val_u16; > > > > > + int err; > > > > > > > > > > vdpa_get_config(vdev, 0, &config, sizeof(config)); > > > > > > > > > > @@ -746,6 +761,9 @@ static int vdpa_dev_net_config_fill(struct vdpa_device *vdev, struct sk_buff *ms > > > > > return -EMSGSIZE; > > > > > > > > > > features = vdev->config->get_features(vdev); > > > > > + err = vdpa_dev_net_ctrl_vq_fill(vdev, msg, features); > > > > > + if (err) > > > > > + return err; > > > > > > > > > > return vdpa_dev_net_mq_config_fill(vdev, msg, features, &config); > > > > > } > > > > > diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h > > > > > index 820621c59365..fca9bfeed9ba 100644 > > > > > --- a/include/linux/vdpa.h > > > > > +++ b/include/linux/vdpa.h > > > > > @@ -274,6 +274,7 @@ struct vdpa_config_ops { > > > > > (*get_vq_notification)(struct vdpa_device *vdev, u16 idx); > > > > > /* vq irq is not expected to be changed once DRIVER_OK is set */ > > > > > int (*get_vq_irq)(struct vdpa_device *vdev, u16 idx); > > > > > + u16 (*get_ctrl_vq_idx)(struct vdpa_device *vdev); > > > > > > > > > > /* Device ops */ > > > > > u32 (*get_vq_align)(struct vdpa_device *vdev); > > > > > diff --git a/include/uapi/linux/vdpa.h b/include/uapi/linux/vdpa.h > > > > > index a252f06f9dfd..2e3a7f89f42d 100644 > > > > > --- a/include/uapi/linux/vdpa.h > > > > > +++ b/include/uapi/linux/vdpa.h > > > > > @@ -34,6 +34,7 @@ enum vdpa_attr { > > > > > VDPA_ATTR_DEV_MAX_VQS, /* u32 */ > > > > > VDPA_ATTR_DEV_MAX_VQ_SIZE, /* u16 */ > > > > > VDPA_ATTR_DEV_MIN_VQ_SIZE, /* u16 */ > > > > > + VDPA_ATTR_DEV_CTRL_VQ_IDX, /* u16 */ > > > > > > > > > > VDPA_ATTR_DEV_NET_CFG_MACADDR, /* binary */ > > > > > VDPA_ATTR_DEV_NET_STATUS, /* u8 */ > > > > > -- > > > > > 2.33.1 > > > > > > > > > > > > > > >