On Tue, Nov 07, 2017 at 05:14:50PM +0100, Andrew Jones wrote:
> On Thu, Oct 12, 2017 at 12:41:19PM +0200, Christoffer Dall wrote:
> > There is no need to reset the VTTBR to zero when exiting the guest on
> > VHE systems. VHE systems don't use stage 2 translations for the EL2&0
> > translation regime used by the host.
> >
> > Signed-off-by: Christoffer Dall <christoffer.dall@xxxxxxxxxx>
> > ---
> > arch/arm64/kvm/hyp/switch.c | 5 ++---
> > 1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> > index b72dc66..2cedf12 100644
> > --- a/arch/arm64/kvm/hyp/switch.c
> > +++ b/arch/arm64/kvm/hyp/switch.c
> > @@ -136,13 +136,13 @@ static void __hyp_text __deactivate_traps(struct kvm_vcpu *vcpu)
> > write_sysreg(0, pmuserenr_el0);
> > }
> >
> > -static void __hyp_text __activate_vm(struct kvm_vcpu *vcpu)
> > +static inline void __hyp_text __activate_vm(struct kvm_vcpu *vcpu)
> > {
> > struct kvm *kvm = kern_hyp_va(vcpu->kvm);
>
> Hmm, should we change __activate_vm to take a kvm pointer instead of a
> vcpu, and then call the function from the VHE kvm_vcpu_run normally, but
> from the non-VHE kvm_vcpu_run with kern_hyp_va(vcpu->kvm)? I only ask,
> because it appears to be the last kern_hyp_va() that VHE code would still
> invoke, at least considering the code in arch/arm64/kvm/hyp/switch.c
>
Yes, that's a reasonable point.
> > write_sysreg(kvm->arch.vttbr, vttbr_el2);
> > }
> >
> > -static void __hyp_text __deactivate_vm(struct kvm_vcpu *vcpu)
> > +static inline void __hyp_text __deactivate_vm(struct kvm_vcpu *vcpu)
>
> Adding these 'inline' attributes is unrelated to this patch, and probably
> unnecessary, as the compiler probably knew to do so anyway, but whatever.
>
I actually saw an additional function being inlined on the compiler I
used with the inline attribute, but not without, but I cannot reproduce
currently, so I'll drop these attributes.
> > {
> > write_sysreg(0, vttbr_el2);
> > }
> > @@ -360,7 +360,6 @@ int kvm_vcpu_run(struct kvm_vcpu *vcpu)
> > __vgic_save_state(vcpu);
> >
> > __deactivate_traps(vcpu);
> > - __deactivate_vm(vcpu);
> >
> > __sysreg_restore_host_state(host_ctxt);
> >
> > --
> > 2.9.0
> >
>
> Reviewed-by: Andrew Jones <drjones@xxxxxxxxxx>
Thanks,
-Christoffer
