diff mbox series

[04/10] KVM: x86/mmu: Add Suppress VE bit to EPT shadow_mmio_mask/shadow_present_mask

Message ID 20240412173532.3481264-5-pbonzini@redhat.com (mailing list archive)
State New
Headers show
Series KVM: MMU changes for confidential computing | expand

Commit Message

Paolo Bonzini April 12, 2024, 5:35 p.m. UTC 
From: Isaku Yamahata <isaku.yamahata@intel.com>

To make use of the same value of shadow_mmio_mask and shadow_present_mask
for TDX and VMX, add Suppress-VE bit to shadow_mmio_mask and
shadow_present_mask so that they can be common for both VMX and TDX.

TDX will require shadow_mmio_mask and shadow_present_mask to include
VMX_SUPPRESS_VE for shared GPA so that EPT violation is triggered for
shared GPA.  For VMX, VMX_SUPPRESS_VE doesn't matter for MMIO because the
spte value is defined so as to cause EPT misconfig.

Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
Message-Id: <97cc616b3563cd8277be91aaeb3e14bce23c3649.1705965635.git.isaku.yamahata@intel.com>
Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/include/asm/vmx.h | 1 +
 arch/x86/kvm/mmu/spte.c    | 6 ++++--
 2 files changed, 5 insertions(+), 2 deletions(-)

Comments

Chao Gao April 15, 2024, 1:07 p.m. UTC | #1 
>+++ b/arch/x86/include/asm/vmx.h
>@@ -514,6 +514,7 @@ enum vmcs_field {
> #define VMX_EPT_IPAT_BIT    			(1ull << 6)
> #define VMX_EPT_ACCESS_BIT			(1ull << 8)
> #define VMX_EPT_DIRTY_BIT			(1ull << 9)
>+#define VMX_EPT_SUPPRESS_VE_BIT			(1ull << 63)
> #define VMX_EPT_RWX_MASK                        (VMX_EPT_READABLE_MASK |       \
> 						 VMX_EPT_WRITABLE_MASK |       \
> 						 VMX_EPT_EXECUTABLE_MASK)
>diff --git a/arch/x86/kvm/mmu/spte.c b/arch/x86/kvm/mmu/spte.c
>index 6c7ab3aa6aa7..d97c4725c0b7 100644
>--- a/arch/x86/kvm/mmu/spte.c
>+++ b/arch/x86/kvm/mmu/spte.c
>@@ -413,7 +413,9 @@ void kvm_mmu_set_ept_masks(bool has_ad_bits, bool has_exec_only)
> 	shadow_dirty_mask	= has_ad_bits ? VMX_EPT_DIRTY_BIT : 0ull;
> 	shadow_nx_mask		= 0ull;
> 	shadow_x_mask		= VMX_EPT_EXECUTABLE_MASK;
>-	shadow_present_mask	= has_exec_only ? 0ull : VMX_EPT_READABLE_MASK;
>+	/* VMX_EPT_SUPPRESS_VE_BIT is needed for W or X violation. */
>+	shadow_present_mask	=
>+		(has_exec_only ? 0ull : VMX_EPT_READABLE_MASK) | VMX_EPT_SUPPRESS_VE_BIT;

This change makes !shadow_present_mask checks in FNAME(sync_spte) and
make_spte() pointless as shadow_present_mask will never be zero.

And the first sentence below in make_spte() also becomes stale. I suppose
this needs an update.

	/*
	 * For the EPT case, shadow_present_mask is 0 if hardware
	 * supports exec-only page table entries.  In that case,
	 * ACC_USER_MASK and shadow_user_mask are used to represent
	 * read access.  See FNAME(gpte_access) in paging_tmpl.h.
	 */


