On Thu, Mar 14, 2019 at 10:45:25PM +0800, Jason Wang wrote: > > On 2019/2/25 下午8:33, Michael S. Tsirkin wrote: > > On Mon, Feb 25, 2019 at 03:47:48PM +0800, Jason Wang wrote: > > > On 2019/2/22 下午12:22, Michael S. Tsirkin wrote: > > > > On Thu, Feb 21, 2019 at 10:10:08PM -0500, Michael S. Tsirkin wrote: > > > > > On Fri, Feb 22, 2019 at 11:04:05AM +0800, Jason Wang wrote: > > > > > > On 2019/2/22 上午9:35, Michael S. Tsirkin wrote: > > > > > > > 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. > > > > > > So you meant write to queue_enable is forbidden after DRIVER_OK > > > > > > (though it's > > > > > > not very clear to me from the spec). And if a driver want to > > > > > > enable new > > > > > > queues, it must reset the device? > > > > > That's my reading. What do you think? > > > Looks like I can infer this from the spec, maybe it's better to clarify. > > > > > > > > > > Btw some legacy drivers might violate this by addig buffers > > > > before driver_ok. > > > Yes, but it's probably too late to fix them. > > > > > > Thanks. > > Right. As a work around virtio net also checks rings > > when it detects driver ok. We can disable that when VIRTIO_1 > > has been negotiated. > > > > So, it looks to me the proper fix is probably start the vhost virtqueues > selectively only when the vring address is not zero (for 0.9x) or > queue_enable is set (for 1.0). > > Thanks
Sounds good. -- MST
