OASIS Mailing List ArchivesView the OASIS mailing list archive below
or browse/search using MarkMail.


Help: OASIS Mailing Lists Help | MarkMail Help

virtio-dev message

[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]

Subject: Re: [virtio-dev] Re: queue_enable vs QueueReady

On 2020/6/2 äå12:20, Michael S. Tsirkin wrote:
On Tue, Jun 02, 2020 at 10:57:19AM +0800, Jason Wang wrote:
On 2020/6/1 äå2:01, Michael S. Tsirkin wrote:
On Thu, May 28, 2020 at 09:06:36PM +0800, Jason Wang wrote:

I found ambiguity in the virtio specification:

In PCI part, it describes the queue_enable as:

The driver uses this to selectively prevent the device from executing
requests from this virtqueue. 1 - enabled; 0 - disabled.

In MMIO part, it describes the QueueReady as:

Writing one (0x1) to this register notifies the device that it can execute
requests from this virtual queue. Reading from this register returns the
last value written to it. Both read and write accesses apply to the queue
selected by writing to QueueSel.

If I understand this correctly, they have the same meaning, but the driver
requirements section looks conflict:

PCI said: The driver MUST NOT write a 0 to queue_enable.

MMIO said:

To stop using the queue the driver MUST write zero (0x0) to this QueueReady
and MUST read the value back to ensure synchronization.

So we can't disable a queue via queue_enable but QueueReady. Any reason for
such inconsistency?

PCI assumed device reset is enough to stop all queues.  We had tons of
bugs around shutdown because of this, so in hindsight, MMIO had maybe a
better idea.

Ability to stop a queue and take back buffers would be nice, e.g. serial
is kind of messed up around port disconnect without it.

Yes, but this inconsistency brings trouble for vDPA implementation which has
a set_queue_ready() and we do the following in virtio_vdpa as MMIO did:

 ÂÂÂ /* Select and deactivate the queue */
 ÂÂÂ ops->set_vq_ready(vdpa, index, 0);
 ÂÂÂ WARN_ON(ops->get_vq_ready(vdpa, index));

The codes seems to work fine for IFC (a violation of the spec probably) but
not Qemu's virtio-net-pci.

What do you mean "not Qemu's virtio-net-pci"? Which implementation
of .set_vq_ready do you use?

I meant the queue_enable implemented in qemu virtio-net-pci.

See my patch: [PATCH] virtio-pci: fix queue_enable write

According to spec, I would expect this callback to do something
other that writing into queue_enable.

BTW I noticed this:

         if (cmd == VHOST_VDPA_SET_VRING_ENABLE) {
                 if (copy_from_user(&s, argp, sizeof(s)))
                         return -EFAULT;
                 ops->set_vq_ready(vdpa, idx, s.num);
                 return 0;

I'm guessing s.num should be 1, right?

It was a value set by userspace, so I think we can't have this assumption.


[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]