diff mbox series

[2/2] vdpa: vp_vdpa: don't use hard-coded maximum virtqueue size

Message ID 20210705071910.31965-2-jasowang@redhat.com (mailing list archive)
State New, archived
Headers show
Series [1/2] vdpa: support per virtqueue max queue size | expand

Commit Message

Jason Wang July 5, 2021, 7:19 a.m. UTC
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(-)

Comments

Michael S. Tsirkin July 5, 2021, 7:26 a.m. UTC | #1
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
Jason Wang July 5, 2021, 7:29 a.m. UTC | #2
在 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
Michael S. Tsirkin July 5, 2021, 5:59 p.m. UTC | #3
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
Jason Wang July 6, 2021, 2:30 a.m. UTC | #4
在 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 mbox series

Patch

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,