diff mbox series

[v3,4/9] KVM: PPC: Ultravisor: Add generic ultravisor call handler

Message ID 20190606173614.32090-5-cclaudio@linux.ibm.com
State Superseded
Headers show
Series kvmppc: Paravirtualize KVM to support ultravisor | expand

Commit Message

Claudio Carvalho June 6, 2019, 5:36 p.m. UTC 
From: Ram Pai <linuxram@us.ibm.com>

Add the ucall() function, which can be used to make ultravisor calls
with varied number of in and out arguments. Ultravisor calls can be made
from the host or guests.

This copies the implementation of plpar_hcall().

Signed-off-by: Ram Pai <linuxram@us.ibm.com>
[Change ucall.S to not save CR, rename and move the headers, build
 ucall.S if CONFIG_PPC_UV set, and add some comments in the code]
Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com>
---
 arch/powerpc/include/asm/ultravisor-api.h | 20 +++++++++++++++
 arch/powerpc/include/asm/ultravisor.h     | 20 +++++++++++++++
 arch/powerpc/kernel/Makefile              |  2 +-
 arch/powerpc/kernel/ucall.S               | 31 +++++++++++++++++++++++
 arch/powerpc/kernel/ultravisor.c          |  4 +++
 5 files changed, 76 insertions(+), 1 deletion(-)
 create mode 100644 arch/powerpc/include/asm/ultravisor-api.h
 create mode 100644 arch/powerpc/kernel/ucall.S

Comments

Paul Mackerras June 15, 2019, 7:37 a.m. UTC | #1 
On Thu, Jun 06, 2019 at 02:36:09PM -0300, Claudio Carvalho wrote:
> From: Ram Pai <linuxram@us.ibm.com>
> 
> Add the ucall() function, which can be used to make ultravisor calls
> with varied number of in and out arguments. Ultravisor calls can be made
> from the host or guests.
> 
> This copies the implementation of plpar_hcall().

Again, we will want all of this on every powernv-capable kernel, since
they will all need to do UV_WRITE_PATE, even if they have no other
support for the ultravisor.

Paul.
Paul Mackerras June 17, 2019, 2:06 a.m. UTC | #2 
On Thu, Jun 06, 2019 at 02:36:09PM -0300, Claudio Carvalho wrote:
> From: Ram Pai <linuxram@us.ibm.com>
> 
> Add the ucall() function, which can be used to make ultravisor calls
> with varied number of in and out arguments. Ultravisor calls can be made
> from the host or guests.
> 
> This copies the implementation of plpar_hcall().

One point which I missed when I looked at this patch previously is
that the ABI that we're defining here is different from the hcall ABI
in that we are putting the ucall number in r0, whereas hcalls have the
hcall number in r3.  That makes ucalls more like syscalls, which have
the syscall number in r0.  So that last sentence quoted above is
somewhat misleading.

The thing we need to consider is that when SMFCTRL[E] = 0, a ucall
instruction becomes a hcall (that is, sc 2 is executed as if it was
sc 1).  In that case, the first argument to the ucall will be
interpreted as the hcall number.  Mostly that will happen not to be a
valid hcall number, but sometimes it might unavoidably be a valid but
unintended hcall number.

I think that will make it difficult to get ucalls to fail gracefully
in the case where SMF/PEF is disabled.  It seems like the assignment
of ucall numbers was made so that they wouldn't overlap with valid
hcall numbers; presumably that was so that we could tell when an hcall
was actually intended to be a ucall.  However, using a different GPR
to pass the ucall number defeats that.

I realize that there is ultravisor code in development that takes the
ucall number in r0, and also that having the ucall number in r3 would
possibly make life more difficult for the place where we call
UV_RETURN in assembler code.  Nevertheless, perhaps we should consider
changing the ABI to be like the hcall ABI before everything gets set
in concrete.

