linux-kernel - Re: [PATCH v7 008/102] KVM: x86: Refactor KVM VMX module init/exit functions

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: <20220712003811.GB1379820@ls.amr.corp.intel.com>
Date:   Mon, 11 Jul 2022 17:38:11 -0700
From:   Isaku Yamahata <isaku.yamahata@...il.com>
To:     Kai Huang <kai.huang@...el.com>
Cc:     isaku.yamahata@...el.com, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org, isaku.yamahata@...il.com,
        Paolo Bonzini <pbonzini@...hat.com>
Subject: Re: [PATCH v7 008/102] KVM: x86: Refactor KVM VMX module init/exit
 functions

On Tue, Jun 28, 2022 at 03:53:31PM +1200,
Kai Huang <kai.huang@...el.com> wrote:

> On Mon, 2022-06-27 at 14:53 -0700, isaku.yamahata@...el.com wrote:
> > From: Isaku Yamahata <isaku.yamahata@...el.com>
> > 
> > Currently, KVM VMX module initialization/exit functions are a single
> > function each.  Refactor KVM VMX module initialization functions into KVM
> > common part and VMX part so that TDX specific part can be added cleanly.
> > Opportunistically refactor module exit function as well.
> > 
> > The current module initialization flow is, 1.) calculate the sizes of VMX
> > kvm structure and VMX vcpu structure, 2.) hyper-v specific initialization
> > 3.) report those sizes to the KVM common layer and KVM common
> > initialization, and 4.) VMX specific system-wide initialization.
> > 
> > Refactor the KVM VMX module initialization function into functions with a
> > wrapper function to separate VMX logic in vmx.c from a file, main.c, common
> > among VMX and TDX.  We have a wrapper function, "vt_init() {vmx kvm/vcpu
> > size calculation; hv_vp_assist_page_init(); kvm_init(); vmx_init(); }" in
> > main.c, and hv_vp_assist_page_init() and vmx_init() in vmx.c.
> > hv_vp_assist_page_init() initializes hyper-v specific assist pages,
> > kvm_init() does system-wide initialization of the KVM common layer, and
> > vmx_init() does system-wide VMX initialization.
> > 
> > The KVM architecture common layer allocates struct kvm with reported size
> > for architecture-specific code.  The KVM VMX module defines its structure
> > as struct vmx_kvm { struct kvm; VMX specific members;} and uses it as
> > struct vmx kvm.  Similar for vcpu structure. TDX KVM patches will define
> > TDX specific kvm and vcpu structures, add tdx_pre_kvm_init() to report the
> > sizes of them to the KVM common layer.
> > 
> > The current module exit function is also a single function, a combination
> > of VMX specific logic and common KVM logic.  Refactor it into VMX specific
> > logic and KVM common logic.  This is just refactoring to keep the VMX
> > specific logic in vmx.c from main.c.
> 
> This patch, coupled with the patch:
> 
> 	KVM: VMX: Move out vmx_x86_ops to 'main.c' to wrap VMX and TDX
> 
> Basically provides an infrastructure to support both VMX and TDX.  Why we cannot
> merge them into one patch?  What's the benefit of splitting them?
> 
> At least, why the two patches cannot be put together closely?

It is trivial for the change of "KVM: VMX: Move out vmx_x86_ops to 'main.c' to
wrap VMX and TDX" to introduce no functional change.  But it's not trivial
for this patch to introduce no functional change.

So I moved this patch right after the main.c patch.
-- 
Isaku Yamahata <isaku.yamahata@...il.com>