Re: [PATCH v5 05/13] virt: gunyah: Add hypercalls to identify Gunyah

[Dmitry Baryshkov <dmitry.baryshkov@linaro.org>]

On 11/10/2022 03:08, 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@quicinc.com>
> ---
>   MAINTAINERS                   |  2 +
>   arch/arm64/Kbuild             |  1 +
>   arch/arm64/gunyah/Makefile    |  2 +
>   arch/arm64/gunyah/hypercall.c | 71 +++++++++++++++++++++++++++++++++++
>   drivers/virt/Kconfig          |  1 +
>   drivers/virt/gunyah/Kconfig   | 13 +++++++
>   include/asm-generic/gunyah.h  | 36 ++++++++++++++++++
>   7 files changed, 126 insertions(+)
>   create mode 100644 arch/arm64/gunyah/Makefile
>   create mode 100644 arch/arm64/gunyah/hypercall.c
>   create mode 100644 drivers/virt/gunyah/Kconfig
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 4fe8cec61551..ed2bc98c3818 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8886,6 +8886,8 @@ L:	linux-arm-msm@vger.kernel.org
>   S:	Supported
>   F:	Documentation/devicetree/bindings/firmware/gunyah-hypervisor.yaml
>   F:	Documentation/virt/gunyah/
> +F:	arch/arm64/gunyah/
> +F:	drivers/virt/gunyah/
>   F:	include/asm-generic/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..f71a9533c266
> --- /dev/null
> +++ b/arch/arm64/gunyah/Makefile
> @@ -0,0 +1,2 @@
> +obj-$(CONFIG_GUNYAH) += gunyah_hypercall.o
> +gunyah_hypercall-y += hypercall.o

You don't have to do this. Just rename your source file to 
gunyah_hypercall.c

> diff --git a/arch/arm64/gunyah/hypercall.c b/arch/arm64/gunyah/hypercall.c
> new file mode 100644
> index 000000000000..5b08c9d80de0
> --- /dev/null
> +++ b/arch/arm64/gunyah/hypercall.c
> @@ -0,0 +1,71 @@
> +// 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 <asm-generic/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
> +
> +#define GH_SERVICE(fn)		ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, ARM_SMCCC_SMC_32, \
> +						   ARM_SMCCC_OWNER_VENDOR_HYP, \
> +						   (GH_CALL_TYPE_SERVICE << GH_CALL_TYPE_SHIFT) \
> +							| ((fn) & GH_CALL_FUNCTION_NUM_MASK))

Add a #define for (GH_CALL_TYPE_SERVICE << GH_CALL_TYPE_SHIFT) | ((fn) & 
GH_CALL_FUNCTION_NUM_MASK)), then use it here and below.

> +
> +#define GH_HYPERCALL_CALL_UID			GH_SERVICE(0x3f01)
> +
> +#define GH_HYPERCALL(fn)	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, ARM_SMCCC_SMC_64, \
> +						   ARM_SMCCC_OWNER_VENDOR_HYP, \
> +						   (GH_CALL_TYPE_HYPERCALL << GH_CALL_TYPE_SHIFT) \
> +							| ((fn) & GH_CALL_FUNCTION_NUM_MASK))
> +
> +#define GH_HYPERCALL_HYP_IDENTIFY		GH_HYPERCALL(0x0000)
> +
> +/**
> + * gh_hypercall_get_uid() - Returns a UID when running under a Gunyah hypervisor.
> + * @uid: An array of 4 u32's (u32 uid[4];)
> + *
> + * The UID will be either QC_HYP_UID or GUNYAH_UID defined in include/asm-generic/gunyah.h.
> + * QC_HYP_UID is returned on platforms using Qualcomm's version of Gunyah.
> + * GUNYAH_UID is returned on platforms using open source version of Gunyah.
> + * If the uid is not one of the above two UIDs, then it is assumed that the hypervisor or firmware
> + * is not Gunyah.
> + */
> +void gh_hypercall_get_uid(u32 *uid)
> +{
> +	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;
> +}
> +EXPORT_SYMBOL_GPL(gh_hypercall_get_uid);
> +
> +/**
> + * gh_hypercall_hyp_identify() - Returns build information and feature flags supported by Gunyah.
> + * @hyp_identify: 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..7ac917e0aa3f
> --- /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
> +	select AUXILIARY_BUS
> +	help
> +	  The Gunyah drivers are the helper interfaces that runs 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/asm-generic/gunyah.h b/include/asm-generic/gunyah.h
> index 64a02dd3b5ad..86eb59e203ef 100644
> --- a/include/asm-generic/gunyah.h
> +++ b/include/asm-generic/gunyah.h
> @@ -71,4 +71,40 @@ static inline int gh_remap_error(int gh_error)
>   	}
>   }
>   
> +#define QC_HYP_UID0 0x19bd54bd
> +#define QC_HYP_UID1 0x0b37571b
> +#define QC_HYP_UID2 0x946f609b
> +#define QC_HYP_UID3 0x54539de6
> +
> +#define GUNYAH_UID0 0x673d5f14
> +#define GUNYAH_UID1 0x9265ce36
> +#define GUNYAH_UID2 0xa4535fdb
> +#define GUNYAH_UID3 0xc1d58fcd

It would be better to define them as arrays

> +
> +#define gh_uid_matches(prefix, uid)	\
> +	((uid)[0] == prefix ## _UID0 && (uid)[1] == prefix ## _UID1 && \
> +	 (uid)[2] == prefix ## _UID2 && (uid)[3] == prefix ## _UID3)

... then you could do memcmp() here.

> +
> +#define GH_API_INFO_API_VERSION(x)	(((x) >> 0) & 0x3fff)

No need to >> 0. And also you can use FIELD_GET to ease review.

> +#define GH_API_INFO_BIG_ENDIAN(x)	(((x) >> 14) & 1)
> +#define GH_API_INFO_IS_64BIT(x)		(((x) >> 15) & 1)
> +#define GH_API_INFO_VARIANT(x)		(((x) >> 56) & 0xff)

Use FIELD_GET here.

> +
> +#define GH_IDENTIFY_PARTITION_CSPACE(flags)	(((flags)[0] >> 0) & 1)
> +#define GH_IDENTIFY_DOORBELL(flags)		(((flags)[0] >> 1) & 1)
> +#define GH_IDENTIFY_MSGQUEUE(flags)		(((flags)[0] >> 2) & 1)
> +#define GH_IDENTIFY_VIC(flags)			(((flags)[0] >> 3) & 1)
> +#define GH_IDENTIFY_VPM(flags)			(((flags)[0] >> 4) & 1)
> +#define GH_IDENTIFY_VCPU(flags)			(((flags)[0] >> 5) & 1)
> +#define GH_IDENTIFY_MEMEXTENT(flags)		(((flags)[0] >> 6) & 1)
> +#define GH_IDENTIFY_TRACE_CTRL(flags)		(((flags)[0] >> 7) & 1)

Ugh. #define GH_IDENTIFY_DOORBELLflags)  (flags & BIT(1)), etc.

> +
> +struct gh_hypercall_hyp_identify_resp {
> +	u64 api_info;
> +	u64 flags[3];
> +};
> +
> +void gh_hypercall_get_uid(u32 *uid);
> +void gh_hypercall_hyp_identify(struct gh_hypercall_hyp_identify_resp *hyp_identity);
> +
>   #endif

-- 
With best wishes
Dmitry