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;
From:   Nadav Amit <nadav.amit@gmail.com>
Message-Id: <372BBE7D-1397-4EFB-BB60-0638710F11DB@gmail.com>
Content-Type: multipart/signed;
 boundary="Apple-Mail=_180F44C5-8A2E-46EE-9C01-57D6FF334752";
 protocol="application/pgp-signature"; micalg=pgp-sha512
Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\))
Subject: Re: [PATCH v2 2/2] x86: disable IRQs before changing CR4
Date:   Sat, 25 Nov 2017 09:31:03 -0800
In-Reply-To: <alpine.DEB.2.20.1711251823230.2316@nanos>
Cc:     LKML <linux-kernel@vger.kernel.org>, linux-edac@vger.kernel.org,
        Andy Lutomirski <luto@kernel.org>,
        Ingo Molnar <mingo@redhat.com>,
        "H. Peter Anvin" <hpa@zytor.com>, x86@kernel.org,
        Tony Luck <tony.luck@intel.com>,
        Borislav Petkov <bp@alien8.de>,
        Paolo Bonzini <pbonzini@redhat.com>,
        =?utf-8?B?UmFkaW0gS3LEjW3DocWZ?= <rkrcmar@redhat.com>
To:     Thomas Gleixner <tglx@linutronix.de>
References: <20171125032907.2241-1-namit@vmware.com>
 <20171125032907.2241-3-namit@vmware.com>
 <alpine.DEB.2.20.1711251017580.2316@nanos>
 <803D7D69-1F01-4CB8-A966-A040F302D1C0@gmail.com>
 <alpine.DEB.2.20.1711251823230.2316@nanos>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


--Apple-Mail=_180F44C5-8A2E-46EE-9C01-57D6FF334752
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=us-ascii

Thomas Gleixner <tglx@linutronix.de> wrote:

> On Sat, 25 Nov 2017, Nadav Amit wrote:
>> Thomas Gleixner <tglx@linutronix.de> wrote:
>>=20
>>> On Fri, 24 Nov 2017, Nadav Amit wrote:
>>>> /* Set in this cpu's CR4. */
>>>> -static inline void cr4_set_bits(unsigned long mask)
>>>> +static inline void cr4_set_bits_irqs_off(unsigned long mask)
>>>=20
>>> This change is kinda weird. I'd expect that there is a corresponding
>>> function cr4_set_bits() which takes care of disabling interrupts. =
But there
>>> is not. All it does is creating a lot of pointless churn.
>>>=20
>>>> static __always_inline void setup_smep(struct cpuinfo_x86 *c)
>>>> static __init int setup_disable_smap(char *arg)
>>>> static __always_inline void setup_pku(struct cpuinfo_x86 *c)
>>>=20
>>> Why are you not doing this at the call site around all calls which =
fiddle
>>> with cr4, i.e. in identify_cpu() ?
>>>=20
>>> identify_cpu() is called from two places:
>>>=20
>>>   identify_boot_cpu() and identify_secondary_cpu()
>>>=20
>>> identify_secondary_cpu is called with interrupts disabled anyway and =
there
>>> is no reason why we can't enforce interrupts being disabled around
>>> identify_cpu() completely.
>>>=20
>>> But if we actually do the right thing, i.e. having cr4_set_bit() and
>>> cr4_set_bit_irqsoff() all of this churn goes away magically.
>>>=20
>>> Then the only place which needs to be changed is the context switch =
because
>>> here interrupts are already disabled and we really care about =
performance.
>>>=20
>>>> @@ -293,7 +303,7 @@ void __switch_to_xtra(struct task_struct =
*prev_p, struct task_struct *next_p,
>>>> 	}
>>>>=20
>>>> 	if ((tifp ^ tifn) & _TIF_NOTSC)
>>>> -		cr4_toggle_bits(X86_CR4_TSD);
>>>> +		cr4_toggle_bits_irqs_off(X86_CR4_TSD);
>>=20
>> You make a good point. I will add cr4_set_bit(). I will leave =
identify_cpu()
>> as is, since it is rather hard to maintain code that enables/disables =
irqs
>> at one point and rely on these operations at a completely different =
place.
>> As you said, it is less of an issue once cr4_set_bit() and friends =
are
>> introduced.
>=20
> I fixed that up already as I wanted to have it done, see the tip-bot =
mail
> in your inbox.

Thanks, your changes made it much better. At some point it might be =
better
to make the MTRR code to use these interfaces too instead of meddling =
with
CR4 directly. Anyhow, that is a different story.

Regards,
Nadav

--Apple-Mail=_180F44C5-8A2E-46EE-9C01-57D6FF334752
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
	filename=signature.asc
Content-Type: application/pgp-signature;
	name=signature.asc
Content-Description: Message signed with OpenPGP

-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEE0YCJM4pMIpzxUdmOK4dOkxJsY0AFAloZqNcACgkQK4dOkxJs
Y0DECA/9HpNVG0WiXDmY6+VbesNpXYxkGE9kMFyVWXLrbYacykTLj7sw4LFTsbat
z07w73tmswu4RmbSD8fxFW5CDOPu+dOJpWPD3EtvbyQi4y2DY9GS3DVnFTYOwplP
A5zUckn2Ve01doC/n1Oazy4/pOGBbaq1zifT6PFsXdCpK7R7Cm2mQ2oqysNEpdZB
GDwYsqIKSBNqP4Kwz4Qn2AFwSEOVWzLrOxML8CVeR602Yzb8RmwfPvoUA2yvmFFk
z2MscnTcCPyFfxq79kD3NKM+4Mnn9TM2jorPpqksAqybeNcDUWluSd5chQdh1FPV
rP/vqWGAeePy0Gnifpy8Dk82cKgfIKxlWiWE2HUp/VLGdDe6MRlxtBGWGu/jBRxL
DK0SetskIj/mSraLLEd1Dgx6QugGF47to3kFRrx0rxQ03tbFZczgj5BatFK3aWAG
TRzcGSe2Q5EnN98KVDng7kD6soWwuxF3J0CBwkIm9N3vdTPrjr8s4x58Xlpyz8i9
WPTPqb0a0R5VhUVNtZkFfqg15/YXDyF0xzDSxvbz69naHlv8/4KYvlHg7lLnQTrI
ifLz98s0nAo1VfwiZnRaI4STaHF4Edl5tyDC4vnYGlIZGQS21Fe+rCd+Amg+MMNU
j9XDjXmd2YjG0rNf/6EnyB8RN837K73uPjfEA4SbeIWpZY4jnMY=
=g7W6
-----END PGP SIGNATURE-----

--Apple-Mail=_180F44C5-8A2E-46EE-9C01-57D6FF334752--

From 1585059764283489646@xxx Sat Nov 25 17:26:27 +0000 2017
X-GM-THRID: 1585008173950843474
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread