Re: [PATCH] x86/mm: Fix PAGE_KERNEL_IO removal breakage

From: Lucas De Marchi
Date: Fri Dec 03 2021 - 13:48:23 EST


On Fri, Dec 03, 2021 at 11:32:45AM -0600, Tom Lendacky wrote:
On 12/3/21 11:28 AM, Tom Lendacky wrote:
On 12/2/21 6:25 PM, Lucas De Marchi wrote:
On Thu, Dec 02, 2021 at 07:55:14AM -0800, Lucas De Marchi wrote:
On Thu, Dec 02, 2021 at 03:46:46PM +0100, Joerg Roedel wrote:
From: Joerg Roedel <jroedel@xxxxxxx>

The removal of PAGE_KERNEL_IO broke SEV-ES because it changed the
mapping of ioremap and some fixmap areas (like the local APIC page)
from unencrypted to encrypted. Change those mappings back to
be unencrypted.

Cc: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
Fixes: 27dff0f58bde ("x86/mm: Nuke PAGE_KERNEL_IO")
Signed-off-by: Joerg Roedel <jroedel@xxxxxxx>

oops, missed the fact PAGE_KERNEL had `| ENC` while PAGE_KERENL_IO
didn't have it. Thanks for the fixup.

on a second thought, the fact that PAGE_KERNEL is _not_ the same as
PAGE_KERNEL_IO, completely invalidates those 2 patches I sent. It seems
I screwed it up big here.

About the first patch,
6b2a2138cf36 ("drm/i915/gem: Stop using PAGE_KERNEL_IO"),
I didn't notice any regression on the i915
side though. Is it safe to keep it? Otherwise we are probably better
off reverting everything.

Is i915 for just integrated graphics? In which case SME/SEV-ES aren't available on Intel.

No, it's also for discrete graphics - so it could also be paired with an
AMD cpu.

Currently the i915 driver has some assumptions about the architecture
(x86/x86-64) since it used to be for integrated-only and thus Intel-only.
But we are now extending it to other architectures like arm64. That's
why this patch was written: I misread PAGE_KERNEL and PAGE_KERNEL_IO
being the same thing nowadays, so I was switched to the one available in
other archs.

thanks
Lucas De Marchi


Thanks,
Tom


I'm wondering why the addition of memory encryption
in 21729f81ce8a ("x86/mm: Provide general kernel support for memory encryption")
didn't break io_mapping_init_wc() though as it had already done a
s/PAGE_KERNEL_IO/PAGE_KERNEL/ in commit
ac96b5566926 ("io-mapping.h: s/PAGE_KERNEL_IO/PAGE_KERNEL/")

If I follow it correctly, since SME/SEV-ES are X86_64 only, io_mapping_init_wc() takes the ioremap_wc() path which uses PAGE_KERNEL_IO. iomap_create_wc() is only called when CONFIG_HAVE_ATOMIC_IOMAP is set, which isn't set for X86_64.

Thanks,
Tom


thanks
Lucas De Marchi


Reviewed-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>

Lucas De Marchi

---
arch/x86/include/asm/fixmap.h        |  2 +-
arch/x86/include/asm/pgtable_types.h | 21 +++++++++++----------
arch/x86/mm/ioremap.c                |  2 +-
3 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/arch/x86/include/asm/fixmap.h b/arch/x86/include/asm/fixmap.h
index 5e186a69db10..a2eaf265f784 100644
--- a/arch/x86/include/asm/fixmap.h
+++ b/arch/x86/include/asm/fixmap.h
@@ -173,7 +173,7 @@ static inline void __set_fixmap(enum fixed_addresses idx,
* supported for MMIO addresses, so make sure that the memory encryption
* mask is not part of the page attributes.
*/
-#define FIXMAP_PAGE_NOCACHE PAGE_KERNEL_NOCACHE
+#define FIXMAP_PAGE_NOCACHE PAGE_KERNEL_NOCACHE_NOENC

/*
* Early memremap routines used for in-place encryption. The mappings created
diff --git a/arch/x86/include/asm/pgtable_types.h b/arch/x86/include/asm/pgtable_types.h
index a87224767ff3..fc9b6995cb22 100644
--- a/arch/x86/include/asm/pgtable_types.h
+++ b/arch/x86/include/asm/pgtable_types.h
@@ -208,16 +208,17 @@ enum page_cache_mode {

#define __pgprot_mask(x)    __pgprot((x) & __default_kernel_pte_mask)

-#define PAGE_KERNEL        __pgprot_mask(__PAGE_KERNEL            | _ENC)
-#define PAGE_KERNEL_NOENC    __pgprot_mask(__PAGE_KERNEL |    0)
-#define PAGE_KERNEL_RO        __pgprot_mask(__PAGE_KERNEL_RO | _ENC)
-#define PAGE_KERNEL_EXEC    __pgprot_mask(__PAGE_KERNEL_EXEC       | _ENC)
-#define PAGE_KERNEL_EXEC_NOENC __pgprot_mask(__PAGE_KERNEL_EXEC |    0)
-#define PAGE_KERNEL_ROX        __pgprot_mask(__PAGE_KERNEL_ROX | _ENC)
-#define PAGE_KERNEL_NOCACHE    __pgprot_mask(__PAGE_KERNEL_NOCACHE | _ENC)
-#define PAGE_KERNEL_LARGE    __pgprot_mask(__PAGE_KERNEL_LARGE      | _ENC)
-#define PAGE_KERNEL_LARGE_EXEC __pgprot_mask(__PAGE_KERNEL_LARGE_EXEC | _ENC)
-#define PAGE_KERNEL_VVAR    __pgprot_mask(__PAGE_KERNEL_VVAR       | _ENC)
+#define PAGE_KERNEL            __pgprot_mask(__PAGE_KERNEL | _ENC)
+#define PAGE_KERNEL_NOENC __pgprot_mask(__PAGE_KERNEL            |    0)
+#define PAGE_KERNEL_RO __pgprot_mask(__PAGE_KERNEL_RO         | _ENC)
+#define PAGE_KERNEL_EXEC        __pgprot_mask(__PAGE_KERNEL_EXEC | _ENC)
+#define PAGE_KERNEL_EXEC_NOENC __pgprot_mask(__PAGE_KERNEL_EXEC |    0)
+#define PAGE_KERNEL_ROX __pgprot_mask(__PAGE_KERNEL_ROX        | _ENC)
+#define PAGE_KERNEL_NOCACHE __pgprot_mask(__PAGE_KERNEL_NOCACHE    | _ENC)
+#define PAGE_KERNEL_NOCACHE_NOENC __pgprot_mask(__PAGE_KERNEL_NOCACHE    |    0)
+#define PAGE_KERNEL_LARGE __pgprot_mask(__PAGE_KERNEL_LARGE      | _ENC)
+#define PAGE_KERNEL_LARGE_EXEC __pgprot_mask(__PAGE_KERNEL_LARGE_EXEC | _ENC)
+#define PAGE_KERNEL_VVAR        __pgprot_mask(__PAGE_KERNEL_VVAR | _ENC)

#endif    /* __ASSEMBLY__ */

diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index 3102dda4b152..4fe8d43d53bb 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -243,7 +243,7 @@ __ioremap_caller(resource_size_t phys_addr, unsigned long size,
     * make sure the memory encryption attribute is enabled in the
     * resulting mapping.
     */
-    prot = PAGE_KERNEL;
+    prot = PAGE_KERNEL_NOENC;
    if ((io_desc.flags & IORES_MAP_ENCRYPTED) || encrypted)
        prot = pgprot_encrypted(prot);

--
2.34.0