[v2,2/3] kvm: arm64: handle single-stepping trapped instructions

Message ID 20171109170021.2984-3-alex.bennee@linaro.org
State New
Headers show
Series
  • KVM: arm64: single step emulation instructions
Related show

Commit Message

Alex Bennée Nov. 9, 2017, 5 p.m.
If we are using guest debug to single-step the guest we need to ensure
we exit after emulating the instruction. This only affects
instructions completely emulated by the kernel. For userspace emulated
instructions we need to exit and return to complete the emulation.

The kvm_arm_handle_step_debug() helper sets up the necessary exit
state if needed.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---
v2
  - use helper from patch 1
  - if (handled > 0) instead of if (handled) so errors propagate
---
 arch/arm64/kvm/handle_exit.c | 47 +++++++++++++++++++++++++++++++-------------
 1 file changed, 33 insertions(+), 14 deletions(-)

Comments

Julien Thierry Nov. 13, 2017, 11:02 a.m. | #1
On 09/11/17 17:00, Alex Bennée wrote:
> If we are using guest debug to single-step the guest we need to ensure
> we exit after emulating the instruction. This only affects
> instructions completely emulated by the kernel. For userspace emulated
> instructions we need to exit and return to complete the emulation.
> 
> The kvm_arm_handle_step_debug() helper sets up the necessary exit
> state if needed.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

Reviewed-by: Julien Thierry <julien.thierry@arm.com>

> 
> ---
> v2
>    - use helper from patch 1
>    - if (handled > 0) instead of if (handled) so errors propagate
> ---
>   arch/arm64/kvm/handle_exit.c | 47 +++++++++++++++++++++++++++++++-------------
>   1 file changed, 33 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
> index 7debb74843a0..af1c804742f6 100644
> --- a/arch/arm64/kvm/handle_exit.c
> +++ b/arch/arm64/kvm/handle_exit.c
> @@ -178,6 +178,38 @@ static exit_handle_fn kvm_get_exit_handler(struct kvm_vcpu *vcpu)
>   	return arm_exit_handlers[hsr_ec];
>   }
>   
> +/*
> + * We may be single-stepping an emulated instruction. If the emulation
> + * has been completed in-kernel we can return to userspace with a
> + * KVM_EXIT_DEBUG, otherwise the userspace needs to complete its
> + * emulation first.
> + */
> +
> +static int handle_trap_exceptions(struct kvm_vcpu *vcpu, struct kvm_run *run)
> +{
> +	int handled;
> +
> +	/*
> +	 * See ARM ARM B1.14.1: "Hyp traps on instructions
> +	 * that fail their condition code check"
> +	 */
> +	if (!kvm_condition_valid(vcpu)) {
> +		kvm_skip_instr(vcpu, kvm_vcpu_trap_il_is32bit(vcpu));
> +		handled = 1;
> +	} else {
> +		exit_handle_fn exit_handler;
> +
> +		exit_handler = kvm_get_exit_handler(vcpu);
> +		handled = exit_handler(vcpu, run);
> +	}
> +
> +	/* helper sets exit_reason if we need to return to userspace */
> +	if (handled > 0 && kvm_arm_handle_step_debug(vcpu, run))
> +		handled = 0;
> +
> +	return handled;
> +}
> +
>   /*
>    * Return > 0 to return to guest, < 0 on error, 0 (and set exit_reason) on
>    * proper exit to userspace.
> @@ -185,8 +217,6 @@ static exit_handle_fn kvm_get_exit_handler(struct kvm_vcpu *vcpu)
>   int handle_exit(struct kvm_vcpu *vcpu, struct kvm_run *run,
>   		       int exception_index)
>   {
> -	exit_handle_fn exit_handler;
> -
>   	if (ARM_SERROR_PENDING(exception_index)) {
>   		u8 hsr_ec = ESR_ELx_EC(kvm_vcpu_get_hsr(vcpu));
>   
> @@ -214,18 +244,7 @@ int handle_exit(struct kvm_vcpu *vcpu, struct kvm_run *run,
>   		kvm_inject_vabt(vcpu);
>   		return 1;
>   	case ARM_EXCEPTION_TRAP:
> -		/*
> -		 * See ARM ARM B1.14.1: "Hyp traps on instructions
> -		 * that fail their condition code check"
> -		 */
> -		if (!kvm_condition_valid(vcpu)) {
> -			kvm_skip_instr(vcpu, kvm_vcpu_trap_il_is32bit(vcpu));
> -			return 1;
> -		}
> -
> -		exit_handler = kvm_get_exit_handler(vcpu);
> -
> -		return exit_handler(vcpu, run);
> +		return handle_trap_exceptions(vcpu, run);
>   	case ARM_EXCEPTION_HYP_GONE:
>   		/*
>   		 * EL2 has been reset to the hyp-stub. This happens when a guest
>

Patch

diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index 7debb74843a0..af1c804742f6 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -178,6 +178,38 @@  static exit_handle_fn kvm_get_exit_handler(struct kvm_vcpu *vcpu)
 	return arm_exit_handlers[hsr_ec];
 }
 
+/*
+ * We may be single-stepping an emulated instruction. If the emulation
+ * has been completed in-kernel we can return to userspace with a
+ * KVM_EXIT_DEBUG, otherwise the userspace needs to complete its
+ * emulation first.
+ */
+
+static int handle_trap_exceptions(struct kvm_vcpu *vcpu, struct kvm_run *run)
+{
+	int handled;
+
+	/*
+	 * See ARM ARM B1.14.1: "Hyp traps on instructions
+	 * that fail their condition code check"
+	 */
+	if (!kvm_condition_valid(vcpu)) {
+		kvm_skip_instr(vcpu, kvm_vcpu_trap_il_is32bit(vcpu));
+		handled = 1;
+	} else {
+		exit_handle_fn exit_handler;
+
+		exit_handler = kvm_get_exit_handler(vcpu);
+		handled = exit_handler(vcpu, run);
+	}
+
+	/* helper sets exit_reason if we need to return to userspace */
+	if (handled > 0 && kvm_arm_handle_step_debug(vcpu, run))
+		handled = 0;
+
+	return handled;
+}
+
 /*
  * Return > 0 to return to guest, < 0 on error, 0 (and set exit_reason) on
  * proper exit to userspace.
@@ -185,8 +217,6 @@  static exit_handle_fn kvm_get_exit_handler(struct kvm_vcpu *vcpu)
 int handle_exit(struct kvm_vcpu *vcpu, struct kvm_run *run,
 		       int exception_index)
 {
-	exit_handle_fn exit_handler;
-
 	if (ARM_SERROR_PENDING(exception_index)) {
 		u8 hsr_ec = ESR_ELx_EC(kvm_vcpu_get_hsr(vcpu));
 
@@ -214,18 +244,7 @@  int handle_exit(struct kvm_vcpu *vcpu, struct kvm_run *run,
 		kvm_inject_vabt(vcpu);
 		return 1;
 	case ARM_EXCEPTION_TRAP:
-		/*
-		 * See ARM ARM B1.14.1: "Hyp traps on instructions
-		 * that fail their condition code check"
-		 */
-		if (!kvm_condition_valid(vcpu)) {
-			kvm_skip_instr(vcpu, kvm_vcpu_trap_il_is32bit(vcpu));
-			return 1;
-		}
-
-		exit_handler = kvm_get_exit_handler(vcpu);
-
-		return exit_handler(vcpu, run);
+		return handle_trap_exceptions(vcpu, run);
 	case ARM_EXCEPTION_HYP_GONE:
 		/*
 		 * EL2 has been reset to the hyp-stub. This happens when a guest