linux-kernel - Re: [PATCH 10/21] KVM: TDX: Require TDP MMU and mmio caching for TDX

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: <78d7b648-ad06-4065-bc87-c195c790f699@intel.com>
Date: Thu, 12 Sep 2024 12:15:18 +1200
From: "Huang, Kai" <kai.huang@...el.com>
To: "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>, "seanjc@...gle.com"
	<seanjc@...gle.com>, "pbonzini@...hat.com" <pbonzini@...hat.com>,
	"kvm@...r.kernel.org" <kvm@...r.kernel.org>
CC: "dmatlack@...gle.com" <dmatlack@...gle.com>, "isaku.yamahata@...il.com"
	<isaku.yamahata@...il.com>, "Zhao, Yan Y" <yan.y.zhao@...el.com>,
	"nik.borisov@...e.com" <nik.borisov@...e.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 10/21] KVM: TDX: Require TDP MMU and mmio caching for TDX



On 4/09/2024 3:07 pm, Edgecombe, Rick P wrote:
> From: Isaku Yamahata <isaku.yamahata@...el.com>
> 
> Disable TDX support when TDP MMU or mmio caching aren't supported.
> 
> As TDP MMU is becoming main stream than the legacy MMU, the legacy MMU
> support for TDX isn't implemented.

Nitpickings:

I suppose we should use imperative mode since this is part of what this 
patch does?

Like:

TDX needs extensive MMU code change to make it work.  As TDP MMU is 
becoming main stream than the legacy MMU, for simplicity only support 
TDX for TDP MMU for now.

> 
> TDX requires KVM mmio caching. Without mmio caching, KVM will go to MMIO
> emulation without installing SPTEs for MMIOs. However, TDX guest is
> protected and KVM would meet errors when trying to emulate MMIOs for TDX
> guest during instruction decoding. So, TDX guest relies on SPTEs being
> installed for MMIOs, which are with no RWX bits and with VE suppress bit
> unset, to inject VE to TDX guest. The TDX guest would then issue TDVMCALL
> in the VE handler to perform instruction decoding and have host do MMIO
> emulation.

AFAICT the above two paragraphs are talking about two different things 
that one thing doens't have hard dependency to the other.

Should we separate this into two patches:  one patch to change 'checking 
enable_ept' to 'checking tdp_mmu_enabled' (which justifies the first 
paragraph), and the other to add MMIO caching checking.

The final code after the two patches could still end up with ...

[...]

> +	if (!tdp_mmu_enabled || !enable_mmio_caching)
> +		return -EOPNOTSUPP;
> +

... this though.

But feel free to ignore (since nitpickings).