On 10/14/20 9:27 PM, Collin Walling wrote: > The DIAGNOSE 0x0318 instruction, unique to s390x, is a privileged call > that must be intercepted via SIE, handled in userspace, and the > information set by the instruction is communicated back to KVM. It might be nice to have a few words in here about what information can be set via the diag. > > To test the instruction interception, an ad-hoc handler is defined which > simply has a VM execute the instruction and then userspace will extract > the necessary info. The handler is defined such that the instruction > invocation occurs only once. It is up the the caller to determine how the > info returned by this handler should be used. > > The diag318 info is communicated from userspace to KVM via a sync_regs > call. This is tested during a sync_regs test, where the diag318 info is > requested via the handler, then the info is stored in the appropriate > register in KVM via a sync registers call. > > The diag318 info is checked to be 0 after a normal and clear reset. > > If KVM does not support diag318, then the tests will print a message > stating that diag318 was skipped, and the asserts will simply test > against a value of 0. > > Signed-off-by: Collin Walling Checkpatch throws lots of errors on this patch. Could you check if my workflow misteriously introduced windows line endings or if they were introduced on your side? > --- > tools/testing/selftests/kvm/Makefile | 2 +- > .../kvm/include/s390x/diag318_test_handler.h | 13 +++ > .../kvm/lib/s390x/diag318_test_handler.c | 80 +++++++++++++++++++ > tools/testing/selftests/kvm/s390x/resets.c | 6 ++ > .../selftests/kvm/s390x/sync_regs_test.c | 16 +++- > 5 files changed, 115 insertions(+), 2 deletions(-) > create mode 100644 tools/testing/selftests/kvm/include/s390x/diag318_test_handler.h > create mode 100644 tools/testing/selftests/kvm/lib/s390x/diag318_test_handler.c > > diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile > index 4a166588d99f..ed172a0b83b6 100644 > --- a/tools/testing/selftests/kvm/Makefile > +++ b/tools/testing/selftests/kvm/Makefile > @@ -36,7 +36,7 @@ endif > LIBKVM = lib/assert.c lib/elf.c lib/io.c lib/kvm_util.c lib/sparsebit.c lib/test_util.c > LIBKVM_x86_64 = lib/x86_64/processor.c lib/x86_64/vmx.c lib/x86_64/svm.c lib/x86_64/ucall.c > LIBKVM_aarch64 = lib/aarch64/processor.c lib/aarch64/ucall.c > -LIBKVM_s390x = lib/s390x/processor.c lib/s390x/ucall.c > +LIBKVM_s390x = lib/s390x/processor.c lib/s390x/ucall.c lib/s390x/diag318_test_handler.c > > TEST_GEN_PROGS_x86_64 = x86_64/cr4_cpuid_sync_test > TEST_GEN_PROGS_x86_64 += x86_64/evmcs_test > diff --git a/tools/testing/selftests/kvm/include/s390x/diag318_test_handler.h b/tools/testing/selftests/kvm/include/s390x/diag318_test_handler.h > new file mode 100644 > index 000000000000..d8233ebf317b > --- /dev/null > +++ b/tools/testing/selftests/kvm/include/s390x/diag318_test_handler.h > @@ -0,0 +1,13 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Test handler for the s390x DIAGNOSE 0x0318 instruction. > + * > + * Copyright (C) 2020, IBM > + */ > + > +#ifndef SELFTEST_KVM_DIAG318_TEST_HANDLER > +#define SELFTEST_KVM_DIAG318_TEST_HANDLER > + > +uint64_t get_diag318_info(void); > + > +#endif > diff --git a/tools/testing/selftests/kvm/lib/s390x/diag318_test_handler.c b/tools/testing/selftests/kvm/lib/s390x/diag318_test_handler.c > new file mode 100644 > index 000000000000..024e3a9ffda7 > --- /dev/null > +++ b/tools/testing/selftests/kvm/lib/s390x/diag318_test_handler.c > @@ -0,0 +1,80 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Test handler for the s390x DIAGNOSE 0x0318 instruction. > + * > + * Copyright (C) 2020, IBM > + */ > + > +#include "test_util.h" > +#include "kvm_util.h" > + > +#define VCPU_ID 5 > + > +#define ICPT_INSTRUCTION 0x04 > +#define IPA0_DIAG 0x8300 > + > +static void guest_code(void) > +{ > + uint64_t diag318_info = 0x12345678; > + > + asm volatile ("diag %0,0,0x318\n" : : "d" (diag318_info)); > +} > + > +/* > + * The DIAGNOSE 0x0318 instruction call must be handled via userspace. As such, > + * we create an ad-hoc VM here to handle the instruction then extract the > + * necessary data. It is up to the caller to decide what to do with that data. > + */ > +static uint64_t diag318_handler(void) > +{ > + struct kvm_vm *vm; > + struct kvm_run *run; > + uint64_t reg; > + uint64_t diag318_info; > + > + vm = vm_create_default(VCPU_ID, 0, guest_code); > + vcpu_run(vm, VCPU_ID); > + run = vcpu_state(vm, VCPU_ID); > + > + TEST_ASSERT(run->exit_reason == KVM_EXIT_S390_SIEIC, > + "DIAGNOSE 0x0318 instruction was not intercepted"); > + TEST_ASSERT(run->s390_sieic.icptcode == ICPT_INSTRUCTION, > + "Unexpected intercept code: 0x%x", run->s390_sieic.icptcode); > + TEST_ASSERT((run->s390_sieic.ipa & 0xff00) == IPA0_DIAG, > + "Unexpected IPA0 code: 0x%x", (run->s390_sieic.ipa & 0xff00)); > + > + reg = (run->s390_sieic.ipa & 0x00f0) >> 4; > + diag318_info = run->s.regs.gprs[reg]; > + > + kvm_vm_free(vm); > + > + return diag318_info; > +} > + > +uint64_t get_diag318_info(void) > +{ > + static uint64_t diag318_info; > + static bool printed_skip; > + > + /* > + * If KVM does not support diag318, then return 0 to > + * ensure tests do not break. > + */ > + if (!kvm_check_cap(KVM_CAP_S390_DIAG318)) { > + if (!printed_skip) { > + fprintf(stdout, "KVM_CAP_S390_DIAG318 not supported. " Whitespace after . > + "Skipping diag318 test.\n"); > + printed_skip = true; > + } > + return 0; > + } > + > + /* > + * If a test has previously requested the diag318 info, > + * then don't bother spinning up a temporary VM again. > + */ > + if (!diag318_info) > + diag318_info = diag318_handler(); > + > + return diag318_info; > +} > diff --git a/tools/testing/selftests/kvm/s390x/resets.c b/tools/testing/selftests/kvm/s390x/resets.c > index b143db6d8693..d0416ba94ec5 100644 > --- a/tools/testing/selftests/kvm/s390x/resets.c > +++ b/tools/testing/selftests/kvm/s390x/resets.c > @@ -12,6 +12,7 @@ > > #include "test_util.h" > #include "kvm_util.h" > +#include "diag318_test_handler.h" > > #define VCPU_ID 3 > #define LOCAL_IRQS 32 > @@ -110,6 +111,8 @@ static void assert_clear(void) > > TEST_ASSERT(!memcmp(sync_regs->vrs, regs_null, sizeof(sync_regs->vrs)), > "vrs0-15 == 0 (sync_regs)"); > + > + TEST_ASSERT(sync_regs->diag318 == 0, "diag318 == 0 (sync_regs)"); > } > > static void assert_initial_noclear(void) > @@ -182,6 +185,7 @@ static void assert_normal(void) > test_one_reg(KVM_REG_S390_PFTOKEN, KVM_S390_PFAULT_TOKEN_INVALID); > TEST_ASSERT(sync_regs->pft == KVM_S390_PFAULT_TOKEN_INVALID, > "pft == 0xff..... (sync_regs)"); > + TEST_ASSERT(sync_regs->diag318 == 0, "diag318 == 0 (sync_regs)"); > assert_noirq(); > } > > @@ -206,6 +210,7 @@ static void test_normal(void) > /* Create VM */ > vm = vm_create_default(VCPU_ID, 0, guest_code_initial); > run = vcpu_state(vm, VCPU_ID); > + run->s.regs.diag318 = get_diag318_info(); > sync_regs = &run->s.regs; > > vcpu_run(vm, VCPU_ID); > @@ -250,6 +255,7 @@ static void test_clear(void) > pr_info("Testing clear reset\n"); > vm = vm_create_default(VCPU_ID, 0, guest_code_initial); > run = vcpu_state(vm, VCPU_ID); > + run->s.regs.diag318 = get_diag318_info(); > sync_regs = &run->s.regs; > > vcpu_run(vm, VCPU_ID); > diff --git a/tools/testing/selftests/kvm/s390x/sync_regs_test.c b/tools/testing/selftests/kvm/s390x/sync_regs_test.c > index 5731ccf34917..caf7b8859a94 100644 > --- a/tools/testing/selftests/kvm/s390x/sync_regs_test.c > +++ b/tools/testing/selftests/kvm/s390x/sync_regs_test.c > @@ -20,6 +20,7 @@ > > #include "test_util.h" > #include "kvm_util.h" > +#include "diag318_test_handler.h" > > #define VCPU_ID 5 > > @@ -70,7 +71,7 @@ static void compare_sregs(struct kvm_sregs *left, struct kvm_sync_regs *right) > > #undef REG_COMPARE > > -#define TEST_SYNC_FIELDS (KVM_SYNC_GPRS|KVM_SYNC_ACRS|KVM_SYNC_CRS) > +#define TEST_SYNC_FIELDS (KVM_SYNC_GPRS|KVM_SYNC_ACRS|KVM_SYNC_CRS|KVM_SYNC_DIAG318) > #define INVALID_SYNC_FIELD 0x80000000 > > int main(int argc, char *argv[]) > @@ -152,6 +153,12 @@ int main(int argc, char *argv[]) > > run->kvm_valid_regs = TEST_SYNC_FIELDS; > run->kvm_dirty_regs = KVM_SYNC_GPRS | KVM_SYNC_ACRS; > + > + if (get_diag318_info() > 0) { > + run->s.regs.diag318 = get_diag318_info(); > + run->kvm_dirty_regs |= KVM_SYNC_DIAG318; > + } > + > rv = _vcpu_run(vm, VCPU_ID); > TEST_ASSERT(rv == 0, "vcpu_run failed: %d\n", rv); > TEST_ASSERT(run->exit_reason == KVM_EXIT_S390_SIEIC, > @@ -164,6 +171,9 @@ int main(int argc, char *argv[]) > TEST_ASSERT(run->s.regs.acrs[0] == 1 << 11, > "acr0 sync regs value incorrect 0x%x.", > run->s.regs.acrs[0]); > + TEST_ASSERT(run->s.regs.diag318 == get_diag318_info(), > + "diag318 sync regs value incorrect 0x%llx.", > + run->s.regs.diag318); > > vcpu_regs_get(vm, VCPU_ID, ®s); > compare_regs(®s, &run->s.regs); > @@ -177,6 +187,7 @@ int main(int argc, char *argv[]) > run->kvm_valid_regs = TEST_SYNC_FIELDS; > run->kvm_dirty_regs = 0; > run->s.regs.gprs[11] = 0xDEADBEEF; > + run->s.regs.diag318 = 0x4B1D; > rv = _vcpu_run(vm, VCPU_ID); > TEST_ASSERT(rv == 0, "vcpu_run failed: %d\n", rv); > TEST_ASSERT(run->exit_reason == KVM_EXIT_S390_SIEIC, > @@ -186,6 +197,9 @@ int main(int argc, char *argv[]) > TEST_ASSERT(run->s.regs.gprs[11] != 0xDEADBEEF, > "r11 sync regs value incorrect 0x%llx.", > run->s.regs.gprs[11]); > + TEST_ASSERT(run->s.regs.diag318 != 0x4B1D, > + "diag318 sync regs value incorrect 0x%llx.", > + run->s.regs.diag318); > > kvm_vm_free(vm); > >