Re: [PATCH v8 05/28] virt: gunyah: Add hypercalls to identify Gunyah

From: Elliot Berman
Date: Tue Jan 10 2023 - 12:59:09 EST




On 1/9/2023 1:34 PM, Alex Elder wrote:
On 12/19/22 4:58 PM, Elliot Berman wrote:
Add hypercalls to identify when Linux is running a virtual machine under
Gunyah.

There are two calls to help identify Gunyah:

1. gh_hypercall_get_uid() returns a UID when running under a Gunyah
    hypervisor.
2. gh_hypercall_hyp_identify() returns build information and a set of
    feature flags that are supported by Gunyah.

The first is a "service", while the second is a "hypercall".
Can you explain the distinction?  The sentence at the top
refers to both as "hypercalls".


I learned more details about this to answer your question. "get_uid()" is a standardized call that is ARM_SMCCC_VENDOR_HYP_CALL_UID_FUNC_ID defined in include/arm-smccc.h. I'll use that.


Signed-off-by: Elliot Berman <quic_eberman@xxxxxxxxxxx>
---
  MAINTAINERS                          |  2 +
  arch/arm64/Kbuild                    |  1 +
  arch/arm64/gunyah/Makefile           |  1 +
  arch/arm64/gunyah/gunyah_hypercall.c | 69 ++++++++++++++++++++++++++++
  drivers/virt/Kconfig                 |  1 +
  drivers/virt/gunyah/Kconfig          | 12 +++++
  include/linux/gunyah.h               | 25 ++++++++++
  7 files changed, 111 insertions(+)
  create mode 100644 arch/arm64/gunyah/Makefile
  create mode 100644 arch/arm64/gunyah/gunyah_hypercall.c
  create mode 100644 drivers/virt/gunyah/Kconfig

diff --git a/MAINTAINERS b/MAINTAINERS
index 74e76e0ab14d..36698df6b0e5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8941,6 +8941,8 @@ L:    linux-arm-msm@xxxxxxxxxxxxxxx
  S:    Supported
  F:    Documentation/devicetree/bindings/firmware/gunyah-hypervisor.yaml
  F:    Documentation/virt/gunyah/
+F:    arch/arm64/gunyah/
+F:    drivers/virt/gunyah/
  F:    include/linux/gunyah.h
  HABANALABS PCI DRIVER
diff --git a/arch/arm64/Kbuild b/arch/arm64/Kbuild
index 5bfbf7d79c99..e4847ba0e3c9 100644
--- a/arch/arm64/Kbuild
+++ b/arch/arm64/Kbuild
@@ -3,6 +3,7 @@ obj-y            += kernel/ mm/ net/
  obj-$(CONFIG_KVM)    += kvm/
  obj-$(CONFIG_XEN)    += xen/
  obj-$(subst m,y,$(CONFIG_HYPERV))    += hyperv/
+obj-$(CONFIG_GUNYAH)    += gunyah/
  obj-$(CONFIG_CRYPTO)    += crypto/
  # for cleaning
diff --git a/arch/arm64/gunyah/Makefile b/arch/arm64/gunyah/Makefile
new file mode 100644
index 000000000000..9fbc720b6fb6
--- /dev/null
+++ b/arch/arm64/gunyah/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_GUNYAH) += gunyah_hypercall.o
diff --git a/arch/arm64/gunyah/gunyah_hypercall.c b/arch/arm64/gunyah/gunyah_hypercall.c
new file mode 100644
index 000000000000..0beb3123d650
--- /dev/null
+++ b/arch/arm64/gunyah/gunyah_hypercall.c
@@ -0,0 +1,69 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#include <linux/arm-smccc.h>
+#include <linux/module.h>
+#include <linux/gunyah.h>
+
+#define GH_CALL_TYPE_PLATFORM_CALL        0
+#define GH_CALL_TYPE_HYPERCALL            2
+#define GH_CALL_TYPE_SERVICE            3
+#define GH_CALL_TYPE_SHIFT            14
+#define GH_CALL_FUNCTION_NUM_MASK        0x3fff

A FN_ID is a 32-bit value.  Are all 18 high-order bits considered
part of the call type?  It might be good to specify that explicitly
by defining a mask for it.


With above in mind, I decided to simplify the macros and drop the TYPE field.

+
+#define GH_FN_ID(type, num)    ((type) << GH_CALL_TYPE_SHIFT | ((num) & GH_CALL_FUNCTION_NUM_MASK))
+

Is there any need for the endianness of these values to be specified?
Does Gunyah operate with a well-defined endianness?  Is there any
chance a VM can run with an endianness different from Gunyah?  I
see that the arm_smcc_* structures are defined without endianness.
(Sorry if these are dumb questions.)


All of the data transfers for hypercalls happen via registers, so endianness doesn't have impact here (there is no "low address" in a register).

+#define GH_SERVICE(fn)        ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, ARM_SMCCC_SMC_32, \
+                           ARM_SMCCC_OWNER_VENDOR_HYP, \
+                           GH_FN_ID(GH_CALL_TYPE_SERVICE, fn))
+
+#define GH_HYPERCALL_CALL_UID            GH_SERVICE(0x3f01)

Perhaps 0x3f01 could be defined symbolically.

However if this is the only place it's ever used, doing so
doesn't add much value (meaning, just do it the way you did).

+
+#define GH_HYPERCALL(fn)    ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, ARM_SMCCC_SMC_64, \
+                           ARM_SMCCC_OWNER_VENDOR_HYP, \
+                           GH_FN_ID(GH_CALL_TYPE_HYPERCALL, fn))
+
+#define GH_HYPERCALL_HYP_IDENTIFY        GH_HYPERCALL(0x0000)

