Re: [RFC PATCH 2/2] powerpc/mm: Replace _PAGE_USER with _PAGE_PRIV

["Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>]

Paul Mackerras <paulus@ozlabs.org> writes:

> [ text/plain ]
> On Fri, Feb 26, 2016 at 08:50:50AM +0530, Aneesh Kumar K.V wrote:
>> _PAGE_PRIV means the page can be accessed only by kernel. This is done
>> to keep pte bits similar to PowerISA 3.0 radix PTE format. User
>> pages are now makred by clearing _PAGE_PRIV bit.
>
>

.....

>> @@ -40,6 +40,11 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,
>>  		if (unlikely(access & ~old_pte))
>>  			return 1;
>
> This check is going to do a different thing now as far as
> _PAGE_USER/_PAGE_PRIV is concerned: previously it would prevent a
> non-privileged access to a privileged page from creating a HPTE, now
> it prevents a privileged access to a non-privileged page from creating
> a HPTE.  A privileged access means an access by the kernel to a high
> address, and arguably we would never have a non-privileged PTE at a
> high (i.e. kernel) address, but it's still a semantic change that
> should have been flagged in the patch description.


We don't set _PAGE_PRIVILGED when we have a privilged acess to a non
privilged page. We set it as below (with updated comments)

	/*
	 * We set _PAGE_PRIVILEGED only when
	 * kernel mode access kernel space.
	 *
	 * _PAGE_PRIVILGED is NOT set
	 * 1) when kernel mode access user space
	 * 2) user space access kernel space.
	 */
	if (!(msr & MSR_PR) && !(REGION_ID(ea) == USER_REGION_ID))
		access |= _PAGE_PRIVILEGED;



>
> In fact it wouldn't really matter if we didn't check the privilege
> here and went ahead and created the HPTE - if it's a non-privileged
> access to a privileged page, the HPTE will get its permission bits set
> so as to prevent non-privileged access anyway.
>
>>  		/*
>> +		 * access from user, but pte in _PAGE_PRIV
>> +		 */
>> +		if (unlikely((access & _PAGE_PRIV) != (old_pte & _PAGE_PRIV)))
>> +			return 1;
>
> And this catches both the privileged access to non-privileged page
> case (which the previous statement already caught) and the
> non-privileged access to privileged page case.

The actual PRIVILEGED check is done by the above conditional. 


-aneesh