Date:   Tue, 7 Mar 2023 17:11:42 -0800
From:   Isaku Yamahata <isaku.yamahata@gmail.com>
To:     Kai Huang <kai.huang@intel.com>
Cc:     linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
        linux-mm@kvack.org, dave.hansen@intel.com, peterz@infradead.org,
        tglx@linutronix.de, seanjc@google.com, pbonzini@redhat.com,
        dan.j.williams@intel.com, rafael.j.wysocki@intel.com,
        kirill.shutemov@linux.intel.com, ying.huang@intel.com,
        reinette.chatre@intel.com, len.brown@intel.com,
        tony.luck@intel.com, ak@linux.intel.com, isaku.yamahata@intel.com,
        chao.gao@intel.com, sathyanarayanan.kuppuswamy@linux.intel.com,
        david@redhat.com, bagasdotme@gmail.com, sagis@google.com,
        imammedo@redhat.com, isaku.yamahata@gmail.com
Subject: Re: [PATCH v10 00/16] TDX host kernel support
Message-ID: <20230308011142.GA2841114@ls.amr.corp.intel.com>
References: <cover.1678111292.git.kai.huang@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <cover.1678111292.git.kai.huang@intel.com>
Precedence: bulk

On Tue, Mar 07, 2023 at 03:13:45AM +1300,
Kai Huang <kai.huang@intel.com> wrote:

> Intel Trusted Domain Extensions (TDX) protects guest VMs from malicious
> host and certain physical attacks.  TDX specs are available in [1].
> 
> This series is the initial support to enable TDX with minimal code to
> allow KVM to create and run TDX guests.  KVM support for TDX is being
> developed separately[2].  A new "userspace inaccessible memfd" approach
> to support TDX private memory is also being developed[3].  The KVM will
> only support the new "userspace inaccessible memfd" as TDX guest memory.
> 
> This series doesn't aim to support all functionalities, and doesn't aim
> to resolve all things perfectly.  For example, memory hotplug is handled
> in simple way (please refer to "Kernel policy on TDX memory" and "Memory
> hotplug" sections below).
> 
> (For memory hotplug, sorry for broadcasting widely but I cc'ed the
> linux-mm@kvack.org following Kirill's suggestion so MM experts can also
> help to provide comments.)
> 
> And TDX module metadata allocation just uses alloc_contig_pages() to
> allocate large chunk at runtime, thus it can fail.  It is imperfect now
> but _will_ be improved in the future.
> 
> Also, the patch to add the new kernel comline tdx="force" isn't included
> in this initial version, as Dave suggested it isn't mandatory.  But I
> _will_ add one once this initial version gets merged.
> 
> All other optimizations will be posted as follow-up once this initial
> TDX support is upstreamed.
> 
> Hi Dave, Peter, Thomas, Dan (and Intel reviewers),
> 
> The environment to test the new LP.INIT SEAMCALL behaviour hasn't been
> done yet, thus I haven't tested the new behaviour.  Instead, I tested
> with all cpus are online when initializing the TDX module.  CPU hotplug
> path isn't really tested although I did some basic test that I can
> offline some cpus after module initialization, online them again and the
> LP.INIT was skipped successfully for them.
> 
> However I believe there should be no issue when the new module is ready.
> I will test and report back when the new module is ready.
> 
> I would appreciate if folks could review this presumptive series anyway.
>    
> And I would appreciate reviewed-by or acked-by tags if the patches look
> good to you.
> 
> ----- Changelog history: ------
> 
> - v9 -> v10:
> 
>  - Changed the per-cpu initalization handling
>    - Gave up "ensuring all online cpus are TDX-runnable when TDX module
>      is initialized", but just provide two basic functions, tdx_enable()
>      and tdx_cpu_enable(), to let the user of TDX to make sure the
>      tdx_cpu_enable() has been done successfully when the user wants to
>      use particular cpu for TDX.
>    - Thus, moved per-cpu initialization out of tdx_enable().  Now
>      tdx_enable() just assumes VMXON and tdx_cpu_enable() has been done
>      on all online cpus before calling it.
>    - Merged the tdx_enable() skeleton patch and per-cpu initialization
>      patch together to tell better story.
>    - Moved "SEAMCALL infrastructure" patch before the tdx_enable() patch.
> 
>  v9: https://lore.kernel.org/lkml/cover.1676286526.git.kai.huang@intel.com/
> 
> - v8 -> v9:
> 
>  - Added patches to handle TDH.SYS.INIT and TDH.SYS.LP.INIT back.
>  - Other changes please refer to changelog histroy in individual patches.

I've rebased my TDX KVM patches to this patch series and updated initialization.
With all LPs online with the existing TDX module and I did cpu online/offline
while TD running.

Test-by: Isaku Yamahata <isaku.yamahata@intel.com>
-- 
Isaku Yamahata <isaku.yamahata@gmail.com>