All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 0/2] KVM: arm64: mixed-width check should be skipped for uninitialized vCPUs
@ 2022-03-21  5:08 ` Reiji Watanabe
  0 siblings, 0 replies; 18+ messages in thread
From: Reiji Watanabe @ 2022-03-21  5:08 UTC (permalink / raw)
  To: Marc Zyngier, kvmarm
  Cc: kvm, linux-arm-kernel, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Andrew Jones, Paolo Bonzini, Will Deacon,
	Peter Shier, Ricardo Koller, Oliver Upton, Jing Zhang,
	Raghavendra Rao Anata, Reiji Watanabe

KVM allows userspace to configure either all EL1 32bit or 64bit vCPUs
for a guest.  At vCPU reset, vcpu_allowed_register_width() checks
if the vcpu's register width is consistent with all other vCPUs'.
Since the checking is done even against vCPUs that are not initialized
(KVM_ARM_VCPU_INIT has not been done) yet, the uninitialized vCPUs
are erroneously treated as 64bit vCPU, which causes the function to
incorrectly detect a mixed-width VM.

This series will fix this problem by introducing a new VM flag that
indicates the guest needs to be configured with all 32bit or 64bit
vCPUs and checking vcpu's register width against the new flag at
the vcpu's KVM_ARM_VCPU_INIT (instead of against other vCPUs'
register width).

Patch-1 introduces KVM_ARCH_FLAG_EL1_32BIT and
KVM_ARCH_FLAG_REG_WIDTH_CONFIGURED bits for kvm->arch.flags and
uses them to check vcpu's register width to fix the problem.

Patch-2 introduces a selftest that can test non-mixed-width vCPUs (all
64bit vCPUs or all 32bit vcPUs) can be configured, and mixed-width
vCPUs cannot be configured.

The series is based on kvmarm/next's at tag: kvmarm-5.18.

v5:
  - Rebase to kvmarm/next (and drop the patch-1 "KVM: arm64: Generalise
    VM features into a set of flags")
  - Use kernel-doc style comments for kvm_set_vm_width() [Oliver]
  - Change kvm_set_vm_width() to use if/else instead of a ternary
    operator for KVM_ARCH_FLAG_EL1_32BIT check [Oliver]

v4: https://lore.kernel.org/all/20220314061959.3349716-1-reijiw@google.com/
  - Use different implementation of vcpu_el1_is_32bit() depending on
    the context. [Marc]
  - Rename kvm_register_width_check_or_init() to kvm_set_vm_width(), and
    call it from kvm_rest_vcpu() instead of from kvm_vcpu_set_target()
  - Remove vcpu_allowed_register_width(), and does the same checking
    in kvm_set_vm_width() instead.

v3: https://lore.kernel.org/all/20220303035408.3708241-1-reijiw@google.com/
  - Introduced 'flags' to kvm_arch, and use bits of the flags for
    a set of booleans for VM feature.
  - Changed 'el1_reg_width' to two bits of 'flags' of kvm_arch.

v2: https://lore.kernel.org/all/20220118041923.3384602-1-reijiw@google.com/
  - Introduced 'el1_reg_width' for kvm_arch and use it to check vcpu's
    register width against the flag at the vcpu's KVM_ARM_VCPU_INIT.

v1: https://lore.kernel.org/all/20220110054042.1079932-1-reijiw@google.com/

[1] https://lore.kernel.org/all/20210715163159.1480168-2-maz@kernel.org/

Reiji Watanabe (2):
  KVM: arm64: mixed-width check should be skipped for uninitialized
    vCPUs
  KVM: arm64: selftests: Introduce vcpu_width_config

 arch/arm64/include/asm/kvm_emulate.h          |  27 ++--
 arch/arm64/include/asm/kvm_host.h             |  10 ++
 arch/arm64/kvm/reset.c                        |  65 ++++++---
 tools/testing/selftests/kvm/.gitignore        |   1 +
 tools/testing/selftests/kvm/Makefile          |   1 +
 .../selftests/kvm/aarch64/vcpu_width_config.c | 125 ++++++++++++++++++
 6 files changed, 199 insertions(+), 30 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/aarch64/vcpu_width_config.c

-- 
2.35.1.894.gb6a874cedc-goog


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v5 0/2] KVM: arm64: mixed-width check should be skipped for uninitialized vCPUs
@ 2022-03-21  5:08 ` Reiji Watanabe
  0 siblings, 0 replies; 18+ messages in thread
From: Reiji Watanabe @ 2022-03-21  5:08 UTC (permalink / raw)
  To: Marc Zyngier, kvmarm
  Cc: kvm, Will Deacon, Peter Shier, Paolo Bonzini, linux-arm-kernel

KVM allows userspace to configure either all EL1 32bit or 64bit vCPUs
for a guest.  At vCPU reset, vcpu_allowed_register_width() checks
if the vcpu's register width is consistent with all other vCPUs'.
Since the checking is done even against vCPUs that are not initialized
(KVM_ARM_VCPU_INIT has not been done) yet, the uninitialized vCPUs
are erroneously treated as 64bit vCPU, which causes the function to
incorrectly detect a mixed-width VM.

This series will fix this problem by introducing a new VM flag that
indicates the guest needs to be configured with all 32bit or 64bit
vCPUs and checking vcpu's register width against the new flag at
the vcpu's KVM_ARM_VCPU_INIT (instead of against other vCPUs'
register width).

Patch-1 introduces KVM_ARCH_FLAG_EL1_32BIT and
KVM_ARCH_FLAG_REG_WIDTH_CONFIGURED bits for kvm->arch.flags and
uses them to check vcpu's register width to fix the problem.

Patch-2 introduces a selftest that can test non-mixed-width vCPUs (all
64bit vCPUs or all 32bit vcPUs) can be configured, and mixed-width
vCPUs cannot be configured.

The series is based on kvmarm/next's at tag: kvmarm-5.18.

v5:
  - Rebase to kvmarm/next (and drop the patch-1 "KVM: arm64: Generalise
    VM features into a set of flags")
  - Use kernel-doc style comments for kvm_set_vm_width() [Oliver]
  - Change kvm_set_vm_width() to use if/else instead of a ternary
    operator for KVM_ARCH_FLAG_EL1_32BIT check [Oliver]

v4: https://lore.kernel.org/all/20220314061959.3349716-1-reijiw@google.com/
  - Use different implementation of vcpu_el1_is_32bit() depending on
    the context. [Marc]
  - Rename kvm_register_width_check_or_init() to kvm_set_vm_width(), and
    call it from kvm_rest_vcpu() instead of from kvm_vcpu_set_target()
  - Remove vcpu_allowed_register_width(), and does the same checking
    in kvm_set_vm_width() instead.

v3: https://lore.kernel.org/all/20220303035408.3708241-1-reijiw@google.com/
  - Introduced 'flags' to kvm_arch, and use bits of the flags for
    a set of booleans for VM feature.
  - Changed 'el1_reg_width' to two bits of 'flags' of kvm_arch.

v2: https://lore.kernel.org/all/20220118041923.3384602-1-reijiw@google.com/
  - Introduced 'el1_reg_width' for kvm_arch and use it to check vcpu's
    register width against the flag at the vcpu's KVM_ARM_VCPU_INIT.

v1: https://lore.kernel.org/all/20220110054042.1079932-1-reijiw@google.com/

[1] https://lore.kernel.org/all/20210715163159.1480168-2-maz@kernel.org/

Reiji Watanabe (2):
  KVM: arm64: mixed-width check should be skipped for uninitialized
    vCPUs
  KVM: arm64: selftests: Introduce vcpu_width_config

 arch/arm64/include/asm/kvm_emulate.h          |  27 ++--
 arch/arm64/include/asm/kvm_host.h             |  10 ++
 arch/arm64/kvm/reset.c                        |  65 ++++++---
 tools/testing/selftests/kvm/.gitignore        |   1 +
 tools/testing/selftests/kvm/Makefile          |   1 +
 .../selftests/kvm/aarch64/vcpu_width_config.c | 125 ++++++++++++++++++
 6 files changed, 199 insertions(+), 30 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/aarch64/vcpu_width_config.c

-- 
2.35.1.894.gb6a874cedc-goog

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v5 0/2] KVM: arm64: mixed-width check should be skipped for uninitialized vCPUs
@ 2022-03-21  5:08 ` Reiji Watanabe
  0 siblings, 0 replies; 18+ messages in thread
From: Reiji Watanabe @ 2022-03-21  5:08 UTC (permalink / raw)
  To: Marc Zyngier, kvmarm
  Cc: kvm, linux-arm-kernel, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Andrew Jones, Paolo Bonzini, Will Deacon,
	Peter Shier, Ricardo Koller, Oliver Upton, Jing Zhang,
	Raghavendra Rao Anata, Reiji Watanabe

KVM allows userspace to configure either all EL1 32bit or 64bit vCPUs
for a guest.  At vCPU reset, vcpu_allowed_register_width() checks
if the vcpu's register width is consistent with all other vCPUs'.
Since the checking is done even against vCPUs that are not initialized
(KVM_ARM_VCPU_INIT has not been done) yet, the uninitialized vCPUs
are erroneously treated as 64bit vCPU, which causes the function to
incorrectly detect a mixed-width VM.

This series will fix this problem by introducing a new VM flag that
indicates the guest needs to be configured with all 32bit or 64bit
vCPUs and checking vcpu's register width against the new flag at
the vcpu's KVM_ARM_VCPU_INIT (instead of against other vCPUs'
register width).

Patch-1 introduces KVM_ARCH_FLAG_EL1_32BIT and
KVM_ARCH_FLAG_REG_WIDTH_CONFIGURED bits for kvm->arch.flags and
uses them to check vcpu's register width to fix the problem.

Patch-2 introduces a selftest that can test non-mixed-width vCPUs (all
64bit vCPUs or all 32bit vcPUs) can be configured, and mixed-width
vCPUs cannot be configured.

The series is based on kvmarm/next's at tag: kvmarm-5.18.

v5:
  - Rebase to kvmarm/next (and drop the patch-1 "KVM: arm64: Generalise
    VM features into a set of flags")
  - Use kernel-doc style comments for kvm_set_vm_width() [Oliver]
  - Change kvm_set_vm_width() to use if/else instead of a ternary
    operator for KVM_ARCH_FLAG_EL1_32BIT check [Oliver]

v4: https://lore.kernel.org/all/20220314061959.3349716-1-reijiw@google.com/
  - Use different implementation of vcpu_el1_is_32bit() depending on
    the context. [Marc]
  - Rename kvm_register_width_check_or_init() to kvm_set_vm_width(), and
    call it from kvm_rest_vcpu() instead of from kvm_vcpu_set_target()
  - Remove vcpu_allowed_register_width(), and does the same checking
    in kvm_set_vm_width() instead.

v3: https://lore.kernel.org/all/20220303035408.3708241-1-reijiw@google.com/
  - Introduced 'flags' to kvm_arch, and use bits of the flags for
    a set of booleans for VM feature.
  - Changed 'el1_reg_width' to two bits of 'flags' of kvm_arch.

v2: https://lore.kernel.org/all/20220118041923.3384602-1-reijiw@google.com/
  - Introduced 'el1_reg_width' for kvm_arch and use it to check vcpu's
    register width against the flag at the vcpu's KVM_ARM_VCPU_INIT.

v1: https://lore.kernel.org/all/20220110054042.1079932-1-reijiw@google.com/

[1] https://lore.kernel.org/all/20210715163159.1480168-2-maz@kernel.org/

Reiji Watanabe (2):
  KVM: arm64: mixed-width check should be skipped for uninitialized
    vCPUs
  KVM: arm64: selftests: Introduce vcpu_width_config

 arch/arm64/include/asm/kvm_emulate.h          |  27 ++--
 arch/arm64/include/asm/kvm_host.h             |  10 ++
 arch/arm64/kvm/reset.c                        |  65 ++++++---
 tools/testing/selftests/kvm/.gitignore        |   1 +
 tools/testing/selftests/kvm/Makefile          |   1 +
 .../selftests/kvm/aarch64/vcpu_width_config.c | 125 ++++++++++++++++++
 6 files changed, 199 insertions(+), 30 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/aarch64/vcpu_width_config.c

-- 
2.35.1.894.gb6a874cedc-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v5 1/2] KVM: arm64: mixed-width check should be skipped for uninitialized vCPUs
  2022-03-21  5:08 ` Reiji Watanabe
  (?)
