| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 23 Feb 2023 18:09:12 -0600
From: Alex Elder <alex.elder@...aro.org>
To: Elliot Berman <quic_eberman@...cinc.com>,
Alex Elder <elder@...aro.org>,
Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
Catalin Marinas <catalin.marinas@....com>,
Will Deacon <will@...nel.org>,
Prakruthi Deepak Heragu <quic_pheragu@...cinc.com>
Cc: Murali Nalajala <quic_mnalajal@...cinc.com>,
Trilok Soni <quic_tsoni@...cinc.com>,
Srivatsa Vaddagiri <quic_svaddagi@...cinc.com>,
Carl van Schaik <quic_cvanscha@...cinc.com>,
Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio <konrad.dybcio@...aro.org>,
Arnd Bergmann <arnd@...db.de>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Jonathan Corbet <corbet@....net>,
Bagas Sanjaya <bagasdotme@...il.com>,
Jassi Brar <jassisinghbrar@...il.com>,
linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v10 04/26] virt: gunyah: Add hypercalls to identify Gunyah
On 2/14/23 3:12 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.
>
> Signed-off-by: Elliot Berman <quic_eberman@...cinc.com>
> ---
> arch/arm64/Kbuild | 1 +
> arch/arm64/gunyah/Makefile | 3 ++
> arch/arm64/gunyah/gunyah_hypercall.c | 61 ++++++++++++++++++++++++++++
> drivers/virt/Kconfig | 2 +
> drivers/virt/gunyah/Kconfig | 13 ++++++
> include/linux/gunyah.h | 33 +++++++++++++++
> 6 files changed, 113 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/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..84f1e38cafb1
> --- /dev/null
> +++ b/arch/arm64/gunyah/Makefile
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +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..f30d06ee80cf
> --- /dev/null
> +++ b/arch/arm64/gunyah/gunyah_hypercall.c
> @@ -0,0 +1,61 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +
> +#include <linux/arm-smccc.h>
> +#include <linux/module.h>
> +#include <linux/gunyah.h>
> +
> +static const uint32_t gunyah_known_uuids[][4] = {
> + {0x19bd54bd, 0x0b37571b, 0x946f609b, 0x54539de6}, /* QC_HYP (Qualcomm's build) */
> + {0x673d5f14, 0x9265ce36, 0xa4535fdb, 0xc1d58fcd}, /* GUNYAH (open source build) */
> +};
Are these really UUIDs? Standard ones? Define them using
the standard Linux way of doing it. See <linux/uuid.h>.
> +
> +bool arch_is_gunyah_guest(void)
> +{
> + struct arm_smccc_res res;
> + u32 uid[4];
> + int i;
> +
> + arm_smccc_1_1_hvc(ARM_SMCCC_VENDOR_HYP_CALL_UID_FUNC_ID, &res);
> +
> + uid[0] = lower_32_bits(res.a0);
> + uid[1] = lower_32_bits(res.a1);
> + uid[2] = lower_32_bits(res.a2);
> + uid[3] = lower_32_bits(res.a3);
> +
> + for (i = 0; i < ARRAY_SIZE(gunyah_known_uuids); i++)
> + if (!memcmp(uid, gunyah_known_uuids[i], sizeof(uid)))
> + break;
> +
> + return i != ARRAY_SIZE(gunyah_known_uuids);
> +}
> +EXPORT_SYMBOL_GPL(arch_is_gunyah_guest);
> +
> +#define GH_HYPERCALL(fn) ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, ARM_SMCCC_SMC_64, \
> + ARM_SMCCC_OWNER_VENDOR_HYP, \
> + fn)
> +
> +#define GH_HYPERCALL_HYP_IDENTIFY GH_HYPERCALL(0x8000)
> +
> +/**
> + * 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 f79ab13a5c28..85bd6626ffc9 100644
> --- a/drivers/virt/Kconfig
> +++ b/drivers/virt/Kconfig
> @@ -54,4 +54,6 @@ source "drivers/virt/coco/sev-guest/Kconfig"
>
> source "drivers/virt/coco/tdx-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..1a737694c333
> --- /dev/null
> +++ b/drivers/virt/gunyah/Kconfig
> @@ -0,0 +1,13 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +
> +config GUNYAH
> + tristate "Gunyah Virtualization drivers"
> + depends on ARM64
> + depends on MAILBOX
> + help
> + The Gunyah drivers are the helper interfaces that run in a guest VM
> + such as basic inter-VM IPC and signaling mechanisms, 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 59ef4c735ae8..3fef2854c5e1 100644
> --- a/include/linux/gunyah.h
> +++ b/include/linux/gunyah.h
> @@ -6,8 +6,10 @@
> #ifndef _LINUX_GUNYAH_H
> #define _LINUX_GUNYAH_H
>
> +#include <linux/bitfield.h>
> #include <linux/errno.h>
> #include <linux/limits.h>
> +#include <linux/types.h>
>
> /******************************************************************************/
> /* Common arch-independent definitions for Gunyah hypercalls */
> @@ -79,4 +81,35 @@ static inline int gh_remap_error(enum gh_error gh_error)
> }
> }
>
> +enum gh_api_feature {
> + GH_API_FEATURE_DOORBELL,
> + GH_API_FEATURE_MSGQUEUE,
> + GH_API_FEATURE_VCPU,
> + GH_API_FEATURE_MEMEXTENT,
> +};
Can't you reuse these symbols, so that the same set
represents the feature and the identify response?
I'm not sure what a good naming scheme would be, but
you could easily do:
enum gh_api_feature {
GH_FEATURE_DOORBELL = 1,
GH_FEATURE_MSGQUEUE = 2,
GH_FEATURE_VCPU = 5,
GH_FEATURE_MEMEXTENT = 6,
};
And then you could do:
bool gh_api_has_feature(enum gh_api_feature feature)
{
switch (feature) {
case GH_FEATURE_DOORBELL:
case GH_FEATURES_MSGQUEUE:
case GH_FEATURE_VCPU:
case GH_FEATURE_MEMEXTENT:
return !!(gunyah_api.flags[0] & BIT_ULL(feature));
default:
return false;
}
}
> +
> +bool arch_is_gunyah_guest(void);
> +
> +u16 gh_api_version(void);
> +bool gh_api_has_feature(enum gh_api_feature feature);
> +
> +#define GUNYAH_API_V1 1
> +
Rather than _INFO_ here, maybe _IDENTIFY_?
Why is "API_" needed in these symbol names?
> +#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)
> +
> +#define GH_IDENTIFY_DOORBELL BIT_ULL(1)
> +#define GH_IDENTIFY_MSGQUEUE BIT_ULL(2)
> +#define GH_IDENTIFY_VCPU BIT_ULL(5)
> +#define GH_IDENTIFY_MEMEXTENT BIT_ULL(6)
> +
> +struct gh_hypercall_hyp_identify_resp {
> + u64 api_info;
> + u64 flags[3];
> +};
> +
> +void gh_hypercall_hyp_identify(struct gh_hypercall_hyp_identify_resp *hyp_identity);
Since this is a user space API, you *could* consider having
this function return an int. Just in case there's a future
reason that a failure could occur, or that you want to
supply some other information. If this truly doesn't make
sense, it's fine as-is...
> +
> #endif
Powered by blists - more mailing lists