[PATCH 0/9] x86/mm: CPA robustness fixes and cleanups

[PATCH 0/9] x86/mm: CPA robustness fixes and cleanups

[Mike Rapoport (Microsoft)]

These are x86 CPA robustness fixes and cleanups I found while working on
a generic implementation of the set_memory APIs.

The first three patches fix issues that are unlikely to happen in
practice, but are still worth fixing even if only to avoid LLM spam:
* CPA does not flush the TLB when only an alias of the requested range
  changes, and the alias keeps stale TLB entries.
* mark_rodata_ro() ignores a failure to write protect the kernel and
  enables the read-only protections anyway.
* set_memory_encrypted() and set_memory_decrypted() map random physical
  memory at an unmapped address instead of failing.

The rest are smaller robustness changes and cleanups.

---
Mike Rapoport (Microsoft) (9):
      x86/mm/pat: flush the TLB when only an alias mapping changed
      x86/mm: don't set kernel_set_to_readonly when write protecting fails
      x86/mm/pat: don't populate page tables in set_memory_{en,de}crypted()
      x86/mm/pat: don't BUG() on inconsistent page count
      x86/mm/pat: don't warn about W^X violations before mark_rodata_ro()
      x86/mm/pat: remove CPA_ARRAY
      x86/mm/pat: pass the address to CPA by value
      x86/mm/pat: move the collapse of large pages out of cpa_flush()
      x86/mm/pat: use cpa_flush() on CPA errors

 arch/x86/mm/init_32.c        |   8 +-
 arch/x86/mm/init_64.c        |   8 +-
 arch/x86/mm/pat/cpa-test.c   |  15 ++--
 arch/x86/mm/pat/set_memory.c | 203 +++++++++++++++++++------------------------
 4 files changed, 108 insertions(+), 126 deletions(-)
---
base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
change-id: 20261112-fixes-x86-0c15b6adc212

--
Sincerely yours,
Mike.

[PATCH 1/9] x86/mm/pat: flush the TLB when only an alias mapping changed

[Mike Rapoport (Microsoft)]

CPA flushes the TLB only when CPA_FLUSHTLB is set. cpa_process_alias()
sets the flag only in alias_cpa and never in the original request.

When the requested range already has the new protection and only its
alias changes, the alias keeps stale TLB entries. For example:

	/* the vmalloc mapping is created read-only */
	p = vmap(pages, nr, VM_MAP, PAGE_KERNEL_RO);

	/* only the direct map changes and nothing is flushed */
	set_memory_ro((unsigned long)p, nr);

Setting ->force_flush_all in cpa_process_alias() does not help: it only
chooses how the TLB is flushed, and cpa_flush() is skipped entirely
without CPA_FLUSHTLB.

Worse, when the alias wasn't changed at all, for example when the primary
mapping changes from ROX to RO or vice versa, ->force_flush_all in
cpa_process_alias() causes an unnecessary flush of all TLBs.

Propagate CPA_FLUSHTLB from the alias update to the original request, and
flush the entire TLB only when an alias actually changed.

Fixes: ab5130186d74 ("x86/mm/cpa: Flush direct map alias during cpa")
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/x86/mm/pat/set_memory.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
index b1d5c5a0ffb41..fb2413cb8d314 100644
--- a/arch/x86/mm/pat/set_memory.c
+++ b/arch/x86/mm/pat/set_memory.c
@@ -1975,7 +1975,8 @@ static int cpa_process_alias(struct cpa_data *cpa)
 
 		alias_cpa = *cpa;
 		alias_cpa.vaddr = &laddr;
-		alias_cpa.flags &= ~(CPA_PAGES_ARRAY | CPA_ARRAY);
+		alias_cpa.flags &= ~(CPA_PAGES_ARRAY | CPA_ARRAY |
+				     CPA_FLUSHTLB);
 		alias_cpa.curpage = 0;
 
 		/* Directmap always has NX set, do not modify. */
@@ -1984,9 +1985,11 @@ static int cpa_process_alias(struct cpa_data *cpa)
 			alias_cpa.mask_set.pgprot &= ~_PAGE_NX;
 		}
 
-		cpa->force_flush_all = 1;
-
 		ret = __change_page_attr_set_clr(&alias_cpa, 0);
+		if (alias_cpa.flags & CPA_FLUSHTLB) {
+			cpa->flags |= CPA_FLUSHTLB;
+			cpa->force_flush_all = 1;
+		}
 		if (ret)
 			return ret;
 	}
@@ -2003,7 +2006,8 @@ static int cpa_process_alias(struct cpa_data *cpa)
 					       __START_KERNEL_map - phys_base;
 		alias_cpa = *cpa;
 		alias_cpa.vaddr = &temp_cpa_vaddr;
-		alias_cpa.flags &= ~(CPA_PAGES_ARRAY | CPA_ARRAY);
+		alias_cpa.flags &= ~(CPA_PAGES_ARRAY | CPA_ARRAY |
+				     CPA_FLUSHTLB);
 		alias_cpa.curpage = 0;
 
 		/*
@@ -2015,12 +2019,15 @@ static int cpa_process_alias(struct cpa_data *cpa)
 			alias_cpa.mask_set.pgprot &= ~_PAGE_NX;
 		}
 
-		cpa->force_flush_all = 1;
 		/*
 		 * The high mapping range is imprecise, so ignore the
 		 * return value.
 		 */
 		__change_page_attr_set_clr(&alias_cpa, 0);
+		if (alias_cpa.flags & CPA_FLUSHTLB) {
+			cpa->flags |= CPA_FLUSHTLB;
+			cpa->force_flush_all = 1;
+		}
 	}
 #endif
 

-- 
2.53.0

[PATCH 2/9] x86/mm: don't set kernel_set_to_readonly when write protecting fails

[Mike Rapoport (Microsoft)]

mark_rodata_ro() ignores the result of write protecting the kernel text
and rodata. It can fail when CPA cannot allocate a page table to split a
large page, and then a part of the text and rodata stays writable.

