Date:   Fri, 29 Apr 2022 17:43:27 +1200
From:   Kai Huang <kai.huang@...el.com>
To:     Dan Williams <dan.j.williams@...el.com>
Cc:     Dave Hansen <dave.hansen@...el.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        KVM list <kvm@...r.kernel.org>,
        Sean Christopherson <seanjc@...gle.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        "Brown, Len" <len.brown@...el.com>,
        "Luck, Tony" <tony.luck@...el.com>,
        Rafael J Wysocki <rafael.j.wysocki@...el.com>,
        Reinette Chatre <reinette.chatre@...el.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Andi Kleen <ak@...ux.intel.com>,
        "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
        Kuppuswamy Sathyanarayanan 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>,
        Isaku Yamahata <isaku.yamahata@...el.com>
Subject: Re: [PATCH v3 00/21] TDX host kernel support

On Thu, 2022-04-28 at 19:58 -0700, Dan Williams wrote:
> On Wed, Apr 27, 2022 at 6:21 PM Kai Huang <kai.huang@...el.com> wrote:
> > 
> > On Wed, 2022-04-27 at 18:01 -0700, Dan Williams wrote:
> > > On Tue, Apr 26, 2022 at 1:10 PM Dave Hansen <dave.hansen@...el.com> wrote:
> > > [..]
> > > > > 3. Memory hotplug
> > > > > 
> > > > > The first generation of TDX architecturally doesn't support memory
> > > > > hotplug.  And the first generation of TDX-capable platforms don't support
> > > > > physical memory hotplug.  Since it physically cannot happen, this series
> > > > > doesn't add any check in ACPI memory hotplug code path to disable it.
> > > > > 
> > > > > A special case of memory hotplug is adding NVDIMM as system RAM using
> > > 
> > > Saw "NVDIMM" mentioned while browsing this, so stopped to make a comment...
> > > 
> > > > > kmem driver.  However the first generation of TDX-capable platforms
> > > > > cannot enable TDX and NVDIMM simultaneously, so in practice this cannot
> > > > > happen either.
> > > > 
> > > > What prevents this code from today's code being run on tomorrow's
> > > > platforms and breaking these assumptions?
> > > 
> > > The assumption is already broken today with NVDIMM-N. The lack of
> > > DDR-T support on TDX enabled platforms has zero effect on DDR-based
> > > persistent memory solutions. In other words, please describe the
> > > actual software and hardware conflicts at play here, and do not make
> > > the mistake of assuming that "no DDR-T support on TDX platforms" ==
> > > "no NVDIMM support".
> > 
> > Sorry I got this information from planning team or execution team I guess. I was
> > told NVDIMM and TDX cannot "co-exist" on the first generation of TDX capable
> > machine.  "co-exist" means they cannot be turned on simultaneously on the same
> > platform.  I am also not aware NVDIMM-N, nor the difference between DDR based
> > and DDR-T based persistent memory.  Could you give some more background here so
> > I can take a look?
> 
> My rough understanding is that TDX makes use of metadata communicated
> "on the wire" for DDR, but that infrastructure is not there for DDR-T.
> However, there are plenty of DDR based NVDIMMs that use super-caps /
> batteries and flash to save contents. I believe the concern for TDX is
> that the kernel needs to know not use TDX accepted PMEM as PMEM
> because the contents saved by the DIMM's onboard energy source are
> unreadable outside of a TD.
> 
> Here is one of the links that comes up in a search for NVDIMM-N.
> 
> https://www.snia.org/educational-library/what-you-can-do-nvdimm-n-and-nvdimm-p-2019

Thanks for the info.  I need some more time to digest those different types of
DDRs and NVDIMMs.  However I guess they are not quite relevant since TDX has a
concept of "Convertible Memory Region".  Please see below.

> 
> > 
> > > 
> > > > > Another case is admin can use 'memmap' kernel command line to create
> > > > > legacy PMEMs and use them as TD guest memory, or theoretically, can use
> > > > > kmem driver to add them as system RAM.  To avoid having to change memory
> > > > > hotplug code to prevent this from happening, this series always include
> > > > > legacy PMEMs when constructing TDMRs so they are also TDX memory.
> > > 
> > > I am not sure what you are trying to say here?
> > 
> > We want to always make sure the memory managed by page allocator is TDX memory.
> 
> That only seems possible if the kernel is given a TDX capable physical
> address map at the beginning of time.

