On Tue, 16 Feb 2021 15:40:10 +0100 Halil Pasic <pa...@linux.ibm.com> wrote:
> On Tue, 16 Feb 2021 12:00:56 +0100 > Thomas Huth <th...@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 >
