Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Content-Type: text/plain;
        charset=utf-8
Mime-Version: 1.0 (1.0)
Subject: Re: [RFC PATCH 04/10] x86/fpu: eager switch PKRU state
From:   Andy Lutomirski <luto@amacapital.net>
In-Reply-To: <a1ee2e5226c780312bd8ef5658f4b6422e7cf434.camel@surriel.com>
Date:   Wed, 12 Sep 2018 08:49:01 -0700
Cc:     Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
        linux-kernel@vger.kernel.org, x86@kernel.org,
        Andy Lutomirski <luto@kernel.org>,
        Paolo Bonzini <pbonzini@redhat.com>,
        =?utf-8?Q?Radim_Kr=C4=8Dm=C3=A1=C5=99?= <rkrcmar@redhat.com>,
        kvm@vger.kernel.org, "Jason A. Donenfeld" <Jason@zx2c4.com>
Content-Transfer-Encoding: quoted-printable
Message-Id: <7C1494C8-C6CA-4599-A6B3-0833F358E3A2@amacapital.net>
References: <20180912133353.20595-1-bigeasy@linutronix.de> <20180912133353.20595-5-bigeasy@linutronix.de> <181B4EB8-9FEB-415C-8069-192FB8A5B418@amacapital.net> <a1ee2e5226c780312bd8ef5658f4b6422e7cf434.camel@surriel.com>
To:     Rik van Riel <riel@surriel.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


> On Sep 12, 2018, at 8:30 AM, Rik van Riel <riel@surriel.com> wrote:
>=20
> On Wed, 2018-09-12 at 08:20 -0700, Andy Lutomirski wrote:
>>>=20
>>> --- a/arch/x86/mm/pkeys.c
>>> +++ b/arch/x86/mm/pkeys.c
>>> @@ -18,6 +18,20 @@
>>>=20
>>> #include <asm/cpufeature.h>             /* boot_cpu_has,
>>> ...            */
>>> #include <asm/mmu_context.h>            /*
>>> vma_pkey()                   */
>>> +#include <asm/fpu/internal.h>
>>> +
>>> +void write_pkru(u32 pkru)
>>> +{
>>> +    if (!boot_cpu_has(X86_FEATURE_OSPKE))
>>> +        return;
>>> +
>>> +    current->thread.fpu.pkru =3D pkru;
>>> +
>>=20
>> I thought that the offset of PKRU in the xstate was fixed after boot.
>=20
> You are right, it is.  However, that offset would need
> to be stored somewhere, and the value read every time
> we wanted to read or store the PKRU value from/to the
> floating point state.
>=20
> I suspect that would not be any faster than keeping a
> copy of the PKRU value in a known location.
>=20
>> Anyway, as written, this needs a lockdep assertion that we=E2=80=99re not=

>> preemptible, an explicit preempt_disable(), or a comment explaining
>> why it=E2=80=99s okay if we get preempted in this function.
>>=20
>>> +    __fpregs_changes_begin();
>=20
> This handles the preemption disabling, see patch
> 3 of the series.

Sure, but the first write is *before* this. So we can be preempted with the t=
wo copies of PKRU being out of sync.

>=20
>>> +    __fpregs_load_activate(&current->thread.fpu,
>>> smp_processor_id());
>>> +    __write_pkru(pkru);
>>> +    __fpregs_changes_end();
>>> +}
>>>=20
>>> int __execute_only_pkey(struct mm_struct *mm)
>>> {
>>> --=20
>>> 2.19.0
>>>=20
>>=20
>>=20
> --=20
> All Rights Reversed.