[PATCH 3/3] powerpc: mm: support page table check

[Christophe Leroy]

Le 13/09/2022 à 03:21, Rohan McLure a écrit :
> 
> 
>> On 12 Sep 2022, at 4:11 pm, Christophe Leroy <christophe.leroy at csgroup.eu> wrote:
>>
>>
>>
>> Le 12/09/2022 à 03:47, Rohan McLure a écrit :
>>> On creation and clearing of a page table mapping, instrument such calls
>>> by invoking page_table_check_pte_set and page_table_check_pte_clear
>>> respectively. These calls serve as a sanity check against illegal
>>> mappings.
>>>
>>> Enable ARCH_SUPPORTS_PAGE_TABLE_CHECK for all ppc64, and 32-bit
>>> platforms implementing Book3S.
>>
>> Why only book3s on 32 bits ?
> 
> Sorry. I failed to update that commit message. This patch instead supports,
> page table checks on all platforms, but I began writing this patch series to
> target just Book3S, and then updated it to include all platforms. The only
> barrier to doing so was the need for the pud_pfn and
> page_table_check_pud_{clear,set} bloat.
> 
>>> +++ b/arch/powerpc/include/asm/pgtable.h
>>> @@ -166,7 +166,11 @@ static inline int pud_pfn(pud_t pud)
>>>   	 * check so this should never be used. If it grows another user we
>>>   	 * want to know about it.
>>>   	 */
>>> +#ifndef CONFIG_PAGE_TABLE_CHECK
>>>   	BUILD_BUG();
>>> +#else
>>> +	BUG();
>>> +#endif
>>
>> Awfull.
> 
> Quite right. I suspect you can infer the intention here, which is to enforce
> that this dead code must not be included anywhere in generic code, but rather
> be gated by pud_devmap. I will relax this to a WARN().
> 

That's the #ifndef/#else/#endif that I find awful, more than the BUG() 
itself. Either this is not supposed to be used at all, and the 
BUILD_BUG() should be the way, or it is supposed to be used, and then it 
should really be implemented.

By the way, the comment should also be updated. because it is more than 
devmap now.

However, as pud_pfn() is starting to get really used now, maybe it is 
time to implement it for real, isn't it ?

Or the other solution is to get pud_user_accessible_page() return always 
false.

Christophe