Re: [PATCH v14 09/25] gunyah: vm_mgr: Introduce basic VM Manager

From: Bjorn Andersson <andersson@kernel.org>
To: Elliot Berman <quic_eberman@quicinc.com>
Cc: Alex Elder <elder@linaro.org>,
	Srinivas Kandagatla <srinivas.kandagatla@linaro.org>,
	Jonathan Corbet <corbet@lwn.net>,
	Prakruthi Deepak Heragu <quic_pheragu@quicinc.com>,
	Murali Nalajala <quic_mnalajal@quicinc.com>,
	Trilok Soni <quic_tsoni@quicinc.com>,
	Srivatsa Vaddagiri <quic_svaddagi@quicinc.com>,
	Carl van Schaik <quic_cvanscha@quicinc.com>,
	Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
	Konrad Dybcio <konrad.dybcio@linaro.org>,
	Arnd Bergmann <arnd@arndb.de>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Bagas Sanjaya <bagasdotme@gmail.com>,
	Will Deacon <will@kernel.org>, Andy Gross <agross@kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Jassi Brar <jassisinghbrar@gmail.com>,
	linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v14 09/25] gunyah: vm_mgr: Introduce basic VM Manager
Date: Sat, 5 Aug 2023 10:26:54 -0700	[thread overview]
Message-ID: <nczfl5gh2uwxyswgntzmwdhb5zlrybostekanvsrdyfv7sjwlp@fb4m2sz4ojoa> (raw)
In-Reply-To: <20230613172054.3959700-10-quic_eberman@quicinc.com>

On Tue, Jun 13, 2023 at 10:20:37AM -0700, Elliot Berman wrote:
> diff --git a/drivers/virt/gunyah/rsc_mgr.c b/drivers/virt/gunyah/rsc_mgr.c
> index 04c8e131d259f..a0faf126ee56e 100644
> --- a/drivers/virt/gunyah/rsc_mgr.c
> +++ b/drivers/virt/gunyah/rsc_mgr.c
> @@ -15,8 +15,10 @@
>  #include <linux/completion.h>
>  #include <linux/gunyah_rsc_mgr.h>
>  #include <linux/platform_device.h>
> +#include <linux/miscdevice.h>

'm' < 'p'

>  
>  #include "rsc_mgr.h"
> +#include "vm_mgr.h"
>  
>  #define RM_RPC_API_VERSION_MASK		GENMASK(3, 0)
>  #define RM_RPC_HEADER_WORDS_MASK	GENMASK(7, 4)
> @@ -130,6 +132,7 @@ struct gh_rm_connection {
>   * @cache: cache for allocating Tx messages
>   * @send_lock: synchronization to allow only one request to be sent at a time
>   * @nh: notifier chain for clients interested in RM notification messages
> + * @miscdev: /dev/gunyah
>   */
>  struct gh_rm {
>  	struct device *dev;
> @@ -146,6 +149,8 @@ struct gh_rm {
>  	struct kmem_cache *cache;
>  	struct mutex send_lock;
>  	struct blocking_notifier_head nh;
> +
> +	struct miscdevice miscdev;
>  };
>  
>  /**
> @@ -580,6 +585,33 @@ int gh_rm_notifier_unregister(struct gh_rm *rm, struct notifier_block *nb)
>  }
>  EXPORT_SYMBOL_GPL(gh_rm_notifier_unregister);
>  
> +struct device *gh_rm_get(struct gh_rm *rm)
> +{
> +	return get_device(rm->miscdev.this_device);

Please add a comment to why the VM is parented off the miscdevice and
not the RM itself, when the function name indicates that you're
acquiring a reference to the rm...

> +}
> +EXPORT_SYMBOL_GPL(gh_rm_get);
> +
[..]
> diff --git a/drivers/virt/gunyah/vm_mgr.c b/drivers/virt/gunyah/vm_mgr.c
> new file mode 100644
> index 0000000000000..a43401cb34f7d
> --- /dev/null
> +++ b/drivers/virt/gunyah/vm_mgr.c
> @@ -0,0 +1,93 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +
> +#define pr_fmt(fmt) "gh_vm_mgr: " fmt

Unused?

Regards,
Bjorn