Re: [PATCH 4/4] KVM: SVM: Disable SEV-ES support if MMIO caching is disable

[Kai Huang <kai.huang@intel.com>]

On Thu, 2022-07-28 at 22:17 +0000, Sean Christopherson wrote:
> Disable SEV-ES if MMIO caching is disabled as SEV-ES relies on MMIO SPTEs
> generating #NPF(RSVD), which are reflected by the CPU into the guest as
> a #VC.  With SEV-ES, the untrusted host, a.k.a. KVM, doesn't have access
> to the guest instruction stream or register state and so can't directly
> emulate in response to a #NPF on an emulated MMIO GPA.  Disabling MMIO
> caching means guest accesses to emulated MMIO ranges cause #NPF(!PRESENT),
> and those flavors of #NPF cause automatic VM-Exits, not #VC.
> 
> Fixes: b09763da4dd8 ("KVM: x86/mmu: Add module param to disable MMIO caching (for testing)")
> Reported-by: Michael Roth <michael.roth@amd.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/kvm/mmu.h      |  2 ++
>  arch/x86/kvm/mmu/spte.c |  1 +
>  arch/x86/kvm/mmu/spte.h |  2 --
>  arch/x86/kvm/svm/sev.c  | 10 ++++++++++
>  4 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h
> index a99acec925eb..6bdaacb6faa0 100644
> --- a/arch/x86/kvm/mmu.h
> +++ b/arch/x86/kvm/mmu.h
> @@ -6,6 +6,8 @@
>  #include "kvm_cache_regs.h"
>  #include "cpuid.h"
>  
> +extern bool __read_mostly enable_mmio_caching;
> +
>  #define PT_WRITABLE_SHIFT 1
>  #define PT_USER_SHIFT 2
>  
> diff --git a/arch/x86/kvm/mmu/spte.c b/arch/x86/kvm/mmu/spte.c
> index 66f76f5a15bd..03ca740bf721 100644
> --- a/arch/x86/kvm/mmu/spte.c
> +++ b/arch/x86/kvm/mmu/spte.c
> @@ -22,6 +22,7 @@
>  bool __read_mostly enable_mmio_caching = true;
>  static bool __ro_after_init allow_mmio_caching;
>  module_param_named(mmio_caching, enable_mmio_caching, bool, 0444);
> +EXPORT_SYMBOL_GPL(enable_mmio_caching);
>  
>  u64 __read_mostly shadow_host_writable_mask;
>  u64 __read_mostly shadow_mmu_writable_mask;
> diff --git a/arch/x86/kvm/mmu/spte.h b/arch/x86/kvm/mmu/spte.h
> index 26b144ffd146..9a9414b8d1d6 100644
> --- a/arch/x86/kvm/mmu/spte.h
> +++ b/arch/x86/kvm/mmu/spte.h
> @@ -5,8 +5,6 @@
>  
>  #include "mmu_internal.h"
>  
> -extern bool __read_mostly enable_mmio_caching;
> -
>  /*
>   * A MMU present SPTE is backed by actual memory and may or may not be present
>   * in hardware.  E.g. MMIO SPTEs are not considered present.  Use bit 11, as it
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 309bcdb2f929..05bf6301acac 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -22,6 +22,7 @@
>  #include <asm/trapnr.h>
>  #include <asm/fpu/xcr.h>
>  
> +#include "mmu.h"
>  #include "x86.h"
>  #include "svm.h"
>  #include "svm_ops.h"
> @@ -2205,6 +2206,15 @@ void __init sev_hardware_setup(void)
>  	if (!sev_es_enabled)
>  		goto out;
>  
> +	/*
> +	 * SEV-ES requires MMIO caching as KVM doesn't have access to the guest
> +	 * instruction stream, i.e. can't emulate in response to a #NPF and
> +	 * instead relies on #NPF(RSVD) being reflected into the guest as #VC
> +	 * (the guest can then do a #VMGEXIT to request MMIO emulation).
> +	 */
> +	if (!enable_mmio_caching)
> +		goto out;
> +
> 

I am not familiar with SEV, but looks it is similar to TDX -- they both causes
#VE to guest instead of faulting into KVM.  And they both require explicit call
from guest to do MMIO.

In this case, does existing MMIO caching logic still apply to them?  Should we
still treat SEV and TDX's MMIO handling as MMIO caching being enabled?  Or
perhaps another variable?

-- 
Thanks,
-Kai