Re: [virtio-dev] Re: Virtio Spec clarification

["Michael S. Tsirkin" <mst@redhat.com>]

On Mon, Feb 06, 2017 at 10:50:01AM +0100, Paolo Bonzini wrote:
> 
> 
> On 31/01/2017 19:27, Michael S. Tsirkin wrote:
> > On Tue, Jan 31, 2017 at 01:13:37PM -0500, Karandeep Chahal wrote:
> >> Hello Virtio Developers,
> >>
> >> I need some clarification about the virtio spec, and I was wondering if you
> >> can help me with that.
> >>
> >> If VIRTIO_F_EVENT_IDX feature is negotiated, when is the driver supposed to
> >> update the used_event? Is the driver supposed to do it before or after it
> >> updates the avail_idx?
> > 
> > used_event is related to processing used ring entries.
> > It requests an interrupt each time the used index will reach
> > a specific value.
> > After updating it, the spec suggests that the driver check that the
> > index did not reach that value yet.
> > 
> >> The reason I am asking this question is because if the driver updates the
> >> used_event after it updates the avail_idx, there is a possibility that the
> >> device will process and complete the IO before the driver can update the
> >> used_event. My device is constantly polling for changes to avail_idx, and
> >> sometimes it does not interrupt the driver because the used_event has not
> >> been updated by the driver yet.
> >
> > I think this should not be a problem if driver checks the
> > used index after updating used_event.
> 
> You also need a memory barrier instruction after updating the used_event.
> 
> Paolo

+1
And the linux driver works on the assumption that device is
virtual and running on another CPU, so CPU ordering rules apply.

This isn't something we ever documented but we really should.

So if your device isn't actually a CPU and is not following CPU cache
synchronization rules being on a different bus, a heavier barrier
flushing writes in from the bus might be required.

Try hacking vq->weak_barriers in Linux (it's a parameter passed
to vring_create_virtqueue) to be false.

If that helps, there's your answer. And to fix that we should
probably add another feature bit. Maybe VIRTIO_F_BARRIERS_PLATFORM?

-- 
MST