Re: [PATCHv2 00/29] TDX Guest: TDX core support

From: Kai Huang <kai.huang@intel.com>
To: Borislav Petkov <bp@alien8.de>
Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
	tglx@linutronix.de, mingo@redhat.com, dave.hansen@intel.com,
	luto@kernel.org, peterz@infradead.org,
	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
Subject: Re: [PATCHv2 00/29] TDX Guest: TDX core support
Date: Thu, 10 Feb 2022 00:30:33 +1300	[thread overview]
Message-ID: <20220210003033.e57c2925b69bab5cfabf7292@intel.com> (raw)
In-Reply-To: <YgOgog9Pb886lfsv@zn.tnic>

On Wed, 9 Feb 2022 12:08:18 +0100 Borislav Petkov wrote:
> On Wed, Feb 09, 2022 at 11:56:13PM +1300, Kai Huang wrote:
> > TDX host support basically does detection of SEAM, TDX KeyIDs, P-SEAMLDR and
> > initialize the TDX module, so likely TDX host support will introduce couple of
> > new files to do above things respectively,
> 
> Why a couple of new files? How much code is that?

This is the fine names and code size of current internal version that I have:

 .../admin-guide/kernel-parameters.txt         |   6 +
 Documentation/x86/index.rst                   |   1 +
 Documentation/x86/intel-tdx.rst               | 259 +++++++
 arch/x86/Kconfig                              |  14 +
 arch/x86/include/asm/seam.h                   | 213 ++++++
 arch/x86/include/asm/tdx_host.h               |  20 +
 arch/x86/kernel/asm-offsets_64.c              |  18 +
 arch/x86/kernel/cpu/Makefile                  |   1 +
 arch/x86/kernel/cpu/intel.c                   |   6 +
 arch/x86/kernel/cpu/tdx/Makefile              |   1 +
 arch/x86/kernel/cpu/tdx/p-seamldr.c           | 109 +++
 arch/x86/kernel/cpu/tdx/p-seamldr.h           |  14 +
 arch/x86/kernel/cpu/tdx/seam.c                | 105 +++
 arch/x86/kernel/cpu/tdx/seamcall.S            |  80 ++
 arch/x86/kernel/cpu/tdx/tdmr.c                | 581 ++++++++++++++
 arch/x86/kernel/cpu/tdx/tdmr.h                |  28 +
 arch/x86/kernel/cpu/tdx/tdx.c                 | 707 ++++++++++++++++++
 arch/x86/kernel/cpu/tdx/tdx_arch.h            |  88 +++
 arch/x86/kernel/cpu/tdx/tdx_seamcall.h        | 138 ++++
 19 files changed, 2389 insertions(+)
 create mode 100644 Documentation/x86/intel-tdx.rst
 create mode 100644 arch/x86/include/asm/seam.h
 create mode 100644 arch/x86/include/asm/tdx_host.h
 create mode 100644 arch/x86/kernel/cpu/tdx/Makefile
 create mode 100644 arch/x86/kernel/cpu/tdx/p-seamldr.c
 create mode 100644 arch/x86/kernel/cpu/tdx/p-seamldr.h
 create mode 100644 arch/x86/kernel/cpu/tdx/seam.c
 create mode 100644 arch/x86/kernel/cpu/tdx/seamcall.S
 create mode 100644 arch/x86/kernel/cpu/tdx/tdmr.c
 create mode 100644 arch/x86/kernel/cpu/tdx/tdmr.h
 create mode 100644 arch/x86/kernel/cpu/tdx/tdx.c
 create mode 100644 arch/x86/kernel/cpu/tdx/tdx_arch.h
 create mode 100644 arch/x86/kernel/cpu/tdx/tdx_seamcall.h

Because SEAM, P-SEAMLDR can logically be independent, so I feel it's better to
have separate C files for them.  TDMR (TD Memory Region, which is the structure
defined by TDX architecture to manage usable TDX memory) is split out as a
separate file as the logic to deal with it requires non-trival LOC too.

> 
> > and the majority of the code could be self-contained under some
> > directory (currently under arch/x86/kernel/cpu/tdx/, but can be
> > changed of course). Could we have some suggestions on how to organize?
> 
> So we slowly try to move stuff away from arch/x86/kernel/ as that is a
> dumping ground for everything and everything there is "kernel" so that
> part of the path is kinda redundant.
> 
> That's why, for example, we stuck the entry code under arch/x86/entry/.
> 
> I'm thinking long term we probably should stick all confidentail
> computing stuff under its own folder:
> 
> arch/x86/coco/
> 
> for example. The "coco" being COnfidential COmputing, for lack of a
> better idea.
> 
> And there you'll have
> 
> arch/x86/coco/tdx and
> arch/x86/coco/sev
> 
> where to we'll start migrating the AMD stuff eventually too.

Thanks for the information.  However, for now does it make sense to also put
TDX host files under arch/x86/kernel/, or maybe arch/x86/kernel/tdx_host/?

As suggested by Thomas, host SEAMCALL can share TDX guest's __tdx_module_call()
implementation.  Kirill will have a arch/x86/kernel/tdxcall.S which implements
the core body of __tdx_module_call() and is supposed to be included by the new
assembly file to implement the host SEAMCALL function.  From this perspective,
it seems more reasonable to just put all TDX host files under arch/x86/kernel/?

Thanks in advance.