linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
	mingo@redhat.com, bp@alien8.de, dave.hansen@intel.com,
	luto@kernel.org, peterz@infradead.org
Cc: sathyanarayanan.kuppuswamy@linux.intel.com, aarcange@redhat.com,
	ak@linux.intel.com, dan.j.williams@intel.com, david@redhat.com,
	hpa@zytor.com, jgross@suse.com, jmattson@google.com,
	joro@8bytes.org, jpoimboe@redhat.com, knsathya@kernel.org,
	pbonzini@redhat.com, sdeep@vmware.com, seanjc@google.com,
	tony.luck@intel.com, vkuznets@redhat.com, wanpengli@tencent.com,
	x86@kernel.org, linux-kernel@vger.kernel.org,
	"Kirill A . Shutemov" <kirill.shutemov@linux.intel.com>,
	Kai Huang <kai.huang@linux.intel.com>
Subject: Re: [PATCHv2 03/29] x86/tdx: Add __tdx_module_call() and __tdx_hypercall() helper functions
Date: Tue, 01 Feb 2022 20:58:59 +0100	[thread overview]
Message-ID: <87a6faz7cs.ffs@tglx> (raw)
In-Reply-To: <20220124150215.36893-4-kirill.shutemov@linux.intel.com>

Kirill,

On Mon, Jan 24 2022 at 18:01, Kirill A. Shutemov wrote:
> --- a/arch/x86/include/asm/tdx.h
> +++ b/arch/x86/include/asm/tdx.h
> @@ -8,11 +8,51 @@
>  #define TDX_CPUID_LEAF_ID	0x21
>  #define TDX_IDENT		"IntelTDX    "
>  
> +#define TDX_HYPERCALL_STANDARD  0
> +
> +/*
> + * Used in __tdx_module_call() to gather the output registers'
> + * values of the TDCALL instruction when requesting services from
> + * the TDX module. This is a software only structure and not part
> + * of the TDX module/VMM ABI
> + */
> +struct tdx_module_output {
> +	u64 rcx;
> +	u64 rdx;
> +	u64 r8;
> +	u64 r9;
> +	u64 r10;
> +	u64 r11;
> +};

I've seen exactly the same struct named seamcall_regs_out in the TDX
host series. I assume that's not coincidence which begs the question why
this is required twice with different names.

> +#ifdef CONFIG_INTEL_TDX_GUEST
> +	BLANK();
> +	/* Offset for fields in tdx_module_output */
> +	OFFSET(TDX_MODULE_rcx, tdx_module_output, rcx);
> +	OFFSET(TDX_MODULE_rdx, tdx_module_output, rdx);
> +	OFFSET(TDX_MODULE_r8,  tdx_module_output, r8);
> +	OFFSET(TDX_MODULE_r9,  tdx_module_output, r9);
> +	OFFSET(TDX_MODULE_r10, tdx_module_output, r10);
> +	OFFSET(TDX_MODULE_r11, tdx_module_output, r11);

Which obviously duplicates the above part as well.

> + *-------------------------------------------------------------------------
> + * TDCALL ABI:
> + *-------------------------------------------------------------------------
> + * Input Registers:
> + *
> + * RAX                 - TDCALL Leaf number.
> + * RCX,RDX,R8-R9       - TDCALL Leaf specific input registers.
> + *
> + * Output Registers:
> + *
> + * RAX                 - TDCALL instruction error code.
> + * RCX,RDX,R8-R11      - TDCALL Leaf specific output registers.
> + *
> + *-------------------------------------------------------------------------
> + *
> + * __tdx_module_call() function ABI:
> + *
> + * @fn  (RDI)          - TDCALL Leaf ID,    moved to RAX
> + * @rcx (RSI)          - Input parameter 1, moved to RCX
> + * @rdx (RDX)          - Input parameter 2, moved to RDX
> + * @r8  (RCX)          - Input parameter 3, moved to R8
> + * @r9  (R8)           - Input parameter 4, moved to R9
> + *
> + * @out (R9)           - struct tdx_module_output pointer
> + *                       stored temporarily in R12 (not
> + *                       shared with the TDX module). It
> + *                       can be NULL.
> + *
> + * Return status of TDCALL via RAX.
> + */

And unsurprisingly this function and __seamcall of the other patch set
are very similar aside of the calling convention (__seamcall has a
struct for the input parameters) and the obvious difference that one
issues TDCALL and the other SEAMCALL.

So can we please have _one_ implementation and the same struct(s) for
the module call which is exactly the same for host and guest except for
the instruction used.

IOW, this begs a macro implementation

