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

[Peter Maydell <peter.maydell@linaro.org>]

On Tue, 16 Feb 2021 at 11:01, 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)
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>

This noticeably increases the reliability of the test for me:
it goes from failing one time in 2 or 3 to failing about one time
in 5 or 6. However it does still hit the virtio_scsi_parse_req()
check on "out_size && in_size" eventually. So my guess is that this
is not a sufficient supply of barriers. I'm firmly not a virtio expert
but my reading of the spec suggested that some of the way the
s390-ccw code is doing things might be in the wrong order and
need restructuring beyond merely "add barriers to existing code".

thanks
-- PMM