kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alexandru Elisei <alexandru.elisei@arm.com>
To: kvm@vger.kernel.org
Cc: pbonzini@redhat.com, rkrcmar@redhat.com, drjones@redhat.com,
	maz@kernel.org, andre.przywara@arm.com, vladimir.murzin@arm.com,
	mark.rutland@arm.com
Subject: [kvm-unit-tests PATCH 04/18] lib: arm/arm64: Use WRITE_ONCE to update the translation tables
Date: Wed, 27 Nov 2019 14:23:56 +0000	[thread overview]
Message-ID: <20191127142410.1994-5-alexandru.elisei@arm.com> (raw)
In-Reply-To: <20191127142410.1994-1-alexandru.elisei@arm.com>

Use WRITE_ONCE to prevent store tearing when updating an entry in the
translation tables. Without WRITE_ONCE, the compiler, even though it is
unlikely, can emit several stores when changing the table, and we might
end up with bogus TLB entries.

It's worth noting that the existing code is mostly fine without any
changes because the translation tables are updated in one of the
following situations:

- When the tables are being created with the MMU off, which means no TLB
  caching is being performed.

- When new page table entries are added as a result of vmalloc'ing a
  stack for a secondary CPU, which doesn't happen very often.

- When clearing the PTE_USER bit for the cache test, and store tearing
  has no effect on the table walker because there are no intermediate
  values between bit values 0 and 1. We still use WRITE_ONCE in this case
  for consistency.

However, the functions are global and there is nothing preventing someone
from writing a test that uses them in a different scenario. Let's make
sure that when that happens, there will be no breakage once in a blue
moon.

Reported-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 lib/arm/asm/pgtable.h   | 12 ++++++++----
 lib/arm64/asm/pgtable.h |  7 +++++--
 lib/arm/mmu.c           | 19 +++++++++++++------
 3 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/lib/arm/asm/pgtable.h b/lib/arm/asm/pgtable.h
index 241dff69b38a..794514b8c927 100644
--- a/lib/arm/asm/pgtable.h
+++ b/lib/arm/asm/pgtable.h
@@ -19,6 +19,8 @@
  * because we always allocate their pages with alloc_page(), and
  * alloc_page() always returns identity mapped pages.
  */
+#include <linux/compiler.h>
+
 #define pgtable_va(x)		((void *)(unsigned long)(x))
 #define pgtable_pa(x)		((unsigned long)(x))
 
@@ -58,8 +60,9 @@ static inline pmd_t *pmd_alloc_one(void)
 static inline pmd_t *pmd_alloc(pgd_t *pgd, unsigned long addr)
 {
 	if (pgd_none(*pgd)) {
-		pmd_t *pmd = pmd_alloc_one();
-		pgd_val(*pgd) = pgtable_pa(pmd) | PMD_TYPE_TABLE;
+		pgd_t entry;
+		pgd_val(entry) = pgtable_pa(pmd_alloc_one()) | PMD_TYPE_TABLE;
+		WRITE_ONCE(*pgd, entry);
 	}
 	return pmd_offset(pgd, addr);
 }
@@ -84,8 +87,9 @@ static inline pte_t *pte_alloc_one(void)
 static inline pte_t *pte_alloc(pmd_t *pmd, unsigned long addr)
 {
 	if (pmd_none(*pmd)) {
-		pte_t *pte = pte_alloc_one();
-		pmd_val(*pmd) = pgtable_pa(pte) | PMD_TYPE_TABLE;
+		pmd_t entry;
+		pmd_val(entry) = pgtable_pa(pte_alloc_one()) | PMD_TYPE_TABLE;
+		WRITE_ONCE(*pmd, entry);
 	}
 	return pte_offset(pmd, addr);
 }
diff --git a/lib/arm64/asm/pgtable.h b/lib/arm64/asm/pgtable.h
index ee0a2c88cc18..dbf9e7253b71 100644
--- a/lib/arm64/asm/pgtable.h
+++ b/lib/arm64/asm/pgtable.h
@@ -18,6 +18,8 @@
 #include <asm/page.h>
 #include <asm/pgtable-hwdef.h>
 
