Received-SPF: pass (google.com: domain of linux-kernel+bounces-180515-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Wed, 15 May 2024 17:27:16 -0700
From: Isaku Yamahata <isaku.yamahata@intel.com>
To: "Edgecombe, Rick P" <rick.p.edgecombe@intel.com>
Cc: "seanjc@google.com" <seanjc@google.com>,
	"Huang, Kai" <kai.huang@intel.com>,
	"Yamahata, Isaku" <isaku.yamahata@intel.com>,
	"pbonzini@redhat.com" <pbonzini@redhat.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"sagis@google.com" <sagis@google.com>,
	"isaku.yamahata@linux.intel.com" <isaku.yamahata@linux.intel.com>,
	"Aktas, Erdem" <erdemaktas@google.com>,
	"Zhao, Yan Y" <yan.y.zhao@intel.com>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"dmatlack@google.com" <dmatlack@google.com>,
	"isaku.yamahata@gmail.com" <isaku.yamahata@gmail.com>
Subject: Re: [PATCH 08/16] KVM: x86/mmu: Bug the VM if kvm_zap_gfn_range() is
 called for TDX
Message-ID: <20240516002716.GH168153@ls.amr.corp.intel.com>
References: <20240515005952.3410568-1-rick.p.edgecombe@intel.com>
 <20240515005952.3410568-9-rick.p.edgecombe@intel.com>
 <ZkTWDfuYD-ThdYe6@google.com>
 <20240515162240.GC168153@ls.amr.corp.intel.com>
 <eab9201e-702e-46bc-9782-d6dfe3da2127@intel.com>
 <d4c96caffd2633a70a140861d91794cdb54c7655.camel@intel.com>
 <66afc965-b3f5-41e5-8b8e-d19e7084b690@intel.com>
 <3879ed41213652da74c5de3e437f732dfb2324d7.camel@intel.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <3879ed41213652da74c5de3e437f732dfb2324d7.camel@intel.com>

On Thu, May 16, 2024 at 12:13:44AM +0000,
"Edgecombe, Rick P" <rick.p.edgecombe@intel.com> wrote:

> On Thu, 2024-05-16 at 11:38 +1200, Huang, Kai wrote:
> > On 16/05/2024 11:14 am, Edgecombe, Rick P wrote:
> > > On Thu, 2024-05-16 at 10:17 +1200, Huang, Kai wrote:
> > > > > TDX has several aspects related to the TDP MMU.
> > > > > 1) Based on the faulting GPA, determine which KVM page table to walk.
> > > > >        (private-vs-shared)
> > > > > 2) Need to call TDX SEAMCALL to operate on Secure-EPT instead of direct
> > > > > memory
> > > > >        load/store.  TDP MMU needs hooks for it.
> > > > > 3) The tables must be zapped from the leaf. not the root or the middle.
> > > > > 
> > > > > For 1) and 2), what about something like this?  TDX backend code will
> > > > > set
> > > > > kvm->arch.has_mirrored_pt = true; I think we will use
> > > > > kvm_gfn_shared_mask()
> > > > > only
> > > > > for address conversion (shared<->private).
> > > 
> > > 1 and 2 are not the same as "mirrored" though. You could have a design that
> > > mirrors half of the EPT and doesn't track it with separate roots. In fact, 1
> > > might be just a KVM design choice, even for TDX.
> > 
> > I am not sure whether I understand this correctly.  If they are not 
> > tracked with separate roots, it means they use the same page table (root).
> 
> There are three roots, right? Shared, private and mirrored. Shared and mirrored
> don't have to be different roots, but it makes some operations arguably easier
> to have it that way.

Do you have something like KVM_X86_SW_PROTECTED_VM with mirrored PT in mind?
or TDX thing?


> > So IIUC what you said is to support "mirror PT" at any sub-tree of the 
> > page table?
> > 
> > That will only complicate things.  I don't think we should consider 
> > this.  In reality, we only have TDX and SEV-SNP.  We should have a 
> > simple solution to cover both of them.
> 
> Look at "bool is_private" in kvm_tdp_mmu_map(). Do you see how it switches
> between different roots in the iterator? That is one use.
> 
> The second use is to decide whether to call out to the x86_ops. It happens via
> the role bit in the sp, which is copied from the parent sp role. The root's bit
> is set originally via a kvm_gfn_shared_mask() check.
> 
> BTW, the role bit is the thing I'm wondering if we really need, because we have
> shared_mask. While the shared_mask is used for lots of things today, we need
> still need it for masking GPAs. Where as the role bit is only needed to know if
> a SP is for private (which we can tell from the GPA).

I started the discussion at [1] for it.

[1] https://lore.kernel.org/kvm/20240516001530.GG168153@ls.amr.corp.intel.com/ 
-- 
Isaku Yamahata <isaku.yamahata@intel.com>