On Thu, Mar 14, 2024 at 02:21:04PM +0800,
Binbin Wu <binbin.wu@xxxxxxxxxxxxxxx> wrote:
> > 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));
> > 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().
>
> The comment is not right?
> In this patch, vt_x86_ops.vm_size is set in vt_hardware_setup(),
> which is called in kvm_x86_vendor_init().
>
> Since kvm_x86_ops is updated by kvm_ops_update() with the fields of
> vt_x86_ops. I guess you wanted to say vt_x86_ops.vm_size must be set
> before kvm_ops_update()?
Correct. Here's an updated version.
/*
* vt_hardware_setup() updates vt_x86_ops. Because kvm_ops_update()
* copies vt_x86_ops to kvm_x86_op, vt_x86_ops must be updated before
* kvm_ops_update() called by kvm_x86_vendor_init().
*/
--
Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
