Re: [kvm-unit-tests PATCH 5/9] lib: s390x: uv: Add UVC_ERR_DEBUG switch

[Janosch Frank <frankja@linux.ibm.com>]

On 9/22/21 11:23 AM, Claudio Imbrenda wrote:
> On Wed, 22 Sep 2021 07:18:07 +0000
> Janosch Frank <frankja@linux.ibm.com> wrote:
> 
>> Every time something goes wrong in a way we don't expect, we need to
>> add debug prints to some UVC to get the unexpected return code.
>>
>> Let's just put the printing behind a macro so we can enable it if
>> needed via a simple switch.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> 
> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> 
> but see a nit below
> 
>> ---
>>  lib/s390x/asm/uv.h | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/lib/s390x/asm/uv.h b/lib/s390x/asm/uv.h
>> index 2f099553..0e958ad7 100644
>> --- a/lib/s390x/asm/uv.h
>> +++ b/lib/s390x/asm/uv.h
>> @@ -12,6 +12,9 @@
>>  #ifndef _ASMS390X_UV_H_
>>  #define _ASMS390X_UV_H_
>>  
>> +/* Enables printing of command code and return codes for failed UVCs
>> */ +#define UVC_ERR_DEBUG	0
>> +
>>  #define UVC_RC_EXECUTED		0x0001
>>  #define UVC_RC_INV_CMD		0x0002
>>  #define UVC_RC_INV_STATE	0x0003
>> @@ -194,6 +197,15 @@ static inline int uv_call_once(unsigned long r1,
>> unsigned long r2) : [cc] "=d" (cc)
>>  		: [r1] "a" (r1), [r2] "a" (r2)
>>  		: "memory", "cc");
>> +
>> +#if UVC_ERR_DEBUG
>> +	if (cc)
> 
> it probably looks cleaner like this:
> 
> 	if (UVC_ERR_DEBUG && cc)
> 
> and without the #if; the compiler should be smart enough to remove the
> dead code. In practice it doesn't really matter in the end, so feel free
> to ignore this comment :)

Didn't consider that, I'll fix it up.
Thank you

> 
>> +		printf("UV call error: call %x rc %x rrc %x\n",
>> +		       ((struct uv_cb_header *)r2)->cmd,
>> +		       ((struct uv_cb_header *)r2)->rc,
>> +		       ((struct uv_cb_header *)r2)->rrc);
>> +#endif
>> +
>>  	return cc;
>>  }
>>  
>