Will there be a growing set of well-known hypervisor call functions?
Perhaps 0x0000 should be defined symbolically.  (Or not if it's only
used here.)


Yes, we would add growing set of well-known hypercalls. 0x0000 would only be used here.

+
+/**
+ * gh_hypercall_get_uid() - Returns a UID when running under a Gunyah hypervisor
+ * @uid: An array of 4 u32's (u32 uid[4];)
+ *
+ * Caller should compare the resulting UID to a list of known Gunyah UIDs to
+ * confirm that Linux is running as a guest of Gunyah.

I presume that, if the returned UID isn't well-known, then no other
Gunyah-related calls are meaningful.  Is that correct?


That's correct.

+ */
+void gh_hypercall_get_uid(u32 uid[4])
+{
+    struct arm_smccc_res res;
+
+    arm_smccc_1_1_hvc(GH_HYPERCALL_CALL_UID, &res);
+
+    uid[0] = res.a0;
+    uid[1] = res.a1;
+    uid[2] = res.a2;
+    uid[3] = res.a3;

I see in the definition of struct arm_smccc_res that the four
fields are unsigned long values.  That differs from the u32
array passed as argument.  Are the resource IDs guaranteed to
be four 32-bit values?  I personally prefer being explicit
about the upper 32-bits being discarded (though some don't
agree with that convention).


Done.

+}
+EXPORT_SYMBOL_GPL(gh_hypercall_get_uid);
+
+/**
+ * gh_hypercall_hyp_identify() - Returns build information and feature flags
+ *                               supported by Gunyah.
+ * @hyp_identity: filled by the hypercall with the API info and feature flags.
+ */
+void gh_hypercall_hyp_identify(struct gh_hypercall_hyp_identify_resp *hyp_identity)
+{
+    struct arm_smccc_res res;
+
+    arm_smccc_1_1_hvc(GH_HYPERCALL_HYP_IDENTIFY, &res);
+
+    hyp_identity->api_info = res.a0;
+    hyp_identity->flags[0] = res.a1;
+    hyp_identity->flags[1] = res.a2;
+    hyp_identity->flags[2] = res.a3;
+}
+EXPORT_SYMBOL_GPL(gh_hypercall_hyp_identify);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Gunyah Hypervisor Hypercalls");
diff --git a/drivers/virt/Kconfig b/drivers/virt/Kconfig
index 87ef258cec64..259dc2be6cad 100644
--- a/drivers/virt/Kconfig
+++ b/drivers/virt/Kconfig
@@ -52,4 +52,5 @@ source "drivers/virt/coco/efi_secret/Kconfig"
  source "drivers/virt/coco/sev-guest/Kconfig"
+source "drivers/virt/gunyah/Kconfig"
  endif
diff --git a/drivers/virt/gunyah/Kconfig b/drivers/virt/gunyah/Kconfig
new file mode 100644
index 000000000000..127156a678a6
--- /dev/null
+++ b/drivers/virt/gunyah/Kconfig
@@ -0,0 +1,12 @@
+# SPDX-License-Identifier: GPL-2.0-only
+
+config GUNYAH

Maybe config QCOM_GUNYAH?  Will this ever run on hardware
other than Qualcomm's?


Yes, Gunyah can run on other hardware. We have support for QEMU and other hardware support is anticipated.

+    tristate "Gunyah Virtualization drivers"
+    depends on ARM64
+    help
+      The Gunyah drivers are the helper interfaces that runs in a guest VM

s/runs/run/

+      such as basic inter-VM IPC and signalingmechanisms, and higher level
+      services such as memory/device sharing, IRQ sharing, and so on.
+
+      Say Y/M here to enable the drivers needed to interact in a Gunyah
+      virtual environment.
diff --git a/include/linux/gunyah.h b/include/linux/gunyah.h
index 824e20a11d27..2765d2b40198 100644
--- a/include/linux/gunyah.h
+++ b/include/linux/gunyah.h
@@ -6,6 +6,7 @@
  #ifndef _GUNYAH_H
  #define _GUNYAH_H
+#include <linux/bitfield.h>
  #include <linux/types.h>
  #include <linux/errno.h>
@@ -71,4 +72,28 @@ static inline int gh_remap_error(int gh_error)
      }
  }
+#define GUNYAH_API_V1            1
+
+#define GH_API_INFO_API_VERSION_MASK    GENMASK_ULL(13, 0)
+#define GH_API_INFO_BIG_ENDIAN        BIT_ULL(14)
+#define GH_API_INFO_IS_64BIT        BIT_ULL(15)
+#define GH_API_INFO_VARIANT_MASK    GENMASK_ULL(63, 56)
+

How are the GH_IDENTIFY bits below used?  Are they encoded
in the three 64-bit flags fields in the response structure?
Does that mean only the first of those three is (currently)
used?


That's correct.

+#define GH_IDENTIFY_PARTITION_CSPACE        BIT_ULL(0)
+#define GH_IDENTIFY_DOORBELL            BIT_ULL(1)
+#define GH_IDENTIFY_MSGQUEUE            BIT_ULL(2)
+#define GH_IDENTIFY_VIC                BIT_ULL(3)
+#define GH_IDENTIFY_VPM                BIT_ULL(4)
+#define GH_IDENTIFY_VCPU            BIT_ULL(5)
+#define GH_IDENTIFY_MEMEXTENT            BIT_ULL(6)
+#define GH_IDENTIFY_TRACE_CTRL            BIT_ULL(7)
+
+struct gh_hypercall_hyp_identify_resp {
+    u64 api_info;
+    u64 flags[3];
+};

Again I'll ask about endianness.  This is a response coming *from*
Gunyah.  Is it guaranteed to use the same byte order convention as
the running operating system (Linux) guest?


Yes, that guarantee is there.

Thanks,
Elliot