From: Peter Collingbourne <pcc@google.com>
To: linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu
Cc: Catalin Marinas <catalin.marinas@arm.com>,
Cornelia Huck <cohuck@redhat.com>, Will Deacon <will@kernel.org>,
Marc Zyngier <maz@kernel.org>,
Evgenii Stepanov <eugenis@google.com>,
kvm@vger.kernel.org, Steven Price <steven.price@arm.com>,
Vincenzo Frascino <vincenzo.frascino@arm.com>,
Peter Collingbourne <pcc@google.com>
Subject: [PATCH v3 4/7] arm64: mte: Lock a page for MTE tag initialisation
Date: Wed, 10 Aug 2022 12:30:30 -0700 [thread overview]
Message-ID: <20220810193033.1090251-5-pcc@google.com> (raw)
In-Reply-To: <20220810193033.1090251-1-pcc@google.com>
From: Catalin Marinas <catalin.marinas@arm.com>
Initialising the tags and setting PG_mte_tagged flag for a page can race
between multiple set_pte_at() on shared pages or setting the stage 2 pte
via user_mem_abort(). Introduce a new PG_mte_lock flag as PG_arch_3 and
set it before attempting page initialisation. Given that PG_mte_tagged
is never cleared for a page, consider setting this flag to mean page
unlocked and wait on this bit with acquire semantics if the page is
locked:
- try_page_mte_tagging() - lock the page for tagging, return true if it
can be tagged, false if already tagged. No acquire semantics if it
returns true (PG_mte_tagged not set) as there is no serialisation with
a previous set_page_mte_tagged().
- set_page_mte_tagged() - set PG_mte_tagged with release semantics.
The two-bit locking is based on Peter Collingbourne's idea.
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Steven Price <steven.price@arm.com>
Cc: Peter Collingbourne <pcc@google.com>
---
arch/arm64/include/asm/mte.h | 32 ++++++++++++++++++++++++++++++++
arch/arm64/include/asm/pgtable.h | 1 +
arch/arm64/kernel/cpufeature.c | 2 +-
arch/arm64/kernel/mte.c | 7 +++++--
arch/arm64/kvm/guest.c | 16 ++++++++++------
arch/arm64/kvm/mmu.c | 2 +-
arch/arm64/mm/copypage.c | 2 ++
arch/arm64/mm/fault.c | 2 ++
arch/arm64/mm/mteswap.c | 3 +++
9 files changed, 57 insertions(+), 10 deletions(-)
diff --git a/arch/arm64/include/asm/mte.h b/arch/arm64/include/asm/mte.h
index 46618c575eac..ea5158f6f6cb 100644
--- a/arch/arm64/include/asm/mte.h
+++ b/arch/arm64/include/asm/mte.h
@@ -36,6 +36,8 @@ void mte_free_tag_storage(char *storage);
/* track which pages have valid allocation tags */
#define PG_mte_tagged PG_arch_2
+/* simple lock to avoid multiple threads tagging the same page */
+#define PG_mte_lock PG_arch_3
static inline void set_page_mte_tagged(struct page *page)
{
@@ -60,6 +62,32 @@ static inline bool page_mte_tagged(struct page *page)
return ret;
}
+/*
+ * Lock the page for tagging and return 'true' if the page can be tagged,
+ * 'false' if already tagged. PG_mte_tagged is never cleared and therefore the
+ * locking only happens once for page initialisation.
+ *
+ * The page MTE lock state:
+ *
+ * Locked: PG_mte_lock && !PG_mte_tagged
+ * Unlocked: !PG_mte_lock || PG_mte_tagged
+ *
+ * Acquire semantics only if the page is tagged (returning 'false').
+ */
+static inline bool try_page_mte_tagging(struct page *page)
+{
+ if (!test_and_set_bit(PG_mte_lock, &page->flags))
+ return true;
+
+ /*
+ * The tags are either being initialised or have already been initialised,
+ * wait for the PG_mte_tagged flag to be set.
+ */
+ smp_cond_load_acquire(&page->flags, VAL & (1UL << PG_mte_tagged));
+
+ return false;
+}
+
void mte_zero_clear_page_tags(void *addr);
void mte_sync_tags(pte_t old_pte, pte_t pte);
void mte_copy_page_tags(void *kto, const void *kfrom);
@@ -84,6 +112,10 @@ static inline bool page_mte_tagged(struct page *page)
{
return false;
}
+static inline bool try_page_mte_tagging(struct page *page)
+{
+ return false;
+}
static inline void mte_zero_clear_page_tags(void *addr)
{
}
diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
index 82719fa42c0e..e6b82ad1e9e6 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -1049,6 +1049,7 @@ static inline void arch_swap_invalidate_area(int type)
#define __HAVE_ARCH_SWAP_RESTORE
static inline void arch_swap_restore(swp_entry_t entry, struct folio *folio)
{
+ /* mte_restore_tags() takes the PG_mte_lock */
if (system_supports_mte() && mte_restore_tags(entry, &folio->page))
set_page_mte_tagged(&folio->page);
}
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 562c301bbf15..33d342ddef87 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -2037,7 +2037,7 @@ static void cpu_enable_mte(struct arm64_cpu_capabilities const *cap)
* Clear the tags in the zero page. This needs to be done via the
* linear map which has the Tagged attribute.
*/
- if (!page_mte_tagged(ZERO_PAGE(0))) {
+ if (try_page_mte_tagging(ZERO_PAGE(0))) {
mte_clear_page_tags(lm_alias(empty_zero_page));
set_page_mte_tagged(ZERO_PAGE(0));
}
diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
index 2287316639f3..634e089b5933 100644
--- a/arch/arm64/kernel/mte.c
+++ b/arch/arm64/kernel/mte.c
@@ -41,6 +41,7 @@ static void mte_sync_page_tags(struct page *page, pte_t old_pte,
if (check_swap && is_swap_pte(old_pte)) {
swp_entry_t entry = pte_to_swp_entry(old_pte);
+ /* mte_restore_tags() takes the PG_mte_lock */
if (!non_swap_entry(entry) && mte_restore_tags(entry, page)) {
set_page_mte_tagged(page);
return;
@@ -50,8 +51,10 @@ static void mte_sync_page_tags(struct page *page, pte_t old_pte,
if (!pte_is_tagged)
return;
- mte_clear_page_tags(page_address(page));
- set_page_mte_tagged(page);
+ if (try_page_mte_tagging(page)) {
+ mte_clear_page_tags(page_address(page));
+ set_page_mte_tagged(page);
+ }
}
void mte_sync_tags(pte_t old_pte, pte_t pte)
diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
index 3b04e69006b4..059b38e7a9e8 100644
--- a/arch/arm64/kvm/guest.c
+++ b/arch/arm64/kvm/guest.c
@@ -1067,15 +1067,19 @@ long kvm_vm_ioctl_mte_copy_tags(struct kvm *kvm,
clear_user(tags, MTE_GRANULES_PER_PAGE);
kvm_release_pfn_clean(pfn);
} else {
+ /*
+ * Only locking to serialise with a concurrent
+ * set_pte_at() in the VMM but still overriding the
+ * tags, hence ignoring the return value.
+ */
+ try_page_mte_tagging(page);
num_tags = mte_copy_tags_from_user(maddr, tags,
MTE_GRANULES_PER_PAGE);
- /*
- * Set the flag after checking the write
- * completed fully
- */
- if (num_tags == MTE_GRANULES_PER_PAGE)
- set_page_mte_tagged(page);
+ /* uaccess failed, don't leave stale tags */
+ if (num_tags != MTE_GRANULES_PER_PAGE)
+ mte_clear_page_tags(page);
+ set_page_mte_tagged(page);
kvm_release_pfn_dirty(pfn);
}
diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index 1a3707aeb41f..750a69a97994 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -1066,7 +1066,7 @@ static void sanitise_mte_tags(struct kvm *kvm, kvm_pfn_t pfn,
return;
for (i = 0; i < nr_pages; i++, page++) {
- if (!page_mte_tagged(page)) {
+ if (try_page_mte_tagging(page)) {
mte_clear_page_tags(page_address(page));
set_page_mte_tagged(page);
}
diff --git a/arch/arm64/mm/copypage.c b/arch/arm64/mm/copypage.c
index 4223389b6180..a3fa650ceca4 100644
--- a/arch/arm64/mm/copypage.c
+++ b/arch/arm64/mm/copypage.c
@@ -22,6 +22,8 @@ void copy_highpage(struct page *to, struct page *from)
copy_page(kto, kfrom);
if (system_supports_mte() && page_mte_tagged(from)) {
+ /* It's a new page, shouldn't have been tagged yet */
+ WARN_ON_ONCE(!try_page_mte_tagging(to));
mte_copy_page_tags(kto, kfrom);
set_page_mte_tagged(to);
}
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index d095bfa16771..6407a29cab0d 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -930,6 +930,8 @@ struct page *alloc_zeroed_user_highpage_movable(struct vm_area_struct *vma,
void tag_clear_highpage(struct page *page)
{
+ /* Newly allocated page, shouldn't have been tagged yet */
+ WARN_ON_ONCE(!try_page_mte_tagging(page));
mte_zero_clear_page_tags(page_address(page));
set_page_mte_tagged(page);
}
diff --git a/arch/arm64/mm/mteswap.c b/arch/arm64/mm/mteswap.c
index a78c1db23c68..cd5ad0936e16 100644
--- a/arch/arm64/mm/mteswap.c
+++ b/arch/arm64/mm/mteswap.c
@@ -53,6 +53,9 @@ bool mte_restore_tags(swp_entry_t entry, struct page *page)
if (!tags)
return false;
+ /* racing tag restoring? */
+ if (!try_page_mte_tagging(page))
+ return false;
mte_restore_page_tags(page_address(page), tags);
return true;
--
2.37.1.559.g78731f0fdb-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2022-08-10 19:32 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-10 19:30 [PATCH v3 0/7] KVM: arm64: permit MAP_SHARED mappings with MTE enabled Peter Collingbourne
2022-08-10 19:30 ` [PATCH v3 1/7] arm64: mte: Fix/clarify the PG_mte_tagged semantics Peter Collingbourne
2022-09-01 15:49 ` Catalin Marinas
2022-09-02 10:26 ` Cornelia Huck
2022-09-02 14:47 ` Steven Price
2022-08-10 19:30 ` [PATCH v3 2/7] KVM: arm64: Simplify the sanitise_mte_tags() logic Peter Collingbourne
2022-09-02 14:47 ` Steven Price
2022-08-10 19:30 ` [PATCH v3 3/7] mm: Add PG_arch_3 page flag Peter Collingbourne
2022-08-11 7:16 ` kernel test robot
2022-09-01 17:59 ` Catalin Marinas
2022-09-05 17:01 ` Catalin Marinas
2022-09-19 18:12 ` Marc Zyngier
2022-09-20 15:39 ` Catalin Marinas
2022-09-20 16:33 ` Marc Zyngier
2022-09-20 16:58 ` Catalin Marinas
2022-09-21 3:53 ` Peter Collingbourne
2022-08-10 19:30 ` Peter Collingbourne [this message]
2022-09-02 14:47 ` [PATCH v3 4/7] arm64: mte: Lock a page for MTE tag initialisation Steven Price
2022-09-02 16:28 ` Catalin Marinas
2022-09-02 16:58 ` Catalin Marinas
2022-09-05 7:37 ` Steven Price
2022-08-10 19:30 ` [PATCH v3 5/7] KVM: arm64: unify the tests for VMAs in memslots when MTE is enabled Peter Collingbourne
2022-09-02 13:41 ` Catalin Marinas
2022-09-02 14:47 ` Steven Price
2022-08-10 19:30 ` [PATCH v3 6/7] KVM: arm64: permit all VM_MTE_ALLOWED mappings with MTE enabled Peter Collingbourne
2022-09-02 13:45 ` Catalin Marinas
2022-09-02 14:47 ` Steven Price
2022-09-12 16:23 ` Marc Zyngier
2022-09-13 4:10 ` Peter Collingbourne
2022-08-10 19:30 ` [PATCH v3 7/7] Documentation: document the ABI changes for KVM_CAP_ARM_MTE Peter Collingbourne
2022-09-02 13:49 ` Catalin Marinas
2022-09-02 14:05 ` [PATCH v3 0/7] KVM: arm64: permit MAP_SHARED mappings with MTE enabled Catalin Marinas
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=20220810193033.1090251-5-pcc@google.com \
--to=pcc@google.com \
--cc=catalin.marinas@arm.com \
--cc=cohuck@redhat.com \
--cc=eugenis@google.com \
--cc=kvm@vger.kernel.org \
--cc=kvmarm@lists.cs.columbia.edu \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=maz@kernel.org \
--cc=steven.price@arm.com \
--cc=vincenzo.frascino@arm.com \
--cc=will@kernel.org \
/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).