RE: [PATCH] arm64: mm: add missing PTE_SPECIAL in pte_mkdevmap on arm64

[Justin He (Arm Technology China)]

Hi Anshuman
Thanks for the comments, please see my comments below

> -----Original Message-----
> From: Anshuman Khandual <anshuman.khandual@xxxxxxx>
> Sent: 2019å8æ8æ 13:19
> To: Justin He (Arm Technology China) <Justin.He@xxxxxxx>; Catalin
> Marinas <Catalin.Marinas@xxxxxxx>; Will Deacon <will@xxxxxxxxxx>;
> Mark Rutland <Mark.Rutland@xxxxxxx>; James Morse
> <James.Morse@xxxxxxx>
> Cc: Christoffer Dall <Christoffer.Dall@xxxxxxx>; Punit Agrawal
> <punitagrawal@xxxxxxxxx>; Qian Cai <cai@xxxxxx>; Jun Yao
> <yaojun8558363@xxxxxxxxx>; Alex Van Brunt <avanbrunt@xxxxxxxxxx>;
> Robin Murphy <Robin.Murphy@xxxxxxx>; Thomas Gleixner
> <tglx@xxxxxxxxxxxxx>; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx
> 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.
Devmap page structure can be stored in ram/pmem/none.

>
> 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. ð

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

No idea


--
Cheers,
Justin (Jia He)


IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.