linux-kernel - Re: [PATCH v1 05/11] x86/tdx: Add __tdx_module_call() and __tdx

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 14 Jun 2021 22:11:14 +0200
From:   Borislav Petkov <bp@...en8.de>
To:     "Kuppuswamy, Sathyanarayanan" 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Andy Lutomirski <luto@...nel.org>,
        Peter H Anvin <hpa@...or.com>,
        Dave Hansen <dave.hansen@...el.com>,
        Tony Luck <tony.luck@...el.com>,
        Dan Williams <dan.j.williams@...el.com>,
        Andi Kleen <ak@...ux.intel.com>,
        Kirill Shutemov <kirill.shutemov@...ux.intel.com>,
        Kuppuswamy Sathyanarayanan <knsathya@...nel.org>,
        Sean Christopherson <seanjc@...gle.com>,
        linux-kernel@...r.kernel.org, x86@...nel.org
Subject: Re: [PATCH v1 05/11] x86/tdx: Add __tdx_module_call() and
 __tdx_hypercall() helper functions

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.

> But, I am fine with passing it via stack, if this is recommended.
> 
> Please let me know.

Yes, please do.

> 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.

> Ok. I will follow your recommendation. I have done it this way to fix
> checkpatch reports.

Yeah but you should not take checkpatch reports to the letter and use
common sense instead.

> If we need helper functions for other output registers in future, we might
> have to add the suffix.

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.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette