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

Janosch Frank
2021-09-22T11:38:00Z

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;
>> }
>>
>