| Message ID | 20210713145713.2815167-1-hca@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: s390: generate kvm hypercall functions | expand |
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>
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.
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>
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(?).
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) >
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)
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.
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.
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.
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)
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(-)