Messages in this thread | | | Date | Mon, 30 Jan 2023 11:05:25 -0800 | Subject | Re: [PATCH v9 04/27] virt: gunyah: Add hypercalls to identify Gunyah | From | Elliot Berman <> |
| |
On 1/30/2023 2:01 AM, Greg Kroah-Hartman wrote: > On Fri, Jan 20, 2023 at 02:46:03PM -0800, 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 | 3 ++ >> arch/arm64/gunyah/gunyah_hypercall.c | 75 ++++++++++++++++++++++++++++ >> drivers/virt/Kconfig | 2 + >> drivers/virt/gunyah/Kconfig | 14 ++++++ >> include/linux/gunyah.h | 33 ++++++++++++ >> 7 files changed, 130 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 f1e07e39b2f5..fe19e71efc6d 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -9048,6 +9048,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/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..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..ffed4b71641f >> --- /dev/null >> +++ b/arch/arm64/gunyah/gunyah_hypercall.c >> @@ -0,0 +1,75 @@ >> +// 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] = { > > uint32_t is not a kernel type, please use sane ones, like u32. > >> + {0x19bd54bd, 0x0b37571b, 0x946f609b, 0x54539de6}, /* QC_HYP (Qualcomm's build) */ >> + {0x673d5f14, 0x9265ce36, 0xa4535fdb, 0xc1d58fcd}, /* GUNYAH (open source build) */ > > And why not use the kernel uuid type here? Why create your own? > >> +}; >> + >> +#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];) >> + * >> + * Caller should compare the resulting UID to a list of known Gunyah UIDs to >> + * confirm that Linux is running as a guest of Gunyah. >> + */ >> +void gh_hypercall_get_uid(u32 uid[4]) >> +{ >> + struct arm_smccc_res res; >> + u32 uid[4]; > > How does this work when you have a local variable the same name as the > parameter to the function? What shadows what and how? The compiler > didn't complain about this? > >> + 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); > > How can a function that returns void actually return anything? > > This obviously was never compiled. I'm stopping here in my review. >
Fell victim to trusting automatic git commit --fixup tools here. I'll send out v10 with proper bisectability.
| |