Skip to contentSkip to conversation

KVM: arm64: Report SMCCC_VERSION and SMCCC_ARCH_FEATURES as implemented

PATCH Applied Fuad Tabba 2 participants 3 messages Sep 14 – Sep 27

Subsystems (4)
KVM Kselftest arm64 KVM arm64
lore
Share & tools
lore

Fuad Tabba

author reviewer
#0 lore raw

PATCH 1/2 KVM: arm64: Report SMCCC_VERSION and SMCCC_ARCH_FEATURES as implemented

KVM reports SMCCC v1.1 to guests, which makes SMCCC_VERSION and SMCCC_ARCH_FEATURES mandatory, and DEN0028H 7.3.6 requires SMCCC_ARCH_FEATURES to return SUCCESS for both function ids: "This function must return SUCCESS when arch_func_id is the SMCCC_VERSION or SMCCC_ARCH_FEATURES function id." With arch_func_id set to either, KVM's guest handler returns NOT_SUPPORTED. Return SUCCESS.

A Linux guest is unaffected: it discovers SMCCC_VERSION through PSCI_FEATURES, and issues SMCCC_ARCH_FEATURES only with arch_func_id set to ARCH_SOC_ID, the ARCH_WORKAROUND_* ids or HV_PV_TIME_FEATURES.

Signed-off-by Fuad Tabba <[email protected]>
---
 arch/arm64/kvm/hypercalls.c | 4 ++++
 1 file changed, 4 insertions(+)
arch/arm64/kvm/hypercalls.c +4 -0
index b11b8821c9fbc..66f16bf9a8ea7 100644
@@ -293,6 +293,10 @@ int kvm_smccc_call_handler(struct kvm_vcpu *vcpu)
 	case ARM_SMCCC_ARCH_FEATURES_FUNC_ID:
 		feature = smccc_get_arg1(vcpu);
 		switch (feature) {
+		case ARM_SMCCC_VERSION_FUNC_ID:
+		case ARM_SMCCC_ARCH_FEATURES_FUNC_ID:
+			val[0] = SMCCC_RET_SUCCESS;
+			break;
 		case ARM_SMCCC_ARCH_WORKAROUND_1:
 			switch (arm64_get_spectre_v2_state()) {
 			case SPECTRE_VULNERABLE:
Signature · 1 line
2.39.5
author reviewer
#1 lore raw

PATCH 2/2 KVM: arm64: selftests: Check the mandatory SMCCC_ARCH_FEATURES queries

tools/testing/selftests/kvm/arm64/hypercalls.c checks SMCCC_ARCH_FEATURES with two expectations: hvc_info[] expects NOT_SUPPORTED with the pseudo-firmware bitmaps cleared and something else with them set, and false_hvc_info[] expects NOT_SUPPORTED for mismatched pairs. DEN0028H 7.3.6 mandates SUCCESS for arch_func_id SMCCC_VERSION and SMCCC_ARCH_FEATURES itself, whatever the bitmaps hold, which neither expectation allows.

Check those two in the feature-disabled and feature-enabled stages, so the answer is asserted with the bitmaps cleared and again at their reset values. They pass only with the preceding patch applied.

Signed-off-by Fuad Tabba <[email protected]>
---
 .../testing/selftests/kvm/arm64/hypercalls.c  | 27 ++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)
tools/testing/selftests/kvm/arm64/hypercalls.c +26 -1
index 5d96cdf382c47..e74410cef0fe0 100644
@@ -6,7 +6,8 @@
  * via the psuedo-firmware bitmap register. This includes the registers'
  * read/write behavior before and after the VM has started, and if the
  * hypercalls are properly masked or unmasked to the guest when disabled or
- * enabled from the KVM userspace, respectively.
+ * enabled from the KVM userspace, respectively. It also checks the two
+ * SMCCC_ARCH_FEATURES queries whose answer SMCCC pins to SUCCESS.
  */
 #include <errno.h>
 #include <linux/arm-smccc.h>

@@ -97,6 +98,12 @@ static const struct test_hvc_info false_hvc_info[] = {
 	TEST_HVC_INFO(ARM_SMCCC_HV_PV_TIME_FEATURES, ARM_SMCCC_TRNG_RND64),
 };
 
+/* SMCCC pins the answer to SUCCESS, whatever the bitmaps hold. */
+static const struct test_hvc_info always_hvc_info[] = {
+	TEST_HVC_INFO(ARM_SMCCC_ARCH_FEATURES_FUNC_ID, ARM_SMCCC_VERSION_FUNC_ID),
+	TEST_HVC_INFO(ARM_SMCCC_ARCH_FEATURES_FUNC_ID, ARM_SMCCC_ARCH_FEATURES_FUNC_ID),
+};
+
 static void guest_test_hvc(const struct test_hvc_info *hc_info)
 {
 	unsigned int i;

@@ -128,6 +135,23 @@ static void guest_test_hvc(const struct test_hvc_info *hc_info)
 	}
 }
 
+static void guest_test_always_hvc(void)
+{
+	struct arm_smccc_res res;
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(always_hvc_info); i++) {
+		memset(&res, 0, sizeof(res));
+		do_smccc(always_hvc_info[i].func_id, always_hvc_info[i].arg1,
+			 0, 0, 0, 0, 0, 0, &res);
+
+		__GUEST_ASSERT(res.a0 == SMCCC_RET_SUCCESS,
+			       "a0 = 0x%lx, func_id = 0x%x, arg1 = 0x%lx, stage = %u",
+			       res.a0, always_hvc_info[i].func_id,
+			       always_hvc_info[i].arg1, stage);
+	}
+}
+
 static void guest_code(void)
 {
 	while (stage != TEST_STAGE_END) {

@@ -137,6 +161,7 @@ static void guest_code(void)
 		case TEST_STAGE_HVC_IFACE_FEAT_DISABLED:
 		case TEST_STAGE_HVC_IFACE_FEAT_ENABLED:
 			guest_test_hvc(hvc_info);
+			guest_test_always_hvc();
 			break;
 		case TEST_STAGE_HVC_IFACE_FALSE_INFO:
 			guest_test_hvc(false_hvc_info);
Signature · 1 line
2.39.5
maintainer Applied
#2 lore raw
In reply to Fuad Tabba
Quoted 8 lines · Fuad Tabba

KVM reports SMCCC v1.1 to guests, which makes SMCCC_VERSION and SMCCC_ARCH_FEATURES mandatory, and DEN0028H 7.3.6 requires SMCCC_ARCH_FEATURES to return SUCCESS for both function ids: "This function must return SUCCESS when arch_func_id is the SMCCC_VERSION or SMCCC_ARCH_FEATURES function id." With arch_func_id set to either, KVM's guest handler returns NOT_SUPPORTED. Return SUCCESS.

[...]

Applied to next, thanks!

[1/2] KVM: arm64: Report SMCCC_VERSION and SMCCC_ARCH_FEATURES as implemented

      commit: 7ef1b9e11dd7b3832124e26a6fee703b342d6f7f
[2/2] KVM: arm64: selftests: Check the mandatory SMCCC_ARCH_FEATURES queries
      commit: 67d00fcc00761e9fe4debeac781eb4f992dd8506

Cheers,

	M.
Signature · 1 line
Without deviation from the norm, progress is not possible.