Re: [PATCH v19 024/130] KVM: TDX: Add placeholders for TDX VM/vcpu structure

[Isaku Yamahata]

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>