* [PATCH v2 00/12] powerpc/64: machine check and system reset fixes
From: Nicholas Piggin @ 2020-03-25 10:33 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Mahesh Salgaonkar, Ganesh Goudar, Nicholas Piggin
There's a bunch of problems we hit bringing up fwnmi sreset and testing
with mce injection on QEMU. Mostly pseries issues.
This series of fixes applies on top of next-test, the machine
check reconcile patch won't apply cleanly to previous kernels but
it might want to be backported. We can do that after upstreaming.
This doesn't solve all known problems yet, but fwnmi machine check
and system reset injection in QEMU is significantly better. There
will be more to come but these should be ready for review now.
Thanks,
Nick
Nicholas Piggin (12):
powerpc/64s/exceptions: Fix in_mce accounting in unrecoverable path
powerpc/64s/exceptions: Change irq reconcile for NMIs from reusing
_DAR to RESULT
powerpc/64s/exceptions: machine check reconcile irq state
powerpc/pseries/ras: avoid calling rtas_token in NMI paths
powerpc/pseries/ras: FWNMI_VALID off by one
powerpc/pseries/ras: fwnmi avoid modifying r3 in error case
powerpc/pseries/ras: fwnmi sreset should not interlock
powerpc/pseries: limit machine check stack to 4GB
powerpc/pseries: machine check use rtas_call_unlocked with args on
stack
powerpc/64s: machine check interrupt update NMI accounting
powerpc/64s: machine check do not trace real-mode handler
powerpc/64s: system reset do not trace
arch/powerpc/include/asm/firmware.h | 1 +
arch/powerpc/kernel/exceptions-64s.S | 33 +++++++++++---
arch/powerpc/kernel/mce.c | 13 +++++-
arch/powerpc/kernel/process.c | 2 +-
arch/powerpc/kernel/setup_64.c | 15 ++++++-
arch/powerpc/kernel/traps.c | 18 +++-----
arch/powerpc/platforms/pseries/ras.c | 62 +++++++++++++++++++-------
arch/powerpc/platforms/pseries/setup.c | 13 ++++--
8 files changed, 118 insertions(+), 39 deletions(-)
--
2.23.0
^ permalink raw reply
* Re: [PATCH v4] powerpc: Replace setup_irq() by request_irq()
From: Michael Ellerman @ 2020-03-25 10:22 UTC (permalink / raw)
To: afzal mohammed, Thomas Gleixner
Cc: linux-kernel, Scott Wood, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200324110637.GA5836@afzalpc>
afzal mohammed <afzal.mohd.ma@gmail.com> writes:
> Hi Michael Ellerman,
> On Thu, Mar 12, 2020 at 12:12:55PM +0530, afzal mohammed wrote:
>> request_irq() is preferred over setup_irq(). Invocations of setup_irq()
>> occur after memory allocators are ready.
>>
>> Per tglx[1], setup_irq() existed in olden days when allocators were not
>> ready by the time early interrupts were initialized.
>>
>> Hence replace setup_irq() by request_irq().
>>
>> [1] https://lkml.kernel.org/r/alpine.DEB.2.20.1710191609480.1971@nanos
>>
>> Signed-off-by: afzal mohammed <afzal.mohd.ma@gmail.com>
>
> This patch is seen in next-test branch for last 4-5 days, i don't know
> exactly how powerpc workflow happens, so a question - this would be
> appear in linux-next soon right ? (for last 4-5 days i had been daily
> checking -next, but not appearing there).
Yeah it will appear in next "soon".
It's been stuck behind a big series that has hit some bugs during
testing, so that has delayed me pushing the whole branch.
> Sorry for the query for this trivial patch, i am asking because Thomas
> had mentioned [1] to get setup_irq() cleanup thr' respective
> maintainers (earlier it was part of tree-wide series), check -next after
> -rc6 & resubmit ignored ones to him, this patch is neither in -next,
> neither ignored, so i am at a loss what to do :(
That's OK. I will take this one, you can stop worrying about it.
It should appear in next tomorrow or Friday.
cheers
^ permalink raw reply
* Re: [PATCH] powerpc/prom_init: Include the termination message in ibm, os-term RTAS call
From: Michael Ellerman @ 2020-03-25 10:06 UTC (permalink / raw)
To: Fabiano Rosas, linuxppc-dev; +Cc: linuxram, paulus, kvm-ppc
In-Reply-To: <20200324201211.1055236-1-farosas@linux.ibm.com>
Fabiano Rosas <farosas@linux.ibm.com> writes:
> QEMU can now print the ibm,os-term message[1], so let's include it in
> the RTAS call. E.g.:
>
> qemu-system-ppc64: OS terminated: Switch to secure mode failed.
>
> 1- https://git.qemu.org/?p=qemu.git;a=commitdiff;h=a4c3791ae0
>
> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
> arch/powerpc/kernel/prom_init.c | 3 +++
> 1 file changed, 3 insertions(+)
I have this queued:
https://patchwork.ozlabs.org/patch/1253390/
Which I think does the same thing?
cheers
> diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
> index 577345382b23..d543fb6d29c5 100644
> --- a/arch/powerpc/kernel/prom_init.c
> +++ b/arch/powerpc/kernel/prom_init.c
> @@ -1773,6 +1773,9 @@ static void __init prom_rtas_os_term(char *str)
> if (token == 0)
> prom_panic("Could not get token for ibm,os-term\n");
> os_term_args.token = cpu_to_be32(token);
> + os_term_args.nargs = cpu_to_be32(1);
> + os_term_args.args[0] = cpu_to_be32(__pa(str));
> +
> prom_rtas_hcall((uint64_t)&os_term_args);
> }
> #endif /* CONFIG_PPC_SVM */
> --
> 2.23.0
^ permalink raw reply
* Re: [PATCH 1/4] hugetlbfs: add arch_hugetlb_valid_size
From: Christian Borntraeger @ 2020-03-25 9:38 UTC (permalink / raw)
To: Longpeng (Mike, Cloud Infrastructure Service Product Dept.),
Mike Kravetz, Dave Hansen, linux-mm, linux-kernel,
linux-arm-kernel, linuxppc-dev, linux-riscv, linux-s390,
sparclinux, linux-doc
Cc: Albert Ou, Andrew Morton, Vasily Gorbik, Jonathan Corbet,
Catalin Marinas, Dave Hansen, Heiko Carstens, Ingo Molnar,
Palmer Dabbelt, Paul Walmsley, Paul Mackerras, Thomas Gleixner,
Will Deacon, David S.Miller
In-Reply-To: <5ea6313e-ec4f-a043-632b-ef2901ce2cc9@huawei.com>
On 25.03.20 03:58, Longpeng (Mike, Cloud Infrastructure Service Product Dept.) wrote:
[...]
> Hi Mike,
>
> Inspired by Dave's opinion, it seems the x86-specific hugepages_supported should
> also need to use cpu_feature_enabled instead.
>
> Also, I wonder if the hugepages_supported is correct ? There're two arch
> specific hugepages_supported:
> x86:
> #define hugepages_supported() boot_cpu_has(X86_FEATURE_PSE)
> and
> s390:
> #define hugepages_supported() (MACHINE_HAS_EDAT1)
>
> Is it possible that x86 has X86_FEATURE_GBPAGES but hasn't X86_FEATURE_GBPAGES
> or s390 has MACHINE_HAS_EDAT2 but hasn't MACHINE_HAS_EDAT1 ?
The s390 architecture says that
When EDAT-2 applies, the following function is available in the DAT process:
- EDAT-1 applies.
[..]
So if the machine has EDAT-2 it also has EDAT-1.
^ permalink raw reply
* [PATCH 1/2] Fix 2 "[v3, 25/32] powerpc/64: system call implement entry/exit logic in C"
From: Nicholas Piggin @ 2020-03-25 9:30 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin
This fixes 4 issues caught by TM selftests. First was a tm-syscall bug
that hit due to tabort_syscall being called after interrupts were reconciled
(in a subsequent patch), which led to interrupts being enabled before
tabort_syscall was called. Rather than going through an un-reconciling
interrupts for the return, I just go back to putting the test early in
asm, the C-ification of that wasn't a big win anyway.
Second is the syscall return _TIF_USER_WORK_MASK check would go into an
infinite loop if _TIF_RESTORE_TM became set. The asm code uses
_TIF_USER_WORK_MASK to brach to slowpath which includes restore_tm_state.
Third is system call return was not calling restore_tm_state, I missed
this completely (alhtough it's in the return from interrupt C conversion
because when the asm syscall code encountered problems it would branch
to the interrupt return code.
Fourth is MSR_VEC missing from restore_math, which was caught by
tm-unavailable selftest taking an unexpected facility unavailable
interrupt when testing VSX unavailble exception with MSR.FP=1 MSR.VEC=1.
Fourth case also has a fixup in a subsequent patch.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kernel/entry_64.S | 12 +++++++++---
arch/powerpc/kernel/syscall_64.c | 25 +++++++++++++------------
2 files changed, 22 insertions(+), 15 deletions(-)
diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index dc7fd3196d20..403224acdaa8 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -64,6 +64,12 @@ exception_marker:
.globl system_call_common
system_call_common:
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
+BEGIN_FTR_SECTION
+ extrdi. r10, r12, 1, (63-MSR_TS_T_LG) /* transaction active? */
+ bne .Ltabort_syscall
+END_FTR_SECTION_IFSET(CPU_FTR_TM)
+#endif
_ASM_NOKPROBE_SYMBOL(system_call_common)
mr r10,r1
ld r1,PACAKSAVE(r13)
@@ -179,7 +185,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
b .Lsyscall_restore_regs_cont
#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
-_GLOBAL(tabort_syscall) /* (unsigned long nip, unsigned long msr) */
+.Ltabort_syscall:
/* Firstly we need to enable TM in the kernel */
mfmsr r10
li r9, 1
@@ -199,8 +205,8 @@ _GLOBAL(tabort_syscall) /* (unsigned long nip, unsigned long msr) */
li r9, MSR_RI
andc r10, r10, r9
mtmsrd r10, 1
- mtspr SPRN_SRR0, r3
- mtspr SPRN_SRR1, r4
+ mtspr SPRN_SRR0, r11
+ mtspr SPRN_SRR1, r12
RFI_TO_USER
b . /* prevent speculative execution */
#endif
diff --git a/arch/powerpc/kernel/syscall_64.c b/arch/powerpc/kernel/syscall_64.c
index ffd601d87065..56533a26f3b7 100644
--- a/arch/powerpc/kernel/syscall_64.c
+++ b/arch/powerpc/kernel/syscall_64.c
@@ -15,8 +15,6 @@
#include <asm/time.h>
#include <asm/unistd.h>
-extern void __noreturn tabort_syscall(unsigned long nip, unsigned long msr);
-
typedef long (*syscall_fn)(long, long, long, long, long, long);
/* Has to run notrace because it is entered "unreconciled" */
@@ -32,10 +30,6 @@ notrace long system_call_exception(long r3, long r4, long r5, long r6, long r7,
BUG_ON(!FULL_REGS(regs));
BUG_ON(regs->softe != IRQS_ENABLED);
- if (IS_ENABLED(CONFIG_PPC_TRANSACTIONAL_MEM) &&
- unlikely(regs->msr & MSR_TS_T))
- tabort_syscall(regs->nip, regs->msr);
-
account_cpu_user_entry();
#ifdef CONFIG_PPC_SPLPAR
@@ -161,7 +155,7 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
again:
local_irq_disable();
ti_flags = READ_ONCE(*ti_flagsp);
- while (unlikely(ti_flags & _TIF_USER_WORK_MASK)) {
+ while (unlikely(ti_flags & (_TIF_USER_WORK_MASK & ~_TIF_RESTORE_TM))) {
local_irq_enable();
if (ti_flags & _TIF_NEED_RESCHED) {
schedule();
@@ -180,13 +174,20 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
}
if (IS_ENABLED(CONFIG_PPC_BOOK3S) && IS_ENABLED(CONFIG_PPC_FPU)) {
- unsigned long mathflags = MSR_FP;
+ if (IS_ENABLED(CONFIG_PPC_TRANSACTIONAL_MEM) &&
+ unlikely((ti_flags & _TIF_RESTORE_TM))) {
+ restore_tm_state(regs);
+ } else {
+ unsigned long mathflags = MSR_FP;
- if (IS_ENABLED(CONFIG_ALTIVEC))
- mathflags |= MSR_VEC;
+ if (cpu_has_feature(CPU_FTR_VSX))
+ mathflags |= MSR_VEC | MSR_VSX;
+ else if (cpu_has_feature(CPU_FTR_ALTIVEC))
+ mathflags |= MSR_VEC;
- if ((regs->msr & mathflags) != mathflags)
- restore_math(regs);
+ if ((regs->msr & mathflags) != mathflags)
+ restore_math(regs);
+ }
}
/* This must be done with RI=1 because tracing may touch vmaps */
--
2.23.0
^ permalink raw reply related
* [PATCH 2/2] Fix 3 "[v3, 28/32] powerpc/64s: interrupt implement exit logic in C"
From: Nicholas Piggin @ 2020-03-25 9:30 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin
In-Reply-To: <20200325093014.149962-1-npiggin@gmail.com>
This fixes the interrupt-return part of the MSR_VSX restore bug caught
by tm-unavailable selftest.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kernel/syscall_64.c | 24 +++++++++++++-----------
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/kernel/syscall_64.c b/arch/powerpc/kernel/syscall_64.c
index 56533a26f3b7..a2995909b83b 100644
--- a/arch/powerpc/kernel/syscall_64.c
+++ b/arch/powerpc/kernel/syscall_64.c
@@ -251,19 +251,21 @@ notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs, unsigned
ti_flags = READ_ONCE(*ti_flagsp);
}
- if (IS_ENABLED(CONFIG_PPC_BOOK3S)) {
- unsigned long mathflags = 0;
-
- if (IS_ENABLED(CONFIG_PPC_FPU))
- mathflags |= MSR_FP;
- if (IS_ENABLED(CONFIG_ALTIVEC))
- mathflags |= MSR_VEC;
-
+ if (IS_ENABLED(CONFIG_PPC_BOOK3S) && IS_ENABLED(CONFIG_PPC_FPU)) {
if (IS_ENABLED(CONFIG_PPC_TRANSACTIONAL_MEM) &&
- (ti_flags & _TIF_RESTORE_TM))
+ unlikely((ti_flags & _TIF_RESTORE_TM))) {
restore_tm_state(regs);
- else if ((regs->msr & mathflags) != mathflags)
- restore_math(regs);
+ } else {
+ unsigned long mathflags = MSR_FP;
+
+ if (cpu_has_feature(CPU_FTR_VSX))
+ mathflags |= MSR_VEC | MSR_VSX;
+ else if (cpu_has_feature(CPU_FTR_ALTIVEC))
+ mathflags |= MSR_VEC;
+
+ if ((regs->msr & mathflags) != mathflags)
+ restore_math(regs);
+ }
}
trace_hardirqs_on();
--
2.23.0
^ permalink raw reply related
* Re: [PATCH] powerpc/boot: Delete unneeded .globl _zimage_start
From: Alan Modra @ 2020-03-25 8:37 UTC (permalink / raw)
To: Joel Stanley
Cc: Nick Desaulniers, linuxppc-dev, Fangrui Song, clang-built-linux
In-Reply-To: <CACPK8Xf70Yq2szW110G_2f2Q3J6inVqQY4Y-y0tggkVmT7tmfw@mail.gmail.com>
On Wed, Mar 25, 2020 at 05:22:31AM +0000, Joel Stanley wrote:
> On Wed, 25 Mar 2020 at 05:19, Fangrui Song <maskray@google.com> wrote:
> >
> > .globl sets the symbol binding to STB_GLOBAL while .weak sets the
> > binding to STB_WEAK. They should not be used together. It is accidetal
> > rather then intentional that GNU as let .weak override .globl while
> > clang integrated assembler let the last win.
No, it isn't accidental. gas deliberately lets .weak override .globl.
Since 1996-07-26, git commit 5ca547dc239
I'm fine with the patch so far as it is true that there is no need for
both .globl and .weak (and it looks silly to have both), but the
explanation isn't true. The patch is needed because the clang
assembler is incompatible with gas in this detail.
> > Fixes: cd197ffcf10b "[POWERPC] zImage: Cleanup and improve zImage entry point"
> > Fixes: ee9d21b3b358 "powerpc/boot: Ensure _zimage_start is a weak symbol"
> > Link: https://github.com/ClangBuiltLinux/linux/issues/937
> > Signed-off-by: Fangrui Song <maskray@google.com>
> > Cc: Joel Stanley <joel@jms.id.au>
> > Cc: Michael Ellerman <mpe@ellerman.id.au>
> > Cc: Nick Desaulniers <ndesaulniers@google.com>
> > Cc: clang-built-linux@googlegroups.com
> > ---
> > arch/powerpc/boot/crt0.S | 3 ---
> > 1 file changed, 3 deletions(-)
> >
> > diff --git a/arch/powerpc/boot/crt0.S b/arch/powerpc/boot/crt0.S
> > index 92608f34d312..1d83966f5ef6 100644
> > --- a/arch/powerpc/boot/crt0.S
> > +++ b/arch/powerpc/boot/crt0.S
> > @@ -44,9 +44,6 @@ p_end: .long _end
> > p_pstack: .long _platform_stack_top
> > #endif
> >
> > - .globl _zimage_start
> > - /* Clang appears to require the .weak directive to be after the symbol
> > - * is defined. See https://bugs.llvm.org/show_bug.cgi?id=38921 */
> > .weak _zimage_start
> > _zimage_start:
>
> Your explanation makes sense to me. I've added Alan to cc for his review.
>
> Reviewed-by: Joel Stanley <joel@jms.id.au>
>
> Thanks for the patch.
>
> Cheers,
>
> Joel
>
> > .globl _zimage_start_lib
> > --
> > 2.25.1.696.g5e7596f4ac-goog
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply
* Re: [patch V3 03/20] usb: gadget: Use completion interface instead of open coding it
From: Felipe Balbi @ 2020-03-25 8:37 UTC (permalink / raw)
To: Thomas Gleixner, LKML
Cc: Randy Dunlap, linux-ia64, Peter Zijlstra, linux-pci,
Sebastian Siewior, platform-driver-x86, Guo Ren, Joel Fernandes,
Vincent Chen, Ingo Molnar, Jonathan Corbet, Davidlohr Bueso,
kbuild test robot, Brian Cain, linux-acpi, Paul E . McKenney,
linux-hexagon, Rafael J. Wysocki, linux-csky, Linus Torvalds,
Darren Hart, Zhang Rui, Len Brown, Fenghua Yu, Arnd Bergmann,
linux-pm, linuxppc-dev, Greentime Hu, Bjorn Helgaas,
Kurt Schwemmer, Kalle Valo, Michal Simek, Tony Luck, Nick Hu,
Geoff Levand, Greg Kroah-Hartman, linux-usb, linux-wireless,
Oleg Nesterov, Davidlohr Bueso, netdev, Logan Gunthorpe,
David S. Miller, Andy Shevchenko
In-Reply-To: <20200321113241.043380271@linutronix.de>
[-- Attachment #1: Type: text/plain, Size: 1032 bytes --]
Thomas Gleixner <tglx@linutronix.de> writes:
> From: Thomas Gleixner <tglx@linutronix.de>
>
> ep_io() uses a completion on stack and open codes the waiting with:
>
> wait_event_interruptible (done.wait, done.done);
> and
> wait_event (done.wait, done.done);
>
> This waits in non-exclusive mode for complete(), but there is no reason to
> do so because the completion can only be waited for by the task itself and
> complete() wakes exactly one exlusive waiter.
>
> Replace the open coded implementation with the corresponding
> wait_for_completion*() functions.
>
> No functional change.
>
> Reported-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Felipe Balbi <balbi@kernel.org>
> Cc: linux-usb@vger.kernel.org
Do you want to carry it via your tree? If so:
Acked-by: Felipe Balbi <balbi@kernel.org>
Otherwise, let me know and I'll pick this patch.
--
balbi
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] dma-mapping: add a dma_ops_bypass flag to struct device
From: Christoph Hellwig @ 2020-03-25 8:37 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: Greg Kroah-Hartman, Joerg Roedel, Robin Murphy, linux-kernel,
iommu, Aneesh Kumar K.V, linuxppc-dev, Christoph Hellwig,
Lu Baolu
In-Reply-To: <41975da3-3a4a-fc3c-2b90-8d607cf220e6@ozlabs.ru>
On Wed, Mar 25, 2020 at 03:51:36PM +1100, Alexey Kardashevskiy wrote:
> >> This is for persistent memory which you can DMA to/from but yet it does
> >> not appear in the system as a normal memory and therefore requires
> >> special handling anyway (O_DIRECT or DAX, I do not know the exact
> >> mechanics). All other devices in the system should just run as usual,
> >> i.e. use 1:1 mapping if possible.
> >
> > On other systems (x86 and arm) pmem as long as it is page backed does
> > not require any special handling. This must be some weird way powerpc
> > fucked up again, and I suspect you'll have to suffer from it.
>
>
> It does not matter if it is backed by pages or not, the problem may also
> appear if we wanted for example p2p PCI via IOMMU (between PHBs) and
> MMIO might be mapped way too high in the system address space and make
> 1:1 impossible.
How can it be mapped too high for a direct mapping with a 64-bit DMA
mask?
^ permalink raw reply
* Re: [PATCH] mm/sparse: Fix kernel crash with pfn_section_valid check
From: Baoquan He @ 2020-03-25 8:36 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: Sachin Sant, david, linux-kernel, mhocko, linux-mm, akpm,
linuxppc-dev, dan.j.williams
In-Reply-To: <5cdf5334-7fbb-8427-1918-ed67d5f23834@linux.ibm.com>
On 03/25/20 at 01:42pm, Aneesh Kumar K.V wrote:
> On 3/25/20 1:07 PM, Baoquan He wrote:
> > On 03/25/20 at 03:06pm, Baoquan He wrote:
> > > On 03/25/20 at 08:49am, Aneesh Kumar K.V wrote:
> >
> > > > mm/sparse.c | 2 ++
> > > > 1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/mm/sparse.c b/mm/sparse.c
> > > > index aadb7298dcef..3012d1f3771a 100644
> > > > --- a/mm/sparse.c
> > > > +++ b/mm/sparse.c
> > > > @@ -781,6 +781,8 @@ static void section_deactivate(unsigned long pfn, unsigned long nr_pages,
> > > > ms->usage = NULL;
> > > > }
> > > > memmap = sparse_decode_mem_map(ms->section_mem_map, section_nr);
> > > > + /* Mark the section invalid */
> > > > + ms->section_mem_map &= ~SECTION_HAS_MEM_MAP;
> > >
> > > Not sure if we should add checking in valid_section() or pfn_valid(),
> > > e.g check ms->usage validation too. Otherwise, this fix looks good to
> > > me.
> >
> > With SPASEMEM_VMEMAP enabled, we should do validation check on ms->usage
> > before checking any subsection is valid. Since now we do have case
> > in which ms->usage is released, people still try to check it.
> >
> > diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> > index f0a2c184eb9a..d79bd938852e 100644
> > --- a/include/linux/mmzone.h
> > +++ b/include/linux/mmzone.h
> > @@ -1306,6 +1306,8 @@ static inline int pfn_section_valid(struct mem_section *ms, unsigned long pfn)
> > {
> > int idx = subsection_map_index(pfn);
> > + if (!ms->usage)
> > + return 0;
> > return test_bit(idx, ms->usage->subsection_map);
> > }
> > #else
> >
>
> We always check for section valid, before we check if pfn_section_valid().
>
> static inline int pfn_valid(unsigned long pfn)
>
> struct mem_section *ms;
>
> if (pfn_to_section_nr(pfn) >= NR_MEM_SECTIONS)
> return 0;
> ms = __nr_to_section(pfn_to_section_nr(pfn));
> if (!valid_section(ms))
> return 0;
> /*
> * Traditionally early sections always returned pfn_valid() for
> * the entire section-sized span.
> */
> return early_section(ms) || pfn_section_valid(ms, pfn);
> }
>
>
> IMHO adding that if (!ms->usage) is redundant.
Yeah, I tend to agree. Consider this happens in the only small window
between ms->usage releasing and ms->section_mem_map releasing when
removing a section. Just thought adding this check to enhance it even
though we have had your fix, because we only check ms->section_mem_map
in valid_section(). Anyway, your fix looks good to me, see if other
people have any comment.
Thanks
Baoquan
^ permalink raw reply
* Re: [PATCH] mm/sparse: Fix kernel crash with pfn_section_valid check
From: Aneesh Kumar K.V @ 2020-03-25 8:12 UTC (permalink / raw)
To: Baoquan He
Cc: Sachin Sant, linux-kernel, linux-mm, akpm, linuxppc-dev,
dan.j.williams
In-Reply-To: <20200325073707.GI3039@MiWiFi-R3L-srv>
On 3/25/20 1:07 PM, Baoquan He wrote:
> On 03/25/20 at 03:06pm, Baoquan He wrote:
>> On 03/25/20 at 08:49am, Aneesh Kumar K.V wrote:
>
>>> mm/sparse.c | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/mm/sparse.c b/mm/sparse.c
>>> index aadb7298dcef..3012d1f3771a 100644
>>> --- a/mm/sparse.c
>>> +++ b/mm/sparse.c
>>> @@ -781,6 +781,8 @@ static void section_deactivate(unsigned long pfn, unsigned long nr_pages,
>>> ms->usage = NULL;
>>> }
>>> memmap = sparse_decode_mem_map(ms->section_mem_map, section_nr);
>>> + /* Mark the section invalid */
>>> + ms->section_mem_map &= ~SECTION_HAS_MEM_MAP;
>>
>> Not sure if we should add checking in valid_section() or pfn_valid(),
>> e.g check ms->usage validation too. Otherwise, this fix looks good to
>> me.
>
> With SPASEMEM_VMEMAP enabled, we should do validation check on ms->usage
> before checking any subsection is valid. Since now we do have case
> in which ms->usage is released, people still try to check it.
>
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index f0a2c184eb9a..d79bd938852e 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -1306,6 +1306,8 @@ static inline int pfn_section_valid(struct mem_section *ms, unsigned long pfn)
> {
> int idx = subsection_map_index(pfn);
>
> + if (!ms->usage)
> + return 0;
> return test_bit(idx, ms->usage->subsection_map);
> }
> #else
>
We always check for section valid, before we check if pfn_section_valid().
static inline int pfn_valid(unsigned long pfn)
struct mem_section *ms;
if (pfn_to_section_nr(pfn) >= NR_MEM_SECTIONS)
return 0;
ms = __nr_to_section(pfn_to_section_nr(pfn));
if (!valid_section(ms))
return 0;
/*
* Traditionally early sections always returned pfn_valid() for
* the entire section-sized span.
*/
return early_section(ms) || pfn_section_valid(ms, pfn);
}
IMHO adding that if (!ms->usage) is redundant.
-aneesh
^ permalink raw reply
* Re: [PATCH] mm/sparse: Fix kernel crash with pfn_section_valid check
From: Baoquan He @ 2020-03-25 7:37 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: Sachin Sant, linux-kernel, linux-mm, akpm, linuxppc-dev,
dan.j.williams
In-Reply-To: <20200325070643.GH3039@MiWiFi-R3L-srv>
On 03/25/20 at 03:06pm, Baoquan He wrote:
> On 03/25/20 at 08:49am, Aneesh Kumar K.V wrote:
> > mm/sparse.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/mm/sparse.c b/mm/sparse.c
> > index aadb7298dcef..3012d1f3771a 100644
> > --- a/mm/sparse.c
> > +++ b/mm/sparse.c
> > @@ -781,6 +781,8 @@ static void section_deactivate(unsigned long pfn, unsigned long nr_pages,
> > ms->usage = NULL;
> > }
> > memmap = sparse_decode_mem_map(ms->section_mem_map, section_nr);
> > + /* Mark the section invalid */
> > + ms->section_mem_map &= ~SECTION_HAS_MEM_MAP;
>
> Not sure if we should add checking in valid_section() or pfn_valid(),
> e.g check ms->usage validation too. Otherwise, this fix looks good to
> me.
With SPASEMEM_VMEMAP enabled, we should do validation check on ms->usage
before checking any subsection is valid. Since now we do have case
in which ms->usage is released, people still try to check it.
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index f0a2c184eb9a..d79bd938852e 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -1306,6 +1306,8 @@ static inline int pfn_section_valid(struct mem_section *ms, unsigned long pfn)
{
int idx = subsection_map_index(pfn);
+ if (!ms->usage)
+ return 0;
return test_bit(idx, ms->usage->subsection_map);
}
#else
^ permalink raw reply related
* Re: [PATCH] mm/sparse: Fix kernel crash with pfn_section_valid check
From: Baoquan He @ 2020-03-25 7:06 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: Sachin Sant, linux-kernel, linux-mm, akpm, linuxppc-dev,
dan.j.williams
In-Reply-To: <20200325031914.107660-1-aneesh.kumar@linux.ibm.com>
On 03/25/20 at 08:49am, Aneesh Kumar K.V wrote:
> Fixes the below crash
>
> BUG: Kernel NULL pointer dereference on read at 0x00000000
> Faulting instruction address: 0xc000000000c3447c
> Oops: Kernel access of bad area, sig: 11 [#1]
> LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
> CPU: 11 PID: 7519 Comm: lt-ndctl Not tainted 5.6.0-rc7-autotest #1
> ...
> NIP [c000000000c3447c] vmemmap_populated+0x98/0xc0
> LR [c000000000088354] vmemmap_free+0x144/0x320
> Call Trace:
> section_deactivate+0x220/0x240
> __remove_pages+0x118/0x170
> arch_remove_memory+0x3c/0x150
> memunmap_pages+0x1cc/0x2f0
> devm_action_release+0x30/0x50
> release_nodes+0x2f8/0x3e0
> device_release_driver_internal+0x168/0x270
> unbind_store+0x130/0x170
> drv_attr_store+0x44/0x60
> sysfs_kf_write+0x68/0x80
> kernfs_fop_write+0x100/0x290
> __vfs_write+0x3c/0x70
> vfs_write+0xcc/0x240
> ksys_write+0x7c/0x140
> system_call+0x5c/0x68
>
> With commit: d41e2f3bd546 ("mm/hotplug: fix hot remove failure in SPARSEMEM|!VMEMMAP case")
> section_mem_map is set to NULL after depopulate_section_mem(). This
> was done so that pfn_page() can work correctly with kernel config that disables
> SPARSEMEM_VMEMMAP. With that config pfn_to_page does
>
> __section_mem_map_addr(__sec) + __pfn;
> where
>
> static inline struct page *__section_mem_map_addr(struct mem_section *section)
> {
> unsigned long map = section->section_mem_map;
> map &= SECTION_MAP_MASK;
> return (struct page *)map;
> }
>
> Now with SPASEMEM_VMEMAP enabled, mem_section->usage->subsection_map is used to
> check the pfn validity (pfn_valid()). Since section_deactivate release
> mem_section->usage if a section is fully deactivated, pfn_valid() check after
> a subsection_deactivate cause a kernel crash.
>
> static inline int pfn_valid(unsigned long pfn)
> {
> ...
> return early_section(ms) || pfn_section_valid(ms, pfn);
> }
>
> where
>
> static inline int pfn_section_valid(struct mem_section *ms, unsigned long pfn)
> {
> int idx = subsection_map_index(pfn);
>
> return test_bit(idx, ms->usage->subsection_map);
> }
>
> Avoid this by clearing SECTION_HAS_MEM_MAP when mem_section->usage is freed.
>
> Fixes: d41e2f3bd546 ("mm/hotplug: fix hot remove failure in SPARSEMEM|!VMEMMAP case")
> Cc: Baoquan He <bhe@redhat.com>
> Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Maybe add Sachin's Tested-by, Sachin has tested and confirmed this fix
works.
> ---
> mm/sparse.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/mm/sparse.c b/mm/sparse.c
> index aadb7298dcef..3012d1f3771a 100644
> --- a/mm/sparse.c
> +++ b/mm/sparse.c
> @@ -781,6 +781,8 @@ static void section_deactivate(unsigned long pfn, unsigned long nr_pages,
> ms->usage = NULL;
> }
> memmap = sparse_decode_mem_map(ms->section_mem_map, section_nr);
> + /* Mark the section invalid */
> + ms->section_mem_map &= ~SECTION_HAS_MEM_MAP;
Not sure if we should add checking in valid_section() or pfn_valid(),
e.g check ms->usage validation too. Otherwise, this fix looks good to
me.
Reviewed-by: Baoquan He <bhe@redhat.com>
^ permalink raw reply
* Re: [PATCH] powerpc/boot: Delete unneeded .globl _zimage_start
From: Segher Boessenkool @ 2020-03-25 6:23 UTC (permalink / raw)
To: Fangrui Song
Cc: Nick Desaulniers, linuxppc-dev, Joel Stanley, clang-built-linux
In-Reply-To: <20200325051820.163253-1-maskray@google.com>
On Tue, Mar 24, 2020 at 10:18:20PM -0700, Fangrui Song wrote:
> .globl sets the symbol binding to STB_GLOBAL while .weak sets the
> binding to STB_WEAK. They should not be used together. It is accidetal
> rather then intentional that GNU as let .weak override .globl while
> clang integrated assembler let the last win.
Nothing is "overridden".
The as manual says (.weak):
This directive sets the weak attribute on the comma separated list of
symbol 'names'. If the symbols do not already exist, they will be
created.
so this behaviour is obviously as intended (or was later documented in
any case), so LLVM has a bug to fix (whether you like this (much saner)
behaviour or not).
Segher
^ permalink raw reply
* Re: Argh, can't find dcache properties !
From: Michael Ellerman @ 2020-03-25 6:01 UTC (permalink / raw)
To: Qian Cai, Chris Packham
Cc: Hamish Martin, linux-kernel@vger.kernel.org, paulus@samba.org,
tglx@linutronix.de, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <48F1D8CF-13A1-4348-8973-81503782A451@lca.pw>
Qian Cai <cai@lca.pw> writes:
>> On Mar 24, 2020, at 4:06 PM, Chris Packham <Chris.Packham@alliedtelesis.co.nz> wrote:
>> On Tue, 2020-03-24 at 15:47 +1100, Michael Ellerman wrote:
>>> Chris Packham <Chris.Packham@alliedtelesis.co.nz> writes:
>>>> Hi All,
>>>>
>>>> Just booting up v5.5.11 on a Freescale T2080RDB and I'm seeing the
>>>> following mesage.
>>>>
>>>> kern.warning linuxbox kernel: Argh, can't find dcache properties !
>>>> kern.warning linuxbox kernel: Argh, can't find icache properties !
...
>
> BTW, POWER9 PowerNV would have the same thing.
Ugh, you're right.
Because we're missing the cache-line-size properties, even though they
are optional when the block & line size are the same.
# find /proc/device-tree/cpus/PowerPC\,POWER9@0/ -name '*cache*'
/proc/device-tree/cpus/PowerPC,POWER9@0/l2-cache
/proc/device-tree/cpus/PowerPC,POWER9@0/d-cache-block-size
/proc/device-tree/cpus/PowerPC,POWER9@0/d-cache-size
/proc/device-tree/cpus/PowerPC,POWER9@0/i-cache-size
/proc/device-tree/cpus/PowerPC,POWER9@0/d-cache-sets
/proc/device-tree/cpus/PowerPC,POWER9@0/i-cache-block-size
/proc/device-tree/cpus/PowerPC,POWER9@0/i-cache-sets
skiboot even explicitly omits them:
if (cache->icache_line_size != cache->icache_block_size)
dt_add_property_cells(cpu, "i-cache-line-size",
be32_to_cpu(cache->icache_line_size));
if (cache->l1_dcache_line_size != cache->dcache_block_size)
dt_add_property_cells(cpu, "d-cache-line-size",
be32_to_cpu(cache->l1_dcache_line_size));
Looks like it was broken ~3 years ago, in:
bd067f83b084 ("powerpc/64: Fix naming of cache block vs. cache line")
Previously we did:
lsizep = of_get_property(np, "d-cache-block-size",
NULL);
/* fallback if block size missing */
if (lsizep == NULL)
lsizep = of_get_property(np,
"d-cache-line-size",
NULL);
if (lsizep != NULL)
lsize = be32_to_cpu(*lsizep);
if (sizep == NULL || lsizep == NULL)
DBG("Argh, can't find dcache properties ! "
"sizep: %p, lsizep: %p\n", sizep, lsizep);
ie. fallback from block size to line size, and only print if both are missing.
That commit changed the names and the logic, but not in a consistent
fashion, making "d-cache-line-size" required to avoid the Argh:
bsizep = of_get_property(np, "d-cache-block-size",
NULL);
lsizep = of_get_property(np, "d-cache-line-size",
NULL);
if (bsizep == NULL)
bsizep = lsizep;
if (lsizep != NULL)
lsize = be32_to_cpu(*lsizep);
if (bsizep != NULL)
bsize = be32_to_cpu(*bsizep);
if (sizep == NULL || bsizep == NULL || lsizep == NULL)
DBG("Argh, can't find dcache properties ! "
"sizep: %p, bsizep: %p, lsizep: %p\n",
sizep, bsizep, lsizep);
Back then we fell back to cur_cpu_spec->dcache_bsize, which should be
correct. But since then we introduced the device tree CPU features
parsing, which does:
static struct cpu_spec __initdata base_cpu_spec = {
...
.icache_bsize = 32, /* minimum block size, fixed by */
.dcache_bsize = 32, /* cache info init. */
So on systems with new enough skiboot we now default to 32, which is
wrong on Power9.
Luckily this info is not used by the sysfs cache files, because that
code doesn't use the values we parse here, it goes and looks at the
device tree itself. Which is pretty gross but actually saves us in this
case.
These values do end up in the vdso_data, and I can see the wrong values
in the vdso_data:
1c:mon> d4 c000000002390000
c000000002390000 54535953 46434d45 50503a47 00343643 eyecatcher
c000000002390010 00000001 00000001 00000100 004e1202 major minor platform processor
c000000002390020 000000b0 00000000 00000000 0000003c processorCount physicalMemorySize
c000000002390030 57b7623f 0000ac10 1e848000 00000000 tb_orig_stamp tb_ticks_per_sec
c000000002390040 de6d9e42 008637af 8de66bca 0005e7ae tb_to_xs stamp_xsec
c000000002390050 000ff730 00000000 00000000 00000000 tb_update_count tz_minuteswest tz_dsttime
c000000002390060 00008000 00000020 00008000 00000020 dcache_size line_size icache_size icache_line_size
^
32 != 128
And that appears to flow through to glibc, ie. I see:
_SC_LEVEL1_ICACHE_LINESIZE = 32
_SC_LEVEL1_DCACHE_LINESIZE = 32
_SC_LEVEL2_CACHE_LINESIZE = 32
_SC_LEVEL3_CACHE_LINESIZE = 32
So excuse me while I go and swear at something.
cheers
^ permalink raw reply
* Re: [PATCH] powerpc/boot: Delete unneeded .globl _zimage_start
From: Joel Stanley @ 2020-03-25 5:22 UTC (permalink / raw)
To: Fangrui Song, Alan Modra
Cc: Nick Desaulniers, linuxppc-dev, clang-built-linux
In-Reply-To: <20200325051820.163253-1-maskray@google.com>
On Wed, 25 Mar 2020 at 05:19, Fangrui Song <maskray@google.com> wrote:
>
> .globl sets the symbol binding to STB_GLOBAL while .weak sets the
> binding to STB_WEAK. They should not be used together. It is accidetal
> rather then intentional that GNU as let .weak override .globl while
> clang integrated assembler let the last win.
>
> Fixes: cd197ffcf10b "[POWERPC] zImage: Cleanup and improve zImage entry point"
> Fixes: ee9d21b3b358 "powerpc/boot: Ensure _zimage_start is a weak symbol"
> Link: https://github.com/ClangBuiltLinux/linux/issues/937
> Signed-off-by: Fangrui Song <maskray@google.com>
> Cc: Joel Stanley <joel@jms.id.au>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Cc: clang-built-linux@googlegroups.com
> ---
> arch/powerpc/boot/crt0.S | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/arch/powerpc/boot/crt0.S b/arch/powerpc/boot/crt0.S
> index 92608f34d312..1d83966f5ef6 100644
> --- a/arch/powerpc/boot/crt0.S
> +++ b/arch/powerpc/boot/crt0.S
> @@ -44,9 +44,6 @@ p_end: .long _end
> p_pstack: .long _platform_stack_top
> #endif
>
> - .globl _zimage_start
> - /* Clang appears to require the .weak directive to be after the symbol
> - * is defined. See https://bugs.llvm.org/show_bug.cgi?id=38921 */
> .weak _zimage_start
> _zimage_start:
Your explanation makes sense to me. I've added Alan to cc for his review.
Reviewed-by: Joel Stanley <joel@jms.id.au>
Thanks for the patch.
Cheers,
Joel
> .globl _zimage_start_lib
> --
> 2.25.1.696.g5e7596f4ac-goog
>
^ permalink raw reply
* [PATCH] powerpc/boot: Delete unneeded .globl _zimage_start
From: Fangrui Song @ 2020-03-25 5:18 UTC (permalink / raw)
To: linuxppc-dev
Cc: Nick Desaulniers, Joel Stanley, Fangrui Song, clang-built-linux
.globl sets the symbol binding to STB_GLOBAL while .weak sets the
binding to STB_WEAK. They should not be used together. It is accidetal
rather then intentional that GNU as let .weak override .globl while
clang integrated assembler let the last win.
Fixes: cd197ffcf10b "[POWERPC] zImage: Cleanup and improve zImage entry point"
Fixes: ee9d21b3b358 "powerpc/boot: Ensure _zimage_start is a weak symbol"
Link: https://github.com/ClangBuiltLinux/linux/issues/937
Signed-off-by: Fangrui Song <maskray@google.com>
Cc: Joel Stanley <joel@jms.id.au>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: clang-built-linux@googlegroups.com
---
arch/powerpc/boot/crt0.S | 3 ---
1 file changed, 3 deletions(-)
diff --git a/arch/powerpc/boot/crt0.S b/arch/powerpc/boot/crt0.S
index 92608f34d312..1d83966f5ef6 100644
--- a/arch/powerpc/boot/crt0.S
+++ b/arch/powerpc/boot/crt0.S
@@ -44,9 +44,6 @@ p_end: .long _end
p_pstack: .long _platform_stack_top
#endif
- .globl _zimage_start
- /* Clang appears to require the .weak directive to be after the symbol
- * is defined. See https://bugs.llvm.org/show_bug.cgi?id=38921 */
.weak _zimage_start
_zimage_start:
.globl _zimage_start_lib
--
2.25.1.696.g5e7596f4ac-goog
^ permalink raw reply related
* Re: [PATCH 1/2] dma-mapping: add a dma_ops_bypass flag to struct device
From: Alexey Kardashevskiy @ 2020-03-25 4:51 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Aneesh Kumar K.V, Joerg Roedel, Robin Murphy, linux-kernel, iommu,
Greg Kroah-Hartman, linuxppc-dev, Lu Baolu
In-Reply-To: <20200324075402.GJ23447@lst.de>
On 24/03/2020 18:54, Christoph Hellwig wrote:
> On Tue, Mar 24, 2020 at 02:05:54PM +1100, Alexey Kardashevskiy wrote:
>> This is for persistent memory which you can DMA to/from but yet it does
>> not appear in the system as a normal memory and therefore requires
>> special handling anyway (O_DIRECT or DAX, I do not know the exact
>> mechanics). All other devices in the system should just run as usual,
>> i.e. use 1:1 mapping if possible.
>
> On other systems (x86 and arm) pmem as long as it is page backed does
> not require any special handling. This must be some weird way powerpc
> fucked up again, and I suspect you'll have to suffer from it.
It does not matter if it is backed by pages or not, the problem may also
appear if we wanted for example p2p PCI via IOMMU (between PHBs) and
MMIO might be mapped way too high in the system address space and make
1:1 impossible.
--
Alexey
^ permalink raw reply
* [PATCH] powerpc/tm: Document h/rfid and mtmsrd quirk
From: Michael Neuling @ 2020-03-25 4:05 UTC (permalink / raw)
To: mpe; +Cc: Michael Neuling, linuxppc-dev, npiggin, Gustavo Romero
The ISA has a quirk that's useful for the Linux implementation.
Document it here so others are less likely to trip over it.
Signed-off-by: Michael Neuling <mikey@neuling.org>
Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
---
.../powerpc/transactional_memory.rst | 27 +++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/Documentation/powerpc/transactional_memory.rst b/Documentation/powerpc/transactional_memory.rst
index 09955103ac..74ae71001a 100644
--- a/Documentation/powerpc/transactional_memory.rst
+++ b/Documentation/powerpc/transactional_memory.rst
@@ -245,3 +245,30 @@ POWER9N DD2.2.
Guest migration from POWER8 to POWER9 will work with POWER9N DD2.2 and
POWER9C DD1.2. Since earlier POWER9 processors don't support TM
emulation, migration from POWER8 to POWER9 is not supported there.
+
+Kernel implementation
+=====================
+
+h/rfid mtmsrd quirk
+===================
+
+As defined in the ISA, rfid has a quirk which is useful in early
+exception handling. When in a userspace transaction and we enter the
+kernel via some exception, MSR will end up as TM=0 and TS=01 (ie. TM
+off but TM suspended). Regularly the kernel will want change bits in
+the MSR and will perform an rfid to do this. In this case rfid can
+have SRR0 TM = 0 and TS = 00 (ie. TM off and non transaction) and the
+resulting MSR will retain TM = 0 and TS=01 from before (ie. stay in
+suspend). This is a quirk in the architecture as this would normally
+be a transition from TS=01 to TS=00 (ie. suspend -> non transactional)
+which is an illegal transition.
+
+This quirk is described the architecture in the definition of rfid
+with these lines:
+
+ if (MSR 29:31 ¬ = 0b010 | SRR1 29:31 ¬ = 0b000) then
+ MSR 29:31 <- SRR1 29:31
+
+hrfid and mtmsrd have the same quirk.
+
+The Linux kernel uses this quirk in it's early exception handling.
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v4 3/9] powerpc/vas: Add VAS user space API
From: Haren Myneni @ 2020-03-25 3:48 UTC (permalink / raw)
To: Michael Ellerman
Cc: mikey, herbert, npiggin, linux-crypto, sukadev, linuxppc-dev,
Daniel Axtens
In-Reply-To: <87zhc6xvuk.fsf@mpe.ellerman.id.au>
On Tue, 2020-03-24 at 14:41 +1100, Michael Ellerman wrote:
> Daniel Axtens <dja@axtens.net> writes:
> > Michael Ellerman <mpe@ellerman.id.au> writes:
> >> Daniel Axtens <dja@axtens.net> writes:
> >>> Haren Myneni <haren@linux.ibm.com> writes:
> >>>> diff --git a/arch/powerpc/platforms/powernv/vas-api.c b/arch/powerpc/platforms/powernv/vas-api.c
> >>>> new file mode 100644
> >>>> index 0000000..7d049af
> >>>> --- /dev/null
> >>>> +++ b/arch/powerpc/platforms/powernv/vas-api.c
> >>>> @@ -0,0 +1,257 @@
> >> ...
> >>>> +
> >>>> +static int coproc_mmap(struct file *fp, struct vm_area_struct *vma)
> >>>> +{
> >>>> + struct vas_window *txwin = fp->private_data;
> >>>> + unsigned long pfn;
> >>>> + u64 paste_addr;
> >>>> + pgprot_t prot;
> >>>> + int rc;
> >>>> +
> >>>> + if ((vma->vm_end - vma->vm_start) > PAGE_SIZE) {
> >>>
> >>> I think you said this should be 4096 rather than 64k, regardless of what
> >>> PAGE_SIZE you are compiled with?
> >>
> >> You can't mmap less than a page, a page is PAGE_SIZE bytes.
> >>
> >> So if that checked for 4K explicitly it would prevent mmap on 64K
> >> kernels always, which seems like not what you want?
> >
> > Ah. My bad. Carry on then :)
>
> Well you were just quoting something from Haren, so I think it's over to
> him.
Sorry my mistake.
I should change in documentation. vas_win_paste_addr() always returns 1
page. Restriction should be PAGE_SIZE.
>
> cheers
^ permalink raw reply
* Re: [PATCH v8 12/14] powerpc/vas: Return credits after handling fault
From: Haren Myneni @ 2020-03-25 3:35 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: mikey, ajd, hch, oohall, sukadev, linuxppc-dev, herbert
In-Reply-To: <1584931224.oij3nwbo4z.astroid@bobo.none>
On Mon, 2020-03-23 at 12:44 +1000, Nicholas Piggin wrote:
> Haren Myneni's on March 19, 2020 4:19 pm:
> >
> > NX expects OS to return credit for send window after processing each
> > fault. Also credit has to be returned even for fault window.
>
> And this should be merged in the fault handler function.
credits are assigned and used per VAS window - default value is 1024 for
user space windows, and fault_fifo_size/CRb_SIZE for fault window.
When user space submits request, credit is taken on specific window (by
VAS). After successful processing of this request, NX return credit. In
case if NX sees fault, expects OS return credit for the corresponding
user space window after handling fault CRB.
Similarly NX takes credit on fault window after pasting fault CRB and
expects return credit after handling fault CRB. NX workbook has on
credits usage and How this credit system works.
Thought vas_return_credit() is unique function and added as separate
patch so that easy to review.
>
> >
> > Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
> > Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> > ---
> > arch/powerpc/platforms/powernv/vas-fault.c | 9 +++++++++
> > arch/powerpc/platforms/powernv/vas-window.c | 17 +++++++++++++++++
> > arch/powerpc/platforms/powernv/vas.h | 1 +
> > 3 files changed, 27 insertions(+)
> >
> > diff --git a/arch/powerpc/platforms/powernv/vas-fault.c b/arch/powerpc/platforms/powernv/vas-fault.c
> > index 40e1de4..292f7ba 100644
> > --- a/arch/powerpc/platforms/powernv/vas-fault.c
> > +++ b/arch/powerpc/platforms/powernv/vas-fault.c
> > @@ -238,6 +238,10 @@ irqreturn_t vas_fault_thread_fn(int irq, void *data)
> > memcpy(crb, fifo, CRB_SIZE);
> > entry->stamp.nx.pswid = cpu_to_be32(FIFO_INVALID_ENTRY);
> > entry->ccw |= cpu_to_be32(CCW0_INVALID);
> > + /*
> > + * Return credit for the fault window.
> > + */
>
> None of the comments in this patch are useful.
>
> > + vas_return_credit(vinst->fault_win, 0);
>
> Can you use true/false for bools?
>
> > mutex_unlock(&vinst->mutex);
> >
> > pr_devel("VAS[%d] fault_fifo %p, fifo %p, fault_crbs %d\n",
> > @@ -267,6 +271,11 @@ irqreturn_t vas_fault_thread_fn(int irq, void *data)
> > }
> >
> > update_csb(window, crb);
> > + /*
> > + * Return credit for send window after processing
> > + * fault CRB.
> > + */
>
> Any chance of a little bit of explanation how the credit system works?
> Or is it in the code somewhere already?
Sure will add few comments on credit usage.
>
> I don't suppose there is a chance to batch credit updates with multiple
> faults? (maybe the MMIO is insignificant)
Yes, we return credit after processing each CRB. In the case of fault
window, NX can continue pasting fault CRB whenever the credit is
available.
Thanks
Haren
>
> > + vas_return_credit(window, 1);
> > } while (true);
> > }
> >
>
> Thanks,
> Nick
^ permalink raw reply
* [PATCH] mm/sparse: Fix kernel crash with pfn_section_valid check
From: Aneesh Kumar K.V @ 2020-03-25 3:19 UTC (permalink / raw)
To: linux-mm, akpm
Cc: Sachin Sant, Baoquan He, linux-kernel, Aneesh Kumar K.V,
linuxppc-dev
Fixes the below crash
BUG: Kernel NULL pointer dereference on read at 0x00000000
Faulting instruction address: 0xc000000000c3447c
Oops: Kernel access of bad area, sig: 11 [#1]
LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
CPU: 11 PID: 7519 Comm: lt-ndctl Not tainted 5.6.0-rc7-autotest #1
...
NIP [c000000000c3447c] vmemmap_populated+0x98/0xc0
LR [c000000000088354] vmemmap_free+0x144/0x320
Call Trace:
section_deactivate+0x220/0x240
__remove_pages+0x118/0x170
arch_remove_memory+0x3c/0x150
memunmap_pages+0x1cc/0x2f0
devm_action_release+0x30/0x50
release_nodes+0x2f8/0x3e0
device_release_driver_internal+0x168/0x270
unbind_store+0x130/0x170
drv_attr_store+0x44/0x60
sysfs_kf_write+0x68/0x80
kernfs_fop_write+0x100/0x290
__vfs_write+0x3c/0x70
vfs_write+0xcc/0x240
ksys_write+0x7c/0x140
system_call+0x5c/0x68
With commit: d41e2f3bd546 ("mm/hotplug: fix hot remove failure in SPARSEMEM|!VMEMMAP case")
section_mem_map is set to NULL after depopulate_section_mem(). This
was done so that pfn_page() can work correctly with kernel config that disables
SPARSEMEM_VMEMMAP. With that config pfn_to_page does
__section_mem_map_addr(__sec) + __pfn;
where
static inline struct page *__section_mem_map_addr(struct mem_section *section)
{
unsigned long map = section->section_mem_map;
map &= SECTION_MAP_MASK;
return (struct page *)map;
}
Now with SPASEMEM_VMEMAP enabled, mem_section->usage->subsection_map is used to
check the pfn validity (pfn_valid()). Since section_deactivate release
mem_section->usage if a section is fully deactivated, pfn_valid() check after
a subsection_deactivate cause a kernel crash.
static inline int pfn_valid(unsigned long pfn)
{
...
return early_section(ms) || pfn_section_valid(ms, pfn);
}
where
static inline int pfn_section_valid(struct mem_section *ms, unsigned long pfn)
{
int idx = subsection_map_index(pfn);
return test_bit(idx, ms->usage->subsection_map);
}
Avoid this by clearing SECTION_HAS_MEM_MAP when mem_section->usage is freed.
Fixes: d41e2f3bd546 ("mm/hotplug: fix hot remove failure in SPARSEMEM|!VMEMMAP case")
Cc: Baoquan He <bhe@redhat.com>
Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
mm/sparse.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/mm/sparse.c b/mm/sparse.c
index aadb7298dcef..3012d1f3771a 100644
--- a/mm/sparse.c
+++ b/mm/sparse.c
@@ -781,6 +781,8 @@ static void section_deactivate(unsigned long pfn, unsigned long nr_pages,
ms->usage = NULL;
}
memmap = sparse_decode_mem_map(ms->section_mem_map, section_nr);
+ /* Mark the section invalid */
+ ms->section_mem_map &= ~SECTION_HAS_MEM_MAP;
}
if (section_is_early && memmap)
--
2.25.1
^ permalink raw reply related
* [PATCH v2] powerpc/setup_64: Set cache-line-size based on cache-block-size
From: Chris Packham @ 2020-03-25 3:18 UTC (permalink / raw)
To: benh, paulus, mpe, christophe.leroy, tglx, oss
Cc: Hamish Martin, Chris Packham, linuxppc-dev, linux-kernel
In-Reply-To: <dd342c71e03e654a8786302d82f9662004418c6e.camel@alliedtelesis.co.nz>
If {i,d}-cache-block-size is set and {i,d}-cache-line-size is not, use
the block-size value for both. Per the devicetree spec cache-line-size
is only needed if it differs from the block size.
Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
It looks as though the bsizep = lsizep is not required per the spec but it's
probably safer to retain it.
Changes in v2:
- Scott pointed out that u-boot should be filling in the cache properties
(which it does). But it does not specify a cache-line-size because it
provides a cache-block-size and the spec says you don't have to if they are
the same. So the error is in the parsing not in the devicetree itself.
arch/powerpc/kernel/setup_64.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
index e05e6dd67ae6..dd8a238b54b8 100644
--- a/arch/powerpc/kernel/setup_64.c
+++ b/arch/powerpc/kernel/setup_64.c
@@ -516,6 +516,8 @@ static bool __init parse_cache_info(struct device_node *np,
lsizep = of_get_property(np, propnames[3], NULL);
if (bsizep == NULL)
bsizep = lsizep;
+ if (lsizep == NULL)
+ lsizep = bsizep;
if (lsizep != NULL)
lsize = be32_to_cpu(*lsizep);
if (bsizep != NULL)
--
2.25.1
^ permalink raw reply related
* Re: [PATCH 3/7] docs: fix broken references to text files
From: Stephen Boyd @ 2020-03-25 2:42 UTC (permalink / raw)
To: Linux Doc Mailing List, Mauro Carvalho Chehab
Cc: linux-arch, linux-nfs, kvm, Jonathan Corbet,
Mauro Carvalho Chehab, netdev, linux-unionfs, kvm-ppc, linux-mm,
dri-devel, linux-fsdevel, linuxppc-dev, kvmarm, linux-arm-kernel,
linux-rdma
In-Reply-To: <5cfeed6df208b74913312a1c97235ee615180f91.1582361737.git.mchehab+huawei@kernel.org>
Quoting Mauro Carvalho Chehab (2020-02-22 01:00:03)
> Several references got broken due to txt to ReST conversion.
>
> Several of them can be automatically fixed with:
>
> scripts/documentation-file-ref-check --fix
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> ---
> drivers/hwtracing/coresight/Kconfig | 2 +-
>
> diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig
> index 6ff30e25af55..6d42a6d3766f 100644
> --- a/drivers/hwtracing/coresight/Kconfig
> +++ b/drivers/hwtracing/coresight/Kconfig
> @@ -107,7 +107,7 @@ config CORESIGHT_CPU_DEBUG
> can quickly get to know program counter (PC), secure state,
> exception level, etc. Before use debugging functionality, platform
> needs to ensure the clock domain and power domain are enabled
> - properly, please refer Documentation/trace/coresight-cpu-debug.rst
> + properly, please refer Documentation/trace/coresight/coresight-cpu-debug.rst
> for detailed description and the example for usage.
>
> endif
I ran into this today and almost sent a patch. Can you split this patch
up into more pieces and send it off to the respective subsystem
maintainers?
^ permalink raw reply
* Re: [PATCH v8 11/14] powerpc/vas: Do not use default credits for receive window
From: Haren Myneni @ 2020-03-25 3:04 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: mikey, ajd, hch, oohall, sukadev, linuxppc-dev, herbert
In-Reply-To: <1584931140.88q80wx2at.astroid@bobo.none>
On Mon, 2020-03-23 at 12:40 +1000, Nicholas Piggin wrote:
> Haren Myneni's on March 19, 2020 4:18 pm:
> >
> > System checkstops if RxFIFO overruns with more requests than the
> > maximum possible number of CRBs allowed in FIFO at any time. So
> > max credits value (rxattr.wcreds_max) is set and is passed to
> > vas_rx_win_open() by the the driver.
>
> This seems like it should be a bug fix or merged in the NX fault
> window register patch or something.
Yes, it is a bug fix and can affect with any VAS windows, Not related to
NX fault window. Hence added as separate patch.
>
> Thanks,
> Nick
>
^ 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