1. Patchew
  2. QEMU
  3. [PATCH 0/2] virtio: failover: allow to keep the VFIO device rather than the virtio-net one
  4. View patch

[PATCH 1/2] virtio: add a way to disable a queue

Laurent Vivier posted 2 patches 4 years, 6 months ago
Maintainers: Jason Wang <jasowang@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>
There is a newer version of this series
[PATCH 1/2] virtio: add a way to disable a queue
Posted by Laurent Vivier 4 years, 6 months ago 
Add virtio_queue_disable()/virtio_queue_enable() to disable/enable a queue
by setting vring.num to 0 (or num_default).
This is needed to be able to disable a guest driver from the host side

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 include/hw/virtio/virtio.h |  2 ++
 hw/virtio/virtio.c         | 10 ++++++++++
 2 files changed, 12 insertions(+)

diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 8bab9cfb7507..6a3f71b4cd88 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -251,6 +251,8 @@ void virtio_config_modern_writel(VirtIODevice *vdev,
                                  uint32_t addr, uint32_t data);
 void virtio_queue_set_addr(VirtIODevice *vdev, int n, hwaddr addr);
 hwaddr virtio_queue_get_addr(VirtIODevice *vdev, int n);
+void virtio_queue_enable(VirtIODevice *vdev, int n);
+void virtio_queue_disable(VirtIODevice *vdev, int n);
 void virtio_queue_set_num(VirtIODevice *vdev, int n, int num);
 int virtio_queue_get_num(VirtIODevice *vdev, int n);
 int virtio_queue_get_max_num(VirtIODevice *vdev, int n);
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 874377f37a70..fa5228c1a2d6 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2244,6 +2244,16 @@ void virtio_queue_set_rings(VirtIODevice *vdev, int n, hwaddr desc,
     virtio_init_region_cache(vdev, n);
 }
 
