On 1/30/20 11:51 AM, Thomas Huth wrote:
> On 29/01/2020 21.03, Janosch Frank wrote:
>> Test if the registers end up having the correct values after a normal,
>> initial and clear reset.
>>
>> Signed-off-by: Janosch Frank <frankja@xxxxxxxxxxxxx>
>> ---
>> tools/testing/selftests/kvm/Makefile | 1 +
>> tools/testing/selftests/kvm/s390x/resets.c | 165 +++++++++++++++++++++
>> 2 files changed, 166 insertions(+)
>> create mode 100644 tools/testing/selftests/kvm/s390x/resets.c
>>
>> diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
>> index 3138a916574a..fe1ea294730c 100644
>> --- a/tools/testing/selftests/kvm/Makefile
>> +++ b/tools/testing/selftests/kvm/Makefile
>> @@ -36,6 +36,7 @@ TEST_GEN_PROGS_aarch64 += kvm_create_max_vcpus
>>
>> TEST_GEN_PROGS_s390x = s390x/memop
>> TEST_GEN_PROGS_s390x += s390x/sync_regs_test
>> +TEST_GEN_PROGS_s390x += s390x/resets
>> TEST_GEN_PROGS_s390x += dirty_log_test
>> TEST_GEN_PROGS_s390x += kvm_create_max_vcpus
>>
>> diff --git a/tools/testing/selftests/kvm/s390x/resets.c b/tools/testing/selftests/kvm/s390x/resets.c
>> new file mode 100644
>> index 000000000000..2b2378cc9e80
>> --- /dev/null
>> +++ b/tools/testing/selftests/kvm/s390x/resets.c
>> @@ -0,0 +1,165 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Test for s390x CPU resets
>> + *
>> + * Copyright (C) 2020, IBM
>> + */
>> +
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <string.h>
>> +#include <sys/ioctl.h>
>> +
>> +#include "test_util.h"
>> +#include "kvm_util.h"
>> +
>> +#define VCPU_ID 3
>> +
>> +struct kvm_vm *vm;
>> +struct kvm_run *run;
>> +struct kvm_sync_regs *regs;
>> +static uint64_t regs_null[16];
>> +
>> +static uint64_t crs[16] = { 0x40000ULL,
>> + 0x42000ULL,
>> + 0, 0, 0, 0, 0,
>> + 0x43000ULL,
>> + 0, 0, 0, 0, 0,
>> + 0x44000ULL,
>> + 0, 0
>> +};
>> +
>> +static void guest_code_initial(void)
>> +{
>> + /* Round toward 0 */
>> + uint32_t fpc = 0x11;
>> +
>> + /* Dirty registers */
>> + asm volatile (
>> + " lctlg 0,15,%0\n"
>> + " sfpc %1\n"
>> + : : "Q" (crs), "d" (fpc));
>
> I'd recommend to add a GUEST_SYNC(0) here ... otherwise the guest code
> tries to return from this function and will cause a crash - which will
> also finish execution of the guest, but might have unexpected side effects.
Ok
>
>> +}
>> +
>> +static void test_one_reg(uint64_t id, uint64_t value)
>> +{
>> + struct kvm_one_reg reg;
>> + uint64_t eval_reg;
>> +
>> + reg.addr = (uintptr_t)&eval_reg;
>> + reg.id = id;
>> + vcpu_get_reg(vm, VCPU_ID, ®);
>> + TEST_ASSERT(eval_reg == value, "value == %s", value);
>> +}
>> +
>> +static void assert_clear(void)
>> +{
>> + struct kvm_sregs sregs;
>> + struct kvm_regs regs;
>> + struct kvm_fpu fpu;
>> +
>> + vcpu_regs_get(vm, VCPU_ID, ®s);
>> + TEST_ASSERT(!memcmp(®s.gprs, regs_null, sizeof(regs.gprs)), "grs == 0");
>> +
>> + vcpu_sregs_get(vm, VCPU_ID, &sregs);
>> + TEST_ASSERT(!memcmp(&sregs.acrs, regs_null, sizeof(sregs.acrs)), "acrs == 0");
>> +
>> + vcpu_fpu_get(vm, VCPU_ID, &fpu);
>> + TEST_ASSERT(!memcmp(&fpu.fprs, regs_null, sizeof(fpu.fprs)), "fprs == 0");
>> +}
>> +
>> +static void assert_initial(void)
>> +{
>> + struct kvm_sregs sregs;
>> + struct kvm_fpu fpu;
>> +
>> + vcpu_sregs_get(vm, VCPU_ID, &sregs);
>> + TEST_ASSERT(sregs.crs[0] == 0xE0UL, "cr0 == 0xE0");
>> + TEST_ASSERT(sregs.crs[14] == 0xC2000000UL, "cr14 == 0xC2000000");
>> + TEST_ASSERT(!memcmp(&sregs.crs[1], regs_null, sizeof(sregs.crs[1]) * 12),
>> + "cr1-13 == 0");
>> + TEST_ASSERT(sregs.crs[15] == 0, "cr15 == 0");
>> +
>> + vcpu_fpu_get(vm, VCPU_ID, &fpu);
>> + TEST_ASSERT(!fpu.fpc, "fpc == 0");
>> +
>> + test_one_reg(KVM_REG_S390_GBEA, 1);
>> + test_one_reg(KVM_REG_S390_PP, 0);
>> + test_one_reg(KVM_REG_S390_TODPR, 0);
>> + test_one_reg(KVM_REG_S390_CPU_TIMER, 0);
>> + test_one_reg(KVM_REG_S390_CLOCK_COMP, 0);
>> +}
>> +
>> +static void assert_normal(void)
>> +{
>> + test_one_reg(KVM_REG_S390_PFTOKEN, KVM_S390_PFAULT_TOKEN_INVALID);
>> +}
>> +
>> +static void test_normal(void)
>> +{
>> + printf("Testing notmal reset\n");
>> + /* Create VM */
>> + vm = vm_create_default(VCPU_ID, 0, guest_code_initial);
>> + run = vcpu_state(vm, VCPU_ID);
>> + regs = &run->s.regs;
>> +
>> + _vcpu_run(vm, VCPU_ID);
>
> Could you use vcpu_run() instead of _vcpu_run() ?
Done.
>
>> + vcpu_ioctl(vm, VCPU_ID, KVM_S390_NORMAL_RESET, 0);
>> + assert_normal();
>> + kvm_vm_free(vm);
>> +}
>> +
>> +static int test_initial(void)
>> +{
>> + int rv;
>> +
>> + printf("Testing initial reset\n");
>> + /* Create VM */
>> + vm = vm_create_default(VCPU_ID, 0, guest_code_initial);
>> + run = vcpu_state(vm, VCPU_ID);
>> + regs = &run->s.regs;
>> +
>> + rv = _vcpu_run(vm, VCPU_ID);
>
> Extra bonus points if you check here that the registers contain the
> values that have been set by the guest ;-)
I started working on that yesterday
>
>> + vcpu_ioctl(vm, VCPU_ID, KVM_S390_INITIAL_RESET, 0);
>> + assert_normal();
>> + assert_initial();
>> + kvm_vm_free(vm);
>> + return rv;
>> +}
>> +
>> +static int test_clear(void)
>> +{
>> + int rv;
>> +
>> + printf("Testing clear reset\n");
>> + /* Create VM */
>> + vm = vm_create_default(VCPU_ID, 0, guest_code_initial);
>> + run = vcpu_state(vm, VCPU_ID);
>> + regs = &run->s.regs;
>> +
>> + rv = _vcpu_run(vm, VCPU_ID);
>> +
>> + vcpu_ioctl(vm, VCPU_ID, KVM_S390_CLEAR_RESET, 0);
>> + assert_normal();
>> + assert_initial();
>> + assert_clear();
>> + kvm_vm_free(vm);
>> + return rv;
>> +}
>> +
>> +int main(int argc, char *argv[])
>> +{
>> + int addl_resets;
>> +
>> + setbuf(stdout, NULL); /* Tell stdout not to buffer its content */
>> + addl_resets = kvm_check_cap(KVM_CAP_S390_VCPU_RESETS);
>> +
>> + test_initial();
>> + if (addl_resets) {
>
> I think you could still fit this into one line, without the need to
> declare the addl_resets variable:
The other question is if we still need to check that if the test is
bundled with the kernel anyway?
>
> if (kvm_check_cap(KVM_CAP_S390_VCPU_RESETS)) {
>
>> + test_normal();
>> + test_clear();
>> + }
>> + return 0;
>> +}
>
> Apart from the nits, this looks pretty good already, thanks for putting
> it together!
>
> Thomas
>
Attachment:
signature.asc
Description: OpenPGP digital signature
