Re: [RFC PATCH v1 10/30] KVM: arm64: Add accessors for hypervisor state in kvm_vcpu_arch

[Quentin Perret <qperret@google.com>]

On Friday 24 Sep 2021 at 13:53:39 (+0100), Fuad Tabba wrote:
> Some of the members of vcpu_arch represent state that belongs to
> the hypervisor. Future patches will factor these out into their
> own structure. To simplify the refactoring and make it easier to
> read, add accessors for the members of kvm_vcpu_arch that
> represent the hypervisor state.
> 
> Signed-off-by: Fuad Tabba <tabba@google.com>
> ---
>  arch/arm64/include/asm/kvm_emulate.h | 182 ++++++++++++++++++++++-----
>  arch/arm64/include/asm/kvm_host.h    |  38 ++++--
>  2 files changed, 181 insertions(+), 39 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
> index 7d09a9356d89..e095afeecd10 100644
> --- a/arch/arm64/include/asm/kvm_emulate.h
> +++ b/arch/arm64/include/asm/kvm_emulate.h
> @@ -41,9 +41,14 @@ void kvm_inject_vabt(struct kvm_vcpu *vcpu);
>  void kvm_inject_dabt(struct kvm_vcpu *vcpu, unsigned long addr);
>  void kvm_inject_pabt(struct kvm_vcpu *vcpu, unsigned long addr);
>  
> +static __always_inline bool hyp_state_el1_is_32bit(struct vcpu_hyp_state *vcpu_hyps)
> +{
> +	return !(hyp_state_hcr_el2(vcpu_hyps) & HCR_RW);
> +}
> +
>  static __always_inline bool vcpu_el1_is_32bit(struct kvm_vcpu *vcpu)
>  {
> -	return !(vcpu_hcr_el2(vcpu) & HCR_RW);
> +	return hyp_state_el1_is_32bit(&hyp_state(vcpu));
>  }
>  
>  static inline void vcpu_reset_hcr(struct kvm_vcpu *vcpu)
> @@ -252,14 +257,19 @@ static inline bool vcpu_mode_priv(const struct kvm_vcpu *vcpu)
>  	return mode != PSR_MODE_EL0t;
>  }
>  
> +static __always_inline u32 kvm_hyp_state_get_esr(const struct vcpu_hyp_state *vcpu_hyps)
> +{
> +	return hyp_state_fault(vcpu_hyps).esr_el2;
> +}
> +
>  static __always_inline u32 kvm_vcpu_get_esr(const struct kvm_vcpu *vcpu)
>  {
> -	return vcpu_fault(vcpu).esr_el2;
> +	return kvm_hyp_state_get_esr(&hyp_state(vcpu));
>  }
>  
> -static __always_inline int kvm_vcpu_get_condition(const struct kvm_vcpu *vcpu)
> +static __always_inline u32 kvm_hyp_state_get_condition(const struct vcpu_hyp_state *vcpu_hyps)
>  {
> -	u32 esr = kvm_vcpu_get_esr(vcpu);
> +	u32 esr = kvm_hyp_state_get_esr(vcpu_hyps);
>  
>  	if (esr & ESR_ELx_CV)
>  		return (esr & ESR_ELx_COND_MASK) >> ESR_ELx_COND_SHIFT;
> @@ -267,111 +277,216 @@ static __always_inline int kvm_vcpu_get_condition(const struct kvm_vcpu *vcpu)
>  	return -1;
>  }
>  
> +static __always_inline int kvm_vcpu_get_condition(const struct kvm_vcpu *vcpu)
> +{
> +	return kvm_hyp_state_get_condition(&hyp_state(vcpu));
> +}
> +
> +static __always_inline phys_addr_t kvm_hyp_state_get_hfar(const struct vcpu_hyp_state *vcpu_hyps)
> +{
> +	return hyp_state_fault(vcpu_hyps).far_el2;
> +}
> +
>  static __always_inline unsigned long kvm_vcpu_get_hfar(const struct kvm_vcpu *vcpu)
>  {
> -	return vcpu_fault(vcpu).far_el2;
> +	return kvm_hyp_state_get_hfar(&hyp_state(vcpu));
> +}
> +
> +static __always_inline phys_addr_t kvm_hyp_state_get_fault_ipa(const struct vcpu_hyp_state *vcpu_hyps)
> +{
> +	return ((phys_addr_t) hyp_state_fault(vcpu_hyps).hpfar_el2 & HPFAR_MASK) << 8;
>  }
>  
>  static __always_inline phys_addr_t kvm_vcpu_get_fault_ipa(const struct kvm_vcpu *vcpu)
>  {
> -	return ((phys_addr_t) vcpu_fault(vcpu).hpfar_el2 & HPFAR_MASK) << 8;
> +	return kvm_hyp_state_get_fault_ipa(&hyp_state(vcpu));
> +}
> +
> +static __always_inline u32 kvm_hyp_state_get_disr(const struct vcpu_hyp_state *vcpu_hyps)
> +{
> +	return hyp_state_fault(vcpu_hyps).disr_el1;
>  }

Looks like kvm_hyp_state_get_disr() (as well as most of the
kvm_hyp_state_*() helpers below) are never used outside of their
kvm_vcpu_*() counterparts, so maybe let's merge them for now? This series
is really quite large, so I'm just hoping we can trim a bit the bits
that aren't strictly necessary :)

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