linux-kernel - Re: [PATCH] arm64: mm: add missing PTE_SPECIAL in pte

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Thu, 8 Aug 2019 12:28:58 +0530
From:   Anshuman Khandual <anshuman.khandual@....com>
To:     "Justin He (Arm Technology China)" <Justin.He@....com>,
        Catalin Marinas <Catalin.Marinas@....com>,
        Will Deacon <will@...nel.org>,
        Mark Rutland <Mark.Rutland@....com>,
        James Morse <James.Morse@....com>
Cc:     Christoffer Dall <Christoffer.Dall@....com>,
        Punit Agrawal <punitagrawal@...il.com>, Qian Cai <cai@....pw>,
        Jun Yao <yaojun8558363@...il.com>,
        Alex Van Brunt <avanbrunt@...dia.com>,
        Robin Murphy <Robin.Murphy@....com>,
        Thomas Gleixner <tglx@...utronix.de>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        Dan Williams <dan.j.williams@...el.com>,
        Jérôme Glisse <jglisse@...hat.com>,
        Logan Gunthorpe <logang@...tatee.com>,
        Christoph Hellwig <hch@....de>
Subject: Re: [PATCH] arm64: mm: add missing PTE_SPECIAL in pte_mkdevmap on
 arm64



On 08/08/2019 11:50 AM, Justin He (Arm Technology China) wrote:
> Hi Anshuman
> Thanks for the comments, please see my comments below
> 
>> -----Original Message-----
>> From: Anshuman Khandual <anshuman.khandual@....com>
>> Sent: 2019年8月8日 13:19
>> To: Justin He (Arm Technology China) <Justin.He@....com>; Catalin
>> Marinas <Catalin.Marinas@....com>; Will Deacon <will@...nel.org>;
>> Mark Rutland <Mark.Rutland@....com>; James Morse
>> <James.Morse@....com>
>> Cc: Christoffer Dall <Christoffer.Dall@....com>; Punit Agrawal
>> <punitagrawal@...il.com>; Qian Cai <cai@....pw>; Jun Yao
>> <yaojun8558363@...il.com>; Alex Van Brunt <avanbrunt@...dia.com>;
>> Robin Murphy <Robin.Murphy@....com>; Thomas Gleixner
>> <tglx@...utronix.de>; linux-arm-kernel@...ts.infradead.org; linux-
>> kernel@...r.kernel.org
>> Subject: Re: [PATCH] arm64: mm: add missing PTE_SPECIAL in
>> pte_mkdevmap on arm64
>>
> [...]
>>> diff --git a/arch/arm64/include/asm/pgtable.h
>> b/arch/arm64/include/asm/pgtable.h
>>> index 5fdcfe237338..e09760ece844 100644
>>> --- a/arch/arm64/include/asm/pgtable.h
>>> +++ b/arch/arm64/include/asm/pgtable.h
>>> @@ -209,7 +209,7 @@ static inline pmd_t pmd_mkcont(pmd_t pmd)
>>>
>>>  static inline pte_t pte_mkdevmap(pte_t pte)
>>>  {
>>> -	return set_pte_bit(pte, __pgprot(PTE_DEVMAP));
>>> +	return set_pte_bit(pte, __pgprot(PTE_DEVMAP | PTE_SPECIAL));
>>>  }
>>>
>>>  static inline void set_pte(pte_t *ptep, pte_t pte)
>>> @@ -396,7 +396,10 @@ static inline int pmd_protnone(pmd_t pmd)
>>>  #ifdef CONFIG_TRANSPARENT_HUGEPAGE
>>>  #define pmd_devmap(pmd)		pte_devmap(pmd_pte(pmd))
>>>  #endif
>>> -#define pmd_mkdevmap(pmd)
>> 	pte_pmd(pte_mkdevmap(pmd_pte(pmd)))
>>> +static inline pmd_t pmd_mkdevmap(pmd_t pmd)
>>> +{
>>> +	return pte_pmd(set_pte_bit(pmd_pte(pmd),
>> __pgprot(PTE_DEVMAP)));
>>> +}
>>
>> Though I could see other platforms like powerpc and x86 following same
>> approach (DEVMAP + SPECIAL) for pte so that it checks positive for
>> pte_special() but then just DEVMAP for pmd which could never have a
>> pmd_special(). But a more fundamental question is - why should a devmap
>> be a special pte as well ?
> 
> IIUC, special pte bit make things handling easier compare with those arches which
> have no special bit. The memory codes will regard devmap page as a special one 
> compared with normal page.

For that we have PTE_DEVMAP on arm64 which differentiates device memory
entries from others and it should not again need PTE_SPECIAL as well for
that. We set both bits while creating the entries with pte_mkdevmap()
and check just one bit PTE_DEVMAP with pte_devmap(). Problem is it will
also test positive for pte_special() and risks being identified as one.

> Devmap page structure can be stored in ram/pmem/none.

That is altogether a different aspect which is handled with vmem_altmap
during hotplug and nothing to do with how device memory is mapped in the
page table. I am not sure about "none" though. IIUC unlike traditional
device pfn all ZONE_DEVICE memory will have struct page backing either
on system RAM or in the device memory itself.

> 
>>
>> Also in vm_normal_page() why cannot it tests for pte_devmap() before it
>> starts looking for CONFIG_ARCH_HAS_PTE_SPECIAL. Is this the only path
>> for
> 
> AFAICT, yes, but it changes to much besides arm codes. 😊

If this is the only path for which all platforms have to set PTE_SPECIAL
in their device mapping, then it should just be fixed in vm_normal_page().

> 
>> which we need to set SPECIAL bit on a devmap pte or there are other paths
>> where this semantics is assumed ?
> 
> No idea

Probably something to be asked in the mm community.

1. Why pte_mkdevmap() should set SPECIAL bit for a positive pte_special()
   check. Why the same mapping be identified as pte_devmap() as well as
   pte_special().

2. Can pte_devmap() and pte_special() re-ordering at vm_normal_page() will
   remove this dependency or there are other commons MM paths which assume
   this behavior ?

+ linux-mm@...ck.org <linux-mm@...ck.org>
+ Dan Williams <dan.j.williams@...el.com>
+ Jérôme Glisse <jglisse@...hat.com>
+ Logan Gunthorpe <logang@...tatee.com>
+ Christoph Hellwig <hch@....de>