On Thu, Aug 11, 2022 at 09:59:34AM +0000, "Huang, Kai" <kai.huang@xxxxxxxxx> wrote: > On Sun, 2022-08-07 at 15:00 -0700, isaku.yamahata@xxxxxxxxx wrote: > > From: Chao Gao <chao.gao@xxxxxxxxx> > > > > This partially reverts commit b99040853738 ("KVM: Pass kvm_init()'s opaque > > param to additional arch funcs") remove opaque from > > kvm_arch_check_processor_compat because no one uses this opaque now. > > Address conflicts for ARM (due to file movement) and manually handle RISC-V > > which comes after the commit. The change about kvm_arch_hardware_setup() > > in original commit are still needed so they are not reverted. > > > > The current implementation enables hardware (e.g. enable VMX on all CPUs), > > arch-specific initialization for VM creation, > > > > I guess you need to point out _first_ VM? Yes. I'll add "first". > > > and disables hardware (in > > x86, disable VMX on all CPUs) for last VM destruction. > > > > TDX requires its initialization on loading KVM module with VMX enabled on > > all available CPUs. It needs to enable/disable hardware on module > > initialization. To reuse the same logic, one way is to pass around the > > To reuse the same logic for what? I think you need to be specific (and focus) > on why we need this patch: we will opportunistically move CPU compatibility > check to hardware_enable_nolock(), which doesn't take any argument, and this > patch is a preparation to do that. > > > > unused opaque argument, another way is to remove the unused opaque > > argument. This patch is a preparation for the latter by removing the > > argument > > So how about replacing the last two paragraphs with: > > " > Initializing TDX will be done during module loading time, and in order to do > that hardware_enable_all() will be done during module loading time too, as > initializing TDX requires all cpus being in VMX operation. As a result, CPU > compatibility check will be opportunistically moved to hardware_enable_nolock(), > which doesn't take any argument. Instead of passing 'opaque' around to > hardware_enable_nolock() and hardware_enable_all(), just remove the unused > 'opaque' argument from kvm_arch_check_processor_compat(). > " > > Or even simpler: > > " > To support TDX, hardware_enable_all() will be done during module loading time. > As a result, CPU compatibility check will be opportunistically moved to > hardware_enable_nolock(), which doesn't take any argument. Instead of passing > 'opaque' around to hardware_enable_nolock() and hardware_enable_all(), just > remove the unused 'opaque' argument from kvm_arch_check_processor_compat(). > " > > With changelog updated: Thanks, I'll adapt the simpler one. -- Isaku Yamahata <isaku.yamahata@xxxxxxxxx>