| Date | Tue, 28 Jun 2022 16:30:53 +0800 | Subject | Re: [PATCH v7 025/102] KVM: TDX: initialize VM with TDX specific parameters | From | Xiaoyao Li <> |
| |
On 6/28/2022 5:53 AM, isaku.yamahata@intel.com wrote: > From: Xiaoyao Li <xiaoyao.li@intel.com> > > TDX requires additional parameters for TDX VM for confidential execution to > protect its confidentiality of its memory contents and its CPU state from > any other software, including VMM. When creating guest TD VM before > creating vcpu, the number of vcpu, TSC frequency (that is same among > vcpus. and it can't be changed.) CPUIDs which is emulated by the TDX > module. It means guest can trust those CPUIDs. and sha384 values for > measurement. > > Add new subcommand, KVM_TDX_INIT_VM, to pass parameters for TDX guest. It > assigns encryption key to the TDX guest for memory encryption. TDX > encrypts memory per-guest bases. It assigns device model passes per-VM > parameters for the TDX guest. The maximum number of vcpus, tsc frequency > (TDX guest has fised VM-wide TSC frequency. not per-vcpu. The TDX guest > can not change it.), attributes (production or debug), available extended > features (which is reflected into guest XCR0, IA32_XSS MSR), cpuids, sha384 > measurements, and etc. > > This subcommand is called before creating vcpu and KVM_SET_CPUID2, i.e. > cpuids configurations aren't available yet. So CPUIDs configuration values > needs to be passed in struct kvm_init_vm. It's device model responsibility > to make this cpuid config for KVM_TDX_INIT_VM and KVM_SET_CPUID2. > > Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com> > Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com> > --- > arch/x86/include/asm/kvm_host.h | 2 + > arch/x86/include/asm/tdx.h | 3 + > arch/x86/include/uapi/asm/kvm.h | 33 +++++ > arch/x86/kvm/vmx/tdx.c | 206 ++++++++++++++++++++++++++ > arch/x86/kvm/vmx/tdx.h | 23 +++ > tools/arch/x86/include/uapi/asm/kvm.h | 33 +++++ > 6 files changed, 300 insertions(+) > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > index 342decc69649..81638987cdb9 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -1338,6 +1338,8 @@ struct kvm_arch { > * the global KVM_MAX_VCPU_IDS may lead to significant memory waste. > */ > u32 max_vcpu_ids; > + > + gfn_t gfn_shared_mask;
I think it's better to put in a seperate patch or the patch that consumes it.
> }; > ...
> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c > index 2a9dfd54189f..1273b60a1a00 100644 > --- a/arch/x86/kvm/vmx/tdx.c > +++ b/arch/x86/kvm/vmx/tdx.c > @@ -438,6 +438,209 @@ int tdx_dev_ioctl(void __user *argp) > return 0; > } > > +/* > + * cpuid entry lookup in TDX cpuid config way. > + * The difference is how to specify index(subleaves). > + * Specify index to TDX_CPUID_NO_SUBLEAF for CPUID leaf with no-subleaves. > + */ > +static const struct kvm_cpuid_entry2 *tdx_find_cpuid_entry( > + const struct kvm_cpuid2 *cpuid, u32 function, u32 index) > +{ > + int i; > + > +
superfluous line
> + /* In TDX CPU CONFIG, TDX_CPUID_NO_SUBLEAF means index = 0. */ > + if (index == TDX_CPUID_NO_SUBLEAF) > + index = 0; > + > + for (i = 0; i < cpuid->nent; i++) { > + const struct kvm_cpuid_entry2 *e = &cpuid->entries[i]; > + > + if (e->function == function && > + (e->index == index || > + !(e->flags & KVM_CPUID_FLAG_SIGNIFCANT_INDEX))) > + return e; > + } > + return NULL; > +}
no need for kvm_tdx->tsc_khz field. We have kvm->arch.default_tsc_khz. It seems kvm_tdx->tsc_khz is not used in the following patches.
...
> + > + kvm_tdx->tsc_offset = td_tdcs_exec_read64(kvm_tdx, TD_TDCS_EXEC_TSC_OFFSET); > + kvm_tdx->attributes = td_params->attributes; > + kvm_tdx->xfam = td_params->xfam; > + kvm_tdx->tsc_khz = TDX_TSC_25MHZ_TO_KHZ(td_params->tsc_frequency); > + kvm->max_vcpus = td_params->max_vcpus; > + > + if (td_params->exec_controls & TDX_EXEC_CONTROL_MAX_GPAW) > + kvm->arch.gfn_shared_mask = gpa_to_gfn(BIT_ULL(51)); > + else > + kvm->arch.gfn_shared_mask = gpa_to_gfn(BIT_ULL(47)); > +
....
> diff --git a/tools/arch/x86/include/uapi/asm/kvm.h b/tools/arch/x86/include/uapi/asm/kvm.h > index a9ea3573be1b..779dfd683d66 100644 > --- a/tools/arch/x86/include/uapi/asm/kvm.h > +++ b/tools/arch/x86/include/uapi/asm/kvm.h > @@ -531,6 +531,7 @@ struct kvm_pmu_event_filter { > /* Trust Domain eXtension sub-ioctl() commands. */ > enum kvm_tdx_cmd_id { > KVM_TDX_CAPABILITIES = 0, > + KVM_TDX_INIT_VM, > > KVM_TDX_CMD_NR_MAX, > }; > @@ -576,4 +577,36 @@ struct kvm_tdx_capabilities { > struct kvm_tdx_cpuid_config cpuid_configs[0]; > }; > > +struct kvm_tdx_init_vm { > + __u64 attributes; > + __u32 max_vcpus; > + __u32 tsc_khz;
it needs to align with arch/x86/include/uapi/asm/kvm.h that @tsc_khz needs to be removed.
|