[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9b4a8665-4fb8-42e7-b63f-d154ae75a4f0@os.amperecomputing.com>
Date: Fri, 7 Feb 2025 18:56:41 +0530
From: Ganapatrao Kulkarni <gankulkarni@...amperecomputing.com>
To: Marc Zyngier <maz@...nel.org>
Cc: kvm@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
kvmarm@...ts.linux.dev, linux-kernel@...r.kernel.org,
linux-kselftest@...r.kernel.org, oliver.upton@...ux.dev, joey.gouly@....com,
suzuki.poulose@....com, darren@...amperecomputing.com,
scott@...amperecomputing.com
Subject: Re: [RFC PATCH 1/2] KVM: arm64: nv: selftests: Add guest hypervisor
test
Hi Marc,
On 07-02-2025 02:44 am, Marc Zyngier wrote:
> On Thu, 06 Feb 2025 16:41:19 +0000,
> Ganapatrao Kulkarni <gankulkarni@...amperecomputing.com> wrote:
>>
>> This patch adds the required changes to init vcpu in vEL2 context.
>> Also adds a KVM selftest to execute guest code as a guest hypervisor(L1).
>>
>> Signed-off-by: Ganapatrao Kulkarni <gankulkarni@...amperecomputing.com>
>> ---
>> tools/testing/selftests/kvm/Makefile.kvm | 1 +
>> .../selftests/kvm/arm64/nv_guest_hypervisor.c | 83 +++++++++++++++++++
>> .../kvm/include/arm64/kvm_util_arch.h | 3 +
>> .../selftests/kvm/include/arm64/nv_util.h | 28 +++++++
>> .../testing/selftests/kvm/include/kvm_util.h | 1 +
>> .../selftests/kvm/lib/arm64/processor.c | 59 +++++++++----
>> 6 files changed, 161 insertions(+), 14 deletions(-)
>> create mode 100644 tools/testing/selftests/kvm/arm64/nv_guest_hypervisor.c
>> create mode 100644 tools/testing/selftests/kvm/include/arm64/nv_util.h
>>
>> diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm
>> index 4277b983cace..a85d3bec9fb1 100644
>> --- a/tools/testing/selftests/kvm/Makefile.kvm
>> +++ b/tools/testing/selftests/kvm/Makefile.kvm
>> @@ -154,6 +154,7 @@ TEST_GEN_PROGS_arm64 += arm64/vgic_irq
>> TEST_GEN_PROGS_arm64 += arm64/vgic_lpi_stress
>> TEST_GEN_PROGS_arm64 += arm64/vpmu_counter_access
>> TEST_GEN_PROGS_arm64 += arm64/no-vgic-v3
>> +TEST_GEN_PROGS_arm64 += arm64/nv_guest_hypervisor
>> TEST_GEN_PROGS_arm64 += access_tracking_perf_test
>> TEST_GEN_PROGS_arm64 += arch_timer
>> TEST_GEN_PROGS_arm64 += coalesced_io_test
>> diff --git a/tools/testing/selftests/kvm/arm64/nv_guest_hypervisor.c b/tools/testing/selftests/kvm/arm64/nv_guest_hypervisor.c
>> new file mode 100644
>> index 000000000000..5aeefe43aff7
>> --- /dev/null
>> +++ b/tools/testing/selftests/kvm/arm64/nv_guest_hypervisor.c
>> @@ -0,0 +1,83 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (c) 2025 Ampere Computing LLC
>> + */
>> +#include <kvm_util.h>
>> +#include <nv_util.h>
>> +#include <processor.h>
>> +#include <vgic.h>
>> +
>> +static void guest_code(void)
>> +{
>> + if (read_sysreg(CurrentEL) == CurrentEL_EL2)
>> + GUEST_PRINTF("Executing guest code in vEL2\n");
>> + else
>> + GUEST_FAIL("Fail to run in vEL2\n");
>> +
>> + GUEST_DONE();
>> +}
>> +
>> +static void guest_undef_handler(struct ex_regs *regs)
>> +{
>> + GUEST_FAIL("Unexpected exception far_el1 = 0x%lx", read_sysreg(far_el1));
>> +}
>> +
>> +static void test_run_vcpu(struct kvm_vcpu *vcpu)
>> +{
>> + struct ucall uc;
>> +
>> + do {
>> + vcpu_run(vcpu);
>> +
>> + switch (get_ucall(vcpu, &uc)) {
>> + case UCALL_ABORT:
>> + REPORT_GUEST_ASSERT(uc);
>> + break;
>> + case UCALL_PRINTF:
>> + printf("%s", uc.buffer);
>> + break;
>> + case UCALL_DONE:
>> + printf("Test PASS\n");
>> + break;
>> + default:
>> + TEST_FAIL("Unknown ucall %lu", uc.cmd);
>> + }
>> + } while (uc.cmd != UCALL_DONE);
>> +}
>> +
>> +static void test_nv_guest_hypervisor(void)
>> +{
>> + struct kvm_vcpu *vcpu;
>> + struct kvm_vm *vm;
>> + struct kvm_vcpu_init init;
>> + int gic_fd;
>> +
>> + vm = vm_create(1);
>> + vm_ioctl(vm, KVM_ARM_PREFERRED_TARGET, &init);
>> +
>> + init.features[0] = 0;
>> + init_vcpu_nested(&init);
>> + vcpu = aarch64_vcpu_add(vm, 0, &init, guest_code);
>> +
>> + __TEST_REQUIRE(is_vcpu_nested(vcpu), "Failed to Enable NV");
>> +
>> + vm_init_descriptor_tables(vm);
>> + vcpu_init_descriptor_tables(vcpu);
>> + gic_fd = vgic_v3_setup(vm, 1, 64);
>> + __TEST_REQUIRE(gic_fd >= 0, "Failed to create vgic-v3");
>> +
>> + vm_install_sync_handler(vm, VECTOR_SYNC_CURRENT,
>> + ESR_ELx_EC_UNKNOWN, guest_undef_handler);
>> +
>> + test_run_vcpu(vcpu);
>> + kvm_vm_free(vm);
>> +}
>> +
>> +int main(int argc, char *argv[])
>> +{
>> + TEST_REQUIRE(kvm_has_cap(KVM_CAP_ARM_EL2));
>> +
>> + test_nv_guest_hypervisor();
>> +
>> + return 0;
>> +}
>> diff --git a/tools/testing/selftests/kvm/include/arm64/kvm_util_arch.h b/tools/testing/selftests/kvm/include/arm64/kvm_util_arch.h
>> index e43a57d99b56..ab5279c24413 100644
>> --- a/tools/testing/selftests/kvm/include/arm64/kvm_util_arch.h
>> +++ b/tools/testing/selftests/kvm/include/arm64/kvm_util_arch.h
>> @@ -2,6 +2,9 @@
>> #ifndef SELFTEST_KVM_UTIL_ARCH_H
>> #define SELFTEST_KVM_UTIL_ARCH_H
>>
>> +#define CurrentEL_EL1 (1 << 2)
>> +#define CurrentEL_EL2 (2 << 2)
>> +
>> struct kvm_vm_arch {};
>>
>> #endif // SELFTEST_KVM_UTIL_ARCH_H
>> diff --git a/tools/testing/selftests/kvm/include/arm64/nv_util.h b/tools/testing/selftests/kvm/include/arm64/nv_util.h
>> new file mode 100644
>> index 000000000000..4fecf1f18554
>> --- /dev/null
>> +++ b/tools/testing/selftests/kvm/include/arm64/nv_util.h
>> @@ -0,0 +1,28 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * Copyright (c) 2025 Ampere Computing
>> + */
>> +#ifndef SELFTEST_NV_UTIL_H
>> +#define SELFTEST_NV_UTIL_H
>> +
>> +#include <linux/bitmap.h>
>> +
>> +/* NV helpers */
>> +static inline void init_vcpu_nested(struct kvm_vcpu_init *init)
>> +{
>> + init->features[0] |= (1 << KVM_ARM_VCPU_HAS_EL2);
>> +}
>> +
>> +static inline bool kvm_arm_vcpu_has_el2(struct kvm_vcpu_init *init)
>> +{
>> + unsigned long features = init->features[0];
>> +
>> + return test_bit(KVM_ARM_VCPU_HAS_EL2, &features);
>> +}
>> +
>> +static inline bool is_vcpu_nested(struct kvm_vcpu *vcpu)
>> +{
>> + return vcpu->nested;
>> +}
>> +
>> +#endif /* SELFTEST_NV_UTIL_H */
>> diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
>> index 4c4e5a847f67..8c53dbc17f8f 100644
>> --- a/tools/testing/selftests/kvm/include/kvm_util.h
>> +++ b/tools/testing/selftests/kvm/include/kvm_util.h
>> @@ -58,6 +58,7 @@ struct kvm_vcpu {
>> struct kvm_dirty_gfn *dirty_gfns;
>> uint32_t fetch_index;
>> uint32_t dirty_gfns_count;
>> + bool nested;
>> };
>>
>> struct userspace_mem_regions {
>> diff --git a/tools/testing/selftests/kvm/lib/arm64/processor.c b/tools/testing/selftests/kvm/lib/arm64/processor.c
>> index 7ba3aa3755f3..35ba2ace61a2 100644
>> --- a/tools/testing/selftests/kvm/lib/arm64/processor.c
>> +++ b/tools/testing/selftests/kvm/lib/arm64/processor.c
>> @@ -10,6 +10,7 @@
>>
>> #include "guest_modes.h"
>> #include "kvm_util.h"
>> +#include "nv_util.h"
>> #include "processor.h"
>> #include "ucall_common.h"
>>
>> @@ -258,14 +259,47 @@ void virt_arch_dump(FILE *stream, struct kvm_vm *vm, uint8_t indent)
>> }
>> }
>>
>> +static void aarch64_vcpu_set_reg(struct kvm_vcpu *vcpu, uint64_t sctlr_el1,
>> + uint64_t tcr_el1, uint64_t ttbr0_el1)
>> +{
>> + uint64_t fpen;
>> +
>> + /*
>> + * Enable FP/ASIMD to avoid trapping when accessing Q0-Q15
>> + * registers, which the variable argument list macros do.
>> + */
>> + fpen = 3 << 20;
>> +
>> + if (is_vcpu_nested(vcpu)) {
>> + vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(SYS_CPTR_EL2), fpen);
>> + vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(SYS_SCTLR_EL2), sctlr_el1);
>> + vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(SYS_TCR_EL2), tcr_el1);
>> + vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(SYS_MAIR_EL2), DEFAULT_MAIR_EL1);
>> + vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(SYS_TTBR0_EL2), ttbr0_el1);
>> + vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(SYS_TPIDR_EL2), vcpu->id);
>
> How about some of the basics such as HCR_EL2, MDCR_EL2? A bunch of
> things there do have an impact on how the guest behaves, and relying
> on defaults feels like a bad idea.
Sure, I will try to have these registers also set to required value
explicitly.
>
> This also assumes VHE, without trying to enforce it.
Ok, I will try to set specific bits of HCR_EL2 to force it run in VHE.
>
> Finally, how to you plan to make all the existing tests run as EL2
> guests if TPIDR_EL1 isn't populated with the expected value? Surely
> you need to change the read side...
OK, I have not yet tried running existing tests modifying to run as EL2
guests. I will try to run them modifying to run in vEL2.
Should we modify them to run as EL2 guests by default, if the host
supports/detected NV? or command line argument based run? either in
El1(default) or in EL2?
BTW, I have also ran all existing tests on L1, most of the tests are
passing(atleast I did not see any failure prints).
arm64/debug-exceptions is failing on L1 and needs to be
debugged/fixed/skipped.
arm64/arch_timer_edge_cases fails on both L0 and L1.
--
Thanks,
Ganapat/GK
Powered by blists - more mailing lists