[PATCH v5 0/1] x86/test: Add a test for AMD SEV-ES #VC handling

[PATCH v5 0/1] x86/test: Add a test for AMD SEV-ES #VC handling

[Vasant Karasulli]

Hello!

   This is the version 5 of the patch written to add a test for
   AMD SEV-ES #VC handling. This version attempts to
   address review comments to the previous version of the patch in 
   https://lore.kernel.org/kvm/YNRgHbPVGpLaByjH@zn.tnic/.
   
   Following changes have been made to address the comments: 
   1. Changed the subject to x86/test from x86.
   2. In file arch/x86/Kconfig.debug enclosed the config AMD_SEV_TEST_VC
      inside the guard X86_TESTS.
   3. Renamed the test file to sev-test-vc.c to indicate that
      file includes all the #VC handling related tests.
   4. Memory barries are apparently not necessary and have been removed.
 
Thanks,
Vasant

----------------------------------------------------------------------
 arch/x86/Kbuild              |   2 +
 arch/x86/Kconfig.debug       |  16 ++++
 arch/x86/kernel/Makefile     |   7 ++
 arch/x86/tests/Makefile      |   3 +
 arch/x86/tests/sev-test-vc.c | 154 +++++++++++++++++++++++++++++++++++
 5 files changed, 182 insertions(+)
 create mode 100644 arch/x86/tests/Makefile
 create mode 100644 arch/x86/tests/sev-test-vc.c


base-commit: dfd42facf1e4ada021b939b4e19c935dcdd55566
prerequisite-patch-id: 660950af15f9a972b1f71aa7702ebebe7dd9636b
prerequisite-patch-id: 9635b44cd178a2ee20d83acbc7a07101fd7e81a3
prerequisite-patch-id: 3a7cfa2a885ad20a9f325febcbe31ef53b41e0b1
prerequisite-patch-id: 5e48ede5d4df9de5df3d483874b4aa1711b5dcf0
prerequisite-patch-id: b66ebf185fceb6e8e98b466c974ed777f0200787
prerequisite-patch-id: d5bde21908fbada516dc757e29c6ad70926ab5bf
prerequisite-patch-id: ed38b7fc213fa66845dc25502f18b79e3dc907db
prerequisite-patch-id: 053546f9741643d742f5149c8441f8484e1918d1
prerequisite-patch-id: a20f9a6fd1ad2db0bae2550d5442d7275c481b01
prerequisite-patch-id: bde103392dd4c0a5c360446dbc8d2e5f82671575
prerequisite-patch-id: af44d5e0a6f51e1fe5cf6aef7d0be36e201663b4
prerequisite-patch-id: 952ebde20c8c084f43523f8230ced76beae08b3e
prerequisite-patch-id: 0b295c1545756626a9bca5bce8be4557742b7a56
prerequisite-patch-id: 98bf6777201eeab52938c28de1090387d454ef55
prerequisite-patch-id: d64628531375578e4bc6de3bcba1a3de658e28d8
prerequisite-patch-id: 752ea9954abeb624b803ad86e4974769f97ddd6e
-- 
2.32.0

[PATCH v5 1/1] x86/test: Add a test for AMD SEV-ES #VC handling

[Vasant Karasulli]

From: Varad Gautam <varad.gautam@suse.com>

Add a KUnit based test to validate Linux's VC handling, and introduce
a new CONFIG_X86_TESTS to cover such tests. The test:
1. installs a kretprobe on the #VC handler (sev_es_ghcb_hv_call, to
   access GHCB before/after the resulting VMGEXIT).
2. triggers an NAE.
3. checks that the kretprobe was hit with the right exit_code available
   in GHCB.

Since relying on kprobes, the test does not cover NMI contexts.

Signed-off-by: Varad Gautam <varad.gautam@suse.com>
Signed-off-by: Vasant Karasulli <vkarasulli@suse.de>
---
 arch/x86/Kbuild              |   2 +
 arch/x86/Kconfig.debug       |  16 ++++
 arch/x86/kernel/Makefile     |   7 ++
 arch/x86/tests/Makefile      |   3 +
 arch/x86/tests/sev-test-vc.c | 154 +++++++++++++++++++++++++++++++++++
 5 files changed, 182 insertions(+)
 create mode 100644 arch/x86/tests/Makefile
 create mode 100644 arch/x86/tests/sev-test-vc.c

diff --git a/arch/x86/Kbuild b/arch/x86/Kbuild
index f384cb1a4f7a..90470c76866a 100644
--- a/arch/x86/Kbuild
+++ b/arch/x86/Kbuild
@@ -26,5 +26,7 @@ obj-y += net/
 
 obj-$(CONFIG_KEXEC_FILE) += purgatory/
 
+obj-y += tests/
+
 # for cleaning
 subdir- += boot tools
diff --git a/arch/x86/Kconfig.debug b/arch/x86/Kconfig.debug
index d3a6f74a94bd..e4f61af66816 100644
--- a/arch/x86/Kconfig.debug
+++ b/arch/x86/Kconfig.debug
@@ -279,3 +279,19 @@ endchoice
 config FRAME_POINTER
 	depends on !UNWINDER_ORC && !UNWINDER_GUESS
 	bool
+
+config X86_TESTS
+	bool "Tests for x86"
+	help
+	    This enables building the tests under arch/x86/tests.
+
+if X86_TESTS
+config AMD_SEV_TEST_VC
+	bool "Test for AMD SEV VC exception handling"
+	depends on AMD_MEM_ENCRYPT
+	select FUNCTION_TRACER
+	select KPROBES
+	select KUNIT
+	help
+	  Enable KUnit-based testing for AMD SEV #VC exception handling.
+endif # X86_TESTS
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 6aef9ee28a39..69472a576909 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -24,6 +24,13 @@ CFLAGS_REMOVE_sev.o = -pg
 CFLAGS_REMOVE_cc_platform.o = -pg
 endif
 
+# AMD_SEV_TEST_VC registers a kprobe by function name. IPA-SRA creates
+# function copies and renames them to have an .isra suffix, which breaks kprobes'
+# lookup. Build with -fno-ipa-sra for the test.
+ifdef CONFIG_AMD_SEV_TEST_VC
+CFLAGS_sev.o	+= -fno-ipa-sra
+endif
+
 KASAN_SANITIZE_head$(BITS).o				:= n
 KASAN_SANITIZE_dumpstack.o				:= n
 KASAN_SANITIZE_dumpstack_$(BITS).o			:= n
diff --git a/arch/x86/tests/Makefile b/arch/x86/tests/Makefile
new file mode 100644
index 000000000000..4beca64bd2aa
--- /dev/null
+++ b/arch/x86/tests/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-$(CONFIG_AMD_SEV_TEST_VC)	+= sev-test-vc.o
diff --git a/arch/x86/tests/sev-test-vc.c b/arch/x86/tests/sev-test-vc.c
new file mode 100644
index 000000000000..eb5f790b64f5
--- /dev/null
+++ b/arch/x86/tests/sev-test-vc.c
@@ -0,0 +1,154 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2021 SUSE
+ *
+ * Author: Varad Gautam <varad.gautam@suse.com>
+ */
+
+#include <asm/cpufeature.h>
+#include <asm/debugreg.h>
+#include <asm/io.h>
+#include <asm/sev-common.h>
+#include <asm/svm.h>
+#include <kunit/test.h>
+#include <linux/kprobes.h>
+
+static struct kretprobe hv_call_krp;
+
+static int hv_call_krp_entry(struct kretprobe_instance *krpi,
+			     struct pt_regs *regs)
+{
+	unsigned long ghcb_vaddr = regs_get_kernel_argument(regs, 0);
+	*((unsigned long *) krpi->data) = ghcb_vaddr;
+
+	return 0;
+}
+
+static int hv_call_krp_ret(struct kretprobe_instance *krpi,
+			   struct pt_regs *regs)
+{
+	unsigned long ghcb_vaddr = *((unsigned long *) krpi->data);
+	struct ghcb *ghcb = (struct ghcb *) ghcb_vaddr;
+	struct kunit *test = current->kunit_test;
+
+	if (test && strstr(test->name, "sev_es_") && test->priv)
+		*((u64 *)test->priv) = ghcb->save.sw_exit_code;
+
+	return 0;
+}
+
+int sev_es_test_vc_init(struct kunit *test)
+{
+	int ret;
+
+	if (!cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT)) {
+		kunit_info(test, "Not a SEV-ES guest. Skipping.");
+		ret = -EINVAL;
+		goto out;
+	}
+
+	memset(&hv_call_krp, 0, sizeof(hv_call_krp));
+	hv_call_krp.entry_handler = hv_call_krp_entry;
+	hv_call_krp.handler = hv_call_krp_ret;
+	hv_call_krp.maxactive = 100;
+	hv_call_krp.data_size = sizeof(unsigned long);
+	hv_call_krp.kp.symbol_name = "sev_es_ghcb_hv_call";
+	hv_call_krp.kp.addr = 0;
+
+	ret = register_kretprobe(&hv_call_krp);
+	if (ret) {
+		kunit_info(test, "Could not register kretprobe. Skipping.");
+		goto out;
+	}
+
+	test->priv = kunit_kzalloc(test, sizeof(u64), GFP_KERNEL);
+	if (!test->priv) {
+		ret = -ENOMEM;
+		kunit_info(test, "Could not allocate. Skipping.");
+		goto out;
+	}
+
+out:
+	return ret;
+}
+
+void sev_es_test_vc_exit(struct kunit *test)
+{
+	if (test->priv)
+		kunit_kfree(test, test->priv);
+
+	if (hv_call_krp.kp.addr)
+		unregister_kretprobe(&hv_call_krp);
+}
+
+#define check_op(kt, ec, op)			\
+do {						\
+	struct kunit *t = (struct kunit *) kt;	\
+	op;					\
+	KUNIT_EXPECT_EQ(t, (typeof(ec)) ec,	\
+		*((typeof(ec) *)(t->priv)));		\
+} while (0)
+
+static void sev_es_nae_cpuid(struct kunit *test)
+{
+	unsigned int cpuid_fn = 0x8000001f;
+
+	check_op(test, SVM_EXIT_CPUID, native_cpuid_eax(cpuid_fn));
+}
+
+static void sev_es_nae_wbinvd(struct kunit *test)
+{
+	check_op(test, SVM_EXIT_WBINVD, wbinvd());
+}
+
+static void sev_es_nae_msr(struct kunit *test)
+{
+	check_op(test, SVM_EXIT_MSR, __rdmsr(MSR_IA32_TSC));
+}
+
+static void sev_es_nae_dr7_rw(struct kunit *test)
+{
+	check_op(test, SVM_EXIT_WRITE_DR7,
+		   native_set_debugreg(7, native_get_debugreg(7)));
+}
+
+static void sev_es_nae_ioio(struct kunit *test)
+{
+	unsigned long port = 0x80;
+	char val = 0;
+
+	check_op(test, SVM_EXIT_IOIO, val = inb(port));
+	check_op(test, SVM_EXIT_IOIO, outb(val, port));
+	check_op(test, SVM_EXIT_IOIO, insb(port, &val, sizeof(val)));
+	check_op(test, SVM_EXIT_IOIO, outsb(port, &val, sizeof(val)));
+}
+
+static void sev_es_nae_mmio(struct kunit *test)
+{
+	unsigned long lapic_ver_pa = 0xfee00030; /* APIC_DEFAULT_PHYS_BASE + APIC_LVR */
+	unsigned long __iomem *lapic = ioremap(lapic_ver_pa, 0x4);
+	unsigned long lapic_version = 0;
+
+	check_op(test, SVM_VMGEXIT_MMIO_READ, lapic_version = *lapic);
+	check_op(test, SVM_VMGEXIT_MMIO_WRITE, *lapic = lapic_version);
+
+	iounmap(lapic);
+}
+
+static struct kunit_case sev_es_vc_testcases[] = {
+	KUNIT_CASE(sev_es_nae_cpuid),
+	KUNIT_CASE(sev_es_nae_wbinvd),
+	KUNIT_CASE(sev_es_nae_msr),
+	KUNIT_CASE(sev_es_nae_dr7_rw),
+	KUNIT_CASE(sev_es_nae_ioio),
+	KUNIT_CASE(sev_es_nae_mmio),
+	{}
+};
+
+static struct kunit_suite sev_es_vc_test_suite = {
+	.name = "sev_es_test_vc",
+	.init = sev_es_test_vc_init,
+	.exit = sev_es_test_vc_exit,
+	.test_cases = sev_es_vc_testcases,
+};
+kunit_test_suite(sev_es_vc_test_suite);
-- 
2.32.0

