All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alistair Popple <apopple@nvidia.com>
To: Andrew Morton <akpm@linux-foundation.org>,
	will@kernel.org, catalin.marinas@arm.com
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	robin.murphy@arm.com, nicolinc@nvidia.com,
	linux-arm-kernel@lists.infradead.org, kvm@vger.kernel.org,
	jgg@nvidia.com, John Hubbard <jhubbard@nvidia.com>,
	zhi.wang.linux@gmail.com, Sean Christopherson <seanjc@google.com>,
	Alistair Popple <apopple@nvidia.com>
Subject: [PATCH 2/2] arm64: Notify on pte permission upgrades
Date: Wed, 24 May 2023 11:47:29 +1000	[thread overview]
Message-ID: <5d8e1f752051173d2d1b5c3e14b54eb3506ed3ef.1684892404.git-series.apopple@nvidia.com> (raw)
In-Reply-To: <3cece716fc09724793aa832e755abfc9d70a8bb3.1684892404.git-series.apopple@nvidia.com>

ARM64 requires TLB invalidates when upgrading pte permission from
read-only to read-write. However mmu_notifiers assume upgrades do not
need notifications and none are sent. This causes problems when a
secondary TLB such as implemented by an ARM SMMU doesn't support
broadcast TLB maintenance (BTM) and caches a read-only PTE.

As no notification is sent and the SMMU does not snoop TLB invalidates
it will continue to return read-only entries to a device even though
the CPU page table contains a writable entry. This leads to a
continually faulting device and no way of handling the fault.

The ARM SMMU driver already registers for mmu notifier events to keep
any secondary TLB synchronised. Therefore sending a notifier on
permission upgrade fixes the problem.

Rather than adding notifier calls to generic architecture independent
code where it may cause performance regressions on architectures that
don't require it add it to the architecture specific
ptep_set_access_flags() where the CPU TLB is invalidated.

Signed-off-by: Alistair Popple <apopple@nvidia.com>

---

A version of this fix was previously posted here:

https://lore.kernel.org/linux-mm/ZGxg+I8FWz3YqBMk@infradead.org/T/

That fix updated generic architecture independent code by adding a new
mmu notifier range event that would allow filtering by
architectures/drivers that didn't require flushing for upgrades.

This was done because calling notifiers from architecture specific
code requires calling the notifier while holding the ptl. It wasn't
immediately obvious that that was safe, but review comments and git
history suggests it must be hence the updated approach here.
---
 arch/arm64/mm/fault.c       |  6 +++++-
 arch/arm64/mm/hugetlbpage.c |  9 +++++++++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index cb21ccd..1ee45a8 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -25,6 +25,7 @@
 #include <linux/perf_event.h>
 #include <linux/preempt.h>
 #include <linux/hugetlb.h>
+#include <linux/mmu_notifier.h>
 
 #include <asm/acpi.h>
 #include <asm/bug.h>
@@ -225,8 +226,11 @@ int ptep_set_access_flags(struct vm_area_struct *vma,
 	} while (pteval != old_pteval);
 
 	/* Invalidate a stale read-only entry */
-	if (dirty)
+	if (dirty) {
 		flush_tlb_page(vma, address);
+		mmu_notifier_invalidate_range(vma->vm_mm, address & PAGE_MASK,
+					(address & PAGE_MASK) + PAGE_SIZE);
+	}
 	return 1;
 }
 
diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
index 95364e8..677f0d1 100644
--- a/arch/arm64/mm/hugetlbpage.c
+++ b/arch/arm64/mm/hugetlbpage.c
@@ -14,6 +14,7 @@
 #include <linux/pagemap.h>
 #include <linux/err.h>
 #include <linux/sysctl.h>
+#include <linux/mmu_notifier.h>
 #include <asm/mman.h>
 #include <asm/tlb.h>
 #include <asm/tlbflush.h>
