Messages in this thread | ![/](/images/icornerl.gif) | | Date | Wed, 22 Mar 2023 12:17:27 -0700 | Subject | Re: [PATCH v11 15/26] gunyah: rsc_mgr: Add platform ops on mem_lend/mem_reclaim | From | Elliot Berman <> |
| |
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@quicinc.com> >> Signed-off-by: Prakruthi Deepak Heragu <quic_pheragu@quicinc.com> >> Signed-off-by: Elliot Berman <quic_eberman@quicinc.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
| ![\](/images/icornerr.gif) |