Re: [PATCH v16 1/9] arm64: Probe for the presence of KVM hypervisor

2021-02-02 Thread Marc Zyngier

On 2020-12-09 06:09, Jianyong Wu wrote:

From: Will Deacon 

Although the SMCCC specification provides some limited functionality 
for

describing the presence of hypervisor and firmware services, this is
generally applicable only to functions designated as "Arm Architecture
Service Functions" and no portable discovery mechanism is provided for
standard hypervisor services, despite having a designated range of
function identifiers reserved by the specification.

In an attempt to avoid the need for additional firmware changes every
time a new function is added, introduce a UID to identify the service
provider as being compatible with KVM. Once this has been established,
additional services can be discovered via a feature bitmap.

Change from Jianyong Wu:
mv kvm_arm_hyp_service_available to common place to let both arm/arm64 
touch it.

add kvm_init_hyp_services also under arm arch to let arm kvm guest use
this service.

Cc: Marc Zyngier 
Signed-off-by: Will Deacon 
Signed-off-by: Jianyong Wu 
---
 arch/arm/kernel/setup.c|  5 
 arch/arm64/kernel/setup.c  |  1 +
 drivers/firmware/smccc/smccc.c | 37 +
 include/linux/arm-smccc.h  | 43 ++
 4 files changed, 86 insertions(+)

diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index 1a5edf562e85..adcefa9c8fab 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -1156,6 +1156,11 @@ void __init setup_arch(char **cmdline_p)

arm_dt_init_cpu_maps();
psci_dt_init();
+
+#ifdef CONFIG_HAVE_ARM_SMCCC_DISCOVERY
+   kvm_init_hyp_services();
+#endif
+
 #ifdef CONFIG_SMP
