diff mbox series

[v3,11/34] mips: mm: Add p?d_large() definitions

Message ID 20190227170608.27963-12-steven.price@arm.com (mailing list archive)
State Superseded
Headers show
Series None | expand

Commit Message

Steven Price Feb. 27, 2019, 5:05 p.m. UTC
walk_page_range() is going to be allowed to walk page tables other than
those of user space. For this it needs to know when it has reached a
'leaf' entry in the page tables. This information is provided by the
p?d_large() functions/macros.

For mips, we don't support large pages on 32 bit so add stubs returning 0.
For 64 bit look for _PAGE_HUGE flag being set. This means exposing the
flag when !CONFIG_MIPS_HUGE_TLB_SUPPORT.

CC: Ralf Baechle <ralf@linux-mips.org>
CC: Paul Burton <paul.burton@mips.com>
CC: James Hogan <jhogan@kernel.org>
CC: linux-mips@vger.kernel.org
Signed-off-by: Steven Price <steven.price@arm.com>
---
 arch/mips/include/asm/pgtable-32.h   |  5 +++++
 arch/mips/include/asm/pgtable-64.h   | 15 +++++++++++++++
 arch/mips/include/asm/pgtable-bits.h |  2 +-
 3 files changed, 21 insertions(+), 1 deletion(-)

Comments

Paul Burton Feb. 28, 2019, 2:15 a.m. UTC | #1
Hi Steven,

On Wed, Feb 27, 2019 at 05:05:45PM +0000, Steven Price wrote:
> For mips, we don't support large pages on 32 bit so add stubs returning 0.

So far so good :)

> For 64 bit look for _PAGE_HUGE flag being set. This means exposing the
> flag when !CONFIG_MIPS_HUGE_TLB_SUPPORT.

Here I have to ask why? We could just return 0 like the mips32 case when
CONFIG_MIPS_HUGE_TLB_SUPPORT=n, let the compiler optimize the whole
thing out and avoid redundant work at runtime.

This could be unified too in asm/pgtable.h - checking for
CONFIG_MIPS_HUGE_TLB_SUPPORT should be sufficient to cover the mips32
case along with the subset of mips64 configurations without huge pages.

Thanks,
    Paul
Steven Price Feb. 28, 2019, 12:11 p.m. UTC | #2
On 28/02/2019 02:15, Paul Burton wrote:
> Hi Steven,
> 
> On Wed, Feb 27, 2019 at 05:05:45PM +0000, Steven Price wrote:
>> For mips, we don't support large pages on 32 bit so add stubs returning 0.
> 
> So far so good :)
> 
>> For 64 bit look for _PAGE_HUGE flag being set. This means exposing the
>> flag when !CONFIG_MIPS_HUGE_TLB_SUPPORT.
> 
> Here I have to ask why? We could just return 0 like the mips32 case when
> CONFIG_MIPS_HUGE_TLB_SUPPORT=n, let the compiler optimize the whole
> thing out and avoid redundant work at runtime.
> 
> This could be unified too in asm/pgtable.h - checking for
> CONFIG_MIPS_HUGE_TLB_SUPPORT should be sufficient to cover the mips32
> case along with the subset of mips64 configurations without huge pages.

The intention here is to define a new set of macros/functions which will
always tell us whether we're at the leaf of a page table walk, whether
or not huge pages are compiled into the kernel. Basically this allows
the page walking code to be used on page tables other than user space,
for instance the kernel page tables (which e.g. might use a large
mapping for linear memory even if huge pages are not compiled in) or
page tables from firmware (e.g. EFI on arm64).

I'm not familiar enough with mips to know how it handles things like the
linear map so I don't know how relevant that is, but I'm trying to
introduce a new set of functions which differ from the existing
p?d_huge() macros by not depending on whether these mappings could exist
for a user space VMA (i.e. not depending on HUGETLB support and existing
for all levels that architecturally they can occur at).

Steve
Paul Burton Feb. 28, 2019, 6:55 p.m. UTC | #3
Hi Steven,

