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

[Halil Pasic]

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.

Conny, what do you think?

Regards,
Halil