Re: [PATCH kvm-unit-tests 1/2] s390x: diag288: Add missing clobber

Claudio Imbrenda
2021-12-17T14:23:21Z

On Fri, 17 Dec 2021 15:16:34 +0100
Janosch Frank <frankja@linux.ibm.com> wrote:

> On 12/17/21 14:47, Thomas Huth wrote:
> > On 17/12/2021 11.31, Nico Boehr wrote:
> >> We clobber r0 and thus should let the compiler know we're doing so.
> >>
> >> Because we change from basic to extended ASM, we need to change the
> >> register names, as %r0 will be interpreted as a token in the assembler
> >> template.
> >>
> >> For consistency, we align with the common style in kvm-unit-tests which
> >> is just 0.
> >>
> >> Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
> >> ---
> >> s390x/diag288.c | 7 ++++---
> >> 1 file changed, 4 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/s390x/diag288.c b/s390x/diag288.c
> >> index 072c04a5cbd6..da7b06c365bf 100644
> >> --- a/s390x/diag288.c
> >> +++ b/s390x/diag288.c
> >> @@ -94,11 +94,12 @@ static void test_bite(void)
> >> /* Arm watchdog */
> >> lc->restart_new_psw.mask = extract_psw_mask() & ~PSW_MASK_EXT;
> >> diag288(CODE_INIT, 15, ACTION_RESTART);
> >> - asm volatile(" larl %r0, 1f\n"
> >> - " stg %r0, 424\n"
> >> + asm volatile(" larl 0, 1f\n"
> >> + " stg 0, 424\n"
> >
> > Would it work to use %%r0 instead?
>
> Yes, but I told him that looks weird, so that one is on me.
> @claudio @thomas What's your preferred way of dealing with this?

I would prefer just 0 since that's what we use everywhere else too,
but I won't oppose %%r0 if there are strong arguments for it (but then
we need to decide a policy and stick to it)

>
> >
> >> "0: nop\n"
> >> " j 0b\n"
> >> - "1:");
> >> + "1:"
> >> + : : : "0");
> >> report_pass("restart");
> >> }
> >
> > Anyway:
> > Reviewed-by: Thomas Huth <thuth@redhat.com>
> >
>