Re: [PATCH] powerpc/powernv: Rework local TLB flush for boot and MCE on POWER9

[Benjamin Herrenschmidt]

On Wed, 2017-06-28 at 08:21 +0530, Aneesh Kumar K.V wrote:
> 
> 
> I am not sure the new location of flushing the tlb is correct/perfect. For ex:
> may be we should do it before htab_initialize() so that we start with
> all everything flushed ? But otherwise

Doesn't matter as long as you do it before you turn on MSR:DR/IR

> Reviewed-by: Aneesh Kumar K.V 
> 
> 
> > 
> > Signed-off-by: Nicholas Piggin 
> > ---
> >  arch/powerpc/include/asm/book3s/64/tlbflush-hash.h |  1 +
> >  .../powerpc/include/asm/book3s/64/tlbflush-radix.h |  3 +
> >  arch/powerpc/include/asm/book3s/64/tlbflush.h  | 34 +
> >  arch/powerpc/include/asm/cputable.h| 12 
> >  arch/powerpc/kernel/cpu_setup_power.S  | 43 
> >  arch/powerpc/kernel/cputable.c | 14 
> >  arch/powerpc/kernel/dt_cpu_ftrs.c  | 42 ---
> >  arch/powerpc/kernel/mce_power.c| 61 +---
> >  arch/powerpc/kvm/book3s_hv_ras.c   |  6 +-
> >  arch/powerpc/mm/hash_native_64.c   | 82 
> > ++
> >  arch/powerpc/mm/hash_utils_64.c|  4 ++
> >  arch/powerpc/mm/pgtable-radix.c|  4 ++
> >  arch/powerpc/mm/tlb-radix.c| 57 +++
> >  13 files changed, 189 insertions(+), 174 deletions(-)
> > 
> > diff --git a/arch/powerpc/include/asm/book3s/64/tlbflush-hash.h 
> > b/arch/powerpc/include/asm/book3s/64/tlbflush-hash.h
> > index 2f6373144e2c..c02ece27fd7b 100644
> > --- a/arch/powerpc/include/asm/book3s/64/tlbflush-hash.h
> > +++ b/arch/powerpc/include/asm/book3s/64/tlbflush-hash.h
> > @@ -50,6 +50,7 @@ static inline void arch_leave_lazy_mmu_mode(void)
> > 
> >  #define arch_flush_lazy_mmu_mode()  do {} while (0)
> > 
> > +extern void hash__tlbiel_all(unsigned int action);
> > 
> >  extern void flush_hash_page(unsigned long vpn, real_pte_t pte, int psize,
> > int ssize, unsigned long flags);
> > diff --git a/arch/powerpc/include/asm/book3s/64/tlbflush-radix.h 
> > b/arch/powerpc/include/asm/book3s/64/tlbflush-radix.h
> > index cc7fbde4f53c..e7b767a3b2fa 100644
> > --- a/arch/powerpc/include/asm/book3s/64/tlbflush-radix.h
> > +++ b/arch/powerpc/include/asm/book3s/64/tlbflush-radix.h
> > @@ -10,6 +10,8 @@ static inline int mmu_get_ap(int psize)
> > return mmu_psize_defs[psize].ap;
> >  }
> > 
> > +extern void radix__tlbiel_all(unsigned int action);
> > +
> >  extern void radix__flush_hugetlb_tlb_range(struct vm_area_struct *vma,
> >unsigned long start, unsigned long 
> > end);
> >  extern void radix__flush_tlb_range_psize(struct mm_struct *mm, unsigned 
> > long start,
> > @@ -44,4 +46,5 @@ extern void radix__flush_tlb_lpid(unsigned long lpid);
> >  extern void radix__flush_tlb_all(void);
> >  extern void radix__flush_tlb_pte_p9_dd1(unsigned long old_pte, struct 
> > mm_struct *mm,
> > unsigned long address);
> > +
> >  #endif
> > diff --git a/arch/powerpc/include/asm/book3s/64/tlbflush.h 
> > b/arch/powerpc/include/asm/book3s/64/tlbflush.h
> > index 72b925f97bab..a6f3a210d4de 100644
> > --- a/arch/powerpc/include/asm/book3s/64/tlbflush.h
> > +++ b/arch/powerpc/include/asm/book3s/64/tlbflush.h
> > @@ -7,6 +7,40 @@
> >  #include 
> >  #include 
> > 
> > +/* TLB flush actions. Used as argument to tlbiel_all() */
> > +enum {
> > +   TLB_INVAL_SCOPE_GLOBAL = 0, /* invalidate all TLBs */
> > +   TLB_INVAL_SCOPE_LPID = 1,   /* invalidate TLBs for current LPID */
> > +};
> > +
> > +static inline void tlbiel_all(void)
> > +{
> > +   /*
> > +* This is used for host machine check and bootup.
> > +*
> > +* This could be reimplemented more robustly without using the
> > +* radix_is_enabled(), cpu_feature(), etc. calls. However these
> > +* should be set up before relocation starts to be used at boot,
> > +* so we shouldn't see TLB machine checks before then.
> > +*/
> > +   if (radix_enabled())
> > +   radix__tlbiel_all(TLB_INVAL_SCOPE_GLOBAL);
> > +   else
> > +   hash__tlbiel_all(TLB_INVAL_SCOPE_GLOBAL);
> > +}
> > +
> > +static inline void tlbiel_all_lpid(bool radix)
> > +{
> > +   /*
> > +* This is used for guest machine check.
> > +*/
> > +   if (radix)
> > +   radix__tlbiel_all(TLB_INVAL_SCOPE_LPID);
> > +   else
> > +   hash__tlbiel_all(TLB_INVAL_SCOPE_LPID);
> > +}
> > +
> > +
> >  #define __HAVE_ARCH_FLUSH_PMD_TLB_RANGE
> >  static inline void flush_pmd_tlb_range(struct vm_area_struct *vma,
> >unsigned long start, unsigned long end)
> > diff --git a/arch/powerpc/include/asm/cputable.h 
> > b/arch/powerpc/include/asm/cputable.h
> > index c2d509584a98..808a5aa4bcf2 100644
> > --- a/arch/powerpc/include/asm/cputable.h
> > +++ b/arch/powerpc/include/asm/cputable.h
> > @@ 

Re: [PATCH] powerpc/powernv: Rework local TLB flush for boot and MCE on POWER9

[Nicholas Piggin]

On Wed, 28 Jun 2017 08:21:55 +0530
"Aneesh Kumar K.V"  wrote:

> Nicholas Piggin  writes:
> 
> > There are two cases outside the normal address space management
> > where a CPU's local TLB is to be flushed:
> >
> >   1. Host boot; in case something has left stale entries in the
> >  TLB (e.g., kexec).
> >
> >   2. Machine check; to clean corrupted TLB entries.
> >
> > CPU state restore from deep idle states also flushes the TLB. However
> > this seems to be a side effect of reusing the boot code to set CPU
> > state, rather than a requirement itself.
> >
> > This type of TLB flush is coded inflexibly, several times for each CPU
> > type, and they have a number of problems with ISA v3.0B:
> >
> > - The current radix mode of the MMU is not taken into account. tlbiel
> >   is undefined if the R field does not match the current radix mode.
> >
> > - ISA v3.0B hash mode should be flushing the partition and process
> >   table caches.
> >
> > - ISA v3.0B radix mode should be flushing partition and process table
> >   caches, and also the page walk cache.
> >
> > To improve this situation, consolidate the flushing code and implement
> > it in C and inline asm under the mm/ directory, and add ISA v3.0B cases
> > for radix and hash.
> >
> > Take it out from early cputable detection hooks, and move it later in
> > the boot process after the MMU registers are set up and before
> > relocation is first turned on.
> >
> > Provide capability for LPID flush to specify radix mode.
> >
> > TLB flush is no longer called when restoring from deep idle states.  
> 
> 
> I am not sure the new location of flushing the tlb is correct/perfect. For ex:
> may be we should do it before htab_initialize() so that we start with
> all everything flushed ? But otherwise
> 
> Reviewed-by: Aneesh Kumar K.V 


Thanks for taking a look over it. The location of the flush is based on
the thinking that:

1. We don't have to flush while MSR IR/DR = 0 because real mode
   translation entries should be correct (if not we have much bigger
   problems). But we must flush before setting IR/DR.

2. We should flush after all setup is done (e.g., all SPRs set) in
   case there is some influence on internal translation structures
   or invalidation.

The conclusion is that we should flush just before turning on MSR IR/DR.

If there is something wrong with my assumptions, it would be be
important to adjust the patch.

Thanks,
Nick

Re: [PATCH] powerpc/powernv: Rework local TLB flush for boot and MCE on POWER9

[Aneesh Kumar K.V]

Nicholas Piggin  writes:

> There are two cases outside the normal address space management
> where a CPU's local TLB is to be flushed:
>
>   1. Host boot; in case something has left stale entries in the
>  TLB (e.g., kexec).
>
>   2. Machine check; to clean corrupted TLB entries.
>
> CPU state restore from deep idle states also flushes the TLB. However
> this seems to be a side effect of reusing the boot code to set CPU
> state, rather than a requirement itself.
>
> This type of TLB flush is coded inflexibly, several times for each CPU
> type, and they have a number of problems with ISA v3.0B:
>
> - The current radix mode of the MMU is not taken into account. tlbiel
>   is undefined if the R field does not match the current radix mode.
>
> - ISA v3.0B hash mode should be flushing the partition and process
>   table caches.
>
> - ISA v3.0B radix mode should be flushing partition and process table
>   caches, and also the page walk cache.
>
> To improve this situation, consolidate the flushing code and implement
> it in C and inline asm under the mm/ directory, and add ISA v3.0B cases
> for radix and hash.
>
> Take it out from early cputable detection hooks, and move it later in
> the boot process after the MMU registers are set up and before
> relocation is first turned on.
>
> Provide capability for LPID flush to specify radix mode.
>
> TLB flush is no longer called when restoring from deep idle states.


I am not sure the new location of flushing the tlb is correct/perfect. For ex:
may be we should do it before htab_initialize() so that we start with
all everything flushed ? But otherwise

Reviewed-by: Aneesh Kumar K.V 


>
> Signed-off-by: Nicholas Piggin 
> ---
>  arch/powerpc/include/asm/book3s/64/tlbflush-hash.h |  1 +
>  .../powerpc/include/asm/book3s/64/tlbflush-radix.h |  3 +
>  arch/powerpc/include/asm/book3s/64/tlbflush.h  | 34 +
>  arch/powerpc/include/asm/cputable.h| 12 
>  arch/powerpc/kernel/cpu_setup_power.S  | 43 
>  arch/powerpc/kernel/cputable.c | 14 
>  arch/powerpc/kernel/dt_cpu_ftrs.c  | 42 ---
>  arch/powerpc/kernel/mce_power.c| 61 +---
>  arch/powerpc/kvm/book3s_hv_ras.c   |  6 +-
>  arch/powerpc/mm/hash_native_64.c   | 82 
> ++
>  arch/powerpc/mm/hash_utils_64.c|  4 ++
>  arch/powerpc/mm/pgtable-radix.c|  4 ++
>  arch/powerpc/mm/tlb-radix.c| 57 +++
>  13 files changed, 189 insertions(+), 174 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/book3s/64/tlbflush-hash.h 
> b/arch/powerpc/include/asm/book3s/64/tlbflush-hash.h
> index 2f6373144e2c..c02ece27fd7b 100644
> --- a/arch/powerpc/include/asm/book3s/64/tlbflush-hash.h
> +++ b/arch/powerpc/include/asm/book3s/64/tlbflush-hash.h
> @@ -50,6 +50,7 @@ static inline void arch_leave_lazy_mmu_mode(void)
>
>  #define arch_flush_lazy_mmu_mode()  do {} while (0)
>
> +extern void hash__tlbiel_all(unsigned int action);
>
>  extern void flush_hash_page(unsigned long vpn, real_pte_t pte, int psize,
>   int ssize, unsigned long flags);
> diff --git a/arch/powerpc/include/asm/book3s/64/tlbflush-radix.h 
> b/arch/powerpc/include/asm/book3s/64/tlbflush-radix.h
> index cc7fbde4f53c..e7b767a3b2fa 100644
> --- a/arch/powerpc/include/asm/book3s/64/tlbflush-radix.h
> +++ b/arch/powerpc/include/asm/book3s/64/tlbflush-radix.h
> @@ -10,6 +10,8 @@ static inline int mmu_get_ap(int psize)
>   return mmu_psize_defs[psize].ap;
>  }
>
> +extern void radix__tlbiel_all(unsigned int action);
> +
>  extern void radix__flush_hugetlb_tlb_range(struct vm_area_struct *vma,
>  unsigned long start, unsigned long 
> end);
>  extern void radix__flush_tlb_range_psize(struct mm_struct *mm, unsigned long 
> start,
> @@ -44,4 +46,5 @@ extern void radix__flush_tlb_lpid(unsigned long lpid);
>  extern void radix__flush_tlb_all(void);
>  extern void radix__flush_tlb_pte_p9_dd1(unsigned long old_pte, struct 
> mm_struct *mm,
>   unsigned long address);
> +
>  #endif
> diff --git a/arch/powerpc/include/asm/book3s/64/tlbflush.h 
> b/arch/powerpc/include/asm/book3s/64/tlbflush.h
> index 72b925f97bab..a6f3a210d4de 100644
> --- a/arch/powerpc/include/asm/book3s/64/tlbflush.h
> +++ b/arch/powerpc/include/asm/book3s/64/tlbflush.h
> @@ -7,6 +7,40 @@
>  #include 
>  #include 
>
> +/* TLB flush actions. Used as argument to tlbiel_all() */
> +enum {
> + TLB_INVAL_SCOPE_GLOBAL = 0, /* invalidate all TLBs */
> + TLB_INVAL_SCOPE_LPID = 1,   /* invalidate TLBs for current LPID */
> +};
> +
> +static inline void tlbiel_all(void)
> +{
> + /*
> +  * This is used for host machine check and bootup.
> +  *
> +  * This could be 

Re: [PATCH] powerpc/powernv: Rework local TLB flush for boot and MCE on POWER9

[kbuild test robot]

Hi Nicholas,

[auto build test ERROR on powerpc/next]
[also build test ERROR on v4.12-rc4 next-20170607]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Nicholas-Piggin/powerpc-powernv-Rework-local-TLB-flush-for-boot-and-MCE-on-POWER9/20170607-084940
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-ps3_defconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget 
https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=powerpc 

All errors (new ones prefixed by >>):

   arch/powerpc/kernel/built-in.o: In function `tlbiel_all':
>> arch/powerpc/include/asm/book3s/64/tlbflush.h:29: undefined reference to 
>> `.hash__tlbiel_all'
   arch/powerpc/mm/built-in.o: In function `tlbiel_all':
>> arch/powerpc/include/asm/book3s/64/tlbflush.h:29: undefined reference to 
>> `.hash__tlbiel_all'
>> arch/powerpc/include/asm/book3s/64/tlbflush.h:29: undefined reference to 
>> `.hash__tlbiel_all'
>> arch/powerpc/include/asm/book3s/64/tlbflush.h:29: undefined reference to 
>> `.hash__tlbiel_all'
>> arch/powerpc/include/asm/book3s/64/tlbflush.h:29: undefined reference to 
>> `.hash__tlbiel_all'

vim +29 arch/powerpc/include/asm/book3s/64/tlbflush.h

23   * should be set up before relocation starts to be used at boot,
24   * so we shouldn't see TLB machine checks before then.
25   */
26  if (radix_enabled())
27  radix__tlbiel_all(TLB_INVAL_SCOPE_GLOBAL);
28  else
  > 29  hash__tlbiel_all(TLB_INVAL_SCOPE_GLOBAL);
30  }
31  
32  static inline void tlbiel_all_lpid(bool radix)

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

[PATCH] powerpc/powernv: Rework local TLB flush for boot and MCE on POWER9

[Nicholas Piggin]

There are two cases outside the normal address space management
where a CPU's local TLB is to be flushed:

  1. Host boot; in case something has left stale entries in the
 TLB (e.g., kexec).

  2. Machine check; to clean corrupted TLB entries.

CPU state restore from deep idle states also flushes the TLB. However
this seems to be a side effect of reusing the boot code to set CPU
state, rather than a requirement itself.

This type of TLB flush is coded inflexibly, several times for each CPU
type, and they have a number of problems with ISA v3.0B:

- The current radix mode of the MMU is not taken into account. tlbiel
  is undefined if the R field does not match the current radix mode.

- ISA v3.0B hash mode should be flushing the partition and process
  table caches.

- ISA v3.0B radix mode should be flushing partition and process table
  caches, and also the page walk cache.

To improve this situation, consolidate the flushing code and implement
it in C and inline asm under the mm/ directory, and add ISA v3.0B cases
for radix and hash.

Take it out from early cputable detection hooks, and move it later in
the boot process after the MMU registers are set up and before
relocation is first turned on.

Provide capability for LPID flush to specify radix mode.

TLB flush is no longer called when restoring from deep idle states.

Signed-off-by: Nicholas Piggin 
---
 arch/powerpc/include/asm/book3s/64/tlbflush-hash.h |  1 +
 .../powerpc/include/asm/book3s/64/tlbflush-radix.h |  3 +
 arch/powerpc/include/asm/book3s/64/tlbflush.h  | 34 +
 arch/powerpc/include/asm/cputable.h| 12 
 arch/powerpc/kernel/cpu_setup_power.S  | 43 
 arch/powerpc/kernel/cputable.c | 14 
 arch/powerpc/kernel/dt_cpu_ftrs.c  | 42 ---
 arch/powerpc/kernel/mce_power.c| 61 +---
 arch/powerpc/kvm/book3s_hv_ras.c   |  6 +-
 arch/powerpc/mm/hash_native_64.c   | 82 ++
 arch/powerpc/mm/hash_utils_64.c|  4 ++
 arch/powerpc/mm/pgtable-radix.c|  4 ++
 arch/powerpc/mm/tlb-radix.c| 57 +++
 13 files changed, 189 insertions(+), 174 deletions(-)

diff --git a/arch/powerpc/include/asm/book3s/64/tlbflush-hash.h 
b/arch/powerpc/include/asm/book3s/64/tlbflush-hash.h
index 2f6373144e2c..c02ece27fd7b 100644
--- a/arch/powerpc/include/asm/book3s/64/tlbflush-hash.h
+++ b/arch/powerpc/include/asm/book3s/64/tlbflush-hash.h
@@ -50,6 +50,7 @@ static inline void arch_leave_lazy_mmu_mode(void)
 
 #define arch_flush_lazy_mmu_mode()  do {} while (0)
 
+extern void hash__tlbiel_all(unsigned int action);
 
 extern void flush_hash_page(unsigned long vpn, real_pte_t pte, int psize,
int ssize, unsigned long flags);
diff --git a/arch/powerpc/include/asm/book3s/64/tlbflush-radix.h 
b/arch/powerpc/include/asm/book3s/64/tlbflush-radix.h
index cc7fbde4f53c..e7b767a3b2fa 100644
--- a/arch/powerpc/include/asm/book3s/64/tlbflush-radix.h
+++ b/arch/powerpc/include/asm/book3s/64/tlbflush-radix.h
@@ -10,6 +10,8 @@ static inline int mmu_get_ap(int psize)
return mmu_psize_defs[psize].ap;
 }
 
