Messages in this thread | | | Date | Tue, 17 Jan 2023 14:29:53 -0800 | Subject | Re: [PATCH v8 06/28] virt: gunyah: Identify hypervisor version | From | Elliot Berman <> |
| |
On 1/17/2023 11:21 AM, Alex Elder wrote: > On 1/10/23 11:56 AM, Elliot Berman wrote: >> >> >> On 1/9/2023 1:34 PM, Alex Elder wrote: >>> On 12/19/22 4:58 PM, Elliot Berman wrote: >>>> Export the version of Gunyah which is reported via the hyp_identify >>>> hypercall. Increments of the major API version indicate possibly >>>> backwards incompatible changes. >>>> >>>> Export the hypervisor identity so that Gunyah drivers can act according >>>> to the major API version. >>>> >>>> Signed-off-by: Elliot Berman <quic_eberman@quicinc.com> >>>> --- >>>> drivers/virt/Makefile | 1 + >>>> drivers/virt/gunyah/Makefile | 1 + >>>> drivers/virt/gunyah/gunyah.c | 46 >>>> ++++++++++++++++++++++++++++++++++++ >>>> include/linux/gunyah.h | 6 +++++ >>>> 4 files changed, 54 insertions(+) >>>> create mode 100644 drivers/virt/gunyah/Makefile >>>> create mode 100644 drivers/virt/gunyah/gunyah.c >>>> >>>> diff --git a/drivers/virt/Makefile b/drivers/virt/Makefile >>>> index 093674e05c40..92b7e5311548 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-y += gunyah/ >>>> diff --git a/drivers/virt/gunyah/Makefile >>>> b/drivers/virt/gunyah/Makefile >>>> new file mode 100644 >>>> index 000000000000..2ac4ee64b89d >>>> --- /dev/null >>>> +++ b/drivers/virt/gunyah/Makefile >>>> @@ -0,0 +1 @@ >>>> +obj-$(CONFIG_GUNYAH) += gunyah.o >>>> diff --git a/drivers/virt/gunyah/gunyah.c >>>> b/drivers/virt/gunyah/gunyah.c >>>> new file mode 100644 >>>> index 000000000000..c34c9046fc08 >>>> --- /dev/null >>>> +++ b/drivers/virt/gunyah/gunyah.c >>>> @@ -0,0 +1,46 @@ >>>> +// 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/gunyah.h> >>>> +#include <linux/init.h> >>>> +#include <linux/module.h> >>>> +#include <linux/printk.h> >>>> + >>>> +struct gh_hypercall_hyp_identify_resp gunyah_api; >>>> +EXPORT_SYMBOL_GPL(gunyah_api); >>>> + >>>> +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) */ >>>> +}; >>>> + >>>> +static int __init gunyah_init(void) >>>> +{ >>>> + u32 uid[4]; >>>> + int i; >>>> + >>>> + gh_hypercall_get_uid(uid); >>>> + >>>> + for (i = 0; i < ARRAY_SIZE(gunyah_known_uuids); i++) >>>> + if (!memcmp(uid, gunyah_known_uuids[i], sizeof(uid))) >>>> + break; >>>> + >>>> + if (i == ARRAY_SIZE(gunyah_known_uuids)) >>>> + return -ENODEV; >>>> + >>>> + gh_hypercall_hyp_identify(&gunyah_api); >>>> + >>>> + pr_info("Running under Gunyah hypervisor %llx/v%u\n", >>>> + FIELD_GET(GH_API_INFO_VARIANT_MASK, gunyah_api.api_info), >>>> + gh_api_version()); >>>> + >>>> + return 0; >>>> +} >>>> +arch_initcall(gunyah_init); >>>> + >>>> +MODULE_LICENSE("GPL"); >>>> +MODULE_DESCRIPTION("Gunyah Hypervisor Driver"); >>>> diff --git a/include/linux/gunyah.h b/include/linux/gunyah.h >>>> index 2765d2b40198..166156f69df9 100644 >>>> --- a/include/linux/gunyah.h >>>> +++ b/include/linux/gunyah.h >>>> @@ -92,6 +92,12 @@ struct gh_hypercall_hyp_identify_resp { >>>> u64 api_info; >>>> u64 flags[3]; >>>> }; >>>> +extern struct gh_hypercall_hyp_identify_resp gunyah_api; >>> >>> Must this global variable be exposed? Can't you hide it--as >>> well as its interpretation--inside "gunyah.c"? >>> >> + >>>> +static inline u16 gh_api_version(void) >>>> +{ >>>> + return FIELD_GET(GH_API_INFO_API_VERSION_MASK, >>>> gunyah_api.api_info); >>>> +} >>> >>> If you don't make the above function inline, can you hide the >>> definition of gunyah_api? >>> >> >> This seems like a good idea to me. I'm thinking to have the following >> functions: >> >> enum gh_api_feature { >> GH_API_FEATURE_DOORBELL, >> GH_API_FEATURE_MSGQUEUE, >> GH_API_FEATURE_VIC, >> GH_API_FEATURE_VPM, >> GH_API_FEATURE_VCPU, >> GH_API_FEATURE_MEMEXTENT, >> GH_API_FEATURE_TRACE_CTRL, >> }; > > My point wasn't necessarily to expose all of that. Already > "gunyah.h" defines the bit positions in which these things > are recorded in the identify response. My main point was > about (not) exposing that global variable. > > I support your suggestion of abstracting the feature > interface that way, but I suggest waiting until it's > actually needed before you add it. I don't see any > code that uses GH_IDENTIFY_PARTITION_CSPACE (and the > rest) currently.
Sure, I will do that. > > Oh, and interestingly enough I see GH_API_INFO_BIG_ENDIAN > defined in the API info... > > -Alex > >> >> u16 gh_api_version(void); >> bool gh_api_has_feature(enum gh_api_feature feature); >> >> Thanks, >> Elliot >
| |