From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-23.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_IN_DEF_DKIM_WL autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id DD066C433FE for ; Fri, 3 Sep 2021 20:42:22 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C7267610C8 for ; Fri, 3 Sep 2021 20:42:22 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1350960AbhICUnV (ORCPT ); Fri, 3 Sep 2021 16:43:21 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40524 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1350931AbhICUnU (ORCPT ); Fri, 3 Sep 2021 16:43:20 -0400 Received: from mail-yb1-xb30.google.com (mail-yb1-xb30.google.com [IPv6:2607:f8b0:4864:20::b30]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 78B7AC061575 for ; Fri, 3 Sep 2021 13:42:19 -0700 (PDT) Received: by mail-yb1-xb30.google.com with SMTP id j195so762816ybg.6 for ; Fri, 03 Sep 2021 13:42:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=xuC1AbQeewFRo/xFFqcL88Ah+7CpgJ5CaVM6UZ6mtTI=; b=jM60ipIM3egn5pQ+GdkesL8xzx68ExyAKWHEiBTtmGEKVkhU5ELoPgYxRdVsTDKCEU 6u1/Hhv3qdofh57ovdeLp0PBJq3/ccJC8RrajCa0wjhuwEUdWl/2YqgwPLQrODcGJGIj bbnZs9KUiFrGZ4FKcNl/vToO7Z8Kf4cbLMDsJlHWUSYPWSWy/2iwtVG0WrEMTkmcrHAK LtyQkK0XNOK/Hf4fal2UGoJzxeLpfX+e0FaUVEjtWrarqQle35I+bLgIeX3OglM5bDo4 uSfu6rleePJB6h2EMQPXvGc0kyEjSHGFdvUNn5/9HpFMUk8TsL/x83yE/uCCt/3d7xgx NmRw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=xuC1AbQeewFRo/xFFqcL88Ah+7CpgJ5CaVM6UZ6mtTI=; b=NYvo4kbAlwiq1S3qP3+K7syqvFo++2alQXcfkrrrA9MqlxfI4Br9IicA5Ua88sT8Js kHes5AgiTVNHm3sX1q1y+Qoyr70Cd2bD/HPqw/wFgjEIi/XLrK+skFnUPDBHDmwE6frk kPaNdhCUKMP7BIEKJa6t7bBLSnNfVIwx8t9e7FATXoPzgtQsBmFTaOosvR67++4FjjWc GTzu/upBANhCxDSi3JLaAnDega2BELLfIFloAOLcIOcnQw2/4EkeiMUUYtul4YnNpRLm JDCuzwO+CQOIxvSO+qBqzAg2OGmqPpuQRzwYtMppn1smH/SEcUBvd5xZlWJ+IWKQD+4U 9wgw== X-Gm-Message-State: AOAM532o64O6NdsXFm5LJItlhEZT9C2qb3PWKIfgI2tMqLz9r69aKy5F lystTFOE8mU9ROkCmtmd0k0OE3efXFZDThDbY08f7Q== X-Google-Smtp-Source: ABdhPJyHg7Q6U+LEI9cT8RqvbrXGuG61A+G+I9rNAlLOGZo9V713C/qUl4kUtsr4QUdE6efN1jSpFLbnaBJoUQjOKmQ= X-Received: by 2002:a25:188b:: with SMTP id 133mr1222109yby.80.1630701738352; Fri, 03 Sep 2021 13:42:18 -0700 (PDT) MIME-Version: 1.0 References: <20210901211412.4171835-1-rananta@google.com> <20210901211412.4171835-12-rananta@google.com> <20210903104823.ih4aj34vrbhlfhy3@gator.home> In-Reply-To: <20210903104823.ih4aj34vrbhlfhy3@gator.home> From: Raghavendra Rao Ananta Date: Fri, 3 Sep 2021 13:42:05 -0700 Message-ID: Subject: Re: [PATCH v3 11/12] KVM: arm64: selftests: Add arch_timer test To: Andrew Jones Cc: Paolo Bonzini , Marc Zyngier , James Morse , Alexandru Elisei , Suzuki K Poulose , kvm@vger.kernel.org, Catalin Marinas , Peter Shier , linux-kernel@vger.kernel.org, Will Deacon , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Sep 3, 2021 at 3:48 AM Andrew Jones wrote: > > On Wed, Sep 01, 2021 at 09:14:11PM +0000, Raghavendra Rao Ananta wrote: > > Add a KVM selftest to validate the arch_timer functionality. > > Primarily, the test sets up periodic timer interrupts and > > validates the basic architectural expectations upon its receipt. > > > > The test provides command-line options to configure the period > > of the timer, number of iterations, and number of vCPUs. > > > > Signed-off-by: Raghavendra Rao Ananta > > --- > > tools/testing/selftests/kvm/.gitignore | 1 + > > tools/testing/selftests/kvm/Makefile | 1 + > > .../selftests/kvm/aarch64/arch_timer.c | 351 ++++++++++++++++++ > > 3 files changed, 353 insertions(+) > > create mode 100644 tools/testing/selftests/kvm/aarch64/arch_timer.c > > > > diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore > > index 98053d3afbda..c6058df0cd18 100644 > > --- a/tools/testing/selftests/kvm/.gitignore > > +++ b/tools/testing/selftests/kvm/.gitignore > > @@ -1,4 +1,5 @@ > > # SPDX-License-Identifier: GPL-2.0-only > > +/aarch64/arch_timer > > /aarch64/debug-exceptions > > /aarch64/get-reg-list > > /aarch64/psci_cpu_on_test > > diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile > > index 8342f65c1d96..46d43e706b20 100644 > > --- a/tools/testing/selftests/kvm/Makefile > > +++ b/tools/testing/selftests/kvm/Makefile > > @@ -84,6 +84,7 @@ TEST_GEN_PROGS_x86_64 += set_memory_region_test > > TEST_GEN_PROGS_x86_64 += steal_time > > TEST_GEN_PROGS_x86_64 += kvm_binary_stats_test > > > > +TEST_GEN_PROGS_aarch64 += aarch64/arch_timer > > TEST_GEN_PROGS_aarch64 += aarch64/debug-exceptions > > TEST_GEN_PROGS_aarch64 += aarch64/get-reg-list > > TEST_GEN_PROGS_aarch64 += aarch64/psci_cpu_on_test > > diff --git a/tools/testing/selftests/kvm/aarch64/arch_timer.c b/tools/testing/selftests/kvm/aarch64/arch_timer.c > > new file mode 100644 > > index 000000000000..1383f33850e9 > > --- /dev/null > > +++ b/tools/testing/selftests/kvm/aarch64/arch_timer.c > > @@ -0,0 +1,351 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * arch_timer.c - Tests the aarch64 timer IRQ functionality > > + * > > + * The test validates both the virtual and physical timer IRQs using > > + * CVAL and TVAL registers. This consitutes the four stages in the test. > > + * The guest's main thread configures the timer interrupt for a stage > > + * and waits for it to fire, with a timeout equal to the timer period. > > + * It asserts that the timeout doesn't exceed the timer period. > > + * > > + * On the other hand, upon receipt of an interrupt, the guest's interrupt > > + * handler validates the interrupt by checking if the architectural state > > + * is in compliance with the specifications. > > + * > > + * The test provides command-line options to configure the timer's > > + * period (-p), number of vCPUs (-n), and iterations per stage (-i). > > + * > > + * Copyright (c) 2021, Google LLC. > > + */ > > + > > +#define _GNU_SOURCE > > + > > +#include > > +#include > > +#include > > +#include > > + > > +#include "kvm_util.h" > > +#include "processor.h" > > +#include "delay.h" > > +#include "arch_timer.h" > > +#include "gic.h" > > +#include "vgic.h" > > + > > +#define NR_VCPUS_DEF 4 > > +#define NR_TEST_ITERS_DEF 5 > > +#define TIMER_TEST_PERIOD_MS_DEF 10 > > +#define TIMER_TEST_ERR_MARGIN_US 100 > > + > > +struct test_args { > > + int nr_vcpus; > > + int nr_iter; > > + int timer_period_ms; > > +}; > > + > > +static struct test_args test_args = { > > + .nr_vcpus = NR_VCPUS_DEF, > > + .nr_iter = NR_TEST_ITERS_DEF, > > + .timer_period_ms = TIMER_TEST_PERIOD_MS_DEF, > > +}; > > + > > +#define msecs_to_usecs(msec) ((msec) * 1000LL) > > + > > +#define VTIMER_IRQ 27 > > +#define PTIMER_IRQ 30 > > + > > +#define GICD_BASE_GPA 0x8000000ULL > > +#define GICR_BASE_GPA 0x80A0000ULL > > + > > +enum guest_stage { > > + GUEST_STAGE_VTIMER_CVAL = 1, > > + GUEST_STAGE_VTIMER_TVAL, > > + GUEST_STAGE_PTIMER_CVAL, > > + GUEST_STAGE_PTIMER_TVAL, > > + GUEST_STAGE_MAX, > > +}; > > + > > +/* Sahred variables between host and guest */ > > Shared > > > +struct test_vcpu_shared_data { > > + int nr_iter; > > + enum guest_stage guest_stage; > > + uint64_t xcnt; > > +}; > > + > > +struct test_vcpu { > > + uint32_t vcpuid; > > + pthread_t pt_vcpu_run; > > + struct kvm_vm *vm; > > +}; > > + > > +static struct test_vcpu test_vcpu[KVM_MAX_VCPUS]; > > +static struct test_vcpu_shared_data vcpu_shared_data[KVM_MAX_VCPUS]; > > + > > +static void > > +guest_configure_timer_action(struct test_vcpu_shared_data *shared_data) > > +{ > > + switch (shared_data->guest_stage) { > > + case GUEST_STAGE_VTIMER_CVAL: > > + timer_set_next_cval_ms(VIRTUAL, test_args.timer_period_ms); > > + shared_data->xcnt = timer_get_cntct(VIRTUAL); > > + timer_set_ctl(VIRTUAL, CTL_ENABLE); > > + break; > > + case GUEST_STAGE_VTIMER_TVAL: > > + timer_set_next_tval_ms(VIRTUAL, test_args.timer_period_ms); > > + shared_data->xcnt = timer_get_cntct(VIRTUAL); > > + timer_set_ctl(VIRTUAL, CTL_ENABLE); > > + break; > > + case GUEST_STAGE_PTIMER_CVAL: > > + timer_set_next_cval_ms(PHYSICAL, test_args.timer_period_ms); > > + shared_data->xcnt = timer_get_cntct(PHYSICAL); > > + timer_set_ctl(PHYSICAL, CTL_ENABLE); > > + break; > > + case GUEST_STAGE_PTIMER_TVAL: > > + timer_set_next_tval_ms(PHYSICAL, test_args.timer_period_ms); > > + shared_data->xcnt = timer_get_cntct(PHYSICAL); > > + timer_set_ctl(PHYSICAL, CTL_ENABLE); > > + break; > > Since we divide the stages up for vtimer and ptimer, then I'm not sure we > need the wrapper fuctions for timer register get/set with the vtimer and > ptimer switches too. > I understand it's a little redundant. But like you saw in the arch_timer framework patch, the helper functions to program the cval, or tval, or any others to follow, can be made very slim. Thanks for catching the typos. Will fix them. Regards, Raghavendra > > + default: > > + GUEST_ASSERT(0); > > + } > > +} > > + > > +static void guest_validate_irq(unsigned int intid, > > + struct test_vcpu_shared_data *shared_data) > > +{ > > + enum guest_stage stage = shared_data->guest_stage; > > + uint64_t xcnt = 0, xcnt_diff_us, cval = 0; > > + unsigned long xctl = 0; > > + unsigned int timer_irq = 0; > > + > > + if (stage == GUEST_STAGE_VTIMER_CVAL || > > + stage == GUEST_STAGE_VTIMER_TVAL) { > > + xctl = timer_get_ctl(VIRTUAL); > > + timer_set_ctl(VIRTUAL, CTL_IMASK); > > + xcnt = timer_get_cntct(VIRTUAL); > > + cval = timer_get_cval(VIRTUAL); > > + timer_irq = VTIMER_IRQ; > > + } else if (stage == GUEST_STAGE_PTIMER_CVAL || > > + stage == GUEST_STAGE_PTIMER_TVAL) { > > + xctl = timer_get_ctl(PHYSICAL); > > + timer_set_ctl(PHYSICAL, CTL_IMASK); > > + xcnt = timer_get_cntct(PHYSICAL); > > + cval = timer_get_cval(PHYSICAL); > > + timer_irq = PTIMER_IRQ; > > + } else { > > + GUEST_ASSERT(0); > > + } > > + > > + xcnt_diff_us = cycles_to_usec(xcnt - shared_data->xcnt); > > + > > + /* Make sure we are dealing with the correct timer IRQ */ > > + GUEST_ASSERT_2(intid == timer_irq, intid, timer_irq); > > + > > + /* Basic 'timer codition met' check */ > > condition > > > + GUEST_ASSERT_3(xcnt >= cval, xcnt, cval, xcnt_diff_us); > > + GUEST_ASSERT_1(xctl & CTL_ISTATUS, xctl); > > +} > > + > > +static void guest_irq_handler(struct ex_regs *regs) > > +{ > > + unsigned int intid = gic_get_and_ack_irq(); > > + uint32_t cpu = get_vcpuid(); > > + struct test_vcpu_shared_data *shared_data = &vcpu_shared_data[cpu]; > > + > > + guest_validate_irq(intid, shared_data); > > + > > + WRITE_ONCE(shared_data->nr_iter, shared_data->nr_iter + 1); > > + > > + gic_set_eoi(intid); > > +} > > + > > +static void guest_run_stage(struct test_vcpu_shared_data *shared_data, > > + enum guest_stage stage) > > +{ > > + uint32_t irq_iter, config_iter; > > + > > + shared_data->guest_stage = stage; > > + shared_data->nr_iter = 0; > > + > > + for (config_iter = 0; config_iter < test_args.nr_iter; config_iter++) { > > + /* Setup the next interrupt */ > > + guest_configure_timer_action(shared_data); > > + > > + /* Setup a timeout for the interrupt to arrive */ > > + udelay(msecs_to_usecs(test_args.timer_period_ms) + > > + TIMER_TEST_ERR_MARGIN_US); > > + > > + irq_iter = READ_ONCE(shared_data->nr_iter); > > + GUEST_ASSERT_2(config_iter + 1 == irq_iter, > > + config_iter + 1, irq_iter); > > + }; > > extra ; > > > +} > > + > > +static void guest_code(void) > > +{ > > + uint32_t cpu = get_vcpuid(); > > + struct test_vcpu_shared_data *shared_data = &vcpu_shared_data[cpu]; > > + > > + local_irq_disable(); > > + > > + gic_init(GIC_V3, test_args.nr_vcpus, > > + (void *)GICD_BASE_GPA, (void *)GICR_BASE_GPA); > > + > > + timer_set_ctl(VIRTUAL, CTL_IMASK); > > + timer_set_ctl(PHYSICAL, CTL_IMASK); > > + > > + gic_irq_enable(VTIMER_IRQ); > > + gic_irq_enable(PTIMER_IRQ); > > + local_irq_enable(); > > + > > + guest_run_stage(shared_data, GUEST_STAGE_VTIMER_CVAL); > > + guest_run_stage(shared_data, GUEST_STAGE_VTIMER_TVAL); > > + guest_run_stage(shared_data, GUEST_STAGE_PTIMER_CVAL); > > + guest_run_stage(shared_data, GUEST_STAGE_PTIMER_TVAL); > > + > > + GUEST_DONE(); > > +} > > + > > +static void *test_vcpu_run(void *arg) > > +{ > > + struct ucall uc; > > + struct test_vcpu *vcpu = arg; > > + struct kvm_vm *vm = vcpu->vm; > > + uint32_t vcpuid = vcpu->vcpuid; > > + struct test_vcpu_shared_data *shared_data = &vcpu_shared_data[vcpuid]; > > + > > + vcpu_run(vm, vcpuid); > > + > > + switch (get_ucall(vm, vcpuid, &uc)) { > > + case UCALL_SYNC: > > + case UCALL_DONE: > > + break; > > + case UCALL_ABORT: > > + sync_global_from_guest(vm, *shared_data); > > + TEST_ASSERT(false, > > TEST_FAIL(fmt, ...) can be used. > > > + "%s at %s:%ld\n\tvalues: %lu, %lu; %lu, vcpu: %u; stage: %u; iter: %u", > > + (const char *)uc.args[0], __FILE__, uc.args[1], > > + uc.args[2], uc.args[3], uc.args[4], vcpuid, > > + shared_data->guest_stage, shared_data->nr_iter); > > + break; > > + default: > > + TEST_FAIL("Unexpected guest exit\n"); > > + } > > + > > + return NULL; > > +} > > + > > +static void test_run(struct kvm_vm *vm) > > +{ > > + int i, ret; > > + > > + for (i = 0; i < test_args.nr_vcpus; i++) { > > + ret = pthread_create(&test_vcpu[i].pt_vcpu_run, NULL, > > + test_vcpu_run, &test_vcpu[i]); > > + TEST_ASSERT(!ret, "Failed to create vCPU-%d pthread\n", i); > > + } > > + > > + for (i = 0; i < test_args.nr_vcpus; i++) > > + pthread_join(test_vcpu[i].pt_vcpu_run, NULL); > > +} > > + > > +static struct kvm_vm *test_vm_create(void) > > +{ > > + struct kvm_vm *vm; > > + unsigned int i; > > + int nr_vcpus = test_args.nr_vcpus; > > + > > + vm = vm_create_default_with_vcpus(nr_vcpus, 0, 0, guest_code, NULL); > > + > > + vm_init_descriptor_tables(vm); > > + vm_install_exception_handler(vm, VECTOR_IRQ_CURRENT, guest_irq_handler); > > + > > + for (i = 0; i < nr_vcpus; i++) { > > + vcpu_init_descriptor_tables(vm, i); > > + > > + test_vcpu[i].vcpuid = i; > > + test_vcpu[i].vm = vm; > > + } > > + > > + ucall_init(vm, NULL); > > + vgic_v3_setup(vm, nr_vcpus, GICD_BASE_GPA, GICR_BASE_GPA, 1); > > + > > + /* Make all the test's cmdline args visible to the guest */ > > + sync_global_to_guest(vm, test_args); > > + > > + return vm; > > +} > > + > > +static void test_print_help(char *name) > > +{ > > + pr_info("Usage: %s [-h] [-n nr_vcpus] [-i iterations] [-p timer_period_ms]\n", > > + name); > > + pr_info("\t-n: Number of vCPUs to configure (default: %u; max: %u)\n", > > + NR_VCPUS_DEF, KVM_MAX_VCPUS); > > + pr_info("\t-i: Number of iterations per stage (default: %u)\n", > > + NR_TEST_ITERS_DEF); > > + pr_info("\t-p: Periodicity (in ms) of the guest timer (default: %u)\n", > > + TIMER_TEST_PERIOD_MS_DEF); > > + pr_info("\t-h: print this help screen\n"); > > +} > > + > > +static bool parse_args(int argc, char *argv[]) > > +{ > > + int opt; > > + > > + while ((opt = getopt(argc, argv, "hn:i:p:")) != -1) { > > + switch (opt) { > > + case 'n': > > + test_args.nr_vcpus = atoi(optarg); > > + if (test_args.nr_vcpus <= 0) { > > + pr_info("Positive value needed for -n\n"); > > + goto err; > > + } else if (test_args.nr_vcpus > KVM_MAX_VCPUS) { > > + pr_info("Max allowed vCPUs: %u\n", > > + KVM_MAX_VCPUS); > > + goto err; > > + } > > + break; > > + case 'i': > > + test_args.nr_iter = atoi(optarg); > > + if (test_args.nr_iter <= 0) { > > + pr_info("Positive value needed for -i\n"); > > + goto err; > > + } > > + break; > > + case 'p': > > + test_args.timer_period_ms = atoi(optarg); > > + if (test_args.timer_period_ms <= 0) { > > + pr_info("Positive value needed for -p\n"); > > + goto err; > > + } > > + break; > > + case 'h': > > + default: > > + goto err; > > + } > > + } > > + > > + return true; > > + > > +err: > > + test_print_help(argv[0]); > > + return false; > > +} > > + > > +int main(int argc, char *argv[]) > > +{ > > + struct kvm_vm *vm; > > + > > + /* Tell stdout not to buffer its content */ > > + setbuf(stdout, NULL); > > + > > + if (!parse_args(argc, argv)) > > + exit(KSFT_SKIP); > > + > > + vm = test_vm_create(); > > + test_run(vm); > > + kvm_vm_free(vm); > > + > > + return 0; > > +} > > -- > > 2.33.0.153.gba50c8fa24-goog > > Besides the nits, > > Reviewed-by: Andrew Jones > > Thanks, > drew > From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.5 required=3.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED,DKIM_INVALID,DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 39F74C433F5 for ; Fri, 3 Sep 2021 20:42:25 +0000 (UTC) Received: from mm01.cs.columbia.edu (mm01.cs.columbia.edu [128.59.11.253]) by mail.kernel.org (Postfix) with ESMTP id 9EB67610E6 for ; Fri, 3 Sep 2021 20:42:24 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 9EB67610E6 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.cs.columbia.edu Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 16F004B215; Fri, 3 Sep 2021 16:42:24 -0400 (EDT) X-Virus-Scanned: at lists.cs.columbia.edu Authentication-Results: mm01.cs.columbia.edu (amavisd-new); dkim=softfail (fail, message has been altered) header.i=@google.com Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id lUsZNJNiVpde; Fri, 3 Sep 2021 16:42:22 -0400 (EDT) Received: from mm01.cs.columbia.edu (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 98C094B17B; Fri, 3 Sep 2021 16:42:22 -0400 (EDT) Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id B5AA24B17B for ; Fri, 3 Sep 2021 16:42:20 -0400 (EDT) X-Virus-Scanned: at lists.cs.columbia.edu Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 7FmLXp9gJcLT for ; Fri, 3 Sep 2021 16:42:19 -0400 (EDT) Received: from mail-yb1-f178.google.com (mail-yb1-f178.google.com [209.85.219.178]) by mm01.cs.columbia.edu (Postfix) with ESMTPS id 40C1B4B178 for ; Fri, 3 Sep 2021 16:42:19 -0400 (EDT) Received: by mail-yb1-f178.google.com with SMTP id f15so784989ybg.3 for ; Fri, 03 Sep 2021 13:42:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=xuC1AbQeewFRo/xFFqcL88Ah+7CpgJ5CaVM6UZ6mtTI=; b=jM60ipIM3egn5pQ+GdkesL8xzx68ExyAKWHEiBTtmGEKVkhU5ELoPgYxRdVsTDKCEU 6u1/Hhv3qdofh57ovdeLp0PBJq3/ccJC8RrajCa0wjhuwEUdWl/2YqgwPLQrODcGJGIj bbnZs9KUiFrGZ4FKcNl/vToO7Z8Kf4cbLMDsJlHWUSYPWSWy/2iwtVG0WrEMTkmcrHAK LtyQkK0XNOK/Hf4fal2UGoJzxeLpfX+e0FaUVEjtWrarqQle35I+bLgIeX3OglM5bDo4 uSfu6rleePJB6h2EMQPXvGc0kyEjSHGFdvUNn5/9HpFMUk8TsL/x83yE/uCCt/3d7xgx NmRw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=xuC1AbQeewFRo/xFFqcL88Ah+7CpgJ5CaVM6UZ6mtTI=; b=R+sNJ0EU6wK03NcwETvAZbZ/czVwUXd2lJibH6hQoGkvGpG6wiMh6h8ObSaXFzerCB W7qC2Q/wnnHZJh/OfABDbkUcE5SIi2768YtQ418jArFtnlZzDevF2XsjPsTxsUJvAzYA sxz68k3cgbUinvGjIq0aAGlSwdsCmrelSgObaeK6KEUFwKHllZ3coaR1u4st4iHeGNqW dk/mirKZV0DP2nsjM0lwVaJoAClFIge7GoAY1TrJipRxVArWQCSpEliJx0eHrojo2jtw v1HsetSQ5/ks2XCoUs/r4rsiU7bSqBP/FXC1Pz7WfOFbECWmoeSp0DqFT5FAIDlFZeAP 5zpw== X-Gm-Message-State: AOAM533MC3XX8HiZ3NkgZNLgso6QJmi7N8aWoB0lwQqUjV9Js5rzHtdo +HaZKzL0+owFumHtiTVNXSfnwTiKZl3IfZPVA9IYjQ== X-Google-Smtp-Source: ABdhPJyHg7Q6U+LEI9cT8RqvbrXGuG61A+G+I9rNAlLOGZo9V713C/qUl4kUtsr4QUdE6efN1jSpFLbnaBJoUQjOKmQ= X-Received: by 2002:a25:188b:: with SMTP id 133mr1222109yby.80.1630701738352; Fri, 03 Sep 2021 13:42:18 -0700 (PDT) MIME-Version: 1.0 References: <20210901211412.4171835-1-rananta@google.com> <20210901211412.4171835-12-rananta@google.com> <20210903104823.ih4aj34vrbhlfhy3@gator.home> In-Reply-To: <20210903104823.ih4aj34vrbhlfhy3@gator.home> From: Raghavendra Rao Ananta Date: Fri, 3 Sep 2021 13:42:05 -0700 Message-ID: Subject: Re: [PATCH v3 11/12] KVM: arm64: selftests: Add arch_timer test To: Andrew Jones Cc: kvm@vger.kernel.org, Marc Zyngier , Peter Shier , linux-kernel@vger.kernel.org, Will Deacon , Catalin Marinas , Paolo Bonzini , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org X-BeenThere: kvmarm@lists.cs.columbia.edu X-Mailman-Version: 2.1.14 Precedence: list List-Id: Where KVM/ARM decisions are made List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: kvmarm-bounces@lists.cs.columbia.edu Sender: kvmarm-bounces@lists.cs.columbia.edu On Fri, Sep 3, 2021 at 3:48 AM Andrew Jones wrote: > > On Wed, Sep 01, 2021 at 09:14:11PM +0000, Raghavendra Rao Ananta wrote: > > Add a KVM selftest to validate the arch_timer functionality. > > Primarily, the test sets up periodic timer interrupts and > > validates the basic architectural expectations upon its receipt. > > > > The test provides command-line options to configure the period > > of the timer, number of iterations, and number of vCPUs. > > > > Signed-off-by: Raghavendra Rao Ananta > > --- > > tools/testing/selftests/kvm/.gitignore | 1 + > > tools/testing/selftests/kvm/Makefile | 1 + > > .../selftests/kvm/aarch64/arch_timer.c | 351 ++++++++++++++++++ > > 3 files changed, 353 insertions(+) > > create mode 100644 tools/testing/selftests/kvm/aarch64/arch_timer.c > > > > diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore > > index 98053d3afbda..c6058df0cd18 100644 > > --- a/tools/testing/selftests/kvm/.gitignore > > +++ b/tools/testing/selftests/kvm/.gitignore > > @@ -1,4 +1,5 @@ > > # SPDX-License-Identifier: GPL-2.0-only > > +/aarch64/arch_timer > > /aarch64/debug-exceptions > > /aarch64/get-reg-list > > /aarch64/psci_cpu_on_test > > diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile > > index 8342f65c1d96..46d43e706b20 100644 > > --- a/tools/testing/selftests/kvm/Makefile > > +++ b/tools/testing/selftests/kvm/Makefile > > @@ -84,6 +84,7 @@ TEST_GEN_PROGS_x86_64 += set_memory_region_test > > TEST_GEN_PROGS_x86_64 += steal_time > > TEST_GEN_PROGS_x86_64 += kvm_binary_stats_test > > > > +TEST_GEN_PROGS_aarch64 += aarch64/arch_timer > > TEST_GEN_PROGS_aarch64 += aarch64/debug-exceptions > > TEST_GEN_PROGS_aarch64 += aarch64/get-reg-list > > TEST_GEN_PROGS_aarch64 += aarch64/psci_cpu_on_test > > diff --git a/tools/testing/selftests/kvm/aarch64/arch_timer.c b/tools/testing/selftests/kvm/aarch64/arch_timer.c > > new file mode 100644 > > index 000000000000..1383f33850e9 > > --- /dev/null > > +++ b/tools/testing/selftests/kvm/aarch64/arch_timer.c > > @@ -0,0 +1,351 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * arch_timer.c - Tests the aarch64 timer IRQ functionality > > + * > > + * The test validates both the virtual and physical timer IRQs using > > + * CVAL and TVAL registers. This consitutes the four stages in the test. > > + * The guest's main thread configures the timer interrupt for a stage > > + * and waits for it to fire, with a timeout equal to the timer period. > > + * It asserts that the timeout doesn't exceed the timer period. > > + * > > + * On the other hand, upon receipt of an interrupt, the guest's interrupt > > + * handler validates the interrupt by checking if the architectural state > > + * is in compliance with the specifications. > > + * > > + * The test provides command-line options to configure the timer's > > + * period (-p), number of vCPUs (-n), and iterations per stage (-i). > > + * > > + * Copyright (c) 2021, Google LLC. > > + */ > > + > > +#define _GNU_SOURCE > > + > > +#include > > +#include > > +#include > > +#include > > + > > +#include "kvm_util.h" > > +#include "processor.h" > > +#include "delay.h" > > +#include "arch_timer.h" > > +#include "gic.h" > > +#include "vgic.h" > > + > > +#define NR_VCPUS_DEF 4 > > +#define NR_TEST_ITERS_DEF 5 > > +#define TIMER_TEST_PERIOD_MS_DEF 10 > > +#define TIMER_TEST_ERR_MARGIN_US 100 > > + > > +struct test_args { > > + int nr_vcpus; > > + int nr_iter; > > + int timer_period_ms; > > +}; > > + > > +static struct test_args test_args = { > > + .nr_vcpus = NR_VCPUS_DEF, > > + .nr_iter = NR_TEST_ITERS_DEF, > > + .timer_period_ms = TIMER_TEST_PERIOD_MS_DEF, > > +}; > > + > > +#define msecs_to_usecs(msec) ((msec) * 1000LL) > > + > > +#define VTIMER_IRQ 27 > > +#define PTIMER_IRQ 30 > > + > > +#define GICD_BASE_GPA 0x8000000ULL > > +#define GICR_BASE_GPA 0x80A0000ULL > > + > > +enum guest_stage { > > + GUEST_STAGE_VTIMER_CVAL = 1, > > + GUEST_STAGE_VTIMER_TVAL, > > + GUEST_STAGE_PTIMER_CVAL, > > + GUEST_STAGE_PTIMER_TVAL, > > + GUEST_STAGE_MAX, > > +}; > > + > > +/* Sahred variables between host and guest */ > > Shared > > > +struct test_vcpu_shared_data { > > + int nr_iter; > > + enum guest_stage guest_stage; > > + uint64_t xcnt; > > +}; > > + > > +struct test_vcpu { > > + uint32_t vcpuid; > > + pthread_t pt_vcpu_run; > > + struct kvm_vm *vm; > > +}; > > + > > +static struct test_vcpu test_vcpu[KVM_MAX_VCPUS]; > > +static struct test_vcpu_shared_data vcpu_shared_data[KVM_MAX_VCPUS]; > > + > > +static void > > +guest_configure_timer_action(struct test_vcpu_shared_data *shared_data) > > +{ > > + switch (shared_data->guest_stage) { > > + case GUEST_STAGE_VTIMER_CVAL: > > + timer_set_next_cval_ms(VIRTUAL, test_args.timer_period_ms); > > + shared_data->xcnt = timer_get_cntct(VIRTUAL); > > + timer_set_ctl(VIRTUAL, CTL_ENABLE); > > + break; > > + case GUEST_STAGE_VTIMER_TVAL: > > + timer_set_next_tval_ms(VIRTUAL, test_args.timer_period_ms); > > + shared_data->xcnt = timer_get_cntct(VIRTUAL); > > + timer_set_ctl(VIRTUAL, CTL_ENABLE); > > + break; > > + case GUEST_STAGE_PTIMER_CVAL: > > + timer_set_next_cval_ms(PHYSICAL, test_args.timer_period_ms); > > + shared_data->xcnt = timer_get_cntct(PHYSICAL); > > + timer_set_ctl(PHYSICAL, CTL_ENABLE); > > + break; > > + case GUEST_STAGE_PTIMER_TVAL: > > + timer_set_next_tval_ms(PHYSICAL, test_args.timer_period_ms); > > + shared_data->xcnt = timer_get_cntct(PHYSICAL); > > + timer_set_ctl(PHYSICAL, CTL_ENABLE); > > + break; > > Since we divide the stages up for vtimer and ptimer, then I'm not sure we > need the wrapper fuctions for timer register get/set with the vtimer and > ptimer switches too. > I understand it's a little redundant. But like you saw in the arch_timer framework patch, the helper functions to program the cval, or tval, or any others to follow, can be made very slim. Thanks for catching the typos. Will fix them. Regards, Raghavendra > > + default: > > + GUEST_ASSERT(0); > > + } > > +} > > + > > +static void guest_validate_irq(unsigned int intid, > > + struct test_vcpu_shared_data *shared_data) > > +{ > > + enum guest_stage stage = shared_data->guest_stage; > > + uint64_t xcnt = 0, xcnt_diff_us, cval = 0; > > + unsigned long xctl = 0; > > + unsigned int timer_irq = 0; > > + > > + if (stage == GUEST_STAGE_VTIMER_CVAL || > > + stage == GUEST_STAGE_VTIMER_TVAL) { > > + xctl = timer_get_ctl(VIRTUAL); > > + timer_set_ctl(VIRTUAL, CTL_IMASK); > > + xcnt = timer_get_cntct(VIRTUAL); > > + cval = timer_get_cval(VIRTUAL); > > + timer_irq = VTIMER_IRQ; > > + } else if (stage == GUEST_STAGE_PTIMER_CVAL || > > + stage == GUEST_STAGE_PTIMER_TVAL) { > > + xctl = timer_get_ctl(PHYSICAL); > > + timer_set_ctl(PHYSICAL, CTL_IMASK); > > + xcnt = timer_get_cntct(PHYSICAL); > > + cval = timer_get_cval(PHYSICAL); > > + timer_irq = PTIMER_IRQ; > > + } else { > > + GUEST_ASSERT(0); > > + } > > + > > + xcnt_diff_us = cycles_to_usec(xcnt - shared_data->xcnt); > > + > > + /* Make sure we are dealing with the correct timer IRQ */ > > + GUEST_ASSERT_2(intid == timer_irq, intid, timer_irq); > > + > > + /* Basic 'timer codition met' check */ > > condition > > > + GUEST_ASSERT_3(xcnt >= cval, xcnt, cval, xcnt_diff_us); > > + GUEST_ASSERT_1(xctl & CTL_ISTATUS, xctl); > > +} > > + > > +static void guest_irq_handler(struct ex_regs *regs) > > +{ > > + unsigned int intid = gic_get_and_ack_irq(); > > + uint32_t cpu = get_vcpuid(); > > + struct test_vcpu_shared_data *shared_data = &vcpu_shared_data[cpu]; > > + > > + guest_validate_irq(intid, shared_data); > > + > > + WRITE_ONCE(shared_data->nr_iter, shared_data->nr_iter + 1); > > + > > + gic_set_eoi(intid); > > +} > > + > > +static void guest_run_stage(struct test_vcpu_shared_data *shared_data, > > + enum guest_stage stage) > > +{ > > + uint32_t irq_iter, config_iter; > > + > > + shared_data->guest_stage = stage; > > + shared_data->nr_iter = 0; > > + > > + for (config_iter = 0; config_iter < test_args.nr_iter; config_iter++) { > > + /* Setup the next interrupt */ > > + guest_configure_timer_action(shared_data); > > + > > + /* Setup a timeout for the interrupt to arrive */ > > + udelay(msecs_to_usecs(test_args.timer_period_ms) + > > + TIMER_TEST_ERR_MARGIN_US); > > + > > + irq_iter = READ_ONCE(shared_data->nr_iter); > > + GUEST_ASSERT_2(config_iter + 1 == irq_iter, > > + config_iter + 1, irq_iter); > > + }; > > extra ; > > > +} > > + > > +static void guest_code(void) > > +{ > > + uint32_t cpu = get_vcpuid(); > > + struct test_vcpu_shared_data *shared_data = &vcpu_shared_data[cpu]; > > + > > + local_irq_disable(); > > + > > + gic_init(GIC_V3, test_args.nr_vcpus, > > + (void *)GICD_BASE_GPA, (void *)GICR_BASE_GPA); > > + > > + timer_set_ctl(VIRTUAL, CTL_IMASK); > > + timer_set_ctl(PHYSICAL, CTL_IMASK); > > + > > + gic_irq_enable(VTIMER_IRQ); > > + gic_irq_enable(PTIMER_IRQ); > > + local_irq_enable(); > > + > > + guest_run_stage(shared_data, GUEST_STAGE_VTIMER_CVAL); > > + guest_run_stage(shared_data, GUEST_STAGE_VTIMER_TVAL); > > + guest_run_stage(shared_data, GUEST_STAGE_PTIMER_CVAL); > > + guest_run_stage(shared_data, GUEST_STAGE_PTIMER_TVAL); > > + > > + GUEST_DONE(); > > +} > > + > > +static void *test_vcpu_run(void *arg) > > +{ > > + struct ucall uc; > > + struct test_vcpu *vcpu = arg; > > + struct kvm_vm *vm = vcpu->vm; > > + uint32_t vcpuid = vcpu->vcpuid; > > + struct test_vcpu_shared_data *shared_data = &vcpu_shared_data[vcpuid]; > > + > > + vcpu_run(vm, vcpuid); > > + > > + switch (get_ucall(vm, vcpuid, &uc)) { > > + case UCALL_SYNC: > > + case UCALL_DONE: > > + break; > > + case UCALL_ABORT: > > + sync_global_from_guest(vm, *shared_data); > > + TEST_ASSERT(false, > > TEST_FAIL(fmt, ...) can be used. > > > + "%s at %s:%ld\n\tvalues: %lu, %lu; %lu, vcpu: %u; stage: %u; iter: %u", > > + (const char *)uc.args[0], __FILE__, uc.args[1], > > + uc.args[2], uc.args[3], uc.args[4], vcpuid, > > + shared_data->guest_stage, shared_data->nr_iter); > > + break; > > + default: > > + TEST_FAIL("Unexpected guest exit\n"); > > + } > > + > > + return NULL; > > +} > > + > > +static void test_run(struct kvm_vm *vm) > > +{ > > + int i, ret; > > + > > + for (i = 0; i < test_args.nr_vcpus; i++) { > > + ret = pthread_create(&test_vcpu[i].pt_vcpu_run, NULL, > > + test_vcpu_run, &test_vcpu[i]); > > + TEST_ASSERT(!ret, "Failed to create vCPU-%d pthread\n", i); > > + } > > + > > + for (i = 0; i < test_args.nr_vcpus; i++) > > + pthread_join(test_vcpu[i].pt_vcpu_run, NULL); > > +} > > + > > +static struct kvm_vm *test_vm_create(void) > > +{ > > + struct kvm_vm *vm; > > + unsigned int i; > > + int nr_vcpus = test_args.nr_vcpus; > > + > > + vm = vm_create_default_with_vcpus(nr_vcpus, 0, 0, guest_code, NULL); > > + > > + vm_init_descriptor_tables(vm); > > + vm_install_exception_handler(vm, VECTOR_IRQ_CURRENT, guest_irq_handler); > > + > > + for (i = 0; i < nr_vcpus; i++) { > > + vcpu_init_descriptor_tables(vm, i); > > + > > + test_vcpu[i].vcpuid = i; > > + test_vcpu[i].vm = vm; > > + } > > + > > + ucall_init(vm, NULL); > > + vgic_v3_setup(vm, nr_vcpus, GICD_BASE_GPA, GICR_BASE_GPA, 1); > > + > > + /* Make all the test's cmdline args visible to the guest */ > > + sync_global_to_guest(vm, test_args); > > + > > + return vm; > > +} > > + > > +static void test_print_help(char *name) > > +{ > > + pr_info("Usage: %s [-h] [-n nr_vcpus] [-i iterations] [-p timer_period_ms]\n", > > + name); > > + pr_info("\t-n: Number of vCPUs to configure (default: %u; max: %u)\n", > > + NR_VCPUS_DEF, KVM_MAX_VCPUS); > > + pr_info("\t-i: Number of iterations per stage (default: %u)\n", > > + NR_TEST_ITERS_DEF); > > + pr_info("\t-p: Periodicity (in ms) of the guest timer (default: %u)\n", > > + TIMER_TEST_PERIOD_MS_DEF); > > + pr_info("\t-h: print this help screen\n"); > > +} > > + > > +static bool parse_args(int argc, char *argv[]) > > +{ > > + int opt; > > + > > + while ((opt = getopt(argc, argv, "hn:i:p:")) != -1) { > > + switch (opt) { > > + case 'n': > > + test_args.nr_vcpus = atoi(optarg); > > + if (test_args.nr_vcpus <= 0) { > > + pr_info("Positive value needed for -n\n"); > > + goto err; > > + } else if (test_args.nr_vcpus > KVM_MAX_VCPUS) { > > + pr_info("Max allowed vCPUs: %u\n", > > + KVM_MAX_VCPUS); > > + goto err; > > + } > > + break; > > + case 'i': > > + test_args.nr_iter = atoi(optarg); > > + if (test_args.nr_iter <= 0) { > > + pr_info("Positive value needed for -i\n"); > > + goto err; > > + } > > + break; > > + case 'p': > > + test_args.timer_period_ms = atoi(optarg); > > + if (test_args.timer_period_ms <= 0) { > > + pr_info("Positive value needed for -p\n"); > > + goto err; > > + } > > + break; > > + case 'h': > > + default: > > + goto err; > > + } > > + } > > + > > + return true; > > + > > +err: > > + test_print_help(argv[0]); > > + return false; > > +} > > + > > +int main(int argc, char *argv[]) > > +{ > > + struct kvm_vm *vm; > > + > > + /* Tell stdout not to buffer its content */ > > + setbuf(stdout, NULL); > > + > > + if (!parse_args(argc, argv)) > > + exit(KSFT_SKIP); > > + > > + vm = test_vm_create(); > > + test_run(vm); > > + kvm_vm_free(vm); > > + > > + return 0; > > +} > > -- > > 2.33.0.153.gba50c8fa24-goog > > Besides the nits, > > Reviewed-by: Andrew Jones > > Thanks, > drew > _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-14.1 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_ADSP_CUSTOM_MED,DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E05B1C433EF for ; Fri, 3 Sep 2021 20:44:20 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id AAFB4610D2 for ; Fri, 3 Sep 2021 20:44:20 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org AAFB4610D2 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Cc:To:Subject:Message-ID:Date:From: In-Reply-To:References:MIME-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=jSnZAzjKE8WCq0GGH7JzFBNDqaPUppvy7BIjTRfw1Pg=; b=WUevNJA8FLWKqA V9l6355A1w+Ic7xv0NIgSny+LCPruGComtBnIEcZybdm+ns2HdQsBY2pn3Mj0/WTBznIHMSxuAcrB mBAgUkryDeKa7eyUYarWamGBIcljN2ueuHAO2x48YMfcPy9jxrdKCgfxMbCrYZqqUF42Gdxvi5lVg VZ9SSoohaDfoRY6TcShO4TJjn9BrKJr+40QX4H5LA4iuQci7UoVB0nqGG1EQ1QLjhFGUyO3/IB69Y Y3BZBHC90EOA6mqyHRLKa0B46YIXHQgcQKp8ychvgySmk59tE2JSpjB3+TuKURp0ShqdIJZdIFmxb FxK1AUsX4ibixmtJfpNw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mMG16-00CvER-26; Fri, 03 Sep 2021 20:42:28 +0000 Received: from mail-yb1-xb2f.google.com ([2607:f8b0:4864:20::b2f]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mMG10-00CvDH-T9 for linux-arm-kernel@lists.infradead.org; Fri, 03 Sep 2021 20:42:25 +0000 Received: by mail-yb1-xb2f.google.com with SMTP id v17so744601ybs.9 for ; Fri, 03 Sep 2021 13:42:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=xuC1AbQeewFRo/xFFqcL88Ah+7CpgJ5CaVM6UZ6mtTI=; b=jM60ipIM3egn5pQ+GdkesL8xzx68ExyAKWHEiBTtmGEKVkhU5ELoPgYxRdVsTDKCEU 6u1/Hhv3qdofh57ovdeLp0PBJq3/ccJC8RrajCa0wjhuwEUdWl/2YqgwPLQrODcGJGIj bbnZs9KUiFrGZ4FKcNl/vToO7Z8Kf4cbLMDsJlHWUSYPWSWy/2iwtVG0WrEMTkmcrHAK LtyQkK0XNOK/Hf4fal2UGoJzxeLpfX+e0FaUVEjtWrarqQle35I+bLgIeX3OglM5bDo4 uSfu6rleePJB6h2EMQPXvGc0kyEjSHGFdvUNn5/9HpFMUk8TsL/x83yE/uCCt/3d7xgx NmRw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=xuC1AbQeewFRo/xFFqcL88Ah+7CpgJ5CaVM6UZ6mtTI=; b=B7wDNwoTWzdIrKZQnWy7S6cbNTkZyyHN5Jg62rS7j7yO0rbd/r7CZl4SoBeDw+mx+t klShFO0i0CAh/pdhX2UlLg+CxJPm9o8fj8Y0Poxjr8S7M+UDq+eFJLkKeVwEfFxxC3KS fRTQWSQY0DtVT4bse0FVSCSn9pCb3K/7y995yJdPXlv2JRuQUGxpGUiGBVSlWqrfs5tH +odDhwfu+x//hLEF9IPu2/ozILy58w8v004AkbA3qy7SUtkGGuKPBWT0Qxcv9xJHbCHQ YJxfVQaila2h2N+B9mAaevcQTOTbmxcAIBwoYNQ6WLEBRie4+qZQqxOSsHftpTTXV/BQ dGqQ== X-Gm-Message-State: AOAM532VmuoPG5uNafIA3xjy8gAlJRvTkv9B7avvUyPWHDDY1t+ZVrYM xLAX9syvpiWxELYfas5r5ZvJ8EnPEQS80xYC3VUE/A== X-Google-Smtp-Source: ABdhPJyHg7Q6U+LEI9cT8RqvbrXGuG61A+G+I9rNAlLOGZo9V713C/qUl4kUtsr4QUdE6efN1jSpFLbnaBJoUQjOKmQ= X-Received: by 2002:a25:188b:: with SMTP id 133mr1222109yby.80.1630701738352; Fri, 03 Sep 2021 13:42:18 -0700 (PDT) MIME-Version: 1.0 References: <20210901211412.4171835-1-rananta@google.com> <20210901211412.4171835-12-rananta@google.com> <20210903104823.ih4aj34vrbhlfhy3@gator.home> In-Reply-To: <20210903104823.ih4aj34vrbhlfhy3@gator.home> From: Raghavendra Rao Ananta Date: Fri, 3 Sep 2021 13:42:05 -0700 Message-ID: Subject: Re: [PATCH v3 11/12] KVM: arm64: selftests: Add arch_timer test To: Andrew Jones Cc: Paolo Bonzini , Marc Zyngier , James Morse , Alexandru Elisei , Suzuki K Poulose , kvm@vger.kernel.org, Catalin Marinas , Peter Shier , linux-kernel@vger.kernel.org, Will Deacon , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210903_134223_015206_96C98B26 X-CRM114-Status: GOOD ( 39.32 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, Sep 3, 2021 at 3:48 AM Andrew Jones wrote: > > On Wed, Sep 01, 2021 at 09:14:11PM +0000, Raghavendra Rao Ananta wrote: > > Add a KVM selftest to validate the arch_timer functionality. > > Primarily, the test sets up periodic timer interrupts and > > validates the basic architectural expectations upon its receipt. > > > > The test provides command-line options to configure the period > > of the timer, number of iterations, and number of vCPUs. > > > > Signed-off-by: Raghavendra Rao Ananta > > --- > > tools/testing/selftests/kvm/.gitignore | 1 + > > tools/testing/selftests/kvm/Makefile | 1 + > > .../selftests/kvm/aarch64/arch_timer.c | 351 ++++++++++++++++++ > > 3 files changed, 353 insertions(+) > > create mode 100644 tools/testing/selftests/kvm/aarch64/arch_timer.c > > > > diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore > > index 98053d3afbda..c6058df0cd18 100644 > > --- a/tools/testing/selftests/kvm/.gitignore > > +++ b/tools/testing/selftests/kvm/.gitignore > > @@ -1,4 +1,5 @@ > > # SPDX-License-Identifier: GPL-2.0-only > > +/aarch64/arch_timer > > /aarch64/debug-exceptions > > /aarch64/get-reg-list > > /aarch64/psci_cpu_on_test > > diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile > > index 8342f65c1d96..46d43e706b20 100644 > > --- a/tools/testing/selftests/kvm/Makefile > > +++ b/tools/testing/selftests/kvm/Makefile > > @@ -84,6 +84,7 @@ TEST_GEN_PROGS_x86_64 += set_memory_region_test > > TEST_GEN_PROGS_x86_64 += steal_time > > TEST_GEN_PROGS_x86_64 += kvm_binary_stats_test > > > > +TEST_GEN_PROGS_aarch64 += aarch64/arch_timer > > TEST_GEN_PROGS_aarch64 += aarch64/debug-exceptions > > TEST_GEN_PROGS_aarch64 += aarch64/get-reg-list > > TEST_GEN_PROGS_aarch64 += aarch64/psci_cpu_on_test > > diff --git a/tools/testing/selftests/kvm/aarch64/arch_timer.c b/tools/testing/selftests/kvm/aarch64/arch_timer.c > > new file mode 100644 > > index 000000000000..1383f33850e9 > > --- /dev/null > > +++ b/tools/testing/selftests/kvm/aarch64/arch_timer.c > > @@ -0,0 +1,351 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * arch_timer.c - Tests the aarch64 timer IRQ functionality > > + * > > + * The test validates both the virtual and physical timer IRQs using > > + * CVAL and TVAL registers. This consitutes the four stages in the test. > > + * The guest's main thread configures the timer interrupt for a stage > > + * and waits for it to fire, with a timeout equal to the timer period. > > + * It asserts that the timeout doesn't exceed the timer period. > > + * > > + * On the other hand, upon receipt of an interrupt, the guest's interrupt > > + * handler validates the interrupt by checking if the architectural state > > + * is in compliance with the specifications. > > + * > > + * The test provides command-line options to configure the timer's > > + * period (-p), number of vCPUs (-n), and iterations per stage (-i). > > + * > > + * Copyright (c) 2021, Google LLC. > > + */ > > + > > +#define _GNU_SOURCE > > + > > +#include > > +#include > > +#include > > +#include > > + > > +#include "kvm_util.h" > > +#include "processor.h" > > +#include "delay.h" > > +#include "arch_timer.h" > > +#include "gic.h" > > +#include "vgic.h" > > + > > +#define NR_VCPUS_DEF 4 > > +#define NR_TEST_ITERS_DEF 5 > > +#define TIMER_TEST_PERIOD_MS_DEF 10 > > +#define TIMER_TEST_ERR_MARGIN_US 100 > > + > > +struct test_args { > > + int nr_vcpus; > > + int nr_iter; > > + int timer_period_ms; > > +}; > > + > > +static struct test_args test_args = { > > + .nr_vcpus = NR_VCPUS_DEF, > > + .nr_iter = NR_TEST_ITERS_DEF, > > + .timer_period_ms = TIMER_TEST_PERIOD_MS_DEF, > > +}; > > + > > +#define msecs_to_usecs(msec) ((msec) * 1000LL) > > + > > +#define VTIMER_IRQ 27 > > +#define PTIMER_IRQ 30 > > + > > +#define GICD_BASE_GPA 0x8000000ULL > > +#define GICR_BASE_GPA 0x80A0000ULL > > + > > +enum guest_stage { > > + GUEST_STAGE_VTIMER_CVAL = 1, > > + GUEST_STAGE_VTIMER_TVAL, > > + GUEST_STAGE_PTIMER_CVAL, > > + GUEST_STAGE_PTIMER_TVAL, > > + GUEST_STAGE_MAX, > > +}; > > + > > +/* Sahred variables between host and guest */ > > Shared > > > +struct test_vcpu_shared_data { > > + int nr_iter; > > + enum guest_stage guest_stage; > > + uint64_t xcnt; > > +}; > > + > > +struct test_vcpu { > > + uint32_t vcpuid; > > + pthread_t pt_vcpu_run; > > + struct kvm_vm *vm; > > +}; > > + > > +static struct test_vcpu test_vcpu[KVM_MAX_VCPUS]; > > +static struct test_vcpu_shared_data vcpu_shared_data[KVM_MAX_VCPUS]; > > + > > +static void > > +guest_configure_timer_action(struct test_vcpu_shared_data *shared_data) > > +{ > > + switch (shared_data->guest_stage) { > > + case GUEST_STAGE_VTIMER_CVAL: > > + timer_set_next_cval_ms(VIRTUAL, test_args.timer_period_ms); > > + shared_data->xcnt = timer_get_cntct(VIRTUAL); > > + timer_set_ctl(VIRTUAL, CTL_ENABLE); > > + break; > > + case GUEST_STAGE_VTIMER_TVAL: > > + timer_set_next_tval_ms(VIRTUAL, test_args.timer_period_ms); > > + shared_data->xcnt = timer_get_cntct(VIRTUAL); > > + timer_set_ctl(VIRTUAL, CTL_ENABLE); > > + break; > > + case GUEST_STAGE_PTIMER_CVAL: > > + timer_set_next_cval_ms(PHYSICAL, test_args.timer_period_ms); > > + shared_data->xcnt = timer_get_cntct(PHYSICAL); > > + timer_set_ctl(PHYSICAL, CTL_ENABLE); > > + break; > > + case GUEST_STAGE_PTIMER_TVAL: > > + timer_set_next_tval_ms(PHYSICAL, test_args.timer_period_ms); > > + shared_data->xcnt = timer_get_cntct(PHYSICAL); > > + timer_set_ctl(PHYSICAL, CTL_ENABLE); > > + break; > > Since we divide the stages up for vtimer and ptimer, then I'm not sure we > need the wrapper fuctions for timer register get/set with the vtimer and > ptimer switches too. > I understand it's a little redundant. But like you saw in the arch_timer framework patch, the helper functions to program the cval, or tval, or any others to follow, can be made very slim. Thanks for catching the typos. Will fix them. Regards, Raghavendra > > + default: > > + GUEST_ASSERT(0); > > + } > > +} > > + > > +static void guest_validate_irq(unsigned int intid, > > + struct test_vcpu_shared_data *shared_data) > > +{ > > + enum guest_stage stage = shared_data->guest_stage; > > + uint64_t xcnt = 0, xcnt_diff_us, cval = 0; > > + unsigned long xctl = 0; > > + unsigned int timer_irq = 0; > > + > > + if (stage == GUEST_STAGE_VTIMER_CVAL || > > + stage == GUEST_STAGE_VTIMER_TVAL) { > > + xctl = timer_get_ctl(VIRTUAL); > > + timer_set_ctl(VIRTUAL, CTL_IMASK); > > + xcnt = timer_get_cntct(VIRTUAL); > > + cval = timer_get_cval(VIRTUAL); > > + timer_irq = VTIMER_IRQ; > > + } else if (stage == GUEST_STAGE_PTIMER_CVAL || > > + stage == GUEST_STAGE_PTIMER_TVAL) { > > + xctl = timer_get_ctl(PHYSICAL); > > + timer_set_ctl(PHYSICAL, CTL_IMASK); > > + xcnt = timer_get_cntct(PHYSICAL); > > + cval = timer_get_cval(PHYSICAL); > > + timer_irq = PTIMER_IRQ; > > + } else { > > + GUEST_ASSERT(0); > > + } > > + > > + xcnt_diff_us = cycles_to_usec(xcnt - shared_data->xcnt); > > + > > + /* Make sure we are dealing with the correct timer IRQ */ > > + GUEST_ASSERT_2(intid == timer_irq, intid, timer_irq); > > + > > + /* Basic 'timer codition met' check */ > > condition > > > + GUEST_ASSERT_3(xcnt >= cval, xcnt, cval, xcnt_diff_us); > > + GUEST_ASSERT_1(xctl & CTL_ISTATUS, xctl); > > +} > > + > > +static void guest_irq_handler(struct ex_regs *regs) > > +{ > > + unsigned int intid = gic_get_and_ack_irq(); > > + uint32_t cpu = get_vcpuid(); > > + struct test_vcpu_shared_data *shared_data = &vcpu_shared_data[cpu]; > > + > > + guest_validate_irq(intid, shared_data); > > + > > + WRITE_ONCE(shared_data->nr_iter, shared_data->nr_iter + 1); > > + > > + gic_set_eoi(intid); > > +} > > + > > +static void guest_run_stage(struct test_vcpu_shared_data *shared_data, > > + enum guest_stage stage) > > +{ > > + uint32_t irq_iter, config_iter; > > + > > + shared_data->guest_stage = stage; > > + shared_data->nr_iter = 0; > > + > > + for (config_iter = 0; config_iter < test_args.nr_iter; config_iter++) { > > + /* Setup the next interrupt */ > > + guest_configure_timer_action(shared_data); > > + > > + /* Setup a timeout for the interrupt to arrive */ > > + udelay(msecs_to_usecs(test_args.timer_period_ms) + > > + TIMER_TEST_ERR_MARGIN_US); > > + > > + irq_iter = READ_ONCE(shared_data->nr_iter); > > + GUEST_ASSERT_2(config_iter + 1 == irq_iter, > > + config_iter + 1, irq_iter); > > + }; > > extra ; > > > +} > > + > > +static void guest_code(void) > > +{ > > + uint32_t cpu = get_vcpuid(); > > + struct test_vcpu_shared_data *shared_data = &vcpu_shared_data[cpu]; > > + > > + local_irq_disable(); > > + > > + gic_init(GIC_V3, test_args.nr_vcpus, > > + (void *)GICD_BASE_GPA, (void *)GICR_BASE_GPA); > > + > > + timer_set_ctl(VIRTUAL, CTL_IMASK); > > + timer_set_ctl(PHYSICAL, CTL_IMASK); > > + > > + gic_irq_enable(VTIMER_IRQ); > > + gic_irq_enable(PTIMER_IRQ); > > + local_irq_enable(); > > + > > + guest_run_stage(shared_data, GUEST_STAGE_VTIMER_CVAL); > > + guest_run_stage(shared_data, GUEST_STAGE_VTIMER_TVAL); > > + guest_run_stage(shared_data, GUEST_STAGE_PTIMER_CVAL); > > + guest_run_stage(shared_data, GUEST_STAGE_PTIMER_TVAL); > > + > > + GUEST_DONE(); > > +} > > + > > +static void *test_vcpu_run(void *arg) > > +{ > > + struct ucall uc; > > + struct test_vcpu *vcpu = arg; > > + struct kvm_vm *vm = vcpu->vm; > > + uint32_t vcpuid = vcpu->vcpuid; > > + struct test_vcpu_shared_data *shared_data = &vcpu_shared_data[vcpuid]; > > + > > + vcpu_run(vm, vcpuid); > > + > > + switch (get_ucall(vm, vcpuid, &uc)) { > > + case UCALL_SYNC: > > + case UCALL_DONE: > > + break; > > + case UCALL_ABORT: > > + sync_global_from_guest(vm, *shared_data); > > + TEST_ASSERT(false, > > TEST_FAIL(fmt, ...) can be used. > > > + "%s at %s:%ld\n\tvalues: %lu, %lu; %lu, vcpu: %u; stage: %u; iter: %u", > > + (const char *)uc.args[0], __FILE__, uc.args[1], > > + uc.args[2], uc.args[3], uc.args[4], vcpuid, > > + shared_data->guest_stage, shared_data->nr_iter); > > + break; > > + default: > > + TEST_FAIL("Unexpected guest exit\n"); > > + } > > + > > + return NULL; > > +} > > + > > +static void test_run(struct kvm_vm *vm) > > +{ > > + int i, ret; > > + > > + for (i = 0; i < test_args.nr_vcpus; i++) { > > + ret = pthread_create(&test_vcpu[i].pt_vcpu_run, NULL, > > + test_vcpu_run, &test_vcpu[i]); > > + TEST_ASSERT(!ret, "Failed to create vCPU-%d pthread\n", i); > > + } > > + > > + for (i = 0; i < test_args.nr_vcpus; i++) > > + pthread_join(test_vcpu[i].pt_vcpu_run, NULL); > > +} > > + > > +static struct kvm_vm *test_vm_create(void) > > +{ > > + struct kvm_vm *vm; > > + unsigned int i; > > + int nr_vcpus = test_args.nr_vcpus; > > + > > + vm = vm_create_default_with_vcpus(nr_vcpus, 0, 0, guest_code, NULL); > > + > > + vm_init_descriptor_tables(vm); > > + vm_install_exception_handler(vm, VECTOR_IRQ_CURRENT, guest_irq_handler); > > + > > + for (i = 0; i < nr_vcpus; i++) { > > + vcpu_init_descriptor_tables(vm, i); > > + > > + test_vcpu[i].vcpuid = i; > > + test_vcpu[i].vm = vm; > > + } > > + > > + ucall_init(vm, NULL); > > + vgic_v3_setup(vm, nr_vcpus, GICD_BASE_GPA, GICR_BASE_GPA, 1); > > + > > + /* Make all the test's cmdline args visible to the guest */ > > + sync_global_to_guest(vm, test_args); > > + > > + return vm; > > +} > > + > > +static void test_print_help(char *name) > > +{ > > + pr_info("Usage: %s [-h] [-n nr_vcpus] [-i iterations] [-p timer_period_ms]\n", > > + name); > > + pr_info("\t-n: Number of vCPUs to configure (default: %u; max: %u)\n", > > + NR_VCPUS_DEF, KVM_MAX_VCPUS); > > + pr_info("\t-i: Number of iterations per stage (default: %u)\n", > > + NR_TEST_ITERS_DEF); > > + pr_info("\t-p: Periodicity (in ms) of the guest timer (default: %u)\n", > > + TIMER_TEST_PERIOD_MS_DEF); > > + pr_info("\t-h: print this help screen\n"); > > +} > > + > > +static bool parse_args(int argc, char *argv[]) > > +{ > > + int opt; > > + > > + while ((opt = getopt(argc, argv, "hn:i:p:")) != -1) { > > + switch (opt) { > > + case 'n': > > + test_args.nr_vcpus = atoi(optarg); > > + if (test_args.nr_vcpus <= 0) { > > + pr_info("Positive value needed for -n\n"); > > + goto err; > > + } else if (test_args.nr_vcpus > KVM_MAX_VCPUS) { > > + pr_info("Max allowed vCPUs: %u\n", > > + KVM_MAX_VCPUS); > > + goto err; > > + } > > + break; > > + case 'i': > > + test_args.nr_iter = atoi(optarg); > > + if (test_args.nr_iter <= 0) { > > + pr_info("Positive value needed for -i\n"); > > + goto err; > > + } > > + break; > > + case 'p': > > + test_args.timer_period_ms = atoi(optarg); > > + if (test_args.timer_period_ms <= 0) { > > + pr_info("Positive value needed for -p\n"); > > + goto err; > > + } > > + break; > > + case 'h': > > + default: > > + goto err; > > + } > > + } > > + > > + return true; > > + > > +err: > > + test_print_help(argv[0]); > > + return false; > > +} > > + > > +int main(int argc, char *argv[]) > > +{ > > + struct kvm_vm *vm; > > + > > + /* Tell stdout not to buffer its content */ > > + setbuf(stdout, NULL); > > + > > + if (!parse_args(argc, argv)) > > + exit(KSFT_SKIP); > > + > > + vm = test_vm_create(); > > + test_run(vm); > > + kvm_vm_free(vm); > > + > > + return 0; > > +} > > -- > > 2.33.0.153.gba50c8fa24-goog > > Besides the nits, > > Reviewed-by: Andrew Jones > > Thanks, > drew > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel