* [PATCH 2/5] powerpc/64s: Add new security feature flags for count cache flush
2018-07-23 15:07 [PATCH 1/5] powerpc/asm: Add a patch_site macro & helpers for patching instructions Michael Ellerman
@ 2018-07-23 15:07 ` Michael Ellerman
2018-07-23 15:07 ` [PATCH 3/5] powerpc/64s: Add support for software " Michael Ellerman
` (4 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Michael Ellerman @ 2018-07-23 15:07 UTC (permalink / raw)
To: linuxppc-dev
Add security feature flags to indicate the need for software to flush
the count cache on context switch, and for the presence of a hardware
assisted count cache flush.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/include/asm/security_features.h | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/powerpc/include/asm/security_features.h b/arch/powerpc/include/asm/security_features.h
index 44989b22383c..a0d47bc18a5c 100644
--- a/arch/powerpc/include/asm/security_features.h
+++ b/arch/powerpc/include/asm/security_features.h
@@ -59,6 +59,9 @@ static inline bool security_ftr_enabled(unsigned long feature)
// Indirect branch prediction cache disabled
#define SEC_FTR_COUNT_CACHE_DISABLED 0x0000000000000020ull
+// bcctr 2,0,0 triggers a hardware assisted count cache flush
+#define SEC_FTR_BCCTR_FLUSH_ASSIST 0x0000000000000800ull
+
// Features indicating need for Spectre/Meltdown mitigations
@@ -74,6 +77,9 @@ static inline bool security_ftr_enabled(unsigned long feature)
// Firmware configuration indicates user favours security over performance
#define SEC_FTR_FAVOUR_SECURITY 0x0000000000000200ull
+// Software required to flush count cache on context switch
+#define SEC_FTR_FLUSH_COUNT_CACHE 0x0000000000000400ull
+
// Features enabled by default
#define SEC_FTR_DEFAULT \
--
2.14.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 3/5] powerpc/64s: Add support for software count cache flush
2018-07-23 15:07 [PATCH 1/5] powerpc/asm: Add a patch_site macro & helpers for patching instructions Michael Ellerman
2018-07-23 15:07 ` [PATCH 2/5] powerpc/64s: Add new security feature flags for count cache flush Michael Ellerman
@ 2018-07-23 15:07 ` Michael Ellerman
2018-07-23 15:07 ` [PATCH 4/5] powerpc/pseries: Query hypervisor for count cache flush settings Michael Ellerman
` (3 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Michael Ellerman @ 2018-07-23 15:07 UTC (permalink / raw)
To: linuxppc-dev
Some CPU revisions support a mode where the count cache needs to be
flushed by software on context switch. Additionally some revisions may
have a hardware accelerated flush, in which case the software flush
sequence can be shortened.
If we detect the appropriate flag from firmware we patch a branch
into _switch() which takes us to a count cache flush sequence.
That sequence in turn may be patched to return early if we detect that
the CPU supports accelerating the flush sequence in hardware.
Add debugfs support for reporting the state of the flush, as well as
runtime disabling it.
And modify the spectre_v2 sysfs file to report the state of the
software flush.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/include/asm/asm-prototypes.h | 6 ++
arch/powerpc/include/asm/security_features.h | 1 +
arch/powerpc/kernel/entry_64.S | 54 ++++++++++++++++
arch/powerpc/kernel/security.c | 96 ++++++++++++++++++++++++++--
4 files changed, 152 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/include/asm/asm-prototypes.h b/arch/powerpc/include/asm/asm-prototypes.h
index 769567b66c0c..70fdc5b9b9fb 100644
--- a/arch/powerpc/include/asm/asm-prototypes.h
+++ b/arch/powerpc/include/asm/asm-prototypes.h
@@ -143,4 +143,10 @@ struct kvm_vcpu;
void _kvmppc_restore_tm_pr(struct kvm_vcpu *vcpu, u64 guest_msr);
void _kvmppc_save_tm_pr(struct kvm_vcpu *vcpu, u64 guest_msr);
+/* Patch sites */
+extern s32 patch__call_flush_count_cache;
+extern s32 patch__flush_count_cache_return;
+
+extern long flush_count_cache;
+
#endif /* _ASM_POWERPC_ASM_PROTOTYPES_H */
diff --git a/arch/powerpc/include/asm/security_features.h b/arch/powerpc/include/asm/security_features.h
index a0d47bc18a5c..759597bf0fd8 100644
--- a/arch/powerpc/include/asm/security_features.h
+++ b/arch/powerpc/include/asm/security_features.h
@@ -22,6 +22,7 @@ enum stf_barrier_type {
void setup_stf_barrier(void);
void do_stf_barrier_fixups(enum stf_barrier_type types);
+void setup_count_cache_flush(void);
static inline void security_ftr_set(unsigned long feature)
{
diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index 0357f87a013c..017cf70f01d7 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -25,6 +25,7 @@
#include <asm/page.h>
#include <asm/mmu.h>
#include <asm/thread_info.h>
+#include <asm/code-patching-asm.h>
#include <asm/ppc_asm.h>
#include <asm/asm-offsets.h>
#include <asm/cputable.h>
@@ -504,6 +505,57 @@ _GLOBAL(ret_from_kernel_thread)
li r3,0
b .Lsyscall_exit
+#ifdef CONFIG_PPC_BOOK3S_64
+
+#define FLUSH_COUNT_CACHE \
+1: nop; \
+ patch_site 1b, patch__call_flush_count_cache
+
+
+#define BCCTR_FLUSH .long 0x4c400420
+
+.macro nops number
+ .rept \number
+ nop
+ .endr
+.endm
+
+.balign 32
+.global flush_count_cache
+flush_count_cache:
+ /* Save LR into r9 */
+ mflr r9
+
+ .rept 64
+ bl .+4
+ .endr
+ b 1f
+ nops 6
+
+ .balign 32
+ /* Restore LR */
+1: mtlr r9
+ li r9,0x7fff
+ mtctr r9
+
+ BCCTR_FLUSH
+
+2: nop
+ patch_site 2b patch__flush_count_cache_return
+
+ nops 3
+
+ .rept 278
+ .balign 32
+ BCCTR_FLUSH
+ nops 7
+ .endr
+
+ blr
+#else
+#define FLUSH_COUNT_CACHE
+#endif /* CONFIG_PPC_BOOK3S_64 */
+
/*
* This routine switches between two different tasks. The process
* state of one is saved on its kernel stack. Then the state
@@ -535,6 +587,8 @@ _GLOBAL(_switch)
std r23,_CCR(r1)
std r1,KSP(r3) /* Set old stack pointer */
+ FLUSH_COUNT_CACHE
+
/*
* On SMP kernels, care must be taken because a task may be
* scheduled off CPUx and on to CPUy. Memory ordering must be
diff --git a/arch/powerpc/kernel/security.c b/arch/powerpc/kernel/security.c
index 4cb8f1f7b593..fa9366b53eb7 100644
--- a/arch/powerpc/kernel/security.c
+++ b/arch/powerpc/kernel/security.c
@@ -8,6 +8,8 @@
#include <linux/device.h>
#include <linux/seq_buf.h>
+#include <asm/asm-prototypes.h>
+#include <asm/code-patching.h>
#include <asm/debugfs.h>
#include <asm/security_features.h>
#include <asm/setup.h>
@@ -15,6 +17,13 @@
unsigned long powerpc_security_features __read_mostly = SEC_FTR_DEFAULT;
+enum count_cache_flush_type {
+ COUNT_CACHE_FLUSH_NONE = 0x1,
+ COUNT_CACHE_FLUSH_SW = 0x2,
+ COUNT_CACHE_FLUSH_HW = 0x4,
+};
+static enum count_cache_flush_type count_cache_flush_type;
+
bool barrier_nospec_enabled;
static void enable_barrier_nospec(bool enable)
@@ -147,17 +156,29 @@ ssize_t cpu_show_spectre_v2(struct device *dev, struct device_attribute *attr, c
bcs = security_ftr_enabled(SEC_FTR_BCCTRL_SERIALISED);
ccd = security_ftr_enabled(SEC_FTR_COUNT_CACHE_DISABLED);
- if (bcs || ccd) {
+ if (bcs || ccd || count_cache_flush_type != COUNT_CACHE_FLUSH_NONE) {
+ bool comma = false;
seq_buf_printf(&s, "Mitigation: ");
- if (bcs)
+ if (bcs) {
seq_buf_printf(&s, "Indirect branch serialisation (kernel only)");
+ comma = true;
+ }
+
+ if (ccd) {
+ if (comma)
+ seq_buf_printf(&s, ", ");
+ seq_buf_printf(&s, "Indirect branch cache disabled");
+ comma = true;
+ }
- if (bcs && ccd)
+ if (comma)
seq_buf_printf(&s, ", ");
- if (ccd)
- seq_buf_printf(&s, "Indirect branch cache disabled");
+ seq_buf_printf(&s, "Software count cache flush");
+
+ if (count_cache_flush_type == COUNT_CACHE_FLUSH_HW)
+ seq_buf_printf(&s, "(hardware accelerated)");
} else
seq_buf_printf(&s, "Vulnerable");
@@ -313,3 +334,68 @@ static __init int stf_barrier_debugfs_init(void)
}
device_initcall(stf_barrier_debugfs_init);
#endif /* CONFIG_DEBUG_FS */
+
+static void toggle_count_cache_flush(bool enable)
+{
+ if (!enable || !security_ftr_enabled(SEC_FTR_FLUSH_COUNT_CACHE)) {
+ patch_instruction_site(&patch__call_flush_count_cache, PPC_INST_NOP);
+ count_cache_flush_type = COUNT_CACHE_FLUSH_NONE;
+ pr_info("count-cache-flush: software flush disabled.\n");
+ return;
+ }
+
+ patch_branch_site(&patch__call_flush_count_cache,
+ (u64)&flush_count_cache, BRANCH_SET_LINK);
+
+ if (!security_ftr_enabled(SEC_FTR_BCCTR_FLUSH_ASSIST)) {
+ count_cache_flush_type = COUNT_CACHE_FLUSH_SW;
+ pr_info("count-cache-flush: full software flush sequence enabled.\n");
+ return;
+ }
+
+ patch_instruction_site(&patch__flush_count_cache_return, PPC_INST_BLR);
+ count_cache_flush_type = COUNT_CACHE_FLUSH_HW;
+ pr_info("count-cache-flush: hardware assisted flush sequence enabled\n");
+}
+
+void setup_count_cache_flush(void)
+{
+ toggle_count_cache_flush(true);
+}
+
+#ifdef CONFIG_DEBUG_FS
+static int count_cache_flush_set(void *data, u64 val)
+{
+ bool enable;
+
+ if (val == 1)
+ enable = true;
+ else if (val == 0)
+ enable = false;
+ else
+ return -EINVAL;
+
+ toggle_count_cache_flush(enable);
+
+ return 0;
+}
+
+static int count_cache_flush_get(void *data, u64 *val)
+{
+ if (count_cache_flush_type == COUNT_CACHE_FLUSH_NONE)
+ *val = 0;
+ else
+ *val = 1;
+
+ return 0;
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(fops_count_cache_flush, count_cache_flush_get, count_cache_flush_set, "%llu\n");
+
+static __init int count_cache_flush_debugfs_init(void)
+{
+ debugfs_create_file("count_cache_flush", 0600, powerpc_debugfs_root, NULL, &fops_count_cache_flush);
+ return 0;
+}
+device_initcall(count_cache_flush_debugfs_init);
+#endif /* CONFIG_DEBUG_FS */
--
2.14.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 4/5] powerpc/pseries: Query hypervisor for count cache flush settings
2018-07-23 15:07 [PATCH 1/5] powerpc/asm: Add a patch_site macro & helpers for patching instructions Michael Ellerman
2018-07-23 15:07 ` [PATCH 2/5] powerpc/64s: Add new security feature flags for count cache flush Michael Ellerman
2018-07-23 15:07 ` [PATCH 3/5] powerpc/64s: Add support for software " Michael Ellerman
@ 2018-07-23 15:07 ` Michael Ellerman
2018-07-23 15:07 ` [PATCH 5/5] powerpc/powernv: Query firmware " Michael Ellerman
` (2 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Michael Ellerman @ 2018-07-23 15:07 UTC (permalink / raw)
To: linuxppc-dev
Use the existing hypercall to determine the appropriate settings for
the count cache flush, and then call the generic powerpc code to set
it up based on the security feature flags.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/include/asm/hvcall.h | 2 ++
arch/powerpc/platforms/pseries/setup.c | 7 +++++++
2 files changed, 9 insertions(+)
diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
index 662c8347d699..a0b17f9f1ea4 100644
--- a/arch/powerpc/include/asm/hvcall.h
+++ b/arch/powerpc/include/asm/hvcall.h
@@ -342,10 +342,12 @@
#define H_CPU_CHAR_BRANCH_HINTS_HONORED (1ull << 58) // IBM bit 5
#define H_CPU_CHAR_THREAD_RECONFIG_CTRL (1ull << 57) // IBM bit 6
#define H_CPU_CHAR_COUNT_CACHE_DISABLED (1ull << 56) // IBM bit 7
+#define H_CPU_CHAR_BCCTR_FLUSH_ASSIST (1ull << 54) // IBM bit 9
#define H_CPU_BEHAV_FAVOUR_SECURITY (1ull << 63) // IBM bit 0
#define H_CPU_BEHAV_L1D_FLUSH_PR (1ull << 62) // IBM bit 1
#define H_CPU_BEHAV_BNDS_CHK_SPEC_BAR (1ull << 61) // IBM bit 2
+#define H_CPU_BEHAV_FLUSH_COUNT_CACHE (1ull << 58) // IBM bit 5
/* Flag values used in H_REGISTER_PROC_TBL hcall */
#define PROC_TABLE_OP_MASK 0x18
diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
index 139f0af6c3d9..04805a79cbda 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -484,6 +484,12 @@ static void init_cpu_char_feature_flags(struct h_cpu_char_result *result)
if (result->character & H_CPU_CHAR_COUNT_CACHE_DISABLED)
security_ftr_set(SEC_FTR_COUNT_CACHE_DISABLED);
+ if (result->character & H_CPU_CHAR_BCCTR_FLUSH_ASSIST)
+ security_ftr_set(SEC_FTR_BCCTR_FLUSH_ASSIST);
+
+ if (result->behaviour & H_CPU_BEHAV_FLUSH_COUNT_CACHE)
+ security_ftr_set(SEC_FTR_FLUSH_COUNT_CACHE);
+
/*
* The features below are enabled by default, so we instead look to see
* if firmware has *disabled* them, and clear them if so.
@@ -535,6 +541,7 @@ void pseries_setup_rfi_flush(void)
setup_rfi_flush(types, enable);
setup_barrier_nospec();
+ setup_count_cache_flush();
}
#ifdef CONFIG_PCI_IOV
--
2.14.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 5/5] powerpc/powernv: Query firmware for count cache flush settings
2018-07-23 15:07 [PATCH 1/5] powerpc/asm: Add a patch_site macro & helpers for patching instructions Michael Ellerman
` (2 preceding siblings ...)
2018-07-23 15:07 ` [PATCH 4/5] powerpc/pseries: Query hypervisor for count cache flush settings Michael Ellerman
@ 2018-07-23 15:07 ` Michael Ellerman
2018-08-08 14:25 ` [1/5] powerpc/asm: Add a patch_site macro & helpers for patching instructions Michael Ellerman
2018-08-08 16:30 ` [PATCH 1/5] " Christophe LEROY
5 siblings, 0 replies; 9+ messages in thread
From: Michael Ellerman @ 2018-07-23 15:07 UTC (permalink / raw)
To: linuxppc-dev
Look for fw-features properties to determine the appropriate settings
for the count cache flush, and then call the generic powerpc code to
set it up based on the security feature flags.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/platforms/powernv/setup.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/powerpc/platforms/powernv/setup.c b/arch/powerpc/platforms/powernv/setup.c
index f96df0a25d05..0988d050becd 100644
--- a/arch/powerpc/platforms/powernv/setup.c
+++ b/arch/powerpc/platforms/powernv/setup.c
@@ -78,6 +78,12 @@ static void init_fw_feat_flags(struct device_node *np)
if (fw_feature_is("enabled", "fw-count-cache-disabled", np))
security_ftr_set(SEC_FTR_COUNT_CACHE_DISABLED);
+ if (fw_feature_is("enabled", "fw-count-cache-flush-bcctr2,0,0", np))
+ security_ftr_set(SEC_FTR_BCCTR_FLUSH_ASSIST);
+
+ if (fw_feature_is("enabled", "needs-count-cache-flush-on-context-switch", np))
+ security_ftr_set(SEC_FTR_FLUSH_COUNT_CACHE);
+
/*
* The features below are enabled by default, so we instead look to see
* if firmware has *disabled* them, and clear them if so.
@@ -125,6 +131,7 @@ static void pnv_setup_rfi_flush(void)
setup_rfi_flush(type, enable);
setup_barrier_nospec();
+ setup_count_cache_flush();
}
static void __init pnv_setup_arch(void)
--
2.14.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [1/5] powerpc/asm: Add a patch_site macro & helpers for patching instructions
2018-07-23 15:07 [PATCH 1/5] powerpc/asm: Add a patch_site macro & helpers for patching instructions Michael Ellerman
` (3 preceding siblings ...)
2018-07-23 15:07 ` [PATCH 5/5] powerpc/powernv: Query firmware " Michael Ellerman
@ 2018-08-08 14:25 ` Michael Ellerman
2018-08-08 16:30 ` [PATCH 1/5] " Christophe LEROY
5 siblings, 0 replies; 9+ messages in thread
From: Michael Ellerman @ 2018-08-08 14:25 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev
On Mon, 2018-07-23 at 15:07:52 UTC, Michael Ellerman wrote:
> Add a macro and some helper C functions for patching single asm
> instructions.
>
> The gas macro means we can do something like:
>
> 1: nop
> patch_site 1b, patch__foo
>
> Which is less visually distracting than defining a GLOBAL symbol at 1,
> and also doesn't pollute the symbol table which can confuse eg. perf.
>
> These are obviously similar to our existing feature sections, but are
> not automatically patched based on CPU/MMU features, rather they are
> designed to be manually patched by C code at some arbitrary point.
>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Series applied to powerpc next.
https://git.kernel.org/powerpc/c/06d0bbc6d0f56dacac3a79900e9a9a
cheers
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/5] powerpc/asm: Add a patch_site macro & helpers for patching instructions
2018-07-23 15:07 [PATCH 1/5] powerpc/asm: Add a patch_site macro & helpers for patching instructions Michael Ellerman
` (4 preceding siblings ...)
2018-08-08 14:25 ` [1/5] powerpc/asm: Add a patch_site macro & helpers for patching instructions Michael Ellerman
@ 2018-08-08 16:30 ` Christophe LEROY
2018-08-09 6:56 ` Christophe LEROY
5 siblings, 1 reply; 9+ messages in thread
From: Christophe LEROY @ 2018-08-08 16:30 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev
Le 23/07/2018 à 17:07, Michael Ellerman a écrit :
> Add a macro and some helper C functions for patching single asm
> instructions.
>
> The gas macro means we can do something like:
>
> 1: nop
> patch_site 1b, patch__foo
>
> Which is less visually distracting than defining a GLOBAL symbol at 1,
> and also doesn't pollute the symbol table which can confuse eg. perf.
>
> These are obviously similar to our existing feature sections, but are
> not automatically patched based on CPU/MMU features, rather they are
> designed to be manually patched by C code at some arbitrary point.
>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
> arch/powerpc/include/asm/code-patching-asm.h | 18 ++++++++++++++++++
> arch/powerpc/include/asm/code-patching.h | 2 ++
> arch/powerpc/lib/code-patching.c | 16 ++++++++++++++++
> 3 files changed, 36 insertions(+)
> create mode 100644 arch/powerpc/include/asm/code-patching-asm.h
>
> diff --git a/arch/powerpc/include/asm/code-patching-asm.h b/arch/powerpc/include/asm/code-patching-asm.h
> new file mode 100644
> index 000000000000..ed7b1448493a
> --- /dev/null
> +++ b/arch/powerpc/include/asm/code-patching-asm.h
> @@ -0,0 +1,18 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright 2018, Michael Ellerman, IBM Corporation.
> + */
> +#ifndef _ASM_POWERPC_CODE_PATCHING_ASM_H
> +#define _ASM_POWERPC_CODE_PATCHING_ASM_H
> +
> +/* Define a "site" that can be patched */
> +.macro patch_site label name
> + .pushsection ".rodata"
> + .balign 4
> + .global \name
> +\name:
> + .4byte \label - .
> + .popsection
> +.endm
> +
> +#endif /* _ASM_POWERPC_CODE_PATCHING_ASM_H */
> diff --git a/arch/powerpc/include/asm/code-patching.h b/arch/powerpc/include/asm/code-patching.h
> index 812535f40124..b2051234ada8 100644
> --- a/arch/powerpc/include/asm/code-patching.h
> +++ b/arch/powerpc/include/asm/code-patching.h
> @@ -32,6 +32,8 @@ unsigned int create_cond_branch(const unsigned int *addr,
> 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);
> +int patch_instruction_site(s32 *addr, unsigned int instr);
> +int patch_branch_site(s32 *site, unsigned long target, int flags);
Why use s32* instead of unsigned int* as usual for pointer to code ?
Christophe
>
> int instr_is_relative_branch(unsigned int instr);
> int instr_is_relative_link_branch(unsigned int instr);
> diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
> index e0d881ab304e..850f3b8f4da5 100644
> --- a/arch/powerpc/lib/code-patching.c
> +++ b/arch/powerpc/lib/code-patching.c
> @@ -195,6 +195,22 @@ int patch_branch(unsigned int *addr, unsigned long target, int flags)
> return patch_instruction(addr, create_branch(addr, target, flags));
> }
>
> +int patch_branch_site(s32 *site, unsigned long target, int flags)
> +{
> + unsigned int *addr;
> +
> + addr = (unsigned int *)((unsigned long)site + *site);
> + return patch_instruction(addr, create_branch(addr, target, flags));
> +}
> +
> +int patch_instruction_site(s32 *site, unsigned int instr)
> +{
> + unsigned int *addr;
> +
> + addr = (unsigned int *)((unsigned long)site + *site);
> + return patch_instruction(addr, instr);
> +}
> +
> bool is_offset_in_branch_range(long offset)
> {
> /*
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/5] powerpc/asm: Add a patch_site macro & helpers for patching instructions
2018-08-08 16:30 ` [PATCH 1/5] " Christophe LEROY
@ 2018-08-09 6:56 ` Christophe LEROY
2018-08-09 9:52 ` Michael Ellerman
0 siblings, 1 reply; 9+ messages in thread
From: Christophe LEROY @ 2018-08-09 6:56 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev
Le 08/08/2018 à 18:30, Christophe LEROY a écrit :
>
>
> Le 23/07/2018 à 17:07, Michael Ellerman a écrit :
>> Add a macro and some helper C functions for patching single asm
>> instructions.
>>
>> The gas macro means we can do something like:
>>
>> 1: nop
>> patch_site 1b, patch__foo
>>
>> Which is less visually distracting than defining a GLOBAL symbol at 1,
>> and also doesn't pollute the symbol table which can confuse eg. perf.
>>
>> These are obviously similar to our existing feature sections, but are
>> not automatically patched based on CPU/MMU features, rather they are
>> designed to be manually patched by C code at some arbitrary point.
>>
>> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
>> ---
>> arch/powerpc/include/asm/code-patching-asm.h | 18 ++++++++++++++++++
>> arch/powerpc/include/asm/code-patching.h | 2 ++
>> arch/powerpc/lib/code-patching.c | 16 ++++++++++++++++
>> 3 files changed, 36 insertions(+)
>> create mode 100644 arch/powerpc/include/asm/code-patching-asm.h
>>
>> diff --git a/arch/powerpc/include/asm/code-patching-asm.h
>> b/arch/powerpc/include/asm/code-patching-asm.h
>> new file mode 100644
>> index 000000000000..ed7b1448493a
>> --- /dev/null
>> +++ b/arch/powerpc/include/asm/code-patching-asm.h
>> @@ -0,0 +1,18 @@
>> +/* SPDX-License-Identifier: GPL-2.0+ */
>> +/*
>> + * Copyright 2018, Michael Ellerman, IBM Corporation.
>> + */
>> +#ifndef _ASM_POWERPC_CODE_PATCHING_ASM_H
>> +#define _ASM_POWERPC_CODE_PATCHING_ASM_H
>> +
>> +/* Define a "site" that can be patched */
>> +.macro patch_site label name
>> + .pushsection ".rodata"
>> + .balign 4
>> + .global \name
>> +\name:
>> + .4byte \label - .
>> + .popsection
>> +.endm
>> +
>> +#endif /* _ASM_POWERPC_CODE_PATCHING_ASM_H */
>> diff --git a/arch/powerpc/include/asm/code-patching.h
>> b/arch/powerpc/include/asm/code-patching.h
>> index 812535f40124..b2051234ada8 100644
>> --- a/arch/powerpc/include/asm/code-patching.h
>> +++ b/arch/powerpc/include/asm/code-patching.h
>> @@ -32,6 +32,8 @@ unsigned int create_cond_branch(const unsigned int
>> *addr,
>> 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);
>> +int patch_instruction_site(s32 *addr, unsigned int instr);
>> +int patch_branch_site(s32 *site, unsigned long target, int flags);
>
> Why use s32* instead of unsigned int* as usual for pointer to code ?
Forget my stupid question, I didn't see it was a relative address and
not an absolute one.
Christophe
>
> Christophe
>
>> int instr_is_relative_branch(unsigned int instr);
>> int instr_is_relative_link_branch(unsigned int instr);
>> diff --git a/arch/powerpc/lib/code-patching.c
>> b/arch/powerpc/lib/code-patching.c
>> index e0d881ab304e..850f3b8f4da5 100644
>> --- a/arch/powerpc/lib/code-patching.c
>> +++ b/arch/powerpc/lib/code-patching.c
>> @@ -195,6 +195,22 @@ int patch_branch(unsigned int *addr, unsigned
>> long target, int flags)
>> return patch_instruction(addr, create_branch(addr, target, flags));
>> }
>> +int patch_branch_site(s32 *site, unsigned long target, int flags)
>> +{
>> + unsigned int *addr;
>> +
>> + addr = (unsigned int *)((unsigned long)site + *site);
>> + return patch_instruction(addr, create_branch(addr, target, flags));
>> +}
>> +
>> +int patch_instruction_site(s32 *site, unsigned int instr)
>> +{
>> + unsigned int *addr;
>> +
>> + addr = (unsigned int *)((unsigned long)site + *site);
>> + return patch_instruction(addr, instr);
>> +}
>> +
>> bool is_offset_in_branch_range(long offset)
>> {
>> /*
>>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/5] powerpc/asm: Add a patch_site macro & helpers for patching instructions
2018-08-09 6:56 ` Christophe LEROY
@ 2018-08-09 9:52 ` Michael Ellerman
0 siblings, 0 replies; 9+ messages in thread
From: Michael Ellerman @ 2018-08-09 9:52 UTC (permalink / raw)
To: Christophe LEROY, linuxppc-dev
Christophe LEROY <christophe.leroy@c-s.fr> writes:
> Le 08/08/2018 =C3=A0 18:30, Christophe LEROY a =C3=A9crit=C2=A0:
>> Le 23/07/2018 =C3=A0 17:07, Michael Ellerman a =C3=A9crit=C2=A0:
...
>>> diff --git a/arch/powerpc/include/asm/code-patching.h=20
>>> b/arch/powerpc/include/asm/code-patching.h
>>> index 812535f40124..b2051234ada8 100644
>>> --- a/arch/powerpc/include/asm/code-patching.h
>>> +++ b/arch/powerpc/include/asm/code-patching.h
>>> @@ -32,6 +32,8 @@ unsigned int create_cond_branch(const unsigned int=20
>>> *addr,
>>> =C2=A0 int patch_branch(unsigned int *addr, unsigned long target, int f=
lags);
>>> =C2=A0 int patch_instruction(unsigned int *addr, unsigned int instr);
>>> =C2=A0 int raw_patch_instruction(unsigned int *addr, unsigned int instr=
);
>>> +int patch_instruction_site(s32 *addr, unsigned int instr);
>>> +int patch_branch_site(s32 *site, unsigned long target, int flags);
>>=20
>> Why use s32* instead of unsigned int* as usual for pointer to code ?
>
> Forget my stupid question, I didn't see it was a relative address and=20
> not an absolute one.
No worries.=20
It is a bit non-obvious at first glance, it looks like the s32 * points
to the instruction. But it points to the s32 that holds the relative
offset from itself, of the instruction.
We could add a typedef to try and make that more obvious, but I
generally don't like typedefs that hide pointerness.
cheers
^ permalink raw reply [flat|nested] 9+ messages in thread