Paul.
Ram Pai June 17, 2019, 11:51 p.m. UTC | #3 
On Mon, Jun 17, 2019 at 12:06:32PM +1000, Paul Mackerras wrote:
> On Thu, Jun 06, 2019 at 02:36:09PM -0300, Claudio Carvalho wrote:
> > From: Ram Pai <linuxram@us.ibm.com>
> > 
> > Add the ucall() function, which can be used to make ultravisor calls
> > with varied number of in and out arguments. Ultravisor calls can be made
> > from the host or guests.
> > 
> > This copies the implementation of plpar_hcall().
> 
> One point which I missed when I looked at this patch previously is
> that the ABI that we're defining here is different from the hcall ABI
> in that we are putting the ucall number in r0, whereas hcalls have the
> hcall number in r3.  That makes ucalls more like syscalls, which have
> the syscall number in r0.  So that last sentence quoted above is
> somewhat misleading.
> 
> The thing we need to consider is that when SMFCTRL[E] = 0, a ucall
> instruction becomes a hcall (that is, sc 2 is executed as if it was
> sc 1).  In that case, the first argument to the ucall will be
> interpreted as the hcall number.  Mostly that will happen not to be a
> valid hcall number, but sometimes it might unavoidably be a valid but
> unintended hcall number.
> 
> I think that will make it difficult to get ucalls to fail gracefully
> in the case where SMF/PEF is disabled.  It seems like the assignment
> of ucall numbers was made so that they wouldn't overlap with valid
> hcall numbers; presumably that was so that we could tell when an hcall
> was actually intended to be a ucall.  However, using a different GPR
> to pass the ucall number defeats that.

Right this is a valid point. Glad that you caught it, otherwise it would
have become a difficult to fix it in the future.

> 
> I realize that there is ultravisor code in development that takes the
> ucall number in r0, and also that having the ucall number in r3 would
> possibly make life more difficult for the place where we call
> UV_RETURN in assembler code.  

Its called from one place in the hypervisor, and the changes look
simple.

-       LOAD_REG_IMMEDIATE(r0, UV_RETURN)
+       LOAD_REG_IMMEDIATE(r3, UV_RETURN)
        ld      r7, VCPU_GPR(R7)(r4)
        ld      r6, VCPU_GPR(R6)(r4)
        ld      r4, VCPU_GPR(R4)(r4)

What am i missing?



> Nevertheless, perhaps we should consider
> changing the ABI to be like the hcall ABI before everything gets set
> in concrete.


yes.

Thanks Paul!
RP
Segher Boessenkool June 18, 2019, 11:47 a.m. UTC | #4 
Hi Paul,

On Mon, Jun 17, 2019 at 12:06:32PM +1000, Paul Mackerras wrote:
> The thing we need to consider is that when SMFCTRL[E] = 0, a ucall
> instruction becomes a hcall (that is, sc 2 is executed as if it was
> sc 1).  In that case, the first argument to the ucall will be
> interpreted as the hcall number.  Mostly that will happen not to be a
> valid hcall number, but sometimes it might unavoidably be a valid but
> unintended hcall number.

Shouldn't a caller of the ultravisor *know* that it is talking to the
ultravisor in the first place?  And not to the hypervisor.


Segher
Ram Pai June 18, 2019, 3:25 p.m. UTC | #5 
On Tue, Jun 18, 2019 at 06:47:01AM -0500, Segher Boessenkool wrote:
> Hi Paul,
> 
> On Mon, Jun 17, 2019 at 12:06:32PM +1000, Paul Mackerras wrote:
> > The thing we need to consider is that when SMFCTRL[E] = 0, a ucall
> > instruction becomes a hcall (that is, sc 2 is executed as if it was
> > sc 1).  In that case, the first argument to the ucall will be
> > interpreted as the hcall number.  Mostly that will happen not to be a
> > valid hcall number, but sometimes it might unavoidably be a valid but
> > unintended hcall number.
> 
> Shouldn't a caller of the ultravisor *know* that it is talking to the
> ultravisor in the first place?  And not to the hypervisor.

It may or may not.  But if it knows and still decides to make the ucall,
   the hypervisor must gracefully handle it.  
   
 We can't control who makes a ucall. A normal process within the VM could
 make a ucall too. Or a normal process running on top of the hypervisor
 could make a ucall.

RP
diff mbox series

Patch

