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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <07e40bbcf734e8c63630168406262bb2@kernel.org>
Date:   Mon, 07 Dec 2020 15:55:56 +0000
From:   Marc Zyngier <maz@...nel.org>
To:     Steven Price <steven.price@....com>
Cc:     Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will@...nel.org>,
        James Morse <james.morse@....com>,
        Julien Thierry <julien.thierry.kdev@...il.com>,
        Suzuki K Poulose <suzuki.poulose@....com>,
        kvmarm@...ts.cs.columbia.edu, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org, Dave Martin <Dave.Martin@....com>,
        Mark Rutland <mark.rutland@....com>,
        Thomas Gleixner <tglx@...utronix.de>, qemu-devel@...gnu.org,
        Juan Quintela <quintela@...hat.com>,
        "Dr. David Alan Gilbert" <dgilbert@...hat.com>,
        Richard Henderson <richard.henderson@...aro.org>,
        Peter Maydell <peter.maydell@...aro.org>,
        Haibo Xu <Haibo.Xu@....com>, Andrew Jones <drjones@...hat.com>
Subject: Re: [PATCH v6 1/2] arm64: kvm: Save/restore MTE registers

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