Re: [PATCH v17 08/26] x86/mm: Introduce _PAGE_COW

[Randy Dunlap]

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

> There is only an |= against the target.
> 
> Something horrid with ^= might set and clear.


-- 
~Randy
"He closes his eyes and drops the goggles.  You can't get hurt
by looking at a bitmap.  Or can you?"
(Neal Stephenson: Snow Crash)