| Message ID | 20210705071910.31965-2-jasowang@redhat.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [1/2] vdpa: support per virtqueue max queue size | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Mon, Jul 05, 2021 at 03:19:10PM +0800, Jason Wang wrote: > This patch switch to read virtqueue size from the capability instead > of depending on the hardcoded value. This allows the per virtqueue > size could be advertised. > > Signed-off-by: Jason Wang <jasowang@redhat.com> So let's add an ioctl for this? It's really a bug we don't.. > --- > drivers/vdpa/virtio_pci/vp_vdpa.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/vdpa/virtio_pci/vp_vdpa.c b/drivers/vdpa/virtio_pci/vp_vdpa.c > index 2926641fb586..198f7076e4d9 100644 > --- a/drivers/vdpa/virtio_pci/vp_vdpa.c > +++ b/drivers/vdpa/virtio_pci/vp_vdpa.c > @@ -18,7 +18,6 @@ > #include <linux/virtio_pci.h> > #include <linux/virtio_pci_modern.h> > > -#define VP_VDPA_QUEUE_MAX 256 > #define VP_VDPA_DRIVER_NAME "vp_vdpa" > #define VP_VDPA_NAME_SIZE 256 > > @@ -197,7 +196,10 @@ static void vp_vdpa_set_status(struct vdpa_device *vdpa, u8 status) > > static u16 vp_vdpa_get_vq_num_max(struct vdpa_device *vdpa, u16 qid) > { > - return VP_VDPA_QUEUE_MAX; > + struct vp_vdpa *vp_vdpa = vdpa_to_vp(vdpa); > + struct virtio_pci_modern_device *mdev = &vp_vdpa->mdev; > + > + return vp_modern_get_queue_size(mdev, qid); > } > > static int vp_vdpa_get_vq_state(struct vdpa_device *vdpa, u16 qid, > -- > 2.25.1
在 2021/7/5 下午3:26, Michael S. Tsirkin 写道: > On Mon, Jul 05, 2021 at 03:19:10PM +0800, Jason Wang wrote: >> This patch switch to read virtqueue size from the capability instead >> of depending on the hardcoded value. This allows the per virtqueue >> size could be advertised. >> >> Signed-off-by: Jason Wang <jasowang@redhat.com> > So let's add an ioctl for this? It's really a bug we don't.. As explained in patch 1. Qemu doesn't use VHOST_VDPA_GET_VRING_NUM actually. Instead it checks the result VHOST_VDPA_SET_VRING_NUM. So I change VHOST_VDPA_GET_VRING_NUM to return the minimal size of all the virtqueues. If you wish we can add a VHOST_VDPA_GET_VRING_NUM2, but I'm not sure it will have a user or not. Thanks > >> --- >> drivers/vdpa/virtio_pci/vp_vdpa.c | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/vdpa/virtio_pci/vp_vdpa.c b/drivers/vdpa/virtio_pci/vp_vdpa.c >> index 2926641fb586..198f7076e4d9 100644 >> --- a/drivers/vdpa/virtio_pci/vp_vdpa.c >> +++ b/drivers/vdpa/virtio_pci/vp_vdpa.c >> @@ -18,7 +18,6 @@ >> #include <linux/virtio_pci.h> >> #include <linux/virtio_pci_modern.h> >> >> -#define VP_VDPA_QUEUE_MAX 256 >> #define VP_VDPA_DRIVER_NAME "vp_vdpa" >> #define VP_VDPA_NAME_SIZE 256 >> >> @@ -197,7 +196,10 @@ static void vp_vdpa_set_status(struct vdpa_device *vdpa, u8 status) >> >> static u16 vp_vdpa_get_vq_num_max(struct vdpa_device *vdpa, u16 qid) >> { >> - return VP_VDPA_QUEUE_MAX; >> + struct vp_vdpa *vp_vdpa = vdpa_to_vp(vdpa); >> + struct virtio_pci_modern_device *mdev = &vp_vdpa->mdev; >> + >> + return vp_modern_get_queue_size(mdev, qid); >> } >> >> static int vp_vdpa_get_vq_state(struct vdpa_device *vdpa, u16 qid, >> -- >> 2.25.1
On Mon, Jul 05, 2021 at 03:29:47PM +0800, Jason Wang wrote: > > 在 2021/7/5 下午3:26, Michael S. Tsirkin 写道: > > On Mon, Jul 05, 2021 at 03:19:10PM +0800, Jason Wang wrote: > > > This patch switch to read virtqueue size from the capability instead > > > of depending on the hardcoded value. This allows the per virtqueue > > > size could be advertised. > > > > > > Signed-off-by: Jason Wang <jasowang@redhat.com> > > So let's add an ioctl for this? It's really a bug we don't.. > > > As explained in patch 1. Qemu doesn't use VHOST_VDPA_GET_VRING_NUM actually. > Instead it checks the result VHOST_VDPA_SET_VRING_NUM. > > So I change VHOST_VDPA_GET_VRING_NUM to return the minimal size of all the > virtqueues. > > If you wish we can add a VHOST_VDPA_GET_VRING_NUM2, but I'm not sure it will > have a user or not. > > Thanks Question is how do we know returning the minimal and not e.g. the max size is the right thing to do? > > > > > > --- > > > drivers/vdpa/virtio_pci/vp_vdpa.c | 6 ++++-- > > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/vdpa/virtio_pci/vp_vdpa.c b/drivers/vdpa/virtio_pci/vp_vdpa.c > > > index 2926641fb586..198f7076e4d9 100644 > > > --- a/drivers/vdpa/virtio_pci/vp_vdpa.c > > > +++ b/drivers/vdpa/virtio_pci/vp_vdpa.c > > > @@ -18,7 +18,6 @@ > > > #include <linux/virtio_pci.h> > > > #include <linux/virtio_pci_modern.h> > > > -#define VP_VDPA_QUEUE_MAX 256 > > > #define VP_VDPA_DRIVER_NAME "vp_vdpa" > > > #define VP_VDPA_NAME_SIZE 256 > > > @@ -197,7 +196,10 @@ static void vp_vdpa_set_status(struct vdpa_device *vdpa, u8 status) > > > static u16 vp_vdpa_get_vq_num_max(struct vdpa_device *vdpa, u16 qid) > > > { > > > - return VP_VDPA_QUEUE_MAX; > > > + struct vp_vdpa *vp_vdpa = vdpa_to_vp(vdpa); > > > + struct virtio_pci_modern_device *mdev = &vp_vdpa->mdev; > > > + > > > + return vp_modern_get_queue_size(mdev, qid); > > > } > > > static int vp_vdpa_get_vq_state(struct vdpa_device *vdpa, u16 qid, > > > -- > > > 2.25.1
在 2021/7/6 上午1:59, Michael S. Tsirkin 写道: > On Mon, Jul 05, 2021 at 03:29:47PM +0800, Jason Wang wrote: >> 在 2021/7/5 下午3:26, Michael S. Tsirkin 写道: >>> On Mon, Jul 05, 2021 at 03:19:10PM +0800, Jason Wang wrote: >>>> This patch switch to read virtqueue size from the capability instead >>>> of depending on the hardcoded value. This allows the per virtqueue >>>> size could be advertised. >>>> >>>> Signed-off-by: Jason Wang <jasowang@redhat.com> >>> So let's add an ioctl for this? It's really a bug we don't.. >> >> As explained in patch 1. Qemu doesn't use VHOST_VDPA_GET_VRING_NUM actually. >> Instead it checks the result VHOST_VDPA_SET_VRING_NUM. >> >> So I change VHOST_VDPA_GET_VRING_NUM to return the minimal size of all the >> virtqueues. >> >> If you wish we can add a VHOST_VDPA_GET_VRING_NUM2, but I'm not sure it will >> have a user or not. >> >> Thanks > Question is how do we know returning the minimal and not e.g. the max > size is the right thing to do? For the new ioctl, it will return the max queue size per vq. It's probably too late to fix the old one, so it's only safe to return the minimal one. Actually, most of the vDPA parents should be fine except for the vp_vdpa. When running in a nested environment, Qemu only advertise cvq with 64 entries. Thanks > > >>>> --- >>>> drivers/vdpa/virtio_pci/vp_vdpa.c | 6 ++++-- >>>> 1 file changed, 4 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/vdpa/virtio_pci/vp_vdpa.c b/drivers/vdpa/virtio_pci/vp_vdpa.c >>>> index 2926641fb586..198f7076e4d9 100644 >>>> --- a/drivers/vdpa/virtio_pci/vp_vdpa.c >>>> +++ b/drivers/vdpa/virtio_pci/vp_vdpa.c >>>> @@ -18,7 +18,6 @@ >>>> #include <linux/virtio_pci.h> >>>> #include <linux/virtio_pci_modern.h> >>>> -#define VP_VDPA_QUEUE_MAX 256 >>>> #define VP_VDPA_DRIVER_NAME "vp_vdpa" >>>> #define VP_VDPA_NAME_SIZE 256 >>>> @@ -197,7 +196,10 @@ static void vp_vdpa_set_status(struct vdpa_device *vdpa, u8 status) >>>> static u16 vp_vdpa_get_vq_num_max(struct vdpa_device *vdpa, u16 qid) >>>> { >>>> - return VP_VDPA_QUEUE_MAX; >>>> + struct vp_vdpa *vp_vdpa = vdpa_to_vp(vdpa); >>>> + struct virtio_pci_modern_device *mdev = &vp_vdpa->mdev; >>>> + >>>> + return vp_modern_get_queue_size(mdev, qid); >>>> } >>>> static int vp_vdpa_get_vq_state(struct vdpa_device *vdpa, u16 qid, >>>> -- >>>> 2.25.1
diff --git a/drivers/vdpa/virtio_pci/vp_vdpa.c b/drivers/vdpa/virtio_pci/vp_vdpa.c index 2926641fb586..198f7076e4d9 100644 --- a/drivers/vdpa/virtio_pci/vp_vdpa.c +++ b/drivers/vdpa/virtio_pci/vp_vdpa.c @@ -18,7 +18,6 @@ #include <linux/virtio_pci.h> #include <linux/virtio_pci_modern.h> -#define VP_VDPA_QUEUE_MAX 256 #define VP_VDPA_DRIVER_NAME "vp_vdpa" #define VP_VDPA_NAME_SIZE 256 @@ -197,7 +196,10 @@ static void vp_vdpa_set_status(struct vdpa_device *vdpa, u8 status) static u16 vp_vdpa_get_vq_num_max(struct vdpa_device *vdpa, u16 qid) { - return VP_VDPA_QUEUE_MAX; + struct vp_vdpa *vp_vdpa = vdpa_to_vp(vdpa); + struct virtio_pci_modern_device *mdev = &vp_vdpa->mdev; + + return vp_modern_get_queue_size(mdev, qid); } static int vp_vdpa_get_vq_state(struct vdpa_device *vdpa, u16 qid,
This patch switch to read virtqueue size from the capability instead of depending on the hardcoded value. This allows the per virtqueue size could be advertised. Signed-off-by: Jason Wang <jasowang@redhat.com> --- drivers/vdpa/virtio_pci/vp_vdpa.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)