* Re: [PATCH v2] powerpc/xive: Do not skip CPU-less nodes when creating the IPIs
From: Michael Ellerman @ 2021-08-13 11:57 UTC (permalink / raw)
To: linuxppc-dev, Cédric Le Goater
Cc: Laurent Vivier, Geetika Moolchandani, Srikar Dronamraju, stable
In-Reply-To: <20210807072057.184698-1-clg@kaod.org>
On Sat, 7 Aug 2021 09:20:57 +0200, Cédric Le Goater wrote:
> On PowerVM, CPU-less nodes can be populated with hot-plugged CPUs at
> runtime. Today, the IPI is not created for such nodes, and hot-plugged
> CPUs use a bogus IPI, which leads to soft lockups.
>
> We can not directly allocate and request the IPI on demand because
> bringup_up() is called under the IRQ sparse lock. The alternative is
> to allocate the IPIs for all possible nodes at startup and to request
> the mapping on demand when the first CPU of a node is brought up.
Applied to powerpc/fixes.
[1/1] powerpc/xive: Do not skip CPU-less nodes when creating the IPIs
https://git.kernel.org/powerpc/c/cbc06f051c524dcfe52ef0d1f30647828e226d30
cheers
^ permalink raw reply
* Re: [PATCH v2] powerpc/kprobes: Fix kprobe Oops happens in booke
From: Michael Ellerman @ 2021-08-13 11:57 UTC (permalink / raw)
To: mhiramat, benh, ruscur, naveen.n.rao, christophe.leroy, npiggin,
mpe, peterz, Pu Lehui, paulus
Cc: zhangjinhao2, linuxppc-dev, linux-kernel
In-Reply-To: <20210809023658.218915-1-pulehui@huawei.com>
On Mon, 9 Aug 2021 10:36:58 +0800, Pu Lehui wrote:
> When using kprobe on powerpc booke series processor, Oops happens
> as show bellow:
>
> / # echo "p:myprobe do_nanosleep" > /sys/kernel/debug/tracing/kprobe_events
> / # echo 1 > /sys/kernel/debug/tracing/events/kprobes/myprobe/enable
> / # sleep 1
> [ 50.076730] Oops: Exception in kernel mode, sig: 5 [#1]
> [ 50.077017] BE PAGE_SIZE=4K SMP NR_CPUS=24 QEMU e500
> [ 50.077221] Modules linked in:
> [ 50.077462] CPU: 0 PID: 77 Comm: sleep Not tainted 5.14.0-rc4-00022-g251a1524293d #21
> [ 50.077887] NIP: c0b9c4e0 LR: c00ebecc CTR: 00000000
> [ 50.078067] REGS: c3883de0 TRAP: 0700 Not tainted (5.14.0-rc4-00022-g251a1524293d)
> [ 50.078349] MSR: 00029000 <CE,EE,ME> CR: 24000228 XER: 20000000
> [ 50.078675]
> [ 50.078675] GPR00: c00ebdf0 c3883e90 c313e300 c3883ea0 00000001 00000000 c3883ecc 00000001
> [ 50.078675] GPR08: c100598c c00ea250 00000004 00000000 24000222 102490c2 bff4180c 101e60d4
> [ 50.078675] GPR16: 00000000 102454ac 00000040 10240000 10241100 102410f8 10240000 00500000
> [ 50.078675] GPR24: 00000002 00000000 c3883ea0 00000001 00000000 0000c350 3b9b8d50 00000000
> [ 50.080151] NIP [c0b9c4e0] do_nanosleep+0x0/0x190
> [ 50.080352] LR [c00ebecc] hrtimer_nanosleep+0x14c/0x1e0
> [ 50.080638] Call Trace:
> [ 50.080801] [c3883e90] [c00ebdf0] hrtimer_nanosleep+0x70/0x1e0 (unreliable)
> [ 50.081110] [c3883f00] [c00ec004] sys_nanosleep_time32+0xa4/0x110
> [ 50.081336] [c3883f40] [c001509c] ret_from_syscall+0x0/0x28
> [ 50.081541] --- interrupt: c00 at 0x100a4d08
> [ 50.081749] NIP: 100a4d08 LR: 101b5234 CTR: 00000003
> [ 50.081931] REGS: c3883f50 TRAP: 0c00 Not tainted (5.14.0-rc4-00022-g251a1524293d)
> [ 50.082183] MSR: 0002f902 <CE,EE,PR,FP,ME> CR: 24000222 XER: 00000000
> [ 50.082457]
> [ 50.082457] GPR00: 000000a2 bf980040 1024b4d0 bf980084 bf980084 64000000 00555345 fefefeff
> [ 50.082457] GPR08: 7f7f7f7f 101e0000 00000069 00000003 28000422 102490c2 bff4180c 101e60d4
> [ 50.082457] GPR16: 00000000 102454ac 00000040 10240000 10241100 102410f8 10240000 00500000
> [ 50.082457] GPR24: 00000002 bf9803f4 10240000 00000000 00000000 100039e0 00000000 102444e8
> [ 50.083789] NIP [100a4d08] 0x100a4d08
> [ 50.083917] LR [101b5234] 0x101b5234
> [ 50.084042] --- interrupt: c00
> [ 50.084238] Instruction dump:
> [ 50.084483] 4bfffc40 60000000 60000000 60000000 9421fff0 39400402 914200c0 38210010
> [ 50.084841] 4bfffc20 00000000 00000000 00000000 <7fe00008> 7c0802a6 7c892378 93c10048
> [ 50.085487] ---[ end trace f6fffe98e2fa8f3e ]---
> [ 50.085678]
> Trace/breakpoint trap
>
> [...]
Applied to powerpc/fixes.
[1/1] powerpc/kprobes: Fix kprobe Oops happens in booke
https://git.kernel.org/powerpc/c/43e8f76006592cb1573a959aa287c45421066f9c
cheers
^ permalink raw reply
* Re: [PATCH] powerpc/pseries: Fix update of LPAR security flavor after LPM
From: Michael Ellerman @ 2021-08-13 11:57 UTC (permalink / raw)
To: mpe, benh, paulus, Laurent Dufour; +Cc: linuxppc-dev, linux-kernel, stable
In-Reply-To: <20210805152308.33988-1-ldufour@linux.ibm.com>
On Thu, 5 Aug 2021 17:23:08 +0200, Laurent Dufour wrote:
> After LPM, when migrating from a system with security mitigation enabled to
> a system with mitigation disabled, the security flavor exposed in /proc is
> not correctly set back to 0.
>
> Do not assume the value of the security flavor is set to 0 when entering
> init_cpu_char_feature_flags(), so when called after a LPM, the value is set
> correctly even if the mitigation are not turned off.
>
> [...]
Applied to powerpc/fixes.
[1/1] powerpc/pseries: Fix update of LPAR security flavor after LPM
https://git.kernel.org/powerpc/c/c18956e6e0b95f78dad2773ecc8c61a9e41f6405
cheers
^ permalink raw reply
* Re: [PATCH] powerpc/smp: Fix OOPS in topology_init()
From: Michael Ellerman @ 2021-08-13 11:57 UTC (permalink / raw)
To: Michael Ellerman, Christophe Leroy, Benjamin Herrenschmidt,
Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <75287841cbb8740edd44880fe60be66d489160d9.1628097995.git.christophe.leroy@csgroup.eu>
On Wed, 4 Aug 2021 18:24:10 +0000 (UTC), Christophe Leroy wrote:
> Running an SMP kernel on an UP platform not prepared for it,
> I encountered the following OOPS:
>
> BUG: Kernel NULL pointer dereference on read at 0x00000034
> Faulting instruction address: 0xc0a04110
> Oops: Kernel access of bad area, sig: 11 [#1]
> BE PAGE_SIZE=4K SMP NR_CPUS=2 CMPCPRO
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.13.0-pmac-00001-g230fedfaad21 #5234
> NIP: c0a04110 LR: c0a040d8 CTR: c0a04084
> REGS: e100dda0 TRAP: 0300 Not tainted (5.13.0-pmac-00001-g230fedfaad21)
> MSR: 00009032 <EE,ME,IR,DR,RI> CR: 84000284 XER: 00000000
> DAR: 00000034 DSISR: 20000000
> GPR00: c0006bd4 e100de60 c1033320 00000000 00000000 c0942274 00000000 00000000
> GPR08: 00000000 00000000 00000001 00000063 00000007 00000000 c0006f30 00000000
> GPR16: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000005
> GPR24: c0c67d74 c0c67f1c c0c60000 c0c67d70 c0c0c558 1efdf000 c0c00020 00000000
> NIP [c0a04110] topology_init+0x8c/0x138
> LR [c0a040d8] topology_init+0x54/0x138
> Call Trace:
> [e100de60] [80808080] 0x80808080 (unreliable)
> [e100de90] [c0006bd4] do_one_initcall+0x48/0x1bc
> [e100def0] [c0a0150c] kernel_init_freeable+0x1c8/0x278
> [e100df20] [c0006f44] kernel_init+0x14/0x10c
> [e100df30] [c00190fc] ret_from_kernel_thread+0x14/0x1c
> Instruction dump:
> 7c692e70 7d290194 7c035040 7c7f1b78 5529103a 546706fe 5468103a 39400001
> 7c641b78 40800054 80c690b4 7fb9402e <81060034> 7fbeea14 2c080000 7fa3eb78
> ---[ end trace b246ffbc6bbbb6fb ]---
>
> [...]
Applied to powerpc/fixes.
[1/1] powerpc/smp: Fix OOPS in topology_init()
https://git.kernel.org/powerpc/c/8241461536f21bbe51308a6916d1c9fb2e6b75a7
cheers
^ permalink raw reply
* Re: [PATCH] powerpc/32s: Fix napping restore in data storage interrupt (DSI)
From: Michael Ellerman @ 2021-08-13 11:57 UTC (permalink / raw)
To: Finn Thain, Benjamin Herrenschmidt, Michael Ellerman, userm57,
Christophe Leroy, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <731694e0885271f6ee9ffc179eb4bcee78313682.1628003562.git.christophe.leroy@csgroup.eu>
On Tue, 3 Aug 2021 15:14:27 +0000 (UTC), Christophe Leroy wrote:
> When a DSI (Data Storage Interrupt) is taken while in NAP mode,
> r11 doesn't survive the call to power_save_ppc32_restore().
>
> So use r1 instead of r11 as they both contain the virtual stack
> pointer at that point.
Applied to powerpc/fixes.
[1/1] powerpc/32s: Fix napping restore in data storage interrupt (DSI)
https://git.kernel.org/powerpc/c/62376365048878f770d8b7d11b89b8b3e18018f1
cheers
^ permalink raw reply
* Re: [PATCH] powerpc/32: Fix critical and debug interrupts on BOOKE
From: Michael Ellerman @ 2021-08-13 11:57 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Michael Ellerman, radu.rendec,
Christophe Leroy, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <028d5483b4851b01ea4334d0751e7f260419092b.1625637264.git.christophe.leroy@csgroup.eu>
On Wed, 7 Jul 2021 05:55:07 +0000 (UTC), Christophe Leroy wrote:
> 32 bits BOOKE have special interrupts for debug and other
> critical events.
>
> When handling those interrupts, dedicated registers are saved
> in the stack frame in addition to the standard registers, leading
> to a shift of the pt_regs struct.
>
> [...]
Applied to powerpc/fixes.
[1/1] powerpc/32: Fix critical and debug interrupts on BOOKE
https://git.kernel.org/powerpc/c/b5cfc9cd7b0426e94ffd9e9ed79d1b00ace7780a
cheers
^ permalink raw reply
* Re: [PATCH] powerpc/interrupt: Fix OOPS by not calling do_IRQ() from timer_interrupt()
From: Michael Ellerman @ 2021-08-13 11:57 UTC (permalink / raw)
To: Benjamin Herrenschmidt, fthain, npiggin, Michael Ellerman,
userm57, Christophe Leroy, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <c17d234f4927d39a1d7100864a8e1145323d33a0.1628611927.git.christophe.leroy@csgroup.eu>
On Tue, 10 Aug 2021 16:13:16 +0000 (UTC), Christophe Leroy wrote:
> An interrupt handler shall not be called from another interrupt
> handler otherwise this leads to problems like the following:
>
> Kernel attempted to write user page (afd4fa84) - exploit attempt? (uid: 1000)
> ------------[ cut here ]------------
> Bug: Write fault blocked by KUAP!
> WARNING: CPU: 0 PID: 1617 at arch/powerpc/mm/fault.c:230 do_page_fault+0x484/0x720
> Modules linked in:
> CPU: 0 PID: 1617 Comm: sshd Tainted: G W 5.13.0-pmac-00010-g8393422eb77 #7
> NIP: c001b77c LR: c001b77c CTR: 00000000
> REGS: cb9e5bc0 TRAP: 0700 Tainted: G W (5.13.0-pmac-00010-g8393422eb77)
> MSR: 00021032 <ME,IR,DR,RI> CR: 24942424 XER: 00000000
>
> [...]
Applied to powerpc/fixes.
[1/1] powerpc/interrupt: Fix OOPS by not calling do_IRQ() from timer_interrupt()
https://git.kernel.org/powerpc/c/98694166c27d473c36b434bd3572934c2f2a16ab
cheers
^ permalink raw reply
* Re: [PATCH] powerpc/interrupt: Do not call single_step_exception() from other exceptions
From: Michael Ellerman @ 2021-08-13 11:57 UTC (permalink / raw)
To: Benjamin Herrenschmidt, fthain, npiggin, Michael Ellerman,
userm57, Christophe Leroy, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <aed174f5cbc06f2cf95233c071d8aac948e46043.1628611921.git.christophe.leroy@csgroup.eu>
On Tue, 10 Aug 2021 16:13:17 +0000 (UTC), Christophe Leroy wrote:
> single_step_exception() is called by emulate_single_step() which
> is called from (at least) alignment exception() handler and
> program_check_exception() handler.
>
> Redefine it as a regular __single_step_exception() which is called
> by both single_step_exception() handler and emulate_single_step()
> function.
Applied to powerpc/fixes.
[1/1] powerpc/interrupt: Do not call single_step_exception() from other exceptions
https://git.kernel.org/powerpc/c/01fcac8e4dfc112f420dcaeb70056a74e326cacf
cheers
^ permalink raw reply
* Re: [PATCH v6 1/2] tty: hvc: pass DMA capable memory to put_chars()
From: Xianting TIan @ 2021-08-13 11:27 UTC (permalink / raw)
To: Greg KH
Cc: arnd, amit, jirislaby, linux-kernel, virtualization, linuxppc-dev,
osandov
In-Reply-To: <YRYeyeZ/22rR26u7@kroah.com>
在 2021/8/13 下午3:27, Greg KH 写道:
> On Thu, Aug 12, 2021 at 05:45:31PM +0800, Xianting Tian wrote:
>> As well known, hvc backend can register its opertions to hvc backend.
>> the opertions contain put_chars(), get_chars() and so on.
>>
>> Some hvc backend may do dma in its opertions. eg, put_chars() of
>> virtio-console. But in the code of hvc framework, it may pass DMA
>> incapable memory to put_chars() under a specific configuration, which
>> is explained in commit c4baad5029(virtio-console: avoid DMA from stack):
>> 1, c[] is on stack,
>> hvc_console_print():
>> char c[N_OUTBUF] __ALIGNED__;
>> cons_ops[index]->put_chars(vtermnos[index], c, i);
>> 2, ch is on stack,
>> static void hvc_poll_put_char(,,char ch)
>> {
>> struct tty_struct *tty = driver->ttys[0];
>> struct hvc_struct *hp = tty->driver_data;
>> int n;
>>
>> do {
>> n = hp->ops->put_chars(hp->vtermno, &ch, 1);
>> } while (n <= 0);
>> }
>>
>> Commit c4baad5029 is just the fix to avoid DMA from stack memory, which
>> is passed to virtio-console by hvc framework in above code. But I think
>> the fix is aggressive, it directly uses kmemdup() to alloc new buffer
>> from kmalloc area and do memcpy no matter the memory is in kmalloc area
>> or not. But most importantly, it should better be fixed in the hvc
>> framework, by changing it to never pass stack memory to the put_chars()
>> function in the first place. Otherwise, we still face the same issue if
>> a new hvc backend using dma added in the furture.
>>
>> We make 'char c[N_OUTBUF]' part of 'struct hvc_struct', so hp->c is no
>> longer the stack memory. we can use it in above two cases.
>>
>> Other fix is use L1_CACHE_BYTES as the alignment, use 'sizeof(long)' as
>> dma alignment is wrong. And use struct_size() to calculate size of
>> hvc_struct.
>>
>> Introduce another array(cons_outbuf[]) for the hp->c pointers next to
>> the cons_ops[] and vtermnos[] arrays.
>>
>> With the patch, we can remove the fix c4baad5029.
>>
>> Signed-off-by: Xianting Tian <xianting.tian@linux.alibaba.com>
>> Tested-by: Xianting Tian <xianting.tian@linux.alibaba.com>
> As the build shows, you obviously did not test this code :(
>
> Also, no need to add a tested-by line as that should be implicit if you
> wrote and signed off on it.
>
> I am going to ask you to get some help from some other developers at
> your company, and get them to test and sign off on this series before
> sending it out again, as there seems to be a bit of a disconnect as to
> what is actually needed to do when sending a patch for us to review.
>
> That is now a requirement for us to be able to take your changes here.
>
> thanks,
Sorry for this.
I tested V1-V4, But for V6, I take it for granted that there is no
problem when I just switch to use array(cons_outbuf[]). I indeed didn't
test it:(
I will test it and find virtualization test expert to test again before
sending next patch.
>
> greg k-h
^ permalink raw reply
* Re: [PATCH kernel] KVM: PPC: Book3S HV: Make unique debugfs nodename
From: Alexey Kardashevskiy @ 2021-08-13 9:50 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev, linux-kernel, kvm-ppc
In-Reply-To: <20210707041344.3803554-1-aik@ozlabs.ru>
On 07/07/2021 14:13, Alexey Kardashevskiy wrote:
> Currently it is vm-$currentpid which works as long as there is just one
> VM per the userspace (99.99% cases) but produces a bunch
> of "debugfs: Directory 'vm16679' with parent 'kvm' already present!"
> when syzkaller (syscall fuzzer) is running so only one VM is present in
> the debugfs for a given process.
>
> This changes the debugfs node to include the LPID which alone should be
> system wide unique. This leaves the existing pid for the convenience of
> matching the VM's debugfs with the running userspace process (QEMU).
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Looks like this is not enough as syzkaller still manages to cause the
error message, I need more robust approach as in
https://lore.kernel.org/patchwork/patch/1472025/ or alternatively
move this debugfs stuff under the platform-independent directory, how
about that?
> ---
> arch/powerpc/kvm/book3s_hv.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 1d1fcc290fca..0223ddc0eed0 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -5227,7 +5227,7 @@ static int kvmppc_core_init_vm_hv(struct kvm *kvm)
> /*
> * Create a debugfs directory for the VM
> */
> - snprintf(buf, sizeof(buf), "vm%d", current->pid);
> + snprintf(buf, sizeof(buf), "vm%d-lp%ld", current->pid, lpid);
> kvm->arch.debugfs_dir = debugfs_create_dir(buf, kvm_debugfs_dir);
> kvmppc_mmu_debugfs_init(kvm);
> if (radix_enabled())
>
--
Alexey
^ permalink raw reply
* Re: [PATCH v2 2/2] powerpc/perf: Return regs->nip as instruction pointer value when SIAR is 0
From: Christophe Leroy @ 2021-08-13 9:34 UTC (permalink / raw)
To: Kajol Jain, mpe, linuxppc-dev; +Cc: atrajeev, maddy, rnsastry
In-Reply-To: <20210813082450.429320-2-kjain@linux.ibm.com>
Le 13/08/2021 à 10:24, Kajol Jain a écrit :
> Incase of random sampling, there can be scenarios where SIAR is not
> latching sample address and results in 0 value. Since current code
> directly returning the siar value, we could see multiple instruction
> pointer values as 0 in perf report.
> Patch resolves this issue by adding a ternary condition to return
> regs->nip incase SIAR is 0.
Your description seems rather similar to
https://github.com/linuxppc/linux/commit/2ca13a4cc56c920a6c9fc8ee45d02bccacd7f46c
Does it mean that the problem occurs on more than the power10 DD1 ?
In that case, can the solution be common instead of doing something for power10 DD1 and something
for others ?
>
> Fixes: 75382aa72f06 ("powerpc/perf: Move code to select SIAR or pt_regs
> into perf_read_regs")
> Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
> ---
> arch/powerpc/perf/core-book3s.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
> index 1b464aad29c4..aeecaaf6810f 100644
> --- a/arch/powerpc/perf/core-book3s.c
> +++ b/arch/powerpc/perf/core-book3s.c
> @@ -2260,7 +2260,7 @@ unsigned long perf_instruction_pointer(struct pt_regs *regs)
> else
> return regs->nip;
> } else if (use_siar && siar_valid(regs))
> - return siar + perf_ip_adjust(regs);
> + return siar ? siar + perf_ip_adjust(regs) : regs->nip;
> else if (use_siar)
> return 0; // no valid instruction pointer
> else
>
^ permalink raw reply
* Re: [PATCH v2 2/2] powerpc/perf: Return regs->nip as instruction pointer value when SIAR is 0
From: Christophe Leroy @ 2021-08-13 9:29 UTC (permalink / raw)
To: Kajol Jain, mpe, linuxppc-dev; +Cc: atrajeev, maddy, rnsastry
In-Reply-To: <20210813082450.429320-2-kjain@linux.ibm.com>
Le 13/08/2021 à 10:24, Kajol Jain a écrit :
> Incase of random sampling, there can be scenarios where SIAR is not
> latching sample address and results in 0 value. Since current code
> directly returning the siar value, we could see multiple instruction
> pointer values as 0 in perf report.
> Patch resolves this issue by adding a ternary condition to return
> regs->nip incase SIAR is 0.
>
> Fixes: 75382aa72f06 ("powerpc/perf: Move code to select SIAR or pt_regs
> into perf_read_regs")
> Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
> ---
> arch/powerpc/perf/core-book3s.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
> index 1b464aad29c4..aeecaaf6810f 100644
> --- a/arch/powerpc/perf/core-book3s.c
> +++ b/arch/powerpc/perf/core-book3s.c
> @@ -2260,7 +2260,7 @@ unsigned long perf_instruction_pointer(struct pt_regs *regs)
> else
> return regs->nip;
> } else if (use_siar && siar_valid(regs))
> - return siar + perf_ip_adjust(regs);
> + return siar ? siar + perf_ip_adjust(regs) : regs->nip;
Why bother about returning SIAR at all if regs->nip is ok ? Why not just return regs->nip all the time ?
> else if (use_siar)
> return 0; // no valid instruction pointer
> else
>
^ permalink raw reply
* Re: [PATCH v2 1/2] powerpc/perf: Use stack siar instead of mfspr
From: Christophe Leroy @ 2021-08-13 9:23 UTC (permalink / raw)
To: kajoljain, mpe, linuxppc-dev; +Cc: atrajeev, maddy, rnsastry
In-Reply-To: <37c6279f-a05c-7dd9-b034-05ca524bc6b0@linux.ibm.com>
Le 13/08/2021 à 10:29, kajoljain a écrit :
>
>
> On 8/13/21 1:54 PM, Kajol Jain wrote:
>> Minor optimization in the 'perf_instruction_pointer' function code by
>> making use of stack siar instead of mfspr.
>>
>> Fixes: 75382aa72f06 ("powerpc/perf: Move code to select SIAR or pt_regs
>> into perf_read_regs")
>> Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
>
> Please ignore this patch-set as I mentioned wrong version number. I will resend
> this patch-set again with correct version. Sorry for the confusion.
I fear you are creating even more confusion by sending a v1 after sending a v2 ...
Christophe
^ permalink raw reply
* [PATCH 2/2] powerpc/perf: Return regs->nip as instruction pointer value when SIAR is 0
From: Kajol Jain @ 2021-08-13 8:30 UTC (permalink / raw)
To: mpe, linuxppc-dev; +Cc: kjain, atrajeev, maddy, rnsastry
In-Reply-To: <20210813083037.431299-1-kjain@linux.ibm.com>
Incase of random sampling, there can be scenarios where SIAR is not
latching sample address and results in 0 value. Since current code
directly returning the siar value, we could see multiple instruction
pointer values as 0 in perf report.
Patch resolves this issue by adding a ternary condition to return
regs->nip incase SIAR is 0.
Fixes: 75382aa72f06 ("powerpc/perf: Move code to select SIAR or pt_regs
into perf_read_regs")
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
arch/powerpc/perf/core-book3s.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 1b464aad29c4..aeecaaf6810f 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -2260,7 +2260,7 @@ unsigned long perf_instruction_pointer(struct pt_regs *regs)
else
return regs->nip;
} else if (use_siar && siar_valid(regs))
- return siar + perf_ip_adjust(regs);
+ return siar ? siar + perf_ip_adjust(regs) : regs->nip;
else if (use_siar)
return 0; // no valid instruction pointer
else
--
2.26.2
^ permalink raw reply related
* [PATCH 1/2] powerpc/perf: Use stack siar instead of mfspr
From: Kajol Jain @ 2021-08-13 8:30 UTC (permalink / raw)
To: mpe, linuxppc-dev; +Cc: kjain, atrajeev, maddy, rnsastry
Minor optimization in the 'perf_instruction_pointer' function code by
making use of stack siar instead of mfspr.
Fixes: 75382aa72f06 ("powerpc/perf: Move code to select SIAR or pt_regs
into perf_read_regs")
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
arch/powerpc/perf/core-book3s.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index bb0ee716de91..1b464aad29c4 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -2260,7 +2260,7 @@ unsigned long perf_instruction_pointer(struct pt_regs *regs)
else
return regs->nip;
} else if (use_siar && siar_valid(regs))
- return mfspr(SPRN_SIAR) + perf_ip_adjust(regs);
+ return siar + perf_ip_adjust(regs);
else if (use_siar)
return 0; // no valid instruction pointer
else
--
2.26.2
^ permalink raw reply related
* Re: [PATCH v2 1/2] powerpc/perf: Use stack siar instead of mfspr
From: kajoljain @ 2021-08-13 8:29 UTC (permalink / raw)
To: mpe, linuxppc-dev; +Cc: atrajeev, maddy, rnsastry
In-Reply-To: <20210813082450.429320-1-kjain@linux.ibm.com>
On 8/13/21 1:54 PM, Kajol Jain wrote:
> Minor optimization in the 'perf_instruction_pointer' function code by
> making use of stack siar instead of mfspr.
>
> Fixes: 75382aa72f06 ("powerpc/perf: Move code to select SIAR or pt_regs
> into perf_read_regs")
> Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
Please ignore this patch-set as I mentioned wrong version number. I will resend
this patch-set again with correct version. Sorry for the confusion.
Thanks,
Kajol Jain
> ---
> arch/powerpc/perf/core-book3s.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
> index bb0ee716de91..1b464aad29c4 100644
> --- a/arch/powerpc/perf/core-book3s.c
> +++ b/arch/powerpc/perf/core-book3s.c
> @@ -2260,7 +2260,7 @@ unsigned long perf_instruction_pointer(struct pt_regs *regs)
> else
> return regs->nip;
> } else if (use_siar && siar_valid(regs))
> - return mfspr(SPRN_SIAR) + perf_ip_adjust(regs);
> + return siar + perf_ip_adjust(regs);
> else if (use_siar)
> return 0; // no valid instruction pointer
> else
>
^ permalink raw reply
* [PATCH v2 2/2] powerpc/perf: Return regs->nip as instruction pointer value when SIAR is 0
From: Kajol Jain @ 2021-08-13 8:24 UTC (permalink / raw)
To: mpe, linuxppc-dev; +Cc: kjain, atrajeev, maddy, rnsastry
In-Reply-To: <20210813082450.429320-1-kjain@linux.ibm.com>
Incase of random sampling, there can be scenarios where SIAR is not
latching sample address and results in 0 value. Since current code
directly returning the siar value, we could see multiple instruction
pointer values as 0 in perf report.
Patch resolves this issue by adding a ternary condition to return
regs->nip incase SIAR is 0.
Fixes: 75382aa72f06 ("powerpc/perf: Move code to select SIAR or pt_regs
into perf_read_regs")
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
arch/powerpc/perf/core-book3s.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 1b464aad29c4..aeecaaf6810f 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -2260,7 +2260,7 @@ unsigned long perf_instruction_pointer(struct pt_regs *regs)
else
return regs->nip;
} else if (use_siar && siar_valid(regs))
- return siar + perf_ip_adjust(regs);
+ return siar ? siar + perf_ip_adjust(regs) : regs->nip;
else if (use_siar)
return 0; // no valid instruction pointer
else
--
2.26.2
^ permalink raw reply related
* [PATCH v2 1/2] powerpc/perf: Use stack siar instead of mfspr
From: Kajol Jain @ 2021-08-13 8:24 UTC (permalink / raw)
To: mpe, linuxppc-dev; +Cc: kjain, atrajeev, maddy, rnsastry
Minor optimization in the 'perf_instruction_pointer' function code by
making use of stack siar instead of mfspr.
Fixes: 75382aa72f06 ("powerpc/perf: Move code to select SIAR or pt_regs
into perf_read_regs")
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
arch/powerpc/perf/core-book3s.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index bb0ee716de91..1b464aad29c4 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -2260,7 +2260,7 @@ unsigned long perf_instruction_pointer(struct pt_regs *regs)
else
return regs->nip;
} else if (use_siar && siar_valid(regs))
- return mfspr(SPRN_SIAR) + perf_ip_adjust(regs);
+ return siar + perf_ip_adjust(regs);
else if (use_siar)
return 0; // no valid instruction pointer
else
--
2.26.2
^ permalink raw reply related
* [PATCH] powerpc/perf/hv-gpci: Fix the logic to compute counter value from the hcall result buffer.
From: Kajol Jain @ 2021-08-13 8:21 UTC (permalink / raw)
To: mpe, linuxppc-dev; +Cc: kjain, suka, maddy, rnsastry, atrajeev
H_GetPerformanceCounterInfo (0xF080) hcall returns the counter data in the
result buffer. Result buffer has specific format defined in the PAPR
specification. One of the field is counter offset and width of the counter
data returned.
Counter data are returned in a unsigned char array. To
get the final counter data, these values should be left shifted
byte at a time. But commit 220a0c609ad17 ("powerpc/perf: Add support
for the hv gpci (get performance counter info) interface") made the
shifting bitwise. Because of this, hcall counters values could end up
in lower side, which messes the counter prev vs now calculation. This
lead to huge counter value reporting
[command]#: perf stat -e hv_gpci/system_tlbie_count_and_time_tlbie_instructions_issued/
-C 0 -I 1000
time counts unit events
1.000078854 18,446,744,073,709,535,232 hv_gpci/system_tlbie_count_and_time_tlbie_instructions_issued/
2.000213293 0 hv_gpci/system_tlbie_count_and_time_tlbie_instructions_issued/
3.000320107 0 hv_gpci/system_tlbie_count_and_time_tlbie_instructions_issued/
4.000428392 0 hv_gpci/system_tlbie_count_and_time_tlbie_instructions_issued/
5.000537864 0 hv_gpci/system_tlbie_count_and_time_tlbie_instructions_issued/
6.000649087 0 hv_gpci/system_tlbie_count_and_time_tlbie_instructions_issued/
7.000760312 0 hv_gpci/system_tlbie_count_and_time_tlbie_instructions_issued/
8.000865218 16,448 hv_gpci/system_tlbie_count_and_time_tlbie_instructions_issued/
9.000978985 18,446,744,073,709,535,232 hv_gpci/system_tlbie_count_and_time_tlbie_instructions_issued/
10.001088891 16,384 hv_gpci/system_tlbie_count_and_time_tlbie_instructions_issued/
11.001201435 0 hv_gpci/system_tlbie_count_and_time_tlbie_instructions_issued/
12.001307937 18,446,744,073,709,535,232 hv_gpci/system_tlbie_count_and_time_tlbie_instructions_issued/
Patch here fixes the shifting logic to make is byte-wise with which no more the issue seen.
Fixes: e4f226b1580b3 ("powerpc/perf/hv-gpci: Increase request buffer size")
Reported-by: Nageswara R Sastry<rnsastry@linux.ibm.com>
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
arch/powerpc/perf/hv-gpci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/perf/hv-gpci.c b/arch/powerpc/perf/hv-gpci.c
index d48413e28c39..c756228a081f 100644
--- a/arch/powerpc/perf/hv-gpci.c
+++ b/arch/powerpc/perf/hv-gpci.c
@@ -175,7 +175,7 @@ static unsigned long single_gpci_request(u32 req, u32 starting_index,
*/
count = 0;
for (i = offset; i < offset + length; i++)
- count |= arg->bytes[i] << (i - offset);
+ count |= (u64)(arg->bytes[i]) << ((length - 1 - (i - offset)) * 8);
*value = count;
out:
--
2.26.2
^ permalink raw reply related
* Re: [PATCH v2 1/2] powerpc/book3s64/radix: make tlb_single_page_flush_ceiling a debugfs entry
From: Daniel Axtens @ 2021-08-13 7:39 UTC (permalink / raw)
To: Aneesh Kumar K.V, linuxppc-dev, mpe; +Cc: Aneesh Kumar K.V
In-Reply-To: <20210812132831.233794-1-aneesh.kumar@linux.ibm.com>
"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
> Similar to x86/s390 add a debugfs file to tune tlb_single_page_flush_ceiling.
> Also add a debugfs entry for tlb_local_single_page_flush_ceiling.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
> Changes from v1:
> * switch to debugfs_create_u32
>
> arch/powerpc/mm/book3s64/radix_tlb.c | 16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/mm/book3s64/radix_tlb.c b/arch/powerpc/mm/book3s64/radix_tlb.c
> index aefc100d79a7..1fa2bc6a969e 100644
> --- a/arch/powerpc/mm/book3s64/radix_tlb.c
> +++ b/arch/powerpc/mm/book3s64/radix_tlb.c
> @@ -17,6 +17,7 @@
> #include <asm/trace.h>
> #include <asm/cputhreads.h>
> #include <asm/plpar_wrappers.h>
> +#include <asm/debugfs.h>
>
> #include "internal.h"
>
> @@ -1106,8 +1107,8 @@ EXPORT_SYMBOL(radix__flush_tlb_kernel_range);
> * invalidating a full PID, so it has a far lower threshold to change from
> * individual page flushes to full-pid flushes.
> */
> -static unsigned long tlb_single_page_flush_ceiling __read_mostly = 33;
> -static unsigned long tlb_local_single_page_flush_ceiling __read_mostly = POWER9_TLB_SETS_RADIX * 2;
> +static u32 tlb_single_page_flush_ceiling __read_mostly = 33;
> +static u32 tlb_local_single_page_flush_ceiling __read_mostly = POWER9_TLB_SETS_RADIX * 2;
>
> static inline void __radix__flush_tlb_range(struct mm_struct *mm,
> unsigned long start, unsigned long end)
> @@ -1524,3 +1525,14 @@ void do_h_rpt_invalidate_prt(unsigned long pid, unsigned long lpid,
> EXPORT_SYMBOL_GPL(do_h_rpt_invalidate_prt);
>
> #endif /* CONFIG_KVM_BOOK3S_HV_POSSIBLE */
> +
> +static int __init create_tlb_single_page_flush_ceiling(void)
> +{
> + debugfs_create_u32("tlb_single_page_flush_ceiling", 0600,
> + powerpc_debugfs_root, &tlb_single_page_flush_ceiling);
> + debugfs_create_u32("tlb_local_single_page_flush_ceiling", 0600,
> + powerpc_debugfs_root, &tlb_local_single_page_flush_ceiling);
> + return 0;
> +}
> +late_initcall(create_tlb_single_page_flush_ceiling);
This patch seems to do what the commit message says, and it does seem to
make sense to have these parameters as tunables.
I was briefly concerned that switching from an unsigned long to a u32
might lead to suboptimal code generation in older gcc versions, but it
doesn't seem to be a case where a single instruction is going to make a
huge impact.
I also wondered what the C integer promotion rules would do with a the
nr_pages > tlb*flush_ceiling comparisons, but if we are trying to flush
more than 4 billion pages we might have other, bigger problems! (Also,
if I understand the C integer rules correctly the u32 will get promoted
to an unsigned long anyway.)
All in all this seems good to me.
Reviewed-by: Daniel Axtens <dja@axtens.net>
Kind regards,
Daniel
> +
> --
> 2.31.1
^ permalink raw reply
* Re: [PATCH v6 1/2] tty: hvc: pass DMA capable memory to put_chars()
From: Greg KH @ 2021-08-13 7:27 UTC (permalink / raw)
To: Xianting Tian
Cc: arnd, amit, jirislaby, linux-kernel, virtualization, linuxppc-dev,
osandov
In-Reply-To: <20210812094532.145497-2-xianting.tian@linux.alibaba.com>
On Thu, Aug 12, 2021 at 05:45:31PM +0800, Xianting Tian wrote:
> As well known, hvc backend can register its opertions to hvc backend.
> the opertions contain put_chars(), get_chars() and so on.
>
> Some hvc backend may do dma in its opertions. eg, put_chars() of
> virtio-console. But in the code of hvc framework, it may pass DMA
> incapable memory to put_chars() under a specific configuration, which
> is explained in commit c4baad5029(virtio-console: avoid DMA from stack):
> 1, c[] is on stack,
> hvc_console_print():
> char c[N_OUTBUF] __ALIGNED__;
> cons_ops[index]->put_chars(vtermnos[index], c, i);
> 2, ch is on stack,
> static void hvc_poll_put_char(,,char ch)
> {
> struct tty_struct *tty = driver->ttys[0];
> struct hvc_struct *hp = tty->driver_data;
> int n;
>
> do {
> n = hp->ops->put_chars(hp->vtermno, &ch, 1);
> } while (n <= 0);
> }
>
> Commit c4baad5029 is just the fix to avoid DMA from stack memory, which
> is passed to virtio-console by hvc framework in above code. But I think
> the fix is aggressive, it directly uses kmemdup() to alloc new buffer
> from kmalloc area and do memcpy no matter the memory is in kmalloc area
> or not. But most importantly, it should better be fixed in the hvc
> framework, by changing it to never pass stack memory to the put_chars()
> function in the first place. Otherwise, we still face the same issue if
> a new hvc backend using dma added in the furture.
>
> We make 'char c[N_OUTBUF]' part of 'struct hvc_struct', so hp->c is no
> longer the stack memory. we can use it in above two cases.
>
> Other fix is use L1_CACHE_BYTES as the alignment, use 'sizeof(long)' as
> dma alignment is wrong. And use struct_size() to calculate size of
> hvc_struct.
>
> Introduce another array(cons_outbuf[]) for the hp->c pointers next to
> the cons_ops[] and vtermnos[] arrays.
>
> With the patch, we can remove the fix c4baad5029.
>
> Signed-off-by: Xianting Tian <xianting.tian@linux.alibaba.com>
> Tested-by: Xianting Tian <xianting.tian@linux.alibaba.com>
As the build shows, you obviously did not test this code :(
Also, no need to add a tested-by line as that should be implicit if you
wrote and signed off on it.
I am going to ask you to get some help from some other developers at
your company, and get them to test and sign off on this series before
sending it out again, as there seems to be a bit of a disconnect as to
what is actually needed to do when sending a patch for us to review.
That is now a requirement for us to be able to take your changes here.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v2 2/2] powerpc/bug: Provide better flexibility to WARN_ON/__WARN_FLAGS() with asm goto
From: Nicholas Piggin @ 2021-08-13 6:19 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Christophe Leroy, Michael Ellerman,
Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <389962b1b702e3c78d169e59bcfac56282889173.1618331882.git.christophe.leroy@csgroup.eu>
Excerpts from Christophe Leroy's message of April 14, 2021 2:38 am:
> Using asm goto in __WARN_FLAGS() and WARN_ON() allows more
> flexibility to GCC.
>
> For that add an entry to the exception table so that
> program_check_exception() knowns where to resume execution
> after a WARNING.
Nice idea. How much does it bloat the exception table?
How easy would it be to make a different exception table for
unimportant stuff like WARN_ON faults?
>
> Here are two exemples. The first one is done on PPC32 (which
> benefits from the previous patch), the second is on PPC64.
>
> unsigned long test(struct pt_regs *regs)
> {
> int ret;
>
> WARN_ON(regs->msr & MSR_PR);
>
> return regs->gpr[3];
> }
>
> unsigned long test9w(unsigned long a, unsigned long b)
> {
> if (WARN_ON(!b))
> return 0;
> return a / b;
> }
>
> Before the patch:
>
> 000003a8 <test>:
> 3a8: 81 23 00 84 lwz r9,132(r3)
> 3ac: 71 29 40 00 andi. r9,r9,16384
> 3b0: 40 82 00 0c bne 3bc <test+0x14>
> 3b4: 80 63 00 0c lwz r3,12(r3)
> 3b8: 4e 80 00 20 blr
>
> 3bc: 0f e0 00 00 twui r0,0
> 3c0: 80 63 00 0c lwz r3,12(r3)
> 3c4: 4e 80 00 20 blr
>
> 0000000000000bf0 <.test9w>:
> bf0: 7c 89 00 74 cntlzd r9,r4
> bf4: 79 29 d1 82 rldicl r9,r9,58,6
> bf8: 0b 09 00 00 tdnei r9,0
> bfc: 2c 24 00 00 cmpdi r4,0
> c00: 41 82 00 0c beq c0c <.test9w+0x1c>
> c04: 7c 63 23 92 divdu r3,r3,r4
> c08: 4e 80 00 20 blr
>
> c0c: 38 60 00 00 li r3,0
> c10: 4e 80 00 20 blr
>
> After the patch:
>
> 000003a8 <test>:
> 3a8: 81 23 00 84 lwz r9,132(r3)
> 3ac: 71 29 40 00 andi. r9,r9,16384
> 3b0: 40 82 00 0c bne 3bc <test+0x14>
> 3b4: 80 63 00 0c lwz r3,12(r3)
> 3b8: 4e 80 00 20 blr
>
> 3bc: 0f e0 00 00 twui r0,0
>
> 0000000000000c50 <.test9w>:
> c50: 7c 89 00 74 cntlzd r9,r4
> c54: 79 29 d1 82 rldicl r9,r9,58,6
> c58: 0b 09 00 00 tdnei r9,0
> c5c: 7c 63 23 92 divdu r3,r3,r4
> c60: 4e 80 00 20 blr
>
> c70: 38 60 00 00 li r3,0
> c74: 4e 80 00 20 blr
One thing that would be nice for WARN_ON_ONCE is to patch out the trap
after the program interrupt. I've seen sometimes the WARN_ON_ONCE starts
firing a lot and slows down the kernel. That gets harder to do if the
trap is now part of the control flow.
I guess that's less important case for performance though. And in theory
you might be able to replace the trap with an equivalent cmp and branch
(but that's probably going too over engineering it).
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH v2 1/2] powerpc/bug: Remove specific powerpc BUG_ON() and WARN_ON() on PPC32
From: Nicholas Piggin @ 2021-08-13 6:08 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Christophe Leroy, Michael Ellerman,
Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <b286e07fb771a664b631cd07a40b09c06f26e64b.1618331881.git.christophe.leroy@csgroup.eu>
Excerpts from Christophe Leroy's message of April 14, 2021 2:38 am:
> powerpc BUG_ON() and WARN_ON() are based on using twnei instruction.
>
> For catching simple conditions like a variable having value 0, this
> is efficient because it does the test and the trap at the same time.
> But most conditions used with BUG_ON or WARN_ON are more complex and
> forces GCC to format the condition into a 0 or 1 value in a register.
> This will usually require 2 to 3 instructions.
>
> The most efficient solution would be to use __builtin_trap() because
> GCC is able to optimise the use of the different trap instructions
> based on the requested condition, but this is complex if not
> impossible for the following reasons:
> - __builtin_trap() is a non-recoverable instruction, so it can't be
> used for WARN_ON
> - Knowing which line of code generated the trap would require the
> analysis of DWARF information. This is not a feature we have today.
>
> As mentioned in commit 8d4fbcfbe0a4 ("Fix WARN_ON() on bitfield ops")
> the way WARN_ON() is implemented is suboptimal. That commit also
> mentions an issue with 'long long' condition. It fixed it for
> WARN_ON() but the same problem still exists today with BUG_ON() on
> PPC32. It will be fixed by using the generic implementation.
>
> By using the generic implementation, gcc will naturally generate a
> branch to the unconditional trap generated by BUG().
>
> As modern powerpc implement zero-cycle branch,
> that's even more efficient.
>
> And for the functions using WARN_ON() and its return, the test
> on return from WARN_ON() is now also used for the WARN_ON() itself.
>
> On PPC64 we don't want it because we want to be able to use CFAR
> register to track how we entered the code that trapped. The CFAR
> register would be clobbered by the branch.
>
> A simple test function:
>
> unsigned long test9w(unsigned long a, unsigned long b)
> {
> if (WARN_ON(!b))
> return 0;
> return a / b;
> }
>
> Before the patch:
>
> 0000046c <test9w>:
> 46c: 7c 89 00 34 cntlzw r9,r4
> 470: 55 29 d9 7e rlwinm r9,r9,27,5,31
> 474: 0f 09 00 00 twnei r9,0
> 478: 2c 04 00 00 cmpwi r4,0
> 47c: 41 82 00 0c beq 488 <test9w+0x1c>
> 480: 7c 63 23 96 divwu r3,r3,r4
> 484: 4e 80 00 20 blr
>
> 488: 38 60 00 00 li r3,0
> 48c: 4e 80 00 20 blr
>
> After the patch:
>
> 00000468 <test9w>:
> 468: 2c 04 00 00 cmpwi r4,0
> 46c: 41 82 00 0c beq 478 <test9w+0x10>
> 470: 7c 63 23 96 divwu r3,r3,r4
> 474: 4e 80 00 20 blr
>
> 478: 0f e0 00 00 twui r0,0
> 47c: 38 60 00 00 li r3,0
> 480: 4e 80 00 20 blr
That's clearly better because we have a branch anyway.
>
> So we see before the patch we need 3 instructions on the likely path
> to handle the WARN_ON(). With the patch the trap goes on the unlikely
> path.
>
> See below the difference at the entry of system_call_exception where
> we have several BUG_ON(), allthough less impressing.
>
> With the patch:
>
> 00000000 <system_call_exception>:
> 0: 81 6a 00 84 lwz r11,132(r10)
> 4: 90 6a 00 88 stw r3,136(r10)
> 8: 71 60 00 02 andi. r0,r11,2
> c: 41 82 00 70 beq 7c <system_call_exception+0x7c>
> 10: 71 60 40 00 andi. r0,r11,16384
> 14: 41 82 00 6c beq 80 <system_call_exception+0x80>
> 18: 71 6b 80 00 andi. r11,r11,32768
> 1c: 41 82 00 68 beq 84 <system_call_exception+0x84>
> 20: 94 21 ff e0 stwu r1,-32(r1)
> 24: 93 e1 00 1c stw r31,28(r1)
> 28: 7d 8c 42 e6 mftb r12
> ...
> 7c: 0f e0 00 00 twui r0,0
> 80: 0f e0 00 00 twui r0,0
> 84: 0f e0 00 00 twui r0,0
>
> Without the patch:
>
> 00000000 <system_call_exception>:
> 0: 94 21 ff e0 stwu r1,-32(r1)
> 4: 93 e1 00 1c stw r31,28(r1)
> 8: 90 6a 00 88 stw r3,136(r10)
> c: 81 6a 00 84 lwz r11,132(r10)
> 10: 69 60 00 02 xori r0,r11,2
> 14: 54 00 ff fe rlwinm r0,r0,31,31,31
> 18: 0f 00 00 00 twnei r0,0
> 1c: 69 60 40 00 xori r0,r11,16384
> 20: 54 00 97 fe rlwinm r0,r0,18,31,31
> 24: 0f 00 00 00 twnei r0,0
> 28: 69 6b 80 00 xori r11,r11,32768
> 2c: 55 6b 8f fe rlwinm r11,r11,17,31,31
> 30: 0f 0b 00 00 twnei r11,0
> 34: 7d 8c 42 e6 mftb r12
This one possibly the branches end up in predictors, whereas conditional
trap is always just speculated not to hit. Branches may also have a
throughput limit on execution whereas trap could be more (1 per cycle
vs 4 per cycle on POWER9).
On typical ppc32 CPUs, maybe it's a more obvious win. As you say there
is the CFAR issue as well which makes it a problem for 64s. It would
have been nice if it could use the same code though.
Maybe one day gcc's __builtin_trap() will become smart enough around
conditional statements that it it generates better code and tries to
avoid branches.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH v6 1/2] tty: hvc: pass DMA capable memory to put_chars()
From: Xianting TIan @ 2021-08-13 6:04 UTC (permalink / raw)
To: Jiri Slaby, kernel test robot, gregkh, amit, arnd, osandov
Cc: clang-built-linux, kbuild-all, linuxppc-dev, linux-kernel,
virtualization
In-Reply-To: <0c808001-7643-fdcf-66ba-738654ec0c21@kernel.org>
在 2021/8/13 下午1:53, Jiri Slaby 写道:
> Hi,
>
> On 12. 08. 21, 14:26, kernel test robot wrote:
>>>> drivers/tty/hvc/hvc_console.c:190:26: warning: variable 'hp' is
>>>> uninitialized when used here [-Wuninitialized]
>> spin_unlock_irqrestore(&hp->c_lock, flags);
>> ^~
>> drivers/tty/hvc/hvc_console.c:149:23: note: initialize the
>> variable 'hp' to silence this warning
>> struct hvc_struct *hp;
>> ^
>> = NULL
>
> So you clearly didn't test your change as it would crash quite
> instantly. I wonder, where do you intend to get hp from in the
> console::print() hook?
I am very sorry for the inconvenience caused.
This is caused by my carelessness:(
I take it for granted that there is no problem when I just switch to use
array(cons_outbuf[]).
sorry agin.
>
> thanks,
^ permalink raw reply
* Re: [PATCH v6 1/2] tty: hvc: pass DMA capable memory to put_chars()
From: Jiri Slaby @ 2021-08-13 5:53 UTC (permalink / raw)
To: kernel test robot, Xianting Tian, gregkh, amit, arnd, osandov
Cc: clang-built-linux, kbuild-all, linuxppc-dev, linux-kernel,
virtualization
In-Reply-To: <202108122040.lBf24DNp-lkp@intel.com>
Hi,
On 12. 08. 21, 14:26, kernel test robot wrote:
>>> drivers/tty/hvc/hvc_console.c:190:26: warning: variable 'hp' is uninitialized when used here [-Wuninitialized]
> spin_unlock_irqrestore(&hp->c_lock, flags);
> ^~
> drivers/tty/hvc/hvc_console.c:149:23: note: initialize the variable 'hp' to silence this warning
> struct hvc_struct *hp;
> ^
> = NULL
So you clearly didn't test your change as it would crash quite
instantly. I wonder, where do you intend to get hp from in the
console::print() hook?
thanks,
--
js
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