@ 2022-03-21  5:08   ` Reiji Watanabe
  -1 siblings, 0 replies; 18+ messages in thread
From: Reiji Watanabe @ 2022-03-21  5:08 UTC (permalink / raw)
  To: Marc Zyngier, kvmarm
  Cc: kvm, linux-arm-kernel, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Andrew Jones, Paolo Bonzini, Will Deacon,
	Peter Shier, Ricardo Koller, Oliver Upton, Jing Zhang,
	Raghavendra Rao Anata, Reiji Watanabe

KVM allows userspace to configure either all EL1 32bit or 64bit vCPUs
for a guest.  At vCPU reset, vcpu_allowed_register_width() checks
if the vcpu's register width is consistent with all other vCPUs'.
Since the checking is done even against vCPUs that are not initialized
(KVM_ARM_VCPU_INIT has not been done) yet, the uninitialized vCPUs
are erroneously treated as 64bit vCPU, which causes the function to
incorrectly detect a mixed-width VM.

Introduce KVM_ARCH_FLAG_EL1_32BIT and KVM_ARCH_FLAG_REG_WIDTH_CONFIGURED
bits for kvm->arch.flags.  A value of the EL1_32BIT bit indicates that
the guest needs to be configured with all 32bit or 64bit vCPUs, and
a value of the REG_WIDTH_CONFIGURED bit indicates if a value of the
EL1_32BIT bit is valid (already set up). Values in those bits are set at
the first KVM_ARM_VCPU_INIT for the guest based on KVM_ARM_VCPU_EL1_32BIT
configuration for the vCPU.

Check vcpu's register width against those new bits at the vcpu's
KVM_ARM_VCPU_INIT (instead of against other vCPUs' register width).

Fixes: 66e94d5cafd4 ("KVM: arm64: Prevent mixed-width VM creation")
Signed-off-by: Reiji Watanabe <reijiw@google.com>
Reviewed-by: Oliver Upton <oupton@google.com>
---
 arch/arm64/include/asm/kvm_emulate.h | 27 ++++++++----
 arch/arm64/include/asm/kvm_host.h    | 10 +++++
 arch/arm64/kvm/reset.c               | 65 ++++++++++++++++++----------
 3 files changed, 72 insertions(+), 30 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
index d62405ce3e6d..7496deab025a 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -43,10 +43,22 @@ void kvm_inject_pabt(struct kvm_vcpu *vcpu, unsigned long addr);
 
 void kvm_vcpu_wfi(struct kvm_vcpu *vcpu);
 
+#if defined(__KVM_VHE_HYPERVISOR__) || defined(__KVM_NVHE_HYPERVISOR__)
 static __always_inline bool vcpu_el1_is_32bit(struct kvm_vcpu *vcpu)
 {
 	return !(vcpu->arch.hcr_el2 & HCR_RW);
 }
+#else
+static __always_inline bool vcpu_el1_is_32bit(struct kvm_vcpu *vcpu)
+{
+	struct kvm *kvm = vcpu->kvm;
+
+	WARN_ON_ONCE(!test_bit(KVM_ARCH_FLAG_REG_WIDTH_CONFIGURED,
+			       &kvm->arch.flags));
+
+	return test_bit(KVM_ARCH_FLAG_EL1_32BIT, &kvm->arch.flags);
+}
+#endif
 
 static inline void vcpu_reset_hcr(struct kvm_vcpu *vcpu)
 {
@@ -72,15 +84,14 @@ static inline void vcpu_reset_hcr(struct kvm_vcpu *vcpu)
 		vcpu->arch.hcr_el2 |= HCR_TVM;
 	}
 
-	if (test_bit(KVM_ARM_VCPU_EL1_32BIT, vcpu->arch.features))
+	if (vcpu_el1_is_32bit(vcpu))
 		vcpu->arch.hcr_el2 &= ~HCR_RW;
-
-	/*
-	 * TID3: trap feature register accesses that we virtualise.
-	 * For now this is conditional, since no AArch32 feature regs
-	 * are currently virtualised.
-	 */
-	if (!vcpu_el1_is_32bit(vcpu))
+	else
+		/*
+		 * TID3: trap feature register accesses that we virtualise.
+		 * For now this is conditional, since no AArch32 feature regs
+		 * are currently virtualised.
+		 */
 		vcpu->arch.hcr_el2 |= HCR_TID3;
 
 	if (cpus_have_const_cap(ARM64_MISMATCHED_CACHE_TYPE) ||
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 0e96087885fe..f7781c5e0c6a 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -127,6 +127,16 @@ struct kvm_arch {
 #define KVM_ARCH_FLAG_MTE_ENABLED			1
 	/* At least one vCPU has ran in the VM */
 #define KVM_ARCH_FLAG_HAS_RAN_ONCE			2
+	/*
+	 * The following two bits are used to indicate the guest's EL1
+	 * register width configuration. A value of KVM_ARCH_FLAG_EL1_32BIT
+	 * bit is valid only when KVM_ARCH_FLAG_REG_WIDTH_CONFIGURED is set.
+	 * Otherwise, the guest's EL1 register width has not yet been
+	 * determined yet.
+	 */
+#define KVM_ARCH_FLAG_REG_WIDTH_CONFIGURED		3
+#define KVM_ARCH_FLAG_EL1_32BIT				4
+
 	unsigned long flags;
 
 	/*
diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
index ecc40c8cd6f6..bc8b3909640f 100644
--- a/arch/arm64/kvm/reset.c
+++ b/arch/arm64/kvm/reset.c
@@ -181,27 +181,46 @@ static int kvm_vcpu_enable_ptrauth(struct kvm_vcpu *vcpu)
 	return 0;
 }
 
-static bool vcpu_allowed_register_width(struct kvm_vcpu *vcpu)
+/**
+ * kvm_set_vm_width() - set a register width for the guest
+ * @kvm: Pointer to the KVM struct
+ * @is32bit: Whether the register width of the guest is 32-bit or not (64-bit)
+ *
+ * Set KVM_ARCH_FLAG_EL1_32BIT bit in kvm->arch.flags based on @is32bit
+ * and also set KVM_ARCH_FLAG_REG_WIDTH_CONFIGURED bit in the flags.
+ * When REG_WIDTH_CONFIGURED bit is already set in the flags, @is32bit
+ * must be consistent with the value of FLAG_EL1_32BIT bit in the flags.
+ *
+ * Return: 0 on success, negative error code on failure.
+ */
+static int kvm_set_vm_width(struct kvm *kvm, bool is32bit)
 {
-	struct kvm_vcpu *tmp;
-	bool is32bit;
-	unsigned long i;
+	lockdep_assert_held(&kvm->lock);
+
+	if (test_bit(KVM_ARCH_FLAG_REG_WIDTH_CONFIGURED, &kvm->arch.flags)) {
+		/*
+		 * The guest's register width is already configured.
+		 * Make sure that @is32bit is consistent with it.
+		 */
+		if (is32bit == test_bit(KVM_ARCH_FLAG_EL1_32BIT, &kvm->arch.flags))
+			return 0;
+		else
+			return -EINVAL;
+	}
 
-	is32bit = vcpu_has_feature(vcpu, KVM_ARM_VCPU_EL1_32BIT);
 	if (!cpus_have_const_cap(ARM64_HAS_32BIT_EL1) && is32bit)
-		return false;
+		return -EINVAL;
 
 	/* MTE is incompatible with AArch32 */
-	if (kvm_has_mte(vcpu->kvm) && is32bit)
-		return false;
+	if (kvm_has_mte(kvm) && is32bit)
+		return -EINVAL;
 
-	/* Check that the vcpus are either all 32bit or all 64bit */
-	kvm_for_each_vcpu(i, tmp, vcpu->kvm) {
-		if (vcpu_has_feature(tmp, KVM_ARM_VCPU_EL1_32BIT) != is32bit)
-			return false;
-	}
+	if (is32bit)
+		set_bit(KVM_ARCH_FLAG_EL1_32BIT, &kvm->arch.flags);
 
-	return true;
+	set_bit(KVM_ARCH_FLAG_REG_WIDTH_CONFIGURED, &kvm->arch.flags);
+
+	return 0;
 }
 
 /**
@@ -230,10 +249,17 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
 	u32 pstate;
 
 	mutex_lock(&vcpu->kvm->lock);
-	reset_state = vcpu->arch.reset_state;
-	WRITE_ONCE(vcpu->arch.reset_state.reset, false);
+	ret = kvm_set_vm_width(vcpu->kvm,
+			       vcpu_has_feature(vcpu, KVM_ARM_VCPU_EL1_32BIT));
+	if (!ret) {
+		reset_state = vcpu->arch.reset_state;
+		WRITE_ONCE(vcpu->arch.reset_state.reset, false);
+	}
 	mutex_unlock(&vcpu->kvm->lock);
 
+	if (ret)
+		return ret;
+
 	/* Reset PMU outside of the non-preemptible section */
 	kvm_pmu_vcpu_reset(vcpu);
 
@@ -260,14 +286,9 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
 		}
 	}
 
-	if (!vcpu_allowed_register_width(vcpu)) {
-		ret = -EINVAL;
-		goto out;
-	}
-
 	switch (vcpu->arch.target) {
 	default:
-		if (test_bit(KVM_ARM_VCPU_EL1_32BIT, vcpu->arch.features)) {
+		if (vcpu_el1_is_32bit(vcpu)) {
 			pstate = VCPU_RESET_PSTATE_SVC;
 		} else {
 			pstate = VCPU_RESET_PSTATE_EL1;
-- 
2.35.1.894.gb6a874cedc-goog


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v5 1/2] KVM: arm64: mixed-width check should be skipped for uninitialized vCPUs
@ 2022-03-21  5:08   ` Reiji Watanabe
  0 siblings, 0 replies; 18+ messages in thread
From: Reiji Watanabe @ 2022-03-21  5:08 UTC (permalink / raw)
  To: Marc Zyngier, kvmarm
  Cc: kvm, Will Deacon, Peter Shier, Paolo Bonzini, linux-arm-kernel

KVM allows userspace to configure either all EL1 32bit or 64bit vCPUs
for a guest.  At vCPU reset, vcpu_allowed_register_width() checks
if the vcpu's register width is consistent with all other vCPUs'.
Since the checking is done even against vCPUs that are not initialized
(KVM_ARM_VCPU_INIT has not been done) yet, the uninitialized vCPUs
are erroneously treated as 64bit vCPU, which causes the function to
incorrectly detect a mixed-width VM.

Introduce KVM_ARCH_FLAG_EL1_32BIT and KVM_ARCH_FLAG_REG_WIDTH_CONFIGURED
bits for kvm->arch.flags.  A value of the EL1_32BIT bit indicates that
the guest needs to be configured with all 32bit or 64bit vCPUs, and
a value of the REG_WIDTH_CONFIGURED bit indicates if a value of the
EL1_32BIT bit is valid (already set up). Values in those bits are set at
the first KVM_ARM_VCPU_INIT for the guest based on KVM_ARM_VCPU_EL1_32BIT
configuration for the vCPU.

