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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240314163753.GP935089@ls.amr.corp.intel.com>
Date: Thu, 14 Mar 2024 09:37:53 -0700
From: Isaku Yamahata <isaku.yamahata@...el.com>
To: Binbin Wu <binbin.wu@...ux.intel.com>
Cc: isaku.yamahata@...el.com, kvm@...r.kernel.org,
	linux-kernel@...r.kernel.org, isaku.yamahata@...il.com,
	Paolo Bonzini <pbonzini@...hat.com>, erdemaktas@...gle.com,
	Sean Christopherson <seanjc@...gle.com>,
	Sagi Shahar <sagis@...gle.com>, Kai Huang <kai.huang@...el.com>,
	chen.bo@...el.com, hang.yuan@...el.com, tina.zhang@...el.com,
	isaku.yamahata@...ux.intel.com
Subject: Re: [PATCH v19 024/130] KVM: TDX: Add placeholders for TDX VM/vcpu
 structure

On Thu, Mar 14, 2024 at 02:21:04PM +0800,
Binbin Wu <binbin.wu@...ux.intel.com> 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@...el.com>