Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 26 Jul 2022 16:57:57 -0700
From:   Isaku Yamahata <isaku.yamahata@gmail.com>
To:     Kai Huang <kai.huang@intel.com>
Cc:     isaku.yamahata@intel.com, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org, isaku.yamahata@gmail.com,
        Paolo Bonzini <pbonzini@redhat.com>,
        Chao Gao <chao.gao@intel.com>,
        Sean Christopherson <seanjc@google.com>,
        Suzuki K Poulose <suzuki.poulose@arm.com>,
        Anup Patel <anup@brainfault.org>,
        Claudio Imbrenda <imbrenda@linux.ibm.com>
Subject: Re: [PATCH v7 002/102] Partially revert "KVM: Pass kvm_init()'s
 opaque param to additional arch funcs"
Message-ID: <20220726235757.GF1379820@ls.amr.corp.intel.com>
References: <cover.1656366337.git.isaku.yamahata@intel.com>
 <4566737e3c57c5ab17c0bc29d6f4758744b6eed1.1656366337.git.isaku.yamahata@intel.com>
 <e4604ad23788a6d2950c091d04b7b805684a1a01.camel@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <e4604ad23788a6d2950c091d04b7b805684a1a01.camel@intel.com>
Precedence: bulk

On Wed, Jul 13, 2022 at 01:55:46PM +1200,
Kai Huang <kai.huang@intel.com> wrote:

> On Mon, 2022-06-27 at 14:52 -0700, isaku.yamahata@intel.com wrote:
> > From: Chao Gao <chao.gao@intel.com>
> > 
> > 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.
> > 
> > And changes about kvm_arch_hardware_setup() in original commit are still
> > needed so they are not reverted.
> 
> I tried to dig the history to find out why we are doing this.
> 
> IMHO it's better to give a reason why you need to revert the opaque.  I guess no
> one uses this opaque now doesn't mean we need to remove it?
> 
> Perhaps you should mention this is a preparation to
> hardware_enable_all()/hardware_disable_all() during module loading time. 
> Instead of extending hardware_enable_all()/hardware_disable_all() to take the
> opaque and pass to kvm_arch_check_process_compat(), just remove the opaque.
> 
> Or perhaps just merge this patch to next one?


Here is the updated commit message.

    Partially revert "KVM: Pass kvm_init()'s opaque param to additional arch funcs"
    
    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, 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
    unused opaque argument, another way is to remove the unused opaque
    argument.  This patch is a preparation for the latter by removing the
    argument.


-- 
Isaku Yamahata <isaku.yamahata@gmail.com>