* [PATCH v2 00/30] KVM: PPC: Book3S PR: Transaction memory support on PR KVM
From: wei.guo.simon @ 2018-02-27 17:37 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Paul Mackerras, kvm, kvm-ppc, Simon Guo
From: Simon Guo <wei.guo.simon@gmail.com>
In current days, many OS distributions have utilized transaction
memory functionality. In PowerPC, HV KVM supports TM. But PR KVM
does not.
The drive for the transaction memory support of PR KVM is the
openstack Continuous Integration testing - They runs a HV(hypervisor)
KVM(as level 1) and then run PR KVM(as level 2) on top of that.
This patch set add transaction memory support on PR KVM.
v1->v2 changes:
1. Correct a bug in trechkpt emulation: the tm sprs need to be
flushed to vcpu before trechkpt.
2. add PR kvm ioctl functionalities for TM.
3. removed save_msr_tm and use kvmppc_get_msr() to determine
whether a transaction state need to be restored.
4. Remove "KVM: PPC: Book3S PR: set MSR HV bit accordingly
for PPC970 and others." patch.
It will prevent PR KVM to start as L1 hypervisor. Since if
we set HV bit to 0 when rfid to guest (who is supposed to
run at HV=1 && PR=1), the guest will not be able to access
its original memory.
The original code always set HV bits for shadow_msr, it is
benign since:
HV bits can only be altered by sc instruction; it can only
be set to 0 by rfid/hrfid instruction.
We return to guest with rfid. So:
* if KVM are running as L1 hypervisor, guest physical MSR
expects HV=1.
* if KVM are running as L2 hypervisor, rfid cannot update
HV =1 so the HV is still 0.
5. add XER register implementation to
kvmppc_copyto_vcpu_tm/kvmppc_copyfrom_vcpu_tm()
6. remove unnecessary stack frame in _kvmppc_save/restore_tm().
7. move MSR bits sync into kvmppc_copy_from_svcpu() so that
we always see inconsistent shadow_msr/kvmppc_get_msr()
even when preemption.
8. doing failure recording in treclaim emulation when TEXASR_FS
is 0.
Test cases performed:
linux/tools/testing/selftests/powerpc/tm/tm-syscall
linux/tools/testing/selftests/powerpc/tm/tm-fork
linux/tools/testing/selftests/powerpc/tm/tm-vmx-unavail
linux/tools/testing/selftests/powerpc/tm/tm-tmspr
linux/tools/testing/selftests/powerpc/tm/tm-signal-msr-resv
linux/tools/testing/selftests/powerpc/math/vsx_preempt
linux/tools/testing/selftests/powerpc/math/fpu_signal
linux/tools/testing/selftests/powerpc/math/vmx_preempt
linux/tools/testing/selftests/powerpc/math/fpu_syscall
linux/tools/testing/selftests/powerpc/math/vmx_syscall
linux/tools/testing/selftests/powerpc/math/fpu_preempt
linux/tools/testing/selftests/powerpc/math/vmx_signal
linux/tools/testing/selftests/powerpc/ptrace/ptrace-tm-gpr
linux/tools/testing/selftests/powerpc/ptrace/ptrace-tm-spd-gpr
linux/tools/testing/selftests/powerpc/ptrace/ptrace-tm-spd-vsx
linux/tools/testing/selftests/powerpc/ptrace/ptrace-tm-spr
linux/tools/testing/selftests/powerpc/ptrace/ptrace-tm-vsx
https://github.com/justdoitqd/publicFiles/blob/master/test_tbegin_pr.c
https://github.com/justdoitqd/publicFiles/blob/master/test_tabort.c
https://github.com/justdoitqd/publicFiles/blob/master/test_kvm_htm_cap.c
https://github.com/justdoitqd/publicFiles/blob/master/test-tm-mig.c
Simon Guo (30):
powerpc: export symbol msr_check_and_set().
powerpc: add TEXASR related macros
powerpc: export tm_enable()/tm_disable/tm_abort() APIs
KVM: PPC: Book3S PR: Move kvmppc_save_tm/kvmppc_restore_tm to separate
file
KVM: PPC: Book3S PR: add new parameter (guest MSR) for
kvmppc_save_tm()/kvmppc_restore_tm()
KVM: PPC: Book3S PR: turn on FP/VSX/VMX MSR bits in kvmppc_save_tm()
KVM: PPC: Book3S PR: add C function wrapper for
_kvmppc_save/restore_tm()
KVM: PPC: Book3S PR: In PR KVM suspends Transactional state when
inject an interrupt.
KVM: PPC: Book3S PR: PR KVM pass through MSR TM/TS bits to shadow_msr.
KVM: PPC: Book3S PR: Sync TM bits to shadow msr for problem state
guest
KVM: PPC: Book3S PR: implement RFID TM behavior to suppress change
from S0 to N0
KVM: PPC: Book3S PR: prevent TS bits change in kvmppc_interrupt_pr()
KVM: PPC: Book3S PR: adds new
kvmppc_copyto_vcpu_tm/kvmppc_copyfrom_vcpu_tm API for PR KVM.
KVM: PPC: Book3S PR: add kvmppc_save/restore_tm_sprs() APIs
KVM: PPC: Book3S PR: add transaction memory save/restore skeleton for
PR KVM
KVM: PPC: Book3S PR: add math support for PR KVM HTM
KVM: PPC: Book3S PR: make mtspr/mfspr emulation behavior based on
active TM SPRs
KVM: PPC: Book3S PR: always fail transaction in guest privilege state
KVM: PPC: Book3S PR: enable NV reg restore for reading TM SPR at guest
privilege state
KVM: PPC: Book3S PR: adds emulation for treclaim.
KVM: PPC: Book3S PR: add emulation for trechkpt in PR KVM.
KVM: PPC: Book3S PR: add emulation for tabort. for privilege guest
KVM: PPC: Book3S PR: add guard code to prevent returning to guest with
PR=0 and Transactional state
KVM: PPC: Book3S PR: Support TAR handling for PR KVM HTM.
KVM: PPC: Book3S PR: enable HTM for PR KVM for KVM_CHECK_EXTENSION
ioctl
KVM: PPC: move vcpu_load/vcpu_put down to each ioctl case in
kvm_arch_vcpu_ioctl
KVM: PPC: remove load/put vcpu for KVM_GET/SET_ONE_REG ioctl
KVM: PPC: remove load/put vcpu for KVM_GET_REGS/KVM_SET_REGS
KVM: PPC: add KVM_SET_ONE_REG/KVM_GET_ONE_REG to async ioctl
KVM: PPC: Book3S PR: enable kvmppc_get/set_one_reg_pr() for HTM
registers
arch/powerpc/include/asm/asm-prototypes.h | 9 +
arch/powerpc/include/asm/kvm_book3s.h | 16 ++
arch/powerpc/include/asm/kvm_host.h | 1 -
arch/powerpc/include/asm/reg.h | 25 +-
arch/powerpc/include/asm/tm.h | 2 -
arch/powerpc/include/uapi/asm/tm.h | 2 +-
arch/powerpc/kernel/process.c | 1 +
arch/powerpc/kernel/tm.S | 12 +
arch/powerpc/kvm/Makefile | 3 +
arch/powerpc/kvm/book3s.c | 6 -
arch/powerpc/kvm/book3s.h | 6 +
arch/powerpc/kvm/book3s_64_mmu.c | 11 +-
arch/powerpc/kvm/book3s_emulate.c | 369 ++++++++++++++++++++++++++-
arch/powerpc/kvm/book3s_hv_rmhandlers.S | 259 ++-----------------
arch/powerpc/kvm/book3s_pr.c | 337 +++++++++++++++++++++++--
arch/powerpc/kvm/book3s_segment.S | 13 +
arch/powerpc/kvm/powerpc.c | 23 +-
arch/powerpc/kvm/tm.S | 379 ++++++++++++++++++++++++++++
arch/powerpc/mm/hash_utils_64.c | 1 +
arch/powerpc/platforms/powernv/copy-paste.h | 3 +-
20 files changed, 1179 insertions(+), 299 deletions(-)
create mode 100644 arch/powerpc/kvm/tm.S
--
1.8.3.1
^ permalink raw reply
* Re: [PATCH 01/21] powerpc: Remove warning on array size when empty
From: Mathieu Malaterre @ 2018-02-27 17:25 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Christophe LEROY, Linux Kernel Mailing List, Paul Mackerras,
Jiri Slaby, open list:LINUX FOR POWERPC PA SEMI PWRFICIENT
In-Reply-To: <CAHp75VePco3hNo_vM916CiLzBAd18i=O1TaecD5YEDewvMpYYQ@mail.gmail.com>
On Tue, Feb 27, 2018 at 4:52 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Tue, Feb 27, 2018 at 9:44 AM, Mathieu Malaterre <malat@debian.org> wro=
te:
>> On Tue, Feb 27, 2018 at 8:33 AM, Christophe LEROY
>> <christophe.leroy@c-s.fr> wrote:
>
>>>>>> Much simpler is just add
>>>>>>
>>>>>> if (ARRAY_SIZE() =3D=3D 0)
>>>>>> return;
>
>>> Or add in front:
>>> if (!ARRAY_SIZE(feature_properties))
>>> return;
>>
>> (not tested) I believe the compiler still go over the for() loop and
>> will complain about the original unsigned comparison.
>
> Did you run tests? Did you look into object file?
The goal of this series is simply to remove warning treated as error.
I tried from home and I can still see the error using gcc 6.3.0, so
the original for() loop needs to be rewritten.
CC arch/powerpc/kernel/prom.o
arch/powerpc/kernel/prom.c: In function =E2=80=98check_cpu_feature_properti=
es=E2=80=99:
arch/powerpc/kernel/prom.c:301:16: error: comparison of unsigned
expression < 0 is always false [-Werror=3Dtype-limits]
for (i =3D 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
^
cc1: all warnings being treated as errors
> In kernel we much rely on the compiling away the code which is
> deterministically not in use.
> Here I'm pretty sure it will compile away entire function.
>
> --
> With Best Regards,
> Andy Shevchenko
^ permalink raw reply
* [PATCH 2/2] ppc64le save_stack_trace_tsk_reliable (Was: HAVE_RELIABLE_STACKTRACE)
From: Torsten Duwe @ 2018-02-27 16:09 UTC (permalink / raw)
To: Miroslav Benes
Cc: Michael Ellerman, Jiri Kosina, Balbir Singh, linux-kernel,
linuxppc-dev, live-patching
In-Reply-To: <alpine.LSU.2.21.1712121308440.21907@pobox.suse.cz>
On Tue, Dec 12, 2017 at 01:12:37PM +0100, Miroslav Benes wrote:
>
> I think that this is not enough. You need to also implement
> save_stack_trace_tsk_reliable() for powerpc defined as __weak in
> kernel/stacktrace.c.
So here is my initial proposal. I'd really like to get the successful
exit stricter, i.e. hit the initial stack value exactly instead of just
a window. Also, the check for kernel code looks clumsy IMHO. IOW:
Comments more than welcome!
Most of it is Copy&Waste, nonetheless:
Signed-off-by: Torsten Duwe <duwe@suse.de>
diff --git a/arch/powerpc/kernel/stacktrace.c b/arch/powerpc/kernel/stacktrace.c
index d534ed901538..e08af49e71d0 100644
--- a/arch/powerpc/kernel/stacktrace.c
+++ b/arch/powerpc/kernel/stacktrace.c
@@ -13,6 +13,7 @@
#include <linux/export.h>
#include <linux/sched.h>
#include <linux/sched/debug.h>
+#include <linux/sched/task_stack.h>
#include <linux/stacktrace.h>
#include <asm/ptrace.h>
#include <asm/processor.h>
@@ -76,3 +77,58 @@ save_stack_trace_regs(struct pt_regs *regs, struct stack_trace *trace)
save_context_stack(trace, regs->gpr[1], current, 0);
}
EXPORT_SYMBOL_GPL(save_stack_trace_regs);
+
+#ifdef CONFIG_HAVE_RELIABLE_STACKTRACE
+int
+save_stack_trace_tsk_reliable(struct task_struct *tsk,
+ struct stack_trace *trace)
+{
+ unsigned long sp;
+ unsigned long stack_page = (unsigned long)task_stack_page(tsk);
+ /* the last frame (unwinding first) may not yet have saved its LR onto the stack. */
+ int firstframe = 1;
+
+ if (tsk == current)
+ sp = current_stack_pointer();
+ else
+ sp = tsk->thread.ksp;
+
+ if (sp < stack_page + sizeof(struct thread_struct)
+ || sp > stack_page + THREAD_SIZE - STACK_FRAME_OVERHEAD)
+ return 1;
+
+ for (;;) {
+ unsigned long *stack = (unsigned long *) sp;
+ unsigned long newsp, ip;
+
+ newsp = stack[0];
+ /* Stack grows downwards; unwinder may only go up */
+ if (newsp <= sp)
+ return 1;
+
+ if (newsp >= stack_page + THREAD_SIZE)
+ return 1; /* invalid backlink, too far up! */
+
+ /* Examine the saved LR: it must point into kernel code. */
+ ip = stack[STACK_FRAME_LR_SAVE];
+ if ( (ip & 0xEFFF000000000000) != CONFIG_KERNEL_START
+ && !firstframe)
+ return 1;
+ firstframe = 0;
+
+ if (!trace->skip)
+ trace->entries[trace->nr_entries++] = ip;
+ else
+ trace->skip--;
+
+ if (newsp > stack_page + THREAD_SIZE - STACK_FRAME_OVERHEAD)
+ break; /* hit the window for last frame */
+
+ if (trace->nr_entries >= trace->max_entries)
+ return -E2BIG;
+
+ sp = newsp;
+ }
+ return 0;
+}
+#endif /* CONFIG_HAVE_RELIABLE_STACKTRACE */
^ permalink raw reply related
* Re: [PATCH 01/21] powerpc: Remove warning on array size when empty
From: Andy Shevchenko @ 2018-02-27 15:52 UTC (permalink / raw)
To: Mathieu Malaterre
Cc: Christophe LEROY, Linux Kernel Mailing List, Paul Mackerras,
Jiri Slaby, open list:LINUX FOR POWERPC PA SEMI PWRFICIENT
In-Reply-To: <CA+7wUsys=J8vkEOtzd26B6MGEhfTBRzaA_myuUWNzf24rMvn+A@mail.gmail.com>
On Tue, Feb 27, 2018 at 9:44 AM, Mathieu Malaterre <malat@debian.org> wrote:
> On Tue, Feb 27, 2018 at 8:33 AM, Christophe LEROY
> <christophe.leroy@c-s.fr> wrote:
>>>>> Much simpler is just add
>>>>>
>>>>> if (ARRAY_SIZE() == 0)
>>>>> return;
>> Or add in front:
>> if (!ARRAY_SIZE(feature_properties))
>> return;
>
> (not tested) I believe the compiler still go over the for() loop and
> will complain about the original unsigned comparison.
Did you run tests? Did you look into object file?
In kernel we much rely on the compiling away the code which is
deterministically not in use.
Here I'm pretty sure it will compile away entire function.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [RFC][PATCH bpf] tools: bpftool: Fix tags for bpf-to-bpf calls
From: Daniel Borkmann @ 2018-02-27 14:44 UTC (permalink / raw)
To: Sandipan Das, naveen.n.rao; +Cc: ast, mpe, jakub.kicinski, netdev, linuxppc-dev
In-Reply-To: <20180227121346.16199-1-sandipan@linux.vnet.ibm.com>
On 02/27/2018 01:13 PM, Sandipan Das wrote:
> "Naveen N. Rao" wrote:
>> I'm wondering if we can instead encode the bpf prog id in
>> imm32. That way, we should be able to indicate the BPF
>> function being called into. Daniel, is that something we
>> can consider?
>
> Since each subprog does not get a separate id, we cannot fetch
> the fd and therefore the tag of a subprog. Instead we can use
> the tag of the complete program as shown below.
>
> "Daniel Borkmann" wrote:
>> I think one limitation that would still need to be addressed later
>> with such approach would be regarding the xlated prog dump in bpftool,
>> see 'BPF calls via JIT' in 7105e828c087 ("bpf: allow for correlation
>> of maps and helpers in dump"). Any ideas for this (potentially if we
>> could use off + imm for calls, we'd get to 48 bits, but that seems
>> still not be enough as you say)?
>
> As an alternative, this is what I am thinking of:
>
> Currently, for bpf-to-bpf calls, if bpf_jit_kallsyms is enabled,
> bpftool looks up the name of the corresponding symbol for the
> JIT-ed subprogram and shows it in the xlated dump alongside the
> actual call instruction. However, the lookup is based on the
> target address which is calculated using the imm field of the
> instruction. So, once again, if imm is truncated, we will end
> up with the wrong address. Also, the subprog aux data (which
> has been proposed as a mitigation for this) is not accessible
> from this tool.
>
> We can still access the tag for the complete bpf program and use
> this with the correct offset in an objdump-like notation as an
> alterative for the name of the subprog that is the target of a
> bpf-to-bpf call instruction.
>
> Currently, an xlated dump looks like this:
> 0: (85) call pc+2#bpf_prog_5f76847930402518_F
> 1: (b7) r0 = 1
> 2: (95) exit
> 3: (b7) r0 = 2
> 4: (95) exit
>
> With this patch, it will look like this:
> 0: (85) call pc+2#bpf_prog_8f85936f29a7790a+3
(Note the +2 is the insn->off already.)
> 1: (b7) r0 = 1
> 2: (95) exit
> 3: (b7) r0 = 2
> 4: (95) exit
>
> where 8f85936f29a7790a is the tag of the bpf program and 3 is
> the offset to the start of the subprog from the start of the
> program.
The problem with this approach would be that right now the name is
something like bpf_prog_5f76847930402518_F where the subprog tag is
just a placeholder so in future, this may well adapt to e.g. the actual
function name from the elf file. Note that when kallsyms is enabled
then a name like bpf_prog_5f76847930402518_F will also appear in stack
traces, perf records, etc, so for correlation/debugging it would really
help to have them the same everywhere.
Worst case if there's nothing better, potentially what one could do in
bpf_prog_get_info_by_fd() is to dump an array of full addresses and
have the imm part as the index pointing to one of them, just unfortunate
that it's likely only needed in ppc64.
^ permalink raw reply
* Re: [RFC REBASED 5/5] powerpc/mm/slice: use the dynamic high slice size to limit bitmap operations
From: Aneesh Kumar K.V @ 2018-02-27 12:41 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: Christophe Leroy, linux-kernel, linuxppc-dev
In-Reply-To: <20180227191125.659d5cbe@roar.ozlabs.ibm.com>
Nicholas Piggin <npiggin@gmail.com> writes:
> On Tue, 27 Feb 2018 14:31:07 +0530
> "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
>
>> Christophe Leroy <christophe.leroy@c-s.fr> writes:
>>
>> > The number of high slices a process might use now depends on its
>> > address space size, and what allocation address it has requested.
>> >
>> > This patch uses that limit throughout call chains where possible,
>> > rather than use the fixed SLICE_NUM_HIGH for bitmap operations.
>> > This saves some cost for processes that don't use very large address
>> > spaces.
>>
>> I haven't really looked at the final code. One of the issue we had was
>> with the below scenario.
>>
>> mmap(addr, len) where addr < 128TB and addr+len > 128TB We want to make
>> sure we build the mask such that we don't find the addr available.
>
> We should run it through the mmap regression tests. I *think* we moved
> all of that logic from the slice code to get_ummapped_area before going
> in to slices. I may have missed something though, it would be good to
> have more eyes on it.
>
mmap(-1,...) failed with the test. Something like below fix it
@@ -756,7 +770,7 @@ void slice_set_user_psize(struct mm_struct *mm, unsigned int psize)
mm->context.low_slices_psize = lpsizes;
hpsizes = mm->context.high_slices_psize;
- high_slices = GET_HIGH_SLICE_INDEX(mm->context.slb_addr_limit);
+ high_slices = SLICE_NUM_HIGH;
for (i = 0; i < high_slices; i++) {
mask_index = i & 0x1;
index = i >> 1;
I guess for everything in the mm_context_t, we should compute it till
SLICE_NUM_HIGH. The reason for failure was, even though we recompute the
slice mask cached in mm_context on slb_addr_limit, it was still derived
from the high_slices_psizes which was computed with lower value.
-aneesh
^ permalink raw reply
* [RFC][PATCH bpf] tools: bpftool: Fix tags for bpf-to-bpf calls
From: Sandipan Das @ 2018-02-27 12:13 UTC (permalink / raw)
To: daniel, naveen.n.rao; +Cc: ast, mpe, jakub.kicinski, netdev, linuxppc-dev
In-Reply-To: <1519153431.im5wioxel1.naveen@linux.ibm.com>
"Naveen N. Rao" wrote:
> I'm wondering if we can instead encode the bpf prog id in
> imm32. That way, we should be able to indicate the BPF
> function being called into. Daniel, is that something we
> can consider?
Since each subprog does not get a separate id, we cannot fetch
the fd and therefore the tag of a subprog. Instead we can use
the tag of the complete program as shown below.
"Daniel Borkmann" wrote:
> I think one limitation that would still need to be addressed later
> with such approach would be regarding the xlated prog dump in bpftool,
> see 'BPF calls via JIT' in 7105e828c087 ("bpf: allow for correlation
> of maps and helpers in dump"). Any ideas for this (potentially if we
> could use off + imm for calls, we'd get to 48 bits, but that seems
> still not be enough as you say)?
As an alternative, this is what I am thinking of:
Currently, for bpf-to-bpf calls, if bpf_jit_kallsyms is enabled,
bpftool looks up the name of the corresponding symbol for the
JIT-ed subprogram and shows it in the xlated dump alongside the
actual call instruction. However, the lookup is based on the
target address which is calculated using the imm field of the
instruction. So, once again, if imm is truncated, we will end
up with the wrong address. Also, the subprog aux data (which
has been proposed as a mitigation for this) is not accessible
from this tool.
We can still access the tag for the complete bpf program and use
this with the correct offset in an objdump-like notation as an
alterative for the name of the subprog that is the target of a
bpf-to-bpf call instruction.
Currently, an xlated dump looks like this:
0: (85) call pc+2#bpf_prog_5f76847930402518_F
1: (b7) r0 = 1
2: (95) exit
3: (b7) r0 = 2
4: (95) exit
With this patch, it will look like this:
0: (85) call pc+2#bpf_prog_8f85936f29a7790a+3
1: (b7) r0 = 1
2: (95) exit
3: (b7) r0 = 2
4: (95) exit
where 8f85936f29a7790a is the tag of the bpf program and 3 is
the offset to the start of the subprog from the start of the
program.
Signed-off-by: Sandipan Das <sandipan@linux.vnet.ibm.com>
---
tools/bpf/bpftool/prog.c | 23 ++++++++++++++---------
1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index e8e2baaf93c2..93746d5d1e3c 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -415,9 +415,11 @@ struct kernel_sym {
};
struct dump_data {
+ unsigned char prog_tag[BPF_TAG_SIZE];
unsigned long address_call_base;
struct kernel_sym *sym_mapping;
__u32 sym_count;
+ unsigned int curr_line;
char scratch_buff[SYM_MAX_NAME];
};
@@ -499,16 +501,16 @@ static void print_insn(struct bpf_verifier_env *env, const char *fmt, ...)
}
static const char *print_call_pcrel(struct dump_data *dd,
- struct kernel_sym *sym,
- unsigned long address,
const struct bpf_insn *insn)
{
- if (sym)
- snprintf(dd->scratch_buff, sizeof(dd->scratch_buff),
- "%+d#%s", insn->off, sym->name);
- else
- snprintf(dd->scratch_buff, sizeof(dd->scratch_buff),
- "%+d#0x%lx", insn->off, address);
+ snprintf(dd->scratch_buff, sizeof(dd->scratch_buff),
+ "+%d#bpf_prog_%02x%02x%02x%02x%02x%02x%02x%02x+%d",
+ insn->off,
+ dd->prog_tag[0], dd->prog_tag[1],
+ dd->prog_tag[2], dd->prog_tag[3],
+ dd->prog_tag[4], dd->prog_tag[5],
+ dd->prog_tag[6], dd->prog_tag[7],
+ dd->curr_line + insn->off + 1);
return dd->scratch_buff;
}
@@ -534,7 +536,7 @@ static const char *print_call(void *private_data,
sym = kernel_syms_search(dd, address);
if (insn->src_reg == BPF_PSEUDO_CALL)
- return print_call_pcrel(dd, sym, address, insn);
+ return print_call_pcrel(dd, insn);
else
return print_call_helper(dd, sym, address);
}
@@ -576,6 +578,7 @@ static void dump_xlated_plain(struct dump_data *dd, void *buf,
double_insn = insn[i].code == (BPF_LD | BPF_IMM | BPF_DW);
printf("% 4d: ", i);
+ dd->curr_line = i;
print_bpf_insn(&cbs, NULL, insn + i, true);
if (opcodes) {
@@ -628,6 +631,7 @@ static void dump_xlated_json(struct dump_data *dd, void *buf,
jsonw_start_object(json_wtr);
jsonw_name(json_wtr, "disasm");
+ dd->curr_line = i;
print_bpf_insn(&cbs, NULL, insn + i, true);
if (opcodes) {
@@ -788,6 +792,7 @@ static int do_dump(int argc, char **argv)
disasm_print_insn(buf, *member_len, opcodes, name);
} else {
+ memcpy(dd.prog_tag, info.tag, sizeof(info.tag));
kernel_syms_load(&dd);
if (json_output)
dump_xlated_json(&dd, buf, *member_len, opcodes);
--
2.14.3
^ permalink raw reply related
* [PATCH] powerpc/8xx: fix cpm_cascade() dual end of interrupt
From: Christophe Leroy @ 2018-02-27 11:25 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Scott Wood, Vitaly Bordug
Cc: linux-kernel, linuxppc-dev
cpm_cascade() doesn't have to call eoi() as it is already called
by handle_fasteoi_irq()
And cpm_get_irq() will always return an unsigned int so the test
is useless
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/platforms/8xx/m8xx_setup.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/arch/powerpc/platforms/8xx/m8xx_setup.c b/arch/powerpc/platforms/8xx/m8xx_setup.c
index e1274db53d48..2188d691a40f 100644
--- a/arch/powerpc/platforms/8xx/m8xx_setup.c
+++ b/arch/powerpc/platforms/8xx/m8xx_setup.c
@@ -217,13 +217,7 @@ void __noreturn mpc8xx_restart(char *cmd)
static void cpm_cascade(struct irq_desc *desc)
{
- struct irq_chip *chip = irq_desc_get_chip(desc);
- int cascade_irq = cpm_get_irq();
-
- if (cascade_irq >= 0)
- generic_handle_irq(cascade_irq);
-
- chip->irq_eoi(&desc->irq_data);
+ generic_handle_irq(cpm_get_irq());
}
/* Initialize the internal interrupt controllers. The number of
--
2.13.3
^ permalink raw reply related
* Re: Linux 4.16: Reported regressions as of Monday, 2018-02-26 (Was: Linux 4.16-rc3)
From: Rafael J. Wysocki @ 2018-02-27 11:21 UTC (permalink / raw)
To: Thorsten Leemhuis
Cc: Linus Torvalds, Linux Kernel Mailing List, linuxppc-dev,
Jonathan Corbet, x86, William Grant, Woody Suwalski
In-Reply-To: <2cf45d87-066c-7368-3fbd-20675f2d74ae@leemhuis.info>
On Monday, February 26, 2018 11:25:09 AM CET Thorsten Leemhuis wrote:
> On 26.02.2018 04:05, Linus Torvalds wrote:
> > We're on the normal schedule for 4.16 and everything still looks very regular.
>
> Hi! Find below my second regression report for Linux 4.16. It lists 8
> regressions I'm currently aware of.
>
> To anyone reading this: Are you aware of any other regressions that got
> introduced this development cycle? Then please let me know by mail (a
> simple bounce or forward to the email address is enough!).
>
> For details see http://bit.ly/lnxregtrackid And please tell me if there
> is anything in the report that shouldn't be there.
Please add https://bugzilla.kernel.org/show_bug.cgi?id=198763
It has a bisect result and seems to be readily reproducible.
No response from the author of the problematic commit so far.
Thanks,
Rafael
^ permalink raw reply
* Re: [RFC REBASED 5/5] powerpc/mm/slice: use the dynamic high slice size to limit bitmap operations
From: Nicholas Piggin @ 2018-02-27 9:11 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: Christophe Leroy, linux-kernel, linuxppc-dev
In-Reply-To: <87bmga7qng.fsf@linux.vnet.ibm.com>
On Tue, 27 Feb 2018 14:31:07 +0530
"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
> Christophe Leroy <christophe.leroy@c-s.fr> writes:
>
> > The number of high slices a process might use now depends on its
> > address space size, and what allocation address it has requested.
> >
> > This patch uses that limit throughout call chains where possible,
> > rather than use the fixed SLICE_NUM_HIGH for bitmap operations.
> > This saves some cost for processes that don't use very large address
> > spaces.
>
> I haven't really looked at the final code. One of the issue we had was
> with the below scenario.
>
> mmap(addr, len) where addr < 128TB and addr+len > 128TB We want to make
> sure we build the mask such that we don't find the addr available.
We should run it through the mmap regression tests. I *think* we moved
all of that logic from the slice code to get_ummapped_area before going
in to slices. I may have missed something though, it would be good to
have more eyes on it.
Thanks,
Nick
^ permalink raw reply
* Re: [RFC REBASED 4/5] powerpc/mm/slice: Use const pointers to cached slice masks where possible
From: Nicholas Piggin @ 2018-02-27 9:08 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: Christophe Leroy, linux-kernel, linuxppc-dev
In-Reply-To: <87h8q27uvi.fsf@linux.vnet.ibm.com>
On Tue, 27 Feb 2018 12:59:53 +0530
"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
> Christophe Leroy <christophe.leroy@c-s.fr> writes:
>
> > The slice_mask cache was a basic conversion which copied the slice
> > mask into caller's structures, because that's how the original code
> > worked. In most cases the pointer can be used directly instead, saving
> > a copy and an on-stack structure.
> >
> > This also converts the slice_mask bit operation helpers to be the usual
> > 3-operand kind, which is clearer to work with. And we remove some
> > unnecessary intermediate bitmaps, reducing stack and copy overhead
> > further.
>
> Can we move the reduce unncessary intermediate bitmaps as another patch?
This was rightly split out -- it moved to Christophe's patch he already
made in his series. The changelog can just be adjusted.
Thanks,
Nick
^ permalink raw reply
* Re: [RFC REBASED 3/5] powerpc/mm/slice: implement slice_check_range_fits
From: Nicholas Piggin @ 2018-02-27 9:04 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: Christophe Leroy, linux-kernel, linuxppc-dev
In-Reply-To: <87k1uy7vbr.fsf@linux.vnet.ibm.com>
On Tue, 27 Feb 2018 12:50:08 +0530
"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
> Christophe Leroy <christophe.leroy@c-s.fr> writes:
> + if ((start + len) > SLICE_LOW_TOP) {
> > + unsigned long start_index = GET_HIGH_SLICE_INDEX(start);
> > + unsigned long align_end = ALIGN(end, (1UL << SLICE_HIGH_SHIFT));
> > + unsigned long count = GET_HIGH_SLICE_INDEX(align_end) - start_index;
> > + unsigned long i;
> >
> > - slice_bitmap_and(result, mask->high_slices, available->high_slices,
> > - slice_count);
> > + for (i = start_index; i < start_index + count; i++) {
> > + if (!test_bit(i, available->high_slices))
> > + return false;
> > + }
> > + }
>
> why not bitmap_equal here instead of test_bit in loop?
Because we only have the available bitmap now. If we see large ranges
here we could use some bitmap operation like find_next_zero_bit perhaps.
Thanks,
Nick
^ permalink raw reply
* Re: [RFC REBASED 5/5] powerpc/mm/slice: use the dynamic high slice size to limit bitmap operations
From: Aneesh Kumar K.V @ 2018-02-27 9:01 UTC (permalink / raw)
To: Christophe Leroy, Nicholas Piggin; +Cc: linux-kernel, linuxppc-dev
In-Reply-To: <5badd882663833576c10b8aafe235fe1e443f119.1518382747.git.christophe.leroy@c-s.fr>
Christophe Leroy <christophe.leroy@c-s.fr> writes:
> The number of high slices a process might use now depends on its
> address space size, and what allocation address it has requested.
>
> This patch uses that limit throughout call chains where possible,
> rather than use the fixed SLICE_NUM_HIGH for bitmap operations.
> This saves some cost for processes that don't use very large address
> spaces.
I haven't really looked at the final code. One of the issue we had was
with the below scenario.
mmap(addr, len) where addr < 128TB and addr+len > 128TB We want to make
sure we build the mask such that we don't find the addr available.
-aneesh
^ permalink raw reply
* Re: [PATCH 01/21] powerpc: Remove warning on array size when empty
From: Mathieu Malaterre @ 2018-02-27 7:44 UTC (permalink / raw)
To: Christophe LEROY
Cc: Andy Shevchenko, Linux Kernel Mailing List, Paul Mackerras,
Jiri Slaby, open list:LINUX FOR POWERPC PA SEMI PWRFICIENT
In-Reply-To: <603d5335-6220-73f2-d902-b92bc74bc79e@c-s.fr>
On Tue, Feb 27, 2018 at 8:33 AM, Christophe LEROY
<christophe.leroy@c-s.fr> wrote:
>
>
> Le 27/02/2018 =C3=A0 08:25, Mathieu Malaterre a =C3=A9crit :
>>
>> On Mon, Feb 26, 2018 at 3:45 PM, Andy Shevchenko
>> <andy.shevchenko@gmail.com> wrote:
>>>
>>> On Mon, Feb 26, 2018 at 4:44 PM, Andy Shevchenko
>>> <andy.shevchenko@gmail.com> wrote:
>>>>
>>>> On Sun, Feb 25, 2018 at 7:22 PM, Mathieu Malaterre <malat@debian.org>
>>>> wrote:
>>>
>>>
>>>>> static void __init check_cpu_feature_properties(unsigned long node)
>>>>> {
>>>>> - unsigned long i;
>>>>> struct feature_property *fp =3D feature_properties;
>>>>> const __be32 *prop;
>>>>>
>>>>
>>>> Much simpler is just add
>>>>
>>>> if (ARRAY_SIZE() =3D=3D 0)
>>>> return;
>>>>
>>>>> - for (i =3D 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) =
{
>>>>> + for (; fp !=3D feature_properties +
>>>>> ARRAY_SIZE(feature_properties); ++fp) {
>>>
>>>
>>> ...or convert to while(), which will be more readable.
>>
>>
>> So you'd prefer something like:
>>
>> while (fp < feature_properties + ARRAY_SIZE(feature_properties)) {
>> ...
>> ++fp;
>> }
>>
>> right ?
>>
>
>
> Why not do as suggested by Segher, ie just replace < by !=3D in the origi=
nal
> form ?
I can do that.
> Or add in front:
> if (!ARRAY_SIZE(feature_properties))
> return;
(not tested) I believe the compiler still go over the for() loop and
will complain about the original unsigned comparison.
> Christophe
^ permalink raw reply
* Re: [PATCH 01/21] powerpc: Remove warning on array size when empty
From: Christophe LEROY @ 2018-02-27 7:33 UTC (permalink / raw)
To: Mathieu Malaterre, Andy Shevchenko
Cc: Linux Kernel Mailing List, Paul Mackerras, Jiri Slaby,
open list:LINUX FOR POWERPC PA SEMI PWRFICIENT
In-Reply-To: <CA+7wUszK++8Tr9j1=4jQtRSu-36MH=dqgimq-ST9D9hvacrJSQ@mail.gmail.com>
Le 27/02/2018 à 08:25, Mathieu Malaterre a écrit :
> On Mon, Feb 26, 2018 at 3:45 PM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
>> On Mon, Feb 26, 2018 at 4:44 PM, Andy Shevchenko
>> <andy.shevchenko@gmail.com> wrote:
>>> On Sun, Feb 25, 2018 at 7:22 PM, Mathieu Malaterre <malat@debian.org> wrote:
>>
>>>> static void __init check_cpu_feature_properties(unsigned long node)
>>>> {
>>>> - unsigned long i;
>>>> struct feature_property *fp = feature_properties;
>>>> const __be32 *prop;
>>>>
>>>
>>> Much simpler is just add
>>>
>>> if (ARRAY_SIZE() == 0)
>>> return;
>>>
>>>> - for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
>>>> + for (; fp != feature_properties + ARRAY_SIZE(feature_properties); ++fp) {
>>
>> ...or convert to while(), which will be more readable.
>
> So you'd prefer something like:
>
> while (fp < feature_properties + ARRAY_SIZE(feature_properties)) {
> ...
> ++fp;
> }
>
> right ?
>
Why not do as suggested by Segher, ie just replace < by != in the
original form ?
Or add in front:
if (!ARRAY_SIZE(feature_properties))
return;
Christophe
^ permalink raw reply
* Re: [RFC REBASED 4/5] powerpc/mm/slice: Use const pointers to cached slice masks where possible
From: Aneesh Kumar K.V @ 2018-02-27 7:29 UTC (permalink / raw)
To: Christophe Leroy, Nicholas Piggin; +Cc: linux-kernel, linuxppc-dev
In-Reply-To: <6a8c9183257dfcfeb1d8ed1ecc778ec3da19dcd4.1518382747.git.christophe.leroy@c-s.fr>
Christophe Leroy <christophe.leroy@c-s.fr> writes:
> The slice_mask cache was a basic conversion which copied the slice
> mask into caller's structures, because that's how the original code
> worked. In most cases the pointer can be used directly instead, saving
> a copy and an on-stack structure.
>
> This also converts the slice_mask bit operation helpers to be the usual
> 3-operand kind, which is clearer to work with. And we remove some
> unnecessary intermediate bitmaps, reducing stack and copy overhead
> further.
Can we move the reduce unncessary intermediate bitmaps as another patch?
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> arch/powerpc/include/asm/book3s/64/slice.h | 7 +++
> arch/powerpc/include/asm/nohash/32/slice.h | 6 +++
> arch/powerpc/mm/slice.c | 77 ++++++++++++++++++------------
> 3 files changed, 59 insertions(+), 31 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/book3s/64/slice.h b/arch/powerpc/include/asm/book3s/64/slice.h
> index f9a2c8bd7a77..be1ce8e91ad1 100644
> --- a/arch/powerpc/include/asm/book3s/64/slice.h
> +++ b/arch/powerpc/include/asm/book3s/64/slice.h
> @@ -63,6 +63,13 @@ static inline void slice_bitmap_set(unsigned long *map, unsigned int start,
> {
> bitmap_set(map, start, nbits);
> }
> +
> +static inline void slice_bitmap_copy(unsigned long *dst,
> + const unsigned long *src,
> + unsigned int nbits)
> +{
> + bitmap_copy(dst, src, nbits);
> +}
> #endif /* __ASSEMBLY__ */
>
> #else /* CONFIG_PPC_MM_SLICES */
> diff --git a/arch/powerpc/include/asm/nohash/32/slice.h b/arch/powerpc/include/asm/nohash/32/slice.h
> index bcb4924f7d22..38f041e01a0a 100644
> --- a/arch/powerpc/include/asm/nohash/32/slice.h
> +++ b/arch/powerpc/include/asm/nohash/32/slice.h
> @@ -58,6 +58,12 @@ static inline void slice_bitmap_set(unsigned long *map, unsigned int start,
> unsigned int nbits)
> {
> }
> +
> +static inline void slice_bitmap_copy(unsigned long *dst,
> + const unsigned long *src,
> + unsigned int nbits)
> +{
> +}
> #endif /* __ASSEMBLY__ */
>
> #endif /* CONFIG_PPC_MM_SLICES */
> diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
> index 311168ca3939..b8b691369c29 100644
> --- a/arch/powerpc/mm/slice.c
> +++ b/arch/powerpc/mm/slice.c
> @@ -468,21 +468,30 @@ static unsigned long slice_find_area(struct mm_struct *mm, unsigned long len,
> return slice_find_area_bottomup(mm, len, mask, psize, high_limit);
> }
>
> -static inline void slice_or_mask(struct slice_mask *dst,
> +static inline void slice_copy_mask(struct slice_mask *dst,
> const struct slice_mask *src)
> {
> - dst->low_slices |= src->low_slices;
> - slice_bitmap_or(dst->high_slices, dst->high_slices, src->high_slices,
> + dst->low_slices = src->low_slices;
> + slice_bitmap_copy(dst->high_slices, src->high_slices, SLICE_NUM_HIGH);
> +}
> +
> +static inline void slice_or_mask(struct slice_mask *dst,
> + const struct slice_mask *src1,
> + const struct slice_mask *src2)
> +{
> + dst->low_slices = src1->low_slices | src2->low_slices;
> + slice_bitmap_or(dst->high_slices, src1->high_slices, src2->high_slices,
> SLICE_NUM_HIGH);
> }
>
> static inline void slice_andnot_mask(struct slice_mask *dst,
> - const struct slice_mask *src)
> + const struct slice_mask *src1,
> + const struct slice_mask *src2)
> {
> - dst->low_slices &= ~src->low_slices;
> + dst->low_slices = src1->low_slices & ~src2->low_slices;
>
> - slice_bitmap_andnot(dst->high_slices, dst->high_slices,
> - src->high_slices, SLICE_NUM_HIGH);
> + slice_bitmap_andnot(dst->high_slices, src1->high_slices,
> + src2->high_slices, SLICE_NUM_HIGH);
> }
>
> #ifdef CONFIG_PPC_64K_PAGES
> @@ -495,10 +504,10 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
> unsigned long flags, unsigned int psize,
> int topdown)
> {
> - struct slice_mask mask;
> struct slice_mask good_mask;
> struct slice_mask potential_mask;
> - struct slice_mask compat_mask;
> + const struct slice_mask *maskp;
> + const struct slice_mask *compat_maskp = NULL;
> int fixed = (flags & MAP_FIXED);
> int pshift = max_t(int, mmu_psize_defs[psize].shift, PAGE_SHIFT);
> unsigned long page_size = 1UL << pshift;
> @@ -537,9 +546,6 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
> potential_mask.low_slices = 0;
> slice_bitmap_zero(potential_mask.high_slices, SLICE_NUM_HIGH);
>
> - compat_mask.low_slices = 0;
> - slice_bitmap_zero(compat_mask.high_slices, SLICE_NUM_HIGH);
> -
> /* Sanity checks */
> BUG_ON(mm->task_size == 0);
> BUG_ON(mm->context.slb_addr_limit == 0);
> @@ -562,7 +568,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
> /* First make up a "good" mask of slices that have the right size
> * already
> */
> - good_mask = *slice_mask_for_size(mm, psize);
> + maskp = slice_mask_for_size(mm, psize);
> slice_print_mask(" good_mask", &good_mask);
>
> /*
> @@ -587,11 +593,16 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
> #ifdef CONFIG_PPC_64K_PAGES
> /* If we support combo pages, we can allow 64k pages in 4k slices */
> if (psize == MMU_PAGE_64K) {
> - compat_mask = *slice_mask_for_size(mm, MMU_PAGE_4K);
> + compat_maskp = slice_mask_for_size(mm, MMU_PAGE_4K);
> if (fixed)
> - slice_or_mask(&good_mask, &compat_mask);
> - }
> + slice_or_mask(&good_mask, maskp, compat_maskp);
> + else
> + slice_copy_mask(&good_mask, maskp);
> + } else
> #endif
> + {
> + slice_copy_mask(&good_mask, maskp);
> + }
>
> /* First check hint if it's valid or if we have MAP_FIXED */
> if (addr || fixed) {
> @@ -621,7 +632,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
> * empty and thus can be converted
> */
> slice_mask_for_free(mm, &potential_mask, high_limit);
> - slice_or_mask(&potential_mask, &good_mask);
> + slice_or_mask(&potential_mask, &potential_mask, &good_mask);
> slice_print_mask(" potential", &potential_mask);
>
> if (addr || fixed) {
> @@ -658,7 +669,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
> #ifdef CONFIG_PPC_64K_PAGES
> if (addr == -ENOMEM && psize == MMU_PAGE_64K) {
> /* retry the search with 4k-page slices included */
> - slice_or_mask(&potential_mask, &compat_mask);
> + slice_or_mask(&potential_mask, &potential_mask, compat_maskp);
> addr = slice_find_area(mm, len, &potential_mask,
> psize, topdown, high_limit);
> }
> @@ -667,16 +678,17 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
> if (addr == -ENOMEM)
> return -ENOMEM;
>
> - slice_range_to_mask(addr, len, &mask);
> + slice_range_to_mask(addr, len, &potential_mask);
Can we avoid reusing variables like that. I had to look at the change
below to ensure that this is intended change. The print below is wrong
then. In my patch series I did try to remove potential_mask and compat
mask and used tmp_mask in its place. IMHO that resulted in much simpiler
code. I do recompute tmp_mask for 4K size because of lack of variable.
But then i guess with your change to cache mask for different page size,
it should not have an impact?
> slice_dbg(" found potential area at 0x%lx\n", addr);
> - slice_print_mask(" mask", &mask);
> + slice_print_mask(" mask", maskp);
>
> convert:
> - slice_andnot_mask(&mask, &good_mask);
> - slice_andnot_mask(&mask, &compat_mask);
> - if (mask.low_slices ||
> - !slice_bitmap_empty(mask.high_slices, SLICE_NUM_HIGH)) {
> - slice_convert(mm, &mask, psize);
> + slice_andnot_mask(&potential_mask, &potential_mask, &good_mask);
> + if (compat_maskp && !fixed)
> + slice_andnot_mask(&potential_mask, &potential_mask, compat_maskp);
> + if (potential_mask.low_slices ||
> + !slice_bitmap_empty(potential_mask.high_slices, SLICE_NUM_HIGH)) {
> + slice_convert(mm, &potential_mask, psize);
> if (psize > MMU_PAGE_BASE)
> on_each_cpu(slice_flush_segments, mm, 1);
> }
> @@ -834,19 +846,22 @@ void slice_set_range_psize(struct mm_struct *mm, unsigned long start,
> int is_hugepage_only_range(struct mm_struct *mm, unsigned long addr,
> unsigned long len)
> {
> - struct slice_mask available;
> + const struct slice_mask *maskp;
> unsigned int psize = mm->context.user_psize;
>
> if (radix_enabled())
> return 0;
>
> - available = *slice_mask_for_size(mm, psize);
> + maskp = slice_mask_for_size(mm, psize);
> #ifdef CONFIG_PPC_64K_PAGES
> /* We need to account for 4k slices too */
> if (psize == MMU_PAGE_64K) {
> - struct slice_mask compat_mask;
> - compat_mask = *slice_mask_for_size(mm, MMU_PAGE_4K);
> - slice_or_mask(&available, &compat_mask);
> + const struct slice_mask *compat_maskp;
> + struct slice_mask available;
> +
> + compat_maskp = slice_mask_for_size(mm, MMU_PAGE_4K);
> + slice_or_mask(&available, maskp, compat_maskp);
> + return !slice_check_range_fits(mm, &available, addr, len);
> }
> #endif
>
> @@ -856,6 +871,6 @@ int is_hugepage_only_range(struct mm_struct *mm, unsigned long addr,
> slice_print_mask(" mask", &mask);
> slice_print_mask(" available", &available);
> #endif
> - return !slice_check_range_fits(mm, &available, addr, len);
> + return !slice_check_range_fits(mm, maskp, addr, len);
> }
> #endif
> --
> 2.13.3
^ permalink raw reply
* Re: [PATCH 01/21] powerpc: Remove warning on array size when empty
From: Mathieu Malaterre @ 2018-02-27 7:25 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
Jiri Slaby, open list:LINUX FOR POWERPC PA SEMI PWRFICIENT,
Linux Kernel Mailing List
In-Reply-To: <CAHp75Ve_zxp4_N_YzzvJ=H8Qs65G5j7bgGBv_TvsZ6Ye2nuECA@mail.gmail.com>
On Mon, Feb 26, 2018 at 3:45 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Mon, Feb 26, 2018 at 4:44 PM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
>> On Sun, Feb 25, 2018 at 7:22 PM, Mathieu Malaterre <malat@debian.org> wrote:
>
>>> static void __init check_cpu_feature_properties(unsigned long node)
>>> {
>>> - unsigned long i;
>>> struct feature_property *fp = feature_properties;
>>> const __be32 *prop;
>>>
>>
>> Much simpler is just add
>>
>> if (ARRAY_SIZE() == 0)
>> return;
>>
>>> - for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
>>> + for (; fp != feature_properties + ARRAY_SIZE(feature_properties); ++fp) {
>
> ...or convert to while(), which will be more readable.
So you'd prefer something like:
while (fp < feature_properties + ARRAY_SIZE(feature_properties)) {
...
++fp;
}
right ?
^ permalink raw reply
* Re: [RFC REBASED 3/5] powerpc/mm/slice: implement slice_check_range_fits
From: Aneesh Kumar K.V @ 2018-02-27 7:20 UTC (permalink / raw)
To: Christophe Leroy, Nicholas Piggin; +Cc: linux-kernel, linuxppc-dev
In-Reply-To: <caa9d52e0a8ef15719bafe8002a4ad2dc359c634.1518382747.git.christophe.leroy@c-s.fr>
Christophe Leroy <christophe.leroy@c-s.fr> writes:
+ if ((start + len) > SLICE_LOW_TOP) {
> + unsigned long start_index = GET_HIGH_SLICE_INDEX(start);
> + unsigned long align_end = ALIGN(end, (1UL << SLICE_HIGH_SHIFT));
> + unsigned long count = GET_HIGH_SLICE_INDEX(align_end) - start_index;
> + unsigned long i;
>
> - slice_bitmap_and(result, mask->high_slices, available->high_slices,
> - slice_count);
> + for (i = start_index; i < start_index + count; i++) {
> + if (!test_bit(i, available->high_slices))
> + return false;
> + }
> + }
why not bitmap_equal here instead of test_bit in loop?
>
> - return (mask->low_slices & available->low_slices) == mask->low_slices &&
> - slice_bitmap_equal(result, mask->high_slices, slice_count);
> + return true;
> }
-aneesh
^ permalink raw reply
* Re: [RFC REBASED 1/5] powerpc/mm/slice: pass pointers to struct slice_mask where possible
From: Christophe LEROY @ 2018-02-27 7:04 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: linuxppc-dev@lists.ozlabs.org, Nicholas Piggin
In-Reply-To: <87muzu7w58.fsf@linux.vnet.ibm.com>
Le 27/02/2018 à 08:02, Aneesh Kumar K.V a écrit :
>
> Hi,
>
> Do you have an updated series for this with your latest PPC32 slice
> post?
No, but I can make one if it is on any interest.
Christophe
>
> Christophe Leroy <christophe.leroy@c-s.fr> writes:
>
>> Pass around const pointers to struct slice_mask where possible, rather
>> than copies of slice_mask, to reduce stack and call overhead.
>>
>> checkstack.pl gives, before:
>> 0x00000de4 slice_get_unmapped_area [slice.o]: 656
>> 0x00001b4c is_hugepage_only_range [slice.o]: 512
>> 0x0000075c slice_find_area_topdown [slice.o]: 416
>> 0x000004c8 slice_find_area_bottomup.isra.1 [slice.o]: 272
>> 0x00001aa0 slice_set_range_psize [slice.o]: 240
>> 0x00000a64 slice_find_area [slice.o]: 176
>> 0x00000174 slice_check_fit [slice.o]: 112
>>
>> after:
>> 0x00000bd4 slice_get_unmapped_area [slice.o]: 496
>> 0x000017cc is_hugepage_only_range [slice.o]: 352
>> 0x00000758 slice_find_area [slice.o]: 144
>> 0x00001750 slice_set_range_psize [slice.o]: 144
>> 0x00000180 slice_check_fit [slice.o]: 128
>> 0x000005b0 slice_find_area_bottomup.isra.2 [slice.o]: 128
>>
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>> ---
>> rebased on top of "[v4,3/5] powerpc/mm/slice: Fix hugepage allocation at hint address on 8xx" (https://patchwork.ozlabs.org/patch/871675/)
>>
>> arch/powerpc/mm/slice.c | 81 +++++++++++++++++++++++++++----------------------
>> 1 file changed, 44 insertions(+), 37 deletions(-)
>>
>> diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
>> index 549704dfa777..db1278ac21c2 100644
>> --- a/arch/powerpc/mm/slice.c
>> +++ b/arch/powerpc/mm/slice.c
>> @@ -50,19 +50,21 @@ struct slice_mask {
>> #ifdef DEBUG
>> int _slice_debug = 1;
>>
>> -static void slice_print_mask(const char *label, struct slice_mask mask)
>> +static void slice_print_mask(const char *label, const struct slice_mask *mask)
>> {
>> if (!_slice_debug)
>> return;
>> - pr_devel("%s low_slice: %*pbl\n", label, (int)SLICE_NUM_LOW, &mask.low_slices);
>> - pr_devel("%s high_slice: %*pbl\n", label, (int)SLICE_NUM_HIGH, mask.high_slices);
>> + pr_devel("%s low_slice: %*pbl\n", label,
>> + (int)SLICE_NUM_LOW, &mask->low_slices);
>> + pr_devel("%s high_slice: %*pbl\n", label,
>> + (int)SLICE_NUM_HIGH, mask->high_slices);
>> }
>>
>> #define slice_dbg(fmt...) do { if (_slice_debug) pr_devel(fmt); } while (0)
>>
>> #else
>>
>> -static void slice_print_mask(const char *label, struct slice_mask mask) {}
>> +static void slice_print_mask(const char *label, const struct slice_mask *mask) {}
>> #define slice_dbg(fmt...)
>>
>> #endif
>> @@ -145,7 +147,8 @@ static void slice_mask_for_free(struct mm_struct *mm, struct slice_mask *ret,
>> __set_bit(i, ret->high_slices);
>> }
>>
>> -static void slice_mask_for_size(struct mm_struct *mm, int psize, struct slice_mask *ret,
>> +static void slice_mask_for_size(struct mm_struct *mm, int psize,
>> + struct slice_mask *ret,
>> unsigned long high_limit)
>> {
>> unsigned char *hpsizes;
>> @@ -174,7 +177,8 @@ static void slice_mask_for_size(struct mm_struct *mm, int psize, struct slice_ma
>> }
>>
>> static int slice_check_fit(struct mm_struct *mm,
>> - struct slice_mask mask, struct slice_mask available)
>> + const struct slice_mask *mask,
>> + const struct slice_mask *available)
>> {
>> DECLARE_BITMAP(result, SLICE_NUM_HIGH);
>> /*
>> @@ -183,11 +187,11 @@ static int slice_check_fit(struct mm_struct *mm,
>> */
>> unsigned long slice_count = GET_HIGH_SLICE_INDEX(mm->context.slb_addr_limit);
>>
>> - slice_bitmap_and(result, mask.high_slices, available.high_slices,
>> + slice_bitmap_and(result, mask->high_slices, available->high_slices,
>> slice_count);
>>
>> - return (mask.low_slices & available.low_slices) == mask.low_slices &&
>> - slice_bitmap_equal(result, mask.high_slices, slice_count);
>> + return (mask->low_slices & available->low_slices) == mask->low_slices &&
>> + slice_bitmap_equal(result, mask->high_slices, slice_count);
>> }
>>
>> static void slice_flush_segments(void *parm)
>> @@ -207,7 +211,8 @@ static void slice_flush_segments(void *parm)
>> #endif
>> }
>>
>> -static void slice_convert(struct mm_struct *mm, struct slice_mask mask, int psize)
>> +static void slice_convert(struct mm_struct *mm,
>> + const struct slice_mask *mask, int psize)
>> {
>> int index, mask_index;
>> /* Write the new slice psize bits */
>> @@ -225,7 +230,7 @@ static void slice_convert(struct mm_struct *mm, struct slice_mask mask, int psiz
>>
>> lpsizes = mm->context.low_slices_psize;
>> for (i = 0; i < SLICE_NUM_LOW; i++)
>> - if (mask.low_slices & (1u << i))
>> + if (mask->low_slices & (1u << i))
>> lpsizes = (lpsizes & ~(0xful << (i * 4))) |
>> (((unsigned long)psize) << (i * 4));
>>
>> @@ -236,7 +241,7 @@ static void slice_convert(struct mm_struct *mm, struct slice_mask mask, int psiz
>> for (i = 0; i < GET_HIGH_SLICE_INDEX(mm->context.slb_addr_limit); i++) {
>> mask_index = i & 0x1;
>> index = i >> 1;
>> - if (test_bit(i, mask.high_slices))
>> + if (test_bit(i, mask->high_slices))
>> hpsizes[index] = (hpsizes[index] &
>> ~(0xf << (mask_index * 4))) |
>> (((unsigned long)psize) << (mask_index * 4));
>> @@ -259,26 +264,25 @@ static void slice_convert(struct mm_struct *mm, struct slice_mask mask, int psiz
>> * 'available' slice_mark.
>> */
>> static bool slice_scan_available(unsigned long addr,
>> - struct slice_mask available,
>> - int end,
>> - unsigned long *boundary_addr)
>> + const struct slice_mask *available,
>> + int end, unsigned long *boundary_addr)
>> {
>> unsigned long slice;
>> if (addr < SLICE_LOW_TOP) {
>> slice = GET_LOW_SLICE_INDEX(addr);
>> *boundary_addr = (slice + end) << SLICE_LOW_SHIFT;
>> - return !!(available.low_slices & (1u << slice));
>> + return !!(available->low_slices & (1u << slice));
>> } else {
>> slice = GET_HIGH_SLICE_INDEX(addr);
>> *boundary_addr = (slice + end) ?
>> ((slice + end) << SLICE_HIGH_SHIFT) : SLICE_LOW_TOP;
>> - return !!test_bit(slice, available.high_slices);
>> + return !!test_bit(slice, available->high_slices);
>> }
>> }
>>
>> static unsigned long slice_find_area_bottomup(struct mm_struct *mm,
>> unsigned long len,
>> - struct slice_mask available,
>> + const struct slice_mask *available,
>> int psize, unsigned long high_limit)
>> {
>> int pshift = max_t(int, mmu_psize_defs[psize].shift, PAGE_SHIFT);
>> @@ -324,7 +328,7 @@ static unsigned long slice_find_area_bottomup(struct mm_struct *mm,
>>
>> static unsigned long slice_find_area_topdown(struct mm_struct *mm,
>> unsigned long len,
>> - struct slice_mask available,
>> + const struct slice_mask *available,
>> int psize, unsigned long high_limit)
>> {
>> int pshift = max_t(int, mmu_psize_defs[psize].shift, PAGE_SHIFT);
>> @@ -382,7 +386,7 @@ static unsigned long slice_find_area_topdown(struct mm_struct *mm,
>>
>>
>> static unsigned long slice_find_area(struct mm_struct *mm, unsigned long len,
>> - struct slice_mask mask, int psize,
>> + const struct slice_mask *mask, int psize,
>> int topdown, unsigned long high_limit)
>> {
>> if (topdown)
>> @@ -391,14 +395,16 @@ static unsigned long slice_find_area(struct mm_struct *mm, unsigned long len,
>> return slice_find_area_bottomup(mm, len, mask, psize, high_limit);
>> }
>>
>> -static inline void slice_or_mask(struct slice_mask *dst, struct slice_mask *src)
>> +static inline void slice_or_mask(struct slice_mask *dst,
>> + const struct slice_mask *src)
>> {
>> dst->low_slices |= src->low_slices;
>> slice_bitmap_or(dst->high_slices, dst->high_slices, src->high_slices,
>> SLICE_NUM_HIGH);
>> }
>>
>> -static inline void slice_andnot_mask(struct slice_mask *dst, struct slice_mask *src)
>> +static inline void slice_andnot_mask(struct slice_mask *dst,
>> + const struct slice_mask *src)
>> {
>> dst->low_slices &= ~src->low_slices;
>>
>> @@ -483,7 +489,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
>> * already
>> */
>> slice_mask_for_size(mm, psize, &good_mask, high_limit);
>> - slice_print_mask(" good_mask", good_mask);
>> + slice_print_mask(" good_mask", &good_mask);
>>
>> /*
>> * Here "good" means slices that are already the right page size,
>> @@ -517,12 +523,12 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
>> if (addr != 0 || fixed) {
>> /* Build a mask for the requested range */
>> slice_range_to_mask(addr, len, &mask);
>> - slice_print_mask(" mask", mask);
>> + slice_print_mask(" mask", &mask);
>>
>> /* Check if we fit in the good mask. If we do, we just return,
>> * nothing else to do
>> */
>> - if (slice_check_fit(mm, mask, good_mask)) {
>> + if (slice_check_fit(mm, &mask, &good_mask)) {
>> slice_dbg(" fits good !\n");
>> return addr;
>> }
>> @@ -530,7 +536,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
>> /* Now let's see if we can find something in the existing
>> * slices for that size
>> */
>> - newaddr = slice_find_area(mm, len, good_mask,
>> + newaddr = slice_find_area(mm, len, &good_mask,
>> psize, topdown, high_limit);
>> if (newaddr != -ENOMEM) {
>> /* Found within the good mask, we don't have to setup,
>> @@ -546,9 +552,10 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
>> */
>> slice_mask_for_free(mm, &potential_mask, high_limit);
>> slice_or_mask(&potential_mask, &good_mask);
>> - slice_print_mask(" potential", potential_mask);
>> + slice_print_mask(" potential", &potential_mask);
>>
>> - if ((addr != 0 || fixed) && slice_check_fit(mm, mask, potential_mask)) {
>> + if ((addr != 0 || fixed) &&
>> + slice_check_fit(mm, &mask, &potential_mask)) {
>> slice_dbg(" fits potential !\n");
>> goto convert;
>> }
>> @@ -563,7 +570,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
>> * anywhere in the good area.
>> */
>> if (addr) {
>> - addr = slice_find_area(mm, len, good_mask,
>> + addr = slice_find_area(mm, len, &good_mask,
>> psize, topdown, high_limit);
>> if (addr != -ENOMEM) {
>> slice_dbg(" found area at 0x%lx\n", addr);
>> @@ -574,14 +581,14 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
>> /* Now let's see if we can find something in the existing slices
>> * for that size plus free slices
>> */
>> - addr = slice_find_area(mm, len, potential_mask,
>> + addr = slice_find_area(mm, len, &potential_mask,
>> psize, topdown, high_limit);
>>
>> #ifdef CONFIG_PPC_64K_PAGES
>> if (addr == -ENOMEM && psize == MMU_PAGE_64K) {
>> /* retry the search with 4k-page slices included */
>> slice_or_mask(&potential_mask, &compat_mask);
>> - addr = slice_find_area(mm, len, potential_mask,
>> + addr = slice_find_area(mm, len, &potential_mask,
>> psize, topdown, high_limit);
>> }
>> #endif
>> @@ -591,14 +598,14 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
>>
>> slice_range_to_mask(addr, len, &mask);
>> slice_dbg(" found potential area at 0x%lx\n", addr);
>> - slice_print_mask(" mask", mask);
>> + slice_print_mask(" mask", &mask);
>>
>> convert:
>> slice_andnot_mask(&mask, &good_mask);
>> slice_andnot_mask(&mask, &compat_mask);
>> if (mask.low_slices ||
>> !slice_bitmap_empty(mask.high_slices, SLICE_NUM_HIGH)) {
>> - slice_convert(mm, mask, psize);
>> + slice_convert(mm, &mask, psize);
>> if (psize > MMU_PAGE_BASE)
>> on_each_cpu(slice_flush_segments, mm, 1);
>> }
>> @@ -727,7 +734,7 @@ void slice_set_range_psize(struct mm_struct *mm, unsigned long start,
>> VM_BUG_ON(radix_enabled());
>>
>> slice_range_to_mask(start, len, &mask);
>> - slice_convert(mm, mask, psize);
>> + slice_convert(mm, &mask, psize);
>> }
>>
>> #ifdef CONFIG_HUGETLB_PAGE
>> @@ -774,9 +781,9 @@ int is_hugepage_only_range(struct mm_struct *mm, unsigned long addr,
>> #if 0 /* too verbose */
>> slice_dbg("is_hugepage_only_range(mm=%p, addr=%lx, len=%lx)\n",
>> mm, addr, len);
>> - slice_print_mask(" mask", mask);
>> - slice_print_mask(" available", available);
>> + slice_print_mask(" mask", &mask);
>> + slice_print_mask(" available", &available);
>> #endif
>> - return !slice_check_fit(mm, mask, available);
>> + return !slice_check_fit(mm, &mask, &available);
>> }
>> #endif
>> --
>> 2.13.3
^ permalink raw reply
* Re: [PATCH V2 4/4] powerpc/mm/hash64: Increase the VA range
From: Aneesh Kumar K.V @ 2018-02-27 4:02 UTC (permalink / raw)
To: Murilo Opsfelder Araujo, benh, paulus, mpe; +Cc: linuxppc-dev
In-Reply-To: <5876ab5b-237f-6e05-62e9-59fb2fe4a8f1@linux.vnet.ibm.com>
Murilo Opsfelder Araujo <muriloo@linux.vnet.ibm.com> writes:
> On 02/26/2018 11:08 AM, Aneesh Kumar K.V wrote:
>> ---
>> arch/powerpc/include/asm/book3s/64/hash-64k.h | 2 +-
>> arch/powerpc/include/asm/processor.h | 9 ++++++++-
>> arch/powerpc/mm/init_64.c | 6 ------
>> arch/powerpc/mm/pgtable_64.c | 5 -----
>> 4 files changed, 9 insertions(+), 13 deletions(-)
>
> Hi, Aneesh.
>
> This patch is missing Signed-off-by: line. You're encouraged to run
> checkpatch.pl to inspect your patches.
>
> You may also want to add a brief paragraph in the commit message
> explaining the "why" of this change.
>
Will update.
-aneesh
^ permalink raw reply
* Re: [PATCH V2 2/4] powerpc/mm/slice: Reduce the stack usage in slice_get_unmapped_area
From: Aneesh Kumar K.V @ 2018-02-27 4:01 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: Benjamin Herrenschmidt, paulus, mpe, linuxppc-dev
In-Reply-To: <CANashXPZ=KZdixoqCoO7hTy4z7kJLqAst6C-77fg7OdAdfe8EA@mail.gmail.com>
Nicholas Piggin <nicholas.piggin@gmail.com> writes:
> I had a series which goes significantly further with stack reduction. What
> do you think about just going with that?
I am yet to review that. What I did here is minimum that is required to
get 4PB series compiled.
>
> I wonder if we should switch to dynamically allocating the slice stuff on
> ppc64
-aneesh
^ permalink raw reply
* Re: [PATCH V2] powerpc: Don't do runtime futex_cmpxchg test
From: Aneesh Kumar K.V @ 2018-02-27 4:00 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: Benjamin Herrenschmidt, paulus, mpe, linuxppc-dev
In-Reply-To: <CANashXPc952+httE=Vhx=wGrkN3Ycva7j+a+WE6jXQaw38p=YQ@mail.gmail.com>
Nicholas Piggin <nicholas.piggin@gmail.com> writes:
> On 27 Feb. 2018 00:34, "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
> wrote:
>
> futex_detect_cmpxchg() does a cmpxchg_futex_value_locked on a NULL user
> addr to
> runtime detect whether architecture implements atomic cmpxchg for futex.
> POWER
> do implement the feature and hence we can enable the config instead of
> depending
> on runtime detection.
>
> We could possible enable this on everything. For now limitted to book3s_64
>
>
> I think everything implements it? Might as well do it all at once.
>
> I wouldn't mind putting in an explicit null dereference test if we take
> this out.
If the config is not selected we do that with futex_detect_cmpxchg()
right? Or are you suggesting something else?
>
> Thanks,
> Nick
>
>
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
> arch/powerpc/platforms/Kconfig.cputype | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/powerpc/platforms/Kconfig.cputype
> b/arch/powerpc/platforms/Kconfig.cputype
> index a429d859f15d..31bc2bd5dfd1 100644
> --- a/arch/powerpc/platforms/Kconfig.cputype
> +++ b/arch/powerpc/platforms/Kconfig.cputype
> @@ -75,6 +75,7 @@ config PPC_BOOK3S_64
> select ARCH_SUPPORTS_NUMA_BALANCING
> select IRQ_WORK
> select HAVE_KERNEL_XZ
> + select HAVE_FUTEX_CMPXCHG if FUTEX
>
> config PPC_BOOK3E_64
> bool "Embedded processors"
> --
> 2.14.3
^ permalink raw reply
* Re: [PATCH 15/27] cpufreq: powerenv: Don't validate the frequency table twice
From: Viresh Kumar @ 2018-02-27 3:45 UTC (permalink / raw)
To: Michael Ellerman
Cc: Rafael Wysocki, Benjamin Herrenschmidt, Paul Mackerras, linux-pm,
Vincent Guittot, linuxppc-dev
In-Reply-To: <871sh89dbr.fsf@concordia.ellerman.id.au>
On 26-02-18, 22:53, Michael Ellerman wrote:
> Viresh Kumar <viresh.kumar@linaro.org> writes:
> > Subject: Re: [PATCH 15/27] cpufreq: powerenv: Don't validate the frequency table twice
> ^
> powernv
>
> > The cpufreq core is already validating the CPU frequency table after
> > calling the ->init() callback of the cpufreq drivers and the drivers
> > don't need to do the same anymore. Though they need to set the
> > policy->freq_table field directly from the ->init() callback now.
> >
> > Stop validating the frequency table from powerenv driver.
powernv :)
Will fix both of them.
--
viresh
^ permalink raw reply
* Re: [PATCH v2] powerpc/npu: Cleanup MMIO ATSD flushing
From: Alistair Popple @ 2018-02-27 0:47 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Balbir Singh, Michael Ellerman, Aneesh Kumar KV
In-Reply-To: <CAKTCnzkO+NWKDwj8RE_jX_N0uu-vY5HRf4AdkJyjsLvL1Loqhg@mail.gmail.com>
> @aneesh can you please look at this? @mpe can we pick this up if there
> are no objections?
@mpe any objections to picking this up for this release? Or do you want to wait
for the next one? (there are likely more bugfixes coming for ATS support).
- Alistair
> Balbir Singh
>
^ permalink raw reply
* Re: [PATCH] xmon: Setup xmon debugger hooks when first break-point is set
From: Balbir Singh @ 2018-02-27 0:26 UTC (permalink / raw)
To: Vaibhav Jain, linuxppc-dev, Michael Ellerman
Cc: Benjamin Herrenschmidt, Paul Mackerras, Nicholas Piggin,
Douglas Miller, Frederic Barrat
In-Reply-To: <20180226113607.20321-1-vaibhav@linux.vnet.ibm.com>
On Mon, 2018-02-26 at 17:06 +0530, Vaibhav Jain wrote:
> Presently sysrq key for xmon('x') is registered during kernel init
> irrespective of the value of kernel param 'xmon'. Thus xmon is enabled
> even if 'xmon=off' is passed on the kernel command line. However this
> doesn't enable the kernel debugger hooks needed for instruction or data
> breakpoints. Thus when a break-point is hit with xmon=off a kernel oops
> of the form below is reported:
>
> Oops: Exception in kernel mode, sig: 5 [#1]
> < snip >
> Trace/breakpoint trap
>
> To fix this the patch checks and enables debugger hooks when an
> instruction or data break-point is set via xmon console. It also clears
> all breakpoints when xmon is disabled via debugfs.
>
> Signed-off-by: Vaibhav Jain <vaibhav@linux.vnet.ibm.com>
> ---
> arch/powerpc/xmon/xmon.c | 34 +++++++++++++++++++++++++++++++++-
> 1 file changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index 82e1a3ee6e0f..3679f5417a7e 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -1295,6 +1295,7 @@ bpt_cmds(void)
> switch (cmd) {
> #ifndef CONFIG_PPC_8xx
> static const char badaddr[] = "Only kernel addresses are permitted for breakpoints\n";
> + static const char warnxmon[] = "xmon: Enabling debugger hooks\n";
> int mode;
> case 'd': /* bd - hardware data breakpoint */
> mode = 7;
> @@ -1315,6 +1316,11 @@ bpt_cmds(void)
> dabr.address &= ~HW_BRK_TYPE_DABR;
> dabr.enabled = mode | BP_DABR;
> }
> + /* Enable xmon hooks if needed */
> + if (!xmon_on) {
> + printf(warnxmon);
> + xmon_on = 1;
> + }
Can we get this three liner into an inline function?
> break;
>
> case 'i': /* bi - hardware instr breakpoint */
> @@ -1335,6 +1341,12 @@ bpt_cmds(void)
> if (bp != NULL) {
> bp->enabled |= BP_CIABR;
> iabr = bp;
> +
> + /* Enable xmon hooks if needed */
> + if (!xmon_on) {
> + printf(warnxmon);
> + xmon_on = 1;
> + }
> }
> break;
> #endif
> @@ -1399,8 +1411,15 @@ bpt_cmds(void)
> if (!check_bp_loc(a))
> break;
> bp = new_breakpoint(a);
> - if (bp != NULL)
> + if (bp != NULL) {
> bp->enabled |= BP_TRAP;
> +
> + /* Enable xmon hooks if needed */
> + if (!xmon_on) {
> + printf(warnxmon);
> + xmon_on = 1;
> + }
> + }
> break;
> }
> }
> @@ -3651,9 +3670,22 @@ device_initcall(setup_xmon_sysrq);
> #ifdef CONFIG_DEBUG_FS
> static int xmon_dbgfs_set(void *data, u64 val)
> {
> + int i;
> +
> xmon_on = !!val;
> xmon_init(xmon_on);
>
> + /* make sure all breakpoints removed when disabling */
> + if (!xmon_on) {
> + for (i = 0; i < NBPTS; ++i)
> + bpts[i].enabled = 0;
> + /* if anything set inform user that breakpoints are cleared */
> + if (iabr || dabr.enabled)
> + pr_info("xmon: All breakpoints cleared\n");
> +
> + iabr = NULL;
> + dabr.enabled = 0;
Is this sufficient or do we need remove_bpts()
Balbir Singh
^ 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