by Simon Horman

[permalink] [raw]

Subject: Re: [PATCH v10 19/21] virt: geniezone: Provide individual VM memory statistics within debugfs

On Fri, Apr 12, 2024 at 02:57:16PM +0800, Yi-De Wu wrote:
> From: "Jerry Wang" <[email protected]>
>
> Created a dedicated per-VM debugfs folder under gzvm, providing
> user-level programs with easy access to per-VM memory statistics for
> debugging and profiling purposes. This enables users to effectively
> analyze and optimize the memory usage of individual virtual machines.
>
> Two types of information can be obtained:
>
> `cat /sys/kernel/debug/gzvm/<pid>-<vmid>/protected_hyp_mem` shows memory
> used by the hypervisor and the size of the stage 2 table in bytes.
>
> `cat /sys/kernel/debug/gzvm/<pid>-<vmid>/protected_shared_mem` gives
> memory used by the shared resources of the guest and host in bytes.
>
> For example:
> console:/ # cat /sys/kernel/debug/gzvm/3417-15/protected_hyp_mem
> 180328
> console:/ # cat /sys/kernel/debug/gzvm/3417-15/protected_shared_mem
> 262144
> console:/ #
>
> More stats will be added in the future.
>
> Signed-off-by: Jerry Wang <[email protected]>
> Signed-off-by: Liju-Clr Chen <[email protected]>
> Signed-off-by: Yi-De Wu <[email protected]>

..

> diff --git a/drivers/virt/geniezone/gzvm_vm.c b/drivers/virt/geniezone/gzvm_vm.c

..

> @@ -398,6 +409,113 @@ static void setup_vm_demand_paging(struct gzvm *vm)
> }
> }
>
> +static int debugfs_open(struct inode *inode, struct file *file)
> +{
> + file->private_data = inode->i_private;
> + return 0;
> +}

nit: Coccinelle suggests that simple_open() can be used in place
of the debugfs_open() implementation above.

..

> +static const struct file_operations hyp_mem_fops = {
> + .owner = THIS_MODULE,
> + .open = debugfs_open,
> + .read = hyp_mem_read,
> + .llseek = no_llseek,
> +};
> +
> +static const struct file_operations shared_mem_fops = {
> + .owner = THIS_MODULE,
> + .open = debugfs_open,
> + .read = shared_mem_read,
> + .llseek = no_llseek,
> +};

..

2024-04-15 17:07:00

by Conor Dooley

[permalink] [raw]

Subject: Re: [PATCH v10 03/21] dt-bindings: hypervisor: Add MediaTek GenieZone hypervisor

On Fri, Apr 12, 2024 at 02:57:00PM +0800, Yi-De Wu wrote:
> From: "Yingshiuan Pan" <[email protected]>
>
> Add documentation for GenieZone(gzvm) node. This node informs gzvm
> driver to start probing if geniezone hypervisor is available and
> able to do virtual machine operations.
>
> [Reason to use dt solution]
> - The GenieZone hypervisor serves as a vendor model for facilitating
> platform virtualization, with an implementation that is independent
> from Linuxism.
> - In contrast to the dt solution, our previous approach involved probing
> via hypercall to determine the existence of our hypervisor. However, this
> method raised concerns about potentially impacting all systems, including
> those without the GenieZone hypervisor embedded[ref].
>
> Link: https://lore.kernel.org/all/[email protected]/

> +properties:
> + compatible:
> + const: mediatek,geniezone-hyp

Been avoiding this binding every time it shows up because Rob had
already told you no and hasn't revisited it since, but I feel this
should be s/-hyp// since that's redundant information.

> +description:
> + This interface is designed for integrating GenieZone hypervisor into Android
> + Virtualization Framework(AVF) along with Crosvm as a VMM.
> + It acts like a wrapper for every hypercall to GenieZone hypervisor in
> + order to control guest VM lifecycles and virtual interrupt injections.

The description however doesn't really make sense. The binding claims to
be for geniezone but the description talks about something else entirely
that "acts like a wrapper" between the OS and geniezone. What is the
binding actually for?

Attachments:

(No filename) (1.66 kB)
signature.asc (235.00 B)
Download all attachments