+void virtio_queue_disable(VirtIODevice *vdev, int n)
+{
+    vdev->vq[n].vring.num = 0;
+}
+
+void virtio_queue_enable(VirtIODevice *vdev, int n)
+{
+    vdev->vq[n].vring.num = vdev->vq[n].vring.num_default;
+}
+
 void virtio_queue_set_num(VirtIODevice *vdev, int n, int num)
 {
     /* Don't allow guest to flip queue between existent and
-- 
2.31.1
Re: [PATCH 1/2] virtio: add a way to disable a queue
Posted by Jason Wang 4 years, 6 months ago 
在 2021/7/30 上午3:19, Laurent Vivier 写道:
> Add virtio_queue_disable()/virtio_queue_enable() to disable/enable a queue
> by setting vring.num to 0 (or num_default).
> This is needed to be able to disable a guest driver from the host side


I suspect this won't work correclty for vhost.

And I believe we should only do this after the per queue 
enabling/disabling is supported by the spec.

(only MMIO support that AFAIK)

Thanks


>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>   include/hw/virtio/virtio.h |  2 ++
>   hw/virtio/virtio.c         | 10 ++++++++++
>   2 files changed, 12 insertions(+)
>
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index 8bab9cfb7507..6a3f71b4cd88 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -251,6 +251,8 @@ void virtio_config_modern_writel(VirtIODevice *vdev,
>                                    uint32_t addr, uint32_t data);
>   void virtio_queue_set_addr(VirtIODevice *vdev, int n, hwaddr addr);
>   hwaddr virtio_queue_get_addr(VirtIODevice *vdev, int n);
> +void virtio_queue_enable(VirtIODevice *vdev, int n);
> +void virtio_queue_disable(VirtIODevice *vdev, int n);
>   void virtio_queue_set_num(VirtIODevice *vdev, int n, int num);
>   int virtio_queue_get_num(VirtIODevice *vdev, int n);
>   int virtio_queue_get_max_num(VirtIODevice *vdev, int n);
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 874377f37a70..fa5228c1a2d6 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -2244,6 +2244,16 @@ void virtio_queue_set_rings(VirtIODevice *vdev, int n, hwaddr desc,
>       virtio_init_region_cache(vdev, n);
>   }
>   
> +void virtio_queue_disable(VirtIODevice *vdev, int n)
> +{
> +    vdev->vq[n].vring.num = 0;
> +}
> +
> +void virtio_queue_enable(VirtIODevice *vdev, int n)
> +{
> +    vdev->vq[n].vring.num = vdev->vq[n].vring.num_default;
> +}
> +
>   void virtio_queue_set_num(VirtIODevice *vdev, int n, int num)
>   {
>       /* Don't allow guest to flip queue between existent and
Re: [PATCH 1/2] virtio: add a way to disable a queue
Posted by Laurent Vivier 4 years, 6 months ago 
On 02/08/2021 06:50, Jason Wang wrote:
> 
> 在 2021/7/30 上午3:19, Laurent Vivier 写道:
>> Add virtio_queue_disable()/virtio_queue_enable() to disable/enable a queue
>> by setting vring.num to 0 (or num_default).
>> This is needed to be able to disable a guest driver from the host side
> 
> 
> I suspect this won't work correclty for vhost.

With my test it seems to work with vhost too.

> 
> And I believe we should only do this after the per queue enabling/disabling is supported
> by the spec.
> 
> (only MMIO support that AFAIK)

I don't want to modify the spec.

I need something that works without modifying existing (old) drivers.

The idea is to be able to disable the virtio-net kernel driver from QEMU if the driver is
too old (i.e. it doesn't support STANDBY feature).

Setting vring.num to 0 forces the kernel driver to exit on error in the probe function.
It's what I want: the device is present but disabled (the driver is not loaded).

Any other suggestion?

Thanks,
Laurent

> Thanks
> 
> 
>>
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>> ---
>>   include/hw/virtio/virtio.h |  2 ++
>>   hw/virtio/virtio.c         | 10 ++++++++++
>>   2 files changed, 12 insertions(+)
>>
>> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
>> index 8bab9cfb7507..6a3f71b4cd88 100644
>> --- a/include/hw/virtio/virtio.h
>> +++ b/include/hw/virtio/virtio.h
>> @@ -251,6 +251,8 @@ void virtio_config_modern_writel(VirtIODevice *vdev,
>>                                    uint32_t addr, uint32_t data);
>>   void virtio_queue_set_addr(VirtIODevice *vdev, int n, hwaddr addr);
>>   hwaddr virtio_queue_get_addr(VirtIODevice *vdev, int n);
>> +void virtio_queue_enable(VirtIODevice *vdev, int n);
>> +void virtio_queue_disable(VirtIODevice *vdev, int n);
>>   void virtio_queue_set_num(VirtIODevice *vdev, int n, int num);
>>   int virtio_queue_get_num(VirtIODevice *vdev, int n);
>>   int virtio_queue_get_max_num(VirtIODevice *vdev, int n);
>> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
>> index 874377f37a70..fa5228c1a2d6 100644
>> --- a/hw/virtio/virtio.c
>> +++ b/hw/virtio/virtio.c
>> @@ -2244,6 +2244,16 @@ void virtio_queue_set_rings(VirtIODevice *vdev, int n, hwaddr desc,
>>       virtio_init_region_cache(vdev, n);
>>   }
>>   +void virtio_queue_disable(VirtIODevice *vdev, int n)
>> +{
>> +    vdev->vq[n].vring.num = 0;
>> +}
>> +
>> +void virtio_queue_enable(VirtIODevice *vdev, int n)
>> +{
>> +    vdev->vq[n].vring.num = vdev->vq[n].vring.num_default;
>> +}
>> +
>>   void virtio_queue_set_num(VirtIODevice *vdev, int n, int num)
>>   {
>>       /* Don't allow guest to flip queue between existent and
>
Re: [PATCH 1/2] virtio: add a way to disable a queue
Posted by Jason Wang 4 years, 6 months ago 
在 2021/8/2 下午4:42, Laurent Vivier 写道:
> On 02/08/2021 06:50, Jason Wang wrote:
>> 在 2021/7/30 上午3:19, Laurent Vivier 写道:
>>> Add virtio_queue_disable()/virtio_queue_enable() to disable/enable a queue
>>> by setting vring.num to 0 (or num_default).
>>> This is needed to be able to disable a guest driver from the host side
>>
>> I suspect this won't work correclty for vhost.
> With my test it seems to work with vhost too.


So setting 0 will lead -EINVAL to be returned during 
VHOST_SET_VRING_NUM. I think qemu will warn the failure in this case.

What's more important, it's not guaranteed to work for the case of 
vhost-user or vhost-vDPA.


>
>> And I believe we should only do this after the per queue enabling/disabling is supported
>> by the spec.
>>
>> (only MMIO support that AFAIK)
> I don't want to modify the spec.
>
> I need something that works without modifying existing (old) drivers.
>
> The idea is to be able to disable the virtio-net kernel driver from QEMU if the driver is
> too old (i.e. it doesn't support STANDBY feature).
>
> Setting vring.num to 0 forces the kernel driver to exit on error in the probe function.
> It's what I want: the device is present but disabled (the driver is not loaded).
>
> Any other suggestion?


I think we should probably disable the device instead of doing it per 
virtqueue.

Thanks


>
> Thanks,
> Laurent
>
>> Thanks
>>
>>
>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>> ---
>>>    include/hw/virtio/virtio.h |  2 ++
>>>    hw/virtio/virtio.c         | 10 ++++++++++
>>>    2 files changed, 12 insertions(+)
>>>
>>> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
>>> index 8bab9cfb7507..6a3f71b4cd88 100644
>>> --- a/include/hw/virtio/virtio.h
>>> +++ b/include/hw/virtio/virtio.h
>>> @@ -251,6 +251,8 @@ void virtio_config_modern_writel(VirtIODevice *vdev,
>>>                                     uint32_t addr, uint32_t data);
>>>    void virtio_queue_set_addr(VirtIODevice *vdev, int n, hwaddr addr);
>>>    hwaddr virtio_queue_get_addr(VirtIODevice *vdev, int n);
>>> +void virtio_queue_enable(VirtIODevice *vdev, int n);
>>> +void virtio_queue_disable(VirtIODevice *vdev, int n);
>>>    void virtio_queue_set_num(VirtIODevice *vdev, int n, int num);
>>>    int virtio_queue_get_num(VirtIODevice *vdev, int n);
>>>    int virtio_queue_get_max_num(VirtIODevice *vdev, int n);
>>> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
>>> index 874377f37a70..fa5228c1a2d6 100644
>>> --- a/hw/virtio/virtio.c
>>> +++ b/hw/virtio/virtio.c
>>> @@ -2244,6 +2244,16 @@ void virtio_queue_set_rings(VirtIODevice *vdev, int n, hwaddr desc,
>>>        virtio_init_region_cache(vdev, n);
>>>    }
>>>    +void virtio_queue_disable(VirtIODevice *vdev, int n)
>>> +{
>>> +    vdev->vq[n].vring.num = 0;
>>> +}
>>> +
>>> +void virtio_queue_enable(VirtIODevice *vdev, int n)
>>> +{
>>> +    vdev->vq[n].vring.num = vdev->vq[n].vring.num_default;
>>> +}
>>> +
>>>    void virtio_queue_set_num(VirtIODevice *vdev, int n, int num)
>>>    {
>>>        /* Don't allow guest to flip queue between existent and
Re: [PATCH 1/2] virtio: add a way to disable a queue
Posted by Laurent Vivier 4 years, 6 months ago 
On 06/08/2021 08:25, Jason Wang wrote:
> 
> 在 2021/8/2 下午4:42, Laurent Vivier 写道:
>> On 02/08/2021 06:50, Jason Wang wrote:
>>> 在 2021/7/30 上午3:19, Laurent Vivier 写道:
>>>> Add virtio_queue_disable()/virtio_queue_enable() to disable/enable a queue
>>>> by setting vring.num to 0 (or num_default).
>>>> This is needed to be able to disable a guest driver from the host side
>>>
>>> I suspect this won't work correclty for vhost.
>> With my test it seems to work with vhost too.
> 
> 
> So setting 0 will lead -EINVAL to be returned during VHOST_SET_VRING_NUM. I think qemu
> will warn the failure in this case.

I didn't see any error when I tried. I will check the code.

> What's more important, it's not guaranteed to work for the case of vhost-user or vhost-vDPA.

Perhaps we can target only the vhost host case, as this is used for failover and usually
the virtio-net device is backed by a bridge on same network as the VFIO device?

> 
> 
>>
>>> And I believe we should only do this after the per queue enabling/disabling is supported
>>> by the spec.
>>>
>>> (only MMIO support that AFAIK)
>> I don't want to modify the spec.
>>
>> I need something that works without modifying existing (old) drivers.
>>
>> The idea is to be able to disable the virtio-net kernel driver from QEMU if the driver is
>> too old (i.e. it doesn't support STANDBY feature).
>>
>> Setting vring.num to 0 forces the kernel driver to exit on error in the probe function.
>> It's what I want: the device is present but disabled (the driver is not loaded).
>>
>> Any other suggestion?
> 
> 
> I think we should probably disable the device instead of doing it per virtqueue.
> 

I tried to use virtio_set_disabled() but it doesn't work.
Perhaps it's too late when I call the function (I need to do that in
virtio_net_set_features()). What I want is to prevent the load of the driver in the guest
kernel to hide the virtio-net device. Setting vring.num to 0 triggers an error in the
driver probe function and prevents the load of the driver.

Thanks,
Laurent
Re: [PATCH 1/2] virtio: add a way to disable a queue
Posted by Jason Wang 4 years, 6 months ago 
在 2021/8/6 下午3:27, Laurent Vivier 写道:
> On 06/08/2021 08:25, Jason Wang wrote:
>> 在 2021/8/2 下午4:42, Laurent Vivier 写道:
>>> On 02/08/2021 06:50, Jason Wang wrote:
>>>> 在 2021/7/30 上午3:19, Laurent Vivier 写道:
>>>>> Add virtio_queue_disable()/virtio_queue_enable() to disable/enable a queue
>>>>> by setting vring.num to 0 (or num_default).
>>>>> This is needed to be able to disable a guest driver from the host side
>>>> I suspect this won't work correclty for vhost.
>>> With my test it seems to work with vhost too.
>>
>> So setting 0 will lead -EINVAL to be returned during VHOST_SET_VRING_NUM. I think qemu
>> will warn the failure in this case.
> I didn't see any error when I tried. I will check the code.
>
>> What's more important, it's not guaranteed to work for the case of vhost-user or vhost-vDPA.
> Perhaps we can target only the vhost host case, as this is used for failover and usually
> the virtio-net device is backed by a bridge on same network as the VFIO device?


Probably not, it should be a general feature that can work for all types 
of virtio/vhost backends.


>
>>
>>>> And I believe we should only do this after the per queue enabling/disabling is supported
>>>> by the spec.
>>>>
>>>> (only MMIO support that AFAIK)
>>> I don't want to modify the spec.
>>>
>>> I need something that works without modifying existing (old) drivers.
>>>
>>> The idea is to be able to disable the virtio-net kernel driver from QEMU if the driver is
>>> too old (i.e. it doesn't support STANDBY feature).
>>>
>>> Setting vring.num to 0 forces the kernel driver to exit on error in the probe function.
>>> It's what I want: the device is present but disabled (the driver is not loaded).
>>>
>>> Any other suggestion?
>>
>> I think we should probably disable the device instead of doing it per virtqueue.
>>
> I tried to use virtio_set_disabled() but it doesn't work.
> Perhaps it's too late when I call the function (I need to do that in
> virtio_net_set_features()). What I want is to prevent the load of the driver in the guest
> kernel to hide the virtio-net device. Setting vring.num to 0 triggers an error in the
> driver probe function and prevents the load of the driver.


How about fail the validate_features() in this case?

Thanks


>
> Thanks,
> Laurent
>
Re: [PATCH 1/2] virtio: add a way to disable a queue
Posted by Laurent Vivier 4 years, 6 months ago 
On 09/08/2021 05:01, Jason Wang wrote:
> 
> 在 2021/8/6 下午3:27, Laurent Vivier 写道:
>> On 06/08/2021 08:25, Jason Wang wrote:
>>> 在 2021/8/2 下午4:42, Laurent Vivier 写道:
>>>> On 02/08/2021 06:50, Jason Wang wrote:
>>>>> 在 2021/7/30 上午3:19, Laurent Vivier 写道:
>>>>>> Add virtio_queue_disable()/virtio_queue_enable() to disable/enable a queue
>>>>>> by setting vring.num to 0 (or num_default).
>>>>>> This is needed to be able to disable a guest driver from the host side
>>>>> I suspect this won't work correclty for vhost.
>>>> With my test it seems to work with vhost too.
>>>
>>> So setting 0 will lead -EINVAL to be returned during VHOST_SET_VRING_NUM. I think qemu
>>> will warn the failure in this case.
>> I didn't see any error when I tried. I will check the code.
>>
>>> What's more important, it's not guaranteed to work for the case of vhost-user or
>>> vhost-vDPA.
>> Perhaps we can target only the vhost host case, as this is used for failover and usually
>> the virtio-net device is backed by a bridge on same network as the VFIO device?
> 
> 
> Probably not, it should be a general feature that can work for all types of virtio/vhost
> backends.
> 
> 
>>
>>>
>>>>> And I believe we should only do this after the per queue enabling/disabling is supported
>>>>> by the spec.
>>>>>
>>>>> (only MMIO support that AFAIK)
>>>> I don't want to modify the spec.
>>>>
>>>> I need something that works without modifying existing (old) drivers.
>>>>
>>>> The idea is to be able to disable the virtio-net kernel driver from QEMU if the driver is
>>>> too old (i.e. it doesn't support STANDBY feature).
>>>>
>>>> Setting vring.num to 0 forces the kernel driver to exit on error in the probe function.
>>>> It's what I want: the device is present but disabled (the driver is not loaded).
>>>>
>>>> Any other suggestion?
>>>
>>> I think we should probably disable the device instead of doing it per virtqueue.
>>>
>> I tried to use virtio_set_disabled() but it doesn't work.
>> Perhaps it's too late when I call the function (I need to do that in
>> virtio_net_set_features()). What I want is to prevent the load of the driver in the guest
>> kernel to hide the virtio-net device. Setting vring.num to 0 triggers an error in the
>> driver probe function and prevents the load of the driver.
> 
> 
> How about fail the validate_features() in this case?

It's a good suggestion and it seems to work.

I'm going to send an updated patch.

Thanks,
Laurent

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.