Re: [PATCH v4 06/14] virt: gunyah: Add sysfs nodes
From: Elliot Berman
Date: Tue Oct 04 2022 - 19:51:01 EST
On 9/30/2022 5:09 AM, Greg Kroah-Hartman wrote:
On Wed, Sep 28, 2022 at 12:56:25PM -0700, Elliot Berman wrote:
Add /sys/hypervisor support when detecting that Linux is running in a
Gunyah environment. Export the version of Gunyah which is reported via
the hyp_identify hypercall.
Signed-off-by: Elliot Berman <quic_eberman@xxxxxxxxxxx>
---
.../ABI/testing/sysfs-hypervisor-gunyah | 15 ++++
MAINTAINERS | 1 +
drivers/virt/Makefile | 1 +
drivers/virt/gunyah/Makefile | 2 +
drivers/virt/gunyah/sysfs.c | 71 +++++++++++++++++++
5 files changed, 90 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-hypervisor-gunyah
create mode 100644 drivers/virt/gunyah/Makefile
create mode 100644 drivers/virt/gunyah/sysfs.c
diff --git a/Documentation/ABI/testing/sysfs-hypervisor-gunyah b/Documentation/ABI/testing/sysfs-hypervisor-gunyah
new file mode 100644
index 000000000000..7d74e74e9edd
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-hypervisor-gunyah
@@ -0,0 +1,15 @@
+What: /sys/hypervisor/gunyah/api
+Date: October 2022
+KernelVersion: 6.1
+Contact: linux-arm-msm@xxxxxxxxxxxxxxx
+Description: If running under Gunyah:
+ The Gunyah API version.
What does this version mean? What format is it in?
The version is incremented on backwards-incompatible API changes. It's
an integer: I've updated the description to mention it's an integer. FYI
-- we are still currently at "1" and not aiming to increment this
number. I'd like to get it reported in sysfs in case the version is
incremented later.
+
+What: /sys/hypervisor/gunyah/variant
+Date: October 2022
+KernelVersion: 6.1
+Contact: linux-arm-msm@xxxxxxxxxxxxxxx
+Description: If running under Gunyah:
+ Reports the build variant of Gunyah:
+ The open source build of Gunyah will report "81".
+ The Qualcomm build of Gunyah will report "72".
So there are only 2 versions variants? What happens when you get a
third? And why the odd numbers?
The kernel isn't parsing the reported build variant and is passing the
reported value up to the sysfs node. If a new third variant comes along,
its build variant number would be reported. Would it be preferred to
instead link to Gunyah's definitions for the build variant?
What will userspace do with this information and what tool will parse
it? >
The usecase I'm envisioning is to help user check what build of Gunyah
is present on the host. We don't have any intention to require userspace
(or kernel) to behave differently whether they are on Qualcomm-built
Gunyah or the open-source Gunyah.
diff --git a/MAINTAINERS b/MAINTAINERS
index feafac12db35..a26e67ef36b4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8885,6 +8885,7 @@ M: Elliot Berman <quic_eberman@xxxxxxxxxxx>
M: Murali Nalajala <quic_mnalajal@xxxxxxxxxxx>
L: linux-arm-msm@xxxxxxxxxxxxxxx
S: Supported
+F: Documentation/ABI/testing/sysfs-hypervisor-gunyah
F: Documentation/devicetree/bindings/firmware/gunyah-hypervisor.yaml
F: Documentation/virt/gunyah/
F: arch/arm64/gunyah/
diff --git a/drivers/virt/Makefile b/drivers/virt/Makefile
index 093674e05c40..10b87f934730 100644
--- a/drivers/virt/Makefile
+++ b/drivers/virt/Makefile
@@ -11,3 +11,4 @@ obj-$(CONFIG_NITRO_ENCLAVES) += nitro_enclaves/
obj-$(CONFIG_ACRN_HSM) += acrn/
obj-$(CONFIG_EFI_SECRET) += coco/efi_secret/
obj-$(CONFIG_SEV_GUEST) += coco/sev-guest/
+obj-$(CONFIG_GUNYAH) += gunyah/
diff --git a/drivers/virt/gunyah/Makefile b/drivers/virt/gunyah/Makefile
new file mode 100644
index 000000000000..e15f16c17142
--- /dev/null
+++ b/drivers/virt/gunyah/Makefile
@@ -0,0 +1,2 @@
+gunyah-y += sysfs.o
+obj-$(CONFIG_GUNYAH) += gunyah.o
diff --git a/drivers/virt/gunyah/sysfs.c b/drivers/virt/gunyah/sysfs.c
new file mode 100644
index 000000000000..ec11510cbece
--- /dev/null
+++ b/drivers/virt/gunyah/sysfs.c
@@ -0,0 +1,71 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#define pr_fmt(fmt) "gunyah: " fmt
+
+#include <linux/kobject.h>
+#include <linux/module.h>
+#include <linux/printk.h>
+#include <linux/init.h>
+#include <asm-generic/gunyah.h>
+
+static struct gh_hypercall_hyp_identify_resp gunyah_api;
+
+static ssize_t api_show(struct kobject *kobj, struct kobj_attribute *attr, char *buffer)
+{
+ return sysfs_emit(buffer, "%d\n", (int)GH_API_INFO_API_VERSION(gunyah_api.api_info));
+}
+static struct kobj_attribute api_attr = __ATTR_RO(api);
+
+static ssize_t variant_show(struct kobject *kobj, struct kobj_attribute *attr, char *buffer)
+{
+ return sysfs_emit(buffer, "%d\n", (int)GH_API_INFO_VARIANT(gunyah_api.api_info));
+}
+static struct kobj_attribute variant_attr = __ATTR_RO(variant);
+
+static struct attribute *gunyah_attrs[] = {
+ &api_attr.attr,
+ &variant_attr.attr,
+ NULL
+};
+
+static const struct attribute_group gunyah_group = {
+ .name = "gunyah",
+ .attrs = gunyah_attrs,
+};
+
+static int __init gunyah_init(void)
+{
+ u32 uid[4];
+
+ gh_hypercall_get_uid(uid);
No error checking?
The UID is filled by the first 4 return registers of the hypercall. If
running under some other hypervisor or no hypervisor is present, then
our expectation is that the relevant handler would fill some error value
that isn't the UUID.
KVM uses similar approach to allow guests to identify they are a KVM guest:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/firmware/smccc/kvm_guest.c?h=v6.0-rc7#n23
+
+ if (!(gh_uid_matches(GUNYAH, uid) || gh_uid_matches(QC_HYP, uid)))
+ return 0;
+
+ gh_hypercall_hyp_identify(&gunyah_api);
+
+ if (GH_API_INFO_API_VERSION(gunyah_api.api_info) != 1) {
+ pr_warn("Unrecognized gunyah version: %llu. Currently supported: 1\n",
+ GH_API_INFO_API_VERSION(gunyah_api.api_info));
Shouldn't the "1" be defined somewhere?
+ return 0;
+ }
+
+ pr_notice("Running under Gunyah hypervisor %llx/v%lld\n",
+ GH_API_INFO_VARIANT(gunyah_api.api_info),
+ GH_API_INFO_API_VERSION(gunyah_api.api_info));
When kernel code is working properly, it is quiet. What is this going
to be used for?
Xen and KVM guest drivers also report when they detect that they are
running under a hypervisor. I'll drop this down to pr_info to match.
thanks,
greg k-h