[PATCH] ARM: KVM: Cleanup exception injection

[christoffer.dall@linaro.org (Christoffer Dall)]

On Mon, Dec 14, 2015 at 06:06:36PM +0000, Marc Zyngier wrote:
> David Binderman reported that the exception injection code had a
> couple of unused variables lingering around.
> 
> Upon examination, it looked like this code could do with an
> anticipated spring cleaning, which amounts to deduplicating
> the CPSR/SPSR update.
> 
> The spurious variables are removed in the process.
> 
> Reported-by: David Binderman <dcb314@hotmail.com>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm/kvm/emulate.c | 59 ++++++++++++++++++++------------------------------
>  1 file changed, 23 insertions(+), 36 deletions(-)
> 
> diff --git a/arch/arm/kvm/emulate.c b/arch/arm/kvm/emulate.c
> index d6c0052..245106e 100644
> --- a/arch/arm/kvm/emulate.c
> +++ b/arch/arm/kvm/emulate.c
> @@ -275,6 +275,25 @@ static u32 exc_vector_base(struct kvm_vcpu *vcpu)
>  		return vbar;
>  }
>  
> +/* Switch to an exception mode, updating both CPSR and SPSR */
> +static void kvm_update_psr(struct kvm_vcpu *vcpu, unsigned long mode)
> +{
> +	unsigned long cpsr = *vcpu_cpsr(vcpu);
> +	u32 sctlr = vcpu->arch.cp15[c1_SCTLR];
> +
> +	*vcpu_cpsr(vcpu) = (cpsr & ~MODE_MASK) | mode;
> +	*vcpu_cpsr(vcpu) |= PSR_I_BIT;
> +	*vcpu_cpsr(vcpu) &= ~(PSR_IT_MASK | PSR_J_BIT | PSR_E_BIT | PSR_T_BIT);
> +
> +	if (sctlr & SCTLR_TE)
> +		*vcpu_cpsr(vcpu) |= PSR_T_BIT;
> +	if (sctlr & SCTLR_EE)
> +		*vcpu_cpsr(vcpu) |= PSR_E_BIT;
> +
> +	/* Note: These now point to the mode banked copies */
> +	*vcpu_spsr(vcpu) = cpsr;
> +}
> +
>  /**
>   * kvm_inject_undefined - inject an undefined exception into the guest
>   * @vcpu: The VCPU to receive the undefined exception
> @@ -286,29 +305,13 @@ static u32 exc_vector_base(struct kvm_vcpu *vcpu)
>   */
>  void kvm_inject_undefined(struct kvm_vcpu *vcpu)
>  {
> -	unsigned long new_lr_value;
> -	unsigned long new_spsr_value;
>  	unsigned long cpsr = *vcpu_cpsr(vcpu);
> -	u32 sctlr = vcpu->arch.cp15[c1_SCTLR];
>  	bool is_thumb = (cpsr & PSR_T_BIT);
>  	u32 vect_offset = 4;
>  	u32 return_offset = (is_thumb) ? 2 : 4;
>  
> -	new_spsr_value = cpsr;
> -	new_lr_value = *vcpu_pc(vcpu) - return_offset;
> -
> -	*vcpu_cpsr(vcpu) = (cpsr & ~MODE_MASK) | UND_MODE;
> -	*vcpu_cpsr(vcpu) |= PSR_I_BIT;
> -	*vcpu_cpsr(vcpu) &= ~(PSR_IT_MASK | PSR_J_BIT | PSR_E_BIT | PSR_T_BIT);
> -
> -	if (sctlr & SCTLR_TE)
> -		*vcpu_cpsr(vcpu) |= PSR_T_BIT;
> -	if (sctlr & SCTLR_EE)
> -		*vcpu_cpsr(vcpu) |= PSR_E_BIT;
> -
> -	/* Note: These now point to UND banked copies */
> -	*vcpu_spsr(vcpu) = cpsr;
> -	*vcpu_reg(vcpu, 14) = new_lr_value;
> +	kvm_update_psr(vcpu, UND_MODE);
> +	*vcpu_reg(vcpu, 14) = *vcpu_pc(vcpu) - return_offset;
>  
>  	/* Branch to exception vector */
>  	*vcpu_pc(vcpu) = exc_vector_base(vcpu) + vect_offset;
> @@ -320,30 +323,14 @@ void kvm_inject_undefined(struct kvm_vcpu *vcpu)
>   */
>  static void inject_abt(struct kvm_vcpu *vcpu, bool is_pabt, unsigned long addr)
>  {
> -	unsigned long new_lr_value;
> -	unsigned long new_spsr_value;
>  	unsigned long cpsr = *vcpu_cpsr(vcpu);
> -	u32 sctlr = vcpu->arch.cp15[c1_SCTLR];
>  	bool is_thumb = (cpsr & PSR_T_BIT);
>  	u32 vect_offset;
>  	u32 return_offset = (is_thumb) ? 4 : 0;
>  	bool is_lpae;
>  
> -	new_spsr_value = cpsr;
> -	new_lr_value = *vcpu_pc(vcpu) + return_offset;
> -
> -	*vcpu_cpsr(vcpu) = (cpsr & ~MODE_MASK) | ABT_MODE;
> -	*vcpu_cpsr(vcpu) |= PSR_I_BIT | PSR_A_BIT;

seems like you're losing the PSR_A_BIT in kvm_update_psr() now?

> -	*vcpu_cpsr(vcpu) &= ~(PSR_IT_MASK | PSR_J_BIT | PSR_E_BIT | PSR_T_BIT);
> -
> -	if (sctlr & SCTLR_TE)
> -		*vcpu_cpsr(vcpu) |= PSR_T_BIT;
> -	if (sctlr & SCTLR_EE)
> -		*vcpu_cpsr(vcpu) |= PSR_E_BIT;
> -
> -	/* Note: These now point to ABT banked copies */
> -	*vcpu_spsr(vcpu) = cpsr;
> -	*vcpu_reg(vcpu, 14) = new_lr_value;
> +	kvm_update_psr(vcpu, ABT_MODE);
> +	*vcpu_reg(vcpu, 14) = *vcpu_pc(vcpu) + return_offset;
>  
>  	if (is_pabt)
>  		vect_offset = 12;
> -- 
> 2.1.4
> 

Otherwise looks like a welcome cleanup!

Thanks,
-Christoffer