Check vcpu's register width against those new bits at the vcpu's
KVM_ARM_VCPU_INIT (instead of against other vCPUs' register width).

Fixes: 66e94d5cafd4 ("KVM: arm64: Prevent mixed-width VM creation")
Signed-off-by: Reiji Watanabe <reijiw@google.com>
Reviewed-by: Oliver Upton <oupton@google.com>
---
 arch/arm64/include/asm/kvm_emulate.h | 27 ++++++++----
 arch/arm64/include/asm/kvm_host.h    | 10 +++++
 arch/arm64/kvm/reset.c               | 65 ++++++++++++++++++----------
 3 files changed, 72 insertions(+), 30 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
index d62405ce3e6d..7496deab025a 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -43,10 +43,22 @@ void kvm_inject_pabt(struct kvm_vcpu *vcpu, unsigned long addr);
 
 void kvm_vcpu_wfi(struct kvm_vcpu *vcpu);
 
+#if defined(__KVM_VHE_HYPERVISOR__) || defined(__KVM_NVHE_HYPERVISOR__)
 static __always_inline bool vcpu_el1_is_32bit(struct kvm_vcpu *vcpu)
 {
 	return !(vcpu->arch.hcr_el2 & HCR_RW);
 }
+#else
+static __always_inline bool vcpu_el1_is_32bit(struct kvm_vcpu *vcpu)
+{
+	struct kvm *kvm = vcpu->kvm;
+
+	WARN_ON_ONCE(!test_bit(KVM_ARCH_FLAG_REG_WIDTH_CONFIGURED,
+			       &kvm->arch.flags));
+
+	return test_bit(KVM_ARCH_FLAG_EL1_32BIT, &kvm->arch.flags);
+}
+#endif
 
 static inline void vcpu_reset_hcr(struct kvm_vcpu *vcpu)
 {
@@ -72,15 +84,14 @@ static inline void vcpu_reset_hcr(struct kvm_vcpu *vcpu)
 		vcpu->arch.hcr_el2 |= HCR_TVM;
 	}
 
-	if (test_bit(KVM_ARM_VCPU_EL1_32BIT, vcpu->arch.features))
+	if (vcpu_el1_is_32bit(vcpu))
 		vcpu->arch.hcr_el2 &= ~HCR_RW;
-
-	/*
-	 * TID3: trap feature register accesses that we virtualise.
-	 * For now this is conditional, since no AArch32 feature regs
-	 * are currently virtualised.
-	 */
-	if (!vcpu_el1_is_32bit(vcpu))
+	else
+		/*
+		 * TID3: trap feature register accesses that we virtualise.
+		 * For now this is conditional, since no AArch32 feature regs
+		 * are currently virtualised.
+		 */
 		vcpu->arch.hcr_el2 |= HCR_TID3;
 
 	if (cpus_have_const_cap(ARM64_MISMATCHED_CACHE_TYPE) ||
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 0e96087885fe..f7781c5e0c6a 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -127,6 +127,16 @@ struct kvm_arch {
 #define KVM_ARCH_FLAG_MTE_ENABLED			1
 	/* At least one vCPU has ran in the VM */
 #define KVM_ARCH_FLAG_HAS_RAN_ONCE			2
+	/*
+	 * The following two bits are used to indicate the guest's EL1
+	 * register width configuration. A value of KVM_ARCH_FLAG_EL1_32BIT
+	 * bit is valid only when KVM_ARCH_FLAG_REG_WIDTH_CONFIGURED is set.
+	 * Otherwise, the guest's EL1 register width has not yet been
+	 * determined yet.
+	 */
+#define KVM_ARCH_FLAG_REG_WIDTH_CONFIGURED		3
+#define KVM_ARCH_FLAG_EL1_32BIT				4
+
 	unsigned long flags;
 
 	/*
diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
index ecc40c8cd6f6..bc8b3909640f 100644
--- a/arch/arm64/kvm/reset.c
+++ b/arch/arm64/kvm/reset.c
@@ -181,27 +181,46 @@ static int kvm_vcpu_enable_ptrauth(struct kvm_vcpu *vcpu)
 	return 0;
 }
 
-static bool vcpu_allowed_register_width(struct kvm_vcpu *vcpu)
+/**
+ * kvm_set_vm_width() - set a register width for the guest
+ * @kvm: Pointer to the KVM struct
+ * @is32bit: Whether the register width of the guest is 32-bit or not (64-bit)
+ *
+ * Set KVM_ARCH_FLAG_EL1_32BIT bit in kvm->arch.flags based on @is32bit
+ * and also set KVM_ARCH_FLAG_REG_WIDTH_CONFIGURED bit in the flags.
+ * When REG_WIDTH_CONFIGURED bit is already set in the flags, @is32bit
+ * must be consistent with the value of FLAG_EL1_32BIT bit in the flags.
+ *
+ * Return: 0 on success, negative error code on failure.
+ */
+static int kvm_set_vm_width(struct kvm *kvm, bool is32bit)
 {
-	struct kvm_vcpu *tmp;
-	bool is32bit;
-	unsigned long i;
+	lockdep_assert_held(&kvm->lock);
+
+	if (test_bit(KVM_ARCH_FLAG_REG_WIDTH_CONFIGURED, &kvm->arch.flags)) {
+		/*
+		 * The guest's register width is already configured.
+		 * Make sure that @is32bit is consistent with it.
+		 */
+		if (is32bit == test_bit(KVM_ARCH_FLAG_EL1_32BIT, &kvm->arch.flags))
+			return 0;
+		else
+			return -EINVAL;
+	}
 
-	is32bit = vcpu_has_feature(vcpu, KVM_ARM_VCPU_EL1_32BIT);
 	if (!cpus_have_const_cap(ARM64_HAS_32BIT_EL1) && is32bit)
-		return false;
+		return -EINVAL;
 
 	/* MTE is incompatible with AArch32 */
-	if (kvm_has_mte(vcpu->kvm) && is32bit)
-		return false;
+	if (kvm_has_mte(kvm) && is32bit)
+		return -EINVAL;
 
-	/* Check that the vcpus are either all 32bit or all 64bit */
-	kvm_for_each_vcpu(i, tmp, vcpu->kvm) {
-		if (vcpu_has_feature(tmp, KVM_ARM_VCPU_EL1_32BIT) != is32bit)
-			return false;
-	}
+	if (is32bit)
+		set_bit(KVM_ARCH_FLAG_EL1_32BIT, &kvm->arch.flags);
 
-	return true;
+	set_bit(KVM_ARCH_FLAG_REG_WIDTH_CONFIGURED, &kvm->arch.flags);
+
+	return 0;
 }
 
 /**
@@ -230,10 +249,17 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
 	u32 pstate;
 
 	mutex_lock(&vcpu->kvm->lock);
-	reset_state = vcpu->arch.reset_state;
-	WRITE_ONCE(vcpu->arch.reset_state.reset, false);
+	ret = kvm_set_vm_width(vcpu->kvm,
+			       vcpu_has_feature(vcpu, KVM_ARM_VCPU_EL1_32BIT));
+	if (!ret) {
+		reset_state = vcpu->arch.reset_state;
+		WRITE_ONCE(vcpu->arch.reset_state.reset, false);
+	}
 	mutex_unlock(&vcpu->kvm->lock);
 
+	if (ret)
+		return ret;
+
 	/* Reset PMU outside of the non-preemptible section */
 	kvm_pmu_vcpu_reset(vcpu);
 
@@ -260,14 +286,9 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
 		}
 	}
 
-	if (!vcpu_allowed_register_width(vcpu)) {
-		ret = -EINVAL;
-		goto out;
-	}
-
 	switch (vcpu->arch.target) {
 	default:
-		if (test_bit(KVM_ARM_VCPU_EL1_32BIT, vcpu->arch.features)) {
+		if (vcpu_el1_is_32bit(vcpu)) {
 			pstate = VCPU_RESET_PSTATE_SVC;
 		} else {
 			pstate = VCPU_RESET_PSTATE_EL1;
-- 
2.35.1.894.gb6a874cedc-goog

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v5 1/2] KVM: arm64: mixed-width check should be skipped for uninitialized vCPUs
@ 2022-03-21  5:08   ` Reiji Watanabe
  0 siblings, 0 replies; 18+ messages in thread
From: Reiji Watanabe @ 2022-03-21  5:08 UTC (permalink / raw)
  To: Marc Zyngier, kvmarm
  Cc: kvm, linux-arm-kernel, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Andrew Jones, Paolo Bonzini, Will Deacon,
	Peter Shier, Ricardo Koller, Oliver Upton, Jing Zhang,
	Raghavendra Rao Anata, Reiji Watanabe

KVM allows userspace to configure either all EL1 32bit or 64bit vCPUs
for a guest.  At vCPU reset, vcpu_allowed_register_width() checks
if the vcpu's register width is consistent with all other vCPUs'.
Since the checking is done even against vCPUs that are not initialized
(KVM_ARM_VCPU_INIT has not been done) yet, the uninitialized vCPUs
are erroneously treated as 64bit vCPU, which causes the function to
incorrectly detect a mixed-width VM.

Introduce KVM_ARCH_FLAG_EL1_32BIT and KVM_ARCH_FLAG_REG_WIDTH_CONFIGURED
bits for kvm->arch.flags.  A value of the EL1_32BIT bit indicates that
the guest needs to be configured with all 32bit or 64bit vCPUs, and
a value of the REG_WIDTH_CONFIGURED bit indicates if a value of the
EL1_32BIT bit is valid (already set up). Values in those bits are set at
the first KVM_ARM_VCPU_INIT for the guest based on KVM_ARM_VCPU_EL1_32BIT
configuration for the vCPU.

Check vcpu's register width against those new bits at the vcpu's
KVM_ARM_VCPU_INIT (instead of against other vCPUs' register width).

Fixes: 66e94d5cafd4 ("KVM: arm64: Prevent mixed-width VM creation")
Signed-off-by: Reiji Watanabe <reijiw@google.com>
Reviewed-by: Oliver Upton <oupton@google.com>
---
 arch/arm64/include/asm/kvm_emulate.h | 27 ++++++++----
 arch/arm64/include/asm/kvm_host.h    | 10 +++++
 arch/arm64/kvm/reset.c               | 65 ++++++++++++++++++----------
 3 files changed, 72 insertions(+), 30 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
index d62405ce3e6d..7496deab025a 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -43,10 +43,22 @@ void kvm_inject_pabt(struct kvm_vcpu *vcpu, unsigned long addr);
 
 void kvm_vcpu_wfi(struct kvm_vcpu *vcpu);
 
+#if defined(__KVM_VHE_HYPERVISOR__) || defined(__KVM_NVHE_HYPERVISOR__)
 static __always_inline bool vcpu_el1_is_32bit(struct kvm_vcpu *vcpu)
 {
 	return !(vcpu->arch.hcr_el2 & HCR_RW);
 }
+#else
+static __always_inline bool vcpu_el1_is_32bit(struct kvm_vcpu *vcpu)
+{
+	struct kvm *kvm = vcpu->kvm;
+
+	WARN_ON_ONCE(!test_bit(KVM_ARCH_FLAG_REG_WIDTH_CONFIGURED,
+			       &kvm->arch.flags));
+
+	return test_bit(KVM_ARCH_FLAG_EL1_32BIT, &kvm->arch.flags);
+}
+#endif
 
 static inline void vcpu_reset_hcr(struct kvm_vcpu *vcpu)
 {
@@ -72,15 +84,14 @@ static inline void vcpu_reset_hcr(struct kvm_vcpu *vcpu)
 		vcpu->arch.hcr_el2 |= HCR_TVM;
 	}
 
-	if (test_bit(KVM_ARM_VCPU_EL1_32BIT, vcpu->arch.features))
+	if (vcpu_el1_is_32bit(vcpu))
 		vcpu->arch.hcr_el2 &= ~HCR_RW;
-
-	/*
-	 * TID3: trap feature register accesses that we virtualise.
-	 * For now this is conditional, since no AArch32 feature regs
-	 * are currently virtualised.
-	 */
-	if (!vcpu_el1_is_32bit(vcpu))
+	else
+		/*
+		 * TID3: trap feature register accesses that we virtualise.
+		 * For now this is conditional, since no AArch32 feature regs
+		 * are currently virtualised.
+		 */
 		vcpu->arch.hcr_el2 |= HCR_TID3;
 
 	if (cpus_have_const_cap(ARM64_MISMATCHED_CACHE_TYPE) ||
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 0e96087885fe..f7781c5e0c6a 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -127,6 +127,16 @@ struct kvm_arch {
 #define KVM_ARCH_FLAG_MTE_ENABLED			1
 	/* At least one vCPU has ran in the VM */
 #define KVM_ARCH_FLAG_HAS_RAN_ONCE			2
+	/*
+	 * The following two bits are used to indicate the guest's EL1
+	 * register width configuration. A value of KVM_ARCH_FLAG_EL1_32BIT
+	 * bit is valid only when KVM_ARCH_FLAG_REG_WIDTH_CONFIGURED is set.
+	 * Otherwise, the guest's EL1 register width has not yet been
+	 * determined yet.
+	 */
+#define KVM_ARCH_FLAG_REG_WIDTH_CONFIGURED		3
+#define KVM_ARCH_FLAG_EL1_32BIT				4
+
 	unsigned long flags;
 
 	/*
diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
index ecc40c8cd6f6..bc8b3909640f 100644
--- a/arch/arm64/kvm/reset.c
+++ b/arch/arm64/kvm/reset.c
@@ -181,27 +181,46 @@ static int kvm_vcpu_enable_ptrauth(struct kvm_vcpu *vcpu)
 	return 0;
 }
 
-static bool vcpu_allowed_register_width(struct kvm_vcpu *vcpu)
+/**
+ * kvm_set_vm_width() - set a register width for the guest
+ * @kvm: Pointer to the KVM struct
+ * @is32bit: Whether the register width of the guest is 32-bit or not (64-bit)
+ *
+ * Set KVM_ARCH_FLAG_EL1_32BIT bit in kvm->arch.flags based on @is32bit
+ * and also set KVM_ARCH_FLAG_REG_WIDTH_CONFIGURED bit in the flags.
+ * When REG_WIDTH_CONFIGURED bit is already set in the flags, @is32bit
+ * must be consistent with the value of FLAG_EL1_32BIT bit in the flags.
+ *
+ * Return: 0 on success, negative error code on failure.
+ */
+static int kvm_set_vm_width(struct kvm *kvm, bool is32bit)
 {
-	struct kvm_vcpu *tmp;
-	bool is32bit;
-	unsigned long i;
+	lockdep_assert_held(&kvm->lock);
+
+	if (test_bit(KVM_ARCH_FLAG_REG_WIDTH_CONFIGURED, &kvm->arch.flags)) {
+		/*
+		 * The guest's register width is already configured.
+		 * Make sure that @is32bit is consistent with it.
+		 */
+		if (is32bit == test_bit(KVM_ARCH_FLAG_EL1_32BIT, &kvm->arch.flags))
+			return 0;
+		else
+			return -EINVAL;
+	}
 
-	is32bit = vcpu_has_feature(vcpu, KVM_ARM_VCPU_EL1_32BIT);
 	if (!cpus_have_const_cap(ARM64_HAS_32BIT_EL1) && is32bit)
-		return false;
+		return -EINVAL;
 
 	/* MTE is incompatible with AArch32 */
-	if (kvm_has_mte(vcpu->kvm) && is32bit)
-		return false;
+	if (kvm_has_mte(kvm) && is32bit)
+		return -EINVAL;
 
-	/* Check that the vcpus are either all 32bit or all 64bit */
-	kvm_for_each_vcpu(i, tmp, vcpu->kvm) {
-		if (vcpu_has_feature(tmp, KVM_ARM_VCPU_EL1_32BIT) != is32bit)
-			return false;
-	}
+	if (is32bit)
+		set_bit(KVM_ARCH_FLAG_EL1_32BIT, &kvm->arch.flags);
 
-	return true;
+	set_bit(KVM_ARCH_FLAG_REG_WIDTH_CONFIGURED, &kvm->arch.flags);
+
+	return 0;
 }
 
 /**
@@ -230,10 +249,17 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
 	u32 pstate;
 
 	mutex_lock(&vcpu->kvm->lock);
-	reset_state = vcpu->arch.reset_state;
-	WRITE_ONCE(vcpu->arch.reset_state.reset, false);
+	ret = kvm_set_vm_width(vcpu->kvm,
+			       vcpu_has_feature(vcpu, KVM_ARM_VCPU_EL1_32BIT));
+	if (!ret) {
+		reset_state = vcpu->arch.reset_state;
+		WRITE_ONCE(vcpu->arch.reset_state.reset, false);
+	}
 	mutex_unlock(&vcpu->kvm->lock);
 
+	if (ret)
+		return ret;
+
 	/* Reset PMU outside of the non-preemptible section */
 	kvm_pmu_vcpu_reset(vcpu);
 
@@ -260,14 +286,9 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
 		}
 	}
 
-	if (!vcpu_allowed_register_width(vcpu)) {
-		ret = -EINVAL;
-		goto out;
-	}
-
 	switch (vcpu->arch.target) {
 	default:
-		if (test_bit(KVM_ARM_VCPU_EL1_32BIT, vcpu->arch.features)) {
+		if (vcpu_el1_is_32bit(vcpu)) {
 			pstate = VCPU_RESET_PSTATE_SVC;
 		} else {
 			pstate = VCPU_RESET_PSTATE_EL1;
-- 
2.35.1.894.gb6a874cedc-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v5 2/2] KVM: arm64: selftests: Introduce vcpu_width_config
  2022-03-21  5:08 ` Reiji Watanabe
  (?)
@ 2022-03-21  5:08   ` Reiji Watanabe
  -1 siblings, 0 replies; 18+ messages in thread
From: Reiji Watanabe @ 2022-03-21  5:08 UTC (permalink / raw)
  To: Marc Zyngier, kvmarm
  Cc: kvm, linux-arm-kernel, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Andrew Jones, Paolo Bonzini, Will Deacon,
	Peter Shier, Ricardo Koller, Oliver Upton, Jing Zhang,
	Raghavendra Rao Anata, Reiji Watanabe

Introduce a test for aarch64 that ensures non-mixed-width vCPUs
(all 64bit vCPUs or all 32bit vcPUs) can be configured, and
mixed-width vCPUs cannot be configured.

Reviewed-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Reiji Watanabe <reijiw@google.com>
---
 tools/testing/selftests/kvm/.gitignore        |   1 +
 tools/testing/selftests/kvm/Makefile          |   1 +
 .../selftests/kvm/aarch64/vcpu_width_config.c | 125 ++++++++++++++++++
 3 files changed, 127 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/aarch64/vcpu_width_config.c

diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore
index dce7de7755e6..4e884e29b2a8 100644
--- a/tools/testing/selftests/kvm/.gitignore
+++ b/tools/testing/selftests/kvm/.gitignore
@@ -3,6 +3,7 @@
 /aarch64/debug-exceptions
 /aarch64/get-reg-list
 /aarch64/psci_cpu_on_test
+/aarch64/vcpu_width_config
 /aarch64/vgic_init
 /aarch64/vgic_irq
 /s390x/memop
diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index 0e4926bc9a58..06a5a982123e 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -104,6 +104,7 @@ 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
+TEST_GEN_PROGS_aarch64 += aarch64/vcpu_width_config
 TEST_GEN_PROGS_aarch64 += aarch64/vgic_init
 TEST_GEN_PROGS_aarch64 += aarch64/vgic_irq
 TEST_GEN_PROGS_aarch64 += demand_paging_test
diff --git a/tools/testing/selftests/kvm/aarch64/vcpu_width_config.c b/tools/testing/selftests/kvm/aarch64/vcpu_width_config.c
new file mode 100644
index 000000000000..6e6e6a9f69e3
--- /dev/null
+++ b/tools/testing/selftests/kvm/aarch64/vcpu_width_config.c
@@ -0,0 +1,125 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * vcpu_width_config - Test KVM_ARM_VCPU_INIT() with KVM_ARM_VCPU_EL1_32BIT.
+ *
+ * Copyright (c) 2022 Google LLC.
+ *
+ * This is a test that ensures that non-mixed-width vCPUs (all 64bit vCPUs
+ * or all 32bit vcPUs) can be configured and mixed-width vCPUs cannot be
+ * configured.
+ */
+
+#define _GNU_SOURCE
+
+#include "kvm_util.h"
+#include "processor.h"
+#include "test_util.h"
+
+
+/*
+ * Add a vCPU, run KVM_ARM_VCPU_INIT with @init1, and then
+ * add another vCPU, and run KVM_ARM_VCPU_INIT with @init2.
+ */
+static int add_init_2vcpus(struct kvm_vcpu_init *init1,
+			   struct kvm_vcpu_init *init2)
+{
+	struct kvm_vm *vm;
+	int ret;
+
+	vm = vm_create(VM_MODE_DEFAULT, DEFAULT_GUEST_PHY_PAGES, O_RDWR);
+
+	vm_vcpu_add(vm, 0);
+	ret = _vcpu_ioctl(vm, 0, KVM_ARM_VCPU_INIT, init1);
+	if (ret)
+		goto free_exit;
+
+	vm_vcpu_add(vm, 1);
+	ret = _vcpu_ioctl(vm, 1, KVM_ARM_VCPU_INIT, init2);
+
+free_exit:
+	kvm_vm_free(vm);
+	return ret;
+}
+
+/*
+ * Add two vCPUs, then run KVM_ARM_VCPU_INIT for one vCPU with @init1,
+ * and run KVM_ARM_VCPU_INIT for another vCPU with @init2.
+ */
+static int add_2vcpus_init_2vcpus(struct kvm_vcpu_init *init1,
+				  struct kvm_vcpu_init *init2)
+{
+	struct kvm_vm *vm;
+	int ret;
+
+	vm = vm_create(VM_MODE_DEFAULT, DEFAULT_GUEST_PHY_PAGES, O_RDWR);
+
+	vm_vcpu_add(vm, 0);
+	vm_vcpu_add(vm, 1);
+
+	ret = _vcpu_ioctl(vm, 0, KVM_ARM_VCPU_INIT, init1);
+	if (ret)
+		goto free_exit;
+
+	ret = _vcpu_ioctl(vm, 1, KVM_ARM_VCPU_INIT, init2);
+
+free_exit:
+	kvm_vm_free(vm);
+	return ret;
+}
+
+/*
+ * Tests that two 64bit vCPUs can be configured, two 32bit vCPUs can be
+ * configured, and two mixed-witgh vCPUs cannot be configured.
+ * Each of those three cases, configure vCPUs in two different orders.
+ * The one is running KVM_CREATE_VCPU for 2 vCPUs, and then running
+ * KVM_ARM_VCPU_INIT for them.
+ * The other is running KVM_CREATE_VCPU and KVM_ARM_VCPU_INIT for a vCPU,
+ * and then run those commands for another vCPU.
+ */
+int main(void)
+{
+	struct kvm_vcpu_init init1, init2;
+	struct kvm_vm *vm;
+	int ret;
+
+	if (kvm_check_cap(KVM_CAP_ARM_EL1_32BIT) <= 0) {
+		print_skip("KVM_CAP_ARM_EL1_32BIT is not supported");
+		exit(KSFT_SKIP);
+	}
+
+	/* Get the preferred target type and copy that to init2 */
+	vm = vm_create(VM_MODE_DEFAULT, DEFAULT_GUEST_PHY_PAGES, O_RDWR);
+	vm_ioctl(vm, KVM_ARM_PREFERRED_TARGET, &init1);
+	kvm_vm_free(vm);
+	memcpy(&init2, &init1, sizeof(init2));
+
+	/* Test with 64bit vCPUs */
+	ret = add_init_2vcpus(&init1, &init2);
+	TEST_ASSERT(ret == 0,
+		    "Configuring 64bit EL1 vCPUs failed unexpectedly");
+	ret = add_2vcpus_init_2vcpus(&init1, &init2);
+	TEST_ASSERT(ret == 0,
+		    "Configuring 64bit EL1 vCPUs failed unexpectedly");
+
+	/* Test with 32bit vCPUs */
+	init1.features[0] = (1 << KVM_ARM_VCPU_EL1_32BIT);
+	init2.features[0] = (1 << KVM_ARM_VCPU_EL1_32BIT);
+	ret = add_init_2vcpus(&init1, &init2);
+	TEST_ASSERT(ret == 0,
+		    "Configuring 32bit EL1 vCPUs failed unexpectedly");
+	ret = add_2vcpus_init_2vcpus(&init1, &init2);
+	TEST_ASSERT(ret == 0,
+		    "Configuring 32bit EL1 vCPUs failed unexpectedly");
+
+	/* Test with mixed-width vCPUs  */
+	init1.features[0] = 0;
+	init2.features[0] = (1 << KVM_ARM_VCPU_EL1_32BIT);
+	ret = add_init_2vcpus(&init1, &init2);
+	TEST_ASSERT(ret != 0,
+		    "Configuring mixed-width vCPUs worked unexpectedly");
+	ret = add_2vcpus_init_2vcpus(&init1, &init2);
+	TEST_ASSERT(ret != 0,
+		    "Configuring mixed-width vCPUs worked unexpectedly");
+
+	return 0;
+}
-- 
2.35.1.894.gb6a874cedc-goog


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v5 2/2] KVM: arm64: selftests: Introduce vcpu_width_config
@ 2022-03-21  5:08   ` Reiji Watanabe
  0 siblings, 0 replies; 18+ messages in thread
From: Reiji Watanabe @ 2022-03-21  5:08 UTC (permalink / raw)
  To: Marc Zyngier, kvmarm
  Cc: kvm, Will Deacon, Peter Shier, Paolo Bonzini, linux-arm-kernel

Introduce a test for aarch64 that ensures non-mixed-width vCPUs
(all 64bit vCPUs or all 32bit vcPUs) can be configured, and
mixed-width vCPUs cannot be configured.

Reviewed-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Reiji Watanabe <reijiw@google.com>
---
 tools/testing/selftests/kvm/.gitignore        |   1 +
 tools/testing/selftests/kvm/Makefile          |   1 +
 .../selftests/kvm/aarch64/vcpu_width_config.c | 125 ++++++++++++++++++
 3 files changed, 127 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/aarch64/vcpu_width_config.c

diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore
index dce7de7755e6..4e884e29b2a8 100644
--- a/tools/testing/selftests/kvm/.gitignore
+++ b/tools/testing/selftests/kvm/.gitignore
@@ -3,6 +3,7 @@
 /aarch64/debug-exceptions
 /aarch64/get-reg-list
 /aarch64/psci_cpu_on_test
+/aarch64/vcpu_width_config
 /aarch64/vgic_init
 /aarch64/vgic_irq
 /s390x/memop
diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index 0e4926bc9a58..06a5a982123e 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -104,6 +104,7 @@ 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
+TEST_GEN_PROGS_aarch64 += aarch64/vcpu_width_config
 TEST_GEN_PROGS_aarch64 += aarch64/vgic_init
 TEST_GEN_PROGS_aarch64 += aarch64/vgic_irq
 TEST_GEN_PROGS_aarch64 += demand_paging_test
diff --git a/tools/testing/selftests/kvm/aarch64/vcpu_width_config.c b/tools/testing/selftests/kvm/aarch64/vcpu_width_config.c
new file mode 100644
index 000000000000..6e6e6a9f69e3
--- /dev/null
+++ b/tools/testing/selftests/kvm/aarch64/vcpu_width_config.c
@@ -0,0 +1,125 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * vcpu_width_config - Test KVM_ARM_VCPU_INIT() with KVM_ARM_VCPU_EL1_32BIT.
+ *
+ * Copyright (c) 2022 Google LLC.
+ *
+ * This is a test that ensures that non-mixed-width vCPUs (all 64bit vCPUs
+ * or all 32bit vcPUs) can be configured and mixed-width vCPUs cannot be
+ * configured.
+ */
+
+#define _GNU_SOURCE
+
+#include "kvm_util.h"
+#include "processor.h"
+#include "test_util.h"
+
+
+/*
+ * Add a vCPU, run KVM_ARM_VCPU_INIT with @init1, and then
+ * add another vCPU, and run KVM_ARM_VCPU_INIT with @init2.
+ */
+static int add_init_2vcpus(struct kvm_vcpu_init *init1,
+			   struct kvm_vcpu_init *init2)
+{
+	struct kvm_vm *vm;
+	int ret;
+
+	vm = vm_create(VM_MODE_DEFAULT, DEFAULT_GUEST_PHY_PAGES, O_RDWR);
+
+	vm_vcpu_add(vm, 0);
+	ret = _vcpu_ioctl(vm, 0, KVM_ARM_VCPU_INIT, init1);
+	if (ret)
+		goto free_exit;
+
+	vm_vcpu_add(vm, 1);
+	ret = _vcpu_ioctl(vm, 1, KVM_ARM_VCPU_INIT, init2);
+
+free_exit:
+	kvm_vm_free(vm);
+	return ret;
+}
+
+/*
+ * Add two vCPUs, then run KVM_ARM_VCPU_INIT for one vCPU with @init1,
+ * and run KVM_ARM_VCPU_INIT for another vCPU with @init2.
+ */
+static int add_2vcpus_init_2vcpus(struct kvm_vcpu_init *init1,
+				  struct kvm_vcpu_init *init2)
+{
+	struct kvm_vm *vm;
+	int ret;
+
+	vm = vm_create(VM_MODE_DEFAULT, DEFAULT_GUEST_PHY_PAGES, O_RDWR);
+
+	vm_vcpu_add(vm, 0);
+	vm_vcpu_add(vm, 1);
+
+	ret = _vcpu_ioctl(vm, 0, KVM_ARM_VCPU_INIT, init1);
+	if (ret)
+		goto free_exit;
+
+	ret = _vcpu_ioctl(vm, 1, KVM_ARM_VCPU_INIT, init2);
+
+free_exit:
+	kvm_vm_free(vm);
+	return ret;
+}
+
+/*
+ * Tests that two 64bit vCPUs can be configured, two 32bit vCPUs can be
+ * configured, and two mixed-witgh vCPUs cannot be configured.
+ * Each of those three cases, configure vCPUs in two different orders.
+ * The one is running KVM_CREATE_VCPU for 2 vCPUs, and then running
+ * KVM_ARM_VCPU_INIT for them.
+ * The other is running KVM_CREATE_VCPU and KVM_ARM_VCPU_INIT for a vCPU,
+ * and then run those commands for another vCPU.
+ */
+int main(void)
+{
+	struct kvm_vcpu_init init1, init2;
+	struct kvm_vm *vm;
+	int ret;
+
+	if (kvm_check_cap(KVM_CAP_ARM_EL1_32BIT) <= 0) {
+		print_skip("KVM_CAP_ARM_EL1_32BIT is not supported");
+		exit(KSFT_SKIP);
+	}
+
+	/* Get the preferred target type and copy that to init2 */
+	vm = vm_create(VM_MODE_DEFAULT, DEFAULT_GUEST_PHY_PAGES, O_RDWR);
+	vm_ioctl(vm, KVM_ARM_PREFERRED_TARGET, &init1);
+	kvm_vm_free(vm);
+	memcpy(&init2, &init1, sizeof(init2));
+
+	/* Test with 64bit vCPUs */
+	ret = add_init_2vcpus(&init1, &init2);
+	TEST_ASSERT(ret == 0,
+		    "Configuring 64bit EL1 vCPUs failed unexpectedly");
+	ret = add_2vcpus_init_2vcpus(&init1, &init2);
+	TEST_ASSERT(ret == 0,
+		    "Configuring 64bit EL1 vCPUs failed unexpectedly");
+
+	/* Test with 32bit vCPUs */
+	init1.features[0] = (1 << KVM_ARM_VCPU_EL1_32BIT);
+	init2.features[0] = (1 << KVM_ARM_VCPU_EL1_32BIT);
+	ret = add_init_2vcpus(&init1, &init2);
+	TEST_ASSERT(ret == 0,
+		    "Configuring 32bit EL1 vCPUs failed unexpectedly");
+	ret = add_2vcpus_init_2vcpus(&init1, &init2);
+	TEST_ASSERT(ret == 0,
+		    "Configuring 32bit EL1 vCPUs failed unexpectedly");
+
+	/* Test with mixed-width vCPUs  */
+	init1.features[0] = 0;
+	init2.features[0] = (1 << KVM_ARM_VCPU_EL1_32BIT);
+	ret = add_init_2vcpus(&init1, &init2);
+	TEST_ASSERT(ret != 0,
+		    "Configuring mixed-width vCPUs worked unexpectedly");
+	ret = add_2vcpus_init_2vcpus(&init1, &init2);
+	TEST_ASSERT(ret != 0,
+		    "Configuring mixed-width vCPUs worked unexpectedly");
+
+	return 0;
+}
-- 
2.35.1.894.gb6a874cedc-goog

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v5 2/2] KVM: arm64: selftests: Introduce vcpu_width_config
@ 2022-03-21  5:08   ` Reiji Watanabe
  0 siblings, 0 replies; 18+ messages in thread
From: Reiji Watanabe @ 2022-03-21  5:08 UTC (permalink / raw)
  To: Marc Zyngier, kvmarm
  Cc: kvm, linux-arm-kernel, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Andrew Jones, Paolo Bonzini, Will Deacon,
	Peter Shier, Ricardo Koller, Oliver Upton, Jing Zhang,
	Raghavendra Rao Anata, Reiji Watanabe

Introduce a test for aarch64 that ensures non-mixed-width vCPUs
(all 64bit vCPUs or all 32bit vcPUs) can be configured, and
mixed-width vCPUs cannot be configured.

Reviewed-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Reiji Watanabe <reijiw@google.com>
---
 tools/testing/selftests/kvm/.gitignore        |   1 +
 tools/testing/selftests/kvm/Makefile          |   1 +
 .../selftests/kvm/aarch64/vcpu_width_config.c | 125 ++++++++++++++++++
 3 files changed, 127 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/aarch64/vcpu_width_config.c

diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore
index dce7de7755e6..4e884e29b2a8 100644
--- a/tools/testing/selftests/kvm/.gitignore
+++ b/tools/testing/selftests/kvm/.gitignore
@@ -3,6 +3,7 @@
 /aarch64/debug-exceptions
 /aarch64/get-reg-list
 /aarch64/psci_cpu_on_test
+/aarch64/vcpu_width_config
 /aarch64/vgic_init
 /aarch64/vgic_irq
 /s390x/memop
diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index 0e4926bc9a58..06a5a982123e 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -104,6 +104,7 @@ 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
+TEST_GEN_PROGS_aarch64 += aarch64/vcpu_width_config
 TEST_GEN_PROGS_aarch64 += aarch64/vgic_init
 TEST_GEN_PROGS_aarch64 += aarch64/vgic_irq
 TEST_GEN_PROGS_aarch64 += demand_paging_test
diff --git a/tools/testing/selftests/kvm/aarch64/vcpu_width_config.c b/tools/testing/selftests/kvm/aarch64/vcpu_width_config.c
new file mode 100644
index 000000000000..6e6e6a9f69e3
--- /dev/null
+++ b/tools/testing/selftests/kvm/aarch64/vcpu_width_config.c
@@ -0,0 +1,125 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * vcpu_width_config - Test KVM_ARM_VCPU_INIT() with KVM_ARM_VCPU_EL1_32BIT.
+ *
+ * Copyright (c) 2022 Google LLC.
+ *
+ * This is a test that ensures that non-mixed-width vCPUs (all 64bit vCPUs
+ * or all 32bit vcPUs) can be configured and mixed-width vCPUs cannot be
+ * configured.
+ */
+
+#define _GNU_SOURCE
+
+#include "kvm_util.h"
+#include "processor.h"
+#include "test_util.h"
+
+
+/*
+ * Add a vCPU, run KVM_ARM_VCPU_INIT with @init1, and then
+ * add another vCPU, and run KVM_ARM_VCPU_INIT with @init2.
+ */
+static int add_init_2vcpus(struct kvm_vcpu_init *init1,
+			   struct kvm_vcpu_init *init2)
+{
+	struct kvm_vm *vm;
+	int ret;
+
+	vm = vm_create(VM_MODE_DEFAULT, DEFAULT_GUEST_PHY_PAGES, O_RDWR);
+
+	vm_vcpu_add(vm, 0);
+	ret = _vcpu_ioctl(vm, 0, KVM_ARM_VCPU_INIT, init1);
+	if (ret)
+		goto free_exit;
+
+	vm_vcpu_add(vm, 1);
+	ret = _vcpu_ioctl(vm, 1, KVM_ARM_VCPU_INIT, init2);
+
+free_exit:
+	kvm_vm_free(vm);
+	return ret;
+}
+
+/*
+ * Add two vCPUs, then run KVM_ARM_VCPU_INIT for one vCPU with @init1,
+ * and run KVM_ARM_VCPU_INIT for another vCPU with @init2.
+ */
+static int add_2vcpus_init_2vcpus(struct kvm_vcpu_init *init1,
+				  struct kvm_vcpu_init *init2)
+{
+	struct kvm_vm *vm;
+	int ret;
+
+	vm = vm_create(VM_MODE_DEFAULT, DEFAULT_GUEST_PHY_PAGES, O_RDWR);
+
+	vm_vcpu_add(vm, 0);
+	vm_vcpu_add(vm, 1);
+
+	ret = _vcpu_ioctl(vm, 0, KVM_ARM_VCPU_INIT, init1);
+	if (ret)
+		goto free_exit;
+
+	ret = _vcpu_ioctl(vm, 1, KVM_ARM_VCPU_INIT, init2);
+
+free_exit:
+	kvm_vm_free(vm);
+	return ret;
+}
+
+/*
+ * Tests that two 64bit vCPUs can be configured, two 32bit vCPUs can be
+ * configured, and two mixed-witgh vCPUs cannot be configured.
+ * Each of those three cases, configure vCPUs in two different orders.
+ * The one is running KVM_CREATE_VCPU for 2 vCPUs, and then running
+ * KVM_ARM_VCPU_INIT for them.
+ * The other is running KVM_CREATE_VCPU and KVM_ARM_VCPU_INIT for a vCPU,
+ * and then run those commands for another vCPU.
+ */
+int main(void)
+{
+	struct kvm_vcpu_init init1, init2;
+	struct kvm_vm *vm;
+	int ret;
+
+	if (kvm_check_cap(KVM_CAP_ARM_EL1_32BIT) <= 0) {
+		print_skip("KVM_CAP_ARM_EL1_32BIT is not supported");
+		exit(KSFT_SKIP);
+	}
+
+	/* Get the preferred target type and copy that to init2 */
+	vm = vm_create(VM_MODE_DEFAULT, DEFAULT_GUEST_PHY_PAGES, O_RDWR);
+	vm_ioctl(vm, KVM_ARM_PREFERRED_TARGET, &init1);
+	kvm_vm_free(vm);
+	memcpy(&init2, &init1, sizeof(init2));
+
+	/* Test with 64bit vCPUs */
+	ret = add_init_2vcpus(&init1, &init2);
+	TEST_ASSERT(ret == 0,
+		    "Configuring 64bit EL1 vCPUs failed unexpectedly");
+	ret = add_2vcpus_init_2vcpus(&init1, &init2);
+	TEST_ASSERT(ret == 0,
+		    "Configuring 64bit EL1 vCPUs failed unexpectedly");
+
+	/* Test with 32bit vCPUs */
+	init1.features[0] = (1 << KVM_ARM_VCPU_EL1_32BIT);
+	init2.features[0] = (1 << KVM_ARM_VCPU_EL1_32BIT);
+	ret = add_init_2vcpus(&init1, &init2);
+	TEST_ASSERT(ret == 0,
+		    "Configuring 32bit EL1 vCPUs failed unexpectedly");
+	ret = add_2vcpus_init_2vcpus(&init1, &init2);
+	TEST_ASSERT(ret == 0,
+		    "Configuring 32bit EL1 vCPUs failed unexpectedly");
+
+	/* Test with mixed-width vCPUs  */
+	init1.features[0] = 0;
+	init2.features[0] = (1 << KVM_ARM_VCPU_EL1_32BIT);
+	ret = add_init_2vcpus(&init1, &init2);
+	TEST_ASSERT(ret != 0,
+		    "Configuring mixed-width vCPUs worked unexpectedly");
+	ret = add_2vcpus_init_2vcpus(&init1, &init2);
+	TEST_ASSERT(ret != 0,
+		    "Configuring mixed-width vCPUs worked unexpectedly");
+
+	return 0;
+}
-- 
2.35.1.894.gb6a874cedc-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 2/2] KVM: arm64: selftests: Introduce vcpu_width_config
  2022-03-21  5:08   ` Reiji Watanabe
  (?)
@ 2022-03-21  6:17     ` Oliver Upton
  -1 siblings, 0 replies; 18+ messages in thread
From: Oliver Upton @ 2022-03-21  6:17 UTC (permalink / raw)
  To: Reiji Watanabe
  Cc: Marc Zyngier, kvmarm, kvm, linux-arm-kernel, James Morse,
	Alexandru Elisei, Suzuki K Poulose, Andrew Jones, Paolo Bonzini,
	Will Deacon, Peter Shier, Ricardo Koller, Jing Zhang,
	Raghavendra Rao Anata

Hi Reiji,

On Sun, Mar 20, 2022 at 10:08:04PM -0700, Reiji Watanabe wrote:
> Introduce a test for aarch64 that ensures non-mixed-width vCPUs
> (all 64bit vCPUs or all 32bit vcPUs) can be configured, and
> mixed-width vCPUs cannot be configured.
> 
> Reviewed-by: Andrew Jones <drjones@redhat.com>
> Signed-off-by: Reiji Watanabe <reijiw@google.com>

Tiny nits, but looks fine to me. Only bother addressing if you do
another spin, otherwise:

Reviewed-by: Oliver Upton <oupton@google.com>


> ---
>  tools/testing/selftests/kvm/.gitignore        |   1 +
>  tools/testing/selftests/kvm/Makefile          |   1 +
>  .../selftests/kvm/aarch64/vcpu_width_config.c | 125 ++++++++++++++++++
>  3 files changed, 127 insertions(+)
>  create mode 100644 tools/testing/selftests/kvm/aarch64/vcpu_width_config.c
> 
> diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore
> index dce7de7755e6..4e884e29b2a8 100644
> --- a/tools/testing/selftests/kvm/.gitignore
> +++ b/tools/testing/selftests/kvm/.gitignore
> @@ -3,6 +3,7 @@
>  /aarch64/debug-exceptions
>  /aarch64/get-reg-list
>  /aarch64/psci_cpu_on_test
> +/aarch64/vcpu_width_config
>  /aarch64/vgic_init
>  /aarch64/vgic_irq
>  /s390x/memop
> diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
> index 0e4926bc9a58..06a5a982123e 100644
> --- a/tools/testing/selftests/kvm/Makefile
> +++ b/tools/testing/selftests/kvm/Makefile
> @@ -104,6 +104,7 @@ 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
> +TEST_GEN_PROGS_aarch64 += aarch64/vcpu_width_config
>  TEST_GEN_PROGS_aarch64 += aarch64/vgic_init
>  TEST_GEN_PROGS_aarch64 += aarch64/vgic_irq
>  TEST_GEN_PROGS_aarch64 += demand_paging_test
> diff --git a/tools/testing/selftests/kvm/aarch64/vcpu_width_config.c b/tools/testing/selftests/kvm/aarch64/vcpu_width_config.c
> new file mode 100644
> index 000000000000..6e6e6a9f69e3
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/aarch64/vcpu_width_config.c
> @@ -0,0 +1,125 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * vcpu_width_config - Test KVM_ARM_VCPU_INIT() with KVM_ARM_VCPU_EL1_32BIT.
> + *
> + * Copyright (c) 2022 Google LLC.
> + *
> + * This is a test that ensures that non-mixed-width vCPUs (all 64bit vCPUs
> + * or all 32bit vcPUs) can be configured and mixed-width vCPUs cannot be
> + * configured.
> + */
> +
> +#define _GNU_SOURCE

In other instances where we define _GNU_SOURCE, it is said we do it for
program_invocation_short_name. Nonetheless, I cannot find anywhere that
the symbol is actually being used.

This looks to be some leftover crud from our internal test library
before we upstreamed KVM selftests a few years ago.

> +#include "kvm_util.h"
> +#include "processor.h"
> +#include "test_util.h"
> +
> +
> +/*
> + * Add a vCPU, run KVM_ARM_VCPU_INIT with @init1, and then
> + * add another vCPU, and run KVM_ARM_VCPU_INIT with @init2.
> + */
> +static int add_init_2vcpus(struct kvm_vcpu_init *init1,
> +			   struct kvm_vcpu_init *init2)
> +{
> +	struct kvm_vm *vm;
> +	int ret;
> +
> +	vm = vm_create(VM_MODE_DEFAULT, DEFAULT_GUEST_PHY_PAGES, O_RDWR);
> +
> +	vm_vcpu_add(vm, 0);
> +	ret = _vcpu_ioctl(vm, 0, KVM_ARM_VCPU_INIT, init1);
> +	if (ret)
> +		goto free_exit;
> +
> +	vm_vcpu_add(vm, 1);
> +	ret = _vcpu_ioctl(vm, 1, KVM_ARM_VCPU_INIT, init2);
> +
> +free_exit:
> +	kvm_vm_free(vm);
> +	return ret;
> +}
> +
> +/*
> + * Add two vCPUs, then run KVM_ARM_VCPU_INIT for one vCPU with @init1,
> + * and run KVM_ARM_VCPU_INIT for another vCPU with @init2.
> + */
> +static int add_2vcpus_init_2vcpus(struct kvm_vcpu_init *init1,
> +				  struct kvm_vcpu_init *init2)
> +{
> +	struct kvm_vm *vm;
> +	int ret;
> +
> +	vm = vm_create(VM_MODE_DEFAULT, DEFAULT_GUEST_PHY_PAGES, O_RDWR);
> +
> +	vm_vcpu_add(vm, 0);
> +	vm_vcpu_add(vm, 1);
> +
> +	ret = _vcpu_ioctl(vm, 0, KVM_ARM_VCPU_INIT, init1);
> +	if (ret)
> +		goto free_exit;
> +
> +	ret = _vcpu_ioctl(vm, 1, KVM_ARM_VCPU_INIT, init2);
> +
> +free_exit:
> +	kvm_vm_free(vm);
> +	return ret;
> +}
> +
> +/*
> + * Tests that two 64bit vCPUs can be configured, two 32bit vCPUs can be
> + * configured, and two mixed-witgh vCPUs cannot be configured.

mixed-width

> + * Each of those three cases, configure vCPUs in two different orders.
> + * The one is running KVM_CREATE_VCPU for 2 vCPUs, and then running
> + * KVM_ARM_VCPU_INIT for them.
> + * The other is running KVM_CREATE_VCPU and KVM_ARM_VCPU_INIT for a vCPU,
> + * and then run those commands for another vCPU.
> + */
> +int main(void)
> +{
> +	struct kvm_vcpu_init init1, init2;
> +	struct kvm_vm *vm;
> +	int ret;
> +
> +	if (kvm_check_cap(KVM_CAP_ARM_EL1_32BIT) <= 0) {

nit: this is fine, but KVM_CHECK_EXTENSION returns exactly 0 if a
capability is not supported.

> +		print_skip("KVM_CAP_ARM_EL1_32BIT is not supported");
> +		exit(KSFT_SKIP);
> +	}
> +
> +	/* Get the preferred target type and copy that to init2 */
> +	vm = vm_create(VM_MODE_DEFAULT, DEFAULT_GUEST_PHY_PAGES, O_RDWR);
> +	vm_ioctl(vm, KVM_ARM_PREFERRED_TARGET, &init1);
> +	kvm_vm_free(vm);
> +	memcpy(&init2, &init1, sizeof(init2));

nit: I think you can just assign init2 = init1.

> +	/* Test with 64bit vCPUs */
> +	ret = add_init_2vcpus(&init1, &init2);
> +	TEST_ASSERT(ret == 0,
> +		    "Configuring 64bit EL1 vCPUs failed unexpectedly");
> +	ret = add_2vcpus_init_2vcpus(&init1, &init2);

nit: for the homogeneous vCPU configuration tests, you could pass two
pointers to the same init structure to really drive the point home that
the vCPUs are the same. The underlying ioctl does not write back
anything to userspace.

> +	TEST_ASSERT(ret == 0,
> +		    "Configuring 64bit EL1 vCPUs failed unexpectedly");
> +
> +	/* Test with 32bit vCPUs */
> +	init1.features[0] = (1 << KVM_ARM_VCPU_EL1_32BIT);
> +	init2.features[0] = (1 << KVM_ARM_VCPU_EL1_32BIT);
> +	ret = add_init_2vcpus(&init1, &init2);
> +	TEST_ASSERT(ret == 0,
> +		    "Configuring 32bit EL1 vCPUs failed unexpectedly");
> +	ret = add_2vcpus_init_2vcpus(&init1, &init2);
> +	TEST_ASSERT(ret == 0,
> +		    "Configuring 32bit EL1 vCPUs failed unexpectedly");
> +
> +	/* Test with mixed-width vCPUs  */
> +	init1.features[0] = 0;
> +	init2.features[0] = (1 << KVM_ARM_VCPU_EL1_32BIT);
> +	ret = add_init_2vcpus(&init1, &init2);
> +	TEST_ASSERT(ret != 0,
> +		    "Configuring mixed-width vCPUs worked unexpectedly");
> +	ret = add_2vcpus_init_2vcpus(&init1, &init2);
> +	TEST_ASSERT(ret != 0,
> +		    "Configuring mixed-width vCPUs worked unexpectedly");
> +
> +	return 0;
> +}
> -- 
> 2.35.1.894.gb6a874cedc-goog
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 2/2] KVM: arm64: selftests: Introduce vcpu_width_config
@ 2022-03-21  6:17     ` Oliver Upton
  0 siblings, 0 replies; 18+ messages in thread
From: Oliver Upton @ 2022-03-21  6:17 UTC (permalink / raw)
  To: Reiji Watanabe
  Cc: kvm, Marc Zyngier, Peter Shier, Will Deacon, Paolo Bonzini,
	kvmarm, linux-arm-kernel

Hi Reiji,

On Sun, Mar 20, 2022 at 10:08:04PM -0700, Reiji Watanabe wrote:
> Introduce a test for aarch64 that ensures non-mixed-width vCPUs
> (all 64bit vCPUs or all 32bit vcPUs) can be configured, and
> mixed-width vCPUs cannot be configured.
> 
> Reviewed-by: Andrew Jones <drjones@redhat.com>
> Signed-off-by: Reiji Watanabe <reijiw@google.com>

Tiny nits, but looks fine to me. Only bother addressing if you do
another spin, otherwise:

Reviewed-by: Oliver Upton <oupton@google.com>


> ---
>  tools/testing/selftests/kvm/.gitignore        |   1 +
>  tools/testing/selftests/kvm/Makefile          |   1 +
>  .../selftests/kvm/aarch64/vcpu_width_config.c | 125 ++++++++++++++++++
>  3 files changed, 127 insertions(+)
>  create mode 100644 tools/testing/selftests/kvm/aarch64/vcpu_width_config.c
> 
> diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore
> index dce7de7755e6..4e884e29b2a8 100644
> --- a/tools/testing/selftests/kvm/.gitignore
> +++ b/tools/testing/selftests/kvm/.gitignore
> @@ -3,6 +3,7 @@
>  /aarch64/debug-exceptions
>  /aarch64/get-reg-list
>  /aarch64/psci_cpu_on_test
> +/aarch64/vcpu_width_config
>  /aarch64/vgic_init
>  /aarch64/vgic_irq
>  /s390x/memop
> diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
> index 0e4926bc9a58..06a5a982123e 100644
> --- a/tools/testing/selftests/kvm/Makefile
> +++ b/tools/testing/selftests/kvm/Makefile
> @@ -104,6 +104,7 @@ 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
> +TEST_GEN_PROGS_aarch64 += aarch64/vcpu_width_config
>  TEST_GEN_PROGS_aarch64 += aarch64/vgic_init
>  TEST_GEN_PROGS_aarch64 += aarch64/vgic_irq
>  TEST_GEN_PROGS_aarch64 += demand_paging_test
> diff --git a/tools/testing/selftests/kvm/aarch64/vcpu_width_config.c b/tools/testing/selftests/kvm/aarch64/vcpu_width_config.c
> new file mode 100644
> index 000000000000..6e6e6a9f69e3
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/aarch64/vcpu_width_config.c
> @@ -0,0 +1,125 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * vcpu_width_config - Test KVM_ARM_VCPU_INIT() with KVM_ARM_VCPU_EL1_32BIT.
> + *
> + * Copyright (c) 2022 Google LLC.
> + *
> + * This is a test that ensures that non-mixed-width vCPUs (all 64bit vCPUs
> + * or all 32bit vcPUs) can be configured and mixed-width vCPUs cannot be
> + * configured.
> + */
> +
> +#define _GNU_SOURCE

In other instances where we define _GNU_SOURCE, it is said we do it for
program_invocation_short_name. Nonetheless, I cannot find anywhere that
the symbol is actually being used.

This looks to be some leftover crud from our internal test library
before we upstreamed KVM selftests a few years ago.

> +#include "kvm_util.h"
> +#include "processor.h"
> +#include "test_util.h"
> +
> +
> +/*
> + * Add a vCPU, run KVM_ARM_VCPU_INIT with @init1, and then
> + * add another vCPU, and run KVM_ARM_VCPU_INIT with @init2.
> + */
> +static int add_init_2vcpus(struct kvm_vcpu_init *init1,
> +			   struct kvm_vcpu_init *init2)
> +{
> +	struct kvm_vm *vm;
> +	int ret;
> +
> +	vm = vm_create(VM_MODE_DEFAULT, DEFAULT_GUEST_PHY_PAGES, O_RDWR);
> +
> +	vm_vcpu_add(vm, 0);
> +	ret = _vcpu_ioctl(vm, 0, KVM_ARM_VCPU_INIT, init1);
> +	if (ret)
> +		goto free_exit;
> +
> +	vm_vcpu_add(vm, 1);
> +	ret = _vcpu_ioctl(vm, 1, KVM_ARM_VCPU_INIT, init2);
> +
> +free_exit:
> +	kvm_vm_free(vm);
> +	return ret;
> +}
> +
> +/*
> + * Add two vCPUs, then run KVM_ARM_VCPU_INIT for one vCPU with @init1,
> + * and run KVM_ARM_VCPU_INIT for another vCPU with @init2.
> + */
> +static int add_2vcpus_init_2vcpus(struct kvm_vcpu_init *init1,
> +				  struct kvm_vcpu_init *init2)
> +{
> +	struct kvm_vm *vm;
> +	int ret;
> +
> +	vm = vm_create(VM_MODE_DEFAULT, DEFAULT_GUEST_PHY_PAGES, O_RDWR);
> +
> +	vm_vcpu_add(vm, 0);
> +	vm_vcpu_add(vm, 1);
> +
> +	ret = _vcpu_ioctl(vm, 0, KVM_ARM_VCPU_INIT, init1);
> +	if (ret)
> +		goto free_exit;
> +
> +	ret = _vcpu_ioctl(vm, 1, KVM_ARM_VCPU_INIT, init2);
> +
> +free_exit:
> +	kvm_vm_free(vm);
> +	return ret;
> +}
> +
> +/*
> + * Tests that two 64bit vCPUs can be configured, two 32bit vCPUs can be
> + * configured, and two mixed-witgh vCPUs cannot be configured.

mixed-width

> + * Each of those three cases, configure vCPUs in two different orders.
> + * The one is running KVM_CREATE_VCPU for 2 vCPUs, and then running
> + * KVM_ARM_VCPU_INIT for them.
> + * The other is running KVM_CREATE_VCPU and KVM_ARM_VCPU_INIT for a vCPU,
> + * and then run those commands for another vCPU.
> + */
> +int main(void)
> +{
> +	struct kvm_vcpu_init init1, init2;
> +	struct kvm_vm *vm;
> +	int ret;
> +
> +	if (kvm_check_cap(KVM_CAP_ARM_EL1_32BIT) <= 0) {

nit: this is fine, but KVM_CHECK_EXTENSION returns exactly 0 if a
capability is not supported.

> +		print_skip("KVM_CAP_ARM_EL1_32BIT is not supported");
> +		exit(KSFT_SKIP);
> +	}
> +
> +	/* Get the preferred target type and copy that to init2 */
> +	vm = vm_create(VM_MODE_DEFAULT, DEFAULT_GUEST_PHY_PAGES, O_RDWR);
> +	vm_ioctl(vm, KVM_ARM_PREFERRED_TARGET, &init1);
> +	kvm_vm_free(vm);
> +	memcpy(&init2, &init1, sizeof(init2));

nit: I think you can just assign init2 = init1.

> +	/* Test with 64bit vCPUs */
> +	ret = add_init_2vcpus(&init1, &init2);
> +	TEST_ASSERT(ret == 0,
> +		    "Configuring 64bit EL1 vCPUs failed unexpectedly");
> +	ret = add_2vcpus_init_2vcpus(&init1, &init2);

nit: for the homogeneous vCPU configuration tests, you could pass two
pointers to the same init structure to really drive the point home that
the vCPUs are the same. The underlying ioctl does not write back
anything to userspace.

> +	TEST_ASSERT(ret == 0,
> +		    "Configuring 64bit EL1 vCPUs failed unexpectedly");
> +
> +	/* Test with 32bit vCPUs */
> +	init1.features[0] = (1 << KVM_ARM_VCPU_EL1_32BIT);
> +	init2.features[0] = (1 << KVM_ARM_VCPU_EL1_32BIT);
> +	ret = add_init_2vcpus(&init1, &init2);
> +	TEST_ASSERT(ret == 0,
> +		    "Configuring 32bit EL1 vCPUs failed unexpectedly");
> +	ret = add_2vcpus_init_2vcpus(&init1, &init2);
> +	TEST_ASSERT(ret == 0,
> +		    "Configuring 32bit EL1 vCPUs failed unexpectedly");
> +
> +	/* Test with mixed-width vCPUs  */
> +	init1.features[0] = 0;
> +	init2.features[0] = (1 << KVM_ARM_VCPU_EL1_32BIT);
> +	ret = add_init_2vcpus(&init1, &init2);
> +	TEST_ASSERT(ret != 0,
> +		    "Configuring mixed-width vCPUs worked unexpectedly");
> +	ret = add_2vcpus_init_2vcpus(&init1, &init2);
> +	TEST_ASSERT(ret != 0,
> +		    "Configuring mixed-width vCPUs worked unexpectedly");
> +
> +	return 0;
> +}
> -- 
> 2.35.1.894.gb6a874cedc-goog
> 
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 2/2] KVM: arm64: selftests: Introduce vcpu_width_config
@ 2022-03-21  6:17     ` Oliver Upton
  0 siblings, 0 replies; 18+ messages in thread
From: Oliver Upton @ 2022-03-21  6:17 UTC (permalink / raw)
  To: Reiji Watanabe
  Cc: Marc Zyngier, kvmarm, kvm, linux-arm-kernel, James Morse,
	Alexandru Elisei, Suzuki K Poulose, Andrew Jones, Paolo Bonzini,
	Will Deacon, Peter Shier, Ricardo Koller, Jing Zhang,
	Raghavendra Rao Anata

Hi Reiji,

On Sun, Mar 20, 2022 at 10:08:04PM -0700, Reiji Watanabe wrote:
> Introduce a test for aarch64 that ensures non-mixed-width vCPUs
> (all 64bit vCPUs or all 32bit vcPUs) can be configured, and
> mixed-width vCPUs cannot be configured.
> 
> Reviewed-by: Andrew Jones <drjones@redhat.com>
> Signed-off-by: Reiji Watanabe <reijiw@google.com>

Tiny nits, but looks fine to me. Only bother addressing if you do
another spin, otherwise:

Reviewed-by: Oliver Upton <oupton@google.com>


> ---
>  tools/testing/selftests/kvm/.gitignore        |   1 +
>  tools/testing/selftests/kvm/Makefile          |   1 +
>  .../selftests/kvm/aarch64/vcpu_width_config.c | 125 ++++++++++++++++++
>  3 files changed, 127 insertions(+)
>  create mode 100644 tools/testing/selftests/kvm/aarch64/vcpu_width_config.c
> 
> diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore
> index dce7de7755e6..4e884e29b2a8 100644
> --- a/tools/testing/selftests/kvm/.gitignore
> +++ b/tools/testing/selftests/kvm/.gitignore
> @@ -3,6 +3,7 @@
>  /aarch64/debug-exceptions
>  /aarch64/get-reg-list
>  /aarch64/psci_cpu_on_test
> +/aarch64/vcpu_width_config
>  /aarch64/vgic_init
>  /aarch64/vgic_irq
>  /s390x/memop
> diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
> index 0e4926bc9a58..06a5a982123e 100644
> --- a/tools/testing/selftests/kvm/Makefile
> +++ b/tools/testing/selftests/kvm/Makefile
> @@ -104,6 +104,7 @@ 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
> +TEST_GEN_PROGS_aarch64 += aarch64/vcpu_width_config
>  TEST_GEN_PROGS_aarch64 += aarch64/vgic_init
>  TEST_GEN_PROGS_aarch64 += aarch64/vgic_irq
>  TEST_GEN_PROGS_aarch64 += demand_paging_test
> diff --git a/tools/testing/selftests/kvm/aarch64/vcpu_width_config.c b/tools/testing/selftests/kvm/aarch64/vcpu_width_config.c
> new file mode 100644
> index 000000000000..6e6e6a9f69e3
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/aarch64/vcpu_width_config.c
> @@ -0,0 +1,125 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * vcpu_width_config - Test KVM_ARM_VCPU_INIT() with KVM_ARM_VCPU_EL1_32BIT.
> + *
> + * Copyright (c) 2022 Google LLC.
> + *
> + * This is a test that ensures that non-mixed-width vCPUs (all 64bit vCPUs
> + * or all 32bit vcPUs) can be configured and mixed-width vCPUs cannot be
> + * configured.
> + */
> +
> +#define _GNU_SOURCE

In other instances where we define _GNU_SOURCE, it is said we do it for
program_invocation_short_name. Nonetheless, I cannot find anywhere that
the symbol is actually being used.

This looks to be some leftover crud from our internal test library
before we upstreamed KVM selftests a few years ago.

> +#include "kvm_util.h"
> +#include "processor.h"
> +#include "test_util.h"
> +
> +
> +/*
> + * Add a vCPU, run KVM_ARM_VCPU_INIT with @init1, and then
> + * add another vCPU, and run KVM_ARM_VCPU_INIT with @init2.
> + */
> +static int add_init_2vcpus(struct kvm_vcpu_init *init1,
> +			   struct kvm_vcpu_init *init2)
> +{
> +	struct kvm_vm *vm;
> +	int ret;
> +
> +	vm = vm_create(VM_MODE_DEFAULT, DEFAULT_GUEST_PHY_PAGES, O_RDWR);
> +
> +	vm_vcpu_add(vm, 0);
> +	ret = _vcpu_ioctl(vm, 0, KVM_ARM_VCPU_INIT, init1);
> +	if (ret)
> +		goto free_exit;
> +
> +	vm_vcpu_add(vm, 1);
> +	ret = _vcpu_ioctl(vm, 1, KVM_ARM_VCPU_INIT, init2);
> +
> +free_exit:
> +	kvm_vm_free(vm);
> +	return ret;
> +}
> +
> +/*
> + * Add two vCPUs, then run KVM_ARM_VCPU_INIT for one vCPU with @init1,
> + * and run KVM_ARM_VCPU_INIT for another vCPU with @init2.
> + */
> +static int add_2vcpus_init_2vcpus(struct kvm_vcpu_init *init1,
> +				  struct kvm_vcpu_init *init2)
> +{
> +	struct kvm_vm *vm;
> +	int ret;
> +
> +	vm = vm_create(VM_MODE_DEFAULT, DEFAULT_GUEST_PHY_PAGES, O_RDWR);
> +
> +	vm_vcpu_add(vm, 0);
> +	vm_vcpu_add(vm, 1);
> +
> +	ret = _vcpu_ioctl(vm, 0, KVM_ARM_VCPU_INIT, init1);
> +	if (ret)
> +		goto free_exit;
> +
> +	ret = _vcpu_ioctl(vm, 1, KVM_ARM_VCPU_INIT, init2);
> +
> +free_exit:
> +	kvm_vm_free(vm);
> +	return ret;
> +}
> +
> +/*
> + * Tests that two 64bit vCPUs can be configured, two 32bit vCPUs can be
> + * configured, and two mixed-witgh vCPUs cannot be configured.

mixed-width

> + * Each of those three cases, configure vCPUs in two different orders.
> + * The one is running KVM_CREATE_VCPU for 2 vCPUs, and then running
> + * KVM_ARM_VCPU_INIT for them.
> + * The other is running KVM_CREATE_VCPU and KVM_ARM_VCPU_INIT for a vCPU,
> + * and then run those commands for another vCPU.
> + */
> +int main(void)
> +{
> +	struct kvm_vcpu_init init1, init2;
> +	struct kvm_vm *vm;
> +	int ret;
> +
> +	if (kvm_check_cap(KVM_CAP_ARM_EL1_32BIT) <= 0) {

nit: this is fine, but KVM_CHECK_EXTENSION returns exactly 0 if a
capability is not supported.

> +		print_skip("KVM_CAP_ARM_EL1_32BIT is not supported");
> +		exit(KSFT_SKIP);
> +	}
> +
> +	/* Get the preferred target type and copy that to init2 */
> +	vm = vm_create(VM_MODE_DEFAULT, DEFAULT_GUEST_PHY_PAGES, O_RDWR);
> +	vm_ioctl(vm, KVM_ARM_PREFERRED_TARGET, &init1);
> +	kvm_vm_free(vm);
> +	memcpy(&init2, &init1, sizeof(init2));

nit: I think you can just assign init2 = init1.

> +	/* Test with 64bit vCPUs */
> +	ret = add_init_2vcpus(&init1, &init2);
> +	TEST_ASSERT(ret == 0,
> +		    "Configuring 64bit EL1 vCPUs failed unexpectedly");
> +	ret = add_2vcpus_init_2vcpus(&init1, &init2);

nit: for the homogeneous vCPU configuration tests, you could pass two
pointers to the same init structure to really drive the point home that
the vCPUs are the same. The underlying ioctl does not write back
anything to userspace.

> +	TEST_ASSERT(ret == 0,
> +		    "Configuring 64bit EL1 vCPUs failed unexpectedly");
> +
> +	/* Test with 32bit vCPUs */
> +	init1.features[0] = (1 << KVM_ARM_VCPU_EL1_32BIT);
> +	init2.features[0] = (1 << KVM_ARM_VCPU_EL1_32BIT);
> +	ret = add_init_2vcpus(&init1, &init2);
> +	TEST_ASSERT(ret == 0,
> +		    "Configuring 32bit EL1 vCPUs failed unexpectedly");
> +	ret = add_2vcpus_init_2vcpus(&init1, &init2);
> +	TEST_ASSERT(ret == 0,
> +		    "Configuring 32bit EL1 vCPUs failed unexpectedly");
> +
> +	/* Test with mixed-width vCPUs  */
> +	init1.features[0] = 0;
> +	init2.features[0] = (1 << KVM_ARM_VCPU_EL1_32BIT);
> +	ret = add_init_2vcpus(&init1, &init2);
> +	TEST_ASSERT(ret != 0,
> +		    "Configuring mixed-width vCPUs worked unexpectedly");
> +	ret = add_2vcpus_init_2vcpus(&init1, &init2);
> +	TEST_ASSERT(ret != 0,
> +		    "Configuring mixed-width vCPUs worked unexpectedly");
> +
> +	return 0;
> +}
> -- 
> 2.35.1.894.gb6a874cedc-goog
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 2/2] KVM: arm64: selftests: Introduce vcpu_width_config
  2022-03-21  6:17     ` Oliver Upton
  (?)
@ 2022-03-21  8:28       ` Oliver Upton
  -1 siblings, 0 replies; 18+ messages in thread
From: Oliver Upton @ 2022-03-21  8:28 UTC (permalink / raw)
  To: Reiji Watanabe
  Cc: Marc Zyngier, kvmarm, kvm, linux-arm-kernel, James Morse,
	Alexandru Elisei, Suzuki K Poulose, Andrew Jones, Paolo Bonzini,
	Will Deacon, Peter Shier, Ricardo Koller, Jing Zhang,
	Raghavendra Rao Anata

On Mon, Mar 21, 2022 at 06:17:43AM +0000, Oliver Upton wrote:
> Hi Reiji,
> 
> On Sun, Mar 20, 2022 at 10:08:04PM -0700, Reiji Watanabe wrote:

[...]

> > +#define _GNU_SOURCE
> 
> In other instances where we define _GNU_SOURCE, it is said we do it for
> program_invocation_short_name. Nonetheless, I cannot find anywhere that
> the symbol is actually being used.
> 
> This looks to be some leftover crud from our internal test library
> before we upstreamed KVM selftests a few years ago.
>

Ah, it's because we're actually using program_invocation_name. This
already gets defined in lib/kvm_util.c, so except for a few oddball
tests that directly call kvm_vm_elf_load(), this is unnecessary.

--
Thanks,
Oliver

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 2/2] KVM: arm64: selftests: Introduce vcpu_width_config
@ 2022-03-21  8:28       ` Oliver Upton
  0 siblings, 0 replies; 18+ messages in thread
From: Oliver Upton @ 2022-03-21  8:28 UTC (permalink / raw)
  To: Reiji Watanabe
  Cc: kvm, Marc Zyngier, Peter Shier, Will Deacon, Paolo Bonzini,
	kvmarm, linux-arm-kernel

On Mon, Mar 21, 2022 at 06:17:43AM +0000, Oliver Upton wrote:
> Hi Reiji,
> 
> On Sun, Mar 20, 2022 at 10:08:04PM -0700, Reiji Watanabe wrote:

[...]

> > +#define _GNU_SOURCE
> 
> In other instances where we define _GNU_SOURCE, it is said we do it for
> program_invocation_short_name. Nonetheless, I cannot find anywhere that
> the symbol is actually being used.
> 
> This looks to be some leftover crud from our internal test library
> before we upstreamed KVM selftests a few years ago.
>

Ah, it's because we're actually using program_invocation_name. This
already gets defined in lib/kvm_util.c, so except for a few oddball
tests that directly call kvm_vm_elf_load(), this is unnecessary.

--
Thanks,
Oliver
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 2/2] KVM: arm64: selftests: Introduce vcpu_width_config
@ 2022-03-21  8:28       ` Oliver Upton
  0 siblings, 0 replies; 18+ messages in thread
From: Oliver Upton @ 2022-03-21  8:28 UTC (permalink / raw)
  To: Reiji Watanabe
  Cc: Marc Zyngier, kvmarm, kvm, linux-arm-kernel, James Morse,
	Alexandru Elisei, Suzuki K Poulose, Andrew Jones, Paolo Bonzini,
	Will Deacon, Peter Shier, Ricardo Koller, Jing Zhang,
	Raghavendra Rao Anata

On Mon, Mar 21, 2022 at 06:17:43AM +0000, Oliver Upton wrote:
> Hi Reiji,
> 
> On Sun, Mar 20, 2022 at 10:08:04PM -0700, Reiji Watanabe wrote:

[...]

> > +#define _GNU_SOURCE
> 
> In other instances where we define _GNU_SOURCE, it is said we do it for
> program_invocation_short_name. Nonetheless, I cannot find anywhere that
> the symbol is actually being used.
> 
> This looks to be some leftover crud from our internal test library
> before we upstreamed KVM selftests a few years ago.
>

Ah, it's because we're actually using program_invocation_name. This
already gets defined in lib/kvm_util.c, so except for a few oddball
tests that directly call kvm_vm_elf_load(), this is unnecessary.

--
Thanks,
Oliver

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 2/2] KVM: arm64: selftests: Introduce vcpu_width_config
  2022-03-21  6:17     ` Oliver Upton
  (?)