@@ -487,6 +488,14 @@ int huge_ptep_set_access_flags(struct vm_area_struct *vma,
 
 	orig_pte = get_clear_contig_flush(mm, addr, ptep, pgsize, ncontig);
 
+	/*
+	 * Make sure any cached read-only entries are removed from
+	 * secondary TLBs.
+	 */
+	if (dirty)
+		mmu_notifier_invalidate_range(mm, addr,
+					addr + (pgsize + ncontig));
+
 	/* Make sure we don't lose the dirty or young state */
 	if (pte_dirty(orig_pte))
 		pte = pte_mkdirty(pte);
-- 
git-series 0.9.1

WARNING: multiple messages have this Message-ID (diff)
From: Alistair Popple <apopple@nvidia.com>
To: Andrew Morton <akpm@linux-foundation.org>,
	will@kernel.org, catalin.marinas@arm.com
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	robin.murphy@arm.com, nicolinc@nvidia.com,
	linux-arm-kernel@lists.infradead.org, kvm@vger.kernel.org,
	jgg@nvidia.com, John Hubbard <jhubbard@nvidia.com>,
	zhi.wang.linux@gmail.com, Sean Christopherson <seanjc@google.com>,
	Alistair Popple <apopple@nvidia.com>
Subject: [PATCH 2/2] arm64: Notify on pte permission upgrades
Date: Wed, 24 May 2023 11:47:29 +1000	[thread overview]
Message-ID: <5d8e1f752051173d2d1b5c3e14b54eb3506ed3ef.1684892404.git-series.apopple@nvidia.com> (raw)
In-Reply-To: <3cece716fc09724793aa832e755abfc9d70a8bb3.1684892404.git-series.apopple@nvidia.com>

ARM64 requires TLB invalidates when upgrading pte permission from
read-only to read-write. However mmu_notifiers assume upgrades do not
need notifications and none are sent. This causes problems when a
secondary TLB such as implemented by an ARM SMMU doesn't support
broadcast TLB maintenance (BTM) and caches a read-only PTE.

As no notification is sent and the SMMU does not snoop TLB invalidates
it will continue to return read-only entries to a device even though
the CPU page table contains a writable entry. This leads to a
continually faulting device and no way of handling the fault.

The ARM SMMU driver already registers for mmu notifier events to keep
any secondary TLB synchronised. Therefore sending a notifier on
permission upgrade fixes the problem.

Rather than adding notifier calls to generic architecture independent
code where it may cause performance regressions on architectures that
don't require it add it to the architecture specific
ptep_set_access_flags() where the CPU TLB is invalidated.

Signed-off-by: Alistair Popple <apopple@nvidia.com>

---

A version of this fix was previously posted here:

https://lore.kernel.org/linux-mm/ZGxg+I8FWz3YqBMk@infradead.org/T/

That fix updated generic architecture independent code by adding a new
mmu notifier range event that would allow filtering by
architectures/drivers that didn't require flushing for upgrades.

This was done because calling notifiers from architecture specific
code requires calling the notifier while holding the ptl. It wasn't
immediately obvious that that was safe, but review comments and git
history suggests it must be hence the updated approach here.
---
 arch/arm64/mm/fault.c       |  6 +++++-
 arch/arm64/mm/hugetlbpage.c |  9 +++++++++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index cb21ccd..1ee45a8 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -25,6 +25,7 @@
 #include <linux/perf_event.h>
 #include <linux/preempt.h>
 #include <linux/hugetlb.h>
+#include <linux/mmu_notifier.h>
 
 #include <asm/acpi.h>
 #include <asm/bug.h>
@@ -225,8 +226,11 @@ int ptep_set_access_flags(struct vm_area_struct *vma,
 	} while (pteval != old_pteval);
 
 	/* Invalidate a stale read-only entry */
-	if (dirty)
+	if (dirty) {
 		flush_tlb_page(vma, address);
+		mmu_notifier_invalidate_range(vma->vm_mm, address & PAGE_MASK,
+					(address & PAGE_MASK) + PAGE_SIZE);
+	}
 	return 1;
 }
 
diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
index 95364e8..677f0d1 100644
--- a/arch/arm64/mm/hugetlbpage.c
+++ b/arch/arm64/mm/hugetlbpage.c
@@ -14,6 +14,7 @@
 #include <linux/pagemap.h>
 #include <linux/err.h>
 #include <linux/sysctl.h>
+#include <linux/mmu_notifier.h>
 #include <asm/mman.h>
 #include <asm/tlb.h>
 #include <asm/tlbflush.h>
