From: Andrew Jones
Sent: 16 June 2022 17:26
On Thu, Jun 16, 2022 at 03:58:52PM +0000, David Laight wrote:
From: Andrew Jones
Sent: 16 June 2022 13:03
On Wed, Jun 15, 2022 at 06:57:06PM +0000, Raghavendra Rao Ananta wrote:
The selftests, when built with newer versions of clang, is found to have over optimized guests' ucall() function, and eliminating the stores for uc.cmd (perhaps due to no immediate readers). This resulted in the userspace side always reading a value of '0', and causing multiple test failures.
As a result, prevent the compiler from optimizing the stores in ucall() with WRITE_ONCE().
Suggested-by: Ricardo Koller ricarkol@google.com Suggested-by: Reiji Watanabe reijiw@google.com Signed-off-by: Raghavendra Rao Ananta rananta@google.com
tools/testing/selftests/kvm/lib/aarch64/ucall.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/kvm/lib/aarch64/ucall.c
b/tools/testing/selftests/kvm/lib/aarch64/ucall.c
index e0b0164e9af8..be1d9728c4ce 100644 --- a/tools/testing/selftests/kvm/lib/aarch64/ucall.c +++ b/tools/testing/selftests/kvm/lib/aarch64/ucall.c @@ -73,20 +73,19 @@ void ucall_uninit(struct kvm_vm *vm)
void ucall(uint64_t cmd, int nargs, ...) {
- struct ucall uc = {
.cmd = cmd,- };
struct ucall uc = {}; va_list va; int i;
WRITE_ONCE(uc.cmd, cmd); nargs = nargs <= UCALL_MAX_ARGS ? nargs : UCALL_MAX_ARGS;
va_start(va, nargs); for (i = 0; i < nargs; ++i)
uc.args[i] = va_arg(va, uint64_t);
WRITE_ONCE(uc.args[i], va_arg(va, uint64_t));
- *ucall_exit_mmio_addr = (vm_vaddr_t)&uc;
- WRITE_ONCE(*ucall_exit_mmio_addr, (vm_vaddr_t)&uc);
}
Am I misreading things again? That function looks like it writes the address of an on-stack item into global data.
The write to the address that the global points at causes a switch from guest to host context. The guest's stack remains intact while executing host code and the host can access the uc stack variable directly by its address. Take a look at lib/aarch64/ucall.c to see all the details.
No wonder I was confused. It's not surprising the compiler optimises it all away.
It doesn't seem right to be 'abusing' WRITE_ONCE() here. Just adding barrier() should be enough and much more descriptive.
David
- Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)