Yes TDX architecture has a concept "Convertible Memory Region" (CMR). The memory
used by TDX must be convertible memory.  BIOS generates an array of CMR entries
during boot and they are verified by MCHECK.  CMRs are static during machine's
runtime.

> 
> > So if the legacy PMEMs are unconditionally configured as TDX memory, then we
> > don't need to prevent them from being added as system memory via kmem driver.
> 
> I think that is too narrow of a focus.
> 
> Does a memory map exist for the physical address ranges that are TDX
> capable? Please don't say EFI_MEMORY_CPU_CRYPTO, as that single bit is
> ambiguous beyond the point of utility across the industry's entire
> range of confidential computing memory capabilities.
> 
> One strawman would be an ACPI table with contents like:
> 
> struct acpi_protected_memory {
>    struct range range;
>    uuid_t platform_mem_crypto_capability;
> };
> 
> With some way to map those uuids to a set of platform vendor specific
> constraints and specifications. Some would be shared across
> confidential computing vendors, some might be unique. Otherwise, I do
> not see how you enforce the expectation of "all memory in the page
> allocator is TDX capable". 
> 

Please see above.  TDX has CMR.

> The other alternative is that *none* of the
> memory in the page allocator is TDX capable and a special memory
> allocation device is used to map memory for TDs. In either case a map
> of all possible TDX memory is needed and the discussion above seems
> like an incomplete / "hopeful" proposal about the memory dax_kmem, or
> other sources, might online. 

Yes we are also developing a new memfd based approach to support TD guest
memory.  Please see my another reply to you.


> See the CXL CEDT CFWMS (CXL Fixed Memory
> Window Structure) as an example of an ACPI table that sets the
> kernel's expectations about how a physical address range might be
> used.
> 
> https://www.computeexpresslink.org/spec-landing

Thanks for the info. I'll take a look to get some background.

> 
> > 
> > > 
> > > > > 4. CPU hotplug
> > > > > 
> > > > > The first generation of TDX architecturally doesn't support ACPI CPU
> > > > > hotplug.  All logical cpus are enabled by BIOS in MADT table.  Also, the
> > > > > first generation of TDX-capable platforms don't support ACPI CPU hotplug
> > > > > either.  Since this physically cannot happen, this series doesn't add any
> > > > > check in ACPI CPU hotplug code path to disable it.
> > > 
> > > What are the actual challenges posed to TDX with respect to CPU hotplug?
> > 
> > During the TDX module initialization, there is a step to call SEAMCALL on all
> > logical cpus to initialize per-cpu TDX staff.  TDX doesn't support initializing
> > the new hot-added CPUs after the initialization.  There are MCHECK/BIOS changes
> > to enforce this check too I guess but I don't know details about this.
> 
> Is there an ACPI table that indicates CPU-x passed the check? Or since
> the BIOS is invoked in the CPU-online path, is it trusted to suppress
> those events for CPUs outside of the mcheck domain?

No the TDX module (and the P-SEAMLDR) internally maintains some data to record
the total number of LPs and packages, and which logical cpu has been
initialized, etc.

I asked Intel guys whether BIOS would suppress an ACPI CPU hotplug event but I
never got a concrete answer.  I'll try again.

> 
> > > > > Also, only TDX module initialization requires all BIOS-enabled cpus are
> > > 
> > > Please define "BIOS-enabled" cpus. There is no "BIOS-enabled" line in
> > > /proc/cpuinfo for example.
> > 
> > It means the CPUs with "enable" bit set in the MADT table.
> 
> That just indicates to the present CPUs and then a hot add event
> changes the state of now present CPUs to enabled. Per above is the
> BIOS responsible for rejecting those new CPUs, or is the kernel?

I'll ask BIOS guys again to see whether BIOS will suppress ACPI CPU hotplug
event.  But I think we can have a simple patch to reject ACPI CPU hotplug if
platform is TDX-capable?

Or do you think we don't need to explicitly reject ACPI CPU hotplug if we can
confirm with BIOS guys that it will suppress on TDX capable machine?

-- 
Thanks,
-Kai

Powered by blists - more mailing lists