Re: [PATCH v19 022/130] KVM: x86/vmx: Refactor KVM VMX module init/exit functions

[Huang, Kai]

On Mon, 2024-02-26 at 00:25 -0800, isaku.yamahata@xxxxxxxxx wrote:
> From: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
> 
> 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?