[PATCH v2] KVM: s390: Remove false WARN_ON_ONCE for the PQAP instruction

[PATCH v2] KVM: s390: Remove false WARN_ON_ONCE for the PQAP instruction

[Christian Borntraeger]

In LPAR we will only get an intercept for FC==3 for the PQAP
instruction. Running nested under z/VM can result in other intercepts as
well as ECA_APIE is an effective bit: If one hypervisor layer has
turned this bit off, the end result will be that we will get intercepts for
all function codes. Usually the first one will be a query like PQAP(QCI).
So the WARN_ON_ONCE is not right. Let us simply remove it.

Cc: Pierre Morel <pmorel@linux.ibm.com>
Cc: Tony Krowiak <akrowiak@linux.ibm.com>
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/kvm/20200505073525.2287-1-borntraeger@de.ibm.com
Reported-by: Qian Cai <cailca@icloud.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
---
 arch/s390/kvm/priv.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/s390/kvm/priv.c b/arch/s390/kvm/priv.c
index 69a824f9ef0b..893893642415 100644
--- a/arch/s390/kvm/priv.c
+++ b/arch/s390/kvm/priv.c
@@ -626,10 +626,12 @@ static int handle_pqap(struct kvm_vcpu *vcpu)
 	 * available for the guest are AQIC and TAPQ with the t bit set
 	 * since we do not set IC.3 (FIII) we currently will only intercept
 	 * the AQIC function code.
+	 * Note: running nested under z/VM can result in intercepts for other
+	 * function codes, e.g. PQAP(QCI). We do not support this and bail out.
 	 */
 	reg0 = vcpu->run->s.regs.gprs[0];
 	fc = (reg0 >> 24) & 0xff;
-	if (WARN_ON_ONCE(fc != 0x03))
+	if (fc != 0x03)
 		return -EOPNOTSUPP;
 
 	/* PQAP instruction is allowed for guest kernel only */
-- 
2.25.4

Re: [PATCH v2] KVM: s390: Remove false WARN_ON_ONCE for the PQAP instruction

[Cornelia Huck]

On Tue,  5 May 2020 10:35:15 +0200
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> In LPAR we will only get an intercept for FC==3 for the PQAP
> instruction. Running nested under z/VM can result in other intercepts as
> well as ECA_APIE is an effective bit: If one hypervisor layer has
> turned this bit off, the end result will be that we will get intercepts for
> all function codes. Usually the first one will be a query like PQAP(QCI).
> So the WARN_ON_ONCE is not right. Let us simply remove it.

Thanks, that is helpful to describe.

Fixes: e5282de93105 ("s390: ap: kvm: add PQAP interception for AQIC")

> Cc: Pierre Morel <pmorel@linux.ibm.com>
> Cc: Tony Krowiak <akrowiak@linux.ibm.com>
> Cc: stable@vger.kernel.org
> Link: https://lore.kernel.org/kvm/20200505073525.2287-1-borntraeger@de.ibm.com

This links to v1, which is probably not what you want :)

> Reported-by: Qian Cai <cailca@icloud.com>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Reviewed-by: David Hildenbrand <david@redhat.com>
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> ---
>  arch/s390/kvm/priv.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/s390/kvm/priv.c b/arch/s390/kvm/priv.c
> index 69a824f9ef0b..893893642415 100644
> --- a/arch/s390/kvm/priv.c
> +++ b/arch/s390/kvm/priv.c
> @@ -626,10 +626,12 @@ static int handle_pqap(struct kvm_vcpu *vcpu)
>  	 * available for the guest are AQIC and TAPQ with the t bit set
>  	 * since we do not set IC.3 (FIII) we currently will only intercept
>  	 * the AQIC function code.
> +	 * Note: running nested under z/VM can result in intercepts for other
> +	 * function codes, e.g. PQAP(QCI). We do not support this and bail out.
>  	 */
>  	reg0 = vcpu->run->s.regs.gprs[0];
>  	fc = (reg0 >> 24) & 0xff;
> -	if (WARN_ON_ONCE(fc != 0x03))
> +	if (fc != 0x03)
>  		return -EOPNOTSUPP;
>  
>  	/* PQAP instruction is allowed for guest kernel only */

