Re: [PATCH v1 05/11] x86/tdx: Add __tdx_module_call() and __tdx_hypercall() helper functions

Kuppuswamy, Sathyanarayanan
2021-06-14T21:37:42Z

On 6/14/21 1:11 PM, Borislav Petkov wrote:
> On Mon, Jun 14, 2021 at 12:45:45PM -0700, Kuppuswamy, Sathyanarayanan wrote:
>> May be I should define a macro for it and use Mov to keep it uniform
>> with other register updates.
>
> Macro?
>
> There's the, well, *MOV* instruction, if you insist on keeping it
> uniform. But this is not about keeping it uniform - it is about having
> the code as clear as understandable as possible:
>
>
> /* Set RAX to TDCALL leaf function 0 */
> xor %eax, %eax
>
> Plain and simple and clear why the XORing is done.

Ok. I will fix the comment.

>> With the trace support, they should be able to see the flow before making
>> the tdx_*_call(). That should be enough clue for debug right?
>
> Are you expecting all those cloud users to trace their guests just to
> figure that out? I'm sceptical they will...
>
> Rather, I'd try to allocate a special error value that
> do_tdx_hypercall() returns in %eax and then have the wrapper which will
> puts %r10 on the stack, check that error value and panic with a nice
> error message.
>

I will add r10 to struct tdx_hypercall_output and return it to callers to
check it.

>
> Btw, where is that function used? Gurgling, it shows it in some MMIO
> patch, I'm guessing that's still coming.
>
> As to how to do it properly, you pass in
>
> struct tdx_hypercall_output *out
>
> as a function parameter and caller can pick out whatever it wants from
> that struct.

It looks like it is used only in MMIO use case now. I think we don't need
it anymore. I will remove it.

>
> Thx.
>

--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer