Re: AMD boot woe due to "x86/mm: Cleanup pgprot_4k_2_large() and pgprot_large_2_4k()"

[Christoph Hellwig <hch@lst.de>]

[-- Attachment #1: Type: text/plain, Size: 886 bytes --]

On Wed, Apr 22, 2020 at 11:55:54AM -0400, Qian Cai wrote:
> Reverted the linux-next commit and its dependency,
> 
> a85573f7e741 ("x86/mm: Unexport __cachemode2pte_tbl”)
> 9e294786c89a (“x86/mm: Cleanup pgprot_4k_2_large() and pgprot_large_2_4k()”)
> 
> fixed crashes or hard reset on AMD machines during boot that have been flagged by
> KASAN in different forms indicating some sort of memory corruption with this config,

Interesting.  Your config seems to boot fine in my VM until the point
where the lack of virtio-blk support stops it from mounting the root
file system.

Looking at the patch I found one bug, although that should not affect
your config (it should use the pgprotval_t type), and one difference
that could affect code generation, although I prefer the new version
(use of __pgprot vs a local variable + pgprot_val()).

Two patches attached, can you try them?

[-- Attachment #2: 0001-x86-Use-pgprotval_t-in-protval_4k_2_large-and-pgprot.patch --]
[-- Type: text/x-patch, Size: 1378 bytes --]

From 71829ed28a4f3d616382e7a362d501eb9ea7dc13 Mon Sep 17 00:00:00 2001
From: Christoph Hellwig <hch@lst.de>
Date: Wed, 22 Apr 2020 18:53:08 +0200
Subject: x86: Use pgprotval_t in protval_4k_2_large and pgprot_4k_2_large

Use the proper type for "raw" page table values.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/x86/include/asm/pgtable_types.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/pgtable_types.h b/arch/x86/include/asm/pgtable_types.h
index 567abdbd64d3..7b6ddcf77d70 100644
--- a/arch/x86/include/asm/pgtable_types.h
+++ b/arch/x86/include/asm/pgtable_types.h
@@ -478,7 +478,7 @@ static inline pteval_t pte_flags(pte_t pte)
 
 unsigned long cachemode2protval(enum page_cache_mode pcm);
 
-static inline unsigned long protval_4k_2_large(unsigned long val)
+static inline pgprotval_t protval_4k_2_large(pgprotval_t val)
 {
 	return (val & ~(_PAGE_PAT | _PAGE_PAT_LARGE)) |
 		((val & _PAGE_PAT) << (_PAGE_BIT_PAT_LARGE - _PAGE_BIT_PAT));
@@ -487,7 +487,7 @@ static inline pgprot_t pgprot_4k_2_large(pgprot_t pgprot)
 {
 	return __pgprot(protval_4k_2_large(pgprot_val(pgprot)));
 }
-static inline unsigned long protval_large_2_4k(unsigned long val)
+static inline pgprotval_t protval_large_2_4k(pgprotval_t val)
 {
 	return (val & ~(_PAGE_PAT | _PAGE_PAT_LARGE)) |
 		((val & _PAGE_PAT_LARGE) >>
-- 
2.26.1


[-- Attachment #3: 0002-foo.patch --]
[-- Type: text/x-patch, Size: 1206 bytes --]

From e5a6c2e84accad3d528c5c90c74071d10079db9a Mon Sep 17 00:00:00 2001
From: Christoph Hellwig <hch@lst.de>
Date: Wed, 22 Apr 2020 18:54:45 +0200
Subject: foo

---
 arch/x86/include/asm/pgtable_types.h | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/pgtable_types.h b/arch/x86/include/asm/pgtable_types.h
index 7b6ddcf77d70..c6d4725269bb 100644
--- a/arch/x86/include/asm/pgtable_types.h
+++ b/arch/x86/include/asm/pgtable_types.h
@@ -485,7 +485,10 @@ static inline pgprotval_t protval_4k_2_large(pgprotval_t val)
 }
 static inline pgprot_t pgprot_4k_2_large(pgprot_t pgprot)
 {
-	return __pgprot(protval_4k_2_large(pgprot_val(pgprot)));
+	pgprot_t new;
+
+	pgprot_val(new) = protval_4k_2_large(pgprot_val(pgprot));
+	return new;
 }
 static inline pgprotval_t protval_large_2_4k(pgprotval_t val)
 {
@@ -495,9 +498,11 @@ static inline pgprotval_t protval_large_2_4k(pgprotval_t val)
 }
 static inline pgprot_t pgprot_large_2_4k(pgprot_t pgprot)
 {
-	return __pgprot(protval_large_2_4k(pgprot_val(pgprot)));
-}
+	pgprot_t new;
 
+	pgprot_val(new) = protval_large_2_4k(pgprot_val(pgprot));
+	return new;
+}
 
 typedef struct page *pgtable_t;
 
-- 
2.26.1