+extern void radix__tlbiel_all(unsigned int action);
+
 extern void radix__flush_hugetlb_tlb_range(struct vm_area_struct *vma,
   unsigned long start, unsigned long 
end);
 extern void radix__flush_tlb_range_psize(struct mm_struct *mm, unsigned long 
start,
@@ -44,4 +46,5 @@ extern void radix__flush_tlb_lpid(unsigned long lpid);
 extern void radix__flush_tlb_all(void);
 extern void radix__flush_tlb_pte_p9_dd1(unsigned long old_pte, struct 
mm_struct *mm,
unsigned long address);
+
 #endif
diff --git a/arch/powerpc/include/asm/book3s/64/tlbflush.h 
b/arch/powerpc/include/asm/book3s/64/tlbflush.h
index 72b925f97bab..a6f3a210d4de 100644
--- a/arch/powerpc/include/asm/book3s/64/tlbflush.h
+++ b/arch/powerpc/include/asm/book3s/64/tlbflush.h
@@ -7,6 +7,40 @@
 #include 
 #include 
 
+/* TLB flush actions. Used as argument to tlbiel_all() */
+enum {
+   TLB_INVAL_SCOPE_GLOBAL = 0, /* invalidate all TLBs */
+   TLB_INVAL_SCOPE_LPID = 1,   /* invalidate TLBs for current LPID */
+};
+
+static inline void tlbiel_all(void)
+{
+   /*
+* This is used for host machine check and bootup.
+*
+* This could be reimplemented more robustly without using the
+* radix_is_enabled(), cpu_feature(), etc. calls. However these
+* should be set up before relocation starts to be used at boot,
+* so we shouldn't see TLB machine checks before then.
+*/
+   if (radix_enabled())
+   radix__tlbiel_all(TLB_INVAL_SCOPE_GLOBAL);
+   else
+   hash__tlbiel_all(TLB_INVAL_SCOPE_GLOBAL);
+}
+
+static inline void tlbiel_all_lpid(bool radix)
+{
+