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

[Marc Zyngier <maz@kernel.org>]

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...