[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6739f846-141f-d23d-536a-e00d18eb11be@linaro.org>
Date: Fri, 31 Mar 2023 09:26:46 -0500
From: Alex Elder <elder@...aro.org>
To: Elliot Berman <quic_eberman@...cinc.com>,
Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
Prakruthi Deepak Heragu <quic_pheragu@...cinc.com>
Cc: Murali Nalajala <quic_mnalajal@...cinc.com>,
Trilok Soni <quic_tsoni@...cinc.com>,
Srivatsa Vaddagiri <quic_svaddagi@...cinc.com>,
Carl van Schaik <quic_cvanscha@...cinc.com>,
Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio <konrad.dybcio@...aro.org>,
Arnd Bergmann <arnd@...db.de>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Jonathan Corbet <corbet@....net>,
Bagas Sanjaya <bagasdotme@...il.com>,
Will Deacon <will@...nel.org>, Andy Gross <agross@...nel.org>,
Catalin Marinas <catalin.marinas@....com>,
Jassi Brar <jassisinghbrar@...il.com>,
linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v11 15/26] gunyah: rsc_mgr: Add platform ops on
mem_lend/mem_reclaim
On 3/3/23 7:06 PM, Elliot Berman wrote:
> On Qualcomm platforms, there is a firmware entity which controls access
> to physical pages. In order to share memory with another VM, this entity
> needs to be informed that the guest VM should have access to the memory.
Will Gunyah ever be used on something other than a Qualcomm
platform?
Is there really any need to have these "platform hooks"
conditionally compiled?
One other comment below.
-Alex
> Co-developed-by: Prakruthi Deepak Heragu <quic_pheragu@...cinc.com>
> Signed-off-by: Prakruthi Deepak Heragu <quic_pheragu@...cinc.com>
> Signed-off-by: Elliot Berman <quic_eberman@...cinc.com>
> ---
> drivers/virt/gunyah/Kconfig | 4 ++
> drivers/virt/gunyah/Makefile | 1 +
> drivers/virt/gunyah/gunyah_platform_hooks.c | 80 +++++++++++++++++++++
> drivers/virt/gunyah/rsc_mgr.h | 3 +
> drivers/virt/gunyah/rsc_mgr_rpc.c | 18 ++++-
> include/linux/gunyah_rsc_mgr.h | 17 +++++
> 6 files changed, 121 insertions(+), 2 deletions(-)
> create mode 100644 drivers/virt/gunyah/gunyah_platform_hooks.c
>
> diff --git a/drivers/virt/gunyah/Kconfig b/drivers/virt/gunyah/Kconfig
> index 1a737694c333..de815189dab6 100644
> --- a/drivers/virt/gunyah/Kconfig
> +++ b/drivers/virt/gunyah/Kconfig
> @@ -4,6 +4,7 @@ config GUNYAH
> tristate "Gunyah Virtualization drivers"
> depends on ARM64
> depends on MAILBOX
> + select GUNYAH_PLATFORM_HOOKS
> help
> The Gunyah drivers are the helper interfaces that run in a guest VM
> such as basic inter-VM IPC and signaling mechanisms, and higher level
> @@ -11,3 +12,6 @@ config GUNYAH
>
> Say Y/M here to enable the drivers needed to interact in a Gunyah
> virtual environment.
> +
> +config GUNYAH_PLATFORM_HOOKS
> + tristate
> diff --git a/drivers/virt/gunyah/Makefile b/drivers/virt/gunyah/Makefile
> index ff8bc4925392..6b8f84dbfe0d 100644
> --- a/drivers/virt/gunyah/Makefile
> +++ b/drivers/virt/gunyah/Makefile
> @@ -1,6 +1,7 @@
> # SPDX-License-Identifier: GPL-2.0
>
> obj-$(CONFIG_GUNYAH) += gunyah.o
> +obj-$(CONFIG_GUNYAH_PLATFORM_HOOKS) += gunyah_platform_hooks.o
>
> gunyah_rsc_mgr-y += rsc_mgr.o rsc_mgr_rpc.o vm_mgr.o vm_mgr_mm.o
> obj-$(CONFIG_GUNYAH) += gunyah_rsc_mgr.o
> diff --git a/drivers/virt/gunyah/gunyah_platform_hooks.c b/drivers/virt/gunyah/gunyah_platform_hooks.c
> new file mode 100644
> index 000000000000..60da0e154e98
> --- /dev/null
> +++ b/drivers/virt/gunyah/gunyah_platform_hooks.c
> @@ -0,0 +1,80 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/rwsem.h>
> +#include <linux/gunyah_rsc_mgr.h>
> +
> +#include "rsc_mgr.h"
> +
> +static struct gh_rm_platform_ops *rm_platform_ops;
> +static DECLARE_RWSEM(rm_platform_ops_lock);
> +
> +int gh_rm_platform_pre_mem_share(struct gh_rm *rm, struct gh_rm_mem_parcel *mem_parcel)
> +{
> + int ret = 0;
> +
> + down_read(&rm_platform_ops_lock);
> + if (rm_platform_ops && rm_platform_ops->pre_mem_share)
> + ret = rm_platform_ops->pre_mem_share(rm, mem_parcel);
> + up_read(&rm_platform_ops_lock);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(gh_rm_platform_pre_mem_share);
> +
> +int gh_rm_platform_post_mem_reclaim(struct gh_rm *rm, struct gh_rm_mem_parcel *mem_parcel)
> +{
> + int ret = 0;
> +
> + down_read(&rm_platform_ops_lock);
> + if (rm_platform_ops && rm_platform_ops->post_mem_reclaim)
> + ret = rm_platform_ops->post_mem_reclaim(rm, mem_parcel);
> + up_read(&rm_platform_ops_lock);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(gh_rm_platform_post_mem_reclaim);
> +
> +int gh_rm_register_platform_ops(struct gh_rm_platform_ops *platform_ops)
> +{
> + int ret = 0;
> +
> + down_write(&rm_platform_ops_lock);
> + if (!rm_platform_ops)
> + rm_platform_ops = platform_ops;
> + else
> + ret = -EEXIST;
> + up_write(&rm_platform_ops_lock);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(gh_rm_register_platform_ops);
> +
> +void gh_rm_unregister_platform_ops(struct gh_rm_platform_ops *platform_ops)
> +{
> + down_write(&rm_platform_ops_lock);
> + if (rm_platform_ops == platform_ops)
> + rm_platform_ops = NULL;
> + up_write(&rm_platform_ops_lock);
> +}
> +EXPORT_SYMBOL_GPL(gh_rm_unregister_platform_ops);
> +
> +static void _devm_gh_rm_unregister_platform_ops(void *data)
> +{
> + gh_rm_unregister_platform_ops(data);
> +}
> +
> +int devm_gh_rm_register_platform_ops(struct device *dev, struct gh_rm_platform_ops *ops)
> +{
> + int ret;
> +
> + ret = gh_rm_register_platform_ops(ops);
> + if (ret)
> + return ret;
> +
> + return devm_add_action(dev, _devm_gh_rm_unregister_platform_ops, ops);
> +}
> +EXPORT_SYMBOL_GPL(devm_gh_rm_register_platform_ops);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Gunyah Platform Hooks");
> diff --git a/drivers/virt/gunyah/rsc_mgr.h b/drivers/virt/gunyah/rsc_mgr.h
> index 3665ebc7b020..6838e736f361 100644
> --- a/drivers/virt/gunyah/rsc_mgr.h
> +++ b/drivers/virt/gunyah/rsc_mgr.h
> @@ -13,4 +13,7 @@ struct gh_rm;
> int gh_rm_call(struct gh_rm *rsc_mgr, u32 message_id, void *req_buff, size_t req_buf_size,
> void **resp_buf, size_t *resp_buf_size);
>
> +int gh_rm_platform_pre_mem_share(struct gh_rm *rm, struct gh_rm_mem_parcel *mem_parcel);
> +int gh_rm_platform_post_mem_reclaim(struct gh_rm *rm, struct gh_rm_mem_parcel *mem_parcel);
> +
> #endif
> diff --git a/drivers/virt/gunyah/rsc_mgr_rpc.c b/drivers/virt/gunyah/rsc_mgr_rpc.c
> index 3df15ad5b97d..733be4dc8dd2 100644
> --- a/drivers/virt/gunyah/rsc_mgr_rpc.c
> +++ b/drivers/virt/gunyah/rsc_mgr_rpc.c
> @@ -204,6 +204,12 @@ static int gh_rm_mem_lend_common(struct gh_rm *rm, u32 message_id, struct gh_rm_
> if (!msg)
> return -ENOMEM;
>
> + ret = gh_rm_platform_pre_mem_share(rm, p);
> + if (ret) {
> + kfree(msg);
> + return ret;
> + }
> +
> req_header = msg;
> acl_section = (void *)req_header + sizeof(*req_header);
> mem_section = (void *)acl_section + struct_size(acl_section, entries, p->n_acl_entries);
> @@ -227,8 +233,10 @@ static int gh_rm_mem_lend_common(struct gh_rm *rm, u32 message_id, struct gh_rm_
> ret = gh_rm_call(rm, message_id, msg, msg_size, (void **)&resp, &resp_size);
> kfree(msg);
>
> - if (ret)
> + if (ret) {
> + gh_rm_platform_post_mem_reclaim(rm, p);
> return ret;
> + }
>
> p->mem_handle = le32_to_cpu(*resp);
>
> @@ -283,8 +291,14 @@ int gh_rm_mem_reclaim(struct gh_rm *rm, struct gh_rm_mem_parcel *parcel)
> struct gh_rm_mem_release_req req = {
> .mem_handle = cpu_to_le32(parcel->mem_handle),
> };
> + int ret;
> +
> + ret = gh_rm_call(rm, GH_RM_RPC_MEM_RECLAIM, &req, sizeof(req), NULL, NULL);
> + /* Do not call platform mem reclaim hooks: the reclaim didn't happen*/
Move the comment above the gh_rm_call() call, and rephrase, such as:
/* Only call platform mem reclaim hooks if... */
> + if (ret)
> + return ret;
>
> - return gh_rm_call(rm, GH_RM_RPC_MEM_RECLAIM, &req, sizeof(req), NULL, NULL);
> + return gh_rm_platform_post_mem_reclaim(rm, parcel);
> }
>
> /**
> diff --git a/include/linux/gunyah_rsc_mgr.h b/include/linux/gunyah_rsc_mgr.h
> index 8b0b46f28e39..515087931a2b 100644
> --- a/include/linux/gunyah_rsc_mgr.h
> +++ b/include/linux/gunyah_rsc_mgr.h
> @@ -145,4 +145,21 @@ int gh_rm_get_hyp_resources(struct gh_rm *rm, u16 vmid,
> struct gh_rm_hyp_resources **resources);
> int gh_rm_get_vmid(struct gh_rm *rm, u16 *vmid);
>
> +struct gunyah_rm_platform_ops {
> + int (*pre_mem_share)(struct gh_rm *rm, struct gh_rm_mem_parcel *mem_parcel);
> + int (*post_mem_reclaim)(struct gh_rm *rm, struct gh_rm_mem_parcel *mem_parcel);
> +};
> +
> +#if IS_ENABLED(CONFIG_GUNYAH_PLATFORM_HOOKS)
> +int gh_rm_register_platform_ops(struct gh_rm_platform_ops *platform_ops);
> +void gh_rm_unregister_platform_ops(struct gh_rm_platform_ops *platform_ops);
> +int devm_gh_rm_register_platform_ops(struct device *dev, struct gh_rm_platform_ops *ops);
> +#else
> +static inline int gh_rm_register_platform_ops(struct gh_rm_platform_ops *platform_ops)
> + { return 0; }
> +static inline void gh_rm_unregister_platform_ops(struct gh_rm_platform_ops *platform_ops) { }
> +static inline int devm_gh_rm_register_platform_ops(struct device *dev,
> + struct gh_rm_platform_ops *ops) { return 0; }
> +#endif
> +
> #endif
Powered by blists - more mailing lists