@ 2022-03-22  3:30       ` Reiji Watanabe
  -1 siblings, 0 replies; 18+ messages in thread
From: Reiji Watanabe @ 2022-03-22  3:30 UTC (permalink / raw)
  To: Oliver Upton
  Cc: Marc Zyngier, kvmarm, kvm, Linux ARM, James Morse,
	Alexandru Elisei, Suzuki K Poulose, Andrew Jones, Paolo Bonzini,
	Will Deacon, Peter Shier, Ricardo Koller, Jing Zhang,
	Raghavendra Rao Anata

Hi Oliver,

On Sun, Mar 20, 2022 at 11:17 PM Oliver Upton <oupton@google.com> wrote:
>
> Hi Reiji,
>
> On Sun, Mar 20, 2022 at 10:08:04PM -0700, Reiji Watanabe wrote:
> > Introduce a test for aarch64 that ensures non-mixed-width vCPUs
> > (all 64bit vCPUs or all 32bit vcPUs) can be configured, and
> > mixed-width vCPUs cannot be configured.
> >
> > Reviewed-by: Andrew Jones <drjones@redhat.com>
> > Signed-off-by: Reiji Watanabe <reijiw@google.com>
>
> Tiny nits, but looks fine to me. Only bother addressing if you do
> another spin, otherwise:
>
> Reviewed-by: Oliver Upton <oupton@google.com>

Thank you for the review!
Since I would like to fix one of your comments (the typo) at least,
I will respin the series (and will address all the comments).

Thanks,
Reiji

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 2/2] KVM: arm64: selftests: Introduce vcpu_width_config
@ 2022-03-22  3:30       ` Reiji Watanabe
  0 siblings, 0 replies; 18+ messages in thread