+#include <linux/compiler.h>
+
 /*
  * We can convert va <=> pa page table addresses with simple casts
  * because we always allocate their pages with alloc_page(), and
@@ -66,8 +68,9 @@ static inline pte_t *pte_alloc_one(void)
 static inline pte_t *pte_alloc(pmd_t *pmd, unsigned long addr)
 {
 	if (pmd_none(*pmd)) {
-		pte_t *pte = pte_alloc_one();
-		pmd_val(*pmd) = pgtable_pa(pte) | PMD_TYPE_TABLE;
+		pmd_t entry;
+		pmd_val(entry) = pgtable_pa(pte_alloc_one()) | PMD_TYPE_TABLE;
+		WRITE_ONCE(*pmd, entry);
 	}
 	return pte_offset(pmd, addr);
 }
diff --git a/lib/arm/mmu.c b/lib/arm/mmu.c
index 72043c333b55..cc03b25aa77e 100644
--- a/lib/arm/mmu.c
+++ b/lib/arm/mmu.c
@@ -17,6 +17,8 @@
 #include <asm/pgtable-hwdef.h>
 #include <asm/pgtable.h>
 
+#include <linux/compiler.h>
+
 extern unsigned long etext;
 
 pgd_t *mmu_idmap;
@@ -86,7 +88,7 @@ static pteval_t *install_pte(pgd_t *pgtable, uintptr_t vaddr, pteval_t pte)
 {
 	pteval_t *p_pte = get_pte(pgtable, vaddr);
 
-	*p_pte = pte;
+	WRITE_ONCE(*p_pte, pte);
 	dsb(ishst);
 	flush_tlb_page(vaddr);
 
@@ -133,12 +135,15 @@ void mmu_set_range_sect(pgd_t *pgtable, uintptr_t virt_offset,
 	phys_addr_t paddr = phys_start & PGDIR_MASK;
 	uintptr_t vaddr = virt_offset & PGDIR_MASK;
 	uintptr_t virt_end = phys_end - paddr + vaddr;
+	pgd_t *pgd;
+	pgd_t entry;
 
 	for (; vaddr < virt_end; vaddr += PGDIR_SIZE, paddr += PGDIR_SIZE) {
-		pgd_t *pgd = pgd_offset(pgtable, vaddr);
-		pgd_val(*pgd) = paddr;
-		pgd_val(*pgd) |= PMD_TYPE_SECT | PMD_SECT_AF | PMD_SECT_S;
-		pgd_val(*pgd) |= pgprot_val(prot);
+		pgd_val(entry) = paddr;
+		pgd_val(entry) |= PMD_TYPE_SECT | PMD_SECT_AF | PMD_SECT_S;
+		pgd_val(entry) |= pgprot_val(prot);
+		pgd = pgd_offset(pgtable, vaddr);
+		WRITE_ONCE(*pgd, entry);
 		dsb(ishst);
 		flush_tlb_page(vaddr);
 	}
@@ -213,6 +218,7 @@ void mmu_clear_user(unsigned long vaddr)
 {
 	pgd_t *pgtable;
 	pteval_t *pte;
+	pteval_t entry;
 
 	if (!mmu_enabled())
 		return;
@@ -220,7 +226,8 @@ void mmu_clear_user(unsigned long vaddr)
 	pgtable = current_thread_info()->pgtable;
 	pte = get_pte(pgtable, vaddr);
 
-	*pte &= ~PTE_USER;
+	entry = *pte & ~PTE_USER;
+	WRITE_ONCE(*pte, entry);
 	dsb(ishst);
 	flush_tlb_page(vaddr);
 }
-- 
2.20.1


  parent reply	other threads:[~2019-11-27 14:25 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-27 14:23 [kvm-unit-tests PATCH 00/18] Various fixes Alexandru Elisei
2019-11-27 14:23 ` [kvm-unit-tests PATCH 01/18] lib: arm/arm64: Remove unnecessary dcache maintenance operations Alexandru Elisei
2019-11-27 14:23 ` [kvm-unit-tests PATCH 02/18] lib: arm64: Remove barriers before TLB operations Alexandru Elisei
2019-11-27 14:23 ` [kvm-unit-tests PATCH 03/18] lib: Add WRITE_ONCE and READ_ONCE implementations in compiler.h Alexandru Elisei
2019-11-27 14:23 ` Alexandru Elisei [this message]
2019-11-27 14:23 ` [kvm-unit-tests PATCH 05/18] lib: arm/arm64: Remove unused CPU_OFF parameter Alexandru Elisei
2019-11-27 14:23 ` [kvm-unit-tests PATCH 06/18] arm/arm64: psci: Don't run C code without stack or vectors Alexandru Elisei
2019-11-27 14:23 ` [kvm-unit-tests PATCH 07/18] lib: arm/arm64: Add missing include for alloc_page.h in pgtable.h Alexandru Elisei
2019-11-27 14:24 ` [kvm-unit-tests PATCH 08/18] lib: arm: Implement flush_tlb_all Alexandru Elisei
2019-11-27 14:24 ` [kvm-unit-tests PATCH 09/18] lib: arm/arm64: Teach mmu_clear_user about block mappings Alexandru Elisei
2019-11-27 14:24 ` [kvm-unit-tests PATCH 10/18] arm/arm64: selftest: Add prefetch abort test Alexandru Elisei
2019-11-27 18:47   ` Andrew Jones
2019-11-28  9:59     ` Alexandru Elisei
2019-11-28 16:56       ` Alexandru Elisei
2019-11-27 14:24 ` [kvm-unit-tests PATCH 11/18] arm64: timer: Write to ICENABLER to disable timer IRQ Alexandru Elisei
2019-11-27 14:24 ` [kvm-unit-tests PATCH 12/18] arm64: timer: EOIR the interrupt after masking the timer Alexandru Elisei
2019-11-27 14:24 ` [kvm-unit-tests PATCH 13/18] arm64: timer: Test behavior when timer disabled or masked Alexandru Elisei
2019-11-27 14:24 ` [kvm-unit-tests PATCH 14/18] lib: arm/arm64: Refuse to disable the MMU with non-identity stack pointer Alexandru Elisei
2019-11-27 14:24 ` [kvm-unit-tests PATCH 15/18] arm/arm64: Perform dcache clean + invalidate after turning MMU off Alexandru Elisei
2019-11-27 14:24 ` [kvm-unit-tests PATCH 16/18] arm: cstart64.S: Downgrade TLBI to non-shareable in asm_mmu_enable Alexandru Elisei
2019-11-27 14:24 ` [kvm-unit-tests PATCH 17/18] arm/arm64: Invalidate TLB before enabling MMU Alexandru Elisei
2019-11-27 14:24 ` [kvm-unit-tests PATCH 18/18] arm: cstart64.S: Remove icache invalidation from asm_mmu_enable Alexandru Elisei
2019-11-27 14:41   ` Alexandru Elisei

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191127142410.1994-5-alexandru.elisei@arm.com \
    --to=alexandru.elisei@arm.com \
    --cc=andre.przywara@arm.com \
    --cc=drjones@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=maz@kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=rkrcmar@redhat.com \
    --cc=vladimir.murzin@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).