if (is_smp()) {
if (!mdesc->smp_init || !mdesc->smp_init()) {
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index a950d5bc1ba5..97037b15c6ea 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -353,6 +353,7 @@ void __init __no_sanitize_address setup_arch(char
**cmdline_p)
else
psci_acpi_init();

+   kvm_init_hyp_services();


Given that there is a dependency between the KVM discovery and PSCI,
it may make more sense to directly call this from the PSCI init,
and leave the arch-specific code alone.


init_bootcpu_ops();
smp_init_cpus();
smp_build_mpidr_hash();
diff --git a/drivers/firmware/smccc/smccc.c 
b/drivers/firmware/smccc/smccc.c

index 00c88b809c0c..e153c71ece99 100644
--- a/drivers/firmware/smccc/smccc.c
+++ b/drivers/firmware/smccc/smccc.c
@@ -7,10 +7,47 @@

 #include 
 #include 
+#include 
+#include 

 static u32 smccc_version = ARM_SMCCC_VERSION_1_0;
 static enum arm_smccc_conduit smccc_conduit = SMCCC_CONDUIT_NONE;

+DECLARE_BITMAP(__kvm_arm_hyp_services, ARM_SMCCC_KVM_NUM_FUNCS) = { };
+EXPORT_SYMBOL_GPL(__kvm_arm_hyp_services);
+
+void __init kvm_init_hyp_services(void)
+{
+   int i;
+   struct arm_smccc_res res;
+
+   if (arm_smccc_get_version() == ARM_SMCCC_VERSION_1_0)
+   return;
+
+   arm_smccc_1_1_invoke(ARM_SMCCC_VENDOR_HYP_CALL_UID_FUNC_ID, );


I think we may end-up calling into EL3 when running this on bare
metal if the systems implement PSCI 1,1. Robust firmware should
handle it, but there is plenty of broken ones around...

Checking on the SMCCC conduit would be safer.


+   if (res.a0 != ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_0 ||
+   res.a1 != ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_1 ||
+   res.a2 != ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_2 ||
+   res.a3 != ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_3)
+   return;
+
+   memset(, 0, sizeof(res));
+	arm_smccc_1_1_invoke(ARM_SMCCC_VENDOR_HYP_KVM_FEATURES_FUNC_ID, 
);

+   for (i = 0; i < 32; ++i) {
+   if (res.a0 & (i))
+   set_bit(i + (32 * 0), __kvm_arm_hyp_services);
+   if (res.a1 & (i))
+   set_bit(i + (32 * 1), __kvm_arm_hyp_services);
+   if (res.a2 & (i))
+   set_bit(i + (32 * 2), __kvm_arm_hyp_services);
+   if (res.a3 & (i))
+   set_bit(i + (32 * 3), __kvm_arm_hyp_services);
+   }
+
+   pr_info("KVM hypervisor services detected (0x%08lx 0x%08lx 0x%08lx
0x%08lx)\n",
+res.a3, res.a2, res.a1, res.a0);
+}
+


Overall, this code should be in its own file, much like we already
do for the SOC_ID stuff.

 void __init arm_smccc_version_init(u32 version, enum arm_smccc_conduit 
conduit)

 {
smccc_version = version;
diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
index f860645f6512..d75408141137 100644
--- a/include/linux/arm-smccc.h
+++ b/include/linux/arm-smccc.h
@@ -55,6 +55,8 @@
 #define ARM_SMCCC_OWNER_TRUSTED_OS 50
 #define ARM_SMCCC_OWNER_TRUSTED_OS_END 63

+#define ARM_SMCCC_FUNC_QUERY_CALL_UID  0xff01
+
 #define ARM_SMCCC_QUIRK_NONE   0
 #define ARM_SMCCC_QUIRK_QCOM_A61 /* Save/restore register a6 */

@@ -87,6 +89,29 @@
  

[PATCH v16 1/9] arm64: Probe for the presence of KVM hypervisor

2020-12-08 Thread Jianyong Wu
From: Will Deacon 

Although the SMCCC specification provides some limited functionality for
describing the presence of hypervisor and firmware services, this is
generally applicable only to functions designated as "Arm Architecture
Service Functions" and no portable discovery mechanism is provided for
standard hypervisor services, despite having a designated range of
function identifiers reserved by the specification.

In an attempt to avoid the need for additional firmware changes every
time a new function is added, introduce a UID to identify the service
provider as being compatible with KVM. Once this has been established,
additional services can be discovered via a feature bitmap.

Change from Jianyong Wu:
mv kvm_arm_hyp_service_available to common place to let both arm/arm64 touch it.
add kvm_init_hyp_services also under arm arch to let arm kvm guest use this 
service.

Cc: Marc Zyngier 
Signed-off-by: Will Deacon 
Signed-off-by: Jianyong Wu 
---
 arch/arm/kernel/setup.c|  5 
 arch/arm64/kernel/setup.c  |  1 +
 drivers/firmware/smccc/smccc.c | 37 +
 include/linux/arm-smccc.h  | 43 ++
 4 files changed, 86 insertions(+)

diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index 1a5edf562e85..adcefa9c8fab 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -1156,6 +1156,11 @@ void __init setup_arch(char **cmdline_p)
 
arm_dt_init_cpu_maps();
psci_dt_init();
+
+#ifdef CONFIG_HAVE_ARM_SMCCC_DISCOVERY
+   kvm_init_hyp_services();
+#endif
+
 #ifdef CONFIG_SMP
if (is_smp()) {
if (!mdesc->smp_init || !mdesc->smp_init()) {
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index a950d5bc1ba5..97037b15c6ea 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -353,6 +353,7 @@ void __init __no_sanitize_address setup_arch(char 
**cmdline_p)
else
psci_acpi_init();
 
+   kvm_init_hyp_services();
init_bootcpu_ops();
smp_init_cpus();
smp_build_mpidr_hash();
diff --git a/drivers/firmware/smccc/smccc.c b/drivers/firmware/smccc/smccc.c
index 00c88b809c0c..e153c71ece99 100644
--- a/drivers/firmware/smccc/smccc.c
+++ b/drivers/firmware/smccc/smccc.c
@@ -7,10 +7,47 @@
 
 #include 
 #include 
+#include 
+#include 
 
 static u32 smccc_version = ARM_SMCCC_VERSION_1_0;
 static enum arm_smccc_conduit smccc_conduit = SMCCC_CONDUIT_NONE;
 
+DECLARE_BITMAP(__kvm_arm_hyp_services, ARM_SMCCC_KVM_NUM_FUNCS) = { };
+EXPORT_SYMBOL_GPL(__kvm_arm_hyp_services);
+
+void __init kvm_init_hyp_services(void)
+{
+   int i;
+   struct arm_smccc_res res;
+
+   if (arm_smccc_get_version() == ARM_SMCCC_VERSION_1_0)
+   return;
+
+   arm_smccc_1_1_invoke(ARM_SMCCC_VENDOR_HYP_CALL_UID_FUNC_ID, );
+   if (res.a0 != ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_0 ||
+   res.a1 != ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_1 ||
+   res.a2 != ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_2 ||
+   res.a3 != ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_3)
+   return;
+
+   memset(, 0, sizeof(res));
+   arm_smccc_1_1_invoke(ARM_SMCCC_VENDOR_HYP_KVM_FEATURES_FUNC_ID, );
+   for (i = 0; i < 32; ++i) {
+   if (res.a0 & (i))
+   set_bit(i + (32 * 0), __kvm_arm_hyp_services);
+   if (res.a1 & (i))
+   set_bit(i + (32 * 1), __kvm_arm_hyp_services);
+   if (res.a2 & (i))
+   set_bit(i + (32 * 2), __kvm_arm_hyp_services);
+   if (res.a3 & (i))
+   set_bit(i + (32 * 3), __kvm_arm_hyp_services);
+   }
+
+   pr_info("KVM hypervisor services detected (0x%08lx 0x%08lx 0x%08lx 
0x%08lx)\n",
+res.a3, res.a2, res.a1, res.a0);
+}
+
 void __init arm_smccc_version_init(u32 version, enum arm_smccc_conduit conduit)
 {
smccc_version = version;
diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
index f860645f6512..d75408141137 100644
--- a/include/linux/arm-smccc.h
+++ b/include/linux/arm-smccc.h
@@ -55,6 +55,8 @@
 #define ARM_SMCCC_OWNER_TRUSTED_OS 50
 #define ARM_SMCCC_OWNER_TRUSTED_OS_END 63
 
+#define ARM_SMCCC_FUNC_QUERY_CALL_UID  0xff01
+
 #define ARM_SMCCC_QUIRK_NONE   0
 #define ARM_SMCCC_QUIRK_QCOM_A61 /* Save/restore register a6 */
 
@@ -87,6 +89,29 @@
   ARM_SMCCC_SMC_32,\
   0, 0x7fff)
 
+#define ARM_SMCCC_VENDOR_HYP_CALL_UID_FUNC_ID  \
+   ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
+  ARM_SMCCC_SMC_32,\
+  ARM_SMCCC_OWNER_VENDOR_HYP,  \
+  ARM_SMCCC_FUNC_QUERY_CALL_UID)
+
+/* KVM UID value: 28b46fb6-2ec5-11e9-a9ca-4b564d003a74 */
+#define