Re: [PATCH v5 1/1] x86/test: Add a test for AMD SEV-ES #VC handling

[Joerg Roedel]

Hi Vasant,

thanks for submitting this.

On Tue, Feb 08, 2022 at 05:26:23PM +0100, Vasant Karasulli wrote:
> From: Varad Gautam <varad.gautam@suse.com>
> 
> Add a KUnit based test to validate Linux's VC handling, and introduce
> a new CONFIG_X86_TESTS to cover such tests. The test:
> 1. installs a kretprobe on the #VC handler (sev_es_ghcb_hv_call, to
>    access GHCB before/after the resulting VMGEXIT).
> 2. triggers an NAE.
> 3. checks that the kretprobe was hit with the right exit_code available
>    in GHCB.
> 
> Since relying on kprobes, the test does not cover NMI contexts.
> 
> Signed-off-by: Varad Gautam <varad.gautam@suse.com>
> Signed-off-by: Vasant Karasulli <vkarasulli@suse.de>
> ---
>  arch/x86/Kbuild              |   2 +
>  arch/x86/Kconfig.debug       |  16 ++++
>  arch/x86/kernel/Makefile     |   7 ++
>  arch/x86/tests/Makefile      |   3 +
>  arch/x86/tests/sev-test-vc.c | 154 +++++++++++++++++++++++++++++++++++
>  5 files changed, 182 insertions(+)
>  create mode 100644 arch/x86/tests/Makefile
>  create mode 100644 arch/x86/tests/sev-test-vc.c

Can we split this up a bit? Say into an initial patch which adds
arch/x86/tests/ and the Kconfig option and then three patches adding the
tests:

	1. Infrastructure and instruction-based tests (cpuid, wbinvd)
	2. Register access tests (MSR, DR7)
	3. IO tests (mmio, ioio)

> +static void sev_es_nae_mmio(struct kunit *test)
> +{
> +	unsigned long lapic_ver_pa = 0xfee00030; /* APIC_DEFAULT_PHYS_BASE + APIC_LVR */

Instead of the comment, please use the values directly to assign the
variable.

Regards,

-- 
Jörg Rödel
jroedel@suse.de

SUSE Software Solutions Germany GmbH
Maxfeldstr. 5
90409 Nürnberg
Germany
 
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev