* [PATCH v1 12/20] powerpc/32s: Inline flush_tlb_range() and flush_tlb_kernel_range()
From: Christophe Leroy @ 2020-10-22 6:29 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <648e2448e938d52d0b5887445e018ca584edc06d.1603348103.git.christophe.leroy@csgroup.eu>
flush_tlb_range() and flush_tlb_kernel_range() are trivial calls to
flush_range().
Make flush_range() global and inline flush_tlb_range()
and flush_tlb_kernel_range().
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/book3s/32/tlbflush.h | 15 ++++++++--
arch/powerpc/mm/book3s32/tlb.c | 30 +++++--------------
2 files changed, 19 insertions(+), 26 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/32/tlbflush.h b/arch/powerpc/include/asm/book3s/32/tlbflush.h
index 542765944531..2f480d184526 100644
--- a/arch/powerpc/include/asm/book3s/32/tlbflush.h
+++ b/arch/powerpc/include/asm/book3s/32/tlbflush.h
@@ -8,9 +8,7 @@
*/
void hash__flush_tlb_mm(struct mm_struct *mm);
void hash__flush_tlb_page(struct vm_area_struct *vma, unsigned long vmaddr);
-extern void flush_tlb_range(struct vm_area_struct *vma, unsigned long start,
- unsigned long end);
-extern void flush_tlb_kernel_range(unsigned long start, unsigned long end);
+void flush_range(struct mm_struct *mm, unsigned long start, unsigned long end);
#ifdef CONFIG_SMP
void _tlbie(unsigned long address);
@@ -38,6 +36,17 @@ static inline void flush_tlb_page(struct vm_area_struct *vma, unsigned long vmad
_tlbie(vmaddr);
}
+static inline void
+flush_tlb_range(struct vm_area_struct *vma, unsigned long start, unsigned long end)
+{
+ flush_range(vma->vm_mm, start, end);
+}
+
+static inline void flush_tlb_kernel_range(unsigned long start, unsigned long end)
+{
+ flush_range(&init_mm, start, end);
+}
+
static inline void local_flush_tlb_page(struct vm_area_struct *vma,
unsigned long vmaddr)
{
diff --git a/arch/powerpc/mm/book3s32/tlb.c b/arch/powerpc/mm/book3s32/tlb.c
index 65389bfe2eb8..f9b8e1ce4371 100644
--- a/arch/powerpc/mm/book3s32/tlb.c
+++ b/arch/powerpc/mm/book3s32/tlb.c
@@ -71,8 +71,12 @@ void tlb_flush(struct mmu_gather *tlb)
* -- Cort
*/
-static void flush_range(struct mm_struct *mm, unsigned long start,
- unsigned long end)
+/*
+ * For each address in the range, find the pte for the address
+ * and check _PAGE_HASHPTE bit; if it is set, find and destroy
+ * the corresponding HPTE.
+ */
+void flush_range(struct mm_struct *mm, unsigned long start, unsigned long end)
{
pmd_t *pmd;
unsigned long pmd_end;
@@ -105,15 +109,7 @@ static void flush_range(struct mm_struct *mm, unsigned long start,
++pmd;
}
}
-
-/*
- * Flush kernel TLB entries in the given range
- */
-void flush_tlb_kernel_range(unsigned long start, unsigned long end)
-{
- flush_range(&init_mm, start, end);
-}
-EXPORT_SYMBOL(flush_tlb_kernel_range);
+EXPORT_SYMBOL(flush_range);
/*
* Flush all the (user) entries for the address space described by mm.
@@ -145,18 +141,6 @@ void hash__flush_tlb_page(struct vm_area_struct *vma, unsigned long vmaddr)
}
EXPORT_SYMBOL(hash__flush_tlb_page);
-/*
- * For each address in the range, find the pte for the address
- * and check _PAGE_HASHPTE bit; if it is set, find and destroy
- * the corresponding HPTE.
- */
-void flush_tlb_range(struct vm_area_struct *vma, unsigned long start,
- unsigned long end)
-{
- flush_range(vma->vm_mm, start, end);
-}
-EXPORT_SYMBOL(flush_tlb_range);
-
void __init early_init_mmu(void)
{
}
--
2.25.0
^ permalink raw reply related
* [PATCH v1 13/20] powerpc/32s: Split and inline flush_range()
From: Christophe Leroy @ 2020-10-22 6:29 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <648e2448e938d52d0b5887445e018ca584edc06d.1603348103.git.christophe.leroy@csgroup.eu>
flush_range() handle both the MMU_FTR_HPTE_TABLE case and
the other case.
The non MMU_FTR_HPTE_TABLE case is trivial as it is only a call
to _tlbie()/_tlbia() which is not worth a dedicated function.
Make flush_range() a hash specific and call it from tlbflush.h based
on mmu_has_feature(MMU_FTR_HPTE_TABLE).
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/book3s/32/tlbflush.h | 13 ++++++++++++-
arch/powerpc/mm/book3s32/tlb.c | 13 +++----------
2 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/32/tlbflush.h b/arch/powerpc/include/asm/book3s/32/tlbflush.h
index 2f480d184526..42708c1719d6 100644
--- a/arch/powerpc/include/asm/book3s/32/tlbflush.h
+++ b/arch/powerpc/include/asm/book3s/32/tlbflush.h
@@ -8,7 +8,7 @@
*/
void hash__flush_tlb_mm(struct mm_struct *mm);
void hash__flush_tlb_page(struct vm_area_struct *vma, unsigned long vmaddr);
-void flush_range(struct mm_struct *mm, unsigned long start, unsigned long end);
+void hash__flush_range(struct mm_struct *mm, unsigned long start, unsigned long end);
#ifdef CONFIG_SMP
void _tlbie(unsigned long address);
@@ -20,6 +20,17 @@ static inline void _tlbie(unsigned long address)
#endif
void _tlbia(void);
+static inline void flush_range(struct mm_struct *mm, unsigned long start, unsigned long end)
+{
+ start &= PAGE_MASK;
+ if (mmu_has_feature(MMU_FTR_HPTE_TABLE))
+ hash__flush_range(mm, start, end);
+ else if (end - start <= PAGE_SIZE)
+ _tlbie(start);
+ else
+ _tlbia();
+}
+
static inline void flush_tlb_mm(struct mm_struct *mm)
{
if (mmu_has_feature(MMU_FTR_HPTE_TABLE))
diff --git a/arch/powerpc/mm/book3s32/tlb.c b/arch/powerpc/mm/book3s32/tlb.c
index f9b8e1ce4371..f0edbad5966c 100644
--- a/arch/powerpc/mm/book3s32/tlb.c
+++ b/arch/powerpc/mm/book3s32/tlb.c
@@ -76,7 +76,7 @@ void tlb_flush(struct mmu_gather *tlb)
* and check _PAGE_HASHPTE bit; if it is set, find and destroy
* the corresponding HPTE.
*/
-void flush_range(struct mm_struct *mm, unsigned long start, unsigned long end)
+void hash__flush_range(struct mm_struct *mm, unsigned long start, unsigned long end)
{
pmd_t *pmd;
unsigned long pmd_end;
@@ -84,13 +84,6 @@ void flush_range(struct mm_struct *mm, unsigned long start, unsigned long end)
unsigned int ctx = mm->context.id;
start &= PAGE_MASK;
- if (!mmu_has_feature(MMU_FTR_HPTE_TABLE)) {
- if (end - start <= PAGE_SIZE)
- _tlbie(start);
- else
- _tlbia();
- return;
- }
if (start >= end)
return;
end = (end - 1) | ~PAGE_MASK;
@@ -109,7 +102,7 @@ void flush_range(struct mm_struct *mm, unsigned long start, unsigned long end)
++pmd;
}
}
-EXPORT_SYMBOL(flush_range);
+EXPORT_SYMBOL(hash__flush_range);
/*
* Flush all the (user) entries for the address space described by mm.
@@ -125,7 +118,7 @@ void hash__flush_tlb_mm(struct mm_struct *mm)
* but it seems dup_mmap is the only SMP case which gets here.
*/
for (mp = mm->mmap; mp != NULL; mp = mp->vm_next)
- flush_range(mp->vm_mm, mp->vm_start, mp->vm_end);
+ hash__flush_range(mp->vm_mm, mp->vm_start, mp->vm_end);
}
EXPORT_SYMBOL(hash__flush_tlb_mm);
--
2.25.0
^ permalink raw reply related
* [PATCH v1 14/20] powerpc/32s: Inline tlb_flush()
From: Christophe Leroy @ 2020-10-22 6:29 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <648e2448e938d52d0b5887445e018ca584edc06d.1603348103.git.christophe.leroy@csgroup.eu>
On book3s/32, tlb_flush() does nothing when the CPU has a hash table,
it calls _tlbia() otherwise.
Inline it.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/book3s/32/tlbflush.h | 11 +++++++++++
arch/powerpc/mm/book3s32/tlb.c | 15 ---------------
2 files changed, 11 insertions(+), 15 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/32/tlbflush.h b/arch/powerpc/include/asm/book3s/32/tlbflush.h
index 42708c1719d6..d941c06d4f2e 100644
--- a/arch/powerpc/include/asm/book3s/32/tlbflush.h
+++ b/arch/powerpc/include/asm/book3s/32/tlbflush.h
@@ -20,6 +20,17 @@ static inline void _tlbie(unsigned long address)
#endif
void _tlbia(void);
+/*
+ * Called at the end of a mmu_gather operation to make sure the
+ * TLB flush is completely done.
+ */
+static inline void tlb_flush(struct mmu_gather *tlb)
+{
+ /* 603 needs to flush the whole TLB here since it doesn't use a hash table. */
+ if (!mmu_has_feature(MMU_FTR_HPTE_TABLE))
+ _tlbia();
+}
+
static inline void flush_range(struct mm_struct *mm, unsigned long start, unsigned long end)
{
start &= PAGE_MASK;
diff --git a/arch/powerpc/mm/book3s32/tlb.c b/arch/powerpc/mm/book3s32/tlb.c
index f0edbad5966c..e7865a3f0231 100644
--- a/arch/powerpc/mm/book3s32/tlb.c
+++ b/arch/powerpc/mm/book3s32/tlb.c
@@ -43,21 +43,6 @@ void flush_hash_entry(struct mm_struct *mm, pte_t *ptep, unsigned long addr)
}
EXPORT_SYMBOL(flush_hash_entry);
-/*
- * Called at the end of a mmu_gather operation to make sure the
- * TLB flush is completely done.
- */
-void tlb_flush(struct mmu_gather *tlb)
-{
- if (!mmu_has_feature(MMU_FTR_HPTE_TABLE)) {
- /*
- * 603 needs to flush the whole TLB here since
- * it doesn't use a hash table.
- */
- _tlbia();
- }
-}
-
/*
* TLB flushing:
*
--
2.25.0
^ permalink raw reply related
* [PATCH v1 15/20] powerpc/32s: Inline flush_hash_entry()
From: Christophe Leroy @ 2020-10-22 6:29 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <648e2448e938d52d0b5887445e018ca584edc06d.1603348103.git.christophe.leroy@csgroup.eu>
flush_hash_entry() is a simple function calling
flush_hash_pages() if it's a hash MMU or doing nothing otherwise.
Inline it.
And use it also in __ptep_test_and_clear_young().
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/book3s/32/pgtable.h | 17 +++++++++++------
arch/powerpc/include/asm/tlb.h | 3 ---
arch/powerpc/mm/book3s32/tlb.c | 14 --------------
3 files changed, 11 insertions(+), 23 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/32/pgtable.h b/arch/powerpc/include/asm/book3s/32/pgtable.h
index 36443cda8dcf..914c19959a84 100644
--- a/arch/powerpc/include/asm/book3s/32/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/32/pgtable.h
@@ -238,8 +238,14 @@ extern void add_hash_page(unsigned context, unsigned long va,
unsigned long pmdval);
/* Flush an entry from the TLB/hash table */
-extern void flush_hash_entry(struct mm_struct *mm, pte_t *ptep,
- unsigned long address);
+static inline void flush_hash_entry(struct mm_struct *mm, pte_t *ptep, unsigned long addr)
+{
+ if (mmu_has_feature(MMU_FTR_HPTE_TABLE)) {
+ unsigned long ptephys = __pa(ptep) & PAGE_MASK;
+
+ flush_hash_pages(mm->context.id, addr, ptephys, 1);
+ }
+}
/*
* PTE updates. This function is called whenever an existing
@@ -291,10 +297,9 @@ static inline int __ptep_test_and_clear_young(struct mm_struct *mm,
{
unsigned long old;
old = pte_update(mm, addr, ptep, _PAGE_ACCESSED, 0, 0);
- if (old & _PAGE_HASHPTE) {
- unsigned long ptephys = __pa(ptep) & PAGE_MASK;
- flush_hash_pages(mm->context.id, addr, ptephys, 1);
- }
+ if (old & _PAGE_HASHPTE)
+ flush_hash_entry(mm, ptep, addr);
+
return (old & _PAGE_ACCESSED) != 0;
}
#define ptep_test_and_clear_young(__vma, __addr, __ptep) \
diff --git a/arch/powerpc/include/asm/tlb.h b/arch/powerpc/include/asm/tlb.h
index d97f061fecac..160422a439aa 100644
--- a/arch/powerpc/include/asm/tlb.h
+++ b/arch/powerpc/include/asm/tlb.h
@@ -40,9 +40,6 @@ extern void tlb_flush(struct mmu_gather *tlb);
/* Get the generic bits... */
#include <asm-generic/tlb.h>
-extern void flush_hash_entry(struct mm_struct *mm, pte_t *ptep,
- unsigned long address);
-
static inline void __tlb_remove_tlb_entry(struct mmu_gather *tlb, pte_t *ptep,
unsigned long address)
{
diff --git a/arch/powerpc/mm/book3s32/tlb.c b/arch/powerpc/mm/book3s32/tlb.c
index e7865a3f0231..0d412953fe58 100644
--- a/arch/powerpc/mm/book3s32/tlb.c
+++ b/arch/powerpc/mm/book3s32/tlb.c
@@ -29,20 +29,6 @@
#include <mm/mmu_decl.h>
-/*
- * Called when unmapping pages to flush entries from the TLB/hash table.
- */
-void flush_hash_entry(struct mm_struct *mm, pte_t *ptep, unsigned long addr)
-{
- unsigned long ptephys;
-
- if (mmu_has_feature(MMU_FTR_HPTE_TABLE)) {
- ptephys = __pa(ptep) & PAGE_MASK;
- flush_hash_pages(mm->context.id, addr, ptephys, 1);
- }
-}
-EXPORT_SYMBOL(flush_hash_entry);
-
/*
* TLB flushing:
*
--
2.25.0
^ permalink raw reply related
* [PATCH v1 16/20] powerpc/32s: Move early_mmu_init() into mmu.c
From: Christophe Leroy @ 2020-10-22 6:29 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <648e2448e938d52d0b5887445e018ca584edc06d.1603348103.git.christophe.leroy@csgroup.eu>
early_mmu_init() is independent of MMU type and not
directly linked to tlb handling.
In a following patch, tlb.c will be restricted to HASH mmu.
Move early_mmu_init() into mmu.c which is common.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/mm/book3s32/mmu.c | 4 ++++
arch/powerpc/mm/book3s32/tlb.c | 4 ----
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/mm/book3s32/mmu.c b/arch/powerpc/mm/book3s32/mmu.c
index e7ff1ec73499..e58f48b07b46 100644
--- a/arch/powerpc/mm/book3s32/mmu.c
+++ b/arch/powerpc/mm/book3s32/mmu.c
@@ -473,3 +473,7 @@ void __init setup_kuap(bool disabled)
pr_warn("KUAP cannot be disabled yet on 6xx when compiled in\n");
}
#endif
+
+void __init early_init_mmu(void)
+{
+}
diff --git a/arch/powerpc/mm/book3s32/tlb.c b/arch/powerpc/mm/book3s32/tlb.c
index 0d412953fe58..19f0ef950d77 100644
--- a/arch/powerpc/mm/book3s32/tlb.c
+++ b/arch/powerpc/mm/book3s32/tlb.c
@@ -104,7 +104,3 @@ void hash__flush_tlb_page(struct vm_area_struct *vma, unsigned long vmaddr)
flush_hash_pages(mm->context.id, vmaddr, pmd_val(*pmd), 1);
}
EXPORT_SYMBOL(hash__flush_tlb_page);
-
-void __init early_init_mmu(void)
-{
-}
--
2.25.0
^ permalink raw reply related
* [PATCH v1 17/20] powerpc/32s: Remove CONFIG_PPC_BOOK3S_6xx
From: Christophe Leroy @ 2020-10-22 6:29 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <648e2448e938d52d0b5887445e018ca584edc06d.1603348103.git.christophe.leroy@csgroup.eu>
As 601 is gone, CONFIG_PPC_BOO3S_6xx and CONFIG_PPC_BOOK3S_32
are dedundant.
Remove CONFIG_PPC_BOOK3S_6xx.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/kernel/cputable.c | 4 ++--
arch/powerpc/platforms/Kconfig.cputype | 6 +-----
2 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputable.c
index 492c0b36aff6..0828a7756595 100644
--- a/arch/powerpc/kernel/cputable.c
+++ b/arch/powerpc/kernel/cputable.c
@@ -609,7 +609,7 @@ static struct cpu_spec __initdata cpu_specs[] = {
#endif /* CONFIG_PPC_BOOK3S_64 */
#ifdef CONFIG_PPC32
-#ifdef CONFIG_PPC_BOOK3S_6xx
+#ifdef CONFIG_PPC_BOOK3S_32
{ /* 603 */
.pvr_mask = 0xffff0000,
.pvr_value = 0x00030000,
@@ -1239,7 +1239,7 @@ static struct cpu_spec __initdata cpu_specs[] = {
.machine_check = machine_check_generic,
.platform = "ppc603",
},
-#endif /* CONFIG_PPC_BOOK3S_6xx */
+#endif /* CONFIG_PPC_BOOK3S_32 */
#ifdef CONFIG_PPC_8xx
{ /* 8xx */
.pvr_mask = 0xffff0000,
diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
index c194c4ae8bc7..818a41c9e274 100644
--- a/arch/powerpc/platforms/Kconfig.cputype
+++ b/arch/powerpc/platforms/Kconfig.cputype
@@ -11,9 +11,6 @@ config PPC64
This option selects whether a 32-bit or a 64-bit kernel
will be built.
-config PPC_BOOK3S_32
- bool
-
menu "Processor support"
choice
prompt "Processor Type"
@@ -29,9 +26,8 @@ choice
If unsure, select 52xx/6xx/7xx/74xx/82xx/83xx/86xx.
-config PPC_BOOK3S_6xx
+config PPC_BOOK3S_32
bool "512x/52xx/6xx/7xx/74xx/82xx/83xx/86xx"
- select PPC_BOOK3S_32
select PPC_FPU
select PPC_HAVE_PMU_SUPPORT
select PPC_HAVE_KUEP
--
2.25.0
^ permalink raw reply related
* [PATCH v1 18/20] powerpc/32s: Regroup 603 based CPUs in cputable
From: Christophe Leroy @ 2020-10-22 6:29 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <648e2448e938d52d0b5887445e018ca584edc06d.1603348103.git.christophe.leroy@csgroup.eu>
In order to selectively build the kernel for 603 SW TLB handling,
regroup all 603 based CPUs together.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/cputable.h | 14 +++---
arch/powerpc/kernel/cputable.c | 78 ++++++++++++++---------------
2 files changed, 47 insertions(+), 45 deletions(-)
diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
index c596bab134e2..89f2d6b68cd7 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -510,15 +510,16 @@ static inline void cpu_feature_keys_init(void) { }
enum {
CPU_FTRS_POSSIBLE =
#ifdef CONFIG_PPC_BOOK3S_32
- CPU_FTRS_603 | CPU_FTRS_604 | CPU_FTRS_740_NOTAU |
+ CPU_FTRS_604 | CPU_FTRS_740_NOTAU |
CPU_FTRS_740 | CPU_FTRS_750 | CPU_FTRS_750FX1 |
CPU_FTRS_750FX2 | CPU_FTRS_750FX | CPU_FTRS_750GX |
CPU_FTRS_7400_NOTAU | CPU_FTRS_7400 | CPU_FTRS_7450_20 |
CPU_FTRS_7450_21 | CPU_FTRS_7450_23 | CPU_FTRS_7455_1 |
CPU_FTRS_7455_20 | CPU_FTRS_7455 | CPU_FTRS_7447_10 |
- CPU_FTRS_7447 | CPU_FTRS_7447A | CPU_FTRS_82XX |
- CPU_FTRS_G2_LE | CPU_FTRS_E300 | CPU_FTRS_E300C2 |
+ CPU_FTRS_7447 | CPU_FTRS_7447A |
CPU_FTRS_CLASSIC32 |
+ CPU_FTRS_603 | CPU_FTRS_82XX |
+ CPU_FTRS_G2_LE | CPU_FTRS_E300 | CPU_FTRS_E300C2 |
#endif
#ifdef CONFIG_PPC_8xx
CPU_FTRS_8XX |
@@ -584,15 +585,16 @@ enum {
enum {
CPU_FTRS_ALWAYS =
#ifdef CONFIG_PPC_BOOK3S_32
- CPU_FTRS_603 & CPU_FTRS_604 & CPU_FTRS_740_NOTAU &
+ CPU_FTRS_604 & CPU_FTRS_740_NOTAU &
CPU_FTRS_740 & CPU_FTRS_750 & CPU_FTRS_750FX1 &
CPU_FTRS_750FX2 & CPU_FTRS_750FX & CPU_FTRS_750GX &
CPU_FTRS_7400_NOTAU & CPU_FTRS_7400 & CPU_FTRS_7450_20 &
CPU_FTRS_7450_21 & CPU_FTRS_7450_23 & CPU_FTRS_7455_1 &
CPU_FTRS_7455_20 & CPU_FTRS_7455 & CPU_FTRS_7447_10 &
- CPU_FTRS_7447 & CPU_FTRS_7447A & CPU_FTRS_82XX &
- CPU_FTRS_G2_LE & CPU_FTRS_E300 & CPU_FTRS_E300C2 &
+ CPU_FTRS_7447 & CPU_FTRS_7447A &
CPU_FTRS_CLASSIC32 &
+ CPU_FTRS_603 & CPU_FTRS_82XX &
+ CPU_FTRS_G2_LE & CPU_FTRS_E300 & CPU_FTRS_E300C2 &
#endif
#ifdef CONFIG_PPC_8xx
CPU_FTRS_8XX &
diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputable.c
index 0828a7756595..ba96127d2e8c 100644
--- a/arch/powerpc/kernel/cputable.c
+++ b/arch/powerpc/kernel/cputable.c
@@ -610,45 +610,6 @@ static struct cpu_spec __initdata cpu_specs[] = {
#ifdef CONFIG_PPC32
#ifdef CONFIG_PPC_BOOK3S_32
- { /* 603 */
- .pvr_mask = 0xffff0000,
- .pvr_value = 0x00030000,
- .cpu_name = "603",
- .cpu_features = CPU_FTRS_603,
- .cpu_user_features = COMMON_USER | PPC_FEATURE_PPC_LE,
- .mmu_features = 0,
- .icache_bsize = 32,
- .dcache_bsize = 32,
- .cpu_setup = __setup_cpu_603,
- .machine_check = machine_check_generic,
- .platform = "ppc603",
- },
- { /* 603e */
- .pvr_mask = 0xffff0000,
- .pvr_value = 0x00060000,
- .cpu_name = "603e",
- .cpu_features = CPU_FTRS_603,
- .cpu_user_features = COMMON_USER | PPC_FEATURE_PPC_LE,
- .mmu_features = 0,
- .icache_bsize = 32,
- .dcache_bsize = 32,
- .cpu_setup = __setup_cpu_603,
- .machine_check = machine_check_generic,
- .platform = "ppc603",
- },
- { /* 603ev */
- .pvr_mask = 0xffff0000,
- .pvr_value = 0x00070000,
- .cpu_name = "603ev",
- .cpu_features = CPU_FTRS_603,
- .cpu_user_features = COMMON_USER | PPC_FEATURE_PPC_LE,
- .mmu_features = 0,
- .icache_bsize = 32,
- .dcache_bsize = 32,
- .cpu_setup = __setup_cpu_603,
- .machine_check = machine_check_generic,
- .platform = "ppc603",
- },
{ /* 604 */
.pvr_mask = 0xffff0000,
.pvr_value = 0x00040000,
@@ -1138,6 +1099,45 @@ static struct cpu_spec __initdata cpu_specs[] = {
.machine_check = machine_check_generic,
.platform = "ppc7450",
},
+ { /* 603 */
+ .pvr_mask = 0xffff0000,
+ .pvr_value = 0x00030000,
+ .cpu_name = "603",
+ .cpu_features = CPU_FTRS_603,
+ .cpu_user_features = COMMON_USER | PPC_FEATURE_PPC_LE,
+ .mmu_features = 0,
+ .icache_bsize = 32,
+ .dcache_bsize = 32,
+ .cpu_setup = __setup_cpu_603,
+ .machine_check = machine_check_generic,
+ .platform = "ppc603",
+ },
+ { /* 603e */
+ .pvr_mask = 0xffff0000,
+ .pvr_value = 0x00060000,
+ .cpu_name = "603e",
+ .cpu_features = CPU_FTRS_603,
+ .cpu_user_features = COMMON_USER | PPC_FEATURE_PPC_LE,
+ .mmu_features = 0,
+ .icache_bsize = 32,
+ .dcache_bsize = 32,
+ .cpu_setup = __setup_cpu_603,
+ .machine_check = machine_check_generic,
+ .platform = "ppc603",
+ },
+ { /* 603ev */
+ .pvr_mask = 0xffff0000,
+ .pvr_value = 0x00070000,
+ .cpu_name = "603ev",
+ .cpu_features = CPU_FTRS_603,
+ .cpu_user_features = COMMON_USER | PPC_FEATURE_PPC_LE,
+ .mmu_features = 0,
+ .icache_bsize = 32,
+ .dcache_bsize = 32,
+ .cpu_setup = __setup_cpu_603,
+ .machine_check = machine_check_generic,
+ .platform = "ppc603",
+ },
{ /* 82xx (8240, 8245, 8260 are all 603e cores) */
.pvr_mask = 0x7fff0000,
.pvr_value = 0x00810000,
--
2.25.0
^ permalink raw reply related
* [PATCH v1 19/20] powerpc/32s: Make support for 603 and 604+ selectable
From: Christophe Leroy @ 2020-10-22 6:29 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <648e2448e938d52d0b5887445e018ca584edc06d.1603348103.git.christophe.leroy@csgroup.eu>
book3s/32 has two main families:
- CPU with 603 cores that don't have HASH PTE table and
perform SW TLB loading.
- Other CPUs based on 604+ cores that have HASH PTE table.
This leads to some complex logic and additionnal code to
support both. This makes sense for distribution kernels
that aim at running on any CPU, but when you are fine
tuning a kernel for an embedded 603 based board you
don't need all the HASH logic.
Allow selection of support for each family, in order to opt
out unneeded parts of code. At least one must be selected.
Note that some of the CPU supporting HASH also support SW TLB
loading, however it is not supported by Linux kernel at the
time being, because they do not have alternate registers in
the TLB miss exception handlers.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/cputable.h | 8 ++++++--
arch/powerpc/include/asm/mmu.h | 5 ++++-
arch/powerpc/kernel/cputable.c | 6 ++++++
arch/powerpc/platforms/Kconfig.cputype | 16 ++++++++++++++++
4 files changed, 32 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
index 89f2d6b68cd7..72214635d79a 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -509,7 +509,7 @@ static inline void cpu_feature_keys_init(void) { }
#else
enum {
CPU_FTRS_POSSIBLE =
-#ifdef CONFIG_PPC_BOOK3S_32
+#ifdef CONFIG_PPC_BOOK3S_604
CPU_FTRS_604 | CPU_FTRS_740_NOTAU |
CPU_FTRS_740 | CPU_FTRS_750 | CPU_FTRS_750FX1 |
CPU_FTRS_750FX2 | CPU_FTRS_750FX | CPU_FTRS_750GX |
@@ -518,6 +518,8 @@ enum {
CPU_FTRS_7455_20 | CPU_FTRS_7455 | CPU_FTRS_7447_10 |
CPU_FTRS_7447 | CPU_FTRS_7447A |
CPU_FTRS_CLASSIC32 |
+#endif
+#ifdef CONFIG_PPC_BOOK3S_603
CPU_FTRS_603 | CPU_FTRS_82XX |
CPU_FTRS_G2_LE | CPU_FTRS_E300 | CPU_FTRS_E300C2 |
#endif
@@ -584,7 +586,7 @@ enum {
#else
enum {
CPU_FTRS_ALWAYS =
-#ifdef CONFIG_PPC_BOOK3S_32
+#ifdef CONFIG_PPC_BOOK3S_604
CPU_FTRS_604 & CPU_FTRS_740_NOTAU &
CPU_FTRS_740 & CPU_FTRS_750 & CPU_FTRS_750FX1 &
CPU_FTRS_750FX2 & CPU_FTRS_750FX & CPU_FTRS_750GX &
@@ -593,6 +595,8 @@ enum {
CPU_FTRS_7455_20 & CPU_FTRS_7455 & CPU_FTRS_7447_10 &
CPU_FTRS_7447 & CPU_FTRS_7447A &
CPU_FTRS_CLASSIC32 &
+#endif
+#ifdef CONFIG_PPC_BOOK3S_603
CPU_FTRS_603 & CPU_FTRS_82XX &
CPU_FTRS_G2_LE & CPU_FTRS_E300 & CPU_FTRS_E300C2 &
#endif
diff --git a/arch/powerpc/include/asm/mmu.h b/arch/powerpc/include/asm/mmu.h
index 7f57a9f7999f..85e050dd3673 100644
--- a/arch/powerpc/include/asm/mmu.h
+++ b/arch/powerpc/include/asm/mmu.h
@@ -154,7 +154,7 @@ DECLARE_PER_CPU(int, next_tlbcam_idx);
enum {
MMU_FTRS_POSSIBLE =
-#ifdef CONFIG_PPC_BOOK3S
+#if defined(CONFIG_PPC_BOOK3S_64) || defined(CONFIG_PPC_BOOK3S_604)
MMU_FTR_HPTE_TABLE |
#endif
#ifdef CONFIG_PPC_8xx
@@ -201,6 +201,9 @@ enum {
0,
};
+#if defined(CONFIG_PPC_BOOK3S_604) && !defined(CONFIG_PPC_BOOK3S_603)
+#define MMU_FTRS_ALWAYS MMU_FTR_HPTE_TABLE
+#endif
#ifdef CONFIG_PPC_8xx
#define MMU_FTRS_ALWAYS MMU_FTR_TYPE_8xx
#endif
diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputable.c
index ba96127d2e8c..84840575e639 100644
--- a/arch/powerpc/kernel/cputable.c
+++ b/arch/powerpc/kernel/cputable.c
@@ -610,6 +610,7 @@ static struct cpu_spec __initdata cpu_specs[] = {
#ifdef CONFIG_PPC32
#ifdef CONFIG_PPC_BOOK3S_32
+#ifdef CONFIG_PPC_BOOK3S_604
{ /* 604 */
.pvr_mask = 0xffff0000,
.pvr_value = 0x00040000,
@@ -1099,6 +1100,8 @@ static struct cpu_spec __initdata cpu_specs[] = {
.machine_check = machine_check_generic,
.platform = "ppc7450",
},
+#endif /* CONFIG_PPC_BOOK3S_604 */
+#ifdef CONFIG_PPC_BOOK3S_603
{ /* 603 */
.pvr_mask = 0xffff0000,
.pvr_value = 0x00030000,
@@ -1227,6 +1230,8 @@ static struct cpu_spec __initdata cpu_specs[] = {
.platform = "ppc603",
},
#endif
+#endif /* CONFIG_PPC_BOOK3S_603 */
+#ifdef CONFIG_PPC_BOOK3S_604
{ /* default match, we assume split I/D cache & TB (non-601)... */
.pvr_mask = 0x00000000,
.pvr_value = 0x00000000,
@@ -1239,6 +1244,7 @@ static struct cpu_spec __initdata cpu_specs[] = {
.machine_check = machine_check_generic,
.platform = "ppc603",
},
+#endif /* CONFIG_PPC_BOOK3S_604 */
#endif /* CONFIG_PPC_BOOK3S_32 */
#ifdef CONFIG_PPC_8xx
{ /* 8xx */
diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
index 818a41c9e274..12ad1dedc678 100644
--- a/arch/powerpc/platforms/Kconfig.cputype
+++ b/arch/powerpc/platforms/Kconfig.cputype
@@ -67,6 +67,22 @@ config E200
endchoice
+config PPC_BOOK3S_603
+ bool "Support for 603 SW loaded TLB"
+ depends on PPC_BOOK3S_32
+ default y
+ help
+ Provide support for processors based on the 603 cores. Those
+ processors don't have a HASH MMU and provide SW TLB loading.
+
+config PPC_BOOK3S_604
+ bool "Support for 604+ HASH MMU" if PPC_BOOK3S_603
+ depends on PPC_BOOK3S_32
+ default y
+ help
+ Provide support for processors not based on the 603 cores.
+ Those processors have a HASH MMU.
+
choice
prompt "Processor Type"
depends on PPC64
--
2.25.0
^ permalink raw reply related
* [PATCH v1 20/20] powerpc/32s: Only build hash code when CONFIG_PPC_BOOK3S_604 is selected
From: Christophe Leroy @ 2020-10-22 6:29 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <648e2448e938d52d0b5887445e018ca584edc06d.1603348103.git.christophe.leroy@csgroup.eu>
It is now possible to only build book3s/32 kernel for
CPUs without hash table.
Opt out hash related code when CONFIG_PPC_BOOK3S_604 is not selected.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/kernel/head_book3s_32.S | 12 ++++++++++++
arch/powerpc/mm/book3s32/Makefile | 4 +++-
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/head_book3s_32.S b/arch/powerpc/kernel/head_book3s_32.S
index 620af1dda70c..4fc4091aaade 100644
--- a/arch/powerpc/kernel/head_book3s_32.S
+++ b/arch/powerpc/kernel/head_book3s_32.S
@@ -287,6 +287,7 @@ DataAccess:
#ifdef CONFIG_VMAP_STACK
mtspr SPRN_SPRG_SCRATCH0,r10
mfspr r10, SPRN_SPRG_THREAD
+#ifdef CONFIG_PPC_BOOK3S_604
BEGIN_MMU_FTR_SECTION
stw r11, THR11(r10)
mfspr r10, SPRN_DSISR
@@ -302,6 +303,7 @@ BEGIN_MMU_FTR_SECTION
mtcr r11
lwz r11, THR11(r10)
END_MMU_FTR_SECTION_IFSET(MMU_FTR_HPTE_TABLE)
+#endif
mtspr SPRN_SPRG_SCRATCH1,r11
mfspr r11, SPRN_DAR
stw r11, DAR(r10)
@@ -319,6 +321,7 @@ END_MMU_FTR_SECTION_IFSET(MMU_FTR_HPTE_TABLE)
#else /* CONFIG_VMAP_STACK */
EXCEPTION_PROLOG handle_dar_dsisr=1
get_and_save_dar_dsisr_on_stack r4, r5, r11
+#ifdef CONFIG_PPC_BOOK3S_604
BEGIN_MMU_FTR_SECTION
#ifdef CONFIG_PPC_KUAP
andis. r0, r5, (DSISR_BAD_FAULT_32S | DSISR_DABRMATCH | DSISR_PROTFAULT)@h
@@ -330,8 +333,11 @@ BEGIN_MMU_FTR_SECTION
bl hash_page
b handle_page_fault_tramp_1
FTR_SECTION_ELSE
+#endif
b handle_page_fault_tramp_2
+#ifdef CONFIG_PPC_BOOK3S_604
ALT_MMU_FTR_SECTION_END_IFSET(MMU_FTR_HPTE_TABLE)
+#endif
#endif /* CONFIG_VMAP_STACK */
/* Instruction access exception. */
@@ -347,12 +353,14 @@ InstructionAccess:
mfspr r11, SPRN_SRR1 /* check whether user or kernel */
stw r11, SRR1(r10)
mfcr r10
+#ifdef CONFIG_PPC_BOOK3S_604
BEGIN_MMU_FTR_SECTION
andis. r11, r11, SRR1_ISI_NOPT@h /* no pte found? */
bne hash_page_isi
.Lhash_page_isi_cont:
mfspr r11, SPRN_SRR1 /* check whether user or kernel */
END_MMU_FTR_SECTION_IFSET(MMU_FTR_HPTE_TABLE)
+#endif
andi. r11, r11, MSR_PR
EXCEPTION_PROLOG_1
@@ -363,9 +371,11 @@ END_MMU_FTR_SECTION_IFSET(MMU_FTR_HPTE_TABLE)
beq 1f /* if so, try to put a PTE */
li r3,0 /* into the hash table */
mr r4,r12 /* SRR0 is fault address */
+#ifdef CONFIG_PPC_BOOK3S_604
BEGIN_MMU_FTR_SECTION
bl hash_page
END_MMU_FTR_SECTION_IFSET(MMU_FTR_HPTE_TABLE)
+#endif
#endif /* CONFIG_VMAP_STACK */
1: mr r4,r12
andis. r5,r9,DSISR_SRR1_MATCH_32S@h /* Filter relevant SRR1 bits */
@@ -703,6 +713,7 @@ handle_page_fault_tramp_2:
EXC_XFER_LITE(0x300, handle_page_fault)
#ifdef CONFIG_VMAP_STACK
+#ifdef CONFIG_PPC_BOOK3S_604
.macro save_regs_thread thread
stw r0, THR0(\thread)
stw r3, THR3(\thread)
@@ -774,6 +785,7 @@ fast_hash_page_return:
mfspr r11, SPRN_SPRG_SCRATCH1
mfspr r10, SPRN_SPRG_SCRATCH0
RFI
+#endif /* CONFIG_PPC_BOOK3S_604 */
stack_overflow:
vmap_stack_overflow_exception
diff --git a/arch/powerpc/mm/book3s32/Makefile b/arch/powerpc/mm/book3s32/Makefile
index 3f972db17761..446d9de88ce4 100644
--- a/arch/powerpc/mm/book3s32/Makefile
+++ b/arch/powerpc/mm/book3s32/Makefile
@@ -6,4 +6,6 @@ ifdef CONFIG_KASAN
CFLAGS_mmu.o += -DDISABLE_BRANCH_PROFILING
endif
-obj-y += mmu.o hash_low.o mmu_context.o tlb.o nohash_low.o
+obj-y += mmu.o mmu_context.o
+obj-$(CONFIG_PPC_BOOK3S_603) += nohash_low.o
+obj-$(CONFIG_PPC_BOOK3S_604) += hash_low.o tlb.o
--
2.25.0
^ permalink raw reply related
* [PATCH] powerpc/time: enable sched clock for irqtime
From: Pingfan Liu @ 2020-10-22 6:51 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Christophe Leroy, Nicholas Piggin, Pingfan Liu
When CONFIG_IRQ_TIME_ACCOUNTING and CONFIG_VIRT_CPU_ACCOUNTING_GEN, powerpc
does not enable "sched_clock_irqtime" and can not utilize irq time
accounting.
Like x86, powerpc does not use the sched_clock_register() interface. So it
needs an dedicated call to enable_sched_clock_irqtime() to enable irq time
accounting.
Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Christophe Leroy <christophe.leroy@c-s.fr>
Cc: Nicholas Piggin <npiggin@gmail.com>
To: linuxppc-dev@lists.ozlabs.org
---
arch/powerpc/kernel/time.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index f85539e..4083b59e 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -53,6 +53,7 @@
#include <linux/of_clk.h>
#include <linux/suspend.h>
#include <linux/sched/cputime.h>
+#include <linux/sched/clock.h>
#include <linux/processor.h>
#include <asm/trace.h>
@@ -1134,6 +1135,7 @@ void __init time_init(void)
tick_setup_hrtimer_broadcast();
of_clk_init(NULL);
+ enable_sched_clock_irqtime();
}
/*
--
2.7.5
^ permalink raw reply related
* Re: [PATCH] powerpc/time: enable sched clock for irqtime
From: Pingfan Liu @ 2020-10-22 6:58 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Christophe Leroy, Nicholas Piggin
In-Reply-To: <1603349479-26185-1-git-send-email-kernelfans@gmail.com>
I encounter a irq flood on Power9 machine, and tries a way to work
around it by https://www.spinics.net/lists/kernel/msg3705028.html
As irq time accounting is the foundation for the method, it needs to
make irq accounting take effect on powerpc platform.
On Thu, Oct 22, 2020 at 2:51 PM Pingfan Liu <kernelfans@gmail.com> wrote:
>
> When CONFIG_IRQ_TIME_ACCOUNTING and CONFIG_VIRT_CPU_ACCOUNTING_GEN, powerpc
> does not enable "sched_clock_irqtime" and can not utilize irq time
> accounting.
>
> Like x86, powerpc does not use the sched_clock_register() interface. So it
> needs an dedicated call to enable_sched_clock_irqtime() to enable irq time
> accounting.
>
> Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Christophe Leroy <christophe.leroy@c-s.fr>
> Cc: Nicholas Piggin <npiggin@gmail.com>
> To: linuxppc-dev@lists.ozlabs.org
> ---
> arch/powerpc/kernel/time.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
> index f85539e..4083b59e 100644
> --- a/arch/powerpc/kernel/time.c
> +++ b/arch/powerpc/kernel/time.c
> @@ -53,6 +53,7 @@
> #include <linux/of_clk.h>
> #include <linux/suspend.h>
> #include <linux/sched/cputime.h>
> +#include <linux/sched/clock.h>
> #include <linux/processor.h>
> #include <asm/trace.h>
>
> @@ -1134,6 +1135,7 @@ void __init time_init(void)
> tick_setup_hrtimer_broadcast();
>
> of_clk_init(NULL);
> + enable_sched_clock_irqtime();
> }
>
> /*
> --
> 2.7.5
>
^ permalink raw reply
* Re: [PATCH] serial: pmac_zilog: don't init if zilog is not available
From: Laurent Vivier @ 2020-10-22 7:16 UTC (permalink / raw)
To: Finn Thain
Cc: Greg KH, linux-kernel, linux-m68k, Geert Uytterhoeven,
linux-serial, Brad Boyer, Paul Mackerras, linuxppc-dev,
Joshua Thompson
In-Reply-To: <alpine.LNX.2.23.453.2010221347000.6@nippy.intranet>
Le 22/10/2020 à 05:23, Finn Thain a écrit :
> On Wed, 21 Oct 2020, Laurent Vivier wrote:
>
>> Le 21/10/2020 à 01:43, Finn Thain a écrit :
>>
>>> Laurent, can we avoid the irq == 0 warning splat like this?
>>>
>>> diff --git a/drivers/tty/serial/pmac_zilog.c b/drivers/tty/serial/pmac_zilog.c
>>> index 96e7aa479961..7db600cd8cc7 100644
>>> --- a/drivers/tty/serial/pmac_zilog.c
>>> +++ b/drivers/tty/serial/pmac_zilog.c
>>> @@ -1701,8 +1701,10 @@ static int __init pmz_init_port(struct uart_pmac_port *uap)
>>> int irq;
>>>
>>> r_ports = platform_get_resource(uap->pdev, IORESOURCE_MEM, 0);
>>> + if (!r_ports)
>>> + return -ENODEV;
>>> irq = platform_get_irq(uap->pdev, 0);
>>> - if (!r_ports || irq <= 0)
>>> + if (irq <= 0)
>>> return -ENODEV;
>>>
>>> uap->port.mapbase = r_ports->start;
>>>
>>
>> No, this doesn't fix the problem.
>>
>
> Then I had better stop guessing and start up Aranym...
>
> The patch below seems to fix the problem for me. Does it work on your
> system(s)?
It works like a charm.
Thank you,
Laurent
^ permalink raw reply
* Re: [PATCH] serial: pmac_zilog: don't init if zilog is not available
From: Geert Uytterhoeven @ 2020-10-22 7:26 UTC (permalink / raw)
To: Finn Thain
Cc: Greg KH, Laurent Vivier, Linux Kernel Mailing List, linux-m68k,
Paul Mackerras, open list:SERIAL DRIVERS, Brad Boyer,
linuxppc-dev, Joshua Thompson
In-Reply-To: <alpine.LNX.2.23.453.2010221347000.6@nippy.intranet>
Hi Finn,
On Thu, Oct 22, 2020 at 5:23 AM Finn Thain <fthain@telegraphics.com.au> wrote:
> The patch below seems to fix the problem for me. Does it work on your
> system(s)?
Thanks for your patch!
> --- a/arch/m68k/mac/config.c
> +++ b/arch/m68k/mac/config.c
> @@ -776,16 +776,12 @@ static struct resource scc_b_rsrcs[] = {
> struct platform_device scc_a_pdev = {
> .name = "scc",
> .id = 0,
> - .num_resources = ARRAY_SIZE(scc_a_rsrcs),
> - .resource = scc_a_rsrcs,
> };
> EXPORT_SYMBOL(scc_a_pdev);
>
> struct platform_device scc_b_pdev = {
> .name = "scc",
> .id = 1,
> - .num_resources = ARRAY_SIZE(scc_b_rsrcs),
> - .resource = scc_b_rsrcs,
> };
> EXPORT_SYMBOL(scc_b_pdev);
>
> @@ -812,10 +808,15 @@ static void __init mac_identify(void)
>
> /* Set up serial port resources for the console initcall. */
>
> - scc_a_rsrcs[0].start = (resource_size_t) mac_bi_data.sccbase + 2;
> - scc_a_rsrcs[0].end = scc_a_rsrcs[0].start;
> - scc_b_rsrcs[0].start = (resource_size_t) mac_bi_data.sccbase;
> - scc_b_rsrcs[0].end = scc_b_rsrcs[0].start;
> + scc_a_rsrcs[0].start = (resource_size_t)mac_bi_data.sccbase + 2;
> + scc_a_rsrcs[0].end = scc_a_rsrcs[0].start;
> + scc_a_pdev.num_resources = ARRAY_SIZE(scc_a_rsrcs);
> + scc_a_pdev.resource = scc_a_rsrcs;
> +
> + scc_b_rsrcs[0].start = (resource_size_t)mac_bi_data.sccbase;
> + scc_b_rsrcs[0].end = scc_b_rsrcs[0].start;
> + scc_b_pdev.num_resources = ARRAY_SIZE(scc_b_rsrcs);
> + scc_b_pdev.resource = scc_b_rsrcs;
I can't say I'm a fan of this...
>
> switch (macintosh_config->scc_type) {
> case MAC_SCC_PSC:
> diff --git a/drivers/tty/serial/pmac_zilog.c b/drivers/tty/serial/pmac_zilog.c
> index 96e7aa479961..95abdb305d67 100644
> --- a/drivers/tty/serial/pmac_zilog.c
> +++ b/drivers/tty/serial/pmac_zilog.c
> @@ -1697,18 +1697,17 @@ extern struct platform_device scc_a_pdev, scc_b_pdev;
The real issue is this "extern struct platform_device scc_a_pdev, scc_b_pdev",
circumventing the driver framework.
Can we get rid of that?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
From: Greg KH @ 2020-10-22 8:26 UTC (permalink / raw)
To: Al Viro, Nick Desaulniers
Cc: linux-aio, linux-mips, David Howells, linux-mm, keyrings,
sparclinux, Christoph Hellwig, linux-arch, linux-s390, linux-scsi,
kernel-team, Arnd Bergmann, linux-block, io-uring,
linux-arm-kernel, Jens Axboe, linux-parisc, netdev, linux-kernel,
linux-security-module, David Laight, linux-fsdevel, Andrew Morton,
linuxppc-dev
In-Reply-To: <20201021233914.GR3576660@ZenIV.linux.org.uk>
On Thu, Oct 22, 2020 at 12:39:14AM +0100, Al Viro wrote:
> On Wed, Oct 21, 2020 at 06:13:01PM +0200, Greg KH wrote:
> > On Fri, Sep 25, 2020 at 06:51:39AM +0200, Christoph Hellwig wrote:
> > > From: David Laight <David.Laight@ACULAB.COM>
> > >
> > > This lets the compiler inline it into import_iovec() generating
> > > much better code.
> > >
> > > Signed-off-by: David Laight <david.laight@aculab.com>
> > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > > ---
> > > fs/read_write.c | 179 ------------------------------------------------
> > > lib/iov_iter.c | 176 +++++++++++++++++++++++++++++++++++++++++++++++
> > > 2 files changed, 176 insertions(+), 179 deletions(-)
> >
> > Strangely, this commit causes a regression in Linus's tree right now.
> >
> > I can't really figure out what the regression is, only that this commit
> > triggers a "large Android system binary" from working properly. There's
> > no kernel log messages anywhere, and I don't have any way to strace the
> > thing in the testing framework, so any hints that people can provide
> > would be most appreciated.
>
> It's a pure move - modulo changed line breaks in the argument lists
> the functions involved are identical before and after that (just checked
> that directly, by checking out the trees before and after, extracting two
> functions in question from fs/read_write.c and lib/iov_iter.c (before and
> after, resp.) and checking the diff between those.
>
> How certain is your bisection?
The bisection is very reproducable.
But, this looks now to be a compiler bug. I'm using the latest version
of clang and if I put "noinline" at the front of the function,
everything works.
Nick, any ideas here as to who I should report this to?
I'll work on a fixup patch for the Android kernel tree to see if I can
work around it there, but others will hit this in Linus's tree sooner or
later...
thanks,
greg k-h
^ permalink raw reply
* Re: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
From: David Hildenbrand @ 2020-10-22 8:35 UTC (permalink / raw)
To: Greg KH, Al Viro, Nick Desaulniers
Cc: linux-aio, linux-mips, David Howells, linux-mm, keyrings,
sparclinux, Christoph Hellwig, linux-arch, linux-s390, linux-scsi,
kernel-team, Arnd Bergmann, linux-block, io-uring,
linux-arm-kernel, Jens Axboe, linux-parisc, netdev, linux-kernel,
linux-security-module, David Laight, linux-fsdevel, Andrew Morton,
linuxppc-dev
In-Reply-To: <20201022082654.GA1477657@kroah.com>
On 22.10.20 10:26, Greg KH wrote:
> On Thu, Oct 22, 2020 at 12:39:14AM +0100, Al Viro wrote:
>> On Wed, Oct 21, 2020 at 06:13:01PM +0200, Greg KH wrote:
>>> On Fri, Sep 25, 2020 at 06:51:39AM +0200, Christoph Hellwig wrote:
>>>> From: David Laight <David.Laight@ACULAB.COM>
>>>>
>>>> This lets the compiler inline it into import_iovec() generating
>>>> much better code.
>>>>
>>>> Signed-off-by: David Laight <david.laight@aculab.com>
>>>> Signed-off-by: Christoph Hellwig <hch@lst.de>
>>>> ---
>>>> fs/read_write.c | 179 ------------------------------------------------
>>>> lib/iov_iter.c | 176 +++++++++++++++++++++++++++++++++++++++++++++++
>>>> 2 files changed, 176 insertions(+), 179 deletions(-)
>>>
>>> Strangely, this commit causes a regression in Linus's tree right now.
>>>
>>> I can't really figure out what the regression is, only that this commit
>>> triggers a "large Android system binary" from working properly. There's
>>> no kernel log messages anywhere, and I don't have any way to strace the
>>> thing in the testing framework, so any hints that people can provide
>>> would be most appreciated.
>>
>> It's a pure move - modulo changed line breaks in the argument lists
>> the functions involved are identical before and after that (just checked
>> that directly, by checking out the trees before and after, extracting two
>> functions in question from fs/read_write.c and lib/iov_iter.c (before and
>> after, resp.) and checking the diff between those.
>>
>> How certain is your bisection?
>
> The bisection is very reproducable.
>
> But, this looks now to be a compiler bug. I'm using the latest version
> of clang and if I put "noinline" at the front of the function,
> everything works.
Well, the compiler can do more invasive optimizations when inlining. If
you have buggy code that relies on some unspecified behavior, inlining
can change the behavior ... but going over that code, there isn't too
much action going on. At least nothing screamed at me.
--
Thanks,
David / dhildenb
^ permalink raw reply
* [PATCH] selftests/powerpc/eeh: disable kselftest timeout setting for eeh-basic
From: Po-Hsu Lin @ 2020-10-22 8:36 UTC (permalink / raw)
To: linux-kernel, linuxppc-dev, linux-kselftest
Cc: joe.lawrence, po-hsu.lin, mathieu.desnoyers, mbenes, shuah
The eeh-basic test got its own 60 seconds timeout (defined in commit
414f50434aa2 "selftests/eeh: Bump EEH wait time to 60s") per breakable
device.
And we have discovered that the number of breakable devices varies
on different hardware. The device recovery time ranges from 0 to 35
seconds. In our test pool it will take about 30 seconds to run on a
Power8 system that with 5 breakable devices, 60 seconds to run on a
Power9 system that with 4 breakable devices.
Thus it's better to disable the default 45 seconds timeout setting in
the kselftest framework to give it a chance to finish. And let the
test to take care of the timeout control.
Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
---
tools/testing/selftests/powerpc/eeh/Makefile | 2 +-
tools/testing/selftests/powerpc/eeh/settings | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/powerpc/eeh/settings
diff --git a/tools/testing/selftests/powerpc/eeh/Makefile b/tools/testing/selftests/powerpc/eeh/Makefile
index b397bab..ae963eb 100644
--- a/tools/testing/selftests/powerpc/eeh/Makefile
+++ b/tools/testing/selftests/powerpc/eeh/Makefile
@@ -3,7 +3,7 @@ noarg:
$(MAKE) -C ../
TEST_PROGS := eeh-basic.sh
-TEST_FILES := eeh-functions.sh
+TEST_FILES := eeh-functions.sh settings
top_srcdir = ../../../../..
include ../../lib.mk
diff --git a/tools/testing/selftests/powerpc/eeh/settings b/tools/testing/selftests/powerpc/eeh/settings
new file mode 100644
index 0000000..e7b9417
--- /dev/null
+++ b/tools/testing/selftests/powerpc/eeh/settings
@@ -0,0 +1 @@
+timeout=0
--
2.7.4
^ permalink raw reply related
* RE: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
From: David Laight @ 2020-10-22 8:40 UTC (permalink / raw)
To: 'David Hildenbrand', Greg KH, Al Viro, Nick Desaulniers
Cc: linux-aio@kvack.org, linux-mips@vger.kernel.org, David Howells,
linux-mm@kvack.org, keyrings@vger.kernel.org,
sparclinux@vger.kernel.org, Christoph Hellwig,
linux-arch@vger.kernel.org, linux-s390@vger.kernel.org,
linux-scsi@vger.kernel.org, kernel-team@android.com,
Arnd Bergmann, linux-block@vger.kernel.org,
io-uring@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
Jens Axboe, linux-parisc@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-security-module@vger.kernel.org,
linux-fsdevel@vger.kernel.org, Andrew Morton,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <80a2e5fa-718a-8433-1ab0-dd5b3e3b5416@redhat.com>
From: David Hildenbrand
> Sent: 22 October 2020 09:35
>
> On 22.10.20 10:26, Greg KH wrote:
> > On Thu, Oct 22, 2020 at 12:39:14AM +0100, Al Viro wrote:
> >> On Wed, Oct 21, 2020 at 06:13:01PM +0200, Greg KH wrote:
> >>> On Fri, Sep 25, 2020 at 06:51:39AM +0200, Christoph Hellwig wrote:
> >>>> From: David Laight <David.Laight@ACULAB.COM>
> >>>>
> >>>> This lets the compiler inline it into import_iovec() generating
> >>>> much better code.
> >>>>
> >>>> Signed-off-by: David Laight <david.laight@aculab.com>
> >>>> Signed-off-by: Christoph Hellwig <hch@lst.de>
> >>>> ---
> >>>> fs/read_write.c | 179 ------------------------------------------------
> >>>> lib/iov_iter.c | 176 +++++++++++++++++++++++++++++++++++++++++++++++
> >>>> 2 files changed, 176 insertions(+), 179 deletions(-)
> >>>
> >>> Strangely, this commit causes a regression in Linus's tree right now.
> >>>
> >>> I can't really figure out what the regression is, only that this commit
> >>> triggers a "large Android system binary" from working properly. There's
> >>> no kernel log messages anywhere, and I don't have any way to strace the
> >>> thing in the testing framework, so any hints that people can provide
> >>> would be most appreciated.
> >>
> >> It's a pure move - modulo changed line breaks in the argument lists
> >> the functions involved are identical before and after that (just checked
> >> that directly, by checking out the trees before and after, extracting two
> >> functions in question from fs/read_write.c and lib/iov_iter.c (before and
> >> after, resp.) and checking the diff between those.
> >>
> >> How certain is your bisection?
> >
> > The bisection is very reproducable.
> >
> > But, this looks now to be a compiler bug. I'm using the latest version
> > of clang and if I put "noinline" at the front of the function,
> > everything works.
>
> Well, the compiler can do more invasive optimizations when inlining. If
> you have buggy code that relies on some unspecified behavior, inlining
> can change the behavior ... but going over that code, there isn't too
> much action going on. At least nothing screamed at me.
Apart from all the optimisations that get rid off the 'pass be reference'
parameters and strange conditional tests.
Plenty of scope for the compiler getting it wrong.
But nothing even vaguely illegal.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* Re: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
From: David Hildenbrand @ 2020-10-22 8:48 UTC (permalink / raw)
To: David Laight, Greg KH, Al Viro, Nick Desaulniers
Cc: linux-aio@kvack.org, linux-mips@vger.kernel.org, David Howells,
linux-mm@kvack.org, keyrings@vger.kernel.org,
sparclinux@vger.kernel.org, Christoph Hellwig,
linux-arch@vger.kernel.org, linux-s390@vger.kernel.org,
linux-scsi@vger.kernel.org, kernel-team@android.com,
Arnd Bergmann, linux-block@vger.kernel.org,
io-uring@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
Jens Axboe, linux-parisc@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-security-module@vger.kernel.org,
linux-fsdevel@vger.kernel.org, Andrew Morton,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <5d2ecb24db1e415b8ff88261435386ec@AcuMS.aculab.com>
On 22.10.20 10:40, David Laight wrote:
> From: David Hildenbrand
>> Sent: 22 October 2020 09:35
>>
>> On 22.10.20 10:26, Greg KH wrote:
>>> On Thu, Oct 22, 2020 at 12:39:14AM +0100, Al Viro wrote:
>>>> On Wed, Oct 21, 2020 at 06:13:01PM +0200, Greg KH wrote:
>>>>> On Fri, Sep 25, 2020 at 06:51:39AM +0200, Christoph Hellwig wrote:
>>>>>> From: David Laight <David.Laight@ACULAB.COM>
>>>>>>
>>>>>> This lets the compiler inline it into import_iovec() generating
>>>>>> much better code.
>>>>>>
>>>>>> Signed-off-by: David Laight <david.laight@aculab.com>
>>>>>> Signed-off-by: Christoph Hellwig <hch@lst.de>
>>>>>> ---
>>>>>> fs/read_write.c | 179 ------------------------------------------------
>>>>>> lib/iov_iter.c | 176 +++++++++++++++++++++++++++++++++++++++++++++++
>>>>>> 2 files changed, 176 insertions(+), 179 deletions(-)
>>>>>
>>>>> Strangely, this commit causes a regression in Linus's tree right now.
>>>>>
>>>>> I can't really figure out what the regression is, only that this commit
>>>>> triggers a "large Android system binary" from working properly. There's
>>>>> no kernel log messages anywhere, and I don't have any way to strace the
>>>>> thing in the testing framework, so any hints that people can provide
>>>>> would be most appreciated.
>>>>
>>>> It's a pure move - modulo changed line breaks in the argument lists
>>>> the functions involved are identical before and after that (just checked
>>>> that directly, by checking out the trees before and after, extracting two
>>>> functions in question from fs/read_write.c and lib/iov_iter.c (before and
>>>> after, resp.) and checking the diff between those.
>>>>
>>>> How certain is your bisection?
>>>
>>> The bisection is very reproducable.
>>>
>>> But, this looks now to be a compiler bug. I'm using the latest version
>>> of clang and if I put "noinline" at the front of the function,
>>> everything works.
>>
>> Well, the compiler can do more invasive optimizations when inlining. If
>> you have buggy code that relies on some unspecified behavior, inlining
>> can change the behavior ... but going over that code, there isn't too
>> much action going on. At least nothing screamed at me.
>
> Apart from all the optimisations that get rid off the 'pass be reference'
> parameters and strange conditional tests.
> Plenty of scope for the compiler getting it wrong.
> But nothing even vaguely illegal.
Not the first time that people blame the compiler to then figure out
that something else is wrong ... but maybe this time is different :)
--
Thanks,
David / dhildenb
^ permalink raw reply
* Re: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
From: Greg KH @ 2020-10-22 9:01 UTC (permalink / raw)
To: David Hildenbrand
Cc: linux-aio@kvack.org, linux-mips@vger.kernel.org, David Howells,
linux-mm@kvack.org, keyrings@vger.kernel.org,
sparclinux@vger.kernel.org, Christoph Hellwig,
linux-arch@vger.kernel.org, linux-s390@vger.kernel.org,
linux-scsi@vger.kernel.org, kernel-team@android.com,
Arnd Bergmann, linux-block@vger.kernel.org, Al Viro,
io-uring@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
Jens Axboe, linux-parisc@vger.kernel.org, netdev@vger.kernel.org,
Nick Desaulniers, linux-kernel@vger.kernel.org,
linux-security-module@vger.kernel.org, David Laight,
linux-fsdevel@vger.kernel.org, Andrew Morton,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <df2e0758-b8ed-5aec-6adc-a18f499c0179@redhat.com>
On Thu, Oct 22, 2020 at 10:48:59AM +0200, David Hildenbrand wrote:
> On 22.10.20 10:40, David Laight wrote:
> > From: David Hildenbrand
> >> Sent: 22 October 2020 09:35
> >>
> >> On 22.10.20 10:26, Greg KH wrote:
> >>> On Thu, Oct 22, 2020 at 12:39:14AM +0100, Al Viro wrote:
> >>>> On Wed, Oct 21, 2020 at 06:13:01PM +0200, Greg KH wrote:
> >>>>> On Fri, Sep 25, 2020 at 06:51:39AM +0200, Christoph Hellwig wrote:
> >>>>>> From: David Laight <David.Laight@ACULAB.COM>
> >>>>>>
> >>>>>> This lets the compiler inline it into import_iovec() generating
> >>>>>> much better code.
> >>>>>>
> >>>>>> Signed-off-by: David Laight <david.laight@aculab.com>
> >>>>>> Signed-off-by: Christoph Hellwig <hch@lst.de>
> >>>>>> ---
> >>>>>> fs/read_write.c | 179 ------------------------------------------------
> >>>>>> lib/iov_iter.c | 176 +++++++++++++++++++++++++++++++++++++++++++++++
> >>>>>> 2 files changed, 176 insertions(+), 179 deletions(-)
> >>>>>
> >>>>> Strangely, this commit causes a regression in Linus's tree right now.
> >>>>>
> >>>>> I can't really figure out what the regression is, only that this commit
> >>>>> triggers a "large Android system binary" from working properly. There's
> >>>>> no kernel log messages anywhere, and I don't have any way to strace the
> >>>>> thing in the testing framework, so any hints that people can provide
> >>>>> would be most appreciated.
> >>>>
> >>>> It's a pure move - modulo changed line breaks in the argument lists
> >>>> the functions involved are identical before and after that (just checked
> >>>> that directly, by checking out the trees before and after, extracting two
> >>>> functions in question from fs/read_write.c and lib/iov_iter.c (before and
> >>>> after, resp.) and checking the diff between those.
> >>>>
> >>>> How certain is your bisection?
> >>>
> >>> The bisection is very reproducable.
> >>>
> >>> But, this looks now to be a compiler bug. I'm using the latest version
> >>> of clang and if I put "noinline" at the front of the function,
> >>> everything works.
> >>
> >> Well, the compiler can do more invasive optimizations when inlining. If
> >> you have buggy code that relies on some unspecified behavior, inlining
> >> can change the behavior ... but going over that code, there isn't too
> >> much action going on. At least nothing screamed at me.
> >
> > Apart from all the optimisations that get rid off the 'pass be reference'
> > parameters and strange conditional tests.
> > Plenty of scope for the compiler getting it wrong.
> > But nothing even vaguely illegal.
>
> Not the first time that people blame the compiler to then figure out
> that something else is wrong ... but maybe this time is different :)
I agree, I hate to blame the compiler, that's almost never the real
problem, but this one sure "feels" like it.
I'm running some more tests, trying to narrow things down as just adding
a "noinline" to the function that got moved here doesn't work on Linus's
tree at the moment because the function was split into multiple
functions.
Give me a few hours...
thanks,
greg k-h
^ permalink raw reply
* RE: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
From: David Laight @ 2020-10-22 9:02 UTC (permalink / raw)
To: 'David Hildenbrand', Greg KH, Al Viro, Nick Desaulniers
Cc: linux-aio@kvack.org, linux-mips@vger.kernel.org, David Howells,
linux-mm@kvack.org, keyrings@vger.kernel.org,
sparclinux@vger.kernel.org, Christoph Hellwig,
linux-arch@vger.kernel.org, linux-s390@vger.kernel.org,
linux-scsi@vger.kernel.org, kernel-team@android.com,
Arnd Bergmann, linux-block@vger.kernel.org,
io-uring@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
Jens Axboe, linux-parisc@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-security-module@vger.kernel.org,
linux-fsdevel@vger.kernel.org, Andrew Morton,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <df2e0758-b8ed-5aec-6adc-a18f499c0179@redhat.com>
From: David Hildenbrand
> Sent: 22 October 2020 09:49
...
> >>> But, this looks now to be a compiler bug. I'm using the latest version
> >>> of clang and if I put "noinline" at the front of the function,
> >>> everything works.
> >>
> >> Well, the compiler can do more invasive optimizations when inlining. If
> >> you have buggy code that relies on some unspecified behavior, inlining
> >> can change the behavior ... but going over that code, there isn't too
> >> much action going on. At least nothing screamed at me.
> >
> > Apart from all the optimisations that get rid off the 'pass be reference'
> > parameters and strange conditional tests.
> > Plenty of scope for the compiler getting it wrong.
> > But nothing even vaguely illegal.
>
> Not the first time that people blame the compiler to then figure out
> that something else is wrong ... but maybe this time is different :)
Usually down to missing asm 'memory' constraints...
Need to read the obj file to see what the compiler did.
The code must be 'approximately right' or nothing would run.
So I'd guess it has to do with > 8 fragments.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* RE: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
From: David Laight @ 2020-10-22 9:06 UTC (permalink / raw)
To: 'Greg KH', David Hildenbrand
Cc: linux-aio@kvack.org, linux-mips@vger.kernel.org, David Howells,
linux-mm@kvack.org, keyrings@vger.kernel.org,
sparclinux@vger.kernel.org, Christoph Hellwig,
linux-arch@vger.kernel.org, linux-s390@vger.kernel.org,
linux-scsi@vger.kernel.org, kernel-team@android.com,
Arnd Bergmann, linux-block@vger.kernel.org, Al Viro,
io-uring@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
Jens Axboe, linux-parisc@vger.kernel.org, netdev@vger.kernel.org,
Nick Desaulniers, linux-kernel@vger.kernel.org,
linux-security-module@vger.kernel.org,
linux-fsdevel@vger.kernel.org, Andrew Morton,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20201022090155.GA1483166@kroah.com>
From: Greg KH
> Sent: 22 October 2020 10:02
...
> I'm running some more tests, trying to narrow things down as just adding
> a "noinline" to the function that got moved here doesn't work on Linus's
> tree at the moment because the function was split into multiple
> functions.
I was going to look at that once rc2 is in - and the kernel is
likely to work.
I suspect the split version doesn't get inlined the same way?
Which leaves the horrid argument conversion the inline got
rid of back there again.
Which all rather begs the question of why the compiler doesn't
generate the expected code.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* [PATCH] powerpc/mm: move setting pte specific flags to pfn_pmd
From: Aneesh Kumar K.V @ 2020-10-22 9:11 UTC (permalink / raw)
To: linuxppc-dev, mpe; +Cc: linux-mm, Aneesh Kumar K.V
powerpc used to set the pte specific flags in set_pte_at(). This is
different from other architectures. To be consistent with other
architecture powerpc updated pfn_pte to set _PAGE_PTE with
commit 379c926d6334 ("powerpc/mm: move setting pte specific flags to pfn_pte")
The commit didn't do the same w.r.t pfn_pmd because we expect pmd_mkhuge
to do that. But as per Linus that is a bad rule [1].
Hence update pfn_pmd to set _PAGE_PTE.
[1]
" The rule that you must use "pmd_mkhuge()" seems _completely_ wrong.
It's insane. The only valid use to ever make a pmd out of a pfn is to
make a huge-page."
message-id: CAHk-=whG+Z2mBFTT026PZAdjn=gSsLk9bk0wnYJ5peyuVGfwyg@mail.gmail.com
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
arch/powerpc/include/asm/book3s/64/pgtable.h | 17 ++++++++++++++++-
arch/powerpc/mm/book3s64/pgtable.c | 8 +++++++-
2 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
index cd3feeac6e87..a39886681629 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -1231,13 +1231,28 @@ static inline int pmd_same(pmd_t pmd_a, pmd_t pmd_b)
return hash__pmd_same(pmd_a, pmd_b);
}
-static inline pmd_t pmd_mkhuge(pmd_t pmd)
+static inline pmd_t __pmd_mkhuge(pmd_t pmd)
{
if (radix_enabled())
return radix__pmd_mkhuge(pmd);
return hash__pmd_mkhuge(pmd);
}
+/*
+ * pfn_pmd return a pmd_t that can be used as pmd pte entry.
+ */
+static inline pmd_t pmd_mkhuge(pmd_t pmd)
+{
+#ifdef CONFIG_DEBUG_VM
+ if (radix_enabled())
+ WARN_ON((pmd_raw(pmd) & cpu_to_be64(_PAGE_PTE)) == 0);
+ else
+ WARN_ON((pmd_raw(pmd) & cpu_to_be64(_PAGE_PTE | H_PAGE_THP_HUGE)) !=
+ cpu_to_be64(_PAGE_PTE | H_PAGE_THP_HUGE));
+#endif
+ return pmd;
+}
+
#define __HAVE_ARCH_PMDP_SET_ACCESS_FLAGS
extern int pmdp_set_access_flags(struct vm_area_struct *vma,
unsigned long address, pmd_t *pmdp,
diff --git a/arch/powerpc/mm/book3s64/pgtable.c b/arch/powerpc/mm/book3s64/pgtable.c
index e18ae50a275c..5b3a3bae21aa 100644
--- a/arch/powerpc/mm/book3s64/pgtable.c
+++ b/arch/powerpc/mm/book3s64/pgtable.c
@@ -136,12 +136,18 @@ static pmd_t pmd_set_protbits(pmd_t pmd, pgprot_t pgprot)
return __pmd(pmd_val(pmd) | pgprot_val(pgprot));
}
+/*
+ * At some point we should be able to get rid of
+ * pmd_mkhuge() and mk_huge_pmd() when we update all the
+ * other archs to mark the pmd huge in pfn_pmd()
+ */
pmd_t pfn_pmd(unsigned long pfn, pgprot_t pgprot)
{
unsigned long pmdv;
pmdv = (pfn << PAGE_SHIFT) & PTE_RPN_MASK;
- return pmd_set_protbits(__pmd(pmdv), pgprot);
+
+ return __pmd_mkhuge(pmd_set_protbits(__pmd(pmdv), pgprot));
}
pmd_t mk_pmd(struct page *page, pgprot_t pgprot)
--
2.26.2
^ permalink raw reply related
* Re: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
From: Arnd Bergmann @ 2020-10-22 9:14 UTC (permalink / raw)
To: Greg KH
Cc: linux-aio, open list:BROADCOM NVRAM DRIVER, David Howells,
Linux-MM, keyrings, sparclinux, Christoph Hellwig, linux-arch,
linux-s390, linux-scsi, Android Kernel Team, linux-block, Al Viro,
io-uring, Linux ARM, Jens Axboe, Parisc List, Networking,
Nick Desaulniers, linux-kernel@vger.kernel.org, LSM List,
David Laight, Linux FS-devel Mailing List, Andrew Morton,
linuxppc-dev
In-Reply-To: <20201022082654.GA1477657@kroah.com>
On Thu, Oct 22, 2020 at 10:26 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> On Thu, Oct 22, 2020 at 12:39:14AM +0100, Al Viro wrote:
> > On Wed, Oct 21, 2020 at 06:13:01PM +0200, Greg KH wrote:
> > > On Fri, Sep 25, 2020 at 06:51:39AM +0200, Christoph Hellwig wrote:
> > >
> > > I can't really figure out what the regression is, only that this commit
> > > triggers a "large Android system binary" from working properly. There's
> > > no kernel log messages anywhere, and I don't have any way to strace the
> > > thing in the testing framework, so any hints that people can provide
> > > would be most appreciated.
> >
> > It's a pure move - modulo changed line breaks in the argument lists
> > the functions involved are identical before and after that (just checked
> > that directly, by checking out the trees before and after, extracting two
> > functions in question from fs/read_write.c and lib/iov_iter.c (before and
> > after, resp.) and checking the diff between those.
> >
> > How certain is your bisection?
>
> The bisection is very reproducable.
>
> But, this looks now to be a compiler bug. I'm using the latest version
> of clang and if I put "noinline" at the front of the function,
> everything works.
>
> Nick, any ideas here as to who I should report this to?
>
> I'll work on a fixup patch for the Android kernel tree to see if I can
> work around it there, but others will hit this in Linus's tree sooner or
> later...
I see that Christoph rewrote the function again in bfdc59701d6d
("iov_iter: refactor rw_copy_check_uvector and import_iovec"),
do you know if the current mainline version is also affected?
Do you know if it happens across multiple architectures or might
be specific to either x86 or arm64?
https://bugs.llvm.org/ is obviously the place for reporting the
issue if it does turn out to be a bug in clang, but that requires
a specific thing going wrong in the output.
One idea I have for debugging it is to sprinkle the inlined
function with lots of barrier()s to prevent a lot of the optimizations.
If that solves the issue, you can bisect through those until you
find one barrier that makes the difference between working and
broken and then look at diff of the assembler output.
Arnd
^ permalink raw reply
* Re: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
From: David Hildenbrand @ 2020-10-22 9:19 UTC (permalink / raw)
To: Greg KH
Cc: linux-aio@kvack.org, linux-mips@vger.kernel.org, David Howells,
linux-mm@kvack.org, keyrings@vger.kernel.org,
sparclinux@vger.kernel.org, Christoph Hellwig,
linux-arch@vger.kernel.org, linux-s390@vger.kernel.org,
linux-scsi@vger.kernel.org, kernel-team@android.com,
Arnd Bergmann, linux-block@vger.kernel.org, Al Viro,
io-uring@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
Jens Axboe, linux-parisc@vger.kernel.org, netdev@vger.kernel.org,
Nick Desaulniers, linux-kernel@vger.kernel.org,
linux-security-module@vger.kernel.org, David Laight,
linux-fsdevel@vger.kernel.org, Andrew Morton,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20201022090155.GA1483166@kroah.com>
On 22.10.20 11:01, Greg KH wrote:
> On Thu, Oct 22, 2020 at 10:48:59AM +0200, David Hildenbrand wrote:
>> On 22.10.20 10:40, David Laight wrote:
>>> From: David Hildenbrand
>>>> Sent: 22 October 2020 09:35
>>>>
>>>> On 22.10.20 10:26, Greg KH wrote:
>>>>> On Thu, Oct 22, 2020 at 12:39:14AM +0100, Al Viro wrote:
>>>>>> On Wed, Oct 21, 2020 at 06:13:01PM +0200, Greg KH wrote:
>>>>>>> On Fri, Sep 25, 2020 at 06:51:39AM +0200, Christoph Hellwig wrote:
>>>>>>>> From: David Laight <David.Laight@ACULAB.COM>
>>>>>>>>
>>>>>>>> This lets the compiler inline it into import_iovec() generating
>>>>>>>> much better code.
>>>>>>>>
>>>>>>>> Signed-off-by: David Laight <david.laight@aculab.com>
>>>>>>>> Signed-off-by: Christoph Hellwig <hch@lst.de>
>>>>>>>> ---
>>>>>>>> fs/read_write.c | 179 ------------------------------------------------
>>>>>>>> lib/iov_iter.c | 176 +++++++++++++++++++++++++++++++++++++++++++++++
>>>>>>>> 2 files changed, 176 insertions(+), 179 deletions(-)
>>>>>>>
>>>>>>> Strangely, this commit causes a regression in Linus's tree right now.
>>>>>>>
>>>>>>> I can't really figure out what the regression is, only that this commit
>>>>>>> triggers a "large Android system binary" from working properly. There's
>>>>>>> no kernel log messages anywhere, and I don't have any way to strace the
>>>>>>> thing in the testing framework, so any hints that people can provide
>>>>>>> would be most appreciated.
>>>>>>
>>>>>> It's a pure move - modulo changed line breaks in the argument lists
>>>>>> the functions involved are identical before and after that (just checked
>>>>>> that directly, by checking out the trees before and after, extracting two
>>>>>> functions in question from fs/read_write.c and lib/iov_iter.c (before and
>>>>>> after, resp.) and checking the diff between those.
>>>>>>
>>>>>> How certain is your bisection?
>>>>>
>>>>> The bisection is very reproducable.
>>>>>
>>>>> But, this looks now to be a compiler bug. I'm using the latest version
>>>>> of clang and if I put "noinline" at the front of the function,
>>>>> everything works.
>>>>
>>>> Well, the compiler can do more invasive optimizations when inlining. If
>>>> you have buggy code that relies on some unspecified behavior, inlining
>>>> can change the behavior ... but going over that code, there isn't too
>>>> much action going on. At least nothing screamed at me.
>>>
>>> Apart from all the optimisations that get rid off the 'pass be reference'
>>> parameters and strange conditional tests.
>>> Plenty of scope for the compiler getting it wrong.
>>> But nothing even vaguely illegal.
>>
>> Not the first time that people blame the compiler to then figure out
>> that something else is wrong ... but maybe this time is different :)
>
> I agree, I hate to blame the compiler, that's almost never the real
> problem, but this one sure "feels" like it.
>
> I'm running some more tests, trying to narrow things down as just adding
> a "noinline" to the function that got moved here doesn't work on Linus's
> tree at the moment because the function was split into multiple
> functions.
>
> Give me a few hours...
I might be wrong but
a) import_iovec() uses:
- unsigned nr_segs -> int
- unsigned fast_segs -> int
b) rw_copy_check_uvector() uses:
- unsigned long nr_segs -> long
- unsigned long fast_seg -> long
So when calling rw_copy_check_uvector(), we have to zero-extend the
registers used for passing the arguments. That's definitely done when
calling the function explicitly. Maybe when inlining something is messed up?
Just a thought ...
--
Thanks,
David / dhildenb
^ permalink raw reply
* Re: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
From: David Hildenbrand @ 2020-10-22 9:25 UTC (permalink / raw)
To: Greg KH
Cc: linux-aio@kvack.org, linux-mips@vger.kernel.org, David Howells,
linux-mm@kvack.org, keyrings@vger.kernel.org,
sparclinux@vger.kernel.org, Christoph Hellwig,
linux-arch@vger.kernel.org, linux-s390@vger.kernel.org,
linux-scsi@vger.kernel.org, kernel-team@android.com,
Arnd Bergmann, linux-block@vger.kernel.org, Al Viro,
io-uring@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
Jens Axboe, linux-parisc@vger.kernel.org, netdev@vger.kernel.org,
Nick Desaulniers, linux-kernel@vger.kernel.org,
linux-security-module@vger.kernel.org, David Laight,
linux-fsdevel@vger.kernel.org, Andrew Morton,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <e04d0c5d-e834-a15b-7844-44dcc82785cc@redhat.com>
On 22.10.20 11:19, David Hildenbrand wrote:
> On 22.10.20 11:01, Greg KH wrote:
>> On Thu, Oct 22, 2020 at 10:48:59AM +0200, David Hildenbrand wrote:
>>> On 22.10.20 10:40, David Laight wrote:
>>>> From: David Hildenbrand
>>>>> Sent: 22 October 2020 09:35
>>>>>
>>>>> On 22.10.20 10:26, Greg KH wrote:
>>>>>> On Thu, Oct 22, 2020 at 12:39:14AM +0100, Al Viro wrote:
>>>>>>> On Wed, Oct 21, 2020 at 06:13:01PM +0200, Greg KH wrote:
>>>>>>>> On Fri, Sep 25, 2020 at 06:51:39AM +0200, Christoph Hellwig wrote:
>>>>>>>>> From: David Laight <David.Laight@ACULAB.COM>
>>>>>>>>>
>>>>>>>>> This lets the compiler inline it into import_iovec() generating
>>>>>>>>> much better code.
>>>>>>>>>
>>>>>>>>> Signed-off-by: David Laight <david.laight@aculab.com>
>>>>>>>>> Signed-off-by: Christoph Hellwig <hch@lst.de>
>>>>>>>>> ---
>>>>>>>>> fs/read_write.c | 179 ------------------------------------------------
>>>>>>>>> lib/iov_iter.c | 176 +++++++++++++++++++++++++++++++++++++++++++++++
>>>>>>>>> 2 files changed, 176 insertions(+), 179 deletions(-)
>>>>>>>>
>>>>>>>> Strangely, this commit causes a regression in Linus's tree right now.
>>>>>>>>
>>>>>>>> I can't really figure out what the regression is, only that this commit
>>>>>>>> triggers a "large Android system binary" from working properly. There's
>>>>>>>> no kernel log messages anywhere, and I don't have any way to strace the
>>>>>>>> thing in the testing framework, so any hints that people can provide
>>>>>>>> would be most appreciated.
>>>>>>>
>>>>>>> It's a pure move - modulo changed line breaks in the argument lists
>>>>>>> the functions involved are identical before and after that (just checked
>>>>>>> that directly, by checking out the trees before and after, extracting two
>>>>>>> functions in question from fs/read_write.c and lib/iov_iter.c (before and
>>>>>>> after, resp.) and checking the diff between those.
>>>>>>>
>>>>>>> How certain is your bisection?
>>>>>>
>>>>>> The bisection is very reproducable.
>>>>>>
>>>>>> But, this looks now to be a compiler bug. I'm using the latest version
>>>>>> of clang and if I put "noinline" at the front of the function,
>>>>>> everything works.
>>>>>
>>>>> Well, the compiler can do more invasive optimizations when inlining. If
>>>>> you have buggy code that relies on some unspecified behavior, inlining
>>>>> can change the behavior ... but going over that code, there isn't too
>>>>> much action going on. At least nothing screamed at me.
>>>>
>>>> Apart from all the optimisations that get rid off the 'pass be reference'
>>>> parameters and strange conditional tests.
>>>> Plenty of scope for the compiler getting it wrong.
>>>> But nothing even vaguely illegal.
>>>
>>> Not the first time that people blame the compiler to then figure out
>>> that something else is wrong ... but maybe this time is different :)
>>
>> I agree, I hate to blame the compiler, that's almost never the real
>> problem, but this one sure "feels" like it.
>>
>> I'm running some more tests, trying to narrow things down as just adding
>> a "noinline" to the function that got moved here doesn't work on Linus's
>> tree at the moment because the function was split into multiple
>> functions.
>>
>> Give me a few hours...
>
> I might be wrong but
>
> a) import_iovec() uses:
> - unsigned nr_segs -> int
> - unsigned fast_segs -> int
> b) rw_copy_check_uvector() uses:
> - unsigned long nr_segs -> long
> - unsigned long fast_seg -> long
>
> So when calling rw_copy_check_uvector(), we have to zero-extend the
> registers used for passing the arguments. That's definitely done when
> calling the function explicitly. Maybe when inlining something is messed up?
>
> Just a thought ...
>
... especially because I recall that clang and gcc behave slightly
differently:
https://github.com/hjl-tools/x86-psABI/issues/2
"Function args are different: narrow types are sign or zero extended to
32 bits, depending on their type. clang depends on this for incoming
args, but gcc doesn't make that assumption. But both compilers do it
when calling, so gcc code can call clang code.
The upper 32 bits of registers are always undefined garbage for types
smaller than 64 bits."
Again, just a thought.
--
Thanks,
David / dhildenb
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox