[PATCH] KVM: x86: avoid write-tearing of TDP

[PATCH] KVM: x86: avoid write-tearing of TDP

[Nadav Amit]

In theory, nothing prevents the compiler from write-tearing PTEs, or
split PTE writes. These partially-modified PTEs can be fetched by other
cores and cause mayhem. I have not really encountered such case in
real-life, but it does seem possible.

For example, the compiler may try to do something creative for
kvm_set_pte_rmapp() and perform multiple writes to the PTE.

Signed-off-by: Nadav Amit <nadav.amit@gmail.com>
---
 arch/x86/kvm/mmu.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 1ff4dbb..4fca1cb 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -336,12 +336,12 @@ static gfn_t pse36_gfn_delta(u32 gpte)
 #ifdef CONFIG_X86_64
 static void __set_spte(u64 *sptep, u64 spte)
 {
-	*sptep = spte;
+	WRITE_ONCE(*sptep, spte);
 }
 
 static void __update_clear_spte_fast(u64 *sptep, u64 spte)
 {
-	*sptep = spte;
+	WRITE_ONCE(*sptep, spte);
 }
 
 static u64 __update_clear_spte_slow(u64 *sptep, u64 spte)
@@ -390,7 +390,7 @@ static void __set_spte(u64 *sptep, u64 spte)
 	 */
 	smp_wmb();
 
-	ssptep->spte_low = sspte.spte_low;
+	WRITE_ONCE(ssptep->spte_low, sspte.spte_low);
 }
 
 static void __update_clear_spte_fast(u64 *sptep, u64 spte)
@@ -400,7 +400,7 @@ static void __update_clear_spte_fast(u64 *sptep, u64 spte)
 	ssptep = (union split_spte *)sptep;
 	sspte = (union split_spte)spte;
 
-	ssptep->spte_low = sspte.spte_low;
+	WRITE_ONCE(ssptep->spte_low, sspte.spte_low);
 
 	/*
 	 * If we map the spte from present to nonpresent, we should clear
-- 
2.7.4

Re: [PATCH] KVM: x86: avoid write-tearing of TDP

[Nadav Amit]

I mistakenly sent this patch with wrong date, which is hopefully the 
reason you ignored it.

So this email is in order to bump it up.

Thanks,
Nadav 

Nadav Amit <nadav.amit@gmail.com> wrote:

> In theory, nothing prevents the compiler from write-tearing PTEs, or
> split PTE writes. These partially-modified PTEs can be fetched by other
> cores and cause mayhem. I have not really encountered such case in
> real-life, but it does seem possible.
> 
> For example, the compiler may try to do something creative for
> kvm_set_pte_rmapp() and perform multiple writes to the PTE.
> 
> Signed-off-by: Nadav Amit <nadav.amit@gmail.com>
> ---
> arch/x86/kvm/mmu.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index 1ff4dbb..4fca1cb 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -336,12 +336,12 @@ static gfn_t pse36_gfn_delta(u32 gpte)
> #ifdef CONFIG_X86_64
> static void __set_spte(u64 *sptep, u64 spte)
> {
> -	*sptep = spte;
> +	WRITE_ONCE(*sptep, spte);
> }
> 
> static void __update_clear_spte_fast(u64 *sptep, u64 spte)
> {
> -	*sptep = spte;
> +	WRITE_ONCE(*sptep, spte);
> }
> 
> static u64 __update_clear_spte_slow(u64 *sptep, u64 spte)
> @@ -390,7 +390,7 @@ static void __set_spte(u64 *sptep, u64 spte)
> 	 */
> 	smp_wmb();
> 
> -	ssptep->spte_low = sspte.spte_low;
> +	WRITE_ONCE(ssptep->spte_low, sspte.spte_low);
> }
> 
> static void __update_clear_spte_fast(u64 *sptep, u64 spte)
> @@ -400,7 +400,7 @@ static void __update_clear_spte_fast(u64 *sptep, u64 spte)
> 	ssptep = (union split_spte *)sptep;
> 	sspte = (union split_spte)spte;
> 
> -	ssptep->spte_low = sspte.spte_low;
> +	WRITE_ONCE(ssptep->spte_low, sspte.spte_low);
> 
> 	/*
> 	 * If we map the spte from present to nonpresent, we should clear
> -- 
> 2.7.4

Re: [PATCH] KVM: x86: avoid write-tearing of TDP

[Radim Krčmář]

2016-05-11 08:04-0700, Nadav Amit:
> In theory, nothing prevents the compiler from write-tearing PTEs, or
> split PTE writes. These partially-modified PTEs can be fetched by other
> cores and cause mayhem. I have not really encountered such case in
> real-life, but it does seem possible.
> 
> For example, the compiler may try to do something creative for
> kvm_set_pte_rmapp() and perform multiple writes to the PTE.
> 
> Signed-off-by: Nadav Amit <nadav.amit@gmail.com>
> ---

We don't lose anything by preventing tearing even if it wasn't possible.
Applied,

Thanks.