[PATCH] arm64: kvm: Stop clobbering x0 for HVC_SOFT_RESTART

[PATCH] arm64: kvm: Stop clobbering x0 for HVC_SOFT_RESTART

[Andrew Scull]

HVC_SOFT_RESTART is given values for x0-2 that it should installed
before exiting to the new address so should not set x0 to stub HVC
success or failure code.

Signed-off-by: Andrew Scull <ascull@google.com>
---
 arch/arm64/kvm/hyp-init.S | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/kvm/hyp-init.S b/arch/arm64/kvm/hyp-init.S
index 6e6ed5581eed..e76c0e89d48e 100644
--- a/arch/arm64/kvm/hyp-init.S
+++ b/arch/arm64/kvm/hyp-init.S
@@ -136,11 +136,15 @@ SYM_CODE_START(__kvm_handle_stub_hvc)
 
 1:	cmp	x0, #HVC_RESET_VECTORS
 	b.ne	1f
-reset:
+
 	/*
-	 * Reset kvm back to the hyp stub. Do not clobber x0-x4 in
-	 * case we coming via HVC_SOFT_RESTART.
+	 * Set the HVC_RESET_VECTORS return code before entering the common
+	 * path so that we do not clobber x0-x2 in case we are coming via
+	 * HVC_SOFT_RESTART.
 	 */
+	mov	x0, xzr
+reset:
+	/* Reset kvm back to the hyp stub. */
 	mrs	x5, sctlr_el2
 	mov_q	x6, SCTLR_ELx_FLAGS
 	bic	x5, x5, x6		// Clear SCTL_M and etc
@@ -151,7 +155,6 @@ reset:
 	/* Install stub vectors */
 	adr_l	x5, __hyp_stub_vectors
 	msr	vbar_el2, x5
-	mov	x0, xzr
 	eret
 
 1:	/* Bad stub call */
-- 
2.27.0.383.g050319c2ae-goog

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH] arm64: kvm: Stop clobbering x0 for HVC_SOFT_RESTART

[Marc Zyngier]

On 2020-07-06 10:52, Andrew Scull wrote:
> HVC_SOFT_RESTART is given values for x0-2 that it should installed
> before exiting to the new address so should not set x0 to stub HVC
> success or failure code.
> 
> Signed-off-by: Andrew Scull <ascull@google.com>
> ---
>  arch/arm64/kvm/hyp-init.S | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/kvm/hyp-init.S b/arch/arm64/kvm/hyp-init.S
> index 6e6ed5581eed..e76c0e89d48e 100644
> --- a/arch/arm64/kvm/hyp-init.S
> +++ b/arch/arm64/kvm/hyp-init.S
> @@ -136,11 +136,15 @@ SYM_CODE_START(__kvm_handle_stub_hvc)
> 
>  1:	cmp	x0, #HVC_RESET_VECTORS
>  	b.ne	1f
> -reset:
> +
>  	/*
> -	 * Reset kvm back to the hyp stub. Do not clobber x0-x4 in
> -	 * case we coming via HVC_SOFT_RESTART.
> +	 * Set the HVC_RESET_VECTORS return code before entering the common
> +	 * path so that we do not clobber x0-x2 in case we are coming via
> +	 * HVC_SOFT_RESTART.
>  	 */
> +	mov	x0, xzr
> +reset:
> +	/* Reset kvm back to the hyp stub. */
>  	mrs	x5, sctlr_el2
>  	mov_q	x6, SCTLR_ELx_FLAGS
>  	bic	x5, x5, x6		// Clear SCTL_M and etc
> @@ -151,7 +155,6 @@ reset:
>  	/* Install stub vectors */
>  	adr_l	x5, __hyp_stub_vectors
>  	msr	vbar_el2, x5
> -	mov	x0, xzr
>  	eret
> 
>  1:	/* Bad stub call */

Huh, nice catch. I wonder what the fuss is about kexec, really,
given that it is *that* broken. This deserves:

Fixes: af42f20480bf1 ("arm64: hyp-stub: Zero x0 on successful stub 
handling")
Cc: stable@vger.kernel.org

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH] arm64: kvm: Stop clobbering x0 for HVC_SOFT_RESTART

[Marc Zyngier]

On Mon, 6 Jul 2020 10:52:59 +0100, Andrew Scull wrote:
> HVC_SOFT_RESTART is given values for x0-2 that it should installed
> before exiting to the new address so should not set x0 to stub HVC
> success or failure code.

Applied to kvm-arm64/fixes-5.8-3, thanks!

[1/1] KVM: arm64: Stop clobbering x0 for HVC_SOFT_RESTART
      commit: e86a688bce9e2c77087c797504dd3dc97d688a62

Cheers,

	M.
-- 
Without deviation from the norm, progress is not possible.


_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH] arm64: kvm: Stop clobbering x0 for HVC_SOFT_RESTART

[James Morse]

Hi Marc, Andrew,

On 06/07/2020 11:11, Marc Zyngier wrote:
> On 2020-07-06 10:52, Andrew Scull wrote:
>> HVC_SOFT_RESTART is given values for x0-2 that it should installed
>> before exiting to the new address so should not set x0 to stub HVC
>> success or failure code.

>> diff --git a/arch/arm64/kvm/hyp-init.S b/arch/arm64/kvm/hyp-init.S
>> index 6e6ed5581eed..e76c0e89d48e 100644
>> --- a/arch/arm64/kvm/hyp-init.S
>> +++ b/arch/arm64/kvm/hyp-init.S
>> @@ -136,11 +136,15 @@ SYM_CODE_START(__kvm_handle_stub_hvc)
>>
>>  1:    cmp    x0, #HVC_RESET_VECTORS
>>      b.ne    1f
>> -reset:
>> +
>>      /*
>> -     * Reset kvm back to the hyp stub. Do not clobber x0-x4 in
>> -     * case we coming via HVC_SOFT_RESTART.
>> +     * Set the HVC_RESET_VECTORS return code before entering the common
>> +     * path so that we do not clobber x0-x2 in case we are coming via
>> +     * HVC_SOFT_RESTART.
>>       */
>> +    mov    x0, xzr
>> +reset:
>> +    /* Reset kvm back to the hyp stub. */
>>      mrs    x5, sctlr_el2
>>      mov_q    x6, SCTLR_ELx_FLAGS
>>      bic    x5, x5, x6        // Clear SCTL_M and etc
>> @@ -151,7 +155,6 @@ reset:
>>      /* Install stub vectors */
>>      adr_l    x5, __hyp_stub_vectors
>>      msr    vbar_el2, x5
>> -    mov    x0, xzr
>>      eret
>>
>>  1:    /* Bad stub call */

> Huh, nice catch. I wonder what the fuss is about kexec, really,
> given that it is *that* broken.

This would only bite kdump on a v8.0 machine was also running a KVM guest.

Regular kexec would happen after KVM's kvm_reboot_notifier() has called
hardware_disable_nolock() which unloads KVM and restores the hyp-stub.

I'm glad its been caught and fixed!


Thanks,

James
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm