LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 5/7] KVM: PPC: clean up redundant kvm_run parameters in assembly
From: Tianjia Zhang @ 2020-04-22 12:58 UTC (permalink / raw)
  To: pbonzini, tsbogend, paulus, mpe, benh, borntraeger, frankja,
	david, cohuck, heiko.carstens, gor, sean.j.christopherson,
	vkuznets, wanpengli, jmattson, joro, tglx, mingo, bp, x86, hpa,
	maz, james.morse, julien.thierry.kdev, suzuki.poulose,
	christoffer.dall, peterx, thuth
  Cc: linux-s390, tianjia.zhang, kvm, linux-mips, kvm-ppc, linux-kernel,
	linuxppc-dev, kvmarm, linux-arm-kernel
In-Reply-To: <20200422125810.34847-1-tianjia.zhang@linux.alibaba.com>

In the current kvm version, 'kvm_run' has been included in the 'kvm_vcpu'
structure. Earlier than historical reasons, many kvm-related function
parameters retain the 'kvm_run' and 'kvm_vcpu' parameters at the same time.
This patch does a unified cleanup of these remaining redundant parameters.

Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
---
 arch/powerpc/include/asm/kvm_ppc.h    |  2 +-
 arch/powerpc/kvm/book3s_interrupts.S  | 17 ++++++++---------
 arch/powerpc/kvm/book3s_pr.c          |  9 ++++-----
 arch/powerpc/kvm/booke.c              |  9 ++++-----
 arch/powerpc/kvm/booke_interrupts.S   |  9 ++++-----
 arch/powerpc/kvm/bookehv_interrupts.S | 10 +++++-----
 6 files changed, 26 insertions(+), 30 deletions(-)

diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
index ccf66b3a4c1d..0a056c64c317 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -59,7 +59,7 @@ enum xlate_readwrite {
 };
 
 extern int kvmppc_vcpu_run(struct kvm_vcpu *vcpu);
-extern int __kvmppc_vcpu_run(struct kvm_run *run, struct kvm_vcpu *vcpu);
+extern int __kvmppc_vcpu_run(struct kvm_vcpu *vcpu);
 extern void kvmppc_handler_highmem(void);
 
 extern void kvmppc_dump_vcpu(struct kvm_vcpu *vcpu);
diff --git a/arch/powerpc/kvm/book3s_interrupts.S b/arch/powerpc/kvm/book3s_interrupts.S
index f7ad99d972ce..0eff749d8027 100644
--- a/arch/powerpc/kvm/book3s_interrupts.S
+++ b/arch/powerpc/kvm/book3s_interrupts.S
@@ -55,8 +55,7 @@
  ****************************************************************************/
 
 /* Registers:
- *  r3: kvm_run pointer
- *  r4: vcpu pointer
+ *  r3: vcpu pointer
  */
 _GLOBAL(__kvmppc_vcpu_run)
 
@@ -68,8 +67,8 @@ kvm_start_entry:
 	/* Save host state to the stack */
 	PPC_STLU r1, -SWITCH_FRAME_SIZE(r1)
 
-	/* Save r3 (kvm_run) and r4 (vcpu) */
-	SAVE_2GPRS(3, r1)
+	/* Save r3 (vcpu) */
+	SAVE_GPR(3, r1)
 
 	/* Save non-volatile registers (r14 - r31) */
 	SAVE_NVGPRS(r1)
@@ -82,11 +81,11 @@ kvm_start_entry:
 	PPC_STL	r0, _LINK(r1)
 
 	/* Load non-volatile guest state from the vcpu */
-	VCPU_LOAD_NVGPRS(r4)
+	VCPU_LOAD_NVGPRS(r3)
 
 kvm_start_lightweight:
 	/* Copy registers into shadow vcpu so we can access them in real mode */
-	mr	r3, r4
+	mr	r4, r3
 	bl	FUNC(kvmppc_copy_to_svcpu)
 	nop
 	REST_GPR(4, r1)
@@ -191,10 +190,10 @@ after_sprg3_load:
 	PPC_STL	r31, VCPU_GPR(R31)(r7)
 
 	/* Pass the exit number as 3rd argument to kvmppc_handle_exit */
-	lwz	r5, VCPU_TRAP(r7)
+	lwz	r4, VCPU_TRAP(r7)
 
-	/* Restore r3 (kvm_run) and r4 (vcpu) */
-	REST_2GPRS(3, r1)
+	/* Restore r3 (vcpu) */
+	REST_GPR(3, r1)
 	bl	FUNC(kvmppc_handle_exit_pr)
 
 	/* If RESUME_GUEST, get back in the loop */
diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
index ef54f917bdaf..01c8fe5abe0d 100644
--- a/arch/powerpc/kvm/book3s_pr.c
+++ b/arch/powerpc/kvm/book3s_pr.c
@@ -1151,9 +1151,9 @@ static int kvmppc_exit_pr_progint(struct kvm_vcpu *vcpu, unsigned int exit_nr)
 	return r;
 }
 
