[PATCH] x86/sev: Unroll string mmio with CC_ATTR_GUEST_UNROLL_STRING_IO

[PATCH] x86/sev: Unroll string mmio with CC_ATTR_GUEST_UNROLL_STRING_IO

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

The io specific memcpy/memset functions use string mmio accesses to do
their work. Under SEV the hypervisor can't emulate these instructions,
because they read/write directly from/to encrypted memory.

KVM will inject a page fault exception into the guest when it is asked
to emulate string mmio instructions for an SEV guest:

	BUG: unable to handle page fault for address: ffffc90000065068
	#PF: supervisor read access in kernel mode
	#PF: error_code(0x0000) - not-present page
	PGD 8000100000067 P4D 8000100000067 PUD 80001000fb067 PMD 80001000fc067 PTE 80000000fed40173
	Oops: 0000 [#1] PREEMPT SMP NOPTI
	CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.17.0-rc7 #3

As string mmio for an SEV guest can not be supported by the
hypervisor, unroll the instructions for CC_ATTR_GUEST_UNROLL_STRING_IO
enabled kernels.

Cc: stable@vger.kernel.org #4.15+
Fixes: d8aa7eea78a1 ('x86/mm: Add Secure Encrypted Virtualization (SEV) support')
Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 arch/x86/lib/iomem.c | 63 ++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 58 insertions(+), 5 deletions(-)

diff --git a/arch/x86/lib/iomem.c b/arch/x86/lib/iomem.c
index df50451d94ef..1246dd558f8d 100644
--- a/arch/x86/lib/iomem.c
+++ b/arch/x86/lib/iomem.c
@@ -22,7 +22,7 @@ static __always_inline void rep_movs(void *to, const void *from, size_t n)
 		     : "memory");
 }
 
-void memcpy_fromio(void *to, const volatile void __iomem *from, size_t n)
+static void string_memcpy_fromio(void *to, const volatile void __iomem *from, size_t n)
 {
 	if (unlikely(!n))
 		return;
@@ -38,9 +38,8 @@ void memcpy_fromio(void *to, const volatile void __iomem *from, size_t n)
 	}
 	rep_movs(to, (const void *)from, n);
 }
-EXPORT_SYMBOL(memcpy_fromio);
 
-void memcpy_toio(volatile void __iomem *to, const void *from, size_t n)
+static void string_memcpy_toio(volatile void __iomem *to, const void *from, size_t n)
 {
 	if (unlikely(!n))
 		return;
@@ -56,9 +55,8 @@ void memcpy_toio(volatile void __iomem *to, const void *from, size_t n)
 	}
 	rep_movs((void *)to, (const void *) from, n);
 }
-EXPORT_SYMBOL(memcpy_toio);
 
-void memset_io(volatile void __iomem *a, int b, size_t c)
+static void string_memset_io(volatile void __iomem *a, int b, size_t c)
 {
 	/*
 	 * TODO: memset can mangle the IO patterns quite a bit.
@@ -66,4 +64,59 @@ void memset_io(volatile void __iomem *a, int b, size_t c)
 	 */
 	memset((void *)a, b, c);
 }
+
+static void unrolled_memcpy_fromio(void *to, const volatile void __iomem *from, size_t n)
+{
+	const volatile char __iomem *in = from;
+	char *out = to;
+	int i;
+
+	for (i = 0; i < n; ++i)
+		out[i] = in[i];
+}
+
+static void unrolled_memcpy_toio(volatile void __iomem *to, const void *from, size_t n)
+{
+	volatile char __iomem *out = to;
+	const char *in = from;
+	int i;
+
+	for (i = 0; i < n; ++i)
+		out[i] = in[i];
+}
+
+static void unrolled_memset_io(volatile void __iomem *a, int b, size_t c)
+{
+	volatile char __iomem *mem = a;
+	int i;
+
+	for (i = 0; i < c; ++i)
+		mem[i] = b;
+}
+
+void memcpy_fromio(void *to, const volatile void __iomem *from, size_t n)
+{
+	if (cc_platform_has(CC_ATTR_GUEST_UNROLL_STRING_IO))
+		unrolled_memcpy_fromio(to, from, n);
+	else
+		string_memcpy_fromio(to, from, n);
+}
+EXPORT_SYMBOL(memcpy_fromio);
+
+void memcpy_toio(volatile void __iomem *to, const void *from, size_t n)
+{
+	if (cc_platform_has(CC_ATTR_GUEST_UNROLL_STRING_IO))
+		unrolled_memcpy_toio(to, from, n);
+	else
+		string_memcpy_toio(to, from, n);
+}
+EXPORT_SYMBOL(memcpy_toio);
+
+void memset_io(volatile void __iomem *a, int b, size_t c)
+{
+	if (cc_platform_has(CC_ATTR_GUEST_UNROLL_STRING_IO))
+		unrolled_memset_io(a, b, c);
+	else
+		string_memset_io(a, b, c);
+}
 EXPORT_SYMBOL(memset_io);
-- 
2.35.1

Re: [PATCH] x86/sev: Unroll string mmio with CC_ATTR_GUEST_UNROLL_STRING_IO

[Borislav Petkov]

On Thu, Mar 10, 2022 at 12:26:15PM +0100, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel@suse.de>
> 
> The io specific memcpy/memset functions use string mmio accesses to do
> their work. Under SEV the hypervisor can't emulate these instructions,
> because they read/write directly from/to encrypted memory.
> 
> KVM will inject a page fault exception into the guest when it is asked
> to emulate string mmio instructions for an SEV guest:
> 
> 	BUG: unable to handle page fault for address: ffffc90000065068
> 	#PF: supervisor read access in kernel mode
> 	#PF: error_code(0x0000) - not-present page
> 	PGD 8000100000067 P4D 8000100000067 PUD 80001000fb067 PMD 80001000fc067 PTE 80000000fed40173
> 	Oops: 0000 [#1] PREEMPT SMP NOPTI
> 	CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.17.0-rc7 #3
> 
> As string mmio for an SEV guest can not be supported by the
> hypervisor, unroll the instructions for CC_ATTR_GUEST_UNROLL_STRING_IO
> enabled kernels.

What I'm missing in this description is why wasn't it a problem until now?

You mentioned something about libvirt adding TPMs and that causing this
but I'm still unclear as to why exactly this is causing the issue. I'm
guessing SEV guests didn't do string IO but libvirt adding a TPM is
somehow causing them to use them now...

> @@ -56,9 +55,8 @@ void memcpy_toio(volatile void __iomem *to, const void *from, size_t n)
>  	}
>  	rep_movs((void *)to, (const void *) from, n);
>  }
> -EXPORT_SYMBOL(memcpy_toio);
>  
> -void memset_io(volatile void __iomem *a, int b, size_t c)
> +static void string_memset_io(volatile void __iomem *a, int b, size_t c)

You can simply remove that wrapper and use memset() at the callsite.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette