Gunyah is a Type-1 hypervisor independent of any
high-level OS kernel, and runs in a higher CPU privilege level. It does
not depend on any lower-privileged OS kernel/code for its core
functionality. This increases its security and can support a much smaller
trusted computing base than a Type-2 hypervisor.
Gunyah is an open source hypervisor. The source repo is available at
https://github.com/quic/gunyah-hypervisor.
The diagram below shows the architecture.
::
VM A VM B
+-----+ +-----+ | +-----+ +-----+ +-----+
| | | | | | | | | | |
EL0 | APP | | APP | | | APP | | APP | | APP |
| | | | | | | | | | |
+-----+ +-----+ | +-----+ +-----+ +-----+
---------------------|-------------------------
+--------------+ | +----------------------+
| | | | |
EL1 | Linux Kernel | | |Linux kernel/Other OS | ...
| | | | |
+--------------+ | +----------------------+
--------hvc/smc------|------hvc/smc------------
+----------------------------------------+
| |
EL2 | Gunyah Hypervisor |
| |
+----------------------------------------+
Gunyah provides these following features.
- Threads and Scheduling: The scheduler schedules virtual CPUs (VCPUs) on
physical CPUs and enables time-sharing of the CPUs.
- Memory Management: Gunyah tracks memory ownership and use of all memory
under its control. Memory partitioning between VMs is a fundamental
security feature.
- Interrupt Virtualization: All interrupts are handled in the hypervisor
and routed to the assigned VM.
- Inter-VM Communication: There are several different mechanisms provided
for communicating between VMs.
- Device Virtualization: Para-virtualization of devices is supported using
inter-VM communication. Low level system features and devices such as
interrupt controllers are supported with emulation where required.
This series adds the basic framework for detecting that Linux is running
under Gunyah as a virtual machine, communication with the Gunyah Resource
Manager, and a tty driver to demonstrate end-to-end use case of communicating
with RM. Presently, the TTY driver is only capable of behaving as a loopback
device: data sent to ttyGH0 shows up in ttyGH1 and vice versa. More Gunyah
consoles can be exposed as Linux learns about other VMs running in Gunyah.
In a future series, I'll introduce a Gunyah VM loader which can load and run
VMs. This loader will follow the design philosophy of the /dev/kvm.
Changes in v4:
- Tidied up documentation throughout based on questions/feedback received
- Switched message queue implementation to use mailboxes
- Renamed "gunyah_device" as "gunyah_resource"
Changes in v3: https://lore.kernel.org/all/[email protected]/
- /Maintained/Supported/ in MAINTAINERS
- Tidied up documentation throughout based on questions/feedback received
- Moved hypercalls into arch/arm64/gunyah/; following hyper-v's implementation
- Drop opaque typedefs
- Move sysfs nodes under /sys/hypervisor/gunyah/
- Moved Gunyah console driver to drivers/tty/
- Reworked gunyah_device design to drop the Gunyah bus.
Changes in v2: https://lore.kernel.org/all/[email protected]/
- DT bindings clean up
- Switch hypercalls to follow SMCCC
v1: https://lore.kernel.org/all/[email protected]/
Elliot Berman (14):
docs: gunyah: Introduce Gunyah Hypervisor
dt-bindings: Add binding for gunyah hypervisor
gunyah: Common types and error codes for Gunyah hypercalls
arm64: smccc: Include alternative-macros.h
virt: gunyah: Add hypercalls to identify Gunyah
virt: gunyah: Add sysfs nodes
mailbox: Allow direct registration to a channel
virt: gunyah: msgq: Add hypercalls to send and receive messages
mailbox: Add Gunyah message queue mailbox
gunyah: sysfs: Add node to describe supported features
gunyah: rsc_mgr: Add resource manager RPC core
gunyah: rsc_mgr: Add RPC for console services
gunyah: rsc_mgr: Add auxiliary devices for console
tty: gunyah: Add tty console driver for RM Console Services
.../ABI/testing/sysfs-hypervisor-gunyah | 30 +
.../bindings/firmware/gunyah-hypervisor.yaml | 87 +++
Documentation/virt/gunyah/index.rst | 114 ++++
Documentation/virt/gunyah/message-queue.rst | 52 ++
Documentation/virt/index.rst | 1 +
MAINTAINERS | 15 +
arch/arm64/Kbuild | 1 +
arch/arm64/gunyah/Makefile | 2 +
arch/arm64/gunyah/hypercall.c | 104 +++
drivers/mailbox/Kconfig | 10 +
drivers/mailbox/Makefile | 2 +
drivers/mailbox/gunyah-msgq.c | 232 +++++++
drivers/mailbox/mailbox.c | 96 ++-
drivers/tty/Kconfig | 8 +
drivers/tty/Makefile | 1 +
drivers/tty/gunyah_tty.c | 409 ++++++++++++
drivers/virt/Kconfig | 2 +
drivers/virt/Makefile | 1 +
drivers/virt/gunyah/Kconfig | 30 +
drivers/virt/gunyah/Makefile | 5 +
drivers/virt/gunyah/rsc_mgr.c | 629 ++++++++++++++++++
drivers/virt/gunyah/rsc_mgr.h | 56 ++
drivers/virt/gunyah/rsc_mgr_rpc.c | 151 +++++
drivers/virt/gunyah/sysfs.c | 86 +++
include/asm-generic/gunyah.h | 115 ++++
include/linux/arm-smccc.h | 1 +
include/linux/gunyah.h | 67 ++
include/linux/gunyah_rsc_mgr.h | 42 ++
include/linux/mailbox_client.h | 1 +
29 files changed, 2322 insertions(+), 28 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-hypervisor-gunyah
create mode 100644 Documentation/devicetree/bindings/firmware/gunyah-hypervisor.yaml
create mode 100644 Documentation/virt/gunyah/index.rst
create mode 100644 Documentation/virt/gunyah/message-queue.rst
create mode 100644 arch/arm64/gunyah/Makefile
create mode 100644 arch/arm64/gunyah/hypercall.c
create mode 100644 drivers/mailbox/gunyah-msgq.c
create mode 100644 drivers/tty/gunyah_tty.c
create mode 100644 drivers/virt/gunyah/Kconfig
create mode 100644 drivers/virt/gunyah/Makefile
create mode 100644 drivers/virt/gunyah/rsc_mgr.c
create mode 100644 drivers/virt/gunyah/rsc_mgr.h
create mode 100644 drivers/virt/gunyah/rsc_mgr_rpc.c
create mode 100644 drivers/virt/gunyah/sysfs.c
create mode 100644 include/asm-generic/gunyah.h
create mode 100644 include/linux/gunyah.h
create mode 100644 include/linux/gunyah_rsc_mgr.h
base-commit: f76349cf41451c5c42a99f18a9163377e4b364ff
--
2.25.1
Gunyah is an open-source Type-1 hypervisor developed by Qualcomm. It
does not depend on any lower-privileged OS/kernel code for its core
functionality. This increases its security and can support a smaller
trusted computing based when compared to Type-2 hypervisors.
Add documentation describing the Gunyah hypervisor and the main
components of the Gunyah hypervisor which are of interest to Linux
virtualization development.
Signed-off-by: Elliot Berman <[email protected]>
---
Documentation/virt/gunyah/index.rst | 114 ++++++++++++++++++++
Documentation/virt/gunyah/message-queue.rst | 52 +++++++++
Documentation/virt/index.rst | 1 +
MAINTAINERS | 7 ++
4 files changed, 174 insertions(+)
create mode 100644 Documentation/virt/gunyah/index.rst
create mode 100644 Documentation/virt/gunyah/message-queue.rst
diff --git a/Documentation/virt/gunyah/index.rst b/Documentation/virt/gunyah/index.rst
new file mode 100644
index 000000000000..959f451caccd
--- /dev/null
+++ b/Documentation/virt/gunyah/index.rst
@@ -0,0 +1,114 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=================
+Gunyah Hypervisor
+=================
+
+.. toctree::
+ :maxdepth: 1
+
+ message-queue
+
+Gunyah is a Type-1 hypervisor which is independent of any OS kernel, and runs in
+a higher CPU privilege level. It does not depend on any lower-privileged operating system
+for its core functionality. This increases its security and can support a much smaller
+trusted computing base than a Type-2 hypervisor.
+
+Gunyah is an open source hypervisor. The source repo is available at
+https://github.com/quic/gunyah-hypervisor.
+
+Gunyah provides these following features.
+
+- Scheduling:
+
+ A scheduler for virtual CPUs (vCPUs) on physical CPUs and enables time-sharing
+ of the CPUs. Gunyah supports two models of scheduling:
+
+ 1. "Behind the back" scheduling in which Gunyah hypervisor schedules vCPUS on its own
+ 2. "Proxy" scheduling in which a delegated VM can donate part of one of its vCPU slice
+ to another VM's vCPU via a hypercall.
+
+- Memory Management:
+
+ APIs handling memory, abstracted as objects, limiting direct use of physical
+ addresses. Memory ownership and usage tracking of all memory under its control.
+ Memory partitioning between VMs is a fundamental security feature.
+
+- Interrupt Virtualization:
+
+ Uses CPU hardware interrupt virtualization capabilities. Interrupts are handled
+ in the hypervisor and routed to the assigned VM.
+
+- Inter-VM Communication:
+
+ There are several different mechanisms provided for communicating between VMs.
+
+- Virtual platform:
+
+ Architectural devices such as interrupt controllers and CPU timers are directly provided
+ by the hypervisor as well as core virtual platform devices and system APIs such as ARM PSCI.
+
+- Device Virtualization:
+
+ Para-virtualization of devices is supported using inter-VM communication.
+
+Architectures supported
+=======================
+AArch64 with a GIC
+
+Resources and Capabilities
+==========================
+
+Some services or resources provided by the Gunyah hypervisor are described to a virtual machine by
+capability IDs. For instance, inter-VM communication is performed with doorbells and message queues.
+Gunyah allows access to manipulate that doorbell via the capability ID. These devices are described
+in Linux as a struct gunyah_resource.
+
+High level management of these resources is performed by the resource manager VM. RM informs a
+guest VM about resources it can access through either the device tree or via guest-initiated RPC.
+
+For each virtual machine, Gunyah maintains a table of resources which can be accessed by that VM.
+An entry in this table is called a "capability" and VMs can only access resources via this
+capability table. Hence, virtual Gunyah devices are referenced by a "capability IDs" and not a
+"resource IDs". A VM can have multiple capability IDs mapping to the same resource. If 2 VMs have
+access to the same resource, they may not be using the same capability ID to access that resource
+since the tables are independent per VM.
+
+Resource Manager
+================
+
+The resource manager (RM) is a privileged application VM supporting the Gunyah Hypervisor.
+It provides policy enforcement aspects of the virtualization system. The resource manager can
+be treated as an extension of the Hypervisor but is separated to its own partition to ensure
+that the hypervisor layer itself remains small and secure and to maintain a separation of policy
+and mechanism in the platform. On arm64, RM runs at NS-EL1 similar to other virtual machines.
+
+Communication with the resource manager from each guest VM happens with message-queue.rst. Details
+about the specific messages can be found in drivers/virt/gunyah/rsc_mgr.c
+
+::
+
+ +-------+ +--------+ +--------+
+ | RM | | VM_A | | VM_B |
+ +-.-.-.-+ +---.----+ +---.----+
+ | | | |
+ +-.-.-----------.------------.----+
+ | | \==========/ | |
+ | \========================/ |
+ | Gunyah |
+ +---------------------------------+
+
+The source for the resource manager is available at https://github.com/quic/gunyah-resource-manager.
+
+The resource manager provides the following features:
+
+- VM lifecycle management: allocating a VM, starting VMs, destruction of VMs
+- VM access control policy, including memory sharing and lending
+- Interrupt routing configuration
+- Forwarding of system-level events (e.g. VM shutdown) to owner VM
+
+When booting a virtual machine which uses a devicetree, resource manager overlays a
+/hypervisor node. This node can let Linux know it is running as a Gunyah guest VM,
+how to communicate with resource manager, and basic description and capabilities of
+this VM. See Documentation/devicetree/bindings/firmware/gunyah-hypervisor.yaml for a description
+of this node.
diff --git a/Documentation/virt/gunyah/message-queue.rst b/Documentation/virt/gunyah/message-queue.rst
new file mode 100644
index 000000000000..e130f124ed52
--- /dev/null
+++ b/Documentation/virt/gunyah/message-queue.rst
@@ -0,0 +1,52 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+Message Queues
+==============
+Message queue is a simple low-capacity IPC channel between two VMs. It is
+intended for sending small control and configuration messages. Each message
+queue object is unidirectional, so a full-duplex IPC channel requires a pair of
+objects.
+
+Messages can be up to 1024 bytes in length. Longer messages require a further
+protocol on top of the message queue messages themselves. For instance, communication
+with the resource manager adds a header field for sending longer messages via multiple
+message fragments.
+
+The diagram below shows how message queue works. A typical configuration involves
+2 message queues. Message queue 1 allows VM_A to send messages to VM_B. Message
+queue 2 allows VM_B to send messages to VM_A.
+
+1. VM_A sends a message of up to 1024 bytes in length. It raises a hypercall
+ with the message to inform the hypervisor to add the message to
+ message queue 1's queue.
+2. Gunyah raises the corresponding interrupt for VM_B when any of these happens:
+ a. gh_msgq_send has PUSH flag. Queue is immediately flushed. This is the typical case.
+ b. Explicility with gh_msgq_push command from VM_A.
+ c. Message queue has reached a threshold depth.
+3. VM_B calls gh_msgq_recv and Gunyah copies message to requested buffer.
+
+For VM_B to send a message to VM_A, the process is identical, except that hypercalls
+reference message queue 2's capability ID.
+
+::
+
+ +---------------+ +-----------------+ +---------------+
+ | VM_A | |Gunyah hypervisor| | VM_B |
+ | | | | | |
+ | | | | | |
+ | | Tx | | | |
+ | |-------->| | Rx vIRQ | |
+ |gh_msgq_send() | Tx vIRQ |Message queue 1 |-------->|gh_msgq_recv() |
+ | |<------- | | | |
+ | | | | | |
+ | Message Queue | | | | Message Queue |
+ | driver | | | | driver |
+ | | | | | |
+ | | | | | |
+ | | | | Tx | |
+ | | Rx vIRQ | |<--------| |
+ |gh_msgq_recv() |<--------|Message queue 2 | Tx vIRQ |gh_msgq_send() |
+ | | | |-------->| |
+ | | | | | |
+ | | | | | |
+ +---------------+ +-----------------+ +---------------+
diff --git a/Documentation/virt/index.rst b/Documentation/virt/index.rst
index 2f1cffa87b1b..418d540f5484 100644
--- a/Documentation/virt/index.rst
+++ b/Documentation/virt/index.rst
@@ -15,6 +15,7 @@ Linux Virtualization Support
acrn/index
coco/sev-guest
hyperv/index
+ gunyah/index
.. only:: html and subproject
diff --git a/MAINTAINERS b/MAINTAINERS
index f5ca4aefd184..e88ebb7cbcb8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8880,6 +8880,13 @@ L: [email protected]
S: Maintained
F: block/partitions/efi.*
+GUNYAH HYPERVISOR DRIVER
+M: Elliot Berman <[email protected]>
+M: Murali Nalajala <[email protected]>
+L: [email protected]
+S: Supported
+F: Documentation/virt/gunyah/
+
HABANALABS PCI DRIVER
M: Oded Gabbay <[email protected]>
S: Supported
--
2.25.1
Fix build error when CONFIG_ARM64_SVE is selected and
asm/alternative-macros.h wasn't implicitly included by another header.
Fixes: cfa7ff959a78 ("arm64: smccc: Support SMCCC v1.3 SVE register saving hint")
Signed-off-by: Elliot Berman <[email protected]>
---
include/linux/arm-smccc.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
index 220c8c60e021..6a627cdbbdec 100644
--- a/include/linux/arm-smccc.h
+++ b/include/linux/arm-smccc.h
@@ -383,6 +383,7 @@ asmlinkage void __arm_smccc_hvc(unsigned long a0, unsigned long a1,
/* nVHE hypervisor doesn't have a current thread so needs separate checks */
#if defined(CONFIG_ARM64_SVE) && !defined(__KVM_NVHE_HYPERVISOR__)
+#include <asm/alternative-macros.h>
#define SMCCC_SVE_CHECK ALTERNATIVE("nop \n", "bl __arm_smccc_sve_check \n", \
ARM64_SVE)
--
2.25.1
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 <[email protected]>
---
MAINTAINERS | 2 +
arch/arm64/Kbuild | 1 +
arch/arm64/gunyah/Makefile | 2 +
arch/arm64/gunyah/hypercall.c | 71 +++++++++++++++++++++++++++++++++++
drivers/virt/Kconfig | 2 +
drivers/virt/gunyah/Kconfig | 13 +++++++
include/asm-generic/gunyah.h | 36 ++++++++++++++++++
7 files changed, 127 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 31bda0197f9a..feafac12db35 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8887,6 +8887,8 @@ L: [email protected]
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..fbcde0d5cec8 100644
--- a/arch/arm64/Kbuild
+++ b/arch/arm64/Kbuild
@@ -4,6 +4,7 @@ obj-$(CONFIG_KVM) += kvm/
obj-$(CONFIG_XEN) += xen/
obj-$(subst m,y,$(CONFIG_HYPERV)) += hyperv/
obj-$(CONFIG_CRYPTO) += crypto/
+obj-$(CONFIG_GUNYAH) += gunyah/
# for cleaning
subdir- += boot
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
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))
+
+#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..c39409b69d01 100644
--- a/drivers/virt/Kconfig
+++ b/drivers/virt/Kconfig
@@ -52,4 +52,6 @@ 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
+
+#define gh_uid_matches(prefix, uid) \
+ ((uid)[0] == prefix ## _UID0 && (uid)[1] == prefix ## _UID1 && \
+ (uid)[2] == prefix ## _UID2 && (uid)[3] == prefix ## _UID3)
+
+#define GH_API_INFO_API_VERSION(x) (((x) >> 0) & 0x3fff)
+#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)
+
+#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)
+
+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
--
2.25.1
Support virtual mailbox controllers and clients which are not platform
devices or come from the devicetree by allowing them to match client to
channel via some other mechanism.
Signed-off-by: Elliot Berman <[email protected]>
---
drivers/mailbox/mailbox.c | 96 ++++++++++++++++++++++++----------
include/linux/mailbox_client.h | 1 +
2 files changed, 69 insertions(+), 28 deletions(-)
diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
index 4229b9b5da98..adf36c05fa43 100644
--- a/drivers/mailbox/mailbox.c
+++ b/drivers/mailbox/mailbox.c
@@ -317,6 +317,71 @@ int mbox_flush(struct mbox_chan *chan, unsigned long timeout)
}
EXPORT_SYMBOL_GPL(mbox_flush);
+static int __mbox_bind_client(struct mbox_chan *chan, struct mbox_client *cl)
+{
+ struct device *dev = cl->dev;
+ unsigned long flags;
+ int ret;
+
+ if (chan->cl || !try_module_get(chan->mbox->dev->driver->owner)) {
+ dev_dbg(dev, "%s: mailbox not free\n", __func__);
+ return -EBUSY;
+ }
+
+ spin_lock_irqsave(&chan->lock, flags);
+ chan->msg_free = 0;
+ chan->msg_count = 0;
+ chan->active_req = NULL;
+ chan->cl = cl;
+ init_completion(&chan->tx_complete);
+
+ if (chan->txdone_method == TXDONE_BY_POLL && cl->knows_txdone)
+ chan->txdone_method = TXDONE_BY_ACK;
+
+ spin_unlock_irqrestore(&chan->lock, flags);
+
+ if (chan->mbox->ops->startup) {
+ ret = chan->mbox->ops->startup(chan);
+
+ if (ret) {
+ dev_err(dev, "Unable to startup the chan (%d)\n", ret);
+ mbox_free_channel(chan);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+/**
+ * mbox_bind_client - Request a mailbox channel.
+ * @chan: The mailbox channel to bind the client to.
+ * @cl: Identity of the client requesting the channel.
+ *
+ * The Client specifies its requirements and capabilities while asking for
+ * a mailbox channel. It can't be called from atomic context.
+ * The channel is exclusively allocated and can't be used by another
+ * client before the owner calls mbox_free_channel.
+ * After assignment, any packet received on this channel will be
+ * handed over to the client via the 'rx_callback'.
+ * The framework holds reference to the client, so the mbox_client
+ * structure shouldn't be modified until the mbox_free_channel returns.
+ *
+ * Return: 0 if the channel was assigned to the client successfully.
+ * <0 for request failure.
+ */
+int mbox_bind_client(struct mbox_chan *chan, struct mbox_client *cl)
+{
+ int ret;
+
+ mutex_lock(&con_mutex);
+ ret = __mbox_bind_client(chan, cl);
+ mutex_unlock(&con_mutex);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(mbox_bind_client);
+
/**
* mbox_request_channel - Request a mailbox channel.
* @cl: Identity of the client requesting the channel.
@@ -340,7 +405,6 @@ struct mbox_chan *mbox_request_channel(struct mbox_client *cl, int index)
struct mbox_controller *mbox;
struct of_phandle_args spec;
struct mbox_chan *chan;
- unsigned long flags;
int ret;
if (!dev || !dev->of_node) {
@@ -372,33 +436,9 @@ struct mbox_chan *mbox_request_channel(struct mbox_client *cl, int index)
return chan;
}
- if (chan->cl || !try_module_get(mbox->dev->driver->owner)) {
- dev_dbg(dev, "%s: mailbox not free\n", __func__);
- mutex_unlock(&con_mutex);
- return ERR_PTR(-EBUSY);
- }
-
- spin_lock_irqsave(&chan->lock, flags);
- chan->msg_free = 0;
- chan->msg_count = 0;
- chan->active_req = NULL;
- chan->cl = cl;
- init_completion(&chan->tx_complete);
-
- if (chan->txdone_method == TXDONE_BY_POLL && cl->knows_txdone)
- chan->txdone_method = TXDONE_BY_ACK;
-
- spin_unlock_irqrestore(&chan->lock, flags);
-
- if (chan->mbox->ops->startup) {
- ret = chan->mbox->ops->startup(chan);
-
- if (ret) {
- dev_err(dev, "Unable to startup the chan (%d)\n", ret);
- mbox_free_channel(chan);
- chan = ERR_PTR(ret);
- }
- }
+ ret = __mbox_bind_client(chan, cl);
+ if (ret)
+ chan = ERR_PTR(ret);
mutex_unlock(&con_mutex);
return chan;
diff --git a/include/linux/mailbox_client.h b/include/linux/mailbox_client.h
index 65229a45590f..734694912ef7 100644
--- a/include/linux/mailbox_client.h
+++ b/include/linux/mailbox_client.h
@@ -37,6 +37,7 @@ struct mbox_client {
void (*tx_done)(struct mbox_client *cl, void *mssg, int r);
};
+int mbox_bind_client(struct mbox_chan *chan, struct mbox_client *cl);
struct mbox_chan *mbox_request_channel_byname(struct mbox_client *cl,
const char *name);
struct mbox_chan *mbox_request_channel(struct mbox_client *cl, int index);
--
2.25.1
The resource manager is a special virtual machine which is always
running on a Gunyah system. It provides APIs for creating and destroying
VMs, secure memory management, sharing/lending of memory between VMs,
and setup of inter-VM communication. Calls to the resource manager are
made via message queues.
This patch implements the basic probing and RPC mechanism to make those
API calls. Request/response calls can be made with gh_rm_call.
Drivers can also register to notifications pushed by RM via
gh_rm_register_notifier
Specific API calls that resource manager supports will be implemented in
subsequent patches.
Signed-off-by: Elliot Berman <[email protected]>
---
MAINTAINERS | 2 +-
drivers/virt/gunyah/Kconfig | 20 +-
drivers/virt/gunyah/Makefile | 3 +
drivers/virt/gunyah/rsc_mgr.c | 608 +++++++++++++++++++++++++++++++++
drivers/virt/gunyah/rsc_mgr.h | 34 ++
include/linux/gunyah_rsc_mgr.h | 26 ++
6 files changed, 689 insertions(+), 4 deletions(-)
create mode 100644 drivers/virt/gunyah/rsc_mgr.c
create mode 100644 drivers/virt/gunyah/rsc_mgr.h
create mode 100644 include/linux/gunyah_rsc_mgr.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 74053d8ff7ea..a0cba618e5f6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8892,7 +8892,7 @@ F: arch/arm64/gunyah/
F: drivers/mailbox/gunyah-msgq.c
F: drivers/virt/gunyah/
F: include/asm-generic/gunyah.h
-F: include/linux/gunyah.h
+F: include/linux/gunyah*.h
HABANALABS PCI DRIVER
M: Oded Gabbay <[email protected]>
diff --git a/drivers/virt/gunyah/Kconfig b/drivers/virt/gunyah/Kconfig
index f4c822a82f1a..78deed3c4562 100644
--- a/drivers/virt/gunyah/Kconfig
+++ b/drivers/virt/gunyah/Kconfig
@@ -3,9 +3,6 @@
config GUNYAH
tristate "Gunyah Virtualization drivers"
depends on ARM64
- select AUXILIARY_BUS
- select MAILBOX
- select GUNYAH_MESSAGE_QUEUES
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
@@ -13,3 +10,20 @@ config GUNYAH
Say Y/M here to enable the drivers needed to interact in a Gunyah
virtual environment.
+
+if GUNYAH
+
+config GUNYAH_RESORUCE_MANAGER
+ tristate "Gunyah Resource Manager"
+ select MAILBOX
+ select GUNYAH_MESSAGE_QUEUES
+ default y
+ help
+ The resource manager (RM) is a privileged application VM supporting
+ the Gunyah Hypervisor. Enable this driver to support communicating
+ with Gunyah RM. This is typically required for a VM running under
+ Gunyah wanting to have Gunyah-awareness.
+
+ Say Y/M here if unsure.
+
+endif
diff --git a/drivers/virt/gunyah/Makefile b/drivers/virt/gunyah/Makefile
index e15f16c17142..7c512490f921 100644
--- a/drivers/virt/gunyah/Makefile
+++ b/drivers/virt/gunyah/Makefile
@@ -1,2 +1,5 @@
gunyah-y += sysfs.o
obj-$(CONFIG_GUNYAH) += gunyah.o
+
+gunyah_rsc_mgr-y += rsc_mgr.o
+obj-$(CONFIG_GUNYAH_RESORUCE_MANAGER) += gunyah_rsc_mgr.o
diff --git a/drivers/virt/gunyah/rsc_mgr.c b/drivers/virt/gunyah/rsc_mgr.c
new file mode 100644
index 000000000000..7f7e89a6436b
--- /dev/null
+++ b/drivers/virt/gunyah/rsc_mgr.c
@@ -0,0 +1,608 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#define pr_fmt(fmt) "gh_rsc_mgr: " fmt
+
+#include <linux/of.h>
+#include <linux/slab.h>
+#include <linux/mutex.h>
+#include <linux/sched.h>
+#include <linux/gunyah.h>
+#include <linux/module.h>
+#include <linux/of_irq.h>
+#include <linux/kthread.h>
+#include <linux/notifier.h>
+#include <linux/workqueue.h>
+#include <linux/completion.h>
+#include <linux/gunyah_rsc_mgr.h>
+#include <linux/platform_device.h>
+
+#include "rsc_mgr.h"
+
+/* Resource Manager Header */
+struct gh_rm_rpc_hdr {
+ u8 version:4,
+ hdr_words:4;
+ u8 type:2,
+ fragments:6;
+ u16 seq;
+ u32 msg_id;
+} __packed;
+
+/* Standard reply header */
+struct gh_rm_rpc_reply_hdr {
+ struct gh_rm_rpc_hdr rpc_hdr;
+ u32 err_code;
+} __packed;
+
+/* RPC Header versions */
+#define GH_RM_RPC_HDR_VERSION_ONE 0x1
+
+/* RPC Header words */
+#define GH_RM_RPC_HDR_WORDS 0x2
+
+/* RPC Message types */
+#define GH_RM_RPC_TYPE_CONT 0x0
+#define GH_RM_RPC_TYPE_REQ 0x1
+#define GH_RM_RPC_TYPE_RPLY 0x2
+#define GH_RM_RPC_TYPE_NOTIF 0x3
+
+#define GH_RM_MAX_NUM_FRAGMENTS 62
+
+#define GH_RM_MAX_MSG_SIZE (GH_MSGQ_MAX_MSG_SIZE - sizeof(struct gh_rm_rpc_hdr))
+
+/**
+ * struct gh_rm_connection - Represents a complete message from resource manager
+ * @payload: Combined payload of all the fragments (i.e. msg headers stripped off).
+ * @size: Size of the payload.
+ * @ret: Linux return code, set in case there was an error processing the connection.
+ * @msg_id: Message ID from the header.
+ * @type: GH_RM_RPC_TYPE_RPLY or GH_RM_RPC_TYPE_NOTIF.
+ * @num_fragments: total number of fragments expected to be received for this connection.
+ * @fragments_recieved: fragments received so far.
+ * @rm_error: For request/reply sequences with standard replies.
+ * @seq: Sequence ID for the main message.
+ */
+struct gh_rm_connection {
+ void *payload;
+ size_t size;
+ int ret;
+ u32 msg_id;
+ u8 type;
+
+ u8 num_fragments;
+ u8 fragments_received;
+
+ /* only for req/reply sequence */
+ u32 rm_error;
+ u16 seq;
+ struct completion seq_done;
+};
+
+struct gh_rm_notif_complete {
+ struct gh_rm_connection *conn;
+ struct work_struct work;
+};
+
+struct gh_rsc_mgr {
+ struct gunyah_msgq msgq;
+ struct mbox_client msgq_client;
+ struct gh_rm_connection *active_rx_connection;
+ int last_tx_ret;
+
+ struct idr call_idr;
+ struct mutex call_idr_lock;
+
+ struct mutex send_lock;
+
+ struct work_struct recv_work;
+};
+
+static struct gh_rsc_mgr *__rsc_mgr;
+SRCU_NOTIFIER_HEAD_STATIC(gh_rm_notifier);
+
+static struct gh_rm_connection *gh_rm_alloc_connection(u32 msg_id, u8 type)
+{
+ struct gh_rm_connection *connection;
+
+ connection = kzalloc(sizeof(*connection), GFP_KERNEL);
+ if (!connection)
+ return NULL;
+
+ connection->type = type;
+ connection->msg_id = msg_id;
+
+ return connection;
+}
+
+/**
+ * gh_rm_init_connection_payload() - Fills the first message for a connection.
+ */
+static int gh_rm_init_connection_payload(struct gh_rm_connection *connection, void *msg,
+ size_t hdr_size, size_t payload_size)
+{
+ struct gh_rm_rpc_hdr *hdr = msg;
+ size_t max_buf_size;
+
+ connection->num_fragments = hdr->fragments;
+ connection->fragments_received = 0;
+ connection->type = hdr->type;
+
+ /* There's not going to be any payload, no need to allocate buffer. */
+ if (!payload_size && !connection->num_fragments)
+ return 0;
+
+ /*
+ * maximum payload size is GH_MSGQ_MAX_MSG_SIZE - hdr_size
+ * and can received (hdr->fragments + 1) of those
+ */
+ max_buf_size = (GH_MSGQ_MAX_MSG_SIZE - hdr_size) * (hdr->fragments + 1);
+
+ connection->payload = kzalloc(max_buf_size, GFP_KERNEL);
+ if (!connection->payload)
+ return -ENOMEM;
+
+ memcpy(connection->payload, msg + hdr_size, payload_size);
+ connection->size = payload_size;
+ return 0;
+}
+
+static void gh_rm_notif_work(struct work_struct *work)
+{
+ struct gh_rm_notif_complete *notif = container_of(work, struct gh_rm_notif_complete, work);
+ struct gh_rm_connection *connection = notif->conn;
+ u32 notif_id = connection->msg_id;
+ struct gh_rm_notification notification = {
+ .buff = connection->payload,
+ .size = connection->size,
+ };
+
+ srcu_notifier_call_chain(&gh_rm_notifier, notif_id, ¬ification);
+
+ kfree(connection->payload);
+ kfree(connection);
+ kfree(notif);
+}
+
+static struct gh_rm_connection *gh_rm_process_notif(struct gh_rsc_mgr *rsc_mgr,
+ void *msg, size_t msg_size)
+{
+ struct gh_rm_rpc_hdr *hdr = msg;
+ struct gh_rm_connection *connection;
+
+ connection = gh_rm_alloc_connection(hdr->msg_id, hdr->type);
+ if (!connection) {
+ pr_err("Failed to alloc connection for notification, dropping.\n");
+ return NULL;
+ }
+
+ if (gh_rm_init_connection_payload(connection, msg, sizeof(*hdr), msg_size - sizeof(*hdr))) {
+ pr_err("Failed to alloc connection buffer for notification, dropping.\n");
+ kfree(connection);
+ return NULL;
+ }
+
+ return connection;
+}
+
+static struct gh_rm_connection *gh_rm_process_rply(struct gh_rsc_mgr *rsc_mgr,
+ void *msg, size_t msg_size)
+{
+ struct gh_rm_rpc_reply_hdr *reply_hdr = msg;
+ struct gh_rm_rpc_hdr *hdr = msg;
+ struct gh_rm_connection *connection;
+
+ if (mutex_lock_interruptible(&rsc_mgr->call_idr_lock))
+ return ERR_PTR(-ERESTARTSYS);
+
+ connection = idr_find(&rsc_mgr->call_idr, hdr->seq);
+ mutex_unlock(&rsc_mgr->call_idr_lock);
+
+ if (!connection) {
+ pr_err("Failed to find connection for sequence %u\n", hdr->seq);
+ return NULL;
+ }
+ if (connection->msg_id != hdr->msg_id) {
+ pr_err("Reply for sequence %u expected msg_id: %x but got %x\n", hdr->seq,
+ connection->msg_id, hdr->msg_id);
+ /*
+ * Don't complete connection and error the client, maybe resource manager will
+ * send us the expected reply sequence soon.
+ */
+ return NULL;
+ }
+
+ if (gh_rm_init_connection_payload(connection, msg, sizeof(*reply_hdr),
+ msg_size - sizeof(*reply_hdr))) {
+ pr_err("Failed to alloc connection buffer for sequence %d\n", hdr->seq);
+ /* Send connection complete and error the client. */
+ connection->ret = -ENOMEM;
+ complete(&connection->seq_done);
+ return NULL;
+ }
+
+ connection->rm_error = reply_hdr->err_code;
+ return connection;
+}
+
+static void gh_rm_process_cont(struct gh_rm_connection *connection, void *msg, size_t msg_size)
+{
+ struct gh_rm_rpc_hdr *hdr = msg;
+ size_t payload_size = msg_size - sizeof(*hdr);
+
+ /*
+ * hdr->fragments and hdr->msg_id preserves the value from first reply or notif message.
+ * For sake of sanity, check if it's still intact.
+ */
+ if (connection->msg_id != hdr->msg_id)
+ pr_warn("Appending mismatched continuation with id %d to connection with id %d\n",
+ hdr->msg_id, connection->msg_id);
+ if (connection->num_fragments != hdr->fragments)
+ pr_warn("Number of fragments mismatch for seq: %d\n", hdr->seq);
+
+ memcpy(connection->payload + connection->size, msg + sizeof(*hdr), payload_size);
+ connection->size += payload_size;
+ connection->fragments_received++;
+}
+
+static bool gh_rm_complete_connection(struct gh_rm_connection *connection)
+{
+ struct gh_rm_notif_complete *notif_work;
+
+ if (!connection)
+ return false;
+
+ if (connection->fragments_received != connection->num_fragments)
+ return false;
+
+ switch (connection->type) {
+ case GH_RM_RPC_TYPE_RPLY:
+ complete(&connection->seq_done);
+ break;
+ case GH_RM_RPC_TYPE_NOTIF:
+ notif_work = kzalloc(sizeof(*notif_work), GFP_KERNEL);
+ if (notif_work == NULL)
+ break;
+
+ notif_work->conn = connection;
+ INIT_WORK(¬if_work->work, gh_rm_notif_work);
+
+ schedule_work(¬if_work->work);
+ break;
+ default:
+ pr_err("Invalid message type (%d) received\n", connection->type);
+ break;
+ }
+
+ return true;
+}
+
+static void gh_rm_abort_connection(struct gh_rm_connection *connection)
+{
+ switch (connection->type) {
+ case GH_RM_RPC_TYPE_RPLY:
+ connection->ret = -EIO;
+ complete(&connection->seq_done);
+ break;
+ case GH_RM_RPC_TYPE_NOTIF:
+ fallthrough;
+ default:
+ kfree(connection->payload);
+ kfree(connection);
+ }
+}
+
+static void gh_rm_msgq_rx_data(struct mbox_client *cl, void *mssg)
+{
+ struct gh_rsc_mgr *rsc_mgr = container_of(cl, struct gh_rsc_mgr, msgq_client);
+ struct gunyah_msgq_rx_data *rx_data = mssg;
+ void *msg = rx_data->data;
+ size_t msg_size = rx_data->length;
+ struct gh_rm_rpc_hdr *hdr;
+
+ if (msg_size <= sizeof(struct gh_rm_rpc_hdr)) {
+ pr_err("Invalid message size received: %ld is too small\n", msg_size);
+ return;
+ }
+
+ hdr = msg;
+ switch (hdr->type) {
+ case GH_RM_RPC_TYPE_NOTIF:
+ if (rsc_mgr->active_rx_connection) {
+ /* Not possible per protocol. Do something better than BUG_ON */
+ pr_warn("Received start of new notification without finishing existing message series.\n");
+ gh_rm_abort_connection(rsc_mgr->active_rx_connection);
+ }
+ rsc_mgr->active_rx_connection = gh_rm_process_notif(rsc_mgr, msg, msg_size);
+ break;
+ case GH_RM_RPC_TYPE_RPLY:
+ if (rsc_mgr->active_rx_connection) {
+ /* Not possible per protocol. Do something better than BUG_ON */
+ pr_warn("Received start of new reply without finishing existing message series.\n");
+ gh_rm_abort_connection(rsc_mgr->active_rx_connection);
+ }
+ rsc_mgr->active_rx_connection = gh_rm_process_rply(rsc_mgr, msg, msg_size);
+ break;
+ case GH_RM_RPC_TYPE_CONT:
+ if (!rsc_mgr->active_rx_connection) {
+ pr_warn("Received a continuation message without receiving initial message\n");
+ break;
+ }
+ gh_rm_process_cont(rsc_mgr->active_rx_connection, msg, msg_size);
+ break;
+ default:
+ pr_err("Invalid message type (%d) received\n", hdr->type);
+ return;
+ }
+
+ if (gh_rm_complete_connection(rsc_mgr->active_rx_connection))
+ rsc_mgr->active_rx_connection = NULL;
+}
+
+static void gh_rm_msgq_tx_done(struct mbox_client *cl, void *mssg, int r)
+{
+ struct gh_rsc_mgr *rsc_mgr = container_of(cl, struct gh_rsc_mgr, msgq_client);
+
+ kfree(mssg);
+ rsc_mgr->last_tx_ret = r;
+}
+
+static int gh_rm_send_request(struct gh_rsc_mgr *rsc_mgr, u32 message_id,
+ const void *req_buff, size_t req_buff_size,
+ struct gh_rm_connection *connection)
+{
+ size_t buff_size_remaining = req_buff_size;
+ const void *req_buff_curr = req_buff;
+ struct gh_rm_rpc_hdr *hdr;
+ u32 num_fragments = 0;
+ size_t payload_size;
+ struct gunyah_msgq_tx_data *msg;
+ int i, ret = 0;
+
+ if (req_buff_size > GH_RM_MAX_MSG_SIZE)
+ num_fragments = req_buff_size / GH_RM_MAX_MSG_SIZE;
+
+ if (WARN(num_fragments > GH_RM_MAX_NUM_FRAGMENTS,
+ "Limit exceeded for the number of fragments: %u\n", num_fragments))
+ return -E2BIG;
+
+ /*
+ * The above calculation also includes the count for the 'request' packet.
+ * Exclude it as the header needs to fill the num. of fragments to follow.
+ */
+ if (num_fragments)
+ num_fragments--;
+
+ if (mutex_lock_interruptible(&rsc_mgr->send_lock))
+ return -ERESTARTSYS;
+
+
+ /* Consider also the 'request' packet for the loop count */
+ for (i = 0; i <= num_fragments; i++) {
+ if (buff_size_remaining > GH_RM_MAX_MSG_SIZE) {
+ payload_size = GH_RM_MAX_MSG_SIZE;
+ buff_size_remaining -= payload_size;
+ } else {
+ payload_size = buff_size_remaining;
+ }
+
+ msg = kzalloc(sizeof(*msg) + GH_MSGQ_MAX_MSG_SIZE, GFP_KERNEL);
+ if (!msg) {
+ mutex_unlock(&rsc_mgr->send_lock);
+ return -ENOMEM;
+ }
+
+ /* Fill header */
+ hdr = (struct gh_rm_rpc_hdr *)msg->data;
+ hdr->version = GH_RM_RPC_HDR_VERSION_ONE;
+ hdr->hdr_words = GH_RM_RPC_HDR_WORDS;
+ hdr->type = i == 0 ? GH_RM_RPC_TYPE_REQ : GH_RM_RPC_TYPE_CONT;
+ hdr->fragments = num_fragments;
+ hdr->seq = connection->seq;
+ hdr->msg_id = message_id;
+
+ /* Copy payload */
+ memcpy(msg->data + sizeof(*hdr), req_buff_curr, payload_size);
+ req_buff_curr += payload_size;
+
+ /* Force the last fragment to immediately alert the receiver */
+ msg->push = i == num_fragments;
+ msg->length = sizeof(*hdr) + payload_size;
+
+ ret = mbox_send_message(gunyah_msgq_chan(&rsc_mgr->msgq), msg);
+ if (ret < 0) {
+ kfree(msg);
+ break;
+ }
+
+ if (rsc_mgr->last_tx_ret) {
+ ret = rsc_mgr->last_tx_ret;
+ break;
+ }
+ }
+
+ mutex_unlock(&rsc_mgr->send_lock);
+ return ret < 0 ? ret : 0;
+}
+
+/**
+ * gh_rm_call: Achieve request-response type communication with RPC
+ * @message_id: The RM RPC message-id
+ * @req_buff: Request buffer that contains the payload
+ * @req_buff_size: Total size of the payload
+ * @resp_buf: Pointer to a response buffer
+ * @resp_buff_size: Size of the response buffer
+ * @reply_err_code: Returns Gunyah standard error code for the response
+ *
+ * Make a request to the RM-VM and wait for reply back. For a successful
+ * response, the function returns the payload. The size of the payload is set in resp_buff_size.
+ * The resp_buf should be freed by the caller.
+ *
+ * Context: Process context. Will sleep waiting for reply.
+ * Return: >0 is standard reply error from RM. <0 on internal error.
+ */
+int gh_rm_call(u32 message_id, void *req_buff, size_t req_buff_size,
+ void **resp_buf, size_t *resp_buff_size)
+{
+ struct gh_rm_connection *connection;
+ int ret;
+ struct gh_rsc_mgr *rsc_mgr = __rsc_mgr;
+
+ /* messaged_id 0 is reserved */
+ if (!message_id)
+ return -EINVAL;
+
+ if (!rsc_mgr)
+ return -EPROBE_DEFER;
+
+ connection = gh_rm_alloc_connection(message_id, GH_RM_RPC_TYPE_RPLY);
+ if (!connection)
+ return -ENOMEM;
+
+ init_completion(&connection->seq_done);
+
+ /* Allocate a new seq number for this connection */
+ if (mutex_lock_interruptible(&rsc_mgr->call_idr_lock)) {
+ kfree(connection);
+ return -ERESTARTSYS;
+ }
+ connection->seq = idr_alloc_cyclic(&rsc_mgr->call_idr, connection, 0, U16_MAX, GFP_KERNEL);
+ mutex_unlock(&rsc_mgr->call_idr_lock);
+
+ /* Send the request to the Resource Manager */
+ ret = gh_rm_send_request(rsc_mgr, message_id, req_buff, req_buff_size, connection);
+ if (ret < 0)
+ goto out;
+
+ /* Wait for response */
+ wait_for_completion(&connection->seq_done);
+
+ if (connection->ret) {
+ ret = connection->ret;
+ kfree(connection->payload);
+ goto out;
+ }
+
+ if (connection->rm_error) {
+ ret = connection->rm_error;
+ kfree(connection->payload);
+ goto out;
+ }
+
+ *resp_buf = connection->payload;
+ *resp_buff_size = connection->size;
+
+out:
+ mutex_lock(&rsc_mgr->call_idr_lock);
+ idr_remove(&rsc_mgr->call_idr, connection->seq);
+ mutex_unlock(&rsc_mgr->call_idr_lock);
+
+ kfree(connection);
+ return ret;
+}
+
+int gh_rm_register_notifier(struct notifier_block *nb)
+{
+ return srcu_notifier_chain_register(&gh_rm_notifier, nb);
+}
+EXPORT_SYMBOL_GPL(gh_rm_register_notifier);
+
+int gh_rm_unregister_notifier(struct notifier_block *nb)
+{
+ return srcu_notifier_chain_unregister(&gh_rm_notifier, nb);
+}
+EXPORT_SYMBOL_GPL(gh_rm_unregister_notifier);
+
+static int gh_msgq_platform_probe_direction(struct platform_device *pdev,
+ u8 gh_type, int idx, struct gunyah_resource *ghrsc)
+{
+ int ret;
+ struct device_node *node = pdev->dev.of_node;
+
+ ghrsc->type = gh_type;
+
+ ghrsc->irq = platform_get_irq(pdev, idx);
+ if (ghrsc->irq < 0) {
+ dev_err(&pdev->dev, "Failed to get irq%d: %d\n", idx, ghrsc->irq);
+ return ghrsc->irq;
+ }
+
+ ret = of_property_read_u64_index(node, "reg", idx, &ghrsc->capid);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to get capid%d: %d\n", idx, ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int gh_rm_drv_probe(struct platform_device *pdev)
+{
+ struct gh_rsc_mgr *rsc_mgr;
+ struct gunyah_resource tx_ghrsc, rx_ghrsc;
+ int ret;
+
+ rsc_mgr = devm_kzalloc(&pdev->dev, sizeof(*rsc_mgr), GFP_KERNEL);
+ if (!rsc_mgr)
+ return -ENOMEM;
+ platform_set_drvdata(pdev, rsc_mgr);
+
+ mutex_init(&rsc_mgr->call_idr_lock);
+ idr_init(&rsc_mgr->call_idr);
+ mutex_init(&rsc_mgr->send_lock);
+
+ ret = gh_msgq_platform_probe_direction(pdev, GUNYAH_RESOURCE_TYPE_MSGQ_TX, 0, &tx_ghrsc);
+ if (ret)
+ return ret;
+
+ ret = gh_msgq_platform_probe_direction(pdev, GUNYAH_RESOURCE_TYPE_MSGQ_RX, 1, &rx_ghrsc);
+ if (ret)
+ return ret;
+
+ rsc_mgr->msgq_client.dev = &pdev->dev;
+ rsc_mgr->msgq_client.tx_block = true;
+ rsc_mgr->msgq_client.rx_callback = gh_rm_msgq_rx_data;
+ rsc_mgr->msgq_client.tx_done = gh_rm_msgq_tx_done;
+
+ ret = gunyah_msgq_init(&pdev->dev, &rsc_mgr->msgq, &rsc_mgr->msgq_client,
+ &tx_ghrsc, &rx_ghrsc);
+ if (ret)
+ return ret;
+
+ __rsc_mgr = rsc_mgr;
+
+ return 0;
+}
+
+static int gh_rm_drv_remove(struct platform_device *pdev)
+{
+ struct gh_rsc_mgr *rsc_mgr = platform_get_drvdata(pdev);
+
+ __rsc_mgr = NULL;
+
+ mbox_free_channel(gunyah_msgq_chan(&rsc_mgr->msgq));
+ gunyah_msgq_remove(&rsc_mgr->msgq);
+
+ return 0;
+}
+
+static const struct of_device_id gh_rm_of_match[] = {
+ { .compatible = "gunyah-resource-manager" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, gh_rm_of_match);
+
+static struct platform_driver gh_rsc_mgr_driver = {
+ .probe = gh_rm_drv_probe,
+ .remove = gh_rm_drv_remove,
+ .driver = {
+ .name = "gh_rsc_mgr",
+ .of_match_table = gh_rm_of_match,
+ },
+};
+module_platform_driver(gh_rsc_mgr_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Gunyah Resource Manager Driver");
diff --git a/drivers/virt/gunyah/rsc_mgr.h b/drivers/virt/gunyah/rsc_mgr.h
new file mode 100644
index 000000000000..e4f2499267bf
--- /dev/null
+++ b/drivers/virt/gunyah/rsc_mgr.h
@@ -0,0 +1,34 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+#ifndef __GH_RSC_MGR_PRIV_H
+#define __GH_RSC_MGR_PRIV_H
+
+#include <linux/gunyah.h>
+
+/* RM Error codes */
+#define GH_RM_ERROR_OK 0x0
+#define GH_RM_ERROR_UNIMPLEMENTED 0xFFFFFFFF
+#define GH_RM_ERROR_NOMEM 0x1
+#define GH_RM_ERROR_NORESOURCE 0x2
+#define GH_RM_ERROR_DENIED 0x3
+#define GH_RM_ERROR_INVALID 0x4
+#define GH_RM_ERROR_BUSY 0x5
+#define GH_RM_ERROR_ARGUMENT_INVALID 0x6
+#define GH_RM_ERROR_HANDLE_INVALID 0x7
+#define GH_RM_ERROR_VALIDATE_FAILED 0x8
+#define GH_RM_ERROR_MAP_FAILED 0x9
+#define GH_RM_ERROR_MEM_INVALID 0xA
+#define GH_RM_ERROR_MEM_INUSE 0xB
+#define GH_RM_ERROR_MEM_RELEASED 0xC
+#define GH_RM_ERROR_VMID_INVALID 0xD
+#define GH_RM_ERROR_LOOKUP_FAILED 0xE
+#define GH_RM_ERROR_IRQ_INVALID 0xF
+#define GH_RM_ERROR_IRQ_INUSE 0x10
+#define GH_RM_ERROR_IRQ_RELEASED 0x11
+
+int gh_rm_call(u32 message_id, void *req_buff, size_t req_buff_size,
+ void **resp_buf, size_t *resp_buff_size);
+
+#endif
diff --git a/include/linux/gunyah_rsc_mgr.h b/include/linux/gunyah_rsc_mgr.h
new file mode 100644
index 000000000000..b3b37225b7fb
--- /dev/null
+++ b/include/linux/gunyah_rsc_mgr.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#ifndef _GUNYAH_RSC_MGR_H
+#define _GUNYAH_RSC_MGR_H
+
+#include <linux/list.h>
+#include <linux/notifier.h>
+#include <linux/gunyah.h>
+
+#define GH_VMID_INVAL U16_MAX
+
+/* Gunyah recognizes VMID0 as an alias to the current VM's ID */
+#define GH_VMID_SELF 0
+
+struct gh_rm_notification {
+ const void *buff;
+ const size_t size;
+};
+
+int gh_rm_register_notifier(struct notifier_block *nb);
+int gh_rm_unregister_notifier(struct notifier_block *nb);
+
+#endif
--
2.25.1
Gunyah message queues are a unidirectional inter-VM pipe for messages up
to 1024 bytes. This driver supports pairing a receiver message queue and
a transmitter message queue to expose a single mailbox channel.
Signed-off-by: Elliot Berman <[email protected]>
---
MAINTAINERS | 2 +
drivers/mailbox/Kconfig | 10 ++
drivers/mailbox/Makefile | 2 +
drivers/mailbox/gunyah-msgq.c | 232 ++++++++++++++++++++++++++++++++++
drivers/virt/gunyah/Kconfig | 2 +
include/linux/gunyah.h | 67 ++++++++++
6 files changed, 315 insertions(+)
create mode 100644 drivers/mailbox/gunyah-msgq.c
create mode 100644 include/linux/gunyah.h
diff --git a/MAINTAINERS b/MAINTAINERS
index a26e67ef36b4..74053d8ff7ea 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8889,8 +8889,10 @@ F: Documentation/ABI/testing/sysfs-hypervisor-gunyah
F: Documentation/devicetree/bindings/firmware/gunyah-hypervisor.yaml
F: Documentation/virt/gunyah/
F: arch/arm64/gunyah/
+F: drivers/mailbox/gunyah-msgq.c
F: drivers/virt/gunyah/
F: include/asm-generic/gunyah.h
+F: include/linux/gunyah.h
HABANALABS PCI DRIVER
M: Oded Gabbay <[email protected]>
diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index 05d6fae800e3..baf9451c5f04 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -41,6 +41,16 @@ config IMX_MBOX
help
Mailbox implementation for i.MX Messaging Unit (MU).
+config GUNYAH_MESSAGE_QUEUES
+ tristate "Gunyah Message Queue Mailbox"
+ depends on GUNYAH
+ help
+ Mailbox implementation for Gunyah Message Queues. Gunyah message queues
+ are an IPC mechanism to pass short messages between virtual machines
+ running under the Gunyah hypervisor.
+
+ Say Y here if you run Linux as a Gunyah virtual machine.
+
config PLATFORM_MHU
tristate "Platform MHU Mailbox"
depends on OF
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index fc9376117111..5f929bb55e9a 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -55,6 +55,8 @@ obj-$(CONFIG_MTK_CMDQ_MBOX) += mtk-cmdq-mailbox.o
obj-$(CONFIG_ZYNQMP_IPI_MBOX) += zynqmp-ipi-mailbox.o
+obj-$(CONFIG_GUNYAH) += gunyah-msgq.o
+
obj-$(CONFIG_SUN6I_MSGBOX) += sun6i-msgbox.o
obj-$(CONFIG_SPRD_MBOX) += sprd-mailbox.o
diff --git a/drivers/mailbox/gunyah-msgq.c b/drivers/mailbox/gunyah-msgq.c
new file mode 100644
index 000000000000..359f682d101c
--- /dev/null
+++ b/drivers/mailbox/gunyah-msgq.c
@@ -0,0 +1,232 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#include <linux/mailbox_controller.h>
+#include <linux/interrupt.h>
+#include <linux/gunyah.h>
+#include <linux/printk.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/wait.h>
+
+#define mbox_chan_to_msgq(chan) (container_of(chan->mbox, struct gunyah_msgq, mbox))
+
+static inline bool gh_msgq_has_tx(struct gunyah_msgq *msgq)
+{
+ return msgq->tx_ghrsc.type == GUNYAH_RESOURCE_TYPE_MSGQ_TX;
+}
+
+static inline bool gh_msgq_has_rx(struct gunyah_msgq *msgq)
+{
+ return msgq->rx_ghrsc.type == GUNYAH_RESOURCE_TYPE_MSGQ_RX;
+}
+
+static ssize_t __gh_msgq_recv(struct gunyah_msgq *msgq, void *buff, size_t size, bool *ready)
+{
+ unsigned long gh_err;
+ size_t recv_size;
+ ssize_t ret;
+
+ gh_err = gh_hypercall_msgq_recv(msgq->rx_ghrsc.capid, (uintptr_t)buff, size,
+ &recv_size, ready);
+ switch (gh_err) {
+ case GH_ERROR_OK:
+ ret = recv_size;
+ break;
+ case GH_ERROR_MSGQUEUE_EMPTY:
+ ret = -EAGAIN;
+ *ready = false;
+ break;
+ default:
+ ret = gh_remap_error(gh_err);
+ break;
+ }
+
+ return ret;
+}
+
+static irqreturn_t gh_msgq_rx_irq_handler(int irq, void *data)
+{
+ struct gunyah_msgq *msgq = data;
+ struct gunyah_msgq_rx_data rx_data;
+ ssize_t ret;
+ bool more;
+
+ do {
+ ret = __gh_msgq_recv(msgq, &rx_data.data, sizeof(rx_data.data), &more);
+
+ if (ret >= 0) {
+ rx_data.length = ret;
+ mbox_chan_received_data(gunyah_msgq_chan(msgq), &rx_data);
+ } else if (ret != -EAGAIN)
+ pr_warn("Failed to receive data from msgq for %s: %ld\n",
+ msgq->mbox.dev ? dev_name(msgq->mbox.dev) : "", ret);
+ } while (ret >= 0 && more);
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t gh_msgq_tx_irq_handler(int irq, void *data)
+{
+ struct gunyah_msgq *msgq = data;
+
+ mbox_chan_txdone(gunyah_msgq_chan(msgq), 0);
+
+ return IRQ_HANDLED;
+}
+
+static void gh_msgq_txdone_tasklet(unsigned long data)
+{
+ struct gunyah_msgq *msgq = (struct gunyah_msgq *)data;
+
+ mbox_chan_txdone(gunyah_msgq_chan(msgq), msgq->last_status);
+}
+
+static int __gh_msgq_send(struct gunyah_msgq *msgq, void *buff, size_t size, u64 tx_flags)
+{
+ unsigned long gh_err;
+ ssize_t ret;
+ bool ready;
+
+ gh_err = gh_hypercall_msgq_send(msgq->tx_ghrsc.capid, size, (uintptr_t)buff, tx_flags,
+ &ready);
+ switch (gh_err) {
+ case GH_ERROR_OK:
+ ret = !ready;
+ break;
+ case GH_ERROR_MSGQUEUE_FULL:
+ ret = -EAGAIN;
+ break;
+ default:
+ /* Not sure how to propagate these out to client. If we get here, nobody is going
+ * to trigger a retry
+ */
+ ret = gh_remap_error(gh_err);
+ break;
+ }
+
+ return ret;
+}
+
+static int gh_msgq_send_data(struct mbox_chan *chan, void *data)
+{
+ struct gunyah_msgq *msgq = mbox_chan_to_msgq(chan);
+ struct gunyah_msgq_tx_data *msgq_data = data;
+ u64 tx_flags = 0;
+ int ret;
+
+ if (msgq_data->push)
+ tx_flags |= GH_HYPERCALL_MSGQ_TX_FLAGS_PUSH;
+
+ ret = __gh_msgq_send(msgq, msgq_data->data, msgq_data->length, tx_flags);
+
+ /**
+ * EAGAIN: message didn't send.
+ * ret = 1: message sent, but now the message queue is full and we can't send any more msgs.
+ * Either way, don't report that this message is done.
+ */
+ if (ret == -EAGAIN || ret == 1)
+ return ret;
+
+ /**
+ * Mailbox framework requires that tx done happens asynchronously to sending the message
+ * IOW, a notification that the message was sent happens after sending the message.
+ * To work around this, defer the txdone to a tasklet.
+ */
+ msgq->last_status = ret;
+ tasklet_schedule(&msgq->txdone_tasklet);
+
+ return 0;
+}
+
+struct mbox_chan_ops gunyah_msgq_ops = {
+ .send_data = gh_msgq_send_data,
+};
+
+/**
+ * gunyah_msgq_init() - Initialize a Gunyah message queue with an mbox_client
+ * @parent: optional, device parent used for the mailbox controller
+ * @msgq: Pointer to the gunyah_msgq to initialize
+ * @cl: A mailbox client to bind to the mailbox channel that the message queue creates
+ * @tx_ghrsc: optional, the transmission side of the message queue
+ * @rx_ghrsc: optional, the receiving side of the message queue
+ *
+ * At least one of tx_ghrsc and rx_ghrsc should be not NULL. Most message queue use cases come with
+ * a pair of message queues to facilitiate bidirectional communication. When tx_ghrsc is set,
+ * the client can send messages with mbox_send_message(gunyah_msgq_chan(msgq), msg). When rx_ghrsc
+ * is set, the mbox_client should register an .rx_callback() and the message queue driver will
+ * push all available messages upon receiving the RX ready interrupt. The messages should be
+ * consumed or copied by the client right away as the gunyah_msgq_rx_data will be replaced/destroyed
+ * after the callback.
+ *
+ * Returns - 0 on success, negative otherwise
+ */
+int gunyah_msgq_init(struct device *parent, struct gunyah_msgq *msgq, struct mbox_client *cl,
+ struct gunyah_resource *tx_ghrsc, struct gunyah_resource *rx_ghrsc)
+{
+ int ret;
+
+ /* Must have at least a tx_ghrsc or rx_ghrsc and that they are the right device types */
+ if ((!tx_ghrsc && !rx_ghrsc) ||
+ (tx_ghrsc && tx_ghrsc->type != GUNYAH_RESOURCE_TYPE_MSGQ_TX) ||
+ (rx_ghrsc && rx_ghrsc->type != GUNYAH_RESOURCE_TYPE_MSGQ_RX))
+ return -EINVAL;
+
+ msgq->tx_ghrsc = *tx_ghrsc;
+ msgq->rx_ghrsc = *rx_ghrsc;
+
+ msgq->mbox.dev = parent;
+ msgq->mbox.ops = &gunyah_msgq_ops;
+ msgq->mbox.chans = kcalloc(1, sizeof(*msgq->mbox.chans), GFP_KERNEL);
+ msgq->mbox.num_chans = 1;
+ msgq->mbox.txdone_irq = true;
+
+ if (gh_msgq_has_tx(msgq)) {
+ ret = request_irq(msgq->tx_ghrsc.irq, gh_msgq_tx_irq_handler, 0, "gh_msgq_tx",
+ msgq);
+ if (ret)
+ goto err_chans;
+ }
+
+ if (gh_msgq_has_rx(msgq)) {
+ ret = request_threaded_irq(msgq->rx_ghrsc.irq, NULL, gh_msgq_rx_irq_handler,
+ IRQF_ONESHOT, "gh_msgq_rx", msgq);
+ if (ret)
+ goto err_tx_irq;
+ }
+
+ tasklet_init(&msgq->txdone_tasklet, gh_msgq_txdone_tasklet, (unsigned long)msgq);
+
+ ret = mbox_controller_register(&msgq->mbox);
+ if (ret)
+ goto err_rx_irq;
+
+ ret = mbox_bind_client(gunyah_msgq_chan(msgq), cl);
+ if (ret)
+ goto err_mbox;
+
+ return 0;
+err_mbox:
+ mbox_controller_unregister(&msgq->mbox);
+err_rx_irq:
+ if (gh_msgq_has_rx(msgq))
+ free_irq(msgq->rx_ghrsc.irq, msgq);
+err_tx_irq:
+ if (gh_msgq_has_tx(msgq))
+ free_irq(msgq->tx_ghrsc.irq, msgq);
+err_chans:
+ kfree(msgq->mbox.chans);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(gunyah_msgq_init);
+
+void gunyah_msgq_remove(struct gunyah_msgq *msgq)
+{
+ if (gh_msgq_has_tx(msgq))
+ free_irq(msgq->tx_ghrsc.irq, msgq);
+
+ kfree(msgq->mbox.chans);
+}
+EXPORT_SYMBOL_GPL(gunyah_msgq_remove);
diff --git a/drivers/virt/gunyah/Kconfig b/drivers/virt/gunyah/Kconfig
index 7ac917e0aa3f..f4c822a82f1a 100644
--- a/drivers/virt/gunyah/Kconfig
+++ b/drivers/virt/gunyah/Kconfig
@@ -4,6 +4,8 @@ config GUNYAH
tristate "Gunyah Virtualization drivers"
depends on ARM64
select AUXILIARY_BUS
+ select MAILBOX
+ select GUNYAH_MESSAGE_QUEUES
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
diff --git a/include/linux/gunyah.h b/include/linux/gunyah.h
new file mode 100644
index 000000000000..d5e80e2defab
--- /dev/null
+++ b/include/linux/gunyah.h
@@ -0,0 +1,67 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#ifndef _GUNYAH_H
+#define _GUNYAH_H
+
+#include <linux/mailbox_controller.h>
+#include <linux/mailbox_client.h>
+#include <linux/interrupt.h>
+
+#include <asm-generic/gunyah.h>
+
+/* Follows resource manager's resource types for VM_GET_HYP_RESOURCES */
+enum gunyah_resource_type {
+ GUNYAH_RESOURCE_TYPE_BELL_TX = 0,
+ GUNYAH_RESOURCE_TYPE_BELL_RX = 1,
+ GUNYAH_RESOURCE_TYPE_MSGQ_TX = 2,
+ GUNYAH_RESOURCE_TYPE_MSGQ_RX = 3,
+ GUNYAH_RESOURCE_TYPE_VCPU = 4,
+};
+
+struct gunyah_resource {
+ enum gunyah_resource_type type;
+ u64 capid;
+ int irq;
+};
+
+/**
+ * Gunyah Message Queues
+ */
+
+#define GH_MSGQ_MAX_MSG_SIZE 1024
+
+struct gunyah_msgq_tx_data {
+ size_t length;
+ bool push;
+ char data[];
+};
+
+struct gunyah_msgq_rx_data {
+ size_t length;
+ char data[GH_MSGQ_MAX_MSG_SIZE];
+};
+
+struct gunyah_msgq {
+ struct gunyah_resource tx_ghrsc;
+ struct gunyah_resource rx_ghrsc;
+
+ /* msgq private */
+ int last_status;
+ struct mbox_controller mbox;
+ struct tasklet_struct txdone_tasklet;
+};
+
+
+int gunyah_msgq_init(struct device *parent, struct gunyah_msgq *msgq, struct mbox_client *cl,
+ struct gunyah_resource *tx_ghrsc, struct gunyah_resource *rx_ghrsc);
+void gunyah_msgq_remove(struct gunyah_msgq *msgq);
+
+static inline struct mbox_chan *gunyah_msgq_chan(struct gunyah_msgq *msgq)
+{
+ return &msgq->mbox.chans[0];
+}
+
+#endif
--
2.25.1
Add /sys/hypervisor support when detecting that Linux is running in a
Gunyah environment. Export the version of Gunyah which is reported via
the hyp_identify hypercall.
Signed-off-by: Elliot Berman <[email protected]>
---
.../ABI/testing/sysfs-hypervisor-gunyah | 15 ++++
MAINTAINERS | 1 +
drivers/virt/Makefile | 1 +
drivers/virt/gunyah/Makefile | 2 +
drivers/virt/gunyah/sysfs.c | 71 +++++++++++++++++++
5 files changed, 90 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-hypervisor-gunyah
create mode 100644 drivers/virt/gunyah/Makefile
create mode 100644 drivers/virt/gunyah/sysfs.c
diff --git a/Documentation/ABI/testing/sysfs-hypervisor-gunyah b/Documentation/ABI/testing/sysfs-hypervisor-gunyah
new file mode 100644
index 000000000000..7d74e74e9edd
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-hypervisor-gunyah
@@ -0,0 +1,15 @@
+What: /sys/hypervisor/gunyah/api
+Date: October 2022
+KernelVersion: 6.1
+Contact: [email protected]
+Description: If running under Gunyah:
+ The Gunyah API version.
+
+What: /sys/hypervisor/gunyah/variant
+Date: October 2022
+KernelVersion: 6.1
+Contact: [email protected]
+Description: If running under Gunyah:
+ Reports the build variant of Gunyah:
+ The open source build of Gunyah will report "81".
+ The Qualcomm build of Gunyah will report "72".
diff --git a/MAINTAINERS b/MAINTAINERS
index feafac12db35..a26e67ef36b4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8885,6 +8885,7 @@ M: Elliot Berman <[email protected]>
M: Murali Nalajala <[email protected]>
L: [email protected]
S: Supported
+F: Documentation/ABI/testing/sysfs-hypervisor-gunyah
F: Documentation/devicetree/bindings/firmware/gunyah-hypervisor.yaml
F: Documentation/virt/gunyah/
F: arch/arm64/gunyah/
diff --git a/drivers/virt/Makefile b/drivers/virt/Makefile
index 093674e05c40..10b87f934730 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-$(CONFIG_GUNYAH) += gunyah/
diff --git a/drivers/virt/gunyah/Makefile b/drivers/virt/gunyah/Makefile
new file mode 100644
index 000000000000..e15f16c17142
--- /dev/null
+++ b/drivers/virt/gunyah/Makefile
@@ -0,0 +1,2 @@
+gunyah-y += sysfs.o
+obj-$(CONFIG_GUNYAH) += gunyah.o
diff --git a/drivers/virt/gunyah/sysfs.c b/drivers/virt/gunyah/sysfs.c
new file mode 100644
index 000000000000..ec11510cbece
--- /dev/null
+++ b/drivers/virt/gunyah/sysfs.c
@@ -0,0 +1,71 @@
+// 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/kobject.h>
+#include <linux/module.h>
+#include <linux/printk.h>
+#include <linux/init.h>
+#include <asm-generic/gunyah.h>
+
+static struct gh_hypercall_hyp_identify_resp gunyah_api;
+
+static ssize_t api_show(struct kobject *kobj, struct kobj_attribute *attr, char *buffer)
+{
+ return sysfs_emit(buffer, "%d\n", (int)GH_API_INFO_API_VERSION(gunyah_api.api_info));
+}
+static struct kobj_attribute api_attr = __ATTR_RO(api);
+
+static ssize_t variant_show(struct kobject *kobj, struct kobj_attribute *attr, char *buffer)
+{
+ return sysfs_emit(buffer, "%d\n", (int)GH_API_INFO_VARIANT(gunyah_api.api_info));
+}
+static struct kobj_attribute variant_attr = __ATTR_RO(variant);
+
+static struct attribute *gunyah_attrs[] = {
+ &api_attr.attr,
+ &variant_attr.attr,
+ NULL
+};
+
+static const struct attribute_group gunyah_group = {
+ .name = "gunyah",
+ .attrs = gunyah_attrs,
+};
+
+static int __init gunyah_init(void)
+{
+ u32 uid[4];
+
+ gh_hypercall_get_uid(uid);
+
+ if (!(gh_uid_matches(GUNYAH, uid) || gh_uid_matches(QC_HYP, uid)))
+ return 0;
+
+ gh_hypercall_hyp_identify(&gunyah_api);
+
+ if (GH_API_INFO_API_VERSION(gunyah_api.api_info) != 1) {
+ pr_warn("Unrecognized gunyah version: %llu. Currently supported: 1\n",
+ GH_API_INFO_API_VERSION(gunyah_api.api_info));
+ return 0;
+ }
+
+ pr_notice("Running under Gunyah hypervisor %llx/v%lld\n",
+ GH_API_INFO_VARIANT(gunyah_api.api_info),
+ GH_API_INFO_API_VERSION(gunyah_api.api_info));
+
+ return sysfs_create_group(hypervisor_kobj, &gunyah_group);
+}
+module_init(gunyah_init);
+
+static void __exit gunyah_exit(void)
+{
+ sysfs_remove_group(hypervisor_kobj, &gunyah_group);
+}
+module_exit(gunyah_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Gunyah Hypervisor Driver");
--
2.25.1
Gunyah resource manager defines a simple API for virtual machine log
sharing with the console service. A VM's own log can be opened by using
GH_VMID_SELF. Another VM's log can be accessed via its VMID. Once
opened, characters can be written to the log with a write command.
Characters are received with resource manager notifications (using ID
GH_RM_NOTIF_VM_CONSOLE_CHARS).
These high level rpc calls are kept in
drivers/virt/gunyah/rsc_mgr_rpc.c. Future RPC calls, e.g. to launch a VM
will also be maintained in this file.
Signed-off-by: Elliot Berman <[email protected]>
---
drivers/virt/gunyah/Makefile | 2 +-
drivers/virt/gunyah/rsc_mgr.h | 22 +++++
drivers/virt/gunyah/rsc_mgr_rpc.c | 151 ++++++++++++++++++++++++++++++
include/linux/gunyah_rsc_mgr.h | 16 ++++
4 files changed, 190 insertions(+), 1 deletion(-)
create mode 100644 drivers/virt/gunyah/rsc_mgr_rpc.c
diff --git a/drivers/virt/gunyah/Makefile b/drivers/virt/gunyah/Makefile
index 7c512490f921..73339ed445b3 100644
--- a/drivers/virt/gunyah/Makefile
+++ b/drivers/virt/gunyah/Makefile
@@ -1,5 +1,5 @@
gunyah-y += sysfs.o
obj-$(CONFIG_GUNYAH) += gunyah.o
-gunyah_rsc_mgr-y += rsc_mgr.o
+gunyah_rsc_mgr-y += rsc_mgr.o rsc_mgr_rpc.o
obj-$(CONFIG_GUNYAH_RESORUCE_MANAGER) += gunyah_rsc_mgr.o
diff --git a/drivers/virt/gunyah/rsc_mgr.h b/drivers/virt/gunyah/rsc_mgr.h
index e4f2499267bf..deb884979209 100644
--- a/drivers/virt/gunyah/rsc_mgr.h
+++ b/drivers/virt/gunyah/rsc_mgr.h
@@ -28,6 +28,28 @@
#define GH_RM_ERROR_IRQ_INUSE 0x10
#define GH_RM_ERROR_IRQ_RELEASED 0x11
+/* Message IDs: VM Management */
+#define GH_RM_RPC_VM_GET_VMID 0x56000024
+
+/* Message IDs: VM Services */
+#define GH_RM_RPC_VM_CONSOLE_OPEN_ID 0x56000081
+#define GH_RM_RPC_VM_CONSOLE_CLOSE_ID 0x56000082
+#define GH_RM_RPC_VM_CONSOLE_WRITE_ID 0x56000083
+#define GH_RM_RPC_VM_CONSOLE_FLUSH_ID 0x56000084
+
+/* Call: CONSOLE_OPEN, CONSOLE_CLOSE, CONSOLE_FLUSH */
+struct gh_vm_console_common_req {
+ u16 vmid;
+ u16 reserved0;
+} __packed;
+
+/* Call: CONSOLE_WRITE */
+struct gh_vm_console_write_req {
+ u16 vmid;
+ u16 num_bytes;
+ u8 data[0];
+} __packed;
+
int gh_rm_call(u32 message_id, void *req_buff, size_t req_buff_size,
void **resp_buf, size_t *resp_buff_size);
diff --git a/drivers/virt/gunyah/rsc_mgr_rpc.c b/drivers/virt/gunyah/rsc_mgr_rpc.c
new file mode 100644
index 000000000000..8238c6ef301f
--- /dev/null
+++ b/drivers/virt/gunyah/rsc_mgr_rpc.c
@@ -0,0 +1,151 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#define pr_fmt(fmt) "gh_rsc_mgr: " fmt
+
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <linux/printk.h>
+#include <linux/gunyah_rsc_mgr.h>
+
+#include "rsc_mgr.h"
+
+/**
+ * gh_rm_get_vmid() - Retrieve VMID of this virtual machine
+ * @vmid: Filled with the VMID of this VM
+ */
+int gh_rm_get_vmid(u16 *vmid)
+{
+ void *resp;
+ size_t resp_size;
+ int ret;
+ int payload = 0;
+
+ ret = gh_rm_call(GH_RM_RPC_VM_GET_VMID, &payload, sizeof(payload), &resp, &resp_size);
+ if (ret)
+ return ret;
+
+ if (resp_size != sizeof(*vmid))
+ return -EIO;
+ *vmid = *(u16 *)resp;
+ kfree(resp);
+
+ return ret;
+}
+
+/**
+ * gh_rm_console_open() - Open a console with a VM
+ * @vmid: VMID of the other VM whose console to open. If VMID is GH_VMID_SELF, the
+ * console associated with this VM is opened.
+ */
+int gh_rm_console_open(u16 vmid)
+{
+ void *resp;
+ struct gh_vm_console_common_req req_payload = {0};
+ size_t resp_size;
+ int ret;
+
+ req_payload.vmid = vmid;
+
+ ret = gh_rm_call(GH_RM_RPC_VM_CONSOLE_OPEN_ID,
+ &req_payload, sizeof(req_payload),
+ &resp, &resp_size);
+ kfree(resp);
+
+ if (!ret && resp_size)
+ pr_warn("Received unexpected payload for CONSOLE_OPEN: %lu\n", resp_size);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(gh_rm_console_open);
+
+/**
+ * gh_rm_console_close() - Close a console with a VM
+ * @vmid: The vmid of the vm whose console to close.
+ */
+int gh_rm_console_close(u16 vmid)
+{
+ void *resp;
+ struct gh_vm_console_common_req req_payload = {0};
+ size_t resp_size;
+ int ret;
+
+ req_payload.vmid = vmid;
+
+ ret = gh_rm_call(GH_RM_RPC_VM_CONSOLE_CLOSE_ID,
+ &req_payload, sizeof(req_payload),
+ &resp, &resp_size);
+ kfree(resp);
+
+ if (!ret && resp_size)
+ pr_warn("Received unexpected payload for CONSOLE_CLOSE: %lu\n", resp_size);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(gh_rm_console_close);
+
+/**
+ * gh_rm_console_write() - Write to a VM's console
+ * @vmid: The vmid of the vm whose console to write to.
+ * @buf: Buffer to write to the VM's console
+ * @size: Size of the buffer
+ */
+int gh_rm_console_write(u16 vmid, const char *buf, size_t size)
+{
+ void *resp;
+ struct gh_vm_console_write_req *req_payload;
+ size_t resp_size;
+ int ret = 0;
+ size_t req_payload_size = sizeof(*req_payload) + size;
+
+ if (size < 1 || size > (U32_MAX - sizeof(*req_payload)))
+ return -EINVAL;
+
+ req_payload = kzalloc(req_payload_size, GFP_KERNEL);
+
+ if (!req_payload)
+ return -ENOMEM;
+
+ req_payload->vmid = vmid;
+ req_payload->num_bytes = size;
+ memcpy(req_payload->data, buf, size);
+
+ ret = gh_rm_call(GH_RM_RPC_VM_CONSOLE_WRITE_ID,
+ req_payload, req_payload_size,
+ &resp, &resp_size);
+ kfree(req_payload);
+ kfree(resp);
+
+ if (!ret && resp_size)
+ pr_warn("Received unexpected payload for CONSOLE_WRITE: %lu\n", resp_size);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(gh_rm_console_write);
+
+/**
+ * gh_rm_console_flush() - Flush a console with a VM
+ * @vmid: The vmid of the vm whose console to flush
+ */
+int gh_rm_console_flush(u16 vmid)
+{
+ void *resp;
+ struct gh_vm_console_common_req req_payload = {0};
+ size_t resp_size;
+ int ret;
+
+ req_payload.vmid = vmid;
+
+ ret = gh_rm_call(GH_RM_RPC_VM_CONSOLE_FLUSH_ID,
+ &req_payload, sizeof(req_payload),
+ &resp, &resp_size);
+ kfree(resp);
+
+ if (!ret && resp_size)
+ pr_warn("Received unexpected payload for CONSOLE_FLUSH: %lu\n", resp_size);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(gh_rm_console_flush);
diff --git a/include/linux/gunyah_rsc_mgr.h b/include/linux/gunyah_rsc_mgr.h
index b3b37225b7fb..f831ca921c26 100644
--- a/include/linux/gunyah_rsc_mgr.h
+++ b/include/linux/gunyah_rsc_mgr.h
@@ -23,4 +23,20 @@ struct gh_rm_notification {
int gh_rm_register_notifier(struct notifier_block *nb);
int gh_rm_unregister_notifier(struct notifier_block *nb);
+/* Notification type Message IDs */
+#define GH_RM_NOTIF_VM_CONSOLE_CHARS 0x56100080
+
+struct gh_rm_notif_vm_console_chars {
+ u16 vmid;
+ u16 num_bytes;
+ u8 bytes[0];
+} __packed;
+
+/* RPC Calls */
+int gh_rm_get_vmid(u16 *vmid);
+int gh_rm_console_open(u16 vmid);
+int gh_rm_console_close(u16 vmid);
+int gh_rm_console_write(u16 vmid, const char *buf, size_t size);
+int gh_rm_console_flush(u16 vmid);
+
#endif
--
2.25.1
Add hypercalls to send and receive messages on a Gunyah message queue.
Signed-off-by: Elliot Berman <[email protected]>
---
arch/arm64/gunyah/hypercall.c | 33 +++++++++++++++++++++++++++++++++
include/asm-generic/gunyah.h | 5 +++++
2 files changed, 38 insertions(+)
diff --git a/arch/arm64/gunyah/hypercall.c b/arch/arm64/gunyah/hypercall.c
index 5b08c9d80de0..042cca31879e 100644
--- a/arch/arm64/gunyah/hypercall.c
+++ b/arch/arm64/gunyah/hypercall.c
@@ -26,6 +26,8 @@
| ((fn) & GH_CALL_FUNCTION_NUM_MASK))
#define GH_HYPERCALL_HYP_IDENTIFY GH_HYPERCALL(0x0000)
+#define GH_HYPERCALL_MSGQ_SEND GH_HYPERCALL(0x001B)
+#define GH_HYPERCALL_MSGQ_RECV GH_HYPERCALL(0x001C)
/**
* gh_hypercall_get_uid() - Returns a UID when running under a Gunyah hypervisor.
@@ -67,5 +69,36 @@ void gh_hypercall_hyp_identify(struct gh_hypercall_hyp_identify_resp *hyp_identi
}
EXPORT_SYMBOL_GPL(gh_hypercall_hyp_identify);
+int gh_hypercall_msgq_send(u64 capid, size_t size, uintptr_t buff, int tx_flags, bool *ready)
+{
+ struct arm_smccc_res res;
+
+ arm_smccc_1_1_hvc(GH_HYPERCALL_MSGQ_SEND, capid, size, buff, tx_flags, 0, &res);
+
+ if (res.a0)
+ return res.a0;
+
+ *ready = res.a1;
+
+ return res.a0;
+}
+EXPORT_SYMBOL_GPL(gh_hypercall_msgq_send);
+
+int gh_hypercall_msgq_recv(u64 capid, uintptr_t buff, size_t size, size_t *recv_size, bool *ready)
+{
+ struct arm_smccc_res res;
+
+ arm_smccc_1_1_hvc(GH_HYPERCALL_MSGQ_RECV, capid, buff, size, 0, &res);
+
+ if (res.a0)
+ return res.a0;
+
+ *recv_size = res.a1;
+ *ready = res.a2;
+
+ return res.a0;
+}
+EXPORT_SYMBOL_GPL(gh_hypercall_msgq_recv);
+
MODULE_LICENSE("GPL");
MODULE_DESCRIPTION("Gunyah Hypervisor Hypercalls");
diff --git a/include/asm-generic/gunyah.h b/include/asm-generic/gunyah.h
index 86eb59e203ef..43915faea704 100644
--- a/include/asm-generic/gunyah.h
+++ b/include/asm-generic/gunyah.h
@@ -107,4 +107,9 @@ struct gh_hypercall_hyp_identify_resp {
void gh_hypercall_get_uid(u32 *uid);
void gh_hypercall_hyp_identify(struct gh_hypercall_hyp_identify_resp *hyp_identity);
+#define GH_HYPERCALL_MSGQ_TX_FLAGS_PUSH BIT(0)
+
+int gh_hypercall_msgq_send(u64 capid, size_t size, uintptr_t buff, int tx_flags, bool *ready);
+int gh_hypercall_msgq_recv(u64 capid, uintptr_t buff, size_t size, size_t *recv_size, bool *ready);
+
#endif
--
2.25.1
Add a sysfs node to list the features that the Gunyah hypervisor and
Linux supports. For now, Linux support cspace (capability IDs) and
message queues, so only report those..
Signed-off-by: Elliot Berman <[email protected]>
---
Documentation/ABI/testing/sysfs-hypervisor-gunyah | 15 +++++++++++++++
drivers/virt/gunyah/sysfs.c | 15 +++++++++++++++
2 files changed, 30 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-hypervisor-gunyah b/Documentation/ABI/testing/sysfs-hypervisor-gunyah
index 7d74e74e9edd..6d0cde30355a 100644
--- a/Documentation/ABI/testing/sysfs-hypervisor-gunyah
+++ b/Documentation/ABI/testing/sysfs-hypervisor-gunyah
@@ -1,3 +1,18 @@
+What: /sys/hypervisor/gunyah/features
+Date: October 2022
+KernelVersion: 6.1
+Contact: [email protected]
+Description: If running under Gunyah:
+ Space separated list of features supported by Linux and Gunyah:
+ "cspace": Gunyah devices
+ "doorbell": Sending/receiving virtual interrupts via Gunyah doorbells
+ "message-queue": Sending/receiving messages via Gunyah message queues
+ "vic": Interrupt lending
+ "vpm": Virtual platform management
+ "vcpu": Virtual CPU management
+ "memextent": Memory lending/management
+ "trace": Gunyah hypervisor tracing
+
What: /sys/hypervisor/gunyah/api
Date: October 2022
KernelVersion: 6.1
diff --git a/drivers/virt/gunyah/sysfs.c b/drivers/virt/gunyah/sysfs.c
index ec11510cbece..f8ec0553c197 100644
--- a/drivers/virt/gunyah/sysfs.c
+++ b/drivers/virt/gunyah/sysfs.c
@@ -25,9 +25,24 @@ static ssize_t variant_show(struct kobject *kobj, struct kobj_attribute *attr, c
}
static struct kobj_attribute variant_attr = __ATTR_RO(variant);
+static ssize_t features_show(struct kobject *kobj, struct kobj_attribute *attr, char *buffer)
+{
+ int len = 0;
+
+ if (GH_IDENTIFY_PARTITION_CSPACE(gunyah_api.flags))
+ len += sysfs_emit_at(buffer, len, "cspace ");
+ if (GH_IDENTIFY_MSGQUEUE(gunyah_api.flags))
+ len += sysfs_emit_at(buffer, len, "message-queue ");
+
+ len += sysfs_emit_at(buffer, len, "\n");
+ return len;
+}
+static struct kobj_attribute features_attr = __ATTR_RO(features);
+
static struct attribute *gunyah_attrs[] = {
&api_attr.attr,
&variant_attr.attr,
+ &features_attr.attr,
NULL
};
--
2.25.1
Gunyah resource manager exposes a concrete functionalities which
complicate a single resource manager driver. Use auxiliary bus
to help split high level functions for the resource manager and keep the
primary resource manager driver focused on the RPC with RM itself.
Delegate Resource Manager's console functionality to the auxiliary bus.
Signed-off-by: Elliot Berman <[email protected]>
---
drivers/virt/gunyah/Kconfig | 1 +
drivers/virt/gunyah/rsc_mgr.c | 21 +++++++++++++++++++++
2 files changed, 22 insertions(+)
diff --git a/drivers/virt/gunyah/Kconfig b/drivers/virt/gunyah/Kconfig
index 78deed3c4562..610c8586005b 100644
--- a/drivers/virt/gunyah/Kconfig
+++ b/drivers/virt/gunyah/Kconfig
@@ -17,6 +17,7 @@ config GUNYAH_RESORUCE_MANAGER
tristate "Gunyah Resource Manager"
select MAILBOX
select GUNYAH_MESSAGE_QUEUES
+ select AUXILIARY_BUS
default y
help
The resource manager (RM) is a privileged application VM supporting
diff --git a/drivers/virt/gunyah/rsc_mgr.c b/drivers/virt/gunyah/rsc_mgr.c
index 7f7e89a6436b..435fe0149915 100644
--- a/drivers/virt/gunyah/rsc_mgr.c
+++ b/drivers/virt/gunyah/rsc_mgr.c
@@ -16,6 +16,7 @@
#include <linux/notifier.h>
#include <linux/workqueue.h>
#include <linux/completion.h>
+#include <linux/auxiliary_bus.h>
#include <linux/gunyah_rsc_mgr.h>
#include <linux/platform_device.h>
@@ -98,6 +99,8 @@ struct gh_rsc_mgr {
struct mutex send_lock;
struct work_struct recv_work;
+
+ struct auxiliary_device console_adev;
};
static struct gh_rsc_mgr *__rsc_mgr;
@@ -573,13 +576,31 @@ static int gh_rm_drv_probe(struct platform_device *pdev)
__rsc_mgr = rsc_mgr;
+ rsc_mgr->console_adev.dev.parent = &pdev->dev;
+ rsc_mgr->console_adev.name = "console";
+ ret = auxiliary_device_init(&rsc_mgr->console_adev);
+ if (ret)
+ goto err_msgq;
+ ret = auxiliary_device_add(&rsc_mgr->console_adev);
+ if (ret)
+ goto err_console_adev_uninit;
+
return 0;
+
+err_console_adev_uninit:
+ auxiliary_device_uninit(&rsc_mgr->console_adev);
+err_msgq:
+ gunyah_msgq_remove(&rsc_mgr->msgq);
+ return ret;
}
static int gh_rm_drv_remove(struct platform_device *pdev)
{
struct gh_rsc_mgr *rsc_mgr = platform_get_drvdata(pdev);
+ auxiliary_device_delete(&rsc_mgr->console_adev);
+ auxiliary_device_uninit(&rsc_mgr->console_adev);
+
__rsc_mgr = NULL;
mbox_free_channel(gunyah_msgq_chan(&rsc_mgr->msgq));
--
2.25.1
When Linux is booted as a guest under the Gunyah hypervisor, the Gunyah
Resource Manager applies a devicetree overlay describing the virtual
platform configuration of the guest VM, such as the message queue
capability IDs for communicating with the Resource Manager. This
information is not otherwise discoverable by a VM: the Gunyah hypervisor
core does not provide a direct interface to discover capability IDs nor
a way to communicate with RM without having already known the
corresponding message queue capability ID. Add the DT bindings that
Gunyah adheres for the hypervisor node and message queues.
Signed-off-by: Elliot Berman <[email protected]>
---
.../bindings/firmware/gunyah-hypervisor.yaml | 87 +++++++++++++++++++
MAINTAINERS | 1 +
2 files changed, 88 insertions(+)
create mode 100644 Documentation/devicetree/bindings/firmware/gunyah-hypervisor.yaml
diff --git a/Documentation/devicetree/bindings/firmware/gunyah-hypervisor.yaml b/Documentation/devicetree/bindings/firmware/gunyah-hypervisor.yaml
new file mode 100644
index 000000000000..f0a14101e2fd
--- /dev/null
+++ b/Documentation/devicetree/bindings/firmware/gunyah-hypervisor.yaml
@@ -0,0 +1,87 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/firmware/gunyah-hypervisor.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Gunyah Hypervisor
+
+maintainers:
+ - Murali Nalajala <[email protected]>
+ - Elliot Berman <[email protected]>
+
+description: |+
+ On systems which support devicetree, Gunyah generates and overlays a deviceetree overlay which
+ describes the basic configuration of the hypervisor. Virtual machines use this information to determine
+ the capability IDs of the message queues used to communicate with the Gunyah Resource Manager.
+ See also: https://github.com/quic/gunyah-resource-manager/blob/develop/src/vm_creation/dto_construct.c
+
+properties:
+ compatible:
+ items:
+ - const: gunyah-hypervisor-1.0
+ - const: gunyah-hypervisor
+
+ "#address-cells":
+ description: Number of cells needed to represent 64-bit capability IDs.
+ const: 2
+
+ "#size-cells":
+ description: must be 0, because capability IDs are not memory address
+ ranges and do not have a size.
+ const: 0
+
+patternProperties:
+ "^gunyah-resource-mgr(@.*)?":
+ type: object
+ description:
+ Resource Manager node which is required to communicate to Resource
+ Manager VM using Gunyah Message Queues.
+
+ properties:
+ compatible:
+ items:
+ - const: gunyah-resource-manager-1-0
+ - const: gunyah-resource-manager
+
+ reg:
+ items:
+ - description: Gunyah capability ID of the TX message queue
+ - description: Gunyah capability ID of the RX message queue
+
+ interrupts:
+ items:
+ - description: Interrupt for the TX message queue
+ - description: Interrupt for the RX message queue
+
+ additionalProperties: false
+
+ required:
+ - compatible
+ - reg
+ - interrupts
+
+additionalProperties: false
+
+required:
+ - compatible
+ - "#address-cells"
+ - "#size-cells"
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+ hypervisor {
+ #address-cells = <2>;
+ #size-cells = <0>;
+ compatible = "gunyah-hypervisor-1.0", "gunyah-hypervisor";
+
+ gunyah-resource-mgr@0 {
+ compatible = "gunyah-resource-manager-1-0", "gunyah-resource-manager";
+ interrupts = <GIC_SPI 3 IRQ_TYPE_EDGE_RISING>, /* TX full IRQ */
+ <GIC_SPI 4 IRQ_TYPE_EDGE_RISING>; /* RX empty IRQ */
+ reg = <0x00000000 0x00000000>, <0x00000000 0x00000001>;
+ /* TX, RX cap ids */
+ };
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index e88ebb7cbcb8..5b6b6f25c604 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8885,6 +8885,7 @@ M: Elliot Berman <[email protected]>
M: Murali Nalajala <[email protected]>
L: [email protected]
S: Supported
+F: Documentation/devicetree/bindings/firmware/gunyah-hypervisor.yaml
F: Documentation/virt/gunyah/
HABANALABS PCI DRIVER
--
2.25.1
Gunyah provides a console for each VM using the VM console resource
manager APIs. This driver allows console data from other
VMs to be accessed via a TTY device and exports a console device to dump
Linux's own logs to our console.
Signed-off-by: Elliot Berman <[email protected]>
---
MAINTAINERS | 1 +
drivers/tty/Kconfig | 8 +
drivers/tty/Makefile | 1 +
drivers/tty/gunyah_tty.c | 409 +++++++++++++++++++++++++++++++++++++++
4 files changed, 419 insertions(+)
create mode 100644 drivers/tty/gunyah_tty.c
diff --git a/MAINTAINERS b/MAINTAINERS
index a0cba618e5f6..e8d4a6d9491a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8890,6 +8890,7 @@ F: Documentation/devicetree/bindings/firmware/gunyah-hypervisor.yaml
F: Documentation/virt/gunyah/
F: arch/arm64/gunyah/
F: drivers/mailbox/gunyah-msgq.c
+F: drivers/tty/gunyah_tty.c
F: drivers/virt/gunyah/
F: include/asm-generic/gunyah.h
F: include/linux/gunyah*.h
diff --git a/drivers/tty/Kconfig b/drivers/tty/Kconfig
index cc30ff93e2e4..ff86e977f9ac 100644
--- a/drivers/tty/Kconfig
+++ b/drivers/tty/Kconfig
@@ -380,6 +380,14 @@ config RPMSG_TTY
To compile this driver as a module, choose M here: the module will be
called rpmsg_tty.
+config GUNYAH_CONSOLE
+ tristate "Gunyah Consoles"
+ depends on GUNYAH
+ help
+ This enables support for console output using Gunyah's Resource Manager RPC.
+ This is normally used when a secondary VM which does not have exclusive access
+ to a real or virtualized serial device and virtio-console is unavailable.
+
endif # TTY
source "drivers/tty/serdev/Kconfig"
diff --git a/drivers/tty/Makefile b/drivers/tty/Makefile
index 07aca5184a55..d183fbfd835b 100644
--- a/drivers/tty/Makefile
+++ b/drivers/tty/Makefile
@@ -27,5 +27,6 @@ obj-$(CONFIG_GOLDFISH_TTY) += goldfish.o
obj-$(CONFIG_MIPS_EJTAG_FDC_TTY) += mips_ejtag_fdc.o
obj-$(CONFIG_VCC) += vcc.o
obj-$(CONFIG_RPMSG_TTY) += rpmsg_tty.o
+obj-$(CONFIG_GUNYAH_CONSOLE) += gunyah_tty.o
obj-y += ipwireless/
diff --git a/drivers/tty/gunyah_tty.c b/drivers/tty/gunyah_tty.c
new file mode 100644
index 000000000000..80a20da11ad0
--- /dev/null
+++ b/drivers/tty/gunyah_tty.c
@@ -0,0 +1,409 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#define pr_fmt(fmt) "gh_rsc_mgr_console: " fmt
+
+#include <linux/gunyah_rsc_mgr.h>
+#include <linux/auxiliary_bus.h>
+#include <linux/workqueue.h>
+#include <linux/spinlock.h>
+#include <linux/tty_flip.h>
+#include <linux/console.h>
+#include <linux/module.h>
+#include <linux/kfifo.h>
+#include <linux/kref.h>
+#include <linux/slab.h>
+#include <linux/tty.h>
+
+/*
+ * The Linux TTY code does not support dynamic addition of tty derived devices so we need to know
+ * how many tty devices we might need when space is allocated for the tty device. Since VMs might be
+ * added/removed dynamically, we need to make sure we have enough allocated.
+ */
+#define RSC_MGR_TTY_ADAPTERS 16
+
+/* # of payload bytes that can fit in a 1-fragment CONSOLE_WRITE message */
+#define RM_CONS_WRITE_MSG_SIZE ((1 * (GH_MSGQ_MAX_MSG_SIZE - 8)) - 4)
+
+struct rm_cons_port {
+ struct tty_port port;
+ u16 vmid;
+ bool open;
+ unsigned int index;
+
+ DECLARE_KFIFO(put_fifo, char, 1024);
+ spinlock_t fifo_lock;
+ struct work_struct put_work;
+
+ struct rm_cons_data *cons_data;
+};
+
+struct rm_cons_data {
+ struct tty_driver *tty_driver;
+ struct device *dev;
+
+ spinlock_t ports_lock;
+ struct rm_cons_port *ports[RSC_MGR_TTY_ADAPTERS];
+
+ struct notifier_block rsc_mgr_notif;
+ struct console console;
+};
+
+static void put_work_fn(struct work_struct *ws)
+{
+ char buf[RM_CONS_WRITE_MSG_SIZE];
+ int count, ret;
+ struct rm_cons_port *port = container_of(ws, struct rm_cons_port, put_work);
+
+ while (!kfifo_is_empty(&port->put_fifo)) {
+ count = kfifo_out_spinlocked(&port->put_fifo, buf, sizeof(buf), &port->fifo_lock);
+ if (count <= 0)
+ continue;
+
+ ret = gh_rm_console_write(port->vmid, buf, count);
+ if (ret) {
+ pr_warn_once("failed to send characters: %d\n", ret);
+ break;
+ }
+ }
+}
+
+static int rsc_mgr_console_notif(struct notifier_block *nb, unsigned long cmd, void *data)
+{
+ int count, i;
+ struct rm_cons_port *rm_port = NULL;
+ struct tty_port *tty_port = NULL;
+ struct rm_cons_data *cons_data = container_of(nb, struct rm_cons_data, rsc_mgr_notif);
+ const struct gh_rm_notification *notif = data;
+ struct gh_rm_notif_vm_console_chars const * const msg = notif->buff;
+
+ if (cmd != GH_RM_NOTIF_VM_CONSOLE_CHARS ||
+ notif->size < sizeof(*msg))
+ return NOTIFY_DONE;
+
+ spin_lock(&cons_data->ports_lock);
+ for (i = 0; i < RSC_MGR_TTY_ADAPTERS; i++) {
+ if (!cons_data->ports[i])
+ continue;
+ if (cons_data->ports[i]->vmid == msg->vmid) {
+ rm_port = cons_data->ports[i];
+ break;
+ }
+ }
+ if (rm_port)
+ tty_port = tty_port_get(&rm_port->port);
+ spin_unlock(&cons_data->ports_lock);
+
+ if (!rm_port)
+ pr_warn("Received unexpected console characters for VMID %u\n", msg->vmid);
+ if (!tty_port)
+ return NOTIFY_DONE;
+
+ count = tty_buffer_request_room(tty_port, msg->num_bytes);
+ tty_insert_flip_string(tty_port, msg->bytes, count);
+ tty_flip_buffer_push(tty_port);
+
+ tty_port_put(tty_port);
+ return NOTIFY_OK;
+}
+
+static ssize_t vmid_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ struct rm_cons_port *rm_port = dev_get_drvdata(dev);
+
+ if (rm_port->vmid == GH_VMID_SELF)
+ return sysfs_emit(buf, "self\n");
+
+ return sysfs_emit(buf, "%u\n", rm_port->vmid);
+}
+
+static DEVICE_ATTR_RO(vmid);
+
+static struct attribute *rsc_mgr_tty_dev_attrs[] = {
+ &dev_attr_vmid.attr,
+ NULL
+};
+
+static const struct attribute_group rsc_mgr_tty_dev_attr_group = {
+ .attrs = rsc_mgr_tty_dev_attrs,
+};
+
+static const struct attribute_group *rsc_mgr_tty_dev_attr_groups[] = {
+ &rsc_mgr_tty_dev_attr_group,
+ NULL
+};
+
+static int rsc_mgr_tty_open(struct tty_struct *tty, struct file *filp)
+{
+ int ret;
+ struct rm_cons_port *rm_port = dev_get_drvdata(tty->dev);
+
+ if (!rm_port->open) {
+ ret = gh_rm_console_open(rm_port->vmid);
+ if (ret) {
+ pr_err("Failed to open RM console for vmid %x: %d\n", rm_port->vmid, ret);
+ return ret;
+ }
+ rm_port->open = true;
+ }
+
+ return tty_port_open(&rm_port->port, tty, filp);
+}
+
+static void rsc_mgr_tty_close(struct tty_struct *tty, struct file *filp)
+{
+ int ret;
+ struct rm_cons_port *rm_port = dev_get_drvdata(tty->dev);
+
+ if (rm_port->open) {
+ if (rm_port->vmid != GH_VMID_SELF) {
+ ret = gh_rm_console_close(rm_port->vmid);
+ if (ret)
+ pr_warn("Failed to close RM console for vmid %d: %d\n",
+ rm_port->vmid, ret);
+ }
+ rm_port->open = false;
+
+ tty_port_close(&rm_port->port, tty, filp);
+ }
+
+}
+
+static int rsc_mgr_tty_write(struct tty_struct *tty, const unsigned char *buf, int count)
+{
+ struct rm_cons_port *rm_port = dev_get_drvdata(tty->dev);
+ int ret;
+
+ ret = kfifo_in_spinlocked(&rm_port->put_fifo, buf, count, &rm_port->fifo_lock);
+ if (ret > 0)
+ schedule_work(&rm_port->put_work);
+
+ return ret;
+}
+
+static unsigned int rsc_mgr_mgr_tty_write_room(struct tty_struct *tty)
+{
+ struct rm_cons_port *rm_port = dev_get_drvdata(tty->dev);
+
+ return kfifo_avail(&rm_port->put_fifo);
+}
+
+static void rsc_mgr_console_write(struct console *co, const char *buf, unsigned count)
+{
+ struct rm_cons_port *rm_port = co->data;
+ int ret;
+
+ ret = kfifo_in_spinlocked(&rm_port->put_fifo, buf, count, &rm_port->fifo_lock);
+ if (ret > 0)
+ schedule_work(&rm_port->put_work);
+}
+
+static struct tty_driver *rsc_mgr_console_device(struct console *co, int *index)
+{
+ struct rm_cons_port *rm_port = co->data;
+
+ *index = rm_port->index;
+ return rm_port->port.tty->driver;
+}
+
+static int rsc_mgr_console_setup(struct console *co, char *unused)
+{
+ int ret;
+ struct rm_cons_port *rm_port = co->data;
+
+ if (!rm_port->open) {
+ ret = gh_rm_console_open(rm_port->vmid);
+ if (ret) {
+ pr_err("Failed to open RM console for vmid %x: %d\n", rm_port->vmid, ret);
+ return ret;
+ }
+ rm_port->open = true;
+ }
+
+ return 0;
+}
+
+static int rsc_mgr_console_exit(struct console *co)
+{
+ int ret;
+ struct rm_cons_port *rm_port = co->data;
+
+ if (rm_port->open) {
+ ret = gh_rm_console_close(rm_port->vmid);
+ if (ret) {
+ pr_err("Failed to close RM console for vmid %x: %d\n", rm_port->vmid, ret);
+ return ret;
+ }
+ rm_port->open = false;
+ }
+
+ return 0;
+}
+
+static const struct tty_operations rsc_mgr_tty_ops = {
+ .open = rsc_mgr_tty_open,
+ .close = rsc_mgr_tty_close,
+ .write = rsc_mgr_tty_write,
+ .write_room = rsc_mgr_mgr_tty_write_room,
+};
+
+static void rsc_mgr_port_destruct(struct tty_port *port)
+{
+ struct rm_cons_port *rm_port = container_of(port, struct rm_cons_port, port);
+ struct rm_cons_data *cons_data = rm_port->cons_data;
+
+ spin_lock(&cons_data->ports_lock);
+ WARN_ON(cons_data->ports[rm_port->index] != rm_port);
+ cons_data->ports[rm_port->index] = NULL;
+ spin_unlock(&cons_data->ports_lock);
+ kfree(rm_port);
+}
+
+static const struct tty_port_operations rsc_mgr_port_ops = {
+ .destruct = rsc_mgr_port_destruct,
+};
+
+static struct rm_cons_port *rsc_mgr_port_create(struct rm_cons_data *cons_data, u16 vmid)
+{
+ struct rm_cons_port *rm_port;
+ struct device *ttydev;
+ unsigned int index;
+ int ret;
+
+ rm_port = kzalloc(sizeof(*rm_port), GFP_KERNEL);
+ rm_port->vmid = vmid;
+ INIT_KFIFO(rm_port->put_fifo);
+ spin_lock_init(&rm_port->fifo_lock);
+ INIT_WORK(&rm_port->put_work, put_work_fn);
+ tty_port_init(&rm_port->port);
+ rm_port->port.ops = &rsc_mgr_port_ops;
+
+ spin_lock(&cons_data->ports_lock);
+ for (index = 0; index < RSC_MGR_TTY_ADAPTERS; index++) {
+ if (!cons_data->ports[index]) {
+ cons_data->ports[index] = rm_port;
+ rm_port->index = index;
+ break;
+ }
+ }
+ spin_unlock(&cons_data->ports_lock);
+ if (index >= RSC_MGR_TTY_ADAPTERS) {
+ ret = -ENOSPC;
+ goto err_put_port;
+ }
+
+ ttydev = tty_port_register_device_attr(&rm_port->port, cons_data->tty_driver, index,
+ cons_data->dev, rm_port, rsc_mgr_tty_dev_attr_groups);
+ if (IS_ERR(ttydev)) {
+ ret = PTR_ERR(ttydev);
+ goto err_put_port;
+ }
+
+ return rm_port;
+err_put_port:
+ tty_port_put(&rm_port->port);
+ return ERR_PTR(ret);
+}
+
+static int rsc_mgr_console_probe(struct auxiliary_device *auxdev,
+ const struct auxiliary_device_id *aux_dev_id)
+{
+ struct rm_cons_data *cons_data;
+ struct rm_cons_port *rm_port;
+ int ret;
+ u16 vmid;
+
+ cons_data = devm_kzalloc(&auxdev->dev, sizeof(*cons_data), GFP_KERNEL);
+ if (!cons_data)
+ return -ENOMEM;
+ dev_set_drvdata(&auxdev->dev, cons_data);
+ cons_data->dev = &auxdev->dev;
+
+ cons_data->tty_driver = tty_alloc_driver(RSC_MGR_TTY_ADAPTERS,
+ TTY_DRIVER_REAL_RAW | TTY_DRIVER_DYNAMIC_DEV);
+ if (IS_ERR(cons_data->tty_driver))
+ return PTR_ERR(cons_data->tty_driver);
+
+ cons_data->tty_driver->driver_name = "gh";
+ cons_data->tty_driver->name = "ttyGH";
+ cons_data->tty_driver->type = TTY_DRIVER_TYPE_SYSTEM;
+ cons_data->tty_driver->init_termios = tty_std_termios;
+ tty_set_operations(cons_data->tty_driver, &rsc_mgr_tty_ops);
+
+ ret = tty_register_driver(cons_data->tty_driver);
+ if (ret) {
+ dev_err(&auxdev->dev, "Could not register tty driver: %d\n", ret);
+ goto err_put_tty;
+ }
+
+ spin_lock_init(&cons_data->ports_lock);
+
+ cons_data->rsc_mgr_notif.notifier_call = rsc_mgr_console_notif;
+ ret = gh_rm_register_notifier(&cons_data->rsc_mgr_notif);
+ if (ret) {
+ dev_err(&auxdev->dev, "Could not register for resource manager notifications: %d\n",
+ ret);
+ goto err_put_tty;
+ }
+
+ rm_port = rsc_mgr_port_create(cons_data, GH_VMID_SELF);
+ if (IS_ERR(rm_port)) {
+ ret = PTR_ERR(rm_port);
+ dev_err(&auxdev->dev, "Could not create own console: %d\n", ret);
+ goto err_unreg_notif;
+ }
+
+ strncpy(cons_data->console.name, "ttyGH", sizeof(cons_data->console.name));
+ cons_data->console.write = rsc_mgr_console_write;
+ cons_data->console.device = rsc_mgr_console_device;
+ cons_data->console.setup = rsc_mgr_console_setup;
+ cons_data->console.exit = rsc_mgr_console_exit;
+ cons_data->console.index = rm_port->index;
+ cons_data->console.data = rm_port;
+ register_console(&cons_data->console);
+
+ ret = gh_rm_get_vmid(&vmid);
+ if (!ret) {
+ rm_port = rsc_mgr_port_create(cons_data, vmid);
+ if (IS_ERR(rm_port))
+ dev_warn(&auxdev->dev, "Could not create loop-back console: %ld\n",
+ PTR_ERR(rm_port));
+ } else {
+ dev_warn(&auxdev->dev, "Failed to get this VM's VMID: %d. Not creating loop-back console\n",
+ ret);
+ }
+
+ return 0;
+err_unreg_notif:
+ gh_rm_unregister_notifier(&cons_data->rsc_mgr_notif);
+err_put_tty:
+ tty_driver_kref_put(cons_data->tty_driver);
+ return ret;
+}
+
+static void rsc_mgr_console_remove(struct auxiliary_device *auxdev)
+{
+ struct rm_cons_data *cons_data = dev_get_drvdata(&auxdev->dev);
+
+ unregister_console(&cons_data->console);
+ gh_rm_unregister_notifier(&cons_data->rsc_mgr_notif);
+ tty_driver_kref_put(cons_data->tty_driver);
+}
+
+static struct auxiliary_device_id rsc_mgr_console_ids[] = {
+ { .name = "gunyah_rsc_mgr.console" },
+ {}
+};
+MODULE_DEVICE_TABLE(auxiliary, rsc_mgr_console_ids);
+
+static struct auxiliary_driver rsc_mgr_console_drv = {
+ .probe = rsc_mgr_console_probe,
+ .remove = rsc_mgr_console_remove,
+ .id_table = rsc_mgr_console_ids,
+};
+module_auxiliary_driver(rsc_mgr_console_drv);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Gunyah Console");
--
2.25.1
Add architecture-independent standard error codes, types, and macros for
Gunyah hypercalls.
Signed-off-by: Elliot Berman <[email protected]>
---
MAINTAINERS | 1 +
include/asm-generic/gunyah.h | 74 ++++++++++++++++++++++++++++++++++++
2 files changed, 75 insertions(+)
create mode 100644 include/asm-generic/gunyah.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 5b6b6f25c604..31bda0197f9a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8887,6 +8887,7 @@ L: [email protected]
S: Supported
F: Documentation/devicetree/bindings/firmware/gunyah-hypervisor.yaml
F: Documentation/virt/gunyah/
+F: include/asm-generic/gunyah.h
HABANALABS PCI DRIVER
M: Oded Gabbay <[email protected]>
diff --git a/include/asm-generic/gunyah.h b/include/asm-generic/gunyah.h
new file mode 100644
index 000000000000..64a02dd3b5ad
--- /dev/null
+++ b/include/asm-generic/gunyah.h
@@ -0,0 +1,74 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#ifndef _ASM_GUNYAH_H
+#define _ASM_GUNYAH_H
+
+#include <linux/types.h>
+#include <linux/errno.h>
+
+/* Common Gunyah macros */
+#define GH_CAPID_INVAL U64_MAX
+#define GH_VMID_ROOT_VM 0xff
+
+#define GH_ERROR_OK 0
+
+#define GH_ERROR_UNIMPLEMENTED -1
+#define GH_ERROR_RETRY -2
+
+#define GH_ERROR_ARG_INVAL 1
+#define GH_ERROR_ARG_SIZE 2
+#define GH_ERROR_ARG_ALIGN 3
+
+#define GH_ERROR_NOMEM 10
+
+#define GH_ERROR_ADDR_OVFL 20
+#define GH_ERROR_ADDR_UNFL 21
+#define GH_ERROR_ADDR_INVAL 22
+
+#define GH_ERROR_DENIED 30
+#define GH_ERROR_BUSY 31
+#define GH_ERROR_IDLE 32
+
+#define GH_ERROR_IRQ_BOUND 40
+#define GH_ERROR_IRQ_UNBOUND 41
+
+#define GH_ERROR_CSPACE_CAP_NULL 50
+#define GH_ERROR_CSPACE_CAP_REVOKED 51
+#define GH_ERROR_CSPACE_WRONG_OBJ_TYPE 52
+#define GH_ERROR_CSPACE_INSUF_RIGHTS 53
+#define GH_ERROR_CSPACE_FULL 54
+
+#define GH_ERROR_MSGQUEUE_EMPTY 60
+#define GH_ERROR_MSGQUEUE_FULL 61
+
+static inline int gh_remap_error(int gh_error)
+{
+ switch (gh_error) {
+ case GH_ERROR_OK:
+ return 0;
+ case GH_ERROR_NOMEM:
+ return -ENOMEM;
+ case GH_ERROR_DENIED:
+ case GH_ERROR_CSPACE_CAP_NULL:
+ case GH_ERROR_CSPACE_CAP_REVOKED:
+ case GH_ERROR_CSPACE_WRONG_OBJ_TYPE:
+ case GH_ERROR_CSPACE_INSUF_RIGHTS:
+ case GH_ERROR_CSPACE_FULL:
+ return -EACCES;
+ case GH_ERROR_BUSY:
+ case GH_ERROR_IDLE:
+ return -EBUSY;
+ case GH_ERROR_IRQ_BOUND:
+ case GH_ERROR_IRQ_UNBOUND:
+ case GH_ERROR_MSGQUEUE_FULL:
+ case GH_ERROR_MSGQUEUE_EMPTY:
+ return -EPERM;
+ default:
+ return -EINVAL;
+ }
+}
+
+#endif
--
2.25.1
On 9/28/2022 8:43 PM, Bagas Sanjaya wrote:
> On Wed, Sep 28, 2022 at 12:56:20PM -0700, Elliot Berman wrote:
>> diff --git a/Documentation/virt/gunyah/index.rst b/Documentation/virt/gunyah/index.rst
>> new file mode 100644
>> index 000000000000..959f451caccd
>> --- /dev/null
>> +++ b/Documentation/virt/gunyah/index.rst
>> @@ -0,0 +1,114 @@
>> +.. SPDX-License-Identifier: GPL-2.0
>> +
>> +=================
>> +Gunyah Hypervisor
>> +=================
>> +
>> +.. toctree::
>> + :maxdepth: 1
>> +
>> + message-queue
>> +
>> +Gunyah is a Type-1 hypervisor which is independent of any OS kernel, and runs in
>> +a higher CPU privilege level. It does not depend on any lower-privileged operating system
>> +for its core functionality. This increases its security and can support a much smaller
>> +trusted computing base than a Type-2 hypervisor.
>> +
>> +Gunyah is an open source hypervisor. The source repo is available at
>> +https://github.com/quic/gunyah-hypervisor.
>> +
>> +Gunyah provides these following features.
>> +
>> +- Scheduling:
>> +
>> + A scheduler for virtual CPUs (vCPUs) on physical CPUs and enables time-sharing
>> + of the CPUs. Gunyah supports two models of scheduling:
>> +
>> + 1. "Behind the back" scheduling in which Gunyah hypervisor schedules vCPUS on its own
>> + 2. "Proxy" scheduling in which a delegated VM can donate part of one of its vCPU slice
>> + to another VM's vCPU via a hypercall.
>> +
>> +- Memory Management:
>> +
>> + APIs handling memory, abstracted as objects, limiting direct use of physical
>> + addresses. Memory ownership and usage tracking of all memory under its control.
>> + Memory partitioning between VMs is a fundamental security feature.
>> +
>> +- Interrupt Virtualization:
>> +
>> + Uses CPU hardware interrupt virtualization capabilities. Interrupts are handled
>> + in the hypervisor and routed to the assigned VM.
>> +
>> +- Inter-VM Communication:
>> +
>> + There are several different mechanisms provided for communicating between VMs.
>> +
>> +- Virtual platform:
>> +
>> + Architectural devices such as interrupt controllers and CPU timers are directly provided
>> + by the hypervisor as well as core virtual platform devices and system APIs such as ARM PSCI.
>> +
>> +- Device Virtualization:
>> +
>> + Para-virtualization of devices is supported using inter-VM communication.
>> +
>> +Architectures supported
>> +=======================
>> +AArch64 with a GIC
>> +
>> +Resources and Capabilities
>> +==========================
>> +
>> +Some services or resources provided by the Gunyah hypervisor are described to a virtual machine by
>> +capability IDs. For instance, inter-VM communication is performed with doorbells and message queues.
>> +Gunyah allows access to manipulate that doorbell via the capability ID. These devices are described
>> +in Linux as a struct gunyah_resource.
>> +
>> +High level management of these resources is performed by the resource manager VM. RM informs a
>> +guest VM about resources it can access through either the device tree or via guest-initiated RPC.
>> +
>> +For each virtual machine, Gunyah maintains a table of resources which can be accessed by that VM.
>> +An entry in this table is called a "capability" and VMs can only access resources via this
>> +capability table. Hence, virtual Gunyah devices are referenced by a "capability IDs" and not a
>> +"resource IDs". A VM can have multiple capability IDs mapping to the same resource. If 2 VMs have
>> +access to the same resource, they may not be using the same capability ID to access that resource
>> +since the tables are independent per VM.
>> +
>> +Resource Manager
>> +================
>> +
>> +The resource manager (RM) is a privileged application VM supporting the Gunyah Hypervisor.
>> +It provides policy enforcement aspects of the virtualization system. The resource manager can
>> +be treated as an extension of the Hypervisor but is separated to its own partition to ensure
>> +that the hypervisor layer itself remains small and secure and to maintain a separation of policy
>> +and mechanism in the platform. On arm64, RM runs at NS-EL1 similar to other virtual machines.
>> +
>> +Communication with the resource manager from each guest VM happens with message-queue.rst. Details
>> +about the specific messages can be found in drivers/virt/gunyah/rsc_mgr.c
>> +
>> +::
>> +
>> + +-------+ +--------+ +--------+
>> + | RM | | VM_A | | VM_B |
>> + +-.-.-.-+ +---.----+ +---.----+
>> + | | | |
>> + +-.-.-----------.------------.----+
>> + | | \==========/ | |
>> + | \========================/ |
>> + | Gunyah |
>> + +---------------------------------+
>> +
>> +The source for the resource manager is available at https://github.com/quic/gunyah-resource-manager.
>> +
>> +The resource manager provides the following features:
>> +
>> +- VM lifecycle management: allocating a VM, starting VMs, destruction of VMs
>> +- VM access control policy, including memory sharing and lending
>> +- Interrupt routing configuration
>> +- Forwarding of system-level events (e.g. VM shutdown) to owner VM
>> +
>> +When booting a virtual machine which uses a devicetree, resource manager overlays a
>> +/hypervisor node. This node can let Linux know it is running as a Gunyah guest VM,
>> +how to communicate with resource manager, and basic description and capabilities of
>> +this VM. See Documentation/devicetree/bindings/firmware/gunyah-hypervisor.yaml for a description
>> +of this node.
>
> The documentation LGTM.
>
>> diff --git a/Documentation/virt/gunyah/message-queue.rst b/Documentation/virt/gunyah/message-queue.rst
>> new file mode 100644
>> index 000000000000..e130f124ed52
>> --- /dev/null
>> +++ b/Documentation/virt/gunyah/message-queue.rst
>> <snipped>...
>> +The diagram below shows how message queue works. A typical configuration involves
>> +2 message queues. Message queue 1 allows VM_A to send messages to VM_B. Message
>> +queue 2 allows VM_B to send messages to VM_A.
>> +
>> +1. VM_A sends a message of up to 1024 bytes in length. It raises a hypercall
>> + with the message to inform the hypervisor to add the message to
>> + message queue 1's queue.
>> +2. Gunyah raises the corresponding interrupt for VM_B when any of these happens:
>> + a. gh_msgq_send has PUSH flag. Queue is immediately flushed. This is the typical case.
>> + b. Explicility with gh_msgq_push command from VM_A.
>> + c. Message queue has reached a threshold depth.
>> +3. VM_B calls gh_msgq_recv and Gunyah copies message to requested buffer.
>> +
>
> The nested list above should be separated with blank lines to be
> rendered properly:
>
> ---- >8 ----
>
> diff --git a/Documentation/virt/gunyah/message-queue.rst b/Documentation/virt/gunyah/message-queue.rst
> index e130f124ed525a..afaad99db215e6 100644
> --- a/Documentation/virt/gunyah/message-queue.rst
> +++ b/Documentation/virt/gunyah/message-queue.rst
> @@ -20,9 +20,11 @@ queue 2 allows VM_B to send messages to VM_A.
> with the message to inform the hypervisor to add the message to
> message queue 1's queue.
> 2. Gunyah raises the corresponding interrupt for VM_B when any of these happens:
> +
> a. gh_msgq_send has PUSH flag. Queue is immediately flushed. This is the typical case.
> b. Explicility with gh_msgq_push command from VM_A.
> c. Message queue has reached a threshold depth.
> +
> 3. VM_B calls gh_msgq_recv and Gunyah copies message to requested buffer.
>
> For VM_B to send a message to VM_A, the process is identical, except that hypercalls
>
> Thanks.
>
Thanks! Applied for next version.
On Wed, Sep 28, 2022 at 12:56:20PM -0700, Elliot Berman wrote:
> diff --git a/Documentation/virt/gunyah/index.rst b/Documentation/virt/gunyah/index.rst
> new file mode 100644
> index 000000000000..959f451caccd
> --- /dev/null
> +++ b/Documentation/virt/gunyah/index.rst
> @@ -0,0 +1,114 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +=================
> +Gunyah Hypervisor
> +=================
> +
> +.. toctree::
> + :maxdepth: 1
> +
> + message-queue
> +
> +Gunyah is a Type-1 hypervisor which is independent of any OS kernel, and runs in
> +a higher CPU privilege level. It does not depend on any lower-privileged operating system
> +for its core functionality. This increases its security and can support a much smaller
> +trusted computing base than a Type-2 hypervisor.
> +
> +Gunyah is an open source hypervisor. The source repo is available at
> +https://github.com/quic/gunyah-hypervisor.
> +
> +Gunyah provides these following features.
> +
> +- Scheduling:
> +
> + A scheduler for virtual CPUs (vCPUs) on physical CPUs and enables time-sharing
> + of the CPUs. Gunyah supports two models of scheduling:
> +
> + 1. "Behind the back" scheduling in which Gunyah hypervisor schedules vCPUS on its own
> + 2. "Proxy" scheduling in which a delegated VM can donate part of one of its vCPU slice
> + to another VM's vCPU via a hypercall.
> +
> +- Memory Management:
> +
> + APIs handling memory, abstracted as objects, limiting direct use of physical
> + addresses. Memory ownership and usage tracking of all memory under its control.
> + Memory partitioning between VMs is a fundamental security feature.
> +
> +- Interrupt Virtualization:
> +
> + Uses CPU hardware interrupt virtualization capabilities. Interrupts are handled
> + in the hypervisor and routed to the assigned VM.
> +
> +- Inter-VM Communication:
> +
> + There are several different mechanisms provided for communicating between VMs.
> +
> +- Virtual platform:
> +
> + Architectural devices such as interrupt controllers and CPU timers are directly provided
> + by the hypervisor as well as core virtual platform devices and system APIs such as ARM PSCI.
> +
> +- Device Virtualization:
> +
> + Para-virtualization of devices is supported using inter-VM communication.
> +
> +Architectures supported
> +=======================
> +AArch64 with a GIC
> +
> +Resources and Capabilities
> +==========================
> +
> +Some services or resources provided by the Gunyah hypervisor are described to a virtual machine by
> +capability IDs. For instance, inter-VM communication is performed with doorbells and message queues.
> +Gunyah allows access to manipulate that doorbell via the capability ID. These devices are described
> +in Linux as a struct gunyah_resource.
> +
> +High level management of these resources is performed by the resource manager VM. RM informs a
> +guest VM about resources it can access through either the device tree or via guest-initiated RPC.
> +
> +For each virtual machine, Gunyah maintains a table of resources which can be accessed by that VM.
> +An entry in this table is called a "capability" and VMs can only access resources via this
> +capability table. Hence, virtual Gunyah devices are referenced by a "capability IDs" and not a
> +"resource IDs". A VM can have multiple capability IDs mapping to the same resource. If 2 VMs have
> +access to the same resource, they may not be using the same capability ID to access that resource
> +since the tables are independent per VM.
> +
> +Resource Manager
> +================
> +
> +The resource manager (RM) is a privileged application VM supporting the Gunyah Hypervisor.
> +It provides policy enforcement aspects of the virtualization system. The resource manager can
> +be treated as an extension of the Hypervisor but is separated to its own partition to ensure
> +that the hypervisor layer itself remains small and secure and to maintain a separation of policy
> +and mechanism in the platform. On arm64, RM runs at NS-EL1 similar to other virtual machines.
> +
> +Communication with the resource manager from each guest VM happens with message-queue.rst. Details
> +about the specific messages can be found in drivers/virt/gunyah/rsc_mgr.c
> +
> +::
> +
> + +-------+ +--------+ +--------+
> + | RM | | VM_A | | VM_B |
> + +-.-.-.-+ +---.----+ +---.----+
> + | | | |
> + +-.-.-----------.------------.----+
> + | | \==========/ | |
> + | \========================/ |
> + | Gunyah |
> + +---------------------------------+
> +
> +The source for the resource manager is available at https://github.com/quic/gunyah-resource-manager.
> +
> +The resource manager provides the following features:
> +
> +- VM lifecycle management: allocating a VM, starting VMs, destruction of VMs
> +- VM access control policy, including memory sharing and lending
> +- Interrupt routing configuration
> +- Forwarding of system-level events (e.g. VM shutdown) to owner VM
> +
> +When booting a virtual machine which uses a devicetree, resource manager overlays a
> +/hypervisor node. This node can let Linux know it is running as a Gunyah guest VM,
> +how to communicate with resource manager, and basic description and capabilities of
> +this VM. See Documentation/devicetree/bindings/firmware/gunyah-hypervisor.yaml for a description
> +of this node.
The documentation LGTM.
> diff --git a/Documentation/virt/gunyah/message-queue.rst b/Documentation/virt/gunyah/message-queue.rst
> new file mode 100644
> index 000000000000..e130f124ed52
> --- /dev/null
> +++ b/Documentation/virt/gunyah/message-queue.rst
> <snipped>...
> +The diagram below shows how message queue works. A typical configuration involves
> +2 message queues. Message queue 1 allows VM_A to send messages to VM_B. Message
> +queue 2 allows VM_B to send messages to VM_A.
> +
> +1. VM_A sends a message of up to 1024 bytes in length. It raises a hypercall
> + with the message to inform the hypervisor to add the message to
> + message queue 1's queue.
> +2. Gunyah raises the corresponding interrupt for VM_B when any of these happens:
> + a. gh_msgq_send has PUSH flag. Queue is immediately flushed. This is the typical case.
> + b. Explicility with gh_msgq_push command from VM_A.
> + c. Message queue has reached a threshold depth.
> +3. VM_B calls gh_msgq_recv and Gunyah copies message to requested buffer.
> +
The nested list above should be separated with blank lines to be
rendered properly:
---- >8 ----
diff --git a/Documentation/virt/gunyah/message-queue.rst b/Documentation/virt/gunyah/message-queue.rst
index e130f124ed525a..afaad99db215e6 100644
--- a/Documentation/virt/gunyah/message-queue.rst
+++ b/Documentation/virt/gunyah/message-queue.rst
@@ -20,9 +20,11 @@ queue 2 allows VM_B to send messages to VM_A.
with the message to inform the hypervisor to add the message to
message queue 1's queue.
2. Gunyah raises the corresponding interrupt for VM_B when any of these happens:
+
a. gh_msgq_send has PUSH flag. Queue is immediately flushed. This is the typical case.
b. Explicility with gh_msgq_push command from VM_A.
c. Message queue has reached a threshold depth.
+
3. VM_B calls gh_msgq_recv and Gunyah copies message to requested buffer.
For VM_B to send a message to VM_A, the process is identical, except that hypercalls
Thanks.
--
An old man doll... just what I always wanted! - Clara
On Wed, 2022-09-28 at 12:56 -0700, Elliot Berman wrote:
> Add a sysfs node to list the features that the Gunyah hypervisor and
> Linux supports. For now, Linux support cspace (capability IDs) and
> message queues, so only report those..
[]
> diff --git a/drivers/virt/gunyah/sysfs.c b/drivers/virt/gunyah/sysfs.c
[]
> @@ -25,9 +25,24 @@ static ssize_t variant_show(struct kobject *kobj, struct kobj_attribute *attr, c
> }
> static struct kobj_attribute variant_attr = __ATTR_RO(variant);
>
> +static ssize_t features_show(struct kobject *kobj, struct kobj_attribute *attr, char *buffer)
> +{
> + int len = 0;
> +
> + if (GH_IDENTIFY_PARTITION_CSPACE(gunyah_api.flags))
> + len += sysfs_emit_at(buffer, len, "cspace ");
> + if (GH_IDENTIFY_MSGQUEUE(gunyah_api.flags))
> + len += sysfs_emit_at(buffer, len, "message-queue ");
> +
> + len += sysfs_emit_at(buffer, len, "\n");
> + return len;
> +}
It's generally nicer to avoid unnecessary output spaces.
Perhaps:
{
int len = 0;
if (GH_IDENTIFY_PARTITION_CSPACE(gunyah_api.flags))
len += sysfs_emit_at(buffer, len, "cspace");
if (GH_IDENTIFY_MSGQUEUE(gunyah_api.flags)) {
if (len)
len += sysfs_emit_at(buffer, len, " ");
len += sysfs_emit_at(buffer, len, "message-queue");
}
len += sysfs_emit_at(buffer, len, "\n");
return len;
}
On Wed, Sep 28, 2022 at 12:56:28PM -0700, Elliot Berman wrote:
[..]
> diff --git a/drivers/mailbox/gunyah-msgq.c b/drivers/mailbox/gunyah-msgq.c
[..]
> +static int gh_msgq_send_data(struct mbox_chan *chan, void *data)
> +{
> + struct gunyah_msgq *msgq = mbox_chan_to_msgq(chan);
> + struct gunyah_msgq_tx_data *msgq_data = data;
The mailbox framework abstracts hardware mailboxes and @data
should be the data to be written to the hardware.
Using the void * to pass composite data types means that the client and
provider are tightly coupled and as such the mailbox framework does not
provide you any abstraction.
You also only expose a single channel, so a direct function call between
the two specific drivers would be a better fit.
Regards,
Bjorn
On Thu, Sep 29, 2022 at 11:59 AM Bjorn Andersson <[email protected]> wrote:
>
> On Wed, Sep 28, 2022 at 12:56:28PM -0700, Elliot Berman wrote:
> [..]
> > diff --git a/drivers/mailbox/gunyah-msgq.c b/drivers/mailbox/gunyah-msgq.c
> [..]
> > +static int gh_msgq_send_data(struct mbox_chan *chan, void *data)
> > +{
> > + struct gunyah_msgq *msgq = mbox_chan_to_msgq(chan);
> > + struct gunyah_msgq_tx_data *msgq_data = data;
>
> The mailbox framework abstracts hardware mailboxes and @data
> should be the data to be written to the hardware.
>
> Using the void * to pass composite data types means that the client and
> provider are tightly coupled and as such the mailbox framework does not
> provide you any abstraction.
>
It is so for every platform, because there is no "standard format" in
which data is to be exchanged between endpoints.
Mailbox api is used mainly to avoid redoing message queuing and
response handling.
cheers.
On 9/29/2022 12:36 AM, Joe Perches wrote:
> On Wed, 2022-09-28 at 12:56 -0700, Elliot Berman wrote:
>> Add a sysfs node to list the features that the Gunyah hypervisor and
>> Linux supports. For now, Linux support cspace (capability IDs) and
>> message queues, so only report those..
> []
>> diff --git a/drivers/virt/gunyah/sysfs.c b/drivers/virt/gunyah/sysfs.c
> []
>> @@ -25,9 +25,24 @@ static ssize_t variant_show(struct kobject *kobj, struct kobj_attribute *attr, c
>> }
>> static struct kobj_attribute variant_attr = __ATTR_RO(variant);
>>
>> +static ssize_t features_show(struct kobject *kobj, struct kobj_attribute *attr, char *buffer)
>> +{
>> + int len = 0;
>> +
>> + if (GH_IDENTIFY_PARTITION_CSPACE(gunyah_api.flags))
>> + len += sysfs_emit_at(buffer, len, "cspace ");
>> + if (GH_IDENTIFY_MSGQUEUE(gunyah_api.flags))
>> + len += sysfs_emit_at(buffer, len, "message-queue ");
>> +
>> + len += sysfs_emit_at(buffer, len, "\n");
>> + return len;
>> +}
>
> It's generally nicer to avoid unnecessary output spaces.
>
> Perhaps:
>
> {
> int len = 0;
>
> if (GH_IDENTIFY_PARTITION_CSPACE(gunyah_api.flags))
> len += sysfs_emit_at(buffer, len, "cspace");
> if (GH_IDENTIFY_MSGQUEUE(gunyah_api.flags)) {
> if (len)
> len += sysfs_emit_at(buffer, len, " ");
> len += sysfs_emit_at(buffer, len, "message-queue");
> }
>
> len += sysfs_emit_at(buffer, len, "\n");
>
> return len;
> }
>
Thanks! Applied for the next version.
On Wed, Sep 28, 2022 at 12:56:25PM -0700, Elliot Berman wrote:
> Add /sys/hypervisor support when detecting that Linux is running in a
> Gunyah environment. Export the version of Gunyah which is reported via
> the hyp_identify hypercall.
>
> Signed-off-by: Elliot Berman <[email protected]>
> ---
> .../ABI/testing/sysfs-hypervisor-gunyah | 15 ++++
> MAINTAINERS | 1 +
> drivers/virt/Makefile | 1 +
> drivers/virt/gunyah/Makefile | 2 +
> drivers/virt/gunyah/sysfs.c | 71 +++++++++++++++++++
> 5 files changed, 90 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-hypervisor-gunyah
> create mode 100644 drivers/virt/gunyah/Makefile
> create mode 100644 drivers/virt/gunyah/sysfs.c
>
> diff --git a/Documentation/ABI/testing/sysfs-hypervisor-gunyah b/Documentation/ABI/testing/sysfs-hypervisor-gunyah
> new file mode 100644
> index 000000000000..7d74e74e9edd
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-hypervisor-gunyah
> @@ -0,0 +1,15 @@
> +What: /sys/hypervisor/gunyah/api
> +Date: October 2022
> +KernelVersion: 6.1
> +Contact: [email protected]
> +Description: If running under Gunyah:
> + The Gunyah API version.
What does this version mean? What format is it in?
> +
> +What: /sys/hypervisor/gunyah/variant
> +Date: October 2022
> +KernelVersion: 6.1
> +Contact: [email protected]
> +Description: If running under Gunyah:
> + Reports the build variant of Gunyah:
> + The open source build of Gunyah will report "81".
> + The Qualcomm build of Gunyah will report "72".
So there are only 2 versions variants? What happens when you get a
third? And why the odd numbers?
What will userspace do with this information and what tool will parse
it?
> diff --git a/MAINTAINERS b/MAINTAINERS
> index feafac12db35..a26e67ef36b4 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8885,6 +8885,7 @@ M: Elliot Berman <[email protected]>
> M: Murali Nalajala <[email protected]>
> L: [email protected]
> S: Supported
> +F: Documentation/ABI/testing/sysfs-hypervisor-gunyah
> F: Documentation/devicetree/bindings/firmware/gunyah-hypervisor.yaml
> F: Documentation/virt/gunyah/
> F: arch/arm64/gunyah/
> diff --git a/drivers/virt/Makefile b/drivers/virt/Makefile
> index 093674e05c40..10b87f934730 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-$(CONFIG_GUNYAH) += gunyah/
> diff --git a/drivers/virt/gunyah/Makefile b/drivers/virt/gunyah/Makefile
> new file mode 100644
> index 000000000000..e15f16c17142
> --- /dev/null
> +++ b/drivers/virt/gunyah/Makefile
> @@ -0,0 +1,2 @@
> +gunyah-y += sysfs.o
> +obj-$(CONFIG_GUNYAH) += gunyah.o
> diff --git a/drivers/virt/gunyah/sysfs.c b/drivers/virt/gunyah/sysfs.c
> new file mode 100644
> index 000000000000..ec11510cbece
> --- /dev/null
> +++ b/drivers/virt/gunyah/sysfs.c
> @@ -0,0 +1,71 @@
> +// 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/kobject.h>
> +#include <linux/module.h>
> +#include <linux/printk.h>
> +#include <linux/init.h>
> +#include <asm-generic/gunyah.h>
> +
> +static struct gh_hypercall_hyp_identify_resp gunyah_api;
> +
> +static ssize_t api_show(struct kobject *kobj, struct kobj_attribute *attr, char *buffer)
> +{
> + return sysfs_emit(buffer, "%d\n", (int)GH_API_INFO_API_VERSION(gunyah_api.api_info));
> +}
> +static struct kobj_attribute api_attr = __ATTR_RO(api);
> +
> +static ssize_t variant_show(struct kobject *kobj, struct kobj_attribute *attr, char *buffer)
> +{
> + return sysfs_emit(buffer, "%d\n", (int)GH_API_INFO_VARIANT(gunyah_api.api_info));
> +}
> +static struct kobj_attribute variant_attr = __ATTR_RO(variant);
> +
> +static struct attribute *gunyah_attrs[] = {
> + &api_attr.attr,
> + &variant_attr.attr,
> + NULL
> +};
> +
> +static const struct attribute_group gunyah_group = {
> + .name = "gunyah",
> + .attrs = gunyah_attrs,
> +};
> +
> +static int __init gunyah_init(void)
> +{
> + u32 uid[4];
> +
> + gh_hypercall_get_uid(uid);
No error checking?
> +
> + if (!(gh_uid_matches(GUNYAH, uid) || gh_uid_matches(QC_HYP, uid)))
> + return 0;
> +
> + gh_hypercall_hyp_identify(&gunyah_api);
> +
> + if (GH_API_INFO_API_VERSION(gunyah_api.api_info) != 1) {
> + pr_warn("Unrecognized gunyah version: %llu. Currently supported: 1\n",
> + GH_API_INFO_API_VERSION(gunyah_api.api_info));
Shouldn't the "1" be defined somewhere?
> + return 0;
> + }
> +
> + pr_notice("Running under Gunyah hypervisor %llx/v%lld\n",
> + GH_API_INFO_VARIANT(gunyah_api.api_info),
> + GH_API_INFO_API_VERSION(gunyah_api.api_info));
When kernel code is working properly, it is quiet. What is this going
to be used for?
thanks,
greg k-h
On Wed, Sep 28, 2022 at 12:56:29PM -0700, Elliot Berman wrote:
> Add a sysfs node to list the features that the Gunyah hypervisor and
> Linux supports. For now, Linux support cspace (capability IDs) and
> message queues, so only report those..
>
> Signed-off-by: Elliot Berman <[email protected]>
> ---
> Documentation/ABI/testing/sysfs-hypervisor-gunyah | 15 +++++++++++++++
> drivers/virt/gunyah/sysfs.c | 15 +++++++++++++++
> 2 files changed, 30 insertions(+)
>
> diff --git a/Documentation/ABI/testing/sysfs-hypervisor-gunyah b/Documentation/ABI/testing/sysfs-hypervisor-gunyah
> index 7d74e74e9edd..6d0cde30355a 100644
> --- a/Documentation/ABI/testing/sysfs-hypervisor-gunyah
> +++ b/Documentation/ABI/testing/sysfs-hypervisor-gunyah
> @@ -1,3 +1,18 @@
> +What: /sys/hypervisor/gunyah/features
> +Date: October 2022
> +KernelVersion: 6.1
> +Contact: [email protected]
> +Description: If running under Gunyah:
> + Space separated list of features supported by Linux and Gunyah:
> + "cspace": Gunyah devices
> + "doorbell": Sending/receiving virtual interrupts via Gunyah doorbells
> + "message-queue": Sending/receiving messages via Gunyah message queues
> + "vic": Interrupt lending
> + "vpm": Virtual platform management
> + "vcpu": Virtual CPU management
> + "memextent": Memory lending/management
> + "trace": Gunyah hypervisor tracing
Please no. Why do you really need this type of list? What hypervisor
will NOT have them all present already? Who will use this file and what
will it be used for?
sysfs files should just be 1 value and not need to be parsed. Yes, we
have lists of features at times, but really, you need a very very good
reason why this is the only way this information can be exposed and who
is going to use it in order to be able to have this accepted.
thanks,
greg k-h
On Wed, Sep 28, 2022 at 12:56:31PM -0700, Elliot Berman wrote:
> Gunyah resource manager defines a simple API for virtual machine log
> sharing with the console service. A VM's own log can be opened by using
> GH_VMID_SELF. Another VM's log can be accessed via its VMID. Once
> opened, characters can be written to the log with a write command.
> Characters are received with resource manager notifications (using ID
> GH_RM_NOTIF_VM_CONSOLE_CHARS).
>
> These high level rpc calls are kept in
> drivers/virt/gunyah/rsc_mgr_rpc.c. Future RPC calls, e.g. to launch a VM
> will also be maintained in this file.
>
> Signed-off-by: Elliot Berman <[email protected]>
> ---
> drivers/virt/gunyah/Makefile | 2 +-
> drivers/virt/gunyah/rsc_mgr.h | 22 +++++
> drivers/virt/gunyah/rsc_mgr_rpc.c | 151 ++++++++++++++++++++++++++++++
> include/linux/gunyah_rsc_mgr.h | 16 ++++
> 4 files changed, 190 insertions(+), 1 deletion(-)
> create mode 100644 drivers/virt/gunyah/rsc_mgr_rpc.c
>
> diff --git a/drivers/virt/gunyah/Makefile b/drivers/virt/gunyah/Makefile
> index 7c512490f921..73339ed445b3 100644
> --- a/drivers/virt/gunyah/Makefile
> +++ b/drivers/virt/gunyah/Makefile
> @@ -1,5 +1,5 @@
> gunyah-y += sysfs.o
> obj-$(CONFIG_GUNYAH) += gunyah.o
>
> -gunyah_rsc_mgr-y += rsc_mgr.o
> +gunyah_rsc_mgr-y += rsc_mgr.o rsc_mgr_rpc.o
> obj-$(CONFIG_GUNYAH_RESORUCE_MANAGER) += gunyah_rsc_mgr.o
> diff --git a/drivers/virt/gunyah/rsc_mgr.h b/drivers/virt/gunyah/rsc_mgr.h
> index e4f2499267bf..deb884979209 100644
> --- a/drivers/virt/gunyah/rsc_mgr.h
> +++ b/drivers/virt/gunyah/rsc_mgr.h
> @@ -28,6 +28,28 @@
> #define GH_RM_ERROR_IRQ_INUSE 0x10
> #define GH_RM_ERROR_IRQ_RELEASED 0x11
>
> +/* Message IDs: VM Management */
> +#define GH_RM_RPC_VM_GET_VMID 0x56000024
> +
> +/* Message IDs: VM Services */
> +#define GH_RM_RPC_VM_CONSOLE_OPEN_ID 0x56000081
> +#define GH_RM_RPC_VM_CONSOLE_CLOSE_ID 0x56000082
> +#define GH_RM_RPC_VM_CONSOLE_WRITE_ID 0x56000083
> +#define GH_RM_RPC_VM_CONSOLE_FLUSH_ID 0x56000084
> +
> +/* Call: CONSOLE_OPEN, CONSOLE_CLOSE, CONSOLE_FLUSH */
> +struct gh_vm_console_common_req {
> + u16 vmid;
> + u16 reserved0;
> +} __packed;
> +
> +/* Call: CONSOLE_WRITE */
> +struct gh_vm_console_write_req {
> + u16 vmid;
> + u16 num_bytes;
> + u8 data[0];
> +} __packed;
> +
> int gh_rm_call(u32 message_id, void *req_buff, size_t req_buff_size,
> void **resp_buf, size_t *resp_buff_size);
>
> diff --git a/drivers/virt/gunyah/rsc_mgr_rpc.c b/drivers/virt/gunyah/rsc_mgr_rpc.c
> new file mode 100644
> index 000000000000..8238c6ef301f
> --- /dev/null
> +++ b/drivers/virt/gunyah/rsc_mgr_rpc.c
> @@ -0,0 +1,151 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +
> +#define pr_fmt(fmt) "gh_rsc_mgr: " fmt
> +
> +#include <linux/slab.h>
> +#include <linux/types.h>
> +#include <linux/printk.h>
> +#include <linux/gunyah_rsc_mgr.h>
> +
> +#include "rsc_mgr.h"
> +
> +/**
> + * gh_rm_get_vmid() - Retrieve VMID of this virtual machine
> + * @vmid: Filled with the VMID of this VM
> + */
> +int gh_rm_get_vmid(u16 *vmid)
> +{
> + void *resp;
> + size_t resp_size;
> + int ret;
> + int payload = 0;
> +
> + ret = gh_rm_call(GH_RM_RPC_VM_GET_VMID, &payload, sizeof(payload), &resp, &resp_size);
> + if (ret)
> + return ret;
> +
> + if (resp_size != sizeof(*vmid))
> + return -EIO;
> + *vmid = *(u16 *)resp;
> + kfree(resp);
> +
> + return ret;
> +}
> +
> +/**
> + * gh_rm_console_open() - Open a console with a VM
> + * @vmid: VMID of the other VM whose console to open. If VMID is GH_VMID_SELF, the
> + * console associated with this VM is opened.
> + */
> +int gh_rm_console_open(u16 vmid)
> +{
> + void *resp;
> + struct gh_vm_console_common_req req_payload = {0};
> + size_t resp_size;
> + int ret;
> +
> + req_payload.vmid = vmid;
> +
> + ret = gh_rm_call(GH_RM_RPC_VM_CONSOLE_OPEN_ID,
> + &req_payload, sizeof(req_payload),
> + &resp, &resp_size);
> + kfree(resp);
> +
> + if (!ret && resp_size)
> + pr_warn("Received unexpected payload for CONSOLE_OPEN: %lu\n", resp_size);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(gh_rm_console_open);
> +
> +/**
> + * gh_rm_console_close() - Close a console with a VM
> + * @vmid: The vmid of the vm whose console to close.
> + */
> +int gh_rm_console_close(u16 vmid)
> +{
> + void *resp;
> + struct gh_vm_console_common_req req_payload = {0};
> + size_t resp_size;
> + int ret;
> +
> + req_payload.vmid = vmid;
> +
> + ret = gh_rm_call(GH_RM_RPC_VM_CONSOLE_CLOSE_ID,
> + &req_payload, sizeof(req_payload),
> + &resp, &resp_size);
> + kfree(resp);
> +
> + if (!ret && resp_size)
> + pr_warn("Received unexpected payload for CONSOLE_CLOSE: %lu\n", resp_size);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(gh_rm_console_close);
> +
> +/**
> + * gh_rm_console_write() - Write to a VM's console
> + * @vmid: The vmid of the vm whose console to write to.
> + * @buf: Buffer to write to the VM's console
> + * @size: Size of the buffer
> + */
> +int gh_rm_console_write(u16 vmid, const char *buf, size_t size)
> +{
> + void *resp;
> + struct gh_vm_console_write_req *req_payload;
> + size_t resp_size;
> + int ret = 0;
> + size_t req_payload_size = sizeof(*req_payload) + size;
> +
> + if (size < 1 || size > (U32_MAX - sizeof(*req_payload)))
> + return -EINVAL;
> +
> + req_payload = kzalloc(req_payload_size, GFP_KERNEL);
> +
> + if (!req_payload)
> + return -ENOMEM;
> +
> + req_payload->vmid = vmid;
> + req_payload->num_bytes = size;
> + memcpy(req_payload->data, buf, size);
> +
> + ret = gh_rm_call(GH_RM_RPC_VM_CONSOLE_WRITE_ID,
> + req_payload, req_payload_size,
> + &resp, &resp_size);
> + kfree(req_payload);
> + kfree(resp);
> +
> + if (!ret && resp_size)
> + pr_warn("Received unexpected payload for CONSOLE_WRITE: %lu\n", resp_size);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(gh_rm_console_write);
> +
> +/**
> + * gh_rm_console_flush() - Flush a console with a VM
> + * @vmid: The vmid of the vm whose console to flush
> + */
> +int gh_rm_console_flush(u16 vmid)
> +{
> + void *resp;
> + struct gh_vm_console_common_req req_payload = {0};
> + size_t resp_size;
> + int ret;
> +
> + req_payload.vmid = vmid;
> +
> + ret = gh_rm_call(GH_RM_RPC_VM_CONSOLE_FLUSH_ID,
> + &req_payload, sizeof(req_payload),
> + &resp, &resp_size);
> + kfree(resp);
> +
> + if (!ret && resp_size)
> + pr_warn("Received unexpected payload for CONSOLE_FLUSH: %lu\n", resp_size);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(gh_rm_console_flush);
> diff --git a/include/linux/gunyah_rsc_mgr.h b/include/linux/gunyah_rsc_mgr.h
> index b3b37225b7fb..f831ca921c26 100644
> --- a/include/linux/gunyah_rsc_mgr.h
> +++ b/include/linux/gunyah_rsc_mgr.h
> @@ -23,4 +23,20 @@ struct gh_rm_notification {
> int gh_rm_register_notifier(struct notifier_block *nb);
> int gh_rm_unregister_notifier(struct notifier_block *nb);
>
> +/* Notification type Message IDs */
> +#define GH_RM_NOTIF_VM_CONSOLE_CHARS 0x56100080
> +
> +struct gh_rm_notif_vm_console_chars {
> + u16 vmid;
> + u16 num_bytes;
> + u8 bytes[0];
Please do not use [0] for new structures, otherwise we will just have to
fix them up again as we are trying to get rid of all of these from the
kernel. Just use "bytes[];" instead.
thanks,
greg k-h
On Wed, Sep 28, 2022 at 12:56:33PM -0700, Elliot Berman wrote:
> Gunyah provides a console for each VM using the VM console resource
> manager APIs. This driver allows console data from other
> VMs to be accessed via a TTY device and exports a console device to dump
> Linux's own logs to our console.
>
> Signed-off-by: Elliot Berman <[email protected]>
> ---
> MAINTAINERS | 1 +
> drivers/tty/Kconfig | 8 +
> drivers/tty/Makefile | 1 +
> drivers/tty/gunyah_tty.c | 409 +++++++++++++++++++++++++++++++++++++++
> 4 files changed, 419 insertions(+)
> create mode 100644 drivers/tty/gunyah_tty.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a0cba618e5f6..e8d4a6d9491a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8890,6 +8890,7 @@ F: Documentation/devicetree/bindings/firmware/gunyah-hypervisor.yaml
> F: Documentation/virt/gunyah/
> F: arch/arm64/gunyah/
> F: drivers/mailbox/gunyah-msgq.c
> +F: drivers/tty/gunyah_tty.c
> F: drivers/virt/gunyah/
> F: include/asm-generic/gunyah.h
> F: include/linux/gunyah*.h
> diff --git a/drivers/tty/Kconfig b/drivers/tty/Kconfig
> index cc30ff93e2e4..ff86e977f9ac 100644
> --- a/drivers/tty/Kconfig
> +++ b/drivers/tty/Kconfig
> @@ -380,6 +380,14 @@ config RPMSG_TTY
> To compile this driver as a module, choose M here: the module will be
> called rpmsg_tty.
>
> +config GUNYAH_CONSOLE
> + tristate "Gunyah Consoles"
> + depends on GUNYAH
> + help
> + This enables support for console output using Gunyah's Resource Manager RPC.
> + This is normally used when a secondary VM which does not have exclusive access
> + to a real or virtualized serial device and virtio-console is unavailable.
module name?
> +
> endif # TTY
>
> source "drivers/tty/serdev/Kconfig"
> diff --git a/drivers/tty/Makefile b/drivers/tty/Makefile
> index 07aca5184a55..d183fbfd835b 100644
> --- a/drivers/tty/Makefile
> +++ b/drivers/tty/Makefile
> @@ -27,5 +27,6 @@ obj-$(CONFIG_GOLDFISH_TTY) += goldfish.o
> obj-$(CONFIG_MIPS_EJTAG_FDC_TTY) += mips_ejtag_fdc.o
> obj-$(CONFIG_VCC) += vcc.o
> obj-$(CONFIG_RPMSG_TTY) += rpmsg_tty.o
> +obj-$(CONFIG_GUNYAH_CONSOLE) += gunyah_tty.o
>
> obj-y += ipwireless/
> diff --git a/drivers/tty/gunyah_tty.c b/drivers/tty/gunyah_tty.c
> new file mode 100644
> index 000000000000..80a20da11ad0
> --- /dev/null
> +++ b/drivers/tty/gunyah_tty.c
> @@ -0,0 +1,409 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +
> +#define pr_fmt(fmt) "gh_rsc_mgr_console: " fmt
You are a driver, use dev_printk() functions, no need for pr_fmt() at
all, right?
> +
> +#include <linux/gunyah_rsc_mgr.h>
> +#include <linux/auxiliary_bus.h>
> +#include <linux/workqueue.h>
> +#include <linux/spinlock.h>
> +#include <linux/tty_flip.h>
> +#include <linux/console.h>
> +#include <linux/module.h>
> +#include <linux/kfifo.h>
> +#include <linux/kref.h>
> +#include <linux/slab.h>
> +#include <linux/tty.h>
> +
> +/*
> + * The Linux TTY code does not support dynamic addition of tty derived devices so we need to know
> + * how many tty devices we might need when space is allocated for the tty device. Since VMs might be
> + * added/removed dynamically, we need to make sure we have enough allocated.
Wrap comments at 80 columns please.
> + */
> +#define RSC_MGR_TTY_ADAPTERS 16
We can have dynamic tty devices, so I don't understand this comment.
What really is the problem here?
> +
> +/* # of payload bytes that can fit in a 1-fragment CONSOLE_WRITE message */
> +#define RM_CONS_WRITE_MSG_SIZE ((1 * (GH_MSGQ_MAX_MSG_SIZE - 8)) - 4)
> +
> +struct rm_cons_port {
> + struct tty_port port;
> + u16 vmid;
> + bool open;
Why do you care if it is open or not?
> + unsigned int index;
> +
> + DECLARE_KFIFO(put_fifo, char, 1024);
> + spinlock_t fifo_lock;
> + struct work_struct put_work;
> +
> + struct rm_cons_data *cons_data;
> +};
> +
> +struct rm_cons_data {
> + struct tty_driver *tty_driver;
> + struct device *dev;
> +
> + spinlock_t ports_lock;
> + struct rm_cons_port *ports[RSC_MGR_TTY_ADAPTERS];
> +
> + struct notifier_block rsc_mgr_notif;
> + struct console console;
> +};
> +
> +static void put_work_fn(struct work_struct *ws)
> +{
> + char buf[RM_CONS_WRITE_MSG_SIZE];
> + int count, ret;
> + struct rm_cons_port *port = container_of(ws, struct rm_cons_port, put_work);
> +
> + while (!kfifo_is_empty(&port->put_fifo)) {
> + count = kfifo_out_spinlocked(&port->put_fifo, buf, sizeof(buf), &port->fifo_lock);
> + if (count <= 0)
> + continue;
> +
> + ret = gh_rm_console_write(port->vmid, buf, count);
> + if (ret) {
> + pr_warn_once("failed to send characters: %d\n", ret);
What will this warning help with?
> + break;
If an error happens, shouldn't you keep trying to send the rest of the
data?
> + }
> + }
> +}
> +
> +static int rsc_mgr_console_notif(struct notifier_block *nb, unsigned long cmd, void *data)
> +{
> + int count, i;
> + struct rm_cons_port *rm_port = NULL;
> + struct tty_port *tty_port = NULL;
> + struct rm_cons_data *cons_data = container_of(nb, struct rm_cons_data, rsc_mgr_notif);
> + const struct gh_rm_notification *notif = data;
> + struct gh_rm_notif_vm_console_chars const * const msg = notif->buff;
> +
> + if (cmd != GH_RM_NOTIF_VM_CONSOLE_CHARS ||
> + notif->size < sizeof(*msg))
> + return NOTIFY_DONE;
> +
> + spin_lock(&cons_data->ports_lock);
> + for (i = 0; i < RSC_MGR_TTY_ADAPTERS; i++) {
> + if (!cons_data->ports[i])
> + continue;
> + if (cons_data->ports[i]->vmid == msg->vmid) {
> + rm_port = cons_data->ports[i];
> + break;
> + }
> + }
> + if (rm_port)
> + tty_port = tty_port_get(&rm_port->port);
> + spin_unlock(&cons_data->ports_lock);
> +
> + if (!rm_port)
> + pr_warn("Received unexpected console characters for VMID %u\n", msg->vmid);
> + if (!tty_port)
> + return NOTIFY_DONE;
> +
> + count = tty_buffer_request_room(tty_port, msg->num_bytes);
> + tty_insert_flip_string(tty_port, msg->bytes, count);
> + tty_flip_buffer_push(tty_port);
> +
> + tty_port_put(tty_port);
> + return NOTIFY_OK;
> +}
> +
> +static ssize_t vmid_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + struct rm_cons_port *rm_port = dev_get_drvdata(dev);
> +
> + if (rm_port->vmid == GH_VMID_SELF)
> + return sysfs_emit(buf, "self\n");
> +
> + return sysfs_emit(buf, "%u\n", rm_port->vmid);
You didn't document this sysfs file, why not?
And tty drivers should not have random sysfs files, please don't add
this.
> +}
> +
> +static DEVICE_ATTR_RO(vmid);
> +
> +static struct attribute *rsc_mgr_tty_dev_attrs[] = {
> + &dev_attr_vmid.attr,
> + NULL
> +};
> +
> +static const struct attribute_group rsc_mgr_tty_dev_attr_group = {
> + .attrs = rsc_mgr_tty_dev_attrs,
> +};
> +
> +static const struct attribute_group *rsc_mgr_tty_dev_attr_groups[] = {
> + &rsc_mgr_tty_dev_attr_group,
> + NULL
> +};
> +
> +static int rsc_mgr_tty_open(struct tty_struct *tty, struct file *filp)
> +{
> + int ret;
> + struct rm_cons_port *rm_port = dev_get_drvdata(tty->dev);
> +
> + if (!rm_port->open) {
Why are you caring if the port is open already or not?
> + ret = gh_rm_console_open(rm_port->vmid);
Can't this just be called for every open()?
And what happens if this changes right after it is checked?
> + if (ret) {
> + pr_err("Failed to open RM console for vmid %x: %d\n", rm_port->vmid, ret);
dev_err()
> + return ret;
> + }
> + rm_port->open = true;
> + }
> +
> + return tty_port_open(&rm_port->port, tty, filp);
> +}
> +
> +static void rsc_mgr_tty_close(struct tty_struct *tty, struct file *filp)
> +{
> + int ret;
> + struct rm_cons_port *rm_port = dev_get_drvdata(tty->dev);
> +
> + if (rm_port->open) {
> + if (rm_port->vmid != GH_VMID_SELF) {
> + ret = gh_rm_console_close(rm_port->vmid);
> + if (ret)
> + pr_warn("Failed to close RM console for vmid %d: %d\n",
> + rm_port->vmid, ret);
> + }
> + rm_port->open = false;
So if you had multiple open/close this would close the console the first
close call, but not the second?
Are you sure you tested this out properly?
> +
> + tty_port_close(&rm_port->port, tty, filp);
> + }
> +
> +}
> +
> +static int rsc_mgr_tty_write(struct tty_struct *tty, const unsigned char *buf, int count)
> +{
> + struct rm_cons_port *rm_port = dev_get_drvdata(tty->dev);
> + int ret;
> +
> + ret = kfifo_in_spinlocked(&rm_port->put_fifo, buf, count, &rm_port->fifo_lock);
> + if (ret > 0)
> + schedule_work(&rm_port->put_work);
Why not just do the write here? Why is a work queue needed?
> +
> + return ret;
> +}
> +
> +static unsigned int rsc_mgr_mgr_tty_write_room(struct tty_struct *tty)
> +{
> + struct rm_cons_port *rm_port = dev_get_drvdata(tty->dev);
> +
> + return kfifo_avail(&rm_port->put_fifo);
> +}
> +
> +static void rsc_mgr_console_write(struct console *co, const char *buf, unsigned count)
> +{
> + struct rm_cons_port *rm_port = co->data;
> + int ret;
> +
> + ret = kfifo_in_spinlocked(&rm_port->put_fifo, buf, count, &rm_port->fifo_lock);
> + if (ret > 0)
> + schedule_work(&rm_port->put_work);
Same here, why not just send the data now?
> +}
> +
> +static struct tty_driver *rsc_mgr_console_device(struct console *co, int *index)
> +{
> + struct rm_cons_port *rm_port = co->data;
> +
> + *index = rm_port->index;
> + return rm_port->port.tty->driver;
Love the locking :(
> +}
> +
> +static int rsc_mgr_console_setup(struct console *co, char *unused)
> +{
> + int ret;
> + struct rm_cons_port *rm_port = co->data;
> +
> + if (!rm_port->open) {
> + ret = gh_rm_console_open(rm_port->vmid);
> + if (ret) {
> + pr_err("Failed to open RM console for vmid %x: %d\n", rm_port->vmid, ret);
> + return ret;
> + }
> + rm_port->open = true;
Again, don't mess with open/close.
> + }
> +
> + return 0;
> +}
> +
> +static int rsc_mgr_console_exit(struct console *co)
> +{
> + int ret;
> + struct rm_cons_port *rm_port = co->data;
> +
> + if (rm_port->open) {
> + ret = gh_rm_console_close(rm_port->vmid);
> + if (ret) {
> + pr_err("Failed to close RM console for vmid %x: %d\n", rm_port->vmid, ret);
> + return ret;
> + }
> + rm_port->open = false;
> + }
> +
> + return 0;
> +}
> +
> +static const struct tty_operations rsc_mgr_tty_ops = {
> + .open = rsc_mgr_tty_open,
> + .close = rsc_mgr_tty_close,
> + .write = rsc_mgr_tty_write,
> + .write_room = rsc_mgr_mgr_tty_write_room,
> +};
> +
> +static void rsc_mgr_port_destruct(struct tty_port *port)
> +{
> + struct rm_cons_port *rm_port = container_of(port, struct rm_cons_port, port);
> + struct rm_cons_data *cons_data = rm_port->cons_data;
> +
> + spin_lock(&cons_data->ports_lock);
> + WARN_ON(cons_data->ports[rm_port->index] != rm_port);
Does this mean you just crashed the system if something went wrong?
How can this ever happen?
> + cons_data->ports[rm_port->index] = NULL;
> + spin_unlock(&cons_data->ports_lock);
> + kfree(rm_port);
> +}
> +
> +static const struct tty_port_operations rsc_mgr_port_ops = {
> + .destruct = rsc_mgr_port_destruct,
> +};
> +
> +static struct rm_cons_port *rsc_mgr_port_create(struct rm_cons_data *cons_data, u16 vmid)
> +{
> + struct rm_cons_port *rm_port;
> + struct device *ttydev;
> + unsigned int index;
> + int ret;
> +
> + rm_port = kzalloc(sizeof(*rm_port), GFP_KERNEL);
> + rm_port->vmid = vmid;
> + INIT_KFIFO(rm_port->put_fifo);
> + spin_lock_init(&rm_port->fifo_lock);
> + INIT_WORK(&rm_port->put_work, put_work_fn);
> + tty_port_init(&rm_port->port);
> + rm_port->port.ops = &rsc_mgr_port_ops;
> +
> + spin_lock(&cons_data->ports_lock);
> + for (index = 0; index < RSC_MGR_TTY_ADAPTERS; index++) {
> + if (!cons_data->ports[index]) {
> + cons_data->ports[index] = rm_port;
> + rm_port->index = index;
> + break;
> + }
> + }
> + spin_unlock(&cons_data->ports_lock);
> + if (index >= RSC_MGR_TTY_ADAPTERS) {
> + ret = -ENOSPC;
> + goto err_put_port;
> + }
> +
> + ttydev = tty_port_register_device_attr(&rm_port->port, cons_data->tty_driver, index,
> + cons_data->dev, rm_port, rsc_mgr_tty_dev_attr_groups);
> + if (IS_ERR(ttydev)) {
> + ret = PTR_ERR(ttydev);
> + goto err_put_port;
> + }
> +
> + return rm_port;
> +err_put_port:
> + tty_port_put(&rm_port->port);
> + return ERR_PTR(ret);
> +}
> +
> +static int rsc_mgr_console_probe(struct auxiliary_device *auxdev,
> + const struct auxiliary_device_id *aux_dev_id)
> +{
> + struct rm_cons_data *cons_data;
> + struct rm_cons_port *rm_port;
> + int ret;
> + u16 vmid;
> +
> + cons_data = devm_kzalloc(&auxdev->dev, sizeof(*cons_data), GFP_KERNEL);
> + if (!cons_data)
> + return -ENOMEM;
> + dev_set_drvdata(&auxdev->dev, cons_data);
> + cons_data->dev = &auxdev->dev;
> +
> + cons_data->tty_driver = tty_alloc_driver(RSC_MGR_TTY_ADAPTERS,
> + TTY_DRIVER_REAL_RAW | TTY_DRIVER_DYNAMIC_DEV);
> + if (IS_ERR(cons_data->tty_driver))
> + return PTR_ERR(cons_data->tty_driver);
> +
> + cons_data->tty_driver->driver_name = "gh";
> + cons_data->tty_driver->name = "ttyGH";
Where did you pick this name from?
Where is it documented?
> + cons_data->tty_driver->type = TTY_DRIVER_TYPE_SYSTEM;
> + cons_data->tty_driver->init_termios = tty_std_termios;
> + tty_set_operations(cons_data->tty_driver, &rsc_mgr_tty_ops);
> +
> + ret = tty_register_driver(cons_data->tty_driver);
> + if (ret) {
> + dev_err(&auxdev->dev, "Could not register tty driver: %d\n", ret);
> + goto err_put_tty;
> + }
> +
> + spin_lock_init(&cons_data->ports_lock);
> +
> + cons_data->rsc_mgr_notif.notifier_call = rsc_mgr_console_notif;
> + ret = gh_rm_register_notifier(&cons_data->rsc_mgr_notif);
> + if (ret) {
> + dev_err(&auxdev->dev, "Could not register for resource manager notifications: %d\n",
> + ret);
> + goto err_put_tty;
> + }
> +
> + rm_port = rsc_mgr_port_create(cons_data, GH_VMID_SELF);
> + if (IS_ERR(rm_port)) {
> + ret = PTR_ERR(rm_port);
> + dev_err(&auxdev->dev, "Could not create own console: %d\n", ret);
> + goto err_unreg_notif;
> + }
> +
> + strncpy(cons_data->console.name, "ttyGH", sizeof(cons_data->console.name));
Again, where did this name come from?
thanks,
greg k-h
On Wed, Sep 28, 2022 at 12:56:32PM -0700, Elliot Berman wrote:
> Gunyah resource manager exposes a concrete functionalities which
> complicate a single resource manager driver.
I am sorry, but I do not understand this sentance. What is so
complicated about individual devices being created? Where are they
created? What bus?
Use auxiliary bus
> to help split high level functions for the resource manager and keep the
> primary resource manager driver focused on the RPC with RM itself.
> Delegate Resource Manager's console functionality to the auxiliary bus.
>
> Signed-off-by: Elliot Berman <[email protected]>
> ---
> drivers/virt/gunyah/Kconfig | 1 +
> drivers/virt/gunyah/rsc_mgr.c | 21 +++++++++++++++++++++
> 2 files changed, 22 insertions(+)
>
> diff --git a/drivers/virt/gunyah/Kconfig b/drivers/virt/gunyah/Kconfig
> index 78deed3c4562..610c8586005b 100644
> --- a/drivers/virt/gunyah/Kconfig
> +++ b/drivers/virt/gunyah/Kconfig
> @@ -17,6 +17,7 @@ config GUNYAH_RESORUCE_MANAGER
> tristate "Gunyah Resource Manager"
> select MAILBOX
> select GUNYAH_MESSAGE_QUEUES
> + select AUXILIARY_BUS
> default y
> help
> The resource manager (RM) is a privileged application VM supporting
> diff --git a/drivers/virt/gunyah/rsc_mgr.c b/drivers/virt/gunyah/rsc_mgr.c
> index 7f7e89a6436b..435fe0149915 100644
> --- a/drivers/virt/gunyah/rsc_mgr.c
> +++ b/drivers/virt/gunyah/rsc_mgr.c
> @@ -16,6 +16,7 @@
> #include <linux/notifier.h>
> #include <linux/workqueue.h>
> #include <linux/completion.h>
> +#include <linux/auxiliary_bus.h>
> #include <linux/gunyah_rsc_mgr.h>
> #include <linux/platform_device.h>
>
> @@ -98,6 +99,8 @@ struct gh_rsc_mgr {
> struct mutex send_lock;
>
> struct work_struct recv_work;
> +
> + struct auxiliary_device console_adev;
> };
>
> static struct gh_rsc_mgr *__rsc_mgr;
> @@ -573,13 +576,31 @@ static int gh_rm_drv_probe(struct platform_device *pdev)
>
> __rsc_mgr = rsc_mgr;
>
> + rsc_mgr->console_adev.dev.parent = &pdev->dev;
> + rsc_mgr->console_adev.name = "console";
> + ret = auxiliary_device_init(&rsc_mgr->console_adev);
> + if (ret)
> + goto err_msgq;
> + ret = auxiliary_device_add(&rsc_mgr->console_adev);
> + if (ret)
> + goto err_console_adev_uninit;
> +
> return 0;
> +
> +err_console_adev_uninit:
> + auxiliary_device_uninit(&rsc_mgr->console_adev);
> +err_msgq:
> + gunyah_msgq_remove(&rsc_mgr->msgq);
> + return ret;
> }
Why can't you just have individual platform devices for the individual
devices your hypervisor exposes?
You control the platform devices, why force them to be shared like this?
thanks,
greg k-h
On 9/29/2022 12:36 AM, Joe Perches wrote:
> On Wed, 2022-09-28 at 12:56 -0700, Elliot Berman wrote:
>> Add a sysfs node to list the features that the Gunyah hypervisor and
>> Linux supports. For now, Linux support cspace (capability IDs) and
>> message queues, so only report those..
> []
>> diff --git a/drivers/virt/gunyah/sysfs.c b/drivers/virt/gunyah/sysfs.c
> []
>> @@ -25,9 +25,24 @@ static ssize_t variant_show(struct kobject *kobj, struct kobj_attribute *attr, c
>> }
>> static struct kobj_attribute variant_attr = __ATTR_RO(variant);
>>
>> +static ssize_t features_show(struct kobject *kobj, struct kobj_attribute *attr, char *buffer)
>> +{
>> + int len = 0;
>> +
>> + if (GH_IDENTIFY_PARTITION_CSPACE(gunyah_api.flags))
>> + len += sysfs_emit_at(buffer, len, "cspace ");
>> + if (GH_IDENTIFY_MSGQUEUE(gunyah_api.flags))
>> + len += sysfs_emit_at(buffer, len, "message-queue ");
>> +
>> + len += sysfs_emit_at(buffer, len, "\n");
>> + return len;
>> +}
>
> It's generally nicer to avoid unnecessary output spaces.
>
> Perhaps:
>
> {
> int len = 0;
>
> if (GH_IDENTIFY_PARTITION_CSPACE(gunyah_api.flags))
> len += sysfs_emit_at(buffer, len, "cspace");
> if (GH_IDENTIFY_MSGQUEUE(gunyah_api.flags)) {
> if (len)
> len += sysfs_emit_at(buffer, len, " ");
> len += sysfs_emit_at(buffer, len, "message-queue");
> }
>
> len += sysfs_emit_at(buffer, len, "\n");
>
> return len;
> }
>
that approach seems ok for 2 features, but imo doesn't scale for more.
I like the original code with one exception:
if (GH_IDENTIFY_PARTITION_CSPACE(gunyah_api.flags))
len += sysfs_emit_at(buffer, len, "cspace ");
if (GH_IDENTIFY_MSGQUEUE(gunyah_api.flags))
len += sysfs_emit_at(buffer, len, "message-queue ");
/* overwrite last trailing space */
if (len)
len--;
len += sysfs_emit_at(buffer, len, "\n");
return len;
On Sun, 2022-10-02 at 16:30 -0700, Jeff Johnson wrote:
> On 9/29/2022 12:36 AM, Joe Perches wrote:
> > On Wed, 2022-09-28 at 12:56 -0700, Elliot Berman wrote:
> > > Add a sysfs node to list the features that the Gunyah hypervisor and
> > > Linux supports. For now, Linux support cspace (capability IDs) and
> > > message queues, so only report those..
> > []
> > > diff --git a/drivers/virt/gunyah/sysfs.c b/drivers/virt/gunyah/sysfs.c
> > []
> > > @@ -25,9 +25,24 @@ static ssize_t variant_show(struct kobject *kobj, struct kobj_attribute *attr, c
> > > }
> > > static struct kobj_attribute variant_attr = __ATTR_RO(variant);
> > >
> > > +static ssize_t features_show(struct kobject *kobj, struct kobj_attribute *attr, char *buffer)
> > > +{
> > > + int len = 0;
> > > +
> > > + if (GH_IDENTIFY_PARTITION_CSPACE(gunyah_api.flags))
> > > + len += sysfs_emit_at(buffer, len, "cspace ");
> > > + if (GH_IDENTIFY_MSGQUEUE(gunyah_api.flags))
> > > + len += sysfs_emit_at(buffer, len, "message-queue ");
> > > +
> > > + len += sysfs_emit_at(buffer, len, "\n");
> > > + return len;
> > > +}
> >
> > It's generally nicer to avoid unnecessary output spaces.
> >
> > Perhaps:
> >
> > {
> > int len = 0;
> >
> > if (GH_IDENTIFY_PARTITION_CSPACE(gunyah_api.flags))
> > len += sysfs_emit_at(buffer, len, "cspace");
> > if (GH_IDENTIFY_MSGQUEUE(gunyah_api.flags)) {
> > if (len)
> > len += sysfs_emit_at(buffer, len, " ");
> > len += sysfs_emit_at(buffer, len, "message-queue");
> > }
> >
> > len += sysfs_emit_at(buffer, len, "\n");
> >
> > return len;
> > }
> >
>
> that approach seems ok for 2 features, but imo doesn't scale for more.
> I like the original code with one exception:
>
> if (GH_IDENTIFY_PARTITION_CSPACE(gunyah_api.flags))
> len += sysfs_emit_at(buffer, len, "cspace ");
> if (GH_IDENTIFY_MSGQUEUE(gunyah_api.flags))
> len += sysfs_emit_at(buffer, len, "message-queue ");
>
> /* overwrite last trailing space */
> if (len)
> len--;
>
> len += sysfs_emit_at(buffer, len, "\n");
> return len;
>
That's fine as long as every formatted output uses a trailing space.
A trivial negative would be that the linker would generally not be
able to deduplicate these output strings with trailing spaces across
the entire codebase.
On Fri, 2022-09-30 at 14:22 +0200, Greg Kroah-Hartman wrote:
> On Wed, Sep 28, 2022 at 12:56:31PM -0700, Elliot Berman wrote:
> > Gunyah resource manager defines a simple API for virtual machine log
> > sharing with the console service.
[]
> > diff --git a/include/linux/gunyah_rsc_mgr.h b/include/linux/gunyah_rsc_mgr.h
[]
> > +struct gh_rm_notif_vm_console_chars {
> > + u16 vmid;
> > + u16 num_bytes;
> > + u8 bytes[0];
>
> Please do not use [0] for new structures, otherwise we will just have to
> fix them up again as we are trying to get rid of all of these from the
> kernel. Just use "bytes[];" instead.
Maybe a checkpatch addition like:
---
scripts/checkpatch.pl | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 2737e4ced5745..187ed84c1f80a 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3948,6 +3948,17 @@ sub process {
}
}
+# check for zero length array declarations in likely structs
+ if ($line =~ /^\+\t($Declare\s*$Ident)\s*\[\s*0\s*\]\s*;\s*$/ &&
+ defined $lines[$linenr] &&
+ $lines[$linenr] =~ /^[\+ ]\}\s*(?:__\w+\s*(?:$balanced_parens)?)\s*;\s*$/) {
+ if (WARN("FLEXIBLE_ARRAY_ZERO",
+ "Prefer flexible length array declarations with [] over [0]\n" . $herecurr) &&
+ $fix) {
+ $fixed[$fixlinenr] =~ s/\[\s*0\s*\]/[]/;
+ }
+ }
+
# check for multiple consecutive blank lines
if ($prevline =~ /^[\+ ]\s*$/ &&
$line =~ /^\+\s*$/ &&
On Mon, 2022-10-03 at 07:29 +0200, Greg Kroah-Hartman wrote:
> On Sun, Oct 02, 2022 at 06:46:30PM -0700, Joe Perches wrote:
> > On Fri, 2022-09-30 at 14:22 +0200, Greg Kroah-Hartman wrote:
> > > On Wed, Sep 28, 2022 at 12:56:31PM -0700, Elliot Berman wrote:
> > > > Gunyah resource manager defines a simple API for virtual machine log
> > > > sharing with the console service.
> > []
> > > > diff --git a/include/linux/gunyah_rsc_mgr.h b/include/linux/gunyah_rsc_mgr.h
> > []
> > > > +struct gh_rm_notif_vm_console_chars {
> > > > + u16 vmid;
> > > > + u16 num_bytes;
> > > > + u8 bytes[0];
> > >
> > > Please do not use [0] for new structures, otherwise we will just have to
> > > fix them up again as we are trying to get rid of all of these from the
> > > kernel. Just use "bytes[];" instead.
> >
> > Maybe a checkpatch addition like:
> > ---
[]
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> > @@ -3948,6 +3948,17 @@ sub process {
> > }
> > }
> >
> > +# check for zero length array declarations in likely structs
> > + if ($line =~ /^\+\t($Declare\s*$Ident)\s*\[\s*0\s*\]\s*;\s*$/ &&
> > + defined $lines[$linenr] &&
> > + $lines[$linenr] =~ /^[\+ ]\}\s*(?:__\w+\s*(?:$balanced_parens)?)\s*;\s*$/) {
This should actually be:
$lines[$linenr] =~ /^[\+ ]\}(?:\s*__\w+\s*(?:$balanced_parens)?)*\s*;\s*$/) {
as it was missing a * for uses like
int foo[0];
} __packed __aligned(4);
and uses without any attribute at all
> > + if (WARN("FLEXIBLE_ARRAY_ZERO",
> > + "Prefer flexible length array declarations with [] over [0]\n" . $herecurr) &&
> > + $fix) {
> > + $fixed[$fixlinenr] =~ s/\[\s*0\s*\]/[]/;
> > + }
> > + }
> > +
> > # check for multiple consecutive blank lines
> > if ($prevline =~ /^[\+ ]\s*$/ &&
> > $line =~ /^\+\s*$/ &&
>
> This is a question for Gustavo, who did all the work here. Gustavo,
> does the above checkpatch change look good to you?
On Sun, Oct 02, 2022 at 06:46:30PM -0700, Joe Perches wrote:
> On Fri, 2022-09-30 at 14:22 +0200, Greg Kroah-Hartman wrote:
> > On Wed, Sep 28, 2022 at 12:56:31PM -0700, Elliot Berman wrote:
> > > Gunyah resource manager defines a simple API for virtual machine log
> > > sharing with the console service.
> []
> > > diff --git a/include/linux/gunyah_rsc_mgr.h b/include/linux/gunyah_rsc_mgr.h
> []
> > > +struct gh_rm_notif_vm_console_chars {
> > > + u16 vmid;
> > > + u16 num_bytes;
> > > + u8 bytes[0];
> >
> > Please do not use [0] for new structures, otherwise we will just have to
> > fix them up again as we are trying to get rid of all of these from the
> > kernel. Just use "bytes[];" instead.
>
> Maybe a checkpatch addition like:
> ---
> scripts/checkpatch.pl | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 2737e4ced5745..187ed84c1f80a 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -3948,6 +3948,17 @@ sub process {
> }
> }
>
> +# check for zero length array declarations in likely structs
> + if ($line =~ /^\+\t($Declare\s*$Ident)\s*\[\s*0\s*\]\s*;\s*$/ &&
> + defined $lines[$linenr] &&
> + $lines[$linenr] =~ /^[\+ ]\}\s*(?:__\w+\s*(?:$balanced_parens)?)\s*;\s*$/) {
> + if (WARN("FLEXIBLE_ARRAY_ZERO",
> + "Prefer flexible length array declarations with [] over [0]\n" . $herecurr) &&
> + $fix) {
> + $fixed[$fixlinenr] =~ s/\[\s*0\s*\]/[]/;
> + }
> + }
> +
> # check for multiple consecutive blank lines
> if ($prevline =~ /^[\+ ]\s*$/ &&
> $line =~ /^\+\s*$/ &&
This is a question for Gustavo, who did all the work here. Gustavo,
does the above checkpatch change look good to you?
thanks,
greg k-h
On 28. 09. 22, 21:56, Elliot Berman wrote:
> --- /dev/null
> +++ b/drivers/tty/gunyah_tty.c
> @@ -0,0 +1,409 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +
> +#define pr_fmt(fmt) "gh_rsc_mgr_console: " fmt
> +
> +#include <linux/gunyah_rsc_mgr.h>
> +#include <linux/auxiliary_bus.h>
> +#include <linux/workqueue.h>
> +#include <linux/spinlock.h>
> +#include <linux/tty_flip.h>
> +#include <linux/console.h>
> +#include <linux/module.h>
> +#include <linux/kfifo.h>
> +#include <linux/kref.h>
> +#include <linux/slab.h>
> +#include <linux/tty.h>
Sort alphabetically, please. Not by inv. xmas tree.
> +/*
> + * The Linux TTY code does not support dynamic addition of tty derived devices so we need to know
> + * how many tty devices we might need when space is allocated for the tty device. Since VMs might be
> + * added/removed dynamically, we need to make sure we have enough allocated.
> + */
> +#define RSC_MGR_TTY_ADAPTERS 16
> +
> +/* # of payload bytes that can fit in a 1-fragment CONSOLE_WRITE message */
> +#define RM_CONS_WRITE_MSG_SIZE ((1 * (GH_MSGQ_MAX_MSG_SIZE - 8)) - 4)
"1 *" is kind of superfluous.
"- 8 - 4" -- it's too many magic constants in here. Define macros for them.
> +struct rm_cons_port {
> + struct tty_port port;
> + u16 vmid;
> + bool open;
> + unsigned int index;
> +
> + DECLARE_KFIFO(put_fifo, char, 1024);
Why is tty_port::xmit_fifo not enough?
> + spinlock_t fifo_lock;
> + struct work_struct put_work;
> +
> + struct rm_cons_data *cons_data;
> +};
> +
> +struct rm_cons_data {
> + struct tty_driver *tty_driver;
It looks weird to have a driver per console/device.
> + struct device *dev;
> +
> + spinlock_t ports_lock;
> + struct rm_cons_port *ports[RSC_MGR_TTY_ADAPTERS];
> +
> + struct notifier_block rsc_mgr_notif;
> + struct console console;
> +};
> +
> +static void put_work_fn(struct work_struct *ws)
> +{
> + char buf[RM_CONS_WRITE_MSG_SIZE];
Ugh, is this 1024-12? Do not do this on stack!
> + int count, ret;
> + struct rm_cons_port *port = container_of(ws, struct rm_cons_port, put_work);
> +
> + while (!kfifo_is_empty(&port->put_fifo)) {
> + count = kfifo_out_spinlocked(&port->put_fifo, buf, sizeof(buf), &port->fifo_lock);
> + if (count <= 0)
> + continue;
This does not make much sense. 1) kfifo_is_empty() is not locked; 2)
it's overly complicated. It can be, IMO:
while (1) {
count = kfifo_out_spinlocked();
if (!count)
break;
> +static int rsc_mgr_console_notif(struct notifier_block *nb, unsigned long cmd, void *data)
> +{
> + int count, i;
Not unsigned?
> + struct rm_cons_port *rm_port = NULL;
> + struct tty_port *tty_port = NULL;
> + struct rm_cons_data *cons_data = container_of(nb, struct rm_cons_data, rsc_mgr_notif);
> + const struct gh_rm_notification *notif = data;
> + struct gh_rm_notif_vm_console_chars const * const msg = notif->buff;
Interesting mix of inconsistencies. Once you start with const, once you
place it after struct. Please make it consistent (start with const).
ANd here, you should apply inv. xmas tree sorting.
> +
> + if (cmd != GH_RM_NOTIF_VM_CONSOLE_CHARS ||
> + notif->size < sizeof(*msg))
> + return NOTIFY_DONE;
Weird indentation. notif->size should start with either 4 spaces less or
one more tab.
regards,
--
js
suse labs
On 9/30/2022 5:19 AM, Greg Kroah-Hartman wrote:
> On Wed, Sep 28, 2022 at 12:56:32PM -0700, Elliot Berman wrote:
>> Gunyah resource manager exposes a concrete functionalities which
>> complicate a single resource manager driver.
>
> I am sorry, but I do not understand this sentance. What is so
> complicated about individual devices being created? Where are they
> created? What bus?
There's no complexity here with using individual devices, that's why I
wanted to create secondary (auxiliary devices).
IOW -- "I have a platform device that does a lot of different things.
Split up the different functionalities of that device into sub devices
using the auxiliary bus."
>
> Use auxiliary bus
>> to help split high level functions for the resource manager and keep the
>> primary resource manager driver focused on the RPC with RM itself.
>> Delegate Resource Manager's console functionality to the auxiliary bus.
>>
>> Signed-off-by: Elliot Berman <[email protected]>
>> ---
>> drivers/virt/gunyah/Kconfig | 1 +
>> drivers/virt/gunyah/rsc_mgr.c | 21 +++++++++++++++++++++
>> 2 files changed, 22 insertions(+)
>>
>> diff --git a/drivers/virt/gunyah/Kconfig b/drivers/virt/gunyah/Kconfig
>> index 78deed3c4562..610c8586005b 100644
>> --- a/drivers/virt/gunyah/Kconfig
>> +++ b/drivers/virt/gunyah/Kconfig
>> @@ -17,6 +17,7 @@ config GUNYAH_RESORUCE_MANAGER
>> tristate "Gunyah Resource Manager"
>> select MAILBOX
>> select GUNYAH_MESSAGE_QUEUES
>> + select AUXILIARY_BUS
>> default y
>> help
>> The resource manager (RM) is a privileged application VM supporting
>> diff --git a/drivers/virt/gunyah/rsc_mgr.c b/drivers/virt/gunyah/rsc_mgr.c
>> index 7f7e89a6436b..435fe0149915 100644
>> --- a/drivers/virt/gunyah/rsc_mgr.c
>> +++ b/drivers/virt/gunyah/rsc_mgr.c
>> @@ -16,6 +16,7 @@
>> #include <linux/notifier.h>
>> #include <linux/workqueue.h>
>> #include <linux/completion.h>
>> +#include <linux/auxiliary_bus.h>
>> #include <linux/gunyah_rsc_mgr.h>
>> #include <linux/platform_device.h>
>>
>> @@ -98,6 +99,8 @@ struct gh_rsc_mgr {
>> struct mutex send_lock;
>>
>> struct work_struct recv_work;
>> +
>> + struct auxiliary_device console_adev;
>> };
>>
>> static struct gh_rsc_mgr *__rsc_mgr;
>> @@ -573,13 +576,31 @@ static int gh_rm_drv_probe(struct platform_device *pdev)
>>
>> __rsc_mgr = rsc_mgr;
>>
>> + rsc_mgr->console_adev.dev.parent = &pdev->dev;
>> + rsc_mgr->console_adev.name = "console";
>> + ret = auxiliary_device_init(&rsc_mgr->console_adev);
>> + if (ret)
>> + goto err_msgq;
>> + ret = auxiliary_device_add(&rsc_mgr->console_adev);
>> + if (ret)
>> + goto err_console_adev_uninit;
>> +
>> return 0;
>> +
>> +err_console_adev_uninit:
>> + auxiliary_device_uninit(&rsc_mgr->console_adev);
>> +err_msgq:
>> + gunyah_msgq_remove(&rsc_mgr->msgq);
>> + return ret;
>> }
>
> Why can't you just have individual platform devices for the individual
> devices your hypervisor exposes?
>
> You control the platform devices, why force them to be shared like this?
>
The resource manager exposes quite a bit of functionality, and I think
it makes sense to expose them as auxiliary devices. From
Documentation/driver-api/auxiliary_bus.rst:
A key requirement for utilizing the auxiliary bus is that there is no
dependency on a physical bus, device, register accesses or regmap support.
These individual devices split from the core cannot live on the platform
bus as
they are not physical devices that are controlled by DT/ACPI.
> thanks,
>
> greg k-h
On 9/30/2022 5:09 AM, Greg Kroah-Hartman wrote:
> On Wed, Sep 28, 2022 at 12:56:25PM -0700, Elliot Berman wrote:
>> Add /sys/hypervisor support when detecting that Linux is running in a
>> Gunyah environment. Export the version of Gunyah which is reported via
>> the hyp_identify hypercall.
>>
>> Signed-off-by: Elliot Berman <[email protected]>
>> ---
>> .../ABI/testing/sysfs-hypervisor-gunyah | 15 ++++
>> MAINTAINERS | 1 +
>> drivers/virt/Makefile | 1 +
>> drivers/virt/gunyah/Makefile | 2 +
>> drivers/virt/gunyah/sysfs.c | 71 +++++++++++++++++++
>> 5 files changed, 90 insertions(+)
>> create mode 100644 Documentation/ABI/testing/sysfs-hypervisor-gunyah
>> create mode 100644 drivers/virt/gunyah/Makefile
>> create mode 100644 drivers/virt/gunyah/sysfs.c
>>
>> diff --git a/Documentation/ABI/testing/sysfs-hypervisor-gunyah b/Documentation/ABI/testing/sysfs-hypervisor-gunyah
>> new file mode 100644
>> index 000000000000..7d74e74e9edd
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-hypervisor-gunyah
>> @@ -0,0 +1,15 @@
>> +What: /sys/hypervisor/gunyah/api
>> +Date: October 2022
>> +KernelVersion: 6.1
>> +Contact: [email protected]
>> +Description: If running under Gunyah:
>> + The Gunyah API version.
>
> What does this version mean? What format is it in?
>
The version is incremented on backwards-incompatible API changes. It's
an integer: I've updated the description to mention it's an integer. FYI
-- we are still currently at "1" and not aiming to increment this
number. I'd like to get it reported in sysfs in case the version is
incremented later.
>> +
>> +What: /sys/hypervisor/gunyah/variant
>> +Date: October 2022
>> +KernelVersion: 6.1
>> +Contact: [email protected]
>> +Description: If running under Gunyah:
>> + Reports the build variant of Gunyah:
>> + The open source build of Gunyah will report "81".
>> + The Qualcomm build of Gunyah will report "72".
>
> So there are only 2 versions variants? What happens when you get a
> third? And why the odd numbers?
>
The kernel isn't parsing the reported build variant and is passing the
reported value up to the sysfs node. If a new third variant comes along,
its build variant number would be reported. Would it be preferred to
instead link to Gunyah's definitions for the build variant?
> What will userspace do with this information and what tool will parse
> it? >
The usecase I'm envisioning is to help user check what build of Gunyah
is present on the host. We don't have any intention to require userspace
(or kernel) to behave differently whether they are on Qualcomm-built
Gunyah or the open-source Gunyah.
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index feafac12db35..a26e67ef36b4 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -8885,6 +8885,7 @@ M: Elliot Berman <[email protected]>
>> M: Murali Nalajala <[email protected]>
>> L: [email protected]
>> S: Supported
>> +F: Documentation/ABI/testing/sysfs-hypervisor-gunyah
>> F: Documentation/devicetree/bindings/firmware/gunyah-hypervisor.yaml
>> F: Documentation/virt/gunyah/
>> F: arch/arm64/gunyah/
>> diff --git a/drivers/virt/Makefile b/drivers/virt/Makefile
>> index 093674e05c40..10b87f934730 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-$(CONFIG_GUNYAH) += gunyah/
>> diff --git a/drivers/virt/gunyah/Makefile b/drivers/virt/gunyah/Makefile
>> new file mode 100644
>> index 000000000000..e15f16c17142
>> --- /dev/null
>> +++ b/drivers/virt/gunyah/Makefile
>> @@ -0,0 +1,2 @@
>> +gunyah-y += sysfs.o
>> +obj-$(CONFIG_GUNYAH) += gunyah.o
>> diff --git a/drivers/virt/gunyah/sysfs.c b/drivers/virt/gunyah/sysfs.c
>> new file mode 100644
>> index 000000000000..ec11510cbece
>> --- /dev/null
>> +++ b/drivers/virt/gunyah/sysfs.c
>> @@ -0,0 +1,71 @@
>> +// 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/kobject.h>
>> +#include <linux/module.h>
>> +#include <linux/printk.h>
>> +#include <linux/init.h>
>> +#include <asm-generic/gunyah.h>
>> +
>> +static struct gh_hypercall_hyp_identify_resp gunyah_api;
>> +
>> +static ssize_t api_show(struct kobject *kobj, struct kobj_attribute *attr, char *buffer)
>> +{
>> + return sysfs_emit(buffer, "%d\n", (int)GH_API_INFO_API_VERSION(gunyah_api.api_info));
>> +}
>> +static struct kobj_attribute api_attr = __ATTR_RO(api);
>> +
>> +static ssize_t variant_show(struct kobject *kobj, struct kobj_attribute *attr, char *buffer)
>> +{
>> + return sysfs_emit(buffer, "%d\n", (int)GH_API_INFO_VARIANT(gunyah_api.api_info));
>> +}
>> +static struct kobj_attribute variant_attr = __ATTR_RO(variant);
>> +
>> +static struct attribute *gunyah_attrs[] = {
>> + &api_attr.attr,
>> + &variant_attr.attr,
>> + NULL
>> +};
>> +
>> +static const struct attribute_group gunyah_group = {
>> + .name = "gunyah",
>> + .attrs = gunyah_attrs,
>> +};
>> +
>> +static int __init gunyah_init(void)
>> +{
>> + u32 uid[4];
>> +
>> + gh_hypercall_get_uid(uid);
>
> No error checking?
>
The UID is filled by the first 4 return registers of the hypercall. If
running under some other hypervisor or no hypervisor is present, then
our expectation is that the relevant handler would fill some error value
that isn't the UUID.
KVM uses similar approach to allow guests to identify they are a KVM guest:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/firmware/smccc/kvm_guest.c?h=v6.0-rc7#n23
>> +
>> + if (!(gh_uid_matches(GUNYAH, uid) || gh_uid_matches(QC_HYP, uid)))
>> + return 0;
>> +
>> + gh_hypercall_hyp_identify(&gunyah_api);
>> +
>> + if (GH_API_INFO_API_VERSION(gunyah_api.api_info) != 1) {
>> + pr_warn("Unrecognized gunyah version: %llu. Currently supported: 1\n",
>> + GH_API_INFO_API_VERSION(gunyah_api.api_info));
>
> Shouldn't the "1" be defined somewhere?
>
>> + return 0;
>> + }
>> +
>> + pr_notice("Running under Gunyah hypervisor %llx/v%lld\n",
>> + GH_API_INFO_VARIANT(gunyah_api.api_info),
>> + GH_API_INFO_API_VERSION(gunyah_api.api_info));
>
> When kernel code is working properly, it is quiet. What is this going
> to be used for?
>
Xen and KVM guest drivers also report when they detect that they are
running under a hypervisor. I'll drop this down to pr_info to match.
> thanks,
>
> greg k-h
On 9/30/2022 5:06 AM, Greg Kroah-Hartman wrote:
> On Wed, Sep 28, 2022 at 12:56:29PM -0700, Elliot Berman wrote:
>> Add a sysfs node to list the features that the Gunyah hypervisor and
>> Linux supports. For now, Linux support cspace (capability IDs) and
>> message queues, so only report those..
>>
>> Signed-off-by: Elliot Berman <[email protected]>
>> ---
>> Documentation/ABI/testing/sysfs-hypervisor-gunyah | 15 +++++++++++++++
>> drivers/virt/gunyah/sysfs.c | 15 +++++++++++++++
>> 2 files changed, 30 insertions(+)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-hypervisor-gunyah b/Documentation/ABI/testing/sysfs-hypervisor-gunyah
>> index 7d74e74e9edd..6d0cde30355a 100644
>> --- a/Documentation/ABI/testing/sysfs-hypervisor-gunyah
>> +++ b/Documentation/ABI/testing/sysfs-hypervisor-gunyah
>> @@ -1,3 +1,18 @@
>> +What: /sys/hypervisor/gunyah/features
>> +Date: October 2022
>> +KernelVersion: 6.1
>> +Contact: [email protected]
>> +Description: If running under Gunyah:
>> + Space separated list of features supported by Linux and Gunyah:
>> + "cspace": Gunyah devices
>> + "doorbell": Sending/receiving virtual interrupts via Gunyah doorbells
>> + "message-queue": Sending/receiving messages via Gunyah message queues
>> + "vic": Interrupt lending
>> + "vpm": Virtual platform management
>> + "vcpu": Virtual CPU management
>> + "memextent": Memory lending/management
>> + "trace": Gunyah hypervisor tracing
>
> Please no. Why do you really need this type of list? What hypervisor
> will NOT have them all present already? Who will use this file and what
> will it be used for?
>
> sysfs files should just be 1 value and not need to be parsed. Yes, we
> have lists of features at times, but really, you need a very very good
> reason why this is the only way this information can be exposed and who
> is going to use it in order to be able to have this accepted.
>
We're currently at phase where all the features implemented so far as
considered part of the "base" featureset. We're thinking of future
features implemented in Gunyah: userspace might need to know that some
hypervisor feature is present and that it should make use of the feature
instead of some fallback behavior.
I can drop this and it should be OK IMO to introduce it later if needed.
The lack of the "gunyah/features" node would be sufficient for a
userspace program to know that some new feature isn't present.
> thanks,
>
> greg k-h
On Tue, Oct 04, 2022 at 04:49:27PM -0700, Elliot Berman wrote:
> On 9/30/2022 5:19 AM, Greg Kroah-Hartman wrote:
> > On Wed, Sep 28, 2022 at 12:56:32PM -0700, Elliot Berman wrote:
> > > Gunyah resource manager exposes a concrete functionalities which
> > > complicate a single resource manager driver.
> >
> > I am sorry, but I do not understand this sentance. What is so
> > complicated about individual devices being created? Where are they
> > created? What bus?
>
> There's no complexity here with using individual devices, that's why I
> wanted to create secondary (auxiliary devices).
>
> IOW -- "I have a platform device that does a lot of different things. Split
> up the different functionalities of that device into sub devices using the
> auxiliary bus."
Why not just have multiple platform devices? You control them, don't
make it more complex than it should be.
And why are these platform devices at all?
As you say:
> A key requirement for utilizing the auxiliary bus is that there is no
> dependency on a physical bus, device, register accesses or regmap support.
> These individual devices split from the core cannot live on the platform bus
> as they are not physical devices that are controlled by DT/ACPI.
These are not in the DT. So just make your own bus for them instead of
using a platform device. Don't abuse a platform device please.
thanks,
greg k-h
On Tue, Oct 04, 2022 at 04:50:51PM -0700, Elliot Berman wrote:
>
>
> On 9/30/2022 5:09 AM, Greg Kroah-Hartman wrote:
> > On Wed, Sep 28, 2022 at 12:56:25PM -0700, Elliot Berman wrote:
> > > Add /sys/hypervisor support when detecting that Linux is running in a
> > > Gunyah environment. Export the version of Gunyah which is reported via
> > > the hyp_identify hypercall.
> > >
> > > Signed-off-by: Elliot Berman <[email protected]>
> > > ---
> > > .../ABI/testing/sysfs-hypervisor-gunyah | 15 ++++
> > > MAINTAINERS | 1 +
> > > drivers/virt/Makefile | 1 +
> > > drivers/virt/gunyah/Makefile | 2 +
> > > drivers/virt/gunyah/sysfs.c | 71 +++++++++++++++++++
> > > 5 files changed, 90 insertions(+)
> > > create mode 100644 Documentation/ABI/testing/sysfs-hypervisor-gunyah
> > > create mode 100644 drivers/virt/gunyah/Makefile
> > > create mode 100644 drivers/virt/gunyah/sysfs.c
> > >
> > > diff --git a/Documentation/ABI/testing/sysfs-hypervisor-gunyah b/Documentation/ABI/testing/sysfs-hypervisor-gunyah
> > > new file mode 100644
> > > index 000000000000..7d74e74e9edd
> > > --- /dev/null
> > > +++ b/Documentation/ABI/testing/sysfs-hypervisor-gunyah
> > > @@ -0,0 +1,15 @@
> > > +What: /sys/hypervisor/gunyah/api
> > > +Date: October 2022
> > > +KernelVersion: 6.1
> > > +Contact: [email protected]
> > > +Description: If running under Gunyah:
> > > + The Gunyah API version.
> >
> > What does this version mean? What format is it in?
> >
>
> The version is incremented on backwards-incompatible API changes. It's an
> integer: I've updated the description to mention it's an integer. FYI -- we
> are still currently at "1" and not aiming to increment this number. I'd like
> to get it reported in sysfs in case the version is incremented later.
If you change it in the future to be backwards incompatible, then you
have to change your kernel code anyway and userspace still has to keep
working the same so it would use a totally different interface.
So why is this even needed at all?
> > > +
> > > +What: /sys/hypervisor/gunyah/variant
> > > +Date: October 2022
> > > +KernelVersion: 6.1
> > > +Contact: [email protected]
> > > +Description: If running under Gunyah:
> > > + Reports the build variant of Gunyah:
> > > + The open source build of Gunyah will report "81".
> > > + The Qualcomm build of Gunyah will report "72".
> >
> > So there are only 2 versions variants? What happens when you get a
> > third? And why the odd numbers?
> >
>
> The kernel isn't parsing the reported build variant and is passing the
> reported value up to the sysfs node. If a new third variant comes along, its
> build variant number would be reported. Would it be preferred to instead
> link to Gunyah's definitions for the build variant?
Just document what these random numbers mean please and what userspace
is to do with them. If nothing, and this is just information, who would
use that information? And if no one, why report it at all?
thanks,
greg k-h
On 10/4/2022 11:21 PM, Greg Kroah-Hartman wrote:
> On Tue, Oct 04, 2022 at 04:49:27PM -0700, Elliot Berman wrote:
>> On 9/30/2022 5:19 AM, Greg Kroah-Hartman wrote:
>>> On Wed, Sep 28, 2022 at 12:56:32PM -0700, Elliot Berman wrote:
>>>> Gunyah resource manager exposes a concrete functionalities which
>>>> complicate a single resource manager driver.
>>>
>>> I am sorry, but I do not understand this sentance. What is so
>>> complicated about individual devices being created? Where are they
>>> created? What bus?
>>
>> There's no complexity here with using individual devices, that's why I
>> wanted to create secondary (auxiliary devices).
>>
>> IOW -- "I have a platform device that does a lot of different things. Split
>> up the different functionalities of that device into sub devices using the
>> auxiliary bus."
>
> Why not just have multiple platform devices? You control them, don't
> make it more complex than it should be.
>
> And why are these platform devices at all?
>
> As you say:
>
>> A key requirement for utilizing the auxiliary bus is that there is no
>> dependency on a physical bus, device, register accesses or regmap support.
>> These individual devices split from the core cannot live on the platform bus
>> as they are not physical devices that are controlled by DT/ACPI.
>
> These are not in the DT. So just make your own bus for them instead of
> using a platform device. Don't abuse a platform device please.
>
I'll avoid creating platform devices. Are there any concerns with
creating auxiliary device under the platform device? There will only be
2 auxiliary devices under this Resource Manager device: one for console,
and one for a VM loader.
> thanks,
>
> greg k-h
On Wed, Oct 05, 2022 at 02:47:46PM -0700, Elliot Berman wrote:
>
>
> On 10/4/2022 11:21 PM, Greg Kroah-Hartman wrote:
> > On Tue, Oct 04, 2022 at 04:49:27PM -0700, Elliot Berman wrote:
> > > On 9/30/2022 5:19 AM, Greg Kroah-Hartman wrote:
> > > > On Wed, Sep 28, 2022 at 12:56:32PM -0700, Elliot Berman wrote:
> > > > > Gunyah resource manager exposes a concrete functionalities which
> > > > > complicate a single resource manager driver.
> > > >
> > > > I am sorry, but I do not understand this sentance. What is so
> > > > complicated about individual devices being created? Where are they
> > > > created? What bus?
> > >
> > > There's no complexity here with using individual devices, that's why I
> > > wanted to create secondary (auxiliary devices).
> > >
> > > IOW -- "I have a platform device that does a lot of different things. Split
> > > up the different functionalities of that device into sub devices using the
> > > auxiliary bus."
> >
> > Why not just have multiple platform devices? You control them, don't
> > make it more complex than it should be.
> >
> > And why are these platform devices at all?
> >
> > As you say:
> >
> > > A key requirement for utilizing the auxiliary bus is that there is no
> > > dependency on a physical bus, device, register accesses or regmap support.
> > > These individual devices split from the core cannot live on the platform bus
> > > as they are not physical devices that are controlled by DT/ACPI.
> >
> > These are not in the DT. So just make your own bus for them instead of
> > using a platform device. Don't abuse a platform device please.
> >
>
> I'll avoid creating platform devices. Are there any concerns with creating
> auxiliary device under the platform device?
Yes, don't do it if you do not have to, auxiliary devices are there only
if you have no other choice.
Just make 2 real devices on your own virtual bus please.
thanks,
greg k-h
On 9/30/2022 5:17 AM, Greg Kroah-Hartman wrote:
> On Wed, Sep 28, 2022 at 12:56:33PM -0700, Elliot Berman wrote:
>> Gunyah provides a console for each VM using the VM console resource
>> manager APIs. This driver allows console data from other
>> VMs to be accessed via a TTY device and exports a console device to dump
>> Linux's own logs to our console.
>>
>> Signed-off-by: Elliot Berman <[email protected]>
>> ---
>> MAINTAINERS | 1 +
>> drivers/tty/Kconfig | 8 +
>> drivers/tty/Makefile | 1 +
>> drivers/tty/gunyah_tty.c | 409 +++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 419 insertions(+)
>> create mode 100644 drivers/tty/gunyah_tty.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index a0cba618e5f6..e8d4a6d9491a 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -8890,6 +8890,7 @@ F: Documentation/devicetree/bindings/firmware/gunyah-hypervisor.yaml
>> F: Documentation/virt/gunyah/
>> F: arch/arm64/gunyah/
>> F: drivers/mailbox/gunyah-msgq.c
>> +F: drivers/tty/gunyah_tty.c
>> F: drivers/virt/gunyah/
>> F: include/asm-generic/gunyah.h
>> F: include/linux/gunyah*.h
>> diff --git a/drivers/tty/Kconfig b/drivers/tty/Kconfig
>> index cc30ff93e2e4..ff86e977f9ac 100644
>> --- a/drivers/tty/Kconfig
>> +++ b/drivers/tty/Kconfig
>> @@ -380,6 +380,14 @@ config RPMSG_TTY
>> To compile this driver as a module, choose M here: the module will be
>> called rpmsg_tty.
>>
>> +config GUNYAH_CONSOLE
>> + tristate "Gunyah Consoles"
>> + depends on GUNYAH
>> + help
>> + This enables support for console output using Gunyah's Resource Manager RPC.
>> + This is normally used when a secondary VM which does not have exclusive access
>> + to a real or virtualized serial device and virtio-console is unavailable.
>
> module name?
>
>> +
>> endif # TTY
>>
>> source "drivers/tty/serdev/Kconfig"
>> diff --git a/drivers/tty/Makefile b/drivers/tty/Makefile
>> index 07aca5184a55..d183fbfd835b 100644
>> --- a/drivers/tty/Makefile
>> +++ b/drivers/tty/Makefile
>> @@ -27,5 +27,6 @@ obj-$(CONFIG_GOLDFISH_TTY) += goldfish.o
>> obj-$(CONFIG_MIPS_EJTAG_FDC_TTY) += mips_ejtag_fdc.o
>> obj-$(CONFIG_VCC) += vcc.o
>> obj-$(CONFIG_RPMSG_TTY) += rpmsg_tty.o
>> +obj-$(CONFIG_GUNYAH_CONSOLE) += gunyah_tty.o
>>
>> obj-y += ipwireless/
>> diff --git a/drivers/tty/gunyah_tty.c b/drivers/tty/gunyah_tty.c
>> new file mode 100644
>> index 000000000000..80a20da11ad0
>> --- /dev/null
>> +++ b/drivers/tty/gunyah_tty.c
>> @@ -0,0 +1,409 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
>> + */
>> +
>> +#define pr_fmt(fmt) "gh_rsc_mgr_console: " fmt
>
> You are a driver, use dev_printk() functions, no need for pr_fmt() at
> all, right?
>
>> +
>> +#include <linux/gunyah_rsc_mgr.h>
>> +#include <linux/auxiliary_bus.h>
>> +#include <linux/workqueue.h>
>> +#include <linux/spinlock.h>
>> +#include <linux/tty_flip.h>
>> +#include <linux/console.h>
>> +#include <linux/module.h>
>> +#include <linux/kfifo.h>
>> +#include <linux/kref.h>
>> +#include <linux/slab.h>
>> +#include <linux/tty.h>
>> +
>> +/*
>> + * The Linux TTY code does not support dynamic addition of tty derived devices so we need to know
>> + * how many tty devices we might need when space is allocated for the tty device. Since VMs might be
>> + * added/removed dynamically, we need to make sure we have enough allocated.
>
> Wrap comments at 80 columns please.
>
>> + */
>> +#define RSC_MGR_TTY_ADAPTERS 16
>
> We can have dynamic tty devices, so I don't understand this comment.
> What really is the problem here?
>
Yes, I see the confusion. Dynamic device addition of tty devices is
supported. As I understand, you need to know the maximum number of lines
that could be added, and that is limitation I was referring to.
Is this comment better?
The Linux TTY code requires us to know ahead of time how many lines we
might need. Each line here corresponds to a VM. 16 seems like a
reasonable number of lines for systems that are running Gunyah and using
the provided console interface.
>> +
>> +/* # of payload bytes that can fit in a 1-fragment CONSOLE_WRITE message */
>> +#define RM_CONS_WRITE_MSG_SIZE ((1 * (GH_MSGQ_MAX_MSG_SIZE - 8)) - 4)
>> +
>> +struct rm_cons_port {
>> + struct tty_port port;
>> + u16 vmid;
>> + bool open;
>
> Why do you care if it is open or not?
>
I can clean it out.
>> + unsigned int index;
>> +
>> + DECLARE_KFIFO(put_fifo, char, 1024);
>> + spinlock_t fifo_lock;
>> + struct work_struct put_work;
>> +
>> + struct rm_cons_data *cons_data;
>> +};
>> +
>> +struct rm_cons_data {
>> + struct tty_driver *tty_driver;
>> + struct device *dev;
>> +
>> + spinlock_t ports_lock;
>> + struct rm_cons_port *ports[RSC_MGR_TTY_ADAPTERS];
>> +
>> + struct notifier_block rsc_mgr_notif;
>> + struct console console;
>> +};
>> +
>> +static void put_work_fn(struct work_struct *ws)
>> +{
>> + char buf[RM_CONS_WRITE_MSG_SIZE];
>> + int count, ret;
>> + struct rm_cons_port *port = container_of(ws, struct rm_cons_port, put_work);
>> +
>> + while (!kfifo_is_empty(&port->put_fifo)) {
>> + count = kfifo_out_spinlocked(&port->put_fifo, buf, sizeof(buf), &port->fifo_lock);
>> + if (count <= 0)
>> + continue;
>> +
>> + ret = gh_rm_console_write(port->vmid, buf, count);
>> + if (ret) {
>> + pr_warn_once("failed to send characters: %d\n", ret);
>
> What will this warning help with?
>
>> + break;
>
> If an error happens, shouldn't you keep trying to send the rest of the
> data?
>
I'll update to retry on anything but ENOMEM.
>> + }
>> + }
>> +}
>> +
>> +static int rsc_mgr_console_notif(struct notifier_block *nb, unsigned long cmd, void *data)
>> +{
>> + int count, i;
>> + struct rm_cons_port *rm_port = NULL;
>> + struct tty_port *tty_port = NULL;
>> + struct rm_cons_data *cons_data = container_of(nb, struct rm_cons_data, rsc_mgr_notif);
>> + const struct gh_rm_notification *notif = data;
>> + struct gh_rm_notif_vm_console_chars const * const msg = notif->buff;
>> +
>> + if (cmd != GH_RM_NOTIF_VM_CONSOLE_CHARS ||
>> + notif->size < sizeof(*msg))
>> + return NOTIFY_DONE;
>> +
>> + spin_lock(&cons_data->ports_lock);
>> + for (i = 0; i < RSC_MGR_TTY_ADAPTERS; i++) {
>> + if (!cons_data->ports[i])
>> + continue;
>> + if (cons_data->ports[i]->vmid == msg->vmid) {
>> + rm_port = cons_data->ports[i];
>> + break;
>> + }
>> + }
>> + if (rm_port)
>> + tty_port = tty_port_get(&rm_port->port);
>> + spin_unlock(&cons_data->ports_lock);
>> +
>> + if (!rm_port)
>> + pr_warn("Received unexpected console characters for VMID %u\n", msg->vmid);
>> + if (!tty_port)
>> + return NOTIFY_DONE;
>> +
>> + count = tty_buffer_request_room(tty_port, msg->num_bytes);
>> + tty_insert_flip_string(tty_port, msg->bytes, count);
>> + tty_flip_buffer_push(tty_port);
>> +
>> + tty_port_put(tty_port);
>> + return NOTIFY_OK;
>> +}
>> +
>> +static ssize_t vmid_show(struct device *dev, struct device_attribute *attr, char *buf)
>> +{
>> + struct rm_cons_port *rm_port = dev_get_drvdata(dev);
>> +
>> + if (rm_port->vmid == GH_VMID_SELF)
>> + return sysfs_emit(buf, "self\n");
>> +
>> + return sysfs_emit(buf, "%u\n", rm_port->vmid);
>
> You didn't document this sysfs file, why not?
>
> And tty drivers should not have random sysfs files, please don't add
> this.
>
Removed
>> +}
>> +
>> +static DEVICE_ATTR_RO(vmid);
>> +
>> +static struct attribute *rsc_mgr_tty_dev_attrs[] = {
>> + &dev_attr_vmid.attr,
>> + NULL
>> +};
>> +
>> +static const struct attribute_group rsc_mgr_tty_dev_attr_group = {
>> + .attrs = rsc_mgr_tty_dev_attrs,
>> +};
>> +
>> +static const struct attribute_group *rsc_mgr_tty_dev_attr_groups[] = {
>> + &rsc_mgr_tty_dev_attr_group,
>> + NULL
>> +};
>> +
>> +static int rsc_mgr_tty_open(struct tty_struct *tty, struct file *filp)
>> +{
>> + int ret;
>> + struct rm_cons_port *rm_port = dev_get_drvdata(tty->dev);
>> +
>> + if (!rm_port->open) {
>
> Why are you caring if the port is open already or not?
>
>> + ret = gh_rm_console_open(rm_port->vmid);
>
> Can't this just be called for every open()?
>
> And what happens if this changes right after it is checked?
>
I've moved the open/close callbacks to the activate/shutdown
tty_port_operations.
>> + if (ret) {
>> + pr_err("Failed to open RM console for vmid %x: %d\n", rm_port->vmid, ret);
>
> dev_err()
>
>> + return ret;
>> + }
>> + rm_port->open = true;
>> + }
>> +
>> + return tty_port_open(&rm_port->port, tty, filp);
>> +}
>> +
>> +static void rsc_mgr_tty_close(struct tty_struct *tty, struct file *filp)
>> +{
>> + int ret;
>> + struct rm_cons_port *rm_port = dev_get_drvdata(tty->dev);
>> +
>> + if (rm_port->open) {
>> + if (rm_port->vmid != GH_VMID_SELF) {
>> + ret = gh_rm_console_close(rm_port->vmid);
>> + if (ret)
>> + pr_warn("Failed to close RM console for vmid %d: %d\n",
>> + rm_port->vmid, ret);
>> + }
>> + rm_port->open = false;
>
> So if you had multiple open/close this would close the console the first
> close call, but not the second?
>
> Are you sure you tested this out properly?
>
>> +
>> + tty_port_close(&rm_port->port, tty, filp);
>> + }
>> +
>> +}
>> +
>> +static int rsc_mgr_tty_write(struct tty_struct *tty, const unsigned char *buf, int count)
>> +{
>> + struct rm_cons_port *rm_port = dev_get_drvdata(tty->dev);
>> + int ret;
>> +
>> + ret = kfifo_in_spinlocked(&rm_port->put_fifo, buf, count, &rm_port->fifo_lock);
>> + if (ret > 0)
>> + schedule_work(&rm_port->put_work);
>
> Why not just do the write here? Why is a work queue needed?
>
The gh_rm_console_* calls will sleep. console_write can be called in an
atomic context, so I put the characters on FIFO. I'll update so that
FIFO only used for console.
>> +
>> + return ret;
>> +}
>> +
>> +static unsigned int rsc_mgr_mgr_tty_write_room(struct tty_struct *tty)
>> +{
>> + struct rm_cons_port *rm_port = dev_get_drvdata(tty->dev);
>> +
>> + return kfifo_avail(&rm_port->put_fifo);
>> +}
>> +
>> +static void rsc_mgr_console_write(struct console *co, const char *buf, unsigned count)
>> +{
>> + struct rm_cons_port *rm_port = co->data;
>> + int ret;
>> +
>> + ret = kfifo_in_spinlocked(&rm_port->put_fifo, buf, count, &rm_port->fifo_lock);
>> + if (ret > 0)
>> + schedule_work(&rm_port->put_work);
>
> Same here, why not just send the data now?
>
>> +}
>> +
>> +static struct tty_driver *rsc_mgr_console_device(struct console *co, int *index)
>> +{
>> + struct rm_cons_port *rm_port = co->data;
>> +
>> + *index = rm_port->index;
>> + return rm_port->port.tty->driver;
>
> Love the locking :(
>
>> +}
>> +
>> +static int rsc_mgr_console_setup(struct console *co, char *unused)
>> +{
>> + int ret;
>> + struct rm_cons_port *rm_port = co->data;
>> +
>> + if (!rm_port->open) {
>> + ret = gh_rm_console_open(rm_port->vmid);
>> + if (ret) {
>> + pr_err("Failed to open RM console for vmid %x: %d\n", rm_port->vmid, ret);
>> + return ret;
>> + }
>> + rm_port->open = true;
>
> Again, don't mess with open/close.
>
In general, is it acceptable to use tty_port(_set)_initialized in the
console_setup/console_exit?
static int rsc_mgr_console_setup(struct console *co, char *unused)
{
struct rm_cons_port *rm_port = co->data;
int ret;
if (!tty_port_get(&rm_port->port))
return -ENODEV;
mutex_lock(&rm_port->port.mutex);
if (!tty_port_initialized(&rm_port->port)) {
ret = gh_rm_console_open(rm_port->vmid);
if (ret) {
dev_err(rm_port->port.tty->dev, "Failed to open %s%d: %d\n",
co->name, rm_port->index, ret);
goto err;
}
tty_port_set_initialized(&rm_port->port, true);
}
rm_port->port.console = true;
mutex_unlock(&rm_port->port.mutex);
return 0;
err:
mutex_unlock(&rm_port->port.mutex);
tty_port_put(&rm_port->port);
return ret;
}
static int rsc_mgr_console_exit(struct console *co)
{
int ret;
struct rm_cons_port *rm_port = co->data;
mutex_lock(&rm_port->port.mutex);
rm_port->port.console = false;
if (!tty_port_active(&rm_port->port)) {
ret = gh_rm_console_close(rm_port->vmid);
if (ret)
dev_err(rm_port->port.tty->dev, "Failed to close %s%d: %d\n",
co->name, rm_port->index, ret);
tty_port_set_initialized(&rm_port->port, false);
}
mutex_unlock(&rm_port->port.mutex);
tty_port_put(&rm_port->port);
return 0;
}
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int rsc_mgr_console_exit(struct console *co)
>> +{
>> + int ret;
>> + struct rm_cons_port *rm_port = co->data;
>> +
>> + if (rm_port->open) {
>> + ret = gh_rm_console_close(rm_port->vmid);
>> + if (ret) {
>> + pr_err("Failed to close RM console for vmid %x: %d\n", rm_port->vmid, ret);
>> + return ret;
>> + }
>> + rm_port->open = false;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static const struct tty_operations rsc_mgr_tty_ops = {
>> + .open = rsc_mgr_tty_open,
>> + .close = rsc_mgr_tty_close,
>> + .write = rsc_mgr_tty_write,
>> + .write_room = rsc_mgr_mgr_tty_write_room,
>> +};
>> +
>> +static void rsc_mgr_port_destruct(struct tty_port *port)
>> +{
>> + struct rm_cons_port *rm_port = container_of(port, struct rm_cons_port, port);
>> + struct rm_cons_data *cons_data = rm_port->cons_data;
>> +
>> + spin_lock(&cons_data->ports_lock);
>> + WARN_ON(cons_data->ports[rm_port->index] != rm_port);
>
> Does this mean you just crashed the system if something went wrong?
>
> How can this ever happen?
>
>
This can't happen and was added defensively. Will drop.
>> + cons_data->ports[rm_port->index] = NULL;
>> + spin_unlock(&cons_data->ports_lock);
>> + kfree(rm_port);
>> +}
>> +
>> +static const struct tty_port_operations rsc_mgr_port_ops = {
>> + .destruct = rsc_mgr_port_destruct,
>> +};
>> +
>> +static struct rm_cons_port *rsc_mgr_port_create(struct rm_cons_data *cons_data, u16 vmid)
>> +{
>> + struct rm_cons_port *rm_port;
>> + struct device *ttydev;
>> + unsigned int index;
>> + int ret;
>> +
>> + rm_port = kzalloc(sizeof(*rm_port), GFP_KERNEL);
>> + rm_port->vmid = vmid;
>> + INIT_KFIFO(rm_port->put_fifo);
>> + spin_lock_init(&rm_port->fifo_lock);
>> + INIT_WORK(&rm_port->put_work, put_work_fn);
>> + tty_port_init(&rm_port->port);
>> + rm_port->port.ops = &rsc_mgr_port_ops;
>> +
>> + spin_lock(&cons_data->ports_lock);
>> + for (index = 0; index < RSC_MGR_TTY_ADAPTERS; index++) {
>> + if (!cons_data->ports[index]) {
>> + cons_data->ports[index] = rm_port;
>> + rm_port->index = index;
>> + break;
>> + }
>> + }
>> + spin_unlock(&cons_data->ports_lock);
>> + if (index >= RSC_MGR_TTY_ADAPTERS) {
>> + ret = -ENOSPC;
>> + goto err_put_port;
>> + }
>> +
>> + ttydev = tty_port_register_device_attr(&rm_port->port, cons_data->tty_driver, index,
>> + cons_data->dev, rm_port, rsc_mgr_tty_dev_attr_groups);
>> + if (IS_ERR(ttydev)) {
>> + ret = PTR_ERR(ttydev);
>> + goto err_put_port;
>> + }
>> +
>> + return rm_port;
>> +err_put_port:
>> + tty_port_put(&rm_port->port);
>> + return ERR_PTR(ret);
>> +}
>> +
>> +static int rsc_mgr_console_probe(struct auxiliary_device *auxdev,
>> + const struct auxiliary_device_id *aux_dev_id)
>> +{
>> + struct rm_cons_data *cons_data;
>> + struct rm_cons_port *rm_port;
>> + int ret;
>> + u16 vmid;
>> +
>> + cons_data = devm_kzalloc(&auxdev->dev, sizeof(*cons_data), GFP_KERNEL);
>> + if (!cons_data)
>> + return -ENOMEM;
>> + dev_set_drvdata(&auxdev->dev, cons_data);
>> + cons_data->dev = &auxdev->dev;
>> +
>> + cons_data->tty_driver = tty_alloc_driver(RSC_MGR_TTY_ADAPTERS,
>> + TTY_DRIVER_REAL_RAW | TTY_DRIVER_DYNAMIC_DEV);
>> + if (IS_ERR(cons_data->tty_driver))
>> + return PTR_ERR(cons_data->tty_driver);
>> +
>> + cons_data->tty_driver->driver_name = "gh";
>> + cons_data->tty_driver->name = "ttyGH";
>
> Where did you pick this name from?
>
> Where is it documented?
>
"GH" is the shorthand we've been using for "Gunyah". I didn't find
documentation for dynamically assigned char devices, but if it exists, I
can add entry for ttyGH.
>> + cons_data->tty_driver->type = TTY_DRIVER_TYPE_SYSTEM;
>> + cons_data->tty_driver->init_termios = tty_std_termios;
>> + tty_set_operations(cons_data->tty_driver, &rsc_mgr_tty_ops);
>> +
>> + ret = tty_register_driver(cons_data->tty_driver);
>> + if (ret) {
>> + dev_err(&auxdev->dev, "Could not register tty driver: %d\n", ret);
>> + goto err_put_tty;
>> + }
>> +
>> + spin_lock_init(&cons_data->ports_lock);
>> +
>> + cons_data->rsc_mgr_notif.notifier_call = rsc_mgr_console_notif;
>> + ret = gh_rm_register_notifier(&cons_data->rsc_mgr_notif);
>> + if (ret) {
>> + dev_err(&auxdev->dev, "Could not register for resource manager notifications: %d\n",
>> + ret);
>> + goto err_put_tty;
>> + }
>> +
>> + rm_port = rsc_mgr_port_create(cons_data, GH_VMID_SELF);
>> + if (IS_ERR(rm_port)) {
>> + ret = PTR_ERR(rm_port);
>> + dev_err(&auxdev->dev, "Could not create own console: %d\n", ret);
>> + goto err_unreg_notif;
>> + }
>> +
>> + strncpy(cons_data->console.name, "ttyGH", sizeof(cons_data->console.name));
>
> Again, where did this name come from?
>
> thanks,
>
> greg k-h
On Thu, Oct 06, 2022 at 10:59:51PM -0700, Elliot Berman wrote:
> > > + */
> > > +#define RSC_MGR_TTY_ADAPTERS 16
> >
> > We can have dynamic tty devices, so I don't understand this comment.
> > What really is the problem here?
> >
>
> Yes, I see the confusion. Dynamic device addition of tty devices is
> supported. As I understand, you need to know the maximum number of lines
> that could be added, and that is limitation I was referring to.
What do you mean by "lines"? That's not a tty kernel term.
> Is this comment better?
>
> The Linux TTY code requires us to know ahead of time how many lines we
> might need. Each line here corresponds to a VM. 16 seems like a
> reasonable number of lines for systems that are running Gunyah and using
> the provided console interface.
Again, line? Do you mean port?
> > > + cons_data->tty_driver->driver_name = "gh";
KBUILD_MODNAME?
> > > + cons_data->tty_driver->name = "ttyGH";
> >
> > Where did you pick this name from?
> >
> > Where is it documented?
> >
>
> "GH" is the shorthand we've been using for "Gunyah". I didn't find
> documentation for dynamically assigned char devices, but if it exists, I can
> add entry for ttyGH.
Why use a new name at all? Why not stick with the existing tty names
and device numbers?
thanks,
greg k-h
>> +
>> +/*
>> + * The Linux TTY code does not support dynamic addition of tty derived devices so we need to know
>> + * how many tty devices we might need when space is allocated for the tty device. Since VMs might be
>> + * added/removed dynamically, we need to make sure we have enough allocated.
>
> Wrap comments at 80 columns please.
>
checkpatch has extend LINE MAX to 100,do we still suggest wrap to 80?
On Sat, Oct 08, 2022 at 03:41:53PM +0800, Zhou Furong wrote:
>
> > > +
> > > +/*
> > > + * The Linux TTY code does not support dynamic addition of tty derived devices so we need to know
> > > + * how many tty devices we might need when space is allocated for the tty device. Since VMs might be
> > > + * added/removed dynamically, we need to make sure we have enough allocated.
> >
> > Wrap comments at 80 columns please.
> >
>
> checkpatch has extend LINE MAX to 100,do we still suggest wrap to 80?
For comment blocks, yes please.
On 10/7/2022 12:40 AM, Greg Kroah-Hartman wrote:
> On Thu, Oct 06, 2022 at 10:59:51PM -0700, Elliot Berman wrote:
>>
>> "GH" is the shorthand we've been using for "Gunyah". I didn't find
>> documentation for dynamically assigned char devices, but if it exists, I can
>> add entry for ttyGH.
>
> Why use a new name at all? Why not stick with the existing tty names
> and device numbers?
>
I can use hvc framework, although driver-level buffering is needed on
both the get_chars/put_chars paths because:
- get_chars wants to poll for characters, but Gunyah will push
characters to Linux
- put_chars can be called in atomic context in the printk console path.
Gunyah RM calls can sleep, so we add to buffer and queue work to
write the characters.
I also chose to use new tty driver because the Gunyah hypervisor call to
open the console (gh_rm_console_open) can only be done after starting
the VM. Gunyah will only forward characters sent from the other VM to
Linux after the gh_rm_console_open call is made. When launching a VM,
users would want to open console before VM starts so they can get
startup messages from the VM. I planned to use the carrier_raised() to
hold tty_port_block_until_ready until the VM is started and the
gh_rm_console_open() happens.
Thanks,
Elliot
On 10/3/2022 12:01 AM, Jiri Slaby wrote:
> On 28. 09. 22, 21:56, Elliot Berman wrote:
>> +struct rm_cons_port {
>> + struct tty_port port;
>> + u16 vmid;
>> + bool open;
>> + unsigned int index;
>> +
>> + DECLARE_KFIFO(put_fifo, char, 1024);
>
> Why is tty_port::xmit_fifo not enough?
>
xmit_fifo gave me some inspiration to avoid using KFIFO here and skip
extra an extra memcpy into/out of the FIFO.
I've also dropped out the FIFO usage for tty_operations and this
buffering is only used for printk console.
Thanks,
Elliot
On Sun, Oct 09, 2022 at 01:59:21PM -0700, Elliot Berman wrote:
>
>
> On 10/7/2022 12:40 AM, Greg Kroah-Hartman wrote:
> > On Thu, Oct 06, 2022 at 10:59:51PM -0700, Elliot Berman wrote:
> > >
> > > "GH" is the shorthand we've been using for "Gunyah". I didn't find
> > > documentation for dynamically assigned char devices, but if it exists, I can
> > > add entry for ttyGH.
> >
> > Why use a new name at all? Why not stick with the existing tty names
> > and device numbers?
> >
>
> I can use hvc framework, although driver-level buffering is needed on
> both the get_chars/put_chars paths because:
I'm not asking about the framework (although that is a good question,
you need to document why this has to be new.) I'm asking why pick a new
name? You will not have a name conflict in your system with this device
with any other tty name right?
> - get_chars wants to poll for characters, but Gunyah will push
> characters to Linux
> - put_chars can be called in atomic context in the printk console path.
> Gunyah RM calls can sleep, so we add to buffer and queue work to
> write the characters.
>
> I also chose to use new tty driver because the Gunyah hypervisor call to
> open the console (gh_rm_console_open) can only be done after starting the
> VM. Gunyah will only forward characters sent from the other VM to Linux
> after the gh_rm_console_open call is made. When launching a VM, users would
> want to open console before VM starts so they can get startup messages from
> the VM. I planned to use the carrier_raised() to hold
> tty_port_block_until_ready until the VM is started and the
> gh_rm_console_open() happens.
I'm sorry, but I don't understand this.
Why is this all a new api at all? What about the virtio api? Why not
just use that instead?
thanks,
greg k-h
On 10/10/2022 1:23 PM, Greg Kroah-Hartman wrote:
> On Sun, Oct 09, 2022 at 01:59:21PM -0700, Elliot Berman wrote:
>>
>>
>> On 10/7/2022 12:40 AM, Greg Kroah-Hartman wrote:
>>> On Thu, Oct 06, 2022 at 10:59:51PM -0700, Elliot Berman wrote:
>>>>
>>>> "GH" is the shorthand we've been using for "Gunyah". I didn't find
>>>> documentation for dynamically assigned char devices, but if it exists, I can
>>>> add entry for ttyGH.
>>>
>>> Why use a new name at all? Why not stick with the existing tty names
>>> and device numbers?
>>>
>>
>> I can use hvc framework, although driver-level buffering is needed on
>> both the get_chars/put_chars paths because:
>
> I'm not asking about the framework (although that is a good question,
> you need to document why this has to be new.) I'm asking why pick a new
> name? You will not have a name conflict in your system with this device
> with any other tty name right?
>
That's correct, I didn't see any other instances of "ttyGH" in kernel.
>> - get_chars wants to poll for characters, but Gunyah will push
>> characters to Linux
>> - put_chars can be called in atomic context in the printk console path.
>> Gunyah RM calls can sleep, so we add to buffer and queue work to
>> write the characters.
>>
>> I also chose to use new tty driver because the Gunyah hypervisor call to
>> open the console (gh_rm_console_open) can only be done after starting the
>> VM. Gunyah will only forward characters sent from the other VM to Linux
>> after the gh_rm_console_open call is made. When launching a VM, users would
>> want to open console before VM starts so they can get startup messages from
>> the VM. I planned to use the carrier_raised() to hold
>> tty_port_block_until_ready until the VM is started and the
>> gh_rm_console_open() happens.
>
> I'm sorry, but I don't understand this.
>
> Why is this all a new api at all? What about the virtio api? Why not
> just use that instead?
We want to support virtual machines and Virtual Machine Managers (the
userspace component) that don't support virtio. Qualcomm has some
lightweight VMs that have a Gunyah driver stack but no virtio stack.
Further, implementing a simple userspace VMM to launch a Linux kernel is
much easier with the Gunyah console as no device paravirtualization is
needed to get some output from Linux. I don't anticipate these VMs to be
commonplace, but they do exist.
Thanks,
Elliot
On Mon, Oct 10, 2022 at 01:58:00PM -0700, Elliot Berman wrote:
>
>
> On 10/10/2022 1:23 PM, Greg Kroah-Hartman wrote:
> > On Sun, Oct 09, 2022 at 01:59:21PM -0700, Elliot Berman wrote:
> > >
> > > > > > On 10/7/2022 12:40 AM, Greg Kroah-Hartman wrote:
> > > > On Thu, Oct 06, 2022 at 10:59:51PM -0700, Elliot Berman wrote:
> > > > >
> > > > > "GH" is the shorthand we've been using for "Gunyah". I didn't find
> > > > > documentation for dynamically assigned char devices, but if it exists, I can
> > > > > add entry for ttyGH.
> > > >
> > > > Why use a new name at all? Why not stick with the existing tty names
> > > > and device numbers?
> > > >
> > >
> > > I can use hvc framework, although driver-level buffering is needed on
> > > both the get_chars/put_chars paths because:
> >
> > I'm not asking about the framework (although that is a good question,
> > you need to document why this has to be new.) I'm asking why pick a new
> > name? You will not have a name conflict in your system with this device
> > with any other tty name right?
> >
>
> That's correct, I didn't see any other instances of "ttyGH" in kernel.
Do you see any instances of ttyS? Any other existing name? Why pick a
new name at all?
And if you do pick a new name, you need to document it really really
well, not bury it downn within the tty driver code.
thanks,
greg k-h
On 10/10/2022 11:22 PM, Greg Kroah-Hartman wrote:
> On Mon, Oct 10, 2022 at 01:58:00PM -0700, Elliot Berman wrote:
>>
>>
>> On 10/10/2022 1:23 PM, Greg Kroah-Hartman wrote:
>>> On Sun, Oct 09, 2022 at 01:59:21PM -0700, Elliot Berman wrote:
>>>>
>>>>>>> On 10/7/2022 12:40 AM, Greg Kroah-Hartman wrote:
>>>>> On Thu, Oct 06, 2022 at 10:59:51PM -0700, Elliot Berman wrote:
>>>>>>
>>>>>> "GH" is the shorthand we've been using for "Gunyah". I didn't find
>>>>>> documentation for dynamically assigned char devices, but if it exists, I can
>>>>>> add entry for ttyGH.
>>>>>
>>>>> Why use a new name at all? Why not stick with the existing tty names
>>>>> and device numbers?
>>>>>
>>>>
>>>> I can use hvc framework, although driver-level buffering is needed on
>>>> both the get_chars/put_chars paths because:
>>>
>>> I'm not asking about the framework (although that is a good question,
>>> you need to document why this has to be new.) I'm asking why pick a new
>>> name? You will not have a name conflict in your system with this device
>>> with any other tty name right?
>>>
>>
>> That's correct, I didn't see any other instances of "ttyGH" in kernel.
>
> Do you see any instances of ttyS? Any other existing name? Why pick a
> new name at all?
>
> And if you do pick a new name, you need to document it really really
> well, not bury it downn within the tty driver code.
>
Seems other drivers are adding a comment in Kconfig help text. I can do
the same.
On Tue, Oct 11, 2022 at 03:04:08PM -0700, Elliot Berman wrote:
>
>
> On 10/10/2022 11:22 PM, Greg Kroah-Hartman wrote:
> > On Mon, Oct 10, 2022 at 01:58:00PM -0700, Elliot Berman wrote:
> > >
> > >
> > > On 10/10/2022 1:23 PM, Greg Kroah-Hartman wrote:
> > > > On Sun, Oct 09, 2022 at 01:59:21PM -0700, Elliot Berman wrote:
> > > > >
> > > > > > > > On 10/7/2022 12:40 AM, Greg Kroah-Hartman wrote:
> > > > > > On Thu, Oct 06, 2022 at 10:59:51PM -0700, Elliot Berman wrote:
> > > > > > >
> > > > > > > "GH" is the shorthand we've been using for "Gunyah". I didn't find
> > > > > > > documentation for dynamically assigned char devices, but if it exists, I can
> > > > > > > add entry for ttyGH.
> > > > > >
> > > > > > Why use a new name at all? Why not stick with the existing tty names
> > > > > > and device numbers?
> > > > > >
> > > > >
> > > > > I can use hvc framework, although driver-level buffering is needed on
> > > > > both the get_chars/put_chars paths because:
> > > >
> > > > I'm not asking about the framework (although that is a good question,
> > > > you need to document why this has to be new.) I'm asking why pick a new
> > > > name? You will not have a name conflict in your system with this device
> > > > with any other tty name right?
> > > >
> > >
> > > That's correct, I didn't see any other instances of "ttyGH" in kernel.
> >
> > Do you see any instances of ttyS? Any other existing name? Why pick a
> > new name at all?
> >
> > And if you do pick a new name, you need to document it really really
> > well, not bury it downn within the tty driver code.
> >
>
> Seems other drivers are adding a comment in Kconfig help text. I can do the
> same.
If you really want this, yes. But again, you have not justified a whole
new name yet...
On 10/3/22 00:38, Joe Perches wrote:
> On Mon, 2022-10-03 at 07:29 +0200, Greg Kroah-Hartman wrote:
>> On Sun, Oct 02, 2022 at 06:46:30PM -0700, Joe Perches wrote:
>>> On Fri, 2022-09-30 at 14:22 +0200, Greg Kroah-Hartman wrote:
>>>> On Wed, Sep 28, 2022 at 12:56:31PM -0700, Elliot Berman wrote:
>>>>> Gunyah resource manager defines a simple API for virtual machine log
>>>>> sharing with the console service.
>>> []
>>>>> diff --git a/include/linux/gunyah_rsc_mgr.h b/include/linux/gunyah_rsc_mgr.h
>>> []
>>>>> +struct gh_rm_notif_vm_console_chars {
>>>>> + u16 vmid;
>>>>> + u16 num_bytes;
>>>>> + u8 bytes[0];
>>>>
>>>> Please do not use [0] for new structures, otherwise we will just have to
>>>> fix them up again as we are trying to get rid of all of these from the
>>>> kernel. Just use "bytes[];" instead.
>>>
>>> Maybe a checkpatch addition like:
>>> ---
> []
>>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> []
>>> @@ -3948,6 +3948,17 @@ sub process {
>>> }
>>> }
>>>
>>> +# check for zero length array declarations in likely structs
>>> + if ($line =~ /^\+\t($Declare\s*$Ident)\s*\[\s*0\s*\]\s*;\s*$/ &&
>>> + defined $lines[$linenr] &&
>>> + $lines[$linenr] =~ /^[\+ ]\}\s*(?:__\w+\s*(?:$balanced_parens)?)\s*;\s*$/) {
This sounds great. We need the same for one-element arrays. :)
Both zero-length and one-element arrays are deprecated.
>
> This should actually be:
>
> $lines[$linenr] =~ /^[\+ ]\}(?:\s*__\w+\s*(?:$balanced_parens)?)*\s*;\s*$/) {
I agree. Thanks.
>
> as it was missing a * for uses like
>
> int foo[0];
> } __packed __aligned(4);
>
> and uses without any attribute at all
>
>>> + if (WARN("FLEXIBLE_ARRAY_ZERO",
>>> + "Prefer flexible length array declarations with [] over [0]\n" . $herecurr) &&
>>> + $fix) {
>>> + $fixed[$fixlinenr] =~ s/\[\s*0\s*\]/[]/;
>>> + }
>>> + }
>>> +
>>> # check for multiple consecutive blank lines
>>> if ($prevline =~ /^[\+ ]\s*$/ &&
>>> $line =~ /^\+\s*$/ &&
>>
>> This is a question for Gustavo, who did all the work here. Gustavo,
>> does the above checkpatch change look good to you?
Yep; the idea is great. :)
Another alternative to stop those fake flex-arrays from entering the codebase
is to run Coccinelle scripts during linux-next builds, as suggested by Elena
Reshetova at LSSEU a couple of months ago.
However, if these can be stopped with checkpatch it'd be really helpful, as well.
Thanks
--
Gustavo