[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b6eda0f414f34634b4e1aca80c4b5d5d@AcuMS.aculab.com>
Date: Thu, 21 Jan 2021 22:32:35 +0000
From: David Laight <David.Laight@...LAB.COM>
To: 'Randy Dunlap' <rdunlap@...radead.org>,
"'Yu, Yu-cheng'" <yu-cheng.yu@...el.com>,
Borislav Petkov <bp@...en8.de>
CC: "x86@...nel.org" <x86@...nel.org>,
"H. Peter Anvin" <hpa@...or.com>,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
"linux-mm@...ck.org" <linux-mm@...ck.org>,
"linux-arch@...r.kernel.org" <linux-arch@...r.kernel.org>,
"linux-api@...r.kernel.org" <linux-api@...r.kernel.org>,
Arnd Bergmann <arnd@...db.de>,
Andy Lutomirski <luto@...nel.org>,
Balbir Singh <bsingharora@...il.com>,
Cyrill Gorcunov <gorcunov@...il.com>,
Dave Hansen <dave.hansen@...ux.intel.com>,
"Eugene Syromiatnikov" <esyr@...hat.com>,
Florian Weimer <fweimer@...hat.com>,
"H.J. Lu" <hjl.tools@...il.com>, Jann Horn <jannh@...gle.com>,
Jonathan Corbet <corbet@....net>,
Kees Cook <keescook@...omium.org>,
Mike Kravetz <mike.kravetz@...cle.com>,
Nadav Amit <nadav.amit@...il.com>,
Oleg Nesterov <oleg@...hat.com>, Pavel Machek <pavel@....cz>,
Peter Zijlstra <peterz@...radead.org>,
"Ravi V. Shankar" <ravi.v.shankar@...el.com>,
Vedvyas Shanbhogue <vedvyas.shanbhogue@...el.com>,
Dave Martin <Dave.Martin@....com>,
Weijiang Yang <weijiang.yang@...el.com>,
Pengfei Xu <pengfei.xu@...el.com>
Subject: RE: [PATCH v17 08/26] x86/mm: Introduce _PAGE_COW
From: Randy Dunlap
> Sent: 21 January 2021 22:19
>
> On 1/21/21 2:16 PM, David Laight wrote:
> > From: Yu, Yu-cheng
> >>
> >> On 1/21/2021 10:44 AM, Borislav Petkov wrote:
> >>> On Tue, Dec 29, 2020 at 01:30:35PM -0800, Yu-cheng Yu wrote:
> >> [...]
> >>>> @@ -343,6 +349,16 @@ static inline pte_t pte_mkold(pte_t pte)
> >>>>
> >>>> static inline pte_t pte_wrprotect(pte_t pte)
> >>>> {
> >>>> + /*
> >>>> + * Blindly clearing _PAGE_RW might accidentally create
> >>>> + * a shadow stack PTE (RW=0, Dirty=1). Move the hardware
> >>>> + * dirty value to the software bit.
> >>>> + */
> >>>> + if (cpu_feature_enabled(X86_FEATURE_SHSTK)) {
> >>>> + pte.pte |= (pte.pte & _PAGE_DIRTY) >> _PAGE_BIT_DIRTY << _PAGE_BIT_COW;
> >>>
> >>> Why the unreadable shifting when you can simply do:
> >>>
> >>> if (pte.pte & _PAGE_DIRTY)
> >>> pte.pte |= _PAGE_COW;
> >>>
> >
> >>> ?
> >>
> >> It clears _PAGE_DIRTY and sets _PAGE_COW. That is,
> >>
> >> if (pte.pte & _PAGE_DIRTY) {
> >> pte.pte &= ~_PAGE_DIRTY;
> >> pte.pte |= _PAGE_COW;
> >> }
> >>
> >> So, shifting makes resulting code more efficient.
> >
> > Does the compiler manage to do one shift?
> >
> > How can it clear anything?
>
> It could shift it off either end since there are both << and >>.
It is still:
pte.pte |= xxxxxxx;
> > There is only an |= against the target.
> >
> > Something horrid with ^= might set and clear.
It could be 4 instructions:
is_dirty = pte.pte & PAGE_DIRTY;
pte.pte &= ~PAGE_DIRTY; // or pte.pte ^= is_dirty
is_cow = is_dirty << (BIT_COW - BIT_DIRTY); // or equivalent >>
pte.pte |= is_cow;
provided you've a three operand form for one of the first two instructions.
Something like ARM might manage to merge the last two as well.
But the register dependency chain length may matter more than
the number of instructions.
The above is likely to be three long.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
Powered by blists - more mailing lists