Hi All,
This is the version 6 of the patch written to add tests 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/[email protected]/.
Changes in this version:
1. The patch in the previous version is split into 4 parts.
2. Constants in function sev_es_nae_mmio are replaced by macros.
PS: I am resending this version because I made a mistake in the subject.
Apologies for the spam.
Thanks,
Vasant
Vasant Karasulli (4):
x86/tests: Add Kconfig options for testing AMD SEV related features.
x86/tests: Add KUnit based tests to validate Linux's VC handling for
instructions cpuid and wbinvd. These tests: 1. install a kretprobe
on the #VC handler (sev_es_ghcb_hv_call, to access GHCB
before/after the resulting VMGEXIT). 2. trigger an NAE by executing
either cpuid or wbinvd. 3. check that the kretprobe was hit with the
right exit_code available in GHCB.
x86/tests: Add KUnit based tests to validate Linux's VC handling for
instructions accessing registers such as MSR and DR7. These tests:
1. install a kretprobe on the #VC handler (sev_es_ghcb_hv_call,
to access GHCB before/after the resulting VMGEXIT). 2.
trigger an NAE by accessing either MSR or DR7. 3. check that the
kretprobe was hit with the right exit_code available in GHCB.
x86/tests: Add KUnit based tests to validate Linux's VC handling for
IO instructions. These tests: 1. install a kretprobe
on the #VC handler (sev_es_ghcb_hv_call, to access GHCB
before/after the resulting VMGEXIT). 2. trigger an NAE by
issuing an IO instruction. 3. check that the kretprobe was
hit with the right exit_code available in GHCB.
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 | 155 +++++++++++++++++++++++++++++++++++
5 files changed, 183 insertions(+)
create mode 100644 arch/x86/tests/Makefile
create mode 100644 arch/x86/tests/sev-test-vc.c
base-commit: 09688c0166e76ce2fb85e86b9d99be8b0084cdf9
prerequisite-patch-id: b74bc39d7ca69ad86b5f9090047c44ab039f4622
prerequisite-patch-id: a53a291b59b4ceaffa25a9a08dfa08b5a78a01b9
--
2.32.0
Add KUnit based tests to validate Linux's VC handling for
IO instructions.
These tests:
1. install a kretprobe on the #VC handler (sev_es_ghcb_hv_call, to
access GHCB before/after the resulting VMGEXIT).
2. trigger an NAE by issuing an IO instruction.
3. check that the kretprobe was hit with the right
exit_code available in GHCB.
Signed-off-by: Vasant Karasulli <[email protected]>
---
arch/x86/tests/sev-test-vc.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/arch/x86/tests/sev-test-vc.c b/arch/x86/tests/sev-test-vc.c
index b27b9f114a12..d0e742e2bf9f 100644
--- a/arch/x86/tests/sev-test-vc.c
+++ b/arch/x86/tests/sev-test-vc.c
@@ -8,7 +8,9 @@
#include <asm/cpufeature.h>
#include <asm/sev-common.h>
#include <asm/debugreg.h>
+#include <asm/io.h>
#include <asm/svm.h>
+#include <asm/apicdef.h>
#include <kunit/test.h>
#include <linux/kprobes.h>
@@ -111,11 +113,36 @@ static void sev_es_nae_dr7_rw(struct kunit *test)
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 = 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),
{}
};
--
2.32.0
Add Kconfig options for testing AMD SEV
related features.
Signed-off-by: Vasant Karasulli <[email protected]>
---
arch/x86/Kbuild | 2 ++
arch/x86/Kconfig.debug | 16 ++++++++++++++++
arch/x86/kernel/Makefile | 7 +++++++
arch/x86/tests/Makefile | 1 +
4 files changed, 26 insertions(+)
create mode 100644 arch/x86/tests/Makefile
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..f66554cd5c45
--- /dev/null
+++ b/arch/x86/tests/Makefile
@@ -0,0 +1 @@
+# SPDX-License-Identifier: GPL-2.0
--
2.32.0
Add KUnit based tests to validate Linux's
VC handling for instructions accessing registers such as MSR and DR7.
These tests:
1. install a kretprobe on the #VC handler (sev_es_ghcb_hv_call, to
access GHCB before/after the resulting VMGEXIT).
2. trigger an NAE by accessing either MSR or DR7.
3. check that the kretprobe was hit with the right exit_code available
in GHCB.
Signed-off-by: Vasant Karasulli <[email protected]>
---
arch/x86/tests/sev-test-vc.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/arch/x86/tests/sev-test-vc.c b/arch/x86/tests/sev-test-vc.c
index 9d415b9708dc..b27b9f114a12 100644
--- a/arch/x86/tests/sev-test-vc.c
+++ b/arch/x86/tests/sev-test-vc.c
@@ -7,6 +7,7 @@
#include <asm/cpufeature.h>
#include <asm/sev-common.h>
+#include <asm/debugreg.h>
#include <asm/svm.h>
#include <kunit/test.h>
#include <linux/kprobes.h>
@@ -99,9 +100,22 @@ 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 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),
{}
};
--
2.32.0
Add KUnit based tests to validate Linux's
VC handling for instructions cpuid and wbinvd.
These tests:
1. install a kretprobe on the #VC handler (sev_es_ghcb_hv_call, to
access GHCB before/after the resulting VMGEXIT).
2. trigger an NAE by executing either cpuid or wbinvd.
3. check that the kretprobe was hit with the right exit_code
available in GHCB.
Signed-off-by: Vasant Karasulli <[email protected]>
---
arch/x86/tests/Makefile | 2 +
arch/x86/tests/sev-test-vc.c | 114 +++++++++++++++++++++++++++++++++++
2 files changed, 116 insertions(+)
create mode 100644 arch/x86/tests/sev-test-vc.c
diff --git a/arch/x86/tests/Makefile b/arch/x86/tests/Makefile
index f66554cd5c45..4beca64bd2aa 100644
--- a/arch/x86/tests/Makefile
+++ b/arch/x86/tests/Makefile
@@ -1 +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..9d415b9708dc
--- /dev/null
+++ b/arch/x86/tests/sev-test-vc.c
@@ -0,0 +1,114 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2021 SUSE
+ *
+ * Author: Varad Gautam <[email protected]>
+ */
+
+#include <asm/cpufeature.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 struct kunit_case sev_es_vc_testcases[] = {
+ KUNIT_CASE(sev_es_nae_cpuid),
+ KUNIT_CASE(sev_es_nae_wbinvd),
+ {}
+};
+
+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
> Subject: Re: [PATCH v6 1/4] x86/tests: Add tests for AMD SEV-ES #VC handling
Your subject need to summarize each patch and not be the same for each
patch.
On Fri, Mar 18, 2022 at 11:46:43AM +0100, Vasant Karasulli wrote:
> Add Kconfig options for testing AMD SEV
> related features.
>
> Signed-off-by: Vasant Karasulli <[email protected]>
> ---
> arch/x86/Kbuild | 2 ++
> arch/x86/Kconfig.debug | 16 ++++++++++++++++
> arch/x86/kernel/Makefile | 7 +++++++
> arch/x86/tests/Makefile | 1 +
> 4 files changed, 26 insertions(+)
> create mode 100644 arch/x86/tests/Makefile
>
> 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"
"x86 unit tests"
or so.
> + help
> + This enables building the tests under arch/x86/tests.
This needs to explain more what "the tests" are and how people should
run them or at least there should be a pointer to a doc how. Running
tests should be trivial to mount so that everyone can run them. You want
as many people testing stuff as possible so the testing infra needs to
be easy to use.
For example, I have no clue how to run those tests.
Also, I have no clue why those tests are in arch/x86/tests/ and not
somewhere in tools/testing/selftests/x86/ or so.
All this stuff needs to be explained in the commit message.
Also, you should read
Documentation/process/submitting-patches.rst
first as there it is explained at length how a patch should look like.
> +
> +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
Why ifdef?
I think you want this to be enabled unconditionally since the VC tests
select KRPOBES.
> +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..f66554cd5c45
> --- /dev/null
> +++ b/arch/x86/tests/Makefile
> @@ -0,0 +1 @@
> +# SPDX-License-Identifier: GPL-2.0
Add that file with the next patch - this hunk is just silly as it is.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Fri, Mar 18, 2022 at 11:46:44AM +0100, Vasant Karasulli wrote:
> Add KUnit based tests to validate Linux's
> VC handling for instructions cpuid and wbinvd.
This looks like a commit message title to me.
Also, please write x86 instructions in all caps: CPUID and WBINVD.
> These tests:
> 1. install a kretprobe on the #VC handler (sev_es_ghcb_hv_call, to
> access GHCB before/after the resulting VMGEXIT).
> 2. trigger an NAE by executing either cpuid or wbinvd.
> 3. check that the kretprobe was hit with the right exit_code
> available in GHCB.
Yes, I'd appreciate a short step-by-step thing how to run them or a
pointer to the docs.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Fr 08-04-22 15:44:26, Borislav Petkov wrote:
> > Subject: Re: [PATCH v6 1/4] x86/tests: Add tests for AMD SEV-ES #VC handling
>
> > 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
>
> Why ifdef?
>
> I think you want this to be enabled unconditionally since the VC tests
> select KRPOBES.
>
VC tests added in this patch series depend on the configuration
option CONFIG_AMD_SEV_TEST_VC which in turn selects KPROBES.
I think compiler flag -fno-ipa-sra wouldn't be necessary if
configuration option CONFIG_AMD_SEV_TEST_VC is disabled.
> > +CFLAGS_sev.o += -fno-ipa-sra
> > +endif
> > +
>
> --
> Regards/Gruss,
> Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette
--
Vasant Karasulli
Kernel generalist
http://www.suse.com<http://www.suse.com>
[https://www.suse.com/assets/img/social-platforms-suse-logo.png]<http://www.suse.com/>
SUSE - Open Source Solutions for Enterprise Servers & Cloud<http://www.suse.com/>
Modernize your infrastructure with SUSE Linux Enterprise servers, cloud technology for IaaS, and SUSE's software-defined storage.
http://www.suse.com
![https://www.suse.com/assets/img/social-platforms-suse-logo.png]<http://www.suse.com/>](https://www.suse.com/assets/img/social-platforms-suse-logo.png]<http://www.suse.com/>){kind=link}