kernel_set_to_readonly is set anyway, and CPA enforces the static
protections of rodata from then on. On 64-bit the next call,
set_memory_nx() for the data, may find the writable rodata and warn
about a conflict with the static protections, which does not say what
actually went wrong.

Report the failure and set kernel_set_to_readonly only when write
protecting succeeds.

Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/x86/mm/init_32.c | 8 ++++++--
 arch/x86/mm/init_64.c | 8 ++++++--
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/arch/x86/mm/init_32.c b/arch/x86/mm/init_32.c
index 0908c44d51e6f..8323c1f599f25 100644
--- a/arch/x86/mm/init_32.c
+++ b/arch/x86/mm/init_32.c
@@ -741,12 +741,16 @@ void mark_rodata_ro(void)
 {
 	unsigned long start = PFN_ALIGN(_text);
 	unsigned long size = (unsigned long)__end_rodata - start;
+	int err;
 
-	set_pages_ro(virt_to_page(start), size >> PAGE_SHIFT);
+	err = set_pages_ro(virt_to_page(start), size >> PAGE_SHIFT);
 	pr_info("Write protecting kernel text and read-only data: %luk\n",
 		size >> 10);
+	if (err)
+		pr_err("Failed to write protect kernel text and read-only data: %d\n",
+		       err);
 
-	kernel_set_to_readonly = 1;
+	kernel_set_to_readonly = !err;
 
 #ifdef CONFIG_CPA_DEBUG
 	pr_info("Testing CPA: Reverting %lx-%lx\n", start, start + size);
diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index 8b6d86fb5a42e..a68998f359bf3 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -1365,12 +1365,16 @@ void mark_rodata_ro(void)
 	unsigned long text_end = PFN_ALIGN(_etext);
 	unsigned long rodata_end = PFN_ALIGN(__end_rodata);
 	unsigned long all_end;
+	int err;
 
 	printk(KERN_INFO "Write protecting the kernel read-only data: %luk\n",
 	       (end - start) >> 10);
-	set_memory_ro(start, (end - start) >> PAGE_SHIFT);
+	err = set_memory_ro(start, (end - start) >> PAGE_SHIFT);
+	if (err)
+		pr_err("Failed to write protect the kernel read-only data: %d\n",
+		       err);
 
-	kernel_set_to_readonly = 1;
+	kernel_set_to_readonly = !err;
 
 	/*
 	 * The rodata/data/bss/brk section (but not the kernel text!)

-- 
2.53.0

[PATCH 3/9] x86/mm/pat: don't populate page tables in set_memory_{en,de}crypted()

[Mike Rapoport (Microsoft)]

__set_memory_enc_pgtable() sets cpa.pgd to init_mm.pgd. With cpa.pgd
set, CPA populates holes in init_mm instead of failing, and
__cpa_process_fault() says that only EFI does that.

So when an address in the range is not mapped, the request does not
fail. populate_pgd() maps the physical memory at cpa.pfn there, with
_KERNPG_TABLE and the encryption bits: present, writable and executable.

A caller that passes an unmapped address is buggy, but it must not get a
new mapping of some random physical memory.

Drop the assignment. The request then walks init_mm as usual: a hole in
the direct map is skipped and any other unmapped address fails with
-EFAULT, as for the other set_memory APIs.

Fixes: 77bd2342d430 ("x86/mm: Add support for changing the memory encryption attribute")
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/x86/mm/pat/set_memory.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
index fb2413cb8d314..aa7052d247e60 100644
--- a/arch/x86/mm/pat/set_memory.c
+++ b/arch/x86/mm/pat/set_memory.c
@@ -2457,7 +2457,6 @@ static int __set_memory_enc_pgtable(unsigned long addr, int numpages, bool enc)
 	cpa.numpages = numpages;
 	cpa.mask_set = enc ? pgprot_encrypted(empty) : pgprot_decrypted(empty);
 	cpa.mask_clr = enc ? pgprot_decrypted(empty) : pgprot_encrypted(empty);
-	cpa.pgd = init_mm.pgd;
 
 	/* Must avoid aliasing mappings in the highmem code */
 	kmap_flush_unused();

-- 
2.53.0

[PATCH 4/9] x86/mm/pat: don't BUG() on inconsistent page count

[Mike Rapoport (Microsoft)]

If a step of __change_page_attr_set_clr() covers more pages than remain
in the range, or no pages at all, that's a bug in CPA, but it does not
justify crashing the kernel.

Warn and fail the request instead.

Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/x86/mm/pat/set_memory.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
index aa7052d247e60..a1bea16a3c3bb 100644
--- a/arch/x86/mm/pat/set_memory.c
+++ b/arch/x86/mm/pat/set_memory.c
@@ -2101,7 +2101,10 @@ static int __change_page_attr_set_clr(struct cpa_data *cpa, int primary)
 		 * CPA operation. Either a large page has been
 		 * preserved or a single page update happened.
 		 */
-		BUG_ON(cpa->numpages > rempages || !cpa->numpages);
+		if (WARN_ON_ONCE(cpa->numpages > rempages || !cpa->numpages)) {
+			ret = -EINVAL;
+			goto out;
+		}
 		rempages -= cpa->numpages;
 		cpa->curpage += cpa->numpages;
 	}

-- 
2.53.0

[PATCH 5/9] x86/mm/pat: don't warn about W^X violations before mark_rodata_ro()

[Mike Rapoport (Microsoft)]

The kernel text is mapped RWX until mark_rodata_ro(), so W^X violations
that verify_rwx() detects before that can be false positives.

CHECK_WX is enabled by default with STRICT_KERNEL_RWX and it reports
mappings that remain W+X after mark_rodata_ro().

