diff mbox series

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

Message ID 20200505083515.2720-1-borntraeger@de.ibm.com (mailing list archive)
State New, archived
Headers show
Series [v2] KVM: s390: Remove false WARN_ON_ONCE for the PQAP instruction | expand

Commit Message

Christian Borntraeger May 5, 2020, 8:35 a.m. UTC
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(-)

Comments

Cornelia Huck May 5, 2020, 8:46 a.m. UTC | #1
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 */
David Hildenbrand May 5, 2020, 8:47 a.m. UTC | #2
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.
Christian Borntraeger May 5, 2020, 9:15 a.m. UTC | #3
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?
Qian Cai May 5, 2020, 3:03 p.m. UTC | #4
> 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.
diff mbox series

Patch

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 */