diff mbox series

[RFC,1/2] KVM: arm64: nv: selftests: Add guest hypervisor test

Message ID 20250206164120.4045569-2-gankulkarni@os.amperecomputing.com (mailing list archive)
State New
Headers show
Series Add NV Selftest cases | expand

Commit Message

Ganapatrao Kulkarni Feb. 6, 2025, 4:41 p.m. UTC
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@os.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

Comments

Marc Zyngier Feb. 6, 2025, 9:14 p.m. UTC | #1
On Thu, 06 Feb 2025 16:41:19 +0000,
Ganapatrao Kulkarni <gankulkarni@os.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@os.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.

This also assumes VHE, without trying to enforce it.

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...

	M.
Ganapatrao Kulkarni Feb. 7, 2025, 1:26 p.m. UTC | #2
Hi Marc,

On 07-02-2025 02:44 am, Marc Zyngier wrote:
> On Thu, 06 Feb 2025 16:41:19 +0000,
> Ganapatrao Kulkarni <gankulkarni@os.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@os.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.
Marc Zyngier Feb. 7, 2025, 1:59 p.m. UTC | #3
On Fri, 07 Feb 2025 13:26:41 +0000,
Ganapatrao Kulkarni <gankulkarni@os.amperecomputing.com> wrote:
> 
> >> +	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.

You shouldn't try to modify them. Just make them take a parameter so
that the initialisation is done by configuring everything at EL2.

> 
> 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?

EL1 by default.

> 
> 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.

Then I guess you have some work to do to debug these problems, and it
once more means that NV is not ready for merging.

	M.
Ganapatrao Kulkarni Feb. 7, 2025, 4:46 p.m. UTC | #4
On 07-02-2025 07:29 pm, Marc Zyngier wrote:
> On Fri, 07 Feb 2025 13:26:41 +0000,
> Ganapatrao Kulkarni <gankulkarni@os.amperecomputing.com> wrote:
>>
>>>> +	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.
> 
> You shouldn't try to modify them. Just make them take a parameter so
> that the initialisation is done by configuring everything at EL2.
> 

OK, make sense, thanks.
I will work on one of the test case and post it in the v2.
>>
>> 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?
> 
> EL1 by default.

OK.
> 
>>
>> 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.
> 
> Then I guess you have some work to do to debug these problems, and it
> once more means that NV is not ready for merging.

I will debug and share the findings/fix at the earliest.
Ganapatrao Kulkarni Feb. 19, 2025, 12:47 p.m. UTC | #5
On 07-02-2025 10:16 pm, Ganapatrao Kulkarni wrote:
> 
> 
> On 07-02-2025 07:29 pm, Marc Zyngier wrote:
>> On Fri, 07 Feb 2025 13:26:41 +0000,
>> Ganapatrao Kulkarni <gankulkarni@os.amperecomputing.com> wrote:
>>>
>>>>> +    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...
>>>

IIUC, we need not write to TPIDR_EL2, instead write always to TPIDR_EL1 
and in guest code(like function cpu_copy_el2regs in Linux kernel) copy 
TPIDR_EL1 value to TPIDR_EL2.

OR, Write to both TPIDR_EL1 and TPIDR_EL2 in the test code itself during 
vcpu setup.
diff mbox series

Patch

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);
+	} else {
+		vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(SYS_CPACR_EL1), fpen);
+		vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(SYS_SCTLR_EL1), sctlr_el1);
+		vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(SYS_TCR_EL1), tcr_el1);
+		vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(SYS_MAIR_EL1), DEFAULT_MAIR_EL1);
+		vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(SYS_TTBR0_EL1), ttbr0_el1);
+		vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(SYS_TPIDR_EL1), vcpu->id);
+}
+}
+
 void aarch64_vcpu_setup(struct kvm_vcpu *vcpu, struct kvm_vcpu_init *init)
 {
 	struct kvm_vcpu_init default_init = { .target = -1, };
 	struct kvm_vm *vm = vcpu->vm;
 	uint64_t sctlr_el1, tcr_el1, ttbr0_el1;
 
-	if (!init)
+	if (!init) {
 		init = &default_init;
+	} else {
+		/* Is this vcpu a Guest-Hypersior */
+		if (kvm_arm_vcpu_has_el2(init))
+			vcpu->nested = true;
+	}
 
 	if (init->target == -1) {
 		struct kvm_vcpu_init preferred;
@@ -275,12 +309,6 @@  void aarch64_vcpu_setup(struct kvm_vcpu *vcpu, struct kvm_vcpu_init *init)
 
 	vcpu_ioctl(vcpu, KVM_ARM_VCPU_INIT, init);
 
-	/*
-	 * Enable FP/ASIMD to avoid trapping when accessing Q0-Q15
-	 * registers, which the variable argument list macros do.
-	 */
-	vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(SYS_CPACR_EL1), 3 << 20);
-
 	sctlr_el1 = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_SCTLR_EL1));
 	tcr_el1 = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_TCR_EL1));
 
@@ -349,11 +377,7 @@  void aarch64_vcpu_setup(struct kvm_vcpu *vcpu, struct kvm_vcpu_init *init)
 	if (use_lpa2_pte_format(vm))
 		tcr_el1 |= (1ul << 59) /* DS */;
 
-	vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(SYS_SCTLR_EL1), sctlr_el1);
-	vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(SYS_TCR_EL1), tcr_el1);
-	vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(SYS_MAIR_EL1), DEFAULT_MAIR_EL1);
-	vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(SYS_TTBR0_EL1), ttbr0_el1);
-	vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(SYS_TPIDR_EL1), vcpu->id);
+	aarch64_vcpu_set_reg(vcpu, sctlr_el1, tcr_el1, ttbr0_el1);
 }
 
 void vcpu_arch_dump(FILE *stream, struct kvm_vcpu *vcpu, uint8_t indent)
@@ -387,7 +411,11 @@  static struct kvm_vcpu *__aarch64_vcpu_add(struct kvm_vm *vm, uint32_t vcpu_id,
 
 	aarch64_vcpu_setup(vcpu, init);
 
-	vcpu_set_reg(vcpu, ARM64_CORE_REG(sp_el1), stack_vaddr + stack_size);
+	if (is_vcpu_nested(vcpu))
+		vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(SYS_SP_EL2), stack_vaddr + stack_size);
+	else
+		vcpu_set_reg(vcpu, ARM64_CORE_REG(sp_el1), stack_vaddr + stack_size);
+
 	return vcpu;
 }
 
@@ -457,7 +485,10 @@  void vcpu_init_descriptor_tables(struct kvm_vcpu *vcpu)
 {
 	extern char vectors;
 
-	vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(SYS_VBAR_EL1), (uint64_t)&vectors);
+	if (is_vcpu_nested(vcpu))
+		vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(SYS_VBAR_EL2), (uint64_t)&vectors);
+	else
+		vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(SYS_VBAR_EL1), (uint64_t)&vectors);
 }
 
 void route_exception(struct ex_regs *regs, int vector)