On Thu, Feb 28, 2019 at 12:11:24PM +0000, Steven Price wrote:
> On 28/02/2019 02:15, Paul Burton wrote:
> > On Wed, Feb 27, 2019 at 05:05:45PM +0000, Steven Price wrote:
> >> For mips, we don't support large pages on 32 bit so add stubs returning 0.
> > 
> > So far so good :)
> > 
> >> For 64 bit look for _PAGE_HUGE flag being set. This means exposing the
> >> flag when !CONFIG_MIPS_HUGE_TLB_SUPPORT.
> > 
> > Here I have to ask why? We could just return 0 like the mips32 case when
> > CONFIG_MIPS_HUGE_TLB_SUPPORT=n, let the compiler optimize the whole
> > thing out and avoid redundant work at runtime.
> > 
> > This could be unified too in asm/pgtable.h - checking for
> > CONFIG_MIPS_HUGE_TLB_SUPPORT should be sufficient to cover the mips32
> > case along with the subset of mips64 configurations without huge pages.
> 
> The intention here is to define a new set of macros/functions which will
> always tell us whether we're at the leaf of a page table walk, whether
> or not huge pages are compiled into the kernel. Basically this allows
> the page walking code to be used on page tables other than user space,
> for instance the kernel page tables (which e.g. might use a large
> mapping for linear memory even if huge pages are not compiled in) or
> page tables from firmware (e.g. EFI on arm64).
> 
> I'm not familiar enough with mips to know how it handles things like the
> linear map so I don't know how relevant that is, but I'm trying to
> introduce a new set of functions which differ from the existing
> p?d_huge() macros by not depending on whether these mappings could exist
> for a user space VMA (i.e. not depending on HUGETLB support and existing
> for all levels that architecturally they can occur at).

Thanks for the explanation - the background helps.

Right now for MIPS, with one exception, there'll be no difference
between a page being huge or large. So for the vast majority of kernels
with CONFIG_MIPS_HUGE_TLB_SUPPORT=n we should just return 0.

The one exception I mentioned is old SGI IP27 support, which allows the
kernel to be mapped through the TLB & does that using 2x 16MB pages when
CONFIG_MAPPED_KERNEL=y. However even there your patch as-is won't pick
up on that for 2 reasons:

  1) The pages in question don't appear to actually be recorded in the
     page tables - they're just written straight into the TLB as wired
     entries (ie. entries that will never be evicted).

  2) Even if they were in the page tables the _PAGE_HUGE bit isn't set.

Since those pages aren't recorded in the page tables anyway we'd either
need to:

  a) Add them to the page tables, and set the _PAGE_HUGE bit.

  b) Ignore them if the code you're working on won't be operating on the
     memory mapping the kernel.

For other platforms the kernel is run from unmapped memory, and for all
cases including IP27 the kernel will use unmapped memory to access
lowmem or peripherals when possible. That is, MIPS has virtual address
regions ((c)kseg[01] or xkphys) which are architecturally defined as
linear maps to physical memory & so VA->PA translation doesn't use the
TLB at all.

So my thought would be that for almost everything we could just do:

  #define pmd_large(pmd)	pmd_huge(pmd)
  #define pud_large(pmd)	pud_huge(pmd)

And whether we need to do anything about IP27 depends on whether a) or
b) is chosen above.

Or alternatively you could do something like:

  #ifdef _PAGE_HUGE

  static inline int pmd_large(pmd_t pmd)
  {
  	return (pmd_val(pmd) & _PAGE_HUGE) != 0;
  }

  static inline int pud_large(pud_t pud)
  {
  	return (pud_val(pud) & _PAGE_HUGE) != 0;
  }

  #else
  # define pmd_large(pmd)	0
  # define pud_large(pud)	0
  #endif

That would cover everything except for the IP27, but would make it pick
up the IP27 kernel pages automatically if someone later defines
_PAGE_HUGE for IP27 CONFIG_MAPPED_KERNEL=y & makes use of it for those
pages.

Thanks,
    Paul
