Re: [PATCH v6 1/2] arm64: kvm: Save/restore MTE registers

[Marc Zyngier]

On 2020-12-07 14:48, Steven Price wrote:
> On 03/12/2020 17:07, Marc Zyngier wrote:
> > > diff --git a/arch/arm64/include/asm/sysreg.h 
> > > b/arch/arm64/include/asm/sysreg.h
> > > index e2ef4c2edf06..b6668ffa04d9 100644
> > > --- a/arch/arm64/include/asm/sysreg.h
> > > +++ b/arch/arm64/include/asm/sysreg.h
> > > @@ -569,7 +569,8 @@
> > >  #define SCTLR_ELx_M    (BIT(0))
> > >
> > >  #define SCTLR_ELx_FLAGS    (SCTLR_ELx_M  | SCTLR_ELx_A | SCTLR_ELx_C 
> > > | \
> > > -             SCTLR_ELx_SA | SCTLR_ELx_I | SCTLR_ELx_IESB)
> > > +             SCTLR_ELx_SA | SCTLR_ELx_I | SCTLR_ELx_IESB | \
> > > +             SCTLR_ELx_ITFSB)
> > >
> > >  /* SCTLR_EL2 specific flags. */
> > >  #define SCTLR_EL2_RES1    ((BIT(4))  | (BIT(5))  | (BIT(11)) | 
> > > (BIT(16)) | \
> > > diff --git a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
> > > b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
> > > index cce43bfe158f..45255ba60152 100644
> > > --- a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
> > > +++ b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
> > > @@ -18,6 +18,11 @@
> > >  static inline void __sysreg_save_common_state(struct kvm_cpu_context 
> > > *ctxt)
> > >  {
> > >      ctxt_sys_reg(ctxt, MDSCR_EL1)    = read_sysreg(mdscr_el1);
> > > +    if (system_supports_mte()) {
> >
> > Please move the per-VM predicate to this patch so that it can be used
> > not to save/restore the MTE registers if we don't need to.
>
> There isn't a valid struct kvm_vcpu or struct kvm here. I know there's
> ctx->__hyp_running_vcpu but AFAICT that is only valid with the host
> context.

We have per-CPU variables for the host context. If 
ctx->__hyp_running_vcpu
is non NULL, you know you're on the host.


> > > +        ctxt_sys_reg(ctxt, RGSR_EL1)    = 
> > > read_sysreg_s(SYS_RGSR_EL1);
> > > +        ctxt_sys_reg(ctxt, GCR_EL1)    = read_sysreg_s(SYS_GCR_EL1);
> > > +        ctxt_sys_reg(ctxt, TFSRE0_EL1)    = 
> > > read_sysreg_s(SYS_TFSRE0_EL1);
> > > +    }
> >
> > Overall, I still don't understand how this is going to work once
> > we have MTE in the kernel. You mentioned having the ability to
> > create turn off the tag checks at times, but I don't see that
> > in this patch (and I'm not sure we want that either).
>
> Given that this is now highly unlikely to be merged for v5.11, I'll
> rebase onto of the KASAN MTE series and double check exactly what
> happens. My thought was that it should be as simple as setting TCO,
> but your previous comment about moving the save/restore into assembler
> might be wise in case the compiler starts playing with TCO itself.

Indeed.

Thanks,

        M.
--
Jazz is not dead. It just smells funny...