.macro TDX_MODULE_CALL host:req

       ....

        .if \host
        seamcall
        .else
	tdcall
        .endif

       ....

So the actual functions become:

SYM_FUNC_START(__tdx_module_call)
        FRAME_BEGIN
        TDX_MODULE_CALL host=0
        FRAME_END
        ret
SYM_FUNC_END(__tdx_module_call)

SYM_FUNC_START(__tdx_seam_call)
        FRAME_BEGIN
        TDX_MODULE_CALL host=1
        FRAME_END
        ret
SYM_FUNC_END(__tdx_seam_call)

Hmm?

> +/*
> + * Wrapper for standard use of __tdx_hypercall with panic report
> + * for TDCALL error.
> + */
> +static inline u64 _tdx_hypercall(u64 fn, u64 r12, u64 r13, u64 r14,
> +				 u64 r15, struct tdx_hypercall_output
> *out)

This begs the question whether having a struct hypercall_input similar
to the way how seamcall input parameters are implemented makes more
sense than 7 function arguments. Hmm?

Thanks,

        tglx

  reply	other threads:[~2022-02-01 19:59 UTC|newest]

Thread overview: 154+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-24 15:01 [PATCHv2 00/29] TDX Guest: TDX core support Kirill A. Shutemov
2022-01-24 15:01 ` [PATCHv2 01/29] x86/tdx: Detect running as a TDX guest in early boot Kirill A. Shutemov
2022-02-01 19:29   ` Thomas Gleixner
2022-02-01 23:14     ` Kirill A. Shutemov
2022-02-03  0:32       ` Josh Poimboeuf
2022-02-03 14:09         ` Kirill A. Shutemov
2022-02-03 15:13           ` Dave Hansen
2022-01-24 15:01 ` [PATCHv2 02/29] x86/tdx: Extend the cc_platform_has() API to support TDX guests Kirill A. Shutemov
2022-02-01 19:31   ` Thomas Gleixner
2022-01-24 15:01 ` [PATCHv2 03/29] x86/tdx: Add __tdx_module_call() and __tdx_hypercall() helper functions Kirill A. Shutemov
2022-02-01 19:58   ` Thomas Gleixner [this message]
2022-02-02  2:55     ` Kirill A. Shutemov
2022-02-02 10:59       ` Kai Huang
2022-02-03 14:44         ` Kirill A. Shutemov
2022-02-03 23:47           ` Kai Huang
2022-02-04  3:43           ` Kirill A. Shutemov
2022-02-04  9:51             ` Kai Huang
2022-02-04 13:20               ` Kirill A. Shutemov
2022-02-04 10:12             ` Kai Huang
2022-02-04 13:18               ` Kirill A. Shutemov
2022-02-05  0:06                 ` Kai Huang
2022-02-02 17:08       ` Thomas Gleixner
2022-01-24 15:01 ` [PATCHv2 04/29] x86/traps: Add #VE support for TDX guest Kirill A. Shutemov
2022-02-01 21:02   ` Thomas Gleixner
2022-02-01 21:26     ` Sean Christopherson
2022-02-12  1:42     ` Kirill A. Shutemov
2022-01-24 15:01 ` [PATCHv2 05/29] x86/tdx: Add HLT support for TDX guests Kirill A. Shutemov
2022-01-29 14:53   ` Borislav Petkov
2022-01-29 22:30     ` [PATCHv2.1 " Kirill A. Shutemov
2022-02-01 21:21       ` Thomas Gleixner
2022-02-02 12:48         ` Kirill A. Shutemov
2022-02-02 17:17           ` Thomas Gleixner
2022-02-04 16:55             ` Kirill A. Shutemov
2022-02-07 22:52               ` Sean Christopherson
2022-02-09 14:34                 ` Kirill A. Shutemov
2022-02-09 18:05                   ` Sean Christopherson
2022-02-09 22:23                     ` Kirill A. Shutemov
2022-02-10  1:21                       ` Sean Christopherson
2022-01-24 15:01 ` [PATCHv2 06/29] x86/tdx: Add MSR " Kirill A. Shutemov
2022-02-01 21:38   ` Thomas Gleixner
2022-02-02 13:06     ` Kirill A. Shutemov
2022-02-02 17:18       ` Thomas Gleixner
2022-01-24 15:01 ` [PATCHv2 07/29] x86/tdx: Handle CPUID via #VE Kirill A. Shutemov
2022-02-01 21:39   ` Thomas Gleixner
2022-01-24 15:01 ` [PATCHv2 08/29] x86/tdx: Handle in-kernel MMIO Kirill A. Shutemov
2022-01-24 19:30   ` Josh Poimboeuf
2022-01-24 22:08     ` Kirill A. Shutemov
2022-01-24 23:04       ` Josh Poimboeuf
2022-01-24 22:40   ` Dave Hansen
2022-01-24 23:04     ` [PATCHv2.1 " Kirill A. Shutemov
2022-02-01 16:14       ` Borislav Petkov
2022-02-01 22:30   ` [PATCHv2 " Thomas Gleixner
2022-01-24 15:01 ` [PATCHv2 09/29] x86/tdx: Detect TDX at early kernel decompression time Kirill A. Shutemov
2022-02-01 18:30   ` Borislav Petkov
2022-02-01 22:33   ` Thomas Gleixner
2022-01-24 15:01 ` [PATCHv2 10/29] x86: Consolidate port I/O helpers Kirill A. Shutemov
2022-02-01 22:36   ` Thomas Gleixner
2022-01-24 15:01 ` [PATCHv2 11/29] x86/boot: Allow to hook up alternative " Kirill A. Shutemov
2022-02-01 19:02   ` Borislav Petkov
2022-02-01 22:39   ` Thomas Gleixner
2022-02-01 22:53     ` Thomas Gleixner
2022-02-02 17:20       ` Kirill A. Shutemov
2022-02-02 19:05         ` Thomas Gleixner
2022-01-24 15:01 ` [PATCHv2 12/29] x86/boot/compressed: Support TDX guest port I/O at decompression time Kirill A. Shutemov
2022-02-01 22:55   ` Thomas Gleixner
2022-01-24 15:01 ` [PATCHv2 13/29] x86/tdx: Add port I/O emulation Kirill A. Shutemov
2022-02-01 23:01   ` Thomas Gleixner
2022-02-02  6:22   ` Borislav Petkov
2022-01-24 15:02 ` [PATCHv2 14/29] x86/tdx: Early boot handling of port I/O Kirill A. Shutemov
2022-02-01 23:02   ` Thomas Gleixner
2022-02-02 10:09   ` Borislav Petkov
2022-01-24 15:02 ` [PATCHv2 15/29] x86/tdx: Wire up KVM hypercalls Kirill A. Shutemov
2022-02-01 23:05   ` Thomas Gleixner
2022-01-24 15:02 ` [PATCHv2 16/29] x86/boot: Add a trampoline for booting APs via firmware handoff Kirill A. Shutemov
2022-02-01 23:06   ` Thomas Gleixner
2022-02-02 11:27   ` Borislav Petkov
2022-02-04 11:27     ` Kuppuswamy, Sathyanarayanan
2022-02-04 13:49       ` Borislav Petkov
2022-02-15 21:36         ` Kirill A. Shutemov
2022-02-16 10:07           ` Borislav Petkov
2022-02-16 14:10             ` Kirill A. Shutemov
2022-02-10  0:25       ` Kai Huang
2022-01-24 15:02 ` [PATCHv2 17/29] x86/acpi, x86/boot: Add multiprocessor wake-up support Kirill A. Shutemov
2022-02-01 23:27   ` Thomas Gleixner
2022-02-05 12:37     ` Kuppuswamy, Sathyanarayanan
2022-01-24 15:02 ` [PATCHv2 18/29] x86/boot: Avoid #VE during boot for TDX platforms Kirill A. Shutemov
2022-02-02  0:04   ` Thomas Gleixner
2022-02-11 16:13     ` Kirill A. Shutemov
2022-01-24 15:02 ` [PATCHv2 19/29] x86/topology: Disable CPU online/offline control for TDX guests Kirill A. Shutemov
2022-02-02  0:09   ` Thomas Gleixner
2022-02-02  0:11     ` Thomas Gleixner
2022-02-03 15:00       ` Borislav Petkov
2022-02-03 21:26         ` Thomas Gleixner
2022-01-24 15:02 ` [PATCHv2 20/29] x86/tdx: Get page shared bit info from the TDX module Kirill A. Shutemov
2022-02-02  0:14   ` Thomas Gleixner
2022-02-07 22:27     ` Sean Christopherson
2022-02-07 10:44   ` Borislav Petkov
2022-01-24 15:02 ` [PATCHv2 21/29] x86/tdx: Exclude shared bit from __PHYSICAL_MASK Kirill A. Shutemov
2022-02-02  0:18   ` Thomas Gleixner
2022-01-24 15:02 ` [PATCHv2 22/29] x86/tdx: Make pages shared in ioremap() Kirill A. Shutemov
2022-02-02  0:25   ` Thomas Gleixner
2022-02-02 19:27     ` Kirill A. Shutemov
2022-02-02 19:47       ` Thomas Gleixner
2022-02-07 16:27   ` Borislav Petkov
2022-02-07 16:57     ` Dave Hansen
2022-02-07 17:28       ` Borislav Petkov
2022-02-14 22:09         ` Kirill A. Shutemov
2022-02-15 10:50           ` Borislav Petkov
2022-02-15 14:49           ` Tom Lendacky
2022-02-15 15:41             ` Kirill A. Shutemov
2022-02-15 15:55               ` Tom Lendacky
2022-02-15 16:27                 ` Kirill A. Shutemov
2022-02-15 16:34                   ` Dave Hansen
2022-02-15 17:33                     ` Kirill A. Shutemov
2022-02-16  9:58                       ` Borislav Petkov
2022-02-16 15:37                         ` Kirill A. Shutemov
2022-02-17 15:24                           ` Borislav Petkov
2022-01-24 15:02 ` [PATCHv2 23/29] x86/tdx: Add helper to convert memory between shared and private Kirill A. Shutemov
2022-02-02  0:35   ` Thomas Gleixner
2022-02-08 12:12   ` Borislav Petkov
2022-02-09 23:21     ` Kirill A. Shutemov
2022-01-24 15:02 ` [PATCHv2 24/29] x86/mm/cpa: Add support for TDX shared memory Kirill A. Shutemov
2022-02-02  1:27   ` Thomas Gleixner
2022-01-24 15:02 ` [PATCHv2 25/29] x86/kvm: Use bounce buffers for TD guest Kirill A. Shutemov
2022-01-24 15:02 ` [PATCHv2 26/29] x86/tdx: ioapic: Add shared bit for IOAPIC base address Kirill A. Shutemov
2022-02-02  1:33   ` Thomas Gleixner
2022-02-04 22:09     ` Yamahata, Isaku
2022-02-04 22:31     ` Kirill A. Shutemov
2022-02-07 14:08       ` Tom Lendacky
2022-01-24 15:02 ` [PATCHv2 27/29] ACPICA: Avoid cache flush on TDX guest Kirill A. Shutemov
2022-01-24 15:02 ` [PATCHv2 28/29] x86/tdx: Warn about unexpected WBINVD Kirill A. Shutemov
2022-02-02  1:46   ` Thomas Gleixner
2022-02-04 21:35     ` Kirill A. Shutemov
2022-01-24 15:02 ` [PATCHv2 29/29] Documentation/x86: Document TDX kernel architecture Kirill A. Shutemov
2022-02-24  9:08   ` Xiaoyao Li
2022-02-09 10:56 ` [PATCHv2 00/29] TDX Guest: TDX core support Kai Huang
2022-02-09 11:08   ` Borislav Petkov
2022-02-09 11:30     ` Kai Huang
2022-02-09 11:40       ` Borislav Petkov
2022-02-09 11:48         ` Kai Huang
2022-02-09 11:56           ` Borislav Petkov
2022-02-09 11:58             ` Kai Huang
2022-02-09 16:50             ` Sean Christopherson
2022-02-09 19:11               ` Borislav Petkov
2022-02-09 20:07                 ` Sean Christopherson
2022-02-09 20:36                   ` Borislav Petkov
2022-02-10  0:05                     ` Kai Huang
2022-02-16 16:08                       ` Sean Christopherson
2022-02-16 15:48                     ` Kirill A. Shutemov
2022-02-17 15:19                       ` Borislav Petkov
2022-02-17 15:26                         ` Kirill A. Shutemov
2022-02-17 15:34                           ` Borislav Petkov
2022-02-17 15:29                         ` Sean Christopherson
2022-02-17 15:31                           ` Borislav Petkov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87a6faz7cs.ffs@tglx \
    --to=tglx@linutronix.de \
    --cc=aarcange@redhat.com \
    --cc=ak@linux.intel.com \
    --cc=bp@alien8.de \
    --cc=dan.j.williams@intel.com \
    --cc=dave.hansen@intel.com \
    --cc=david@redhat.com \
    --cc=hpa@zytor.com \
    --cc=jgross@suse.com \
    --cc=jmattson@google.com \
    --cc=joro@8bytes.org \
    --cc=jpoimboe@redhat.com \
    --cc=kai.huang@linux.intel.com \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=knsathya@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mingo@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peterz@infradead.org \
    --cc=sathyanarayanan.kuppuswamy@linux.intel.com \
    --cc=sdeep@vmware.com \
    --cc=seanjc@google.com \
    --cc=tony.luck@intel.com \
    --cc=vkuznets@redhat.com \
    --cc=wanpengli@tencent.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).