Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v3 01/14] KVM: s390: pv: add macros for UVC CC values
To:     David Hildenbrand <david@redhat.com>,
        Claudio Imbrenda <imbrenda@linux.ibm.com>, kvm@vger.kernel.org
Cc:     cohuck@redhat.com, borntraeger@de.ibm.com, thuth@redhat.com,
        pasic@linux.ibm.com, linux-s390@vger.kernel.org,
        linux-kernel@vger.kernel.org, Ulrich.Weigand@de.ibm.com
References: <20210804154046.88552-1-imbrenda@linux.ibm.com>
 <20210804154046.88552-2-imbrenda@linux.ibm.com>
 <f3fc81a7-ea71-56f6-16e0-e43fc36d646e@redhat.com>
From:   Janosch Frank <frankja@linux.ibm.com>
Message-ID: <54a3b55e-bf05-e661-0618-7839f3d2c8dd@linux.ibm.com>
Date:   Fri, 6 Aug 2021 17:15:46 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.11.0
MIME-Version: 1.0
In-Reply-To: <f3fc81a7-ea71-56f6-16e0-e43fc36d646e@redhat.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 8/6/21 9:26 AM, David Hildenbrand wrote:
> On 04.08.21 17:40, Claudio Imbrenda wrote:
>> Add macros to describe the 4 possible CC values returned by the UVC
>> instruction.
>>
>> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
>> ---
>>   arch/s390/include/asm/uv.h | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/arch/s390/include/asm/uv.h b/arch/s390/include/asm/uv.h
>> index 12c5f006c136..b35add51b967 100644
>> --- a/arch/s390/include/asm/uv.h
>> +++ b/arch/s390/include/asm/uv.h
>> @@ -18,6 +18,11 @@
>>   #include <asm/page.h>
>>   #include <asm/gmap.h>
>>   
>> +#define UVC_CC_OK	0
>> +#define UVC_CC_ERROR	1
>> +#define UVC_CC_BUSY 	2
>> +#define UVC_CC_PARTIAL	3
>> +
>>   #define UVC_RC_EXECUTED		0x0001
>>   #define UVC_RC_INV_CMD		0x0002
>>   #define UVC_RC_INV_STATE	0x0003
>>
> 
> Do we have any users we could directly fix up? AFAIKs, most users don't 
> really care about the cc value, only about cc vs !cc.
> 
> The only instances I was able to spot quickly:

The only fix for the functions below that I would accept would be to
check for cc 2 and 3. A cc >= UVC_CC_BUSY confuses me way too much when
reading.

But honestly for those two I'd just keep the code as is. I only asked
Claudio to fix the code in the next patch and add this patch as it was
not clearly visible he was dealing with a CC.

> 
> 
> diff --git a/arch/s390/include/asm/uv.h b/arch/s390/include/asm/uv.h
> index 12c5f006c136..dd72d325f9e8 100644
> --- a/arch/s390/include/asm/uv.h
> +++ b/arch/s390/include/asm/uv.h
> @@ -233,7 +233,7 @@ static inline int uv_call(unsigned long r1, unsigned 
> long r2)
> 
>          do {
>                  cc = __uv_call(r1, r2);
> -       } while (cc > 1);
> +       } while (cc >= UVC_CC_BUSY);
>          return cc;
>   }
> 
> @@ -245,7 +245,7 @@ static inline int uv_call_sched(unsigned long r1, 
> unsigned long r2)
>          do {
>                  cc = __uv_call(r1, r2);
>                  cond_resched();
> -       } while (cc > 1);
> +       } while (cc >= UVC_CC_BUSY);
>          return cc;
>   }
> 
> 
> Of course, we could replace all checks for cc vs !cc with "cc != 
> UVC_CC_OK" vs "cc == UVC_CC_OK".
>