On Thu, 24 Oct 2019 17:10:44 +0100,
James Morse <james.morse@xxxxxxx> wrote:
Hi James,
> Hi Marc,
>
> On 19/10/2019 10:55, Marc Zyngier wrote:
> > When handling erratum 1319367, we must ensure that the page table
> > walker cannot parse the S1 page tables while the guest is in an
> > inconsistent state. This is done as follows:
> >
> > On guest entry:
> > - TCR_EL1.EPD{0,1} are set, ensuring that no PTW can occur
> > - all system registers are restored, except for TCR_EL1 and SCTLR_EL1
> > - stage-2 is restored
> > - SCTLR_EL1 and TCR_EL1 are restored
> >
> > On guest exit:
> > - SCTLR_EL1.M and TCR_EL1.EPD{0,1} are set, ensuring that no PTW can occur
> > - stage-2 is disabled
> > - All host system registers are restored
>
> Reviewed-by: James Morse <james.morse@xxxxxxx>
>
> (whitespace nit below)
>
>
> > diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> > index 69e10b29cbd0..5765b17c38c7 100644
> > --- a/arch/arm64/kvm/hyp/switch.c
> > +++ b/arch/arm64/kvm/hyp/switch.c
> > @@ -118,6 +118,20 @@ static void __hyp_text __activate_traps_nvhe(struct kvm_vcpu *vcpu)
> > }
> >
> > write_sysreg(val, cptr_el2);
> > +
> > + if (cpus_have_const_cap(ARM64_WORKAROUND_1319367)) {
> > + struct kvm_cpu_context *ctxt = &vcpu->arch.ctxt;
> > +
> > + isb();
> > + /*
> > + * At this stage, and thanks to the above isb(), S2 is
> > + * configured and enabled. We can now restore the guest's S1
> > + * configuration: SCTLR, and only then TCR.
> > + */
>
> (note for my future self: because the guest may have had M=0 and rubbish in the TTBRs)
>
> > + write_sysreg_el1(ctxt->sys_regs[SCTLR_EL1], SYS_SCTLR);
> > + isb();
> > + write_sysreg_el1(ctxt->sys_regs[TCR_EL1], SYS_TCR);
> > + }
> > }
> >
>
>
> > diff --git a/arch/arm64/kvm/hyp/sysreg-sr.c b/arch/arm64/kvm/hyp/sysreg-sr.c
> > index 7ddbc849b580..fb97547bfa79 100644
> > --- a/arch/arm64/kvm/hyp/sysreg-sr.c
> > +++ b/arch/arm64/kvm/hyp/sysreg-sr.c
> > @@ -117,12 +117,26 @@ static void __hyp_text __sysreg_restore_el1_state(struct kvm_cpu_context *ctxt)
> > {
> > write_sysreg(ctxt->sys_regs[MPIDR_EL1], vmpidr_el2);
> > write_sysreg(ctxt->sys_regs[CSSELR_EL1], csselr_el1);
> > - write_sysreg_el1(ctxt->sys_regs[SCTLR_EL1], SYS_SCTLR);
> > +
> > + if (!cpus_have_const_cap(ARM64_WORKAROUND_1319367)) {
> > + write_sysreg_el1(ctxt->sys_regs[SCTLR_EL1], SYS_SCTLR);
> > + write_sysreg_el1(ctxt->sys_regs[TCR_EL1], SYS_TCR);
> > + } else if (!ctxt->__hyp_running_vcpu) {
> > + /*
> > + * Must only be done for guest registers, hence the context
> > + * test. We'recoming from the host, so SCTLR.M is already
>
> (Nit: We'recoming?)
Well spotted, now fixed. And thanks for the reviewing, much appreciated.
Catalin, Will: given that this series conflicts with the workaround for
erratum 1542419, do you mind taking it via the arm64 tree?
To make things a bit simpler, I've updated the series with James' tags at
[1], and pushed out a resolution of the merge with arm64/for-next/core [2].
Thanks,
M.
[1] git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git kvm-arm64/erratum-1319367
[2] git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git kvm-arm64/erratum-1319367-resolved
--
Jazz is not dead, it just smells funny.