From: Reiji Watanabe @ 2022-03-22  3:30 UTC (permalink / raw)
  To: Oliver Upton
  Cc: kvm, Marc Zyngier, Peter Shier, Will Deacon, Paolo Bonzini,
	kvmarm, Linux ARM

Hi Oliver,

On Sun, Mar 20, 2022 at 11:17 PM Oliver Upton <oupton@google.com> wrote:
>
> Hi Reiji,
>
> On Sun, Mar 20, 2022 at 10:08:04PM -0700, Reiji Watanabe wrote:
> > Introduce a test for aarch64 that ensures non-mixed-width vCPUs
> > (all 64bit vCPUs or all 32bit vcPUs) can be configured, and
> > mixed-width vCPUs cannot be configured.
> >
> > Reviewed-by: Andrew Jones <drjones@redhat.com>
> > Signed-off-by: Reiji Watanabe <reijiw@google.com>
>
> Tiny nits, but looks fine to me. Only bother addressing if you do
> another spin, otherwise:
>
> Reviewed-by: Oliver Upton <oupton@google.com>

Thank you for the review!
Since I would like to fix one of your comments (the typo) at least,
I will respin the series (and will address all the comments).

Thanks,
Reiji
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 2/2] KVM: arm64: selftests: Introduce vcpu_width_config
@ 2022-03-22  3:30       ` Reiji Watanabe
  0 siblings, 0 replies; 18+ messages in thread
From: Reiji Watanabe @ 2022-03-22  3:30 UTC (permalink / raw)
  To: Oliver Upton
  Cc: Marc Zyngier, kvmarm, kvm, Linux ARM, James Morse,
	Alexandru Elisei, Suzuki K Poulose, Andrew Jones, Paolo Bonzini,
	Will Deacon, Peter Shier, Ricardo Koller, Jing Zhang,
	Raghavendra Rao Anata

Hi Oliver,

On Sun, Mar 20, 2022 at 11:17 PM Oliver Upton <oupton@google.com> wrote:
>
> Hi Reiji,
>
> On Sun, Mar 20, 2022 at 10:08:04PM -0700, Reiji Watanabe wrote:
> > Introduce a test for aarch64 that ensures non-mixed-width vCPUs
> > (all 64bit vCPUs or all 32bit vcPUs) can be configured, and
> > mixed-width vCPUs cannot be configured.
> >
> > Reviewed-by: Andrew Jones <drjones@redhat.com>
> > Signed-off-by: Reiji Watanabe <reijiw@google.com>
>
> Tiny nits, but looks fine to me. Only bother addressing if you do
> another spin, otherwise:
>
> Reviewed-by: Oliver Upton <oupton@google.com>

Thank you for the review!
Since I would like to fix one of your comments (the typo) at least,
I will respin the series (and will address all the comments).

Thanks,
Reiji

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2022-03-22  3:32 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-21  5:08 [PATCH v5 0/2] KVM: arm64: mixed-width check should be skipped for uninitialized vCPUs Reiji Watanabe
2022-03-21  5:08 ` Reiji Watanabe
2022-03-21  5:08 ` Reiji Watanabe
2022-03-21  5:08 ` [PATCH v5 1/2] " Reiji Watanabe
2022-03-21  5:08   ` Reiji Watanabe
2022-03-21  5:08   ` Reiji Watanabe
2022-03-21  5:08 ` [PATCH v5 2/2] KVM: arm64: selftests: Introduce vcpu_width_config Reiji Watanabe
2022-03-21  5:08   ` Reiji Watanabe
2022-03-21  5:08   ` Reiji Watanabe
2022-03-21  6:17   ` Oliver Upton
2022-03-21  6:17     ` Oliver Upton
2022-03-21  6:17     ` Oliver Upton
2022-03-21  8:28     ` Oliver Upton
2022-03-21  8:28       ` Oliver Upton
2022-03-21  8:28       ` Oliver Upton
2022-03-22  3:30     ` Reiji Watanabe
2022-03-22  3:30       ` Reiji Watanabe
2022-03-22  3:30       ` Reiji Watanabe

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.