Received-SPF: pass (google.com: domain of linux-kernel+bounces-117768-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Mon, 25 Mar 2024 12:05:25 -0700
From: Isaku Yamahata <isaku.yamahata@intel.com>
To: "Edgecombe, Rick P" <rick.p.edgecombe@intel.com>
Cc: "Yamahata, Isaku" <isaku.yamahata@intel.com>,
	"Zhang, Tina" <tina.zhang@intel.com>,
	"isaku.yamahata@gmail.com" <isaku.yamahata@gmail.com>,
	"seanjc@google.com" <seanjc@google.com>,
	"Huang, Kai" <kai.huang@intel.com>, "Chen, Bo2" <chen.bo@intel.com>,
	"sagis@google.com" <sagis@google.com>,
	"isaku.yamahata@linux.intel.com" <isaku.yamahata@linux.intel.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Aktas, Erdem" <erdemaktas@google.com>,
	"sean.j.christopherson@intel.com" <sean.j.christopherson@intel.com>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"Yuan, Hang" <hang.yuan@intel.com>,
	"pbonzini@redhat.com" <pbonzini@redhat.com>
Subject: Re: [PATCH v19 059/130] KVM: x86/tdp_mmu: Don't zap private pages
 for unsupported cases
Message-ID: <20240325190525.GG2357401@ls.amr.corp.intel.com>
References: <cover.1708933498.git.isaku.yamahata@intel.com>
 <1ed955a44cd81738b498fe52823766622d8ad57f.1708933498.git.isaku.yamahata@intel.com>
 <618614fa6c62a232d95da55546137251e1847f48.camel@intel.com>
 <20240319235654.GC1994522@ls.amr.corp.intel.com>
 <1c2283aab681bd882111d14e8e71b4b35549e345.camel@intel.com>
 <f63d19a8fe6d14186aecc8fcf777284879441ef6.camel@intel.com>
 <20240321225910.GU1994522@ls.amr.corp.intel.com>
 <96fcb59cd53ece2c0d269f39c424d087876b3c73.camel@intel.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <96fcb59cd53ece2c0d269f39c424d087876b3c73.camel@intel.com>

On Fri, Mar 22, 2024 at 12:40:12AM +0000,
"Edgecombe, Rick P" <rick.p.edgecombe@intel.com> wrote:

> On Thu, 2024-03-21 at 15:59 -0700, Isaku Yamahata wrote:
> > > 
> > > Ok, I see now how this works. MTRRs and APIC zapping happen to use
> > > the
> > > same function: kvm_zap_gfn_range(). So restricting that function
> > > from
> > > zapping private pages has the desired affect. I think it's not
> > > ideal
> > > that kvm_zap_gfn_range() silently skips zapping some ranges. I
> > > wonder
> > > if we could pass something in, so it's more clear to the caller.
> > > 
> > > But can these code paths even get reaches in TDX? It sounded like
> > > MTRRs
> > > basically weren't supported.
> > 
> > We can make the code paths so with the (new) assumption that guest
> > MTRR can
> > be disabled cleanly.
> 
> So the situation is (please correct):
> KVM has a no "making up architectural behavior" rule, which is an
> important one. But TDX module doesn't support MTRRs. So TD guests can't
> have architectural behavior for MTRRs. So this patch is trying as best
> as possible to match what MTRR behavior it can (not crash the guest if
> someone tries).
>
> First of all, if the guest unmaps the private memory, doesn't it have
> to accept it again when gets re-added? So will the guest not crash
> anyway?

Right, the guest has to accept it on VE.  If the unmap was intentional by guest,
that's fine.  The unmap is unintentional (with vMTRR), the guest doesn't expect
VE with the GPA.


> But, I guess we should punt to userspace is the guest tries to use
> MTRRs, not that userspace can handle it happening in a TD...  But it
> seems cleaner and safer then skipping zapping some pages inside the
> zapping code.
> 
> I'm still not sure if I understand the intention and constraints fully.
> So please correct. This (the skipping the zapping for some operations)
> is a theoretical correctness issue right? It doesn't resolve a TD
> crash?

For lapic, it's safe guard. Because TDX KVM disables APICv with
APICV_INHIBIT_REASON_TDX, apicv won't call kvm_zap_gfn_range().

For MTRR, the purpose is to make the guest boot (without the guest kernel
command line like clearcpuid=mtrr) .
If we can assume the guest won't touch MTRR registers somehow, KVM can return an
error to TDG.VP.VMCALL<RDMSR, WRMSR>(MTRR registers). So it doesn't call
kvm_zap_gfn_range(). Or we can use KVM_EXIT_X86_{RDMSR, WRMSR} as you suggested.
-- 
isaku Yamahata <isaku.yamahata@intel.com>