-int kvmppc_handle_exit_pr(struct kvm_run *run, struct kvm_vcpu *vcpu,
-			  unsigned int exit_nr)
+int kvmppc_handle_exit_pr(struct kvm_vcpu *vcpu, unsigned int exit_nr)
 {
+	struct kvm_run *run = vcpu->run;
 	int r = RESUME_HOST;
 	int s;
 
@@ -1826,7 +1826,6 @@ static void kvmppc_core_vcpu_free_pr(struct kvm_vcpu *vcpu)
 
 static int kvmppc_vcpu_run_pr(struct kvm_vcpu *vcpu)
 {
-	struct kvm_run *run = vcpu->run;
 	int ret;
 #ifdef CONFIG_ALTIVEC
 	unsigned long uninitialized_var(vrsave);
@@ -1834,7 +1833,7 @@ static int kvmppc_vcpu_run_pr(struct kvm_vcpu *vcpu)
 
 	/* Check if we can run the vcpu at all */
 	if (!vcpu->arch.sane) {
-		run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
+		vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
 		ret = -EINVAL;
 		goto out;
 	}
@@ -1861,7 +1860,7 @@ static int kvmppc_vcpu_run_pr(struct kvm_vcpu *vcpu)
 
 	kvmppc_fix_ee_before_entry();
 
-	ret = __kvmppc_vcpu_run(run, vcpu);
+	ret = __kvmppc_vcpu_run(vcpu);
 
 	kvmppc_clear_debug(vcpu);
 
diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index 26b3f5900b72..942039aae598 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -732,12 +732,11 @@ int kvmppc_core_check_requests(struct kvm_vcpu *vcpu)
 
 int kvmppc_vcpu_run(struct kvm_vcpu *vcpu)
 {
-	struct kvm_run *run = vcpu->run;
 	int ret, s;
 	struct debug_reg debug;
 
 	if (!vcpu->arch.sane) {
-		run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
+		vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
 		return -EINVAL;
 	}
 
@@ -779,7 +778,7 @@ int kvmppc_vcpu_run(struct kvm_vcpu *vcpu)
 	vcpu->arch.pgdir = vcpu->kvm->mm->pgd;
 	kvmppc_fix_ee_before_entry();
 
-	ret = __kvmppc_vcpu_run(run, vcpu);
+	ret = __kvmppc_vcpu_run(vcpu);
 
 	/* No need for guest_exit. It's done in handle_exit.
 	   We also get here with interrupts enabled. */
@@ -983,9 +982,9 @@ static int kvmppc_resume_inst_load(struct kvm_vcpu *vcpu,
  *
  * Return value is in the form (errcode<<2 | RESUME_FLAG_HOST | RESUME_FLAG_NV)
  */
-int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu,
-                       unsigned int exit_nr)
+int kvmppc_handle_exit(struct kvm_vcpu *vcpu, unsigned int exit_nr)
 {
+	struct kvm_run *run = vcpu->run;
 	int r = RESUME_HOST;
 	int s;
 	int idx;
diff --git a/arch/powerpc/kvm/booke_interrupts.S b/arch/powerpc/kvm/booke_interrupts.S
index 2e56ab5a5f55..6fa82efe833b 100644
--- a/arch/powerpc/kvm/booke_interrupts.S
+++ b/arch/powerpc/kvm/booke_interrupts.S
@@ -237,7 +237,7 @@ _GLOBAL(kvmppc_resume_host)
 	/* Switch to kernel stack and jump to handler. */
 	LOAD_REG_ADDR(r3, kvmppc_handle_exit)
 	mtctr	r3
-	lwz	r3, HOST_RUN(r1)
+	mr	r3, r4
 	lwz	r2, HOST_R2(r1)
 	mr	r14, r4 /* Save vcpu pointer. */
 
@@ -337,15 +337,14 @@ heavyweight_exit:
 
 
 /* Registers:
- *  r3: kvm_run pointer
- *  r4: vcpu pointer
+ *  r3: vcpu pointer
  */
 _GLOBAL(__kvmppc_vcpu_run)
 	stwu	r1, -HOST_STACK_SIZE(r1)
-	stw	r1, VCPU_HOST_STACK(r4)	/* Save stack pointer to vcpu. */
+	stw	r1, VCPU_HOST_STACK(r3)	/* Save stack pointer to vcpu. */
 
 	/* Save host state to stack. */
-	stw	r3, HOST_RUN(r1)
+	mr	r4, r3
 	mflr	r3
 	stw	r3, HOST_STACK_LR(r1)
 	mfcr	r5
diff --git a/arch/powerpc/kvm/bookehv_interrupts.S b/arch/powerpc/kvm/bookehv_interrupts.S
index c577ba4b3169..8262c14fc9e6 100644
--- a/arch/powerpc/kvm/bookehv_interrupts.S
+++ b/arch/powerpc/kvm/bookehv_interrupts.S
@@ -434,9 +434,10 @@ _GLOBAL(kvmppc_resume_host)
 #endif
 
 	/* Switch to kernel stack and jump to handler. */
-	PPC_LL	r3, HOST_RUN(r1)
+	mr	r3, r4
 	mr	r5, r14 /* intno */
 	mr	r14, r4 /* Save vcpu pointer. */
+	mr	r4, r5
 	bl	kvmppc_handle_exit
 
 	/* Restore vcpu pointer and the nonvolatiles we used. */
@@ -525,15 +526,14 @@ heavyweight_exit:
 	blr
 
 /* Registers:
- *  r3: kvm_run pointer
- *  r4: vcpu pointer
+ *  r3: vcpu pointer
  */
 _GLOBAL(__kvmppc_vcpu_run)
 	stwu	r1, -HOST_STACK_SIZE(r1)
-	PPC_STL	r1, VCPU_HOST_STACK(r4)	/* Save stack pointer to vcpu. */
+	PPC_STL	r1, VCPU_HOST_STACK(r3)	/* Save stack pointer to vcpu. */
 
 	/* Save host state to stack. */
-	PPC_STL	r3, HOST_RUN(r1)
+	mr	r4, r3
 	mflr	r3
 	mfcr	r5
 	PPC_STL	r3, HOST_STACK_LR(r1)
-- 
2.17.1


^ permalink raw reply related

* [tip: perf/core] perf expr: Add expr_scanner_ctx object
From: tip-bot2 for Jiri Olsa @ 2020-04-22 12:17 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Mark Rutland, Madhavan Srinivasan, Peter Zijlstra, Jin Yao,
	Kan Liang, Andi Kleen, x86, Alexander Shishkin, Anju T Sudhakar,
	Mamatha Inamdar, Sukadev Bhattiprolu, Ravi Bangoria, Kajol Jain,
	Arnaldo Carvalho de Melo, Joe Mario, Namhyung Kim,
	Thomas Gleixner, Michael Petlan, Greg Kroah-Hartman, LKML,
	Jiri Olsa, linuxppc-dev
In-Reply-To: <20200401203340.31402-3-kjain@linux.ibm.com>

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     871f9f599db8d9d2387c0717e712af405290edea
Gitweb:        https://git.kernel.org/tip/871f9f599db8d9d2387c0717e712af405290edea
Author:        Jiri Olsa <jolsa@kernel.org>
AuthorDate:    Thu, 02 Apr 2020 02:03:35 +05:30
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Thu, 16 Apr 2020 12:19:13 -03:00

perf expr: Add expr_scanner_ctx object

Add the expr_scanner_ctx object to hold user data for the expr scanner.
Currently it holds only start_token, Kajol Jain will use it to hold 24x7
runtime param.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Anju T Sudhakar <anju@linux.vnet.ibm.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Joe Mario <jmario@redhat.com>
Cc: Kajol Jain <kjain@linux.ibm.com>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
Cc: Mamatha Inamdar <mamatha4@linux.vnet.ibm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Michael Petlan <mpetlan@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@ozlabs.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linuxppc-dev@lists.ozlabs.org
Link: http://lore.kernel.org/lkml/20200401203340.31402-3-kjain@linux.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/expr.c |  6 ++++--
 tools/perf/util/expr.h |  4 ++++
 tools/perf/util/expr.l | 10 +++++-----
 3 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c
index c8ccc54..c3382d5 100644
--- a/tools/perf/util/expr.c
+++ b/tools/perf/util/expr.c
@@ -3,7 +3,6 @@
 #include <assert.h>
 #include "expr.h"
 #include "expr-bison.h"
-#define YY_EXTRA_TYPE int
 #include "expr-flex.h"
 
 #ifdef PARSER_DEBUG
@@ -30,11 +29,14 @@ static int
 __expr__parse(double *val, struct expr_parse_ctx *ctx, const char *expr,
 	      int start)
 {
+	struct expr_scanner_ctx scanner_ctx = {
+		.start_token = start,
+	};
 	YY_BUFFER_STATE buffer;
 	void *scanner;
 	int ret;
 
-	ret = expr_lex_init_extra(start, &scanner);
+	ret = expr_lex_init_extra(&scanner_ctx, &scanner);
 	if (ret)
 		return ret;
 
diff --git a/tools/perf/util/expr.h b/tools/perf/util/expr.h
index b9e53f2..0938ad1 100644
--- a/tools/perf/util/expr.h
+++ b/tools/perf/util/expr.h
@@ -15,6 +15,10 @@ struct expr_parse_ctx {
 	struct expr_parse_id ids[MAX_PARSE_ID];
 };
 
+struct expr_scanner_ctx {
+	int start_token;
+};
+
 void expr__ctx_init(struct expr_parse_ctx *ctx);
 void expr__add_id(struct expr_parse_ctx *ctx, const char *id, double val);
 int expr__parse(double *final_val, struct expr_parse_ctx *ctx, const char *expr);
diff --git a/tools/perf/util/expr.l b/tools/perf/util/expr.l
index eaad292..2582c24 100644
--- a/tools/perf/util/expr.l
+++ b/tools/perf/util/expr.l
@@ -76,13 +76,13 @@ sym		[0-9a-zA-Z_\.:@]+
 symbol		{spec}*{sym}*{spec}*{sym}*
 
 %%
-	{
-		int start_token;
+	struct expr_scanner_ctx *sctx = expr_get_extra(yyscanner);
 
-		start_token = expr_get_extra(yyscanner);
+	{
+		int start_token = sctx->start_token;
 
-		if (start_token) {
-			expr_set_extra(NULL, yyscanner);
+		if (sctx->start_token) {
+			sctx->start_token = 0;
 			return start_token;
 		}
 	}

^ permalink raw reply related

* [tip: perf/core] perf metrictroup: Split the metricgroup__add_metric function
From: tip-bot2 for Kajol Jain @ 2020-04-22 12:17 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Mark Rutland, Madhavan Srinivasan, Peter Zijlstra, Jin Yao,
	Jiri Olsa, Kan Liang, Andi Kleen, x86, Alexander Shishkin,
	Anju T Sudhakar, Mamatha Inamdar, Sukadev Bhattiprolu,
	Ravi Bangoria, Kajol Jain, Arnaldo Carvalho de Melo, Joe Mario,
	Namhyung Kim, Thomas Gleixner, Michael Petlan, Greg Kroah-Hartman,
	LKML, linuxppc-dev
In-Reply-To: <20200401203340.31402-4-kjain@linux.ibm.com>

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     47352aba40035ab3fdc50dd03a94456feabed7d8
Gitweb:        https://git.kernel.org/tip/47352aba40035ab3fdc50dd03a94456feabed7d8
Author:        Kajol Jain <kjain@linux.ibm.com>
AuthorDate:    Thu, 02 Apr 2020 02:03:36 +05:30
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Thu, 16 Apr 2020 12:19:13 -03:00

perf metrictroup: Split the metricgroup__add_metric function

This patch refactors metricgroup__add_metric function where some part of
it move to function metricgroup__add_metric_param.  No logic change.

Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
Acked-by: Jiri Olsa <jolsa@redhat.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Anju T Sudhakar <anju@linux.vnet.ibm.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Joe Mario <jmario@redhat.com>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
Cc: Mamatha Inamdar <mamatha4@linux.vnet.ibm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Michael Petlan <mpetlan@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@ozlabs.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linuxppc-dev@lists.ozlabs.org
Link: http://lore.kernel.org/lkml/20200401203340.31402-4-kjain@linux.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/metricgroup.c | 60 +++++++++++++++++++---------------
 1 file changed, 35 insertions(+), 25 deletions(-)

diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 926449a..7ad81c8 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -485,6 +485,39 @@ static bool metricgroup__has_constraint(struct pmu_event *pe)
 	return false;
 }
 
+static int __metricgroup__add_metric(struct strbuf *events,
+			struct list_head *group_list, struct pmu_event *pe)
+{
+
+	const char **ids;
+	int idnum;
+	struct egroup *eg;
+
+	if (expr__find_other(pe->metric_expr, NULL, &ids, &idnum) < 0)
+		return -EINVAL;
+
+	if (events->len > 0)
+		strbuf_addf(events, ",");
+
+	if (metricgroup__has_constraint(pe))
+		metricgroup__add_metric_non_group(events, ids, idnum);
+	else
+		metricgroup__add_metric_weak_group(events, ids, idnum);
+
+	eg = malloc(sizeof(*eg));
+	if (!eg)
+		return -ENOMEM;
+
+	eg->ids = ids;
+	eg->idnum = idnum;
+	eg->metric_name = pe->metric_name;
+	eg->metric_expr = pe->metric_expr;
+	eg->metric_unit = pe->unit;
+	list_add_tail(&eg->nd, group_list);
+
+	return 0;
+}
+
 static int metricgroup__add_metric(const char *metric, struct strbuf *events,
 				   struct list_head *group_list)
 {
@@ -504,35 +537,12 @@ static int metricgroup__add_metric(const char *metric, struct strbuf *events,
 			continue;
 		if (match_metric(pe->metric_group, metric) ||
 		    match_metric(pe->metric_name, metric)) {
-			const char **ids;
-			int idnum;
-			struct egroup *eg;
 
 			pr_debug("metric expr %s for %s\n", pe->metric_expr, pe->metric_name);
 
-			if (expr__find_other(pe->metric_expr,
-					     NULL, &ids, &idnum) < 0)
-				continue;
-			if (events->len > 0)
-				strbuf_addf(events, ",");
-
-			if (metricgroup__has_constraint(pe))
-				metricgroup__add_metric_non_group(events, ids, idnum);
-			else
-				metricgroup__add_metric_weak_group(events, ids, idnum);
-
-			eg = malloc(sizeof(struct egroup));
-			if (!eg) {
-				ret = -ENOMEM;
+			ret = __metricgroup__add_metric(events,	group_list, pe);
+			if (ret == -ENOMEM)
 				break;
-			}
-			eg->ids = ids;
-			eg->idnum = idnum;
-			eg->metric_name = pe->metric_name;
-			eg->metric_expr = pe->metric_expr;
-			eg->metric_unit = pe->unit;
-			list_add_tail(&eg->nd, group_list);
-			ret = 0;
 		}
 	}
 	return ret;

^ permalink raw reply related

* [tip: perf/core] perf expr: Add expr_ prefix for parse_ctx and parse_id
From: tip-bot2 for Jiri Olsa @ 2020-04-22 12:17 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Mark Rutland, Madhavan Srinivasan, Peter Zijlstra, Jin Yao,
	Kan Liang, Andi Kleen, x86, Alexander Shishkin, Anju T Sudhakar,
	Mamatha Inamdar, Sukadev Bhattiprolu, Ravi Bangoria, Kajol Jain,
	Arnaldo Carvalho de Melo, Joe Mario, Namhyung Kim,
	Thomas Gleixner, Michael Petlan, Greg Kroah-Hartman, LKML,
	Jiri Olsa, linuxppc-dev
In-Reply-To: <20200401203340.31402-2-kjain@linux.ibm.com>

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     aecce63e2b98f28606b063949cca06facf215d6c
Gitweb:        https://git.kernel.org/tip/aecce63e2b98f28606b063949cca06facf215d6c
Author:        Jiri Olsa <jolsa@kernel.org>
AuthorDate:    Thu, 02 Apr 2020 02:03:34 +05:30
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Thu, 16 Apr 2020 12:19:13 -03:00

perf expr: Add expr_ prefix for parse_ctx and parse_id

Adding expr_ prefix for parse_ctx and parse_id, to straighten out the
expr* namespace.

There's no functional change.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Anju T Sudhakar <anju@linux.vnet.ibm.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Joe Mario <jmario@redhat.com>
Cc: Kajol Jain <kjain@linux.ibm.com>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
Cc: Mamatha Inamdar <mamatha4@linux.vnet.ibm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Michael Petlan <mpetlan@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@ozlabs.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linuxppc-dev@lists.ozlabs.org
Link: http://lore.kernel.org/lkml/20200401203340.31402-2-kjain@linux.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/tests/expr.c       |  4 ++--
 tools/perf/util/expr.c        | 10 +++++-----
 tools/perf/util/expr.h        | 12 ++++++------
 tools/perf/util/expr.y        |  6 +++---
 tools/perf/util/stat-shadow.c |  2 +-
 5 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/tools/perf/tests/expr.c b/tools/perf/tests/expr.c
index 28313e5..ea10fc4 100644
--- a/tools/perf/tests/expr.c
+++ b/tools/perf/tests/expr.c
@@ -6,7 +6,7 @@
 #include <string.h>
 #include <linux/zalloc.h>
 
-static int test(struct parse_ctx *ctx, const char *e, double val2)
+static int test(struct expr_parse_ctx *ctx, const char *e, double val2)
 {
 	double val;
 
@@ -22,7 +22,7 @@ int test__expr(struct test *t __maybe_unused, int subtest __maybe_unused)
 	const char **other;
 	double val;
 	int i, ret;
-	struct parse_ctx ctx;
+	struct expr_parse_ctx ctx;
 	int num_other;
 
 	expr__ctx_init(&ctx);
diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c
index fd192dd..c8ccc54 100644
--- a/tools/perf/util/expr.c
+++ b/tools/perf/util/expr.c
@@ -11,7 +11,7 @@ extern int expr_debug;
 #endif
 
 /* Caller must make sure id is allocated */
-void expr__add_id(struct parse_ctx *ctx, const char *name, double val)
+void expr__add_id(struct expr_parse_ctx *ctx, const char *name, double val)
 {
 	int idx;
 
@@ -21,13 +21,13 @@ void expr__add_id(struct parse_ctx *ctx, const char *name, double val)
 	ctx->ids[idx].val = val;
 }
 
-void expr__ctx_init(struct parse_ctx *ctx)
+void expr__ctx_init(struct expr_parse_ctx *ctx)
 {
 	ctx->num_ids = 0;
 }
 
 static int
-__expr__parse(double *val, struct parse_ctx *ctx, const char *expr,
+__expr__parse(double *val, struct expr_parse_ctx *ctx, const char *expr,
 	      int start)
 {
 	YY_BUFFER_STATE buffer;
@@ -52,7 +52,7 @@ __expr__parse(double *val, struct parse_ctx *ctx, const char *expr,
 	return ret;
 }
 
-int expr__parse(double *final_val, struct parse_ctx *ctx, const char *expr)
+int expr__parse(double *final_val, struct expr_parse_ctx *ctx, const char *expr)
 {
 	return __expr__parse(final_val, ctx, expr, EXPR_PARSE) ? -1 : 0;
 }
@@ -75,7 +75,7 @@ int expr__find_other(const char *expr, const char *one, const char ***other,
 		     int *num_other)
 {
 	int err, i = 0, j = 0;
-	struct parse_ctx ctx;
+	struct expr_parse_ctx ctx;
 
 	expr__ctx_init(&ctx);
 	err = __expr__parse(NULL, &ctx, expr, EXPR_OTHER);
diff --git a/tools/perf/util/expr.h b/tools/perf/util/expr.h
index 9377538..b9e53f2 100644
--- a/tools/perf/util/expr.h
+++ b/tools/perf/util/expr.h
@@ -5,19 +5,19 @@
 #define EXPR_MAX_OTHER 20
 #define MAX_PARSE_ID EXPR_MAX_OTHER
 
-struct parse_id {
+struct expr_parse_id {
 	const char *name;
 	double val;
 };
 
-struct parse_ctx {
+struct expr_parse_ctx {
 	int num_ids;
-	struct parse_id ids[MAX_PARSE_ID];
+	struct expr_parse_id ids[MAX_PARSE_ID];
 };
 
-void expr__ctx_init(struct parse_ctx *ctx);
-void expr__add_id(struct parse_ctx *ctx, const char *id, double val);
-int expr__parse(double *final_val, struct parse_ctx *ctx, const char *expr);
+void expr__ctx_init(struct expr_parse_ctx *ctx);
+void expr__add_id(struct expr_parse_ctx *ctx, const char *id, double val);
+int expr__parse(double *final_val, struct expr_parse_ctx *ctx, const char *expr);
 int expr__find_other(const char *expr, const char *one, const char ***other,
 		int *num_other);
 
diff --git a/tools/perf/util/expr.y b/tools/perf/util/expr.y
index 4720cbe..cd17486 100644
--- a/tools/perf/util/expr.y
+++ b/tools/perf/util/expr.y
@@ -15,7 +15,7 @@
 %define api.pure full
 
 %parse-param { double *final_val }
-%parse-param { struct parse_ctx *ctx }
+%parse-param { struct expr_parse_ctx *ctx }
 %parse-param {void *scanner}
 %lex-param {void* scanner}
 
@@ -39,14 +39,14 @@
 
 %{
 static void expr_error(double *final_val __maybe_unused,
-		       struct parse_ctx *ctx __maybe_unused,
+		       struct expr_parse_ctx *ctx __maybe_unused,
 		       void *scanner,
 		       const char *s)
 {
 	pr_debug("%s\n", s);
 }
 
-static int lookup_id(struct parse_ctx *ctx, char *id, double *val)
+static int lookup_id(struct expr_parse_ctx *ctx, char *id, double *val)
 {
 	int i;
 
diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c
index 03ecb8c..1ad5c5b 100644
--- a/tools/perf/util/stat-shadow.c
+++ b/tools/perf/util/stat-shadow.c
@@ -729,7 +729,7 @@ static void generic_metric(struct perf_stat_config *config,
 			   struct runtime_stat *st)
 {
 	print_metric_t print_metric = out->print_metric;
-	struct parse_ctx pctx;
+	struct expr_parse_ctx pctx;
 	double ratio, scale;
 	int i;
 	void *ctxp = out->ctx;

^ permalink raw reply related

* Re: ppc64 early slub caches have zero random value
From: Vlastimil Babka @ 2020-04-22 11:13 UTC (permalink / raw)
  To: Nicolai Stange
  Cc: Jason A. Donenfeld, Theodore Ts'o, Kees Cook, linux-kernel,
	linux-mm, Andy Lutomirski, Michal Suchánek, linuxppc-dev
In-Reply-To: <871rohz0zk.fsf@suse.de>

On 4/21/20 10:39 AM, Nicolai Stange wrote:
> Hi
> 
> [adding some drivers/char/random folks + LKML to CC]
> 
> Vlastimil Babka <vbabka@suse.cz> writes:
> 
>> On 4/17/20 6:53 PM, Michal Suchánek wrote:
>>> Hello,
>>
>> Hi, thanks for reproducing on latest upstream!
>>
>>> instrumenting the kernel with the following patch
>>> 
>>> ---
>>>  mm/slub.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>> 
>>> diff --git a/mm/slub.c b/mm/slub.c
>>> index d6787bbe0248..d40995d5f8ff 100644
>>> --- a/mm/slub.c
>>> +++ b/mm/slub.c
>>> @@ -3633,6 +3633,7 @@ static int kmem_cache_open(struct kmem_cache *s, slab_flags_t flags)
>>>  	s->flags = kmem_cache_flags(s->size, flags, s->name, s->ctor);
>>>  #ifdef CONFIG_SLAB_FREELIST_HARDENED
>>>  	s->random = get_random_long();
>>> +	pr_notice("Creating cache %s with s->random=%ld\n", s->name, s->random);
>>>  #endif
>>>  
>>>  	if (!calculate_sizes(s, -1))
>>> 
>>> I get:
>>> 
>>> [    0.000000] random: get_random_u64 called from kmem_cache_open+0x3c/0x5b0
>> with crng_init=0
>>> [    0.000000] Creating cache kmem_cache_node with s->random=0
>>> [    0.000000] Creating cache kmem_cache with s->random=0
>>> [    0.000000] Creating cache kmalloc-8 with s->random=0
>>> [    0.000000] Creating cache kmalloc-16 with s->random=0
>>> [    0.000000] Creating cache kmalloc-32 with s->random=0
>>> [    0.000000] Creating cache kmalloc-64 with s->random=0
>>> [    0.000000] Creating cache kmalloc-96 with s->random=0
>>> [    0.000000] Creating cache kmalloc-128 with s->random=0
>>> [    0.000000] Creating cache kmalloc-192 with s->random=-682532147323126958
>>> 
>>> The earliest caches created invariably end up with s->random of zero.

I have also realized that the rest of the early created caches is initialized
with albeit non-zero, but in fact deterministically same same sequence values.

>> It seems that reliably it's the first 8 calls get_random_u64(), which sounds
>> more like some off-by-X bug than a genuine lack entropy that would become fixed
>> in the meanwhile?
>>
>>> This is a problem for crash which does not recognize these as randomized
>>> and fails to read them. While this can be addressed in crash is it
>>> intended to create caches with zero random value in the kernel?
>>
>> Definitely not. The question is more likely what guarantees we have with
>> crng_init=0. Probably we can't expect cryptographically strong randomness, but
>> zeroes still do look like a bug to me?
>>
>>> This is broken at least in the 5.4~5.7 range but it is not clear if this
>>> ever worked. All examples of earlier kernels I have at hand use slab mm.
>>> 
>>> Thanks
>>> 
>>> Michal
>>>
> 
> FWIW, I've seen something similar in a slightly different context,
> c.f. [1].
> 
> Basically, the issue is that on anything but x86_64 (and perhaps arm64
> IIRC), arch_get_random_long() is unavailable and thus, get_random_u64()
> falls through to that batched extract_crng() extraction. That is, it
> extracts eight random longs from the chacha20 based RNG at once and
> batches them up for consumption by the current and subsequent
> get_random_u64() invocations. Which is in line with your observation
> that get_random_u64() returned zero exactly eight times in a row.
> 
> The fact that extract_crng() actually extracted eight successive zero
> values surprised me though. But from looking at chacha20_block(), called
> from _extract_crng() with the primary_crng instance's state buffer as
> input, it seems like a zeroed state buffer gets identity transformed and
> that all this fancy shifting and rolling and whatnot in chacha_permute()
> would have no effect at all. So I suppose that the primary_crng's state
> buffer is still zeroed out at that point during boot.

Looks so, thanks for explanation. So there's simply no entropy and the kmalloc-X
caches have deterministic s->random. Zeroes are just more obvious and may fail
e.g. to prevent detection for random corruptions (so we should still fix
those?), while for an attacker anything predictable is bad, and I don't know
what to do about it. If we were to e.g. reseed the early created kmalloc caches
later where crng is fully inited, we would have to basically "stop the world"
and rewrite existing freelists.

> Thanks,
> 
> Nicolai
> 
> [1] https://lkml.kernel.org/r/87d08rbbg9.fsf@suse.de
> 


^ permalink raw reply

* Re: [PATCH v3 3/4] hugetlbfs: remove hugetlb_add_hstate() warning for existing hstate
From: Aneesh Kumar K.V @ 2020-04-22 10:42 UTC (permalink / raw)
  To: Mike Kravetz, linux-mm, linux-kernel, linux-arm-kernel,
	linuxppc-dev, linux-riscv, linux-s390, sparclinux, linux-doc
  Cc: Dave Hansen, Heiko Carstens, Peter Xu, Paul Mackerras,
	Will Deacon, Mina Almasry, Jonathan Corbet, Christian Borntraeger,
	Ingo Molnar, Catalin Marinas, Longpeng, Albert Ou, Vasily Gorbik,
	Paul Walmsley, Thomas Gleixner, Nitesh Narayan Lal, Randy Dunlap,
	Palmer Dabbelt, Andrew Morton, David S . Miller, Mike Kravetz
In-Reply-To: <20200417185049.275845-4-mike.kravetz@oracle.com>

Mike Kravetz <mike.kravetz@oracle.com> writes:

> The routine hugetlb_add_hstate prints a warning if the hstate already
> exists.  This was originally done as part of kernel command line
> parsing.  If 'hugepagesz=' was specified more than once, the warning
> 	pr_warn("hugepagesz= specified twice, ignoring\n");
> would be printed.
>
> Some architectures want to enable all huge page sizes.  They would
> call hugetlb_add_hstate for all supported sizes.  However, this was
> done after command line processing and as a result hstates could have
> already been created for some sizes.  To make sure no warning were
> printed, there would often be code like:
> 	if (!size_to_hstate(size)
> 		hugetlb_add_hstate(ilog2(size) - PAGE_SHIFT)
>
> The only time we want to print the warning is as the result of command
> line processing.

Does this patch break hugepages=x command line? I haven't tested this
yet. But one of the details w.r.t. skipping that hugetlb_add_hstate is
to make sure we can configure the max_huge_pages. 


>So, remove the warning from hugetlb_add_hstate and
> add it to the single arch independent routine processing "hugepagesz=".
> After this, calls to size_to_hstate() in arch specific code can be
> removed and hugetlb_add_hstate can be called without worrying about
> warning messages.
>

-aneesh

^ permalink raw reply

* Re: [PATCH 1/3] kexec: Prevent removal of memory in use by a loaded kexec image
From: Baoquan He @ 2020-04-22 10:36 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: piliu, Anshuman Khandual, Catalin Marinas, Bhupesh Sharma,
	linuxppc-dev, kexec, Russell King - ARM Linux admin, linux-mm,
	James Morse, Eric W. Biederman, Andrew Morton, Will Deacon,
	linux-arm-kernel
In-Reply-To: <b0e7b7e8-01d0-e03e-83a8-4763ccbede43@redhat.com>

On 04/22/20 at 12:05pm, David Hildenbrand wrote:
> On 22.04.20 11:57, Baoquan He wrote:
> > On 04/22/20 at 11:24am, David Hildenbrand wrote:
> >> On 22.04.20 11:17, Baoquan He wrote:
> >>> On 04/21/20 at 03:29pm, David Hildenbrand wrote:
> >>>>>> ACPI SRAT is embeded into efi, need read out the rsdp pointer. If we don't
> >>>>>> pass the efi, it won't get the SRAT table correctly, if I remember
> >>>>>> correctly. Yeah, I remeber kvm guest can get memory hotplugged with
> >>>>>> ACPI only, this won't happen on bare metal though. Need check carefully. 
> >>>>>> I have been using kvm guest with uefi firmwire recently.
> >>>>>
> >>>>> Yeah, I can imagine that bare metal is different. kvm only uses ACPI.
> >>>>>
> >>>>> I'm also asking because of virtio-mem. Memory added via virtio-mem is
> >>>>> not part of any efi tables or whatsoever. So I assume the kexec kernel
> >>>>> will not detect it automatically (good!), instead load the virtio-mem
> >>>>> driver and let it add memory back to the system.
> >>>>>
> >>>>> I should probably play with kexec and virtio-mem once I have some spare
> >>>>> cycles ... to find out what's broken and needs to be addressed :)
> >>>>
> >>>> FWIW, I just gave virtio-mem and kexec/kdump a try.
> >>>>
> >>>> a) kdump seems to work. Memory added by virtio-mem is getting dumped.
> >>>> The kexec kernel only uses memory in the crash region. The virtio-mem
> >>>> driver properly bails out due to is_kdump_kernel().
> >>>
> >>> Right, kdump is not impacted later added memory.
> >>>
> >>>>
> >>>> b) "kexec -s -l" seems to work fine. For now, the kernel does not seem
> >>>> to get placed on virtio-mem memory (pure luck due to the left-to-right
> >>>> search). Memory added by virtio-mem is not getting added to the e820
> >>>> map. Once the virtio-mem driver comes back up in the kexec kernel, the
> >>>> right memory is readded.
> >>>
> >>> kexec_file_load just behaves as you tested. It doesn't collect later
> >>> added memory to e820 because it uses e820_table_kexec directly to pass
> >>> e820 to kexec-ed kernel. However, this e820_table_kexec is only updated
> >>> during boot stage. I tried hot adding DIMM after boot, kexec-ed kernel
> >>> doesn't have it in e820 during bootup, but it's recoginized and added
> >>> when ACPI scanning. I think we should update e820_table_kexec when hot
> >>> add/remove memory, at least for DIMM. Not sure if DLPAR, virtio-mem,
> >>> balloon will need be added into e820_table_kexec too, and if this is
> >>> expected behaviour.
> >>>
> >>> But whatever we do, it won't impact the kexec file_loading, because of
> >>> the searching strategy bottom up. Just adding them into e820_table_kexec
> >>> will make it consistent with cold reboot which get recognizes and get
> >>> them into e820 during bootup.
> >>
> >> Yeah, I think whatever a cold-booted kernel will see is what kexec-ed
> >> kernel should see. Not more, not less.
> >>
> >> Regarding virtio-mem: Not in e820 on cold-boot.
> >> Regarding DIMMs: DIMMs under KVM will never show up in the e820 map
> >> IIRC. I think on real HW it can be different.
> > 
> > Yeah, DIMMs under KVM won't show up in e820 map. While this is not feature
> > of QEMU/KVM, but a defect of it. I ever asked Igor who is developer of
> > QEMU/KVM guest in this area, why we don't make kvm guest recognize
> > hotpluggable DIMM and add it into e820 map, he said he had tried to make
> > it, but this will corrupt guest on HyperV. So he had to revert the
> 
> Yeah, I remember that this had to be reverted due to something breaking.
> But OTOH, it allows us to online coldplugged DIMMs online_movable
> easily, so I'd say it's even a feature (although, does not behave like
> real HW we have).
> 
> I use this extensively when testing memory hot(un)plug via coldplugged
> DIMMs.
> 
> I do wonder if there is real HW, where this is also the case.

None for what I know. Hotplug on real HW includes two parts, the boot
mem being hotpluggable is more flexiable one. It allows people to
replace bad DIMM. And you can see code in boot stage has been adjusted a
lot on this purpose, at that time, people haven't thought about kvm
guest.

> 
> > commit on qemu. So I think we can leave it for now for both real HW and
> > kvm, or update the e820_table_kexec to include added DIMM for both real
> > HW and KVM. I hope one day KVM dev will find a way to conquer the defect
> > on HyperV and make the e820map consistent with bare metal. After all,
> > kvm guest is trying to imitate real HW for the most part.
> > 
> > Anyway, I will think about the e820_table_kexec updating. See if we can
> > do something about it.
> 
> Yeah, for DIMMs on real HW it might definitely make sense. We might be
> able to hook into updates of /sys/firmware/memmap on memory add/remove.


^ permalink raw reply

* [PATCH 5.6 076/166] KVM: PPC: Book3S HV: Fix H_CEDE return code for nested guests
From: Greg Kroah-Hartman @ 2020-04-22  9:56 UTC (permalink / raw)
  To: linux-kernel
  Cc: Sasha Levin, Greg Kroah-Hartman, Michael Roth, stable,
	linuxppc-dev, David Gibson
In-Reply-To: <20200422095047.669225321@linuxfoundation.org>

From: Michael Roth <mdroth@linux.vnet.ibm.com>

[ Upstream commit 1f50cc1705350a4697923203fedd7d8fb1087fe2 ]

The h_cede_tm kvm-unit-test currently fails when run inside an L1 guest
via the guest/nested hypervisor.

  ./run-tests.sh -v
  ...
  TESTNAME=h_cede_tm TIMEOUT=90s ACCEL= ./powerpc/run powerpc/tm.elf -smp 2,threads=2 -machine cap-htm=on -append "h_cede_tm"
  FAIL h_cede_tm (2 tests, 1 unexpected failures)

While the test relates to transactional memory instructions, the actual
failure is due to the return code of the H_CEDE hypercall, which is
reported as 224 instead of 0. This happens even when no TM instructions
are issued.

224 is the value placed in r3 to execute a hypercall for H_CEDE, and r3
is where the caller expects the return code to be placed upon return.

In the case of guest running under a nested hypervisor, issuing H_CEDE
causes a return from H_ENTER_NESTED. In this case H_CEDE is
specially-handled immediately rather than later in
kvmppc_pseries_do_hcall() as with most other hcalls, but we forget to
set the return code for the caller, hence why kvm-unit-test sees the
224 return code and reports an error.

Guest kernels generally don't check the return value of H_CEDE, so
that likely explains why this hasn't caused issues outside of
kvm-unit-tests so far.

Fix this by setting r3 to 0 after we finish processing the H_CEDE.

RHBZ: 1778556

Fixes: 4bad77799fed ("KVM: PPC: Book3S HV: Handle hypercalls correctly when nested")
Cc: linuxppc-dev@ozlabs.org
Cc: David Gibson <david@gibson.dropbear.id.au>
Cc: Paul Mackerras <paulus@ozlabs.org>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Paul Mackerras <paulus@ozlabs.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 arch/powerpc/kvm/book3s_hv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 2cefd071b8483..c0c43a7338304 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -3616,6 +3616,7 @@ int kvmhv_p9_guest_entry(struct kvm_vcpu *vcpu, u64 time_limit,
 		if (trap == BOOK3S_INTERRUPT_SYSCALL && !vcpu->arch.nested &&
 		    kvmppc_get_gpr(vcpu, 3) == H_CEDE) {
 			kvmppc_nested_cede(vcpu);
+			kvmppc_set_gpr(vcpu, 3, 0);
 			trap = 0;
 		}
 	} else {
-- 
2.20.1




^ permalink raw reply related

* [PATCH 5.4 054/118] KVM: PPC: Book3S HV: Fix H_CEDE return code for nested guests
From: Greg Kroah-Hartman @ 2020-04-22  9:56 UTC (permalink / raw)
  To: linux-kernel
  Cc: Sasha Levin, Greg Kroah-Hartman, Michael Roth, stable,
	linuxppc-dev, David Gibson
In-Reply-To: <20200422095031.522502705@linuxfoundation.org>

From: Michael Roth <mdroth@linux.vnet.ibm.com>

[ Upstream commit 1f50cc1705350a4697923203fedd7d8fb1087fe2 ]

The h_cede_tm kvm-unit-test currently fails when run inside an L1 guest
via the guest/nested hypervisor.

  ./run-tests.sh -v
  ...
  TESTNAME=h_cede_tm TIMEOUT=90s ACCEL= ./powerpc/run powerpc/tm.elf -smp 2,threads=2 -machine cap-htm=on -append "h_cede_tm"
  FAIL h_cede_tm (2 tests, 1 unexpected failures)

While the test relates to transactional memory instructions, the actual
failure is due to the return code of the H_CEDE hypercall, which is
reported as 224 instead of 0. This happens even when no TM instructions
are issued.

224 is the value placed in r3 to execute a hypercall for H_CEDE, and r3
is where the caller expects the return code to be placed upon return.

In the case of guest running under a nested hypervisor, issuing H_CEDE
causes a return from H_ENTER_NESTED. In this case H_CEDE is
specially-handled immediately rather than later in
kvmppc_pseries_do_hcall() as with most other hcalls, but we forget to
set the return code for the caller, hence why kvm-unit-test sees the
224 return code and reports an error.

Guest kernels generally don't check the return value of H_CEDE, so
that likely explains why this hasn't caused issues outside of
kvm-unit-tests so far.

Fix this by setting r3 to 0 after we finish processing the H_CEDE.

RHBZ: 1778556

Fixes: 4bad77799fed ("KVM: PPC: Book3S HV: Handle hypercalls correctly when nested")
Cc: linuxppc-dev@ozlabs.org
Cc: David Gibson <david@gibson.dropbear.id.au>
Cc: Paul Mackerras <paulus@ozlabs.org>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Paul Mackerras <paulus@ozlabs.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 arch/powerpc/kvm/book3s_hv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 36abbe3c346df..e2183fed947d4 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -3623,6 +3623,7 @@ int kvmhv_p9_guest_entry(struct kvm_vcpu *vcpu, u64 time_limit,
 		if (trap == BOOK3S_INTERRUPT_SYSCALL && !vcpu->arch.nested &&
 		    kvmppc_get_gpr(vcpu, 3) == H_CEDE) {
 			kvmppc_nested_cede(vcpu);
+			kvmppc_set_gpr(vcpu, 3, 0);
 			trap = 0;
 		}
 	} else {
-- 
2.20.1




^ permalink raw reply related

* Re: [PATCH 1/3] kexec: Prevent removal of memory in use by a loaded kexec image
From: David Hildenbrand @ 2020-04-22 10:05 UTC (permalink / raw)
  To: Baoquan He
  Cc: piliu, Anshuman Khandual, Catalin Marinas, Bhupesh Sharma,
	linuxppc-dev, kexec, Russell King - ARM Linux admin, linux-mm,
	James Morse, Eric W. Biederman, Andrew Morton, Will Deacon,
	linux-arm-kernel
In-Reply-To: <20200422095733.GU4247@MiWiFi-R3L-srv>

On 22.04.20 11:57, Baoquan He wrote:
> On 04/22/20 at 11:24am, David Hildenbrand wrote:
>> On 22.04.20 11:17, Baoquan He wrote:
>>> On 04/21/20 at 03:29pm, David Hildenbrand wrote:
>>>>>> ACPI SRAT is embeded into efi, need read out the rsdp pointer. If we don't
>>>>>> pass the efi, it won't get the SRAT table correctly, if I remember
>>>>>> correctly. Yeah, I remeber kvm guest can get memory hotplugged with
>>>>>> ACPI only, this won't happen on bare metal though. Need check carefully. 
>>>>>> I have been using kvm guest with uefi firmwire recently.
>>>>>
>>>>> Yeah, I can imagine that bare metal is different. kvm only uses ACPI.
>>>>>
>>>>> I'm also asking because of virtio-mem. Memory added via virtio-mem is
>>>>> not part of any efi tables or whatsoever. So I assume the kexec kernel
>>>>> will not detect it automatically (good!), instead load the virtio-mem
>>>>> driver and let it add memory back to the system.
>>>>>
>>>>> I should probably play with kexec and virtio-mem once I have some spare
>>>>> cycles ... to find out what's broken and needs to be addressed :)
>>>>
>>>> FWIW, I just gave virtio-mem and kexec/kdump a try.
>>>>
>>>> a) kdump seems to work. Memory added by virtio-mem is getting dumped.
>>>> The kexec kernel only uses memory in the crash region. The virtio-mem
>>>> driver properly bails out due to is_kdump_kernel().
>>>
>>> Right, kdump is not impacted later added memory.
>>>
>>>>
>>>> b) "kexec -s -l" seems to work fine. For now, the kernel does not seem
>>>> to get placed on virtio-mem memory (pure luck due to the left-to-right
>>>> search). Memory added by virtio-mem is not getting added to the e820
>>>> map. Once the virtio-mem driver comes back up in the kexec kernel, the
>>>> right memory is readded.
>>>
>>> kexec_file_load just behaves as you tested. It doesn't collect later
>>> added memory to e820 because it uses e820_table_kexec directly to pass
>>> e820 to kexec-ed kernel. However, this e820_table_kexec is only updated
>>> during boot stage. I tried hot adding DIMM after boot, kexec-ed kernel
>>> doesn't have it in e820 during bootup, but it's recoginized and added
>>> when ACPI scanning. I think we should update e820_table_kexec when hot
>>> add/remove memory, at least for DIMM. Not sure if DLPAR, virtio-mem,
>>> balloon will need be added into e820_table_kexec too, and if this is
>>> expected behaviour.
>>>
>>> But whatever we do, it won't impact the kexec file_loading, because of
>>> the searching strategy bottom up. Just adding them into e820_table_kexec
>>> will make it consistent with cold reboot which get recognizes and get
>>> them into e820 during bootup.
>>
>> Yeah, I think whatever a cold-booted kernel will see is what kexec-ed
>> kernel should see. Not more, not less.
>>
>> Regarding virtio-mem: Not in e820 on cold-boot.
>> Regarding DIMMs: DIMMs under KVM will never show up in the e820 map
>> IIRC. I think on real HW it can be different.
> 
> Yeah, DIMMs under KVM won't show up in e820 map. While this is not feature
> of QEMU/KVM, but a defect of it. I ever asked Igor who is developer of
> QEMU/KVM guest in this area, why we don't make kvm guest recognize
> hotpluggable DIMM and add it into e820 map, he said he had tried to make
> it, but this will corrupt guest on HyperV. So he had to revert the

Yeah, I remember that this had to be reverted due to something breaking.
But OTOH, it allows us to online coldplugged DIMMs online_movable
easily, so I'd say it's even a feature (although, does not behave like
real HW we have).

I use this extensively when testing memory hot(un)plug via coldplugged
DIMMs.

I do wonder if there is real HW, where this is also the case.

> commit on qemu. So I think we can leave it for now for both real HW and
> kvm, or update the e820_table_kexec to include added DIMM for both real
> HW and KVM. I hope one day KVM dev will find a way to conquer the defect
> on HyperV and make the e820map consistent with bare metal. After all,
> kvm guest is trying to imitate real HW for the most part.
> 
> Anyway, I will think about the e820_table_kexec updating. See if we can
> do something about it.

Yeah, for DIMMs on real HW it might definitely make sense. We might be
able to hook into updates of /sys/firmware/memmap on memory add/remove.

-- 
Thanks,

David / dhildenb


^ permalink raw reply

* Re: [PATCH 1/3] kexec: Prevent removal of memory in use by a loaded kexec image
From: Baoquan He @ 2020-04-22  9:57 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: piliu, Anshuman Khandual, Catalin Marinas, Bhupesh Sharma,
	linuxppc-dev, kexec, Russell King - ARM Linux admin, linux-mm,
	James Morse, Eric W. Biederman, Andrew Morton, Will Deacon,
	linux-arm-kernel
In-Reply-To: <b9173744-6be4-5b41-d205-319542d821ba@redhat.com>

On 04/22/20 at 11:24am, David Hildenbrand wrote:
> On 22.04.20 11:17, Baoquan He wrote:
> > On 04/21/20 at 03:29pm, David Hildenbrand wrote:
> >>>> ACPI SRAT is embeded into efi, need read out the rsdp pointer. If we don't
> >>>> pass the efi, it won't get the SRAT table correctly, if I remember
> >>>> correctly. Yeah, I remeber kvm guest can get memory hotplugged with
> >>>> ACPI only, this won't happen on bare metal though. Need check carefully. 
> >>>> I have been using kvm guest with uefi firmwire recently.
> >>>
> >>> Yeah, I can imagine that bare metal is different. kvm only uses ACPI.
> >>>
> >>> I'm also asking because of virtio-mem. Memory added via virtio-mem is
> >>> not part of any efi tables or whatsoever. So I assume the kexec kernel
> >>> will not detect it automatically (good!), instead load the virtio-mem
> >>> driver and let it add memory back to the system.
> >>>
> >>> I should probably play with kexec and virtio-mem once I have some spare
> >>> cycles ... to find out what's broken and needs to be addressed :)
> >>
> >> FWIW, I just gave virtio-mem and kexec/kdump a try.
> >>
> >> a) kdump seems to work. Memory added by virtio-mem is getting dumped.
> >> The kexec kernel only uses memory in the crash region. The virtio-mem
> >> driver properly bails out due to is_kdump_kernel().
> > 
> > Right, kdump is not impacted later added memory.
> > 
> >>
> >> b) "kexec -s -l" seems to work fine. For now, the kernel does not seem
> >> to get placed on virtio-mem memory (pure luck due to the left-to-right
> >> search). Memory added by virtio-mem is not getting added to the e820
> >> map. Once the virtio-mem driver comes back up in the kexec kernel, the
> >> right memory is readded.
> > 
> > kexec_file_load just behaves as you tested. It doesn't collect later
> > added memory to e820 because it uses e820_table_kexec directly to pass
> > e820 to kexec-ed kernel. However, this e820_table_kexec is only updated
> > during boot stage. I tried hot adding DIMM after boot, kexec-ed kernel
> > doesn't have it in e820 during bootup, but it's recoginized and added
> > when ACPI scanning. I think we should update e820_table_kexec when hot
> > add/remove memory, at least for DIMM. Not sure if DLPAR, virtio-mem,
> > balloon will need be added into e820_table_kexec too, and if this is
> > expected behaviour.
> > 
> > But whatever we do, it won't impact the kexec file_loading, because of
> > the searching strategy bottom up. Just adding them into e820_table_kexec
> > will make it consistent with cold reboot which get recognizes and get
> > them into e820 during bootup.
> 
> Yeah, I think whatever a cold-booted kernel will see is what kexec-ed
> kernel should see. Not more, not less.
> 
> Regarding virtio-mem: Not in e820 on cold-boot.
> Regarding DIMMs: DIMMs under KVM will never show up in the e820 map
> IIRC. I think on real HW it can be different.

Yeah, DIMMs under KVM won't show up in e820 map. While this is not feature
of QEMU/KVM, but a defect of it. I ever asked Igor who is developer of
QEMU/KVM guest in this area, why we don't make kvm guest recognize
hotpluggable DIMM and add it into e820 map, he said he had tried to make
it, but this will corrupt guest on HyperV. So he had to revert the
commit on qemu. So I think we can leave it for now for both real HW and
kvm, or update the e820_table_kexec to include added DIMM for both real
HW and KVM. I hope one day KVM dev will find a way to conquer the defect
on HyperV and make the e820map consistent with bare metal. After all,
kvm guest is trying to imitate real HW for the most part.

Anyway, I will think about the e820_table_kexec updating. See if we can
do something about it.


^ permalink raw reply

* [PATCH] powerpc/64: Have MPROFILE_KERNEL depend on FUNCTION_TRACER
From: Naveen N. Rao @ 2020-04-22  9:26 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Steven Rostedt

Currently, it is possible to have CONFIG_FUNCTION_TRACER disabled, but
CONFIG_MPROFILE_KERNEL enabled. Though all existing users of
MPROFILE_KERNEL are doing the right thing, it is weird to have
MPROFILE_KERNEL enabled when the function tracer isn't. Fix this by
making MPROFILE_KERNEL depend on FUNCTION_TRACER.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 924c541a9260..8324d98728db 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -476,7 +476,7 @@ config LD_HEAD_STUB_CATCH
 	  If unsure, say "N".
 
 config MPROFILE_KERNEL
-	depends on PPC64 && CPU_LITTLE_ENDIAN
+	depends on PPC64 && CPU_LITTLE_ENDIAN && FUNCTION_TRACER
 	def_bool $(success,$(srctree)/arch/powerpc/tools/gcc-check-mprofile-kernel.sh $(CC) -I$(srctree)/include -D__KERNEL__)
 
 config HOTPLUG_CPU

base-commit: a9aa21d05c33c556e48c5062b6632a9b94906570
-- 
2.25.1


^ permalink raw reply related

* Re: [PATCH 1/3] kexec: Prevent removal of memory in use by a loaded kexec image
From: David Hildenbrand @ 2020-04-22  9:24 UTC (permalink / raw)
  To: Baoquan He
  Cc: piliu, Anshuman Khandual, Catalin Marinas, Bhupesh Sharma,
	linuxppc-dev, kexec, Russell King - ARM Linux admin, linux-mm,
	James Morse, Eric W. Biederman, Andrew Morton, Will Deacon,
	linux-arm-kernel
In-Reply-To: <20200422091718.GT4247@MiWiFi-R3L-srv>

On 22.04.20 11:17, Baoquan He wrote:
> On 04/21/20 at 03:29pm, David Hildenbrand wrote:
>>>> ACPI SRAT is embeded into efi, need read out the rsdp pointer. If we don't
>>>> pass the efi, it won't get the SRAT table correctly, if I remember
>>>> correctly. Yeah, I remeber kvm guest can get memory hotplugged with
>>>> ACPI only, this won't happen on bare metal though. Need check carefully. 
>>>> I have been using kvm guest with uefi firmwire recently.
>>>
>>> Yeah, I can imagine that bare metal is different. kvm only uses ACPI.
>>>
>>> I'm also asking because of virtio-mem. Memory added via virtio-mem is
>>> not part of any efi tables or whatsoever. So I assume the kexec kernel
>>> will not detect it automatically (good!), instead load the virtio-mem
>>> driver and let it add memory back to the system.
>>>
>>> I should probably play with kexec and virtio-mem once I have some spare
>>> cycles ... to find out what's broken and needs to be addressed :)
>>
>> FWIW, I just gave virtio-mem and kexec/kdump a try.
>>
>> a) kdump seems to work. Memory added by virtio-mem is getting dumped.
>> The kexec kernel only uses memory in the crash region. The virtio-mem
>> driver properly bails out due to is_kdump_kernel().
> 
> Right, kdump is not impacted later added memory.
> 
>>
>> b) "kexec -s -l" seems to work fine. For now, the kernel does not seem
>> to get placed on virtio-mem memory (pure luck due to the left-to-right
>> search). Memory added by virtio-mem is not getting added to the e820
>> map. Once the virtio-mem driver comes back up in the kexec kernel, the
>> right memory is readded.
> 
> kexec_file_load just behaves as you tested. It doesn't collect later
> added memory to e820 because it uses e820_table_kexec directly to pass
> e820 to kexec-ed kernel. However, this e820_table_kexec is only updated
> during boot stage. I tried hot adding DIMM after boot, kexec-ed kernel
> doesn't have it in e820 during bootup, but it's recoginized and added
> when ACPI scanning. I think we should update e820_table_kexec when hot
> add/remove memory, at least for DIMM. Not sure if DLPAR, virtio-mem,
> balloon will need be added into e820_table_kexec too, and if this is
> expected behaviour.
> 
> But whatever we do, it won't impact the kexec file_loading, because of
> the searching strategy bottom up. Just adding them into e820_table_kexec
> will make it consistent with cold reboot which get recognizes and get
> them into e820 during bootup.

Yeah, I think whatever a cold-booted kernel will see is what kexec-ed
kernel should see. Not more, not less.

Regarding virtio-mem: Not in e820 on cold-boot.
Regarding DIMMs: DIMMs under KVM will never show up in the e820 map
IIRC. I think on real HW it can be different.

-- 
Thanks,

David / dhildenb


^ permalink raw reply

* Re: [PATCH 1/3] kexec: Prevent removal of memory in use by a loaded kexec image
From: Baoquan He @ 2020-04-22  9:17 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: piliu, Anshuman Khandual, Catalin Marinas, Bhupesh Sharma,
	linuxppc-dev, kexec, Russell King - ARM Linux admin, linux-mm,
	James Morse, Eric W. Biederman, Andrew Morton, Will Deacon,
	linux-arm-kernel
In-Reply-To: <9a4eb1d7-33bf-8707-9c0c-1ca657c3e502@redhat.com>

On 04/21/20 at 03:29pm, David Hildenbrand wrote:
> >> ACPI SRAT is embeded into efi, need read out the rsdp pointer. If we don't
> >> pass the efi, it won't get the SRAT table correctly, if I remember
> >> correctly. Yeah, I remeber kvm guest can get memory hotplugged with
> >> ACPI only, this won't happen on bare metal though. Need check carefully. 
> >> I have been using kvm guest with uefi firmwire recently.
> > 
> > Yeah, I can imagine that bare metal is different. kvm only uses ACPI.
> > 
> > I'm also asking because of virtio-mem. Memory added via virtio-mem is
> > not part of any efi tables or whatsoever. So I assume the kexec kernel
> > will not detect it automatically (good!), instead load the virtio-mem
> > driver and let it add memory back to the system.
> > 
> > I should probably play with kexec and virtio-mem once I have some spare
> > cycles ... to find out what's broken and needs to be addressed :)
> 
> FWIW, I just gave virtio-mem and kexec/kdump a try.
> 
> a) kdump seems to work. Memory added by virtio-mem is getting dumped.
> The kexec kernel only uses memory in the crash region. The virtio-mem
> driver properly bails out due to is_kdump_kernel().

Right, kdump is not impacted later added memory.

> 
> b) "kexec -s -l" seems to work fine. For now, the kernel does not seem
> to get placed on virtio-mem memory (pure luck due to the left-to-right
> search). Memory added by virtio-mem is not getting added to the e820
> map. Once the virtio-mem driver comes back up in the kexec kernel, the
> right memory is readded.

kexec_file_load just behaves as you tested. It doesn't collect later
added memory to e820 because it uses e820_table_kexec directly to pass
e820 to kexec-ed kernel. However, this e820_table_kexec is only updated
during boot stage. I tried hot adding DIMM after boot, kexec-ed kernel
doesn't have it in e820 during bootup, but it's recoginized and added
when ACPI scanning. I think we should update e820_table_kexec when hot
add/remove memory, at least for DIMM. Not sure if DLPAR, virtio-mem,
balloon will need be added into e820_table_kexec too, and if this is
expected behaviour.

But whatever we do, it won't impact the kexec file_loading, because of
the searching strategy bottom up. Just adding them into e820_table_kexec
will make it consistent with cold reboot which get recognizes and get
them into e820 during bootup.
> 
> c) "kexec -c -l" does not work properly. All memory added by virtio-mem
> is added to the e820 map, which is wrong. Memory that should not be
> touched will be touched by the kexec kernel. I assume kexec-tools just
> goes ahead and adds anything it can find in /proc/iomem (or
> /sys/firmware/memmap/) to the e820 map of the new kernel.
> 
> Due to c), I assume all hotplugged memory (e.g., ACPI DIMMs) is
> similarly added to the e820 map and, therefore, won't be able to be
> onlined MOVABLE easily.

Yes, kexec_load will read memory regions from /sys/firmware/memmap/ or
/proc/iomem. Making it right seems a little harder, we can export them
to /proc/iomem or /sys/firmware/memmap/ with mark them with 'hotplug',
but the attribute that which zone they belongs to is not easy to tell.

We are proactive on widely testing kexec_file_load on x86_64, s390,
arm64 by adding test cases into CKI.

> 
> 
> At least for virtio-mem, I would either have to
> a) Not support "kexec -c -l". A viable option if we would be planning on
> not supporting it either way in the long term. I could block this
> in-kernel somehow eventually.
> 
> b) Teach kexec-tools to leave virtio-mem added memory alone. E.g., by
> indicating it in /proc/iomem in a special way ("System RAM
> (hotplugged)"/"System RAM (virtio-mem)").
> 
> Baoquan, any opinion on that?
> 
> -- 
> Thanks,
> 
> David / dhildenb


^ permalink raw reply

* Re: [PATCH kernel v2 0/7] powerpc/powenv/ioda: Allow huge DMA window at 4GB
From: Oliver O'Halloran @ 2020-04-22  9:11 UTC (permalink / raw)
  To: Alexey Kardashevskiy
  Cc: KVM list, Fabiano Rosas, Alistair Popple, kvm-ppc, linuxppc-dev,
	David Gibson
In-Reply-To: <ee3fd87f-f2b6-1439-a310-fedc614e6155@ozlabs.ru>

On Wed, Apr 22, 2020 at 4:49 PM Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>
> 32bit MMIO is what puzzles me in this picture, how does it work?

For devices with no m64 we allocate a PE number as described above. In
the 32bit MMIO window we have a segment-to-PE remapping table so any
m32 segment can be assigned to any PE. As a result slave PE concept
isn't really needed. If the BARs of a device span multiple m32
segments then we can setup the remapping table so that all the
segments point to the same PE.

> > I was thinking we should try minimise the number of DMA-only PEs since
> > it complicates the EEH freeze handling. When MMIO and DMA are mapped
> > to the same PE an error on either will cause the hardware to stop
> > both. When seperate PEs are used for DMA and MMIO you lose that
> > atomicity. It's not a big deal if DMA is stopped and MMIO allowed
> > since PAPR (sort-of) allows that, but having MMIO frozen with DMA
> > unfrozen is a bit sketch.
>
> You suggested using slave PEs for crippled functions - won't we have the
> same problem then?

Yes, but I think it's probably worth doing in that case. You get
slightly janky EEH in exchange for better DMA performance.

> And is this "slave PE" something the hardware supports or it is a
> software concept?

It's all in software. The hardware does have the PELT-V which allows
you to specify a group of PEs to additionally freeze when a PE is
frozen, but the PELT-V is only used when handling AER messages.  All
other error sources (DMAs, MMIOs, etc) will only freeze one PE (or all
of them in very rare cases).

> > There's been no official FW releases with a skiboot that supports the
> > phb get/set option opal calls so the only systems that can actually
> > take advantage of it are our lab systems. It might still be useful for
> > future systems, but I'd rather something that doesn't depend on FW
> > support.
>
> Pensando folks use it ;)

the what folks

Oliver

^ permalink raw reply

* Re: [musl] Re: Powerpc Linux 'scv' system call ABI proposal take 2
From: Florian Weimer @ 2020-04-22  8:11 UTC (permalink / raw)
  To: Nicholas Piggin
  Cc: Rich Felker, libc-alpha@sourceware.org, musl, David Laight,
	Adhemerval Zanella, libc-dev@lists.llvm.org,
	linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1587540390.vde84z8edw.astroid@bobo.none>

* Nicholas Piggin:

> So I would be disinclined to use SIGSYS unless there are no other better 
> signal types, and we don't want to use SIGILL for some good reason -- is 
> there a good reason to add complexity for userspace by differentiating 
> these two situations?

No, SIGILL seems fine to me.  scv 0 and scv 1 could well be considered
different instructions eventually (with different mnemonics).

^ permalink raw reply

* Re: [PATCH] powerpc/book3s64/kuap: SPRN_AMR modification need CSI instructions before and after
From: Nicholas Piggin @ 2020-04-22  7:45 UTC (permalink / raw)
  To: Aneesh Kumar K.V, linuxppc-dev, Michael Ellerman
In-Reply-To: <87y2qqhc2w.fsf@mpe.ellerman.id.au>

Excerpts from Michael Ellerman's message of April 20, 2020 5:04 pm:
> Nicholas Piggin <npiggin@gmail.com> writes:
>> Excerpts from Nicholas Piggin's message of April 20, 2020 10:17 am:
>>> Excerpts from Aneesh Kumar K.V's message of April 19, 2020 11:53 pm:
>>>> As per the ISA, context synchronizing instructions is needed before and after
>>>> SPRN_AMR update. Use isync before and the CSI after is implied by the rfid
>>>> that we will use to switch to a new context.
>>> 
>>> Not entirely sure if we need this. This will restore AMR to more 
>>> permissive, so if it executes ahead of a stray load from this
>>> context, it won't make it fault.
> 
> I thought we'd convinced ourselves it didn't matter in practice due to
> the proximity of the entry/exit.

I don't remember exactly. We can always drop the isync from the side 
that pairs with an entry or exit.

If we drop it from the other side, what it means in theory is it could 
float past some of the accesses we're doing in the interrupt context 
that we thought were protected. So we won't take faults, but it's 
possible we would let through a user access.

I think it's likey that we'd end up executing the mtspr before anything 
much can take advantage of it, but you never know, and I guess the 
problem is it becomes impossile to audit and be sure.

Thanks,
Nick

^ permalink raw reply

* Re: [PATCH 0/3] powerpc/module_64: Fix _mcount() stub
From: Naveen N. Rao @ 2020-04-22  7:47 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Qian Cai, Steven Rostedt
In-Reply-To: <cover.1587488954.git.naveen.n.rao@linux.vnet.ibm.com>

Naveen N. Rao wrote:
> This series addresses the crash reported by Qian Cai on ppc64le with 
> -mprofile-kernel here:
> https://lore.kernel.org/r/15AC5B0E-A221-4B8C-9039-FA96B8EF7C88@lca.pw
> 
> While fixing patch_instruction() should address the crash, we should 
> still change the default stub we setup for _mcount() for cases where a 
> kernel is built without ftrace.

I notice that I didn't write that properly yesterday. I meant that this 
is required for cases where ftrace gets disabled for any reason, 
resulting in the default calls to _mcount() remaining in the kernel 
modules.

- Naveen


^ permalink raw reply

* Re: [musl] Re: Powerpc Linux 'scv' system call ABI proposal take 2
From: Nicholas Piggin @ 2020-04-22  7:31 UTC (permalink / raw)
  To: Florian Weimer
  Cc: Rich Felker, libc-alpha@sourceware.org, musl, David Laight,
	Adhemerval Zanella, libc-dev@lists.llvm.org,
	linuxppc-dev@lists.ozlabs.org
In-Reply-To: <874ktcng8z.fsf@mid.deneb.enyo.de>

Excerpts from Florian Weimer's message of April 22, 2020 5:15 pm:
> * Nicholas Piggin:
> 
>> Another option would be to use a different signal. I don't see that any 
>> are more suitable.
> 
> SIGSYS comes to my mind.  But I don't know how exclusively it is
> associated with seccomp these days.

SIGSYS is entirely seccomp now. There looks like a single obscure MIPS 
user of it in Linux that's not seccomp, but it would be entirely new for 
powerpc (or any of the common platforms, arm, x86 etc).

So I would be disinclined to use SIGSYS unless there are no other better 
signal types, and we don't want to use SIGILL for some good reason -- is 
there a good reason to add complexity for userspace by differentiating 
these two situations?

Thanks,
Nick

^ permalink raw reply

* Re: [musl] Re: Powerpc Linux 'scv' system call ABI proposal take 2
From: Florian Weimer @ 2020-04-22  7:15 UTC (permalink / raw)
  To: Nicholas Piggin
  Cc: Rich Felker, libc-alpha@sourceware.org, musl, David Laight,
	Adhemerval Zanella, libc-dev@lists.llvm.org,
	linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1587536988.ivnp421w2w.astroid@bobo.none>

* Nicholas Piggin:

> Another option would be to use a different signal. I don't see that any 
> are more suitable.

SIGSYS comes to my mind.  But I don't know how exclusively it is
associated with seccomp these days.

^ permalink raw reply

* Re: Powerpc Linux 'scv' system call ABI proposal take 2
From: Nicholas Piggin @ 2020-04-22  6:54 UTC (permalink / raw)
  To: Adhemerval Zanella, Rich Felker, David Laight
  Cc: libc-dev@lists.llvm.org, libc-alpha@sourceware.org,
	linuxppc-dev@lists.ozlabs.org, musl@lists.openwall.com
In-Reply-To: <960127e0-57a0-55b4-f309-ae0a675c7756@linaro.org>

Let me try to summarise what we have.

- vdso style call is ruled out as unnecessary with possible security 
  concerns. Caller can internally use indirect branch to select variant 
  if it wants to use that mechanism to select.

- LR clobber seems to handled okay by gcc. It can increase size of small 
  leaf wrapper functions, but they can use the caller stack frame for 
  this (and even red zone for saving other things if necessary), but not 
  a huge amount.

- -ve error return seems to be favoured by everyone. Experimentally, 
  it's better for musl (but musl could probably improve cr0[SO] error 
  handling a bit 'asm goto').

- Preserving syscall args and volatiles up to r8 is a small but 
  noticable help for cases that inline the call rather than always call 
  wrappers. This is unlikely to be helpful unless 'sc' support is 
  compiled out but I'll consider doing it for the long term. Next step 
  is to trace and test on real hardware.

- One thing that nobody has really asked about is error handling for 
  unsupported scv vectors, so I would like to just go over it:

Today, the scv facility is disabled by the kernel (FSCR[SCV] is 
cleared), which makes any `scv` instruction take a facility
unavailable, which ends up printing a kernel message about SCV
facility unavilable, and SIGILL's the process with ILL_ILLOPC.

Enabling 'scv 0' will enable 1-127 as well, so the kernel has to handle 
those somehow.

What we are saying is that we will allocate HWCAP bits in future if we 
implement more scv vectors, so userspace is not *supposed* to rely on 
this, but kernel has to choose some behaviour for invalid vectors.

My proposal was to do the same SIGILL (with no kernel facility message),
so it appears to behave the same way to userspace as it does now. There 
is also the ILL_ILLOPN code that could be used as invalid operand, but
powerpc does not use this much, and e.g., the static instruction 
coded operands e.g., invalid mfspr generate ILL_ILLOPC so we could 
consider the entire instruction as the opcode, and input register values 
as operands.

Now I don't know why a process would want to distinguish between 
FSCR[SCV]=0 and the case where it is enabled but kernel doesn't 
implement the vector, but maybe it does?

Another option would be to use a different signal. I don't see that any 
are more suitable.

Or return without a signal but -ENOSYS or something in r3. This doesn't 
seem so good because an invalid scv vector is not a system call, and a 
failure ABI would constrain any future implementation just a little bit.

Any objections to SIGILL ILL_ILLOPC?

Thanks,
Nick

^ permalink raw reply

* Re: [PATCH kernel v2 0/7] powerpc/powenv/ioda: Allow huge DMA window at 4GB
From: Alexey Kardashevskiy @ 2020-04-22  6:49 UTC (permalink / raw)
  To: Oliver O'Halloran
  Cc: KVM list, Fabiano Rosas, Alistair Popple, kvm-ppc, linuxppc-dev,
	David Gibson
In-Reply-To: <CAOSf1CG_qiR2HvSFVTbgTyqVmDt4+Oy60PNWY23K2ihHib1K7Q@mail.gmail.com>



On 21/04/2020 16:35, Oliver O'Halloran wrote:
> On Tue, Apr 21, 2020 at 3:11 PM Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>
>> One example of a problem device is AMD GPU with 64bit video PCI function
>> and 32bit audio, no?
>>
>> What PEs will they get assigned to now? Where will audio's MMIO go? It
>> cannot be the same 64bit MMIO segment, right? If so, it is a separate PE
>> already. If not, then I do not understand "we're free to assign whatever
>> PE number we want.
> 
> The BARs stay in the same place and as far as MMIO is concerned
> nothing has changed. For MMIO the PHB uses the MMIO address to find a
> PE via the M64 BAR table, but for DMA it uses a *completely* different
> mechanism. Instead it takes the BDFN (included in the DMA packet
> header) and the Requester Translation Table (RTT) to map the BDFN to a
> PE. Normally you would configure the PHB so the same PE used for MMIO
> and DMA, but you don't have to.

32bit MMIO is what puzzles me in this picture, how does it work?


>>> I think the key thing to realise is that we'd only be using the DMA-PE
>>> when a crippled DMA mask is set by the driver. In all other cases we
>>> can just use the "native PE" and when the driver unbinds we can de-
>>> allocate our DMA-PE and return the device to the PE containing it's
>>> MMIO BARs. I think we can keep things relatively sane that way and the
>>> real issue is detecting EEH events on the DMA-PE.
>>
>>
>> Oooor we could just have 1 DMA window (or, more precisely, a single
>> "TVE" as it is either window or bypass) per a PE and give every function
>> its own PE and create a window or a table when a device sets a DMA mask.
>> I feel I am missing something here though.
> 
> Yes, we could do that, but do we want to?
> 
> I was thinking we should try minimise the number of DMA-only PEs since
> it complicates the EEH freeze handling. When MMIO and DMA are mapped
> to the same PE an error on either will cause the hardware to stop
> both. When seperate PEs are used for DMA and MMIO you lose that
> atomicity. It's not a big deal if DMA is stopped and MMIO allowed
> since PAPR (sort-of) allows that, but having MMIO frozen with DMA
> unfrozen is a bit sketch.


You suggested using slave PEs for crippled functions - won't we have the
same problem then?
And is this "slave PE" something the hardware supports or it is a
software concept?


>>>> For the time being, this patchset is good for:
>>>> 1. weird hardware which has limited DMA mask (this is why the patchset
>>>> was written in the first place)
>>>> 2. debug DMA by routing it via IOMMU (even when 4GB hack is not enabled).
>>>
>>> Sure, but it's still dependent on having firmware which supports the
>>> 4GB hack and I don't think that's in any offical firmware releases yet.
>>
>> It's been a while :-/
> 
> There's been no official FW releases with a skiboot that supports the
> phb get/set option opal calls so the only systems that can actually
> take advantage of it are our lab systems. It might still be useful for
> future systems, but I'd rather something that doesn't depend on FW
> support.

Pensando folks use it ;)


-- 
Alexey

^ permalink raw reply

* linux-next: build failure after merge of the akpm tree
From: Stephen Rothwell @ 2020-04-22  6:39 UTC (permalink / raw)
  To: Andrew Morton, Michael Ellerman, PowerPC
  Cc: Linux Next Mailing List, Haren Myneni, Linux Kernel Mailing List,
	Christoph Hellwig

[-- Attachment #1: Type: text/plain, Size: 1958 bytes --]

Hi all,

After merging the akpm tree, today's linux-next build (powerpc
ppc64_defconfig) failed like this:

arch/powerpc/platforms/powernv/vas-fault.c: In function 'update_csb':
arch/powerpc/platforms/powernv/vas-fault.c:130:2: error: implicit declaration of function 'use_mm' [-Werror=implicit-function-declaration]
  130 |  use_mm(window->mm);
      |  ^~~~~~
arch/powerpc/platforms/powernv/vas-fault.c:142:2: error: implicit declaration of function 'unuse_mm' [-Werror=implicit-function-declaration]
  142 |  unuse_mm(window->mm);
      |  ^~~~~~~~

Caused by commit

  7fe021a3f1c9 ("kernel: better document the use_mm/unuse_mm API contract")

interacting with commit

  c96c4436aba4 ("powerpc/vas: Update CSB and notify process for fault CRBs")

from the powerpc tree.

I added the following patch for today:

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Wed, 22 Apr 2020 16:35:23 +1000
Subject: [PATCH] powerpc/vas: fix up for {un}use_mm() rename

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 arch/powerpc/platforms/powernv/vas-fault.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/vas-fault.c b/arch/powerpc/platforms/powernv/vas-fault.c
index 25db70be4c9c..266a6ca5e15e 100644
--- a/arch/powerpc/platforms/powernv/vas-fault.c
+++ b/arch/powerpc/platforms/powernv/vas-fault.c
@@ -127,7 +127,7 @@ static void update_csb(struct vas_window *window,
 		return;
 	}
 
-	use_mm(window->mm);
+	kthread_use_mm(window->mm);
 	rc = copy_to_user(csb_addr, &csb, sizeof(csb));
 	/*
 	 * User space polls on csb.flags (first byte). So add barrier
@@ -139,7 +139,7 @@ static void update_csb(struct vas_window *window,
 		smp_mb();
 		rc = copy_to_user(csb_addr, &csb, sizeof(u8));
 	}
-	unuse_mm(window->mm);
+	kthread_unuse_mm(window->mm);
 	put_task_struct(tsk);
 
 	/* Success */
-- 
2.25.1

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply related

* Re: linux-next: build failure after merge of the powerpc tree
From: Haren Myneni @ 2020-04-22  6:35 UTC (permalink / raw)
  To: Stephen Rothwell, Michael Ellerman, PowerPC
  Cc: Linux Next Mailing List, Linux Kernel Mailing List
In-Reply-To: <20200422154129.11f988fd@canb.auug.org.au>

Stephen, Sorry missed it. Thanks for fixing it.

Acked-by: Haren Myneni <haren@linux.ibm.com>

On 4/21/20 10:41 PM, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the powerpc tree, today's linux-next build (powerpc
> allyesconfig) failed like this:
> 
> In file included from <command-line>:32:
> ./usr/include/asm/vas-api.h:15:2: error: unknown type name '__u32'
>    15 |  __u32 version;
>       |  ^~~~~
> ./usr/include/asm/vas-api.h:16:2: error: unknown type name '__s16'
>    16 |  __s16 vas_id; /* specific instance of vas or -1 for default */
>       |  ^~~~~
> ./usr/include/asm/vas-api.h:17:2: error: unknown type name '__u16'
>    17 |  __u16 reserved1;
>       |  ^~~~~
> ./usr/include/asm/vas-api.h:18:2: error: unknown type name '__u64'
>    18 |  __u64 flags; /* Future use */
>       |  ^~~~~
> ./usr/include/asm/vas-api.h:19:2: error: unknown type name '__u64'
>    19 |  __u64 reserved2[6];
>       |  ^~~~~
> 
> Caused by commit
> 
>   45f25a79fe50 ("powerpc/vas: Define VAS_TX_WIN_OPEN ioctl API")
> 
> uapi headers should be self contained.  I have added the following patch
> for today:
> 
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Wed, 22 Apr 2020 15:28:26 +1000
> Subject: [PATCH] powerpc/vas: uapi headers should be self contained
> 
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>> ---
>  arch/powerpc/include/uapi/asm/vas-api.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/powerpc/include/uapi/asm/vas-api.h b/arch/powerpc/include/uapi/asm/vas-api.h
> index fe95d67e3bab..ebd4b2424785 100644
> --- a/arch/powerpc/include/uapi/asm/vas-api.h
> +++ b/arch/powerpc/include/uapi/asm/vas-api.h
> @@ -6,6 +6,8 @@
>  #ifndef _UAPI_MISC_VAS_H
>  #define _UAPI_MISC_VAS_H
>  
> +#include <linux/types.h>
> +
>  #include <asm/ioctl.h>
>  
>  #define VAS_MAGIC	'v'
> 

^ permalink raw reply

* Re: [musl] Powerpc Linux 'scv' system call ABI proposal take 2
From: Nicholas Piggin @ 2020-04-22  6:29 UTC (permalink / raw)
  To: Rich Felker; +Cc: libc-dev, libc-alpha, linuxppc-dev, Adhemerval Zanella, musl
In-Reply-To: <1587531042.1qvc287tsc.astroid@bobo.none>

Excerpts from Nicholas Piggin's message of April 22, 2020 4:18 pm:
> If we go further and try to preserve r3 as well by putting the return 
> value in r9 or r0, we go backwards about 300 bytes. It's good for the 
> lock loops and complex functions, but hurts a lot of simpler functions 
> that have to add 'mr r3,r9' etc.  
> 
> Most of the time there are saved non-volatile GPRs around anyway though, 
> so not sure which way to go on this. Text size savings can't be ignored
> and it's pretty easy for the kernel to do (we already save r3-r8 and
> zero them on exit, so we could load them instead from cache line that's
> should be hot).
> 
> So I may be inclined to go this way, even if we won't see benefit now.

By, "this way" I don't mean r9 or r0 return value (which is larger code),
but r3 return value with r0,r4-r8 preserved.

Thanks,
Nick

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox