On 26/02/2024 9:25 pm, Yamahata, Isaku wrote:
From: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
Add placeholders TDX VM/vcpu structure that overlays with VMX VM/vcpu
structures. Initialize VM structure size and vcpu size/align so that x86
KVM common code knows those size irrespective of VMX or TDX. Those
structures will be populated as guest creation logic develops.
Add helper functions to check if the VM is guest TD and add conversion
functions between KVM VM/VCPU and TDX VM/VCPU.
The changelog is essentially only saying "doing what" w/o "why".
Please at least explain why you invented the 'struct kvm_tdx' and
'struct vcpu_tdx', and why they are invented in this way.
E.g., can we extend 'struct kvm_vmx' for TDX?
struct kvm_tdx {
struct kvm_vmx vmx;
...
};
Signed-off-by: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
---
v19:
- correctly update ops.vm_size, vcpu_size and, vcpu_align by Xiaoyao
v14 -> v15:
- use KVM_X86_TDX_VM
Signed-off-by: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
---
arch/x86/kvm/vmx/main.c | 14 ++++++++++++
arch/x86/kvm/vmx/tdx.c | 1 +
arch/x86/kvm/vmx/tdx.h | 50 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 65 insertions(+)
create mode 100644 arch/x86/kvm/vmx/tdx.h
diff --git a/arch/x86/kvm/vmx/main.c b/arch/x86/kvm/vmx/main.c
index 18aef6e23aab..e11edbd19e7c 100644
--- a/arch/x86/kvm/vmx/main.c
+++ b/arch/x86/kvm/vmx/main.c
@@ -5,6 +5,7 @@
#include "vmx.h"
#include "nested.h"
#include "pmu.h"
+#include "tdx.h"
static bool enable_tdx __ro_after_init;
module_param_named(tdx, enable_tdx, bool, 0444);
@@ -18,6 +19,9 @@ static __init int vt_hardware_setup(void)
return ret;
enable_tdx = enable_tdx && !tdx_hardware_setup(&vt_x86_ops);
+ if (enable_tdx)
+ vt_x86_ops.vm_size = max_t(unsigned int, vt_x86_ops.vm_size,
+ sizeof(struct kvm_tdx));
Now I see why you included 'struct kvm_x86_ops' as function parameter.
Please move it to this patch.
return 0;
}
@@ -215,8 +219,18 @@ static int __init vt_init(void)
* Common KVM initialization _must_ come last, after this, /dev/kvm is
* exposed to userspace!
*/
+ /*
+ * kvm_x86_ops is updated with vt_x86_ops. vt_x86_ops.vm_size must
+ * be set before kvm_x86_vendor_init().
+ */
vcpu_size = sizeof(struct vcpu_vmx);
vcpu_align = __alignof__(struct vcpu_vmx);
+ if (enable_tdx) {
+ vcpu_size = max_t(unsigned int, vcpu_size,
+ sizeof(struct vcpu_tdx));
+ vcpu_align = max_t(unsigned int, vcpu_align,
+ __alignof__(struct vcpu_tdx));
+ }
Since you are updating vm_size in vt_hardware_setup(), I am wondering
whether we can do similar thing for vcpu_size and vcpu_align.
That is, we put them both to 'struct kvm_x86_ops', and you update them
in vt_hardware_setup().
kvm_init() can then just access them directly in this way both
'vcpu_size' and 'vcpu_align' function parameters can be removed.
r = kvm_init(vcpu_size, vcpu_align, THIS_MODULE);
if (r)
goto err_kvm_init;
