Message ID | 20240812224820.34826-3-rick.p.edgecombe@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | TDX vCPU/VM creation | expand |
On 8/13/2024 6:47 AM, Rick Edgecombe wrote: > +/* > + * TD_PARAMS is provided as an input to TDH_MNG_INIT, the size of which is 1024B. > + */ > +struct td_params { > + u64 attributes; > + u64 xfam; > + u16 max_vcpus; > + u8 reserved0[6]; > + > + u64 eptp_controls; > + u64 exec_controls; TDX 1.5 renames 'exec_controls' to 'config_flags', maybe we need update it to match TDX 1.5 since the minimum supported TDX module of linux starts from 1.5. Besides, TDX 1.5 defines more fields that was reserved in TDX 1.0, but most of them are not used by current TDX enabling patches. If we update TD_PARAMS to match with TDX 1.5, should we add them as well? This leads to another topic that defining all the TDX structure in this patch seems unfriendly for review. It seems better to put the introduction of definition and its user in a single patch. > + u16 tsc_frequency; > + u8 reserved1[38]; > + > + u64 mrconfigid[6]; > + u64 mrowner[6]; > + u64 mrownerconfig[6]; > + u64 reserved2[4]; > + > + union { > + DECLARE_FLEX_ARRAY(struct tdx_cpuid_value, cpuid_values); > + u8 reserved3[768]; > + }; > +} __packed __aligned(1024);
On Thu, 2024-08-29 at 21:25 +0800, Xiaoyao Li wrote: > On 8/13/2024 6:47 AM, Rick Edgecombe wrote: > > +/* > > + * TD_PARAMS is provided as an input to TDH_MNG_INIT, the size of which is > > 1024B. > > + */ > > +struct td_params { > > + u64 attributes; > > + u64 xfam; > > + u16 max_vcpus; > > + u8 reserved0[6]; > > + > > + u64 eptp_controls; > > + u64 exec_controls; > > TDX 1.5 renames 'exec_controls' to 'config_flags', maybe we need update > it to match TDX 1.5 since the minimum supported TDX module of linux > starts from 1.5. Agreed. > > Besides, TDX 1.5 defines more fields that was reserved in TDX 1.0, but > most of them are not used by current TDX enabling patches. If we update > TD_PARAMS to match with TDX 1.5, should we add them as well? You mean config_flags or supported "features0"? For config_flags, it seems just one is missing. I don't think we need to add it. > > This leads to another topic that defining all the TDX structure in this > patch seems unfriendly for review. It seems better to put the > introduction of definition and its user in a single patch. Yea.
On 8/30/2024 3:46 AM, Edgecombe, Rick P wrote: > On Thu, 2024-08-29 at 21:25 +0800, Xiaoyao Li wrote: >> On 8/13/2024 6:47 AM, Rick Edgecombe wrote: >>> +/* >>> + * TD_PARAMS is provided as an input to TDH_MNG_INIT, the size of which is >>> 1024B. >>> + */ >>> +struct td_params { >>> + u64 attributes; >>> + u64 xfam; >>> + u16 max_vcpus; >>> + u8 reserved0[6]; >>> + >>> + u64 eptp_controls; >>> + u64 exec_controls; >> >> TDX 1.5 renames 'exec_controls' to 'config_flags', maybe we need update >> it to match TDX 1.5 since the minimum supported TDX module of linux >> starts from 1.5. > > Agreed. > >> >> Besides, TDX 1.5 defines more fields that was reserved in TDX 1.0, but >> most of them are not used by current TDX enabling patches. If we update >> TD_PARAMS to match with TDX 1.5, should we add them as well? > > You mean config_flags or supported "features0"? For config_flags, it seems just > one is missing. I don't think we need to add it. No. I meant NUM_L2_VMS, MSR_CONFIG_CTLS, IA32_ARCH_CAPABILITIES_CONFIG, MRCONFIGSVN and MROWNERCONFIGSVN introduced in TD_PARAMS from TDX 1.5. Only MSR_CONFIG_CTLS and IA32_ARCH_CAPABILITIES_CONFIG likely need enabling for now since they relates to MSR_IA32_ARCH_CAPABILITIES virtualization of TDs. >> >> This leads to another topic that defining all the TDX structure in this >> patch seems unfriendly for review. It seems better to put the >> introduction of definition and its user in a single patch. > > Yea.
On Fri, Aug 30, 2024 at 09:29:19AM +0800, Xiaoyao Li wrote: > On 8/30/2024 3:46 AM, Edgecombe, Rick P wrote: > > On Thu, 2024-08-29 at 21:25 +0800, Xiaoyao Li wrote: > > > On 8/13/2024 6:47 AM, Rick Edgecombe wrote: > > > > +/* > > > > + * TD_PARAMS is provided as an input to TDH_MNG_INIT, the size of which is > > > > 1024B. > > > > + */ > > > > +struct td_params { > > > > + u64 attributes; > > > > + u64 xfam; > > > > + u16 max_vcpus; > > > > + u8 reserved0[6]; > > > > + > > > > + u64 eptp_controls; > > > > + u64 exec_controls; > > > > > > TDX 1.5 renames 'exec_controls' to 'config_flags', maybe we need update > > > it to match TDX 1.5 since the minimum supported TDX module of linux > > > starts from 1.5. > > > > Agreed. I'm doing a patch for this FYI. > > > Besides, TDX 1.5 defines more fields that was reserved in TDX 1.0, but > > > most of them are not used by current TDX enabling patches. If we update > > > TD_PARAMS to match with TDX 1.5, should we add them as well? > > > > You mean config_flags or supported "features0"? For config_flags, it seems just > > one is missing. I don't think we need to add it. > > No. I meant NUM_L2_VMS, MSR_CONFIG_CTLS, IA32_ARCH_CAPABILITIES_CONFIG, > MRCONFIGSVN and MROWNERCONFIGSVN introduced in TD_PARAMS from TDX 1.5. > > Only MSR_CONFIG_CTLS and IA32_ARCH_CAPABILITIES_CONFIG likely need enabling > for now since they relates to MSR_IA32_ARCH_CAPABILITIES virtualization of > TDs. Seems these changes can be separate additional patches. Regards, Tony
On 8/29/24 21:46, Edgecombe, Rick P wrote: >> This leads to another topic that defining all the TDX structure in this >> patch seems unfriendly for review. It seems better to put the >> introduction of definition and its user in a single patch. > > Yea. I don't know, it's easier to check a single patch against the manual. I don't have any objection to leaving everything here instead of scattering it over multiple patches, in fact I think I prefer it. Paolo
On Tue, Sep 10, 2024, Paolo Bonzini wrote: > On 8/29/24 21:46, Edgecombe, Rick P wrote: > > > This leads to another topic that defining all the TDX structure in this > > > patch seems unfriendly for review. It seems better to put the > > > introduction of definition and its user in a single patch. > > > > Yea. > > I don't know, it's easier to check a single patch against the manual. I > don't have any objection to leaving everything here instead of scattering it > over multiple patches, in fact I think I prefer it. +1. There is so much to understand with TDX that trying to provide some form of temporal locality between architectural definitions and the first code to use a given definition seems futile/pointless.
diff --git a/arch/x86/kvm/vmx/tdx.h b/arch/x86/kvm/vmx/tdx.h index e6a232d58e6a..1d6fa81a072d 100644 --- a/arch/x86/kvm/vmx/tdx.h +++ b/arch/x86/kvm/vmx/tdx.h @@ -1,6 +1,8 @@ #ifndef __KVM_X86_VMX_TDX_H #define __KVM_X86_VMX_TDX_H +#include "tdx_arch.h" + #ifdef CONFIG_INTEL_TDX_HOST void tdx_bringup(void); void tdx_cleanup(void); diff --git a/arch/x86/kvm/vmx/tdx_arch.h b/arch/x86/kvm/vmx/tdx_arch.h new file mode 100644 index 000000000000..413619dd92ef --- /dev/null +++ b/arch/x86/kvm/vmx/tdx_arch.h @@ -0,0 +1,158 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* architectural constants/data definitions for TDX SEAMCALLs */ + +#ifndef __KVM_X86_TDX_ARCH_H +#define __KVM_X86_TDX_ARCH_H + +#include <linux/types.h> + +#define TDX_VERSION_SHIFT 16 + +/* + * TDX SEAMCALL API function leaves + */ +#define TDH_VP_ENTER 0 +#define TDH_MNG_ADDCX 1 +#define TDH_MEM_PAGE_ADD 2 +#define TDH_MEM_SEPT_ADD 3 +#define TDH_VP_ADDCX 4 +#define TDH_MEM_PAGE_AUG 6 +#define TDH_MEM_RANGE_BLOCK 7 +#define TDH_MNG_KEY_CONFIG 8 +#define TDH_MNG_CREATE 9 +#define TDH_VP_CREATE 10 +#define TDH_MNG_RD 11 +#define TDH_MR_EXTEND 16 +#define TDH_MR_FINALIZE 17 +#define TDH_VP_FLUSH 18 +#define TDH_MNG_VPFLUSHDONE 19 +#define TDH_MNG_KEY_FREEID 20 +#define TDH_MNG_INIT 21 +#define TDH_VP_INIT 22 +#define TDH_VP_RD 26 +#define TDH_MNG_KEY_RECLAIMID 27 +#define TDH_PHYMEM_PAGE_RECLAIM 28 +#define TDH_MEM_PAGE_REMOVE 29 +#define TDH_MEM_SEPT_REMOVE 30 +#define TDH_SYS_RD 34 +#define TDH_MEM_TRACK 38 +#define TDH_MEM_RANGE_UNBLOCK 39 +#define TDH_PHYMEM_CACHE_WB 40 +#define TDH_PHYMEM_PAGE_WBINVD 41 +#define TDH_VP_WR 43 + +/* TDX control structure (TDR/TDCS/TDVPS) field access codes */ +#define TDX_NON_ARCH BIT_ULL(63) +#define TDX_CLASS_SHIFT 56 +#define TDX_FIELD_MASK GENMASK_ULL(31, 0) + +#define __BUILD_TDX_FIELD(non_arch, class, field) \ + (((non_arch) ? TDX_NON_ARCH : 0) | \ + ((u64)(class) << TDX_CLASS_SHIFT) | \ + ((u64)(field) & TDX_FIELD_MASK)) + +#define BUILD_TDX_FIELD(class, field) \ + __BUILD_TDX_FIELD(false, (class), (field)) + +#define BUILD_TDX_FIELD_NON_ARCH(class, field) \ + __BUILD_TDX_FIELD(true, (class), (field)) + + +/* Class code for TD */ +#define TD_CLASS_EXECUTION_CONTROLS 17ULL + +/* Class code for TDVPS */ +#define TDVPS_CLASS_VMCS 0ULL +#define TDVPS_CLASS_GUEST_GPR 16ULL +#define TDVPS_CLASS_OTHER_GUEST 17ULL +#define TDVPS_CLASS_MANAGEMENT 32ULL + +enum tdx_tdcs_execution_control { + TD_TDCS_EXEC_TSC_OFFSET = 10, +}; + +/* @field is any of enum tdx_tdcs_execution_control */ +#define TDCS_EXEC(field) BUILD_TDX_FIELD(TD_CLASS_EXECUTION_CONTROLS, (field)) + +/* @field is the VMCS field encoding */ +#define TDVPS_VMCS(field) BUILD_TDX_FIELD(TDVPS_CLASS_VMCS, (field)) + +/* @field is any of enum tdx_guest_other_state */ +#define TDVPS_STATE(field) BUILD_TDX_FIELD(TDVPS_CLASS_OTHER_GUEST, (field)) +#define TDVPS_STATE_NON_ARCH(field) BUILD_TDX_FIELD_NON_ARCH(TDVPS_CLASS_OTHER_GUEST, (field)) + +/* Management class fields */ +enum tdx_vcpu_guest_management { + TD_VCPU_PEND_NMI = 11, +}; + +/* @field is any of enum tdx_vcpu_guest_management */ +#define TDVPS_MANAGEMENT(field) BUILD_TDX_FIELD(TDVPS_CLASS_MANAGEMENT, (field)) + +#define TDX_EXTENDMR_CHUNKSIZE 256 + +struct tdx_cpuid_value { + u32 eax; + u32 ebx; + u32 ecx; + u32 edx; +} __packed; + +#define TDX_TD_ATTR_DEBUG BIT_ULL(0) +#define TDX_TD_ATTR_SEPT_VE_DISABLE BIT_ULL(28) +#define TDX_TD_ATTR_PKS BIT_ULL(30) +#define TDX_TD_ATTR_KL BIT_ULL(31) +#define TDX_TD_ATTR_PERFMON BIT_ULL(63) + +/* + * TD_PARAMS is provided as an input to TDH_MNG_INIT, the size of which is 1024B. + */ +struct td_params { + u64 attributes; + u64 xfam; + u16 max_vcpus; + u8 reserved0[6]; + + u64 eptp_controls; + u64 exec_controls; + u16 tsc_frequency; + u8 reserved1[38]; + + u64 mrconfigid[6]; + u64 mrowner[6]; + u64 mrownerconfig[6]; + u64 reserved2[4]; + + union { + DECLARE_FLEX_ARRAY(struct tdx_cpuid_value, cpuid_values); + u8 reserved3[768]; + }; +} __packed __aligned(1024); + +/* + * Guest uses MAX_PA for GPAW when set. + * 0: GPA.SHARED bit is GPA[47] + * 1: GPA.SHARED bit is GPA[51] + */ +#define TDX_EXEC_CONTROL_MAX_GPAW BIT_ULL(0) + +/* + * TDH.VP.ENTER, TDG.VP.VMCALL preserves RBP + * 0: RBP can be used for TDG.VP.VMCALL input. RBP is clobbered. + * 1: RBP can't be used for TDG.VP.VMCALL input. RBP is preserved. + */ +#define TDX_CONTROL_FLAG_NO_RBP_MOD BIT_ULL(2) + + +/* + * TDX requires the frequency to be defined in units of 25MHz, which is the + * frequency of the core crystal clock on TDX-capable platforms, i.e. the TDX + * module can only program frequencies that are multiples of 25MHz. The + * frequency must be between 100mhz and 10ghz (inclusive). + */ +#define TDX_TSC_KHZ_TO_25MHZ(tsc_in_khz) ((tsc_in_khz) / (25 * 1000)) +#define TDX_TSC_25MHZ_TO_KHZ(tsc_in_25mhz) ((tsc_in_25mhz) * (25 * 1000)) +#define TDX_MIN_TSC_FREQUENCY_KHZ (100 * 1000) +#define TDX_MAX_TSC_FREQUENCY_KHZ (10 * 1000 * 1000) + +#endif /* __KVM_X86_TDX_ARCH_H */