From: Halil Pasic <pasic@linux.ibm.com>
To: Matthew Rosato <mjrosato@linux.ibm.com>,
Cornelia Huck <cohuck@redhat.com>,
Christian Borntraeger <borntraeger@de.ibm.com>,
"Michael S. Tsirkin" <mst@redhat.com>,
qemu-s390x@nongnu.org, qemu-devel@nongnu.org
Cc: Halil Pasic <pasic@linux.ibm.com>, Thomas Huth <thuth@redhat.com>,
David Hildenbrand <david@redhat.com>,
Richard Henderson <rth@twiddle.net>
Subject: [PATCH 1/2] virtio-ccw: fix virtio_set_ind_atomic
Date: Tue, 16 Jun 2020 06:50:34 +0200 [thread overview]
Message-ID: <20200616045035.51641-2-pasic@linux.ibm.com> (raw)
In-Reply-To: <20200616045035.51641-1-pasic@linux.ibm.com>
The atomic_cmpxchg() loop is broken because we occasionally end up with
old and _old having different values (a legit compiler can generate code
that accessed *ind_addr again to pick up a value for _old instead of
using the value of old that was already fetched according to the
rules of the abstract machine). This means the underlying CS instruction
may use a different old (_old) than the one we intended to use if
atomic_cmpxchg() performed the xchg part.
Let us use volatile to force the rules of the abstract machine for
accesses to *ind_addr. Let us also rewrite the loop so, we that the
new old is used to compute the new desired value if the xchg part
is not performed.
Signed-off-by: Halil Pasic <pasic@linux.ibm.com>
Reported-by: Andre Wild <Andre.Wild1@ibm.com>
Fixes: 7e7494627f ("s390x/virtio-ccw: Adapter interrupt support.")
---
hw/s390x/virtio-ccw.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index c1f4bb1d33..3c988a000b 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -786,9 +786,10 @@ static inline VirtioCcwDevice *to_virtio_ccw_dev_fast(DeviceState *d)
static uint8_t virtio_set_ind_atomic(SubchDev *sch, uint64_t ind_loc,
uint8_t to_be_set)
{
- uint8_t ind_old, ind_new;
+ uint8_t expected, actual;
hwaddr len = 1;
- uint8_t *ind_addr;
+ /* avoid multiple fetches */
+ uint8_t volatile *ind_addr;
ind_addr = cpu_physical_memory_map(ind_loc, &len, true);
if (!ind_addr) {
@@ -796,14 +797,15 @@ static uint8_t virtio_set_ind_atomic(SubchDev *sch, uint64_t ind_loc,
__func__, sch->cssid, sch->ssid, sch->schid);
return -1;
}
+ actual = *ind_addr;
do {
- ind_old = *ind_addr;
- ind_new = ind_old | to_be_set;
- } while (atomic_cmpxchg(ind_addr, ind_old, ind_new) != ind_old);
- trace_virtio_ccw_set_ind(ind_loc, ind_old, ind_new);
- cpu_physical_memory_unmap(ind_addr, len, 1, len);
+ expected = actual;
+ actual = atomic_cmpxchg(ind_addr, expected, expected | to_be_set);
+ } while (actual != expected);
+ trace_virtio_ccw_set_ind(ind_loc, actual, actual | to_be_set);
+ cpu_physical_memory_unmap((void *)ind_addr, len, 1, len);
- return ind_old;
+ return actual;
}
static void virtio_ccw_notify(DeviceState *d, uint16_t vector)
--
2.17.1
next prev parent reply other threads:[~2020-06-16 4:51 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-16 4:50 [PATCH 0/2] two atomic_cmpxchg() related fixes Halil Pasic
2020-06-16 4:50 ` Halil Pasic [this message]
2020-06-16 5:58 ` [PATCH 1/2] virtio-ccw: fix virtio_set_ind_atomic Christian Borntraeger
2020-06-16 6:33 ` Cornelia Huck
2020-06-16 6:45 ` Christian Borntraeger
2020-06-19 7:14 ` Cornelia Huck
2020-06-17 23:56 ` Halil Pasic
2020-06-19 7:33 ` David Hildenbrand
2020-06-19 8:17 ` Halil Pasic
2020-06-16 9:31 ` Halil Pasic
2020-07-01 13:13 ` Christian Borntraeger
2020-07-04 18:34 ` Michael S. Tsirkin
2020-07-06 5:44 ` Christian Borntraeger
2020-07-06 11:19 ` Halil Pasic
2020-06-16 4:50 ` [PATCH 2/2] s390x/pci: fix set_ind_atomic Halil Pasic
2020-07-01 13:14 ` Christian Borntraeger
2020-07-01 12:01 ` [PATCH 0/2] two atomic_cmpxchg() related fixes Cornelia Huck
2020-07-01 12:06 ` Christian Borntraeger
2020-07-01 13:10 ` Cornelia Huck
2020-07-03 13:37 ` Halil Pasic
2020-07-03 14:03 ` Cornelia Huck
2020-07-02 11:18 ` Cornelia Huck
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200616045035.51641-2-pasic@linux.ibm.com \
--to=pasic@linux.ibm.com \
--cc=borntraeger@de.ibm.com \
--cc=cohuck@redhat.com \
--cc=david@redhat.com \
--cc=mjrosato@linux.ibm.com \
--cc=mst@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-s390x@nongnu.org \
--cc=rth@twiddle.net \
--cc=thuth@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.