> 	/*
> 	 * EPT overrides the host MTRRs, and so KVM must program the desired
> 	 * memtype directly into the SPTEs.  Note, this mask is just the mask
>@@ -430,7 +432,7 @@ void kvm_mmu_set_ept_masks(bool has_ad_bits, bool has_exec_only)
> 	 * of an EPT paging-structure entry is 110b (write/execute).
> 	 */
> 	kvm_mmu_set_mmio_spte_mask(VMX_EPT_MISCONFIG_WX_VALUE,
>-				   VMX_EPT_RWX_MASK, 0);
>+				   VMX_EPT_RWX_MASK | VMX_EPT_SUPPRESS_VE_BIT, 0);
> }
> EXPORT_SYMBOL_GPL(kvm_mmu_set_ept_masks);
> 
>-- 
>2.43.0
>
>
>
Paolo Bonzini April 16, 2024, 5:41 p.m. UTC | #2 
On Mon, Apr 15, 2024 at 3:08 PM Chao Gao <chao.gao@intel.com> wrote:
>
> >+++ b/arch/x86/include/asm/vmx.h
> >@@ -514,6 +514,7 @@ enum vmcs_field {
> > #define VMX_EPT_IPAT_BIT                      (1ull << 6)
> > #define VMX_EPT_ACCESS_BIT                    (1ull << 8)
> > #define VMX_EPT_DIRTY_BIT                     (1ull << 9)
> >+#define VMX_EPT_SUPPRESS_VE_BIT                       (1ull << 63)
> > #define VMX_EPT_RWX_MASK                        (VMX_EPT_READABLE_MASK |       \
> >                                                VMX_EPT_WRITABLE_MASK |       \
> >                                                VMX_EPT_EXECUTABLE_MASK)
> >diff --git a/arch/x86/kvm/mmu/spte.c b/arch/x86/kvm/mmu/spte.c
> >index 6c7ab3aa6aa7..d97c4725c0b7 100644
> >--- a/arch/x86/kvm/mmu/spte.c
> >+++ b/arch/x86/kvm/mmu/spte.c
> >@@ -413,7 +413,9 @@ void kvm_mmu_set_ept_masks(bool has_ad_bits, bool has_exec_only)
> >       shadow_dirty_mask       = has_ad_bits ? VMX_EPT_DIRTY_BIT : 0ull;
> >       shadow_nx_mask          = 0ull;
> >       shadow_x_mask           = VMX_EPT_EXECUTABLE_MASK;
> >-      shadow_present_mask     = has_exec_only ? 0ull : VMX_EPT_READABLE_MASK;
> >+      /* VMX_EPT_SUPPRESS_VE_BIT is needed for W or X violation. */
> >+      shadow_present_mask     =
> >+              (has_exec_only ? 0ull : VMX_EPT_READABLE_MASK) | VMX_EPT_SUPPRESS_VE_BIT;
>
> This change makes !shadow_present_mask checks in FNAME(sync_spte) and
> make_spte() pointless as shadow_present_mask will never be zero.

It makes them wrong, not pointless. :)

The checks verify that there are "some" bits that are different
between non-present and present PTEs. They need to remove
SHADOW_NONPRESENT_MASK from shadow_present_mask.

Paolo
diff mbox series

Patch

diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
index 4dba17363008..ac6da0a5f5e6 100644
--- a/arch/x86/include/asm/vmx.h
+++ b/arch/x86/include/asm/vmx.h
@@ -514,6 +514,7 @@  enum vmcs_field {
 #define VMX_EPT_IPAT_BIT    			(1ull << 6)
 #define VMX_EPT_ACCESS_BIT			(1ull << 8)
 #define VMX_EPT_DIRTY_BIT			(1ull << 9)
+#define VMX_EPT_SUPPRESS_VE_BIT			(1ull << 63)
 #define VMX_EPT_RWX_MASK                        (VMX_EPT_READABLE_MASK |       \
 						 VMX_EPT_WRITABLE_MASK |       \
 						 VMX_EPT_EXECUTABLE_MASK)
diff --git a/arch/x86/kvm/mmu/spte.c b/arch/x86/kvm/mmu/spte.c
index 6c7ab3aa6aa7..d97c4725c0b7 100644
--- a/arch/x86/kvm/mmu/spte.c
+++ b/arch/x86/kvm/mmu/spte.c
@@ -413,7 +413,9 @@  void kvm_mmu_set_ept_masks(bool has_ad_bits, bool has_exec_only)
 	shadow_dirty_mask	= has_ad_bits ? VMX_EPT_DIRTY_BIT : 0ull;
 	shadow_nx_mask		= 0ull;
 	shadow_x_mask		= VMX_EPT_EXECUTABLE_MASK;
-	shadow_present_mask	= has_exec_only ? 0ull : VMX_EPT_READABLE_MASK;
+	/* VMX_EPT_SUPPRESS_VE_BIT is needed for W or X violation. */
+	shadow_present_mask	=
+		(has_exec_only ? 0ull : VMX_EPT_READABLE_MASK) | VMX_EPT_SUPPRESS_VE_BIT;
 	/*
 	 * EPT overrides the host MTRRs, and so KVM must program the desired
 	 * memtype directly into the SPTEs.  Note, this mask is just the mask
@@ -430,7 +432,7 @@  void kvm_mmu_set_ept_masks(bool has_ad_bits, bool has_exec_only)
 	 * of an EPT paging-structure entry is 110b (write/execute).
 	 */
 	kvm_mmu_set_mmio_spte_mask(VMX_EPT_MISCONFIG_WX_VALUE,
-				   VMX_EPT_RWX_MASK, 0);
+				   VMX_EPT_RWX_MASK | VMX_EPT_SUPPRESS_VE_BIT, 0);
 }
 EXPORT_SYMBOL_GPL(kvm_mmu_set_ept_masks);