LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 6/6] arm64: ptrace: add support for syscall emulation
From: Sudeep Holla @ 2019-03-18 10:49 UTC (permalink / raw)
  To: x86, linux-arm-kernel, linux-kernel, linuxppc-dev
  Cc: Haibo Xu, Steve Capper, jdike, Sudeep Holla, Will Deacon,
	Oleg Nesterov, Bin Lu, Richard Weinberger, Ingo Molnar,
	Paul Mackerras, Andy Lutomirski, Catalin Marinas, Thomas Gleixner
In-Reply-To: <20190318104925.16600-1-sudeep.holla@arm.com>

Add PTRACE_SYSEMU and PTRACE_SYSEMU_SINGLESTEP support on arm64.
We can just make sure of the generic ptrace_syscall_enter hook to
support PTRACE_SYSEMU. We don't need any special handling for
PTRACE_SYSEMU_SINGLESTEP.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 arch/arm64/include/asm/thread_info.h | 5 ++++-
 arch/arm64/kernel/ptrace.c           | 3 +++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
index eb3ef73e07cf..c285d1ce7186 100644
--- a/arch/arm64/include/asm/thread_info.h
+++ b/arch/arm64/include/asm/thread_info.h
@@ -75,6 +75,7 @@ void arch_release_task_struct(struct task_struct *tsk);
  *  TIF_SYSCALL_TRACE	- syscall trace active
  *  TIF_SYSCALL_TRACEPOINT - syscall tracepoint for ftrace
  *  TIF_SYSCALL_AUDIT	- syscall auditing
+ *  TIF_SYSCALL_EMU     - syscall emulation active
  *  TIF_SECOMP		- syscall secure computing
  *  TIF_SIGPENDING	- signal pending
  *  TIF_NEED_RESCHED	- rescheduling necessary
@@ -91,6 +92,7 @@ void arch_release_task_struct(struct task_struct *tsk);
 #define TIF_SYSCALL_AUDIT	9
 #define TIF_SYSCALL_TRACEPOINT	10
 #define TIF_SECCOMP		11
+#define TIF_SYSCALL_EMU		12
 #define TIF_MEMDIE		18	/* is terminating due to OOM killer */
 #define TIF_FREEZE		19
 #define TIF_RESTORE_SIGMASK	20
@@ -109,6 +111,7 @@ void arch_release_task_struct(struct task_struct *tsk);
 #define _TIF_SYSCALL_AUDIT	(1 << TIF_SYSCALL_AUDIT)
 #define _TIF_SYSCALL_TRACEPOINT	(1 << TIF_SYSCALL_TRACEPOINT)
 #define _TIF_SECCOMP		(1 << TIF_SECCOMP)
+#define _TIF_SYSCALL_EMU	(1 << TIF_SYSCALL_EMU)
 #define _TIF_UPROBE		(1 << TIF_UPROBE)
 #define _TIF_FSCHECK		(1 << TIF_FSCHECK)
 #define _TIF_32BIT		(1 << TIF_32BIT)
@@ -120,7 +123,7 @@ void arch_release_task_struct(struct task_struct *tsk);
 
 #define _TIF_SYSCALL_WORK	(_TIF_SYSCALL_TRACE | _TIF_SYSCALL_AUDIT | \
 				 _TIF_SYSCALL_TRACEPOINT | _TIF_SECCOMP | \
