Re: [PATCH 24/25] KVM: arm64: Make FEAT_MOPS UNDEF if not advertised to the guest

[Marc Zyngier <maz@kernel.org>]

On Thu, 25 Jan 2024 16:25:38 +0000,
Joey Gouly <joey.gouly@arm.com> wrote:
> 
> On Mon, Jan 22, 2024 at 08:18:51PM +0000, Marc Zyngier wrote:
> > We unconditionally enable FEAT_MOPS, which is obviously wrong.
> > 
> > So let's only do that when it is advertised to the guest.
> > Which means we need to rely on a per-vcpu HCRX_EL2 shadow register.
> > 
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > ---
> >  arch/arm64/include/asm/kvm_arm.h        | 4 +---
> >  arch/arm64/include/asm/kvm_host.h       | 1 +
> >  arch/arm64/kvm/hyp/include/hyp/switch.h | 2 +-
> >  arch/arm64/kvm/sys_regs.c               | 8 ++++++++
> >  4 files changed, 11 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
> > index 3c6f8ba1e479..a1769e415d72 100644
> > --- a/arch/arm64/include/asm/kvm_arm.h
> > +++ b/arch/arm64/include/asm/kvm_arm.h
> > @@ -102,9 +102,7 @@
> >  #define HCR_HOST_NVHE_PROTECTED_FLAGS (HCR_HOST_NVHE_FLAGS | HCR_TSC)
> >  #define HCR_HOST_VHE_FLAGS (HCR_RW | HCR_TGE | HCR_E2H)
> >  
> > -#define HCRX_GUEST_FLAGS \
> > -	(HCRX_EL2_SMPME | HCRX_EL2_TCR2En | \
> > -	 (cpus_have_final_cap(ARM64_HAS_MOPS) ? (HCRX_EL2_MSCEn | HCRX_EL2_MCE2) : 0))
> > +#define HCRX_GUEST_FLAGS (HCRX_EL2_SMPME | HCRX_EL2_TCR2En)
> >  #define HCRX_HOST_FLAGS (HCRX_EL2_MSCEn | HCRX_EL2_TCR2En)
> >  
> >  /* TCR_EL2 Registers bits */
> > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> > index fe5ed4bcded0..22343354db3e 100644
> > --- a/arch/arm64/include/asm/kvm_host.h
> > +++ b/arch/arm64/include/asm/kvm_host.h
> > @@ -584,6 +584,7 @@ struct kvm_vcpu_arch {
> >  
> >  	/* Values of trap registers for the guest. */
> >  	u64 hcr_el2;
> > +	u64 hcrx_el2;
> >  	u64 mdcr_el2;
> >  	u64 cptr_el2;
> >  
> > diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
> > index 2d5891518006..e3fcf8c4d5b4 100644
> > --- a/arch/arm64/kvm/hyp/include/hyp/switch.h
> > +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
> > @@ -236,7 +236,7 @@ static inline void __activate_traps_common(struct kvm_vcpu *vcpu)
> >  	write_sysreg(vcpu->arch.mdcr_el2, mdcr_el2);
> >  
> >  	if (cpus_have_final_cap(ARM64_HAS_HCX)) {
> > -		u64 hcrx = HCRX_GUEST_FLAGS;
> > +		u64 hcrx = vcpu->arch.hcrx_el2;
> >  		if (vcpu_has_nv(vcpu) && !is_hyp_ctxt(vcpu)) {
> >  			u64 clr = 0, set = 0;
> >  
> > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > index afe6975fcf5c..b7977e08e4ef 100644
> > --- a/arch/arm64/kvm/sys_regs.c
> > +++ b/arch/arm64/kvm/sys_regs.c
> > @@ -3952,6 +3952,14 @@ void kvm_init_sysreg(struct kvm_vcpu *vcpu)
> >  	if (!kvm_has_feat(kvm, ID_AA64ISAR0_EL1, TLB, OS))
> >  		vcpu->arch.hcr_el2 |= HCR_TTLBOS;
> >  
> > +	if (cpus_have_final_cap(ARM64_HAS_HCX)) {
> > +		vcpu->arch.hcrx_el2 = HCRX_GUEST_FLAGS;
> > +
> > +		if (kvm_has_feat(kern_hyp_va(vcpu->kvm),
> 
> Not sure if the use of kern_hyp_va is intentional, seems out of place since we
> use the bare `kvm` variable everyone else.

That's totally wrong. No idea where that came from...

Thanks for spotting it!

	M.

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