* [PATCH] KVM: s390: generate kvm hypercall functions
@ 2021-07-13 14:57 Heiko Carstens
2021-07-13 15:41 ` Christian Borntraeger
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Heiko Carstens @ 2021-07-13 14:57 UTC (permalink / raw)
To: Christian Borntraeger, Janosch Frank
Cc: David Hildenbrand, Cornelia Huck, Claudio Imbrenda, kvm, linux-s390
Generate kvm hypercall functions with a macro instead of duplicating
the more or less identical code seven times. This also reduces number
of lines of code.
However the main purpose is to get rid of as many as possible open
coded error prone register asm constructs in s390 architecture code.
For the only user of kvm_hypercall identical code is created
before/after this patch (drivers/s390/virtio/virtio_ccw.c).
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
arch/s390/include/asm/kvm_para.h | 229 ++++++++++---------------------
1 file changed, 73 insertions(+), 156 deletions(-)
diff --git a/arch/s390/include/asm/kvm_para.h b/arch/s390/include/asm/kvm_para.h
index cbc7c3a68e4d..df73a052760c 100644
--- a/arch/s390/include/asm/kvm_para.h
+++ b/arch/s390/include/asm/kvm_para.h
@@ -24,162 +24,79 @@
#include <uapi/asm/kvm_para.h>
#include <asm/diag.h>
-static inline long __kvm_hypercall0(unsigned long nr)
-{
- register unsigned long __nr asm("1") = nr;
- register long __rc asm("2");
-
- asm volatile ("diag 2,4,0x500\n"
- : "=d" (__rc) : "d" (__nr): "memory", "cc");
- return __rc;
-}
-
-static inline long kvm_hypercall0(unsigned long nr)
-{
- diag_stat_inc(DIAG_STAT_X500);
- return __kvm_hypercall0(nr);
-}
-
-static inline long __kvm_hypercall1(unsigned long nr, unsigned long p1)
-{
- register unsigned long __nr asm("1") = nr;
- register unsigned long __p1 asm("2") = p1;
- register long __rc asm("2");
-
- asm volatile ("diag 2,4,0x500\n"
- : "=d" (__rc) : "d" (__nr), "0" (__p1) : "memory", "cc");
- return __rc;
-}
-
-static inline long kvm_hypercall1(unsigned long nr, unsigned long p1)
-{
- diag_stat_inc(DIAG_STAT_X500);
- return __kvm_hypercall1(nr, p1);
-}
-
-static inline long __kvm_hypercall2(unsigned long nr, unsigned long p1,
- unsigned long p2)
-{
- register unsigned long __nr asm("1") = nr;
- register unsigned long __p1 asm("2") = p1;
- register unsigned long __p2 asm("3") = p2;
- register long __rc asm("2");
-
- asm volatile ("diag 2,4,0x500\n"
- : "=d" (__rc) : "d" (__nr), "0" (__p1), "d" (__p2)
- : "memory", "cc");
- return __rc;
-}
-
-static inline long kvm_hypercall2(unsigned long nr, unsigned long p1,
- unsigned long p2)
-{
- diag_stat_inc(DIAG_STAT_X500);
- return __kvm_hypercall2(nr, p1, p2);
-}
-
-static inline long __kvm_hypercall3(unsigned long nr, unsigned long p1,
- unsigned long p2, unsigned long p3)
-{
- register unsigned long __nr asm("1") = nr;
- register unsigned long __p1 asm("2") = p1;
- register unsigned long __p2 asm("3") = p2;
- register unsigned long __p3 asm("4") = p3;
- register long __rc asm("2");
-
- asm volatile ("diag 2,4,0x500\n"
- : "=d" (__rc) : "d" (__nr), "0" (__p1), "d" (__p2),
- "d" (__p3) : "memory", "cc");
- return __rc;
-}
-
-static inline long kvm_hypercall3(unsigned long nr, unsigned long p1,
- unsigned long p2, unsigned long p3)
-{
- diag_stat_inc(DIAG_STAT_X500);
- return __kvm_hypercall3(nr, p1, p2, p3);
-}
-
-static inline long __kvm_hypercall4(unsigned long nr, unsigned long p1,
- unsigned long p2, unsigned long p3,
- unsigned long p4)
-{
- register unsigned long __nr asm("1") = nr;
- register unsigned long __p1 asm("2") = p1;
- register unsigned long __p2 asm("3") = p2;
- register unsigned long __p3 asm("4") = p3;
- register unsigned long __p4 asm("5") = p4;
- register long __rc asm("2");
-
- asm volatile ("diag 2,4,0x500\n"
- : "=d" (__rc) : "d" (__nr), "0" (__p1), "d" (__p2),
- "d" (__p3), "d" (__p4) : "memory", "cc");
- return __rc;
-}
-
-static inline long kvm_hypercall4(unsigned long nr, unsigned long p1,
- unsigned long p2, unsigned long p3,
- unsigned long p4)
-{
- diag_stat_inc(DIAG_STAT_X500);
- return __kvm_hypercall4(nr, p1, p2, p3, p4);
-}
-
-static inline long __kvm_hypercall5(unsigned long nr, unsigned long p1,
- unsigned long p2, unsigned long p3,
- unsigned long p4, unsigned long p5)
-{
- register unsigned long __nr asm("1") = nr;
- register unsigned long __p1 asm("2") = p1;
- register unsigned long __p2 asm("3") = p2;
- register unsigned long __p3 asm("4") = p3;
- register unsigned long __p4 asm("5") = p4;
- register unsigned long __p5 asm("6") = p5;
- register long __rc asm("2");
-
- asm volatile ("diag 2,4,0x500\n"
- : "=d" (__rc) : "d" (__nr), "0" (__p1), "d" (__p2),
- "d" (__p3), "d" (__p4), "d" (__p5) : "memory", "cc");
- return __rc;
-}
-
-static inline long kvm_hypercall5(unsigned long nr, unsigned long p1,
- unsigned long p2, unsigned long p3,
- unsigned long p4, unsigned long p5)
-{
- diag_stat_inc(DIAG_STAT_X500);
- return __kvm_hypercall5(nr, p1, p2, p3, p4, p5);
-}
-
-static inline long __kvm_hypercall6(unsigned long nr, unsigned long p1,
- unsigned long p2, unsigned long p3,
- unsigned long p4, unsigned long p5,
- unsigned long p6)
-{
- register unsigned long __nr asm("1") = nr;
- register unsigned long __p1 asm("2") = p1;
- register unsigned long __p2 asm("3") = p2;
- register unsigned long __p3 asm("4") = p3;
- register unsigned long __p4 asm("5") = p4;
- register unsigned long __p5 asm("6") = p5;
- register unsigned long __p6 asm("7") = p6;
- register long __rc asm("2");
-
- asm volatile ("diag 2,4,0x500\n"
- : "=d" (__rc) : "d" (__nr), "0" (__p1), "d" (__p2),
- "d" (__p3), "d" (__p4), "d" (__p5), "d" (__p6)
- : "memory", "cc");
- return __rc;
-}
-
-static inline long kvm_hypercall6(unsigned long nr, unsigned long p1,
- unsigned long p2, unsigned long p3,
- unsigned long p4, unsigned long p5,
- unsigned long p6)
-{
- diag_stat_inc(DIAG_STAT_X500);
- return __kvm_hypercall6(nr, p1, p2, p3, p4, p5, p6);
-}
+#define HYPERCALL_FMT_0
+#define HYPERCALL_FMT_1 , "0" (r2)
+#define HYPERCALL_FMT_2 , "d" (r3) HYPERCALL_FMT_1
+#define HYPERCALL_FMT_3 , "d" (r4) HYPERCALL_FMT_2
+#define HYPERCALL_FMT_4 , "d" (r5) HYPERCALL_FMT_3
+#define HYPERCALL_FMT_5 , "d" (r6) HYPERCALL_FMT_4
+#define HYPERCALL_FMT_6 , "d" (r7) HYPERCALL_FMT_5
+
+#define HYPERCALL_PARM_0
+#define HYPERCALL_PARM_1 , unsigned long arg1
+#define HYPERCALL_PARM_2 HYPERCALL_PARM_1, unsigned long arg2
+#define HYPERCALL_PARM_3 HYPERCALL_PARM_2, unsigned long arg3
+#define HYPERCALL_PARM_4 HYPERCALL_PARM_3, unsigned long arg4
+#define HYPERCALL_PARM_5 HYPERCALL_PARM_4, unsigned long arg5
+#define HYPERCALL_PARM_6 HYPERCALL_PARM_5, unsigned long arg6
+
+#define HYPERCALL_REGS_0
+#define HYPERCALL_REGS_1 \
+ register unsigned long r2 asm("2") = arg1
+#define HYPERCALL_REGS_2 \
+ HYPERCALL_REGS_1; \
+ register unsigned long r3 asm("3") = arg2
+#define HYPERCALL_REGS_3 \
+ HYPERCALL_REGS_2; \
+ register unsigned long r4 asm("4") = arg3
+#define HYPERCALL_REGS_4 \
+ HYPERCALL_REGS_3; \
+ register unsigned long r5 asm("5") = arg4
+#define HYPERCALL_REGS_5 \
+ HYPERCALL_REGS_4; \
+ register unsigned long r6 asm("6") = arg5
+#define HYPERCALL_REGS_6 \
+ HYPERCALL_REGS_5; \
+ register unsigned long r7 asm("7") = arg6
+
+#define HYPERCALL_ARGS_0
+#define HYPERCALL_ARGS_1 , arg1
+#define HYPERCALL_ARGS_2 HYPERCALL_ARGS_1, arg2
+#define HYPERCALL_ARGS_3 HYPERCALL_ARGS_2, arg3
+#define HYPERCALL_ARGS_4 HYPERCALL_ARGS_3, arg4
+#define HYPERCALL_ARGS_5 HYPERCALL_ARGS_4, arg5
+#define HYPERCALL_ARGS_6 HYPERCALL_ARGS_5, arg6
+
+#define GENERATE_KVM_HYPERCALL_FUNC(args) \
+static inline \
+long __kvm_hypercall##args(unsigned long nr HYPERCALL_PARM_##args) \
+{ \
+ register unsigned long __nr asm("1") = nr; \
+ register long __rc asm("2"); \
+ HYPERCALL_REGS_##args; \
+ \
+ asm volatile ( \
+ " diag 2,4,0x500\n" \
+ : "=d" (__rc) \
+ : "d" (__nr) HYPERCALL_FMT_##args \
+ : "memory", "cc"); \
+ return __rc; \
+} \
+ \
+static inline \
+long kvm_hypercall##args(unsigned long nr HYPERCALL_PARM_##args) \
+{ \
+ diag_stat_inc(DIAG_STAT_X500); \
+ return __kvm_hypercall##args(nr HYPERCALL_ARGS_##args); \
+}
+
+GENERATE_KVM_HYPERCALL_FUNC(0)
+GENERATE_KVM_HYPERCALL_FUNC(1)
+GENERATE_KVM_HYPERCALL_FUNC(2)
+GENERATE_KVM_HYPERCALL_FUNC(3)
+GENERATE_KVM_HYPERCALL_FUNC(4)
+GENERATE_KVM_HYPERCALL_FUNC(5)
+GENERATE_KVM_HYPERCALL_FUNC(6)
/* kvm on s390 is always paravirtualization enabled */
static inline int kvm_para_available(void)
--
2.25.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH] KVM: s390: generate kvm hypercall functions
2021-07-13 14:57 [PATCH] KVM: s390: generate kvm hypercall functions Heiko Carstens
@ 2021-07-13 15:41 ` Christian Borntraeger
2021-07-13 15:52 ` Heiko Carstens
2021-07-13 18:06 ` Christian Borntraeger
2021-07-14 9:38 ` Claudio Imbrenda
2 siblings, 1 reply; 10+ messages in thread
From: Christian Borntraeger @ 2021-07-13 15:41 UTC (permalink / raw)
To: Heiko Carstens, Janosch Frank
Cc: David Hildenbrand, Cornelia Huck, Claudio Imbrenda, kvm, linux-s390
On 13.07.21 16:57, Heiko Carstens wrote:
[..]
> +#define HYPERCALL_FMT_0
> +#define HYPERCALL_FMT_1 , "0" (r2)
> +#define HYPERCALL_FMT_2 , "d" (r3) HYPERCALL_FMT_1
> +#define HYPERCALL_FMT_3 , "d" (r4) HYPERCALL_FMT_2
> +#define HYPERCALL_FMT_4 , "d" (r5) HYPERCALL_FMT_3
> +#define HYPERCALL_FMT_5 , "d" (r6) HYPERCALL_FMT_4
> +#define HYPERCALL_FMT_6 , "d" (r7) HYPERCALL_FMT_5
This will result in reverse order.
old:
"d" (__nr), "0" (__p1), "d" (__p2), "d" (__p3), "d" (__p4), "d" (__p5), "d" (__p6)
new:
"d"(__nr), "d"(r7), "d"(r6), "d"(r5), "d"(r4), "d"(r3), "0"(r2)
As we do not reference the variable in the asm this should not matter,
I just noticed it when comparing the result of the preprocessed files.
Assuming that we do not care this looks good.
Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] KVM: s390: generate kvm hypercall functions
2021-07-13 15:41 ` Christian Borntraeger
@ 2021-07-13 15:52 ` Heiko Carstens
2021-07-13 15:59 ` Christian Borntraeger
0 siblings, 1 reply; 10+ messages in thread
From: Heiko Carstens @ 2021-07-13 15:52 UTC (permalink / raw)
To: Christian Borntraeger
Cc: Janosch Frank, David Hildenbrand, Cornelia Huck,
Claudio Imbrenda, kvm, linux-s390
On Tue, Jul 13, 2021 at 05:41:33PM +0200, Christian Borntraeger wrote:
> On 13.07.21 16:57, Heiko Carstens wrote:
> [..]
> > +#define HYPERCALL_FMT_0
> > +#define HYPERCALL_FMT_1 , "0" (r2)
> > +#define HYPERCALL_FMT_2 , "d" (r3) HYPERCALL_FMT_1
> > +#define HYPERCALL_FMT_3 , "d" (r4) HYPERCALL_FMT_2
> > +#define HYPERCALL_FMT_4 , "d" (r5) HYPERCALL_FMT_3
> > +#define HYPERCALL_FMT_5 , "d" (r6) HYPERCALL_FMT_4
> > +#define HYPERCALL_FMT_6 , "d" (r7) HYPERCALL_FMT_5
>
> This will result in reverse order.
> old:
> "d" (__nr), "0" (__p1), "d" (__p2), "d" (__p3), "d" (__p4), "d" (__p5), "d" (__p6)
> new:
> "d"(__nr), "d"(r7), "d"(r6), "d"(r5), "d"(r4), "d"(r3), "0"(r2)
>
> As we do not reference the variable in the asm this should not matter,
> I just noticed it when comparing the result of the preprocessed files.
>
> Assuming that we do not care this looks good.
Yes, it does not matter. Please let me know if should change it anyway.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] KVM: s390: generate kvm hypercall functions
2021-07-13 15:52 ` Heiko Carstens
@ 2021-07-13 15:59 ` Christian Borntraeger
2021-07-13 16:15 ` Heiko Carstens
0 siblings, 1 reply; 10+ messages in thread
From: Christian Borntraeger @ 2021-07-13 15:59 UTC (permalink / raw)
To: Heiko Carstens
Cc: Janosch Frank, David Hildenbrand, Cornelia Huck,
Claudio Imbrenda, kvm, linux-s390
On 13.07.21 17:52, Heiko Carstens wrote:
> On Tue, Jul 13, 2021 at 05:41:33PM +0200, Christian Borntraeger wrote:
>> On 13.07.21 16:57, Heiko Carstens wrote:
>> [..]
>>> +#define HYPERCALL_FMT_0
>>> +#define HYPERCALL_FMT_1 , "0" (r2)
>>> +#define HYPERCALL_FMT_2 , "d" (r3) HYPERCALL_FMT_1
>>> +#define HYPERCALL_FMT_3 , "d" (r4) HYPERCALL_FMT_2
>>> +#define HYPERCALL_FMT_4 , "d" (r5) HYPERCALL_FMT_3
>>> +#define HYPERCALL_FMT_5 , "d" (r6) HYPERCALL_FMT_4
>>> +#define HYPERCALL_FMT_6 , "d" (r7) HYPERCALL_FMT_5
>>
>> This will result in reverse order.
>> old:
>> "d" (__nr), "0" (__p1), "d" (__p2), "d" (__p3), "d" (__p4), "d" (__p5), "d" (__p6)
>> new:
>> "d"(__nr), "d"(r7), "d"(r6), "d"(r5), "d"(r4), "d"(r3), "0"(r2)
>>
>> As we do not reference the variable in the asm this should not matter,
>> I just noticed it when comparing the result of the preprocessed files.
>>
>> Assuming that we do not care this looks good.
>
> Yes, it does not matter. Please let me know if should change it anyway.
No, I think this is ok.
Shall I take it via the kvm tree or do you want to take it via the s390 tree?
For that
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] KVM: s390: generate kvm hypercall functions
2021-07-13 15:59 ` Christian Borntraeger
@ 2021-07-13 16:15 ` Heiko Carstens
0 siblings, 0 replies; 10+ messages in thread
From: Heiko Carstens @ 2021-07-13 16:15 UTC (permalink / raw)
To: Christian Borntraeger
Cc: Janosch Frank, David Hildenbrand, Cornelia Huck,
Claudio Imbrenda, kvm, linux-s390
On Tue, Jul 13, 2021 at 05:59:17PM +0200, Christian Borntraeger wrote:
> On 13.07.21 17:52, Heiko Carstens wrote:
> > On Tue, Jul 13, 2021 at 05:41:33PM +0200, Christian Borntraeger wrote:
> > > On 13.07.21 16:57, Heiko Carstens wrote:
> > > [..]
> > > > +#define HYPERCALL_FMT_0
> > > > +#define HYPERCALL_FMT_1 , "0" (r2)
> > > > +#define HYPERCALL_FMT_2 , "d" (r3) HYPERCALL_FMT_1
> > > > +#define HYPERCALL_FMT_3 , "d" (r4) HYPERCALL_FMT_2
> > > > +#define HYPERCALL_FMT_4 , "d" (r5) HYPERCALL_FMT_3
> > > > +#define HYPERCALL_FMT_5 , "d" (r6) HYPERCALL_FMT_4
> > > > +#define HYPERCALL_FMT_6 , "d" (r7) HYPERCALL_FMT_5
> > >
> > > This will result in reverse order.
> > > old:
> > > "d" (__nr), "0" (__p1), "d" (__p2), "d" (__p3), "d" (__p4), "d" (__p5), "d" (__p6)
> > > new:
> > > "d"(__nr), "d"(r7), "d"(r6), "d"(r5), "d"(r4), "d"(r3), "0"(r2)
> > >
> > > As we do not reference the variable in the asm this should not matter,
> > > I just noticed it when comparing the result of the preprocessed files.
> > >
> > > Assuming that we do not care this looks good.
> >
> > Yes, it does not matter. Please let me know if should change it anyway.
>
> No, I think this is ok.
> Shall I take it via the kvm tree or do you want to take it via the s390 tree?
I think this should go via kvm tree. It probably has to wait until next merge
window anyway(?).
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] KVM: s390: generate kvm hypercall functions
2021-07-13 14:57 [PATCH] KVM: s390: generate kvm hypercall functions Heiko Carstens
2021-07-13 15:41 ` Christian Borntraeger
@ 2021-07-13 18:06 ` Christian Borntraeger
2021-07-14 9:38 ` Claudio Imbrenda
2 siblings, 0 replies; 10+ messages in thread
From: Christian Borntraeger @ 2021-07-13 18:06 UTC (permalink / raw)
To: Heiko Carstens, Janosch Frank
Cc: David Hildenbrand, Cornelia Huck, Claudio Imbrenda, kvm, linux-s390
On 13.07.21 16:57, Heiko Carstens wrote:
> Generate kvm hypercall functions with a macro instead of duplicating
> the more or less identical code seven times. This also reduces number
> of lines of code.
> However the main purpose is to get rid of as many as possible open
> coded error prone register asm constructs in s390 architecture code.
>
> For the only user of kvm_hypercall identical code is created
> before/after this patch (drivers/s390/virtio/virtio_ccw.c).
>
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
Thanks applied. Will queue for 5.15.
> ---
> arch/s390/include/asm/kvm_para.h | 229 ++++++++++---------------------
> 1 file changed, 73 insertions(+), 156 deletions(-)
>
> diff --git a/arch/s390/include/asm/kvm_para.h b/arch/s390/include/asm/kvm_para.h
> index cbc7c3a68e4d..df73a052760c 100644
> --- a/arch/s390/include/asm/kvm_para.h
> +++ b/arch/s390/include/asm/kvm_para.h
> @@ -24,162 +24,79 @@
> #include <uapi/asm/kvm_para.h>
> #include <asm/diag.h>
>
> -static inline long __kvm_hypercall0(unsigned long nr)
> -{
> - register unsigned long __nr asm("1") = nr;
> - register long __rc asm("2");
> -
> - asm volatile ("diag 2,4,0x500\n"
> - : "=d" (__rc) : "d" (__nr): "memory", "cc");
> - return __rc;
> -}
> -
> -static inline long kvm_hypercall0(unsigned long nr)
> -{
> - diag_stat_inc(DIAG_STAT_X500);
> - return __kvm_hypercall0(nr);
> -}
> -
> -static inline long __kvm_hypercall1(unsigned long nr, unsigned long p1)
> -{
> - register unsigned long __nr asm("1") = nr;
> - register unsigned long __p1 asm("2") = p1;
> - register long __rc asm("2");
> -
> - asm volatile ("diag 2,4,0x500\n"
> - : "=d" (__rc) : "d" (__nr), "0" (__p1) : "memory", "cc");
> - return __rc;
> -}
> -
> -static inline long kvm_hypercall1(unsigned long nr, unsigned long p1)
> -{
> - diag_stat_inc(DIAG_STAT_X500);
> - return __kvm_hypercall1(nr, p1);
> -}
> -
> -static inline long __kvm_hypercall2(unsigned long nr, unsigned long p1,
> - unsigned long p2)
> -{
> - register unsigned long __nr asm("1") = nr;
> - register unsigned long __p1 asm("2") = p1;
> - register unsigned long __p2 asm("3") = p2;
> - register long __rc asm("2");
> -
> - asm volatile ("diag 2,4,0x500\n"
> - : "=d" (__rc) : "d" (__nr), "0" (__p1), "d" (__p2)
> - : "memory", "cc");
> - return __rc;
> -}
> -
> -static inline long kvm_hypercall2(unsigned long nr, unsigned long p1,
> - unsigned long p2)
> -{
> - diag_stat_inc(DIAG_STAT_X500);
> - return __kvm_hypercall2(nr, p1, p2);
> -}
> -
> -static inline long __kvm_hypercall3(unsigned long nr, unsigned long p1,
> - unsigned long p2, unsigned long p3)
> -{
> - register unsigned long __nr asm("1") = nr;
> - register unsigned long __p1 asm("2") = p1;
> - register unsigned long __p2 asm("3") = p2;
> - register unsigned long __p3 asm("4") = p3;
> - register long __rc asm("2");
> -
> - asm volatile ("diag 2,4,0x500\n"
> - : "=d" (__rc) : "d" (__nr), "0" (__p1), "d" (__p2),
> - "d" (__p3) : "memory", "cc");
> - return __rc;
> -}
> -
> -static inline long kvm_hypercall3(unsigned long nr, unsigned long p1,
> - unsigned long p2, unsigned long p3)
> -{
> - diag_stat_inc(DIAG_STAT_X500);
> - return __kvm_hypercall3(nr, p1, p2, p3);
> -}
> -
> -static inline long __kvm_hypercall4(unsigned long nr, unsigned long p1,
> - unsigned long p2, unsigned long p3,
> - unsigned long p4)
> -{
> - register unsigned long __nr asm("1") = nr;
> - register unsigned long __p1 asm("2") = p1;
> - register unsigned long __p2 asm("3") = p2;
> - register unsigned long __p3 asm("4") = p3;
> - register unsigned long __p4 asm("5") = p4;
> - register long __rc asm("2");
> -
> - asm volatile ("diag 2,4,0x500\n"
> - : "=d" (__rc) : "d" (__nr), "0" (__p1), "d" (__p2),
> - "d" (__p3), "d" (__p4) : "memory", "cc");
> - return __rc;
> -}
> -
> -static inline long kvm_hypercall4(unsigned long nr, unsigned long p1,
> - unsigned long p2, unsigned long p3,
> - unsigned long p4)
> -{
> - diag_stat_inc(DIAG_STAT_X500);
> - return __kvm_hypercall4(nr, p1, p2, p3, p4);
> -}
> -
> -static inline long __kvm_hypercall5(unsigned long nr, unsigned long p1,
> - unsigned long p2, unsigned long p3,
> - unsigned long p4, unsigned long p5)
> -{
> - register unsigned long __nr asm("1") = nr;
> - register unsigned long __p1 asm("2") = p1;
> - register unsigned long __p2 asm("3") = p2;
> - register unsigned long __p3 asm("4") = p3;
> - register unsigned long __p4 asm("5") = p4;
> - register unsigned long __p5 asm("6") = p5;
> - register long __rc asm("2");
> -
> - asm volatile ("diag 2,4,0x500\n"
> - : "=d" (__rc) : "d" (__nr), "0" (__p1), "d" (__p2),
> - "d" (__p3), "d" (__p4), "d" (__p5) : "memory", "cc");
> - return __rc;
> -}
> -
> -static inline long kvm_hypercall5(unsigned long nr, unsigned long p1,
> - unsigned long p2, unsigned long p3,
> - unsigned long p4, unsigned long p5)
> -{
> - diag_stat_inc(DIAG_STAT_X500);
> - return __kvm_hypercall5(nr, p1, p2, p3, p4, p5);
> -}
> -
> -static inline long __kvm_hypercall6(unsigned long nr, unsigned long p1,
> - unsigned long p2, unsigned long p3,
> - unsigned long p4, unsigned long p5,
> - unsigned long p6)
> -{
> - register unsigned long __nr asm("1") = nr;
> - register unsigned long __p1 asm("2") = p1;
> - register unsigned long __p2 asm("3") = p2;
> - register unsigned long __p3 asm("4") = p3;
> - register unsigned long __p4 asm("5") = p4;
> - register unsigned long __p5 asm("6") = p5;
> - register unsigned long __p6 asm("7") = p6;
> - register long __rc asm("2");
> -
> - asm volatile ("diag 2,4,0x500\n"
> - : "=d" (__rc) : "d" (__nr), "0" (__p1), "d" (__p2),
> - "d" (__p3), "d" (__p4), "d" (__p5), "d" (__p6)
> - : "memory", "cc");
> - return __rc;
> -}
> -
> -static inline long kvm_hypercall6(unsigned long nr, unsigned long p1,
> - unsigned long p2, unsigned long p3,
> - unsigned long p4, unsigned long p5,
> - unsigned long p6)
> -{
> - diag_stat_inc(DIAG_STAT_X500);
> - return __kvm_hypercall6(nr, p1, p2, p3, p4, p5, p6);
> -}
> +#define HYPERCALL_FMT_0
> +#define HYPERCALL_FMT_1 , "0" (r2)
> +#define HYPERCALL_FMT_2 , "d" (r3) HYPERCALL_FMT_1
> +#define HYPERCALL_FMT_3 , "d" (r4) HYPERCALL_FMT_2
> +#define HYPERCALL_FMT_4 , "d" (r5) HYPERCALL_FMT_3
> +#define HYPERCALL_FMT_5 , "d" (r6) HYPERCALL_FMT_4
> +#define HYPERCALL_FMT_6 , "d" (r7) HYPERCALL_FMT_5
> +
> +#define HYPERCALL_PARM_0
> +#define HYPERCALL_PARM_1 , unsigned long arg1
> +#define HYPERCALL_PARM_2 HYPERCALL_PARM_1, unsigned long arg2
> +#define HYPERCALL_PARM_3 HYPERCALL_PARM_2, unsigned long arg3
> +#define HYPERCALL_PARM_4 HYPERCALL_PARM_3, unsigned long arg4
> +#define HYPERCALL_PARM_5 HYPERCALL_PARM_4, unsigned long arg5
> +#define HYPERCALL_PARM_6 HYPERCALL_PARM_5, unsigned long arg6
> +
> +#define HYPERCALL_REGS_0
> +#define HYPERCALL_REGS_1 \
> + register unsigned long r2 asm("2") = arg1
> +#define HYPERCALL_REGS_2 \
> + HYPERCALL_REGS_1; \
> + register unsigned long r3 asm("3") = arg2
> +#define HYPERCALL_REGS_3 \
> + HYPERCALL_REGS_2; \
> + register unsigned long r4 asm("4") = arg3
> +#define HYPERCALL_REGS_4 \
> + HYPERCALL_REGS_3; \
> + register unsigned long r5 asm("5") = arg4
> +#define HYPERCALL_REGS_5 \
> + HYPERCALL_REGS_4; \
> + register unsigned long r6 asm("6") = arg5
> +#define HYPERCALL_REGS_6 \
> + HYPERCALL_REGS_5; \
> + register unsigned long r7 asm("7") = arg6
> +
> +#define HYPERCALL_ARGS_0
> +#define HYPERCALL_ARGS_1 , arg1
> +#define HYPERCALL_ARGS_2 HYPERCALL_ARGS_1, arg2
> +#define HYPERCALL_ARGS_3 HYPERCALL_ARGS_2, arg3
> +#define HYPERCALL_ARGS_4 HYPERCALL_ARGS_3, arg4
> +#define HYPERCALL_ARGS_5 HYPERCALL_ARGS_4, arg5
> +#define HYPERCALL_ARGS_6 HYPERCALL_ARGS_5, arg6
> +
> +#define GENERATE_KVM_HYPERCALL_FUNC(args) \
> +static inline \
> +long __kvm_hypercall##args(unsigned long nr HYPERCALL_PARM_##args) \
> +{ \
> + register unsigned long __nr asm("1") = nr; \
> + register long __rc asm("2"); \
> + HYPERCALL_REGS_##args; \
> + \
> + asm volatile ( \
> + " diag 2,4,0x500\n" \
> + : "=d" (__rc) \
> + : "d" (__nr) HYPERCALL_FMT_##args \
> + : "memory", "cc"); \
> + return __rc; \
> +} \
> + \
> +static inline \
> +long kvm_hypercall##args(unsigned long nr HYPERCALL_PARM_##args) \
> +{ \
> + diag_stat_inc(DIAG_STAT_X500); \
> + return __kvm_hypercall##args(nr HYPERCALL_ARGS_##args); \
> +}
> +
> +GENERATE_KVM_HYPERCALL_FUNC(0)
> +GENERATE_KVM_HYPERCALL_FUNC(1)
> +GENERATE_KVM_HYPERCALL_FUNC(2)
> +GENERATE_KVM_HYPERCALL_FUNC(3)
> +GENERATE_KVM_HYPERCALL_FUNC(4)
> +GENERATE_KVM_HYPERCALL_FUNC(5)
> +GENERATE_KVM_HYPERCALL_FUNC(6)
>
> /* kvm on s390 is always paravirtualization enabled */
> static inline int kvm_para_available(void)
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] KVM: s390: generate kvm hypercall functions
2021-07-13 14:57 [PATCH] KVM: s390: generate kvm hypercall functions Heiko Carstens
2021-07-13 15:41 ` Christian Borntraeger
2021-07-13 18:06 ` Christian Borntraeger
@ 2021-07-14 9:38 ` Claudio Imbrenda
2021-07-14 9:50 ` Heiko Carstens
2 siblings, 1 reply; 10+ messages in thread
From: Claudio Imbrenda @ 2021-07-14 9:38 UTC (permalink / raw)
To: Heiko Carstens
Cc: Christian Borntraeger, Janosch Frank, David Hildenbrand,
Cornelia Huck, kvm, linux-s390
On Tue, 13 Jul 2021 16:57:13 +0200
Heiko Carstens <hca@linux.ibm.com> wrote:
[snip]
> +#define HYPERCALL_ARGS_0
> +#define HYPERCALL_ARGS_1 , arg1
> +#define HYPERCALL_ARGS_2 HYPERCALL_ARGS_1, arg2
> +#define HYPERCALL_ARGS_3 HYPERCALL_ARGS_2, arg3
> +#define HYPERCALL_ARGS_4 HYPERCALL_ARGS_3, arg4
> +#define HYPERCALL_ARGS_5 HYPERCALL_ARGS_4, arg5
> +#define HYPERCALL_ARGS_6 HYPERCALL_ARGS_5, arg6
> +
> +#define GENERATE_KVM_HYPERCALL_FUNC(args)
> \ +static inline
> \ +long __kvm_hypercall##args(unsigned long
> nr HYPERCALL_PARM_##args) \ +{
> \
> + register unsigned long __nr asm("1") = nr;
> \
> + register long __rc asm("2");
> \
didn't we want to get rid of asm register allocations?
this would have been a nice time to do such a cleanup
> + HYPERCALL_REGS_##args;
> \
> +
> \
> + asm volatile (
> \
> + " diag 2,4,0x500\n"
> \
> + : "=d" (__rc)
> \
> + : "d" (__nr) HYPERCALL_FMT_##args
> \
> + : "memory", "cc");
> \
> + return __rc;
> \ +}
> \
> +
> \ +static inline
> \ +long kvm_hypercall##args(unsigned long nr
> HYPERCALL_PARM_##args) \ +{
> \
> + diag_stat_inc(DIAG_STAT_X500);
> \
> + return __kvm_hypercall##args(nr
> HYPERCALL_ARGS_##args); \ +}
> +
> +GENERATE_KVM_HYPERCALL_FUNC(0)
> +GENERATE_KVM_HYPERCALL_FUNC(1)
> +GENERATE_KVM_HYPERCALL_FUNC(2)
> +GENERATE_KVM_HYPERCALL_FUNC(3)
> +GENERATE_KVM_HYPERCALL_FUNC(4)
> +GENERATE_KVM_HYPERCALL_FUNC(5)
> +GENERATE_KVM_HYPERCALL_FUNC(6)
>
> /* kvm on s390 is always paravirtualization enabled */
> static inline int kvm_para_available(void)
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] KVM: s390: generate kvm hypercall functions
2021-07-14 9:38 ` Claudio Imbrenda
@ 2021-07-14 9:50 ` Heiko Carstens
2021-07-14 10:03 ` Christian Borntraeger
0 siblings, 1 reply; 10+ messages in thread
From: Heiko Carstens @ 2021-07-14 9:50 UTC (permalink / raw)
To: Claudio Imbrenda
Cc: Christian Borntraeger, Janosch Frank, David Hildenbrand,
Cornelia Huck, kvm, linux-s390
On Wed, Jul 14, 2021 at 11:38:43AM +0200, Claudio Imbrenda wrote:
> On Tue, 13 Jul 2021 16:57:13 +0200
> Heiko Carstens <hca@linux.ibm.com> wrote:
>
> [snip]
>
> > +#define HYPERCALL_ARGS_0
> > +#define HYPERCALL_ARGS_1 , arg1
> > +#define HYPERCALL_ARGS_2 HYPERCALL_ARGS_1, arg2
> > +#define HYPERCALL_ARGS_3 HYPERCALL_ARGS_2, arg3
> > +#define HYPERCALL_ARGS_4 HYPERCALL_ARGS_3, arg4
> > +#define HYPERCALL_ARGS_5 HYPERCALL_ARGS_4, arg5
> > +#define HYPERCALL_ARGS_6 HYPERCALL_ARGS_5, arg6
> > +
> > +#define GENERATE_KVM_HYPERCALL_FUNC(args)
> > \ +static inline
> > \ +long __kvm_hypercall##args(unsigned long
> > nr HYPERCALL_PARM_##args) \ +{
> > \
> > + register unsigned long __nr asm("1") = nr;
> > \
> > + register long __rc asm("2");
> > \
>
> didn't we want to get rid of asm register allocations?
>
> this would have been a nice time to do such a cleanup
I see only two ways to get rid them, both are suboptimal, therefore I
decided to keep them at very few places; one of them this one.
Alternatively to this approach it would be possible to:
a) write the function entirely in assembler (instead of inlining it).
b) pass a structure with all parameters to the inline assembly and
clobber a large amount of registers, which _might_ even lead to
compile errors since the compiler might run out of registers when
allocating registers for the inline asm.
Given that hypercall is slow anyway a) might be an option. But that's
up to you guys. Otherwise I would consider this the "final" solution
until we get compiler support which allows for something better.
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] KVM: s390: generate kvm hypercall functions
2021-07-14 9:50 ` Heiko Carstens
@ 2021-07-14 10:03 ` Christian Borntraeger
2021-07-14 11:21 ` Heiko Carstens
0 siblings, 1 reply; 10+ messages in thread
From: Christian Borntraeger @ 2021-07-14 10:03 UTC (permalink / raw)
To: Heiko Carstens, Claudio Imbrenda
Cc: Janosch Frank, David Hildenbrand, Cornelia Huck, kvm, linux-s390
On 14.07.21 11:50, Heiko Carstens wrote:
> On Wed, Jul 14, 2021 at 11:38:43AM +0200, Claudio Imbrenda wrote:
>> On Tue, 13 Jul 2021 16:57:13 +0200
>> Heiko Carstens <hca@linux.ibm.com> wrote:
>>
>> [snip]
>>
>>> +#define HYPERCALL_ARGS_0
>>> +#define HYPERCALL_ARGS_1 , arg1
>>> +#define HYPERCALL_ARGS_2 HYPERCALL_ARGS_1, arg2
>>> +#define HYPERCALL_ARGS_3 HYPERCALL_ARGS_2, arg3
>>> +#define HYPERCALL_ARGS_4 HYPERCALL_ARGS_3, arg4
>>> +#define HYPERCALL_ARGS_5 HYPERCALL_ARGS_4, arg5
>>> +#define HYPERCALL_ARGS_6 HYPERCALL_ARGS_5, arg6
>>> +
>>> +#define GENERATE_KVM_HYPERCALL_FUNC(args)
>>> \ +static inline
>>> \ +long __kvm_hypercall##args(unsigned long
>>> nr HYPERCALL_PARM_##args) \ +{
>>> \
>>> + register unsigned long __nr asm("1") = nr;
>>> \
>>> + register long __rc asm("2");
>>> \
>>
>> didn't we want to get rid of asm register allocations?
>>
>> this would have been a nice time to do such a cleanup
>
> I see only two ways to get rid them, both are suboptimal, therefore I
> decided to keep them at very few places; one of them this one.
>
> Alternatively to this approach it would be possible to:
>
> a) write the function entirely in assembler (instead of inlining it).
I would like to keep this as is, unless we know that this could break.
Maybe we should add something like nokasan or whatever?
> b) pass a structure with all parameters to the inline assembly and
> clobber a large amount of registers, which _might_ even lead to
> compile errors since the compiler might run out of registers when
> allocating registers for the inline asm.
>
> Given that hypercall is slow anyway a) might be an option. But that's
> up to you guys. Otherwise I would consider this the "final" solution
> until we get compiler support which allows for something better.
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] KVM: s390: generate kvm hypercall functions
2021-07-14 10:03 ` Christian Borntraeger
@ 2021-07-14 11:21 ` Heiko Carstens
0 siblings, 0 replies; 10+ messages in thread
From: Heiko Carstens @ 2021-07-14 11:21 UTC (permalink / raw)
To: Christian Borntraeger
Cc: Claudio Imbrenda, Janosch Frank, David Hildenbrand,
Cornelia Huck, kvm, linux-s390
On Wed, Jul 14, 2021 at 12:03:20PM +0200, Christian Borntraeger wrote:
> > > didn't we want to get rid of asm register allocations?
> > > this would have been a nice time to do such a cleanup
> >
> > I see only two ways to get rid them, both are suboptimal, therefore I
> > decided to keep them at very few places; one of them this one.
> >
> > Alternatively to this approach it would be possible to:
> >
> > a) write the function entirely in assembler (instead of inlining it).
>
> I would like to keep this as is, unless we know that this could break.
> Maybe we should add something like nokasan or whatever?
That would only make sense if the function would not be inlined. For
that we have e.g. __no_kasan_or_inline. But then I'd rather prefer
__always_inline. But that wouldn't solve any problems, if you see any.
From my point of view this should be safe wrt instrumentation. There
are only scalar assignments without memory accesses or anything else
the could be instrumented. If even that wouldn't work then "register
asm" would be completely useless. Even though, given all the potential
pitfalls, it is very close to being useless.
So if you want to go that route (noinstr, or whatever), then we need
those functions in a way that they can't be inlined. But keep in mind
that we also have e.g. call_on_stack() with a similar construct which
I'd like to keep inlined for performance reasons.
Let me know if you want any changes to the hypercall code.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-07-14 11:22 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-13 14:57 [PATCH] KVM: s390: generate kvm hypercall functions Heiko Carstens
2021-07-13 15:41 ` Christian Borntraeger
2021-07-13 15:52 ` Heiko Carstens
2021-07-13 15:59 ` Christian Borntraeger
2021-07-13 16:15 ` Heiko Carstens
2021-07-13 18:06 ` Christian Borntraeger
2021-07-14 9:38 ` Claudio Imbrenda
2021-07-14 9:50 ` Heiko Carstens
2021-07-14 10:03 ` Christian Borntraeger
2021-07-14 11:21 ` Heiko Carstens
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).