On 12/19/23 4:51 PM, kirill.shutemov@xxxxxxxxxxxxxxx wrote:
On Tue, Dec 19, 2023 at 04:17:40PM -0800, Alexey Makhalov wrote:
On 12/19/23 3:20 PM, kirill.shutemov@xxxxxxxxxxxxxxx wrote:
On Tue, Dec 19, 2023 at 01:57:47PM -0800, Alexey Makhalov wrote:
+static inline
+unsigned long vmware_hypercall1(unsigned long cmd, unsigned long in1)
...
+static inline
+unsigned long vmware_hypercall3(unsigned long cmd, unsigned long in1,
+ uint32_t *out1, uint32_t *out2)
...
+static inline
+unsigned long vmware_hypercall4(unsigned long cmd, unsigned long in1,
+ uint32_t *out1, uint32_t *out2,
+ uint32_t *out3)
...
+static inline
+unsigned long vmware_hypercall5(unsigned long cmd, unsigned long in1,
+ unsigned long in3, unsigned long in4,
+ unsigned long in5, uint32_t *out2)
...
+static inline
+unsigned long vmware_hypercall6(unsigned long cmd, unsigned long in1,
+ unsigned long in3, uint32_t *out2,
+ uint32_t *out3, uint32_t *out4,
+ uint32_t *out5)
...
+static inline
+unsigned long vmware_hypercall7(unsigned long cmd, unsigned long in1,
+ unsigned long in3, unsigned long in4,
+ unsigned long in5, uint32_t *out1,
+ uint32_t *out2, uint32_t *out3)
Naming is weird. The number in the name doesn't help much as there seems
no system on how many of the parameters are ins and outs.
There was internal discussion on hypercall API naming. One of proposals was
using 2 digits - number of input and number of output arguments.
And it definitely looked weird. So, we agreed to have just single number -
total number of arguments excluding cmd.
Have you considered naming them by number of input parameters? Number of
output parameters as demanded by users.
So vmware_hypercall4() will become vmware_hypercall1() and current
vmware_hypercall1() and vmware_hypercall3() will go away.
It is still awful, but /maybe/ better that this, I donno.
Deprecating vmware_hypercall1 and vmware_hypercall3 in favor of
vmware_hypercall4 will generate less efficient code for the caller of
first ones.
Using current vmware_hypercall4 instead of vmware_hypercall1 will force
the caller to allocate additional variables (register or on stack
memory) for hypercall asm inline to put additional output registers on.
And specifically to 'usage' of *out3 - compiler will unnecessary
'clobber' useful rdx, when hypervisor will keep it unchanged.
Unfortunately VMware hypercall ABI is not as beautiful as KVM one,
especially in number of output arguments and their ordering. rbp
register usage as an argument is a separate bummer((. So we have to work
with what we have.
Current set of functions includes only 6 functions (for LB), which is
the optimum between readability, maintainability and performance. It
covers all current kernel callers and all new callers from yet to be
upstreamed patches that we have in Photon OS including 2 patches for x86
and arm64 guest support.
Regards,
--Alexey
