[lkml]   [2022]   [Mar]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCHv5.1 04/30] x86/tdx: Extend the confidential computing API to support TDX guests
On 3/9/22 08:01, Kirill A. Shutemov wrote:
> +/*
> + * Wrapper for __tdx_module_call() for cases when the call doesn't suppose to
> + * fail. Panic if the call fails.
> + */
> +static inline void tdx_module_call(u64 fn, u64 rcx, u64 rdx, u64 r8, u64 r9,
> + struct tdx_module_output *out)
> +{
> + if (__tdx_module_call(fn, rcx, rdx, r8, r9, out))
> + panic("TDCALL %lld failed (Buggy TDX module!)\n", fn);
> +}

That comment didn't do much for me. I know it's a wrapper. I know it
panics() if the call returns a failure. That's what the code *does*. I
want a comment to tell me *why* it does that.

I _think_ I may have been getting this confused with the TDVMCALL mechanism.

All TDVMCALLs that return with rax==0 are fatal, we jump right to a ud2
instruction. A __tdx_module_call() (via TDCALL) with rax==0 doesn't
*have* to be fatal. But, this establishes a policy that all TDCALLs via
tdx_module_call() *ARE* fatal.

How about this for a comment?

* Used for TDX guests to make calls directly to the TD module. This
* should only be used for calls that have no legitimate reason to fail
* or where the kernel can not survive the call failing.

That tells me a *LOT*: This is a guest -> TD module thing. Not a host
thing, not a hypercall. And, no the naming isn't good enough to tell me
that. Also, it give me advice. It tells me when I should use this
function. If it look at the call site, it even makes sense. A guest
can't even build a sane PTE without this call succeeding. If *COURSE*
we panic() if the call fails.

You could even call this information out in the comment in get_info():

* The GPA width that comes out of this call is critical. TDX
* guests can not meaningfully run without it.

Then it all kinda fits together. Oh, this panic() is awfully harsh.
Oh, it's only supposed to be used for important things that the guest
really needs. Then there's a comment about why it needs it so badly.

Otherwise the panic() just looks superfluous and mean.

 \ /
  Last update: 2022-03-09 19:37    [W:0.227 / U:0.736 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site