Re: [PATCH] pc-bios/s390-ccw: Use memory barriers in virtio code

[Cornelia Huck <cohuck@redhat.com>]

On Tue, 16 Feb 2021 15:40:10 +0100
Halil Pasic <pasic@linux.ibm.com> wrote:

> On Tue, 16 Feb 2021 12:00:56 +0100
> Thomas Huth <thuth@redhat.com> wrote:
> 
> > According to the virtio specification, a memory barrier should be
> > used before incrementing the idx field in the "available" ring.
> > So far, we did not do this in the s390-ccw bios yet, but recently
> > Peter Maydell saw problems with the s390-ccw bios when running
> > the qtests on an aarch64 host (the bios panic'ed with the message:
> > "SCSI cannot report LUNs: response VS RESP=09"), which could
> > maybe be related to the missing memory barriers. Thus let's add
> > those barriers now. Since we've only seen the problem on TCG so far,
> > a "bcr 14,0" should be sufficient here to trigger the tcg_gen_mb()
> > in the TCG translate code.
> > 
> > (Note: The virtio spec also talks about using a memory barrier
> > *after* incrementing the idx field, but if I understood correctly
> > this is only required when using notification suppression - which
> > we don't use in the s390-ccw bios here)  
> 
> I suggest to the barrier after incrementing the idx field for two
> reasons. First: If the device were to see the notification, but
> not see the incremented idx field, it would effectively loose
> initiative. That is pretty straight forward, because the
> notification just says 'check out that queue', and if we don't
> see the incremented index, miss the buffer that was made available
> by incrementing idx.
> 
> Second: We are in the bios, and I hope even an unnecessary barrier
> would not hurt us significantly.

Yes to both, I think that is worth a try.

> 
> Conny, what do you think?
> 
> Regards,
> Halil
>