Re: [PATCH v4 29/40] KVM: arm64: Prepare to handle deferred save/restore of 32-bit registers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Feb 15, 2018 at 10:03:21PM +0100, Christoffer Dall wrote:
> 32-bit registers are not used by a 64-bit host kernel and can be
> deferred, but we need to rework the accesses to this register to access

these registers

> the latest value depending on whether or not guest system registers are

values

> loaded on the CPU or only reside in memory.
> 
> Signed-off-by: Christoffer Dall <christoffer.dall@xxxxxxxxxx>
> ---
> 
> Notes:
>     Changes since v3:
>      - Don't also try to write hardware spsr when sysregs are not loaded
>      - Adapted patch to use switch-based sysreg save/restore approach
>      - (Kept additional BUG_ON() in vcpu_read_spsr32() to keep the compiler happy)
>     
>     Changes since v2:
>      - New patch (deferred register handling has been reworked)
> 
>  arch/arm64/include/asm/kvm_emulate.h | 32 +++++------------
>  arch/arm64/kvm/regmap.c              | 67 +++++++++++++++++++++++++++---------
>  arch/arm64/kvm/sys_regs.c            |  6 ++++
>  3 files changed, 65 insertions(+), 40 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
> index 9cb13b23c7a1..23b33e8ea03a 100644
> --- a/arch/arm64/include/asm/kvm_emulate.h
> +++ b/arch/arm64/include/asm/kvm_emulate.h
> @@ -33,7 +33,8 @@
>  #include <asm/virt.h>
>  
>  unsigned long *vcpu_reg32(const struct kvm_vcpu *vcpu, u8 reg_num);
> -unsigned long *vcpu_spsr32(const struct kvm_vcpu *vcpu);
> +unsigned long vcpu_read_spsr32(const struct kvm_vcpu *vcpu);
> +void vcpu_write_spsr32(struct kvm_vcpu *vcpu, unsigned long v);
>  
>  bool kvm_condition_valid32(const struct kvm_vcpu *vcpu);
>  void kvm_skip_instr32(struct kvm_vcpu *vcpu, bool is_wide_instr);
> @@ -162,41 +163,26 @@ static inline void vcpu_set_reg(struct kvm_vcpu *vcpu, u8 reg_num,
>  
>  static inline unsigned long vcpu_read_spsr(const struct kvm_vcpu *vcpu)
>  {
> -	unsigned long *p = (unsigned long *)&vcpu_gp_regs(vcpu)->spsr[KVM_SPSR_EL1];
> -
> -	if (vcpu_mode_is_32bit(vcpu)) {
> -		unsigned long *p_32bit = vcpu_spsr32(vcpu);
> -
> -		/* KVM_SPSR_SVC aliases KVM_SPSR_EL1 */
> -		if (p_32bit != (unsigned long *)p)
> -			return *p_32bit;
> -	}
> +	if (vcpu_mode_is_32bit(vcpu))
> +		return vcpu_read_spsr32(vcpu);
>  
>  	if (vcpu->arch.sysregs_loaded_on_cpu)
>  		return read_sysreg_el1(spsr);
>  	else
> -		return *p;
> +		return vcpu_gp_regs(vcpu)->spsr[KVM_SPSR_EL1];
>  }
>  
> -static inline void vcpu_write_spsr(const struct kvm_vcpu *vcpu, unsigned long v)
> +static inline void vcpu_write_spsr(struct kvm_vcpu *vcpu, unsigned long v)
>  {
> -	unsigned long *p = (unsigned long *)&vcpu_gp_regs(vcpu)->spsr[KVM_SPSR_EL1];
> -
> -	/* KVM_SPSR_SVC aliases KVM_SPSR_EL1 */
>  	if (vcpu_mode_is_32bit(vcpu)) {
> -		unsigned long *p_32bit = vcpu_spsr32(vcpu);
> -
> -		/* KVM_SPSR_SVC aliases KVM_SPSR_EL1 */
> -		if (p_32bit != (unsigned long *)p) {
> -			*p_32bit = v;
> -			return;
> -		}
> +		vcpu_write_spsr32(vcpu, v);
> +		return;
>  	}
>  
>  	if (vcpu->arch.sysregs_loaded_on_cpu)
>  		write_sysreg_el1(v, spsr);
>  	else
> -		*p = v;
> +		vcpu_gp_regs(vcpu)->spsr[KVM_SPSR_EL1] = v;
>  }
>  
>  static inline bool vcpu_mode_priv(const struct kvm_vcpu *vcpu)
> diff --git a/arch/arm64/kvm/regmap.c b/arch/arm64/kvm/regmap.c
> index bbc6ae32e4af..eefe403a2e63 100644
> --- a/arch/arm64/kvm/regmap.c
> +++ b/arch/arm64/kvm/regmap.c
> @@ -141,28 +141,61 @@ unsigned long *vcpu_reg32(const struct kvm_vcpu *vcpu, u8 reg_num)
>  /*
>   * Return the SPSR for the current mode of the virtual CPU.
>   */
> -unsigned long *vcpu_spsr32(const struct kvm_vcpu *vcpu)
> +static int vcpu_spsr32_mode(const struct kvm_vcpu *vcpu)
>  {
>  	unsigned long mode = *vcpu_cpsr(vcpu) & COMPAT_PSR_MODE_MASK;
>  	switch (mode) {
> -	case COMPAT_PSR_MODE_SVC:
> -		mode = KVM_SPSR_SVC;
> -		break;
> -	case COMPAT_PSR_MODE_ABT:
> -		mode = KVM_SPSR_ABT;
> -		break;
> -	case COMPAT_PSR_MODE_UND:
> -		mode = KVM_SPSR_UND;
> -		break;
> -	case COMPAT_PSR_MODE_IRQ:
> -		mode = KVM_SPSR_IRQ;
> -		break;
> -	case COMPAT_PSR_MODE_FIQ:
> -		mode = KVM_SPSR_FIQ;
> -		break;
> +	case COMPAT_PSR_MODE_SVC: return KVM_SPSR_SVC;
> +	case COMPAT_PSR_MODE_ABT: return KVM_SPSR_ABT;
> +	case COMPAT_PSR_MODE_UND: return KVM_SPSR_UND;
> +	case COMPAT_PSR_MODE_IRQ: return KVM_SPSR_IRQ;
> +	case COMPAT_PSR_MODE_FIQ: return KVM_SPSR_FIQ;
> +	default: BUG();
> +	}
> +}
> +
> +unsigned long vcpu_read_spsr32(const struct kvm_vcpu *vcpu)
> +{
> +	int spsr_idx = vcpu_spsr32_mode(vcpu);
> +
> +	if (!vcpu->arch.sysregs_loaded_on_cpu)
> +		return vcpu_gp_regs(vcpu)->spsr[spsr_idx];
> +
> +	switch (spsr_idx) {
> +	case KVM_SPSR_SVC:
> +		return read_sysreg_el1(spsr);
> +	case KVM_SPSR_ABT:
> +		return read_sysreg(spsr_abt);
> +	case KVM_SPSR_UND:
> +		return read_sysreg(spsr_und);
> +	case KVM_SPSR_IRQ:
> +		return read_sysreg(spsr_irq);
> +	case KVM_SPSR_FIQ:
> +		return read_sysreg(spsr_fiq);
>  	default:
>  		BUG();
>  	}
> +}
> +
> +void vcpu_write_spsr32(struct kvm_vcpu *vcpu, unsigned long v)
> +{
> +	int spsr_idx = vcpu_spsr32_mode(vcpu);
> +
> +	if (!vcpu->arch.sysregs_loaded_on_cpu) {
> +		vcpu_gp_regs(vcpu)->spsr[spsr_idx] = v;
> +		return;
> +	}
>  
> -	return (unsigned long *)&vcpu_gp_regs(vcpu)->spsr[mode];
> +	switch (spsr_idx) {
> +	case KVM_SPSR_SVC:
> +		write_sysreg_el1(v, spsr);
> +	case KVM_SPSR_ABT:
> +		write_sysreg(v, spsr_abt);
> +	case KVM_SPSR_UND:
> +		write_sysreg(v, spsr_und);
> +	case KVM_SPSR_IRQ:
> +		write_sysreg(v, spsr_irq);
> +	case KVM_SPSR_FIQ:
> +		write_sysreg(v, spsr_fiq);
> +	}
>  }
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index f060309337aa..d2324560c9f5 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -107,6 +107,9 @@ u64 vcpu_read_sys_reg(struct kvm_vcpu *vcpu, int reg)
>  	case AMAIR_EL1:		return read_sysreg_s(amair_EL12);
>  	case CNTKCTL_EL1:	return read_sysreg_s(cntkctl_EL12);
>  	case PAR_EL1:		return read_sysreg_s(SYS_PAR_EL1);
> +	case DACR32_EL2:	return read_sysreg_s(SYS_DACR32_EL2);
> +	case IFSR32_EL2:	return read_sysreg_s(SYS_IFSR32_EL2);
> +	case DBGVCR32_EL2:	return read_sysreg_s(SYS_DBGVCR32_EL2);
>  	}
>  
>  immediate_read:
> @@ -143,6 +146,9 @@ void vcpu_write_sys_reg(struct kvm_vcpu *vcpu, int reg, u64 val)
>  	case AMAIR_EL1:		write_sysreg_s(val, amair_EL12);	return;
>  	case CNTKCTL_EL1:	write_sysreg_s(val, cntkctl_EL12);	return;
>  	case PAR_EL1:		write_sysreg_s(val, SYS_PAR_EL1);	return;
> +	case DACR32_EL2:	write_sysreg_s(val, SYS_DACR32_EL2);	return;
> +	case IFSR32_EL2:	write_sysreg_s(val, SYS_IFSR32_EL2);	return;
> +	case DBGVCR32_EL2:	write_sysreg_s(val, SYS_DBGVCR32_EL2);	return;
>  	}
>  
>  immediate_write:
> -- 
> 2.14.2
>

I think the last two hunks would fit better in the next patch. It's not a
huge deal, though. If they do get moved, then I guess the patch summary
and commit message should only focus on sprs32 - returning its grammar to
the singular case.

Reviewed-by: Andrew Jones <drjones@xxxxxxxxxx>

Thanks,
drew



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux