On Mon, Oct 28, 2019 at 04:19:55PM +0000, Marc Zyngier wrote:
> On Mon, 28 Oct 2019 15:12:39 +0000,
> Alexandru Elisei <alexandru.elisei@xxxxxxx> wrote:
> > On 10/28/19 1:05 PM, Christoffer Dall wrote:
> > > diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
> > > index d69c1efc63e7..70509799a2a9 100644
> > > --- a/arch/arm64/include/asm/kvm_emulate.h
> > > +++ b/arch/arm64/include/asm/kvm_emulate.h
> > > @@ -53,8 +53,18 @@ static inline void vcpu_reset_hcr(struct kvm_vcpu *vcpu)
> > > /* trap error record accesses */
> > > vcpu->arch.hcr_el2 |= HCR_TERR;
> > > }
> > > - if (cpus_have_const_cap(ARM64_HAS_STAGE2_FWB))
> > > +
> > > + if (cpus_have_const_cap(ARM64_HAS_STAGE2_FWB)) {
> > > vcpu->arch.hcr_el2 |= HCR_FWB;
> > > + } else {
> > > + /*
> > > + * For non-FWB CPUs, we trap VM ops (HCR_EL2.TVM) until M+C
> > > + * get set in SCTLR_EL1 such that we can detect when the guest
> > > + * MMU gets turned off and do the necessary cache maintenance
> > > + * then.
> > > + */
> > > + vcpu->arch.hcr_el2 &= ~HCR_TVM;
> >
> > Don't we want to set the bit here, so we're consistent with the
> > previous behaviour and the comment? Because with this patch, we
> > never set HCR_EL2.TVM...
>
> Of course you're right. This is how I plan to fix it:
>
> diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
> index 47c774c2d18b..7b835337f78b 100644
> --- a/arch/arm64/include/asm/kvm_emulate.h
> +++ b/arch/arm64/include/asm/kvm_emulate.h
> @@ -63,7 +63,7 @@ static inline void vcpu_reset_hcr(struct kvm_vcpu *vcpu)
> * MMU gets turned on and do the necessary cache maintenance
> * then.
> */
> - vcpu->arch.hcr_el2 &= ~HCR_TVM;
> + vcpu->arch.hcr_el2 |= HCR_TVM;
> }
Ouch, yes. That was as suggested for v1, and I missed it when saying my
R-B held. :(
Mark.
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
