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

Janosch Frank
2021-12-17T14:16:41Z

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?

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