[PATCH v1 1/1] KVM: s390: pqap: fix AP status written to wrong half of GR1

[PATCH v1 1/1] KVM: s390: pqap: fix AP status written to wrong half of GR1

[Gautam Gala]

The AP Queue Status Word (APQSW) occupies bit positions 32-63 of GR1,
i.e. the low 32 bits of the 64-bit register, while the upper 32 bits
are reserved.

The no-hook fallback path in handle_pqap() synthesizes an APQSW and
copies struct ap_queue_status directly into GPR1 using memcpy(). On
big-endian s390 this places the APQSW in the upper 32 bits of the
register rather than in the architecturally defined APQSW field.

Use union ap_queue_status_reg and assign its value directly to GPR1.
This places APQSW in the correct half of the register and aligns the
fallback path with the register layout used by the AP infrastructure.

Fixes: e5282de93105 ("s390: ap: kvm: add PQAP interception for AQIC")
Reported-by: Christian Borntraeger <borntraeger@linux.ibm.com>
Signed-off-by: Gautam Gala <ggala@linux.ibm.com>
---
 arch/s390/kvm/s390/priv.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/s390/kvm/s390/priv.c b/arch/s390/kvm/s390/priv.c
index b3cb2c2c3aa7..ccb2a7830265 100644
--- a/arch/s390/kvm/s390/priv.c
+++ b/arch/s390/kvm/s390/priv.c
@@ -592,7 +592,7 @@ EXPORT_SYMBOL_FOR_MODULES(kvm_s390_is_gpa_in_memslot, "vfio_ap");
  */
 static int handle_pqap(struct kvm_vcpu *vcpu)
 {
-	struct ap_queue_status status = {};
+	union ap_queue_status_reg status = {};
 	crypto_hook pqap_hook;
 	unsigned long reg0;
 	int ret;
@@ -661,8 +661,8 @@ static int handle_pqap(struct kvm_vcpu *vcpu)
 	 * No hook means no driver to enable the SIE CRYCB and no queues.
 	 * We send this response to the guest.
 	 */
-	status.response_code = 0x01;
-	memcpy(&vcpu->run->s.regs.gprs[1], &status, sizeof(status));
+	status.status.response_code = 0x01;
+	vcpu->run->s.regs.gprs[1] = status.value;
 	kvm_s390_set_psw_cc(vcpu, 3);
 	return 0;
 }
-- 
2.56.0

Re: [PATCH v1 1/1] KVM: s390: pqap: fix AP status written to wrong half of GR1

[Christian Borntraeger]

Am 20.10.26 um 10:25 schrieb Gautam Gala:
> The AP Queue Status Word (APQSW) occupies bit positions 32-63 of GR1,
> i.e. the low 32 bits of the 64-bit register, while the upper 32 bits
> are reserved.
> 
> The no-hook fallback path in handle_pqap() synthesizes an APQSW and
> copies struct ap_queue_status directly into GPR1 using memcpy(). On
> big-endian s390 this places the APQSW in the upper 32 bits of the
> register rather than in the architecturally defined APQSW field.
> 
> Use union ap_queue_status_reg and assign its value directly to GPR1.
> This places APQSW in the correct half of the register and aligns the
> fallback path with the register layout used by the AP infrastructure.
> 
> Fixes: e5282de93105 ("s390: ap: kvm: add PQAP interception for AQIC")
> Reported-by: Christian Borntraeger <borntraeger@linux.ibm.com>
> Signed-off-by: Gautam Gala <ggala@linux.ibm.com>

Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>

Thanks. applied.

> ---
>   arch/s390/kvm/s390/priv.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/s390/kvm/s390/priv.c b/arch/s390/kvm/s390/priv.c
> index b3cb2c2c3aa7..ccb2a7830265 100644
> --- a/arch/s390/kvm/s390/priv.c
> +++ b/arch/s390/kvm/s390/priv.c
> @@ -592,7 +592,7 @@ EXPORT_SYMBOL_FOR_MODULES(kvm_s390_is_gpa_in_memslot, "vfio_ap");
>    */
>   static int handle_pqap(struct kvm_vcpu *vcpu)
>   {
> -	struct ap_queue_status status = {};
> +	union ap_queue_status_reg status = {};
>   	crypto_hook pqap_hook;
>   	unsigned long reg0;
>   	int ret;
> @@ -661,8 +661,8 @@ static int handle_pqap(struct kvm_vcpu *vcpu)
>   	 * No hook means no driver to enable the SIE CRYCB and no queues.
>   	 * We send this response to the guest.
>   	 */
> -	status.response_code = 0x01;
> -	memcpy(&vcpu->run->s.regs.gprs[1], &status, sizeof(status));
> +	status.status.response_code = 0x01;
> +	vcpu->run->s.regs.gprs[1] = status.value;
>   	kvm_s390_set_psw_cc(vcpu, 3);
>   	return 0;
>   }