Re: [Qemu-devel] [PATCH] virtio-net: do not start queues that are not enabled by the guest

[Michael S. Tsirkin]

On Thu, Feb 21, 2019 at 05:40:22PM +0800, Jason Wang wrote:
> 
> On 2019/2/21 下午4:18, Yuri Benditovich wrote:
> 
>         For 1.0 device, we can fix the queue_enable, but for 0.9x device how 
> do
>         you enable one specific queue in this case? (setting status?)
> 
> 
>     Do I understand correctly that for 0.9 device in some cases the device 
> will
>     receive feature _MQ set, but will not receive 
> VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET?
>     Or the problem is different?
> 
> 
> Let me clarify, VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET is used to control the the
> number of queue pairs used by device for doing transmission and reception. It
> was not used to enable or disable a virtqueue.
> 
> For 1.0 device, we should use queue_enable in pci cfg to enable and disable
> queue:
> 
> 
> We could do:
> 
> 1) allocate memory and set queue_enable for vq0
> 
> 2) allocate memory and set queue_enable for vq1
> 
> 3) Set vq paris to 1
> 
> 4) allocate memory and set queue_enable for vq2
> 
> 5) allocate memory and set queue_enable for vq3
> 
> 6) set vq pairs to 2


I do not think spec allows this.


The driver MUST follow this sequence to initialize a device:
1. Reset the device.
2. Set the ACKNOWLEDGE status bit: the guest OS has noticed the device.
3. Set the DRIVER status bit: the guest OS knows how to drive the device.
4. Read device feature bits, and write the subset of feature bits understood by 
the OS and driver to the
device. During this step the driver MAY read (but MUST NOT write) the 
device-specific configuration
fields to check that it can support the device before accepting it.
5. Set the FEATURES_OK status bit. The driver MUST NOT accept new feature bits 
after this step.
6. Re-read device status to ensure the FEATURES_OK bit is still set: otherwise, 
the device does not
support our subset of features and the device is unusable.
7. Perform device-specific setup, including discovery of virtqueues for the 
device, optional per-bus setup,
reading and possibly writing the device’s virtio configuration space, and 
population of virtqueues.
8. Set the DRIVER_OK status bit. At this point the device is “live”.


Thus vqs are setup at step 7.

# of vq pairs are set up through a command which is a special
buffer, and spec says:

The driver MUST NOT send any buffer available notifications to the device 
before setting DRIVER_OK.


> 
> But this requires a proper implementation for queue_enable for vhost which is
> missed in qemu and probably what you really want to do.
> 
> but for 0.9x device, there's no such way to do this. That's the issue.

0.9x there's no queue enable, assumption is PA!=0 means VQ has
been enabled.


> So
> driver must allocate all queBes before starting the device, otherwise there's
> no way to enable it afterwards.


As per spec queues must be allocated before DRIVER_OK.

That is universal.


> There're tricks to make it work like what is
> done in your patch, but it depends on a specific implementation like qemu 
> which
> is sub-optimal.
> 
> 
> 
> 
>         A fundamental question is what prevents you from just initialization 
> all
>         queues during driver start? It looks to me this save lots of efforts
>         than allocating queue dynamically.
> 
> 
>     This is not so trivial in Windows driver, as it does not have objects for 
> queues
>     that it does not use. Linux driver first of all allocates all the
>     queues and then
>     adds Rx/Tx to those it will use. Windows driver first decides how many 
> queues
>     it will use then allocates objects for them and initializes them from 
> zero to
>     fully functional state.
> 
> 
> Well, you just need to allocate some memory for the virtqueue, there's no need
> to make it visible to the rest until it was enabled.
> 
> Thanks
> 
> 
> 
>