KVM: MMU: always invalidate and flush on spte page size change

Always invalidate spte and flush TLBs when changing page size, to make sure different sized translations for the same address are never cached in a CPU's TLB.

The first case where this occurs is when a non-leaf spte pointer is overwritten by a leaf, large spte entry. This can happen after dirty logging is disabled on a memslot, for example.

The second case is a leaf, large spte entry is overwritten with a non-leaf spte pointer, in __direct_map. Note this cannot happen now because the only potential source of such overwrite is dirty logging being enabled, which zaps all MMU pages. But this might change in the future, so better be robust against it.

Noticed by Andrea.

KVM-Stable-Tag

Signed-off-by Marcelo Tosatti<[email protected]>

Index: kvm/arch/x86/kvm/mmu.c
===================================================================

arch/x86/kvm/mmu.c +1 -0

@@ -2015,6 +2017,16 @@ static int __direct_map(struct kvm_vcpu
  			break;
  		}
 
+		if (is_shadow_present_pte(*iterator.sptep)&&

Quoted 24 lines · Marcelo Tosatti

Always invalidate spte and flush TLBs when changing page size, to make sure different sized translations for the same address are never cached in a CPU's TLB.

The first case where this occurs is when a non-leaf spte pointer is overwritten by a leaf, large spte entry. This can happen after dirty logging is disabled on a memslot, for example.

The second case is a leaf, large spte entry is overwritten with a non-leaf spte pointer, in __direct_map. Note this cannot happen now because the only potential source of such overwrite is dirty logging being enabled, which zaps all MMU pages. But this might change in the future, so better be robust against it.

Noticed by Andrea.

KVM-Stable-Tag

Signed-off-by Marcelo Tosatti<[email protected]>

Index: kvm/arch/x86/kvm/mmu.c
===================================================================

arch/x86/kvm/mmu.c static void mmu_set_spte(struct kvm_vcpu · line 1959

Quoted 8 lines

arch/x86/kvm/mmu.c +2 -0

@@ -1952,6 +1952,8 @@ static void mmu_set_spte(struct kvm_vcpu
 
 			child = page_header(pte&  PT64_BASE_ADDR_MASK);
 			mmu_page_remove_parent_pte(child, sptep);
+			__set_spte(sptep, shadow_trap_nonpresent_pte);
+			kvm_flush_remote_tlbs(vcpu->kvm);
 		} else if (pfn != spte_to_pfn(*sptep)) {
 			pgprintk("hfn old %lx new %lx\n",
 				 spte_to_pfn(*sptep), pfn);

Applied this bit.

Quoted 4 lines

arch/x86/kvm/mmu.c +0 -0

@@ -2015,6 +2017,16 @@ static int __direct_map(struct kvm_vcpu
 			break;
 		}
 

arch/x86/kvm/mmu.c static int __direct_map(struct kvm_vcpu · line 2029

Quoted 10 lines

arch/x86/kvm/mmu.c +10 -0

+		if (is_shadow_present_pte(*iterator.sptep)&&
+		    !is_large_pte(*iterator.sptep))
+			continue;
+
+		if (is_large_pte(*iterator.sptep)) {
+			rmap_remove(vcpu->kvm, iterator.sptep);
+			__set_spte(iterator.sptep, shadow_trap_nonpresent_pte);
+			kvm_flush_remote_tlbs(vcpu->kvm);
+		}
+

Don't we have exactly the same issue in FNAME(fetch)()?

Quoted 42 lines · Avi Kivity

Quoted 25 lines · Marcelo Tosatti

Always invalidate spte and flush TLBs when changing page size, to make sure different sized translations for the same address are never cached in a CPU's TLB.

The first case where this occurs is when a non-leaf spte pointer is overwritten by a leaf, large spte entry. This can happen after dirty logging is disabled on a memslot, for example.

The second case is a leaf, large spte entry is overwritten with a non-leaf spte pointer, in __direct_map. Note this cannot happen now because the only potential source of such overwrite is dirty logging being enabled, which zaps all MMU pages. But this might change in the future, so better be robust against it.

Noticed by Andrea.

KVM-Stable-Tag

Signed-off-by Marcelo Tosatti<[email protected]>

Index: kvm/arch/x86/kvm/mmu.c
===================================================================

arch/x86/kvm/mmu.c static void mmu_set_spte(struct kvm_vcpu · line 1959

Quoted 8 lines

arch/x86/kvm/mmu.c +2 -0

@@ -1952,6 +1952,8 @@ static void mmu_set_spte(struct kvm_vcpu
 
  			child = page_header(pte&   PT64_BASE_ADDR_MASK);
  			mmu_page_remove_parent_pte(child, sptep);
+			__set_spte(sptep, shadow_trap_nonpresent_pte);
+			kvm_flush_remote_tlbs(vcpu->kvm);
  		} else if (pfn != spte_to_pfn(*sptep)) {
  			pgprintk("hfn old %lx new %lx\n",
  				 spte_to_pfn(*sptep), pfn);

Applied this bit.

Quoted 4 lines

arch/x86/kvm/mmu.c +0 -0

@@ -2015,6 +2017,16 @@ static int __direct_map(struct kvm_vcpu
  			break;
  		}
 

Quoted 13 lines

arch/x86/kvm/mmu.c static int __direct_map(struct kvm_vcpu · line 2032

Quoted 11 lines

arch/x86/kvm/mmu.c +10 -0

+		if (is_shadow_present_pte(*iterator.sptep)&&
+		    !is_large_pte(*iterator.sptep))
+			continue;
+
+		if (is_large_pte(*iterator.sptep)) {
+			rmap_remove(vcpu->kvm, iterator.sptep);
+			__set_spte(iterator.sptep, shadow_trap_nonpresent_pte);
+			kvm_flush_remote_tlbs(vcpu->kvm);
+		}
+
 

Don't we have exactly the same issue in FNAME(fetch)()?

Yes and its already handled there:

                 if (is_shadow_present_pte(*sptep)&&  !is_large_pte(*sptep))
                         continue;

                 if (is_large_pte(*sptep)) {
                         rmap_remove(vcpu->kvm, sptep);
                         __set_spte(sptep, shadow_trap_nonpresent_pte);
                         kvm_flush_remote_tlbs(vcpu->kvm);
                 }