Re: [RFC V1 21/31] parisc/mm: Enable ARCH_HAS_VM_GET_PAGE_PROT

[Anshuman Khandual]

On 1/25/22 10:23 PM, Rolf Eike Beer wrote:
> Anshuman Khandual wrote:
>> This defines and exports a platform specific custom vm_get_page_prot() via
>> subscribing ARCH_HAS_VM_GET_PAGE_PROT. Subsequently all __SXXX and __PXXX
>> macros can be dropped which are no longer needed.
>>
>> Cc: "James E.J. Bottomley" <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx>
>> Cc: linux-parisc@xxxxxxxxxxxxxxx
>> Cc: linux-kernel@xxxxxxxxxxxxxxx
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@xxxxxxx>
>> ---
>>  arch/parisc/Kconfig               |  1 +
>>  arch/parisc/include/asm/pgtable.h | 20 ---------------
>>  arch/parisc/mm/init.c             | 41 +++++++++++++++++++++++++++++++
>>  3 files changed, 42 insertions(+), 20 deletions(-)
>>
>> diff --git a/arch/parisc/Kconfig b/arch/parisc/Kconfig
>> index 43c1c880def6..de512f120b50 100644
>> --- a/arch/parisc/Kconfig
>> +++ b/arch/parisc/Kconfig
>> @@ -10,6 +10,7 @@ config PARISC
>>  	select ARCH_HAS_ELF_RANDOMIZE
>>  	select ARCH_HAS_STRICT_KERNEL_RWX
>>  	select ARCH_HAS_UBSAN_SANITIZE_ALL
>> +	select ARCH_HAS_VM_GET_PAGE_PROT
>>  	select ARCH_NO_SG_CHAIN
>>  	select ARCH_SUPPORTS_HUGETLBFS if PA20
>>  	select ARCH_SUPPORTS_MEMORY_FAILURE
>> diff --git a/arch/parisc/include/asm/pgtable.h
>> b/arch/parisc/include/asm/pgtable.h index 3e7cf882639f..80d99b2b5913 100644
>> --- a/arch/parisc/include/asm/pgtable.h
>> +++ b/arch/parisc/include/asm/pgtable.h
>> @@ -269,26 +269,6 @@ extern void __update_cache(pte_t pte);
>>   * pages.
>>   */
>>
>> -	 /*xwr*/
>> -#define __P000  PAGE_NONE
>> -#define __P001  PAGE_READONLY
>> -#define __P010  __P000 /* copy on write */
>> -#define __P011  __P001 /* copy on write */
>> -#define __P100  PAGE_EXECREAD
>> -#define __P101  PAGE_EXECREAD
>> -#define __P110  __P100 /* copy on write */
>> -#define __P111  __P101 /* copy on write */
>> -
>> -#define __S000  PAGE_NONE
>> -#define __S001  PAGE_READONLY
>> -#define __S010  PAGE_WRITEONLY
>> -#define __S011  PAGE_SHARED
>> -#define __S100  PAGE_EXECREAD
>> -#define __S101  PAGE_EXECREAD
>> -#define __S110  PAGE_RWX
>> -#define __S111  PAGE_RWX
>> -
>> -
>>  extern pgd_t swapper_pg_dir[]; /* declared in init_task.c */
>>
>>  /* initial page tables for 0-8MB for kernel */
>> diff --git a/arch/parisc/mm/init.c b/arch/parisc/mm/init.c
>> index 1ae31db9988f..c8316e97e1a2 100644
>> --- a/arch/parisc/mm/init.c
>> +++ b/arch/parisc/mm/init.c
>> @@ -866,3 +866,44 @@ void flush_tlb_all(void)
>>  	spin_unlock(&sid_lock);
>>  }
>>  #endif
>> +
>> +pgprot_t vm_get_page_prot(unsigned long vm_flags)
>> +{
>> +	switch (vm_flags & (VM_READ | VM_WRITE | VM_EXEC | VM_SHARED)) {
>> +	case VM_NONE:
>> +		return PAGE_NONE;
>> +	case VM_READ:
>> +		return PAGE_READONLY;
>> +	case VM_WRITE:
>> +		return PAGE_NONE;
>> +	case VM_READ | VM_WRITE:
>> +		return PAGE_READONLY;
> This looks extremely strange. It probably is correct when it comes to CoW, how 
> about including the comment that was in the original definitions for the cases 
> where CoW is expected?
> 

Assuming that you suggest the following four comments here, sure will add them.

-#define __P000  PAGE_NONE
-#define __P001  PAGE_READONLY
-#define __P010  __P000 /* copy on write */
-#define __P011  __P001 /* copy on write */
-#define __P100  PAGE_EXECREAD
-#define __P101  PAGE_EXECREAD
-#define __P110  __P100 /* copy on write */
-#define __P111  __P101 /* copy on write */