linux-kernel - Re: [PATCH v19 022/130] KVM: x86/vmx: 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: <0f466c5845e9d75b25392ecb5129c4e984052c1b.camel@intel.com>
Date: Thu, 21 Mar 2024 11:27:46 +0000
From: "Huang, Kai" <kai.huang@...el.com>
To: "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "Yamahata,
 Isaku" <isaku.yamahata@...el.com>
CC: "Zhang, Tina" <tina.zhang@...el.com>, "seanjc@...gle.com"
	<seanjc@...gle.com>, "Yuan, Hang" <hang.yuan@...el.com>, "Chen, Bo2"
	<chen.bo@...el.com>, "sagis@...gle.com" <sagis@...gle.com>,
	"isaku.yamahata@...il.com" <isaku.yamahata@...il.com>, "Aktas, Erdem"
	<erdemaktas@...gle.com>, "pbonzini@...hat.com" <pbonzini@...hat.com>
Subject: Re: [PATCH v19 022/130] KVM: x86/vmx: Refactor KVM VMX module
 init/exit functions

On Mon, 2024-02-26 at 00:25 -0800, 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,

					  ^ ',' -> ':'

And please add an empty line to make text more breathable.

> 0.) Check if VMX is supported,
> 1.) hyper-v specific initialization,
> 2.) system-wide x86 specific and vendor specific initialization,
> 3.) Final VMX specific system-wide initialization,
> 4.) calculate the sizes of VMX kvm structure and VMX vcpu structure,
> 5.) report those sizes to the KVM common layer and KVM common
>     initialization

Is there any difference between "KVM common layer" and "KVM common
initialization"?  I think you can remove the former.

> 
> 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.  Introduce a wrapper function for vmx_init().

Sorry I don't quite follow what your are trying to say in the above paragraph.

You have adequately put what is the _current_ flow, and I am expecting to see
the flow _after_ the refactor here.
  
> 
> 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

	 ^vmx_kvm.

Please be more consistent on the words.

> TDX specific kvm and vcpu structures.

Is this paragraph related to the changes in this patch?

For instance, why do you need to point out we will have TDX-specific 'kvm and
vcpu' structures?

> 
> 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.

It's better to make this as a separate paragraph, because it is a summary to
this patch.

And in other words: No functional change intended?