-				 _TIF_NOHZ)
+				 _TIF_NOHZ | _TIF_SYSCALL_EMU)
 
 #define INIT_THREAD_INFO(tsk)						\
 {									\
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index b82e0a9b3da3..cf29275cd4d9 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -1819,6 +1819,9 @@ static void tracehook_report_syscall(struct pt_regs *regs,
 
 int syscall_trace_enter(struct pt_regs *regs)
 {
+	if (unlikely(ptrace_syscall_enter(regs)))
+		return -1;
+
 	if (test_thread_flag(TIF_SYSCALL_TRACE))
 		tracehook_report_syscall(regs, PTRACE_SYSCALL_ENTER);
 
-- 
2.17.1


^ permalink raw reply related

* [PATCH v2 5/6] arm64: add PTRACE_SYSEMU{, SINGLESTEP} definations to uapi headers
From: Sudeep Holla @ 2019-03-18 10:49 UTC (permalink / raw)
  To: x86, linux-arm-kernel, linux-kernel, linuxppc-dev
  Cc: Haibo Xu, Steve Capper, jdike, Sudeep Holla, Will Deacon,
	Oleg Nesterov, Bin Lu, Richard Weinberger, Ingo Molnar,
	Paul Mackerras, Andy Lutomirski, Catalin Marinas, Thomas Gleixner
In-Reply-To: <20190318104925.16600-1-sudeep.holla@arm.com>

x86 and um use 31 and 32 for PTRACE_SYSEMU and PTRACE_SYSEMU_SINGLESTEP
while powerpc uses different value maybe for legacy reasons.

Though handling of PTRACE_SYSEMU can be made architecture independent,
it's hard to make these definations generic. To add to this existing
mess few architectures like arm, c6x and sh use 31 for PTRACE_GETFDPIC
(get the ELF fdpic loadmap address). It's not possible to move the
definations to generic headers.

So we unfortunately have to duplicate the same defination to ARM64 if
we need to support PTRACE_SYSEMU.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 arch/arm64/include/uapi/asm/ptrace.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm64/include/uapi/asm/ptrace.h b/arch/arm64/include/uapi/asm/ptrace.h
index d78623acb649..627ac57c1581 100644
--- a/arch/arm64/include/uapi/asm/ptrace.h
+++ b/arch/arm64/include/uapi/asm/ptrace.h
@@ -62,6 +62,9 @@
 #define PSR_x		0x0000ff00	/* Extension		*/
 #define PSR_c		0x000000ff	/* Control		*/
 
+/* syscall emulation path in ptrace */
+#define PTRACE_SYSEMU		  31
+#define PTRACE_SYSEMU_SINGLESTEP  32
 
 #ifndef __ASSEMBLY__
 
-- 
2.17.1


^ permalink raw reply related

* [PATCH v2 4/6] powerpc: use common ptrace_syscall_enter hook to handle _TIF_SYSCALL_EMU
From: Sudeep Holla @ 2019-03-18 10:49 UTC (permalink / raw)
  To: x86, linux-arm-kernel, linux-kernel, linuxppc-dev
  Cc: Haibo Xu, Steve Capper, jdike, Sudeep Holla, Will Deacon,
	Oleg Nesterov, Bin Lu, Richard Weinberger, Ingo Molnar,
	Paul Mackerras, Andy Lutomirski, Catalin Marinas, Thomas Gleixner
In-Reply-To: <20190318104925.16600-1-sudeep.holla@arm.com>

Now that we have a new hook ptrace_syscall_enter that can be called from
syscall entry code and it handles PTRACE_SYSEMU in generic code, we
can do some cleanup using the same in do_syscall_trace_enter.

Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 arch/powerpc/kernel/ptrace.c | 48 ++++++++++++++++--------------------
 1 file changed, 21 insertions(+), 27 deletions(-)

diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index 2e2183b800a8..05579a5dcb12 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -3278,35 +3278,29 @@ long do_syscall_trace_enter(struct pt_regs *regs)
 
 	user_exit();
 
-	flags = READ_ONCE(current_thread_info()->flags) &
-		(_TIF_SYSCALL_EMU | _TIF_SYSCALL_TRACE);
-
-	if (flags) {
-		int rc = tracehook_report_syscall_entry(regs);
+	if (unlikely(ptrace_syscall_enter(regs))) {
+		/*
+		 * A nonzero return code from tracehook_report_syscall_entry()
+		 * tells us to prevent the syscall execution, but we are not
+		 * going to execute it anyway.
+		 *
+		 * Returning -1 will skip the syscall execution. We want to
+		 * avoid clobbering any registers, so we don't goto the skip
+		 * label below.
+		 */
+		return -1;
+	}
 
-		if (unlikely(flags & _TIF_SYSCALL_EMU)) {
-			/*
-			 * A nonzero return code from
-			 * tracehook_report_syscall_entry() tells us to prevent
-			 * the syscall execution, but we are not going to
-			 * execute it anyway.
-			 *
-			 * Returning -1 will skip the syscall execution. We want
-			 * to avoid clobbering any registers, so we don't goto
-			 * the skip label below.
-			 */
-			return -1;
-		}
+	flags = READ_ONCE(current_thread_info()->flags) & _TIF_SYSCALL_TRACE;
 
-		if (rc) {
-			/*
-			 * The tracer decided to abort the syscall. Note that
-			 * the tracer may also just change regs->gpr[0] to an
-			 * invalid syscall number, that is handled below on the
-			 * exit path.
-			 */
-			goto skip;
-		}
+	if (flags && tracehook_report_syscall_entry(regs)) {
+		/*
+		 * The tracer decided to abort the syscall. Note that
+		 * the tracer may also just change regs->gpr[0] to an
+		 * invalid syscall number, that is handled below on the
+		 * exit path.
+		 */
+		goto skip;
 	}
 
 	/* Run seccomp after ptrace; allow it to set gpr[3]. */
-- 
2.17.1


^ permalink raw reply related

* [PATCH v2 3/6] x86: clean up _TIF_SYSCALL_EMU handling using ptrace_syscall_enter hook
From: Sudeep Holla @ 2019-03-18 10:49 UTC (permalink / raw)
  To: x86, linux-arm-kernel, linux-kernel, linuxppc-dev
  Cc: Haibo Xu, Steve Capper, jdike, Sudeep Holla, Will Deacon,
	Oleg Nesterov, Bin Lu, Richard Weinberger, Ingo Molnar,
	Paul Mackerras, Andy Lutomirski, Catalin Marinas, Borislav Petkov,
	Thomas Gleixner
In-Reply-To: <20190318104925.16600-1-sudeep.holla@arm.com>

Now that we have a new hook ptrace_syscall_enter that can be called from
syscall entry code and it handles PTRACE_SYSEMU in generic code, we
can do some cleanup using the same in syscall_trace_enter.

Further the extra logic to find single stepping PTRACE_SYSEMU_SINGLESTEP
in syscall_slow_exit_work seems unnecessary. Let's remove the same.

Cc: Andy Lutomirski <luto@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 arch/x86/entry/common.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
index 7bc105f47d21..5d7590994964 100644
--- a/arch/x86/entry/common.c
+++ b/arch/x86/entry/common.c
@@ -70,22 +70,16 @@ static long syscall_trace_enter(struct pt_regs *regs)
 
 	struct thread_info *ti = current_thread_info();
 	unsigned long ret = 0;
-	bool emulated = false;
 	u32 work;
 
 	if (IS_ENABLED(CONFIG_DEBUG_ENTRY))
 		BUG_ON(regs != task_pt_regs(current));
 
-	work = READ_ONCE(ti->flags) & _TIF_WORK_SYSCALL_ENTRY;
-
-	if (unlikely(work & _TIF_SYSCALL_EMU))
-		emulated = true;
-
-	if ((emulated || (work & _TIF_SYSCALL_TRACE)) &&
-	    tracehook_report_syscall_entry(regs))
+	if (unlikely(ptrace_syscall_enter(regs)))
 		return -1L;
 
-	if (emulated)
+	work = READ_ONCE(ti->flags) & _TIF_WORK_SYSCALL_ENTRY;
+	if ((work & _TIF_SYSCALL_TRACE) && tracehook_report_syscall_entry(regs))
 		return -1L;
 
 #ifdef CONFIG_SECCOMP
-- 
2.17.1


^ permalink raw reply related

* [PATCH v2 2/6] ptrace: introduce ptrace_syscall_enter to consolidate PTRACE_SYSEMU handling
From: Sudeep Holla @ 2019-03-18 10:49 UTC (permalink / raw)
  To: x86, linux-arm-kernel, linux-kernel, linuxppc-dev
  Cc: Haibo Xu, Steve Capper, jdike, Sudeep Holla, Will Deacon,
	Oleg Nesterov, Bin Lu, Richard Weinberger, Ingo Molnar,
	Paul Mackerras, Andy Lutomirski, Catalin Marinas, Thomas Gleixner
In-Reply-To: <20190318104925.16600-1-sudeep.holla@arm.com>

Currently each architecture handles PTRACE_SYSEMU in very similar way.
It's completely arch independent and can be handled in the code helping
to consolidate PTRACE_SYSEMU handling.

Let's introduce a hook 'ptrace_syscall_enter' that arch specific syscall
entry code can call.

Cc: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 include/linux/ptrace.h |  1 +
 kernel/ptrace.c        | 22 ++++++++++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h
index edb9b040c94c..e30f51e3363e 100644
--- a/include/linux/ptrace.h
+++ b/include/linux/ptrace.h
@@ -407,6 +407,7 @@ static inline void user_single_step_report(struct pt_regs *regs)
 #define current_user_stack_pointer() user_stack_pointer(current_pt_regs())
 #endif
 
+extern long ptrace_syscall_enter(struct pt_regs *regs);
 extern int task_current_syscall(struct task_struct *target, long *callno,
 				unsigned long args[6], unsigned int maxargs,
 				unsigned long *sp, unsigned long *pc);
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 4fa3b7f4c3c7..c9c505c483df 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -29,6 +29,7 @@
 #include <linux/hw_breakpoint.h>
 #include <linux/cn_proc.h>
 #include <linux/compat.h>
+#include <linux/tracehook.h>
 
 /*
  * Access another process' address space via ptrace.
@@ -557,6 +558,27 @@ static int ptrace_detach(struct task_struct *child, unsigned int data)
 	return 0;
 }
 
+/*
+ * Hook to check and report for PTRACE_SYSEMU, can be called from arch
+ * arch syscall entry code
+ */
+long ptrace_syscall_enter(struct pt_regs *regs)
+{
+#ifdef TIF_SYSCALL_EMU
+	if (test_thread_flag(TIF_SYSCALL_EMU)) {
+		if (tracehook_report_syscall_entry(regs))
+			/*
+			 * We can ignore the return code here as we need
+			 * return -1 always for syscall emulation irrespective
+			 * of whether the tracehook report fails or succeed.
+			 */
+			;
+		return -1L;
+	}
+#endif
+	return 0;
+}
+
 /*
  * Detach all tasks we were using ptrace on. Called with tasklist held
  * for writing.
-- 
2.17.1


^ permalink raw reply related

* [PATCH v2 1/6] ptrace: move clearing of TIF_SYSCALL_EMU flag to core
From: Sudeep Holla @ 2019-03-18 10:49 UTC (permalink / raw)
  To: x86, linux-arm-kernel, linux-kernel, linuxppc-dev
  Cc: Haibo Xu, Steve Capper, jdike, Sudeep Holla, Will Deacon,
	Oleg Nesterov, Bin Lu, Richard Weinberger, Ingo Molnar,
	Paul Mackerras, Andy Lutomirski, Catalin Marinas, Thomas Gleixner
In-Reply-To: <20190318104925.16600-1-sudeep.holla@arm.com>

While the TIF_SYSCALL_EMU is set in ptrace_resume independent of any
architecture, currently only powerpc and x86 unset the TIF_SYSCALL_EMU
flag in ptrace_disable which gets called from ptrace_detach.

Let's move the clearing of TIF_SYSCALL_EMU flag to ptrace_detach after
we return from ptrace_disable to ensure there's no change in the flow.

Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 arch/powerpc/kernel/ptrace.c | 1 -
 arch/x86/kernel/ptrace.c     | 3 ---
 kernel/ptrace.c              | 4 ++++
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index d9ac7d94656e..2e2183b800a8 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -2520,7 +2520,6 @@ void ptrace_disable(struct task_struct *child)
 {
 	/* make sure the single step bit is not set. */
 	user_disable_single_step(child);
-	clear_tsk_thread_flag(child, TIF_SYSCALL_EMU);
 }
 
 #ifdef CONFIG_PPC_ADV_DEBUG_REGS
diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c
index 4b8ee05dd6ad..45792dbd2443 100644
--- a/arch/x86/kernel/ptrace.c
+++ b/arch/x86/kernel/ptrace.c
@@ -746,9 +746,6 @@ static int ioperm_get(struct task_struct *target,
 void ptrace_disable(struct task_struct *child)
 {
 	user_disable_single_step(child);
-#ifdef TIF_SYSCALL_EMU
-	clear_tsk_thread_flag(child, TIF_SYSCALL_EMU);
-#endif
 }
 
 #if defined CONFIG_X86_32 || defined CONFIG_IA32_EMULATION
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 771e93f9c43f..4fa3b7f4c3c7 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -534,6 +534,10 @@ static int ptrace_detach(struct task_struct *child, unsigned int data)
 	/* Architecture-specific hardware disable .. */
 	ptrace_disable(child);
 
+#ifdef TIF_SYSCALL_EMU
+	clear_tsk_thread_flag(child, TIF_SYSCALL_EMU);
+#endif
+
 	write_lock_irq(&tasklist_lock);
 	/*
 	 * We rely on ptrace_freeze_traced(). It can't be killed and
-- 
2.17.1


^ permalink raw reply related

* [PATCH v2 0/6] ptrace: consolidate PTRACE_SYSEMU handling and add support for arm64
From: Sudeep Holla @ 2019-03-18 10:49 UTC (permalink / raw)
  To: x86, linux-arm-kernel, linux-kernel, linuxppc-dev
  Cc: Haibo Xu, Steve Capper, jdike, Sudeep Holla, Will Deacon,
	Oleg Nesterov, Bin Lu, Richard Weinberger, Ingo Molnar,
	Paul Mackerras, Andy Lutomirski, Catalin Marinas, Thomas Gleixner

Hi,

This patchset evolved from the discussion in the thread[0][1]. When we
wanted to add PTRACE_SYSEMU support to ARM64, we thought instead of
duplicating what other architectures like x86 and powerpc have done,
let consolidate the existing support and move it to the core as there's
nothing arch specific in it.

v1->v2:
	- added comment for empty statement after tracehook_report_syscall_entry
	- dropped x86 change in syscall_slow_exit_work as I had ended
	  up changing logic unintentionally
	- removed spurious change in powerpc moving user_exit()

Regards,
Sudeep

[0] https://patchwork.kernel.org/patch/10585505/
[1] https://patchwork.kernel.org/patch/10675237/


Sudeep Holla (6):
  ptrace: move clearing of TIF_SYSCALL_EMU flag to core
  ptrace: introduce ptrace_syscall_enter to consolidate PTRACE_SYSEMU handling
  x86: clean up _TIF_SYSCALL_EMU handling using ptrace_syscall_enter hook
  powerpc: use common ptrace_syscall_enter hook to handle _TIF_SYSCALL_EMU
  arm64: add PTRACE_SYSEMU{,SINGLESTEP} definations to uapi headers
  arm64: ptrace: add support for syscall emulation

 arch/arm64/include/asm/thread_info.h |  5 ++-
 arch/arm64/include/uapi/asm/ptrace.h |  3 ++
 arch/arm64/kernel/ptrace.c           |  3 ++
 arch/powerpc/kernel/ptrace.c         | 49 ++++++++++++----------------
 arch/x86/entry/common.c              | 12 ++-----
 arch/x86/kernel/ptrace.c             |  3 --
 include/linux/ptrace.h               |  1 +
 kernel/ptrace.c                      | 26 +++++++++++++++
 8 files changed, 61 insertions(+), 41 deletions(-)

--
2.17.1


^ permalink raw reply

* Re: [RFC v3] sched/topology: fix kernel crash when a CPU is hotplugged in a memoryless node
From: Srikar Dronamraju @ 2019-03-18 10:47 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Laurent Vivier, linux-kernel, Michael Bringmann, Ingo Molnar,
	Suravee Suthikulpanit, Nathan Fontenot, Borislav Petkov,
	linuxppc-dev, David Gibson
In-Reply-To: <20190305115952.GH32477@hirez.programming.kicks-ass.net>

> > node 0 (because firmware doesn't provide the distance information for
> > memoryless/cpuless nodes):
> > 
> >   node   0   1   2   3
> >     0:  10  40  10  10
> >     1:  40  10  40  40
> >     2:  10  40  10  10
> >     3:  10  40  10  10
> 
> *groan*... what does it do for things like percpu memory? ISTR the
> per-cpu chunks are all allocated early too. Having them all use memory
> out of node-0 would seem sub-optimal.

In the specific failing case, there is only one node with memory; all other
nodes are cpu only nodes.

However in the generic case since its just a cpu hotplug ops, the memory
allocated for per-cpu chunks allocated early would remain.

May be Michael Ellerman can correct me here.

> 
> > We should have:
> > 
> >   node   0   1   2   3
> >     0:  10  40  40  40
> >     1:  40  10  40  40
> >     2:  40  40  10  40
> >     3:  40  40  40  10
> 
> Can it happen that it introduces a new distance in the table? One that
> hasn't been seen before? This example only has 10 and 40, but suppose
> the new node lands at distance 20 (or 80); can such a thing happen?
> 
> If not; why not?

Yes distances can be 20, 40 or 80. There is nothing that makes the node
distance to be 40 always.

> So you're relying on sched_domain_numa_masks_set/clear() to fix this up,
> but that in turn relies on the sched_domain_numa_levels thing to stay
> accurate.
> 
> This all seems very fragile and unfortunate.
> 

Any reasons why this is fragile?

-- 
Thanks and Regards
Srikar Dronamraju


^ permalink raw reply

* Re: [PATCH v3 03/17] KVM: PPC: Book3S HV: XIVE: introduce a new capability KVM_CAP_PPC_IRQ_XIVE
From: Cédric Le Goater @ 2019-03-18 10:00 UTC (permalink / raw)
  To: David Gibson; +Cc: linuxppc-dev, Paul Mackerras, kvm, kvm-ppc
In-Reply-To: <20190318001955.GD6874@umbus.fritz.box>

On 3/18/19 1:19 AM, David Gibson wrote:
> On Fri, Mar 15, 2019 at 01:05:55PM +0100, Cédric Le Goater wrote:
>> The user interface exposes a new capability KVM_CAP_PPC_IRQ_XIVE to
>> let QEMU connect the vCPU presenters to the XIVE KVM device if
>> required. The capability is not advertised for now as the full support
>> for the XIVE native exploitation mode is not yet available. When this
>> is case, the capability will be advertised on PowerNV Hypervisors
>> only. Nested guests (pseries KVM Hypervisor) are not supported.
>>
>> Internally, the interface to the new KVM device is protected with a
>> new interrupt mode: KVMPPC_IRQ_XIVE.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> 
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> 
> Though a couple of minor nits are noted below.
> 
>> ---
>>
>>  Changes since v2:
>>
>>  - made use of the xive_vp() macro to compute VP identifiers
>>  - reworked locking in kvmppc_xive_native_connect_vcpu() to fix races 
>>  - stop advertising KVM_CAP_PPC_IRQ_XIVE as support is not fully
>>    available yet 
>>  
>>  arch/powerpc/include/asm/kvm_host.h   |   1 +
>>  arch/powerpc/include/asm/kvm_ppc.h    |  13 +++
>>  arch/powerpc/kvm/book3s_xive.h        |  11 ++
>>  include/uapi/linux/kvm.h              |   1 +
>>  arch/powerpc/kvm/book3s_xive.c        |  88 ++++++++-------
>>  arch/powerpc/kvm/book3s_xive_native.c | 150 ++++++++++++++++++++++++++
>>  arch/powerpc/kvm/powerpc.c            |  36 +++++++
>>  Documentation/virtual/kvm/api.txt     |   9 ++
>>  8 files changed, 268 insertions(+), 41 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
>> index 9f75a75a07f2..eb8581be0ee8 100644
>> --- a/arch/powerpc/include/asm/kvm_host.h
>> +++ b/arch/powerpc/include/asm/kvm_host.h
>> @@ -448,6 +448,7 @@ struct kvmppc_passthru_irqmap {
>>  #define KVMPPC_IRQ_DEFAULT	0
>>  #define KVMPPC_IRQ_MPIC		1
>>  #define KVMPPC_IRQ_XICS		2 /* Includes a XIVE option */
>> +#define KVMPPC_IRQ_XIVE		3 /* XIVE native exploitation mode */
>>  
>>  #define MMIO_HPTE_CACHE_SIZE	4
>>  
>> diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
>> index 4b72ddde7dc1..1e61877fe147 100644
>> --- a/arch/powerpc/include/asm/kvm_ppc.h
>> +++ b/arch/powerpc/include/asm/kvm_ppc.h
>> @@ -594,6 +594,14 @@ extern int kvmppc_xive_set_irq(struct kvm *kvm, int irq_source_id, u32 irq,
>>  			       int level, bool line_status);
>>  extern void kvmppc_xive_push_vcpu(struct kvm_vcpu *vcpu);
>>  
>> +static inline int kvmppc_xive_enabled(struct kvm_vcpu *vcpu)
>> +{
>> +	return vcpu->arch.irq_type == KVMPPC_IRQ_XIVE;
>> +}
>> +
>> +extern int kvmppc_xive_native_connect_vcpu(struct kvm_device *dev,
>> +					   struct kvm_vcpu *vcpu, u32 cpu);
>> +extern void kvmppc_xive_native_cleanup_vcpu(struct kvm_vcpu *vcpu);
>>  extern void kvmppc_xive_native_init_module(void);
>>  extern void kvmppc_xive_native_exit_module(void);
>>  
>> @@ -621,6 +629,11 @@ static inline int kvmppc_xive_set_irq(struct kvm *kvm, int irq_source_id, u32 ir
>>  				      int level, bool line_status) { return -ENODEV; }
>>  static inline void kvmppc_xive_push_vcpu(struct kvm_vcpu *vcpu) { }
>>  
>> +static inline int kvmppc_xive_enabled(struct kvm_vcpu *vcpu)
>> +	{ return 0; }
>> +static inline int kvmppc_xive_native_connect_vcpu(struct kvm_device *dev,
>> +			  struct kvm_vcpu *vcpu, u32 cpu) { return -EBUSY; }
>> +static inline void kvmppc_xive_native_cleanup_vcpu(struct kvm_vcpu *vcpu) { }
>>  static inline void kvmppc_xive_native_init_module(void) { }
>>  static inline void kvmppc_xive_native_exit_module(void) { }
>>  
>> diff --git a/arch/powerpc/kvm/book3s_xive.h b/arch/powerpc/kvm/book3s_xive.h
>> index a08ae6fd4c51..d366df69b9cb 100644
>> --- a/arch/powerpc/kvm/book3s_xive.h
>> +++ b/arch/powerpc/kvm/book3s_xive.h
>> @@ -198,6 +198,11 @@ static inline struct kvmppc_xive_src_block *kvmppc_xive_find_source(struct kvmpp
>>  	return xive->src_blocks[bid];
>>  }
>>  
>> +static inline u32 kvmppc_xive_vp(struct kvmppc_xive *xive, u32 server)
>> +{
>> +	return xive->vp_base + kvmppc_pack_vcpu_id(xive->kvm, server);
>> +}
>> +
>>  /*
>>   * Mapping between guest priorities and host priorities
>>   * is as follow.
>> @@ -248,5 +253,11 @@ extern int (*__xive_vm_h_ipi)(struct kvm_vcpu *vcpu, unsigned long server,
>>  extern int (*__xive_vm_h_cppr)(struct kvm_vcpu *vcpu, unsigned long cppr);
>>  extern int (*__xive_vm_h_eoi)(struct kvm_vcpu *vcpu, unsigned long xirr);
>>  
>> +/*
>> + * Common Xive routines for XICS-over-XIVE and XIVE native
>> + */
>> +void kvmppc_xive_disable_vcpu_interrupts(struct kvm_vcpu *vcpu);
>> +int kvmppc_xive_debug_show_queues(struct seq_file *m, struct kvm_vcpu *vcpu);
>> +
>>  #endif /* CONFIG_KVM_XICS */
>>  #endif /* _KVM_PPC_BOOK3S_XICS_H */
>> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
>> index e6368163d3a0..52bf74a1616e 100644
>> --- a/include/uapi/linux/kvm.h
>> +++ b/include/uapi/linux/kvm.h
>> @@ -988,6 +988,7 @@ struct kvm_ppc_resize_hpt {
>>  #define KVM_CAP_ARM_VM_IPA_SIZE 165
>>  #define KVM_CAP_MANUAL_DIRTY_LOG_PROTECT 166
>>  #define KVM_CAP_HYPERV_CPUID 167
>> +#define KVM_CAP_PPC_IRQ_XIVE 168
>>  
>>  #ifdef KVM_CAP_IRQ_ROUTING
>>  
>> diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
>> index f78d002f0fe0..e7f1ada1c3de 100644
>> --- a/arch/powerpc/kvm/book3s_xive.c
>> +++ b/arch/powerpc/kvm/book3s_xive.c
>> @@ -380,11 +380,6 @@ static int xive_select_target(struct kvm *kvm, u32 *server, u8 prio)
>>  	return -EBUSY;
>>  }
>>  
>> -static u32 xive_vp(struct kvmppc_xive *xive, u32 server)
>> -{
>> -	return xive->vp_base + kvmppc_pack_vcpu_id(xive->kvm, server);
>> -}
>> -
>>  static u8 xive_lock_and_mask(struct kvmppc_xive *xive,
>>  			     struct kvmppc_xive_src_block *sb,
>>  			     struct kvmppc_xive_irq_state *state)
>> @@ -430,8 +425,8 @@ static u8 xive_lock_and_mask(struct kvmppc_xive *xive,
>>  	 */
>>  	if (xd->flags & OPAL_XIVE_IRQ_MASK_VIA_FW) {
>>  		xive_native_configure_irq(hw_num,
>> -					  xive_vp(xive, state->act_server),
>> -					  MASKED, state->number);
>> +				kvmppc_xive_vp(xive, state->act_server),
>> +				MASKED, state->number);
>>  		/* set old_p so we can track if an H_EOI was done */
>>  		state->old_p = true;
>>  		state->old_q = false;
>> @@ -486,8 +481,8 @@ static void xive_finish_unmask(struct kvmppc_xive *xive,
>>  	 */
>>  	if (xd->flags & OPAL_XIVE_IRQ_MASK_VIA_FW) {
>>  		xive_native_configure_irq(hw_num,
>> -					  xive_vp(xive, state->act_server),
>> -					  state->act_priority, state->number);
>> +				kvmppc_xive_vp(xive, state->act_server),
>> +				state->act_priority, state->number);
>>  		/* If an EOI is needed, do it here */
>>  		if (!state->old_p)
>>  			xive_vm_source_eoi(hw_num, xd);
>> @@ -563,7 +558,7 @@ static int xive_target_interrupt(struct kvm *kvm,
>>  	kvmppc_xive_select_irq(state, &hw_num, NULL);
>>  
>>  	return xive_native_configure_irq(hw_num,
>> -					 xive_vp(xive, server),
>> +					 kvmppc_xive_vp(xive, server),
>>  					 prio, state->number);
>>  }
>>  
>> @@ -951,7 +946,7 @@ int kvmppc_xive_set_mapped(struct kvm *kvm, unsigned long guest_irq,
>>  	 * which is fine for a never started interrupt.
>>  	 */
>>  	xive_native_configure_irq(hw_irq,
>> -				  xive_vp(xive, state->act_server),
>> +				  kvmppc_xive_vp(xive, state->act_server),
>>  				  state->act_priority, state->number);
>>  
>>  	/*
>> @@ -1027,7 +1022,7 @@ int kvmppc_xive_clr_mapped(struct kvm *kvm, unsigned long guest_irq,
>>  
>>  	/* Reconfigure the IPI */
>>  	xive_native_configure_irq(state->ipi_number,
>> -				  xive_vp(xive, state->act_server),
>> +				  kvmppc_xive_vp(xive, state->act_server),
>>  				  state->act_priority, state->number);
>>  
>>  	/*
>> @@ -1049,7 +1044,7 @@ int kvmppc_xive_clr_mapped(struct kvm *kvm, unsigned long guest_irq,
>>  }
>>  EXPORT_SYMBOL_GPL(kvmppc_xive_clr_mapped);
>>  
>> -static void kvmppc_xive_disable_vcpu_interrupts(struct kvm_vcpu *vcpu)
>> +void kvmppc_xive_disable_vcpu_interrupts(struct kvm_vcpu *vcpu)
>>  {
>>  	struct kvmppc_xive_vcpu *xc = vcpu->arch.xive_vcpu;
>>  	struct kvm *kvm = vcpu->kvm;
>> @@ -1166,7 +1161,7 @@ int kvmppc_xive_connect_vcpu(struct kvm_device *dev,
>>  	xc->xive = xive;
>>  	xc->vcpu = vcpu;
>>  	xc->server_num = cpu;
>> -	xc->vp_id = xive_vp(xive, cpu);
>> +	xc->vp_id = kvmppc_xive_vp(xive, cpu);
>>  	xc->mfrr = 0xff;
>>  	xc->valid = true;
>>  
>> @@ -1883,6 +1878,43 @@ static int kvmppc_xive_create(struct kvm_device *dev, u32 type)
>>  	return 0;
>>  }
>>  
>> +int kvmppc_xive_debug_show_queues(struct seq_file *m, struct kvm_vcpu *vcpu)
>> +{
>> +	struct kvmppc_xive_vcpu *xc = vcpu->arch.xive_vcpu;
>> +	unsigned int i;
>> +
>> +	for (i = 0; i < KVMPPC_XIVE_Q_COUNT; i++) {
>> +		struct xive_q *q = &xc->queues[i];
>> +		u32 i0, i1, idx;
>> +
>> +		if (!q->qpage && !xc->esc_virq[i])
>> +			continue;
>> +
>> +		seq_printf(m, " [q%d]: ", i);
>> +
>> +		if (q->qpage) {
>> +			idx = q->idx;
>> +			i0 = be32_to_cpup(q->qpage + idx);
>> +			idx = (idx + 1) & q->msk;
>> +			i1 = be32_to_cpup(q->qpage + idx);
>> +			seq_printf(m, "T=%d %08x %08x...\n", q->toggle,
>> +				   i0, i1);
>> +		}
>> +		if (xc->esc_virq[i]) {
>> +			struct irq_data *d = irq_get_irq_data(xc->esc_virq[i]);
>> +			struct xive_irq_data *xd =
>> +				irq_data_get_irq_handler_data(d);
>> +			u64 pq = xive_vm_esb_load(xd, XIVE_ESB_GET);
>> +
>> +			seq_printf(m, "E:%c%c I(%d:%llx:%llx)",
>> +				   (pq & XIVE_ESB_VAL_P) ? 'P' : 'p',
>> +				   (pq & XIVE_ESB_VAL_Q) ? 'Q' : 'q',
>> +				   xc->esc_virq[i], pq, xd->eoi_page);
>> +			seq_puts(m, "\n");
>> +		}
>> +	}
>> +	return 0;
>> +}
>>  
>>  static int xive_debug_show(struct seq_file *m, void *private)
>>  {
>> @@ -1908,7 +1940,6 @@ static int xive_debug_show(struct seq_file *m, void *private)
>>  
>>  	kvm_for_each_vcpu(i, vcpu, kvm) {
>>  		struct kvmppc_xive_vcpu *xc = vcpu->arch.xive_vcpu;
>> -		unsigned int i;
>>  
>>  		if (!xc)
>>  			continue;
>> @@ -1918,33 +1949,8 @@ static int xive_debug_show(struct seq_file *m, void *private)
>>  			   xc->server_num, xc->cppr, xc->hw_cppr,
>>  			   xc->mfrr, xc->pending,
>>  			   xc->stat_rm_h_xirr, xc->stat_vm_h_xirr);
>> -		for (i = 0; i < KVMPPC_XIVE_Q_COUNT; i++) {
>> -			struct xive_q *q = &xc->queues[i];
>> -			u32 i0, i1, idx;
>>  
>> -			if (!q->qpage && !xc->esc_virq[i])
>> -				continue;
>> -
>> -			seq_printf(m, " [q%d]: ", i);
>> -
>> -			if (q->qpage) {
>> -				idx = q->idx;
>> -				i0 = be32_to_cpup(q->qpage + idx);
>> -				idx = (idx + 1) & q->msk;
>> -				i1 = be32_to_cpup(q->qpage + idx);
>> -				seq_printf(m, "T=%d %08x %08x... \n", q->toggle, i0, i1);
>> -			}
>> -			if (xc->esc_virq[i]) {
>> -				struct irq_data *d = irq_get_irq_data(xc->esc_virq[i]);
>> -				struct xive_irq_data *xd = irq_data_get_irq_handler_data(d);
>> -				u64 pq = xive_vm_esb_load(xd, XIVE_ESB_GET);
>> -				seq_printf(m, "E:%c%c I(%d:%llx:%llx)",
>> -					   (pq & XIVE_ESB_VAL_P) ? 'P' : 'p',
>> -					   (pq & XIVE_ESB_VAL_Q) ? 'Q' : 'q',
>> -					   xc->esc_virq[i], pq, xd->eoi_page);
>> -				seq_printf(m, "\n");
>> -			}
>> -		}
>> +		kvmppc_xive_debug_show_queues(m, vcpu);
>>  
>>  		t_rm_h_xirr += xc->stat_rm_h_xirr;
>>  		t_rm_h_ipoll += xc->stat_rm_h_ipoll;
>> diff --git a/arch/powerpc/kvm/book3s_xive_native.c b/arch/powerpc/kvm/book3s_xive_native.c
>> index 76d45bcc7060..a078f99bc156 100644
>> --- a/arch/powerpc/kvm/book3s_xive_native.c
>> +++ b/arch/powerpc/kvm/book3s_xive_native.c
>> @@ -31,6 +31,134 @@
>>  
>>  #include "book3s_xive.h"
>>  
>> +static void kvmppc_xive_native_cleanup_queue(struct kvm_vcpu *vcpu, int prio)
>> +{
>> +	struct kvmppc_xive_vcpu *xc = vcpu->arch.xive_vcpu;
>> +	struct xive_q *q = &xc->queues[prio];
>> +
>> +	xive_native_disable_queue(xc->vp_id, q, prio);
>> +	if (q->qpage) {
>> +		put_page(virt_to_page(q->qpage));
>> +		q->qpage = NULL;
>> +	}
>> +}
>> +
>> +void kvmppc_xive_native_cleanup_vcpu(struct kvm_vcpu *vcpu)
>> +{
>> +	struct kvmppc_xive_vcpu *xc = vcpu->arch.xive_vcpu;
>> +	int i;
>> +
>> +	if (!kvmppc_xive_enabled(vcpu))
>> +		return;
>> +
>> +	if (!xc)
>> +		return;
>> +
>> +	pr_devel("native_cleanup_vcpu(cpu=%d)\n", xc->server_num);
>> +
>> +	/* Ensure no interrupt is still routed to that VP */
>> +	xc->valid = false;
>> +	kvmppc_xive_disable_vcpu_interrupts(vcpu);
>> +
>> +	/* Disable the VP */
>> +	xive_native_disable_vp(xc->vp_id);
>> +
>> +	/* Free the queues & associated interrupts */
>> +	for (i = 0; i < KVMPPC_XIVE_Q_COUNT; i++) {
>> +		/* Free the escalation irq */
>> +		if (xc->esc_virq[i]) {
>> +			free_irq(xc->esc_virq[i], vcpu);
>> +			irq_dispose_mapping(xc->esc_virq[i]);
>> +			kfree(xc->esc_virq_names[i]);
>> +			xc->esc_virq[i] = 0;
> 
> Should that actually be NO_IRQ on the right (it evaluates to the same
> thing).  Fine to fix that as a later followup.

yes. It would be better to clarify some tests in both KVM devices. 

( NO_IRQ is rarely used and its value is platform specific )

>> +		}
>> +
>> +		/* Free the queue */
>> +		kvmppc_xive_native_cleanup_queue(vcpu, i);
>> +	}
>> +
>> +	/* Free the VP */
>> +	kfree(xc);
>> +
>> +	/* Cleanup the vcpu */
>> +	vcpu->arch.irq_type = KVMPPC_IRQ_DEFAULT;
>> +	vcpu->arch.xive_vcpu = NULL;
>> +}
>> +
>> +int kvmppc_xive_native_connect_vcpu(struct kvm_device *dev,
>> +				    struct kvm_vcpu *vcpu, u32 server_num)
>> +{
>> +	struct kvmppc_xive *xive = dev->private;
>> +	struct kvmppc_xive_vcpu *xc = NULL;
>> +	int rc;
>> +
>> +	pr_devel("native_connect_vcpu(server=%d)\n", server_num);
>> +
>> +	if (dev->ops != &kvm_xive_native_ops) {
>> +		pr_devel("Wrong ops !\n");
>> +		return -EPERM;
>> +	}
>> +	if (xive->kvm != vcpu->kvm)
>> +		return -EPERM;
>> +	if (vcpu->arch.irq_type != KVMPPC_IRQ_DEFAULT)
>> +		return -EBUSY;
>> +	if (server_num >= KVM_MAX_VCPUS) {
>> +		pr_devel("Out of bounds !\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	mutex_lock(&vcpu->kvm->lock);
>> +
>> +	if (kvmppc_xive_find_server(vcpu->kvm, server_num)) {
>> +		pr_devel("Duplicate !\n");
>> +		rc = -EEXIST;
>> +		goto bail;
>> +	}
>> +
>> +	xc = kzalloc(sizeof(*xc), GFP_KERNEL);
>> +	if (!xc) {
>> +		rc = -ENOMEM;
>> +		goto bail;
>> +	}
>> +
>> +	vcpu->arch.xive_vcpu = xc;
>> +	xc->xive = xive;
>> +	xc->vcpu = vcpu;
>> +	xc->server_num = server_num;
>> +
>> +	xc->vp_id = kvmppc_xive_vp(xive, server_num);
>> +	xc->valid = true;
> 
> This 'valid' field doesn't seem useful, since it's initialized to true
> immediately after allocating xc and set to false moments before
> free()ing it. ..and I can't see anything that tests it.

It is used under the hood by kvmppc_xive_select_target() when configuring
the target of a source. I agree it is not very useful as it is redundant 
with the xc pointer. It seems it is also the case for the XICS-on-XIVE 
KVM device.

> Again, ok to deal with that as a later cleanup.

Yes I rather do that for both device at the same time.

Thanks,

C.


> 
>> +	vcpu->arch.irq_type = KVMPPC_IRQ_XIVE;
>> +
>> +	rc = xive_native_get_vp_info(xc->vp_id, &xc->vp_cam, &xc->vp_chip_id);
>> +	if (rc) {
>> +		pr_err("Failed to get VP info from OPAL: %d\n", rc);
>> +		goto bail;
>> +	}
>> +
>> +	/*
>> +	 * Enable the VP first as the single escalation mode will
>> +	 * affect escalation interrupts numbering
>> +	 */
>> +	rc = xive_native_enable_vp(xc->vp_id, xive->single_escalation);
>> +	if (rc) {
>> +		pr_err("Failed to enable VP in OPAL: %d\n", rc);
>> +		goto bail;
>> +	}
>> +
>> +	/* Configure VCPU fields for use by assembly push/pull */
>> +	vcpu->arch.xive_saved_state.w01 = cpu_to_be64(0xff000000);
>> +	vcpu->arch.xive_cam_word = cpu_to_be32(xc->vp_cam | TM_QW1W2_VO);
>> +
>> +	/* TODO: reset all queues to a clean state ? */
>> +bail:
>> +	mutex_unlock(&vcpu->kvm->lock);
>> +	if (rc)
>> +		kvmppc_xive_native_cleanup_vcpu(vcpu);
>> +
>> +	return rc;
>> +}
>> +
>>  static int kvmppc_xive_native_set_attr(struct kvm_device *dev,
>>  				       struct kvm_device_attr *attr)
>>  {
>> @@ -119,10 +247,32 @@ static int xive_native_debug_show(struct seq_file *m, void *private)
>>  {
>>  	struct kvmppc_xive *xive = m->private;
>>  	struct kvm *kvm = xive->kvm;
>> +	struct kvm_vcpu *vcpu;
>> +	unsigned int i;
>>  
>>  	if (!kvm)
>>  		return 0;
>>  
>> +	seq_puts(m, "=========\nVCPU state\n=========\n");
>> +
>> +	kvm_for_each_vcpu(i, vcpu, kvm) {
>> +		struct kvmppc_xive_vcpu *xc = vcpu->arch.xive_vcpu;
>> +
>> +		if (!xc)
>> +			continue;
>> +
>> +		seq_printf(m, "cpu server %#x NSR=%02x CPPR=%02x IBP=%02x PIPR=%02x w01=%016llx w2=%08x\n",
>> +			   xc->server_num,
>> +			   vcpu->arch.xive_saved_state.nsr,
>> +			   vcpu->arch.xive_saved_state.cppr,
>> +			   vcpu->arch.xive_saved_state.ipb,
>> +			   vcpu->arch.xive_saved_state.pipr,
>> +			   vcpu->arch.xive_saved_state.w01,
>> +			   (u32) vcpu->arch.xive_cam_word);
>> +
>> +		kvmppc_xive_debug_show_queues(m, vcpu);
>> +	}
>> +
>>  	return 0;
>>  }
>>  
>> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
>> index 8c69af10f91d..bb51faf29162 100644
>> --- a/arch/powerpc/kvm/powerpc.c
>> +++ b/arch/powerpc/kvm/powerpc.c
>> @@ -570,6 +570,15 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>>  	case KVM_CAP_PPC_GET_CPU_CHAR:
>>  		r = 1;
>>  		break;
>> +#ifdef CONFIG_KVM_XIVE
>> +	case KVM_CAP_PPC_IRQ_XIVE:
>> +		/*
>> +		 * Return false until all the XIVE infrastructure is
>> +		 * in place including support for migration.
>> +		 */
>> +		r = 0;
>> +		break;
>> +#endif
>>  
>>  	case KVM_CAP_PPC_ALLOC_HTAB:
>>  		r = hv_enabled;
>> @@ -753,6 +762,9 @@ void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu)
>>  		else
>>  			kvmppc_xics_free_icp(vcpu);
>>  		break;
>> +	case KVMPPC_IRQ_XIVE:
>> +		kvmppc_xive_native_cleanup_vcpu(vcpu);
>> +		break;
>>  	}
>>  
>>  	kvmppc_core_vcpu_free(vcpu);
>> @@ -1941,6 +1953,30 @@ static int kvm_vcpu_ioctl_enable_cap(struct kvm_vcpu *vcpu,
>>  		break;
>>  	}
>>  #endif /* CONFIG_KVM_XICS */
>> +#ifdef CONFIG_KVM_XIVE
>> +	case KVM_CAP_PPC_IRQ_XIVE: {
>> +		struct fd f;
>> +		struct kvm_device *dev;
>> +
>> +		r = -EBADF;
>> +		f = fdget(cap->args[0]);
>> +		if (!f.file)
>> +			break;
>> +
>> +		r = -ENXIO;
>> +		if (!xive_enabled())
>> +			break;
>> +
>> +		r = -EPERM;
>> +		dev = kvm_device_from_filp(f.file);
>> +		if (dev)
>> +			r = kvmppc_xive_native_connect_vcpu(dev, vcpu,
>> +							    cap->args[1]);
>> +
>> +		fdput(f);
>> +		break;
>> +	}
>> +#endif /* CONFIG_KVM_XIVE */
>>  #ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
>>  	case KVM_CAP_PPC_FWNMI:
>>  		r = -EINVAL;
>> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
>> index 356156f5c52d..1db1435769b4 100644
>> --- a/Documentation/virtual/kvm/api.txt
>> +++ b/Documentation/virtual/kvm/api.txt
>> @@ -4458,6 +4458,15 @@ struct kvm_sync_regs {
>>          struct kvm_vcpu_events events;
>>  };
>>  
>> +6.75 KVM_CAP_PPC_IRQ_XIVE
>> +
>> +Architectures: ppc
>> +Target: vcpu
>> +Parameters: args[0] is the XIVE device fd
>> +            args[1] is the XIVE CPU number (server ID) for this vcpu
>> +
>> +This capability connects the vcpu to an in-kernel XIVE device.
>> +
>>  7. Capabilities that can be enabled on VMs
>>  ------------------------------------------
>>  
> 


^ permalink raw reply

* Re: [PATCH] powerpc/6xx: fix setup and use of SPRN_PGDIR for hash32
From: Michael Ellerman @ 2019-03-18  9:54 UTC (permalink / raw)
  To: Guenter Roeck, Christophe Leroy
  Cc: linuxppc-dev, Paul Mackerras, linux-kernel
In-Reply-To: <fac535c2-af61-1a1b-ebed-82ca0bd2e4a4@roeck-us.net>

Guenter Roeck <linux@roeck-us.net> writes:
> On 3/15/19 6:20 AM, Christophe Leroy wrote:
>> Michael,
>> 
>> Are you able to get this merged before 5.1-rc1 comes out ?
>
> Looks like this patch got lost. I don't see it in Michael's most recent
> pull request, the one that got merged today.

Sorry, will pick it up for fixes now.

cheers

^ permalink raw reply

* Re: [PATCH] crypto: vmx - fix copy-paste error in CTR mode
From: Ard Biesheuvel @ 2019-03-18  9:13 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: leo.barbosa, Herbert Xu, Stephan Mueller, nayna, omosnacek,
	Eric Biggers, marcelo.cerri, pfsmorigo,
	open list:HARDWARE RANDOM NUMBER GENERATOR CORE, leitao,
	linuxppc-dev, Daniel Axtens
In-Reply-To: <87pnqo7ewp.fsf@concordia.ellerman.id.au>

On Mon, 18 Mar 2019 at 09:41, Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> Eric Biggers <ebiggers@kernel.org> writes:
> > On Fri, Mar 15, 2019 at 03:24:35PM +1100, Daniel Axtens wrote:
> ...
> >> >> This leads to corruption of the IV, which leads to subsequent blocks
> >> >> being corrupted.
> >> >>
> >> >> This can be detected with libkcapi test suite, which is available at
> >> >> https://github.com/smuellerDD/libkcapi
> >> >
> >> > Is this also detected by the kernel's crypto self-tests, and if not why not?
> >> > What about with the new option CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y?
> >>
> >> It seems the self-tests do not catch it. To catch it, there has to be a
> >> test where the blkcipher_walk creates a walk.nbytes such that
> >> [(the number of AES blocks) mod 8] is either 2 or 3. This happens with
> >> AF_ALG pretty frequently, but when I booted with self-tests it only hit
> >> 1, 4, 5, 6 and 7 - it missed 0, 2 and 3.
> >>
> >> I don't have the EXTRA_TESTS option - I'm testing with 5.0-rc6. Is it in
> >> -next?
> >
> > The improvements I recently made to the self-tests are intended to catch exactly
> > this sort of bug.  They were just merged for v5.1, so try the latest mainline.
> > This almost certainly would be caught by EXTRA_TESTS (and if not I'd want to
> > know), but it may be caught by the regular self-tests now too.
>
> Enabling the crypto tests (CONFIG_CRYPTO_MANAGER_DISABLE_TESTS=n)
> actually hides the bug for me.
>
> By which I mean I can't trigger the bug via kcapi-enc-tests.sh, because
> the VMX code is never called.
>
> ie:
>   # zgrep -e CRYPTO_MANAGER -e VMX /proc/config.gz
>   CONFIG_CRYPTO_MANAGER=y
>   CONFIG_CRYPTO_MANAGER2=y
>   # CONFIG_CRYPTO_MANAGER_DISABLE_TESTS is not set
>   # CONFIG_CRYPTO_MANAGER_EXTRA_TESTS is not set
>   CONFIG_CRYPTO_DEV_VMX=y
>   CONFIG_CRYPTO_DEV_VMX_ENCRYPT=y
>
>   # echo "p:p8_aes_ctr_crypt p8_aes_ctr_crypt" > /sys/kernel/debug/tracing/kprobe_events
>   # echo 1 > /sys/kernel/debug/tracing/events/kprobes/enable
>   # ./kcapi-enc-test.sh
>   ...
>   Number of failures: 0
>   # grep -c p8_aes_ctr_crypt  /sys/kernel/debug/tracing/trace
>   0
>
>
> I don't understand how the crypto core chooses which crypto_alg to use,
> but I didn't expect enabling the tests to change it?
>

This is not entirely unexpected. Based on the tests, algos that are
found to be broken are disregarded for further use, and you should see
a warning in the kernel log about this.

^ permalink raw reply

* Re: [PATCH] crypto: vmx - fix copy-paste error in CTR mode
From: Michael Ellerman @ 2019-03-18  8:41 UTC (permalink / raw)
  To: Eric Biggers, Daniel Axtens
  Cc: leo.barbosa, Herbert Xu, Stephan Mueller, nayna, omosnacek,
	marcelo.cerri, pfsmorigo, linux-crypto, leitao, linuxppc-dev
In-Reply-To: <20190315043433.GC1671@sol.localdomain>

Eric Biggers <ebiggers@kernel.org> writes:
> On Fri, Mar 15, 2019 at 03:24:35PM +1100, Daniel Axtens wrote:
...
>> >> This leads to corruption of the IV, which leads to subsequent blocks
>> >> being corrupted.
>> >> 
>> >> This can be detected with libkcapi test suite, which is available at
>> >> https://github.com/smuellerDD/libkcapi
>> >
>> > Is this also detected by the kernel's crypto self-tests, and if not why not?
>> > What about with the new option CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y?
>> 
>> It seems the self-tests do not catch it. To catch it, there has to be a
>> test where the blkcipher_walk creates a walk.nbytes such that
>> [(the number of AES blocks) mod 8] is either 2 or 3. This happens with
>> AF_ALG pretty frequently, but when I booted with self-tests it only hit
>> 1, 4, 5, 6 and 7 - it missed 0, 2 and 3.
>> 
>> I don't have the EXTRA_TESTS option - I'm testing with 5.0-rc6. Is it in
>> -next?
>
> The improvements I recently made to the self-tests are intended to catch exactly
> this sort of bug.  They were just merged for v5.1, so try the latest mainline.
> This almost certainly would be caught by EXTRA_TESTS (and if not I'd want to
> know), but it may be caught by the regular self-tests now too.

Enabling the crypto tests (CONFIG_CRYPTO_MANAGER_DISABLE_TESTS=n)
actually hides the bug for me.

By which I mean I can't trigger the bug via kcapi-enc-tests.sh, because
the VMX code is never called.

ie:
  # zgrep -e CRYPTO_MANAGER -e VMX /proc/config.gz
  CONFIG_CRYPTO_MANAGER=y
  CONFIG_CRYPTO_MANAGER2=y
  # CONFIG_CRYPTO_MANAGER_DISABLE_TESTS is not set
  # CONFIG_CRYPTO_MANAGER_EXTRA_TESTS is not set
  CONFIG_CRYPTO_DEV_VMX=y
  CONFIG_CRYPTO_DEV_VMX_ENCRYPT=y

  # echo "p:p8_aes_ctr_crypt p8_aes_ctr_crypt" > /sys/kernel/debug/tracing/kprobe_events
  # echo 1 > /sys/kernel/debug/tracing/events/kprobes/enable
  # ./kcapi-enc-test.sh
  ...
  Number of failures: 0
  # grep -c p8_aes_ctr_crypt  /sys/kernel/debug/tracing/trace
  0


I don't understand how the crypto core chooses which crypto_alg to use,
but I didn't expect enabling the tests to change it?

cheers

^ permalink raw reply

* Re: [PATCH v2 10/45] drivers: tty: serial: zs: use devm_* functions
From: Maciej W. Rozycki @ 2019-03-18  8:03 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult
  Cc: linux-arm-msm, yamada.masahiro, jacmet, festevam, stefan.wahren,
	f.fainelli, bcm-kernel-feedback-list, linux-imx, linux-serial,
	u.kleine-koenig, andy.gross, tklauser, david.brown, rjui, s.hauer,
	slemieux.tyco, linuxppc-dev, vz, matthias.bgg, andriy.shevchenko,
	Enrico Weigelt, metux IT consult, baohua, sbranden, eric,
	richard.genoud, Greg KH, linux-kernel, kernel, shawnguo
In-Reply-To: <fd8d6a9b-3e85-9904-2195-1f1bbc42bd2d@metux.net>

On Sat, 16 Mar 2019, Enrico Weigelt, metux IT consult wrote:

> > No, it's just that those systems do not allow those devices to be
> > removed because they are probably not on a removable bus.
> 
> Ok, devices (hw) might not be removable - that also the case for uarts
> builtin some SoCs, or the good old PC w/ 8250. But does that also mean
> that the driver should not be removable ?
> 
> IMHO, even if that's the case, it's still inconsistent. The driver then
> shouldn't support a remove at all (or even builtin only), not just
> incomplete remove.

 This device (as well as `dz') is typically used for the serial console as 
well, so being built-in is the usual configuration.  Nevertheless modular 
operation is supposed to be supported, however it may not have been 
verified for ages.

 A further complication is in the virtual console configuration one of the 
serial lines is dedicated for the keyboard, so again you want the driver 
built-in (although hooking up the virtual console keyboard this way has 
been broken with the conversion to the serial core in the 2.6 timeframe 
and I have never figured it out how it is supposed to be done correctly 
with the new serial infrastructure and SERIO_SERPORT; I believe some 
platforms do it with the use of horrible hacks rather than SERIO_SERPORT).

  Maciej

^ permalink raw reply

* Re: [PATCH v2 07/45] drivers: tty: serial: 8250_uniphier: use devm_ioremap_resource()
From: Masahiro Yamada @ 2019-03-18  7:44 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult
  Cc: linux-arm-msm, David Brown, Maciej W. Rozycki, jacmet,
	Fabio Estevam, Stefan Wahren, Florian Fainelli,
	Broadcom Kernel Feedback List, linux-imx, linux-serial,
	Uwe Kleine-König, Andy Gross, Tobias Klauser, Ray Jui,
	Sascha Hauer, Sylvain Lemieux, linuxppc-dev, Vladimir Zapolskiy,
	Matthias Brugger, Andy Shevchenko, Barry Song, Scott Branden,
	Eric Anholt, Richard Genoud, Greg Kroah-Hartman,
	Linux Kernel Mailing List, Sascha Hauer, Shawn Guo
In-Reply-To: <1552602855-26086-8-git-send-email-info@metux.net>

On Fri, Mar 15, 2019 at 7:35 AM Enrico Weigelt, metux IT consult
<info@metux.net> wrote:
>
> Instead of fetching out data from a struct resource for passing
> it to devm_ioremap(), directly use devm_ioremap_resource()
>
> Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
> ---


NACK.


This patch would break my driver.

Probably, all the 8250* drivers would be broken in the same way.


For 8250 drivers, request_mem_region() is called
from 8250_port.c


Let's not touch around the code you do not
understand how it works.


Thanks.

Masahiro Yamada



>  drivers/tty/serial/8250/8250_uniphier.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/8250/8250_uniphier.c b/drivers/tty/serial/8250/8250_uniphier.c
> index 164ba13..9c1244e 100644
> --- a/drivers/tty/serial/8250/8250_uniphier.c
> +++ b/drivers/tty/serial/8250/8250_uniphier.c
> @@ -171,7 +171,7 @@ static int uniphier_uart_probe(struct platform_device *pdev)
>                 return -EINVAL;
>         }
>
> -       membase = devm_ioremap(dev, regs->start, resource_size(regs));
> +       membase = devm_ioremap_resource(dev, regs);
>         if (!membase)
>                 return -ENOMEM;
>
> --
> 1.9.1
>

^ permalink raw reply

* Re: [PATCH v7 4/4] hugetlb: allow to free gigantic pages regardless of the configuration
From: Alex Ghiti @ 2019-03-18  7:00 UTC (permalink / raw)
  To: christophe leroy, aneesh.kumar, mpe, Andrew Morton,
	Vlastimil Babka, Catalin Marinas, Will Deacon,
	Benjamin Herrenschmidt, Paul Mackerras, Martin Schwidefsky,
	Heiko Carstens, Yoshinori Sato, Rich Felker, David S . Miller,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, H . Peter Anvin,
	x86, Dave Hansen, Andy Lutomirski, Peter Zijlstra, Mike Kravetz,
	linux-arm-kernel, linux-kernel, linuxppc-dev, linux-s390,
	linux-sh, sparclinux, linux-mm
In-Reply-To: <f434892d-80b2-f09d-31d6-754a1be0b97a@c-s.fr>

On 3/17/19 2:31 PM, christophe leroy wrote:
>
>
> Le 17/03/2019 à 17:28, Alexandre Ghiti a écrit :
>> On systems without CONTIG_ALLOC activated but that support gigantic 
>> pages,
>> boottime reserved gigantic pages can not be freed at all. This patch
>> simply enables the possibility to hand back those pages to memory
>> allocator.
>>
>> Signed-off-by: Alexandre Ghiti <alex@ghiti.fr>
>> Acked-by: David S. Miller <davem@davemloft.net> [sparc]
>> ---
>>   arch/arm64/Kconfig                           |  2 +-
>>   arch/arm64/include/asm/hugetlb.h             |  4 --
>>   arch/powerpc/include/asm/book3s/64/hugetlb.h |  7 ---
>>   arch/powerpc/platforms/Kconfig.cputype       |  2 +-
>>   arch/s390/Kconfig                            |  2 +-
>>   arch/s390/include/asm/hugetlb.h              |  3 --
>>   arch/sh/Kconfig                              |  2 +-
>>   arch/sparc/Kconfig                           |  2 +-
>>   arch/x86/Kconfig                             |  2 +-
>>   arch/x86/include/asm/hugetlb.h               |  4 --
>>   include/asm-generic/hugetlb.h                | 14 +++++
>>   include/linux/gfp.h                          |  2 +-
>>   mm/hugetlb.c                                 | 54 ++++++++++++++------
>>   mm/page_alloc.c                              |  4 +-
>>   14 files changed, 61 insertions(+), 43 deletions(-)
>>
>
> [...]
>
>> diff --git a/include/asm-generic/hugetlb.h 
>> b/include/asm-generic/hugetlb.h
>> index 71d7b77eea50..aaf14974ee5f 100644
>> --- a/include/asm-generic/hugetlb.h
>> +++ b/include/asm-generic/hugetlb.h
>> @@ -126,4 +126,18 @@ static inline pte_t huge_ptep_get(pte_t *ptep)
>>   }
>>   #endif
>>   +#ifndef __HAVE_ARCH_GIGANTIC_PAGE_RUNTIME_SUPPORTED
>> +#ifdef CONFIG_ARCH_HAS_GIGANTIC_PAGE
>> +static inline bool gigantic_page_runtime_supported(void)
>> +{
>> +    return true;
>> +}
>> +#else
>> +static inline bool gigantic_page_runtime_supported(void)
>> +{
>> +    return false;
>> +}
>> +#endif /* CONFIG_ARCH_HAS_GIGANTIC_PAGE */
>
> What about the following instead:
>
> static inline bool gigantic_page_runtime_supported(void)
> {
>     return IS_ENABLED(CONFIG_ARCH_HAS_GIGANTIC_PAGE);
> }
>

Totally, it already was like that in v2 or v3...


>
>> +#endif /* __HAVE_ARCH_GIGANTIC_PAGE_RUNTIME_SUPPORTED */
>> +
>>   #endif /* _ASM_GENERIC_HUGETLB_H */
>> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
>> index 1f1ad9aeebb9..58ea44bf75de 100644
>> --- a/include/linux/gfp.h
>> +++ b/include/linux/gfp.h
>> @@ -589,8 +589,8 @@ static inline bool pm_suspended_storage(void)
>>   /* The below functions must be run on a range from a single zone. */
>>   extern int alloc_contig_range(unsigned long start, unsigned long end,
>>                     unsigned migratetype, gfp_t gfp_mask);
>> -extern void free_contig_range(unsigned long pfn, unsigned nr_pages);
>>   #endif
>> +extern void free_contig_range(unsigned long pfn, unsigned int 
>> nr_pages);
>
> 'extern' is unneeded and should be avoided (iaw checkpatch)
>

Ok, I did fix a checkpatch warning here, but did not notice the 'extern' 
one.


Thanks for your time,


Alex


> Christophe
>
>>     #ifdef CONFIG_CMA
>>   /* CMA stuff */
>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>> index afef61656c1e..4e55aa38704f 100644
>> --- a/mm/hugetlb.c
>> +++ b/mm/hugetlb.c
>> @@ -1058,6 +1058,7 @@ static void free_gigantic_page(struct page 
>> *page, unsigned int order)
>>       free_contig_range(page_to_pfn(page), 1 << order);
>>   }
>>   +#ifdef CONFIG_CONTIG_ALLOC
>>   static int __alloc_gigantic_page(unsigned long start_pfn,
>>                   unsigned long nr_pages, gfp_t gfp_mask)
>>   {
>> @@ -1142,11 +1143,20 @@ static struct page 
>> *alloc_gigantic_page(struct hstate *h, gfp_t gfp_mask,
>>     static void prep_new_huge_page(struct hstate *h, struct page 
>> *page, int nid);
>>   static void prep_compound_gigantic_page(struct page *page, unsigned 
>> int order);
>> +#else /* !CONFIG_CONTIG_ALLOC */
>> +static struct page *alloc_gigantic_page(struct hstate *h, gfp_t 
>> gfp_mask,
>> +                    int nid, nodemask_t *nodemask)
>> +{
>> +    return NULL;
>> +}
>> +#endif /* CONFIG_CONTIG_ALLOC */
>>     #else /* !CONFIG_ARCH_HAS_GIGANTIC_PAGE */
>> -static inline bool gigantic_page_supported(void) { return false; }
>>   static struct page *alloc_gigantic_page(struct hstate *h, gfp_t 
>> gfp_mask,
>> -        int nid, nodemask_t *nodemask) { return NULL; }
>> +                    int nid, nodemask_t *nodemask)
>> +{
>> +    return NULL;
>> +}
>>   static inline void free_gigantic_page(struct page *page, unsigned 
>> int order) { }
>>   static inline void destroy_compound_gigantic_page(struct page *page,
>>                           unsigned int order) { }
>> @@ -1156,7 +1166,7 @@ static void update_and_free_page(struct hstate 
>> *h, struct page *page)
>>   {
>>       int i;
>>   -    if (hstate_is_gigantic(h) && !gigantic_page_supported())
>> +    if (hstate_is_gigantic(h) && !gigantic_page_runtime_supported())
>>           return;
>>         h->nr_huge_pages--;
>> @@ -2276,13 +2286,27 @@ static int adjust_pool_surplus(struct hstate 
>> *h, nodemask_t *nodes_allowed,
>>   }
>>     #define persistent_huge_pages(h) (h->nr_huge_pages - 
>> h->surplus_huge_pages)
>> -static unsigned long set_max_huge_pages(struct hstate *h, unsigned 
>> long count,
>> -                        nodemask_t *nodes_allowed)
>> +static int set_max_huge_pages(struct hstate *h, unsigned long count,
>> +                  nodemask_t *nodes_allowed)
>>   {
>>       unsigned long min_count, ret;
>>   -    if (hstate_is_gigantic(h) && !gigantic_page_supported())
>> -        return h->max_huge_pages;
>> +    spin_lock(&hugetlb_lock);
>> +
>> +    /*
>> +     * Gigantic pages runtime allocation depend on the capability 
>> for large
>> +     * page range allocation.
>> +     * If the system does not provide this feature, return an error 
>> when
>> +     * the user tries to allocate gigantic pages but let the user 
>> free the
>> +     * boottime allocated gigantic pages.
>> +     */
>> +    if (hstate_is_gigantic(h) && !IS_ENABLED(CONFIG_CONTIG_ALLOC)) {
>> +        if (count > persistent_huge_pages(h)) {
>> +            spin_unlock(&hugetlb_lock);
>> +            return -EINVAL;
>> +        }
>> +        /* Fall through to decrease pool */
>> +    }
>>         /*
>>        * Increase the pool size
>> @@ -2295,7 +2319,6 @@ static unsigned long set_max_huge_pages(struct 
>> hstate *h, unsigned long count,
>>        * pool might be one hugepage larger than it needs to be, but
>>        * within all the constraints specified by the sysctls.
>>        */
>> -    spin_lock(&hugetlb_lock);
>>       while (h->surplus_huge_pages && count > 
>> persistent_huge_pages(h)) {
>>           if (!adjust_pool_surplus(h, nodes_allowed, -1))
>>               break;
>> @@ -2350,9 +2373,10 @@ static unsigned long set_max_huge_pages(struct 
>> hstate *h, unsigned long count,
>>               break;
>>       }
>>   out:
>> -    ret = persistent_huge_pages(h);
>> +    h->max_huge_pages = persistent_huge_pages(h);
>>       spin_unlock(&hugetlb_lock);
>> -    return ret;
>> +
>> +    return 0;
>>   }
>>     #define HSTATE_ATTR_RO(_name) \
>> @@ -2404,7 +2428,7 @@ static ssize_t __nr_hugepages_store_common(bool 
>> obey_mempolicy,
>>       int err;
>>       NODEMASK_ALLOC(nodemask_t, nodes_allowed, GFP_KERNEL | 
>> __GFP_NORETRY);
>>   -    if (hstate_is_gigantic(h) && !gigantic_page_supported()) {
>> +    if (hstate_is_gigantic(h) && !gigantic_page_runtime_supported()) {
>>           err = -EINVAL;
>>           goto out;
>>       }
>> @@ -2428,15 +2452,13 @@ static ssize_t 
>> __nr_hugepages_store_common(bool obey_mempolicy,
>>       } else
>>           nodes_allowed = &node_states[N_MEMORY];
>>   -    h->max_huge_pages = set_max_huge_pages(h, count, nodes_allowed);
>> +    err = set_max_huge_pages(h, count, nodes_allowed);
>>   +out:
>>       if (nodes_allowed != &node_states[N_MEMORY])
>>           NODEMASK_FREE(nodes_allowed);
>>   -    return len;
>> -out:
>> -    NODEMASK_FREE(nodes_allowed);
>> -    return err;
>> +    return err ? err : len;
>>   }
>>     static ssize_t nr_hugepages_store_common(bool obey_mempolicy,
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index ac9c45ffb344..a4547d90fa7a 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -8234,8 +8234,9 @@ int alloc_contig_range(unsigned long start, 
>> unsigned long end,
>>                   pfn_max_align_up(end), migratetype);
>>       return ret;
>>   }
>> +#endif /* CONFIG_CONTIG_ALLOC */
>>   -void free_contig_range(unsigned long pfn, unsigned nr_pages)
>> +void free_contig_range(unsigned long pfn, unsigned int nr_pages)
>>   {
>>       unsigned int count = 0;
>>   @@ -8247,7 +8248,6 @@ void free_contig_range(unsigned long pfn, 
>> unsigned nr_pages)
>>       }
>>       WARN(count != 0, "%d pages are still in use!\n", count);
>>   }
>> -#endif
>>     #ifdef CONFIG_MEMORY_HOTPLUG
>>   /*
>>
>
> ---
> L'absence de virus dans ce courrier électronique a été vérifiée par le 
> logiciel antivirus Avast.
> https://www.avast.com/antivirus
>

^ permalink raw reply

* Re: [PATCH v3 16/17] KVM: introduce a KVM_DESTROY_DEVICE ioctl
From: David Gibson @ 2019-03-18  6:42 UTC (permalink / raw)
  To: Cédric Le Goater
  Cc: kvm, kvm-ppc, Paul Mackerras, Paolo Bonzini, linuxppc-dev
In-Reply-To: <20190315120609.25910-17-clg@kaod.org>

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

On Fri, Mar 15, 2019 at 01:06:08PM +0100, Cédric Le Goater wrote:
> The 'destroy' method is currently used to destroy all devices when the
> VM is destroyed after the vCPUs have been freed.
> 
> This new KVM ioctl exposes the same KVM device method. It acts as a
> software reset of the VM to 'destroy' selected devices when necessary
> and perform the required cleanups on the vCPUs. Called with the
> kvm->lock.
> 
> The 'destroy' method could be improved by returning an error code.
> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
> 
>   Changes since v2 :
> 
>  - checked that device is owned by VM
>  
>  include/uapi/linux/kvm.h          |  7 ++++++
>  virt/kvm/kvm_main.c               | 42 +++++++++++++++++++++++++++++++
>  Documentation/virtual/kvm/api.txt | 20 +++++++++++++++
>  3 files changed, 69 insertions(+)
> 
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 52bf74a1616e..d78fafa54274 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -1183,6 +1183,11 @@ struct kvm_create_device {
>  	__u32	flags;	/* in: KVM_CREATE_DEVICE_xxx */
>  };
>  
> +struct kvm_destroy_device {
> +	__u32	fd;	/* in: device handle */
> +	__u32	flags;	/* in: unused */
> +};
> +
>  struct kvm_device_attr {
>  	__u32	flags;		/* no flags currently defined */
>  	__u32	group;		/* device-defined */
> @@ -1331,6 +1336,8 @@ struct kvm_s390_ucas_mapping {
>  #define KVM_GET_DEVICE_ATTR	  _IOW(KVMIO,  0xe2, struct kvm_device_attr)
>  #define KVM_HAS_DEVICE_ATTR	  _IOW(KVMIO,  0xe3, struct kvm_device_attr)
>  
> +#define KVM_DESTROY_DEVICE	  _IOWR(KVMIO,  0xf0, struct kvm_destroy_device)
> +
>  /*
>   * ioctls for vcpu fds
>   */
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index e4881a8c2a6f..7b616a1d48cf 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -3026,6 +3026,34 @@ static int kvm_ioctl_create_device(struct kvm *kvm,
>  	return 0;
>  }
>  
> +static int kvm_ioctl_destroy_device(struct kvm *kvm,
> +				    struct kvm_destroy_device *dd)
> +{
> +	struct fd f;
> +	struct kvm_device *dev;
> +
> +	f = fdget(dd->fd);
> +	if (!f.file)
> +		return -EBADF;
> +
> +	dev = kvm_device_from_filp(f.file);
> +	fdput(f);
> +
> +	if (!dev)
> +		return -ENODEV;
> +
> +	if (dev->kvm != kvm)
> +		return -EPERM;
> +
> +	mutex_lock(&kvm->lock);
> +	list_del(&dev->vm_node);
> +	dev->ops->destroy(dev);
> +	mutex_unlock(&kvm->lock);
> +
> +	/* TODO: kvm_put_kvm() crashes the host on some occasion ? */
> +	return 0;
> +}
> +
>  static long kvm_vm_ioctl_check_extension_generic(struct kvm *kvm, long arg)
>  {
>  	switch (arg) {
> @@ -3270,6 +3298,20 @@ static long kvm_vm_ioctl(struct file *filp,
>  		r = 0;
>  		break;
>  	}
> +	case KVM_DESTROY_DEVICE: {
> +		struct kvm_destroy_device dd;
> +
> +		r = -EFAULT;
> +		if (copy_from_user(&dd, argp, sizeof(dd)))
> +			goto out;
> +
> +		r = kvm_ioctl_destroy_device(kvm, &dd);
> +		if (r)
> +			goto out;
> +
> +		r = 0;
> +		break;
> +	}
>  	case KVM_CHECK_EXTENSION:
>  		r = kvm_vm_ioctl_check_extension_generic(kvm, arg);
>  		break;
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index 1db1435769b4..914471494602 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -3857,6 +3857,26 @@ number of valid entries in the 'entries' array, which is then filled.
>  'index' and 'flags' fields in 'struct kvm_cpuid_entry2' are currently reserved,
>  userspace should not expect to get any particular value there.
>  
> +4.119 KVM_DESTROY_DEVICE
> +
> +Capability: KVM_CAP_DEVICE_CTRL
> +Type: vm ioctl
> +Parameters: struct kvm_destroy_device (in)
> +Returns: 0 on success, -1 on error
> +Errors:
> +  ENODEV: The device type is unknown or unsupported
> +  EPERM: The device does not belong to the VM
> +
> +  Other error conditions may be defined by individual device types or
> +  have their standard meanings.
> +
> +Destroys an emulated device in the kernel.
> +
> +struct kvm_destroy_device {
> +	__u32	fd;	/* in: device handle */
> +	__u32	flags;	/* unused */
> +};
> +
>  5. The kvm_run structure
>  ------------------------
>  

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH v3 15/17] KVM: PPC: Book3S HV: XIVE: activate XIVE exploitation mode
From: David Gibson @ 2019-03-18  6:42 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: linuxppc-dev, Paul Mackerras, kvm, kvm-ppc
In-Reply-To: <20190315120609.25910-16-clg@kaod.org>

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

On Fri, Mar 15, 2019 at 01:06:07PM +0100, Cédric Le Goater wrote:
> Full support for the XIVE native exploitation mode is now available,
> advertise the capability KVM_CAP_PPC_IRQ_XIVE for guests running on
> PowerNV KVM Hypervisors only. Support for nested guests (pseries KVM
> Hypervisor) is not yet available. XIVE should also have been activated
> which is default setting on POWER9 systems running a recent Linux
> kernel.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  arch/powerpc/kvm/powerpc.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> index bb51faf29162..d70b19f8725b 100644
> --- a/arch/powerpc/kvm/powerpc.c
> +++ b/arch/powerpc/kvm/powerpc.c
> @@ -573,10 +573,11 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>  #ifdef CONFIG_KVM_XIVE
>  	case KVM_CAP_PPC_IRQ_XIVE:
>  		/*
> -		 * Return false until all the XIVE infrastructure is
> -		 * in place including support for migration.
> +		 * We need XIVE to be enabled on the platform (implies
> +		 * a POWER9 processor) and the PowerNV platform, as
> +		 * nested is not yet supported.
>  		 */
> -		r = 0;
> +		r = xive_enabled() && !!cpu_has_feature(CPU_FTR_HVMODE);

Nit: && is already a logical operation, so there should be no need for
the '!!' here.

>  		break;
>  #endif
>  

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH] crypto: vmx - fix copy-paste error in CTR mode
From: Michael Ellerman @ 2019-03-18  6:03 UTC (permalink / raw)
  To: Daniel Axtens, omosnacek, linux-crypto, Herbert Xu
  Cc: leo.barbosa, Stephan Mueller, nayna, marcelo.cerri, pfsmorigo,
	leitao, linuxppc-dev
In-Reply-To: <20190315020901.16509-1-dja@axtens.net>

Daniel Axtens <dja@axtens.net> writes:
> The original assembly imported from OpenSSL has two copy-paste
> errors in handling CTR mode. When dealing with a 2 or 3 block tail,
> the code branches to the CBC decryption exit path, rather than to
> the CTR exit path.
>
> This leads to corruption of the IV, which leads to subsequent blocks
> being corrupted.
>
> This can be detected with libkcapi test suite, which is available at
> https://github.com/smuellerDD/libkcapi
>
> Reported-by: Ondrej Mosnáček <omosnacek@gmail.com>
> Fixes: 5c380d623ed3 ("crypto: vmx - Add support for VMS instructions by ASM")
> Cc: stable@vger.kernel.org
> Signed-off-by: Daniel Axtens <dja@axtens.net>

Thanks, this fixes kcapi-enc-test.sh for me.

Tested-by: Michael Ellerman <mpe@ellerman.id.au>

cheers

^ permalink raw reply

* Re: [PATCH v3 08/17] KVM: PPC: Book3S HV: XIVE: add a control to sync the sources
From: David Gibson @ 2019-03-18  3:28 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: linuxppc-dev, Paul Mackerras, kvm, kvm-ppc
In-Reply-To: <20190315120609.25910-9-clg@kaod.org>

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

On Fri, Mar 15, 2019 at 01:06:00PM +0100, Cédric Le Goater wrote:
> This control will be used by the H_INT_SYNC hcall from QEMU to flush
> event notifications on the XIVE IC owning the source.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
> 
>  Changes since v2 :
> 
>  - fixed locking on source block
> 
>  arch/powerpc/include/uapi/asm/kvm.h        |  1 +
>  arch/powerpc/kvm/book3s_xive_native.c      | 36 ++++++++++++++++++++++
>  Documentation/virtual/kvm/devices/xive.txt |  8 +++++
>  3 files changed, 45 insertions(+)
> 
> diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
> index 95e82ab57c03..fc9211dbfec8 100644
> --- a/arch/powerpc/include/uapi/asm/kvm.h
> +++ b/arch/powerpc/include/uapi/asm/kvm.h
> @@ -681,6 +681,7 @@ struct kvm_ppc_cpu_char {
>  #define KVM_DEV_XIVE_GRP_SOURCE		2	/* 64-bit source identifier */
>  #define KVM_DEV_XIVE_GRP_SOURCE_CONFIG	3	/* 64-bit source identifier */
>  #define KVM_DEV_XIVE_GRP_EQ_CONFIG	4	/* 64-bit EQ identifier */
> +#define KVM_DEV_XIVE_GRP_SOURCE_SYNC	5       /* 64-bit source identifier */
>  
>  /* Layout of 64-bit XIVE source attribute values */
>  #define KVM_XIVE_LEVEL_SENSITIVE	(1ULL << 0)
> diff --git a/arch/powerpc/kvm/book3s_xive_native.c b/arch/powerpc/kvm/book3s_xive_native.c
> index 3385c336fd89..26ac3c505cd2 100644
> --- a/arch/powerpc/kvm/book3s_xive_native.c
> +++ b/arch/powerpc/kvm/book3s_xive_native.c
> @@ -340,6 +340,38 @@ static int kvmppc_xive_native_set_source_config(struct kvmppc_xive *xive,
>  						       priority, masked, eisn);
>  }
>  
> +static int kvmppc_xive_native_sync_source(struct kvmppc_xive *xive,
> +					  long irq, u64 addr)
> +{
> +	struct kvmppc_xive_src_block *sb;
> +	struct kvmppc_xive_irq_state *state;
> +	struct xive_irq_data *xd;
> +	u32 hw_num;
> +	u16 src;
> +	int rc = 0;
> +
> +	pr_devel("%s irq=0x%lx", __func__, irq);
> +
> +	sb = kvmppc_xive_find_source(xive, irq, &src);
> +	if (!sb)
> +		return -ENOENT;
> +
> +	state = &sb->irq_state[src];
> +
> +	rc = -EINVAL;
> +
> +	arch_spin_lock(&sb->lock);
> +
> +	if (state->valid) {
> +		kvmppc_xive_select_irq(state, &hw_num, &xd);
> +		xive_native_sync_source(hw_num);
> +		rc = 0;
> +	}
> +
> +	arch_spin_unlock(&sb->lock);
> +	return rc;
> +}
> +
>  static int xive_native_validate_queue_size(u32 qsize)
>  {
>  	/*
> @@ -658,6 +690,9 @@ static int kvmppc_xive_native_set_attr(struct kvm_device *dev,
>  	case KVM_DEV_XIVE_GRP_EQ_CONFIG:
>  		return kvmppc_xive_native_set_queue_config(xive, attr->attr,
>  							   attr->addr);
> +	case KVM_DEV_XIVE_GRP_SOURCE_SYNC:
> +		return kvmppc_xive_native_sync_source(xive, attr->attr,
> +						      attr->addr);
>  	}
>  	return -ENXIO;
>  }
> @@ -687,6 +722,7 @@ static int kvmppc_xive_native_has_attr(struct kvm_device *dev,
>  		break;
>  	case KVM_DEV_XIVE_GRP_SOURCE:
>  	case KVM_DEV_XIVE_GRP_SOURCE_CONFIG:
> +	case KVM_DEV_XIVE_GRP_SOURCE_SYNC:
>  		if (attr->attr >= KVMPPC_XIVE_FIRST_IRQ &&
>  		    attr->attr < KVMPPC_XIVE_NR_IRQS)
>  			return 0;
> diff --git a/Documentation/virtual/kvm/devices/xive.txt b/Documentation/virtual/kvm/devices/xive.txt
> index e1893d303ab7..055aed0c2abb 100644
> --- a/Documentation/virtual/kvm/devices/xive.txt
> +++ b/Documentation/virtual/kvm/devices/xive.txt
> @@ -89,3 +89,11 @@ the legacy interrupt mode, referred as XICS (POWER7/8).
>      -EINVAL: Invalid queue address
>      -EFAULT: Invalid user pointer for attr->addr.
>      -EIO:    Configuration of the underlying HW failed
> +
> +  5. KVM_DEV_XIVE_GRP_SOURCE_SYNC (write only)
> +  Synchronize the source to flush event notifications
> +  Attributes:
> +    Interrupt source number  (64-bit)
> +  Errors:
> +    -ENOENT: Unknown source number
> +    -EINVAL: Not initialized source number

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH v3 06/17] KVM: PPC: Book3S HV: XIVE: add controls for the EQ configuration
From: David Gibson @ 2019-03-18  3:23 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: linuxppc-dev, Paul Mackerras, kvm, kvm-ppc
In-Reply-To: <20190315120609.25910-7-clg@kaod.org>

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

On Fri, Mar 15, 2019 at 01:05:58PM +0100, Cédric Le Goater wrote:
> These controls will be used by the H_INT_SET_QUEUE_CONFIG and
> H_INT_GET_QUEUE_CONFIG hcalls from QEMU to configure the underlying
> Event Queue in the XIVE IC. They will also be used to restore the
> configuration of the XIVE EQs and to capture the internal run-time
> state of the EQs. Both 'get' and 'set' rely on an OPAL call to access
> the EQ toggle bit and EQ index which are updated by the XIVE IC when
> event notifications are enqueued in the EQ.
> 
> The value of the guest physical address of the event queue is saved in
> the XIVE internal xive_q structure for later use. That is when
> migration needs to mark the EQ pages dirty to capture a consistent
> memory state of the VM.
> 
> To be noted that H_INT_SET_QUEUE_CONFIG does not require the extra
> OPAL call setting the EQ toggle bit and EQ index to configure the EQ,
> but restoring the EQ state will.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> 
>  Changes since v2 :
>  
>  - fixed comments on the KVM device attribute definitions
>  - fixed check on supported EQ size to restrict to 64K pages
>  - checked kvm_eq.flags that need to be zero
>  - removed the OPAL call when EQ qtoggle bit and index are zero. 
> 
>  arch/powerpc/include/asm/xive.h            |   2 +
>  arch/powerpc/include/uapi/asm/kvm.h        |  21 ++
>  arch/powerpc/kvm/book3s_xive.h             |   2 +
>  arch/powerpc/kvm/book3s_xive.c             |  15 +-
>  arch/powerpc/kvm/book3s_xive_native.c      | 232 +++++++++++++++++++++
>  Documentation/virtual/kvm/devices/xive.txt |  31 +++
>  6 files changed, 297 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/xive.h b/arch/powerpc/include/asm/xive.h
> index b579a943407b..46891f321606 100644
> --- a/arch/powerpc/include/asm/xive.h
> +++ b/arch/powerpc/include/asm/xive.h
> @@ -73,6 +73,8 @@ struct xive_q {
>  	u32			esc_irq;
>  	atomic_t		count;
>  	atomic_t		pending_count;
> +	u64			guest_qpage;
> +	u32			guest_qsize;
>  };
>  
>  /* Global enable flags for the XIVE support */
> diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
> index 12bb01baf0ae..1cd728c87d7c 100644
> --- a/arch/powerpc/include/uapi/asm/kvm.h
> +++ b/arch/powerpc/include/uapi/asm/kvm.h
> @@ -679,6 +679,7 @@ struct kvm_ppc_cpu_char {
>  #define KVM_DEV_XIVE_GRP_CTRL		1
>  #define KVM_DEV_XIVE_GRP_SOURCE		2	/* 64-bit source identifier */
>  #define KVM_DEV_XIVE_GRP_SOURCE_CONFIG	3	/* 64-bit source identifier */
> +#define KVM_DEV_XIVE_GRP_EQ_CONFIG	4	/* 64-bit EQ identifier */
>  
>  /* Layout of 64-bit XIVE source attribute values */
>  #define KVM_XIVE_LEVEL_SENSITIVE	(1ULL << 0)
> @@ -694,4 +695,24 @@ struct kvm_ppc_cpu_char {
>  #define KVM_XIVE_SOURCE_EISN_SHIFT	33
>  #define KVM_XIVE_SOURCE_EISN_MASK	0xfffffffe00000000ULL
>  
> +/* Layout of 64-bit EQ identifier */
> +#define KVM_XIVE_EQ_PRIORITY_SHIFT	0
> +#define KVM_XIVE_EQ_PRIORITY_MASK	0x7
> +#define KVM_XIVE_EQ_SERVER_SHIFT	3
> +#define KVM_XIVE_EQ_SERVER_MASK		0xfffffff8ULL
> +
> +/* Layout of EQ configuration values (64 bytes) */
> +struct kvm_ppc_xive_eq {
> +	__u32 flags;
> +	__u32 qsize;
> +	__u64 qpage;
> +	__u32 qtoggle;
> +	__u32 qindex;
> +	__u8  pad[40];
> +};
> +
> +#define KVM_XIVE_EQ_FLAG_ENABLED	0x00000001
> +#define KVM_XIVE_EQ_FLAG_ALWAYS_NOTIFY	0x00000002
> +#define KVM_XIVE_EQ_FLAG_ESCALATE	0x00000004
> +
>  #endif /* __LINUX_KVM_POWERPC_H */
> diff --git a/arch/powerpc/kvm/book3s_xive.h b/arch/powerpc/kvm/book3s_xive.h
> index ae26fe653d98..622f594d93e1 100644
> --- a/arch/powerpc/kvm/book3s_xive.h
> +++ b/arch/powerpc/kvm/book3s_xive.h
> @@ -272,6 +272,8 @@ struct kvmppc_xive_src_block *kvmppc_xive_create_src_block(
>  	struct kvmppc_xive *xive, int irq);
>  void kvmppc_xive_free_sources(struct kvmppc_xive_src_block *sb);
>  int kvmppc_xive_select_target(struct kvm *kvm, u32 *server, u8 prio);
> +int kvmppc_xive_attach_escalation(struct kvm_vcpu *vcpu, u8 prio,
> +				  bool single_escalation);
>  
>  #endif /* CONFIG_KVM_XICS */
>  #endif /* _KVM_PPC_BOOK3S_XICS_H */
> diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
> index e09f3addffe5..c1b7aa7dbc28 100644
> --- a/arch/powerpc/kvm/book3s_xive.c
> +++ b/arch/powerpc/kvm/book3s_xive.c
> @@ -166,7 +166,8 @@ static irqreturn_t xive_esc_irq(int irq, void *data)
>  	return IRQ_HANDLED;
>  }
>  
> -static int xive_attach_escalation(struct kvm_vcpu *vcpu, u8 prio)
> +int kvmppc_xive_attach_escalation(struct kvm_vcpu *vcpu, u8 prio,
> +				  bool single_escalation)
>  {
>  	struct kvmppc_xive_vcpu *xc = vcpu->arch.xive_vcpu;
>  	struct xive_q *q = &xc->queues[prio];
> @@ -185,7 +186,7 @@ static int xive_attach_escalation(struct kvm_vcpu *vcpu, u8 prio)
>  		return -EIO;
>  	}
>  
> -	if (xc->xive->single_escalation)
> +	if (single_escalation)
>  		name = kasprintf(GFP_KERNEL, "kvm-%d-%d",
>  				 vcpu->kvm->arch.lpid, xc->server_num);
>  	else
> @@ -217,7 +218,7 @@ static int xive_attach_escalation(struct kvm_vcpu *vcpu, u8 prio)
>  	 * interrupt, thus leaving it effectively masked after
>  	 * it fires once.
>  	 */
> -	if (xc->xive->single_escalation) {
> +	if (single_escalation) {
>  		struct irq_data *d = irq_get_irq_data(xc->esc_virq[prio]);
>  		struct xive_irq_data *xd = irq_data_get_irq_handler_data(d);
>  
> @@ -291,7 +292,8 @@ static int xive_check_provisioning(struct kvm *kvm, u8 prio)
>  			continue;
>  		rc = xive_provision_queue(vcpu, prio);
>  		if (rc == 0 && !xive->single_escalation)
> -			xive_attach_escalation(vcpu, prio);
> +			kvmppc_xive_attach_escalation(vcpu, prio,
> +						      xive->single_escalation);
>  		if (rc)
>  			return rc;
>  	}
> @@ -1214,7 +1216,8 @@ int kvmppc_xive_connect_vcpu(struct kvm_device *dev,
>  		if (xive->qmap & (1 << i)) {
>  			r = xive_provision_queue(vcpu, i);
>  			if (r == 0 && !xive->single_escalation)
> -				xive_attach_escalation(vcpu, i);
> +				kvmppc_xive_attach_escalation(
> +					vcpu, i, xive->single_escalation);
>  			if (r)
>  				goto bail;
>  		} else {
> @@ -1229,7 +1232,7 @@ int kvmppc_xive_connect_vcpu(struct kvm_device *dev,
>  	}
>  
>  	/* If not done above, attach priority 0 escalation */
> -	r = xive_attach_escalation(vcpu, 0);
> +	r = kvmppc_xive_attach_escalation(vcpu, 0, xive->single_escalation);
>  	if (r)
>  		goto bail;
>  
> diff --git a/arch/powerpc/kvm/book3s_xive_native.c b/arch/powerpc/kvm/book3s_xive_native.c
> index b841d339f674..42e824658a30 100644
> --- a/arch/powerpc/kvm/book3s_xive_native.c
> +++ b/arch/powerpc/kvm/book3s_xive_native.c
> @@ -340,6 +340,226 @@ static int kvmppc_xive_native_set_source_config(struct kvmppc_xive *xive,
>  						       priority, masked, eisn);
>  }
>  
> +static int xive_native_validate_queue_size(u32 qsize)
> +{
> +	/*
> +	 * We only support 64K pages for the moment. This is also
> +	 * advertised in the DT property "ibm,xive-eq-sizes"

IIUC, that won't work properly if you had a guest using 4kiB pages.
That's fine, but do we have somewhere that checks for that case and
throws a suitable error?

> +	 */
> +	switch (qsize) {
> +	case 0: /* EQ reset */
> +	case 16:
> +		return 0;
> +	case 12:
> +	case 21:
> +	case 24:
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int kvmppc_xive_native_set_queue_config(struct kvmppc_xive *xive,
> +					       long eq_idx, u64 addr)
> +{
> +	struct kvm *kvm = xive->kvm;
> +	struct kvm_vcpu *vcpu;
> +	struct kvmppc_xive_vcpu *xc;
> +	void __user *ubufp = (u64 __user *) addr;

Nit: that should be (void __user *) on the right, shouldn't it?

> +	u32 server;
> +	u8 priority;
> +	struct kvm_ppc_xive_eq kvm_eq;
> +	int rc;
> +	__be32 *qaddr = 0;
> +	struct page *page;
> +	struct xive_q *q;
> +
> +	/*
> +	 * Demangle priority/server tuple from the EQ identifier
> +	 */
> +	priority = (eq_idx & KVM_XIVE_EQ_PRIORITY_MASK) >>
> +		KVM_XIVE_EQ_PRIORITY_SHIFT;
> +	server = (eq_idx & KVM_XIVE_EQ_SERVER_MASK) >>
> +		KVM_XIVE_EQ_SERVER_SHIFT;
> +
> +	if (copy_from_user(&kvm_eq, ubufp, sizeof(kvm_eq)))
> +		return -EFAULT;
> +
> +	vcpu = kvmppc_xive_find_server(kvm, server);
> +	if (!vcpu) {
> +		pr_err("Can't find server %d\n", server);
> +		return -ENOENT;
> +	}
> +	xc = vcpu->arch.xive_vcpu;
> +
> +	if (priority != xive_prio_from_guest(priority)) {
> +		pr_err("Trying to restore invalid queue %d for VCPU %d\n",
> +		       priority, server);
> +		return -EINVAL;
> +	}
> +	q = &xc->queues[priority];
> +
> +	pr_devel("%s VCPU %d priority %d fl:%x sz:%d addr:%llx g:%d idx:%d\n",
> +		 __func__, server, priority, kvm_eq.flags,
> +		 kvm_eq.qsize, kvm_eq.qpage, kvm_eq.qtoggle, kvm_eq.qindex);
> +
> +	/*
> +	 * We can not tune the EQ configuration from user space. All
> +	 * is done in OPAL.
> +	 */
> +	if (kvm_eq.flags != 0) {
> +		pr_err("invalid flags %d\n", kvm_eq.flags);
> +		return -EINVAL;
> +	}
> +
> +	rc = xive_native_validate_queue_size(kvm_eq.qsize);
> +	if (rc) {
> +		pr_err("invalid queue size %d\n", kvm_eq.qsize);
> +		return rc;
> +	}
> +
> +	/* reset queue and disable queueing */
> +	if (!kvm_eq.qsize) {
> +		q->guest_qpage = 0;
> +		q->guest_qsize = 0;
> +
> +		rc = xive_native_configure_queue(xc->vp_id, q, priority,
> +						 NULL, 0, true);
> +		if (rc) {
> +			pr_err("Failed to reset queue %d for VCPU %d: %d\n",
> +			       priority, xc->server_num, rc);
> +			return rc;
> +		}
> +
> +		if (q->qpage) {
> +			put_page(virt_to_page(q->qpage));
> +			q->qpage = NULL;
> +		}
> +
> +		return 0;
> +	}
> +
> +
> +	page = gfn_to_page(kvm, gpa_to_gfn(kvm_eq.qpage));
> +	if (is_error_page(page)) {
> +		pr_warn("Couldn't get guest page for %llx!\n", kvm_eq.qpage);
> +		return -EINVAL;
> +	}

Yeah.. for the case of a 4kiB page host (these days weird, but not
actually prohibited, AFAIK) you need to check that the qsize selected
actually fits within the page.

> +	qaddr = page_to_virt(page) + (kvm_eq.qpage & ~PAGE_MASK);
> +
> +	/* Backup queue page guest address for migration */

Hm.. KVM itself shouldn't generally need to know about migration.
IIUC these values won't change from what qemu set them to be, so it
should be able to store and migrate them without have to get them back
from the kernel.

> +	q->guest_qpage = kvm_eq.qpage;
> +	q->guest_qsize = kvm_eq.qsize;
> +
> +	rc = xive_native_configure_queue(xc->vp_id, q, priority,
> +					 (__be32 *) qaddr, kvm_eq.qsize, true);
> +	if (rc) {
> +		pr_err("Failed to configure queue %d for VCPU %d: %d\n",
> +		       priority, xc->server_num, rc);
> +		put_page(page);
> +		return rc;
> +	}
> +
> +	/*
> +	 * Only restore the queue state when needed. When doing the
> +	 * H_INT_SET_SOURCE_CONFIG hcall, it should not.
> +	 */
> +	if (kvm_eq.qtoggle != 0 || kvm_eq.qindex != 0) {
> +		rc = xive_native_set_queue_state(xc->vp_id, priority,
> +						 kvm_eq.qtoggle,
> +						 kvm_eq.qindex);
> +		if (rc)
> +			goto error;
> +	}
> +
> +	rc = kvmppc_xive_attach_escalation(vcpu, priority,
> +					   xive->single_escalation);
> +error:
> +	if (rc)
> +		kvmppc_xive_native_cleanup_queue(vcpu, priority);
> +	return rc;
> +}
> +
> +static int kvmppc_xive_native_get_queue_config(struct kvmppc_xive *xive,
> +					       long eq_idx, u64 addr)
> +{
> +	struct kvm *kvm = xive->kvm;
> +	struct kvm_vcpu *vcpu;
> +	struct kvmppc_xive_vcpu *xc;
> +	struct xive_q *q;
> +	void __user *ubufp = (u64 __user *) addr;
> +	u32 server;
> +	u8 priority;
> +	struct kvm_ppc_xive_eq kvm_eq;
> +	u64 qpage;
> +	u64 qsize;
> +	u64 qeoi_page;
> +	u32 escalate_irq;
> +	u64 qflags;
> +	int rc;
> +
> +	/*
> +	 * Demangle priority/server tuple from the EQ identifier
> +	 */
> +	priority = (eq_idx & KVM_XIVE_EQ_PRIORITY_MASK) >>
> +		KVM_XIVE_EQ_PRIORITY_SHIFT;
> +	server = (eq_idx & KVM_XIVE_EQ_SERVER_MASK) >>
> +		KVM_XIVE_EQ_SERVER_SHIFT;
> +
> +	vcpu = kvmppc_xive_find_server(kvm, server);
> +	if (!vcpu) {
> +		pr_err("Can't find server %d\n", server);
> +		return -ENOENT;
> +	}
> +	xc = vcpu->arch.xive_vcpu;
> +
> +	if (priority != xive_prio_from_guest(priority)) {
> +		pr_err("invalid priority for queue %d for VCPU %d\n",
> +		       priority, server);
> +		return -EINVAL;
> +	}
> +	q = &xc->queues[priority];
> +
> +	memset(&kvm_eq, 0, sizeof(kvm_eq));
> +
> +	if (!q->qpage)
> +		return 0;
> +
> +	rc = xive_native_get_queue_info(xc->vp_id, priority, &qpage, &qsize,
> +					&qeoi_page, &escalate_irq, &qflags);
> +	if (rc)
> +		return rc;
> +
> +	/*
> +	 * Return some information on the EQ configuration in
> +	 * OPAL. This is purely informative for now as we can't really
> +	 * tune the EQ configuration from user space.
> +	 */
> +	kvm_eq.flags = 0;
> +	if (qflags & OPAL_XIVE_EQ_ENABLED)
> +		kvm_eq.flags |= KVM_XIVE_EQ_FLAG_ENABLED;
> +	if (qflags & OPAL_XIVE_EQ_ALWAYS_NOTIFY)
> +		kvm_eq.flags |= KVM_XIVE_EQ_FLAG_ALWAYS_NOTIFY;
> +	if (qflags & OPAL_XIVE_EQ_ESCALATE)
> +		kvm_eq.flags |= KVM_XIVE_EQ_FLAG_ESCALATE;

If there's not really anything it can do about it, does it make sense
to even expose this info to userspace?

> +	kvm_eq.qsize = q->guest_qsize;
> +	kvm_eq.qpage = q->guest_qpage;

> +	rc = xive_native_get_queue_state(xc->vp_id, priority, &kvm_eq.qtoggle,
> +					 &kvm_eq.qindex);
> +	if (rc)
> +		return rc;
> +
> +	pr_devel("%s VCPU %d priority %d fl:%x sz:%d addr:%llx g:%d idx:%d\n",
> +		 __func__, server, priority, kvm_eq.flags,
> +		 kvm_eq.qsize, kvm_eq.qpage, kvm_eq.qtoggle, kvm_eq.qindex);
> +
> +	if (copy_to_user(ubufp, &kvm_eq, sizeof(kvm_eq)))
> +		return -EFAULT;
> +
> +	return 0;
> +}
> +
>  static int kvmppc_xive_native_set_attr(struct kvm_device *dev,
>  				       struct kvm_device_attr *attr)
>  {
> @@ -354,6 +574,9 @@ static int kvmppc_xive_native_set_attr(struct kvm_device *dev,
>  	case KVM_DEV_XIVE_GRP_SOURCE_CONFIG:
>  		return kvmppc_xive_native_set_source_config(xive, attr->attr,
>  							    attr->addr);
> +	case KVM_DEV_XIVE_GRP_EQ_CONFIG:
> +		return kvmppc_xive_native_set_queue_config(xive, attr->attr,
> +							   attr->addr);
>  	}
>  	return -ENXIO;
>  }
> @@ -361,6 +584,13 @@ static int kvmppc_xive_native_set_attr(struct kvm_device *dev,
>  static int kvmppc_xive_native_get_attr(struct kvm_device *dev,
>  				       struct kvm_device_attr *attr)
>  {
> +	struct kvmppc_xive *xive = dev->private;
> +
> +	switch (attr->group) {
> +	case KVM_DEV_XIVE_GRP_EQ_CONFIG:
> +		return kvmppc_xive_native_get_queue_config(xive, attr->attr,
> +							   attr->addr);
> +	}
>  	return -ENXIO;
>  }
>  
> @@ -376,6 +606,8 @@ static int kvmppc_xive_native_has_attr(struct kvm_device *dev,
>  		    attr->attr < KVMPPC_XIVE_NR_IRQS)
>  			return 0;
>  		break;
> +	case KVM_DEV_XIVE_GRP_EQ_CONFIG:
> +		return 0;
>  	}
>  	return -ENXIO;
>  }
> diff --git a/Documentation/virtual/kvm/devices/xive.txt b/Documentation/virtual/kvm/devices/xive.txt
> index 33c64b2cdbe8..a4de64f6e79c 100644
> --- a/Documentation/virtual/kvm/devices/xive.txt
> +++ b/Documentation/virtual/kvm/devices/xive.txt
> @@ -53,3 +53,34 @@ the legacy interrupt mode, referred as XICS (POWER7/8).
>      -ENXIO:  CPU event queues not configured or configuration of the
>               underlying HW interrupt failed
>      -EBUSY:  No CPU available to serve interrupt
> +
> +  4. KVM_DEV_XIVE_GRP_EQ_CONFIG (read-write)
> +  Configures an event queue of a CPU
> +  Attributes:
> +    EQ descriptor identifier (64-bit)
> +  The EQ descriptor identifier is a tuple (server, priority) :
> +  bits:     | 63   ....  32 | 31 .. 3 |  2 .. 0
> +  values:   |    unused     |  server | priority
> +  The kvm_device_attr.addr points to :
> +    struct kvm_ppc_xive_eq {
> +	__u32 flags;
> +	__u32 qsize;
> +	__u64 qpage;
> +	__u32 qtoggle;
> +	__u32 qindex;
> +	__u8  pad[40];
> +    };
> +  - flags: queue flags
> +  - qsize: queue size (power of 2)
> +  - qpage: real address of queue
> +  - qtoggle: current queue toggle bit
> +  - qindex: current queue index
> +  - pad: reserved for future use
> +  Errors:
> +    -ENOENT: Invalid CPU number
> +    -EINVAL: Invalid priority
> +    -EINVAL: Invalid flags
> +    -EINVAL: Invalid queue size
> +    -EINVAL: Invalid queue address
> +    -EFAULT: Invalid user pointer for attr->addr.
> +    -EIO:    Configuration of the underlying HW failed

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH v3 09/17] KVM: PPC: Book3S HV: XIVE: add a control to dirty the XIVE EQ pages
From: David Gibson @ 2019-03-18  3:31 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: linuxppc-dev, Paul Mackerras, kvm, kvm-ppc
In-Reply-To: <20190315120609.25910-10-clg@kaod.org>

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

On Fri, Mar 15, 2019 at 01:06:01PM +0100, Cédric Le Goater wrote:
> When migration of a VM is initiated, a first copy of the RAM is
> transferred to the destination before the VM is stopped, but there is
> no guarantee that the EQ pages in which the event notifications are
> queued have not been modified.
> 
> To make sure migration will capture a consistent memory state, the
> XIVE device should perform a XIVE quiesce sequence to stop the flow of
> event notifications and stabilize the EQs. This is the purpose of the
> KVM_DEV_XIVE_EQ_SYNC control which will also marks the EQ pages dirty
> to force their transfer.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
> 
>  Changes since v2 :
> 
>  - Extra comments
>  - fixed locking on source block
> 
>  arch/powerpc/include/uapi/asm/kvm.h        |  1 +
>  arch/powerpc/kvm/book3s_xive_native.c      | 85 ++++++++++++++++++++++
>  Documentation/virtual/kvm/devices/xive.txt | 29 ++++++++
>  3 files changed, 115 insertions(+)
> 
> diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
> index fc9211dbfec8..caf52be89494 100644
> --- a/arch/powerpc/include/uapi/asm/kvm.h
> +++ b/arch/powerpc/include/uapi/asm/kvm.h
> @@ -678,6 +678,7 @@ struct kvm_ppc_cpu_char {
>  /* POWER9 XIVE Native Interrupt Controller */
>  #define KVM_DEV_XIVE_GRP_CTRL		1
>  #define   KVM_DEV_XIVE_RESET		1
> +#define   KVM_DEV_XIVE_EQ_SYNC		2
>  #define KVM_DEV_XIVE_GRP_SOURCE		2	/* 64-bit source identifier */
>  #define KVM_DEV_XIVE_GRP_SOURCE_CONFIG	3	/* 64-bit source identifier */
>  #define KVM_DEV_XIVE_GRP_EQ_CONFIG	4	/* 64-bit EQ identifier */
> diff --git a/arch/powerpc/kvm/book3s_xive_native.c b/arch/powerpc/kvm/book3s_xive_native.c
> index 26ac3c505cd2..ea091c0a8fb6 100644
> --- a/arch/powerpc/kvm/book3s_xive_native.c
> +++ b/arch/powerpc/kvm/book3s_xive_native.c
> @@ -669,6 +669,88 @@ static int kvmppc_xive_reset(struct kvmppc_xive *xive)
>  	return 0;
>  }
>  
> +static void kvmppc_xive_native_sync_sources(struct kvmppc_xive_src_block *sb)
> +{
> +	int j;
> +
> +	for (j = 0; j < KVMPPC_XICS_IRQ_PER_ICS; j++) {
> +		struct kvmppc_xive_irq_state *state = &sb->irq_state[j];
> +		struct xive_irq_data *xd;
> +		u32 hw_num;
> +
> +		if (!state->valid)
> +			continue;
> +
> +		/*
> +		 * The struct kvmppc_xive_irq_state reflects the state
> +		 * of the EAS configuration and not the state of the
> +		 * source. The source is masked setting the PQ bits to
> +		 * '-Q', which is what is being done before calling
> +		 * the KVM_DEV_XIVE_EQ_SYNC control.
> +		 *
> +		 * If a source EAS is configured, OPAL syncs the XIVE
> +		 * IC of the source and the XIVE IC of the previous
> +		 * target if any.
> +		 *
> +		 * So it should be fine ignoring MASKED sources as
> +		 * they have been synced already.
> +		 */
> +		if (state->act_priority == MASKED)
> +			continue;
> +
> +		kvmppc_xive_select_irq(state, &hw_num, &xd);
> +		xive_native_sync_source(hw_num);
> +		xive_native_sync_queue(hw_num);
> +	}
> +}
> +
> +static int kvmppc_xive_native_vcpu_eq_sync(struct kvm_vcpu *vcpu)
> +{
> +	struct kvmppc_xive_vcpu *xc = vcpu->arch.xive_vcpu;
> +	unsigned int prio;
> +
> +	if (!xc)
> +		return -ENOENT;
> +
> +	for (prio = 0; prio < KVMPPC_XIVE_Q_COUNT; prio++) {
> +		struct xive_q *q = &xc->queues[prio];
> +
> +		if (!q->qpage)
> +			continue;
> +
> +		/* Mark EQ page dirty for migration */
> +		mark_page_dirty(vcpu->kvm, gpa_to_gfn(q->guest_qpage));
> +	}
> +	return 0;
> +}
> +
> +static int kvmppc_xive_native_eq_sync(struct kvmppc_xive *xive)
> +{
> +	struct kvm *kvm = xive->kvm;
> +	struct kvm_vcpu *vcpu;
> +	unsigned int i;
> +
> +	pr_devel("%s\n", __func__);
> +
> +	mutex_lock(&kvm->lock);
> +	for (i = 0; i <= xive->max_sbid; i++) {
> +		struct kvmppc_xive_src_block *sb = xive->src_blocks[i];
> +
> +		if (sb) {
> +			arch_spin_lock(&sb->lock);
> +			kvmppc_xive_native_sync_sources(sb);
> +			arch_spin_unlock(&sb->lock);
> +		}
> +	}
> +
> +	kvm_for_each_vcpu(i, vcpu, kvm) {
> +		kvmppc_xive_native_vcpu_eq_sync(vcpu);
> +	}
> +	mutex_unlock(&kvm->lock);
> +
> +	return 0;
> +}
> +
>  static int kvmppc_xive_native_set_attr(struct kvm_device *dev,
>  				       struct kvm_device_attr *attr)
>  {
> @@ -679,6 +761,8 @@ static int kvmppc_xive_native_set_attr(struct kvm_device *dev,
>  		switch (attr->attr) {
>  		case KVM_DEV_XIVE_RESET:
>  			return kvmppc_xive_reset(xive);
> +		case KVM_DEV_XIVE_EQ_SYNC:
> +			return kvmppc_xive_native_eq_sync(xive);
>  		}
>  		break;
>  	case KVM_DEV_XIVE_GRP_SOURCE:
> @@ -717,6 +801,7 @@ static int kvmppc_xive_native_has_attr(struct kvm_device *dev,
>  	case KVM_DEV_XIVE_GRP_CTRL:
>  		switch (attr->attr) {
>  		case KVM_DEV_XIVE_RESET:
> +		case KVM_DEV_XIVE_EQ_SYNC:
>  			return 0;
>  		}
>  		break;
> diff --git a/Documentation/virtual/kvm/devices/xive.txt b/Documentation/virtual/kvm/devices/xive.txt
> index 055aed0c2abb..e6a984592189 100644
> --- a/Documentation/virtual/kvm/devices/xive.txt
> +++ b/Documentation/virtual/kvm/devices/xive.txt
> @@ -23,6 +23,12 @@ the legacy interrupt mode, referred as XICS (POWER7/8).
>      queues. To be used by kexec and kdump.
>      Errors: none
>  
> +    1.2 KVM_DEV_XIVE_EQ_SYNC (write only)
> +    Sync all the sources and queues and mark the EQ pages dirty. This
> +    to make sure that a consistent memory state is captured when
> +    migrating the VM.
> +    Errors: none
> +
>    2. KVM_DEV_XIVE_GRP_SOURCE (write only)
>    Initializes a new source in the XIVE device and mask it.
>    Attributes:
> @@ -97,3 +103,26 @@ the legacy interrupt mode, referred as XICS (POWER7/8).
>    Errors:
>      -ENOENT: Unknown source number
>      -EINVAL: Not initialized source number
> +
> +* Migration:
> +
> +  Saving the state of a VM using the XIVE native exploitation mode
> +  should follow a specific sequence. When the VM is stopped :
> +
> +  1. Mask all sources (PQ=01) to stop the flow of events.
> +
> +  2. Sync the XIVE device with the KVM control KVM_DEV_XIVE_EQ_SYNC to
> +  flush any in-flight event notification and to stabilize the EQs. At
> +  this stage, the EQ pages are marked dirty to make sure they are
> +  transferred in the migration sequence.
> +
> +  3. Capture the state of the source targeting, the EQs configuration
> +  and the state of thread interrupt context registers.
> +
> +  Restore is similar :
> +
> +  1. Restore the EQ configuration. As targeting depends on it.
> +  2. Restore targeting
> +  3. Restore the thread interrupt contexts
> +  4. Restore the source states
> +  5. Let the vCPU run

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH v3 11/17] KVM: introduce a 'mmap' method for KVM devices
From: David Gibson @ 2019-03-18  3:32 UTC (permalink / raw)
  To: Cédric Le Goater
  Cc: kvm, kvm-ppc, Paul Mackerras, Paolo Bonzini, linuxppc-dev
In-Reply-To: <20190315120609.25910-12-clg@kaod.org>

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

On Fri, Mar 15, 2019 at 01:06:03PM +0100, Cédric Le Goater wrote:
> Some KVM devices will want to handle special mappings related to the
> underlying HW. For instance, the XIVE interrupt controller of the
> POWER9 processor has MMIO pages for thread interrupt management and
> for interrupt source control that need to be exposed to the guest when
> the OS has the required support.
> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  include/linux/kvm_host.h |  1 +
>  virt/kvm/kvm_main.c      | 11 +++++++++++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index c38cc5eb7e73..cbf81487b69f 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -1223,6 +1223,7 @@ struct kvm_device_ops {
>  	int (*has_attr)(struct kvm_device *dev, struct kvm_device_attr *attr);
>  	long (*ioctl)(struct kvm_device *dev, unsigned int ioctl,
>  		      unsigned long arg);
> +	int (*mmap)(struct kvm_device *dev, struct vm_area_struct *vma);
>  };
>  
>  void kvm_device_get(struct kvm_device *dev);
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 076bc38963bf..e4881a8c2a6f 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -2878,6 +2878,16 @@ static long kvm_vcpu_compat_ioctl(struct file *filp,
>  }
>  #endif
>  
> +static int kvm_device_mmap(struct file *filp, struct vm_area_struct *vma)
> +{
> +	struct kvm_device *dev = filp->private_data;
> +
> +	if (dev->ops->mmap)
> +		return dev->ops->mmap(dev, vma);
> +
> +	return -ENODEV;
> +}
> +
>  static int kvm_device_ioctl_attr(struct kvm_device *dev,
>  				 int (*accessor)(struct kvm_device *dev,
>  						 struct kvm_device_attr *attr),
> @@ -2927,6 +2937,7 @@ static const struct file_operations kvm_device_fops = {
>  	.unlocked_ioctl = kvm_device_ioctl,
>  	.release = kvm_device_release,
>  	KVM_COMPAT(kvm_device_ioctl),
> +	.mmap = kvm_device_mmap,
>  };
>  
>  struct kvm_device *kvm_device_from_filp(struct file *filp)

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH v3 07/17] KVM: PPC: Book3S HV: XIVE: add a global reset control
From: David Gibson @ 2019-03-18  3:25 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: linuxppc-dev, Paul Mackerras, kvm, kvm-ppc
In-Reply-To: <20190315120609.25910-8-clg@kaod.org>

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

On Fri, Mar 15, 2019 at 01:05:59PM +0100, Cédric Le Goater wrote:
> This control is to be used by the H_INT_RESET hcall from QEMU. Its
> purpose is to clear all configuration of the sources and EQs. This is
> necessary in case of a kexec (for a kdump kernel for instance) to make
> sure that no remaining configuration is left from the previous boot
> setup so that the new kernel can start safely from a clean state.
> 
> The queue 7 is ignored when the XIVE device is configured to run in
> single escalation mode. Prio 7 is used by escalations.
> 
> The XIVE VP is kept enabled as the vCPU is still active and connected
> to the XIVE device.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
> 
>  Changes since v2 :
> 
>  - fixed locking on source block
> 
>  arch/powerpc/include/uapi/asm/kvm.h        |  1 +
>  arch/powerpc/kvm/book3s_xive_native.c      | 85 ++++++++++++++++++++++
>  Documentation/virtual/kvm/devices/xive.txt |  5 ++
>  3 files changed, 91 insertions(+)
> 
> diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
> index 1cd728c87d7c..95e82ab57c03 100644
> --- a/arch/powerpc/include/uapi/asm/kvm.h
> +++ b/arch/powerpc/include/uapi/asm/kvm.h
> @@ -677,6 +677,7 @@ struct kvm_ppc_cpu_char {
>  
>  /* POWER9 XIVE Native Interrupt Controller */
>  #define KVM_DEV_XIVE_GRP_CTRL		1
> +#define   KVM_DEV_XIVE_RESET		1
>  #define KVM_DEV_XIVE_GRP_SOURCE		2	/* 64-bit source identifier */
>  #define KVM_DEV_XIVE_GRP_SOURCE_CONFIG	3	/* 64-bit source identifier */
>  #define KVM_DEV_XIVE_GRP_EQ_CONFIG	4	/* 64-bit EQ identifier */
> diff --git a/arch/powerpc/kvm/book3s_xive_native.c b/arch/powerpc/kvm/book3s_xive_native.c
> index 42e824658a30..3385c336fd89 100644
> --- a/arch/powerpc/kvm/book3s_xive_native.c
> +++ b/arch/powerpc/kvm/book3s_xive_native.c
> @@ -560,6 +560,83 @@ static int kvmppc_xive_native_get_queue_config(struct kvmppc_xive *xive,
>  	return 0;
>  }
>  
> +static void kvmppc_xive_reset_sources(struct kvmppc_xive_src_block *sb)
> +{
> +	int i;
> +
> +	for (i = 0; i < KVMPPC_XICS_IRQ_PER_ICS; i++) {
> +		struct kvmppc_xive_irq_state *state = &sb->irq_state[i];
> +
> +		if (!state->valid)
> +			continue;
> +
> +		if (state->act_priority == MASKED)
> +			continue;
> +
> +		state->eisn = 0;
> +		state->act_server = 0;
> +		state->act_priority = MASKED;
> +		xive_vm_esb_load(&state->ipi_data, XIVE_ESB_SET_PQ_01);
> +		xive_native_configure_irq(state->ipi_number, 0, MASKED, 0);
> +		if (state->pt_number) {
> +			xive_vm_esb_load(state->pt_data, XIVE_ESB_SET_PQ_01);
> +			xive_native_configure_irq(state->pt_number,
> +						  0, MASKED, 0);
> +		}
> +	}
> +}
> +
> +static int kvmppc_xive_reset(struct kvmppc_xive *xive)
> +{
> +	struct kvm *kvm = xive->kvm;
> +	struct kvm_vcpu *vcpu;
> +	unsigned int i;
> +
> +	pr_devel("%s\n", __func__);
> +
> +	mutex_lock(&kvm->lock);
> +
> +	kvm_for_each_vcpu(i, vcpu, kvm) {
> +		struct kvmppc_xive_vcpu *xc = vcpu->arch.xive_vcpu;
> +		unsigned int prio;
> +
> +		if (!xc)
> +			continue;
> +
> +		kvmppc_xive_disable_vcpu_interrupts(vcpu);
> +
> +		for (prio = 0; prio < KVMPPC_XIVE_Q_COUNT; prio++) {
> +
> +			/* Single escalation, no queue 7 */
> +			if (prio == 7 && xive->single_escalation)
> +				break;
> +
> +			if (xc->esc_virq[prio]) {
> +				free_irq(xc->esc_virq[prio], vcpu);
> +				irq_dispose_mapping(xc->esc_virq[prio]);
> +				kfree(xc->esc_virq_names[prio]);
> +				xc->esc_virq[prio] = 0;
> +			}
> +
> +			kvmppc_xive_native_cleanup_queue(vcpu, prio);
> +		}
> +	}
> +
> +	for (i = 0; i <= xive->max_sbid; i++) {
> +		struct kvmppc_xive_src_block *sb = xive->src_blocks[i];
> +
> +		if (sb) {
> +			arch_spin_lock(&sb->lock);
> +			kvmppc_xive_reset_sources(sb);
> +			arch_spin_unlock(&sb->lock);
> +		}
> +	}
> +
> +	mutex_unlock(&kvm->lock);
> +
> +	return 0;
> +}
> +
>  static int kvmppc_xive_native_set_attr(struct kvm_device *dev,
>  				       struct kvm_device_attr *attr)
>  {
> @@ -567,6 +644,10 @@ static int kvmppc_xive_native_set_attr(struct kvm_device *dev,
>  
>  	switch (attr->group) {
>  	case KVM_DEV_XIVE_GRP_CTRL:
> +		switch (attr->attr) {
> +		case KVM_DEV_XIVE_RESET:
> +			return kvmppc_xive_reset(xive);
> +		}
>  		break;
>  	case KVM_DEV_XIVE_GRP_SOURCE:
>  		return kvmppc_xive_native_set_source(xive, attr->attr,
> @@ -599,6 +680,10 @@ static int kvmppc_xive_native_has_attr(struct kvm_device *dev,
>  {
>  	switch (attr->group) {
>  	case KVM_DEV_XIVE_GRP_CTRL:
> +		switch (attr->attr) {
> +		case KVM_DEV_XIVE_RESET:
> +			return 0;
> +		}
>  		break;
>  	case KVM_DEV_XIVE_GRP_SOURCE:
>  	case KVM_DEV_XIVE_GRP_SOURCE_CONFIG:
> diff --git a/Documentation/virtual/kvm/devices/xive.txt b/Documentation/virtual/kvm/devices/xive.txt
> index a4de64f6e79c..e1893d303ab7 100644
> --- a/Documentation/virtual/kvm/devices/xive.txt
> +++ b/Documentation/virtual/kvm/devices/xive.txt
> @@ -17,6 +17,11 @@ the legacy interrupt mode, referred as XICS (POWER7/8).
>  
>    1. KVM_DEV_XIVE_GRP_CTRL
>    Provides global controls on the device
> +  Attributes:
> +    1.1 KVM_DEV_XIVE_RESET (write only)
> +    Resets the interrupt controller configuration for sources and event
> +    queues. To be used by kexec and kdump.
> +    Errors: none
>  
>    2. KVM_DEV_XIVE_GRP_SOURCE (write only)
>    Initializes a new source in the XIVE device and mask it.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* [RESEND 7/7] IB/mthca: Use the new FOLL_LONGTERM flag to get_user_pages_fast()
From: ira.weiny @ 2019-03-17 18:34 UTC (permalink / raw)
  To: Andrew Morton, John Hubbard, Michal Hocko, Kirill A. Shutemov,
	Peter Zijlstra, Jason Gunthorpe, Benjamin Herrenschmidt,
	Paul Mackerras, David S. Miller, Martin Schwidefsky,
	Heiko Carstens, Rich Felker, Yoshinori Sato, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Ralf Baechle, James Hogan
  Cc: linux-s390, linux-sh, linux-rdma, Ira Weiny, linux-kernel,
	linux-mips, linux-mm, netdev, sparclinux, linuxppc-dev
In-Reply-To: <20190317183438.2057-1-ira.weiny@intel.com>

From: Ira Weiny <ira.weiny@intel.com>

Use the new FOLL_LONGTERM to get_user_pages_fast() to protect against
FS DAX pages being mapped.

Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
 drivers/infiniband/hw/mthca/mthca_memfree.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/mthca/mthca_memfree.c b/drivers/infiniband/hw/mthca/mthca_memfree.c
index 112d2f38e0de..8ff0e90d7564 100644
--- a/drivers/infiniband/hw/mthca/mthca_memfree.c
+++ b/drivers/infiniband/hw/mthca/mthca_memfree.c
@@ -472,7 +472,8 @@ int mthca_map_user_db(struct mthca_dev *dev, struct mthca_uar *uar,
 		goto out;
 	}
 
-	ret = get_user_pages_fast(uaddr & PAGE_MASK, 1, FOLL_WRITE, pages);
+	ret = get_user_pages_fast(uaddr & PAGE_MASK, 1,
+				  FOLL_WRITE | FOLL_LONGTERM, pages);
 	if (ret < 0)
 		goto out;
 
-- 
2.20.1


^ permalink raw reply related

* [RESEND 6/7] IB/qib: Use the new FOLL_LONGTERM flag to get_user_pages_fast()
From: ira.weiny @ 2019-03-17 18:34 UTC (permalink / raw)
  To: Andrew Morton, John Hubbard, Michal Hocko, Kirill A. Shutemov,
	Peter Zijlstra, Jason Gunthorpe, Benjamin Herrenschmidt,
	Paul Mackerras, David S. Miller, Martin Schwidefsky,
	Heiko Carstens, Rich Felker, Yoshinori Sato, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Ralf Baechle, James Hogan
  Cc: linux-s390, linux-sh, linux-rdma, Ira Weiny, linux-kernel,
	linux-mips, linux-mm, netdev, sparclinux, linuxppc-dev
In-Reply-To: <20190317183438.2057-1-ira.weiny@intel.com>

From: Ira Weiny <ira.weiny@intel.com>

Use the new FOLL_LONGTERM to get_user_pages_fast() to protect against
FS DAX pages being mapped.

Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
 drivers/infiniband/hw/qib/qib_user_sdma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/qib/qib_user_sdma.c b/drivers/infiniband/hw/qib/qib_user_sdma.c
index 31c523b2a9f5..b53cc0240e02 100644
--- a/drivers/infiniband/hw/qib/qib_user_sdma.c
+++ b/drivers/infiniband/hw/qib/qib_user_sdma.c
@@ -673,7 +673,7 @@ static int qib_user_sdma_pin_pages(const struct qib_devdata *dd,
 		else
 			j = npages;
 
-		ret = get_user_pages_fast(addr, j, 0, pages);
+		ret = get_user_pages_fast(addr, j, FOLL_LONGTERM, pages);
 		if (ret != j) {
 			i = 0;
 			j = ret;
-- 
2.20.1


^ permalink raw reply related


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