* [PATCH v11 9/9] powerpc/32: use set_memory_attr()
From: Jordan Niethe @ 2021-04-29 3:16 UTC (permalink / raw)
To: linuxppc-dev
Cc: ajd, cmr, npiggin, aneesh.kumar, naveen.n.rao, Jordan Niethe, dja
In-Reply-To: <20210429031602.2606654-1-jniethe5@gmail.com>
From: Christophe Leroy <christophe.leroy@csgroup.eu>
Use set_memory_attr() instead of the PPC32 specific change_page_attr()
change_page_attr() was checking that the address was not mapped by
blocks and was handling highmem, but that's unneeded because the
affected pages can't be in highmem and block mapping verification
is already done by the callers.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
[ruscur: rebase on powerpc/merge with Christophe's new patches]
Signed-off-by: Russell Currey <ruscur@russell.cc>
Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
arch/powerpc/mm/pgtable_32.c | 60 ++++++------------------------------
1 file changed, 10 insertions(+), 50 deletions(-)
diff --git a/arch/powerpc/mm/pgtable_32.c b/arch/powerpc/mm/pgtable_32.c
index e0ec67a16887..dcf5ecca19d9 100644
--- a/arch/powerpc/mm/pgtable_32.c
+++ b/arch/powerpc/mm/pgtable_32.c
@@ -23,6 +23,7 @@
#include <linux/highmem.h>
#include <linux/memblock.h>
#include <linux/slab.h>
+#include <linux/set_memory.h>
#include <asm/pgalloc.h>
#include <asm/fixmap.h>
@@ -132,64 +133,20 @@ void __init mapin_ram(void)
}
}
-static int __change_page_attr_noflush(struct page *page, pgprot_t prot)
-{
- pte_t *kpte;
- unsigned long address;
-
- BUG_ON(PageHighMem(page));
- address = (unsigned long)page_address(page);
-
- if (v_block_mapped(address))
- return 0;
- kpte = virt_to_kpte(address);
- if (!kpte)
- return -EINVAL;
- __set_pte_at(&init_mm, address, kpte, mk_pte(page, prot), 0);
-
- return 0;
-}
-
-/*
- * Change the page attributes of an page in the linear mapping.
- *
- * THIS DOES NOTHING WITH BAT MAPPINGS, DEBUG USE ONLY
- */
-static int change_page_attr(struct page *page, int numpages, pgprot_t prot)
-{
- int i, err = 0;
- unsigned long flags;
- struct page *start = page;
-
- local_irq_save(flags);
- for (i = 0; i < numpages; i++, page++) {
- err = __change_page_attr_noflush(page, prot);
- if (err)
- break;
- }
- wmb();
- local_irq_restore(flags);
- flush_tlb_kernel_range((unsigned long)page_address(start),
- (unsigned long)page_address(page));
- return err;
-}
-
void mark_initmem_nx(void)
{
- struct page *page = virt_to_page(_sinittext);
unsigned long numpages = PFN_UP((unsigned long)_einittext) -
PFN_DOWN((unsigned long)_sinittext);
if (v_block_mapped((unsigned long)_sinittext))
mmu_mark_initmem_nx();
else
- change_page_attr(page, numpages, PAGE_KERNEL);
+ set_memory_attr((unsigned long)_sinittext, numpages, PAGE_KERNEL);
}
#ifdef CONFIG_STRICT_KERNEL_RWX
void mark_rodata_ro(void)
{
- struct page *page;
unsigned long numpages;
if (v_block_mapped((unsigned long)_stext + 1)) {
@@ -198,20 +155,18 @@ void mark_rodata_ro(void)
return;
}
- page = virt_to_page(_stext);
numpages = PFN_UP((unsigned long)_etext) -
PFN_DOWN((unsigned long)_stext);
- change_page_attr(page, numpages, PAGE_KERNEL_ROX);
+ set_memory_attr((unsigned long)_stext, numpages, PAGE_KERNEL_ROX);
/*
* mark .rodata as read only. Use __init_begin rather than __end_rodata
* to cover NOTES and EXCEPTION_TABLE.
*/
- page = virt_to_page(__start_rodata);
numpages = PFN_UP((unsigned long)__init_begin) -
PFN_DOWN((unsigned long)__start_rodata);
- change_page_attr(page, numpages, PAGE_KERNEL_RO);
+ set_memory_attr((unsigned long)__start_rodata, numpages, PAGE_KERNEL_RO);
// mark_initmem_nx() should have already run by now
ptdump_check_wx();
@@ -221,9 +176,14 @@ void mark_rodata_ro(void)
#ifdef CONFIG_DEBUG_PAGEALLOC
void __kernel_map_pages(struct page *page, int numpages, int enable)
{
+ unsigned long addr = (unsigned long)page_address(page);
+
if (PageHighMem(page))
return;
- change_page_attr(page, numpages, enable ? PAGE_KERNEL : __pgprot(0));
+ if (enable)
+ set_memory_attr(addr, numpages, PAGE_KERNEL);
+ else
+ set_memory_attr(addr, numpages, __pgprot(0));
}
#endif /* CONFIG_DEBUG_PAGEALLOC */
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v11 2/9] powerpc/lib/code-patching: Set up Strict RWX patching earlier
From: Christophe Leroy @ 2021-04-29 4:53 UTC (permalink / raw)
To: Jordan Niethe, linuxppc-dev
Cc: ajd, npiggin, cmr, aneesh.kumar, naveen.n.rao, dja
In-Reply-To: <20210429031602.2606654-3-jniethe5@gmail.com>
Le 29/04/2021 à 05:15, Jordan Niethe a écrit :
> setup_text_poke_area() is a late init call so it runs before
> mark_rodata_ro() and after the init calls. This lets all the init code
> patching simply write to their locations. In the future, kprobes is
> going to allocate its instruction pages RO which means they will need
> setup_text__poke_area() to have been already called for their code
> patching. However, init_kprobes() (which allocates and patches some
> instruction pages) is an early init call so it happens before
> setup_text__poke_area().
>
> start_kernel() calls poking_init() before any of the init calls. On
> powerpc, poking_init() is currently a nop. setup_text_poke_area() relies
> on kernel virtual memory, cpu hotplug and per_cpu_areas being setup.
> setup_per_cpu_areas(), boot_cpu_hotplug_init() and mm_init() are called
> before poking_init().
>
> Turn setup_text_poke_area() into poking_init().
I can't remember, maybe I already asked the question:
Have you done some performance measurement or at least some performance analysis ?
Christophe
>
> Reviewed-by: Russell Currey <ruscur@russell.cc>
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> ---
> v9: New to series
> ---
> arch/powerpc/lib/code-patching.c | 12 ++++--------
> 1 file changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
> index 870b30d9be2f..15296207e1ba 100644
> --- a/arch/powerpc/lib/code-patching.c
> +++ b/arch/powerpc/lib/code-patching.c
> @@ -70,14 +70,11 @@ static int text_area_cpu_down(unsigned int cpu)
> }
>
> /*
> - * Run as a late init call. This allows all the boot time patching to be done
> - * simply by patching the code, and then we're called here prior to
> - * mark_rodata_ro(), which happens after all init calls are run. Although
> - * BUG_ON() is rude, in this case it should only happen if ENOMEM, and we judge
> - * it as being preferable to a kernel that will crash later when someone tries
> - * to use patch_instruction().
> + * Although BUG_ON() is rude, in this case it should only happen if ENOMEM, and
> + * we judge it as being preferable to a kernel that will crash later when
> + * someone tries to use patch_instruction().
> */
> -static int __init setup_text_poke_area(void)
> +int __init poking_init(void)
> {
> BUG_ON(!cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
> "powerpc/text_poke:online", text_area_cpu_up,
> @@ -85,7 +82,6 @@ static int __init setup_text_poke_area(void)
>
> return 0;
> }
> -late_initcall(setup_text_poke_area);
>
> /*
> * This can be called for kernel text or a module.
>
^ permalink raw reply
* [PATCH] powerpc/perf: Update MMCR2 to support event exclude_idle
From: Madhavan Srinivasan @ 2021-04-29 5:02 UTC (permalink / raw)
To: mpe; +Cc: Madhavan Srinivasan, linuxppc-dev
struct perf_event_attr supports exclude counting of idle task.
This is sent to kernel via perf_event_attr.exclude_idle and
in perf tool, user can use ":I" event modifier to enable this
for specific event.
Monitor Mode Control Register 2 (MMCR2) SPR has control bits
for each PMCs to freeze counting based on the Control Register
CTRL[RUN] state. CTRL[RUN] is not set when idle task is
running. Patch adds a check for event attr.exclude_idle to
set MMCR2[FCnWAIT] bit.
Signed-off-by: Madhavan Srinivasan <maddy@linux.ibm.com>
---
arch/powerpc/perf/isa207-common.c | 3 +++
arch/powerpc/perf/isa207-common.h | 1 +
2 files changed, 4 insertions(+)
diff --git a/arch/powerpc/perf/isa207-common.c b/arch/powerpc/perf/isa207-common.c
index e4f577da33d8..5e05f90dd7df 100644
--- a/arch/powerpc/perf/isa207-common.c
+++ b/arch/powerpc/perf/isa207-common.c
@@ -587,6 +587,9 @@ int isa207_compute_mmcr(u64 event[], int n_ev,
mmcr2 |= MMCR2_FCS(pmc);
}
+ if (pevents[i]->attr.exclude_idle)
+ mmcr2 |= MMCR2_FCWAIT(pmc);
+
if (cpu_has_feature(CPU_FTR_ARCH_31)) {
if (pmc <= 4) {
val = (event[i] >> p10_EVENT_MMCR3_SHIFT) &
diff --git a/arch/powerpc/perf/isa207-common.h b/arch/powerpc/perf/isa207-common.h
index 1af0e8c97ac7..a95a810fa832 100644
--- a/arch/powerpc/perf/isa207-common.h
+++ b/arch/powerpc/perf/isa207-common.h
@@ -248,6 +248,7 @@
/* Bits in MMCR2 for PowerISA v2.07 */
#define MMCR2_FCS(pmc) (1ull << (63 - (((pmc) - 1) * 9)))
#define MMCR2_FCP(pmc) (1ull << (62 - (((pmc) - 1) * 9)))
+#define MMCR2_FCWAIT(pmc) (1ull << (58 - (((pmc) - 1) * 9)))
#define MMCR2_FCH(pmc) (1ull << (57 - (((pmc) - 1) * 9)))
#define MAX_ALT 2
--
2.26.2
^ permalink raw reply related
* Re: [PATCH v11 3/9] powerpc: Always define MODULES_{VADDR,END}
From: Christophe Leroy @ 2021-04-29 5:04 UTC (permalink / raw)
To: Jordan Niethe, linuxppc-dev
Cc: ajd, npiggin, cmr, aneesh.kumar, naveen.n.rao, dja
In-Reply-To: <20210429031602.2606654-4-jniethe5@gmail.com>
Le 29/04/2021 à 05:15, Jordan Niethe a écrit :
> If MODULES_{VADDR,END} are not defined set them to VMALLOC_START and
> VMALLOC_END respectively. This reduces the need for special cases. For
> example, powerpc's module_alloc() was previously predicated on
> MODULES_VADDR being defined but now is unconditionally defined.
>
> This will be useful reducing conditional code in other places that need
> to allocate from the module region (i.e., kprobes).
>
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> ---
> v10: New to series
> v11: - Consider more places MODULES_VADDR was being used
> ---
> arch/powerpc/include/asm/pgtable.h | 11 +++++++++++
> arch/powerpc/kernel/module.c | 5 +----
> arch/powerpc/mm/kasan/kasan_init_32.c | 10 +++++-----
> arch/powerpc/mm/ptdump/ptdump.c | 4 ++--
> 4 files changed, 19 insertions(+), 11 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/pgtable.h b/arch/powerpc/include/asm/pgtable.h
> index c6a676714f04..882fda779648 100644
> --- a/arch/powerpc/include/asm/pgtable.h
> +++ b/arch/powerpc/include/asm/pgtable.h
> @@ -39,6 +39,17 @@ struct mm_struct;
> #define __S110 PAGE_SHARED_X
> #define __S111 PAGE_SHARED_X
>
> +#ifndef MODULES_VADDR
> +#define MODULES_VADDR VMALLOC_START
> +#define MODULES_END VMALLOC_END
> +#endif
> +
> +#if defined(CONFIG_PPC_BOOK3S_32) && defined(CONFIG_STRICT_KERNEL_RWX)
No no.
TASK_SIZE > MODULES_VADDR is ALWAYS wrong, for any target, in any configuration.
Why is it a problem to leave the test as a BUILD_BUG_ON() in module_alloc() ?
> +#if TASK_SIZE > MODULES_VADDR
> +#error TASK_SIZE > MODULES_VADDR
> +#endif
> +#endif
> +
> #ifndef __ASSEMBLY__
>
> /* Keep these as a macros to avoid include dependency mess */
> diff --git a/arch/powerpc/kernel/module.c b/arch/powerpc/kernel/module.c
> index fab84024650c..c60c7457ff47 100644
> --- a/arch/powerpc/kernel/module.c
> +++ b/arch/powerpc/kernel/module.c
> @@ -15,6 +15,7 @@
> #include <linux/sort.h>
> #include <asm/setup.h>
> #include <asm/sections.h>
> +#include <linux/mm.h>
>
> static LIST_HEAD(module_bug_list);
>
> @@ -88,7 +89,6 @@ int module_finalize(const Elf_Ehdr *hdr,
> return 0;
> }
>
> -#ifdef MODULES_VADDR
> static __always_inline void *
> __module_alloc(unsigned long size, unsigned long start, unsigned long end)
> {
> @@ -102,8 +102,6 @@ void *module_alloc(unsigned long size)
> unsigned long limit = (unsigned long)_etext - SZ_32M;
> void *ptr = NULL;
>
> - BUILD_BUG_ON(TASK_SIZE > MODULES_VADDR);
> -
> /* First try within 32M limit from _etext to avoid branch trampolines */
> if (MODULES_VADDR < PAGE_OFFSET && MODULES_END > limit)
> ptr = __module_alloc(size, limit, MODULES_END);
> @@ -113,4 +111,3 @@ void *module_alloc(unsigned long size)
>
> return ptr;
> }
> -#endif
> diff --git a/arch/powerpc/mm/kasan/kasan_init_32.c b/arch/powerpc/mm/kasan/kasan_init_32.c
> index cf8770b1a692..42c057366ac7 100644
> --- a/arch/powerpc/mm/kasan/kasan_init_32.c
> +++ b/arch/powerpc/mm/kasan/kasan_init_32.c
> @@ -116,11 +116,11 @@ static void __init kasan_unmap_early_shadow_vmalloc(void)
>
> kasan_update_early_region(k_start, k_end, __pte(0));
>
> -#ifdef MODULES_VADDR
> - k_start = (unsigned long)kasan_mem_to_shadow((void *)MODULES_VADDR);
> - k_end = (unsigned long)kasan_mem_to_shadow((void *)MODULES_END);
> - kasan_update_early_region(k_start, k_end, __pte(0));
> -#endif
> + if (MODULES_VADDR != VMALLOC_START && MODULES_END != VMALLOC_END) {
Shouldn't it be an || ?
As soon as either MODULES_VADDR or MODULES_END differs from the vmalloc boundaries, it needs to be
done I think.
> + k_start = (unsigned long)kasan_mem_to_shadow((void *)MODULES_VADDR);
> + k_end = (unsigned long)kasan_mem_to_shadow((void *)MODULES_END);
> + kasan_update_early_region(k_start, k_end, __pte(0));
> + }
> }
>
> void __init kasan_mmu_init(void)
> diff --git a/arch/powerpc/mm/ptdump/ptdump.c b/arch/powerpc/mm/ptdump/ptdump.c
> index aca354fb670b..0431457f668f 100644
> --- a/arch/powerpc/mm/ptdump/ptdump.c
> +++ b/arch/powerpc/mm/ptdump/ptdump.c
> @@ -73,7 +73,7 @@ struct addr_marker {
>
> static struct addr_marker address_markers[] = {
> { 0, "Start of kernel VM" },
> -#ifdef MODULES_VADDR
> +#if defined(CONFIG_BOOK32_32) && defined(CONFIG_STRICT_KERNEL_RWX)
Not valid anymore, see https://github.com/linuxppc/linux/commit/80edc68e0479 and
https://github.com/linuxppc/linux/commit/9132a2e82adc
The best would be to be able to do something like:
#if MODULES_VADDR != VMALLOC_START
If it doesn't work, then it has to be
#if defined(CONFIG_BOOK32_32) || defined(CONFIG_PPC_8xx)
> { 0, "modules start" },
> { 0, "modules end" },
> #endif
> @@ -359,7 +359,7 @@ static void populate_markers(void)
> #else
> address_markers[i++].start_address = TASK_SIZE;
> #endif
> -#ifdef MODULES_VADDR
> +#if defined(CONFIG_BOOK32_32) && defined(CONFIG_STRICT_KERNEL_RWX)
Same.
> address_markers[i++].start_address = MODULES_VADDR;
> address_markers[i++].start_address = MODULES_END;
> #endif
>
^ permalink raw reply
* [PATCH v2] cpuidle/pseries: Fixup CEDE0 latency only for POWER10 onwards
From: Gautham R. Shenoy @ 2021-04-29 5:18 UTC (permalink / raw)
To: Rafael J. Wysocki, Daniel Lezcano, Michael Ellerman,
Aneesh Kumar K.V, Vaidyanathan Srinivasan
Cc: Gautham R. Shenoy, linux-pm, joedecke, Michal Suchanek,
linuxppc-dev
From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
Commit d947fb4c965c ("cpuidle: pseries: Fixup exit latency for
CEDE(0)") sets the exit latency of CEDE(0) based on the latency values
of the Extended CEDE states advertised by the platform
On POWER9 LPARs, the firmwares advertise a very low value of 2us for
CEDE1 exit latency on a Dedicated LPAR. The latency advertized by the
PHYP hypervisor corresponds to the latency required to wakeup from the
underlying hardware idle state. However the wakeup latency from the
LPAR perspective should include
1. The time taken to transition the CPU from the Hypervisor into the
LPAR post wakeup from platform idle state
2. Time taken to send the IPI from the source CPU (waker) to the idle
target CPU (wakee).
1. can be measured via timer idle test, where we queue a timer, say
for 1ms, and enter the CEDE state. When the timer fires, in the timer
handler we compute how much extra timer over the expected 1ms have we
consumed. On a a POWER9 LPAR the numbers are
CEDE latency measured using a timer (numbers in ns)
N Min Median Avg 90%ile 99%ile Max Stddev
400 2601 5677 5668.74 5917 6413 9299 455.01
1. and 2. combined can be determined by an IPI latency test where we
send an IPI to an idle CPU and in the handler compute the time
difference between when the IPI was sent and when the handler ran. We
see the following numbers on POWER9 LPAR.
CEDE latency measured using an IPI (numbers in ns)
N Min Median Avg 90%ile 99%ile Max Stddev
400 711 7564 7369.43 8559 9514 9698 1200.01
Suppose, we consider the 99th percentile latency value measured using
the IPI to be the wakeup latency, the value would be 9.5us This is in
the ballpark of the default value of 10us.
Hence, use the exit latency of CEDE(0) based on the latency values
advertized by platform only from POWER10 onwards. The values
advertized on POWER10 platforms is more realistic and informed by the
latency measurements. For earlier platforms stick to the default value
of 10us.
Reported-by: Enrico Joedecke <joedecke@de.ibm.com>
Fixes: commit d947fb4c965c ("cpuidle: pseries: Fixup exit latency for
CEDE(0)")
Cc: Michal Suchanek <msuchanek@suse.de>
Cc: Vaidyanathan Srinivasan <svaidy@linux.ibm.com>
Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
---
v1-->v2 : Updated the commit log with the details of the measured
latencies on POWER9 LPARs.
drivers/cpuidle/cpuidle-pseries.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/cpuidle/cpuidle-pseries.c b/drivers/cpuidle/cpuidle-pseries.c
index a2b5c6f..7207467 100644
--- a/drivers/cpuidle/cpuidle-pseries.c
+++ b/drivers/cpuidle/cpuidle-pseries.c
@@ -419,7 +419,8 @@ static int pseries_idle_probe(void)
cpuidle_state_table = shared_states;
max_idle_state = ARRAY_SIZE(shared_states);
} else {
- fixup_cede0_latency();
+ if (pvr_version_is(PVR_POWER10))
+ fixup_cede0_latency();
cpuidle_state_table = dedicated_states;
max_idle_state = NR_DEDICATED_STATES;
}
--
1.8.3.1
^ permalink raw reply related
* [PATCH v7] powerpc/kexec_file: use current CPU info while setting up FDT
From: Sourabh Jain @ 2021-04-29 6:02 UTC (permalink / raw)
To: mpe; +Cc: mahesh, Sourabh Jain, linuxppc-dev, stable, hbathini, bauerman
kexec_file_load uses initial_boot_params in setting up the device-tree
for the kernel to be loaded. Though initial_boot_params holds info
about CPUs at the time of boot, it doesn't account for hot added CPUs.
So, kexec'ing with kexec_file_load syscall would leave the kexec'ed
kernel with inaccurate CPU info. Also, if kdump kernel is loaded with
kexec_file_load syscall and the system crashes on a hot added CPU,
capture kernel hangs failing to identify the boot CPU.
Kernel panic - not syncing: sysrq triggered crash
CPU: 24 PID: 6065 Comm: echo Kdump: loaded Not tainted 5.12.0-rc5upstream #54
Call Trace:
[c0000000e590fac0] [c0000000007b2400] dump_stack+0xc4/0x114 (unreliable)
[c0000000e590fb00] [c000000000145290] panic+0x16c/0x41c
[c0000000e590fba0] [c0000000008892e0] sysrq_handle_crash+0x30/0x40
[c0000000e590fc00] [c000000000889cdc] __handle_sysrq+0xcc/0x1f0
[c0000000e590fca0] [c00000000088a538] write_sysrq_trigger+0xd8/0x178
[c0000000e590fce0] [c0000000005e9b7c] proc_reg_write+0x10c/0x1b0
[c0000000e590fd10] [c0000000004f26d0] vfs_write+0xf0/0x330
[c0000000e590fd60] [c0000000004f2aec] ksys_write+0x7c/0x140
[c0000000e590fdb0] [c000000000031ee0] system_call_exception+0x150/0x290
[c0000000e590fe10] [c00000000000ca5c] system_call_common+0xec/0x278
--- interrupt: c00 at 0x7fff905b9664
NIP: 00007fff905b9664 LR: 00007fff905320c4 CTR: 0000000000000000
REGS: c0000000e590fe80 TRAP: 0c00 Not tainted (5.12.0-rc5upstream)
MSR: 800000000280f033 <SF,VEC,VSX,EE,PR,FP,ME,IR,DR,RI,LE> CR: 28000242
XER: 00000000
IRQMASK: 0
GPR00: 0000000000000004 00007ffff5fedf30 00007fff906a7300 0000000000000001
GPR04: 000001002a7355b0 0000000000000002 0000000000000001 00007ffff5fef616
GPR08: 0000000000000001 0000000000000000 0000000000000000 0000000000000000
GPR12: 0000000000000000 00007fff9073a160 0000000000000000 0000000000000000
GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR20: 0000000000000000 00007fff906a4ee0 0000000000000002 0000000000000001
GPR24: 00007fff906a0898 0000000000000000 0000000000000002 000001002a7355b0
GPR28: 0000000000000002 00007fff906a1790 000001002a7355b0 0000000000000002
NIP [00007fff905b9664] 0x7fff905b9664
LR [00007fff905320c4] 0x7fff905320c4
--- interrupt: c00
To avoid this from happening, extract current CPU info from of_root
device node and use it for setting up the fdt in kexec_file_load case.
Fixes: 6ecd0163d360 ("powerpc/kexec_file: Add appropriate regions for memory reserve map")
Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
Reviewed-by: Hari Bathini <hbathini@linux.ibm.com>
Cc: <stable@vger.kernel.org>
---
arch/powerpc/kexec/file_load_64.c | 92 +++++++++++++++++++++++++++++++
1 file changed, 92 insertions(+)
---
Changelog:
v1 -> v5
- https://lists.ozlabs.org/pipermail/linuxppc-dev/2021-April/227950.html
v5 -> v6
- use exiting macro (for_each_property_of_node) to loop through all
properties of a node.
- removed devtree_lock while accessing the node properties.
- function name update, add_node_prop to add_node_props.
v6 -> v7
- decrement the cpus_node refcount.
---
diff --git a/arch/powerpc/kexec/file_load_64.c b/arch/powerpc/kexec/file_load_64.c
index 02b9e4d0dc40..a8a7cb71086b 100644
--- a/arch/powerpc/kexec/file_load_64.c
+++ b/arch/powerpc/kexec/file_load_64.c
@@ -960,6 +960,93 @@ unsigned int kexec_fdt_totalsize_ppc64(struct kimage *image)
return fdt_size;
}
+/**
+ * add_node_props - Reads node properties from device node structure and add
+ * them to fdt.
+ * @fdt: Flattened device tree of the kernel
+ * @node_offset: offset of the node to add a property at
+ * @dn: device node pointer
+ *
+ * Returns 0 on success, negative errno on error.
+ */
+static int add_node_props(void *fdt, int node_offset, const struct device_node *dn)
+{
+ int ret = 0;
+ struct property *pp;
+
+ if (!dn)
+ return -EINVAL;
+
+ for_each_property_of_node(dn, pp) {
+ ret = fdt_setprop(fdt, node_offset, pp->name, pp->value, pp->length);
+ if (ret < 0) {
+ pr_err("Unable to add %s property: %s\n", pp->name, fdt_strerror(ret));
+ return ret;
+ }
+ }
+ return ret;
+}
+
+/**
+ * update_cpus_node - Update cpus node of flattened device tree using of_root
+ * device node.
+ * @fdt: Flattened device tree of the kernel.
+ *
+ * Returns 0 on success, negative errno on error.
+ */
+static int update_cpus_node(void *fdt)
+{
+ struct device_node *cpus_node, *dn;
+ int cpus_offset, cpus_subnode_offset, ret = 0;
+
+ cpus_offset = fdt_path_offset(fdt, "/cpus");
+ if (cpus_offset < 0 && cpus_offset != -FDT_ERR_NOTFOUND) {
+ pr_err("Malformed device tree: error reading /cpus node: %s\n",
+ fdt_strerror(cpus_offset));
+ return cpus_offset;
+ }
+
+ if (cpus_offset > 0) {
+ ret = fdt_del_node(fdt, cpus_offset);
+ if (ret < 0) {
+ pr_err("Error deleting /cpus node: %s\n", fdt_strerror(ret));
+ return -EINVAL;
+ }
+ }
+
+ /* Add cpus node to fdt */
+ cpus_offset = fdt_add_subnode(fdt, fdt_path_offset(fdt, "/"), "cpus");
+ if (cpus_offset < 0) {
+ pr_err("Error creating /cpus node: %s\n", fdt_strerror(cpus_offset));
+ return -EINVAL;
+ }
+
+ /* Add cpus node properties */
+ cpus_node = of_find_node_by_path("/cpus");
+ ret = add_node_props(fdt, cpus_offset, cpus_node);
+ of_node_put(cpus_node);
+ if (ret < 0)
+ return ret;
+
+ /* Loop through all subnodes of cpus and add them to fdt */
+ for_each_node_by_type(dn, "cpu") {
+ cpus_subnode_offset = fdt_add_subnode(fdt, cpus_offset, dn->full_name);
+ if (cpus_subnode_offset < 0) {
+ pr_err("Unable to add %s subnode: %s\n", dn->full_name,
+ fdt_strerror(cpus_subnode_offset));
+ ret = cpus_subnode_offset;
+ goto out;
+ }
+
+ ret = add_node_props(fdt, cpus_subnode_offset, dn);
+ if (ret < 0)
+ goto out;
+ }
+out:
+ of_node_put(dn);
+ return ret;
+}
+
/**
* setup_new_fdt_ppc64 - Update the flattend device-tree of the kernel
* being loaded.
@@ -1020,6 +1107,11 @@ int setup_new_fdt_ppc64(const struct kimage *image, void *fdt,
}
}
+ /* Update cpus nodes information to account hotplug CPUs. */
+ ret = update_cpus_node(fdt);
+ if (ret < 0)
+ goto out;
+
/* Update memory reserve map */
ret = get_reserved_memory_ranges(&rmem);
if (ret)
--
2.26.3
^ permalink raw reply related
* Re: [PATCH v6] powerpc/kexec_file: use current CPU info while setting up FDT
From: Sourabh Jain @ 2021-04-29 6:05 UTC (permalink / raw)
To: Tyrel Datwyler, mpe; +Cc: mahesh, bauerman, hbathini, stable, linuxppc-dev
In-Reply-To: <8c3da505-f034-289a-819d-2841a5c7e580@linux.ibm.com>
On 28/04/21 6:02 am, Tyrel Datwyler wrote:
> On 4/26/21 9:51 PM, Sourabh Jain wrote:
>> kexec_file_load uses initial_boot_params in setting up the device-tree
>> for the kernel to be loaded. Though initial_boot_params holds info
>> about CPUs at the time of boot, it doesn't account for hot added CPUs.
>>
>> So, kexec'ing with kexec_file_load syscall would leave the kexec'ed
>> kernel with inaccurate CPU info. Also, if kdump kernel is loaded with
>> kexec_file_load syscall and the system crashes on a hot added CPU,
>> capture kernel hangs failing to identify the boot CPU.
>>
>> Kernel panic - not syncing: sysrq triggered crash
>> CPU: 24 PID: 6065 Comm: echo Kdump: loaded Not tainted 5.12.0-rc5upstream #54
>> Call Trace:
>> [c0000000e590fac0] [c0000000007b2400] dump_stack+0xc4/0x114 (unreliable)
>> [c0000000e590fb00] [c000000000145290] panic+0x16c/0x41c
>> [c0000000e590fba0] [c0000000008892e0] sysrq_handle_crash+0x30/0x40
>> [c0000000e590fc00] [c000000000889cdc] __handle_sysrq+0xcc/0x1f0
>> [c0000000e590fca0] [c00000000088a538] write_sysrq_trigger+0xd8/0x178
>> [c0000000e590fce0] [c0000000005e9b7c] proc_reg_write+0x10c/0x1b0
>> [c0000000e590fd10] [c0000000004f26d0] vfs_write+0xf0/0x330
>> [c0000000e590fd60] [c0000000004f2aec] ksys_write+0x7c/0x140
>> [c0000000e590fdb0] [c000000000031ee0] system_call_exception+0x150/0x290
>> [c0000000e590fe10] [c00000000000ca5c] system_call_common+0xec/0x278
>> --- interrupt: c00 at 0x7fff905b9664
>> NIP: 00007fff905b9664 LR: 00007fff905320c4 CTR: 0000000000000000
>> REGS: c0000000e590fe80 TRAP: 0c00 Not tainted (5.12.0-rc5upstream)
>> MSR: 800000000280f033 <SF,VEC,VSX,EE,PR,FP,ME,IR,DR,RI,LE> CR: 28000242
>> XER: 00000000
>> IRQMASK: 0
>> GPR00: 0000000000000004 00007ffff5fedf30 00007fff906a7300 0000000000000001
>> GPR04: 000001002a7355b0 0000000000000002 0000000000000001 00007ffff5fef616
>> GPR08: 0000000000000001 0000000000000000 0000000000000000 0000000000000000
>> GPR12: 0000000000000000 00007fff9073a160 0000000000000000 0000000000000000
>> GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>> GPR20: 0000000000000000 00007fff906a4ee0 0000000000000002 0000000000000001
>> GPR24: 00007fff906a0898 0000000000000000 0000000000000002 000001002a7355b0
>> GPR28: 0000000000000002 00007fff906a1790 000001002a7355b0 0000000000000002
>> NIP [00007fff905b9664] 0x7fff905b9664
>> LR [00007fff905320c4] 0x7fff905320c4
>> --- interrupt: c00
>>
>> To avoid this from happening, extract current CPU info from of_root
>> device node and use it for setting up the fdt in kexec_file_load case.
>>
>> Fixes: 6ecd0163d360 ("powerpc/kexec_file: Add appropriate regions for memory reserve map")
>>
>> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
>> Reviewed-by: Hari Bathini <hbathini@linux.ibm.com>
>> Cc: <stable@vger.kernel.org>
>> ---
>> arch/powerpc/kexec/file_load_64.c | 88 +++++++++++++++++++++++++++++++
>> 1 file changed, 88 insertions(+)
>>
>> ---
>> Changelog:
>>
>> v1 -> v5
>> - https://lists.ozlabs.org/pipermail/linuxppc-dev/2021-April/227950.html
>>
>> v5 -> v6
>> - use exiting macro (for_each_property_of_node) to loop through all
>> properties of a node.
>> - removed devtree_lock while accessing the node properties.
>> - function name update, add_node_prop to add_node_props.
>> ---
>>
>> diff --git a/arch/powerpc/kexec/file_load_64.c b/arch/powerpc/kexec/file_load_64.c
>> index 02b9e4d0dc40..4f7d4c10f939 100644
>> --- a/arch/powerpc/kexec/file_load_64.c
>> +++ b/arch/powerpc/kexec/file_load_64.c
>> @@ -960,6 +960,89 @@ unsigned int kexec_fdt_totalsize_ppc64(struct kimage *image)
>> return fdt_size;
>> }
>>
>> +/**
>> + * add_node_props - Reads node properties from device node structure and add
>> + * them to fdt.
>> + * @fdt: Flattened device tree of the kernel
>> + * @node_offset: offset of the node to add a property at
>> + * @dn: device node pointer
>> + *
>> + * Returns 0 on success, negative errno on error.
>> + */
>> +static int add_node_props(void *fdt, int node_offset, const struct device_node *dn)
>> +{
>> + int ret = 0;
>> + struct property *pp;
>> +
>> + if (!dn)
>> + return -EINVAL;
>> +
>> + for_each_property_of_node(dn, pp) {
>> + ret = fdt_setprop(fdt, node_offset, pp->name, pp->value, pp->length);
>> + if (ret < 0) {
>> + pr_err("Unable to add %s property: %s\n", pp->name, fdt_strerror(ret));
>> + return ret;
>> + }
>> + }
>> + return ret;
>> +}
>> +
>> +/**
>> + * update_cpus_node - Update cpus node of flattened device tree using of_root
>> + * device node.
>> + * @fdt: Flattened device tree of the kernel.
>> + *
>> + * Returns 0 on success, negative errno on error.
>> + */
>> +static int update_cpus_node(void *fdt)
>> +{
>> + struct device_node *cpus_node, *dn;
>> + int cpus_offset, cpus_subnode_offset, ret = 0;
>> +
>> + cpus_offset = fdt_path_offset(fdt, "/cpus");
>> + if (cpus_offset < 0 && cpus_offset != -FDT_ERR_NOTFOUND) {
>> + pr_err("Malformed device tree: error reading /cpus node: %s\n",
>> + fdt_strerror(cpus_offset));
>> + return cpus_offset;
>> + }
>> +
>> + if (cpus_offset > 0) {
>> + ret = fdt_del_node(fdt, cpus_offset);
>> + if (ret < 0) {
>> + pr_err("Error deleting /cpus node: %s\n", fdt_strerror(ret));
>> + return -EINVAL;
>> + }
>> + }
>> +
>> + /* Add cpus node to fdt */
>> + cpus_offset = fdt_add_subnode(fdt, fdt_path_offset(fdt, "/"), "cpus");
>> + if (cpus_offset < 0) {
>> + pr_err("Error creating /cpus node: %s\n", fdt_strerror(cpus_offset));
>> + return -EINVAL;
>> + }
>> +
>> + /* Add cpus node properties */
>> + cpus_node = of_find_node_by_path("/cpus");
> Pretty sure that of_find_node_by_path() returns a device_node with its refcount
> incremented.
>
>> + ret = add_node_props(fdt, cpus_offset, cpus_node);
> Need a of_node_put(cpus_node) here.
>
> Thanks for the review Tyrel. updated in v7.
>
> - Sourabh
^ permalink raw reply
* [PATCH v4 09/11] lkdtm/powerpc: Fix code patching hijack test
From: Christopher M. Riedl @ 2021-04-29 7:20 UTC (permalink / raw)
To: linuxppc-dev; +Cc: tglx, x86, linux-hardening, keescook
In-Reply-To: <20210429072057.8870-1-cmr@bluescreens.de>
Code patching on powerpc with a STRICT_KERNEL_RWX uses a userspace
address in a temporary mm now. Use __put_user() to avoid write failures
due to KUAP when attempting a "hijack" on the patching address.
Signed-off-by: Christopher M. Riedl <cmr@bluescreens.de>
---
drivers/misc/lkdtm/perms.c | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/drivers/misc/lkdtm/perms.c b/drivers/misc/lkdtm/perms.c
index 55c3bec6d3b72..af9bf285fe326 100644
--- a/drivers/misc/lkdtm/perms.c
+++ b/drivers/misc/lkdtm/perms.c
@@ -268,16 +268,7 @@ static inline u32 lkdtm_read_patch_site(void)
/* Returns True if the write succeeds */
static inline bool lkdtm_try_write(u32 data, u32 *addr)
{
-#ifdef CONFIG_PPC
- __put_kernel_nofault(addr, &data, u32, err);
- return true;
-
-err:
- return false;
-#endif
-#ifdef CONFIG_X86_64
return !__put_user(data, addr);
-#endif
}
static int lkdtm_patching_cpu(void *data)
--
2.26.1
^ permalink raw reply related
* [PATCH v4 01/11] powerpc: Add LKDTM accessor for patching addr
From: Christopher M. Riedl @ 2021-04-29 7:20 UTC (permalink / raw)
To: linuxppc-dev; +Cc: tglx, x86, linux-hardening, keescook
In-Reply-To: <20210429072057.8870-1-cmr@bluescreens.de>
When live patching with STRICT_KERNEL_RWX a mapping is installed at a
"patching address" with temporary write permissions. Provide a
LKDTM-only accessor function for this address in preparation for a LKDTM
test which attempts to "hijack" this mapping by writing to it from
another CPU.
Signed-off-by: Christopher M. Riedl <cmr@bluescreens.de>
---
arch/powerpc/include/asm/code-patching.h | 4 ++++
arch/powerpc/lib/code-patching.c | 7 +++++++
2 files changed, 11 insertions(+)
diff --git a/arch/powerpc/include/asm/code-patching.h b/arch/powerpc/include/asm/code-patching.h
index f1d029bf906e5..e51c81e4a9bda 100644
--- a/arch/powerpc/include/asm/code-patching.h
+++ b/arch/powerpc/include/asm/code-patching.h
@@ -188,4 +188,8 @@ static inline unsigned long ppc_kallsyms_lookup_name(const char *name)
___PPC_RA(__REG_R1) | PPC_LR_STKOFF)
#endif /* CONFIG_PPC64 */
+#if IS_BUILTIN(CONFIG_LKDTM) && IS_ENABLED(CONFIG_STRICT_KERNEL_RWX)
+unsigned long read_cpu_patching_addr(unsigned int cpu);
+#endif
+
#endif /* _ASM_POWERPC_CODE_PATCHING_H */
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 870b30d9be2f8..2b1b3e9043ade 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -48,6 +48,13 @@ int raw_patch_instruction(struct ppc_inst *addr, struct ppc_inst instr)
#ifdef CONFIG_STRICT_KERNEL_RWX
static DEFINE_PER_CPU(struct vm_struct *, text_poke_area);
+#if IS_BUILTIN(CONFIG_LKDTM)
+unsigned long read_cpu_patching_addr(unsigned int cpu)
+{
+ return (unsigned long)(per_cpu(text_poke_area, cpu))->addr;
+}
+#endif
+
static int text_area_cpu_up(unsigned int cpu)
{
struct vm_struct *area;
--
2.26.1
^ permalink raw reply related
* [PATCH v4 04/11] lkdtm/x86_64: Add test to hijack a patch mapping
From: Christopher M. Riedl @ 2021-04-29 7:20 UTC (permalink / raw)
To: linuxppc-dev; +Cc: tglx, x86, linux-hardening, keescook
In-Reply-To: <20210429072057.8870-1-cmr@bluescreens.de>
A previous commit implemented an LKDTM test on powerpc to exploit the
temporary mapping established when patching code with STRICT_KERNEL_RWX
enabled. Extend the test to work on x86_64 as well.
Signed-off-by: Christopher M. Riedl <cmr@bluescreens.de>
---
drivers/misc/lkdtm/perms.c | 29 ++++++++++++++++++++++++++---
1 file changed, 26 insertions(+), 3 deletions(-)
diff --git a/drivers/misc/lkdtm/perms.c b/drivers/misc/lkdtm/perms.c
index c6f96ebffccfd..55c3bec6d3b72 100644
--- a/drivers/misc/lkdtm/perms.c
+++ b/drivers/misc/lkdtm/perms.c
@@ -224,7 +224,7 @@ void lkdtm_ACCESS_NULL(void)
}
#if (IS_BUILTIN(CONFIG_LKDTM) && defined(CONFIG_STRICT_KERNEL_RWX) && \
- defined(CONFIG_PPC))
+ (defined(CONFIG_PPC) || defined(CONFIG_X86_64)))
/*
* This is just a dummy location to patch-over.
*/
@@ -233,28 +233,51 @@ static void patching_target(void)
return;
}
+#ifdef CONFIG_PPC
#include <asm/code-patching.h>
struct ppc_inst * const patch_site = (struct ppc_inst *)&patching_target;
+#endif
+
+#ifdef CONFIG_X86_64
+#include <asm/text-patching.h>
+u32 * const patch_site = (u32 *)&patching_target;
+#endif
static inline int lkdtm_do_patch(u32 data)
{
+#ifdef CONFIG_PPC
return patch_instruction(patch_site, ppc_inst(data));
+#endif
+#ifdef CONFIG_X86_64
+ text_poke(patch_site, &data, sizeof(u32));
+ return 0;
+#endif
}
static inline u32 lkdtm_read_patch_site(void)
{
+#ifdef CONFIG_PPC
struct ppc_inst inst = READ_ONCE(*patch_site);
return ppc_inst_val(ppc_inst_read(&inst));
+#endif
+#ifdef CONFIG_X86_64
+ return READ_ONCE(*patch_site);
+#endif
}
/* Returns True if the write succeeds */
static inline bool lkdtm_try_write(u32 data, u32 *addr)
{
+#ifdef CONFIG_PPC
__put_kernel_nofault(addr, &data, u32, err);
return true;
err:
return false;
+#endif
+#ifdef CONFIG_X86_64
+ return !__put_user(data, addr);
+#endif
}
static int lkdtm_patching_cpu(void *data)
@@ -347,8 +370,8 @@ void lkdtm_HIJACK_PATCH(void)
void lkdtm_HIJACK_PATCH(void)
{
- if (!IS_ENABLED(CONFIG_PPC))
- pr_err("XFAIL: this test only runs on powerpc\n");
+ if (!IS_ENABLED(CONFIG_PPC) && !IS_ENABLED(CONFIG_X86_64))
+ pr_err("XFAIL: this test only runs on powerpc and x86_64\n");
if (!IS_ENABLED(CONFIG_STRICT_KERNEL_RWX))
pr_err("XFAIL: this test requires CONFIG_STRICT_KERNEL_RWX\n");
if (!IS_BUILTIN(CONFIG_LKDTM))
--
2.26.1
^ permalink raw reply related
* [PATCH v4 10/11] powerpc: Protect patching_mm with a lock
From: Christopher M. Riedl @ 2021-04-29 7:20 UTC (permalink / raw)
To: linuxppc-dev; +Cc: tglx, x86, linux-hardening, keescook
In-Reply-To: <20210429072057.8870-1-cmr@bluescreens.de>
Powerpc allows for multiple CPUs to patch concurrently. When patching
with STRICT_KERNEL_RWX a single patching_mm is allocated for use by all
CPUs for the few times that patching occurs. Use a spinlock to protect
the patching_mm from concurrent use.
Modify patch_instruction() to acquire the lock, perform the patch op,
and then release the lock.
Also introduce {lock,unlock}_patching() along with
patch_instruction_unlocked() to avoid per-iteration lock overhead when
patch_instruction() is called in a loop. A follow-up patch converts some
uses of patch_instruction() to use patch_instruction_unlocked() instead.
Signed-off-by: Christopher M. Riedl <cmr@bluescreens.de>
---
v4: * New to series.
---
arch/powerpc/include/asm/code-patching.h | 4 ++
arch/powerpc/lib/code-patching.c | 85 +++++++++++++++++++++---
2 files changed, 79 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/include/asm/code-patching.h b/arch/powerpc/include/asm/code-patching.h
index e51c81e4a9bda..2efa11b68cd8f 100644
--- a/arch/powerpc/include/asm/code-patching.h
+++ b/arch/powerpc/include/asm/code-patching.h
@@ -28,8 +28,12 @@ int create_branch(struct ppc_inst *instr, const struct ppc_inst *addr,
int create_cond_branch(struct ppc_inst *instr, const struct ppc_inst *addr,
unsigned long target, int flags);
int patch_branch(struct ppc_inst *addr, unsigned long target, int flags);
+int patch_branch_unlocked(struct ppc_inst *addr, unsigned long target, int flags);
int patch_instruction(struct ppc_inst *addr, struct ppc_inst instr);
+int patch_instruction_unlocked(struct ppc_inst *addr, struct ppc_inst instr);
int raw_patch_instruction(struct ppc_inst *addr, struct ppc_inst instr);
+unsigned long lock_patching(void);
+void unlock_patching(unsigned long flags);
static inline unsigned long patch_site_addr(s32 *site)
{
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 7e15abc09ec04..0a496bb52bbf4 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -52,13 +52,17 @@ int raw_patch_instruction(struct ppc_inst *addr, struct ppc_inst instr)
#ifdef CONFIG_STRICT_KERNEL_RWX
+static DEFINE_SPINLOCK(patching_lock);
+
struct temp_mm {
struct mm_struct *temp;
struct mm_struct *prev;
struct arch_hw_breakpoint brk[HBP_NUM_MAX];
+ spinlock_t *lock; /* protect access to the temporary mm */
};
-static inline void init_temp_mm(struct temp_mm *temp_mm, struct mm_struct *mm)
+static inline void init_temp_mm(struct temp_mm *temp_mm, struct mm_struct *mm,
+ spinlock_t *lock)
{
/* Do not preload SLB entries from the thread_info struct */
if (IS_ENABLED(CONFIG_PPC_BOOK3S_64) && !radix_enabled())
@@ -66,12 +70,14 @@ static inline void init_temp_mm(struct temp_mm *temp_mm, struct mm_struct *mm)
temp_mm->temp = mm;
temp_mm->prev = NULL;
+ temp_mm->lock = lock;
memset(&temp_mm->brk, 0, sizeof(temp_mm->brk));
}
static inline void use_temporary_mm(struct temp_mm *temp_mm)
{
lockdep_assert_irqs_disabled();
+ lockdep_assert_held(temp_mm->lock);
temp_mm->prev = current->active_mm;
switch_mm_irqs_off(temp_mm->prev, temp_mm->temp, current);
@@ -93,11 +99,13 @@ static inline void use_temporary_mm(struct temp_mm *temp_mm)
static inline void unuse_temporary_mm(struct temp_mm *temp_mm)
{
lockdep_assert_irqs_disabled();
+ lockdep_assert_held(temp_mm->lock);
switch_mm_irqs_off(temp_mm->temp, temp_mm->prev, current);
/*
- * On book3s64 the active_cpus counter increments in
+ * The temporary mm can only be in use on a single CPU at a time due to
+ * the temp_mm->lock. On book3s64 the active_cpus counter increments in
* switch_mm_irqs_off(). With the Hash MMU this counter affects if TLB
* flushes are local. We have to manually decrement that counter here
* along with removing our current CPU from the mm's cpumask so that in
@@ -230,7 +238,7 @@ static int map_patch(const void *addr, struct patch_mapping *patch_mapping)
pte = pte_mkdirty(pte);
set_pte_at(patching_mm, patching_addr, patch_mapping->ptep, pte);
- init_temp_mm(&patch_mapping->temp_mm, patching_mm);
+ init_temp_mm(&patch_mapping->temp_mm, patching_mm, &patching_lock);
use_temporary_mm(&patch_mapping->temp_mm);
/*
@@ -258,7 +266,6 @@ static int do_patch_instruction(struct ppc_inst *addr, struct ppc_inst instr)
{
int err;
struct ppc_inst *patch_addr = NULL;
- unsigned long flags;
struct patch_mapping patch_mapping;
/*
@@ -269,11 +276,12 @@ static int do_patch_instruction(struct ppc_inst *addr, struct ppc_inst instr)
if (!patching_mm)
return raw_patch_instruction(addr, instr);
- local_irq_save(flags);
+ lockdep_assert_held(&patching_lock);
+ lockdep_assert_irqs_disabled();
err = map_patch(addr, &patch_mapping);
if (err)
- goto out;
+ return err;
patch_addr = (struct ppc_inst *)(patching_addr | offset_in_page(addr));
@@ -287,11 +295,33 @@ static int do_patch_instruction(struct ppc_inst *addr, struct ppc_inst instr)
WARN_ON(!ppc_inst_equal(ppc_inst_read(addr), instr));
-out:
- local_irq_restore(flags);
-
return err;
}
+
+unsigned long lock_patching(void)
+{
+ unsigned long flags;
+
+ /* We don't need the lock if we're not using the patching_mm. */
+ if (!patching_mm)
+ return 0;
+
+ spin_lock_irqsave(&patching_lock, flags);
+ return flags;
+}
+
+void unlock_patching(const unsigned long flags)
+{
+ /* We never held the lock if we're not using the patching_mm. */
+ if (!patching_mm)
+ return;
+
+ lockdep_assert_held(&patching_lock);
+ lockdep_assert_irqs_disabled();
+
+ spin_unlock_irqrestore(&patching_lock, flags);
+}
+
#else /* !CONFIG_STRICT_KERNEL_RWX */
static int do_patch_instruction(struct ppc_inst *addr, struct ppc_inst instr)
@@ -299,19 +329,46 @@ static int do_patch_instruction(struct ppc_inst *addr, struct ppc_inst instr)
return raw_patch_instruction(addr, instr);
}
+unsigned long lock_patching(void)
+{
+ return 0;
+}
+
+void unlock_patching(const unsigned long flags) {}
+
#endif /* CONFIG_STRICT_KERNEL_RWX */
int patch_instruction(struct ppc_inst *addr, struct ppc_inst instr)
{
+ int err;
+ unsigned long flags;
+
/* Make sure we aren't patching a freed init section */
if (init_mem_is_free && init_section_contains(addr, 4)) {
pr_debug("Skipping init section patching addr: 0x%px\n", addr);
return 0;
}
- return do_patch_instruction(addr, instr);
+
+ flags = lock_patching();
+ err = do_patch_instruction(addr, instr);
+ unlock_patching(flags);
+
+ return err;
}
NOKPROBE_SYMBOL(patch_instruction);
+int patch_instruction_unlocked(struct ppc_inst *addr, struct ppc_inst instr)
+{
+ /* Make sure we aren't patching a freed init section */
+ if (init_mem_is_free && init_section_contains(addr, 4)) {
+ pr_debug("Skipping init section patching addr: 0x%p\n", addr);
+ return 0;
+ }
+
+ return do_patch_instruction(addr, instr);
+}
+NOKPROBE_SYMBOL(patch_instruction_unlocked);
+
int patch_branch(struct ppc_inst *addr, unsigned long target, int flags)
{
struct ppc_inst instr;
@@ -320,6 +377,14 @@ int patch_branch(struct ppc_inst *addr, unsigned long target, int flags)
return patch_instruction(addr, instr);
}
+int patch_branch_unlocked(struct ppc_inst *addr, unsigned long target, int flags)
+{
+ struct ppc_inst instr;
+
+ create_branch(&instr, addr, target, flags);
+ return patch_instruction_unlocked(addr, instr);
+}
+
bool is_offset_in_branch_range(long offset)
{
/*
--
2.26.1
^ permalink raw reply related
* [PATCH v4 00/11] Use per-CPU temporary mappings for patching
From: Christopher M. Riedl @ 2021-04-29 7:20 UTC (permalink / raw)
To: linuxppc-dev; +Cc: tglx, x86, linux-hardening, keescook
When compiled with CONFIG_STRICT_KERNEL_RWX, the kernel must create
temporary mappings when patching itself. These mappings temporarily
override the strict RWX text protections to permit a write. Currently,
powerpc allocates a per-CPU VM area for patching. Patching occurs as
follows:
1. Map page in per-CPU VM area w/ PAGE_KERNEL protection
2. Patch text
3. Remove the temporary mapping
While the VM area is per-CPU, the mapping is actually inserted into the
kernel page tables. Presumably, this could allow another CPU to access
the normally write-protected text - either malicously or accidentally -
via this same mapping if the address of the VM area is known. Ideally,
the mapping should be kept local to the CPU doing the patching [0].
x86 introduced "temporary mm" structs which allow the creation of
mappings local to a particular CPU [1]. This series intends to bring the
notion of a temporary mm to powerpc and harden powerpc by using such a
mapping for patching a kernel with strict RWX permissions.
The first four patches implement an LKDTM test "proof-of-concept" which
exploits the potential vulnerability (ie. the temporary mapping during
patching is exposed in the kernel page tables and accessible by other
CPUs) using a simple brute-force approach. This test is implemented for
both powerpc and x86_64. The test passes on powerpc with this new
series, fails on upstream powerpc, passes on upstream x86_64, and fails
on an older (ancient) x86_64 tree without the x86_64 temporary mm
patches. The remaining patches add support for and use a temporary mm
for code patching on powerpc.
Tested boot, ftrace, and repeated LKDTM "hijack":
- QEMU+KVM (host: POWER9 Blackbird): Radix MMU w/ KUAP
- QEMU+KVM (host: POWER9 Blackbird): Hash MMU w/o KUAP
- QEMU+KVM (host: POWER9 Blackbird): Hash MMU w/ KUAP
Tested repeated LKDTM "hijack":
- QEMU+KVM (host: AMD desktop): x86_64 upstream
- QEMU+KVM (host: AMD desktop): x86_64 w/o percpu temp mm to
verify the LKDTM "hijack" fails
Tested boot and ftrace:
- QEMU+TCG: ppc44x (bamboo)
- QEMU+TCG: g5 (mac99)
I also tested with various extra config options enabled as suggested in
section 12) in Documentation/process/submit-checklist.rst.
v4: * It's time to revisit this series again since @jpn and @mpe fixed
our known STRICT_*_RWX bugs on powerpc/64s.
* Rebase on linuxppc/next:
commit ee1bc694fbaec ("powerpc/kvm: Fix build error when PPC_MEM_KEYS/PPC_PSERIES=n")
* Completely rework how map_patch() works on book3s64 Hash MMU
* Split the LKDTM x86_64 and powerpc bits into separate patches
* Annotate commit messages with changes from v3 instead of
listing them here completely out-of context...
v3: * Rebase on linuxppc/next: commit 9123e3a74ec7 ("Linux 5.9-rc1")
* Move temporary mm implementation into code-patching.c where it
belongs
* Implement LKDTM hijacker test on x86_64 (on IBM time oof) Do
* not use address zero for the patching address in the
temporary mm (thanks @dja for pointing this out!)
* Wrap the LKDTM test w/ CONFIG_SMP as suggested by Christophe
Leroy
* Comments to clarify PTE pre-allocation and patching addr
selection
v2: * Rebase on linuxppc/next:
commit 105fb38124a4 ("powerpc/8xx: Modify ptep_get()")
* Always dirty pte when mapping patch
* Use `ppc_inst_len` instead of `sizeof` on instructions
* Declare LKDTM patching addr accessor in header where it belongs
v1: * Rebase on linuxppc/next (4336b9337824)
* Save and restore second hw watchpoint
* Use new ppc_inst_* functions for patching check and in LKDTM test
rfc-v2: * Many fixes and improvements mostly based on extensive feedback
and testing by Christophe Leroy (thanks!).
* Make patching_mm and patching_addr static and move
'__ro_after_init' to after the variable name (more common in
other parts of the kernel)
* Use 'asm/debug.h' header instead of 'asm/hw_breakpoint.h' to
fix PPC64e compile
* Add comment explaining why we use BUG_ON() during the init
call to setup for patching later
* Move ptep into patch_mapping to avoid walking page tables a
second time when unmapping the temporary mapping
* Use KUAP under non-radix, also manually dirty the PTE for patch
mapping on non-BOOK3S_64 platforms
* Properly return any error from __patch_instruction
* Do not use 'memcmp' where a simple comparison is appropriate
* Simplify expression for patch address by removing pointer maths
* Add LKDTM test
[0]: https://github.com/linuxppc/issues/issues/224
[1]: https://lore.kernel.org/kernel-hardening/20190426232303.28381-1-nadav.amit@gmail.com/
Christopher M. Riedl (11):
powerpc: Add LKDTM accessor for patching addr
lkdtm/powerpc: Add test to hijack a patch mapping
x86_64: Add LKDTM accessor for patching addr
lkdtm/x86_64: Add test to hijack a patch mapping
powerpc/64s: Add ability to skip SLB preload
powerpc: Introduce temporary mm
powerpc/64s: Make slb_allocate_user() non-static
powerpc: Initialize and use a temporary mm for patching
lkdtm/powerpc: Fix code patching hijack test
powerpc: Protect patching_mm with a lock
powerpc: Use patch_instruction_unlocked() in loops
arch/powerpc/include/asm/book3s/64/mmu-hash.h | 1 +
arch/powerpc/include/asm/book3s/64/mmu.h | 3 +
arch/powerpc/include/asm/code-patching.h | 8 +
arch/powerpc/include/asm/debug.h | 1 +
arch/powerpc/include/asm/mmu_context.h | 13 +
arch/powerpc/kernel/epapr_paravirt.c | 9 +-
arch/powerpc/kernel/optprobes.c | 22 +-
arch/powerpc/kernel/process.c | 5 +
arch/powerpc/lib/code-patching.c | 348 +++++++++++++-----
arch/powerpc/lib/feature-fixups.c | 114 ++++--
arch/powerpc/mm/book3s64/mmu_context.c | 2 +
arch/powerpc/mm/book3s64/slb.c | 60 +--
arch/powerpc/xmon/xmon.c | 22 +-
arch/x86/include/asm/text-patching.h | 4 +
arch/x86/kernel/alternative.c | 7 +
drivers/misc/lkdtm/core.c | 1 +
drivers/misc/lkdtm/lkdtm.h | 1 +
drivers/misc/lkdtm/perms.c | 149 ++++++++
18 files changed, 608 insertions(+), 162 deletions(-)
--
2.26.1
^ permalink raw reply
* [PATCH v4 11/11] powerpc: Use patch_instruction_unlocked() in loops
From: Christopher M. Riedl @ 2021-04-29 7:20 UTC (permalink / raw)
To: linuxppc-dev; +Cc: tglx, x86, linux-hardening, keescook
In-Reply-To: <20210429072057.8870-1-cmr@bluescreens.de>
Now that patching requires a lock to prevent concurrent access to
patching_mm, every call to patch_instruction() acquires and releases a
spinlock. There are several places where patch_instruction() is called
in a loop. Convert these to acquire the lock once before the loop, call
patch_instruction_unlocked() in the loop body, and then release the lock
again after the loop terminates - as in:
for (i = 0; i < n; ++i)
patch_instruction(...); <-- lock/unlock every iteration
changes to:
flags = lock_patching(); <-- lock once
for (i = 0; i < n; ++i)
patch_instruction_unlocked(...);
unlock_patching(flags); <-- unlock once
Signed-off-by: Christopher M. Riedl <cmr@bluescreens.de>
---
v4: * New to series.
---
arch/powerpc/kernel/epapr_paravirt.c | 9 ++-
arch/powerpc/kernel/optprobes.c | 22 ++++--
arch/powerpc/lib/feature-fixups.c | 114 +++++++++++++++++++--------
arch/powerpc/xmon/xmon.c | 22 ++++--
4 files changed, 120 insertions(+), 47 deletions(-)
diff --git a/arch/powerpc/kernel/epapr_paravirt.c b/arch/powerpc/kernel/epapr_paravirt.c
index 2ed14d4a47f59..b639e71cf9dec 100644
--- a/arch/powerpc/kernel/epapr_paravirt.c
+++ b/arch/powerpc/kernel/epapr_paravirt.c
@@ -28,6 +28,7 @@ static int __init early_init_dt_scan_epapr(unsigned long node,
const u32 *insts;
int len;
int i;
+ unsigned long flags;
insts = of_get_flat_dt_prop(node, "hcall-instructions", &len);
if (!insts)
@@ -36,14 +37,18 @@ static int __init early_init_dt_scan_epapr(unsigned long node,
if (len % 4 || len > (4 * 4))
return -1;
+ flags = lock_patching();
+
for (i = 0; i < (len / 4); i++) {
struct ppc_inst inst = ppc_inst(be32_to_cpu(insts[i]));
- patch_instruction((struct ppc_inst *)(epapr_hypercall_start + i), inst);
+ patch_instruction_unlocked((struct ppc_inst *)(epapr_hypercall_start + i), inst);
#if !defined(CONFIG_64BIT) || defined(CONFIG_PPC_BOOK3E_64)
- patch_instruction((struct ppc_inst *)(epapr_ev_idle_start + i), inst);
+ patch_instruction_unlocked((struct ppc_inst *)(epapr_ev_idle_start + i), inst);
#endif
}
+ unlock_patching(flags);
+
#if !defined(CONFIG_64BIT) || defined(CONFIG_PPC_BOOK3E_64)
if (of_get_flat_dt_prop(node, "has-idle", NULL))
epapr_has_idle = true;
diff --git a/arch/powerpc/kernel/optprobes.c b/arch/powerpc/kernel/optprobes.c
index cdf87086fa33a..deaeb6e8d1a00 100644
--- a/arch/powerpc/kernel/optprobes.c
+++ b/arch/powerpc/kernel/optprobes.c
@@ -200,7 +200,7 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
struct ppc_inst branch_op_callback, branch_emulate_step, temp;
kprobe_opcode_t *op_callback_addr, *emulate_step_addr, *buff;
long b_offset;
- unsigned long nip, size;
+ unsigned long nip, size, flags;
int rc, i;
kprobe_ppc_optinsn_slots.insn_size = MAX_OPTINSN_SIZE;
@@ -237,13 +237,20 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
/* We can optimize this via patch_instruction_window later */
size = (TMPL_END_IDX * sizeof(kprobe_opcode_t)) / sizeof(int);
pr_devel("Copying template to %p, size %lu\n", buff, size);
+
+ flags = lock_patching();
+
for (i = 0; i < size; i++) {
- rc = patch_instruction((struct ppc_inst *)(buff + i),
- ppc_inst(*(optprobe_template_entry + i)));
- if (rc < 0)
+ rc = patch_instruction_unlocked((struct ppc_inst *)(buff + i),
+ ppc_inst(*(optprobe_template_entry + i)));
+ if (rc < 0) {
+ unlock_patching(flags);
goto error;
+ }
}
+ unlock_patching(flags);
+
/*
* Fixup the template with instructions to:
* 1. load the address of the actual probepoint
@@ -322,6 +329,9 @@ void arch_optimize_kprobes(struct list_head *oplist)
struct ppc_inst instr;
struct optimized_kprobe *op;
struct optimized_kprobe *tmp;
+ unsigned long flags;
+
+ flags = lock_patching();
list_for_each_entry_safe(op, tmp, oplist, list) {
/*
@@ -333,9 +343,11 @@ void arch_optimize_kprobes(struct list_head *oplist)
create_branch(&instr,
(struct ppc_inst *)op->kp.addr,
(unsigned long)op->optinsn.insn, 0);
- patch_instruction((struct ppc_inst *)op->kp.addr, instr);
+ patch_instruction_unlocked((struct ppc_inst *)op->kp.addr, instr);
list_del_init(&op->list);
}
+
+ unlock_patching(flags);
}
void arch_unoptimize_kprobe(struct optimized_kprobe *op)
diff --git a/arch/powerpc/lib/feature-fixups.c b/arch/powerpc/lib/feature-fixups.c
index 1fd31b4b0e139..2c3d413c9d9b3 100644
--- a/arch/powerpc/lib/feature-fixups.c
+++ b/arch/powerpc/lib/feature-fixups.c
@@ -123,6 +123,7 @@ static void do_stf_entry_barrier_fixups(enum stf_barrier_type types)
unsigned int instrs[3], *dest;
long *start, *end;
int i;
+ unsigned long flags;
start = PTRRELOC(&__start___stf_entry_barrier_fixup);
end = PTRRELOC(&__stop___stf_entry_barrier_fixup);
@@ -144,24 +145,29 @@ static void do_stf_entry_barrier_fixups(enum stf_barrier_type types)
instrs[i++] = 0x63ff0000; /* ori 31,31,0 speculation barrier */
}
+ flags = lock_patching();
+
for (i = 0; start < end; start++, i++) {
dest = (void *)start + *start;
pr_devel("patching dest %lx\n", (unsigned long)dest);
- patch_instruction((struct ppc_inst *)dest, ppc_inst(instrs[0]));
+ patch_instruction_unlocked((struct ppc_inst *)dest, ppc_inst(instrs[0]));
if (types & STF_BARRIER_FALLBACK)
- patch_branch((struct ppc_inst *)(dest + 1),
- (unsigned long)&stf_barrier_fallback,
- BRANCH_SET_LINK);
+ patch_branch_unlocked((struct ppc_inst *)(dest + 1),
+ (unsigned long)&stf_barrier_fallback,
+ BRANCH_SET_LINK);
else
- patch_instruction((struct ppc_inst *)(dest + 1),
- ppc_inst(instrs[1]));
+ patch_instruction_unlocked((struct ppc_inst *)(dest + 1),
+ ppc_inst(instrs[1]));
- patch_instruction((struct ppc_inst *)(dest + 2), ppc_inst(instrs[2]));
+ patch_instruction_unlocked((struct ppc_inst *)(dest + 2),
+ ppc_inst(instrs[2]));
}
+ unlock_patching(flags);
+
printk(KERN_DEBUG "stf-barrier: patched %d entry locations (%s barrier)\n", i,
(types == STF_BARRIER_NONE) ? "no" :
(types == STF_BARRIER_FALLBACK) ? "fallback" :
@@ -175,6 +181,7 @@ static void do_stf_exit_barrier_fixups(enum stf_barrier_type types)
unsigned int instrs[6], *dest;
long *start, *end;
int i;
+ unsigned long flags;
start = PTRRELOC(&__start___stf_exit_barrier_fixup);
end = PTRRELOC(&__stop___stf_exit_barrier_fixup);
@@ -207,18 +214,23 @@ static void do_stf_exit_barrier_fixups(enum stf_barrier_type types)
instrs[i++] = 0x7e0006ac; /* eieio + bit 6 hint */
}
+ flags = lock_patching();
+
for (i = 0; start < end; start++, i++) {
dest = (void *)start + *start;
pr_devel("patching dest %lx\n", (unsigned long)dest);
- patch_instruction((struct ppc_inst *)dest, ppc_inst(instrs[0]));
- patch_instruction((struct ppc_inst *)(dest + 1), ppc_inst(instrs[1]));
- patch_instruction((struct ppc_inst *)(dest + 2), ppc_inst(instrs[2]));
- patch_instruction((struct ppc_inst *)(dest + 3), ppc_inst(instrs[3]));
- patch_instruction((struct ppc_inst *)(dest + 4), ppc_inst(instrs[4]));
- patch_instruction((struct ppc_inst *)(dest + 5), ppc_inst(instrs[5]));
+ patch_instruction_unlocked((struct ppc_inst *)dest, ppc_inst(instrs[0]));
+ patch_instruction_unlocked((struct ppc_inst *)(dest + 1), ppc_inst(instrs[1]));
+ patch_instruction_unlocked((struct ppc_inst *)(dest + 2), ppc_inst(instrs[2]));
+ patch_instruction_unlocked((struct ppc_inst *)(dest + 3), ppc_inst(instrs[3]));
+ patch_instruction_unlocked((struct ppc_inst *)(dest + 4), ppc_inst(instrs[4]));
+ patch_instruction_unlocked((struct ppc_inst *)(dest + 5), ppc_inst(instrs[5]));
}
+
+ unlock_patching(flags);
+
printk(KERN_DEBUG "stf-barrier: patched %d exit locations (%s barrier)\n", i,
(types == STF_BARRIER_NONE) ? "no" :
(types == STF_BARRIER_FALLBACK) ? "fallback" :
@@ -239,6 +251,7 @@ void do_uaccess_flush_fixups(enum l1d_flush_type types)
unsigned int instrs[4], *dest;
long *start, *end;
int i;
+ unsigned long flags;
start = PTRRELOC(&__start___uaccess_flush_fixup);
end = PTRRELOC(&__stop___uaccess_flush_fixup);
@@ -262,18 +275,22 @@ void do_uaccess_flush_fixups(enum l1d_flush_type types)
if (types & L1D_FLUSH_MTTRIG)
instrs[i++] = 0x7c12dba6; /* mtspr TRIG2,r0 (SPR #882) */
+ flags = lock_patching();
+
for (i = 0; start < end; start++, i++) {
dest = (void *)start + *start;
pr_devel("patching dest %lx\n", (unsigned long)dest);
- patch_instruction((struct ppc_inst *)dest, ppc_inst(instrs[0]));
+ patch_instruction_unlocked((struct ppc_inst *)dest, ppc_inst(instrs[0]));
- patch_instruction((struct ppc_inst *)(dest + 1), ppc_inst(instrs[1]));
- patch_instruction((struct ppc_inst *)(dest + 2), ppc_inst(instrs[2]));
- patch_instruction((struct ppc_inst *)(dest + 3), ppc_inst(instrs[3]));
+ patch_instruction_unlocked((struct ppc_inst *)(dest + 1), ppc_inst(instrs[1]));
+ patch_instruction_unlocked((struct ppc_inst *)(dest + 2), ppc_inst(instrs[2]));
+ patch_instruction_unlocked((struct ppc_inst *)(dest + 3), ppc_inst(instrs[3]));
}
+ unlock_patching(flags);
+
printk(KERN_DEBUG "uaccess-flush: patched %d locations (%s flush)\n", i,
(types == L1D_FLUSH_NONE) ? "no" :
(types == L1D_FLUSH_FALLBACK) ? "fallback displacement" :
@@ -289,6 +306,7 @@ void do_entry_flush_fixups(enum l1d_flush_type types)
unsigned int instrs[3], *dest;
long *start, *end;
int i;
+ unsigned long flags;
instrs[0] = 0x60000000; /* nop */
instrs[1] = 0x60000000; /* nop */
@@ -309,6 +327,8 @@ void do_entry_flush_fixups(enum l1d_flush_type types)
if (types & L1D_FLUSH_MTTRIG)
instrs[i++] = 0x7c12dba6; /* mtspr TRIG2,r0 (SPR #882) */
+ flags = lock_patching();
+
start = PTRRELOC(&__start___entry_flush_fixup);
end = PTRRELOC(&__stop___entry_flush_fixup);
for (i = 0; start < end; start++, i++) {
@@ -316,15 +336,17 @@ void do_entry_flush_fixups(enum l1d_flush_type types)
pr_devel("patching dest %lx\n", (unsigned long)dest);
- patch_instruction((struct ppc_inst *)dest, ppc_inst(instrs[0]));
+ patch_instruction_unlocked((struct ppc_inst *)dest, ppc_inst(instrs[0]));
if (types == L1D_FLUSH_FALLBACK)
- patch_branch((struct ppc_inst *)(dest + 1), (unsigned long)&entry_flush_fallback,
- BRANCH_SET_LINK);
+ patch_branch_unlocked((struct ppc_inst *)(dest + 1),
+ (unsigned long)&entry_flush_fallback,
+ BRANCH_SET_LINK);
else
- patch_instruction((struct ppc_inst *)(dest + 1), ppc_inst(instrs[1]));
+ patch_instruction_unlocked((struct ppc_inst *)(dest + 1),
+ ppc_inst(instrs[1]));
- patch_instruction((struct ppc_inst *)(dest + 2), ppc_inst(instrs[2]));
+ patch_instruction_unlocked((struct ppc_inst *)(dest + 2), ppc_inst(instrs[2]));
}
start = PTRRELOC(&__start___scv_entry_flush_fixup);
@@ -334,17 +356,20 @@ void do_entry_flush_fixups(enum l1d_flush_type types)
pr_devel("patching dest %lx\n", (unsigned long)dest);
- patch_instruction((struct ppc_inst *)dest, ppc_inst(instrs[0]));
+ patch_instruction_unlocked((struct ppc_inst *)dest, ppc_inst(instrs[0]));
if (types == L1D_FLUSH_FALLBACK)
- patch_branch((struct ppc_inst *)(dest + 1), (unsigned long)&scv_entry_flush_fallback,
- BRANCH_SET_LINK);
+ patch_branch_unlocked((struct ppc_inst *)(dest + 1),
+ (unsigned long)&scv_entry_flush_fallback,
+ BRANCH_SET_LINK);
else
- patch_instruction((struct ppc_inst *)(dest + 1), ppc_inst(instrs[1]));
+ patch_instruction_unlocked((struct ppc_inst *)(dest + 1),
+ ppc_inst(instrs[1]));
- patch_instruction((struct ppc_inst *)(dest + 2), ppc_inst(instrs[2]));
+ patch_instruction_unlocked((struct ppc_inst *)(dest + 2), ppc_inst(instrs[2]));
}
+ unlock_patching(flags);
printk(KERN_DEBUG "entry-flush: patched %d locations (%s flush)\n", i,
(types == L1D_FLUSH_NONE) ? "no" :
@@ -361,6 +386,7 @@ void do_rfi_flush_fixups(enum l1d_flush_type types)
unsigned int instrs[3], *dest;
long *start, *end;
int i;
+ unsigned long flags;
start = PTRRELOC(&__start___rfi_flush_fixup);
end = PTRRELOC(&__stop___rfi_flush_fixup);
@@ -382,16 +408,20 @@ void do_rfi_flush_fixups(enum l1d_flush_type types)
if (types & L1D_FLUSH_MTTRIG)
instrs[i++] = 0x7c12dba6; /* mtspr TRIG2,r0 (SPR #882) */
+ flags = lock_patching();
+
for (i = 0; start < end; start++, i++) {
dest = (void *)start + *start;
pr_devel("patching dest %lx\n", (unsigned long)dest);
- patch_instruction((struct ppc_inst *)dest, ppc_inst(instrs[0]));
- patch_instruction((struct ppc_inst *)(dest + 1), ppc_inst(instrs[1]));
- patch_instruction((struct ppc_inst *)(dest + 2), ppc_inst(instrs[2]));
+ patch_instruction_unlocked((struct ppc_inst *)dest, ppc_inst(instrs[0]));
+ patch_instruction_unlocked((struct ppc_inst *)(dest + 1), ppc_inst(instrs[1]));
+ patch_instruction_unlocked((struct ppc_inst *)(dest + 2), ppc_inst(instrs[2]));
}
+ unlock_patching(flags);
+
printk(KERN_DEBUG "rfi-flush: patched %d locations (%s flush)\n", i,
(types == L1D_FLUSH_NONE) ? "no" :
(types == L1D_FLUSH_FALLBACK) ? "fallback displacement" :
@@ -407,6 +437,7 @@ void do_barrier_nospec_fixups_range(bool enable, void *fixup_start, void *fixup_
unsigned int instr, *dest;
long *start, *end;
int i;
+ unsigned long flags;
start = fixup_start;
end = fixup_end;
@@ -418,13 +449,17 @@ void do_barrier_nospec_fixups_range(bool enable, void *fixup_start, void *fixup_
instr = 0x63ff0000; /* ori 31,31,0 speculation barrier */
}
+ flags = lock_patching();
+
for (i = 0; start < end; start++, i++) {
dest = (void *)start + *start;
pr_devel("patching dest %lx\n", (unsigned long)dest);
- patch_instruction((struct ppc_inst *)dest, ppc_inst(instr));
+ patch_instruction_unlocked((struct ppc_inst *)dest, ppc_inst(instr));
}
+ unlock_patching(flags);
+
printk(KERN_DEBUG "barrier-nospec: patched %d locations\n", i);
}
@@ -448,6 +483,7 @@ void do_barrier_nospec_fixups_range(bool enable, void *fixup_start, void *fixup_
unsigned int instr[2], *dest;
long *start, *end;
int i;
+ unsigned long flags;
start = fixup_start;
end = fixup_end;
@@ -461,27 +497,37 @@ void do_barrier_nospec_fixups_range(bool enable, void *fixup_start, void *fixup_
instr[1] = PPC_INST_SYNC;
}
+ flags = lock_patching();
+
for (i = 0; start < end; start++, i++) {
dest = (void *)start + *start;
pr_devel("patching dest %lx\n", (unsigned long)dest);
- patch_instruction((struct ppc_inst *)dest, ppc_inst(instr[0]));
- patch_instruction((struct ppc_inst *)(dest + 1), ppc_inst(instr[1]));
+ patch_instruction_unlocked((struct ppc_inst *)dest, ppc_inst(instr[0]));
+ patch_instruction_unlocked((struct ppc_inst *)(dest + 1), ppc_inst(instr[1]));
}
+ unlock_patching(flags);
+
printk(KERN_DEBUG "barrier-nospec: patched %d locations\n", i);
}
static void patch_btb_flush_section(long *curr)
{
unsigned int *start, *end;
+ unsigned long flags;
start = (void *)curr + *curr;
end = (void *)curr + *(curr + 1);
+
+ flags = lock_patching();
+
for (; start < end; start++) {
pr_devel("patching dest %lx\n", (unsigned long)start);
- patch_instruction((struct ppc_inst *)start, ppc_inst(PPC_INST_NOP));
+ patch_instruction_unlocked((struct ppc_inst *)start, ppc_inst(PPC_INST_NOP));
}
+
+ unlock_patching(flags);
}
void do_btb_flush_fixups(void)
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index ff2b92bfeedcc..e8a00041c04bf 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -905,6 +905,9 @@ static void insert_bpts(void)
int i;
struct ppc_inst instr, instr2;
struct bpt *bp, *bp2;
+ unsigned long flags;
+
+ flags = lock_patching();
bp = bpts;
for (i = 0; i < NBPTS; ++i, ++bp) {
@@ -945,19 +948,21 @@ static void insert_bpts(void)
continue;
}
- patch_instruction(bp->instr, instr);
- patch_instruction(ppc_inst_next(bp->instr, &instr),
- ppc_inst(bpinstr));
+ patch_instruction_unlocked(bp->instr, instr);
+ patch_instruction_unlocked(ppc_inst_next(bp->instr, &instr),
+ ppc_inst(bpinstr));
if (bp->enabled & BP_CIABR)
continue;
- if (patch_instruction((struct ppc_inst *)bp->address,
- ppc_inst(bpinstr)) != 0) {
+ if (patch_instruction_unlocked((struct ppc_inst *)bp->address,
+ ppc_inst(bpinstr)) != 0) {
printf("Couldn't write instruction at %lx, "
"disabling breakpoint there\n", bp->address);
bp->enabled &= ~BP_TRAP;
continue;
}
}
+
+ unlock_patching(flags);
}
static void insert_cpu_bpts(void)
@@ -984,6 +989,9 @@ static void remove_bpts(void)
int i;
struct bpt *bp;
struct ppc_inst instr;
+ unsigned long flags;
+
+ flags = lock_patching();
bp = bpts;
for (i = 0; i < NBPTS; ++i, ++bp) {
@@ -991,11 +999,13 @@ static void remove_bpts(void)
continue;
if (mread_instr(bp->address, &instr)
&& ppc_inst_equal(instr, ppc_inst(bpinstr))
- && patch_instruction(
+ && patch_instruction_unlocked(
(struct ppc_inst *)bp->address, ppc_inst_read(bp->instr)) != 0)
printf("Couldn't remove breakpoint at %lx\n",
bp->address);
}
+
+ unlock_patching(flags);
}
static void remove_cpu_bpts(void)
--
2.26.1
^ permalink raw reply related
* [PATCH v4 02/11] lkdtm/powerpc: Add test to hijack a patch mapping
From: Christopher M. Riedl @ 2021-04-29 7:20 UTC (permalink / raw)
To: linuxppc-dev; +Cc: tglx, x86, linux-hardening, keescook
In-Reply-To: <20210429072057.8870-1-cmr@bluescreens.de>
When live patching with STRICT_KERNEL_RWX the CPU doing the patching
must temporarily remap the page(s) containing the patch site with +W
permissions. While this temporary mapping is in use, another CPU could
write to the same mapping and maliciously alter kernel text. Implement a
LKDTM test to attempt to exploit such an opening during code patching.
The test is implemented on powerpc and requires LKDTM built into the
kernel (building LKDTM as a module is insufficient).
The LKDTM "hijack" test works as follows:
1. A CPU executes an infinite loop to patch an instruction. This is
the "patching" CPU.
2. Another CPU attempts to write to the address of the temporary
mapping used by the "patching" CPU. This other CPU is the
"hijacker" CPU. The hijack either fails with a fault/error or
succeeds, in which case some kernel text is now overwritten.
The virtual address of the temporary patch mapping is provided via an
LKDTM-specific accessor to the hijacker CPU. This test assumes a
hypothetical situation where this address was leaked previously.
How to run the test:
mount -t debugfs none /sys/kernel/debug
(echo HIJACK_PATCH > /sys/kernel/debug/provoke-crash/DIRECT)
A passing test indicates that it is not possible to overwrite kernel
text from another CPU by using the temporary mapping established by
a CPU for patching.
Signed-off-by: Christopher M. Riedl <cmr@bluescreens.de>
---
v4: * Separate the powerpc and x86_64 bits into individual patches.
* Use __put_kernel_nofault() when attempting to hijack the mapping
* Use raw_smp_processor_id() to avoid triggering the BUG() when
calling smp_processor_id() in preemptible code - the only thing
that matters is that one of the threads is bound to a different
CPU - we are not using smp_processor_id() to access any per-cpu
data or similar where preemption should be disabled.
* Rework the patching_cpu() kthread stop condition to avoid:
https://lwn.net/Articles/628628/
---
drivers/misc/lkdtm/core.c | 1 +
drivers/misc/lkdtm/lkdtm.h | 1 +
drivers/misc/lkdtm/perms.c | 135 +++++++++++++++++++++++++++++++++++++
3 files changed, 137 insertions(+)
diff --git a/drivers/misc/lkdtm/core.c b/drivers/misc/lkdtm/core.c
index b2aff4d87c014..857d218840eb8 100644
--- a/drivers/misc/lkdtm/core.c
+++ b/drivers/misc/lkdtm/core.c
@@ -146,6 +146,7 @@ static const struct crashtype crashtypes[] = {
CRASHTYPE(WRITE_RO),
CRASHTYPE(WRITE_RO_AFTER_INIT),
CRASHTYPE(WRITE_KERN),
+ CRASHTYPE(HIJACK_PATCH),
CRASHTYPE(REFCOUNT_INC_OVERFLOW),
CRASHTYPE(REFCOUNT_ADD_OVERFLOW),
CRASHTYPE(REFCOUNT_INC_NOT_ZERO_OVERFLOW),
diff --git a/drivers/misc/lkdtm/lkdtm.h b/drivers/misc/lkdtm/lkdtm.h
index 5ae48c64df24d..c8de54d189c27 100644
--- a/drivers/misc/lkdtm/lkdtm.h
+++ b/drivers/misc/lkdtm/lkdtm.h
@@ -61,6 +61,7 @@ void lkdtm_EXEC_USERSPACE(void);
void lkdtm_EXEC_NULL(void);
void lkdtm_ACCESS_USERSPACE(void);
void lkdtm_ACCESS_NULL(void);
+void lkdtm_HIJACK_PATCH(void);
/* refcount.c */
void lkdtm_REFCOUNT_INC_OVERFLOW(void);
diff --git a/drivers/misc/lkdtm/perms.c b/drivers/misc/lkdtm/perms.c
index 2dede2ef658f3..c6f96ebffccfd 100644
--- a/drivers/misc/lkdtm/perms.c
+++ b/drivers/misc/lkdtm/perms.c
@@ -9,6 +9,7 @@
#include <linux/vmalloc.h>
#include <linux/mman.h>
#include <linux/uaccess.h>
+#include <linux/kthread.h>
#include <asm/cacheflush.h>
/* Whether or not to fill the target memory area with do_nothing(). */
@@ -222,6 +223,140 @@ void lkdtm_ACCESS_NULL(void)
pr_err("FAIL: survived bad write\n");
}
+#if (IS_BUILTIN(CONFIG_LKDTM) && defined(CONFIG_STRICT_KERNEL_RWX) && \
+ defined(CONFIG_PPC))
+/*
+ * This is just a dummy location to patch-over.
+ */
+static void patching_target(void)
+{
+ return;
+}
+
+#include <asm/code-patching.h>
+struct ppc_inst * const patch_site = (struct ppc_inst *)&patching_target;
+
+static inline int lkdtm_do_patch(u32 data)
+{
+ return patch_instruction(patch_site, ppc_inst(data));
+}
+
+static inline u32 lkdtm_read_patch_site(void)
+{
+ struct ppc_inst inst = READ_ONCE(*patch_site);
+ return ppc_inst_val(ppc_inst_read(&inst));
+}
+
+/* Returns True if the write succeeds */
+static inline bool lkdtm_try_write(u32 data, u32 *addr)
+{
+ __put_kernel_nofault(addr, &data, u32, err);
+ return true;
+
+err:
+ return false;
+}
+
+static int lkdtm_patching_cpu(void *data)
+{
+ int err = 0;
+ u32 val = 0xdeadbeef;
+
+ pr_info("starting patching_cpu=%d\n", raw_smp_processor_id());
+
+ do {
+ err = lkdtm_do_patch(val);
+ } while (lkdtm_read_patch_site() == val && !err && !kthread_should_stop());
+
+ if (err)
+ pr_warn("XFAIL: patch_instruction returned error: %d\n", err);
+
+ while (!kthread_should_stop()) {
+ set_current_state(TASK_INTERRUPTIBLE);
+ schedule();
+ }
+
+ return err;
+}
+
+void lkdtm_HIJACK_PATCH(void)
+{
+ struct task_struct *patching_kthrd;
+ int patching_cpu, hijacker_cpu, attempts;
+ unsigned long addr;
+ bool hijacked;
+ const u32 bad_data = 0xbad00bad;
+ const u32 original_insn = lkdtm_read_patch_site();
+
+ if (!IS_ENABLED(CONFIG_SMP)) {
+ pr_err("XFAIL: this test requires CONFIG_SMP\n");
+ return;
+ }
+
+ if (num_online_cpus() < 2) {
+ pr_warn("XFAIL: this test requires at least two cpus\n");
+ return;
+ }
+
+ hijacker_cpu = raw_smp_processor_id();
+ patching_cpu = cpumask_any_but(cpu_online_mask, hijacker_cpu);
+
+ patching_kthrd = kthread_create_on_node(&lkdtm_patching_cpu, NULL,
+ cpu_to_node(patching_cpu),
+ "lkdtm_patching_cpu");
+ kthread_bind(patching_kthrd, patching_cpu);
+ wake_up_process(patching_kthrd);
+
+ addr = offset_in_page(patch_site) | read_cpu_patching_addr(patching_cpu);
+
+ pr_info("starting hijacker_cpu=%d\n", hijacker_cpu);
+ for (attempts = 0; attempts < 100000; ++attempts) {
+ /* Try to write to the other CPU's temp patch mapping */
+ hijacked = lkdtm_try_write(bad_data, (u32 *)addr);
+
+ if (hijacked) {
+ if (kthread_stop(patching_kthrd)) {
+ pr_info("hijack attempts: %d\n", attempts);
+ pr_err("XFAIL: error stopping patching cpu\n");
+ return;
+ }
+ break;
+ }
+ }
+ pr_info("hijack attempts: %d\n", attempts);
+
+ if (hijacked) {
+ if (lkdtm_read_patch_site() == bad_data)
+ pr_err("overwrote kernel text\n");
+ /*
+ * There are window conditions where the hijacker cpu manages to
+ * write to the patch site but the site gets overwritten again by
+ * the patching cpu. We still consider that a "successful" hijack
+ * since the hijacker cpu did not fault on the write.
+ */
+ pr_err("FAIL: wrote to another cpu's patching area\n");
+ } else {
+ kthread_stop(patching_kthrd);
+ }
+
+ /* Restore the original data to be able to run the test again */
+ lkdtm_do_patch(original_insn);
+}
+
+#else
+
+void lkdtm_HIJACK_PATCH(void)
+{
+ if (!IS_ENABLED(CONFIG_PPC))
+ pr_err("XFAIL: this test only runs on powerpc\n");
+ if (!IS_ENABLED(CONFIG_STRICT_KERNEL_RWX))
+ pr_err("XFAIL: this test requires CONFIG_STRICT_KERNEL_RWX\n");
+ if (!IS_BUILTIN(CONFIG_LKDTM))
+ pr_err("XFAIL: this test requires CONFIG_LKDTM=y (not =m!)\n");
+}
+
+#endif
+
void __init lkdtm_perms_init(void)
{
/* Make sure we can write to __ro_after_init values during __init */
--
2.26.1
^ permalink raw reply related
* [PATCH v4 05/11] powerpc/64s: Add ability to skip SLB preload
From: Christopher M. Riedl @ 2021-04-29 7:20 UTC (permalink / raw)
To: linuxppc-dev; +Cc: tglx, x86, linux-hardening, keescook
In-Reply-To: <20210429072057.8870-1-cmr@bluescreens.de>
Switching to a different mm with Hash translation causes SLB entries to
be preloaded from the current thread_info. This reduces SLB faults, for
example when threads share a common mm but operate on different address
ranges.
Preloading entries from the thread_info struct may not always be
appropriate - such as when switching to a temporary mm. Introduce a new
boolean in mm_context_t to skip the SLB preload entirely. Also move the
SLB preload code into a separate function since switch_slb() is already
quite long. The default behavior (preloading SLB entries from the
current thread_info struct) remains unchanged.
Signed-off-by: Christopher M. Riedl <cmr@bluescreens.de>
---
v4: * New to series.
---
arch/powerpc/include/asm/book3s/64/mmu.h | 3 ++
arch/powerpc/include/asm/mmu_context.h | 13 ++++++
arch/powerpc/mm/book3s64/mmu_context.c | 2 +
arch/powerpc/mm/book3s64/slb.c | 56 ++++++++++++++----------
4 files changed, 50 insertions(+), 24 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/64/mmu.h b/arch/powerpc/include/asm/book3s/64/mmu.h
index eace8c3f7b0a1..b23a9dcdee5af 100644
--- a/arch/powerpc/include/asm/book3s/64/mmu.h
+++ b/arch/powerpc/include/asm/book3s/64/mmu.h
@@ -130,6 +130,9 @@ typedef struct {
u32 pkey_allocation_map;
s16 execute_only_pkey; /* key holding execute-only protection */
#endif
+
+ /* Do not preload SLB entries from thread_info during switch_slb() */
+ bool skip_slb_preload;
} mm_context_t;
static inline u16 mm_ctx_user_psize(mm_context_t *ctx)
diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
index 4bc45d3ed8b0e..264787e90b1a1 100644
--- a/arch/powerpc/include/asm/mmu_context.h
+++ b/arch/powerpc/include/asm/mmu_context.h
@@ -298,6 +298,19 @@ static inline int arch_dup_mmap(struct mm_struct *oldmm,
return 0;
}
+#ifdef CONFIG_PPC_BOOK3S_64
+
+static inline void skip_slb_preload_mm(struct mm_struct *mm)
+{
+ mm->context.skip_slb_preload = true;
+}
+
+#else
+
+static inline void skip_slb_preload_mm(struct mm_struct *mm) {}
+
+#endif /* CONFIG_PPC_BOOK3S_64 */
+
#include <asm-generic/mmu_context.h>
#endif /* __KERNEL__ */
diff --git a/arch/powerpc/mm/book3s64/mmu_context.c b/arch/powerpc/mm/book3s64/mmu_context.c
index c10fc8a72fb37..3479910264c59 100644
--- a/arch/powerpc/mm/book3s64/mmu_context.c
+++ b/arch/powerpc/mm/book3s64/mmu_context.c
@@ -202,6 +202,8 @@ int init_new_context(struct task_struct *tsk, struct mm_struct *mm)
atomic_set(&mm->context.active_cpus, 0);
atomic_set(&mm->context.copros, 0);
+ mm->context.skip_slb_preload = false;
+
return 0;
}
diff --git a/arch/powerpc/mm/book3s64/slb.c b/arch/powerpc/mm/book3s64/slb.c
index c91bd85eb90e3..da0836cb855af 100644
--- a/arch/powerpc/mm/book3s64/slb.c
+++ b/arch/powerpc/mm/book3s64/slb.c
@@ -441,10 +441,39 @@ static void slb_cache_slbie_user(unsigned int index)
asm volatile("slbie %0" : : "r" (slbie_data));
}
+static void preload_slb_entries(struct task_struct *tsk, struct mm_struct *mm)
+{
+ struct thread_info *ti = task_thread_info(tsk);
+ unsigned char i;
+
+ /*
+ * We gradually age out SLBs after a number of context switches to
+ * reduce reload overhead of unused entries (like we do with FP/VEC
+ * reload). Each time we wrap 256 switches, take an entry out of the
+ * SLB preload cache.
+ */
+ tsk->thread.load_slb++;
+ if (!tsk->thread.load_slb) {
+ unsigned long pc = KSTK_EIP(tsk);
+
+ preload_age(ti);
+ preload_add(ti, pc);
+ }
+
+ for (i = 0; i < ti->slb_preload_nr; i++) {
+ unsigned char idx;
+ unsigned long ea;
+
+ idx = (ti->slb_preload_tail + i) % SLB_PRELOAD_NR;
+ ea = (unsigned long)ti->slb_preload_esid[idx] << SID_SHIFT;
+
+ slb_allocate_user(mm, ea);
+ }
+}
+
/* Flush all user entries from the segment table of the current processor. */
void switch_slb(struct task_struct *tsk, struct mm_struct *mm)
{
- struct thread_info *ti = task_thread_info(tsk);
unsigned char i;
/*
@@ -502,29 +531,8 @@ void switch_slb(struct task_struct *tsk, struct mm_struct *mm)
copy_mm_to_paca(mm);
- /*
- * We gradually age out SLBs after a number of context switches to
- * reduce reload overhead of unused entries (like we do with FP/VEC
- * reload). Each time we wrap 256 switches, take an entry out of the
- * SLB preload cache.
- */
- tsk->thread.load_slb++;
- if (!tsk->thread.load_slb) {
- unsigned long pc = KSTK_EIP(tsk);
-
- preload_age(ti);
- preload_add(ti, pc);
- }
-
- for (i = 0; i < ti->slb_preload_nr; i++) {
- unsigned char idx;
- unsigned long ea;
-
- idx = (ti->slb_preload_tail + i) % SLB_PRELOAD_NR;
- ea = (unsigned long)ti->slb_preload_esid[idx] << SID_SHIFT;
-
- slb_allocate_user(mm, ea);
- }
+ if (!mm->context.skip_slb_preload)
+ preload_slb_entries(tsk, mm);
/*
* Synchronize slbmte preloads with possible subsequent user memory
--
2.26.1
^ permalink raw reply related
* [PATCH v4 07/11] powerpc/64s: Make slb_allocate_user() non-static
From: Christopher M. Riedl @ 2021-04-29 7:20 UTC (permalink / raw)
To: linuxppc-dev; +Cc: tglx, x86, linux-hardening, keescook
In-Reply-To: <20210429072057.8870-1-cmr@bluescreens.de>
With Book3s64 Hash translation, manually inserting a PTE requires
updating the Linux PTE, inserting a SLB entry, and inserting the hashed
page. The first is handled via the usual kernel abstractions, the second
requires slb_allocate_user() which is currently 'static', and the third
is available via hash_page_mm() already.
Make slb_allocate_user() non-static and add a prototype so the next
patch can use it during code-patching.
Signed-off-by: Christopher M. Riedl <cmr@bluescreens.de>
---
v4: * New to series.
---
arch/powerpc/include/asm/book3s/64/mmu-hash.h | 1 +
arch/powerpc/mm/book3s64/slb.c | 4 +---
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/64/mmu-hash.h b/arch/powerpc/include/asm/book3s/64/mmu-hash.h
index 3004f3323144d..189854eebba77 100644
--- a/arch/powerpc/include/asm/book3s/64/mmu-hash.h
+++ b/arch/powerpc/include/asm/book3s/64/mmu-hash.h
@@ -525,6 +525,7 @@ void slb_dump_contents(struct slb_entry *slb_ptr);
extern void slb_vmalloc_update(void);
extern void slb_set_size(u16 size);
void preload_new_slb_context(unsigned long start, unsigned long sp);
+long slb_allocate_user(struct mm_struct *mm, unsigned long ea);
#endif /* __ASSEMBLY__ */
/*
diff --git a/arch/powerpc/mm/book3s64/slb.c b/arch/powerpc/mm/book3s64/slb.c
index da0836cb855af..532eb51bc5211 100644
--- a/arch/powerpc/mm/book3s64/slb.c
+++ b/arch/powerpc/mm/book3s64/slb.c
@@ -29,8 +29,6 @@
#include "internal.h"
-static long slb_allocate_user(struct mm_struct *mm, unsigned long ea);
-
bool stress_slb_enabled __initdata;
static int __init parse_stress_slb(char *p)
@@ -791,7 +789,7 @@ static long slb_allocate_kernel(unsigned long ea, unsigned long id)
return slb_insert_entry(ea, context, flags, ssize, true);
}
-static long slb_allocate_user(struct mm_struct *mm, unsigned long ea)
+long slb_allocate_user(struct mm_struct *mm, unsigned long ea)
{
unsigned long context;
unsigned long flags;
--
2.26.1
^ permalink raw reply related
* [PATCH v4 03/11] x86_64: Add LKDTM accessor for patching addr
From: Christopher M. Riedl @ 2021-04-29 7:20 UTC (permalink / raw)
To: linuxppc-dev; +Cc: tglx, x86, linux-hardening, keescook
In-Reply-To: <20210429072057.8870-1-cmr@bluescreens.de>
When live patching with STRICT_KERNEL_RWX a mapping is installed at a
"patching address" with temporary write permissions. Provide a
LKDTM-only accessor function for this address in preparation for a LKDTM
test which attempts to "hijack" this mapping by writing to it from
another CPU.
Signed-off-by: Christopher M. Riedl <cmr@bluescreens.de>
---
arch/x86/include/asm/text-patching.h | 4 ++++
arch/x86/kernel/alternative.c | 7 +++++++
2 files changed, 11 insertions(+)
diff --git a/arch/x86/include/asm/text-patching.h b/arch/x86/include/asm/text-patching.h
index b7421780e4e92..f0caf9ee13bd8 100644
--- a/arch/x86/include/asm/text-patching.h
+++ b/arch/x86/include/asm/text-patching.h
@@ -167,4 +167,8 @@ void int3_emulate_ret(struct pt_regs *regs)
}
#endif /* !CONFIG_UML_X86 */
+#if IS_BUILTIN(CONFIG_LKDTM)
+unsigned long read_cpu_patching_addr(unsigned int cpu);
+#endif
+
#endif /* _ASM_X86_TEXT_PATCHING_H */
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 8d778e46725d2..4c95fdd9b1965 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -852,6 +852,13 @@ static inline void unuse_temporary_mm(temp_mm_state_t prev_state)
__ro_after_init struct mm_struct *poking_mm;
__ro_after_init unsigned long poking_addr;
+#if IS_BUILTIN(CONFIG_LKDTM)
+unsigned long read_cpu_patching_addr(unsigned int cpu)
+{
+ return poking_addr;
+}
+#endif
+
static void *__text_poke(void *addr, const void *opcode, size_t len)
{
bool cross_page_boundary = offset_in_page(addr) + len > PAGE_SIZE;
--
2.26.1
^ permalink raw reply related
* [PATCH v4 08/11] powerpc: Initialize and use a temporary mm for patching
From: Christopher M. Riedl @ 2021-04-29 7:20 UTC (permalink / raw)
To: linuxppc-dev; +Cc: tglx, x86, linux-hardening, keescook
In-Reply-To: <20210429072057.8870-1-cmr@bluescreens.de>
When code patching a STRICT_KERNEL_RWX kernel the page containing the
address to be patched is temporarily mapped as writeable. Currently, a
per-cpu vmalloc patch area is used for this purpose. While the patch
area is per-cpu, the temporary page mapping is inserted into the kernel
page tables for the duration of patching. The mapping is exposed to CPUs
other than the patching CPU - this is undesirable from a hardening
perspective. Use a temporary mm instead which keeps the mapping local to
the CPU doing the patching.
Use the `poking_init` init hook to prepare a temporary mm and patching
address. Initialize the temporary mm by copying the init mm. Choose a
randomized patching address inside the temporary mm userspace address
space. The patching address is randomized between PAGE_SIZE and
DEFAULT_MAP_WINDOW-PAGE_SIZE. The upper limit is necessary due to how
the Book3s64 Hash MMU operates - by default the space above
DEFAULT_MAP_WINDOW is not available. For now, the patching address for
all platforms/MMUs is randomized inside this range. The number of
possible random addresses is dependent on PAGE_SIZE and limited by
DEFAULT_MAP_WINDOW.
Bits of entropy with 64K page size on BOOK3S_64:
bits of entropy = log2(DEFAULT_MAP_WINDOW_USER64 / PAGE_SIZE)
PAGE_SIZE=64K, DEFAULT_MAP_WINDOW_USER64=128TB
bits of entropy = log2(128TB / 64K) bits of entropy = 31
Randomization occurs only once during initialization at boot.
Introduce two new functions, map_patch() and unmap_patch(), to
respectively create and remove the temporary mapping with write
permissions at patching_addr. The Hash MMU on Book3s64 requires mapping
the page for patching with PAGE_SHARED since the kernel cannot access
userspace pages with the PAGE_PRIVILEGED (PAGE_KERNEL) bit set.
Also introduce hash_prefault_mapping() to preload the SLB entry and HPTE
for the patching_addr when using the Hash MMU on Book3s64 to avoid
taking an SLB and Hash fault during patching.
Since patching_addr is now a userspace address, lock/unlock KUAP on
non-Book3s64 platforms. On Book3s64 with a Radix MMU, mapping the page
with PAGE_KERNEL sets EAA[0] for the PTE which ignores the AMR (KUAP)
according to PowerISA v3.0b Figure 35. On Book3s64 with a Hash MMU, the
hash PTE for the mapping is inserted with HPTE_USE_KERNEL_KEY which
similarly avoids the need for switching KUAP.
Finally, add a new WARN_ON() to check that the instruction was patched
as intended after the temporary mapping is torn down.
Based on x86 implementation:
commit 4fc19708b165
("x86/alternatives: Initialize temporary mm for patching")
and:
commit b3fd8e83ada0
("x86/alternatives: Use temporary mm for text poking")
Signed-off-by: Christopher M. Riedl <cmr@bluescreens.de>
---
v4: * In the previous series this was two separate patches: one to init
the temporary mm in poking_init() (unused in powerpc at the time)
and the other to use it for patching (which removed all the
per-cpu vmalloc code). Now that we use poking_init() in the
existing per-cpu vmalloc approach, that separation doesn't work
as nicely anymore so I just merged the two patches into one.
* Preload the SLB entry and hash the page for the patching_addr
when using Hash on book3s64 to avoid taking an SLB and Hash fault
during patching. The previous implementation was a hack which
changed current->mm to allow the SLB and Hash fault handlers to
work with the temporary mm since both of those code-paths always
assume mm == current->mm.
* Also (hmm - seeing a trend here) with the book3s64 Hash MMU we
have to manage the mm->context.active_cpus counter and mm cpumask
since they determine (via mm_is_thread_local()) if the TLB flush
in pte_clear() is local or not - it should always be local when
we're using the temporary mm. On book3s64's Radix MMU we can
just call local_flush_tlb_mm().
* Use HPTE_USE_KERNEL_KEY on Hash to avoid costly lock/unlock of
KUAP.
---
arch/powerpc/lib/code-patching.c | 209 ++++++++++++++++++-------------
1 file changed, 121 insertions(+), 88 deletions(-)
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index cbdfba8a39360..7e15abc09ec04 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -11,6 +11,8 @@
#include <linux/cpuhotplug.h>
#include <linux/slab.h>
#include <linux/uaccess.h>
+#include <linux/sched/task.h>
+#include <linux/random.h>
#include <asm/tlbflush.h>
#include <asm/page.h>
@@ -19,6 +21,7 @@
#include <asm/inst.h>
#include <asm/mmu_context.h>
#include <asm/debug.h>
+#include <asm/tlb.h>
static int __patch_instruction(struct ppc_inst *exec_addr, struct ppc_inst instr,
struct ppc_inst *patch_addr)
@@ -113,113 +116,142 @@ static inline void unuse_temporary_mm(struct temp_mm *temp_mm)
}
}
-static DEFINE_PER_CPU(struct vm_struct *, text_poke_area);
+static struct mm_struct *patching_mm __ro_after_init;
+static unsigned long patching_addr __ro_after_init;
+
+void __init poking_init(void)
+{
+ spinlock_t *ptl; /* for protecting pte table */
+ pte_t *ptep;
+
+ /*
+ * Some parts of the kernel (static keys for example) depend on
+ * successful code patching. Code patching under STRICT_KERNEL_RWX
+ * requires this setup - otherwise we cannot patch at all. We use
+ * BUG_ON() here and later since an early failure is preferred to
+ * buggy behavior and/or strange crashes later.
+ */
+ patching_mm = copy_init_mm();
+ BUG_ON(!patching_mm);
+
+ /*
+ * Choose a randomized, page-aligned address from the range:
+ * [PAGE_SIZE, DEFAULT_MAP_WINDOW - PAGE_SIZE]
+ * The lower address bound is PAGE_SIZE to avoid the zero-page.
+ * The upper address bound is DEFAULT_MAP_WINDOW - PAGE_SIZE to stay
+ * under DEFAULT_MAP_WINDOW with the Book3s64 Hash MMU.
+ */
+ patching_addr = PAGE_SIZE + ((get_random_long() & PAGE_MASK)
+ % (DEFAULT_MAP_WINDOW - 2 * PAGE_SIZE));
+
+ /*
+ * PTE allocation uses GFP_KERNEL which means we need to pre-allocate
+ * the PTE here. We cannot do the allocation during patching with IRQs
+ * disabled (ie. "atomic" context).
+ */
+ ptep = get_locked_pte(patching_mm, patching_addr, &ptl);
+ BUG_ON(!ptep);
+ pte_unmap_unlock(ptep, ptl);
+}
#if IS_BUILTIN(CONFIG_LKDTM)
unsigned long read_cpu_patching_addr(unsigned int cpu)
{
- return (unsigned long)(per_cpu(text_poke_area, cpu))->addr;
+ return patching_addr;
}
#endif
-static int text_area_cpu_up(unsigned int cpu)
+struct patch_mapping {
+ spinlock_t *ptl; /* for protecting pte table */
+ pte_t *ptep;
+ struct temp_mm temp_mm;
+};
+
+#ifdef CONFIG_PPC_BOOK3S_64
+
+static inline int hash_prefault_mapping(pgprot_t pgprot)
{
- struct vm_struct *area;
+ int err;
- area = get_vm_area(PAGE_SIZE, VM_ALLOC);
- if (!area) {
- WARN_ONCE(1, "Failed to create text area for cpu %d\n",
- cpu);
- return -1;
- }
- this_cpu_write(text_poke_area, area);
+ if (radix_enabled())
+ return 0;
- return 0;
-}
+ err = slb_allocate_user(patching_mm, patching_addr);
+ if (err)
+ pr_warn("map patch: failed to allocate slb entry\n");
-static int text_area_cpu_down(unsigned int cpu)
-{
- free_vm_area(this_cpu_read(text_poke_area));
- return 0;
+ err = hash_page_mm(patching_mm, patching_addr, pgprot_val(pgprot), 0,
+ HPTE_USE_KERNEL_KEY);
+ if (err)
+ pr_warn("map patch: failed to insert hashed page\n");
+
+ /* See comment in switch_slb() in mm/book3s64/slb.c */
+ isync();
+
+ return err;
}
-/*
- * Run as a late init call. This allows all the boot time patching to be done
- * simply by patching the code, and then we're called here prior to
- * mark_rodata_ro(), which happens after all init calls are run. Although
- * BUG_ON() is rude, in this case it should only happen if ENOMEM, and we judge
- * it as being preferable to a kernel that will crash later when someone tries
- * to use patch_instruction().
- */
-static int __init setup_text_poke_area(void)
-{
- BUG_ON(!cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
- "powerpc/text_poke:online", text_area_cpu_up,
- text_area_cpu_down));
+#else
+static inline int hash_prefault_mapping(pgprot_t pgprot)
+{
return 0;
}
-late_initcall(setup_text_poke_area);
+
+#endif /* CONFIG_PPC_BOOK3S_64 */
/*
* This can be called for kernel text or a module.
*/
-static int map_patch_area(void *addr, unsigned long text_poke_addr)
+static int map_patch(const void *addr, struct patch_mapping *patch_mapping)
{
- unsigned long pfn;
- int err;
+ struct page *page;
+ pte_t pte;
+ pgprot_t pgprot;
if (is_vmalloc_or_module_addr(addr))
- pfn = vmalloc_to_pfn(addr);
+ page = vmalloc_to_page(addr);
else
- pfn = __pa_symbol(addr) >> PAGE_SHIFT;
+ page = virt_to_page(addr);
- err = map_kernel_page(text_poke_addr, (pfn << PAGE_SHIFT), PAGE_KERNEL);
+ if (radix_enabled())
+ pgprot = PAGE_KERNEL;
+ else
+ pgprot = PAGE_SHARED;
- pr_devel("Mapped addr %lx with pfn %lx:%d\n", text_poke_addr, pfn, err);
- if (err)
+ patch_mapping->ptep = get_locked_pte(patching_mm, patching_addr,
+ &patch_mapping->ptl);
+ if (unlikely(!patch_mapping->ptep)) {
+ pr_warn("map patch: failed to allocate pte for patching\n");
return -1;
+ }
- return 0;
-}
-
-static inline int unmap_patch_area(unsigned long addr)
-{
- pte_t *ptep;
- pmd_t *pmdp;
- pud_t *pudp;
- p4d_t *p4dp;
- pgd_t *pgdp;
-
- pgdp = pgd_offset_k(addr);
- if (unlikely(!pgdp))
- return -EINVAL;
-
- p4dp = p4d_offset(pgdp, addr);
- if (unlikely(!p4dp))
- return -EINVAL;
+ pte = mk_pte(page, pgprot);
+ pte = pte_mkdirty(pte);
+ set_pte_at(patching_mm, patching_addr, patch_mapping->ptep, pte);
- pudp = pud_offset(p4dp, addr);
- if (unlikely(!pudp))
- return -EINVAL;
+ init_temp_mm(&patch_mapping->temp_mm, patching_mm);
+ use_temporary_mm(&patch_mapping->temp_mm);
- pmdp = pmd_offset(pudp, addr);
- if (unlikely(!pmdp))
- return -EINVAL;
+ /*
+ * On Book3s64 with the Hash MMU we have to manually insert the SLB
+ * entry and HPTE to prevent taking faults on the patching_addr later.
+ */
+ return(hash_prefault_mapping(pgprot));
+}
- ptep = pte_offset_kernel(pmdp, addr);
- if (unlikely(!ptep))
- return -EINVAL;
+static void unmap_patch(struct patch_mapping *patch_mapping)
+{
+ /* Book3s64 Hash MMU: pte_clear() flushes the TLB */
+ pte_clear(patching_mm, patching_addr, patch_mapping->ptep);
- pr_devel("clearing mm %p, pte %p, addr %lx\n", &init_mm, ptep, addr);
+ /* Book3s64 Radix MMU: explicitly flush the TLB (no-op in Hash MMU) */
+ local_flush_tlb_mm(patching_mm);
- /*
- * In hash, pte_clear flushes the tlb, in radix, we have to
- */
- pte_clear(&init_mm, addr, ptep);
- flush_tlb_kernel_range(addr, addr + PAGE_SIZE);
+ pte_unmap_unlock(patch_mapping->ptep, patch_mapping->ptl);
- return 0;
+ /* Book3s64 Hash MMU: switch_mm_irqs_off() invalidates the SLB */
+ unuse_temporary_mm(&patch_mapping->temp_mm);
}
static int do_patch_instruction(struct ppc_inst *addr, struct ppc_inst instr)
@@ -227,32 +259,33 @@ static int do_patch_instruction(struct ppc_inst *addr, struct ppc_inst instr)
int err;
struct ppc_inst *patch_addr = NULL;
unsigned long flags;
- unsigned long text_poke_addr;
- unsigned long kaddr = (unsigned long)addr;
+ struct patch_mapping patch_mapping;
/*
- * During early early boot patch_instruction is called
- * when text_poke_area is not ready, but we still need
- * to allow patching. We just do the plain old patching
+ * The patching_mm is initialized before calling mark_rodata_ro. Prior
+ * to this, patch_instruction is called when we don't have (and don't
+ * need) the patching_mm so just do plain old patching.
*/
- if (!this_cpu_read(text_poke_area))
+ if (!patching_mm)
return raw_patch_instruction(addr, instr);
local_irq_save(flags);
- text_poke_addr = (unsigned long)__this_cpu_read(text_poke_area)->addr;
- if (map_patch_area(addr, text_poke_addr)) {
- err = -1;
+ err = map_patch(addr, &patch_mapping);
+ if (err)
goto out;
- }
- patch_addr = (struct ppc_inst *)(text_poke_addr + (kaddr & ~PAGE_MASK));
+ patch_addr = (struct ppc_inst *)(patching_addr | offset_in_page(addr));
- __patch_instruction(addr, instr, patch_addr);
+ if (!IS_ENABLED(CONFIG_PPC_BOOK3S_64))
+ allow_read_write_user(patch_addr, patch_addr, ppc_inst_len(instr));
+ err = __patch_instruction(addr, instr, patch_addr);
+ if (!IS_ENABLED(CONFIG_PPC_BOOK3S_64))
+ prevent_read_write_user(patch_addr, patch_addr, ppc_inst_len(instr));
- err = unmap_patch_area(text_poke_addr);
- if (err)
- pr_warn("failed to unmap %lx\n", text_poke_addr);
+ unmap_patch(&patch_mapping);
+
+ WARN_ON(!ppc_inst_equal(ppc_inst_read(addr), instr));
out:
local_irq_restore(flags);
--
2.26.1
^ permalink raw reply related
* [PATCH v4 06/11] powerpc: Introduce temporary mm
From: Christopher M. Riedl @ 2021-04-29 7:20 UTC (permalink / raw)
To: linuxppc-dev; +Cc: tglx, x86, linux-hardening, keescook
In-Reply-To: <20210429072057.8870-1-cmr@bluescreens.de>
x86 supports the notion of a temporary mm which restricts access to
temporary PTEs to a single CPU. A temporary mm is useful for situations
where a CPU needs to perform sensitive operations (such as patching a
STRICT_KERNEL_RWX kernel) requiring temporary mappings without exposing
said mappings to other CPUs. A side benefit is that other CPU TLBs do
not need to be flushed when the temporary mm is torn down.
Mappings in the temporary mm can be set in the userspace portion of the
address-space.
Interrupts must be disabled while the temporary mm is in use. HW
breakpoints, which may have been set by userspace as watchpoints on
addresses now within the temporary mm, are saved and disabled when
loading the temporary mm. The HW breakpoints are restored when unloading
the temporary mm. All HW breakpoints are indiscriminately disabled while
the temporary mm is in use.
With the Book3s64 Hash MMU the SLB is preloaded with entries from the
current thread_info struct during switch_slb(). This could cause a
Machine Check (MCE) due to an SLB Multihit when creating arbitrary
userspace mappings in the temporary mm later. Disable SLB preload from
the thread_info struct for any temporary mm to avoid this.
Based on x86 implementation:
commit cefa929c034e
("x86/mm: Introduce temporary mm structs")
Signed-off-by: Christopher M. Riedl <cmr@bluescreens.de>
---
v4: * Pass the prev mm instead of NULL to switch_mm_irqs_off() when
using/unusing the temp mm as suggested by Jann Horn to keep
the context.active counter in-sync on mm/nohash.
* Disable SLB preload in the temporary mm when initializing the
temp_mm struct.
* Include asm/debug.h header to fix build issue with
ppc44x_defconfig.
---
arch/powerpc/include/asm/debug.h | 1 +
arch/powerpc/kernel/process.c | 5 +++
arch/powerpc/lib/code-patching.c | 67 ++++++++++++++++++++++++++++++++
3 files changed, 73 insertions(+)
diff --git a/arch/powerpc/include/asm/debug.h b/arch/powerpc/include/asm/debug.h
index 86a14736c76c3..dfd82635ea8b3 100644
--- a/arch/powerpc/include/asm/debug.h
+++ b/arch/powerpc/include/asm/debug.h
@@ -46,6 +46,7 @@ static inline int debugger_fault_handler(struct pt_regs *regs) { return 0; }
#endif
void __set_breakpoint(int nr, struct arch_hw_breakpoint *brk);
+void __get_breakpoint(int nr, struct arch_hw_breakpoint *brk);
bool ppc_breakpoint_available(void);
#ifdef CONFIG_PPC_ADV_DEBUG_REGS
extern void do_send_trap(struct pt_regs *regs, unsigned long address,
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 89e34aa273e21..8e94cabaea3c3 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -864,6 +864,11 @@ static inline int set_breakpoint_8xx(struct arch_hw_breakpoint *brk)
return 0;
}
+void __get_breakpoint(int nr, struct arch_hw_breakpoint *brk)
+{
+ memcpy(brk, this_cpu_ptr(¤t_brk[nr]), sizeof(*brk));
+}
+
void __set_breakpoint(int nr, struct arch_hw_breakpoint *brk)
{
memcpy(this_cpu_ptr(¤t_brk[nr]), brk, sizeof(*brk));
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 2b1b3e9043ade..cbdfba8a39360 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -17,6 +17,8 @@
#include <asm/code-patching.h>
#include <asm/setup.h>
#include <asm/inst.h>
+#include <asm/mmu_context.h>
+#include <asm/debug.h>
static int __patch_instruction(struct ppc_inst *exec_addr, struct ppc_inst instr,
struct ppc_inst *patch_addr)
@@ -46,6 +48,71 @@ int raw_patch_instruction(struct ppc_inst *addr, struct ppc_inst instr)
}
#ifdef CONFIG_STRICT_KERNEL_RWX
+
+struct temp_mm {
+ struct mm_struct *temp;
+ struct mm_struct *prev;
+ struct arch_hw_breakpoint brk[HBP_NUM_MAX];
+};
+
+static inline void init_temp_mm(struct temp_mm *temp_mm, struct mm_struct *mm)
+{
+ /* Do not preload SLB entries from the thread_info struct */
+ if (IS_ENABLED(CONFIG_PPC_BOOK3S_64) && !radix_enabled())
+ skip_slb_preload_mm(mm);
+
+ temp_mm->temp = mm;
+ temp_mm->prev = NULL;
+ memset(&temp_mm->brk, 0, sizeof(temp_mm->brk));
+}
+
+static inline void use_temporary_mm(struct temp_mm *temp_mm)
+{
+ lockdep_assert_irqs_disabled();
+
+ temp_mm->prev = current->active_mm;
+ switch_mm_irqs_off(temp_mm->prev, temp_mm->temp, current);
+
+ WARN_ON(!mm_is_thread_local(temp_mm->temp));
+
+ if (ppc_breakpoint_available()) {
+ struct arch_hw_breakpoint null_brk = {0};
+ int i = 0;
+
+ for (; i < nr_wp_slots(); ++i) {
+ __get_breakpoint(i, &temp_mm->brk[i]);
+ if (temp_mm->brk[i].type != 0)
+ __set_breakpoint(i, &null_brk);
+ }
+ }
+}
+
+static inline void unuse_temporary_mm(struct temp_mm *temp_mm)
+{
+ lockdep_assert_irqs_disabled();
+
+ switch_mm_irqs_off(temp_mm->temp, temp_mm->prev, current);
+
+ /*
+ * On book3s64 the active_cpus counter increments in
+ * switch_mm_irqs_off(). With the Hash MMU this counter affects if TLB
+ * flushes are local. We have to manually decrement that counter here
+ * along with removing our current CPU from the mm's cpumask so that in
+ * the future a different CPU can reuse the temporary mm and still rely
+ * on local TLB flushes.
+ */
+ dec_mm_active_cpus(temp_mm->temp);
+ cpumask_clear_cpu(smp_processor_id(), mm_cpumask(temp_mm->temp));
+
+ if (ppc_breakpoint_available()) {
+ int i = 0;
+
+ for (; i < nr_wp_slots(); ++i)
+ if (temp_mm->brk[i].type != 0)
+ __set_breakpoint(i, &temp_mm->brk[i]);
+ }
+}
+
static DEFINE_PER_CPU(struct vm_struct *, text_poke_area);
#if IS_BUILTIN(CONFIG_LKDTM)
--
2.26.1
^ permalink raw reply related
* Re: [PATCH v11 1/9] powerpc/mm: Implement set_memory() routines
From: Christophe Leroy @ 2021-04-29 7:32 UTC (permalink / raw)
To: Jordan Niethe, linuxppc-dev
Cc: ajd, npiggin, cmr, aneesh.kumar, naveen.n.rao, dja
In-Reply-To: <20210429031602.2606654-2-jniethe5@gmail.com>
Le 29/04/2021 à 05:15, Jordan Niethe a écrit :
> From: Russell Currey <ruscur@russell.cc>
>
> The set_memory_{ro/rw/nx/x}() functions are required for
> STRICT_MODULE_RWX, and are generally useful primitives to have. This
> implementation is designed to be generic across powerpc's many MMUs.
> It's possible that this could be optimised to be faster for specific
> MMUs.
>
> This implementation does not handle cases where the caller is attempting
> to change the mapping of the page it is executing from, or if another
> CPU is concurrently using the page being altered. These cases likely
> shouldn't happen, but a more complex implementation with MMU-specific code
> could safely handle them.
>
> On hash, the linear mapping is not kept in the linux pagetable, so this
> will not change the protection if used on that range. Currently these
> functions are not used on the linear map so just WARN for now.
>
> Reviewed-by: Daniel Axtens <dja@axtens.net>
> Signed-off-by: Russell Currey <ruscur@russell.cc>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> [jpn: - Allow set memory functions to be used without Strict RWX
> - Hash: Disallow certain regions
> - Have change_page_attr() take function pointers to manipulate ptes
Did you look at the resulting generated code ? I find it awful.
pte manipulation helpers are meant to be inlined. Here you force the compiler to outline them. This
also means that the input and output goes through memory.
And now set_memory_xx are not tiny inlined functions anymore.
What is the reason you abandonned the way it was done up to now, through the use of an 'action'
value ? With the previous approach the generated code was a lot lighter.
> - Radix: Add ptesync after set_pte_at()]
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> ---
> v10: WARN if trying to change the hash linear map
> v11: - Update copywrite dates
> - Allow set memory functions to be used without Strict RWX
> - Hash: Disallow certain regions and add comment explaining why
> - Have change_page_attr() take function pointers to manipulate ptes
> - Clarify change_page_attr()'s comment
> - Radix: Add ptesync after set_pte_at()
> ---
> arch/powerpc/Kconfig | 1 +
> arch/powerpc/include/asm/set_memory.h | 10 +++
> arch/powerpc/mm/Makefile | 2 +-
> arch/powerpc/mm/pageattr.c | 105 ++++++++++++++++++++++++++
> 4 files changed, 117 insertions(+), 1 deletion(-)
> create mode 100644 arch/powerpc/include/asm/set_memory.h
> create mode 100644 arch/powerpc/mm/pageattr.c
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index cb2d44ee4e38..94c34932a74b 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -138,6 +138,7 @@ config PPC
> select ARCH_HAS_MEMBARRIER_CALLBACKS
> select ARCH_HAS_MEMBARRIER_SYNC_CORE
> select ARCH_HAS_SCALED_CPUTIME if VIRT_CPU_ACCOUNTING_NATIVE && PPC_BOOK3S_64
> + select ARCH_HAS_SET_MEMORY
> select ARCH_HAS_STRICT_KERNEL_RWX if ((PPC_BOOK3S_64 || PPC32) && !HIBERNATION)
> select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
> select ARCH_HAS_UACCESS_FLUSHCACHE
> diff --git a/arch/powerpc/include/asm/set_memory.h b/arch/powerpc/include/asm/set_memory.h
> new file mode 100644
> index 000000000000..d1cd69b1a43a
> --- /dev/null
> +++ b/arch/powerpc/include/asm/set_memory.h
> @@ -0,0 +1,10 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _ASM_POWERPC_SET_MEMORY_H
> +#define _ASM_POWERPC_SET_MEMORY_H
> +
> +int set_memory_ro(unsigned long addr, int numpages);
> +int set_memory_rw(unsigned long addr, int numpages);
> +int set_memory_nx(unsigned long addr, int numpages);
> +int set_memory_x(unsigned long addr, int numpages);
> +
> +#endif
> diff --git a/arch/powerpc/mm/Makefile b/arch/powerpc/mm/Makefile
> index c3df3a8501d4..9142cf1fb0d5 100644
> --- a/arch/powerpc/mm/Makefile
> +++ b/arch/powerpc/mm/Makefile
> @@ -5,7 +5,7 @@
>
> ccflags-$(CONFIG_PPC64) := $(NO_MINIMAL_TOC)
>
> -obj-y := fault.o mem.o pgtable.o mmap.o maccess.o \
> +obj-y := fault.o mem.o pgtable.o mmap.o maccess.o pageattr.o \
> init_$(BITS).o pgtable_$(BITS).o \
> pgtable-frag.o ioremap.o ioremap_$(BITS).o \
> init-common.o mmu_context.o drmem.o \
> diff --git a/arch/powerpc/mm/pageattr.c b/arch/powerpc/mm/pageattr.c
> new file mode 100644
> index 000000000000..3b4aa72e555e
> --- /dev/null
> +++ b/arch/powerpc/mm/pageattr.c
> @@ -0,0 +1,105 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * MMU-generic set_memory implementation for powerpc
> + *
> + * Copyright 2019-2021, IBM Corporation.
> + */
> +
> +#include <linux/mm.h>
> +#include <linux/set_memory.h>
> +
> +#include <asm/mmu.h>
> +#include <asm/page.h>
> +#include <asm/pgtable.h>
> +
> +
> +/*
> + * Updates the attributes of a page in three steps:
> + *
> + * 1. invalidate the page table entry
> + * 2. flush the TLB
> + * 3. install the new entry with the updated attributes
> + *
> + * Invalidating the pte means there are situations where this will not work
> + * when in theory it should.
> + * For example:
> + * - removing write from page whilst it is being executed
> + * - setting a page read-only whilst it is being read by another CPU
> + *
> + */
> +static int change_page_attr(pte_t *ptep, unsigned long addr, void *data)
> +{
> + pte_t (*fn)(pte_t) = data;
> + pte_t pte;
> +
> + spin_lock(&init_mm.page_table_lock);
> +
> + /* invalidate the PTE so it's safe to modify */
> + pte = ptep_get_and_clear(&init_mm, addr, ptep);
> + flush_tlb_kernel_range(addr, addr + PAGE_SIZE);
> +
> + /* modify the PTE bits as desired, then apply */
> + pte = fn(pte);
> +
> + set_pte_at(&init_mm, addr, ptep, pte);
> +
> + /* See ptesync comment in radix__set_pte_at() */
> + if (radix_enabled())
> + asm volatile("ptesync": : :"memory");
> + spin_unlock(&init_mm.page_table_lock);
> +
> + return 0;
> +}
> +
> +static int change_memory_attr(unsigned long addr, int numpages, pte_t (*fn)(pte_t))
> +{
> + unsigned long start = ALIGN_DOWN(addr, PAGE_SIZE);
> + unsigned long size = numpages * PAGE_SIZE;
> +
> + if (!numpages)
> + return 0;
> +
> +#ifdef CONFIG_PPC_BOOK3S_64
> + /*
> + * On hash, the linear mapping is not in the Linux page table so
> + * apply_to_existing_page_range() will have no effect. If in the future
> + * the set_memory_* functions are used on the linear map this will need
> + * to be updated.
> + */
> + if (!radix_enabled()) {
> + int region = get_region_id(addr);
> +
> + if (WARN_ON_ONCE(region != VMALLOC_REGION_ID && region != IO_REGION_ID))
> + return -EINVAL;
> + }
> +#endif
> +
> + return apply_to_existing_page_range(&init_mm, start, size,
> + change_page_attr, fn);
> +}
> +
> +int set_memory_ro(unsigned long addr, int numpages)
> +{
> + return change_memory_attr(addr, numpages, pte_wrprotect);
> +}
> +
> +static pte_t pte_mkdirtywrite(pte_t pte)
> +{
> + return pte_mkwrite(pte_mkdirty(pte));
> +}
> +
> +int set_memory_rw(unsigned long addr, int numpages)
> +{
> + return change_memory_attr(addr, numpages, pte_mkdirtywrite);
> +}
> +
> +int set_memory_nx(unsigned long addr, int numpages)
> +{
> + return change_memory_attr(addr, numpages, pte_exprotect);
> +}
> +
> +int set_memory_x(unsigned long addr, int numpages)
> +{
> + return change_memory_attr(addr, numpages, pte_mkexec);
> +}
>
^ permalink raw reply
* [PATCH] powerpc: mark local variables around longjmp as volatile
From: Arnd Bergmann @ 2021-04-29 8:06 UTC (permalink / raw)
To: Michael Ellerman
Cc: Ravi Bangoria, Arnd Bergmann, linux-kernel, Paul Mackerras,
Nicholas Piggin, linuxppc-dev
From: Arnd Bergmann <arnd@arndb.de>
gcc-11 points out that modifying local variables next to a
longjmp/setjmp may cause undefined behavior:
arch/powerpc/kexec/crash.c: In function 'crash_kexec_prepare_cpus.constprop':
arch/powerpc/kexec/crash.c:108:22: error: variable 'ncpus' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbere
d]
arch/powerpc/kexec/crash.c:109:13: error: variable 'tries' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbere
d]
arch/powerpc/xmon/xmon.c: In function 'xmon_print_symbol':
arch/powerpc/xmon/xmon.c:3625:21: error: variable 'name' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
arch/powerpc/xmon/xmon.c: In function 'stop_spus':
arch/powerpc/xmon/xmon.c:4057:13: error: variable 'i' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
arch/powerpc/xmon/xmon.c: In function 'restart_spus':
arch/powerpc/xmon/xmon.c:4098:13: error: variable 'i' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
arch/powerpc/xmon/xmon.c: In function 'dump_opal_msglog':
arch/powerpc/xmon/xmon.c:3008:16: error: variable 'pos' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
arch/powerpc/xmon/xmon.c: In function 'show_pte':
arch/powerpc/xmon/xmon.c:3207:29: error: variable 'tsk' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
arch/powerpc/xmon/xmon.c: In function 'show_tasks':
arch/powerpc/xmon/xmon.c:3302:29: error: variable 'tsk' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
arch/powerpc/xmon/xmon.c: In function 'xmon_core':
arch/powerpc/xmon/xmon.c:494:13: error: variable 'cmd' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
arch/powerpc/xmon/xmon.c:860:21: error: variable 'bp' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
arch/powerpc/xmon/xmon.c:860:21: error: variable 'bp' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
arch/powerpc/xmon/xmon.c:492:48: error: argument 'fromipi' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
According to the documentation, marking these as 'volatile' is
sufficient to avoid the problem, and it shuts up the warning.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
arch/powerpc/kexec/crash.c | 4 ++--
arch/powerpc/xmon/xmon.c | 22 +++++++++++-----------
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/arch/powerpc/kexec/crash.c b/arch/powerpc/kexec/crash.c
index 0196d0c211ac..10f997e6bb95 100644
--- a/arch/powerpc/kexec/crash.c
+++ b/arch/powerpc/kexec/crash.c
@@ -105,8 +105,8 @@ void crash_ipi_callback(struct pt_regs *regs)
static void crash_kexec_prepare_cpus(int cpu)
{
unsigned int msecs;
- unsigned int ncpus = num_online_cpus() - 1;/* Excluding the panic cpu */
- int tries = 0;
+ volatile unsigned int ncpus = num_online_cpus() - 1;/* Excluding the panic cpu */
+ volatile int tries = 0;
int (*old_handler)(struct pt_regs *regs);
printk(KERN_EMERG "Sending IPI to other CPUs\n");
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index c8173e92f19d..ce0eacf77645 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -489,10 +489,10 @@ static void xmon_touch_watchdogs(void)
touch_nmi_watchdog();
}
-static int xmon_core(struct pt_regs *regs, int fromipi)
+static int xmon_core(struct pt_regs *regs, volatile int fromipi)
{
- int cmd = 0;
- struct bpt *bp;
+ volatile int cmd = 0;
+ struct bpt *volatile bp;
long recurse_jmp[JMP_BUF_LEN];
bool locked_down;
unsigned long offset;
@@ -857,7 +857,7 @@ static inline void force_enable_xmon(void)
static struct bpt *at_breakpoint(unsigned long pc)
{
int i;
- struct bpt *bp;
+ struct bpt *volatile bp;
bp = bpts;
for (i = 0; i < NBPTS; ++i, ++bp)
@@ -3005,7 +3005,7 @@ static void dump_opal_msglog(void)
{
unsigned char buf[128];
ssize_t res;
- loff_t pos = 0;
+ volatile loff_t pos = 0;
if (!firmware_has_feature(FW_FEATURE_OPAL)) {
printf("Machine is not running OPAL firmware.\n");
@@ -3160,7 +3160,7 @@ memzcan(void)
printf("%.8lx\n", a - mskip);
}
-static void show_task(struct task_struct *tsk)
+static void show_task(struct task_struct *volatile tsk)
{
char state;
@@ -3204,7 +3204,7 @@ static void format_pte(void *ptep, unsigned long pte)
static void show_pte(unsigned long addr)
{
unsigned long tskv = 0;
- struct task_struct *tsk = NULL;
+ struct task_struct *volatile tsk = NULL;
struct mm_struct *mm;
pgd_t *pgdp;
p4d_t *p4dp;
@@ -3299,7 +3299,7 @@ static void show_pte(unsigned long addr)
static void show_tasks(void)
{
unsigned long tskv;
- struct task_struct *tsk = NULL;
+ struct task_struct *volatile tsk = NULL;
printf(" task_struct ->thread.ksp ->thread.regs PID PPID S P CMD\n");
@@ -3622,7 +3622,7 @@ static void xmon_print_symbol(unsigned long address, const char *mid,
const char *after)
{
char *modname;
- const char *name = NULL;
+ const char *volatile name = NULL;
unsigned long offset, size;
printf(REG, address);
@@ -4054,7 +4054,7 @@ void xmon_register_spus(struct list_head *list)
static void stop_spus(void)
{
struct spu *spu;
- int i;
+ volatile int i;
u64 tmp;
for (i = 0; i < XMON_NUM_SPUS; i++) {
@@ -4095,7 +4095,7 @@ static void stop_spus(void)
static void restart_spus(void)
{
struct spu *spu;
- int i;
+ volatile int i;
for (i = 0; i < XMON_NUM_SPUS; i++) {
if (!spu_info[i].spu)
--
2.29.2
^ permalink raw reply related
* Re: [PATCH v2] cpuidle/pseries: Fixup CEDE0 latency only for POWER10 onwards
From: Michael Ellerman @ 2021-04-29 9:56 UTC (permalink / raw)
To: Gautham R. Shenoy, Rafael J. Wysocki, Daniel Lezcano,
Aneesh Kumar K.V, Vaidyanathan Srinivasan
Cc: Gautham R. Shenoy, linux-pm, joedecke, Michal Suchanek,
linuxppc-dev
In-Reply-To: <1619673517-10853-1-git-send-email-ego@linux.vnet.ibm.com>
"Gautham R. Shenoy" <ego@linux.vnet.ibm.com> writes:
> From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
>
> Commit d947fb4c965c ("cpuidle: pseries: Fixup exit latency for
> CEDE(0)") sets the exit latency of CEDE(0) based on the latency values
> of the Extended CEDE states advertised by the platform
>
> On POWER9 LPARs, the firmwares advertise a very low value of 2us for
> CEDE1 exit latency on a Dedicated LPAR. The latency advertized by the
> PHYP hypervisor corresponds to the latency required to wakeup from the
> underlying hardware idle state. However the wakeup latency from the
> LPAR perspective should include
>
> 1. The time taken to transition the CPU from the Hypervisor into the
> LPAR post wakeup from platform idle state
>
> 2. Time taken to send the IPI from the source CPU (waker) to the idle
> target CPU (wakee).
>
> 1. can be measured via timer idle test, where we queue a timer, say
> for 1ms, and enter the CEDE state. When the timer fires, in the timer
> handler we compute how much extra timer over the expected 1ms have we
> consumed. On a a POWER9 LPAR the numbers are
>
> CEDE latency measured using a timer (numbers in ns)
> N Min Median Avg 90%ile 99%ile Max Stddev
> 400 2601 5677 5668.74 5917 6413 9299 455.01
>
> 1. and 2. combined can be determined by an IPI latency test where we
> send an IPI to an idle CPU and in the handler compute the time
> difference between when the IPI was sent and when the handler ran. We
> see the following numbers on POWER9 LPAR.
>
> CEDE latency measured using an IPI (numbers in ns)
> N Min Median Avg 90%ile 99%ile Max Stddev
> 400 711 7564 7369.43 8559 9514 9698 1200.01
>
> Suppose, we consider the 99th percentile latency value measured using
> the IPI to be the wakeup latency, the value would be 9.5us This is in
> the ballpark of the default value of 10us.
>
> Hence, use the exit latency of CEDE(0) based on the latency values
> advertized by platform only from POWER10 onwards. The values
^^^^^^^
> advertized on POWER10 platforms is more realistic and informed by the
> latency measurements. For earlier platforms stick to the default value
> of 10us.
...
> diff --git a/drivers/cpuidle/cpuidle-pseries.c b/drivers/cpuidle/cpuidle-pseries.c
> index a2b5c6f..7207467 100644
> --- a/drivers/cpuidle/cpuidle-pseries.c
> +++ b/drivers/cpuidle/cpuidle-pseries.c
> @@ -419,7 +419,8 @@ static int pseries_idle_probe(void)
> cpuidle_state_table = shared_states;
> max_idle_state = ARRAY_SIZE(shared_states);
> } else {
> - fixup_cede0_latency();
> + if (pvr_version_is(PVR_POWER10))
> + fixup_cede0_latency();
A PVR check like that tests for *only* Power10, not Power10 and onwards
as you say in the change log.
The other question is what should happen on a Power10 LPAR that's
running in Power9 compat mode. I assume in that case we *do* want to use
the firmware provided values, because they're tied to the underlying
CPU, not the compat mode?
In which case a check for !PVR_POWER9 would seem to achieve what we
want?
cheers
^ permalink raw reply
* Re: [PATCH v3] pseries/drmem: update LMBs after LPM
From: Aneesh Kumar K.V @ 2021-04-29 10:27 UTC (permalink / raw)
To: Laurent Dufour, mpe, benh, paulus
Cc: nathanl, Tyrel Datwyler, linuxppc-dev, linux-kernel
In-Reply-To: <20210428094758.28665-1-ldufour@linux.ibm.com>
Laurent Dufour <ldufour@linux.ibm.com> writes:
> After a LPM, the device tree node ibm,dynamic-reconfiguration-memory may be
> updated by the hypervisor in the case the NUMA topology of the LPAR's
> memory is updated.
>
> This is caught by the kernel, but the memory's node is updated because
> there is no way to move a memory block between nodes.
>
> If later a memory block is added or removed, drmem_update_dt() is called
> and it is overwriting the DT node to match the added or removed LMB. But
> the LMB's associativity node has not been updated after the DT node update
> and thus the node is overwritten by the Linux's topology instead of the
> hypervisor one.
>
> Introduce a hook called when the ibm,dynamic-reconfiguration-memory node is
> updated to force an update of the LMB's associativity.
>
> Cc: Tyrel Datwyler <tyreld@linux.ibm.com>
> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
> ---
>
> V3:
> - Check rd->dn->name instead of rd->dn->full_name
> V2:
> - Take Tyrel's idea to rely on OF_RECONFIG_UPDATE_PROPERTY instead of
> introducing a new hook mechanism.
> ---
> arch/powerpc/include/asm/drmem.h | 1 +
> arch/powerpc/mm/drmem.c | 35 +++++++++++++++++++
> .../platforms/pseries/hotplug-memory.c | 4 +++
> 3 files changed, 40 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/drmem.h b/arch/powerpc/include/asm/drmem.h
> index bf2402fed3e0..4265d5e95c2c 100644
> --- a/arch/powerpc/include/asm/drmem.h
> +++ b/arch/powerpc/include/asm/drmem.h
> @@ -111,6 +111,7 @@ int drmem_update_dt(void);
> int __init
> walk_drmem_lmbs_early(unsigned long node, void *data,
> int (*func)(struct drmem_lmb *, const __be32 **, void *));
> +void drmem_update_lmbs(struct property *prop);
> #endif
>
> static inline void invalidate_lmb_associativity_index(struct drmem_lmb *lmb)
> diff --git a/arch/powerpc/mm/drmem.c b/arch/powerpc/mm/drmem.c
> index 9af3832c9d8d..f0a6633132af 100644
> --- a/arch/powerpc/mm/drmem.c
> +++ b/arch/powerpc/mm/drmem.c
> @@ -307,6 +307,41 @@ int __init walk_drmem_lmbs_early(unsigned long node, void *data,
> return ret;
> }
>
> +/*
> + * Update the LMB associativity index.
> + */
> +static int update_lmb(struct drmem_lmb *updated_lmb,
> + __maybe_unused const __be32 **usm,
> + __maybe_unused void *data)
> +{
> + struct drmem_lmb *lmb;
> +
> + /*
> + * Brut force there may be better way to fetch the LMB
> + */
> + for_each_drmem_lmb(lmb) {
> + if (lmb->drc_index != updated_lmb->drc_index)
> + continue;
> +
> + lmb->aa_index = updated_lmb->aa_index;
> + break;
> + }
> + return 0;
> +}
> +
> +/*
> + * Update the LMB associativity index.
> + *
> + * This needs to be called when the hypervisor is updating the
> + * dynamic-reconfiguration-memory node property.
> + */
> +void drmem_update_lmbs(struct property *prop)
> +{
> + if (!strcmp(prop->name, "ibm,dynamic-memory"))
> + __walk_drmem_v1_lmbs(prop->value, NULL, NULL, update_lmb);
> + else if (!strcmp(prop->name, "ibm,dynamic-memory-v2"))
> + __walk_drmem_v2_lmbs(prop->value, NULL, NULL, update_lmb);
> +}
> #endif
>
> static int init_drmem_lmb_size(struct device_node *dn)
> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
> index 8377f1f7c78e..672ffbee2e78 100644
> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
> @@ -949,6 +949,10 @@ static int pseries_memory_notifier(struct notifier_block *nb,
> case OF_RECONFIG_DETACH_NODE:
> err = pseries_remove_mem_node(rd->dn);
> break;
> + case OF_RECONFIG_UPDATE_PROPERTY:
> + if (!strcmp(rd->dn->name,
> + "ibm,dynamic-reconfiguration-memory"))
> + drmem_update_lmbs(rd->prop);
> }
> return notifier_from_errno(err);
How will this interact with DLPAR memory? When we dlpar memory,
ibm,configure-connector is used to fetch the new associativity details
and set drmem_lmb->aa_index correctly there. Once that is done kernel
then call drmem_update_dt() which will result in the above notifier
callback?
IIUC, the call back then will update drmem_lmb->aa_index again?
-aneesh
^ permalink raw reply
* Re: [PATCH kernel] powerpc/makefile: Do not redefine $(CPP) for preprocessor
From: Michael Ellerman @ 2021-04-29 10:47 UTC (permalink / raw)
To: Daniel Axtens, Alexey Kardashevskiy, linuxppc-dev; +Cc: Alexey Kardashevskiy
In-Reply-To: <87mttqylho.fsf@dja-thinkpad.axtens.net>
Daniel Axtens <dja@axtens.net> writes:
> Hi Alexey,
>
>> The $(CPP) (do only preprocessing) macro is already defined in Makefile.
>> However POWERPC redefines it and adds $(KBUILD_CFLAGS) which results
>> in flags duplication. Which is not a big deal by itself except for
>> the flags which depend on other flags and the compiler checks them
>> as it parses the command line.
>>
>> Specifically, scripts/Makefile.build:304 generates ksyms for .S files.
>> If clang+llvm+sanitizer are enabled, this results in
>> -fno-lto -flto -fsanitize=cfi-mfcall .... -fno-lto -flto -fsanitize=cfi-mfcall
>
> Checkpatch doesn't like this line:
> WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
> #14:
> -fno-lto -flto -fsanitize=cfi-mfcall .... -fno-lto -flto -fsanitize=cfi-mfcall
> However, it doesn't make sense to wrap the line so we should just ignore
> checkpatch here.
>
>> in the clang command line and triggers error:
>>
>> clang-13: error: invalid argument '-fsanitize=cfi-mfcall' only allowed with '-flto'
>>
>> This removes unnecessary CPP redifinition.
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>> arch/powerpc/Makefile | 1 -
>> 1 file changed, 1 deletion(-)
>>
>> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
>> index c9d2c7825cd6..3a2f2001c62b 100644
>> --- a/arch/powerpc/Makefile
>> +++ b/arch/powerpc/Makefile
>> @@ -214,7 +214,6 @@ KBUILD_CPPFLAGS += -I $(srctree)/arch/$(ARCH) $(asinstr)
>> KBUILD_AFLAGS += $(AFLAGS-y)
>> KBUILD_CFLAGS += $(call cc-option,-msoft-float)
>> KBUILD_CFLAGS += -pipe $(CFLAGS-y)
>> -CPP = $(CC) -E $(KBUILD_CFLAGS)
>
> I was trying to check the history to see why powerpc has its own
> definition. It seems to date back at least as far as merging the two
> powerpc platforms into one, maybe it was helpful then. I agree it
> doesn't seem to be needed now.
>
> Snowpatch claims that this breaks the build, but I haven't been able to
> reproduce the issue in either pmac32 or ppc64 defconfig. I have sent it
> off to a fork of mpe's linux-ci repo to see if any of those builds hit
> any issues: https://github.com/daxtens/linux-ci/actions
It does break the build.
cheers
^ permalink raw reply
* Re: [PATCH v2] cpuidle/pseries: Fixup CEDE0 latency only for POWER10 onwards
From: Gautham R Shenoy @ 2021-04-29 11:10 UTC (permalink / raw)
To: Michael Ellerman
Cc: Gautham R. Shenoy, Michal Suchanek, linux-pm, Aneesh Kumar K.V,
Daniel Lezcano, Rafael J. Wysocki, joedecke,
Vaidyanathan Srinivasan, linuxppc-dev
In-Reply-To: <87r1it9zxy.fsf@mpe.ellerman.id.au>
Hello Michael,
On Thu, Apr 29, 2021 at 07:56:25PM +1000, Michael Ellerman wrote:
> "Gautham R. Shenoy" <ego@linux.vnet.ibm.com> writes:
> > From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
> >
> > Commit d947fb4c965c ("cpuidle: pseries: Fixup exit latency for
> > CEDE(0)") sets the exit latency of CEDE(0) based on the latency values
> > of the Extended CEDE states advertised by the platform
> >
> > On POWER9 LPARs, the firmwares advertise a very low value of 2us for
> > CEDE1 exit latency on a Dedicated LPAR. The latency advertized by the
> > PHYP hypervisor corresponds to the latency required to wakeup from the
> > underlying hardware idle state. However the wakeup latency from the
> > LPAR perspective should include
> >
> > 1. The time taken to transition the CPU from the Hypervisor into the
> > LPAR post wakeup from platform idle state
> >
> > 2. Time taken to send the IPI from the source CPU (waker) to the idle
> > target CPU (wakee).
> >
> > 1. can be measured via timer idle test, where we queue a timer, say
> > for 1ms, and enter the CEDE state. When the timer fires, in the timer
> > handler we compute how much extra timer over the expected 1ms have we
> > consumed. On a a POWER9 LPAR the numbers are
> >
> > CEDE latency measured using a timer (numbers in ns)
> > N Min Median Avg 90%ile 99%ile Max Stddev
> > 400 2601 5677 5668.74 5917 6413 9299 455.01
> >
> > 1. and 2. combined can be determined by an IPI latency test where we
> > send an IPI to an idle CPU and in the handler compute the time
> > difference between when the IPI was sent and when the handler ran. We
> > see the following numbers on POWER9 LPAR.
> >
> > CEDE latency measured using an IPI (numbers in ns)
> > N Min Median Avg 90%ile 99%ile Max Stddev
> > 400 711 7564 7369.43 8559 9514 9698 1200.01
> >
> > Suppose, we consider the 99th percentile latency value measured using
> > the IPI to be the wakeup latency, the value would be 9.5us This is in
> > the ballpark of the default value of 10us.
> >
> > Hence, use the exit latency of CEDE(0) based on the latency values
> > advertized by platform only from POWER10 onwards. The values
> ^^^^^^^
> > advertized on POWER10 platforms is more realistic and informed by the
> > latency measurements. For earlier platforms stick to the default value
> > of 10us.
>
> ...
>
> > diff --git a/drivers/cpuidle/cpuidle-pseries.c b/drivers/cpuidle/cpuidle-pseries.c
> > index a2b5c6f..7207467 100644
> > --- a/drivers/cpuidle/cpuidle-pseries.c
> > +++ b/drivers/cpuidle/cpuidle-pseries.c
> > @@ -419,7 +419,8 @@ static int pseries_idle_probe(void)
> > cpuidle_state_table = shared_states;
> > max_idle_state = ARRAY_SIZE(shared_states);
> > } else {
> > - fixup_cede0_latency();
> > + if (pvr_version_is(PVR_POWER10))
> > + fixup_cede0_latency();
>
> A PVR check like that tests for *only* Power10, not Power10 and onwards
> as you say in the change log.
Right. The accurate thing would be to check not do the fix up for
!(PVR_POWER4 || PVR_POWER4p || POWER_POWER5 || PVR_POWER5p || PVR_POWER6 || PVR_POWER7
|| PVR_POWER8 || PVR_POWER9)
But that was a bit mouthful. I will go with your suggestion (from
private correspondence)
if (cpu_has_feature(CPU_FTR_ARCH_31) || pvr_version_is(PVR_POWER10))
fixup_cede0_latency();
since it will allow the fixup for Processors suporting ISA 3.1
(POWER10 and above) and also on POWER10 CPUs running in compat mode.
>
> The other question is what should happen on a Power10 LPAR that's
> running in Power9 compat mode. I assume in that case we *do* want to use
> the firmware provided values, because they're tied to the underlying
> CPU, not the compat mode?
>
Yes, the firmware provided values are tied to the underlying CPU. Not
the compat mode.
> In which case a check for !PVR_POWER9 would seem to achieve what we
> want?
>
> cheers
--
Thanks and Regards
gautham.
^ 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