Steven Price March 1, 2019, 11:02 a.m. UTC | #4
On 28/02/2019 18:55, Paul Burton wrote:
> Hi Steven,
> 
> On Thu, Feb 28, 2019 at 12:11:24PM +0000, Steven Price wrote:
>> On 28/02/2019 02:15, Paul Burton wrote:
>>> On Wed, Feb 27, 2019 at 05:05:45PM +0000, Steven Price wrote:
>>>> For mips, we don't support large pages on 32 bit so add stubs returning 0.
>>>
>>> So far so good :)
>>>
>>>> For 64 bit look for _PAGE_HUGE flag being set. This means exposing the
>>>> flag when !CONFIG_MIPS_HUGE_TLB_SUPPORT.
>>>
>>> Here I have to ask why? We could just return 0 like the mips32 case when
>>> CONFIG_MIPS_HUGE_TLB_SUPPORT=n, let the compiler optimize the whole
>>> thing out and avoid redundant work at runtime.
>>>
>>> This could be unified too in asm/pgtable.h - checking for
>>> CONFIG_MIPS_HUGE_TLB_SUPPORT should be sufficient to cover the mips32
>>> case along with the subset of mips64 configurations without huge pages.
>>
>> The intention here is to define a new set of macros/functions which will
>> always tell us whether we're at the leaf of a page table walk, whether
>> or not huge pages are compiled into the kernel. Basically this allows
>> the page walking code to be used on page tables other than user space,
>> for instance the kernel page tables (which e.g. might use a large
>> mapping for linear memory even if huge pages are not compiled in) or
>> page tables from firmware (e.g. EFI on arm64).
>>
>> I'm not familiar enough with mips to know how it handles things like the
>> linear map so I don't know how relevant that is, but I'm trying to
>> introduce a new set of functions which differ from the existing
>> p?d_huge() macros by not depending on whether these mappings could exist
>> for a user space VMA (i.e. not depending on HUGETLB support and existing
>> for all levels that architecturally they can occur at).
> 
> Thanks for the explanation - the background helps.
> 
> Right now for MIPS, with one exception, there'll be no difference
> between a page being huge or large. So for the vast majority of kernels
> with CONFIG_MIPS_HUGE_TLB_SUPPORT=n we should just return 0.
> 
> The one exception I mentioned is old SGI IP27 support, which allows the
> kernel to be mapped through the TLB & does that using 2x 16MB pages when
> CONFIG_MAPPED_KERNEL=y. However even there your patch as-is won't pick
> up on that for 2 reasons:
> 
>   1) The pages in question don't appear to actually be recorded in the
>      page tables - they're just written straight into the TLB as wired
>      entries (ie. entries that will never be evicted).
> 
>   2) Even if they were in the page tables the _PAGE_HUGE bit isn't set.
> 
> Since those pages aren't recorded in the page tables anyway we'd either
> need to:
> 
>   a) Add them to the page tables, and set the _PAGE_HUGE bit.
> 
>   b) Ignore them if the code you're working on won't be operating on the
>      memory mapping the kernel.
> 
> For other platforms the kernel is run from unmapped memory, and for all
> cases including IP27 the kernel will use unmapped memory to access
> lowmem or peripherals when possible. That is, MIPS has virtual address
> regions ((c)kseg[01] or xkphys) which are architecturally defined as
> linear maps to physical memory & so VA->PA translation doesn't use the
> TLB at all.
> 
> So my thought would be that for almost everything we could just do:
> 
>   #define pmd_large(pmd)	pmd_huge(pmd)
>   #define pud_large(pmd)	pud_huge(pmd)
> 
> And whether we need to do anything about IP27 depends on whether a) or
> b) is chosen above.
> 
> Or alternatively you could do something like:
> 
>   #ifdef _PAGE_HUGE
> 
>   static inline int pmd_large(pmd_t pmd)
>   {
>   	return (pmd_val(pmd) & _PAGE_HUGE) != 0;
>   }
> 
>   static inline int pud_large(pud_t pud)
>   {
>   	return (pud_val(pud) & _PAGE_HUGE) != 0;
>   }
> 
>   #else
>   # define pmd_large(pmd)	0
>   # define pud_large(pud)	0
>   #endif
> 
> That would cover everything except for the IP27, but would make it pick
> up the IP27 kernel pages automatically if someone later defines
> _PAGE_HUGE for IP27 CONFIG_MAPPED_KERNEL=y & makes use of it for those
> pages.

