[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1b893760-0be7-e9f5-24c8-3e6dcbe8e237@quicinc.com>
Date: Wed, 22 Mar 2023 12:17:27 -0700
From: Elliot Berman <quic_eberman@...cinc.com>
To: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
Alex Elder <elder@...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/21/2023 7:23 AM, Srinivas Kandagatla wrote:
>
>
> On 04/03/2023 01:06, 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.
>>
>> 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)
>
> I think I have asked this question but I can not find the answer to this
> from old replies.
> Why is this platform hooks not part of core gunyah? Do we need a
> dedicated module for this.
> By the looks of APIs I see this is very much close to rm and i think
> this functionality should live with rm.
>
If the platform hooks were part of core gunyah, then core gunyah could
only be enabled as =y in the default defconfig because QCOM_SCM would
rely on the platform hooks (core gunyah).
With the suggestion to move the SCM platform hooks into separate module,
I can/will bring the platform hooks back into core gunyah module.
- Elliot
> --srini
> +{
>> + 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*/
>> + 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