Re: [PATCH v2] KVM: s390: Remove false WARN_ON_ONCE for the PQAP instruction

[David Hildenbrand]

On 05.05.20 10:46, Cornelia Huck wrote:
> On Tue,  5 May 2020 10:35:15 +0200
> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> 
>> In LPAR we will only get an intercept for FC==3 for the PQAP
>> instruction. Running nested under z/VM can result in other intercepts as
>> well as ECA_APIE is an effective bit: If one hypervisor layer has
>> turned this bit off, the end result will be that we will get intercepts for
>> all function codes. Usually the first one will be a query like PQAP(QCI).
>> So the WARN_ON_ONCE is not right. Let us simply remove it.
> 
> Thanks, that is helpful to describe.
> 
> Fixes: e5282de93105 ("s390: ap: kvm: add PQAP interception for AQIC")

And maybe add a # v5.3+ to the stable-cc.

-- 
Thanks,

David / dhildenb

Re: [PATCH v2] KVM: s390: Remove false WARN_ON_ONCE for the PQAP instruction

[Christian Borntraeger]

On 05.05.20 10:35, Christian Borntraeger wrote:
> In LPAR we will only get an intercept for FC==3 for the PQAP
> instruction. Running nested under z/VM can result in other intercepts as
> well as ECA_APIE is an effective bit: If one hypervisor layer has
> turned this bit off, the end result will be that we will get intercepts for
> all function codes. Usually the first one will be a query like PQAP(QCI).
> So the WARN_ON_ONCE is not right. Let us simply remove it.
> 
> Cc: Pierre Morel <pmorel@linux.ibm.com>
> Cc: Tony Krowiak <akrowiak@linux.ibm.com>
> Cc: stable@vger.kernel.org
> Link: https://lore.kernel.org/kvm/20200505073525.2287-1-borntraeger@de.ibm.com
> Reported-by: Qian Cai <cailca@icloud.com>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Reviewed-by: David Hildenbrand <david@redhat.com>
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> ---
>  arch/s390/kvm/priv.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/s390/kvm/priv.c b/arch/s390/kvm/priv.c
> index 69a824f9ef0b..893893642415 100644
> --- a/arch/s390/kvm/priv.c
> +++ b/arch/s390/kvm/priv.c
> @@ -626,10 +626,12 @@ static int handle_pqap(struct kvm_vcpu *vcpu)
>  	 * available for the guest are AQIC and TAPQ with the t bit set
>  	 * since we do not set IC.3 (FIII) we currently will only intercept
>  	 * the AQIC function code.
> +	 * Note: running nested under z/VM can result in intercepts for other
> +	 * function codes, e.g. PQAP(QCI). We do not support this and bail out.
>  	 */
>  	reg0 = vcpu->run->s.regs.gprs[0];
>  	fc = (reg0 >> 24) & 0xff;
> -	if (WARN_ON_ONCE(fc != 0x03))
> +	if (fc != 0x03)
>  		return -EOPNOTSUPP;
>  
>  	/* PQAP instruction is allowed for guest kernel only */
> 

applied for kvms390/master.

Qian Cai, can you verify that this fixes the issue?

Re: [PATCH v2] KVM: s390: Remove false WARN_ON_ONCE for the PQAP instruction

[Qian Cai]

> On May 5, 2020, at 5:15 AM, Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> applied for kvms390/master.
> 
> Qian Cai, can you verify that this fixes the issue?

Thank you for tracking it down and removed the warning, so there is no way for me to trigger it anymore. Otherwise, my simple test case works fine for z/VM nested KVM here.