diff --git a/arch/powerpc/include/asm/ultravisor-api.h b/arch/powerpc/include/asm/ultravisor-api.h
new file mode 100644
index 000000000000..5f538f33c704
--- /dev/null
+++ b/arch/powerpc/include/asm/ultravisor-api.h
@@ -0,0 +1,20 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Ultravisor calls.
+ *
+ * Copyright 2019, IBM Corporation.
+ *
+ */
+#ifndef _ASM_POWERPC_ULTRAVISOR_API_H
+#define _ASM_POWERPC_ULTRAVISOR_API_H
+
+#include <asm/hvcall.h>
+
+/* Return codes */
+#define U_NOT_AVAILABLE		H_NOT_AVAILABLE
+#define U_SUCCESS		H_SUCCESS
+#define U_FUNCTION		H_FUNCTION
+#define U_PARAMETER		H_PARAMETER
+
+#endif /* _ASM_POWERPC_ULTRAVISOR_API_H */
+
diff --git a/arch/powerpc/include/asm/ultravisor.h b/arch/powerpc/include/asm/ultravisor.h
index e5009b0d84ea..7500771a8ebd 100644
--- a/arch/powerpc/include/asm/ultravisor.h
+++ b/arch/powerpc/include/asm/ultravisor.h
@@ -8,8 +8,28 @@ 
 #ifndef _ASM_POWERPC_ULTRAVISOR_H
 #define _ASM_POWERPC_ULTRAVISOR_H
 
+#include <asm/ultravisor-api.h>
+
+#if !defined(__ASSEMBLY__)
+
 /* Internal functions */
 extern int early_init_dt_scan_ultravisor(unsigned long node, const char *uname,
 					 int depth, void *data);
 
+/* API functions */
+#define UCALL_BUFSIZE 4
+/**
+ * ucall: Make a powerpc ultravisor call.
+ * @opcode: The ultravisor call to make.
+ * @retbuf: Buffer to store up to 4 return arguments in.
+ *
+ * This call supports up to 6 arguments and 4 return arguments. Use
+ * UCALL_BUFSIZE to size the return argument buffer.
+ */
+#if defined(CONFIG_PPC_UV)
+long ucall(unsigned long opcode, unsigned long *retbuf, ...);
+#endif
+
+#endif /* !__ASSEMBLY__ */
+
 #endif	/* _ASM_POWERPC_ULTRAVISOR_H */
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index c8ca219e54bf..43ff4546e469 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -154,7 +154,7 @@  endif
 
 obj-$(CONFIG_EPAPR_PARAVIRT)	+= epapr_paravirt.o epapr_hcalls.o
 obj-$(CONFIG_KVM_GUEST)		+= kvm.o kvm_emul.o
-obj-$(CONFIG_PPC_UV)		+= ultravisor.o
+obj-$(CONFIG_PPC_UV)		+= ultravisor.o ucall.o
 
 # Disable GCOV, KCOV & sanitizers in odd or sensitive code
 GCOV_PROFILE_prom_init.o := n
diff --git a/arch/powerpc/kernel/ucall.S b/arch/powerpc/kernel/ucall.S
new file mode 100644
index 000000000000..ecc88998a13b
--- /dev/null
+++ b/arch/powerpc/kernel/ucall.S
@@ -0,0 +1,31 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Generic code to perform an ultravisor call.
+ *
+ * Copyright 2019, IBM Corporation.
+ *
+ */
+#include <asm/ppc_asm.h>
+
+/*
+ * This function is based on the plpar_hcall()
+ */
+_GLOBAL_TOC(ucall)
+	mr	r0,r3
+	std     r4,STK_PARAM(R4)(r1)     /* Save ret buffer */
+	mr	r3,r5
+	mr	r4,r6
+	mr	r5,r7
+	mr	r6,r8
+	mr	r7,r9
+	mr	r8,r10
+
+	sc 2				/* invoke the ultravisor */
+
+	ld	r12,STK_PARAM(R4)(r1)
+	std	r4,  0(r12)
+	std	r5,  8(r12)
+	std	r6, 16(r12)
+	std	r7, 24(r12)
+
+	blr				/* return r3 = status */
diff --git a/arch/powerpc/kernel/ultravisor.c b/arch/powerpc/kernel/ultravisor.c
index dc6021f63c97..02ddf79a9522 100644
--- a/arch/powerpc/kernel/ultravisor.c
+++ b/arch/powerpc/kernel/ultravisor.c
@@ -8,10 +8,14 @@ 
 #include <linux/init.h>
 #include <linux/printk.h>
 #include <linux/string.h>
+#include <linux/export.h>
 
 #include <asm/ultravisor.h>
 #include <asm/firmware.h>
 
+/* in ucall.S */
+EXPORT_SYMBOL_GPL(ucall);
+
 int __init early_init_dt_scan_ultravisor(unsigned long node, const char *uname,
 					 int depth, void *data)
 {