* [PATCH 17/20] selftest/powerpc/pmu/: Add interface test for mmcr2_l2l3 field
From: Kajol Jain @ 2022-01-27 7:20 UTC (permalink / raw)
To: mpe; +Cc: atrajeev, rnsastry, kjain, maddy, Madhavan Srinivasan,
linuxppc-dev
In-Reply-To: <20220127072012.662451-1-kjain@linux.ibm.com>
From: Madhavan Srinivasan <maddy@linux.ibm.com>
The testcases uses event code 0x010000046080 to verify
the l2l3 bit setting for Monitor Mode Control Register 2
(MMCR2). check if this bit is set correctly via perf interface
in ISA v3.1 platform.
Signed-off-by: Madhavan Srinivasan <maddy@linux.ibm.com>
---
.../powerpc/pmu/sampling_tests/Makefile | 4 +-
.../pmu/sampling_tests/mmcr2_l2l3_test.c | 74 +++++++++++++++++++
2 files changed, 76 insertions(+), 2 deletions(-)
create mode 100644 tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr2_l2l3_test.c
diff --git a/tools/testing/selftests/powerpc/pmu/sampling_tests/Makefile b/tools/testing/selftests/powerpc/pmu/sampling_tests/Makefile
index 345ad66bd1b2..f01666d7f2e0 100644
--- a/tools/testing/selftests/powerpc/pmu/sampling_tests/Makefile
+++ b/tools/testing/selftests/powerpc/pmu/sampling_tests/Makefile
@@ -3,7 +3,7 @@ include ../../../../../../scripts/Kbuild.include
all: $(TEST_GEN_PROGS) mmcr0_exceptionbits_test.c mmcr0_cc56run_test.c mmcr0_pmccext_test.c \
mmcr0_pmcjce_test.c mmcr0_fc56_pmc1ce_test.c mmcr0_fc56_pmc56_test.c \
- mmcr1_comb_test.c mmcr1_sel_unit_cache_test.c
+ mmcr1_comb_test.c mmcr1_sel_unit_cache_test.c mmcr2_l2l3_test.c
noarg:
$(MAKE) -C ../../
@@ -16,7 +16,7 @@ no-pie-option := $(call try-run, echo 'int main() { return 0; }' | \
TEST_GEN_PROGS := mmcr0_exceptionbits_test mmcr0_cc56run_test mmcr0_pmccext_test \
mmcr0_pmcjce_test mmcr0_fc56_pmc1ce_test mmcr0_fc56_pmc56_test \
- mmcr1_comb_test mmcr1_sel_unit_cache_test
+ mmcr1_comb_test mmcr1_sel_unit_cache_test mmcr2_l2l3_test
LDFLAGS += $(no-pie-option)
diff --git a/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr2_l2l3_test.c b/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr2_l2l3_test.c
new file mode 100644
index 000000000000..7167e2217ba8
--- /dev/null
+++ b/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr2_l2l3_test.c
@@ -0,0 +1,74 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2022, Madhavan Srinivasan, IBM Corp.
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+
+#include "../event.h"
+#include "misc.h"
+#include "utils.h"
+
+/* All successful D-side store dispatches for this thread */
+#define EventCode 0x010000046080
+
+#define MALLOC_SIZE (0x10000 * 10) /* Ought to be enough .. */
+
+/*
+ * A perf sampling test for mmcr2
+ * fields : l2l3
+ */
+static int mmcr2_l2l3(void)
+{
+ struct event event;
+ u64 *intr_regs;
+ char *p;
+ int i;
+
+ /* Check for platform support for the test */
+ SKIP_IF(check_pvr_for_sampling_tests());
+ SKIP_IF(!have_hwcap2(PPC_FEATURE2_ARCH_3_1));
+
+ /* Init the event for the sampling test */
+ event_init_sampling(&event, EventCode);
+ event.attr.sample_regs_intr = platform_extended_mask;
+ FAIL_IF(event_open(&event));
+ event.mmap_buffer = event_sample_buf_mmap(event.fd, 1);
+
+ event_enable(&event);
+
+ /* workload to make the event overflow */
+ p = malloc(MALLOC_SIZE);
+ FAIL_IF(!p);
+
+ for (i = 0; i < MALLOC_SIZE; i += 0x10000)
+ p[i] = i;
+
+ event_disable(&event);
+
+ /* Check for sample count */
+ FAIL_IF(!collect_samples(event.mmap_buffer));
+
+ intr_regs = get_intr_regs(&event, event.mmap_buffer);
+
+ /* Check for intr_regs */
+ FAIL_IF(!intr_regs);
+
+ /*
+ * Verify that l2l3 field of MMCR2 match with
+ * corresponding event code field
+ */
+ FAIL_IF(EV_CODE_EXTRACT(event.attr.config, l2l3) !=
+ GET_MMCR_FIELD(2, get_reg_value(intr_regs, "MMCR2"), 4, l2l3));
+
+ event_close(&event);
+ free(p);
+
+ return 0;
+}
+
+int main(void)
+{
+ FAIL_IF(test_harness(mmcr2_l2l3, "mmcr2_l2l3"));
+}
--
2.27.0
^ permalink raw reply related
* [PATCH 18/20] selftest/powerpc/pmu/: Add interface test for mmcr2_fcs_fch fields
From: Kajol Jain @ 2022-01-27 7:20 UTC (permalink / raw)
To: mpe; +Cc: atrajeev, rnsastry, kjain, maddy, Madhavan Srinivasan,
linuxppc-dev
In-Reply-To: <20220127072012.662451-1-kjain@linux.ibm.com>
From: Madhavan Srinivasan <maddy@linux.ibm.com>
The testcases uses cycles event to verify the freeze counter
settings in Monitor Mode Control Register 2 (MMCR2). Event
modifier (exclude_kernel) setting is used for the event attribute
to check the FCxS and FCxH ( Freeze counter in privileged and
hypervisor state ) settings via perf interface.
Signed-off-by: Madhavan Srinivasan <maddy@linux.ibm.com>
---
.../powerpc/pmu/sampling_tests/Makefile | 6 +-
.../pmu/sampling_tests/mmcr2_fcs_fch_test.c | 67 +++++++++++++++++++
2 files changed, 71 insertions(+), 2 deletions(-)
create mode 100644 tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr2_fcs_fch_test.c
diff --git a/tools/testing/selftests/powerpc/pmu/sampling_tests/Makefile b/tools/testing/selftests/powerpc/pmu/sampling_tests/Makefile
index f01666d7f2e0..1deaab5a4ebf 100644
--- a/tools/testing/selftests/powerpc/pmu/sampling_tests/Makefile
+++ b/tools/testing/selftests/powerpc/pmu/sampling_tests/Makefile
@@ -3,7 +3,8 @@ include ../../../../../../scripts/Kbuild.include
all: $(TEST_GEN_PROGS) mmcr0_exceptionbits_test.c mmcr0_cc56run_test.c mmcr0_pmccext_test.c \
mmcr0_pmcjce_test.c mmcr0_fc56_pmc1ce_test.c mmcr0_fc56_pmc56_test.c \
- mmcr1_comb_test.c mmcr1_sel_unit_cache_test.c mmcr2_l2l3_test.c
+ mmcr1_comb_test.c mmcr1_sel_unit_cache_test.c mmcr2_l2l3_test.c \
+ mmcr2_fcs_fch_test.c
noarg:
$(MAKE) -C ../../
@@ -16,7 +17,8 @@ no-pie-option := $(call try-run, echo 'int main() { return 0; }' | \
TEST_GEN_PROGS := mmcr0_exceptionbits_test mmcr0_cc56run_test mmcr0_pmccext_test \
mmcr0_pmcjce_test mmcr0_fc56_pmc1ce_test mmcr0_fc56_pmc56_test \
- mmcr1_comb_test mmcr1_sel_unit_cache_test mmcr2_l2l3_test
+ mmcr1_comb_test mmcr1_sel_unit_cache_test mmcr2_l2l3_test \
+ mmcr2_fcs_fch_test
LDFLAGS += $(no-pie-option)
diff --git a/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr2_fcs_fch_test.c b/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr2_fcs_fch_test.c
new file mode 100644
index 000000000000..1c1c48ca7d4c
--- /dev/null
+++ b/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr2_fcs_fch_test.c
@@ -0,0 +1,67 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2022, Madhavan Srinivasan, IBM Corp.
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+
+#include "../event.h"
+#include "misc.h"
+#include "utils.h"
+
+extern void thirty_two_instruction_loop(int loops);
+
+/*
+ * A perf sampling test for mmcr2
+ * fields : fcs, fch.
+ */
+static int mmcr2_fcs_fch(void)
+{
+ struct event event;
+ u64 *intr_regs;
+
+ /* Check for platform support for the test */
+ SKIP_IF(check_pvr_for_sampling_tests());
+
+ /* Init the event for the sampling test */
+ event_init_sampling(&event, 0x1001e);
+ event.attr.sample_regs_intr = platform_extended_mask;
+ event.attr.exclude_kernel = 1;
+ FAIL_IF(event_open(&event));
+ event.mmap_buffer = event_sample_buf_mmap(event.fd, 1);
+
+ event_enable(&event);
+
+ /* workload to make the event overflow */
+ thirty_two_instruction_loop(10000);
+
+ event_disable(&event);
+
+ /* Check for sample count */
+ FAIL_IF(!collect_samples(event.mmap_buffer));
+
+ intr_regs = get_intr_regs(&event, event.mmap_buffer);
+
+ /* Check for intr_regs */
+ FAIL_IF(!intr_regs);
+
+ /*
+ * Verify that fcs and fch field of MMCR2 match
+ * with corresponding modifier fields.
+ */
+ if (is_pSeries())
+ FAIL_IF(GET_ATTR_FIELD(&event, exclude_kernel) !=
+ GET_MMCR_FIELD(2, get_reg_value(intr_regs, "MMCR2"), 1, fcs));
+ else
+ FAIL_IF(GET_ATTR_FIELD(&event, exclude_kernel) !=
+ GET_MMCR_FIELD(2, get_reg_value(intr_regs, "MMCR2"), 1, fch));
+
+ event_close(&event);
+ return 0;
+}
+
+int main(void)
+{
+ FAIL_IF(test_harness(mmcr2_fcs_fch, "mmcr2_fcs_fch"));
+}
--
2.27.0
^ permalink raw reply related
* [PATCH 19/20] selftest/powerpc/pmu/: Add interface test for mmcr3_src fields
From: Kajol Jain @ 2022-01-27 7:20 UTC (permalink / raw)
To: mpe; +Cc: kjain, atrajeev, maddy, linuxppc-dev, rnsastry
In-Reply-To: <20220127072012.662451-1-kjain@linux.ibm.com>
The testcase uses event code 0x1340000001c040 to verify
the settings for different src fields in Monitor Mode Control
Register 3 (MMCR3). Checks if these fields are translated
correctly via perf interface to MMCR3 on ISA v3.1 platform.
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
.../powerpc/pmu/sampling_tests/Makefile | 4 +-
.../pmu/sampling_tests/mmcr3_src_test.c | 67 +++++++++++++++++++
2 files changed, 69 insertions(+), 2 deletions(-)
create mode 100644 tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr3_src_test.c
diff --git a/tools/testing/selftests/powerpc/pmu/sampling_tests/Makefile b/tools/testing/selftests/powerpc/pmu/sampling_tests/Makefile
index 1deaab5a4ebf..58d3ddf779d2 100644
--- a/tools/testing/selftests/powerpc/pmu/sampling_tests/Makefile
+++ b/tools/testing/selftests/powerpc/pmu/sampling_tests/Makefile
@@ -4,7 +4,7 @@ include ../../../../../../scripts/Kbuild.include
all: $(TEST_GEN_PROGS) mmcr0_exceptionbits_test.c mmcr0_cc56run_test.c mmcr0_pmccext_test.c \
mmcr0_pmcjce_test.c mmcr0_fc56_pmc1ce_test.c mmcr0_fc56_pmc56_test.c \
mmcr1_comb_test.c mmcr1_sel_unit_cache_test.c mmcr2_l2l3_test.c \
- mmcr2_fcs_fch_test.c
+ mmcr2_fcs_fch_test.c mmcr3_src_test.c
noarg:
$(MAKE) -C ../../
@@ -18,7 +18,7 @@ no-pie-option := $(call try-run, echo 'int main() { return 0; }' | \
TEST_GEN_PROGS := mmcr0_exceptionbits_test mmcr0_cc56run_test mmcr0_pmccext_test \
mmcr0_pmcjce_test mmcr0_fc56_pmc1ce_test mmcr0_fc56_pmc56_test \
mmcr1_comb_test mmcr1_sel_unit_cache_test mmcr2_l2l3_test \
- mmcr2_fcs_fch_test
+ mmcr2_fcs_fch_test mmcr3_src_test
LDFLAGS += $(no-pie-option)
diff --git a/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr3_src_test.c b/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr3_src_test.c
new file mode 100644
index 000000000000..d8d6ee0bb696
--- /dev/null
+++ b/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr3_src_test.c
@@ -0,0 +1,67 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2022, Kajol Jain, IBM Corp.
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+
+#include "../event.h"
+#include "misc.h"
+#include "utils.h"
+
+extern void thirty_two_instruction_loop_with_ll_sc(u64 loops, u64 *ll_sc_target);
+
+/* The data cache was reloaded from local core's L3 due to a demand load */
+#define EventCode 0x1340000001c040
+
+/*
+ * A perf sampling test for mmcr3
+ * fields.
+ */
+static int mmcr3_src(void)
+{
+ struct event event;
+ u64 *intr_regs;
+ u64 dummy;
+
+ /* Check for platform support for the test */
+ SKIP_IF(check_pvr_for_sampling_tests());
+ SKIP_IF(!have_hwcap2(PPC_FEATURE2_ARCH_3_1));
+
+ /* Init the event for the sampling test */
+ event_init_sampling(&event, EventCode);
+ event.attr.sample_regs_intr = platform_extended_mask;
+ FAIL_IF(event_open(&event));
+ event.mmap_buffer = event_sample_buf_mmap(event.fd, 1);
+
+ event_enable(&event);
+
+ /* workload to make event overflow */
+ thirty_two_instruction_loop_with_ll_sc(1000000, &dummy);
+
+ event_disable(&event);
+
+ /* Check for sample count */
+ FAIL_IF(!collect_samples(event.mmap_buffer));
+
+ intr_regs = get_intr_regs(&event, event.mmap_buffer);
+
+ /* Check for intr_regs */
+ FAIL_IF(!intr_regs);
+
+ /*
+ * Verify that src field of MMCR3 match with
+ * corresponding event code field
+ */
+ FAIL_IF(EV_CODE_EXTRACT(event.attr.config, mmcr3_src) !=
+ GET_MMCR_FIELD(3, get_reg_value(intr_regs, "MMCR3"), 1, src));
+
+ event_close(&event);
+ return 0;
+}
+
+int main(void)
+{
+ return test_harness(mmcr3_src, "mmcr3_src");
+}
--
2.27.0
^ permalink raw reply related
* [PATCH 20/20] selftest/powerpc/pmu: Add interface test for mmcra register fields
From: Kajol Jain @ 2022-01-27 7:20 UTC (permalink / raw)
To: mpe; +Cc: kjain, atrajeev, maddy, linuxppc-dev, rnsastry
In-Reply-To: <20220127072012.662451-1-kjain@linux.ibm.com>
The testcase uses event code 0x35340401e0 to verify
the settings for different fields in Monitor Mode Control
Register A (MMCRA). The fields include thresh_start, thresh_stop
thresh_select, sdar mode, sample and marked bit. Checks if
these fields are translated correctly via perf interface to MMCRA.
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
.../powerpc/pmu/sampling_tests/Makefile | 4 +-
.../mmcra_thresh_marked_sample_test.c | 80 +++++++++++++++++++
2 files changed, 82 insertions(+), 2 deletions(-)
create mode 100644 tools/testing/selftests/powerpc/pmu/sampling_tests/mmcra_thresh_marked_sample_test.c
diff --git a/tools/testing/selftests/powerpc/pmu/sampling_tests/Makefile b/tools/testing/selftests/powerpc/pmu/sampling_tests/Makefile
index 58d3ddf779d2..a9bf343df911 100644
--- a/tools/testing/selftests/powerpc/pmu/sampling_tests/Makefile
+++ b/tools/testing/selftests/powerpc/pmu/sampling_tests/Makefile
@@ -4,7 +4,7 @@ include ../../../../../../scripts/Kbuild.include
all: $(TEST_GEN_PROGS) mmcr0_exceptionbits_test.c mmcr0_cc56run_test.c mmcr0_pmccext_test.c \
mmcr0_pmcjce_test.c mmcr0_fc56_pmc1ce_test.c mmcr0_fc56_pmc56_test.c \
mmcr1_comb_test.c mmcr1_sel_unit_cache_test.c mmcr2_l2l3_test.c \
- mmcr2_fcs_fch_test.c mmcr3_src_test.c
+ mmcr2_fcs_fch_test.c mmcr3_src_test.c mmcra_thresh_marked_sample_test.c
noarg:
$(MAKE) -C ../../
@@ -18,7 +18,7 @@ no-pie-option := $(call try-run, echo 'int main() { return 0; }' | \
TEST_GEN_PROGS := mmcr0_exceptionbits_test mmcr0_cc56run_test mmcr0_pmccext_test \
mmcr0_pmcjce_test mmcr0_fc56_pmc1ce_test mmcr0_fc56_pmc56_test \
mmcr1_comb_test mmcr1_sel_unit_cache_test mmcr2_l2l3_test \
- mmcr2_fcs_fch_test mmcr3_src_test
+ mmcr2_fcs_fch_test mmcr3_src_test mmcra_thresh_marked_sample_test
LDFLAGS += $(no-pie-option)
diff --git a/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcra_thresh_marked_sample_test.c b/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcra_thresh_marked_sample_test.c
new file mode 100644
index 000000000000..ca6bdb335fa5
--- /dev/null
+++ b/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcra_thresh_marked_sample_test.c
@@ -0,0 +1,80 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2022, Kajol Jain, IBM Corp.
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+
+#include "../event.h"
+#include "misc.h"
+#include "utils.h"
+
+/*
+ * Primary PMU event used here is PM_MRK_INST_CMPL (0x401e0)
+ * Threshold event selection used is issue to complete for cycles
+ * Sampling criteria is Load only sampling
+ */
+#define EventCode 0x35340401e0
+
+extern void thirty_two_instruction_loop_with_ll_sc(u64 loops, u64 *ll_sc_target);
+
+/* A perf sampling test to test mmcra fields */
+static int mmcra_thresh_marked_sample(void)
+{
+ struct event event;
+ u64 *intr_regs;
+ u64 dummy;
+
+ /* Check for platform support for the test */
+ SKIP_IF(check_pvr_for_sampling_tests());
+
+ /* Init the event for the sampling test */
+ event_init_sampling(&event, EventCode);
+ event.attr.sample_regs_intr = platform_extended_mask;
+ FAIL_IF(event_open(&event));
+ event.mmap_buffer = event_sample_buf_mmap(event.fd, 1);
+
+ event_enable(&event);
+
+ /* workload to make the event overflow */
+ thirty_two_instruction_loop_with_ll_sc(1000000, &dummy);
+
+ event_disable(&event);
+
+ /* Check for sample count */
+ FAIL_IF(!collect_samples(event.mmap_buffer));
+
+ intr_regs = get_intr_regs(&event, event.mmap_buffer);
+
+ /* Check for intr_regs */
+ FAIL_IF(!intr_regs);
+
+ /*
+ * Verify that thresh sel/start/stop, marked, random sample
+ * eligibility, sdar mode and sample mode fields match with
+ * the corresponding event code fields
+ */
+ FAIL_IF(EV_CODE_EXTRACT(event.attr.config, thd_sel) !=
+ GET_MMCR_FIELD(A, get_reg_value(intr_regs, "MMCRA"), 4, thd_sel));
+ FAIL_IF(EV_CODE_EXTRACT(event.attr.config, thd_start) !=
+ GET_MMCR_FIELD(A, get_reg_value(intr_regs, "MMCRA"), 4, thd_start));
+ FAIL_IF(EV_CODE_EXTRACT(event.attr.config, thd_stop) !=
+ GET_MMCR_FIELD(A, get_reg_value(intr_regs, "MMCRA"), 4, thd_stop));
+ FAIL_IF(EV_CODE_EXTRACT(event.attr.config, marked) !=
+ GET_MMCR_FIELD(A, get_reg_value(intr_regs, "MMCRA"), 4, marked));
+ FAIL_IF(EV_CODE_EXTRACT(event.attr.config, sample >> 2) !=
+ GET_MMCR_FIELD(A, get_reg_value(intr_regs, "MMCRA"), 4, rand_samp_elig));
+ FAIL_IF(EV_CODE_EXTRACT(event.attr.config, sample & 0x3) !=
+ GET_MMCR_FIELD(A, get_reg_value(intr_regs, "MMCRA"), 4, sample_mode));
+ FAIL_IF(EV_CODE_EXTRACT(event.attr.config, sm) !=
+ GET_MMCR_FIELD(A, get_reg_value(intr_regs, "MMCRA"), 4, sm));
+
+ event_close(&event);
+ return 0;
+}
+
+int main(void)
+{
+ FAIL_IF(test_harness(mmcra_thresh_marked_sample, "mmcra_thresh_marked_sample"));
+}
--
2.27.0
^ permalink raw reply related
* Re: [PATCH v5 5/5] KVM: PPC: Book3s: mmio: Deliver DSI after emulation failure
From: Nicholas Piggin @ 2022-01-27 7:34 UTC (permalink / raw)
To: Fabiano Rosas, kvm-ppc; +Cc: aik, linuxppc-dev
In-Reply-To: <20220125215655.1026224-6-farosas@linux.ibm.com>
Excerpts from Fabiano Rosas's message of January 26, 2022 7:56 am:
> MMIO emulation can fail if the guest uses an instruction that we are
> not prepared to emulate. Since these instructions can be and most
> likely are valid ones, this is (slightly) closer to an access fault
> than to an illegal instruction, so deliver a Data Storage interrupt
> instead of a Program interrupt.
>
> BookE ignores bad faults, so it will keep using a Program interrupt
> because a DSI would cause a fault loop in the guest.
>
> Suggested-by: Nicholas Piggin <npiggin@gmail.com>
> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
Thanks this looks good to me. (And thanks for updating patch 4/5 with
the kvm debug print helper.)
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/kvm/emulate_loadstore.c | 10 +++-------
> arch/powerpc/kvm/powerpc.c | 22 ++++++++++++++++++++++
> 2 files changed, 25 insertions(+), 7 deletions(-)
>
> diff --git a/arch/powerpc/kvm/emulate_loadstore.c b/arch/powerpc/kvm/emulate_loadstore.c
> index 48272a9b9c30..cfc9114b87d0 100644
> --- a/arch/powerpc/kvm/emulate_loadstore.c
> +++ b/arch/powerpc/kvm/emulate_loadstore.c
> @@ -73,7 +73,6 @@ int kvmppc_emulate_loadstore(struct kvm_vcpu *vcpu)
> {
> u32 inst;
> enum emulation_result emulated = EMULATE_FAIL;
> - int advance = 1;
> struct instruction_op op;
>
> /* this default type might be overwritten by subcategories */
> @@ -98,6 +97,8 @@ int kvmppc_emulate_loadstore(struct kvm_vcpu *vcpu)
> int type = op.type & INSTR_TYPE_MASK;
> int size = GETSIZE(op.type);
>
> + vcpu->mmio_is_write = OP_IS_STORE(type);
> +
> switch (type) {
> case LOAD: {
> int instr_byte_swap = op.type & BYTEREV;
> @@ -355,15 +356,10 @@ int kvmppc_emulate_loadstore(struct kvm_vcpu *vcpu)
> }
> }
>
> - if (emulated == EMULATE_FAIL) {
> - advance = 0;
> - kvmppc_core_queue_program(vcpu, 0);
> - }
> -
> trace_kvm_ppc_instr(inst, kvmppc_get_pc(vcpu), emulated);
>
> /* Advance past emulated instruction. */
> - if (advance)
> + if (emulated != EMULATE_FAIL)
> kvmppc_set_pc(vcpu, kvmppc_get_pc(vcpu) + 4);
>
> return emulated;
> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> index acb0d2a4bdb9..82d889db2b6b 100644
> --- a/arch/powerpc/kvm/powerpc.c
> +++ b/arch/powerpc/kvm/powerpc.c
> @@ -309,6 +309,28 @@ int kvmppc_emulate_mmio(struct kvm_vcpu *vcpu)
> kvmppc_get_last_inst(vcpu, INST_GENERIC, &last_inst);
> kvm_debug_ratelimited("Guest access to device memory using unsupported instruction (opcode: %#08x)\n",
> last_inst);
> +
> + /*
> + * Injecting a Data Storage here is a bit more
> + * accurate since the instruction that caused the
> + * access could still be a valid one.
> + */
> + if (!IS_ENABLED(CONFIG_BOOKE)) {
> + ulong dsisr = DSISR_BADACCESS;
> +
> + if (vcpu->mmio_is_write)
> + dsisr |= DSISR_ISSTORE;
> +
> + kvmppc_core_queue_data_storage(vcpu, vcpu->arch.vaddr_accessed, dsisr);
> + } else {
> + /*
> + * BookE does not send a SIGBUS on a bad
> + * fault, so use a Program interrupt instead
> + * to avoid a fault loop.
> + */
> + kvmppc_core_queue_program(vcpu, 0);
> + }
> +
> r = RESUME_GUEST;
> break;
> }
> --
> 2.34.1
>
>
^ permalink raw reply
* Re: [PATCH 0/2] powerpc: Disable syscall emulation and stepping
From: Nicholas Piggin @ 2022-01-27 7:39 UTC (permalink / raw)
To: Christophe Leroy, naverao1; +Cc: Naveen N. Rao, linuxppc-dev
In-Reply-To: <52b03748fdeff1bb2eb67f6038311e26@imap.linux.ibm.com>
Excerpts from naverao1's message of January 25, 2022 8:48 pm:
> On 2022-01-25 11:23, Christophe Leroy wrote:
>> Le 25/01/2022 à 04:04, Nicholas Piggin a écrit :
>>> +Naveen (sorry missed cc'ing you at first)
>>>
>>> Excerpts from Christophe Leroy's message of January 24, 2022 4:39 pm:
>>>>
>>>>
>>>> Le 24/01/2022 à 06:57, Nicholas Piggin a écrit :
>>>>> As discussed previously
>>>>>
>>>>> https://lists.ozlabs.org/pipermail/linuxppc-dev/2022-January/238946.html
>>>>>
>>>>> I'm wondering whether PPC32 should be returning -1 for syscall
>>>>> instructions too here? That could be done in another patch anyway.
>>>>>
>>>>
>>>> The 'Programming Environments Manual for 32-Bit Implementations of
>>>> the
>>>> PowerPC™ Architecture' says:
>>>>
>>>> The following are not traced:
>>>> • rfi instruction
>>>> • sc and trap instructions that trap
>>>> • Other instructions that cause interrupts (other than trace
>>>> interrupts)
>>>> • The first instruction of any interrupt handler
>>>> • Instructions that are emulated by software
>>>>
>>>>
>>>> So I think PPC32 should return -1 as well.
>>>
>>> I agree.
>>>
>>> What about the trap instructions? analyse_instr returns 0 for them
>>> which falls through to return 0 for emulate_step, should they
>>> return -1 as well or am I missing something?
>
> Yeah, good point about the trap instructions.
>
>>>
>>
>> For the traps I don't know. The manual says "trap instructions that
>> trap" are not traced. It means that "trap instructions that _don't_
>> trap" are traced. Taking into account that trap instructions don't trap
>> at least 99.9% of the time, not sure if returning -1 is needed.
>>
>> Allthought that'd probably be the safest.
>
> 'trap' is a special case since it is predominantly used by debuggers
> and/or tracing infrastructure. Kprobes and Uprobes do not allow probes
> on a trap instruction. But, xmon can be asked to step on a trap
> instruction and that can interfere with kprobes in weird ways.
>
> So, I think it is best if we also exclude trap instructions from being
> single stepped.
>
>>
>> But then what happens with other instruction that will sparsely
>> generate
>> an exception like a DSI or so ? If we do it for the traps then we
>> should
>> do it for this as well, and then it becomes a non ending story.
>
> For a DSI, we restart the same instruction after handling the page
> fault.
> The single step exception is raised on the subsequent successful
> completion of the instruction.
Although it can cause a signal, and the signal handler can decide
to resume somewhere else. Or kernel mode equivalent it can go to a
fixup handler and resume somewhere else.
How are those handled?
Thanks,
Nick
> For most other interrupts (alignment, vsx
> unavailable, ...), we end up emulating the single step exception itself
> (see emulate_single_step()). So, those are ok if caused by an
> instruction
> being stepped.
>
>
> - Naveen
>
^ permalink raw reply
* Re: [PATCH 2/2] powerpc/uprobes: Reject uprobe on a system call instruction
From: Nicholas Piggin @ 2022-01-27 7:44 UTC (permalink / raw)
To: linuxppc-dev, Michael Ellerman; +Cc: Naveen N . Rao
In-Reply-To: <874k5sm42l.fsf@mpe.ellerman.id.au>
Excerpts from Michael Ellerman's message of January 25, 2022 9:45 pm:
> Nicholas Piggin <npiggin@gmail.com> writes:
>> Per the ISA, a Trace interrupt is not generated for a system call
>> [vectored] instruction. Reject uprobes on such instructions as we are
>> not emulating a system call [vectored] instruction anymore.
>
> This should really be patch 1, otherwise there's a single commit window
> where we allow uprobes on sc but don't honour them.
Yep true. I also messed up Naveen's attribution! Will re-send (or maybe
Naveen would take over the series).
>
>> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
>> [np: Switch to pr_info_ratelimited]
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>> arch/powerpc/include/asm/ppc-opcode.h | 1 +
>> arch/powerpc/kernel/uprobes.c | 6 ++++++
>> 2 files changed, 7 insertions(+)
>>
>> diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
>> index 9675303b724e..8bbe16ce5173 100644
>> --- a/arch/powerpc/include/asm/ppc-opcode.h
>> +++ b/arch/powerpc/include/asm/ppc-opcode.h
>> @@ -411,6 +411,7 @@
>> #define PPC_RAW_DCBFPS(a, b) (0x7c0000ac | ___PPC_RA(a) | ___PPC_RB(b) | (4 << 21))
>> #define PPC_RAW_DCBSTPS(a, b) (0x7c0000ac | ___PPC_RA(a) | ___PPC_RB(b) | (6 << 21))
>> #define PPC_RAW_SC() (0x44000002)
>> +#define PPC_RAW_SCV() (0x44000001)
>> #define PPC_RAW_SYNC() (0x7c0004ac)
>> #define PPC_RAW_ISYNC() (0x4c00012c)
>>
>> diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c
>> index c6975467d9ff..3779fde804bd 100644
>> --- a/arch/powerpc/kernel/uprobes.c
>> +++ b/arch/powerpc/kernel/uprobes.c
>> @@ -41,6 +41,12 @@ int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe,
>> if (addr & 0x03)
>> return -EINVAL;
>>
>> + if (ppc_inst_val(ppc_inst_read(auprobe->insn)) == PPC_RAW_SC() ||
>> + ppc_inst_val(ppc_inst_read(auprobe->insn)) == PPC_RAW_SCV()) {
>
> We should probably reject hypercall too?
>
> There's also a lot of reserved fields in `sc`, so doing an exact match
> like this risks missing instructions that are badly formed but the CPU
> will happily execute as `sc`.
Yeah, scv as well has lev != 0 unsupported so should be excluded.
>
> We'd obviously never expect to see those in compiler generated code, but
> it'd still be safer to mask. We could probably just reject opcode 17
> entirely.
>
> And I guess for a subsequent patch, but we should be rejecting some
> others here as well shouldn't we? Like rfid etc.
Traps under discussion I guess. For uprobe, rfid will be just another
privilege fault. Is that dealt with somehow or do all privileged and
illegal instructions also need to be excluded from stepping? (I assume
we must handle that in a general way somehow)
Thanks,
Nick
^ permalink raw reply
* Re: ppc: hard lockup / hang in v5.17-rc1 under QEMU
From: Nicholas Piggin @ 2022-01-27 7:47 UTC (permalink / raw)
To: Cédric Le Goater, Miguel Ojeda; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <CANiq72mV3AzmBDVJM+tQriEoDu_9LFBrK_vR6GC4qEmLw0UepQ@mail.gmail.com>
Excerpts from Miguel Ojeda's message of January 27, 2022 4:57 am:
> On Wed, Jan 26, 2022 at 4:03 PM Cédric Le Goater <clg@kaod.org> wrote:
>>
>> Indeed. I could reproduce.
>
> Thanks for the quick confirmation!
>
>> Could you please send the QEMU command line and the full dmesg ? and
>> possibly open an issue on :
>>
>> https://gitlab.com/qemu-project/qemu/-/issues/
>>
>> I guess it's a QEMU modeling issue.
>
> Of course -- done (details there):
>
> https://gitlab.com/qemu-project/qemu/-/issues/842
That sounds like my fault actually.
https://lists.ozlabs.org/pipermail/linuxppc-dev/2022-January/239178.html
Thanks,
Nick
^ permalink raw reply
* Re: ppc: hard lockup / hang in v5.17-rc1 under QEMU
From: Michael Ellerman @ 2022-01-27 10:54 UTC (permalink / raw)
To: Miguel Ojeda, linuxppc-dev, linux-kernel
In-Reply-To: <CANiq72n_FmDx=r-o9J8gYc6LpwRL5EGmhM6Xzwv27Xc7h1TNDw@mail.gmail.com>
Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> writes:
> Hi PPC folks,
>
> Our ppc64le CI deterministically triggers a hard lockup / hang under
> QEMU since v5.17-rc1 (upgrading from v5.16).
>
> Bisecting points to 0faf20a1ad16 ("powerpc/64s/interrupt: Don't enable
> MSR[EE] in irq handlers unless perf is in use").
Hi Miguel,
Thanks for the report.
Nick has posted one fix for the commit you identified:
http://patchwork.ozlabs.org/project/linuxppc-dev/patch/20220124143930.3923442-1-npiggin@gmail.com/
It looks like your kernel-ppc64le-release.config does not have the
hardlockup detector enabled, so I suspect you're hitting the bug
described in that patch.
That fix will hit linux-next in the next day or so and should be in rc2.
cheers
^ permalink raw reply
* Re: WARN_ON() is buggy for 32 bit systems
From: Michael Ellerman @ 2022-01-27 11:10 UTC (permalink / raw)
To: Dan Carpenter, Christophe Leroy; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20220126134948.GA1978@kadam>
Dan Carpenter <dan.carpenter@oracle.com> writes:
> On Wed, Jan 26, 2022 at 12:21:49PM +0000, Christophe Leroy wrote:
>> The code is enclosed in a #ifdef CONFIG_PPC64, it is not used for PPC32:
>>
>> /arch/powerpc/include/asm/bug.h
>> 99 #ifdef CONFIG_PPC64
>
> Ah...
>
> You know, life would be a lot easier for me personally if we added an
> #ifndef __CHECKER__ as well... I can't compile PowerPC code so I can't
> test a patch like that.
Ubuntu & Fedora both have cross compilers packaged, or there's cross
compilers on kernel.org. But I assume you mean you'd rather not bother
compiling for powerpc, which is fair enough.
Do you mean something like below?
I'm not sure about that, as it would prevent sparse from checking the
actual BUG_ON code we're using, vs the generic version which we never
use on 64-bit. Is there a smatch specific macro we could check?
cheers
diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h
index 02c08d1492f8..5cbfe9d8232d 100644
--- a/arch/powerpc/include/asm/bug.h
+++ b/arch/powerpc/include/asm/bug.h
@@ -96,7 +96,7 @@ __label_warn_on: \
break; \
} while (0)
-#ifdef CONFIG_PPC64
+#if defined(CONFIG_PPC64) && !defined(__CHECKER__)
#define BUG_ON(x) do { \
if (__builtin_constant_p(x)) { \
if (x) \
^ permalink raw reply related
* Re: [PATCH v2 1/2] PCI/AER: Disable AER service when link is in L2/L3 ready, L2 and L3 state
From: Kai-Heng Feng @ 2022-01-27 11:14 UTC (permalink / raw)
To: Lu Baolu
Cc: Joerg Roedel, Lalithambika Krishnakumar, linuxppc-dev, linux-pci,
linux-kernel, koba.ko, Oliver O'Halloran, bhelgaas,
mika.westerberg
In-Reply-To: <0259955f-8bbb-1778-f234-398f1356db8b@linux.intel.com>
On Thu, Jan 27, 2022 at 3:01 PM Lu Baolu <baolu.lu@linux.intel.com> wrote:
>
> On 2022/1/27 10:54, Kai-Heng Feng wrote:
> > Commit 50310600ebda ("iommu/vt-d: Enable PCI ACS for platform opt in
> > hint") enables ACS, and some platforms lose its NVMe after resume from
> > S3:
> > [ 50.947816] pcieport 0000:00:1b.0: DPC: containment event, status:0x1f01 source:0x0000
> > [ 50.947817] pcieport 0000:00:1b.0: DPC: unmasked uncorrectable error detected
> > [ 50.947829] pcieport 0000:00:1b.0: PCIe Bus Error: severity=Uncorrected (Non-Fatal), type=Transaction Layer, (Receiver ID)
> > [ 50.947830] pcieport 0000:00:1b.0: device [8086:06ac] error status/mask=00200000/00010000
> > [ 50.947831] pcieport 0000:00:1b.0: [21] ACSViol (First)
> > [ 50.947841] pcieport 0000:00:1b.0: AER: broadcast error_detected message
> > [ 50.947843] nvme nvme0: frozen state error detected, reset controller
> >
> > It happens right after ACS gets enabled during resume.
> >
> > There's another case, when Thunderbolt reaches D3cold:
> > [ 30.100211] pcieport 0000:00:1d.0: AER: Uncorrected (Non-Fatal) error received: 0000:00:1d.0
> > [ 30.100251] pcieport 0000:00:1d.0: PCIe Bus Error: severity=Uncorrected (Non-Fatal), type=Transaction Layer, (Requester ID)
> > [ 30.100256] pcieport 0000:00:1d.0: device [8086:7ab0] error status/mask=00100000/00004000
> > [ 30.100262] pcieport 0000:00:1d.0: [20] UnsupReq (First)
> > [ 30.100267] pcieport 0000:00:1d.0: AER: TLP Header: 34000000 08000052 00000000 00000000
> > [ 30.100372] thunderbolt 0000:0a:00.0: AER: can't recover (no error_detected callback)
> > [ 30.100401] xhci_hcd 0000:3e:00.0: AER: can't recover (no error_detected callback)
> > [ 30.100427] pcieport 0000:00:1d.0: AER: device recovery failed
> >
> > So disable AER service to avoid the noises from turning power rails
> > on/off when the device is in low power states (D3hot and D3cold), as
> > PCIe spec "5.2 Link State Power Management" states that TLP and DLLP
> > transmission is disabled for a Link in L2/L3 Ready (D3hot), L2 (D3cold
> > with aux power) and L3 (D3cold).
> >
> > Bugzilla:https://bugzilla.kernel.org/show_bug.cgi?id=209149
> > Bugzilla:https://bugzilla.kernel.org/show_bug.cgi?id=215453
> > Fixes: 50310600ebda ("iommu/vt-d: Enable PCI ACS for platform opt in hint")
>
> I don't know what this fix has to do with the commit 50310600ebda.
Commit 50310600ebda only exposed the underlying issue. Do you think
"Fixes:" tag should change to other commits?
> Commit 50310600ebda only makes sure that PCI ACS is enabled whenever
> Intel IOMMU is on. Before this commit, PCI ACS could also be enabled
> and result in the same problem. Or anything I missed?
The system in question didn't enable ACS before commit 50310600ebda.
Kai-Heng
>
> Best regards,
> baolu
^ permalink raw reply
* [PATCH v2 0/5] Allocate module text and data separately
From: Christophe Leroy @ 2022-01-27 11:27 UTC (permalink / raw)
To: Luis Chamberlain, Jessica Yu
Cc: linux-arch@vger.kernel.org, kgdb-bugreport@lists.sourceforge.net,
linux-kernel@vger.kernel.org, linux-mm@kvack.org,
linuxppc-dev@lists.ozlabs.org
This series allow architectures to request having modules data in
vmalloc area instead of module area.
This is required on powerpc book3s/32 in order to set data non
executable, because it is not possible to set executability on page
basis, this is done per 256 Mbytes segments. The module area has exec
right, vmalloc area has noexec. Without this change module data
remains executable regardless of CONFIG_STRICT_MODULES_RWX.
This can also be useful on other powerpc/32 in order to maximize the
chance of code being close enough to kernel core to avoid branch
trampolines.
Changes in v2:
- Dropped first two patches which are not necessary. They may be added back later as a follow-up series.
- Fixed the printks in GDB
Christophe Leroy (5):
modules: Always have struct mod_tree_root
modules: Prepare for handling several RB trees
modules: Introduce data_layout
modules: Add CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
powerpc: Select ARCH_WANTS_MODULES_DATA_IN_VMALLOC on book3s/32 and
8xx
arch/Kconfig | 6 ++
arch/powerpc/Kconfig | 1 +
include/linux/module.h | 8 ++
kernel/debug/kdb/kdb_main.c | 10 +-
kernel/module.c | 193 +++++++++++++++++++++++++-----------
5 files changed, 156 insertions(+), 62 deletions(-)
--
2.33.1
^ permalink raw reply
* [PATCH v2 2/5] modules: Prepare for handling several RB trees
From: Christophe Leroy @ 2022-01-27 11:28 UTC (permalink / raw)
To: Luis Chamberlain, Jessica Yu
Cc: linux-arch@vger.kernel.org, kgdb-bugreport@lists.sourceforge.net,
linux-kernel@vger.kernel.org, linux-mm@kvack.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <cover.1643282353.git.christophe.leroy@csgroup.eu>
In order to separate text and data, we need to setup
two rb trees. So modify functions to give the tree
as a parameter.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
kernel/module.c | 38 +++++++++++++++++++-------------------
1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/kernel/module.c b/kernel/module.c
index c0f9d63d3f05..2b9a3d9d3c0d 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -159,14 +159,14 @@ static const struct latch_tree_ops mod_tree_ops = {
.comp = mod_tree_comp,
};
-static noinline void __mod_tree_insert(struct mod_tree_node *node)
+static noinline void __mod_tree_insert(struct mod_tree_node *node, struct mod_tree_root *tree)
{
- latch_tree_insert(&node->node, &mod_tree.root, &mod_tree_ops);
+ latch_tree_insert(&node->node, &tree->root, &mod_tree_ops);
}
-static void __mod_tree_remove(struct mod_tree_node *node)
+static void __mod_tree_remove(struct mod_tree_node *node, struct mod_tree_root *tree)
{
- latch_tree_erase(&node->node, &mod_tree.root, &mod_tree_ops);
+ latch_tree_erase(&node->node, &tree->root, &mod_tree_ops);
}
/*
@@ -178,28 +178,28 @@ static void mod_tree_insert(struct module *mod)
mod->core_layout.mtn.mod = mod;
mod->init_layout.mtn.mod = mod;
- __mod_tree_insert(&mod->core_layout.mtn);
+ __mod_tree_insert(&mod->core_layout.mtn, &mod_tree);
if (mod->init_layout.size)
- __mod_tree_insert(&mod->init_layout.mtn);
+ __mod_tree_insert(&mod->init_layout.mtn, &mod_tree);
}
static void mod_tree_remove_init(struct module *mod)
{
if (mod->init_layout.size)
- __mod_tree_remove(&mod->init_layout.mtn);
+ __mod_tree_remove(&mod->init_layout.mtn, &mod_tree);
}
static void mod_tree_remove(struct module *mod)
{
- __mod_tree_remove(&mod->core_layout.mtn);
+ __mod_tree_remove(&mod->core_layout.mtn, &mod_tree);
mod_tree_remove_init(mod);
}
-static struct module *mod_find(unsigned long addr)
+static struct module *mod_find(unsigned long addr, struct mod_tree_root *tree)
{
struct latch_tree_node *ltn;
- ltn = latch_tree_find((void *)addr, &mod_tree.root, &mod_tree_ops);
+ ltn = latch_tree_find((void *)addr, &tree->root, &mod_tree_ops);
if (!ltn)
return NULL;
@@ -212,7 +212,7 @@ static void mod_tree_insert(struct module *mod) { }
static void mod_tree_remove_init(struct module *mod) { }
static void mod_tree_remove(struct module *mod) { }
-static struct module *mod_find(unsigned long addr)
+static struct module *mod_find(unsigned long addr, struct mod_tree_root *tree)
{
struct module *mod;
@@ -231,22 +231,22 @@ static struct module *mod_find(unsigned long addr)
* Bounds of module text, for speeding up __module_address.
* Protected by module_mutex.
*/
-static void __mod_update_bounds(void *base, unsigned int size)
+static void __mod_update_bounds(void *base, unsigned int size, struct mod_tree_root *tree)
{
unsigned long min = (unsigned long)base;
unsigned long max = min + size;
- if (min < mod_tree.addr_min)
- mod_tree.addr_min = min;
- if (max > mod_tree.addr_max)
- mod_tree.addr_max = max;
+ if (min < tree->addr_min)
+ tree->addr_min = min;
+ if (max > tree->addr_max)
+ tree->addr_max = max;
}
static void mod_update_bounds(struct module *mod)
{
- __mod_update_bounds(mod->core_layout.base, mod->core_layout.size);
+ __mod_update_bounds(mod->core_layout.base, mod->core_layout.size, &mod_tree);
if (mod->init_layout.size)
- __mod_update_bounds(mod->init_layout.base, mod->init_layout.size);
+ __mod_update_bounds(mod->init_layout.base, mod->init_layout.size, &mod_tree);
}
#ifdef CONFIG_KGDB_KDB
@@ -4739,7 +4739,7 @@ struct module *__module_address(unsigned long addr)
module_assert_mutex_or_preempt();
- mod = mod_find(addr);
+ mod = mod_find(addr, &mod_tree);
if (mod) {
BUG_ON(!within_module(addr, mod));
if (mod->state == MODULE_STATE_UNFORMED)
--
2.33.1
^ permalink raw reply related
* [PATCH v2 3/5] modules: Introduce data_layout
From: Christophe Leroy @ 2022-01-27 11:28 UTC (permalink / raw)
To: Luis Chamberlain, Jessica Yu
Cc: linux-arch@vger.kernel.org, kgdb-bugreport@lists.sourceforge.net,
linux-kernel@vger.kernel.org, linux-mm@kvack.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <cover.1643282353.git.christophe.leroy@csgroup.eu>
In order to allow separation of data from text, add another layout,
called data_layout. For architectures requesting separation of text
and data, only text will go in core_layout and data will go in
data_layout.
For architectures which keep text and data together, make data_layout
an alias of core_layout, that way data_layout can be used for all
data manipulations, regardless of whether data is in core_layout or
data_layout.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
kernel/module.c | 52 ++++++++++++++++++++++++++++---------------------
1 file changed, 30 insertions(+), 22 deletions(-)
diff --git a/kernel/module.c b/kernel/module.c
index 2b9a3d9d3c0d..2b70b997a36d 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -81,6 +81,8 @@
/* If this is set, the section belongs in the init part of the module */
#define INIT_OFFSET_MASK (1UL << (BITS_PER_LONG-1))
+#define data_layout core_layout
+
/*
* Mutex protects:
* 1) List of modules (also safely readable with preempt_disable),
@@ -2011,19 +2013,20 @@ static void module_enable_ro(const struct module *mod, bool after_init)
set_vm_flush_reset_perms(mod->init_layout.base);
frob_text(&mod->core_layout, set_memory_ro);
- frob_rodata(&mod->core_layout, set_memory_ro);
+ frob_rodata(&mod->data_layout, set_memory_ro);
+
frob_text(&mod->init_layout, set_memory_ro);
frob_rodata(&mod->init_layout, set_memory_ro);
if (after_init)
- frob_ro_after_init(&mod->core_layout, set_memory_ro);
+ frob_ro_after_init(&mod->data_layout, set_memory_ro);
}
static void module_enable_nx(const struct module *mod)
{
- frob_rodata(&mod->core_layout, set_memory_nx);
- frob_ro_after_init(&mod->core_layout, set_memory_nx);
- frob_writable_data(&mod->core_layout, set_memory_nx);
+ frob_rodata(&mod->data_layout, set_memory_nx);
+ frob_ro_after_init(&mod->data_layout, set_memory_nx);
+ frob_writable_data(&mod->data_layout, set_memory_nx);
frob_rodata(&mod->init_layout, set_memory_nx);
frob_writable_data(&mod->init_layout, set_memory_nx);
}
@@ -2201,7 +2204,7 @@ static void free_module(struct module *mod)
percpu_modfree(mod);
/* Free lock-classes; relies on the preceding sync_rcu(). */
- lockdep_free_key_range(mod->core_layout.base, mod->core_layout.size);
+ lockdep_free_key_range(mod->data_layout.base, mod->data_layout.size);
/* Finally, free the core (containing the module structure) */
module_memfree(mod->core_layout.base);
@@ -2448,7 +2451,10 @@ static void layout_sections(struct module *mod, struct load_info *info)
|| s->sh_entsize != ~0UL
|| module_init_layout_section(sname))
continue;
- s->sh_entsize = get_offset(mod, &mod->core_layout.size, s, i);
+ if (m)
+ s->sh_entsize = get_offset(mod, &mod->data_layout.size, s, i);
+ else
+ s->sh_entsize = get_offset(mod, &mod->core_layout.size, s, i);
pr_debug("\t%s\n", sname);
}
switch (m) {
@@ -2457,15 +2463,15 @@ static void layout_sections(struct module *mod, struct load_info *info)
mod->core_layout.text_size = mod->core_layout.size;
break;
case 1: /* RO: text and ro-data */
- mod->core_layout.size = debug_align(mod->core_layout.size);
- mod->core_layout.ro_size = mod->core_layout.size;
+ mod->data_layout.size = debug_align(mod->data_layout.size);
+ mod->data_layout.ro_size = mod->data_layout.size;
break;
case 2: /* RO after init */
- mod->core_layout.size = debug_align(mod->core_layout.size);
- mod->core_layout.ro_after_init_size = mod->core_layout.size;
+ mod->data_layout.size = debug_align(mod->data_layout.size);
+ mod->data_layout.ro_after_init_size = mod->data_layout.size;
break;
case 4: /* whole core */
- mod->core_layout.size = debug_align(mod->core_layout.size);
+ mod->data_layout.size = debug_align(mod->data_layout.size);
break;
}
}
@@ -2718,12 +2724,12 @@ static void layout_symtab(struct module *mod, struct load_info *info)
}
/* Append room for core symbols at end of core part. */
- info->symoffs = ALIGN(mod->core_layout.size, symsect->sh_addralign ?: 1);
- info->stroffs = mod->core_layout.size = info->symoffs + ndst * sizeof(Elf_Sym);
- mod->core_layout.size += strtab_size;
- info->core_typeoffs = mod->core_layout.size;
- mod->core_layout.size += ndst * sizeof(char);
- mod->core_layout.size = debug_align(mod->core_layout.size);
+ info->symoffs = ALIGN(mod->data_layout.size, symsect->sh_addralign ?: 1);
+ info->stroffs = mod->data_layout.size = info->symoffs + ndst * sizeof(Elf_Sym);
+ mod->data_layout.size += strtab_size;
+ info->core_typeoffs = mod->data_layout.size;
+ mod->data_layout.size += ndst * sizeof(char);
+ mod->data_layout.size = debug_align(mod->data_layout.size);
/* Put string table section at end of init part of module. */
strsect->sh_flags |= SHF_ALLOC;
@@ -2767,9 +2773,9 @@ static void add_kallsyms(struct module *mod, const struct load_info *info)
* Now populate the cut down core kallsyms for after init
* and set types up while we still have access to sections.
*/
- mod->core_kallsyms.symtab = dst = mod->core_layout.base + info->symoffs;
- mod->core_kallsyms.strtab = s = mod->core_layout.base + info->stroffs;
- mod->core_kallsyms.typetab = mod->core_layout.base + info->core_typeoffs;
+ mod->core_kallsyms.symtab = dst = mod->data_layout.base + info->symoffs;
+ mod->core_kallsyms.strtab = s = mod->data_layout.base + info->stroffs;
+ mod->core_kallsyms.typetab = mod->data_layout.base + info->core_typeoffs;
src = mod->kallsyms->symtab;
for (ndst = i = 0; i < mod->kallsyms->num_symtab; i++) {
mod->kallsyms->typetab[i] = elf_type(src + i, info);
@@ -3465,6 +3471,8 @@ static int move_module(struct module *mod, struct load_info *info)
if (shdr->sh_entsize & INIT_OFFSET_MASK)
dest = mod->init_layout.base
+ (shdr->sh_entsize & ~INIT_OFFSET_MASK);
+ else if (!(shdr->sh_flags & SHF_EXECINSTR))
+ dest = mod->data_layout.base + shdr->sh_entsize;
else
dest = mod->core_layout.base + shdr->sh_entsize;
@@ -4170,7 +4178,7 @@ static int load_module(struct load_info *info, const char __user *uargs,
mutex_unlock(&module_mutex);
free_module:
/* Free lock-classes; relies on the preceding sync_rcu() */
- lockdep_free_key_range(mod->core_layout.base, mod->core_layout.size);
+ lockdep_free_key_range(mod->data_layout.base, mod->data_layout.size);
module_deallocate(mod, info);
free_copy:
--
2.33.1
^ permalink raw reply related
* [PATCH v2 4/5] modules: Add CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
From: Christophe Leroy @ 2022-01-27 11:28 UTC (permalink / raw)
To: Luis Chamberlain, Jessica Yu
Cc: linux-arch@vger.kernel.org, Daniel Thompson,
kgdb-bugreport@lists.sourceforge.net,
linux-kernel@vger.kernel.org, Douglas Anderson,
linux-mm@kvack.org, Jason Wessel, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <cover.1643282353.git.christophe.leroy@csgroup.eu>
Add CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC to allow architectures
to request having modules data in vmalloc area instead of module area.
This is required on powerpc book3s/32 in order to set data non
executable, because it is not possible to set executability on page
basis, this is done per 256 Mbytes segments. The module area has exec
right, vmalloc area has noexec.
This can also be useful on other powerpc/32 in order to maximize the
chance of code being close enough to kernel core to avoid branch
trampolines.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: Jason Wessel <jason.wessel@windriver.com>
Cc: Daniel Thompson <daniel.thompson@linaro.org>
Cc: Douglas Anderson <dianders@chromium.org>
---
arch/Kconfig | 6 +++
include/linux/module.h | 8 ++++
kernel/debug/kdb/kdb_main.c | 10 ++++-
kernel/module.c | 76 +++++++++++++++++++++++++++++++++++--
4 files changed, 94 insertions(+), 6 deletions(-)
diff --git a/arch/Kconfig b/arch/Kconfig
index 678a80713b21..b5d1f2c19c27 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -882,6 +882,12 @@ config MODULES_USE_ELF_REL
Modules only use ELF REL relocations. Modules with ELF RELA
relocations will give an error.
+config ARCH_WANTS_MODULES_DATA_IN_VMALLOC
+ bool
+ help
+ For architectures like powerpc/32 which have constraints on module
+ allocation and need to allocate module data outside of module area.
+
config HAVE_IRQ_EXIT_ON_IRQ_STACK
bool
help
diff --git a/include/linux/module.h b/include/linux/module.h
index 1e135fd5c076..3a892bdcbb5f 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -422,6 +422,9 @@ struct module {
/* Core layout: rbtree is accessed frequently, so keep together. */
struct module_layout core_layout __module_layout_align;
struct module_layout init_layout;
+#ifdef CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
+ struct module_layout data_layout;
+#endif
/* Arch-specific module values */
struct mod_arch_specific arch;
@@ -569,6 +572,11 @@ bool is_module_text_address(unsigned long addr);
static inline bool within_module_core(unsigned long addr,
const struct module *mod)
{
+#ifdef CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
+ if ((unsigned long)mod->data_layout.base <= addr &&
+ addr < (unsigned long)mod->data_layout.base + mod->data_layout.size)
+ return true;
+#endif
return (unsigned long)mod->core_layout.base <= addr &&
addr < (unsigned long)mod->core_layout.base + mod->core_layout.size;
}
diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 0852a537dad4..85d3fd40b7fe 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -2022,8 +2022,11 @@ static int kdb_lsmod(int argc, const char **argv)
if (mod->state == MODULE_STATE_UNFORMED)
continue;
- kdb_printf("%-20s%8u 0x%px ", mod->name,
- mod->core_layout.size, (void *)mod);
+ kdb_printf("%-20s%8u", mod->name, mod->core_layout.size);
+#ifdef CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
+ kdb_printf("/%8u", mod->data_layout.size);
+#endif
+ kdb_printf(" 0x%px ", (void *)mod);
#ifdef CONFIG_MODULE_UNLOAD
kdb_printf("%4d ", module_refcount(mod));
#endif
@@ -2034,6 +2037,9 @@ static int kdb_lsmod(int argc, const char **argv)
else
kdb_printf(" (Live)");
kdb_printf(" 0x%px", mod->core_layout.base);
+#ifdef CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
+ kdb_printf("/0x%px", mod->data_layout.base);
+#endif
#ifdef CONFIG_MODULE_UNLOAD
{
diff --git a/kernel/module.c b/kernel/module.c
index 2b70b997a36d..884c9fb11813 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -81,7 +81,9 @@
/* If this is set, the section belongs in the init part of the module */
#define INIT_OFFSET_MASK (1UL << (BITS_PER_LONG-1))
+#ifndef CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
#define data_layout core_layout
+#endif
/*
* Mutex protects:
@@ -108,6 +110,12 @@ static struct mod_tree_root {
.addr_min = -1UL,
};
+#ifdef CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
+static struct mod_tree_root mod_data_tree __cacheline_aligned = {
+ .addr_min = -1UL,
+};
+#endif
+
#ifdef CONFIG_MODULES_TREE_LOOKUP
/*
@@ -183,6 +191,11 @@ static void mod_tree_insert(struct module *mod)
__mod_tree_insert(&mod->core_layout.mtn, &mod_tree);
if (mod->init_layout.size)
__mod_tree_insert(&mod->init_layout.mtn, &mod_tree);
+
+#ifdef CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
+ mod->data_layout.mtn.mod = mod;
+ __mod_tree_insert(&mod->data_layout.mtn, &mod_data_tree);
+#endif
}
static void mod_tree_remove_init(struct module *mod)
@@ -195,6 +208,9 @@ static void mod_tree_remove(struct module *mod)
{
__mod_tree_remove(&mod->core_layout.mtn, &mod_tree);
mod_tree_remove_init(mod);
+#ifdef CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
+ __mod_tree_remove(&mod->core_layout.mtn, &mod_data_tree);
+#endif
}
static struct module *mod_find(unsigned long addr, struct mod_tree_root *tree)
@@ -249,6 +265,9 @@ static void mod_update_bounds(struct module *mod)
__mod_update_bounds(mod->core_layout.base, mod->core_layout.size, &mod_tree);
if (mod->init_layout.size)
__mod_update_bounds(mod->init_layout.base, mod->init_layout.size, &mod_tree);
+#ifdef CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
+ __mod_update_bounds(mod->data_layout.base, mod->data_layout.size, &mod_data_tree);
+#endif
}
#ifdef CONFIG_KGDB_KDB
@@ -1178,6 +1197,17 @@ static ssize_t show_coresize(struct module_attribute *mattr,
static struct module_attribute modinfo_coresize =
__ATTR(coresize, 0444, show_coresize, NULL);
+#ifdef CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
+static ssize_t show_datasize(struct module_attribute *mattr,
+ struct module_kobject *mk, char *buffer)
+{
+ return sprintf(buffer, "%u\n", mk->mod->data_layout.size);
+}
+
+static struct module_attribute modinfo_datasize =
+ __ATTR(datasize, 0444, show_datasize, NULL);
+#endif
+
static ssize_t show_initsize(struct module_attribute *mattr,
struct module_kobject *mk, char *buffer)
{
@@ -1206,6 +1236,9 @@ static struct module_attribute *modinfo_attrs[] = {
&modinfo_srcversion,
&modinfo_initstate,
&modinfo_coresize,
+#ifdef CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
+ &modinfo_datasize,
+#endif
&modinfo_initsize,
&modinfo_taint,
#ifdef CONFIG_MODULE_UNLOAD
@@ -2208,6 +2241,9 @@ static void free_module(struct module *mod)
/* Finally, free the core (containing the module structure) */
module_memfree(mod->core_layout.base);
+#ifdef CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
+ vfree(mod->data_layout.base);
+#endif
}
void *__symbol_get(const char *symbol)
@@ -3459,6 +3495,24 @@ static int move_module(struct module *mod, struct load_info *info)
} else
mod->init_layout.base = NULL;
+#ifdef CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
+ /* Do the allocs. */
+ ptr = vmalloc(mod->data_layout.size);
+ /*
+ * The pointer to this block is stored in the module structure
+ * which is inside the block. Just mark it as not being a
+ * leak.
+ */
+ kmemleak_not_leak(ptr);
+ if (!ptr) {
+ module_memfree(mod->core_layout.base);
+ module_memfree(mod->init_layout.base);
+ return -ENOMEM;
+ }
+
+ memset(ptr, 0, mod->data_layout.size);
+ mod->data_layout.base = ptr;
+#endif
/* Transfer each section which specifies SHF_ALLOC */
pr_debug("final section addresses:\n");
for (i = 0; i < info->hdr->e_shnum; i++) {
@@ -3634,6 +3688,9 @@ static void module_deallocate(struct module *mod, struct load_info *info)
module_arch_freeing_init(mod);
module_memfree(mod->init_layout.base);
module_memfree(mod->core_layout.base);
+#ifdef CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
+ vfree(mod->data_layout.base);
+#endif
}
int __weak module_finalize(const Elf_Ehdr *hdr,
@@ -4612,13 +4669,17 @@ static int m_show(struct seq_file *m, void *p)
struct module *mod = list_entry(p, struct module, list);
char buf[MODULE_FLAGS_BUF_SIZE];
void *value;
+ unsigned int size;
/* We always ignore unformed modules. */
if (mod->state == MODULE_STATE_UNFORMED)
return 0;
- seq_printf(m, "%s %u",
- mod->name, mod->init_layout.size + mod->core_layout.size);
+ size = mod->init_layout.size + mod->core_layout.size;
+#ifdef CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
+ size += mod->data_layout.size;
+#endif
+ seq_printf(m, "%s %u", mod->name, size);
print_unload_info(m, mod);
/* Informative for users. */
@@ -4741,13 +4802,20 @@ bool is_module_address(unsigned long addr)
struct module *__module_address(unsigned long addr)
{
struct module *mod;
+ struct mod_tree_root *tree;
- if (addr < mod_tree.addr_min || addr > mod_tree.addr_max)
+ if (addr >= mod_tree.addr_min && addr <= mod_tree.addr_max)
+ tree = &mod_tree;
+#ifdef CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
+ else if (addr >= mod_data_tree.addr_min && addr <= mod_data_tree.addr_max)
+ tree = &mod_data_tree;
+#endif
+ else
return NULL;
module_assert_mutex_or_preempt();
- mod = mod_find(addr, &mod_tree);
+ mod = mod_find(addr, tree);
if (mod) {
BUG_ON(!within_module(addr, mod));
if (mod->state == MODULE_STATE_UNFORMED)
--
2.33.1
^ permalink raw reply related
* [PATCH v2 5/5] powerpc: Select ARCH_WANTS_MODULES_DATA_IN_VMALLOC on book3s/32 and 8xx
From: Christophe Leroy @ 2022-01-27 11:28 UTC (permalink / raw)
To: Luis Chamberlain, Jessica Yu
Cc: linux-arch@vger.kernel.org, kgdb-bugreport@lists.sourceforge.net,
linux-kernel@vger.kernel.org, linux-mm@kvack.org, Paul Mackerras,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <cover.1643282353.git.christophe.leroy@csgroup.eu>
book3s/32 and 8xx have a separate area for allocating modules,
defined by MODULES_VADDR / MODULES_END.
On book3s/32, it is not possible to protect against execution
on a page basis. A full 256M segment is either Exec or NoExec.
The module area is in an Exec segment while vmalloc area is
in a NoExec segment.
In order to protect module data against execution, select
ARCH_WANTS_MODULES_DATA_IN_VMALLOC.
For the 8xx (and possibly other 32 bits platform in the future),
there is no such constraint on Exec/NoExec protection, however
there is a critical distance between kernel functions and callers
that needs to remain below 32Mbytes in order to avoid costly
trampolines. By allocating data outside of module area, we
increase the chance for module text to remain within acceptable
distance from kernel core text.
So select ARCH_WANTS_MODULES_DATA_IN_VMALLOC for 8xx as well.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
---
arch/powerpc/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index b779603978e1..242eed8cedf8 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -152,6 +152,7 @@ config PPC
select ARCH_WANT_IPC_PARSE_VERSION
select ARCH_WANT_IRQS_OFF_ACTIVATE_MM
select ARCH_WANT_LD_ORPHAN_WARN
+ select ARCH_WANTS_MODULES_DATA_IN_VMALLOC if PPC_BOOK3S_32 || PPC_8xx
select ARCH_WEAK_RELEASE_ACQUIRE
select BINFMT_ELF
select BUILDTIME_TABLE_SORT
--
2.33.1
^ permalink raw reply related
* [PATCH v2 1/5] modules: Always have struct mod_tree_root
From: Christophe Leroy @ 2022-01-27 11:27 UTC (permalink / raw)
To: Luis Chamberlain, Jessica Yu
Cc: linux-arch@vger.kernel.org, kgdb-bugreport@lists.sourceforge.net,
linux-kernel@vger.kernel.org, linux-mm@kvack.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <cover.1643282353.git.christophe.leroy@csgroup.eu>
In order to separate text and data, we need to setup
two rb trees.
This also means that struct mod_tree_root is required even without
MODULES_TREE_LOOKUP.
Also remove module_addr_min and module_addr_max as there will
be one min and one max for each tree.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
kernel/module.c | 39 ++++++++++++++++++---------------------
1 file changed, 18 insertions(+), 21 deletions(-)
diff --git a/kernel/module.c b/kernel/module.c
index 24dab046e16c..c0f9d63d3f05 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -85,7 +85,7 @@
* Mutex protects:
* 1) List of modules (also safely readable with preempt_disable),
* 2) module_use links,
- * 3) module_addr_min/module_addr_max.
+ * 3) mod_tree.addr_min/mod_tree.addr_max.
* (delete and add uses RCU list operations).
*/
static DEFINE_MUTEX(module_mutex);
@@ -96,6 +96,16 @@ static void do_free_init(struct work_struct *w);
static DECLARE_WORK(init_free_wq, do_free_init);
static LLIST_HEAD(init_free_list);
+static struct mod_tree_root {
+#ifdef CONFIG_MODULES_TREE_LOOKUP
+ struct latch_tree_root root;
+#endif
+ unsigned long addr_min;
+ unsigned long addr_max;
+} mod_tree __cacheline_aligned = {
+ .addr_min = -1UL,
+};
+
#ifdef CONFIG_MODULES_TREE_LOOKUP
/*
@@ -149,17 +159,6 @@ static const struct latch_tree_ops mod_tree_ops = {
.comp = mod_tree_comp,
};
-static struct mod_tree_root {
- struct latch_tree_root root;
- unsigned long addr_min;
- unsigned long addr_max;
-} mod_tree __cacheline_aligned = {
- .addr_min = -1UL,
-};
-
-#define module_addr_min mod_tree.addr_min
-#define module_addr_max mod_tree.addr_max
-
static noinline void __mod_tree_insert(struct mod_tree_node *node)
{
latch_tree_insert(&node->node, &mod_tree.root, &mod_tree_ops);
@@ -209,8 +208,6 @@ static struct module *mod_find(unsigned long addr)
#else /* MODULES_TREE_LOOKUP */
-static unsigned long module_addr_min = -1UL, module_addr_max = 0;
-
static void mod_tree_insert(struct module *mod) { }
static void mod_tree_remove_init(struct module *mod) { }
static void mod_tree_remove(struct module *mod) { }
@@ -239,10 +236,10 @@ static void __mod_update_bounds(void *base, unsigned int size)
unsigned long min = (unsigned long)base;
unsigned long max = min + size;
- if (min < module_addr_min)
- module_addr_min = min;
- if (max > module_addr_max)
- module_addr_max = max;
+ if (min < mod_tree.addr_min)
+ mod_tree.addr_min = min;
+ if (max > mod_tree.addr_max)
+ mod_tree.addr_max = max;
}
static void mod_update_bounds(struct module *mod)
@@ -4546,14 +4543,14 @@ static void cfi_init(struct module *mod)
mod->exit = *exit;
#endif
- cfi_module_add(mod, module_addr_min);
+ cfi_module_add(mod, mod_tree.addr_min);
#endif
}
static void cfi_cleanup(struct module *mod)
{
#ifdef CONFIG_CFI_CLANG
- cfi_module_remove(mod, module_addr_min);
+ cfi_module_remove(mod, mod_tree.addr_min);
#endif
}
@@ -4737,7 +4734,7 @@ struct module *__module_address(unsigned long addr)
{
struct module *mod;
- if (addr < module_addr_min || addr > module_addr_max)
+ if (addr < mod_tree.addr_min || addr > mod_tree.addr_max)
return NULL;
module_assert_mutex_or_preempt();
--
2.33.1
^ permalink raw reply related
* Re: [PATCH 1/7] modules: Refactor within_module_core() and within_module_init()
From: Christophe Leroy @ 2022-01-27 11:32 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-arch@vger.kernel.org, kgdb-bugreport@lists.sourceforge.net,
linux-kernel@vger.kernel.org, linux-mm@kvack.org,
Luis Chamberlain, Jessica Yu, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <Ye6cTJKTD9JehwnY@infradead.org>
Le 24/01/2022 à 13:32, Christoph Hellwig a écrit :
> On Mon, Jan 24, 2022 at 09:22:15AM +0000, Christophe Leroy wrote:
>> +static inline bool within_range(unsigned long addr, void *base, unsigned int size)
>
> Please avoid the overly long line.
>
> .. But given that this function only has a single caller I see no
> point in factoring it out anyway.
I finally decided to drop this change from the series as it brings
little added value.
Thanks
Christophe
^ permalink raw reply
* Re: [PATCH 1/7] modules: Refactor within_module_core() and within_module_init()
From: Christophe Leroy @ 2022-01-27 11:33 UTC (permalink / raw)
To: Mike Rapoport
Cc: linux-arch@vger.kernel.org, kgdb-bugreport@lists.sourceforge.net,
linux-kernel@vger.kernel.org, linux-mm@kvack.org,
Luis Chamberlain, Jessica Yu, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <YfG+zVl8aV+UszoE@kernel.org>
Le 26/01/2022 à 22:36, Mike Rapoport a écrit :
> On Mon, Jan 24, 2022 at 09:22:15AM +0000, Christophe Leroy wrote:
>> within_module_core() and within_module_init() are doing the exact same
>> test, one on core_layout, the second on init_layout.
>>
>> In preparation of increasing the complexity of that verification,
>> refactor it into a single function called within_module_layout().
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>> ---
>> include/linux/module.h | 17 +++++++++++++----
>> 1 file changed, 13 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/linux/module.h b/include/linux/module.h
>> index c9f1200b2312..33b4db8f5ca5 100644
>> --- a/include/linux/module.h
>> +++ b/include/linux/module.h
>> @@ -565,18 +565,27 @@ bool __is_module_percpu_address(unsigned long addr, unsigned long *can_addr);
>> bool is_module_percpu_address(unsigned long addr);
>> bool is_module_text_address(unsigned long addr);
>>
>> +static inline bool within_range(unsigned long addr, void *base, unsigned int size)
>> +{
>> + return addr >= (unsigned long)base && addr < (unsigned long)base + size;
>> +}
>
> There's also 'within' at least in arch/x86/mm/pat/set_memory.c and surely
> tons of open-coded "address within" code.
>
> Should it live in, say, include/linux/range.h?
>
include/linux/range.h has functions that work with struct ranges.
It might be an alternative, to be investigated a bit more.
At the time being, this change finally brings little added value so
I drop the two first patches from the series.
Thanks
Christophe
^ permalink raw reply
* Re: [powerpc] ftrace warning kernel/trace/ftrace.c:2068 with code-patching selftests
From: Mark Rutland @ 2022-01-27 11:46 UTC (permalink / raw)
To: Yinan Liu
Cc: keescook, linux-kernel, Steven Rostedt, Sachin Sant, linuxppc-dev,
ardb
In-Reply-To: <YfFclROd+0/61q2d@FVFF77S0Q05N>
[adding LKML so this is easier for others to find]
If anyone wants to follow the thread from the start, it's at:
https://lore.kernel.org/linuxppc-dev/944D10DA-8200-4BA9-8D0A-3BED9AA99F82@linux.ibm.com/
Ard, I was under the impression that the 32-bit arm kernel was (virtually)
relocatable, but I couldn't spot where, and suspect I'm mistaken. Do you know
whether it currently does any boot-time dynamic relocation?
Kees, there's an x86_64 relocation question for you at the end.
On Wed, Jan 26, 2022 at 02:37:16PM +0000, Mark Rutland wrote:
> Hi,
>
> Steve pointed me at this thread over IRC -- I'm not subscribed to this list so
> grabbed a copy of the thread thus far via b4.
>
> On Tue, Jan 25, 2022 at 11:20:27AM +0800, Yinan Liu wrote:
> > > Yeah, I think it's time to opt in, instead of opting out.
>
> I agree this must be opt-in rather than opt-out.
>
> However, I think most architectures were broken (in at least some
> configurations) by commit:
>
> 72b3942a173c387b ("scripts: ftrace - move the sort-processing in ftrace_init")
>
> ... and so I don't think this fix is correct as-is, and we might want to revert
> that or at least mark is as BROKEN for now.
Steve asked for a bit more detail on IRC, so the below is an attempt to explain
what's actually going on here.
The short answer is that relocatable kernels (e.g. those with KASLR support)
need to handle the kernel being loaded at (somewhat) arbitrary virtual
addresses. Even where code can be position-independent, any pointers in the
kernel image (e.g. the contents of the mcount_loc table) need to be updated to
account for the specific VA the kernel was loaded at -- arch code does this
early at boot time by applying dynamic (ELF) relocations.
Walking through how we get there, considering arm64 specifically:
1) When an object is created with traceable functions:
The compiler records the addresses of the callsites into a section. Those
are absolute virtual addresses, but the final virtual addresses are not yet
known, so the compiler generates ELF relocations to tell the linker how to
fill these in later.
On arm64, since the compiler doesn't know the final value yet, it fills the
actual values with zero for now. Other architectures might do differently.
For example, per `objdump -r init/main.o`:
| RELOCATION RECORDS FOR [__patchable_function_entries]:
| OFFSET TYPE VALUE
| 0000000000000000 R_AARCH64_ABS64 .text+0x0000000000000028
| 0000000000000008 R_AARCH64_ABS64 .text+0x0000000000000088
| 0000000000000010 R_AARCH64_ABS64 .text+0x00000000000000e8
2) When vmlinux is linked:
The linker script accumulates the callsite pointers from all the object
files into the mcount_loc table. Since the kernel is relocatable, the
runtime absolute addresses are still not yet known, but the offset relative
to the kernel base is known, and so the linker consumes the absolute
relocations created by the compiler and generates new relocations relative
to the kernel's default load address so that these can be adjusted at boot
time.
On arm64, those are RELA and/or RELR relocations, which our vmlinux.lds.S
accumulates those into a location in the initdata section that the kernel
can find at boot time:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/kernel/vmlinux.lds.S?h=v5.17-rc1#n252
For example, per `objdump -s vmlinux -j .rela.dyn`:
| vmlinux: file format elf64-littleaarch64
|
| Contents of section .rela.dyn:
| ffff800009beb0d0 b01a0b09 0080ffff 03040000 00000000 ................
| ffff800009beb0e0 00600b09 0080ffff b81a0b09 0080ffff .`..............
| ffff800009beb0f0 03040000 00000000 80710b09 0080ffff .........q......
| ffff800009beb100 e8670b09 0080ffff 03040000 00000000 .g..............
| ffff800009beb110 48e60809 0080ffff f0670b09 0080ffff H........g......
| ffff800009beb120 03040000 00000000 ec190b09 0080ffff ................
Each of the relocations in .rela.dyn consists of 3 64-bit little-endian
values:
* The first (e.g. 0xffff8000090b1ab0) is the VA of the pointer to write to,
assuming the kernel's default load address (e.g. 0xffff800008000000). An
offset must be applied to this depending on where the kernel was actually
loaded relative to that default load address.
* The second (e.g. 0x0000000000000403) is the ELF relocation type (1027, AKA
R_AARCH64_RELATIVE).
* The third, (e.g. 0xffff8000090b6000) is the VA to write to the pointer,
assuming the kernel's default load address (e.g. 0xffff800008000000). An
offset must be applied to this depending on where the kernel was actually
loaded relative to that default load address.
The AArch64 ELF spec defines our relocations, e.g.
https://github.com/ARM-software/abi-aa/blob/main/aaelf64/aaelf64.rst#5712dynamic-relocations
In general, relocations might depend on the default value that was
initialized (e.g. OR-ing in high bits assuming the low bits are already
correct). I'm not sure if any of the architectures we support uses such
relocations for relocatable kernels, but they could in theory.
3) When the vmlinux mcount_loc table is sorted:
The sorttable code sorts the data values within the mcount_loc table, but it
doesn't read the relocations to resolve the VA of each entry, nor does it
update the relocations when it swaps entries.
For arm64, where entries were all initialized to zero at compile time and
have not been updated since, the sort does nothing. When the relocations are
applied later, the result will be an unsorted table.
In general, where sorting *does* swap entries, it doesn't update the
corresponding relocations. Where entries A and B get swapped by the sort,
any relocation(s) for entry A will apply to the location of entry B, and
vice-versa. Depending on the specific relocation used, that may or may not
be a problem (e.g. the example of OR-ing in high bits would be broken).
4) When relocations are applied at boot time:
On arm64, to account for KASLR or any other virtual offset we might have to
account for, we apply the relocations early in boot in the
__relocate_kernel() assembly function:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/kernel/head.S?h=v5.17-rc1#n753
Since we didn't update the relocations during the build-time sort, and the
mcount_loc table was not sorted to begin with, applying all the relocations
for the mcount_loc table results in an unsorted table. Hence things go
wrong later for any code relying on this having been sorted already.
IIUC the s390 and powerpc cases are structurally similar, though the fine
detail might differ a bit.
I'm not sure how x86 works here; AFAICT the relocations are performed during
decompression, but it looks like there's some special build-time processing
associated with that, and the vmlinux doesn't contain standard ELF relocations.
Kees, IIUC you added the x86_64 support there, can you shed any light on if/how
this works on x86?
In practice, building v5.17-rc1 with CONFIG_FTRACE_SORT_STARTUP_TEST=y I see
the following splat:
| ------------[ cut here ]------------
| [14] unknown_bootoption+0x4/0x1c8 at ffffa1a861200738 is not sorted with trace_initcall_finish_cb+0x4/0x6c at ffffa1a85f8130b8
| WARNING: CPU: 0 PID: 0 at kernel/trace/ftrace.c:6403 ftrace_process_locs.isra.0+0x370/0x440
| Modules linked in:
| CPU: 0 PID: 0 Comm: swapper Not tainted 5.17.0-rc1 #2
| Hardware name: linux,dummy-virt (DT)
| pstate: 600000c5 (nZCv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
| pc : ftrace_process_locs.isra.0+0x370/0x440
| lr : ftrace_process_locs.isra.0+0x370/0x440
| sp : ffffa1a861b23d90
| x29: ffffa1a861b23d90 x28: 0000000000000000 x27: 0000000000000000
| x26: 0000000000000000 x25: 0000000000000000 x24: ffffa1a8612c0008
| x23: ffffa1a861302ea8 x22: ffffa1a861374320 x21: 0000000000000001
| x20: 000000000000e28f x19: 000000000000000e x18: ffffffffffffffff
| x17: 726f7320746f6e20 x16: 7369203833373030 x15: 3231363861316166
| x14: 6666662074612038 x13: 3862303331386635 x12: 3861316166666666
| x11: 2074612063367830 x10: ffffa1a861b4b1d0 x9 : ffffa1a861b4b1d0
| x8 : 00000000ffffefff x7 : ffffa1a861ba31d0 x6 : ffffa1a861ba31d0
| x5 : 000000000000bff4 x4 : 0000000000000000 x3 : 0000000000000000
| x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffffa1a861b33e80
| Call trace:
| ftrace_process_locs.isra.0+0x370/0x440
| ftrace_init+0xb4/0x15c
| start_kernel+0x40c/0x6d4
| __primary_switched+0xc0/0xc8
| ---[ end trace 0000000000000000 ]---
Where those symbol adddresses are:
| [mark@lakrids:~/src/linux]% usekorg 11.1.0 aarch64-linux-objdump -t vmlinux | grep -w unknown_bootoption
| ffff800009a00734 l F .init.text 00000000000001c8 unknown_bootoption
| [mark@lakrids:~/src/linux]% usekorg 11.1.0 aarch64-linux-objdump -t vmlinux | grep -w trace_initcall_finish_cb
| ffff8000080130b4 l F .text 0000000000000064 trace_initcall_finish_cb
... and are obviously not sorted.
Further, the ftrace tests fail quite horribly, e.g.
| # ./ftracetest
| === Ftrace unit tests ===
| [1] Basic trace file check [PASS]
| [2] Basic test for tracers[ 38.979280] ------------[ ftrace bug ]------------
| [ 38.980225] ftrace faulted on modifying
| [ 38.980227] [<ffffa8ebbe6003fc>] set_reset_devices+0x8/0x24
| [ 38.982078] Setting ftrace call site to call ftrace function
| [ 38.983160] ftrace record flags: 80000001
| [ 38.983921] (1)
| [ 38.983921] expected tramp: ffffa8ebbcc2ba20
| [ 38.985132] ------------[ cut here ]------------
| [ 38.986013] WARNING: CPU: 3 PID: 265 at kernel/trace/ftrace.c:2068 ftrace_bug+0x284/0x2b4
| [ 38.987649] Modules linked in:
| [ 38.988275] CPU: 3 PID: 265 Comm: ftracetest Tainted: G W 5.17.0-rc1-dirty #4
| [ 38.989979] Hardware name: linux,dummy-virt (DT)
| [ 38.990916] pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
| [ 38.992318] pc : ftrace_bug+0x284/0x2b4
| [ 38.993111] lr : ftrace_bug+0x284/0x2b4
| [ 38.993893] sp : ffff80000888bb00
| [ 38.994566] x29: ffff80000888bb00 x28: 0000000000000020 x27: ffffa8ebbefcaa70
| [ 38.996002] x26: ffffa8ebbefcb7e0 x25: ffffa8ebbf36723c x24: ffffa8ebbf364000
| [ 38.997447] x23: 00000000fffffff2 x22: ffffa8ebbe38f6e8 x21: ffffa8ebbef29c78
| [ 38.998885] x20: ffff5ec380080030 x19: ffffa8ebbef29000 x18: ffffffffffffffff
| [ 39.000316] x17: 0000000000000000 x16: 0000000000000000 x15: ffff80008888b827
| [ 39.001758] x14: 0000000000000000 x13: 3032616232636362 x12: 6265386166666666
| [ 39.003202] x11: 203a706d61727420 x10: ffffa8ebbef4b1d0 x9 : ffffa8ebbcd0dff0
| [ 39.004645] x8 : 00000000ffffefff x7 : ffffa8ebbefa31d0 x6 : ffffa8ebbefa31d0
| [ 39.006092] x5 : ffff5ec4befa29d8 x4 : 0000000000000000 x3 : 0000000000000000
| [ 39.007538] x2 : 0000000000000000 x1 : ffff5ec380cf5580 x0 : 0000000000000022
| [ 39.008982] Call trace:
| [ 39.009495] ftrace_bug+0x284/0x2b4
| [ 39.010212] ftrace_replace_code+0x9c/0xa4
| [ 39.011054] ftrace_modify_all_code+0xe4/0x14c
| [ 39.011964] arch_ftrace_update_code+0x18/0x24
| [ 39.012874] ftrace_run_update_code+0x24/0x7c
| [ 39.013770] ftrace_startup+0xf8/0x1b0
| [ 39.014541] register_ftrace_graph+0x2dc/0x324
| [ 39.015449] graph_trace_init+0x6c/0x74
| [ 39.016232] tracing_set_tracer+0xec/0x17c
| [ 39.017071] tracing_set_trace_write+0xe8/0x150
| [ 39.017989] vfs_write+0xfc/0x2a0
| [ 39.018671] ksys_write+0x74/0x100
| [ 39.019370] __arm64_sys_write+0x28/0x3c
| [ 39.020172] invoke_syscall+0x50/0x120
| [ 39.020947] el0_svc_common.constprop.0+0xdc/0x100
| [ 39.021935] do_el0_svc+0x34/0xa0
| [ 39.022620] el0_svc+0x28/0x80
| [ 39.023253] el0t_64_sync_handler+0xa8/0x130
| [ 39.024121] el0t_64_sync+0x1a0/0x1a4
| [ 39.024874] ---[ end trace 0000000000000000 ]---
| [PASS]
| [3] Basic trace clock test [FAIL]
| [4] Basic event tracing check [FAIL]
| [5] Change the ringbuffer size [FAIL]
Thanks,
Mark.
>
> More on that below.
>
> > >
> > > Something like this:
> > >
> > > -- Steve
> > >
> > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > > index c2724d986fa0..5256ebe57451 100644
> > > --- a/arch/arm/Kconfig
> > > +++ b/arch/arm/Kconfig
> > > @@ -82,6 +82,7 @@ config ARM
> > > select HAVE_EBPF_JIT if !CPU_ENDIAN_BE32
> > > select HAVE_CONTEXT_TRACKING
> > > select HAVE_C_RECORDMCOUNT
> > > + select HAVE_BUILDTIME_MCOUNT_SORT
> > > select HAVE_DEBUG_KMEMLEAK if !XIP_KERNEL
> > > select HAVE_DMA_CONTIGUOUS if MMU
> > > select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && !CPU_ENDIAN_BE32 && MMU
>
> IIUC the 32-bit arm kernel can be relocated at boot time, so I don't believe
> this is correct, and I believe any relocatable arm kernel has been broken since
> htat was introduced.
>
> > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > > index c4207cf9bb17..7996548b2b27 100644
> > > --- a/arch/arm64/Kconfig
> > > +++ b/arch/arm64/Kconfig
> > > @@ -166,6 +166,7 @@ config ARM64
> > > select HAVE_ASM_MODVERSIONS
> > > select HAVE_EBPF_JIT
> > > select HAVE_C_RECORDMCOUNT
> > > + select HAVE_BUILDTIME_MCOUNT_SORT
> > > select HAVE_CMPXCHG_DOUBLE
> > > select HAVE_CMPXCHG_LOCAL
> > > select HAVE_CONTEXT_TRACKING
>
> The arm64 kernel is relocatable by default, and has been broken since the
> build-time sort was introduced -- I see ftrace test failures, and the
> CONFIG_FTRACE_SORT_STARTUP_TEST screams at boot time.
>
> > > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > > index 7399327d1eff..46080dea5dba 100644
> > > --- a/arch/x86/Kconfig
> > > +++ b/arch/x86/Kconfig
> > > @@ -186,6 +186,7 @@ config X86
> > > select HAVE_CONTEXT_TRACKING_OFFSTACK if HAVE_CONTEXT_TRACKING
> > > select HAVE_C_RECORDMCOUNT
> > > select HAVE_OBJTOOL_MCOUNT if STACK_VALIDATION
> > > + select HAVE_BUILDTIME_MCOUNT_SORT
>
> Isn't x86 relocatable in some configurations (e.g. for KASLR)?
>
> I can't see how the sort works for those cases, because the mcount_loc entries
> are absolute, and either:
>
> * The sorted entries will get overwritten by the unsorted relocation entries,
> and won't be sorted.
>
> * The sorted entries won't get overwritten, but then the absolute address will
> be wrong since they hadn't been relocated.
>
> How does that work?
>
> Thanks,
> Mark.
>
> > > select HAVE_DEBUG_KMEMLEAK
> > > select HAVE_DMA_CONTIGUOUS
> > > select HAVE_DYNAMIC_FTRACE
> > > diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
> > > index 752ed89a293b..7e5b92090faa 100644
> > > --- a/kernel/trace/Kconfig
> > > +++ b/kernel/trace/Kconfig
> > > @@ -70,10 +70,16 @@ config HAVE_C_RECORDMCOUNT
> > > help
> > > C version of recordmcount available?
> > > +config HAVE_BUILDTIME_MCOUNT_SORT
> > > + bool
> > > + help
> > > + An architecture selects this if it sorts the mcount_loc section
> > > + at build time.
> > > +
> > > config BUILDTIME_MCOUNT_SORT
> > > bool
> > > default y
> > > - depends on BUILDTIME_TABLE_SORT && !S390
> > > + depends on HAVE_BUILDTIME_MCOUNT_SORT
> > > help
> > > Sort the mcount_loc section at build time.
> >
> > LGTM. This will no longer destroy ftrace on other architectures.
> > Those arches that we are not sure about can test and enable this function by
> > themselves.
> >
> >
> > Best regards
> > --yinan
> >
^ permalink raw reply
* Re: [powerpc] ftrace warning kernel/trace/ftrace.c:2068 with code-patching selftests
From: Ard Biesheuvel @ 2022-01-27 12:03 UTC (permalink / raw)
To: Mark Rutland
Cc: Kees Cook, Linux Kernel Mailing List, Steven Rostedt, Sachin Sant,
Yinan Liu, open list:LINUX FOR POWERPC (32-BIT AND 64-BIT)
In-Reply-To: <YfKGKWW5UfZ15kCW@FVFF77S0Q05N>
On Thu, 27 Jan 2022 at 12:47, Mark Rutland <mark.rutland@arm.com> wrote:
>
> [adding LKML so this is easier for others to find]
>
> If anyone wants to follow the thread from the start, it's at:
>
> https://lore.kernel.org/linuxppc-dev/944D10DA-8200-4BA9-8D0A-3BED9AA99F82@linux.ibm.com/
>
> Ard, I was under the impression that the 32-bit arm kernel was (virtually)
> relocatable, but I couldn't spot where, and suspect I'm mistaken. Do you know
> whether it currently does any boot-time dynamic relocation?
>
No, it does not.
..
> > Steve pointed me at this thread over IRC -- I'm not subscribed to this list so
> > grabbed a copy of the thread thus far via b4.
> >
> > On Tue, Jan 25, 2022 at 11:20:27AM +0800, Yinan Liu wrote:
> > > > Yeah, I think it's time to opt in, instead of opting out.
> >
> > I agree this must be opt-in rather than opt-out.
> >
> > However, I think most architectures were broken (in at least some
> > configurations) by commit:
> >
> > 72b3942a173c387b ("scripts: ftrace - move the sort-processing in ftrace_init")
> >
> > ... and so I don't think this fix is correct as-is, and we might want to revert
> > that or at least mark is as BROKEN for now.
>
> Steve asked for a bit more detail on IRC, so the below is an attempt to explain
> what's actually going on here.
>
> The short answer is that relocatable kernels (e.g. those with KASLR support)
> need to handle the kernel being loaded at (somewhat) arbitrary virtual
> addresses. Even where code can be position-independent, any pointers in the
> kernel image (e.g. the contents of the mcount_loc table) need to be updated to
> account for the specific VA the kernel was loaded at -- arch code does this
> early at boot time by applying dynamic (ELF) relocations.
>
These architectures use place-relative extables for the same reason:
place relative references are resolved at build time rather than at
runtime during relocation, making a build time sort feasible.
arch/alpha/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
arch/arm64/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
arch/ia64/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
arch/parisc/include/asm/uaccess.h:#define ARCH_HAS_RELATIVE_EXTABLE
arch/powerpc/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
arch/riscv/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
arch/s390/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
arch/x86/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
Note that the swap routine becomes something like the below, given
that the relative references need to be fixed up after the entry
changes place in the sorted list.
static void swap_ex(void *a, void *b, int size)
{
struct exception_table_entry *x = a, *y = b, tmp;
int delta = b - a;
tmp = *x;
x->insn = y->insn + delta;
y->insn = tmp.insn - delta;
...
}
As a bonus, the resulting footprint of the table in the image is
reduced by 8x, given that every 8 byte pointer has an accompanying 24
byte RELA record, so we go from 32 bytes to 4 bytes for every call to
__gnu_mcount_mc.
> Walking through how we get there, considering arm64 specifically:
>
> 1) When an object is created with traceable functions:
>
> The compiler records the addresses of the callsites into a section. Those
> are absolute virtual addresses, but the final virtual addresses are not yet
> known, so the compiler generates ELF relocations to tell the linker how to
> fill these in later.
>
> On arm64, since the compiler doesn't know the final value yet, it fills the
> actual values with zero for now. Other architectures might do differently.
>
> For example, per `objdump -r init/main.o`:
>
> | RELOCATION RECORDS FOR [__patchable_function_entries]:
> | OFFSET TYPE VALUE
> | 0000000000000000 R_AARCH64_ABS64 .text+0x0000000000000028
> | 0000000000000008 R_AARCH64_ABS64 .text+0x0000000000000088
> | 0000000000000010 R_AARCH64_ABS64 .text+0x00000000000000e8
>
> 2) When vmlinux is linked:
>
> The linker script accumulates the callsite pointers from all the object
> files into the mcount_loc table. Since the kernel is relocatable, the
> runtime absolute addresses are still not yet known, but the offset relative
> to the kernel base is known, and so the linker consumes the absolute
> relocations created by the compiler and generates new relocations relative
> to the kernel's default load address so that these can be adjusted at boot
> time.
>
> On arm64, those are RELA and/or RELR relocations, which our vmlinux.lds.S
> accumulates those into a location in the initdata section that the kernel
> can find at boot time:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/kernel/vmlinux.lds.S?h=v5.17-rc1#n252
>
> For example, per `objdump -s vmlinux -j .rela.dyn`:
>
> | vmlinux: file format elf64-littleaarch64
> |
> | Contents of section .rela.dyn:
> | ffff800009beb0d0 b01a0b09 0080ffff 03040000 00000000 ................
> | ffff800009beb0e0 00600b09 0080ffff b81a0b09 0080ffff .`..............
> | ffff800009beb0f0 03040000 00000000 80710b09 0080ffff .........q......
> | ffff800009beb100 e8670b09 0080ffff 03040000 00000000 .g..............
> | ffff800009beb110 48e60809 0080ffff f0670b09 0080ffff H........g......
> | ffff800009beb120 03040000 00000000 ec190b09 0080ffff ................
>
> Each of the relocations in .rela.dyn consists of 3 64-bit little-endian
> values:
>
> * The first (e.g. 0xffff8000090b1ab0) is the VA of the pointer to write to,
> assuming the kernel's default load address (e.g. 0xffff800008000000). An
> offset must be applied to this depending on where the kernel was actually
> loaded relative to that default load address.
>
> * The second (e.g. 0x0000000000000403) is the ELF relocation type (1027, AKA
> R_AARCH64_RELATIVE).
>
> * The third, (e.g. 0xffff8000090b6000) is the VA to write to the pointer,
> assuming the kernel's default load address (e.g. 0xffff800008000000). An
> offset must be applied to this depending on where the kernel was actually
> loaded relative to that default load address.
>
> The AArch64 ELF spec defines our relocations, e.g.
>
> https://github.com/ARM-software/abi-aa/blob/main/aaelf64/aaelf64.rst#5712dynamic-relocations
>
> In general, relocations might depend on the default value that was
> initialized (e.g. OR-ing in high bits assuming the low bits are already
> correct). I'm not sure if any of the architectures we support uses such
> relocations for relocatable kernels, but they could in theory.
>
> 3) When the vmlinux mcount_loc table is sorted:
>
> The sorttable code sorts the data values within the mcount_loc table, but it
> doesn't read the relocations to resolve the VA of each entry, nor does it
> update the relocations when it swaps entries.
>
> For arm64, where entries were all initialized to zero at compile time and
> have not been updated since, the sort does nothing. When the relocations are
> applied later, the result will be an unsorted table.
>
> In general, where sorting *does* swap entries, it doesn't update the
> corresponding relocations. Where entries A and B get swapped by the sort,
> any relocation(s) for entry A will apply to the location of entry B, and
> vice-versa. Depending on the specific relocation used, that may or may not
> be a problem (e.g. the example of OR-ing in high bits would be broken).
>
> 4) When relocations are applied at boot time:
>
> On arm64, to account for KASLR or any other virtual offset we might have to
> account for, we apply the relocations early in boot in the
> __relocate_kernel() assembly function:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/kernel/head.S?h=v5.17-rc1#n753
>
> Since we didn't update the relocations during the build-time sort, and the
> mcount_loc table was not sorted to begin with, applying all the relocations
> for the mcount_loc table results in an unsorted table. Hence things go
> wrong later for any code relying on this having been sorted already.
>
> IIUC the s390 and powerpc cases are structurally similar, though the fine
> detail might differ a bit.
>
> I'm not sure how x86 works here; AFAICT the relocations are performed during
> decompression, but it looks like there's some special build-time processing
> associated with that, and the vmlinux doesn't contain standard ELF relocations.
>
> Kees, IIUC you added the x86_64 support there, can you shed any light on if/how
> this works on x86?
>
>
>
> In practice, building v5.17-rc1 with CONFIG_FTRACE_SORT_STARTUP_TEST=y I see
> the following splat:
>
> | ------------[ cut here ]------------
> | [14] unknown_bootoption+0x4/0x1c8 at ffffa1a861200738 is not sorted with trace_initcall_finish_cb+0x4/0x6c at ffffa1a85f8130b8
> | WARNING: CPU: 0 PID: 0 at kernel/trace/ftrace.c:6403 ftrace_process_locs.isra.0+0x370/0x440
> | Modules linked in:
> | CPU: 0 PID: 0 Comm: swapper Not tainted 5.17.0-rc1 #2
> | Hardware name: linux,dummy-virt (DT)
> | pstate: 600000c5 (nZCv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> | pc : ftrace_process_locs.isra.0+0x370/0x440
> | lr : ftrace_process_locs.isra.0+0x370/0x440
> | sp : ffffa1a861b23d90
> | x29: ffffa1a861b23d90 x28: 0000000000000000 x27: 0000000000000000
> | x26: 0000000000000000 x25: 0000000000000000 x24: ffffa1a8612c0008
> | x23: ffffa1a861302ea8 x22: ffffa1a861374320 x21: 0000000000000001
> | x20: 000000000000e28f x19: 000000000000000e x18: ffffffffffffffff
> | x17: 726f7320746f6e20 x16: 7369203833373030 x15: 3231363861316166
> | x14: 6666662074612038 x13: 3862303331386635 x12: 3861316166666666
> | x11: 2074612063367830 x10: ffffa1a861b4b1d0 x9 : ffffa1a861b4b1d0
> | x8 : 00000000ffffefff x7 : ffffa1a861ba31d0 x6 : ffffa1a861ba31d0
> | x5 : 000000000000bff4 x4 : 0000000000000000 x3 : 0000000000000000
> | x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffffa1a861b33e80
> | Call trace:
> | ftrace_process_locs.isra.0+0x370/0x440
> | ftrace_init+0xb4/0x15c
> | start_kernel+0x40c/0x6d4
> | __primary_switched+0xc0/0xc8
> | ---[ end trace 0000000000000000 ]---
>
> Where those symbol adddresses are:
>
> | [mark@lakrids:~/src/linux]% usekorg 11.1.0 aarch64-linux-objdump -t vmlinux | grep -w unknown_bootoption
> | ffff800009a00734 l F .init.text 00000000000001c8 unknown_bootoption
> | [mark@lakrids:~/src/linux]% usekorg 11.1.0 aarch64-linux-objdump -t vmlinux | grep -w trace_initcall_finish_cb
> | ffff8000080130b4 l F .text 0000000000000064 trace_initcall_finish_cb
>
> ... and are obviously not sorted.
>
> Further, the ftrace tests fail quite horribly, e.g.
>
> | # ./ftracetest
> | === Ftrace unit tests ===
> | [1] Basic trace file check [PASS]
> | [2] Basic test for tracers[ 38.979280] ------------[ ftrace bug ]------------
> | [ 38.980225] ftrace faulted on modifying
> | [ 38.980227] [<ffffa8ebbe6003fc>] set_reset_devices+0x8/0x24
> | [ 38.982078] Setting ftrace call site to call ftrace function
> | [ 38.983160] ftrace record flags: 80000001
> | [ 38.983921] (1)
> | [ 38.983921] expected tramp: ffffa8ebbcc2ba20
> | [ 38.985132] ------------[ cut here ]------------
> | [ 38.986013] WARNING: CPU: 3 PID: 265 at kernel/trace/ftrace.c:2068 ftrace_bug+0x284/0x2b4
> | [ 38.987649] Modules linked in:
> | [ 38.988275] CPU: 3 PID: 265 Comm: ftracetest Tainted: G W 5.17.0-rc1-dirty #4
> | [ 38.989979] Hardware name: linux,dummy-virt (DT)
> | [ 38.990916] pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> | [ 38.992318] pc : ftrace_bug+0x284/0x2b4
> | [ 38.993111] lr : ftrace_bug+0x284/0x2b4
> | [ 38.993893] sp : ffff80000888bb00
> | [ 38.994566] x29: ffff80000888bb00 x28: 0000000000000020 x27: ffffa8ebbefcaa70
> | [ 38.996002] x26: ffffa8ebbefcb7e0 x25: ffffa8ebbf36723c x24: ffffa8ebbf364000
> | [ 38.997447] x23: 00000000fffffff2 x22: ffffa8ebbe38f6e8 x21: ffffa8ebbef29c78
> | [ 38.998885] x20: ffff5ec380080030 x19: ffffa8ebbef29000 x18: ffffffffffffffff
> | [ 39.000316] x17: 0000000000000000 x16: 0000000000000000 x15: ffff80008888b827
> | [ 39.001758] x14: 0000000000000000 x13: 3032616232636362 x12: 6265386166666666
> | [ 39.003202] x11: 203a706d61727420 x10: ffffa8ebbef4b1d0 x9 : ffffa8ebbcd0dff0
> | [ 39.004645] x8 : 00000000ffffefff x7 : ffffa8ebbefa31d0 x6 : ffffa8ebbefa31d0
> | [ 39.006092] x5 : ffff5ec4befa29d8 x4 : 0000000000000000 x3 : 0000000000000000
> | [ 39.007538] x2 : 0000000000000000 x1 : ffff5ec380cf5580 x0 : 0000000000000022
> | [ 39.008982] Call trace:
> | [ 39.009495] ftrace_bug+0x284/0x2b4
> | [ 39.010212] ftrace_replace_code+0x9c/0xa4
> | [ 39.011054] ftrace_modify_all_code+0xe4/0x14c
> | [ 39.011964] arch_ftrace_update_code+0x18/0x24
> | [ 39.012874] ftrace_run_update_code+0x24/0x7c
> | [ 39.013770] ftrace_startup+0xf8/0x1b0
> | [ 39.014541] register_ftrace_graph+0x2dc/0x324
> | [ 39.015449] graph_trace_init+0x6c/0x74
> | [ 39.016232] tracing_set_tracer+0xec/0x17c
> | [ 39.017071] tracing_set_trace_write+0xe8/0x150
> | [ 39.017989] vfs_write+0xfc/0x2a0
> | [ 39.018671] ksys_write+0x74/0x100
> | [ 39.019370] __arm64_sys_write+0x28/0x3c
> | [ 39.020172] invoke_syscall+0x50/0x120
> | [ 39.020947] el0_svc_common.constprop.0+0xdc/0x100
> | [ 39.021935] do_el0_svc+0x34/0xa0
> | [ 39.022620] el0_svc+0x28/0x80
> | [ 39.023253] el0t_64_sync_handler+0xa8/0x130
> | [ 39.024121] el0t_64_sync+0x1a0/0x1a4
> | [ 39.024874] ---[ end trace 0000000000000000 ]---
> | [PASS]
> | [3] Basic trace clock test [FAIL]
> | [4] Basic event tracing check [FAIL]
> | [5] Change the ringbuffer size [FAIL]
>
> Thanks,
> Mark.
>
> >
> > More on that below.
> >
> > > >
> > > > Something like this:
> > > >
> > > > -- Steve
> > > >
> > > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > > > index c2724d986fa0..5256ebe57451 100644
> > > > --- a/arch/arm/Kconfig
> > > > +++ b/arch/arm/Kconfig
> > > > @@ -82,6 +82,7 @@ config ARM
> > > > select HAVE_EBPF_JIT if !CPU_ENDIAN_BE32
> > > > select HAVE_CONTEXT_TRACKING
> > > > select HAVE_C_RECORDMCOUNT
> > > > + select HAVE_BUILDTIME_MCOUNT_SORT
> > > > select HAVE_DEBUG_KMEMLEAK if !XIP_KERNEL
> > > > select HAVE_DMA_CONTIGUOUS if MMU
> > > > select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && !CPU_ENDIAN_BE32 && MMU
> >
> > IIUC the 32-bit arm kernel can be relocated at boot time, so I don't believe
> > this is correct, and I believe any relocatable arm kernel has been broken since
> > htat was introduced.
> >
> > > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > > > index c4207cf9bb17..7996548b2b27 100644
> > > > --- a/arch/arm64/Kconfig
> > > > +++ b/arch/arm64/Kconfig
> > > > @@ -166,6 +166,7 @@ config ARM64
> > > > select HAVE_ASM_MODVERSIONS
> > > > select HAVE_EBPF_JIT
> > > > select HAVE_C_RECORDMCOUNT
> > > > + select HAVE_BUILDTIME_MCOUNT_SORT
> > > > select HAVE_CMPXCHG_DOUBLE
> > > > select HAVE_CMPXCHG_LOCAL
> > > > select HAVE_CONTEXT_TRACKING
> >
> > The arm64 kernel is relocatable by default, and has been broken since the
> > build-time sort was introduced -- I see ftrace test failures, and the
> > CONFIG_FTRACE_SORT_STARTUP_TEST screams at boot time.
> >
> > > > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > > > index 7399327d1eff..46080dea5dba 100644
> > > > --- a/arch/x86/Kconfig
> > > > +++ b/arch/x86/Kconfig
> > > > @@ -186,6 +186,7 @@ config X86
> > > > select HAVE_CONTEXT_TRACKING_OFFSTACK if HAVE_CONTEXT_TRACKING
> > > > select HAVE_C_RECORDMCOUNT
> > > > select HAVE_OBJTOOL_MCOUNT if STACK_VALIDATION
> > > > + select HAVE_BUILDTIME_MCOUNT_SORT
> >
> > Isn't x86 relocatable in some configurations (e.g. for KASLR)?
> >
> > I can't see how the sort works for those cases, because the mcount_loc entries
> > are absolute, and either:
> >
> > * The sorted entries will get overwritten by the unsorted relocation entries,
> > and won't be sorted.
> >
> > * The sorted entries won't get overwritten, but then the absolute address will
> > be wrong since they hadn't been relocated.
> >
> > How does that work?
> >
> > Thanks,
> > Mark.
> >
> > > > select HAVE_DEBUG_KMEMLEAK
> > > > select HAVE_DMA_CONTIGUOUS
> > > > select HAVE_DYNAMIC_FTRACE
> > > > diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
> > > > index 752ed89a293b..7e5b92090faa 100644
> > > > --- a/kernel/trace/Kconfig
> > > > +++ b/kernel/trace/Kconfig
> > > > @@ -70,10 +70,16 @@ config HAVE_C_RECORDMCOUNT
> > > > help
> > > > C version of recordmcount available?
> > > > +config HAVE_BUILDTIME_MCOUNT_SORT
> > > > + bool
> > > > + help
> > > > + An architecture selects this if it sorts the mcount_loc section
> > > > + at build time.
> > > > +
> > > > config BUILDTIME_MCOUNT_SORT
> > > > bool
> > > > default y
> > > > - depends on BUILDTIME_TABLE_SORT && !S390
> > > > + depends on HAVE_BUILDTIME_MCOUNT_SORT
> > > > help
> > > > Sort the mcount_loc section at build time.
> > >
> > > LGTM. This will no longer destroy ftrace on other architectures.
> > > Those arches that we are not sure about can test and enable this function by
> > > themselves.
> > >
> > >
> > > Best regards
> > > --yinan
> > >
^ permalink raw reply
* Re: [powerpc] ftrace warning kernel/trace/ftrace.c:2068 with code-patching selftests
From: Mark Rutland @ 2022-01-27 12:20 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Kees Cook, Linux Kernel Mailing List, Steven Rostedt, Sachin Sant,
Yinan Liu, open list:LINUX FOR POWERPC (32-BIT AND 64-BIT)
In-Reply-To: <CAMj1kXHgpr0KYx5PYO_SpqaN8Ar2kfmc9Pb-d26uaYDpjwTz9w@mail.gmail.com>
On Thu, Jan 27, 2022 at 01:03:34PM +0100, Ard Biesheuvel wrote:
> On Thu, 27 Jan 2022 at 12:47, Mark Rutland <mark.rutland@arm.com> wrote:
> >
> > [adding LKML so this is easier for others to find]
> >
> > If anyone wants to follow the thread from the start, it's at:
> >
> > https://lore.kernel.org/linuxppc-dev/944D10DA-8200-4BA9-8D0A-3BED9AA99F82@linux.ibm.com/
> >
> > Ard, I was under the impression that the 32-bit arm kernel was (virtually)
> > relocatable, but I couldn't spot where, and suspect I'm mistaken. Do you know
> > whether it currently does any boot-time dynamic relocation?
>
> No, it does not.
Thanks for comfirming!
So 32-bit arm should be able to opt into the build-time sort as-is.
> > Steve asked for a bit more detail on IRC, so the below is an attempt to explain
> > what's actually going on here.
> >
> > The short answer is that relocatable kernels (e.g. those with KASLR support)
> > need to handle the kernel being loaded at (somewhat) arbitrary virtual
> > addresses. Even where code can be position-independent, any pointers in the
> > kernel image (e.g. the contents of the mcount_loc table) need to be updated to
> > account for the specific VA the kernel was loaded at -- arch code does this
> > early at boot time by applying dynamic (ELF) relocations.
>
> These architectures use place-relative extables for the same reason:
> place relative references are resolved at build time rather than at
> runtime during relocation, making a build time sort feasible.
>
> arch/alpha/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
> arch/arm64/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
> arch/ia64/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
> arch/parisc/include/asm/uaccess.h:#define ARCH_HAS_RELATIVE_EXTABLE
> arch/powerpc/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
> arch/riscv/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
> arch/s390/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
> arch/x86/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
>
> Note that the swap routine becomes something like the below, given
> that the relative references need to be fixed up after the entry
> changes place in the sorted list.
>
> static void swap_ex(void *a, void *b, int size)
> {
> struct exception_table_entry *x = a, *y = b, tmp;
> int delta = b - a;
>
> tmp = *x;
> x->insn = y->insn + delta;
> y->insn = tmp.insn - delta;
> ...
> }
>
> As a bonus, the resulting footprint of the table in the image is
> reduced by 8x, given that every 8 byte pointer has an accompanying 24
> byte RELA record, so we go from 32 bytes to 4 bytes for every call to
> __gnu_mcount_mc.
Absolutely -- it'd be great if we could do that for the callsite locations; the
difficulty is that the entries are generated by the compiler itself, so we'd
either need some build/link time processing to convert each absolute 64-bit
value to a relative 32-bit offset, or new compiler options to generate those as
relative offsets from the outset.
Thanks,
Mark.
^ permalink raw reply
* [PATCH v3] powerpc: Fix virt_addr_valid() check
From: Kefeng Wang @ 2022-01-27 12:37 UTC (permalink / raw)
To: linuxppc-dev, mpe, benh, paulus, linux-kernel
Cc: linux-mm, akpm, npiggin, Kefeng Wang
When run ethtool eth0 on PowerPC64, the BUG occurred,
usercopy: Kernel memory exposure attempt detected from SLUB object not in SLUB page?! (offset 0, size 1048)!
kernel BUG at mm/usercopy.c:99
...
usercopy_abort+0x64/0xa0 (unreliable)
__check_heap_object+0x168/0x190
__check_object_size+0x1a0/0x200
dev_ethtool+0x2494/0x2b20
dev_ioctl+0x5d0/0x770
sock_do_ioctl+0xf0/0x1d0
sock_ioctl+0x3ec/0x5a0
__se_sys_ioctl+0xf0/0x160
system_call_exception+0xfc/0x1f0
system_call_common+0xf8/0x200
The code shows below,
data = vzalloc(array_size(gstrings.len, ETH_GSTRING_LEN));
copy_to_user(useraddr, data, gstrings.len * ETH_GSTRING_LEN))
The data is alloced by vmalloc(), virt_addr_valid(ptr) will return true
on PowerPC64, which leads to the panic.
As commit 4dd7554a6456 ("powerpc/64: Add VIRTUAL_BUG_ON checks for __va
and __pa addresses") does, let's check the virt addr above PAGE_OFFSET in
the virt_addr_valid() for PowerPC64, which will make sure that the passed
address is a valid linear map address.
Meanwhile, PAGE_OFFSET is the virtual address of the start of lowmem,
the check is suitable for PowerPC32 too.
Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
v3:
- update changelog and remove a redundant cast
arch/powerpc/include/asm/page.h | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
index 254687258f42..a8a29a23ce2d 100644
--- a/arch/powerpc/include/asm/page.h
+++ b/arch/powerpc/include/asm/page.h
@@ -132,7 +132,10 @@ static inline bool pfn_valid(unsigned long pfn)
#define virt_to_page(kaddr) pfn_to_page(virt_to_pfn(kaddr))
#define pfn_to_kaddr(pfn) __va((pfn) << PAGE_SHIFT)
-#define virt_addr_valid(kaddr) pfn_valid(virt_to_pfn(kaddr))
+#define virt_addr_valid(vaddr) ({ \
+ unsigned long _addr = (unsigned long)vaddr; \
+ _addr >= PAGE_OFFSET && pfn_valid(virt_to_pfn(_addr)); \
+})
/*
* On Book-E parts we need __va to parse the device tree and we can't
--
2.26.2
^ permalink raw reply related
* Re: [powerpc] ftrace warning kernel/trace/ftrace.c:2068 with code-patching selftests
From: Ard Biesheuvel @ 2022-01-27 12:22 UTC (permalink / raw)
To: Mark Rutland
Cc: Kees Cook, Linux Kernel Mailing List, Steven Rostedt, Sachin Sant,
Yinan Liu, open list:LINUX FOR POWERPC (32-BIT AND 64-BIT)
In-Reply-To: <YfKOENgR6sLnHQmA@FVFF77S0Q05N>
On Thu, 27 Jan 2022 at 13:20, Mark Rutland <mark.rutland@arm.com> wrote:
>
> On Thu, Jan 27, 2022 at 01:03:34PM +0100, Ard Biesheuvel wrote:
> > On Thu, 27 Jan 2022 at 12:47, Mark Rutland <mark.rutland@arm.com> wrote:
> > >
> > > [adding LKML so this is easier for others to find]
> > >
> > > If anyone wants to follow the thread from the start, it's at:
> > >
> > > https://lore.kernel.org/linuxppc-dev/944D10DA-8200-4BA9-8D0A-3BED9AA99F82@linux.ibm.com/
> > >
> > > Ard, I was under the impression that the 32-bit arm kernel was (virtually)
> > > relocatable, but I couldn't spot where, and suspect I'm mistaken. Do you know
> > > whether it currently does any boot-time dynamic relocation?
> >
> > No, it does not.
>
> Thanks for comfirming!
>
> So 32-bit arm should be able to opt into the build-time sort as-is.
>
> > > Steve asked for a bit more detail on IRC, so the below is an attempt to explain
> > > what's actually going on here.
> > >
> > > The short answer is that relocatable kernels (e.g. those with KASLR support)
> > > need to handle the kernel being loaded at (somewhat) arbitrary virtual
> > > addresses. Even where code can be position-independent, any pointers in the
> > > kernel image (e.g. the contents of the mcount_loc table) need to be updated to
> > > account for the specific VA the kernel was loaded at -- arch code does this
> > > early at boot time by applying dynamic (ELF) relocations.
> >
> > These architectures use place-relative extables for the same reason:
> > place relative references are resolved at build time rather than at
> > runtime during relocation, making a build time sort feasible.
> >
> > arch/alpha/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
> > arch/arm64/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
> > arch/ia64/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
> > arch/parisc/include/asm/uaccess.h:#define ARCH_HAS_RELATIVE_EXTABLE
> > arch/powerpc/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
> > arch/riscv/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
> > arch/s390/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
> > arch/x86/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
> >
> > Note that the swap routine becomes something like the below, given
> > that the relative references need to be fixed up after the entry
> > changes place in the sorted list.
> >
> > static void swap_ex(void *a, void *b, int size)
> > {
> > struct exception_table_entry *x = a, *y = b, tmp;
> > int delta = b - a;
> >
> > tmp = *x;
> > x->insn = y->insn + delta;
> > y->insn = tmp.insn - delta;
> > ...
> > }
> >
> > As a bonus, the resulting footprint of the table in the image is
> > reduced by 8x, given that every 8 byte pointer has an accompanying 24
> > byte RELA record, so we go from 32 bytes to 4 bytes for every call to
> > __gnu_mcount_mc.
>
> Absolutely -- it'd be great if we could do that for the callsite locations; the
> difficulty is that the entries are generated by the compiler itself, so we'd
> either need some build/link time processing to convert each absolute 64-bit
> value to a relative 32-bit offset, or new compiler options to generate those as
> relative offsets from the outset.
>
Don't we use scripts/recordmcount.pl for that?
^ permalink raw reply
* Re: [powerpc] ftrace warning kernel/trace/ftrace.c:2068 with code-patching selftests
From: Mark Rutland @ 2022-01-27 12:27 UTC (permalink / raw)
To: Sven Schnelle
Cc: keescook, linux-kernel, Steven Rostedt, Sachin Sant, Yinan Liu,
linuxppc-dev, ardb
In-Reply-To: <yt9dy231gzae.fsf@linux.ibm.com>
On Thu, Jan 27, 2022 at 01:04:41PM +0100, Sven Schnelle wrote:
> Mark Rutland <mark.rutland@arm.com> writes:
>
> >> Isn't x86 relocatable in some configurations (e.g. for KASLR)?
> >>
> >> I can't see how the sort works for those cases, because the mcount_loc entries
> >> are absolute, and either:
> >>
> >> * The sorted entries will get overwritten by the unsorted relocation entries,
> >> and won't be sorted.
> >>
> >> * The sorted entries won't get overwritten, but then the absolute address will
> >> be wrong since they hadn't been relocated.
> >>
> >> How does that work?
>
> From what i've seen when looking into this ftrace sort problem x86 has a
> a relocation tool, which is run before final linking: arch/x86/tools/relocs.c
> This tools converts all the required relocations to three types:
>
> - 32 bit relocations
> - 64 bit relocations
> - inverse 32 bit relocations
>
> These are added to the end of the image.
>
> The decompressor then iterates over that array, and just adds/subtracts
> the KASLR offset - see arch/x86/boot/compressed/misc.c, handle_relocations()
>
> So IMHO x86 never uses 'real' relocations during boot, and just
> adds/subtracts. That's why the order stays the same, and the compile
> time sort works.
Ah, so those non-ELF relocations for the mcount_loc table just mean "apply the
KASLR offset here", which is equivalent for all entries.
That makes sense, thanks!
Mark.
^ 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