ban the use of _PAGE_XXX flags outside target specific code

[chleroy]

Today flags like for instance _PAGE_RW or _PAGE_USER are used through common parts of code.
Using those directly in common parts of code have proven to lead to mistakes or misbehaviour, because their use is not always as trivial as one could think.

For instance, (flags & _PAGE_USER) == 0 isn't enough to tell that a page is a kernel page, because some targets are using _PAGE_PRIVILEDGED and not _PAGE_USER, so the test has to be (flags & (_PAGE_USER | _PAGE_PRIVILEDGED)) == _PAGE_PRIVILEDGED
This has to (bad) consequences:

• All targets must define every bit, even the unsupported ones, leading to a lot of useless #define _PAGE_XXX 0
• If someone forgets to take into account all possible _PAGE_XXX bits for the case, we can get unexpected behaviour on some targets.

This becomes even more complex when we come to using _PAGE_RW. Testing (flags & _PAGE_RW) is not enough to test whether a page if writable or not, because:

• Some targets have _PAGE_RO instead, which has to be unset to tell a page is writable
• Some targets have _PAGE_R and _PAGE_W, in which case _PAGE_RW = _PAGE_R | _PAGE_W
  Even knowing whether a page is readable is not always trivial because:
• Some targets requires to check that _PAGE_R is set to ensure page is readable
• Some targets requires to check that _PAGE_NA is not set
• Some targets requires to check that _PAGE_RO or _PAGE_RW is set

Etc ....

Therefore, in order to work around all those issues and minimise the risks of errors, it would be advisable to remove all use of _PAGE_XXX flags from powerpc code and always use pte_xxx() and pte_mkxxx() accessors instead. Those accessors should then be defined in target specific parts of the kernel code.

[mpe]

Yeah this seems like a good idea to me.

[chleroy]

One misuse of _PAGE_XX bits introduced recently (arch/powerpc/include/asm/nohash/pgtable.h):

static inline bool pte_access_permitted(pte_t pte, bool write)
{
	unsigned long pteval = pte_val(pte);
	/*
	 * A read-only access is controlled by _PAGE_USER bit.
	 * We have _PAGE_READ set for WRITE and EXECUTE
	 */
	unsigned long need_pte_bits = _PAGE_PRESENT | _PAGE_USER;

	if (write)
		need_pte_bits |= _PAGE_WRITE;

	if ((pteval & need_pte_bits) != need_pte_bits)
		return false;

	return true;
}

On the 8xx, _PAGE_WRITE is 0 and _PAGE_USER is 0. So this function only checks _PAGE_PRESENT.

[chleroy]

Serie https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=69827&state=* merged in 4.20, thanks @mpe