Skip to content

Commit 1a8242f

Browse files
halil-pasiccohuck
authored andcommitted
virtio-ccw: fix virtio_set_ind_atomic
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. Fixes: 7e74946 ("s390x/virtio-ccw: Adapter interrupt support.") Reported-by: Andre Wild <[email protected]> Signed-off-by: Halil Pasic <[email protected]> Reviewed-by: Christian Borntraeger <[email protected]> Message-Id: <[email protected]> Signed-off-by: Cornelia Huck <[email protected]>
1 parent 9bf728a commit 1a8242f

File tree

1 file changed

+10
-8
lines changed

1 file changed

+10
-8
lines changed

hw/s390x/virtio-ccw.c

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -786,24 +786,26 @@ static inline VirtioCcwDevice *to_virtio_ccw_dev_fast(DeviceState *d)
786786
static uint8_t virtio_set_ind_atomic(SubchDev *sch, uint64_t ind_loc,
787787
uint8_t to_be_set)
788788
{
789-
uint8_t ind_old, ind_new;
789+
uint8_t expected, actual;
790790
hwaddr len = 1;
791-
uint8_t *ind_addr;
791+
/* avoid multiple fetches */
792+
uint8_t volatile *ind_addr;
792793

793794
ind_addr = cpu_physical_memory_map(ind_loc, &len, true);
794795
if (!ind_addr) {
795796
error_report("%s(%x.%x.%04x): unable to access indicator",
796797
__func__, sch->cssid, sch->ssid, sch->schid);
797798
return -1;
798799
}
800+
actual = *ind_addr;
799801
do {
800-
ind_old = *ind_addr;
801-
ind_new = ind_old | to_be_set;
802-
} while (atomic_cmpxchg(ind_addr, ind_old, ind_new) != ind_old);
803-
trace_virtio_ccw_set_ind(ind_loc, ind_old, ind_new);
804-
cpu_physical_memory_unmap(ind_addr, len, 1, len);
802+
expected = actual;
803+
actual = atomic_cmpxchg(ind_addr, expected, expected | to_be_set);
804+
} while (actual != expected);
805+
trace_virtio_ccw_set_ind(ind_loc, actual, actual | to_be_set);
806+
cpu_physical_memory_unmap((void *)ind_addr, len, 1, len);
805807

806-
return ind_old;
808+
return actual;
807809
}
808810

809811
static void virtio_ccw_notify(DeviceState *d, uint16_t vector)

0 commit comments

Comments
 (0)