lkml.org 
[lkml]   [2022]   [Mar]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
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