lkml.org 
[lkml]   [2023]   [Jan]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH v8 05/28] virt: gunyah: Add hypercalls to identify Gunyah


    On 1/9/2023 1:34 PM, Alex Elder wrote:
    > On 12/19/22 4:58 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.
    >
    > The first is a "service", while the second is a "hypercall".
    > Can you explain the distinction?  The sentence at the top
    > refers to both as "hypercalls".
    >

    I learned more details about this to answer your question. "get_uid()"
    is a standardized call that is ARM_SMCCC_VENDOR_HYP_CALL_UID_FUNC_ID
    defined in include/arm-smccc.h. I'll use that.

    >>
    >> Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
    >> ---
    >>   MAINTAINERS                          |  2 +
    >>   arch/arm64/Kbuild                    |  1 +
    >>   arch/arm64/gunyah/Makefile           |  1 +
    >>   arch/arm64/gunyah/gunyah_hypercall.c | 69 ++++++++++++++++++++++++++++
    >>   drivers/virt/Kconfig                 |  1 +
    >>   drivers/virt/gunyah/Kconfig          | 12 +++++
    >>   include/linux/gunyah.h               | 25 ++++++++++
    >>   7 files changed, 111 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 74e76e0ab14d..36698df6b0e5 100644
    >> --- a/MAINTAINERS
    >> +++ b/MAINTAINERS
    >> @@ -8941,6 +8941,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..9fbc720b6fb6
    >> --- /dev/null
    >> +++ b/arch/arm64/gunyah/Makefile
    >> @@ -0,0 +1 @@
    >> +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..0beb3123d650
    >> --- /dev/null
    >> +++ b/arch/arm64/gunyah/gunyah_hypercall.c
    >> @@ -0,0 +1,69 @@
    >> +// 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 <linux/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
    > A FN_ID is a 32-bit value.  Are all 18 high-order bits considered
    > part of the call type?  It might be good to specify that explicitly
    > by defining a mask for it.
    >

    With above in mind, I decided to simplify the macros and drop the TYPE
    field.

    >> +
    >> +#define GH_FN_ID(type, num)    ((type) << GH_CALL_TYPE_SHIFT | ((num)
    >> & GH_CALL_FUNCTION_NUM_MASK))
    >> +
    >
    > Is there any need for the endianness of these values to be specified?
    > Does Gunyah operate with a well-defined endianness?  Is there any
    > chance a VM can run with an endianness different from Gunyah?  I
    > see that the arm_smcc_* structures are defined without endianness.
    > (Sorry if these are dumb questions.)
    >

    All of the data transfers for hypercalls happen via registers, so
    endianness doesn't have impact here (there is no "low address" in a
    register).

    >> +#define GH_SERVICE(fn)        ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,
    >> ARM_SMCCC_SMC_32, \
    >> +                           ARM_SMCCC_OWNER_VENDOR_HYP, \
    >> +                           GH_FN_ID(GH_CALL_TYPE_SERVICE, fn))
    >> +
    >> +#define GH_HYPERCALL_CALL_UID            GH_SERVICE(0x3f01)
    >
    > Perhaps 0x3f01 could be defined symbolically.
    >
    > However if this is the only place it's ever used, doing so
    > doesn't add much value (meaning, just do it the way you did).
    >
    >> +
    >> +#define GH_HYPERCALL(fn)    ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,
    >> ARM_SMCCC_SMC_64, \
    >> +                           ARM_SMCCC_OWNER_VENDOR_HYP, \
    >> +                           GH_FN_ID(GH_CALL_TYPE_HYPERCALL, fn))
    >> +
    >> +#define GH_HYPERCALL_HYP_IDENTIFY        GH_HYPERCALL(0x0000)
    >
    > Will there be a growing set of well-known hypervisor call functions?
    > Perhaps 0x0000 should be defined symbolically.  (Or not if it's only
    > used here.)
    >

    Yes, we would add growing set of well-known hypercalls. 0x0000 would
    only be used here.
    >> +
    >> +/**
    >> + * 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.
    >
    > I presume that, if the returned UID isn't well-known, then no other
    > Gunyah-related calls are meaningful.  Is that correct?
    >

    That's correct.

    >> + */
    >> +void gh_hypercall_get_uid(u32 uid[4])
    >> +{
    >> +    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;
    >
    > I see in the definition of struct arm_smccc_res that the four
    > fields are unsigned long values.  That differs from the u32
    > array passed as argument.  Are the resource IDs guaranteed to
    > be four 32-bit values?  I personally prefer being explicit
    > about the upper 32-bits being discarded (though some don't
    > agree with that convention).
    >

    Done.

    >> +}
    >> +EXPORT_SYMBOL_GPL(gh_hypercall_get_uid);
    >> +
    >> +/**
    >> + * 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 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..127156a678a6
    >> --- /dev/null
    >> +++ b/drivers/virt/gunyah/Kconfig
    >> @@ -0,0 +1,12 @@
    >> +# SPDX-License-Identifier: GPL-2.0-only
    >> +
    >> +config GUNYAH
    >
    > Maybe config QCOM_GUNYAH?  Will this ever run on hardware
    > other than Qualcomm's?
    >

    Yes, Gunyah can run on other hardware. We have support for QEMU and
    other hardware support is anticipated.

    >> +    tristate "Gunyah Virtualization drivers"
    >> +    depends on ARM64
    >> +    help
    >> +      The Gunyah drivers are the helper interfaces that runs in a
    >> guest VM
    >
    > s/runs/run/
    >
    >> +      such as basic inter-VM IPC and signalingmechanisms, 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 824e20a11d27..2765d2b40198 100644
    >> --- a/include/linux/gunyah.h
    >> +++ b/include/linux/gunyah.h
    >> @@ -6,6 +6,7 @@
    >>   #ifndef _GUNYAH_H
    >>   #define _GUNYAH_H
    >> +#include <linux/bitfield.h>
    >>   #include <linux/types.h>
    >>   #include <linux/errno.h>
    >> @@ -71,4 +72,28 @@ static inline int gh_remap_error(int gh_error)
    >>       }
    >>   }
    >> +#define GUNYAH_API_V1            1
    >> +
    >> +#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)
    >> +
    >
    > How are the GH_IDENTIFY bits below used?  Are they encoded
    > in the three 64-bit flags fields in the response structure?
    > Does that mean only the first of those three is (currently)
    > used?
    >

    That's correct.

    >> +#define GH_IDENTIFY_PARTITION_CSPACE        BIT_ULL(0)
    >> +#define GH_IDENTIFY_DOORBELL            BIT_ULL(1)
    >> +#define GH_IDENTIFY_MSGQUEUE            BIT_ULL(2)
    >> +#define GH_IDENTIFY_VIC                BIT_ULL(3)
    >> +#define GH_IDENTIFY_VPM                BIT_ULL(4)
    >> +#define GH_IDENTIFY_VCPU            BIT_ULL(5)
    >> +#define GH_IDENTIFY_MEMEXTENT            BIT_ULL(6)
    >> +#define GH_IDENTIFY_TRACE_CTRL            BIT_ULL(7)
    >> +
    >> +struct gh_hypercall_hyp_identify_resp {
    >> +    u64 api_info;
    >> +    u64 flags[3];
    >> +};
    >
    > Again I'll ask about endianness.  This is a response coming *from*
    > Gunyah.  Is it guaranteed to use the same byte order convention as
    > the running operating system (Linux) guest?
    >

    Yes, that guarantee is there.

    Thanks,
    Elliot

    \
     
     \ /
      Last update: 2023-03-26 23:35    [W:4.151 / U:0.184 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site