Thanks for the detailed explanation. I think my preference is your
change above (#ifdef _PAGE_HUGE) because I'm trying to stop people
thinking p?d_large==p?d_huge. MIPS is a little different from other
architectures in that the hardware doesn't walk the page tables, so
there isn't a definitive answer as to whether there is a 'huge' bit in
the tables or not - it actually does depend on the kernel configuration.

For the IP27 case I think the current situation is probably fine - the
intention is to walk the page tables, so even though the TLBs (and
therefore the actual translations) might not match, at least p?d_large
will accurately tell when the leaf of the page table tree has been
reached. And as you say, using _PAGE_HUGE as the #ifdef means that
should support be added the code should automatically make use of it.

Thanks for your help,

Steve
diff mbox series

Patch

diff --git a/arch/mips/include/asm/pgtable-32.h b/arch/mips/include/asm/pgtable-32.h
index 74afe8c76bdd..58cab62d768b 100644
--- a/arch/mips/include/asm/pgtable-32.h
+++ b/arch/mips/include/asm/pgtable-32.h
@@ -104,6 +104,11 @@  static inline int pmd_present(pmd_t pmd)
 	return pmd_val(pmd) != (unsigned long) invalid_pte_table;
 }
 
+static inline int pmd_large(pmd_t pmd)
+{
+	return 0;
+}
+
 static inline void pmd_clear(pmd_t *pmdp)
 {
 	pmd_val(*pmdp) = ((unsigned long) invalid_pte_table);
diff --git a/arch/mips/include/asm/pgtable-64.h b/arch/mips/include/asm/pgtable-64.h
index 93a9dce31f25..981930e1f843 100644
--- a/arch/mips/include/asm/pgtable-64.h
+++ b/arch/mips/include/asm/pgtable-64.h
@@ -204,6 +204,11 @@  static inline int pgd_present(pgd_t pgd)
 	return pgd_val(pgd) != (unsigned long)invalid_pud_table;
 }
 
+static inline int pgd_large(pgd_t pgd)
+{
+	return 0;
+}
+
 static inline void pgd_clear(pgd_t *pgdp)
 {
 	pgd_val(*pgdp) = (unsigned long)invalid_pud_table;
@@ -273,6 +278,11 @@  static inline int pmd_present(pmd_t pmd)
 	return pmd_val(pmd) != (unsigned long) invalid_pte_table;
 }
 
+static inline int pmd_large(pmd_t pmd)
+{
+	return (pmd_val(pmd) & _PAGE_HUGE) != 0;
+}
+
 static inline void pmd_clear(pmd_t *pmdp)
 {
 	pmd_val(*pmdp) = ((unsigned long) invalid_pte_table);
@@ -297,6 +307,11 @@  static inline int pud_present(pud_t pud)
 	return pud_val(pud) != (unsigned long) invalid_pmd_table;
 }
 
+static inline int pud_large(pud_t pud)
+{
+	return (pud_val(pud) & _PAGE_HUGE) != 0;
+}
+
 static inline void pud_clear(pud_t *pudp)
 {
 	pud_val(*pudp) = ((unsigned long) invalid_pmd_table);
diff --git a/arch/mips/include/asm/pgtable-bits.h b/arch/mips/include/asm/pgtable-bits.h
index f88a48cd68b2..5ab296dee8fa 100644
--- a/arch/mips/include/asm/pgtable-bits.h
+++ b/arch/mips/include/asm/pgtable-bits.h
@@ -132,7 +132,7 @@  enum pgtable_bits {
 #define _PAGE_WRITE		(1 << _PAGE_WRITE_SHIFT)
 #define _PAGE_ACCESSED		(1 << _PAGE_ACCESSED_SHIFT)
 #define _PAGE_MODIFIED		(1 << _PAGE_MODIFIED_SHIFT)
-#if defined(CONFIG_64BIT) && defined(CONFIG_MIPS_HUGE_TLB_SUPPORT)
+#if defined(CONFIG_64BIT)
 # define _PAGE_HUGE		(1 << _PAGE_HUGE_SHIFT)
 #endif