| Message ID | 20220928195633.2348848-1-quic_eberman@quicinc.com |
|---|---|
| Headers | show |
| Series | Drivers for gunyah hypervisor | expand |
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.
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 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: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 <quic_eberman@quicinc.com> > --- > 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: linux-arm-msm@vger.kernel.org > +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 <quic_eberman@quicinc.com> > --- > 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 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 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 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 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 <quic_eberman@quicinc.com> >> --- >> 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: linux-arm-msm@vger.kernel.org >> +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 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
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/20220811214107.1074343-1-quic_eberman@quicinc.com/ - /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/20220801211240.597859-1-quic_eberman@quicinc.com/ - DT bindings clean up - Switch hypercalls to follow SMCCC v1: https://lore.kernel.org/all/20220223233729.1571114-1-quic_eberman@quicinc.com/ 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