linux-kernel - Re: [PATCH v19 059/130] KVM: x86/tdp_mmu: Don't zap private pages for unsupported cases

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5917c0ee26cf2bb82a4ff14d35e46c219b40a13f.camel@intel.com>
Date: Mon, 25 Mar 2024 19:55:04 +0000
From: "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
To: "Yamahata, Isaku" <isaku.yamahata@...el.com>
CC: "Zhang, Tina" <tina.zhang@...el.com>, "seanjc@...gle.com"
	<seanjc@...gle.com>, "Huang, Kai" <kai.huang@...el.com>,
	"pbonzini@...hat.com" <pbonzini@...hat.com>, "Chen, Bo2" <chen.bo@...el.com>,
	"sagis@...gle.com" <sagis@...gle.com>, "isaku.yamahata@...il.com"
	<isaku.yamahata@...il.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "Aktas, Erdem" <erdemaktas@...gle.com>,
	"isaku.yamahata@...ux.intel.com" <isaku.yamahata@...ux.intel.com>,
	"sean.j.christopherson@...el.com" <sean.j.christopherson@...el.com>, "Yuan,
 Hang" <hang.yuan@...el.com>, "kvm@...r.kernel.org" <kvm@...r.kernel.org>
Subject: Re: [PATCH v19 059/130] KVM: x86/tdp_mmu: Don't zap private pages for
 unsupported cases

On Mon, 2024-03-25 at 12:05 -0700, Isaku Yamahata wrote:
> 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().
Ah, I see it:
https://lore.kernel.org/lkml/38e2f8a77e89301534d82325946eb74db3e47815.1708933498.git.isaku.yamahata@intel.com/

Then it seems a warning would be more appropriate if we are worried there might be a way to still
call it. If we are confident it can't, then we can just ignore this case.

> 
> 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.

My understanding is that Sean prefers to exit to userspace when KVM can't handle something, versus
making up behavior that keeps known guests alive. So I would think we should change this patch to
only be about not using the zapping roots optimization. Then a separate patch should exit to
userspace on attempt to use MTRRs. And we ignore the APIC one.

This is trying to guess what maintainers would want here. I'm less sure what Paolo prefers.