linux-kernel - Re: [RFC PATCH v5 008/104] KVM: TDX: Add a function to initialize TDX module

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>] [day] [month] [year] [list]

Date:   Fri, 1 Apr 2022 13:07:26 +0800
From:   Chao Gao <chao.gao@...el.com>
To:     Sean Christopherson <seanjc@...gle.com>
Cc:     Isaku Yamahata <isaku.yamahata@...il.com>,
        Paolo Bonzini <pbonzini@...hat.com>, isaku.yamahata@...el.com,
        kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
        Jim Mattson <jmattson@...gle.com>, erdemaktas@...gle.com,
        Connor Kuehl <ckuehl@...hat.com>, Chao Gao <chao.gao@...el.com>
Subject: Re: [RFC PATCH v5 008/104] KVM: TDX: Add a function to initialize
 TDX module

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