On 5/17/24 17:28, Dave Hansen wrote:
On 5/17/24 07:19, Kirill A. Shutemov wrote:
  static inline void tdx_io_out(int size, u16 port, u32 value)
  {
-       struct tdx_module_args args = {
-               .r10 = TDX_HYPERCALL_STANDARD,
-               .r11 = hcall_func(EXIT_REASON_IO_INSTRUCTION),
-               .r12 = size,
-               .r13 = 1,
-               .r14 = port,
-               .r15 = value,
-       };
-
-       __tdx_hypercall(&args);
+       TDVMCALL_0(hcall_func(EXIT_REASON_IO_INSTRUCTION),
+                  size, TDX_PORT_WRITE, port, value);
  }

I actually really like the self-documenting nature of the structures.  I
don't think it's a win if this is where the lines-of-code savings comes
from.


It's just a tradeoff.  For example someone could well have written

#define TDVMCALL_0(reason, a1, a2, a3, a4) \
  do { \
        struct tdx_module_args args = {
                .r10 = TDX_HYPERCALL_STANDARD,
                .r11 = reason,
                .r12 = a1,
                .r13 = a2,
                .r14 = a3,
                .r15 = a4,
        __tdx_hypercall(&args);
  } while(0)

even with the current __tdx_hypercall() implementation.

I agree that TDVMCALL_x is somewhat less legible; on the other hand it highlights that these TDVMCALLs all have a common convention for passing parameters / retrieving results, and reduces the potential for silly typos.

This is also why I asked about the different approaches for TDCALL vs. TDVMCALL. Given that there are only a handful of appearances for tdvmcall_trampoline, maybe the best of both worlds is just to inline the whole thing? This way the code in the macros matches the parameter passing convention of the GHCI.

Paolo


Reply via email to