@@ -487,6 +488,14 @@ int huge_ptep_set_access_flags(struct vm_area_struct *vma,
 
 	orig_pte = get_clear_contig_flush(mm, addr, ptep, pgsize, ncontig);
 
+	/*
+	 * Make sure any cached read-only entries are removed from
+	 * secondary TLBs.
+	 */
+	if (dirty)
+		mmu_notifier_invalidate_range(mm, addr,
+					addr + (pgsize + ncontig));
+
 	/* Make sure we don't lose the dirty or young state */
 	if (pte_dirty(orig_pte))
 		pte = pte_mkdirty(pte);
-- 
git-series 0.9.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2023-05-24  1:47 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-24  1:47 [PATCH 1/2] mmu_notifiers: Restore documentation for .invalidate_range() Alistair Popple
2023-05-24  1:47 ` Alistair Popple
2023-05-24  1:47 ` Alistair Popple [this message]
2023-05-24  1:47   ` [PATCH 2/2] arm64: Notify on pte permission upgrades Alistair Popple
2023-05-28  0:02   ` Jason Gunthorpe
2023-05-28  0:02     ` Jason Gunthorpe
2023-05-30  8:05     ` Alistair Popple
2023-05-30  8:05       ` Alistair Popple
2023-05-30 11:54       ` Jason Gunthorpe
2023-05-30 11:54         ` Jason Gunthorpe
2023-05-30 12:14         ` Robin Murphy
2023-05-30 12:14           ` Robin Murphy
2023-05-30 12:52           ` Jason Gunthorpe
2023-05-30 12:52             ` Jason Gunthorpe
2023-05-30 13:44             ` Robin Murphy
2023-05-30 13:44               ` Robin Murphy
2023-05-30 14:06               ` Jason Gunthorpe
2023-05-30 14:06                 ` Jason Gunthorpe
2023-05-30 21:44                 ` Sean Christopherson
2023-05-30 21:44                   ` Sean Christopherson
2023-05-30 23:08                   ` Jason Gunthorpe
2023-05-30 23:08                     ` Jason Gunthorpe
2023-05-31  0:30                     ` Alistair Popple
2023-05-31  0:30                       ` Alistair Popple
2023-05-31  0:32                       ` Jason Gunthorpe
2023-05-31  0:32                         ` Jason Gunthorpe
2023-05-31  2:46                         ` Alistair Popple
2023-05-31  2:46                           ` Alistair Popple
2023-05-31 15:30                           ` Jason Gunthorpe
2023-05-31 15:30                             ` Jason Gunthorpe
2023-05-31 23:56                             ` Alistair Popple
2023-05-31 23:56                               ` Alistair Popple
     [not found]                       ` <31cdd164783fefad4c9ef4a6d33c1e0094405d0f03added523a82dd9febdf15f@mu.id>
2023-06-09  2:06                         ` Alistair Popple
2023-06-09  2:06                           ` Alistair Popple
2023-06-09  6:05                           ` Alistair Popple
2023-06-09  6:05                             ` Alistair Popple
2023-05-24  2:20 ` [PATCH 1/2] mmu_notifiers: Restore documentation for .invalidate_range() John Hubbard
2023-05-24  2:20   ` John Hubbard
2023-05-24  4:45   ` Alistair Popple
2023-05-24  4:45     ` Alistair Popple
2023-05-27 23:56   ` Jason Gunthorpe
2023-05-27 23:56     ` Jason Gunthorpe
2023-05-24  3:48 ` Zhi Wang
2023-05-24  3:48   ` Zhi Wang
2023-05-24  4:57   ` Alistair Popple
2023-05-24  4:57     ` Alistair Popple

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=5d8e1f752051173d2d1b5c3e14b54eb3506ed3ef.1684892404.git-series.apopple@nvidia.com \
    --to=apopple@nvidia.com \
    --cc=akpm@linux-foundation.org \
    --cc=catalin.marinas@arm.com \
    --cc=jgg@nvidia.com \
    --cc=jhubbard@nvidia.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=nicolinc@nvidia.com \
    --cc=robin.murphy@arm.com \
    --cc=seanjc@google.com \
    --cc=will@kernel.org \
    --cc=zhi.wang.linux@gmail.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.