Re: [PATCH v3 00/21] TDX host kernel support

From: Kai Huang <kai.huang@intel.com>
To: Dave Hansen <dave.hansen@intel.com>,
	linux-kernel@vger.kernel.org, kvm@vger.kernel.org
Cc: seanjc@google.com, pbonzini@redhat.com, len.brown@intel.com,
	tony.luck@intel.com, rafael.j.wysocki@intel.com,
	reinette.chatre@intel.com, dan.j.williams@intel.com,
	peterz@infradead.org, ak@linux.intel.com,
	kirill.shutemov@linux.intel.com,
	sathyanarayanan.kuppuswamy@linux.intel.com,
	isaku.yamahata@intel.com
Subject: Re: [PATCH v3 00/21] TDX host kernel support
Date: Thu, 28 Apr 2022 12:58:31 +1200	[thread overview]
Message-ID: <c90a10763969077826f42be6f492e3a3e062326b.camel@intel.com> (raw)
In-Reply-To: <d98ca73b-2d2d-757d-e937-acc83cfedfb0@intel.com>

On Wed, 2022-04-27 at 17:50 -0700, Dave Hansen wrote:
> On 4/27/22 17:37, Kai Huang wrote:
> > On Wed, 2022-04-27 at 14:59 -0700, Dave Hansen wrote:
> > > In 5 years, if someone takes this code and runs it on Intel hardware
> > > with memory hotplug, CPU hotplug, NVDIMMs *AND* TDX support, what happens?
> > 
> > I thought we could document this in the documentation saying that this code can
> > only work on TDX machines that don't have above capabilities (SPR for now).  We
> > can change the code and the documentation  when we add the support of those
> > features in the future, and update the documentation.
> > 
> > If 5 years later someone takes this code, he/she should take a look at the
> > documentation and figure out that he/she should choose a newer kernel if the
> > machine support those features.
> > 
> > I'll think about design solutions if above doesn't look good for you.
> 
> No, it doesn't look good to me.
> 
> You can't just say:
> 
> 	/*
> 	 * This code will eat puppies if used on systems with hotplug.
> 	 */
> 
> and merrily await the puppy bloodbath.
> 
> If it's not compatible, then you have to *MAKE* it not compatible in a
> safe, controlled way.
> 
> > > You can't just ignore the problems because they're not present on one
> > > version of the hardware.
> 
> Please, please read this again ^^

OK.  I'll think about solutions and come back later.

> 
> > > What about all the concerns about TDX module configuration changing?
> > 
> > Leaving the TDX module in fully initialized state or shutdown state (in case of
> > error during it's initialization) to the new kernel is fine.  If the new kernel
> > doesn't use TDX at all, then the TDX module won't access memory using it's
> > global TDX KeyID.  If the new kernel wants to use TDX, it will fail on the very
> > first SEAMCALL when it tries to initialize the TDX module, and won't use
> > SEAMCALL to call the TDX module again.  If the new kernel doesn't follow this,
> > then it is a bug in the new kernel, or the new kernel is malicious, in which
> > case it can potentially corrupt the data.  But I don't think we need to consider
> > this as if the new kernel is malicious, then it can corrupt data anyway.
> > 
> > Does this make sense?
> 
> No, I'm pretty lost.  But, I'll look at the next version of this with
> fresh eyes and hopefully you'll have had time to streamline the text by
> then.

OK thanks.

-- 
Thanks,
-Kai