On Sat, Mar 05, 2016 at 01:59:02PM +0530, Aneesh Kumar K.V wrote: > Paul Mackerras <pau...@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;
You're confusing a page in the user part of the address space with a non-privileged page. Now, we would certainly expect that all pages in the user part of the address space would be non-privileged pages and all pages in the kernel part would be privileged pages, but nothing actually enforces that. The semantic change is that if we did somehow happen to have a non-privileged page (one without _PAGE_PRIVILEGED set) in the kernel part of the address space, we can no longer access it from the kernel. Now you can argue that we never have non-privileged pages in the kernel part of the address space, and so the change doesn't matter. That is probably a good argument, but you do need to mention the change and make that argument in your patch description. Paul. _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev