linux-kernel - Re: [PATCH v7 1/3] 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: <bf1fe1204a075abbd294435944973f62@kernel.org>
Date:   Thu, 04 Feb 2021 14:56:13 +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 v7 1/3] arm64: kvm: Save/restore MTE registers

On 2021-02-04 14:33, Steven Price wrote:
> On 02/02/2021 15:36, Marc Zyngier wrote:
>> On 2021-01-15 15:28, Steven Price wrote:
>>> Define the new system registers that MTE introduces and context 
>>> switch
>>> them. The MTE feature is still hidden from the ID register as it 
>>> isn't
>>> supported in a VM yet.
>>> 
>>> Signed-off-by: Steven Price <steven.price@....com>
>>> ---
>>>  arch/arm64/include/asm/kvm_host.h          |  4 ++
>>>  arch/arm64/include/asm/kvm_mte.h           | 74 
>>> ++++++++++++++++++++++
>>>  arch/arm64/include/asm/sysreg.h            |  3 +-
>>>  arch/arm64/kernel/asm-offsets.c            |  3 +
>>>  arch/arm64/kvm/hyp/entry.S                 |  7 ++
>>>  arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h |  4 ++
>>>  arch/arm64/kvm/sys_regs.c                  | 14 ++--
>>>  7 files changed, 104 insertions(+), 5 deletions(-)
>>>  create mode 100644 arch/arm64/include/asm/kvm_mte.h
>>> 
>>> diff --git a/arch/arm64/include/asm/kvm_host.h
>>> b/arch/arm64/include/asm/kvm_host.h
>>> index 11beda85ee7e..51590a397e4b 100644
>>> --- a/arch/arm64/include/asm/kvm_host.h
>>> +++ b/arch/arm64/include/asm/kvm_host.h
>>> @@ -148,6 +148,8 @@ enum vcpu_sysreg {
>>>      SCTLR_EL1,    /* System Control Register */
>>>      ACTLR_EL1,    /* Auxiliary Control Register */
>>>      CPACR_EL1,    /* Coprocessor Access Control */
>>> +    RGSR_EL1,    /* Random Allocation Tag Seed Register */
>>> +    GCR_EL1,    /* Tag Control Register */
>>>      ZCR_EL1,    /* SVE Control */
>>>      TTBR0_EL1,    /* Translation Table Base Register 0 */
>>>      TTBR1_EL1,    /* Translation Table Base Register 1 */
>>> @@ -164,6 +166,8 @@ enum vcpu_sysreg {
>>>      TPIDR_EL1,    /* Thread ID, Privileged */
>>>      AMAIR_EL1,    /* Aux Memory Attribute Indirection Register */
>>>      CNTKCTL_EL1,    /* Timer Control Register (EL1) */
>>> +    TFSRE0_EL1,    /* Tag Fault Status Register (EL0) */
>>> +    TFSR_EL1,    /* Tag Fault Stauts Register (EL1) */
>> 
>> s/Stauts/Status/
>> 
>> Is there any reason why the MTE registers aren't grouped together?
> 
> I has been under the impression this list is sorted by the encoding of
> the system registers, although double checking I've screwed up the
> order of TFSRE0_EL1/TFSR_EL1, and not all the other fields are sorted
> that way.

It grew organically, and was initially matching the original order
of the save/restore sequence. This order has long disappeared with
VHE, and this is essentially nothing more than a bag of indices
(although NV does bring some order back to deal with VNCR-backed
registers).

[...]

>>> diff --git a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
>>> b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
>>> index cce43bfe158f..94d9736f0133 100644
>>> --- a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
>>> +++ b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
>>> @@ -45,6 +45,8 @@ static inline void __sysreg_save_el1_state(struct
>>> kvm_cpu_context *ctxt)
>>>      ctxt_sys_reg(ctxt, CNTKCTL_EL1)    = 
>>> read_sysreg_el1(SYS_CNTKCTL);
>>>      ctxt_sys_reg(ctxt, PAR_EL1)    = read_sysreg_par();
>>>      ctxt_sys_reg(ctxt, TPIDR_EL1)    = read_sysreg(tpidr_el1);
>>> +    if (system_supports_mte())
>>> +        ctxt_sys_reg(ctxt, TFSR_EL1) = read_sysreg_el1(SYS_TFSR);
>> 
>> I already asked for it, and I'm going to ask for it again:
>> Most of the sysreg save/restore is guarded by a per-vcpu check
>> (HCR_EL2.ATA), while this one is unconditionally saved/restore
>> if the host is MTE capable. Why is that so?
> 
> Sorry, I thought your concern was for registers that affect the host
> (as they are obviously more performance critical as they are hit on
> every guest exit). Although I guess that's incorrect for nVHE which is
> what all the cool kids want now ;)

I think we want both correctness *and* performance, for both VHE
and nVHE. Things like EL0 registers should be able to be moved
to load/put on all implementations, and the correct switching
be done at the right spot only when required.

Thanks,

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