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

Reply via email to