* Re: [PATCH 1/2] dma-mapping: add a dma_ops_bypass flag to struct device
From: Alexey Kardashevskiy @ 2020-04-07 10:12 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Aneesh Kumar K.V, Joerg Roedel, Robin Murphy, linux-kernel, iommu,
Greg Kroah-Hartman, linuxppc-dev, Lu Baolu
In-Reply-To: <20200406171706.GA3231@lst.de>
On 07/04/2020 03:17, Christoph Hellwig wrote:
> On Mon, Apr 06, 2020 at 11:25:09PM +1000, Alexey Kardashevskiy wrote:
>>>> Do you see any serious problem with this approach? Thanks!
>>>
>>> Do you have a link to the whole branch? The github UI is unfortunately
>>> unusable for that (or I'm missing something).
>>
>> The UI shows the branch but since I rebased and forcepushed it, it does
>> not. Here is the current one with:
>>
>> https://github.com/aik/linux/commits/dma-bypass.3
>
> Ok, so we use the core bypass without persistent memory, and then
> have another bypass mode on top. Not great, but I can't think
> of anything better. Note that your checks for the map_sg case
> aren't very efficient - for one it would make sense to calculate
> the limit only once,
Good points, I'll post revised version when you post your v3 of this.
> but also it would make sense to reuse the
> calculted diecect mapping addresses instead of doing another pass
> later on in the dma-direct code.
Probably but I wonder what kind of hardware we need to see the
difference. I might try, just need to ride to the office to plug the
cable in my 100GBit eth machines :) Thanks,
--
Alexey
^ permalink raw reply
* Re: [PATCH v12 5/8] powerpc/64: make buildable without CONFIG_COMPAT
From: Michal Suchánek @ 2020-04-07 9:57 UTC (permalink / raw)
To: Christophe Leroy
Cc: Mark Rutland, Gustavo Luiz Duarte, Peter Zijlstra,
Sebastian Andrzej Siewior, linux-kernel, Paul Mackerras,
Jiri Olsa, Rob Herring, Michael Neuling, Mauro Carvalho Chehab,
Masahiro Yamada, Nayna Jain, Alexander Shishkin, Ingo Molnar,
Allison Randal, Jordan Niethe, Valentin Schneider, Arnd Bergmann,
Arnaldo Carvalho de Melo, Alexander Viro, Jonathan Cameron,
Namhyung Kim, Thomas Gleixner, Andy Shevchenko, Hari Bathini,
Greg Kroah-Hartman, Nicholas Piggin, Claudio Carvalho,
Eric Richter, Eric W. Biederman, linux-fsdevel, linuxppc-dev,
David S. Miller, Thiago Jung Bauermann
In-Reply-To: <b420b304-05e9-df58-7149-31169b0b01e2@c-s.fr>
On Tue, Apr 07, 2020 at 07:50:30AM +0200, Christophe Leroy wrote:
>
>
> Le 20/03/2020 à 11:20, Michal Suchanek a écrit :
> > There are numerous references to 32bit functions in generic and 64bit
> > code so ifdef them out.
> >
> > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > ---
> > v2:
> > - fix 32bit ifdef condition in signal.c
> > - simplify the compat ifdef condition in vdso.c - 64bit is redundant
> > - simplify the compat ifdef condition in callchain.c - 64bit is redundant
> > v3:
> > - use IS_ENABLED and maybe_unused where possible
> > - do not ifdef declarations
> > - clean up Makefile
> > v4:
> > - further makefile cleanup
> > - simplify is_32bit_task conditions
> > - avoid ifdef in condition by using return
> > v5:
> > - avoid unreachable code on 32bit
> > - make is_current_64bit constant on !COMPAT
> > - add stub perf_callchain_user_32 to avoid some ifdefs
> > v6:
> > - consolidate current_is_64bit
> > v7:
> > - remove leftover perf_callchain_user_32 stub from previous series version
> > v8:
> > - fix build again - too trigger-happy with stub removal
> > - remove a vdso.c hunk that causes warning according to kbuild test robot
> > v9:
> > - removed current_is_64bit in previous patch
> > v10:
> > - rebase on top of 70ed86f4de5bd
> > ---
> > arch/powerpc/include/asm/thread_info.h | 4 ++--
> > arch/powerpc/kernel/Makefile | 6 +++---
> > arch/powerpc/kernel/entry_64.S | 2 ++
> > arch/powerpc/kernel/signal.c | 3 +--
> > arch/powerpc/kernel/syscall_64.c | 6 ++----
> > arch/powerpc/kernel/vdso.c | 3 ++-
> > arch/powerpc/perf/callchain.c | 8 +++++++-
> > 7 files changed, 19 insertions(+), 13 deletions(-)
> >
>
> [...]
>
> > diff --git a/arch/powerpc/kernel/syscall_64.c b/arch/powerpc/kernel/syscall_64.c
> > index 87d95b455b83..2dcbfe38f5ac 100644
> > --- a/arch/powerpc/kernel/syscall_64.c
> > +++ b/arch/powerpc/kernel/syscall_64.c
> > @@ -24,7 +24,6 @@ notrace long system_call_exception(long r3, long r4, long r5,
> > long r6, long r7, long r8,
> > unsigned long r0, struct pt_regs *regs)
> > {
> > - unsigned long ti_flags;
> > syscall_fn f;
> > if (IS_ENABLED(CONFIG_PPC_IRQ_SOFT_MASK_DEBUG))
> > @@ -68,8 +67,7 @@ notrace long system_call_exception(long r3, long r4, long r5,
> > local_irq_enable();
> > - ti_flags = current_thread_info()->flags;
> > - if (unlikely(ti_flags & _TIF_SYSCALL_DOTRACE)) {
> > + if (unlikely(current_thread_info()->flags & _TIF_SYSCALL_DOTRACE)) {
> > /*
> > * We use the return value of do_syscall_trace_enter() as the
> > * syscall number. If the syscall was rejected for any reason
> > @@ -94,7 +92,7 @@ notrace long system_call_exception(long r3, long r4, long r5,
> > /* May be faster to do array_index_nospec? */
> > barrier_nospec();
> > - if (unlikely(ti_flags & _TIF_32BIT)) {
> > + if (unlikely(is_32bit_task())) {
>
> is_compat() should be used here instead, because we dont want to use
is_compat_task()
> compat_sys_call_table() on PPC32.
>
> > f = (void *)compat_sys_call_table[r0];
> > r3 &= 0x00000000ffffffffULL;
>
That only applies once you use this for 32bit as well. Right now it's
64bit only so the two are the same.
Thanks
Michal
^ permalink raw reply
* Re: [PATCH v2 1/4] powerpc/64s: implement probe_kernel_read/write without touching AMR
From: Daniel Borkmann @ 2020-04-07 9:09 UTC (permalink / raw)
To: Nicholas Piggin, Christophe Leroy, linuxppc-dev
Cc: linux-arch, x86, Alexei Starovoitov, linux-kernel,
Masami Hiramatsu, Linus Torvalds
In-Reply-To: <1586230235.0xvc3pjkcj.astroid@bobo.none>
Hey Nicholas,
On 4/7/20 6:01 AM, Nicholas Piggin wrote:
> Nicholas Piggin's on April 3, 2020 9:05 pm:
>> Christophe Leroy's on April 3, 2020 8:31 pm:
>>> Le 03/04/2020 à 11:35, Nicholas Piggin a écrit :
>>>> There is no need to allow user accesses when probing kernel addresses.
>>>
>>> I just discovered the following commit
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=75a1a607bb7e6d918be3aca11ec2214a275392f4
>>>
>>> This commit adds probe_kernel_read_strict() and probe_kernel_write_strict().
>>>
>>> When reading the commit log, I understand that probe_kernel_read() may
>>> be used to access some user memory. Which will not work anymore with
>>> your patch.
>>
>> Hmm, I looked at _strict but obviously not hard enough. Good catch.
>>
>> I don't think probe_kernel_read() should ever access user memory,
>> the comment certainly says it doesn't, but that patch sort of implies
>> that they do.
>>
>> I think it's wrong. The non-_strict maybe could return userspace data to
>> you if you did pass a user address? I don't see why that shouldn't just
>> be disallowed always though.
>>
>> And if the _strict version is required to be safe, then it seems like a
>> bug or security issue to just allow everyone that doesn't explicitly
>> override it to use the default implementation.
>>
>> Also, the way the weak linkage is done in that patch, means parisc and
>> um archs that were previously overriding probe_kernel_read() now get
>> the default probe_kernel_read_strict(), which would be wrong for them.
>
> The changelog in commit 75a1a607bb7 makes it a bit clearer. If the
> non-_strict variant is used on non-kernel addresses, then it might not
> return -EFAULT or it might cause a kernel warning. The _strict variant
> is supposed to be usable with any address and it will return -EFAULT if
> it was not a valid and mapped kernel address.
>
> The non-_strict variant can not portably access user memory because it
> uses KERNEL_DS, and its documentation says its only for kernel pointers.
> So powerpc should be fine to run that under KUAP AFAIKS.
>
> I don't know why the _strict behaviour is not just made default, but
> the implementation of it does seem to be broken on the archs that
> override the non-_strict variant.
Yeah, we should make it default and only add a "opt out" for the old legacy
cases; there was also same discussion started over here just recently [0].
Thanks,
Daniel
[0] https://lore.kernel.org/lkml/20200403133533.GA3424@infradead.org/T/
^ permalink raw reply
* Re: [PATCH v5 0/5] Track and expose idle PURR and SPURR ticks
From: Naveen N. Rao @ 2020-04-07 9:05 UTC (permalink / raw)
To: Gautham R. Shenoy, Kamalesh Babulal, Michael Ellerman,
Nathan Lynch, Vaidyanathan Srinivasan, Tyrel Datwyler
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1586249263-14048-1-git-send-email-ego@linux.vnet.ibm.com>
Gautham R. Shenoy wrote:
> From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
>
> Hi,
>
> This is the fifth version of the patches to track and expose idle PURR
> and SPURR ticks. These patches are required by tools such as lparstat
> to compute system utilization for capacity planning purposes.
>
> The previous versions can be found here:
> v4: https://lkml.org/lkml/2020/3/27/323
> v3: https://lkml.org/lkml/2020/3/11/331
> v2: https://lkml.org/lkml/2020/2/21/21
> v1: https://lore.kernel.org/patchwork/cover/1159341/
>
> They changes from v4 are:
>
> - As suggested by Naveen, moved the functions read_this_idle_purr()
> and read_this_idle_spurr() from Patch 2 and Patch 3 respectively
> to Patch 4 where it is invoked.
>
> - Dropped Patch 6 which cached the values of purr, spurr,
> idle_purr, idle_spurr in order to minimize the number of IPIs
> sent.
>
> - Updated the dates for the idle_purr, idle_spurr in the
> Documentation Patch 5.
>
> Motivation:
> ===========
> On PSeries LPARs, the data centers planners desire a more accurate
> view of system utilization per resource such as CPU to plan the system
> capacity requirements better. Such accuracy can be obtained by reading
> PURR/SPURR registers for CPU resource utilization.
>
> Tools such as lparstat which are used to compute the utilization need
> to know [S]PURR ticks when the cpu was busy or idle. The [S]PURR
> counters are already exposed through sysfs. We already account for
> PURR ticks when we go to idle so that we can update the VPA area. This
> patchset extends support to account for SPURR ticks when idle, and
> expose both via per-cpu sysfs files.
>
> These patches are required for enhancement to the lparstat utility
> that compute the CPU utilization based on PURR and SPURR which can be
> found here :
> https://groups.google.com/forum/#!topic/powerpc-utils-devel/fYRo69xO9r4
>
>
> With the patches, when lparstat is run on a LPAR running CPU-Hogs,
> =========================================================================
> sudo ./src/lparstat -E 1 3
>
> System Configuration
> type=Dedicated mode=Capped smt=8 lcpu=2 mem=4834112 kB cpus=0 ent=2.00
>
> ---Actual--- -Normalized-
> %busy %idle Frequency %busy %idle
> ------ ------ ------------- ------ ------
> 1 99.99 0.00 3.35GHz[111%] 110.99 0.00
> 2 100.00 0.00 3.35GHz[111%] 111.01 0.00
> 3 100.00 0.00 3.35GHz[111%] 111.00 0.00
>
> With patches, when lparstat is run on and idle LPAR
> =========================================================================
> System Configuration
> type=Dedicated mode=Capped smt=8 lcpu=2 mem=4834112 kB cpus=0 ent=2.00
> ---Actual--- -Normalized-
> %busy %idle Frequency %busy %idle
> ------ ------ ------------- ------ ------
> 1 0.15 99.84 2.17GHz[ 72%] 0.11 71.89
> 2 0.24 99.76 2.11GHz[ 70%] 0.18 69.82
> 3 0.24 99.75 2.11GHz[ 70%] 0.18 69.81
>
> Gautham R. Shenoy (5):
> powerpc: Move idle_loop_prolog()/epilog() functions to header file
> powerpc/idle: Store PURR snapshot in a per-cpu global variable
> powerpc/pseries: Account for SPURR ticks on idle CPUs
> powerpc/sysfs: Show idle_purr and idle_spurr for every CPU
> Documentation: Document sysfs interfaces purr, spurr, idle_purr,
> idle_spurr
Thanks, LGTM. For the series:
Acked-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
- Naveen
^ permalink raw reply
* [PATCH v5 5/5] Documentation: Document sysfs interfaces purr, spurr, idle_purr, idle_spurr
From: Gautham R. Shenoy @ 2020-04-07 8:47 UTC (permalink / raw)
To: Nathan Lynch, Michael Ellerman, Vaidyanathan Srinivasan,
Kamalesh Babulal, Naveen N. Rao, Tyrel Datwyler
Cc: Gautham R. Shenoy, linuxppc-dev, linux-kernel
In-Reply-To: <1586249263-14048-1-git-send-email-ego@linux.vnet.ibm.com>
From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
Add documentation for the following sysfs interfaces:
/sys/devices/system/cpu/cpuX/purr
/sys/devices/system/cpu/cpuX/spurr
/sys/devices/system/cpu/cpuX/idle_purr
/sys/devices/system/cpu/cpuX/idle_spurr
Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
---
Documentation/ABI/testing/sysfs-devices-system-cpu | 39 ++++++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-devices-system-cpu b/Documentation/ABI/testing/sysfs-devices-system-cpu
index 2e0e3b4..b73b8b5 100644
--- a/Documentation/ABI/testing/sysfs-devices-system-cpu
+++ b/Documentation/ABI/testing/sysfs-devices-system-cpu
@@ -580,3 +580,42 @@ Description: Secure Virtual Machine
If 1, it means the system is using the Protected Execution
Facility in POWER9 and newer processors. i.e., it is a Secure
Virtual Machine.
+
+What: /sys/devices/system/cpu/cpuX/purr
+Date: Apr 2005
+Contact: Linux for PowerPC mailing list <linuxppc-dev@ozlabs.org>
+Description: PURR ticks for this CPU since the system boot.
+
+ The Processor Utilization Resources Register (PURR) is
+ a 64-bit counter which provides an estimate of the
+ resources used by the CPU thread. The contents of this
+ register increases monotonically. This sysfs interface
+ exposes the number of PURR ticks for cpuX.
+
+What: /sys/devices/system/cpu/cpuX/spurr
+Date: Dec 2006
+Contact: Linux for PowerPC mailing list <linuxppc-dev@ozlabs.org>
+Description: SPURR ticks for this CPU since the system boot.
+
+ The Scaled Processor Utilization Resources Register
+ (SPURR) is a 64-bit counter that provides a frequency
+ invariant estimate of the resources used by the CPU
+ thread. The contents of this register increases
+ monotonically. This sysfs interface exposes the number
+ of SPURR ticks for cpuX.
+
+What: /sys/devices/system/cpu/cpuX/idle_purr
+Date: Apr 2020
+Contact: Linux for PowerPC mailing list <linuxppc-dev@ozlabs.org>
+Description: PURR ticks for cpuX when it was idle.
+
+ This sysfs interface exposes the number of PURR ticks
+ for cpuX when it was idle.
+
+What: /sys/devices/system/cpu/cpuX/idle_spurr
+Date: Apr 2020
+Contact: Linux for PowerPC mailing list <linuxppc-dev@ozlabs.org>
+Description: SPURR ticks for cpuX when it was idle.
+
+ This sysfs interface exposes the number of SPURR ticks
+ for cpuX when it was idle.
--
1.9.4
^ permalink raw reply related
* [PATCH v5 4/5] powerpc/sysfs: Show idle_purr and idle_spurr for every CPU
From: Gautham R. Shenoy @ 2020-04-07 8:47 UTC (permalink / raw)
To: Nathan Lynch, Michael Ellerman, Vaidyanathan Srinivasan,
Kamalesh Babulal, Naveen N. Rao, Tyrel Datwyler
Cc: Gautham R. Shenoy, linuxppc-dev, linux-kernel
In-Reply-To: <1586249263-14048-1-git-send-email-ego@linux.vnet.ibm.com>
From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
On Pseries LPARs, to calculate utilization, we need to know the
[S]PURR ticks when the CPUs were busy or idle.
The total PURR and SPURR ticks are already exposed via the per-cpu
sysfs files "purr" and "spurr". This patch adds support for exposing
the idle PURR and SPURR ticks via new per-cpu sysfs files named
"idle_purr" and "idle_spurr".
This patch also adds helper functions to accurately read the values of
idle_purr and idle_spurr especially from an interrupt context between
when the interrupt has occurred between the pseries_idle_prolog() and
pseries_idle_epilog(). This will ensure that the idle purr/spurr
values corresponding to the latest idle period is accounted for before
these values are read.
Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/idle.h | 32 ++++++++++++++++
arch/powerpc/kernel/sysfs.c | 82 +++++++++++++++++++++++++++++++++++++++--
2 files changed, 111 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/asm/idle.h b/arch/powerpc/include/asm/idle.h
index 0efb250..accd1f5 100644
--- a/arch/powerpc/include/asm/idle.h
+++ b/arch/powerpc/include/asm/idle.h
@@ -57,5 +57,37 @@ static inline void pseries_idle_epilog(void)
ppc64_runlatch_on();
}
+static inline u64 read_this_idle_purr(void)
+{
+ /*
+ * If we are reading from an idle context, update the
+ * idle-purr cycles corresponding to the last idle period.
+ * Since the idle context is not yet over, take a fresh
+ * snapshot of the idle-purr.
+ */
+ if (unlikely(get_lppaca()->idle == 1)) {
+ update_idle_purr_accounting();
+ snapshot_purr_idle_entry();
+ }
+
+ return be64_to_cpu(get_lppaca()->wait_state_cycles);
+}
+
+static inline u64 read_this_idle_spurr(void)
+{
+ /*
+ * If we are reading from an idle context, update the
+ * idle-spurr cycles corresponding to the last idle period.
+ * Since the idle context is not yet over, take a fresh
+ * snapshot of the idle-spurr.
+ */
+ if (get_lppaca()->idle == 1) {
+ update_idle_spurr_accounting();
+ snapshot_spurr_idle_entry();
+ }
+
+ return *this_cpu_ptr(&idle_spurr_cycles);
+}
+
#endif /* CONFIG_PPC_PSERIES */
#endif
diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
index 479c706..571b325 100644
--- a/arch/powerpc/kernel/sysfs.c
+++ b/arch/powerpc/kernel/sysfs.c
@@ -19,6 +19,7 @@
#include <asm/smp.h>
#include <asm/pmc.h>
#include <asm/firmware.h>
+#include <asm/idle.h>
#include <asm/svm.h>
#include "cacheinfo.h"
@@ -760,6 +761,74 @@ static void create_svm_file(void)
}
#endif /* CONFIG_PPC_SVM */
+#ifdef CONFIG_PPC_PSERIES
+static void read_idle_purr(void *val)
+{
+ u64 *ret = val;
+
+ *ret = read_this_idle_purr();
+}
+
+static ssize_t idle_purr_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct cpu *cpu = container_of(dev, struct cpu, dev);
+ u64 val;
+
+ smp_call_function_single(cpu->dev.id, read_idle_purr, &val, 1);
+ return sprintf(buf, "%llx\n", val);
+}
+static DEVICE_ATTR(idle_purr, 0400, idle_purr_show, NULL);
+
+static void create_idle_purr_file(struct device *s)
+{
+ if (firmware_has_feature(FW_FEATURE_LPAR))
+ device_create_file(s, &dev_attr_idle_purr);
+}
+
+static void remove_idle_purr_file(struct device *s)
+{
+ if (firmware_has_feature(FW_FEATURE_LPAR))
+ device_remove_file(s, &dev_attr_idle_purr);
+}
+
+static void read_idle_spurr(void *val)
+{
+ u64 *ret = val;
+
+ *ret = read_this_idle_spurr();
+}
+
+static ssize_t idle_spurr_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct cpu *cpu = container_of(dev, struct cpu, dev);
+ u64 val;
+
+ smp_call_function_single(cpu->dev.id, read_idle_spurr, &val, 1);
+ return sprintf(buf, "%llx\n", val);
+}
+static DEVICE_ATTR(idle_spurr, 0400, idle_spurr_show, NULL);
+
+static void create_idle_spurr_file(struct device *s)
+{
+ if (firmware_has_feature(FW_FEATURE_LPAR))
+ device_create_file(s, &dev_attr_idle_spurr);
+}
+
+static void remove_idle_spurr_file(struct device *s)
+{
+ if (firmware_has_feature(FW_FEATURE_LPAR))
+ device_remove_file(s, &dev_attr_idle_spurr);
+}
+
+#else /* CONFIG_PPC_PSERIES */
+#define create_idle_purr_file(s)
+#define remove_idle_purr_file(s)
+#define create_idle_spurr_file(s)
+#define remove_idle_spurr_file(s)
+#endif /* CONFIG_PPC_PSERIES */
+
static int register_cpu_online(unsigned int cpu)
{
struct cpu *c = &per_cpu(cpu_devices, cpu);
@@ -823,10 +892,13 @@ static int register_cpu_online(unsigned int cpu)
if (!firmware_has_feature(FW_FEATURE_LPAR))
add_write_permission_dev_attr(&dev_attr_purr);
device_create_file(s, &dev_attr_purr);
+ create_idle_purr_file(s);
}
- if (cpu_has_feature(CPU_FTR_SPURR))
+ if (cpu_has_feature(CPU_FTR_SPURR)) {
device_create_file(s, &dev_attr_spurr);
+ create_idle_spurr_file(s);
+ }
if (cpu_has_feature(CPU_FTR_DSCR))
device_create_file(s, &dev_attr_dscr);
@@ -910,11 +982,15 @@ static int unregister_cpu_online(unsigned int cpu)
device_remove_file(s, &dev_attr_mmcra);
#endif /* CONFIG_PMU_SYSFS */
- if (cpu_has_feature(CPU_FTR_PURR))
+ if (cpu_has_feature(CPU_FTR_PURR)) {
device_remove_file(s, &dev_attr_purr);
+ remove_idle_purr_file(s);
+ }
- if (cpu_has_feature(CPU_FTR_SPURR))
+ if (cpu_has_feature(CPU_FTR_SPURR)) {
device_remove_file(s, &dev_attr_spurr);
+ remove_idle_spurr_file(s);
+ }
if (cpu_has_feature(CPU_FTR_DSCR))
device_remove_file(s, &dev_attr_dscr);
--
1.9.4
^ permalink raw reply related
* [PATCH v5 2/5] powerpc/idle: Store PURR snapshot in a per-cpu global variable
From: Gautham R. Shenoy @ 2020-04-07 8:47 UTC (permalink / raw)
To: Nathan Lynch, Michael Ellerman, Vaidyanathan Srinivasan,
Kamalesh Babulal, Naveen N. Rao, Tyrel Datwyler
Cc: Gautham R. Shenoy, linuxppc-dev, linux-kernel
In-Reply-To: <1586249263-14048-1-git-send-email-ego@linux.vnet.ibm.com>
From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
Currently when CPU goes idle, we take a snapshot of PURR via
pseries_idle_prolog() which is used at the CPU idle exit to compute
the idle PURR cycles via the function pseries_idle_epilog(). Thus,
the value of idle PURR cycle thus read before pseries_idle_prolog() and
after pseries_idle_epilog() is always correct.
However, if we were to read the idle PURR cycles from an interrupt
context between pseries_idle_prolog() and pseries_idle_epilog() (this
will be done in a future patch), then, the value of the idle PURR thus
read will not include the cycles spent in the most recent idle period.
Thus, in that interrupt context, we will need access to the snapshot
of the PURR before going idle, in order to compute the idle PURR
cycles for the latest idle duration.
In this patch, we save the snapshot of PURR in pseries_idle_prolog()
in a per-cpu variable, instead of on the stack, so that it can be
accessed from an interrupt context.
Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/idle.h | 31 ++++++++++++++++++++++---------
arch/powerpc/platforms/pseries/setup.c | 7 +++----
drivers/cpuidle/cpuidle-pseries.c | 15 ++++++---------
3 files changed, 31 insertions(+), 22 deletions(-)
diff --git a/arch/powerpc/include/asm/idle.h b/arch/powerpc/include/asm/idle.h
index 32064a4c..b90d75a 100644
--- a/arch/powerpc/include/asm/idle.h
+++ b/arch/powerpc/include/asm/idle.h
@@ -5,10 +5,27 @@
#include <asm/paca.h>
#ifdef CONFIG_PPC_PSERIES
-static inline void pseries_idle_prolog(unsigned long *in_purr)
+DECLARE_PER_CPU(u64, idle_entry_purr_snap);
+
+static inline void snapshot_purr_idle_entry(void)
+{
+ *this_cpu_ptr(&idle_entry_purr_snap) = mfspr(SPRN_PURR);
+}
+
+static inline void update_idle_purr_accounting(void)
+{
+ u64 wait_cycles;
+ u64 in_purr = *this_cpu_ptr(&idle_entry_purr_snap);
+
+ wait_cycles = be64_to_cpu(get_lppaca()->wait_state_cycles);
+ wait_cycles += mfspr(SPRN_PURR) - in_purr;
+ get_lppaca()->wait_state_cycles = cpu_to_be64(wait_cycles);
+}
+
+static inline void pseries_idle_prolog(void)
{
ppc64_runlatch_off();
- *in_purr = mfspr(SPRN_PURR);
+ snapshot_purr_idle_entry();
/*
* Indicate to the HV that we are idle. Now would be
* a good time to find other work to dispatch.
@@ -16,16 +33,12 @@ static inline void pseries_idle_prolog(unsigned long *in_purr)
get_lppaca()->idle = 1;
}
-static inline void pseries_idle_epilog(unsigned long in_purr)
+static inline void pseries_idle_epilog(void)
{
- u64 wait_cycles;
-
- wait_cycles = be64_to_cpu(get_lppaca()->wait_state_cycles);
- wait_cycles += mfspr(SPRN_PURR) - in_purr;
- get_lppaca()->wait_state_cycles = cpu_to_be64(wait_cycles);
+ update_idle_purr_accounting();
get_lppaca()->idle = 0;
-
ppc64_runlatch_on();
}
+
#endif /* CONFIG_PPC_PSERIES */
#endif
diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
index 2f53e6b..4905c96 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -318,10 +318,9 @@ static int alloc_dispatch_log_kmem_cache(void)
}
machine_early_initcall(pseries, alloc_dispatch_log_kmem_cache);
+DEFINE_PER_CPU(u64, idle_entry_purr_snap);
static void pseries_lpar_idle(void)
{
- unsigned long in_purr;
-
/*
* Default handler to go into low thread priority and possibly
* low power mode by ceding processor to hypervisor
@@ -331,7 +330,7 @@ static void pseries_lpar_idle(void)
return;
/* Indicate to hypervisor that we are idle. */
- pseries_idle_prolog(&in_purr);
+ pseries_idle_prolog();
/*
* Yield the processor to the hypervisor. We return if
@@ -342,7 +341,7 @@ static void pseries_lpar_idle(void)
*/
cede_processor();
- pseries_idle_epilog(in_purr);
+ pseries_idle_epilog();
}
/*
diff --git a/drivers/cpuidle/cpuidle-pseries.c b/drivers/cpuidle/cpuidle-pseries.c
index 46d5e05..6513ef2 100644
--- a/drivers/cpuidle/cpuidle-pseries.c
+++ b/drivers/cpuidle/cpuidle-pseries.c
@@ -36,12 +36,11 @@ static int snooze_loop(struct cpuidle_device *dev,
struct cpuidle_driver *drv,
int index)
{
- unsigned long in_purr;
u64 snooze_exit_time;
set_thread_flag(TIF_POLLING_NRFLAG);
- pseries_idle_prolog(&in_purr);
+ pseries_idle_prolog();
local_irq_enable();
snooze_exit_time = get_tb() + snooze_timeout;
@@ -65,7 +64,7 @@ static int snooze_loop(struct cpuidle_device *dev,
local_irq_disable();
- pseries_idle_epilog(in_purr);
+ pseries_idle_epilog();
return index;
}
@@ -91,9 +90,8 @@ static int dedicated_cede_loop(struct cpuidle_device *dev,
struct cpuidle_driver *drv,
int index)
{
- unsigned long in_purr;
- pseries_idle_prolog(&in_purr);
+ pseries_idle_prolog();
get_lppaca()->donate_dedicated_cpu = 1;
HMT_medium();
@@ -102,7 +100,7 @@ static int dedicated_cede_loop(struct cpuidle_device *dev,
local_irq_disable();
get_lppaca()->donate_dedicated_cpu = 0;
- pseries_idle_epilog(in_purr);
+ pseries_idle_epilog();
return index;
}
@@ -111,9 +109,8 @@ static int shared_cede_loop(struct cpuidle_device *dev,
struct cpuidle_driver *drv,
int index)
{
- unsigned long in_purr;
- pseries_idle_prolog(&in_purr);
+ pseries_idle_prolog();
/*
* Yield the processor to the hypervisor. We return if
@@ -125,7 +122,7 @@ static int shared_cede_loop(struct cpuidle_device *dev,
check_and_cede_processor();
local_irq_disable();
- pseries_idle_epilog(in_purr);
+ pseries_idle_epilog();
return index;
}
--
1.9.4
^ permalink raw reply related
* [PATCH v5 3/5] powerpc/pseries: Account for SPURR ticks on idle CPUs
From: Gautham R. Shenoy @ 2020-04-07 8:47 UTC (permalink / raw)
To: Nathan Lynch, Michael Ellerman, Vaidyanathan Srinivasan,
Kamalesh Babulal, Naveen N. Rao, Tyrel Datwyler
Cc: Gautham R. Shenoy, linuxppc-dev, linux-kernel
In-Reply-To: <1586249263-14048-1-git-send-email-ego@linux.vnet.ibm.com>
From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
On Pseries LPARs, to calculate utilization, we need to know the
[S]PURR ticks when the CPUs were busy or idle.
Via pseries_idle_prolog(), pseries_idle_epilog(), we track the idle
PURR ticks in the VPA variable "wait_state_cycles". This patch extends
the support to account for the idle SPURR ticks.
Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/idle.h | 17 +++++++++++++++++
arch/powerpc/platforms/pseries/setup.c | 2 ++
2 files changed, 19 insertions(+)
diff --git a/arch/powerpc/include/asm/idle.h b/arch/powerpc/include/asm/idle.h
index b90d75a..0efb250 100644
--- a/arch/powerpc/include/asm/idle.h
+++ b/arch/powerpc/include/asm/idle.h
@@ -5,13 +5,20 @@
#include <asm/paca.h>
#ifdef CONFIG_PPC_PSERIES
+DECLARE_PER_CPU(u64, idle_spurr_cycles);
DECLARE_PER_CPU(u64, idle_entry_purr_snap);
+DECLARE_PER_CPU(u64, idle_entry_spurr_snap);
static inline void snapshot_purr_idle_entry(void)
{
*this_cpu_ptr(&idle_entry_purr_snap) = mfspr(SPRN_PURR);
}
+static inline void snapshot_spurr_idle_entry(void)
+{
+ *this_cpu_ptr(&idle_entry_spurr_snap) = mfspr(SPRN_SPURR);
+}
+
static inline void update_idle_purr_accounting(void)
{
u64 wait_cycles;
@@ -22,10 +29,19 @@ static inline void update_idle_purr_accounting(void)
get_lppaca()->wait_state_cycles = cpu_to_be64(wait_cycles);
}
+static inline void update_idle_spurr_accounting(void)
+{
+ u64 *idle_spurr_cycles_ptr = this_cpu_ptr(&idle_spurr_cycles);
+ u64 in_spurr = *this_cpu_ptr(&idle_entry_spurr_snap);
+
+ *idle_spurr_cycles_ptr += mfspr(SPRN_SPURR) - in_spurr;
+}
+
static inline void pseries_idle_prolog(void)
{
ppc64_runlatch_off();
snapshot_purr_idle_entry();
+ snapshot_spurr_idle_entry();
/*
* Indicate to the HV that we are idle. Now would be
* a good time to find other work to dispatch.
@@ -36,6 +52,7 @@ static inline void pseries_idle_prolog(void)
static inline void pseries_idle_epilog(void)
{
update_idle_purr_accounting();
+ update_idle_spurr_accounting();
get_lppaca()->idle = 0;
ppc64_runlatch_on();
}
diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
index 4905c96..1b55e80 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -318,7 +318,9 @@ static int alloc_dispatch_log_kmem_cache(void)
}
machine_early_initcall(pseries, alloc_dispatch_log_kmem_cache);
+DEFINE_PER_CPU(u64, idle_spurr_cycles);
DEFINE_PER_CPU(u64, idle_entry_purr_snap);
+DEFINE_PER_CPU(u64, idle_entry_spurr_snap);
static void pseries_lpar_idle(void)
{
/*
--
1.9.4
^ permalink raw reply related
* [PATCH v5 1/5] powerpc: Move idle_loop_prolog()/epilog() functions to header file
From: Gautham R. Shenoy @ 2020-04-07 8:47 UTC (permalink / raw)
To: Nathan Lynch, Michael Ellerman, Vaidyanathan Srinivasan,
Kamalesh Babulal, Naveen N. Rao, Tyrel Datwyler
Cc: Gautham R. Shenoy, linuxppc-dev, linux-kernel
In-Reply-To: <1586249263-14048-1-git-send-email-ego@linux.vnet.ibm.com>
From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
Currently prior to entering an idle state on a Linux Guest, the
pseries cpuidle driver implement an idle_loop_prolog() and
idle_loop_epilog() functions which ensure that idle_purr is correctly
computed, and the hypervisor is informed that the CPU cycles have been
donated.
These prolog and epilog functions are also required in the default
idle call, i.e pseries_lpar_idle(). Hence move these accessor
functions to a common header file and call them from
pseries_lpar_idle(). Since the existing header files such as
asm/processor.h have enough clutter, create a new header file
asm/idle.h. Finally rename idle_loop_prolog() and idle_loop_epilog()
to pseries_idle_prolog() and pseries_idle_epilog() as they are only
relavent for on pseries guests.
Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/idle.h | 31 +++++++++++++++++++++++++++++
arch/powerpc/platforms/pseries/setup.c | 7 +++++--
drivers/cpuidle/cpuidle-pseries.c | 36 +++++++---------------------------
3 files changed, 43 insertions(+), 31 deletions(-)
create mode 100644 arch/powerpc/include/asm/idle.h
diff --git a/arch/powerpc/include/asm/idle.h b/arch/powerpc/include/asm/idle.h
new file mode 100644
index 0000000..32064a4c
--- /dev/null
+++ b/arch/powerpc/include/asm/idle.h
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+#ifndef _ASM_POWERPC_IDLE_H
+#define _ASM_POWERPC_IDLE_H
+#include <asm/runlatch.h>
+#include <asm/paca.h>
+
+#ifdef CONFIG_PPC_PSERIES
+static inline void pseries_idle_prolog(unsigned long *in_purr)
+{
+ ppc64_runlatch_off();
+ *in_purr = mfspr(SPRN_PURR);
+ /*
+ * Indicate to the HV that we are idle. Now would be
+ * a good time to find other work to dispatch.
+ */
+ get_lppaca()->idle = 1;
+}
+
+static inline void pseries_idle_epilog(unsigned long in_purr)
+{
+ u64 wait_cycles;
+
+ wait_cycles = be64_to_cpu(get_lppaca()->wait_state_cycles);
+ wait_cycles += mfspr(SPRN_PURR) - in_purr;
+ get_lppaca()->wait_state_cycles = cpu_to_be64(wait_cycles);
+ get_lppaca()->idle = 0;
+
+ ppc64_runlatch_on();
+}
+#endif /* CONFIG_PPC_PSERIES */
+#endif
diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
index 0c8421d..2f53e6b 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -68,6 +68,7 @@
#include <asm/isa-bridge.h>
#include <asm/security_features.h>
#include <asm/asm-const.h>
+#include <asm/idle.h>
#include <asm/swiotlb.h>
#include <asm/svm.h>
@@ -319,6 +320,8 @@ static int alloc_dispatch_log_kmem_cache(void)
static void pseries_lpar_idle(void)
{
+ unsigned long in_purr;
+
/*
* Default handler to go into low thread priority and possibly
* low power mode by ceding processor to hypervisor
@@ -328,7 +331,7 @@ static void pseries_lpar_idle(void)
return;
/* Indicate to hypervisor that we are idle. */
- get_lppaca()->idle = 1;
+ pseries_idle_prolog(&in_purr);
/*
* Yield the processor to the hypervisor. We return if
@@ -339,7 +342,7 @@ static void pseries_lpar_idle(void)
*/
cede_processor();
- get_lppaca()->idle = 0;
+ pseries_idle_epilog(in_purr);
}
/*
diff --git a/drivers/cpuidle/cpuidle-pseries.c b/drivers/cpuidle/cpuidle-pseries.c
index 74c2479..46d5e05 100644
--- a/drivers/cpuidle/cpuidle-pseries.c
+++ b/drivers/cpuidle/cpuidle-pseries.c
@@ -19,6 +19,7 @@
#include <asm/machdep.h>
#include <asm/firmware.h>
#include <asm/runlatch.h>
+#include <asm/idle.h>
#include <asm/plpar_wrappers.h>
struct cpuidle_driver pseries_idle_driver = {
@@ -31,29 +32,6 @@ struct cpuidle_driver pseries_idle_driver = {
static u64 snooze_timeout __read_mostly;
static bool snooze_timeout_en __read_mostly;
-static inline void idle_loop_prolog(unsigned long *in_purr)
-{
- ppc64_runlatch_off();
- *in_purr = mfspr(SPRN_PURR);
- /*
- * Indicate to the HV that we are idle. Now would be
- * a good time to find other work to dispatch.
- */
- get_lppaca()->idle = 1;
-}
-
-static inline void idle_loop_epilog(unsigned long in_purr)
-{
- u64 wait_cycles;
-
- wait_cycles = be64_to_cpu(get_lppaca()->wait_state_cycles);
- wait_cycles += mfspr(SPRN_PURR) - in_purr;
- get_lppaca()->wait_state_cycles = cpu_to_be64(wait_cycles);
- get_lppaca()->idle = 0;
-
- ppc64_runlatch_on();
-}
-
static int snooze_loop(struct cpuidle_device *dev,
struct cpuidle_driver *drv,
int index)
@@ -63,7 +41,7 @@ static int snooze_loop(struct cpuidle_device *dev,
set_thread_flag(TIF_POLLING_NRFLAG);
- idle_loop_prolog(&in_purr);
+ pseries_idle_prolog(&in_purr);
local_irq_enable();
snooze_exit_time = get_tb() + snooze_timeout;
@@ -87,7 +65,7 @@ static int snooze_loop(struct cpuidle_device *dev,
local_irq_disable();
- idle_loop_epilog(in_purr);
+ pseries_idle_epilog(in_purr);
return index;
}
@@ -115,7 +93,7 @@ static int dedicated_cede_loop(struct cpuidle_device *dev,
{
unsigned long in_purr;
- idle_loop_prolog(&in_purr);
+ pseries_idle_prolog(&in_purr);
get_lppaca()->donate_dedicated_cpu = 1;
HMT_medium();
@@ -124,7 +102,7 @@ static int dedicated_cede_loop(struct cpuidle_device *dev,
local_irq_disable();
get_lppaca()->donate_dedicated_cpu = 0;
- idle_loop_epilog(in_purr);
+ pseries_idle_epilog(in_purr);
return index;
}
@@ -135,7 +113,7 @@ static int shared_cede_loop(struct cpuidle_device *dev,
{
unsigned long in_purr;
- idle_loop_prolog(&in_purr);
+ pseries_idle_prolog(&in_purr);
/*
* Yield the processor to the hypervisor. We return if
@@ -147,7 +125,7 @@ static int shared_cede_loop(struct cpuidle_device *dev,
check_and_cede_processor();
local_irq_disable();
- idle_loop_epilog(in_purr);
+ pseries_idle_epilog(in_purr);
return index;
}
--
1.9.4
^ permalink raw reply related
* [PATCH v5 0/5] Track and expose idle PURR and SPURR ticks
From: Gautham R. Shenoy @ 2020-04-07 8:47 UTC (permalink / raw)
To: Nathan Lynch, Michael Ellerman, Vaidyanathan Srinivasan,
Kamalesh Babulal, Naveen N. Rao, Tyrel Datwyler
Cc: Gautham R. Shenoy, linuxppc-dev, linux-kernel
From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
Hi,
This is the fifth version of the patches to track and expose idle PURR
and SPURR ticks. These patches are required by tools such as lparstat
to compute system utilization for capacity planning purposes.
The previous versions can be found here:
v4: https://lkml.org/lkml/2020/3/27/323
v3: https://lkml.org/lkml/2020/3/11/331
v2: https://lkml.org/lkml/2020/2/21/21
v1: https://lore.kernel.org/patchwork/cover/1159341/
They changes from v4 are:
- As suggested by Naveen, moved the functions read_this_idle_purr()
and read_this_idle_spurr() from Patch 2 and Patch 3 respectively
to Patch 4 where it is invoked.
- Dropped Patch 6 which cached the values of purr, spurr,
idle_purr, idle_spurr in order to minimize the number of IPIs
sent.
- Updated the dates for the idle_purr, idle_spurr in the
Documentation Patch 5.
Motivation:
===========
On PSeries LPARs, the data centers planners desire a more accurate
view of system utilization per resource such as CPU to plan the system
capacity requirements better. Such accuracy can be obtained by reading
PURR/SPURR registers for CPU resource utilization.
Tools such as lparstat which are used to compute the utilization need
to know [S]PURR ticks when the cpu was busy or idle. The [S]PURR
counters are already exposed through sysfs. We already account for
PURR ticks when we go to idle so that we can update the VPA area. This
patchset extends support to account for SPURR ticks when idle, and
expose both via per-cpu sysfs files.
These patches are required for enhancement to the lparstat utility
that compute the CPU utilization based on PURR and SPURR which can be
found here :
https://groups.google.com/forum/#!topic/powerpc-utils-devel/fYRo69xO9r4
With the patches, when lparstat is run on a LPAR running CPU-Hogs,
=========================================================================
sudo ./src/lparstat -E 1 3
System Configuration
type=Dedicated mode=Capped smt=8 lcpu=2 mem=4834112 kB cpus=0 ent=2.00
---Actual--- -Normalized-
%busy %idle Frequency %busy %idle
------ ------ ------------- ------ ------
1 99.99 0.00 3.35GHz[111%] 110.99 0.00
2 100.00 0.00 3.35GHz[111%] 111.01 0.00
3 100.00 0.00 3.35GHz[111%] 111.00 0.00
With patches, when lparstat is run on and idle LPAR
=========================================================================
System Configuration
type=Dedicated mode=Capped smt=8 lcpu=2 mem=4834112 kB cpus=0 ent=2.00
---Actual--- -Normalized-
%busy %idle Frequency %busy %idle
------ ------ ------------- ------ ------
1 0.15 99.84 2.17GHz[ 72%] 0.11 71.89
2 0.24 99.76 2.11GHz[ 70%] 0.18 69.82
3 0.24 99.75 2.11GHz[ 70%] 0.18 69.81
Gautham R. Shenoy (5):
powerpc: Move idle_loop_prolog()/epilog() functions to header file
powerpc/idle: Store PURR snapshot in a per-cpu global variable
powerpc/pseries: Account for SPURR ticks on idle CPUs
powerpc/sysfs: Show idle_purr and idle_spurr for every CPU
Documentation: Document sysfs interfaces purr, spurr, idle_purr,
idle_spurr
Documentation/ABI/testing/sysfs-devices-system-cpu | 39 +++++++++
arch/powerpc/include/asm/idle.h | 93 ++++++++++++++++++++++
arch/powerpc/kernel/sysfs.c | 82 ++++++++++++++++++-
arch/powerpc/platforms/pseries/setup.c | 8 +-
drivers/cpuidle/cpuidle-pseries.c | 39 ++-------
5 files changed, 224 insertions(+), 37 deletions(-)
create mode 100644 arch/powerpc/include/asm/idle.h
--
1.9.4
^ permalink raw reply
* Re: [PATCH v5 05/21] powerpc: Use a function for getting the instruction op code
From: Jordan Niethe @ 2020-04-07 8:32 UTC (permalink / raw)
To: Balamuruhan S
Cc: Alistair Popple, linuxppc-dev, Nicholas Piggin, Daniel Axtens
In-Reply-To: <1f3f9f12af1f40339045737910e8e63265edb28d.camel@linux.ibm.com>
On Tue, Apr 7, 2020 at 5:05 PM Balamuruhan S <bala24@linux.ibm.com> wrote:
>
> On Mon, 2020-04-06 at 18:09 +1000, Jordan Niethe wrote:
> > In preparation for using a data type for instructions that can not be
> > directly used with the '>>' operator use a function for getting the op
> > code of an instruction.
>
> vecemu.c and sstep.c will need ppc_inst_opcode().
Sorry, I forget you pointed those files out before. I added other
things to them but then I forgot to add this. Will do for next time.
>
> -- Bala
>
> >
> > Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> > ---
> > v4: New to series
> > ---
> > arch/powerpc/include/asm/inst.h | 5 +++++
> > arch/powerpc/kernel/align.c | 2 +-
> > arch/powerpc/lib/code-patching.c | 4 ++--
> > 3 files changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/inst.h
> > b/arch/powerpc/include/asm/inst.h
> > index 5298ba33b6e5..93959016fe4b 100644
> > --- a/arch/powerpc/include/asm/inst.h
> > +++ b/arch/powerpc/include/asm/inst.h
> > @@ -8,4 +8,9 @@
> >
> > #define ppc_inst(x) (x)
> >
> > +static inline int ppc_inst_opcode(u32 x)
> > +{
> > + return x >> 26;
> > +}
> > +
> > #endif /* _ASM_INST_H */
> > diff --git a/arch/powerpc/kernel/align.c b/arch/powerpc/kernel/align.c
> > index 86e9bf62f18c..691013aa9f3c 100644
> > --- a/arch/powerpc/kernel/align.c
> > +++ b/arch/powerpc/kernel/align.c
> > @@ -314,7 +314,7 @@ int fix_alignment(struct pt_regs *regs)
> > }
> >
> > #ifdef CONFIG_SPE
> > - if ((instr >> 26) == 0x4) {
> > + if (ppc_inst_opcode(instr) == 0x4) {
> > int reg = (instr >> 21) & 0x1f;
> > PPC_WARN_ALIGNMENT(spe, regs);
> > return emulate_spe(regs, reg, instr);
> > diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-
> > patching.c
> > index fdf0d6ea3575..099a515202aa 100644
> > --- a/arch/powerpc/lib/code-patching.c
> > +++ b/arch/powerpc/lib/code-patching.c
> > @@ -231,7 +231,7 @@ bool is_offset_in_branch_range(long offset)
> > */
> > bool is_conditional_branch(unsigned int instr)
> > {
> > - unsigned int opcode = instr >> 26;
> > + unsigned int opcode = ppc_inst_opcode(instr);
> >
> > if (opcode == 16) /* bc, bca, bcl, bcla */
> > return true;
> > @@ -289,7 +289,7 @@ int create_cond_branch(unsigned int *instr, const
> > unsigned int *addr,
> >
> > static unsigned int branch_opcode(unsigned int instr)
> > {
> > - return (instr >> 26) & 0x3F;
> > + return ppc_inst_opcode(instr) & 0x3F;
> > }
> >
> > static int instr_is_branch_iform(unsigned int instr)
>
^ permalink raw reply
* Re: [PATCH v5 04/21] powerpc: Use a macro for creating instructions from u32s
From: Jordan Niethe @ 2020-04-07 8:27 UTC (permalink / raw)
To: Balamuruhan S
Cc: Alistair Popple, linuxppc-dev, Nicholas Piggin, Daniel Axtens
In-Reply-To: <8e725ed4e48011261a171c0160966d0c2c670f9a.camel@linux.ibm.com>
On Tue, Apr 7, 2020 at 4:40 PM Balamuruhan S <bala24@linux.ibm.com> wrote:
>
> On Mon, 2020-04-06 at 18:09 +1000, Jordan Niethe wrote:
> > In preparation for instructions having a more complex data type start
> > using a macro, ppc_inst(), for making an instruction out of a u32. A
> > macro is used so that instructions can be used as initializer elements.
> > Currently this does nothing, but it will allow for creating a data type
> > that can represent prefixed instructions.
> >
> > Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> > ---
> > v4: New to series
> > v5: - Rename PPC_INST() -> ppc_inst().
> > - Use on epapr_paravirt.c, kgdb.c
> > ---
> > arch/powerpc/include/asm/code-patching.h | 3 +-
> > arch/powerpc/include/asm/inst.h | 11 +++++
> > arch/powerpc/kernel/align.c | 1 +
> > arch/powerpc/kernel/epapr_paravirt.c | 5 ++-
> > arch/powerpc/kernel/hw_breakpoint.c | 3 +-
> > arch/powerpc/kernel/jump_label.c | 3 +-
> > arch/powerpc/kernel/kgdb.c | 5 ++-
> > arch/powerpc/kernel/kprobes.c | 5 ++-
> > arch/powerpc/kernel/module_64.c | 3 +-
> > arch/powerpc/kernel/optprobes.c | 31 ++++++-------
> > arch/powerpc/kernel/security.c | 9 ++--
> > arch/powerpc/kernel/trace/ftrace.c | 25 ++++++-----
> > arch/powerpc/kernel/uprobes.c | 1 +
> > arch/powerpc/kvm/emulate_loadstore.c | 2 +-
> > arch/powerpc/lib/code-patching.c | 57 ++++++++++++------------
> > arch/powerpc/lib/feature-fixups.c | 39 ++++++++--------
> > arch/powerpc/lib/test_emulate_step.c | 39 ++++++++--------
> > arch/powerpc/xmon/xmon.c | 7 +--
> > 18 files changed, 138 insertions(+), 111 deletions(-)
> > create mode 100644 arch/powerpc/include/asm/inst.h
> >
> > diff --git a/arch/powerpc/include/asm/code-patching.h
> > b/arch/powerpc/include/asm/code-patching.h
> > index 351dda7215b6..48e021957ee5 100644
> > --- a/arch/powerpc/include/asm/code-patching.h
> > +++ b/arch/powerpc/include/asm/code-patching.h
> > @@ -11,6 +11,7 @@
> > #include <linux/string.h>
> > #include <linux/kallsyms.h>
> > #include <asm/asm-compat.h>
> > +#include <asm/inst.h>
> >
> > /* Flags for create_branch:
> > * "b" == create_branch(addr, target, 0);
> > @@ -48,7 +49,7 @@ static inline int patch_branch_site(s32 *site, unsigned
> > long target, int flags)
> > static inline int modify_instruction(unsigned int *addr, unsigned int clr,
> > unsigned int set)
> > {
> > - return patch_instruction(addr, (*addr & ~clr) | set);
> > + return patch_instruction(addr, ppc_inst((*addr & ~clr) | set));
> > }
> >
> > static inline int modify_instruction_site(s32 *site, unsigned int clr,
> > unsigned int set)
> > diff --git a/arch/powerpc/include/asm/inst.h
> > b/arch/powerpc/include/asm/inst.h
> > new file mode 100644
> > index 000000000000..5298ba33b6e5
> > --- /dev/null
> > +++ b/arch/powerpc/include/asm/inst.h
> > @@ -0,0 +1,11 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +#ifndef _ASM_INST_H
> > +#define _ASM_INST_H
> > +
> > +/*
> > + * Instruction data type for POWER
> > + */
> > +
> > +#define ppc_inst(x) (x)
> > +
> > +#endif /* _ASM_INST_H */
> > diff --git a/arch/powerpc/kernel/align.c b/arch/powerpc/kernel/align.c
> > index 92045ed64976..86e9bf62f18c 100644
> > --- a/arch/powerpc/kernel/align.c
> > +++ b/arch/powerpc/kernel/align.c
> > @@ -24,6 +24,7 @@
> > #include <asm/disassemble.h>
> > #include <asm/cpu_has_feature.h>
> > #include <asm/sstep.h>
> > +#include <asm/inst.h>
> >
> > struct aligninfo {
> > unsigned char len;
> > diff --git a/arch/powerpc/kernel/epapr_paravirt.c
> > b/arch/powerpc/kernel/epapr_paravirt.c
> > index 9d32158ce36f..c53e863fb484 100644
> > --- a/arch/powerpc/kernel/epapr_paravirt.c
> > +++ b/arch/powerpc/kernel/epapr_paravirt.c
> > @@ -11,6 +11,7 @@
> > #include <asm/cacheflush.h>
> > #include <asm/code-patching.h>
> > #include <asm/machdep.h>
> > +#include <asm/inst.h>
> >
> > #if !defined(CONFIG_64BIT) || defined(CONFIG_PPC_BOOK3E_64)
> > extern void epapr_ev_idle(void);
> > @@ -37,9 +38,9 @@ static int __init early_init_dt_scan_epapr(unsigned long
> > node,
> >
> > for (i = 0; i < (len / 4); i++) {
> > u32 inst = be32_to_cpu(insts[i]);
> > - patch_instruction(epapr_hypercall_start + i, inst);
> > + patch_instruction(epapr_hypercall_start + i, ppc_inst(inst));
> > #if !defined(CONFIG_64BIT) || defined(CONFIG_PPC_BOOK3E_64)
> > - patch_instruction(epapr_ev_idle_start + i, inst);
> > + patch_instruction(epapr_ev_idle_start + i, ppc_inst(inst));
>
>
> can we do it once while assigning inst ?
>
> u32 inst = ppc_inst(be32_to_cpu(insts[i]));
Yeah that would be better.
>
> -- Bala
>
> > #endif
> > }
> >
> > diff --git a/arch/powerpc/kernel/hw_breakpoint.c
> > b/arch/powerpc/kernel/hw_breakpoint.c
> > index 2462cd7c565c..79f51f182a83 100644
> > --- a/arch/powerpc/kernel/hw_breakpoint.c
> > +++ b/arch/powerpc/kernel/hw_breakpoint.c
> > @@ -24,6 +24,7 @@
> > #include <asm/debug.h>
> > #include <asm/debugfs.h>
> > #include <asm/hvcall.h>
> > +#include <asm/inst.h>
> > #include <linux/uaccess.h>
> >
> > /*
> > @@ -243,7 +244,7 @@ dar_range_overlaps(unsigned long dar, int size, struct
> > arch_hw_breakpoint *info)
> > static bool stepping_handler(struct pt_regs *regs, struct perf_event *bp,
> > struct arch_hw_breakpoint *info)
> > {
> > - unsigned int instr = 0;
> > + unsigned int instr = ppc_inst(0);
> > int ret, type, size;
> > struct instruction_op op;
> > unsigned long addr = info->address;
> > diff --git a/arch/powerpc/kernel/jump_label.c
> > b/arch/powerpc/kernel/jump_label.c
> > index ca37702bde97..daa4afce7ec8 100644
> > --- a/arch/powerpc/kernel/jump_label.c
> > +++ b/arch/powerpc/kernel/jump_label.c
> > @@ -6,6 +6,7 @@
> > #include <linux/kernel.h>
> > #include <linux/jump_label.h>
> > #include <asm/code-patching.h>
> > +#include <asm/inst.h>
> >
> > void arch_jump_label_transform(struct jump_entry *entry,
> > enum jump_label_type type)
> > @@ -15,5 +16,5 @@ void arch_jump_label_transform(struct jump_entry *entry,
> > if (type == JUMP_LABEL_JMP)
> > patch_branch(addr, entry->target, 0);
> > else
> > - patch_instruction(addr, PPC_INST_NOP);
> > + patch_instruction(addr, ppc_inst(PPC_INST_NOP));
> > }
> > diff --git a/arch/powerpc/kernel/kgdb.c b/arch/powerpc/kernel/kgdb.c
> > index 7dd55eb1259d..a6b38a19133f 100644
> > --- a/arch/powerpc/kernel/kgdb.c
> > +++ b/arch/powerpc/kernel/kgdb.c
> > @@ -26,6 +26,7 @@
> > #include <asm/debug.h>
> > #include <asm/code-patching.h>
> > #include <linux/slab.h>
> > +#include <asm/inst.h>
> >
> > /*
> > * This table contains the mapping between PowerPC hardware trap types, and
> > @@ -424,7 +425,7 @@ int kgdb_arch_set_breakpoint(struct kgdb_bkpt *bpt)
> > if (err)
> > return err;
> >
> > - err = patch_instruction(addr, BREAK_INSTR);
> > + err = patch_instruction(addr, ppc_inst(BREAK_INSTR));
> > if (err)
> > return -EFAULT;
> >
> > @@ -439,7 +440,7 @@ int kgdb_arch_remove_breakpoint(struct kgdb_bkpt *bpt)
> > unsigned int instr = *(unsigned int *)bpt->saved_instr;
> > unsigned int *addr = (unsigned int *)bpt->bpt_addr;
> >
> > - err = patch_instruction(addr, instr);
> > + err = patch_instruction(addr, ppc_inst(instr));
> > if (err)
> > return -EFAULT;
> >
> > diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
> > index 2d27ec4feee4..a1a3686f41c6 100644
> > --- a/arch/powerpc/kernel/kprobes.c
> > +++ b/arch/powerpc/kernel/kprobes.c
> > @@ -23,6 +23,7 @@
> > #include <asm/cacheflush.h>
> > #include <asm/sstep.h>
> > #include <asm/sections.h>
> > +#include <asm/inst.h>
> > #include <linux/uaccess.h>
> >
> > DEFINE_PER_CPU(struct kprobe *, current_kprobe) = NULL;
> > @@ -138,13 +139,13 @@ NOKPROBE_SYMBOL(arch_prepare_kprobe);
> >
> > void arch_arm_kprobe(struct kprobe *p)
> > {
> > - patch_instruction(p->addr, BREAKPOINT_INSTRUCTION);
> > + patch_instruction(p->addr, ppc_inst(BREAKPOINT_INSTRUCTION));
> > }
> > NOKPROBE_SYMBOL(arch_arm_kprobe);
> >
> > void arch_disarm_kprobe(struct kprobe *p)
> > {
> > - patch_instruction(p->addr, p->opcode);
> > + patch_instruction(p->addr, ppc_inst(p->opcode));
> > }
> > NOKPROBE_SYMBOL(arch_disarm_kprobe);
> >
> > diff --git a/arch/powerpc/kernel/module_64.c
> > b/arch/powerpc/kernel/module_64.c
> > index 007606a48fd9..7fd6b29edcb2 100644
> > --- a/arch/powerpc/kernel/module_64.c
> > +++ b/arch/powerpc/kernel/module_64.c
> > @@ -20,6 +20,7 @@
> > #include <linux/sort.h>
> > #include <asm/setup.h>
> > #include <asm/sections.h>
> > +#include <asm/inst.h>
> >
> > /* FIXME: We don't do .init separately. To do this, we'd need to have
> > a separate r2 value in the init and core section, and stub between
> > @@ -506,7 +507,7 @@ static int restore_r2(const char *name, u32 *instruction,
> > struct module *me)
> > * "link" branches and they don't return, so they don't need the r2
> > * restore afterwards.
> > */
> > - if (!instr_is_relative_link_branch(*prev_insn))
> > + if (!instr_is_relative_link_branch(ppc_inst(*prev_insn)))
> > return 1;
> >
> > if (*instruction != PPC_INST_NOP) {
> > diff --git a/arch/powerpc/kernel/optprobes.c
> > b/arch/powerpc/kernel/optprobes.c
> > index 445b3dad82dc..3b33ebf18859 100644
> > --- a/arch/powerpc/kernel/optprobes.c
> > +++ b/arch/powerpc/kernel/optprobes.c
> > @@ -16,6 +16,7 @@
> > #include <asm/code-patching.h>
> > #include <asm/sstep.h>
> > #include <asm/ppc-opcode.h>
> > +#include <asm/inst.h>
> >
> > #define TMPL_CALL_HDLR_IDX \
> > (optprobe_template_call_handler - optprobe_template_entry)
> > @@ -147,13 +148,13 @@ void arch_remove_optimized_kprobe(struct
> > optimized_kprobe *op)
> > void patch_imm32_load_insns(unsigned int val, kprobe_opcode_t *addr)
> > {
> > /* addis r4,0,(insn)@h */
> > - patch_instruction(addr, PPC_INST_ADDIS | ___PPC_RT(4) |
> > - ((val >> 16) & 0xffff));
> > + patch_instruction(addr, ppc_inst(PPC_INST_ADDIS | ___PPC_RT(4) |
> > + ((val >> 16) & 0xffff)));
> > addr++;
> >
> > /* ori r4,r4,(insn)@l */
> > - patch_instruction(addr, PPC_INST_ORI | ___PPC_RA(4) |
> > - ___PPC_RS(4) | (val & 0xffff));
> > + patch_instruction(addr, ppc_inst(PPC_INST_ORI | ___PPC_RA(4) |
> > + ___PPC_RS(4) | (val & 0xffff)));
> > }
> >
> > /*
> > @@ -163,28 +164,28 @@ void patch_imm32_load_insns(unsigned int val,
> > kprobe_opcode_t *addr)
> > void patch_imm64_load_insns(unsigned long val, kprobe_opcode_t *addr)
> > {
> > /* lis r3,(op)@highest */
> > - patch_instruction(addr, PPC_INST_ADDIS | ___PPC_RT(3) |
> > - ((val >> 48) & 0xffff));
> > + patch_instruction(addr, ppc_inst(PPC_INST_ADDIS | ___PPC_RT(3) |
> > + ((val >> 48) & 0xffff)));
> > addr++;
> >
> > /* ori r3,r3,(op)@higher */
> > - patch_instruction(addr, PPC_INST_ORI | ___PPC_RA(3) |
> > - ___PPC_RS(3) | ((val >> 32) & 0xffff));
> > + patch_instruction(addr, ppc_inst(PPC_INST_ORI | ___PPC_RA(3) |
> > + ___PPC_RS(3) | ((val >> 32) & 0xffff)));
> > addr++;
> >
> > /* rldicr r3,r3,32,31 */
> > - patch_instruction(addr, PPC_INST_RLDICR | ___PPC_RA(3) |
> > - ___PPC_RS(3) | __PPC_SH64(32) | __PPC_ME64(31));
> > + patch_instruction(addr, ppc_inst(PPC_INST_RLDICR | ___PPC_RA(3) |
> > + ___PPC_RS(3) | __PPC_SH64(32) | __PPC_ME64(31)));
> > addr++;
> >
> > /* oris r3,r3,(op)@h */
> > - patch_instruction(addr, PPC_INST_ORIS | ___PPC_RA(3) |
> > - ___PPC_RS(3) | ((val >> 16) & 0xffff));
> > + patch_instruction(addr, ppc_inst(PPC_INST_ORIS | ___PPC_RA(3) |
> > + ___PPC_RS(3) | ((val >> 16) & 0xffff)));
> > addr++;
> >
> > /* ori r3,r3,(op)@l */
> > - patch_instruction(addr, PPC_INST_ORI | ___PPC_RA(3) |
> > - ___PPC_RS(3) | (val & 0xffff));
> > + patch_instruction(addr, ppc_inst(PPC_INST_ORI | ___PPC_RA(3) |
> > + ___PPC_RS(3) | (val & 0xffff)));
> > }
> >
> > int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe
> > *p)
> > @@ -230,7 +231,7 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe
> > *op, struct kprobe *p)
> > size = (TMPL_END_IDX * sizeof(kprobe_opcode_t)) / sizeof(int);
> > pr_devel("Copying template to %p, size %lu\n", buff, size);
> > for (i = 0; i < size; i++) {
> > - rc = patch_instruction(buff + i, *(optprobe_template_entry +
> > i));
> > + rc = patch_instruction(buff + i,
> > ppc_inst(*(optprobe_template_entry + i)));
> > if (rc < 0)
> > goto error;
> > }
> > diff --git a/arch/powerpc/kernel/security.c b/arch/powerpc/kernel/security.c
> > index bd70f5be1c27..81a288b1a603 100644
> > --- a/arch/powerpc/kernel/security.c
> > +++ b/arch/powerpc/kernel/security.c
> > @@ -14,6 +14,7 @@
> > #include <asm/debugfs.h>
> > #include <asm/security_features.h>
> > #include <asm/setup.h>
> > +#include <asm/inst.h>
> >
> >
> > u64 powerpc_security_features __read_mostly = SEC_FTR_DEFAULT;
> > @@ -403,9 +404,9 @@ static void toggle_count_cache_flush(bool enable)
> > enable = false;
> >
> > if (!enable) {
> > - patch_instruction_site(&patch__call_flush_count_cache,
> > PPC_INST_NOP);
> > + patch_instruction_site(&patch__call_flush_count_cache,
> > ppc_inst(PPC_INST_NOP));
> > #ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
> > - patch_instruction_site(&patch__call_kvm_flush_link_stack,
> > PPC_INST_NOP);
> > + patch_instruction_site(&patch__call_kvm_flush_link_stack,
> > ppc_inst(PPC_INST_NOP));
> > #endif
> > pr_info("link-stack-flush: software flush disabled.\n");
> > link_stack_flush_enabled = false;
> > @@ -428,7 +429,7 @@ static void toggle_count_cache_flush(bool enable)
> >
> > // If we just need to flush the link stack, patch an early return
> > if (!security_ftr_enabled(SEC_FTR_FLUSH_COUNT_CACHE)) {
> > - patch_instruction_site(&patch__flush_link_stack_return,
> > PPC_INST_BLR);
> > + patch_instruction_site(&patch__flush_link_stack_return,
> > ppc_inst(PPC_INST_BLR));
> > no_count_cache_flush();
> > return;
> > }
> > @@ -439,7 +440,7 @@ static void toggle_count_cache_flush(bool enable)
> > return;
> > }
> >
> > - patch_instruction_site(&patch__flush_count_cache_return, PPC_INST_BLR);
> > + patch_instruction_site(&patch__flush_count_cache_return,
> > ppc_inst(PPC_INST_BLR));
> > count_cache_flush_type = COUNT_CACHE_FLUSH_HW;
> > pr_info("count-cache-flush: hardware assisted flush sequence
> > enabled\n");
> > }
> > diff --git a/arch/powerpc/kernel/trace/ftrace.c
> > b/arch/powerpc/kernel/trace/ftrace.c
> > index 4dcb379133f8..47233e671c18 100644
> > --- a/arch/powerpc/kernel/trace/ftrace.c
> > +++ b/arch/powerpc/kernel/trace/ftrace.c
> > @@ -27,6 +27,7 @@
> > #include <asm/code-patching.h>
> > #include <asm/ftrace.h>
> > #include <asm/syscall.h>
> > +#include <asm/inst.h>
> >
> >
> > #ifdef CONFIG_DYNAMIC_FTRACE
> > @@ -161,7 +162,7 @@ __ftrace_make_nop(struct module *mod,
> >
> > #ifdef CONFIG_MPROFILE_KERNEL
> > /* When using -mkernel_profile there is no load to jump over */
> > - pop = PPC_INST_NOP;
> > + pop = ppc_inst(PPC_INST_NOP);
> >
> > if (probe_kernel_read(&op, (void *)(ip - 4), 4)) {
> > pr_err("Fetching instruction at %lx failed.\n", ip - 4);
> > @@ -169,7 +170,7 @@ __ftrace_make_nop(struct module *mod,
> > }
> >
> > /* We expect either a mflr r0, or a std r0, LRSAVE(r1) */
> > - if (op != PPC_INST_MFLR && op != PPC_INST_STD_LR) {
> > + if (op != ppc_inst(PPC_INST_MFLR) && op != ppc_inst(PPC_INST_STD_LR)) {
> > pr_err("Unexpected instruction %08x around bl _mcount\n", op);
> > return -EINVAL;
> > }
> > @@ -188,7 +189,7 @@ __ftrace_make_nop(struct module *mod,
> > * Use a b +8 to jump over the load.
> > */
> >
> > - pop = PPC_INST_BRANCH | 8; /* b +8 */
> > + pop = ppc_inst(PPC_INST_BRANCH | 8); /* b +8 */
> >
> > /*
> > * Check what is in the next instruction. We can see ld r2,40(r1), but
> > @@ -199,7 +200,7 @@ __ftrace_make_nop(struct module *mod,
> > return -EFAULT;
> > }
> >
> > - if (op != PPC_INST_LD_TOC) {
> > + if (op != ppc_inst(PPC_INST_LD_TOC)) {
> > pr_err("Expected %08x found %08x\n", PPC_INST_LD_TOC, op);
> > return -EINVAL;
> > }
> > @@ -275,7 +276,7 @@ __ftrace_make_nop(struct module *mod,
> > return -EINVAL;
> > }
> >
> > - op = PPC_INST_NOP;
> > + op = ppc_inst(PPC_INST_NOP);
> >
> > if (patch_instruction((unsigned int *)ip, op))
> > return -EPERM;
> > @@ -420,7 +421,7 @@ static int __ftrace_make_nop_kernel(struct dyn_ftrace
> > *rec, unsigned long addr)
> > }
> > }
> >
> > - if (patch_instruction((unsigned int *)ip, PPC_INST_NOP)) {
> > + if (patch_instruction((unsigned int *)ip, ppc_inst(PPC_INST_NOP))) {
> > pr_err("Patching NOP failed.\n");
> > return -EPERM;
> > }
> > @@ -442,7 +443,7 @@ int ftrace_make_nop(struct module *mod,
> > if (test_24bit_addr(ip, addr)) {
> > /* within range */
> > old = ftrace_call_replace(ip, addr, 1);
> > - new = PPC_INST_NOP;
> > + new = ppc_inst(PPC_INST_NOP);
> > return ftrace_modify_code(ip, old, new);
> > } else if (core_kernel_text(ip))
> > return __ftrace_make_nop_kernel(rec, addr);
> > @@ -496,7 +497,7 @@ expected_nop_sequence(void *ip, unsigned int op0,
> > unsigned int op1)
> > * The load offset is different depending on the ABI. For simplicity
> > * just mask it out when doing the compare.
> > */
> > - if ((op0 != 0x48000008) || ((op1 & 0xffff0000) != 0xe8410000))
> > + if ((op0 != ppc_inst(0x48000008)) || ((op1 & 0xffff0000) !=
> > 0xe8410000))
> > return 0;
> > return 1;
> > }
> > @@ -505,7 +506,7 @@ static int
> > expected_nop_sequence(void *ip, unsigned int op0, unsigned int op1)
> > {
> > /* look for patched "NOP" on ppc64 with -mprofile-kernel */
> > - if (op0 != PPC_INST_NOP)
> > + if (op0 != ppc_inst(PPC_INST_NOP))
> > return 0;
> > return 1;
> > }
> > @@ -588,7 +589,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long
> > addr)
> > return -EFAULT;
> >
> > /* It should be pointing to a nop */
> > - if (op != PPC_INST_NOP) {
> > + if (op != ppc_inst(PPC_INST_NOP)) {
> > pr_err("Expected NOP but have %x\n", op);
> > return -EINVAL;
> > }
> > @@ -645,7 +646,7 @@ static int __ftrace_make_call_kernel(struct dyn_ftrace
> > *rec, unsigned long addr)
> > return -EFAULT;
> > }
> >
> > - if (op != PPC_INST_NOP) {
> > + if (op != ppc_inst(PPC_INST_NOP)) {
> > pr_err("Unexpected call sequence at %p: %x\n", ip, op);
> > return -EINVAL;
> > }
> > @@ -676,7 +677,7 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned
> > long addr)
> > */
> > if (test_24bit_addr(ip, addr)) {
> > /* within range */
> > - old = PPC_INST_NOP;
> > + old = ppc_inst(PPC_INST_NOP);
> > new = ftrace_call_replace(ip, addr, 1);
> > return ftrace_modify_code(ip, old, new);
> > } else if (core_kernel_text(ip))
> > diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c
> > index 1cfef0e5fec5..31c870287f2b 100644
> > --- a/arch/powerpc/kernel/uprobes.c
> > +++ b/arch/powerpc/kernel/uprobes.c
> > @@ -14,6 +14,7 @@
> > #include <linux/kdebug.h>
> >
> > #include <asm/sstep.h>
> > +#include <asm/inst.h>
> >
> > #define UPROBE_TRAP_NR UINT_MAX
> >
> > diff --git a/arch/powerpc/kvm/emulate_loadstore.c
> > b/arch/powerpc/kvm/emulate_loadstore.c
> > index 1139bc56e004..135d0e686622 100644
> > --- a/arch/powerpc/kvm/emulate_loadstore.c
> > +++ b/arch/powerpc/kvm/emulate_loadstore.c
> > @@ -95,7 +95,7 @@ int kvmppc_emulate_loadstore(struct kvm_vcpu *vcpu)
> >
> > emulated = EMULATE_FAIL;
> > vcpu->arch.regs.msr = vcpu->arch.shared->msr;
> > - if (analyse_instr(&op, &vcpu->arch.regs, inst) == 0) {
> > + if (analyse_instr(&op, &vcpu->arch.regs, ppc_inst(inst)) == 0) {
> > int type = op.type & INSTR_TYPE_MASK;
> > int size = GETSIZE(op.type);
> >
> > diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-
> > patching.c
> > index 4af564aeedf1..fdf0d6ea3575 100644
> > --- a/arch/powerpc/lib/code-patching.c
> > +++ b/arch/powerpc/lib/code-patching.c
> > @@ -17,6 +17,7 @@
> > #include <asm/page.h>
> > #include <asm/code-patching.h>
> > #include <asm/setup.h>
> > +#include <asm/inst.h>
> >
> > static int __patch_instruction(unsigned int *exec_addr, unsigned int instr,
> > unsigned int *patch_addr)
> > @@ -413,37 +414,37 @@ static void __init test_branch_iform(void)
> > addr = (unsigned long)&instr;
> >
> > /* The simplest case, branch to self, no flags */
> > - check(instr_is_branch_iform(0x48000000));
> > + check(instr_is_branch_iform(ppc_inst(0x48000000)));
> > /* All bits of target set, and flags */
> > - check(instr_is_branch_iform(0x4bffffff));
> > + check(instr_is_branch_iform(ppc_inst(0x4bffffff)));
> > /* High bit of opcode set, which is wrong */
> > - check(!instr_is_branch_iform(0xcbffffff));
> > + check(!instr_is_branch_iform(ppc_inst(0xcbffffff)));
> > /* Middle bits of opcode set, which is wrong */
> > - check(!instr_is_branch_iform(0x7bffffff));
> > + check(!instr_is_branch_iform(ppc_inst(0x7bffffff)));
> >
> > /* Simplest case, branch to self with link */
> > - check(instr_is_branch_iform(0x48000001));
> > + check(instr_is_branch_iform(ppc_inst(0x48000001)));
> > /* All bits of targets set */
> > - check(instr_is_branch_iform(0x4bfffffd));
> > + check(instr_is_branch_iform(ppc_inst(0x4bfffffd)));
> > /* Some bits of targets set */
> > - check(instr_is_branch_iform(0x4bff00fd));
> > + check(instr_is_branch_iform(ppc_inst(0x4bff00fd)));
> > /* Must be a valid branch to start with */
> > - check(!instr_is_branch_iform(0x7bfffffd));
> > + check(!instr_is_branch_iform(ppc_inst(0x7bfffffd)));
> >
> > /* Absolute branch to 0x100 */
> > - instr = 0x48000103;
> > + instr = ppc_inst(0x48000103);
> > check(instr_is_branch_to_addr(&instr, 0x100));
> > /* Absolute branch to 0x420fc */
> > - instr = 0x480420ff;
> > + instr = ppc_inst(0x480420ff);
> > check(instr_is_branch_to_addr(&instr, 0x420fc));
> > /* Maximum positive relative branch, + 20MB - 4B */
> > - instr = 0x49fffffc;
> > + instr = ppc_inst(0x49fffffc);
> > check(instr_is_branch_to_addr(&instr, addr + 0x1FFFFFC));
> > /* Smallest negative relative branch, - 4B */
> > - instr = 0x4bfffffc;
> > + instr = ppc_inst(0x4bfffffc);
> > check(instr_is_branch_to_addr(&instr, addr - 4));
> > /* Largest negative relative branch, - 32 MB */
> > - instr = 0x4a000000;
> > + instr = ppc_inst(0x4a000000);
> > check(instr_is_branch_to_addr(&instr, addr - 0x2000000));
> >
> > /* Branch to self, with link */
> > @@ -477,7 +478,7 @@ static void __init test_branch_iform(void)
> > /* Check flags are masked correctly */
> > err = create_branch(&instr, &instr, addr, 0xFFFFFFFC);
> > check(instr_is_branch_to_addr(&instr, addr));
> > - check(instr == 0x48000000);
> > + check(instr == ppc_inst(0x48000000));
> > }
> >
> > static void __init test_create_function_call(void)
> > @@ -504,28 +505,28 @@ static void __init test_branch_bform(void)
> > addr = (unsigned long)iptr;
> >
> > /* The simplest case, branch to self, no flags */
> > - check(instr_is_branch_bform(0x40000000));
> > + check(instr_is_branch_bform(ppc_inst(0x40000000)));
> > /* All bits of target set, and flags */
> > - check(instr_is_branch_bform(0x43ffffff));
> > + check(instr_is_branch_bform(ppc_inst(0x43ffffff)));
> > /* High bit of opcode set, which is wrong */
> > - check(!instr_is_branch_bform(0xc3ffffff));
> > + check(!instr_is_branch_bform(ppc_inst(0xc3ffffff)));
> > /* Middle bits of opcode set, which is wrong */
> > - check(!instr_is_branch_bform(0x7bffffff));
> > + check(!instr_is_branch_bform(ppc_inst(0x7bffffff)));
> >
> > /* Absolute conditional branch to 0x100 */
> > - instr = 0x43ff0103;
> > + instr = ppc_inst(0x43ff0103);
> > check(instr_is_branch_to_addr(&instr, 0x100));
> > /* Absolute conditional branch to 0x20fc */
> > - instr = 0x43ff20ff;
> > + instr = ppc_inst(0x43ff20ff);
> > check(instr_is_branch_to_addr(&instr, 0x20fc));
> > /* Maximum positive relative conditional branch, + 32 KB - 4B */
> > - instr = 0x43ff7ffc;
> > + instr = ppc_inst(0x43ff7ffc);
> > check(instr_is_branch_to_addr(&instr, addr + 0x7FFC));
> > /* Smallest negative relative conditional branch, - 4B */
> > - instr = 0x43fffffc;
> > + instr = ppc_inst(0x43fffffc);
> > check(instr_is_branch_to_addr(&instr, addr - 4));
> > /* Largest negative relative conditional branch, - 32 KB */
> > - instr = 0x43ff8000;
> > + instr = ppc_inst(0x43ff8000);
> > check(instr_is_branch_to_addr(&instr, addr - 0x8000));
> >
> > /* All condition code bits set & link */
> > @@ -562,7 +563,7 @@ static void __init test_branch_bform(void)
> > /* Check flags are masked correctly */
> > err = create_cond_branch(&instr, iptr, addr, 0xFFFFFFFC);
> > check(instr_is_branch_to_addr(&instr, addr));
> > - check(instr == 0x43FF0000);
> > + check(instr == ppc_inst(0x43FF0000));
> > }
> >
> > static void __init test_translate_branch(void)
> > @@ -596,7 +597,7 @@ static void __init test_translate_branch(void)
> > patch_instruction(q, instr);
> > check(instr_is_branch_to_addr(p, addr));
> > check(instr_is_branch_to_addr(q, addr));
> > - check(*q == 0x4a000000);
> > + check(*q == ppc_inst(0x4a000000));
> >
> > /* Maximum positive case, move x to x - 32 MB + 4 */
> > p = buf + 0x2000000;
> > @@ -607,7 +608,7 @@ static void __init test_translate_branch(void)
> > patch_instruction(q, instr);
> > check(instr_is_branch_to_addr(p, addr));
> > check(instr_is_branch_to_addr(q, addr));
> > - check(*q == 0x49fffffc);
> > + check(*q == ppc_inst(0x49fffffc));
> >
> > /* Jump to x + 16 MB moved to x + 20 MB */
> > p = buf;
> > @@ -653,7 +654,7 @@ static void __init test_translate_branch(void)
> > patch_instruction(q, instr);
> > check(instr_is_branch_to_addr(p, addr));
> > check(instr_is_branch_to_addr(q, addr));
> > - check(*q == 0x43ff8000);
> > + check(*q == ppc_inst(0x43ff8000));
> >
> > /* Maximum positive case, move x to x - 32 KB + 4 */
> > p = buf + 0x8000;
> > @@ -665,7 +666,7 @@ static void __init test_translate_branch(void)
> > patch_instruction(q, instr);
> > check(instr_is_branch_to_addr(p, addr));
> > check(instr_is_branch_to_addr(q, addr));
> > - check(*q == 0x43ff7ffc);
> > + check(*q == ppc_inst(0x43ff7ffc));
> >
> > /* Jump to x + 12 KB moved to x + 20 KB */
> > p = buf;
> > diff --git a/arch/powerpc/lib/feature-fixups.c b/arch/powerpc/lib/feature-
> > fixups.c
> > index b129d7b4e7dd..6e7479b8887a 100644
> > --- a/arch/powerpc/lib/feature-fixups.c
> > +++ b/arch/powerpc/lib/feature-fixups.c
> > @@ -21,6 +21,7 @@
> > #include <asm/setup.h>
> > #include <asm/security_features.h>
> > #include <asm/firmware.h>
> > +#include <asm/inst.h>
> >
> > struct fixup_entry {
> > unsigned long mask;
> > @@ -89,7 +90,7 @@ static int patch_feature_section(unsigned long value,
> > struct fixup_entry *fcur)
> > }
> >
> > for (; dest < end; dest++)
> > - raw_patch_instruction(dest, PPC_INST_NOP);
> > + raw_patch_instruction(dest, ppc_inst(PPC_INST_NOP));
> >
> > return 0;
> > }
> > @@ -146,15 +147,15 @@ static void do_stf_entry_barrier_fixups(enum
> > stf_barrier_type types)
> >
> > pr_devel("patching dest %lx\n", (unsigned long)dest);
> >
> > - patch_instruction(dest, instrs[0]);
> > + patch_instruction(dest, ppc_inst(instrs[0]));
> >
> > if (types & STF_BARRIER_FALLBACK)
> > patch_branch(dest + 1, (unsigned
> > long)&stf_barrier_fallback,
> > BRANCH_SET_LINK);
> > else
> > - patch_instruction(dest + 1, instrs[1]);
> > + patch_instruction(dest + 1, ppc_inst(instrs[1]));
> >
> > - patch_instruction(dest + 2, instrs[2]);
> > + patch_instruction(dest + 2, ppc_inst(instrs[2]));
> > }
> >
> > printk(KERN_DEBUG "stf-barrier: patched %d entry locations (%s
> > barrier)\n", i,
> > @@ -207,12 +208,12 @@ static void do_stf_exit_barrier_fixups(enum
> > stf_barrier_type types)
> >
> > pr_devel("patching dest %lx\n", (unsigned long)dest);
> >
> > - patch_instruction(dest, instrs[0]);
> > - patch_instruction(dest + 1, instrs[1]);
> > - patch_instruction(dest + 2, instrs[2]);
> > - patch_instruction(dest + 3, instrs[3]);
> > - patch_instruction(dest + 4, instrs[4]);
> > - patch_instruction(dest + 5, instrs[5]);
> > + patch_instruction(dest, ppc_inst(instrs[0]));
> > + patch_instruction(dest + 1, ppc_inst(instrs[1]));
> > + patch_instruction(dest + 2, ppc_inst(instrs[2]));
> > + patch_instruction(dest + 3, ppc_inst(instrs[3]));
> > + patch_instruction(dest + 4, ppc_inst(instrs[4]));
> > + patch_instruction(dest + 5, ppc_inst(instrs[5]));
> > }
> > printk(KERN_DEBUG "stf-barrier: patched %d exit locations (%s
> > barrier)\n", i,
> > (types == STF_BARRIER_NONE) ? "no" :
> > @@ -260,9 +261,9 @@ void do_rfi_flush_fixups(enum l1d_flush_type types)
> >
> > pr_devel("patching dest %lx\n", (unsigned long)dest);
> >
> > - patch_instruction(dest, instrs[0]);
> > - patch_instruction(dest + 1, instrs[1]);
> > - patch_instruction(dest + 2, instrs[2]);
> > + patch_instruction(dest, ppc_inst(instrs[0]));
> > + patch_instruction(dest + 1, ppc_inst(instrs[1]));
> > + patch_instruction(dest + 2, ppc_inst(instrs[2]));
> > }
> >
> > printk(KERN_DEBUG "rfi-flush: patched %d locations (%s flush)\n", i,
> > @@ -295,7 +296,7 @@ void do_barrier_nospec_fixups_range(bool enable, void
> > *fixup_start, void *fixup_
> > dest = (void *)start + *start;
> >
> > pr_devel("patching dest %lx\n", (unsigned long)dest);
> > - patch_instruction(dest, instr);
> > + patch_instruction(dest, ppc_inst(instr));
> > }
> >
> > printk(KERN_DEBUG "barrier-nospec: patched %d locations\n", i);
> > @@ -338,8 +339,8 @@ void do_barrier_nospec_fixups_range(bool enable, void
> > *fixup_start, void *fixup_
> > dest = (void *)start + *start;
> >
> > pr_devel("patching dest %lx\n", (unsigned long)dest);
> > - patch_instruction(dest, instr[0]);
> > - patch_instruction(dest + 1, instr[1]);
> > + patch_instruction(dest, ppc_inst(instr[0]));
> > + patch_instruction(dest + 1, ppc_inst(instr[1]));
> > }
> >
> > printk(KERN_DEBUG "barrier-nospec: patched %d locations\n", i);
> > @@ -353,7 +354,7 @@ static void patch_btb_flush_section(long *curr)
> > end = (void *)curr + *(curr + 1);
> > for (; start < end; start++) {
> > pr_devel("patching dest %lx\n", (unsigned long)start);
> > - patch_instruction(start, PPC_INST_NOP);
> > + patch_instruction(start, ppc_inst(PPC_INST_NOP));
> > }
> > }
> >
> > @@ -382,7 +383,7 @@ void do_lwsync_fixups(unsigned long value, void
> > *fixup_start, void *fixup_end)
> >
> > for (; start < end; start++) {
> > dest = (void *)start + *start;
> > - raw_patch_instruction(dest, PPC_INST_LWSYNC);
> > + raw_patch_instruction(dest, ppc_inst(PPC_INST_LWSYNC));
> > }
> > }
> >
> > @@ -400,7 +401,7 @@ static void do_final_fixups(void)
> > length = (__end_interrupts - _stext) / sizeof(int);
> >
> > while (length--) {
> > - raw_patch_instruction(dest, *src);
> > + raw_patch_instruction(dest, ppc_inst(*src));
> > src++;
> > dest++;
> > }
> > diff --git a/arch/powerpc/lib/test_emulate_step.c
> > b/arch/powerpc/lib/test_emulate_step.c
> > index 42347067739c..88b17226a493 100644
> > --- a/arch/powerpc/lib/test_emulate_step.c
> > +++ b/arch/powerpc/lib/test_emulate_step.c
> > @@ -11,6 +11,7 @@
> > #include <asm/sstep.h>
> > #include <asm/ppc-opcode.h>
> > #include <asm/code-patching.h>
> > +#include <asm/inst.h>
> >
> > #define IMM_L(i) ((uintptr_t)(i) & 0xffff)
> >
> > @@ -18,40 +19,40 @@
> > * Defined with TEST_ prefix so it does not conflict with other
> > * definitions.
> > */
> > -#define TEST_LD(r, base, i) (PPC_INST_LD | ___PPC_RT(r) | \
> > +#define TEST_LD(r, base, i) ppc_inst(PPC_INST_LD | ___PPC_RT(r) |
> > \
> > ___PPC_RA(base) | IMM_L(i))
> > -#define TEST_LWZ(r, base, i) (PPC_INST_LWZ | ___PPC_RT(r) | \
> > +#define TEST_LWZ(r, base, i) ppc_inst(PPC_INST_LWZ | ___PPC_RT(r) |
> > \
> > ___PPC_RA(base) | IMM_L(i))
> > -#define TEST_LWZX(t, a, b) (PPC_INST_LWZX | ___PPC_RT(t) | \
> > +#define TEST_LWZX(t, a, b) ppc_inst(PPC_INST_LWZX | ___PPC_RT(t) |
> > \
> > ___PPC_RA(a) | ___PPC_RB(b))
> > -#define TEST_STD(r, base, i) (PPC_INST_STD | ___PPC_RS(r) | \
> > +#define TEST_STD(r, base, i) ppc_inst(PPC_INST_STD | ___PPC_RS(r) |
> > \
> > ___PPC_RA(base) | ((i) & 0xfffc))
> > -#define TEST_LDARX(t, a, b, eh) (PPC_INST_LDARX | ___PPC_RT(t) | \
> > +#define TEST_LDARX(t, a, b, eh) ppc_inst(PPC_INST_LDARX | ___PPC_RT(t)
> > | \
> > ___PPC_RA(a) | ___PPC_RB(b) | \
> > __PPC_EH(eh))
> > -#define TEST_STDCX(s, a, b) (PPC_INST_STDCX | ___PPC_RS(s) | \
> > +#define TEST_STDCX(s, a, b) ppc_inst(PPC_INST_STDCX | ___PPC_RS(s) | \
> > ___PPC_RA(a) | ___PPC_RB(b))
> > -#define TEST_LFSX(t, a, b) (PPC_INST_LFSX | ___PPC_RT(t) | \
> > +#define TEST_LFSX(t, a, b) ppc_inst(PPC_INST_LFSX | ___PPC_RT(t) |
> > \
> > ___PPC_RA(a) | ___PPC_RB(b))
> > -#define TEST_STFSX(s, a, b) (PPC_INST_STFSX | ___PPC_RS(s) | \
> > +#define TEST_STFSX(s, a, b) ppc_inst(PPC_INST_STFSX | ___PPC_RS(s) | \
> > ___PPC_RA(a) | ___PPC_RB(b))
> > -#define TEST_LFDX(t, a, b) (PPC_INST_LFDX | ___PPC_RT(t) | \
> > +#define TEST_LFDX(t, a, b) ppc_inst(PPC_INST_LFDX | ___PPC_RT(t) |
> > \
> > ___PPC_RA(a) | ___PPC_RB(b))
> > -#define TEST_STFDX(s, a, b) (PPC_INST_STFDX | ___PPC_RS(s) | \
> > +#define TEST_STFDX(s, a, b) ppc_inst(PPC_INST_STFDX | ___PPC_RS(s) | \
> > ___PPC_RA(a) | ___PPC_RB(b))
> > -#define TEST_LVX(t, a, b) (PPC_INST_LVX | ___PPC_RT(t) | \
> > +#define TEST_LVX(t, a, b) ppc_inst(PPC_INST_LVX | ___PPC_RT(t) |
> > \
> > ___PPC_RA(a) | ___PPC_RB(b))
> > -#define TEST_STVX(s, a, b) (PPC_INST_STVX | ___PPC_RS(s) | \
> > +#define TEST_STVX(s, a, b) ppc_inst(PPC_INST_STVX | ___PPC_RS(s) |
> > \
> > ___PPC_RA(a) | ___PPC_RB(b))
> > -#define TEST_LXVD2X(s, a, b) (PPC_INST_LXVD2X | VSX_XX1((s), R##a, R##b))
> > -#define TEST_STXVD2X(s, a, b) (PPC_INST_STXVD2X | VSX_XX1((s), R##a,
> > R##b))
> > -#define TEST_ADD(t, a, b) (PPC_INST_ADD | ___PPC_RT(t) | \
> > +#define TEST_LXVD2X(s, a, b) ppc_inst(PPC_INST_LXVD2X | VSX_XX1((s), R##a,
> > R##b))
> > +#define TEST_STXVD2X(s, a, b) ppc_inst(PPC_INST_STXVD2X |
> > VSX_XX1((s), R##a, R##b))
> > +#define TEST_ADD(t, a, b) ppc_inst(PPC_INST_ADD | ___PPC_RT(t) |
> > \
> > ___PPC_RA(a) | ___PPC_RB(b))
> > -#define TEST_ADD_DOT(t, a, b) (PPC_INST_ADD | ___PPC_RT(t) |
> > \
> > +#define TEST_ADD_DOT(t, a, b) ppc_inst(PPC_INST_ADD | ___PPC_RT(t) |
> > \
> > ___PPC_RA(a) | ___PPC_RB(b) | 0x1)
> > -#define TEST_ADDC(t, a, b) (PPC_INST_ADDC | ___PPC_RT(t) | \
> > +#define TEST_ADDC(t, a, b) ppc_inst(PPC_INST_ADDC | ___PPC_RT(t) |
> > \
> > ___PPC_RA(a) | ___PPC_RB(b))
> > -#define TEST_ADDC_DOT(t, a, b) (PPC_INST_ADDC | ___PPC_RT(t) |
> > \
> > +#define TEST_ADDC_DOT(t, a, b) ppc_inst(PPC_INST_ADDC | ___PPC_RT(t) |
> > \
> > ___PPC_RA(a) | ___PPC_RB(b) | 0x1)
> >
> > #define MAX_SUBTESTS 16
> > @@ -471,7 +472,7 @@ static struct compute_test compute_tests[] = {
> > .subtests = {
> > {
> > .descr = "R0 = LONG_MAX",
> > - .instr = PPC_INST_NOP,
> > + .instr = ppc_inst(PPC_INST_NOP),
> > .regs = {
> > .gpr[0] = LONG_MAX,
> > }
> > diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> > index 049375206510..68ecb39dd9cc 100644
> > --- a/arch/powerpc/xmon/xmon.c
> > +++ b/arch/powerpc/xmon/xmon.c
> > @@ -54,6 +54,7 @@
> > #include <asm/firmware.h>
> > #include <asm/code-patching.h>
> > #include <asm/sections.h>
> > +#include <asm/inst.h>
> >
> > #ifdef CONFIG_PPC64
> > #include <asm/hvcall.h>
> > @@ -949,7 +950,7 @@ static void remove_bpts(void)
> > if ((bp->enabled & (BP_TRAP|BP_CIABR)) != BP_TRAP)
> > continue;
> > if (mread(bp->address, &instr, 4) == 4
> > - && instr == bpinstr
> > + && instr == ppc_inst(bpinstr)
> > && patch_instruction(
> > (unsigned int *)bp->address, bp->instr[0]) != 0)
> > printf("Couldn't remove breakpoint at %lx\n",
> > @@ -2845,7 +2846,7 @@ generic_inst_dump(unsigned long adr, long count, int
> > praddr,
> > {
> > int nr, dotted;
> > unsigned long first_adr;
> > - unsigned int inst, last_inst = 0;
> > + unsigned int inst, last_inst = ppc_inst(0);
> > unsigned char val[4];
> >
> > dotted = 0;
> > @@ -2858,7 +2859,7 @@ generic_inst_dump(unsigned long adr, long count, int
> > praddr,
> > }
> > break;
> > }
> > - inst = GETWORD(val);
> > + inst = ppc_inst(GETWORD(val));
> > if (adr > first_adr && inst == last_inst) {
> > if (!dotted) {
> > printf(" ...\n");
>
^ permalink raw reply
* Re: [PATCH v5 07/21] powerpc: Use a function for byte swapping instructions
From: Balamuruhan S @ 2020-04-07 7:42 UTC (permalink / raw)
To: Jordan Niethe, linuxppc-dev; +Cc: alistair, npiggin, dja
In-Reply-To: <20200406080936.7180-8-jniethe5@gmail.com>
On Mon, 2020-04-06 at 18:09 +1000, Jordan Niethe wrote:
> Use a function for byte swapping instructions in preparation of a more
> complicated instruction type.
Reviewed-by: Balamuruhan S <bala24@linux.ibm.com>
>
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> ---
> arch/powerpc/include/asm/inst.h | 5 +++++
> arch/powerpc/kernel/align.c | 2 +-
> 2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/inst.h
> b/arch/powerpc/include/asm/inst.h
> index 15f570bef936..78eb1481f1f6 100644
> --- a/arch/powerpc/include/asm/inst.h
> +++ b/arch/powerpc/include/asm/inst.h
> @@ -18,4 +18,9 @@ static inline int ppc_inst_opcode(u32 x)
> return x >> 26;
> }
>
> +static inline u32 ppc_inst_swab(u32 x)
> +{
> + return ppc_inst(swab32(ppc_inst_val(x)));
> +}
> +
> #endif /* _ASM_INST_H */
> diff --git a/arch/powerpc/kernel/align.c b/arch/powerpc/kernel/align.c
> index a83d32c6513d..46870cf6a6dc 100644
> --- a/arch/powerpc/kernel/align.c
> +++ b/arch/powerpc/kernel/align.c
> @@ -310,7 +310,7 @@ int fix_alignment(struct pt_regs *regs)
> /* We don't handle PPC little-endian any more... */
> if (cpu_has_feature(CPU_FTR_PPC_LE))
> return -EIO;
> - instr = swab32(instr);
> + instr = ppc_inst_swab(instr);
> }
>
> #ifdef CONFIG_SPE
^ permalink raw reply
* Re: [PATCH v5 08/21] powerpc: Introduce functions for instruction equality
From: Balamuruhan S @ 2020-04-07 7:37 UTC (permalink / raw)
To: Jordan Niethe, linuxppc-dev; +Cc: alistair, npiggin, dja
In-Reply-To: <20200406080936.7180-9-jniethe5@gmail.com>
On Mon, 2020-04-06 at 18:09 +1000, Jordan Niethe wrote:
> In preparation for an instruction data type that can not be directly
> used with the '==' operator use functions for checking equality.
LGTM except one comment below, otherwise
Reviewed-by: Balamuruhan S <bala24@linux.ibm.com>
>
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> ---
> v5: Remove ppc_inst_null()
> ---
> arch/powerpc/include/asm/inst.h | 5 +++++
> arch/powerpc/kernel/trace/ftrace.c | 15 ++++++++-------
> arch/powerpc/lib/code-patching.c | 12 ++++++------
> arch/powerpc/lib/test_emulate_step.c | 2 +-
> arch/powerpc/xmon/xmon.c | 4 ++--
> 5 files changed, 22 insertions(+), 16 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/inst.h
> b/arch/powerpc/include/asm/inst.h
> index 78eb1481f1f6..54ee46b0a7c9 100644
> --- a/arch/powerpc/include/asm/inst.h
> +++ b/arch/powerpc/include/asm/inst.h
> @@ -23,4 +23,9 @@ static inline u32 ppc_inst_swab(u32 x)
> return ppc_inst(swab32(ppc_inst_val(x)));
> }
>
> +static inline bool ppc_inst_equal(u32 x, u32 y)
> +{
> + return x == y;
> +}
> +
> #endif /* _ASM_INST_H */
> diff --git a/arch/powerpc/kernel/trace/ftrace.c
> b/arch/powerpc/kernel/trace/ftrace.c
> index 62ff429bddc4..784b5746cc55 100644
> --- a/arch/powerpc/kernel/trace/ftrace.c
> +++ b/arch/powerpc/kernel/trace/ftrace.c
> @@ -72,7 +72,7 @@ ftrace_modify_code(unsigned long ip, unsigned int old,
> unsigned int new)
> return -EFAULT;
>
> /* Make sure it is what we expect it to be */
> - if (replaced != old) {
> + if (!ppc_inst_equal(replaced, old)) {
> pr_err("%p: replaced (%#x) != old (%#x)",
> (void *)ip, ppc_inst_val(replaced), ppc_inst_val(old));
> return -EINVAL;
> @@ -170,7 +170,8 @@ __ftrace_make_nop(struct module *mod,
> }
>
> /* We expect either a mflr r0, or a std r0, LRSAVE(r1) */
> - if (op != ppc_inst(PPC_INST_MFLR) && op != ppc_inst(PPC_INST_STD_LR)) {
> + if (!ppc_inst_equal(op, ppc_inst(PPC_INST_MFLR)) &&
> + !ppc_inst_equal(op, ppc_inst(PPC_INST_STD_LR))) {
> pr_err("Unexpected instruction %08x around bl _mcount\n",
> ppc_inst_val(op));
> return -EINVAL;
> }
> @@ -200,7 +201,7 @@ __ftrace_make_nop(struct module *mod,
> return -EFAULT;
> }
>
> - if (op != ppc_inst(PPC_INST_LD_TOC)) {
> + if (!ppc_inst_equal(op, ppc_inst(PPC_INST_LD_TOC))) {
> pr_err("Expected %08x found %08x\n", PPC_INST_LD_TOC,
> ppc_inst_val(op));
> return -EINVAL;
> }
> @@ -497,7 +498,7 @@ expected_nop_sequence(void *ip, unsigned int op0,
> unsigned int op1)
> * The load offset is different depending on the ABI. For simplicity
> * just mask it out when doing the compare.
> */
> - if ((op0 != ppc_inst(0x48000008)) || (ppc_inst_val(op1) & 0xffff0000)
> != 0xe8410000)
> + if ((!ppc_inst_equal(op0), ppc_inst(0x48000008)) || (ppc_inst_val(op1)
> & 0xffff0000) != 0xe8410000)
> return 0;
> return 1;
> }
> @@ -506,7 +507,7 @@ static int
> expected_nop_sequence(void *ip, unsigned int op0, unsigned int op1)
> {
> /* look for patched "NOP" on ppc64 with -mprofile-kernel */
> - if (op0 != ppc_inst(PPC_INST_NOP))
> + if (!ppc_inst_equal(op0, ppc_inst(PPC_INST_NOP)))
> return 0;
> return 1;
> }
> @@ -589,7 +590,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long
> addr)
> return -EFAULT;
>
> /* It should be pointing to a nop */
> - if (op != ppc_inst(PPC_INST_NOP)) {
> + if (!ppc_inst_equal(op, ppc_inst(PPC_INST_NOP))) {
> pr_err("Expected NOP but have %x\n", op);
> return -EINVAL;
> }
> @@ -646,7 +647,7 @@ static int __ftrace_make_call_kernel(struct dyn_ftrace
> *rec, unsigned long addr)
> return -EFAULT;
> }
>
> - if (op != ppc_inst(PPC_INST_NOP)) {
> + if (!ppc_inst_equal(op, ppc_inst(PPC_INST_NOP))) {
> pr_err("Unexpected call sequence at %p: %x\n", ip,
> ppc_inst_val(op));
> return -EINVAL;
> }
> diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-
> patching.c
> index 3f88d2a4400c..33654c6334a9 100644
> --- a/arch/powerpc/lib/code-patching.c
> +++ b/arch/powerpc/lib/code-patching.c
> @@ -478,7 +478,7 @@ static void __init test_branch_iform(void)
> /* Check flags are masked correctly */
> err = create_branch(&instr, &instr, addr, 0xFFFFFFFC);
> check(instr_is_branch_to_addr(&instr, addr));
> - check(instr == ppc_inst(0x48000000));
> + check(ppc_inst_equal(instr, ppc_inst(0x48000000)));
> }
>
> static void __init test_create_function_call(void)
> @@ -563,7 +563,7 @@ static void __init test_branch_bform(void)
> /* Check flags are masked correctly */
> err = create_cond_branch(&instr, iptr, addr, 0xFFFFFFFC);
> check(instr_is_branch_to_addr(&instr, addr));
> - check(instr == ppc_inst(0x43FF0000));
> + check(ppc_inst_equal(instr, ppc_inst(0x43FF0000)));
> }
>
> static void __init test_translate_branch(void)
> @@ -597,7 +597,7 @@ static void __init test_translate_branch(void)
> patch_instruction(q, instr);
> check(instr_is_branch_to_addr(p, addr));
> check(instr_is_branch_to_addr(q, addr));
> - check(*q == ppc_inst(0x4a000000));
> + check(ppc_inst_equal(*q, ppc_inst(0x4a000000)));
>
> /* Maximum positive case, move x to x - 32 MB + 4 */
> p = buf + 0x2000000;
> @@ -608,7 +608,7 @@ static void __init test_translate_branch(void)
> patch_instruction(q, instr);
> check(instr_is_branch_to_addr(p, addr));
> check(instr_is_branch_to_addr(q, addr));
> - check(*q == ppc_inst(0x49fffffc));
> + check(ppc_inst_equal(*q, ppc_inst(0x49fffffc)));
>
> /* Jump to x + 16 MB moved to x + 20 MB */
> p = buf;
> @@ -654,7 +654,7 @@ static void __init test_translate_branch(void)
> patch_instruction(q, instr);
> check(instr_is_branch_to_addr(p, addr));
> check(instr_is_branch_to_addr(q, addr));
> - check(*q == ppc_inst(0x43ff8000));
> + check(ppc_inst_equal(*q, ppc_inst(0x43ff8000)));
>
> /* Maximum positive case, move x to x - 32 KB + 4 */
> p = buf + 0x8000;
> @@ -666,7 +666,7 @@ static void __init test_translate_branch(void)
> patch_instruction(q, instr);
> check(instr_is_branch_to_addr(p, addr));
> check(instr_is_branch_to_addr(q, addr));
> - check(*q == ppc_inst(0x43ff7ffc));
> + check(ppc_inst_equal(*q, ppc_inst(0x43ff7ffc)));
>
> /* Jump to x + 12 KB moved to x + 20 KB */
> p = buf;
> diff --git a/arch/powerpc/lib/test_emulate_step.c
> b/arch/powerpc/lib/test_emulate_step.c
> index 60f7eb24d742..16387a9bfda0 100644
> --- a/arch/powerpc/lib/test_emulate_step.c
> +++ b/arch/powerpc/lib/test_emulate_step.c
> @@ -865,7 +865,7 @@ static int __init execute_compute_instr(struct pt_regs
> *regs,
> extern int exec_instr(struct pt_regs *regs);
> extern s32 patch__exec_instr;
>
> - if (!regs || !instr)
> + if (!regs || !ppc_inst_val(instr))
This change should go in to below patch,
[PATCH v5 06/21] powerpc: Use an accessor for instructions
-- Bala
> return -EINVAL;
>
> /* Patch the NOP with the actual instruction */
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index 3c1fb46bfacf..f6c87d3d53ea 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -950,7 +950,7 @@ static void remove_bpts(void)
> if ((bp->enabled & (BP_TRAP|BP_CIABR)) != BP_TRAP)
> continue;
> if (mread(bp->address, &instr, 4) == 4
> - && instr == ppc_inst(bpinstr)
> + && ppc_inst_equal(instr, ppc_inst(bpinstr))
> && patch_instruction(
> (unsigned int *)bp->address, bp->instr[0]) != 0)
> printf("Couldn't remove breakpoint at %lx\n",
> @@ -2860,7 +2860,7 @@ generic_inst_dump(unsigned long adr, long count, int
> praddr,
> break;
> }
> inst = ppc_inst(GETWORD(val));
> - if (adr > first_adr && inst == last_inst) {
> + if (adr > first_adr && ppc_inst_equal(inst, last_inst)) {
> if (!dotted) {
> printf(" ...\n");
> dotted = 1;
^ permalink raw reply
* Re: [PATCH v8 1/7] perf expr: Add expr_ prefix for parse_ctx and parse_id
From: kajoljain @ 2020-04-07 7:11 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: mark.rutland, maddy, peterz, yao.jin, mingo, kan.liang, ak,
alexander.shishkin, anju, mamatha4, sukadev, ravi.bangoria,
jmario, namhyung, tglx, mpetlan, gregkh, linux-kernel,
linux-perf-users, jolsa, linuxppc-dev
In-Reply-To: <20200406141357.GG29826@kernel.org>
On 4/6/20 7:43 PM, Arnaldo Carvalho de Melo wrote:
> Em Thu, Apr 02, 2020 at 02:03:34AM +0530, Kajol Jain escreveu:
>> From: Jiri Olsa <jolsa@kernel.org>
>>
>> Adding expr_ prefix for parse_ctx and parse_id,
>> to straighten out the expr* namespace.
>>
>> There's no functional change.
>
> Next time please add your Signed-off-by: as well when pushing 3rd party
> patches.
>
> Applied.
>
> - Arnaldo
Hi Arnaldo,
Thanks, I will take care of it next time.
Regards,
Kajol
>
>> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
>> ---
>> tools/perf/tests/expr.c | 4 ++--
>> tools/perf/util/expr.c | 10 +++++-----
>> tools/perf/util/expr.h | 12 ++++++------
>> tools/perf/util/expr.y | 6 +++---
>> tools/perf/util/stat-shadow.c | 2 +-
>> 5 files changed, 17 insertions(+), 17 deletions(-)
>>
>> diff --git a/tools/perf/tests/expr.c b/tools/perf/tests/expr.c
>> index 28313e59d6f6..ea10fc4412c4 100644
>> --- a/tools/perf/tests/expr.c
>> +++ b/tools/perf/tests/expr.c
>> @@ -6,7 +6,7 @@
>> #include <string.h>
>> #include <linux/zalloc.h>
>>
>> -static int test(struct parse_ctx *ctx, const char *e, double val2)
>> +static int test(struct expr_parse_ctx *ctx, const char *e, double val2)
>> {
>> double val;
>>
>> @@ -22,7 +22,7 @@ int test__expr(struct test *t __maybe_unused, int subtest __maybe_unused)
>> const char **other;
>> double val;
>> int i, ret;
>> - struct parse_ctx ctx;
>> + struct expr_parse_ctx ctx;
>> int num_other;
>>
>> expr__ctx_init(&ctx);
>> diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c
>> index fd192ddf93c1..c8ccc548a585 100644
>> --- a/tools/perf/util/expr.c
>> +++ b/tools/perf/util/expr.c
>> @@ -11,7 +11,7 @@ extern int expr_debug;
>> #endif
>>
>> /* Caller must make sure id is allocated */
>> -void expr__add_id(struct parse_ctx *ctx, const char *name, double val)
>> +void expr__add_id(struct expr_parse_ctx *ctx, const char *name, double val)
>> {
>> int idx;
>>
>> @@ -21,13 +21,13 @@ void expr__add_id(struct parse_ctx *ctx, const char *name, double val)
>> ctx->ids[idx].val = val;
>> }
>>
>> -void expr__ctx_init(struct parse_ctx *ctx)
>> +void expr__ctx_init(struct expr_parse_ctx *ctx)
>> {
>> ctx->num_ids = 0;
>> }
>>
>> static int
>> -__expr__parse(double *val, struct parse_ctx *ctx, const char *expr,
>> +__expr__parse(double *val, struct expr_parse_ctx *ctx, const char *expr,
>> int start)
>> {
>> YY_BUFFER_STATE buffer;
>> @@ -52,7 +52,7 @@ __expr__parse(double *val, struct parse_ctx *ctx, const char *expr,
>> return ret;
>> }
>>
>> -int expr__parse(double *final_val, struct parse_ctx *ctx, const char *expr)
>> +int expr__parse(double *final_val, struct expr_parse_ctx *ctx, const char *expr)
>> {
>> return __expr__parse(final_val, ctx, expr, EXPR_PARSE) ? -1 : 0;
>> }
>> @@ -75,7 +75,7 @@ int expr__find_other(const char *expr, const char *one, const char ***other,
>> int *num_other)
>> {
>> int err, i = 0, j = 0;
>> - struct parse_ctx ctx;
>> + struct expr_parse_ctx ctx;
>>
>> expr__ctx_init(&ctx);
>> err = __expr__parse(NULL, &ctx, expr, EXPR_OTHER);
>> diff --git a/tools/perf/util/expr.h b/tools/perf/util/expr.h
>> index 9377538f4097..b9e53f2b5844 100644
>> --- a/tools/perf/util/expr.h
>> +++ b/tools/perf/util/expr.h
>> @@ -5,19 +5,19 @@
>> #define EXPR_MAX_OTHER 20
>> #define MAX_PARSE_ID EXPR_MAX_OTHER
>>
>> -struct parse_id {
>> +struct expr_parse_id {
>> const char *name;
>> double val;
>> };
>>
>> -struct parse_ctx {
>> +struct expr_parse_ctx {
>> int num_ids;
>> - struct parse_id ids[MAX_PARSE_ID];
>> + struct expr_parse_id ids[MAX_PARSE_ID];
>> };
>>
>> -void expr__ctx_init(struct parse_ctx *ctx);
>> -void expr__add_id(struct parse_ctx *ctx, const char *id, double val);
>> -int expr__parse(double *final_val, struct parse_ctx *ctx, const char *expr);
>> +void expr__ctx_init(struct expr_parse_ctx *ctx);
>> +void expr__add_id(struct expr_parse_ctx *ctx, const char *id, double val);
>> +int expr__parse(double *final_val, struct expr_parse_ctx *ctx, const char *expr);
>> int expr__find_other(const char *expr, const char *one, const char ***other,
>> int *num_other);
>>
>> diff --git a/tools/perf/util/expr.y b/tools/perf/util/expr.y
>> index 4720cbe79357..cd17486c1c5d 100644
>> --- a/tools/perf/util/expr.y
>> +++ b/tools/perf/util/expr.y
>> @@ -15,7 +15,7 @@
>> %define api.pure full
>>
>> %parse-param { double *final_val }
>> -%parse-param { struct parse_ctx *ctx }
>> +%parse-param { struct expr_parse_ctx *ctx }
>> %parse-param {void *scanner}
>> %lex-param {void* scanner}
>>
>> @@ -39,14 +39,14 @@
>>
>> %{
>> static void expr_error(double *final_val __maybe_unused,
>> - struct parse_ctx *ctx __maybe_unused,
>> + struct expr_parse_ctx *ctx __maybe_unused,
>> void *scanner,
>> const char *s)
>> {
>> pr_debug("%s\n", s);
>> }
>>
>> -static int lookup_id(struct parse_ctx *ctx, char *id, double *val)
>> +static int lookup_id(struct expr_parse_ctx *ctx, char *id, double *val)
>> {
>> int i;
>>
>> diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c
>> index 0fd713d3674f..402af3e8d287 100644
>> --- a/tools/perf/util/stat-shadow.c
>> +++ b/tools/perf/util/stat-shadow.c
>> @@ -729,7 +729,7 @@ static void generic_metric(struct perf_stat_config *config,
>> struct runtime_stat *st)
>> {
>> print_metric_t print_metric = out->print_metric;
>> - struct parse_ctx pctx;
>> + struct expr_parse_ctx pctx;
>> double ratio, scale;
>> int i;
>> void *ctxp = out->ctx;
>> --
>> 2.21.0
>>
>
^ permalink raw reply
* Re: [PATCH v5 05/21] powerpc: Use a function for getting the instruction op code
From: Balamuruhan S @ 2020-04-07 7:04 UTC (permalink / raw)
To: Jordan Niethe, linuxppc-dev; +Cc: alistair, npiggin, dja
In-Reply-To: <20200406080936.7180-6-jniethe5@gmail.com>
On Mon, 2020-04-06 at 18:09 +1000, Jordan Niethe wrote:
> In preparation for using a data type for instructions that can not be
> directly used with the '>>' operator use a function for getting the op
> code of an instruction.
vecemu.c and sstep.c will need ppc_inst_opcode().
-- Bala
>
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> ---
> v4: New to series
> ---
> arch/powerpc/include/asm/inst.h | 5 +++++
> arch/powerpc/kernel/align.c | 2 +-
> arch/powerpc/lib/code-patching.c | 4 ++--
> 3 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/inst.h
> b/arch/powerpc/include/asm/inst.h
> index 5298ba33b6e5..93959016fe4b 100644
> --- a/arch/powerpc/include/asm/inst.h
> +++ b/arch/powerpc/include/asm/inst.h
> @@ -8,4 +8,9 @@
>
> #define ppc_inst(x) (x)
>
> +static inline int ppc_inst_opcode(u32 x)
> +{
> + return x >> 26;
> +}
> +
> #endif /* _ASM_INST_H */
> diff --git a/arch/powerpc/kernel/align.c b/arch/powerpc/kernel/align.c
> index 86e9bf62f18c..691013aa9f3c 100644
> --- a/arch/powerpc/kernel/align.c
> +++ b/arch/powerpc/kernel/align.c
> @@ -314,7 +314,7 @@ int fix_alignment(struct pt_regs *regs)
> }
>
> #ifdef CONFIG_SPE
> - if ((instr >> 26) == 0x4) {
> + if (ppc_inst_opcode(instr) == 0x4) {
> int reg = (instr >> 21) & 0x1f;
> PPC_WARN_ALIGNMENT(spe, regs);
> return emulate_spe(regs, reg, instr);
> diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-
> patching.c
> index fdf0d6ea3575..099a515202aa 100644
> --- a/arch/powerpc/lib/code-patching.c
> +++ b/arch/powerpc/lib/code-patching.c
> @@ -231,7 +231,7 @@ bool is_offset_in_branch_range(long offset)
> */
> bool is_conditional_branch(unsigned int instr)
> {
> - unsigned int opcode = instr >> 26;
> + unsigned int opcode = ppc_inst_opcode(instr);
>
> if (opcode == 16) /* bc, bca, bcl, bcla */
> return true;
> @@ -289,7 +289,7 @@ int create_cond_branch(unsigned int *instr, const
> unsigned int *addr,
>
> static unsigned int branch_opcode(unsigned int instr)
> {
> - return (instr >> 26) & 0x3F;
> + return ppc_inst_opcode(instr) & 0x3F;
> }
>
> static int instr_is_branch_iform(unsigned int instr)
^ permalink raw reply
* Re: [PATCH v5 03/21] powerpc: Change calling convention for create_branch() et. al.
From: Balamuruhan S @ 2020-04-07 6:59 UTC (permalink / raw)
To: Jordan Niethe
Cc: Alistair Popple, linuxppc-dev, Nicholas Piggin, Daniel Axtens
In-Reply-To: <CACzsE9qAUALdPkAA-0dx+L1Yhcj=3y7rw1yCTQNv0KF5MptwJw@mail.gmail.com>
On Tue, 2020-04-07 at 16:35 +1000, Jordan Niethe wrote:
> On Tue, Apr 7, 2020 at 4:10 PM Balamuruhan S <bala24@linux.ibm.com> wrote:
> > On Mon, 2020-04-06 at 18:09 +1000, Jordan Niethe wrote:
> > > create_branch(), create_cond_branch() and translate_branch() return the
> > > instruction that they create, or return 0 to signal an error. Seperate
> >
> > s/seperate/separate
> thanks.
> > > these concerns in preparation for an instruction type that is not just
> > > an unsigned int. Fill the created instruction to a pointer passed as
> > > the first parameter to the function and use a non-zero return value to
> > > signify an error.
> > >
> > > Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> > > ---
> > > v5: New to series
> > > ---
> > > arch/powerpc/include/asm/code-patching.h | 12 +-
> > > arch/powerpc/kernel/optprobes.c | 24 ++--
> > > arch/powerpc/kernel/setup_32.c | 2 +-
> > > arch/powerpc/kernel/trace/ftrace.c | 24 ++--
> > > arch/powerpc/lib/code-patching.c | 133 +++++++++++++----------
> > > arch/powerpc/lib/feature-fixups.c | 5 +-
> > > 6 files changed, 117 insertions(+), 83 deletions(-)
> > >
> > > diff --git a/arch/powerpc/include/asm/code-patching.h
> > > b/arch/powerpc/include/asm/code-patching.h
> > > index 898b54262881..351dda7215b6 100644
> > > --- a/arch/powerpc/include/asm/code-patching.h
> > > +++ b/arch/powerpc/include/asm/code-patching.h
> > > @@ -22,10 +22,10 @@
> > > #define BRANCH_ABSOLUTE 0x2
> > >
> > > bool is_offset_in_branch_range(long offset);
> > > -unsigned int create_branch(const unsigned int *addr,
> > > - unsigned long target, int flags);
> > > -unsigned int create_cond_branch(const unsigned int *addr,
> > > - unsigned long target, int flags);
> > > +int create_branch(unsigned int *instr, const unsigned int *addr,
> > > + unsigned long target, int flags);
> > > +int create_cond_branch(unsigned int *instr, const unsigned int *addr,
> > > + unsigned long target, int flags);
> > > int patch_branch(unsigned int *addr, unsigned long target, int flags);
> > > int patch_instruction(unsigned int *addr, unsigned int instr);
> > > int raw_patch_instruction(unsigned int *addr, unsigned int instr);
> > > @@ -60,8 +60,8 @@ int instr_is_relative_branch(unsigned int instr);
> > > int instr_is_relative_link_branch(unsigned int instr);
> > > int instr_is_branch_to_addr(const unsigned int *instr, unsigned long
> > > addr);
> > > unsigned long branch_target(const unsigned int *instr);
> > > -unsigned int translate_branch(const unsigned int *dest,
> > > - const unsigned int *src);
> > > +int translate_branch(unsigned int *instr, const unsigned int *dest,
> > > + const unsigned int *src);
> > > extern bool is_conditional_branch(unsigned int instr);
> > > #ifdef CONFIG_PPC_BOOK3E_64
> > > void __patch_exception(int exc, unsigned long addr);
> > > diff --git a/arch/powerpc/kernel/optprobes.c
> > > b/arch/powerpc/kernel/optprobes.c
> > > index 024f7aad1952..445b3dad82dc 100644
> > > --- a/arch/powerpc/kernel/optprobes.c
> > > +++ b/arch/powerpc/kernel/optprobes.c
> > > @@ -251,15 +251,17 @@ int arch_prepare_optimized_kprobe(struct
> > > optimized_kprobe *op, struct kprobe *p)
> > > goto error;
> > > }
> > >
> > > - branch_op_callback = create_branch((unsigned int *)buff +
> > > TMPL_CALL_HDLR_IDX,
> > > - (unsigned long)op_callback_addr,
> > > - BRANCH_SET_LINK);
> > > + rc = create_branch(&branch_op_callback,
> > > + (unsigned int *)buff + TMPL_CALL_HDLR_IDX,
> > > + (unsigned long)op_callback_addr,
> > > + BRANCH_SET_LINK);
> > >
> > > - branch_emulate_step = create_branch((unsigned int *)buff +
> > > TMPL_EMULATE_IDX,
> > > - (unsigned long)emulate_step_addr,
> > > - BRANCH_SET_LINK);
> > > + rc |= create_branch(&branch_emulate_step,
> > > + (unsigned int *)buff + TMPL_EMULATE_IDX,
> > > + (unsigned long)emulate_step_addr,
> > > + BRANCH_SET_LINK);
> > >
> > > - if (!branch_op_callback || !branch_emulate_step)
> > > + if (rc)
> > > goto error;
> > >
> > > patch_instruction(buff + TMPL_CALL_HDLR_IDX, branch_op_callback);
> > > @@ -305,6 +307,7 @@ int arch_check_optimized_kprobe(struct
> > > optimized_kprobe
> > > *op)
> > >
> > > void arch_optimize_kprobes(struct list_head *oplist)
> > > {
> > > + unsigned int instr;
> > > struct optimized_kprobe *op;
> > > struct optimized_kprobe *tmp;
> > >
> > > @@ -315,9 +318,10 @@ void arch_optimize_kprobes(struct list_head *oplist)
> > > */
> > > memcpy(op->optinsn.copied_insn, op->kp.addr,
> > > RELATIVEJUMP_SIZE);
> > > - patch_instruction(op->kp.addr,
> > > - create_branch((unsigned int *)op->kp.addr,
> > > - (unsigned long)op->optinsn.insn, 0));
> > > + create_branch(&instr,
> > > + (unsigned int *)op->kp.addr,
> > > + (unsigned long)op->optinsn.insn, 0);
> > > + patch_instruction(op->kp.addr, instr);
> > > list_del_init(&op->list);
> > > }
> > > }
> > > diff --git a/arch/powerpc/kernel/setup_32.c
> > > b/arch/powerpc/kernel/setup_32.c
> > > index 5b49b26eb154..c1bdd462c5c0 100644
> > > --- a/arch/powerpc/kernel/setup_32.c
> > > +++ b/arch/powerpc/kernel/setup_32.c
> > > @@ -88,7 +88,7 @@ notrace void __init machine_init(u64 dt_ptr)
> > >
> > > patch_instruction_site(&patch__memcpy_nocache, PPC_INST_NOP);
> > >
> > > - insn = create_cond_branch(addr, branch_target(addr), 0x820000);
> > > + create_cond_branch(&insn, addr, branch_target(addr), 0x820000);
> > > patch_instruction(addr, insn); /* replace b by bne cr0 */
> > >
> > > /* Do some early initialization based on the flat device tree */
> > > diff --git a/arch/powerpc/kernel/trace/ftrace.c
> > > b/arch/powerpc/kernel/trace/ftrace.c
> > > index 7ea0ca044b65..4dcb379133f8 100644
> > > --- a/arch/powerpc/kernel/trace/ftrace.c
> > > +++ b/arch/powerpc/kernel/trace/ftrace.c
> > > @@ -48,7 +48,7 @@ ftrace_call_replace(unsigned long ip, unsigned long
> > > addr,
> > > int link)
> > > addr = ppc_function_entry((void *)addr);
> > >
> > > /* if (link) set op to 'bl' else 'b' */
> > > - op = create_branch((unsigned int *)ip, addr, link ? 1 : 0);
> > > + create_branch(&op, (unsigned int *)ip, addr, link ? 1 : 0);
> > >
> > > return op;
> > > }
> > > @@ -89,10 +89,11 @@ ftrace_modify_code(unsigned long ip, unsigned int
> > > old,
> > > unsigned int new)
> > > */
> > > static int test_24bit_addr(unsigned long ip, unsigned long addr)
> > > {
> > > + unsigned int op;
> > > addr = ppc_function_entry((void *)addr);
> > >
> > > /* use the create_branch to verify that this offset can be branched
> > > */
> > > - return create_branch((unsigned int *)ip, addr, 0);
> > > + return create_branch(&op, (unsigned int *)ip, addr, 0) == 0;
> > > }
> > >
> > > static int is_bl_op(unsigned int op)
> > > @@ -287,6 +288,7 @@ __ftrace_make_nop(struct module *mod,
> > > static unsigned long find_ftrace_tramp(unsigned long ip)
> > > {
> > > int i;
> > > + unsigned int instr;
> > >
> > > /*
> > > * We have the compiler generated long_branch tramps at the end
> > > @@ -295,7 +297,8 @@ static unsigned long find_ftrace_tramp(unsigned long
> > > ip)
> > > for (i = NUM_FTRACE_TRAMPS - 1; i >= 0; i--)
> > > if (!ftrace_tramps[i])
> > > continue;
> > > - else if (create_branch((void *)ip, ftrace_tramps[i], 0))
> > > + else if (create_branch(&instr, (void *)ip,
> > > + ftrace_tramps[i], 0) == 0)
> > > return ftrace_tramps[i];
> > >
> > > return 0;
> > > @@ -324,6 +327,7 @@ static int setup_mcount_compiler_tramp(unsigned long
> > > tramp)
> > > {
> > > int i, op;
> > > unsigned long ptr;
> > > + unsigned int instr;
> > > static unsigned long ftrace_plt_tramps[NUM_FTRACE_TRAMPS];
> > >
> > > /* Is this a known long jump tramp? */
> > > @@ -366,7 +370,7 @@ static int setup_mcount_compiler_tramp(unsigned long
> > > tramp)
> > > #else
> > > ptr = ppc_global_function_entry((void *)ftrace_caller);
> > > #endif
> > > - if (!create_branch((void *)tramp, ptr, 0)) {
> > > + if (create_branch(&instr, (void *)tramp, ptr, 0)) {
> > > pr_debug("%ps is not reachable from existing mcount
> > > tramp\n",
> > > (void *)ptr);
> > > return -1;
> > > @@ -511,6 +515,7 @@ static int
> > > __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
> > > {
> > > unsigned int op[2];
> > > + unsigned int instr;
> > > void *ip = (void *)rec->ip;
> > > unsigned long entry, ptr, tramp;
> > > struct module *mod = rec->arch.mod;
> > > @@ -557,7 +562,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned
> > > long
> > > addr)
> > > }
> > >
> > > /* Ensure branch is within 24 bits */
> > > - if (!create_branch(ip, tramp, BRANCH_SET_LINK)) {
> > > + if (create_branch(&instr, ip, tramp, BRANCH_SET_LINK)) {
> > > pr_err("Branch out of range\n");
> > > return -EINVAL;
> > > }
> > > @@ -574,6 +579,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned
> > > long
> > > addr)
> > > static int
> > > __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
> > > {
> > > + int err;
> > > unsigned int op;
> > > unsigned long ip = rec->ip;
> > >
> > > @@ -594,9 +600,9 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned
> > > long
> > > addr)
> > > }
> > >
> > > /* create the branch to the trampoline */
> > > - op = create_branch((unsigned int *)ip,
> > > - rec->arch.mod->arch.tramp, BRANCH_SET_LINK);
> > > - if (!op) {
> > > + err = create_branch(&op, (unsigned int *)ip,
> > > + rec->arch.mod->arch.tramp, BRANCH_SET_LINK);
> > > + if (!err) {
> >
> > should this be `if (err)` as per your new changes ?
> You are right.
> >
> > > pr_err("REL24 out of range!\n");
> > > return -EINVAL;
> > > }
> > > @@ -776,7 +782,7 @@ __ftrace_modify_call(struct dyn_ftrace *rec, unsigned
> > > long old_addr,
> > > }
> > >
> > > /* Ensure branch is within 24 bits */
> > > - if (!create_branch((unsigned int *)ip, tramp, BRANCH_SET_LINK)) {
> > > + if (create_branch(&op, (unsigned int *)ip, tramp, BRANCH_SET_LINK))
> > > {
> > > pr_err("Branch out of range\n");
> > > return -EINVAL;
> > > }
> > > diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-
> > > patching.c
> > > index 3345f039a876..4af564aeedf1 100644
> > > --- a/arch/powerpc/lib/code-patching.c
> > > +++ b/arch/powerpc/lib/code-patching.c
> > > @@ -196,7 +196,10 @@ NOKPROBE_SYMBOL(patch_instruction);
> > >
> > > int patch_branch(unsigned int *addr, unsigned long target, int flags)
> > > {
> > > - return patch_instruction(addr, create_branch(addr, target, flags));
> > > + unsigned int instr;
> > > +
> > > + create_branch(&instr, addr, target, flags);
> > > + return patch_instruction(addr, instr);
> > > }
> > >
> > > bool is_offset_in_branch_range(long offset)
> > > @@ -243,30 +246,30 @@ bool is_conditional_branch(unsigned int instr)
> > > }
> > > NOKPROBE_SYMBOL(is_conditional_branch);
> > >
> > > -unsigned int create_branch(const unsigned int *addr,
> > > - unsigned long target, int flags)
> > > +int create_branch(unsigned int *instr,
> > > + const unsigned int *addr,
> > > + unsigned long target, int flags)
> > > {
> > > - unsigned int instruction;
> > > long offset;
> > >
> > > + *instr = 0;
> > > offset = target;
> > > if (! (flags & BRANCH_ABSOLUTE))
> > > offset = offset - (unsigned long)addr;
> > >
> > > /* Check we can represent the target in the instruction format */
> > > if (!is_offset_in_branch_range(offset))
> > > - return 0;
> > > + return 1;
> > >
> > > /* Mask out the flags and target, so they don't step on each other.
> > > */
> > > - instruction = 0x48000000 | (flags & 0x3) | (offset & 0x03FFFFFC);
> > > + *instr = 0x48000000 | (flags & 0x3) | (offset & 0x03FFFFFC);
> > >
> > > - return instruction;
> > > + return 0;
> > > }
> > >
> > > -unsigned int create_cond_branch(const unsigned int *addr,
> > > - unsigned long target, int flags)
> > > +int create_cond_branch(unsigned int *instr, const unsigned int *addr,
> > > + unsigned long target, int flags)
> > > {
> > > - unsigned int instruction;
> > > long offset;
> > >
> > > offset = target;
> > > @@ -275,12 +278,12 @@ unsigned int create_cond_branch(const unsigned int
> > > *addr,
> > >
> > > /* Check we can represent the target in the instruction format */
> > > if (offset < -0x8000 || offset > 0x7FFF || offset & 0x3)
> > > - return 0;
> > > + return 1;
> > >
> > > /* Mask out the flags and target, so they don't step on each other.
> > > */
> > > - instruction = 0x40000000 | (flags & 0x3FF0003) | (offset & 0xFFFC);
> > > + *instr = 0x40000000 | (flags & 0x3FF0003) | (offset & 0xFFFC);
> > >
> > > - return instruction;
> > > + return 0;
> > > }
> > >
> > > static unsigned int branch_opcode(unsigned int instr)
> > > @@ -361,18 +364,18 @@ int instr_is_branch_to_addr(const unsigned int
> > > *instr,
> > > unsigned long addr)
> > > return 0;
> > > }
> > >
> > > -unsigned int translate_branch(const unsigned int *dest, const unsigned
> > > int
> > > *src)
> > > +int translate_branch(unsigned int *instr, const unsigned int *dest,
> > > const
> > > unsigned int *src)
> > > {
> > > unsigned long target;
> > >
> > > target = branch_target(src);
> > >
> > > if (instr_is_branch_iform(*src))
> > > - return create_branch(dest, target, *src);
> > > + return create_branch(instr, dest, target, *src);
> > > else if (instr_is_branch_bform(*src))
> > > - return create_cond_branch(dest, target, *src);
> > > + return create_cond_branch(instr, dest, target, *src);
> > >
> > > - return 0;
> > > + return 1;
> > > }
> > >
> > > #ifdef CONFIG_PPC_BOOK3E_64
> > > @@ -403,6 +406,7 @@ static void __init test_trampoline(void)
> > >
> > > static void __init test_branch_iform(void)
> > > {
> > > + int err;
> > > unsigned int instr;
> > > unsigned long addr;
> > >
> > > @@ -443,35 +447,35 @@ static void __init test_branch_iform(void)
> > > check(instr_is_branch_to_addr(&instr, addr - 0x2000000));
> > >
> > > /* Branch to self, with link */
> > > - instr = create_branch(&instr, addr, BRANCH_SET_LINK);
> > > + err = create_branch(&instr, &instr, addr, BRANCH_SET_LINK);
> > > check(instr_is_branch_to_addr(&instr, addr));
> > >
> > > /* Branch to self - 0x100, with link */
> > > - instr = create_branch(&instr, addr - 0x100, BRANCH_SET_LINK);
> > > + err = create_branch(&instr, &instr, addr - 0x100, BRANCH_SET_LINK);
> > > check(instr_is_branch_to_addr(&instr, addr - 0x100));
> > >
> > > /* Branch to self + 0x100, no link */
> > > - instr = create_branch(&instr, addr + 0x100, 0);
> > > + err = create_branch(&instr, &instr, addr + 0x100, 0);
> > > check(instr_is_branch_to_addr(&instr, addr + 0x100));
> > >
> > > /* Maximum relative negative offset, - 32 MB */
> > > - instr = create_branch(&instr, addr - 0x2000000, BRANCH_SET_LINK);
> > > + err = create_branch(&instr, &instr, addr - 0x2000000,
> > > BRANCH_SET_LINK);
> > > check(instr_is_branch_to_addr(&instr, addr - 0x2000000));
> > >
> > > /* Out of range relative negative offset, - 32 MB + 4*/
> > > - instr = create_branch(&instr, addr - 0x2000004, BRANCH_SET_LINK);
> > > - check(instr == 0);
> > > + err = create_branch(&instr, &instr, addr - 0x2000004,
> > > BRANCH_SET_LINK);
> > > + check(err);
> > >
> > > /* Out of range relative positive offset, + 32 MB */
> > > - instr = create_branch(&instr, addr + 0x2000000, BRANCH_SET_LINK);
> > > - check(instr == 0);
> > > + err = create_branch(&instr, &instr, addr + 0x2000000,
> > > BRANCH_SET_LINK);
> > > + check(err);
> > >
> > > /* Unaligned target */
> > > - instr = create_branch(&instr, addr + 3, BRANCH_SET_LINK);
> > > - check(instr == 0);
> > > + err = create_branch(&instr, &instr, addr + 3, BRANCH_SET_LINK);
> > > + check(err);
> > >
> > > /* Check flags are masked correctly */
> > > - instr = create_branch(&instr, addr, 0xFFFFFFFC);
> > > + err = create_branch(&instr, &instr, addr, 0xFFFFFFFC);
> > > check(instr_is_branch_to_addr(&instr, addr));
> > > check(instr == 0x48000000);
> > > }
> > > @@ -480,16 +484,19 @@ static void __init test_create_function_call(void)
> > > {
> > > unsigned int *iptr;
> > > unsigned long dest;
> > > + unsigned int instr;
> > >
> > > /* Check we can create a function call */
> > > iptr = (unsigned int *)ppc_function_entry(test_trampoline);
> > > dest = ppc_function_entry(test_create_function_call);
> > > - patch_instruction(iptr, create_branch(iptr, dest,
> > > BRANCH_SET_LINK));
> > > + create_branch(&instr, iptr, dest, BRANCH_SET_LINK);
> > > + patch_instruction(iptr, instr);
> >
> > Are we sure that `create_branch()` would always succeed here to do
> > patch_instruction() ?
> >
> >
> > > check(instr_is_branch_to_addr(iptr, dest));
> > > }
> > >
> > > static void __init test_branch_bform(void)
> > > {
> > > + int err;
> > > unsigned long addr;
> > > unsigned int *iptr, instr, flags;
> > >
> > > @@ -525,35 +532,35 @@ static void __init test_branch_bform(void)
> > > flags = 0x3ff000 | BRANCH_SET_LINK;
> > >
> > > /* Branch to self */
> > > - instr = create_cond_branch(iptr, addr, flags);
> > > + err = create_cond_branch(&instr, iptr, addr, flags);
> > > check(instr_is_branch_to_addr(&instr, addr));
> > >
> > > /* Branch to self - 0x100 */
> > > - instr = create_cond_branch(iptr, addr - 0x100, flags);
> > > + err = create_cond_branch(&instr, iptr, addr - 0x100, flags);
> > > check(instr_is_branch_to_addr(&instr, addr - 0x100));
> > >
> > > /* Branch to self + 0x100 */
> > > - instr = create_cond_branch(iptr, addr + 0x100, flags);
> > > + err = create_cond_branch(&instr, iptr, addr + 0x100, flags);
> > > check(instr_is_branch_to_addr(&instr, addr + 0x100));
> > >
> > > /* Maximum relative negative offset, - 32 KB */
> > > - instr = create_cond_branch(iptr, addr - 0x8000, flags);
> > > + err = create_cond_branch(&instr, iptr, addr - 0x8000, flags);
> > > check(instr_is_branch_to_addr(&instr, addr - 0x8000));
> > >
> > > /* Out of range relative negative offset, - 32 KB + 4*/
> > > - instr = create_cond_branch(iptr, addr - 0x8004, flags);
> > > - check(instr == 0);
> > > + err = create_cond_branch(&instr, iptr, addr - 0x8004, flags);
> > > + check(err);
> > >
> > > /* Out of range relative positive offset, + 32 KB */
> > > - instr = create_cond_branch(iptr, addr + 0x8000, flags);
> > > - check(instr == 0);
> > > + err = create_cond_branch(&instr, iptr, addr + 0x8000, flags);
> > > + check(err);
> > >
> > > /* Unaligned target */
> > > - instr = create_cond_branch(iptr, addr + 3, flags);
> > > - check(instr == 0);
> > > + err = create_cond_branch(&instr, iptr, addr + 3, flags);
> > > + check(err);
> > >
> > > /* Check flags are masked correctly */
> > > - instr = create_cond_branch(iptr, addr, 0xFFFFFFFC);
> > > + err = create_cond_branch(&instr, iptr, addr, 0xFFFFFFFC);
> > > check(instr_is_branch_to_addr(&instr, addr));
> > > check(instr == 0x43FF0000);
> > > }
> > > @@ -562,6 +569,7 @@ static void __init test_translate_branch(void)
> > > {
> > > unsigned long addr;
> > > unsigned int *p, *q;
> > > + unsigned int instr;
> > > void *buf;
> > >
> > > buf = vmalloc(PAGE_ALIGN(0x2000000 + 1));
> > > @@ -575,7 +583,8 @@ static void __init test_translate_branch(void)
> > > patch_branch(p, addr, 0);
> > > check(instr_is_branch_to_addr(p, addr));
> > > q = p + 1;
> > > - patch_instruction(q, translate_branch(q, p));
> > > + translate_branch(&instr, q, p);
> > > + patch_instruction(q, instr);
> >
> > same here and elsewhere in similar such places where we do
> > patch_instruction()
> I figured that if the existing code was not checking the return then
> we probably did not need to add a check.
yes existing code does not checking, I just wanted to ensure we do right
thing. Thanks!
-- Bala
> > -- Bala
> >
> > > check(instr_is_branch_to_addr(q, addr));
> > >
> > > /* Maximum negative case, move b . to addr + 32 MB */
> > > @@ -583,7 +592,8 @@ static void __init test_translate_branch(void)
> > > addr = (unsigned long)p;
> > > patch_branch(p, addr, 0);
> > > q = buf + 0x2000000;
> > > - patch_instruction(q, translate_branch(q, p));
> > > + translate_branch(&instr, q, p);
> > > + patch_instruction(q, instr);
> > > check(instr_is_branch_to_addr(p, addr));
> > > check(instr_is_branch_to_addr(q, addr));
> > > check(*q == 0x4a000000);
> > > @@ -593,7 +603,8 @@ static void __init test_translate_branch(void)
> > > addr = (unsigned long)p;
> > > patch_branch(p, addr, 0);
> > > q = buf + 4;
> > > - patch_instruction(q, translate_branch(q, p));
> > > + translate_branch(&instr, q, p);
> > > + patch_instruction(q, instr);
> > > check(instr_is_branch_to_addr(p, addr));
> > > check(instr_is_branch_to_addr(q, addr));
> > > check(*q == 0x49fffffc);
> > > @@ -603,7 +614,8 @@ static void __init test_translate_branch(void)
> > > addr = 0x1000000 + (unsigned long)buf;
> > > patch_branch(p, addr, BRANCH_SET_LINK);
> > > q = buf + 0x1400000;
> > > - patch_instruction(q, translate_branch(q, p));
> > > + translate_branch(&instr, q, p);
> > > + patch_instruction(q, instr);
> > > check(instr_is_branch_to_addr(p, addr));
> > > check(instr_is_branch_to_addr(q, addr));
> > >
> > > @@ -612,7 +624,8 @@ static void __init test_translate_branch(void)
> > > addr = 0x2000000 + (unsigned long)buf;
> > > patch_branch(p, addr, 0);
> > > q = buf + 4;
> > > - patch_instruction(q, translate_branch(q, p));
> > > + translate_branch(&instr, q, p);
> > > + patch_instruction(q, instr);
> > > check(instr_is_branch_to_addr(p, addr));
> > > check(instr_is_branch_to_addr(q, addr));
> > >
> > > @@ -622,18 +635,22 @@ static void __init test_translate_branch(void)
> > > /* Simple case, branch to self moved a little */
> > > p = buf;
> > > addr = (unsigned long)p;
> > > - patch_instruction(p, create_cond_branch(p, addr, 0));
> > > + create_cond_branch(&instr, p, addr, 0);
> > > + patch_instruction(p, instr);
> > > check(instr_is_branch_to_addr(p, addr));
> > > q = p + 1;
> > > - patch_instruction(q, translate_branch(q, p));
> > > + translate_branch(&instr, q, p);
> > > + patch_instruction(q, instr);
> > > check(instr_is_branch_to_addr(q, addr));
> > >
> > > /* Maximum negative case, move b . to addr + 32 KB */
> > > p = buf;
> > > addr = (unsigned long)p;
> > > - patch_instruction(p, create_cond_branch(p, addr, 0xFFFFFFFC));
> > > + create_cond_branch(&instr, p, addr, 0xFFFFFFFC);
> > > + patch_instruction(p, instr);
> > > q = buf + 0x8000;
> > > - patch_instruction(q, translate_branch(q, p));
> > > + translate_branch(&instr, q, p);
> > > + patch_instruction(q, instr);
> > > check(instr_is_branch_to_addr(p, addr));
> > > check(instr_is_branch_to_addr(q, addr));
> > > check(*q == 0x43ff8000);
> > > @@ -641,9 +658,11 @@ static void __init test_translate_branch(void)
> > > /* Maximum positive case, move x to x - 32 KB + 4 */
> > > p = buf + 0x8000;
> > > addr = (unsigned long)p;
> > > - patch_instruction(p, create_cond_branch(p, addr, 0xFFFFFFFC));
> > > + create_cond_branch(&instr, p, addr, 0xFFFFFFFC);
> > > + patch_instruction(p, instr);
> > > q = buf + 4;
> > > - patch_instruction(q, translate_branch(q, p));
> > > + translate_branch(&instr, q, p);
> > > + patch_instruction(q, instr);
> > > check(instr_is_branch_to_addr(p, addr));
> > > check(instr_is_branch_to_addr(q, addr));
> > > check(*q == 0x43ff7ffc);
> > > @@ -651,18 +670,22 @@ static void __init test_translate_branch(void)
> > > /* Jump to x + 12 KB moved to x + 20 KB */
> > > p = buf;
> > > addr = 0x3000 + (unsigned long)buf;
> > > - patch_instruction(p, create_cond_branch(p, addr, BRANCH_SET_LINK));
> > > + create_cond_branch(&instr, p, addr, BRANCH_SET_LINK);
> > > + patch_instruction(p, instr);
> > > q = buf + 0x5000;
> > > - patch_instruction(q, translate_branch(q, p));
> > > + translate_branch(&instr, q, p);
> > > + patch_instruction(q, instr);
> > > check(instr_is_branch_to_addr(p, addr));
> > > check(instr_is_branch_to_addr(q, addr));
> > >
> > > /* Jump to x + 8 KB moved to x - 8 KB + 4 */
> > > p = buf + 0x2000;
> > > addr = 0x4000 + (unsigned long)buf;
> > > - patch_instruction(p, create_cond_branch(p, addr, 0));
> > > + create_cond_branch(&instr, p, addr, 0);
> > > + patch_instruction(p, instr);
> > > q = buf + 4;
> > > - patch_instruction(q, translate_branch(q, p));
> > > + translate_branch(&instr, q, p);
> > > + patch_instruction(q, instr);
> > > check(instr_is_branch_to_addr(p, addr));
> > > check(instr_is_branch_to_addr(q, addr));
> > >
> > > diff --git a/arch/powerpc/lib/feature-fixups.c
> > > b/arch/powerpc/lib/feature-
> > > fixups.c
> > > index 4ba634b89ce5..b129d7b4e7dd 100644
> > > --- a/arch/powerpc/lib/feature-fixups.c
> > > +++ b/arch/powerpc/lib/feature-fixups.c
> > > @@ -44,6 +44,7 @@ static unsigned int *calc_addr(struct fixup_entry
> > > *fcur,
> > > long offset)
> > > static int patch_alt_instruction(unsigned int *src, unsigned int *dest,
> > > unsigned int *alt_start, unsigned int
> > > *alt_end)
> > > {
> > > + int err;
> > > unsigned int instr;
> > >
> > > instr = *src;
> > > @@ -53,8 +54,8 @@ static int patch_alt_instruction(unsigned int *src,
> > > unsigned int *dest,
> > >
> > > /* Branch within the section doesn't need translating */
> > > if (target < alt_start || target > alt_end) {
> > > - instr = translate_branch(dest, src);
> > > - if (!instr)
> > > + err = translate_branch(&instr, dest, src);
> > > + if (err)
> > > return 1;
> > > }
> > > }
^ permalink raw reply
* Re: [PATCH v5 02/21] powerpc/xmon: Move out-of-line instructions to text section
From: Balamuruhan S @ 2020-04-07 6:45 UTC (permalink / raw)
To: Jordan Niethe, linuxppc-dev; +Cc: alistair, dja, npiggin
In-Reply-To: <20200406080936.7180-3-jniethe5@gmail.com>
On Mon, 2020-04-06 at 18:09 +1000, Jordan Niethe wrote:
> To execute an instruction out of line after a breakpoint, the NIP is set
> to the address of struct bpt::instr. Here a copy of the instruction that
> was replaced with a breakpoint is kept, along with a trap so normal flow
> can be resumed after XOLing. The struct bpt's are located within the
> data section. This is problematic as the data section may be marked as
> no execute.
>
> Instead of each struct bpt holding the instructions to be XOL'd, make a
> new array, bpt_table[], with enough space to hold instructions for the
> number of supported breakpoints. Place this array in the text section.
> Make struct bpt::instr a pointer to the instructions in bpt_table[]
> associated with that breakpoint. This association is a simple mapping:
> bpts[n] -> bpt_table[n * words per breakpoint]. Currently we only need
> the copied instruction followed by a trap, so 2 words per breakpoint.
we do 2 things here and we discussed to split it into 2 commits, may be
it is up to your thoughts and leave it to you. But otherwise rest looks
good to me.
-- Bala
>
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> ---
> v4: New to series
> v5: - Do not use __section(), use a .space directive in .S file
> - Simplify in_breakpoint_table() calculation
> - Define BPT_SIZE
> ---
> arch/powerpc/xmon/Makefile | 2 +-
> arch/powerpc/xmon/xmon.c | 23 +++++++++++++----------
> arch/powerpc/xmon/xmon_bpts.S | 8 ++++++++
> arch/powerpc/xmon/xmon_bpts.h | 8 ++++++++
> 4 files changed, 30 insertions(+), 11 deletions(-)
> create mode 100644 arch/powerpc/xmon/xmon_bpts.S
> create mode 100644 arch/powerpc/xmon/xmon_bpts.h
>
> diff --git a/arch/powerpc/xmon/Makefile b/arch/powerpc/xmon/Makefile
> index c3842dbeb1b7..515a13ea6f28 100644
> --- a/arch/powerpc/xmon/Makefile
> +++ b/arch/powerpc/xmon/Makefile
> @@ -21,7 +21,7 @@ endif
>
> ccflags-$(CONFIG_PPC64) := $(NO_MINIMAL_TOC)
>
> -obj-y += xmon.o nonstdio.o spr_access.o
> +obj-y += xmon.o nonstdio.o spr_access.o xmon_bpts.o
>
> ifdef CONFIG_XMON_DISASSEMBLY
> obj-y += ppc-dis.o ppc-opc.o
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index 02e3bd62cab4..049375206510 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -62,6 +62,7 @@
>
> #include "nonstdio.h"
> #include "dis-asm.h"
> +#include "xmon_bpts.h"
>
> #ifdef CONFIG_SMP
> static cpumask_t cpus_in_xmon = CPU_MASK_NONE;
> @@ -97,7 +98,7 @@ static long *xmon_fault_jmp[NR_CPUS];
> /* Breakpoint stuff */
> struct bpt {
> unsigned long address;
> - unsigned int instr[2];
> + unsigned int *instr;
> atomic_t ref_count;
> int enabled;
> unsigned long pad;
> @@ -108,7 +109,6 @@ struct bpt {
> #define BP_TRAP 2
> #define BP_DABR 4
>
> -#define NBPTS 256
> static struct bpt bpts[NBPTS];
> static struct bpt dabr;
> static struct bpt *iabr;
> @@ -116,6 +116,10 @@ static unsigned bpinstr = 0x7fe00008; /* trap */
>
> #define BP_NUM(bp) ((bp) - bpts + 1)
>
> +#define BPT_SIZE (sizeof(unsigned int) * 2)
> +#define BPT_WORDS (BPT_SIZE / sizeof(unsigned int))
> +extern unsigned int bpt_table[NBPTS * BPT_WORDS];
> +
> /* Prototypes */
> static int cmds(struct pt_regs *);
> static int mread(unsigned long, void *, int);
> @@ -853,15 +857,13 @@ static struct bpt *in_breakpoint_table(unsigned long
> nip, unsigned long *offp)
> {
> unsigned long off;
>
> - off = nip - (unsigned long) bpts;
> - if (off >= sizeof(bpts))
> + off = nip - (unsigned long) bpt_table;
> + if (off >= sizeof(bpt_table))
> return NULL;
> - off %= sizeof(struct bpt);
> - if (off != offsetof(struct bpt, instr[0])
> - && off != offsetof(struct bpt, instr[1]))
> + *offp = off % BPT_SIZE;
> + if (*offp != 0 && *offp != 4)
> return NULL;
> - *offp = off - offsetof(struct bpt, instr[0]);
> - return (struct bpt *) (nip - off);
> + return bpts + (off / BPT_SIZE);
> }
>
> static struct bpt *new_breakpoint(unsigned long a)
> @@ -876,7 +878,8 @@ static struct bpt *new_breakpoint(unsigned long a)
> for (bp = bpts; bp < &bpts[NBPTS]; ++bp) {
> if (!bp->enabled && atomic_read(&bp->ref_count) == 0) {
> bp->address = a;
> - patch_instruction(&bp->instr[1], bpinstr);
> + bp->instr = bpt_table + ((bp - bpts) * BPT_WORDS);
> + patch_instruction(bp->instr + 1, bpinstr);
> return bp;
> }
> }
> diff --git a/arch/powerpc/xmon/xmon_bpts.S b/arch/powerpc/xmon/xmon_bpts.S
> new file mode 100644
> index 000000000000..ebb2dbc70ca8
> --- /dev/null
> +++ b/arch/powerpc/xmon/xmon_bpts.S
> @@ -0,0 +1,8 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#include <asm/ppc_asm.h>
> +#include <asm/asm-compat.h>
> +#include "xmon_bpts.h"
> +
> +.global bpt_table
> +bpt_table:
> + .space NBPTS * 8
> diff --git a/arch/powerpc/xmon/xmon_bpts.h b/arch/powerpc/xmon/xmon_bpts.h
> new file mode 100644
> index 000000000000..840e70be7945
> --- /dev/null
> +++ b/arch/powerpc/xmon/xmon_bpts.h
> @@ -0,0 +1,8 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef XMON_BPTS_H
> +#define XMON_BPTS_H
> +
> +#define NBPTS 256
> +
> +#endif /* XMON_BPTS_H */
> +
^ permalink raw reply
* Re: [PATCH v5 04/21] powerpc: Use a macro for creating instructions from u32s
From: Balamuruhan S @ 2020-04-07 6:40 UTC (permalink / raw)
To: Jordan Niethe, linuxppc-dev; +Cc: alistair, npiggin, dja
In-Reply-To: <20200406080936.7180-5-jniethe5@gmail.com>
On Mon, 2020-04-06 at 18:09 +1000, Jordan Niethe wrote:
> In preparation for instructions having a more complex data type start
> using a macro, ppc_inst(), for making an instruction out of a u32. A
> macro is used so that instructions can be used as initializer elements.
> Currently this does nothing, but it will allow for creating a data type
> that can represent prefixed instructions.
>
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> ---
> v4: New to series
> v5: - Rename PPC_INST() -> ppc_inst().
> - Use on epapr_paravirt.c, kgdb.c
> ---
> arch/powerpc/include/asm/code-patching.h | 3 +-
> arch/powerpc/include/asm/inst.h | 11 +++++
> arch/powerpc/kernel/align.c | 1 +
> arch/powerpc/kernel/epapr_paravirt.c | 5 ++-
> arch/powerpc/kernel/hw_breakpoint.c | 3 +-
> arch/powerpc/kernel/jump_label.c | 3 +-
> arch/powerpc/kernel/kgdb.c | 5 ++-
> arch/powerpc/kernel/kprobes.c | 5 ++-
> arch/powerpc/kernel/module_64.c | 3 +-
> arch/powerpc/kernel/optprobes.c | 31 ++++++-------
> arch/powerpc/kernel/security.c | 9 ++--
> arch/powerpc/kernel/trace/ftrace.c | 25 ++++++-----
> arch/powerpc/kernel/uprobes.c | 1 +
> arch/powerpc/kvm/emulate_loadstore.c | 2 +-
> arch/powerpc/lib/code-patching.c | 57 ++++++++++++------------
> arch/powerpc/lib/feature-fixups.c | 39 ++++++++--------
> arch/powerpc/lib/test_emulate_step.c | 39 ++++++++--------
> arch/powerpc/xmon/xmon.c | 7 +--
> 18 files changed, 138 insertions(+), 111 deletions(-)
> create mode 100644 arch/powerpc/include/asm/inst.h
>
> diff --git a/arch/powerpc/include/asm/code-patching.h
> b/arch/powerpc/include/asm/code-patching.h
> index 351dda7215b6..48e021957ee5 100644
> --- a/arch/powerpc/include/asm/code-patching.h
> +++ b/arch/powerpc/include/asm/code-patching.h
> @@ -11,6 +11,7 @@
> #include <linux/string.h>
> #include <linux/kallsyms.h>
> #include <asm/asm-compat.h>
> +#include <asm/inst.h>
>
> /* Flags for create_branch:
> * "b" == create_branch(addr, target, 0);
> @@ -48,7 +49,7 @@ static inline int patch_branch_site(s32 *site, unsigned
> long target, int flags)
> static inline int modify_instruction(unsigned int *addr, unsigned int clr,
> unsigned int set)
> {
> - return patch_instruction(addr, (*addr & ~clr) | set);
> + return patch_instruction(addr, ppc_inst((*addr & ~clr) | set));
> }
>
> static inline int modify_instruction_site(s32 *site, unsigned int clr,
> unsigned int set)
> diff --git a/arch/powerpc/include/asm/inst.h
> b/arch/powerpc/include/asm/inst.h
> new file mode 100644
> index 000000000000..5298ba33b6e5
> --- /dev/null
> +++ b/arch/powerpc/include/asm/inst.h
> @@ -0,0 +1,11 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +#ifndef _ASM_INST_H
> +#define _ASM_INST_H
> +
> +/*
> + * Instruction data type for POWER
> + */
> +
> +#define ppc_inst(x) (x)
> +
> +#endif /* _ASM_INST_H */
> diff --git a/arch/powerpc/kernel/align.c b/arch/powerpc/kernel/align.c
> index 92045ed64976..86e9bf62f18c 100644
> --- a/arch/powerpc/kernel/align.c
> +++ b/arch/powerpc/kernel/align.c
> @@ -24,6 +24,7 @@
> #include <asm/disassemble.h>
> #include <asm/cpu_has_feature.h>
> #include <asm/sstep.h>
> +#include <asm/inst.h>
>
> struct aligninfo {
> unsigned char len;
> diff --git a/arch/powerpc/kernel/epapr_paravirt.c
> b/arch/powerpc/kernel/epapr_paravirt.c
> index 9d32158ce36f..c53e863fb484 100644
> --- a/arch/powerpc/kernel/epapr_paravirt.c
> +++ b/arch/powerpc/kernel/epapr_paravirt.c
> @@ -11,6 +11,7 @@
> #include <asm/cacheflush.h>
> #include <asm/code-patching.h>
> #include <asm/machdep.h>
> +#include <asm/inst.h>
>
> #if !defined(CONFIG_64BIT) || defined(CONFIG_PPC_BOOK3E_64)
> extern void epapr_ev_idle(void);
> @@ -37,9 +38,9 @@ static int __init early_init_dt_scan_epapr(unsigned long
> node,
>
> for (i = 0; i < (len / 4); i++) {
> u32 inst = be32_to_cpu(insts[i]);
> - patch_instruction(epapr_hypercall_start + i, inst);
> + patch_instruction(epapr_hypercall_start + i, ppc_inst(inst));
> #if !defined(CONFIG_64BIT) || defined(CONFIG_PPC_BOOK3E_64)
> - patch_instruction(epapr_ev_idle_start + i, inst);
> + patch_instruction(epapr_ev_idle_start + i, ppc_inst(inst));
can we do it once while assigning inst ?
u32 inst = ppc_inst(be32_to_cpu(insts[i]));
-- Bala
> #endif
> }
>
> diff --git a/arch/powerpc/kernel/hw_breakpoint.c
> b/arch/powerpc/kernel/hw_breakpoint.c
> index 2462cd7c565c..79f51f182a83 100644
> --- a/arch/powerpc/kernel/hw_breakpoint.c
> +++ b/arch/powerpc/kernel/hw_breakpoint.c
> @@ -24,6 +24,7 @@
> #include <asm/debug.h>
> #include <asm/debugfs.h>
> #include <asm/hvcall.h>
> +#include <asm/inst.h>
> #include <linux/uaccess.h>
>
> /*
> @@ -243,7 +244,7 @@ dar_range_overlaps(unsigned long dar, int size, struct
> arch_hw_breakpoint *info)
> static bool stepping_handler(struct pt_regs *regs, struct perf_event *bp,
> struct arch_hw_breakpoint *info)
> {
> - unsigned int instr = 0;
> + unsigned int instr = ppc_inst(0);
> int ret, type, size;
> struct instruction_op op;
> unsigned long addr = info->address;
> diff --git a/arch/powerpc/kernel/jump_label.c
> b/arch/powerpc/kernel/jump_label.c
> index ca37702bde97..daa4afce7ec8 100644
> --- a/arch/powerpc/kernel/jump_label.c
> +++ b/arch/powerpc/kernel/jump_label.c
> @@ -6,6 +6,7 @@
> #include <linux/kernel.h>
> #include <linux/jump_label.h>
> #include <asm/code-patching.h>
> +#include <asm/inst.h>
>
> void arch_jump_label_transform(struct jump_entry *entry,
> enum jump_label_type type)
> @@ -15,5 +16,5 @@ void arch_jump_label_transform(struct jump_entry *entry,
> if (type == JUMP_LABEL_JMP)
> patch_branch(addr, entry->target, 0);
> else
> - patch_instruction(addr, PPC_INST_NOP);
> + patch_instruction(addr, ppc_inst(PPC_INST_NOP));
> }
> diff --git a/arch/powerpc/kernel/kgdb.c b/arch/powerpc/kernel/kgdb.c
> index 7dd55eb1259d..a6b38a19133f 100644
> --- a/arch/powerpc/kernel/kgdb.c
> +++ b/arch/powerpc/kernel/kgdb.c
> @@ -26,6 +26,7 @@
> #include <asm/debug.h>
> #include <asm/code-patching.h>
> #include <linux/slab.h>
> +#include <asm/inst.h>
>
> /*
> * This table contains the mapping between PowerPC hardware trap types, and
> @@ -424,7 +425,7 @@ int kgdb_arch_set_breakpoint(struct kgdb_bkpt *bpt)
> if (err)
> return err;
>
> - err = patch_instruction(addr, BREAK_INSTR);
> + err = patch_instruction(addr, ppc_inst(BREAK_INSTR));
> if (err)
> return -EFAULT;
>
> @@ -439,7 +440,7 @@ int kgdb_arch_remove_breakpoint(struct kgdb_bkpt *bpt)
> unsigned int instr = *(unsigned int *)bpt->saved_instr;
> unsigned int *addr = (unsigned int *)bpt->bpt_addr;
>
> - err = patch_instruction(addr, instr);
> + err = patch_instruction(addr, ppc_inst(instr));
> if (err)
> return -EFAULT;
>
> diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
> index 2d27ec4feee4..a1a3686f41c6 100644
> --- a/arch/powerpc/kernel/kprobes.c
> +++ b/arch/powerpc/kernel/kprobes.c
> @@ -23,6 +23,7 @@
> #include <asm/cacheflush.h>
> #include <asm/sstep.h>
> #include <asm/sections.h>
> +#include <asm/inst.h>
> #include <linux/uaccess.h>
>
> DEFINE_PER_CPU(struct kprobe *, current_kprobe) = NULL;
> @@ -138,13 +139,13 @@ NOKPROBE_SYMBOL(arch_prepare_kprobe);
>
> void arch_arm_kprobe(struct kprobe *p)
> {
> - patch_instruction(p->addr, BREAKPOINT_INSTRUCTION);
> + patch_instruction(p->addr, ppc_inst(BREAKPOINT_INSTRUCTION));
> }
> NOKPROBE_SYMBOL(arch_arm_kprobe);
>
> void arch_disarm_kprobe(struct kprobe *p)
> {
> - patch_instruction(p->addr, p->opcode);
> + patch_instruction(p->addr, ppc_inst(p->opcode));
> }
> NOKPROBE_SYMBOL(arch_disarm_kprobe);
>
> diff --git a/arch/powerpc/kernel/module_64.c
> b/arch/powerpc/kernel/module_64.c
> index 007606a48fd9..7fd6b29edcb2 100644
> --- a/arch/powerpc/kernel/module_64.c
> +++ b/arch/powerpc/kernel/module_64.c
> @@ -20,6 +20,7 @@
> #include <linux/sort.h>
> #include <asm/setup.h>
> #include <asm/sections.h>
> +#include <asm/inst.h>
>
> /* FIXME: We don't do .init separately. To do this, we'd need to have
> a separate r2 value in the init and core section, and stub between
> @@ -506,7 +507,7 @@ static int restore_r2(const char *name, u32 *instruction,
> struct module *me)
> * "link" branches and they don't return, so they don't need the r2
> * restore afterwards.
> */
> - if (!instr_is_relative_link_branch(*prev_insn))
> + if (!instr_is_relative_link_branch(ppc_inst(*prev_insn)))
> return 1;
>
> if (*instruction != PPC_INST_NOP) {
> diff --git a/arch/powerpc/kernel/optprobes.c
> b/arch/powerpc/kernel/optprobes.c
> index 445b3dad82dc..3b33ebf18859 100644
> --- a/arch/powerpc/kernel/optprobes.c
> +++ b/arch/powerpc/kernel/optprobes.c
> @@ -16,6 +16,7 @@
> #include <asm/code-patching.h>
> #include <asm/sstep.h>
> #include <asm/ppc-opcode.h>
> +#include <asm/inst.h>
>
> #define TMPL_CALL_HDLR_IDX \
> (optprobe_template_call_handler - optprobe_template_entry)
> @@ -147,13 +148,13 @@ void arch_remove_optimized_kprobe(struct
> optimized_kprobe *op)
> void patch_imm32_load_insns(unsigned int val, kprobe_opcode_t *addr)
> {
> /* addis r4,0,(insn)@h */
> - patch_instruction(addr, PPC_INST_ADDIS | ___PPC_RT(4) |
> - ((val >> 16) & 0xffff));
> + patch_instruction(addr, ppc_inst(PPC_INST_ADDIS | ___PPC_RT(4) |
> + ((val >> 16) & 0xffff)));
> addr++;
>
> /* ori r4,r4,(insn)@l */
> - patch_instruction(addr, PPC_INST_ORI | ___PPC_RA(4) |
> - ___PPC_RS(4) | (val & 0xffff));
> + patch_instruction(addr, ppc_inst(PPC_INST_ORI | ___PPC_RA(4) |
> + ___PPC_RS(4) | (val & 0xffff)));
> }
>
> /*
> @@ -163,28 +164,28 @@ void patch_imm32_load_insns(unsigned int val,
> kprobe_opcode_t *addr)
> void patch_imm64_load_insns(unsigned long val, kprobe_opcode_t *addr)
> {
> /* lis r3,(op)@highest */
> - patch_instruction(addr, PPC_INST_ADDIS | ___PPC_RT(3) |
> - ((val >> 48) & 0xffff));
> + patch_instruction(addr, ppc_inst(PPC_INST_ADDIS | ___PPC_RT(3) |
> + ((val >> 48) & 0xffff)));
> addr++;
>
> /* ori r3,r3,(op)@higher */
> - patch_instruction(addr, PPC_INST_ORI | ___PPC_RA(3) |
> - ___PPC_RS(3) | ((val >> 32) & 0xffff));
> + patch_instruction(addr, ppc_inst(PPC_INST_ORI | ___PPC_RA(3) |
> + ___PPC_RS(3) | ((val >> 32) & 0xffff)));
> addr++;
>
> /* rldicr r3,r3,32,31 */
> - patch_instruction(addr, PPC_INST_RLDICR | ___PPC_RA(3) |
> - ___PPC_RS(3) | __PPC_SH64(32) | __PPC_ME64(31));
> + patch_instruction(addr, ppc_inst(PPC_INST_RLDICR | ___PPC_RA(3) |
> + ___PPC_RS(3) | __PPC_SH64(32) | __PPC_ME64(31)));
> addr++;
>
> /* oris r3,r3,(op)@h */
> - patch_instruction(addr, PPC_INST_ORIS | ___PPC_RA(3) |
> - ___PPC_RS(3) | ((val >> 16) & 0xffff));
> + patch_instruction(addr, ppc_inst(PPC_INST_ORIS | ___PPC_RA(3) |
> + ___PPC_RS(3) | ((val >> 16) & 0xffff)));
> addr++;
>
> /* ori r3,r3,(op)@l */
> - patch_instruction(addr, PPC_INST_ORI | ___PPC_RA(3) |
> - ___PPC_RS(3) | (val & 0xffff));
> + patch_instruction(addr, ppc_inst(PPC_INST_ORI | ___PPC_RA(3) |
> + ___PPC_RS(3) | (val & 0xffff)));
> }
>
> int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe
> *p)
> @@ -230,7 +231,7 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe
> *op, struct kprobe *p)
> size = (TMPL_END_IDX * sizeof(kprobe_opcode_t)) / sizeof(int);
> pr_devel("Copying template to %p, size %lu\n", buff, size);
> for (i = 0; i < size; i++) {
> - rc = patch_instruction(buff + i, *(optprobe_template_entry +
> i));
> + rc = patch_instruction(buff + i,
> ppc_inst(*(optprobe_template_entry + i)));
> if (rc < 0)
> goto error;
> }
> diff --git a/arch/powerpc/kernel/security.c b/arch/powerpc/kernel/security.c
> index bd70f5be1c27..81a288b1a603 100644
> --- a/arch/powerpc/kernel/security.c
> +++ b/arch/powerpc/kernel/security.c
> @@ -14,6 +14,7 @@
> #include <asm/debugfs.h>
> #include <asm/security_features.h>
> #include <asm/setup.h>
> +#include <asm/inst.h>
>
>
> u64 powerpc_security_features __read_mostly = SEC_FTR_DEFAULT;
> @@ -403,9 +404,9 @@ static void toggle_count_cache_flush(bool enable)
> enable = false;
>
> if (!enable) {
> - patch_instruction_site(&patch__call_flush_count_cache,
> PPC_INST_NOP);
> + patch_instruction_site(&patch__call_flush_count_cache,
> ppc_inst(PPC_INST_NOP));
> #ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
> - patch_instruction_site(&patch__call_kvm_flush_link_stack,
> PPC_INST_NOP);
> + patch_instruction_site(&patch__call_kvm_flush_link_stack,
> ppc_inst(PPC_INST_NOP));
> #endif
> pr_info("link-stack-flush: software flush disabled.\n");
> link_stack_flush_enabled = false;
> @@ -428,7 +429,7 @@ static void toggle_count_cache_flush(bool enable)
>
> // If we just need to flush the link stack, patch an early return
> if (!security_ftr_enabled(SEC_FTR_FLUSH_COUNT_CACHE)) {
> - patch_instruction_site(&patch__flush_link_stack_return,
> PPC_INST_BLR);
> + patch_instruction_site(&patch__flush_link_stack_return,
> ppc_inst(PPC_INST_BLR));
> no_count_cache_flush();
> return;
> }
> @@ -439,7 +440,7 @@ static void toggle_count_cache_flush(bool enable)
> return;
> }
>
> - patch_instruction_site(&patch__flush_count_cache_return, PPC_INST_BLR);
> + patch_instruction_site(&patch__flush_count_cache_return,
> ppc_inst(PPC_INST_BLR));
> count_cache_flush_type = COUNT_CACHE_FLUSH_HW;
> pr_info("count-cache-flush: hardware assisted flush sequence
> enabled\n");
> }
> diff --git a/arch/powerpc/kernel/trace/ftrace.c
> b/arch/powerpc/kernel/trace/ftrace.c
> index 4dcb379133f8..47233e671c18 100644
> --- a/arch/powerpc/kernel/trace/ftrace.c
> +++ b/arch/powerpc/kernel/trace/ftrace.c
> @@ -27,6 +27,7 @@
> #include <asm/code-patching.h>
> #include <asm/ftrace.h>
> #include <asm/syscall.h>
> +#include <asm/inst.h>
>
>
> #ifdef CONFIG_DYNAMIC_FTRACE
> @@ -161,7 +162,7 @@ __ftrace_make_nop(struct module *mod,
>
> #ifdef CONFIG_MPROFILE_KERNEL
> /* When using -mkernel_profile there is no load to jump over */
> - pop = PPC_INST_NOP;
> + pop = ppc_inst(PPC_INST_NOP);
>
> if (probe_kernel_read(&op, (void *)(ip - 4), 4)) {
> pr_err("Fetching instruction at %lx failed.\n", ip - 4);
> @@ -169,7 +170,7 @@ __ftrace_make_nop(struct module *mod,
> }
>
> /* We expect either a mflr r0, or a std r0, LRSAVE(r1) */
> - if (op != PPC_INST_MFLR && op != PPC_INST_STD_LR) {
> + if (op != ppc_inst(PPC_INST_MFLR) && op != ppc_inst(PPC_INST_STD_LR)) {
> pr_err("Unexpected instruction %08x around bl _mcount\n", op);
> return -EINVAL;
> }
> @@ -188,7 +189,7 @@ __ftrace_make_nop(struct module *mod,
> * Use a b +8 to jump over the load.
> */
>
> - pop = PPC_INST_BRANCH | 8; /* b +8 */
> + pop = ppc_inst(PPC_INST_BRANCH | 8); /* b +8 */
>
> /*
> * Check what is in the next instruction. We can see ld r2,40(r1), but
> @@ -199,7 +200,7 @@ __ftrace_make_nop(struct module *mod,
> return -EFAULT;
> }
>
> - if (op != PPC_INST_LD_TOC) {
> + if (op != ppc_inst(PPC_INST_LD_TOC)) {
> pr_err("Expected %08x found %08x\n", PPC_INST_LD_TOC, op);
> return -EINVAL;
> }
> @@ -275,7 +276,7 @@ __ftrace_make_nop(struct module *mod,
> return -EINVAL;
> }
>
> - op = PPC_INST_NOP;
> + op = ppc_inst(PPC_INST_NOP);
>
> if (patch_instruction((unsigned int *)ip, op))
> return -EPERM;
> @@ -420,7 +421,7 @@ static int __ftrace_make_nop_kernel(struct dyn_ftrace
> *rec, unsigned long addr)
> }
> }
>
> - if (patch_instruction((unsigned int *)ip, PPC_INST_NOP)) {
> + if (patch_instruction((unsigned int *)ip, ppc_inst(PPC_INST_NOP))) {
> pr_err("Patching NOP failed.\n");
> return -EPERM;
> }
> @@ -442,7 +443,7 @@ int ftrace_make_nop(struct module *mod,
> if (test_24bit_addr(ip, addr)) {
> /* within range */
> old = ftrace_call_replace(ip, addr, 1);
> - new = PPC_INST_NOP;
> + new = ppc_inst(PPC_INST_NOP);
> return ftrace_modify_code(ip, old, new);
> } else if (core_kernel_text(ip))
> return __ftrace_make_nop_kernel(rec, addr);
> @@ -496,7 +497,7 @@ expected_nop_sequence(void *ip, unsigned int op0,
> unsigned int op1)
> * The load offset is different depending on the ABI. For simplicity
> * just mask it out when doing the compare.
> */
> - if ((op0 != 0x48000008) || ((op1 & 0xffff0000) != 0xe8410000))
> + if ((op0 != ppc_inst(0x48000008)) || ((op1 & 0xffff0000) !=
> 0xe8410000))
> return 0;
> return 1;
> }
> @@ -505,7 +506,7 @@ static int
> expected_nop_sequence(void *ip, unsigned int op0, unsigned int op1)
> {
> /* look for patched "NOP" on ppc64 with -mprofile-kernel */
> - if (op0 != PPC_INST_NOP)
> + if (op0 != ppc_inst(PPC_INST_NOP))
> return 0;
> return 1;
> }
> @@ -588,7 +589,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long
> addr)
> return -EFAULT;
>
> /* It should be pointing to a nop */
> - if (op != PPC_INST_NOP) {
> + if (op != ppc_inst(PPC_INST_NOP)) {
> pr_err("Expected NOP but have %x\n", op);
> return -EINVAL;
> }
> @@ -645,7 +646,7 @@ static int __ftrace_make_call_kernel(struct dyn_ftrace
> *rec, unsigned long addr)
> return -EFAULT;
> }
>
> - if (op != PPC_INST_NOP) {
> + if (op != ppc_inst(PPC_INST_NOP)) {
> pr_err("Unexpected call sequence at %p: %x\n", ip, op);
> return -EINVAL;
> }
> @@ -676,7 +677,7 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned
> long addr)
> */
> if (test_24bit_addr(ip, addr)) {
> /* within range */
> - old = PPC_INST_NOP;
> + old = ppc_inst(PPC_INST_NOP);
> new = ftrace_call_replace(ip, addr, 1);
> return ftrace_modify_code(ip, old, new);
> } else if (core_kernel_text(ip))
> diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c
> index 1cfef0e5fec5..31c870287f2b 100644
> --- a/arch/powerpc/kernel/uprobes.c
> +++ b/arch/powerpc/kernel/uprobes.c
> @@ -14,6 +14,7 @@
> #include <linux/kdebug.h>
>
> #include <asm/sstep.h>
> +#include <asm/inst.h>
>
> #define UPROBE_TRAP_NR UINT_MAX
>
> diff --git a/arch/powerpc/kvm/emulate_loadstore.c
> b/arch/powerpc/kvm/emulate_loadstore.c
> index 1139bc56e004..135d0e686622 100644
> --- a/arch/powerpc/kvm/emulate_loadstore.c
> +++ b/arch/powerpc/kvm/emulate_loadstore.c
> @@ -95,7 +95,7 @@ int kvmppc_emulate_loadstore(struct kvm_vcpu *vcpu)
>
> emulated = EMULATE_FAIL;
> vcpu->arch.regs.msr = vcpu->arch.shared->msr;
> - if (analyse_instr(&op, &vcpu->arch.regs, inst) == 0) {
> + if (analyse_instr(&op, &vcpu->arch.regs, ppc_inst(inst)) == 0) {
> int type = op.type & INSTR_TYPE_MASK;
> int size = GETSIZE(op.type);
>
> diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-
> patching.c
> index 4af564aeedf1..fdf0d6ea3575 100644
> --- a/arch/powerpc/lib/code-patching.c
> +++ b/arch/powerpc/lib/code-patching.c
> @@ -17,6 +17,7 @@
> #include <asm/page.h>
> #include <asm/code-patching.h>
> #include <asm/setup.h>
> +#include <asm/inst.h>
>
> static int __patch_instruction(unsigned int *exec_addr, unsigned int instr,
> unsigned int *patch_addr)
> @@ -413,37 +414,37 @@ static void __init test_branch_iform(void)
> addr = (unsigned long)&instr;
>
> /* The simplest case, branch to self, no flags */
> - check(instr_is_branch_iform(0x48000000));
> + check(instr_is_branch_iform(ppc_inst(0x48000000)));
> /* All bits of target set, and flags */
> - check(instr_is_branch_iform(0x4bffffff));
> + check(instr_is_branch_iform(ppc_inst(0x4bffffff)));
> /* High bit of opcode set, which is wrong */
> - check(!instr_is_branch_iform(0xcbffffff));
> + check(!instr_is_branch_iform(ppc_inst(0xcbffffff)));
> /* Middle bits of opcode set, which is wrong */
> - check(!instr_is_branch_iform(0x7bffffff));
> + check(!instr_is_branch_iform(ppc_inst(0x7bffffff)));
>
> /* Simplest case, branch to self with link */
> - check(instr_is_branch_iform(0x48000001));
> + check(instr_is_branch_iform(ppc_inst(0x48000001)));
> /* All bits of targets set */
> - check(instr_is_branch_iform(0x4bfffffd));
> + check(instr_is_branch_iform(ppc_inst(0x4bfffffd)));
> /* Some bits of targets set */
> - check(instr_is_branch_iform(0x4bff00fd));
> + check(instr_is_branch_iform(ppc_inst(0x4bff00fd)));
> /* Must be a valid branch to start with */
> - check(!instr_is_branch_iform(0x7bfffffd));
> + check(!instr_is_branch_iform(ppc_inst(0x7bfffffd)));
>
> /* Absolute branch to 0x100 */
> - instr = 0x48000103;
> + instr = ppc_inst(0x48000103);
> check(instr_is_branch_to_addr(&instr, 0x100));
> /* Absolute branch to 0x420fc */
> - instr = 0x480420ff;
> + instr = ppc_inst(0x480420ff);
> check(instr_is_branch_to_addr(&instr, 0x420fc));
> /* Maximum positive relative branch, + 20MB - 4B */
> - instr = 0x49fffffc;
> + instr = ppc_inst(0x49fffffc);
> check(instr_is_branch_to_addr(&instr, addr + 0x1FFFFFC));
> /* Smallest negative relative branch, - 4B */
> - instr = 0x4bfffffc;
> + instr = ppc_inst(0x4bfffffc);
> check(instr_is_branch_to_addr(&instr, addr - 4));
> /* Largest negative relative branch, - 32 MB */
> - instr = 0x4a000000;
> + instr = ppc_inst(0x4a000000);
> check(instr_is_branch_to_addr(&instr, addr - 0x2000000));
>
> /* Branch to self, with link */
> @@ -477,7 +478,7 @@ static void __init test_branch_iform(void)
> /* Check flags are masked correctly */
> err = create_branch(&instr, &instr, addr, 0xFFFFFFFC);
> check(instr_is_branch_to_addr(&instr, addr));
> - check(instr == 0x48000000);
> + check(instr == ppc_inst(0x48000000));
> }
>
> static void __init test_create_function_call(void)
> @@ -504,28 +505,28 @@ static void __init test_branch_bform(void)
> addr = (unsigned long)iptr;
>
> /* The simplest case, branch to self, no flags */
> - check(instr_is_branch_bform(0x40000000));
> + check(instr_is_branch_bform(ppc_inst(0x40000000)));
> /* All bits of target set, and flags */
> - check(instr_is_branch_bform(0x43ffffff));
> + check(instr_is_branch_bform(ppc_inst(0x43ffffff)));
> /* High bit of opcode set, which is wrong */
> - check(!instr_is_branch_bform(0xc3ffffff));
> + check(!instr_is_branch_bform(ppc_inst(0xc3ffffff)));
> /* Middle bits of opcode set, which is wrong */
> - check(!instr_is_branch_bform(0x7bffffff));
> + check(!instr_is_branch_bform(ppc_inst(0x7bffffff)));
>
> /* Absolute conditional branch to 0x100 */
> - instr = 0x43ff0103;
> + instr = ppc_inst(0x43ff0103);
> check(instr_is_branch_to_addr(&instr, 0x100));
> /* Absolute conditional branch to 0x20fc */
> - instr = 0x43ff20ff;
> + instr = ppc_inst(0x43ff20ff);
> check(instr_is_branch_to_addr(&instr, 0x20fc));
> /* Maximum positive relative conditional branch, + 32 KB - 4B */
> - instr = 0x43ff7ffc;
> + instr = ppc_inst(0x43ff7ffc);
> check(instr_is_branch_to_addr(&instr, addr + 0x7FFC));
> /* Smallest negative relative conditional branch, - 4B */
> - instr = 0x43fffffc;
> + instr = ppc_inst(0x43fffffc);
> check(instr_is_branch_to_addr(&instr, addr - 4));
> /* Largest negative relative conditional branch, - 32 KB */
> - instr = 0x43ff8000;
> + instr = ppc_inst(0x43ff8000);
> check(instr_is_branch_to_addr(&instr, addr - 0x8000));
>
> /* All condition code bits set & link */
> @@ -562,7 +563,7 @@ static void __init test_branch_bform(void)
> /* Check flags are masked correctly */
> err = create_cond_branch(&instr, iptr, addr, 0xFFFFFFFC);
> check(instr_is_branch_to_addr(&instr, addr));
> - check(instr == 0x43FF0000);
> + check(instr == ppc_inst(0x43FF0000));
> }
>
> static void __init test_translate_branch(void)
> @@ -596,7 +597,7 @@ static void __init test_translate_branch(void)
> patch_instruction(q, instr);
> check(instr_is_branch_to_addr(p, addr));
> check(instr_is_branch_to_addr(q, addr));
> - check(*q == 0x4a000000);
> + check(*q == ppc_inst(0x4a000000));
>
> /* Maximum positive case, move x to x - 32 MB + 4 */
> p = buf + 0x2000000;
> @@ -607,7 +608,7 @@ static void __init test_translate_branch(void)
> patch_instruction(q, instr);
> check(instr_is_branch_to_addr(p, addr));
> check(instr_is_branch_to_addr(q, addr));
> - check(*q == 0x49fffffc);
> + check(*q == ppc_inst(0x49fffffc));
>
> /* Jump to x + 16 MB moved to x + 20 MB */
> p = buf;
> @@ -653,7 +654,7 @@ static void __init test_translate_branch(void)
> patch_instruction(q, instr);
> check(instr_is_branch_to_addr(p, addr));
> check(instr_is_branch_to_addr(q, addr));
> - check(*q == 0x43ff8000);
> + check(*q == ppc_inst(0x43ff8000));
>
> /* Maximum positive case, move x to x - 32 KB + 4 */
> p = buf + 0x8000;
> @@ -665,7 +666,7 @@ static void __init test_translate_branch(void)
> patch_instruction(q, instr);
> check(instr_is_branch_to_addr(p, addr));
> check(instr_is_branch_to_addr(q, addr));
> - check(*q == 0x43ff7ffc);
> + check(*q == ppc_inst(0x43ff7ffc));
>
> /* Jump to x + 12 KB moved to x + 20 KB */
> p = buf;
> diff --git a/arch/powerpc/lib/feature-fixups.c b/arch/powerpc/lib/feature-
> fixups.c
> index b129d7b4e7dd..6e7479b8887a 100644
> --- a/arch/powerpc/lib/feature-fixups.c
> +++ b/arch/powerpc/lib/feature-fixups.c
> @@ -21,6 +21,7 @@
> #include <asm/setup.h>
> #include <asm/security_features.h>
> #include <asm/firmware.h>
> +#include <asm/inst.h>
>
> struct fixup_entry {
> unsigned long mask;
> @@ -89,7 +90,7 @@ static int patch_feature_section(unsigned long value,
> struct fixup_entry *fcur)
> }
>
> for (; dest < end; dest++)
> - raw_patch_instruction(dest, PPC_INST_NOP);
> + raw_patch_instruction(dest, ppc_inst(PPC_INST_NOP));
>
> return 0;
> }
> @@ -146,15 +147,15 @@ static void do_stf_entry_barrier_fixups(enum
> stf_barrier_type types)
>
> pr_devel("patching dest %lx\n", (unsigned long)dest);
>
> - patch_instruction(dest, instrs[0]);
> + patch_instruction(dest, ppc_inst(instrs[0]));
>
> if (types & STF_BARRIER_FALLBACK)
> patch_branch(dest + 1, (unsigned
> long)&stf_barrier_fallback,
> BRANCH_SET_LINK);
> else
> - patch_instruction(dest + 1, instrs[1]);
> + patch_instruction(dest + 1, ppc_inst(instrs[1]));
>
> - patch_instruction(dest + 2, instrs[2]);
> + patch_instruction(dest + 2, ppc_inst(instrs[2]));
> }
>
> printk(KERN_DEBUG "stf-barrier: patched %d entry locations (%s
> barrier)\n", i,
> @@ -207,12 +208,12 @@ static void do_stf_exit_barrier_fixups(enum
> stf_barrier_type types)
>
> pr_devel("patching dest %lx\n", (unsigned long)dest);
>
> - patch_instruction(dest, instrs[0]);
> - patch_instruction(dest + 1, instrs[1]);
> - patch_instruction(dest + 2, instrs[2]);
> - patch_instruction(dest + 3, instrs[3]);
> - patch_instruction(dest + 4, instrs[4]);
> - patch_instruction(dest + 5, instrs[5]);
> + patch_instruction(dest, ppc_inst(instrs[0]));
> + patch_instruction(dest + 1, ppc_inst(instrs[1]));
> + patch_instruction(dest + 2, ppc_inst(instrs[2]));
> + patch_instruction(dest + 3, ppc_inst(instrs[3]));
> + patch_instruction(dest + 4, ppc_inst(instrs[4]));
> + patch_instruction(dest + 5, ppc_inst(instrs[5]));
> }
> printk(KERN_DEBUG "stf-barrier: patched %d exit locations (%s
> barrier)\n", i,
> (types == STF_BARRIER_NONE) ? "no" :
> @@ -260,9 +261,9 @@ void do_rfi_flush_fixups(enum l1d_flush_type types)
>
> pr_devel("patching dest %lx\n", (unsigned long)dest);
>
> - patch_instruction(dest, instrs[0]);
> - patch_instruction(dest + 1, instrs[1]);
> - patch_instruction(dest + 2, instrs[2]);
> + patch_instruction(dest, ppc_inst(instrs[0]));
> + patch_instruction(dest + 1, ppc_inst(instrs[1]));
> + patch_instruction(dest + 2, ppc_inst(instrs[2]));
> }
>
> printk(KERN_DEBUG "rfi-flush: patched %d locations (%s flush)\n", i,
> @@ -295,7 +296,7 @@ void do_barrier_nospec_fixups_range(bool enable, void
> *fixup_start, void *fixup_
> dest = (void *)start + *start;
>
> pr_devel("patching dest %lx\n", (unsigned long)dest);
> - patch_instruction(dest, instr);
> + patch_instruction(dest, ppc_inst(instr));
> }
>
> printk(KERN_DEBUG "barrier-nospec: patched %d locations\n", i);
> @@ -338,8 +339,8 @@ void do_barrier_nospec_fixups_range(bool enable, void
> *fixup_start, void *fixup_
> dest = (void *)start + *start;
>
> pr_devel("patching dest %lx\n", (unsigned long)dest);
> - patch_instruction(dest, instr[0]);
> - patch_instruction(dest + 1, instr[1]);
> + patch_instruction(dest, ppc_inst(instr[0]));
> + patch_instruction(dest + 1, ppc_inst(instr[1]));
> }
>
> printk(KERN_DEBUG "barrier-nospec: patched %d locations\n", i);
> @@ -353,7 +354,7 @@ static void patch_btb_flush_section(long *curr)
> end = (void *)curr + *(curr + 1);
> for (; start < end; start++) {
> pr_devel("patching dest %lx\n", (unsigned long)start);
> - patch_instruction(start, PPC_INST_NOP);
> + patch_instruction(start, ppc_inst(PPC_INST_NOP));
> }
> }
>
> @@ -382,7 +383,7 @@ void do_lwsync_fixups(unsigned long value, void
> *fixup_start, void *fixup_end)
>
> for (; start < end; start++) {
> dest = (void *)start + *start;
> - raw_patch_instruction(dest, PPC_INST_LWSYNC);
> + raw_patch_instruction(dest, ppc_inst(PPC_INST_LWSYNC));
> }
> }
>
> @@ -400,7 +401,7 @@ static void do_final_fixups(void)
> length = (__end_interrupts - _stext) / sizeof(int);
>
> while (length--) {
> - raw_patch_instruction(dest, *src);
> + raw_patch_instruction(dest, ppc_inst(*src));
> src++;
> dest++;
> }
> diff --git a/arch/powerpc/lib/test_emulate_step.c
> b/arch/powerpc/lib/test_emulate_step.c
> index 42347067739c..88b17226a493 100644
> --- a/arch/powerpc/lib/test_emulate_step.c
> +++ b/arch/powerpc/lib/test_emulate_step.c
> @@ -11,6 +11,7 @@
> #include <asm/sstep.h>
> #include <asm/ppc-opcode.h>
> #include <asm/code-patching.h>
> +#include <asm/inst.h>
>
> #define IMM_L(i) ((uintptr_t)(i) & 0xffff)
>
> @@ -18,40 +19,40 @@
> * Defined with TEST_ prefix so it does not conflict with other
> * definitions.
> */
> -#define TEST_LD(r, base, i) (PPC_INST_LD | ___PPC_RT(r) | \
> +#define TEST_LD(r, base, i) ppc_inst(PPC_INST_LD | ___PPC_RT(r) |
> \
> ___PPC_RA(base) | IMM_L(i))
> -#define TEST_LWZ(r, base, i) (PPC_INST_LWZ | ___PPC_RT(r) | \
> +#define TEST_LWZ(r, base, i) ppc_inst(PPC_INST_LWZ | ___PPC_RT(r) |
> \
> ___PPC_RA(base) | IMM_L(i))
> -#define TEST_LWZX(t, a, b) (PPC_INST_LWZX | ___PPC_RT(t) | \
> +#define TEST_LWZX(t, a, b) ppc_inst(PPC_INST_LWZX | ___PPC_RT(t) |
> \
> ___PPC_RA(a) | ___PPC_RB(b))
> -#define TEST_STD(r, base, i) (PPC_INST_STD | ___PPC_RS(r) | \
> +#define TEST_STD(r, base, i) ppc_inst(PPC_INST_STD | ___PPC_RS(r) |
> \
> ___PPC_RA(base) | ((i) & 0xfffc))
> -#define TEST_LDARX(t, a, b, eh) (PPC_INST_LDARX | ___PPC_RT(t) | \
> +#define TEST_LDARX(t, a, b, eh) ppc_inst(PPC_INST_LDARX | ___PPC_RT(t)
> | \
> ___PPC_RA(a) | ___PPC_RB(b) | \
> __PPC_EH(eh))
> -#define TEST_STDCX(s, a, b) (PPC_INST_STDCX | ___PPC_RS(s) | \
> +#define TEST_STDCX(s, a, b) ppc_inst(PPC_INST_STDCX | ___PPC_RS(s) | \
> ___PPC_RA(a) | ___PPC_RB(b))
> -#define TEST_LFSX(t, a, b) (PPC_INST_LFSX | ___PPC_RT(t) | \
> +#define TEST_LFSX(t, a, b) ppc_inst(PPC_INST_LFSX | ___PPC_RT(t) |
> \
> ___PPC_RA(a) | ___PPC_RB(b))
> -#define TEST_STFSX(s, a, b) (PPC_INST_STFSX | ___PPC_RS(s) | \
> +#define TEST_STFSX(s, a, b) ppc_inst(PPC_INST_STFSX | ___PPC_RS(s) | \
> ___PPC_RA(a) | ___PPC_RB(b))
> -#define TEST_LFDX(t, a, b) (PPC_INST_LFDX | ___PPC_RT(t) | \
> +#define TEST_LFDX(t, a, b) ppc_inst(PPC_INST_LFDX | ___PPC_RT(t) |
> \
> ___PPC_RA(a) | ___PPC_RB(b))
> -#define TEST_STFDX(s, a, b) (PPC_INST_STFDX | ___PPC_RS(s) | \
> +#define TEST_STFDX(s, a, b) ppc_inst(PPC_INST_STFDX | ___PPC_RS(s) | \
> ___PPC_RA(a) | ___PPC_RB(b))
> -#define TEST_LVX(t, a, b) (PPC_INST_LVX | ___PPC_RT(t) | \
> +#define TEST_LVX(t, a, b) ppc_inst(PPC_INST_LVX | ___PPC_RT(t) |
> \
> ___PPC_RA(a) | ___PPC_RB(b))
> -#define TEST_STVX(s, a, b) (PPC_INST_STVX | ___PPC_RS(s) | \
> +#define TEST_STVX(s, a, b) ppc_inst(PPC_INST_STVX | ___PPC_RS(s) |
> \
> ___PPC_RA(a) | ___PPC_RB(b))
> -#define TEST_LXVD2X(s, a, b) (PPC_INST_LXVD2X | VSX_XX1((s), R##a, R##b))
> -#define TEST_STXVD2X(s, a, b) (PPC_INST_STXVD2X | VSX_XX1((s), R##a,
> R##b))
> -#define TEST_ADD(t, a, b) (PPC_INST_ADD | ___PPC_RT(t) | \
> +#define TEST_LXVD2X(s, a, b) ppc_inst(PPC_INST_LXVD2X | VSX_XX1((s), R##a,
> R##b))
> +#define TEST_STXVD2X(s, a, b) ppc_inst(PPC_INST_STXVD2X |
> VSX_XX1((s), R##a, R##b))
> +#define TEST_ADD(t, a, b) ppc_inst(PPC_INST_ADD | ___PPC_RT(t) |
> \
> ___PPC_RA(a) | ___PPC_RB(b))
> -#define TEST_ADD_DOT(t, a, b) (PPC_INST_ADD | ___PPC_RT(t) |
> \
> +#define TEST_ADD_DOT(t, a, b) ppc_inst(PPC_INST_ADD | ___PPC_RT(t) |
> \
> ___PPC_RA(a) | ___PPC_RB(b) | 0x1)
> -#define TEST_ADDC(t, a, b) (PPC_INST_ADDC | ___PPC_RT(t) | \
> +#define TEST_ADDC(t, a, b) ppc_inst(PPC_INST_ADDC | ___PPC_RT(t) |
> \
> ___PPC_RA(a) | ___PPC_RB(b))
> -#define TEST_ADDC_DOT(t, a, b) (PPC_INST_ADDC | ___PPC_RT(t) |
> \
> +#define TEST_ADDC_DOT(t, a, b) ppc_inst(PPC_INST_ADDC | ___PPC_RT(t) |
> \
> ___PPC_RA(a) | ___PPC_RB(b) | 0x1)
>
> #define MAX_SUBTESTS 16
> @@ -471,7 +472,7 @@ static struct compute_test compute_tests[] = {
> .subtests = {
> {
> .descr = "R0 = LONG_MAX",
> - .instr = PPC_INST_NOP,
> + .instr = ppc_inst(PPC_INST_NOP),
> .regs = {
> .gpr[0] = LONG_MAX,
> }
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index 049375206510..68ecb39dd9cc 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -54,6 +54,7 @@
> #include <asm/firmware.h>
> #include <asm/code-patching.h>
> #include <asm/sections.h>
> +#include <asm/inst.h>
>
> #ifdef CONFIG_PPC64
> #include <asm/hvcall.h>
> @@ -949,7 +950,7 @@ static void remove_bpts(void)
> if ((bp->enabled & (BP_TRAP|BP_CIABR)) != BP_TRAP)
> continue;
> if (mread(bp->address, &instr, 4) == 4
> - && instr == bpinstr
> + && instr == ppc_inst(bpinstr)
> && patch_instruction(
> (unsigned int *)bp->address, bp->instr[0]) != 0)
> printf("Couldn't remove breakpoint at %lx\n",
> @@ -2845,7 +2846,7 @@ generic_inst_dump(unsigned long adr, long count, int
> praddr,
> {
> int nr, dotted;
> unsigned long first_adr;
> - unsigned int inst, last_inst = 0;
> + unsigned int inst, last_inst = ppc_inst(0);
> unsigned char val[4];
>
> dotted = 0;
> @@ -2858,7 +2859,7 @@ generic_inst_dump(unsigned long adr, long count, int
> praddr,
> }
> break;
> }
> - inst = GETWORD(val);
> + inst = ppc_inst(GETWORD(val));
> if (adr > first_adr && inst == last_inst) {
> if (!dotted) {
> printf(" ...\n");
^ permalink raw reply
* Re: [PATCH v5 03/21] powerpc: Change calling convention for create_branch() et. al.
From: Jordan Niethe @ 2020-04-07 6:35 UTC (permalink / raw)
To: Balamuruhan S
Cc: Alistair Popple, Nicholas Piggin, linuxppc-dev, Daniel Axtens
In-Reply-To: <a0bfb808a0fe815ffbf7ad655d32ed837426943f.camel@linux.ibm.com>
On Tue, Apr 7, 2020 at 4:10 PM Balamuruhan S <bala24@linux.ibm.com> wrote:
>
> On Mon, 2020-04-06 at 18:09 +1000, Jordan Niethe wrote:
> > create_branch(), create_cond_branch() and translate_branch() return the
> > instruction that they create, or return 0 to signal an error. Seperate
>
> s/seperate/separate
thanks.
>
> > these concerns in preparation for an instruction type that is not just
> > an unsigned int. Fill the created instruction to a pointer passed as
> > the first parameter to the function and use a non-zero return value to
> > signify an error.
> >
> > Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> > ---
> > v5: New to series
> > ---
> > arch/powerpc/include/asm/code-patching.h | 12 +-
> > arch/powerpc/kernel/optprobes.c | 24 ++--
> > arch/powerpc/kernel/setup_32.c | 2 +-
> > arch/powerpc/kernel/trace/ftrace.c | 24 ++--
> > arch/powerpc/lib/code-patching.c | 133 +++++++++++++----------
> > arch/powerpc/lib/feature-fixups.c | 5 +-
> > 6 files changed, 117 insertions(+), 83 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/code-patching.h
> > b/arch/powerpc/include/asm/code-patching.h
> > index 898b54262881..351dda7215b6 100644
> > --- a/arch/powerpc/include/asm/code-patching.h
> > +++ b/arch/powerpc/include/asm/code-patching.h
> > @@ -22,10 +22,10 @@
> > #define BRANCH_ABSOLUTE 0x2
> >
> > bool is_offset_in_branch_range(long offset);
> > -unsigned int create_branch(const unsigned int *addr,
> > - unsigned long target, int flags);
> > -unsigned int create_cond_branch(const unsigned int *addr,
> > - unsigned long target, int flags);
> > +int create_branch(unsigned int *instr, const unsigned int *addr,
> > + unsigned long target, int flags);
> > +int create_cond_branch(unsigned int *instr, const unsigned int *addr,
> > + unsigned long target, int flags);
> > int patch_branch(unsigned int *addr, unsigned long target, int flags);
> > int patch_instruction(unsigned int *addr, unsigned int instr);
> > int raw_patch_instruction(unsigned int *addr, unsigned int instr);
> > @@ -60,8 +60,8 @@ int instr_is_relative_branch(unsigned int instr);
> > int instr_is_relative_link_branch(unsigned int instr);
> > int instr_is_branch_to_addr(const unsigned int *instr, unsigned long addr);
> > unsigned long branch_target(const unsigned int *instr);
> > -unsigned int translate_branch(const unsigned int *dest,
> > - const unsigned int *src);
> > +int translate_branch(unsigned int *instr, const unsigned int *dest,
> > + const unsigned int *src);
> > extern bool is_conditional_branch(unsigned int instr);
> > #ifdef CONFIG_PPC_BOOK3E_64
> > void __patch_exception(int exc, unsigned long addr);
> > diff --git a/arch/powerpc/kernel/optprobes.c
> > b/arch/powerpc/kernel/optprobes.c
> > index 024f7aad1952..445b3dad82dc 100644
> > --- a/arch/powerpc/kernel/optprobes.c
> > +++ b/arch/powerpc/kernel/optprobes.c
> > @@ -251,15 +251,17 @@ int arch_prepare_optimized_kprobe(struct
> > optimized_kprobe *op, struct kprobe *p)
> > goto error;
> > }
> >
> > - branch_op_callback = create_branch((unsigned int *)buff +
> > TMPL_CALL_HDLR_IDX,
> > - (unsigned long)op_callback_addr,
> > - BRANCH_SET_LINK);
> > + rc = create_branch(&branch_op_callback,
> > + (unsigned int *)buff + TMPL_CALL_HDLR_IDX,
> > + (unsigned long)op_callback_addr,
> > + BRANCH_SET_LINK);
> >
> > - branch_emulate_step = create_branch((unsigned int *)buff +
> > TMPL_EMULATE_IDX,
> > - (unsigned long)emulate_step_addr,
> > - BRANCH_SET_LINK);
> > + rc |= create_branch(&branch_emulate_step,
> > + (unsigned int *)buff + TMPL_EMULATE_IDX,
> > + (unsigned long)emulate_step_addr,
> > + BRANCH_SET_LINK);
> >
> > - if (!branch_op_callback || !branch_emulate_step)
> > + if (rc)
> > goto error;
> >
> > patch_instruction(buff + TMPL_CALL_HDLR_IDX, branch_op_callback);
> > @@ -305,6 +307,7 @@ int arch_check_optimized_kprobe(struct optimized_kprobe
> > *op)
> >
> > void arch_optimize_kprobes(struct list_head *oplist)
> > {
> > + unsigned int instr;
> > struct optimized_kprobe *op;
> > struct optimized_kprobe *tmp;
> >
> > @@ -315,9 +318,10 @@ void arch_optimize_kprobes(struct list_head *oplist)
> > */
> > memcpy(op->optinsn.copied_insn, op->kp.addr,
> > RELATIVEJUMP_SIZE);
> > - patch_instruction(op->kp.addr,
> > - create_branch((unsigned int *)op->kp.addr,
> > - (unsigned long)op->optinsn.insn, 0));
> > + create_branch(&instr,
> > + (unsigned int *)op->kp.addr,
> > + (unsigned long)op->optinsn.insn, 0);
> > + patch_instruction(op->kp.addr, instr);
> > list_del_init(&op->list);
> > }
> > }
> > diff --git a/arch/powerpc/kernel/setup_32.c b/arch/powerpc/kernel/setup_32.c
> > index 5b49b26eb154..c1bdd462c5c0 100644
> > --- a/arch/powerpc/kernel/setup_32.c
> > +++ b/arch/powerpc/kernel/setup_32.c
> > @@ -88,7 +88,7 @@ notrace void __init machine_init(u64 dt_ptr)
> >
> > patch_instruction_site(&patch__memcpy_nocache, PPC_INST_NOP);
> >
> > - insn = create_cond_branch(addr, branch_target(addr), 0x820000);
> > + create_cond_branch(&insn, addr, branch_target(addr), 0x820000);
> > patch_instruction(addr, insn); /* replace b by bne cr0 */
> >
> > /* Do some early initialization based on the flat device tree */
> > diff --git a/arch/powerpc/kernel/trace/ftrace.c
> > b/arch/powerpc/kernel/trace/ftrace.c
> > index 7ea0ca044b65..4dcb379133f8 100644
> > --- a/arch/powerpc/kernel/trace/ftrace.c
> > +++ b/arch/powerpc/kernel/trace/ftrace.c
> > @@ -48,7 +48,7 @@ ftrace_call_replace(unsigned long ip, unsigned long addr,
> > int link)
> > addr = ppc_function_entry((void *)addr);
> >
> > /* if (link) set op to 'bl' else 'b' */
> > - op = create_branch((unsigned int *)ip, addr, link ? 1 : 0);
> > + create_branch(&op, (unsigned int *)ip, addr, link ? 1 : 0);
> >
> > return op;
> > }
> > @@ -89,10 +89,11 @@ ftrace_modify_code(unsigned long ip, unsigned int old,
> > unsigned int new)
> > */
> > static int test_24bit_addr(unsigned long ip, unsigned long addr)
> > {
> > + unsigned int op;
> > addr = ppc_function_entry((void *)addr);
> >
> > /* use the create_branch to verify that this offset can be branched */
> > - return create_branch((unsigned int *)ip, addr, 0);
> > + return create_branch(&op, (unsigned int *)ip, addr, 0) == 0;
> > }
> >
> > static int is_bl_op(unsigned int op)
> > @@ -287,6 +288,7 @@ __ftrace_make_nop(struct module *mod,
> > static unsigned long find_ftrace_tramp(unsigned long ip)
> > {
> > int i;
> > + unsigned int instr;
> >
> > /*
> > * We have the compiler generated long_branch tramps at the end
> > @@ -295,7 +297,8 @@ static unsigned long find_ftrace_tramp(unsigned long ip)
> > for (i = NUM_FTRACE_TRAMPS - 1; i >= 0; i--)
> > if (!ftrace_tramps[i])
> > continue;
> > - else if (create_branch((void *)ip, ftrace_tramps[i], 0))
> > + else if (create_branch(&instr, (void *)ip,
> > + ftrace_tramps[i], 0) == 0)
> > return ftrace_tramps[i];
> >
> > return 0;
> > @@ -324,6 +327,7 @@ static int setup_mcount_compiler_tramp(unsigned long
> > tramp)
> > {
> > int i, op;
> > unsigned long ptr;
> > + unsigned int instr;
> > static unsigned long ftrace_plt_tramps[NUM_FTRACE_TRAMPS];
> >
> > /* Is this a known long jump tramp? */
> > @@ -366,7 +370,7 @@ static int setup_mcount_compiler_tramp(unsigned long
> > tramp)
> > #else
> > ptr = ppc_global_function_entry((void *)ftrace_caller);
> > #endif
> > - if (!create_branch((void *)tramp, ptr, 0)) {
> > + if (create_branch(&instr, (void *)tramp, ptr, 0)) {
> > pr_debug("%ps is not reachable from existing mcount tramp\n",
> > (void *)ptr);
> > return -1;
> > @@ -511,6 +515,7 @@ static int
> > __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
> > {
> > unsigned int op[2];
> > + unsigned int instr;
> > void *ip = (void *)rec->ip;
> > unsigned long entry, ptr, tramp;
> > struct module *mod = rec->arch.mod;
> > @@ -557,7 +562,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long
> > addr)
> > }
> >
> > /* Ensure branch is within 24 bits */
> > - if (!create_branch(ip, tramp, BRANCH_SET_LINK)) {
> > + if (create_branch(&instr, ip, tramp, BRANCH_SET_LINK)) {
> > pr_err("Branch out of range\n");
> > return -EINVAL;
> > }
> > @@ -574,6 +579,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long
> > addr)
> > static int
> > __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
> > {
> > + int err;
> > unsigned int op;
> > unsigned long ip = rec->ip;
> >
> > @@ -594,9 +600,9 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long
> > addr)
> > }
> >
> > /* create the branch to the trampoline */
> > - op = create_branch((unsigned int *)ip,
> > - rec->arch.mod->arch.tramp, BRANCH_SET_LINK);
> > - if (!op) {
> > + err = create_branch(&op, (unsigned int *)ip,
> > + rec->arch.mod->arch.tramp, BRANCH_SET_LINK);
> > + if (!err) {
>
>
> should this be `if (err)` as per your new changes ?
You are right.
>
>
> > pr_err("REL24 out of range!\n");
> > return -EINVAL;
> > }
> > @@ -776,7 +782,7 @@ __ftrace_modify_call(struct dyn_ftrace *rec, unsigned
> > long old_addr,
> > }
> >
> > /* Ensure branch is within 24 bits */
> > - if (!create_branch((unsigned int *)ip, tramp, BRANCH_SET_LINK)) {
> > + if (create_branch(&op, (unsigned int *)ip, tramp, BRANCH_SET_LINK)) {
> > pr_err("Branch out of range\n");
> > return -EINVAL;
> > }
> > diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-
> > patching.c
> > index 3345f039a876..4af564aeedf1 100644
> > --- a/arch/powerpc/lib/code-patching.c
> > +++ b/arch/powerpc/lib/code-patching.c
> > @@ -196,7 +196,10 @@ NOKPROBE_SYMBOL(patch_instruction);
> >
> > int patch_branch(unsigned int *addr, unsigned long target, int flags)
> > {
> > - return patch_instruction(addr, create_branch(addr, target, flags));
> > + unsigned int instr;
> > +
> > + create_branch(&instr, addr, target, flags);
> > + return patch_instruction(addr, instr);
> > }
> >
> > bool is_offset_in_branch_range(long offset)
> > @@ -243,30 +246,30 @@ bool is_conditional_branch(unsigned int instr)
> > }
> > NOKPROBE_SYMBOL(is_conditional_branch);
> >
> > -unsigned int create_branch(const unsigned int *addr,
> > - unsigned long target, int flags)
> > +int create_branch(unsigned int *instr,
> > + const unsigned int *addr,
> > + unsigned long target, int flags)
> > {
> > - unsigned int instruction;
> > long offset;
> >
> > + *instr = 0;
> > offset = target;
> > if (! (flags & BRANCH_ABSOLUTE))
> > offset = offset - (unsigned long)addr;
> >
> > /* Check we can represent the target in the instruction format */
> > if (!is_offset_in_branch_range(offset))
> > - return 0;
> > + return 1;
> >
> > /* Mask out the flags and target, so they don't step on each other. */
> > - instruction = 0x48000000 | (flags & 0x3) | (offset & 0x03FFFFFC);
> > + *instr = 0x48000000 | (flags & 0x3) | (offset & 0x03FFFFFC);
> >
> > - return instruction;
> > + return 0;
> > }
> >
> > -unsigned int create_cond_branch(const unsigned int *addr,
> > - unsigned long target, int flags)
> > +int create_cond_branch(unsigned int *instr, const unsigned int *addr,
> > + unsigned long target, int flags)
> > {
> > - unsigned int instruction;
> > long offset;
> >
> > offset = target;
> > @@ -275,12 +278,12 @@ unsigned int create_cond_branch(const unsigned int
> > *addr,
> >
> > /* Check we can represent the target in the instruction format */
> > if (offset < -0x8000 || offset > 0x7FFF || offset & 0x3)
> > - return 0;
> > + return 1;
> >
> > /* Mask out the flags and target, so they don't step on each other. */
> > - instruction = 0x40000000 | (flags & 0x3FF0003) | (offset & 0xFFFC);
> > + *instr = 0x40000000 | (flags & 0x3FF0003) | (offset & 0xFFFC);
> >
> > - return instruction;
> > + return 0;
> > }
> >
> > static unsigned int branch_opcode(unsigned int instr)
> > @@ -361,18 +364,18 @@ int instr_is_branch_to_addr(const unsigned int *instr,
> > unsigned long addr)
> > return 0;
> > }
> >
> > -unsigned int translate_branch(const unsigned int *dest, const unsigned int
> > *src)
> > +int translate_branch(unsigned int *instr, const unsigned int *dest, const
> > unsigned int *src)
> > {
> > unsigned long target;
> >
> > target = branch_target(src);
> >
> > if (instr_is_branch_iform(*src))
> > - return create_branch(dest, target, *src);
> > + return create_branch(instr, dest, target, *src);
> > else if (instr_is_branch_bform(*src))
> > - return create_cond_branch(dest, target, *src);
> > + return create_cond_branch(instr, dest, target, *src);
> >
> > - return 0;
> > + return 1;
> > }
> >
> > #ifdef CONFIG_PPC_BOOK3E_64
> > @@ -403,6 +406,7 @@ static void __init test_trampoline(void)
> >
> > static void __init test_branch_iform(void)
> > {
> > + int err;
> > unsigned int instr;
> > unsigned long addr;
> >
> > @@ -443,35 +447,35 @@ static void __init test_branch_iform(void)
> > check(instr_is_branch_to_addr(&instr, addr - 0x2000000));
> >
> > /* Branch to self, with link */
> > - instr = create_branch(&instr, addr, BRANCH_SET_LINK);
> > + err = create_branch(&instr, &instr, addr, BRANCH_SET_LINK);
> > check(instr_is_branch_to_addr(&instr, addr));
> >
> > /* Branch to self - 0x100, with link */
> > - instr = create_branch(&instr, addr - 0x100, BRANCH_SET_LINK);
> > + err = create_branch(&instr, &instr, addr - 0x100, BRANCH_SET_LINK);
> > check(instr_is_branch_to_addr(&instr, addr - 0x100));
> >
> > /* Branch to self + 0x100, no link */
> > - instr = create_branch(&instr, addr + 0x100, 0);
> > + err = create_branch(&instr, &instr, addr + 0x100, 0);
> > check(instr_is_branch_to_addr(&instr, addr + 0x100));
> >
> > /* Maximum relative negative offset, - 32 MB */
> > - instr = create_branch(&instr, addr - 0x2000000, BRANCH_SET_LINK);
> > + err = create_branch(&instr, &instr, addr - 0x2000000, BRANCH_SET_LINK);
> > check(instr_is_branch_to_addr(&instr, addr - 0x2000000));
> >
> > /* Out of range relative negative offset, - 32 MB + 4*/
> > - instr = create_branch(&instr, addr - 0x2000004, BRANCH_SET_LINK);
> > - check(instr == 0);
> > + err = create_branch(&instr, &instr, addr - 0x2000004, BRANCH_SET_LINK);
> > + check(err);
> >
> > /* Out of range relative positive offset, + 32 MB */
> > - instr = create_branch(&instr, addr + 0x2000000, BRANCH_SET_LINK);
> > - check(instr == 0);
> > + err = create_branch(&instr, &instr, addr + 0x2000000, BRANCH_SET_LINK);
> > + check(err);
> >
> > /* Unaligned target */
> > - instr = create_branch(&instr, addr + 3, BRANCH_SET_LINK);
> > - check(instr == 0);
> > + err = create_branch(&instr, &instr, addr + 3, BRANCH_SET_LINK);
> > + check(err);
> >
> > /* Check flags are masked correctly */
> > - instr = create_branch(&instr, addr, 0xFFFFFFFC);
> > + err = create_branch(&instr, &instr, addr, 0xFFFFFFFC);
> > check(instr_is_branch_to_addr(&instr, addr));
> > check(instr == 0x48000000);
> > }
> > @@ -480,16 +484,19 @@ static void __init test_create_function_call(void)
> > {
> > unsigned int *iptr;
> > unsigned long dest;
> > + unsigned int instr;
> >
> > /* Check we can create a function call */
> > iptr = (unsigned int *)ppc_function_entry(test_trampoline);
> > dest = ppc_function_entry(test_create_function_call);
> > - patch_instruction(iptr, create_branch(iptr, dest, BRANCH_SET_LINK));
> > + create_branch(&instr, iptr, dest, BRANCH_SET_LINK);
> > + patch_instruction(iptr, instr);
>
>
> Are we sure that `create_branch()` would always succeed here to do
> patch_instruction() ?
>
>
> > check(instr_is_branch_to_addr(iptr, dest));
> > }
> >
> > static void __init test_branch_bform(void)
> > {
> > + int err;
> > unsigned long addr;
> > unsigned int *iptr, instr, flags;
> >
> > @@ -525,35 +532,35 @@ static void __init test_branch_bform(void)
> > flags = 0x3ff000 | BRANCH_SET_LINK;
> >
> > /* Branch to self */
> > - instr = create_cond_branch(iptr, addr, flags);
> > + err = create_cond_branch(&instr, iptr, addr, flags);
> > check(instr_is_branch_to_addr(&instr, addr));
> >
> > /* Branch to self - 0x100 */
> > - instr = create_cond_branch(iptr, addr - 0x100, flags);
> > + err = create_cond_branch(&instr, iptr, addr - 0x100, flags);
> > check(instr_is_branch_to_addr(&instr, addr - 0x100));
> >
> > /* Branch to self + 0x100 */
> > - instr = create_cond_branch(iptr, addr + 0x100, flags);
> > + err = create_cond_branch(&instr, iptr, addr + 0x100, flags);
> > check(instr_is_branch_to_addr(&instr, addr + 0x100));
> >
> > /* Maximum relative negative offset, - 32 KB */
> > - instr = create_cond_branch(iptr, addr - 0x8000, flags);
> > + err = create_cond_branch(&instr, iptr, addr - 0x8000, flags);
> > check(instr_is_branch_to_addr(&instr, addr - 0x8000));
> >
> > /* Out of range relative negative offset, - 32 KB + 4*/
> > - instr = create_cond_branch(iptr, addr - 0x8004, flags);
> > - check(instr == 0);
> > + err = create_cond_branch(&instr, iptr, addr - 0x8004, flags);
> > + check(err);
> >
> > /* Out of range relative positive offset, + 32 KB */
> > - instr = create_cond_branch(iptr, addr + 0x8000, flags);
> > - check(instr == 0);
> > + err = create_cond_branch(&instr, iptr, addr + 0x8000, flags);
> > + check(err);
> >
> > /* Unaligned target */
> > - instr = create_cond_branch(iptr, addr + 3, flags);
> > - check(instr == 0);
> > + err = create_cond_branch(&instr, iptr, addr + 3, flags);
> > + check(err);
> >
> > /* Check flags are masked correctly */
> > - instr = create_cond_branch(iptr, addr, 0xFFFFFFFC);
> > + err = create_cond_branch(&instr, iptr, addr, 0xFFFFFFFC);
> > check(instr_is_branch_to_addr(&instr, addr));
> > check(instr == 0x43FF0000);
> > }
> > @@ -562,6 +569,7 @@ static void __init test_translate_branch(void)
> > {
> > unsigned long addr;
> > unsigned int *p, *q;
> > + unsigned int instr;
> > void *buf;
> >
> > buf = vmalloc(PAGE_ALIGN(0x2000000 + 1));
> > @@ -575,7 +583,8 @@ static void __init test_translate_branch(void)
> > patch_branch(p, addr, 0);
> > check(instr_is_branch_to_addr(p, addr));
> > q = p + 1;
> > - patch_instruction(q, translate_branch(q, p));
> > + translate_branch(&instr, q, p);
> > + patch_instruction(q, instr);
>
>
> same here and elsewhere in similar such places where we do patch_instruction()
I figured that if the existing code was not checking the return then
we probably did not need to add a check.
>
> -- Bala
>
> > check(instr_is_branch_to_addr(q, addr));
> >
> > /* Maximum negative case, move b . to addr + 32 MB */
> > @@ -583,7 +592,8 @@ static void __init test_translate_branch(void)
> > addr = (unsigned long)p;
> > patch_branch(p, addr, 0);
> > q = buf + 0x2000000;
> > - patch_instruction(q, translate_branch(q, p));
> > + translate_branch(&instr, q, p);
> > + patch_instruction(q, instr);
> > check(instr_is_branch_to_addr(p, addr));
> > check(instr_is_branch_to_addr(q, addr));
> > check(*q == 0x4a000000);
> > @@ -593,7 +603,8 @@ static void __init test_translate_branch(void)
> > addr = (unsigned long)p;
> > patch_branch(p, addr, 0);
> > q = buf + 4;
> > - patch_instruction(q, translate_branch(q, p));
> > + translate_branch(&instr, q, p);
> > + patch_instruction(q, instr);
> > check(instr_is_branch_to_addr(p, addr));
> > check(instr_is_branch_to_addr(q, addr));
> > check(*q == 0x49fffffc);
> > @@ -603,7 +614,8 @@ static void __init test_translate_branch(void)
> > addr = 0x1000000 + (unsigned long)buf;
> > patch_branch(p, addr, BRANCH_SET_LINK);
> > q = buf + 0x1400000;
> > - patch_instruction(q, translate_branch(q, p));
> > + translate_branch(&instr, q, p);
> > + patch_instruction(q, instr);
> > check(instr_is_branch_to_addr(p, addr));
> > check(instr_is_branch_to_addr(q, addr));
> >
> > @@ -612,7 +624,8 @@ static void __init test_translate_branch(void)
> > addr = 0x2000000 + (unsigned long)buf;
> > patch_branch(p, addr, 0);
> > q = buf + 4;
> > - patch_instruction(q, translate_branch(q, p));
> > + translate_branch(&instr, q, p);
> > + patch_instruction(q, instr);
> > check(instr_is_branch_to_addr(p, addr));
> > check(instr_is_branch_to_addr(q, addr));
> >
> > @@ -622,18 +635,22 @@ static void __init test_translate_branch(void)
> > /* Simple case, branch to self moved a little */
> > p = buf;
> > addr = (unsigned long)p;
> > - patch_instruction(p, create_cond_branch(p, addr, 0));
> > + create_cond_branch(&instr, p, addr, 0);
> > + patch_instruction(p, instr);
> > check(instr_is_branch_to_addr(p, addr));
> > q = p + 1;
> > - patch_instruction(q, translate_branch(q, p));
> > + translate_branch(&instr, q, p);
> > + patch_instruction(q, instr);
> > check(instr_is_branch_to_addr(q, addr));
> >
> > /* Maximum negative case, move b . to addr + 32 KB */
> > p = buf;
> > addr = (unsigned long)p;
> > - patch_instruction(p, create_cond_branch(p, addr, 0xFFFFFFFC));
> > + create_cond_branch(&instr, p, addr, 0xFFFFFFFC);
> > + patch_instruction(p, instr);
> > q = buf + 0x8000;
> > - patch_instruction(q, translate_branch(q, p));
> > + translate_branch(&instr, q, p);
> > + patch_instruction(q, instr);
> > check(instr_is_branch_to_addr(p, addr));
> > check(instr_is_branch_to_addr(q, addr));
> > check(*q == 0x43ff8000);
> > @@ -641,9 +658,11 @@ static void __init test_translate_branch(void)
> > /* Maximum positive case, move x to x - 32 KB + 4 */
> > p = buf + 0x8000;
> > addr = (unsigned long)p;
> > - patch_instruction(p, create_cond_branch(p, addr, 0xFFFFFFFC));
> > + create_cond_branch(&instr, p, addr, 0xFFFFFFFC);
> > + patch_instruction(p, instr);
> > q = buf + 4;
> > - patch_instruction(q, translate_branch(q, p));
> > + translate_branch(&instr, q, p);
> > + patch_instruction(q, instr);
> > check(instr_is_branch_to_addr(p, addr));
> > check(instr_is_branch_to_addr(q, addr));
> > check(*q == 0x43ff7ffc);
> > @@ -651,18 +670,22 @@ static void __init test_translate_branch(void)
> > /* Jump to x + 12 KB moved to x + 20 KB */
> > p = buf;
> > addr = 0x3000 + (unsigned long)buf;
> > - patch_instruction(p, create_cond_branch(p, addr, BRANCH_SET_LINK));
> > + create_cond_branch(&instr, p, addr, BRANCH_SET_LINK);
> > + patch_instruction(p, instr);
> > q = buf + 0x5000;
> > - patch_instruction(q, translate_branch(q, p));
> > + translate_branch(&instr, q, p);
> > + patch_instruction(q, instr);
> > check(instr_is_branch_to_addr(p, addr));
> > check(instr_is_branch_to_addr(q, addr));
> >
> > /* Jump to x + 8 KB moved to x - 8 KB + 4 */
> > p = buf + 0x2000;
> > addr = 0x4000 + (unsigned long)buf;
> > - patch_instruction(p, create_cond_branch(p, addr, 0));
> > + create_cond_branch(&instr, p, addr, 0);
> > + patch_instruction(p, instr);
> > q = buf + 4;
> > - patch_instruction(q, translate_branch(q, p));
> > + translate_branch(&instr, q, p);
> > + patch_instruction(q, instr);
> > check(instr_is_branch_to_addr(p, addr));
> > check(instr_is_branch_to_addr(q, addr));
> >
> > diff --git a/arch/powerpc/lib/feature-fixups.c b/arch/powerpc/lib/feature-
> > fixups.c
> > index 4ba634b89ce5..b129d7b4e7dd 100644
> > --- a/arch/powerpc/lib/feature-fixups.c
> > +++ b/arch/powerpc/lib/feature-fixups.c
> > @@ -44,6 +44,7 @@ static unsigned int *calc_addr(struct fixup_entry *fcur,
> > long offset)
> > static int patch_alt_instruction(unsigned int *src, unsigned int *dest,
> > unsigned int *alt_start, unsigned int
> > *alt_end)
> > {
> > + int err;
> > unsigned int instr;
> >
> > instr = *src;
> > @@ -53,8 +54,8 @@ static int patch_alt_instruction(unsigned int *src,
> > unsigned int *dest,
> >
> > /* Branch within the section doesn't need translating */
> > if (target < alt_start || target > alt_end) {
> > - instr = translate_branch(dest, src);
> > - if (!instr)
> > + err = translate_branch(&instr, dest, src);
> > + if (err)
> > return 1;
> > }
> > }
>
^ permalink raw reply
* Re: [PATCH v5 03/21] powerpc: Change calling convention for create_branch() et. al.
From: Balamuruhan S @ 2020-04-07 6:10 UTC (permalink / raw)
To: Jordan Niethe, linuxppc-dev; +Cc: alistair, dja, npiggin
In-Reply-To: <20200406080936.7180-4-jniethe5@gmail.com>
On Mon, 2020-04-06 at 18:09 +1000, Jordan Niethe wrote:
> create_branch(), create_cond_branch() and translate_branch() return the
> instruction that they create, or return 0 to signal an error. Seperate
s/seperate/separate
> these concerns in preparation for an instruction type that is not just
> an unsigned int. Fill the created instruction to a pointer passed as
> the first parameter to the function and use a non-zero return value to
> signify an error.
>
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> ---
> v5: New to series
> ---
> arch/powerpc/include/asm/code-patching.h | 12 +-
> arch/powerpc/kernel/optprobes.c | 24 ++--
> arch/powerpc/kernel/setup_32.c | 2 +-
> arch/powerpc/kernel/trace/ftrace.c | 24 ++--
> arch/powerpc/lib/code-patching.c | 133 +++++++++++++----------
> arch/powerpc/lib/feature-fixups.c | 5 +-
> 6 files changed, 117 insertions(+), 83 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/code-patching.h
> b/arch/powerpc/include/asm/code-patching.h
> index 898b54262881..351dda7215b6 100644
> --- a/arch/powerpc/include/asm/code-patching.h
> +++ b/arch/powerpc/include/asm/code-patching.h
> @@ -22,10 +22,10 @@
> #define BRANCH_ABSOLUTE 0x2
>
> bool is_offset_in_branch_range(long offset);
> -unsigned int create_branch(const unsigned int *addr,
> - unsigned long target, int flags);
> -unsigned int create_cond_branch(const unsigned int *addr,
> - unsigned long target, int flags);
> +int create_branch(unsigned int *instr, const unsigned int *addr,
> + unsigned long target, int flags);
> +int create_cond_branch(unsigned int *instr, const unsigned int *addr,
> + unsigned long target, int flags);
> int patch_branch(unsigned int *addr, unsigned long target, int flags);
> int patch_instruction(unsigned int *addr, unsigned int instr);
> int raw_patch_instruction(unsigned int *addr, unsigned int instr);
> @@ -60,8 +60,8 @@ int instr_is_relative_branch(unsigned int instr);
> int instr_is_relative_link_branch(unsigned int instr);
> int instr_is_branch_to_addr(const unsigned int *instr, unsigned long addr);
> unsigned long branch_target(const unsigned int *instr);
> -unsigned int translate_branch(const unsigned int *dest,
> - const unsigned int *src);
> +int translate_branch(unsigned int *instr, const unsigned int *dest,
> + const unsigned int *src);
> extern bool is_conditional_branch(unsigned int instr);
> #ifdef CONFIG_PPC_BOOK3E_64
> void __patch_exception(int exc, unsigned long addr);
> diff --git a/arch/powerpc/kernel/optprobes.c
> b/arch/powerpc/kernel/optprobes.c
> index 024f7aad1952..445b3dad82dc 100644
> --- a/arch/powerpc/kernel/optprobes.c
> +++ b/arch/powerpc/kernel/optprobes.c
> @@ -251,15 +251,17 @@ int arch_prepare_optimized_kprobe(struct
> optimized_kprobe *op, struct kprobe *p)
> goto error;
> }
>
> - branch_op_callback = create_branch((unsigned int *)buff +
> TMPL_CALL_HDLR_IDX,
> - (unsigned long)op_callback_addr,
> - BRANCH_SET_LINK);
> + rc = create_branch(&branch_op_callback,
> + (unsigned int *)buff + TMPL_CALL_HDLR_IDX,
> + (unsigned long)op_callback_addr,
> + BRANCH_SET_LINK);
>
> - branch_emulate_step = create_branch((unsigned int *)buff +
> TMPL_EMULATE_IDX,
> - (unsigned long)emulate_step_addr,
> - BRANCH_SET_LINK);
> + rc |= create_branch(&branch_emulate_step,
> + (unsigned int *)buff + TMPL_EMULATE_IDX,
> + (unsigned long)emulate_step_addr,
> + BRANCH_SET_LINK);
>
> - if (!branch_op_callback || !branch_emulate_step)
> + if (rc)
> goto error;
>
> patch_instruction(buff + TMPL_CALL_HDLR_IDX, branch_op_callback);
> @@ -305,6 +307,7 @@ int arch_check_optimized_kprobe(struct optimized_kprobe
> *op)
>
> void arch_optimize_kprobes(struct list_head *oplist)
> {
> + unsigned int instr;
> struct optimized_kprobe *op;
> struct optimized_kprobe *tmp;
>
> @@ -315,9 +318,10 @@ void arch_optimize_kprobes(struct list_head *oplist)
> */
> memcpy(op->optinsn.copied_insn, op->kp.addr,
> RELATIVEJUMP_SIZE);
> - patch_instruction(op->kp.addr,
> - create_branch((unsigned int *)op->kp.addr,
> - (unsigned long)op->optinsn.insn, 0));
> + create_branch(&instr,
> + (unsigned int *)op->kp.addr,
> + (unsigned long)op->optinsn.insn, 0);
> + patch_instruction(op->kp.addr, instr);
> list_del_init(&op->list);
> }
> }
> diff --git a/arch/powerpc/kernel/setup_32.c b/arch/powerpc/kernel/setup_32.c
> index 5b49b26eb154..c1bdd462c5c0 100644
> --- a/arch/powerpc/kernel/setup_32.c
> +++ b/arch/powerpc/kernel/setup_32.c
> @@ -88,7 +88,7 @@ notrace void __init machine_init(u64 dt_ptr)
>
> patch_instruction_site(&patch__memcpy_nocache, PPC_INST_NOP);
>
> - insn = create_cond_branch(addr, branch_target(addr), 0x820000);
> + create_cond_branch(&insn, addr, branch_target(addr), 0x820000);
> patch_instruction(addr, insn); /* replace b by bne cr0 */
>
> /* Do some early initialization based on the flat device tree */
> diff --git a/arch/powerpc/kernel/trace/ftrace.c
> b/arch/powerpc/kernel/trace/ftrace.c
> index 7ea0ca044b65..4dcb379133f8 100644
> --- a/arch/powerpc/kernel/trace/ftrace.c
> +++ b/arch/powerpc/kernel/trace/ftrace.c
> @@ -48,7 +48,7 @@ ftrace_call_replace(unsigned long ip, unsigned long addr,
> int link)
> addr = ppc_function_entry((void *)addr);
>
> /* if (link) set op to 'bl' else 'b' */
> - op = create_branch((unsigned int *)ip, addr, link ? 1 : 0);
> + create_branch(&op, (unsigned int *)ip, addr, link ? 1 : 0);
>
> return op;
> }
> @@ -89,10 +89,11 @@ ftrace_modify_code(unsigned long ip, unsigned int old,
> unsigned int new)
> */
> static int test_24bit_addr(unsigned long ip, unsigned long addr)
> {
> + unsigned int op;
> addr = ppc_function_entry((void *)addr);
>
> /* use the create_branch to verify that this offset can be branched */
> - return create_branch((unsigned int *)ip, addr, 0);
> + return create_branch(&op, (unsigned int *)ip, addr, 0) == 0;
> }
>
> static int is_bl_op(unsigned int op)
> @@ -287,6 +288,7 @@ __ftrace_make_nop(struct module *mod,
> static unsigned long find_ftrace_tramp(unsigned long ip)
> {
> int i;
> + unsigned int instr;
>
> /*
> * We have the compiler generated long_branch tramps at the end
> @@ -295,7 +297,8 @@ static unsigned long find_ftrace_tramp(unsigned long ip)
> for (i = NUM_FTRACE_TRAMPS - 1; i >= 0; i--)
> if (!ftrace_tramps[i])
> continue;
> - else if (create_branch((void *)ip, ftrace_tramps[i], 0))
> + else if (create_branch(&instr, (void *)ip,
> + ftrace_tramps[i], 0) == 0)
> return ftrace_tramps[i];
>
> return 0;
> @@ -324,6 +327,7 @@ static int setup_mcount_compiler_tramp(unsigned long
> tramp)
> {
> int i, op;
> unsigned long ptr;
> + unsigned int instr;
> static unsigned long ftrace_plt_tramps[NUM_FTRACE_TRAMPS];
>
> /* Is this a known long jump tramp? */
> @@ -366,7 +370,7 @@ static int setup_mcount_compiler_tramp(unsigned long
> tramp)
> #else
> ptr = ppc_global_function_entry((void *)ftrace_caller);
> #endif
> - if (!create_branch((void *)tramp, ptr, 0)) {
> + if (create_branch(&instr, (void *)tramp, ptr, 0)) {
> pr_debug("%ps is not reachable from existing mcount tramp\n",
> (void *)ptr);
> return -1;
> @@ -511,6 +515,7 @@ static int
> __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
> {
> unsigned int op[2];
> + unsigned int instr;
> void *ip = (void *)rec->ip;
> unsigned long entry, ptr, tramp;
> struct module *mod = rec->arch.mod;
> @@ -557,7 +562,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long
> addr)
> }
>
> /* Ensure branch is within 24 bits */
> - if (!create_branch(ip, tramp, BRANCH_SET_LINK)) {
> + if (create_branch(&instr, ip, tramp, BRANCH_SET_LINK)) {
> pr_err("Branch out of range\n");
> return -EINVAL;
> }
> @@ -574,6 +579,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long
> addr)
> static int
> __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
> {
> + int err;
> unsigned int op;
> unsigned long ip = rec->ip;
>
> @@ -594,9 +600,9 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long
> addr)
> }
>
> /* create the branch to the trampoline */
> - op = create_branch((unsigned int *)ip,
> - rec->arch.mod->arch.tramp, BRANCH_SET_LINK);
> - if (!op) {
> + err = create_branch(&op, (unsigned int *)ip,
> + rec->arch.mod->arch.tramp, BRANCH_SET_LINK);
> + if (!err) {
should this be `if (err)` as per your new changes ?
> pr_err("REL24 out of range!\n");
> return -EINVAL;
> }
> @@ -776,7 +782,7 @@ __ftrace_modify_call(struct dyn_ftrace *rec, unsigned
> long old_addr,
> }
>
> /* Ensure branch is within 24 bits */
> - if (!create_branch((unsigned int *)ip, tramp, BRANCH_SET_LINK)) {
> + if (create_branch(&op, (unsigned int *)ip, tramp, BRANCH_SET_LINK)) {
> pr_err("Branch out of range\n");
> return -EINVAL;
> }
> diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-
> patching.c
> index 3345f039a876..4af564aeedf1 100644
> --- a/arch/powerpc/lib/code-patching.c
> +++ b/arch/powerpc/lib/code-patching.c
> @@ -196,7 +196,10 @@ NOKPROBE_SYMBOL(patch_instruction);
>
> int patch_branch(unsigned int *addr, unsigned long target, int flags)
> {
> - return patch_instruction(addr, create_branch(addr, target, flags));
> + unsigned int instr;
> +
> + create_branch(&instr, addr, target, flags);
> + return patch_instruction(addr, instr);
> }
>
> bool is_offset_in_branch_range(long offset)
> @@ -243,30 +246,30 @@ bool is_conditional_branch(unsigned int instr)
> }
> NOKPROBE_SYMBOL(is_conditional_branch);
>
> -unsigned int create_branch(const unsigned int *addr,
> - unsigned long target, int flags)
> +int create_branch(unsigned int *instr,
> + const unsigned int *addr,
> + unsigned long target, int flags)
> {
> - unsigned int instruction;
> long offset;
>
> + *instr = 0;
> offset = target;
> if (! (flags & BRANCH_ABSOLUTE))
> offset = offset - (unsigned long)addr;
>
> /* Check we can represent the target in the instruction format */
> if (!is_offset_in_branch_range(offset))
> - return 0;
> + return 1;
>
> /* Mask out the flags and target, so they don't step on each other. */
> - instruction = 0x48000000 | (flags & 0x3) | (offset & 0x03FFFFFC);
> + *instr = 0x48000000 | (flags & 0x3) | (offset & 0x03FFFFFC);
>
> - return instruction;
> + return 0;
> }
>
> -unsigned int create_cond_branch(const unsigned int *addr,
> - unsigned long target, int flags)
> +int create_cond_branch(unsigned int *instr, const unsigned int *addr,
> + unsigned long target, int flags)
> {
> - unsigned int instruction;
> long offset;
>
> offset = target;
> @@ -275,12 +278,12 @@ unsigned int create_cond_branch(const unsigned int
> *addr,
>
> /* Check we can represent the target in the instruction format */
> if (offset < -0x8000 || offset > 0x7FFF || offset & 0x3)
> - return 0;
> + return 1;
>
> /* Mask out the flags and target, so they don't step on each other. */
> - instruction = 0x40000000 | (flags & 0x3FF0003) | (offset & 0xFFFC);
> + *instr = 0x40000000 | (flags & 0x3FF0003) | (offset & 0xFFFC);
>
> - return instruction;
> + return 0;
> }
>
> static unsigned int branch_opcode(unsigned int instr)
> @@ -361,18 +364,18 @@ int instr_is_branch_to_addr(const unsigned int *instr,
> unsigned long addr)
> return 0;
> }
>
> -unsigned int translate_branch(const unsigned int *dest, const unsigned int
> *src)
> +int translate_branch(unsigned int *instr, const unsigned int *dest, const
> unsigned int *src)
> {
> unsigned long target;
>
> target = branch_target(src);
>
> if (instr_is_branch_iform(*src))
> - return create_branch(dest, target, *src);
> + return create_branch(instr, dest, target, *src);
> else if (instr_is_branch_bform(*src))
> - return create_cond_branch(dest, target, *src);
> + return create_cond_branch(instr, dest, target, *src);
>
> - return 0;
> + return 1;
> }
>
> #ifdef CONFIG_PPC_BOOK3E_64
> @@ -403,6 +406,7 @@ static void __init test_trampoline(void)
>
> static void __init test_branch_iform(void)
> {
> + int err;
> unsigned int instr;
> unsigned long addr;
>
> @@ -443,35 +447,35 @@ static void __init test_branch_iform(void)
> check(instr_is_branch_to_addr(&instr, addr - 0x2000000));
>
> /* Branch to self, with link */
> - instr = create_branch(&instr, addr, BRANCH_SET_LINK);
> + err = create_branch(&instr, &instr, addr, BRANCH_SET_LINK);
> check(instr_is_branch_to_addr(&instr, addr));
>
> /* Branch to self - 0x100, with link */
> - instr = create_branch(&instr, addr - 0x100, BRANCH_SET_LINK);
> + err = create_branch(&instr, &instr, addr - 0x100, BRANCH_SET_LINK);
> check(instr_is_branch_to_addr(&instr, addr - 0x100));
>
> /* Branch to self + 0x100, no link */
> - instr = create_branch(&instr, addr + 0x100, 0);
> + err = create_branch(&instr, &instr, addr + 0x100, 0);
> check(instr_is_branch_to_addr(&instr, addr + 0x100));
>
> /* Maximum relative negative offset, - 32 MB */
> - instr = create_branch(&instr, addr - 0x2000000, BRANCH_SET_LINK);
> + err = create_branch(&instr, &instr, addr - 0x2000000, BRANCH_SET_LINK);
> check(instr_is_branch_to_addr(&instr, addr - 0x2000000));
>
> /* Out of range relative negative offset, - 32 MB + 4*/
> - instr = create_branch(&instr, addr - 0x2000004, BRANCH_SET_LINK);
> - check(instr == 0);
> + err = create_branch(&instr, &instr, addr - 0x2000004, BRANCH_SET_LINK);
> + check(err);
>
> /* Out of range relative positive offset, + 32 MB */
> - instr = create_branch(&instr, addr + 0x2000000, BRANCH_SET_LINK);
> - check(instr == 0);
> + err = create_branch(&instr, &instr, addr + 0x2000000, BRANCH_SET_LINK);
> + check(err);
>
> /* Unaligned target */
> - instr = create_branch(&instr, addr + 3, BRANCH_SET_LINK);
> - check(instr == 0);
> + err = create_branch(&instr, &instr, addr + 3, BRANCH_SET_LINK);
> + check(err);
>
> /* Check flags are masked correctly */
> - instr = create_branch(&instr, addr, 0xFFFFFFFC);
> + err = create_branch(&instr, &instr, addr, 0xFFFFFFFC);
> check(instr_is_branch_to_addr(&instr, addr));
> check(instr == 0x48000000);
> }
> @@ -480,16 +484,19 @@ static void __init test_create_function_call(void)
> {
> unsigned int *iptr;
> unsigned long dest;
> + unsigned int instr;
>
> /* Check we can create a function call */
> iptr = (unsigned int *)ppc_function_entry(test_trampoline);
> dest = ppc_function_entry(test_create_function_call);
> - patch_instruction(iptr, create_branch(iptr, dest, BRANCH_SET_LINK));
> + create_branch(&instr, iptr, dest, BRANCH_SET_LINK);
> + patch_instruction(iptr, instr);
Are we sure that `create_branch()` would always succeed here to do
patch_instruction() ?
> check(instr_is_branch_to_addr(iptr, dest));
> }
>
> static void __init test_branch_bform(void)
> {
> + int err;
> unsigned long addr;
> unsigned int *iptr, instr, flags;
>
> @@ -525,35 +532,35 @@ static void __init test_branch_bform(void)
> flags = 0x3ff000 | BRANCH_SET_LINK;
>
> /* Branch to self */
> - instr = create_cond_branch(iptr, addr, flags);
> + err = create_cond_branch(&instr, iptr, addr, flags);
> check(instr_is_branch_to_addr(&instr, addr));
>
> /* Branch to self - 0x100 */
> - instr = create_cond_branch(iptr, addr - 0x100, flags);
> + err = create_cond_branch(&instr, iptr, addr - 0x100, flags);
> check(instr_is_branch_to_addr(&instr, addr - 0x100));
>
> /* Branch to self + 0x100 */
> - instr = create_cond_branch(iptr, addr + 0x100, flags);
> + err = create_cond_branch(&instr, iptr, addr + 0x100, flags);
> check(instr_is_branch_to_addr(&instr, addr + 0x100));
>
> /* Maximum relative negative offset, - 32 KB */
> - instr = create_cond_branch(iptr, addr - 0x8000, flags);
> + err = create_cond_branch(&instr, iptr, addr - 0x8000, flags);
> check(instr_is_branch_to_addr(&instr, addr - 0x8000));
>
> /* Out of range relative negative offset, - 32 KB + 4*/
> - instr = create_cond_branch(iptr, addr - 0x8004, flags);
> - check(instr == 0);
> + err = create_cond_branch(&instr, iptr, addr - 0x8004, flags);
> + check(err);
>
> /* Out of range relative positive offset, + 32 KB */
> - instr = create_cond_branch(iptr, addr + 0x8000, flags);
> - check(instr == 0);
> + err = create_cond_branch(&instr, iptr, addr + 0x8000, flags);
> + check(err);
>
> /* Unaligned target */
> - instr = create_cond_branch(iptr, addr + 3, flags);
> - check(instr == 0);
> + err = create_cond_branch(&instr, iptr, addr + 3, flags);
> + check(err);
>
> /* Check flags are masked correctly */
> - instr = create_cond_branch(iptr, addr, 0xFFFFFFFC);
> + err = create_cond_branch(&instr, iptr, addr, 0xFFFFFFFC);
> check(instr_is_branch_to_addr(&instr, addr));
> check(instr == 0x43FF0000);
> }
> @@ -562,6 +569,7 @@ static void __init test_translate_branch(void)
> {
> unsigned long addr;
> unsigned int *p, *q;
> + unsigned int instr;
> void *buf;
>
> buf = vmalloc(PAGE_ALIGN(0x2000000 + 1));
> @@ -575,7 +583,8 @@ static void __init test_translate_branch(void)
> patch_branch(p, addr, 0);
> check(instr_is_branch_to_addr(p, addr));
> q = p + 1;
> - patch_instruction(q, translate_branch(q, p));
> + translate_branch(&instr, q, p);
> + patch_instruction(q, instr);
same here and elsewhere in similar such places where we do patch_instruction()
-- Bala
> check(instr_is_branch_to_addr(q, addr));
>
> /* Maximum negative case, move b . to addr + 32 MB */
> @@ -583,7 +592,8 @@ static void __init test_translate_branch(void)
> addr = (unsigned long)p;
> patch_branch(p, addr, 0);
> q = buf + 0x2000000;
> - patch_instruction(q, translate_branch(q, p));
> + translate_branch(&instr, q, p);
> + patch_instruction(q, instr);
> check(instr_is_branch_to_addr(p, addr));
> check(instr_is_branch_to_addr(q, addr));
> check(*q == 0x4a000000);
> @@ -593,7 +603,8 @@ static void __init test_translate_branch(void)
> addr = (unsigned long)p;
> patch_branch(p, addr, 0);
> q = buf + 4;
> - patch_instruction(q, translate_branch(q, p));
> + translate_branch(&instr, q, p);
> + patch_instruction(q, instr);
> check(instr_is_branch_to_addr(p, addr));
> check(instr_is_branch_to_addr(q, addr));
> check(*q == 0x49fffffc);
> @@ -603,7 +614,8 @@ static void __init test_translate_branch(void)
> addr = 0x1000000 + (unsigned long)buf;
> patch_branch(p, addr, BRANCH_SET_LINK);
> q = buf + 0x1400000;
> - patch_instruction(q, translate_branch(q, p));
> + translate_branch(&instr, q, p);
> + patch_instruction(q, instr);
> check(instr_is_branch_to_addr(p, addr));
> check(instr_is_branch_to_addr(q, addr));
>
> @@ -612,7 +624,8 @@ static void __init test_translate_branch(void)
> addr = 0x2000000 + (unsigned long)buf;
> patch_branch(p, addr, 0);
> q = buf + 4;
> - patch_instruction(q, translate_branch(q, p));
> + translate_branch(&instr, q, p);
> + patch_instruction(q, instr);
> check(instr_is_branch_to_addr(p, addr));
> check(instr_is_branch_to_addr(q, addr));
>
> @@ -622,18 +635,22 @@ static void __init test_translate_branch(void)
> /* Simple case, branch to self moved a little */
> p = buf;
> addr = (unsigned long)p;
> - patch_instruction(p, create_cond_branch(p, addr, 0));
> + create_cond_branch(&instr, p, addr, 0);
> + patch_instruction(p, instr);
> check(instr_is_branch_to_addr(p, addr));
> q = p + 1;
> - patch_instruction(q, translate_branch(q, p));
> + translate_branch(&instr, q, p);
> + patch_instruction(q, instr);
> check(instr_is_branch_to_addr(q, addr));
>
> /* Maximum negative case, move b . to addr + 32 KB */
> p = buf;
> addr = (unsigned long)p;
> - patch_instruction(p, create_cond_branch(p, addr, 0xFFFFFFFC));
> + create_cond_branch(&instr, p, addr, 0xFFFFFFFC);
> + patch_instruction(p, instr);
> q = buf + 0x8000;
> - patch_instruction(q, translate_branch(q, p));
> + translate_branch(&instr, q, p);
> + patch_instruction(q, instr);
> check(instr_is_branch_to_addr(p, addr));
> check(instr_is_branch_to_addr(q, addr));
> check(*q == 0x43ff8000);
> @@ -641,9 +658,11 @@ static void __init test_translate_branch(void)
> /* Maximum positive case, move x to x - 32 KB + 4 */
> p = buf + 0x8000;
> addr = (unsigned long)p;
> - patch_instruction(p, create_cond_branch(p, addr, 0xFFFFFFFC));
> + create_cond_branch(&instr, p, addr, 0xFFFFFFFC);
> + patch_instruction(p, instr);
> q = buf + 4;
> - patch_instruction(q, translate_branch(q, p));
> + translate_branch(&instr, q, p);
> + patch_instruction(q, instr);
> check(instr_is_branch_to_addr(p, addr));
> check(instr_is_branch_to_addr(q, addr));
> check(*q == 0x43ff7ffc);
> @@ -651,18 +670,22 @@ static void __init test_translate_branch(void)
> /* Jump to x + 12 KB moved to x + 20 KB */
> p = buf;
> addr = 0x3000 + (unsigned long)buf;
> - patch_instruction(p, create_cond_branch(p, addr, BRANCH_SET_LINK));
> + create_cond_branch(&instr, p, addr, BRANCH_SET_LINK);
> + patch_instruction(p, instr);
> q = buf + 0x5000;
> - patch_instruction(q, translate_branch(q, p));
> + translate_branch(&instr, q, p);
> + patch_instruction(q, instr);
> check(instr_is_branch_to_addr(p, addr));
> check(instr_is_branch_to_addr(q, addr));
>
> /* Jump to x + 8 KB moved to x - 8 KB + 4 */
> p = buf + 0x2000;
> addr = 0x4000 + (unsigned long)buf;
> - patch_instruction(p, create_cond_branch(p, addr, 0));
> + create_cond_branch(&instr, p, addr, 0);
> + patch_instruction(p, instr);
> q = buf + 4;
> - patch_instruction(q, translate_branch(q, p));
> + translate_branch(&instr, q, p);
> + patch_instruction(q, instr);
> check(instr_is_branch_to_addr(p, addr));
> check(instr_is_branch_to_addr(q, addr));
>
> diff --git a/arch/powerpc/lib/feature-fixups.c b/arch/powerpc/lib/feature-
> fixups.c
> index 4ba634b89ce5..b129d7b4e7dd 100644
> --- a/arch/powerpc/lib/feature-fixups.c
> +++ b/arch/powerpc/lib/feature-fixups.c
> @@ -44,6 +44,7 @@ static unsigned int *calc_addr(struct fixup_entry *fcur,
> long offset)
> static int patch_alt_instruction(unsigned int *src, unsigned int *dest,
> unsigned int *alt_start, unsigned int
> *alt_end)
> {
> + int err;
> unsigned int instr;
>
> instr = *src;
> @@ -53,8 +54,8 @@ static int patch_alt_instruction(unsigned int *src,
> unsigned int *dest,
>
> /* Branch within the section doesn't need translating */
> if (target < alt_start || target > alt_end) {
> - instr = translate_branch(dest, src);
> - if (!instr)
> + err = translate_branch(&instr, dest, src);
> + if (err)
> return 1;
> }
> }
^ permalink raw reply
* Re: [PATCH v12 5/8] powerpc/64: make buildable without CONFIG_COMPAT
From: Christophe Leroy @ 2020-04-07 5:50 UTC (permalink / raw)
To: Michal Suchanek, linuxppc-dev
Cc: Mark Rutland, Gustavo Luiz Duarte, Peter Zijlstra,
Sebastian Andrzej Siewior, linux-kernel, Paul Mackerras,
Jiri Olsa, Rob Herring, Michael Neuling, Mauro Carvalho Chehab,
Masahiro Yamada, Nayna Jain, Alexander Shishkin, Ingo Molnar,
Allison Randal, Jordan Niethe, Valentin Schneider, Arnd Bergmann,
Arnaldo Carvalho de Melo, Alexander Viro, Jonathan Cameron,
Namhyung Kim, Thomas Gleixner, Andy Shevchenko, Hari Bathini,
Greg Kroah-Hartman, Nicholas Piggin, Claudio Carvalho,
Eric Richter, Eric W. Biederman, linux-fsdevel, David S. Miller,
Thiago Jung Bauermann
In-Reply-To: <e5619617020ef3a1f54f0c076e7d74cb9ec9f3bf.1584699455.git.msuchanek@suse.de>
Le 20/03/2020 à 11:20, Michal Suchanek a écrit :
> There are numerous references to 32bit functions in generic and 64bit
> code so ifdef them out.
>
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
> v2:
> - fix 32bit ifdef condition in signal.c
> - simplify the compat ifdef condition in vdso.c - 64bit is redundant
> - simplify the compat ifdef condition in callchain.c - 64bit is redundant
> v3:
> - use IS_ENABLED and maybe_unused where possible
> - do not ifdef declarations
> - clean up Makefile
> v4:
> - further makefile cleanup
> - simplify is_32bit_task conditions
> - avoid ifdef in condition by using return
> v5:
> - avoid unreachable code on 32bit
> - make is_current_64bit constant on !COMPAT
> - add stub perf_callchain_user_32 to avoid some ifdefs
> v6:
> - consolidate current_is_64bit
> v7:
> - remove leftover perf_callchain_user_32 stub from previous series version
> v8:
> - fix build again - too trigger-happy with stub removal
> - remove a vdso.c hunk that causes warning according to kbuild test robot
> v9:
> - removed current_is_64bit in previous patch
> v10:
> - rebase on top of 70ed86f4de5bd
> ---
> arch/powerpc/include/asm/thread_info.h | 4 ++--
> arch/powerpc/kernel/Makefile | 6 +++---
> arch/powerpc/kernel/entry_64.S | 2 ++
> arch/powerpc/kernel/signal.c | 3 +--
> arch/powerpc/kernel/syscall_64.c | 6 ++----
> arch/powerpc/kernel/vdso.c | 3 ++-
> arch/powerpc/perf/callchain.c | 8 +++++++-
> 7 files changed, 19 insertions(+), 13 deletions(-)
>
[...]
> diff --git a/arch/powerpc/kernel/syscall_64.c b/arch/powerpc/kernel/syscall_64.c
> index 87d95b455b83..2dcbfe38f5ac 100644
> --- a/arch/powerpc/kernel/syscall_64.c
> +++ b/arch/powerpc/kernel/syscall_64.c
> @@ -24,7 +24,6 @@ notrace long system_call_exception(long r3, long r4, long r5,
> long r6, long r7, long r8,
> unsigned long r0, struct pt_regs *regs)
> {
> - unsigned long ti_flags;
> syscall_fn f;
>
> if (IS_ENABLED(CONFIG_PPC_IRQ_SOFT_MASK_DEBUG))
> @@ -68,8 +67,7 @@ notrace long system_call_exception(long r3, long r4, long r5,
>
> local_irq_enable();
>
> - ti_flags = current_thread_info()->flags;
> - if (unlikely(ti_flags & _TIF_SYSCALL_DOTRACE)) {
> + if (unlikely(current_thread_info()->flags & _TIF_SYSCALL_DOTRACE)) {
> /*
> * We use the return value of do_syscall_trace_enter() as the
> * syscall number. If the syscall was rejected for any reason
> @@ -94,7 +92,7 @@ notrace long system_call_exception(long r3, long r4, long r5,
> /* May be faster to do array_index_nospec? */
> barrier_nospec();
>
> - if (unlikely(ti_flags & _TIF_32BIT)) {
> + if (unlikely(is_32bit_task())) {
is_compat() should be used here instead, because we dont want to use
compat_sys_call_table() on PPC32.
> f = (void *)compat_sys_call_table[r0];
>
> r3 &= 0x00000000ffffffffULL;
Christophe
^ permalink raw reply
* Re: [PATCH v3 15/15] powerpc: make unrecoverable NMIs die instead of panic
From: Christophe Leroy @ 2020-04-07 5:46 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: Mahesh Salgaonkar, Ganesh Goudar
In-Reply-To: <20200407051636.648369-16-npiggin@gmail.com>
Le 07/04/2020 à 07:16, Nicholas Piggin a écrit :
> System Reset and Machine Check interrupts that are not recoverable due
> to being nested or interrupting when RI=0 currently panic. This is
> not necessary, and can often just kill the current context and recover.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Reviewed-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> arch/powerpc/kernel/traps.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> index 1beae89bb871..afed3de33a9a 100644
> --- a/arch/powerpc/kernel/traps.c
> +++ b/arch/powerpc/kernel/traps.c
> @@ -513,11 +513,11 @@ void system_reset_exception(struct pt_regs *regs)
> #ifdef CONFIG_PPC_BOOK3S_64
> BUG_ON(get_paca()->in_nmi == 0);
> if (get_paca()->in_nmi > 1)
> - nmi_panic(regs, "Unrecoverable nested System Reset");
> + die("Unrecoverable nested System Reset", regs, SIGABRT);
> #endif
> /* Must die if the interrupt is not recoverable */
> if (!(regs->msr & MSR_RI))
> - nmi_panic(regs, "Unrecoverable System Reset");
> + die("Unrecoverable System Reset", regs, SIGABRT);
>
> if (saved_hsrrs) {
> mtspr(SPRN_HSRR0, hsrr0);
> @@ -858,7 +858,7 @@ void machine_check_exception(struct pt_regs *regs)
> bail:
> /* Must die if the interrupt is not recoverable */
> if (!(regs->msr & MSR_RI))
> - nmi_panic(regs, "Unrecoverable Machine check");
> + die("Unrecoverable Machine check", regs, SIGBUS);
> }
>
> void SMIException(struct pt_regs *regs)
>
^ permalink raw reply
* Re: [PATCH v3 14/15] powerpc/64s: system reset do not trace
From: Christophe Leroy @ 2020-04-07 5:45 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
Cc: Mahesh Salgaonkar, Naveen N . Rao, Ganesh Goudar
In-Reply-To: <20200407051636.648369-15-npiggin@gmail.com>
Le 07/04/2020 à 07:16, Nicholas Piggin a écrit :
> Similarly to the previous patch, do not trace system reset. This code
> is used when there is a crash or hang, and tracing disturbs the system
> more and has been known to crash in the crash handling path.
>
> Acked-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Reviewed-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> arch/powerpc/kernel/traps.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> index 9f221772eb73..1beae89bb871 100644
> --- a/arch/powerpc/kernel/traps.c
> +++ b/arch/powerpc/kernel/traps.c
> @@ -443,6 +443,9 @@ void system_reset_exception(struct pt_regs *regs)
> unsigned long hsrr0, hsrr1;
> bool nested = in_nmi();
> bool saved_hsrrs = false;
> + u8 ftrace_enabled = this_cpu_get_ftrace_enabled();
> +
> + this_cpu_set_ftrace_enabled(0);
>
> /*
> * Avoid crashes in case of nested NMI exceptions. Recoverability
> @@ -524,6 +527,8 @@ void system_reset_exception(struct pt_regs *regs)
> if (!nested)
> nmi_exit();
>
> + this_cpu_set_ftrace_enabled(ftrace_enabled);
> +
> /* What should we do here? We could issue a shutdown or hard reset. */
> }
>
>
^ permalink raw reply
* Re: [PATCH v3 13/15] powerpc/64s: machine check do not trace real-mode handler
From: Christophe Leroy @ 2020-04-07 5:45 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
Cc: Mahesh Salgaonkar, Naveen N . Rao, Ganesh Goudar
In-Reply-To: <20200407051636.648369-14-npiggin@gmail.com>
Le 07/04/2020 à 07:16, Nicholas Piggin a écrit :
> Rather than notrace annotations throughout a significant part of the
> machine check code across kernel/ pseries/ and powernv/ which can
> easily be broken and is infrequently tested, use paca->ftrace_enabled
> to blanket-disable tracing of the real-mode non-maskable handler.
>
> Acked-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Reviewed-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> arch/powerpc/kernel/mce.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/mce.c b/arch/powerpc/kernel/mce.c
> index be7e3f92a7b5..fd90c0eda229 100644
> --- a/arch/powerpc/kernel/mce.c
> +++ b/arch/powerpc/kernel/mce.c
> @@ -16,6 +16,7 @@
> #include <linux/export.h>
> #include <linux/irq_work.h>
> #include <linux/extable.h>
> +#include <linux/ftrace.h>
>
> #include <asm/machdep.h>
> #include <asm/mce.h>
> @@ -571,10 +572,14 @@ EXPORT_SYMBOL_GPL(machine_check_print_event_info);
> *
> * regs->nip and regs->msr contains srr0 and ssr1.
> */
> -long machine_check_early(struct pt_regs *regs)
> +long notrace machine_check_early(struct pt_regs *regs)
> {
> long handled = 0;
> bool nested = in_nmi();
> + u8 ftrace_enabled = this_cpu_get_ftrace_enabled();
> +
> + this_cpu_set_ftrace_enabled(0);
> +
> if (!nested)
> nmi_enter();
>
> @@ -589,6 +594,8 @@ long machine_check_early(struct pt_regs *regs)
> if (!nested)
> nmi_exit();
>
> + this_cpu_set_ftrace_enabled(ftrace_enabled);
> +
> return handled;
> }
>
>
^ 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