On 10/10/2019 09.52, Christian Borntraeger wrote:
> similar to commit 2c57da356800 ("selftests: kvm: fix sync_regs_test with
> newer gccs") and commit 204c91eff798a ("KVM: selftests: do not blindly
> clobber registers in guest asm") we better do not rely on gcc leaving
> r11 untouched. We can write the simple ucall inline and have the guest
> code completely as small assembler function.
>
> Signed-off-by: Christian Borntraeger <borntraeger@xxxxxxxxxx>
> Suggested-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> ---
> .../testing/selftests/kvm/s390x/sync_regs_test.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/s390x/sync_regs_test.c b/tools/testing/selftests/kvm/s390x/sync_regs_test.c
> index d5290b4ad636..04d6abe68961 100644
> --- a/tools/testing/selftests/kvm/s390x/sync_regs_test.c
> +++ b/tools/testing/selftests/kvm/s390x/sync_regs_test.c
> @@ -25,12 +25,15 @@
>
> static void guest_code(void)
> {
> - register u64 stage asm("11") = 0;
> -
> - for (;;) {
> - GUEST_SYNC(0);
> - asm volatile ("ahi %0,1" : : "r"(stage));
> - }
> + /*
> + * we embed diag 501 here instead of a ucall as the called function
> + * could mess with the content of r11 when doing the hypercall
As far as I understood the issue on x86, it's not the called function
that is messing with the counter register (since r11 is not volatile
between function calls), but the compiler shuffled it around here
between the GUEST_SYNC and the inline assembler code. So I'd prefer a
comment like in the x86 version:
/*
* We embed diag 501 here instead of doing a ucall to avoid that
* the compiler reshuffles registers before calling the function.
*/
?
With such an updated comment:
Reviewed-by: Thomas Huth <thuth@xxxxxxxxxx>
> + */
> + asm volatile (
> + "0: diag 0,0,0x501\n"
> + " ahi 11,1\n"
> + " j 0b\n"
> + );
> }
