* [PATCH] powerpc/64s: remplement power4_idle code in C
From: Nicholas Piggin @ 2019-05-02 5:22 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin
This implements the tricky tracing and soft irq handling bits in C,
leaving the low level bit to asm.
A functional difference is that this redirects the interrupt exit to
a return stub to execute blr, rather than the lr address itself. This
is probably irrelevant or barely measurable on real hardware, but it
follows the modern practice of balancing the link stack.
Tested with QEMU.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/processor.h | 3 +
arch/powerpc/kernel/Makefile | 3 +-
arch/powerpc/kernel/exceptions-64s.S | 9 ---
arch/powerpc/kernel/idle.c | 25 ++++++++
arch/powerpc/kernel/idle_book3s.S | 35 +++++++++++
arch/powerpc/kernel/idle_power4.S | 87 ----------------------------
arch/powerpc/platforms/Kconfig | 4 ++
7 files changed, 68 insertions(+), 98 deletions(-)
delete mode 100644 arch/powerpc/kernel/idle_power4.S
diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
index 706ac5df546f..c0d870c57061 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -418,6 +418,9 @@ static inline unsigned long get_clean_sp(unsigned long sp, int is_32)
extern unsigned long isa300_idle_stop_noloss(unsigned long psscr_val);
extern unsigned long isa300_idle_stop_mayloss(unsigned long psscr_val);
extern unsigned long isa206_idle_insn_mayloss(unsigned long type);
+#ifdef CONFIG_PPC_970_NAP
+extern void power4_idle_nap(void);
+#endif
extern unsigned long cpuidle_disable;
enum idle_boot_override {IDLE_NO_OVERRIDE = 0, IDLE_POWERSAVE_OFF};
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index cddadccf551d..2af8ee47b2ee 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -51,8 +51,7 @@ obj-$(CONFIG_PPC_BOOK3E_64) += exceptions-64e.o idle_book3e.o
obj-$(CONFIG_PPC_BARRIER_NOSPEC) += security.o
obj-$(CONFIG_PPC64) += vdso64/
obj-$(CONFIG_ALTIVEC) += vecemu.o
-obj-$(CONFIG_PPC_970_NAP) += idle_power4.o
-obj-$(CONFIG_PPC_P7_NAP) += idle_book3s.o
+obj-$(CONFIG_PPC_BOOK3S_IDLE) += idle_book3s.o
procfs-y := proc_powerpc.o
obj-$(CONFIG_PROC_FS) += $(procfs-y)
rtaspci-$(CONFIG_PPC64)-$(CONFIG_PCI) := rtas_pci.o
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index 6b86055e5251..e173690dad52 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -1683,15 +1683,6 @@ USE_FIXED_SECTION(virt_trampolines)
__end_interrupts:
DEFINE_FIXED_SYMBOL(__end_interrupts)
-#ifdef CONFIG_PPC_970_NAP
-EXC_COMMON_BEGIN(power4_fixup_nap)
- andc r9,r9,r10
- std r9,TI_LOCAL_FLAGS(r11)
- ld r10,_LINK(r1) /* make idle task do the */
- std r10,_NIP(r1) /* equivalent of a blr */
- blr
-#endif
-
CLOSE_FIXED_SECTION(real_vectors);
CLOSE_FIXED_SECTION(real_trampolines);
CLOSE_FIXED_SECTION(virt_vectors);
diff --git a/arch/powerpc/kernel/idle.c b/arch/powerpc/kernel/idle.c
index d7216c9abda1..605defde43b9 100644
--- a/arch/powerpc/kernel/idle.c
+++ b/arch/powerpc/kernel/idle.c
@@ -81,6 +81,31 @@ void arch_cpu_idle(void)
int powersave_nap;
+#ifdef CONFIG_PPC_970_NAP
+void power4_idle(void)
+{
+ if (!cpu_has_feature(CPU_FTR_CAN_NAP))
+ return;
+
+ if (!powersave_nap)
+ return;
+
+ if (!prep_irq_for_idle())
+ return;
+
+ if (cpu_has_feature(CPU_FTR_ALTIVEC))
+ asm volatile("DSSALL ; sync" ::: "memory");
+
+ power4_idle_nap();
+
+ /*
+ * power4_idle_nap returns with interrupts enabled (soft and hard).
+ * to our caller with interrupts enabled (soft and hard). Our caller
+ * can cope with either interrupts disabled or enabled upon return.
+ */
+}
+#endif
+
#ifdef CONFIG_SYSCTL
/*
* Register the sysctl to set/clear powersave_nap.
diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
index 2dfbd5d5b932..5afac9177fec 100644
--- a/arch/powerpc/kernel/idle_book3s.S
+++ b/arch/powerpc/kernel/idle_book3s.S
@@ -19,7 +19,9 @@
#include <asm/asm-offsets.h>
#include <asm/ppc-opcode.h>
#include <asm/cpuidle.h>
+#include <asm/thread_info.h> /* TLF_NAPPING */
+#ifdef CONFIG_PPC_P7_NAP
/*
* Desired PSSCR in r3
*
@@ -185,4 +187,37 @@ _GLOBAL(isa206_idle_insn_mayloss)
bne 2f
IDLE_STATE_ENTER_SEQ_NORET(PPC_SLEEP)
2: IDLE_STATE_ENTER_SEQ_NORET(PPC_WINKLE)
+#endif
+#ifdef CONFIG_PPC_970_NAP
+_GLOBAL(power4_idle_nap)
+ LOAD_REG_IMMEDIATE(r7, MSR_KERNEL|MSR_EE|MSR_POW)
+ ld r9,PACA_THREAD_INFO(r13)
+ ld r8,TI_LOCAL_FLAGS(r9)
+ ori r8,r8,_TLF_NAPPING
+ std r8,TI_LOCAL_FLAGS(r9)
+ /*
+ * NAPPING bit is set, from this point onward power4_fixup_nap
+ * will cause exceptions to return to power4_idle_nap_return.
+ */
+1: sync
+ isync
+ mtmsrd r7
+ isync
+ b 1b
+power4_idle_nap_return:
+ blr
+
+ /*
+ * Called by exception entry code if _TLF_NAPPING was set, this clear
+ * the NAPPING flag, and redirects the exception exit to
+ * power4_fixup_nap_return.
+ */
+ .globl power4_fixup_nap
+power4_fixup_nap:
+ andc r9,r9,r10
+ std r9,TI_LOCAL_FLAGS(r11)
+ LOAD_REG_ADDR(r10, power4_idle_nap_return)
+ std r10,_NIP(r1)
+ blr
+#endif
diff --git a/arch/powerpc/kernel/idle_power4.S b/arch/powerpc/kernel/idle_power4.S
deleted file mode 100644
index a2fdb0a34b75..000000000000
--- a/arch/powerpc/kernel/idle_power4.S
+++ /dev/null
@@ -1,87 +0,0 @@
-/*
- * This file contains the power_save function for 970-family CPUs.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * as published by the Free Software Foundation; either version
- * 2 of the License, or (at your option) any later version.
- */
-
-#include <linux/threads.h>
-#include <asm/processor.h>
-#include <asm/page.h>
-#include <asm/cputable.h>
-#include <asm/thread_info.h>
-#include <asm/ppc_asm.h>
-#include <asm/asm-offsets.h>
-#include <asm/irqflags.h>
-#include <asm/hw_irq.h>
-#include <asm/feature-fixups.h>
-
-#undef DEBUG
-
- .text
-
-_GLOBAL(power4_idle)
-BEGIN_FTR_SECTION
- blr
-END_FTR_SECTION_IFCLR(CPU_FTR_CAN_NAP)
- /* Now check if user or arch enabled NAP mode */
- LOAD_REG_ADDRBASE(r3,powersave_nap)
- lwz r4,ADDROFF(powersave_nap)(r3)
- cmpwi 0,r4,0
- beqlr
-
- /* This sequence is similar to prep_irq_for_idle() */
-
- /* Hard disable interrupts */
- mfmsr r7
- rldicl r0,r7,48,1
- rotldi r0,r0,16
- mtmsrd r0,1
-
- /* Check if something happened while soft-disabled */
- lbz r0,PACAIRQHAPPENED(r13)
- cmpwi cr0,r0,0
- bne- 2f
-
- /*
- * Soft-enable interrupts. This will make power4_fixup_nap return
- * to our caller with interrupts enabled (soft and hard). The caller
- * can cope with either interrupts disabled or enabled upon return.
- */
-#ifdef CONFIG_TRACE_IRQFLAGS
- /* Tell the tracer interrupts are on, because idle responds to them. */
- mflr r0
- std r0,16(r1)
- stdu r1,-128(r1)
- bl trace_hardirqs_on
- addi r1,r1,128
- ld r0,16(r1)
- mtlr r0
- mfmsr r7
-#endif /* CONFIG_TRACE_IRQFLAGS */
-
- li r0,IRQS_ENABLED
- stb r0,PACAIRQSOFTMASK(r13) /* we'll hard-enable shortly */
-BEGIN_FTR_SECTION
- DSSALL
- sync
-END_FTR_SECTION_IFSET(CPU_FTR_ALTIVEC)
- ld r9, PACA_THREAD_INFO(r13)
- ld r8,TI_LOCAL_FLAGS(r9) /* set napping bit */
- ori r8,r8,_TLF_NAPPING /* so when we take an exception */
- std r8,TI_LOCAL_FLAGS(r9) /* it will return to our caller */
- ori r7,r7,MSR_EE
- oris r7,r7,MSR_POW@h
-1: sync
- isync
- mtmsrd r7
- isync
- b 1b
-
-2: /* Return if an interrupt had happened while soft disabled */
- /* Set the HARD_DIS flag because interrupts are now hard disabled */
- ori r0,r0,PACA_IRQ_HARD_DIS
- stb r0,PACAIRQHAPPENED(r13)
- blr
diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig
index f3fb79fccc72..736325451bc4 100644
--- a/arch/powerpc/platforms/Kconfig
+++ b/arch/powerpc/platforms/Kconfig
@@ -177,6 +177,10 @@ config PPC_970_NAP
config PPC_P7_NAP
bool
+config PPC_BOOK3S_IDLE
+ def_bool y
+ depends on (PPC_970_NAP || PPC_P7_NAP)
+
config PPC_INDIRECT_PIO
bool
select GENERIC_IOMAP
--
2.20.1
^ permalink raw reply related
* Re: [PATCH 2/2] powerpc/mm: Warn if W+X pages found on boot
From: Christophe Leroy @ 2019-05-02 5:23 UTC (permalink / raw)
To: Russell Currey, linuxppc-dev; +Cc: Julia.Lawall, rashmica.g
In-Reply-To: <ca5e1db5fca5c12ca69d7810d575a437ae39ec87.camel@russell.cc>
Le 01/05/2019 à 09:04, Russell Currey a écrit :
> On Wed, 2019-04-24 at 09:14 +0200, Christophe Leroy wrote:
>>
>> Le 24/04/2019 à 08:39, Russell Currey a écrit :
>>> Implement code to walk all pages and warn if any are found to be
>>> both
>>> writable and executable. Depends on STRICT_KERNEL_RWX enabled, and
>>> is
>>> behind the DEBUG_WX config option.
>>>
>>> This only runs on boot and has no runtime performance implications.
>>>
>>> Very heavily influenced (and in some cases copied verbatim) from
>>> the
>>> ARM64 code written by Laura Abbott (thanks!), since our ptdump
>>> infrastructure is similar.
>>>
>>> Signed-off-by: Russell Currey <ruscur@russell.cc>
>>> ---
>>> arch/powerpc/Kconfig.debug | 19 +++++++++++++++
>>> arch/powerpc/include/asm/pgtable.h | 5 ++++
>>> arch/powerpc/mm/pgtable_32.c | 5 ++++
>>> arch/powerpc/mm/pgtable_64.c | 5 ++++
>>> arch/powerpc/mm/ptdump/ptdump.c | 38
>>> ++++++++++++++++++++++++++++++
>>> 5 files changed, 72 insertions(+)
>>>
>>> diff --git a/arch/powerpc/Kconfig.debug
>>> b/arch/powerpc/Kconfig.debug
>>> index 4e00cb0a5464..a4160ff02ed4 100644
>>> --- a/arch/powerpc/Kconfig.debug
>>> +++ b/arch/powerpc/Kconfig.debug
>>> @@ -361,6 +361,25 @@ config PPC_PTDUMP
>>>
>>> If you are unsure, say N.
>>>
>>> +config DEBUG_WX
>>
>> I would call it PPC_DEBUG_WX to avoid confusion.
>
> It's the same functionality as on other architectures and is an arch-
> local thing, I personally think it should be left as-is but given we
> already put the PPC prefix on PTDUMP, I'll add it so it's consistent
>
> <snip>
>
>>> + if (radix_enabled())
>>> + st.start_address = PAGE_OFFSET;
>>> + else
>> + st.start_address = KERN_VIRT_START;
>>
>> KERN_VIRT_START doesn't exist on PPC32.
>>
>> Christophe
>>
> Thanks a lot for the review! Applied all your suggestions. What
> should I use on PPC32 instead?
Indeed it looks like KERN_VIRT_START is defined as 0 for PPC32 at the
top of ptdump.c, which look strange to me.
I guess PAGE_OFFSET should be the good value for KERN_VIRT_START on PPC32.
Christophe
>
> - Russell
>
^ permalink raw reply
* Re: [PATCH 2/2] powerpc/mm: Warn if W+X pages found on boot
From: Russell Currey @ 2019-05-02 5:51 UTC (permalink / raw)
To: Christophe Leroy, linuxppc-dev; +Cc: Julia.Lawall, rashmica.g
In-Reply-To: <673532fb-cd09-748f-e936-14c0cab854b1@c-s.fr>
> > > > + if (radix_enabled())
> > > > + st.start_address = PAGE_OFFSET;
> > > > + else
> > > + st.start_address = KERN_VIRT_START;
> > >
> > > KERN_VIRT_START doesn't exist on PPC32.
> > >
> > > Christophe
> > >
> > Thanks a lot for the review! Applied all your suggestions. What
> > should I use on PPC32 instead?
>
> Indeed it looks like KERN_VIRT_START is defined as 0 for PPC32 at
> the
> top of ptdump.c, which look strange to me.
>
> I guess PAGE_OFFSET should be the good value for KERN_VIRT_START on
> PPC32.
>
> Christophe
git blame says you put it there :) I'll set it to PAGE_OFFSET instead
of zero. Cheers
- Russell
^ permalink raw reply
* Re: [PATCHv2] kernel/crash: make parse_crashkernel()'s return value more indicant
From: Pingfan Liu @ 2019-05-02 6:22 UTC (permalink / raw)
To: Matthias Brugger
Cc: Rich Felker, linux-ia64, Julien Thierry, Yangtao Li,
Palmer Dabbelt, Heiko Carstens, Stefan Agner, linux-mips,
Paul Mackerras, H. Peter Anvin, Thomas Gleixner, Logan Gunthorpe,
linux-s390, Florian Fainelli, Yoshinori Sato, linux-sh, x86,
Russell King, Ingo Molnar, Hari Bathini, Catalin Marinas,
James Hogan, Dave Young, Fenghua Yu, Will Deacon, Johannes Weiner,
Borislav Petkov, David Hildenbrand, linux-arm-kernel, Jens Axboe,
Tony Luck, Baoquan He, Ard Biesheuvel, Robin Murphy, LKML,
Ralf Baechle, Thomas Bogendoerfer, Paul Burton,
Greg Kroah-Hartman, Martin Schwidefsky, Andrew Morton,
linuxppc-dev, Greg Hackmann
In-Reply-To: <CAFgQCTstd667wP6g+maxYekz4u3iBR2R=FHUiS1V=XxTs6MKUw@mail.gmail.com>
On Thu, Apr 25, 2019 at 4:20 PM Pingfan Liu <kernelfans@gmail.com> wrote:
>
> On Wed, Apr 24, 2019 at 4:31 PM Matthias Brugger <mbrugger@suse.com> wrote:
> >
> >
> [...]
> > > @@ -139,6 +141,8 @@ static int __init parse_crashkernel_simple(char *cmdline,
> > > pr_warn("crashkernel: unrecognized char: %c\n", *cur);
> > > return -EINVAL;
> > > }
> > > + if (*crash_size == 0)
> > > + return -EINVAL;
> >
> > This covers the case where I pass an argument like "crashkernel=0M" ?
> > Can't we fix that by using kstrtoull() in memparse and check if the return value
> > is < 0? In that case we could return without updating the retptr and we will be
> > fine.
After a series of work, I suddenly realized that it can not be done
like this way. "0M" causes kstrtoull() to return -EINVAL, but this is
caused by "M", not "0". If passing "0" to kstrtoull(), it will return
0 on success.
> >
> It seems that kstrtoull() treats 0M as invalid parameter, while
> simple_strtoull() does not.
>
My careless going through the code. And I tested with a valid value
"256M" using kstrtoull(), it also returned -EINVAL.
So I think there is no way to distinguish 0 from a positive value
inside this basic math function.
Do I miss anything?
Thanks and regards,
Pingfan
^ permalink raw reply
* Re: [PATCH v1 2/4] powerpc/mm: Move book3s64 specifics in subdirectory mm/book3s64
From: Michael Ellerman @ 2019-05-02 7:11 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <c4afde657ef9e4ad0266ae62e9907313c41c4a16.1553853405.git.christophe.leroy@c-s.fr>
Christophe Leroy <christophe.leroy@c-s.fr> writes:
> Many files in arch/powerpc/mm are only for book3S64. This patch
> creates a subdirectory for them.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> arch/powerpc/mm/Makefile | 25 +++----------------
> arch/powerpc/mm/book3s64/Makefile | 28 ++++++++++++++++++++++
> arch/powerpc/mm/{ => book3s64}/hash64_4k.c | 0
> arch/powerpc/mm/{ => book3s64}/hash64_64k.c | 0
> arch/powerpc/mm/{ => book3s64}/hash_native_64.c | 0
> arch/powerpc/mm/{ => book3s64}/hash_utils_64.c | 0
> arch/powerpc/mm/{ => book3s64}/hugepage-hash64.c | 0
> .../powerpc/mm/{ => book3s64}/hugetlbpage-hash64.c | 0
> arch/powerpc/mm/{ => book3s64}/hugetlbpage-radix.c | 0
> .../mm/{ => book3s64}/mmu_context_book3s64.c | 0
> arch/powerpc/mm/{ => book3s64}/mmu_context_iommu.c | 0
> arch/powerpc/mm/{ => book3s64}/pgtable-book3s64.c | 0
> arch/powerpc/mm/{ => book3s64}/pgtable-hash64.c | 0
> arch/powerpc/mm/{ => book3s64}/pgtable-radix.c | 0
> arch/powerpc/mm/{ => book3s64}/pkeys.c | 0
> arch/powerpc/mm/{ => book3s64}/slb.c | 0
> arch/powerpc/mm/{ => book3s64}/subpage-prot.c | 0
> arch/powerpc/mm/{ => book3s64}/tlb-radix.c | 0
> arch/powerpc/mm/{ => book3s64}/tlb_hash64.c | 0
> arch/powerpc/mm/{ => book3s64}/vphn.c | 0
> arch/powerpc/mm/{ => book3s64}/vphn.h | 0
> arch/powerpc/mm/numa.c | 2 +-
> 22 files changed, 32 insertions(+), 23 deletions(-)
> create mode 100644 arch/powerpc/mm/book3s64/Makefile
> rename arch/powerpc/mm/{ => book3s64}/hash64_4k.c (100%)
> rename arch/powerpc/mm/{ => book3s64}/hash64_64k.c (100%)
> rename arch/powerpc/mm/{ => book3s64}/hash_native_64.c (100%)
> rename arch/powerpc/mm/{ => book3s64}/hash_utils_64.c (100%)
> rename arch/powerpc/mm/{ => book3s64}/hugepage-hash64.c (100%)
> rename arch/powerpc/mm/{ => book3s64}/hugetlbpage-hash64.c (100%)
> rename arch/powerpc/mm/{ => book3s64}/hugetlbpage-radix.c (100%)
> rename arch/powerpc/mm/{ => book3s64}/mmu_context_book3s64.c (100%)
> rename arch/powerpc/mm/{ => book3s64}/mmu_context_iommu.c (100%)
> rename arch/powerpc/mm/{ => book3s64}/pgtable-book3s64.c (100%)
> rename arch/powerpc/mm/{ => book3s64}/pgtable-hash64.c (100%)
> rename arch/powerpc/mm/{ => book3s64}/pgtable-radix.c (100%)
> rename arch/powerpc/mm/{ => book3s64}/pkeys.c (100%)
> rename arch/powerpc/mm/{ => book3s64}/slb.c (100%)
> rename arch/powerpc/mm/{ => book3s64}/subpage-prot.c (100%)
> rename arch/powerpc/mm/{ => book3s64}/tlb-radix.c (100%)
> rename arch/powerpc/mm/{ => book3s64}/tlb_hash64.c (100%)
Do you mind if I take this but rework the destination names in the process?
I don't like having eg. book3s64/pgtable-book3s64.c
And some of the other names could use a bit of cleanup too.
What about:
arch/powerpc/mm/{hash64_4k.c => book3s64/hash_4k.c}
arch/powerpc/mm/{hash64_64k.c => book3s64/hash_64k.c}
arch/powerpc/mm/{hugepage-hash64.c => book3s64/hash_hugepage.c}
arch/powerpc/mm/{hugetlbpage-hash64.c => book3s64/hash_hugetlbpage.c}
arch/powerpc/mm/{hash_native_64.c => book3s64/hash_native.c}
arch/powerpc/mm/{pgtable-hash64.c => book3s64/hash_pgtable.c}
arch/powerpc/mm/{tlb_hash64.c => book3s64/hash_tlb.c}
arch/powerpc/mm/{hash_utils_64.c => book3s64/hash_utils.c}
arch/powerpc/mm/{mmu_context_iommu.c => book3s64/iommu_api.c}
arch/powerpc/mm/{mmu_context_book3s64.c => book3s64/mmu_context.c}
arch/powerpc/mm/{pgtable-book3s64.c => book3s64/pgtable.c}
arch/powerpc/mm/{hugetlbpage-radix.c => book3s64/radix_hugetlbpage.c}
arch/powerpc/mm/{pgtable-radix.c => book3s64/radix_pgtable.c}
arch/powerpc/mm/{tlb-radix.c => book3s64/radix_tlb.c}
cheers
^ permalink raw reply
* Re: [PATCH v1 2/4] powerpc/mm: Move book3s64 specifics in subdirectory mm/book3s64
From: Christophe Leroy @ 2019-05-02 7:15 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <87sgtx5o0j.fsf@concordia.ellerman.id.au>
Le 02/05/2019 à 09:11, Michael Ellerman a écrit :
> Christophe Leroy <christophe.leroy@c-s.fr> writes:
>
>> Many files in arch/powerpc/mm are only for book3S64. This patch
>> creates a subdirectory for them.
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>> ---
>> arch/powerpc/mm/Makefile | 25 +++----------------
>> arch/powerpc/mm/book3s64/Makefile | 28 ++++++++++++++++++++++
>> arch/powerpc/mm/{ => book3s64}/hash64_4k.c | 0
>> arch/powerpc/mm/{ => book3s64}/hash64_64k.c | 0
>> arch/powerpc/mm/{ => book3s64}/hash_native_64.c | 0
>> arch/powerpc/mm/{ => book3s64}/hash_utils_64.c | 0
>> arch/powerpc/mm/{ => book3s64}/hugepage-hash64.c | 0
>> .../powerpc/mm/{ => book3s64}/hugetlbpage-hash64.c | 0
>> arch/powerpc/mm/{ => book3s64}/hugetlbpage-radix.c | 0
>> .../mm/{ => book3s64}/mmu_context_book3s64.c | 0
>> arch/powerpc/mm/{ => book3s64}/mmu_context_iommu.c | 0
>> arch/powerpc/mm/{ => book3s64}/pgtable-book3s64.c | 0
>> arch/powerpc/mm/{ => book3s64}/pgtable-hash64.c | 0
>> arch/powerpc/mm/{ => book3s64}/pgtable-radix.c | 0
>> arch/powerpc/mm/{ => book3s64}/pkeys.c | 0
>> arch/powerpc/mm/{ => book3s64}/slb.c | 0
>> arch/powerpc/mm/{ => book3s64}/subpage-prot.c | 0
>> arch/powerpc/mm/{ => book3s64}/tlb-radix.c | 0
>> arch/powerpc/mm/{ => book3s64}/tlb_hash64.c | 0
>> arch/powerpc/mm/{ => book3s64}/vphn.c | 0
>> arch/powerpc/mm/{ => book3s64}/vphn.h | 0
>> arch/powerpc/mm/numa.c | 2 +-
>> 22 files changed, 32 insertions(+), 23 deletions(-)
>> create mode 100644 arch/powerpc/mm/book3s64/Makefile
>> rename arch/powerpc/mm/{ => book3s64}/hash64_4k.c (100%)
>> rename arch/powerpc/mm/{ => book3s64}/hash64_64k.c (100%)
>> rename arch/powerpc/mm/{ => book3s64}/hash_native_64.c (100%)
>> rename arch/powerpc/mm/{ => book3s64}/hash_utils_64.c (100%)
>> rename arch/powerpc/mm/{ => book3s64}/hugepage-hash64.c (100%)
>> rename arch/powerpc/mm/{ => book3s64}/hugetlbpage-hash64.c (100%)
>> rename arch/powerpc/mm/{ => book3s64}/hugetlbpage-radix.c (100%)
>> rename arch/powerpc/mm/{ => book3s64}/mmu_context_book3s64.c (100%)
>> rename arch/powerpc/mm/{ => book3s64}/mmu_context_iommu.c (100%)
>> rename arch/powerpc/mm/{ => book3s64}/pgtable-book3s64.c (100%)
>> rename arch/powerpc/mm/{ => book3s64}/pgtable-hash64.c (100%)
>> rename arch/powerpc/mm/{ => book3s64}/pgtable-radix.c (100%)
>> rename arch/powerpc/mm/{ => book3s64}/pkeys.c (100%)
>> rename arch/powerpc/mm/{ => book3s64}/slb.c (100%)
>> rename arch/powerpc/mm/{ => book3s64}/subpage-prot.c (100%)
>> rename arch/powerpc/mm/{ => book3s64}/tlb-radix.c (100%)
>> rename arch/powerpc/mm/{ => book3s64}/tlb_hash64.c (100%)
>
> Do you mind if I take this but rework the destination names in the process?
I don't mind, I think it's a good idea.
>
> I don't like having eg. book3s64/pgtable-book3s64.c
>
> And some of the other names could use a bit of cleanup too.
>
> What about:
>
> arch/powerpc/mm/{hash64_4k.c => book3s64/hash_4k.c}
> arch/powerpc/mm/{hash64_64k.c => book3s64/hash_64k.c}
> arch/powerpc/mm/{hugepage-hash64.c => book3s64/hash_hugepage.c}
> arch/powerpc/mm/{hugetlbpage-hash64.c => book3s64/hash_hugetlbpage.c}
> arch/powerpc/mm/{hash_native_64.c => book3s64/hash_native.c}
> arch/powerpc/mm/{pgtable-hash64.c => book3s64/hash_pgtable.c}
> arch/powerpc/mm/{tlb_hash64.c => book3s64/hash_tlb.c}
> arch/powerpc/mm/{hash_utils_64.c => book3s64/hash_utils.c}
> arch/powerpc/mm/{mmu_context_iommu.c => book3s64/iommu_api.c}
> arch/powerpc/mm/{mmu_context_book3s64.c => book3s64/mmu_context.c}
> arch/powerpc/mm/{pgtable-book3s64.c => book3s64/pgtable.c}
> arch/powerpc/mm/{hugetlbpage-radix.c => book3s64/radix_hugetlbpage.c}
> arch/powerpc/mm/{pgtable-radix.c => book3s64/radix_pgtable.c}
> arch/powerpc/mm/{tlb-radix.c => book3s64/radix_tlb.c}
Looks good
Christophe
^ permalink raw reply
* [PATCH] MAINTAINERS: Update cxl/ocxl email address
From: Andrew Donnellan @ 2019-05-02 6:00 UTC (permalink / raw)
To: linuxppc-dev; +Cc: fbarrat
Use my @linux.ibm.com email to avoid a layer of redirection.
Signed-off-by: Andrew Donnellan <ajd@linux.ibm.com>
---
MAINTAINERS | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 5c38f21aee78..386e2336fe7e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4293,7 +4293,7 @@ F: drivers/net/ethernet/chelsio/cxgb4vf/
CXL (IBM Coherent Accelerator Processor Interface CAPI) DRIVER
M: Frederic Barrat <fbarrat@linux.ibm.com>
-M: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
+M: Andrew Donnellan <ajd@linux.ibm.com>
L: linuxppc-dev@lists.ozlabs.org
S: Supported
F: arch/powerpc/platforms/powernv/pci-cxl.c
@@ -11173,7 +11173,7 @@ F: tools/objtool/
OCXL (Open Coherent Accelerator Processor Interface OpenCAPI) DRIVER
M: Frederic Barrat <fbarrat@linux.ibm.com>
-M: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
+M: Andrew Donnellan <ajd@linux.ibm.com>
L: linuxppc-dev@lists.ozlabs.org
S: Supported
F: arch/powerpc/platforms/powernv/ocxl.c
--
2.20.1
^ permalink raw reply related
* Re: [PATCH 2/3] powerpc/module32: Use symbolic instructions names.
From: Christophe Leroy @ 2019-05-02 7:24 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: Paul Mackerras, linuxppc-dev, linux-kernel
In-Reply-To: <20190429115431.GN8599@gate.crashing.org>
Le 29/04/2019 à 13:54, Segher Boessenkool a écrit :
> On Mon, Apr 29, 2019 at 10:43:27AM +0000, Christophe Leroy wrote:
>> To increase readability/maintainability, replace hard coded
>> instructions values by symbolic names.
>
>> + /* lis r12,sym@ha */
>> +#define ENTRY_JMP0(sym) (PPC_INST_ADDIS | __PPC_RT(R12) | PPC_HA(sym))
>> + /* addi r12,r12,sym@l */
>> +#define ENTRY_JMP1(sym) (PPC_INST_ADDI | __PPC_RT(R12) | __PPC_RA(R12) | PPC_LO(sym))
>
> Those aren't "jump" instructions though, as the name suggests... And you
> only have names for the first two of the four insns. ("2" and "3" were
> still available ;-) )
Well, the idea was to say they are defining the jump destination.
Anyway, as they are used only once, let's put it directly in.
>
>> - entry->jump[0] = 0x3d800000+((val+0x8000)>>16); /* lis r12,sym@ha */
>> - entry->jump[1] = 0x398c0000 + (val&0xffff); /* addi r12,r12,sym@l*/
>> - entry->jump[2] = 0x7d8903a6; /* mtctr r12 */
>> - entry->jump[3] = 0x4e800420; /* bctr */
>> + entry->jump[0] = ENTRY_JMP0(val);
>> + entry->jump[1] = ENTRY_JMP1(val);
>> + entry->jump[2] = PPC_INST_MTCTR | __PPC_RS(R12);
>> + entry->jump[3] = PPC_INST_BCTR;
>
> Deleting the comment here is not an improvement imo.
Ok, I'll leave them in as I did for module64
Christophe
>
>
> Segher
>
^ permalink raw reply
* [PATCH v2 1/2] powerpc/mm/ptdump: Wrap seq_printf() to handle NULL pointers
From: Russell Currey @ 2019-05-02 7:39 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Julia.Lawall, rashmica.g, Russell Currey
Lovingly borrowed from the arch/arm64 ptdump code.
This doesn't seem to be an issue in practice, but is necessary for my
upcoming commit.
Signed-off-by: Russell Currey <ruscur@russell.cc>
---
v2: Fix putc to actually putc thanks to Christophe Leroy
arch/powerpc/mm/ptdump/ptdump.c | 32 ++++++++++++++++++++++----------
1 file changed, 22 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/mm/ptdump/ptdump.c b/arch/powerpc/mm/ptdump/ptdump.c
index 37138428ab55..a4a132f92810 100644
--- a/arch/powerpc/mm/ptdump/ptdump.c
+++ b/arch/powerpc/mm/ptdump/ptdump.c
@@ -104,6 +104,18 @@ static struct addr_marker address_markers[] = {
{ -1, NULL },
};
+#define pt_dump_seq_printf(m, fmt, args...) \
+({ \
+ if (m) \
+ seq_printf(m, fmt, ##args); \
+})
+
+#define pt_dump_seq_putc(m, c) \
+({ \
+ if (m) \
+ seq_putc(m, c); \
+})
+
static void dump_flag_info(struct pg_state *st, const struct flag_info
*flag, u64 pte, int num)
{
@@ -121,19 +133,19 @@ static void dump_flag_info(struct pg_state *st, const struct flag_info
val = pte & flag->val;
if (flag->shift)
val = val >> flag->shift;
- seq_printf(st->seq, " %s:%llx", flag->set, val);
+ pt_dump_seq_printf(st->seq, " %s:%llx", flag->set, val);
} else {
if ((pte & flag->mask) == flag->val)
s = flag->set;
else
s = flag->clear;
if (s)
- seq_printf(st->seq, " %s", s);
+ pt_dump_seq_printf(st->seq, " %s", s);
}
st->current_flags &= ~flag->mask;
}
if (st->current_flags != 0)
- seq_printf(st->seq, " unknown flags:%llx", st->current_flags);
+ pt_dump_seq_printf(st->seq, " unknown flags:%llx", st->current_flags);
}
static void dump_addr(struct pg_state *st, unsigned long addr)
@@ -148,12 +160,12 @@ static void dump_addr(struct pg_state *st, unsigned long addr)
#define REG "0x%08lx"
#endif
- seq_printf(st->seq, REG "-" REG " ", st->start_address, addr - 1);
+ pt_dump_seq_printf(st->seq, REG "-" REG " ", st->start_address, addr - 1);
if (st->start_pa == st->last_pa && st->start_address + PAGE_SIZE != addr) {
- seq_printf(st->seq, "[" REG "]", st->start_pa);
+ pt_dump_seq_printf(st->seq, "[" REG "]", st->start_pa);
delta = PAGE_SIZE >> 10;
} else {
- seq_printf(st->seq, " " REG " ", st->start_pa);
+ pt_dump_seq_printf(st->seq, " " REG " ", st->start_pa);
delta = (addr - st->start_address) >> 10;
}
/* Work out what appropriate unit to use */
@@ -161,7 +173,7 @@ static void dump_addr(struct pg_state *st, unsigned long addr)
delta >>= 10;
unit++;
}
- seq_printf(st->seq, "%9lu%c", delta, *unit);
+ pt_dump_seq_printf(st->seq, "%9lu%c", delta, *unit);
}
@@ -178,7 +190,7 @@ static void note_page(struct pg_state *st, unsigned long addr,
st->start_address = addr;
st->start_pa = pa;
st->last_pa = pa;
- seq_printf(st->seq, "---[ %s ]---\n", st->marker->name);
+ pt_dump_seq_printf(st->seq, "---[ %s ]---\n", st->marker->name);
/*
* Dump the section of virtual memory when:
* - the PTE flags from one entry to the next differs.
@@ -202,7 +214,7 @@ static void note_page(struct pg_state *st, unsigned long addr,
st->current_flags,
pg_level[st->level].num);
- seq_putc(st->seq, '\n');
+ pt_dump_seq_putc(st->seq, '\n');
}
/*
@@ -211,7 +223,7 @@ static void note_page(struct pg_state *st, unsigned long addr,
*/
while (addr >= st->marker[1].start_address) {
st->marker++;
- seq_printf(st->seq, "---[ %s ]---\n", st->marker->name);
+ pt_dump_seq_printf(st->seq, "---[ %s ]---\n", st->marker->name);
}
st->start_address = addr;
st->start_pa = pa;
--
2.21.0
^ permalink raw reply related
* [PATCH v2 2/2] powerpc/mm: Warn if W+X pages found on boot
From: Russell Currey @ 2019-05-02 7:39 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Julia.Lawall, rashmica.g, Russell Currey
In-Reply-To: <20190502073947.6481-1-ruscur@russell.cc>
Implement code to walk all pages and warn if any are found to be both
writable and executable. Depends on STRICT_KERNEL_RWX enabled, and is
behind the DEBUG_WX config option.
This only runs on boot and has no runtime performance implications.
Very heavily influenced (and in some cases copied verbatim) from the
ARM64 code written by Laura Abbott (thanks!), since our ptdump
infrastructure is similar.
Signed-off-by: Russell Currey <ruscur@russell.cc>
---
v2: A myriad of fixes and cleanups thanks to Christophe Leroy
arch/powerpc/Kconfig.debug | 19 ++++++++++++++
arch/powerpc/include/asm/pgtable.h | 6 +++++
arch/powerpc/mm/pgtable_32.c | 3 +++
arch/powerpc/mm/pgtable_64.c | 3 +++
arch/powerpc/mm/ptdump/ptdump.c | 41 +++++++++++++++++++++++++++++-
5 files changed, 71 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
index 4e00cb0a5464..9e8bcddd8b8f 100644
--- a/arch/powerpc/Kconfig.debug
+++ b/arch/powerpc/Kconfig.debug
@@ -361,6 +361,25 @@ config PPC_PTDUMP
If you are unsure, say N.
+config PPC_DEBUG_WX
+ bool "Warn on W+X mappings at boot"
+ select PPC_PTDUMP
+ help
+ Generate a warning if any W+X mappings are found at boot.
+
+ This is useful for discovering cases where the kernel is leaving
+ W+X mappings after applying NX, as such mappings are a security risk.
+
+ Note that even if the check fails, your kernel is possibly
+ still fine, as W+X mappings are not a security hole in
+ themselves, what they do is that they make the exploitation
+ of other unfixed kernel bugs easier.
+
+ There is no runtime or memory usage effect of this option
+ once the kernel has booted up - it's a one time check.
+
+ If in doubt, say "Y".
+
config PPC_FAST_ENDIAN_SWITCH
bool "Deprecated fast endian-switch syscall"
depends on DEBUG_KERNEL && PPC_BOOK3S_64
diff --git a/arch/powerpc/include/asm/pgtable.h b/arch/powerpc/include/asm/pgtable.h
index 505550fb2935..50c0d06fac2f 100644
--- a/arch/powerpc/include/asm/pgtable.h
+++ b/arch/powerpc/include/asm/pgtable.h
@@ -108,6 +108,12 @@ void mark_initmem_nx(void);
static inline void mark_initmem_nx(void) { }
#endif
+#ifdef CONFIG_PPC_DEBUG_WX
+void ptdump_check_wx(void);
+#else
+static inline void ptdump_check_wx(void) { }
+#endif
+
/*
* When used, PTE_FRAG_NR is defined in subarch pgtable.h
* so we are sure it is included when arriving here.
diff --git a/arch/powerpc/mm/pgtable_32.c b/arch/powerpc/mm/pgtable_32.c
index 6e56a6240bfa..6f919779ee06 100644
--- a/arch/powerpc/mm/pgtable_32.c
+++ b/arch/powerpc/mm/pgtable_32.c
@@ -384,6 +384,9 @@ void mark_rodata_ro(void)
PFN_DOWN((unsigned long)__start_rodata);
change_page_attr(page, numpages, PAGE_KERNEL_RO);
+
+ // mark_initmem_nx() should have already run by now
+ ptdump_check_wx();
}
#endif
diff --git a/arch/powerpc/mm/pgtable_64.c b/arch/powerpc/mm/pgtable_64.c
index fb1375c07e8c..bfa18453625e 100644
--- a/arch/powerpc/mm/pgtable_64.c
+++ b/arch/powerpc/mm/pgtable_64.c
@@ -328,6 +328,9 @@ void mark_rodata_ro(void)
radix__mark_rodata_ro();
else
hash__mark_rodata_ro();
+
+ // mark_initmem_nx() should have already run by now
+ ptdump_check_wx();
}
void mark_initmem_nx(void)
diff --git a/arch/powerpc/mm/ptdump/ptdump.c b/arch/powerpc/mm/ptdump/ptdump.c
index a4a132f92810..e69b53a8a841 100644
--- a/arch/powerpc/mm/ptdump/ptdump.c
+++ b/arch/powerpc/mm/ptdump/ptdump.c
@@ -31,7 +31,7 @@
#include "ptdump.h"
#ifdef CONFIG_PPC32
-#define KERN_VIRT_START 0
+#define KERN_VIRT_START PAGE_OFFSET
#endif
/*
@@ -68,6 +68,8 @@ struct pg_state {
unsigned long last_pa;
unsigned int level;
u64 current_flags;
+ bool check_wx;
+ unsigned long wx_pages;
};
struct addr_marker {
@@ -177,6 +179,20 @@ static void dump_addr(struct pg_state *st, unsigned long addr)
}
+static void note_prot_wx(struct pg_state *st, unsigned long addr)
+{
+ if (!st->check_wx)
+ return;
+
+ if (!((st->current_flags & pgprot_val(PAGE_KERNEL_X)) == pgprot_val(PAGE_KERNEL_X)))
+ return;
+
+ WARN_ONCE(1, "powerpc/mm: Found insecure W+X mapping at address %p/%pS\n",
+ (void *)st->start_address, (void *)st->start_address);
+
+ st->wx_pages += (addr - st->start_address) / PAGE_SIZE;
+}
+
static void note_page(struct pg_state *st, unsigned long addr,
unsigned int level, u64 val)
{
@@ -206,6 +222,7 @@ static void note_page(struct pg_state *st, unsigned long addr,
/* Check the PTE flags */
if (st->current_flags) {
+ note_prot_wx(st, addr);
dump_addr(st, addr);
/* Dump all the flags */
@@ -378,6 +395,28 @@ static void build_pgtable_complete_mask(void)
pg_level[i].mask |= pg_level[i].flag[j].mask;
}
+void ptdump_check_wx(void)
+{
+ struct pg_state st = {
+ .seq = NULL,
+ .marker = address_markers,
+ .check_wx = true,
+ };
+
+ if (radix_enabled())
+ st.start_address = PAGE_OFFSET;
+ else
+ st.start_address = KERN_VIRT_START;
+
+ walk_pagetables(&st);
+
+ if (st.wx_pages)
+ pr_warn("Checked W+X mappings: FAILED, %lu W+X pages found\n",
+ st.wx_pages);
+ else
+ pr_info("Checked W+X mappings: passed, no W+X pages found\n");
+}
+
static int ptdump_init(void)
{
struct dentry *debugfs_file;
--
2.21.0
^ permalink raw reply related
* RE: [PATCH v2 9/9] dpaa_eth: fix SG frame cleanup
From: Laurentiu Tudor @ 2019-05-02 9:05 UTC (permalink / raw)
To: jocke@infinera.com, netdev@vger.kernel.org,
Madalin-cristian Bucur, Leo Li, Roy Pledge,
Camelia Alexandra Groza
Cc: iommu@lists.linux-foundation.org, linuxppc-dev@lists.ozlabs.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, davem@davemloft.net
In-Reply-To: <2c6f5d170edab346e0a87b1dfeb12e2f65801685.camel@infinera.com>
Hi Joakim,
> -----Original Message-----
> From: Joakim Tjernlund <Joakim.Tjernlund@infinera.com>
> Sent: Saturday, April 27, 2019 8:11 PM
>
> On Sat, 2019-04-27 at 10:10 +0300, laurentiu.tudor@nxp.com wrote:
> > From: Laurentiu Tudor <laurentiu.tudor@nxp.com>
> >
> > Fix issue with the entry indexing in the sg frame cleanup code being
> > off-by-1. This problem showed up when doing some basic iperf tests and
> > manifested in traffic coming to a halt.
> >
> > Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
> > Acked-by: Madalin Bucur <madalin.bucur@nxp.com>
>
> Wasn't this a stable candidate too?
Yes, it is. I forgot to add the cc:stable tag, sorry about that.
---
Best Regards, Laurentiu
> > ---
> > drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > index daede7272768..40420edc9ce6 100644
> > --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > @@ -1663,7 +1663,7 @@ static struct sk_buff *dpaa_cleanup_tx_fd(const
> struct dpaa_priv *priv,
> > qm_sg_entry_get_len(&sgt[0]), dma_dir);
> >
> > /* remaining pages were mapped with skb_frag_dma_map()
> */
> > - for (i = 1; i < nr_frags; i++) {
> > + for (i = 1; i <= nr_frags; i++) {
> > WARN_ON(qm_sg_entry_is_ext(&sgt[i]));
> >
> > dma_unmap_page(dev, qm_sg_addr(&sgt[i]),
> > --
> > 2.17.1
> >
^ permalink raw reply
* RE: [PATCH v2 7/9] dpaa_eth: fix iova handling for contiguous frames
From: Laurentiu Tudor @ 2019-05-02 9:10 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Madalin-cristian Bucur, netdev@vger.kernel.org, Roy Pledge,
linux-kernel@vger.kernel.org, Leo Li,
iommu@lists.linux-foundation.org, Camelia Alexandra Groza,
linuxppc-dev@lists.ozlabs.org, davem@davemloft.net,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190427164612.GA12450@infradead.org>
> -----Original Message-----
> From: Christoph Hellwig <hch@infradead.org>
> Sent: Saturday, April 27, 2019 7:46 PM
>
> On Sat, Apr 27, 2019 at 10:10:29AM +0300, laurentiu.tudor@nxp.com wrote:
> > From: Laurentiu Tudor <laurentiu.tudor@nxp.com>
> >
> > The driver relies on the no longer valid assumption that dma addresses
> > (iovas) are identical to physical addressees and uses phys_to_virt() to
> > make iova -> vaddr conversions. Fix this by adding a function that does
> > proper iova -> phys conversions using the iommu api and update the code
> > to use it.
> > Also, a dma_unmap_single() call had to be moved further down the code
> > because iova -> vaddr conversions were required before the unmap.
> > For now only the contiguous frame case is handled and the SG case is
> > split in a following patch.
> > While at it, clean-up a redundant dpaa_bpid2pool() and pass the bp
> > as parameter.
>
> Err, this is broken. A driver using the DMA API has no business
> call IOMMU APIs. Just save the _virtual_ address used for the mapping
> away and use that again. We should not go through crazy gymnastics
> like this.
I think that due to the particularity of this hardware we don't have a way of saving the VA, but I'd let my colleagues maintaining this driver to comment more on why we need to do this.
---
Best Regards, Laurentiu
^ permalink raw reply
* RE: [EXT] Re: [PATCH V4] ASoC: fsl_esai: Add pm runtime function
From: S.j. Wang @ 2019-05-02 9:13 UTC (permalink / raw)
To: Mark Brown
Cc: alsa-devel@alsa-project.org, timur@kernel.org,
Xiubo.Lee@gmail.com, festevam@gmail.com,
linux-kernel@vger.kernel.org, nicoleotsuka@gmail.com,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20190502023945.GA19532@sirena.org.uk>
Hi Mark
> On Sun, Apr 28, 2019 at 02:24:54AM +0000, S.j. Wang wrote:
> > Add pm runtime support and move clock handling there.
> > Close the clocks at suspend to reduce the power consumption.
> >
> > fsl_esai_suspend is replaced by pm_runtime_force_suspend.
> > fsl_esai_resume is replaced by pm_runtime_force_resume.
>
> This doesn't apply against for-5.2 again. Sorry about this, I think this one is
> due to some messups with my scripts which caused some patches to be
> dropped for a while (and it's likely to be what happened the last time as
> well). Can you check and resend again please? Like I say sorry about this, I
> think it's my mistake.
I am checking, but I don't know why this patch failed in your side. I
Tried to apply this patch on for-5.1, for 5.2, for-linus and for-next, all are
Successful. The git is git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git.
I can't reproduce your problem. Is there any my operation wrong?
Best regards
Wang shengjiu
^ permalink raw reply
* [PATCH][next] KVM: PPC: Book3S HV: XIVE: fix spelling mistake "acessing" -> "accessing"
From: Colin King @ 2019-05-02 10:23 UTC (permalink / raw)
To: Paul Mackerras, Benjamin Herrenschmidt, Michael Ellerman, kvm-ppc,
linuxppc-dev
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
There is a spelling mistake in a pr_err message, fix it.
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
arch/powerpc/kvm/book3s_xive_native.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kvm/book3s_xive_native.c b/arch/powerpc/kvm/book3s_xive_native.c
index 5e14df1a4403..6a8e698c4b6e 100644
--- a/arch/powerpc/kvm/book3s_xive_native.c
+++ b/arch/powerpc/kvm/book3s_xive_native.c
@@ -235,7 +235,7 @@ static vm_fault_t xive_native_esb_fault(struct vm_fault *vmf)
arch_spin_unlock(&sb->lock);
if (WARN_ON(!page)) {
- pr_err("%s: acessing invalid ESB page for source %lx !\n",
+ pr_err("%s: accessing invalid ESB page for source %lx !\n",
__func__, irq);
return VM_FAULT_SIGBUS;
}
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v2 9/9] dpaa_eth: fix SG frame cleanup
From: Joakim Tjernlund @ 2019-05-02 10:36 UTC (permalink / raw)
To: netdev@vger.kernel.org, madalin.bucur@nxp.com, leoyang.li@nxp.com,
laurentiu.tudor@nxp.com, roy.pledge@nxp.com,
camelia.groza@nxp.com
Cc: iommu@lists.linux-foundation.org, linuxppc-dev@lists.ozlabs.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, davem@davemloft.net
In-Reply-To: <VI1PR04MB5134C0D6707E78D674B96898EC340@VI1PR04MB5134.eurprd04.prod.outlook.com>
On Thu, 2019-05-02 at 09:05 +0000, Laurentiu Tudor wrote:
> Hi Joakim,
>
> > -----Original Message-----
> > From: Joakim Tjernlund <Joakim.Tjernlund@infinera.com>
> > Sent: Saturday, April 27, 2019 8:11 PM
> >
> > On Sat, 2019-04-27 at 10:10 +0300, laurentiu.tudor@nxp.com wrote:
> > > From: Laurentiu Tudor <laurentiu.tudor@nxp.com>
> > >
> > > Fix issue with the entry indexing in the sg frame cleanup code being
> > > off-by-1. This problem showed up when doing some basic iperf tests and
> > > manifested in traffic coming to a halt.
> > >
> > > Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
> > > Acked-by: Madalin Bucur <madalin.bucur@nxp.com>
> >
> > Wasn't this a stable candidate too?
>
> Yes, it is. I forgot to add the cc:stable tag, sorry about that.
Then this is a bug fix that should go directly to linus/stable.
I note that https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/log/drivers/net/ethernet/freescale/dpaa?h=linux-4.19.y
is in 4.19 but not in 4.14 , is it not appropriate for 4.14?
Jocke
>
> ---
> Best Regards, Laurentiu
>
> > > ---
> > > drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > > index daede7272768..40420edc9ce6 100644
> > > --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > > +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > > @@ -1663,7 +1663,7 @@ static struct sk_buff *dpaa_cleanup_tx_fd(const
> > struct dpaa_priv *priv,
> > > qm_sg_entry_get_len(&sgt[0]), dma_dir);
> > >
> > > /* remaining pages were mapped with skb_frag_dma_map()
> > */
> > > - for (i = 1; i < nr_frags; i++) {
> > > + for (i = 1; i <= nr_frags; i++) {
> > > WARN_ON(qm_sg_entry_is_ext(&sgt[i]));
> > >
> > > dma_unmap_page(dev, qm_sg_addr(&sgt[i]),
> > > --
> > > 2.17.1
> > >
^ permalink raw reply
* Re: [PATCH] crypto: caam/jr - Remove extra memory barrier during job ring dequeue
From: Horia Geanta @ 2019-05-02 11:08 UTC (permalink / raw)
To: Michael Ellerman, Vakul Garg
Cc: herbert@gondor.apana.org.au, Aymen Sghaier,
linuxppc-dev@lists.ozlabs.org, linux-crypto@vger.kernel.org,
davem@davemloft.net
In-Reply-To: <87pnp2aflz.fsf@concordia.ellerman.id.au>
On 5/1/2019 8:49 AM, Michael Ellerman wrote:
> Vakul Garg wrote:
>> In function caam_jr_dequeue(), a full memory barrier is used before
>> writing response job ring's register to signal removal of the completed
>> job. Therefore for writing the register, we do not need another write
>> memory barrier. Hence it is removed by replacing the call to wr_reg32()
>> with a newly defined function wr_reg32_relaxed().
>>
>> Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
>> ---
>> drivers/crypto/caam/jr.c | 2 +-
>> drivers/crypto/caam/regs.h | 8 ++++++++
>> 2 files changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c
>> index 4e9b3fca5627..2ce6d7d2ad72 100644
>> --- a/drivers/crypto/caam/jr.c
>> +++ b/drivers/crypto/caam/jr.c
>> @@ -266,7 +266,7 @@ static void caam_jr_dequeue(unsigned long devarg)
>> mb();
>>
>> /* set done */
>> - wr_reg32(&jrp->rregs->outring_rmvd, 1);
>> + wr_reg32_relaxed(&jrp->rregs->outring_rmvd, 1);
>>
>> jrp->out_ring_read_index = (jrp->out_ring_read_index + 1) &
>> (JOBR_DEPTH - 1);
>> diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h
>> index 3cd0822ea819..9e912c722e33 100644
>> --- a/drivers/crypto/caam/regs.h
>> +++ b/drivers/crypto/caam/regs.h
>> @@ -96,6 +96,14 @@ cpu_to_caam(16)
>> cpu_to_caam(32)
>> cpu_to_caam(64)
>>
>> +static inline void wr_reg32_relaxed(void __iomem *reg, u32 data)
>> +{
>> + if (caam_little_end)
>> + writel_relaxed(data, reg);
>> + else
>> + writel_relaxed(cpu_to_be32(data), reg);
>> +}
When both core (PPC) and crypto engine (caam) are big endian, data ends up being
swapped - which is incorrect:
writel_relaxed -> writel -> __do_writel -> out_le32 -> swap
cpu_to_be32(data) -> data
>> +
>> static inline void wr_reg32(void __iomem *reg, u32 data)
>> {
>> if (caam_little_end)
>
> This crashes on my p5020ds. Did you test on powerpc?
>
> # first bad commit: [bbfcac5ff5f26aafa51935a62eb86b6eacfe8a49] crypto: caam/jr - Remove extra memory barrier during job ring dequeue
Thanks for the report Michael.
Any hint what would be the proper approach here - to have relaxed I/O accessors
that would work both for ARM and PPC, and avoid ifdeffery etc.?
For non-relaxed version, we used iowriteXX and iowriteXXbe - which work fine on
ARM and PPC, covering all the endianness combinations (core + crypto engine):
static inline void wr_reg32(void __iomem *reg, u32 data)
{
if (caam_little_end)
iowrite32(data, reg);
else
iowrite32be(data, reg);
}
Thanks,
Horia
^ permalink raw reply
* Re: [PATCH v1 2/4] powerpc/mm: Move book3s64 specifics in subdirectory mm/book3s64
From: Michael Ellerman @ 2019-05-02 11:14 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <7dbe8476-bfa3-29ac-5155-a67823d39ef4@c-s.fr>
Christophe Leroy <christophe.leroy@c-s.fr> writes:
> Le 02/05/2019 à 09:11, Michael Ellerman a écrit :
>> Christophe Leroy <christophe.leroy@c-s.fr> writes:
>>
>>> Many files in arch/powerpc/mm are only for book3S64. This patch
>>> creates a subdirectory for them.
>>>
>>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>>> ---
>>> arch/powerpc/mm/Makefile | 25 +++----------------
>>> arch/powerpc/mm/book3s64/Makefile | 28 ++++++++++++++++++++++
>>> arch/powerpc/mm/{ => book3s64}/hash64_4k.c | 0
>>> arch/powerpc/mm/{ => book3s64}/hash64_64k.c | 0
>>> arch/powerpc/mm/{ => book3s64}/hash_native_64.c | 0
>>> arch/powerpc/mm/{ => book3s64}/hash_utils_64.c | 0
>>> arch/powerpc/mm/{ => book3s64}/hugepage-hash64.c | 0
>>> .../powerpc/mm/{ => book3s64}/hugetlbpage-hash64.c | 0
>>> arch/powerpc/mm/{ => book3s64}/hugetlbpage-radix.c | 0
>>> .../mm/{ => book3s64}/mmu_context_book3s64.c | 0
>>> arch/powerpc/mm/{ => book3s64}/mmu_context_iommu.c | 0
>>> arch/powerpc/mm/{ => book3s64}/pgtable-book3s64.c | 0
>>> arch/powerpc/mm/{ => book3s64}/pgtable-hash64.c | 0
>>> arch/powerpc/mm/{ => book3s64}/pgtable-radix.c | 0
>>> arch/powerpc/mm/{ => book3s64}/pkeys.c | 0
>>> arch/powerpc/mm/{ => book3s64}/slb.c | 0
>>> arch/powerpc/mm/{ => book3s64}/subpage-prot.c | 0
>>> arch/powerpc/mm/{ => book3s64}/tlb-radix.c | 0
>>> arch/powerpc/mm/{ => book3s64}/tlb_hash64.c | 0
>>> arch/powerpc/mm/{ => book3s64}/vphn.c | 0
>>> arch/powerpc/mm/{ => book3s64}/vphn.h | 0
>>> arch/powerpc/mm/numa.c | 2 +-
>>> 22 files changed, 32 insertions(+), 23 deletions(-)
>>> create mode 100644 arch/powerpc/mm/book3s64/Makefile
>>> rename arch/powerpc/mm/{ => book3s64}/hash64_4k.c (100%)
>>> rename arch/powerpc/mm/{ => book3s64}/hash64_64k.c (100%)
>>> rename arch/powerpc/mm/{ => book3s64}/hash_native_64.c (100%)
>>> rename arch/powerpc/mm/{ => book3s64}/hash_utils_64.c (100%)
>>> rename arch/powerpc/mm/{ => book3s64}/hugepage-hash64.c (100%)
>>> rename arch/powerpc/mm/{ => book3s64}/hugetlbpage-hash64.c (100%)
>>> rename arch/powerpc/mm/{ => book3s64}/hugetlbpage-radix.c (100%)
>>> rename arch/powerpc/mm/{ => book3s64}/mmu_context_book3s64.c (100%)
>>> rename arch/powerpc/mm/{ => book3s64}/mmu_context_iommu.c (100%)
>>> rename arch/powerpc/mm/{ => book3s64}/pgtable-book3s64.c (100%)
>>> rename arch/powerpc/mm/{ => book3s64}/pgtable-hash64.c (100%)
>>> rename arch/powerpc/mm/{ => book3s64}/pgtable-radix.c (100%)
>>> rename arch/powerpc/mm/{ => book3s64}/pkeys.c (100%)
>>> rename arch/powerpc/mm/{ => book3s64}/slb.c (100%)
>>> rename arch/powerpc/mm/{ => book3s64}/subpage-prot.c (100%)
>>> rename arch/powerpc/mm/{ => book3s64}/tlb-radix.c (100%)
>>> rename arch/powerpc/mm/{ => book3s64}/tlb_hash64.c (100%)
>>
>> Do you mind if I take this but rework the destination names in the process?
>
> I don't mind, I think it's a good idea.
>
>>
>> I don't like having eg. book3s64/pgtable-book3s64.c
>>
>> And some of the other names could use a bit of cleanup too.
>>
>> What about:
>>
>> arch/powerpc/mm/{hash64_4k.c => book3s64/hash_4k.c}
>> arch/powerpc/mm/{hash64_64k.c => book3s64/hash_64k.c}
>> arch/powerpc/mm/{hugepage-hash64.c => book3s64/hash_hugepage.c}
>> arch/powerpc/mm/{hugetlbpage-hash64.c => book3s64/hash_hugetlbpage.c}
>> arch/powerpc/mm/{hash_native_64.c => book3s64/hash_native.c}
>> arch/powerpc/mm/{pgtable-hash64.c => book3s64/hash_pgtable.c}
>> arch/powerpc/mm/{tlb_hash64.c => book3s64/hash_tlb.c}
>> arch/powerpc/mm/{hash_utils_64.c => book3s64/hash_utils.c}
>> arch/powerpc/mm/{mmu_context_iommu.c => book3s64/iommu_api.c}
>> arch/powerpc/mm/{mmu_context_book3s64.c => book3s64/mmu_context.c}
>> arch/powerpc/mm/{pgtable-book3s64.c => book3s64/pgtable.c}
>> arch/powerpc/mm/{hugetlbpage-radix.c => book3s64/radix_hugetlbpage.c}
>> arch/powerpc/mm/{pgtable-radix.c => book3s64/radix_pgtable.c}
>> arch/powerpc/mm/{tlb-radix.c => book3s64/radix_tlb.c}
>
> Looks good
Thanks. I'll do something similar for 32-bit & nohash.
cheers
^ permalink raw reply
* Re: [PATCH v1 3/4] powerpc/mm: Move book3s32 specifics in subdirectory mm/book3s64
From: Michael Ellerman @ 2019-05-02 11:32 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <12c1ba4fc9e2e55ca44c5c57225669b296d48c74.1553853405.git.christophe.leroy@c-s.fr>
Christophe Leroy <christophe.leroy@c-s.fr> writes:
> Several files in arch/powerpc/mm are only for book3S32. This patch
> creates a subdirectory for them.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> arch/powerpc/mm/Makefile | 3 +--
> arch/powerpc/mm/book3s32/Makefile | 6 ++++++
> arch/powerpc/mm/{ => book3s32}/hash_low_32.S | 0
> arch/powerpc/mm/{ => book3s32}/mmu_context_hash32.c | 0
> arch/powerpc/mm/{ => book3s32}/ppc_mmu_32.c | 0
> arch/powerpc/mm/{ => book3s32}/tlb_hash32.c | 0
> 6 files changed, 7 insertions(+), 2 deletions(-)
> create mode 100644 arch/powerpc/mm/book3s32/Makefile
> rename arch/powerpc/mm/{ => book3s32}/hash_low_32.S (100%)
> rename arch/powerpc/mm/{ => book3s32}/mmu_context_hash32.c (100%)
> rename arch/powerpc/mm/{ => book3s32}/ppc_mmu_32.c (100%)
> rename arch/powerpc/mm/{ => book3s32}/tlb_hash32.c (100%)
I shortened them to:
arch/powerpc/mm/{hash_low_32.S => book3s32/hash_low.S}
arch/powerpc/mm/{ppc_mmu_32.c => book3s32/mmu.c}
arch/powerpc/mm/{mmu_context_hash32.c => book3s32/mmu_context.c}
arch/powerpc/mm/{tlb_hash32.c => book3s32/tlb.c}
cheers
^ permalink raw reply
* Re: [PATCH v1 4/4] powerpc/mm: Move nohash specifics in subdirectory mm/nohash
From: Michael Ellerman @ 2019-05-02 11:38 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <a87606761d285e4f3dee5c2de1e6691f04e429b6.1553853405.git.christophe.leroy@c-s.fr>
Christophe Leroy <christophe.leroy@c-s.fr> writes:
> Many files in arch/powerpc/mm are only for nohash. This patch
> creates a subdirectory for them.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> arch/powerpc/mm/Makefile | 17 +----------------
> arch/powerpc/mm/{ => nohash}/40x_mmu.c | 0
> arch/powerpc/mm/{ => nohash}/44x_mmu.c | 0
> arch/powerpc/mm/{ => nohash}/8xx_mmu.c | 0
> arch/powerpc/mm/nohash/Makefile | 21 +++++++++++++++++++++
> arch/powerpc/mm/{ => nohash}/fsl_booke_mmu.c | 0
> arch/powerpc/mm/{ => nohash}/hugetlbpage-book3e.c | 0
> arch/powerpc/mm/{ => nohash}/mmu_context_nohash.c | 0
> arch/powerpc/mm/{ => nohash}/pgtable-book3e.c | 0
> arch/powerpc/mm/{ => nohash}/tlb_low_64e.S | 0
> arch/powerpc/mm/{ => nohash}/tlb_nohash.c | 0
> arch/powerpc/mm/{ => nohash}/tlb_nohash_low.S | 0
> 12 files changed, 22 insertions(+), 16 deletions(-)
> rename arch/powerpc/mm/{ => nohash}/40x_mmu.c (100%)
> rename arch/powerpc/mm/{ => nohash}/44x_mmu.c (100%)
> rename arch/powerpc/mm/{ => nohash}/8xx_mmu.c (100%)
> create mode 100644 arch/powerpc/mm/nohash/Makefile
> rename arch/powerpc/mm/{ => nohash}/fsl_booke_mmu.c (100%)
> rename arch/powerpc/mm/{ => nohash}/hugetlbpage-book3e.c (100%)
> rename arch/powerpc/mm/{ => nohash}/mmu_context_nohash.c (100%)
> rename arch/powerpc/mm/{ => nohash}/pgtable-book3e.c (100%)
> rename arch/powerpc/mm/{ => nohash}/tlb_low_64e.S (100%)
> rename arch/powerpc/mm/{ => nohash}/tlb_nohash.c (100%)
> rename arch/powerpc/mm/{ => nohash}/tlb_nohash_low.S (100%)
I went with:
arch/powerpc/mm/{40x_mmu.c => nohash/40x.c}
arch/powerpc/mm/{44x_mmu.c => nohash/44x.c}
arch/powerpc/mm/{8xx_mmu.c => nohash/8xx.c}
arch/powerpc/mm/{hugetlbpage-book3e.c => nohash/book3e_hugetlbpage.c}
arch/powerpc/mm/{pgtable-book3e.c => nohash/book3e_pgtable.c}
arch/powerpc/mm/{fsl_booke_mmu.c => nohash/fsl_booke.c}
arch/powerpc/mm/{mmu_context_nohash.c => nohash/mmu_context.c}
arch/powerpc/mm/{tlb_nohash.c => nohash/tlb.c}
arch/powerpc/mm/{tlb_nohash_low.S => nohash/tlb_low.S}
arch/powerpc/mm/{ => nohash}/tlb_low_64e.S
cheers
^ permalink raw reply
* Re: [PATCH v1 3/4] powerpc/mm: Move book3s32 specifics in subdirectory mm/book3s64
From: Christophe Leroy @ 2019-05-02 11:43 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <87tvedxfa6.fsf@concordia.ellerman.id.au>
Le 02/05/2019 à 13:32, Michael Ellerman a écrit :
> Christophe Leroy <christophe.leroy@c-s.fr> writes:
>
>> Several files in arch/powerpc/mm are only for book3S32. This patch
>> creates a subdirectory for them.
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>> ---
>> arch/powerpc/mm/Makefile | 3 +--
>> arch/powerpc/mm/book3s32/Makefile | 6 ++++++
>> arch/powerpc/mm/{ => book3s32}/hash_low_32.S | 0
>> arch/powerpc/mm/{ => book3s32}/mmu_context_hash32.c | 0
>> arch/powerpc/mm/{ => book3s32}/ppc_mmu_32.c | 0
>> arch/powerpc/mm/{ => book3s32}/tlb_hash32.c | 0
>> 6 files changed, 7 insertions(+), 2 deletions(-)
>> create mode 100644 arch/powerpc/mm/book3s32/Makefile
>> rename arch/powerpc/mm/{ => book3s32}/hash_low_32.S (100%)
>> rename arch/powerpc/mm/{ => book3s32}/mmu_context_hash32.c (100%)
>> rename arch/powerpc/mm/{ => book3s32}/ppc_mmu_32.c (100%)
>> rename arch/powerpc/mm/{ => book3s32}/tlb_hash32.c (100%)
>
> I shortened them to:
>
> arch/powerpc/mm/{hash_low_32.S => book3s32/hash_low.S}
> arch/powerpc/mm/{ppc_mmu_32.c => book3s32/mmu.c}
To be consistent with what you did in nohash/ dir, shouldn't we rename
the above 'ppc.c' or 'ppc_32.c' instead of 'mmu.c' ?
Christophe
> arch/powerpc/mm/{mmu_context_hash32.c => book3s32/mmu_context.c}
> arch/powerpc/mm/{tlb_hash32.c => book3s32/tlb.c}
>
> cheers
>
^ permalink raw reply
* Re: [PATCH v2 01/17] powerpc/mm: Don't BUG() in hugepd_page()
From: Michael Ellerman @ 2019-05-02 12:02 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
aneesh.kumar
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <ff4366d14b3ef4de6af835a880a772477577139f.1556258135.git.christophe.leroy@c-s.fr>
Christophe Leroy <christophe.leroy@c-s.fr> writes:
> Use VM_BUG_ON() instead of BUG_ON(), as those BUG_ON()
> are not there to catch runtime errors but to catch errors
> during development cycle only.
I've dropped this one and the next, because I don't like VM_BUG_ON().
Why not? Because it's contradictory. It's a condition that's so
important that we should BUG, but only if the kernel has been built
specially for debugging.
I don't really buy the development cycle distinction, it's not like we
have a rigorous test suite that we run and then we declare everything's
gold and ship a product. We often don't find bugs until they're hit in
the wild.
For example the recent corruption Joel discovered with STRICT_KERNEL_RWX
could have been caught by a BUG_ON() to check we weren't patching kernel
text in radix__change_memory_range(), but he wouldn't have been using
CONFIG_DEBUG_VM. (See 8adddf349fda)
I know Aneesh disagrees with me on this, so maybe you two can convince
me otherwise.
cheers
> diff --git a/arch/powerpc/include/asm/hugetlb.h b/arch/powerpc/include/asm/hugetlb.h
> index 8d40565ad0c3..7f1867e428c0 100644
> --- a/arch/powerpc/include/asm/hugetlb.h
> +++ b/arch/powerpc/include/asm/hugetlb.h
> @@ -14,7 +14,7 @@
> */
> static inline pte_t *hugepd_page(hugepd_t hpd)
> {
> - BUG_ON(!hugepd_ok(hpd));
> + VM_BUG_ON(!hugepd_ok(hpd));
> /*
> * We have only four bits to encode, MMU page size
> */
> @@ -42,7 +42,7 @@ static inline void flush_hugetlb_page(struct vm_area_struct *vma,
>
> static inline pte_t *hugepd_page(hugepd_t hpd)
> {
> - BUG_ON(!hugepd_ok(hpd));
> + VM_BUG_ON(!hugepd_ok(hpd));
> #ifdef CONFIG_PPC_8xx
> return (pte_t *)__va(hpd_val(hpd) & ~HUGEPD_SHIFT_MASK);
> #else
> --
> 2.13.3
^ permalink raw reply
* Re: [PATCH v2 01/17] powerpc/mm: Don't BUG() in hugepd_page()
From: Christophe Leroy @ 2019-05-02 12:11 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
aneesh.kumar
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <87o94lxdxe.fsf@concordia.ellerman.id.au>
Le 02/05/2019 à 14:02, Michael Ellerman a écrit :
> Christophe Leroy <christophe.leroy@c-s.fr> writes:
>> Use VM_BUG_ON() instead of BUG_ON(), as those BUG_ON()
>> are not there to catch runtime errors but to catch errors
>> during development cycle only.
>
> I've dropped this one and the next, because I don't like VM_BUG_ON().
>
> Why not? Because it's contradictory. It's a condition that's so
> important that we should BUG, but only if the kernel has been built
> specially for debugging.
>
> I don't really buy the development cycle distinction, it's not like we
> have a rigorous test suite that we run and then we declare everything's
> gold and ship a product. We often don't find bugs until they're hit in
> the wild.
>
> For example the recent corruption Joel discovered with STRICT_KERNEL_RWX
> could have been caught by a BUG_ON() to check we weren't patching kernel
> text in radix__change_memory_range(), but he wouldn't have been using
> CONFIG_DEBUG_VM. (See 8adddf349fda)
>
> I know Aneesh disagrees with me on this, so maybe you two can convince
> me otherwise.
>
I have no strong oppinion about this. In v1, I replaced them with a
WARN_ON(), and Aneesh suggested to go with VM_BUG_ON() instead.
My main purpose was to reduce the amount of BUG/BUG_ON and I thought
those were good candidates, but if you prefer keeping the BUG(), that's
ok for me. Or maybe you prefered v1 alternatives (series at
https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=98170) ?
Christophe
^ permalink raw reply
* Re: Linux 5.1-rc5
From: Greg KH @ 2019-05-02 12:21 UTC (permalink / raw)
To: Linus Torvalds
Cc: linux-s390, Linux List Kernel Mailing, Christoph Hellwig,
Martin Schwidefsky, linuxppc-dev
In-Reply-To: <CAHk-=wj7jgMOVFW0tiU-X+zhg6+Rn7mEBTej+f26rV3zXezOSA@mail.gmail.com>
On Mon, Apr 15, 2019 at 09:17:10AM -0700, Linus Torvalds wrote:
> On Sun, Apr 14, 2019 at 10:19 PM Christoph Hellwig <hch@infradead.org> wrote:
> >
> > Can we please have the page refcount overflow fixes out on the list
> > for review, even if it is after the fact?
>
> They were actually on a list for review long before the fact, but it
> was the security mailing list. The issue actually got discussed back
> in January along with early versions of the patches, but then we
> dropped the ball because it just wasn't on anybody's radar and it got
> resurrected late March. Willy wrote a rather bigger patch-series, and
> review of that is what then resulted in those commits. So they may
> look recent, but that's just because the original patches got
> seriously edited down and rewritten.
>
> That said, powerpc and s390 should at least look at maybe adding a
> check for the page ref in their gup paths too. Powerpc has the special
> gup_hugepte() case, and s390 has its own version of gup entirely. I
> was actually hoping the s390 guys would look at using the generic gup
> code.
>
> I ruthlessly also entirely ignored MIPS, SH and sparc, since they seem
> largely irrelevant, partly since even theoretically this whole issue
> needs a _lot_ of memory.
>
> Michael, Martin, see commit 6b3a70773630 ("Merge branch 'page-refs'
> (page ref overflow)"). You may or may not really care.
I've now queued these patches up for the next round of stable releases,
as some people seem to care about these.
I didn't see any follow-on patches for s390 or ppc64 hit the tree for
these changes, am I just missing them and should also queue up a few
more to handle this issue on those platforms?
thanks,
greg k-h
^ permalink raw reply
* RE: [PATCH v2 9/9] dpaa_eth: fix SG frame cleanup
From: Laurentiu Tudor @ 2019-05-02 12:58 UTC (permalink / raw)
To: jocke@infinera.com, netdev@vger.kernel.org,
Madalin-cristian Bucur, Leo Li, Roy Pledge,
Camelia Alexandra Groza
Cc: iommu@lists.linux-foundation.org, linuxppc-dev@lists.ozlabs.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, davem@davemloft.net
In-Reply-To: <728fe477849debcc14bb1af01e35bc7b184a0a03.camel@infinera.com>
> -----Original Message-----
> From: Joakim Tjernlund <Joakim.Tjernlund@infinera.com>
> Sent: Thursday, May 2, 2019 1:37 PM
>
> On Thu, 2019-05-02 at 09:05 +0000, Laurentiu Tudor wrote:
> > Hi Joakim,
> >
> > > -----Original Message-----
> > > From: Joakim Tjernlund <Joakim.Tjernlund@infinera.com>
> > > Sent: Saturday, April 27, 2019 8:11 PM
> > >
> > > On Sat, 2019-04-27 at 10:10 +0300, laurentiu.tudor@nxp.com wrote:
> > > > From: Laurentiu Tudor <laurentiu.tudor@nxp.com>
> > > >
> > > > Fix issue with the entry indexing in the sg frame cleanup code being
> > > > off-by-1. This problem showed up when doing some basic iperf tests
> and
> > > > manifested in traffic coming to a halt.
> > > >
> > > > Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
> > > > Acked-by: Madalin Bucur <madalin.bucur@nxp.com>
> > >
> > > Wasn't this a stable candidate too?
> >
> > Yes, it is. I forgot to add the cc:stable tag, sorry about that.
>
> Then this is a bug fix that should go directly to linus/stable.
>
> I note that
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/log/drivers/net/ethernet/freescale/dpaa?h=linux-4.19.y
Not sure I understand ... I don't see the patch in the link.
> is in 4.19 but not in 4.14 , is it not appropriate for 4.14?
I think it makes sense to go in both stable trees.
---
Best Regards, Laurentiu
> >
> > > > ---
> > > > drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > > b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > > > index daede7272768..40420edc9ce6 100644
> > > > --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > > > +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > > > @@ -1663,7 +1663,7 @@ static struct sk_buff
> *dpaa_cleanup_tx_fd(const
> > > struct dpaa_priv *priv,
> > > > qm_sg_entry_get_len(&sgt[0]),
> dma_dir);
> > > >
> > > > /* remaining pages were mapped with
> skb_frag_dma_map()
> > > */
> > > > - for (i = 1; i < nr_frags; i++) {
> > > > + for (i = 1; i <= nr_frags; i++) {
> > > > WARN_ON(qm_sg_entry_is_ext(&sgt[i]));
> > > >
> > > > dma_unmap_page(dev, qm_sg_addr(&sgt[i]),
> > > > --
> > > > 2.17.1
> > > >
^ permalink raw reply
* Re: [PATCH 1/2] x86, numa: always initialize all possible nodes
From: Michal Hocko @ 2019-05-02 13:00 UTC (permalink / raw)
To: Barret Rhoden
Cc: Tony Luck, linux-ia64, Dave Hansen, Peter Zijlstra, x86, LKML,
Pingfan Liu, linux-mm, Ingo Molnar, linuxppc-dev
In-Reply-To: <34f96661-41c2-27cc-422d-5a7aab526f87@google.com>
On Wed 01-05-19 15:12:32, Barret Rhoden wrote:
[...]
> A more elegant solution may be to avoid registering with sysfs during early
> boot, or something else entirely. But I figured I'd ask for help at this
> point. =)
Thanks for the report and an excellent analysis! This is really helpful.
I will think about this some more but I am traveling this week. It seems
really awkward to register a sysfs file for an empty range. That looks
like a bug to me.
--
Michal Hocko
SUSE Labs
^ 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