Drop the warning about W^X violations before mark_rodata_ro().

Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/x86/mm/pat/set_memory.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
index a1bea16a3c3bb..a674433850dc9 100644
--- a/arch/x86/mm/pat/set_memory.c
+++ b/arch/x86/mm/pat/set_memory.c
@@ -718,21 +718,15 @@ static inline pgprot_t verify_rwx(pgprot_t old, pgprot_t new, unsigned long star
 	if (!rw || nx)
 		return new;
 
-	end = start + npg * PAGE_SIZE - 1;
-
 	/*
-	 * If the kernel text is still RWX, gently complain, this could be a
-	 * false positive.
-	 * Once debug_checkwx() becomes mandatory for CONFIG_STRICT_KERNEL_RWX,
-	 * the warning can be removed completely.
+	 * The kernel text is RWX until mark_rodata_ro() and the entire kernel
+	 * page table is verified after the text is protected, so there is no
+	 * need to warn.
 	 */
-	if (!kernel_set_to_readonly) {
-		pr_warn_once("CPA detected W^X violation: %016llx -> %016llx range: 0x%016lx - 0x%016lx PFN %lx\n",
-		  (unsigned long long)pgprot_val(old),
-		  (unsigned long long)pgprot_val(new),
-		  start, end, pfn);
+	if (!kernel_set_to_readonly)
 		return new;
-	}
+
+	end = start + npg * PAGE_SIZE - 1;
 
 	WARN_ONCE(1, "CPA detected W^X violation: %016llx -> %016llx range: 0x%016lx - 0x%016lx PFN %lx\n",
 		  (unsigned long long)pgprot_val(old),

-- 
2.53.0

[PATCH 6/9] x86/mm/pat: remove CPA_ARRAY

[Mike Rapoport (Microsoft)]

Since commit a919198b97c8 ("x86/mm: Remove the unused
set_memory_array_*() functions") the only user of CPA_ARRAY is the CPA
self test.

Remove CPA_ARRAY and the address array case from the self test.

Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/x86/mm/pat/cpa-test.c   | 15 ++++-------
 arch/x86/mm/pat/set_memory.c | 61 ++++++++++++++++----------------------------
 2 files changed, 27 insertions(+), 49 deletions(-)

diff --git a/arch/x86/mm/pat/cpa-test.c b/arch/x86/mm/pat/cpa-test.c
index ad3c1feec990d..e56a6942f8732 100644
--- a/arch/x86/mm/pat/cpa-test.c
+++ b/arch/x86/mm/pat/cpa-test.c
@@ -22,7 +22,7 @@
 static __read_mostly int print = 1;
 
 enum {
-	NTEST			= 3 * 100,
+	NTEST			= 2 * 100,
 	NPAGES			= 100,
 #ifdef CONFIG_X86_64
 	LPS			= (1 << PMD_SHIFT),
@@ -111,7 +111,6 @@ static unsigned long addr[NTEST];
 static unsigned int len[NTEST];
 
 static struct page *pages[NPAGES];
-static unsigned long addrs[NPAGES];
 
 /* Change the global bit on random pages in the direct mapping */
 static int pageattr_test(void)
@@ -169,7 +168,6 @@ static int pageattr_test(void)
 				break;
 			}
 			__set_bit(pfn + k, bm);
-			addrs[k] = addr[i] + k*PAGE_SIZE;
 			pages[k] = pfn_to_page(pfn + k);
 		}
 		if (!addr[i] || !pte || !k) {
@@ -177,16 +175,13 @@ static int pageattr_test(void)
 			continue;
 		}
 
-		switch (i % 3) {
+		switch (i % 2) {
 		case 0:
-			err = change_page_attr_set(&addr[i], len[i], PAGE_CPA_TEST, 0);
+			err = change_page_attr_set(&addr[i], len[i],
+						   PAGE_CPA_TEST);
 			break;
 
 		case 1:
-			err = change_page_attr_set(addrs, len[i], PAGE_CPA_TEST, 1);
-			break;
-
-		case 2:
 			err = cpa_set_pages_array(pages, len[i], PAGE_CPA_TEST);
 			break;
 		}
@@ -223,7 +218,7 @@ static int pageattr_test(void)
 			failed++;
 			continue;
 		}
-		err = change_page_attr_clear(&addr[i], len[i], PAGE_CPA_TEST, 0);
+		err = change_page_attr_clear(&addr[i], len[i], PAGE_CPA_TEST);
 		if (err < 0) {
 			printk(KERN_ERR "CPA reverting failed: %d\n", err);
 			failed++;
diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
index a674433850dc9..dcc9fa2d4b47f 100644
--- a/arch/x86/mm/pat/set_memory.c
+++ b/arch/x86/mm/pat/set_memory.c
@@ -71,7 +71,6 @@ static const int cpa_warn_level = CPA_PROTECT;
 static DEFINE_SPINLOCK(cpa_lock);
 
 #define CPA_FLUSHTLB		0x01
-#define CPA_ARRAY		0x02
 #define CPA_PAGES_ARRAY		0x04
 #define CPA_NO_CHECK_ALIAS	0x08 /* Do not search for aliases */
 #define CPA_COLLAPSE		0x10 /* try to collapse large pages */
@@ -315,9 +314,6 @@ static unsigned long __cpa_addr(struct cpa_data *cpa, unsigned long idx)
 		return (unsigned long)page_address(page);
 	}
 
-	if (cpa->flags & CPA_ARRAY)
-		return cpa->vaddr[idx];
-
 	return *cpa->vaddr + idx * PAGE_SIZE;
 }
 
@@ -421,7 +417,7 @@ static void __cpa_collapse_large_pages(struct cpa_data *cpa)
 
 	guard(spinlock)(&cpa_lock);
 
-	if (cpa->flags & (CPA_PAGES_ARRAY | CPA_ARRAY)) {
+	if (cpa->flags & CPA_PAGES_ARRAY) {
 		for (i = 0; i < cpa->numpages; i++)
 			collapsed += collapse_large_pages(__cpa_addr(cpa, i),
 							  &pgtables);
@@ -1969,8 +1965,7 @@ static int cpa_process_alias(struct cpa_data *cpa)
 
 		alias_cpa = *cpa;
 		alias_cpa.vaddr = &laddr;
-		alias_cpa.flags &= ~(CPA_PAGES_ARRAY | CPA_ARRAY |
-				     CPA_FLUSHTLB);
+		alias_cpa.flags &= ~(CPA_PAGES_ARRAY | CPA_FLUSHTLB);
 		alias_cpa.curpage = 0;
 
 		/* Directmap always has NX set, do not modify. */
@@ -2000,8 +1995,7 @@ static int cpa_process_alias(struct cpa_data *cpa)
 					       __START_KERNEL_map - phys_base;
 		alias_cpa = *cpa;
 		alias_cpa.vaddr = &temp_cpa_vaddr;
-		alias_cpa.flags &= ~(CPA_PAGES_ARRAY | CPA_ARRAY |
-				     CPA_FLUSHTLB);
+		alias_cpa.flags &= ~(CPA_PAGES_ARRAY | CPA_FLUSHTLB);
 		alias_cpa.curpage = 0;
 
 		/*
@@ -2072,7 +2066,7 @@ static int __change_page_attr_set_clr(struct cpa_data *cpa, int primary)
 		 */
 		cpa->numpages = rempages;
 		/* for array changes, we can't use large page */
-		if (cpa->flags & (CPA_ARRAY | CPA_PAGES_ARRAY))
+		if (cpa->flags & CPA_PAGES_ARRAY)
 			cpa->numpages = 1;
 
 		if (lock) {
@@ -2129,15 +2123,7 @@ static int change_page_attr_set_clr(unsigned long *addr, int numpages,
 		return 0;
 
 	/* Ensure we are PAGE_SIZE aligned */
-	if (in_flag & CPA_ARRAY) {
-		int i;
-		for (i = 0; i < numpages; i++) {
-			if (addr[i] & ~PAGE_MASK) {
-				addr[i] &= PAGE_MASK;
-				WARN_ON_ONCE(1);
-			}
-		}
-	} else if (!(in_flag & CPA_PAGES_ARRAY)) {
+	if (!(in_flag & CPA_PAGES_ARRAY)) {
 		/*
 		 * in_flag of CPA_PAGES_ARRAY implies it is aligned.
 		 * No need to check in that case
@@ -2197,17 +2183,17 @@ static int change_page_attr_set_clr(unsigned long *addr, int numpages,
 }
 
 static inline int change_page_attr_set(unsigned long *addr, int numpages,
-				       pgprot_t mask, int array)
+				       pgprot_t mask)
 {
 	return change_page_attr_set_clr(addr, numpages, mask, __pgprot(0), 0,
-		(array ? CPA_ARRAY : 0), NULL);
+					0, NULL);
 }
 
 static inline int change_page_attr_clear(unsigned long *addr, int numpages,
-					 pgprot_t mask, int array)
+					 pgprot_t mask)
 {
 	return change_page_attr_set_clr(addr, numpages, __pgprot(0), mask, 0,
-		(array ? CPA_ARRAY : 0), NULL);
+					0, NULL);
 }
 
 static inline int cpa_set_pages_array(struct page **pages, int numpages,
@@ -2233,8 +2219,7 @@ int _set_memory_uc(unsigned long addr, int numpages)
 	 * these helpers cannot work with IO memory.
 	 */
 	return change_page_attr_set(&addr, numpages,
-				    cachemode2pgprot(_PAGE_CACHE_MODE_UC_MINUS),
-				    0);
+				cachemode2pgprot(_PAGE_CACHE_MODE_UC_MINUS));
 }
 
 int set_memory_uc(unsigned long addr, int numpages)
@@ -2267,8 +2252,7 @@ int _set_memory_wc(unsigned long addr, int numpages)
 	int ret;
 
 	ret = change_page_attr_set(&addr, numpages,
-				   cachemode2pgprot(_PAGE_CACHE_MODE_UC_MINUS),
-				   0);
+				   cachemode2pgprot(_PAGE_CACHE_MODE_UC_MINUS));
 	if (!ret) {
 		ret = change_page_attr_set_clr(&addr, numpages,
 					       cachemode2pgprot(_PAGE_CACHE_MODE_WC),
@@ -2298,14 +2282,14 @@ EXPORT_SYMBOL(set_memory_wc);
 int _set_memory_wt(unsigned long addr, int numpages)
 {
 	return change_page_attr_set(&addr, numpages,
-				    cachemode2pgprot(_PAGE_CACHE_MODE_WT), 0);
+				    cachemode2pgprot(_PAGE_CACHE_MODE_WT));
 }
 
 int _set_memory_wb(unsigned long addr, int numpages)
 {
 	/* WB cache mode is hard wired to all cache attribute bits being 0 */
 	return change_page_attr_clear(&addr, numpages,
-				      __pgprot(_PAGE_CACHE_MASK), 0);
+				      __pgprot(_PAGE_CACHE_MASK));
 }
 
 int set_memory_wb(unsigned long addr, int numpages)
@@ -2367,7 +2351,7 @@ int set_memory_x(unsigned long addr, int numpages)
 	if (!(__supported_pte_mask & _PAGE_NX))
 		return 0;
 
-	return change_page_attr_clear(&addr, numpages, __pgprot(_PAGE_NX), 0);
+	return change_page_attr_clear(&addr, numpages, __pgprot(_PAGE_NX));
 }
 
 int set_memory_nx(unsigned long addr, int numpages)
@@ -2375,12 +2359,13 @@ int set_memory_nx(unsigned long addr, int numpages)
 	if (!(__supported_pte_mask & _PAGE_NX))
 		return 0;
 
-	return change_page_attr_set(&addr, numpages, __pgprot(_PAGE_NX), 0);
+	return change_page_attr_set(&addr, numpages, __pgprot(_PAGE_NX));
 }
 
 int set_memory_ro(unsigned long addr, int numpages)
 {
-	return change_page_attr_clear(&addr, numpages, __pgprot(_PAGE_RW | _PAGE_DIRTY), 0);
+	return change_page_attr_clear(&addr, numpages,
+				      __pgprot(_PAGE_RW | _PAGE_DIRTY));
 }
 
 int set_memory_rox(unsigned long addr, int numpages)
@@ -2396,12 +2381,12 @@ int set_memory_rox(unsigned long addr, int numpages)
 
 int set_memory_rw(unsigned long addr, int numpages)
 {
-	return change_page_attr_set(&addr, numpages, __pgprot(_PAGE_RW), 0);
+	return change_page_attr_set(&addr, numpages, __pgprot(_PAGE_RW));
 }
 
 int set_memory_np(unsigned long addr, int numpages)
 {
-	return change_page_attr_clear(&addr, numpages, __pgprot(_PAGE_PRESENT), 0);
+	return change_page_attr_clear(&addr, numpages, __pgprot(_PAGE_PRESENT));
 }
 
 int set_memory_np_noalias(unsigned long addr, int numpages)
@@ -2413,7 +2398,7 @@ int set_memory_np_noalias(unsigned long addr, int numpages)
 
 int set_memory_p(unsigned long addr, int numpages)
 {
-	return change_page_attr_set(&addr, numpages, __pgprot(_PAGE_PRESENT), 0);
+	return change_page_attr_set(&addr, numpages, __pgprot(_PAGE_PRESENT));
 }
 
 int set_memory_4k(unsigned long addr, int numpages)
@@ -2425,14 +2410,12 @@ int set_memory_4k(unsigned long addr, int numpages)
 
 int set_memory_nonglobal(unsigned long addr, int numpages)
 {
-	return change_page_attr_clear(&addr, numpages,
-				      __pgprot(_PAGE_GLOBAL), 0);
+	return change_page_attr_clear(&addr, numpages, __pgprot(_PAGE_GLOBAL));
 }
 
 int set_memory_global(unsigned long addr, int numpages)
 {
-	return change_page_attr_set(&addr, numpages,
-				    __pgprot(_PAGE_GLOBAL), 0);
+	return change_page_attr_set(&addr, numpages, __pgprot(_PAGE_GLOBAL));
 }
 
 /*

-- 
2.53.0

[PATCH 7/9] x86/mm/pat: pass the address to CPA by value

[Mike Rapoport (Microsoft)]

cpa_data::vaddr is a pointer because it could point to an array of
addresses with CPA_ARRAY. With CPA_ARRAY gone it always points to a
single address on the caller's stack.

Every caller has to keep a local variable alive just to take its
address, and the alias processing creates temporaries for the same
reason.

Store the address in cpa_data itself.

Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/x86/mm/pat/cpa-test.c   |  4 +--
 arch/x86/mm/pat/set_memory.c | 78 +++++++++++++++++++++-----------------------
 2 files changed, 39 insertions(+), 43 deletions(-)

diff --git a/arch/x86/mm/pat/cpa-test.c b/arch/x86/mm/pat/cpa-test.c
index e56a6942f8732..eef7fae6399bb 100644
--- a/arch/x86/mm/pat/cpa-test.c
+++ b/arch/x86/mm/pat/cpa-test.c
@@ -177,7 +177,7 @@ static int pageattr_test(void)
 
 		switch (i % 2) {
 		case 0:
-			err = change_page_attr_set(&addr[i], len[i],
+			err = change_page_attr_set(addr[i], len[i],
 						   PAGE_CPA_TEST);
 			break;
 
@@ -218,7 +218,7 @@ static int pageattr_test(void)
 			failed++;
 			continue;
 		}
-		err = change_page_attr_clear(&addr[i], len[i], PAGE_CPA_TEST);
+		err = change_page_attr_clear(addr[i], len[i], PAGE_CPA_TEST);
 		if (err < 0) {
 			printk(KERN_ERR "CPA reverting failed: %d\n", err);
 			failed++;
diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
index dcc9fa2d4b47f..94b661c5f7885 100644
--- a/arch/x86/mm/pat/set_memory.c
+++ b/arch/x86/mm/pat/set_memory.c
@@ -40,7 +40,7 @@
  * The current flushing context - we pass it instead of 5 arguments:
  */
 struct cpa_data {
-	unsigned long	*vaddr;
+	unsigned long	addr;
 	pgd_t		*pgd;
 	pgprot_t	mask_set;
 	pgprot_t	mask_clr;
@@ -314,7 +314,7 @@ static unsigned long __cpa_addr(struct cpa_data *cpa, unsigned long idx)
 		return (unsigned long)page_address(page);
 	}
 
-	return *cpa->vaddr + idx * PAGE_SIZE;
+	return cpa->addr + idx * PAGE_SIZE;
 }
 
 /*
@@ -1855,8 +1855,8 @@ static int __cpa_process_fault(struct cpa_data *cpa, unsigned long vaddr,
 		return -EFAULT;
 	} else {
 		WARN(1, KERN_WARNING "CPA: called for zero pte. "
-			"vaddr = %lx cpa->vaddr = %lx\n", vaddr,
-			*cpa->vaddr);
+			"vaddr = %lx cpa->addr = %lx\n", vaddr,
+			cpa->addr);
 
 		return -EFAULT;
 	}
@@ -1948,7 +1948,6 @@ static int __change_page_attr_set_clr(struct cpa_data *cpa, int primary);
 static int cpa_process_alias(struct cpa_data *cpa)
 {
 	struct cpa_data alias_cpa;
-	unsigned long laddr = (unsigned long)__va(cpa->pfn << PAGE_SHIFT);
 	unsigned long vaddr;
 	int ret;
 
@@ -1964,7 +1963,7 @@ static int cpa_process_alias(struct cpa_data *cpa)
 		    PAGE_OFFSET + (max_pfn_mapped << PAGE_SHIFT)))) {
 
 		alias_cpa = *cpa;
-		alias_cpa.vaddr = &laddr;
+		alias_cpa.addr = (unsigned long)__va(cpa->pfn << PAGE_SHIFT);
 		alias_cpa.flags &= ~(CPA_PAGES_ARRAY | CPA_FLUSHTLB);
 		alias_cpa.curpage = 0;
 
@@ -1991,10 +1990,9 @@ static int cpa_process_alias(struct cpa_data *cpa)
 	 */
 	if (!within(vaddr, (unsigned long)_text, _brk_end) &&
 	    __cpa_pfn_in_highmap(cpa->pfn)) {
-		unsigned long temp_cpa_vaddr = (cpa->pfn << PAGE_SHIFT) +
-					       __START_KERNEL_map - phys_base;
 		alias_cpa = *cpa;
-		alias_cpa.vaddr = &temp_cpa_vaddr;
+		alias_cpa.addr = (cpa->pfn << PAGE_SHIFT) +
+				 __START_KERNEL_map - phys_base;
 		alias_cpa.flags &= ~(CPA_PAGES_ARRAY | CPA_FLUSHTLB);
 		alias_cpa.curpage = 0;
 
@@ -2103,7 +2101,7 @@ static int __change_page_attr_set_clr(struct cpa_data *cpa, int primary)
 	return ret;
 }
 
-static int change_page_attr_set_clr(unsigned long *addr, int numpages,
+static int change_page_attr_set_clr(unsigned long addr, int numpages,
 				    pgprot_t mask_set, pgprot_t mask_clr,
 				    int force_split, int in_flag,
 				    struct page **pages)
@@ -2128,8 +2126,8 @@ static int change_page_attr_set_clr(unsigned long *addr, int numpages,
 		 * in_flag of CPA_PAGES_ARRAY implies it is aligned.
 		 * No need to check in that case
 		 */
-		if (*addr & ~PAGE_MASK) {
-			*addr &= PAGE_MASK;
+		if (addr & ~PAGE_MASK) {
+			addr &= PAGE_MASK;
 			/*
 			 * People should not be passing in unaligned addresses:
 			 */
@@ -2142,7 +2140,7 @@ static int change_page_attr_set_clr(unsigned long *addr, int numpages,
 
 	vm_unmap_aliases();
 
-	cpa.vaddr = addr;
+	cpa.addr = addr;
 	cpa.pages = pages;
 	cpa.numpages = numpages;
 	cpa.mask_set = mask_set;
@@ -2182,14 +2180,14 @@ static int change_page_attr_set_clr(unsigned long *addr, int numpages,
 	return ret;
 }
 
-static inline int change_page_attr_set(unsigned long *addr, int numpages,
+static inline int change_page_attr_set(unsigned long addr, int numpages,
 				       pgprot_t mask)
 {
 	return change_page_attr_set_clr(addr, numpages, mask, __pgprot(0), 0,
 					0, NULL);
 }
 
-static inline int change_page_attr_clear(unsigned long *addr, int numpages,
+static inline int change_page_attr_clear(unsigned long addr, int numpages,
 					 pgprot_t mask)
 {
 	return change_page_attr_set_clr(addr, numpages, __pgprot(0), mask, 0,
@@ -2199,14 +2197,14 @@ static inline int change_page_attr_clear(unsigned long *addr, int numpages,
 static inline int cpa_set_pages_array(struct page **pages, int numpages,
 				       pgprot_t mask)
 {
-	return change_page_attr_set_clr(NULL, numpages, mask, __pgprot(0), 0,
+	return change_page_attr_set_clr(0, numpages, mask, __pgprot(0), 0,
 		CPA_PAGES_ARRAY, pages);
 }
 
 static inline int cpa_clear_pages_array(struct page **pages, int numpages,
 					 pgprot_t mask)
 {
-	return change_page_attr_set_clr(NULL, numpages, __pgprot(0), mask, 0,
+	return change_page_attr_set_clr(0, numpages, __pgprot(0), mask, 0,
 		CPA_PAGES_ARRAY, pages);
 }
 
@@ -2218,7 +2216,7 @@ int _set_memory_uc(unsigned long addr, int numpages)
 	 * that you cannot override IO areas with set_memory_*() as
 	 * these helpers cannot work with IO memory.
 	 */
-	return change_page_attr_set(&addr, numpages,
+	return change_page_attr_set(addr, numpages,
 				cachemode2pgprot(_PAGE_CACHE_MODE_UC_MINUS));
 }
 
@@ -2251,10 +2249,10 @@ int _set_memory_wc(unsigned long addr, int numpages)
 {
 	int ret;
 
-	ret = change_page_attr_set(&addr, numpages,
+	ret = change_page_attr_set(addr, numpages,
 				   cachemode2pgprot(_PAGE_CACHE_MODE_UC_MINUS));
 	if (!ret) {
-		ret = change_page_attr_set_clr(&addr, numpages,
+		ret = change_page_attr_set_clr(addr, numpages,
 					       cachemode2pgprot(_PAGE_CACHE_MODE_WC),
 					       __pgprot(_PAGE_CACHE_MASK),
 					       0, 0, NULL);
@@ -2281,14 +2279,14 @@ EXPORT_SYMBOL(set_memory_wc);
 
 int _set_memory_wt(unsigned long addr, int numpages)
 {
-	return change_page_attr_set(&addr, numpages,
+	return change_page_attr_set(addr, numpages,
 				    cachemode2pgprot(_PAGE_CACHE_MODE_WT));
 }
 
 int _set_memory_wb(unsigned long addr, int numpages)
 {
 	/* WB cache mode is hard wired to all cache attribute bits being 0 */
-	return change_page_attr_clear(&addr, numpages,
+	return change_page_attr_clear(addr, numpages,
 				      __pgprot(_PAGE_CACHE_MASK));
 }
 
@@ -2351,7 +2349,7 @@ int set_memory_x(unsigned long addr, int numpages)
 	if (!(__supported_pte_mask & _PAGE_NX))
 		return 0;
 
-	return change_page_attr_clear(&addr, numpages, __pgprot(_PAGE_NX));
+	return change_page_attr_clear(addr, numpages, __pgprot(_PAGE_NX));
 }
 
 int set_memory_nx(unsigned long addr, int numpages)
@@ -2359,12 +2357,12 @@ int set_memory_nx(unsigned long addr, int numpages)
 	if (!(__supported_pte_mask & _PAGE_NX))
 		return 0;
 
-	return change_page_attr_set(&addr, numpages, __pgprot(_PAGE_NX));
+	return change_page_attr_set(addr, numpages, __pgprot(_PAGE_NX));
 }
 
 int set_memory_ro(unsigned long addr, int numpages)
 {
-	return change_page_attr_clear(&addr, numpages,
+	return change_page_attr_clear(addr, numpages,
 				      __pgprot(_PAGE_RW | _PAGE_DIRTY));
 }
 
@@ -2375,47 +2373,47 @@ int set_memory_rox(unsigned long addr, int numpages)
 	if (__supported_pte_mask & _PAGE_NX)
 		clr.pgprot |= _PAGE_NX;
 
-	return change_page_attr_set_clr(&addr, numpages, __pgprot(0), clr, 0,
+	return change_page_attr_set_clr(addr, numpages, __pgprot(0), clr, 0,
 					CPA_COLLAPSE, NULL);
 }
 
 int set_memory_rw(unsigned long addr, int numpages)
 {
-	return change_page_attr_set(&addr, numpages, __pgprot(_PAGE_RW));
+	return change_page_attr_set(addr, numpages, __pgprot(_PAGE_RW));
 }
 
 int set_memory_np(unsigned long addr, int numpages)
 {
-	return change_page_attr_clear(&addr, numpages, __pgprot(_PAGE_PRESENT));
+	return change_page_attr_clear(addr, numpages, __pgprot(_PAGE_PRESENT));
 }
 
 int set_memory_np_noalias(unsigned long addr, int numpages)
 {
-	return change_page_attr_set_clr(&addr, numpages, __pgprot(0),
+	return change_page_attr_set_clr(addr, numpages, __pgprot(0),
 					__pgprot(_PAGE_PRESENT), 0,
 					CPA_NO_CHECK_ALIAS, NULL);
 }
 
 int set_memory_p(unsigned long addr, int numpages)
 {
-	return change_page_attr_set(&addr, numpages, __pgprot(_PAGE_PRESENT));
+	return change_page_attr_set(addr, numpages, __pgprot(_PAGE_PRESENT));
 }
 
 int set_memory_4k(unsigned long addr, int numpages)
 {
-	return change_page_attr_set_clr(&addr, numpages,
+	return change_page_attr_set_clr(addr, numpages,
 					__pgprot(_PAGE_KERNEL_4K),
 					__pgprot(0), 1, 0, NULL);
 }
 
 int set_memory_nonglobal(unsigned long addr, int numpages)
 {
-	return change_page_attr_clear(&addr, numpages, __pgprot(_PAGE_GLOBAL));
+	return change_page_attr_clear(addr, numpages, __pgprot(_PAGE_GLOBAL));
 }
 
 int set_memory_global(unsigned long addr, int numpages)
 {
-	return change_page_attr_set(&addr, numpages, __pgprot(_PAGE_GLOBAL));
+	return change_page_attr_set(addr, numpages, __pgprot(_PAGE_GLOBAL));
 }
 
 /*
@@ -2433,7 +2431,7 @@ static int __set_memory_enc_pgtable(unsigned long addr, int numpages, bool enc)
 		addr &= PAGE_MASK;
 
 	memset(&cpa, 0, sizeof(cpa));
-	cpa.vaddr = &addr;
+	cpa.addr = addr;
 	cpa.numpages = numpages;
 	cpa.mask_set = enc ? pgprot_encrypted(empty) : pgprot_decrypted(empty);
 	cpa.mask_clr = enc ? pgprot_decrypted(empty) : pgprot_encrypted(empty);
@@ -2569,7 +2567,7 @@ static int _set_pages_array(struct page **pages, int numpages,
 	ret = cpa_set_pages_array(pages, numpages,
 				  cachemode2pgprot(set_type));
 	if (!ret && new_type == _PAGE_CACHE_MODE_WC)
-		ret = change_page_attr_set_clr(NULL, numpages,
+		ret = change_page_attr_set_clr(0, numpages,
 					       cachemode2pgprot(
 						_PAGE_CACHE_MODE_WC),
 					       __pgprot(_PAGE_CACHE_MASK),
@@ -2650,8 +2648,7 @@ int set_pages_rw(struct page *page, int numpages)
 
 static int __set_pages_p(struct page *page, int numpages, unsigned int cpa_flags)
 {
-	unsigned long tempaddr = (unsigned long) page_address(page);
-	struct cpa_data cpa = { .vaddr = &tempaddr,
+	struct cpa_data cpa = { .addr = (unsigned long)page_address(page),
 				.pgd = NULL,
 				.numpages = numpages,
 				.mask_set = __pgprot(_PAGE_PRESENT | _PAGE_RW),
@@ -2669,8 +2666,7 @@ static int __set_pages_p(struct page *page, int numpages, unsigned int cpa_flags
 
 static int __set_pages_np(struct page *page, int numpages, unsigned int cpa_flags)
 {
-	unsigned long tempaddr = (unsigned long) page_address(page);
-	struct cpa_data cpa = { .vaddr = &tempaddr,
+	struct cpa_data cpa = { .addr = (unsigned long)page_address(page),
 				.pgd = NULL,
 				.numpages = numpages,
 				.mask_set = __pgprot(0),
@@ -2756,7 +2752,7 @@ int __init kernel_map_pages_in_pgd(pgd_t *pgd, u64 pfn, unsigned long address,
 	int retval = -EINVAL;
 
 	struct cpa_data cpa = {
-		.vaddr = &address,
+		.addr = address,
 		.pfn = pfn,
 		.pgd = pgd,
 		.numpages = numpages,
@@ -2799,7 +2795,7 @@ int __init kernel_unmap_pages_in_pgd(pgd_t *pgd, unsigned long address,
 	 * the *target* of a mapping, it's not useful while unmapping.
 	 */
 	struct cpa_data cpa = {
-		.vaddr		= &address,
+		.addr		= address,
 		.pfn		= 0,
 		.pgd		= pgd,
 		.numpages	= numpages,

-- 
2.53.0

[PATCH 8/9] x86/mm/pat: move the collapse of large pages out of cpa_flush()

[Mike Rapoport (Microsoft)]

Collapsing of large pages was first written to reduce the fragmentation
of the direct map, and cpa_flush() was the best place to hook it
into [1].

When the collapse was reposted for set_memory_rox() only, it stayed in
cpa_flush(), although it has nothing to do with the TLB flush.

Every early exit in cpa_flush() has to jump to the collapse.

Collapse the large pages in change_page_attr_set_clr() after the flush.

[1] https://lore.kernel.org/all/20200416213229.19174-1-kirill.shutemov@linux.intel.com

Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/x86/mm/pat/set_memory.c | 38 +++++++++++++++++---------------------
 1 file changed, 17 insertions(+), 21 deletions(-)

diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
index 94b661c5f7885..80b5a2faefd7c 100644
--- a/arch/x86/mm/pat/set_memory.c
+++ b/arch/x86/mm/pat/set_memory.c
@@ -407,7 +407,7 @@ static void __cpa_flush_tlb(void *data)
 
 static int collapse_large_pages(unsigned long addr, struct list_head *pgtables);
 
-static void __cpa_collapse_large_pages(struct cpa_data *cpa)
+static void cpa_collapse_large_pages(struct cpa_data *cpa)
 {
 	unsigned long start, addr, end;
 	struct ptdesc *ptdesc, *tmp;
@@ -415,8 +415,6 @@ static void __cpa_collapse_large_pages(struct cpa_data *cpa)
 	int collapsed = 0;
 	int i;
 
-	guard(spinlock)(&cpa_lock);
-
 	if (cpa->flags & CPA_PAGES_ARRAY) {
 		for (i = 0; i < cpa->numpages; i++)
 			collapsed += collapse_large_pages(__cpa_addr(cpa, i),
@@ -449,18 +447,6 @@ static void __cpa_collapse_large_pages(struct cpa_data *cpa)
 	}
 }
 
-static void cpa_collapse_large_pages(struct cpa_data *cpa)
-{
-	/*
-	 * Take the mmap write lock on init_mm to:
-	 * - Avoid a use-after-free if raced by ptdump (which takes its own
-	 *   write lock on init_mm).
-	 * - Serialise concurrent CPA walkers.
-	 */
-	scoped_guard(mmap_write_lock, &init_mm)
-		__cpa_collapse_large_pages(cpa);
-}
-
 static void cpa_flush(struct cpa_data *cpa, int cache)
 {
 	unsigned int i;
@@ -469,7 +455,7 @@ static void cpa_flush(struct cpa_data *cpa, int cache)
 
 	if (cache && !cpu_feature_enabled(X86_FEATURE_CLFLUSH)) {
 		cpa_flush_all(cache);
-		goto collapse_large_pages;
+		return;
 	}
 
 	if (cpa->force_flush_all || cpa->numpages > tlb_single_page_flush_ceiling)
@@ -478,7 +464,7 @@ static void cpa_flush(struct cpa_data *cpa, int cache)
 		on_each_cpu(__cpa_flush_tlb, cpa, 1);
 
 	if (!cache)
-		goto collapse_large_pages;
+		return;
 
 	mb();
 	for (i = 0; i < cpa->numpages; i++) {
@@ -494,10 +480,6 @@ static void cpa_flush(struct cpa_data *cpa, int cache)
 			clflush_cache_range_opt((void *)fix_addr(addr), PAGE_SIZE);
 	}
 	mb();
-
-collapse_large_pages:
-	if (cpa->flags & CPA_COLLAPSE)
-		cpa_collapse_large_pages(cpa);
 }
 
 static bool overlaps(unsigned long r1_start, unsigned long r1_end,
@@ -2176,6 +2158,20 @@ static int change_page_attr_set_clr(unsigned long addr, int numpages,
 	}
 
 	cpa_flush(&cpa, cache);
+
+	if (!(cpa.flags & CPA_COLLAPSE))
+		goto out;
+
+	/*
+	 * Take the mmap write lock on init_mm to:
+	 * - Avoid a use-after-free if raced by ptdump (which takes its own
+	 *   write lock on init_mm).
+	 * - Serialise concurrent CPA walkers.
+	 */
+	scoped_guard(mmap_write_lock, &init_mm) {
+		guard(spinlock)(&cpa_lock);
+		cpa_collapse_large_pages(&cpa);
+	}
 out:
 	return ret;
 }

-- 
2.53.0

[PATCH 9/9] x86/mm/pat: use cpa_flush() on CPA errors

[Mike Rapoport (Microsoft)]

When __change_page_attr_set_clr() fails, change_page_attr_set_clr()
calls cpa_flush_all() instead of cpa_flush(). The flush depends on the
error code rather than on what CPA changed.

Force a flush of the entire TLB on error and always call cpa_flush().

The TLB flush stays the same. When cache attributes change and the CPU
supports CLFLUSH, the requested range is flushed with clflush instead
of wbinvd on all CPUs.

Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/x86/mm/pat/set_memory.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
index 80b5a2faefd7c..2ff37ff1cecdc 100644
--- a/arch/x86/mm/pat/set_memory.c
+++ b/arch/x86/mm/pat/set_memory.c
@@ -2152,13 +2152,14 @@ static int change_page_attr_set_clr(unsigned long addr, int numpages,
 	/*
 	 * On error; flush everything to be sure.
 	 */
-	if (ret) {
-		cpa_flush_all(cache);
-		goto out;
-	}
+	if (ret)
+		cpa.force_flush_all = 1;
 
 	cpa_flush(&cpa, cache);
 
+	if (ret)
+		goto out;
+
 	if (!(cpa.flags & CPA_COLLAPSE))
 		goto out;
 

-- 
2.53.0