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:   Fri, 1 Apr 2022 13:07:26 +0800
From:   Chao Gao <chao.gao@intel.com>
To:     Sean Christopherson <seanjc@google.com>
Cc:     Isaku Yamahata <isaku.yamahata@gmail.com>,
        Paolo Bonzini <pbonzini@redhat.com>, isaku.yamahata@intel.com,
        kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
        Jim Mattson <jmattson@google.com>, erdemaktas@google.com,
        Connor Kuehl <ckuehl@redhat.com>, Chao Gao <chao.gao@intel.com>
Subject: Re: [RFC PATCH v5 008/104] KVM: TDX: Add a function to initialize
 TDX module
Message-ID: <20220401050725.GA12103@gao-cwp>
References: <cover.1646422845.git.isaku.yamahata@intel.com>
 <b92217283fa96b85e9a683ca3fcf1b368cf8d1c4.1646422845.git.isaku.yamahata@intel.com>
 <05aecc5a-e8d2-b357-3bf1-3d0cb247c28d@redhat.com>
 <20220314194513.GD1964605@ls.amr.corp.intel.com>
 <YkTvw5OXTTFf7j4y@google.com>
 <20220331170303.GA2179440@ls.amr.corp.intel.com>
 <YkYCNF3l62IxpmAD@google.com>
 <20220401032741.GA2806@gao-cwp>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220401032741.GA2806@gao-cwp>
User-Agent: Mutt/1.9.4 (2018-02-28)
Precedence: bulk

The original reply was sent to Sean only by mistake. Add others back.

On Fri, Apr 01, 2022 at 11:27:42AM +0800, Chao Gao wrote:
>On Thu, Mar 31, 2022 at 07:34:12PM +0000, Sean Christopherson wrote:
>>+Chao Gao
>>
>>On Thu, Mar 31, 2022, Isaku Yamahata wrote:
>>> On Thu, Mar 31, 2022 at 12:03:15AM +0000, Sean Christopherson <seanjc@google.com> wrote:
>>> > On Mon, Mar 14, 2022, Isaku Yamahata wrote:
>>> > > - VMXON on all pCPUs: The TDX module initialization requires to enable VMX
>>> > > (VMXON) on all present pCPUs.  vmx_hardware_enable() which is called on creating
>>> > > guest does it.  It naturally fits with the TDX module initialization at creating
>>> > > first TD.  I wanted to avoid code to enable VMXON on loading the kvm_intel.ko.
>>> > 
>>> > That's a solvable problem, though making it work without exporting hardware_enable_all()
>>> > could get messy.
>>> 
>>> Could you please explain any reason why it's bad idea to export it?
>>
>>I'd really prefer to keep the hardware enable/disable logic internal to kvm_main.c
>>so that all architectures share a common flow, and so that kvm_main.c is the sole
>>owner.  I'm worried that exposing the helper will lead to other arch/vendor usage,
>>and that will end up with what is effectively duplicate flows.  Deduplicating arch
>>code into generic KVM is usually very difficult.
>>
>>This might also be a good opportunity to make KVM slightly more robust.  Ooh, and
>>we can kill two birds with one stone.  There's an in-flight series to add compatibility
>>checks to hotplug[*].  But rather than special case hotplug, what if we instead do
>>hardware enable/disable during module load, and move the compatibility check into
>>the hardware_enable path?  That fixes the hotplug issue, gives TDX a window for running
>>post-VMXON code in kvm_init(), and makes the broadcast IPI less wasteful on architectures
>>that don't have compatiblity checks.
>
>Sounds good. But more time is wasted on compat checks on architectures
>that have them because they are done each time of enabling hardware.
>A solution for this is caching the result of kvm_arch_check_processor_compat().
>
>>
>>I'm thinking something like this, maybe as a modificatyion to patch 6 in Chao's
>>series, or more likely as a patch 7 so that the hotplug compat checks still get
>>in even
>
>>if the early hardware enable doesn't work on all architectures for some
>>reason.
>
>By "early", do you mean hardware enable during module loading or during CPU hotplug?
>
>And if below change is put into my series, kvm_arch_post_hardware_enable_setup()
>will be an empty function for all architectures until TDX series gets merged.
>So, I prefer to drop kvm_arch_post_hardware_enable_setup() and let TDX series
>introduce it.